plugin check --repo reports a file whose extraction died as a successful extraction — the repo leg drops diagnostics entirely #230

Closed
opened 2026-09-08 16:02:20 +02:00 by buildagent · 1 comment
Member

Found by an external package author while answering a question I asked them about something else. Verified here: crates/cli/src/plugin.rs contains zero occurrences of the string diagnostic. The repo leg reports walked, extracted and refused, and never the extraction diagnostics the guest put on the wire.

The reproduction

A directory with two .lgd files — one healthy, one a malformed fixture that provably emits extract.parse_error and contributes zero facts (graded exhaustively in its own .expected, so this is measured and not inferred):

probe        ran clean
walked       2 file(s)
claimed      2 of 2
extracted    2 of 2 claimed file(s) [3 symbols, 4 refs, 0 imports]
  de.h-dv.timeline/searchdef  2 claimed, 2 extracted [3 symbols, 4 refs, 0 imports]

ran clean. 2 of 2 extracted. One of those two files did not parse, produced nothing, and said so on the wire — and the probe presents it as a fully successful extraction.

Why this is worse than it looks

An empty result is a legitimate state in the reporter's format family: 46 of 272 .lgd files carry an empty <sqlstmt> because the caller supplies rows at runtime. A definition that declares nothing is normal there.

So "0 facts" and "0 facts because the parse died" are genuinely different answers, and this output renders them identically. The operator cannot tell a package working correctly on sparse input from a package failing on their input.

And it lands on the command whose entire purpose is answering that question. --repo is not "are this package's own fixtures right" — it is what would this package do to MY repository. That is the one place a silent extraction failure must not be silent.

The asymmetry that makes this a clear defect rather than a design choice

The fixture leg grades diagnostics exhaustively and reports them well — plugin check will tell you "diagnostic extract.node_limit_reached: emitted, not expected — the table is present, so it is graded exhaustively". The reporter confirmed that path works by mutation (they set a budget to 200, rebuilt, and watched it fire on all three graded fixtures).

The repo leg drops them. Same command, same guest, same wire format, opposite treatment. There is no argument in the tree for the difference; it reads as an omission rather than a decision.

The fix

A per-code census line in the same shape as the existing refused line, which already proves the reporting shape exists (plugin.rs:2096):

refused             {n} × {code} — first at {path}
diagnostics         {n} × {code} — first at {path}

Emitted at zero as a measurement, not omitted when empty — the distinction this whole issue is about is between "none reported" and "not looked at", and an absent line cannot express the first.

What a fix must prove

  • A repo containing one healthy and one diagnostic-emitting file reports the diagnostic. This is the reproduction above and it must go RED against today's code — if it passes, the test is not reading the repo leg.
  • A repo with no diagnostics reports the census at zero rather than omitting the line, so silence never has to be interpreted.
  • A file that emits a diagnostic AND facts still counts as extracted — the census is additional information, not a reclassification. extracted 2 of 2 was not wrong, it was incomplete.
  • The fixture leg's exhaustive grading is untouched; mutate it and its own tests must still go RED.

Mutations

  • Suppress the new census while a diagnostic was emitted → the reproduction test goes RED.
  • Emit the census only when non-empty → the zero-case test goes RED.
  • Make a diagnostic-emitting file count as not extracted → the third test goes RED, catching an over-correction that would make the two legs disagree in the other direction.

Provenance

The reporter went looking for the diagnostic census after I asked them an unrelated question about whether a corpus count was measured before or after a budget change. They could not find one, tested it directly, and found it absent. Their words: "I would not have looked without your question. That is twice today."

Worth recording as method: this is the second defect this week found not by a review pass but by someone checking whether a number they already believed was actually measured.

Found by an external package author while answering a question I asked them about something else. Verified here: **`crates/cli/src/plugin.rs` contains zero occurrences of the string `diagnostic`.** The repo leg reports `walked`, `extracted` and `refused`, and never the extraction diagnostics the guest put on the wire. ## The reproduction A directory with two `.lgd` files — one healthy, one a malformed fixture that provably emits `extract.parse_error` and contributes zero facts (graded exhaustively in its own `.expected`, so this is measured and not inferred): ``` probe ran clean walked 2 file(s) claimed 2 of 2 extracted 2 of 2 claimed file(s) [3 symbols, 4 refs, 0 imports] de.h-dv.timeline/searchdef 2 claimed, 2 extracted [3 symbols, 4 refs, 0 imports] ``` `ran clean`. `2 of 2 extracted`. One of those two files did not parse, produced nothing, **and said so on the wire** — and the probe presents it as a fully successful extraction. ## Why this is worse than it looks An empty result is a **legitimate state** in the reporter's format family: 46 of 272 `.lgd` files carry an empty `<sqlstmt>` because the caller supplies rows at runtime. A definition that declares nothing is normal there. So "0 facts" and "0 facts because the parse died" are genuinely different answers, and this output renders them identically. The operator cannot tell a package working correctly on sparse input from a package failing on their input. And it lands on the command whose entire purpose is answering that question. `--repo` is not "are this package's own fixtures right" — it is *what would this package do to MY repository*. That is the one place a silent extraction failure must not be silent. ## The asymmetry that makes this a clear defect rather than a design choice The **fixture leg** grades diagnostics exhaustively and reports them well — `plugin check` will tell you "diagnostic extract.node_limit_reached: emitted, not expected — the table is present, so it is graded exhaustively". The reporter confirmed that path works by mutation (they set a budget to 200, rebuilt, and watched it fire on all three graded fixtures). The **repo leg** drops them. Same command, same guest, same wire format, opposite treatment. There is no argument in the tree for the difference; it reads as an omission rather than a decision. ## The fix A per-code census line in the same shape as the existing `refused` line, which already proves the reporting shape exists (`plugin.rs:2096`): ``` refused {n} × {code} — first at {path} diagnostics {n} × {code} — first at {path} ``` Emitted at zero as a measurement, not omitted when empty — the distinction this whole issue is about is between "none reported" and "not looked at", and an absent line cannot express the first. ## What a fix must prove * A repo containing one healthy and one diagnostic-emitting file reports the diagnostic. This is the reproduction above and it must go RED against today's code — if it passes, the test is not reading the repo leg. * A repo with no diagnostics reports the census at **zero** rather than omitting the line, so silence never has to be interpreted. * A file that emits a diagnostic AND facts still counts as extracted — the census is additional information, not a reclassification. `extracted 2 of 2` was not wrong, it was incomplete. * The fixture leg's exhaustive grading is untouched; mutate it and its own tests must still go RED. ## Mutations * Suppress the new census while a diagnostic was emitted → the reproduction test goes RED. * Emit the census only when non-empty → the zero-case test goes RED. * Make a diagnostic-emitting file count as *not* extracted → the third test goes RED, catching an over-correction that would make the two legs disagree in the other direction. ## Provenance The reporter went looking for the diagnostic census after I asked them an unrelated question about whether a corpus count was measured before or after a budget change. They could not find one, tested it directly, and found it absent. Their words: *"I would not have looked without your question. That is twice today."* Worth recording as method: this is the second defect this week found not by a review pass but by someone checking whether a number they already believed was actually measured.
Author
Member

Fixed in 31cad14, on master.

Premise confirmed. probe_repo's Ok(facts) arm read symbols/refs/imports off the ValidatedFacts and discarded facts.diagnostics() — the failure arm of the guest's own report was thrown away at the boundary. crates/cli/src/plugin.rs had zero occurrences of diagnostic, so there was nothing to print even if it had been carried.

The fix

A DiagnosticCensus in which the count and the basis it was measured against are one value, so no surface can render one without the other — the same discipline ref_count/name_fallback already uses:

what where
DiagnosticTally (code + count + first witness) crates/indexer/src/conform.rs:1187
DiagnosticCensus — count and basis as one value conform.rs:1219
report_lines() — the only rendering, never empty conform.rs:1242
RepoProbe.diagnostics, unconditional conform.rs:1307
repo_diagnostics_unread conform.rs:1328
the tally in the Ok(facts) arm conform.rs:1507
conditional disclosure of refused files it could not read conform.rs:1615
CLI print crates/cli/src/plugin.rs:2117

Design notes worth keeping: the basis is extracted, not claimed — a refused file's frame never validated, so its diagnostics died with it, and repo_diagnostics_unread states that whenever the two differ. This is not a reclassification: a file with facts and a diagnostic is still extracted, and its outcome is unmoved. The existing packages::facts_diagnostics is reused so the repo leg and the indexing path share one derivation.

Mutations, every one RUN

# mutation result
M0 both source files reverted, new tests kept RED ×4 — no \diagnostics` line in:overprobe ran clean/extracted 2 of 2 claimed file(s)` — the issue's reproduction verbatim
M1 drop the tally RED ×2
M2 report_lines returns empty when per_code is empty RED ×2 (both zero-basis tests); the non-empty tests stayed green
M3 diagnostic-emitting file not counted as extracted RED ×2
M4 anti-vacuity — every extracted file reports extract.parse_error RED on the genuinely-successful test
M5 repo_diagnostics_unread made unconditional RED

M4 is the arm that matters: without it, "report everything as failed" would pass.

M0 re-run independently before merge, not taken from the report: EXIT=101, four tests red with that exact message, source restored by cp and verified identical by md5.

A separate finding this turned up

Gutting grade_diagnostics — the fixture-leg diagnostic grading that #230's whole argument rests on — survives the entire code-index-package crate (42 lib tests) and -p code-index-indexer -p code-index-cli (108 ok blocks). Exactly one assertion in the tree notices, a 21-second daemon e2e at crates/daemon/tests/xaml_package_e2e.rs:1644. Nothing in the crate that owns the function did, because every case there asserts that a correct expectation passes — and a comparison that compares nothing passes those too.

A unit-level control was added at crates/package/src/expect.rs:1785 (0.00s, no worker binary).

Gates: fmt, clippy (host and x86_64-pc-windows-gnu), cargo test --workspace (326 suites, 3504 passed, 0 failed), cargo doc -D warnings, corpus ratchet (9 pinned repos, baseline.json unmoved and never blessed), precision gate 7/7 with phantoms=0 and unshrunken POPULATION lines — all exit 0.

Fixed in `31cad14`, on `master`. **Premise confirmed.** `probe_repo`'s `Ok(facts)` arm read `symbols`/`refs`/`imports` off the `ValidatedFacts` and discarded `facts.diagnostics()` — the failure arm of the guest's own report was thrown away at the boundary. `crates/cli/src/plugin.rs` had **zero** occurrences of `diagnostic`, so there was nothing to print even if it had been carried. ### The fix A `DiagnosticCensus` in which **the count and the basis it was measured against are one value**, so no surface can render one without the other — the same discipline `ref_count`/`name_fallback` already uses: | what | where | |---|---| | `DiagnosticTally` (code + count + first witness) | `crates/indexer/src/conform.rs:1187` | | `DiagnosticCensus` — count and basis as one value | `conform.rs:1219` | | `report_lines()` — the only rendering, never empty | `conform.rs:1242` | | `RepoProbe.diagnostics`, unconditional | `conform.rs:1307` | | `repo_diagnostics_unread` | `conform.rs:1328` | | the tally in the `Ok(facts)` arm | `conform.rs:1507` | | conditional disclosure of refused files it could not read | `conform.rs:1615` | | CLI print | `crates/cli/src/plugin.rs:2117` | Design notes worth keeping: the basis is **`extracted`, not `claimed`** — a refused file's frame never validated, so its diagnostics died with it, and `repo_diagnostics_unread` states that whenever the two differ. This is **not** a reclassification: a file with facts *and* a diagnostic is still `extracted`, and its outcome is unmoved. The existing `packages::facts_diagnostics` is reused so the repo leg and the indexing path share one derivation. ### Mutations, every one RUN | # | mutation | result | |---|---|---| | M0 | both source files reverted, new tests kept | **RED ×4** — `no \`diagnostics\` line in:` over `probe ran clean` / `extracted 2 of 2 claimed file(s)` — the issue's reproduction verbatim | | M1 | drop the tally | RED ×2 | | M2 | `report_lines` returns empty when `per_code` is empty | RED ×2 (both zero-basis tests); the non-empty tests stayed green | | M3 | diagnostic-emitting file not counted as extracted | RED ×2 | | **M4** | **anti-vacuity** — every extracted file reports `extract.parse_error` | **RED** on the genuinely-successful test | | M5 | `repo_diagnostics_unread` made unconditional | RED | M4 is the arm that matters: without it, "report everything as failed" would pass. **M0 re-run independently before merge**, not taken from the report: `EXIT=101`, four tests red with that exact message, source restored by `cp` and verified identical by md5. ### A separate finding this turned up Gutting `grade_diagnostics` — the fixture-leg diagnostic grading that #230's whole argument rests on — **survives the entire `code-index-package` crate (42 lib tests) and `-p code-index-indexer -p code-index-cli` (108 `ok` blocks)**. Exactly one assertion in the tree notices, a 21-second daemon e2e at `crates/daemon/tests/xaml_package_e2e.rs:1644`. Nothing in the crate that *owns* the function did, because every case there asserts that a *correct* expectation passes — and a comparison that compares nothing passes those too. A unit-level control was added at `crates/package/src/expect.rs:1785` (0.00s, no worker binary). Gates: fmt, clippy (host **and** `x86_64-pc-windows-gnu`), `cargo test --workspace` (326 suites, 3504 passed, 0 failed), `cargo doc -D warnings`, corpus ratchet (9 pinned repos, `baseline.json` unmoved and never blessed), precision gate 7/7 with `phantoms=0` and unshrunken POPULATION lines — all exit 0.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
h-dv/code-index#230
No description provided.