Skip to content

Pair a change's most alike lines as the desktop does - #80

Merged
krishhgg merged 13 commits into
masterfrom
t3term/pull-request-realign
Oct 18, 2026
Merged

krishhgg merged 13 commits into
masterfrom
t3term/pull-request-realign

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Pairs the lines of an uneven change by how alike they are, as the desktop's diff does, rather than the first removed beside the first added.

  • A change with more lines on one side can now split so the lines that pair are the most alike. The longer side's other lines show before them, after them or both. The new code/align.rs ports @pierre/diffs 1.5.2's realignChangeBlock (dist/utils/realignChangeContent.js), with its bound of 4,096 comparisons and its half point a pair in favor of the order the patch has.
  • Lines compare without their whitespace, by their UTF-16 code units, as JavaScript compares them.
  • Side by side, each part pairs its lines in order. Stacked, each part shows its lines removed before those it adds, as pierre stacks them, so a line added before the pair now shows above the line removed.
  • A run that alternates removed and added lines now stacks its lines removed first, as pierre does, where t3term kept the patch's order.
  • The note that a line has no line end stays after its line.
  • docs/pull-request-panel.md's Layout row says how pierre pairs and stacks a change's lines. Its known limit on pairing goes, and a new one covers the note.
  • In the Code recipe, reader.rs's removed self.load_unchecked() now pairs with the added self.load(), so the split steps wait for the comment added above it on a row of its own. A check on the sideways capture holds the stacked order: the comment, then the removed line, then the added one (47cb8fc).

Stacked on #79.

Before, on #79's head: #7's Code tab with src/config/reader.rs unfolded, stacked. In reload, line 32 removes self.load_unchecked(), then 33 adds a comment and 34 adds self.load().

After, stacked: the comment added on line 33 shows first, then line 32 removes self.load_unchecked() and line 34 adds self.load() under it.

Before, side by side: self.load_unchecked() on the left sits beside the comment on the right, and self.load() sits beside nothing.

After, side by side: the comment sits beside nothing, and self.load_unchecked() sits beside self.load().

All four come from the fixture server at 120x48. The before shots use #79's head, aa47159. self.load() shares 11 of the 21 code units in self.load_unchecked(), 0.52, and the comment shares none. The pair in order scores 0 plus the half point, so pierre moves the comment ahead.

Decision log

What I noticed. docs/pull-request-panel.md listed as a known limit that "a run of changes that removes more lines than it adds, or adds more than it removes, shows as git wrote it", where the desktop moves the lines left over to line up the most alike lines.

How I confirmed it. @pierre/diffs 1.5.2's parsePatchFiles calls realignChangeContentBySimilarity on every file it reads (dist/utils/parsePatchFiles.js:296). parseDiffFromFile runs the same _processFile, so the desktop's whitespace filter, which calls it (apps/web/src/lib/diffRendering.ts:99), splits changes too. The PR Code tab reads its slices through getRenderablePatch (PullRequestCodeTab.tsx:395), which calls parsePatchFiles. iterateOverDiff stacks each part's deletions before its additions (dist/utils/iterateOverDiff.js:440-446), so a split changes the stacked order as well as the pairs.

Options.

  • Chosen: split each change into parts as pierre's hunkContent holds them, and have both layouts read the parts. stacked and pairs share one walk, so the two can't disagree.
  • Chosen: compare UTF-16 code units. Counted as characters, 😀 and 😃 share nothing, where pierre finds them half alike, and the choice flips (a test pins it).
  • Rejected: reorder the hunk's lines once, when the slice is read. The pairs need the parts' bounds as well as the order, and the whitespace filter builds new hunks that would need splitting again.
  • Rejected: copying pierre's placement of the note that a line has no line end in stacked view. When pierre splits a hunk's last change, its stacked view shows no note for a line outside the last part (dist/utils/iterateOverDiff.js:447-448), which hides that the file lacks its last line end. t3term keeps the note after its line, and the panel doc says so.

What review changed. Greptile scored it 5/5 on 76b05d1 and found nothing to change.

71b0d34 and c1af472 merge #79 at 3731cd9 and c798b74, which bring #66's fix for a hunk that removes nothing at the highest line number (8073497) and its test, with the same patch.

00f9864 merges #79 at 2743a8d, which returns from the Timeline before #75's whitespace steps (d9fd767), with the same patch.

77fe142 and ab66976 merge #79 at 58529f0 and 14ee77c, which bring #72's fix that counts each Enter on a run while the file's text is read, #75's test that no hunk is read once the whitespace steps run out, and the fixes that lay a thread's comments and its loaded pages out again when the theme changes (#68's ad1590c and #69's 942dad5).

Known limits.

  • Stacked, where pierre splits a hunk's last change, t3term shows the note that a line has no line end after its line, where pierre shows none for a line outside the last part.
  • The lines that pair stay together, as in pierre, so a change whose lines left over fall between lines that pair keeps them in order.
  • Laying out takes longer where changes are uneven, as below.

Measurement

On an Apple M5, cargo test --release ran a temporary test, not committed, on #79's head (aa47159) and on this branch. It built one slice of 1,500 changed files, each a hunk that removes 66 lines and adds 67, 199,500 lines in all. Each line is a 300-character line of code with its number in front, and each added line ends in an x, so every pair's lines share their first 300 or so code units. The test unfolded every file and timed code.prepare(width, &theme()) at 52, 51 and 50 columns, wrapped, stacked and side by side.

Stacked Side by side
aa47159 1.36 to 1.38 s 2.15 to 2.25 s
This branch 1.54 to 1.55 s 2.25 to 2.38 s

Every change there compares 132 pairs whose lines match for about 300 code units. A change with as many lines on each side, or lines only on one, compares none.

Tests

  • each_change_splits_as_the_desktops_does: align/pierre.json holds 63 random patches and each hunk's parts as pierre 1.5.2 split them, as the patch has them and with whitespace hidden. pierre.mjs writes it and says how. t3term's parts match in all of them, 39 changes split as the patch has them and 32 with whitespace hidden.
  • Seven tests in align/tests.rs: lines left over before, after and around the pair; the half point and ties; code units outside the Basic Multilingual Plane; whitespace as JavaScript's \s has it; the bound of 4,096 comparisons at 4,096 and 4,097; the notes; and a line that starts another, each of its characters counted once.
  • a_change_pairs_its_most_alike_lines_and_stacks_them_so: the Code tab's rows for a split change, stacked and side by side.
  • whitespace_starts_hidden_and_one_row_shows_it now expects SPACED's alternating run to stack its lines removed first.
  • Mutating the half point, > to >=, the code units, the overlap or the bound each fails at least one test.
  • On c1af472, fmt, clippy and all 925 tests pass (844 lib, 2 main, 31 CLI, 48 RPC). The Code recipe passes on c1af472's build and fails on Wrap a diff's lines where a browser would break them #79's (c798b74) at recipe line 85, the check of reader.rs's stacked order: the comment, the removed line, then the added one. 00f9864 changes only the Code recipe, which passes with it on c1af472's build.
  • On ab66976, fmt, clippy and all 928 tests pass (847 lib, 2 main, 31 CLI, 48 RPC), and the Code recipe passes on its build.

🤖 Generated with Claude Code

RetriggerView in GreptileConfidence Score: 5/5

This PR appears safe to merge; no actionable issues were found.

Summary

Uneven changes now pair their most alike lines. Both layouts read the same change parts.

  • Uneven code changes pair the lines that look most alike.

The full PR diff was reviewed because the previous and current revisions have different merge bases with the target branch.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  H[Hunk lines] --> P[Group change lines and their notes]
  P --> A[Choose the best pairing offset]
  A --> C[Emit change parts]
  C --> S[Stack removed lines before added lines]
  C --> B[Pair lines side by side]
Loading

Reviews (6) · Last reviewed commit: "Merge the word-wrap branch at 14ee77c, w..." · Reviewed by Greptile

@pierre/diffs splits a change with more lines on one side so the lines
that pair are the most alike, and the desktop runs it on every patch.
The Code tab now does the same, stacked and side by side, and stacks
each part's lines removed before those added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg
krishhgg marked this pull request as ready for review October 17, 2026 03:40
krishhgg and others added 12 commits October 17, 2026 14:05


Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ain after a theme change

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ge recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… more toolbar rows

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
 and the fixes and recipes under it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… in the Code recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
reader.rs's removed self.load_unchecked() now pairs with the added
self.load(), the line most like it, so the split steps wait for the
comment added above it on a row of its own, and a check on the
sideways capture holds the stacked order: the comment, then the
removed line, then the added one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… count at the highest line number

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in the boundary unfold test

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e before the whitespace steps

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…un while the file's text is read, tests that no hunk is read once the whitespace steps run out and brings #68's fix that lays review threads out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ments out again when the theme changes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@krishhgg

Copy link
Copy Markdown
Owner Author

PASS+NOTES for head ab669761b3393e94d4f63da25275e17cbfc5da20, patch id 114f11815a759b7efd06f4aaa870c8b3597b8e1c.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 14ee77ca3f725438207b13cd1875161ef986c30c. Scenario links, 100x30 and 80x24, truecolor. This verdict covers this layer against the requested parent head.

Claim Base Head
Uneven changes self.load_unchecked() pairs with the added comment It pairs with self.load(); comment comes before that pair, in stacked and split layouts
Pierre alignment Position-based pairing Actual Rust port and Pierre 1.5.2 realignChangeBlock agree on pairs and stacked order for all 199 hunks
UTF-16, whitespace, ties and bound No similarity alignment Matches non-ASCII and surrogate cases, JS whitespace, half-point preference, ties, 4096 and 4097 comparisons and two multi-pair bounds
Alternating changes and newline notes Patch order Each change part stacks removed lines first; no-newline notes remain with their lines, as the documented intentional difference requires

Regression sweep: each checkout's own smoke passed 13 of 13 recipes on both base and head. Current master c91e3e70964501b463e112c45524c28a08a65290 smoke passed 11 of 13 on each side, with the same two inherited mismatches described below. The changed feature file is present and its full head Code recipe passes.

Rules: live and after-stop checks passed. The identical settled Code flow has the same rpcRequestsByTag on both sides at both sizes: one each of shell/config/thread subscriptions, refresh subscription, detail, activity, stack and filesViewed. Both read diff cursors [null,"s2"]. The settled non-Code flow also has identical counters and screens. Captured differences outside Code are temporary paths, clocks, spinner phase, in-flight stack responses or a feed's transient reconnect state. Raw smoke has cancellation-sensitive counter differences in unchanged linked-list/appearance recipes; the settled paired drives remove that timing ambiguity. See evidence-comparison.json, common-v3-*/ and noncode/.

CI: macOS, E2E smoke, Linux build and Greptile checks passed on this exact head. Greptile: no review threads or new head findings; 5/5 review completed.

Evidence: smoke-own-{base,head}/pull-request-code/sideways-100x30.txt and the split captures show the changed ordering; I inspected their PNGs. The identical Code drive also ran at 100x30 and 80x24.

I downloaded @pierre/diffs 1.5.2 into pierre-download/, checked archive paths, installed no package or lifecycle scripts, and inspected the alignment module. Node ran the actual realignChangeBlock, exposed by an extra export, with filesystem access restricted to this PR's evidence. I compared it with a standalone runner including the unchanged Rust production align.rs, copied verbatim to port-align.rs. Its Git blob matches the head. The 199 hunks include every shown patch hunk in the 63-case test fixture and 18 additional cases, including uneven blocks, alternating lines, notes, non-ASCII, JS whitespace and both comparison limits. align-comparison.json reports zero mismatches. align-cases.json, pierre-pairs.json, port-pairs.txt and both runners retain the inputs and outputs.

Product binaries driven, with LC_UUID:

  • Base /tmp/t3v-bin/14ee77ca3f725438207b13cd1875161ef986c30c/t3term: 104A252F-FE2C-3C94-A24F-14FA2BF9152C.
  • Head /tmp/t3v-bin/ab669761b3393e94d4f63da25275e17cbfc5da20/t3term: 95004F4E-9E90-39AA-BA2E-A3F2817D5057.

The existing f78d5f7 cache was reused. Each newly built head logged Compiling t3term; builds used master's fixed build locking. Evidence stays under /tmp/t3v-verdict/80/.

Notes, inherited verification mismatches only:

  • At current master, .claude/skills/verify-t3term/features/cli.md:32 calls launch, which neither this base nor head implements. Both own CLI recipes pass.
  • At current master, .claude/skills/verify-t3term/features/pull-request-code.md:42 expects tests/config_reload.rs and automatic second-slice loading in the initial viewport. The parent stack's toolbar already pushes that file below it. Both sides fail there; the updated checkout-specific Code recipes pass.

No product findings and no required claim left inconclusive. No PR source, branch, merge state or author fix was changed.

@krishhgg
krishhgg changed the base branch from t3term/pull-request-word-wrap to master October 18, 2026 20:51
@krishhgg
krishhgg merged commit c49cecf into master Oct 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant