Skip to content

refactor: add TRPC, simplify folder structure and add basic error request handling#15

Merged
mattiacerutti merged 6 commits intomainfrom
trpc-refactor
Nov 21, 2025
Merged

refactor: add TRPC, simplify folder structure and add basic error request handling#15
mattiacerutti merged 6 commits intomainfrom
trpc-refactor

Conversation

@mattiacerutti
Copy link
Copy Markdown
Owner

@mattiacerutti mattiacerutti commented Nov 21, 2025

Summary by CodeRabbit

  • New Features

    • Adds a tRPC-based API layer and provider for improved backend communication.
  • Architecture

    • Migrated backend calls to the new tRPC procedures and reorganized internal modules.
    • Removed the previous rate-limiting middleware.
  • Refactor

    • Reworked game snippet flow into a dedicated hook, simplifying state and fetch logic.
    • Game UI: shows loading and error states and enforces a next-button lock/cadence.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 21, 2025

Walkthrough

Adds TRPC (server + client) and a TRPC React provider, replaces two REST snippet endpoints with TRPC procedures, introduces a useGameSnippets hook and related type reorganizations, removes in-repo rate limiter and client fetch adapter, and updates many import paths to a feature-scoped shared types layout.

Changes

Cohort / File(s) Change Summary
Package Dependencies
package.json
Added @tanstack/react-query, @trpc/client, @trpc/react-query, @trpc/server, and zod.
TRPC Server Boot & Routers
src/server/trpc/trpc.ts, src/server/trpc/context.ts, src/server/trpc/routers/index.ts, src/server/trpc/routers/snippet.ts
New TRPC bootstrap, typed context, app router and snippet router with languages and random procedures.
TRPC Route
src/app/api/trpc/[trpc]/route.ts
New Next.js route delegating GET/POST to TRPC fetchRequestHandler.
TRPC React Provider
src/components/providers/trpc-provider.tsx, src/app/layout.tsx
New TRPCReactProvider and api instance; integrated provider into root layout.
Game Snippets Hook & Game Page
src/features/game/hooks/useGameSnippets.ts, src/app/game/page.tsx
New useGameSnippets hook (language/snippet fetching, queue, next-button lock); game page refactored to use the hook, added loading/error UI and event wiring.
Type Migration (shared → features/shared)
multiple files (see examples)
src/features/shared/types/snippet.ts, src/shared/types/snippet.ts, prisma/seed/*, src/features/*...
Moved/added snippet types under src/features/shared/types/*; removed old src/shared/types/snippet.ts; updated many import paths across game, typing, settings, and snippet logic to use @/features/shared/types/....
Client Fetch Adapter Removal
src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts
Removed client-side fetch adapter that called former REST endpoints.
Service Refactor (client-side)
src/features/snippets/services/get-random-snippets.client.ts
Replaced async fetch function with synchronous buildClientSnippets that transforms provided raw snippets.
Repository / Prisma Import Updates
src/features/snippets/infrastructure/repositories/*.ts, prisma/seed/*.ts
Updated Prisma import to @/server/prisma and adjusted type import paths in seed files.
REST Endpoint Removal
src/app/api/snippets/languages/route.ts, src/app/api/snippets/random/route.ts
Removed GET REST endpoints for languages and random snippets (replaced by TRPC).
Rate Limiter Removal
src/core/middleware/rate-limiter.ts
Removed in-memory rate-limiting middleware implementation.
Proxy Header Enforcement
src/proxy.ts
Removed isDev conditional; now always enforces x-internal-api: true header.
Misc. Import Path Updates
src/app/game/layout.tsx, src/features/game/components/*, src/features/game/logic/*, src/features/game/hooks/*, src/features/settings/*, etc.
Updated various component/type import paths to feature-scoped locations (minor/no logic changes).
Config Change
src/core/config/env.ts
Removed exported isDev constant.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant useGameSnippets
    participant TRPC as TRPC Query
    participant Server
    participant DB as Database

    Client->>useGameSnippets: mount / activateLanguage / changeSnippet
    activate useGameSnippets

    useGameSnippets->>TRPC: query snippet.languages
    activate TRPC
    TRPC->>Server: snippet.languages
    Server->>DB: getLanguages()
    DB-->>Server: languages
    Server-->>TRPC: languages
    TRPC-->>useGameSnippets: availableLanguages
    deactivate TRPC

    useGameSnippets->>TRPC: query snippet.random(languageId)
    activate TRPC
    TRPC->>Server: snippet.random
    Server->>DB: getRandomSnippets()
    DB-->>Server: rawSnippets
    Server-->>TRPC: rawSnippets
    TRPC-->>useGameSnippets: rawSnippets
    deactivate TRPC

    useGameSnippets->>useGameSnippets: buildClientSnippets(rawSnippets)
    useGameSnippets-->>Client: activeSnippet / isNextButtonLocked / error
    deactivate useGameSnippets
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • TRPC infra and provider wiring across server and client (type-safety and header propagation).
  • useGameSnippets hook: queue management, async prefetch, locking and integration with game state.
  • Game page refactor: verify behavioral parity and UI states (loading/error/finished).
  • Areas to focus:
    • src/features/game/hooks/useGameSnippets.ts
    • src/server/trpc/routers/snippet.ts
    • src/components/providers/trpc-provider.tsx
    • src/app/game/page.tsx
    • src/proxy.ts (header enforcement effects)

Possibly related PRs

Poem

🐰 I hopped through types and endpoints new,

REST routes leapt to TRPC view,
Hooks now hum and queues align,
Shared paths nested, imports fine,
A carrot-coded cheer — hop, compile! 🥕

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 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 three main changes: adding TRPC infrastructure, reorganizing imports to a feature-scoped folder structure, and removing legacy API routes with header-based error handling.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch trpc-refactor

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 753305a and b4c24ce.

📒 Files selected for processing (1)
  • src/features/game/hooks/useGameSnippets.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/features/game/hooks/useGameSnippets.ts

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (8)
src/components/providers/trpc-provider.tsx (1)

26-50: Consider adding an error boundary.

The TRPC provider should be wrapped with an error boundary to gracefully handle query/mutation failures and prevent the entire app from crashing.

Consider adding an error boundary component around the provider or within it:

import {ErrorBoundary} from 'react-error-boundary';

export function TRPCReactProvider({children}: {children: ReactNode}) {
  const [queryClient] = useState(() => createQueryClient());
  const [trpcClient] = useState(() =>
    api.createClient({
      links: [
        httpBatchLink({
          url: "/api/trpc",
          headers() {
            return {
              "x-internal-api": "true",
            };
          },
        }),
      ],
    })
  );

  return (
    <ErrorBoundary fallback={<div>Something went wrong. Please refresh the page.</div>}>
      <QueryClientProvider client={queryClient}>
        <api.Provider client={trpcClient} queryClient={queryClient}>
          {children}
        </api.Provider>
      </QueryClientProvider>
    </ErrorBoundary>
  );
}
src/app/layout.tsx (1)

15-17: Consider adding React Query DevTools for development.

Adding React Query DevTools can significantly improve the debugging experience during development.

Install and add the DevTools (only in development):

npm install @tanstack/react-query-devtools

Then update the provider:

import { ReactQueryDevtools } from '@tanstack/react-query-devtools';

// Inside TRPCReactProvider's return:
<QueryClientProvider client={queryClient}>
  <api.Provider client={trpcClient} queryClient={queryClient}>
    {children}
    {process.env.NODE_ENV === 'development' && <ReactQueryDevtools initialIsOpen={false} />}
  </api.Provider>
</QueryClientProvider>
src/server/trpc/routers/snippet.ts (1)

11-26: Re‑evaluate random as a mutation and parity with the old REST route

  • This endpoint is read‑only; consider making it a query instead of a mutation to better match tRPC semantics (and to simplify client usage via useQuery) unless you explicitly want to avoid caching for randomness.
  • The legacy REST handler for /api/snippets/random applied rateLimit(request) and wrapped getRandomSnippets in a try/catch to return a generic 500 error. Here, there’s no equivalent rate‑limit or error shaping; any thrown Error will surface as an internal tRPC error with its message. If you still need throttling and generic error messages, you may want a small middleware or local try/catch that throws a TRPCError with a generic message instead.
src/features/game/hooks/useGameSnippets.ts (2)

26-30: Expose a consistent error object instead of mixing boolean and error

languagesError comes from isError (boolean), while randomError is an actual error object. Using languagesError || randomError means error can be either true or an error instance, which is awkward to consume.

You can keep the shape consistent by destructuring error from the query instead:

-  const {data: availableLanguages, isError: languagesError} = api.snippet.languages.useQuery();
+  const {data: availableLanguages, error: languagesError} = api.snippet.languages.useQuery();
   const {error: randomError, ...fetchRandomSnippets} = api.snippet.random.useMutation();

-  const error = languagesError || randomError;
+  const error = languagesError ?? randomError;

This way error is always an error (or null/undefined), and the caller doesn’t have to special‑case booleans.


31-40: Consider guarding and surfacing failures in background snippet refills

fetchSnippetsForLanguage and refillSnippetQueue rely on mutateAsync, which rejects on network/server errors. In refillSnippetQueue, those rejections will propagate to the fire‑and‑forget promise stored in backgroundFetchPromise, potentially causing unhandled rejections and leaving backgroundFetchPromise.current stuck in a rejected state.

You can make this more robust by handling errors and always clearing the ref:

-  const refillSnippetQueue = async (language: ILanguage) => {
-    const snippets = await fetchSnippetsForLanguage(language);
-    addSnippetsToQueue(snippets);
-  };
+  const refillSnippetQueue = async (language: ILanguage) => {
+    const snippets = await fetchSnippetsForLanguage(language);
+    addSnippetsToQueue(snippets);
+  };
+
+  const startBackgroundRefill = (language: ILanguage) => {
+    backgroundFetchPromise.current = refillSnippetQueue(language)
+      .catch((error) => {
+        // optional: log or hook into some error reporting
+        console.error("Background snippet refill failed", error);
+      })
+      .finally(() => {
+        backgroundFetchPromise.current = null;
+      });
+  };

Then use startBackgroundRefill(language) in changeSnippet instead of assigning the raw promise. This avoids unhandled rejections and ensures the ref is reset once the background work finishes.

src/features/snippets/services/get-random-snippets.client.ts (1)

1-16: Snippet mapping helper looks good; only minor readability nit (optional).

The client/server snippet type separation and buildClientSnippets composition over buildClientSnippet are sound and type-safe. If you want, you could spread the map(...).filter(...) pipeline over multiple lines for readability, but it’s not required.

src/app/game/page.tsx (2)

61-72: Async language activation isn’t awaited/caught; consider explicit loading and error handling.

Both handleChangeLanguage and activateLanguageEvent call the async activateLanguage without await or error handling. If activateLanguage (TRPC mutation + initialization) rejects, this can surface as an unhandled promise rejection while you separately rely on the hook’s error to render the error UI.

You might want to:

  • Set GameStatus.LOADING when changing language, to match the snippet-change UX.
  • await activateLanguage in user-initiated flows (and optionally in the effect wrapper) and handle errors explicitly (even if just with a try/catch that lets the hook’s error drive the UI), to avoid unhandled rejections.

For example:

-const handleChangeLanguage = async (selectedLanguage: ILanguage) => {
-  resetStopwatch();
-  activateLanguage(selectedLanguage);
-  track("LANGUAGE_CHANGE", {language: selectedLanguage.name});
-};
+const handleChangeLanguage = async (selectedLanguage: ILanguage) => {
+  resetStopwatch();
+  setStatus(GameStatus.LOADING);
+  try {
+    await activateLanguage(selectedLanguage);
+    track("LANGUAGE_CHANGE", {language: selectedLanguage.name});
+  } catch {
+    // error state is already propagated via useGameSnippets().error
+  }
+};

and similarly wrapping activateLanguageEvent in a try/catch with await if you want initial-load errors to be fully controlled rather than bubbling as unhandled rejections.

Also applies to: 75-93


94-111: Guard against empty snippet list when reparsing on autoclosing changes.

On autoclosing-mode changes you reparse [activeSnippet, ...gameState.getSnippetQueue()], filter out any null results from buildClientSnippet, and then call initialize(activeLanguage, reparsedSnippets). If all reparses fail (even if unlikely), reparsedSnippets will be empty and initialize will throw (Cannot initialize game with empty snippets), taking down the effect.

Consider short‑circuiting when the filtered list is empty, or handling that case more gracefully, e.g.:

-const reparsedSnippets = snippetsToReparse
-  .map((snippet) => buildClientSnippet(snippet.rawSnippet, autoClosingMode !== AutoClosingMode.DISABLED))
-  .filter((snippet): snippet is IClientSnippet => snippet !== null);
-
-initialize(activeLanguage, reparsedSnippets);
+const reparsedSnippets = snippetsToReparse
+  .map((snippet) => buildClientSnippet(snippet.rawSnippet, autoClosingMode !== AutoClosingMode.DISABLED))
+  .filter((snippet): snippet is IClientSnippet => snippet !== null);
+
+if (reparsedSnippets.length === 0) {
+  // Optionally set an error state or just restore previous status instead of throwing.
+  setStatus(GameStatus.FINISHED);
+  return;
+}
+
+initialize(activeLanguage, reparsedSnippets);

Adjust the fallback behavior to whatever UX you prefer, but avoiding the throw will keep the app more resilient.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ad4162 and 753305a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (40)
  • package.json (2 hunks)
  • prisma/seed/files.ts (1 hunks)
  • prisma/seed/languages.ts (1 hunks)
  • src/app/api/snippets/languages/route.ts (0 hunks)
  • src/app/api/snippets/random/route.ts (0 hunks)
  • src/app/api/trpc/[trpc]/route.ts (1 hunks)
  • src/app/game/layout.tsx (1 hunks)
  • src/app/game/page.tsx (2 hunks)
  • src/app/layout.tsx (2 hunks)
  • src/components/providers/trpc-provider.tsx (1 hunks)
  • src/core/config/env.ts (0 hunks)
  • src/core/middleware/rate-limiter.ts (0 hunks)
  • src/features/game/components/game-view.tsx (1 hunks)
  • src/features/game/components/typing-area/character.tsx (1 hunks)
  • src/features/game/components/typing-area/index.tsx (1 hunks)
  • src/features/game/hooks/useGameSnippets.ts (1 hunks)
  • src/features/game/hooks/useTyping.ts (1 hunks)
  • src/features/game/logic/game-logic.ts (1 hunks)
  • src/features/game/logic/typing/add-character.ts (1 hunks)
  • src/features/game/logic/typing/delete-character.ts (1 hunks)
  • src/features/game/logic/typing/delete-line.ts (1 hunks)
  • src/features/game/logic/typing/delete-word.ts (1 hunks)
  • src/features/game/logic/typing/shared.ts (1 hunks)
  • src/features/game/state/game-store.ts (1 hunks)
  • src/features/game/utils/character.ts (1 hunks)
  • src/features/game/utils/typing-metrics.ts (1 hunks)
  • src/features/settings/stores/settings-store.ts (1 hunks)
  • src/features/shared/types/snippet.ts (1 hunks)
  • src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts (0 hunks)
  • src/features/snippets/infrastructure/repositories/language.repository.server.ts (1 hunks)
  • src/features/snippets/infrastructure/repositories/snippet.repository.server.ts (1 hunks)
  • src/features/snippets/logic/parsing/snippet-parse.client.ts (1 hunks)
  • src/features/snippets/services/get-random-snippets.client.ts (2 hunks)
  • src/features/snippets/services/get-random-snippets.server.ts (1 hunks)
  • src/proxy.ts (1 hunks)
  • src/server/trpc/context.ts (1 hunks)
  • src/server/trpc/routers/index.ts (1 hunks)
  • src/server/trpc/routers/snippet.ts (1 hunks)
  • src/server/trpc/trpc.ts (1 hunks)
  • src/shared/types/snippet.ts (0 hunks)
💤 Files with no reviewable changes (6)
  • src/core/middleware/rate-limiter.ts
  • src/shared/types/snippet.ts
  • src/app/api/snippets/random/route.ts
  • src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts
  • src/core/config/env.ts
  • src/app/api/snippets/languages/route.ts
🧰 Additional context used
🧬 Code graph analysis (24)
src/features/snippets/infrastructure/repositories/snippet.repository.server.ts (1)
src/shared/db/prisma.ts (1)
  • base (8-14)
src/components/providers/trpc-provider.tsx (1)
src/server/trpc/routers/index.ts (1)
  • AppRouter (8-8)
src/features/game/logic/typing/add-character.ts (1)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/server/trpc/routers/index.ts (2)
src/server/trpc/trpc.ts (1)
  • createTRPCRouter (6-6)
src/server/trpc/routers/snippet.ts (1)
  • snippetRouter (7-27)
src/server/trpc/trpc.ts (1)
src/server/trpc/context.ts (1)
  • TRPCContext (13-13)
src/features/game/components/game-view.tsx (1)
src/shared/types/language.ts (1)
  • ILanguage (1-6)
src/app/layout.tsx (2)
src/components/providers/trpc-provider.tsx (1)
  • TRPCReactProvider (26-50)
src/shared/components/theme-provider.tsx (1)
  • ThemeProvider (5-11)
src/app/api/trpc/[trpc]/route.ts (2)
src/server/trpc/routers/index.ts (1)
  • appRouter (4-6)
src/server/trpc/context.ts (1)
  • createTRPCContext (7-11)
prisma/seed/files.ts (1)
src/shared/types/language.ts (1)
  • ILanguage (1-6)
src/features/snippets/logic/parsing/snippet-parse.client.ts (1)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/features/snippets/infrastructure/repositories/language.repository.server.ts (1)
src/shared/db/prisma.ts (1)
  • base (8-14)
src/features/game/logic/typing/delete-line.ts (1)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/features/game/components/typing-area/index.tsx (1)
src/shared/types/character.ts (5)
  • CharacterTypes (13-18)
  • INormalCharacter (25-27)
  • IBaseCharacter (20-23)
  • IWhitespaceCharacter (35-38)
  • WhitespaceTypes (7-11)
src/features/game/hooks/useGameSnippets.ts (5)
src/features/game/state/game-store.ts (1)
  • useGameStore (42-150)
src/components/providers/trpc-provider.tsx (1)
  • api (9-9)
src/features/shared/types/language.ts (1)
  • ILanguage (1-6)
src/features/snippets/services/get-random-snippets.client.ts (1)
  • buildClientSnippets (14-16)
src/features/game/config/game.ts (1)
  • REFRESH_BUTTON_MIN_DELAY (10-10)
prisma/seed/languages.ts (1)
src/shared/types/language.ts (1)
  • ILanguage (1-6)
src/features/game/hooks/useTyping.ts (1)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/features/snippets/services/get-random-snippets.server.ts (1)
src/shared/types/snippet.server.ts (1)
  • ISnippet (1-4)
src/server/trpc/routers/snippet.ts (5)
src/server/trpc/trpc.ts (2)
  • createTRPCRouter (6-6)
  • publicProcedure (7-7)
src/features/snippets/infrastructure/repositories/language.repository.server.ts (2)
  • getLanguages (14-25)
  • doesLanguageExist (4-12)
src/features/snippets/services/get-random-snippets.server.ts (1)
  • getRandomSnippets (7-35)
src/app/api/snippets/random/route.ts (1)
  • GET (6-32)
src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts (1)
  • fetchRandomSnippets (8-20)
src/features/game/logic/game-logic.ts (1)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/features/game/components/typing-area/character.tsx (1)
src/shared/types/character.ts (3)
  • IBaseCharacter (20-23)
  • INormalCharacter (25-27)
  • CharacterTypes (13-18)
src/app/game/page.tsx (4)
src/features/game/state/game-store.ts (1)
  • useGameStore (42-150)
src/features/game/hooks/useGameSnippets.ts (1)
  • useGameSnippets (12-86)
src/features/shared/types/language.ts (1)
  • ILanguage (1-6)
src/features/snippets/services/get-random-snippets.client.ts (1)
  • buildClientSnippet (5-12)
src/app/game/layout.tsx (1)
src/shared/components/theme-toggle.tsx (6)
  • ThemeToggle (7-49)
  • isMounted (19-22)
  • frame (12-15)
  • setIsMounted (13-13)
  • cancelAnimationFrame (14-14)
  • e (30-30)
src/features/shared/types/snippet.ts (3)
src/features/shared/types/character.ts (1)
  • ICharacter (45-45)
src/features/shared/types/snippet.server.ts (1)
  • ISnippet (1-4)
src/shared/types/snippet.ts (1)
  • ISnippet (6-9)
src/features/settings/stores/settings-store.ts (1)
src/shared/types/language.ts (1)
  • ILanguage (1-6)
🔇 Additional comments (40)
src/features/game/utils/typing-metrics.ts (1)

1-2: Import refactoring verified successfully.

All imports across the codebase have been consistently updated from @/shared/types/ to @/features/shared/types/. Zero old import paths remain, confirming complete and uniform migration. The file under review is correctly updated.

prisma/seed/files.ts (1)

1-1: LGTM! Consistent import path refactor.

The import path update from @/shared/types/language to @/features/shared/types/language aligns with the folder structure reorganization across the codebase.

src/features/game/components/game-view.tsx (1)

4-4: LGTM! Consistent type import path update.

The import path update maintains type consistency with the folder restructuring.

prisma/seed/languages.ts (1)

1-1: LGTM! Consistent import path update.

src/features/settings/stores/settings-store.ts (1)

1-1: LGTM! Type import path aligned with restructuring.

src/components/providers/trpc-provider.tsx (3)

11-24: LGTM! Solid retry configuration with exponential backoff.

The retry logic implements exponential backoff (1s → 2s → 4s → capped at 10s) for both queries and mutations, which is appropriate for handling transient failures.


27-28: LGTM! Correct memoization pattern for React 19.

Using useState with initializer functions to memoize QueryClient and TRPC client instances is the correct pattern for React 19, preventing unnecessary re-creation on re-renders.


33-37: Server-side validation confirmed—no action required.

The header is properly validated in src/proxy.ts (lines 5–10): the proxy function checks that the "x-internal-api" header equals "true" and returns a 403 Forbidden response if absent or invalid. This provides the required security check.

src/app/game/layout.tsx (1)

4-4: LGTM! Import path aligned with component reorganization.

src/app/layout.tsx (2)

15-17: LGTM! Correct provider nesting establishes TRPC context.

The provider hierarchy (TRPCReactProvider > ThemeProvider > children) is correct, ensuring TRPC and React Query contexts are available throughout the app.


3-4: LGTM! Import paths aligned with provider reorganization.

package.json (2)

53-53: The review comment is incorrect—zod v4.1.12 is a valid, stable release.

The npm registry confirms that 4.1.12 is the current stable version of zod, not a non-existent version. The package.json dependency "zod": "^4.1.12" is correct and valid. There are also pre-release versions of 4.2.0 available as canary builds (dated November 2025). The original review was based on outdated information.

Likely an incorrect or invalid review comment.


22-25: All package versions are secure.

Verification found a HIGH severity tRPC WebSocket DoS vulnerability affecting versions < 11.1.1, but the PR uses version 11.7.1 for all tRPC packages, which is patched and safe. No vulnerabilities detected for React Query or other tRPC dependencies.

src/features/snippets/services/get-random-snippets.server.ts (1)

1-1: LGTM! Clean import path refactor.

The import path update aligns with the folder structure simplification objective.

src/features/game/hooks/useTyping.ts (1)

8-8: LGTM! Import path updated correctly.

The type import path has been successfully migrated to the feature-scoped location.

src/features/game/components/typing-area/character.tsx (1)

1-1: LGTM! Character type imports updated.

The import path refactor is consistent with the folder structure reorganization.

src/features/game/logic/game-logic.ts (1)

1-2: LGTM! Multiple type imports migrated successfully.

Both character and snippet type imports have been updated to the feature-scoped paths, maintaining type safety while supporting the folder structure refactor.

src/features/game/components/typing-area/index.tsx (1)

3-3: LGTM! Character type imports relocated.

The import path update maintains functionality while aligning with the new feature-scoped structure.

src/features/game/logic/typing/delete-character.ts (1)

1-2: LGTM! Type imports updated consistently.

Both character and snippet type imports have been successfully migrated to feature-scoped paths without affecting the deletion logic.

src/features/game/logic/typing/shared.ts (1)

2-3: LGTM! Shared typing utilities updated.

Import paths successfully migrated while preserving all helper function implementations.

src/features/game/logic/typing/delete-line.ts (1)

3-3: Migration verification complete—no issues found.

Verification confirms:

  • ✓ No old @/shared/types/snippet or @/shared/types/character imports remain
  • ✓ New @/features/shared/types paths are in use across 18+ files
  • ✓ File src/features/game/logic/typing/delete-line.ts line 3 contains the correct import: import {IParsedSnippet} from "@/features/shared/types/snippet";

The import path refactor is complete and successful.

src/features/snippets/logic/parsing/snippet-parse.client.ts (1)

1-4: LGTM! Clean import path refactor.

The import paths have been successfully updated to the new feature-scoped structure. This aligns with the PR's objective to reorganize shared types.

src/features/game/state/game-store.ts (1)

2-3: LGTM! Import paths correctly updated.

The type imports have been successfully migrated to the feature-scoped module structure.

src/features/game/utils/character.ts (1)

1-2: LGTM! Import paths updated correctly.

The character type imports have been successfully migrated to the new feature-scoped structure.

src/features/game/logic/typing/delete-word.ts (2)

1-1: LGTM! Import paths refactored correctly.

The character type imports have been successfully updated to use the feature-scoped module path.


4-4: LGTM! Import path updated correctly.

The snippet type import has been successfully migrated to the feature-scoped structure.

src/features/snippets/infrastructure/repositories/language.repository.server.ts (1)

1-2: Import paths updated correctly.

The Prisma client and type imports have been migrated to the new module structure. The Prisma client path verification from the previous file review applies here as well.

src/features/game/logic/typing/add-character.ts (3)

3-4: LGTM! Import paths updated correctly.

The type imports have been successfully migrated to the feature-scoped module structure.


5-5: Good addition! Immer improves immutability handling.

Introducing Immer is a solid choice for managing immutable state updates. It provides cleaner syntax and better performance compared to manual deep cloning.


68-81: LGTM! Clean refactor using Immer's produce.

The immutable update logic is correctly implemented using Immer's produce. The draft mutation approach is more readable and less error-prone than manual cloning while maintaining the same behavior.

src/features/snippets/infrastructure/repositories/snippet.repository.server.ts (1)

1-1: Import path verified and correctly configured.

The new Prisma client path @/server/prisma exists and exports the prisma instance with the required prismaRandom() extension applied. The export matches the import statement in the file, confirming the migration is valid.

src/proxy.ts (1)

5-12: This review comment is based on incorrect assumptions about the code change.

The premise that "previously, this check was skipped in development mode" has no factual basis. The code history shows:

  • The internal API header check was introduced in April 2025 with commit 0c22d9900121333ed6a0e7e2e900a4e2cae7c689 and applies uniformly across all environments—there is no development bypass logic present or recently removed.
  • The most recent change to src/proxy.ts was a file structure refactor (not a logic change).
  • There are no environment-specific conditions (process.env.NODE_ENV, isDev, etc.) in the proxy file.

Development workflows are properly supported because the TRPC provider (configured in src/components/providers/trpc-provider.tsx and used in the root layout) automatically sets the "x-internal-api": "true" header on all requests across all environments. This means the header requirement is transparent to developers and does not require special handling.

Likely an incorrect or invalid review comment.

src/server/trpc/routers/snippet.ts (1)

8-10: Languages query looks good

Thin wrapper over getLanguages() is straightforward and keeps the router lean; types and async handling are correct.

src/features/shared/types/snippet.ts (1)

1-9: Shared snippet types are clear and cohesive

The split between IServerSnippet and the client‑side ISnippet (with parsedSnippet) is well‑structured and will make game logic typing simpler.

src/app/api/trpc/[trpc]/route.ts (1)

1-14: TRPC API handler wiring looks correct

fetchRequestHandler is configured properly with endpoint, appRouter, and createTRPCContext, and re‑exporting it as both GET and POST aligns with tRPC’s expectations.

src/server/trpc/trpc.ts (1)

1-8: Typed TRPC bootstrap is idiomatic

Using initTRPC.context<TRPCContext>().create() and re‑exporting router, procedure, and mergeRouters keeps router files clean and consistent.

src/server/trpc/routers/index.ts (1)

1-8: Root appRouter composition is fine

Mounting snippetRouter under appRouter.snippet and exporting AppRouter for client typing (createTRPCReact<AppRouter>()) is exactly what you need here.

src/server/trpc/context.ts (1)

1-13: Context shape is minimal but correct

Returning just { req } is fine for now and the TRPCContext alias via Awaited<ReturnType<...>> will tolerate future additions (e.g. user/session) without ripple effects.

src/app/game/page.tsx (2)

3-16: General refactor and stopwatch/snippet wiring look solid.

Using useGameSnippets, GameStatus, and buildClientSnippet here simplifies the page nicely, and the renamed elapsed parameter in onTick/pushPositionSample keeps semantics clear while still reading the latest game-store state via useGameStore.getState(). No issues spotted in these parts.

Also applies to: 30-38, 40-42


112-145: Status/error gating keeps availableLanguages! safe; UI branches are consistent.

The FINISHED, error, and LOADING branches ensure you don’t hit <GameView> until status !== LOADING and no error is present. Given that the GameStatus.LOADINGREADY transition only happens after initialize (which depends on availableLanguages), the non-null assertion availableLanguages! is effectively protected by this flow. This wiring looks coherent and doesn’t introduce obvious race conditions in the render path.

Comment on lines +48 to +77
const changeSnippet = async () => {
const snippetQueue = getSnippetQueue();

const startTime = Date.now();
setIsNextButtonLocked(true);

if (snippetQueue.length === 0) {
setStatus(GameStatus.LOADING);

if (backgroundFetchPromise.current) {
await backgroundFetchPromise.current;
backgroundFetchPromise.current = null;
} else if (language) {
await activateLanguage(language);
setIsNextButtonLocked(false);
return;
}
} else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) {
backgroundFetchPromise.current = refillSnippetQueue(language!);
}

const elapsedTime = Date.now() - startTime;
const remainingTime = Math.max(REFRESH_BUTTON_MIN_DELAY - elapsedTime, 0);

goToNextSnippet();

setTimeout(() => {
setIsNextButtonLocked(false);
}, remainingTime);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Bug: queue exhaustion can throw when background refill has already completed

changeSnippet has a subtle queue/async bug:

  • When the queue length drops to <= 3, you start a background refill:
    } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) {
      backgroundFetchPromise.current = refillSnippetQueue(language!);
    }
  • backgroundFetchPromise.current is never reset to null after that promise resolves (unless you later hit the snippetQueue.length === 0 branch while it’s still truthy).
  • After consuming all snippets from both the initial batch and that refill, getSnippetQueue() will eventually return an empty array, but backgroundFetchPromise.current will still be a resolved promise.
  • On the next changeSnippet() call with an empty queue:
    • snippetQueue.length === 0 is true.
    • backgroundFetchPromise.current is truthy, so you await it and then set it to null without starting a new fetch.
    • The underlying store queue is still empty, and you then call goToNextSnippet(), which will throw "Cannot go to next snippet when the queue is empty".

A targeted fix is to ensure the background promise clears itself when it finishes and to avoid treating an already‑resolved promise as “in‑flight”:

-    if (snippetQueue.length === 0) {
-      setStatus(GameStatus.LOADING);
-
-      if (backgroundFetchPromise.current) {
-        await backgroundFetchPromise.current;
-        backgroundFetchPromise.current = null;
-      } else if (language) {
-        await activateLanguage(language);
-        setIsNextButtonLocked(false);
-        return;
-      }
-    } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) {
-      backgroundFetchPromise.current = refillSnippetQueue(language!);
-    }
+    if (snippetQueue.length === 0) {
+      setStatus(GameStatus.LOADING);
+
+      if (backgroundFetchPromise.current) {
+        // Wait for any in‑flight refill to complete; the ref will be cleared in its .finally()
+        await backgroundFetchPromise.current;
+      } else if (language) {
+        await activateLanguage(language);
+        setIsNextButtonLocked(false);
+        return;
+      }
+    } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current && language) {
+      // Kick off a background refill and reset the ref when done
+      backgroundFetchPromise.current = refillSnippetQueue(language).finally(() => {
+        backgroundFetchPromise.current = null;
+      });
+    }

This guarantees that once you’ve drained both the initial and refilled snippets, the next “empty queue” transition will either (a) wait for an actually in‑flight refill, or (b) re‑initialize via activateLanguage, instead of calling goToNextSnippet() on an empty queue.

🤖 Prompt for AI Agents
In src/features/game/hooks/useGameSnippets.ts around lines 48–77, the
changeSnippet flow treats a resolved backgroundFetchPromise as if it were still
"in-flight", causing an empty-queue call to goToNextSnippet and throw; fix by
ensuring backgroundFetchPromise.current is cleared when the refill promise
settles (assign the promise to backgroundFetchPromise.current but attach a
.finally handler that sets it to null) so awaiting it only represents an active
refill, and additionally after awaiting if the queue is still empty then trigger
the fallback path (e.g., call activateLanguage or set status/loading) instead of
immediately calling goToNextSnippet.

@mattiacerutti mattiacerutti merged commit 02232b7 into main Nov 21, 2025
4 checks passed
@mattiacerutti mattiacerutti deleted the trpc-refactor branch November 23, 2025 18:50
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.

1 participant