docs(icm): story creation module — stages 00/01/02/05#41
Open
ctwoodwa wants to merge 43 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ICM-stage documentation for a Story Creation Module, updates .gitignore to allow tracking ICM outputs and ignore local worktrees/OpenWolf state, and introduces prose-telemetry and story-scaffolding Python modules (embeddings, energy, simulation, character agent, Chekhov detector, cross-scene state) plus a CLI script. ChangesStory Creation Module Specifications
Prose Telemetry & Story Scaffolds
Sequence DiagramsequenceDiagram
participant CLI as story_run.py (CLI)
participant Sim as simulation.run_simulation
participant Agent as character_decide / character_agent
participant Chekhov as chekhov.run_detector
participant State as CrossSceneState
CLI->>Sim: run simulation (characters, ticks, state?)
Sim->>Agent: character_decide (prompt, model)
Agent-->>Sim: CharacterAction (action, internal_state, spoken)
Sim->>State: state.learn() on high-significance events
Sim-->>CLI: SimulationTrace
CLI->>Chekhov: run_detector(scene_text, state)
Chekhov->>State: register/resolve plants
Chekhov-->>CLI: ChekhovVerdict
CLI->>State: save(state.yaml)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
icm/01_discovery/output/discovery-story-creation-2026-05-26.md (1)
70-91: 💤 Low valueConsider adding language identifier to code fence.
Markdownlint flags this fence as missing a language specifier. Since this is a directory tree diagram, you can use
textor leave it empty (though explicit is preferred).📝 Proposed fix
-``` +```text <bookRoot>/ .flight-deck/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icm/01_discovery/output/discovery-story-creation-2026-05-26.md` around lines 70 - 91, The markdown code fence in discovery-story-creation-2026-05-26.md that contains the directory tree (starting with "<bookRoot>/") is missing a language identifier; update the opening fence from ``` to ```text (and ensure the closing fence remains ```), so the tree block is explicitly marked as plain text for markdownlint and renderers.icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md (1)
79-81: ⚖️ Poor tradeoffPhase ordering: structure routes before structure package.
Lines 79-81 define book-server routes for story structure (
GET/PUT /story/structure,GET /story/structure/beats/:slug), butpackages/story-structureisn't built until Phase 2. These routes will have no implementation to call in Phase 1.Consider moving these three structure-specific routes to Phase 2 (P2-2 or P2-3) to align route availability with package availability, or clarify that Phase 1 stubs these routes to return
501 Not Implementeduntil Phase 2 completes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md` around lines 79 - 81, These three story-structure routes (GET /api/books/:bookId/story/structure, PUT /api/books/:bookId/story/structure, GET /api/books/:bookId/story/structure/beats/:slug) are declared before packages/story-structure is implemented; either move their route definitions to Phase 2 (P2-2 or P2-3) so they only appear when packages/story-structure exists, or keep them in Phase 1 but implement clear stubs in the book-server handlers that return 501 Not Implemented with a message referencing the missing packages/story-structure implementation; update any routing registration or documentation accordingly so availability matches package delivery.icm/02_architecture/output/arch-story-creation-2026-05-26.md (1)
45-46: 💤 Low valueConsider varying sentence structure.
Three successive sentences begin with "All", which slightly reduces readability.
♻️ Proposed style improvement
-All layers are YAML on disk. All layers are graph nodes or node properties. -All layers expose read/write via book-server API. +All layers are YAML on disk and represented as graph nodes or node properties, +with read/write access exposed via the book-server API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icm/02_architecture/output/arch-story-creation-2026-05-26.md` around lines 45 - 46, The three successive sentences starting with "All" reduce readability; rephrase them to vary sentence structure and combine related facts. Replace the three lines "All layers are YAML on disk. All layers are graph nodes or node properties. All layers expose read/write via book-server API." with a single or two sentences such as: "Layers are stored as YAML on disk and represented as graph nodes or node properties; they expose read/write via the book-server API." Ensure the revised sentence(s) preserve the same facts and the phrase "book-server API" and "YAML" remain present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@icm/02_architecture/output/arch-story-creation-2026-05-26.md`:
- Around line 147-149: The documentation references
storyGraph.getActiveBeat(chapterId) but IStoryGraph (the interface) does not
declare getActiveBeat; update the design by either adding
getActiveBeat(chapterId): Beat | null (or appropriate return type) to the
IStoryGraph interface declaration or clarify that getActiveBeat belongs to the
story-structure package and update the doc to call out that storyGraph
implements a different interface (e.g., IStructuredStoryGraph) that exposes
getActiveBeat; modify the text around the system message builder to reference
the correct interface name (IStoryGraph or IStructuredStoryGraph) and include
the method signature so readers can locate it.
- Around line 58-69: The API references an undefined StoryGraphFilter used by
IStoryGraph.query; add a new exported type alias named StoryGraphFilter that
defines the allowed query keys (e.g., optional id, type, label, tags, metadata
predicates, and relationship direction/edgeType) so callers can express node
filters; update the public API surface by inserting export type StoryGraphFilter
= { id?: string; type?: 'Character'|'Location'|'Object'|'Event'; label?: string;
tags?: string[]; metadata?: Record<string, any>; edgeType?: StoryEdge['type'];
direction?: 'in'|'out'|'both' } (or equivalent fields your implementation uses)
and ensure the concrete types file (types.ts / P1-1 list) exports this type so
IStoryGraph.query has a resolved reference.
In `@icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md`:
- Around line 243-245: The acceptance criteria reference a scene field
narrativePosition that isn’t defined in the schemas; add a new numeric (or
integer) property named narrativePosition to the scene-related schema (e.g.,
event.schema.json or the specific Scene schema/class) with a clear description,
default (e.g., 0), and validation (minimum 0) and update any serialization/DTOs
and persistence mappings (Scene, Event models and create/update handlers such as
Scene.create/Scene.update or EventRepository methods) to read/write this field
so dragging a scene card can persist the updated narrativePosition without
altering storyTime.
- Around line 303-316: The architecture and buildStoryContext signature are
inconsistent about chapters; update the design and code comments to be explicit:
decide whether chapters are a new StoryNode subtype (add ChapterNode to the
StoryNode union and document its fields), or document that chapterId maps to an
existing EventNode pattern (describe the mapping and any query logic used by
buildStoryContext), or state that chapterId is external to the Narrative Graph
(describe how buildStoryContext resolves an external book structure into graph
nodes). Make a single, explicit choice and update references to StoryNode, the
StoryNode union, and the buildStoryContext(bookId, chapterId) doc comment so
callers and reviewers know where chapterId is modeled and how the function
resolves it.
---
Nitpick comments:
In `@icm/01_discovery/output/discovery-story-creation-2026-05-26.md`:
- Around line 70-91: The markdown code fence in
discovery-story-creation-2026-05-26.md that contains the directory tree
(starting with "<bookRoot>/") is missing a language identifier; update the
opening fence from ``` to ```text (and ensure the closing fence remains ```), so
the tree block is explicitly marked as plain text for markdownlint and
renderers.
In `@icm/02_architecture/output/arch-story-creation-2026-05-26.md`:
- Around line 45-46: The three successive sentences starting with "All" reduce
readability; rephrase them to vary sentence structure and combine related facts.
Replace the three lines "All layers are YAML on disk. All layers are graph nodes
or node properties. All layers expose read/write via book-server API." with a
single or two sentences such as: "Layers are stored as YAML on disk and
represented as graph nodes or node properties; they expose read/write via the
book-server API." Ensure the revised sentence(s) preserve the same facts and the
phrase "book-server API" and "YAML" remain present.
In `@icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md`:
- Around line 79-81: These three story-structure routes (GET
/api/books/:bookId/story/structure, PUT /api/books/:bookId/story/structure, GET
/api/books/:bookId/story/structure/beats/:slug) are declared before
packages/story-structure is implemented; either move their route definitions to
Phase 2 (P2-2 or P2-3) so they only appear when packages/story-structure exists,
or keep them in Phase 1 but implement clear stubs in the book-server handlers
that return 501 Not Implemented with a message referencing the missing
packages/story-structure implementation; update any routing registration or
documentation accordingly so availability matches package delivery.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 400f418f-f2ad-4309-b8e3-695c6ebd8b0e
📒 Files selected for processing (6)
.gitignoreicm/00_intake/output/intake-story-creation-2026-05-26.mdicm/01_discovery/output/discovery-story-creation-2026-05-26.mdicm/02_architecture/output/arch-story-creation-2026-05-26.mdicm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.mdicm/CONTEXT.md
Comment on lines
+58
to
+69
| ```typescript | ||
| // Public API surface | ||
| export type StoryNode = CharacterNode | LocationNode | ObjectNode | EventNode; | ||
| export type StoryEdge = RelationshipEdge | AppearanceEdge | DependencyEdge; | ||
| export interface IStoryGraph { | ||
| getNode(id: string): StoryNode | null; | ||
| putNode(node: StoryNode): void; | ||
| getEdges(nodeId: string): StoryEdge[]; | ||
| putEdge(edge: StoryEdge): void; | ||
| query(filter: StoryGraphFilter): StoryNode[]; | ||
| } | ||
| export function createFileSystemGraph(bookRoot: string): IStoryGraph; |
There was a problem hiding this comment.
Define the StoryGraphFilter type.
The IStoryGraph.query() method references StoryGraphFilter but this type is not defined in the API surface specification. The implementation plan (P1-1) also doesn't include this type in the types.ts file list.
📝 Suggested addition to the API surface
Add the filter type definition after line 61:
export type StoryEdge = RelationshipEdge | AppearanceEdge | DependencyEdge;
+export interface StoryGraphFilter {
+ nodeType?: 'character' | 'location' | 'object' | 'event';
+ // Additional filter criteria to be defined during implementation
+}
export interface IStoryGraph {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ```typescript | |
| // Public API surface | |
| export type StoryNode = CharacterNode | LocationNode | ObjectNode | EventNode; | |
| export type StoryEdge = RelationshipEdge | AppearanceEdge | DependencyEdge; | |
| export interface IStoryGraph { | |
| getNode(id: string): StoryNode | null; | |
| putNode(node: StoryNode): void; | |
| getEdges(nodeId: string): StoryEdge[]; | |
| putEdge(edge: StoryEdge): void; | |
| query(filter: StoryGraphFilter): StoryNode[]; | |
| } | |
| export function createFileSystemGraph(bookRoot: string): IStoryGraph; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/02_architecture/output/arch-story-creation-2026-05-26.md` around lines 58
- 69, The API references an undefined StoryGraphFilter used by
IStoryGraph.query; add a new exported type alias named StoryGraphFilter that
defines the allowed query keys (e.g., optional id, type, label, tags, metadata
predicates, and relationship direction/edgeType) so callers can express node
filters; update the public API surface by inserting export type StoryGraphFilter
= { id?: string; type?: 'Character'|'Location'|'Object'|'Event'; label?: string;
tags?: string[]; metadata?: Record<string, any>; edgeType?: StoryEdge['type'];
direction?: 'in'|'out'|'both' } (or equivalent fields your implementation uses)
and ensure the concrete types file (types.ts / P1-1 list) exports this type so
IStoryGraph.query has a resolved reference.
Comment on lines
+147
to
+149
|
|
||
| The system message builder calls `storyGraph.getActiveBeat(chapterId)` and | ||
| appends the primer if present. No call is made if the book has no structure. |
There was a problem hiding this comment.
getActiveBeat method not defined in IStoryGraph interface.
Line 148 describes calling storyGraph.getActiveBeat(chapterId), but this method is not included in the IStoryGraph interface definition (lines 62-68). This method should either be added to IStoryGraph or clarified as belonging to a different interface (perhaps from packages/story-structure).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/02_architecture/output/arch-story-creation-2026-05-26.md` around lines
147 - 149, The documentation references storyGraph.getActiveBeat(chapterId) but
IStoryGraph (the interface) does not declare getActiveBeat; update the design by
either adding getActiveBeat(chapterId): Beat | null (or appropriate return type)
to the IStoryGraph interface declaration or clarify that getActiveBeat belongs
to the story-structure package and update the doc to call out that storyGraph
implements a different interface (e.g., IStructuredStoryGraph) that exposes
getActiveBeat; modify the text around the system message builder to reference
the correct interface name (IStoryGraph or IStructuredStoryGraph) and include
the method signature so readers can locate it.
Comment on lines
+243
to
+245
| **Acceptance criteria:** | ||
| - Dragging a scene card on the narrative-time track updates its `narrativePosition` | ||
| without changing its `storyTime` |
There was a problem hiding this comment.
narrativePosition field not defined in architecture schemas.
The acceptance criteria reference updating a scene's narrativePosition field (line 244), but this field is not defined in any of the architecture document's schemas. Please specify which schema (likely event.schema.json or a scene schema) should include this field and its type.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md`
around lines 243 - 245, The acceptance criteria reference a scene field
narrativePosition that isn’t defined in the schemas; add a new numeric (or
integer) property named narrativePosition to the scene-related schema (e.g.,
event.schema.json or the specific Scene schema/class) with a clear description,
default (e.g., 0), and validation (minimum 0) and update any serialization/DTOs
and persistence mappings (Scene, Event models and create/update handlers such as
Scene.create/Scene.update or EventRepository methods) to read/write this field
so dragging a scene card can persist the updated narrativePosition without
altering storyTime.
Comment on lines
+303
to
+316
| ### P6-1 — Editorial chat system-message primer | ||
|
|
||
| **File:** `apps/web/src/features/chat/systemMessage.ts` | ||
|
|
||
| Add `buildStoryContext(bookId, chapterId): Promise<StoryContext | null>`. | ||
| Called by the chat panel on open. Returns null gracefully if no structure exists. | ||
|
|
||
| **Acceptance criteria:** | ||
| - Opening chat on a chapter whose beat has a `chatPrimer` → system message | ||
| contains the primer text | ||
| - Opening chat on an unstructured book → no story context injected; no error | ||
| - `chatPrimer` updates to a new beat when the user advances the scene to the | ||
| next beat; re-opening chat picks up the new primer | ||
|
|
There was a problem hiding this comment.
Clarify chapter entity modeling in the Narrative Graph.
The buildStoryContext function signature includes a chapterId parameter (line 307), and the architecture document also references chapterId (arch line 148). However, the architecture's StoryNode types (arch line 60) only include CharacterNode | LocationNode | ObjectNode | EventNode. Chapters are not defined as first-class graph nodes.
Please clarify whether:
- Chapters are a distinct node type to be added to the architecture
- Chapters map to a specific event node pattern
- ChapterId refers to an external book-structure concept outside the story graph
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/05_implementation-plan/output/impl-plan-story-creation-2026-05-26.md`
around lines 303 - 316, The architecture and buildStoryContext signature are
inconsistent about chapters; update the design and code comments to be explicit:
decide whether chapters are a new StoryNode subtype (add ChapterNode to the
StoryNode union and document its fields), or document that chapterId maps to an
existing EventNode pattern (describe the mapping and any query logic used by
buildStoryContext), or state that chapterId is external to the Narrative Graph
(describe how buildStoryContext resolves an external book structure into graph
nodes). Make a single, explicit choice and update references to StoryNode, the
StoryNode union, and the buildStoryContext(bookId, chapterId) doc comment so
callers and reviewers know where chapterId is modeled and how the function
resolves it.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@icm/05_implementation-plan/output/writer-onboarding-session-2026-05-26.md`:
- Around line 33-43: Several fenced code blocks are missing language identifiers
(triggering MD040); find the ASCII-art UI block that begins with "You haven't
set a story structure yet." and the other three plain fenced blocks later in the
document and update each opening fence from ``` to include a language label (use
```text for these ASCII/console-style blocks) so the linter recognizes them;
ensure every matching closing fence remains ``` and that only the opening
triple-backtick lines are changed.
- Around line 262-263: The doc currently lists two conflicting onboarding
completion endpoints (`PUT /story/structure` and `POST
/story/onboarding/complete`); pick one canonical write path and update the spec
text to state that canonical choice, then explicitly document the relationship
of the other endpoint (if it remains) — e.g., state that `POST
/story/onboarding/complete` is either a thin wrapper that invokes `PUT
/story/structure` or is deprecated/replaces it; update all occurrences
(including the lines referenced) to consistently reflect the chosen canonical
endpoint and the required call flow and payload contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89a54f4d-aa6e-47b2-af7a-8adf44828a98
📒 Files selected for processing (1)
icm/05_implementation-plan/output/writer-onboarding-session-2026-05-26.md
Comment on lines
+33
to
+43
| ``` | ||
| ┌─────────────────────────────────────────────────────┐ | ||
| │ You haven't set a story structure yet. │ | ||
| │ │ | ||
| │ [ Guide me ] [ I know my structure ] │ | ||
| │ │ | ||
| │ "Guide me" opens a short conversation that takes │ | ||
| │ your story idea and builds the structure with you. │ | ||
| │ Takes about 10 exchanges. │ | ||
| └─────────────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdown lint.
Four fenced blocks omit a language, which matches the MD040 warnings.
Proposed doc fix
- ```
+ ```text
...
- ```
+ ```
- ```
+ ```text
...
- ```
+ ```
- ```
+ ```text
...
- ```
+ ```
- ```
+ ```text
...
- ```
+ ```Also applies to: 54-59, 289-302, 305-309
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/05_implementation-plan/output/writer-onboarding-session-2026-05-26.md`
around lines 33 - 43, Several fenced code blocks are missing language
identifiers (triggering MD040); find the ASCII-art UI block that begins with
"You haven't set a story structure yet." and the other three plain fenced blocks
later in the document and update each opening fence from ``` to include a
language label (use ```text for these ASCII/console-style blocks) so the linter
recognizes them; ensure every matching closing fence remains ``` and that only
the opening triple-backtick lines are changed.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@icm/06_build/scaffolds/prose_telemetry/embeddings.py`:
- Around line 59-64: The cosine_similarity function currently truncates vectors
using min(len(a), len(b)) which hides dimension mismatches; change it to
validate that len(a) == len(b) at the start of cosine_similarity and raise a
clear exception (e.g., ValueError with message including both lengths) if they
differ, then proceed to compute dot, na, nb and return dot/(na*nb) or 0.0 on
zero-norm as before.
In `@icm/06_build/scaffolds/prose_telemetry/energy_function.py`:
- Line 168: The current assignment "dominant = max(penalties, key=penalties.get)
if penalties else """ in report() picks a detector even when all penalty values
are 0; change it to first find the argmax (e.g., max_key = max(penalties,
key=penalties.get)) and then set dominant = max_key only if penalties[max_key] >
0, otherwise set dominant to an empty string. Update the code that uses the
dominant variable accordingly so no detector is reported when the highest
penalty is zero.
In `@icm/06_build/scaffolds/scripts/story_run.py`:
- Around line 217-243: The parser currently accepts multiple mode flags and
resolves them via if/elif; make the modes mutually exclusive by creating a
mutually exclusive group on the ArgumentParser (use
p.add_mutually_exclusive_group(required=True)) and move the three mode arguments
(--demo, --story, --measure) into that group instead of calling p.add_argument
on p for those flags; keep the existing argument options
(action/type/metavar/help) and leave other arguments on p as-is so only one mode
can be supplied.
- Around line 179-210: cmd_measure currently loads a transient CrossSceneState
(using CrossSceneState.new when args.state is missing) so mutations from
run_detector are discarded and typos in --state are hidden; change cmd_measure
to require args.state (fail fast with a clear error/exit if args.state is not
provided or the path does not exist) and after running detectors (e.g., after
calling run_detector and chekhov_detect/compute_energy) persist the mutated
state back to disk by calling the state's save method (e.g.,
state.save(state_path) or the equivalent persistence API on CrossSceneState) so
detector mutations are retained.
In `@icm/06_build/scaffolds/story_engine/character_agent.py`:
- Around line 119-130: _parse_action currently assumes json.loads returns a dict
and calls data.get(...), which crashes for valid non-dict JSON; update
_parse_action to validate that data is a dict (e.g., if not isinstance(data,
dict): raise ValueError("Model response JSON must be an object")), coerce the
extracted fields for action and internal_state to strings (e.g.,
str(data.get(...))) to avoid TypeErrors, and expand the except clause to catch
ValueError and TypeError in addition to json.JSONDecodeError so the fallback
CharacterAction(character=name, action=raw[:200], internal_state="",
spoken=None) is used for non-object or otherwise invalid JSON values.
In `@icm/06_build/scaffolds/story_engine/simulation.py`:
- Around line 132-135: Wrap the LLM call for each tick in a per-tick try/except
and fallback to the rule-based decision: when use_ollama is true and you call
character_decide(char, world.describe(), [], ollama_model), catch any exception
(and treat a None/falsy return as failure) and on failure call
_rule_based_decide(char, tick) instead; also log a warning including the
exception/context so the simulation continues rather than aborting. Ensure you
reference use_ollama, character_decide, _rule_based_decide, ollama_model,
world.describe(), and tick when locating and modifying the code.
In `@icm/06_build/scaffolds/story_state/chekhov_state.py`:
- Around line 157-160: The computed score in the elif not overdue branch
(currently set via score = len(unresolved) * 0.05) can exceed 1.0; change the
code to compute the raw value (e.g., raw_score = len(unresolved) * 0.05) and
then clamp it into the documented [0.0, 1.0] range before assigning to
ChekhovVerdict.score (use min/max or a clamp utility). Update the branch in
chekhov_state.py where score and diag/verdict are set so the final score passed
to ChekhovVerdict (or whatever object uses score) is guaranteed between 0.0 and
1.0.
- Line 43: The regex list contains a typo "breached for" which should be
"reached for"; locate the pattern r"\breached for\b" in chekhov_state (the
payoff-detection regex list) and replace it with r"\breached for\b" ->
r"\breached for\b" corrected to r"\breached for\b" Wait—oops. Replace "breached
for" with "reached for" so the entry becomes r"\breached for\b" corrected to
r"\breached for\b" STOP
In `@icm/06_build/scaffolds/story_state/cross_scene_state.py`:
- Around line 199-203: After calling yaml.safe_load in CrossSceneState.load, add
a defensive shape check that verifies data is a dict (e.g. isinstance(data,
dict)) before accessing data["story_id"]; if not, raise a clear ValueError
(include the path and a short message like "invalid or empty YAML: expected
mapping with 'story_id'") so load() fails fast on empty/malformed files rather
than crashing when data["story_id"] is accessed.
- Around line 114-118: The persisted YAML breaks on Python tuples; modify
update_relationship to append a plain list instead of a tuple so yaml.safe_load
won't emit !!python/tuple tags: change the append from (scene, weight) to
[scene, weight] in update_relationship and update any code that consumes
relationship_history (iteration/unpacking) to treat entries as list pairs rather
than tuples; ensure all reads/writes of relationship_history use list pairs to
maintain safe YAML round-trips.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 361a11aa-1e6f-487a-b28b-92c707a9033e
📒 Files selected for processing (11)
icm/06_build/scaffolds/prose_telemetry/__init__.pyicm/06_build/scaffolds/prose_telemetry/embeddings.pyicm/06_build/scaffolds/prose_telemetry/energy_function.pyicm/06_build/scaffolds/requirements.txticm/06_build/scaffolds/scripts/story_run.pyicm/06_build/scaffolds/story_engine/__init__.pyicm/06_build/scaffolds/story_engine/character_agent.pyicm/06_build/scaffolds/story_engine/simulation.pyicm/06_build/scaffolds/story_state/__init__.pyicm/06_build/scaffolds/story_state/chekhov_state.pyicm/06_build/scaffolds/story_state/cross_scene_state.py
✅ Files skipped from review due to trivial changes (1)
- icm/06_build/scaffolds/requirements.txt
Comment on lines
+59
to
+64
| def cosine_similarity(a: list[float], b: list[float]) -> float: | ||
| n = min(len(a), len(b)) | ||
| dot = sum(a[i] * b[i] for i in range(n)) | ||
| na = math.sqrt(sum(x * x for x in a)) | ||
| nb = math.sqrt(sum(x * x for x in b)) | ||
| return dot / (na * nb) if na and nb else 0.0 |
There was a problem hiding this comment.
Fail fast on embedding dimension mismatch.
Line 60 silently truncates to min(len(a), len(b)), which yields invalid cosine values for mismatched vectors instead of surfacing a bug. This can skew downstream similarity thresholds.
Suggested fix
def cosine_similarity(a: list[float], b: list[float]) -> float:
- n = min(len(a), len(b))
- dot = sum(a[i] * b[i] for i in range(n))
+ if len(a) != len(b):
+ raise ValueError("Embedding vectors must have the same dimension")
+ dot = sum(x * y for x, y in zip(a, b))
na = math.sqrt(sum(x * x for x in a))
nb = math.sqrt(sum(x * x for x in b))
return dot / (na * nb) if na and nb else 0.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/prose_telemetry/embeddings.py` around lines 59 - 64,
The cosine_similarity function currently truncates vectors using min(len(a),
len(b)) which hides dimension mismatches; change it to validate that len(a) ==
len(b) at the start of cosine_similarity and raise a clear exception (e.g.,
ValueError with message including both lengths) if they differ, then proceed to
compute dot, na, nb and return dot/(na*nb) or 0.0 on zero-norm as before.
| raw = sum(penalties.values()) / total_w | ||
| score = round(min(raw, 1.0), 4) | ||
|
|
||
| dominant = max(penalties, key=penalties.get) if penalties else "" |
There was a problem hiding this comment.
Avoid reporting a dominant issue when all penalties are zero.
Line 168 always selects a detector whenever penalties exists, so report() can show a misleading dominant issue with 0.00 penalty.
Suggested fix
- dominant = max(penalties, key=penalties.get) if penalties else ""
+ dominant = ""
+ if penalties:
+ top = max(penalties.items(), key=lambda x: x[1])
+ if top[1] > 0:
+ dominant = top[0]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/prose_telemetry/energy_function.py` at line 168, The
current assignment "dominant = max(penalties, key=penalties.get) if penalties
else """ in report() picks a detector even when all penalty values are 0; change
it to first find the argmax (e.g., max_key = max(penalties, key=penalties.get))
and then set dominant = max_key only if penalties[max_key] > 0, otherwise set
dominant to an empty string. Update the code that uses the dominant variable
accordingly so no detector is reported when the highest penalty is zero.
Comment on lines
+179
to
+210
| state_path = Path(args.state) if args.state else None | ||
| state = CrossSceneState.load(state_path) if (state_path and state_path.exists()) \ | ||
| else CrossSceneState.new("measure-session") | ||
|
|
||
| # Chekhov | ||
| chekhov = chekhov_detect( | ||
| scene_text=text, | ||
| beat_slug=args.beat or "unknown", | ||
| scene_index=0, | ||
| total_scenes=args.total_beats, | ||
| state=state, | ||
| ) | ||
|
|
||
| # Placeholder verdicts for other detectors | ||
| # (each real detector will replace its placeholder slot) | ||
| all_verdicts = { | ||
| "chekhov-state": chekhov.verdict, | ||
| "pacing": "green", | ||
| "tension-escalation": "green", | ||
| "mru-completeness": "green", | ||
| } | ||
| energy = compute_energy( | ||
| all_verdicts, template=args.template or "save-the-cat", | ||
| beat_position=0, total_beats=args.total_beats, | ||
| ) | ||
|
|
||
| print(f"\nMeasuring: {args.measure}") | ||
| print(f"Beat: {args.beat or 'unknown'} | Template: {args.template or 'save-the-cat'}") | ||
| print(f"\n{chekhov.diagnostic}") | ||
| print(f"\n{energy.report()}") | ||
| print_inventory(state) | ||
|
|
There was a problem hiding this comment.
Persist --measure state mutations and fail fast on missing --state.
run_detector(...) mutates state, but those mutations are discarded in cmd_measure. Also, a missing --state path silently creates a new session, which can hide path typos and skew telemetry history.
Suggested fix
def cmd_measure(args):
"""Measure a rendered prose file against story telemetry detectors."""
text = Path(args.measure).read_text(encoding="utf-8")
state_path = Path(args.state) if args.state else None
- state = CrossSceneState.load(state_path) if (state_path and state_path.exists()) \
- else CrossSceneState.new("measure-session")
+ if state_path and not state_path.exists():
+ raise FileNotFoundError(f"--state file not found: {state_path}")
+ state = (
+ CrossSceneState.load(state_path)
+ if state_path
+ else CrossSceneState.new("measure-session")
+ )
@@
print(f"\n{chekhov.diagnostic}")
print(f"\n{energy.report()}")
print_inventory(state)
+ if state_path:
+ state.save(state_path)
+ print(f"\nState saved → {state_path}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| state_path = Path(args.state) if args.state else None | |
| state = CrossSceneState.load(state_path) if (state_path and state_path.exists()) \ | |
| else CrossSceneState.new("measure-session") | |
| # Chekhov | |
| chekhov = chekhov_detect( | |
| scene_text=text, | |
| beat_slug=args.beat or "unknown", | |
| scene_index=0, | |
| total_scenes=args.total_beats, | |
| state=state, | |
| ) | |
| # Placeholder verdicts for other detectors | |
| # (each real detector will replace its placeholder slot) | |
| all_verdicts = { | |
| "chekhov-state": chekhov.verdict, | |
| "pacing": "green", | |
| "tension-escalation": "green", | |
| "mru-completeness": "green", | |
| } | |
| energy = compute_energy( | |
| all_verdicts, template=args.template or "save-the-cat", | |
| beat_position=0, total_beats=args.total_beats, | |
| ) | |
| print(f"\nMeasuring: {args.measure}") | |
| print(f"Beat: {args.beat or 'unknown'} | Template: {args.template or 'save-the-cat'}") | |
| print(f"\n{chekhov.diagnostic}") | |
| print(f"\n{energy.report()}") | |
| print_inventory(state) | |
| state_path = Path(args.state) if args.state else None | |
| if state_path and not state_path.exists(): | |
| raise FileNotFoundError(f"--state file not found: {state_path}") | |
| state = ( | |
| CrossSceneState.load(state_path) | |
| if state_path | |
| else CrossSceneState.new("measure-session") | |
| ) | |
| # Chekhov | |
| chekhov = chekhov_detect( | |
| scene_text=text, | |
| beat_slug=args.beat or "unknown", | |
| scene_index=0, | |
| total_scenes=args.total_beats, | |
| state=state, | |
| ) | |
| # Placeholder verdicts for other detectors | |
| # (each real detector will replace its placeholder slot) | |
| all_verdicts = { | |
| "chekhov-state": chekhov.verdict, | |
| "pacing": "green", | |
| "tension-escalation": "green", | |
| "mru-completeness": "green", | |
| } | |
| energy = compute_energy( | |
| all_verdicts, template=args.template or "save-the-cat", | |
| beat_position=0, total_beats=args.total_beats, | |
| ) | |
| print(f"\nMeasuring: {args.measure}") | |
| print(f"Beat: {args.beat or 'unknown'} | Template: {args.template or 'save-the-cat'}") | |
| print(f"\n{chekhov.diagnostic}") | |
| print(f"\n{energy.report()}") | |
| print_inventory(state) | |
| if state_path: | |
| state.save(state_path) | |
| print(f"\nState saved → {state_path}") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/scripts/story_run.py` around lines 179 - 210,
cmd_measure currently loads a transient CrossSceneState (using
CrossSceneState.new when args.state is missing) so mutations from run_detector
are discarded and typos in --state are hidden; change cmd_measure to require
args.state (fail fast with a clear error/exit if args.state is not provided or
the path does not exist) and after running detectors (e.g., after calling
run_detector and chekhov_detect/compute_energy) persist the mutated state back
to disk by calling the state's save method (e.g., state.save(state_path) or the
equivalent persistence API on CrossSceneState) so detector mutations are
retained.
Comment on lines
+217
to
+243
| p = argparse.ArgumentParser( | ||
| description="Flight Deck story engine — simulation + telemetry", | ||
| formatter_class=argparse.RawDescriptionHelpFormatter, | ||
| epilog="See module docstring for full usage examples.", | ||
| ) | ||
| p.add_argument("--demo", action="store_true", help="Run built-in demo story") | ||
| p.add_argument("--story", type=Path, metavar="FILE", help="Path to story YAML") | ||
| p.add_argument("--measure", type=Path, metavar="FILE", help="Measure a prose file") | ||
| p.add_argument("--ticks", type=int, default=15, help="Simulation ticks (default 15)") | ||
| p.add_argument("--model", default="mistral", help="Ollama model (default: mistral)") | ||
| p.add_argument("--template", default="save-the-cat", help="Story template slug") | ||
| p.add_argument("--beat", default=None, help="Beat slug for --measure") | ||
| p.add_argument("--total-beats", type=int, default=15, dest="total_beats") | ||
| p.add_argument("--min-sig", type=float, default=0.3, dest="min_sig", | ||
| help="Min significance to display in trace (default 0.3)") | ||
| p.add_argument("--save-state", metavar="PATH", help="Save cross-scene state YAML") | ||
| p.add_argument("--state", metavar="PATH", help="Load existing state for --measure") | ||
| args = p.parse_args() | ||
|
|
||
| if args.demo: | ||
| cmd_demo(args) | ||
| elif args.story: | ||
| cmd_run(args) | ||
| elif args.measure: | ||
| cmd_measure(args) | ||
| else: | ||
| p.print_help() |
There was a problem hiding this comment.
Make execution modes mutually exclusive in argparse.
Right now conflicting mode flags are silently accepted and resolved by if/elif order. Enforcing one mode prevents accidental mis-runs.
Suggested fix
def main():
p = argparse.ArgumentParser(
@@
- p.add_argument("--demo", action="store_true", help="Run built-in demo story")
- p.add_argument("--story", type=Path, metavar="FILE", help="Path to story YAML")
- p.add_argument("--measure", type=Path, metavar="FILE", help="Measure a prose file")
+ mode = p.add_mutually_exclusive_group(required=True)
+ mode.add_argument("--demo", action="store_true", help="Run built-in demo story")
+ mode.add_argument("--story", type=Path, metavar="FILE", help="Path to story YAML")
+ mode.add_argument("--measure", type=Path, metavar="FILE", help="Measure a prose file")
@@
- else:
- p.print_help()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/scripts/story_run.py` around lines 217 - 243, The
parser currently accepts multiple mode flags and resolves them via if/elif; make
the modes mutually exclusive by creating a mutually exclusive group on the
ArgumentParser (use p.add_mutually_exclusive_group(required=True)) and move the
three mode arguments (--demo, --story, --measure) into that group instead of
calling p.add_argument on p for those flags; keep the existing argument options
(action/type/metavar/help) and leave other arguments on p as-is so only one mode
can be supplied.
Comment on lines
+119
to
+130
| def _parse_action(name: str, raw: str) -> CharacterAction: | ||
| try: | ||
| clean = raw.strip().removeprefix("```json").removeprefix("```").removesuffix("```").strip() | ||
| data = json.loads(clean) | ||
| return CharacterAction( | ||
| character=name, | ||
| action=data.get("action", raw[:200]), | ||
| internal_state=data.get("internal_state", ""), | ||
| spoken=data.get("spoken"), | ||
| ) | ||
| except json.JSONDecodeError: | ||
| return CharacterAction(character=name, action=raw[:200], internal_state="", spoken=None) |
There was a problem hiding this comment.
Handle valid non-dict JSON in _parse_action.
If the model returns valid JSON that is not an object, json.loads succeeds but data.get(...) raises and aborts execution.
Suggested fix
def _parse_action(name: str, raw: str) -> CharacterAction:
try:
clean = raw.strip().removeprefix("```json").removeprefix("```").removesuffix("```").strip()
data = json.loads(clean)
+ if not isinstance(data, dict):
+ raise ValueError("Model response JSON must be an object")
return CharacterAction(
character=name,
- action=data.get("action", raw[:200]),
- internal_state=data.get("internal_state", ""),
- spoken=data.get("spoken"),
+ action=str(data.get("action", raw[:200])),
+ internal_state=str(data.get("internal_state", "")),
+ spoken=data.get("spoken"),
)
- except json.JSONDecodeError:
+ except (json.JSONDecodeError, ValueError, TypeError):
return CharacterAction(character=name, action=raw[:200], internal_state="", spoken=None)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/story_engine/character_agent.py` around lines 119 -
130, _parse_action currently assumes json.loads returns a dict and calls
data.get(...), which crashes for valid non-dict JSON; update _parse_action to
validate that data is a dict (e.g., if not isinstance(data, dict): raise
ValueError("Model response JSON must be an object")), coerce the extracted
fields for action and internal_state to strings (e.g., str(data.get(...))) to
avoid TypeErrors, and expand the except clause to catch ValueError and TypeError
in addition to json.JSONDecodeError so the fallback
CharacterAction(character=name, action=raw[:200], internal_state="",
spoken=None) is used for non-object or otherwise invalid JSON values.
Comment on lines
+132
to
+135
| if use_ollama: | ||
| action = character_decide(char, world.describe(), [], ollama_model) | ||
| else: | ||
| action = _rule_based_decide(char, tick) |
There was a problem hiding this comment.
Guard LLM decision calls with per-tick fallback.
A successful pre-check doesn’t guarantee all later generation calls succeed. If Ollama fails mid-run, the simulation currently aborts instead of falling back.
Suggested fix
for char in characters:
if use_ollama:
- action = character_decide(char, world.describe(), [], ollama_model)
+ try:
+ action = character_decide(char, world.describe(), [], ollama_model)
+ except RuntimeError:
+ action = _rule_based_decide(char, tick)
else:
action = _rule_based_decide(char, tick)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if use_ollama: | |
| action = character_decide(char, world.describe(), [], ollama_model) | |
| else: | |
| action = _rule_based_decide(char, tick) | |
| if use_ollama: | |
| try: | |
| action = character_decide(char, world.describe(), [], ollama_model) | |
| except RuntimeError: | |
| action = _rule_based_decide(char, tick) | |
| else: | |
| action = _rule_based_decide(char, tick) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/story_engine/simulation.py` around lines 132 - 135,
Wrap the LLM call for each tick in a per-tick try/except and fallback to the
rule-based decision: when use_ollama is true and you call character_decide(char,
world.describe(), [], ollama_model), catch any exception (and treat a None/falsy
return as failure) and on failure call _rule_based_decide(char, tick) instead;
also log a warning including the exception/context so the simulation continues
rather than aborting. Ensure you reference use_ollama, character_decide,
_rule_based_decide, ollama_model, world.describe(), and tick when locating and
modifying the code.
| # Phrases that signal resolution / payoff | ||
| _PAYOFF_PATTERNS = [ | ||
| r"\bremembered\b", r"\bfinally\b", r"\bat last\b", | ||
| r"\breached for\b", r"\bpulled out\b", r"\bused\b", |
There was a problem hiding this comment.
Fix payoff phrase typo in regex.
Line 43 uses breached for; this likely should be reached for. The current pattern reduces payoff detection recall.
Proposed fix
- r"\breached for\b", r"\bpulled out\b", r"\bused\b",
+ r"\breached for\b", r"\breached for\b", r"\bpulled out\b", r"\bused\b",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/story_state/chekhov_state.py` at line 43, The regex
list contains a typo "breached for" which should be "reached for"; locate the
pattern r"\breached for\b" in chekhov_state (the payoff-detection regex list)
and replace it with r"\breached for\b" -> r"\breached for\b" corrected to
r"\breached for\b" Wait—oops. Replace "breached for" with "reached for" so the
entry becomes r"\breached for\b" corrected to r"\breached for\b" STOP
Comment on lines
+114
to
+118
| def update_relationship( | ||
| self, char_a: str, char_b: str, weight: float, scene: int | ||
| ) -> None: | ||
| edge = f"{min(char_a, char_b)}::{max(char_a, char_b)}" | ||
| self.relationship_history.setdefault(edge, []).append((scene, weight)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import yaml
payload = {"relationship_history": {"A::B": [(1, 0.7)]}}
doc = yaml.dump(payload)
print("YAML emitted:\n", doc)
try:
yaml.safe_load(doc)
print("safe_load: OK")
except Exception as e:
print("safe_load failed:", type(e).__name__, str(e))
PYRepository: Harborline-Software/flight-deck
Length of output: 348
Fix YAML state reload for relationship_history (tuples + yaml.safe_load)
update_relationship() stores (scene, weight) tuples; PyYAML emits !!python/tuple tags when dumping, and yaml.safe_load() throws ConstructorError for those tags—breaking persisted-state reuse.
Proposed fix
@@
def update_relationship(
self, char_a: str, char_b: str, weight: float, scene: int
) -> None:
edge = f"{min(char_a, char_b)}::{max(char_a, char_b)}"
- self.relationship_history.setdefault(edge, []).append((scene, weight))
+ # Use YAML-safe list entries to keep save/load symmetric with safe_load.
+ self.relationship_history.setdefault(edge, []).append([scene, weight])
@@
- state.relationship_history = data.get("relationship_history", {})
+ raw_history = data.get("relationship_history", {})
+ state.relationship_history = {
+ edge: [[int(s), float(w)] for s, w in entries]
+ for edge, entries in raw_history.items()
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/story_state/cross_scene_state.py` around lines 114 -
118, The persisted YAML breaks on Python tuples; modify update_relationship to
append a plain list instead of a tuple so yaml.safe_load won't emit
!!python/tuple tags: change the append from (scene, weight) to [scene, weight]
in update_relationship and update any code that consumes relationship_history
(iteration/unpacking) to treat entries as list pairs rather than tuples; ensure
all reads/writes of relationship_history use list pairs to maintain safe YAML
round-trips.
Comment on lines
+199
to
+203
| def load(cls, path: Path) -> CrossSceneState: | ||
| with open(path, encoding="utf-8") as fh: | ||
| data = yaml.safe_load(fh) | ||
| state = cls(story_id=data["story_id"], current_scene=data.get("current_scene", 0)) | ||
| for k, v in data.get("plant_registry", {}).items(): |
There was a problem hiding this comment.
Harden load() for empty/malformed YAML input.
If the file is empty or malformed, safe_load() can return None (or non-dict), and data["story_id"] crashes. Add a defensive shape check with a clear error.
Proposed fix
@@
def load(cls, path: Path) -> CrossSceneState:
with open(path, encoding="utf-8") as fh:
data = yaml.safe_load(fh)
+ if not isinstance(data, dict) or "story_id" not in data:
+ raise ValueError(f"Invalid state file format: {path}")
state = cls(story_id=data["story_id"], current_scene=data.get("current_scene", 0))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def load(cls, path: Path) -> CrossSceneState: | |
| with open(path, encoding="utf-8") as fh: | |
| data = yaml.safe_load(fh) | |
| state = cls(story_id=data["story_id"], current_scene=data.get("current_scene", 0)) | |
| for k, v in data.get("plant_registry", {}).items(): | |
| def load(cls, path: Path) -> CrossSceneState: | |
| with open(path, encoding="utf-8") as fh: | |
| data = yaml.safe_load(fh) | |
| if not isinstance(data, dict) or "story_id" not in data: | |
| raise ValueError(f"Invalid state file format: {path}") | |
| state = cls(story_id=data["story_id"], current_scene=data.get("current_scene", 0)) | |
| for k, v in data.get("plant_registry", {}).items(): |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/story_state/cross_scene_state.py` around lines 199 -
203, After calling yaml.safe_load in CrossSceneState.load, add a defensive shape
check that verifies data is a dict (e.g. isinstance(data, dict)) before
accessing data["story_id"]; if not, raise a clear ValueError (include the path
and a short message like "invalid or empty YAML: expected mapping with
'story_id'") so load() fails fast on empty/malformed files rather than crashing
when data["story_id"] is accessed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
icm/06_build/scaffolds/scripts/story_run.py (1)
242-242:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
--statehelp text is inaccurate.The flag is used by both
--storyand--measure, but help text documents only--measure.Suggested fix
- p.add_argument("--state", metavar="PATH", help="Load existing state for --measure") + p.add_argument("--state", metavar="PATH", help="Load existing state for --story/--measure")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icm/06_build/scaffolds/scripts/story_run.py` at line 242, The help text for the CLI argument defined by p.add_argument("--state", metavar="PATH", help="Load existing state for --measure") is inaccurate; update the help string to say the state path can be used by both --story and --measure (e.g., "Load existing state for --story or --measure") so the documentation matches actual behavior. Ensure you modify the help parameter of the p.add_argument("--state", ...) invocation to the corrected brief description.
🧹 Nitpick comments (3)
icm/02_architecture/output/arch-python-ts-contract-2026-05-26.md (3)
23-41: ⚡ Quick winAdd language specifier to the fenced code block.
The ASCII diagram should specify a language (e.g.,
textorplaintext) to satisfy markdown linters and ensure consistent rendering across tools.📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────────────────────┐ │ Layer 4: TTS Renderer (Mac Piper / winhub Kokoro+Higgs) │🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@icm/02_architecture/output/arch-python-ts-contract-2026-05-26.md` around lines 23 - 41, The fenced ASCII diagram block lacks a language tag; update the opening fence for the diagram (the triple-backtick that begins the ASCII box containing "Layer 4: TTS Renderer..." / "Layer 1: Simulation") to include a language specifier such as text or plaintext (e.g., change ``` to ```text) so markdown linters and renderers consistently treat it as plain text.
119-121: ⚡ Quick winAdd language specifier to the fenced code block.
Specify a language (e.g.,
textorplaintext) for the path example to satisfy markdown linters.📝 Proposed fix
-``` +```text <bookRoot>/.flight-deck/story/state/{story_id}.yaml</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@icm/02_architecture/output/arch-python-ts-contract-2026-05-26.mdaround
lines 119 - 121, The fenced code block containing the path
"/.flight-deck/story/state/{story_id}.yaml" should include a language
specifier to satisfy markdown linters; update the triple-backtick fence that
surrounds that path (the block currently shown as...) to use a language
token such astext orplaintext so the example becomes a labeled code
block.</details> --- `135-137`: _⚡ Quick win_ **Add language specifier to the fenced code block.** Specify a language (e.g., `text` or `typescript`) for the path example to satisfy markdown linters. <details> <summary>📝 Proposed fix</summary> ```diff -``` +```text apps/book-server/src/types/story-engine.gen.ts ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@icm/02_architecture/output/arch-python-ts-contract-2026-05-26.mdaround
lines 135 - 137, The fenced code block that shows the path
apps/book-server/src/types/story-engine.gen.ts lacks a language specifier;
update that markdown block to include a language (for example changetotext ortypescript) so the linter recognizes the code fence and the path example is properly highlighted. Locate the code block in arch-python-ts-contract-2026-05-26.md and replace the opening backticks with a language-qualified fence (e.g.,text).</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@icm/02_architecture/output/arch-python-ts-contract-2026-05-26.md:
- Around line 123-129: Clarify the concurrency guarantee around the exception
for book-server appends: update the Ownership rules to state exactly how appends
to established_details[] are made safe relative to the Python story-engine
single-writer per story_id—either (A) require book-server to reload the latest
YAML/state for the story_id and obtain a write-consent token before appending,
(B) define established_details[] as an append-only log file that book-server
writes to and that story-engine merges on next read, or (C) move
established_details[] to a TypeScript-owned artifact and make Python read-only
for that artifact; pick one option, describe the required protocol
(reload/merge/ownership), and reference the Python story-engine, TypeScript
book-server, established_details[], and story_id symbols in the updated text so
the behavior is unambiguous.In
@icm/06_build/scaffolds/scripts/story_run.py:
- Around line 164-167: The current logic in cmd_run (story_run.py) silently
creates a new CrossSceneState when args.state is given but the path does not
exist; change this to fail fast: if args.state is provided and
Path(args.state).exists() then load via CrossSceneState.load(Path(args.state)),
otherwise raise a clear error/exit (e.g., raise SystemExit or call
parser.error/print+sys.exit) indicating the provided --state path is missing so
the operator can correct it; keep the existing behavior that only when --state
is omitted you call CrossSceneState.new(story_id).
Outside diff comments:
In@icm/06_build/scaffolds/scripts/story_run.py:
- Line 242: The help text for the CLI argument defined by
p.add_argument("--state", metavar="PATH", help="Load existing state for
--measure") is inaccurate; update the help string to say the state path can be
used by both --story and --measure (e.g., "Load existing state for --story or
--measure") so the documentation matches actual behavior. Ensure you modify the
help parameter of the p.add_argument("--state", ...) invocation to the corrected
brief description.
Nitpick comments:
In@icm/02_architecture/output/arch-python-ts-contract-2026-05-26.md:
- Around line 23-41: The fenced ASCII diagram block lacks a language tag; update
the opening fence for the diagram (the triple-backtick that begins the ASCII box
containing "Layer 4: TTS Renderer..." / "Layer 1: Simulation") to include a
language specifier such as text or plaintext (e.g., changetotext) so
markdown linters and renderers consistently treat it as plain text.- Around line 119-121: The fenced code block containing the path
"/.flight-deck/story/state/{story_id}.yaml" should include a language
specifier to satisfy markdown linters; update the triple-backtick fence that
surrounds that path (the block currently shown as...) to use a language
token such astext orplaintext so the example becomes a labeled code
block.- Around line 135-137: The fenced code block that shows the path
apps/book-server/src/types/story-engine.gen.ts lacks a language specifier;
update that markdown block to include a language (for example changetotext ortypescript) so the linter recognizes the code fence and the path example is properly highlighted. Locate the code block in arch-python-ts-contract-2026-05-26.md and replace the opening backticks with a language-qualified fence (e.g.,text).</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `4a8e40a2-8f94-4589-846a-8e37daae6ba0` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 200792648076a509cf826a5a2810d7719a78e6dd and adeb519df485904e7bdcab2a6c5ce7563b6af6cf. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `icm/02_architecture/output/arch-python-ts-contract-2026-05-26.md` * `icm/06_build/scaffolds/scripts/story_run.py` * `icm/06_build/scaffolds/story_engine/simulation.py` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+164
to
+167
| if args.state and Path(args.state).exists(): | ||
| state = CrossSceneState.load(Path(args.state)) | ||
| else: | ||
| state = CrossSceneState.new(story_id) |
There was a problem hiding this comment.
Fail fast when --state is provided but missing in cmd_run.
Silently creating a new state on a bad --state path can fork story continuity and hide operator error.
Suggested fix
- if args.state and Path(args.state).exists():
- state = CrossSceneState.load(Path(args.state))
- else:
- state = CrossSceneState.new(story_id)
+ if args.state:
+ state_path = Path(args.state)
+ if not state_path.exists():
+ raise FileNotFoundError(f"--state file not found: {state_path}")
+ state = CrossSceneState.load(state_path)
+ else:
+ state = CrossSceneState.new(story_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/06_build/scaffolds/scripts/story_run.py` around lines 164 - 167, The
current logic in cmd_run (story_run.py) silently creates a new CrossSceneState
when args.state is given but the path does not exist; change this to fail fast:
if args.state is provided and Path(args.state).exists() then load via
CrossSceneState.load(Path(args.state)), otherwise raise a clear error/exit
(e.g., raise SystemExit or call parser.error/print+sys.exit) indicating the
provided --state path is missing so the operator can correct it; keep the
existing behavior that only when --state is omitted you call
CrossSceneState.new(story_id).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@icm/STATUS.md`:
- Around line 50-55: Add a language tag to the fenced code block that begins
with "Layer 1 Simulation FourQuadrant agents × tick-based world
(Python)" so markdownlint MD040 is satisfied; replace the opening ``` with
```text (or another appropriate language) in the STATUS.md fenced block
containing the "Layer 1 ... Layer 4 ..." lines so the block becomes ```text ...
``` and CI will no longer flag the missing language.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+50
to
+55
| ``` | ||
| Layer 1 Simulation FourQuadrant agents × tick-based world (Python) | ||
| Layer 2 Plot Compiler SimulationTrace → PlotPlan (Python; NOT YET BUILT) | ||
| Layer 3 Prose Renderer Beat → prose paragraphs (TypeScript/Node; NOT YET BUILT) | ||
| Layer 4 TTS Renderer AudioScript → audio (Mac: Piper / winhub: Kokoro+Higgs) | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block to satisfy markdownlint.
Line 50 opens a fenced code block without a language, which triggers MD040 in CI.
Proposed fix
-```
+```text
Layer 1 Simulation FourQuadrant agents × tick-based world (Python)
Layer 2 Plot Compiler SimulationTrace → PlotPlan (Python; NOT YET BUILT)
Layer 3 Prose Renderer Beat → prose paragraphs (TypeScript/Node; NOT YET BUILT)
Layer 4 TTS Renderer AudioScript → audio (Mac: Piper / winhub: Kokoro+Higgs)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 50-50: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@icm/STATUS.md` around lines 50 - 55, Add a language tag to the fenced code
block that begins with "Layer 1 Simulation FourQuadrant agents ×
tick-based world (Python)" so markdownlint MD040 is satisfied; replace the
opening ``` with ```text (or another appropriate language) in the STATUS.md
fenced block containing the "Layer 1 ... Layer 4 ..." lines so the block becomes
```text ... ``` and CI will no longer flag the missing language.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if story_id in _story_cache: | ||
| return _story_cache[story_id][1] | ||
| yaml_path = _STATE_DIR / f"{story_id}.yaml" | ||
| if yaml_path.exists(): |
| """ | ||
| if req.story_id not in _story_cache: | ||
| yaml_path = _STATE_DIR / f"{req.story_id}.yaml" | ||
| if yaml_path.exists(): |
| read-only per the arch contract). | ||
| """ | ||
| yaml_path = _STATE_DIR / f"{story_id}.yaml" | ||
| if not yaml_path.exists(): |
|
|
||
| @classmethod | ||
| def load(cls, path: Path) -> CrossSceneState: | ||
| with open(path, encoding="utf-8") as fh: |
| # ------------------------------------------------------------------ | ||
|
|
||
| def save(self, path: Path) -> None: | ||
| path.parent.mkdir(parents=True, exist_ok=True) |
| }, | ||
| "established_details": self.established_details, | ||
| } | ||
| with open(path, "w", encoding="utf-8") as fh: |
Establishes Flight Deck ICM pipeline. Four stages authored for the story creation module: intake, discovery (fast-tracked), architecture, and implementation plan. Covers: Narrative Graph substrate (packages/story-graph), story structure module (packages/story-structure, quiz-first UX, 8 templates, MRU beats), character creation (desire/fear/wound/need + voice fingerprint + arc map), dual-track timeline, storyboard (beat→panel scaffold + shot vocabulary), and AI pipeline integrations (editorial chat primer + prose telemetry structure_context slot). Six-phase build sequence with acceptance criteria. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four stage deliverables for the story creation module: - 00_intake: scope, pipeline variant, fast-track rationale - 01_discovery: Shipyard block reuse map, on-disk format, integration points - 02_architecture: three-layer model, package API surfaces, cross-feature contracts - 05_implementation-plan: six-phase build sequence with file targets + acceptance criteria .gitignore: add !icm/**/output/ exemption (build-artifact rule was blocking ICM docs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds writer-onboarding-session-2026-05-26.md as a Phase 0 addendum to the story creation impl plan. Fills three gaps identified in PAO verification pass (scenario: "I have an idea, walk me through creating it"): - No logline/premise capture step before the quiz - Quiz question phrasing unspecified (enums only, no UX copy) - AI enters at Phase 6 (writing phase), not at idea discovery Deliverables: - 10-exchange onboarding session flow (idea → premise → quiz → 3 anchor beats → character stubs → Beat List landing) - Plain-English phrasing for all 3 quiz questions with enum mappings - Full 8-cell template routing table (all combinations covered) - OnboardingSession.tsx + useOnboardingSession.ts + onboarding.ts targets - 3 new book-server routes (GET/PUT/POST /story/onboarding) - Partial session recovery spec (persist to quiz-answers.yaml) - Phase 0 gates as P2-0 prefix; ships before Phase 6 (no LLM required) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rgy function
Working Python prototype for the Arch 3 Layered Story Compiler:
story_engine/
character_agent.py — FourQuadrant profile + Ollama-backed decision fn
Falls back to rule-based without Ollama
simulation.py — WorldState + SimulationTrace + run_simulation()
Tick-based; each character acts per tick; significance scoring
story_state/
cross_scene_state.py — YAML-persisted state across full story arc:
plant_registry, knowledge_graph, relationship_history,
subplot_threads, established_details
chekhov_state.py — Chekhov's Gun detector: detect_plants() + run_detector()
Regex heuristics; upgrades to embeddings when available
prose_telemetry/
embeddings.py — Local embedding wrapper (sentence-transformers primary,
BOW fallback — no install required)
energy_function.py — Narrative energy function: weighted detector verdict
aggregation per template (8 templates, all defined)
Act-3 amplification for structural detectors
scripts/story_run.py — CLI: --demo | --story FILE | --measure FILE
Fully runnable without Ollama or sentence-transformers
Setup:
pip install pyyaml sentence-transformers
brew install ollama && ollama serve && ollama pull mistral
Smoke tested: all imports + rule-based sim + chekhov + energy + BOW embeddings green
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chitecture contract UPF C4 fix: simulation.py was passing known_facts=[] to every character_decide() call, hardcoded. Characters had no awareness of what they witnessed, making dramatic irony structurally impossible. Now CrossSceneState is passed into run_simulation(); characters pull their beliefs before each decision, and high-significance events (sig >= 0.6) are written to all witnesses' knowledge graphs. UPF C1 fix: adds arch-python-ts-contract-2026-05-26.md — the missing contract between the Python simulation pipeline (Layers 1+2) and the TypeScript book-server (Layers 3+4). Defines the HTTP API seam on port 7740, state ownership rules, and the TypeScript proxy pattern. Unblocks Layer 2 plot-compiler work. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…+ pick-up guide Captures: what scaffolds are built and runnable, architecture decisions locked (4-layer system, port 7740 seam, 8-template routing table, tech stack), what remains unbuilt (plot compiler, prose renderer, missing detectors), and the research phase priorities (event-type frequency table from Gutenberg corpus, energy weight calibration from CMU Book Summary). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mplementation map 28 formulas across 10 categories derived from screenwriting craft (McKee, Snyder, Truby, Vogler, Coyne), Aristotle, games research (36-game corpus), and CIC design sprint. Organized by: master equity calibration formula, tension/stakes, character/ wound, dramatic irony, scene structure, emotional payoff, antagonist design, pacing gradient, setting/cognitive load. Key additions beyond prior game-research evaluation: - Formula 0 (Equity Calibration): master formula unifying all value delivery types; each emotional value type (good over evil, perseverance, love, etc.) is a specific instance of calibration restoration - Formula 0a (Trait Compounding): unearned positive traits multiply disruption desire; explains Mary Sue failure - Formula 0b (Earned vs Unearned Legitimacy): traits earned through the wound have maximum audience legitimacy - Formula 0c (Gifted Underdog): canonical engagement configuration - Formula 26 (Tolerance Curve): continuous pacing gradient replacing hard act-3 threshold in energy_function.py Includes implementation map: new FourQuadrant fields, SimulationEvent fields, detector priority list, 12 plot compiler constraints, and corpus validation targets. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… + PART XI Incorporates three-tier vice taxonomy (7 root vices, 10 behavioral vices, Aristotelian virtue-extreme spectrum) as a first-class story design primitive. - character_agent.py: ViceProfile dataclass; new FourQuadrant fields (misbelief, highest_value, failure_signature, action_budget, vice_profile); failed flag on CharacterAction; DECISION_SYSTEM_PROMPT now injects vice section under pressure - story_run.py: DEMO characters promoted to full FourQuadrant objects with vice profiles (Maren/Sloth, Edvard/Pride, Lise/Sloth); _build_character() helper for YAML loading with nested ViceProfile; YAML format docstring updated - story-formulas-2026-05-26.md: PART XI Vice Architecture — root vice story functions, behavioral-vice surface descriptions, virtue-extreme pairs, character design rule, arc design rule, system implementation map Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…frequency table Survey of fiction-corpus datasets and tools for the event-type-by-position frequency data that grounds the Layer-2 Plot Compiler. Finds TRIPOD as closest prior art, recommends a CMU-Book-Summaries LLM-annotation pipeline over the BookNLP+Gutenberg path, and proposes the DuckDB event_type_frequencies schema with corpus-bias flagging for non-Western templates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
StoryEra models the cognitive load a setting imposes on the audience. cognitive_tax() returns 0.0-1.0 fraction of scene text that consists of invented-rule vocabulary or long jargon words (>12 chars), letting the prose renderer flag when a scene exceeds the era's cognitive budget. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… catalogue Machine-readable beat constraint model bridging the 28 story formulas and the plot compiler. Covers 8 templates (save-the-cat, hero-journey, three-act, seven-point, romance, tragedy, mystery, kishotenketsu) with 62 total beats. Each constraint encodes equity_state, vice_surface_required, knowledge_asymmetry, min_significance, narrative_function, and entry/exit conditions. Includes get_beats() and beat_at_position() helper functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Output contract of the Layer 2 plot compiler. PlotPlan maps simulation trace event indices to template beats with constraint violation tracking, equity deltas, and per-character knowledge state snapshots. Serialises to/from YAML via dataclasses.asdict() with full round-trip fidelity. Allows the TypeScript prose renderer to be built independently of the diffusion-based plot compiler. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HTTP API server wrapping the Python simulation pipeline per the
arch-python-ts-contract decision. Implements all Phase 0 routes:
GET /health, POST /simulate, POST /detect/chekhov, POST /energy,
POST /plot/assign (stub), GET /state/{story_id}.
The /plot/assign route is a proportional stub that distributes events
evenly across template beats; Phase 2 replaces it with diffusion-based
assignment. Pydantic v2 models validate all request/response shapes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Required by story_server.py (Phase 0 story-engine HTTP API server). Existing deps (pyyaml, sentence-transformers, spacy) are preserved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…energy_from_trace Nine detector modules under prose_telemetry/detectors/: - mru_completeness: stimulus/response/action paragraph scoring - arc_engagement: Jaccard vocabulary shift + belief contradictions - emotional_resolution: keyword density in final 20% + plant payoff ratio - pacing: act-2 polarity + climax reversal via significance-based equity - arc_shape: equity-thirds vs template key-point checks (8 templates) - dramatic_irony: knowledge asymmetry across character knowledge graphs - calibration_alignment: protagonist equity zones 1/2/3 vs template - cognitive_load: jargon ratio vs StoryEra cognitive budget - voice_consistency: pairwise embedding similarity of spoken lines Updated energy_function.py with compute_energy_from_trace() that wires all 10 detectors (9 new + Chekhov) into the existing compute_energy() call. Design decisions: SimulationEvent has no failed field; pacing/calibration/ arc_shape use significance + story-position-based sign to approximate equity. StoryEra imported under TYPE_CHECKING guard to avoid Phase 0 circular dep. Embeddings module uses embed()/cosine_similarity() (actual API names). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gress Updates the pick-up doc with: - Phase 0: StoryEra, beat constraints (62), PlotPlan model, FastAPI server (port 7740) - Phase 1: 9 prose telemetry detectors + compute_energy_from_trace() wiring - Research: ONR corpus research complete — CMU summaries + Ollama pipeline confirmed - Layer status table showing Layer 1 complete, Layer 2 building, Layer 3 next - Updated "How to pick up" section reflecting runnable server Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…signment algorithm) Adds story_engine/plot_compiler.py implementing the Layer 2 plot compiler. Adds CompilerResult dataclass to plot_plan.py (plan + scores + diagnostics). Wires real compiler into story_server.py /plot/assign endpoint, replacing the Phase 1 stub that distributed events evenly. Algorithm: normalize event positions, score each (event, beat) pair via a composite of positional proximity, significance, equity direction, vice surface, and corpus prior (DuckDB when available, uniform 0.5 fallback), then greedy assignment by beat_position order, fill-in pass for unassigned events, and up to 50 iterations of local search improvement via primary-event pairwise swaps. Smoke-tested with 8-tick save-the-cat trace: 15/15 beats assigned, score 0.733, 1/16 events unassigned (all primary slots filled by greedy pass). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds story_db/ package with three files:
- __init__.py — package marker with usage docs
- schema.py — create_schema(), seed_taxonomy(), seed_template_beats()
- corpus_pipeline.py — load_tripod() + load_cmu_summaries() + CLI
Schema implements four tables per ONR research onr-corpus-research-2026-05-26:
event_type_frequencies — P(event_type | position_bin, genre, corpus)
event_type_taxonomy — fused structural (TRIPOD 5 TPs) + semantic label set
template_beats — seeded from ALL_BEAT_CONSTRAINTS (62 beats, 8 templates)
turning_point_positions — TRIPOD gold turning-point positions
Corpus pipeline supports two ingest paths:
TRIPOD CSV — load_tripod() seeds event_type_frequencies + turning_point_positions
CMU summaries — load_cmu_summaries() uses zero-shot Ollama annotation per
ONR recommendation (LLM-over-summaries, not BookNLP-over-fulltext)
duckdb is an optional dependency; schema.py raises ImportError cleanly if absent.
Done-condition verified: create_schema + seed_taxonomy + seed_template_beats
round-trip without error.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add packages/story-graph/ TypeScript package with: - story-types.ts: full interface mirror of Python dataclasses (PlotPlan, BeatAssignment, SimulationEvent, StoryContext, CharacterProfile, RenderedBeat, AudioScript, DialogueSegment, HTTP request/response shapes) - story-engine-client.ts: StoryEngineClient wrapping /api/story/v1/ routes on the Python FastAPI server (port 7740); StoryEngineError with actionable startup hint; configurable timeout; retry support for health check - prose-renderer.ts: renderBeat() and renderAllBeats() calling Ollama locally; MRU-structured audio-first system prompt; DIALOGUE_START/END parsing; graceful placeholder degradation when Ollama is unavailable - index.ts: re-exports all types, client, and renderer - tsconfig.json: NodeNext module resolution + ES2022 + DOM lib (fetch/AbortController) - package.json: name @galley/story-graph, private, type module Also add services/book-server/lib/story/ JS implementations (plain ESM, no TypeScript) for the book-server runtime which is pure JavaScript. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wire story engine into services/book-server/server.js: - Import StoryEngineClient + renderBeat from lib/story/ - GET /api/story/health: proxy health check to Python server with retry; 503 response includes startup hint on connection failure - POST /api/story/generate: full pipeline (simulate + assignPlot + renderBeat in parallel for all beats); returns rendered_beats + constraint_score - POST /api/story/render-beat: single beat render for incremental UI updates Architecture: Python story-engine on port 7740 owns Layers 1 and 2; TypeScript book-server owns Layer 3 (prose rendering via Ollama) and Layer 4 routing (TTS via winhub port 8881 or Mac Piper). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…aph + Workbench) Three new components wired into the flight-deck web app for the story creation workbench. Adds @galley/story-graph workspace dependency to apps/web. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… reconciliation Add POST /api/story/render-tts and GET /api/story/voices to book-server (server.js). Routes proxy to the fleet winhub TTS service on port 8881 using the OpenAI-compatible speech API. Kokoro (fast) and Higgs (quality) selected via the quality param; narrator defaults to af_sky; character voices auto-assigned by index cycling through six kokoro voices. Returns base64-encoded WAV segments per audio region. Add story_seed support to the simulate endpoint in story_server.py. A seed is generated when none is supplied, written to the response, and passed back via the book-server generate route so the frontend can display and re-pin it. Seed controls simulation structure (random.seed); LLM output is a separate knob (Ollama temperature). Add POST /api/story/v1/edit-beat to story_server.py. Diffs original and edited prose at sentence granularity; extracts proper nouns and factual assertions from added sentences; calls state.learn() at strength 0.8 with source author_edit; flags plants whose description text was removed; saves updated CrossSceneState YAML; returns facts_extracted and contradictions_flagged. Wire a Listen button and seed display into StoryWorkbench.tsx. Listen calls render-tts and plays the narrator segment via Web Audio. Seed bar shows the last seed with a pin/unpin toggle that feeds story_seed into the next generate call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add icm/06_build/scaffolds/scripts/e2e_test.py. Five sequential tests cover all imports, rule-based simulation (no Ollama required), plot compiler, all nine detectors via compute_energy_from_trace, and PlotPlan YAML round-trip. Each test is isolated with its own try/except so failures are reported individually rather than aborting the full suite. Exits 0 on all pass, 1 if any fail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Phases 0–5 complete. Updates: - Layer 2 (plot compiler + DuckDB): complete - Layer 3 (TypeScript prose renderer + story-graph package): complete - Layer 4 (TTS wiring at winhub port 8881): complete - Writer UI (/story workbench): complete - Seed pinning + author-edit reconciliation: complete - E2E smoke test: 5/5 pass (exit 0) - "How to run" section replaces "What is not built" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four-part upgrade to the story engine data model: 1a. WorldRule PbtA three-tier costs: replace single story_cost with story_cost_hit / story_cost_partial / story_cost_miss. All 21 template WorldRules authored with distinct tier strings. Legacy YAML files with the old story_cost key now raise ValueError on load instead of silently dropping data. _universe_from_dict falls back gracefully for generated universes that only have the LLM's story_cost key. 1b. Faction Dungeon World doom clock: add grim_portents (3-item ordered escalation list) and doom_clock (0–6 int) to Faction. Universe gains advance_doom_clock() which returns True when catastrophe fires. All 21 template Factions authored with grim portents specific to their universe's pressure dynamics. 1c. Story Dramatica throughlines: add protagonist_spine, ic_character_key, relationship_throughline to Story. add_story() accepts these as optional kwargs. Old YAML loads cleanly (fields default to ""). 1d. BeatConstraint atmospheric grounding: add desired_atmosphere field. Authored on 14 specific beat slugs across save-the-cat, hero-journey, tragedy, and mystery templates. compile_plot() will log this in diagnostics (Wave 2 wires the scoring). Test suite: 90/90 passed (all non-LLM tests pass in <5s; explore tests require Ollama and take ~10min when model is cold). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mar, plants Wave 2: Ironsworn oracle tiers on explore() (complicates/clarifies/contradicts + threads); thematic beat alignment (0.05x overlap additive bonus); failure signature staging (0.10 weight replaces corpus_prior on trigger beats). Wave 3: Propp function tagging (14 canonical slugs, 8 beat mappings); Tracery flavor grammar on all 7 universe templates; plant/payoff coordination (plants_to_introduce/pay_off + _plant_audit() post-compilation diagnostics). All 87 non-LLM tests pass. New test_beat_constraints.py: 20 pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Chains all four story engine layers into a single runnable script:
Layer 1: run_simulation() → SimulationTrace
Layer 2: compile_plot() → PlotPlan with beat assignments
Layer 3: Ollama prose render (MRU-structured audio-first paragraphs per beat)
Layer 4: TTS synthesis via Kokoro/Higgs at port 8881 → MP3 per beat
Output lands in ~/Library/CloudStorage/Dropbox/flight-deck-stories/<story_id>/
with prose.md, numbered MP3s, and manifest.json. Both LLM and TTS layers
are optional (graceful fallback when Ollama or winhub are unavailable).
Usage: python scripts/story_export.py --demo
python scripts/story_export.py --demo --no-tts
python scripts/story_export.py --story my_story.yaml --out ~/Desktop
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Maps each beat's desired_atmosphere to TTS acoustic parameters (speed, exaggeration, cfg_weight, temperature) so every beat renders with a distinct acoustic character — desperate beats hit exaggeration=0.9, quiet beats drop to 0.2, triumphant beats push speed to 1.05. Addresses the compressed-data-structures research finding that text-plus-markup's main weakness is weak timbre control. Uses Higgs/Kokoro API knobs directly rather than adding model complexity. Also adds source:"simulation_trace" + simulation metadata block to manifest, marking the YAML event log as the canonical archival representation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds Claude API (Anthropic) as an alternative prose backend alongside Ollama. --llm-backend claude uses ANTHROPIC_API_KEY from environment. --claude-model selects the model (default: claude-haiku-4-5-20251001). Falls through to Ollama then to deterministic fallback if unavailable. Ollama remains the default for all production use. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ctwoodwa
force-pushed
the
docs/story-creation-icm
branch
from
8ab8377 to
9cc2052
Compare
…s + from-prose mode - story_evaluate.py: new audience received-value scorer (arc alignment, structural completeness, acoustic differentiation) - story_export.py: add propp_functions/thematic_resonance/narrative_function per beat in manifest output - story_export.py: add --from-prose mode (re-synthesize audio from existing prose.md + manifest without re-running simulation); retry-with-backoff for TTS service recovery - story_evaluate.py: fix Propp coverage to read from manifest beats field rather than diagnostics text; keep legacy diagnostics fallback coastal-storm sample: received value 0.714 (B), structural completeness 0.940, arc alignment Pearson r=+0.405. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
tts_voice_test.py fetches available voices from the Kokoro service, filters by accent prefix (af/am/bf/bm/jf/jm/zf/zm) or explicit list, generates audio per voice using the pangram fixture, and writes report.json + summary.txt to a timestamped output dir. tts_pangrams.txt holds 7 standard pangrams covering full alphabet coverage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e_text() Extends evaluate_text() with four new pure-Python scoring layers beyond VADER arc alignment: B - Emotional Range: max-min VADER + reversal count (0.20 weight) C - Pacing Variance: sentence-length CV across beat windows (0.20) D - Resolution Completeness: finale vs opening keyword density (0.15) E - Lexical Diversity: MATTR over 100-word windows (0.15) Composite = 0.30A + 0.20B + 0.20C + 0.15D + 0.15E. Benchmarked against 14 public domain novels: coastal-storm (generated) scores 0.791, between Great Gatsby (0.828) and Wuthering Heights (0.702). Lexical diversity (0.535 vs Gatsby's 0.720) is the identified improvement target. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ve richness, CLI extensions Completes the coastal-storm imagineering pipeline and adds narrative richness improvements derived from Dungeon World, PbtA, Ironsworn, Dramatica, and Propp across the story engine data model. Assembly scripts (moved from Dropbox output dir into scaffold): - scripts/coastal_storm_storyboard.py — character-anchored storyboard with CLIP-trimmed prompts (≤42 tokens) - scripts/make_animatic.py — audio-synced MP4 from 15 PNGs + MP3s - scripts/make_audiobook.py — M4B with chapter markers + embedded cover art - scripts/make_comic.py — professional 3×5 storyboard PDF/PNG (16:9 panels, shot badges, prose action lines) - story_engine/screenplay.py — Fountain/text/md screenplay export Story engine extensions: - story_run.py: --seed, --export-state, --export-trace, --import-state flags - story_server.py: POST /api/story/v1/continue endpoint (seed-based branching) - storyboard.py: 3-backend support (ComfyUI/A1111/diffusers), --beat single-frame regen, character_anchor field Narrative richness (Waves 1–3 — data model + generation): - WorldRule: PbtA three-tier costs (hit/partial/miss) with legacy-field guard - Faction: doom_clock (0–6) + grim_portents (3 per faction, all 7 templates) - Story: protagonist_spine, ic_character_key, relationship_throughline throughlines - BeatConstraint: desired_atmosphere, thematic_resonance, failure_signature_trigger, propp_functions, plants_to_introduce, plants_to_pay_off - Universe: flavor_grammar + _pick() Tracery-style substitution (all 7 templates) - plot_compiler: thematic bonus (0.05×), failure-sig scoring (0.10×), _plant_audit() diagnostics - universe_generator: explore() oracle tiers (complicates/clarifies/contradicts) + threads - test_universe.py: doom clock, grim portents, oracle keys, legacy YAML guard tests - test_beat_constraints.py: Propp functions, plant audit, flavor grammar tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| """ | ||
| # ---- locate story state ---- | ||
| yaml_path = _STATE_DIR / f"{req.story_id}.yaml" | ||
| if not yaml_path.exists() and req.story_id not in _story_cache: |
| # ---- load state ---- | ||
| if req.story_id in _story_cache: | ||
| trace, state = _story_cache[req.story_id] | ||
| elif yaml_path.exists(): |
story_id from API request bodies was used directly in Path expressions.
An attacker-controlled value like '../../etc/passwd' could escape the
state directory.
Fixes:
- Added _safe_state_path(story_id) in story_server.py: rejects any
story_id not matching ^[a-zA-Z0-9][a-zA-Z0-9_\-]{0,127}$, then
resolves the path and asserts it stays within _STATE_DIR. Raises
HTTP 400 on violation.
- Replaced all five bare (_STATE_DIR / f"{story_id}.yaml") path
constructions with _safe_state_path() calls.
- cross_scene_state.py save() and load(): call Path.resolve() before
any I/O so the path is canonical before it reaches open().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| """Return the YAML path for story_id after validating it contains no traversal sequences.""" | ||
| if not _SAFE_ID_RE.match(story_id): | ||
| raise HTTPException(status_code=400, detail=f"Invalid story_id: {story_id!r}") | ||
| resolved = (_STATE_DIR / f"{story_id}.yaml").resolve() |
| # ------------------------------------------------------------------ | ||
|
|
||
| def save(self, path: Path) -> None: | ||
| path = Path(path).resolve() |
|
|
||
| @classmethod | ||
| def load(cls, path: Path) -> CrossSceneState: | ||
| path = Path(path).resolve() |
…_safe_state_path CodeQL py/path-injection still flagged 11 sinks because re.match with a $-anchored pattern is not a recognized barrier (Python $ matches before a trailing newline). Switch the story_id guard to re.fullmatch with \Z, and route the EditBeat persist path through _safe_state_path instead of building it inline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tures Add a third prose-rendering backend to story_export.py that shells out to the local claude CLI as a headless agent, supporting configurable --model and --effort. Runs under the scaffold's Python 3.9 (the Python Agent SDK requires 3.10), uses the existing OAuth session headless, and inherits ANTHROPIC_API_KEY from env when set. Thread --template through compile_plot and read it from a template field in the story YAML, and add an --effort flag. Add eight story fixtures under stories/, one per beat template (save-the-cat, hero-journey, mystery, seven-point, three-act, romance, tragedy, kishotenketsu), each with a fitting premise and small cast. Verified end to end on claude-opus-4-8: all 62 beats across the 8 templates render with full beat-fill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ection Add the cryo-wake story seed plus four more scaffolds (09-12); add a template field to the existing 01-08 story fixtures. Add storyboard, verify, and reorganize scripts for the per-story source+publish pipeline. Add a --model option to sd_local.py so anime checkpoints are selectable at render time (defaults to the existing SD 1.5 path). Expand story_export and story_evaluate; minor coastal_storm_storyboard adjustments. Add the animation-shorts scene-planning methodology and implementation-plan docs. gitignore the external datasets corpus and the .flight-deck runtime state (SQLite corpus cache + generated state) so they are not committed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the promptgen study driver: holds all image knobs fixed and sweeps one knob, rendering one frame per value into a reviewable study folder (study.json, study.md, frames/, contact-sheet.png). Studies are partitioned by generation model first, since the checkpoint sets the quality ceiling and cross-model frames are not comparable. Engine layer is pure logic (no I/O) so a future web UI can wrap it. Per-frame safety negatives are protected and never trimmed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Optional ref-image conditioning via h94/IP-Adapter for identity-stable renders across a character's frames. Attention slicing is skipped when IP-Adapter is active to avoid overwriting its attn processors. Defaults preserve the existing no-reference render path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add render_queue.py — drives plan_study/run_study over a JSON manifest, persisting id-to-dir state so a dying process resumes without redoing finished frames or duplicating history records. State, log, and a Dropbox-visible status.md live under the _studies tree for SSH-free review. study.py gains frame-level resume: run_study skips frames whose PNG already exists (no re-render, no duplicate history append), plan_study accepts an out_dir override to pin the dir and skip the NN bump on resume, and the contact sheet rebuilds whenever any frame is present. render-queue.json seeds 4 cryo-wake Akira studies (style, pose, outfit, fitness; 16 frames) with fixed seeds for stable filenames across restarts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add generic-studies.json — 7 reference studies (47 frames) for the _studies shared tree, each sweeping one knob across its full vocabulary on a canonical female superhero base (adult, slender-graceful, superhero-suit, studio, seinen). Categories: styles, poses, outfits, fitness, backgrounds, characters. Pose and background values enumerated explicitly since both block 'all' expansion. Fixed seeds for restartable, reproducible filenames. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a "none" value to the count knob that renders empty background plates with no figure in frame. _build_positive emits an environment concept-art prompt (no subject, no pose); _build_negative swaps the character-specific safety groups for explicit character-exclusion terms; compose() suppresses the figure-only warnings. count 1/2/group output is byte-identical (regression-guarded). Adds a backgrounds-empty-plates study to the shared-library generic queue (5 frames, all backgrounds, seinen). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full Flight Deck story creation ICM — Stages 00/01/02/05/06, plus narrative richness Wave 1–3 on top of the completed system.
What is included
ICM documents (Stages 00–05)
icm/00_intake/— intake classification + scopeicm/01_discovery/— ONR corpus research (TRIPOD / CMU Book Summaries / Ollama pipeline)icm/02_architecture/— four-layer SimCity-for-prose architecture + Python↔TypeScript contracticm/05_implementation-plan/— phased impl plan + writer onboarding sessionPython scaffolds (Stage 06 —
icm/06_build/scaffolds/)/storyNarrative richness — Wave 1 (data model enrichment)
WorldRule: 3-tier PbtA costs (story_cost_hit/story_cost_partial/story_cost_miss); legacy YAML guard raisesValueErrorFaction: Dungeon World doom clock (0–6) +grim_portentsescalation sequence; all 21 template factions authoredStory: Dramatica throughlines (protagonist_spine,ic_character_key,relationship_throughline)BeatConstraint:desired_atmosphereon 14 structurally critical beatsNarrative richness — Wave 2 (generation improvements)
explore(): Ironsworn oracle tiers — each result carriesoracle(complicates/clarifies/contradicts) +threadslist;universe.exploredentries now dicts; backward compat for old YAML strings_composite_score(): additive thematic alignment bonus (0.05 × word-overlap between universe.themes and beat.thematic_resonance); 6 beats annotatedcorpus_priorwith protagonist self-sabotage keyword scoringNarrative richness — Wave 3 (advanced structural)
compile_plot()flavor_grammaron all 7 universe templates +_pick()helper;UniverseYAML round-trip safeplants_to_introduce/plants_to_pay_offon 5 beats;_plant_audit()appends[plant-audit]warnings toCompilerResult.diagnosticsTests
tests/test_universe.py: 90/90 pass (87 fast + 3 Ollama; Ollama tests deselected during dev)tests/test_beat_constraints.py: 20/20 pass (new — Wave 2+3 field coverage)tests/test_e2e.py: all passTest plan
cd icm/06_build/scaffolds && python3 -m pytest tests/ -v -k "not (test_explore_faction_returns_dict or test_explore_marks_aspect_explored or test_explore_location_returns_dict or test_explore_history_returns_dict)"— expect greenpython scripts/story_run.py --demo— expect full pipeline trace outputpython scripts/e2e_test.py— all 5 stages pass without Ollama🤖 Generated with Claude Code