Skip to content

feat: add overlay frame for improved UI presentation - #19

Open
BreadJS wants to merge 3 commits into
codebase:mainfrom
BreadJS:main
Open

BreadJS wants to merge 3 commits into
codebase:mainfrom
BreadJS:main

Conversation

@BreadJS

@BreadJS BreadJS commented Oct 22, 2026

Copy link
Copy Markdown

Added

  • Full-screen layout — the conversation scrolls (mouse wheel, PgUp/PgDn) while the input bar and status stay pinned. A clickable "Jump to bottom (ctrl+End)" badge shows while scrolled up, and a blank row separates the conversation from the bottom bar. CODEBASE_NO_FULLSCREEN=1 restores the inline layout.
  • Mouse selection — click-and-drag selects conversation text (inverse highlight); Ctrl+C copies it while a selection is shown (otherwise Ctrl+C works as before). The wheel keeps scrolling.
  • Resumed sessions — startup, /resume, and /rewind show the last 100 messages (with a note for earlier ones); the agent still gets the full history.
  • Write/edit diffs — ● Update(path) blocks with line numbers, context lines, and green/red backgrounds with word-level highlights.
  • Syntax highlighting — code boxes, markdown code blocks, and diffs are colored for ~190 languages (highlight.js); untagged blocks and shell commands are auto-detected.
  • Bordered modals — permissions, /model, /rewind, history search, copy picker, tournament, and questions have a rounded border.
  • Status bar meters — Context (green) and account Usage (blue, Codebase credits) 10-cell bars; both turn yellow at 75% and red at 90%. Usage refreshes at startup and after turns.
  • File reads — the transcript shows only what was read (⎿ 50 lines · 20–69 of 75) directly under the read line, not the file contents.
  • /model picker — the active model is marked with a green ✓.

Fixed

  • "Compacting context" on nearly every request: context size summed every reply's whole-request token count (a 17k-token session counted as 193k), and a finished compaction was redone on each request. The estimate now uses only the latest reply's count, and the summary is reused — and saved with the session, so a restart doesn't re-summarize.
  • Sessions saved only the latest exchange (each turn overwrote the history), so resume showed one message.
  • Code box borders: bottom line too long, missing right edge, tabs misaligned.
  • Lines with tabs could wrap and shift the screen.
  • Gap inside the status-bar meters (partial block characters).
  • Edit diffs lost syntax colors for code embedded in another language (CSS in <style>, JS in <script>); edits are now colored from the whole file, the same as writes.

- Introduced a new `overlay-frame.ts` module to handle drawing framed overlays with rounded borders.
- Updated `PermissionOverlay`, `RewindOverlay`, `TournamentOverlay`, `UserQueryOverlay` to utilize the new frame for rendering.
- Enhanced the `ScrollViewport` to manage scrolling and selection more effectively, including mouse interactions.
- Implemented syntax highlighting features in `syntax.ts` and `syntax.test.ts` for better code representation.
- Added a test HTML page for manual testing purposes.

@halfaipg halfaipg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 9004cac. Holding the merge for two reproduced runtime regressions and one cross-platform test failure below.

Local checks: TypeScript PASS, Biome PASS, build PASS. Full tests: 1,363 passed / 1 failed (edit-diff-view.test.ts on macOS). The current GitHub check covers TypeScript only, so it does not catch these cases.

Please add regression tests for streaming-to-completed tool previews and failed/aborted compaction followed by resume, and make the diff test independent of the host's temporary-path length. No CLI package was published and no production changes were made during this review.

Comment thread src/agent/agent.ts
signal,
);
const result = await compaction.compact(working, signal);
compactionMemo.record(messages, view, result.messages, result.details.collapsedMessageCount);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not memoize a failed compaction as a valid summary

CompactionEngine.summarize() catches upstream failures and returns the literal (summarization failed: ...; transcript truncated without a summary) message. This unconditional record() now caches and persists that fallback as though summarization succeeded. I reproduced this with 12 messages, a 2,048-token context, keepRecent=2, and glue.smart throwing: after save/load, the model view has only the failure message/ack and two recent messages, the earlier user constraint is absent, and needsCompaction(view) is false, so the next request does not retry. The raw transcript is still stored, but the model keeps receiving the failed summary even after restart. Only commit a memo after successful, non-aborted summarization; preserve/retry the original context on failure. Please cover the failure + resume path.

Comment thread src/ui-pi/message-view.ts
return wrap(`${frame} ${label}…`, width).map((l) => ansi.cyan(l));
}

if (status === "done" && FILE_EDIT_TOOLS.has(this.name)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Wait for confirmed completion before caching the file preview

An absent tool execution defaults to done above. Streaming toolCall blocks therefore reach this branch before tool_execution_start, and renderFileEdit caches its first preview by call id. Reproduction: render a write with partial content first and an empty tools map, then mark that same call done with final content first\nsecond\nthird\n. The completed view still says Wrote 1 line and shows only first, because it reuses the streaming preview. For edits this can also read the target file before execution/permission completion. Do not build/cache a successful preview from unknown/pending executions; use final args and confirmed success, including appropriate handling of resumed results.

Comment thread src/ui-pi/edit-diff-view.test.ts Outdated
const out = renderFileEdit("e1", "edit_file", { path: file, old_string: "three", new_string: "THREE" }, 80)?.map(
strip,
);
expect(out?.slice(1)).toEqual([

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Make the diff assertion tolerate a wrapped header

npm run check fails here on macOS. The absolute tmpdir() filename is long enough that Update(<path>) wraps at width 80, producing a second header row ( )). out.slice(1) assumes exactly one header line and compares that extra row with the diff summary. The test needs to locate the summary/diff independently of header height, or use a bounded relative fixture path; please keep the normal macOS temp path rather than requiring a special TMPDIR.

@BreadJS

BreadJS commented Oct 29, 2026

Copy link
Copy Markdown
Author

Let me know if these are the changes you were asking for @halfaipg

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.

2 participants