DOC-7003 add report-only fragment checking and link house rules - #3866
Open
andy-stark-redis wants to merge 2 commits into
Open
DOC-7003 add report-only fragment checking and link house rules#3866andy-stark-redis wants to merge 2 commits into
andy-stark-redis wants to merge 2 commits into
Conversation
Adds a second, non-gating lychee pass over fragment-bearing URLs, a documented fragment-only exclusion file, and three external-link rules in AGENTS.md. The ticket proposed enabling fragment checking with "only a config key". Testing against the pinned lychee 0.24.2 showed that cannot work. A missing fragment exits 2, exactly as a dead page does, so one run cannot report anchors without also gating on them. And there is no fragment-specific exclude — passing a host to --exclude drops the URL from checking altogether, so suppressing anchor noise on a host would also stop us noticing when its page dies. Hence a separate invocation over just the fragment-bearing URLs, layering CLI flags on the existing config so throttling and the user agent cannot drift between the two runs, with its exit code deliberately never consulted and its findings going to their own tracking issue. Two smaller findings shaped the implementation. A misspelled config key exits 3 with the list of valid keys, so a typo here fails loudly rather than silently disabling the check. And an exclude file does honour "#" comments, which is what lets the exclusions carry their justifications — established with a comment line that would have matched as a regex if it were being applied as one, since the obvious test cannot tell a working comment from a regex that simply misses. The substantive correction is to the exclusion list. The ticket named eight hosts to exclude as JS-rendered. Fetching one failing URL per host and looking for the anchor in the served HTML supports two of them, and the stated reason is wrong even there: on both AWS and Google the anchor is plainly present in the HTML that curl receives, alongside other server-rendered content ids, so these are checker false positives by some mechanism I could not establish rather than JS rendering. For the other six an independent fetch could not find the anchor either, which makes those failures possibly real, so excluding them would have hidden genuine rot. They stay in the report. An id count is not the test to use here, incidentally, because navigation and page chrome inflate it on any real docs page. The first run also earned its keep immediately: it independently rediscovered all four Part A2 anchors, confirmed the railway anchor that shipped as an unverified gap in the previous PR is genuinely dead, found two dead Wikipedia anchors nobody had reported, and showed that cloud.google.com now redirects to docs.cloud.google.com, which is 16 links wanting a new host. Learned: verify a candidate exclusion by finding the anchor in the served HTML — six of the eight hosts this ticket wanted excluded turned out to be plausibly real failures, and the JS-rendered rationale was wrong even where the exclusion held. Constraint: the fragment pass must never feed the gating flag, because lychee returns exit 2 for a dead anchor and a dead page alike. Rejected: enabling fragments on the existing gating run via a config key | no fragment-specific exclude exists and exit 2 is ambiguous, so it would gate the build on anchor noise Directive: don't add a host to .lychee-fragment-excludes.txt without fetching a failing URL and confirming the anchor really is in the served HTML. Gaps: the 34 surviving failures are classified by host, not individually triaged; the railway anchor, two Wikipedia anchors and the cloud.google.com host move are real findings this PR does not fix. 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}
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 2f669fd. Configure here.
Bugbot found a real interference path, and it defeats the one property this change was supposed to guarantee. The report-only fragment steps sat between flagging broken links and uploading the gating report. `fail: false` covers a non-zero lychee exit, but not a step that dies outright -- an action setup or artifact failure. Because a step whose `if` omits a status function carries an implicit success(), such a death skipped the gating upload. report_broken_links runs on always(), so it would then have gone looking for an artifact that was never produced, failed, and silently suppressed the broken-link tracking issue. A dead anchor could have hidden a dead page, which is exactly backwards. Fixed by ordering rather than by more conditionals: the gating upload and the failing step now come first, and the fragment block runs last with always() on every step so it still survives the exit 1 without being able to interfere. The extract step also now reports zero when external-urls.txt is absent, so a failed build cannot present itself as a fragment problem. Learned: a report-only step is only non-gating if it cannot fail *before* the gating steps — implicit success() makes step order part of the contract, not a cosmetic choice. Constraint: the fragment steps must stay after the gating upload and the failing step, and must keep always(), or a fragment failure suppresses the broken-link issue. Ticket: DOC-7003 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history (5 new this commit):
Memory updated at 8056759 |
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.
Parts B2 and B3 of DOC-7003. Based on
main, independent of #3865.Adds a report-only fragment (anchor) check to the weekly link workflow, a documented
fragment-only exclusion file, and three external-link rules to
AGENTS.md.B2 could not be done the way the ticket described
The ticket says fragment checking "needs only a config key". Tested against the pinned
lychee 0.24.2 locally, it doesn't:
include_fragments = "anchor-only"in TOML--exclude <host>on a fragment run0 OK, 2 Excluded)--exclude-filewith#comments--config .lychee.tomlTwo of those are decisive. Because a fragment failure is indistinguishable from a broken
link by exit code, and because excluding a host to quiet anchor noise would also stop us
noticing when that host's page dies, this is implemented as a separate lychee
invocation rather than a config flag on the existing run.
Design consequences:
gating pass rather than doubling it.
.lychee.tomlinstead of copying it, so throttling, user agent andaccepted status codes cannot drift apart.
anchor report.
link-check-fragments) so an anchor triage backlog can't buryan actually-dead page.
The exclusion list: 2 of the ticket's 8 hosts justified
The acceptance criteria ask for exclusions "justified in a comment". Testing each proposed
host — fetch a failing URL, look for the anchor in the served HTML — most don't survive:
docs.aws.amazon.com(8 failures)id="concepts-availability-zones")cloud.google.com(4)id="overview-of-cloud-billing-roles-in-cloud-iam")expressjs.com,rubydoc.info,sbert.net,registry.terraform.io,docs.brew.sh,docs.docker.com,www.terraform.ioThe stated rationale is wrong even where the exclusion holds. On both AWS and Google the
anchor is plainly in the HTML
curlreceives, next to other server-rendered content ids —so these are checker false positives by a mechanism I could not establish, not JS
rendering. The exclusion file says exactly that rather than repeating a guess.
For the other six, an independent fetch couldn't find the anchor either. Two non-JS methods
agreeing that an anchor isn't in the served HTML makes those failures possibly real, so
excluding them would hide genuine rot. They stay in the report — which is the whole point of
report-only.
One methodological note for whoever extends this: counting
id=attributes is not thetest. Navigation and page chrome inflate it on any real docs page (AWS shows 43, Google
31), so a high count neither confirms nor refutes JS rendering. The discriminating test is
whether the specific fragment is in the served HTML.
The first run already earned its keep
Triaged locally: 47 failures before exclusions, 34 after. Among them:
DOC-7003 retarget dead k8s API anchors to the local reference #3865 fixes, which is a decent correctness signal.
#tcp-proxyinganchor is genuinely dead. It shipped in DOC-7003 fix dead and brittle links in the develop docs #3864 as anunverified
Gaps:note; two independent methods now agree, so it needs a fix.Cloud_computing#Public_cloudandCron#CRON_expression.cloud.google.comsilently 301s todocs.cloud.google.com— 16 anchored links wantrepointing, after which its exclusion may become unnecessary.
third independent confirmation that leaving it alone was right.
Those four findings are not fixed here; they're follow-up work, listed in the commit's
Gaps:trailer so they aren't lost.B3 — three rules in
AGENTS.mdAdded to the existing Site mechanics list rather than a new section, since that file states
it stays small on purpose:
relrefto an external anchor where we document the same thing.SHA you pin. A whole-repository or whole-file link may stay on the default branch.
#:~:text=) fragment.Plus one clause on the existing
relrefbullet: relref validates the page, never theheading, so a relref with a dead
#anchorbuilds clean. That's the class A3 came from andthe reason the offline checker in the next PR is worth building.
Each of these earned its place in the last two PRs rather than being asserted: pinning
proved itself twice in one afternoon (the MANIFESTO anchor was right and pinning preserved
it; the boringssl range was off by one and pinning made the corrected range safe to state),
and two scroll-to-text fragments were removed in #3864.
Verification
— the
-sguard is what makes a skipped step read as "not flagged" rather than flagged.🤖 Generated with Claude Code
Note
Low Risk
Changes are CI reporting and contributor guidance only; the gating external link check behavior is unchanged aside from added post-fail steps that cannot fail the workflow.
Overview
Extends the weekly link_check workflow with a second, non-gating lychee pass on fragment-bearing external URLs (
--include-fragments=anchor-only), filtered to drop#/SPA routes. Fragment steps run after the failing gating step withalways()so broken links still upload artifacts and open issues; a newreport_dead_fragmentsjob fileslink-check-fragmentstracking issues separately from dead pages.Adds
.lychee-fragment-excludes.txt(AWS andcloud.google.comregexes) used only on the fragment run so known lychee false positives do not flood triage while the main check still validates those hosts.Documents external-link house rules in
AGENTS.md:relrefdoes not validate#anchors, prefer internalrelrefover external anchors, pin GitHub deep links to commit SHAs, and avoid#:~:text=scroll-to-text fragments.Reviewed by Cursor Bugbot for commit 8056759. Bugbot is set up for automated code reviews on this repo. Configure here.