Skip to content

Show game chat reload recovery state#5

Open
Promansis wants to merge 1 commit into
local/bunny-style-reviewfrom
fix/1595-game-reload
Open

Show game chat reload recovery state#5
Promansis wants to merge 1 commit into
local/bunny-style-reviewfrom
fix/1595-game-reload

Conversation

@Promansis

Copy link
Copy Markdown
Owner

Linked issue

Closes #

Why this change

What changed

Validation

  • pnpm check passes locally
  • Container (Docker / Podman) built and ran without issue
  • Ran the app, clicked through the changes manually
  • Checked edge cases (light + dark mode, mobile viewport, empty states, error paths)
  • Above manual verification completed (describe below)
  • Read and followed CONTRIBUTING.md

Manual verification notes

Docs and release impact

  • No docs changes needed
  • Updated docs (README / CONTRIBUTING / android/README / CHANGELOG) as needed
  • Version/release files updated (only if this PR includes a version bump)

UI evidence (if applicable)

@github-actions github-actions Bot added the bugfix Bug fix label May 30, 2026
@github-actions

github-actions Bot commented May 30, 2026

Copy link
Copy Markdown

This is concrete. The changed GameConversationView loading state is dead on fresh reload while outer ModeSurface fallback routes to conversation. Need maybe cite lines 35-42 ModeSurface not changed? Finding can cite changed file? Requirement cites concrete file/line or small changed area. Finding caused by changed area not sufficient? We can cite GameConversationView line 90 but fix likely ModeSurface. Since ModeSurface not changed? But reviewers can cite existing code if interaction. We can mention changed area GameConversationView:90 because it assumes it owns missing chat state, but parent never renders. Better cite ModeSurface:35? Finding against PR may cite missing change in parent. It's okay if not changed? CodeRabbit findings usually on changed lines. But line 90 changed. "This guard is ineffective because..." cite changed file line 90.

Need maybe look at actual line numbers? read file line 90. Good.

Need maybe another issue: New useChatSurfaceData returns chatError, but not used elsewhere. No.

Could find tests missing? Maybe no tests but one real bug is enough.

Need final only review text. Include What Checked commands etc. Mention merge-base no merge base and used HEAD^? Setup required base but failed. Keep short. Severity: high? User-visible reload regression remains, maybe medium. Since PR is supposed to fix reload and likely doesn't, high? It doesn't introduce worse? It adds useless inner. But if branch judged against target includes actual change maybe blocking because fix incomplete. Severity "medium" probably.

Open questions about no merge base? Could include. But output should concise. Open question: base branch has no merge base with HEAD; I reviewed HEAD^..HEAD for actual PR commit. Maybe under What checked.

Need include exact command list.

Bunny Review

Findings

  • [medium] src/features/modes/game/components/GameConversationView.tsx:90 - Game reload state is installed below the router that decides the mode. On a fresh reload, ModeSurface does not yet have chat?.mode, so it falls back to lastModeRef.current initialized to "conversation" and renders ConversationModeRoute instead of GameModeRoute until the chat query resolves. That means this new !data.chat loading/error UI in GameConversationView is not reached for the blank/reload window it is meant to cover, and a game chat can still briefly mount the conversation surface with game chat metadata/messages. Move the loading/error recovery to ModeSurface while the active chat detail is unresolved, or persist/derive the last known mode before choosing the route so game chats keep rendering the game route during hydration.

Open Questions

  • The requested base origin/local/bunny-style-review has no merge base with HEAD; I reviewed the actual PR commit range (HEAD^..HEAD) after the required three-dot diff failed.

What I Checked

  • git status --short --branch, git rev-parse --show-toplevel, git merge-base HEAD origin/local/bunny-style-review
  • git diff --stat origin/local/bunny-style-review..HEAD, git diff --name-only origin/local/bunny-style-review..HEAD, and focused git diff HEAD^..HEAD
  • AGENTS.md
  • skills/marinara-architecture-guard/SKILL.md, skills/marinara-mode-separation/SKILL.md, skills/marinara-bugfix-discipline/SKILL.md, skills/marinara-getting-started/SKILL.md
  • src/features/modes/game/components/GameConversationView.tsx
  • src/features/modes/shared/chat-ui/hooks/use-chat-surface-data.ts
  • src/features/modes/router/components/ModeSurface.tsx
  • src/features/modes/conversation/components/ConversationModeRoute.tsx

@Promansis

Copy link
Copy Markdown
Owner Author

/bunny-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant