Repository navigation
Fix the first-session experience: resume, plain language, meter clarity, and turn counting - #550
Draft
charneykaye wants to merge 39 commits into
Draft
charneykaye wants to merge 39 commits into
charneykaye wants to merge 39 commits into
Conversation
Two gaps behind the first-session feedback in issue #501, both in the core. GET /api/sessions did not exist. The web client already called it (api.listSessions) and the shared SessionCreateResponse type already documented it — "Included in list responses (GET /api/sessions)" — but the route was never registered, so the request answered 405: Settings' "Your sessions" list only ever rendered an error, and nothing in the app could discover a conversation to resume. The route now lists the profile's sessions newest first, with a `status` filter whose `in_progress` value is exactly the resumable set (started, not ended) and a bounded `limit`. Each row carries the setup it was created with, which is what lets a resumed conversation rebuild its language, meter and voice settings without the route state it was launched from. turn_count rides along; it already counted whole player+NPC exchanges rather than transcript rows. "Make the model respond faster" was not an exposed concept. The settings that exist are context length, GPU layers, CPU threads and sampling — all named after the runtime, none of them the dominant cost of a turn. The dominant cost is how many tokens the NPC generates, one at a time, so reply_speed turns that into one named choice: fast / balanced / detailed. It scales the scenario's authored NPC word cap rather than replacing it (a pack that writes terse NPCs stays terser than one that writes expansive ones) and moves the generation budget with it. 'balanced' reproduces the previously hardcoded 1024-token, unscaled behaviour exactly, so the default player experience is unchanged. It is read per turn, so a change applies to the player's next message with no restart, and the scenario data is copied rather than mutated so one player's pacing cannot leak into another session playing the same scenario. Tests cover the list endpoint (ordering, the three status filters, limit bounds, whole-turn counts), the profile table, endpoint validation, a garbage value stored by a newer build, and end-to-end assertions that the scaled word cap reaches the RESPONSE_STYLE prompt layer and the token budget reaches the runtime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three UI halves of issue #501: a way back into a conversation, a buffer between player-facing and technical language, and meters that explain themselves. All three are the same report — the app "feels both too sparse and too technical in the first few sessions". Resume. A resume strip lives in the app chrome, so it is on every screen the detour that lost the playtester could reach — including Settings, the screen they were standing on when they gave up and started a new scenario. Home's primary actions lead with it when there is one, and Settings → Your sessions grows a Resume button per unfinished session, so the banner's "the newest one" is not the only answer. The Conversation screen now reads its setup back from convsim-core when it arrives with no router state, which is exactly the resume case: navigating away and back, a reload, and a relaunch all used to fall through to defaults and drop the scene card, the chosen language and the voice settings the player picked. TTS enablement moved behind a ref because the WebSocket handler is installed once per session and would otherwise capture the pre-fetch value. Reply speed. Settings grows a named "Reply speed" section — Quick replies / Balanced / Fuller replies, each with a two- or three-word descriptor, the pattern the note asked for. It sits above the engine knobs rather than behind the advanced toggle, is anchored so the conversation screen's slow-reply notice can link straight to it ("Make replies faster →" instead of the old "you can adjust settings"), and says plainly that a smaller model is a bigger lever. Wording. A two-level buffer, `plain` by default, set by an LLM-familiarity question on the first-run Welcome step and revisitable from a card on the tutorial debrief and from Settings → Wording. Two levels, not a scale: a third intermediate level would need a different word per string with no rule saying which words belong to it. At `plain` the conversation screen drops the session id, the raw flow-state identifier, the per-turn `Flags:` line and the running `Event flags:` strip, says "Something changed: Rapport milestone" instead of "Scenario event: rapport_milestone", and names variables as words. At `technical` every identifier comes back, because that is what a bug report needs. The NPC also has a name now: `npc_name` resolves through the scenario's `npc.ref` and travels with the scenario, so the transcript says "Dana Reyes" rather than "NPC" — the most technical word on that screen, and only there because the name was never exposed. Meters. Moved above the transcript, where the tutorial's opening line has always claimed they are ("Notice the two meters at the top") and where they were not; out of a collapsed "NPC state variables" disclosure and into a labelled "Conversation meters" panel that says in one line that they are not moods. Each meter now draws the tick the tutorial talks about — "▲ +10 this turn" — derived by diffing successive snapshots rather than from the model's requested delta, so a delta the reducer clamped or rejected cannot claim credit for a move that did not happen. The NPC's emotion is gone from the parenthetical beside the turn number and from its adjacency to the bars; it reads "Mood: warm" under the NPC panel and under each reply, which is the whole of the "unclear which meter (if any) the emotions in parentheses are" complaint. Turn counting. One turn is the player's message plus the reply to it, so both rows carry the same number and the NPC's opening is labelled "Opening" rather than consuming turn 1. This is the number convsim-core already counts in turn_count, that duration.max_turns is measured in, and that debrief key moments point at — so the debrief's "#3" and the transcript's "Turn 3" now mean the same moment, and its "go to turn N" button lands on the start of turn N instead of on whichever half-turn was Nth in the list. No pack changes: max_turns was already whole turns. Tests: existing expectations updated to the new behaviour (both wording levels asserted side by side), plus new coverage for whole-turn labelling of an exchange, rollback not consuming a turn number, meter movement, humanized vs raw variable keys, meter placement above the transcript, and the npc_name resolution for every seeded scenario. Full web suite and the convsim-core scenario suites pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Issue #501 §3: "The model in the Tutorial doesn't seem to be able to give an explanation of what the meters are and hallucinates through the scenario, as if it has given a coherent explanation and received positive feedback from the user." It is not a model. The tutorial runs on the scripted runtime, and the script had no branch for a question — each turn was a fixed line that presumed an engaged, positive reply ("Great! See how Engagement just ticked up?", "Did you see that?"). A player who asked "what are the meters?" got the next canned line regardless, which reads exactly like a model confabulating an exchange that never happened. The script now checks the player's message first. A message that reads as a question AND names a topic the tutorial can speak to — the meters, the mood chip, whether this is really AI, how a conversation ends, what the debrief is — gets an answer, then hands the player back to the tour. Question words and topic words are matched on word boundaries, never as bare substrings: "explain" contains "ai" and "show" contains "how", and matching those inside other words is how a keyword table starts answering questions nobody asked. The ending branch still outranks an answer on the final turn, so a question there closes the session rather than leaving it open. An answered question costs one of the tutorial's turns, so the scenario's budget goes from 8 whole turns to 12 — six are the tour, the rest are headroom. The tour's own lines are rewritten twice over. They no longer claim the player said anything in particular, and they no longer teach the mechanics in the engine's vocabulary: "scenario event", "hidden prompt" and "rubric dimensions" were three of the exact words §2 of the issue named as unexplained jargon, used on a player who by definition has not met them yet. A meter crossing a mark and changing how Alex behaves is the same mechanic described in words a first-time player already owns. The pack's summary, player-visible goals, opening line and the warm_moment instruction follow, and the opening's "the two meters at the top" is now true — they moved above the transcript. Tests: a meter question is answered on every tour turn and by five phrasings; a statement that merely mentions a meter is not; an unrelated question does not derail the tour; "can you explain that again?" does not trip the AI cluster; an answer keeps the session open and a question on the last turn still closes it; the answers pass the same output validators as the scripted turns. Two guard tests sweep every tutorial utterance for the named jargon and for the presumptive phrasing the screenshot captured, so neither can come back. The tutorial's canonical-fields contract pins the new turn budget and asserts the guide is named rather than "NPC". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two pure modules behind issue #501 §2 and §4, which the screen tests can only exercise indirectly. wholeTurnNumbersFor is asserted against both shapes the UI has to label — the session export's `event_type` rows and the transcript endpoint's `role` rows — and against the cases that distinguish it from numbering by position: a turn still awaiting its reply, and two NPC rows in a row, which must stay two moments so a debrief key moment lands on the right one. The plain-language helpers pin the defaults that matter: an unasked player and an unrecognised stored value both read as plain wording, and an unmapped flow state never reaches the player as a raw identifier — the failure mode this issue is about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both blocks were landing inside `demo:` rather than beside it, so every key they define resolved to its own path — the resume banner rendered the literal text "resume.label resume.withTurns". The demo edition's own `home` and `setup` sub-blocks have the same names one level down, which is how they ended up there. Tests for ResumeSessionBanner, which is what caught it: the strings actually resolve, it asks only for the resumable set and only for the newest one, it is absent on the conversation and debrief routes rather than offering to resume the thing the player is looking at, it re-asks on focus so it cannot offer a session that has since ended, and it stays silent — rather than stacking another error card — when the core is unreachable or answers a shape this build does not expect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reply speed: the three named options with their descriptors, balanced standing in for a choice never made, an optimistic selection that rolls back when the save fails, no redundant write when the active option is re-clicked, the honest note that a smaller model is the bigger lever, and the 404 path where it steps aside rather than stacking a second copy of the notice RuntimeSettingsPanel already shows. Wording: the two levels, and the familiarity question that sets them. The cases that matter are the defaults — nobody answering leaves plain wording in place — and the re-ask appearing exactly once on the tutorial debrief and nowhere else, since the debrief's buttons navigate away and re-nagging after every later session is the failure mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tutorial is defined twice: as a pack, and as a hardcoded catalog entry in scenarios.py. resolve_scenario_info consults the catalog first — only the demo edition passes prefer_installed_pack=True — so in the full app, which is the playtester's "default settings", the catalog entry is what plays and the pack supplies only the library card, rubric and safety policy. The previous commit rewrote the pack, so none of it reached a full-app player. The one with teeth was the turn budget: the card advertised 12 whole turns while the session enforced 8, so a player who asked two questions would hit the timeout before the endings-and-debrief beat — losing exactly the headroom that commit added for exactly that. The opening line, the player-visible goals and the warm_moment instruction were also still the pre-#501 copy, so a full-app player read "Notice the two meters at the top" (with the meters below the transcript), "Trigger a scenario event to see how events reshape a conversation", and an instruction telling Alex to explain "what a scenario event looks like — a threshold crossing that shifts your hidden instructions". The NPC's own speaking style ("names the mechanic being demonstrated") and hidden agenda ("demonstrate every core mechanic — meters, events, endings, debrief") said the same thing at the model. Two guards so the copies cannot drift again, both verified to fail when one string is reverted: - test_tutorial_copy_avoids_simulator_jargon now sweeps the catalog entry's opening, goals, role brief, event instructions, speaking style and hidden agenda alongside the pack YAML and the scripted runtime's own lines. It passed before because it only ever looked at two of the three sources. - test_tutorial_catalog_entry_matches_the_pack asserts the two definitions agree on the turn budget, the timeout, the opening line, the goals and every event instruction — the fields a player can tell apart. And test_scenario_contract pins the budget as resolved for play, not just as advertised on the card, which is the comparison that would have caught this. Found by a second agent reading the resolution path; verified here end to end before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reference half of issue #501. Three of the issue's open questions were answerable only from the code, so the answers now live in the docs instead. Turn counting. "One turn is one player message plus the NPC's reply to it" is now stated in all three places a reader could ask: the gameplay loop in SPEC.md (where it maps onto steps 6-13), a "What counts as a turn" section in architecture.md next to the ending types, and a "Counting turns" section in scenario-authoring.md next to the trigger table an author actually works from. The issue asked whether the change touches duration.max_turns semantics: it does not, and the docs now say why rather than leaving the next author to re-derive it. turn_pipeline.py numbers a turn turn_count + 1 once per process_turn call, and scenario_state.py compares that same number to max_turns, so max_turns was always whole turns and no pack needs migrating. Two things authors can get wrong are called out: the NPC's opening is free (stored at row 0, labelled rather than numbered), and the limit is evaluated after a turn is processed, so the last turn plays out in full before timeout. The transcript's finer grain is recorded too — opening at row 0, whole turn n at player row 2n-1 and NPC row 2n — because that is the mapping anything reading a raw export has to reproduce. Session listing. architecture.md's session endpoint table listed eight routes and omitted GET /api/sessions, which is the one the resume entry points depend on; SPEC.md's route list omitted it too. Both now carry it with its status and limit parameters, and architecture.md says what each row contains and why — including that setup is what lets a resumed conversation rebuild the language, meter and voice choices it was launched with, and that in_progress excludes NotStarted because nothing has been said yet. Reply speed. troubleshooting.md's "inference is very slow" remedies were GPU layers, VRAM and context length — all of which require knowing what they are, and none of which is what the playtester went to Settings looking for. Reply speed now leads that list, with the reason it is the cheapest lever (a local model emits one token at a time, so the wait is mostly the length of the reply), and the starter-model entry says plainly that a smaller model is the bigger win so the ordering is not mistaken for a ranking of effect size. Quickstart. The conversation-screen description claimed a turn counter and a progress bar, neither of which exists; it now describes the meters above the transcript, the mood label under each reply, and the Turn N / Opening labels. New sections cover leaving and coming back (all three resume entry points) and what to do when replies feel slow. The first-run familiarity question is described where the player meets it, including that skipping is free and plain wording is the default. Also corrected: scenario-authoring.md described a visible variable as "shown as a state meter during the session", which is now neither the label nor the placement. Every UI string quoted here was checked against apps/web/src/i18n/locales/en.ts rather than paraphrased. Docs-only change; no CI docs linter exists to run, and the convsim-core suite (2196 passed, 3 skipped) and apps/web suite (1387 passed) were verified green on this branch beforehand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Settings: the two new sections (reply speed, anchored so the slow-reply notice can deep-link to it; and wording), the engine section renamed from "Runtime", and the per-session Resume button — present for an unfinished session, absent for a finished one and for one that never started, since neither has a conversation to go back to. Sessions are labelled by scenario title and plain state rather than slug and flow-state identifier. Reply speed follows the engine section out of the demo edition; wording stays, being readability rather than full-app depth. Home: Resume leads the primary actions when a conversation is in progress, and is absent otherwise. Its position ahead of "Start a scenario" is asserted, because starting over is what the playtester actually did. Debrief: the opening is labelled and each exchange numbered once, the NPC mood is labelled rather than bracketed beside the turn number, and "go to turn 2" scrolls to the player's message that opens turn 2 — not to the second row in the list, which is the player's message of turn 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The manual QA script still described the pre-#501 screen, so a tester following it would have confirmed behaviour the branch deliberately changed. P-M6 was headed "State meters" and asked the tester to enable "Show state meters" and check that "State variables" were visible — three names for one thing, none of them what the UI says now, and the step would pass against either the old collapsed "NPC state variables" disclosure or the new panel. It is now headed "Conversation meters" and checks what §3 of the issue actually asked for: the panel above the transcript, variables labelled in words rather than raw keys, the per-turn movement drawn on the meter that moved, and the NPC's mood under its own label instead of as a bare parenthetical beside the bars. The toggle is referred to by what it does rather than by its exact label, because that string is still settling; the step reads correctly either way. P-M5 gains a turn-labelling check, since whole-turn numbering is a visible change a tester can otherwise walk straight past: the opening labelled "Opening", and both rows of an exchange carrying one number. P-M6b is new and covers the report that opened this issue — the player who went to Settings mid-scenario and could not find the way back. It walks the same path deliberately: leave an unfinished conversation for Settings, find reply speed without expanding anything, and come back by each of the three routes that now exist. The last item checks the scene card and language survive a reload, which is the part that depends on the server rather than the browser's history state and so is the part that silently regressed before. Checklist-only change; both new rows added to the results table. No CI docs linter exists to run. The cross-reference to scenario-authoring.md#counting-turns resolves against the section added in 421154e. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Conversation Brief's checkbox still read "Show NPC state meters during conversation", one screen before a panel headed "Conversation meters". Issue #501 §3 is a naming mismatch — "the label, the naming and the placement don't agree" — and leaving the control on the old words hands the player the same mismatch in the other direction, from the label they read first. It now reads "Show the conversation meters", with a note saying what they measure, so the brief answers "what am I turning on?" before the conversation rather than after. The not-permitted note loses "state meters" too. The pack field behind it, `state_meters_permitted`, keeps its name: it is an API and pack-schema field and is never shown to a player. Found by a second agent reading the setup page after the panel rewrite landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The toggle this step drives was renamed in 79a04b1 — "Show NPC state meters during conversation" became "Show the conversation meters", matching the panel it switches on. b5ace5d had deliberately described the toggle by function rather than by label, because the string was still moving; now that it has settled, the step quotes it, which is what a tester following the script needs. Also records the not-permitted case, so a tester who picks a scenario that forbids meters recognises the note instead of reporting a missing toggle, and states the mood expectation as the negative it actually is: the §3 complaint was "unclear which meter (if any) the emotions in parentheses are", so the check is that mood reads as a labelled mood and never as one of the bars. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PEP 8 wants two blank lines after a top-level def; the helper added for reply speed left one before MAX_TURN_CONTENT_CHARS, which read as if the constant belonged to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rst-session-ux-resume-plain-langua
charneykaye
added
the
review
…reen The merge at 9fea5ef — which brought in 623c9c1's NPC turn-progress panel — resolved Conversation.tsx by keeping this branch's function bodies and upstream's declarations, and left the tree not compiling. It was pushed in that state, so this fixes forward rather than rewriting it. - npcStatusLabel came back with upstream's two-argument signature while its body still read `isPlain` (TS2304), and so did its call site. - `const npcLabel = scenario?.npc_name ?? 'NPC'` was dropped while three call sites still used it (TS2304 x2 plus the one inside the slow-response copy). - The pre-#501 meters panel came back *below* the transcript, alongside the StateMeters panel above it. Both carried data-testid="state-vars", so five meter tests failed on "Found multiple elements" — the one failure that was a real duplicate rather than a missing symbol, and the one that would have shipped two meter panels to a player. - The running "Event flags: …" strip lost its plain-wording gate, putting raw flag identifiers back on screen by default. - The slow-response notice lost the "Make replies faster →" deep link when upstream's rewrite of that block was taken wholesale; restored into the new block, which is the better home for it anyway — it now sits under a panel that says how long the turn is actually taking. No conflict markers were left behind, which is why it read as a clean merge. Everything else this branch changed survived and was audited file by file: whole-turn labelling, the Opening label, Mood labels, the humanized event banner, the plain subhead, the setup rehydration, the TTS ref, and all of Settings, Home, Debrief, WelcomeStep and ScenarioSetup. Verified at this commit: apps/web `tsc --noEmit` clean, full vitest 1523 passed across 56 files (up from 1456/53 — the merge added six test files), check-i18n clean. convsim-core is untouched by the merge apart from one line in ollama_adapter.py; its suite was 2224 passed / 3 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
d846424 added reply_speed to the shared RuntimeSettings type and implemented it in convsim-core, but apps/api carries its own copy of /api/runtime/settings and was left behind, so `pnpm test:types` failed: NULL_SETTINGS and loadRuntimeSettings() both returned an object missing a now-required property. Filling in the field alone would have typechecked while quietly dropping the value, so the proxy now mirrors convsim-core's semantics instead: reply_speed is persisted and reset alongside the numeric knobs, read back as a string rather than through parseFloat, validated against the three options the UI can render, declared in the PUT body schema, and kept out of RESTART_REQUIRED because it is read per turn. The three options now live in one place, as a REPLY_SPEEDS const beside the type, so the proxy's validation, the Settings panel's radio group and the type cannot drift apart. ReplySpeedPanel drops its local copy. Tests mirror test_reply_speed.py's endpoint cases: persistence, no restart, 422 on an unknown speed, survival of a partial update, reset, and a stored value the build does not recognise reading back as unset. CI does not run apps/api's suite, but it is green locally alongside the whole "Frontend typecheck and tests" job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Addressed failing CI checks and pushed a fix (Frontend typecheck and tests). |
1 similar comment
Contributor
Author
|
Addressed failing CI checks and pushed a fix (Frontend typecheck and tests). |
Contributor
Author
|
Addressed failing CI checks and pushed a fix (Backend tests, Release smoke / Windows x86_64, Release smoke / Linux x86_64). |
Contributor
Author
|
Addressed failing CI checks and pushed a fix (unknown checks). |
…ssions Three gaps in GET /api/sessions, all on the resume path issue #501 added it for: - `status=ended` matched only `flow_state = 'Ended'`. Ending a session is just the first state a finished one passes through: generating the debrief moves the row to DebriefGenerating and then DebriefReady, and a failed debrief leaves Error. So a session the player actually played to the end was in neither filter — `in_progress` rightly excluded it and `ended` missed it. ENDED_FLOW_STATES covers all four, which together with RESUMABLE_FLOW_STATES and NotStarted makes the three filters partition the table. - Creator Workbench previews are written straight into `turn_sessions` under a dynamic `__wbtest__<hex>` scenario id, and nothing ever ends or deletes them. So one preview left the "Conversation in progress" strip up forever, offering to resume a scenario that only ever existed in memory. apps/api's sibling listing has always excluded them; now this one does too, from every status. - A resuming conversation screen had no way to read its meters back. Meter values only ever arrived with a turn, so a player who stepped out to Settings came back to a conversation with no meters at all — the one thing the tutorial tells them to watch — until they sent another message. The single-session endpoint now reports `visible_state`, filtered through the scenario's own variable visibility so a hidden variable stays hidden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The transcript now labels whole turns — one player message plus the reply to it (issue #501 §4) — but the debrief's turning points still carried the NPC row's stored `turn_number`, which is `2n` for turn n. So a key moment in turn 2 was reported as "#4": the badge disagreed with the "Turn 2" label on the message it described, and clicking it scrolled to the start of turn 4, a whole turn past the moment it named. Before the relabelling it was off by one row, so this got worse rather than better — exactly the alignment the PR claimed to have fixed. Fixed at the source rather than in the screen, because the number appears in prose too: `_identify_key_turns` writes "Significant state shift at turn N" into the description, and the model is handed the transcript as "[Turn N]" and asked to cite turn numbers back. Converting only the badge would have left the sentence next to it quoting a row number. - `whole_turn_of` maps a row number to its turn; the opening is 0. - `_identify_key_turns` numbers and describes in whole turns. - `_build_debrief_turn_records` labels both halves of an exchange the same, so a turning point the model cites is a number the player can find. - The validity filter is now the set of whole turns actually played, with the opening excluded: a turning point has to point at an exchange. `compute_metrics` is untouched — its `state_arc` and latency pairing are row-based by necessity and never shown as turn numbers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The REST summary described `status=ended` as the complement of `in_progress` without saying that a finished session is almost never still in the `Ended` state, left the Workbench exclusion undocumented, and did not mention that the single-session route reports `visible_state` — which is the only reason a resumed conversation can draw its meters before the next turn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three defects in the issue #501 work, all in the parts meant to fix the report's own complaints. The scripted tutorial was matching the composer's scaffolding, not the player. The turn pipeline hands the runtime the composed PLAYER_UTTERANCE layer, not bare words, and that layer ends "=== END PLAYER INPUT ===". "end" is a keyword of the endings cluster, so every message that read as a question at all got the endings answer — "How am I scored?" included, which made the debrief answer (a later cluster) unreachable — and even a plain "Tell me about yourself" derailed the tour, because "tell me" is a question marker. _extract_player_text now strips layer tags and sentinel lines. An answer replaced the turn's tour line instead of riding along with it. That dropped the beat and, worse, dropped the beat's state_delta: Engagement starts at 30 and only the script's deltas carry it over the event threshold of 60, so a player who asked two or three questions reached turn 4 and was told "That was it. Engagement crossed the mark and my instructions changed" about an event that never fired. That is the false-confidence failure the issue attached a screenshot of, reintroduced by the fix for it. The answer is now prepended to that turn's line, so every beat and every delta lands where the script put it and asking costs nothing. Turn 3's opener lost its "Good to know." for the same reason any tour line may now follow a question rather than an answer. Because the script closes the session on its sixth turn either way, the widened turn budget was unreachable and its stated rationale ("headroom for the questions, each of which costs a turn") was no longer true, so both copies of the tutorial go back to max_turns 8 — the pre-existing safety net behind a six-turn script. The cross-copy guard keeps them in step. The first-run familiarity question looked like it ignored the answer. It read its selection straight from localStorage on every render, and on the Welcome step nothing else re-renders when an answer is written (there is no onAnswered there), so clicking "New to this" left every option aria-checked=false. Held in state now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MIN/MAX_SCALED_MAX_WORDS were applied as plain bounds on the result, but
`scenario.schema.json` allows `response_style.max_words` anywhere in 10-500, so
any cap outside 30-200 was rewritten at every speed:
authored fast balanced detailed
10 30 30 30
300 150 200 200
500 200 200 200
Two things wrong with that. "Quick replies" *tripled* the cap of a pack that
writes 10-word lines — the one direction this control must never move — and on
such a pack all three options were identical, so the setting did nothing
visible. And `balanced` silently cut a 300-word cap to 200, when the whole
point of `balanced` is that it reproduces the pre-#501 behaviour exactly.
The clamps now bound how far scaling may travel from the authored value instead
of replacing it: `fast` can only shorten, `detailed` can only lengthen, and
`balanced` returns the authored cap untouched. The floor and ceiling still bite
in the range packs actually use — halving 50 still stops at 30, and 1.5x of 180
still stops at 200.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Issue #501 §3 is about the tutorial claiming things that did not happen, and the rewritten turn-4 line makes a specific claim about the UI: "you would have seen a short note above the transcript saying something changed." Played end to end, Engagement crosses 60 on turn 3 and `warm_moment` fires — and no note appears. The engine's own events arrive on the npc_turn payload as `triggered_scenario_events`, and nothing in the web app read that field. The "Something changed" banner was fed solely by the WebSocket `scenario.event` frame, which carries the NPC's *self-declared* `event_flags` (empty for every line of the scripted tutorial) and which convsim-core has no route to send at all. So the surface this PR renamed and re-worded never fired for the thing it is named after. The REST path now announces them, outside the WebSocket commit guard because the reply may already be on screen while the events it fired travel only on this payload, and deduplicated by event id so a future socket frame carrying the same id cannot banner it twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "Conversation in progress" strip is on every screen, and after the player's first exchange it read "The Behavioral Interview — 1 turns so far." Split into the `_one`/`_other` keys this catalog already uses elsewhere (`scenarioCount_one`, `sessionCount_one`): `t` does no plural selection of its own, so the caller picks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
The resume path hydrated the transcript but not the meters, so a player who stepped out to Settings came back to a conversation with no meters at all — on the screen whose opening line tells them to watch the two meters above it — until they sent another message. Verified to fail when the `getSession` read in the already-started branch is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
The scripted tutorial's spoken lines were rewritten for issue #501 §2 to drop "scenario event", "hidden prompt" and "rubric dimensions", and a guard sweep was added to keep them out. The sweep collected the script turns, the interjections, the ending branches and both scenario definitions — but not _DEBRIEF_RESPONSE, the debrief the tutorial hands the player at the end of their first session. So the whole vocabulary survived in the one place the sweep could not see it: "You learned how state meters track conversation dynamics, how scenario events fire at threshold crossings, and how the debrief rubric scores your performance on each dimension." "You saw a live scenario event fire — a key mechanic in every pack." "The warm_moment event fired — this is where Engagement crossed 60 and the NPC instructions shifted." Three of the four words the playtest named, plus a raw pack identifier, on the last screen of the session this issue is about — after six turns of deliberately avoiding them. Rewritten in the same everyday language as the script, with the turning point described as "Engagement passed the mark that changes a conversation" rather than by its event id. The curious ending branch's "every scenario has a rubric that spells out exactly what's being measured" goes the same way: "rubric" is an authoring word, so it is now forbidden outright rather than only in the phrase "rubric dimension". The sweep now collects the debrief's prose fields as well, and a second test fails if a raw event id reaches the player there. Both were verified to catch the strings they replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moving the NPC's emotion out from beside the turn number and under its own "Mood:" label (issue #501 §3) added the label as a bare English literal. Debrief.tsx is on check-i18n's migrated list — every other string on that screen goes through t() — but the scanner's JSX-text pattern needs two English words before a `<`, and "Mood: <span>" gives it one, so it passed. Under the German locale the debrief transcript read "Mood: warm" with the rest of the screen in German. Now `debrief.transcript.moodLabel` in both catalogs, with the emotion kept in its own coloured span rather than interpolated, so the label translates and the layout is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`api.listSessions` now sends `?status=in_progress&limit=1` and the resume affordances offer whatever comes back as "Conversation in progress" (issue #501 §1). convsim-core implements that filter; apps/api's sibling `GET /api/sessions` ignored both parameters and returned every session newest first — so behind that layer the banner, Home's "Resume your conversation" and the Conversation screen's setup re-fetch would all point at the most recent session whether or not it had ended, which is worse than having no resume entry point at all. Same contract as convsim-core: `status` of all / in_progress / ended, the ended set covering DebriefGenerating, DebriefReady and Error rather than only Ended, `limit` defaulting to 50 and 400 outside 1-500. The state lists are mirrored with a comment naming their counterparts, since the two implementations have to agree. reply_speed was already synced into this layer; this is the other half of the same contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The First Words tutorial opens with "Above this conversation you'll see two meters: Engagement ... and Confidence ...", spends three of its six turns on them, and now answers "what are the meters?" with "the two labelled bars above the conversation". On default settings there were none: the Conversation Brief's meters toggle was the one field on that screen not derived from the scenario — difficulty, role name, language, voice and input mode all come from the scenario and the runtime, while show_state_meters sat at a hardcoded false and was only ever forced *off* again for scenarios that forbid meters. So the playtester's "default settings" first session was the tutorial narrating, for six turns, a panel that was switched off — which is the sharpest reading of issue #501 §3's "generally confused about meters and where they appear", and the one thing relocating, renaming and re-visualizing the panel could not fix. Now on wherever the scenario permits them (two player-visible variables in the tutorial's case), still forced off where it does not, and still one click away either way. The player checklist's P-M6 asked the tester to enable the toggle; it now asks them to confirm it is already on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
Contributor
Author
|
Addressed failing CI checks and pushed a fix (Backend tests). |
Settings is an i18n-migrated screen, but the plain wording level rendered `plainFlowState(s.state)` straight into the session list — hardcoded English prose. A German player on plain wording (the default) read "Your turn" and "Finished" in an otherwise German UI, which is the untranslated-string problem the i18n framework exists to prevent, introduced by the fix for issue #501 §2. The fourteen labels move out of a switch into FLOW_STATE_LABELS, whose keys double as locale keys under `sessionStates.`, so the catalogs and the English copy cannot say different things. Settings now renders `t(flowStateLabelKey(s.state))`; `plainFlowState` keeps returning English for the conversation screen, which is not migrated yet. An unmapped state maps to `sessionStates.unknown` rather than to a missing key, which the translator would have rendered as the literal key path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The resume path reads a session's current meter values back from
GET /api/sessions/{id}, because meter values otherwise only ever arrive with a
turn and a player who stepped out to Settings came back to a conversation with
no meters at all. convsim-core reports them as `visible_state`; apps/api's
sibling route did not, so against the TypeScript layer the same resume still
drew an empty meter panel until the next message.
This layer does not model per-variable visibility — its turn route already
broadcasts `state_vars` wholesale — so every tracked variable is reported.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
Issue #501 §2 names 'runtime' as one of four words "the average user might not understand", and its grounding bullet quotes the Settings toggle by name. This branch renamed the section heading from "Runtime" to "AI engine" and left both of the buttons under it saying "runtime": - RuntimeSettingsPanel's disclosure read "Show runtime advanced settings". It is visible on Settings without expanding anything, so the word reached the player before any deliberate peek. Now "Show advanced engine settings"; the notes inside the disclosure stay in engine terms, which is the "deliberately peek into the settings" the issue asks for. - PerformanceWarningBanner's button read "Runtime Settings" — on the conversation screen, in the slow-reply situation §1 is about, directly above the plain-language "Make replies faster →" link this branch added. Now "Open settings", same destination. Settings.test.tsx matched its own privacy disclosure with /show advanced/i, which the new engine label also matches; both selectors are now the exact accessible name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ending a conversation produces a debrief — that is what the conversation screen's End session button leads to. Ending from the resume strip left the player on whatever screen they were on, with the strip simply gone: the session had moved to DebriefGenerating and the debrief it earned was reachable only by typing its URL. That is the same dead end issue #501 is about, in the other direction. A successful end now navigates to that session's debrief; a failed one still just re-asks, so nothing claims a debrief that was never generated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
The transcript endpoint answers a session started with transcript saving off with no rows, always. The resume path only seeded its whole-turn counter from those rows, so resuming such a session restarted the labels at "Turn 1" over a session convsim-core was several turns into — the debrief-says-4 / transcript-says-1 mismatch of issue #501 §4, back on the very screen resume was added for. The session read the resume path already makes for the meters carries `turn_count`, which is the same whole-turn number the labels, max_turns and the debrief all use. Seed from it when there were no rows to count, and only while the player has not already sent a message, so a slow read cannot renumber a turn in flight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Issue #501 §2 named four words that landed badly on a new player, and "runtime" was one of them. The PR renamed it on the two Settings controls, but left it on the surfaces a first session actually passes through: the status badge in the app header (every screen), the Status panel on Home, the unreachable-engine error card, and the first-run load-error step. Home also labelled three rows "LLM", "STT" and "TTS" — acronyms a first-time player has no way to read. These are labels, not identifiers, so they are renamed outright rather than gated on the wording level: "AI engine", "AI model", "Voice input" and "Voice output" are accurate for both audiences and lose nothing a technical reader needs. The engine-term diagnostics behind the advanced disclosure and in api/client.ts are left alone — that is the deliberate peek the issue asks for. SPEC's status-card example and quickstart's status sentence follow. docs/screenshots.md keeps the old wording on purpose: its captions describe captured PNGs that still show it, and that re-capture pass is already tracked as a follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The resumable / ended partition was written out three times: once in convsim-core's listing route, once in the TypeScript proxy's, and once in Settings, which picks unfinished conversations out of a full listing client-side rather than through the filter. Three copies that must agree for resume to behave, with only comments saying so — add a flow state in one place and a conversation either loses its Resume button on the very screen issue #501's playtester was standing on, or keeps one after it has ended. The two TypeScript copies become one in @convsim/shared. The Python copy cannot import it, so a pytest guard parses the shared list and asserts the two still agree; it was verified to fail when one state is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
Issue #501 §3 was a naming mismatch: the tutorial said "the two meters at the top", the panel said "NPC state variables", and the setup toggle said "Show NPC state meters" — three names for one thing. This branch settled on "Conversation meters" for the panel and renamed the toggle to match, but the debrief's sparkline section still read "State meters across turns". So the player watched Conversation meters move for six turns and then reviewed State meters one screen later — the same mismatch, handed over at the end of the session instead of the start. Also finishes the "runtime" → "AI engine" rename this branch made on Home's status rows, the offline badge, the Logbook, the error copy and the Settings headings. Four player-facing strings in the first-run wizard and the model manager still said "runtime", which is one of the four words the playtest named and the first-run wizard is literally "the first few sessions" the report is about. New tests cover both the debrief panel's name and its humanized meter labels; the telemetry panel had no coverage at all before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The slow-reply notice is where the issue #501 detour began: the playtester hit a slow turn, went to Settings to speed the model up, "was a little unclear on how to do that", and started a new session rather than find their way back. Replacing "you can adjust settings" with a named link straight at "Settings → Reply speed" is the fix for that first step, and it had no test — neither the wording nor the #reply-speed anchor the Settings screen scrolls to, so the link could regress to bare /settings and land the player back at the top of a long page with nothing obviously changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renaming the Conversation Brief's toggle to "Show the conversation meters" left `scripts/capture-screenshots.mjs` matching a label that no longer exists. It degrades rather than fails — it logs a warning and carries on — so the capture still shows meters now that they default to on, but the selector is dead and the warning says the toggle is missing when the only thing that moved is its name. Does not touch docs/screenshots.md or the captured PNGs: those captions describe the images, which still show the pre-rewrite screen, and rewording them without re-shooting would make them lie about what they caption. That re-capture pass stays a follow-up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Reviewed code and pushed fixes. |
This branch has not been deployed
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
1 participant
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.
Playtest feedback from the first four sessions (text input, no audio, default settings) said the app "feels both too sparse and too technical in the first few sessions". This addresses all four sections of #501 and answers each of its open questions.
1. Resume, and finding the speed setting
The playtester went to Settings mid-tutorial to speed the model up, could not work out how, then "couldn't easily figure out how to resume my ongoing scenario and resorted to starting a new one".
Resume.
GET /api/sessionsdid not exist — the web client already called it and the shared type already documented it, but the route was never registered, so it answered 405 and nothing in the app could discover a session to resume. It now lists a profile's sessions newest first, with astatusfilter whosein_progressvalue is exactly the resumable set (started, not ended) and a boundedlimit. On top of it:The Conversation screen now reads its setup back from convsim-core when it arrives with no router state, which is exactly the resume case: navigating away and back, a reload, and a relaunch all used to fall through to defaults and drop the scene card, the chosen language and the voice settings the player picked.
Reply speed. "Make the model respond faster" was not an exposed concept. The knobs that existed — context length, GPU layers, CPU threads, sampling — are all named after the runtime and none is the dominant cost of a turn. That cost is how many tokens the NPC generates, one at a time, so
reply_speedturns it into one named choice: Quick replies / Balanced / Fuller replies. It scales the scenario's authored word cap rather than replacing it, so a pack that writes terse NPCs stays terser than one that writes expansive ones, and the generation budget moves with it.balancedreproduces the previously hardcoded 1024-token unscaled behaviour exactly, so the default experience is unchanged. Read per turn, so a change applies to the player's next message with no restart. It sits in its own Settings section above the engine knobs and is anchored, so the conversation screen's slow-reply notice links straight to it ("Make replies faster →" instead of "you can adjust settings").2. A buffer between player-facing and technical language
Two levels, not a scale, chosen by an LLM-familiarity question on the first-run Welcome step and revisitable from the tutorial debrief and Settings → Wording. A third intermediate level would need a different word per string with no rule saying which words belong to it.
At
plain(the default for a never-asked player) the conversation screen drops the session id, the raw flow-state identifier, the per-turnFlags:line and the runningEvent flags:strip; says "Something changed: Rapport milestone" instead of "Scenario event: rapport_milestone"; and names variables as words. Attechnicalevery identifier comes back, because that is what a bug report needs.The NPC also has a name now:
npc_nameresolves through the scenario'snpc.ref, so the transcript says "Alex Chen" rather than "NPC" — the most technical word on that screen, and only there because the name was never exposed over the API.The scripted tutorial was rewritten to drop "scenario event", "hidden prompt" and "rubric dimensions" while teaching the same mechanics.
3. Meter clarity
The tutorial hallucination. It was not a model at all: the tutorial runs on the scripted runtime, and the script had no branch for a question, so "what are the meters?" got the next canned line as if the player had agreed with something. The runtime now answers questions — meters, mood, is-this-AI, endings, debrief — with word-boundary matching, and the tour lines no longer presume a positive reply ("That sounds like a great goal", "Did you see that?"). Two guard sweeps fail if the named jargon or the presumptive phrasing ever returns.
4. Turn counting
One turn is the player's message plus the reply to it, so both rows carry the same number and the NPC's opening is labelled Opening rather than consuming turn 1. This is the number convsim-core already counted in
turn_count, thatduration.max_turnsis measured in, and that debrief key moments point at — so the debrief's "#3" and the transcript's "Turn 3" now mean the same moment, and "go to turn N" lands on the start of turn N instead of whichever half-turn was Nth in the list.No pack migration:
max_turnswas always whole turns.turn_pipelinenumbers a turnturn_count + 1once per exchange andscenario_statecompares that same number tomax_turns.Answers to the issue's open questions
plain/technical). "How familiar are you with AI language models?" with three answers; only "I work with them" selects technical.max_turns?max_turnswas already whole turns.Two bugs found while reviewing, not in the original report
resolve_scenario_infoconsults the catalog first outside the demo edition. So in the full app, which is the playtester's "default settings", the catalog entry played and the pack supplied only the library card: the de-jargoned opening line, goals and event instruction landed on the library card while the catalog entry went on speaking the old vocabulary in the session itself. Both copies now carry the same strings and the same turn budget, with a cross-copy guard (test_tutorial_catalog_entry_matches_the_pack) asserting every field a player reads.demo:, soresume.labeland the familiarity strings rendered as literal key paths.Testing
Run at the branch tip, independently in two sessions:
convsim-corepytest: 2244 passed, 3 skipped, plus the one known environment-only failure on this machine (test_disk_space_passes_when_sufficient—/has ~2 GiB free)apps/webvitest: 1539 passed, 56 filesapps/webtsc --noEmit: cleanscripts/check-i18n.mjs: cleanNew coverage: whole-turn labelling of an exchange, rollback not consuming a turn number, meter movement and placement, humanized vs raw variable keys,
npc_nameresolution for every seeded scenario, the list endpoint (ordering, three status filters, limit bounds, whole-turn counts), reply speed reaching both the prompt layer and the runtime budget, the resume entry points, and the tutorial's question handling. Both jargon guards were verified to fail when a single string is reverted.eslintoverapps/web/srcreports 2 errors and 15 warnings, all pre-existing onmain: the errors areno-unused-varsin__tests__/useMicCapture.test.ts, and the warnings arereact-refresh/only-export-componentsplusexhaustive-depsini18n/index.tsxandsetup/steps/InstallingStep.tsx. None in anything added here.Docs updated to match: a turn definition in
SPEC.md,architecture.mdandscenario-authoring.md;GET /api/sessionsadded to both route listings; reply speed leading the slow-inference remedies introubleshooting.md; resume, meters and the familiarity question inquickstart.md; and the player acceptance checklist brought up to the new screen, including a new step that walks the Settings detour end to end — the journey the report described, which the automated tests structurally cannot cover.Follow-ups, deliberately not in scope
docs/screenshots.mdand the captured PNGs are stale against the meters rewrite. That needs a re-capture pass; rewording the captions without re-shooting would make them lie about the images.Closes #501
🤖 Generated with Claude Code