fix: consolidate window globals and align game renderer tests - #46
Merged
Conversation
…evader, and memory-matrix - Refactor bubble-shooter framework init and align tests - Add evader framework init with tests and renderer improvements - Add memory-matrix renderer tests and DOM cleanup handling Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…est. Touch D-pad buttons would call pressKey before the game started, leaving a stuck key in rawHeldKeys. Match the keyboard handler by checking game state. Update the cleanup test to assert pressKey is not called after cleanup. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Release implicit pointer capture on D-pad buttons so pointerleave fires while dragging on touch devices, and remove them from tab order. Ignore modifier-key Enter/Space on Memory Matrix cards and expose flipped/matched state via aria-labels. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…bal declarations. Introduce the shared AchievementNotification interface in achievements.ts and use it across scoreService.ts, GameInitializer.ts, AchievementAward.astro, and all game init frameworks. This eliminates repeated local definitions and keeps the Window.showAchievementAward augmentation consistent. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Keep keyboard focus on the same card when Memory Matrix re-renders so keyboard activation does not drop focus to <body>. Hide the Evader touch D-pad on fine-pointer (desktop) devices. Update Bubble Shooter pointerdown test to assert aim is set before shooting, and align achievement test fixtures with the condition-based schema. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Centralize the showAchievementAward/showChallengeComplete declarations in env.d.ts, remove the duplicate global from scoreService.ts, and guard the window calls in GameInitializer for SSR safety. Update the Evader test to match the real AchievementNotification shape. Keep Memory Matrix focus on flipped/matched cards with tabindex="-1" so keyboard activation doesn't lose focus to <body>. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughThis PR centralizes achievement/challenge notification types ( ChangesGame UI Contracts and Interaction
Estimated code review effort: 4 (Complex) | ~60 minutes Game Rendering Updates
Estimated code review effort: 2 (Simple) | ~15 minutes Database Schema Compatibility
Estimated code review effort: 3 (Moderate) | ~25 minutes Type-Checking and Test Support
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant PointerControls
participant Game
Player->>PointerControls: pointermove or pointerdown
PointerControls->>Game: update aim or press mapped key
Player->>PointerControls: pointerup, pointerleave, or pointercancel
PointerControls->>Game: release mapped key
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/pages/evader/index.astro (1)
226-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared sci-fi theme tokens for D-pad styling.
The new styles hard-code cyan
rgbaand hex values instead of using the project’s centralized theme utilities or variables, which can cause visual drift across themes.As per coding guidelines, Astro styling should use Tailwind CSS 4.1.3 with the shared sci-fi theme system.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/evader/index.astro` around lines 226 - 260, Update the `#dpad` and .dpad-btn styling to use the shared sci-fi theme tokens or centralized theme utilities instead of hard-coded cyan rgba and hex values. Preserve the existing D-pad layout, interaction states, and responsive visibility while replacing border, background, text, and active-state shadow colors with the established theme variables.Source: Coding guidelines
src/lib/games/bubble-shooter/initFramework.test.ts (1)
492-499: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the mock return value from a typed game-state fixture.
...gameMock.getStatespreads the mock function itself, not the state it returns, andas anyhides the resulting shape mismatch. Use the existing typed fixture/factory or capture the current state value before overriding these fields.As per coding guidelines, strict TypeScript should avoid explicit
anycasts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/bubble-shooter/initFramework.test.ts` around lines 492 - 499, Update the mock setup around gameMock.getState to construct its return value from the existing typed game-state fixture or factory, rather than spreading the getState mock function. Capture or reuse the current state value before overriding the specified fields, and remove the explicit any cast while preserving the intended state values.Source: Coding guidelines
src/lib/games/evader/EvaderRenderer.test.ts (1)
178-193: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the gradient configuration, not only its construction.
This test still passes if the gradient receives incorrect coordinates, no color stops, or is never used for the player fill. Capture the mocked gradient and assert its constructor arguments plus the three
addColorStopcalls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/evader/EvaderRenderer.test.ts` around lines 178 - 193, Update the test “renders the player at its position using FillGradient” to capture the mocked FillGradient instance and assert its constructor coordinates match the player position and configured player size. Also verify exactly three addColorStop calls with the expected offsets and colors, confirming the gradient is configured for the player fill rather than merely constructed.src/lib/services/achievementService.test.ts (1)
59-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
as anyfrom the migrated achievement fixtures.Import
type Achievementand usesatisfies Achievement; these casts prevent the tests from validating the contract they were updated to cover.
src/lib/services/achievementService.test.ts#L59-L64: importtype Achievement.src/lib/services/achievementService.test.ts#L215-L218: replaceas anywithsatisfies Achievement.src/lib/services/achievementService.test.ts#L346-L349: replaceas anywithsatisfies Achievement.src/lib/services/achievementService.test.ts#L438-L441: replaceas anywithsatisfies Achievement.src/lib/services/achievementService.test.ts#L469-L472: replaceas anywithsatisfies Achievement.src/lib/services/achievementService.test.ts#L495-L498: replaceas anywithsatisfies Achievement.src/lib/services/achievementService.test.ts#L521-L524: replaceas anywithsatisfies Achievement.Proposed pattern
-import { getAchievementsByGame, AchievementRarity } from '../achievements' +import { + getAchievementsByGame, + AchievementRarity, + type Achievement, +} from '../achievements' - } as any, + } satisfies Achievement,As per coding guidelines, “Use strict TypeScript configuration with explicit any warnings.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/services/achievementService.test.ts` around lines 59 - 64, Remove the any casts from the migrated achievement fixtures so the tests validate the Achievement contract. In src/lib/services/achievementService.test.ts at lines 59-64, import type Achievement; at lines 215-218, 346-349, 438-441, 469-472, 495-498, and 521-524, replace each as any assertion with satisfies Achievement.Source: Coding guidelines
env.d.ts (1)
29-40: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReuse the shared
ChallengeUpdatescontract.This duplicates
src/lib/games/core/types.ts:42-53; future payload changes can leave the browser callback stale. Type this parameter asimport('@/lib/games/core/types').ChallengeUpdates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@env.d.ts` around lines 29 - 40, Update the showChallengeComplete callback parameter to use the shared ChallengeUpdates type imported from `@/lib/games/core/types` instead of the duplicated inline object definition. Remove the local payload shape while preserving the callback’s existing optional property and return type.src/lib/games/evader/initFramework.test.ts (2)
516-522: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the notification fixture against the shared contract.
Annotate this object as
AchievementNotificationand useAchievementRarity.RARE; the current inferred literal does not detect contract changes to the global callback payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/evader/initFramework.test.ts` around lines 516 - 522, Update the achievement fixture in the notification test to explicitly use the shared AchievementNotification contract and set its rarity through AchievementRarity.RARE. Preserve the existing fixture values while ensuring future contract changes are caught by type checking.
403-512: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Testing Library for the D-pad interactions.
These DOM tests use
document.querySelectorand manual dispatches instead of Testing Library queries/events. Use role-based button queries and Testing Library event helpers, retaining a custom event only wherepointerIdsetup is necessary. As per coding guidelines, “Use Vitest with jsdom environment and Testing Library for unit tests.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/evader/initFramework.test.ts` around lines 403 - 512, Update the touch-controls tests in the “touch controls (D-pad)” suite to use Testing Library role-based button queries and event helpers instead of document.querySelector and manual MouseEvent dispatches. Preserve the existing assertions and cleanup behavior, using a custom event only in the pointer-capture test where pointerId must be explicitly configured.Source: Coding guidelines
src/lib/games/memory-matrix/MemoryMatrixRenderer.test.ts (1)
195-395: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftUse Testing Library for the new renderer tests.
The added tests rely on
querySelector,dispatchEvent, and raw child inspection. Prefer role-based Testing Library queries and its keyboard/event helpers, especially for accessibility behavior. Verify the repository’s existing Testing Library setup before converting.As per coding guidelines, “Use Vitest with jsdom environment and Testing Library for unit tests.”
Also applies to: 524-533
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/memory-matrix/MemoryMatrixRenderer.test.ts` around lines 195 - 395, Convert the added keyboard accessibility and focus-restoration tests in the relevant describe blocks to use the repository’s configured Testing Library utilities. Replace raw querySelector/querySelectorAll lookups with role-based queries, use Testing Library keyboard/event helpers instead of dispatchEvent, and assert focus and accessibility attributes through Testing Library-compatible APIs while preserving the existing behaviors under test.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/games/bubble-shooter/initFramework.ts`:
- Around line 486-490: Update pointerDownHandler to ignore non-primary pointer
presses before calling pointerMoveHandler or game.shoot(). Preserve shooting for
primary-button input while preventing right- and middle-button presses from
firing or triggering related behavior.
In `@src/lib/games/evader/EvaderRenderer.ts`:
- Around line 118-126: Cache and reuse the player gradient instead of
constructing it inside renderGame(), updating its geometry as needed each frame.
Apply the same caching pattern to the coin and bomb gradients created by
renderObjects(), and release all cached gradients in cleanup().
In `@src/lib/games/memory-matrix/MemoryMatrixRenderer.test.ts`:
- Around line 354-366: Update the test case “should not restore focus when no
card was focused” to assert after the second renderer.render(state) that
document.activeElement remains outside boardEl, preserving the intended
no-focus-restoration behavior. Keep the existing board child-count assertion as
a separate structural check.
In `@src/pages/evader/index.astro`:
- Around line 71-107: Update the D-pad buttons identified by their dpad-btn
class to remain in sequential keyboard navigation by removing tabindex="-1" or
setting tabindex="0". Add a visible :focus-visible style for these controls so
keyboard users can locate the focused button.
---
Nitpick comments:
In `@env.d.ts`:
- Around line 29-40: Update the showChallengeComplete callback parameter to use
the shared ChallengeUpdates type imported from `@/lib/games/core/types` instead of
the duplicated inline object definition. Remove the local payload shape while
preserving the callback’s existing optional property and return type.
In `@src/lib/games/bubble-shooter/initFramework.test.ts`:
- Around line 492-499: Update the mock setup around gameMock.getState to
construct its return value from the existing typed game-state fixture or
factory, rather than spreading the getState mock function. Capture or reuse the
current state value before overriding the specified fields, and remove the
explicit any cast while preserving the intended state values.
In `@src/lib/games/evader/EvaderRenderer.test.ts`:
- Around line 178-193: Update the test “renders the player at its position using
FillGradient” to capture the mocked FillGradient instance and assert its
constructor coordinates match the player position and configured player size.
Also verify exactly three addColorStop calls with the expected offsets and
colors, confirming the gradient is configured for the player fill rather than
merely constructed.
In `@src/lib/games/evader/initFramework.test.ts`:
- Around line 516-522: Update the achievement fixture in the notification test
to explicitly use the shared AchievementNotification contract and set its rarity
through AchievementRarity.RARE. Preserve the existing fixture values while
ensuring future contract changes are caught by type checking.
- Around line 403-512: Update the touch-controls tests in the “touch controls
(D-pad)” suite to use Testing Library role-based button queries and event
helpers instead of document.querySelector and manual MouseEvent dispatches.
Preserve the existing assertions and cleanup behavior, using a custom event only
in the pointer-capture test where pointerId must be explicitly configured.
In `@src/lib/games/memory-matrix/MemoryMatrixRenderer.test.ts`:
- Around line 195-395: Convert the added keyboard accessibility and
focus-restoration tests in the relevant describe blocks to use the repository’s
configured Testing Library utilities. Replace raw querySelector/querySelectorAll
lookups with role-based queries, use Testing Library keyboard/event helpers
instead of dispatchEvent, and assert focus and accessibility attributes through
Testing Library-compatible APIs while preserving the existing behaviors under
test.
In `@src/lib/services/achievementService.test.ts`:
- Around line 59-64: Remove the any casts from the migrated achievement fixtures
so the tests validate the Achievement contract. In
src/lib/services/achievementService.test.ts at lines 59-64, import type
Achievement; at lines 215-218, 346-349, 438-441, 469-472, 495-498, and 521-524,
replace each as any assertion with satisfies Achievement.
In `@src/pages/evader/index.astro`:
- Around line 226-260: Update the `#dpad` and .dpad-btn styling to use the shared
sci-fi theme tokens or centralized theme utilities instead of hard-coded cyan
rgba and hex values. Preserve the existing D-pad layout, interaction states, and
responsive visibility while replacing border, background, text, and active-state
shadow colors with the established theme variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6694f40c-27df-4fcb-b2e7-385a00b5443e
📒 Files selected for processing (25)
env.d.tssrc/components/AchievementAward.astrosrc/lib/achievements.test.tssrc/lib/achievements.tssrc/lib/games/2048/initFramework.tssrc/lib/games/bejeweled/init.tssrc/lib/games/bubble-shooter/initFramework.test.tssrc/lib/games/bubble-shooter/initFramework.tssrc/lib/games/core/GameInitializer.tssrc/lib/games/evader/EvaderRenderer.test.tssrc/lib/games/evader/EvaderRenderer.tssrc/lib/games/evader/initFramework.test.tssrc/lib/games/evader/initFramework.tssrc/lib/games/memory-matrix/MemoryMatrixRenderer.test.tssrc/lib/games/memory-matrix/MemoryMatrixRenderer.tssrc/lib/games/memory-matrix/initFramework.tssrc/lib/games/path-navigator/initFramework.tssrc/lib/games/reflex/initFramework.tssrc/lib/games/snake/initFramework.tssrc/lib/games/sudoku/initFramework.tssrc/lib/games/tetris/initFramework.tssrc/lib/games/word-scramble/initFramework.tssrc/lib/services/achievementService.test.tssrc/lib/services/scoreService.tssrc/pages/evader/index.astro
Cache Evader FillGradient instances to avoid recreating them every frame and clean them up on destroy. Ignore non-primary pointer buttons in Bubble Shooter canvas controls so only the main button/touch/pen tip can aim and shoot. Remove tabindex from Evader D-pad buttons so they are naturally focusable and add a visible focus style using the theme accent. Switch Memory Matrix and Evader tests to use Testing Library queries and fireEvent, and type achievement tests with the Achievement type instead of as any. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Reflects the actual post-fire game state in the test assertion, ensuring the framework test matches the updated bubble-shooter logic. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (8)
src/components/ChallengeComplete.astro (1)
165-209: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle failures from
processQueue().All three invocations discard the returned promise. A rendering exception therefore becomes an unhandled rejection and can leave
isShowingset with the queue permanently stalled. Use one caught processing entry point that also resets the overlay, timer, and queue state on failure.As per coding guidelines, always include error handling in async operations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ChallengeComplete.astro` around lines 165 - 209, Update processQueue and its three invocation sites to use a single caught processing entry point that handles rejected promises. On failure, reset isShowing, dismiss or hide the overlay, clear the auto-dismiss timer, and restore queue state so subsequent items can continue processing; ensure the error is handled without creating an unhandled rejection.Source: Coding guidelines
src/pages/api/user/update-streaks.test.ts (1)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the typed environment value instead of
as any.This cast hides a potential string/boolean mismatch between Vitest’s
stubEnvAPI andimport.meta.env.PROD. Use the value type declared by the project, or introduce a typed environment-stubbing helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/user/update-streaks.test.ts` at line 184, Update the test setup using vi.stubEnv to pass a value compatible with the project’s declared environment type, removing the `as any` cast. Reuse the existing typed environment value or add a typed helper, while preserving the intended PROD=true behavior.Source: Coding guidelines
src/lib/games/core/GameInitializer.test.ts (2)
92-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid double-casting
rendererConfig.
unknown as Record<string, unknown>suppresses validation of the fixture againstGameInitializerConfig. Declare the fixture with the expected type directly, or correct the boundary type if this is intended to be a real renderer configuration.Also applies to: 411-412
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/core/GameInitializer.test.ts` around lines 92 - 95, Remove the double cast from the rendererConfig fixtures in GameInitializer tests. Type the fixture directly using the expected GameInitializerConfig-compatible type, or update the boundary type if this represents a valid renderer configuration, while preserving the existing test values at both referenced locations.Source: Coding guidelines
477-482: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake test constructors match the initializer contract directly.
The double assertions allow arbitrary classes to satisfy
new (...args: unknown[]) => ..., hiding incompatible constructor signatures. GiveTestGameandTestRenderermatching constructors or use a typed adapter.Also applies to: 501-506
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/games/core/GameInitializer.test.ts` around lines 477 - 482, Update the TestGame and TestRenderer definitions used by the initializer setup so their constructors directly match the expected new (...args: unknown[]) => instance contract. Remove the double assertions around gameClass and rendererClass, or introduce a typed adapter that preserves this constructor signature without permitting incompatible classes.Source: Coding guidelines
src/lib/auth.ts (1)
57-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid casting the session cookie configuration to
any.This disables validation of every cookie option and can hide future typos or unsupported settings. Prefer a typed Better Auth configuration/augmentation or a narrow compatibility helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/auth.ts` around lines 57 - 64, Replace the broad any cast on the cookie configuration in the Better Auth session options with a typed Better Auth configuration/augmentation or narrow compatibility helper. Preserve the existing sameSite, secure, and httpOnly values while retaining compile-time validation for all cookie options.Source: Coding guidelines
src/lib/auth.test.ts (1)
176-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep OAuth assertions strongly typed.
as anysuppresses the response contract these tests should validate. Define a typed result interface or shared response type so missinguserorisNewUserfields fail during type checking.Also applies to: 202-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/auth.test.ts` at line 176, Replace the `as any` casts in the OAuth assertions around the `result.data.user` and related `isNewUser` checks with a typed result interface or shared response type. Apply the type consistently to the relevant test results so TypeScript validates that both `user` and `isNewUser` are present.Source: Coding guidelines
src/pages/api/scores.test.ts (1)
75-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize typed API-test fixtures instead of repeating
as any.
src/pages/api/scores.test.ts#L75-L95: type the session fixture and use a typedPOSTcontext helper; also remove the laterundefined as anyand element-wiseGameIDcasts.src/pages/api/scores/best.test.ts#L75-L95: type the session fixture and use a typedGETcontext helper; define parameterized IDs asGameID[].src/pages/api/scores/history.test.ts#L61-L68: type the session fixture and use a typedGETcontext helper.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/scores.test.ts` around lines 75 - 95, Centralize typed API-test fixtures and context helpers across the affected tests: in src/pages/api/scores.test.ts:75-95, type the session fixture, use a typed POST context helper, and remove the later undefined as any and element-wise GameID casts; in src/pages/api/scores/best.test.ts:75-95, type the session fixture, use a typed GET context helper, and declare parameterized IDs as GameID[]; in src/pages/api/scores/history.test.ts:61-68, type the session fixture and use a typed GET context helper.Source: Coding guidelines
src/pages/api/dev/add-test-scores.test.ts (1)
77-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid double-casting a partial Kysely builder as the full return type.
The mock only implements
values, but is asserted as the completeinsertIntoreturn type. Use a typed mock-builder helper so missing chained methods are detected during test maintenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/api/dev/add-test-scores.test.ts` around lines 77 - 79, Update the mock returned by the insertInto setup in the add-test-scores test to use a typed mock-builder helper rather than casting the partial object through unknown to ReturnType<typeof db.insertInto>. Ensure the helper models the builder interface while exposing the implemented values method, so missing chained methods are detected by the type system.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/AchievementAward.test.ts`:
- Line 6: Add jsdom TypeScript declarations through the project dependency or an
appropriate local d.ts file, then remove the `@ts-expect-error` above the jsdom
import in AchievementAward.test.ts. Ensure the import type-checks without
suppression.
In `@src/components/ChallengeComplete.astro`:
- Around line 148-159: Update the overlay click handler in ChallengeComplete so
it dismisses whenever the click target is outside the card, rather than
requiring the target to equal the overlay or carry overlayBackdrop metadata.
Preserve clicks inside the card without dismissing, using the existing
overlay/card elements and dismiss() method.
In `@src/components/UserDropdown.astro`:
- Around line 193-197: Update the logout flow around authClient.signOut() so
errors mentioning JSON never trigger a reload or indicate success. Reload only
after signOut completes successfully, or after explicitly confirming the session
is no longer active; preserve the existing error handling for failed logout
attempts.
- Around line 166-167: Update the event-target handling in UserDropdown so
closest('a') is called only after verifying the target is an Element, while
preserving the existing anchor detection behavior for element targets.
In `@src/lib/achievements.ts`:
- Around line 46-51: Update the achievement type definitions around the check
method and checkInGameAchievements so gameId determines the required gameData
shape through a discriminated union or gameId-to-payload mapping. Ensure each
in-game checker, including reflex_coin_streak, can only receive its
corresponding game-specific payload while preserving the existing score and
checker behavior.
In `@src/lib/games/bejeweled/init.test.ts`:
- Line 332: Replace the explicit any[] predicates used to find mocked event
calls with the actual mock call tuple type, or consistently use unknown[] with a
type-narrowing guard. Apply this to src/lib/games/bejeweled/init.test.ts lines
332-332 and 367-367, and src/lib/games/snake/initFramework.test.ts lines
547-547, 571-571, and 588-588, preserving each existing 'end' or
achievement-event lookup behavior.
In `@src/lib/games/sudoku/SudokuGame.test.ts`:
- Around line 67-68: Update the assertion in the Sudoku state test to check
state.timeRemaining directly, removing the (state as any).timer access and
fallback. Preserve the typed SudokuState contract; only add a timer property if
the implementation explicitly requires one.
In `@src/lib/games/tetris/TetrisGame.test.ts`:
- Line 134: Replace the any cast in the generateNextPiece test with a typed test
subclass or narrow test-only interface that explicitly exposes
generateNextPiece(). Keep the existing piece-type collection behavior unchanged
while preserving compile-time type checking.
In `@src/lib/server/db/queries.legacy-schema.test.ts`:
- Around line 88-95: Update the legacy-schema test setup around seedUser so
every test case receives a fresh in-memory database and resets the module-level
migration cache, using isolated module loading where needed. Ensure each test
creates and uses its own database instance before invoking migration logic,
rather than only deleting user_stats rows, so every function exercises its
complete legacy-schema migration path independently.
In `@src/lib/server/db/queries.migrations.test.ts`:
- Line 15: Replace the untyped createClient casts with the typed LibSQL client
factory used by queries.integration.test.ts. Update the client initialization in
src/lib/server/db/queries.migrations.test.ts:15 and
src/lib/server/db/queries.legacy-schema.test.ts:22 to use
libsql.createClient(...) or the equivalent typed alias, preserving the existing
connection options and client/dialect contract.
In `@src/lib/server/db/queries.ts`:
- Around line 1394-1397: Serialize lazy user_stats schema migrations through one
shared, awaited initialization promise, and use it at all four call sites in
src/lib/server/db/queries.ts: lines 1394-1397, 1557-1560, 1678-1681, and
1847-1850. Ensure migration state is recorded only after every migration
succeeds; re-check and retry duplicate or locked DDL failures rather than
swallowing them, so callers cannot proceed until all columns are available.
In `@vitest.config.ts`:
- Around line 5-8: Replace the whole-config any cast around getViteConfig and
defineConfig with a narrowly scoped type escape only at the Astro
UserConfig/Vitest test-key incompatibility boundary. Preserve type checking for
the getViteConfig input and defineConfig output, while retaining valid Vitest
test configuration support.
---
Nitpick comments:
In `@src/components/ChallengeComplete.astro`:
- Around line 165-209: Update processQueue and its three invocation sites to use
a single caught processing entry point that handles rejected promises. On
failure, reset isShowing, dismiss or hide the overlay, clear the auto-dismiss
timer, and restore queue state so subsequent items can continue processing;
ensure the error is handled without creating an unhandled rejection.
In `@src/lib/auth.test.ts`:
- Line 176: Replace the `as any` casts in the OAuth assertions around the
`result.data.user` and related `isNewUser` checks with a typed result interface
or shared response type. Apply the type consistently to the relevant test
results so TypeScript validates that both `user` and `isNewUser` are present.
In `@src/lib/auth.ts`:
- Around line 57-64: Replace the broad any cast on the cookie configuration in
the Better Auth session options with a typed Better Auth
configuration/augmentation or narrow compatibility helper. Preserve the existing
sameSite, secure, and httpOnly values while retaining compile-time validation
for all cookie options.
In `@src/lib/games/core/GameInitializer.test.ts`:
- Around line 92-95: Remove the double cast from the rendererConfig fixtures in
GameInitializer tests. Type the fixture directly using the expected
GameInitializerConfig-compatible type, or update the boundary type if this
represents a valid renderer configuration, while preserving the existing test
values at both referenced locations.
- Around line 477-482: Update the TestGame and TestRenderer definitions used by
the initializer setup so their constructors directly match the expected new
(...args: unknown[]) => instance contract. Remove the double assertions around
gameClass and rendererClass, or introduce a typed adapter that preserves this
constructor signature without permitting incompatible classes.
In `@src/pages/api/dev/add-test-scores.test.ts`:
- Around line 77-79: Update the mock returned by the insertInto setup in the
add-test-scores test to use a typed mock-builder helper rather than casting the
partial object through unknown to ReturnType<typeof db.insertInto>. Ensure the
helper models the builder interface while exposing the implemented values
method, so missing chained methods are detected by the type system.
In `@src/pages/api/scores.test.ts`:
- Around line 75-95: Centralize typed API-test fixtures and context helpers
across the affected tests: in src/pages/api/scores.test.ts:75-95, type the
session fixture, use a typed POST context helper, and remove the later undefined
as any and element-wise GameID casts; in
src/pages/api/scores/best.test.ts:75-95, type the session fixture, use a typed
GET context helper, and declare parameterized IDs as GameID[]; in
src/pages/api/scores/history.test.ts:61-68, type the session fixture and use a
typed GET context helper.
In `@src/pages/api/user/update-streaks.test.ts`:
- Line 184: Update the test setup using vi.stubEnv to pass a value compatible
with the project’s declared environment type, removing the `as any` cast. Reuse
the existing typed environment value or add a typed helper, while preserving the
intended PROD=true behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5fd5713-bfb4-4e6b-8bb2-bd3a30c9dba9
📒 Files selected for processing (36)
astro.config.mjse2e/pages/AuthPage.tse2e/pages/GamePage.tse2e/pages/HomePage.tspackage.jsonsrc/components/AchievementAward.test.tssrc/components/ChallengeComplete.astrosrc/components/UserDropdown.astrosrc/lib/achievements.tssrc/lib/auth.test.tssrc/lib/auth.tssrc/lib/games/2048/initFramework.test.tssrc/lib/games/bejeweled/init.test.tssrc/lib/games/bubble-shooter/initFramework.test.tssrc/lib/games/core/GameInitializer.test.tssrc/lib/games/evader/EvaderRenderer.tssrc/lib/games/quick-math/renderer.test.tssrc/lib/games/reflex/ReflexRenderer.tssrc/lib/games/satellite-sync/renderer.test.tssrc/lib/games/snake/initFramework.test.tssrc/lib/games/snake/utils.tssrc/lib/games/sudoku/SudokuGame.test.tssrc/lib/games/tetris/TetrisGame.test.tssrc/lib/games/word-scramble/initFramework.test.tssrc/lib/preferences.test.tssrc/lib/server/db/queries.legacy-schema.test.tssrc/lib/server/db/queries.migrations.test.tssrc/lib/server/db/queries.tssrc/pages/api/dev/add-test-scores.test.tssrc/pages/api/scores.test.tssrc/pages/api/scores/best.test.tssrc/pages/api/scores/history.test.tssrc/pages/api/settings/index.test.tssrc/pages/api/user/update-streaks.test.tsturbo.jsonvitest.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/games/evader/EvaderRenderer.ts
- src/lib/games/bubble-shooter/initFramework.test.ts
…dge cases. Enable stricter TypeScript typing across tests and config, add dedicated jsdom type declarations, and fix UI/auth edge cases (ChallengeComplete dismissal, UserDropdown logout errors) plus a single-flight guard for lazy user_stats schema migrations. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…t casts. Updates achievementService and its tests to use the renamed `getUserBestScore` query and defaults the result to 0. Also drops unnecessary `as any` casts in score API tests and documents why the legacy-schema test uses a local in-memory DB. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… them. Previously, a single transient failure in ensureUserStatsSchema could leave the shared promise cached for the process lifetime, causing subsequent calls to return the failed run and skip needed DDL. Reset the promise when any helper did not complete, and add an isolated test for the retry behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…columns. - Only reload the page after sign-out when getSession confirms the session is gone without an error, preventing reloads due to network/server errors. - In completeChallengeAndAwardXP and updateUserXPAndLevel, only include the login reward and notification preference columns when their migrations succeeded, preventing insert failures on partially migrated schemas. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ly insert columns. Replace per-feature schema ensure helpers with the centralized ensureUserStatsSchema() so concurrent callers serialize on the same migration promise. Insert user_stats rows with login_reward and preference columns only when those migrations succeeded, avoiding transient DDL failures that silently abort streak updates. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Track keyboard and touch held keys separately so releasing the D-pad does not cancel a key still physically held by the keyboard, and vice versa. Adds overlap tests for both source combinations. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch fixes several game renderer and initialization issues across the platform:
AchievementNotificationtype and removes duplicate global declarations.Commits
6c128effix: consolidate window globals and preserve Memory Matrix focus.2516873fix: preserve Memory Matrix focus and align game renderer tests.2e347e2fix: centralize AchievementNotification type and remove duplicate global declarations.ea2d74efix: add Evader active-state test and set Memory Matrix card role.c229fc0fix: improve Evader touch controls and Memory Matrix accessibility.fca8a7cfix: gate Evader touch controls on active state and tighten cleanup test.d15f468fix: update game renderers and add missing tests for bubble-shooter, evader, and memory-matrixTest plan
bun run test:runpasses (116 test files, 2444 tests)Generated with Devin
Summary by CodeRabbit
New Features
Bug Fixes