Files
ARCADIA-Studio/harness/experiments/analysis-carried-phase-20260925/SOURCE-REVIEW.md
T

58 lines
11 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Independent carried-phase production source review
2026-09-25. **No concrete correctness blocker found; compilation and runtime acceptance pending.** Reviewed uncommitted production changes over `18073a6f2eabce83fc83997f094e331eec835a16` in `.local/as3batch`. Tests were still being authored and are outside this source-review verdict. No source edits, Cargo, application execution or network actions were performed.
Authority: selected `harness/analysis-carried-phase-contract-20260925.md`, SHA256 `d17c28a8bb138063c8c5fbf3ed5850f3c1531047d217dffa7274e8d6339fb6b9`; independent `CARRIED-PHASE-ORACLE.md`, SHA256 `56d1b57f4322c74bfd52f46483e337213fb6f8edf26d5756228b6e34c83692fd`. Paths below are under `crates/oa-core/src/language/analysis_source/`.
| Reviewed production file | Exact working-file SHA256 |
| --- | --- |
| `mixed/carried.rs` | `b376844c42660fec462def3e839ed99db272e5752ba137c885e69cad01c29e6c` |
| `mixed.rs` | `78422c978f9eaf490145ec13bf68928a87966e83370fbf7e5076282e1d578b8b` |
| `budget.rs` | `24656fe30fe17b3d7ae295b2e1eeb0a5e5e7bc4fbc4f30cdd95e3e72e1b54f66` |
| `mixed/project.rs` | `951dfceb111faa10bf795ec73646b794b08cca81e450b02f13a987a49d9f36ee` |
## Correctness and authority findings
- **Bound owner and lifetime:** `carried.rs:6–43` keeps policy/owner fields private; CarriedInput is neither Clone nor Copy. Retained backing borrows the actual allocation-owning RetainedSnapshot. The bundle method derives the reference from that backing. `parse_carried` takes only the input, derives bundle and exact policy, and moves the guard into the parser (`:65–69`). Syntax retains the optional guard (`mixed.rs:46–50,401–408`); existing consumed resolve/index ownership retains syntax. There is no returned self-referential owning bundle or exposed guard-removal method. Rust lifetime/type checking still needs compilation.
- **Construction is fixture-only:** policy capture, retained fixture construction/input and borrowed input construction are all cfg(test) (`carried.rs:71–119`). The retained fixture counts exactly F entries, zero lower occurrences and the actual entry/key/source UTF-8 bytes with checked additions. It does not claim preallocation admission of its fixture allocation. No production nonzero or zero-carry transition is enabled; no emitter authority is implied.
- **Internal parser seam:** `parse_impl` accepts separate arguments and could be misused by future internal code, but its current two callers are fully accounted for: legacy passes None; private parse_carried derives all values and Some(input) from the same guard. No intended API currently allows substitution. This is a sufficient current private caller invariant, not a concrete vulnerability requiring an input-enum redesign. Re-review any future caller or constructor against this invariant.
- **Exact policy and independent work:** `Budget::limits` (`budget.rs:67–70`) returns the immutable complete Limits value. Policy capture retains complete ProjectLimits too. The shared parser constructs one new Budget using those values and preserves the previous maximum-policy validation. There is no policy subtraction, default reconstruction, per-include reset, or modification of an existing Budget. Carried slots never enter Usage or numeric work counters.
- **Explicit mode preserves legacy checks:** `parse_with_profile` delegates with None. The initial and per-file branches preserve its old independent metadata checks; it gets no new completion check. Some(input) always performs combined checks, including borrowed zero carry (`mixed.rs:282–301,348–358,396–400`). Grammar, scanning, ordinary projection and traversal remain shared and unchanged. Thus the oracle's legacy E cap11 versus carried-zero cap11 distinction is represented directly rather than hidden behind `C == 0`.
- **Checked, noncharging aggregation:** `CarriedInput::check` composes C+U+M with existing checked Slots::add and validates all three dimensions through a temporary zero Ledger (`carried.rs:45–61`). The temporary Ledger retains no reservation in the returned witness. All errors propagate immediately through the existing safe diagnostic path. Continuation C is checked before complete-source precheck/traversal; initial C+file metadata is checked before Traversal::new; per-file prospective metadata is checked before position projection; final check follows successful traversal. The old token/value-end/position/owner metadata components occur once.
- **Once-only resolver transfer:** `project.rs:438–449` seeds live Slots from the retained guard, or zero for legacy, then calls the unchanged syntax admission once. The existing syntax walk adds U+M, owned text and bounded scratch. Existing catalog/lower releases subtract their recorded local reservations, not C. There is no new release/reset path for the baseline. Resolved retention should therefore equal the zero-carry result plus C exactly; actual cap boundaries still require runtime verification.
- **Failure attribution and ownership:** parser failures use the entry or current-file closure at the appropriate checkpoint; resolver errors retain existing canonical file/span transport. The guard and partial syntax are consumed/dropped on refusal, while the external fixture owner remains alive under its own scope. No partial witness, reusable continuation or accepted edited output escapes. Default parser span transport remains distinct from resolver full-span transport as the oracle documents.
## Temporal scope and stack risk
This implements phase checks, not admission before every parser allocation. Unit ASTs/tokens and counting scratch already exist before the per-file check; new retained syntax text is accounted only by the existing resolver walk. Existing model/serde/raw-validator and physical allocator/RSS exclusions remain unchanged. No fresh source rescans, Exact conversions, model clones, heap work containers or per-include budgets were introduced by the production path.
Adding inline CarriedInput to ParsedMixedSyntax and adding a shared parse wrapper can change debug stack frames even on the legacy route. From source field widths on the intended x64 target, the borrowed enum payload is approximately 112 bytes (bundle reference8 + Limits48 + ProjectLimits56), versus a retained reference8; actual enum/Option and frame layout is compiler-dependent and was not measured here. This is a concrete reason to retain compiler/strict-Clippy and existing small-stack regression gates, not evidence of a regression or justification for speculative boxing. Existing upstream environment-sensitive parse-stack failure remains unresolved and separate.
## Pending acceptance
Compile/type-check the guard lifetimes and moves, and run strict Clippy before drawing layout/lint conclusions. Independently exercise oracle E/H boundary pairs, exact parser-versus-resolver spans, new-snapshot work counts, retained +C slots, text timing, all inherited policy fields, and zero-mode rejection timing. Entry/text/occurrence overflow injection must remain labeled separately from positive owner provenance; the actual retained owner has zero occurrences. Test failure/drop and unchanged owner bytes through real parse/resolve paths, and preserve prior private/legacy outcomes, 16 inherited full-suite failures and formatting disposition. This review neither certifies unfinished tests nor accepts final emission/readmission or public source8.
## Committed test-observer supplemental readback
Reviewed commit `d11b99bb4ab1c817951557551db3a3c7cbd5c425`. The four working production files match that commit. `mixed/project.rs` now has SHA256 `d0b75ec30c4e433d942d8011f13ddc8321a7c060e35c7b0a320b0979fe22bc40`; the other three file hashes above remain exact. The sole additional change in the previously reviewed project file is the cfg(test) `ResolvedMixedProject::phase_state` observer at lines31–34, returning `(&self.syntax, self.storage.live)`.
The observer returns an immutable borrow tied to the resolved owner and a copied scalar Slots snapshot. It cannot mutate/reset policy, replace the bundle, extract the guard, release ownership or construct a continuation. It is absent from production builds and provides the intended test visibility into real retained state. No authority issue or new source-review blocker found. This supplement does not claim unchanged runtime behavior: compilation and all required runtime gates remain independently pending. No source edit, compiler or application execution was performed for this readback.
## Targeted slim-guard repair review
The subsequent d11b99b full run aborted with exit3221225725 at the existing ordinary-index 128 KiB test, despite the 12 focused tests passing. Read-only inspection of `carried-full-d11b99b-1.result.json` and `carried-debugger-d11b99b-1.log` confirms the reported overflow and a fault stack through Exact parsing, the shared parser body and its new wrapper. It does not establish a single quantified cause. The initial source verdict therefore did not establish legacy runtime compatibility; this regression blocks acceptance until repaired and rechecked.
Reviewed the frozen uncommitted slim-guard candidate relative to d11b99b and the revised selected contract, SHA256 `875b26eb850ab75d6e09222be37327a87a5b4c4a6f0557adf9723984f945d5f5`. **No source correctness blocker found in this targeted repair; runtime effect remains unproven.** Exact candidate hashes:
| File | SHA256 |
| --- | --- |
| `mixed.rs` | `9e67891b6610def59d39376ec78fea6151025e977c88d299dafd97b440ab4e89` |
| `mixed/carried.rs` | `32cf742631490a624e0522db82ff1d708e5d730b54a117f254ed3a106f05eca0` |
| `mixed/carried/tests.rs` | `794c8e987b43d4a652361bf7e31eff314f917955824efdbb1ba74b0216c4018a` |
`CarriedInput::into_parts` consumes the original input: Borrowed moves its captured exact policy; Retained copies the exact policy values from its immutable owner. Both derive the same bound bundle and small guard in one match. The new Budget and syntax ProjectLimits own the transferred policy, so dropping the original seed before parsing remains valid. No policy borrow from the dropped seed is introduced. Only `CarriedGuard { Borrowed | Retained(&owner) }` travels through the shared parser and later witness; Borrowed source lifetime remains tied by syntax.bundle, and Retained continues borrowing its allocation owner. Private construction/caller invariants remain intact.
Slots/check arithmetic, once-only seeding, all phase checkpoints, modes and Usage are unchanged. No heap allocation or new logical container reservation is introduced. Removing duplicated inline policy from later witnesses is a smaller change than adding a policy-owner fixture lifetime or boxing the policy; it directly reduces replicated state while preserving the contract. The extra parser wrapper itself is unchanged and may still matter; only the unchanged regression gates can demonstrate recovery.
The carried test delta changes only the guard-variant identity check and access to the existing direct overflow checks through `into_parts().2`. Expected values and assertions remain intact. `budget.rs` and `mixed/project.rs` are unchanged from d11b99b. The complete existing `mixed/project/ordinary_index/tests.rs`, including the 128 KiB regression, is unchanged from accepted18073; working SHA256 `04e11463a7d489c9984d165d80718d95da027c7613a5b63180f0eaf4038a0395`. No stack size, old fixture, thread setup or old expected outcome was weakened. Compiler, focused/full and required regression evidence must bind the actual repaired commit before acceptance.