[krishhgg]

Shows each slice's files on the Code tab in the desktop's reading order instead of the patch's.

• Source files come first, each after the changed files it imports. Where a cycle leaves none ready, the lowest path goes next.
• Then tests (.test., .spec., or under test, tests or __tests__), each after the source file whose name it has.
• Then lockfiles, snapshots, minified files and anything under dist, build, vendor, __generated__ or __snapshots__, by path.
• Each slice is ordered on its own, so a slice that comes later never moves the files above it. The order is worked out once when a slice comes, not on each frame.
• docs/pull-request-panel.md gets an Order row in the Code tab's table, and the "patch order" limit becomes a narrower one about non-ASCII paths.
• .claude/skills/verify-t3term/features/pull-request-code.md now checks the order of the Rust diff's rows and of a TypeScript diff whose files import each other, and picks the reader with a second Down now that the binary comes first (f863b6e).

Stacked on #66.

Both come from the fixture server with an invented TypeScript patch whose files are out of reading order.

Decision log

What I noticed. #66's Code tab listed files in patch order, which its Known limits named. The desktop orders them so a reviewer reads the change, then what proves it, then what a tool wrote.

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

• orderDiffFiles and its helpers are in apps/web/src/components/pullRequest/pullRequestFileOrder.logic.ts:1-197.
• PullRequestCodeTab.tsx:404-411 calls it on each slice's files separately.
• Its imports come from additionLines and deletionLines. In @pierre/diffs 1.5.2, the version T3 pins, a partial patch's unchanged lines go on both sides (dist/utils/parsePatchFiles.js:208-222). So an import in an unchanged line counts. T3's patch to that library doesn't touch the parser.
• Node's ICU 77 localeCompare sorts every ASCII case in the collation test the same way t3term does.

The desktop's test file (pullRequestFileOrder.logic.test.ts) is ported case for case.

Options.

• Chosen: a small hand-written matcher that does what the desktop's IMPORT_SPECIFIER pattern does, with no new dependency. Rejected: adding the regex crate for one pattern.
• Chosen: Kahn's order with a sorted set of ready files, O((files + imports) log files). Rejected: the desktop's rescan of every remaining file at each step, which is quadratic. Both give the same order, since "the first ready file in path order" is the lowest ready file.
• Chosen: compare paths by a table of the root collation's ASCII order, then lowercase before uppercase. Rejected: byte order, which puts Button.tsx before __tests__ and a_b after a-b, unlike the desktop. Rejected: full Unicode collation, which needs a new dependency for paths outside ASCII.

What review changed. Greptile reviewed it twice, the second time after #66's cache fix merged in at a31ca31, and found nothing to change.

4195b13 merges #66 at 8073497, which brings #66's fix for a hunk that removes nothing at the highest line number (8073497) and its test, with the same patch.

Known limits.

• A path with a character outside ASCII sorts after the letters by code point. The desktop's collation puts é with e.
• Imports are only what the desktop's pattern finds: quoted specifiers after from, import or require(. Rust's use and mod, Python's unquoted imports and the like don't count, as on the desktop.
• Folding is still by path, so a file keeps its fold when the order moves it.

Tests

• order/tests.rs ports the desktop's 15 cases, including tiers across path conventions, chains, ../, index files, the name fallback and its ambiguity, .js for .ts, require and bare imports, cycles, tests by their source, orphan tests, and the same order from any arrival order.
• It adds cases for what the desktop leaves to its library and the browser: imports in unchanged and removed lines, a path a slice has twice keeping its last file, a self-import, a .js specifier that only the path resolves, tests ordered by source against path order, the specifier matcher's edges, and the collation against Node's.
• files_show_folded_and_each_unfolds_into_its_hunks now expects reading order.
• 21 mutations of order.rs each fail a test. Three survived the first run, so the tests gained the self-import, the path-only .js case and the source-position case.
• On 4195b13, fmt, clippy and all 823 tests pass (742 lib, 2 main, 31 CLI, 48 RPC). The Code recipe passes on 4195b13's build and fails on Add a Code tab that shows a pull request's diff, file by file #66's (8073497) at recipe line 24, its check of the Rust diff's file order.

🤖 Generated with Claude Code

Confidence Score: 5/5

The PR appears safe to merge, with the previously reported dotted-test ordering issue remaining as a non-blocking concern.

Findings

1.  Dotted tests sort incorrectly ▶

Fix with agent prompt

### Issue 1
src/tui/pull_requests/code/order.rs:164-166
`tested_name` shortens dotted source names twice. For `user.service.test.ts`, removing `.test.` leaves `user.service`, but the next step cuts that to `user`. The test then matches `user.ts`, or is treated as having no source, instead of following the position of `user.service.ts`.

Return the prefix before `.test.` or `.spec.` directly. Only strip the final extension when neither marker exists.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The Code tab now computes each slice's reading order once when it arrives.

• The Code tab puts each slice's files in reading order.

No new findings were accepted. The full PR diff was reviewed because the previous and current revisions have different merge bases with the target.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Slice arrives] --> B[Parse patch]
  B --> C[Keep last file for each path]
  C --> D[Order source files by imports]
  D --> E[Append tests by source position]
  E --> F[Append generated files by path]
  F --> G[Store slice order]
  G --> H[Append without moving earlier slices]

Loading

_{Reviews (6) · Last reviewed commit: "Merge the code-tab branch at 820c03f, wh..." · Reviewed by Greptile}

[krishhgg]

FAIL for head 4195b1340f3b2d32722d74383c1d75c31ff971da, patch id 59db57c804de794d872fd83d5d9986d4332df498.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 8073497d5c9cc240e4703ddfcb16144cb8d8d19c. Round 1. Scenario links, 100x30 and 80x24, truecolor. This covers this layer against its parent's pinned head.

The behavior checks pass. The FAIL is the review gate: Greptile's new dotted-test finding has no reply and is unresolved. At src/tui/pull_requests/code/order.rs:166, the extra extension stripping matches the pinned desktop's testedBaseName at f570bd2. I checked the source and evaluated its function on dotted names. This calls for an author reply explaining parity, rather than an unreviewed behavior change.

Claim	Base	Head
Each slice uses desktop reading order	Patch order; head recipe fails at line 24	Rust rows and TypeScript import chain have the expected order
Keys and clicks follow displayed files at both widths	Old first file opens	New first file opens; clicks reach panel.tsx; folds survive a read again
Ordering and folding do not read the host	No control-triggered reads	Same; shared recipe prefix has identical RPC counts and diff cursors

Regression sweep: all 13 checkout recipes pass on each build, including the Code rerun with that checkout's fixture; all 12 master recipes pass on both builds. Parent results from the preceding layer are reused by exact SHA. Unchanged captures differ only in run addresses, ages, spinners and reconnect timing, plus master's 80x24 Summary capture straddling the activity answer. That capture was taken 2.971 seconds after the request on base and 3.060 seconds on head, across the fixture's 3-second delay. A settled paired repeat has identical screens and RPC counts, in settled-summary-v2-parent/ and settled-summary-v2-head/. Shared Code steps have identical rpcRequestsByTag and diff cursors. The linked-list recipe can end with one or two refresh subscriptions on either build; paired repeats that wait for the first read to finish have identical counters. Details are in comparison-final.json.

Rules: doctor and live/after-stop checks pass on valid drives. No added control reads the host without the reader asking. The feature file is updated. Local cargo test --locked: 823 tests passed. CI on this head: macOS, E2E smoke, Linux build and Greptile checks passed.

Greptile: one new unanswered thread, discussed above. No independent code defect found.

Binaries driven, with LC_UUID:

• /tmp/t3v-bin/8073497d5c9cc240e4703ddfcb16144cb8d8d19c/t3term: E5A84254-F80A-3246-99E2-F6B1945D8CF3.
• /tmp/t3v-bin/4195b1340f3b2d32722d74383c1d75c31ff971da/t3term: AD48C4F5-18BE-3936-8B68-B9395BD61029.

Evidence: /tmp/t3v-verdict/67/round1/. code-head/pull-request-code/ holds the passing recipe captures, code-negative-parent/ the negative control, code-common-prefix.json the matching request counts, and run-index.json the run paths. manual-head-100x30/folded.txt and manual-head-80x24/read-again.txt show the TypeScript order and retained fold. The rendered order PNG was inspected.

[krishhgg]

PASS for head 4195b1340f3b2d32722d74383c1d75c31ff971da, patch id 59db57c804de794d872fd83d5d9986d4332df498.
Verifier: GPT-6.1 Sol (xhigh), not the author. Base 8073497d5c9cc240e4703ddfcb16144cb8d8d19c. Round 2. Scenario links, 100x30 and 80x24, truecolor.

The head and patch id are unchanged from round 1, so its behavior evidence still holds. This round checks the review gate that blocked that verdict.

Claim	Base	Head
Each slice uses desktop reading order	Patch order; head recipe fails at line 24	Rust rows and TypeScript import chain have the expected order
Keys and clicks follow displayed files at both widths	Old first file opens	New first file opens; clicks reach panel.tsx; folds survive a read again
Ordering and folding do not read the host	No control-triggered reads	Same; shared recipe prefix has identical RPC counts and diff cursors

Regression sweep: round 1's 13 checkout recipes pass on each build, and its 12 master recipes pass on each build. The settled Summary and linked-list repeats match in screens and rpcRequestsByTag. Unchanged captures differ only in run addresses, ages, spinners and reconnect timing. The feature file covers the changed file order.

Rules: doctor and live/after-stop checks pass on the reused valid runs. Local tests in round 1: 823 passed. CI on this exact head: macOS, E2E smoke, Linux build and Greptile passed.

Greptile: one thread, answered and resolved; no open threads or new findings. The author's reply explains that the pinned desktop's testedBaseName also shortens dotted names twice. Greptile withdrew the finding. This matches the desktop-source check recorded in round 1.

Binaries driven in the reused evidence, with LC_UUID:

• /tmp/t3v-bin/8073497d5c9cc240e4703ddfcb16144cb8d8d19c/t3term: E5A84254-F80A-3246-99E2-F6B1945D8CF3.
• /tmp/t3v-bin/4195b1340f3b2d32722d74383c1d75c31ff971da/t3term: AD48C4F5-18BE-3936-8B68-B9395BD61029.

Evidence: /tmp/t3v-verdict/67/round1/, especially comparison-final.json, code-head/pull-request-code/, code-negative-parent/, code-common-prefix.json and run-index.json. This round's review snapshot and CI checks are in /tmp/t3v-verdict/67/round2/.

Notes: none. No code defect found. No pull request or branch was edited, pushed, merged or closed.