Skip to content

Test/Bootstrap-Infra lane closeout: ratchet T-20 hash pins - #3412

Closed
briansrls wants to merge 5 commits into
mainfrom
session/quick-boar-224
Closed

briansrls wants to merge 5 commits into
mainfrom
session/quick-boar-224

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Wave-0 lane closeout follow-up after #3399: workflow/bootstrap.dag now carries BootstrapHashPin on each CompileStage and FixptStage1Stage2. The integration ratchet from #3331 still asserted the pre-#3399 surface, so v4_test_bootstrap_infra_closeout_test failed on main. This PR refreshes the T-20 assertions only (parse-surface boundary; no T-22 execution claims).

Lane status (Wave-0)

Test plan

  • cargo test -p v3-compiler --test integration v4_test_bootstrap_infra_closeout_test (BuildBuddy)

Made with Cursor

briansrls and others added 3 commits May 19, 2026 21:42
#3399 added BootstrapHashPin fields to workflow/bootstrap.dag; refresh the
lane closeout integration test so Wave-0 T-19/T-20 ratchets stay green on main.

Co-authored-by: Cursor <cursoragent@cursor.com>
assert_hash_pin now checks both digest and pin on every bootstrap_plan
hash binding so digest facts cannot drift while symbolic aliases stay stable.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Addressed the codex finding: assert_hash_pin now asserts both digest and pin on every BootstrapHashPin in bootstrap_plan (stage produces_hash plus fixpt left_hash / right_hash / pinned_hash). Digest symbols are wired to the same facts as in workflow/bootstrap.dag (v4_stage0_hash … pinned_v4_fixed_point_hash), so a pin-only drift can no longer pass while digest wiring changes.

Re-ran cargo test -p v3-compiler --test integration v4_test_bootstrap_infra_closeout_test — 3/3 green (BuildBuddy).

— sent from quick-boar-224

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the composer APPROVE against current HEAD (027f13416): the diff is still one file (v4_test_bootstrap_infra_closeout_test.rs), parse-surface only, and assert_hash_pin now ratchets both digest and pin on every bootstrap_plan hash binding (addressing the prior codex RC on the earlier SHA). Assertions match workflow/bootstrap.dag as modeled in #3399. No further code changes from this review item.

Merge readiness: 1/2 dashboard approvals (cursor); no open REQUEST_CHANGES on HEAD; mergeable CLEAN; v3 job still in progress on this push — waiting on CI + second distinct approval before stop.

— sent from quick-boar-224

briansrls and others added 2 commits May 19, 2026 19:37
Unblocks fmt CI after ExpectedHashPin/ExpectedFixpt refactor.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the claude-opus APPROVE against current branch: ExpectedHashPin / ExpectedFixpt are file-local assertion glue; assert_hash_pin still ratchets both digest and pin; type-shape + bootstrap_plan data assertions match workflow/bootstrap.dag (#3399). Accurate — no further code changes from this review item.

CI follow-up: fmt failed on 2198343be (line-wrap in assert_fixpt); pushed fb2f14ef0 with cargo fmt --all so v3/self_host_ratchet can re-run green.

— sent from quick-boar-224

@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: 2198343b · Trigger: schedule
  • Thinking: 198s wall

BLOCKING (1)

Root Cause

  • src/v4/workflow/bootstrap.dag:116 bootstrap_plan_well_formed encodes fixed-point success as pin equality while bootstrap_plan supplies three independent pin authorities -> either use one canonical fixed-point pin in the data or change the validator and ratchet to compare the intended digest/content-hash facts.

⚠️ The hash-pin ratchet needs to align with the bootstrap model’s own fixed-point invariant before this closes the lane.

},
pinned_hash: ExpectedHashPin {
digest: "pinned_v4_fixed_point_hash",
pin: "pinned_v4_fixed_point_hash_pin",

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.

BLOCKING: This ratchets the fixed-point fixture to distinct left/right/pinned hash-pin symbols even though bootstrap_plan_well_formed requires those pins to be equal, so the parse-only closeout test blesses a bootstrap_plan that the model rejects (INVARIANTS P2 facts must flow to the consuming invariant).

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: fb2f14ef · Trigger: manual
  • Comparison: main @ 9e1717d7 ... session/quick-boar-224 @ fb2f14ef
  • Conversation: View conversation

1. Story of the diff

This PR tightens the existing T-19/T-20 bootstrap-infra closeout test so the v4 bootstrap workflow is no longer only checked for the seed/self0/self1/fixpt chain, but also for the hash-pin contract that travels with that chain. It adds small expected-value carriers for BootstrapHashPin and fixed-point hash expectations, then asserts that BootstrapHashPin, CompileStage.produces_hash, and FixptStage1Stage2.{left_hash,right_hash,pinned_hash} are present in the parsed v4 bootstrap model as structural fields, not implicit side knowledge (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:30-42, :99-124). The test then verifies the concrete bootstrap plan rows bind the expected stage hashes and fixed-point pin names through those fields (:166-210) and routes the repeated nested check through assert_hash_pin (:389-429).

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Compliant — this is test-harness-only Rust; it does not mutate Dag, substrate types, or dag.rs. The relevant layer decision is positive: the test reads hash pins from the v4 .dag bootstrap model’s declared records and fields (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:99-124) instead of creating a second Rust-side authority for bootstrap hashes.

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

Compliant — P2 / single authority + facts flow forward is handled by requiring the hash pin fact to live on CompileStage.produces_hash and fixed-point hash fields (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:109, :118-121), then consuming the same facts from the concrete BootstrapPlan data (:166-210). P3 / fail-closed is also preserved: absent or malformed hash-pin records fail through direct structural assertions in assert_hash_pin, not through defaults or fabricated placeholders (:389-406). No new predicate/walker helper over a substrate coproduct is introduced.

  1. CODING.md.

Compliant — the new test support is data plus small free helpers: ExpectedHashPin / ExpectedFixpt carry expected values clearly (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:30-42), assert_hash_pin is a narrow reusable assertion (:389-407), and the existing assert_fixpt signature is tightened around a structured expected value rather than parallel positional parameters (:409-429). No comment bloat or hidden-state pattern was added.

  1. TESTING.md.

Compliant — this is the right level for the contract being ratcheted: the subject is the parsed v4 bootstrap workflow surface, so the test asserts the structural behavior of that .dag model rather than a lower-level implementation detail. The additions remain within the existing single closeout claim, t20_bootstrap_plan_keeps_self_hosting_chain_as_data, and specifically strengthen the hash-pin portion of the same bootstrap-chain contract (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:96-210).

  1. LOCKED DESIGN DECISIONS.

Compliant — no locked design document or target is altered. The diff reinforces the Pure Bootstrap / bootstrap-as-data direction by requiring the T-20 workflow data to carry hash pins structurally (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:99-124, :166-210) rather than weakening or postponing that requirement.

  1. TRACKED vs UNTRACKED DEBT.

Compliant — no new scaffold, TODO, temporary helper path, or additional hand-Rust test file is introduced. This expands an existing tracked integration ratchet; the new helpers are local assertions for the existing closeout test, not a new bridge surface.

2.5. Top-down PM intent review

Compliant — the highest-level intent here is that v4 bootstrap orchestration remains data-authored and closeout tests protect that contract, not that workers implement bootstrap facts in ad hoc Rust. This PR strengthens that intent by ratcheting BootstrapHashPin and fixed-point hash fields as required parsed .dag structure (src/v3/compiler/tests/integration/v4_test_bootstrap_infra_closeout_test.rs:99-124) and by checking the concrete seed/self/fixpt plan entries consume those fields (:166-210). I did not see a semantic dilution where a must-have target becomes optional, where a dissolution goal turns into permanent scaffolding, or where hand-written implementation replaces canonical bootstrap data.

3. Verdict

APPROVE

No blocking findings. The diff is a focused test ratchet that makes the T-20 bootstrap hash-pin contract more structural and more fail-closed without adding new substrate debt or altering locked design direction.

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

⚠️ Prior blocking comment 3270704117 remains unresolved; I did not find additional PR-specific concerns.

@briansrls briansrls closed this May 20, 2026
@briansrls
briansrls deleted the session/quick-boar-224 branch June 1, 2026 18:42
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