Revert terminal scroll position saving by Kitenite · Pull Request #742 · superset-sh/superset

Kitenite · 2026-01-14T00:02:06Z

Summary

Reverts PR fix(desktop): restore terminal scroll position when reattaching #698: "fix(desktop): restore terminal scroll position when reattaching"
Reverts PR fix(desktop): scroll terminals to bottom by default #728: "fix(desktop): scroll terminals to bottom by default"

This removes the terminal scroll position saving/restoring functionality.

Test plan

Verify terminals no longer save scroll position when switching tabs
Verify terminals behave correctly without scroll position persistence

Summary by CodeRabbit

Refactor
- Simplified terminal session management by streamlining viewport handling operations and cleanup processes.
- Reduced complexity in terminal detach operations and scroll position management.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

This reverts commit 3f1d1ce.

…ng (#698)" This reverts commit 212aaa8.

coderabbitai · 2026-01-14T00:02:20Z

📝 Walkthrough

Walkthrough

This PR removes viewportY (viewport position tracking) from the terminal subsystem across multiple layers: type definitions, the terminal manager, the tRPC router, and UI components. This includes eliminating scroll restoration utilities and updating API signatures to no longer pass or return viewport position data.

Changes

Cohort / File(s)	Summary
Terminal Type Definitions `apps/desktop/src/main/lib/terminal/types.ts`	Removed optional `viewportY?: number` property from `TerminalSession` and `SessionResult` interfaces.
Terminal Manager `apps/desktop/src/main/lib/terminal/manager.ts`	Removed `viewportY` from the `SessionResult` return object in `createOrAttach` and removed the optional `viewportY` parameter from the `detach` method signature.
tRPC Router `apps/desktop/src/lib/trpc/routers/terminal/terminal.ts`	Removed `viewportY` from the `createOrAttach` mutation return payload and removed `viewportY` from the `detach` mutation input schema.
Renderer Component & Utilities `apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`, `utils.ts`	Removed scroll restoration logic and `viewportY` handling from the detach call; removed exported utility functions `getScrollOffsetFromBottom` and `restoreScrollPosition`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

fix(desktop): scroll terminals to bottom by default #728 — Directly inverse: this PR removes getScrollOffsetFromBottom, restoreScrollPosition, and viewportY functionality that the related PR introduced.
persist process history #120 — Modifies the same createOrAttach API and terminal session shape, altering the public data contract.
fix(desktop): restore terminal scroll position when reattaching #698 — Directly inverse: the related PR added viewportY to createOrAttach return and detach input; this PR removes it.

Poem

🐰 A viewport once tracked where we'd scroll,
But now we let go of that role—
With viewportY gone and its friends,
The terminal's journey transcends!
Simpler, cleaner, off we go. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Revert terminal scroll position saving' accurately describes the main change—removing terminal scroll position saving/restoring functionality across multiple files.
Description check	✅ Passed	The description clearly explains the purpose (reverting two PRs), what is being removed, and includes a test plan section as required by the template.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab29c5 and 39bfa8c.

📒 Files selected for processing (5)

apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
apps/desktop/src/main/lib/terminal/manager.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts

💤 Files with no reviewable changes (3)

apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/utils.ts
apps/desktop/src/main/lib/terminal/types.ts
apps/desktop/src/lib/trpc/routers/terminal/terminal.ts

🧰 Additional context used

📓 Path-based instructions (6)

apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
Use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

apps/desktop/src/main/lib/terminal/manager.ts
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx

**/*.{ts,tsx}