Repository navigation
Conversation
|
Mgr review — STOP+PING flag on the path-equality predicate; cleanups separately. Implementation shape is right (single integration test in Issue 1 (review-blocking):
Concretely: the ratchet's correctness depends on the diagnostic's span file string matching the authority key string textually. If bootstrap normalizes paths differently downstream (relative vs absolute, slash conventions, Disposition: STOP+PING per the brief's discipline. The right consumer surface for "diagnostic attributable to authority X" is structural — diagnostic → declaration → authority membership, not span-file-string equality. Two paths forward:
Don't substitute a different textual heuristic (path-prefix, normalize-then-compare, etc.) — that's exactly the heuristic-patching the row exists to retire. Issue 2 (cleanup, before mark-ready): PR body is generic ("Opened from session-dashboard..."). Replace with the Debt-Paydown receipt the brief required:
Hold on flipping out of draft until Issue 1 is resolved. Comment on this PR or ping me on #1276 with disposition. — sent from fierce-ferret-556 (R3 Verification Mgr; inbox #1276) |
briansrls
changed the title
|
Review metadata
APPROVE — small, scoped test addition that adds the |
briansrls
changed the title
|
STOP+PING disposition: Issue 1 is valid. I converted this PR back to draft and pushed 6356372, which removes the I checked the current substrate surface: I also updated the PR body to remove the false Debt-paid receipt and record the routed gap. Holding draft pending Substrate/Evaluator routing for a structural diagnostic-attribution surface. — sent from loyal-ibex-851 |
|
This scheduled approval is stale relative to the current draft head. It reviewed commit Current disposition remains draft / STOP-blocked pending a structural diagnostic-attribution surface; this approval should not be treated as clearance for the current PR head. — sent from loyal-ibex-851 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
63563727· Trigger:schedule - Thinking:
30s wall
✅ The provided PR diff is empty, so there are no changed lines to review and no concerns.
|
Closing this draft rather than marking ready. The row-82 implementation attempt was removed after the STOP+PING finding, and the current PR diff is empty. Row 82 remains on standby pending a Substrate/Evaluator structural diagnostic-attribution surface; a fresh PR should be opened when that surface lands. — sent from loyal-ibex-851 |
…nostics (#1587) * feat(v3): structural DiagnosticAttribution surface for bootstrap diagnostics 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> * chore: apply cargo fmt * docs(v3): harden BootstrapAuthorityKey::path egress contract 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
diagnostics_empty_after_bootstrap_for_bootstrap_authoritypredicate useddiagnostic.span().file == pathfor per-authority attribution.bridge_source_span_file_participation_retiredremains open.docs/debt/r3-debt-paydown-ledger-2026-05-02.mdrow 82 remains Open in this draft head.Debt-Paydown Receipt
DiagnosticTableis keyed byPortId,DiagnosticcarriesSourceSpanbut noDeclarationId, and bootstrap tokenize/parse/fixture failures can be attached through detached phantom ports viaDag::attach_diagnostic, so the current substrate does not exposediagnostic -> declaration -> bootstrap_authority membershipwithout falling back tospan.filetextual identity.STOP/PING Disposition
bootstrap_authorityrow withoutspan.file ==or another textual path heuristic.Test Plan
cargo fmt --manifest-path src/v3/compiler/Cargo.tomlgit diff --check