DOC-7104: Add a check for links left in bare, uncanonicalized form - #4102
Merged
Merged
Conversation
migrate_shortcode_links.py's linkify stage correctly canonicalizes almost every converted link to /content/<path>.md[#anchor], but a link that gets a manual post-hoc text fix after the pipeline already ran (e.g. inserting a separator slash a relref-plus-literal-suffix concatenation was missing) never gets a second pass through linkify. The result is a bare /operate/... path that resolves to the exact same rendered href as the canonical form, so build/diff_rendered_hrefs.py -- this migration's usual verification -- is blind to it by construction. Found on PR #4086 (DOC-7104 release-notes/ unit): human review manually flagged 8 malformed links; a corpus-wide grep for the same shape found 21 across 9 files (13 more than manual review caught), plus 8 more that turned out to be genuinely pre-existing dead links in the identical shape, invisible to any prior check since Hugo's relref shortcode only ever validated its own target, never text concatenated onto it afterward. check_uncanonicalized_links.py reuses migrate_shortcode_links.py's own resolver (_find_content_file) so a --fix run applies the exact same rewrite the pipeline would have. Verified against the pre-fix state of PR #4086: reproduces the same 21 FIXABLE / 8 DEAD split exactly, and a full-corpus scan of content/ elsewhere comes back to 3 unrelated hits, confirming it isn't noisy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Contributor
🧠 Redis MemoryFound 9 related items from repository history (4 new this commit):
Memory updated at d2f6723 |
…o slash The original regex required `/` or end-of-string right after the mount name, so a link like `/commands?group=cluster` (no trailing slash before the query) silently passed through unchecked -- this tool's own blind spot, found the hard way: human review caught it by hand on DOC-7104 PR #4093, and the identical instances recurred in #4094/#4096/#4098 before this fix existed to catch them. Also reconsiders the `/commands` special case: it has no backing _index.md on disk, so _find_content_file always reports it unresolvable, but Hugo auto-generates a section page for the directory and GetPage finds it anyway (confirmed by building both /commands?group=x and /content/commands?group=x and comparing rendered hrefs -- identical). Review wanted the canonical form applied there too, so it's now hardcoded as FIXABLE instead of silently skipped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dwdougherty
approved these changes
Sep 24, 2026
dwdougherty
left a comment
Collaborator
There was a problem hiding this comment.
Code skimmed. Approved.
Contributor
Author
|
Thanks for your heroic review of all these PRs @dwdougherty ! Hopefully this check will prevent any more issues like the ones you've found. |
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.
Summary
migrate_shortcode_links.py'slinkifystage correctly canonicalizes almost every converted link to/content/<path>.md[#anchor], but a link that gets a manual post-hoc text fix after the pipeline already ran (e.g. inserting a separator slash a relref-plus-literal-suffix concatenation was missing) never gets a second pass throughlinkify. The result is a bare/operate/...path that resolves to the exact same rendered href as the canonical form, sobuild/diff_rendered_hrefs.py— this migration's usual verification — is blind to it by construction.release-notes/unit): human review manually flagged 8 malformed links; a corpus-wide grep for the same shape found 21 across 9 files (13 more than manual review caught), plus 8 more that turned out to be genuinely pre-existing dead links in the identical shape, invisible to any prior check since Hugo'srelrefshortcode only ever validated its own target, never text concatenated onto it afterward.build/check_uncanonicalized_links.pyreusesmigrate_shortcode_links.py's own resolver (_find_content_file) so a--fixrun applies the exact same rewrite the pipeline would have. Three outcomes:FIXABLE(resolves,--fixrewrites it),MOUNT_ONLY(resolves only through a Hugo module mount — never auto-fixed, same rationalemigrate_shortcode_links.pyalready documents for not following mounts on a rewrite),DEAD(doesn't resolve at all — reported only, never guessed).FIXABLE/ 8DEADsplit. A full-corpus scan ofcontent/elsewhere comes back to 3 unrelated hits, confirming it isn't noisy.Test plan
build/test_check_uncanonicalized_links.py— 7 cases (fixable, fixable-with-anchor, dead, already-canonical, external/anchor-only/mailto skipped,/commands/?group=exception, the exact PR DOC-7104: Migrate content/operate/rs/release-notes/ to render hooks #4086 shape), all passing (nopytestin this repo's venv, ran the test functions directly — matches this migration's otherbuild/test_*.pyscripts, none of which are wired into CI either)release-notes/content into the full corpus and scanningcontent/scan: 3 unrelated hits elsewhere, all genuine (2 bare/commands/<cmd>links in already-convertedoss_and_stackrelease notes, 1 inoperate/rs/7.4/_index.md)🤖 Generated with Claude Code
Note
Low Risk
Build-only tooling and tests; no runtime or published docs behavior unless someone runs
--fixon markdown files.Overview
Adds
build/check_uncanonicalized_links.py, a corpus scanner that catches Markdown links still pointing at bare content mounts (/operate/,/develop/,/integrate/,/commands) instead of the migration’s canonical/content/<path>.mdform—casesdiff_rendered_hrefs.pymisses because they render the same href.Each hit is classified as FIXABLE (same resolver as
migrate_shortcode_links; optional--fixrewrites in place), MOUNT_ONLY (report only), or DEAD (broken link, report only)./commandslinks with query/fragment (no slash before?/#) are treated as fixable to/content/commandseven without an on-disk_index.md.build/test_check_uncanonicalized_links.pyadds filesystem-backed tests for fixable/dead/skipped cases, anchors, the/commands?group=shape, and the PR #4086 relref-plus-suffix defect.Reviewed by Cursor Bugbot for commit d2f6723. Bugbot is set up for automated code reviews on this repo. Configure here.