fix(canon): map batch diagnostics through exact-expression sub-spans#3201
Merged
Conversation
The batch source map used first-match lookup with a linear delta clamped to the mapping end, so a diagnostic on a synthetic prop-check identifier always landed on the same generated column regardless of the authored content — vize check reported TS2322 for :msg="42" at the closing quote while the language server pointed at the bound expression. The sub-span-aware arithmetic now lives in vize_canon::virtual_ts:: mapping, shared verbatim by the language server, the corsa bridge service, and the batch checker: the narrowest mapping wins, an exact-expression sub-span beats its enclosing statement mapping, and clamping keeps every position inside the authored bytes.
ubugeeei
enabled auto-merge (squash)
July 21, 2026 12:29
📝 WalkthroughWalkthroughShared virtual-TypeScript mapping helpers now handle sub-spans, nested ranges, clamping, and diagnostic boundaries. Canon source maps, type checking, and Maestro Corsa diagnostics use the shared range-mapping implementation, with expanded tests. ChangesGenerated-to-source mapping
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Corsa
participant import_source_map
participant map_generated_range_to_source
participant SFC
Corsa->>import_source_map: Convert diagnostic range to generated byte offsets
import_source_map->>map_generated_range_to_source: Map generated start and end
map_generated_range_to_source->>SFC: Return authored source byte range
SFC-->>Corsa: Convert authored bytes to source positions
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
vize
@vizejs/fresco
@vizejs/musea-mcp-server
oxlint-plugin-vize
@vizejs/rspack-plugin
@vizejs/unplugin
@vizejs/vite-plugin
@vizejs/vite-plugin-musea
@vizejs/musea-nuxt
@vizejs/nuxt
@vizeui/core
commit: |
Contributor
PR BenchmarkBase:
Raw run timesCompile SFC
Lint
Format
Type check (1T)
Type check (max)
|
Contributor
Detailed Test ReportCommit: Area Summary
Test InventoryTotal tracked cases: 9475 across 1465 files.
Files
Comment truncated at 64000 characters. Open the workflow run for the full job log. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vize_canon/src/virtual_ts/mapping.rs`:
- Around line 74-91: Add a unit test covering the unwrap_or_else fallback in
map_generated_range_to_source when end_offset is outside every mapping’s
gen_range. Use a mapping where start_offset resolves, choose an unmapped
end_offset, and assert the generated-length estimate is clamped to the mapping’s
src_range end.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c72264ff-185c-44f4-9cd9-b0fe88331237
📒 Files selected for processing (5)
crates/vize_canon/src/batch/source_map.rscrates/vize_canon/src/typecheck_service.rscrates/vize_canon/src/virtual_ts.rscrates/vize_canon/src/virtual_ts/mapping.rscrates/vize_maestro/src/ide/diagnostics/corsa/mapping.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: tool-benchmark
- GitHub Check: vue-parity
- GitHub Check: app-readiness
- GitHub Check: clippy-and-test
- GitHub Check: pr-benchmark
- GitHub Check: test-js-packages
- GitHub Check: test-scripts
- GitHub Check: dialect-guard
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (1)
crates/**
⚙️ CodeRabbit configuration file
crates/**: Focus on parser/compiler correctness, source locations, UTF-8/UTF-16 offset handling, and panic-free LSP behavior. Flag changes that only work for small fixtures but break real Vue/Nuxt projects.
Files:
crates/vize_canon/src/virtual_ts.rscrates/vize_canon/src/batch/source_map.rscrates/vize_canon/src/typecheck_service.rscrates/vize_canon/src/virtual_ts/mapping.rscrates/vize_maestro/src/ide/diagnostics/corsa/mapping.rs
🔇 Additional comments (7)
crates/vize_canon/src/virtual_ts.rs (1)
20-20: LGTM!crates/vize_canon/src/virtual_ts/mapping.rs (3)
1-23: LGTM!Also applies to: 98-139
46-67: 🎯 Functional Correctness
map_generated_offset_to_sourceis fine as-is.VizeMapping.sub_spansis populated with at most one entry per mapping, so there’s no overlapping sub-span to resolve here.> Likely an incorrect or invalid review comment.
29-39: 🎯 Functional CorrectnessNo issue: the end offset is meant to fall back to the narrowest mapping at that position.
mapping_for_end_offsetonly reusesstart_mappingwhile the end stays inside it; otherwisemap_generated_range_to_sourceresolves the end independently, and the boundary behavior is already covered by tests.> Likely an incorrect or invalid review comment.crates/vize_canon/src/batch/source_map.rs (1)
47-70: LGTM!Also applies to: 262-333
crates/vize_canon/src/typecheck_service.rs (1)
388-420: LGTM!crates/vize_maestro/src/ide/diagnostics/corsa/mapping.rs (1)
8-35: LGTM!Also applies to: 100-169
Comment on lines
+74
to
+91
| pub fn map_generated_range_to_source( | ||
| mappings: &[VizeMapping], | ||
| start_offset: usize, | ||
| end_offset: usize, | ||
| ) -> Option<(usize, usize)> { | ||
| let start_mapping = mapping_for_generated_offset(mappings, start_offset)?; | ||
| let src_start = map_generated_offset_to_source(start_mapping, start_offset); | ||
| let src_end = mapping_for_end_offset(mappings, start_mapping, end_offset) | ||
| .map(|mapping| map_generated_offset_to_source(mapping, end_offset)) | ||
| .unwrap_or_else(|| { | ||
| let generated_len = end_offset.saturating_sub(start_offset); | ||
| src_start | ||
| .saturating_add(generated_len) | ||
| .min(start_mapping.src_range.end) | ||
| }) | ||
| .max(src_start.saturating_add(1)); | ||
| Some((src_start, src_end)) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a test for the untested unwrap_or_else fallback branch.
All range tests resolve end_offset through mapping_for_end_offset returning Some. The generated_len-based estimate fallback (Lines 83-88), reached only when no mapping covers end_offset at all, has no test coverage.
Suggested additional test
#[test]
fn falls_back_to_generated_length_when_end_offset_is_unmapped() {
let mappings = [VizeMapping {
gen_range: 10..80,
src_range: 100..140,
sub_spans: Vec::new(),
}];
// end_offset (200) is outside every mapping's gen_range.
assert_eq!(
map_generated_range_to_source(&mappings, 20, 200),
Some((110, 140))
);
}Also applies to: 140-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/vize_canon/src/virtual_ts/mapping.rs` around lines 74 - 91, Add a unit
test covering the unwrap_or_else fallback in map_generated_range_to_source when
end_offset is outside every mapping’s gen_range. Use a mapping where
start_offset resolves, choose an unmapped end_offset, and assert the
generated-length estimate is clamped to the mapping’s src_range end.
Contributor
Tool BenchmarkMeasured: 2026-07-21T12:34:47.748Z
Fairness notes:
Commands: gh workflow run tool-benchmark.yml --ref <branch> -f file_count=3000 -f check_file_count=500 -f vite_file_count=1000 -f nuxt_file_count=250 -f large_blocks=300 -f runs=3 -f warmups=1 -f commit_results=true
node bench/generate.mjs 3000
node bench/compare-tools.mjs --input bench/__in__ --vize-bin target/release/vize --runs 3 --warmups 1 --check-file-count 500 --vite-file-count 1000 --nuxt-file-count 250 --large-blocks 300 --runner-label "blacksmith-32vcpu-ubuntu-2404" --out tool-benchmark-summary.md --json tool-benchmark-results.json --doc performance-blacksmith.mdVariant details and raw run timesSFC compile
Large SFC compile
Large SFC type check
Lint
Format
Type check
Vite build (end-to-end)
Nuxt SPA build (end-to-end)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the mapper half of divergence 2 in #2971:
vize check --format jsonreported TS2322 for:msg="42"at a content-independent generated column (the same column for:msg="count"), becauseSfcSourceMap::get_original_positionused first-match lookup plus a linear delta clamped tosrc_range.end - 1and never consulted the exact-expression sub-spans from #2975.Change
vize_canon::virtual_ts::mappingmodule: narrowest-mapping selection, sub-span-aware offset arithmetic with clamping, and range resolution that keeps at least one authored byte.SfcSourceMap, the corsa-bridgeTypeCheckService, and the language server (vize_maestro) now share it verbatim — the maestro copies are deleted and its tests import the shared functions.Measured on a minimal repro (
defineProps<{ msg: string }>+<HelloWorld :msg="42" />):vize checkmoves from the content-independent11:25to the authored expression11:23; absurd columns past the authored line (e.g.14:285in stale real-project snapshots) can no longer be produced because synthetic text clamps into the authored range.vue-tsc anchors the same diagnostic at the attribute name (
11:18); retargeting the check identifier's sub-span to the attribute name is the follow-up PR so both engines agree byte-for-byte.Tests
SfcSourceMap(synthetic identifier → authored expression; nested-range narrowest wins).vize_canon634 tests and thevize_maestrosuite green; fmt/clippy clean.Part of #2971
Need help on this PR? Tag
/codesmithwith what you need. Autofix is enabled.Summary by CodeRabbit