Skip to content

docs(r3): retire Go UnknownVariant debt row - #1545

Merged
briansrls merged 9 commits into
mainfrom
codex/retire-go-unknownvariant-ledger
May 3, 2026
Merged

briansrls merged 9 commits into
mainfrom
codex/retire-go-unknownvariant-ledger

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Why

silent-boar-29 verified the implementation already landed in PR #820: Go branch emission now returns EmitError::VariantParentNotFound and has regression coverage. The ledger still listed the row as open, so this PR records the actual state and keeps the debt-paydown queue from redispatching stale work.

R3 Debt Receipt

  • Debt paid — Go branch emits UnknownVariant is marked retired in docs/debt/r3-debt-paydown-ledger-2026-05-02.md; ROADMAP.md now cites PR fix(v3): B1 Go UnknownVariant fabrication to VariantParentNotFound #820 as the closure receipt. Dissolution mechanism: typed EmitError::VariantParentNotFound, no UnknownVariant fallback, missing-parent regression coverage.
  • Debt found + routed — none.
  • No debt touched — not applicable.

Validation

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

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 2fe309be · Trigger: schedule
  • Comparison: origin/main @ 3c77a60c ... review/pr-1545-2fe309be @ 2fe309be
  • Thinking: 8.8s wall

Docs-only PR retiring a debt row. Counts and renumbering are consistent (60→59 open, 3→4 retired; list renumbered 4-6, retirement targets renumbered).

Verdict: APPROVE — docs-only debt-ledger update marking the Go UnknownVariant row retired via PR #820. Bucket counts, retired list, highest-leverage list, and bundle recommendation all updated consistently. No code change, no invariant concerns.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head d04038a. The approval did not request changes; the only post-review diff is the parse corpus manifest refresh needed to fix the failing v3 check. No additional review fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: d04038a8 · Trigger: schedule
  • Comparison: origin/main @ 3c77a60c ... review/pr-1545-d04038a8 @ d04038a8
  • Thinking: 69s wall

Verdict: APPROVE

The Go UnknownVariant retirement is backed by existing code and regression coverage from PR #820, and the docs/ledger updates consistently move that row out of active debt. I found no concrete violations of the pinned invariants, coding, or testing guidance in this diff.

I did not run builds or tests, per instruction.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head b6102c7. The approval does not request changes; the only commits after the reviewed SHA are generated parse/bootstrap snapshot refreshes to satisfy CI after the induction.dag drift. No additional review fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: b6102c72 · Trigger: manual
  • Comparison: main @ 3c77a60c ... codex/retire-go-unknownvariant-ledger @ b6102c72
  • Conversation: View conversation

1. Story of the diff

This PR is a debt-ledger retirement, not the emitter fix itself. It takes the previously open ROADMAP row for Go branch emission fabricating UnknownVariant and marks it resolved by PR #820, explicitly saying the live behavior is now EmitError::VariantParentNotFound { variant_id } plus missing-parent regression coverage at ROADMAP.md:468. The R3 debt ledger is kept in sync: open implementation rows drop from 60 to 59, retired/stale rows rise from 3 to 4, the Go row changes from Open to Retired, and the item is removed from both the “highest-leverage” queue and the fail-closed mini-bundle at docs/debt/r3-debt-paydown-ledger-2026-05-02.md:20, :22, :73, :119, and :154.

The generated/test collateral then refreshes the derived view of src/v3/std/induction.dag: both bootstrap fixture snapshots update only numeric SourceSpan byte ranges, and the parse corpus manifest updates the induction entry hash at src/v3/compiler/tests/integration/parse_corpus_manifest.txt:43. I did not see live emitter logic, substrate schema, or hand-written Rust behavior changed in this diff.

2. Invariant categories

  1. LAYER MODEL — Compliant. The diff touches Dag-shaped generated fixture data, but only by updating existing SourceSpan coordinates such as span: SourceSpan::new("src/v3/std/induction.dag", 12076, 12638) at src/v3/compiler/src/bootstrap_generated.rs:24915; it does not introduce a new substrate type, variant, field, or cross-pass authority.
  2. INVARIANTS.md + modeling-discipline.md — Compliant. P5 Progress Is Dissolution is handled directly: the open debt row is struck through and resolved to a typed fail-closed receipt at ROADMAP.md:468, while the ledger changes the same item to Retired at docs/debt/r3-debt-paydown-ledger-2026-05-02.md:73. P3 Fail-Closed is also represented correctly in the retirement text: the recorded mechanism is EmitError::VariantParentNotFound { variant_id }, not a continued target-code fallback.
  3. CODING.md — N/A. No hand-written Rust functions, helpers, APIs, error shapes, naming, or method/free-function structure changed; the Rust changes are generated numeric snapshot updates only, for example src/v3/compiler/src/bootstrap_generated_without_parse_surface.rs:24915.
  4. TESTING.md — Compliant. This PR does not add new behavior needing a new test; it refreshes generated test/snapshot collateral instead. The parse corpus manifest records the updated src/v3/std/induction.dag digest at src/v3/compiler/tests/integration/parse_corpus_manifest.txt:43, and both bootstrap snapshot files carry matching generated span updates.
  5. LOCKED DESIGN DECISIONS — N/A. The diff does not alter a locked design document or change a locked substrate/design commitment; it records an existing PR fix(v3): B1 Go UnknownVariant fabrication to VariantParentNotFound #820 receipt in ROADMAP/ledger prose at ROADMAP.md:468.
  6. TRACKED vs UNTRACKED DEBT — Compliant. No new scaffold, TODO, bridge, or temporary shape is introduced. The tracked debt state moves in the right direction: counts are adjusted at docs/debt/r3-debt-paydown-ledger-2026-05-02.md:20 and :22, the retired receipt is enumerated at :107, and the item is removed from the active fail-closed bundle at :154.

3. Verdict

APPROVE

The diff consistently retires one tracked debt item and keeps the ledger/queue math aligned with that retirement. The generated snapshot and manifest changes are narrow derived-artifact refreshes, and I did not find a diff-supported invariant, testing, or coding finding.

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

Non-blocking — Strengths

  • ROADMAP.md The Go UnknownVariant row is retired with a concrete fail-closed receipt that matches the PR-head emitter behavior and regression test.
  • docs/debt/r3-debt-paydown-ledger-2026-05-02.md Ledger counts, row status, cleanup-only list, and dispatch bundle stay internally consistent after moving Go UnknownVariant to retired.

ROADMAP — Verified

  • Go UnknownVariant retirement: At PR head, Go sum-branch emission returns EmitError::VariantParentNotFound on missing variant-parent info and includes a regression test for that case.

✅ No blocking concerns.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head b6102c7. This is an approval and does not request changes; no code or docs fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: b6102c72 · Trigger: schedule
  • Comparison: origin/main @ 3c77a60c ... review/pr-1545-b6102c72 @ b6102c72
  • Thinking: 15s wall

APPROVE — Docs-only retirement of the Go UnknownVariant debt row plus mechanical bootstrap/parse-corpus snapshot refreshes. Bucket arithmetic is consistent (60→59 open, 3→4 retired, total 69 preserved), section renumbering in the ledger is internally consistent, and ROADMAP strikethrough preserves history while pointing to PR #820 as the retirement receipt. No invariant or modeling-discipline concerns in this diff.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head b6102c7. This review has no blocking concerns and requests no changes; no fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head b6102c7. This is an approval with no requested changes; no fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: a791e4e6 · Trigger: schedule
  • Comparison: origin/main @ d2e72d26 ... review/pr-1545-a791e4e6 @ a791e4e6
  • Thinking: 46s wall

Verdict: APPROVE

Docs-only retirement looks consistent: ROADMAP marks the Go UnknownVariant row resolved, the R3 ledger count moves one row from open to retired, and the dispatch target list removes that already-retired work. I found no concrete violations of the pinned invariants, coding, or testing guidance in this diff. No builds or tests run, per review instructions.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head. This is an approval with no requested changes; no fix required.\n\n— sent from cool-stag-230

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

✅ No blocking concerns in the docs-only retirement update.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head. This review reports no blocking concerns and requests no changes; no fix required.\n\n— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: cursor / composer-2
  • Commit: 4b4eea83 · Trigger: schedule
  • Comparison: origin/main @ 58b52651 ... review/pr-1545-4b4eea83 @ 4b4eea83
  • Thinking: 20s wall

Findings

None. The diff only updates ROADMAP.md and docs/debt/r3-debt-paydown-ledger-2026-05-02.md to record that the Go UnknownVariant item is addressed by PR #820 (EmitError::VariantParentNotFound, regression coverage). That matches INVARIANTS.md P3 (fail-closed) as documentation of closure, not a new behavior. CODING.md / TESTING.md do not apply to these markdown edits. Bucket totals stay consistent (open −1, retired +1; still 69 classified rows).

Verdict

APPROVE — Narrow docs/ledger retirement aligned with a named receipt (PR #820); no invariant or discipline issues in the changed lines.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head cd45d03. This is an approval with no requested changes; the current diff remains limited to ROADMAP.md and the R3 debt ledger retirement entry. No fix required.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: 4b4eea83 · Trigger: manual
  • Comparison: main @ 5c4c94bd ... codex/retire-go-unknownvariant-ledger @ cd45d038
  • Conversation: View conversation

1. Story of the diff

This PR is a debt-ledger cleanup for the Go UnknownVariant fail-closed row, not an implementation change. ROADMAP.md:468 converts the prior open finding into a struck-through historical row and records PR #820 as the receipt: Go branch emission now fails with EmitError::VariantParentNotFound { variant_id } instead of fabricating a Go case label. The R3 debt ledger then keeps the bookkeeping consistent: open rows drop from 60 to 59 and retired/stale rows rise from 3 to 4 at docs/debt/r3-debt-paydown-ledger-2026-05-02.md:20 and :22, the specific ledger row moves from Open to Retired at :73, the stale/retired section gains the Go entry at :107, and the highest-leverage target list plus first retirement bundle remove Go UnknownVariant from active work at :119 and :154.

I treated the PR #820 implementation and tests as outside this diff; this review is only about whether this PR’s documentation and debt-accounting changes are coherent.

2. Invariant categories

  1. LAYER MODEL — N/A. The diff is documentation/debt-ledger bookkeeping only; it does not touch Dag substrate types, implementation data models, lowering, emit code, or cross-pass carriers.
  2. INVARIANTS.md + modeling-discipline.md — Compliant. P3 Fail-Closed / C-6 / C-9 are handled as a retirement receipt rather than hidden: ROADMAP.md:468 says the prior fabricated "UnknownVariant" path is resolved by typed EmitError::VariantParentNotFound { variant_id }, and docs/debt/r3-debt-paydown-ledger-2026-05-02.md:73 mirrors that retirement instead of leaving the row open.
  3. CODING.md — N/A. No Rust implementation, helper shape, API surface, method/free-function choice, or error/result contract is changed in this diff.
  4. TESTING.md — Compliant. This PR does not add tests because it does not add behavior; the retirement text explicitly records that the underlying implementation receipt included missing-parent regression coverage at ROADMAP.md:468 and docs/debt/r3-debt-paydown-ledger-2026-05-02.md:73.
  5. LOCKED DESIGN DECISIONS — N/A. The diff does not alter a locked thesis/design decision or introduce divergence from one; it updates ROADMAP and an R3 debt ledger entry.
  6. TRACKED vs UNTRACKED DEBT — Compliant. The change reduces tracked open debt without adding a new scaffold/TODO: the bucket counts move one item from open to retired at docs/debt/r3-debt-paydown-ledger-2026-05-02.md:20 and :22, the stale/retired list adds the explicit PR fix(v3): B1 Go UnknownVariant fabrication to VariantParentNotFound #820 receipt at :107, and active-work planning removes Go UnknownVariant from the leverage list and bundle at :119 and :154.

3. Verdict

APPROVE

The diff is internally consistent debt retirement: it updates the ROADMAP row, ledger status, aggregate counts, stale/retired section, and active-work queues in the same direction. I found no new substrate shape, implementation behavior, scaffold, or test obligation introduced by this PR.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head c2b9720. This review is an approval with no requested changes; the live diff remains limited to ROADMAP.md and docs/debt/r3-debt-paydown-ledger-2026-05-02.md. No fix required.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: c2b97201 · Trigger: schedule
  • Comparison: origin/main @ 94c479cf ... review/pr-1545-c2b97201 @ c2b97201
  • Thinking: 44s wall

Verdict: APPROVE

Docs-only diff retires the Go UnknownVariant debt row consistently: ROADMAP now records the PR #820 resolution, the R3 ledger count moves one row from open to retired, and the dispatch list removes it from active fail-closed work. I found no concrete invariant, coding, or testing-discipline concerns in the changed lines. No tests run, per review instructions.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head c2b9720. This is an approval with no requested changes; the live diff remains docs-only in ROADMAP.md and docs/debt/r3-debt-paydown-ledger-2026-05-02.md. No fix required.

— sent from cool-stag-230

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: bfaf8616 · Trigger: schedule
  • Comparison: origin/main @ 0427f96f ... review/pr-1545-bfaf8616 @ bfaf8616
  • Thinking: 10s wall

APPROVE — Docs-only update retiring a debt row referencing PR #820. Counts updated consistently (60→59 open, 3→4 retired; numbered list renumbered; highest-leverage list renumbered 3-10 → 3-9; retirement bundle text updated). No code under review, no invariants implicated.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified against current head bfaf861. This is an approval with no requested changes; the live diff remains docs-only in ROADMAP.md and docs/debt/r3-debt-paydown-ledger-2026-05-02.md. No fix required.

— sent from cool-stag-230

@briansrls
briansrls merged commit 7cd1cec into main May 3, 2026
4 checks passed

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

Non-blocking — Strengths

  • ROADMAP.md Docs-only retirement is consistent with THESIS/INVARIANTS fail-closed direction and preserves the PR #820 receipt instead of deleting the historical trail.

ROADMAP — Verified

  • Go UnknownVariant debt retirement: Current emitter code returns typed EmitError::VariantParentNotFound on missing Go variant-parent info and includes regression coverage for that fail-closed path.

✅ No blocking concerns for this design/docs-only retirement update.

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