fix(ci): drop a dead eslint directive and re-equal the warning ceiling - #7753
Conversation
UX Review (Fable 5) — ✅ PASSUX-level review of No user-facing surface in this PR — it changes a CI eslint ceiling and a comment in a build-time sprite generation script. No UI, strings, or screenshots to review. UX-Verdict: PASS No user-facing surface: the diff touches only a CI warning ceiling and a build-script comment — nothing a product user ever sees. [UX-REVIEWED] 998c4f8 |
Design Review (Fable 5) — ✅ PASSDesign-level review of Both hunks verified against the description; the numbers reconcile (598 measured on main, minus the unused-directive warning = 597, ceiling set to 597 with a one-lower failure probe). The rejected alternative (bringing Design-Verdict: PASS Ratchet slack removed and a dead suppression replaced with its rationale — measured, falsified one-lower, no design-level concerns. [DESIGN-REVIEWED] 998c4f8 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsNo findings. The single candidate rests on an unresolved conditional — whether the head tree measures 598 warnings (making the new [OPUS-REVIEWED] 998c4f8 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All verification is done. The change is a pure subtraction with every claim checked against the repo: the eslint config truly scopes every rule block to First-Principles-Verdict: PASS Every item deletes something — a dead suppression, two slots of silent admission, a stale task-log paragraph — and each deletion is mandated by an invariant the repo already wrote down. What this change shipsIntent: close the two-warning slack in the frontend lint ratchet and remove the dead directive that produced part of it. FIX.
No new surface, key, flag, or behavior branch ships, so there is nothing to consumer-count. Siblings of the root cause: grepped [FIRST-PRINCIPLES-REVIEWED] 998c4f8 |
1b36813 to
2ad6856
Compare
Two separate pieces of lint debt on `main`, both invisible to the gate:
1. `crew-ghost-sprite.gen.mjs` in `website/src/apps/issue-radar/`
carries an `eslint-disable-next-line no-eval` that ESLint reports as
unused. It always was: every rule block in `website/eslint.config.js`
is scoped to `src/**/*.{ts,tsx}`, so no rule is enabled for a `.mjs`
file and the directive could never suppress anything.
2. The ceiling sits above the measured count: `--max-warnings 599`
against 598 measured. That is precisely the state the gate's own
comment forbids -- "The ceiling must EQUAL the measured count, not
sit above it: slack is silent admission, and a warning that lands
inside it never surfaces again" -- and the free slot admits the next
real warning without ever failing a check.
Replaced the dead directive with a plain comment that keeps the intent,
why `eval` is safe in that hand-run generator, and records when the
directive should come back. Then re-measured and set the ceiling to the
count: 597, with `--max-warnings 597` exiting 0 and one lower failing,
so the gate carries no slack.
Also drops the transient half of the ceiling comment, which narrated an
earlier episode by commit SHA and PR number. Those numbers go wrong as
soon as the ceiling moves, which this change does, and AGENTS.md forbids
that shape in a comment. The EQUAL-the-count instruction and the command
that re-measures it stay verbatim.
Rejected alternative: adding a `src/**/*.mjs` rule block would make the
directive used and also drop the warning, and would extend lint coverage
to a file currently linted by nothing -- but it brings the generator's
four `console` calls in as `no-console` warnings, so it needs the
ceiling raised.
2ad6856 to
998c4f8
Compare
Disposition: fixed.
Legitimate and proportional, so it is fixed in the single commit. The rule is explicit
Both clauses are present-tense conditions that stay true independently of any past No test was added, because no test pins comment prose and inventing one would pin the |
Disposition: rebutted (the concern is real; the prescribed remedy is rejected).
The concern is correct and is exactly what this PR fixes for the current value. The But there is no "one-line equality assertion" available, and the repo already recorded
So the remedy is not a line, it is a new gate: a second The First Principles lane reached the same conclusion independently on this same head:
The deferral is now stated explicitly in the PR body's Pattern harvest rather than left |
Disposition: rebutted as out of scope for this change, not as unsound.
Correcting the record first: the promotion would be green today, not red. Measured on It is still declined here, for scope. It changes the severity contract for all of The mechanism is recorded as the named rule candidate in the PR body's Pattern harvest, so
|
chenmingwei23
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: chore/build (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: CI plumbing - lowers eslint --max-warnings ceiling to the measured count and removes a dead eslint-disable directive in a .mjs file (no rule enabled there); no runtime impact.
iamwhatever
left a comment
There was a problem hiding this comment.
Tier 1 auto-approve: chore / build tooling (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: CI plumbing only -- removes an inert eslint-disable-next-line no-eval from a .mjs build-time generator (eslint.config.js scopes every rule to src/**/*.{ts,tsx}, so the directive itself was charged against the ceiling) and lowers --max-warnings 599 -> 597 to re-equal the measured count. The eval(program) call is unchanged and the ceiling moves down, not up.
Problem / Motivation
Two pieces of frontend lint debt on
main, both invisible to the gate that is supposedto catch them.
1. A dead eslint directive.
2. The ceiling sits above the measured count. On current
mainthe gate reads--max-warnings 599whilecd website && npx eslint src/measures 598 — one freeslot. That is exactly the state the gate's own comment forbids:
A free slot means the next real warning lands without ever failing a check.
Why it matters
The ratchet only works while the ceiling tracks the count. With slack in it, the check
reports green through the first regressions and the ratchet silently stops being one —
and the ceiling is the only thing standing between the tree and unbounded warning
growth, since none of these rules is an error.
The dead directive is the smaller half, but it is also the more misleading one: it reads
as a deliberate, load-bearing suppression, and it suppresses nothing.
What changed (motivation → approach → change)
The directive was never functional. Every rule block in
website/eslint.config.jsisscoped to
src/**/*.{ts,tsx}, so no rule is enabled for a.mjsfile —no-evalincluded — and the directive could never suppress anything. Replaced it with a plain
comment that keeps the intent (why
evalis safe in that generator:programis atemplate literal built in the same file and handed to the Playwright page so the browser
context can call
drawCrewGhost— nothing remote or user-supplied) and records thecondition under which the directive should come back.
Then re-measured and set the ceiling to the count, 599 → 597, which is the
direction the comment mandates. It is not a raise: it is lower than both the old ceiling
and the count before this change. 597 is measured, not derived —
--max-warnings 597exits 0 and
--max-warnings 596fails.The ceiling comment loses its transient half. The paragraph above the
run:linenarrated a specific earlier 604/603/599 episode, complete with a commit SHA and three PR
numbers. Those numbers are wrong the moment the ceiling moves — which this PR does — and
the root
AGENTS.mdcomment rule forbids that shape outright ("NOT a task log: no PR/CRnumbers … or commit SHAs"). The durable half, the EQUAL-the-count instruction and how to
re-measure, stays verbatim.
Rejected alternative. Adding a
src/**/*.mjsrule block would make the directiveused and also drop the warning — and would extend lint coverage to a file currently
linted by nothing. But it brings the generator's four
consolecalls in asno-consolewarnings, so it needs the ceiling raised, which the comment forbids. Probed and
reverted.
Tests
test/test_eslint_warning_ceiling.pyalready pins the two things that can be pinnedwithout running eslint: that
ci.ymldeclares exactly ONE--max-warningsliteral, andthat no prose transcribes its value. Neither can measure the count, and the module's own
docstring says so — "Only running eslint can measure that, so it is not pinned here --
the gate itself is that test." That gate,
npx eslint src/ --max-warnings 597, is theassertion for this change: with the ceiling at the count, the next added warning fails
it.
Manual verification
Ran the frontend job's gates verbatim from
ci.yml:origin/mainnpx eslint src/598 problems (0 errors, 598 warnings)597 problems (0 errors, 597 warnings)npx eslint src/ --max-warnings 597npx eslint src/ --max-warnings 596npx tsc -bnpx jscpd ..github/workflows/ci.ymlparses as YAML, andtest/test_eslint_warning_ceiling.py test/test_security_posture.pyare green (49 passed)— including
TestGateSideLogRedactorSpelling, whose shard-3 red on the previous head wasthe inherited baseline-redactor census cascade and is cured by the rebase, with no code
change.
The generator is not itself generated — its header documents running it by hand — so
nothing regenerates the removed line back.
Screenshots / video
Why no screenshot: the only
website/srcchange is a comment insidecrew-ghost-sprite.gen.mjs, a hand-run Node generator that is never bundled orrendered. No component, layout, style, or user-visible string changes, and the committed
sprite sheet it produces is byte-identical.
Related Issues
no linked issue: lint-count drift against the gate's own ceiling rule; there is no
tracked issue for it.
Pattern harvest
Rule candidate: lint
Pattern: an
eslint-disabledirective in a file that no config block covers is silentlydead — it suppresses nothing, and the unused-directive warning it produces is charged to
the
--max-warningsratchet instead of failing where it was written. Every rule block inwebsite/eslint.config.jsis scoped tosrc/**/*.{ts,tsx}, so any directive in asrc/**/*.{js,mjs,cjs}file is in this class. Promoting unused disable directives fromwarntoerror(linterOptions.reportUnusedDisableDirectives) would surface the nextone where it is written rather than months later as an unattributable ratchet overflow.
Second, narrower candidate for whoever owns the gate: the ceiling and the count can only
drift apart silently, because a lower count never fails anything. A CI step asserting
equality — rather than
<=— would catch each slot at the moment it opens. Both areadditions to the gate rather than parts of this fix, so both are deferred here.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)