fix: ideation view not working with other providers #650
Conversation
- Updated the ideation service to utilize phase settings for model resolution, improving flexibility in handling model aliases. - Introduced `getPhaseModelWithOverrides` to fetch model and provider information, allowing for dynamic adjustments based on project settings. - Enhanced logging to provide clearer insights into the model and provider being used during suggestion generation. This update streamlines the process of generating suggestions by leveraging phase-specific configurations, ensuring better alignment with user-defined settings.
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where the ideation view failed to operate with alternative providers. The core change involves updating the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughPhase-driven model selection replaces hardcoded model resolution for ideation and suggestions, passing provider and credentials from phase results into execution options (including a readOnly flag). The PR also removes the previous Suggestions feature surface and related routes, types, and UI/electron API bindings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Ideation as IdeationService
participant Settings as SettingsHelper
participant Resolver as ModelResolver
participant Provider as Provider
participant SDK as SDK
Client->>Ideation: request generate/start
Ideation->>Settings: getPhaseModelWithOverrides(phaseKey, context)
Settings-->>Ideation: phaseModel { modelKey, provider?, credentials? }
Ideation->>Resolver: resolvePhaseModel(phaseModel.modelKey)
Resolver-->>Ideation: canonical modelId
Ideation->>Provider: prepare executeOptions (provider, credentials, readOnly)
Ideation->>SDK: execute(modelId, prompt, executeOptions)
SDK-->>Provider: runtime/model invocation
Provider-->>Ideation: stream/response or error
Ideation-->>Client: results / events / logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the ideation view was hardcoded to use a specific model, preventing it from working with other configured providers. The changes correctly refactor the generateSuggestions method to dynamically load the model, provider, and credentials from settings using the getPhaseModelWithOverrides helper. This makes the feature compatible with the broader provider ecosystem. My review includes one suggestion to simplify the model resolution logic by removing a redundant function call, which will improve code clarity and maintainability.
Shironex
added
Bug
Address PR #650 review feedback from gemini-code-assist. The call to resolveModelString was redundant because resolvePhaseModel already returns the fully resolved canonical model ID. When providerId is set, it returns the provider-specific model ID unchanged; otherwise, it already calls resolveModelString internally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updated the comment to better explain why resolveModelString is not needed after resolvePhaseModel - the latter already handles model alias resolution internally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit removes the suggestions routes and associated files from the server, streamlining the codebase. The `suggestionsModel` has been replaced with `ideationModel` across various components, including UI and service layers, to better reflect the updated functionality. Additionally, adjustments were made to ensure that the ideation service correctly utilizes the new model configuration. - Deleted suggestions routes and their handlers. - Updated references from `suggestionsModel` to `ideationModel` in settings and UI components. - Refactored related logic in the ideation service to align with the new model structure.
![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
🤖 Fix all issues with AI agents
In `@apps/server/src/providers/cursor-provider.ts`:
- Around line 676-683: The call to buildSubprocessOptions uses the original
options instead of the normalized effectiveOptions, causing an inconsistency
with the parent CliProvider; update the call in the cursor provider so that
after creating effectiveOptions via embedSystemPromptIntoPrompt and building
cliArgs with buildCliArgs, you call
this.buildSubprocessOptions(effectiveOptions, cliArgs) (reference methods:
embedSystemPromptIntoPrompt, extractPromptText, buildCliArgs,
buildSubprocessOptions, and class CliProvider) to align behavior and improve
maintainability.
In `@apps/ui/src/hooks/mutations/use-ideation-mutations.ts`:
- Around line 101-107: The onError handler in use-ideation-mutations.ts should
defensively guard against non-Error values like other mutations do: within the
onError for the mutation that calls useIdeationStore.getState().updateJobStatus
and toast.error, derive a safe message by checking if error is an instance of
Error and falling back to String(error) (or a default string) and use that safe
message when calling updateJobStatus(variables.jobId, 'error', undefined,
message) and toast.error(`Failed to generate ideas: ${message}`); keep the same
references to updateJobStatus and toast.error.
| // Embed system prompt into user prompt (Cursor CLI doesn't support separate system messages) | ||
| const effectiveOptions = this.embedSystemPromptIntoPrompt(options); | ||
|
|
||
| // Extract prompt text to pass via stdin (avoids shell escaping issues) | ||
| const promptText = this.extractPromptText(options); | ||
| const promptText = this.extractPromptText(effectiveOptions); | ||
|
|
||
| const cliArgs = this.buildCliArgs(options); | ||
| const cliArgs = this.buildCliArgs(effectiveOptions); | ||
| const subprocessOptions = this.buildSubprocessOptions(options, cliArgs); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's understand the repository structure
find . -type f -name "*.ts" -path "*provider*" | head -20Repository: AutoMaker-Org/automaker
Length of output: 1056
🏁 Script executed:
#!/bin/bash
# Search for buildSubprocessOptions implementation
rg -n "buildSubprocessOptions" --type ts -A 30 | head -100Repository: AutoMaker-Org/automaker
Length of output: 8649
🏁 Script executed:
#!/bin/bash
# Also check the cursor-provider.ts file specifically
rg -n "buildSubprocessOptions|buildCliArgs|extractPromptText" apps/server/src/providers/cursor-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 400
🏁 Script executed:
#!/bin/bash
# Get the full buildSubprocessOptions implementation from cli-provider.ts
sed -n '446,545p' apps/server/src/providers/cli-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 3390
🏁 Script executed:
#!/bin/bash
# Also check if there are any prompt-related field accesses in buildSubprocessOptions
rg -n "options\." apps/server/src/providers/cli-provider.ts | grep -A 5 -B 5 "buildSubprocessOptions" | head -50Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Get the complete buildSubprocessOptions method from cli-provider
ast-grep --pattern 'protected buildSubprocessOptions($_, $_) {
$$$
}'Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if cursor-provider extends cli-provider and what it overrides
head -50 apps/server/src/providers/cursor-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 1569
🏁 Script executed:
#!/bin/bash
# Check if cursor-provider has its own buildSubprocessOptions override
rg -n "buildSubprocessOptions" apps/server/src/providers/cursor-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 147
🏁 Script executed:
#!/bin/bash
# Check the class hierarchy and imports in cursor-provider
rg -n "class CursorProvider|extends|import" apps/server/src/providers/cursor-provider.ts | head -20Repository: AutoMaker-Org/automaker
Length of output: 683
🏁 Script executed:
#!/bin/bash
# Check what embedSystemPromptIntoPrompt does and what it returns
rg -n "embedSystemPromptIntoPrompt" apps/server/src/providers/cursor-provider.ts -A 20Repository: AutoMaker-Org/automaker
Length of output: 989
🏁 Script executed:
#!/bin/bash
# Also check the base class implementation
rg -n "embedSystemPromptIntoPrompt" apps/server/src/providers/cli-provider.ts -A 20Repository: AutoMaker-Org/automaker
Length of output: 1669
🏁 Script executed:
#!/bin/bash
# Check the full executeQuery method in cursor-provider to understand the context better
sed -n '670,690p' apps/server/src/providers/cursor-provider.tsRepository: AutoMaker-Org/automaker
Length of output: 1014
Pass effectiveOptions to buildSubprocessOptions for consistency with parent class.
Line 683 currently passes the original options to buildSubprocessOptions, while the parent class CliProvider (line 545) passes effectiveOptions. Although buildSubprocessOptions only accesses non-prompt fields (cwd, reasoningEffort, abortController) and thus the embedded system prompt is not bypassed, this inconsistency with the parent class pattern reduces maintainability. The prompt correctly flows through effectiveOptions → promptText → stdin, so change the call to this.buildSubprocessOptions(effectiveOptions, cliArgs) to align with the parent implementation.
🤖 Prompt for AI Agents
In `@apps/server/src/providers/cursor-provider.ts` around lines 676 - 683, The
call to buildSubprocessOptions uses the original options instead of the
normalized effectiveOptions, causing an inconsistency with the parent
CliProvider; update the call in the cursor provider so that after creating
effectiveOptions via embedSystemPromptIntoPrompt and building cliArgs with
buildCliArgs, you call this.buildSubprocessOptions(effectiveOptions, cliArgs)
(reference methods: embedSystemPromptIntoPrompt, extractPromptText,
buildCliArgs, buildSubprocessOptions, and class CliProvider) to align behavior
and improve maintainability.
| onError: (error, variables) => { | ||
| // Update job status to error - this runs even if the component unmounts | ||
| const updateJobStatus = useIdeationStore.getState().updateJobStatus; | ||
| updateJobStatus(variables.jobId, 'error', undefined, error.message); | ||
|
|
||
| // Show error toast | ||
| toast.error(`Failed to generate ideas: ${error.message}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
fd -t f "use-ideation-mutations" --full-path | head -5Repository: AutoMaker-Org/automaker
Length of output: 120
🏁 Script executed:
# Read the file around lines 101-107
cat -n apps/ui/src/hooks/mutations/use-ideation-mutations.ts | sed -n '95,115p'Repository: AutoMaker-Org/automaker
Length of output: 705
🏁 Script executed:
# Read the entire file to see imports and understand the mutation setup
cat -n apps/ui/src/hooks/mutations/use-ideation-mutations.ts | head -50Repository: AutoMaker-Org/automaker
Length of output: 1812
🏁 Script executed:
# Check react-query type definitions or dependencies
cat -n apps/ui/src/hooks/mutations/use-ideation-mutations.ts | sed -n '60,110p'Repository: AutoMaker-Org/automaker
Length of output: 2176
🏁 Script executed:
# Check the package.json to see react-query version
cat -n package.json | grep -A 5 -B 5 "react-query\|@tanstack/react-query"Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
# Find package.json files
fd -t f "package.json" --full-path | head -10Repository: AutoMaker-Org/automaker
Length of output: 342
🏁 Script executed:
# Check the UI package.json for react-query/tanstack-query version
cat apps/ui/package.json | grep -A 2 -B 2 "@tanstack/react-query"Repository: AutoMaker-Org/automaker
Length of output: 314
🏁 Script executed:
# Check TypeScript configuration for strict mode
cat tsconfig.json | head -30Repository: AutoMaker-Org/automaker
Length of output: 112
🏁 Script executed:
# Find tsconfig files in the UI app
fd -t f "tsconfig.json" apps/ui/Repository: AutoMaker-Org/automaker
Length of output: 88
🏁 Script executed:
# Check the TypeScript configuration
cat apps/ui/tsconfig.jsonRepository: AutoMaker-Org/automaker
Length of output: 589
🏁 Script executed:
# Search for any type annotations on the useMutation call
rg -A 20 "useMutation" apps/ui/src/hooks/mutations/use-ideation-mutations.tsRepository: AutoMaker-Org/automaker
Length of output: 1432
🏁 Script executed:
# Check if there's explicit error type annotation on useMutation
rg -B 5 "onError:" apps/ui/src/hooks/mutations/use-ideation-mutations.tsRepository: AutoMaker-Org/automaker
Length of output: 256
🏁 Script executed:
# Check if other mutations in the same project handle errors similarly
rg -B 3 "onError:" apps/ui/src/hooks/mutations/ -A 3Repository: AutoMaker-Org/automaker
Length of output: 23186
🌐 Web query:
TanStack React Query v5 useMutation onError callback error type unknown
💡 Result:
Short answer — options:
- TanStack Query v5’s default error type is Error (not unknown). You can explicitly type useMutation with generics (TData, TError, TVariables, TContext) so onError receives a typed error, or register a global error type by augmenting the library’s Register interface. (tanstack.com)
Examples
-
Per-call typing (recommended):
const mutation = useMutation<MyData, AxiosError, MyVars>({
mutationFn: myFn,
onError: (err, vars, result, ctx) => {
// err is AxiosError
},
}); (tanstack.com) -
Make callers narrow an unknown error (global):
// global.d.ts
import '@tanstack/react-query';
declare module '@tanstack/react-query' {
interface Register { defaultError: unknown }
}
// now onError error param is unknown and you must narrow it (e.g. axios.isAxiosError). (tanstack.com)
Tip: if you get unknown in onError, either add the second generic (TError) on useMutation or register a global defaultError type, and/or use type guards (e.g. axios.isAxiosError) to narrow. (tanstack.com)
If you want, tell me which approach you prefer and I’ll give the exact TS file snippet for your project.
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/reference/useMutation?utm_source=openai
- 2: https://tanstack.com/query/latest/docs/framework/react/reference/useMutation?utm_source=openai
- 3: https://tanstack.com/query/v5/docs/react/typescript?utm_source=openai
- 4: https://tanstack.com/query/v5/docs/react/typescript?utm_source=openai
🏁 Script executed:
# Check if useMutation has explicit type parameters in use-ideation-mutations.ts
rg -B 2 "return useMutation" apps/ui/src/hooks/mutations/use-ideation-mutations.ts -A 25Repository: AutoMaker-Org/automaker
Length of output: 997
🏁 Script executed:
# Check if there's a global React Query Register configuration
rg -l "Register" apps/ui/src --type ts --type tsx | head -10Repository: AutoMaker-Org/automaker
Length of output: 94
Add error type guard for consistency with codebase patterns.
While React Query v5 defaults the error type to Error, other mutations in this directory (use-spec-mutations.ts, use-github-mutations.ts, use-cursor-permissions-mutations.ts) guard against non-Error values when the onError handler lacks explicit type parameters. Apply the same defensive pattern here for consistency:
Suggested guard
onError: (error, variables) => {
+ const message = error instanceof Error ? error.message : String(error);
// Update job status to error - this runs even if the component unmounts
const updateJobStatus = useIdeationStore.getState().updateJobStatus;
- updateJobStatus(variables.jobId, 'error', undefined, error.message);
+ updateJobStatus(variables.jobId, 'error', undefined, message);
// Show error toast
- toast.error(`Failed to generate ideas: ${error.message}`);
+ toast.error(`Failed to generate ideas: ${message}`);
},📝 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.
| onError: (error, variables) => { | |
| // Update job status to error - this runs even if the component unmounts | |
| const updateJobStatus = useIdeationStore.getState().updateJobStatus; | |
| updateJobStatus(variables.jobId, 'error', undefined, error.message); | |
| // Show error toast | |
| toast.error(`Failed to generate ideas: ${error.message}`); | |
| onError: (error, variables) => { | |
| const message = error instanceof Error ? error.message : String(error); | |
| // Update job status to error - this runs even if the component unmounts | |
| const updateJobStatus = useIdeationStore.getState().updateJobStatus; | |
| updateJobStatus(variables.jobId, 'error', undefined, message); | |
| // Show error toast | |
| toast.error(`Failed to generate ideas: ${message}`); |
🤖 Prompt for AI Agents
In `@apps/ui/src/hooks/mutations/use-ideation-mutations.ts` around lines 101 -
107, The onError handler in use-ideation-mutations.ts should defensively guard
against non-Error values like other mutations do: within the onError for the
mutation that calls useIdeationStore.getState().updateJobStatus and toast.error,
derive a safe message by checking if error is an instance of Error and falling
back to String(error) (or a default string) and use that safe message when
calling updateJobStatus(variables.jobId, 'error', undefined, message) and
toast.error(`Failed to generate ideas: ${message}`); keep the same references to
updateJobStatus and toast.error.
Summary
Fixes the Ideation view to work correctly with non-Claude providers (Codex, Cursor, OpenCode).
Closes #541
Changes
1. Model Resolution Fix (Core Issue)
resolveModelString('sonnet')which always returned Claude Sonnet regardless of user settingsgetPhaseModelWithOverrides('ideationModel', ...)to properly read from user settings, support project overrides, and pass provider/credentials2. Renamed
suggestionsModel→ideationModelThe setting is now called "Ideation" in the UI (instead of "AI Suggestions") to accurately reflect its purpose.
Files updated:
libs/types/src/settings.ts- Type definition & default valueapps/server/src/services/ideation-service.ts- Phase model keyapps/ui/src/hooks/use-settings-sync.ts- Migration mappingapps/ui/src/components/views/settings-view/model-defaults/- UI labelsapps/ui/src/components/views/project-settings-view/- UI labelsapps/ui/scripts/setup-e2e-fixtures.mjs- E2E fixtures3. Removed Dead
/api/suggestionsRoutesCleaned up the old suggestions API that was replaced by the Ideation view:
apps/server/src/routes/suggestions/directory (6 files)'suggestions:event'from event typesSuggestionsAPIfrom http-api-client and electron mock (~300 lines)4. Fixed Cursor Provider for Read-Only Mode
--mode askflag whenreadOnly: true- puts Cursor in Q&A mode without tool usageembedSystemPromptIntoPrompt()call - system prompt was never being included in stdin!5. Enhanced System Prompt for Non-Claude Models
Added stronger instructions to prevent file creation:
6. Cherry-picked Ideation UI Fix (from PR #641)
Included the fix for job status updates when component unmounts - moves status update to mutation hook using
useIdeationStore.getState().Testing
--mode ask) - WorksSummary by CodeRabbit
Refactor
Removed
✏️ Tip: You can customize this high-level summary in your review settings.