Skip to content

feat(v3): structural DiagnosticAttribution surface for bootstrap diagnostics - #1587

Merged
briansrls merged 3 commits into
mainfrom
session/tidy-tern-769-disp3
May 3, 2026
Merged

briansrls merged 3 commits into
mainfrom
session/tidy-tern-769-disp3

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Verification PR #1572 STOP+PING: row 82 (diagnostics_empty_after_bootstrap) couldn't ratchet structurally because consumers had to ask `diagnostic.span().file == bootstrap_authority key` — a forbidden path-string bridge. This PR adds a witness-based attribution surface so bootstrap diagnostics carry structural origin instead.

Receipt against `docs/debt/r3-debt-paydown-ledger-2026-05-02.md` row 82 ("Missing diagnostics-empty bootstrap gate"). Verification owns the row-82 closure PR after this substrate surface lands.

Surface added (in `src/v3/compiler/src/diagnostics.rs`)

  • `BootstrapAuthorityKey(&'static str)` — opaque, validated witness for a row in `src/v3/std/bootstrap_authority.dag`'s `bootstrap_authority` set. Constructor is `pub(crate)`; consumers receive minted keys through `DiagnosticAttribution` and dispatch on witness equality, never by reading `path()` (exposed for diagnostic display only).
  • `DiagnosticAttribution = Unattributed | BootstrapAuthority(key)` carried alongside each `DiagnosticTable` entry; helpers `is_bootstrap()` / `as_bootstrap_authority()`.
  • `DiagnosticTable` accessors: existing diagnostic-only signatures (`get`, `iter`, `is_empty`, `len`, `contains`) preserved; new `attribution(port)` and `iter_attributed()` expose attribution.

Dag attach API (in `src/v3/compiler/src/dag.rs`)

  • `Dag::attach_bootstrap_diagnostic(authority_key, diagnostic)` — sibling of `attach_diagnostic`. Allocates the same detached phantom port (no fabricated producer node, per dispatch constraint . #3) and records `BootstrapAuthority(key)`.
  • `mark_unresolved_with_attribution(port, diag, attribution)` — sibling carrying explicit attribution; old `mark_unresolved` still defaults to `Unattributed`.

Bootstrap loader rewires (per dispatch step 4)

  • `bootstrap.rs::patch_kernel_bool_boolean_algebra_inhabits` (3 sites) → `BootstrapAuthorityKey::new("dsl/std/types.dag")`.
  • `bootstrap.rs::report_pipeline_authority_error` → `BootstrapAuthorityKey::new(PIPELINE_AUTHORITY_FILE)`.
  • `bootstrap_regen_fresh.rs::parse_fixture` (both tokenize-failure and parse-failure branches) → `BootstrapAuthorityKey::new(file)`. Tightens `file: &str` to `&'static str` and propagates through `load_fixtures` / `load_runtime_bootstrap_authorities`'s fixture vec; the producer chain (STAGED_FILES / V3_SPECS / COMPILER_FILES / EXTDEPS_FILES) is already `'static`-keyed by `build.rs`.
  • All other `attach_diagnostic` call sites (lower / infer / int_literal_ranges / branch-condition checks) keep recording `Unattributed` per dispatch step 4 (don't widen scope).

Recommended consumer API (PR #1572 Worker B')

```rust
// Per-port:
dag.diagnostics().attribution(port).map(|a| a.is_bootstrap()).unwrap_or(false)

// Counting / filtering:
let bootstrap_count = dag
.diagnostics()
.iter_attributed()
.filter(|(_, _, a)| a.is_bootstrap())
.count();
```
No `SourceSpan.file` comparison anywhere on the consumer side.

Detached-phantom-port coverage

Every existing bootstrap attach path now goes through `attach_bootstrap_diagnostic`:

  1. Kernel-Bool patch (3 attach sites in `patch_kernel_bool_boolean_algebra_inhabits`) — exercised by `kernel_bool_path_a_diagnostic_carries_bootstrap_authority_attribution`.
  2. Pipeline-authority errors (`report_pipeline_authority_error`) — flowing through the same `attach_bootstrap_diagnostic` surface.
  3. Fixture tokenize/parse failures (`parse_fixture`) — exercised by the two `parse_fixture_*_carries_bootstrap_authority` tests.

All three call sites already used detached phantom ports (`alloc_port(None)` via `attach_diagnostic`); switching to `attach_bootstrap_diagnostic` reuses the same allocator (no fabricated producer node), so detached-phantom-port diagnostics still satisfy the `Unresolved port iff diagnostic` biconditional and now also carry `BootstrapAuthority(_)` attribution.

Focused tests (per dispatch step 5)

  • `crate::diagnostics::tests::diagnostic_attribution_default_is_unattributed_and_distinguishes_bootstrap` — `is_bootstrap` / `as_bootstrap_authority` / witness equality invariants.
  • `crate::diagnostics::tests::diagnostic_table_round_trips_bootstrap_attribution_per_port` — drives both `attach_diagnostic` and `attach_bootstrap_diagnostic` through the public Dag surface and asserts per-port attribution via `iter_attributed`.
  • `crate::bootstrap::tests::kernel_bool_path_a_diagnostic_carries_bootstrap_authority_attribution` — proves the detached-phantom-port path carries `BootstrapAuthority(BootstrapAuthorityKey::new("dsl/std/types.dag"))`, consumed via `iter_attributed` / `attribution(port)` with no `span.file` compare.
  • `crate::bootstrap_regen_fresh::tests::parse_fixture_tokenize_failure_carries_bootstrap_authority` and `parse_fixture_parse_failure_carries_bootstrap_authority` — cover the two `parse_fixture` failure branches.

Validation commands

  • `cargo test -p v3-compiler --lib --features bootstrap-regen-fresh` → 303 passed (incl. 6 new attribution tests).
  • `cargo test -p v3-compiler --lib --features bootstrap-regen-fresh -- attribution bootstrap_regen_fresh::tests bootstrap::tests::kernel_bool` → 6 / 6 of the new tests.
  • `cargo test -p v3-compiler --test integration` → 781 passed; 26 ignored; 0 failed.
  • `cargo run -p v3-compiler --features bootstrap-regen-fresh --bin regen_bootstrap` → clean (attribution is runtime-only, not embedded in the bootstrap fixture; no committed-snapshot delta).
  • `cargo clippy -p v3-compiler --all-targets -- -D warnings` → clean.
  • `cargo fmt --all --check` → clean.

Out of scope (per dispatch constraints)

  • No `span.file ==`, path-prefix, or normalize-then-compare predicates anywhere on the consumer API.
  • No `DeclarationId` forced onto pre-declaration diagnostics; tokenize/parse failures stay covered by the detached-phantom-port path.
  • No fabricated producer nodes for detached diagnostics (`alloc_port(None)` reused).
  • No parallel map keyed by diagnostic message/span text.
  • Verification owns the row-82 final ratchet PR after this lands.

🤖 Generated with Claude Code

briansrls and others added 2 commits May 3, 2026 19:53
…nostics

Verification PR #1572 STOP+PING flagged that row 82
`diagnostics_empty_after_bootstrap` cannot ratchet structurally today:
`DiagnosticTable` was keyed by `PortId` but carried only a `Diagnostic`,
`Dag::attach_diagnostic` allocates detached phantom ports with
`produced_by: None`, and any consumer asking "is this from a bootstrap
authority file?" had to compare `diagnostic.span().file` against a
known authority path — a forbidden path-string bridge.

This change adds a witness-based attribution surface so verification
consumers (PR #1572 Worker B') dispatch on bootstrap origin via
`DiagnosticAttribution`, not span paths.

Substrate addition (`src/v3/compiler/src/diagnostics.rs`):

  - `BootstrapAuthorityKey(&'static str)` — opaque, validated witness
    for a row in `src/v3/std/bootstrap_authority.dag`'s
    `bootstrap_authority` set. Constructor is `pub(crate)` so only
    bootstrap loaders / kernel-patch helpers can mint a key; consumers
    receive minted keys through `DiagnosticAttribution` and dispatch on
    witness equality. `path()` is exposed for diagnostic display only;
    consumers must NOT use it to recover attribution (that's the bridge
    being dissolved).

  - `DiagnosticAttribution = Unattributed | BootstrapAuthority(key)`
    rides alongside each entry in `DiagnosticTable`. Adds
    `is_bootstrap()` and `as_bootstrap_authority()` helpers.

  - `DiagnosticTable.entries` now stores
    `(Diagnostic, DiagnosticAttribution)`. Existing `get` / `iter` /
    `is_empty` / `len` / `contains` accessors preserve the diagnostic-
    only signatures; new `attribution(port)` and `iter_attributed()`
    expose attribution. `insert(port, diag, attribution)` is the only
    breaking signature change (still `pub(crate)`).

Dag attach API (`src/v3/compiler/src/dag.rs`):

  - `mark_unresolved_with_attribution(port, diag, attribution)` —
    new sibling carrying an explicit attribution; old
    `mark_unresolved` defers with `Unattributed`.

  - `attach_bootstrap_diagnostic(authority_key, diagnostic)` — new
    sibling of `attach_diagnostic` for diagnostics raised while
    loading or patching a substrate `bootstrap_authority` row.
    Allocates the same detached phantom port (no fabricated producer
    node, per dispatch constraint #3) but records
    `BootstrapAuthority(key)` so detached-phantom-port bootstrap
    diagnostics carry origin without a `SourceSpan.file` compare.

Bootstrap loader rewires (per dispatch step 4):

  - `bootstrap.rs::patch_kernel_bool_boolean_algebra_inhabits` — three
    diagnostic-attach sites now call `attach_bootstrap_diagnostic`
    with `BootstrapAuthorityKey::new("dsl/std/types.dag")`.

  - `bootstrap.rs::report_pipeline_authority_error` — uses
    `BootstrapAuthorityKey::new(PIPELINE_AUTHORITY_FILE)` (the
    `src/v3/compiler/pipeline.dag` authority).

  - `bootstrap_regen_fresh.rs::parse_fixture` — both the tokenize-
    failure and parse-failure branches now call
    `attach_bootstrap_diagnostic(BootstrapAuthorityKey::new(file), …)`.
    `parse_fixture`'s `file: &str` parameter is tightened to
    `&'static str`; `load_fixtures` and the corresponding
    `Vec<(&'static str, &'static str)>` collection in
    `load_runtime_bootstrap_authorities` carry the `'static` through
    the producer chain (STAGED_FILES / V3_SPECS / COMPILER_FILES /
    EXTDEPS_FILES are already `'static`-keyed by `build.rs`).

Ordinary non-bootstrap diagnostic call sites (lower / infer /
int_literal_ranges / branch-condition checks in `dag.rs`) keep using
`attach_diagnostic`, so they continue to record `Unattributed` per
dispatch step 4 (don't widen scope).

Focused tests (per dispatch step 5):

  - `crate::diagnostics::tests::diagnostic_attribution_default_is_…`
    (witness equality / `is_bootstrap` / `as_bootstrap_authority`
    invariants).
  - `crate::diagnostics::tests::diagnostic_table_round_trips_…` —
    drives both `attach_diagnostic` and `attach_bootstrap_diagnostic`
    through the public Dag surface and asserts per-port attribution
    via `iter_attributed`.
  - `crate::bootstrap::tests::kernel_bool_path_a_diagnostic_carries_…`
    — proves the detached-phantom-port path attaches
    `BootstrapAuthority(BootstrapAuthorityKey::new("dsl/std/types.dag"))`,
    consumed through `iter_attributed` / `attribution(port)` without
    any `span.file ==` compare.
  - `crate::bootstrap_regen_fresh::tests::parse_fixture_tokenize_…`
    and `parse_fixture_parse_failure_carries_bootstrap_authority` —
    cover the two `parse_fixture` failure branches.

Validation:
  - `cargo test -p v3-compiler --lib --features bootstrap-regen-fresh`
    303 passed (incl. 6 new attribution tests).
  - `cargo test -p v3-compiler --test integration` 781 passed.
  - `cargo run -p v3-compiler --features bootstrap-regen-fresh
    --bin regen_bootstrap` clean (no snapshot delta — attribution is
    runtime-only, not embedded in the bootstrap fixture).

Receipt against `docs/debt/r3-debt-paydown-ledger-2026-05-02.md` row 82
("Missing diagnostics-empty bootstrap gate"). Verification owns the
final row-82 ratchet PR after this substrate surface lands; the
recommended consumer API is
`DiagnosticTable::attribution(port).is_bootstrap()` (or filtering via
`iter_attributed`).

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

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 48827c01 · Trigger: schedule
  • Comparison: origin/main @ 9b5b8a7e ... review/pr-1587-48827c01 @ 48827c01
  • Thinking: 27s wall

Verdict: APPROVE — clean structural refactor that replaces a SourceSpan.file string-compare bridge with a witness type (BootstrapAuthorityKey + DiagnosticAttribution). Aligns with modeling discipline (illegal-states/coprod), preserves the unresolved-port biconditional via mark_unresolved_with_attribution, and the new tests exercise the three real attach paths (kernel-Bool patch, tokenize-fail, parse-fail) plus a direct table round-trip.

Exploratory observations (non-blocking):

  • BootstrapAuthorityKey::new (diagnostics.rs:706) accepts any &'static str — the witness contract is "mint sites only call this with rows from the build.rs authority arrays," enforced socially via pub(crate) + doc. A future tightening could route minting through a single &'static [&'static str] lookup so the witness is structurally validated, not just trusted by convention. Not required for this PR; the doc on the type names the constraint clearly.
  • path() is pub while new is pub(crate). That's the right asymmetry for "display-only egress, controlled mint," but it does leave the door open for an external consumer to round-trip key.path() through some other constructor in the future. Worth a comment on the pub fn path if you want to harden the contract beyond the type-level doc.

Per #1587 reviewer's optional follow-up. The asymmetry (pub `path()` /
`pub(crate)` `new`) is intentional but only documented at the type
level; pinning the egress-only rule on the accessor itself makes the
"no `pub fn from_path` / `From<&str>` re-introducing the path-string
bridge" review check explicit. Comment-only; no behavior change.

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

@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: 48827c01 · Trigger: schedule
  • Thinking: 338s wall

BLOCKING (3)

Root Cause

  • src/v3/compiler/src/diagnostics.rs bootstrap diagnostic attribution was added before the bootstrap-path authority and generated-snapshot boundary were made structural consumers → ground key minting in a real authority carrier, add the enum dissolution receipt, and serialize attribution via iter_attributed.

⚠️ The direction is right, but these substrate-level gaps should be closed before this lands.

})
}

/// Opaque, validated witness to a path in the substrate

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: BootstrapAuthorityKey is documented as a validated substrate witness, but the named src/v3/std/bootstrap_authority.dag authority does not exist and new accepts any static path, so Modeling Faithfulness/P2 API-level enforcement rests on convention.

/// bootstrap-authority row. Verification surfaces should prefer
/// this over `span.file == "src/v3/std/..."` predicates.
pub fn is_bootstrap(&self) -> bool {
matches!(self, Self::BootstrapAuthority(_))

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: Because DiagnosticAttribution is stored in Dag.diagnostics and consumed downstream, it is a substrate coproduct and needs the required 🟢/🟡/🔴 dissolution classification comment.

&self,
) -> impl Iterator<Item = (PortId, &Diagnostic, &DiagnosticAttribution)> {
self.entries.iter().map(|(p, (d, a))| (*p, d, a))
}

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: Adding attribution to DiagnosticTable creates a new cross-stage fact, but bootstrap snapshot emission still walks iter() and emits 2-arg inserts, so attribution does not flow through regen output (P2 facts-flow-forward).

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: claude / claude-opus-4-7
  • Commit: 9a0e341f · Trigger: schedule
  • Comparison: origin/main @ 9b5b8a7e ... review/pr-1587-9a0e341f @ 9a0e341f
  • Thinking: 41s wall

Verdict: APPROVE — clean, narrowly scoped substrate change.

This diff replaces a span.file == "src/v3/std/..." string-bridge with a typed witness (BootstrapAuthorityKey + DiagnosticAttribution), exactly the kind of move modeling-discipline endorses (facts-flow-forward, illegal-states-unrepresentable on the bootstrap-vs-user dispatch axis). Notable strengths grounded in the diff:

  • BootstrapAuthorityKey::new is pub(crate) (diagnostics.rs:686) and held as &'static str, mintable only by bootstrap loaders that already iterate &'static-keyed fixture arrays — single authority, no public reconstruction path.
  • bootstrap_regen_fresh.rs:108,158,195 tighten fixture lifetimes to &'static, which is what backs the witness contract — coherent change, not incidental churn.
  • Dag::attach_bootstrap_diagnostic (dag.rs:3520) and mark_unresolved_with_attribution (dag.rs:3481) preserve the existing port.state == Unresolved iff diagnostics.contains(port) biconditional; attribution rides alongside, doesn't fork it.
  • DiagnosticTable::insert stays pub(crate) (diagnostics.rs:805); the table change is internal-only.
  • Tests are behavior-driven and cover both bootstrap attach paths plus a per-port round-trip — no full-pipeline shells.

Exploratory observations (non-blocking):

  • BootstrapAuthorityKey::path() (diagnostics.rs:709) is documented as a one-way egress door policed by reviewers. The doc explicitly flags From<&str>/from_path as the bridge to reject — fine, but the type system can't enforce it. If this becomes a recurring concern, a future move could be to make path() return a newtype BootstrapAuthorityPath(&'static str) whose Display impl is the only public read, severing the &'static str → BootstrapAuthorityKey round-trip even more thoroughly. Not for this PR.
  • DiagnosticAttribution::Unattributed is a reasonable transitional default, but it's now a coproduct sitting on every diagnostic. Worth tracking whether the long-term shape has more variants (user-source, synthesized, etc.) or whether this stays a 2-arm enum — if the latter, a Option<BootstrapAuthorityKey> would be lighter; if the former, the enum is right. No action needed now.

@briansrls
briansrls merged commit bbc1618 into main May 3, 2026
4 checks passed
briansrls added a commit that referenced this pull request May 3, 2026
… attribution

Addresses three blocking findings on PR #1587:

1. **Witness validation moves from social to structural.**
   `BootstrapAuthorityKey::new` previously accepted any `&'static str`
   and relied on doc-only convention to keep mint sites honest. Adds
   `BOOTSTRAP_AUTHORITY_PATHS`, an always-on path-only mirror emitted
   by `build.rs` (siblings of the existing `STAGED_FILES`/`V3_SPECS`/
   `COMPILER_FILES`/`EXTDEPS_FILES` payload arrays — same on-disk
   enumeration that drives the `bootstrap_authority` substrate set).
   Also adds `crate::bootstrap::DSL_STD_BOOTSTRAP_PATHS` (hand-listed
   to mirror the 14 `dsl/std/*.dag` rows in `bootstrap_authority.dag`,
   since `dsl/std/` holds many non-authority files and full-directory
   enumeration would over-include). `is_bootstrap_authority_path`
   checks both; `BootstrapAuthorityKey::new` now panics fail-closed
   on unknown paths so loader-side typos cannot silently mint a bogus
   witness.

2. **`DiagnosticAttribution` carries dissolution receipt.** Per
   modeling-discipline review of substrate coproducts, marks the
   `Unattributed | BootstrapAuthority(_)` enum 🟡 TRANSITIONAL with
   the explicit dissolution trigger: when PB-Bootstrap-Process makes
   the loader data-native and the substrate `bootstrap_authority`
   set is consumed directly, every diagnostic attached during
   bootstrap inherits its authority reference structurally and the
   binary attribution split collapses.

3. **Bootstrap snapshot emission forwards attribution (P2 facts-flow-
   forward).** `regen_bootstrap_emit::render_diagnostics` now walks
   `iter_attributed()` and emits 3-arg `table.insert(port, diag,
   attribution)` calls; `render_diagnostic_attribution` renders
   `DiagnosticAttribution::Unattributed` and
   `DiagnosticAttribution::BootstrapAuthority(BootstrapAuthorityKey
   ::new(<path>))`. The static `bootstrap_*_generated.rs` snapshots
   are still diagnostic-empty in production (the empty-table fast
   path is unchanged), but the emission contract no longer silently
   downgrades regen-time diagnostics to `Unattributed`.

New tests:
  - `crate::diagnostics::tests::bootstrap_authority_key_new_panics_on_non_authority_path`
    (`#[should_panic]`).
  - `crate::diagnostics::tests::bootstrap_authority_key_validator_accepts_known_authority_rows`
    — sanity-mints one row from each of the four mirror sets so a
    regression on any single set surfaces.

Validation:
  - `cargo test -p v3-compiler --lib --features bootstrap-regen-fresh`
    305 passed (incl. 8 attribution tests).
  - `cargo test -p v3-compiler --test integration` 781 passed.
  - `cargo run -p v3-compiler --features bootstrap-regen-fresh
    --bin regen_bootstrap` clean (no snapshot delta — production
    snapshot remains diagnostic-empty).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant