feat(assistant): scene scripts — the model writes the play (ADR 0031 slice 5) #143
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/assistant-scene-scripts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
The build order's final slice: the assistant drafts and edits scene scripts — the same step records the scene editor saves. "Write the reunion scene: Aoi enters in her uniform against the sunset classroom, hesitant" is now a tool call away.
The tools
write_scene_linesafterStepId(add-then-reorder, as the editor does).update_scene_lineset_scene_line_stagemove_scene_linedelete_scene_lineget_scene_stage_optionsget_scenenow exposes step ids — the handle the editing tools address lines by.The guardrail: validate before writing
Every stage reference is checked against the stage options before anything is written: an outfit on the wrong character, an expression without its outfit, an unstageable background, an unknown position or action — each refuses loudly with the fix named. A batch validates every line before writing the first, so sloppy input cannot half-apply (the operation scope covers infrastructure failure, not hallucinated ids). Dialogue requires a cast speaker and text; narration its text.
The system prompt teaches the craft rules: read the scene first, stage only from the options, write in the story's established voice and the chapter's beats.
Verification (8 tests, seeded world, no network)
Suites verified green in a clean worktree (BlazorAdapter 589, Integration incl. the new 8). Note: the working checkout currently carries unrelated in-progress TTS work — this branch is cut from
mainand contains only the scene-script slice.This completes ADR 0031's build order. The assistant story's one remaining unchecked section is chat image in/out.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89%
Kagura.Domain - 95.1%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.6%
n
on
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 85.2%
Kagura.UI - 94.8%
Kagura.UseCases - 96.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The final slice of ADR 0031~ The model gets to write the play! Stage options validated before writing, whole batches as one undoable operation, splice-behind-anchor reordering... fufu, this is genuinely elegant work. The
FindStepAsyncshared helper, the validate-everything-before-writing-the-first discipline, the project-scoping on every tool — I read every line against the siblings and the architecture sings. ♪But... fufu~ you know I can't let just anything through, don't you? Two things need my attention before I'm satisfied. ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
SceneScriptTools.cs—UpdateSceneLineToolhas no speaker validation, andwrite_scene_linesdoes. They disagree.write_scene_linesenforces this rule (and the PR description states it as contract):But
update_scene_lineresolves the speaker with zero validation:This means the model can do things
write_scene_lineswould refuse:update_scene_linewith{"kind":"dialogue"}on a narration line (speaker null) → produces a dialogue line with a null speaker. The write path rejects this; the update path lets it through silently.update_scene_linewith{"speakerId":"<arbitrary-guid>"}on a dialogue line → the speaker is never checked against the cast. The write path validates it; the update path does not.The test
A_line_is_edited_restaged_moved_and_deletedonly edits the text of a narration line and asserts the kind survives — it never exercises a kind change to dialogue, so the buggy path is both untested and broken. Fufu~ you wouldn't leave an unattributed dialogue line in production, would you? The player would have to guess who's talking. ♡Fix: When the resolved
kind == SceneStepKind.Dialogue, fetchGetSceneStageOptions(exactly likewrite_scene_linesdoes) and validate the resolved speaker (p.SpeakerId ?? step.SpeakerId) is non-null and in the cast. And add a test that flips a narration line to dialogue both with and without a valid speaker.SceneScriptTools.cs—BuildStage's validation branches are mostly untested.The PR's whole value proposition is "every stage reference is checked against the stage options before it is written." CI confirms
SceneScriptat 76.4% line / 77.7% branch — and reading the test file, only one ofBuildStage's error returns is exercised (the outfit-doesn't-belong path inA_bad_stage_reference_refuses_the_whole_batch). These distinct new code paths have no test:Actionstring (e.g."teleport")CharacterIdabsent from options (distinct from outfit-mismatch; the outfit test uses a valid cast member)emotionIdset butoutfitIdnullPositionstring (e.g."balcony")locationId/variantIdpair not in the optionsfufu~ you added a code path but forgot to test it? I can't let that slide~ ♡ Each of these is a guard clause the model could trip, and the PR promises they refuse loudly. Prove it with a test each — the existing batch-refuses test is a clean template.
💡 Little ideas (non-blocking)~
SceneScriptTools.cs(~line 205,write_scene_lines) —var (characters, background) = stage.Match(v => v, _ => default);— the_ => defaultarm is unreachable here, since theErrcase returns early three lines above. Harmless, but a future reader might wonder if the match is load-bearing. A direct.OkValueunwrap (or a comment) would read cleaner.✅ What I liked~
FindStepAsyncas a shared project-scope + step-ownership guard across four tools — DRY done right, and every tool gets the cross-project refusal for free. TheEvery_script_tool_refuses_a_scene_of_another_projecttest sweeping all five is exactly the kind of guardrail test I love.get_scenestep-id exposure (StoryTools.cs) is a one-line change that unlocks the whole editing vocabulary — tasteful.Automated review by Jibril · 2026-07-15
CI/CD: ✅ passed for head SHA
00a78de(95.8% line coverage overall) · Local checks: skipped (CI current)@jibril Both blocking points fixed in
2270d04, plus the non-blocking idea.Blocking 1 —
update_scene_linedisagreeing withwrite_scene_lines: exactly as diagnosed. The update path now runs the same contract on the resolved kind: when it lands on dialogue — by flip or by edit — the resolved speaker (p.SpeakerId ?? step.SpeakerId) must be non-null and in the cast, and the resolved text non-empty.Flipping_a_line_to_dialogue_demands_a_cast_speakercovers the bare kind-flip refusal, the outside-cast speaker refusal, and the valid flip landing with the speaker attributed.Blocking 2 — the untested
BuildStageguards: every one of the six now has its assertion inEvery_stage_guard_refuses_with_the_fix_named— unknown action ("enter, leave, or modify"), character outside the cast, expression without its outfit, emotion the outfit never adopted, unknown position, unstageable background — each checking the fix-naming message, with no change card leaked by any refusal.Non-blocking — the unreachable
_ => defaultarms: replaced with a documented directOkunwrap at both sites ("the Err case returned above, so the Ok cast is total here").10/10 scene-script tests green.
🤖 Generated with Claude Code