-
Notifications
You must be signed in to change notification settings - Fork 65
Add customizable completion verbs feature #362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Introduces a new 'completionVerbs' setting for customizing the list of past-tense verbs shown after thinking completes. Adds UI for editing these verbs, updates types, menu, and patching logic to support the new feature.
📝 WalkthroughWalkthroughThis PR adds a feature allowing users to customize the past-tense completion verbs displayed after thinking completes (e.g., "Baked for 42s"). It introduces configuration storage, a new UI component for managing the verb list with CRUD operations, integration with the settings patch pipeline, and menu navigation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new feature for customizing the list of past-tense verbs displayed after Claude Code finishes thinking (e.g., "Baked for 42s"). The implementation mirrors the existing "Thinking verbs" feature but focuses on completion/past-tense verbs.
Changes:
- Adds a new
completionVerbssetting with a configurable list of past-tense verbs - Creates a dedicated UI view for editing completion verbs with keyboard shortcuts (e/d/n/ctrl+r)
- Implements patching logic to replace the default completion verbs in Claude Code's minified JavaScript
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds CompletionVerbsConfig interface and COMPLETION_VERBS menu item enum |
| src/defaultSettings.ts | Defines default completion verbs array (Baked, Brewed, Churned, etc.) |
| src/ui/components/MainMenu.tsx | Adds "Completion verbs" menu item with descriptive text |
| src/ui/components/CompletionVerbsView.tsx | New full-featured editor component for managing completion verbs |
| src/ui/App.tsx | Integrates CompletionVerbsView into the app routing |
| src/patches/completionVerbs.ts | Implements regex-based patching to replace verbs in minified code |
| src/patches/thoughtVerbs.ts | Duplicate file that should be removed |
| src/patches/index.ts | Integrates completion verbs patching into the main apply flow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/patches/completionVerbs.ts:
- Around line 1-54: Delete the unused duplicate file thoughtVerbs.ts (it’s
identical to completionVerbs.ts and not imported anywhere), remove any stray
imports or references to thoughtVerbs.ts, and ensure the codebase continues to
use completionVerbs.ts (the existing functions like getCompletionVerbsLocation
and writeCompletionVerbs remain unchanged); after deletion, run a quick
build/test to verify there are no missing imports.
In @src/patches/index.ts:
- Line 46: The duplicate/incorrect export writeCompletionVerbs in
thoughtVerbs.ts should be removed or renamed: either delete thoughtVerbs.ts if
it was created in error (and remove any imports if present) or rename the export
to writeThoughtVerbs and implement distinct logic for thought verbs; update any
usage to call writeThoughtVerbs instead of writeCompletionVerbs and ensure
src/patches/index.ts imports the correct symbol from completionVerbs.ts (or the
renamed thoughtVerbs export) so there are no unused or misnamed exports.
In @src/patches/thoughtVerbs.ts:
- Around line 1-55: The file exports writeCompletionVerbs but is a duplicate
that targets past-tense "ed" verbs; either remove the dead file or convert it to
a proper thought-verb patch: rename the export to writeThoughtVerbs, update
getCompletionVerbsLocation (rename to getThoughtVerbsLocation) to use an "ing"
regex (match present-participle words e.g.
/[,;]([$\w]+)=\[(?:"[^"{}()]+ing",)+"[^"{}()]+ing"\]/s), update variable names
(varName -> thoughtVarName, verbsJson -> thoughtVerbsJson) and error messages to
reference "thought verbs" instead of "completion verbs", and finally add the new
writeThoughtVerbs export into the patch pipeline in src/patches/index.ts (or
delete the file and remove any references) so the module is no longer unused.
🧹 Nitpick comments (4)
src/patches/completionVerbs.ts (3)
11-12: Regex requires at least two verbs in the array.The pattern
(?:"[^"{}()]+ed",)+requires at least one comma-separated verb before the final verb. A single-verb array like["Worked"]would not match.If users should be able to configure just one verb, consider adjusting:
Proposed fix to support single-verb arrays
const completionVerbsPattern = - /[,;]([$\w]+)=\[(?:"[^"{}()]+ed",)+"[^"{}()]+ed"\]/s; + /[,;]([$\w]+)=\[(?:"[^"{}()]+ed",)*"[^"{}()]+ed"\]/s;
15-15: Use strict equality.Prefer
!== undefinedover!= undefinedto avoid type coercion.Proposed fix
- if (match && match.index != undefined) { + if (match && match.index !== undefined) {
37-38: Add a guard for undefinedvarName.If
location.identifierswere unexpectedly empty, the output would beundefined=[...]. Consider adding a defensive check:Proposed fix
const varName = location.identifiers?.[0]; + if (!varName) { + console.error('patch: completion verbs: no identifier found'); + return null; + } const verbsJson = `${varName}=${JSON.stringify(verbs)}`;src/ui/components/CompletionVerbsView.tsx (1)
207-211: Add guard for empty verbs array in preview.If
verbsis empty (e.g., corrupted settings),verbs[selectedVerbIndex]would beundefined. Consider a fallback:Proposed fix
<Text> <Text color={claudeColor}> - {verbs[selectedVerbIndex]} for 42s + {verbs[selectedVerbIndex] ?? 'Worked'} for 42s </Text> </Text>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/defaultSettings.tssrc/patches/completionVerbs.tssrc/patches/index.tssrc/patches/thoughtVerbs.tssrc/types.tssrc/ui/App.tsxsrc/ui/components/CompletionVerbsView.tsxsrc/ui/components/MainMenu.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
src/patches/completionVerbs.ts (2)
src/patches/index.ts (2)
LocationResult(72-76)showDiff(90-128)src/patches/thoughtVerbs.ts (1)
writeCompletionVerbs(28-54)
src/ui/components/CompletionVerbsView.tsx (3)
src/ui/App.tsx (1)
SettingsContext(32-38)src/utils.ts (1)
getCurrentClaudeCodeTheme(39-49)src/defaultSettings.ts (1)
DEFAULT_SETTINGS(3-778)
src/patches/thoughtVerbs.ts (2)
src/patches/index.ts (2)
LocationResult(72-76)showDiff(90-128)src/patches/completionVerbs.ts (1)
writeCompletionVerbs(28-54)
src/ui/App.tsx (1)
src/ui/components/CompletionVerbsView.tsx (1)
CompletionVerbsView(14-216)
src/patches/index.ts (2)
src/patches/completionVerbs.ts (1)
writeCompletionVerbs(28-54)src/patches/thoughtVerbs.ts (1)
writeCompletionVerbs(28-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (8)
src/ui/App.tsx (1)
6-6: LGTM!The integration of CompletionVerbsView follows the established pattern for other views. The import, routing logic, and rendering are all consistent with the existing codebase.
Also applies to: 153-153, 219-220
src/ui/components/MainMenu.tsx (1)
16-19: LGTM!The new menu item is well-positioned after Thinking verbs and includes a clear, user-friendly description with a helpful example.
src/patches/index.ts (1)
529-534: LGTM!The patch application logic is correct and follows the established pattern. The conditional check and result handling are consistent with other patches in the file.
src/types.ts (1)
74-76: LGTM!The type definitions are well-structured and consistent with the existing codebase. The
CompletionVerbsConfiginterface appropriately includes only theverbsarray without theformatfield (which makes sense for past-tense completion verbs vs. present-tense thinking verbs).Also applies to: 137-137, 194-194
src/defaultSettings.ts (1)
721-732: LGTM!The default completion verbs are well-chosen past-tense forms that match the whimsical style of the project. The project already declares UTF-8 encoding in
.editorconfigand uses other Unicode characters throughout (e.g., "Flambéing", "Sautéing" in pastTenseVerbs, and emoji in thinkingStyle). Modern TypeScript/JavaScript environments handle this without issue.src/ui/components/CompletionVerbsView.tsx (3)
14-33: LGTM!Good defensive programming with the theme fallback chain and default color value.
78-89: Verify the delete selection adjustment logic.The delete logic reads the old
verbs.lengthbefore the settings update propagates. While this appears to work correctly (using old length consistently for both the check and the new index calculation), consider adding a comment to clarify this relies on the stale closure value, or restructure to be more explicit.
137-176: LGTM!The sliding window logic for the verb list is well implemented with proper bounds checking and clear visual indicators for overflow.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/patches/completionVerbs.ts:
- Line 15: In the if condition inside completionVerbs.ts that reads "if (match
&& match.index != undefined)" use a strict equality check: change the non-strict
!= to !== so the condition becomes "match.index !== undefined"; update any
similar checks in the surrounding function (the block referencing match and
match.index) to use strict equality/inequality for type-safety.
- Around line 11-12: The regex stored in completionVerbsPattern is too strict
(requires at least two quoted strings); update it to allow zero or more quoted
strings inside the brackets and drop the unnecessary /s flag. Replace the
current pattern with one that matches \[ then an optional group of a
quoted-string followed by zero-or-more comma+quoted-string (e.g., use an
optional non-capturing group like (?:\"[^\"]+\"(?:,\"[^\"]+\")*)? ), then \] so
it will match [], ["Single"], and ["A","B","C"]; keep the existing surrounding
[,;] capture as-is.
🧹 Nitpick comments (3)
src/patches/completionVerbs.ts (3)
11-12: Minor improvements for code quality.Two optional refinements:
- Line 12: The
/s(dotall) flag is unnecessary since the pattern doesn't use the.metacharacter.- Line 24: Error message mentions "completionVerbsMatch" but no such variable exists. Consider:
'patch: completion verbs: failed to find verbs array'Also applies to: 24-24
28-31: Add input validation for theverbsparameter.The function doesn't validate that
verbsis an array or check its contents. While empty arrays should be allowed (per PR objectives to hide verbs), the parameter should still be validated to prevent runtime errors.🛡️ Proposed validation
export const writeCompletionVerbs = ( oldFile: string, verbs: string[] ): string | null => { + if (!Array.isArray(verbs)) { + console.error('patch: completion verbs: verbs must be an array'); + return null; + } + if (verbs.some(v => typeof v !== 'string')) { + console.error('patch: completion verbs: all verbs must be strings'); + return null; + } const location = getCompletionVerbsLocation(oldFile);
37-38: Be defensive about undefinedvarName.While
location.identifiers?.[0]is safe due to optional chaining, if it returnsundefined, line 38 will produce"undefined=[...]", which is invalid syntax.🛡️ Proposed defensive check
const varName = location.identifiers?.[0]; + if (!varName) { + console.error('patch: completion verbs: failed to extract variable name'); + return null; + } const verbsJson = `${varName}=${JSON.stringify(verbs)}`;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/patches/completionVerbs.tssrc/patches/thoughtVerbs.tssrc/ui/components/CompletionVerbsView.tsx
✅ Files skipped from review due to trivial changes (1)
- src/patches/thoughtVerbs.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/components/CompletionVerbsView.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/patches/completionVerbs.ts (1)
src/patches/index.ts (2)
LocationResult(72-76)showDiff(90-128)
🔇 Additional comments (1)
src/patches/completionVerbs.ts (1)
1-3: LGTM!Imports are correctly structured and the reference comment is helpful.
Refines the regular expression used to match completion verbs, improving accuracy and performance. Also updates a strict equality check for match index.
Introduces a new 'completionVerbs' setting for customizing the list of past-tense verbs shown after thinking completes. Adds UI for editing these verbs, updates types, menu, and patching logic to support the new feature.
Resolves #359
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.