Skip to content

feat(ci): add PR check for underscores in URL paths - #704

Merged
fredericsimard merged 2 commits into
mainfrom
feat/pr-underscore-check/2026-08-13
Aug 14, 2026
Merged

feat(ci): add PR check for underscores in URL paths#704
fredericsimard merged 2 commits into
mainfrom
feat/pr-underscore-check/2026-08-13

Conversation

@fredericsimard

Copy link
Copy Markdown
Contributor

What

Adds a standalone PR check that fails when a pull request introduces an underscore into a path that becomes a public URL:

  • Link targets on lines the PR adds or modifies in Markdown under docs/ — Markdown inline links, reference definitions, and raw HTML href/src.
  • Paths of files the PR adds under docs/ or overrides/ (photos, scripts, stylesheets, new pages).

Two new files, plus a one-line change to the existing review poster:

File Change
scripts/ci/check-underscores.py New. Stdlib-only checker
.github/workflows/pr-underscore-check.yml New. Its own workflow and check run
scripts/ci/post-review.py Review summary text now overridable via REVIEW_BODY, defaulting to the current wording

Why

The site's Nginx config carries an unconditional redirect:

rewrite ^/(.*)_(.*)$ /$1-$2 permanent;

Any request whose path contains an underscore is 301'd to the hyphenated equivalent, whether or not the underscored file exists. Consequences:

  • A file added with an underscore in its path is unreachable by construction — every request for it arrives at the origin hyphenated, finds nothing, and 404s.
  • A link with an underscore costs a 301 round-trip and breaks outright when no hyphenated target exists.

The convention dates back to Google advising the project to move the site off underscores for search ranking; the redirect was the practical way to do that at this site's size. This check stops new content from adding paths that depend on the redirect, or that the redirect makes permanently unreachable.

Scope and exemptions

Diff-only, with no grandfathering — pre-existing underscored paths are untouched, but a new line may not introduce one. Modified files are excluded from the path check, since their path already existed.

Exemptions follow the mechanism rather than taste; each is something the rewrite cannot reach:

  • Anchor fragments (](#stop_timestxt)) — a fragment never leaves the browser. These are GTFS file and field names, underscored by the spec and unrenameable.
  • Query strings — Nginx matches rewrite against the decoded URI and reattaches the query untouched.
  • External URLs and mailto: — another host serves those.
  • Everything on lines the PR did not touch.

Reviewer notes

  • Not a required status check, matching pr-syntax-check.yml: it shows a red X and inline comments without blocking merge. Say the word if it should be required.
  • Reporting mirrors the syntax check — Actions error annotations on the offending line (fork PRs get these too) plus a findings JSON consumed by the review-posting and enforce steps. Findings carry no suggestion block, because swapping _ for - would point at a page that does not exist yet.
  • Stdlib-only, so there is no pip install step and no coupling to requirements.txt.
  • The post-review.py change is additive: without REVIEW_BODY set, the syntax check posts exactly the same text as before.
  • Verified locally against a synthetic fixture commit: correct hits with accurate line and column, and no false positives across anchor, cross-page anchor, external URL, query-string and mailto: cases. This PR adds no underscored paths, so the check should come back green on itself.

🤖 Generated with Claude Code

fredericsimard and others added 2 commits August 13, 2026 18:22
The site's Nginx config carries an unconditional redirect:

    rewrite ^/(.*)_(.*)$ /$1-$2 permanent;

Any request whose path contains an underscore is 301'd to the hyphenated
equivalent whether or not the underscored file exists. A file added with an
underscore in its path is therefore unreachable by construction: every request
for it arrives at the origin hyphenated, finds nothing, and 404s. A link with
an underscore costs a 301 round-trip and breaks outright when no hyphenated
target exists.

This adds a standalone check that catches both cases before merge:

- Link targets on lines the PR adds or modifies in Markdown under docs/,
  covering Markdown inline and reference links plus raw HTML href/src.
- Paths of files the PR adds under docs/ or overrides/.

Scoped to the diff, with no grandfathering: pre-existing underscored paths are
untouched, but a new line may not introduce one. Modified files are excluded
from the path check, since their path already existed.

Exemptions follow the mechanism rather than taste — each is something the
rewrite cannot reach:

- Anchor fragments (#stop_timestxt). A fragment never leaves the browser.
  These are GTFS file and field names, underscored by the spec.
- Query strings. Nginx matches rewrite against the decoded URI and reattaches
  the query untouched.
- External URLs and mailto:. Another host serves those.

Reporting mirrors the syntax check: GitHub Actions error annotations on the
offending line (which fork PRs receive too) plus a findings JSON consumed by
the review-posting and enforce steps. Findings carry no suggestion block —
swapping _ for - would point at a page that does not exist yet, so the repair
is a human decision.

The checker is stdlib-only, so this check needs no pip install step and stays
independent of requirements.txt. Like the syntax check, it is deliberately not
a required status check: it makes the problem visible without blocking merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
post-review.py hardcoded a summary body announcing a syntax check, which is
wrong for any other check reusing it. Read REVIEW_BODY from the environment
instead, defaulting to the existing wording so the syntax check is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@fredericsimard fredericsimard self-assigned this Aug 13, 2026
@fredericsimard
fredericsimard merged commit 9968ed7 into main Aug 14, 2026
2 checks passed
@fredericsimard
fredericsimard deleted the feat/pr-underscore-check/2026-08-13 branch August 14, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants