Repository navigation
Conversation
- 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
left a comment
There was a problem hiding this comment.
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.
| signal, | ||
| ); | ||
| const result = await compaction.compact(working, signal); | ||
| compactionMemo.record(messages, view, result.messages, result.details.collapsedMessageCount); |
There was a problem hiding this comment.
[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.
| return wrap(`${frame} ${label}…`, width).map((l) => ansi.cyan(l)); | ||
| } | ||
|
|
||
| if (status === "done" && FILE_EDIT_TOOLS.has(this.name)) { |
There was a problem hiding this comment.
[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.
| const out = renderFileEdit("e1", "edit_file", { path: file, old_string: "three", new_string: "THREE" }, 80)?.map( | ||
| strip, | ||
| ); | ||
| expect(out?.slice(1)).toEqual([ |
There was a problem hiding this comment.
[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.
… tool execution tracking
|
Let me know if these are the changes you were asking for @halfaipg |
Added
CODEBASE_NO_FULLSCREEN=1restores the inline layout./resume, and/rewindshow the last 100 messages (with a note for earlier ones); the agent still gets the full history.● Update(path)blocks with line numbers, context lines, and green/red backgrounds with word-level highlights./model,/rewind, history search, copy picker, tournament, and questions have a rounded border.Context(green) and accountUsage(blue, Codebase credits) 10-cell bars; both turn yellow at 75% and red at 90%. Usage refreshes at startup and after turns.⎿ 50 lines · 20–69 of 75) directly under the read line, not the file contents./modelpicker — the active model is marked with a green ✓.Fixed
<style>, JS in<script>); edits are now colored from the whole file, the same as writes.