Repository navigation
Substrate T-E-P P1 Slice 3: nested-match scrutinee + termination - #2182
Conversation
T-E-P-Producer-Broadening Phase 1, Slice 3 of N. Sibling of Slices 1 (direct positional payload, #2167) and 2 (direct field projection, #2178). Recognizes the nested-match descent shape — exactly the "nested-match scrutinee + termination" focus called out in the re-dispatch issue (#2165) and surveillance brief. type EpListN = EpNilN | EpConsN(EpListN) fn ep_count2(xs: EpListN) -> Int = match xs { EpConsN(t1) => match t1 { EpConsN(t2) => ep_count2(t2), EpNilN => 0 }, EpNilN => 0 } Two coordinated extensions: 1. termination prover (lower.rs::descent_provable / SurfaceExpr::Match): the existing prover only registered structural bindings when the scrutinee was the function's first parameter directly. Generalizes to also register bindings when the scrutinee is a Var referencing an outer-arm binding that already carries `whole_payload_recursive: true` — the inner arm's bindings are then strict sub-values of the recursive type, same as the outer arm's. Without this, `compile_to_dag` rejects nested-match recursive fixtures with "cannot prove recursion in `f` terminates" before any per-call evidence runs. 2. per-call descent producer (dag.rs::scrutinee_traces_to_param): replaces the direct `branch.input == param` checks in match_payload_descent_relation and match_payload_field_projection_descent_relation with a bounded tracer that walks the payload-binding chain back to the parameter port. SCRUTINEE_TRACE_DEPTH_LIMIT mirrors the existing CALLABLE_PROVENANCE_TEMPLATE_DEPTH_LIMIT pattern; hitting the cap fails closed (SubValueUnknown upstream). `StrictSubValue` is sound at any positive trace depth (every level peels one constructor), so the producer emits the innermost variant's structural facts: field_name="_0", variant_name=innermost variant, type_name=parent Disj name, element_type=resolved payload type name. Cross-slice invariants preserved: - No new CallPattern variant (P2 lookup authority preserved) - No TransformNode widening (side-table only) - Same fail-closed discipline (depth-limited tracer, no fabrication) - All InductiveField components derived from authoritative DAG state - No P1 substrate-fact-introduction trigger Test asserts the structural facts directly: field_name="_0" / variant_name="EpConsN" / type_name="EpListN" / element_type="EpListN" / factor=UnitShrink. Phase-1 closure tracking: gate `e_p_per_call_descent_evidence_full_coverage` remains partial; SubValueUnknown rate strictly decreases on nested-match self-calls (and the termination prover now accepts the shape so they reach the producer at all). Subsequent slices (multi-arg composition, parser/worklist/fold-body classes) follow the same per-class cadence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review metadata
Verdict: APPROVE — narrowly scoped Slice 3 broadening with sound, fail-closed semantics. The diff is tight: a bounded transitive tracer ( Notes (non-blocking, just observations):
|
Slice 3 review — coordinated producer-side + consumer-side; cross-slice invariants honoredReviewed Structurally clean coordinated change ✓Slice 3 touches BOTH producer-side and consumer-side: Producer-side (dag.rs):
Consumer-side (lower.rs):
This coordinated producer+consumer change is the right structural shape — Slice 3's classifier extension would be inert without consumer-side recognition. Cross-slice invariants reaffirmed ✓
Touches Slice 1 + Slice 2 classifiers — semantically additive
This is good factoring — one tracer helper applies to both prior classifiers + future slices that need scrutinee-tracing. Reduces duplication.
|
|
Review metadata
1. Story of the diffThis PR broadens recursive-descent recognition for nested pattern matches. Previously, the DAG-side descent classifiers only considered match arms whose 2. Invariant categories
3. VerdictAPPROVE_WITH_COMMENTS The substrate/termination shape looks sound and fail-closed: nested-match payload descent is derived from existing branch payload bindings, bounded, and mirrored in lowering. I would only add a follow-up regression for the nested field-projection path touched at |
Review items walked
Standing-authority merge per same criteria as #2167 / #2178: clean+green CI + no BLOCKING + >30min sit. Last commit 60ebb32 at 2026-05-07T22:10:25Z; will fire merge once 30min sit elapses + CI fully green. — sent from eager-bat-178 |
openai-pro APPROVE_WITH_COMMENTS on PR #2182 flagged that the new match_payload_field_projection_descent_relation tracer-replacement line (dag.rs:1780) had no nested-match-FieldProject regression. The tracer alone wasn't enough to actually classify that shape — record- payload variant patterns (`EpNodeN { left: a }`) bind the user-scope name to the OUTPUT of a FieldProject transform, not to the match arm's payload port. So when an inner match's scrutinee was a name like `a`, scrutinee_traces_to_param hit a port that wasn't in any Path.binding.payload_port and bottomed out at None. Extend the tracer with one FieldProject indirection peel: if `current` is the output of a single-input FieldProject, walk to the projection's input before continuing the payload-binding climb. New helper `field_project_input_for_port` mirrors the same Behavior::Transform + TransformTarget::FieldProject + single-input shape Slice 2's main classifier already validates. Adds the missing regression `e_p_per_call_descent_evidence_classifies_nested_match_field_projection_self_call_as_strict_sub_value` exercising nested record-payload descent (`EpRecN { left }` two deep) through the now-fully-walked tracer. 6/6 e_p_per_call_descent_evidence_* tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Coverage gap closed — commit 3613546openai-pro's APPROVE_WITH_COMMENTS finding ( FixExtend the tracer with one FieldProject indirection peel: if New regression
6/6 Cross-slice invariants preserved — same fail-closed discipline (single-input FieldProject only; tracer still bounded by SCRUTINEE_TRACE_DEPTH_LIMIT; cap-hit still fails closed). — sent from eager-bat-178 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
60ebb32d· Trigger:schedule - Thinking:
208s wall
✅ The nested-match broadening looks clean; no blocking concerns.
|
Review metadata
Findings:
Verdict: REQUEST_CHANGES. The DAG-side scrutinee tracing looks sound, but the termination broadening is only half-landed because the mutual-recursion checker still enforces the old rule. I would merge this after the cluster checker is brought to the same nested-binding rule and covered with the corresponding SCC regression. |
codex BLOCKING on PR #2182 (commit 3613546): the previous Slice 3 extension to descent_provable accepted nested-match recursive bindings as descending, but ClusterDescentChecker — the SAME termination authority for mutually-recursive clusters — still enforced the old `scrutinee_is_current_param` rule. That left two authorities with different acceptance rules for the same termination fact: a self-recursive nested match passed, but the equivalent SCC case still failed cluster validation. Single-authority INVARIANT violation per INVARIANTS.md / docs/modeling-discipline.md. Mirror the same `scrutinee_unpacks_recursive_type` predicate (`scrutinee_is_current_param || scrutinee_is_recursive_binding`) into ClusterDescentChecker::expr's match arm. Doc-comment cross- references descent_provable's authority to keep the parallel implementations grep-discoverable when one shifts. New regression `e_p_per_call_descent_evidence_classifies_nested_match_in_mutual_recursion_scc` exercises a 2-fn SCC where one side has nested-match descent. The fixture compiles only when ClusterDescentChecker accepts the nested-binding rule consistently with descent_provable. 7/7 e_p_per_call_descent_evidence_* tests green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BLOCKING addressed — commit 31b5f68codex finding verified: FixMirror the same New SCC regression
7/7 — sent from eager-bat-178 |
|
Review metadata
1. Story of the diffThis PR fixes a termination/descent false negative for recursive calls reached through nested matches. Previously the descent classifiers only recognized a match arm as structurally unpacking the recursive parameter when the branch scrutinee was exactly the parameter port; this missed shapes like The surface termination path gets the same rule in both places that can accept recursion: 2. Invariant categories
3. VerdictAPPROVE I do not see a diff-line-backed invariant violation. The PR broadens descent recognition by consuming existing structural facts, bounds the new trace so uncertain cases fail closed, mirrors the rule into the SCC checker, and adds targeted regressions for the exact false-negative shapes. |
|
Review metadata
Verdict: APPROVE — The diff is tightly scoped: bounded Exploratory observations (optional): |
…n range codex BLOCKING on PR #2198 (commit 69023a4): the prior Slice 5 follow-up extended the termination prover to accept any positive divisor > 1, but the descent-evidence carrier only materializes proportional shrink witnesses for `2..=MAX_PEANO_MATERIALIZATION` via `proportional_divisor_from_i64` (dag.rs:1045-1046). That created a parallel-authority split-brain: `f(n / 300)` would pass termination while the producer fail-closed to SubValueUnknown — exactly the single-authority discipline Slice 3 (#2182) cemented for `descent_provable` / `ClusterDescentChecker`. Cap `is_strictly_smaller`'s Div arm at the same `2..=MAX_PEANO_MATERIALIZATION` range the producer materializes. Both authorities now share the same acceptance boundary. New regression `..._proportional_descent_termination_matches_producer_acceptance_boundary` pins the boundary by rejecting `n / 257` at compile time. If either authority shifts the range, this test surfaces the divergence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… range codex BLOCKING inline finding on (closed) PR #2198 surfaced a PRE-EXISTING parallel-authority issue on main, symmetric to the Slice 5 Div issue but for Sub: `is_strictly_smaller`'s Sub arm accepts any positive integer literal, but the per-call descent producer's `positive_amount_from_i64` only materializes `PositiveDescentAmount` for `1..=MAX_PEANO_MATERIALIZATION` (dag.rs:1031-1032). So `f(n - 257)` would pass termination while the producer fails to materialize and falls back to `SubValueUnknown` — same single-authority discipline violation Slice 3 (#2182) cemented for descent_provable / ClusterDescentChecker. This issue predates Slices 1-5; Sub was the only ArithmeticOp the prover accepted, and its acceptance boundary was never tightened to match the producer. closed PR #2198 was a Div extension; this fix is the symmetric Sub cap. Cap `is_strictly_smaller`'s Sub arm at the same `1..=MAX_PEANO_MATERIALIZATION` range as the producer's materialization. Both authorities now share the same acceptance boundary on `param - k`. New regression `..._constant_descent_termination_matches_producer_acceptance_boundary` pins the boundary by rejecting `n - 257` at compile time. If either authority shifts the range, this test surfaces the divergence — same tripwire shape as the indirect-call cementing test (#2200). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… range (#2201) codex BLOCKING inline finding on (closed) PR #2198 surfaced a PRE-EXISTING parallel-authority issue on main, symmetric to the Slice 5 Div issue but for Sub: `is_strictly_smaller`'s Sub arm accepts any positive integer literal, but the per-call descent producer's `positive_amount_from_i64` only materializes `PositiveDescentAmount` for `1..=MAX_PEANO_MATERIALIZATION` (dag.rs:1031-1032). So `f(n - 257)` would pass termination while the producer fails to materialize and falls back to `SubValueUnknown` — same single-authority discipline violation Slice 3 (#2182) cemented for descent_provable / ClusterDescentChecker. This issue predates Slices 1-5; Sub was the only ArithmeticOp the prover accepted, and its acceptance boundary was never tightened to match the producer. closed PR #2198 was a Div extension; this fix is the symmetric Sub cap. Cap `is_strictly_smaller`'s Sub arm at the same `1..=MAX_PEANO_MATERIALIZATION` range as the producer's materialization. Both authorities now share the same acceptance boundary on `param - k`. New regression `..._constant_descent_termination_matches_producer_acceptance_boundary` pins the boundary by rejecting `n - 257` at compile time. If either authority shifts the range, this test surfaces the divergence — same tripwire shape as the indirect-call cementing test (#2200). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
T-E-P-Producer-Broadening Phase 1, Slice 3 of N. Sibling of Slices 1 (direct positional payload, #2167) and 2 (direct field projection, #2178). Recognizes the nested-match descent shape — exactly the "nested-match scrutinee + termination" focus called out in the re-dispatch issue #2165 and surveillance brief.
Two coordinated extensions
Termination prover (
lower.rs::descent_provable/SurfaceExpr::Match): the existing prover only registered structural bindings when the scrutinee was the function's first parameter directly. Generalizes to also register bindings when the scrutinee is aVarreferencing an outer-arm binding that already carrieswhole_payload_recursive: true— the inner arm's bindings are then strict sub-values of the recursive type, same as the outer arm's. Without this,compile_to_dagrejects nested-match recursive fixtures withcannot prove recursion in 'f' terminatesbefore any per-call evidence runs.Per-call descent producer (
dag.rs::scrutinee_traces_to_param): replaces the directbranch.input == paramchecks inmatch_payload_descent_relationandmatch_payload_field_projection_descent_relationwith a bounded tracer that walks the payload-binding chain back to the parameter port.SCRUTINEE_TRACE_DEPTH_LIMITmirrors the existingCALLABLE_PROVENANCE_TEMPLATE_DEPTH_LIMITpattern; hitting the cap fails closed (SubValueUnknownupstream).StrictSubValueis sound at any positive trace depth (every level peels one constructor), so the producer emits the innermost variant's structural facts.Cross-slice invariants reaffirmed
CallPatternvariant (P2 lookup authority preserved)TransformNodewidening (side-table only)InductiveFieldcomponents derived from authoritative DAG state — no user-binding-name leakage (test asserts the innermost-variant_0Conj label, NOT the user'st2binding)INVARIANTS.mdP1 substrate-fact-introduction triggerGate progress
e_p_per_call_descent_evidence_full_coverage(gate Lane B tasks #76, P1): partial — Slice 3 lands.SubValueUnknownrate strictly decreases on nested-match self-calls; termination prover now accepts the shape so they reach the producer at all (previously rejected at compile time).e_p_call_pattern_lookup_authoritative(gate Tasks.md actionable items #77, P2): unchanged.e_p_sub_value_relation_per_call_landed(gate Replace port-type heuristics with NodeKind classification #78, P3): unchanged.STOP-and-PING checks (none triggered)
CallPatternvariantTests
e_p_per_call_descent_evidence_classifies_nested_match_self_call_as_strict_sub_value— compiles a recursive 2-deepEpListNcount fixture and asserts:field_name == "_0"(innermost variant Conj label, NOT user bindingt2)variant_name == "EpConsN"type_name == "EpListN"element_type == "EpListN"factor == ShrinkFactor::UnitShrinke_p_per_call_descent_evidence_*tests green (Slice 1+2 tests preserved)cargo clippy -p v3-compiler --all-targets -- -D warnings: cleancargo fmt --all --check: cleancargo test -p v3-compiler --test integration: 839 pass / 5 fail (the same 5 pre-existing baseline failures verified non-regression on Slices 1 + 2; +2 new tests vs Slice 1's 837)Test plan
SubValueUnknown)countdown(n - 1)stillArithmeticDescent)Authority
docs/briefs/r3-t-e-p-producer-broadening-worker.md(PR R3 Substrate #1782, MERGED 2026-05-06)🤖 Generated with Claude Code