feat(web): expose cheap_model and smart_routing_cascade in settings UI - #1491
wehrmannit wants to merge 2 commits into
Conversation
The LLM_CHEAP_MODEL and SMART_ROUTING_CASCADE options from nearai#1081 were only configurable via env vars. This adds them to the Settings struct and web UI so users can configure smart routing from the browser. Resolution order: env var > settings > default (None / true). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
scope: channel/web
Summary of ChangesHello, 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 enhances the application's LLM configuration capabilities by integrating previously environment-variable-only settings, Highlights
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. Footnotes
|
![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 introduces new configuration options for LLM smart routing: cheap_model and smart_routing_cascade. The cheap_model allows specifying a model for lightweight tasks, while smart_routing_cascade enables a retry mechanism with the primary model if the cheap model's response is uncertain. These settings are added to the application's configuration structure, integrated into the LLM configuration loading logic (prioritizing environment variables over settings), and exposed in the web UI with corresponding internationalization strings. Changes to these settings will require an application restart.
serrrfirat
left a comment
serrrfirat
left a comment
There was a problem hiding this comment.
Paranoid Architect Review - All Findings
🔴 High Severity Issues
1. Type Safety - No validation for cheap_model format
Location: src/config/llm.rs:216-223
The resolve() method reads cheap_model from Settings but there's no validation that it's a valid provider/model combination. Invalid values will cause runtime errors during LLM initialization.
Recommendation: Add validation in Settings deserialization or in resolve() to verify the cheap_model string format matches expected provider:model pattern (e.g., "openai:gpt-4o-mini"). Consider adding a helper method to validate model strings against available providers.
2. Database Compatibility - Missing dual-backend verification
Location: src/settings.rs:87-93
New Optional fields added to Settings struct but no database migration or dual-backend verification. Per CLAUDE.md: "All new persistence features must support both backends." Settings are persisted via db.set_settings_map() - need to verify both PostgreSQL and libSQL handle these new fields.
Recommendation: Verify that both PostgreSQL and libSQL backends correctly serialize/deserialize the new Optional and Optional fields. Add integration tests with #[cfg(feature = "integration")] to validate Settings persistence across both backends.
🟡 Medium Severity Issues
3. User Experience - No client-side validation
Location: src/channels/web/static/app.js:4708-4709
New settings marked as RESTART_REQUIRED but no validation of cheap_model format in the UI. Users can enter arbitrary strings that will cause silent failures or cryptic errors on restart.
Recommendation: Add client-side validation to ensure cheap_model matches expected format (provider:model). Show available models in a dropdown or add format hint text. Validate on blur/change before allowing save.
4. Configuration Priority - Asymmetric defaults
Location: src/config/llm.rs:216-223
The resolution chain is env var → settings → default but there's asymmetry: cheap_model has no default while smart_routing_cascade defaults to true. This creates inconsistent behavior - smart routing might be enabled but fail if no cheap model is configured.
Recommendation: Add defensive logic: if smart_routing_cascade resolves to true but cheap_model is None, either (1) emit a warning and disable cascade, or (2) use the primary model as cheap model. Document the expected behavior when one is set without the other.
5. Documentation - Missing interaction explanation
Location: src/settings.rs:87-92, src/config/llm.rs:216-223
Comments explain what each field does but don't explain the interaction between them. What happens if cascade is true but cheap_model is None? What's the expected format for cheap_model?
Recommendation: Add a module-level doc comment in settings.rs explaining the smart routing feature, the relationship between these two fields, expected cheap_model format (provider:model), and fallback behavior.
🟢 Low Severity Issues
6. Code Quality - Growing resolve() method
Location: src/config/llm.rs:216-223
The resolve() method is growing large with more conditional logic. Adding two more optional fields continues the pattern of manual env-then-settings resolution.
Recommendation: Consider extracting a helper method like resolve_optional<T>(env_key: &str, setting: Option<T>, default: T) -> T to reduce duplication. This PR is fine as-is but sets precedent for future refactoring.
7. Internationalization - Brief translation strings
Location: src/channels/web/static/i18n/zh-CN.js, en.js
Added translations for the new settings but translations are brief. The English description for cheap_model is just "Cheap model for smart routing" which doesn't explain the format or give examples.
Recommendation: Enhance translation strings to include format hints, e.g., "Cheap model for smart routing (format: provider:model, e.g., openai:gpt-4o-mini)". This helps international users understand expected input.
Summary: The PR correctly exposes two LLM configuration options to the Settings UI, but lacks validation for the cheap_model format and needs dual-backend database verification per project requirements. The most critical issues are: (1) no validation that cheap_model is a valid provider:model string, and (2) no verification that PostgreSQL and libSQL both handle the new Optional fields correctly.
|
I did a paranoid pass against current
Why I think the original shape needs tightening in current
What’s in the pushed branch:
Targeted validation I ran locally:
If useful, I can also open a follow-up PR from my branch with just these fixes layered on top of #1491. |
|
Thank you for making smart-routing controls available in the browser. We are closing this PR because it modifies the retired v1 settings and WebUI implementation. Reborn retains the underlying We would be glad to have you port this experience to the Reborn administration API and WebUI v2, including persistence, precedence, and caller-level tests. Thank you for addressing an important usability gap. |
2 participants
Summary
cheap_modelandsmart_routing_cascadefields to theSettingsstruct so they persist via settings.json / DBLlmConfig::resolve()to fall back to settings when env vars aren't set (env var > settings > default)Follows up on #1081 which added
LLM_CHEAP_MODELandSMART_ROUTING_CASCADEenv vars but left them unconfigurable from the browser.Test plan
cargo clippy --all --benches --tests --examples --all-features— zero warningscargo build --release— compiles clean🤖 Generated with Claude Code