build(deps): move to Pierre diffs 1.3.5 - #753
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Greptile SummaryUpgrades
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking opportunity to make the new regression test cover the complete rendered block ordering. The dependency and lockfile update are internally consistent, while the only accepted concern is that the regression test can miss incorrect placement of the surrounding pure additions. Files Needing Attention: src/core/diffFile.test.ts Important Files Changed
Prompt To Fix All With AI### Issue 1
src/core/diffFile.test.ts:96-108
**Assert complete block ordering**
The test filters out the pure-addition blocks and checks only the mixed 1:1 pair, so an ordering regression that places the two setup lines below the paired return line still passes even though split view consumes the complete `hunkContent` sequence in order.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "build(deps): move to Pierre diffs 1.3.5" | Re-trigger Greptile |
| ); | ||
| const paired = changes.filter((change) => change.additions > 0 && change.deletions > 0); | ||
|
|
||
| expect(paired).toHaveLength(1); | ||
| expect(paired[0]).toMatchObject({ additions: 1, deletions: 1 }); | ||
| expect(metadata.deletionLines[paired[0]!.deletionLineIndex]).toContain( | ||
| "return computeTotal(items, taxRate);", | ||
| ); | ||
| expect(metadata.additionLines[paired[0]!.additionLineIndex]).toContain( | ||
| "return computeTotal(items, taxRate, discount);", | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Assert complete block ordering
The test filters out the pure-addition blocks and checks only the mixed 1:1 pair, so an ordering regression that places the two setup lines below the paired return line still passes even though split view consumes the complete hunkContent sequence in order.
Knowledge Base Used: VCS Diff Sourcing
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/diffFile.test.ts
Line: 96-108
Comment:
**Assert complete block ordering**
The test filters out the pure-addition blocks and checks only the mixed 1:1 pair, so an ordering regression that places the two setup lines below the paired return line still passes even though split view consumes the complete `hunkContent` sequence in order.
**Knowledge Base Used:** [VCS Diff Sourcing](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/vcs-diff-sourcing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Updated the regression test to assert both change blocks, including the leading pure-addition block and its order before the paired return line.
This comment was generated by Pi using gpt-5.6-terra
Pierre 1.3.5 re-splits change blocks whose addition and deletion counts differ, pairing lines by content similarity instead of by position. Hunk builds split-view rows and Pierre's inline word-diff spans from those same hunkContent blocks, so the old position-based pairing could sit a changed line opposite an unrelated one and smear the word highlight across the whole row. Across 50 commits of this repo's own history the pairing changes for 3% of files, touching about half of all commits. The upgrade is otherwise a drop-in: no source changes were needed, parse timings are unchanged, and the compiled binary grows 268 KiB (0.2%). Add a regression test for the pairing, which fails on 1.2.2.
76f7262 to
5d81e59
Compare
Upgrades
@pierre/diffsfrom1.2.2to1.3.5. Drop-in: no source changes were needed.Why
Pierre 1.3.5 added
realignChangeContentBySimilarity, which re-splits change blocks whose addition and deletion counts differ, pairing lines by content similarity instead of by position.This matters to Hunk specifically because
diffRows.ts,review/geometry.ts, andlineHighlightPaint.tsall build from the samehunkContentblocks, and Pierre word-diffs a deletion against whatever addition it is paired with. Bad pairing therefore shows up twice: split view puts unrelated lines side by side, and the word-level highlight marks real code as an edit of something it has nothing to do with.Before / after
Demo changeset — two setup lines added, and one extra argument on the existing
return:export function renderInvoice(order: Order) { const header = buildHeader(order); - return formatInvoice(header, order.items, order.taxRate); + assertOrderIsPayable(order); + const discount = resolveDiscount(order.region); + return formatInvoice(header, order.items, order.taxRate, discount); }Before — Pierre 1.2.2. The deleted
returnis paired withassertOrderIsPayable(order);, so split view sets them opposite each other and the two lines that are genuinely new fall to the bottom:After — Pierre 1.3.5. The two inserted lines are reported as pure additions, and the deleted
returnis paired with the addedreturn:The word-level highlight follows the same pairing. Reading the emphasis spans straight off a real PTY render of that diff, identical in
--mode splitand--mode stack:return,formatInvoice,(header,,.items, order.taxRateassertOrderIsPayable,(, discount1.2.2 paints most of an untouched
returnas changed, against a line it never had anything to do with. 1.3.5 marks exactly the edit.How often this fires
Parsed 50 commits of this repo's own history under both versions and compared the resulting block structure:
Small per file, regularly visible in practice.
Performance
Measured A/B in a single process with both copies loaded, interleaved and median-of-N, so machine drift and process startup cancel out.
parsePatchFiles, 50 real commits (3.5 MB)parseDiffFromFile(where Pierre's bundleddiffwent 8→9)The +8% is the realignment pass. Note it is paid more broadly than it pays off: the similarity scan runs on every change block with mismatched counts, but only 3.1% of files actually come out different. Synthetic worst cases — patches where every block is mismatched — cost +37% to +102% on parse, though even the worst measured case is 9.2 ms vs 4.6 ms for a 290 KB patch.
Two caveats worth recording:
bench:changeset-parseis blind to this change. Its three fixtures contain zero mismatched change blocks, so they never execute the realignment path at all. Run against those fixtures 1.3.5 looks 8–21% faster (the file-splitting rewrite is a genuine win); that number just does not describe real diffs. Worth teaching the fixtures a mismatched-block shape in a follow-up.@pierre/diffsfrom source costs ~11% more (94.8 ms → 105.7 ms), but that does not survive into the shipped binary, which tree-shakes — end-to-end launch time is unchanged.Other costs
@pierre/theme2.0.0 +@pierre/theming;diff9.0.0 joins the existing 8.0.3What this does not fix
1.2.2 throws and silently drops any file whose
diff --githeader has a quoted path (non-ASCII or spaces) — it read only the unquoted regex capture groups. 1.3.5 fixes that, but it doesn't reach Hunk:src/core/patch/gitFormat.tsalready canonicalises those headers before Pierre sees them. And 1.3.5 still returns the octal-escaped name (caf\303\251.ts), so that decoding stays either way. No code to delete here.Testing
bun run typecheck,bun run lint,bun run format:check— cleanbun test— 2688 pass, 0 failbun run test:tty-smoke— 9/9bun run test:integration— 109/111. Both failures pass in isolation, and the 1.2.2 baseline fails one of the same tests under full-suite load, so they are pre-existing timing flakes rather than regressions from this change.src/core/diffFile.test.tsfor the pairing, and confirmed it fails on 1.2.2 rather than being a tautology🤖 Generated with Claude Code
https://claude.ai/code/session_01WNJ2ERAzujufYixffz9iR5