Skip to content

refactor(terminal): move the chat lens's terminal logic into hooks - #1767

Merged
devswha merged 1 commit into
mainfrom
refactor/terminal-chat-lens-hooks
Oct 23, 2026
Merged

devswha merged 1 commit into
mainfrom
refactor/terminal-chat-lens-hooks

Conversation

@devswha

@devswha devswha commented Oct 23, 2026

Copy link
Copy Markdown
Owner

PR 3 of 3 of the PaneTerminal.tsx split. The plan is .omo/plans/paneterminal-split.md in the maintainer's workspace, and plan-reviewer approved it. It follows PR #1758 and PR #1764.

This PR moves the chat lens's terminal logic out of src/components/PaneTerminal.tsx into three hooks. Each block moves as one unchanged span. Effects keep their dependency lists, and callbacks keep their useCallback dependencies.

src/components/PaneTerminal.tsx goes from 1,477 to 1,174 lines. Over the three PRs it went from 3,665.

What moves

Hook Block Deps Returns Called
src/lib/usePeekReturn.ts after a peek: the manual-peek and auto-return state, the still-open note, the input-row frame watch, and the peek-end layout effect (153 lines) 29 12 names where the block stood
src/lib/useChatCallbacks.ts the chat lens's callbacks and state: conversation watch, terminal output, sent and metadata, greeting room, prompt, suggestion, restore and pending answer (90 lines) 12 28 names where the block stood, right after usePeekReturn
src/lib/useTypedKeys.ts the key bar's pressKey and toggleModifier (30 lines), and Hermes' one-Ctrl+C stop guard with abortTurn (34 lines) 16 pressKey, toggleModifier, abortTurn where the stop block stood

Hook and effect order

  • useTypedKeys: the key-bar callbacks used to sit about 15 lines above the stop block. They have no effects, and only JSX reads them, so creating them a few hooks later changes nothing. Nothing between the two old positions reads them.
  • usePeekReturn and useChatCallbacks: each sits exactly where its block was, so their effects run in the same order as before.

Other changes

  • StillOpen: this was a local type alias inside the component. Two hooks now use it, so it moves to src/lib/paneTerminalShared.ts as an exported type. Types have no runtime effect.
  • src/lib/i18n.test.ts: PART_FILES.terminal gains lib/usePeekReturn.ts and lib/useTypedKeys.ts, because their t() keys belong to the terminal part.
  • Imports: PaneTerminal.tsx drops the imports only the moved code used.

Checks

  • --color-moved: git diff --color-moved=plain --color-moved-ws=allow-indentation-change origin/main...HEAD shows 629 lines as moved. The other 116 lines are:
    • PaneTerminal.tsx's narrowed imports, the three new imports and the three calls;
    • the hooks' imports, three Deps interfaces (57 field lines), signatures, destructuring lines and returns;
    • the StillOpen type's move;
    • the two PART_FILES entries.
  • Moved blocks are exact: lines 646-735, 819-848 and 863-896 of PaneTerminal.tsx on 2fa93ab49 appear byte for byte in their hooks. Lines 493-645 do too, apart from the one StillOpen line.
  • Typecheck: bun run typecheck passes.
  • Unit tests: HERDR_TEST_MODE=unit bun test src/conventions.test.ts src/lib/i18n.test.ts src/lib/settings.test.ts scripts/conventions.test.ts src/lib/terminalPeek.test.ts src/lib/keyBar.test.ts src/lib/hermesStop.test.ts: 163 pass, 0 fail.
  • bun run check fast: passes after the rebase (unit 6253 pass, 0 fail, 13 skip in 443 files, 8 at a time, 57.5s)
  • bun run check integration browser: the browser lane exits 0 (every browser script passed, 1895 s), run on this commit before its rebase onto the latest main (which changed only src/App.tsx, src/app-hooks.test.ts, src/lib/useSidebarLayout.ts and server files). The integration lane's only failure is server/api.contract.test.ts "creates, edits, searches, inspects, and closes an owned workspace" (line 1540, /api/pane/files returns []), which fails the same way on unchanged origin/main on this PC. CI runs the integration lane in its own environment.

Outside PRs that touch the same files

No other maintainer PR touches these files. One outside contributor's PR touches src/components/PaneTerminal.tsx: #1639. It already conflicts with main (src/lib/i18n.ja.ts, src/lib/i18n.ko.ts, src/lib/i18n.zh.ts) and needs a rebase either way; against this branch it conflicts in src/components/PaneTerminal.tsx, src/lib/i18n.ja.ts, src/lib/i18n.ko.ts, src/lib/i18n.zh.ts.

The moved blocks include the queued-input stop path, Hermes' stop guard and the peek's auto-return. A code-review agent's stand-in review may be wanted before the merge.

Move-only: moved blocks plus imports, deps interfaces, three calls, one shared type and two PART_FILES entries. No changelog fragment: users see no change.

The peek-return block, the chat callbacks and the key-bar/stop callbacks move verbatim
into usePeekReturn, useChatCallbacks and useTypedKeys. The first two are called where
their blocks stood, useTypedKeys where the stop block stood (pressKey/toggleModifier have
no effects, so creating them later is neutral). PART_FILES gains the files with t() keys.
Plan: .omo/plans/paneterminal-split.md PR 3.
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 53b8512 and 7e384af.

📒 Files selected for processing: 6
  • src/components/PaneTerminal.tsx
  • src/lib/i18n.test.ts
  • src/lib/paneTerminalShared.ts
  • src/lib/useChatCallbacks.ts
  • src/lib/usePeekReturn.ts
  • src/lib/useTypedKeys.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Refactor
    • Terminal keyboard controls, turn stopping, chat prompts, and focus handling are now managed consistently across terminal and chat interactions.
    • Existing terminal peek behavior remains available, including warnings when a screen may receive the next message.
    • No changes were made to the component’s public interface.

PaneTerminal now uses dedicated hooks for peek-return tracking, chat callbacks, and terminal key handling. The new hooks contain the extracted state and behavior. The shared StillOpen type describes pane-screen notice data.

Changes: PaneTerminal hooks

Layer / File(s) Summary
Chat callback state
src/lib/useChatCallbacks.ts
useChatCallbacks manages conversation, greeting, prompt-answer, metadata, suggestion, and restored-message state and callbacks.
Peek-return tracking and focus
src/lib/paneTerminalShared.ts, src/lib/usePeekReturn.ts, src/components/PaneTerminal.tsx, src/lib/i18n.test.ts
StillOpen describes open-screen notice data. usePeekReturn tracks peeks, input-row frames, notices, and focus behavior. PaneTerminal uses the hook’s state and callbacks. The terminal test file list includes the new hook files.
Terminal key and abort handling
src/lib/useTypedKeys.ts, src/components/PaneTerminal.tsx
useTypedKeys handles key-bar input, modifier toggles, and turn-abort logic. PaneTerminal delegates its key handlers to the hook.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 7e384

This is a refactor of terminal logic into hooks with no intended user-visible change. No concrete regression was found in the supplied files. It looks safe to merge after the normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: moving the terminal logic into hooks.
Description check Passed The description explains the change, identifies the extracted hooks, documents the move-only intent, and lists detailed validation results. It uses a "Checks" heading instead of the template's "Valida…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the change, identifies the extracted hooks, documents the move-only intent, and lists detailed validation results. It uses a "Checks" heading instead of the template's "Validation" heading, but the required validation information is present.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devswha

devswha commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Merge pass: PaneTerminal split, PR 3 of 3. It merges on its checks as a move-only PR, without waiting for a CodeRabbit review, as AGENTS.md allows.

One relocation. pressKey/toggleModifier move about 15 lines down, into useTypedKeys. Only JSX reads them.

Diff. A line-multiset comparison differs only in:

  • imports;
  • the three Deps interfaces and their destructures;
  • the three calls and their return destructures;
  • export on StillOpen;
  • i18n PART_FILES.

Gate review (on a different model than the author's):

  • Putting the hook bodies back in place of their calls, and undoing the permitted key-bar relocation, rebuilds origin/main's component byte for byte.
  • Callback deps, ref captures, effect order and the Hermes one-Ctrl+C stop guard are unchanged.

Tests at this head:

  • 76 unit tests pass: conventions, i18n, hermesStop, keyBar, terminalPeek.
  • The typecheck passes (merge-pass run).
  • Browser regressions sticky-modifiers-regression and terminal-autoswitch-demo-regression pass.

Overlap. Outside contributor #1639 touches src/components/PaneTerminal.tsx and will need a rebase.

All required checks are green and no review thread is open.

@devswha
devswha merged commit 5e44913 into main Oct 23, 2026
9 checks passed
@devswha
devswha deleted the refactor/terminal-chat-lens-hooks branch October 23, 2026 14:50
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