fix(openai): read lazily discovered models in /v1/models endpoint

[CatherineSue]

Description

Problem

The OpenAI /v1/models endpoint returns an empty model list for wildcard proxy workers. When a worker is in wildcard mode (no API key configured), refresh_external_models() correctly discovers models from backends and stores them via set_models() into models_override. However, get_models() then calls worker.models() which reads from the immutable metadata.spec.models — ignoring the lazily discovered models entirely.

Solution

• Override models() on BasicWorker to check models_override first (same pattern as supports_model() and has_models_discovered())
• Change the trait return type from &[ModelCard] to Vec<ModelCard> so the override can return owned data from the ArcSwap
• Replace RwLock<Option<WorkerModels>> with ArcSwap<WorkerModels> for lock-free reads on the hot path (supports_model is called on every request)

Changes

• worker.rs: Added BasicWorker::models() override; replaced StdRwLock with ArcSwap; simplified supports_model, set_models, has_models_discovered — no more lock guard boilerplate
• worker_builder.rs: Initialize models_override with ArcSwap::from_pointee(WorkerModels::Wildcard) instead of RwLock::new(None)

Test Plan

• cargo check -p smg passes
• cargo test -p smg -- worker — all 14 worker tests pass
• e2e local test passes

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• (Optional) Documentation updated

Summary by CodeRabbit

• Refactor
  • Improved model discovery and routing to use lock-free reads, reducing contention and improving responsiveness under concurrency.
• Breaking Change
  • Updated public model-listing behavior and override handling (interface semantics changed; consumers may need adjustments).
• Tests
  • Added coverage for wildcard discovery and override behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3c3e05 and d956731.

📒 Files selected for processing (1)

• model_gateway/src/core/worker.rs

---

📝 Walkthrough

Walkthrough

Replaced lock-based model override storage with lock-free ArcSwap in the Worker trait and BasicWorker. The Worker trait's models() now returns Vec<ModelCard> (was &[ModelCard]). BasicWorker.models_override changed from Arc<StdRwLock<Option<WorkerModels>>> to Arc<ArcSwap<WorkerModels>>> with updated implementations for model lookup and mutation.

Changes

Cohort / File(s)	Summary
Worker Trait & Implementation model_gateway/src/core/worker.rs	Changed Worker::models() signature to return Vec<ModelCard>. Replaced RwLock-based models_override with Arc<ArcSwap<WorkerModels>>. Refactored models(), supports_model(), set_models(), and has_models_discovered() to use ArcSwap for lock-free reads; added use arc_swap::ArcSwap. Updated imports and concurrency behavior.
Worker Builder / Initialization model_gateway/src/core/worker_builder.rs	Initialized BasicWorker.models_override with Arc<ArcSwap<WorkerModels>> (seeded with WorkerModels::Wildcard) instead of Arc<StdRwLock<Option<WorkerModels>>>. Added ArcSwap import and removed RwLock alias usage.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• refactor(core): consolidate DPAwareWorker into BasicWorker #434 — touches the same core structs (BasicWorker, BasicWorkerBuilder) and construction logic; likely to conflict or need coordination.

Suggested reviewers

• slin1237
• key4ng
• whybeyoung

Poem

🐰
With ArcSwap snug beneath my paw,
I swap the models—no lock-law.
Wildcards wake, overrides play,
I hop through routes in lock-free way. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing the /v1/models endpoint to read lazily discovered models for wildcard proxy workers.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chang/oai-models

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 bug where the OpenAI /v1/models endpoint failed to return lazily discovered models for wildcard proxy workers, leading to an empty model list. The solution refactors the model handling mechanism within the BasicWorker by introducing ArcSwap for efficient, lock-free access to dynamically discovered models. This ensures that the /v1/models endpoint accurately reflects all available models, enhancing the functionality and reliability of the model gateway.

Highlights

• Fixes Model Discovery: Resolved an issue where the /v1/models endpoint returned an empty list for wildcard proxy workers by ensuring lazily discovered models are correctly read.
• Introduces ArcSwap: Replaced std::sync::RwLock with arc_swap::ArcSwap for models_override, enabling lock-free reads on the hot path and improving performance.
• Overrides models() Method: Implemented an override for BasicWorker::models() to prioritize models stored in models_override, aligning with supports_model() and has_models_discovered().
• Simplifies Worker Logic: Streamlined supports_model, set_models, and has_models_discovered methods by removing RwLock boilerplate.

Changelog

• model_gateway/src/core/worker.rs
  • Replaced std::sync::RwLock with arc_swap::ArcSwap for the models_override field.
  • Modified the Worker::models trait method signature to return Vec<ModelCard> instead of &[ModelCard].
  • Implemented BasicWorker::models to check models_override first for discovered models before falling back to metadata.
  • Updated supports_model, set_models, and has_models_discovered to utilize the ArcSwap mechanism, removing RwLock read/write guards.
• model_gateway/src/core/worker_builder.rs
  • Imported the ArcSwap crate.
  • Modified the initialization of models_override to use ArcSwap::from_pointee(WorkerModels::Wildcard) instead of StdRwLock::new(None).

Activity

• The author identified a problem where the /v1/models endpoint returned an empty model list for wildcard proxy workers due to ignoring lazily discovered models.
• A solution was proposed and implemented to override models() in BasicWorker and refactor models_override to use ArcSwap.
• The changes have been tested, with cargo check -p smg and cargo test -p smg -- worker passing all 14 worker tests.
• The author has also included a checklist for cargo +nightly fmt and cargo clippy to ensure code quality.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly addresses a bug where lazily discovered models were not being returned by the /v1/models endpoint for wildcard workers. The migration from RwLock to ArcSwap is a significant improvement, enhancing performance on read-heavy hot paths and simplifying the code by removing lock management boilerplate. The modification to the Worker::models() trait method to return a Vec<ModelCard> is a logical and necessary change to support this fix. I have a couple of suggestions to further refine the code for clarity and conciseness.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@model_gateway/src/core/worker.rs`:
- Around line 657-665: Add a regression unit test that verifies the wildcard
lazy-discovery path: create a wildcard worker (the Worker/WorkerMetadata used in
tests), assert initially models() returns empty, has_models_discovered() is
false and supports_model(...) is false for a sample model; then call
set_models(...) with a Vec<ModelCard> and assert afterwards that models()
returns the new WorkerModels (match length/content), supports_model(...) returns
true for a model present in the Vec, and has_models_discovered() is true. Use
the existing Worker::set_models, Worker::models(), Worker::supports_model(), and
Worker::has_models_discovered() methods and construct ModelCard instances
consistent with other tests so the test focuses only on lazy-discovery behavior.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0b6051 and d3c3e05.

📒 Files selected for processing (2)

• model_gateway/src/core/worker.rs
• model_gateway/src/core/worker_builder.rs