perf(linter): use batch API for semantic diagnostics#844
Conversation
How to use the Graphite Merge QueueAdd the label 0-merge to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Pull request overview
This PR optimizes type-checking performance in the linter by switching semantic diagnostic collection from per-file calls to a single batched call, aligning with the performance goals in issue #843.
Changes:
- Split syntactic vs semantic type error reporting into separate paths.
- Fetch semantic diagnostics in bulk, then apply per-file no-emit filtering, include-processor diagnostics, and sort/dedup before emitting.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
camc314
force-pushed
the
c/03-27-perf_linter_use_batch_api_for_semantic_diagnostics
branch
from
ea433d3 to
aa3dcc5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
camc314
force-pushed
the
c/03-27-perf_linter_use_batch_api_for_semantic_diagnostics
branch
from
12228bb to
d0578d1
Compare
Merge activity
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should report syntactic issues', async () => { | ||
| const testFiles = await getTestFiles('semantic-syntactic-diagnostics'); | ||
| expect(testFiles.length).toBeGreaterThan(0); | ||
|
|
||
| const config = generateConfig(testFiles, ALL_RULES, { | ||
| reportSemantic: true, | ||
| reportSyntactic: true, | ||
| }); |
There was a problem hiding this comment.
This test enables both reportSemantic and reportSyntactic and the fixture contains both a syntactic error (a.ts) and a semantic type error (b.mts), but the test name says only “syntactic issues”. Consider renaming the test to reflect that it asserts both diagnostic kinds to avoid confusion when diagnosing failures.
Before/after  Related to #843
graphite-app
Bot
force-pushed
the
c/03-27-perf_linter_use_batch_api_for_semantic_diagnostics
branch
from
d0578d1 to
803b9de
Compare
graphite-app
Bot
deleted the
c/03-27-perf_linter_use_batch_api_for_semantic_diagnostics
branch
Before/after
Related to #843