-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(models): use models.dev API for model discovery #4
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
Conversation
WalkthroughRemoves per-provider model-listing APIs and callers, centralizes model discovery by fetching from models.dev in the CLI models update flow, updates examples/tests to drop model-listing steps, and writes a models cache to disk. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Command
participant ModelsCmd as Models Command
participant ModelsDev as models.dev API
participant Cache as ModelsCache
participant Disk as Disk Storage
User->>CLI: rullm models update
CLI->>ModelsCmd: run(output_level, config, _cli)
ModelsCmd->>ModelsCmd: Determine supported providers
ModelsCmd->>ModelsDev: Fetch models (models.dev)
ModelsDev-->>ModelsCmd: Return providers & models JSON
ModelsCmd->>ModelsCmd: Build ModelsCache from response
ModelsCmd->>Cache: Ensure cache directory exists
ModelsCmd->>Disk: Write MODEL_FILE_NAME (ModelsCache)
Disk-->>ModelsCmd: Write success
ModelsCmd-->>CLI: Return updated model count
CLI-->>User: Print success message with count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/rullm-core/examples/google_simple.rs (1)
19-23: Consider adding a comment about model discovery.Since
list_models()has been removed, new users following this example might wonder how to discover available models. Consider adding a brief comment explaining that model discovery is now done viarullm models updateor models.dev.Example:
// For model discovery, use: rullm models update // This fetches available models from https://models.dev let client = GoogleClient::from_env()?;crates/rullm-cli/src/provider.rs (1)
94-102: Consider leveraging the Display trait to reduce duplication.The
models_dev_idmethod returns identical strings to theDisplaytrait implementation (lines 14-25). You could simplify this to:pub fn models_dev_id(&self) -> String { self.to_string() }However, the current explicit approach may be preferable if you want to ensure the models.dev identifier remains stable and independent of Display formatting changes.
crates/rullm-cli/src/commands/models.rs (1)
243-247: Consider adding timeout and retry logic for robustness.The HTTP request to models.dev lacks timeout configuration and retry logic. For production use, consider:
- Adding a timeout to prevent indefinite hangs
- Implementing retry logic for transient network failures
- Making the URL configurable for testing/proxy scenarios
Example with timeout
async fn fetch_models_from_models_dev(supported_providers: &[&str]) -> Result<Vec<String>> { let client = reqwest::Client::builder() .timeout(std::time::Duration::from_secs(10)) .build()?; let response = client .get("https://models.dev/api.json") .send() .await? .error_for_status()?; let providers: HashMap<String, ModelsDevProvider> = response.json().await?; // ... rest of function }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
README.md(2 hunks)crates/rullm-cli/src/cli_client.rs(0 hunks)crates/rullm-cli/src/commands/mod.rs(1 hunks)crates/rullm-cli/src/commands/models.rs(5 hunks)crates/rullm-cli/src/provider.rs(1 hunks)crates/rullm-core/examples/README.md(3 hunks)crates/rullm-core/examples/google_simple.rs(1 hunks)crates/rullm-core/examples/openai_config.rs(0 hunks)crates/rullm-core/examples/openai_conversation.rs(0 hunks)crates/rullm-core/examples/openai_simple.rs(1 hunks)crates/rullm-core/examples/test_all_providers.rs(5 hunks)crates/rullm-core/src/providers/anthropic/client.rs(0 hunks)crates/rullm-core/src/providers/google/client.rs(0 hunks)crates/rullm-core/src/providers/openai/client.rs(0 hunks)crates/rullm-core/src/providers/openai_compatible/mod.rs(0 hunks)
💤 Files with no reviewable changes (7)
- crates/rullm-core/src/providers/openai_compatible/mod.rs
- crates/rullm-core/examples/openai_config.rs
- crates/rullm-core/src/providers/google/client.rs
- crates/rullm-core/src/providers/openai/client.rs
- crates/rullm-core/examples/openai_conversation.rs
- crates/rullm-core/src/providers/anthropic/client.rs
- crates/rullm-cli/src/cli_client.rs
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Define provider configuration using configuration traits and builders in `config.rs`
Applied to files:
crates/rullm-cli/src/provider.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Verify examples compile using `cargo check --examples`
Applied to files:
crates/rullm-cli/src/commands/mod.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Organize CLI commands in the `commands/` directory with separate modules for each command (auth.rs, chat.rs, models.rs, alias.rs, templates.rs)
Applied to files:
crates/rullm-cli/src/commands/models.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Define comprehensive error handling using the `LlmError` enum in `error.rs`
Applied to files:
crates/rullm-cli/src/commands/models.rs
🧬 Code graph analysis (2)
crates/rullm-core/examples/test_all_providers.rs (5)
crates/rullm-core/src/providers/anthropic/client.rs (1)
new(21-33)crates/rullm-core/src/providers/google/client.rs (1)
new(21-33)crates/rullm-core/src/providers/openai/client.rs (1)
new(21-33)crates/rullm-core/src/providers/openai_compatible/mod.rs (1)
new(60-71)crates/rullm-core/src/providers/google/config.rs (1)
new(16-22)
crates/rullm-cli/src/commands/models.rs (2)
crates/rullm-cli/src/commands/mod.rs (1)
new(98-103)crates/rullm-cli/src/args.rs (1)
serde_json(170-170)
🔇 Additional comments (14)
crates/rullm-core/examples/google_simple.rs (1)
128-128: LGTM: Comment renumbering is correct.The section numbering has been correctly updated after removing the model listing section.
crates/rullm-cli/src/commands/mod.rs (1)
40-40: LGTM: Documentation update aligns with refactoring.The help text now correctly describes the new models.dev-based approach rather than showing a specific model example.
README.md (1)
69-70: LGTM: Documentation accurately reflects the new approach.The updated text correctly highlights that models.dev is now used for model discovery and that API keys are no longer required for this operation.
crates/rullm-core/examples/openai_simple.rs (1)
124-128: LGTM: Example simplified to health check only.The removal of model listing code aligns with the PR objective to centralize model discovery via models.dev. The example now focuses on the health check functionality.
crates/rullm-cli/src/commands/models.rs (6)
4-5: LGTM: Necessary imports for models.dev integration.The new imports support JSON deserialization of the models.dev API response.
25-32: LGTM: Comments accurately reflect the new behavior.The updated documentation correctly describes the models.dev-based approach.
39-44: Parameter naming follows Rust conventions.The underscore prefix on
_clicorrectly indicates an intentionally unused parameter, likely required by the method signature or trait definition.
68-91: LGTM: Update logic correctly implements models.dev integration.The refactored Update action properly:
- Collects supported provider IDs
- Fetches models from models.dev
- Validates the response is non-empty
- Creates necessary directories
- Saves formatted JSON cache
- Reports success
232-241: LGTM: Deserialization types are well-structured.The types correctly model the models.dev API structure with appropriate use of
HashMapfor dynamic model lists and#[serde(default)]for optional fields.
243-262: No changes needed. The models.dev API is a legitimate, maintained open-source database. It returns valid JSON without authentication requirements or rate limits, and the structure matches the code's deserialization expectations perfectly.crates/rullm-core/examples/README.md (1)
393-435: LGTM: Documentation accurately reflects the refactored test approach.The updates correctly describe the simplified health-check-only validation and match the code changes in
test_all_providers.rs.crates/rullm-core/examples/test_all_providers.rs (3)
9-79: LGTM: Main function correctly implements simplified health-check reporting.The refactored code cleanly handles provider test results and produces a well-formatted summary table focused on health status rather than model counts.
81-127: LGTM: Provider test functions correctly simplified to health checks only.All three test functions consistently:
- Return
Result<(), ...>instead of model lists- Perform only health checks
- Handle errors appropriately
1-127: Verify that examples compile after refactoring.Run
cargo check --examplesto ensure the example compiles successfully and all imports (AnthropicClient, AnthropicConfig, GoogleClient, GoogleAiConfig, OpenAIClient, OpenAIConfig) resolve correctly.
38dba35 to
def142b
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/rullm-cli/src/commands/models.rs (1)
243-262: Consider adding retry logic for improved reliability.Network requests can fail due to transient issues. Adding retry logic with exponential backoff would improve the user experience, especially for a CLI tool that may be run in various network conditions.
You could use the
reqwest-retrycrate or implement a simple retry loop with a crate liketokio-retryto handle temporary failures gracefully.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)crates/rullm-cli/src/commands/models.rs(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Organize CLI commands in the `commands/` directory with separate modules for each command (auth.rs, chat.rs, models.rs, alias.rs, templates.rs)
Applied to files:
crates/rullm-cli/src/commands/models.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Define comprehensive error handling using the `LlmError` enum in `error.rs`
Applied to files:
crates/rullm-cli/src/commands/models.rs
🔇 Additional comments (5)
crates/rullm-cli/src/commands/models.rs (5)
4-5: LGTM!The new imports are appropriate for the models.dev API integration and deserialization logic.
43-43: LGTM!The underscore prefix correctly indicates the parameter is intentionally unused, following Rust conventions.
68-91: LGTM!The Update action implementation is clean and well-structured:
- Proper error handling throughout
- Good defensive check for empty models
- Creates directories before writing
- Provides clear user feedback
232-241: LGTM!The deserialization structures are well-designed with proper handling of optional fields using
#[serde(default)].
244-244: Consider using a versioned or more resilient approach to the models.dev API endpoint.The implementation calls
https://models.dev/api.jsonwith no API versioning. While models.dev is an open-source database project, its API structure and response format could change without advance notice. If the endpoint structure or schema changes, model discovery would break. Consider: documenting the expected response schema, adding error handling for schema mismatches, or exploring whether models.dev provides any versioning guidance in their API documentation.
def142b to
a337075
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/rullm-cli/src/commands/models.rs (1)
238-242: Minor: Redundant#[serde(default)]onOptiontype.The
#[serde(default)]attribute onOption<String>is unnecessary sinceOption<T>already deserializes toNonewhen the field is missing. This doesn't affect correctness but could be removed for clarity.🔎 Proposed simplification
#[derive(Deserialize)] struct ModelsDevModel { - #[serde(default)] id: Option<String>, }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)crates/rullm-cli/src/commands/models.rs(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Organize CLI commands in the `commands/` directory with separate modules for each command (auth.rs, chat.rs, models.rs, alias.rs, templates.rs)
Applied to files:
crates/rullm-cli/src/commands/models.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Define comprehensive error handling using the `LlmError` enum in `error.rs`
Applied to files:
crates/rullm-cli/src/commands/models.rs
🧬 Code graph analysis (1)
crates/rullm-cli/src/commands/models.rs (2)
crates/rullm-cli/src/commands/mod.rs (1)
new(98-103)crates/rullm-cli/src/args.rs (1)
serde_json(170-170)
🔇 Additional comments (6)
README.md (2)
69-70: Heading accurately reflects the refactored workflow.The change from the previous heading to "# Update model list" clearly communicates that this command refreshes the model cache from models.dev, which aligns with the refactoring that centralizes model discovery. This separation from the "List available models" command (line 67) helps users understand the two-step workflow: update → list.
131-132: Code fence formatting cleanup.Adding the trailing newline after the closing code fence is a minor but good cleanup that prevents linting warnings and maintains consistency.
crates/rullm-cli/src/commands/models.rs (4)
26-37: LGTM!The updated documentation clearly describes the new behavior: List shows cached models, and Update now fetches from models.dev rather than per-provider APIs.
40-45: LGTM!The underscore prefix on
_clicorrectly signals the unused parameter while maintaining the consistent signature across command handlers.
69-92: LGTM!The Update action implementation is well-structured:
- Proactively creates the parent directory before writing
- Fails early if no models are returned (prevents writing an empty cache)
- Provides clear progress and success feedback to the user
244-268: Good implementation with proper timeout handling.The function correctly fetches from https://models.dev/api.json, which is an open-source database of AI models organized by provider. The code:
- Uses a 10-second timeout to prevent hanging on network issues
- Falls back to the HashMap key when
model.idis absent- Sorts before calling
dedup()(required sincededuponly removes consecutive duplicates)- Filters to only supported providers
a337075 to
0ab49c6
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/rullm-cli/src/commands/models.rs (1)
243-268: Consider adding a User-Agent header for API requests.While the current implementation works, adding a User-Agent header is good API citizenship and can help the models.dev maintainers understand usage patterns. Some APIs may also rate-limit or block requests without identifiable User-Agent strings.
🔎 Optional enhancement to add User-Agent
async fn fetch_models_from_models_dev(supported_providers: &[String]) -> Result<Vec<String>> { let client = reqwest::Client::builder() .timeout(Duration::from_secs(10)) + .user_agent(format!("rullm-cli/{}", env!("CARGO_PKG_VERSION"))) .build()?; let response = client .get("https://models.dev/api.json") .send() .await? .error_for_status()?;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)crates/rullm-cli/src/commands/models.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Organize CLI commands in the `commands/` directory with separate modules for each command (auth.rs, chat.rs, models.rs, alias.rs, templates.rs)
Applied to files:
crates/rullm-cli/src/commands/models.rs
📚 Learning: 2025-12-21T10:42:39.384Z
Learnt from: CR
Repo: itzlambda/rullm PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T10:42:39.384Z
Learning: Define comprehensive error handling using the `LlmError` enum in `error.rs`
Applied to files:
crates/rullm-cli/src/commands/models.rs
🧬 Code graph analysis (1)
crates/rullm-cli/src/commands/models.rs (2)
crates/rullm-cli/src/commands/mod.rs (1)
new(98-103)crates/rullm-cli/src/args.rs (1)
serde_json(170-170)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build (windows-latest)
🔇 Additional comments (4)
crates/rullm-cli/src/commands/models.rs (4)
4-6: LGTM!The new imports are appropriate for the models.dev API integration—
Deserializefor JSON parsing,HashMapfor the provider response structure, andDurationfor the HTTP client timeout.
26-34: LGTM!The updated doc comments accurately describe the new behavior—listing models from the local cache and fetching from models.dev for updates.
69-92: LGTM!The update flow is clean and follows a logical sequence: fetch models → validate non-empty → construct cache → ensure directories → persist → report success. Error handling via
?propagation is appropriate for CLI usage.
233-241: LGTM!The structs are appropriately minimal—serde ignores unknown fields by default, so only deserializing the
idfield is sufficient. TheOption<String>correctly handles cases whereidmay be absent in the API response.
Summary
Replaces per-provider
list_models()API calls with a centralized fetch from models.dev, eliminating the need for API keys during model discovery.Key changes:
rullm models updatenow fetches from models.dev instead of each provider's APIlist_models()usageSummary by CodeRabbit
Refactor
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.