8.3 KiB
S2c typed writer independent source review
Frozen review pin: f1d79df4979121335ed7b4ce4da84b2fca8a9ef4, direct parent bed5b0e8320bfc106b1f5f62e42339ca23b900cd, in .local/as2-20260913. The worktree was clean at readback. Reviewed the full six-file change, accepted writer plan, author's handoff, actual staged parser/Budget and relevant model/admission contracts. No compiler, Rust tests, formatter, GUI, Gitea or source mutation occurred. Read-only Git object inspection verified the pinned changes and oracle move. Only this report was written.
No demonstrated actionable source defect within the private S2c contract. This disposition does not establish compilation, runtime success, production/public source8 acceptance or complete save/export correctness. All ten added tests remain unrun in this review.
Topology and reference review
write/expression.rs:14-48 matches the actual Pratt parser's precedence table and left associativity. Lower-precedence binary children and equal-precedence right children are grouped; both comparison-child sides are grouped regardless of equality/ordering precedence. Constructors/list/call arguments use root expression context. Parent::Negate groups binary operands only. Canonical negative Exact nodes use ratio syntax, so - 1, ratio(-1, 1) and - ratio(-1, 1) remain distinct trees. The task scheduling reverses complete child/text sequences onto a LIFO stack, preserving left-to-right emission and constructor/list argument order.
All 13 Expr variants are explicit in the iterative match. Power writes the stored signed exponent with its -16..16 check; it does not turn the exponent into another Expr or evaluate/fold anything. Quantity resolves Unit rather than QuantityKind. Property, definition and parameter domains use separate lookups. Checked definition lookup precedes the call's later indexing, and the immutable Project borrow prevents a intervening mutation.
write.rs:294-350 derives argument-value scope from the actual owning binding field. The call path at write/expression.rs:190-237 validates exactly the called calculation's parameter-ID set and emits argument labels in its parameter Vec order. Child expressions retain the caller's calculation scope; callee names do not redirect caller $parameter resolution. Constructor scope tables are per calculation; duplicate IDs/names within a signature fail, while reused IDs across signatures or with a global UUID remain distinct. No call-graph expansion or global parameter-name map is introduced.
write.rs:427-625 emits each typed body, explicit UUID/name/description, required none/unbound/empty forms, Vec parameter order and BTreeMap override order. Short OA/SA words and root-only analysis parent handling are explicit. All catalog definitions are emitted, including unused or unbound calculations. Stored aliases are checked, quoted text streams through serde_json and exact numbers use the existing bounded canonical literal implementation.
Limits and error boundary
The sink checks cumulative UTF-8 bytes before extension. Quoted-string encoding streams into that same sink rather than materializing an unbounded escaped intermediate. Name size is checked before lexical validation at write.rs:165; Exact's temporary is bounded by existing numeric components. run at line179 preserves aggregate counters across successful fragments, discards an incomplete fragment on error and latches that error. typed_declarations returns no partial vector on an error.
Scope-table construction charges definitions/parameters before insertion. Expression tasks charge structural visits; stack extension has a pending-work bound. List width1024, expression depth128 and power bounds fail explicitly. The limits are private typed-output/structural quotas, not a promise that every semantically accepted Project fits every lowered caller quota. Semantic admission remains a caller precondition; the writer does not duplicate global identity/type/layer validation. In particular, value_type intentionally uses owner as error context, while binding/expression methods validate owner/field membership. The typed-declaration convenience still scans the complete element map to find analysis entries; its output quota is not a production-wide traversal/export bound. S4 must retain the separately stated complete Project/envelope/include/resource contract.
Evaluator adequacy and limits
The ten source-authored test functions are meaningful for this checkpoint:
- OA and SA canonical tests start from the independently built complete Project oracle and separately authored ordinary source skeletons. Actual staged parse/lower output must equal the complete original Project, followed by semantic admission and stable re-emission. Expected typed bodies do not come from the writer. Public8 rejection remains explicit.
- All-13 tests use independently constructed Expr trees and exact source goldens, then compare actual parsed/lowered topology. The complete candidate is admitted for the semantically valid 13-variant cases; the corrected AsKind example is retained. Additional precedence cases are structural controls and do not claim semantic validity for the temporary probe calculation.
- Call tests distinguish callee order from argument-map order, caller versus callee parameter names and reused scoped/global UUIDs. Parameter-item checks and a complete model roundtrip preserve identities. These cover the key alias/scope confusion risk.
- Subtree tests exercise explicit parent contexts, retaining Unary topology after replacing the entire grouped operand of
-(1)with binary, negative-literal or unary values, and preserving a right subtraction child. This is a private fragment contract. They do not exercise S3's actual expression-site selection, source edits, comment ownership or full transaction path. - The exact-byte/one-byte-short controls include escaped Unicode and aggregate output across calls; low entry limits and latched errors are exercised. Wrong domains, missing/extra parameter IDs, invalid scope and nearby width/depth bounds are explicit negative controls. Full performance/memory behavior remains unmeasured.
- The previous Negate frame counterexample is retained as a complete case: 127 Unary nodes around a negative Exact leaf, independent tree count, actual staged parser frame counts130/129 and obsolete grouped case/calculation controls257/256. The intended failure records the hard maximum reached at256; it does not falsely assert a successful257-frame parse. Canonical case output must roundtrip to the complete model and remain stable. This directly addresses the earlier design finding rather than relying on a calculation-only witness.
The test suite samples consequential precedence combinations rather than exhaustively enumerating every binary-operator pair. Source comparison found no missing/mismapped operator. No expanded test framework or unrelated production changes are required by this review. Parent-owned executable gates must still check the new ten tests alongside retained lower/declaration/parser/raw-token tests and strict compilation. Previously failed full-library public-header tests must remain reported as failures until separately repaired; S2c does not fix or waive them.
Mechanical move and scope evidence
The old lower/oracle.rs and new model_oracle.rs are exactly 8451 identical bytes, Git blob 7acca881a5f5b1243a8df760b230f57619212eef, SHA256 fa8bcf0a327debb219b145939601da4636cf85ddf7744a4e34c359542dfe6fb1. Lower-test changes only replace its local module declaration with the shared private oracle import. analysis_source.rs only registers the private writer and cfg(test) oracle. No public parser/version dispatch, model, admission, source_project, migration, grammar or ordinary formatter changes are in this pin.
Reviewed writer file identities:
| File under analysis_source | Bytes | SHA256 |
|---|---|---|
| write.rs | 24143 | a49b893225bc03b4ed9324a00ddd0179c73b6b06aed2dbabdd2afa851fb439fe |
| write/expression.rs | 12138 | 8b84f5255ccd51aa20b0f01b4f8d4b3a0b67fe2afe0ffdd4752e38f3253bd446 |
| write/tests.rs | 30351 | 50364af0c82a7430a45edd8ec18ff9c5f1248c656a2234608b7a180bf2e39739 |
Plan SHA256 e6ef6108792896cfb65e825cd19e984bf1c1729cff242653ed7947b4c6e9a803; author handoff SHA256 62c7e60b10b503afc97529707335d47eaebde36cfd6fd3b3bf01a0d7800e2073. These are design/source records, not executed writer results.