From ce5c496e81008911498ad3c30ed6c6f879266b33 Mon Sep 17 00:00:00 2001 From: Nick Charney Kaye Date: Tue, 14 Jul 2026 05:15:28 -0700 Subject: [PATCH 1/4] Fix handleResponse to return ok:true for 204 No Content instead of a parse error JSON.parse('') throws on empty bodies, causing POST /api/setup/outcome (which returns 204) to report a runtime-unreachable error to the caller. The server commits before responding, so the data was always written, but the client-side error was misleading. Return { ok: true, data: undefined } for 204 responses. Co-Authored-By: Claude Sonnet 4.6 --- apps/web/src/api/client.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/web/src/api/client.ts b/apps/web/src/api/client.ts index 0627cc6b..f318ead8 100644 --- a/apps/web/src/api/client.ts +++ b/apps/web/src/api/client.ts @@ -184,6 +184,10 @@ async function handleResponse(res: Response): Promise> { if (!res.ok) { return { ok: false, error: await errorFromResponse(res) } } + // 204 No Content — the server intentionally sent no body. + if (res.status === 204) { + return { ok: true, data: undefined as T } + } let text = '' try { text = await res.text() } catch { /* ignore */ } let data: T From 26152a1a47b2810b5d7d6d5df98ca6b2cb26f849 Mon Sep 17 00:00:00 2001 From: Nick Charney Kaye Date: Tue, 14 Jul 2026 05:15:44 -0700 Subject: [PATCH 2/4] Fix Try New: create tutorial session directly and use scripted runtime for turns Two bugs caused the First Words tutorial to fail on first open: 1. handleStartTutorial() navigated to /setup/first_words_tutorial (a form that required LLM readiness), instead of starting the session immediately. The fix creates the tutorial session directly via api.createSession() and navigates to /conversation/:sessionId, bypassing the setup form entirely. The fallback to the setup form is kept for cases where session creation fails unexpectedly. 2. submit_turn and generate_debrief always used app.state.runtime (the fake runtime started at process startup), ignoring the active config written by use_model. This caused tutorial sessions to produce generic fake responses ("Hello there. I am a simulated NPC.") instead of the authored scripted tutorial content. The fix resolves the runtime from the active DB config so use_model({ runtime_id: 'scripted' }) actually takes effect for play. Co-Authored-By: Claude Sonnet 4.6 --- .../web/src/__tests__/FirstRunWizard.test.tsx | 23 ++--- .../src/setup/__tests__/useSetupFlow.test.ts | 50 +++++++++++ apps/web/src/setup/useSetupFlow.ts | 62 ++++++++++---- .../convsim_core/routers/sessions.py | 26 +++++- .../tests/test_scripted_runtime.py | 83 +++++++++++++++++++ 5 files changed, 216 insertions(+), 28 deletions(-) diff --git a/apps/web/src/__tests__/FirstRunWizard.test.tsx b/apps/web/src/__tests__/FirstRunWizard.test.tsx index 3e879403..fabf3170 100644 --- a/apps/web/src/__tests__/FirstRunWizard.test.tsx +++ b/apps/web/src/__tests__/FirstRunWizard.test.tsx @@ -22,6 +22,7 @@ vi.mock('../api/client', () => ({ startSetupInstall: vi.fn(), getSetupInstallStatus: vi.fn(), cancelSetupInstall: vi.fn(), + createSession: vi.fn().mockResolvedValue({ ok: true, data: { session_id: 'sess-tutorial-1', scenario_id: 'first_words_tutorial', state: 'NotStarted', created_at: '', setup: { scenario_id: 'first_words_tutorial', difficulty: 'standard', player_role_name: 'New Player', language: 'en', input_mode: 'text-only', tts_enabled: false, show_state_meters: true, save_transcript: true, seed: null } } }), }, })) @@ -108,6 +109,7 @@ function renderWizard() { } /> } /> } /> + } /> , ) @@ -158,6 +160,7 @@ beforeEach(() => { mockApi.benchmarkModel.mockResolvedValue({ ok: true, data: DEFAULT_BENCHMARK }) mockApi.recordOnboardingOutcome.mockResolvedValue({ ok: true, data: undefined }) mockApi.getSetupStatus.mockResolvedValue({ ok: true, data: { kind: 'ready' } }) + mockApi.createSession.mockResolvedValue({ ok: true, data: { session_id: 'sess-tutorial-1', scenario_id: 'first_words_tutorial', state: 'NotStarted' as const, created_at: '', setup: { scenario_id: 'first_words_tutorial', difficulty: 'standard' as const, player_role_name: 'New Player', language: 'en', input_mode: 'text-only' as const, tts_enabled: false, show_state_meters: true, save_transcript: true, seed: null } } }) }) // ── Welcome step ───────────────────────────────────────────────────────────── @@ -213,16 +216,16 @@ describe('FirstRunWizard — welcome step', () => { expect(mockApi.startSetupInstall).toHaveBeenCalledWith('qwen3-4b-instruct-q4_k_m') }) - it('Try it right now starts the scripted tutorial (navigates to the tutorial scenario setup route)', async () => { + it('Try it right now starts the scripted tutorial (navigates directly to the conversation)', async () => { renderWizard() fireEvent.click(screen.getByRole('button', { name: /try it right now/i })) - await waitFor(() => expect(screen.getByTestId('setup-page')).toBeInTheDocument()) + await waitFor(() => expect(screen.getByTestId('conversation-page')).toBeInTheDocument()) }) it('marks setup complete when Try it right now is clicked', async () => { renderWizard() fireEvent.click(screen.getByRole('button', { name: /try it right now/i })) - await waitFor(() => expect(screen.getByTestId('setup-page')).toBeInTheDocument()) + await waitFor(() => expect(screen.getByTestId('conversation-page')).toBeInTheDocument()) expect(localStorage.getItem(SETUP_KEYS.firstRunComplete)).toBe('true') }) @@ -1083,17 +1086,17 @@ describe('FirstRunWizard — "Try it right now" tutorial path', () => { ) }) - it('navigates to the tutorial and marks setup complete', async () => { + it('navigates directly to the conversation and marks setup complete', async () => { renderWizard() fireEvent.click(screen.getByRole('button', { name: /try it right now/i })) - await waitFor(() => expect(screen.getByTestId('setup-page')).toBeInTheDocument()) + await waitFor(() => expect(screen.getByTestId('conversation-page')).toBeInTheDocument()) expect(localStorage.getItem(SETUP_KEYS.firstRunComplete)).toBe('true') }) it('labels the session as scripted via localStorage', async () => { renderWizard() fireEvent.click(screen.getByRole('button', { name: /try it right now/i })) - await waitFor(() => expect(screen.getByTestId('setup-page')).toBeInTheDocument()) + await waitFor(() => expect(screen.getByTestId('conversation-page')).toBeInTheDocument()) expect(localStorage.getItem(SETUP_KEYS.activeRuntimeHint)).toBe('scripted') }) @@ -1101,7 +1104,7 @@ describe('FirstRunWizard — "Try it right now" tutorial path', () => { mockApi.useModel.mockResolvedValue({ ok: false, error: { kind: 'network', message: 'runtime unavailable' } }) renderWizard() fireEvent.click(screen.getByRole('button', { name: /try it right now/i })) - await waitFor(() => expect(screen.getByTestId('setup-page')).toBeInTheDocument()) + await waitFor(() => expect(screen.getByTestId('conversation-page')).toBeInTheDocument()) }) }) @@ -1151,7 +1154,7 @@ describe('FirstRunWizard — tutorial CTA during install', () => { await waitFor(() => expect(mockApi.useModel).toHaveBeenCalledWith({ runtime_id: 'scripted', model_id: null }), ) - await screen.findByTestId('setup-page') + await screen.findByTestId('conversation-page') }) it('marks tutorial complete in localStorage when starting the tutorial', async () => { @@ -1188,10 +1191,10 @@ describe('FirstRunWizard — tutorial CTA during install', () => { ) }) - it('navigates to the tutorial scenario setup route (a real, mounted route)', async () => { + it('navigates directly to the tutorial conversation (a real, mounted route)', async () => { await goToInstalling() fireEvent.click(screen.getByRole('button', { name: /start now/i })) - await screen.findByTestId('setup-page') + await screen.findByTestId('conversation-page') }) }) diff --git a/apps/web/src/setup/__tests__/useSetupFlow.test.ts b/apps/web/src/setup/__tests__/useSetupFlow.test.ts index 3d2493df..cdb435e5 100644 --- a/apps/web/src/setup/__tests__/useSetupFlow.test.ts +++ b/apps/web/src/setup/__tests__/useSetupFlow.test.ts @@ -29,12 +29,19 @@ vi.mock('../../api/client', () => ({ startSetupInstall: vi.fn(), getSetupInstallStatus: vi.fn(), cancelSetupInstall: vi.fn(), + createSession: vi.fn().mockResolvedValue({ ok: true, data: { session_id: 'sess-tutorial-1', scenario_id: 'first_words_tutorial', state: 'NotStarted' as const, created_at: '', setup: { scenario_id: 'first_words_tutorial', difficulty: 'standard' as const, player_role_name: 'New Player', language: 'en', input_mode: 'text-only' as const, tts_enabled: false, show_state_meters: true, save_transcript: true, seed: null } } }), }, })) import { api } from '../../api/client' const mockApi = vi.mocked(api) +const mockNavigate = vi.fn() +vi.mock('react-router-dom', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useNavigate: () => mockNavigate } +}) + import type { ModelsResponse, PreflightResponse } from '@convsim/shared' const MODELS_DATA: ModelsResponse = { @@ -102,12 +109,14 @@ const RUNNING_JOB = { beforeEach(() => { vi.clearAllMocks() + mockNavigate.mockReset() localStorage.clear() mockApi.getModels.mockResolvedValue({ ok: true, data: MODELS_DATA }) mockApi.preflight.mockResolvedValue({ ok: true, data: PREFLIGHT_PASS }) mockApi.startSetupInstall.mockResolvedValue({ ok: true, data: RUNNING_JOB }) mockApi.getSetupInstallStatus.mockResolvedValue({ ok: true, data: RUNNING_JOB }) mockApi.cancelSetupInstall.mockResolvedValue({ ok: true, data: undefined }) + mockApi.createSession.mockResolvedValue({ ok: true, data: { session_id: 'sess-tutorial-1', scenario_id: 'first_words_tutorial', state: 'NotStarted' as const, created_at: '', setup: { scenario_id: 'first_words_tutorial', difficulty: 'standard' as const, player_role_name: 'New Player', language: 'en', input_mode: 'text-only' as const, tts_enabled: false, show_state_meters: true, save_transcript: true, seed: null } } }) }) describe('useSetupFlow step machine', () => { @@ -452,6 +461,47 @@ describe('useSetupFlow step machine', () => { expect(localStorage.getItem('convsim.tutorial.complete')).toBe('true') }) + it('handleStartTutorial creates tutorial session directly, bypassing setup form', async () => { + mockApi.useModel.mockResolvedValue(SCRIPTED_RUNTIME_RESPONSE) + const { result } = renderHook(() => useSetupFlow('welcome'), { wrapper }) + + await act(async () => { await result.current.handleStartTutorial() }) + + expect(mockApi.createSession).toHaveBeenCalledWith({ + scenario_id: 'first_words_tutorial', + difficulty: 'standard', + player_role_name: 'New Player', + language: 'en', + input_mode: 'text-only', + tts_enabled: false, + show_state_meters: true, + save_transcript: true, + seed: null, + }) + }) + + it('handleStartTutorial navigates to /conversation/:sessionId on successful session creation', async () => { + mockApi.useModel.mockResolvedValue(SCRIPTED_RUNTIME_RESPONSE) + const { result } = renderHook(() => useSetupFlow('welcome'), { wrapper }) + + await act(async () => { await result.current.handleStartTutorial() }) + + expect(mockNavigate).toHaveBeenCalledWith( + '/conversation/sess-tutorial-1', + expect.objectContaining({ state: expect.objectContaining({ scenario_id: 'first_words_tutorial', show_state_meters: true }) }), + ) + }) + + it('handleStartTutorial falls back to /setup/first_words_tutorial when createSession fails', async () => { + mockApi.useModel.mockResolvedValue(SCRIPTED_RUNTIME_RESPONSE) + mockApi.createSession.mockResolvedValue({ ok: false, error: { kind: 'runtime-unreachable', message: 'Core not running' } }) + const { result } = renderHook(() => useSetupFlow('welcome'), { wrapper }) + + await act(async () => { await result.current.handleStartTutorial() }) + + expect(mockNavigate).toHaveBeenCalledWith('/setup/first_words_tutorial') + }) + it('install-complete effect clears runtime hint keys so real-model sessions are not mislabeled', async () => { // Seed localStorage with stale keys that would have been written by handleStartTutorial localStorage.setItem('convsim.active_runtime_hint', 'scripted') diff --git a/apps/web/src/setup/useSetupFlow.ts b/apps/web/src/setup/useSetupFlow.ts index fee271de..4b726cb8 100644 --- a/apps/web/src/setup/useSetupFlow.ts +++ b/apps/web/src/setup/useSetupFlow.ts @@ -363,23 +363,53 @@ export function useSetupFlow( // called here. const activeInstallId = installId setActionLoading(true) - try { await api.useModel({ runtime_id: 'scripted', model_id: null }) } catch { /* best-effort */ } - finally { setActionLoading(false) } - markTutorialComplete() - // Label all scripted sessions so the user knows they are not talking to the AI. - try { localStorage.setItem(SETUP_KEYS.activeRuntimeHint, 'scripted') } catch { /* ignore */ } - if (activeInstallId != null) { - // A background download is running. Tell Conversation.tsx so it can show - // the model-ready toast when the pipeline finishes. - try { localStorage.setItem(SETUP_KEYS.tutorialInstallId, String(activeInstallId)) } catch { /* ignore */ } - // Record a 'completed-with-model' outcome optimistically — the user already - // committed to the full install path. - await markFirstRunComplete() - } else { - // "Try it right now" path: no install in progress. - await markDemoComplete() + try { + try { await api.useModel({ runtime_id: 'scripted', model_id: null }) } catch { /* best-effort */ } + markTutorialComplete() + // Label all scripted sessions so the user knows they are not talking to the AI. + try { localStorage.setItem(SETUP_KEYS.activeRuntimeHint, 'scripted') } catch { /* ignore */ } + if (activeInstallId != null) { + // A background download is running. Tell Conversation.tsx so it can show + // the model-ready toast when the pipeline finishes. + try { localStorage.setItem(SETUP_KEYS.tutorialInstallId, String(activeInstallId)) } catch { /* ignore */ } + // Record a 'completed-with-model' outcome optimistically — the user already + // committed to the full install path. + await markFirstRunComplete() + } else { + // "Try it right now" path: no install in progress. + await markDemoComplete() + } + // Create the tutorial session directly so the user goes straight to the + // conversation without having to click through the scenario setup form. + // The scripted runtime needs no model, so this always succeeds on first open. + const sessionResult = await api.createSession({ + scenario_id: 'first_words_tutorial', + difficulty: 'standard', + player_role_name: 'New Player', + language: 'en', + input_mode: 'text-only', + tts_enabled: false, + show_state_meters: true, + save_transcript: true, + seed: null, + }) + if (sessionResult.ok) { + navigate(`/conversation/${sessionResult.data.session_id}`, { + state: { + language: 'en', + show_state_meters: true, + scenario_id: 'first_words_tutorial', + input_mode: 'text-only', + tts_enabled: false, + }, + }) + } else { + // Fall back to the setup form if session creation unexpectedly fails. + navigate('/setup/first_words_tutorial') + } + } finally { + setActionLoading(false) } - navigate('/setup/first_words_tutorial') } async function handleCancelInstall() { diff --git a/services/convsim-core/convsim_core/routers/sessions.py b/services/convsim-core/convsim_core/routers/sessions.py index f30fce0f..88386527 100644 --- a/services/convsim-core/convsim_core/routers/sessions.py +++ b/services/convsim-core/convsim_core/routers/sessions.py @@ -24,10 +24,12 @@ from fastapi.responses import PlainTextResponse from pydantic import BaseModel, field_validator +from convsim_core.runtime import build_runtime from convsim_core.scenario_state import build_variable_defs, partition_state_by_visibility from convsim_core.scenarios import get_scenario_info from convsim_core.services.branch_service import fork_session from convsim_core.services.debrief_engine import generate_debrief +from convsim_core.services.model_manager_service import get_active_config from convsim_core.services.relationship_memory import update_relationship_memory from convsim_core.services.timing import thinking_pause_ms_for_difficulty from convsim_core.services.transcript_export import format_transcript_as_markdown @@ -332,6 +334,26 @@ def _row_to_response(row: Any) -> SessionResponse: ) +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _resolve_runtime(request: Request): # type: ignore[return] + """Return the scripted or fake runtime when the active DB config selects one. + + The scripted and fake runtimes are stateless and cheap to instantiate per + request. For sidecar-based runtimes (llama.cpp, Ollama) — and when no + explicit selection exists — the shared app.state.runtime is returned to + preserve connection-pool state and support test-injected runtimes. + """ + active_cfg = get_active_config(request.app.state.db.connection()) + runtime_id = active_cfg.get("runtime_id") + if runtime_id in ("scripted", "fake"): + return build_runtime(runtime_id) + return request.app.state.runtime + + # --------------------------------------------------------------------------- # Routes # --------------------------------------------------------------------------- @@ -487,7 +509,7 @@ async def submit_turn(session_id: str, body: TurnSubmitRequest, request: Request scenario_data = info.get_scenario_data(difficulty) max_turns = info.max_turns - runtime = request.app.state.runtime + runtime = _resolve_runtime(request) save_transcript = setup.get("save_transcript", True) source_mode = setup.get("input_mode", "text-only") @@ -727,7 +749,7 @@ async def create_debrief(session_id: str, request: Request) -> DebriefResponse: raise HTTPException(status_code=500, detail=f"Scenario {scenario_id!r} not found in registry") scenario_data = info.get_scenario_data(difficulty) - runtime = request.app.state.runtime + runtime = _resolve_runtime(request) try: result = await generate_debrief( diff --git a/services/convsim-core/tests/test_scripted_runtime.py b/services/convsim-core/tests/test_scripted_runtime.py index 7dacb383..bf9fc4ae 100644 --- a/services/convsim-core/tests/test_scripted_runtime.py +++ b/services/convsim-core/tests/test_scripted_runtime.py @@ -310,3 +310,86 @@ def test_scripted_debrief_passes_debrief_validation(): narrative = _validate_narrative(_DEBRIEF_RESPONSE) assert narrative.used_fallback is False assert narrative.turning_points # scripted debrief keeps its turning point + + +# ── Session router integration — scripted runtime selection (issue #427) ────── +# When use_model selects the "scripted" runtime, submit_turn and generate_debrief +# must use ScriptedChatRuntime (not the fake runtime stored in app.state.runtime). +# These tests drive the tutorial scenario via the HTTP API to confirm end-to-end. + +_TUTORIAL_SESSION_SETUP = { + "scenario_id": "first_words_tutorial", + "difficulty": "standard", + "player_role_name": "New Player", + "language": "en", + "input_mode": "text-only", + "tts_enabled": False, + "show_state_meters": True, + "save_transcript": True, + "seed": None, +} + + +def test_tutorial_turn_uses_scripted_runtime_when_active_config_is_scripted(tmp_config): + """When active_runtime_id='scripted', submit_turn produces the tutorial script, not a fake response.""" + from convsim_core.app import create_app + from convsim_core.services.model_manager_service import set_active_config + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + # Seed the official packs so first_words_tutorial is resolvable. + import convsim_core.scenarios # noqa: F401 + set_active_config(app.state.db.connection(), runtime_id="scripted") + + res = client.post("/api/sessions", json=_TUTORIAL_SESSION_SETUP) + assert res.status_code == 201, res.text + session_id = res.json()["session_id"] + + client.post(f"/api/sessions/{session_id}/start") + turn_res = client.post( + f"/api/sessions/{session_id}/turn", + json={"content": "Hello, let's get started!"}, + ) + assert turn_res.status_code == 200, turn_res.text + body = turn_res.json() + npc_events = [e for e in body["events"] if e["event_type"] == "npc_turn"] + assert npc_events, "No npc_turn event found in turn response" + npc_text = npc_events[0]["payload"]["content"] + # Scripted runtime produces the authored tutorial text, not the fake placeholder. + assert "meter" in npc_text.lower() or "engagement" in npc_text.lower() or "turn" in npc_text.lower(), ( + f"Expected scripted tutorial response, got: {npc_text!r}" + ) + assert "simulated npc" not in npc_text.lower(), ( + f"Got fake runtime response instead of scripted: {npc_text!r}" + ) + + +def test_tutorial_debrief_uses_scripted_debrief_when_active_config_is_scripted(tmp_config): + """Debrief for a tutorial session uses the scripted runtime's authored debrief content.""" + from convsim_core.app import create_app + from convsim_core.services.model_manager_service import set_active_config + from convsim_core.runtime.scripted import _DEBRIEF_RESPONSE + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + set_active_config(app.state.db.connection(), runtime_id="scripted") + + res = client.post("/api/sessions", json=_TUTORIAL_SESSION_SETUP) + assert res.status_code == 201, res.text + session_id = res.json()["session_id"] + + client.post(f"/api/sessions/{session_id}/start") + for _ in range(6): + client.post( + f"/api/sessions/{session_id}/turn", + json={"content": "I'm excited to learn!"}, + ) + client.post(f"/api/sessions/{session_id}/end") + + debrief_res = client.post(f"/api/sessions/{session_id}/debrief") + assert debrief_res.status_code == 200, debrief_res.text + body = debrief_res.json() + # Scripted debrief has the authored summary, not the fake boilerplate. + assert body["summary"] == _DEBRIEF_RESPONSE["summary"] From 60b8eccbcbe3d9d8944af35eef51ccdc464b8e7e Mon Sep 17 00:00:00 2001 From: Nick Charney Kaye Date: Tue, 14 Jul 2026 05:29:52 -0700 Subject: [PATCH 3/4] Pin scripted/fake sessions to their runtime instead of the live global config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _resolve_runtime() read the active runtime from the DB on every turn and debrief. That setting is global and mutable: routers/setup_install.py calls set_active_config(runtime_id="llama_cpp") the moment a background model download finishes. The "Start now" tutorial CTA is explicitly designed to be played while that download runs, so an install completing mid-tutorial would silently reroute the remaining scripted turns — and, more likely, the tutorial's authored debrief, which is generated after the conversation ends — to the freshly installed model, abandoning the tutorial script. Snapshot the active runtime id into setup_json at session creation when it is scripted or fake, and resolve from that snapshot instead. The session keeps the runtime it started with for its whole lifetime (forks inherit it, since branch_service copies setup_json verbatim). Sidecar runtimes and sessions without a snapshot still resolve to app.state.runtime, so connection-pool state and test-injected runtimes behave as before. Adds a regression test that flips the active config to llama_cpp mid-tutorial and asserts the following turns and the debrief still come from the script. --- .../convsim_core/routers/sessions.py | 45 ++++++++++++----- .../tests/test_scripted_runtime.py | 49 +++++++++++++++++++ 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/services/convsim-core/convsim_core/routers/sessions.py b/services/convsim-core/convsim_core/routers/sessions.py index 88386527..51d61699 100644 --- a/services/convsim-core/convsim_core/routers/sessions.py +++ b/services/convsim-core/convsim_core/routers/sessions.py @@ -25,6 +25,7 @@ from pydantic import BaseModel, field_validator from convsim_core.runtime import build_runtime +from convsim_core.runtime.base import ChatRuntime from convsim_core.scenario_state import build_variable_defs, partition_state_by_visibility from convsim_core.scenarios import get_scenario_info from convsim_core.services.branch_service import fork_session @@ -339,17 +340,32 @@ def _row_to_response(row: Any) -> SessionResponse: # --------------------------------------------------------------------------- -def _resolve_runtime(request: Request): # type: ignore[return] - """Return the scripted or fake runtime when the active DB config selects one. +#: Runtimes that are stateless, model-free and cheap to instantiate per request. +#: A session that starts on one of these keeps it for its whole lifetime, so a +#: scripted tutorial cannot be hijacked by a model install that finishes mid-play. +_SESSION_PINNED_RUNTIME_IDS = ("scripted", "fake") - The scripted and fake runtimes are stateless and cheap to instantiate per - request. For sidecar-based runtimes (llama.cpp, Ollama) — and when no - explicit selection exists — the shared app.state.runtime is returned to - preserve connection-pool state and support test-injected runtimes. + +def _pinned_runtime_id(conn: Any) -> str | None: + """Return the active runtime id if it is one that pins to a session.""" + runtime_id = get_active_config(conn).get("runtime_id") + return runtime_id if runtime_id in _SESSION_PINNED_RUNTIME_IDS else None + + +def _resolve_runtime(request: Request, setup: Dict[str, Any]) -> ChatRuntime: + """Return the runtime this session was pinned to, else the shared runtime. + + ``setup["runtime_id"]`` is snapshotted at session creation (see + ``create_session``) when the active selection is scripted or fake, so a + tutorial keeps answering from its authored script even after a background + model install flips the global active runtime to llama.cpp. + + For sidecar-based runtimes (llama.cpp, Ollama) — and for sessions created + before this snapshot existed — the shared ``app.state.runtime`` is returned + to preserve connection-pool state and support test-injected runtimes. """ - active_cfg = get_active_config(request.app.state.db.connection()) - runtime_id = active_cfg.get("runtime_id") - if runtime_id in ("scripted", "fake"): + runtime_id = setup.get("runtime_id") + if runtime_id in _SESSION_PINNED_RUNTIME_IDS: return build_runtime(runtime_id) return request.app.state.runtime @@ -383,6 +399,13 @@ async def create_session(body: SessionCreateRequest, request: Request) -> Sessio session_id = _generate_session_id() now = _now_iso() setup_dict = body.model_dump() + # Pin scripted/fake sessions to the runtime that was active when they were + # created. Without this the tutorial would follow the global active runtime, + # which flips to llama.cpp the moment a background model install finishes — + # mid-conversation, or before the tutorial's authored debrief is generated. + pinned_runtime_id = _pinned_runtime_id(conn) + if pinned_runtime_id is not None: + setup_dict["runtime_id"] = pinned_runtime_id conn.execute( "INSERT INTO turn_sessions " @@ -509,7 +532,7 @@ async def submit_turn(session_id: str, body: TurnSubmitRequest, request: Request scenario_data = info.get_scenario_data(difficulty) max_turns = info.max_turns - runtime = _resolve_runtime(request) + runtime = _resolve_runtime(request, setup) save_transcript = setup.get("save_transcript", True) source_mode = setup.get("input_mode", "text-only") @@ -749,7 +772,7 @@ async def create_debrief(session_id: str, request: Request) -> DebriefResponse: raise HTTPException(status_code=500, detail=f"Scenario {scenario_id!r} not found in registry") scenario_data = info.get_scenario_data(difficulty) - runtime = _resolve_runtime(request) + runtime = _resolve_runtime(request, setup) try: result = await generate_debrief( diff --git a/services/convsim-core/tests/test_scripted_runtime.py b/services/convsim-core/tests/test_scripted_runtime.py index bf9fc4ae..219c32e6 100644 --- a/services/convsim-core/tests/test_scripted_runtime.py +++ b/services/convsim-core/tests/test_scripted_runtime.py @@ -393,3 +393,52 @@ def test_tutorial_debrief_uses_scripted_debrief_when_active_config_is_scripted(t body = debrief_res.json() # Scripted debrief has the authored summary, not the fake boilerplate. assert body["summary"] == _DEBRIEF_RESPONSE["summary"] + + +def test_tutorial_stays_scripted_after_a_model_install_flips_the_active_runtime(tmp_config): + """A tutorial in progress keeps its scripted runtime when an install completes mid-play. + + The "Start now" CTA runs the tutorial while a model downloads in the + background; the setup-install pipeline calls set_active_config(llama_cpp) + when that download finishes. The session must stay pinned to the runtime it + was created with, or the rest of the tutorial — and its authored debrief — + would be routed to the freshly installed model. + """ + from convsim_core.app import create_app + from convsim_core.services.model_manager_service import set_active_config + from convsim_core.runtime.scripted import _DEBRIEF_RESPONSE + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + conn = app.state.db.connection() + set_active_config(conn, runtime_id="scripted") + + res = client.post("/api/sessions", json=_TUTORIAL_SESSION_SETUP) + assert res.status_code == 201, res.text + session_id = res.json()["session_id"] + client.post(f"/api/sessions/{session_id}/start") + client.post(f"/api/sessions/{session_id}/turn", json={"content": "Hello!"}) + + # The background install finishes: the pipeline flips the global runtime. + set_active_config(conn, runtime_id="llama_cpp", model_id="/tmp/model.gguf") + + turn_res = client.post( + f"/api/sessions/{session_id}/turn", + json={"content": "Still here — what next?"}, + ) + assert turn_res.status_code == 200, turn_res.text + npc_events = [e for e in turn_res.json()["events"] if e["event_type"] == "npc_turn"] + assert npc_events, "No npc_turn event found in turn response" + npc_text = npc_events[0]["payload"]["content"] + assert "simulated npc" not in npc_text.lower(), ( + f"Tutorial fell back to app.state.runtime after the install: {npc_text!r}" + ) + + for _ in range(5): + client.post(f"/api/sessions/{session_id}/turn", json={"content": "Go on."}) + client.post(f"/api/sessions/{session_id}/end") + + debrief_res = client.post(f"/api/sessions/{session_id}/debrief") + assert debrief_res.status_code == 200, debrief_res.text + assert debrief_res.json()["summary"] == _DEBRIEF_RESPONSE["summary"] From c57e5f0f1cf748e251a73867dfda86c4b1a29363 Mon Sep 17 00:00:00 2001 From: Nick Charney Kaye Date: Tue, 14 Jul 2026 07:40:53 -0700 Subject: [PATCH 4/4] Pin the tutorial session by request, not by the global runtime selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session pin added in 60b8eccb was read from the global active runtime at create time, so it only held if api.useModel({runtime_id:'scripted'}) had already landed. Two paths defeat that: - useModel returns an ApiResult and never throws, so its `{ok:false}` result is swallowed by the best-effort try/catch. The tutorial then runs unpinned on app.state.runtime — the fake NPC — which is the exact symptom of issue #427, with no error surfaced. - On the "Start now" path a model is downloading. If the install pipeline finishes in the window between useModel and createSession it calls set_active_config(llama_cpp), and the tutorial session is created unpinned. Let the session declare its own runtime instead: SessionCreateRequest takes an optional runtime_id, restricted by a Literal to the model-free runtimes ("scripted", "fake") so a client can never point a session at a sidecar-backed one. An explicit value wins; otherwise the active-config sniff still pins demo and scripted sessions created through the ordinary setup form. When neither applies, no runtime_id key is written to setup_json, so old and new sessions resolve identically. Also add the missing regression test for the 204 fix in ce5c496e: no test covered handleResponse's empty-body path, which is what regressed. Co-Authored-By: Claude Opus 4.8 --- apps/web/src/api/client.test.ts | 15 +++++ .../src/setup/__tests__/useSetupFlow.test.ts | 16 +++++ apps/web/src/setup/useSetupFlow.ts | 4 ++ packages/shared/src/types/session.ts | 4 ++ .../convsim_core/routers/sessions.py | 41 +++++++++---- .../tests/test_scripted_runtime.py | 61 +++++++++++++++++++ 6 files changed, 130 insertions(+), 11 deletions(-) diff --git a/apps/web/src/api/client.test.ts b/apps/web/src/api/client.test.ts index a7c3b065..383b49fe 100644 --- a/apps/web/src/api/client.test.ts +++ b/apps/web/src/api/client.test.ts @@ -173,6 +173,21 @@ describe('api — network errors', () => { }); }); +// --------------------------------------------------------------------------- +// Empty response bodies +// --------------------------------------------------------------------------- + +describe('api — 204 No Content', () => { + it('returns ok:true for an endpoint that answers 204 with an empty body', async () => { + // POST /api/setup/outcome is declared status_code=204. Parsing its empty body + // as JSON throws, which used to surface a bogus runtime-unreachable error. + mockFetch(204, ''); + const result = await api.recordOnboardingOutcome('demo'); + expect(result.ok).toBe(true); + if (result.ok) expect(result.data).toBeUndefined(); + }); +}); + // --------------------------------------------------------------------------- // api.getScenario // --------------------------------------------------------------------------- diff --git a/apps/web/src/setup/__tests__/useSetupFlow.test.ts b/apps/web/src/setup/__tests__/useSetupFlow.test.ts index cdb435e5..be12dec2 100644 --- a/apps/web/src/setup/__tests__/useSetupFlow.test.ts +++ b/apps/web/src/setup/__tests__/useSetupFlow.test.ts @@ -477,9 +477,25 @@ describe('useSetupFlow step machine', () => { show_state_meters: true, save_transcript: true, seed: null, + runtime_id: 'scripted', }) }) + it('handleStartTutorial pins the tutorial session to the scripted runtime even if useModel fails', async () => { + // useModel only moves the global selection, and it can lose a race with the + // background install that flips it back to llama.cpp. The session must carry + // its own runtime so the tutorial never lands on the fake NPC. + mockApi.useModel.mockResolvedValue({ ok: false, error: { kind: 'network', message: 'runtime unavailable' } }) + const { result } = renderHook(() => useSetupFlow('welcome'), { wrapper }) + + await act(async () => { await result.current.handleStartTutorial() }) + + expect(mockApi.createSession).toHaveBeenCalledWith( + expect.objectContaining({ runtime_id: 'scripted' }), + ) + expect(mockNavigate).toHaveBeenCalledWith('/conversation/sess-tutorial-1', expect.anything()) + }) + it('handleStartTutorial navigates to /conversation/:sessionId on successful session creation', async () => { mockApi.useModel.mockResolvedValue(SCRIPTED_RUNTIME_RESPONSE) const { result } = renderHook(() => useSetupFlow('welcome'), { wrapper }) diff --git a/apps/web/src/setup/useSetupFlow.ts b/apps/web/src/setup/useSetupFlow.ts index 4b726cb8..7ced04af 100644 --- a/apps/web/src/setup/useSetupFlow.ts +++ b/apps/web/src/setup/useSetupFlow.ts @@ -382,6 +382,9 @@ export function useSetupFlow( // Create the tutorial session directly so the user goes straight to the // conversation without having to click through the scenario setup form. // The scripted runtime needs no model, so this always succeeds on first open. + // runtime_id pins the session to the authored script: the useModel call above + // only moves the *global* selection, which the background install flips back + // to llama.cpp the moment it finishes. const sessionResult = await api.createSession({ scenario_id: 'first_words_tutorial', difficulty: 'standard', @@ -392,6 +395,7 @@ export function useSetupFlow( show_state_meters: true, save_transcript: true, seed: null, + runtime_id: 'scripted', }) if (sessionResult.ok) { navigate(`/conversation/${sessionResult.data.session_id}`, { diff --git a/packages/shared/src/types/session.ts b/packages/shared/src/types/session.ts index 39d0dc07..4027f43b 100644 --- a/packages/shared/src/types/session.ts +++ b/packages/shared/src/types/session.ts @@ -37,6 +37,10 @@ export interface SessionCreateRequest { show_state_meters: boolean; save_transcript: boolean; seed: number | null; + // Pins the session to a model-free runtime for its whole lifetime, instead of + // following the globally selected runtime. Only the scripted tutorial and demo + // mode need this; omit it and the backend uses the active selection. + runtime_id?: 'scripted' | 'fake'; } export interface SessionCreateResponse { diff --git a/services/convsim-core/convsim_core/routers/sessions.py b/services/convsim-core/convsim_core/routers/sessions.py index 51d61699..cf5c5093 100644 --- a/services/convsim-core/convsim_core/routers/sessions.py +++ b/services/convsim-core/convsim_core/routers/sessions.py @@ -82,6 +82,10 @@ class SessionCreateRequest(BaseModel): show_state_meters: bool = False save_transcript: bool = True seed: Optional[int] = None + # Pins the session to a model-free runtime for its whole lifetime (issue #427). + # Restricted to the runtimes that need no model and reach nothing off-box, so a + # client can never point a session at a sidecar-backed runtime this way. + runtime_id: Optional[Literal["scripted", "fake"]] = None @field_validator("player_role_name") @classmethod @@ -346,8 +350,18 @@ def _row_to_response(row: Any) -> SessionResponse: _SESSION_PINNED_RUNTIME_IDS = ("scripted", "fake") -def _pinned_runtime_id(conn: Any) -> str | None: - """Return the active runtime id if it is one that pins to a session.""" +def _pinned_runtime_id(requested: str | None, conn: Any) -> str | None: + """Return the runtime id to pin this session to, or None to follow the global one. + + An explicit ``runtime_id`` on the create request wins: the scripted tutorial + asks for its runtime by name, so it cannot land on the fake runtime just + because the preceding ``use_model`` call failed, or because a background + model install flipped the global selection in the window between the two + requests. Otherwise fall back to the active selection, which pins demo-mode + and scripted sessions created through the ordinary setup form. + """ + if requested in _SESSION_PINNED_RUNTIME_IDS: + return requested runtime_id = get_active_config(conn).get("runtime_id") return runtime_id if runtime_id in _SESSION_PINNED_RUNTIME_IDS else None @@ -356,9 +370,10 @@ def _resolve_runtime(request: Request, setup: Dict[str, Any]) -> ChatRuntime: """Return the runtime this session was pinned to, else the shared runtime. ``setup["runtime_id"]`` is snapshotted at session creation (see - ``create_session``) when the active selection is scripted or fake, so a - tutorial keeps answering from its authored script even after a background - model install flips the global active runtime to llama.cpp. + ``create_session``) when the session asked for a scripted/fake runtime or the + active selection was one of those, so a tutorial keeps answering from its + authored script even after a background model install flips the global active + runtime to llama.cpp. For sidecar-based runtimes (llama.cpp, Ollama) — and for sessions created before this snapshot existed — the shared ``app.state.runtime`` is returned @@ -399,12 +414,16 @@ async def create_session(body: SessionCreateRequest, request: Request) -> Sessio session_id = _generate_session_id() now = _now_iso() setup_dict = body.model_dump() - # Pin scripted/fake sessions to the runtime that was active when they were - # created. Without this the tutorial would follow the global active runtime, - # which flips to llama.cpp the moment a background model install finishes — - # mid-conversation, or before the tutorial's authored debrief is generated. - pinned_runtime_id = _pinned_runtime_id(conn) - if pinned_runtime_id is not None: + # Pin scripted/fake sessions to their runtime for the whole session. Without + # this the tutorial would follow the global active runtime, which flips to + # llama.cpp the moment a background model install finishes — mid-conversation, + # or before the tutorial's authored debrief is generated. + pinned_runtime_id = _pinned_runtime_id(body.runtime_id, conn) + if pinned_runtime_id is None: + # Leave no key at all rather than a null one, so _resolve_runtime's + # membership test reads the same for old and new sessions. + setup_dict.pop("runtime_id", None) + else: setup_dict["runtime_id"] = pinned_runtime_id conn.execute( diff --git a/services/convsim-core/tests/test_scripted_runtime.py b/services/convsim-core/tests/test_scripted_runtime.py index 219c32e6..0d165840 100644 --- a/services/convsim-core/tests/test_scripted_runtime.py +++ b/services/convsim-core/tests/test_scripted_runtime.py @@ -442,3 +442,64 @@ def test_tutorial_stays_scripted_after_a_model_install_flips_the_active_runtime( debrief_res = client.post(f"/api/sessions/{session_id}/debrief") assert debrief_res.status_code == 200, debrief_res.text assert debrief_res.json()["summary"] == _DEBRIEF_RESPONSE["summary"] + + +def test_session_runtime_id_pins_the_script_when_the_global_selection_is_not_scripted(tmp_config): + """An explicit runtime_id on the create request pins the session on its own. + + The web client sets the global selection with use_model before creating the + tutorial session, but that call can fail, and the background install can flip + the selection to llama_cpp in the window between the two requests. Neither may + drop the tutorial onto the fake NPC. + """ + from convsim_core.app import create_app + from convsim_core.services.model_manager_service import set_active_config + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + # Global selection is a real model — as if use_model('scripted') never landed. + set_active_config(app.state.db.connection(), runtime_id="llama_cpp", model_id="/tmp/model.gguf") + + res = client.post( + "/api/sessions", + json={**_TUTORIAL_SESSION_SETUP, "runtime_id": "scripted"}, + ) + assert res.status_code == 201, res.text + session_id = res.json()["session_id"] + + client.post(f"/api/sessions/{session_id}/start") + turn_res = client.post(f"/api/sessions/{session_id}/turn", json={"content": "Hello!"}) + assert turn_res.status_code == 200, turn_res.text + npc_events = [e for e in turn_res.json()["events"] if e["event_type"] == "npc_turn"] + assert npc_events, "No npc_turn event found in turn response" + npc_text = npc_events[0]["payload"]["content"] + assert "simulated npc" not in npc_text.lower(), ( + f"Explicit runtime_id was ignored; got the fake runtime: {npc_text!r}" + ) + + +def test_session_runtime_id_rejects_a_sidecar_backed_runtime(tmp_config): + """Only the model-free runtimes may be requested per-session.""" + from convsim_core.app import create_app + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + res = client.post( + "/api/sessions", + json={**_TUTORIAL_SESSION_SETUP, "runtime_id": "llama_cpp"}, + ) + assert res.status_code == 422, res.text + + +def test_session_without_runtime_id_stores_no_runtime_key(tmp_config): + """A normal session records no pin, so it keeps following the global selection.""" + from convsim_core.app import create_app + from fastapi.testclient import TestClient + + app = create_app(tmp_config) + with TestClient(app) as client: + res = client.post("/api/sessions", json=_TUTORIAL_SESSION_SETUP) + assert res.status_code == 201, res.text + assert "runtime_id" not in res.json()["setup"]