feat(cli): add an in-app transcript viewer - #2999
Conversation
2ae2bcc to
ff6601e
Compare
|
Warning Review limit reached
Next review available in: 59 minutes Limit details: You’ve used all 3 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Comment |
PR Summary by Qodofeat(cli): add in-app transcript viewer for long TUI sessions
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
Code Review by Qodo
1.
|
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed exact head 3b211039fc681ef3c2eef118a88f3e07f4b19120, including the full diff, current CI, merge state, and the resolved automated feedback. I found no reproducible P0–P3 code issue.
The problem definition is correct: the canonical transcript projection remains available in memory even when terminal scrollback has discarded older rendered rows. A read-only /transcript overlay is a minimal first-principles solution. It reuses the existing projection and renderer, does not create a second session-history authority, and isolates viewer geometry/cache keys so browsing cannot mutate live scrollback reconciliation.
The prior no-op upward-scroll finding is fixed and covered. I did not find low-quality tests, removable parallel state, or a useful PR split.
The remaining stop is governance rather than code. Issue #2997 says implementation PRs should remain Draft during the repository feature freeze until maintainers decide whether and when to take the feature. This PR is currently Ready and has no maintainer approval. Please restore Draft status or record the maintainer decision before merging.
Disclosure: Codex performed the read-only source, lifecycle, test, CI, and feedback analysis. The human contributor remains responsible for independently verifying the final diff and deciding whether the feature should merge.
中文
代码层面没有 P0–P3,复用了 canonical transcript projection,没有平行 history authority。当前只因 feature-freeze 治理要求暂缓:应恢复 Draft 或取得 maintainer 明确决定。
|
Before this can merge: this is a UI/UX change (new in-app transcript viewer overlay in the TUI), so per the project's review gate it needs before/after screenshots of the visual change — a terminal capture showing the new AI-assisted review disclosure: this comment was written by a human maintainer following the review-gate rule; no AI contributed to this content. |
Review superseded by the main review comment; screenshot request communicated via comment.
Add a full-screen, read-only /transcript overlay with line, page, and boundary navigation. Keep the existing live transcript and rewind interactions unchanged while rendering the viewer through a detached geometry projection.
Give the viewer stable entry clones so its full-document renders cannot replace memoized lines that the live surface has frozen in terminal scrollback. Cover the background-update path with a regression test.
Prefer one transcript row over the navigation footer when only two terminal rows are available, and keep the displayed range valid when no body row fits.
scrollBy derived followsEnd from the scroll direction, so pressing Up on a transcript that already fit the viewport pinned the viewer at the head once it grew scrollable. Derive follow from the clamped position instead: the viewer keeps following whenever it ends at the tail, including the no-op case.
3b21103 to
33fd635
Compare
|
@Astro-Han The requested before/after terminal captures are now in the PR description. They use the same 40-line TUI integration fixture: the live screen has scrolled past lines 1-21, while /transcript + Home recovers the prompt and line 1 from the canonical projection. CLI passes 281/281 on the rebased head. Could you re-review the current head when convenient? |
Astro-Han
left a comment
There was a problem hiding this comment.
Reusing the live transcript projection while isolating geometry/cache keys is the right architecture; this does not create a second history authority. Three concrete lifecycle/routing issues remain: opening the viewer retains a second rendered transcript cache for the rest of the session, /transcript is sent to the model when a turn is running, and resize clamping can land at the tail without restoring tail-follow behavior.
AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 33fd635d1bd850902aa51ba70e21b83bd894aadf, overlay/input routing, render-cache reachability, resize behavior, and current CI. No local tests were run.
中文审查
复用 live transcript projection、只隔离 geometry/cache key 的架构是正确的,没有创建第二份 history authority。当前还有三个问题:打开 viewer 后会在整个会话中保留第二份渲染缓存;turn 运行时 /transcript 会被发送给模型;resize clamp 到尾部后不会恢复 tail-follow。
本次为 AI 辅助审查,已核验精确 head、overlay/input routing、render-cache 可达性、resize 行为和当前 CI;未运行本地测试。
Astro-Han
left a comment
There was a problem hiding this comment.
The implementation review is clean on this head: /transcript is intercepted locally during a turn, the viewer owns and releases its separate rendered cache, tail-follow/resize/tiny-view behavior is covered, all CI is green, and no unresolved code-review finding remains.
I’m using COMMENT rather than APPROVE only because the public acceptance gate in #2997 still says implementation PRs should remain Draft during the feature freeze until a maintainer decides to receive them, while this PR is currently Ready. No code change is requested here; once that maintainer decision is recorded (or the PR returns to Draft), the code itself is ready for approval.
AI-assisted review disclosure: Codex reviewed exact head 7c952e3, including the relevant TUI paths, focused tests, current CI, and review-thread state.
中文说明
当前代码审查已通过:运行中的 /transcript 会在本地处理,viewer 的独立缓存能够释放,tail-follow、resize 和极小窗口行为都有覆盖,CI 全绿,也没有未解决的代码 finding。这里使用 COMMENT 而不是 APPROVE,只因为 #2997 的公开治理条件仍要求 feature freeze 期间保持 Draft、等待 maintainer 接收决定;代码本身不需要再修改。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for this — the transcript viewer reads well and the scope is tight.
What this solves / how: the terminal scrollback window is bounded, so earlier transcript lines scroll out of reach while the canonical in-memory projection still holds them. This renders that same projection into a full-screen read-only overlay rather than introducing a second history source or touching persistence — the fix sits at the same layer as the problem.
Review notes:
- Reuses the existing
renderMakaPiTranscriptprojection; no duplicated transcript authority, no pagination state, no second source of truth. The added geometry isolation and entry clone cache are the minimum needed. - The live TUI already walks the same full projection, so large transcripts don't take a new asymptotic hit here.
- Existing review feedback on this PR is addressed at the current head.
Non-blocking note (not a finding): with a turn running, Ctrl+C inside the viewer is inert — the runner's if (tui.hasOverlay()) return undefined short-circuits before the turn-interrupt branch, and TranscriptViewerOverlay.handleInput only handles q/Esc/navigation. Users can still Esc out and then interrupt, so nothing is stuck. Worth flagging only because Ctrl+C behaviour across overlays is currently uneven: most overlays close themselves on it, user-question gets an explicit allowlist ahead of the hasOverlay() check, and this one does neither. That hasOverlay() short-circuit predates this PR, so unifying the convention belongs in its own change rather than here.
LGTM.
AI-assisted review. Findings were independently checked and an attempt was made to falsify each one before posting; a preliminary finding about the Ctrl+C convention was withdrawn during that check.
#2999 added the /transcript command and the change making midTurn a required field on MakaSlashCommand landed separately. Each was green on its own branch; main broke where they met. 'local' rather than 'refuse': showTranscriptViewer only calls tui.showOverlay, never entering runControl, so it satisfies the local contract — and mid-turn is exactly when reading back the transcript is most useful, so refusing there would remove the command's main value. Generated-by: Claude Opus 5 (Claude Code)
English
Summary
Long TUI sessions currently depend on the terminal's native scrollback. Once the terminal discards old rows, the beginning of the session is no longer reachable even though the CLI still has the transcript projection.
This adds a read-only
/transcriptviewer that:qor Escape.The normal live surface remains unpaged and keeps using terminal scrollback. The viewer renders through the existing CLI transcript projection with detached geometry and stable clone keys, so browsing cannot replace the memoized lines or viewport coordinates used by live scrollback reconciliation.
Escape intentionally closes this read-only overlay. Editing an earlier turn remains on Maka's existing double-Escape rewind flow, which branches the session and refills the prompt; the viewer does not introduce a second mutation path.
Fixes #2997
Before / after
The captures use the same 40-line TUI integration fixture. Before, the live screen has already scrolled past lines 1-21. After opening
/transcriptand pressing Home, the retained canonical projection makes the beginning reachable again.Before — live terminal scrollback only
After —
/transcriptat HomeVerification
npm --workspace packages/cli test— 281/281browses a long transcript without depending on terminal scrollbackgit diff --checksimplify-audit: no P0-P3 findingsStatus
Ready for review. The requested visual evidence is included above.
Checklist
Does this PR entail a change in behavior?
AI disclosure: Codex implemented the change under me2seeks's direction and review.
中文
概要
长 TUI 会话目前依赖终端自身的 scrollback。终端丢弃较早的行后,即使 CLI 仍保留 transcript 投影,用户也无法再回到会话开头。
本 PR 新增只读的
/transcript浏览器:q或 Escape 关闭。正常的实时界面仍不分页,继续使用终端 scrollback。浏览器复用现有 CLI transcript 投影,并使用分离的 geometry 与稳定的 clone key,因此浏览不会替换实时 scrollback reconciliation 所依赖的 memoized lines 或 viewport 坐标。
Escape 在这里只关闭只读 overlay。编辑较早轮次仍使用 Maka 现有的双击 Escape rewind:它会创建会话分支并回填 prompt;浏览器不会引入第二条修改会话的路径。
关联并关闭 #2997。
前后对比
两张截图来自同一个 40 行 TUI 集成夹具。before 中实时界面已滚过第 1–21 行;after 中打开
/transcript并按 Home 后,仍可从 canonical transcript 投影访问会话开头。Before — 仅依赖终端 scrollback
After —
/transcript跳到开头验证
npm --workspace packages/cli test— 281/281browses a long transcript without depending on terminal scrollbackgit diff --checksimplify-audit:无 P0-P3 finding状态
已就绪,可重新 review;维护者要求的视觉证据已补在上方。
AI 披露:本变更由 Codex 在 me2seeks 的指导与审核下实现。