-
Notifications
You must be signed in to change notification settings - Fork 487
fix: Pass SettingsService to spec generation so runtime uses persisted model #623
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
fix: Pass SettingsService to spec generation so runtime uses persisted model #623
Conversation
Summary of ChangesHello @mcbodge, 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 addresses a critical issue where the application specification generation process was not utilizing the user-configured model settings. By integrating the SettingsService into the spec generation route, the system now consistently respects and applies the user's chosen model, ensuring that generated specifications align with their preferences and resolving previously reported discrepancies. 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
|
📝 WalkthroughWalkthroughThe PR injects an optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
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.
Code Review
This pull request correctly passes the SettingsService to the /create app spec handler, ensuring that the spec generation process uses the user-configured model from settings instead of the default. The changes are straightforward, consistent with other route handlers, and effectively resolve the described issue. The implementation is clean and correct, and I have no further suggestions.
|
@webdevcody Hello, may I ask why this was closed? This way I can maybe try to create it again with the requested corrections for the new release branch. It's a bug fix to actually use the models defined in global settings instead of always use the default ones (se the related issues). Kind regards |
Previously the App Spec generation background task used the server defaults (for
example the
opus→claude-opus-4-5-20251101mapping) instead of the model selected in theUI and persisted via the global settings API.
This change forwards the
SettingsServiceinto the App Spec/createhandler sogenerateSpec()reads the persistedphaseModels.specGenerationModelat runtime. Two fileswere updated to pass the service through the route stack.
Related Issue
Closes #528 (and for users that are not using Claude also 585, because it was providing log-in request response instead of actual response)
Changes
settingsServiceinto the/createroute factory.settingsServicetogenerateSpec().eventHookskeys that appeared twice inhydrateStoreFromSettings/buildSettingsUpdateFromStorewithinapps/ui/src/hooks/use-settings-migration.ts, causing test failureThese edits ensure the background spec generation resolves the user's chosen model (including
dynamic provider model strings such as Opencode) and logs the resolved
model via the existing
logger.info('Using model:', model)statement.Testing & Verification
I tested and verified the following:
PUT /api/settings/globalwithphaseModels.specGenerationModel.modelset to your desired modelTrigger App Spec generation from the UI (
Create App Spec) or call the create route.Tail the server logs and confirm to see a line similar to:
.automaker/app_spec.txtcontains the expected structured spec output(XML/JSON per the generator expectations) and that the earlier error about missing structured
output is resolved.
Notes
not change the model resolution logic or provider routing; if a settings value references a
provider/model string that does not exist on the server, the provider discovery and error
handling will still apply (see
GET /api/models/available).Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.