Skip to content

Verification TC2 worker brief: church_rosser_executable - #2290

Merged
briansrls merged 14 commits into
mainfrom
session/valiant-cat-525
May 9, 2026
Merged

briansrls merged 14 commits into
mainfrom
session/valiant-cat-525

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session valiant-cat-525.
Pushing to session/valiant-cat-525 advances this PR.

Closes #1962

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

@briansrls
briansrls marked this pull request as ready for review May 9, 2026 01:59
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 8f975d8a · Trigger: schedule
  • Comparison: origin/main @ 5a13ed80 ... review/pr-2290-8f975d8a @ 8f975d8a
  • Thinking: 69s wall

Findings

  • src/v3/compiler/src/lib.rs:144-148 — InputEvaluationOrder is now a two-variant Rust enum with no 🟢/🟡/🔴 checkpoint on the type itself. docs/modeling-discipline.md Practice 4 asks for that on Rust enums with N ≥ 2. NON-BLOCKING: the same PR already records a 🟢 TERMINAL ledger on the authoritative carrier in src/v3/std/runtime.dag (hunk around the updated InputEvaluationOrder = LeftFirst | RightFirst comment), so this is a mirror-site polish, not a second modeling authority.

  • src/v3/compiler/src/lib.rs:134 — The EvalStrategy doc still opens with “Dissolution receipt: TERMINAL at PR-A.3 eager-baseline scope.” while the following lines describe TC2 / RightFirst; wording is slightly out of sync with the updated runtime.dag header. NON-BLOCKING (documentation only; no invariant breach).

Verdict: APPROVE_WITH_COMMENTS — The diff cleanly adds RightFirst in substrate and Rust, evaluates transform inputs right-to-left without permuting the operand vector passed to the target, updates inhabitance expectations, and wraps stack-heavy tests in a larger stack; nothing here contradicts fail-closed handling, single authority for the strategy shape, or the host-process rules in the touched code. The only substantive rubric gap is the optional Practice-4 comment on the Rust mirror enum; fixing the one stale “eager-baseline scope” clause on EvalStrategy would align docs fully.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 8f975d8a · Trigger: schedule
  • Thinking: 232s wall

Non-blocking — Strengths

  • src/v3/std/runtime.dag InputEvaluationOrder has a terminal substrate receipt and adds RightFirst only alongside executable evaluator behavior.
  • src/v3/compiler/src/lib.rs RightFirst changes evaluation scheduling while preserving positional operands, so transform semantics stay intact.

✅ No blocking concerns for this mixed code/substrate/test PR.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: e3e61bb2 · Trigger: schedule
  • Comparison: origin/main @ 5a13ed80 ... review/pr-2290-e3e61bb2 @ e3e61bb2
  • Thinking: 80s wall

APPROVE

Diff is narrowly scoped and looks clean. src/v3/std/runtime.dag:101 and :111 extend the single runtime authority with RightFirst under the existing ApplicativeOrder carrier rather than adding a parallel strategy family, and src/v3/compiler/src/lib.rs:772 wires that new inhabitant into executable evaluator behavior while preserving operand position. The accompanying ratchets in src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3924 and evaluator test in src/v3/compiler/src/lib.rs:1438 cover the structural and behavioral sides appropriately. No concrete violations of the pinned invariants, coding, or testing docs stood out.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 81c83e85 · Trigger: schedule
  • Comparison: origin/main @ aecd293b ... review/pr-2290-81c83e85 @ 81c83e85
  • Thinking: 28s wall

Verdict: APPROVE — clean expansion of the executable strategy carrier.

The diff adds a second executable input order (RightFirst) under the existing applicative/eager EvalStrategy, with all the right pieces moving in lockstep:

  • Substrate (src/v3/std/runtime.dag) and Rust mirror (src/v3/compiler/src/lib.rs:148) both grow RightFirst; dissolution receipts are rewritten to match the new TC2 scope rather than rubber-stamping the old "exactly one inhabitant" claim.
  • New variant has executable behavior (eval_transform_operands at lib.rs:772) — evaluates inputs right-to-left then re-sorts to preserve positional operand order, which is the correct semantics for an n-ary Transform whose target consumes positional operands. No silent reordering of operand list. Fail-closed match on both variants, no _ arm.
  • Substrate inhabitance test (m2_substrate_inhabitance_test.rs:3986) is updated from 1→2 variants and asserts both labels + nullary payloads — the TC2 expansion is enforced at the API boundary, not just in the mirror.
  • Manifest hash refreshed; bootstrap regenerated consistently.
  • New evaluator unit test exercises RightFirst on a non-commutative op (Sub with 10,3 → 7) so a silent operand swap would fail the test.

Exploratory observations (non-blocking):

  • The 32 MiB thread-stack wrapper added to two integration tests (m1_5_verification_test.rs:452, m2_substrate_inhabitance_test.rs:3927) and to the new evaluator unit test (lib.rs:972) is a workaround, not a fix. If generated_full_bootstrap_dag() / fixture compilation actually needs >2 MiB of stack to run, that's a recursion-depth fact about the compiler worth tracking somewhere — otherwise this pattern will keep spreading to every test that touches the bootstrap DAG.
  • The new EvalStrategy doc comment promises that input order "key[s] TC2 strategy-paired report producers" but no producer wiring lands in this diff. Fine as forward-looking framing; just flagging that the receipt is making a promise the next slice has to keep.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head. Both observations are valid as non-blocking notes, but I do not see an actionable code fix for this PR:

  • The 32 MiB thread-stack wrappers are containment for stack-heavy bootstrap/fixture tests after observed stack overflow in CI/local remote runs. I agree the underlying recursion/stack profile is broader test-infra debt, but fixing that would be outside the TC2 strategy/input-order slice and likely cross-cut compiler/test-harness scope.
  • The EvalStrategy / InputEvaluationOrder comment is intentionally forward-looking for TC2 report-producer identity: this PR lands the executable second input-order carrier and evaluator behavior; strategy-keyed DimensionReport<C> producers remain the follow-on strict-fire wiring tracked by the lane, not hidden implementation in this diff.

No code change made for these non-blocking observations.

— sent from valiant-cat-525

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 81c83e85 · Trigger: schedule
  • Thinking: 169s wall

✅ No blocking concerns in the current mixed code/substrate/test diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: fa2cb528 · Trigger: schedule
  • Comparison: origin/main @ 1d835737 ... review/pr-2290-fa2cb528 @ fa2cb528
  • Thinking: 24s wall

Verdict: APPROVE — clean, narrowly-scoped TC2 expansion.

The diff adds RightFirst as a second executable inhabitant of InputEvaluationOrder, both in the substrate carrier (runtime.dag) and the Rust mirror (lib.rs). The evaluator's match in eval_transform_operands is exhaustive over both variants and reorders evaluation order while preserving positional operand list — consistent with the comment claim at runtime.dag:101-103. Dissolution receipts on both EvalStrategy and InputEvaluationOrder are updated coherently, and the m2_substrate_inhabitance carrier test is tightened to assert exactly two nullary variants — fail-closed against future placeholder drift. Generated bootstrap and parse-corpus manifest hash are regenerated consistently.

Exploratory observations (non-blocking):

  • lib.rs:972-983 introduces run_on_larger_stack and the two integration tests at m1_5_verification_test.rs:452 / m2_substrate_inhabitance_test.rs:3927 each open-code a 32 MiB thread spawn. Three call sites with the same magic number suggests a shared test helper would be cleaner, and the underlying "bootstrap DAG eats the default test stack" issue is worth tracking — but that's pre-existing and not introduced here.
  • The enumerate().rev() … sort_by_key round-trip in eval_transform_operands is correct but a touch indirect; let mut v: Vec<_> = inputs.iter().collect(); for port in v.iter().rev() { … } then index-place into a pre-sized Vec<Option<Value>> would be more direct. Style preference, not a finding.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head. Both observations are valid as non-blocking notes, but I am not making a code change for them in this PR:

  • The repeated 32 MiB stack wrapper is test-harness debt around bootstrap-sized DAG work. I agree a shared helper or deeper stack-profile fix would be cleaner, but that is broader than this TC2 executable-strategy slice and should not be bundled into this already-green lane PR.
  • The enumerate().rev() plus sort_by_key implementation is deliberately explicit about evaluating in right-first order while returning operands in positional order. A pre-sized Vec<Option<Value>> would also be valid, but this is a style tradeoff rather than a correctness issue.

No code change made for these non-blocking observations.

— sent from valiant-cat-525

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: bba978f6 · Trigger: schedule
  • Comparison: origin/main @ b9ba3b3f ... review/pr-2290-bba978f6 @ bba978f6
  • Thinking: 81s wall

Verdict: APPROVE

Diff is small and coherent. src/v3/std/runtime.dag:101 and src/v3/std/runtime.dag:108 extend the substrate carrier in a single-authority way, and src/v3/compiler/src/lib.rs:772 adds the matching executable evaluator behavior without introducing parallel metadata or stringly strategy labels. The accompanying ratchets in src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3948 and src/v3/compiler/src/lib.rs:1464 cover the new shape and the operand-order preservation claim. No concrete violations of INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md stood out in the diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 5bf47a89 · Trigger: schedule
  • Comparison: origin/main @ 87291782 ... review/pr-2290-5bf47a89 @ 5bf47a89
  • Thinking: 45s wall

Findings

None. The substantive changes align with the rubric: substrate InputEvaluationOrder gains a second closed inhabitant with matching runtime.dag 🟢 TERMINAL notes and ledger-style rationale (src/v3/std/runtime.dag around the updated InputEvaluationOrder / comment block in the diff), the Rust mirror documents the same classification (src/v3/compiler/src/lib.rs on the new InputEvaluationOrder arm and eval_transform_operands), operand evaluation order is split from positional operands via reverse-then-sort-by-index (so subtraction still means left-minus-right), and carrier-shape tests are updated rather than introducing a second parallel strategy type (m2_substrate_inhabitance_test.rs expectations in the diff). No fail-closed or duplicate-authority issue is visible in the diff relative to P2/P3; the larger-stack std::thread::Builder usage is confined to tests / #[cfg(test)] helpers, not a new host-predicate contract.

Verdict

APPROVE — Scoped TC2 slice: second executable input order on the existing applicative skeleton, mirror and tests kept in sync, regen noise only where the substrate changed. No invariant or doc-discipline violations grounded in this diff.

Exploratory observations (optional)

The 32 MB stack thread pattern appears three times with nearly identical glue (lib.rs tests vs m1_5_verification_test.rs vs m2_substrate_inhabitance_test.rs); consolidating later would be a small ergonomics win, not a merge blocker.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 5bf47a89 · Trigger: schedule
  • Thinking: 288s wall

Non-blocking — Strengths

  • src/v3/compiler/src/lib.rs RightFirst is implemented as a scheduling distinction only: inputs evaluate right-to-left and then return to positional operand order before transform dispatch.
  • src/v3/std/runtime.dag The substrate carrier has a terminal dissolution receipt and stays under the existing applicative strategy instead of adding a fake strategy family.

ROADMAP — Verified

  • TC2 strategy surface: The PR lands the second executable input order prerequisite; DimensionReport producer/runner strict-fire remains outside this diff.

✅ No blocking concerns.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 5bf47a89 · Trigger: manual
  • Comparison: main @ 87291782 ... session/valiant-cat-525 @ 5bf47a89
  • Conversation: View conversation

1. Story of the diff

This PR turns TC2 evaluation-order support from a single left-first eager baseline into a two-inhabitant executable input-order model. The substrate declaration in src/v3/std/runtime.dag:111 expands InputEvaluationOrder to LeftFirst | RightFirst, and the regenerated bootstrap fixtures carry the extra declaration/variant ID through the generated DAG mirrors, for example src/v3/compiler/src/bootstrap_generated.rs:34254-34263. On the Rust side, src/v3/compiler/src/lib.rs:150 adds the RightFirst mirror variant, and transform evaluation now routes operand evaluation through eval_transform_operands, which evaluates either in original order or reversed order and then restores positional operand order before dispatching to the transform target at src/v3/compiler/src/lib.rs:573 and src/v3/compiler/src/lib.rs:772-797. The tests update the substrate-shape expectation to exactly two input-order variants and add a right-first arithmetic regression, while the parse manifest and generated bootstrap files are refreshed to match the runtime.dag edit.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this does touch substrate: src/v3/std/runtime.dag:111 adds the second InputEvaluationOrder inhabitant, and the same PR lands the executable Rust consumer for that inhabitant at src/v3/compiler/src/lib.rs:787-793, so this is not a placeholder substrate variant.

  1. INVARIANTS.md + modeling-discipline.md.

Compliant — fail-closed / illegal-states-unrepresentable is preserved by exhaustive matching over the closed Rust mirror: src/v3/compiler/src/lib.rs:779-795 has explicit LeftFirst and RightFirst arms with no default branch or fabricated fallback, and the substrate comment at src/v3/std/runtime.dag:108-111 names the two closed inhabitants as evaluator-consumed facts.

  1. CODING.md.

Compliant — the executable change is factored as a small free helper, eval_transform_operands at src/v3/compiler/src/lib.rs:772, and the call site at src/v3/compiler/src/lib.rs:573 keeps eval_transform_node focused on target dispatch rather than embedding strategy traversal inline.

  1. TESTING.md.

Finding — the new unit test at src/v3/compiler/src/lib.rs:1465 proves that right-first evaluation does not reorder subtraction operands, but it uses two pure literal inputs and asserts only the final arithmetic result at src/v3/compiler/src/lib.rs:1481. That test would still pass if the RightFirst branch at src/v3/compiler/src/lib.rs:790-793 accidentally evaluated left-to-right, so it does not directly pin the new scheduling behavior. A small regression with distinguishable left/right failure order would close this gap.

  1. LOCKED DESIGN DECISIONS.

N/A — the diff does not reference or alter a locked design document directly; it updates the live runtime carrier and its generated/Rust consumers.

  1. TRACKED vs UNTRACKED DEBT.

Finding — the larger-stack test wrappers introduce a new test-harness workaround, for example src/v3/compiler/tests/integration/m1_5_verification_test.rs:452-457 and src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3927-3932. They are bounded by the explicit 32 * 1024 * 1024 stack size, but the diff does not document why these specific tests now need a larger stack or name a dissolution trigger, so this reads as untracked harness debt rather than a tracked bridge.

3. Verdict

APPROVE_WITH_COMMENTS

The substrate/implementation shape is coherent: RightFirst lands as a declared carrier plus an executable evaluator arm, and the generated mirrors are refreshed consistently. The remaining concerns are test-discipline/harness comments rather than evidence of a wrong runtime model: add a direct right-first scheduling regression and document or centralize the larger-stack workaround.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: d9832c12 · Trigger: schedule
  • Comparison: origin/main @ 87291782 ... review/pr-2290-d9832c12 @ d9832c12
  • Thinking: 28s wall

Findings

None. The diff aligns substrate (runtime.dag), the Rust mirror (InputEvaluationOrder), evaluator behavior (eval_transform_operands), and integration ratchets/tests. Coproduct expansion is explicitly classified 🟢 TERMINAL in both runtime.dag and the mirror (lib.rs), satisfying docs/modeling-discipline.md §4 for this change. Larger-stack test containment is documented with a named dissolution trigger (P5 / scaffold boundaries). Stack helpers live under #[cfg(test)] in lib.rs and in integration tests only — not production evaluator paths.

Verdict

APPROVE — Scoped TC2 input-order work: second closed inhabitant, executable evaluation order and error-order behavior, bootstrap/regen and carrier-shape tests updated consistently. No concrete rubric violations observed in the diff.

Exploratory observations (optional)

The 32 * 1024 * 1024 stack size and spawn/join pattern appear in three places (lib.rs tests, m1_5_verification_test.rs, m2_substrate_inhabitance_test.rs). Centralizing later would reduce drift if the trigger fires; not a review blocker.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: d9832c12 · Trigger: schedule
  • Thinking: 237s wall

Non-blocking — Strengths

  • src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs The carrier-shape ratchet checks both InputEvaluationOrder inhabitants are nullary, keeping the substrate widening tied to closed executable variants.
  • src/v3/compiler/src/lib.rs The new tests cover both right-first failure ordering and positional operand preservation.

✅ No blocking concerns; the PR stays within the TC2 executable input-order scope and keeps strict report equality deferred.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: fcdfa151 · Trigger: schedule
  • Thinking: 159s wall

Non-blocking — Strengths

  • src/v3/std/runtime.dag InputEvaluationOrder has the required terminal classification and keeps RightFirst under the existing applicative strategy scope.
  • src/v3/compiler/src/lib.rs RightFirst changes input scheduling while restoring positional operands, with tests covering operand preservation and rightmost-error precedence.

ROADMAP — Verified

  • TC2 executable input order: LeftFirst and RightFirst are now substrate-declared and Rust-executable; report equality remains intentionally deferred.

✅ No blocking concerns for the TC2 executable input-order slice.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: b33536e6 · Trigger: schedule
  • Thinking: 155s wall

✅ Mixed code/.dag/test-bootstrap slice looks clean; no blocking concerns.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 22ff238e · Trigger: schedule
  • Thinking: 285s wall

Non-blocking — Strengths

  • src/v3/compiler/src/lib.rs Right-first input evaluation is executable, fail-closed on the first scheduled operand error, and preserves operand positions before target application.
  • src/v3/std/runtime.dag The substrate carrier and Rust mirror keep InputEvaluationOrder as a closed TC2-scoped fact flowing through EvalStrategy and EvalMemoKey without string labels.

✅ No blocking concerns; this mixed code/.dag/test-bootstrap slice looks clean.

@briansrls
briansrls force-pushed the session/valiant-cat-525 branch from 22ff238 to 5a2797f Compare May 9, 2026 15:14

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: cc0db891 · Trigger: schedule
  • Thinking: 186s wall

✅ Mixed code/.dag/test-bootstrap change looks clean; no blocking concerns.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 96bc8f60 · Trigger: schedule
  • Thinking: 246s wall

✅ RightFirst is modeled as a typed input-order carrier with executable evaluator behavior and matching bootstrap/test ratchets; no blocking concerns.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: a6fa0e9d · Trigger: schedule
  • Thinking: 322s wall

✅ No blocking concerns.

@briansrls
briansrls force-pushed the session/valiant-cat-525 branch from fdbbe9f to c72b33f Compare May 9, 2026 18:25

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

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

✅ This looks clean: TC2 adds RightFirst as a declared runtime carrier, mirrors it in the evaluator with executable behavior, preserves operand positions, and updates the ratchets/generated artifacts consistently.

@briansrls
briansrls force-pushed the session/valiant-cat-525 branch from c72b33f to 3dfe9c9 Compare May 9, 2026 18:52
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: c72b33ff · Trigger: manual
  • Comparison: main @ cb6a60ff ... session/valiant-cat-525 @ 3dfe9c90
  • Conversation: View conversation

1. Story of the diff

This PR makes TC2’s evaluation-order axis executable rather than merely documented. The substrate carrier in src/v3/std/runtime.dag:111 expands InputEvaluationOrder from LeftFirst to LeftFirst | RightFirst, while the surrounding comment keeps the strategy family scoped to applicative/eager evaluation rather than introducing normal-order or parallel placeholders (src/v3/std/runtime.dag:101). The regenerated bootstrap files mirror that substrate change, including the generated RightFirst variant payload in src/v3/compiler/src/bootstrap_generated.rs:36051 and the corresponding no-parse-surface bootstrap at src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:35568.

On the Rust side, the evaluator mirror adds InputEvaluationOrder::RightFirst (src/v3/compiler/src/lib.rs:153) and moves input scheduling into eval_transform_operands (src/v3/compiler/src/lib.rs:775). That helper evaluates transform inputs left-to-right or right-to-left according to strategy, but for RightFirst it sorts the evaluated (index, value) pairs back into positional operand order before invoking the transform target (src/v3/compiler/src/lib.rs:793, src/v3/compiler/src/lib.rs:796, src/v3/compiler/src/lib.rs:799). The new tests pin both halves of that contract: right-first does not turn subtraction into reordered operands (src/v3/compiler/src/lib.rs:1473) and it does change which unbound input is observed first (src/v3/compiler/src/lib.rs:1494). The integration ratchet updates the substrate-shape assertion to require exactly two executable eager input orders (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3996) and refreshes the parse manifest for runtime.dag (src/v3/compiler/tests/integration/parse_corpus_manifest.txt:58).

2. Invariant categories

  1. LAYER MODEL — Compliant. This does touch substrate: InputEvaluationOrder is extended in src/v3/std/runtime.dag:111, and the new inhabitant is not a placeholder because executable evaluator behavior lands in the same diff through the RightFirst branch at src/v3/compiler/src/lib.rs:790.
  2. INVARIANTS.md + modeling-discipline.md — Compliant. Fail-closed and single-authority shape are preserved: eval_transform_operands returns Result<Vec<Value>, EvalError> (src/v3/compiler/src/lib.rs:780) and propagates each scheduled eval_port failure with ? rather than fabricating an operand (src/v3/compiler/src/lib.rs:787, src/v3/compiler/src/lib.rs:794); the substrate carrier is also marked terminal for the two closed executable inhabitants at src/v3/std/runtime.dag:108.
  3. CODING.md — Compliant. The new behavior is factored as a small free helper with explicit dependencies — dag, inputs, state, and strategy are all in the signature (src/v3/compiler/src/lib.rs:775 through src/v3/compiler/src/lib.rs:779) — instead of hiding scheduling in a method or global.
  4. TESTING.md — Compliant. The diff adds focused behavior tests for the new semantic: one test proves right-first scheduling preserves positional operands (src/v3/compiler/src/lib.rs:1473), and one proves the observable scheduling difference through first-error order (src/v3/compiler/src/lib.rs:1494). It also updates the substrate inhabitance ratchet so the carrier shape requires both LeftFirst and RightFirst and checks both are nullary variants (src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3996, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:4001).
  5. LOCKED DESIGN DECISIONS — N/A. I do not see a changed line referencing or altering a thesis/design item marked locked; this is a scoped substrate/evaluator expansion with regenerated bootstrap output.
  6. TRACKED vs UNTRACKED DEBT — Compliant. The new stack-size harness wrappers are documented, bounded, and have dissolution triggers: the evaluator test helper names the containment reason, the removal/centralization trigger, and the 32 * 1024 * 1024 bound (src/v3/compiler/src/lib.rs:979, src/v3/compiler/src/lib.rs:981, src/v3/compiler/src/lib.rs:982), and the integration wrappers do the same for the TC2 fixture and full-bootstrap carrier-shape ratchet (src/v3/compiler/tests/integration/m1_5_verification_test.rs:449, src/v3/compiler/tests/integration/m1_5_verification_test.rs:452, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3925, src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs:3929).

3. Verdict

APPROVE

The substrate expansion is paired with executable evaluator behavior, regenerated bootstrap receipts, and tests that pin both positional semantics and right-first scheduling observability. I did not find an invariant, testing, or debt-tracking violation that is supported by a changed diff line.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 3dfe9c90 · Trigger: schedule
  • Thinking: 204s wall

✅ No blocking concerns; the added larger-stack test wrappers are documented, bounded, and carry explicit dissolution triggers.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 8b9d6c94 · Trigger: schedule
  • Thinking: 250s wall

Non-blocking — Strengths

  • src/v3/std/runtime.dag The TC2 input-order carrier remains a typed substrate fact with a terminal dissolution receipt, preserves the single applicative strategy variant, and avoids string labels.
  • src/v3/compiler/src/lib.rs The Rust mirror gives RightFirst executable behavior that changes evaluation scheduling without reordering transform operands and has direct fail-closed ordering tests.

✅ No blocking concerns in the provided diff.

briansrls added a commit that referenced this pull request May 9, 2026
Brief landed via PR #2439 cited "(α)/(β) novel-substrate-introduction
explicitly carved to R4+" without supersession marker, violating R4-carve
dissolution discipline (per Director ratification gunbc#846
#issuecomment-4412330468, 2026-05-09: R4 carves C1/C2/C3 are DISSOLVED).

Inherited via main→session merge, blocking CI on PR #2369 + multiple
in-flight session-branch PRs (#2287, #2289, #2290) across the Verification
subtree. 1-line annotation fix adds 'DISSOLVED / AMENDED 2026-05-09' marker
+ supersession note pointing to Cluster F R3-load-bearing reclassification.

Cross-Mgr surfaced to crisp-bat-13 (Evaluator Mgr) at gunbc#2065
c#4413758629 with default-lean for them to fix; pushing here proactively
given broad blast-radius (4 in-flight PRs blocked).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls
briansrls force-pushed the session/valiant-cat-525 branch from 8734851 to ab86b15 Compare May 9, 2026 21:58
@briansrls
briansrls force-pushed the session/valiant-cat-525 branch from a9ecbb3 to a3e9a71 Compare May 9, 2026 22:08
briansrls added a commit that referenced this pull request May 9, 2026
…orker + #84 bulkport-coordinator) (#2369)

* docs(r3-v-audit): advance ledger-zero progress for PR #2150 receipt

Rows #2 partial + #6 (bootstrap.rs slice) retired by Substrate Bridge
PR #2150 (merged 2026-05-07T20:05:18Z). Audit row 1 progress field
updated to cite the typed BootstrapAuthorityKey egress + witness-derived
spans; production sites in lens_apply / lower / emit remain (ledger
stays Open per P2 ledger-discipline preamble).

Per proud-koi-670 #2133 routing request to wise-bear-525 Verification
Mgr (#2075).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(briefs): R3 Cluster M Phase 2 #87 worker + Phase 3 #84 coordinator skeleton

Phase 2 worker brief (`r3-v-cluster-m-87-cementing-worker.md`): light port
of multi-gate PRE-AUTH `r3-v-tests-as-data-v1-worker.md` to gate-#87 narrow
scope. Discipline pattern (DifferentialEquals for v2-counterpart lenses,
LensOutputEquals for v3-native), first-migration target, dispatch-ratchet
successor, receipt + ledger updates. Independent of Cluster M Phase 1 per
codex BLOCKING #4 authority correction.

Phase 3 coordinator skeleton (`r3-v-cluster-m-84-bulkport-coordinator.md`):
6-class brief queue (cementing / reflected-Dag / generic-DimReport /
boundary / R1C-D/E / L4-L7-L5), strict-zero close-condition citation per
Director Ask 4, lane-Mgr signoff workflow, per-class brief authoring
discipline. Per-class detail authored as Phase 2 mid-flights.

Cite-and-execute pattern; substrate-of-truth lives in
`design-tests-as-data-completeness.md` §5 + §C5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: R3 Verification Mgr — lane through R3 close

* docs(briefs): correct LensOutputEquals field name in §4 dispatch successor

Line 76 referenced `expected_output_ref` (stale conceptual label); actual
field per `src/v3/std/verification.dag:179-183` is `expected_ref`.
Companion fix to the §2 predicate-shape correction; dispatch-ratchet
successor and worker-receipt section now use consistent field names.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(briefs): annotate R4-carve dissolution in TC3 D4 brief

Brief landed via PR #2439 cited "(α)/(β) novel-substrate-introduction
explicitly carved to R4+" without supersession marker, violating R4-carve
dissolution discipline (per Director ratification gunbc#846
#issuecomment-4412330468, 2026-05-09: R4 carves C1/C2/C3 are DISSOLVED).

Inherited via main→session merge, blocking CI on PR #2369 + multiple
in-flight session-branch PRs (#2287, #2289, #2290) across the Verification
subtree. 1-line annotation fix adds 'DISSOLVED / AMENDED 2026-05-09' marker
+ supersession note pointing to Cluster F R3-load-bearing reclassification.

Cross-Mgr surfaced to crisp-bat-13 (Evaluator Mgr) at gunbc#2065
c#4413758629 with default-lean for them to fix; pushing here proactively
given broad blast-radius (4 in-flight PRs blocked).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls
briansrls merged commit 90f355c into main May 9, 2026
4 checks passed
@briansrls
briansrls deleted the session/valiant-cat-525 branch May 9, 2026 22:44

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: e0819ee8 · Trigger: schedule
  • Thinking: 249s wall

✅ The TC2 input-order substrate, Rust evaluator mirror, generated bootstrap refresh, and focused ratchets look aligned with the runtime thesis; I found no blocking concerns.

briansrls added a commit that referenced this pull request May 9, 2026
… 3 pilot) (#2455)

* docs(r3-v-audit): advance ledger-zero progress for PR #2150 receipt

Rows #2 partial + #6 (bootstrap.rs slice) retired by Substrate Bridge
PR #2150 (merged 2026-05-07T20:05:18Z). Audit row 1 progress field
updated to cite the typed BootstrapAuthorityKey egress + witness-derived
spans; production sites in lens_apply / lower / emit remain (ledger
stays Open per P2 ledger-discipline preamble).

Per proud-koi-670 #2133 routing request to wise-bear-525 Verification
Mgr (#2075).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(briefs): R3 Cluster M Phase 2 #87 worker + Phase 3 #84 coordinator skeleton

Phase 2 worker brief (`r3-v-cluster-m-87-cementing-worker.md`): light port
of multi-gate PRE-AUTH `r3-v-tests-as-data-v1-worker.md` to gate-#87 narrow
scope. Discipline pattern (DifferentialEquals for v2-counterpart lenses,
LensOutputEquals for v3-native), first-migration target, dispatch-ratchet
successor, receipt + ledger updates. Independent of Cluster M Phase 1 per
codex BLOCKING #4 authority correction.

Phase 3 coordinator skeleton (`r3-v-cluster-m-84-bulkport-coordinator.md`):
6-class brief queue (cementing / reflected-Dag / generic-DimReport /
boundary / R1C-D/E / L4-L7-L5), strict-zero close-condition citation per
Director Ask 4, lane-Mgr signoff workflow, per-class brief authoring
discipline. Per-class detail authored as Phase 2 mid-flights.

Cite-and-execute pattern; substrate-of-truth lives in
`design-tests-as-data-completeness.md` §5 + §C5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: R3 Verification Mgr — lane through R3 close

* docs(briefs): correct LensOutputEquals field name in §4 dispatch successor

Line 76 referenced `expected_output_ref` (stale conceptual label); actual
field per `src/v3/std/verification.dag:179-183` is `expected_ref`.
Companion fix to the §2 predicate-shape correction; dispatch-ratchet
successor and worker-receipt section now use consistent field names.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(briefs): annotate R4-carve dissolution in TC3 D4 brief

Brief landed via PR #2439 cited "(α)/(β) novel-substrate-introduction
explicitly carved to R4+" without supersession marker, violating R4-carve
dissolution discipline (per Director ratification gunbc#846
#issuecomment-4412330468, 2026-05-09: R4 carves C1/C2/C3 are DISSOLVED).

Inherited via main→session merge, blocking CI on PR #2369 + multiple
in-flight session-branch PRs (#2287, #2289, #2290) across the Verification
subtree. 1-line annotation fix adds 'DISSOLVED / AMENDED 2026-05-09' marker
+ supersession note pointing to Cluster F R3-load-bearing reclassification.

Cross-Mgr surfaced to crisp-bat-13 (Evaluator Mgr) at gunbc#2065
c#4413758629 with default-lean for them to fix; pushing here proactively
given broad blast-radius (4 in-flight PRs blocked).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(briefs): R1C-D/E pre-Phase-1 pilot worker brief (Cluster M Phase 3)

Per Director sanity-check pilot greenlight (gunbc#828 c#4413268466) +
re-task Task A (gunbc#828 c#4413880134): 3-test pilot dispatch brief for
the R1C-D/E sub-class of Phase 3 #84 bulk-port queue.

Scope: r1c_d_pb_census_gates_test.rs + r1c_e_emit_gates_dag_test.rs +
r1c_e_emit_gates_omni_dag_test.rs (3 hand-Rust wrappers around .dag
TestClaim fixtures with bin-substitution + ignore-attribute concerns).

Migration target: testgen Path B (Rust test code emitted from .dag
declarations). Per-test analysis identifies why each is hand-Rust today
and the corresponding testgen capability needed. Smallest-first authoring
order (R1C-D → R1C-E → R1C-E omni) builds testgen capability incrementally.

Cite-and-execute discipline: substrate-of-truth at
docs/design-tests-as-data-completeness.md §3 (migration audit) + §1.3
(Path B emission). No content restatement.

If testgen surfaces shape-questions (e.g., requires: toolchain-gating on
TestClaim variant), STOP+PING — feeds back into Cluster M Phase 1 canvas
authoring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verification TC2 worker brief: church_rosser_executable

1 participant