Skip to content

fix(v3): refresh BinShim ratchets after carrier merge - #1372

Merged
briansrls merged 2 commits into
mainfrom
fix/bin-shim-ratchet-refresh
May 1, 2026
Merged

briansrls merged 2 commits into
mainfrom
fix/bin-shim-ratchet-refresh

Conversation

@briansrls

@briansrls briansrls commented May 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1361 after the BinShim carrier merged and main started failing inherited ratchets on unrelated PRs.

Fixes:

  • adds src/v3/compiler/tests/integration/bin_shim_carrier_test.rs to the SG-0 expected hand-authored test census;
  • adds the new src/v3/std/bin_shim.dag parse corpus manifest row observed by parse_stage4_prep.

No carrier shape changes, no PB-owned BinShim instance rows, no emitter/runtime behavior.

Validation

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

Focused cargo test runs were attempted locally, but this worktree's cold compile path stalled for several minutes. The manifest row is copied from the failing CI output on #1361/#1368, and this PR's CI is the intended full validation gate.

@briansrls briansrls changed the title R3 Substrate fix(v3): refresh BinShim ratchets after carrier merge May 1, 2026
@briansrls
briansrls marked this pull request as ready for review May 1, 2026 04:58
@briansrls

Copy link
Copy Markdown
Contributor Author

Tracking — this is the ratchet-refresh that unblocks 4 stalled PRs (#1368 #1369 #1370 #1371). Watching for CI green; will reply on each downstream PR once #1372 lands. Bounded scope (2-line: SG-0 census + parse manifest); no clippy adjustment yet — flag if that's a separate slice or rolled in. — sent from zesty-bear-812

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: e85127da · Trigger: schedule
  • Comparison: origin/main @ c1054988 ... review/pr-1372-e85127da @ e85127da
  • Thinking: 36s wall

APPROVE — Two-line ratchet refresh adding bin_shim.dag to the parse-corpus manifest and bin_shim_carrier_test.rs to the SG-0 census expected list. Both referenced files exist in the tree, manifest stays alphabetized, and no substrate changes. Matches the PR's stated purpose of refreshing BinShim ratchets after the carrier merge.

@briansrls

Copy link
Copy Markdown
Contributor Author

CI is still running on the ready head. Scope is intentionally the two-line ratchet repair unless this run surfaces a concrete clippy diagnostic on the same head. If ci fails with an actionable clippy error, I will roll that fix into #1372; if ci passes, there is no separate clippy slice from this PR.

— 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: e85127da · Trigger: schedule
  • Thinking: 132s wall

Non-blocking — Strengths

  • src/v3/compiler/tests/integration/parse_corpus_manifest.txt The added BinShim manifest row is a generated parser-corpus ratchet update, not new compiler authority.
  • src/v3/compiler/tests/integration/sg0_census_test.rs The BinShim carrier test census entry is sorted with the adjacent carrier tests and remains covered by the existing bounded SG-0 ratchet note.

✅ No blocking concerns; this is a scoped ratchet refresh with no substrate or compiler behavior change.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 0dee42b8 · Trigger: schedule
  • Comparison: origin/main @ c1054988 ... review/pr-1372-0dee42b8 @ 0dee42b8
  • Thinking: 56s wall

Findings

No issues tied to INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md. The diff only touches integration ratchets (parse_corpus_manifest.txt, sg0_census_test.rs), registers bin_shim.dag like other std files, and replaces a no-argument format!(...) with "...".to_string() for the same NotYetImplemented message — behavior is unchanged and aligns with avoiding pointless format! on a single literal.

Verdict

APPROVE — Narrow, mechanical BinShim ratchet refresh after the carrier merge; no modeling or boundary-discipline concerns in this diff.

@briansrls
briansrls merged commit 84a16cf into main May 1, 2026
4 checks passed
@briansrls
briansrls deleted the fix/bin-shim-ratchet-refresh branch May 1, 2026 05:52
briansrls added a commit that referenced this pull request May 1, 2026
…#1384)

Post-merge inconsistency from #1372 (added the ref at 05:52:48Z) +
#1370 (merged 4 seconds later at 05:52:52Z; included a "move BinShim
carrier ratchet into existing module" commit that deleted
`src/v3/compiler/tests/integration/bin_shim_carrier_test.rs` and
folded its content into `m2_substrate_inhabitance_test.rs`).

The expected list referenced a file that no longer exists, breaking
SG-0 census on every PR after the back-to-back merges. This 1-line
removal restores census consistency. The carrier-test ratchet content
itself stays alive in m2_substrate_inhabitance_test.rs per the move.

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.

1 participant