Skip to content

CAP-1b: make an excluded source file visible to packet sufficiency - #1830

Merged
TheGreenCedar merged 10 commits into
dev/codestory-nextfrom
codex/1802-coverage-evidence
Aug 3, 2026
Merged

CAP-1b: make an excluded source file visible to packet sufficiency#1830
TheGreenCedar merged 10 commits into
dev/codestory-nextfrom
codex/1802-coverage-evidence

Conversation

@TheGreenCedar

Copy link
Copy Markdown
Owner

Closes #1829. #1802 stays open — the polish items (report field, citation
loss_reason, readiness, MCP schema) remain, none of which change a verdict.

A packet could rest a proof-bearing claim on a file the index had deliberately
refused and still report Sufficient. agent/ contained zero references to
coverage_reason or exclusions.

The route that was open is not the one the issue named

An explicitly probed excluded path already caps: exact_path_probe_citation
mints eligible_for_sufficiency: Some(false), so it reaches
missing_exact_path_claims and the packet is already Partial.

The uncapped route is a required file-scoped citation, minted
Some(!explicit) at orchestrator.rs:3084, read straight off disk and never
probed. This is therefore driven by cited paths, not probe paths — the
probe-side fix, which is where I would have started, leaves it untouched.

Shaped after EV-78, with one deliberate inversion

Two cause enums joined by one total mapping in contracts — a producer cause for
what the lookup did, a consumer cause for what a reader may conclude — so packet
and readiness cannot drift, exactly as FreshnessUnknownCauseDto does.

The inversion is the whole asymmetry. Freshness treats a missing
observation as unknown-and-capping, because a check that did not run proves
nothing. Coverage must not: an empty observation list means no path was checked,
which is legitimate and must cap nothing. A failed lookup arrives as a per-path
NotEstablished instead of as an absence.

Copying freshness wholesale would make every packet that cites nothing
Partial. A reviewer pattern-matching on the precedent will ask for the
fresh_index_observation()-style opt-in helper and be wrong;
no_observations_cap_nothing pins it.

Two ways this could have silently done nothing

  • Path matching. Exclusion rows store workspace-relative paths; citations
    carry absolute ones. A string compare — or normalize_path_key — matches on
    some platforms and never on others: plumbing complete, every test green, and
    nothing ever capping. It goes through same_workspace_path.
  • Filtering instead of mapping. The observer returns exactly one observation
    per requested path, including on lookup failure. A producer that dropped
    unknown paths would be indistinguishable from one reporting them covered —
    the defect moved one layer up.

The follow-up filter is not optional

Capping flips terminally_sufficient false, which is precisely what opens
follow-up generation. The cap alone turns a packet that answered and stopped
into one that re-probes a permanently unindexable file every round. Leads arrive
from three sources, so they are filtered once after all three contribute — my
first attempt filtered only the first source and the test caught it.

Verification

Ten tests. The load-bearing ones:

  • a_packet_resting_on_an_excluded_file_cannot_be_sufficient — asserts the
    control is Sufficient first, so it cannot pass vacuously. Removing the cap
    clause reproduces Sufficient over the excluded file while the gap text
    already says the index never read it.
  • no_observations_cap_nothing — the EV-78 asymmetry.
  • every_status_reaches_a_definite_verdict — fails if a status variant is
    added and absorbed into a permissive arm.
  • an_unnamed_defect_is_still_unprovable — an Incomplete with no reason stays
    typed rather than defaulting to covered.
  • an_excluded_file_is_never_offered_as_a_follow_up_lead.

cargo test --workspace, cargo clippy --workspace --all-targets,
cargo fmt --check, and the generalization lint are clean.

🤖 Generated with Claude Code

A packet could rest a proof-bearing claim on a file the index had deliberately
refused and still report `Sufficient`. Nothing under `agent/` referenced a
coverage reason or an exclusion, so the fact never reached the verdict.

The route that was actually open is not the one #1802 named. An explicitly
probed excluded path already caps: `exact_path_probe_citation` mints it
`eligible_for_sufficiency: Some(false)`, so it lands in
`missing_exact_path_claims` and the packet is already `Partial`. The uncapped
route is a *required* file-scoped citation — `orchestrator.rs` mints those
`Some(!explicit)` — read straight off disk and never probed. So this is driven
by cited paths, not probe paths; a probe-side fix leaves the hole untouched.

Shaped after EV-78, including its two-enum split: a producer cause for what the
lookup did, a consumer cause for what a reader may conclude, joined by one
total mapping in contracts so packet and readiness cannot drift.

With one deliberate inversion, which is the whole asymmetry. Freshness treats a
*missing* observation as unknown-and-capping, because a check that did not run
proves nothing. Coverage must not: an empty observation list means no path was
checked, which is legitimate and caps nothing. A failed lookup arrives as a
per-path `NotEstablished` instead of as an absence. Copying freshness wholesale
would make every packet that cites nothing `Partial`; a test pins it, because
that simplification looks right.

Two things that would have made this silently do nothing:

* **Path matching.** Exclusion rows store workspace-relative paths, citations
  carry absolute ones. A string compare, or `normalize_path_key`, matches on
  some platforms and never on others — plumbing complete, tests green, nothing
  ever capping. It goes through `same_workspace_path`.
* **Filtering instead of mapping.** The observer returns exactly one
  observation per requested path, including on lookup failure. A producer that
  dropped unknown paths would be indistinguishable from one reporting them
  covered — the defect moved one layer up.

The follow-up filter is not optional. Capping flips `terminally_sufficient`
false, which is exactly what opens follow-up generation, so the cap alone turns
a packet that answered and stopped into one that re-probes a permanently
unindexable file every round. Leads arrive from three sources, so they are
filtered once after all three have contributed.

Mutation-checked: removing the cap clause makes the packet report `Sufficient`
over the excluded file while its own gap text says the index never read it.

Refs #1802
… causes

My own tests set `source_coverage` directly, so nothing exercised the lookup
itself — which is exactly where this change could most easily do nothing at
all. Exclusion rows store a workspace-relative `normalized_path` while a
citation carries whatever the retrieval layer produced, so a string comparison
matches on some platforms and silently never matches on others: plumbing
complete, every test green, and no packet ever capped.

`coverage_observation_matches_an_exclusion_by_path_identity` indexes a real
project with an oversized structural source and asks about the same file three
ways — workspace-relative, absolute, and `./`-prefixed. Mutation-checked:
replacing `same_workspace_path` with an `OsStr` comparison makes
`./docs/api.json` report `Indexed`, an excluded file looking covered. It also
asserts a file the index *did* cover reports `Indexed`, since a matcher that
over-matched would cap every packet in the repository.

Separately, "there is no published core to ask" and "the query failed" were
both reported as `LookupUnavailable`. They are not the same judgement, and
EV-78's split is the reason to keep them apart: the first is a deliberate,
recoverable state, the second establishes nothing about anything.
`project_unavailable` now maps to `PublicationIncomplete`, which is the variant
that existed for it.

Refs #1802
…e files

Adversarial review found the half of this contract that can actually occur was
shipped without a producer. Both refresh gates refuse to publish on any
coverage reason except `ParserPartial` — they filter exactly that one out — so
`ParserPartial` is the only recorded defect a *served* packet can rest on, and
the producer reported such a file `Indexed`. That contradicts this contract's
own wording: `Indexed` is defined as "no recorded coverage defect" and
`Incomplete` as "the file was indexed but a coverage reason was recorded
against it". The `reason` field existed to carry it and was always `None`.

The reviewer reproduced it end to end; the fix consults
`stored_file_coverage_diagnostics` alongside the exclusion rows.
Mutation-checked: stubbing the diagnostics lookup makes a parser-partial file
report `Indexed` again.

Also removes `tmp_cap_combo_probe.rs` and `tmp_stat_cost_probe.rs`, which were
scratch files written into this working tree by concurrent review agents and
swept into the previous commit by `git add -A`. They were never mine and are
not on `dev`.

That is also the explanation for two test failures I could not reproduce
earlier: `cargo test --workspace -- --list` shows no test named
`probe_live_context_tool_structured_content_vs_published_output_schema`
anywhere, because it was one of those agents' temporary probes, running in my
tree while my own full-suite run was in flight.

Refs #1802
`zz_tmp_structural_cap_probe.rs` is scratch, self-labelled "TEMPORARY probe
(delete after)". It was written into this working tree by an adversarial-review
agent verifying a claim about stale structural exclusion rows, and `git add -A`
carried it into b22a4bb — which merged to `dev` in #1821.

It is dead weight in CI and asserts nothing anyone owns. The behaviour it was
probing is covered properly by
`the_structural_bound_holds_when_the_parser_headroom_is_raised` in
`codestory-workspace` and by the store-side publication tests.

The general lesson, since this is the second and third instance in one session:
`git add -A` is unsafe while other agents can write to the tree. Stage named
paths.
`probe_answer_json`, `probe_context_packet_emits_fields_the_schema_forbids`,
`tmp_experiment_sufficiency`, `tmp_layout_leaks` and `tmp_experiment_layouts`
were written into existing files by adversarial-review agents and carried in by
`git add -A`. One is literally commented "TEMPORARY INVESTIGATION TEST - remove
before commit."

The earlier sweep only caught whole scratch *files*; these live inside files I
was legitimately editing, so a file-level check could not see them. Diffing the
branch against dev for added `probe_`/`tmp_` functions is what found them.
`CONTEXT_PACKET_SCHEMA` is `additionalProperties: false`, and
`context_packet_json` serializes the whole `AgentAnswerDto` — so every optional
field on that DTO becomes a violation of the tool's own published output schema
the moment it is populated. Nothing compared the two, so it had already
happened once: `freshness` has been emitted undeclared since EV-78.
`source_coverage` made it twice.

Both are declared now, and `the_context_packet_emits_only_fields_its_schema_declares`
compares emitted keys against the declared set. It populates the optional
fields on purpose — they are `skip_serializing_if`, so a default fixture would
pass while a real packet failed, which is exactly how this stayed invisible.

Mutation-checked: removing the `source_coverage` declaration fails with
"the packet emits [\"source_coverage\"], which its published output schema
forbids".

Found by adversarial review. Fixing `freshness` here is out of this change's
nominal scope, but it is the same one-line defect in the same list, and leaving
a known-false published contract in place to keep a diff tidy is the wrong
trade.

Refs #1802
Two more from adversarial review, both real on an ordinary repository.

**The cap keyed on every citation.** `covered_paths` took every
`citation.file_path` with no eligibility filter, unlike every other sufficiency
consumer. The filesystem appenders mint citations with
`eligible_for_sufficiency: false` and a real path — and `.sql` is a structural
format, so its effective cap is 1 MiB while `collect_sql_schema_file_candidates`
cites any `.sql` up to 1.5 MB. A 1.2 MB `db/structure.sql` is therefore both
policy-excluded and cited, and would have capped packets over evidence they
were never allowed to rest on. It now filters through
`citation_sufficiency_eligible`. Route B, the hole this closes, is eligible by
construction, so nothing intended is lost.

**The gap sentence asserted a byte overrun that had not happened.** Two disjoint
record kinds qualify as exclusions: byte-bound (`observed_size > byte_cap`) and
structural-unit-bound (`observed_size <= byte_cap`, unit count over its own
cap). The producer populated the sizes for both, so a 300 KB JSON with 3,000
structural units rendered "307200 bytes exceeds the 1048576 byte cap, so the
index never read it" — false twice over, since 307200 does not exceed 1048576
and the index did read it. The sizes are now carried only for a genuinely
byte-bound row; the rest get the plain sentence, which is true.

A number that is wrong is worse than no number, particularly in a gap sentence
whose whole job is to tell an operator what to change.

Refs #1802
… lead filter

CI caught the contract-surface consequence of declaring the two packet fields:
`plugins/codestory/generated-mcp-catalog.json` is generated from
`CONTEXT_PACKET_SCHEMA` and went stale. Regenerated.

Three more from adversarial review:

**The lead filter missed repo-cache leads.** `open_next_paths` entries are
`packet_display_path` output, which strips a named repository root —
`target/repo-cache/repos/axios/lib/core/Axios.js` becomes
`lib/core/Axios.js`. Joining the project root back onto that yields a path
that does not exist, so the identity comparison said "different file" and the
lead survived. The filter was therefore inert for exactly the packets it
matters most for. It now compares display form to display form, keeping the
identity comparison as the fallback for leads that were never stripped.

**Dedup compared strings while everything else compared identity.**
`exact_packet_probe_paths` yields a project-relative spelling and a citation
carries an absolute one, so both survived and the packet shipped the same gap
twice for one file. Deduped by resolved identity now. The one branch that runs
before the project root is known keeps a raw-string dedup, because there is
nothing to resolve against there.

**Restores the `PublicationIncomplete` cause split**, which 9b59b43's message
claimed but which is in no commit — a concurrent review agent writing to this
tree appears to have reverted the edit between my check and my commit. "No
published core to ask" and "the query failed" are not the same judgement.

The path-identity test grew two cases the dedup change demands: two spellings
of one file must produce one observation, and two distinct files must still
produce two. The second is the guard that the dedup did not quietly turn the
producer into a filter.

Refs #1802
The generalization lint rejected the comment I wrote explaining the display-path
filter: I illustrated it with a real path from the evaluation corpus. The lint
exists to stop eval-corpus literals reaching product code, and a comment is
still product text — a later reader tuning behaviour to the named repository is
exactly the over-fitting it guards against.

Same explanation, no corpus path. Caught by CI, not by me: I had run the lint
before this comment existed and not after.
@TheGreenCedar
TheGreenCedar merged commit 8e2e55e into dev/codestory-next Aug 3, 2026
8 checks passed
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