[Task 55] Phase 4e: comprehensive — foundation commit

[boldfield]

Plan B Task 55 — Phase 4e (cadence pivot: PR #26 closeout)

Status: foundation + helper-side lambda-lifting first slices landed; ready to merge. 28 commits on branch. CI green on all 4 lanes at HEAD fe2e6b9. Cadence pivot at fe2e6b9 splits the comprehensive Phase 4e single-PR commitment into PR #26 (this PR — closes here) plus a plan-b-task-55-phase-4e-captures follow-up PR for items 5/6/7 of the deviation entry.

Cadence pivot (mid-Phase-4e)

The original Phase 4e deviation entry committed to a single-PR comprehensive scope. Mid-implementation that turned out wrong in practice — PR #26 reached 27 commits and the projected size for the remaining three lifts (non-tail k, multi-shot k, surrounding-lambda captures) would have pushed the total to ~8,000–10,000 lines and 40+ commits, exceeding the threshold where the reviewer pair can verify a single-pass merge. See the "Cadence pivot" addendum at the end of [DEVIATION Task 55] Phase 4e — comprehensive for full reasoning.

The pivot does NOT revise the architecture: the lambda-lifting + trailing-pair convention committed to in sections 5 and 6 of the deviation entry remains the standing architecture; the captures+ PR implements it as written. Path 2 (synchronous drive in arm body) was investigated mid-Phase-4e and rejected because it nests sigil_run_loop inside arm-body lowering, violating Plan B's stack-bounded trampoline charter.

What ships here (PR #26)

Architectural foundation

• CPS-color user-fn calling convention — uniform CPS ABI: extern "C" fn(closure_ptr, args_ptr, args_len) -> *mut NextStep. CPS-color user fns get this ABI; native-color fns continue with their declared Cranelift signatures.
• Native↔CPS interop wrappers inlined at native callsites of CPS callees (pack args + call CPS fn + drive sigil_run_loop + narrow result).
• Codegen-consumes-color per-fn ABI selection via compute_user_fn_abi.
• Colorer regression-pinning — investigation at 5cc3a58 confirmed the colorer's existing behavior is correct (the asymmetry between find_non_io_perform_in_expr skipping handle bodies and collect_calls_in_expr recursing into them is intentional; the SCC bridge already taints fns whose handle bodies call CPS-color helpers). Three regression tests pin the load-bearing rules.
• Real CPS transform via Option B inline lowering — synthetic continuation closures generated post-walker / post-closure-convert at codegen-pass time, mirroring the HandlerArmSynth pattern. The transitional CpsProgram wrapper was deleted at 3c65054 (accessors moved to ColoredProgram directly).

Helper-side lambda-lifting first slices

• ConstantDone constant-tail synth-cont at b818fc3 — handles helpers with body shape stmt_perform; constant_tail. Inverts statement_form_non_io_perform_inside_handle_compiles_and_runs (stdout 42 → 99).
• Captures-free LetBindThenTail synth-cont at 2a9958b — handles let x = perform; pure_tail arity-0 helpers. Closes hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2's discard_k_handler_does_not_abort_helper_phase_4e_pending (un-#[ignore]'d, stdout flipped from 142 to 42).
• Captures-bearing LetBindThenTail synth-cont at a5ee4c6 — lifts the arity-0 restriction; arity-N helpers with user params referenced in tail.

Code-quality foundation

• FFI-ref dedup at 73c7e53 — prepare_per_fn_refs(module, builder, ctx) extraction replacing 3 duplicated ~70-LOC sites (PerFnRefsCtx<'a> + PerFnRefs structs). Closes the deferred-must-fix flagged in three consecutive mid-flight reviews.
• Pattern-walker dedup + Char capture e2e + doc precision at adf0e23 — drops byte-equivalent arm_body_collect_pattern_bindings; adds cps_abi_captures_bearing_with_char_capture_exercises_widen_narrow_symmetry parallel to the Bool capture test; corrects the "Cranelift prunes unused FuncRefs" wording at four occurrences.

Test coverage

• Pattern matrix complete: all 4 binding shapes (Var, Tuple, Ctor::Unit/Positional/Record) have shadowing-shadow tests.
• Codegen-consumes-color binding-type matrix complete: Bool, Char, Int, String, pointer-typed all have e2e coverage; both write-side widen and read-side narrow tested for non-Int kinds.
• Closure record slot ordering pinned via multi-capture e2e: captures[i] at closure_ptr + 16 + 8*i.
• 466 compiler lib tests + 17 cps-related e2e tests pass locally; CI green on all 4 lanes.

Hard conditions for PR #26 closure

• Hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 (a): discard_k_handler_does_not_abort_helper_phase_4e_pending un-#[ignore]'d, stdout flipped to 42 — closed at 2a9958b.
• Hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 (b): statement_form_non_io_perform_inside_handle_compiles_and_runs stdout flipped to 99 — closed at b818fc3.
• Cadence-pivot addendum in PLAN_B_DEVIATIONS.md mirroring the foundation entry's pattern — landed at fe2e6b9.
• README "Verification limits" updated to reflect what closes at PR [Task 55] Phase 4e: comprehensive — foundation commit #26 vs what's deferred to captures+ — landed at fe2e6b9.
• PLAN_B_PROGRESS.md updated with the cadence pivot — landed at fe2e6b9.
• All 4 CI lanes SUCCESS at HEAD.

Hard condition #1 (carry-forward to captures+)

The original Phase 4e hard condition #1 ("README 'Verification limits' lands in the same PR — explicit, user-facing call-out of what closes here AND what remains for Phase 4f/4g") splits across PRs per the cadence pivot. PR #26's README update names what closes here (discard-k correctness) and what remains for the captures+ PR (non-tail k, multi-shot k, surrounding-lambda captures); the captures+ PR's closeout will land the final cleanup pass with Phase 4f/4g framing.

What's deferred to plan-b-task-55-phase-4e-captures (follow-up PR against main)

• Item 5: Non-tail k use in arm bodies — walker rejection lifts; arm-body lambda-lifting via post-arm-k synth fn; trailing-pair convention [arg, post_arm_k_closure, post_arm_k_fn] at the arm's Call emission; helper's synth-cont signature shifts to uniformly read trailing post-arm-k pair.
• Item 6: Multi-shot k use — resumes_many gating; heap-reified k_closure for resumes: many arms; Choose-style e2e tests pinning multi-invocation invariance.
• Item 7: Surrounding-lambda closure captures into arm bodies — typecheck side-table extension; codegen reads from lambda's closure_ptr for outer-scope captures; the arm_inside_lambda_captures_outer_via_closure_env_load_is_rejected_at_codegen_phase_4e_pending #[ignore]'d test inverts.
• Test inversion + cleanup (walker's Phase-4e-pointing diagnostics removed).
• README "Verification limits" final cleanup with Phase 4f/4g framing.
• PLAN_B_PROGRESS.md Phase 4e final status flip to done-pending-ci.

Stage 9 unblock — split

Stage 9's scripts/validate-spec.sh was originally gated on full Phase 4e closure. Per the pivot:

• PR [Task 55] Phase 4e: comprehensive — foundation commit #26 closes the discard-k correctness gap — the load-bearing semantic gap.
• Captures+ PR closes the multi-shot k gap — required by P20 (multi-shot Choose).
• Stage 9 unblocks when captures+ squash-merges, not PR [Task 55] Phase 4e: comprehensive — foundation commit #26.

The PLAN_B_PROGRESS.md Phase 4e HARD-GATE annotation reflects this split.

Multi-week single-PR caveat

The user accepted the multi-week, single-PR, high review-burden tradeoff at Phase 4e foundation. That tradeoff still holds in spirit — the captures+ PR is the second half of the same multi-week effort — but the cadence pivot accepts that the practical reviewability ceiling forces a split point. This is the same shape pivot that happened at Phase 4b's PR-cadence convention shift from the foundation entry.

Implementing commit roadmap (within this PR)

• Foundation: deviation + PROGRESS status update.
• Real CPS transform in cps.rs. Replaced by inline CPS transform in codegen.rs per Option B pivot; cps.rs deleted at 3c65054.
• Codegen consumes color: CPS-color user fns get CPS ABI; native callers of CPS get inlined wrapper.
• Colorer's handler-discharge classification — pivoted to regression-tests-only at 5cc3a58 (colorer was already correct; no code change needed).
• Helper-side lambda-lifting first slices — ConstantDone at b818fc3; captures-free LetBindThenTail at 2a9958b; captures-bearing LetBindThenTail at a5ee4c6.
• FFI-ref dedup — prepare_per_fn_refs at 73c7e53.
• Pattern-walker dedup + Char capture e2e + doc precision at adf0e23.
• Cadence-pivot closeout-prep at fe2e6b9.
• Multi-shot k lift. → captures+ PR.
• Surrounding-lambda captures. → captures+ PR.
• Arm-body non-tail-k lift + final test inversion + cleanup. → captures+ PR.

Test plan

• Pod-verify clean at every commit.
• CI green on both ubuntu-24.04 (x86_64-linux) and macos-14 (aarch64-darwin) — all 4 lanes at HEAD.
• discard_k_handler_does_not_abort_helper_phase_4e_pending inverts to a positive test — closed at 2a9958b.
• statement_form_non_io_perform_inside_handle_compiles_and_runs inverts to assert 99 — closed at b818fc3.
• All Phase 4d MVP positive tests still pass.
• Cross-call discard-k correctness — closed via codegen-consumes-color.
• Helper-side non-tail body lambda-lifting — closed via synth-cont machinery.
• Arm-body non-tail k. → captures+ PR.
• Multi-shot k. → captures+ PR.

See [DEVIATION Task 55] Phase 4e — comprehensive (especially the "Cadence pivot" addendum at the end) in PLAN_B_DEVIATIONS.md for full architectural details and the captures+ PR scope.

[boldfield]

WIP review at 5cc3a58 — foundation only

Two commits landed: deviation entry (~85 lines) + 3 colorer regression tests (~168 LOC). Per the agent's framing this is the in-flight container for the comprehensive Phase 4e work; implementing commits follow on the same branch.

Reviewing what's there now plus forward-looking concerns for the 7 implementing commits the deviation entry roadmaps.

Per-task

Item	Status
Foundation commit (deviation + PROGRESS)	ship — solid
Colorer regression tests (3)	ship — well-designed
Implementing commits (real CPS transform, codegen consumes color, etc.)	not yet landed; forward-looking notes below

Mechanical (foundation only)

• All 4 CI lanes SUCCESS on 5cc3a58
• 423 → 426 compiler lib tests (+3 new colorer tests)
• Pod-verify clean per commit message

One real issue: deviation entry vs commit message disagree about colorer

The Phase 4e deviation entry (line 1369, section 4 "Colorer's handler-discharge refinement") describes a code change to the colorer:

"The refinement uses the existing call-site instantiation table (CheckedProgram::call_site_instantiations) to drive an inter-procedural reachability check: for each handle body, walk its calls; if any callee transitively performs an effect the handle discharges, the calling fn is CPS."

But commit 5cc3a58's message says:

"the colorer is already correct for these cases, and the actual 'refinement' is in codegen (consume color and emit CPS-color user fns with the CPS calling convention) — not in color.rs."

Test #1 (handle_body_calling_cps_helper_makes_caller_cps_via_bridge) confirms the latter — helper's intrinsic CPS color (row contains Raise) already taints main via the existing SCC bridge through the call graph, no colorer change required. The asymmetry is correct: find_non_io_perform_in_expr skips handle bodies (those performs are discharged so don't taint local color), but the call-graph collection DOES descend into handle bodies (called fns may still need CPS calling convention).

Action: update the deviation entry's section 4 before the colorer-refinement commit lands. Either:

• (a) Reframe section 4 as "Verify colorer already handles this correctly; pin via regression tests" with a note that no color.rs code change is needed
• (b) Identify the specific colorer change that IS needed (if any exists beyond what test Plan A1 code-review fixes: 6 issues from the post-review audit #1 already covers)

Don't ship the deviation entry's section 4 as written when the implementing commit will be "no code change, just tests already landed." That's the kind of doc-vs-code drift the Stage 6 review pattern has been catching consistently.

Foundation pieces verified solid

Deviation entry quality: Substantive, follows the established structure. Comprehensive scope rationale documented (single-PR vs 4-PR sub-sequence; reviewability vs design-coherence tradeoff). All 4 hard conditions enumerated. Carry-forward closure points listed (synchronous-run_loop line 164, Phase 4b stack-vs-arena entry, Phase 4d MVP four enumerated gaps). Bisecting-hint pattern catalogues 5 failure modes by architectural surface.

3 regression tests:

1. handle_body_calling_cps_helper_makes_caller_cps_via_bridge — pins discharge-via-call-graph SCC bridge. Load-bearing for cross-call discard-k correctness.
2. handle_body_with_only_direct_perform_keeps_caller_native_under_existing_local_rule — pins the local-walk skip-the-handle-body rule. Guards against codegen emitting CPS for fns that don't need it (perf regression guard).
3. handle_arm_body_performing_undischarged_effect_taints_caller_intrinsically — pins arm bodies DO contribute to intrinsic-CPS analysis. Defensive — typecheck E0042 catches it in real code, but the colorer must stay robust post-mono.

Test design is right: each test pins a specific load-bearing rule that the implementing commits depend on. If a future refactor breaks one, codegen behavior regresses in a specific traceable way.

Forward-looking concerns for subsequent commits

These are pre-registered concerns the agent should consider while implementing each layer. Categorize as flags for review, not must-fix-yet (the code doesn't exist).

1. CPS transform (cps.rs) — preserve the differential identity property test.
PR #22's MF2 test cps_wrapped_identity_matches_native_on_native_eligible_programs runs Native-eligible programs both raw and wrapped in a vacuous handler, asserting identical output. After Phase 4e, that test must still pass. The CPS transform's correctness on Native-eligible programs is the highest-stakes item (it caught the elaborate non-recursion bug). Worth running this test at every commit checkpoint, not just the final commit.

2. Codegen consumes color — verify main's classification.
What classification does main get after Phase 4e? Today main is wrapped by codegen (entry shim). If main's body contains a handle whose body calls a CPS-color helper, the colorer will classify main as CPS. Does the entry shim still wrap correctly for a CPS-color main? Worth a test pinning the shape — e.g., a program where main is forced CPS via a discharging handle whose body calls a Raise helper.

3. Native↔CPS interop wrappers inlined at every call site — count first.
The deviation entry says wrappers are inlined per call site rather than synthesized as separate fns (rationale: ~4 Cranelift instructions per site; avoids fn-call frame overhead). Reasonable. But: how many native-of-CPS call edges currently exist in the codebase? Stdlib, examples, tests. If it's a large number, the inlining cost could surprise. Worth a --dump-color count before vs after.

4. Colorer refinement commit — minimal scope per the pivot.
Per the section above, this commit may be just "tests already landed; no code change to color.rs." If so, the commit should be small and the message should explicitly say so (to avoid future bisecting agents looking for color.rs changes that don't exist).

5. Non-tail k lift — verify the post-k lambda lift handles match/if branches.
PR #25's MF1 caught walker/detector tail-position mismatch on if c { k(x) } else { k(y) }. Phase 4e lifts non-tail k. Does it also lift the multi-branch tail-k case (if c { k(x) } else { k(y) } becomes if c { call k with continuation } else { call k with continuation })? Or is multi-branch tail-k still rejected, with the rejection diagnostic re-aimed past Phase 4e? Worth pinning.

6. Multi-shot k — verify second-invocation isolation.
The deviation entry section 6 says "calling k(arg2) with the same closure returns a fresh Call with arg2." Critical correctness: the heap-reified continuation's environment must be effectively read-only after construction. A multi-shot test should call k twice with different args, capture both results, and assert they're independent (no shared mutable state). The PR #21 GC stress test pattern (subprocess-isolated tests) might be relevant here if the multi-shot machinery interacts with GC pressure.

7. Args-buffer arena migration — verify the closure point closes correctly.
The deviation entry says CPS-color performs allocate args via sigil_arena_alloc instead of create_sized_stack_slot because the perform-site stack frame dies before the trampoline reads them. This is the Phase 4b stack-vs-arena closure point. Verify both: (a) CPS-color performs use arena allocation, (b) Native-color performs (post-Phase-4e, should be none in well-formed programs?) — what's the expected shape?

8. Test inversion: discard_k_handler_does_not_abort_helper_phase_4e_pending.
Hard condition #2. The test currently asserts stdout 142 (Phase 4d MVP behavior). Post-Phase-4e it should assert 42. Verify: (a) the test inversion lands in the colorer-refinement-OR-codegen commit (whichever closes the gap), (b) the commit message explicitly names the test inversion as a checkpoint, (c) the README "Verification limits" section's discard-k entry gets removed (or marked closed).

Cumulative scope check

Per my prior estimate, Phase 4e was ~1-2 PRs and ~2-3 review rounds. The agent's choice to ship comprehensively as 1 PR over multiple commits is consistent with that estimate's upper bound. Track:

• Foundation: done (this PR)
• Real CPS transform: 1 commit
• Codegen consumes color: 1 commit (the big one)
• Colorer refinement: 1 commit (now pivoted to tests-only per above)
• Non-tail k lift: 1 commit
• Multi-shot k lift: 1 commit
• Surrounding-lambda captures: 1 commit
• Test inversion + cleanup: 1 commit
• PROGRESS final update: 1 commit

= 8 implementing commits + foundation. Realistic for the comprehensive scope. Each commit should be independently pod-verified before the next layer begins (per the deviation entry's explicit commit organisation discipline).

What I'm watching for at each subsequent push

I'll do a focused mid-flight review after the codegen-consumes-color commit lands (the big one) and a comprehensive review after the test-inversion commit. Foundation is solid; implementing commits will need substantive review per architectural surface.

---

Foundation ships clean once the deviation-entry-vs-commit-message inconsistency is resolved. Forward-looking concerns are flags, not blockers — track them as you implement.

[boldfield]

Mid-flight review at df251fc

Two new commits since the WIP review at 5cc3a58. Both address feedback constructively. CI in progress (1 of 4 lanes green); 425 compiler lib tests (was 423).

Verification of WIP review must-fix

Item

Status

Deviation entry section 4 reframed (colorer-already-correct pivot)

✅ Closed via d8a4771. Per the commit message: option (a) chosen — "Verify colorer already handles this correctly; pin via regression tests" with explicit no-code-change-to-color.rs note. Three other passages updated for consistency: rationale (e), commit-organisation roadmap, and bisecting-hint pattern entry. The PR #18 reviewer's open Stage-6 ask is correctly reframed as "codegen consuming the colorer's already-correct classification" rather than producing different classifications. Items 2 and 3 of the comprehensive scope (CPS-color user fn ABI; native↔CPS interop) remain the load-bearing changes

Doc-vs-code drift resolved cleanly. The reframing is internally consistent and the foundation commits' message-vs-doc story now agrees.

Positive discovery worth tracking — second test needs inversion

df251fc surfaces a real find that wasn't in my prior review: the existing passing e2e test statement_form_non_io_perform_inside_handle_compiles_and_runs (compiler/tests/e2e.rs around line 1054) ALREADY exercises the discard-k cross-boundary scenario — main calls helper inside a handle body, helper performs E.op() as a Stmt::Perform, arm discards k. That test currently asserts stdout 42 (Phase 4d MVP synchronous-shape behavior); algebraic semantics gives 99 (discard-k arm fires, helper aborts, handle's overall is the arm value).

This means Phase 4e's codegen-consumes-color commit must invert two tests, not one:

1. discard_k_handler_does_not_abort_helper_phase_4e_pending — un-#[ignore]'d, assert 42 (per hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 already tracked in deviation entry)
2. statement_form_non_io_perform_inside_handle_compiles_and_runs — flip assertion from 42 → 99

The agent should update hard condition #2 in the Phase 4e deviation entry to enumerate both inversions, so the commit organisation's "Test inversion + cleanup" step covers both. Without this update, a reviewer at the test-inversion checkpoint might check only the #[ignore]'d test inversion and miss that the existing positive test also needs to flip its assertion. Cheap doc fix; closes the gap.

Verification of new regression test (df251fc)

The new colorer test pins main's classification (CPS via bridge to helper) for the same source as the e2e test. The reasoning is sound: if a future colorer refactor breaks the bridge classification (e.g., dropping call-graph edges out of Expr::Handle::body), this test fires before the e2e test does — failure points at color.rs, not at codegen. Diagnostic value is real.

Together with the 3 synthesized regression tests from 5cc3a58, the colorer's discharge-via-call-graph classification surface is now comprehensively pinned. Four tests covering: (1) bridge propagation, (2) local-walk skip, (3) arm-body-perform tainting, (4) real shipped e2e source matching.

Foundation status

• 4 foundation commits total (deviation entry, regression tests ×2, deviation reframe)
• 425 compiler lib tests (was 423)
• No implementing code yet; all foundation + test-pinning work
• CI in progress on df251fc

Forward-looking concerns from prior review

All 8 still tracked, no new ones. The strongest signal from this round is that the test-pinning work is producing better regression coverage than I expected — the agent has identified a real adjacent test that needs inversion and pinned the colorer classification that drives the inversion correctness. When the codegen-consumes-color commit lands, the implementing diff will have unusually strong test infrastructure waiting for it.

Action for the agent

One small must-fix:

Update hard condition #2 in the Phase 4e deviation entry to enumerate both test inversions — the #[ignore]'d discard_k_handler_does_not_abort_helper_phase_4e_pending AND the existing statement_form_non_io_perform_inside_handle_compiles_and_runs (flip from 42 → 99). The Test inversion + cleanup commit organisation step needs both in scope.

Otherwise foundation is solid. Implementing commits next.

[boldfield]

Mid-flight review at a756bd3

Two new commits since prior review at df251fc. CI in progress on all 4 lanes; 425 → 430 compiler lib tests (+5 from new cps.rs tests).

Verification of prior must-fix

Item

Status

Hard condition #2 enumerates both test inversions

✅ Closed via eae3589. The deviation entry now lists (a) discard_k_handler_does_not_abort_helper_phase_4e_pending un-#[ignore] + 142→42, and (b) statement_form_non_io_perform_inside_handle_compiles_and_runs flip 42→99. The commit message also requires the test inversion commit to name this deviation entry by title AND explicitly call out both inversions. Closes the "reviewer might miss" failure mode

One new must-fix: option A vs option B doc-vs-code drift

a756bd3's commit message (and the new cps.rs module docs) introduces a real design pivot:

"Design option A (separate IR pass with CpsTailCall/CpsDone/CpsContinue Expr variants) vs option B (inline lowering in codegen, generalising the Phase 4d MVP synthetic-arm-fn machinery). Option B is the leading choice; the deviation entry's section 1 was framed around option A and may need a future update at the implementing commit."

The agent themselves flagged this — good catch — but the resolution should land BEFORE the codegen-consumes-color implementing commit, not deferred. The Phase 4e deviation entry's section 1 currently claims:

"The transform produces a new IR shape (CpsExpr or extension to Expr with a CPS-form variant) that codegen consumes..."

And rationale (b) explicitly defends option A:

"CPS transform produces an extended Expr IR rather than a fresh CpsExpr AST. ... The CPS-form-specific shapes (Expr::CpsTailCall, Expr::CpsDone, Expr::CpsContinue) become new variants on Expr with codegen lowering rules."

If option B is the actual choice, both passages need updating. Same doc-vs-code drift pattern as the section 4 colorer issue from the prior WIP review — fix it now, before the implementing commit lands and a reviewer is left wondering whether the diff matches the design.

Action: update the deviation entry's section 1 + rationale (b) to reflect option B with the rationale (reuses Phase 4d MVP synthetic-arm-fn machinery; avoids touching every Expr-handling pass; testable via existing e2e infrastructure since cps tests already cross-link to the e2e tests). Document why option B beats option A — testability tradeoff is the most important consideration to flag, since unit-testing a separate transform is harder to replace with e2e coverage.

Verification of cps.rs work

The 5 new cps.rs tests are well-designed:

Test	Purpose
transform_is_typed_pass_through_at_head	Pins HEAD behavior — pure plumbing now, real transform comes with codegen-consumes-color
needs_cps_transform_native_main_returns_false	Defensive — Native main doesn't need CPS
needs_cps_transform_unknown_fn_returns_false	Defensive — accessor is a query, not a panic-on-unknown
needs_cps_transform_classifies_cps_color_helper_correctly	Cross-test linkage — uses same source as e2e statement_form_non_io_perform_inside_handle AND df251fc's colorer pinning test. Three-test chain pinning the same scenario at three layers (color → cps query → e2e behavior)
cps_color_user_fns_lists_program_order_cps_only	Reproducibility-stable; Plan A1 test depends on this ordering

The cross-test linkage in test 4 is the load-bearing pattern — if any layer breaks, exactly one test in the chain fires first, pointing at the right layer. That's the diagnostic discipline the codegen-consumes-color commit needs.

Two minor observations on the accessors

These are defensive notes, not must-fix:

1. needs_cps_transform(name) -> bool returns false for unknown fns silently. Codegen's call site should add a debug_assert!(self.cps_program.contains_fn(name), ...) to catch typos that would otherwise silently return false and produce wrong codegen. Cheap defense; the silent-false default is the right shape for the accessor itself.

2. cps_color_user_fns() -> Vec<String> allocates a fresh Vec each call. Codegen's iteration loop will call this once per emit_object. Fine for v1; if profiling later shows it's hot, switch to impl Iterator<Item = &str>. Not a blocker.

Foundation status

• 6 commits on the branch (deviation + 4 test-pinning + 1 cps.rs accessor)
• 430 compiler lib tests (+7 net from foundation: 4 colorer + 1 statement-form colorer + 2 cps.rs query tests; the other 3 are existing test modifications)
• No behavior change yet — codegen still emits user fns with declared sigs regardless of color
• CI in progress on a756bd3

Next milestone

The next commit per the roadmap is codegen-consumes-color — the big diff. That's where:

• needs_cps_transform() gets called per fn at emit_object time
• CPS-color user fns get the uniform CPS ABI (extern "C" fn(closure_ptr, args_ptr, args_len) -> *mut NextStep)
• Native callers of CPS callees get the inlined wrapper (the generalised Phase 4b machinery)
• The two test inversions land (per hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2)

I'll do a deeper review when that commit lands. The test infrastructure waiting for it (4 colorer regression tests + 1 colorer pinning test + 5 cps.rs query tests + 2 e2e tests requiring inversion + the Phase 4d MVP test surface) is unusually strong — if the implementing commit has bugs, the diagnostic surface should pin them precisely.

Forward-looking concerns from prior reviews

All 8 still tracked; one new from this round (option A vs option B doc-vs-code drift). No others.

Action for the agent

1. Update Phase 4e deviation entry section 1 + rationale (b) to reflect option B with explicit rationale. Don't ship the implementing commit with the deviation entry still describing option A.
2. (Optional, defensive) Add debug_assert! at needs_cps_transform callsites in codegen to catch typos.

Otherwise, foundation is solid; cps.rs scaffolding is clean. Ready for codegen-consumes-color when the agent is.

[boldfield]

Mid-flight review at a756bd3

Two new commits since the prior mid-flight review at df251fc:

• eae3589 — closes the prior must-fix (hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 enumerates both test inversions).
• a756bd3 — cps.rs design plan + 2 accessors + 5 unit tests.

CI green on both targets at a756bd3 (build + test + cold-checkout). 425 → 430 compiler lib tests (+5 cps.rs).

Prior must-fix — closed

eae3589 updates hard condition #2 to enumerate both test inversions (discard_k_handler_does_not_abort_helper_phase_4e_pending AND statement_form_non_io_perform_inside_handle_compiles_and_runs, with the 42 → 99 flip explicitly called out). Doc-vs-test gap closed cleanly. ✓

One real issue: deviation-entry-vs-code drift, take two

The deviation entry's section 1 still describes Option A (real CPS transform in cps.rs); the new module docs in cps.rs say Option B (inline lowering in codegen.rs) is the leading choice. This is the second deviation-vs-code drift in this PR — first the colorer section (d8a4771), now the cps.rs section.

The pattern is becoming load-bearing: a multi-week single-PR with a comprehensive deviation entry written upfront is going to drift as architectural choices get refined during implementation. Two options:

• (a) Update the deviation entry now. Reframe section 1 to reflect Option B as leading, with Option A's tradeoffs summarised as the considered-and-deferred alternative. This keeps the deviation entry's "snapshot of decisions" property intact at every commit, matching the discipline the Phase 4d entry maintained.
• (b) Defer the rewrite to the implementing commit. The module docs say "a future deviation-entry update at the implementing commit will reflect the option chosen at that time." This is what the colorer section did too — the choice was committed in d8a4771 after the test-pinning commit revealed the actual shape. Acceptable, but means the deviation entry is stale at HEAD; future bisecting agents need to read the module docs to know the real plan.

Either is fine, but pick deliberately. My recommendation: (a) now. The colorer pivot was unplanned (the agent discovered during test-pinning that the colorer was already correct). The cps.rs Option-A-vs-B is a deliberate design choice the agent has clearly already made; deferring the deviation update is just procedural lag.

Architectural concerns about Option B

If Option B (inline codegen lowering) is the leading choice, three issues need to surface in the deviation entry now, not at the implementing commit:

1. The cps.rs accessors land in this commit but their natural home may be ColoredProgram.

needs_cps_transform(name) -> bool and cps_color_user_fns() -> Vec<String> are queries against colored.colors. They do nothing CPS-form-specific. If Option B wins and cps.rs stays a thin pass-through forever, CpsProgram carries no metadata that justifies its wrapper status. The accessors would equally well live on ColoredProgram.

This isn't a blocker — keeping them on CpsProgram matches the staged-pipeline convention (each pass produces its own typed output even when most are pass-through). But the module docs should call out: "CpsProgram exists as a typed pipeline checkpoint; if Option B ships, future Phase 4e commits may add CPS-form metadata fields here, justifying the wrapper retroactively." Without that note, a future reader will reasonably ask why CpsProgram exists at all.

2. Option B's "side-table extension for synthetic continuation closure records" is understated.

The module docs frame this as a small con of Option B. It's not small. Phase 4d MVP's arm_body_phase_4c_violations (now arm_body_unsupported_construct) explicitly rejects nested Lambda / ClosureRecord in arm bodies precisely because closure_convert ran before codegen. If Phase 4e lifts non-tail k via lambda-lifting at codegen-pass time, the lift produces synthetic continuation closures that closure_convert never saw. Sharing the existing alloc_arm_closure_record machinery is the easy part; threading the synthetic-fn declarations through the codegen FuncId allocation pre-pass is the harder part (per the Phase 4d MVP's HandlerArmSynth precedent — that took a non-trivial pre-pass + side-table to wire up).

Add this as an explicit sub-task under the codegen-consumes-color or non-tail-k-lift commits in the deviation entry's commit roadmap. Otherwise it surfaces as scope creep during implementation.

3. The accessor's behaviour for unknown names is undocumented in the consumer contract.

needs_cps_transform("unknown_fn") -> false is reasonable but means a typo in the consumer (codegen-consumes-color commit) silently treats an unknown fn as Native. The doc comment says "the codegen entry walker is the source of truth for which fns reach codegen; this accessor is a query, not an assertion." OK, but the consumer needs to be assertion-disciplined elsewhere — e.g., the codegen loop iterates cps_color_user_fns() (which IS the source of truth for "this fn needs CPS treatment"), and the per-fn lookup never queries by name. If the consumer follows that pattern, fine. If it ever queries needs_cps_transform(some_name_from_an_AST_walk), the false-for-unknown can mask an unhandled case.

Worth a single-line comment in the module docs: "Consumers should iterate cps_color_user_fns() and never query needs_cps_transform with a name from an AST walk" (or whatever the actual contract is). This is the kind of docstring-on-an-API-boundary that pays off when the codegen-consumes-color commit's reviewer is reading the module cold.

Minor: docstring claim is inaccurate

cps_color_user_fns doc says:

"the underlying colors Vec is BTreeMap-derived and program-order-stable, so this accessor preserves that."

colors is NOT BTreeMap-derived — it's built by iterating mono.anf.checked.program.items in program order (see color.rs:88-91). The docstring conflates two distinct ordering invariants (BTreeMap = alphabetical key order; program-order = source declaration order). For Plan A1 reproducibility, program-order-stable is the actual property; BTreeMap-stability would be a different (and weaker — alphabetical only) property.

Fix: drop the "BTreeMap-derived" phrase. Just say "the underlying colors Vec preserves program declaration order (color.rs::infer_colors iterates mono.anf.checked.program.items), so this accessor preserves that."

Test coverage observation

The 5 cps.rs unit tests cover: pass-through, native main, unknown fn returns false, statement-form classification, ordering on a 3-fn program. Solid baseline. One gap worth a single additional test:

• Multi-level SCC bridge: a → b → c, where c is intrinsically CPS, and verify cps_color_user_fns() lists all three in program order. Pins the transitive-closure invariant for ordering, which is load-bearing if the codegen-consumes-color commit relies on the order. (The current 2-level test (helper, main) doesn't exercise transitive bridge classification through more than one hop.)

Forward-looking concerns from prior review

All 8 still tracked, no new ones. The strongest signal from this round is that the cps.rs design plan is comprehensive and the accessors are the right shape for Option B — the implementation has been thought through more than the deviation entry currently reflects. Sync the deviation entry with that thinking before the implementing commit, not after.

Action items

Must-fix before implementing commit lands:

1. Decide and document: deviation entry section 1 is updated now (Option B reframe) OR explicitly accept that the deviation entry is stale at HEAD until the implementing commit. Pick one; don't leave it ambiguous.
2. Fix the BTreeMap-derived docstring claim — single-word edit.

Should-fix before implementing commit lands:
3. Add Option B's closure_convert side-table extension as an explicit sub-task in the deviation entry's commit roadmap.
4. Document the needs_cps_transform consumer contract (iterate cps_color_user_fns(); don't query by AST-walk name).

Nice-to-have:
5. Multi-level SCC bridge test in cps.rs to pin the transitive-classification ordering invariant.

Foundation continues to be in good shape. The implementing commits have unusually strong test infrastructure waiting for them — the colorer is pinned with 4 regression tests, the accessors are pinned with 5 tests, and the e2e inversions are enumerated with concrete assertions. When the codegen-consumes-color commit lands, the diff will be reviewable against that infrastructure rather than against blind expectations.

[boldfield]

Mid-flight review at 06c3459

One commit since prior review at a756bd3, addressing the Option B must-fix plus 4 additional items the agent self-discovered or improved beyond my suggestions. CI all 4 lanes SUCCESS; 430 → 431 compiler lib tests.

Verification of prior must-fix

Item

Status

Option B reframe in deviation entry section 1

✅ Closed. Section 1 now describes Option B as the chosen approach; Option A summarised as considered-and-deferred alternative. Cross-references cps.rs module docs and a756bd3's commit message. The structural cost of Option B (closure-convert side-table extension for synthetic continuation closures) is called out explicitly with forward-pointer to the codegen-consumes-color and non-tail-k roadmap entries

Doc-vs-code drift resolved. The deviation entry is now internally consistent across foundation commits.

Self-discoveries worth flagging (agent went beyond what I asked)

Real subtle bug catch — cps_color_user_fns docstring conflated two distinct ordering invariants. Original docstring claimed "BTreeMap-derived and program-order-stable." The agent identified that BTreeMap = alphabetical key order (weaker, would silently regress reproducibility) ≠ program declaration order (stronger, what Plan A1's reproducibility test actually depends on). Fixed by replacing the misleading language with: "the underlying colors Vec preserves program declaration order (color.rs::infer_colors iterates mono.anf.checked.program.items) ... A future refactor that silently swaps to a BTreeMap-derived order (alphabetical-only) would change reproducibility-relevant byte sequences without the colorer's tests catching it."

This is exactly the cross-cutting structural-correctness category that user_brian.md memory flags as hard to catch. The doc would have looked correct to most reviewers; the bug would have surfaced as a reproducibility failure months later in Stage 9. Caught entirely in the agent's own re-read during the review-fixup pass.

Better pattern than my "minor observation" suggestion — Consumer contract documented instead of debug_assert at callsites. I suggested adding debug_assert!(cps_program.contains_fn(name), ...) at every codegen callsite of needs_cps_transform. The agent's resolution is structurally better: document the consumer contract on the accessor itself ("consumers driven by per-fn ABI selection should iterate cps_color_user_fns() directly rather than querying needs_cps_transform with a name harvested from an AST walk"). The codegen-consumes-color commit follows the iterate-the-list pattern. Pattern-level fix beats per-callsite defense.

Self-identified architectural concern — closure-convert side-table extension as explicit roadmap entry. The agent recognised that Option B's "side-table extension for synthetic continuation closure records" was understated in the deviation entry. Phase 4d MVP's walker explicitly rejects nested Lambda/ClosureRecord in arm bodies precisely because closure_convert runs before codegen; Phase 4e's lambda-lifted continuation closures bypass that pipeline. Added a new explicit roadmap entry between codegen-consumes-color and non-tail-k, dedicated to the side-table extension. Generalises the existing HandlerArmSynth precedent. This catches scope creep before it surfaces during implementation.

Self-identified architectural concern — CpsProgram retroactive-justification note. The agent noted that CpsProgram is a thin wrapper today (the accessors could live on ColoredProgram). Resolution: module docs explicitly acknowledge both outcomes — wrapper preserved per staged-pipeline convention; future Phase 4e commits may add CPS-form metadata that justifies it, OR a future cleanup commit could fold accessors into ColoredProgram and drop CpsProgram entirely. Doesn't make the call yet; preserves optionality without committing to either direction. Right discipline for the foundation phase.

Nice-to-have — multi-level SCC bridge test. Added cps_color_user_fns_pins_multi_level_scc_bridge_ordering: 3-hop chain a → b → c where c is intrinsic CPS. Existing 2-fn test was single-hop; multi-hop pins the ordering invariant the codegen-consumes-color commit will rely on.

Pattern observation — agent's self-discovery is a force multiplier

This is now consistent across Stage 6 PRs: the agent's review-fixup commits routinely catch issues the reviewers (me + no-context) miss. Phase 4e foundation alone has produced:

• WIP review found 1 must-fix; agent added 0 extra
• Mid-flight Plan A1 code-review fixes: 6 issues from the post-review audit #1 found 1 must-fix; agent added 1 extra (statement_form test inversion discovery)
• Mid-flight Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 found 1 must-fix + 2 minor; agent addressed all, used a better pattern for one, and added 2 self-discovered architectural concerns + 1 subtle docstring bug

Pre-allocate review effort accordingly: the agent is reliably finding what reviewers miss when given the chance during fix-up rounds. Not a reason to do less reviewing; a reason to expect the diff at each fixup to address more than what was strictly flagged.

Foundation status

• 7 commits on the branch (deviation entry + 4 test-pinning + 2 cps.rs scaffolding/cleanup)
• 431 compiler lib tests
• No behavior change yet — codegen still emits user fns with declared sigs regardless of color
• All 4 CI lanes SUCCESS on 06c3459

Next milestone — still codegen-consumes-color

Same as prior review. Foundation is fully in place; test infrastructure is comprehensive (4 colorer regression tests + 1 colorer pinning test + 6 cps.rs tests including the new multi-level SCC test + 2 e2e tests requiring inversion + Phase 4d MVP test surface). The codegen-consumes-color commit will land into unusually strong diagnostic coverage.

Forward-looking concerns

All 8 from prior reviews still tracked. No new ones from this round. The 4 self-discovered items above don't change the forward-looking concerns — they tighten the foundation that the implementing commits will build on.

No action required from the agent for this review round. Foundation is solid; ready for codegen-consumes-color.

[boldfield]

Mid-flight review at 06c3459

One new commit since the prior mid-flight at a756bd3. Bundles all 5 action items from my prior review. CI green on all 4 lanes.

Action items — all closed

Item	Status
MF1 Decide and document deviation entry section 1	✅ Option (a) chosen — section 1 reframed for Option B with explicit rationale (Phase 4d MVP machinery reuse, ~1500 LOC saved vs Option A) and cross-reference to the new closure-convert side-table roadmap entry. The reframe explicitly cites a756bd3's commit message and cps.rs's module docs as the authoritative sources, so the bisecting agent has a paper trail.
MF2 Fix BTreeMap-derived docstring	✅ Replaced with "preserves program declaration order (\color.rs::infer_colors` iterates `mono.anf.checked.program.items`)"`. Doc now also notes that source-declaration-order is stronger than alphabetical and would be silently broken by a BTreeMap-derived refactor — turning the doc fix into a regression-guard rationale.
SF1 Closure-convert side-table extension as explicit roadmap sub-task	✅ Broken out as its own roadmap entry between Codegen consumes color and the colorer-pinning entry. Explanation explicitly traces back to Phase 4d MVP's arm_body_unsupported_construct walker rejection and explains why synthetic continuation closures bypass that pipeline. Generalises the existing HandlerArmSynth pre-pass + FuncId allocation pattern.
SF2 Document needs_cps_transform consumer contract	✅ New "Consumer contract" paragraph in cps_color_user_fns's doc — explicit guidance to iterate the list rather than query by AST-walk name, with the typo-classifies-as-Native failure mode named.
NTH Multi-level SCC bridge test	✅ cps_color_user_fns_pins_multi_level_scc_bridge_ordering — three-hop chain a → b → c with c intrinsically CPS, asserts [c, b, a] in source declaration order. Pins the transitive-classification-preserves-order invariant.

Doc fixes are above-bar — each one carries a regression-guard rationale rather than just a literal correction. The deviation entry's section 1 reframe is particularly substantive (one large paragraph fully rewriting the architectural choice with the ~1500 LOC tradeoff explicit).

Two small things to flag

1. Synthetic continuation lambdas vs. the walker's nested-Lambda rejection.

The new closure-convert side-table roadmap entry says: "The walker's nested-Lambda/ClosureRecord rejection in arm bodies stays in place at this commit (lifts at the surrounding-lambda-captures commit later in this PR)."

Clear about user-level lambdas. But Phase 4e also synthesises its own lambda-shaped continuations at codegen-pass time. Those are NOT user-level Lambdas in the AST and so don't pass through arm_body_unsupported_construct at all — they're generated post-walker, post-closure-convert. Worth one explicit sentence in this roadmap entry confirming the synthetic continuations bypass the walker entirely (so a future reader doesn't think the roadmap entry is contradicting itself when synthetic continuations land alongside the walker rejection still being in place).

2. The 4-test colorer pinning surface + the 6-test cps.rs surface together cover the foundation cleanly. One gap that may matter at the codegen-consumes-color commit:

Currently no test pins what cps_color_user_fns() returns when an SCC contains multiple members all reachable from a CPS leaf (e.g., mutual recursion a ↔ b → c, with c intrinsic CPS). The 3-fn linear-chain test catches transitive classification through forward edges; SCC-collapse + sort behavior is a separate code path the codegen-consumes-color commit may rely on for ABI uniformity within an SCC. Not a blocker for this commit — flag for the implementing commit's review checklist.

Forward-looking concerns from prior reviews

All 8 still tracked. The strongest signal from this round is that the agent's review-fix discipline is working well: each round closes substantively, the doc fixes carry rationale, and the deviation entry is now consistent with the chosen architectural direction. The codegen-consumes-color commit (next big diff) lands against unusually strong infrastructure — colorer pinned with 4 tests, accessors pinned with 6 tests, two e2e inversions enumerated, closure-convert side-table extension pre-registered as its own commit.

What I'm watching for at the next push

The codegen-consumes-color commit is the load-bearing implementation diff. Specifically:

• C-ABI shim for main correctly branches on color (CPS path drives sigil_run_loop; Native path calls sigil_user_main directly).
• Inlined native-of-CPS wrappers don't introduce stack-vs-arena mistakes (the Phase 4b stack-vs-arena closure point closes here).
• discard_k_handler_does_not_abort_helper_phase_4e_pending un-#[ignore]'d AND statement_form_non_io_perform_inside_handle_compiles_and_runs flips 42 → 99 in the same commit (per the enumerated hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2).
• Phase 4d MVP positive tests still pass (the highest-stakes regression surface).

Foundation is solid. Implementing commits next.

[boldfield]

Mid-flight review at 3c65054

Two new commits since prior review at 06c3459. CI still IN_PROGRESS on all 4 lanes — note this is a real architectural change touching multiple modules; wait for CI green before declaring foundation done.

Verification

Commit	Item	Status
5514a67	Closure-convert side-table roadmap entry's "synthetic continuations vs walker rejection" disjointness	✅ Closed. Explicit clarifying sentence appended: user-authored arm-body lambdas (walker rejection target) vs codegen-synthesised continuations (post-walker, post-closure-convert at codegen pre-pass time) are disjoint surfaces. Reviewer-pointed contradiction resolved
5514a67	Multi-member SCC test	✅ Added cps_color_user_fns_pins_mutual_recursion_scc_with_cps_bridge. Three-fn mutual-recursion SCC bridging to intrinsic-CPS leaf; pins per-SCC ABI uniformity invariant. Note: had to fix let _ = c(); → let x: Int = c(); because Sigil parser requires NAME + type annotation (standard pattern, not novel)
3c65054	CpsProgram deletion	✅ Substantively done; awaiting CI confirmation

CpsProgram deletion — the right architectural call

The agent picked the cleanup option from the two outcomes I noted in the prior-prior review ("Should-fix #5 — module docs acknowledge both outcomes: wrapper preserved per staged-pipeline convention OR future cleanup folds accessors into ColoredProgram and drops CpsProgram entirely"). The agent took the latter and the reasoning is sound.

Quoted from the commit message: "Under Option B's commitment, cps.rs carries no CPS-form-specific metadata: the synthetic continuation closure pre-pass + FuncId allocations live in codegen.rs, not in any post-color IR pass. The CpsProgram wrapper is an architectural fiction under Option B — it exists only to match the staged-pipeline convention, not because it transforms data."

That's exactly right. Convention should reflect what's actually being transformed; a thin wrapper that exists to preserve a pipeline shape isn't earning its keep. Better to acknowledge that and simplify than to keep a hedge that misleads future readers.

Pipeline simplification:

Was: lex → parse → resolve → typecheck → elaborate → monomorphize
     → infer_colors → cps::transform → closure_convert → emit_object

Now: lex → parse → resolve → typecheck → elaborate → monomorphize
     → infer_colors → closure_convert → emit_object

One fewer typed-pipeline checkpoint, one fewer module, one fewer architectural fiction. The 6 cps tests relocate to color::tests cleanly (they're queries against the same data, just on a different impl block). The transform_is_typed_pass_through_at_head test is correctly dropped since cps::transform no longer exists.

Files touched (verify scope)

• compiler/src/cps.rs: deleted (334 lines gone)
• compiler/src/color.rs: +212 lines (impl block + relocated tests)
• compiler/src/closure_convert.rs: +19 (CpsProgram → ColoredProgram references)
• compiler/src/pipeline.rs: -2 (drop cps::transform step)
• compiler/src/codegen.rs: 1-line at emit_object entry
• compiler/src/lib.rs: removed pub mod cps
• compiler/tests/e2e.rs: removed cps::transform from stackmap-test helper
• PLAN_B_DEVIATIONS.md: section 1 paragraph documenting the deletion as transitional-artifact correction

Net: -125 lines for the architectural simplification. Not bad.

One flag: CI still in progress

A multi-module pipeline change like this is exactly where CI catches a forgotten consumer. The diff stat looks complete (every consumer I'd expect to need updating is in the file list), but the workspace surface for CpsProgram references is large enough that something subtle could slip past pod-verify and only surface in the full e2e suite.

Wait for green before declaring foundation done. If CI surfaces any broken references, this is the commit that needs the fix-up — don't let a CpsProgram::-related fix get bundled into the codegen-consumes-color implementing commit (that would mix architectural cleanup with the actual feature change and make bisection harder).

Pattern observation

I explicitly said "no action required" in the prior review. Agent took action anyway on two items, plus made a real architectural call on a previously-deferred decision. This is the third consecutive review round where the agent has gone substantively beyond what was explicitly flagged.

The behavior is fine — they're choosing between options I enumerated, not making unilateral calls outside the set of considered outcomes — but worth noting as a pattern. The agent has internalized "ship clean" discipline strongly enough that "no action required" gets treated as "no MUST-fix" rather than "stop." For Phase 4e specifically, where the foundation needs to be solid before the codegen-consumes-color commit lands, this conservative-cleanup pattern is appropriate.

Foundation status

• 9 commits on the branch (deviation + 4 test-pinning + 2 cps.rs scaffolding/cleanup + 2 review-fixup including CpsProgram deletion)
• 431 compiler lib tests (was 432; net -1 from dropping the obsolete pass-through test)
• No behavior change yet — codegen still emits user fns with declared sigs regardless of color
• CI in progress on 3c65054

Next milestone — still codegen-consumes-color

Same as prior. Foundation is now structurally cleaner than before (no architectural hedges; ColoredProgram is the unified IR for color queries). The codegen-consumes-color commit will land into:

• 4 colorer regression tests
• 1 colorer pinning test (statement_form e2e source)
• 7 cps/color tests (now in color::tests after the relocation)
• 2 e2e tests requiring inversion (discard_k_handler_* + statement_form_non_io_perform_*)
• Phase 4d MVP test surface

Diagnostic coverage is unusually strong for a single implementing commit.

Forward-looking concerns

All 8 still tracked. No new ones from this round. The CpsProgram deletion doesn't change the forward-looking concerns — it just simplifies the pipeline they apply to.

Foundation is solid pending CI confirmation. Ready for codegen-consumes-color.

[boldfield]

Mid-flight review at 3c65054

Two new commits since the prior mid-flight at 06c3459:

• 5514a67 — closes both leftover concerns from prior review (synthetic-continuations bypass clarification + mutual-recursion-SCC test). CI green on all 4 lanes.
• 3c65054 — deletes CpsProgram entirely, moves accessors to ColoredProgram. CI in-progress on all 4 lanes.

Prior leftover concerns — closed

Item	Status
Synthetic-continuations bypass the walker (clarification in roadmap entry)	✅ 5514a67 adds an explicit sentence: synthetic continuations are "generated post-walker, post-closure-convert at codegen-pass time as HandlerArmSynth-style pre-pass output"; walker rejection inspects user-level AST nodes, not codegen-synthesised continuations. The two surfaces are disjoint. Cleanly stated.
Multi-member SCC ordering test (mutual recursion)	✅ cps_color_user_fns_pins_mutual_recursion_scc_with_cps_bridge — a ↔ b → c, where a and b mutually recurse and c is intrinsic CPS. Asserts source-declaration order [c, a, b]. CI green on 5514a67 confirms this passes through the full pipeline (so mutual recursion is supported post-Task-48; the test note's hedge about "typecheck currently rejects mutual recursion" is stale prose — see minor #2 below).

CpsProgram deletion (3c65054) — substantive architectural change

The agent went further than my prior recommendation. I had said: "Worth a single-line comment in the module docs: 'CpsProgram exists as a typed pipeline checkpoint; if Option B ships, future Phase 4e commits may add CPS-form metadata fields here, justifying the wrapper retroactively.'" That was a hedging suggestion. The agent chose the stronger position: under Option B's commitment, CpsProgram carries no CPS-form-specific metadata, so it's an architectural fiction — delete it.

This is the right call. The wrapper was justified only by a "convention" rationale, not by data it carries. Pipeline goes from 9 stages to 8. The accessors live where the data lives (ColoredProgram).

Mechanical execution is clean:

• closure_convert.rs: pub cps: CpsProgram → pub colored: ColoredProgram; convert(cps) → convert(colored). Internal cps.colored.mono.* paths shorten by one hop.
• pipeline.rs: drops the cps::transform(colored) step; convert takes colored directly.
• codegen.rs: one-line cc.cps.colored.mono → cc.colored.mono.
• cps.rs: deleted.
• lib.rs: pub mod cps; removed.
• 6 cps tests relocated to color::tests with cps_from_src → color_from_src. Pass-through-test dropped (no longer applicable). Net 432 → 431 lib tests (the dropped pass-through-test is the −1).
• Deviation entry section 1 gains a third paragraph documenting the deletion as a transitional-artifact correction; commit-organisation roadmap updated. Escape hatch documented: "If a future Phase 4e commit discovers a need for post-color CPS-form metadata after all, it lives directly on ColoredProgram (or a fresh wrapper introduced at the time, with the metadata it actually carries)."
• Consumer contract docstring relocated verbatim with Self::cps_color_user_fns self-reference updated.

Meta-observation: architectural reversal pattern

Three commits in this PR have shifted on the wrapper question:

• a756bd3 — keep wrapper, defended in commit msg as "staged-pipeline convention."
• 06c3459 — keep wrapper but add hedge ("could fold into ColoredProgram if no metadata accrues").
• 3c65054 — delete wrapper.

Each step is individually justified. But the hedge-then-flip pattern is two commits where one would do: when I flagged the wrapper-vs-direct concern at a756bd3's review, the cleaner response was either (a) defend the original choice with the data it carries (i.e., commit to keeping it because it WILL carry CPS-form metadata), or (b) delete then. The hedge in 06c3459 was a third option that bought time without making a decision — and the next round had to make the decision anyway.

Not a blocker for this PR. Forward-looking: when an architectural concern is raised mid-flight, prefer to commit to the answer in the next commit rather than landing a hedge. Reviews that hedge tend to predict reviews that flip.

Two small things to flag

1. Deviation entry section 1 is becoming hard to read. Three architectural-revision paragraphs now stack: (i) Option A → Option B, (ii) Option B's structural cost (closure-convert side-table extension), (iii) CpsProgram deletion. All justified individually; together, they read like a changelog rather than a current-state description.

If a fourth revision lands during the implementing commits, consider restructuring: short summary at top ("Phase 4e ships Option B inline lowering; no CpsProgram, no Expr::CpsTailCall/etc."), with a "Revision history" subsection at the bottom for the architectural decisions made during this PR's lifecycle. Bisecting agents would still find what they need; readers absorbing the deviation cold would see the punchline first.

Not blocking. But a sign the deviation entry is accumulating procedural notes that could be moved.

2. The mutual-recursion test note has stale prose. The new cps_color_user_fns_pins_mutual_recursion_scc_with_cps_bridge test contains:

"Note: typecheck currently rejects mutual recursion at function granularity (forward-reference handling is via fn_schemes pre-pass, but it doesn't rebuild SCCs). Phase B Task 48's pre-pass scheme-seeding closes this hole — both a and b's signatures are visible to each other's body checking. Verified passing on the synthetic colorer test infrastructure where the same mutual-recursion pattern appears."

CI green on 5514a67 confirms the test passes through the full pipeline (lex → parse → resolve → typecheck → ...) — meaning mutual recursion does typecheck today via Task 48's pre-pass. The "currently rejects" framing reads as if there's an active limitation, but there isn't. Cheap doc fix: drop "currently rejects" and just state what works (e.g., "Mutual recursion is typechecked via Task 48's fn_schemes pre-pass; both signatures are visible to each other's body checking. The colorer rebuilds SCCs from the call graph and assigns one color per SCC, so this test exercises the SCC-collapse path.").

Pre-merge gates

• CI green on 3c65054 (currently in-progress). The deletion is mechanically substantive (8 files touched, 388 LOC removed); needs the green to validate the relocation is clean across both targets.
• The 6 relocated tests in color::tests should match their prior cps::tests semantics (commit message claims pod-verify clean for these; CI is the cross-platform verification).

Forward-looking concerns from prior reviews

All 8 still tracked, no new ones. Foundation continues to harden cleanly. The codegen-consumes-color commit (the load-bearing implementation diff) lands against:

• 4 colorer regression tests (handle-discharge classification, including 1 e2e-source-matching)
• 7 accessor tests on ColoredProgram (native main, unknown-fn, statement-form, single-hop, multi-hop, mutual-recursion-SCC, plus the existing classification tests)
• Closure-convert side-table extension pre-registered as its own commit
• Two e2e inversions enumerated with concrete assertions
• Pipeline simplified by one stage
• CpsProgram removed as a transitional artifact

Foundation is in unusually good shape. Implementing commits next.

[boldfield]

Correction to my prior reviews — credit reattribution

Two recent comments of mine (at 06c3459 and at 3c65054) credited the agent with "self-discovery" of items that were actually explicit asks from the no-context reviewer's parallel mid-flight at 06c3459. I missed that review entirely — both reviewers post under the same GitHub account, and I didn't notice the timestamp-adjacent duplicate.

Reattribution:

Item	Originally credited to	Actually from
BTreeMap-vs-program-order docstring bug	"agent self-discovery during re-read"	No-context reviewer MF2
Consumer contract pattern (vs debug_assert at callsites)	"agent's better pattern than my suggestion"	No-context reviewer SF2
Closure-convert side-table as explicit roadmap entry	"agent self-identified architectural concern"	No-context reviewer SF1
CpsProgram retroactive-justification → eventual deletion	"agent self-identified architectural concern"	No-context reviewer Architectural Concern #1
Multi-member SCC bridge test	"agent nice-to-have addition"	No-context reviewer "small thing to flag" #2
Synthetic-continuations bypass clarification	(in 3c65054 review, attributed to agent extension)	No-context reviewer "small thing to flag" #1

The agent's actual contribution remains substantive — they addressed each item with high-quality resolutions, picked between options thoughtfully (CpsProgram deletion was the stronger of two enumerated outcomes), and shipped with consistent rationale. But "self-discovery" overcredits the discovery axis. The no-context reviewer's contribution to this PR's foundation has been load-bearing and I underweighted it.

The "agent self-discovery as force multiplier" pattern observation I've been amplifying across recent Stage 6 PRs may also need revisiting — some of those credits in PRs #21–#25 may have similar attribution issues I didn't catch at the time. Worth re-checking those review threads if it matters for the cumulative project narrative; for the live work I'll be more careful about checking parallel reviews going forward.

Substance of the technical reviews stands — the items I agreed with are still real items, and the agent's resolutions are still solid. Only the credit allocation was wrong.

[boldfield]

Mid-flight review at a2840b6

The implementing work has started — three commits since the prior reviews at 3c65054 (mine + the no-context reviewer's). All three are agent-initiated implementing work, not responses to the no-context reviewer's review. CI in progress on all 4 lanes; +511 LOC in codegen.rs alone; 431 → 441 compiler lib tests (+10).

Three new implementing commits

Commit	What	Status
b1244b4	Pure refactor: extract cps_signature(pointer_ty, &module) helper. Centralizes CPS calling convention construction so future call sites (user-fn pre-pass, synthesized continuation closures) are one-line. No behavior change	clean
76c17ae	Body-shape classifier is_simple_tail_perform_body(body). Strict definition: empty stmts + non-IO tail Expr::Perform. Conservative-gate philosophy (false negatives OK; false positives would cause incomplete CPS-form codegen). +6 unit tests. #[allow(dead_code)] transitional pending consumer	clean
a2840b6	Per-fn ABI selection: enum UserFnAbi { Native, Cps } + compute_user_fn_abi(name, body, colored) + UserFnEntry.abi field. Selector returns Cps iff colorer marks fn CPS AND body matches is_simple_tail_perform_body. +4 tests. Field still #[allow(dead_code)] pending signature-selection commit	clean

Verification of the implementing work

Classifier soundness: Strict definition is correct — the alternative shapes (stmt before tail, IO tail perform, non-perform tail, empty body) all need either lambda-lifting (which Phase 4d MVP machinery doesn't yet handle) or different lowering paths (existing native synchronous-run_loop for IO; native value-return for pure tails). Test matrix covers all five negative cases plus two positive shapes (with/without args). The "args don't matter for classification" choice is defensible — the perform site's args-buffer packing (Phase 4b) is independent of whether the fn is CPS.

ABI selector soundness: The conservative gate (CPS-color AND simple-tail-perform) is correct for Phase 4e MVP. Test 3 specifically pins the case where main is CPS via SCC bridge but has complex body → Native fallthrough. Test 4 is interesting — synthetic post-mono program with empty row + intrinsic perform; documents that the colorer classifies the fn as CPS based on the perform (not the row). The test note explicitly says "Tests the rule's actual behaviour on a synthetic shape typecheck would reject in real code." Same defensive pattern as Phase 4c's reject-during-codegen tests.

cps_signature helper: Pure refactor centralization. Docstring enumerates current and upcoming usage sites explicitly so a future bisecting agent investigating a calling-convention regression has a single grep target. Good defensive discipline.

Cadence pivot worth noting

The agent's commit messages explicitly cite "the user's direction" about small reviewable commits. The original roadmap implied the codegen-consumes-color work would be a single big commit; the actual decomposition is:

classifier → ABI metadata → signature selection → body lowering → caller wrapper → e2e test

That's at least 6 commits for what was originally one. This is a positive shift — multi-week single-PR doesn't have to mean monster commits, and each layer ships with its own dedicated tests before the consumer commit lands. The #[allow(dead_code)] transitional pattern (Phase 4d MVP precedent: HandlerArmSynth.captures) is the right tool to make this work; each attribute is removed when the next consumer commit lands.

One process flag: the #[allow(dead_code)] chain needs to close. If the consumer commit doesn't materialize within Phase 4e, these attributes become permanent dead code. Track that the chain is still moving — both classifier and UserFnEntry.abi are now dead code at HEAD; the next two commits (signature selection, then body lowering) need to land for both to become live.

Open items from the no-context reviewer's review at 3c65054

The no-context reviewer flagged three items that remain open across these three implementing commits:

1. Architectural reversal pattern observation — three commits shifted on the CpsProgram wrapper question (a756bd3 keep + defend → 06c3459 keep + hedge → 3c65054 delete). Reviewer's takeaway: "when an architectural concern is raised mid-flight, prefer to commit to the answer in the next commit rather than landing a hedge. Reviews that hedge tend to predict reviews that flip." Forward-looking guidance; not a blocker.
2. Deviation entry section 1 is becoming hard to read — three architectural-revision paragraphs now stack (Option A → Option B; Option B's structural cost; CpsProgram deletion). Suggested restructuring: short summary at top, "Revision history" subsection at bottom. If a fourth revision lands during implementing commits, this becomes worth doing. Not blocking yet.
3. cps_color_user_fns_pins_mutual_recursion_scc_with_cps_bridge test note has stale prose — "typecheck currently rejects mutual recursion" framing is wrong since CI green confirms Task 48's pre-pass handles it. Cheap doc fix.

None of these are blockers, but they're real observations. The implementing-commit cadence is moving fast enough that they could easily slip; worth a tracking note.

Pre-merge gates

• CI green on a2840b6 (currently in-progress on all 4 lanes).
• The 10 new tests should all pass on both targets — the test infrastructure has gotten genuinely good (each layer's tests pin its own behavior independently).

Forward-looking concerns from prior reviews

All 8 still tracked. No new ones from this round — the implementing work is matching the architectural choices the foundation locked in. The classifier-and-selector pattern positions the next commit (signature selection) to be a focused diff: consume UserFnEntry.abi to choose between the existing closure-convention signature and cps_signature(pointer_ty, &module).

Cumulative project state

Phase 4e foundation: 9 commits + 3 implementing commits = 12 total on the branch. 441 compiler lib tests (+18 net since branch start). One module deleted (cps.rs); ColoredProgram is now the unified IR for color queries. Codegen.rs is the location of all Phase 4e new behavior, which is consistent with the Option B design choice (inline lowering generalising Phase 4d MVP machinery).

Looking forward: the next implementing commit (signature selection) is the first one with observable codegen change. After that lands, I'll do a deeper review — that's the layer where regressions in existing Phase 4d MVP semantics could first surface.

No action required from the agent for this review round. The classifier + selector layers are sound; implementing cadence is on track.

[boldfield]

Mid-flight review @ a2840b6

Scope: 4 commits since 5514a67 — CpsProgram deletion, cps_signature extraction, body-shape classifier, per-fn ABI selection scaffolding.

Direction is solid. CpsProgram deletion is the right pivot, and the per-fn ABI selection is being built in tight, no-behavior-change slices. Below is the punch list of things to address before the next commit (codegen-consumes-color). Please fix all of them.

---

Must fix

1. Misleading test name + missing coverage of the actual gating shortcut (a2840b6, codegen.rs)

compute_user_fn_abi_native_for_native_color_with_simple_tail_perform_body does not test what its name claims:

• Name says "Native for Native color".
• Assertion is UserFnAbi::Cps.
• Inline comment ("...wait, actually find_non_io_perform_in_block DOES walk and would taint as CPS...") admits the test premise was contradicted mid-write.

Two distinct problems:

a) Name is wrong. Rename to something like compute_user_fn_abi_cps_for_intrinsic_perform_in_empty_row_synthetic — it actually pins that the colorer taints empty-row+intrinsic-perform bodies as CPS, which is a useful property but a different one.

b) The intended property is now untested. "Color gating happens before body-shape gating; Native color short-circuits regardless of body shape" has no test coverage, because every test routes through infer_colors and find_non_io_perform_in_block will always taint a simple-tail-perform body to CPS.

Fix: build the ColoredProgram directly (all fields are pub) and manually set colors = vec![(\"f\".to_string(), Color::Native)] for a body containing a non-IO perform. That actually exercises the gating shortcut. If someone reorders the && in compute_user_fn_abi, this test should fail.

2. Soundness gap: classifier doesn't verify arg purity (76c17ae, codegen.rs)

is_simple_tail_perform_body returns true for perform E.op(any_expr...) regardless of arg shape. The doc handwaves this:

The classifier doesn't inspect args — they may be arbitrary pure expressions that get lowered alongside the perform's own lowering.

But "pure" isn't enforced. Consider fn helper() -> Int ![E] { perform E.op(other_cps_helper()) }: today's classifier says CPS-eligible, but the args computation calls a CPS-color callee — which by Phase 4e's architecture must yield to the trampoline before its result is available to the perform. You cannot synchronously evaluate other_cps_helper() from inside a fn that's already returning *mut NextStep.

This won't bite at HEAD because compute_user_fn_abi is never consumed yet. But it WILL bite the codegen-consumes-color commit the first time tests exercise CPS-callee-in-args. Either:

• Tighten now: extend the classifier to require args to be call-free and perform-free. Rename to is_simple_tail_perform_with_pure_args_body and check.
• Or, explicitly defer: add a TODO + a #[ignore]'d test pinning the failure mode so a future commit doesn't accidentally classify CPS-call-in-args as eligible.

The current "args may be arbitrary pure expressions" doc-only contract is the kind of structural debt that gets paid through bug bisects, not commit messages.

---

Should fix

3. UserFnAbi::Native overloads "Native" (a2840b6)

Color::Native already exists in the same crate. Now UserFnAbi::Native is added with a deliberately orthogonal meaning — a CPS-colored fn whose body shape is unsupported gets UserFnAbi::Native while still being Color::Cps. The doc on compute_user_fn_abi notes this, but the naming will trip readers.

Two options:

• Rename to UserFnAbi::Sync / UserFnAbi::Cps — names the runtime shape (synchronous run_loop driver vs. trampoline yield), not the color. Preferred.
• Keep Native but add a one-line warning in the rustdoc pointing at the Color::Native divergence.

The current name will quietly breed assumptions like "if compute_user_fn_abi says Native, the fn is Native-colored" — which the gating explicitly rejects.

---

Nice-to-have

4. #[allow(dead_code)] on UserFnAbi (a2840b6)

The enum is used: returned by compute_user_fn_abi, stored in UserFnEntry::abi, asserted on in tests. The #[allow(dead_code)] only papers over the fact that no non-test code reads the variants in a match yet. Acceptable as a transitional marker, but add a one-line rustdoc note (# Why dead_code: removed in the next commit when signature declaration branches on this field) so future-you doesn't hunt for what changed.

---

Positive observations (no action needed)

• CpsProgram deletion (3c65054) is the right architectural correction; the wrapper was fiction under Option B. Pipeline drops one stage; accessors live where the data lives; all six pinning tests relocated cleanly.
• cps_signature helper (b1244b4) is pure DRY with accurate forward-reference doc. One call site at HEAD; pre-staged for two more.
• is_simple_tail_perform_body test coverage (76c17ae) is complete: all six branches covered with synthetic AST. The classifier's conservative-by-design choice (false negatives acceptable, false positives unsafe) is correct.
• Commit boundary discipline is paying off. Each commit lands one analysis surface without conflating analysis and codegen — this PR is reviewable in clean chunks despite the comprehensive scope.

---

Please address items 1–4 before the codegen-consumes-color commit. Items 1 and 2 in particular need to land before signature declaration branches on UserFnAbi, since both are correctness-adjacent the moment the field is consumed.

[boldfield]

Mid-flight review at cbe95fc

One new commit since a2840b6. The agent addressed the no-context reviewer's 5-item must-fix list at a2840b6 (their review at id IC_kwDOSJ6Tbs8AAAABAbY9wg, posted 3 minutes after mine). Credit allocation: this commit is responsive work to the no-context reviewer, NOT agent self-discovery — the no-context reviewer's review was the load-bearing diagnosis here.

CI in progress on 3 of 4 lanes (macos-14 cold-checkout SUCCESS).

Verification — all 5 items closed

Item (no-context reviewer's call)	Status
MF1 — Misleading test name + missing Native short-circuit coverage	✅ Renamed compute_user_fn_abi_native_for_native_color_with_simple_tail_perform_body → compute_user_fn_abi_cps_for_intrinsic_perform_in_empty_row_synthetic (now matches what the test actually pins). Added new test compute_user_fn_abi_sync_when_color_native_short_circuits_regardless_of_body_shape that constructs ColoredProgram directly with colors = vec![("f", Color::Native)] to bypass infer_colors. This is the only way to exercise the Native short-circuit since infer_colors always taints body-walks-perform fns CPS. If a future refactor reorders the && in compute_user_fn_abi, this test fires
MF2 — Classifier soundness gap on impure args	✅ Tightened. Renamed is_simple_tail_perform_body → is_simple_tail_perform_with_pure_args_body at codegen.rs:4987. Added expr_is_pure and block_is_pure helpers (codegen.rs:5024+) that recursively check args are call-free, perform-free, lambda-free, closure-record-free, and handle-free. Pure compounds (binary, unary, conditional, match, block, record literal) accepted only when every sub-expression is itself pure. Three new tests: rejection on call-arg, rejection on nested-perform-arg, acceptance on pure-compound-arg (e.g. if true { 1 } else { 2 }). The reviewer's "tighten now, don't defer" call was the right one — would have bitten the codegen-consumes-color commit on first CPS-callee-in-args test
SF3 — UserFnAbi::Native overloads "Native"	✅ Renamed UserFnAbi::Native → UserFnAbi::Sync at codegen.rs:189-192 + all references. Names the runtime calling shape (synchronous run_loop driver) instead of the colorer's classification. Docstring at codegen.rs:112-117 now opens with explicit "Naming note" calling out the divergence from Color::Native
NTH4 — #[allow(dead_code)] rationale	✅ Comment added explaining the transitional pattern (Phase 4d MVP precedent: HandlerArmSynth.captures). Future-self can grep for the rationale instead of hunting for what changed
Open #5 — Mutual-recursion stale prose	✅ Auto-resolved. Agent grepped and confirmed the stale "typecheck currently rejects mutual recursion" framing was already cleaned up during the CpsProgram-deletion test relocation in 3c65054. Good discipline — checked before assuming work was needed

Substantive observations on the fixes

MF2's tightening is the load-bearing one. The classifier now refuses to classify a body as CPS-eligible when args contain calls, performs, lambdas, closure records, or nested handles. Conservative-by-design: false negatives (route through synchronous Phase 4d MVP path) are acceptable; false positives (classify CPS-eligible when args need synchronous evaluation) would have produced incomplete CPS-form codegen. The reviewer correctly identified that this would only have bitten the codegen-consumes-color commit on first CPS-callee-in-args test — meaning the bug would have been a substantive regression caught only at integration time, not at unit-test time.

MF1's new test is structurally important. compute_user_fn_abi_sync_when_color_native_short_circuits_regardless_of_body_shape directly constructs a ColoredProgram with manually-set Native classification, bypassing infer_colors. This is the only way to test the gating short-circuit because infer_colors will always taint the body-walks-perform fn as CPS. The pattern (synthesizing post-pipeline IR for tests that need to test downstream-of-pipeline gating) is reusable — worth noting for future review prompts.

The Sync rename matters more than it sounds. UserFnAbi::Native would have been actively misleading: a Color::Cps fn can have UserFnAbi::Native (when its body shape doesn't match is_simple_tail_perform_with_pure_args_body). Future readers would have assumed the variants tracked color, not runtime shape. Naming the variants by what they actually represent at the codegen layer (synchronous run_loop driver vs. trampoline yield) prevents that confusion.

Pattern observation: parallel-review structure is paying off

Two consecutive review rounds (3c65054 + a2840b6) where the no-context reviewer's substantive review caught items I missed or underweighted:

• 3c65054: hedge-then-flip pattern observation, deviation-entry readability, mutual-recursion stale prose
• a2840b6: misleading test name, classifier soundness gap, UserFnAbi::Native naming overload

The no-context reviewer is consistently catching the kinds of issues I'm structurally weak against — the soundness-gap-in-doc-only-contract category in particular (MF2) is exactly what user_brian.md memory flags as hard to catch from a single LLM perspective. Two parallel reviews is producing better foundation hardening than either alone.

For the codegen-consumes-color commit (next big one), this matters: the foundation now has both reviewers' concerns addressed. The classifier is tightened, the ABI naming is clear, the gating short-circuit is tested against direct-IR construction. The implementing diff lands against unusually strong infrastructure.

Pre-merge gates

• CI green on cbe95fc (3 of 4 lanes IN_PROGRESS).
• #[allow(dead_code)] chain still open: UserFnAbi is used in tests but no production codegen consumes the enum yet. Next commit (signature selection) should remove the attribute and consume the enum to drive per-fn signature declaration.

Forward-looking concerns from prior reviews

All 8 still tracked. No new ones from this round — the must-fix work matched the items raised. Foundation is now structurally sound enough for the implementing diff to land cleanly.

Cumulative project state

• Phase 4e branch: 13 commits total (foundation + 6 implementing-prep + 7 review-fixup/architecture)
• 444 compiler lib tests (+22 net since branch start)
• One module deleted (cps.rs)
• ColoredProgram is the unified IR for color queries
• All Phase 4e-prep abstractions live in codegen.rs (consistent with Option B)
• 5+ items from no-context reviewer addressed; mine all addressed too

Next milestone

The next implementing commit is signature selection — consume UserFnEntry.abi to choose between the existing closure-convention signature and cps_signature(pointer_ty, &module). That's the first commit with observable codegen change. The #[allow(dead_code)] on UserFnAbi should drop with that commit.

After that lands, I'll do a deeper review — that's where existing Phase 4d MVP test surface could regress.

No action required from the agent for this review round. Foundation hardened; cadence on track.

[boldfield]

Mid-flight review at 33f22318

The first observable codegen change in Phase 4e — signature selection + CPS body emission + native↔CPS interop wrapper. Substantively well-designed; the observational-equivalence reasoning under tail-position perform is sound; the dead-code chain is correctly closed (both UserFnAbi's and UserFnEntry::abi's #[allow(dead_code)] removed).

But there's a real compiler-crash bug the agent admits in the commit message but ships anyway. Per your standing directive on this branch, must-fix.

Per-task

Item	Status
Signature selection (user-fn pre-pass loop)	ship after MF1 below
CPS body emission (hand-rolled)	ship after MF1
Native↔CPS interop wrapper (lower_call)	ship after MF1
cps_abi_helper_* e2e test (zero-arity path)	ship as-is
Dead-code chain closure (UserFnAbi, UserFnEntry::abi)	✅ correctly closed

MF1 — compute_user_fn_abi doesn't gate on user-fn arity; downstream assertion crashes

Verified at compiler/src/codegen.rs:178-188:

fn compute_user_fn_abi(
    name: &str,
    body: &crate::ast::Block,
    colored: &ColoredProgram,
) -> UserFnAbi {
    if colored.needs_cps_transform(name) && is_simple_tail_perform_with_pure_args_body(body) {
        UserFnAbi::Cps
    } else {
        UserFnAbi::Sync
    }
}

is_simple_tail_perform_with_pure_args_body only inspects body shape, not user-fn arity. compute_user_fn_abi doesn't even take fn params. But the body emit path at compiler/src/codegen.rs:2615-2618 asserts:

if entry.abi == UserFnAbi::Cps {
    assert!(
        f.params.is_empty(),
        "codegen Phase 4e: CPS-ABI fn `{}` has user params; ..."
    );

That's assert!, not debug_assert! — aborts the compiler in both dev and release on a valid Sigil program.

Reproducible failure mode (based on code inspection):

effect E { op: (Int) -> Int }
fn helper(x: Int) -> Int ![E] { perform E.op(x) }   // user-fn arity 1, simple-tail-perform body
fn main() -> Int ![IO] {
  let n: Int = handle helper(5) with { E.op(arg, k) => 42 };
  perform IO.println(int_to_string(n));
  0
}

Trace:

1. helper typechecks (intrinsic CPS via ![E])
2. compute_user_fn_abi("helper", body, colored):
   • needs_cps_transform("helper") → true (intrinsic CPS)
   • is_simple_tail_perform_with_pure_args_body(body) → true (empty stmts; tail is perform E.op(x) where x is Expr::Ident, which expr_is_pure accepts)
   • returns UserFnAbi::Cps
3. Body emit branch hits assert!(f.params.is_empty()) with params = [x: Int] → compiler aborts

This is the same MF1 category from PR #25 (walker/detector tail-position mismatch): a gate accepts shapes the downstream code asserts against.

The agent admits this in the commit message:

"Helpers with non-zero user-arg arity OR with perform-args fall through to UserFnAbi::Sync either via the classifier ... or — for non-zero-user-arg helpers that pass the classifier — would crash the assertions in the body emit branch + caller wrapper."

The "would crash the assertions" admission means: shipping this commit with the next-slice still pending means the compiler can crash on a class of valid Sigil programs. Even if the next slice fixes it, the window where the regression exists is a window where the compiler is broken. Same scope-boundary failure mode the Stage 6 review pattern has been catching.

Two valid fixes:

1. Add the arity gate to compute_user_fn_abi now. Pass fn params; require params.is_empty() AND tail-perform-args-empty. ~5 lines. Ships in this commit; preserves the architectural slicing (this commit ships zero-arity CPS path; next commit widens both gates simultaneously with their consumer machinery).

2. Land the next slice immediately as part of this commit. Larger change but eliminates the soundness window entirely.

Recommend (1) — keeps the slicing intact, closes the soundness gap with a single conservative gate addition. Don't ship a compiler that can assert!-crash on valid input even briefly.

The classifier itself is correct as-is (body shape is the right thing to check there); the gate in compute_user_fn_abi is what needs widening.

Substantive observations on the rest

Three code paths read sound:

• Signature selection (emit_object user-fn pre-pass loop, :2353-2399): clean match entry.abi branch. Sync gets existing closure-convention; Cps gets cps_signature(pointer_ty, &module). Correct.
• CPS body emission (emit_object user-fn body emit loop, :2585+): hand-rolled body. Block params (closure_ptr, args_ptr, args_len). Loads (k_closure, k_fn) from args_ptr[0], args_ptr[1]. Tail-calls sigil_perform(effect_id, op_id, null, 0, k_closure_loaded, k_fn_loaded). Returns NextStep directly. Same shape as Phase 4d MVP synthetic-arm-fn machinery — correct reuse of established machinery.
• Native↔CPS interop wrapper (Lowerer::lower_call direct-call arm): mirrors Phase 4d MVP's lower_perform_non_io_to_value. 16-byte stack slot for [null, &sigil_continuation_identity]. Calls CPS fn with that slot + args_len=2. Drives sigil_run_loop. Narrows u64 to declared ret_ty via existing narrow discipline.

Observational-equivalence claim verified: Under tail-position perform, both Phase 4d MVP synchronous shape and Phase 4e CPS-ABI shape produce identical observable behavior:

• Phase 4d MVP: native caller → lower_perform_non_io_to_value → sigil_perform(eff, op, args, len, null, identity) → returns Call(arm, [arg, null, identity]) → run_loop dispatches → arm returns Done(42) → run_loop returns 42 → caller receives 42
• Phase 4e (this commit): native caller → lower_call for raise_e() → inlined wrapper → CPS fn body builds sigil_perform(eff, op, null, 0, k_closure_loaded, k_fn_loaded) where loaded args = [null, identity] → returns Call(arm, [null, identity]) → wrapper drives run_loop → arm returns Done(42) → wrapper narrows ret_ty → caller receives 42

Same data flow; different code organisation. The architectural difference matters only when both caller and callee are CPS-ABI (the Phase 4e endgame for discard-k correctness across function-call boundaries) — not at this commit.

The discard-k-pending test inversion correctly waits for later commits. The agent explicitly notes: "At this commit, only the helper has CPS ABI; main stays Sync. The full inversion of the discard_k_handler_does_not_abort_helper_phase_4e_pending test waits for the lambda-lifting machinery (a later commit) that lets main's complex body match a CPS-eligible shape." Correct framing — discard-k correctness needs both sides CPS-ABI so the trampoline's discard-Done short-circuit propagates through call frames.

E2e test coverage

cps_abi_helper_with_simple_tail_perform_called_from_native_main_returns_arm_value (compiler/tests/e2e.rs): minimal program with fn raise_e() -> Int ![E] { perform E.op() } (CPS-ABI) and main calling it inside handle E.op(k) => 42. Asserts stdout 42. Pins:

• Signature selection routes the right fn through CPS-ABI path
• Body emit produces valid Cranelift function with right shape
• Call-site wrapper packs continuation args correctly + drives trampoline
• Ret-type narrow returns right value

Test only exercises the zero-arity path. The non-zero-arity crash path is uncovered (and would crash if exercised). Adding a regression test like cps_abi_helper_with_user_arg_falls_through_to_sync_abi would lock the MF1 fix's invariant — a non-zero-arity intrinsic-CPS fn must compile cleanly via Sync ABI.

Pre-merge gates

• CI green on 33f22318 (currently IN_PROGRESS on all 4 lanes). The hand-rolled CPS body emit is novel; CI is the cross-platform verification.
• MF1 closes by gate addition + regression test.

Forward-looking concerns from prior reviews

All 8 still tracked. No new ones beyond MF1. The implementing work is matching the architectural choices the foundation locked in.

Action for the agent

MF1: gate compute_user_fn_abi on user-fn arity (and tail-perform-args arity) so non-zero-arity intrinsic-CPS fns fall through to UserFnAbi::Sync cleanly. Add a regression test (e.g., cps_abi_helper_with_user_arg_falls_through_to_sync_abi) that compiles a non-zero-arity intrinsic-CPS helper and asserts it doesn't crash. Alternatively land the next slice in this commit so the gate widens with its consumer machinery — but the conservative gate addition is the cheaper path.

Otherwise the implementing work is solid. Architectural reasoning is sound; mechanical execution is clean; the observational-equivalence claim holds.

[boldfield]

Mid-flight review at 33f2231

One commit since cbe95fc. CPS-ABI signature selection + CPS body emit + native↔CPS interop wrapper at call sites land in a single ~370-line slice. New e2e test passes; lib tests clean. The two e2e perf-ceiling failures (fib_perf_example, tree_example) are environment-bound (~2s vs 50ms/500ms ceilings) and don't touch effects/perform — not commit-bound.

Direction is solid. Split into (signature select) → (body emit) → (call-site wrapper) is the right minimum unit. Assertions guard arity-0 so the next slice can widen both sides together. The new test pins the architectural change against the existing Phase 4d MVP path.

Please address all four items below before the next slice merges. D1 is must-fix before any further commits on this branch (it's a latent codegen panic on plausible user code).

---

D1 (must-fix) — compute_user_fn_abi doesn't check user-fn arity; can panic codegen

is_simple_tail_perform_with_pure_args_body (codegen.rs:5264) only inspects body.tail. It accepts a body regardless of the surrounding fn's param list. Combined with the colorer's row-tainting, a fn like:

fn raise_e(x: Int) -> Int ![E] { perform E.op() }

(arity-1 fn, body's perform is arity-0) gets UserFnAbi::Cps, then panics at the body-emit assert (codegen.rs:2620). No extant test triggers this, but it's plausible user-written code that previously compiled (Sync path) and now unreachable!s the compiler.

Fix: add f.params.is_empty() to the compute_user_fn_abi predicate. One-line change. Removes the body-emit assertion's only failure mode and lets the classifier soundness comment ("false positives are not [acceptable]") stay literally true.

D2 (must-fix) — args_len value passed by the wrapper isn't either established convention

Existing perform-site convention (codegen.rs:3572): args_len = user arg count (0 if no user args, else N). Runtime treats args_ptr=null ⟹ args_len=0 as the safety contract.

The new CPS-callee wrapper passes args_len = 2 (codegen.rs:4118), where 2 = the trailing-k slot count. That's neither user-arg count (0 here) nor a documented "slot count" convention. Body emit ignores it (block_params[2] unused, codegen.rs:2649), so no observable bug today — but the inconsistency will ambush the next slice.

Fix: pick one and document on cps_signature. Strong preference for (a):

• (a) args_len = user-arg count. Wrapper sends 0 here. Body emit can assert!(args_len == 0) to strengthen the arity-0 invariant. When user-arg unpacking lands, args_len is the loop bound and matches the perform-site precedent.
• (b) args_len = total trailing-payload slot count. Wrapper sends 2 here; body emit decodes user_arg_count = args_len - 2. Less natural; diverges from the perform-site precedent.

S1 (should-fix) — magic offsets 0/8 coupled across two sites

Wrapper writes (k_closure, k_fn) at offsets 0/8 (codegen.rs:4109–4110); body emit loads at 0/8 (codegen.rs:2671–2680). The next slice will shift these by N*8 (N = user arg count). Extract K_CLOSURE_OFFSET / K_FN_OFFSET constants alongside cps_signature so the slices can't drift.

S3 (should-fix) — test coverage thin

One test pins the happy-path shape. Three more would harden against regressions in the wrapper's narrow / addressing logic — cheap to add now, expensive to add after a bisect:

• Callee invoked twice in the same caller — pins per-callsite stack slot (not accidentally shared/aliased).
• Callee with Bool return — exercises the ireduce narrow path (codegen.rs:4144).
• Callee with pointer return (e.g. String) — exercises the debug_assert_eq!(ret_ty, pointer_ty) path (codegen.rs:4148).

---

Style note (S2 from the local review, not blocking)

The CPS body branch's restriction enumeration (codegen.rs:2585–2614) and the wrapper's "first slice supports arity-0" block (codegen.rs:4070–4084) read like commit-ordering scratch notes; they become stale when the next slice removes the asserts. Acceptable as transitional. Trim once that slice closes. The load-bearing finalize()+continue borrow-checker note and the args_ptr lifetime discipline both stay.

Non-issues (verified)

• stackmap.push_placeholder on the CPS body's perform call (codegen.rs:2715) — defensive; CPS callee frame has no GC roots in this slice but placeholder is correct policy.
• Stack slot lifetime — slot is in caller's frame, alive across the nested sigil_run_loop. Sound.
• module.declare_data_in_func for div_zero/mod_zero GVs goes unused for CPS fns — pre-existing pattern, Cranelift drops dead refs.
• finalize() + continue ordering in the CPS body branch — correct; the inline comment is justified.

---

Summary action list (please address all)

#	Severity	Item
D1	must-fix	Add arity-0 check to compute_user_fn_abi
D2	must-fix	Pick args_len convention (prefer (a)), document on cps_signature, fix wrapper
S1	should-fix	Extract K_CLOSURE_OFFSET / K_FN_OFFSET constants
S3	should-fix	Add three coverage tests (multi-call, Bool ret, pointer ret)

[boldfield]

Mid-flight review at 5a0459a

The agent addressed all 4 items from the parallel reviews at 33f2231 (mine + the no-context reviewer's). Substantive fixes verified by inspection.

But CI is RED on all 4 lanes for this commit. The agent's commit message claims "lib tests pass locally. Pod-verify clean." Locally I confirm: 447 compiler lib tests pass, all 4 new cps_abi tests pass, only the two pre-existing perf-flake tests fail (fib_perf_example_prints_6765_under_50ms at 3.2s vs 50ms ceiling; tree_example_prints_32767_under_500ms at 2.5s vs 500ms ceiling — environment-bound, well-documented pattern).

The CI status disconnect needs investigation before merge. Possible causes:

1. Perf flakes hitting all 4 lanes simultaneously — possible but unusual; cold-checkout lanes don't typically run the perf-floor e2e suite. If true, this is the same pattern flagged in PRs [Tasks 51 + 52] Stage 5 closeout: generic_map example + P16/P17 prompts #18, [Task 55] Phase 4d MVP: closure-capture + tail-position k via identity continuation #25's CI fixup notes, and the no-context reviewer's review at 33f2231.
2. Real test failure introduced by the fix-up commit — possible. Lib tests + cps_abi e2e tests pass locally, but CI may surface a different test (smoke, reproducibility, or a non-cps test the fix-up accidentally broke).
3. CI infrastructure issue — gh run view --log-failed and the API job-logs endpoint both return errors for this run; logs may be unavailable for an environmental reason rather than a test-failure reason.

Action for the agent: investigate the CI failure before any further commits land on this branch. If perf-flake, push an empty commit or rerun-CI marker to confirm. If a real failure, surface and address it. Don't let a RED CI status sit while the next slice piles on top.

Verification of the 4 review items

Item	Status
D1 — compute_user_fn_abi arity gate	✅ Extended signature to take params: &[Param]. Added two arity gates: params.is_empty() AND body.tail.perform.args.is_empty(). Body emit's assert!s downgraded to debug_assert!s (correctly — they're now defense-in-depth, not user-facing). Two regression tests pin the gates: compute_user_fn_abi_sync_for_arity_n_intrinsic_cps_helper (arity-1 helper falls through) and compute_user_fn_abi_sync_for_arity_0_helper_with_perform_args (arity-1 perform falls through). 444 → 446 lib tests
D2 — args_len convention	✅ Wrapper changed to args_len = user_arg_count (= 0 in this slice). Documented on cps_signature via _cps_args_len_convention_doc placeholder. Matches the perform-site precedent from Phase 4b (lower_perform_non_io_to_value). Future arity-N slice uses args_len as the user-arg unpacking loop bound — consistent with both sides
S1 — magic offset constants	✅ Extracted k_closure_offset(user_arg_count) and k_fn_offset(user_arg_count) helper fns next to cps_signature. Both wrapper and body emit use the helpers. Future arity-N slice updates one caller (passing real user_arg_count instead of 0) and both sides stay aligned automatically
S3 — test coverage	✅ Three new e2e tests verified locally: cps_abi_helper_called_twice_from_one_caller_uses_independent_stack_slots, cps_abi_helper_with_bool_return_exercises_ireduce_narrow, cps_abi_helper_with_string_return_exercises_pointer_ret_path. Each pins a specific arm of the wrapper logic that a future refactor could regress

Substantive observations

D1 fix is exactly the conservative-gate pattern I recommended. Compute_user_fn_abi gets the arity gate; classifier stays body-shape-only (correct division of concerns); body emit asserts downgrade to debug_assert (defense-in-depth contract checks, not user-facing). The gate widens in the next commit alongside its consumer machinery — same pattern as the dead-code attribute chain.

D2 fix picks the right convention. Option (a) — args_len = user arg count — matches Phase 4b's perform-site precedent and avoids the slot-count-vs-arg-count confusion the next slice would have hit. Future readers of either site see the same convention.

S1 fix prevents drift. The K_CLOSURE_OFFSET / K_FN_OFFSET extraction is small but structurally important — without it, the next slice's arity widening would have to update two sites in lockstep, and a missed update would silently miscompile (writer at offset N, reader at offset 0). Helper-function approach binds the two sites at call time.

S3 test coverage is well-scoped. Three tests covering: (1) per-callsite stack slot independence, (2) Bool return narrow path, (3) pointer return path. Together with the happy-path Int test from 33f22318, the wrapper's narrow/addressing logic is now covered across all three Cranelift type bins.

Pre-merge gates

• CI green is the load-bearing gate. Currently RED on all 4 lanes; agent investigation needed.
• The 4 new cps_abi e2e tests verified pass locally; lib tests verified pass locally (447); pod-verify checks pass per agent's commit message.
• If CI failure turns out to be perf-flake on all 4 lanes simultaneously, that's worth its own deviation entry — pattern is now consistent across multiple PRs and may need a structural fix (platform-aware floors, or release-only gating) at Task 60.

Forward-looking concerns from prior reviews

All 8 still tracked. No new ones from this round — the must-fix work matched the items raised. The implementing-commit cadence remains on track for both substantive correctness and architectural slicing.

Action for the agent

Investigate the CI failure on 5a0459a before any further commits. Get the job logs (the gh CLI and API both return errors for the run; may need to fetch via the Actions UI directly). If perf-flake, document and re-trigger CI. If real failure, surface and address.

The 4 items are otherwise closed cleanly; the foundation continues to harden.

[boldfield]

Mid-flight review at 2be70ce

Scope: the single commit since the prior mid-flight at a5ee4c6. Net +266/-31 (216 codegen + 81 e2e). 5 of the 7 prior-review items addressed in this bundle; 2 explicitly deferred with rationale. CI: 1 of 4 lanes green, 3 in progress at review time — re-check before merge of the next slice.

What this slice does

• Item Plan A1 code-review fixes: 6 issues from the post-review audit #1 (Match-pattern walker fix, M) — walk_collect_captures now per-arm-clones bound, extends with pattern-introduced names via the new collect_pattern_bindings helper, then recurses into the arm body. The helper handles all Pattern variants: IntLit/BoolLit/CharLit/Wildcard (no binding), Var (insert name), Tuple (recurse into each), Ctor with Unit/Positional/Record field shapes (recurse into nested patterns). Per-arm clone preserves arm-locality of pattern bindings.
• Item Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 (prior test gaps, partial — 2 of 5) — adds cps_abi_arity_n_helper_with_constant_done_synth_cont (arity-N ConstantDone shape) and captures_bearing_synth_cont_with_two_user_params_captured (multi-capture, pins closure record's slot ordering at offsets 16 and 24).
• Item Plan A2 Tasks 26+28 + PR #2 deferrals + 2 formal deviations #4 — let _args_len = block_params[2] rename applied at the two remaining sites (codegen.rs:3182, :4013); all three callsites now consistent.
• Item Plan A2 Task 29: parser extensions (lambda syntax) #5 — compute_user_fn_abi's underscore-with-comment _params parameter dropped entirely. 8 test call sites + 1 production call site updated. Cleaner.
• Item Plan A2 Tasks 31+32: closure conversion + codegen (closure ABI) #7 — ..._skips_globals test renamed to ..._does_not_recurse_into_call_in_tail with docstring update.
• Items Plan A2 Tasks 24+25: Stage-2 codegen + runtime arith/byte primitives #3 (FFI-ref dedup) and Plan A2 Task 30: typechecker fn types + application unification + capture analysis #6 (linear-search dedup) — explicitly deferred. Plan A2 Tasks 24+25: Stage-2 codegen + runtime arith/byte primitives #3 with a stated commitment ("committed to addressing in the next commit on this branch before any further slice expansion"); Plan A2 Task 30: typechecker fn types + application unification + capture analysis #6 marked informational and skipped.

What's correct

• The Match-pattern fix is the right shape. bound.clone() per arm + extension via collect_pattern_bindings mirrors the lexical scoping the block walker already does for Stmt::Let. The scrutinee walks BEFORE the per-arm clone — correct, since pattern bindings can't reach the scrutinee.
• collect_pattern_bindings is exhaustive over Pattern. Handles Pattern::Var, Pattern::Tuple's nested patterns, Pattern::Ctor's three field shapes (Unit, Positional(Vec<Pattern>), Record(Vec<CtorPatternField>)). The recursion via collect_pattern_bindings(&rf.pattern, out) also picks up nested Pattern::Var inside Ctor::Record (the field-pun shape Point { threshold } produces a CtorPatternField { name: "threshold", pattern: Pattern::Var("threshold") } which the walker resolves correctly).
• The two new e2e tests pin orthogonal axes. cps_abi_arity_n_helper_with_constant_done_synth_cont exercises the ConstantDone code path with helper user-params consumed by the perform's args; captures_bearing_synth_cont_with_two_user_params_captured pins the closure-record slot-ordering invariant (captures[i] at closure_ptr + 16 + 8*i) that was load-bearing-but-unpinned at a5ee4c6.
• The _params removal is the right call given the user's "no backwards-compat hacks" preference. If a future slice needs arity gating, re-adding the param is one-line + 9 callsites — git log will surface the prior shape.
• Pre-fix counterfactual is documented in the new test docstring: "Pre-fix this returned [threshold] (incorrect)." — exactly the regression-pin discipline the prior reviews have been asking for.

---

Issues to address

1. Pattern-fix test coverage gaps for two binding shapes (S/M)

The new tests pin Pattern::Var and Pattern::Ctor::Record shadowing. Two binding shapes the helper handles but no test exercises:

• Pattern::Tuple with binding — e.g., match (x, y) { (threshold, _) => threshold + 1 }. The Tuple(patterns) arm of collect_pattern_bindings recurses into each sub-pattern; if a future refactor breaks tuple recursion, no test fires.
• Pattern::Ctor::Positional with binding — e.g., match opt { Some(threshold) => threshold + 1 }. Same concern; the Positional(patterns) recursion is unexercised at the test level.

Both are simple unit tests in the same shape as the two new ones. Add them as a small follow-up. Worth pinning while the fix is fresh — once the slice expands and the walker grows again, regressions in obscure pattern shapes become harder to bisect.

2. Arity-N ConstantDone synth-cont's run path is unpinned (M)

cps_abi_arity_n_helper_with_constant_done_synth_cont asserts stdout 42 from the source:

fn helper(x: Int) -> Int ![E] { perform E.op(x); 99 }
fn main() -> Int ![IO] {
  let n: Int = handle helper(7) with { E.op(arg, k) => arg + 35 };
  ...
}

The arm E.op(arg, k) => arg + 35 does not reference k, so the arm returns Done(arg + 35) = Done(42) and the ConstantDone synth-cont (which would return Done(99)) never runs. The test pins the BUILD path of arity-N + ConstantDone (helper unpacks x, packs into perform args, dispatches arm) — but the synth-cont's RUN path under this shape is dead code in the test.

To exercise the synth-cont's run path, a companion test with arm E.op(arg, k) => k(arg) would let k(arg) invoke the synth-cont (which then returns Done(99), ignoring arg). Result 99 instead of 42 would pin the synth-cont firing path. Cheap follow-up.

This is the same coverage shape the captures-bearing slice's discard_k + use_k test pair pins for LetBindThenTail. ConstantDone deserves the same pair.

3. 3 of 5 prior-review test gaps still open (M, deferred)

The agent's deferral note says: "the patterns are similar to the existing happy-path tests and the captures-bearing slice already exercises the slot-ordering surface for I64 captures." That's true for the slot-ordering invariant but doesn't cover:

• Non-Int binding in LetBindThenTail — the ireduce narrow path on the binding load (Bool → I8, Char → I32, pointer-typed → ireduce-or-passthrough) is unexercised at runtime. The unit test let_yield_with_match_tail_using_binding_recognised accepts Bool at the classifier level, but no e2e drives it through codegen.
• Pointer-typed binding — the String / User arm of the synth-cont's binding-narrow switch is dead code in the e2e suite.
• Non-Int capture types — slot_kind_for_type_expr_post_mono's Bool / String / User arms have lib-level coverage via compute_user_fn_abi_* tests but no e2e exercises the closure-record slot's widening-on-write + narrow-on-read symmetry for non-Int kinds.

These three are all valuable: the closure-record slot encoding has different code paths per kind, and a regression in (e.g.) the EnvSlotKind::Bool write-side uextend would silently produce garbage in the upper bits without any current e2e firing. Recommend batching them into a single e2e file expansion before the next major slice (multi-yield bodies or multi-shot k) lands.

4. FFI-ref dedup deferral now has an explicit commitment — track it (S, observation)

The commit message states: "committed to addressing in the next commit on this branch before any further slice expansion." That's the right signal — the surface is at three sites and growing each slice would make extraction more painful.

If the next commit on this branch is not the FFI-ref extraction, that's a third-time-flagged backslide and merits a hard pause. Worth a TODO(plan-b-task-55-phase-4e/ffi-ref-extraction) marker at one of the three duplicated sites so the commitment is grep-able and won't slip if the slice ordering shifts.

5. Deep-nesting clone cost (S, theoretical)

bound.clone() per arm in the Match walker is O(N) per arm where N = current bound set size. For deeply nested Match-in-Match-in-If patterns under a wide outer bound, the clones compound. Probably never matters in real code (the classifier surface is narrow and helper bodies are small), but if profiling later shows the captures-collection hot, switch to bound.insert(...) + bound.remove(...) instead of clone-on-extend. Don't bother now.

---

Architectural notes

The deferral discipline is healthy this round. Each deferred item has a stated rationale + a follow-up plan: #3 commits to the next commit, #6 marked informational, #2's three remaining gaps named explicitly. Compare to the prior round where the FFI-ref dedup quietly carried forward without an explicit deferral note. The improvement makes the review-burden negotiation transparent.

The Match-pattern fix opens a small follow-up question. With the walker now correctly tracking pattern bindings, the classifier is_simple_let_yield_then_pure_tail_body could in principle accept Match-tails with arbitrary patterns. At HEAD the classifier doesn't differentiate — expr_is_pure on Expr::Match requires only that scrutinee + arm bodies are pure, not anything about patterns. The walker's correctness is now load-bearing for arbitrary patterns. Worth confirming via a Pattern::Var e2e that the synth-cont actually emits correct code for a shadowing pattern at runtime (the lib test pins the captures vector; an e2e would pin lower_match's per-arm-env extension at runtime). But not required for this slice — the captures-vector correctness is the prerequisite.

Cumulative slice progress:

• Foundation (deviation + tests): done
• CPS transform (cps.rs accessors): done
• Codegen consumes color (sig + body emit + interop wrapper): done
• Lambda-lifting first slice (ConstantDone): done
• Lambda-lifting captures-free let-yield (LetBindThenTail w/o captures): done
• Lambda-lifting captures-bearing let-yield: done
• Pattern-walker correctness fix: done
• FFI-ref dedup: pending (committed for next commit)
• Multi-yield bodies, multi-shot k, surrounding-lambda captures: pending
• Test inversion (already closed at b818fc3 + 2a9958b for hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2)
• README + PROGRESS final cleanup: pending

5 of ~9 implementing commits + the 1 cleanup commit = roughly 6 of 10. On track for the comprehensive-scope upper bound from the foundation-review estimate.

---

Verdict: clean follow-up commit. The Match-pattern fix is the substantive piece and is correctly shaped. Three small open items: pin Pattern::Tuple + Pattern::Ctor::Positional with one unit test each (#1); add the arity-N ConstantDone synth-cont's use-k companion test (#2); batch the three deferred prior-gap e2e tests into the next non-FFI-dedup commit (#3). The FFI-ref dedup must land in the next commit per the agent's stated commitment — track via grep-able marker. CI not fully green at review time; re-verify before stacking the next slice.

[boldfield]

Mid-flight review at 73c7e53

Two new commits since 2be70ce. CI all 4 lanes SUCCESS. Local: 466 compiler lib tests pass; e2e tests pass except for the two pre-existing perf-flake tests (environment-bound, well-documented).

The deferral chain ended cleanly. Both items: (a) all 5 actionable items from the no-context reviewer's mid-flight at 2be70ce closed in f7d4a64, and (b) the FFI-ref dedup that's been deferred for THREE consecutive mid-flights closed in 73c7e53 per the agent's stated commitment.

Per-task

Commit	Item	Status
f7d4a64	Pattern shape coverage gaps (Pattern::Tuple + Pattern::Ctor::Positional)	✅ 2 new lib tests
f7d4a64	ConstantDone synth-cont RUN path test	✅ cps_abi_arity_n_helper_with_constant_done_synth_cont_use_k forces synth-cont to fire; closes BUILD/RUN coverage symmetry that LetBindThenTail already had
f7d4a64	3 deferred prior-gap e2e tests	✅ Bool binding (ireduce narrow), pointer binding (pass-through), non-Int capture types (uextend write + ireduce read symmetry)
f7d4a64	FFI-ref dedup TODO marker (grep-able commitment)	✅ added at user-fn body emit site; references all 3 prior reviews
f7d4a64	Item #5 (deep-nesting clone cost)	⏭ informational; skipped per reviewer guidance
73c7e53	FFI-ref dedup extraction	✅ closes the deferred-must-fix that's been flagged across 33f2231 / a5ee4c6 / 2be70ce reviews

Verification of 73c7e53 (FFI-ref dedup)

Extracted prepare_per_fn_refs(module, builder, ctx) helper called once per fn body emit. Cranelift FuncRefs are per-Function, so the helper has to be called per-fn — consolidates the previously-duplicated ~70 LOC setup into a shared call.

Architecture is right:

• PerFnRefsCtx<'a> struct holds cross-fn FuncIds + side-tables (handler_arm_indices, handler_arm_synth, user_fns, string_literals, lit_ids, div_zero_msg_id, mod_zero_msg_id). Borrows 'a-bound to emit_object's stack frame; constructed once at top.
• PerFnRefs struct holds per-fn outputs: standard FFI FuncRefs, per-handle synth-arm FuncRef map, per-user-fn FuncRef map, string-literal GVs, divmod-zero panic GVs.
• Three duplicate sites destructure the helper's output. User-fn site + synth-cont LetBindThenTail site discard next_step_done_ref / next_step_call_ref / next_step_args_ptr_ref (only synth-arm-fn body emits those calls). Cranelift prunes unused FuncRefs from emitted function — no IR bloat.
• TODO marker from f7d4a64 removed; replaced with single-line comment pointing at prepare_per_fn_refs.

Net code reduction: ~215 LOC removed across three sites; ~150 LOC of helper + ctx struct added. ~65 LOC net reduction. Future slices that add a fourth FFI-ref site land as one-line additions to PerFnRefs instead of growing the duplication.

Discipline pattern observation

This is now the third consecutive review round where the agent has:

1. Closed every actionable item from the parallel reviewer
2. Honored explicit deferral commitments in the next commit

The "deferral accumulation" pattern I flagged in the prior review reversed cleanly. The TODO marker pattern (grep-able, references all flagging reviews, removed when extraction lands) is the right tool — captured the commitment in code, not just in commit messages.

Worth noting as a positive data point: the parallel-review setup + the agent's responsiveness produces sustainable cadence even at 25 commits in-flight.

Test coverage matrix — now complete

Pattern matrix: all 4 binding shapes (Var, Tuple, Ctor::Unit/Positional/Record) have shadowing-shadow tests. A future refactor that breaks any specific pattern's recursion in collect_pattern_bindings fires a specific test.

Codegen-consumes-color binding-type matrix: Bool, Char, Int, String, pointer-typed all have e2e coverage. Both write-side widen and read-side narrow tested for non-Int kinds.

Closure record slot ordering: multi-capture e2e pins captures[i] at closure_ptr + 16 + 8*i.

What's still pending in Phase 4e

Item	Stage 9 required?	Status
Stmt::Perform; non-constant tail (no let-binding)	Likely no	Future slice
Multi-yield bodies (chained synth-conts)	Likely no	Future slice
Multi-shot k via heap-reified continuation	YES (P20)	Major slice candidate
Surrounding-lambda closure captures into arm bodies	Probably yes	Major slice candidate
README + PROGRESS final cleanup	No	End of Phase 4e

5 items remaining. 2 of 5 Stage-9-required.

Pre-merge gates

• All 4 CI lanes SUCCESS on 73c7e53
• 466 lib tests pass locally
• e2e tests pass except for the two pre-existing perf-flake tests (fib_perf_example_prints_6765_under_50ms, tree_example_prints_32767_under_500ms — environment-bound, well-documented across PRs)
• Pod-verify clean per agent's commit messages

Forward-looking concerns from prior reviews

All 8 still tracked. The Match-pattern walker forward concern (raised by the no-context reviewer last round) closed cleanly. The FFI-ref dedup deferral chain closed.

Cumulative project state

• Phase 4e branch: 25 commits total
• 466 compiler lib tests
• 17+ cps-related e2e tests
• Pattern walker matrix complete; binding-type matrix complete; closure record slot ordering pinned
• Hard condition Plan A2 Stage 1.5: scaffolding + pod-verify + cold-checkout fix + debug_assert #2 closed
• FFI-ref dedup chain closed
• 2 of 3 Stage-9-required Phase 4e items pending (multi-shot k, surrounding-lambda captures)

Cadence observation

The pace is steady and the architectural slicing remains intact. 25 commits is at the high end of what's reviewable as a single PR but each commit is shipping reviewable focused work. The remaining 2 Stage-9-required items (multi-shot k, surrounding-lambda captures) are the meaty architectural slices left. If they ship at similar cadence, Phase 4e closes in another ~5–8 commits.

No action required from the agent for this review round. Discipline pattern is strong; deferral chain closed cleanly.

[boldfield]

Mid-flight review at 73c7e53 (no-context)

Two commits since 2be70ce: f7d4a64 (5-item bundle of test coverage + FFI-ref TODO marker) and 73c7e53 (extracts prepare_per_fn_refs helper). CI green on all 4 lanes at HEAD; local compiler lib = 466 tests pass; new e2e tests in f7d4a64 pass.

The deferral chain that's been running across 33f2231 / a5ee4c6 / 2be70ce for the FFI-ref dedup closes here. The architecture is right: shared PerFnRefsCtx constructed once at emit_object top, prepare_per_fn_refs(&mut module, &mut builder, &ctx) called once per fn body emit (Cranelift FuncRefs are per-Function; can't reuse). Net ~65 LOC reduction; future fourth-site additions land as one-line PerFnRefs field additions.

One real finding: duplicate pattern-bindings walker

arm_body_collect_pattern_bindings (compiler/src/codegen.rs:1162, single caller at :944, added Phase 4c in e3ed53a) and collect_pattern_bindings (compiler/src/codegen.rs:1606, single caller at :1573, added in 2be70ce) are byte-for-byte equivalent — identical match arms, identical recursion. Two single-call helpers doing the same thing.

The new pattern-coverage tests in f7d4a64 pin collect_pattern_bindings only. The duplicate arm_body_collect_pattern_bindings is unpinned by the new tests; future drift in either function won't be caught by the other's tests.

Not a correctness issue today. Suggested follow-up: pick one (probably collect_pattern_bindings since it has tests now), drop the other, retarget the line-944 call site. ~30 LOC deletion. Could ride along with the next test-coverage commit or be its own one-liner.

Smaller observations

1. Commit-message test count in f7d4a64 claims "466 → 468 compiler lib tests". Local count at HEAD = 466. Likely the pre-commit count was 464 (codegen.rs went 42 → 44 #[test] attrs, +2 matches the diff). Off by 2 in either reading. Minor; doesn't affect substance.

2. PR description roadmap checkboxes are stale. Description still shows only [x] Foundation, with cps.rs / codegen-consumes-color / colorer-refinement / non-tail-k / captures all unchecked. Per the d8a4771 reframe + a756bd3 design plan + 33f2231 / 1a8ea17 / b818fc3 / 2a9958b / a5ee4c6 actual landings, several of those are effectively closed. A casual reader of the PR description sees ~12% progress when the reality is closer to ~70%. Worth a description refresh on the next non-trivial commit.

3. Bool capture test pins widen/narrow symmetry; Char capture does not. cps_abi_captures_bearing_with_bool_capture_exercises_widen_narrow_symmetry covers Bool (I8 ↔ I64). Char is the other non-Int width-discrepant kind I'd expect to exercise the same uextend-write / ireduce-read path (assuming Char is sub-I64). If Char captures haven't been exercised yet, that's a parallel coverage gap to the Bool one this commit closed. Low priority — Bool is the harder case (single-bit values surface upper-bit corruption more reliably).

4. PerFnRefs 20-field struct, full destructure at all 3 call sites with _: discards for the 3 always-discarded refs at user-fn + synth-cont sites. Pattern is correct (rustc enforces exhaustive destructure), but the discard noise at 2 of 3 sites is real. If a fourth site lands that needs yet another partial subset, consider splitting PerFnRefs into a PerFnRefsCommon (shared) + per-site extension structs. Not warranted today.

5. "Cranelift prunes unused FuncRefs" doc comment is slightly imprecise. The unused FuncRefs are registered in the Function's dfg.ext_funcs table — they aren't strictly pruned. What's true: no relocations are emitted for FuncRefs not referenced by Call/FuncAddr instructions, so the emitted object code is unaffected. Distinction is hair-splitting, but the wording "Cranelift prunes" reads as a stronger claim than the runtime behavior delivers.

Verification of 73c7e53

• All 3 call sites destructure the same PerFnRefs shape.
• Borrow lifetimes: per_fn_refs_ctx holds &'a borrows of immutable side-tables; &mut module and &mut builder are passed fresh per call. No conflicts.
• Per-call work is unchanged from before (15 FFI ref decls + O(handle-arms) + O(user-fns) + O(string-lits)). No perf regression.
• Helper has no direct unit test — exercised via integration through 3 call sites + existing e2e/lib coverage. Acceptable for a structural extraction.

Summary

Both commits ship cleanly. The deferral chain closes correctly. The duplicate pattern walker is the one substantive find — minor, follow-up sized. Test-count and roadmap-checkbox observations are commit-message / PR-description hygiene, not blockers.

Forward-looking: the two Stage-9-required items still pending per the deviation roadmap (multi-shot k via heap-reified continuation; surrounding-lambda closure captures into arm bodies) remain the meaty architectural slices left in Phase 4e.

[boldfield]

Mid-flight review at adf0e23 (no-context)

One commit since 73c7e53. Net +58/-45 (codegen -34 LOC from dedup; +40 e2e for new Char test). CI in progress on all 4 lanes at review time — flagging early; will need re-check before any merge decision.

Local compiler lib = 466 tests pass (unchanged: dedup removes a function with no tests; Char test is e2e). e2e count: 67 → 68.

Verification of 3-of-6 items addressed

Item from prior review	Status	Verification
#1 pattern-walker dedup (real find)	✅ closed	arm_body_collect_pattern_bindings removed; single caller at codegen.rs:944 retargeted to collect_pattern_bindings. Single survivor at codegen.rs:1572, two callers (:944 arm-body-walk, :1539 synth-cont capture walker). ~34 LOC deletion. Bodies were byte-equivalent — no behavior change.
#4 Char capture e2e (parallel coverage gap)	✅ closed	cps_abi_captures_bearing_with_char_capture_exercises_widen_narrow_symmetry added at e2e.rs. Helper marker: Char, captures it; tail if marker == 'A' then x else 0; arm Raise.fail(k) => k(99). Asserts 99/0 for 'A'/'B'. Mirrors the Bool test shape; exercises I32 ↔ I64 widen/narrow at the closure-record slot.
#6 doc precision on FuncRef "pruning"	✅ closed	All 4 occurrences fixed (PerFnRefs doc, prepare_per_fn_refs doc, two inline comments at user-fn + synth-cont sites). Wording now: "unreferenced FuncRefs sit in dfg.ext_funcs without producing relocations, so the emitted object code is unaffected." Accurate.

Items NOT addressed

Item	Status	Note
#2 commit-message test count in f7d4a64 ("466 → 468" vs actual 466)	Open	Past; can't be fixed without rewriting history. Not worth a force-push for this. Closing as won't-fix.
#3 PR description roadmap checkboxes stale	Open	Description still shows only [x] Foundation. Several listed items (cps.rs / codegen-consumes-color / colorer-refinement / non-tail k / captures) have effectively shipped. Cheap to update — one description edit. Recommend doing this before the PR exits draft.
#5 PerFnRefs 20-field destructure	Open	Acknowledged as low priority in prior review ("Not warranted today"). No action needed.

Substance check on adf0e23

Pattern-walker dedup is correct. Both prior fns were byte-for-byte equivalent — same match arms over Pattern::{Var, Wildcard, IntLit, BoolLit, CharLit, Tuple, Ctor::{Unit, Positional, Record}}, same recursion. The two callers share the same semantic need: compute the set of names bound by a pattern. No semantic divergence between the two original implementations; the dedup is a pure refactor.

The 4 prior-existing tests on collect_pattern_bindings (Var, Tuple, Ctor::Positional, Ctor::Record) now transitively cover both call sites. A future drift in pattern-binding semantics fires once across all callers — the goal.

Char capture test is well-designed. Parallel to the Bool test in shape and intent. The discriminating runtime comparison (if marker == 'A') ensures observable failure if either widen or narrow regresses. Two probes ('A' → 99, 'B' → 0) both land cleanly in the diff; CI will validate.

Doc precision fixes are accurate. "Unreferenced FuncRefs sit in dfg.ext_funcs without producing relocations" matches Cranelift's actual behavior — declare_func_in_func registers in the Function's dfg.ext_funcs, and only the FuncRefs referenced by Call/FuncAddr instructions become relocation targets in the emitted object.

Smaller observations on this commit

1. No new code-quality concerns introduced. Pure cleanup + test addition.
2. Char test uses 'A'/'B' Char literals — Sigil parser supports these (per existing Pattern::CharLit). CI will catch any literal-syntax issue at parse.
3. Net codegen LOC down 34 — dedup outpaces test additions. Real shrinkage of the file size; healthy direction.

Forward-looking

Same as prior review: two Stage-9-required Phase 4e items remain (multi-shot k via heap-reified continuation; surrounding-lambda closure captures into arm bodies). Both are major architectural slices.

Pre-merge gates (status snapshot)

• CI all 4 lanes — IN_PROGRESS, re-check needed
• Local lib tests — 466 pass at HEAD
• e2e tests — local CI deferred per agent; full validation pending CI lanes
• PR description checkbox refresh — recommended before exit-draft
• Two Stage-9-required slices — pending

No further action required for this commit's substance once CI confirms green. The 3 closed items resolve cleanly. Recommend the agent picks up the PR description refresh on the next non-trivial commit (it's a 30-second edit and meaningfully helps any reviewer arriving fresh).

[boldfield]

Mid-flight review at adf0e23

One commit since 73c7e53 — agent's response to the no-context reviewer's parallel review at 73c7e53. CI 2 of 4 lanes SUCCESS (macos-14); 2 IN_PROGRESS (ubuntu-24.04). Local: 466 compiler lib tests pass; new cps_abi_captures_bearing_with_char_capture_* e2e test passes.

Credit allocation

The substantive find this round came from the no-context reviewer: duplicate pattern walkers (arm_body_collect_pattern_bindings from Phase 4c e3ed53a + collect_pattern_bindings from 2be70ce — byte-for-byte equivalent, two single-call helpers doing the same thing on the same Pattern enum). I missed it during my mid-flight at 73c7e53.

This is the third consecutive round where the no-context reviewer caught a structural-correctness item I missed (Match-pattern walker bug at a5ee4c6; the asymmetric assert/debug_assert hardening earlier; this duplicate walker at 73c7e53). The pattern is consistent: shape-specific subtleties + structural duplications are where parallel review pays off most.

The no-context reviewer is now self-identifying with "(no-context)" in the review header — useful structural improvement that makes credit allocation cleaner going forward.

Verification — 3 of 5 items closed

#	Item	Status
1	Duplicate pattern walkers	✅ Closed. Dropped arm_body_collect_pattern_bindings; retargeted call site at arm_body_walk's Match arm to use collect_pattern_bindings directly. ~30 LOC deletion. The newer collect_pattern_bindings already had direct lib-test coverage (Pattern::Var/Tuple/Ctor::Positional/Ctor::Record across 2be70ce + f7d4a64); the older walker was unpinned. Future drift now caught by the dedup'd walker's tests
4	Char capture e2e test	✅ Closed. New cps_abi_captures_bearing_with_char_capture_exercises_widen_narrow_symmetry mirrors the Bool test's shape: helper takes marker: Char, captures it; tail if marker == 'A' { x } else { 0 }. Pins the I32 ↔ I64 widen/narrow symmetry for Char captures (was the parallel coverage gap to the Bool path)
6	Doc-comment precision on "Cranelift prunes unused FuncRefs"	✅ Closed. Reviewer flagged the wording was stronger than runtime delivers (Cranelift doesn't prune; unused FuncRefs sit in dfg.ext_funcs without producing relocations). Fixed at four sites: PerFnRefs struct doc, prepare_per_fn_refs fn doc, two inline comments. New wording: "unreferenced FuncRefs sit in dfg.ext_funcs without producing relocations, so the emitted object code is unaffected" — matches Cranelift's actual behavior
2	Commit-msg test count off-by-2	⏭ Skipped — refers to prior commit message; can't fix retroactively without force-push
3	PR description checkboxes stale	⏭ Deferred to separate gh pr edit action (not a code change). Worth doing — current description shows ~12% progress when reality is ~70%
5	PerFnRefs 20-field destructure with _: discards at 2 of 3 sites	⏭ Reviewer marked "not warranted today"; deferred

Verification of pattern-walker dedup

The dropped helper had no test coverage; the retained helper has 4 tests covering the full pattern matrix (Var/Tuple/Ctor::Positional/Ctor::Record). Both call sites — arm_body_walk's Match arm and walk_collect_captures's Match arm — now share the same pattern-bindings collection. Cleanup removes ~30 LOC of duplicated logic; no behavior change.

The Char capture test is structurally important: closes the parallel coverage gap on the captures-bearing closure record's widen/narrow symmetry. Bool was the harder case (single-bit corruption surfaces upper-bit issues more reliably); Char extends to the I32 width for completeness.

Pattern observation across recent rounds

Round	Subtle structural find caught by no-context reviewer	Caught by me?
a2840b6	UserFnAbi::Native overloads Color::Native (naming clash)	No
5a0459a (D2)	args_len convention mismatch between perform-site and wrapper	No
a5ee4c6	Match-pattern walker missing pattern-bound names (latent correctness)	No
73c7e53	Duplicate pattern walkers (~30 LOC byte-equivalent dead code)	No

Four for four where the parallel-review setup caught structural items I missed. Worth making explicit: my reviews are catching the high-level architectural / discipline-level concerns (deferral accumulation, doc-vs-code drift, hard-condition tracking); the no-context reviewer is catching the line-level structural subtleties (naming, deduplication, walker correctness, convention mismatches). The two specialties compose well.

Pre-merge gates

• 2 of 4 CI lanes SUCCESS at HEAD (macos-14 build+test, macos-14 cold-checkout); 2 IN_PROGRESS (ubuntu-24.04 lanes). Wait for green before declaring slice done
• 466 lib tests pass locally; new Char capture e2e passes
• Pod-verify clean per agent's commit message

Deferred items worth tracking

• PR description refresh (item 3) — should happen before any external reader looks at this PR. The current description shows only [x] Foundation with cps.rs / codegen-consumes-color / colorer-refinement / non-tail-k / captures all unchecked. Reality is ~70% done.
• 20-field destructure noise (item 5) — reviewer's "not warranted today" stands; revisit if a 4th call site lands.

Forward-looking concerns from prior reviews

All 8 still tracked. No new ones from this round.

Cumulative project state

• Phase 4e branch: 26 commits total
• 466 compiler lib tests
• 18+ cps-related e2e tests
• Pattern walker dedup'd; Char capture symmetry pinned; doc precision tightened
• 2 of 3 Stage-9-required Phase 4e items pending (multi-shot k, surrounding-lambda captures)

What's still pending in Phase 4e

Per the agent's roadmap (unchanged):

Item	Stage 9 required?
Stmt::Perform; non-constant tail (no let-binding)	Likely no
Multi-yield bodies (chained synth-conts)	Likely no
Multi-shot k via heap-reified continuation	YES (P20)
Surrounding-lambda closure captures into arm bodies	Probably yes
README + PROGRESS final cleanup + PR description refresh	No
Plus: plan-b-invariants flips (deep recursion, multi-shot, selective CPS)	Closeout gate

Plus the plan-b-invariants script's three SKIPs (flagged in our prior conversation) — those are the Phase 4e closeout gates, not implementing items per se. They flip when the underlying machinery + test programs land.

Cadence observation

26 commits on the branch. Each commit shipping reviewable focused work. Three more substantial items (multi-shot k, surrounding-lambda captures, README+PROGRESS+invariants closeout) suggest another ~5–8 commits before Phase 4e closes. The pattern's been stable for 5+ rounds now.

No action required from the agent for this review round beyond the deferred PR-description refresh (which is a one-shot gh pr edit action, not a commit).