7.1 KiB
Project-first semantic slice: bounded independent critique
This review preceded synthesis of the accepted contract. Its two corrections are incorporated there. Original draft names below identify review inputs, not competing current specifications.
2026-09-23. Read-only review of analysis-mixed-semantic-slice-choice-20260923.md, revision 2 boundary, and actual a578ac98a06772a60bec00ae287e65650680d119 code. No implementation, compiler, GUI or publication. Syntax runtime acceptance remains an implementation prerequisite.
Project-first with retained syntax/sites is coherent; I found no necessary throwaway model or second lowering pass. Accept the scope with the two acceptance corrections below.
1. Specify and discriminate the retained-site handoff now
lower.rs:90–119 stores expression parent links and occurrence.expression as indices into one Lowered.expressions arena. lower:269 creates one Lowered output shared by all file/declaration resolvers. index.rs:185 needs typed declarations for header locations, original tokens/source for checked spelling/spans, names and scoped parameters from model records, and these two site vectors. Its later check_syntax_edges walks the retained typed AST and compares structural edges; it does not need to lower again. Moving the typed maps into the complete Project loses no required semantic data, provided the later name builder visits that Project once instead of also visiting the old Lowered maps.
The proposal should make the following handoff invariant explicit: one unchanged global expression arena, its paired occurrence vector, and the same canonical numeric-file-key table move into the sealed result together. Do not independently sort/compact either vector, concatenate separately numbered per-file arenas, or renumber files without an explicit complete remap of every affected link. Site fields are not self-contained after those transformations. This is a consequence of retaining sites across a checkpoint rather than consuming Lowered immediately.
Strengthen fixture group 5 from “inspect retained sites” to a hand-authored full retained-site oracle. Use repeated calls with reversed argument-label order, two calculations reusing a parameter UUID/name, nested child expressions, and coincident local spans in distinct files. Independently enumerate (file, owner, field, parent-edge/path, role, target, original span) for every retained site; check parameter targets remain qualified by callee/calculation and every occurrence.expression resolves to the intended arena entry. Include one ordinary→typed and typed→ordinary reference in the complete Project comparison. Keep numeric arena positions private; the oracle can resolve them to authored structural identities without invoking Index or lowering.
Project equality and unchanged parser/Exact usage alone cannot reveal missing or mislinked sites, because model semantics have already been constructed. Conversely, an intact sealed arena plus retained AST/tokens is enough for the later builder to run the existing independent token/path/edge checks. Index's physical uniqueness/call-site checks are still deferred; do not present this handoff test as completed Index verification.
2. Resolve the diagnostic producer's accounting boundary
The chosen result promises warnings plus successful static limits/usage while retaining syntax and lower sites. Existing APIs do not directly return all of that: validation.rs:182 validate invokes static admission then returns architectural issues while dropping usage; analysis/admission.rs:59 admit retains the environment/usage but consumes architectural issues to select an error and discards warnings. Calling both repeats the static route.
Use the proposed narrow shared internal route to run static admission once, extract owned limits/usage, drop the self-borrowing environment, run the ordinary architecture validator once, and preserve its complete warning/error result for source mapping. Existing wrappers must keep their behavior. This is a required implementation boundary, not permission for a second validator or a new semantic decoder.
There is also a concrete producer missing from the preallocation wording: validation::validate_structure creates its Vec<Issue> and issue-owned strings internally before returning; source_project::model_diagnostics:466–515 currently receives that allocated vector and maps it into another allocated diagnostic collection. Charging only SourceDiagnostic insertion cannot retroactively admit the raw issue vector. Because Project-first keeps syntax/sites alive through final admission, these containers can coexist with the retained reservations.
The parent's selected correction is compatible with this private scope: classify existing validator issue production with the already excluded architectural allocations, then reserve the additional mapped/retained source-diagnostic storage before allocating or copying it. Preserve complete issues and static usage in a narrow shared internal result. No concrete stricter requirement in these proposals or the user's full-tool objective requires routing a budget through every architectural validator in this slice. The existing static meter explicitly excludes that allocation, and the auxiliary ledger was already narrower than all model/serde storage.
Narrow the claim “diagnostic containers remain auxiliary storage” accordingly: the produced raw Issue vector is outside preallocation accounting, while source mappings, retained diagnostic containers and newly copied diagnostic text are inside. Move issue-owned strings where practical; a string transferred into retained diagnostic ownership must acquire the appropriate retained reservation, and a copied string needs an additional reservation before the copy. Count simultaneous in-scope containers and retained syntax/sites. An optional check after raw issue production is only a post-production bound; it cannot be described as preallocation admission. Retain neither an uncharged raw issue collection in the accepted witness nor a self-borrowing analysis environment.
Acceptance should include one valid warning-only model whose exact warning survives, one static-analysis failure and one architecture failure with no witness, plus a tight auxiliary limit at diagnostic mapping while retained syntax/sites are still live. Assert one static admission route through an internal test seam/counter or source inspection; unchanged output alone would not detect duplicate admission. Keep static events distinct from the auxiliary ledger and architectural/wire allocation, as already specified.
No additional scope expansion is indicated. Borrowed declaration views, complete catalog before ordinary construction, disjoint moves into the complete Project, scoped parameter identities, exact retained caller policy, and the active-ordinary-reference hook are compatible with the split. Index/updater changes can remain a named later dependency. Acceptance must continue to describe this result as an admitted private Project with retained provenance, not an occurrence index, mixed editing, or user-facing parity.