Make "does validate-references check anything" a test, not a note (#466) - #475
Merged
Conversation
#466 reported the reference validator as performing zero checks, reading Total checks: 0 All validations passed! as a vacuous pass. It is not. `Total checks` is printed as `len(all_results)` in the upstream CLI, and `all_results` holds validation *issues* — so 0 means no problems were found. Planting a snippet that appears in no publication into taxonomy[0].evidence[0] produces: [ERROR] Text part not found as substring: 'ZZQQ ...' Location: taxonomy[0].evidence[0].snippet Total checks: 1 / Issues found: 1 The justfile already said this, from #257, in four lines directly above the recipe. Prose was not enough, so the claim is now a test that fails if the validator ever does accept fabricated text, plus a second test asserting a real curated snippet is still accepted - a checker that rejects everything would be just as useless and passes the first test alone. Diagnosing it turned up something the issue did not: **SupportingReference is never checked at all.** It is the range of `Discussion.evidence`, and unlike EvidenceItem its `snippet` and `reference` carry no `implements:`, so the plugin's field detection cannot see them. 11 snippets across 8 records are unvalidated, and the module's own description claims the opposite — that it "carries a verbatim snippet so the same anti-hallucination snippet-vs-cached-abstract check the Mechs already run can validate it". Not fixed here on purpose: that lives in mech_shared.yaml, which is vendored byte-identical and sha-pinned across the Mech repos by the vendored-sync job, so editing this copy alone would fail CI and violate the module's own instruction. Filed instead; the gap is pinned by a test that fails when it is closed, so nobody closes it silently. Also confirms what remains true about the truncation class: a snippet cut mid-word still passes, because a cut quote is still a substring and substring matching is what the tool promises. That is #295/#465, covered by test_snippets_are_not_truncated.py, and it is why those eight survived - not vacuity. No behavioural change; tests and comments only.
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
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.
Addresses #466 by correcting its premise and closing the gap that diagnosing it actually revealed.
#466's core claim is wrong
The issue read this as a vacuous pass:
Total checksis printed aslen(all_results)in the upstream CLI (linkml_reference_validator/cli/validate.py:313), andall_resultsholds validation issues. SoTotal checks: 0means no problems found. A clean record prints 0 and always will.Demonstrated by planting a snippet that appears in no publication into
taxonomy[0].evidence[0]:EvidenceItemsnippets are genuinely validated againstreferences_cache/.Worth noting for process: the justfile already said this, from #257, in four lines immediately above the recipe — "It IS validating (a fabricated snippet fails it); only the label is wrong." #466 was filed anyway. That is the argument for the change below: prose next to the code was not enough.
A detail that cost me an hour, recorded so it doesn't cost the next person one
My first plant did appear to confirm #466 — gibberish went in, validator passed. The snippet had landed in
discussions[0].evidence[2], which is aSupportingReference, not anEvidenceItem. Two different findings wearing the same output.What this changes
tests/test_reference_validator_actually_validates.py— five tests, no subprocess and no network, reading the samereferences_cache/the recipe does:validate-referencesrun is meaningless.implements:annotations that make the check reachable at all.SupportingReferencegap, pinned (below).Plus a justfile note pointing the #257 prose at the now-mechanical proof.
The real finding:
SupportingReferenceis never checkedDiscussion.evidenceranges onSupportingReference. UnlikeEvidenceItem, itssnippetandreferencecarry noimplements:, so the plugin's field detection cannot see them:_find_excerpt_fields_find_reference_fieldsEvidenceItem['snippet']['reference']SupportingReference[][]11 snippets across 8 records are unvalidated. The module's own description claims the opposite — that it "carries a verbatim
snippetso the same anti-hallucination snippet-vs-cached-abstract check the Mechs already run can validate it."Deliberately not fixed here. It lives in
mech_shared.yaml, which is vendored byte-identical and sha-pinned across the Mech repos (scripts/check_vendored_sync.sh:33), and the file says "Do not edit one copy in isolation — change it once and re-vendor + re-pin everywhere." Editing this copy alone fails thevendored-syncgate. Filed as a follow-up.The gap is pinned by a test that fails when it is closed, so it cannot be closed silently — whoever fixes it gets a red test pointing at the docstring explaining what to update.
What stays true about the truncation class
A snippet cut mid-word still passes, because a cut quote is a substring and substring matching is what the tool promises. That is why the eight in #295/#465 survived — not vacuity.
tests/test_snippets_are_not_truncated.pycovers that class; this PR does not change it.No behavioural change: tests and comments only.
ruff,blackclean.🤖 Generated with Claude Code