From b60a010afd8c1f95793949b090bd0278b541a9b7 Mon Sep 17 00:00:00 2001 From: pufit Date: Thu, 6 Aug 2026 12:59:39 -0400 Subject: [PATCH] Make the composer model picker per-chat instead of global --- nerve/gateway/routes/sessions.py | 23 +++- tests/test_session_create_model.py | 149 ++++++++++++++++++++------ web/src/api/client.ts | 2 +- web/src/api/websocket.ts | 10 +- web/src/components/Chat/ChatInput.tsx | 38 +++++-- web/src/stores/chatStore.ts | 82 ++++++++------ 6 files changed, 221 insertions(+), 83 deletions(-) diff --git a/nerve/gateway/routes/sessions.py b/nerve/gateway/routes/sessions.py index c49e0c90..5df33fb9 100644 --- a/nerve/gateway/routes/sessions.py +++ b/nerve/gateway/routes/sessions.py @@ -299,7 +299,12 @@ async def get_messages(session_id: str, limit: int = 500, user: dict = Depends(r @router.patch("/api/sessions/{session_id}") async def update_session(session_id: str, req: dict, user: dict = Depends(require_auth)): - """Update session fields (title, starred). + """Update session fields (title, starred, model). + + ``model`` re-points THIS session only — the composer's picker is + per-chat, not a global preference. The engine re-reads the session + row each turn (per-message override -> ``sessions.model`` -> backend + default), so the switch takes effect on the session's next message. When ``sessions.star_project_hook`` is enabled (default off), a ``starred`` 0<->1 transition fires a one-shot internal turn in that same session so the @@ -314,6 +319,22 @@ async def update_session(session_id: str, req: dict, user: dict = Depends(requir fields["title"] = req["title"] if "starred" in req: fields["starred"] = 1 if req["starred"] else 0 + if "model" in req: + requested_model = str(req["model"] or "").strip() + if not requested_model: + raise HTTPException(status_code=400, detail="model cannot be empty") + # Backend is sticky on the session row — validate the pick against + # it (same optional seam as session creation: codex can cheaply + # reject; claude accepts any ID since Ollama models ride it). + backend_id = session.get("backend") or deps.engine.config.agent.backend + selected_backend = deps.engine._backends.get(backend_id) + validate_model = getattr(selected_backend, "validate_model", None) + if validate_model is not None: + try: + await validate_model(requested_model) + except BackendError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + fields["model"] = requested_model if not fields: raise HTTPException(status_code=400, detail="No valid fields to update") old_starred = int(session.get("starred") or 0) diff --git a/tests/test_session_create_model.py b/tests/test_session_create_model.py index a29c2162..a9da875e 100644 --- a/tests/test_session_create_model.py +++ b/tests/test_session_create_model.py @@ -1,9 +1,15 @@ -"""HTTP route tests for POST /api/sessions with a model override. +"""HTTP route tests for the composer model picker's session endpoints. -The composer's model picker sends its choice at session creation so the +POST /api/sessions: the picker sends its choice at session creation so the session row (and the header's model badge) carries the picked model from the first render — instead of the backend default until the first turn -resolves the override. Pattern mirrors test_workflow_routes.py. +resolves the override. + +PATCH /api/sessions/{id}: the picker re-points ONE existing session's +model (per-chat, not a global preference); the engine reads the row each +turn, so the switch applies on that session's next message only. + +Pattern mirrors test_workflow_routes.py. """ from __future__ import annotations @@ -57,39 +63,40 @@ async def get_or_create(self, session_id: str, **kwargs) -> dict: ) -@pytest.mark.asyncio -class TestCreateSessionModel: - @pytest_asyncio.fixture - async def setup(self, db: Database, tmp_path): - from fastapi import FastAPI - from fastapi.testclient import TestClient - - import nerve.config as cfg_mod - from nerve.config import NerveConfig - from nerve.gateway.routes._deps import init_deps - from nerve.gateway.routes.sessions import router as sessions_router - - cfg = NerveConfig() - cfg.workspace = tmp_path - cfg.auth.jwt_secret = "" # require_auth becomes a no-op - cfg_mod._config = cfg - - engine = SimpleNamespace( - _backends={ - "claude": FakeClaudeBackend(), - "codex": FakeCodexBackend(), - }, - config=cfg, - sessions=FakeSessionManager(db), - ) - init_deps(engine=engine, db=db) # type: ignore[arg-type] +@pytest_asyncio.fixture +async def setup(db: Database, tmp_path): + from fastapi import FastAPI + from fastapi.testclient import TestClient - app = FastAPI() - app.include_router(sessions_router) - yield SimpleNamespace(client=TestClient(app), db=db) + import nerve.config as cfg_mod + from nerve.config import NerveConfig + from nerve.gateway.routes._deps import init_deps + from nerve.gateway.routes.sessions import router as sessions_router - cfg_mod._config = None + cfg = NerveConfig() + cfg.workspace = tmp_path + cfg.auth.jwt_secret = "" # require_auth becomes a no-op + cfg_mod._config = cfg + engine = SimpleNamespace( + _backends={ + "claude": FakeClaudeBackend(), + "codex": FakeCodexBackend(), + }, + config=cfg, + sessions=FakeSessionManager(db), + ) + init_deps(engine=engine, db=db) # type: ignore[arg-type] + + app = FastAPI() + app.include_router(sessions_router) + yield SimpleNamespace(client=TestClient(app), db=db) + + cfg_mod._config = None + + +@pytest.mark.asyncio +class TestCreateSessionModel: async def test_model_persisted_at_creation(self, setup): resp = setup.client.post( "/api/sessions", @@ -128,3 +135,79 @@ async def test_codex_accepts_known_model(self, setup): ) assert resp.status_code == 200 assert resp.json()["model"] == "gpt-5.6-sol" + + +@pytest.mark.asyncio +class TestPatchSessionModel: + """PATCH /api/sessions/{id} with a model — the per-chat picker path.""" + + async def _create(self, setup, backend: str = "claude", model: str | None = None) -> dict: + payload: dict = {"backend": backend} + if model: + payload["model"] = model + resp = setup.client.post("/api/sessions", json=payload) + assert resp.status_code == 200 + return resp.json() + + async def test_patch_model_repoints_session(self, setup): + session = await self._create(setup, model="claude-fable-5") + resp = setup.client.patch( + f"/api/sessions/{session['id']}", json={"model": "claude-opus-5"}, + ) + assert resp.status_code == 200 + assert resp.json()["model"] == "claude-opus-5" + row = await setup.db.get_session(session["id"]) + assert row["model"] == "claude-opus-5" + + async def test_patch_model_only_touches_target_session(self, setup): + """The regression this endpoint exists for: a pick in one chat must + never change any other chat's model.""" + a = await self._create(setup, model="claude-fable-5") + b = await self._create(setup, model="claude-fable-5") + resp = setup.client.patch( + f"/api/sessions/{a['id']}", json={"model": "claude-opus-5"}, + ) + assert resp.status_code == 200 + assert (await setup.db.get_session(a["id"]))["model"] == "claude-opus-5" + assert (await setup.db.get_session(b["id"]))["model"] == "claude-fable-5" + + async def test_patch_blank_model_rejected(self, setup): + session = await self._create(setup, model="claude-fable-5") + resp = setup.client.patch( + f"/api/sessions/{session['id']}", json={"model": " "}, + ) + assert resp.status_code == 400 + assert (await setup.db.get_session(session["id"]))["model"] == "claude-fable-5" + + async def test_patch_model_validated_by_session_backend(self, setup): + session = await self._create(setup, backend="codex", model="gpt-5.6-sol") + resp = setup.client.patch( + f"/api/sessions/{session['id']}", json={"model": "claude-opus-5"}, + ) + assert resp.status_code == 400 + assert "not available" in resp.json()["detail"] + assert (await setup.db.get_session(session["id"]))["model"] == "gpt-5.6-sol" + + async def test_patch_model_codex_accepts_known(self, setup): + session = await self._create(setup, backend="codex", model="gpt-5.6-sol") + resp = setup.client.patch( + f"/api/sessions/{session['id']}", json={"model": "gpt-5.6-sol"}, + ) + assert resp.status_code == 200 + assert resp.json()["model"] == "gpt-5.6-sol" + + async def test_patch_model_unknown_session_404(self, setup): + resp = setup.client.patch( + "/api/sessions/nonexist", json={"model": "claude-opus-5"}, + ) + assert resp.status_code == 404 + + async def test_patch_title_alone_still_works(self, setup): + session = await self._create(setup, model="claude-fable-5") + resp = setup.client.patch( + f"/api/sessions/{session['id']}", json={"title": "renamed"}, + ) + assert resp.status_code == 200 + row = await setup.db.get_session(session["id"]) + assert row["title"] == "renamed" + assert row["model"] == "claude-fable-5" diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 6789f0ae..93ba873b 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -306,7 +306,7 @@ export const api = { ), deleteSession: (id: string) => request(`/sessions/${id}`, { method: 'DELETE' }), - updateSession: (id: string, data: { title?: string; starred?: boolean }) => + updateSession: (id: string, data: { title?: string; starred?: boolean; model?: string }) => request(`/sessions/${id}`, { method: 'PATCH', body: JSON.stringify(data) }), getMessages: (sessionId: string, limit = 100) => request<{ messages: any[]; last_usage?: { input_tokens: number; output_tokens: number; cache_creation_input_tokens: number; cache_read_input_tokens: number; cache_creation?: { ephemeral_5m_input_tokens?: number; ephemeral_1h_input_tokens?: number }; max_context_tokens: number; num_turns?: number } }>(`/sessions/${sessionId}/messages?limit=${limit}`), diff --git a/web/src/api/websocket.ts b/web/src/api/websocket.ts index 8095e944..b68a97f7 100644 --- a/web/src/api/websocket.ts +++ b/web/src/api/websocket.ts @@ -134,16 +134,14 @@ export class NerveWebSocket { return 'dropped'; } - sendMessage(content: string, sessionId: string, fileIds?: string[], model?: string): SendStatus { + sendMessage(content: string, sessionId: string, fileIds?: string[]): SendStatus { const msg: Record = { type: 'message', content, session_id: sessionId }; if (fileIds && fileIds.length > 0) { msg.file_ids = fileIds; } - // Per-message model override from the composer's picker (omitted → server - // uses the configured default). May be an Anthropic id or an Ollama model. - if (model) { - msg.model = model; - } + // No model field: the server resolves the session row's model each + // turn (sessions.model, set at creation or via PATCH), so the pick is + // per-chat rather than a client-global override. return this.send(msg); } diff --git a/web/src/components/Chat/ChatInput.tsx b/web/src/components/Chat/ChatInput.tsx index 9d12e849..93a8c248 100644 --- a/web/src/components/Chat/ChatInput.tsx +++ b/web/src/components/Chat/ChatInput.tsx @@ -103,15 +103,21 @@ export function ChatInput({ onSend, onStop, isStreaming, disabled }: { ? (chosenBackend ?? 'claude') : (sessions.find(s => s.id === activeSession)?.backend ?? 'claude'); - // ── Model picker ── + // ── Model picker (per-chat) ── + // A virtual chat's pick lives in newChatModels until the session is + // created; a real session's model IS its row (sessions[].model), so the + // picker always shows — and only ever changes — the current chat. const availableModels = useChatStore(s => s.availableModels); - const selectedModels = useChatStore(s => s.selectedModels); + const newChatModels = useChatStore(s => s.newChatModels); const modelDefaults = useChatStore(s => s.modelDefaults); - const setSelectedModel = useChatStore(s => s.setSelectedModel); + const setNewChatModel = useChatStore(s => s.setNewChatModel); + const setSessionModel = useChatStore(s => s.setSessionModel); const loadModels = useChatStore(s => s.loadModels); const scopedModels = availableModels.filter(m => m.backend === activeBackend); - const selectedModel = selectedModels[activeBackend] ?? null; const modelsDefault = modelDefaults[activeBackend] ?? null; + const currentModel = isVirtualChat + ? (newChatModels[activeBackend] ?? modelsDefault) + : (sessions.find(s => s.id === activeSession)?.model ?? modelsDefault); const [prevQuoteCount, setPrevQuoteCount] = useState(0); @@ -613,16 +619,30 @@ export function ChatInput({ onSend, onStop, isStreaming, disabled }: { {isVirtualChat && ( )} - {/* Backend-scoped model picker. A Claude/Ollama selection can never - leak into Codex (or vice versa) when the backend changes. */} + {/* Backend-scoped, PER-CHAT model picker. A pick here re-points + only this chat: virtual chats bind it at creation, real + sessions PATCH their own row — never a global preference. */} {scopedModels.length > 1 && (