feat: add default runner configuration if none exists#621
Conversation
Summary of ChangesHello @ilopezluna, 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 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
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The nested conditionals when building the default runnerConfig in loader.load are quite deep; consider extracting this logic into a small helper (e.g.,
defaultBackendConfigFromBundle(modelManager, modelID)) to make the flow easier to follow and to keep load() focused on orchestration. - Calling
modelManager.GetBundle(modelID)insideloadfor the default config may introduce extra I/O or parsing; if a bundle is already available in this code path, consider reusing it or caching to avoid redundant bundle loading.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The nested conditionals when building the default runnerConfig in loader.load are quite deep; consider extracting this logic into a small helper (e.g., `defaultBackendConfigFromBundle(modelManager, modelID)`) to make the flow easier to follow and to keep load() focused on orchestration.
- Calling `modelManager.GetBundle(modelID)` inside `load` for the default config may introduce extra I/O or parsing; if a bundle is already available in this code path, consider reusing it or caching to avoid redundant bundle loading.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a feature to create a default runner configuration if one doesn't exist, primarily to ensure the context-size is available. The change in pkg/distribution/internal/store/bundles.go is a good stylistic refactoring. My main feedback is on pkg/inference/scheduling/loader.go, where the new logic can be improved by adding error logging and reducing code nesting for better readability and maintainability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Fixes #407
In Docker Desktop the context size field is expected to be found when showing request/responses:
When no explicit configuration call has been made, there is no runner configuration, so the field
response.config.context-sizeis not populated.This PR fixes that.