Skip to content

Hide whitespace changes on the Code tab, with a row that shows them - #75

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

krishhgg merged 18 commits into
masterfrom
t3term/pull-request-whitespace

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Hides changes to whitespace on the Code tab at first, as the desktop does, and adds a row above the fold row that shows them or hides them again.

  • At first, a line changed only in its whitespace shows unchanged, with its number and text after the change. A file changed only in whitespace shows no counts, and the other files count only the lines left.
  • ¶ Show whitespace changes sits above ⇊ Expand all files once the tab has files. Enter or a click shows the patch's own lines in every file, those read later too, and the row then reads ¶ Hide whitespace changes. A read again and another diff picked from the commits keep the choice. The status line says Enter show whitespace changes or Enter hide whitespace changes.
  • whitespace.rs ports the desktop's hideWhitespaceChanges and the line diff it runs: jsdiff 9.0.0's Myers diff, with jsdiff's choice between paths of the same length, so the same lines pair. JavaScript's \s decides what counts as whitespace, so U+FEFF is whitespace and U+0085 isn't.
  • A slice's hunks are compared smallest first, within 4,000,000 steps between them. The hunks left past that show as the patch has them, and N hunks too large to compare show their whitespace changes. says so under the count.
  • docs/pull-request-panel.md gains a Whitespace row and drops whitespace from the Code tab's known limits. The README names the row among the Code tab's keys and clicks.
  • The Code recipe checks reader.rs at +5 -3 with whitespace changes hidden, shows them with the row, checks that nothing is read, and runs the rest with them shown. Pick a commit on the Code tab to read its own diff #70's Code starts hidden again (69d0304).

Stacked on #74.

The Code tab of #7 with src/config/reader.rs unfolded and the row ¶ Show whitespace changes chosen above ⇈ Collapse all files. The header reads +5 -3. In the second hunk, let first is removed in red, and let config, Ok(config) and the closing brace show unchanged around the added Schema::current().check line. The status line reads Enter show whitespace changes.

After Enter: the row reads ¶ Hide whitespace changes and the header +7 -5. The second hunk shows the patch's lines: let first, let config and a tab-indented Ok(config) removed in red, then let config, Schema::current().check and Ok(config) added in green. The status line reads Enter hide whitespace changes.

Both come from the fixture server at 120x48. #7's invented diff now has two lines changed only in whitespace: a Ok(config) indented with a tab before and spaces after, and Self {version: 3} formatted as Self { version: 3 }. Hidden, jsdiff also pairs let config, which the fixture's patch lists as removed and added again with the same text.

Decision log

What I noticed. docs/pull-request-panel.md listed the desktop's whitespace toggle among what the Code tab lacked. The desktop hides whitespace changes by default, so a reader moving from it to t3term saw a reformatted file's every line as changed where the desktop showed only the real edits.

How I confirmed it. In the pinned T3 source (f570bd2):

  • diffIgnoreWhitespace defaults to true (packages/contracts/src/settings.ts:377). The Code tab's ignoreWhitespace starts from it and isn't reset with the diff picked (apps/web/src/components/pullRequest/PullRequestCodeTab.tsx).
  • The toolbar's toggle says Show whitespace changes while they're hidden and Hide whitespace changes while they show (PullRequestCodeTab.tsx:1211-1234). The parse cache's key holds the toggle, so each slice is parsed again when it flips (384-402).
  • hideWhitespaceChanges (apps/web/src/lib/diffRendering.ts:87-138) takes each hunk's old lines (context and removed) and new lines (context and added), takes every \s out of each, and runs parseDiffFromFile with context: Infinity. That calls jsdiff 9.0.0's structuredPatch, then @pierre/diffs 1.5.2's processFile.
  • To check the port, jsdiff.mjs runs that exact filter in Node with the pinned packages over random hunks: mixed runs, tabs and Unicode spaces, \r, characters JavaScript doesn't call whitespace, and \ No newline at end of file on either side or both. The port matched every one of 101,104 hunks. jsdiff.json keeps 59 patches (261 hunks), which tests.rs checks on every run.

Options.

  • Chosen: port jsdiff's diff by hand, about 100 lines. Rejected: a diff crate such as similar, which is a new dependency and pairs lines differently where several paths have the same length. Since the desktop renders jsdiff's path, a different path would show different lines as changed.
  • Chosen: a row above the fold row, as Unfold or fold every file of the Code tab from one row #74 did for the fold. The desktop's toggle always shows. t3term's shows once the tab has files, since a toggle over no lines does nothing.
  • Chosen: a budget of 4,000,000 steps per slice, with hunks compared smallest first. Myers' diff takes time in proportion to the lines times the differences, so one rewritten 10,000-line file would take seconds. Rejected: a budget per hunk, which caps no slice of many large hunks. Rejected: running in order until the budget runs out, which showed small hunks after a large one as too large (a test found this, see below).
  • Chosen: one \ No newline at end of file note where the desktop marks a row. pierre draws the mark only on the last run of a hunk, and t3term follows that rather than keeping the patch's note where the line moved.

What review changed.

  • A test of two slices showed the shared budget failing every hunk after a large one, small ones included. The hunks are now compared smallest first, and a test checks that a 2-line hunk after two of 1,500 lines is compared.

  • Mutation testing changed whitespace.rs and code.rs in 59 ways, and the tests caught 47 at first. Four of the rest moved the \ No newline at end of file note when a hunk's last line, in context, is left removed or added once whitespace is ignored. Two tests now check where it goes, matched in Node against where pierre puts it, and catch all four. A fifth was a check that a hunk has lines before its note, which can't fail, so the check is gone.

  • Of the other seven, one puts the new side's note before the old side's, and the two notes read the same. Four widen or loosen jsdiff's bounds on the paths it tries: 3,000,000 random hunks of a few letters found no difference from the port, so the bounds stay as jsdiff has them. Two take a file's path or place from the patch's files rather than those shown, and the two lists share both.

  • Greptile pointed out that once the budget ran out, every larger hunk was still stripped and hashed before compare failed on its first step. hide now stops there. The rows are the same, and a slice of 26 hunks past the budget (about 200,000 lines) takes 21 ms in hide rather than 29 ms in a release build.

15bb2da and 3036809 merge #74 at f7866d8 and f4d6ccc, which bring #66's fix for a hunk that removes nothing at the highest line number (8073497) and its test, with the same patch.

The appearance test that #66 added pressed Down once to reach a file, and the whitespace row puts another stop above it. 96bf5df presses Down until the status line says Enter expand file, and 9f252f2 allows up to 10 presses for the rows the next layers add.

CI's E2E smoke then failed on #76, #77 and #79 at the Code recipe's check that a trip to the Timeline reads no marks. On a macOS runner the trip came back 19 s after the marks' first read, past the 15 s in which t3term reads nothing, because this layer's whitespace steps sat in between. d9fd767 moves the trip before them, as on #73 and #74, and adds a gotcha that keeps later steps out of that span.

Sol's round 1 verdict on 3036809 found two gaps. Removing hide's early stop left every test passing, since the hunks after it fail on their first step and show as the patch has them either way. 7d8ecf8 counts the hunks rebuilt reads, in tests only, and no_hunk_is_read_once_the_steps_run_out expects three of four read. Without the stop it reads all four. Sol's sweep also failed the Code recipe at the Timeline trip after the whitespace steps, the span d9fd767 moves.

507c71d and 49e292e merge #74 at ed13d8b and ce3e374, which bring #72's fix that counts each Enter on a run while the file's text is read 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.

  • The diffIgnoreWhitespace setting isn't read, so whitespace always starts hidden, and the row's choice isn't saved.
  • A run of changes that removes more lines than it adds, or adds more than it removes, shows as git wrote it, with or without whitespace. pierre moves the extra lines before or after the rest where that lines up the most alike lines (dist/utils/realignChangeContent.js). This was already the case before this pull request, and docs/pull-request-panel.md now lists it.
  • A hunk past the budget shows its whitespace changes. For a file cut at 200,000 lines, the hunks left out keep their counts.
  • The Code tab still lacks the toolbar's split view and wrap toggles and the file tree.

Tests

  • whitespace/tests.rs: the 59 recorded patches pair their lines and count as the desktop's filter does. Also: a line changed only in whitespace, JavaScript's whitespace, a hunk of whitespace alone, the end-of-file note on each side and after a last line in context left removed or added, hunks past the budget compared smallest first, a cut file's counts, and compare taking jsdiff's path with its step count on 9 cases and the budget's edge.
  • code/tests.rs: no row before a slice or with no files, the rows with whitespace hidden and shown, the row's label, action and color at both states, its width at 8 columns, bodies laid out again after the toggle, the choice kept through a later slice, a read again and a commit's diff, and the note on hunks too large in one slice and two. Every test that compares the tab's rows has the row.
  • pull_requests.rs: ↓ choosing the row first, Enter and a click showing and hiding the change with the row still chosen and nothing read.
  • mod.rs: the status line saying Enter show whitespace changes with the row chosen.
  • On 3036809, fmt, clippy and all 881 tests pass (800 lib, 2 main, 31 CLI, 48 RPC). The Code recipe passes on 3036809's build and fails on Unfold or fold every file of the Code tab from one row #74's (f4d6ccc) at recipe line 24, the first wait for the ¶ Show whitespace changes row. d9fd767 changes only the recipe, which passes with it on 3036809's build.
  • On 49e292e, fmt, clippy and all 884 tests pass (803 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

The PR appears safe to merge; no new actionable issue was found.

Summary

Adds a Code tab row that shows or hides whitespace changes. Whitespace starts hidden, and the choice stays through refreshes and commit changes.

  • The Code tab hides whitespace-only edits from its diff counts.
  • The Code tab lets readers show or hide whitespace edits.

Reviews (7) · Last reviewed commit: "Merge the fold-all branch at ce3e374, wh..." · Reviewed by Greptile

krishhgg and others added 3 commits October 15, 2026 07:21
The Code tab hides changes to whitespace at first, as the desktop's
diffIgnoreWhitespace setting does, and a row above the fold row shows
them or hides them again. Each hunk is compared again without
whitespace by a port of jsdiff 9.0.0's line diff, checked against the
desktop's own filter on 59 recorded patches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tespace row

A hunk too large for the steps left no longer takes the smaller hunks
after it down with it: the hunks are compared from the fewest lines up.
Tests cover the row's label, action, color and width, its toggle through
reads and commits, the note on hunks too large to compare, and Enter and
a click on the panel.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and drop a check that never fails

Mutations showed no test of a hunk whose last line, in context, is left
removed or added once compared without whitespace. pierre puts the note
after that line, and two tests now check it does here. The check that
a hunk has lines before its note can never fail, since a hunk with no
lines has no note.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@krishhgg
krishhgg marked this pull request as ready for review October 15, 2026 14:46
Comment thread src/tui/pull_requests/code/whitespace.rs
krishhgg and others added 11 commits October 15, 2026 08:21
Once a hunk spends the last of `MOST_STEPS`, every larger hunk after it fails on its
first step, so `hide` no longer strips, hashes and lays out their lines first.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in after a theme change

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

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
…the Code recipe

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Changes to whitespace start hidden, so the recipe checks reader.rs at +5 -3
with its reindented and repeated lines unchanged, shows them with the row and
checks that nothing is read, then runs the rest with them shown. #70's Code
starts hidden again. TB counts the second toolbar row.

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>
…n the boundary unfold test

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

Copy link
Copy Markdown
Owner Author

FAIL for head 3036809309f99e7421acfc84ae34112535a786a5, patch id 6d897360550ad01a60501c4949aeba02bbce31d1.
Verifier: GPT-6.1 Sol (xhigh), not the author.
Base f4d6ccc6a1c3d41fa91301a994462a7e77d7d2ad, the parent layer. Scenario links, 100x30 and 80x24, truecolor. Round 1.

Claim Base Head
Open Code before changing the whitespace choice reader.rs +7 -5; no whitespace row reader.rs +5 -3, schema.rs +7 -0; paired whitespace-only lines render unchanged
Use Enter and click on the whitespace row at both sizes Row absent; new recipe fails at line 24 Correct labels, status actions and counts at 100x30 and 80x24; no additional RPC or HTTP diff request
Read again and choose a commit after showing whitespace Always shows the original patch Choice retained through refresh, later slices and commit scope
Check the desktop comparison and bounded comparison fixes Whitespace filter absent Pinned desktop source reached; regenerated reference fixture matches byte for byte; smallest-first and newline controls fail when reverted

Regression sweep: Parent 13 of 13; head 12 of 13 in the full run. The same head Code recipe passed on retry, 1 of 1. The original failure is finding 2 below. The parent sweep reuses the f4d6ccc run in /tmp/t3v-verdict/74/round1/smoke-head-run/. All 12 other recipes have the same rpcRequestsByTag on the paired builds. Captures match after accounting for fixture ports, run IDs, clocks and spinners. One reconnect capture caught a different retry stage, and one sidebar age crossed a minute; neither changes request counts or the settled result. Master's sweep is 11 of 13 on each build, with the two baseline failures described under Notes.
Rules: all runs passed doctor and their checks after t3v stop. Completed positive recipes also passed check while live, before cleanup. Summary issued no diff or viewed read. Toggling the new controls added no RPC or HTTP diff request. Unchanged recipe counters and normalized screens are in sweep-comparison/ and final-comparison.json.

Tests: independent cargo test --locked passed 881 tests. CI on this head passed macOS, E2E smoke (macOS), Linux build and Greptile.
Revert controls: the boundary test still fails without #66's saturating add. The smallest-first test and newline-note test fail when their respective non-test fixes are removed. Greptile's earlier performance thread is resolved and answered; no new Greptile finding is open.

Findings:

  1. P2, src/tui/pull_requests/code/whitespace.rs:44. Removing only the if left == 0 early stop leaves all 881 tests passing. This restores the work after budget exhaustion that the decision log and Greptile identify as a bug. Step 6 requires a regression test that fails with this fix removed. Add a test that detects preparation of later hunks after the budget is spent. Evidence: no-budget-early-stop.diff and no-budget-early-stop.log.
  2. P2, .claude/skills/verify-t3term/features/pull-request-code.md:82. The recipe adds whitespace checks before asserting that the return to Code is within the 15-second viewed freshness window. My full sweep failed at recipe line 51, this source line, because the second marks request was 19.221 seconds after the first request, more than 18 seconds after its answer. The product correctly reread stale marks. An unchanged retry passed in 163.7 seconds. Move the short-window check before those extra steps or establish a fresh read immediately before it, so concurrent verification does not make smoke fail. Evidence: smoke-head-run/pull-request-code/ and code-retry/pull-request-code/.

Notes: Master 7ae927b recipes were also run against both binaries. Its CLI recipe stops at launch, which neither stack revision implements, and its Code recipe predates the changes in the earlier Code layers. These failures occur on the parent too. The changed Code feature file is present in each head. Its other 11 recipes are covered by the comparison above.
The pinned desktop source f570bd21663f56ce94c41829d3b7d72886e25a34 was fetched read-only from pingdotgg/t3code and inspected. The stock fixture always uses host-backed viewed marks and has no over-budget whitespace hunk; environment-backed marks, absent capability, bounds and budget cases were covered by the passing unit tests.

Binary driven LC_UUID
f4d6ccc6a1c3d41fa91301a994462a7e77d7d2ad 0E61109F-29A9-3880-B6F7-495CC0B62ED8
3036809309f99e7421acfc84ae34112535a786a5 52420279-ACDD-3DED-BA46-D2E6BF8189FA

Binaries are /tmp/t3v-bin/<sha>/t3term. The parent cache was reused; each missing head build printed Compiling t3term. Verification used stock t3v from master 7ae927b, with each revision's own recipes for its sweep and the head recipe for the parent negative control.

Evidence: /tmp/t3v-verdict/75/round1/. controls-base/ and controls-head-final/ contain the paired size checks; identity.json records the commits, patch id, paths and UUIDs. Rendered captures were inspected.

The trip that reads no marks has to come back within 15 s of their
first read. The whitespace steps sat inside that span, and on a macOS
runner on CI the trip came back after 19 s, so t3term read the marks
again and the recipe failed at its check on #76, #77 and #79. The trip
now comes right after the second slice, as on #73 and #74, and a
gotcha keeps later steps out of the span.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
krishhgg and others added 3 commits October 18, 2026 06:01
…n while the file's text is read 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>
Removing the early stop left every test passing, as the hunks after it fail on
their first step and show as the patch has them either way. A count of the
hunks read, kept only in tests, now tells them apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ents 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 49e292eb61f04f9facd4595b5ebb94d8398ac21f, patch id 282cb08aad04f9a934af538de8689295cd541e99.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base ce3e374939e3a1d94580596435286c2d215f3604. Scenario links, 100x30 and 80x24, truecolor. This verdict covers this layer against the requested parent head.

Claim Base Head
Whitespace starts hidden and the row shows or hides it without a read No whitespace row; reader.rs is +7 -5 Starts at +5 -3; Enter and a click restore +7 -5; later slices, refreshes and picked commits keep the choice; another link starts hidden
No hunk is prepared after the comparison budget runs out Scratch head with only the early stop removed fails the new test twice, reading 4 hunks Fixed test passes twice, reading 3 hunks; all 803 library tests pass
The Timeline trip precedes the whitespace checks and stays inside the marks' freshness window Parent Code recipe passes Code recipe passes twice; return is 9.628 s and 9.272 s after the first marks request, with just one marks request at each return

The round 1 findings are both closed. test-fixed.log, test-fixed-2.log, test-reverted.log and test-reverted-2.log record the early-stop control, with only if left == 0 { break; } removed. The new assertion fails at whitespace/tests.rs:296 with 4 instead of 3. smoke-head/pull-request-code/whitespace-hidden-100x30.txt and code-repeat/pull-request-code/ hold the two feature drives. The trip's times are measured from the logged pr.filesViewed request; the fake reply comes one second later. Neither trip issues a second marks read.

Regression sweep: 13 of 13 own-checkout recipes pass on each side. Base evidence /tmp/t3v-verdict/75/round2/smoke-base; head evidence /tmp/t3v-verdict/75/round2/smoke-head. The identical parent-head build and recipe run are reused as the next layer's base. The changed source files reach Code layout, review comments, details input and the status line; the sweep drives all mapped features. Matched link-list, Summary and Timeline captures are identical. Thread details differ only by the fake server's port. Other capture differences are elapsed ages, spinner phases, generated run paths, the reconnect attempt's current error, and a Summary capture before versus after the three-second conversation reply fills its reviewers. matched-comparison.json and sweep-capture-comparison.json retain the comparisons.

Rules: doctor and check pass in the focused drives and before and after stop in the sweeps. Matched steps send the same rpcRequestsByTag: one each of subscribeServerConfig, orchestration.subscribeShell, orchestration.subscribeThread, pullRequests.subscribeRefreshes, pullRequests.detail, pullRequests.activity, pullRequests.stack and pullRequests.filesViewed. Both read the same two HTTP diff slices. The matched drive adds no RPC cost; the feature recipes and unit tests also check that toggles read nothing.

CI: all four checks pass on this exact head, including macOS and E2E smoke. Greptile: 1 thread, resolved and answered; no new unanswered finding. The head updates .claude/skills/verify-t3term/features/pull-request-code.md.

Binary provenance: base /tmp/t3v-bin/ce3e374939e3a1d94580596435286c2d215f3604/t3term, LC_UUID 9C6D7C6E-5B76-3B34-B9FA-896755F83597; head /tmp/t3v-bin/49e292eb61f04f9facd4595b5ebb94d8398ac21f/t3term, LC_UUID 829C7A80-B290-39F4-A708-D8899FBEDCCB. The base cache was reused; each newly built head logged Compiling t3term. The builds use t3v from master c250a7510b9d6ee71b635fd8ea6f9fa793f72fbe.

Notes: master's current sweep is 11 of 13 on both sides, at /tmp/t3v-verdict/75/round2/master-smoke-base and /tmp/t3v-verdict/75/round2/master-smoke-head. Its CLI recipe at .claude/skills/verify-t3term/features/cli.md:32 asks these older pinned builds for the later launch command. Its Code recipe at .claude/skills/verify-t3term/features/pull-request-code.md:39 expects the pre-stack count and loading layout, before the viewed count and toolbar rows. Both fail on base as well as head. The updated recipes in this stack pass; these shared master-recipe incompatibilities are not a regression in this layer. A merge with newer master still needs the integration recheck from the skill.

No new blocking findings. Evidence stays under the paths above. All owned runs were stopped and checked; no branch or PR code was changed.

@krishhgg
krishhgg changed the base branch from t3term/pull-request-fold-all to master October 18, 2026 19:32
@krishhgg
krishhgg merged commit 509e0af 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