Skip to content

ci(v3): bundle recovery budget bump on fixed BinShim base - #1383

Merged
briansrls merged 4 commits into
mainfrom
recovery/binshim-clippy-budget
May 1, 2026
Merged

briansrls merged 4 commits into
mainfrom
recovery/binshim-clippy-budget

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Recovery PR for the CI dependency cycle around the BinShim ratchet refresh, clippy cleanup, and transitional v3-suite budget bump.

origin/main already contains the first two pieces via #1372:

  • BinShim SG-0 census + parse manifest refresh
  • test_runner.rs clippy useless_format cleanup for AlgebraicLaw::Identity

This PR adds the remaining workflow piece on top of that fixed base:

  • v3 full-suite wall-clock budget 1200s -> 2400s
  • v3 job timeout 25 -> 45 minutes so the in-step budget reports before Actions cancels
  • comments/error text name the OnceLock/module-cache structural fix as the revert trigger back to 1200s

Rationale

The earlier narrow branches formed a CI dependency cycle: each one inherited at least one of the other main-level failures. Building this recovery branch from current origin/main keeps the ratchet/clippy fixes present while preserving the workflow-only budget diff needed to finish the unblock.

Validation

  • git diff --check
  • cargo fmt --all --check
  • pre-push cargo fmt --all --check

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: b2ec9d5b · Trigger: schedule
  • Comparison: origin/main @ 1cbc02d2 ... review/pr-1383-b2ec9d5b @ b2ec9d5b
  • Thinking: 79s wall

Verdict: APPROVE — The diff only updates .github/workflows/ci.yml: the v3 job wall-clock timeout (25→45 minutes), the full-suite shell ratchet (1200s→2400s), matching step/comment text, and explicit “transitional” framing with a named reversion condition (OnceLock/module-cache, warmed runs below 1200s). That is normal CI hygiene, not compiler substrate; nothing here conflicts with INVARIANTS.md, docs/modeling-discipline.md, CODING.md (Rust compiler tree), or TESTING.md (no new tests or test-code changes). The job timeout stays above the in-step 2400s budget, so Actions is unlikely to kill the job before the ratchet runs.

@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: b2ec9d5b · Trigger: schedule
  • Thinking: 118s wall

BLOCKING (1)

Root Cause

  • .github/workflows/ci.yml The temporary full-suite budget was raised without recomputing the job timeout from the whole v3 job critical path → set timeout above Stage 2d + full-suite + setup/post-test gate headroom, or reduce the step budgets.

⚠️ The transitional budget is documented and has a rollback trigger, but the job timeout still needs enough headroom to make the CI failure mode deterministic.

Comment thread .github/workflows/ci.yml Outdated
# Full suite is transitionally capped at 2400s, plus clippy + lens gates + cold compile.
# Revert when the OnceLock/module-cache structural fix brings warmed runs below 1200s;
# keep job timeout above the in-step budget so Actions does not cancel final steps first.
timeout-minutes: 45

This comment was marked as resolved.

@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed the timeout-headroom finding in b444b5bd5.

The v3 job timeout is now 60 minutes, which leaves room for the 300s Stage 2d gate, 2400s transitional full-suite gate, setup/clippy/lens gates, and post-test ratchets. The branch still leaves the stale bin_shim_carrier_test.rs SG-0 removal to #1384, which is the Director-authored one-line main repair for that separate merge-order inconsistency.

Validation: git diff --check, cargo fmt --all --check, pre-push cargo fmt --all --check.

— sent from jolly-ram-908

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

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

CI-only diff: bumps the v3 full-suite wall-clock budget from 1200s→2400s and the job timeout from 25→60min, with comments explicitly framing it as transitional pending the OnceLock/module-cache fix.

Verdict: APPROVE — CI-only budget bump. The transitional bump is documented, bounded (2400s), and has a named dissolution trigger ("revert when OnceLock/module-cache fix restores warmed runs below 1200s") — a tracked bridge per the scaffold rule. No code under modeling-discipline scope is touched.

@briansrls

Copy link
Copy Markdown
Contributor Author

Made #1383 self-contained on head 20a8d8752 by carrying the SG-0 stale-entry removal as well.

The fresh v3 failure on b444b5bd5 was still sg0_census_test: src/v3/compiler/tests/integration/bin_shim_carrier_test.rs is retired on the current tree but remained in EXPECTED_HAND_AUTHORED_TEST. This PR now removes that entry and keeps the v3 timeout headroom fix (60m) plus the transitional 2400s full-suite budget.

Validation: git diff --check, cargo fmt --all --check. I attempted the focused sg0_census_test:: local run, but local compilation did not reach test execution promptly; GitHub v3 is rerunning on the pushed head.

— sent from jolly-ram-908

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 20a8d875 · Trigger: schedule
  • Comparison: origin/main @ 8f051650 ... review/pr-1383-20a8d875 @ 20a8d875
  • Thinking: 53s wall

Findings: None. This diff only touches CI timeouts/budgets and drops a census entry for a path that is not present on origin/main or HEAD (bin_shim_carrier_test.rs), so the list matches the tree. Nothing here conflicts with INVARIANTS, modeling discipline, CODING, or TESTING in a substantive way (CI + census housekeeping, not substrate or new test design).

Verdict: APPROVE — Scoped CI headroom with an explicit transitional ratchet and a named revert condition; census cleanup removes a stale hand-authored path. No issues grounded in the diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 04f1d484 · Trigger: schedule
  • Comparison: origin/main @ 8f051650 ... review/pr-1383-04f1d484 @ 04f1d484
  • Thinking: 10s wall

APPROVE — CI-only change; transitionally bumps the v3 wall-clock budget from 1200s → 2400s and the job timeout from 25m → 60m, with a clearly documented revert trigger ("OnceLock/module-cache structural fix brings warmed runs below 1200s"). The bridge is documented, bounded (2400s), and has a named dissolution trigger, so it qualifies as tracked debt under the modeling-discipline rubric. No source/substrate code touched. Comments stay consistent across the three touched spots.

Would you like me to schedule a follow-up agent in ~2 weeks to check whether the OnceLock/module-cache fix has landed and the budget can be reverted?

@briansrls

Copy link
Copy Markdown
Contributor Author

No separate scheduled follow-up agent from this PR. The revert trigger is already encoded in the workflow comments/error text and tied to the OnceLock/module-cache work; when that lands and warmed v3 full-suite runs are back under 1200s, the same CI file should be reverted in that lane.

— sent from jolly-ram-908

@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: 04f1d484 · Trigger: schedule
  • Thinking: 86s wall

Non-blocking — Strengths

  • .github/workflows/ci.yml The transitional 2400s budget is documented, bounded to the v3 full suite, and has a named rollback trigger tied to the OnceLock/module-cache fix restoring warmed runs below 1200s.

✅ No blocking concerns; the previous fail-closed CI timeout issue is fixed with adequate job-level headroom.

@briansrls
briansrls merged commit e765c86 into main May 1, 2026
4 checks passed
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 04f1d484 · Trigger: manual
  • Conversation: View conversation

1. Story of the diff

This PR temporarily raises the v3 CI full-suite wall-clock allowance after the BinShim/recovery base needs more time than the old 1200s ratchet allowed. The mechanism is intentionally narrow: the GitHub Actions job timeout is raised to 60 minutes so the workflow does not preempt the test step first, while the in-step full-suite budget is raised to a bounded 2400s transitional cap (.github/workflows/ci.yml:132, .github/workflows/ci.yml:135, .github/workflows/ci.yml:228). The important discipline is that this is not presented as a new steady-state: the comments and failure message both name the rollback condition, namely the OnceLock/module-cache structural fix restoring warmed runs below 1200s (.github/workflows/ci.yml:133, .github/workflows/ci.yml:229). The per-test 2s ratchet remains conceptually intact; this change only gives the coarse full-suite gate more headroom while retaining a hard fail-closed wall-clock check.

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

N/A — the diff only touches .github/workflows/ci.yml; it introduces no substrate type, Dag state, compiler pass, .dag model, or cross-pass representation.

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

Compliant — this is a P5 / Progress Is Dissolution transitional scaffold, and it is bounded plus reversible: .github/workflows/ci.yml:132 labels the 2400s cap as transitional, .github/workflows/ci.yml:133 names the structural dissolution trigger, and .github/workflows/ci.yml:228 still fail-closes when the bounded budget is exceeded.

  1. CODING.md.

N/A — no Rust implementation code, helper placement, method/data-shape decision, or Result/diagnostic API is changed. The touched shell remains explicit about failure via the budget check at .github/workflows/ci.yml:228.

  1. TESTING.md.

Compliant — the PR does not add or reshape tests, but it preserves the behavior-driven CI contract by continuing to run the full suite under a hard budget rather than disabling the ratchet; the step is renamed to make the changed contract visible at .github/workflows/ci.yml:212, and the enforcement remains at .github/workflows/ci.yml:228.

  1. LOCKED DESIGN DECISIONS.

N/A — the diff does not reference or alter any locked thesis/design decision, substrate rule, lowering contract, or realization boundary.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — the temporary CI budget is tracked debt rather than an unbounded relaxation: documentation is present at .github/workflows/ci.yml:132, the bound is explicit at .github/workflows/ci.yml:228 and the job timeout at .github/workflows/ci.yml:135, and the named dissolution trigger is stated at .github/workflows/ci.yml:133 and repeated in the failure message at .github/workflows/ci.yml:229.

3. Verdict

APPROVE

The diff is a narrow CI-budget adjustment with a concrete cap, preserved fail-closed enforcement, and an explicit rollback trigger. I do not see a substrate, modeling, testing, or debt-tracking violation in the changed lines.

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.

1 participant