fix(speak): keep help-offer dialogue coherent across worker and client - #21
Conversation
Distinguish player help offers from help requests, gate casual/social fast lanes when session history exists, inject recent_turns into social LLM prompts, and align shared preview routing with worker parity including extended offer phrases and per-NPC dialogue history on web. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds recent-dialogue context to NPC speak previews and social-turn prompts, distinguishes help offers from help requests, blocks deterministic fast lanes when history is present, and adds web, worker, unit, and Playwright validation. ChangesHistory-aware speak handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant useNpcChat
participant GameRoom
participant llm_social_turn
participant append_recent_dialogue_messages
Player->>useNpcChat: Submit speak message
useNpcChat->>GameRoom: Send message with NPC context
GameRoom->>llm_social_turn: Process speak with recent turns
llm_social_turn->>append_recent_dialogue_messages: Append player and NPC history
append_recent_dialogue_messages-->>llm_social_turn: Return contextual messages
llm_social_turn-->>Player: Generate contextual NPC reply
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf7a71ecf5
ℹ️ 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".
| return None | ||
| inferred = infer_social_from_message(msg) | ||
| if inferred is not None: | ||
| if (recent_turns or player_offers_help(msg)) and inferred.kind in ("rude", "help"): |
There was a problem hiding this comment.
Skip deterministic casual replies when history exists
In the worker interactive path, this guard only disables deterministic rude/help replies. With existing recent_turns, a follow-up such as 你好,用一句话简短回复一下你的计划安排吧 (20 chars, contains 一句话) is still classified as CASUAL; process_job skips the B1 fast lane, but llm_social_turn() calls _deterministic_social_turn() and falls through to the CASUAL branch below, emitting a canned reply before append_recent_dialogue_messages() can run. That reintroduces history-ignoring stub replies for longer continuation/meta-brief messages.
Useful? React with 👍 / 👎.
| if (m.npcId !== npcId) { | ||
| pendingPlayers = []; |
There was a problem hiding this comment.
Preserve pending turns across other NPC replies
When chats with different NPCs overlap, this clears the pending player line for the target NPC as soon as any other NPC reply is seen. Since sendMessage only blocks the same npcId, a user can send to npc-5, switch to npc-1, receive npc-1's reply, then receive npc-5's reply; the npc-5 turn is omitted from recentDialogueTurnsForNpc, so the client may show a casual preview for the next npc-5 follow-up while the server/worker correctly see history. Keep pending turns per NPC or ignore unrelated NPC messages instead of clearing them.
Useful? React with 👍 / 👎.
… NPCs Block deterministic CASUAL/meta replies in llm_social_turn when recent_turns exist (interactive path parity with fast-lane gate). Skip unrelated NPC messages in dialogueTurns without clearing pending player lines. Co-authored-by: Cursor <cursoragent@cursor.com>
Distinguish player help offers from help requests, gate casual/social fast lanes when session history exists, inject recent_turns into social LLM prompts, and align shared preview routing with worker parity including extended offer phrases and per-NPC dialogue history on web.
Summary by CodeRabbit
New Features
Bug Fixes
Tests