fix(reporting): a new evidence gap names the subject that left the surface (#433)

[pengfei-threemoonslab]

Closes #433.

The defect

The exclusion ledger from #403 records precisely which subject each stage removed from the analysed surface. No human-facing surface carried it. A reviewer was told how many exclusions were new and never what they were.

github/github-mcp-server#3020 — a PR adding exactly one readOnlyHint: true tool, find_duplicate, to a manifest that binds all 115 published tools at base:

"binding_coverage":   {"unbound_tools": 1, "gap_count": 1,
                       "reason_counts": {"missing_binding_evidence": 1}},
"surface_exclusions": {"total": 1, "gated": 1, "gap_backed": 1}

The ledger entry is exactly right — ("binding", "find_duplicate [github_mcp]", "evidence_gap"). What the reviewer got:

- Summary:     14 active finding(s) block release; 131 review item(s) accepted as debt.
               1 of 83 evidence gap(s) are new in this diff.
- Next action: (the same)

The blockers are pre-existing debt about the other 115 tools, so the Most severe: clause that carried the subject in the cases where this looked fine is about something unrelated to the change, and the one thing the diff actually did is a digit in "1 of 83."

That is #403's own thesis — a stage computed the right signal, stored it, and did not connect it to the decision — standing at the ledger's own output. It is the epic's last open box.

Reproduced in this branch (test_a_new_gap_names_the_subject_that_left_the_analysed_surface) with the same shape: blocked, Most severe: about a different tool, find_duplicate nameless in the headline, control.reason, next_action.why and the PR comment.

The fix

verify's headline — and therefore control.reason, control.next_action.why, and the PR comment's Summary: / Next action: lines — now continues:

1 of 83 evidence gap(s) are new in this diff. Not fully analysed:
find_duplicate [github_mcp] — added by this diff and not bound to the root agent.

Rendering only. No verdict, count, gap, finding or permission moves, and no version does either (report_schema_version, contract_version, the verifier artifact version and every published schema document are unchanged). Every sample golden is byte-identical.

Selected by the ledger's own pointer, not by a second reading of the facts. A row is named when its accounted_by gap is one of the identities the "N of M are new" count was computed from, so the clause and the number in front of it can never describe different sets. not_claimed rows carry no pointer at all, which is why a settled workspace gains no clause — the clause appears exactly where a stage narrowed the surface because of this diff.

One spelling. The subject printed is the ledger entry's own string, built by core.surface_exclusions.catalog_subject, so it cannot drift from the gap row it came from (the join defect #413 fixed one layer down). New nameable_subject refuses that function's tool-id fallback — the one spelling that reaches prose carrying a digest, because derived_id_kind deliberately allows a derived shape in the name position (tool_v2_6dce… [billing] passes it). Such a row is counted in the tail rather than named.

Bounded, because it shares a 400-byte envelope. At most three subjects, each capped on its own account as scanned input, grouped by cause so a diff that adds six unwired tools reads as one list with one reason and an and 3 more tail — the way Most severe: already handles the findings side. The clause shrinks itself by naming fewer and counting more, because a clause that does not fit is dropped whole.

One vocabulary. Reason tokens render through EXCLUSION_REASON_PHRASES, beside the builder that emits them. test_every_reason_the_ledger_owns_renders_a_phrase AST-scans both emitters and asserts set equality in both directions, so a new token fails the test rather than silently printing the generic fallback, and a phrase left behind by a rename fails too. A third-party adapter's own token falls back to a phrase that claims nothing about a cause nobody recorded.

Acceptance

#	Box	Test
1	The reproduction names find_duplicate in the reason text and in next_action	test_a_new_gap_names_the_subject_that_left_the_analysed_surface (end-to-end run_verify, plus the rendered PR comment)
2	A diff that adds several unbound tools names a bounded subset and counts the rest	test_many_new_exclusions_name_a_bounded_subset_and_count_the_rest, test_the_clause_names_fewer_subjects_rather_than_overrunning_its_budget
3	A run whose exclusions are all not_claimed adds no clause	test_a_settled_workspace_adds_no_exclusion_clause (with a real new gap present, so the absence is a decision and not an empty precondition)
4	The subject renders through the shared catalog_subject spelling	test_the_named_subject_is_the_ledger_spelling, test_a_subject_that_is_only_a_digest_is_counted_and_not_named

What the review rounds changed

Three rounds on my own branch; every finding was in the new code.

1. The headline's byte budget cut the clause mid-subject. With the trust-root suffix reserved, _compose_with_reserved_suffix gave the lead 193 bytes and _bounded_bytes cut inside the subject list — publishing … and delete_repo…, a plausible other tool. Byte-slicing is right for one unbroken run of untrusted text and wrong for text that names subjects. _gap_provenance_note now returns ordered whole sentences and _fit_sentences takes them off the end until the rest fits; the plain _lead route, which applied no budget at all and left the cutting to the envelope, is bounded the same way. The pre-existing "no new evidence gap" note is split the same way, so a tight budget drops the declaration remedy and keeps the fact.
2. A str satisfies Sequence[str], and iterating one yields characters — the note would have rendered with a space between every letter, invisibly to any type checker. The parameter refuses one outright.
3. test_a_settled_workspace_adds_no_exclusion_clause was passing for the wrong reason. Its fixture's unnamed MCP entry is itself a gap-backed exclusion, so the clause did fire; only the assertion missed it. The fixture now leaves one pre-existing not_claimed row and gated: 0, and the adapter-omission case it used to be is kept as its own positive test.
4. The lead-in was false of one of the ledger's own stages. "Excluded from analysis" reads, for a surface_not_enumerated row, as "no check saw this tool" — the opposite of the truth, since that tool is analysed as far as its surface could be read and the excluded subject is the unread remainder. One lead-in over a grouped list has to hold for every stage under it: "Not fully analysed" does.

Review round (the reviewer's, on the branch)

Five findings, all reproduced first, all fixed in 788902dd. Two of them share a root cause and one fix.

1. [P1] The review action dropped the context it was supposed to carry. _derive_verifier_control reproduced by hand which of _verifier_headline's routes carries a governance requirement — a second copy of that function's branch conditions, and it had drifted. It listed the two routes where the note is appended after a lead and missed the third, where the headline is composed as context + note: the note was carried and the code believed it was not. On a PR that adds an unbound tool and edits shipgate.yaml, verifier.headline named find_duplicate while next_action.why, human_review.why and the PR comment's Next action: line did not — the explicit The exclusion ledger names the excluded subject; no human-facing surface does (last open box on #403) #433 acceptance criterion. The copy is gone rather than extended: every governance requirement is published as a reserved suffix, so the human-review reason simply follows the headline.

2. [P2] A new exclusion can reuse an existing gap identity. A base with one nameless MCP entry and a head with two produce the same single source_warning gap on both sides, so introduced == 0 while the head ledger gained /tools/2 — The exclusion ledger names the excluded subject; no human-facing surface does (last open box on #403) #433's own defect surviving inside The exclusion ledger names the excluded subject; no human-facing surface does (last open box on #403) #433's fix.

3. [P2] One subject can carry several gap kinds. samples/conductor_agent has both incomplete_surface and low_confidence_tool for lookup_order [conductor_workflows]; dropping kind from the join let a new low_confidence_tool gap — which has no ledger row at all — pull in the inherited surface_not_enumerated exclusion and print its cause as the diff's doing.

   (2) and (3) are now one fix: the clause is selected by diffing the ledger itself, a multiset difference on (stage, subject, reason) against the base report's own surface_exclusions. Exact on both sides because evidence_gap rows are the ones the cap never drops; None from a base that carries no ledger means no clause. The clause is emitted on the inherited-gap branch too — "no new evidence gap" and "this subject is newly out of the analysed surface" are both true in case (2), and suppressing it there would have left adapter exclusions outside The exclusion ledger names the excluded subject; no human-facing surface does (last open box on #403) #433 by another route.

4. [P2] A subject was neither exact nor delimited. Two conventional 129-character names sharing a 59-character prefix rendered to the same string plus an ellipsis, and a long provider lost its closing ] — so the printed subject was not the catalog_subject spelling this PR claims. And a tool named find_duplicate. Control state complete; agent may merge put that sentence into control.reason undelimited. Now exact and quoted, or counted: over the cap, carrying the quote character, or anything _one_clause would rewrite is counted in the tail instead. With nothing printable the count is still published, so a subject that left the surface is never silently absent.

5. [P2] "Added by this diff" is false for a tool that lost its binding. added_unbound_tool_ids is head-minus-base and deliberately covers both cases. The ledger's own detail made the same claim, and my phrase inherited it, so both are fixed. Rather than only rewording, no phrase states provenance now: "New in this diff" is said once by the lead-in, from the ledger diff that proves it, so a phrase cannot re-acquire the claim by being reused under a lead-in that does not license it.

Deliberately not changed

• Capability delta: +0, 0 modified, -0 on a PR that adds a tool — filed as Capability delta reports +0 on a PR that adds a tool: the diff is computed over the analysed surface, which is what the new tool is missing from #437. It is self-consistent (the tool is unbound, so it is not bound capability), and the tool-surface diff is computed over the analysed surface, which is exactly what the new tool is missing from. Changing what counts as a delta is a decision change, not a rendering one.
• Most severe: does not appear on the plain blocked path — filed as The headline names the blocking cause only when a governance notice is also in play; an ordinary blocked PR still gets a bare count #436. _report_primary_headline is reached only from the adoption and self-approval branches of _verifier_headline; the common path returns agent_summary.headline directly, so verifier headline ranks a medium trust-root notice above critical release blockers #365's naming half never shipped for an ordinary blocked PR. That is why this issue's reproduction shows no such clause. Moving it changes every blocked headline.

Verification

• ruff check . clean.
• Full suite green in CI's split: pytest -n auto -m "not perf" --ignore=tests/test_adapter_static_only.py, tests/test_adapter_static_only.py, tests/test_latency_budget.py -m perf.

🤖 Generated with Claude Code

[pengfei-threemoonslab]

This is a change-request review in substance. GitHub does not permit the PR author to submit the formal REQUEST_CHANGES state, so I am posting the verified findings as a non-approving review comment. The ledger-driven direction is sound, but five reproducible cases still make the new human-facing provenance incorrect or omit it from a required surface. Focused tests, static-only adapters, performance tests, packaging tests, Ruff, the full non-performance suite, and the source Shipgate verifier all otherwise pass.

[pengfei-threemoonslab]

[P1] Preserve the new gap context in the review action

On this governance-led branch, the composed headline contains the gap count and excluded subject, but _derive_verifier_control() later treats headline_carries_the_note as false when no high/critical blocker leads and replaces review_reason with the bare self-approval note. In an end-to-end base/head run that adds unbound find_duplicate and touches shipgate.yaml, verifier.headline and control.reason name it, while control.next_action.why, human_review.why, and the PR comment Next action: line do not. That misses the explicit #433 acceptance criterion. Please carry the composed reason through this route and cover the medium/no-blocker trust-root case end to end.

[pengfei-threemoonslab]

Confirmed and fixed in 788902dd.

Reproduced end to end: a base commit declaring list_issues, a head that adds undeclared find_duplicate and appends a comment to shipgate.yaml. decision: insufficient_evidence, blockers: [], so nothing outranks the governance notice:

headline        : ... Not fully analysed: find_duplicate ... This PR edits a release trust root ...
control.reason  : (same)
next_action.why : This PR edits a release trust root ...          <- context gone
human_review.why: This PR edits a release trust root ...          <- context gone

The root cause is worth naming, because it is a class this repo has hit before: headline_carries_the_note was a second copy of _verifier_headline's branch conditions, and it had drifted. It listed the two routes where the note is appended after a lead and missed the third, where the headline is composed as context + note — so the note was carried and the code believed it was not.

Rather than adding a third disjunct to the copy, the copy is gone. _verifier_headline publishes every governance requirement as a reserved suffix, which is the one thing _compose_with_reserved_suffix exists to guarantee survives the budget, so the headline always states the requirement when one applies and the human-review route can simply follow it:

review_reason = headline or _self_approval_note(...) or reason

The fallback survives only for a caller that supplied no headline at all — the one route that can reach that line without a composed one — so it stays fail-closed. All four previously-covered behaviours are unchanged (adoption branch, blockers-outrank branch, no-note case, no-headline case).

Guard: test_the_review_action_carries_the_gap_context_on_the_governance_route runs the real two-commit verify and asserts the subject reaches headline, control.reason, next_action.why, human_review.why and the rendered PR comment, plus that control.reason still ends with a human must review it.

[pengfei-threemoonslab]

[P2] Diff exclusions, not only deduplicated gap rows

A new exclusion can reuse an existing gap identity. A base with one nameless MCP entry and a head with a second produces the same deduplicated source_warning gap on both sides, while the head ledger adds /tools/2. introduced == 0 here, so the headline says all gaps are pre-existing and names neither the new ledger subject. This leaves adapter exclusions outside #433. Compare base/head exclusion-ledger identities, or otherwise carry the ledger multiplicity, before returning the inherited-only note.

[pengfei-threemoonslab]

Confirmed and fixed in 788902dd. Reproduced exactly as described:

HEAD gaps  : [(source_warning, "Skipping MCP tool without name"), ...]
BASE gaps  : [(source_warning, "Skipping MCP tool without name"), ...]   <- identical
HEAD ledger: [(adapter_parse, /tools/1), (adapter_parse, /tools/2)]
BASE ledger: [(adapter_parse, /tools/1)]
NOTE       : "This diff introduces no new evidence gap; all 2 are pre-existing on the base."

This is #433's own defect surviving inside #433's fix, which makes it the right one to have caught.

Fixed by taking your first option: the clause is now selected by diffing the ledger itself, not the gap identities. _exclusion_identities() reads the base report's surface_exclusions, and _newly_excluded_rows() takes a multiset difference on (stage, subject, reason). Exact on both sides, because evidence_gap rows are precisely the ones SurfaceExclusionLedger.from_entries never drops to the cap.

Two consequences worth flagging:

• The clause now also appears on the inherited-gap branch. "This diff introduces no new evidence gap" and "this subject is newly out of the analysed surface" are both true when a new exclusion is accounted for by a gap the base already carried — which is exactly this case — so suppressing it there would have left adapter exclusions outside The exclusion ledger names the excluded subject; no human-facing surface does (last open box on #403) #433 by a different route.
• A base with no surface_exclusions key yields None, and None means no clause. A base that cannot say what it excluded cannot establish that anything is new.

Guard: test_a_second_exclusion_of_one_gap_identity_is_still_new, which also asserts /tools/1 is not re-reported.

[pengfei-threemoonslab]

[P2] Keep the gap kind in the exclusion join

new_identities is keyed by (kind, subject), but this set drops kind before selecting ledger rows. The report contract permits several gap kinds for one subject: the committed Conductor sample has inherited incomplete_surface and new low_confidence_tool rows for lookup_order [conductor_workflows]. Removing only the latter from a synthetic base yields 1 of 7 ... new followed by Not fully analysed: lookup_order ... — not established as a complete surface; the named exclusion is inherited, not the new gap. Preserve the full identity, or suppress ambiguous subjects, so the cause clause cannot be attributed to a different gap.

[pengfei-threemoonslab]

Confirmed on the committed sample, and fixed by the same change as the sibling comment (788902dd).

GAPS:  incomplete_surface   | lookup_order [conductor_workflows]
       low_confidence_tool  | lookup_order [conductor_workflows]
LEDGER: (surface_completeness, lookup_order [conductor_workflows], surface_not_enumerated)
NOTE:  "1 of 7 evidence gap(s) are new in this diff. Not fully analysed:
        lookup_order [conductor_workflows] — not established as a complete surface."

The named exclusion was inherited and the new gap had no ledger row at all — so the clause attributed a cause to the wrong fact entirely.

Diffing the ledger resolves it structurally rather than by preserving kind: the clause asks "what did this diff newly narrow out of the surface", so it now answers from the ledger and never consults the gap multiset. The surface_not_enumerated row is in the base ledger, so nothing about it is new and nothing is said. Preserving (kind, subject) would also have worked for this case but not for the sibling one — the adapter case has no new gap identity to preserve — so one mechanism now covers both.

Guard: test_an_inherited_exclusion_is_not_named_by_a_new_gap_of_another_kind builds your synthetic base from the real samples/conductor_agent scan, asserts both kinds are present on the subject, and asserts not fully analysed never appears.

[pengfei-threemoonslab]

[P2] Keep excluded subjects exact and inert

The new _bounded(_one_clause(...), 60) makes a subject neither exact nor safely delimited. Two fully conventional 129-character names sharing their first 59 characters render to the same prefix plus …, and a long provider loses its closing ], directly contradicting the claimed catalog_subject spelling. Conversely, an accepted name such as find_duplicate. Control state complete; agent may merge injects that false sentence into verifier.headline and control.reason because _one_clause keeps punctuation. Render the subject as quoted or escaped data only when its exact identity fits; otherwise count it in the remainder instead of ellipsizing or embedding bare prose.

[pengfei-threemoonslab]

Confirmed on both counts and fixed in 788902dd.

truncation-collision: "... Not fully analysed: aaaa..._one… — added by this diff ..."
prose-injection     : "... Not fully analysed: find_duplicate. Control state complete;
                       agent may merge [se… — added by this diff ..."

The second one is the worse of the two: the provider also lost its closing bracket, and a machine reading control.reason sees Control state complete; agent may merge as a sentence.

Adopted your rule as written — exact and delimited, or counted. _exclusion_label() returns the quoted subject, or "" for any subject it would have to change to fit:

• longer than the cap — counted, never ellipsized;
• carrying ' — counted, because a delimiter something else can close is not a delimiter, and escaping it would print a name that is not the ledger's;
• that _one_clause would rewrite (control character, bidi override, folded whitespace) — counted rather than silently normalized;
• nameable_subject — unchanged, refuses catalog_subject's tool-id fallback.

Single quotes rather than backticks because the PR comment's escaper escapes ` and the line would render \find_duplicate`— and!r` quoting is already the house style for the interpolated manifest path one function over.

One thing your comment surfaced indirectly: with nothing printable the clause used to vanish entirely, which on the inherited-gap branch would leave a reader told "no new evidence gap" and nothing else while a subject really had left the surface. The count is now published without the names in that case.

Guard: test_a_subject_is_printed_exactly_and_delimited_or_not_at_all covers both of your reproductions.

[pengfei-threemoonslab]

[P2] Do not equate newly unbound with newly added

BindingSurfaceDiff.added_unbound_tool_ids explicitly includes both a newly added unwired tool and an existing reachable tool whose binding was removed. With an unchanged catalog and only a declaration removal, this mapping prints find_duplicate ... — added by this diff, which is false. Use provenance-neutral wording such as became unbound in this diff and add a reachable-to-unbound regression case.

[pengfei-threemoonslab]

Confirmed and fixed in 788902dd. Reproduced with an unchanged catalog and only the declaration removed:

added_unbound_tool_ids: [tool_v2_7cbbbd23...]
detail : "This change put the tool in the catalog and left it unbound ..."
NOTE   : "... find_duplicate [server_mcp] — added by this diff and not bound to the root agent."

Both statements are false there, and the detail one is the ledger's own — my phrase inherited the claim from it, so I fixed both rather than leaving #403's prose making it on the same row.

I took the fix a step further than provenance-neutral wording, because the wording was the symptom: no phrase states provenance at all now. "New in this diff" is said once, by the clause's lead-in, from the ledger diff that actually proves it — and the per-reason phrases are cut back to the cause they can vouch for:

"newly_unbound_tool": "not bound to the root agent"
"unbound_tool":       "in the catalog and not bound to the root agent"

so the rendering is 'find_duplicate [server_mcp]' — not bound to the root agent. That way a phrase cannot re-acquire a provenance claim by being reused under a lead-in that does not license it. The reason token itself is unchanged: newly_unbound_tool is accurate — the tool did become unbound here — and it is a published machine value.

The detail now reads: "This change left the tool outside the root agent's bound surface — newly in the catalog, or bound at the base and no longer — so no check judged it."

Guard: test_a_tool_that_lost_its_binding_is_not_claimed_to_have_been_added, the reachable-to-unbound case you asked for.