feat: add /clear-screen slash command#647
Open
VitalyOstanin wants to merge 5 commits intoPiebald-AI:mainfrom
Open
feat: add /clear-screen slash command#647VitalyOstanin wants to merge 5 commits intoPiebald-AI:mainfrom
VitalyOstanin wants to merge 5 commits intoPiebald-AI:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new "clear-screen" patch and tests: discovers an existing redraw callback that calls Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/patches/clearScreen.ts (1)
1-1: Drop the new file-level comment unless it was explicitly requested.This adds a non-essential comment on Line 1.
As per coding guidelines:
Do not add comments unless explicitly requested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/clearScreen.ts` at line 1, Remove the non-essential file-level comment at the top of src/patches/clearScreen.ts (the Line 1 comment "// Please see the note about writing patches in ./index") unless that comment was explicitly requested; simply delete that comment so the file starts with real code or the intended export/implementation.src/patches/clearScreen.test.ts (1)
13-55: Add an edge-case test for assistant messages withoutusage.Please add coverage for input where an assistant message exists but
message.usageis absent, to lock in intended fallback behavior for/clear-screen.As per coding guidelines:
Test edge cases and error conditions in test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/clearScreen.test.ts` around lines 13 - 55, Add a new test in src/patches/clearScreen.test.ts that constructs an input via makeInput() but includes an assistant message object that omits message.usage (i.e., assistant message without a usage property), then call writeClearScreen(input) and assert the call does not crash (result not null) and the patch still registers the clear-screen behavior by checking for strings like 'name:"clear-screen"' and 'globalThis.__tweakccForceRedraw' in the result; this uses the existing helpers writeClearScreen and makeInput to locate the proper insertion point and verifies the intended fallback handling for assistant messages without usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/patches/clearScreen.test.ts`:
- Line 1: Remove the redundant import line that pulls in Vitest functions and
rely on the configured globals instead: delete the import { describe, expect, it
} from 'vitest' statement and use the global describe, it, and expect
identifiers directly in the test file (ensure no other references to the
imported symbols remain).
In `@src/patches/clearScreen.ts`:
- Around line 45-49: The current $.setMessages(m=>{...}) callback drops all
messages when no assistant message has message.usage; change the logic so it
still preserves the last assistant message as a fallback: search m from the end
for an assistant with message.usage and if found return a single modified
message with its message.content cleared, otherwise locate the last message
where m[k].type === "assistant" (if any) and return that assistant message with
content emptied; only return [] if there are no assistant messages. Update the
anonymous function passed to $.setMessages (the m parameter and its message
handling) to implement this fallback.
---
Nitpick comments:
In `@src/patches/clearScreen.test.ts`:
- Around line 13-55: Add a new test in src/patches/clearScreen.test.ts that
constructs an input via makeInput() but includes an assistant message object
that omits message.usage (i.e., assistant message without a usage property),
then call writeClearScreen(input) and assert the call does not crash (result not
null) and the patch still registers the clear-screen behavior by checking for
strings like 'name:"clear-screen"' and 'globalThis.__tweakccForceRedraw' in the
result; this uses the existing helpers writeClearScreen and makeInput to locate
the proper insertion point and verifies the intended fallback handling for
assistant messages without usage.
In `@src/patches/clearScreen.ts`:
- Line 1: Remove the non-essential file-level comment at the top of
src/patches/clearScreen.ts (the Line 1 comment "// Please see the note about
writing patches in ./index") unless that comment was explicitly requested;
simply delete that comment so the file starts with real code or the intended
export/implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23f51e3c-88cb-4759-be93-549271f35167
📒 Files selected for processing (3)
src/patches/clearScreen.test.tssrc/patches/clearScreen.tssrc/patches/index.ts
VitalyOstanin
force-pushed
the
feature/clear-screen
branch
from
b5b9501 to
43d31f2
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/patches/clearScreen.ts (1)
68-74: Consider adding a delimiter prefix for regex performance.The pattern starts with
(functionwithout a leading delimiter character. Per the coding guidelines for patches, starting with literal characters like,;{}can significantly improve V8 regex matching performance.That said,
functionas a keyword is specific enough that this is unlikely to cause issues in practice.♻️ Optional: Add delimiter prefix
export const patchRenderFilter = (oldFile: string): string | null => { const pattern = - /(function [$\w]+\([$\w]+,[$\w]+\)\{)if\([$\w]+\.type!=="user"\)return!0;if\([$\w]+\.isMeta\)/; + /([,;{}])(function [$\w]+\([$\w]+,[$\w]+\)\{)if\([$\w]+\.type!=="user"\)return!0;if\([$\w]+\.isMeta\)/; const match = oldFile.match(pattern);Then adjust the replacement to include the delimiter and update capture group indices accordingly.
As per coding guidelines: "Use
,;}{literal characters at regex start instead of\\bfor performance in patch patterns"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/clearScreen.ts` around lines 68 - 74, The regex in patchRenderFilter (variable pattern) should start with a literal delimiter like ',', ';', '{', or '}' before the "function" keyword to improve V8 regex performance; update the pattern to include that leading delimiter and then adjust any replacement logic that relies on capture groups (and their indices) after oldFile.match so the correct groups are referenced in the replacement/patch operation in patchRenderFilter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/patches/clearScreen.ts`:
- Around line 68-74: The regex in patchRenderFilter (variable pattern) should
start with a literal delimiter like ',', ';', '{', or '}' before the "function"
keyword to improve V8 regex performance; update the pattern to include that
leading delimiter and then adjust any replacement logic that relies on capture
groups (and their indices) after oldFile.match so the correct groups are
referenced in the replacement/patch operation in patchRenderFilter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e45c65ff-dae3-4680-af65-29909a7429f1
📒 Files selected for processing (3)
src/patches/clearScreen.test.tssrc/patches/clearScreen.tssrc/patches/index.ts
✅ Files skipped from review due to trivial changes (1)
- src/patches/clearScreen.test.ts
Clear screen without resetting conversation context.
Recommended keybinding: ctrl+k ctrl+l (Chat context).
How it works:
1. Exposes ink's forceRedraw() via globalThis.__tweakccForceRedraw
by patching the app:redraw callback registration site
2. Registers /clear-screen as a local slash command that:
- Clears messages via setMessages, keeping last assistant message
with empty content to preserve context window usage (ctx%)
- Clears terminal scrollback with \x1b[3J
- Triggers ink repaint via forceRedraw()
- setMessages now preserves the last assistant message as fallback when no assistant message has message.usage, instead of returning []. - Add edge-case test for the fallback behavior. - Keep vitest imports: coderabbitai suggested removing them since vitest.config.ts has globals:true, but tsconfig.json lacks "types": ["vitest/globals"], so tsc --noEmit (run by pre-commit hook via pnpm lint) fails without explicit imports. All other test files in the repo use explicit imports for the same reason.
The previous implementation replaced the messages array with a single
empty-content assistant message to preserve prompt cache. This caused
two problems:
1. Claude lost all conversation context and treated the next message
as the start of a new session ("This is the first message...")
2. The prompt cache was not actually preserved — cache breakpoints
are set via cache_control in the API request, and the usage field
in the saved assistant message is a response from the previous
call, not an instruction for the next one.
The new implementation hides messages from the UI without removing
them from the messages array:
- On /clear-screen, all current message UUIDs (first 24 chars) are
collected into a globalThis.__tweakccHiddenUUIDs Set.
- The render filter function (g97) is patched to check this Set and
skip messages whose UUID prefix matches.
- Messages remain in the array and are sent to the API as usual,
preserving full conversation context.
UUID prefix (24 chars) is used because the render pipeline function
lP() splits multi-content assistant messages into separate objects
with modified UUIDs (original.slice(0,24) + hex index). Matching on
the prefix ensures both original and split messages are hidden.
The approach with a per-message flag (__tweakccHiddenFromUI) was
tried first but failed: lP() creates new objects for assistant
messages with a fixed set of fields, dropping custom properties.
User messages worked because lP() uses object spread for string
content, preserving extra fields — but assistant messages did not.
After session resume, globalThis is empty, so previously hidden
messages become visible again (acceptable trade-off).
VitalyOstanin
force-pushed
the
feature/clear-screen
branch
from
43d31f2 to
4eea753
Compare
Anchor patchRenderFilter regex with [,;{}] literal delimiter before
"function" keyword, matching the convention used by redrawPattern
and other patches. Update tests to supply delimiter in test data
and add delimiter variation coverage.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/patches/clearScreen.ts (1)
1-1: Remove the file-level comment.This repo’s TS guidelines say not to add comments unless they’re explicitly requested.
As per coding guidelines, "Do not add comments unless explicitly requested."✂️ Suggested cleanup
-// Please see the note about writing patches in ./index🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/clearScreen.ts` at line 1, Remove the file-level comment at the top of clearScreen.ts ("// Please see the note about writing patches in ./index"); simply delete that comment line so the file contains only implementation code and no repo-guideline comments per the TS guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/patches/clearScreen.ts`:
- Around line 69-84: The current regex in pattern can match different
identifiers for the `type` and `isMeta` checks causing injections against the
wrong variable; update the regex so the first function parameter is captured and
reused (via a backreference) for both `type` and `isMeta` checks (i.e. capture
the first arg name in the function param group and reference that same group
where you currently match `.type` and `.isMeta`), so that funcPrefix, firstArg,
match and replacement all tie to the same identifier; adjust the pattern
variable and the logic that derives firstArg from match groups (instead of
re-parsing match[0]) so replacement uses the guaranteed-first-parameter
identifier.
---
Nitpick comments:
In `@src/patches/clearScreen.ts`:
- Line 1: Remove the file-level comment at the top of clearScreen.ts ("// Please
see the note about writing patches in ./index"); simply delete that comment line
so the file contains only implementation code and no repo-guideline comments per
the TS guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b22168a5-3f55-4c15-bf05-d5727bc58a11
📒 Files selected for processing (3)
src/patches/clearScreen.test.tssrc/patches/clearScreen.tssrc/patches/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/patches/index.ts
Use backreference \3 to ensure type/isMeta checks reference the same identifier as the function's first parameter. Extract firstArg directly from match[3] instead of re-parsing match[0].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
/clear-screenslash command that clears the screen without resetting conversation context.messagesarray (full API context preserved)g97globalThis.__tweakccForceRedrawctrl+k ctrl+l(Chat context)How it works
Three-part patch:
forceRedraw()— patches theapp:redrawcallback registration site to assignglobalThis.__tweakccForceRedrawpointing to the ink instance'sforceRedraw()methodg97— injects a check at the top of the render visibility function: if a message's UUID prefix is inglobalThis.__tweakccHiddenUUIDs, returnfalse(hide from UI). This works for both user and assistant messages, including multi-content assistant messages whose UUIDs are derived viam$(first 24 chars stay the same)/clear-screencommand — atype:"local"slash command that collects all current message UUID prefixes into aSetonglobalThis.__tweakccHiddenUUIDs, clears terminal scrollback, and triggers ink repaintWhy UUID-based hiding instead of replacing messages
The initial approach replaced the messages array via
setMessages, keeping only the last assistant message withcontent: []to preserve ctx%. This caused Claude to lose conversation context — it would respond with "This is the first message in our session" becausesetMessagesis the sole source for API messages. The UUID-based approach leaves all messages intact in the array (preserving full API context) and only hides them from ink rendering.Test plan
/clear-screenclears visible messages while preserving full conversation context/clear-screen, Claude remembers prior conversation (no "first message in session")ctrl+k ctrl+lkeybinding triggers the command (Chat context)writeClearScreenandpatchRenderFilter: success, already patched, pattern not found, context preservation, delimiter variants, different function/argument names)Summary by CodeRabbit
New Features
clear-screenslash command that clears the terminal and triggers a screen redraw, with wiring to message/UUID filtering to avoid affecting hidden items.Tests
Chores