Optimize Tiny Stories end-to-end target-mode flow - #21
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c08ab2754
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mentioned_entities = ( | ||
| guide_entities | ||
| if len(guide_entities) >= 3 | ||
| else _dedupe_preserving_order([*guide_entities, *_extract_entities(clean_seed)]) |
There was a problem hiding this comment.
Merge seed cast even with three guide entities
When guide_context.cast_or_factions already has three entries, this branch stops reading entities from clean_seed entirely. In the guided flow the context extractor often stores role-level terms such as producer, sponsor representative, and backup dancer, while the seed may contain the actual names (for example Cast: Alice the producer, Bob the sponsor representative, Cara the backup dancer). The resulting Brief and fallback opening drop the user-supplied names even though they are still present in the accepted seed; merging/deduping seed entities after guide entities would preserve those facts.
Useful? React with 👍 / 👎.
| and not ( | ||
| brief.player_role | ||
| and entity.display_name.strip().casefold() == brief.player_role.strip().casefold() | ||
| ) |
There was a problem hiding this comment.
Keep excluding the player role in fallback cast
This filter is undone as soon as fewer than three non-player entities remain: the fallback block below rebuilds primary_entities from primary plus secondary entities without applying the same brief.player_role exclusion. For guided briefs with a player role and only two other parties, a live-opening failure will still stage the player as an NPC/cast member while also offering them as the player role, which is the incoherence this new filter is trying to avoid.
Useful? React with 👍 / 👎.
| if len(participants) >= 2: | ||
| slots["active_cast"] = " / ".join(participants) |
There was a problem hiding this comment.
Preserve named cast when no role nouns are present
For seeds such as At the gala, Alice and Bob argue over a missing contract, _extract_active_cast returns no participants because the names do not match PARTICIPANT_RE, and this branch only fills active_cast from recognized role nouns. The previous conjunction fallback treated that seed as cast evidence; now Story Butler asks for cast again and can delay the automatic Brief even though the user already named the parties.
Useful? React with 👍 / 👎.
| PLAYER_ROLE_CAPTURE_RE = re.compile( | ||
| r"\b(?:i am|i'm|i play as|i play|my role is|make me|player is|protagonist is|player should be)\s+(?:a\s+|an\s+|the\s+)?([^.!?;,]{2,100})", | ||
| re.I, |
There was a problem hiding this comment.
Accept role-correction phrases when updating player role
For corrections like Actually, change me to the publicist or switch me to the backup dancer, classify_story_guide_input routes the turn as correction_update, but this new capture regex no longer includes the change me to / switch me to / i should be forms that the old detector accepted. Because correction turns do not fall back to nextMissing, the previous player_role remains in the context and the Brief/opening can preserve the wrong identity.
Useful? React with 👍 / 👎.
| display_title: str | None = Field(default=None, min_length=1, max_length=72) | ||
| display_intro: str | None = Field(default=None, min_length=1, max_length=140) | ||
| premise_summary: str = Field(min_length=1, max_length=260) | ||
| player_role: str | None = Field(default=None, max_length=160) |
There was a problem hiding this comment.
Enforce brief player role during opening consistency
Adding player_role makes the player's identity part of the reviewed Brief, but check_story_brief_opening_consistency never compares this field with opening.player_role_options or the opening text. If the live opening returns an otherwise valid cast and pressure while using a generic or different player role, the consistency gate passes and starts the session from the wrong identity instead of retrying or falling back.
Useful? React with 👍 / 👎.
| guide_context: { | ||
| scene_summary: guideLoopState.context.scene_summary, | ||
| player_role: guideLoopState.context.player_role, |
There was a problem hiding this comment.
Populate guide context when frontend fallback is used
When createNarrativeStoryGuideTurn fails, appendGuideTurn falls back to the local advanceStoryGuideLoop, whose state keeps the filled slots but does not populate context. This new payload then sends an empty guide_context, so the backend cannot set StoryBrief.player_role or preserve confirmed cast/pressure for that otherwise supported fallback path; synthesize the guide context from slots/accepted turns before calling the Brief API.
Useful? React with 👍 / 👎.
What changed
Why
The target-mode audit found three player-visible reliability gaps:
Product impact
The full flow now reads coherently from Home -> New Story -> Story Butler -> automatic Story Brief -> Generate -> Play -> ending -> replay -> Home. Existing Play, reviewer, privacy, retry, and QA-route contracts remain covered by the full test suite.
Validation
pytest -q: passed (one existing skip).npm --prefix frontend2 run check: passed.npm --prefix frontend2 run build: passed; existing Vite large-chunk warning only.live/success; zero fallback reasons.Environment note
The local Live acceptance also uncovered an ignored
.envissue: a valid singular key was shadowed by a stale explicit key pool. The local pool was corrected without committing or exposing credentials; no secret is part of this PR.