Repository navigation
R3 Cluster M #84 runner-shim SG-0 slice - #2716
Conversation
|
[Director conformance read — would-approve | clean runner-shim slice] Small focused slice (+10/−63, 10 files): retires obsolete Conformance:
Velocity tripwire: −1 dissolution event. Combined with #2715 (−3), this tick's Phase 3 batch is −4 — already exceeds prior 7-day dissolution total of 3. PR body discipline: this one IS filled (Summary + SG-0 hand-path delta + Validation commands cited). Cleaner than #2715's auto-template body. No INVARIANTS violations. Would approve. (Director read posted as comment; dashboard tally unchanged.) |
|
Review metadata
1. Story of the diffThis PR retires the Brief D compile-smoke Rust shim from the SG-0 test slice and reclassifies that path as historical, not active authority. The load-bearing move is deleting 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The PM-level intent is to move tests into 3. VerdictAPPROVE. This is a clean SG-0 ratchet slice: it removes a compile-only Rust shim, updates the census, and documents the remaining historical duplicate fixtures with a bounded dissolution trigger. No substrate, fail-closed, locked-design, or untracked-debt issue is visible in the diff. |
|
Review metadata
1. Story of the diffThis PR retires the Brief D compile-smoke Rust shim and updates the surrounding plan/brief language so the source of truth is the runner-backed 2. Invariant categories
N/A — this diff does not touch substrate types,
Compliant — P2/P5 single-authority and dissolution are handled explicitly:
Compliant — the PR deletes an implementation-only Rust smoke harness instead of adding a new helper/method surface; the removed code was a compile-only wrapper around
Compliant — the change moves in the 0-residual direction by removing a Rust-authored integration test from the SG-0 hand-authored test census at
Compliant — the diff does not revise a locked design decision; it aligns with the Pure Bootstrap / tests-as-data direction by retiring a hand-authored Rust test shim and marking the port target as retired (
Compliant — the remaining Brief D 2.5. Top-down PM intent reviewCompliant — the highest-level intent here is tests-as-data / zero hand-authored Rust test surface, and this PR preserves that meaning rather than diluting it. The key PM-sensitive distinction is not “delete a Rust test and call it done,” but “delete the compile-only shim only because runner-backed 3. VerdictAPPROVE — this is a clean shim-retirement PR. It shrinks the SG-0 hand-authored test census, removes the obsolete Rust integration registration, and keeps the remaining historical |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
6ad106a5· Trigger:schedule - Thinking:
285s wall
BLOCKING (1)
Root Cause
docs/briefs/r1-testgen-manager.mdBrief D retirement was recorded in the local Brief D docs instead of the single-source Testgen pre-Rust-deletion checklist → add the sign-off entry there with the three deleted test mappings before removing the shim.
| **Authority:** Post-R2 residuals in `TESTING.md` (compiler-internal `#[cfg(test)]` under `src/v3/compiler/src/`; boundary tests invoking external toolchains). **Schema:** `src/v3/std/verification.dag` (`TestClaim`, `TestPredicate`, `requires: List<ResourceReference>`). | ||
|
|
||
| **Gates (unchanged):** Do not remove or replace existing Rust integration tests as the source of truth. Do not assert `pb_test_file_generated_from_dag` or `pb_rust_tests_outside_residual_zero` until Testgen signals. Draft `.v3` / `.dag` modules here are **fixtures** for eventual runner wiring. | ||
| **Gates (updated after runner-backed coverage):** Do not assert `pb_test_file_generated_from_dag` or `pb_rust_tests_outside_residual_zero` from this brief alone. The former Brief D compile-smoke shim is retired because the matching `.dag` modules are already runner-backed; the remaining draft `.v3` modules are historical fixtures until their named Testgen dissolution trigger fires. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Review metadata
1. Story of the diffThis PR retires the Brief D compile-only Rust shim and updates the surrounding program/brief documentation so the deletion is bounded and not overclaimed. The old shim only lowered three draft 2. Invariant categories
N/A — this is test/docs/ratchet maintenance only. The diff does not touch
Compliant — P5 / Progress Is Dissolution is handled correctly:
Compliant — the PR deletes implementation code instead of adding a new Rust helper/method surface. The removed helper
Compliant — the deleted Rust tests are not silently dropped; the diff maps each removed
Compliant — the diff does not alter locked substrate/runtime design, and it preserves the 0-floor direction by shrinking the SG-0 hand-authored-test census at
Compliant — remaining duplicate 2.5. Top-down PM intent reviewCompliant. The highest-level intent is “tests are data” and hand-authored Rust tests shrink toward zero; this PR does exactly that for one narrow, already-runner-backed compile-smoke shim. It avoids semantic dilution by saying the deletion “does not assert 3. VerdictAPPROVE. The PR is a narrow SG-0 shrink with good guardrails: it deletes one compile-only Rust shim, removes it from the census, and documents exact |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
2426e6d6· Trigger:schedule - Thinking:
139s wall
ROADMAP — Verified
- tests_as_data_demonstration: Row 74’s “retired Rust integration test” wording matches the shim deletion and existing runner-backed TestClaim carrier.
✅ No blocking concerns; the prior deletion-guard issue is resolved.
|
On the optional audit note: verified |
|
Review metadata
1. Story of the diffThis PR retires the Brief D compile-only Rust smoke shim and moves the review narrative to the already-landed runner-backed The docs are updated to make the transition explicit: Brief D’s former shim is now historical, the 2. Invariant categories
2.5. Top-down PM intent reviewCompliant. The highest-level direction in the uploaded authority is that tests become chatgpt-review-56642951-3304-40… chatgpt-review-56642951-3304-40… This PR faithfully preserves that intent: it deletes a compile-only Rust test shim, removes the census entry, and documents the runner-backed 3. VerdictAPPROVE. The PR removes a redundant compile-only Rust shim, shrinks the SG-0 test census, and records a concrete runner-backed mapping for the retired tests. I found no substrate issue, no untracked new debt, and no PM-intent mismatch. |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
7fff7235· Trigger:schedule - Thinking:
152s wall
✅ The SG-0 shrink is paired with explicit deletion-guard receipts, and I found no blocking issues in the changed lines.
|
Review metadata
1. Story of the diffThis PR retires the Brief D compile-smoke Rust shim for 2. Invariant categories
N/A — the diff does not introduce or mutate substrate types, Dag-resident shapes, cross-pass compiler facts, or
Compliant — P5 / Progress Is Dissolution is handled directly:
Compliant — the production-code style surface is not expanded; instead,
Compliant — the deleted tests were compile-only smoke checks over draft fixtures, and the PR replaces their review authority with runner-backed
Compliant — the diff preserves the locked Pure Bootstrap / tests-as-data direction rather than weakening it:
Compliant — the remaining 2.5. Top-down PM intent reviewCompliant — the PM-level intent is that tests become structural data and hand-authored Rust test surface shrinks toward zero; the thesis says tests are This PR faithfully preserves that intent: it removes one hand-authored Rust test file from the module list and census at 3. VerdictAPPROVE — I found no diff-cited invariant violation. The PR is a narrow, well-mapped shim retirement: it shrinks SG-0 test surface, points each removed smoke assertion at runner-backed |
|
Director conformance read — would-approve. Read against INVARIANTS.md P5 (Pure Bootstrap zero) + TESTING.md tests-as-data discipline + Phase 3 Cluster M #84 migration shape. Conformance citations:
No INVARIANTS violations found. Diff is 41+/64- net -23 lines; docs-heavy with a single 50-line test deletion in — sent from zesty-bear-812 |
Summary
t_pb_b_brief_d_fixture_smoke_test.rshost shim.dagsuites int_pb_b_1_dag_runner_testSG-0 hand-path delta: -1
Validation
cargo fmt --checkgit diff --checkcargo test -p v3-compiler --test integration sg0_census_test -- --nocapturecargo test -p v3-compiler --test integration t_pb_b_1_pipeline_smoke_suite_passes_through_runner -- --nocapturecargo test -p v3-compiler --test integration t_pb_b_1_contract_diagnostic_smoke_suite_passes_through_runner -- --nocapturecargo test -p v3-compiler --test integration t_pb_b_1_contract_port_cost_suite_passes_through_runner -- --nocapturecargo test -p v3-compiler --test integration r3_tests_as_data_demonstration_suite_passes_through_runner -- --nocaptureNote
t_pb_b_1_dag_runner_testfilter was also run; it reached the pre-existingExecuteCommandhelper setup case and failed becausegunbc_execute_command_bootstrapwas not built/discoverable in that invocation. The non-ExecuteCommand suites relevant to this deletion are listed above and passed.