Skip to content

feat: harden live spectator mode#708

Open
kimanicode wants to merge 1 commit into
OpenKnight-Foundation:mainfrom
kimanicode:feat/587-live-match-spectator-mode
Open

feat: harden live spectator mode#708
kimanicode wants to merge 1 commit into
OpenKnight-Foundation:mainfrom
kimanicode:feat/587-live-match-spectator-mode

Conversation

@kimanicode

@kimanicode kimanicode commented Apr 29, 2026

Copy link
Copy Markdown

Closes #587


Summary

Implements and hardens the live match spectator mode work for issue #587.

  • Fixes live move list rendering by adding stable keys for grouped move rows
  • Expands spectator utility test coverage for clock normalization, malformed move payloads, and captured-piece symbols
  • Expands spectator WebSocket tests for sync, incremental move updates, clock updates, spectator counts, game-over events, and malformed messages
  • Adds local react-icons type declarations so frontend TypeScript checks pass

Summary by CodeRabbit

Release Notes

  • Tests

    • Expanded test coverage for spectator utilities and socket integration, including validation of edge cases and malformed input handling.
  • Chores

    • Added TypeScript type definitions for icon library imports.
    • Improved component rendering optimization.

@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

These changes expand test coverage for spectator utilities and socket handling, introduce TypeScript declarations for react-icons components, and refactor fragment handling in SpectatorMoveList for improved React reconciliation during rendering.

Changes

Cohort / File(s) Summary
Test Coverage Expansion
frontend/__tests__/spectatorUtils.test.ts, frontend/__tests__/useSpectatorSocket.test.ts
New test cases added: spectatorUtils.test.ts covers edge cases for formatClock (negative seconds, non-integers) and adds tests for getCapturedPieceSymbols piece-to-symbol translation; useSpectatorSocket.test.ts introduces integration tests validating socket message handling (move, clock, spectator_count, game_over) and error handling for malformed payloads.
Type Declarations
frontend/types/react-icons.d.ts
New TypeScript module declarations for react-icons/fa and react-icons/ri, defining IconType as ComponentType<SVGProps<SVGSVGElement>> and exporting specific icon components (e.g., FaBolt, FaSearch, RiAiGenerate, RiRobot2Line).
Component Refactoring
frontend/components/watch/SpectatorMoveList.tsx
Migrated from unkeyed React fragments (<>...</>) to keyed Fragment components to improve React reconciliation during move row mapping and rendering.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 In tests we trust, with coverage bright,
Icons typed, our imports right,
Fragments keyed for render's grace,
Spectators watch from every space! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: hardening live spectator mode through bug fixes (stable keys for move list rendering) and expanded test coverage for utilities and WebSocket integration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@drips-wave

drips-wave Bot commented Apr 29, 2026

Copy link
Copy Markdown

@kimanicode Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/types/react-icons.d.ts (1)

36-44: Consider removing unused icon declarations or documenting their purpose.

All icons currently imported from react-icons/ri in the codebase (RiAliensFill) are properly declared. However, RiAiGenerate and RiRobot2Line are declared but never imported or used anywhere. Either remove these unused declarations or add a comment explaining they are intentional future placeholders.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/types/react-icons.d.ts` around lines 36 - 44, The declarations
RiAiGenerate and RiRobot2Line are unused placeholders while RiAliensFill is
actually used; either remove the unused exports (delete the declarations for
RiAiGenerate and RiRobot2Line) or add a short comment above them documenting
they are intentional future placeholders (e.g., "// reserved for future icons:
RiAiGenerate, RiRobot2Line") so reviewers know they are deliberate; update the
declarations block that currently exports IconType and the three symbols
(RiAiGenerate, RiAliensFill, RiRobot2Line) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/types/react-icons.d.ts`:
- Around line 36-44: The declarations RiAiGenerate and RiRobot2Line are unused
placeholders while RiAliensFill is actually used; either remove the unused
exports (delete the declarations for RiAiGenerate and RiRobot2Line) or add a
short comment above them documenting they are intentional future placeholders
(e.g., "// reserved for future icons: RiAiGenerate, RiRobot2Line") so reviewers
know they are deliberate; update the declarations block that currently exports
IconType and the three symbols (RiAiGenerate, RiAliensFill, RiRobot2Line)
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9b9c32e1-200c-49bb-88a0-ac14541b9af0

📥 Commits

Reviewing files that changed from the base of the PR and between 8f92605 and 0711ae9.

📒 Files selected for processing (4)
  • frontend/__tests__/spectatorUtils.test.ts
  • frontend/__tests__/useSpectatorSocket.test.ts
  • frontend/components/watch/SpectatorMoveList.tsx
  • frontend/types/react-icons.d.ts

@kimanicode

Copy link
Copy Markdown
Author

Closes #587

Summary

Implements and hardens the live match spectator mode work for issue #587.

  • Fixes live move list rendering by adding stable keys for grouped move rows
  • Expands spectator utility test coverage for clock normalization, malformed move payloads, and captured-piece symbols
  • Expands spectator WebSocket tests for sync, incremental move updates, clock updates, spectator counts, game-over events, and malformed messages
  • Adds local react-icons type declarations so frontend TypeScript checks pass

Summary by CodeRabbit

Release Notes

  • Tests

    • Expanded test coverage for spectator utilities and socket integration, including validation of edge cases and malformed input handling.
  • Chores

    • Added TypeScript type definitions for icon library imports.
    • Improved component rendering optimization.

@kimanicode kimanicode closed this May 4, 2026
@kimanicode kimanicode reopened this May 4, 2026
@kimanicode

Copy link
Copy Markdown
Author

Hi @gabito1451 , just check in on this PR when you get a chance. Happy to make any changes if needed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Frontend: Live match spectator mode

1 participant