DOC-7003 add an offline checker for internal links and anchors - #3867
Open
andy-stark-redis wants to merge 6 commits into
Open
DOC-7003 add an offline checker for internal links and anchors#3867andy-stark-redis wants to merge 6 commits into
andy-stark-redis wants to merge 6 commits into
Conversation
Adds build/check_internal_anchors.py, 13 tests, and a make target. It resolves two classes of link that nothing currently validates: absolute redis.io self-links, which Hugo treats as external and .lychee.toml excludes as internal, and relref with an anchor, where the page resolves so the build passes while the fragment is dead. The ticket's self-link count needed reconciling first, and the answer was that the two figures in it measure different things. The built site emits 7,639 absolute redis.io hrefs, but roughly 28 of those are on every page -- footer legal links, try-free, nav -- so that number is theme chrome multiplied by the page count and is useless as a lint target. Hand-authored self-links in live source are 199 distinct across 309 instances. So the checker finds links in source and resolves them against the built tree, rather than extracting hrefs from the build. Only 110 of those 199 are resolvable at all. The rest point at redis.io/blog, /legal, /pricing and similar, which are the marketing site, built elsewhere and legitimately absent from public/. Reporting them as broken would have buried the real findings under twice their number, so they are skipped explicitly. Anchors are read out of Hugo's own output rather than derived from a slug rule, because this repo already has three disagreeing slug implementations and a fourth would be a liability. Two false-positive classes showed up on the first run and are handled rather than tolerated. Non-page artifacts -- sitemap.xml, docs.ndjson, the .md twin of each page -- are real files, so the resolver tries the literal path before treating a path as a page. And the OpenAPI reference pages mount their content with Redoc, so their static HTML carries 3 anchors against 130 on a normal page; those anchors are real in a browser and unreadable here. They are now a third verdict, unverifiable, which removed 71 of an unfiltered 181 findings. A checker that called them missing would be the kind of report people learn to ignore. What survives is 110 findings, of which about 104 are dead relref anchors. Three were verified by hand against the built pages before trusting the number, and all three were real -- one where the target page had split "Create certificates" into four narrower sections, leaving the anchor behind. The suite was also mutation tested: short-circuiting the anchor comparison turns two tests red, so a green run means something. Learned: the ticket's two self-link counts measured different domains, and the built-site figure was theme chrome times page count -- reconcile a count's definition before designing anything around it. Constraint: anchors must come from the built HTML, never from a reimplemented slug rule; three implementations already disagree in this repo. Rejected: extracting self-links from the built site the way extract_external_urls.sh does | every page carries ~28 theme-generated redis.io hrefs, so it measures the template, not the writing Directive: keep the three-way verdict — a page whose content is client-rendered is unverifiable, never missing, or the report becomes noise nobody reads. Gaps: the ~104 relref findings are verified on a sample of three, not individually; docs.ndjson resolves only after a full `make ndjson`, so a bare `hugo` build reports it absent; the post-merge workflow is not wired up yet. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Contributor
🧠 Redis MemoryFound 9 related items from repository history (1 new this commit):
Memory updated at c9e25ee |
Bugbot found two coverage gaps and both were real. Measured on the current tree, the
extraction saw only one of three relref shapes, so 124 distinct slashless refs and 42
distinct trailing-fragment refs were never checked -- invisible rather than reported,
which is the worse failure for a tool whose value is completeness.
I built the pattern around the dominant form and never asked which shapes existed.
That is the blind-sample failure mode, and a reviewer found it rather than the tests,
because every test I wrote used the shape I had already thought of.
Now handled:
{{< relref "./rel#anchor" >}} and {{< relref "bare/rel#anchor" >}}
{{< relref "/abs/path" >}}#anchor
Relative paths resolve against the directory of the page carrying the link, then as a
site path. The resolution is deliberately conservative and can only under-report: a
relref whose path is wrong fails the Hugo build, so every relref in the tree resolves
somehow, and a path this model cannot find is a limit of the model rather than a
defect in the docs. Those count as unhandled and never as findings, which makes a
false positive structurally impossible for the relative forms. Only 2 more landed in
that bucket, so the model covers nearly all of them.
Effect on the same tree: 2,411 to 2,682 anchored relrefs actually checked, and 34
further real defects surfaced. One verified by hand -- fcall_ro.md links
#read-only_scripts with an underscore where the heading slug uses a hyphen.
Six new tests cover both shapes, each with a valid-form control, plus one asserting an
unresolvable relative path is unhandled rather than reported. Mutation tested by
restoring the old single-shape pattern, which turns six of them red.
Learned: my tests all used the link shape I had already thought of, so they could not expose a missing shape — for an extraction tool, enumerate the shapes present in the corpus before trusting any coverage number.
Constraint: resolve_relref returning None must stay "unhandled", never a finding; a bad relref path fails the build, so non-resolution is this model's limit and reporting it would manufacture false positives.
Ticket: DOC-7003
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot caught a false-positive class I introduced one commit earlier. Broadening the path pattern to accept slashless refs made relref "#anchor" match for the first time, with an empty path, and the relative branch then treated empty as "the directory of this page" -- which is the sibling section _index, not the page itself. Two live links were reported broken: jedis/failover.md's #retry-configuration and strings.md's #string-counter-support, both of which are present on their own pages. An empty path now resolves to the source file's own built page, with _index.md mapping to its directory rather than to a child. Findings drop 152 to 150, and the three that remain are real: the same-page anchors in active-active/_index.md carry a trailing slash, and #multi-primary-replication/ is absent from its own page while #multi-primary-replication is present. The fixture for the new tests puts a leaf page and a section _index side by side with *different* anchors, so a resolver that picks the wrong page fails rather than passing by luck. Mutation testing this cost two attempts: the obvious edit hit an identically worded `if not rel:` in resolve_page instead, the suite stayed green, and that looked like the tests being weak when it was the mutation missing its target. Learned: broadening an extraction pattern can hand new input shapes to a resolver that was never written for them — a coverage fix and a resolution fix are different changes, and the second is where the false positives come from. Constraint: an empty relref path means the current page; routing it through the directory-relative branch validates against the sibling section index. Directive: when mutation testing, confirm the mutation landed where you meant — a duplicated line elsewhere in the file will absorb it and the green suite will look like weak tests. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wires build/check_internal_anchors.py into CI, in its own workflow rather than into link_check, which a sibling PR is already rewriting. Report-only by construction. The checker exits 1 on findings so that a local `make check_internal_anchors` is useful, and the job deliberately does not inherit that. It does still fail on exit 2, which is the checker's "resolved nothing at all, so the scoping or the build is broken" signal -- `|| true` would have swallowed the one outcome that means the report cannot be trusted. Weekly rather than post-merge, which is the opposite of alias_check's choice and for a stated reason: that scan needs no build and takes three seconds, whereas this needs a full Hugo build to read the anchors Hugo itself emitted. A per-push run would add about ten minutes to every merge to report rot that is days old rather than anything that merge introduced. The report renderer is embedded in the workflow, so it was extracted from the YAML and run verbatim against real findings before committing. That caught a crash: groupby lives in itertools, not collections, and the mistake would only have surfaced in a scheduled run a week later, after the artifact upload had already succeeded. Learned: an embedded heredoc script gets no linting and no import check, so run it verbatim out of the YAML before trusting it — a scheduled workflow hides that class of error for a week. Constraint: keep the exit-2 escalation; the checker uses it for "resolved nothing", and swallowing it would let a broken build report zero findings as a clean result. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more valid Bugbot findings, both under-reporting rather than false alarms, which is the failure mode this tool can hide best. Hugo's relref accepts a source filename, so relref "/glossary/_index.md#letter-a" means that page. Those paths were forwarded to a resolver that only looks for a literal file, came back unresolved, and were counted as unhandled -- 42 distinct across the corpus, 9 of them in live sources. Stripping the .md, /index.md and /_index.md suffixes is done in the relref path only, deliberately not in resolve_page, because that function also resolves self-links and the docs tree really does publish literal files: sitemap.xml, docs.ndjson, and the .md twin of every page. A link to one of those must keep its extension, and there is now a test pinning that. The client-rendered check matched the bare substrings redoc, swagger-ui and rapidoc anywhere in a page's HTML. That is true of ordinary tutorial pages -- redisom-for-java quotes a springfox-swagger-ui dependency and a localhost:8080/swagger-ui/ URL -- so 14 pages were being skipped when only 7 genuinely build their content in the browser, and any real defect in a link to the other 7 was invisible. It now matches the mount instead: the <redoc> and <rapi-doc> elements, a swagger-ui mount node, or the bundle script. Checked against the corpus first, and every real mount uses the element form; none uses a div id. Which exposed a weak test of my own. The original client-rendered test invented `<div id="redoc">` markup that no page emits, and it passed only because the old detector matched any occurrence of the substring. Tightening the detector turned it red, correctly. The fixture now uses the markup the api-reference pages actually emit. Net effect: 2,684 to 2,699 anchored relrefs checked, unresolved paths 17 to 7, unverifiable 75 to 69, findings 150 to 151. Learned: a fixture that does not match production markup validates nothing — my client-rendered test only passed because the detector was loose enough to match invented markup, so tightening the detector is what exposed the test. Constraint: strip .md suffixes for relref paths only, never in resolve_page — self-links legitimately target published literal files such as sitemap.xml and each page's .md twin. Directive: before adding a marker to CLIENT_RENDERED, grep the corpus for how that tool actually mounts; matching a name rather than a mount silently skips real pages. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e773e88. Configure here.
Bugbot's sixth finding, and the most consequential so far because it made the tool lie in the reassuring direction. The anchor pool accepted any id= or name= attribute. A browser jumps to an id on any element, or a name on an <a>, and nothing else -- so the pool was also collecting <meta name="description">, viewport, robots and generator from every page, the name= inside data-name=, and, via a URL query string, the tag manager container id from a noscript iframe. On the protocol spec page that was 114 values against 86 real targets. The consequence is a false negative: a link to #description would have been reported present on every page in the corpus. Nothing in the corpus exploits it today -- no relref references description, viewport, robots or generator -- so this was latent rather than active. Worth fixing anyway, since comment, format and docset were also in the pool and those are plausible anchor names, and since a checker that can silently pass a dead link is not worth running. Three tightenings, each verified to remove noise without dropping a real target: name= is now read only inside <a> tags; script and style bodies are removed before scanning, because nothing inside them is a jump target; and both attributes require whitespace before them, which is what separates <div id="x"> from gtm.js?id=GTM-XXXX. The pool on that page is now 86, which matches an independent hand count of 69 ids plus 17 <a name> targets. Corpus totals are unchanged at 151 findings and 2,699 relrefs checked, so no real anchor was lost in the process. Learned: a checker's anchor pool is its trust boundary — every spurious member converts a dead link into a silent pass, so measure the pool against an independently counted page rather than assuming a regex is close enough. Constraint: name= counts only on <a> tags, and both attributes require preceding whitespace; loosening either readmits meta names and URL query parameters as anchors. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <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.
Part B1 of DOC-7003. Based on
main, independent of #3865 and #3866.build/check_internal_anchors.py+ 13 tests +make check_internal_anchors. Offline,deterministic, no network. Catches the two classes nothing currently validates:
redis.ioself-links — Hugo treats them as external,.lychee.tomlexcludes
redis.ioas internal, so they fall in a hole. Four legacy ones hard-404 today.relrefwith an anchor — the page resolves so the build passes while the fragmentis silently dead. This is the A3 class.
The count reconciliation came first, and changed the design
The ticket carried two figures. They measure different things:
redis.iohrefs in the built sitepublic/at all7,639 is theme chrome × page count — useless as a lint target. So the checker finds
links in source and resolves them against the built tree, rather than extracting
hrefs from the build the way
extract_external_urls.shdoes.The other 89 point at
redis.io/blog,/legal,/pricingand friends — the marketingsite, built elsewhere and legitimately absent from
public/. Reporting those as brokenwould bury the real findings under twice their number, so they're skipped explicitly.
Two false-positive classes, handled rather than tolerated
The first run produced 181 findings. 71 were noise:
sitemap.xml,docs.ndjson, the.mdtwin of every page arereal files. The resolver now tries the literal path before treating a path as a page.
anchors in static HTML against 130 on a normal page. Those anchors are real in a browser
and unreadable here, so they're a third verdict: unverifiable, never missing. That's
the audit's own "three-way verdict, never pass/fail" rule, and it removed 69 findings.
A checker that called them broken is the kind of report people learn to ignore.
What it finds: ~104 dead relref anchors
110 findings survive. Verified three by hand against the built pages before trusting the
number — all three real. The clearest:
saml-sso.mdlinkscreate-certificates#create-certificates, but that page now hascreate-ca-signed-certificates,create-certificates-with-openssland two more — thesection was split and the anchor left behind.
It also independently rediscovered the redisvl
#range-querydefect the ticketpredicted (fix upstream), and found
redis.io/commands/DEBUG-OBJECT/DEBUG-SEGFAULT,whose uppercase paths won't resolve on a case-sensitive host.
Verification
13 tests, including the two the acceptance criteria name — a deliberately broken absolute
self-link, and a
relrefwith a bad anchor — each with a positive control assertingthe valid form is accepted, so the suite can't pass by finding nothing. Others cover
<a name>legacy anchors, marketing-site skipping, the unverifiable verdict, archived-treescoping, non-page artifacts, and
/docs/vs/docs/latest/prefix equivalence (CI rewritesbaseURL, so a mapping hardcoded to the local layout would silently pass there).Mutation tested: short-circuiting the anchor comparison turns exactly two tests red, and
restoring turns them green. A green run means something.
There's also a guard that exits 2 if zero links resolve — an all-clear from a broken scope
must not look like a pass.
Not in this PR
alias_check.yaml'spost-merge pattern; the checker, tests and make target are self-contained and reviewable
without it, so I've stopped here rather than rush it. Happy to add it to this PR or a
follow-up.
not part of this change.
docs.ndjsonresolves only after a fullmake ndjson; a barehugobuild reports itabsent. Worth knowing before reading a local run's output.
🤖 Generated with Claude Code
Note
Low Risk
Adds report-only CI and local lint tooling; no changes to published docs, Hugo config, or blocking merge checks.
Overview
Adds offline validation for two link types Hugo and lychee do not cover: full
https://redis.io/...self-links in source andrelreflinks with#fragments, by scanning live markdown and resolving targets against a Hugo-builtpublic/tree (anchors taken from rendered HTML, not reimplemented slug rules).build/check_internal_anchors.pyimplements scoping (archives/release notes skipped, marketing URLs skipped, client-rendered OpenAPI pages marked unverifiable), JSON output for automation, and exit codes for localmake check_internal_anchors(needs a prior build).build/test_check_internal_anchors.pyadds broad unit coverage including positive controls and edge cases..github/workflows/internal_anchor_check.yamlruns weekly (and on dispatch): full Hugo build, report-only run (findings do not fail the job; exit ≥2 still fails), artifact report, and a follow-up job that opens or comments on a tracking issue—explicitly not a merge gate given an existing backlog.Reviewed by Cursor Bugbot for commit c9e25ee. Bugbot is set up for automated code reviews on this repo. Configure here.