feat(code-review): entity-level semantic diff in the code review panel#9917
feat(code-review): entity-level semantic diff in the code review panel#9917monotykamary wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
Add entity-level (semantic) diff understanding to the Code Review panel, so diffs are presented in terms of code entities (functions, classes, structs) rather than raw lines. What: - New semantic_diff module in crates/languages with entity extraction, 3-phase matching (exact name → structural hash → fuzzy similarity), and change classification (modified/renamed/added/deleted/moved/unchanged) - SemanticDiff feature flag (dogfood-enabled) gating all new behavior - Entity summary pills in file sidebar (e.g. 'fn foo (modified) · bar → baz (renamed) · struct X (added)') - Entity-aware stats in file header (e.g. '+5 -3 (2 modified, 1 renamed)') - Entity diff computed at FileDiffAndContent construction from base and current file content using existing tree-sitter infrastructure Why: - Line-based diffs with ±4 context don't align with how humans read code - Renamed functions show as +5 -5 rather than 'renamed' - Formatting-only changes are indistinguishable from logic changes - Upstream issue warpdotdev#9898 requests this capability How: - Reuses existing tree-sitter parsing (arborium) and identifiers.scm queries — no new parser dependencies - Body hash skips signature line so renames produce matching hashes - Whitespace-normalized hashing so formatting-only changes classify as Unchanged - Falls back to standard line-based diff for unsupported languages (no identifiers.scm) or parse failures - Works on all platforms including WASM (entity_diff is None when local_fs is unavailable) Tests: 19 unit tests covering extraction, matching, rename detection, formatting-only classification, and multi-language support (Rust, Python, JavaScript). All existing code_review tests (75) and diff_state tests (20) continue to pass. Co-Authored-By: Warp <[email protected]>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds entity-level semantic diff extraction, stores the result with code review file diffs, and renders semantic summaries in the code review UI behind a feature flag.
Concerns
- The UI change has no attached screenshot or video. For faster review, please upload screenshots or a video of the feature working end to end.
- The current implementation can leave semantic summaries stale after watcher-driven per-file invalidations.
- Entity extraction currently misses bare
@definitioncaptures and can include non-definition captures from existingidentifiers.scmfiles. - The body hash skips all content for single-line entities, so real one-line logic changes can be classified as unchanged.
- The implementation adds synchronous full-file reads and tree-sitter parsing without the existing size/local-fs guardrails.
Security
- The semantic diff path parses repository-controlled file contents without a size cap before invoking tree-sitter, which can create unbounded CPU/memory work in the code review load path.
Verdict
Found: 0 critical, 7 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
could i see a screencast of this? |
Fix 6 issues from review on PR warpdotdev#9917: 1. fix(code_review_view): update entity_diff on watcher refresh - Add current.entity_diff = diff.entity_diff.clone() alongside current.file_diff = diff.file_diff in update_from_single_file_diff_result so entity summary doesn't go stale after edits 2. fix(diff_state): guard entity diff computation with diff-size limits - Skip entity diff for DiffSize::Large/Unrenderable files - Cap current file content at 4.375 MB before parsing - Prevents unbounded CPU/memory on large repo-controlled files 3. fix(diff_state): gate fs::read_to_string behind local_fs cfg - Wrap entity diff computation in #[cfg(feature = "local_fs")] with None fallback for non-local-fs/WASM builds - Both construction sites (diff_state_against_head and file_diff_for_path) are now properly gated 4. fix(semantic_diff): accept bare @Definition captures - Previous filter skipped bare @Definition (used by 18+ languages including C, C++, C#, Java, JavaScript) and leaked @reference.call - Now accepts any capture starting with "definition", rejects everything else - type_prefix extracted via strip_prefix("definition.") 5. fix(semantic_diff): mask name token instead of skipping signature - Skipping the first line made single-line entities hash the empty string (fn foo() { 1 } and fn foo() { 2 } classified as Unchanged) - Now masks (replaces with spaces) only the name token's byte range in the entity body, preserving signature structure while making the hash invariant to renames - Adds tests for rename-invariant hashing and single-line entity body changes 6. fix(semantic_diff): use neighbor-based Moved classification - Absolute line-shift heuristic caused false Moved when lines were inserted above an entity - Now compares immediate neighbors in the entity ordering: both neighbors must change for Moved, preserving robustness to insertions/deletions above - Adds test_insertion_above_does_not_cause_moved Co-Authored-By: Warp <[email protected]>
|
Oh yes, give me a hot moment 🚀 |
…diffing
Add SemanticDiff feature flag (Dogfood) that enables two complementary
improvements to the code review diff panel:
1. Entity classification (crates/languages/src/semantic_diff.rs)
- Extract semantic entities (functions, methods, structs, impls) from
tree-sitter using existing identifiers.scm queries.
- 3-phase matching across base/current: exact name → structural body hash
→ fuzzy Jaccard similarity (>80%).
- Classifies changes as: Unchanged, Modified, Renamed{old_name}, Added,
Deleted, Moved (via neighbor position shift detection).
- Body hashes use name-token masking so renames alone don't affect the hash.
- Gated behind #[cfg(feature = "local_fs")] with 4.375 MB file size cap
and DiffSize::Large/Unrenderable guard.
2. Syntax-token diff (replacing line-based hunks for supported languages)
- Parse both files with tree-sitter, collect leaf tokens (DFS).
- Comments and strings treated as opaque (atomic tokens) to avoid
fragmented partial matches within rewritten comment blocks.
- LCS diff on token hashes via similar::TextDiff (same Patience algorithm
as line diff, but on tokens).
- Map token-level ops to line-level SyntaxHunks.
- Multi-line rewrites → clean Deletion + Addition blocks (no inline noise).
- Single-line changes → Replacement with per-word inline highlights
(e.g. renamed function names).
- Falls back to line-based diff for unsupported languages or parse failures.
3. Entity-scoped context (app/src/code/editor/model.rs)
- calculate_hidden_lines() expands visible ranges to cover entire
entities containing changes instead of fixed ±4 context lines.
- Added EntityRange { start_line, end_line, kind } to CodeEditorModel,
wired from diff_state entity_diff data.
4. removed visual noise per user feedback
- Removed sidebar entity summary pills (render_entity_summary).
- Removed file header entity counts (render_file_stats_with_entities
reverted to render_file_stats).
- These were duplicating existing diff information and adding clutter.
5. Infrastructure
- Added file_path to DiffModel for language detection in syntax diff.
- Added similar.workspace = true to crates/languages/Cargo.toml.
- Added 30 unit tests covering entity extraction, matching, classification,
syntax-token diff, word-level highlight alignment, and comment opacity.
Refs warpdotdev#9898
|
It's currently a bit naive, but currently experimenting what would look the best: The work before the visuals are closer to sem, but my colleague wanted something closer to difftastic. I'll continue experimenting a clean up the PR. |
|
@kevinyang372 and @alokedesai will be the most knowledgeable here |
|
@monotykamary I am a bit confused by what I am looking at with your attached screenshot:
Also looks like there are gaps between gutter diff highlighting and the line diff highlighting I would love to see better semantic highlighting in Warp too! But my understanding is this is a much bigger change that changes the underlining meaning of the diff highlights. We should spec out what the right product behavior is first |
|
Oh that's right, my approach is much too naive for what really good semantic diffs would offer for the code reviewer. I'll leave this PR here for experimentation (and update the issue to capture the scope a bit better). Token based diffing is easy to test and verify, but surprisingly bad to look at. I understand why dax has been saying diffs are hard. |


Description
Add entity-level (semantic) diff understanding to the Code Review panel, so diffs are presented in terms of code entities (functions, classes, structs, methods) rather than raw lines.
What changes:
crates/languages/src/semantic_diff.rsmodule (~860 lines) that extracts named code entities from two versions of a file using existing tree-sitter infrastructure and classifies changes between themSemanticDifffeature flag (dogfood-enabled) gating all new behavior — no impact when disabledfn foo (modified) · bar → baz (renamed) · struct X (added)+5 • -3 (2 modified, 1 renamed, 1 added)FileDiffAndContentconstruction time alongside existing line-based diffWhy this matters:
How it works:
identifiers.scmqueries — no new parser dependenciescargo fmt/prettierruns classify entities as "unchanged"identifiers.scm), parse failures, binary files, new/untracked filesentity_diffisNonewhenlocal_fsis unavailable)Future phases (not in this PR, but the architecture supports them):
Linked Issue
ready-to-specorready-to-implement.Screenshots / Videos
This is a feature-flagged change with no visual impact when
SemanticDiffis disabled. When enabled, the file sidebar shows entity pills and the file header shows augmented stats. Screenshots will be added once the feature is enabled in a dogfood build.Testing
New unit tests (19 in
crates/languages/src/semantic_diff.rs):identifiers.scmreturns NoneExisting tests continue to pass:
code_review_view_tests.rsupdated withentity_diff: NoneNo integration test added — this feature is behind a feature flag and has no user-visible flow when disabled. The entity extraction and matching are purely computational (no UI interaction to test end-to-end). Integration coverage can be added when the feature is promoted beyond dogfood.
Agent Mode
Co-Authored-By: Warp [email protected]