Skip to content

fix(lint-framework): coalesce lint requests dropped while one is in flight - #3912

Draft
rodbegbie wants to merge 1 commit into
Automattic:masterfrom
rodbegbie:fix/3911-lint-update-coalescing
Draft

fix(lint-framework): coalesce lint requests dropped while one is in flight#3912
rodbegbie wants to merge 1 commit into
Automattic:masterfrom
rodbegbie:fix/3911-lint-update-coalescing

Conversation

@rodbegbie

Copy link
Copy Markdown

Disclaimer: this patch and description were produced by a Claude Code
agent working interactively under @rodbegbie's direction, per the "Be Honest"
section of AGENT_POLICY.md. Every design decision was reviewed and approved
by a human, and the diff has been read line by line. The limitations in
"How Has This Been Tested?" are stated plainly rather than glossed.

Issues

Refs #3911 — deliberately not "fixes", see the testing caveat below.

Description

requestLintUpdate uses lintRequested as a single-flight mutex, but a
request arriving while a lint is in flight is discarded rather than queued.
Nothing re-schedules it. With the default delay of 0 there is no debounce to
coalesce those calls either, so the only recovery is the 1000 ms safety-net
timer in the constructor.

A burst of input can therefore leave the rendered lints belonging to a stale
prefix of the text. That is invisible on screen, because
remapLintToCurrentSource keeps the highlight correctly positioned. It is not
invisible to the ignore path: LintContext hashes the tokens following a
lint, so a lint computed against a prefix carries a context_hash that never
matches the one derived from the final text. Dismissing such a lint records a
hash matching nothing, the next lint pass returns the same lint unfiltered, and
the highlight comes back and stays.

This is the mechanism behind the flaky Can ignore suggestion Firefox test
described in #3911, and it should be reachable by a real user typing quickly on
a loaded machine and immediately clicking Dismiss.

The fix: track a lintDirty flag, and re-run once after the in-flight pass
releases the mutex. A boolean collapses a burst of N dropped requests into
exactly one follow-up lint, so there is no unbounded recursion — at most one
extra pass per completed pass.

Two details in the diff are load-bearing and easy to "tidy" back into bugs:

  • The re-run sits after the finally, not inside the try. Inside, it
    would fire while lintRequested were still true, hit the same guard, set
    lintDirty again and be dropped — reproducing this exact bug one level
    deeper. There is an inline comment saying so.
  • lintDirty is cleared when a pass starts, not only when it re-runs. A
    pass reads the text as it is at that moment, so it already covers every
    request made before it began; clearing at the start narrows the flag to mean
    precisely "input arrived while this pass was in flight".

The else if marks dirty only when lintRequested is true, so a request
dropped because targets.size === 0 does not schedule a pointless re-run —
addTarget already calls update().

Also included: the lint body is now wrapped in try/finally. Previously
a rejected lintProvider left lintRequested stuck at true, permanently
stopping all linting for the life of the page. That bug is pre-existing, but
the new flag would have inherited it, so fixing it separately would have left
this patch incomplete.

Note on diff size: GitHub shows 49 additions / 32 deletions, but most of that
is reindentation from wrapping the block in try. git diff -w shows 18
changed lines.

Demo

N/A — no user-visible UI change. The behavioural change is that a burst of
input now settles on a lint computed from the final text.

How Has This Been Tested?

Honestly: not as well as it should be, and I would rather say so than
overclaim.

  • cargo fmt --check clean, biome check clean, tsc --noEmit clean on
    packages/lint-framework.
  • All four paths traced by hand: burst typing converges on the final text; the
    immediate ignore path is untouched (it neither sets nor reads either flag);
    empty-targets correctly does not mark dirty; a rejected provider now releases
    the mutex instead of wedging it.

What is not done: there is no automated test proving the coalescing
behaviour. packages/lint-framework currently has no test harness at all
("test": "echo 'no tests'"), and the existing Playwright coverage is a poor
instrument here — the flaky test this addresses already passes roughly two runs
in three, so a green CI run is one sample from a mostly-green distribution and
proves very little either way.

A follow-up branch is planned to add vitest to lint-framework (following the
harper.js / obsidian-plugin precedent) with a deterministic test that drives
a fake lintProvider and asserts exactly one follow-up lint after a burst, plus
the condition-based settle wait that testCanIgnoreSuggestion needs. That was
kept out of this PR to keep it single-purpose.

Happy to fold that work in here instead if you would rather not merge this
without a test — just say.

AI Disclosure

  • I am a human and didn't use any AI.
  • I used LLM features of my editor, but not an agent.
  • I used an AI agent interactively.
  • I am an agent or I got an agent to do the work autonomously.

To be precise: an agent wrote the patch and this description, interactively,
with a human approving the design, the scope and the final diff. Not autonomous.

If Your PR Implements or Enhances a Linter

N/A — this changes lint scheduling in the browser framework, not a linter rule.

Checklist

  • I have performed a self-review of my own code
  • I have added tests to cover my changes — see above; no harness exists in
    this package yet, and a follow-up is planned
  • I have considered splitting this into smaller pull requests

…light

`requestLintUpdate` used `lintRequested` as a single-flight mutex, but a
request arriving while one was in flight was discarded rather than queued.
The default delay is 0, so there is no debounce to coalesce them either. A
burst of input could therefore leave the rendered lints belonging to a stale
prefix of the text, recovered only by the 1000ms safety-net timer.

That staleness is invisible on screen, because `remapLintToCurrentSource`
keeps the highlight correctly positioned. It is not invisible to the ignore
path: `LintContext` hashes the tokens following a lint, so a lint computed
against a prefix carries a `context_hash` that never matches the one derived
from the final text. Dismissing such a lint records a hash that matches
nothing, and the highlight comes straight back and stays.

Track a `lintDirty` flag and re-run once after the in-flight pass releases
the mutex.

Two placement details matter. The re-run must happen after that release, or
it hits the same guard and is dropped in turn. And it belongs inside the
`finally`, so that a pass which threw still hands off the input that arrived
while it was running -- otherwise a rejected lint strands exactly the work
this change exists to preserve, and recovery falls back to the 1000ms timer.

That `finally` also fixes a pre-existing bug: a rejected `lintProvider`
previously left `lintRequested` stuck at true, permanently stopping all
linting.

Refs Automattic#3911

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Entire-Checkpoint: ffb36eab0913
@rodbegbie
rodbegbie force-pushed the fix/3911-lint-update-coalescing branch from 2ae4e5f to 79c14c6 Compare July 29, 2026 05:08
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.

1 participant