feat: configurable auto-route via system configs#187
Conversation
Allow admins to change the model and default parameters for `model: "auto"` chat completion requests via `PATCH /v1/admin/configs` without code changes or redeployment. Falls back to hardcoded defaults (GLM-5-FP8) when no config is set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello @henrypark133, 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 introduces a significant enhancement by making the Highlights
Changelog
Activity
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
|
PR Review: feat: configurable auto-route via system configsThe approach is clean and the fallback behavior is well-designed. Two critical issues before merge: 1.
|
There was a problem hiding this comment.
Code Review
This pull request introduces configurable auto-routing for chat completions, allowing administrators to define the target model and default parameters via system configurations. However, a security vulnerability exists where the endpoint for updating the auto-route configuration does not validate if the provided model ID actually exists, potentially leading to a denial of service for the auto-routing feature. Furthermore, there is a performance concern due to frequent database access for system configurations on every chat completion request, and the validation for the auto-route model identifier in the admin route should be strengthened to ensure the model exists in the proxy service.
- Add SystemConfigsCache (RwLock<Option<CacheEntry>>) with 60s TTL - Auto-route reads from cache instead of hitting DB per request - Admin config updates invalidate cache for immediate effect - Downgrade service-level get_configs log from info to debug Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
model: "auto"routing for chat completions configurable via system configs instead of hardcoded constantsPATCH /v1/admin/configswithout code changes or redeploymentBehavior
{"model": "X"}{"model": "X", "temperature": 0.7}Example
Changes
crates/services/src/system_configs/ports.rs— AddedAutoRouteConfigstruct with requiredmodeland optionaltemperature,top_p,max_tokenscrates/api/src/models.rs— Addedauto_routeto public/admin config responses and upsert requestcrates/api/src/routes/admin.rs— Validation for auto_route fields (non-empty model, valid ranges)crates/api/src/routes/api.rs— Read config from DB on auto-route, graceful fallback to constants on DB failureNo DB migration needed — JSONB storage with
#[serde(default)].Test plan
cargo buildcompilescargo clippycleancargo test --features test— unit tests pass (50/50), existing integration tests unaffectedPATCH /v1/admin/configswithauto_route, sendmodel: "auto"request, verify configured model is usedauto_routeconfig is set🤖 Generated with Claude Code