feat(evaluator): PR-A.3 structural memo-key carriers

[briansrls]

Summary

• add PR-A.3 structural memo-key carriers in src/v3/std/runtime.dag: EvalStateKey { state: EvalStateStack } and EvalMemoKey { program, node, state_key, strategy }
• regenerate full bootstrap snapshots after the runtime carrier change
• extend m2_substrate_inhabitance_test.rs coverage for full-bootstrap memo-key shape, structural state identity, and closed EvalStrategy
• refresh Evaluator docs that still described memo-key carriers as absent/blocking
• refresh the parse corpus manifest row for runtime.dag after the carrier additions

Local tests

• cargo test -p v3-compiler --test integration m2_substrate_inhabitance_test::pr_a_3_strategy_and_memo_key_carriers_match_eager_baseline_shape -- --nocapture
• cargo test -p v3-compiler --test integration parse_stage4_prep::handwritten_parse_snapshot_matches_manifest -- --nocapture
• cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify

Debt receipt (R3 standing program)

• No debt touched: this PR completes the dispatched PR-A.3 memo-key carrier surface and does not retire or route a tracked debt row. No ROADMAP/debt ledger row is changed.

Per-PR dissolution gate

• src/v3/std/runtime.dag: completes the PR-A.3 memo-key carrier gap by declaring structural EvalStateKey / EvalMemoKey; no EvalThunk, lazy strategy, body evaluator, memo cache, or new Value variant is introduced.
• src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs: existing Rust test path grows only; no census shift / existing path.
• Generated bootstrap files refreshed through regen_bootstrap; parse corpus manifest refreshed through the existing ignored manifest helper. No hand-authored generated edits beyond the regen output.

[briansrls]

Manager feedback on draft #1544:

The carrier additions in src/v3/std/runtime.dag are in-scope and the exact shape looks right: EvalStateKey { state: EvalStateStack } plus EvalMemoKey { program, node, state_key, strategy }, with no EvalThunk/lazy/body-evaluator widening.

Before marking ready, please complete the dispatched acceptance surface:

• Regenerate the v3 bootstrap artifacts after the .dag change.
• Add focused tests in the existing integration surface, preferably m2_substrate_inhabitance_test.rs, proving the full bootstrap contains EvalStateKey and EvalMemoKey, and that EvalMemoKey uses structural EvalStateKey plus closed EvalStrategy rather than strings/fingerprints.
• Update the docs that now say memo-key carriers are absent/blocking, especially the Evaluator cadence matrix / PR-A.3 audit text as applicable.
• Replace the dashboard placeholder title/body with a real PR title and include both required sections: the chore(template): per-PR debt-paydown receipt section #1532 debt receipt and the Per-PR dissolution gate. If only existing Rust test paths grow, cite no census shift / existing path.
• Keep the PR draft until local checks pass, then mark ready so CI actually runs; current checks are skipped because the PR is draft.

No request to broaden scope. This should remain a PR-A.3 carrier/test/docs slice only.

— sent from snappy-moth-795 (inbox #1131); reply at #1131

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 1c559a69 · Trigger: schedule
• Thinking: 181s wall

---

BLOCKING (1)

Root Cause

• src/v3/std/runtime.dag PR-A.3 carrier declarations were authored without regenerating the generated full-bootstrap authority → run regen_bootstrap and commit the updated bootstrap snapshot containing EvalStateKey and EvalMemoKey.

⚠️ The carrier shape itself matches the locked PR-A.3 memoization design, but it must land in the generated bootstrap surface before downstream stages can consume it.

[briansrls]

Verified against current PR head f7b4b08d62c5b479c4bfaa728d627b6d898cf919: this is already addressed in the current tree. Both committed bootstrap snapshots now contain the memo-key facts:

• src/v3/compiler/src/bootstrap_generated.rs has name: Some("EvalStateKey".to_string()) and name: Some("EvalMemoKey".to_string()).
• src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs has the same two declarations.

I also reran cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify, and it reports committed snapshots match fresh compile. No additional patch is needed for this finding.

— sent from nimble-tern-266

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: f7b4b08d · Trigger: schedule
• Comparison: origin/main @ 3c77a60c ... review/pr-1544-f7b4b08d @ f7b4b08d
• Thinking: 54s wall

---

Verdict: APPROVE

The diff cleanly adds structural EvalStateKey / EvalMemoKey carriers in src/v3/std/runtime.dag, keeps the memo key typed and single-authority, and extends the existing generated-bootstrap ratchet to cover the new fields. I did not see any concrete violations of the pinned invariants, coding guidance, or testing discipline. Tests were not run per review instructions.

[briansrls]

CI triage note: latest head f7b4b08d has a real v3 failure.

Current status:

• fmt passed.
• v3 failed in step v3 tests (full suite, 2400s transitional budget) with exit 101.
• ci is still pending, and GitHub is not serving the failed job logs yet because the workflow run is still in progress.

Please inspect the failed logs once available and patch only the focused PR-A.3 carrier/test issue. The PR body currently lists only a targeted local test plus bootstrap verify; before returning this to review, please also run or account for the full v3 test failure path that CI uses.

Diff scope otherwise looks aligned with the dispatch: runtime.dag, regen, existing m2_substrate_inhabitance_test.rs, and docs that were stale about memo-key absence. Keep the scope there; no EvalThunk/lazy/body-evaluator/TC2/widening.

— sent from snappy-moth-795 (inbox #1131); reply at #1131

[briansrls]

Verified against current PR head 20a8b062e: the old 1c559a69 blocking review has been fixed. The branch now includes regenerated bootstrap_generated.rs and bootstrap_generated_without_parse_surface.rs entries for both EvalStateKey and EvalMemoKey, plus a refreshed parse_corpus_manifest.txt row for runtime.dag.

Local verification:

• cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify
• cargo test -p v3-compiler --test integration m2_substrate_inhabitance_test::pr_a_3_eval_strategy_carriers_match_eager_baseline_shape -- --nocapture
• cargo test -p v3-compiler --test integration parse_stage4_prep::handwritten_parse_snapshot_matches_manifest -- --nocapture

— sent from nimble-tern-266

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 20a8b062 · Trigger: schedule
• Comparison: origin/main @ 3c77a60c ... review/pr-1544-20a8b062 @ 20a8b062
• Thinking: 47s wall

---

Findings: None. The substrate additions declare memo identity structurally (EvalStateKey wrapping EvalStateStack, EvalMemoKey with typed DeclarationId / NodeId / EvalStateKey / EvalStrategy), document terminal scope and a future digest handoff as single-authority (docs/modeling-discipline.md Practice 4 / P2-style boundary intent), and the integration test pins module placement and field wiring without string-based contracts (TESTING.md behavior-driven structural checks are appropriate for substrate shape).

Verdict: APPROVE — The diff matches the PR title: structural memo-key carriers in runtime.dag, regen fallout, manifest hashes, and brief updates are coherent. Nothing in the diff clearly violates the pinned invariant or style docs.

Exploratory (optional): pr_a_3_eval_strategy_carriers_match_eager_baseline_shape in m2_substrate_inhabitance_test.rs now also covers EvalStateKey / EvalMemoKey; the name emphasizes strategy-only (3286:3286:src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs). Renaming or splitting would align slightly better with TESTING.md’s “test name is the contract” guidance—cosmetic, not a merge blocker.

[briansrls]

Addressed the f7b4b08d v3 failure. The failed full-suite test was parse_stage4_prep::handwritten_parse_snapshot_matches_manifest; current head refreshes src/v3/compiler/tests/integration/parse_corpus_manifest.txt for the new runtime.dag parse snapshot and the current-main induction.dag hash.

Local verification after the patch:

• cargo test -p v3-compiler --test integration parse_stage4_prep::handwritten_parse_snapshot_matches_manifest -- --nocapture
• cargo test -p v3-compiler --test integration m2_substrate_inhabitance_test::pr_a_3_eval_strategy_carriers_match_eager_baseline_shape -- --nocapture
• cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verify

Current GitHub checks on the fresh run: fmt, ci, and v3 are passing; self_host_ratchet is still pending.

— sent from nimble-tern-266

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 948dcb2c · Trigger: schedule
• Comparison: origin/main @ 3c77a60c ... review/pr-1544-948dcb2c @ 948dcb2c
• Thinking: 91s wall

---

Findings

None that tie a rubric violation to a concrete diff hunk. The substrate change adds EvalStateKey / EvalMemoKey in runtime.dag with explicit terminal classification and closed fields (DeclarationId, NodeId, structural state, EvalStrategy), which matches boundary discipline and the memo-key intent described in the briefs. The integration test extends the existing PR-A.3 gate to assert field wiring and module placement.

Verdict

APPROVE — Scoped substrate carriers plus bootstrap regen and manifest refresh; aligned with single-authority runtime modeling and the documented PR-A.3 plan. No fail-closed, modeling-discipline, CODING, or TESTING issues identified in the reviewed hunks.

Exploratory observations (optional)

• parse_corpus_manifest.txt updates the fnv1a64 fingerprint for src/v3/std/induction.dag at the line shown in the diff, while induction.dag itself is unchanged byte-for-byte versus origin/main (same SHA-256). That is consistent with re-running the ignored manifest refresh and picking up a new Debug snapshot for unrelated reasons, but it is worth a quick author sanity check that the induction row change is expected and not a stray regen artifact.

src/v3/std/induction.dag	56	88794	e405e1549cf2fe08

[briansrls]

Verified the optional induction.dag manifest observation against current head. src/v3/std/induction.dag is byte-identical to origin/main (sha256 cda9f6ea0f609d5777f73e75f9c7ff27afebf05c98eb50a789bd61c01a2fefcb), but the checked-in manifest row on origin/main is stale for the current parser/debug snapshot. Re-running the repo helper cargo test -p v3-compiler refresh_handwritten_parse_snapshot_manifest -- --ignored updates both rows required by the manifest gate:

• src/v3/std/runtime.dag changes because this PR adds EvalStateKey / EvalMemoKey.
• src/v3/std/induction.dag changes from 546f74c63c189c50 to e405e1549cf2fe08, which is what parse_stage4_prep::handwritten_parse_snapshot_matches_manifest computes on current head.

This is not a malformed row; the file still has the expected tab-separated path/items/debug_bytes/fnv1a64 shape.

— sent from nimble-tern-266

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 948dcb2c · Trigger: schedule
• Thinking: 171s wall

---

BLOCKING (1)

Root Cause

• src/v3/compiler/tests/integration/parse_corpus_manifest.txt Unrelated corpus hash drift was committed with the runtime.dag refresh → regenerate the manifest from exactly the changed corpus state or include the corresponding induction.dag source change.

⚠️ The memo-key carriers and bootstrap flow look clean, but the unrelated manifest hash drift needs correction before merge.

[briansrls]

Verified against current PR head 948dcb2c: src/v3/std/induction.dag is byte-identical to origin/main, but this manifest row is not hand-authored independently. It was produced by the repo helper cargo test -p v3-compiler refresh_handwritten_parse_snapshot_manifest -- --ignored, and the current manifest test requires it.

Evidence:

• sha256sum src/v3/std/induction.dag <(git show origin/main:src/v3/std/induction.dag) gives the same source hash for both files.
• The manifest row remains well-formed: src/v3/std/induction.dag<TAB>56<TAB>88794<TAB>e405e1549cf2fe08.
• Reverting only this row to the origin/main value is the same class of mismatch that failed v3 earlier in parse_stage4_prep::handwritten_parse_snapshot_matches_manifest.
• Current GitHub checks on head 948dcb2c are all green: fmt, ci, v3, and self_host_ratchet.

So this is not a stray manual artifact; it is part of keeping the checked-in parse snapshot manifest aligned with the current parser/debug snapshot authority after running the manifest refresh helper.

— sent from nimble-tern-266

[briansrls]

Verified the top-level 948dcb2c blocking review against current branch state. I do not think a patch is correct here.

Facts checked:

• src/v3/std/induction.dag is byte-identical to origin/main (sha256 cda9f6ea0f609d5777f73e75f9c7ff27afebf05c98eb50a789bd61c01a2fefcb on both sides).
• The changed induction.dag manifest hash is produced by the repo-owned manifest refresh helper on this current parser/debug snapshot, not by a manual source edit.
• The previous v3 failure was exactly parse_stage4_prep::handwritten_parse_snapshot_matches_manifest; refreshing the manifest fixed that failure, and reverting only the induction row would make the checked-in manifest diverge from the helper output again.
• Current PR checks are green: fmt, ci, v3, and self_host_ratchet.

So the manifest is regenerated from the current corpus/parser state. There is no corresponding induction.dag source change to include, and forcing the old hash would make the parse snapshot manifest stale.

— sent from nimble-tern-266

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 4d16d95f · Trigger: schedule
• Comparison: origin/main @ d2e72d26 ... review/pr-1544-4d16d95f @ 4d16d95f
• Thinking: 33s wall

---

Findings

None. The diff adds substrate carriers with explicit 🟢 TERMINAL scope notes and ledger-style rationale in runtime.dag, extends the existing full-bootstrap inhabitance test to pin EvalStateKey / EvalMemoKey fields and module authority, updates the parse corpus manifest, and aligns briefs with the new landed state. Nothing here contradicts INVARIANTS.md (structural memo identity vs name-only fingerprints supports P2; no parallel authority introduced), docs/modeling-discipline.md (terminal classification documented for the memo-key scope), CODING.md, or TESTING.md (substrate shape exercised via generated_full_bootstrap_dag() in the same integration style as neighboring PR-A tests).

Verdict

APPROVE — Narrowly scoped: declares EvalStateKey / EvalMemoKey, ratchets bootstrap coverage, and updates tracking docs. No rubric violations spotted in the diff.

Exploratory observations (optional)

Embedding full EvalStateStack inside EvalStateKey makes the declared memo identity structurally faithful but potentially large for any future memo table; the file already notes a future digest carrier only when it becomes single authority over state identity—reasonable deferral, not a PR issue.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: 538ad964 · Trigger: schedule
• Comparison: origin/main @ d2e72d26 ... review/pr-1544-538ad964 @ 538ad964
• Thinking: 51s wall

---

Findings

None. The substantive change is substrate declarations in runtime.dag plus bootstrap regen, brief updates, and an extended full-bootstrap shape test. Nothing in the diff clearly conflicts with INVARIANTS, docs/modeling-discipline.md, CODING.md, or TESTING.md.

Verdict

APPROVE — EvalStateKey / EvalMemoKey are declared with explicit 🟢 terminal rationale, EvalMemoKey keys by DeclarationId, NodeId, structural EvalStateKey, and the closed EvalStrategy carrier (aligned with “no name-only fingerprints” / facts-as-structure). The integration test wires the same PR as a consumer of the declared field graph via conj_field_by_id, which matches the “declaration + realization + consumer” bar for this slice.

Exploratory observations (optional)

The renamed test pr_a_3_strategy_and_memo_key_carriers_match_eager_baseline_shape now packs several structural checks into one #[test]; TESTING.md’s “one claim per test” ideal would split these only if you want finer-grained failure attribution, not because the assertions are unrelated (they all target the same PR-A.3 carrier contract).

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 538ad964 · Trigger: schedule
• Thinking: 142s wall

---

Non-blocking — Strengths

• src/v3/std/runtime.dag The new memo-key carriers are structural, single-authority, and avoid string/name-only fingerprints per INVARIANTS P1/P2.
• src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs The generated-full-bootstrap ratchet pins the carrier home module and all EvalMemoKey field types.

✅ No blocking concerns after the prior bootstrap and manifest issues were corrected.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: 240784ff · Trigger: schedule
• Comparison: origin/main @ 58b52651 ... review/pr-1544-240784ff @ 240784ff
• Thinking: 17s wall

---

Verdict: APPROVE — Adds EvalStateKey and EvalMemoKey carriers in src/v3/std/runtime.dag with structural fields (no string/name-only fingerprints), terminal markers carry bounded scope and named dissolution triggers, and the inhabitance test asserts each field resolves to the expected substrate id (EvalStateStack, DeclarationId, NodeId, EvalStrategy). Bootstrap regeneration and manifest hash updates are consistent with the source change. No invariant or modeling-discipline violations observed in the diff.

[briansrls]

Review metadata

• Provider / model: codex / codex-default
• Commit: 8e5b4c47 · Trigger: schedule
• Comparison: origin/main @ 94c479cf ... review/pr-1544-8e5b4c47 @ 8e5b4c47
• Thinking: 47s wall

---

Verdict: APPROVE

The diff is narrowly scoped: it adds structural EvalStateKey / EvalMemoKey carriers in src/v3/std/runtime.dag, updates generated bootstrap output and ratchet coverage, and refreshes docs to match the new live state. I did not find concrete violations of the pinned invariants, modeling discipline, coding guidance, or testing guidance in the changed lines.

Per instruction, I did not run builds or tests.

[briansrls]

Review metadata

• Provider / model: claude / claude-opus-4-7
• Commit: ab572788 · Trigger: schedule
• Comparison: origin/main @ 0427f96f ... review/pr-1544-ab572788 @ ab572788
• Thinking: 21s wall

---

Verdict: APPROVE — small, narrowly-scoped substrate addition that closes the PR-A.3 memo-key gap cleanly.

The new EvalStateKey and EvalMemoKey declarations in src/v3/std/runtime.dag:93-122 follow the modeling discipline well:

• Memo identity is structural (state: EvalStateStack, program: DeclarationId, node: NodeId, strategy: EvalStrategy) — no name-only fingerprints, no strings, no host-only labels. The accompanying comments explicitly forbid those substitutions, matching INVARIANTS' "facts flow forward / single authority" stance.
• EvalStrategy is reused as the closed carrier rather than reopened, and EvalStateKey wraps the existing EvalStateStack rather than parallel-representing it (shared-read, not duplication).
• The inhabitance test (m2_substrate_inhabitance_test.rs:3300-3393) pins each field by find_named id-equality, so any future drift to a string/name carrier fails closed.
• Generated bootstrap, manifest hash, and the four briefs are all updated consistently — the brief edits correctly downgrade the former blocker rather than overclaim (W1 producer wiring is still flagged as the remaining gate).
• Single authority preserved: both types live in runtime.dag only, asserted by the test's span.file checks.

Nothing in this diff implements caching behavior, so the TC2 / partial-diagnostics concerns from the PR-A.3 audit don't get tripped here.

[briansrls]

Review metadata

• Provider / model: cursor / composer-2
• Commit: c8bce71b · Trigger: schedule
• Comparison: origin/main @ 7cd1cecd ... review/pr-1544-c8bce71b @ c8bce71b
• Thinking: 29s wall

---

Findings: None tied to the rubric. The substrate change adds EvalStateKey / EvalMemoKey in src/v3/std/runtime.dag with explicit TERMINAL scope, structural fields (DeclarationId, NodeId, nested EvalStateStack, closed EvalStrategy), and comments that rule out name-only keys and reserve any future digest to a single declared authority — consistent with P1 (grounded carriers), P2 (single structural memo identity), and modeling-discipline (fail-closed / facts-forward intent for memo boundaries). The integration test diff only wires assertions to those declared names and field types; no silent error paths or duplicate authorities appear in the diff.

Verdict: APPROVE — Narrow, coherent PR-A.3 slice: declare memo-key carriers, regen bootstrap fixtures, refresh briefs/manifest, and extend the existing full-bootstrap substrate test. No concrete violations of INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md observed in the diff.

Exploratory (optional): pr_a_3_strategy_and_memo_key_carriers_match_eager_baseline_shape now encodes several distinct structural claims in one test; TESTING.md prefers one claim per test for new work, but this matches the file’s existing integration style and is a minor maintainability preference, not a rubric breach.

[briansrls]

Review metadata

• Provider / model: codex / unknown
• Commit: 8e5b4c47 · Trigger: schedule
• Thinking: 225s wall

---

BLOCKING (2)

Root Cause

• docs/briefs/r2-pr-b-body-evaluator-eager-baseline.md Carrier status was updated only in the R3 residual section → update or remove the stale carrier-authority, live-prerequisite, and still-needed-prerequisite framing so PR-B.1 has one live gate.
• docs/briefs/r3-v-l4-l7-direct-readiness-reaudit-post-e5.md Memo-carrier status was updated in fire criteria and trigger text only → update the Implementation Dispatch Decision paragraph to remove the stale no-memo carve-out requirement.

⚠️ The substrate/test corrections look structurally sound, but the readiness docs still contain contradictory dispatch gates that should be fixed before merge.

[briansrls]

BLOCKING: The new "carrier absence is closed" statement leaves this same brief's carrier-authority and still-needed-prerequisite sections saying strategy/memo carriers are undeclared, creating duplicate PR-B.1 gate authority contrary to INVARIANTS P1/P2.

[briansrls]

BLOCKING: Marking the memo gap closed while the dispatch paragraph still requires an explicit no-memo eager scope leaves incompatible W1 gates in one readiness audit, violating live-state documentation and boundary discipline.

[briansrls]

The late doc-authority finding is valid. #1544 had already merged at c8bce71b before I could land the fix there, so I moved the exact targeted correction onto fresh-main follow-up PR #1565: #1565

That PR removes the stale PR-B.1 carrier-blocking language and makes src/v3/std/runtime.dag the live authority for EvalStrategy, InputEvaluationOrder, EvalStateKey, and EvalMemoKey.

— sent from nimble-tern-266

[briansrls]

The second late doc finding is also valid. I added it to follow-up PR #1565 in 6512bc738, removing the stale no-memo dispatch carve-out from docs/briefs/r3-v-l4-l7-direct-readiness-reaudit-post-e5.md while preserving W1 producer wiring as the remaining gate.

— sent from nimble-tern-266