Repository navigation
perf(terminal): index accepted input pane ownership - #17287
Conversation
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
Greptile SummaryThe PR adds a layout-identity-scoped PTY-to-pane ownership cache so repeated accepted terminal input can update activity without repeatedly scanning every saved layout.
Confidence Score: 5/5The PR appears safe to merge because cached ownership is validated on every reuse and production ownership updates replace the cache identity. The cache preserves the prior ordered lookup on misses, rejects stale tab or leaf bindings before recording activity, and naturally reindexes when production layout ownership changes.
|
| Filename | Overview |
|---|---|
| src/renderer/src/runtime/runtime-terminal-inspection.ts | Adds an identity-scoped ownership cache with per-hit binding validation and fallback to the existing ordered layout scan; no actionable correctness defect was established. |
| src/renderer/src/runtime/runtime-terminal-inspection.test.ts | Adds focused behavioral and work-count coverage for repeated lookup, layout replacement, in-place rebinding, delayed acknowledgements, misses, inherited properties, and malformed duplicate ownership. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Accepted PTY input] --> B{Cache entry for layout identity and PTY?}
B -->|Yes| C{Cached tab, leaf, and PTY binding still valid?}
C -->|Yes| D[Record activity for cached pane key]
C -->|No| E[Evict cached owner]
B -->|No| F[Scan layouts in order]
E --> F
F --> G{Valid matching owner found?}
G -->|Yes| H[Cache owner and record activity]
G -->|No| I[Record no activity]
Reviews (1): Last reviewed commit: "perf(terminal): index accepted input pan..." | Re-trigger Greptile
ELI5
Every accepted keystroke or paste chunk marks its terminal pane as recently used so hibernation does not park an active terminal. That marker rediscovered the same pane by walking every saved terminal layout on every input.
This keeps a weak, layout-identity-scoped PTY-to-pane lookup. A repeated input validates the cached tab/leaf owner in constant time; a replaced layout gets a fresh lookup naturally, an in-place rebind evicts and rescans, and misses are not cached.
Measurement proof
Deterministic work-count oracle: 500 one-leaf layouts, target PTY in the last layout, 100 accepted input markers.
That is 100x less enumeration work for the repeated-input scenario. The production large-draft path can validate and send an allowed 16 MiB paste in more than 1,000 chunks, so this removes accumulated-layout work from a real input path rather than a synthetic background task.
Behavior / reliability sign-off
Invariant: accepted input is still recorded against the current pane owning that PTY (
terminal-input.activity-ownership), including when a remote acknowledgement arrives after ownership moves.WeakMap; only successful PTY owners are retained.terminal-session.layout-pty-ownershipunit gate passed. The lookup is provider/platform-neutral renderer code: local/daemon behavior is visibly exercised; remote-runtime ownership timing is unit-covered; SSH, WSL, Linux, and Windows behavior is unaffected because provider routing and wire data are untouched.Residual gap: production layout writers replace the outer layout record and normalize duplicate ownership. An arbitrary external mutation that inserts an earlier duplicate while retaining the cached owner cannot be observed in constant time; that shape is outside the store mutation contract. Owner removal/rebind within the same identity still evicts safely.
Visible proof
The intended
Orca: nwparker/perf-terminal-input-pane-indexdev instance created a real terminal through the UI. With 500 historical layouts seeded, the realsendRuntimePtyInputVerifiedpath accepted 25/25 characters, recorded the correct pane activity marker, and visibly printedORCA_INPUT_CACHE_OK.The isolated Electron profile fell back to a local PTY after its daemon socket failed to start. Rendered SSH/remote-runtime behavior remains a stated manual gap; deterministic routing and delayed-acknowledgement tests cover those shared semantics.
Validation
pnpm testacross 13 terminal-input and layout-ownership files: 156 passedpnpm tc:webpnpm run check:code-quality:changed: 0 findingsMerge risk
Low. This is renderer-only, adds no timers/listeners/processes/network calls, and retains no layout after that layout object becomes unreachable. A cold PTY lookup still performs the existing ordered scan; only verified repeat ownership skips it.