fix(wacdfpp): detect renamed variables and highlight word-level diffs - #105
Merged
Merged
Conversation
Fixes SciQLop#102. A screen full of unrelated-looking +/- blocks for what were actually renamed variables read as "messy to understand at first glance" - the diff engine matched variables by exact name only, so a rename showed as a full remove plus a full add with no indication they were the same variable. - cdf-diff.js: a greedy content-similarity pass (Jaccard over each variable's shape/type/attributes, same idea as `git diff -M`) pairs up same-group removed/added variables above a similarity threshold before they're finalized as adds/removes, producing a "renamed" status instead. A pure rename collapses to one line; a rename with an actual content change also gets a sub-diff. - compare.js/wacdfpp.html: renames get their own colour + icon (violet, "↦"), distinct from add/remove/change - git's own --color-moved uses a similar principle (a third hue for moved content, never reusing red/green). Changed text fields (e.g. a CATDESC differing only in its last few words) now highlight just the differing words via vendored jsdiff, layered as a second, independently-chosen colour on top of the existing row tint (not a deeper shade of it - matches VS Code's diff editor, which keeps insertedTextBackground separate from insertedLineBackground for the same contrast reason). Attribute rows are now indented under their variable's header, so nesting reads from position, not just font-weight. Vendored diff (jsdiff) v9.0.0, BSD-3-Clause, as a classic <script> (its only single-file build is UMD, not ESM) - wacdfpp/jsdiff.js. TDD: tests/wacdfpp_diff/test.mjs covers pure renames, renames with a content change, unrelated adds/removes that must NOT be paired, greedy best-match selection among multiple candidates, and summary/ buildLines output - all passing before this was wired into the renderer. Verified visually in a browser against a synthetic rename + CATDESC-diff fixture, in both inline and side-by-side views. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #105 +/- ##
=======================================
Coverage 93.23% 93.23%
=======================================
Files 62 62
Lines 4802 4802
=======================================
Hits 4477 4477
Misses 325 325
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- cdf-diff.js: split diffModels() into partitionVariables() + resolveGroupRenames() -- cognitive complexity was 16, over the 15 allowed (S3776). Each half now does one job (bucket exact-name diffs vs. raw removed/added candidates; run the rename pass and finalize the leftovers), matching the file's existing style of small single-purpose functions. - compare.js: statusClass() was a 4-deep nested ternary (S3358 x2). Replaced with a plain lookup object. No behavior change -- tests/wacdfpp_diff/test.mjs (45 assertions) still all pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
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 #102.
The compare view's diff engine matched variables by exact name only, so a renamed variable showed up as a full "removed" block plus a full "added" block, unrelated to each other. When several variables were renamed at once, the result was a screen full of disconnected +/- blocks the reporter found "messy to understand at first glance." The color hint in the issue (red=removed/green=added coinciding with File A/File B) pointed at the same underlying problem: nothing in the UI told the reader these blocks were actually the same variables, renamed.
cdf-diff.js): a greedy content-similarity pass — Jaccard similarity over each variable's shape/type/attributes, the same ideagit diff -Muses for file renames — pairs up same-group removed/added variables above a threshold before they're finalized as adds/removes. A pure rename (identical content, new name) collapses to a single line; a rename that also changed content keeps its sub-diff underneath.--color-moveduses the same principle (a third hue for moved content, never reusing red/green) so a "renamed + changed" row can't be misread as a plain change.diff(jsdiff) v9.0.0, BSD-3-Clause. The highlight is a second, independently-chosen color layered on the existing row tint, not a deeper shade of it, matching VS Code's diff editor (insertedTextBackgroundis picked separately frominsertedLineBackgroundfor contrast reasons).Test plan
tests/wacdfpp_diff(TDD): pure renames, renames with a content change, unrelated adds/removes that must NOT be paired, greedy best-match selection among multiple candidates, summary/buildLines output🤖 Generated with Claude Code