Repository navigation
feat(v3): add BinShim substrate carrier - #1361
Conversation
|
Review metadata
Findings
Verdict APPROVE — Scope matches the locked PB-Runtime bin-shim story (authority cited in-module; PB-owned instances deferred). Bootstrap plus integration ratchet tests mirror existing carrier tests (e.g. Exploratory (optional)
|
# Conflicts: # src/v3/compiler/src/bootstrap_generated.rs # src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs
|
Review metadata
Reviewed Findings: None. Nothing in the diff clearly breaks INVARIANTS,
Verdict: APPROVE — Small, scoped carrier plus bootstrap regen and a focused shape ratchet; no rubric violations identified in the diff. |
Per cursor APPROVE_WITH_COMMENTS on PR #1368 (sha bf1d297): 1. Carrier-shape ratchet path was `src/v3/compiler/tests/integration/ bin_shim_carrier_test.rs` but PR #1370 moved the ratchet test into `src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs` (function `bin_shim_carrier_has_locked_three_field_shape`). The standalone file no longer exists on main. Update the README's ratchet pointer to the live location. 2. `type BinShim {` is at line 19 of `src/v3/std/bin_shim.dag`, not line 18. Verified via `grep -n "^type BinShim" origin/main:... bin_shim.dag`. Bumped the citation. 3. README:7 said the dependency contract appears "once the substrate prerequisite lands," which now reads as if `BinShim` itself isn't landed (it is, via #1361). Reworded to make the post-#1361 state clear: carrier is live; the remaining gate is the `<bin_name>_main` entry-function declaration. No scope change; pure live-state alignment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-binshim-regen-lens-instance Resolve README conflict by preserving both #1381's emitter-readiness section and #1368's post-#1361 entry-target STOP+PING. Specifically: - Status: keep #1381's "live carrier" framing + #1368's "entry-target gap" gate; merge the regen_lens-first-slice / emitter-readiness cross-link from #1381 into the same paragraph. - Naming convention: keep #1381's design-doc §4.2 + live-carrier- alignment intro (with the entrypoint_name vs. design-sketch `name` note + concrete example), then keep #1368's bullet list of fields and imports. - Substrate prerequisite: keep #1368's full STOP+PING content (carrier live; entry-function gap pin; success-shaped placeholder prohibition; fail-closed shape requirement); fold in #1381's "Per-shim rows not yet on main" bullet so the post-#1361 status reads consistently. - Emitter readiness: keep #1381's emitter-readiness section unchanged (planning-brief link + §7.2 worker boundary). Per dispatch on inbox #1149: "preserving both pieces" — no new scope.
…OP+PING (#1368) * docs(pb): refresh bin_shims README post-#1361 carrier landing; new STOP+PING Per Director re-engage on inbox #1149 — BinShim carrier landed via #1361. Refresh the framework README to reflect live state and identify the new substrate gap blocking per-shim instance authoring. Carrier-side facts now LIVE on origin/main: - type BinShim { entrypoint_name: NonEmptyStr, description: String, entry: DeclarationRef } at src/v3/std/bin_shim.dag (carrier-shape ratchet pins exact 3-field shape). - std.process.ProcessExit unchanged at dsl/std/process.dag:39. New STOP+PING: each shim's `entry: DeclarationRef` field needs a live `.dag`-authored `fn <bin_name>_main() -> std.process.ProcessExit` declaration to point at. Verified absent for all PB-owned bins via `grep -rn "^fn .*_main.*ProcessExit" src/v3/ dsl/` (no match). Authoring a stub function locally as part of the instance file would invent emit/runtime semantics for the future BinShim emitter — that crosses into emit/runtime work explicitly out of instance-declaration scope per the dispatch's split between "instance declaration content" and "BinShim emitter / §7.2 fixture / regen_lens.rs retirement." Two paths named for Director / Substrate Manager / PB Manager disposition: - (a) Land each <bin_name>_main entry function as part of the BinShim emitter / per-shim runtime work — natural §4.3 dissolution flow. - (b) Define a "trivial-entry" Substrate convention (stub () -> ExitSuccess as explicit placeholder) — substrate-convention extension following §P1 if surfaced. PR delta: README naming-convention section updated to enumerate the 3 live carrier fields + entry-function dependency. Substrate- prerequisite section refreshed to reflect post-#1361 live state and explicitly call out the new entry-target gap. No instance .dag file authored. No emitter authored. No retirement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): use live .dag import syntax in bin_shims README Per codex non-blocking note on PR #1368 (sha 6fa758a): the README's "Imports a per-shim row needs" line used Rust-style `v3.std.bin_shim:: BinShim` / `std.process::ProcessExit` notation, but live .dag import syntax is `import v3.std.bin_shim { BinShim }` / `import std.process { ProcessExit }`. Since this README is the canonical per-shim authoring contract, the Rust-style path slightly weakens P2 Boundary Discipline for future row authors. Switched to the live import-statement form so per-shim row authors have a copy-paste-correct contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): fix three README live-state drifts post-PR-#1370 Per cursor APPROVE_WITH_COMMENTS on PR #1368 (sha bf1d297): 1. Carrier-shape ratchet path was `src/v3/compiler/tests/integration/ bin_shim_carrier_test.rs` but PR #1370 moved the ratchet test into `src/v3/compiler/tests/integration/m2_substrate_inhabitance_test.rs` (function `bin_shim_carrier_has_locked_three_field_shape`). The standalone file no longer exists on main. Update the README's ratchet pointer to the live location. 2. `type BinShim {` is at line 19 of `src/v3/std/bin_shim.dag`, not line 18. Verified via `grep -n "^type BinShim" origin/main:... bin_shim.dag`. Bumped the citation. 3. README:7 said the dependency contract appears "once the substrate prerequisite lands," which now reads as if `BinShim` itself isn't landed (it is, via #1361). Reworded to make the post-#1361 state clear: carrier is live; the remaining gate is the `<bin_name>_main` entry-function declaration. No scope change; pure live-state alignment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): tighten BinShim placeholder guidance — no ExitSuccess stubs Per gpt-5-5-pro APPROVE_WITH_COMMENTS on PR #1368 (sha bbef4ea): the README's path-forward section listed two options, the second being a "trivial-entry" Substrate convention where a stub `() -> ExitSuccess` function is the placeholder. That risks blessing a success-shaped value as a placeholder for missing runtime semantics — the same fabrication pattern the §"Substrate prerequisite" STOP+PING explicitly rejects (P3 Fail-Closed: API-level enforcement over convention). Tightened to: - Single path forward: land each <bin_name>_main entry function as part of the BinShim emitter / per-shim runtime work that authors its body (design-doc §4.3 natural flow). - Explicit prohibition: success-shaped stubs (ExitSuccess body) are NOT acceptable placeholders — they would have the test suite Pass on a runtime that hasn't been authored. - If a placeholder is structurally needed before the real body lands, it must be fail-closed (e.g. ExitFailure with explicit reason), and even that is a §P1 substrate-convention question, not a unilateral PB call. This eliminates the success-shaped placeholder normalization the reviewer flagged while preserving the §P1 escalation path for any genuinely-needed substrate convention work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): split P3 outcome from Practice 6 in BinShim placeholder note Per cursor non-blocking note on PR #1368 (sha 03d0b98): "API-level enforcement over convention" is the wording of modeling-discipline Practice 6 (verified at docs/modeling-discipline.md:28 — Practice 6 maps to P2 Boundary Discipline, not P3). Split the parenthetical: "P3 Fail-Closed outcome; until Practice 6-style API-level typing exists, the convention itself is what blocks the fabrication" so the invariant index reads accurately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): match live ProcessExit shape in fail-closed placeholder example Per codex non-blocking note on PR #1368 (sha b3caa96): the hypothetical fail-closed placeholder example used `ExitFailure { reason: "..." }` but the live `ExitFailure` variant at `dsl/std/process.dag:41` is `{ code: Int, reason: String }` (two fields). Updated to either use the live `exit_failure(reason)` helper (at `dsl/std/process.dag:50` — fills `code: exit_code_general_error` automatically) or the explicit two-field form. Either matches the live shape; the example no longer omits the `code` field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Merge remote-tracking branch 'origin/main' into session/neat-boar-747-binshim-regen-lens-instance Resolve README conflict by preserving both #1381's emitter-readiness section and #1368's post-#1361 entry-target STOP+PING. Specifically: - Status: keep #1381's "live carrier" framing + #1368's "entry-target gap" gate; merge the regen_lens-first-slice / emitter-readiness cross-link from #1381 into the same paragraph. - Naming convention: keep #1381's design-doc §4.2 + live-carrier- alignment intro (with the entrypoint_name vs. design-sketch `name` note + concrete example), then keep #1368's bullet list of fields and imports. - Substrate prerequisite: keep #1368's full STOP+PING content (carrier live; entry-function gap pin; success-shaped placeholder prohibition; fail-closed shape requirement); fold in #1381's "Per-shim rows not yet on main" bullet so the post-#1361 status reads consistently. - Emitter readiness: keep #1381's emitter-readiness section unchanged (planning-brief link + §7.2 worker boundary). Per dispatch on inbox #1149: "preserving both pieces" — no new scope. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(pb): lock §7.2 BinShim equivalence TestClaim shape + STOP+PING Per dispatch on inbox #1149 (quick-newt archived; PB picks up the §7.2 shape lane). Authors `docs/briefs/r3-pb-binshim-7-2-claim-shape.md` as a docs-only shape artifact + STOP+PING. Live-substrate verification on origin/main HEAD: - TestClaim, TestSuite, ExecuteCommand all LIVE in src/v3/std/ verification.dag. - BinShim carrier LIVE post-#1361. - Three components NOT YET LIVE: comparison script, emitted-Rust artifact, hand-Rust snapshot. Locked predicate choice: Mechanism (1) "canonicalize-then-diff" via ExecuteCommand. Maps cleanly to the live r1_gates.dag:98,106 host-receipt fixture pattern (`p0_no_fabrication_sentinel` / `p0_rest_ops_aligned`). Mechanism (2) AST-equivalence + (3) behavioral run deferred per design-doc §7.2's "worker picks at fixture-authoring time." Locked TestClaim shape: ExecuteCommand("bash", ["-c", "exec bash \"$(git rev-parse --show-toplevel)/scripts/r3_pb_regen_lens_bin_shim_ equivalence.sh\""], 0) with TestSuite registration shape spelled out. Compiles/parses against live substrate today; not landing as a real .dag fixture because the host script + emitted-Rust + hand-Rust snapshot are not on main. STOP+PING with exact unblock order spelled out (R2-Evaluator stable → Item 4 → entry-function landed → instance authored → emitter landed → retirement PR fills script + snapshot + suite registration). Honors all dispatch non-goals: no ReleaseDeferredClaim staging, no invented TestPredicate, no emitter / instance / retirement / comparison-script authoring, no implementation-progress claim. Manager-brief sub-briefs list updated link-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): fix two §7.2 shape doc-accuracy nits Per Director docs-accuracy follow-up on PR #1415: 1. Mechanism (1) row called the precedent fixtures `r1_p0_no_fabrication_sentinel` / `r1_p0_rest_ops_aligned`, but the live fixture at `src/v3/compiler/tests/fixtures/r1_gates.dag:94-108` declares them as `p0_no_fabrication_sentinel` / `p0_rest_ops_aligned` (no `r1_` prefix). Verified via `grep -nE "^data p0_" r1_gates.dag`. Bumped to the correct names + widened the line range to 94-108 (the full TestClaim block pair) so the row is grep-clean. 2. Cross-refs row "Live TestPredicate variants" used stale line refs (TestClaim/TestSuite at 4-32; predicates at 108-280). Verified live anchors on origin/main HEAD: `type TestPredicate` at :109, `ExecuteCommand` at :148-152, `type TestClaim` at :299, `type TestSuite` at :307. Refreshed to current values + added "line numbers drift; grep for the type/variant name to re-anchor" guidance so future doc reads survive moves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(pb): unify §7.2 brief precedent line range (94-108 in both citations) Per cursor non-blocking note on PR #1415 (sha e1f0bec): the brief cited `r1_gates.dag:94-108` at line 33 (Mechanism (1) row) but `r1_gates.dag:95-109` at line 126 (Cross-refs row). Unified on the correct 94-108 range that covers both TestClaim block ends per `grep -nE "^data p0_" r1_gates.dag` (94: p0_no_fabrication_sentinel opens; 102: p0_rest_ops_aligned opens; 108: p0_rest_ops_aligned closes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
entrypoint_name,description, andentryentryasDeclarationRefper docs/design-pb-runtime-interpreter.md §4.2/§5.4; no PipelineStep DSL, emitter, runtime behavior, or PB-owned instance rows in this sliceValidation
cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrapCARGO_INCREMENTAL=0 cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap -- --verifycargo fmt --all --checkgit diff --check HEAD~1 HEADLocal limitation
CARGO_INCREMENTAL=0 cargo test -p v3-compiler --test integration bin_shim_carrier_test -- --nocapturewas attempted twice, but local compilation did not reach test execution after ~30 minutes due sccache fallback/local rustc slowness; no test failure was emitted.