ci(links): exclude dynamic review comment anchors - #2197
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2197 +/- ##
===========================================
- Coverage 84.96% 84.87% -0.09%
===========================================
Files 351 351
Lines 30208 30208
Branches 30208 30208
===========================================
- Hits 25665 25640 -25
- Misses 4171 4196 +25
Partials 372 372 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Establishes an evidence baseline for current external-link-check failures and narrows the Lychee online configuration to exclude only GitHub pull-request review-comment anchors that are dynamically rendered.
Changes:
- Add a comprehensive markdown baseline classifying all 461 Lychee external link failures from a specific hosted run.
- Update the issue/spec documentation to reference the baseline and mark baseline classification as completed/independently reviewed.
- Add an online-only Lychee exclusion for
.../pull/<n>#discussion_r<id>review-comment anchors; update repo wordlist for related terms.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| project-words.txt | Adds kentbeck and stackoverflow to project wordlist (supports docs/link-check triage text). |
| docs/issues/open/2185-2003-triage-advisory-external-link-check-findings/external-link-baseline.md | New baseline evidence doc enumerating and classifying all external-link-check failures. |
| docs/issues/open/2185-2003-triage-advisory-external-link-check-findings/ISSUE.md | Updates the issue/spec status, checklist, and cross-links to include the baseline evidence. |
| .github/lychee-online.toml | Adds a narrowly scoped online-only regex exclusion for GitHub PR review-comment anchors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d493454 to
494b70f
Compare
494b70f to
fcbb4bf
Compare
da2ce7
left a comment
da2ce7
left a comment
There was a problem hiding this comment.
Recomputed review of #2197 at fcbb4bfdc66efb56f82fcc94991970cd78a414d4. Rather than reading the claims, I re-derived them: the retained lychee-external-link-report artifact (id 10102682863, expires 2026-09-23) from run 34347690674 is still available, so every count in external-link-baseline.md was checked against the raw report.
The arithmetic is exact
The artifact's summary reports Total 1677, Unique 1114, Successful 1216, Timeouts 0, Redirected 24, Excluded 0, Unknown 0, Errors 461, Unsupported 0. Every figure in the Run Summary table at external-link-baseline.md:9-19 matches, and 1216 + 461 = 1677. The run's head_sha from the API is 7abc30b2b9fb85b235e7b2ef2a40f5d5f7ed1555, matching the source-revision frontmatter.
The diagnostic prose at external-link-baseline.md:21 is accurate to the bullet: 421 Cannot find fragment, 18 404, 13 cached errors, 5 connection refusals, 3 403, 1 TLS handshake failure, summing to 461.
The nine category counts sum correctly: 416 + 7 + 14 + 3 + 1 + 5 + 3 + 4 + 8 = 461, and C1 + C9 = 424, so "424 of 461" at external-link-baseline.md:138 is exact rather than approximate.
Both inventories are exact too. The 14 package README paths listed under C3 at external-link-baseline.md:96-111 are precisely the 14 distinct source files carrying docs.rs errors in the artifact, and the three C4 URL/source pairs in the table at external-link-baseline.md:117-119 match the report line for line.
The exclusion boundary is exact, and every character of the pattern earns its place
I extracted all 461 URLs from the artifact and applied the regex at .github/lychee-online.toml:12 as a full match. It matches 424 URLs: precisely C1 union C9, no more and no fewer. No discussion_r URL in the report fails to match the anchored form, so the pattern leaves no member of its own category behind, and the residual 9 non-C1/C9/loopback/docs.rs/FSF/403 errors decompose exactly as C4 (3) + C5 (1) + C6 (5).
I then ran a seven-link boundary matrix through lychee --dump with this PR's own config (lychee 0.24.2, no network). Only the matching review-comment anchor is removed. A GitHub issue-comment anchor, a pull URL with no fragment, another repository's review anchor (torrust-index), a suffixed #discussion_r…-extra anchor, the http scheme, and another organisation's path all survive. Since lychee's exclude is an unanchored is_match, the ^, the $ and the escaped dot are what keep this narrow, and the matrix shows each one doing work.
The stated two-link test reproduces verbatim against the same config: [EXCLUDED] …/pull/1733#discussion_r3190844085 | This is due to your 'exclude' values and [ERROR] …/issues/269#issuecomment-1749443211 | Cannot find fragment, ending 2 Total, 0 OK, 1 Error, 1 Excluded, exit 2. The two issue-comment anchors that exist in the baseline are confirmed unmatched by the exclusion, so C6 stays visible.
The exclusion is online-only. The offline configuration is the repository-root lychee.toml, which is untouched; .github/lychee-online.toml is the only file this PR changes under .github/.
A point in the change's favour that the description does not make: running the online config over this PR's own three Markdown files gives 24 total, 18 OK, 0 errors, 6 excluded. Those six are review-comment anchors the PR itself introduces, so without this exclusion the Copilot record added here would have grown the baseline rather than described it.
One correction is needed, and it is in the description rather than the tree
Blocker. Closes #2185 contradicts the spec this same PR commits. ISSUE.md at head leaves AC2 through AC6 unchecked, marks T2/T4/T5 as TODO and T3 as IN_PROGRESS, and leaves "Implementation completed", "Automatic verification completed" and "Manual verification scenarios executed" unticked in the Definition of Done. The description agrees, deferring the hosted rerun to after merge. #2185 is open with no successor issue, and the Deferred Work section at external-link-baseline.md:142 records C3 through C9 as intentionally deferred: fourteen docs.rs repairs, three stale repository links, the Caddy link, and the rerun-first categories.
Merging as written retires the tracking issue for work that has not happened. Related to #2185 matches the state the PR itself records. The edit touches only the description, so the head does not change and no re-review is needed for it.
Everything else checks out
Gates on a clean worktree detached at fcbb4bfd, all green:
| Gate | Result | Wall |
|---|---|---|
linter markdown |
pass | 5.8s |
linter cspell |
pass | 8.5s |
linter yaml |
pass | 0.4s |
linter toml |
pass | 0.1s |
linter shellcheck |
pass | 1.3s |
taplo fmt --check .github/lychee-online.toml |
pass | 0.0s |
lychee --config lychee.toml on the three changed files |
0 errors | 0.0s |
lychee --config .github/lychee-online.toml on the three changed files |
18 OK, 0 errors, 6 excluded | 5.0s |
contrib/dev-tools/checks/format-project-words.sh --check |
already formatted | 0.0s |
No Rust gate was run: the five files this PR owns are one TOML config and four Markdown documents.
CI at fcbb4bfd, snapshot 16:22 UTC after a 25-minute poll:
| Workflow | At fcbb4bfd |
At 494b70ff (pre-rebase) |
|---|---|---|
| Docs Lint | success | success |
| Generate Coverage Report (PR) | success | success |
| Testing | queued | success |
| OS Compatibility | queued | success |
| Container | queued | still running at 32 min |
The three still queued at fcbb4bfd are runner backlog rather than a signal: they were all queued for over twenty minutes without starting. All five had already been observed at 494b70ff, four green and Container still building, and Container runs on this repository routinely take 30 to 60 minutes. Since the rebase changed no file this PR owns and moved the base onto develop itself, I expect those three to land as they did before, but I am reporting what I actually observed rather than what I expect.
project-words.txt gains exactly kentbeck and stackoverflow, both required by new prose in external-link-baseline.md, both in the position the formatter check accepts.
The four commits carry Conventional Commit subjects and no Co-Authored-By or generation trailers.
The Copilot record matches the PR exactly: two threads (PRRT_kwDOGp2yqc6hH6I2, PRRT_kwDOGp2yqc6hH6J1), both resolved, both replied to, and the two rows carry the same thread IDs, paths, comment URLs and reply URLs. No thread is absent from the record and no row lacks a thread. One factual note on the disposition: the || Copilot flagged never existed — grep -c '^||' returns 0 in external-link-baseline.md at all four commits and in ISSUE.md at head — so the threads were false positives rather than outdated. The no-action decision is right either way.
The branch is rebased onto develop at 65a077b5 (the merge of #2189), 0 behind and 4 ahead, and the reviewable diff is 5 files, +218/-12. The description's 8-file figure predates the merges of #2188 and #2189, whose content is now on develop rather than in this branch.
I reviewed 494b70ff first and re-ran everything after the force-push. Comparing the two patches against their respective bases, the only difference is a blob hash and a hunk offset in project-words.txt, caused by pythondontwritebytecode landing on develop between them. The rebase carries no content change, so every result above was reproduced at fcbb4bfd rather than carried over, and the Closes #2185 finding above is unchanged by the push.
|
ACK 3c59070 |
Summary
Validation
linter all.github/lychee-online.tomlpassestaplo fmt --check./contrib/dev-tools/git/hooks/pre-commit.sh./contrib/dev-tools/git/hooks/pre-push.shHosted Verification
The workflow cannot be dispatched from this fork because
external-link-check.yamlis absent from its default branch. After merge, manually dispatch it from upstreamdevelopto confirm the report artifact is retained and the 424 C1/C9 diagnostics disappear while unrelated failures remain visible.Closes #2185
Related to #2003