Skip to content

fix(ci): burn one eslint warning down to the ratchet ceiling - #7480

Merged
bolichen97 merged 1 commit into
mainfrom
fix/eslint-ratchet-burndown
Sep 1, 2026
Merged

fix(ci): burn one eslint warning down to the ratchet ceiling#7480
bolichen97 merged 1 commit into
mainfrom
fix/eslint-ratchet-burndown

Conversation

@chenmingwei23

Copy link
Copy Markdown
Contributor

Problem / Motivation

The Frontend Lint gate runs npx eslint src/ --max-warnings 659. Main currently
measures 660 warnings, 0 errors, so the lane fails on every PR whose changed
surface reaches it -- for reasons that have nothing to do with that PR's diff.

Measured on dd9e002bf (main tip at the time of writing):

$ npx eslint src/
660 problems (0 errors, 660 warnings)

The same count appears in CI. Observed today: #7462
passed this lane at 03:41Z, and PRs whose surface reached it after that failed
with exactly this 660-vs-659 signature.

Why it matters

It is a false red on unrelated work, and a costly one to diagnose: the failing
PR's author has to prove the warnings are not theirs before they can ignore it
(CI lints the merge with main, so the linted frontend tree is main's even for a
backend-only diff). Meanwhile the readiness gate counts the lane as a failure,
so the PR cannot go green, and /ai-review override does not apply because this
is a CI lane rather than a review bot.

Most PRs skip the lane, which is why the drift went unnoticed -- it only bites
the occasional PR whose changed surface pulls the frontend in.

What changed (motivation -> approach -> change)

The ceiling is documented as something to burn DOWN, not raise: "The ceiling must
EQUAL the measured count, not sit above it: slack is silent admission ... do not
raise it." So the fix is to remove one real warning, not to touch the gate.

Removed an unused type ReactNode import from
website/src/test/useQueuedMessageActions.test.tsx. It is the smallest true
warning available: a test file, no product code, no behaviour, and the symbol is
referenced nowhere else in the file (grep ReactNode returns only the import).

Deliberately ONE warning, not a batch: it restores the gate with the smallest
reviewable diff, and it leaves the remaining 659 for a burn-down that can be
judged on its own merits. eslint reports 8 of them as --fix-able if someone
wants to take a bite.

Tests

npx vitest run src/test/useQueuedMessageActions.test.tsx -- 17 tests pass,
unchanged.

Gate verification, the exact CI command:

$ npx eslint src/
659 problems (0 errors, 659 warnings)
$ npx eslint src/ --max-warnings 659 && echo ok
ok

Manual verification

N/A -- a removed unused type import has no runtime surface. The measurement above
is the verification.

Related Issues

None filed; found while diagnosing a red Frontend Lint lane on an unrelated
backend PR (#7435), which
carries the full evidence trail.

Pattern harvest

Rule candidate: ci.
Pattern: "a ratchet whose gate is SKIPPED for most diffs stops being a ratchet --
it becomes a landmine for whoever's changed surface happens to reach it."

The count can only drift upward while the lane is skipped, and the drift is then
charged to an unrelated PR, which is the expensive part: the failing author must
first prove the warnings are not theirs. Two mechanical options, both cheap:

  • Run the measurement (not the whole lane) on every push to main and fail there,
    so the commit that adds warning 660 is the one that gets the red.
  • Or have the gate report the delta it attributes to the PR's own changed files,
    so a backend-only diff reads "0 added by this PR" instead of a bare 660 > 659.

Either turns this class from "diagnose it on someone else's PR" into "the author
who added it sees it".

@chenmingwei23
chenmingwei23 requested a review from a team as a code owner September 1, 2026 04:17
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

UX-level review of ee9aa938378b0663526779e69184e20c0941da52 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The entire diff is a single removed unused type import in a test file — no user-facing surface, no strings, no rendered UI. Clean pass.

UX-Verdict: PASS

One-line unused-import removal in a test file; nothing a user can see, read, or interact with changes.

[UX-REVIEWED] ee9aa93

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of ee9aa938378b0663526779e69184e20c0941da52 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Smallest true fix to the drifted ratchet, correctly deferring the systemic skip-lane gap to a named follow-up.

Suggestions

  • File the "measure on every push to main" follow-up from the pattern-harvest section as an issue now — the drift will recur while the lane stays path-filtered, and the harvest text is the only record.

[DESIGN-REVIEWED] ee9aa93

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed ee9aa938378b0663526779e69184e20c0941da52 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] ee9aa93

Verdict parsed from the review's SHA-scoped output markers for commit ee9aa938378b0663526779e69184e20c0941da52.

False positive or not applicable? A repository writer can comment:
/ai-review override fable ee9aa938378b0663526779e69184e20c0941da52: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of ee9aa938378b0663526779e69184e20c0941da52 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] ee9aa93

False positive or not applicable? A repository writer can comment:
/ai-review override gpt ee9aa938378b0663526779e69184e20c0941da52: <one-sentence reason>

@chenmingwei23

Copy link
Copy Markdown
Contributor Author

Status: the lane this PR exists for is green -- Frontend Lint & Type Check passes on 6d00bed0c, confirming the measurement (660 -> 659, ceiling 659).

The one remaining red is a SECOND main-owned ratchet drift, unrelated to this diff and already being fixed elsewhere:

test/test_security_posture.py::TestGateSideLogRedactorSpelling::test_the_census_holds_no_slack
AssertionError: `_BASELINE_LOG_SITE_CENSUS` is now looser than the code
  -- lower or drop these: dashboard/handlers/files.py: 1 sites, census says 3

This diff touches one file, website/src/test/useQueuedMessageActions.test.tsx, so it cannot move a Python log-site count. #7492 ("re-measure the files.py log-site census after #7293") is the fix; nothing to do here but wait for it.

Worth noting the two drifts are the same failure class from opposite directions -- a one-way ratchet that nobody re-measures when the code moves under it. The eslint ceiling drifted UP past its count; the log-site census stayed ABOVE a count that dropped. This PR's pattern-harvest note argues for measuring on push to main so the commit that moves the number is the one that gets the red; #7492's case is the same argument with the sign flipped, and the census test already says so in its own comment ("Without this the census only ever ratchets one way").

The Frontend Lint gate runs `npx eslint src/ --max-warnings 659`, and main
currently measures 660 warnings with 0 errors, so the lane fails on every PR
whose changed surface reaches it. The ceiling is meant to EQUAL the measured
count, and the workflow's own comment says to burn the warning down rather than
raise it.

Removes an unused `type ReactNode` import from a test file: the smallest true
warning in the list, no product code, no behaviour. Measured 660 -> 659, and
`npx eslint src/ --max-warnings 659` now exits 0.
@chenmingwei23
chenmingwei23 force-pushed the fix/eslint-ratchet-burndown branch from 6d00bed to ee9aa93 Compare September 1, 2026 05:14
@chenmingwei23

Copy link
Copy Markdown
Contributor Author

Rebased onto ecab0babe (ee9aa9383), now that #7492 has landed and main's files.py census reads 1.

Rebased rather than re-run: gh run rerun replays the merge ref computed when the run started, which still carried the broken census, so the three shard reds would simply come back. A fresh head re-computes the merge against the fixed main and re-rolls every lane, which also gives the one genuine flake on the previous head a clean roll -- test_auto_research.py::TestWatchdogStopTombstone::test_settlement_failure_is_logged_without_replacing_cancellation, a Windows TimeoutError with RuntimeError: Event loop is closed alongside 18,694 passes.

Re-measured against the new main before pushing, since main has moved a lot today and the burn-down would be pointless if the count had already come down:

main:   660 problems (0 errors, 660 warnings)
branch: 659 problems (0 errors, 659 warnings)
$ eslint src/ --max-warnings 659 && echo ok
ok

So the drift is still live at 660 and this one-line removal still lands exactly on the ceiling. The diff is unchanged: one unused type ReactNode import in a test file.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 1, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) September 1, 2026 05:46
@bolichen97
bolichen97 merged commit 905ada3 into main Sep 1, 2026
105 of 108 checks passed
@bolichen97
bolichen97 deleted the fix/eslint-ratchet-burndown branch September 1, 2026 05:46
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 1, 2026
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