human review: engine and c_api

.kiro/specs/calculator/design.md

@@ -75,7 +75,7 @@ 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 module re-exports, the `Error` union, and `phrase` |
+| `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 |
 
 Every module declares its own error set; there is no shared one (section 10).

.kiro/specs/calculator/tasks.md

@@ -914,8 +914,35 @@ plans a `--raw` flag, but today it is exercised only by tests.
   100% line coverage, engine 99.44%. CLI output byte-identical across all five rows,
   both byte orders, ASCII packing and the multi-base standard-mode view.
 
+### Task 5.24: The engine's surface is what someone imports
+
+`engine.zig` re-exported thirteen modules; six had no importer outside the engine.
+`Rational`, `tokenizer`, `ast`, `bitwise` and `Parser` had none at all, and `number`
+was reached only through the `Number` alias. A frontend evaluates with `evalString`
+rather than building a tree, and computes on `Number` rather than on the rational tier
+underneath it, so none of those were ever going to be asked for.
+
+What kept them compiling is `test { testing.refAllDecls(@This()); }`, which analyses
+every declaration in the file. That is a useful idiom and it is also why nothing
+flagged six dead exports: they were referenced, just not by anyone outside.
+
+`number` and `Parser` stay as private imports, the first for the `Number` alias and
+the second for the test that walks `Parser.Error`. The rest are gone from the surface
+and still reachable by path from inside the engine. Test count is unchanged at 929,
+which confirms the re-exports were not what pulled those files' tests into the binary:
+each is imported by a module that remains, so it is part of the compilation either
+way.
+
+`c_api.zig` was reviewed in the same pass and left alone. Its three exports
+(`tally_eval`, `tally_result_free`, `tally_version`) are a reasonable shape on the
+surface, but it has no tests, appears in no coverage report because no test target
+builds the shared library, and predates Task 5.17's removal of engine-side display
+defaults. It will be rewritten against a real caller when the Android app lands; open
+items 12 and 15 record the test gap and the `FormatOptions` question.
+
 ### Task 5.23: One answer per loan, and a principal column in whole cents
 
+
 Open item 7 named two amortization defects. Both are real, and neither is the
 mechanism the item blamed.
 

engine/src/engine.zig

@@ -5,31 +5,32 @@
 
 const std = @import("std");
 
-// Vocabulary, lowest first.
+// The engine's surface, lowest first: a caller writes `engine.units.convert` or
+// `engine.financial.solveTvm`. Every module here has an importer outside the engine.
 pub const grouping = @import("grouping.zig");
 pub const Integer = @import("Integer.zig");
-// Exact numeric model (design.md 2.7). The evaluator computes in these.
-pub const Rational = @import("Rational.zig");
-pub const number = @import("number.zig");
-// Language layer.
-pub const tokenizer = @import("tokenizer.zig");
-pub const ast = @import("ast.zig");
-pub const Parser = @import("Parser.zig");
 pub const evaluator = @import("evaluator.zig");
-// Fixed-width integer operations, shared by both modes.
-pub const bitwise = @import("bitwise.zig");
 pub const programmer = @import("programmer.zig");
-// Domains and display.
 pub const float_interp = @import("float_interp.zig");
 pub const units = @import("units.zig");
 pub const financial = @import("financial.zig");
 
-// The modules above are the engine's surface: a caller writes `engine.units.convert`
-// or `engine.financial.solveTvm`. The aliases below exist only for the handful of
-// names used often enough that the module prefix is noise. There used to be a
-// curated re-export of nearly every public declaration, which drifted: two thirds
-// of it had no callers, and `Value` was re-exported after the type it named had
-// stopped being the engine's result type.
+// Reached only from inside this file. `Rational`, `tokenizer`, `ast`, `bitwise` and
+// `Parser` were public too and no frontend imported any of them: a caller evaluates
+// with `evalString` rather than building a tree, and computes on `Number` rather than
+// on the rational tier underneath it. `refAllDecls` in the tests below analyses every
+// declaration, which is what kept them compiling and hid that nothing wanted them.
+//
+// They are still reachable by path (`@import("Rational.zig")`) for anything inside
+// the engine, and their tests still run: every one of them is imported by a module
+// above, so it is part of the compilation either way.
+const number = @import("number.zig");
+const Parser = @import("Parser.zig");
+
+// The aliases below exist only for the handful of names used often enough that the
+// module prefix is noise. There used to be a curated re-export of nearly every public
+// declaration, which drifted: two thirds of it had no callers, and `Value` was
+// re-exported after the type it named had stopped being the engine's result type.
 pub const BitWidth = Integer.BitWidth;
 pub const Environment = evaluator.Environment;
 pub const evalString = evaluator.evalString;