Conversation
`incompleteCarry` scanned the whole PTY batch — up to 64 KB — allocating a `slice` at every ESC byte before testing its length. Both patterns are anchored `^...$`, so a candidate is always a suffix, and one longer than `OSC_CARRY_LENGTH` is rejected anyway: a match cannot start before the last 24 characters. Scanning from there is output-identical. This runs once per PTY batch per pane, and agent TUIs emit hundreds of ESC bytes per KB, so it is the hottest path in the app during a fast stream. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
`baseName` was written verbatim three times and inlined twice more, and `parentDirectory` twice. Four of those copies feed user-visible names — the unsaved-file confirmation, the file-tab chips, the editor header — so a divergence shows one file under two names in two places. `src/lib/path-name.ts` is deliberately NOT `workspaceLabel`: that one is POSIX-only and names a workspace for display; these answer the raw segments of a file path, honouring both separators. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
`folderName` was a second basename for the paths `workspaceLabel` already names, and the two disagreed: it neither trimmed nor folded repeated trailing slashes, so the open board and the agent rail printed different names for the same folder. Behaviour change, stated on purpose: an empty path now reads `Unknown` rather than an empty string — which is the better string for the "is missing" line that prints it. `/foo//` reads `foo`, and a padded path loses its spaces. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
…lection Two consolidations into `head.ts`, the module already declared as the home of the scanners' bounded, symlink-safe reads. `isRegularFile` was declared in three scanners and `isDirectory` in two, so the symlink guard on files Deck did not write had four copies. `childDirs` and `datedFilesIn` replace them, and close a double `lstat`: every transcript was stat'ed once to prove it was a regular file and again for its mtime and size. Directory walks now read `d_type` through `withFileTypes`, with an `lstat` fallback for filesystems that cannot answer it — same refusal of symlinks, fewer syscalls per scan. `selectCandidate` is the second: session restore and the rail's session tail MUST pick the same file — same 30-day cutoff, same cwd predicate, same recency ranking, same greedy dedup — and `session-tail.ts` said so in its own header while hand-copying all of it, cutoff literal included. A correctness invariant held together by a comment is now one function. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
`validateWindowRecord` and `validateArchiveEntry` repeated the same fourteen lines: reject a non-object, reject a non-finite `savedAt`, reject a non-array `tabs`, then cap at `MAX_JOURNAL_TABS` and drop invalid tabs one by one. The cap and the drop rule are the two guarantees this module exists for, so two copies is two places for them to diverge invisibly. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
Every worktree re-filtered the whole tab list, and `worktreeForPath` scans all worktree paths per call — O(worktrees^2 x tabs) longest-prefix work to answer a question each tab has exactly one answer to. One pass now resolves each tab's owner once and buckets by it; order within a worktree is unchanged. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
DL-23.10 widened §23's tooltip to any icon-only chrome control with an action, and three call sites had each retyped the same four handlers. The `onPointerLeave` guard is the one that matters: focus outlives the pointer, so tabbing to a control and then moving the mouse across it must not take away the description the focus asked for. Dropping it is a silent regression, which is why it should not be retyped per call site. `tooltipTriggerProps` spreads after `feature-toolbar`'s own `expansion` bundle, which carries only ARIA attributes — no handler is overridden. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
Six exports had no reference anywhere in src/, electron/, scripts/, src-tauri/, marketing/, the tests or the gallery. Two of them actively misled: `stripFileTabsFor" was documented as "used by both chrome layouts so a single-layout change cannot half-land" and is a pass-through with no callers, and `resetMonacoLoaderForTests` is documented "Tests only" while no test calls it — a seam a future test author would have trusted. Gone: `expandDirectory`, `collapseDirectory`, `stripFileTabsFor`, `isMonacoLoaded`, `resetMonacoLoaderForTests`, `resetExternalApps`. Kept on purpose: `refreshExternalApps`, whose doc names a call site that was never wired — that is a missing wire or dead code, and the answer is a decision, not a cleanup. `.metric-table__scroll`'s scrollbar size and track were byte-identical to the base layer in `01-tokens.css`, whose own comment asks not to be re-declared. The thumb stays: an always-visible bar on a horizontally scrolling table is a legitimate override of the base layer's reveal-on-hover. Claude-Session: https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Quality-only sweep from
/simplifyacrosssrc/andelectron/. No bug hunting — that is/code-review's job. Four review agents (reuse, simplification, efficiency, altitude) surveyed the tree; the mechanical, behaviour-preserving findings on files no other session held are applied here. 30 files, +213 −343.What changed
perf(terminal)OSC progress carryincompleteCarryscanned the whole PTY batch (up to 64 KB), allocating asliceat every ESC byte before testing its length. Both patterns are anchored, so a valid candidate is always in the last 24 chars. Hottest path in the app during a fast agent stream.refactor(files)path splitterbaseNamewritten verbatim 3× and inlined 2× more,parentDirectory2×. Four copies feed user-visible names — one file could be named two ways in two places.refactor(open-board)workspace namefolderNamewas a second basename that disagreed withworkspaceLabel. Carries the one behaviour change (below).refactor(resume)scannerslstat'ed twice per scan. PlusselectCandidate: restore and the rail's session tail must pick the same file, andsession-tail.tssaid so in its header while hand-copying the whole algorithm, cutoff literal included.refactor(lib)journal envelopeperf(repositories)tab bucketingrefactor(ui)tooltip handlersonPointerLeavefocus guard whose omission is a silent regression.choredead exports + CSSThe one behaviour change
folderName→workspaceLabelin the open board (recents rows, worktree form, the "is missing" notice):""""Unknown/foo///foo/foo" /foo/bar ""bar "barEverything else is intended-identical. The OSC change is backed by a 700k-input differential fuzz against the old implementation, 0 mismatches — not just green tests.
Verification
Run on this branch in an isolated worktree, not the shared checkout:
npm run build✅ ·npm run electron:build✅ ·generate:menu:check✅tscrenderer ✅ ·tscelectron ✅npx vitest run— 3530 passed, 1 failedThe one failure (
scripts/ipc-contract.test.ts) plus two.mjsfiles that fail to load are pre-existing: reproduced identically on a pristine worktree atHEAD. No failure originates here.Not run: no
electron:dev/tauri devhost pass, no owner eye review. Every change is internal — no UI shape, no IPC payload, no R4 seam, no DL rule moved.npm run lintcould not run at all —oxlint: command not foundin this checkout. Flagging rather than claiming a clean lint.Reviewer notes
electron/resume/head.tsrenders as "Binary file not shown." Pre-existing, not introduced here:fileCacheKeyembeds 2 literal NUL bytes as separators, deliberately — "NUL-separated so no path can forge another path's key by ending in digits." It is unfortunately the file with the largest change. Reading it locally is the workaround; switching the literals to a\u0000escape sequence (behaviour-identical) or adding a.gitattributesentry would fix it, both out of scope here.refreshExternalAppswas deliberately kept despite having no callers — its doc names a call site that was never wired. That is either a missing wire (installing an app mid-session is currently never noticed) or dead code, and it is a product decision rather than a cleanup.__deckHostpresence check intobridge.tsbroke 3 tests, because each facade computing it independently is what letsworktree-host.test.tsexercise presence-detection while mockingbridge. That duplication is load-bearing for testability.Further findings the agents raised but this PR does not touch — files held by concurrent sessions, R4 seams, or fork-listed areas — are recorded in the session below.
https://claude.ai/code/session_01CotP5z7c3zry3YLbyjYE5K