CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

fix(nav-audit): the error detector flagged any number between 400 and 499 #5498

MergedXSccantynz wants to mergefix/nav-audit-flagged-any-number-400-to-499mainopened 20d ago
ccantynzcommented 20d ago

The bug

The body check was:

/>\s*4\d\d\s*</

That is not an error signature, it is arithmetic. It matches any three-digit number from 400–499 rendered between tags. On 2026-08-12 it failed seven live pages that were completely healthy:

page matched reality
/contributors <span class="contrib-num">446</span> a contributor with 446 commits
/coupling <span class="metric">487</span> a coupling score

Both returned HTTP 200, correct titles, complete content.

It would also have "fixed itself" invisibly once that contributor reached 500 commits — which is not a fix, it's the bug moving out of sample.

I reported these upward as a real defect ("two routes render an error at 200") before reading the markup. That's the cost being fixed here: a QA tool that produces confident false alarms is worse than no tool, because its true findings get discounted at the same rate.

The fix — intent-based, not cosmetic

A genuine error page identifies itself structurally, in both branches of src/views/error-page.tsx:

role="main" aria-labelledby="error-page-title"
<h1 id="error-page-title" class="err-title">
data-error-code="404"

The check now keys on that ARIA relationship. It matches what the page is, survives class renames and copy edits, and cannot be produced by rendering an integer.

It also catches strictly more than the digit test: a 200 with an innocuous title but the error template rendered is exactly the soft-error case the body check exists for — and the digit test only found those when the code happened to start with a 4.

Worth noting the digit test never caught a real error the two checks above it missed: every genuine error page carries a non-200 status and an error title. It contributed false positives only.

Why this file had no tests

main() ran at import and ends in process.exit(...), so importing the module killed the test run. It's now behind import.meta.main.

Verification

Re-run against prod with the fix:

before after
failures 18 12
rendering 1709/1727 1715/1727

The remaining 12 are 7 slow renders + 5 HTTP 429 from the audit's own request rate. Zero broken pages.

Tests use real markup captured from live responses, walk 400/404/418/446/487/499 so a reintroduction can't hide in the unsampled part of the range, and one test restyles the error page's classes to prove the locator survives a redesign. 8 pass, 0 fail. tsc --noEmit clean.

ccantynzAI Reviewcommented 20d ago

AI Triage

(no summary)

Priority: medium Risk area: mixed

Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)

Suggestions only — nothing has been applied. The PR author stays in control.

gluecron[bot]🤖 botAI Reviewcommented 20d ago

AI review unavailable

The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts