[codex] stabilize Tiny Stories gameplay feedback loop - #20
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aceee3b13d
ℹ️ 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".
| code="seed_required", | ||
| message="Seed must not be empty.", | ||
| ) | ||
| return narrative_service.create_story_brief(payload, owner_user_id=session.user.user_id) |
There was a problem hiding this comment.
Enforce quotas for story brief LLM calls
When a text gateway is configured, create_story_brief can invoke the live LLM, but this new endpoint returns directly without calling _enforce_llm_quota or accepting the Request needed for the IP key. A signed-in or auto-created guest user can repeatedly POST to /narrative/story-briefs and bypass the daily per-IP/per-user LLM limits that the adjacent story-guide, template creation, turn, and advisor endpoints apply.
Useful? React with 👍 / 👎.
| for entry in self._gateway_trace_since(start_index): | ||
| usage = entry.get("usage") if isinstance(entry.get("usage"), dict) else {} | ||
| status = _coerce_llm_call_status(entry.get("status"), entry.get("failure_message_bucket")) | ||
| source_label: LLMCallSourceLabel = "live_repaired" if status == "repaired" else "live" | ||
| self._repo.append_llm_call_event( |
There was a problem hiding this comment.
Isolate LLM traces per request
Under overlapping live LLM requests, this iterates every entry appended to the shared gateway call_trace after start_index; because narrative_service owns one shared gateway, entries from another user's create/turn request can land in that slice before this request persists it. Those events are then written with this request's user_id/session_id, corrupting reviewer telemetry and potentially exposing another session's response IDs/token data in the wrong runtime inspector.
Useful? React with 👍 / 👎.
| runtime_state_db_path: str = "artifacts/runtime_state.sqlite3" | ||
| default_actor_id: str = "local-dev" | ||
| agent_trace_reviewer_user_ids: str = "" | ||
| agent_trace_reviewer_usernames: str = "portfolio_reviewer" |
There was a problem hiding this comment.
Require explicit trace reviewers by default
With the current username-only login, leaving portfolio_reviewer in the default allowlist means any visitor can POST /auth/login with that username and immediately receive can_view_agent_trace=true without a password or admin provisioning. In any deployment that does not override APP_AGENT_TRACE_REVIEWER_USERNAMES, the new reviewer/admin gate is effectively public for anyone's own sessions, so this default should be empty or local-only configuration rather than a production setting.
Useful? React with 👍 / 👎.
| const authorReady = await ensureAuthorSession() | ||
| if (!authorReady) return |
There was a problem hiding this comment.
Restore brief readiness when auth is not ready
If Story Brief planning starts while auth is still loading, or the guest login fails, ensureAuthorSession() returns false after the state has already been changed to analyzing; this early return only clears briefBusy in finally and never restores ready_to_brief. The existing seed then no longer satisfies guideReadyToBrief, so the auto-brief effect will not retry and the user has to add an unrelated extra chat turn to recover.
Useful? React with 👍 / 👎.
| code="guide_message_required", | ||
| message="Message must not be empty.", | ||
| ) | ||
| _enforce_llm_quota(request, user_id=session.user.user_id) |
There was a problem hiding this comment.
Charge both live Story Butler calls
For an accepted Story Butler turn with the text gateway configured, create_story_guide_turn performs two provider operations (create.story_butler_context and create.story_butler_turn), but this route debits the quota with the default cost of 1. A user can therefore run the chat loop and consume roughly twice the intended daily LLM operations before the per-IP/per-user limits trigger.
Useful? React with 👍 / 👎.
| opening_fallback_reason: str | None = None | ||
| operation_started_at = time.monotonic() | ||
| trace_start = self._trace_start() | ||
| if request.story_brief is not None and _story_brief_prefers_reliable_opening(request.story_brief): |
There was a problem hiding this comment.
Reject not-fit briefs server-side
When a client posts a story_brief whose advisor result was runtime_fit_status: "not_fit", this branch still proceeds to synthesize a live or fallback opening instead of returning the same revise-first gate enforced in the UI. Direct API callers can bypass the disabled Generate CTA and publish exactly the small-cast/out-of-range briefs that can_generate=false was meant to block.
Useful? React with 👍 / 👎.
Summary
Latest stabilization slice
ea4ebf8 fix: stabilize gameplay metadata feedbackadds optional chipdetail, shortWhy nowchips, per-chip title/ARIA text, and expanded-card reason detail rows.Validation
python -m py_compile rpg_backend/narrative/contracts.py rpg_backend/narrative/service.pygit diff --checkgit diff --cached --checknpm --prefix frontend2 run checknpm --prefix frontend2 run build(existing large chunk warning only)/opt/anaconda3/bin/pytest -q tests/test_play_direction_a_editorial_primitives_contract.py tests/test_published_direct_play_contract.py tests/test_navigation_mental_model_contract.py tests/test_narrative_agent_trace.py(47 passed; existing pytest-asyncio deprecation warning only)Browser Evidence
#/qa/play-gameplay-loop: objective, pressure tracks, clue card present; no overflow or technical leakage.#/qa/play-action: idle -> expanded -> pending receipt/reaction -> resolved remained stable.#/qa/play-action?scenario=long-historyat 390px: Back-to-move cue scrolled action area into view; no horizontal overflow or leakage.#/play/sess_3i_reason_detail:data-gameplay-envelope-source="live_enriched"; chips stayed short; chip titles carried full reasons; expanded selected card showed the fullWhy nowdetail row; console warn/error count 0.Notes / Risks