Repository navigation
D1 slow-test residual sweep: retire stale warn row - #2817
Conversation
briansrls
changed the title
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
d2c04387· Trigger:schedule - Thinking:
149s wall
Non-blocking — Strengths
docs/debt/slow-test-residual-sweep-2026-05-13.mdThe sweep note documents the partial-run boundary and gives a concrete under-budget receipt for the removed warn row.
ROADMAP — Verified
- CI ratchet architecture: The updated row preserves the #102 caveat, names the JSONL manifest bridge, and keeps fresh timing audits plus per-node budgets open.
✅ No blocking concerns.
|
Review metadata
1. Story of the diffThis PR is a narrow residual cleanup for the slow-test timing ratchet. It updates the ROADMAP CI-ratchet debt row so the old free-form 2. Invariant categories
N/A — this diff touches ROADMAP/debt documentation and a test-time JSONL policy manifest; it does not alter
Compliant — P5 Progress Is Dissolution is handled correctly: the PR removes one ad-hoc warn-policy row (
N/A — no Rust implementation, helper API, method/free-function shape, error carrier, or naming surface is introduced. The only machine-readable implementation artifact touched is deletion of one JSONL policy row.
Compliant — no new test is needed because the subject is not new behavior; the PR supplies the right verification level for a manifest-row retirement: a libtest name audit (
Compliant — the diff does not weaken the zero-floor / tests-as-data direction. It explicitly says the JSONL manifest bridge remains pending #102 modeled timing projection (
Compliant — the remaining debt is tracked with documentation, bounds, and a dissolution trigger. Documentation: the new debt receipt ( 2.5. Top-down PM intent reviewCompliant — the PR preserves the PM-level intent. The highest-level direction is that tests and timing policy should ultimately be structural chatgpt-review-9197ea0c-307c-4a… This diff moves in that direction by retiring a stale warn row while refusing to call the JSONL bridge final: 3. VerdictAPPROVE. This is a clean residual paydown: it deletes one measured-under-budget warn row, documents the evidence, and keeps the remaining manifest bridge explicitly temporary rather than claiming #102 closure. I found no blocking or non-blocking findings tied to changed diff lines. |
|
Review metadata
1. Story of the diffThis PR retires one stale slow-test warn-policy row from the Phase-0 wall-clock ratchet. The ROADMAP debt row is rewritten from the old 2. Invariant categories
N/A — this diff does not alter substrate types,
Compliant — P5/Strict Forward Progress is handled correctly: the diff removes one bridge row rather than adding a scaffold, and it explicitly says the JSONL bridge is not #102 closure because policy still reads from
N/A — no Rust implementation code, helper API, methods, error/result shapes, or new impurity surface are introduced; the diff is documentation plus JSONL data.
Compliant — the changed test policy follows the Phase-0 wall-clock contract: the sweep documents the list audit and timing evidence (
Compliant — the PR references the zero-residual/test-cost trajectory but does not dilute it:
Compliant — no new untracked debt is added. The remaining side-manifest bridge is bounded and named: it is pending #102 projection from modeled timing facts ( 2.5. Top-down PM intent reviewCompliant. The highest-level intent is that tests and correctness dimensions move toward structural data rather than hand-maintained behavioral exceptions: the thesis says the proof/test surface is structurally derived and that the previous Rust-side residual carve-out is retracted under the 0-floor target, with tests migrating to This PR moves in that direction at the PM level: it removes one manually maintained warn exception, documents why the remaining JSONL is still only a bridge, and leaves the modeled 3. VerdictAPPROVE The PR is a narrow, well-scoped debt paydown: it removes a stale warn row, records the evidence for doing so, and keeps the larger #102/modeling target open rather than overstating closure. I did not find a diff-cited invariant or PM-intent mismatch. |
|
Verified the relayed claude review against the current HEAD. It is an approval with no findings or requested changes, and the reviewed scope matches the current diff: one stale warn row retired plus ROADMAP/HANDOFF/debt receipt updates. No code or doc change is required for this feedback. — sent from loyal-ferret-30 |
|
Verified the cursor review against current HEAD 93a2169. The optional HANDOFF wording note is accurate: |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
1d973c0f· Trigger:schedule - Thinking:
154s wall
✅ No blocking concerns; the docs/manifest sweep keeps the JSONL bridge explicitly interim pending #102 and removes only the under-budget warn row with bounded timing evidence.
|
Verified the codex no-blocking-concerns review against the current PR state. It has no actionable findings, and it was for older commit 1d973c0; current HEAD is 93a2169 with the same docs/manifest hygiene scope plus green GitHub checks. No code or doc change is required for this feedback. Dashboard readiness still reports 1/2 recognized approval providers, so I am holding merge per manager instruction. — sent from loyal-ferret-30 |
|
Verified the claude approval against current HEAD 523f7f3. The branch head moved by merging origin/main, but the PR diff against origin/main remains the same four-file docs/manifest sweep: ROADMAP, the dated slow-test residual receipt, one JSONL warn-row deletion, and HANDOFF_PHASE0 alignment. The review has no actionable findings, so no code or doc change is required. CI is re-running on the new head and dashboard readiness is still false, so I am holding merge per manager instruction. — sent from loyal-ferret-30 |
|
Review metadata
1. Story of the diffThis PR is a small residual-paydown sweep for the slow-test timing ratchet. It removes one stale warn-policy entry from 2. Invariant categories
N/A — this diff does not touch
Compliant — P5 / Progress Is Dissolution is handled correctly: the PR removes an ad-hoc warn row rather than adding one (
N/A — no Rust implementation code, functions, methods, error carriers, or helper placement changed. The changed surface is documentation plus one JSONL policy-row deletion, so the
Compliant — the diff does not add a new test, but for this kind of manifest cleanup the existing timing-ratchet is the enforcement surface: the debt note records the list audit command (
Compliant — the diff does not alter a locked design doc and does not dilute the 0-floor / tests-as-data direction. It explicitly avoids claiming closure while the side manifest remains (
Compliant — no new scaffold is introduced. The existing JSONL bridge remains tracked with documentation ( 2.5. Top-down PM intent reviewCompliant — the PM-level intent is that tests and CI/process claims keep migrating toward 3. VerdictAPPROVE — this is a narrow, well-scoped cleanup that tightens the slow-test ratchet by retiring one obsolete warn row and keeps the remaining bridge explicitly tracked. I did not find a diff-cited invariant violation or PM-intent mismatch. |
|
Verified the current-head openai-pro review against HEAD 523f7f3. It reports APPROVE with no actionable findings; the PR diff remains the same four-file docs/manifest sweep, and GitHub checks are green. No code or doc change is required for this feedback. Dashboard readiness still reports ready=false / 0 recognized current-head approvals despite the review text, so I am holding merge per the dashboard source-of-truth policy. — sent from loyal-ferret-30 |
|
Verified the current-head codex review against HEAD 251bb1a. It reports APPROVE with no actionable findings, and the PR diff remains the same four-file docs/manifest sweep. No code or doc change is required for this feedback. Dashboard readiness still reports ready=false with only 1/2 recognized approval providers and v3 is re-running on the current head, so I am holding merge per manager instruction. — sent from loyal-ferret-30 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
523f7f3a· Trigger:schedule - Thinking:
106s wall
Non-blocking — Strengths
docs/debt/slow-test-residual-sweep-2026-05-13.mdThe sweep note is bounded to residual paydown, preserves #102 as open, and records concrete 16 ms evidence for the retired warn row.scripts/test-node-wall-clock-ratchet.jsonlThe manifest change shrinks the interim warn-policy bridge instead of expanding hand-maintained timing policy.
ROADMAP — Verified
- CI ratchet architecture: The updated row keeps fresh CI-shaped timing audits, stale warn-row deletion, and per-node duration budgets open after this one-row retirement.
✅ No blocking concerns; this is a clean docs/manifest alignment with the thesis direction toward modeled timing facts.
|
Verified the latest no-finding reviews against the current PR state. Current HEAD is 348009c and the PR diff against origin/main remains the same four-file docs/manifest sweep: ROADMAP, the dated slow-test residual receipt, one JSONL warn-row deletion, and HANDOFF_PHASE0 alignment. The new claude review reports APPROVE with no actionable findings on current HEAD; the relayed codex no-blocking review was for older HEAD 523f7f3 but remains consistent with the unchanged PR diff. No code or doc change is required for either item. Dashboard readiness still reports ready=false with only 1/2 recognized approval providers and v3 is running on current HEAD, so I am holding merge per manager instruction. — sent from loyal-ferret-30 |
|
Verified the latest current-head codex approval against HEAD 1d5c2f5. It reports APPROVE with no actionable findings, and the PR diff remains the same four-file docs/manifest sweep. No code or doc change is required for this feedback. Dashboard readiness still reports ready=false with only 1/2 recognized approval providers; GitHub checks are green but mergeability is currently UNKNOWN, so I am holding merge per manager instruction and dashboard source-of-truth policy. — sent from loyal-ferret-30 |
|
Verified the current-head cursor approval against HEAD 2ccf663. It reports APPROVE with no actionable findings, and the PR diff remains the same four-file docs/manifest sweep. No code or doc change is required for this feedback. GitHub shows checks green and mergeStateStatus CLEAN; dashboard currently recognizes 2/2 providers but still reports checks_state=pending, so I am waiting for the dashboard source of truth to catch up before merging. — sent from loyal-ferret-30 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
348009c8· Trigger:schedule - Thinking:
139s wall
✅ No blocking concerns; the PR keeps #102 open, records bounded timing evidence, and shrinks the interim warn-policy manifest by one row.
1 participant
Summary
Wave-1 D1 slow-test residual sweep after the
TestNodeCostDimension+ JSONL manifest bridge landed in the #2761/#2764 stack.scripts/test-node-wall-clock-ratchet.jsonlROADMAP.mdso the CI ratchet architecture row no longer describes the retiredslow-test-exemptions.txt/TEST_TIMEOUT_MAX_EXEMPTIONSprotocol as current stateEvidence
RUSTC_BOOTSTRAP=1 cargo test -p v3-compiler -- --listconfirmed all 83 pre-cut manifest names existed in the current v3 test list.ctrl-build -- env RUSTC_BOOTSTRAP=1 cargo test -p v3-compiler -- -Z unstable-options --report-timecaptured 391 lib-test timing rows before stopping on a remote-environment helper lookup failure forgunbc_execute_command_bootstrap.bootstrap::tests::kernel_bool_path_a_attaches_diagnostic_when_boolean_algebra_unresolvableat 16 ms, below the 2000 ms Phase-0 budget, so its warn row was removed.scripts/check-test-timeout.sh /tmp/v3-report-time.logparsed the captured log and exited cleanly: remaining over-budget rows were warn-listed, with no unexpected over-budget tests.jq -e . scripts/test-node-wall-clock-ratchet.jsonlpassed; manifest count is now 82 rows.Notes
This PR is D1 residual paydown, not a new #102 closure claim. The timeout script still reads the JSONL bridge directly until slow-test policy is projected from modeled timing facts.