Support adapter-specific chat model selection#44
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds model selection capability to the chat interface and backend. It introduces model metadata types across the stack, extends chat components and session management to track user's model choice per provider, wires model selection through the ACP session lifecycle (creation, prompting, persistence), and updates the API contracts and dispatch layer to accept model parameters during session operations. ChangesModel Selection System
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatUI as Chat UI<br/>(ChatInput)
participant SessionProvider as Session<br/>Provider
participant SessionController as Controller<br/>(use-chat-session)
participant Registry as Registry<br/>(Rust)
participant Runtime as Runtime<br/>(Rust)
participant Provider as ACP<br/>Provider
User->>ChatUI: Select model from dropdown
ChatUI->>SessionProvider: onSelectModel(modelId)
SessionProvider->>SessionProvider: selectModel(providerId, modelId)<br/>updates selectedModelByProvider
SessionProvider->>ChatUI: provides selectedModel via context
ChatUI->>ChatUI: displays selected model
User->>ChatUI: Send prompt
ChatUI->>SessionController: sendPromptForSelectedProvider<br/>with selectedModelId
SessionController->>Registry: POST /sessions/prompt<br/>{ prompt, model: selectedModelId }
Registry->>Runtime: prompt(prompt, model_id)
Runtime->>Runtime: enqueue PromptCommand<br/>{ prompt, model_id }
Runtime->>Provider: SetSessionModelRequest(model_id)<br/>(if model_id present)
Provider-->>Runtime: ack
Runtime->>Provider: send_prompt(prompt)
Provider-->>Runtime: prompt_started { model_id }
Runtime-->>Registry: command processed
Registry-->>SessionController: response
SessionController-->>ChatUI: prompt sent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 855f049789
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+378
to
+380
| let options = self.provider_model_options(&provider).await; | ||
| let (model_id, model_name) = | ||
| resolve_model_selection(&provider, input.model_id.as_deref(), &options, started.model_id.as_deref())?; |
There was a problem hiding this comment.
session.start now validates model against provider_model_options, but this map is only initialized empty in this change and is never populated on the non-test path, so options is empty here. With an explicit model parameter, resolve_model_selection returns invalid_param even for valid models, which breaks the new model-selection API path for any client that sends model.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds adapter-scoped chat model selection across the ACP backend and the gateway-admin chat UI, aiming to let the UI choose a model per provider and persist that choice through session summaries. It touches both the Rust ACP bridge (crates/lab) and the Next.js admin chat client (apps/gateway-admin).
Changes:
- Extend ACP provider/session payloads and persistence with model IDs, model names, and config/model option metadata.
- Thread model selection through ACP API handlers, dispatch, registry, and runtime prompt/session flows.
- Add gateway-admin model selection state/UI, display selected model in chat/session surfaces, and expand unit tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
crates/lab/src/dispatch/acp/persistence.rs |
Adds SQLite schema/migration support for model metadata and config option persistence. |
crates/lab/src/dispatch/acp/dispatch.rs |
Exposes model fields in provider responses and accepts model params for session start/prompt actions. |
crates/lab/src/api/services/acp.rs |
Extends HTTP session create/prompt payloads to carry model selection. |
crates/lab/src/acp/types.rs |
Adds model-related fields to ACP bridge/internal session and provider types. |
crates/lab/src/acp/runtime.rs |
Threads selected model through runtime command handling and prompt lifecycle events. |
crates/lab/src/acp/registry.rs |
Adds provider model tracking and model resolution during session creation/prompt dispatch. |
crates/lab-apis/src/acp/types.rs |
Expands shared ACP API types with model/config option structures. |
apps/gateway-admin/lib/chat/use-chat-session-controller.ts |
Adds selected-model resolution and includes model in prompt requests. |
apps/gateway-admin/lib/chat/chat-session-provider.tsx |
Stores provider-scoped selected model state and wires it into session creation/prompt flows. |
apps/gateway-admin/lib/chat/acp-normalizers.ts |
Normalizes provider/session model metadata from ACP payloads. |
apps/gateway-admin/lib/acp/types.ts |
Extends frontend ACP transport types with model metadata. |
apps/gateway-admin/components/chat/types.ts |
Extends chat-facing agent/run types with model fields. |
apps/gateway-admin/components/chat/session-sidebar.tsx |
Shows the run’s model name in the session list. |
apps/gateway-admin/components/chat/chat-shell.tsx |
Passes selected model data into the chat input and surfaces model name in the header. |
apps/gateway-admin/components/chat/chat-shell.test.tsx |
Adds unit coverage for model selection normalization and prompt payloads. |
apps/gateway-admin/components/chat/chat-input.tsx |
Adds the model picker UI alongside the existing agent picker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| item.defaultModelId === other.defaultModelId && | ||
| item.currentModelId === other.currentModelId && | ||
| models.length === otherModels.length && | ||
| models.every((model, index) => model.id === otherModels[index]?.id) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/lab/src/acp/runtime.rs (1)
467-475:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
config_optionsis still dropped on startup.This always returns
Vec::new(), so the newconfig_options_jsoncolumn will persist[]for every session. After a restart, the registry has no stored model-option metadata to validate against or to rebuild the picker for restored sessions, which undercuts the new “persist and normalize model metadata” path.🤖 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 `@crates/lab/src/acp/runtime.rs` around lines 467 - 475, The StartSessionResult currently sets config_options to Vec::new(), dropping persisted options; replace this with the persisted data from the started record (use started.config_options if that field exists, otherwise deserialize started.config_options_json via serde_json::from_str into the Vec) and propagate that value into StartSessionResult.config_options; ensure you handle/convert types as needed and fall back to an empty Vec only on deserialization errors (and optionally log the error).
🤖 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 `@apps/gateway-admin/lib/chat/use-chat-session-controller.ts`:
- Around line 130-131: The current code collapses candidate selection into one
variable then checks existence, which lets a stale requestedModelId cause
immediate fallback to models[0] and skip later valid fallbacks; change the logic
to sequentially attempt to resolve each id in order (requestedModelId, runModel,
agent?.currentModelId, agent?.defaultModelId) by using models.find(...) for each
non-null id and returning the first found model, and only if none are found
return models[0] or null; update the code paths referencing candidate,
requestedModelId, runModel, agent.currentModelId, agent.defaultModelId, and the
models.find usage accordingly to implement this stepwise lookup.
In `@crates/lab/src/acp/registry.rs`:
- Around line 214-224: Do not synthesize default_model_id or current_model_id
from the first model in provider_models; remove the lines that set
health.default_model_id = provider_models.first().map(|model| model.id.clone())
and health.current_model_id = health.default_model_id.clone(), and only update
health.models = provider_models.clone(). In other words, when iterating over
provider_healths() in the block using self.provider_models.try_read(), populate
health.models but do not overwrite health.default_model_id or
health.current_model_id—leave them None (or their existing cached values) if
they are not already set.
In `@crates/lab/src/api/services/acp.rs`:
- Around line 143-145: User-supplied model strings must be sanitized so an empty
or whitespace-only model does not override model_id; replace uses of
body.model.or(body.model_id) with logic that trims and treats "" as None (e.g.,
compute a sanitized_model from body.model by trimming and treating empty ->
None, then fallback to body.model_id), updating occurrences that reference model
and model_id (e.g., the struct fields model / model_id and call sites using
body.model.or(body.model_id)) so mixed-version clients with model: "" will
correctly fall back to model_id.
---
Outside diff comments:
In `@crates/lab/src/acp/runtime.rs`:
- Around line 467-475: The StartSessionResult currently sets config_options to
Vec::new(), dropping persisted options; replace this with the persisted data
from the started record (use started.config_options if that field exists,
otherwise deserialize started.config_options_json via serde_json::from_str into
the Vec) and propagate that value into StartSessionResult.config_options; ensure
you handle/convert types as needed and fall back to an empty Vec only on
deserialization errors (and optionally log the error).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e40fe973-e342-44da-9728-d1da85d786c8
📒 Files selected for processing (17)
apps/gateway-admin/components/chat/chat-input.tsxapps/gateway-admin/components/chat/chat-shell.test.tsxapps/gateway-admin/components/chat/chat-shell.tsxapps/gateway-admin/components/chat/session-sidebar.tsxapps/gateway-admin/components/chat/types.tsapps/gateway-admin/lib/acp/types.tsapps/gateway-admin/lib/chat/acp-normalizers.test.tsapps/gateway-admin/lib/chat/acp-normalizers.tsapps/gateway-admin/lib/chat/chat-session-provider.tsxapps/gateway-admin/lib/chat/use-chat-session-controller.tscrates/lab-apis/src/acp/types.rscrates/lab/src/acp/registry.rscrates/lab/src/acp/runtime.rscrates/lab/src/acp/types.rscrates/lab/src/api/services/acp.rscrates/lab/src/dispatch/acp/dispatch.rscrates/lab/src/dispatch/acp/persistence.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tests
pnpm exec tsx --test components/chat/chat-shell.test.tsx lib/chat/*.test.ts lib/acp/*.test.ts(38/38 passing)cargo test --manifest-path crates/lab/Cargo.toml acp --lib(85/85 passing before the lab-apis rerun)RUSTC_WRAPPER= cargo test --manifest-path crates/lab-apis/Cargo.toml acp --lib(0 tests after filter, compile passed)pnpm run build(passing)NEXT_PUBLIC_MOCK_DATA=true pnpm run buildfails in unrelated/settings/servicesmock-data prerender path; normal build passesAgent-browser verification
NEXT_PUBLIC_MOCK_DATA=true pnpm exec next dev -H 127.0.0.1 -p 3133http://127.0.0.1:3133/chat/withagent-browser 0.26.0Notes
.env,config.toml, ignored plan files, generated Next files, and node modules were not committed.Summary by cubic
Adds adapter-scoped chat model selection and applies the chosen model to new prompts and sessions. Selection is validated per provider, persisted, and reflected in the chat header, session list, and prompt events.
New Features
models,defaultModelId, andcurrentModelId. Selection is stored per provider, cleared if invalid when switching adapters, and falls back to run model → provider current/default → first option.modelis sent on session create and prompt; server acceptsmodelormodel_id(trims blanks), validates against provider options (or passes through when none cached), applies the model in the runtime, and returnsmodelId/modelNamein session summaries and prompt events.Migration
model_id,model_name, andconfig_options_json; auto-migrates on startup. Existing sessions continue to work.Written for commit 27e7f09. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features