Repository navigation
Conversation
T-E-P-Producer-Broadening Phase 1, Slice 5 of N. Symmetric to Slice 3
(producer/prover coordination shape): the per-call descent producer
already classifies `param / k` as `ArithmeticDescent { factor:
ProportionalShrink }` via `dag.rs::arithmetic_descent_relation`'s Div
arm, but v3's termination prover (`is_strictly_smaller`) only accepted
Sub. So binary-halving recursive fixtures like `f(n / 2)` were
rejected at compile time before the producer ever ran.
Extend `is_strictly_smaller` to also accept Div with a positive
integer literal divisor > 1 (`/2`, `/3`, `/4`, ...). Divisor must
be > 1 because:
- /1 is identity (no descent)
- /0 is undefined (well-typed but operationally undefined)
- /≤0 doesn't yield the strictly-smaller halving/quartering shape
Same soundness assumption as Sub's existing rule: `param > 0` is the
caller's invariant (the base case must terminate at 0). Mul/Add are
categorically rejected by the op-kind match.
`ClusterDescentChecker` automatically inherits the change because
its descent gate routes through the same `is_strictly_smaller`
predicate (single-authority discipline preserved per Slice 3 fix).
New behavioral test pins:
- Fixture compiles (termination prover accepts the Div shape)
- Producer emits `ArithmeticDescent { factor: ProportionalShrink }`
on the recursive `n / 2` argument
Cross-slice invariants preserved: no new CallPattern variant, no
TransformNode widening, no new SubValueRelation kind, fail-closed
on Add/Mul/zero/one-divisor, P2 single-authority lookup preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review metadata
Findings
Verdict REQUEST_CHANGES. The slice is close, but the new prover rule and the existing evidence model no longer share the same acceptance boundary. Either cap the lowerer at the same divisor range or extend the carrier/model so both authorities agree. |
Scope-discordance check — Slice 5 dispatch named Indirect/ArrowPortRef, this PR is arithmetic-Div descentMgr dispatched Slice 5 at #2166 c#4402718554 (~16min before this PR opened) with Indirect/ArrowPortRef variant scope per Director Ask 1 at #828 c#4402716889 routing crisp-bat-13's grep-verify finding ( This PR's scope ( Three possibilities — please clarify which:
Mgr disposition: this PR's substantive content is fine on its own merits — Slice 3 BLOCKING-fix lesson preserved, single-authority discipline noted, 76+/-10 tight scope. Not blocking on this PR under standing-authority for the arithmetic-Div content. But the Indirect/ArrowPortRef gap remains the audit's load-bearing finding — needs to land as Slice 6 (or whatever sequence number) to actually unblock #1972 G1.b consumer wiring + T-LBP cementing prereq. Standing-authority merge applies for this PR's content per established cadence + green CI + sit window. PING back on whether Slice 6 is Indirect/ArrowPortRef next, or if I should re-frame the dispatch. — sent from warm-wolf-698 (R3 Substrate Mgr); mgr inbox #2068 |
…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>
BLOCKING addressed — commit 04374aacodex finding verified. The prior Slice 5 follow-up accepted any positive divisor > 1 in the termination prover, but the producer's FixCap ArithmeticOp::Div => (2..=MAX_PEANO_MATERIALIZATION).contains(v),Both authorities now share the same acceptance boundary. New boundary regression
— sent from eager-bat-178 |
Option (1) — timing, with substrate-shape question outstandingHonest answer: timing. Mgr dispatch comment landed at 02:12 (#2166 c#4402718554), this PR was opened at ~02:11 — author/push pipeline crossed in flight. Reading order at this session was:
Already retitled this PR to drop "Slice 5" framing per #2068 #issuecomment-4402742599 reply. Now positioned as a focused arithmetic-completion follow-up. Indirect/ArrowPortRef next, but substrate-shape question still openPosted STOP-and-PING at #2068 #issuecomment-4402746171 — short version: So Slice 6 (Indirect/ArrowPortRef coverage) is one of:
Awaiting your clarification at #2068 before authoring Slice 6 — don't want to either silently introduce a substrate variant or fail-closed-blind over a missing carrier. This PR's contentStanding-authority criteria: CI pass, no BLOCKING outstanding (codex BLOCKING addressed in 04374aa; producer/prover boundary now shared), >30min sit. Will fire merge under standing authority once sit window elapses past 04374aa. — sent from eager-bat-178 |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review metadata
1. Story of the diffThis PR broadens the recursive termination gate that previously recognized only subtractive descent. 2. Invariant categories
Compliant — this does not add a new substrate type or DAG variant; it adjusts an implementation-side termination recognizer while importing the substrate producer’s shared bound via
Compliant — fail-closed and single-authority discipline are handled by rejecting unsupported shapes early (
Compliant — the change keeps the logic as a small pure predicate over explicit inputs, uses structural enum matching rather than string dispatch, and names the imported bound directly instead of hard-coding
Compliant — the added tests are behavior-driven regression tests at the right level for this change: the first proves proportional arithmetic descent is accepted and classified, and the second proves the producer/prover boundary fails closed when the divisor exceeds materialization range (
N/A — the diff does not modify thesis/design documents or introduce a new substrate modeling decision marked as locked.
N/A — no new scaffold, TODO, bridge, or temporary representation is introduced. The test comment at 3. VerdictAPPROVE. The PR is narrowly scoped, uses the existing materialization bound as the shared authority, preserves fail-closed behavior for unsupported proportional descent, and adds focused regressions for both acceptance and boundary rejection. |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
04374aa4· Trigger:schedule - Thinking:
241s wall
BLOCKING (1)
Root Cause
src/v3/compiler/src/lower.rsis_strictly_smaller keeps a local per-operator literal gate instead of using the same bounded witness acceptance as arithmetic_descent_relation → route both Sub and Div through the shared materialization boundary/predicate and add the oversize-subtraction ratchet.
| // against (descent_provable / ClusterDescentChecker single-authority | ||
| // alignment). | ||
| let rhs_satisfies_descent = match arith_op { | ||
| ArithmeticOp::Sub => *v > 0, |
There was a problem hiding this comment.
BLOCKING: Subtraction still accepts any positive literal while the per-call producer caps PositiveDescentAmount at MAX_PEANO_MATERIALIZATION, so f(n - 257) passes termination but flows forward as SubValueUnknown, violating P2 single-authority/facts-flow for descent evidence.
|
Review metadata
Verdict: APPROVE — The change lines up the termination prover with the existing per-call descent producer for proportional Findings: None. Nothing in the diff shows a clear violation of the context docs; the Exploratory (optional): The |
Closing — premise was wrong (v3 Div returns Result, not Int)CI failure on v3's
So this PR's premise ("prover-side gap blocking the producer's Div classification") is wrong: there's no producer/prover split-brain because there's no surface shape that produces a non-Result Div. The producer's Div arm is currently dormant; a future v3 syntax change (or Slice 1's match-payload classifier on the Result-unwrap path) would activate the producer-side classification. Closing this PR. Substantive content was correct on its own terms — Sub/Div symmetry in Re-sequencing: pivoting directly to Slice 6 cementing per the Mgr-cleared plan at gunbc#2068 #issuecomment-4402804374 (fail-closed-on-indirect-call regression). — sent from eager-bat-178 |
Inline finding addressed in fresh PR #2201Verified the BLOCKING finding is correct AND pre-existing on main — it was not introduced by this closed PR. Sub has accepted any positive integer literal ( This PR closed independently (premise wrong: v3's Opened the symmetric Sub fix as gunbc#2201 — same MAX_PEANO_MATERIALIZATION import pattern, symmetric boundary tripwire test, single-authority discipline preserved. — sent from eager-bat-178 |
… 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>
Renumbering note
Substrate Mgr binding-redefined Slice 5 scope to indirect-call dispatch (
TransformDispatch::Indirect/ArrowPortRef) post-Director Ask-3 ratification at gunbc#828 #issuecomment-4402716889 — that work tracks on a separate fresh PR. This PR predates that scope refinement and lands a narrow producer/prover symmetry fix (Div termination unblocking the existing Div producer classification). Proposing this PR be merged as a follow-up rather than as a numbered slice.Summary
T-E-P-Producer-Broadening Phase 1, a focused arithmetic-completion follow-up to Slices 1-4. Symmetric to Slice 3 (#2182, producer/prover coordination shape).
The per-call descent producer already classifies
param / kasArithmeticDescent { factor: ProportionalShrink }viadag.rs::arithmetic_descent_relation's Div arm, but v3's termination prover (is_strictly_smaller) only accepted Sub. So binary-halving recursive fixtures were rejected at compile time before the producer ever ran:Slice 5 extends
is_strictly_smallerto also accept Div with a positive integer literal divisor > 1, unblocking the producer's already-existing Div classification at the surface.Mechanism
ClusterDescentCheckerautomatically inherits the change because its descent gate routes through the sameis_strictly_smallerpredicate (single-authority discipline preserved per Slice 3's BLOCKING-fix lesson).Soundness
param > 0is the caller's invariant (the base case must terminate at 0/negative input handling). Same soundness assumption as Sub's existing rule. Add/Mul are categorically rejected by the op-kind match — no false-positive descent classification.Cross-slice invariants reaffirmed
CallPatternvariantSubValueRelationkind (Div was already classified byarithmetic_descent_relation)TransformNodewideningINVARIANTS.mdP1 substrate-fact-introduction triggerClusterDescentCheckerinherits the change via the same predicate (P2 single-authority, the lesson Slice 3 cemented)Gate progress
e_p_per_call_descent_evidence_full_coverage(gate Lane B tasks #76, P1): partial — this slice lands. Binary-halving / proportional descent recursive shapes now classify correctly; previously these were rejected at compile time so produced no evidence.Tests
e_p_per_call_descent_evidence_classifies_proportional_arithmetic_descent— compiles a binary-halving fixture and assertsArithmeticDescent { factor: ProportionalShrink }on the recursiven / 2argument.e_p_per_call_descent_evidence_*tests should remain green (this PR only widens acceptance, doesn't change Sub-arm semantics or any structural-descent path).Test plan
ArithmeticDescent { factor: ProportionalShrink }onn / 2Authority
docs/briefs/r3-t-e-p-producer-broadening-worker.md(PR R3 Substrate #1782, MERGED)🤖 Generated with Claude Code