feat: add GET /api/models + POST /api/models/switch + ModelSelector UI Fixes <#302>#304
Conversation
AnkanMisra#302) - Add GET /api/models endpoint to list available AI models - Add POST /api/models/switch endpoint for runtime model switching - Add ModelSelector React component for model selection UI - Add OpenAPI spec entries for both endpoints - Add unit tests for model routes - Fix: Register routes in gateway/routes.go
|
@prince-pokharna is attempting to deploy a commit to the ankanmisra's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds gateway endpoints for listing and switching AI models, documents and registers those routes, and introduces a web UI selector for session-scoped model changes with supporting tests. ChangesAI model selection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ModelSelector
participant Gateway
participant Ollama
User->>ModelSelector: Open summarization page
ModelSelector->>Gateway: GET /api/models
Gateway->>Ollama: GET /api/tags
Ollama-->>Gateway: Available models
Gateway-->>ModelSelector: Model list and active model
User->>ModelSelector: Select model
ModelSelector->>Gateway: POST /api/models/switch
Gateway->>Ollama: Validate model
Gateway-->>ModelSelector: Session-only switch confirmation
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
web/src/components/ModelSelector.tsx (1)
93-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
outline-hiddento fully remove the outline in Tailwind v4.In Tailwind CSS v4,
outline-noneonly removes the outline style, which can leave a visible focus ring conflict behind. Useoutline-hiddento remove the outline completely.♻️ Proposed fix
className="flex-1 rounded border border-gray-300 bg-white px-2 py-1 - text-sm focus:outline-none focus:ring-2 focus:ring-blue-400 + text-sm focus:outline-hidden focus:ring-2 focus:ring-blue-400 disabled:opacity-50 disabled:cursor-not-allowed"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/ModelSelector.tsx` around lines 93 - 95, Update the focus styling in the ModelSelector component’s select element by replacing the Tailwind outline-none utility with outline-hidden, while preserving the existing focus ring and other classes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/routes/models.go`:
- Around line 118-133: Update handleOpenRouterModels to obtain the model through
the session-scoped activeModelOverride used by the model-switch flow, rather
than reading OPENROUTER_MODEL directly. Ensure the response’s currentModel and
active model entry reflect the override after /api/models/switch, while
preserving the existing environment-based value as the fallback when no override
is set.
- Around line 76-86: Respect incoming request context deadlines in all outbound
HTTP calls to the Ollama daemon to prevent goroutine leaks when clients
disconnect. In gateway/routes/models.go at lines 76-86 (handleOllamaModels),
replace the client.Get call with http.NewRequestWithContext passing
c.Request.Context(), then execute the request using client.Do. At lines 175-184
(validateOllamaModelExists), add a ctx context.Context parameter to the function
signature and replace client.Get with http.NewRequestWithContext(ctx, ...) and
client.Do. At lines 154-156 (SwitchModel), update the validateOllamaModelExists
call to pass c.Request.Context() as the new ctx argument. Ensure the context
package is imported in models.go.
In `@web/src/components/ModelSelector.tsx`:
- Line 29: Update the gatewayURL fallback in ModelSelector to use port 3000
instead of 8080, while preserving the existing NEXT_PUBLIC_GATEWAY_URL override
behavior.
- Around line 71-73: Update the render guard in ModelSelector so an initial
fetch error is displayed instead of being hidden when data.models is empty.
Check and render the existing error state before the single-model/empty-model
null return, while preserving the loading behavior and hiding the selector when
there is no error and one or fewer models.
---
Nitpick comments:
In `@web/src/components/ModelSelector.tsx`:
- Around line 93-95: Update the focus styling in the ModelSelector component’s
select element by replacing the Tailwind outline-none utility with
outline-hidden, while preserving the existing focus ring and other classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 725fe045-6d78-4a1a-b2ff-546904bb411f
📒 Files selected for processing (7)
gateway/main.gogateway/openapi.yamlgateway/routes.gogateway/routes/models.gogateway/routes/models_test.goweb/src/app/page.tsxweb/src/components/ModelSelector.tsx
| // Use a short timeout — Ollama is local; if it doesn't respond in 3s it's down | ||
| client := &http.Client{Timeout: 3 * time.Second} | ||
| resp, err := client.Get(fmt.Sprintf("%s/api/tags", ollamaURL)) | ||
| if err != nil { | ||
| c.JSON(http.StatusServiceUnavailable, gin.H{ | ||
| "error": "Cannot connect to Ollama. Is Ollama running? (expected at: " + ollamaURL + ")", | ||
| "provider": "ollama", | ||
| "hint": "Run `ollama serve` or check OLLAMA_URL in your .env", | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Respect context deadlines in outbound Ollama HTTP calls.
Both the model-discovery and model-validation paths construct an HTTP request to query the local Ollama daemon, but they use client.Get() without attaching the incoming request context. As per coding guidelines, every outbound AI call must respect context deadlines to prevent goroutine leaks if the client disconnects or the parent context times out.
gateway/routes/models.go#L76-L86: Replaceclient.Getwithhttp.NewRequestWithContext(c.Request.Context(), ...)inhandleOllamaModels, and execute it usingclient.Do(req).gateway/routes/models.go#L175-L184: UpdatevalidateOllamaModelExiststo accept actx context.Contextparameter, and usehttp.NewRequestWithContext(ctx, ...)instead ofclient.Get.gateway/routes/models.go#L154-L156: Update thevalidateOllamaModelExistscall insideSwitchModelto passc.Request.Context().
(Note: Ensure thecontextpackage is imported inmodels.goafter making these changes).
📍 Affects 1 file
gateway/routes/models.go#L76-L86(this comment)gateway/routes/models.go#L175-L184gateway/routes/models.go#L154-L156
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/routes/models.go` around lines 76 - 86, Respect incoming request
context deadlines in all outbound HTTP calls to the Ollama daemon to prevent
goroutine leaks when clients disconnect. In gateway/routes/models.go at lines
76-86 (handleOllamaModels), replace the client.Get call with
http.NewRequestWithContext passing c.Request.Context(), then execute the request
using client.Do. At lines 175-184 (validateOllamaModelExists), add a ctx
context.Context parameter to the function signature and replace client.Get with
http.NewRequestWithContext(ctx, ...) and client.Do. At lines 154-156
(SwitchModel), update the validateOllamaModelExists call to pass
c.Request.Context() as the new ctx argument. Ensure the context package is
imported in models.go.
Source: Coding guidelines
| func handleOpenRouterModels(c *gin.Context) { | ||
| currentModel := os.Getenv("OPENROUTER_MODEL") | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "provider": "openrouter", | ||
| "currentModel": currentModel, | ||
| "models": []gin.H{ | ||
| { | ||
| "id": currentModel, | ||
| "name": currentModel, | ||
| "provider": "openrouter", | ||
| "isActive": true, | ||
| }, | ||
| }, | ||
| "note": "OpenRouter supports 300+ models. Update OPENROUTER_MODEL in .env to change the model.", | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the active model override for OpenRouter.
handleOpenRouterModels reads the model directly from the OPENROUTER_MODEL environment variable, ignoring the session-scoped activeModelOverride. If a user successfully switches an OpenRouter model via /api/models/switch, subsequent calls to GetAvailableModels will incorrectly report the original environment variable model as active, causing the UI to instantly revert.
🐛 Proposed fix to respect the session override
func handleOpenRouterModels(c *gin.Context) {
- currentModel := os.Getenv("OPENROUTER_MODEL")
+ modelMu.RLock()
+ currentModel := activeModelOverride
+ modelMu.RUnlock()
+
+ if currentModel == "" {
+ currentModel = os.Getenv("OPENROUTER_MODEL")
+ }
+
c.JSON(http.StatusOK, gin.H{
"provider": "openrouter",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func handleOpenRouterModels(c *gin.Context) { | |
| currentModel := os.Getenv("OPENROUTER_MODEL") | |
| c.JSON(http.StatusOK, gin.H{ | |
| "provider": "openrouter", | |
| "currentModel": currentModel, | |
| "models": []gin.H{ | |
| { | |
| "id": currentModel, | |
| "name": currentModel, | |
| "provider": "openrouter", | |
| "isActive": true, | |
| }, | |
| }, | |
| "note": "OpenRouter supports 300+ models. Update OPENROUTER_MODEL in .env to change the model.", | |
| }) | |
| } | |
| func handleOpenRouterModels(c *gin.Context) { | |
| modelMu.RLock() | |
| currentModel := activeModelOverride | |
| modelMu.RUnlock() | |
| if currentModel == "" { | |
| currentModel = os.Getenv("OPENROUTER_MODEL") | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "provider": "openrouter", | |
| "currentModel": currentModel, | |
| "models": []gin.H{ | |
| { | |
| "id": currentModel, | |
| "name": currentModel, | |
| "provider": "openrouter", | |
| "isActive": true, | |
| }, | |
| }, | |
| "note": "OpenRouter supports 300+ models. Update OPENROUTER_MODEL in .env to change the model.", | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gateway/routes/models.go` around lines 118 - 133, Update
handleOpenRouterModels to obtain the model through the session-scoped
activeModelOverride used by the model-switch flow, rather than reading
OPENROUTER_MODEL directly. Ensure the response’s currentModel and active model
entry reflect the override after /api/models/switch, while preserving the
existing environment-based value as the fallback when no override is set.
| const [switching, setSwitching] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| const gatewayURL = process.env.NEXT_PUBLIC_GATEWAY_URL ?? "http://localhost:8080"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the correct default port for the Gateway.
As per coding guidelines, the fallback Gateway port must be 3000.
🔧 Proposed fix
- const gatewayURL = process.env.NEXT_PUBLIC_GATEWAY_URL ?? "http://localhost:8080";
+ const gatewayURL = process.env.NEXT_PUBLIC_GATEWAY_URL ?? "http://localhost:3000";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const gatewayURL = process.env.NEXT_PUBLIC_GATEWAY_URL ?? "http://localhost:8080"; | |
| const gatewayURL = process.env.NEXT_PUBLIC_GATEWAY_URL ?? "http://localhost:3000"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/ModelSelector.tsx` at line 29, Update the gatewayURL
fallback in ModelSelector to use port 3000 instead of 8080, while preserving the
existing NEXT_PUBLIC_GATEWAY_URL override behavior.
Source: Coding guidelines
| // Don't render anything while loading or if only one model (nothing to choose) | ||
| if (loading) return null; | ||
| if (!data || (data.models?.length ?? 0) <= 1) return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Display the error message if the initial fetch fails.
If the gateway returns an error during the initial load (e.g., a 503 because Ollama is unreachable), data.models will be empty. This triggers the <= 1 condition, returning null and silently hiding the helpful backend error message from the user.
🐛 Proposed fix to surface the error
// Don't render anything while loading or if only one model (nothing to choose)
if (loading) return null;
+ if (error && (!data || (data.models?.length ?? 0) === 0)) {
+ return <div className="text-sm text-red-500">{error}</div>;
+ }
if (!data || (data.models?.length ?? 0) <= 1) return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Don't render anything while loading or if only one model (nothing to choose) | |
| if (loading) return null; | |
| if (!data || (data.models?.length ?? 0) <= 1) return null; | |
| // Don't render anything while loading or if only one model (nothing to choose) | |
| if (loading) return null; | |
| if (error && (!data || (data.models?.length ?? 0) === 0)) { | |
| return <div className="text-sm text-red-500">{error}</div>; | |
| } | |
| if (!data || (data.models?.length ?? 0) <= 1) return null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/ModelSelector.tsx` around lines 71 - 73, Update the render
guard in ModelSelector so an initial fetch error is displayed instead of being
hidden when data.models is empty. Check and render the existing error state
before the single-model/empty-model null return, while preserving the loading
behavior and hiding the selector when there is no error and one or fewer models.
|
@macroscope-app review this pr |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
I've started reviewing this PR. The review is in progress and results will be posted as check runs when complete. |
Co-authored-by: codex <codex@users.noreply.github.com>
7cb458c to
4d3ee2e
Compare
…/prince-pokharna/MicroAI-Paygate into fix/model-get-post-modelselector
| return err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
There was a problem hiding this comment.
🟡 Medium routes/models.go:186
validateOllamaModelExists does not check resp.StatusCode, so when Ollama returns a non-2xx response (e.g. a 500 error), the JSON error body decodes into an empty Models slice and the function returns model not found. SwitchModel then reports the model is not installed and tells the user to pull it, masking the actual Ollama service failure. Check the status code and return an error for non-2xx responses before decoding.
🤖 Copy this AI Prompt to have your agent fix this:
In file @gateway/routes/models.go around line 186:
`validateOllamaModelExists` does not check `resp.StatusCode`, so when Ollama returns a non-2xx response (e.g. a 500 error), the JSON error body decodes into an empty `Models` slice and the function returns `model not found`. `SwitchModel` then reports the model is not installed and tells the user to pull it, masking the actual Ollama service failure. Check the status code and return an error for non-2xx responses before decoding.
Summary
Type Of Change
Affected Areas
gateway/)verifier/)web/)tests/,run_e2e.sh)bench/)deploy/, Docker, env, workflows)Contributor Checklist
Verification
List the exact commands you ran and their result.
Screenshots
Required for visible web UI changes. Remove this section if not applicable.
Notes For Reviewers
Call out risky areas, follow-up work, or known limitations.
Note
Add GET /api/models and POST /api/models/switch endpoints with ModelSelector UI
GET /api/modelshandler inroutes/models.gothat returns installed models for Ollama (querying/api/tags) or the configured model for OpenRouter, marking the active model in each response.POST /api/models/switchhandler that validates the requested model exists (Ollama only) and stores the override in-memory under a mutex, scoped to the running session.routes.goand documents them inopenapi.yaml.ModelSelectorReact component inweb/src/components/ModelSelector.tsxthat fetches available models on mount and posts to/api/models/switch; the component hides itself when only one or zero models are available.📊 Macroscope summarized b529c47. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Summary by CodeRabbit
New Features
Bug Fixes