Fail on media the site carries but links from nowhere - #43
Conversation
The URL contract checked media in one direction only. The legacy list proves an inbound link still lands, and the asset check proves a reference names a real file. Both read outward from a reference, so neither can ask whether anything points at a given file. An image the WordPress conversion dropped from a page therefore stays on disk, stays reachable at its own URL, and reports green in both directions while appearing nowhere on the site. It was not hypothetical. 120 of the 1,048 carried media files are linked from no page, and five gallery shortcodes across three posts are empty. The ESP32 post announces "Below are pictures of the finished case and the utility meters:" and renders an empty div, with a sequential run of camera originals from that month sitting unreferenced. Every gate passed throughout. check_orphans reads inward from the file and is the only check that sees this. collect_refs and ref_to_path are factored out of check_assets, since the two checks are the same reference set read in opposite directions and walking the built site twice to build it would be waste. ORPHANED_MEDIA is an exact count rather than a bound. A ceiling would let restored media leave slack for a later regression to hide in, so the check fails when the count falls as well as when it rises, and names the new number to write when it falls. The rising message and the falling one differ, because "a page stopped linking media" and "media was restored" are opposite events that a single count cannot distinguish on its own. The explanation is printed rather than returned in the failure list, so the caller's "N missing" stays the orphan count. Carrying a diagnostic in that list reported 122 for 121 orphans. Demonstrated failing in both directions before being trusted, per the rule that a gate is only trusted after it has been watched failing. Removing a page's reference while leaving the file gives 121 and fails; linking one of the 120 gives 119 and fails with the instruction to lower the constant. The empty shortcodes are deliberately left in place. Deleting them would erase the evidence and leave the prose promising pictures that never arrive, and recovering the images needs the source export, since gallery membership and order are not derivable from what is carried here. TODO.md records the defect, names the three posts, and notes that the 120 are not all gallery losses. Verified through make-release.sh, which is the checker's real caller, then against the local mirror: PASS - 1245 URLs honored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens the URL contract gate by adding an “inward” check that fails when the built site carries media files under public/media / public/external that are not referenced by any built HTML page, closing a gap where conversion-dropped images could remain on disk and still pass existing outward-only checks.
Changes:
- Add
check_orphanstochecks/check-url-parity.py, enforcing an exact expected orphan count viaORPHANED_MEDIA. - Refactor asset reference scanning into
collect_refs/ref_to_pathso assets and orphan checks share the same reference set. - Document the new gate behavior in
checks/README.mdand record the conversion/gallery defect inTODO.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
checks/check-url-parity.py |
Adds orphan-media detection and refactors reference collection to share work between asset and orphan checks. |
checks/README.md |
Updates the gate description and explains why media must be checked in both directions. |
TODO.md |
Records the identified orphan-media/galleries issue and points to ORPHANED_MEDIA as the tracking constant. |
…does check_orphans compared a native path against a set of URL paths. On Windows `relative_to` yields backslashes while every reference is forward-slashed, so nothing would have matched and all 1,048 carried files would have reported as orphans, failing the gate with a number that describes the separator rather than the site. check_render normalizes for exactly this reason, three functions above, so the new code was inconsistent with the convention beside it rather than establishing a new question. The comment names the cause, since the replace reads as redundant on the platform CI runs. Shown rather than asserted, via PureWindowsPath: 'media\2010\05\ami-warn3.png' is absent from a URL-keyed set and 'media/2010/05/ami-warn3.png' is present. The call is a no-op on POSIX, and all three gate states are unchanged there: 120 passes, 121 fails upward, 119 fails downward. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
checks/README.md:13
- The README currently says the parity gate ensures “no carried media file is linked from nowhere”, but
check-url-parity.pyexplicitly allows a non-zero orphan count as long as it matchesORPHANED_MEDIA(currently 120). This description is misleading and could cause confusion when the check reportsorphans: 120 ...but still passes.
| [`check-url-parity.py`](./check-url-parity.py) | Every URL that must render exists as a built page, every legacy image URL resolves, every local asset reference points at a real file, and no carried media file is linked from nowhere | Against `public/`, in CI and before any release is installed |
checks/check-url-parity.py:153
- If neither
public/medianorpublic/externalexists (or they contain no files),carriedstays 0 and the check currently falls into the “media was restored…lower ORPHANED_MEDIA” branch becauselen(orphaned) < ORPHANED_MEDIA. That diagnostic is incorrect in this case; it should be treated as a hard failure indicating the build output is incomplete or mislocated.
print(f"orphans: {len(orphaned)} of {carried} carried media files are linked from no page")
if len(orphaned) == ORPHANED_MEDIA:
return []
# The explanation is printed rather than returned, so the caller's count stays the orphan
# count. A diagnostic carried in the failure list would make the reported total one too many.
if len(orphaned) > ORPHANED_MEDIA:
print(f" expected {ORPHANED_MEDIA} - a page stopped linking media it used to link")
return orphaned
print(
f" expected {ORPHANED_MEDIA} - media was restored to a page, so lower "
f"ORPHANED_MEDIA to {len(orphaned)} in this change rather than leaving the slack"
)
checks/check-url-parity.py:6
- The module docstring says “no carried media file is linked from nowhere”, but the new orphan check allows a non-zero orphan count as long as it matches
ORPHANED_MEDIA. Update the wording so it reflects that this is a baseline-count check rather than asserting zero orphans.
Checks that every URL which must render exists, that every legacy media URL resolves, that
every local asset reference points at a file, and that no carried media file is linked from
nowhere. Redirects need a running server and are checked by check-live-urls.sh instead.
… orphans Three findings from the review of #43, all in the suppressed block. An empty build talked the reader into disabling the gate. With no media trees in the output, `carried` is 0 and `orphaned` is empty, and 0 is fewer than the baseline, so the check reached the branch that reads a drop as progress and advised lowering ORPHANED_MEDIA to 0. Following that advice would have retired the check permanently, on the strength of a build that had produced nothing. That is the failure mode the rule about gates failing loud rather than narrowing quietly exists for, and this one narrowed to nothing while printing a sentence that sounded like good news. No media at all is now a hard failure that names the cause, ahead of any comparison against the baseline. The module docstring and the gate table in checks/README.md both said the check proves "no carried media file is linked from nowhere". It does not, and cannot today: 120 are. It proves the count still equals its recorded baseline. Read beside output beginning `orphans: 120`, the old wording invited the reader to conclude the check was broken rather than that the site is. Both now say what it actually asserts. The new branch was demonstrated failing before being trusted: with the media trees removed the check exits 1, names an incomplete or mislocated build, and emits no advice to lower the constant. The other three states are unchanged at 120 pass, 121 fail, 119 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the three findings in the Suppressed comments (3) block of the round-2 review. They carry no threads to resolve, so they are quoted and answered here. All three were accepted and fixed in
Accepted, and it was the worst of the three. The consequence is larger than an incorrect diagnostic: following the advice would have retired the gate permanently. With no media in the output, That is exactly what the rule about gates failing loud rather than narrowing quietly is for, and this one narrowed all the way to nothing. No media at all is now a hard failure that names the cause, evaluated ahead of any comparison against the baseline. Demonstrated failing before being trusted:
Both accepted. The claim was wrong, and not marginally: 120 carried files are linked from nowhere, which is the entire reason the check was written. Worse, sitting beside output that begins The other three states are unchanged: 120 passes, 121 fails upward, 119 fails downward with the instruction to lower the constant. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
checks/check-url-parity.py:160
- The failure messages in
check_orphansare more specific than the code can actually prove. An orphan-count increase could be caused by adding new unreferenced media (not only by a page dropping a link), and a decrease could also be caused by deleting orphaned files from disk (not only by restoring links). Making the message describe the observable condition avoids misleading future debugging.
if len(orphaned) > ORPHANED_MEDIA:
print(f" expected {ORPHANED_MEDIA} - a page stopped linking media it used to link")
return orphaned
print(
f" expected {ORPHANED_MEDIA} - media was restored to a page, so lower "
f"ORPHANED_MEDIA to {len(orphaned)} in this change rather than leaving the slack"
)
A count is all check_orphans can see, and two causes reach each direction. It rises when a page stops linking media and equally when unlinked media is added. It falls when media is linked from a page and equally when orphaned files are deleted. The messages named one cause each and stated it as fact, so a run that had the other cause would have sent a reader looking for a page that never changed, with the check's own output vouching for the wrong theory. This is the same overclaiming the previous commit removed from the docstring, left in place one function lower. Asserting a cause the code cannot observe is worse in a failure message than in prose, because a failure message is read by someone who has no context yet and is deciding where to look first. Both messages now name both causes. The action after a drop is unchanged and still explicit, since lowering the constant is correct whether the count fell because media was linked or because orphaned files were removed. checks/README.md gains the same distinction, so the reasoning survives the next edit to the message text. All four states re-verified: 120 passes, 121 and 119 fail with the reworded messages, and an empty build still fails outright without advising that the constant be lowered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the round-3 suppressed finding. No thread to resolve, so it is quoted and answered here. Accepted and fixed in
Accepted. A count is all the check can observe, and two causes reach each direction. It rises when a page stops linking media and equally when unlinked media is added; it falls when media is linked from a page and equally when orphaned files are deleted. The consequence is worse in a failure message than in prose. A failure message is read by someone who has no context yet and is deciding where to look first, so a run with the other cause would have sent them hunting for a page that never changed — with the check's own output vouching for the wrong theory. It is also the same overclaiming the previous commit removed from the module docstring, left sitting one function lower. Fixing the description while leaving the diagnostic was half a fix. Both messages now name both causes: The action after a drop is unchanged and still explicit, since lowering the constant is correct either way. All four states re-verified: 120 passes, 121 and 119 fail with the reworded messages, and an empty build still fails outright without advising that the constant be lowered. |
The gap
The URL contract checked media in one direction only:
media: 778/778assets: 1012/1012Both read outward from a reference. Neither can ask whether anything points at a file. So an image the WordPress conversion dropped from a page stays on disk, stays reachable at its own URL, and reports green in both directions while appearing nowhere on the site.
That is the
Verification Disciplineshape exactly: the failure is green.It is not hypothetical
120 of the 1,048 carried media files are linked from no page. Five
galleryshortcodes are empty across three posts, andgalleryis the only shortcode this happens to.The clearest case is the ESP32 post, which says:
and then renders
<div class="gallery gallery-cols-4"></div>. The post references two images; a sequential run of camera originals from that same month sits on disk, referenced by nothing, ending right where the one surviving photo picks up.Every gate passed the entire time.
The change
check_orphansreads inward from the file and is the only check that can see this.collect_refsandref_to_pathare factored out ofcheck_assets, because the two checks are the same reference set read in opposite directions — walking the built site twice to rebuild it would be waste.ORPHANED_MEDIAis an exact count, not a bound. A ceiling would let restored media leave slack behind for a later regression to hide in, so the check fails when the count falls as well as when it rises, and names the number to write when it falls. The two messages differ, because "a page stopped linking media" and "media was restored" are opposite events a single count cannot distinguish on its own.The explanation is printed rather than returned in the failure list, so the caller's
N missingstays the orphan count. Carrying a diagnostic in that list reported122for 121 orphans.Demonstrated failing before being trusted
Per the rule that a gate is only trusted after it has been watched failing, all three states:
orphans: 120 of 1048— PASSorphans: 121→expected 120 - a page stopped linking media it used to link— exit 1orphans: 119→expected 120 - media was restored to a page, so lower ORPHANED_MEDIA to 119 in this change rather than leaving the slack— exit 1The first attempt at the downward case did not fire, because the image picked was already linked in the built site and was not a genuine orphan. Redone against an orphan taken from the check's own output.
Verification
make-release.sh, the checker's real caller, rather than standalone only.PASS - 1245 URLs honored, rules confirmed to come from the release just installed.render 328/328,media 778/778,assets 1012/1012,orphans 120 of 1048.What this deliberately does not do
It does not delete the empty shortcodes. That would erase the evidence and leave the prose promising pictures that never arrive. Recovering the images needs the source export, since gallery membership and order are not derivable from anything carried in this repo.
TODO.mdrecords the defect, names the three posts and the five call sites, and states plainly that the 120 are not all gallery losses — the remainder is unadjudicated and may include uploads the old platform never published. The number is a starting point for that work, not a claim about it.🤖 Generated with Claude Code