Make compare_pdfs coverage fail closed - #184
Merged
silverstein merged 5 commits intoSep 4, 2026
Merged
Conversation
….0/1.1 The shipped v0.11.0 compare_pdfs engine could report `supported` coverage and zero changes on pages and fields it never actually compared. Three truthfulness fixes, all in the engine so every consumer is safe: Bug 1 — derivePdfComparisonCoverage now inspects each compared page's Extraction IR status, not just text truncation. A page whose text layer or extraction failed drops the semantic and text channels to `unavailable`; a partial one drops them to `partial` (typed `<SIDE>_TEXT_LAYER_FAILED` / `_EXTRACTION_FAILED` / `_TEXT_LAYER_PARTIAL` / `_EXTRACTION_PARTIAL`). A scanned or image-only page is never scored as fully text-covered again. Truncation degradation is retained. Bug 2 — any `repeated_ambiguous` alignment (a repeated/template page the aligner refuses to pair, whose content is therefore never compared) now degrades semantic, text, and structure to `partial` with a typed `REPEATED_PAGE_AMBIGUITY` reason. Coverage derivation takes the alignments; buildPdfComparison computes them first. A document of identical repeated pages is no longer trivially green. Bug 3 — resolved by documented exclusion. `appearance_state` (the widget /AS) is captured on the observation but provably redundant with `value`: the pinned pdfjs 5.4.624 resolves a button widget's fieldValue from its /AS, so `value` already reflects the displayed state (measured for checkboxes and radios in the new test). Comparing it would only double-report every checkbox/radio value change, so it stays out of the compared properties with a comment explaining why. Because coverage output is wire-visible, the comparison engine/schema version is bumped 0.1.0 -> 0.2.0 and 1.0 -> 1.1, in server and share output schemas; the mcp-contract tool-contract hash is updated with a changelog note. validatePdfComparisonSemantics now asserts the new invariant: a repeated_ambiguous alignment implies its affected channels are not `supported` and carry the reason. equivalence_claim stays false; no_reported_changes stays fail-closed and now also trips when a channel is degraded. New deterministic fixtures + test/compare-pdfs-coverage.test.js prove the failed/partial-page, repeated-page, and appearance-state cases. Existing compare-pdfs assertions that accepted over-optimistic `supported` coverage (mixed-content fixtures draw a rectangle -> extraction_status partial) were updated intentionally with in-test comments. Server changes mirrored into pdf-toolkit-mcp-share/. Docs reconciled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex diff review found the documented-exclusion over-claimed: pdfjs folds /AS into fieldValue for checkboxes, but for radio groups fieldValue is the shared /V and per-widget appearanceState is not exposed, so a per-widget /AS change with unchanged group /V is NOT detected. Corrected the comment, MCP_CONTRACT wording, and test to name it as a known coverage gap rather than claim full form-appearance coverage. Detecting it needs an observation-layer change, tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
compare_pdfsdegrade semantic, text, and structure coverage when extraction failed, was partial, or page matching is ambiguousno_reported_changesfrom presenting an all-clear result when material content was not actually comparedWhy this matters
A comparison can only claim “no changes” for content it truly examined. This change turns skipped, failed, partial, and ambiguous comparisons into visible coverage limitations instead of a false green result.
Verification
Exact head:
c743659aea1f792c80c7070fcef1ce58cae54409npm run test:allunder Node 22.23.2: 2,813 Vitest passed, 109 declared skips; 62 native passed, 9 platform skipsgit diff --checkpassesScope
No release artifact, benchmark claim, or provider execution is included. The radio-button per-widget appearance-state enhancement remains separately tracked as a lower-priority follow-up.