feat: Add voice mode patch (tengu_amber_quartz + tengu_sotto_voce)#587
feat: Add voice mode patch (tengu_amber_quartz + tengu_sotto_voce)#587georpar merged 7 commits intoPiebald-AI:mainfrom
Conversation
Force-enable the /voice slash command by bypassing the tengu_amber_quartz feature gate, and optionally enable the tengu_sotto_voce concise output mode for voice interactions. Requires Claude.ai OAuth login and SoX (or native audio capture binary). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 "voice mode" feature: two misc settings, UI defaults and types, a new patch module that force-enables voice gates (amber_quartz and optional sotto voce), tests for the patcher, and integration of the patch into the patch pipeline. Changes
Sequence DiagramsequenceDiagram
participant Config as Configuration (misc flags)
participant PatchIndex as Patch System
participant VoicePatch as Voice Mode Module
participant File as File/Source
Config->>PatchIndex: Read enableVoiceMode & enableVoiceSottoVoce
alt enableVoiceMode = true
PatchIndex->>VoicePatch: writeVoiceMode(file, enableVoiceSottoVoce)
VoicePatch->>VoicePatch: patchAmberQuartz(file) — inject unconditional return
alt enableVoiceSottoVoce = true
VoicePatch->>VoicePatch: patchSottoVoce(file) — replace gate with if(!0)
end
VoicePatch-->>PatchIndex: Return modified file
PatchIndex->>File: Write modified file
else
PatchIndex-->>File: Skip voice-mode patch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/patches/voiceMode.ts (3)
28-28: Circular import through re-export.Importing
showDifffrom./indexcreates a circular dependency (index.ts imports from voiceMode.ts, voiceMode.ts imports from index.ts). While this works becauseshowDiffis a static function re-exported from./patchDiffing, importing directly from the source module would be cleaner and avoid potential initialization order issues.🔧 Suggested change
-import { showDiff } from './index'; +import { showDiff } from './patchDiffing';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/voiceMode.ts` at line 28, voiceMode.ts currently imports showDiff from ./index which creates a circular import with index.ts; change the import to pull showDiff directly from its original module (the export source, e.g., ./patchDiffing) so voiceMode.ts imports showDiff from the concrete module that defines it (symbol: showDiff) instead of re-export via index to avoid initialization-order issues.
51-75: Consider using minified boolean form for consistency.Line 60 uses
if(true)but line 41 usesreturn !0;(minified form). For consistency with minified code style and to avoid potential detection by any minification-aware tooling, consider using the minified form.🔧 Suggested change for consistency
- const replacement = 'if(true)'; + const replacement = 'if(!0)';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/voiceMode.ts` around lines 51 - 75, The replacement string in patchSottoVoce currently uses "if(true)" which is inconsistent with the minified boolean style elsewhere (e.g., "return !0;"); update the replacement constant inside the patchSottoVoce function to use the minified form "if(!0)" (and ensure showDiff is still passed the same replacement variable) so the patched output matches the existing minified code style.
30-49: Consider adding word boundary anchor for regex performance.The regex starts with
functionwhich works but may be slower than anchoring with a literal character. Per coding guidelines, starting patterns with,;}{can significantly improve matching performance. Since function declarations typically follow}or;in minified code, consider:🔧 Optional performance improvement
- const pattern = /function [$\w]+\(\)\{return [$\w]+\("tengu_amber_quartz"/; + const pattern = /[;}]function ([$\w]+)\(\)\{return [$\w]+\("tengu_amber_quartz"/;Note: If the function could appear at the start of a chunk, the current pattern is acceptable.
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/voiceMode.ts` around lines 30 - 49, The regex in patchAmberQuartz uses /^function / without a leading literal anchor which can be slower in minified patches; update the pattern variable so it expects a preceding literal character or start-of-string (for example, prepend a non-capturing optional group like (?:[,\;{}\n]|^) before the existing function token) so the match is anchored to a nearby literal delimiter while still allowing the function to appear at the start of the chunk; keep the rest of the logic (insertIndex, insertion, showDiff, return newFile) unchanged and ensure the updated pattern is referenced where pattern is used to compute insertIndex.
🤖 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/ui/components/MiscView.tsx`:
- Around line 84-85: The defaultMisc object defines enableVoiceMode and
enableVoiceSottoVoce but the items array in MiscView (the MiscItem entries)
lacks corresponding UI toggles; add two MiscItem entries to the items array
following the same pattern used for other boolean feature flags (e.g., the
existing enableWorktreeMode/enableContextLimitOverride entries) with keys/ids
matching enableVoiceMode and enableVoiceSottoVoce, user-facing labels (e.g.,
"Enable Voice Mode" and "Enable Sotto Voce"), and boolean toggle behavior so
users can change them via the UI; if these flags are intentionally hidden mark
them with a short inline comment near defaultMisc explaining why instead of
adding UI controls.
---
Nitpick comments:
In `@src/patches/voiceMode.ts`:
- Line 28: voiceMode.ts currently imports showDiff from ./index which creates a
circular import with index.ts; change the import to pull showDiff directly from
its original module (the export source, e.g., ./patchDiffing) so voiceMode.ts
imports showDiff from the concrete module that defines it (symbol: showDiff)
instead of re-export via index to avoid initialization-order issues.
- Around line 51-75: The replacement string in patchSottoVoce currently uses
"if(true)" which is inconsistent with the minified boolean style elsewhere
(e.g., "return !0;"); update the replacement constant inside the patchSottoVoce
function to use the minified form "if(!0)" (and ensure showDiff is still passed
the same replacement variable) so the patched output matches the existing
minified code style.
- Around line 30-49: The regex in patchAmberQuartz uses /^function / without a
leading literal anchor which can be slower in minified patches; update the
pattern variable so it expects a preceding literal character or start-of-string
(for example, prepend a non-capturing optional group like (?:[,\;{}\n]|^) before
the existing function token) so the match is anchored to a nearby literal
delimiter while still allowing the function to appear at the start of the chunk;
keep the rest of the logic (insertIndex, insertion, showDiff, return newFile)
unchanged and ensure the updated pattern is referenced where pattern is used to
compute insertIndex.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2674a2e3-1a43-4e0c-9935-7dc7a14d8ad7
📒 Files selected for processing (5)
src/defaultSettings.tssrc/patches/index.tssrc/patches/voiceMode.tssrc/types.tssrc/ui/components/MiscView.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/voiceMode.test.ts`:
- Around line 29-35: The test should both assert the error path and ensure the
spy is always restored: add an expectation that the console.error spy
(consoleError) was called after invoking writeVoiceMode('const x=1;', false),
and make the mock cleanup failure-safe by restoring the spy in a finally block
(or move the spy setup/restore into an afterEach). Specifically, in the test
that calls writeVoiceMode, add expect(consoleError).toHaveBeenCalled() (or a
toHaveBeenCalledWith if you want to verify the message) and wrap the call and
assertions in try { ... } finally { consoleError.mockRestore(); } (or use
afterEach to call consoleError.mockRestore()) so mockRestore always runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee766c53-76fc-41cc-83ef-2d1ba769aaad
📒 Files selected for processing (2)
src/patches/voiceMode.test.tssrc/patches/voiceMode.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/patches/voiceMode.ts
|
All coderabbit comments addressed |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/patches/voiceMode.test.ts (1)
6-28: Consider adding a test forenableSottoVoce=falsewhen both gates are present.The current tests verify:
- Both gates +
enableSottoVoce=true→ both patched- Only amber_quartz +
enableSottoVoce=false→ amber_quartz patchedMissing scenario: both gates present but
enableSottoVoce=false— this would confirm the flag correctly leaves sotto_voce unchanged while still patching amber_quartz.Optional test case
it('patches only amber quartz when sotto voce is disabled', () => { const file = 'const x=1;' + 'function qX_(){return A9("tengu_amber_quartz",!1)}' + 'if(A9("tengu_sotto_voce",!1))return`# Output efficiency...`'; const result = writeVoiceMode(file, false); expect(result).not.toBeNull(); expect(result).toContain('function qX_(){return !0;return A9('); expect(result).toContain('if(A9("tengu_sotto_voce",!1))'); // unchanged });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patches/voiceMode.test.ts` around lines 6 - 28, Add a unit test that covers the missing scenario where both gates appear in the source but enableSottoVoce is false: call writeVoiceMode(file, false) on the same input used in the first test (both tengu_amber_quartz and tengu_sotto_voce present) and assert that amber_quartz is patched (e.g., contains the '!0' replacement in the function body) while sotto_voce remains unchanged (e.g., still contains the original A9("tengu_sotto_voce",!1) check); place this new test alongside the existing tests in src/patches/voiceMode.test.ts and use the same file string and assertion style as the other cases to keep consistency.
🤖 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/voiceMode.test.ts`:
- Around line 6-28: Add a unit test that covers the missing scenario where both
gates appear in the source but enableSottoVoce is false: call
writeVoiceMode(file, false) on the same input used in the first test (both
tengu_amber_quartz and tengu_sotto_voce present) and assert that amber_quartz is
patched (e.g., contains the '!0' replacement in the function body) while
sotto_voce remains unchanged (e.g., still contains the original
A9("tengu_sotto_voce",!1) check); place this new test alongside the existing
tests in src/patches/voiceMode.test.ts and use the same file string and
assertion style as the other cases to keep consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dce3102-c1d7-48e1-a33f-55db9dfe9769
📒 Files selected for processing (1)
src/patches/voiceMode.test.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@TyceHerrman will you please rename |
|
@georpar, done! |
2 participants
Force-enable the /voice slash command by bypassing the tengu_amber_quartz feature gate, and optionally enable the tengu_sotto_voce concise output mode for voice interactions.
Summary by CodeRabbit