Skip to content

Fix format-on-save to avoid unnecessary oxlint requests#310

Open
kurtextrem wants to merge 7 commits into
oxc-project:mainfrom
kurtextrem:feature/faster-ctrl-s
Open

Fix format-on-save to avoid unnecessary oxlint requests#310
kurtextrem wants to merge 7 commits into
oxc-project:mainfrom
kurtextrem:feature/faster-ctrl-s

Conversation

@kurtextrem

@kurtextrem kurtextrem commented Jun 17, 2026

Copy link
Copy Markdown

Summary

This PR tightens code-action routing so the extension does not ask oxlint for code actions when the request cannot produce an oxc lint fix.

Concretely:

  1. oxlint code actions are skipped for empty automatic unscoped probes (triggerKind = Automatic, only = undefined, no diagnostics)
  2. diagnostic-bearing automatic unscoped requests are preserved so VS Code can still discover oxlint quick fixes for the lightbulb
  3. automatic save-runnable source requests (source, source.fixAll, source.fixAll.oxc) are skipped unless save settings enable oxlint through source, source.fixAll, or source.fixAll.oxc
  4. more-specific save settings override broader ones, so source.fixAll.oxc: "never" opts oxlint out even when source or source.fixAll is enabled
  5. oxlint source-action routing now only accepts exact oxlint-owned source actions (source.fixAll and source.fixAll.oxc), not provider-specific namespaces like source.fixAll.biome
  6. oxfmt's source.format.oxc code action stays separate from formatter-on-save and no longer participates in broad source-action scans

For the reported format-only save setup:

"editor.codeActionsOnSave": {},
"editor.formatOnSave": true

stock VS Code should run the formatter provider only for the save. While testing this path, we saw VS Code also issue empty automatic unscoped code-action probes around the editor flow. Those probes are not useful for oxlint because there are no diagnostics to fix, and they can add avoidable LSP traffic while type-aware linting is already busy. The extension now drops those empty probes before they reach the oxlint LSP.

The narrower guard is intentional: if VS Code sends an automatic unscoped request with diagnostics, we still forward it so users keep oxlint quick-fix lightbulbs.

The PR also covers these save-action combinations:

"editor.codeActionsOnSave": {
  "source.fixAll.biome": "explicit"
},
"editor.defaultFormatter": "oxc.oxc-vscode",
"editor.formatOnSave": true

VS Code still asks the Biome provider for its awaited save action, oxfmt still formats the file, and oxlint receives no code-action request.

"editor.codeActionsOnSave": {
  "source.fixAll": "explicit",
  "source.fixAll.oxc": "never"
},
"editor.defaultFormatter": "oxc.oxc-vscode",
"editor.formatOnSave": true

VS Code can issue the save request as context.only === source.fixAll; the extension now still consults the save settings for that request and respects the oxlint-specific opt-out before forwarding to oxlint.

Testing

  • Added unit tests for linter code-action filtering, including empty automatic unscoped requests, diagnostic-bearing automatic unscoped requests, broad source save settings, setting opt-out precedence, and unrelated provider-specific fix-all kinds.
  • Extended the format-save integration test with fake oxfmt plus a fake slow oxlint server.
  • Added a fake Biome source.fixAll.biome provider in the integration test to verify that Biome's save action can run without routing any oxlint code action.
  • Added a fake generic source.fixAll provider in the integration test to verify that source.fixAll.oxc: "never" prevents oxlint from receiving a save code-action request even when generic fix-all runs.

Local verification:

  • pnpm run fmt:check
  • pnpm run test:unit
  • pnpm run test:format-save

@Sysix

Sysix commented Jun 17, 2026

Copy link
Copy Markdown
Member

hello and thanks for this PR!

Can you explain a bit more when exactly we skip the oxlint code actions? We already check on oxlint side the CodeActionTriggerKind.Automatic kind and will skip the diagnostic.
Maybe we can extend the logic on oxlint side, so every editor can benefit from this.

Happy to hear your thoughts :)

@kurtextrem

Copy link
Copy Markdown
Author

@Sysix

We skip before sending the request to oxlint in these cases:

  1. When context.only is for a kind oxlint does not provide, e.g. source.format, source.organizeImports, or broad formatter source-action scans.
  2. The request is CodeActionTriggerKind.Automatic with only === source, unless VS Code’s editor.codeActionsOnSave explicitly enables source.fixAll or source.fixAll.oxc.

We still forward unscoped/manual requests, quick fixes, explicit source.fixAll.oxc, and automatic broad source requests when the save settings actually ask for oxlint fix-all.

The oxlint-side Automatic check is still useful, but it only helps after a textDocument/codeAction request already reached the server. This extension-side guard avoids the request/roundtrip entirely, which is what the save-latency bug needs. In the exact reported setup:

"editor.codeActionsOnSave": {},
"editor.formatOnSave": true

pressing save should only invoke oxfmt formatting; oxlint should not receive code-action or diagnostic requests from that save path.

That said, I agree it would be good to also add a protocol-level guard in oxlint so other editors benefit. I think the oxlint server can cheaply return [] when context.only does not intersect oxlint’s supported actions.
The one thing it cannot know is VS Code’s editor.codeActionsOnSave value, so the VS Code-specific “no save action configured” decision still belongs in the extension (this PR). So I’d keep this guard here and consider a follow-up oxlint change for the generic context.only filtering.

@Sysix

Sysix commented Jun 18, 2026

Copy link
Copy Markdown
Member

Thank you for the detail, I agree with you that the request should be skipped if possible.
Sadly I am not sure if this is the right gear to turn, I need to check this WE deeply about this problem :)
I would think this is a vscode-languageclient bug. The server is already telling the extension which code actions he supports. So a code action should not be sent to a server, which does not support it.

but it only helps after a textDocument/codeAction request already reached the server. This extension-side guard avoids the request/roundtrip entirely, which is what the save-latency bug needs.

When the oxlint server gets CodeActionTriggerKind.Automatic it will fast-return no results, no internal lint process is started. I am not sure if this is really the desired fix. The code actions are returned in 1ms delay on my machine.

Thank you again. Let me check this deeply the next day. If you have any feedback to help me, I would appreciate it 🙇

@kurtextrem

Copy link
Copy Markdown
Author

Hmm, looks like I tested wrong indeed, thank you for the correction. Reaching out to the LSP is also for me fast (median 0.14ms for a dummy small file). I'll keep digging!

Regarding the trigger action:

  • on e.g. only: source.format, the client/langclient should ideally avoid sending the request based on the server’s advertised codeActionKinds
  • oxlint could cheaply return [] when context.only is present and does not intersect the actions oxlint actually provides. That would help every editor.
  • The request we need to guard is the broad automatic only: source case. Since oxlint advertises source.fixAll.oxc, a broad source request does intersect oxlint’s supported actions, so based on that alone we cannot tell whether this save actually wanted oxlint.

With a config like mine:

"editor.codeActionsOnSave": {},
"editor.formatOnSave": true

a save is only asking for formatting. The oxlint server does not receive editor.codeActionsOnSave, so the extension is the only place that can decide that a broad automatic source request should not be forwarded to oxlint unless source.fixAll or source.fixAll.oxc is explicitly enabled. Hence my change in this PR.

@kurtextrem

kurtextrem commented Jun 19, 2026

Copy link
Copy Markdown
Author

Aha, I think I found something: A small typescript/no-floating-promises LSP diagnostic pull was hundreds of ms, with a cold run around ~1s locally.

I might be related to queueing: If I send textDocument/diagnostic and then a broad automatic textDocument/codeAction while the diagnostic is still running, the code-action response time matches the diagnostic time. In one local run, diagnostic was ~304ms and the code-action response was also ~304ms, even though that same code-action path is sub-ms when isolated (as mentioned earlier).

So I think the trace I posted in the issue can be explained like this:

  • oxlint is not necessarily spending 1s computing code actions;
  • the code-action request can be delayed behind heavier lint/diagnostic work in the same server
  • if a format-only save triggers or waits on such a code-action request, save latency can look like “code actions are slow”.

So the question now is, does this PR fix that

@kurtextrem

Copy link
Copy Markdown
Author

So for oxc, I think 2 things can be done:

  1. Most important: cheap code-action requests should not queue behind type-aware diagnostics
  2. return [] when context.only is present and does not intersect the actions oxlint actually provides

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbee75dbd5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread client/tools/linter.ts Outdated
@kurtextrem kurtextrem marked this pull request as draft June 19, 2026 08:20
@kurtextrem

Copy link
Copy Markdown
Author

I found the cause!

This:

"editor.codeActionsOnSave": {
  "source.fixAll.biome": "explicit"
},
"editor.defaultFormatter": "oxc.oxc-vscode",
"editor.formatOnSave": true

caused VS Code to involve the oxlint LSP during save, even though only oxfmt + Biome fix-all were configured.

Before this PR, the extension forwarded some of those automatic/unscoped requests to oxlint. On small files that is usually invisible, but with type-aware linting on a large TS file oxlint can already be busy with diagnostics/tsgolint work. Then even a normally cheap code-action request can sit behind that work and make Ctrl+S slow.

With the PR: if the save request is not explicitly for oxlint fix-all (source.fixAll / source.fixAll.oxc) or a real user-invoked quick fix, we now drop it before it reaches the oxlint LSP. The integration test covers this through a fake oxfmt, a fake Biome source.fixAll.biome provider, and a fake slow oxlint server. It verifies that oxfmt still formats, Biome still gets its save action, and oxlint receives no code-action request.

@kurtextrem kurtextrem marked this pull request as ready for review June 19, 2026 09:15

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27f95a6b9c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread client/tools/linter.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de3f0b9d10

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread client/tools/linter.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d9c386d0f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread client/tools/linter.ts Outdated
@Sysix

Sysix commented Jun 21, 2026

Copy link
Copy Markdown
Member

Thank you for the detail research why the code action does take longer then expected.
When textDocument/diagnostic response is blocking the textDocument/codeActions request, the core problem is on the oxlint side and not here on the extension.
VS Code will send another textDocument/codeAction request when the server is responding with a diagnostic.
I will search which lock on the server side is responsible for it.

Until now, I would like to keep the PR open, lowering the requests can be better, but I do not want to more bloat the extension for workarounds.

PS: Your new tests are not included in CI.

@Sysix

Sysix commented Jun 24, 2026

Copy link
Copy Markdown
Member

Want to come back to you. I could identify the exact problem with a test setup in oxc-project/oxc#23694.
The right lock is found too, which needs some time until getting ready.

I would like to wait for a release with the fix in oxc-project/oxc#23768. Maybe we do not need to touch the editor side at all🤞

Thank you again for your deep research and pointing out what went wrong.

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