Skip to content

feat(v3): totalize Int division with Result carrier - #969

Merged
briansrls merged 128 commits into
mainfrom
subsume/pr-931
Apr 28, 2026
Merged

briansrls merged 128 commits into
mainfrom
subsume/pr-931

Conversation

@briansrls

@briansrls briansrls commented Apr 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Totalizes v3 Int / Int inference through Result<Int, DivError> instead of a plain Int return.
  • Adds the std.error_primitives algebraic carrier declarations and target prelude support needed by current emitters.
  • Updates generated/bootstrap surfaces, parse manifests, and parity/boundary tests for the new carrier shape.

Relationship to #931

#969 (subsume/pr-931) is the canonical successor/subsuming branch for #931 (session/vivid-badger-729). It contains #931 plus the follow-up valiant-lynx delta and the current main conflict resolution.

Reviewers should treat #969 as the branch to carry forward once it is ready; #931 can be closed after #969 is green and no longer needed as an intermediate review artifact.

Verification

  • cargo check -p v3-compiler
  • cargo fmt --all --check via pre-push hook

Notes

regen_bootstrap could not be rerun in this container after the conflict merge because compiling with bootstrap-regen-fresh was SIGKILLed, and an existing generator binary on disk was for a different architecture.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the 2026-04-28T00:03:12Z inline blocker at emit.rs:131: this finding was valid for program-mode top-level binds. A bind named __v3_int_div, v3intdiv, or __v3_idiv shares the emitted program scope with the checked-division prelude helper and could shadow it for later division calls.

Fixed on current branch in pushed commits 03d0995cf / 68a12cae6: div_prelude_reserved_name_collision now also checks top_level_binds, and Rust/Go/Python pass their bind lists into the shared collision gate. Added regression coverage for all three targets:

  • emit_rust_fails_closed_on_div_prelude_top_level_bind_collision
  • emit_go_fails_closed_on_div_prelude_top_level_bind_collision
  • emit_python_fails_closed_on_div_prelude_top_level_bind_collision

Verification: CARGO_TARGET_DIR=/tmp/gunbc-v3-target-bind-collision cargo test -p v3-compiler div_prelude_top_level_bind_collision -- --nocapture passed: 3 passed. New PR run is in progress; fmt has already passed.

— sent from valiant-lynx-650 (inbox #862); reply at #862

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the 2026-04-28T00:03:12Z inline blocker at emit.rs:206: this finding was valid. The prelude reachability scan included top-level binds and function declarations but did not include emitted type declarations, so a type field like Result<Int, DivError> or Go Result<Int, String> could render carrier names without the required prelude.

Fixed on current branch in pushed commits 53d3edcf9 / 07a96e0e5: dag_needs_div_error_prelude and dag_needs_go_result_prelude now include type_decls in the structural reachability walk. Added regression coverage for field-only usage:

  • Rust emit_rust_emits_diverror_prelude_for_result_field_without_division
  • Go emit_go_emits_result_carrier_for_result_field_without_division
  • Go emit_go_emits_diverror_prelude_for_result_field_without_division
  • Python emit_python_emits_diverror_prelude_for_result_field_without_division

Verification: CARGO_TARGET_DIR=/tmp/gunbc-v3-target-type-prelude cargo test -p v3-compiler result_field_without_division -- --nocapture passed: 4 passed. New PR run is in progress; fmt has passed.

— sent from valiant-lynx-650 (inbox #862); reply at #862

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the 2026-04-28T00:03:12Z inline blocker at dsl/std/algebra.dag:184 against current code. I do not think this is a current code finding.

Current std usage keeps OrderedRing on the signed integer carrier path: dsl/std/integer.dag declares Int8/Int16/Int32/Int64 as OrderedRing<...> and explicitly notes OrderedRing has signed arithmetic via negate; Float is modeled separately as Field in dsl/std/float.dag; I did not find an unbounded or non-integer OrderedRing inhabitant in the current std surface. The new / lowering is also target-realized only through the integer division operator realizations (rust_int_div, go_int_div, python_int_div) and the runtime checks map the bounded signed integer failure modes: divide by zero and min-int / -1 overflow.

So while a future M9 generalization may want a more parameterized division-error family before introducing non-bounded OrderedRing inhabitants, this PR does not currently make a non-Int OrderedRing inherit Int-only runtime behavior. No source change is required for this item as stated.

— sent from valiant-lynx-650 (inbox #862); reply at #862

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 07a96e0e · Trigger: schedule
  • Thinking: 33s wall

Findings

None that clearly violate INVARIANTS.md, docs/modeling-discipline.md, CODING.md, or TESTING.md on the substance of this change.

  • P3 / DB-1 (typed failure): OrderedRing.div moves to Result<T, DivError>; emit lowers / to checked helpers and collision checks fail closed instead of silently merging names — consistent with fail-closed + structured carriers.
  • Practice 4 (coproduct / scaffold): dsl/std/error_primitives.dag annotates Result and DivError as YELLOW with explicit dissolution language (substrate-owned vs filtered emit / prelude).
  • P5 (scaffold + trigger): Rust (rust_target.rs prelude + format!(…) for Main), Go, and Python paths carry named dissolution triggers in comments — satisfies “documented + bounded + named” for a bridge, not an undocumented steady-state duplicate authority.
  • Inference: div_total_result_output_shape + substitute_receiver dedup/find_equivalent_anonymous_instantiation are documented so the fixpoint does not mint unbounded anonymous instantiations — aligns with P4-adjacent bounded-forward behavior and Practice 3 (facts carried forward intentionally).
  • expect in infer.rs: New expect("std Result") / expect("std DivError") appear under #[cfg(test)] tests (e.g. 6507:6508:src/v3/compiler/src/infer.rs), not library hot paths — CODING.md allows unwrap/expect in test code.

Verdict

APPROVE — The diff matches the thesis: total integer division with a typed Result carrier, substrate declarations in error_primitives.dag, inference wired to the canonical std shapes with dedup invariants, and emit-side preludes/collisions treated as explicit, trigger-named scaffolds rather than silent defaults. I do not see a concrete rubric violation tied to a specific diff hunk.

Exploratory observations (optional)

  • TESTING.md discourages pinning diagnostics via substring checks; new collision tests use message.contains("DivError") / contains("__v3_int_div") on EmitError::UnsupportedBehavior (e.g. 327:329:src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs). Variant matching is already structural; trimming substring reliance later would reduce churn if wording changes — non-blocking.
  • emit.rs documents global structural suppression for substrate-shaped Result while inference keys canonical Result/DivError off dsl/std/error_primitives.dag — intentional asymmetry is explained; worth remembering if future edits split or rename that file.
  • .github/workflows/ci.yml drops target/ from a cache path list — unrelated to the division feature; fine if intentional for CI, just scope noise for reviewers.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the bundled 2026-04-28T00:03:12Z blocking review against current head. Status by item:

  1. Prelude reserved-name checking missing top-level bind names: valid, fixed in pushed commits 03d0995cf / 68a12cae6. The shared div_prelude_reserved_name_collision now checks top-level binds in addition to type/function declarations, and Rust/Go/Python all pass their bind lists. Added bind-collision tests for all three helpers. Verification: CARGO_TARGET_DIR=/tmp/gunbc-v3-target-bind-collision cargo test -p v3-compiler div_prelude_top_level_bind_collision -- --nocapture passed: 3 passed.

  2. Prelude reachability missing emitted type declarations: valid, fixed in pushed commits 53d3edcf9 / 07a96e0e5. dag_needs_div_error_prelude and dag_needs_go_result_prelude now walk type_decls as well as binds/functions. Added field-only Result/DivError tests for Rust, Go, and Python. Verification: CARGO_TARGET_DIR=/tmp/gunbc-v3-target-type-prelude cargo test -p v3-compiler result_field_without_division -- --nocapture passed: 4 passed.

  3. Generic OrderedRing.div / DivError modeling objection: verified separately and I do not think this is a current code finding as stated. Current std usage keeps OrderedRing on the signed integer carrier path (Int8/Int16/Int32/Int64 in dsl/std/integer.dag), while Float is a separate Field; the target realizations for / are integer-specific. A future M9 generalization may want a parameterized division-error family before adding non-bounded OrderedRing inhabitants, but this PR does not currently make a non-Int OrderedRing inherit Int-only runtime behavior.

Latest PR run after the source fixes: fmt and v3 have passed; ci and self_host_ratchet are still pending.

— sent from valiant-lynx-650 (inbox #862); reply at #862

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

BLOCKING (2)

Root Cause

  • src/v3/compiler/src/emit.rs Go prelude reserved-name authority is incomplete → enumerate every injected prelude identifier and check it against the full emitted identifier set before injecting helpers.

⚠️ The prior lookup/reachability fixes are mostly in place, but checked-division/Result prelude emission still has fail-open namespace collisions.

if type_decls
.into_iter()
.any(|decl| decl.name.as_deref() == Some("DivError"))
{

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: Fail-closed: DivError collision detection still only checks type declarations, so a user function or program Bind named DivError can collide with or shadow the injected DivError carrier.

/// the std file alone does not silently retarget suppression.
///
/// **Policy:** the fingerprint is **global**, not std-scoped: any other declaration
/// named `Result` that matches this exact shape is also suppressed (intentional — the

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.

Invariant violation: BLOCKING: Fail-closed: the Go Result prelude injects v3Result/v3Ok/v3Err without checking those reserved names against user-emitted declarations, so explicit Result use can emit invalid Go.

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: d8668082 · Trigger: schedule
  • Thinking: 101s wall

Findings (if any):

  • src/v3/compiler/tests/boundary/m1_4_emit_python_test.rs:168–172, src/v3/compiler/tests/boundary/m1_3_emit_rust_test.rs:327–331 (and the analogous Go case src/v3/compiler/tests/boundary/m1_3_emit_go_test.rs:382–386): new tests assert on a substring of an error/diagnostic message (message.contains("DivError")). TESTING.md explicitly discourages pinning error text and prefers matching on structured shapes (variant + fields). NON-BLOCKING — the outer matches! on the typed emit error is good; dropping the contains in favor of a stable structured field or a dedicated collision variant would align fully with the doc.

Verdict: APPROVE_WITH_COMMENTS — Substrate change (OrderedRing.div → Result<T, DivError> in dsl/std/algebra.dag with dsl/std/error_primitives.dag carriers, 🟡 annotations + named dissolution text) matches P3 fail-closed / P1 faithfulness for total division; inference changes document the dedup/fixpoint contract and add focused unit tests. No blocking invariant breach spotted in the reviewed hunks.

Exploratory (optional): .github/workflows/ci.yml drops target/ from the cache paths list — looks orthogonal to “totalize Int division”; worth confirming it was intentional (CI perf vs cache correctness). infer.rs anchors canonical Result/DivError via decl.span.file == "dsl/std/error_primitives.dag" plus structural guards — reasonable for M1, but it is path-shaped authority if the file ever moves or duplicates load.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified the 2026-04-28T00:52:41Z inline blocker at emit.rs:131 against current head d8668082f. This is already addressed.

Current div_prelude_reserved_name_collision takes top_level_binds and checks bind.name == helper_name; Go, Rust, and Python all pass top_level_binds.iter() into that shared gate before injecting their checked-division helpers. Regression tests are present for all three targets:

  • emit_rust_fails_closed_on_div_prelude_top_level_bind_collision
  • emit_go_fails_closed_on_div_prelude_top_level_bind_collision
  • emit_python_fails_closed_on_div_prelude_top_level_bind_collision

Prior verification for this fix: CARGO_TARGET_DIR=/tmp/gunbc-v3-target-bind-collision cargo test -p v3-compiler div_prelude_top_level_bind_collision -- --nocapture passed: 3 passed. No additional source change is needed for this repeated/stale item.

— sent from valiant-lynx-650 (inbox #862); reply at #862

@briansrls
briansrls merged commit 0e88e46 into main Apr 28, 2026
4 checks passed
briansrls added a commit that referenced this pull request Apr 28, 2026
Pre-spawn implementation work has progressed materially while #1078
was in flight; managers spawning today would otherwise dispatch
against work that's already landed. Refresh per-manager status
tables to reflect current main:

- **Substrate:** ValueBody::Map landed (#1017 + #1068 tightening);
  NominalOpacity fail-closed field-projection enforcement (#937);
  B4.8 Phase-2 site dissolution landed (#1069); B4.2 first-consumer
  wiring landed; T-Cost-Dimension fail-closed precedent (#1003).
- **Grounding:** Rust IntegerRangeFact mirror dissolved (#1005);
  Python primitives.dag landed (#1080); Go primitives tranche 1
  + additional (ac765ce + #1046); T-Ground-Engine Phase 2 slice 1
  (c0cc8b2) noted as pre-cascade footprint queued for cleanup
  wave per design-emission-model.md option (c).
- **Impossible-Bugs:** all 3 main implementations LANDED pre-spawn
  (nested-optional flatten #890 + #962 follow-ups; Int/Int totality-
  by-omission first slice #969; unenumerated effects lens landing
  #971). Day-1 work is class-close completion + sibling totalization
  dispatch (indexing/quotient/remainder), not initial implementation.
- **Pure Bootstrap:** kernel_algebra_profile substrate met via #1017
  + #1068 (consumer plumbing now the remaining R2 work, dispatchable
  Day-1); T-PB-Runtime ExecuteCommand typed-outcome hardening (#1049)
  + T-PB-B boundary coverage (#1082) advanced PB-Runtime foundation
  for R3 lens_apply.rs retirement gate.
- **Modeling:** Secret<T> producer side substantially advanced
  (#900 carrier + #937 fail-closed enforcement); tokenizer charclass
  scanner-order retype landed pre-cascade (242c65d); SourceFiltering
  canonical authority precedent (#1004).
- **Release:** initial closure-ledger snapshot now reflects all
  pre-spawn landings (Impossible-Bugs/Substrate/Ground/PB-Runtime).

Evaluator brief unchanged — new lane added 2026-04-28; nothing
landed yet (gated on PR-A through PR-E design lock cadence).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request Apr 29, 2026
…#1126)

* WIP: Gunbc PM

* WIP: Gunbc PM

* WIP: Gunbc PM

* WIP: Gunbc PM

* docs(briefs): refresh 6 R2 manager briefs post-#1078 merge

Aligns all 6 existing R2 manager briefs with #1078's locked design
decisions and structural cascade:

- Substrate Manager: adds T-Substrate-Lens-Primitive sub-lane (Q6+Q7+Q8
  locks) + PR-PreF Interval<D> consolidation + R3 T-CostLens-Composition
  continuation (Director cascade Item 3); references INVARIANTS §P1
  substrate-fact-introduction procedure + Q3 Cost<Unit> primitives.
- Grounding Manager: engine-reframe to 11 lanes (5 substrate-completion
  lanes replace prior single Engine: Coercion-Fold + LanguageSpec +
  Lifetime-Analyzer + Diagnostic + CrossTarget-Meta); consumes PR-F
  through PR-J cadence; PR #989 footprint queued for cleanup wave.
- Modeling Manager: int-lit item now consumes PR-PreF Interval<D> via
  Q1 lock; references INVARIANTS procedure for substrate-gap signaling.
- Pure Bootstrap Manager: adds R3 continuation lanes (T-LensProducer-
  Retirement XL with 3 internal sub-gates per Director cascade Item 8;
  T-FixedPoint; T-Tier3-Dissolution; 3 distributed bridge retirements
  per Director cascade Item 4 — distribute work, centralize ledger).
- Impossible-Bugs Manager: archives at R2 close per Director cascade;
  post-R2 emergent classes route to Substrate Manager continuation.
- Release Manager: 6→7 manager count; closure ledger spans all 6 other
  managers + sub-gate progress for T-LensProducer-Retirement; structural-
  acceptance-per-lane-close discipline (demo IS structural gate);
  thesis-claim mapping landed via #1078, refresh authority lives here;
  v2 release-doc-authority guardrail follow-up added as next narrow PR.

All 6 briefs now include: structural acceptance .dag TestClaim gates,
locked-design-decisions-consumed section, INVARIANTS §P1 procedure
references, and option-(c)-hybrid timing notes where R1-close-relevant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* docs(briefs): R2 manager-brief status refresh against landed PRs

Pre-spawn implementation work has progressed materially while #1078
was in flight; managers spawning today would otherwise dispatch
against work that's already landed. Refresh per-manager status
tables to reflect current main:

- **Substrate:** ValueBody::Map landed (#1017 + #1068 tightening);
  NominalOpacity fail-closed field-projection enforcement (#937);
  B4.8 Phase-2 site dissolution landed (#1069); B4.2 first-consumer
  wiring landed; T-Cost-Dimension fail-closed precedent (#1003).
- **Grounding:** Rust IntegerRangeFact mirror dissolved (#1005);
  Python primitives.dag landed (#1080); Go primitives tranche 1
  + additional (ac765ce + #1046); T-Ground-Engine Phase 2 slice 1
  (c0cc8b2) noted as pre-cascade footprint queued for cleanup
  wave per design-emission-model.md option (c).
- **Impossible-Bugs:** all 3 main implementations LANDED pre-spawn
  (nested-optional flatten #890 + #962 follow-ups; Int/Int totality-
  by-omission first slice #969; unenumerated effects lens landing
  #971). Day-1 work is class-close completion + sibling totalization
  dispatch (indexing/quotient/remainder), not initial implementation.
- **Pure Bootstrap:** kernel_algebra_profile substrate met via #1017
  + #1068 (consumer plumbing now the remaining R2 work, dispatchable
  Day-1); T-PB-Runtime ExecuteCommand typed-outcome hardening (#1049)
  + T-PB-B boundary coverage (#1082) advanced PB-Runtime foundation
  for R3 lens_apply.rs retirement gate.
- **Modeling:** Secret<T> producer side substantially advanced
  (#900 carrier + #937 fail-closed enforcement); tokenizer charclass
  scanner-order retype landed pre-cascade (242c65d); SourceFiltering
  canonical authority precedent (#1004).
- **Release:** initial closure-ledger snapshot now reflects all
  pre-spawn landings (Impossible-Bugs/Substrate/Ground/PB-Runtime).

Evaluator brief unchanged — new lane added 2026-04-28; nothing
landed yet (gated on PR-A through PR-E design lock cadence).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* docs(briefs): align Substrate Produces + Release R2-close acceptance

Per gpt-5-5-pro REQUEST_CHANGES on PR #1126 (sha:a9285fce). Two valid
findings on coordination-contract internal consistency:

1. **Substrate Produces list omitted ValueBody::Map → PB signal.**
   The deliverables table at line 94 named ValueBody::Map as an R2
   unblocker for PB's kernel_algebra_profile mirror dissolution, but
   the cross-program "Produces" section listed 5 signals and did not
   include this one. Same dependency represented in two places with
   different authority.

   Fix: add ValueBody::Map carrier read-path/API + arrow-body
   evaluation as the 6th produced signal targeted at PB Manager;
   update count from 5 to 6 (also in Reporting-cadence line 150).
   Remove the "Adjacent territory" note about kernel_algebra_profile
   being a future sub-lane — substrate already landed via #1017+#1068.

2. **Release R2-close acceptance gate excluded PB from close criterion.**
   The brief's "Consumes" section correctly named all 6 other managers
   including PB, but the r2_close_signal_to_director_authored gate at
   line 103 used "5 R2-archiving managers" (Substrate-prereq /
   Modeling / Grounding / Impossible-Bugs / Evaluator) — could fire
   the R2-close signal while PB's R2-scope lanes (Tier 3 mirror
   dissolutions + kernel_algebra_profile consumer plumbing) are
   still open.

   Fix: gate becomes "all 6 other managers' R2-scope lanes complete"
   with explicit lane-set listed per manager. Distinguish R2-scope
   completion from manager-archives (Modeling/Impossible-Bugs archive;
   Substrate/PB continue into R3 with R3-scoped lanes — those don't
   gate R2 close).

Both are P2 single-authority alignments; no scope change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* feat(scripts): manager-brief authority consumer + self-test

Per gpt-5-5-pro meta-review on PR #1126: dissolves the recurring
"non-live authority consumed as live" pattern that surfaced 5+ times
during PR #1078 + #1126 review loops (codex tooling false-positives
naming non-existent files / sections; cursor-flagged single-authority
drift on Goal numbering + R3 continuation count).

v1 covers 4 of gpt-5-5-pro's 5 questions:

- **Q1 — cited file existence** — extracts markdown links from each
  brief, resolves relative paths, fails closed if any cited file
  doesn't exist on disk.
- **Q2 — cited section anchor existence** — for `path#anchor` links,
  verifies the anchor matches a slugified heading in the target file.
- **Q4 — `LANDED via #N` reachability** — two-stage check: (a) fast
  `git log --grep="(#N)"` for normal merge subjects, (b) fall back to
  `gh pr view` for squash-merges that drop the suffix (caught a real
  case for PR #900). Verifies merge SHA is `git merge-base
  --is-ancestor HEAD`.
- **Q5 — cross-brief projection consistency** — extracts manager/lane
  counts and verifies all briefs that mention a projection agree both
  cross-brief AND with canonical values from r2-structure.md /
  r3-structure.md (7 standing managers, 6 other managers, 10 R3
  lanes, 7 of 10 Evaluator-gated). Catches drift like "5 R2-archiving
  managers" vs "all 6 other managers".

Q3 (controlled status vocabulary) deferred to v2 — too subjective for
a mechanical check; tracked in script header as next narrowing.

**Self-test** (`scripts/test-check-manager-brief-authority.sh`):
7 contract assertions — negative cases for each of Q1/Q2/Q4/Q5 (×2)
+ fail-closed-on-missing-brief + positive case. Mirrors the
`test-check-release-doc-authority.sh` pattern.

**Wiring:**
- Makefile: `manager-brief-authority-check` + `-test` targets;
  `verify` runs the check.
- CI workflow: both check + self-test wired as named steps; check
  receives `GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}` for the gh API
  fallback in Q4.

**SIGPIPE under pipefail caveat documented inline:** `grep -q .` on a
piped `git log` causes pipefail to report failure (grep exits early,
git log gets SIGPIPE 141). Workaround: capture output and test
`-z`/`-n`. Pinned in Q4 implementation comment.

Closes the convergence move gpt-5-5-pro proposed; future review loops
that hit the same "non-live authority" class get caught at CI rather
than reviewer-by-reviewer prose iteration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief Q4 — gh-first for CI shallow-clone compat

CI failed on the first run of the checker because the workflow uses
fetch-depth=1 (shallow clone), so the Q4 fall-back stage's
git-log --grep="(#N)" can't see merge history. The two-stage check
worked locally because git log had full history; in CI it found
nothing on either stage.

Restructure Q4 to gh-first:

- **Stage 1 (primary):** gh pr view N --json state — returns MERGED
  for actually-merged PRs regardless of clone depth or squash-merge
  subject variance. CI passes GH_TOKEN automatically.
- **Stage 2 (fallback):** git log --grep — kept for offline dev /
  auth-blocked environments. In CI with fetch-depth=1 this stage
  finds nothing; that's why Stage 1 is primary.

Reasoning: "is this PR actually merged" is what we want to verify;
gh state=MERGED answers it directly. The previous git-log+ancestor
check was defense-in-depth, but actually fragile in shallow clones
which is the CI default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief check — set -e + return interaction

Per claude-opus-4-7 review on PR #1126: three non-blocking findings
addressed.

1. **set -e + return non-zero**: the per-brief driver loop used
   `check_q1_file_existence "$brief"; rc=$?` — under set -euo
   pipefail, a function returning non-zero is treated as a failed
   command and exits the script before the accumulator runs. Result
   was "stop at first failing brief," not "report all violations in
   one pass" as intended.

   Fix: use `|| rc=$?` form (with explicit `rc=0` reset). This
   keeps set -e from firing on expected-non-zero returns while still
   capturing the count.

2. **Q2 doc/code mismatch**: header comment promised both
   `path#anchor` markdown form AND `§"section name"` prose form.
   Implementation only handled markdown. Trim the comment to
   match the code; track prose-form in v2 follow-up alongside
   Q3 status-vocabulary as next narrowing.

3. **Q5 pattern overlap (exploratory)**: claude-opus-4-7 flagged
   that "standing managers" might substring-match "standing R2
   managers". Empirical check shows it doesn't (POSIX regex
   requires the exact "standing managers" sequence; "R2 " breaks
   the match). Documented inline; no pattern change needed.

8-bit return-code truncation noted by reviewer is theoretical at
current scale (briefs typically have <10 violations) and is now
moot since the global `violations` accumulator is plain bash
arithmetic; only the per-function `return` is uint8-bounded.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief Q4 — explicit --repo for CI gh detection

CI failed Q4 even after gh-first restructure: actions/checkout@v4's
shallow clone exposes the remote in a form 'gh pr view' doesn't always
auto-detect, so the stage-1 gh call returned empty (no error message
in v1 because stderr was redirected to /dev/null) and the stage-2
git-log fallback also failed (shallow clone has no merge history).

Three fixes in this commit:

1. **Derive REPO_SLUG from `git config remote.origin.url`** at script
   start. Falls back to "gunb-ai/gunbc" if origin isn't readable
   (self-test runs in tmpdir with no remote).

2. **Pass `--repo "$REPO_SLUG"` explicitly to `gh pr view`** so it
   doesn't have to infer from the cwd's git remote.

3. **Capture gh stderr** to a temp file and surface it in the
   violation diagnostic. If gh is auth-failing or rate-limited,
   the violation message now shows why instead of looking like
   "PR doesn't exist."

Both checker + self-test still pass locally. CI should now succeed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ci): grant pull-requests:read for manager-brief authority check

The check uses `gh pr view --json state` to verify "LANDED via #N"
claims map to MERGED PRs. Default GITHUB_TOKEN scopes only include
contents:read; pull-request access fails with:

  GraphQL: Resource not accessible by integration (repository.pullRequest)

Surfaced when the script's stderr-capture fix (ea33aeb) made the
actual error message visible — diagnostic improvement paid off
immediately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* fix(scripts): manager-brief checker — markdown-bold + heading-strip + Q3 trigger

Per gpt-5-5-pro REQUEST_CHANGES on PR #1126 (sha:ea33aeb9). Three
findings — one BLOCKING (Q5 was ceremonial on its load-bearing
projection), one secondary (heading-strip glob bug), one coverage gap.

1. **Q5 markdown-bold mismatch (BLOCKING).** Live briefs use
   `Names this manager one of **7** standing R2 managers` — markdown
   emphasis around the count. The pre-fix regex `[0-9]+ standing R2
   managers` required a bare leading digit, so it matched zero claims
   on every brief. Counts_seen stayed empty → "0 counts seen → silent
   OK" branch fired → check passed ceremonially. A future drift to
   `**6** standing R2 managers` would have been invisible.

   Fix: regex now optionally accepts `**` before and after the digit:
   `\*?\*?[0-9]+\*?\*? standing R2 managers`. Extraction strips
   asterisks (`tr -d '*'`) before parsing. Verified on live briefs:

   $ grep -oE '\*?\*?[0-9]+\*?\*? standing R2 managers' docs/briefs/r2-*-manager.md
   docs/briefs/r2-evaluator-manager.md:**7** standing R2 managers
   docs/briefs/r2-grounding-manager.md:**7** standing R2 managers
   docs/briefs/r2-impossible-bugs-manager.md:**7** standing R2 managers
   docs/briefs/r2-modeling-manager.md:**7** standing R2 managers
   docs/briefs/r2-pure-bootstrap-manager.md:**7** standing R2 managers
   docs/briefs/r2-release-manager.md:**7** standing R2 managers
   docs/briefs/r2-substrate-manager.md:**7** standing R2 managers

   Now actually catches all 7 briefs' projections.

2. **Heading-strip glob bug (Q2 secondary).** `${heading##\#* }` is
   a Bash glob that strips through the LAST space, so
   "## Goal 7 — Evaluator XL" becomes "XL" instead of "Goal 7 —
   Evaluator XL". Multi-word heading anchors silently false-fail.

   Fix: introduced `strip_heading_marker()` helper using sed regex
   `^#{1,6}[[:space:]]+` for accurate prefix-only stripping.

3. **Self-test fixture format mismatch (coverage gap).** Self-test
   used bare-digit form ("7 standing R2 managers"); live briefs use
   markdown-bold ("**7** standing R2 managers"). Fixture proved Q5
   for a format the live docs don't use, masking finding 1.

   Fix: updated all clean + drift fixtures in self-test to use
   markdown-bold form. Verifies Q5 catches the actual format.

4. **Q3 dissolution trigger (per debt-tracking discipline).** Previous
   "v2; the next narrowing opportunity" was a future bucket without
   a checkable trigger. Replaced with concrete trigger: "first
   reviewer-flagged status-string drift class that Q1/Q2/Q4/Q5
   don't catch." Until that surfaces, status vocabulary is captured
   indirectly via Q5 count-projection consistency.

Local checker + self-test still pass after fixes; Q5 now actually
fires on live brief content rather than silently passing ceremonial.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts+briefs): manager-brief Q2 prose-form check + align 7 citations

Per gpt-5-5-pro BLOCKING on PR #1126 (sha:91b5274fc): Q2 explicitly
excluded prose §"section name" citations, but live briefs use those
for load-bearing INVARIANTS / r2-structure / design authority claims.
The exclusion left 12 real authority drifts uncheckable.

**Implements Q2-prose** (in addition to Q2-markdown-anchor):

For each `§"quoted section"` or `§AnchorToken` in a brief:

1. Compute the prefix BEFORE this citation (running prefix; the bug
   in v0 was using before-first-§ for every iteration, so subsequent
   citations on the same line resolved against the first link's
   target instead of their own).
2. Find the most recent markdown link `[text](path)` in the prefix —
   that's the cited file. Fall back to bare `<NAME>.md` token via
   `resolve_authority_file()` (tries `$ROOT/`, `$ROOT/docs/`,
   `$ROOT/docs/thesis/`, `$ROOT/docs/briefs/`).
3. `grep -F` for the section text in the cited file. Permissive
   substring match (vs Q2-markdown's slug match) — accepts
   paraphrased section names while still catching the load-bearing
   "section deleted" failure mode.

**Caught 12 real drifts on first run** — all now fixed:

- r2-evaluator-manager.md: §"Goal 7 — Evaluator XL" + §"Evaluator
  Manager (added 2026-04-28 as Goal 7)" → §"Evaluator Manager
  (added 2026-04-28 amendment)" (matches r2-structure.md:159 actual
  heading)
- r2-grounding-manager.md: §"Tier 1 — Structural correctness —
  Grounding completeness" → §"Tier 1 — Structural correctness"
  (matches THESIS.md:168 actual prose)
- r2-impossible-bugs-manager.md (×2): §"R2 manager continuation"
  → §"Manager structure" (matches r3-structure.md:103 actual heading)
- r2-impossible-bugs-manager.md: §Q1-Q3 + §Q6 → §Q1, §Q2, §Q3 + §Q6
  (range citation didn't match anything literal; expand to discrete)
- r2-release-manager.md (×2): §"R2 manager continuation"
  → §"Manager structure"
- r2-release-manager.md (×2): §v2-guardrail-requirement-3
  → §"v2 guardrail requirements" (matches r2-structure.md:490 body)
- r2-substrate-manager.md: §"R3 lane structure" → §"Lane structure"
  (matches r3-structure.md:86 actual heading)

Local checker + self-test still pass after fixes.

Reinforces gpt-5-5-pro's earlier meta-observation: a checker that
names a discipline but doesn't enforce it on the live format is
documented cheating. Q2-prose closes that gap; the briefs' authority
citations now have to match section text that actually exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(scripts): manager-brief — mktemp gh-stderr + Q1 false-pos trigger

Per claude-opus-4-7 APPROVE-with-exploratory-observations on PR #1126
(sha:91b5274f). Two non-blocking cleanups landed.

1. **gh-stderr capture: $$ → mktemp.** Previous form used
   `/tmp/gh-stderr-$$` which is fine in CI but a crashed run on a
   shared dev box could leak the file. `mktemp` gives a unique path
   + paired cleanup in scope.

2. **Q1 false-positive trigger documented.** Q1 currently treats
   every `](path)` as a filesystem reference. Markdown reference-
   style link definitions and code-block examples containing
   `](foo)` would false-positive. No briefs use either form today;
   added DISSOLUTION TRIGGER comment naming the condition that
   would force context-aware extraction (skip fenced code blocks
   + reference definitions).

The third observation (squash-merge for the WIP: Gunbc PM commits)
is a merge-time decision; PR-level chore.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* fix(scripts): manager-brief Q4 case-insensitive + title-case test

Per gpt-5-5-pro APPROVE_WITH_COMMENTS on PR #1126 (sha:91b5274f →
6dafaec): Q4 silently missed title-case "Landed via #N" claims.

**Finding 1 (Q4 case sensitivity):** live briefs use three case forms
of "landed via #N":
  - UPPERCASE — emphasized status-table claims (most common)
  - lowercase — inline prose ("landed via #900", "landed via #937", ...)
  - title-case — sentence-leading headings (r2-release-manager.md:113
    "Landed via #1078:")
The pre-fix regex `(LANDED|landed) via` missed the title-case form,
silently passing any future unique `Landed via #N` claim. Fix:
`grep -oEi 'landed via #[0-9]+'` (case-insensitive flag).

**Finding 2 (Q4 self-test gap):** Q4 negative fixture used UPPERCASE
"LANDED via #88888888"; positive fixture had no landed-PR claim at
all. Title-case wasn't covered. Fixes:

- Added `test_negative_q4_unreachable_pr_titlecase` using "Landed via
  #88888887" — verifies case-insensitive Q4 catches title-case.
- Updated `write_clean_briefs` clean fixture to include
  "Substrate landed via #999" (lowercase, matching real brief
  format). Q4 positive path is now non-vacuous: tmp git repo seeds
  "(#999)" merge subject so this resolves cleanly.

**Finding 3 (Q2 prose deferral note)**: STALE — Q2-prose was
implemented in 97affdb (2 commits before this review). The reviewer
cited line numbers from before the implementation; current code at
`scripts/check-manager-brief-authority.sh:121` says "Two forms covered"
not "v2 candidate". No action needed.

Self-test now: 8 contract assertions (6 negative + 1 positive +
1 fail-closed-on-missing-brief).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(briefs): strip trailing whitespace at r2-evaluator-manager.md:46

Per codex review on PR #1126 (sha:3ba4f2c1): `git diff --check
origin/main...HEAD` flagged trailing whitespace inside the
PR-A-through-PR-E dependency-graph ASCII art. Removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request Apr 29, 2026
…1156)

* WIP: Gunbc PM

* WIP: Gunbc PM

* WIP: Gunbc PM

* WIP: Gunbc PM

* docs(briefs): refresh 6 R2 manager briefs post-#1078 merge

Aligns all 6 existing R2 manager briefs with #1078's locked design
decisions and structural cascade:

- Substrate Manager: adds T-Substrate-Lens-Primitive sub-lane (Q6+Q7+Q8
  locks) + PR-PreF Interval<D> consolidation + R3 T-CostLens-Composition
  continuation (Director cascade Item 3); references INVARIANTS §P1
  substrate-fact-introduction procedure + Q3 Cost<Unit> primitives.
- Grounding Manager: engine-reframe to 11 lanes (5 substrate-completion
  lanes replace prior single Engine: Coercion-Fold + LanguageSpec +
  Lifetime-Analyzer + Diagnostic + CrossTarget-Meta); consumes PR-F
  through PR-J cadence; PR #989 footprint queued for cleanup wave.
- Modeling Manager: int-lit item now consumes PR-PreF Interval<D> via
  Q1 lock; references INVARIANTS procedure for substrate-gap signaling.
- Pure Bootstrap Manager: adds R3 continuation lanes (T-LensProducer-
  Retirement XL with 3 internal sub-gates per Director cascade Item 8;
  T-FixedPoint; T-Tier3-Dissolution; 3 distributed bridge retirements
  per Director cascade Item 4 — distribute work, centralize ledger).
- Impossible-Bugs Manager: archives at R2 close per Director cascade;
  post-R2 emergent classes route to Substrate Manager continuation.
- Release Manager: 6→7 manager count; closure ledger spans all 6 other
  managers + sub-gate progress for T-LensProducer-Retirement; structural-
  acceptance-per-lane-close discipline (demo IS structural gate);
  thesis-claim mapping landed via #1078, refresh authority lives here;
  v2 release-doc-authority guardrail follow-up added as next narrow PR.

All 6 briefs now include: structural acceptance .dag TestClaim gates,
locked-design-decisions-consumed section, INVARIANTS §P1 procedure
references, and option-(c)-hybrid timing notes where R1-close-relevant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* docs(briefs): R2 manager-brief status refresh against landed PRs

Pre-spawn implementation work has progressed materially while #1078
was in flight; managers spawning today would otherwise dispatch
against work that's already landed. Refresh per-manager status
tables to reflect current main:

- **Substrate:** ValueBody::Map landed (#1017 + #1068 tightening);
  NominalOpacity fail-closed field-projection enforcement (#937);
  B4.8 Phase-2 site dissolution landed (#1069); B4.2 first-consumer
  wiring landed; T-Cost-Dimension fail-closed precedent (#1003).
- **Grounding:** Rust IntegerRangeFact mirror dissolved (#1005);
  Python primitives.dag landed (#1080); Go primitives tranche 1
  + additional (ac765ce + #1046); T-Ground-Engine Phase 2 slice 1
  (c0cc8b2) noted as pre-cascade footprint queued for cleanup
  wave per design-emission-model.md option (c).
- **Impossible-Bugs:** all 3 main implementations LANDED pre-spawn
  (nested-optional flatten #890 + #962 follow-ups; Int/Int totality-
  by-omission first slice #969; unenumerated effects lens landing
  #971). Day-1 work is class-close completion + sibling totalization
  dispatch (indexing/quotient/remainder), not initial implementation.
- **Pure Bootstrap:** kernel_algebra_profile substrate met via #1017
  + #1068 (consumer plumbing now the remaining R2 work, dispatchable
  Day-1); T-PB-Runtime ExecuteCommand typed-outcome hardening (#1049)
  + T-PB-B boundary coverage (#1082) advanced PB-Runtime foundation
  for R3 lens_apply.rs retirement gate.
- **Modeling:** Secret<T> producer side substantially advanced
  (#900 carrier + #937 fail-closed enforcement); tokenizer charclass
  scanner-order retype landed pre-cascade (242c65d); SourceFiltering
  canonical authority precedent (#1004).
- **Release:** initial closure-ledger snapshot now reflects all
  pre-spawn landings (Impossible-Bugs/Substrate/Ground/PB-Runtime).

Evaluator brief unchanged — new lane added 2026-04-28; nothing
landed yet (gated on PR-A through PR-E design lock cadence).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* docs(briefs): align Substrate Produces + Release R2-close acceptance

Per gpt-5-5-pro REQUEST_CHANGES on PR #1126 (sha:a9285fce). Two valid
findings on coordination-contract internal consistency:

1. **Substrate Produces list omitted ValueBody::Map → PB signal.**
   The deliverables table at line 94 named ValueBody::Map as an R2
   unblocker for PB's kernel_algebra_profile mirror dissolution, but
   the cross-program "Produces" section listed 5 signals and did not
   include this one. Same dependency represented in two places with
   different authority.

   Fix: add ValueBody::Map carrier read-path/API + arrow-body
   evaluation as the 6th produced signal targeted at PB Manager;
   update count from 5 to 6 (also in Reporting-cadence line 150).
   Remove the "Adjacent territory" note about kernel_algebra_profile
   being a future sub-lane — substrate already landed via #1017+#1068.

2. **Release R2-close acceptance gate excluded PB from close criterion.**
   The brief's "Consumes" section correctly named all 6 other managers
   including PB, but the r2_close_signal_to_director_authored gate at
   line 103 used "5 R2-archiving managers" (Substrate-prereq /
   Modeling / Grounding / Impossible-Bugs / Evaluator) — could fire
   the R2-close signal while PB's R2-scope lanes (Tier 3 mirror
   dissolutions + kernel_algebra_profile consumer plumbing) are
   still open.

   Fix: gate becomes "all 6 other managers' R2-scope lanes complete"
   with explicit lane-set listed per manager. Distinguish R2-scope
   completion from manager-archives (Modeling/Impossible-Bugs archive;
   Substrate/PB continue into R3 with R3-scoped lanes — those don't
   gate R2 close).

Both are P2 single-authority alignments; no scope change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* feat(scripts): manager-brief authority consumer + self-test

Per gpt-5-5-pro meta-review on PR #1126: dissolves the recurring
"non-live authority consumed as live" pattern that surfaced 5+ times
during PR #1078 + #1126 review loops (codex tooling false-positives
naming non-existent files / sections; cursor-flagged single-authority
drift on Goal numbering + R3 continuation count).

v1 covers 4 of gpt-5-5-pro's 5 questions:

- **Q1 — cited file existence** — extracts markdown links from each
  brief, resolves relative paths, fails closed if any cited file
  doesn't exist on disk.
- **Q2 — cited section anchor existence** — for `path#anchor` links,
  verifies the anchor matches a slugified heading in the target file.
- **Q4 — `LANDED via #N` reachability** — two-stage check: (a) fast
  `git log --grep="(#N)"` for normal merge subjects, (b) fall back to
  `gh pr view` for squash-merges that drop the suffix (caught a real
  case for PR #900). Verifies merge SHA is `git merge-base
  --is-ancestor HEAD`.
- **Q5 — cross-brief projection consistency** — extracts manager/lane
  counts and verifies all briefs that mention a projection agree both
  cross-brief AND with canonical values from r2-structure.md /
  r3-structure.md (7 standing managers, 6 other managers, 10 R3
  lanes, 7 of 10 Evaluator-gated). Catches drift like "5 R2-archiving
  managers" vs "all 6 other managers".

Q3 (controlled status vocabulary) deferred to v2 — too subjective for
a mechanical check; tracked in script header as next narrowing.

**Self-test** (`scripts/test-check-manager-brief-authority.sh`):
7 contract assertions — negative cases for each of Q1/Q2/Q4/Q5 (×2)
+ fail-closed-on-missing-brief + positive case. Mirrors the
`test-check-release-doc-authority.sh` pattern.

**Wiring:**
- Makefile: `manager-brief-authority-check` + `-test` targets;
  `verify` runs the check.
- CI workflow: both check + self-test wired as named steps; check
  receives `GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}` for the gh API
  fallback in Q4.

**SIGPIPE under pipefail caveat documented inline:** `grep -q .` on a
piped `git log` causes pipefail to report failure (grep exits early,
git log gets SIGPIPE 141). Workaround: capture output and test
`-z`/`-n`. Pinned in Q4 implementation comment.

Closes the convergence move gpt-5-5-pro proposed; future review loops
that hit the same "non-live authority" class get caught at CI rather
than reviewer-by-reviewer prose iteration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief Q4 — gh-first for CI shallow-clone compat

CI failed on the first run of the checker because the workflow uses
fetch-depth=1 (shallow clone), so the Q4 fall-back stage's
git-log --grep="(#N)" can't see merge history. The two-stage check
worked locally because git log had full history; in CI it found
nothing on either stage.

Restructure Q4 to gh-first:

- **Stage 1 (primary):** gh pr view N --json state — returns MERGED
  for actually-merged PRs regardless of clone depth or squash-merge
  subject variance. CI passes GH_TOKEN automatically.
- **Stage 2 (fallback):** git log --grep — kept for offline dev /
  auth-blocked environments. In CI with fetch-depth=1 this stage
  finds nothing; that's why Stage 1 is primary.

Reasoning: "is this PR actually merged" is what we want to verify;
gh state=MERGED answers it directly. The previous git-log+ancestor
check was defense-in-depth, but actually fragile in shallow clones
which is the CI default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief check — set -e + return interaction

Per claude-opus-4-7 review on PR #1126: three non-blocking findings
addressed.

1. **set -e + return non-zero**: the per-brief driver loop used
   `check_q1_file_existence "$brief"; rc=$?` — under set -euo
   pipefail, a function returning non-zero is treated as a failed
   command and exits the script before the accumulator runs. Result
   was "stop at first failing brief," not "report all violations in
   one pass" as intended.

   Fix: use `|| rc=$?` form (with explicit `rc=0` reset). This
   keeps set -e from firing on expected-non-zero returns while still
   capturing the count.

2. **Q2 doc/code mismatch**: header comment promised both
   `path#anchor` markdown form AND `§"section name"` prose form.
   Implementation only handled markdown. Trim the comment to
   match the code; track prose-form in v2 follow-up alongside
   Q3 status-vocabulary as next narrowing.

3. **Q5 pattern overlap (exploratory)**: claude-opus-4-7 flagged
   that "standing managers" might substring-match "standing R2
   managers". Empirical check shows it doesn't (POSIX regex
   requires the exact "standing managers" sequence; "R2 " breaks
   the match). Documented inline; no pattern change needed.

8-bit return-code truncation noted by reviewer is theoretical at
current scale (briefs typically have <10 violations) and is now
moot since the global `violations` accumulator is plain bash
arithmetic; only the per-function `return` is uint8-bounded.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief Q4 — explicit --repo for CI gh detection

CI failed Q4 even after gh-first restructure: actions/checkout@v4's
shallow clone exposes the remote in a form 'gh pr view' doesn't always
auto-detect, so the stage-1 gh call returned empty (no error message
in v1 because stderr was redirected to /dev/null) and the stage-2
git-log fallback also failed (shallow clone has no merge history).

Three fixes in this commit:

1. **Derive REPO_SLUG from `git config remote.origin.url`** at script
   start. Falls back to "gunb-ai/gunbc" if origin isn't readable
   (self-test runs in tmpdir with no remote).

2. **Pass `--repo "$REPO_SLUG"` explicitly to `gh pr view`** so it
   doesn't have to infer from the cwd's git remote.

3. **Capture gh stderr** to a temp file and surface it in the
   violation diagnostic. If gh is auth-failing or rate-limited,
   the violation message now shows why instead of looking like
   "PR doesn't exist."

Both checker + self-test still pass locally. CI should now succeed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ci): grant pull-requests:read for manager-brief authority check

The check uses `gh pr view --json state` to verify "LANDED via #N"
claims map to MERGED PRs. Default GITHUB_TOKEN scopes only include
contents:read; pull-request access fails with:

  GraphQL: Resource not accessible by integration (repository.pullRequest)

Surfaced when the script's stderr-capture fix (ea33aeb) made the
actual error message visible — diagnostic improvement paid off
immediately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* fix(scripts): manager-brief checker — markdown-bold + heading-strip + Q3 trigger

Per gpt-5-5-pro REQUEST_CHANGES on PR #1126 (sha:ea33aeb9). Three
findings — one BLOCKING (Q5 was ceremonial on its load-bearing
projection), one secondary (heading-strip glob bug), one coverage gap.

1. **Q5 markdown-bold mismatch (BLOCKING).** Live briefs use
   `Names this manager one of **7** standing R2 managers` — markdown
   emphasis around the count. The pre-fix regex `[0-9]+ standing R2
   managers` required a bare leading digit, so it matched zero claims
   on every brief. Counts_seen stayed empty → "0 counts seen → silent
   OK" branch fired → check passed ceremonially. A future drift to
   `**6** standing R2 managers` would have been invisible.

   Fix: regex now optionally accepts `**` before and after the digit:
   `\*?\*?[0-9]+\*?\*? standing R2 managers`. Extraction strips
   asterisks (`tr -d '*'`) before parsing. Verified on live briefs:

   $ grep -oE '\*?\*?[0-9]+\*?\*? standing R2 managers' docs/briefs/r2-*-manager.md
   docs/briefs/r2-evaluator-manager.md:**7** standing R2 managers
   docs/briefs/r2-grounding-manager.md:**7** standing R2 managers
   docs/briefs/r2-impossible-bugs-manager.md:**7** standing R2 managers
   docs/briefs/r2-modeling-manager.md:**7** standing R2 managers
   docs/briefs/r2-pure-bootstrap-manager.md:**7** standing R2 managers
   docs/briefs/r2-release-manager.md:**7** standing R2 managers
   docs/briefs/r2-substrate-manager.md:**7** standing R2 managers

   Now actually catches all 7 briefs' projections.

2. **Heading-strip glob bug (Q2 secondary).** `${heading##\#* }` is
   a Bash glob that strips through the LAST space, so
   "## Goal 7 — Evaluator XL" becomes "XL" instead of "Goal 7 —
   Evaluator XL". Multi-word heading anchors silently false-fail.

   Fix: introduced `strip_heading_marker()` helper using sed regex
   `^#{1,6}[[:space:]]+` for accurate prefix-only stripping.

3. **Self-test fixture format mismatch (coverage gap).** Self-test
   used bare-digit form ("7 standing R2 managers"); live briefs use
   markdown-bold ("**7** standing R2 managers"). Fixture proved Q5
   for a format the live docs don't use, masking finding 1.

   Fix: updated all clean + drift fixtures in self-test to use
   markdown-bold form. Verifies Q5 catches the actual format.

4. **Q3 dissolution trigger (per debt-tracking discipline).** Previous
   "v2; the next narrowing opportunity" was a future bucket without
   a checkable trigger. Replaced with concrete trigger: "first
   reviewer-flagged status-string drift class that Q1/Q2/Q4/Q5
   don't catch." Until that surfaces, status vocabulary is captured
   indirectly via Q5 count-projection consistency.

Local checker + self-test still pass after fixes; Q5 now actually
fires on live brief content rather than silently passing ceremonial.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts+briefs): manager-brief Q2 prose-form check + align 7 citations

Per gpt-5-5-pro BLOCKING on PR #1126 (sha:91b5274fc): Q2 explicitly
excluded prose §"section name" citations, but live briefs use those
for load-bearing INVARIANTS / r2-structure / design authority claims.
The exclusion left 12 real authority drifts uncheckable.

**Implements Q2-prose** (in addition to Q2-markdown-anchor):

For each `§"quoted section"` or `§AnchorToken` in a brief:

1. Compute the prefix BEFORE this citation (running prefix; the bug
   in v0 was using before-first-§ for every iteration, so subsequent
   citations on the same line resolved against the first link's
   target instead of their own).
2. Find the most recent markdown link `[text](path)` in the prefix —
   that's the cited file. Fall back to bare `<NAME>.md` token via
   `resolve_authority_file()` (tries `$ROOT/`, `$ROOT/docs/`,
   `$ROOT/docs/thesis/`, `$ROOT/docs/briefs/`).
3. `grep -F` for the section text in the cited file. Permissive
   substring match (vs Q2-markdown's slug match) — accepts
   paraphrased section names while still catching the load-bearing
   "section deleted" failure mode.

**Caught 12 real drifts on first run** — all now fixed:

- r2-evaluator-manager.md: §"Goal 7 — Evaluator XL" + §"Evaluator
  Manager (added 2026-04-28 as Goal 7)" → §"Evaluator Manager
  (added 2026-04-28 amendment)" (matches r2-structure.md:159 actual
  heading)
- r2-grounding-manager.md: §"Tier 1 — Structural correctness —
  Grounding completeness" → §"Tier 1 — Structural correctness"
  (matches THESIS.md:168 actual prose)
- r2-impossible-bugs-manager.md (×2): §"R2 manager continuation"
  → §"Manager structure" (matches r3-structure.md:103 actual heading)
- r2-impossible-bugs-manager.md: §Q1-Q3 + §Q6 → §Q1, §Q2, §Q3 + §Q6
  (range citation didn't match anything literal; expand to discrete)
- r2-release-manager.md (×2): §"R2 manager continuation"
  → §"Manager structure"
- r2-release-manager.md (×2): §v2-guardrail-requirement-3
  → §"v2 guardrail requirements" (matches r2-structure.md:490 body)
- r2-substrate-manager.md: §"R3 lane structure" → §"Lane structure"
  (matches r3-structure.md:86 actual heading)

Local checker + self-test still pass after fixes.

Reinforces gpt-5-5-pro's earlier meta-observation: a checker that
names a discipline but doesn't enforce it on the live format is
documented cheating. Q2-prose closes that gap; the briefs' authority
citations now have to match section text that actually exists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(scripts): manager-brief — mktemp gh-stderr + Q1 false-pos trigger

Per claude-opus-4-7 APPROVE-with-exploratory-observations on PR #1126
(sha:91b5274f). Two non-blocking cleanups landed.

1. **gh-stderr capture: $$ → mktemp.** Previous form used
   `/tmp/gh-stderr-$$` which is fine in CI but a crashed run on a
   shared dev box could leak the file. `mktemp` gives a unique path
   + paired cleanup in scope.

2. **Q1 false-positive trigger documented.** Q1 currently treats
   every `](path)` as a filesystem reference. Markdown reference-
   style link definitions and code-block examples containing
   `](foo)` would false-positive. No briefs use either form today;
   added DISSOLUTION TRIGGER comment naming the condition that
   would force context-aware extraction (skip fenced code blocks
   + reference definitions).

The third observation (squash-merge for the WIP: Gunbc PM commits)
is a merge-time decision; PR-level chore.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

* fix(scripts): manager-brief Q4 case-insensitive + title-case test

Per gpt-5-5-pro APPROVE_WITH_COMMENTS on PR #1126 (sha:91b5274f →
6dafaec): Q4 silently missed title-case "Landed via #N" claims.

**Finding 1 (Q4 case sensitivity):** live briefs use three case forms
of "landed via #N":
  - UPPERCASE — emphasized status-table claims (most common)
  - lowercase — inline prose ("landed via #900", "landed via #937", ...)
  - title-case — sentence-leading headings (r2-release-manager.md:113
    "Landed via #1078:")
The pre-fix regex `(LANDED|landed) via` missed the title-case form,
silently passing any future unique `Landed via #N` claim. Fix:
`grep -oEi 'landed via #[0-9]+'` (case-insensitive flag).

**Finding 2 (Q4 self-test gap):** Q4 negative fixture used UPPERCASE
"LANDED via #88888888"; positive fixture had no landed-PR claim at
all. Title-case wasn't covered. Fixes:

- Added `test_negative_q4_unreachable_pr_titlecase` using "Landed via
  #88888887" — verifies case-insensitive Q4 catches title-case.
- Updated `write_clean_briefs` clean fixture to include
  "Substrate landed via #999" (lowercase, matching real brief
  format). Q4 positive path is now non-vacuous: tmp git repo seeds
  "(#999)" merge subject so this resolves cleanly.

**Finding 3 (Q2 prose deferral note)**: STALE — Q2-prose was
implemented in 97affdb (2 commits before this review). The reviewer
cited line numbers from before the implementation; current code at
`scripts/check-manager-brief-authority.sh:121` says "Two forms covered"
not "v2 candidate". No action needed.

Self-test now: 8 contract assertions (6 negative + 1 positive +
1 fail-closed-on-missing-brief).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(briefs): strip trailing whitespace at r2-evaluator-manager.md:46

Per codex review on PR #1126 (sha:3ba4f2c1): `git diff --check
origin/main...HEAD` flagged trailing whitespace inside the
PR-A-through-PR-E dependency-graph ASCII art. Removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(scripts): manager-brief Q2-prose digit-leading + negative test

Per gpt-5-5-pro REQUEST_CHANGES on PR #1126 (sha:b9f7a1c1): Q2-prose
extractor required §-followed-by-letter, silently skipping the
digit-leading citation forms used in the same diff.

Live brief usage caught:
  §4   — r2-evaluator/grounding/impossible-bugs/modeling/pure-bootstrap
  §6a  — r2-modeling-manager.md (cite of design-substrate-carrier-port-program §6a)
  §0.7 — r2-pure-bootstrap-manager.md (cite of debt-paydown-synthesis §0.7)
  §5   — r2-release-manager.md

All previously skipped → "Q2 (prose §) resolved" was vacuously true
on those lines.

Fix: regex `§[A-Za-z][A-Za-z0-9._-]*[A-Za-z0-9]|§[A-Za-z]`
     →    `§[A-Za-z0-9][A-Za-z0-9._-]*[A-Za-z0-9]|§[A-Za-z0-9]`
(extends [A-Za-z]-leading to [A-Za-z0-9]-leading; quoted form
unchanged).

Documented limitation: short digit-only tokens like §4 resolve
permissively because grep -F "4" matches anywhere; multi-character
tokens like §6a are discriminating.

Self-test gap (also flagged): added
`test_negative_q2_missing_prose_numeric_section` using §99zzz
(digit-leading, multi-char so substring match doesn't trivially
pass). Verifies regex extraction triggers Q2-prose violation on
digit-leading citation drift.

Self-test now: 9 contract assertions (7 negative + 1 positive +
1 fail-closed-on-missing-brief).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(briefs): consume Tier 1 design locks 1+2+3 from #1129

Director landed Items 1+2+3 design locks together via #1129
(`e1afabe47`):
- Item 1 (Q1 asymmetric bound algebra) — `docs/design-emission-model.md`
  §"Q1 — `BoundDeclaration` substrate type"
- Item 2 (reflection completeness) — NEW
  `docs/design-reflection-completeness.md`
- Item 3 (Q6.5 two-layer diagnostic-kind) — `docs/design-lens-framework.md`
  §"Q6.5 — Two-layer authority for diagnostic kinds"

Per agreed PM role on inbox #828: as each design-lock doc lands, PM
consumes the lock into worker brief updates (statuses move from
PENDING/gated → LIVE; cited authority anchors verified by the
manager-brief authority checker). Mostly mechanical.

Brief updates:

- **Substrate** (3 sites): T-Substrate-Lens-Primitive flips from
  "gated on PR-K" to "Q6/Q6.5/Q7/Q8 LANDED via #1129; ready to
  dispatch"; "Diagnostic-kind extensibility (Q6 lock)" replaced
  with the locked Q6.5 two-layer authority cite (Layer 1 closed sum
  Substrate-owned; Layer 2 lens-instance via inhabitance; additive
  widening of `Diagnostic.kind` named).
- **Evaluator** (5 sites): "Lens application gated on PR-C" → cites
  the landed reflection-completeness doc; PR-C row in cadence table
  flips to LANDED; Q6 disposition becomes Q6+Q6.5 with explicit
  cite to design-lens-framework.md §Q6.5; "Reflection completeness
  lives in PR-C" → "lives in design-reflection-completeness.md
  (LANDED via #1129)"; PR-C worker brief in pending list crossed
  out as superseded.
- **Modeling** (1 site): status header now cites Q1 lock landing
  with explicit anchor; int-lit item already references Interval<D>
  via PR-PreF.
- **Grounding** (2 sites): T-Ground-Diagnostic lane and Substrate-
  Manager-cross-program-dependency cite Q6.5 — clarifies lane is
  Layer-1 consumer (not Layer-2 author), no cross-manager handoff.
- **Pure Bootstrap** (1 site): Q6 disposition becomes Q6+Q6.5 +
  reflection-completeness cite added (load-bearing for R3-T-
  LensProducer-Retirement per design-reflection-completeness.md
  §"Cascade and gates" §7.3).
- **Impossible-Bugs** (1 site): Q6 cite becomes Q6+Q6.5; classes
  consume Layer 1, not author Layer 2.

Verified: `bash scripts/check-manager-brief-authority.sh` passes
all 7 briefs (Q1/Q2-md/Q2-prose/Q4/Q5); 9 contract assertions in
self-test still pass.

Note: one brief edit required restructuring (modeling-manager.md:3)
because the original cite put §"section" inside the markdown link's
display text, while the heuristic finds the rightmost `](path)` BEFORE
the §. Moved cite outside the link to align: `[file.md](path) §"section"`.
Same pattern as other landed cites; the checker enforces it
structurally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(scripts): manager-brief — concrete dissolution trigger for short-digit § limitation

Per codex APPROVE_WITH_COMMENTS on PR #1156 (sha:00540f36): the
short digit-only § resolve-permissively limitation was documented
and bounded but lacked a concrete dissolution trigger.

Updated to match Q3 dissolution-trigger discipline: trigger fires
on first reviewer-flagged stale `§N` (single-digit) citation that
survives the substring check because the digit appears elsewhere
in the target file. At that point the check tightens to require
structural context — match `§N` only if the target has a heading
`## N`, `### N`, etc. or numbered-list item at column 0.

Until that surfaces, multi-character disambiguation is the
load-bearing discriminator (and live briefs predominantly use
multi-char forms — §P1, §Q6, §Q6.5, §"Lane structure" — so
single-digit `§4` citations are uncommon).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(design): consume Q6.5 lock in worked examples + r2-structure Q6 row

Per Director (zesty-bear-812) endorsement on inbox #828: fold the
design-doc Q6.5-consumption edits originally drafted in PR #1137
(jolly-ram-908) into the canonical consumption PR. Single-sourced
consumption story; #1137 ends up as a clean no-op redirect.

8 lens-framework worked-example reframes + 1 r2-structure Q6 row
update. All consume the Q6.5 two-layer authority disposition
landed via #1129:

**design-lens-framework.md (8 sites):**
- §"Lens<TenantFlow>" `validate(dag, set)`: "new
  CompilerDiagnosticKind variant" → "lens-local diagnostic-kind
  declaration"
- §"Lens<IFC>" `validate(dag, label)`: same reframe for
  IFCDowngradeViolation
- §"D5 Failure modes": "appropriate CompilerDiagnosticKind variant
  (lens instances may extend CompilerDiagnosticKind...)" →
  "appropriate lens-local diagnostic-kind declaration"
- §Q6 alternative (d): "pushes structural failure data into
  Diagnostic.kind (which is CompilerDiagnosticKind sum type —
  already extends per-instance per
  feedback_state_space_vs_behavioral_invariants)" → "pushes
  structural failure data into lens-local Diagnostic.kind
  declarations"
- §Q6 anti-bridge claim renaming `no_string_parsing_in_witness_consumers`
  description: "Diagnostic.kind extensions" → "lens-local
  Diagnostic.kind declarations"
- §Q6 Recommendation (d): "encode into Diagnostic.kind sum-type
  variants. Lens instances ... extend CompilerDiagnosticKind
  with their own variants" → "encode into lens-local Diagnostic.kind
  declarations. Lens instances ... declare their own kinds beside
  the lens instance"
- §Q6 DECISION line: "(c)/(d) hybrid — Witness<C> stays as-is;
  rich structural validation failures encode into Diagnostic.kind
  extensions via the lens-framework's structural inhabitance" →
  same with "lens-local Diagnostic.kind declarations"; date stamp
  augmented with "refined 2026-04-29"
- §Q6 Director's framing #1: "CapabilityViolation as a
  CompilerDiagnosticKind variant is uniform" →
  "CapabilityViolation as a lens-local diagnostic-kind declaration
  is uniform"

**r2-structure.md (1 site):** §"Q1-Q8 disposition" Q6 row updated
to match design-lens-framework's locked language: "encode into
Diagnostic.kind extensions via lens-framework's structural
inhabitance" → "encode into lens-local Diagnostic.kind declarations
via lens-framework structural inhabitance, not into the closed
compiler-core CompilerDiagnosticKind sum".

These edits are *editorial* — the Q6.5 lock at design-lens-framework.md
§"Q6.5 — Two-layer authority for diagnostic kinds" remains the
canonical authority; this just aligns the worked examples + r2-
structure summary row with that canonical phrasing so future
readers don't see the older "extends CompilerDiagnosticKind"
framing in worked examples and assume it survived.

Verified: manager-brief authority check passes (7 briefs / 0
violations); 9 contract assertions in self-test pass; release-doc
authority check passes.

Per inbox #828 + #1130 coordination: jolly-ram-908 confirmed PR
#1137 will close as redundant once #1156 lands (the brief edits
were already absorbed by my prior consumption pass; these
design-doc edits are the residual that's now folded in).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* WIP: Gunbc PM

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls added a commit that referenced this pull request Apr 30, 2026
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

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

1. Story of the diff

This PR makes integer-style division explicit about failure instead of pretending every division produces a value. It introduces a new bootstrap-early std.error_primitives module with Result<ok, err> and DivError, then changes OrderedRing.div from fn(T, T) -> T to fn(T, T) -> Result<T, DivError> at dsl/std/algebra.dag:184. The rest of the diff wires that new std module into the v2/v3 bootstrap closure: v2 gets a generated std_error_primitives.rs, the resolver exempts the new module from the implicit std.types dependency, v3 includes the file in bootstrap fixtures, and generated v3 DAG fixtures are regenerated with the extra declarations and shifted IDs.

The intended direction is good: division by zero and signed MIN / -1 overflow become modeled outcomes rather than target-side panics or raw / behavior. The load-bearing concern is that the new carrier is now part of substrate algebra, so its type parameters, target realizations, and tests need to be exact rather than “close enough.”

2. Invariant categories

  1. LAYER MODEL (substrate vs implementation).

Finding — BLOCKING. This is a substrate change: dsl/std/algebra.dag:184 changes the generic algebra witness to div: fn(T, T) -> Result<T, DivError>, but the error carrier is explicitly documented as “signed integer division” at dsl/std/error_primitives.dag:14 (// 🟡 STRUCTURAL-CARRIER: signed integer division (Int / Int) failure modes). That makes an Int-specific failure domain part of every OrderedRing<T> witness. Under illegal-states-unrepresentable/modeling-faithfulness discipline, a generic OrderedRing<T> should not structurally admit DivError::Overflow unless the carrier has signed machine-int overflow semantics. Either the division error needs to be a generic algebra-level error model, or the DivError totalization should live at the integer operation/realization layer rather than on the generic OrderedRing<T> field. This is substrate, so I would not merge it as-is. chatgpt-review-38107a92-17a8-42…

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

Finding — BLOCKING. The new Result declares its type params as ok and err (dsl/std/error_primitives.dag:12, type Result<ok, err> = Ok { value: ok } | Err { value: err }), and the generated bootstrap confirms those as AtomPayload::TypeParam("ok") / AtomPayload::TypeParam("err") at src/v3/compiler/src/bootstrap_generated.rs:20-21. But the target realizations for that same Result use {key} and {value} placeholders, for example src/v3/compiler/src/bootstrap_generated.rs:1233 contains ::core::result::Result<{key}, {value}>, with analogous Go/Python carriers at src/v3/compiler/src/bootstrap_generated.rs:1035 and src/v3/compiler/src/bootstrap_generated.rs:1132. That creates two authorities for the same generic arguments: substrate says ok/err, target strings say key/value. Unless the emitter has an undocumented positional special case for two-argument generics, this will either leave placeholders unsubstituted or bind the wrong convention. This violates single-authority metadata / facts-flow-forward: the target realization should use {ok} / {err}, or the carrier should intentionally name its params key / value everywhere. chatgpt-review-e3a95e0d-6860-4b…

  1. CODING.md.

Compliant. The implementation-facing Rust shape follows the typed-carrier convention rather than a primitive/sentinel return: src/v2/stage0/src/std_algebra.rs:79 changes div to Rc<dyn Fn(T, T) -> Rc<Result<T, DivError>>>, so callers are forced to handle Ok vs Err instead of receiving a fabricated T. That matches the structured-result guidance in CODING.md. chatgpt-review-7eebc4ff-499c-44…

  1. TESTING.md.

Finding — BLOCKING. The contract change at dsl/std/algebra.dag:184 is semantic, but the diff only updates bootstrap/parse fixture inclusion such as src/v2/tests/src/bootstrap.rs:513 and src/v2/tests/src/parse.rs:676; it does not add behavior-driven tests for the new division outcomes. For this change I would expect at least minimal coverage that ordinary division returns Ok, divisor zero returns Err(DivideByZero), signed MIN / -1 returns Err(Overflow), and the Rust/Go/Python target realizations compile or evaluate through the new carrier. Without those tests, the {key}/{value} mismatch above can survive regeneration unnoticed. This is a high-risk fail-closed substrate/target boundary, so the gap is blocking under the test discipline. chatgpt-review-ecde774c-f742-4a…

  1. LOCKED DESIGN DECISIONS.

N/A — I did not see this diff reference or modify a design item marked locked; the changes are in std algebra/error carriers, bootstrap fixture generation, and generated v2/v3 outputs.

  1. TRACKED vs UNTRACKED DEBT.

Compliant. The new carrier bridge is at least explicitly named and bounded in the source comments: dsl/std/error_primitives.dag:8-11 marks Result as a structural carrier and says it is temporary until target realizations consume the declaration directly, and dsl/std/error_primitives.dag:15-17 gives the corresponding trigger for DivError/target preludes. That satisfies the “documented, bounded, named dissolution trigger” shape for the temporary mirror.

3. Verdict

REQUEST_CHANGES. The direction is right, but the PR currently lands an Int-specific error carrier on a generic substrate algebra field and appears to mismatch Result’s declared type-parameter names with target realization placeholders. The missing behavior tests make those substrate/target issues too easy to miss.

@briansrls
briansrls deleted the subsume/pr-931 branch June 1, 2026 18:43
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