From bcdf36bd4891b5cfaa62a8d9874767ede8dfcde8 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 24 Sep 2026 08:19:59 -0700 Subject: [PATCH] update spec files --- .kiro/specs/calculator/design.md | 609 +++++++++++++++++++------ .kiro/specs/calculator/requirements.md | 35 +- .kiro/specs/calculator/tasks.md | 64 ++- 3 files changed, 548 insertions(+), 160 deletions(-) diff --git a/.kiro/specs/calculator/design.md b/.kiro/specs/calculator/design.md index 9ca2603..f5573fe 100644 --- a/.kiro/specs/calculator/design.md +++ b/.kiro/specs/calculator/design.md @@ -9,44 +9,57 @@ ## 1. System Architecture ``` -┌─────────────────────────────────────────────────────────────┐ -│ Frontends │ -├─────────────┬──────────────────┬────────────────────────────┤ -│ CLI │ TUI │ Android │ -│ (Zig) │ (Zig+libvaxis) │ (Kotlin/Compose) │ -└──────┬──────┴────────┬─────────┴──────────────┬─────────────┘ - │ │ │ - │ Zig import │ Zig import │ JNI (C ABI) - │ │ │ -┌──────▼───────────────▼────────────────────────▼─────────────┐ -│ Engine (Zig Library) │ -├─────────────────────────────────────────────────────────────┤ -│ parser │ evaluator │ programmer │ struct_layout │ -│ │ │ │ │ -│ tokens │ standard │ bitwise ops │ abi_profiles │ -│ ast │ financial │ base conv │ layout_compute │ -└─────────────────────────────────────────────────────────────┘ ++-------------------------------------------------------------+ +| Frontends | ++-------------+------------------+----------------------------+ +| CLI | TUI | Android | +| (Zig) | (Zig+libvaxis) | (Kotlin/Compose) | ++------+------+--------+---------+------------+---------------+ + | | | + | Zig import | Zig import | JNI (C ABI, not built yet) + | | | ++------v---------------v----------------------v---------------+ +| Engine (Zig library) | ++-------------------------------------------------------------+ +| Parser.zig | evaluator.zig | programmer.zig units.zig | +| tokenizer | financial.zig | bitwise.zig float_interp| +| ast.zig | number/Rational/Integer grouping.zig| ++-------------------------------------------------------------+ ``` -The engine is a pure Zig library with no I/O. Frontends depend on the engine; the engine depends on nothing outside `std`. +The engine is a pure Zig library with no I/O. Frontends depend on the engine; the +engine depends on nothing outside `std`. The CLI and TUI ship today; the Android +column is a plan, and `c_api.zig` is the seam it will use (section 6). ### Build Graph +One `build.zig` at the workspace root. The per-frontend build files this section +used to describe (`engine/build.zig`, `cli/build.zig`, `tui/build.zig`) were never +written: the engine is one import the two frontends share, and splitting the build +would buy nothing. + ``` -build.zig (workspace root) -|- engine/build.zig -> produces: static lib (.a), C header, shared lib (.so/.dylib/.dll) -|- cli/build.zig -> produces: statically-linked binary -|- tui/build.zig -> produces: statically-linked binary (links libvaxis) -|- android targets -> produces: libtally.so for arm64-v8a, x86_64 - (consumed by android/ Gradle project for APK packaging) +build.zig +|- import "engine" <- engine/src/engine.zig, shared by both frontends +|- exe "tally" <- src/main.zig (CLI + TUI in one binary) +|- lib "tally" <- static and shared libraries from engine/src/engine.zig +|- step "test" <- three test binaries: engine, CLI, TUI +|- step "coverage" <- kcov over the same three, one report each + (build/Coverage.zig, kcov downloaded on first use) +|- step "run" <- runs the binary ``` **Build system philosophy:** - `zig build` is the single entry point for all compilation. No Makefile, no CMake, no shell scripts. -- `zig build` (no args) -> builds engine + CLI + TUI for the host platform. -- `zig build -Dtarget=aarch64-linux-android` -> cross-compiles engine .so for Android arm64. -- `zig build test` -> runs all engine unit tests with coverage reporting. -- The `android/` Gradle project handles only APK packaging and Compose UI compilation - it does not invoke Zig. Prebuilt .so files are committed or produced by a prior `zig build` step. +- `zig build` (no args) -> builds the engine libraries and the `tally` binary for the host. +- `zig build test` -> runs every test in the three trees. +- `zig build coverage -Dcoverage-threshold=N` -> the same tests under kcov, failing below the threshold. +- Android cross-compilation (`-Dtarget=aarch64-linux-android`) is not wired up yet. When it is, the `android/` Gradle project will handle only APK packaging and Compose UI compilation; it will not invoke Zig. + +**Toolchain management (mise):** +- `.mise.toml` at project root declares: Zig version, any other needed tools. +- `mise install` provisions the full dev environment. No other manual setup required. +- CI uses the same `.mise.toml` for reproducibility. **Toolchain management (mise):** - `.mise.toml` at project root declares: Zig version, any other needed tools. @@ -57,9 +70,9 @@ build.zig (workspace root) ## 2. Engine Design -### 2.1 Module Breakdown +### 2.1 File Breakdown -| Module | Responsibility | +| File | Responsibility | |--------|---------------| | `grouping.zig` | The thousands rule, shared by both display paths | | `Integer.zig` | A fixed-width integer (file-as-struct): `raw`, `width`, `signedness`, the interpretations, and the notations it renders in | @@ -75,18 +88,18 @@ build.zig (workspace root) | `units.zig` | Unit conversion tables and resolver | | `financial.zig` | CAGR, TVM, compound interest, amortization, and money display (cents are a property of money, not of a screen) | | `message.zig` | Gone: the wording lives in `engine.zig` beside the `Error` union it words | -| `engine.zig` | Public API surface (Zig-native): the seven modules a frontend imports, nine name aliases, the derived `Error` union, and `phrase` | -| `c_api.zig` | `extern "C"` wrappers for JNI/FFI consumers | +| `engine.zig` | Public API surface (Zig-native): the seven files a frontend imports, nine name aliases, the derived `Error` union, and `phrase` | +| `c_api.zig` | `extern "C"` seam for JNI/FFI consumers. Two of its three exports are stubs; see section 6 | -Every module declares its own error set; there is no shared one (section 10). +Every file declares its own error set; there is no shared one (section 10). There is deliberately no `types.zig` and no `errors.zig`. `types.zig` existed, and being named after a language feature rather than a concept, it accumulated four unrelated groups: the error vocabulary, the fixed-width integer model, `Base` (used only by the lexer and the AST), and `Mode` (which the engine stored and never read). `errors.zig` was where the error vocabulary landed, until the same question showed that -what needed splitting was the type inside it. Each piece has gone to the module that -owns it. `struct_layout.zig` is still unimplemented (Phase 4). +what needed splitting was the type inside it. Each piece has gone to the file that +owns it. `struct_layout.zig` does not exist; section 3 is a design, not a description. A file is named TitleCase when the file *is* the type, with its fields at container level: `Integer.zig`, `Rational.zig` and `Parser.zig`. `number.zig` stays lowercase @@ -99,37 +112,42 @@ AST and both evaluators without a tokenizer anywhere in sight. ### 2.2 Core Data Types +The result of every calculation is a `Number`: two tiers, one type (section 2.7). + ```zig -/// Result of any calculation -pub const Value = union(enum) { - integer: Integer, - float: f64, - boolean: bool, - struct_layout: StructLayoutResult, - multi_base: MultiBaseResult, // programmer mode result +/// number.zig +pub const Number = union(enum) { + exact: Rational, + inexact: f64, }; -pub const Integer = struct { - /// Raw bits stored in u64, interpretation depends on context - raw: u64, - bit_width: BitWidth, - signedness: Signedness, -}; +/// Integer.zig, the fixed-width value programmer mode computes on. The file IS the +/// type, so these are container-level fields. +raw: u128, // two's complement bit pattern, may carry bits above the width +width: BitWidth = .bits64, +signedness: Signedness = .signed, -pub const BitWidth = enum { bits8, bits16, bits32, bits64 }; +pub const BitWidth = enum(u8) { bits8 = 8, bits16 = 16, bits32 = 32, bits64 = 64, bits128 = 128 }; pub const Signedness = enum { signed, unsigned }; - -pub const MultiBaseResult = struct { - value: Integer, - decimal_signed: []const u8, // formatted string - decimal_unsigned: []const u8, - hex: []const u8, - octal: []const u8, - binary: []const u8, - bit_pattern: [64]u1, // individual bits, index 0 = LSB -}; ``` +An earlier version of this section described a `Value` union with `integer`, `float`, +`boolean`, `struct_layout` and `multi_base` members, and a `MultiBaseResult` carrying +five preformatted strings and a `[64]u1` bit array. None of it was built, and the +reasons are worth keeping: + +- **`Value` was never the result type.** The exact tier needs one type that can be + either exact or inexact and can move between the two, which is what `Number` is. A + union with a `boolean` member also promised comparisons the language does not have. +- **`MultiBaseResult` was a frontend's job.** A result does not need to carry five + renderings of itself: `Integer.fmt(.hex, .{})` renders on demand, at the width and + byte order the caller asks for, and the bit grid reads the bits it needs from `raw`. + Precomputing all five meant deciding separator style and grouping inside the engine, + which is exactly the decision NFR-9.9 puts in the frontend. +- **`raw` is a `u128`, not a `u64`.** 128-bit width is supported, so the pattern needs + the width to match; `bit_width` is spelled `width` because the file is already + `Integer`. + ### 2.3 AST Nodes The shape as implemented (`engine/src/ast.zig`); the earlier sketch here omitted @@ -302,11 +320,16 @@ at a width it did not come from. Byte order is `std.builtin.Endian` rather than an engine enum of the same two members. -Standard mode evaluates to `Number`, the exact/inexact union of section 2.7. Programmer mode evaluates to `Integer`, which is a `u128` pattern plus the `IntType` that interprets it. Financial functions compute in `f64` and enter the expression language as inexact `Number` values (section 5.6). +Standard mode evaluates to `Number`, the exact/inexact union of section 2.7. Programmer mode evaluates to `Integer`, which is a `u128` pattern plus the width and signedness that interpret it (the nested `IntType` this used to name is gone; the fields sit on `Integer` itself). ### 2.6 Number Display Formatting -The engine provides raw values; frontends apply display formatting. The engine includes a formatting module that produces display strings and raw (clipboard-friendly) strings separately. +The engine provides values that can render themselves at a budget the caller supplies; +frontends decide the budget (NFR-9.9) and the sink. There is no separate formatter: +`Number.render` and `Integer.fmt` are methods on the values, and both go through +`grouping.zig` for the thousands rule so every surface groups digits identically +(NFR-7). A `formatter.zig` held this once and was dismantled, because it had become a +second place where display decisions lived. #### Decimal Formatting @@ -730,12 +753,23 @@ in `rational.zig` that ordinary tests could never reach: separate statements, each with its own `errdefer`. `floor` and `factorial` were also missing an `errdefer` on their denominator. -The remaining uncovered lines in these modules are `unreachable` arms guarded +The remaining uncovered lines in these files are `unreachable` arms guarded upstream, and diagnostic paths inside the tests themselves. --- -## 3. Struct Layout Engine +## 3. Struct Layout Engine (DESIGN ONLY, NOT BUILT) + +Nothing in this section exists. There is no `struct_layout.zig`, no ABI profile +interface, no `tally struct` subcommand and no struct sub-view in the TUI, and the four +errors this design needs (`InvalidType`, `InvalidFieldName`, `DuplicateFieldName`, +`StructTooLarge`) were deleted from the engine because nothing could produce them. + +The feature is wanted (FR-3) and is sequenced after the Android app. The design is kept +because it is the one part of the plan with a grammar and an algorithm worked out in +advance, and because the shape below is still what we would build. + +Read what follows as a proposal. ### 3.1 Mini-DSL Grammar @@ -1116,7 +1150,7 @@ still reachable as functions (`amort_interest(...)` and friends). ### 5.6 Precision: f64 plus explicit rounding -This module is f64, not the exact `Number` tier used elsewhere (design 2.7). +This part of the engine is f64, not the exact `Number` tier used elsewhere (design 2.7). Every formula here needs a non-integer power or a logarithm - CAGR raises to `1/n`, the period solver takes `ln`, the rate solver iterates - and those escape the rationals by definition, so there is nothing for the exact tier to preserve. @@ -1131,6 +1165,16 @@ upward across many roundings. ## 6. C API (for Android/FFI) +**What exists today:** `engine/src/c_api.zig` is the root of the shared library +(`build.zig` builds `libtally.so` from it), and it declares three exports. +`tally_version` works. `tally_eval` and `tally_result_free` are signatures with +`// TODO: implement` bodies, so the library loads, exports the symbols, and answers +every evaluation with an empty `CalcResult`. No test builds it, so it appears in no +coverage report (open items 12 and 15). + +The rest of this section is the design, and the three decisions it does not yet make +are listed at the end. + ```zig // c_api.zig - extern "C" exports // @@ -1138,11 +1182,6 @@ upward across many roundings. // This avoids buffer overread vulnerabilities and allows embedded nulls. // All returned strings include a length - caller must free with tally_result_free(). -pub const CalcString = extern struct { - ptr: [*]const u8, - len: usize, -}; - pub const CalcResult = extern struct { /// JSON-encoded result. Null if error. json_ptr: ?[*]u8, @@ -1162,15 +1201,6 @@ export fn tally_eval( config_len: usize, // 0 if no config ) callconv(.c) CalcResult; -/// Compute struct layout, return JSON result. -export fn tally_struct_layout( - def_ptr: [*]const u8, - def_len: usize, - abi_ptr: [*]const u8, // "sysv", "win64", "packed" - abi_len: usize, - endian: c_int, // 0=little, 1=big -) callconv(.c) CalcResult; - /// Convert a value between units, return JSON result. export fn tally_convert( value: f64, @@ -1187,32 +1217,274 @@ export fn tally_result_free(result: *CalcResult) callconv(.c) void; export fn tally_version(out_len: *usize) callconv(.c) [*]const u8; ``` +A `tally_struct_layout` export was specified here too; it goes with section 3 and is +not part of the first Android surface. + Design notes: - **No null-terminated strings.** All inputs take `(ptr, len)` pairs. This prevents buffer overread, allows binary data in expressions if ever needed, and is the idiomatic C pattern for length-known strings. - **`CalcResult` struct** bundles success/error in a single return. Caller checks `json_ptr != null` for success. - **JSON result format** keeps the Android/FFI boundary simple - Kotlin parses JSON natively. On the JNI side, the Kotlin bridge reads `(ptr, len)` into a `ByteArray` and decodes UTF-8. +### 6.1 The decisions this design still owes + +The boundary is designed with the Android app rather than ahead of it: the JNI layer is +the only caller, so its access pattern decides the shape. Each decision below is +additive - the Zig API the CLI and TUI use needs nothing new to support any of them. + +1. **Session lifetime.** `Environment` holds the variables and the last answer, and + both frontends keep one alive across evaluations. A one-shot `tally_eval` cannot: + `x = 5` then `x * 2` would fail, and `Ans` would never have a value, on the surface + least able to compensate because a phone has no scrollback to retype from. The C API + needs an opaque handle (`tally_session_new` / `tally_session_free`, with every other + entry point taking one). Decide this first: it changes every signature. +2. **Memory ownership.** See 6.2, which is the longest of these because it is the one + that fails silently. +3. **Who chooses the display budget.** `Number.render` takes `FormatOptions`, and + NFR-9.9 says the frontend supplies them; a phone screen is not an 80-column terminal + and should not inherit one's digit counts. Either the options cross the ABI as + fields on the session config, or the C layer invents a budget and Android lives with + it. The CLI and TUI each declare their own, so there is a precedent to copy rather + than a question to answer from scratch. +4. **What the JSON carries.** A result is more than a string in three places: the + multi-base rows of programmer mode, the decomposition of the float view, and the + amortization schedule. The unit tables and the financial forms also need + enumerating, or Android has to hardcode lists the engine already owns. Rendering + inside the library keeps one grouping rule (NFR-7) and one set of words for errors + (`engine.phrase`), which argues for the JSON carrying rendered text alongside the + raw values rather than values alone. Whatever shape this takes is also what a CLI + `--json` would print, which is why the old FR-6.9 was removed rather than specified + separately. + +### 6.2 Memory ownership across the boundary + +Every engine entry point takes an `Allocator`; nothing allocates behind a caller's +back. That is a good position to start from and it means the ABI gets to choose, so the +choice should be made explicitly rather than inherited from the first export written. + +**What JNI actually does.** Kotlin declares an `external fun`, calls it, and reads the +returned bytes into a `ByteArray` or `String` before returning from the same JNI frame. +The copy is immediate and the app then owns a JVM object. Nothing on the Kotlin side +wants to hold a native pointer, and nothing can be trusted to free one on a predictable +schedule: a `ViewModel` is cleared whenever Android decides. + +**Options considered.** + +- *A library-owned global allocator*, with `tally_result_free` handing memory back to + it. This is the shape the current stub implies. It puts a mutable global in the + shipped library, which is the one property the engine does not have today (the only + container-level `var` in the tree is inside a test). It is also where the + cross-allocator free bug lives: any path that frees a result after its session is + gone, or through the wrong allocator, is a corruption rather than an error. A global + `DebugAllocator` never runs its leak check in a shared library either, so leaks go + unreported. +- *A caller-provided buffer*, snprintf style: the app passes a buffer and a capacity, + the library writes into it and reports the length it needed. No ownership question at + all, no free, thread-safe by construction, and every byte is accounted for as JVM + memory. The cost is the two-call pattern when a result does not fit, which for an + amortization schedule is the common case rather than the rare one. +- *Caller-provided allocator callbacks.* The app passes malloc/free function pointers + at session creation. Maximum control, and more machinery than a calculator needs. + +**The recommendation.** The session owns all of it, and results are borrowed: + +- `tally_session_new` allocates a session that holds the `Environment`, a scratch arena + reset at the end of every call, and a reusable result buffer. No global state: two + sessions share nothing, so a test and an app can hold one each. +- A result is a `(ptr, len)` into that session's result buffer, valid until the next + call on the same session or until `tally_session_free`. The caller copies it + immediately, which is what JNI was going to do anyway. `tally_result_free` disappears, + and with it every way to free the wrong thing at the wrong time. +- One session is used from one thread at a time. That is what a `ViewModel` does + naturally, and it keeps locks out of the library. Documented rather than enforced, + because enforcing it means a mutex on every call for a constraint the only caller + already satisfies. +- Out of memory is a status code on every export, never a crash. The engine already + threads `error.OutOfMemory` through every path, so this costs nothing to honour. + +#### Three lifetimes, three allocators + +The split is not a preference; it is what the engine's own ownership rules already +imply. `Environment.setVar` and `setAns` clone into the environment's own allocator +(`evaluator.zig`), while `evalStringInfo(env, allocator, source)` builds the parse tree +and the returned value from the allocator it is handed. So: + +| Lifetime | Holder | Allocator | +|---|---|---| +| Session: variables, `Ans`, the result buffer | `Environment`, `ArrayList(u8)` | the session's general-purpose allocator | +| One call: parse tree, intermediate rationals, rendered text | nothing; the arena owns it | `ArenaAllocator` over the session allocator, `reset(.retain_capacity)` at the end of each call | +| The bytes handed back | the session's result buffer | session allocator, `clearRetainingCapacity` per call | + +- **The session allocator is `std.heap.smp_allocator`** at the C boundary: thread-safe + by construction, aimed at release builds, and verified to compile for both Android + ABIs without libc. Not `page_allocator` directly - that rounds every allocation up to + a page, and a variables map plus big-integer limbs would burn one each. `page_allocator` + is simply where `smp_allocator` gets its memory. +- **The allocator is a parameter everywhere above the boundary.** The Zig-visible + session type takes an `Allocator`, so tests pass `std.testing.allocator` and get leak + detection; only the `export fn` picks a default. That is how the ABI gets leak-checked + without the library carrying a debug allocator into production. +- **`retain_capacity` is what makes the steady state cheap.** The first evaluation buys + the arena's pages and every later one reuses them, so a calculator being typed into + stops calling the backing allocator almost entirely. + +**One rule this imposes on the ABI code**, learned by writing the probe rather than by +thinking about it: values allocated from the arena must never be individually `deinit`'d. +The arena owns them and the reset frees them, and a `defer value.deinit()` in the same +scope as the reset runs *after* it - which segfaulted on the first attempt, freeing +big-integer limbs into memory the arena had already reclaimed. With an arena the teardown +is the reset, and nothing else. + +A probe that ran `x = 2^100`, then `x + 1`, then `Ans / 2`, resetting the arena between +each, produced exactly `633,825,300,114,114,700,748,351,602,688.5` and leaked nothing +under the testing allocator. That is the shape the real `tally_eval` should have. + +**What this trades away.** A borrowed pointer is a rule the caller has to follow: hold +it past the next call and it is stale. The alternative that removes the rule is the +caller-provided buffer, at the cost of a size-negotiation round trip. If the JNI layer +turns out to want a pointer it can keep, the caller-provided buffer is the fallback, and +the switch touches only the ABI: the session design above is unchanged either way. + +**What was verified, and what is still assumed.** The cross-compile has now been run. +Building the engine for `aarch64-linux-android` and `x86_64-linux-android` produces a +shared object per ABI with **no dynamic dependencies at all** (zero `DT_NEEDED` entries), +containing the full exact tier: a probe that evaluated `2^100 + 1` and rendered it +through `Number.render` compiled and linked on both, at 230KB under `ReleaseSmall`. No +NDK was involved, and the three-lifetime design above compiles for both ABIs too. + +Asking for `-lc` on those targets fails outright: Zig 0.16 bundles glibc and musl and no +Bionic, so it reports `unable to provide libc for target 'aarch64-linux...android.29'` +and suggests the gnu and musl triples instead. Anyone wanting libc there needs an NDK +sysroot. That is what removed `std.heap.c_allocator` from this design - it was the +recommendation here until the build was actually attempted - and the upside is a library +that needs no NDK at all. The cost is that allocations do not pass through `malloc`, so +Android's malloc-based tooling cannot attribute them; for a calculator whose steady state +is an arena reset per keystroke, that is a fair trade. + +Still assumed, and only testable with a device or emulator: that the library loads under +`System.loadLibrary`, that a JVM resolves the symbols, and that a page-backed allocator +behaves well under Android's memory pressure. + +### 6.4 How Kotlin reaches the C ABI + +A Kotlin `external fun` cannot call `tally_eval` directly; something has to translate +`jstring` and `JNIEnv*` into pointers and lengths (design 6.3). Three ways to get that +glue, in the order I would consider them: + +1. **`engine/src/jni.zig`, compiled only for Android.** The glue stays in one language, + is testable from Zig, and adds no dependency to the app. It needs the JNI type + definitions, which means either the NDK's `jni.h` at build time for that one file, or + hand-declaring the dozen function-table entries it uses. Hand-declaring removes the + NDK requirement entirely and puts a silent breakage risk in its place: a wrong vtable + index is a crash with no compiler to catch it. Prefer `jni.h`, and keep the C ABI + itself NDK-free so only this layer needs it. +2. **A small C shim compiled by Gradle** (`externalNativeBuild` with CMake). Standard + Android practice, and it moves native compilation into Gradle, which this design has + so far kept out. +3. **JNA**, which calls a plain C ABI with no per-project glue at all. Cheapest to start + and the only option that needs no NDK anywhere; it costs a dependency and per-call + marshalling overhead that a calculator will never notice. + +This does not need deciding until there is something to load. The C ABI is identical +under all three. + +### 6.3 Who the ABI is for, and what is actually Android-specific + +The shape in 6.2 was chosen from Android's access pattern, which invites a fair +question: should the exports be named for Android, or built differently per target? The +answer is no to the first and yes to the second, for reasons that are not symmetrical. + +**The C ABI is not Android-specific, and its names should not pretend to be.** Take the +decisions one at a time and ask which consumer they came from: + +- *A session handle* is what any caller wants. Variables and `Ans` outlive a single + evaluation for a C program, a Swift app or a WASM page exactly as they do for Kotlin. +- *Borrowed results* suit every managed binding, because they all copy on receipt: JNI + into a `ByteArray`, Swift into a `String` or `Data`, JS into a string. The rule only + chafes for a raw C caller that wants to accumulate results without copying, and + `sqlite3_column_text` has held that exact contract for two decades in the most widely + embedded C library there is. +- *Status codes instead of crashes on OOM* is table stakes anywhere. +- *One session per thread at a time* is what a `ViewModel` does naturally and what a C + caller can arrange trivially. + +Only two choices are platform-flavoured, and neither is visible in a signature: using +`c_allocator` for the session's long-lived allocations (internal), and preferring JSON +for compound results (a preference Kotlin, Swift and JS all share). + +So the exports keep names that say what they do: `tally_session_new`, `tally_eval`, +`tally_session_free`. Naming them `tally_android_eval` would encode the first consumer +into a contract that outlives it, and the second consumer would then either call +functions labelled for a platform it is not, or get a duplicate set to keep in step. +This project has already renamed one thing for that reason: `Coverage.addModule` became +`addReport` because the old name described the caller's mental model rather than the +thing itself. If a future consumer genuinely cannot live with a borrowed pointer, the +answer is one more function beside the existing ones (`tally_eval_into`, taking a +caller-provided buffer), not a rename and not a parallel set. + +**There is an Android-specific layer, and it is not the C ABI.** A Kotlin `external fun` +does not call `tally_eval`. JNI resolves either a symbol named +`Java___` or whatever `JNI_OnLoad` registers with +`RegisterNatives`, and either way the function's signature is in JNI's terms: a +`JNIEnv*`, a `jobject`, and `jstring` arguments that have to be pinned with +`GetStringUTFChars` and released again. That translation is genuinely Android's: + +- it names a Java package, so it cannot be shared with anything else, +- it needs `jni.h` types that do not exist on any other target, +- and it is thin: convert, call the C ABI, copy the result into a `jstring` or + `ByteArray`, return. + +That layer belongs in its own file (`engine/src/jni.zig`), compiled only for Android +behind a build option, with names that do say Android because JNI requires it. The C ABI +stays byte-identical on every target; Android gets an additional, thinner set of exports +on top of it. A Swift or C consumer links the same library and calls the C ABI directly, +with no shim at all, because Swift imports C headers natively. + +**The shim is also what makes borrowed results safe here.** The copy into JVM memory +happens inside `jni.zig`, in the same function, before control returns to Kotlin. The +app never holds a native pointer and cannot outlive one, so the lifetime rule in 6.2 is +enforced by code in this repository that can be tested, rather than by an app developer +remembering a sentence in a document. That is a better position than the caller-buffer +alternative would leave us in, and it is the argument that settled 6.2 for me. + +Two things to add while the boundary is being written, both cheap and both awkward to +retrofit: + +- **A hand-written `include/tally.h`.** Zig no longer emits reliable C headers, and the + header is what a Swift or C consumer actually compiles against. Writing it by hand is + fine; leaving it to drift is not, so a test should compile it against the real exports + (a `translate-c` of the header, or a C file that takes the address of every function, + built as part of `zig build test`). +- **`tally_abi_version()` beside `tally_version()`.** Android ships a prebuilt `.so` + that can fall out of step with the Kotlin calling it, and a struct layout or an + argument order that changed silently is a crash in the field rather than an error. The + product version and the ABI version are different facts and should be separately + readable. + --- ## 7. CLI Design ``` tally [OPTIONS] -tally struct [OPTIONS] -tally convert -tally amort [payment] [--monthly] [--summary] [--exact] +tally convert [to] +tally amort [payment] [--monthly] [--summary] [--exact] Options: -p, --programmer Programmer mode - -b, --bits Bit width: 8, 16, 32, 64 (default: 64) - --base Output base: dec, hex, oct, bin, all (default: all) - --json JSON output - --abi Struct ABI: sysv (default), packed - --endian Endianness: little (default), big + --bits Bit width: 8, 16, 32, 64, 128 (default 64). Implies -p + --signed / --unsigned How to read programmer values (default signed). Imply -p + --endian Byte order of the hex and ASCII rows (default big). Implies -p -h, --help Show help --version Show version + +Each subcommand answers -h with its own usage. ``` +The flags this section used to list (`-b`, `--base`, `--json`, `--abi`) were never +built. `--bits` is the spelling, there is no way to select a single output base (the +multi-base view appears when the expression used a non-decimal literal), and JSON +output belongs to the C API rather than the CLI. `tally struct` goes with section 3. + ### CLI Output Examples Standard: @@ -1233,7 +1505,7 @@ $ tally -p "0xFF & 0x0F" Separators are spaces, not underscores, and the base prefix appears only in the clipboard form. There is no `bits:` line. -Struct: +Struct (SKETCH, section 3 is not built): ``` $ tally struct "struct { u32 x; u8 flags; u16 id; u64 ts; }" ABI: x86-64 System V | Endian: little @@ -1272,6 +1544,12 @@ reports `error: unexpected token`. ## 8. TUI Layout +The mockups in this section are sketches of intent, not records of output. Where one +disagrees with the program, the program is right: the frames in `src/tui.zig`'s render +tests are drawn by the real draw path and asserted character by character, which is +where to look for what a view actually produces. Sketches that are known to have +drifted are marked. + ### 8.0.0 How the TUI is tested Terminal code is usually left uncovered on the grounds that it needs a terminal. @@ -1280,16 +1558,22 @@ Most of it does not. Two harnesses cover nearly all of it: - **Rendered frames.** `src/tui/test_render.zig` draws a real frame through the real widget draw path into a real surface, then reads the cells back as rows of text. Assertions are made against what a user would see (`"= PMT -1,199.10"`, - `"rows 1-7 of 360"`), plus `furniture(rows).intact()`, which checks that the - separator, prompt and status rows are still where they belong. That last check is - the load-bearing one: content which overflows its region lands on those rows, so - checking them is how overflow is detected. + `"rows 1-7 of 360"`), plus two structural checks bundled as + `test_render.expectSound(rows, full_width_rows)`: `furniture(rows).intact()`, which + checks that the separator, prompt and status rows are still where they belong, and + `noTextClipped`, which checks that no row other than the ones drawn full width on + purpose reaches the final column. The first catches content that overflowed its + region downward; the second catches content too wide for the terminal, which + `draw.writeStr` truncates in silence. This originally offered `wellFormed(rows, width)` instead, described here as a check that no column had been miscomputed. It compared each row's length against the width it had just been allocated with, so it could never fail. Two layout bugs (programmer rows drawn over the prompt at 100x16, financial chips drawn over - it at 20x5) lived under a green suite because of it. Style is still not + it at 20x5) lived under a green suite because of it. `noTextClipped` was written at + the same time and then left uncalled for as long, which hid two more: the 64-bit + binary row needed 90 columns and lost its low bits on an 80-column terminal, and + every mode's status line lost its last hint mid-word. Style is still not observable: `flatten` keeps graphemes and drops attributes, so focus highlighting and the cursor cannot be asserted, and tests that care assert a neighbouring glyph and say so. @@ -1371,50 +1655,67 @@ clicking places the cursor for typing instead. ### 8.1 Standard Mode ``` -┌─ Tally ─────────────────────── [Standard] [Programmer] [Financial] ─┐ -│ │ -│ History: │ -│ 2^10 = 1024 │ -│ sqrt(144) = 12 │ -│ Ans * 2 = 24 │ -│ │ -│ ───────────────────────────────────────────────────────────────── │ -│ > 3 * pi + 1_ │ -│ │ -│ = 10.42477796... │ -│ │ -└─ ?:help | Tab:mode | Enter:eval | Ctrl-L:clear | Ctrl-C:quit ───────────┘ ++- Tally ----------------- [Standard] [Programmer] [Financial] [Convert] -+ +| | +| History: | +| 2^10 = 1024 | +| sqrt(144) = 12 | +| Ans * 2 = 24 | +| | +| --------------------------------------------------------------- | +| > 3 * pi + 1_ | +| | +| = 10.42477796... | +| | ++- ?:help | Tab:mode | Enter:eval | Ctrl-L:clear | Ctrl-C:quit -----------+ ``` The footer is the real status line. The `[d]ec [h]ex [o]ct [b]in` quick-switch it used to show was FR-7.8, which has been removed: those letters are hex and binary digits once the value fields became editable. +Two details the sketch simplifies: there is no "History:" heading, and the answer is +not repeated under the prompt. A result appears in the history list as `= value` on the +line under its expression, which is where the sketch's `= 10.42...` line comes from. + ### 8.2 Programmer Mode + +A real 80-column frame, 64-bit, value 0xDEADBEEFCAFEF00D: + ``` -┌─ Tally ─────────────────────── [Standard] [Programmer] [Financial] [Convert] ─┐ -│ Bit Width: [64] Signed: [yes] Endian: [LE] │ -│ │ -│ ┌─ Bit Pattern (63 -> 0) ─────────────────────────────────────────────────────┐ │ -│ │ 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 │ │ -│ │ 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 1 1 1 1 1 1 │ │ -│ └─────────────────────────────────────────────────────────────────────────────┘ │ -│ cursor: bit 3 ▲ │ -│ │ -│ Signed: 255 │ -│ Unsigned: 255 │ -│ Hex: 0x0000_0000_0000_00FF │ -│ Oct: 0o377 │ -│ Bin: 0000 0000 0000 0000 0000 0000 1111 1111 │ -│ │ -│ > 0xFF & 0x0F_ │ -│ = 15 │ -│ │ -└──── arrows:nav Space:toggle w:width s:sign e:endian y:yank -- Tab:mode ----┘ +| Tally Standard Programmer Financial Convert | +| | +| Bits: 64 Signed: yes Endian: BE [float: Ctrl-F] | +| | +| 1101 1110 1010 1101 1011 1110 1110 1111 | +| 1100 1010 1111 1110 1111 0000 0000 1101 | +| | +| DEC(s): -2,401,053,089,206,439,923 | +| DEC(u): 16,045,690,984,503,111,693 | +| HEX: DE AD BE EF CA FE F0 0D | +| ASCII: . . . . . . . . | +| OCT: 1 572 555 756 771 277 570 015 | +| BIN: 1101 1110 1010 1101 1011 1110 1110 1111 | +| 1100 1010 1111 1110 1111 0000 0000 1101 | +| | +| (history) | +|--------------------------------------------------------------------------------| +| > | +| Tab:mode | Arrows:nav | Space:toggle | Up/Down:field | Ctrl-W:width | ``` -**NOTE: The wireframe above (8.2) is superseded by the editable-fields -design below. Keeping the old wireframe for reference during transition.** +The binary row wraps at 32 bits because 64 will not fit an 80-column terminal +(FR-7.3.2); a 90-column one shows it on one line. The status line drops whole hints +from the end rather than cutting one in half (FR-7.9.1), which is why `Ctrl-E:endian` +and `Ctrl-F:float` are absent here and the float affordance sits on the header row +instead. + +An earlier wireframe in this section showed a bordered "Bit Pattern (63 -> 0)" box, a +`cursor: bit 3` indicator, `0x`-prefixed hex, and a footer of single-letter keys +(`w:width s:sign e:endian y:yank`). None of that was built: there are no borders, the +prefix appears only in the clipboard form, the width and sign keys are Ctrl-chords +because plain letters are hex digits in an editable field (FR-7.8), and there is no +yank command yet (open item 14). #### 8.2.1 Programmer Mode - Editable Fields (REVISED DESIGN) @@ -1438,6 +1739,11 @@ Fields (navigable with Up/Down): 7. BIN - editable binary (nibble-separated: 1111 0000) 8. Expression - full expression evaluator (& | ^ ~ << >> etc.) +Field 8 was never built. `ProgField.expression` exists and Up/Down cycles through it, +but no row is drawn for it and no key does anything there, so it reads as a keystroke +that did nothing (open item 18). The expression language is reachable from the input +line instead, which is where it has always been. + Navigation: - Up/Down moves focus between fields - When a text field is focused, typing replaces its content; @@ -1517,7 +1823,7 @@ IEEE 754 float interpretation: float64 uses 64-bit view (auto-switches width when toggling float format) -### 8.3 Struct Visualizer (sub-view of Programmer Mode) +### 8.3 Struct Visualizer (sub-view of Programmer Mode) - DESIGN ONLY, NOT BUILT ``` ┌─ Tally ────────────────── [Programmer > Struct Layout] ─────────────┐ │ ABI: x86-64 SysV Endian: LE │ @@ -1727,7 +2033,18 @@ need. --- -## 9. Android UI Design +## 9. Android UI Design (DESIGN ONLY, NOT BUILT) + +Nothing in this section exists: there is no `android/` directory, no Gradle project, no +Kotlin, and the C ABI it depends on is two stubs (section 6). The screens below are the +plan. Read the struct layout screen (9.4) as doubly speculative, since the engine +feature behind it is also unbuilt. + +These designs are deliberately left unreconciled with what the TUI became during the +review, even where the two now disagree (9.5 has a Calculate button; the TUI dropped it +because results are live). Reconciling them on paper would be guessing twice: the useful +version of this section is the one written after there is a running app to react to. +Expect it to be rewritten from what the app teaches, not from what the TUI settled on. The Android app uses a fundamentally different interaction model from the TUI. Where the TUI is keyboard-driven with a prompt, Android is **touch-first with purpose-built input surfaces** for each mode. No text-cursor-in-a-prompt paradigm - instead, tappable buttons, interactive grids, and form fields. @@ -2025,10 +2342,10 @@ The JNI bridge sends expression strings down and receives JSON results back. Thi ## 10. Error Handling Strategy -Each module declares what it can fail with, and the sets compose: +Each file declares what it can fail with, and the sets compose: ```zig -// parser.zig +// Parser.zig pub const Error = error{ UnexpectedToken, UnmatchedParen, UnexpectedEnd, ExpressionTooLarge, NestingTooDeep, InvalidNumber, UnterminatedString, OutOfMemory, @@ -2043,8 +2360,8 @@ pub const Error = error{ // bitwise.zig pub const Error = error{DomainError}; -// units.zig: its own two, plus whatever the exact path raises -pub const Error = error{ UnknownUnit, IncompatibleUnits, OutOfMemory } || number.Error; +// units.zig: its own three, plus whatever the exact path raises +pub const Error = error{ UnknownUnit, AmbiguousUnit, IncompatibleUnits, OutOfMemory } || number.Error; // financial.zig pub const Error = error{ @@ -2053,8 +2370,10 @@ pub const Error = error{ }; // evaluator.zig: its own, plus every dependency's -pub const Error = error{ UnknownFunction, UnknownVariable, DomainError, Overflow } || - parser.Error || number.Error || bitwise.Error || financial.Error; +pub const Error = error{ + UnknownFunction, WrongArgumentCount, UnknownVariable, + AssignmentToConstant, DomainError, Overflow, +} || Parser.Error || number.Error || bitwise.Error || financial.Error; // engine.zig: what any entry point can return, for frontends to switch over pub const Error = evaluator.Error || programmer.Error || units.Error || financial.Error; @@ -2064,10 +2383,10 @@ This replaced one hand-written `CalcError` with 20 members that every engine fun claimed to return. That signature was false in both directions: `parser.parse` said it might return `ConvergenceFailure` and `UnknownUnit`, so no caller could switch on what a parse can actually produce, and four members (`InvalidType`, `InvalidFieldName`, -`DuplicateFieldName`, `StructTooLarge`) belonged to a struct layout module that does +`DuplicateFieldName`, `StructTooLarge`) belonged to a struct layout feature that does not exist, so nothing could return them while `errorPhrase` still gave them wording. -Error members unify by name in Zig, so the per-module sets compose with no +Error members unify by name in Zig, so the per-file sets compose with no coordination: `parser.Error.OutOfMemory` and `units.Error.OutOfMemory` are the same value. The unions are written with `||` rather than enumerated, so the compiler maintains them. @@ -2086,7 +2405,7 @@ deliberately rather than leaving a half-built shape in place. The wording lives in exactly one place, `engine.phrase`, in the engine rather than a frontend because every frontend needs the same words, including the Android app across -the C ABI. Its switch has no `else`, so an error added to any module's set fails to +the C ABI. Its switch has no `else`, so an error added to any of those sets fails to compile until it is given a phrase, and a comptime block checks the other direction, so it cannot carry wording for an error the engine is incapable of producing. Frontends decorate at comptime (`switch (err) { inline else => ... }`): the CLI adds `error: ` and @@ -2101,14 +2420,16 @@ carry its own copy of the whole table, and the TUI's had drifted three errors be | # | Decision | Rationale | |---|----------|-----------| | 1 | Pratt parser over recursive descent | Easier to extend precedence levels; cleaner for infix with mixed prefix operators | -| 2 | `u64` storage for all programmer integers | Simplest representation; bit_width used for masking/interpretation, not storage | -| 3 | JSON over C ABI boundary | Avoids complex struct marshaling; Kotlin/Swift handle JSON natively; small perf cost acceptable for calculator | -| 4 | Separate parser for struct DSL | Struct grammar is different enough from expressions; keeps both parsers simple | -| 5 | ABI as interface/vtable | Adding Windows x64 or ARM is implementing one struct; no changes to layout algorithm | +| 2 | `u128` storage for all programmer integers | One representation for every width; `width` masks and interprets rather than deciding storage, which is what makes the width a non-destructive display lens (8.2.1). This said `u64` until 128-bit support landed | +| 3 | JSON over C ABI boundary | Avoids complex struct marshaling; Kotlin/Swift handle JSON natively; small perf cost acceptable for calculator. Still the plan; not built (section 6) | +| 4 | Separate parser for struct DSL | Struct grammar is different enough from expressions; keeps both parsers simple. Not built (section 3) | +| 5 | ABI as interface/vtable | Adding Windows x64 or ARM is implementing one struct; no changes to layout algorithm. Not built (section 3) | | 6 | libvaxis for TUI | Most mature Zig TUI; supports Windows/Mac/Linux; active development; used by Ghostty | | 7 | `^` always means power; XOR is the `xor` keyword | Avoids mode-dependent operator overloading; every operator means the same thing in both modes; consistent with existing `rol`/`ror` keyword operators | | 8 | Statically-linked desktop binaries | Zero dependencies for end user; Zig makes this trivial | | 9 | Unit conversion via base-unit normalization | Simple to implement, easy to extend; only temperature needs special case (offset) | -| 10 | Android: button grids, not text prompts | Touch-first UX; no keyboard needed for basic use; purpose-built surfaces per mode beat a generic expression prompt | -| 11 | Android: form-based struct builder + raw toggle | Most users won't want to type DSL syntax on a phone; visual builder is more natural; power users can toggle to raw text | -| 12 | Android: live conversion without "Calculate" button | Conversions are instant; eliminating a tap makes the UX feel responsive and direct | +| 10 | Android: button grids, not text prompts | Touch-first UX; no keyboard needed for basic use; purpose-built surfaces per mode beat a generic expression prompt. Not built (section 9) | +| 11 | Android: form-based struct builder + raw toggle | Most users won't want to type DSL syntax on a phone; visual builder is more natural; power users can toggle to raw text. Not built | +| 12 | Android: live conversion without "Calculate" button | Conversions are instant; eliminating a tap makes the UX feel responsive and direct. Not built | +| 13 | Display budgets belong to the frontend, not the engine | A terminal, a clipboard and a phone screen want different digit counts, so `Number.render` takes `FormatOptions` and each frontend declares its own (NFR-9.9). The engine held these constants once and the CLI inherited a budget chosen for the TUI | +| 14 | One list per shared vocabulary, derived where possible | The financial function list, the general function list, the error wording and the mode tab order each exist once and are rendered by whoever needs them. Every one of these was a duplicated list first, and every duplicate had drifted by the time it was found | diff --git a/.kiro/specs/calculator/requirements.md b/.kiro/specs/calculator/requirements.md index 6f57c7d..60cef82 100644 --- a/.kiro/specs/calculator/requirements.md +++ b/.kiro/specs/calculator/requirements.md @@ -31,7 +31,7 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-2.12**: The `^` operator means exponentiation in all modes (never XOR). This avoids mode-dependent operator overloading. XOR is available only via the `xor` keyword. Power is also available via `**`. This keeps every operator's meaning identical across standard and programmer modes. - **FR-2.5**: Display both signed (two's complement) and unsigned interpretations of the current value. - **FR-2.6**: Visualize the bit pattern as a grid (integer.exposed style) - bits individually addressable/toggleable in TUI and Android. -- **FR-2.7**: Quick toggle between base display formats via dedicated keyboard shortcuts (TUI). +- **FR-2.7**: SUPERSEDED by FR-7.8's removal. Single-letter keys for switching the primary base cannot coexist with editable value fields, where those letters are hex and binary digits. Base selection is by arrow key and mouse, and all four bases are on screen at once anyway (FR-2.2), which is most of what the shortcut was for. - **FR-2.8**: Configurable endianness display (big-endian default, little-endian available). Affects byte order in HEX and ASCII rows and byte-level visualizations. Does not change the underlying value - purely a display/interpretation toggle. Big-endian is the default so the HEX row reads as the number itself, matching the DEC, OCT and BIN rows; the little-endian view (x86 memory order) is one keystroke away. (This requirement previously specified a little-endian default, which the implementation deliberately did not follow.) - **FR-2.9**: ASCII/text literal input via single-quoted strings: `'hello'` packs ASCII bytes into the integer value. First character occupies the most significant used byte (big-endian packing when display is BE) or least significant byte (when display is LE). Useful for examining magic numbers, file signatures, protocol headers. - **FR-2.10**: ASCII interpretation display - when the current value contains printable ASCII bytes, show the text representation alongside the numeric bases (e.g., `ASCII: "ELF."` or `ASCII: ..lf` with dots for non-printable bytes). @@ -46,7 +46,17 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - Allow entering a decimal float value and seeing the closest IEEE 754 representation - Show when a value is not exactly representable (rounding occurred) -### FR-3: Struct Layout Visualizer (Programmer Mode Extension) +### FR-3: Struct Layout Visualizer (Programmer Mode Extension) - NOT BUILT, WANTED, SEQUENCED AFTER ANDROID + +None of FR-3 exists: no DSL parser, no ABI profiles, no layout computation, no CLI +subcommand and no TUI sub-view. The four errors it would need were removed from the +engine because nothing could produce them, and design.md section 3 (the grammar and +algorithm) is marked DESIGN ONLY for the same reason. + +The feature stays in the specification: it is wanted, and it is the reason programmer +mode exists at all for some of its users. It is simply behind the Android app in +priority, so expect the DESIGN ONLY markers to stand for a while rather than to be a +sign of abandonment. - **FR-3.1**: Accept struct definitions in a mini-DSL: ``` @@ -111,12 +121,12 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-6.1**: Invoke as `tally ""` for standard mode evaluation. - **FR-6.2**: Flag `--programmer` or `-p` for programmer mode: `tally -p "0xFF & 0x0F"`. - **FR-6.3**: Flag `--bits <8|16|32|64|128>` to set bit width in programmer mode (default: 64). Accepted as `--bits 8` or `--bits=8`. It implies `-p`, because standard mode is fixed at 64-bit signed and the flag would mean nothing there. -- **FR-6.4**: Flag `--base ` to control primary output format (default: show all). -- **FR-6.5**: Subcommand `tally struct ` for struct layout - accepts inline DSL string or path to a file containing the definition. -- **FR-6.6**: Subcommand `tally cagr ` for quick CAGR. -- **FR-6.7**: Subcommand `tally tvm` with named flags (`--pv`, `--fv`, `--n`, `--rate`, `--pmt`) - omit one to solve for it. +- **FR-6.4**: REMOVED. Flag `--base ` to control primary output format. Programmer mode prints all four rows and standard mode adds them when the expression used a non-decimal literal (FR-1.9), so there is nothing for a selector to select; "primary" has no meaning in a view that shows every base at once. Anyone wanting one base can pipe through a line filter. +- **FR-6.5**: NOT IMPLEMENTED (FR-3 is unbuilt, and sequenced after the Android app). Subcommand `tally struct ` for struct layout - accepts inline DSL string or path to a file containing the definition. +- **FR-6.6**: REMOVED. Subcommand `tally cagr `. Superseded by FR-5.7: `tally 'cagr(1000, 2000, 5)'` is the same calculation and composes with arithmetic, which a subcommand cannot. A second spelling would need its own argument checking and its own help, both of which would drift from the function's. +- **FR-6.7**: REMOVED. Subcommand `tally tvm` with named flags. Superseded by FR-5.7's `tvm_pmt`, `tvm_fv`, `tvm_pv`, `tvm_n` and `tvm_rate`: naming the variable in the function is clearer than omitting one of five flags, and it needs no rule about which omission means "solve". - **FR-6.8**: Subcommand `tally convert ` for unit conversion. -- **FR-6.9**: Output format flags: `--json` for machine-readable output, plain text default. +- **FR-6.9**: REMOVED as specified here. Machine-readable output is still wanted, but not as a CLI flag invented separately from the C ABI's encoding (design 6): two JSON shapes for the same results is the duplication this project keeps finding. When the ABI settles its encoding, the CLI can render the same one, and that will be a new requirement rather than this one. - **FR-6.10**: Exit code 0 on success, non-zero on parse/evaluation errors with stderr message. - **FR-6.11**: Subcommand `tally amort [payment]` prints an amortization schedule as a table. Flags: `--monthly` (the rate given is an annual nominal rate charged over monthly periods, so rate/12 applies per period), `--summary` (totals only, no per-period rows), `--exact` (skip cent rounding). A table is the one financial output that cannot be an expression function, which is why it is a subcommand. - **FR-6.12**: Flags `--signed` (default) and `--unsigned` set how programmer-mode values are interpreted, which decides both the displayed rows and whether `>>` extends the sign. Both imply `-p`. @@ -172,7 +182,12 @@ still required (FR-7.7): the mouse never becomes the only way to do something. - **FR-7.11.12**: Clickable regions are rebuilt every frame from what was actually drawn, so hit targets can never drift out of sync with the display. - **FR-7.11.13**: In financial mode, clicking a calculation chip selects that calculation and clicking a field row focuses it for editing; the mouse wheel scrolls the amortization schedule. In the help overlay the wheel scrolls and a click returns. -### FR-8: Android Frontend +### FR-8: Android Frontend - NOT BUILT + +Nothing in FR-8 exists yet: no `android/` project, no Kotlin, and the C ABI it calls +through is two stub exports (design 6). Design 9 holds the screen designs, and design +6.1 holds the three decisions the ABI has to make before any of this can start. The +requirements below are unchanged; they are the target, not a description. - **FR-8.1**: Native Android app using Kotlin and Jetpack Compose. - **FR-8.2**: Call into the Zig engine via JNI (C ABI shared library). @@ -200,7 +215,7 @@ still required (FR-7.7): the mouse never becomes the only way to do something. ### NFR-2: Portability - Engine, CLI, and TUI must compile and run on: Linux (x86-64, aarch64), macOS (aarch64, x86-64), Windows (x86-64). -- Android engine .so must target: arm64-v8a, armeabi-v7a, x86_64. +- Android engine .so must target: arm64-v8a, armeabi-v7a, x86_64. NOT WIRED UP: `build.zig` produces host static and shared libraries only, and no Android target has been added or tested. - Single `zig build` invocation to produce all desktop targets (cross-compilation). ### NFR-3: Correctness @@ -227,7 +242,7 @@ still required (FR-7.7): the mouse never becomes the only way to do something. ### NFR-6: Build & Distribution -- **Zig build system for everything.** A single `zig build` invocation at the workspace root handles all targets: engine, CLI, TUI, and Android .so cross-compilation. No Gradle required for the native library build - Gradle only wraps the prebuilt .so into the APK. +- **Zig build system for everything.** A single `zig build` invocation at the workspace root handles all targets: engine, CLI, TUI, and Android .so cross-compilation. No Gradle required for the native library build - Gradle only wraps the prebuilt .so into the APK. (Today: engine libraries, the `tally` binary, `test` and `coverage`. The Android half is not wired up.) - **Mise for toolchain management.** All required toolchains (Zig version, Android NDK if needed for validation) are declared in `.mise.toml` at the project root. A developer needs only `mise` installed - running `mise install` provisions everything else. - **No other system-level dependencies.** Outside of mise, a developer should not need to manually install Zig, Android SDK/NDK, or any other toolchain. The `.mise.toml` is the single source of truth for tool versions. - **libvaxis for TUI.** The TUI frontend uses libvaxis (pulled as a Zig build dependency). No other TUI framework. diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index cf7dd18..6a25e51 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -21,7 +21,8 @@ NOTE: Actual structure diverged from spec - single binary at `src/main.zig` (CLI + TUI combined), engine as static lib + shared lib. No separate cli/ or -tui/ build files. kcov-based coverage wired in via `build/Coverage.zig`. +tui/ build files. kcov-based coverage wired in via `build/Coverage.zig`. The Android +cross-compilation targets in the list above were never added; they are Task 6.1's now. ### Task 1.2: Implement core types module [DONE, later dismantled] - Create `engine/src/types.zig` @@ -1690,6 +1691,14 @@ the review would have spent its time on): could never run, because only formatter `raw` strings carry a prefix. STILL OPEN, in the order I would take them: + +As of the end of the file-by-file review: 18 items recorded, 5 struck as fixed, 13 +live. Four of the live ones are engine correctness (1 through 5 below, minus the struck +one), four are display or interaction defects in the TUI, and three are the C ABI's +unfinished business (12, 15, and the session-handle question in design 6.1), which is +what the Android app will meet first. Item 18 is a list of its own: the findings from +`src/tui.zig`, recorded rather than fixed at the point the review reached that file. + 1. ~~`>>` is logical in standard mode and arithmetic in programmer mode, shift amounts wrap in one and clamp in the other, and `evaluator.zig` ignores the configured bit width entirely.~~ FIXED. `engine/src/bitwise.zig` is the one @@ -1877,21 +1886,64 @@ STILL OPEN, in the order I would take them: ## Phase 6: Android App +### Task 6.0: Finish the C ABI (PREREQUISITE, not started) + +Nothing in Phase 6 can be verified until the library answers. `c_api.zig` is the root of +`libtally.so` and has three exports: `tally_version` works, `tally_eval` and +`tally_result_free` have `// TODO: implement` bodies, and no test builds the library, so +it appears in no coverage report (open items 12 and 15). + +- Toolchain first, and all of it through mise: pin the JDK, Gradle and the Android SDK + (plus the NDK if design 6.4 picks the `jni.zig` route) in `.mise.toml`, and check each + one resolves before anything depends on it. Nothing gets installed system-wide. + +- Decide the four open questions in design 6.1, in this order: session lifetime + (an opaque handle, or a calculator with no variables and no `Ans`), memory ownership + (design 6.2 recommends a session-owned buffer with borrowed results and no + `tally_result_free`), who supplies `FormatOptions`, and what the JSON carries. +- Confirm the ground first: DONE for the compile half. Both Android ABIs build a + dependency-free ~230KB shared object containing the whole engine, with no NDK + (design 6.2). What remains is loading it: `System.loadLibrary`, symbol resolution + from a JVM, and the Kotlin binding choice in design 6.4. +- Implement `tally_eval` and `tally_result_free` against those decisions, including + which allocator the library owns and how a caller frees what it is handed. +- Add the units and financial enumerations Android needs for its pickers, so the + category list, the unit list per category and the form field specs come from the + engine rather than being retyped in Kotlin. +- Write `engine/src/jni.zig`: the `Java_*` entry points, compiled only for Android + behind a build option (design 6.3). Keep it thin - convert, call the C ABI, copy the + result into JVM memory - because that copy is what makes the borrowed-result rule in + 6.2 safe without asking an app developer to remember it. +- Hand-write `include/tally.h` and have a test compile against the real exports, and add + `tally_abi_version()` beside `tally_version()` (design 6.3). +- Test the ABI from Zig, calling the exports the way JNI will: a test target that + builds the shared library and exercises each export, including the free path under + a failing allocator. +- Verify: `zig build test` covers `c_api.zig`, it appears in a coverage report, and a + round trip (`x = 5`, then `x * 2`) works across the boundary. + ### Task 6.1: Set up Android project structure - Create `android/` directory with Gradle project (Kotlin DSL) - Configure for Compose, minimum SDK 26 (Android 8.0), Material 3 - Create JNI bridge class `TallyEngine.kt` with `external fun` declarations - Set up `jniLibs/` directory structure for arm64-v8a, x86_64 +- Add the Android targets to `build.zig` (`-Dtarget=aarch64-linux-android`, + `-Dtarget=x86_64-linux-android`); Task 1.1 listed this and it was never done - Native .so files produced by `zig build -Dtarget=aarch64-linux-android` (etc.) - Gradle does not invoke Zig, it consumes prebuilt artifacts - Set up bottom navigation bar (Standard, Programmer, Financial, Convert) - Verify: `zig build -Dtarget=aarch64-linux-android` produces libtally.so; Gradle project builds and empty app launches with nav bar ### Task 6.2: Implement JNI bridge -- Kotlin side: `TallyEngine.evaluate(expr, mode, config): String` (returns JSON) -- Kotlin side: `TallyEngine.structLayout(definition, abi, endian): String` +- Kotlin side: `TallyEngine.evaluate(expr, mode, config): String` (returns JSON), against + whatever session shape Task 6.0 settled on - Kotlin side: `TallyEngine.convert(value, fromUnit, toUnit): String` -- JSON deserialization into Kotlin data classes (Value, MultiBaseResult, StructLayoutResult, ConvertResult, etc.) -- Error handling: parse error JSON, surface to UI +- JSON deserialization into Kotlin data classes. Note that `MultiBaseResult` and + `StructLayoutResult`, named here originally, do not exist in the engine: the first + was deliberately dropped (design 2.2) because a result does not carry five + renderings of itself, and the second belongs to unbuilt FR-3. The shapes to + deserialize are whatever Task 6.0's JSON defines. +- Error handling: parse error JSON, surface to UI. The words come from + `engine.phrase`, so Kotlin should not add its own table - Verify: unit tests calling native functions with known inputs, validating deserialized results ### Task 6.3: Implement standard calculator screen @@ -1912,7 +1964,7 @@ STILL OPEN, in the order I would take them: - FAB/toolbar button navigating to struct layout sub-screen - Verify: bit toggling works, base displays update, expressions evaluate -### Task 6.5: Implement struct visualizer screen +### Task 6.5: Implement struct visualizer screen (BLOCKED: FR-3 is unbuilt) - Navigation: accessed from programmer mode via toolbar button - Form-based struct builder: type dropdown + name field per row, add/remove/reorder fields - Toggle for raw DSL text input (for power users / copy-paste from code)