feat(model): update default model to codex 5.3#110
Conversation
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
WalkthroughUpdates the default Codex model to gpt-5.3-codex, centralizes model configuration/constants and descriptions, adjusts handler lookup to use the new constants/env var, updates tests to expect the new default, and adds a new docs/TODO.md describing unimplemented features and v1.3.4 notes. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/__tests__/default-model.test.ts (1)
31-41:⚠️ Potential issue | 🟠 Major
CODEX_DEFAULT_MODELis not cleared inbeforeEach, risking test pollution.Tests at Lines 43–131 assert the hardcoded default
'gpt-5.3-codex', but they'll break ifCODEX_DEFAULT_MODELis set in the environment (or leaks from a prior test). The env-var-specific tests (Lines 133–179) usetry/finally, butbeforeEachshould still clean up for full isolation.Suggested fix
beforeEach(() => { sessionStorage = new InMemorySessionStorage(); handler = new CodexToolHandler(sessionStorage); mockedExecuteCommand.mockClear(); mockedExecuteCommand.mockResolvedValue({ stdout: 'Test response', stderr: '', }); process.env.STRUCTURED_CONTENT_ENABLED = '1'; delete process.env.CODEX_MCP_CALLBACK_URI; + delete process.env.CODEX_DEFAULT_MODEL; });src/__tests__/resume-functionality.test.ts (1)
31-37:⚠️ Potential issue | 🟠 MajorSame env-var isolation gap:
CODEX_DEFAULT_MODELnot cleared inbeforeEach.Same concern as in
default-model.test.ts— tests asserting the hardcoded default (Lines 54, 159, 185) will fail ifCODEX_DEFAULT_MODELleaks from the environment.Suggested fix
beforeEach(() => { sessionStorage = new InMemorySessionStorage(); handler = new CodexToolHandler(sessionStorage); mockedExecuteCommand.mockClear(); process.env.STRUCTURED_CONTENT_ENABLED = '1'; delete process.env.CODEX_MCP_CALLBACK_URI; + delete process.env.CODEX_DEFAULT_MODEL; });
🤖 Fix all issues with AI agents
In `@src/tools/handlers.ts`:
- Around line 1-4: Run Prettier on the file to fix the formatting error reported
by CI: reformat the import block that includes TOOLS, DEFAULT_CODEX_MODEL, and
CODEX_DEFAULT_MODEL_ENV_VAR (and the rest of src/tools/handlers.ts) using your
project's Prettier config (e.g., run prettier --write) so the import list and
surrounding whitespace match the project's style, then stage and commit the
resulting changes.
🧹 Nitpick comments (5)
src/__tests__/error-scenarios.test.ts (1)
158-164: Consider importingDEFAULT_CODEX_MODELinstead of hardcoding'gpt-5.3-codex'.Since this PR centralizes the default model into
DEFAULT_CODEX_MODELinsrc/types.ts, these test expectations could import and use that constant. This would eliminate the need to update every test file on the next model bump. The same applies to similar hardcoded expectations inmodel-selection.test.ts,context-building.test.ts,edge-cases.test.ts, andindex.test.ts.♻️ Example refactor for this file
+import { DEFAULT_CODEX_MODEL } from '../types.js'; + // ... in the test: expect(mockedExecuteCommand).toHaveBeenCalledWith('codex', [ 'exec', '--model', - 'gpt-5.3-codex', + DEFAULT_CODEX_MODEL, '--skip-git-repo-check', longPrompt, ]);src/tools/definitions.ts (1)
102-105: Review tool description omits the available models list.The
getModelDescription('review')returns only the default model info, whilegetModelDescription('codex')includes the available options. Since both tools reference the sameAVAILABLE_CODEX_MODELSandDEFAULT_CODEX_MODELconstants, consider including the options list in the review description for consistency.src/types.ts (2)
19-29: Consider validatingmodelagainstAVAILABLE_CODEX_MODELSin the Zod schemas.
AVAILABLE_CODEX_MODELSis declared for documentation/reference, but themodelfield inCodexToolSchema(Line 102) andReviewToolSchema(Line 117) isz.string().optional()— any string is accepted. If the intent is to guide users toward valid models while still allowing arbitrary strings, the current approach is fine. Otherwise, you could usez.enum(AVAILABLE_CODEX_MODELS).optional()for compile-time and runtime validation.This is purely a design consideration — no action needed if flexibility is intentional.
31-38: Inconsistent detail betweencodexandreviewdescriptions.The
'codex'branch lists all available models, while the'review'branch only mentions the default. If users of the review tool also benefit from seeing available options, consider includingmodelListin both branches.Suggested change
if (toolType === 'codex') { return `Specify which model to use (defaults to ${DEFAULT_CODEX_MODEL}). Options: ${modelList}`; } - return `Specify which model to use for the review (defaults to ${DEFAULT_CODEX_MODEL})`; + return `Specify which model to use for the review (defaults to ${DEFAULT_CODEX_MODEL}). Options: ${modelList}`; };src/__tests__/resume-functionality.test.ts (1)
215-216: Fragile positional index for extracting the prompt argument.
call?.[1]?.[4]relies on exact argument ordering. If the command construction in the handler changes (e.g., a new flag is inserted), this silently reads the wrong value. Consider usingArray.findor.includes()for a more resilient assertion, or at least assert the full args array.This is pre-existing, not introduced by this PR, so no action required now.
| import { | ||
| TOOLS, | ||
| DEFAULT_CODEX_MODEL, | ||
| CODEX_DEFAULT_MODEL_ENV_VAR, |
There was a problem hiding this comment.
Fix Prettier formatting to unblock CI.
The pipeline reports a Prettier style issue. Run prettier --write on this file to resolve.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In `@src/tools/handlers.ts` around lines 1 - 4, Run Prettier on the file to fix
the formatting error reported by CI: reformat the import block that includes
TOOLS, DEFAULT_CODEX_MODEL, and CODEX_DEFAULT_MODEL_ENV_VAR (and the rest of
src/tools/handlers.ts) using your project's Prettier config (e.g., run prettier
--write) so the import list and surrounding whitespace match the project's
style, then stage and commit the resulting changes.
Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
1 participant
Summary
Changes
Testing
Related Issues
Summary by CodeRabbit
New Features
Documentation