test(ai): cover composer slash commands - #1067
Conversation
wrapWithCommandMarker, TERAX_CMD_RE and tryRunSlashCommand drive the composer's slash and #handle commands but had no tests. Lock the marker wrap and its round-trip through the regex including the optional state attribute, the none outcome for plain text, /init and /claude-code expansion (with the usage message when no request is given), acceptance of a known # command, rejection of an unknown one, and /plan off vs bare /plan toggling.
📝 WalkthroughWalkthroughAdds Vitest coverage for slash command marker generation and command handling, including init, Claude Code, hash commands, and plan mode toggling. ChangesSlash command test coverage
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/ai/lib/slashCommands.test.ts`:
- Line 9: Update the imports in slashCommands.test.ts, including the planStore
mock and the imports on lines 19–23, to use the frontend `@/` alias instead of
relative module paths. Preserve the existing imported modules and test behavior.
- Around line 90-94: Update the `/plan` toggle test around `planMock.active`,
`planMock.toggle`, and `tryRunSlashCommand` to start with plan mode inactive and
mock `toggle` so it changes `active` to true. Keep the existing handled result
and toggle-call assertions, ensuring the test verifies the command performs the
transition rather than merely reading a preloaded post-toggle state.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ef5faeb-3394-4473-b469-a6cd05476f7d
📒 Files selected for processing (1)
src/modules/ai/lib/slashCommands.test.ts
| toggle: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../store/planStore", () => ({ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the frontend import alias.
These relative imports violate the frontend import rule.
Proposed fix
-vi.mock("../store/planStore", () => ({
+vi.mock("`@/modules/ai/store/planStore`", () => ({
@@
-} from "./slashCommands";
+} from "`@/modules/ai/lib/slashCommands`";As per coding guidelines, “Frontend imports must use the @/... alias and must not use relative imports across modules.”
Also applies to: 19-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/ai/lib/slashCommands.test.ts` at line 9, Update the imports in
slashCommands.test.ts, including the planStore mock and the imports on lines
19–23, to use the frontend `@/` alias instead of relative module paths. Preserve
the existing imported modules and test behavior.
Source: Coding guidelines
| it("toggles plan mode with a bare /plan", () => { | ||
| planMock.active = true; // reflects the state after toggle() | ||
| const out = tryRunSlashCommand("/plan"); | ||
| expect(out).toEqual({ kind: "handled", toast: "Plan mode on" }); | ||
| expect(planMock.toggle).toHaveBeenCalled(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Model the /plan state transition in the toggle test.
toggle is a no-op mock, while Line 91 preloads its expected post-toggle value. This passes even if the command reads state before toggling. Start inactive and make this test's toggle mock activate the state.
Proposed fix
- planMock.active = true; // reflects the state after toggle()
+ planMock.toggle.mockImplementationOnce(() => {
+ planMock.active = true;
+ });As per coding guidelines, “Changes to ... AI tool surfaces must include a test locking the relevant invariant.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("toggles plan mode with a bare /plan", () => { | |
| planMock.active = true; // reflects the state after toggle() | |
| const out = tryRunSlashCommand("/plan"); | |
| expect(out).toEqual({ kind: "handled", toast: "Plan mode on" }); | |
| expect(planMock.toggle).toHaveBeenCalled(); | |
| it("toggles plan mode with a bare /plan", () => { | |
| planMock.toggle.mockImplementationOnce(() => { | |
| planMock.active = true; | |
| }); | |
| const out = tryRunSlashCommand("/plan"); | |
| expect(out).toEqual({ kind: "handled", toast: "Plan mode on" }); | |
| expect(planMock.toggle).toHaveBeenCalled(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/ai/lib/slashCommands.test.ts` around lines 90 - 94, Update the
`/plan` toggle test around `planMock.active`, `planMock.toggle`, and
`tryRunSlashCommand` to start with plan mode inactive and mock `toggle` so it
changes `active` to true. Keep the existing handled result and toggle-call
assertions, ensuring the test verifies the command performs the transition
rather than merely reading a preloaded post-toggle state.
Source: Coding guidelines
1 participant
What
Adds unit tests for the composer slash-command helpers in
ai/lib/slashCommands.ts. Test-only.Why
wrapWithCommandMarker,TERAX_CMD_REandtryRunSlashCommanddrive thecomposer's
/and#handlecommands but had no tests.How
Pure logic tested directly; the plan store is mocked so the
/planbranch can beexercised without touching global state.
Locked behavior:
wrapWithCommandMarker/TERAX_CMD_RE: the marker wrap and its round-tripthrough the regex, including the optional
stateattribute.tryRunSlashCommand:nonefor plain text,/initand/claude-codeexpansion (with the usage message when no request follows), acceptance of a
known
#command, rejection of an unknown one, and/plan offvs a bare/plantoggle.Testing
Ran the full suite plus type-check and lint locally on Windows.
pnpm lintclean (unchanged: 103 pre-existing warnings, none introduced)pnpm check-typescleanpnpm testclean (the new suite passes; see reviewer note on onepre-existing suite)
src-tauri/) cargo clippy - N/A, no Rust changessrc-tauri/) cargo nextest - N/A, no Rust changes#[tauri::command]signature) - N/Apnpm tauri dev- N/A, no UI changeScreenshots / GIFs
N/A - no UI change.
Notes for reviewer
mainon purpose (my token lacksthe
workflowscope). Only adds a new file; should merge cleanly.src/app/eager-budget.test.tsfails locally under Windows with
git core.autocrlf=true; the committed blob isLF and it passes on CI.
Summary by CodeRabbit
/init,/claude-code,/plan, and#init.