Skip to content

Conversation

@goofmint
Copy link
Owner

@goofmint goofmint commented Nov 22, 2025

  • Add D1 database schema migration for scores table
  • Create result/create route to save game scores to D1
  • Create result/$id route to display saved game results
  • Update play route to submit scores using useFetcher
  • Add nanoid utility for generating unique IDs
  • Track all solved issues across game sessions for accurate statistics
  • Support player name registration on result page

Features:

  • Automatic score saving when game ends
  • Result page with score, accuracy, and statistics
  • Share result URL functionality
  • Optional player name registration

Summary by CodeRabbit

  • New Features

    • Game scores are automatically saved at the end of a run and a results page is created showing score, accuracy, issues found, code language, and timestamp.
    • You can set or update your player name, share results, copy links, and replay from the results page.
    • Progress across all problems is now tracked for session-wide accuracy.
  • Bug Fixes / UX

    • Shows "Saving score…" during submission and surfaces save errors when they occur.

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

- Add D1 database schema migration for scores table
- Create result/create route to save game scores to D1
- Create result/$id route to display saved game results
- Update play route to submit scores using useFetcher
- Add nanoid utility for generating unique IDs
- Track all solved issues across game sessions for accurate statistics
- Support player name registration on result page

Features:
- Automatic score saving when game ends
- Result page with score, accuracy, and statistics
- Share result URL functionality
- Optional player name registration
@coderabbitai
Copy link

coderabbitai bot commented Nov 22, 2025

Walkthrough

Adds end-game result persistence and display: Play now tracks all solved issue IDs, submits aggregated results to a new /result/create endpoint, and navigates to a persisted Result page. New DB migration, nanoid utility, and route entries support score storage and viewing.

Changes

Cohort / File(s) Change Summary
Game State & Result Submission
app/routes/$lang.$codeLanguage.play.tsx
Added allSolvedIssueIds to GameState; accumulate global solved IDs on correct taps; compute total issues and accuracy at game end; submit result payload to /result/create via fetcher with duplicate-submission guard; display "Saving score…" and navigate to /result/{id} on success; surface submission errors.
Result Page & Actions
app/routes/result.$id.tsx
New result route: loader fetches ScoreRecord by id (handles 404/500), action updates player_name (validates 1–50 chars), and component renders localized result UI with share, copy, and player-name form; uses useFetcher for updates.
Result Creation Action
app/routes/result.create.tsx
New POST action: validates Cloudflare D1 availability, parses JSON payload form field into GameResultData, validates types, generates id/timestamp, inserts record into scores table, returns { success: true, id }, and returns 400/500 on errors.
ID Utility
app/utils/nanoid.ts
New nanoid() utility: uses crypto.randomUUID() when available, otherwise fallback combining monotonic counter, base36 timestamp, and random segments to form a unique id.
Database Migration
migrations/0001_create_scores_table.sql
New migration creating scores table (id, score, issues_found, total_issues, accuracy, ui_language, code_language, player_name, created_at, llm_feedback) and indexes on created_at, score, and code_language (IF NOT EXISTS).
Route Configuration
app/routes.ts
Added routes: result/create -> routes/result.create.tsx and result/:id -> routes/result.$id.tsx.

Sequence Diagram(s)

sequenceDiagram
    participant Player as Player (Browser)
    participant Play as Play Component
    participant Fetcher as Fetcher
    participant ResultAPI as /result/create
    participant DB as D1 Database
    participant ResultPage as /result/:id

    Player->>Play: Finish game
    Play->>Play: Compute totalIssues & accuracy
    Play->>Play: Build resultData (score, issuesFound, totalIssues, accuracy, languages)
    Play->>Fetcher: POST resultData to /result/create
    Fetcher->>ResultAPI: Receive payload
    ResultAPI->>ResultAPI: Validate & generate id, createdAt
    ResultAPI->>DB: INSERT INTO scores
    DB-->>ResultAPI: { success, id }
    ResultAPI-->>Fetcher: Return { success, id }
    Fetcher-->>Play: fetcher.data updated
    Play->>ResultPage: navigate(`/result/${id}`)
    ResultPage->>ResultPage: Loader fetches score by id and renders page
Loading
sequenceDiagram
    participant Browser as Browser
    participant ResultPage as Result Page
    participant Loader as Loader
    participant DB as D1 Database
    participant Player as Player

    Browser->>ResultPage: GET /result/:id
    ResultPage->>Loader: load()
    Loader->>DB: SELECT * FROM scores WHERE id=...
    alt found
        DB-->>Loader: score record
        Loader-->>ResultPage: return { score }
        ResultPage->>Player: render details, show name or name form
    else not found
        Loader-->>ResultPage: 404
        ResultPage->>Player: render not-found
    end
    Player->>ResultPage: submit name (optional)
    ResultPage->>DB: UPDATE player_name
    DB-->>ResultPage: success
    ResultPage->>Player: re-render with name
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to: fetcher submission guard and potential duplicate submissions; validation and error paths in result.create action; loader vs action error handling in result.$id.tsx; nanoid fallback uniqueness and counter wrap behavior; migration column types and index directions.

Possibly related PRs

Poem

🐰 I hopped through lines and found each glitch,
Saved every score without a hitch.
A tiny id, a table filled,
Players cheer — the leaderboard thrilled.
Hop on, play on, bugs meet their match! 🎮✨

Pre-merge checks and finishing touches

✅ Passed checks (3 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: implementing score saving functionality using D1 database, which is the primary focus across multiple new files and routes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 claude/implement-d1-score-saving-01Na71XTJYW9e9Yum5fGiB7k

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

- Change result.create action to return result ID instead of redirect
- Add useNavigate to handle client-side navigation after score save
- Fix 404 error that occurred when game ended
- useFetcher doesn't automatically follow redirects, so we handle navigation manually
Copy link

@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: 2

🧹 Nitpick comments (5)
migrations/0001_create_scores_table.sql (1)

4-15: Consider adding CHECK constraints for data integrity.

The schema would benefit from database-level constraints to ensure data validity:

  1. player_name length: Currently validated only at the application level (line 71 in result.$id.tsx). Adding a constraint ensures consistency even if data is inserted via other paths.
  2. accuracy range: The accuracy field should be between 0 and 1 based on the calculation logic (line 193 in $lang.$codeLanguage.play.tsx).

Apply these constraints to strengthen data integrity:

 CREATE TABLE IF NOT EXISTS scores (
   id TEXT PRIMARY KEY,
   score INTEGER NOT NULL,
   issues_found INTEGER NOT NULL,
   total_issues INTEGER NOT NULL,
-  accuracy REAL NOT NULL,
+  accuracy REAL NOT NULL CHECK (accuracy >= 0 AND accuracy <= 1),
   ui_language TEXT NOT NULL,
   code_language TEXT NOT NULL,
-  player_name TEXT,
+  player_name TEXT CHECK (length(player_name) <= 50),
   created_at TEXT NOT NULL,
   llm_feedback TEXT
 );
app/routes/result.create.tsx (2)

36-41: Consider logging JSON parse errors for debugging.

The catch block for JSON.parse captures the error but doesn't log it. While the error is correctly returned to the client, logging would help with server-side debugging.

   let result: GameResultData;
   try {
     result = JSON.parse(payloadStr);
-  } catch (e) {
+  } catch (error) {
+    console.error('Invalid JSON payload:', error);
     throw data({ error: 'Invalid JSON payload' }, { status: 400 });
   }

43-53: Consider adding range validation for numeric fields.

While type validation is thorough, adding range checks would improve data quality:

  • accuracy should be between 0 and 1
  • score, issuesFound, totalIssues should be non-negative
  • Strings like uiLanguage and codeLanguage could be validated against known values
   // Validate required fields
   if (
     typeof result.score !== 'number' ||
     typeof result.issuesFound !== 'number' ||
     typeof result.totalIssues !== 'number' ||
     typeof result.accuracy !== 'number' ||
     typeof result.uiLanguage !== 'string' ||
     typeof result.codeLanguage !== 'string'
   ) {
     throw data({ error: 'Missing or invalid fields' }, { status: 400 });
   }
+
+  // Validate ranges
+  if (
+    result.score < 0 ||
+    result.issuesFound < 0 ||
+    result.totalIssues < 0 ||
+    result.accuracy < 0 ||
+    result.accuracy > 1
+  ) {
+    throw data({ error: 'Field values out of valid range' }, { status: 400 });
+  }
app/routes/$lang.$codeLanguage.play.tsx (1)

175-213: Score calculation logic is correct; consider minor optimization.

The score-saving implementation correctly:

  • Tracks all solved issues across the entire game session
  • Calculates total issues only from problems actually played
  • Computes accuracy as issuesFound / totalIssues
  • Submits results to the persistence endpoint

The effect runs exactly once when the game ends due to the scoreSaved flag, which is good.

Minor optimization: Lines 179-181 concatenate three getProblems() calls every time. Since this data doesn't change during a game session, you could memoize the problem list or compute it once at the module level.

Optional optimization
+ // Memoize all problems
+ const allProblems = useMemo(
+   () => getProblems(codeLanguage, 1)
+     .concat(getProblems(codeLanguage, 2))
+     .concat(getProblems(codeLanguage, 3)),
+   [codeLanguage]
+ );
+
  // Save score when game ends
  useEffect(() => {
    if (gameEnded && !scoreSaved) {
      // Calculate total issues across all problems
-     const allProblems = getProblems(codeLanguage, 1)
-       .concat(getProblems(codeLanguage, 2))
-       .concat(getProblems(codeLanguage, 3));

      const usedProblems = allProblems.filter((p) =>
        gameState.usedProblemIds.includes(p.id)
      );
app/routes/result.$id.tsx (1)

234-242: Add error handling for clipboard operation.

The clipboard copy button doesn't handle potential failures. The Clipboard API can fail due to permissions, lack of HTTPS, or browser support issues.

              <button
                onClick={() => {
                  const url = `${typeof window !== 'undefined' ? window.location.origin : ''}/result/${score.id}`;
-                 navigator.clipboard.writeText(url);
+                 navigator.clipboard
+                   .writeText(url)
+                   .then(() => {
+                     // Optional: Show success feedback
+                   })
+                   .catch((error) => {
+                     console.error('Failed to copy:', error);
+                     // Fallback: select the input text
+                     document.querySelector('input[readonly]')?.focus();
+                   });
                }}
                className="px-4 py-2 text-sm rounded-md bg-slate-200 dark:bg-slate-700 hover:bg-slate-300 dark:hover:bg-slate-600 transition font-medium whitespace-nowrap"
              >
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 563ef57 and d6acfc5.

📒 Files selected for processing (5)
  • app/routes/$lang.$codeLanguage.play.tsx (9 hunks)
  • app/routes/result.$id.tsx (1 hunks)
  • app/routes/result.create.tsx (1 hunks)
  • app/utils/nanoid.ts (1 hunks)
  • migrations/0001_create_scores_table.sql (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/routes/result.$id.tsx (4)
app/routes/$lang.$codeLanguage.play.tsx (1)
  • loader (29-47)
app/routes/result.create.tsx (1)
  • action (20-86)
app/locales/index.ts (2)
  • SupportedLanguage (8-8)
  • t (21-24)
app/components/Header.tsx (1)
  • Header (9-68)
app/routes/result.create.tsx (2)
app/routes/result.$id.tsx (1)
  • action (55-86)
app/utils/nanoid.ts (1)
  • nanoid (5-15)
app/routes/$lang.$codeLanguage.play.tsx (1)
app/problems/index.ts (1)
  • getProblems (59-67)
🔇 Additional comments (8)
app/utils/nanoid.ts (1)

5-9: LGTM with minor naming consideration.

The function uses crypto.randomUUID() for ID generation, which provides strong entropy and uniqueness guarantees. However, note that crypto.randomUUID() returns standard UUIDs (36 characters with dashes), not the shorter alphanumeric IDs typically associated with the nanoid library. The current implementation is perfectly acceptable for this use case, but the function name might set different expectations.

migrations/0001_create_scores_table.sql (1)

17-20: LGTM! Well-designed indexes for expected query patterns.

The indexes are appropriately chosen for the application's query patterns:

  • idx_scores_created_at supports queries for recent scores
  • idx_scores_score enables efficient ranking/leaderboard queries
  • idx_scores_code_language allows filtering results by programming language

All indexes use IF NOT EXISTS for safe re-execution.

app/routes/result.create.tsx (1)

59-85: LGTM! Database operation is secure and well-structured.

The implementation correctly:

  • Uses parameterized queries to prevent SQL injection
  • Generates unique IDs via nanoid()
  • Creates ISO timestamps for consistency with the schema
  • Handles errors appropriately with logging and structured error responses
  • Redirects to the result page on successful insertion

The parameter binding order matches the schema defined in migrations/0001_create_scores_table.sql.

app/routes/$lang.$codeLanguage.play.tsx (3)

19-20: LGTM! Clear separation of concerns for issue tracking.

The distinction between solvedIssueIds (current problem) and allSolvedIssueIds (all problems) is well-designed and enables accurate statistics across the entire game session.


246-246: LGTM! Correct accumulation of all solved issues.

The implementation properly maintains allSolvedIssueIds across problem transitions while solvedIssueIds is reset for each new problem (line 319). This enables accurate statistics for the entire game session.

Also applies to: 258-258


328-358: LGTM! Good UX with save state feedback.

The implementation correctly:

  • Checks fetcher.state to detect ongoing submission
  • Displays a localized "Saving score..." message during the save operation
  • Provides user feedback that the score is being persisted

This enhances the user experience by making the save operation visible.

app/routes/result.$id.tsx (2)

27-50: LGTM! Robust data fetching with proper error handling.

The loader correctly:

  • Validates database availability
  • Uses parameterized queries for security
  • Returns 404 for missing records (appropriate for shareable result URLs)
  • Logs errors for debugging while returning structured error responses
  • Types the database result with ScoreRecord

55-86: LGTM! Proper validation and secure update operation.

The action implementation correctly:

  • Validates player name presence and type
  • Enforces length constraints (1-50 characters)
  • Uses parameterized queries to prevent SQL injection
  • Returns structured success/error responses

The length validation aligns with the suggested database constraint in the migration file.

Copy link

@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 (4)
app/routes/result.create.tsx (1)

8-15: Runtime validation is good; consider tightening numeric constraints

The type and basic typeof checks are solid, but this will still accept values like Infinity, -1, or accuracy outside [0, 1]. If you care about data quality in the scores table, consider clamping or validating ranges, e.g. Number.isFinite(result.score) and accuracy >= 0 && accuracy <= 1.

app/routes/$lang.$codeLanguage.play.tsx (3)

19-24: New allSolvedIssueIds field fits the stats use case

Tracking allSolvedIssueIds separately from per-problem solvedIssueIds makes the accuracy calculation straightforward and you’re correctly preserving it across problems via ...prev. If your issue IDs are globally unique, this will be accurate; if they’re reused across problems, consider whether you want counts (current behavior) or unique issues (could switch to a Set at save time).


252-270: Global issue tracking update is consistent with the new state field

Appending hitIssue.id to both solvedIssueIds and allSolvedIssueIds inside the functional setGameState keeps per-problem and global tracking in sync. Given that you guard by !gameState.solvedIssueIds.includes(issue.id) before computing hitIssue, this should only add each issue once per problem. If you ever reuse issue IDs across problems and want unique global IDs, you could enforce uniqueness here with a quick Set, but it’s not required for correctness with the current assumptions.


371-376: Minor consistency nit on navigation back to home

onClick={() => (window.location.href = \/${lang}`)}` is fine, but elsewhere in this file you use the non-parenthesized form. For consistency (and to avoid accidental returns if this grows), you might standardize on one style, or eventually switch to router-based navigation when you want SPA-style transitions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6acfc5 and 7e16c5d.

📒 Files selected for processing (2)
  • app/routes/$lang.$codeLanguage.play.tsx (9 hunks)
  • app/routes/result.create.tsx (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/routes/$lang.$codeLanguage.play.tsx (1)
app/problems/index.ts (1)
  • getProblems (59-67)
app/routes/result.create.tsx (2)
app/routes/result.$id.tsx (1)
  • action (55-86)
app/utils/nanoid.ts (1)
  • nanoid (5-15)
🔇 Additional comments (3)
app/routes/$lang.$codeLanguage.play.tsx (2)

120-143: Fetcher / navigation wiring is appropriate for the new result flow

Initializing fetcher and navigate, plus the scoreSaved flag, cleanly separates the score-saving lifecycle from the main game loop. This sets up the subsequent effects in a clear way and keeps the UI logic readable. useFetcher is the recommended pattern for submitting to an action route without navigating — it calls the route action and exposes the response on fetcher.data for you to read and render.


339-370: Consider showing error state when score save fails, but use the correct error pattern

Showing error feedback at game over is a good idea. However, note that thrown responses don't surface on fetcher.data—they trigger error boundaries instead. To drive conditional error UI, return a JSON error result from your action (e.g., return json({ error: 'Failed to save score' })) rather than throwing. Then check fetcher.data?.error to conditionally render an error message.

app/routes/result.create.tsx (1)

59-85: Error handling pattern is correct and recommended; optional improvement to distinguish error types can be deferred

The throw data({ error: '...' }, { status: 500 }) pattern you're using is the documented approach for Remix Single Fetch to surface structured errors to error boundaries. Error boundary components will receive isRouteErrorResponse with error.status and error.data accessible, and useFetcher() will propagate this through the standard error handling flow.

The optional suggestion to distinguish validation errors vs. DB failures (returning different error codes or message keys) remains a reasonable enhancement for future debugging and user-facing messaging, but the current implementation is sound.

- Add result/create and result/:id routes to routes.ts configuration
- React Router v7 requires explicit route definitions
- Add comprehensive debug logging for troubleshooting
- Fix 404 error by properly registering result routes
Copy link

@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

♻️ Duplicate comments (1)
app/routes/$lang.$codeLanguage.play.tsx (1)

176-214: Score save effect still doesn't handle failures or retries robustly.

As noted in the previous review, setScoreSaved(true) is called immediately after fetcher.submit() on line 212, regardless of whether the request succeeds. If the network or D1 insert fails, scoreSaved stays true and the effect never re-runs, causing the user to silently lose their score.

The navigation effect (lines 216-230) checks for success, but by then scoreSaved is already true, preventing retry.

Move the setScoreSaved(true) call to the navigation effect and only set it when data.success is confirmed:

-      setScoreSaved(true);
     }
-  }, [gameEnded, scoreSaved, gameState, codeLanguage, lang, fetcher]);
+  }, [gameEnded, scoreSaved, gameState, codeLanguage, lang, fetcher.submit]);
 
   // Navigate to result page when score is saved
   useEffect(() => {
@@ -225,6 +224,7 @@
       if (data.success && data.id) {
         console.log('[play] Navigating to result page:', `/result/${data.id}`);
         navigate(`/result/${data.id}`);
+        setScoreSaved(true);
       }
     }
   }, [fetcher.data, fetcher.state, navigate]);

Additionally, consider handling data.error to display feedback when submission fails.

🧹 Nitpick comments (1)
app/routes/$lang.$codeLanguage.play.tsx (1)

378-378: Unnecessary parentheses around assignment.

The parentheses around the assignment (window.location.href = \/${lang}`)` are unnecessary and don't change the behavior. The original syntax was fine.

Apply this diff if you prefer the cleaner syntax:

-              onClick={() => (window.location.href = `/${lang}`)}
+              onClick={() => { window.location.href = `/${lang}`; }}

Alternatively, keep the original arrow function with implicit return removed:

-              onClick={() => (window.location.href = `/${lang}`)}
+              onClick={() => window.location.href = `/${lang}`}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e16c5d and 3d64aff.

📒 Files selected for processing (3)
  • app/routes.ts (1 hunks)
  • app/routes/$lang.$codeLanguage.play.tsx (9 hunks)
  • app/routes/result.create.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/routes/result.create.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/routes/$lang.$codeLanguage.play.tsx (1)
app/problems/index.ts (1)
  • getProblems (59-67)
🔇 Additional comments (8)
app/routes.ts (1)

7-8: LGTM!

The new routes for result creation and display are correctly configured and follow the established pattern.

app/routes/$lang.$codeLanguage.play.tsx (7)

2-2: LGTM!

The new imports for useFetcher and useNavigate are necessary for the score-saving and navigation functionality.


19-20: LGTM!

The addition of allSolvedIssueIds to track issues found across all problems is necessary for computing accurate game statistics and aligns with the PR objectives.


122-123: LGTM!

The fetcher and navigate hooks are correctly initialized for score submission and navigation.


135-135: LGTM!

The allSolvedIssueIds array is correctly initialized in the initial game state.


263-263: LGTM!

The addition of newly found issues to allSolvedIssueIds correctly tracks all issues found across all problems for accurate statistics.


275-275: LGTM!

The state update correctly includes allSolvedIssueIds to maintain the tracking of all issues found throughout the game.


345-346: LGTM!

The loading indicator during score submission is a good UX improvement. The isSaving flag correctly reflects the fetcher's state.

Also applies to: 371-375

- Remove all debug console.log statements from play and result.create routes
- Fix scoreSaved state management: only mark as saved when request succeeds
- Add error state and UI to display save failures to users
- Improve nanoid fallback with monotonic counter and multiple random segments
  to reduce collision risk under high throughput
- Navigate effect now handles error cases properly
Copy link

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3d64aff and e139141.

📒 Files selected for processing (3)
  • app/routes/$lang.$codeLanguage.play.tsx (10 hunks)
  • app/routes/result.create.tsx (1 hunks)
  • app/utils/nanoid.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/routes/result.create.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/routes/$lang.$codeLanguage.play.tsx (1)
app/problems/index.ts (1)
  • getProblems (59-67)
🔇 Additional comments (2)
app/utils/nanoid.ts (1)

1-22: Fallback nanoid implementation looks solid for this use case

Timestamp + monotonic counter + two random segments is a big improvement over the prior fallback and should be more than sufficient to avoid collisions at the traffic levels of this game. No changes needed here.

app/routes/$lang.$codeLanguage.play.tsx (1)

19-21: Cross-problem issue tracking and accuracy calculation are wired correctly

Using allSolvedIssueIds to accumulate issue IDs across problems and then deriving issuesFound from its length, while computing totalIssues from usedProblemIds, gives you consistent per-run accuracy stats. The handleLineTap guard against already-solved issues ensures you don’t double-count the same issue ID. This all hangs together well.

Also applies to: 135-136, 195-197, 262-263, 274-274

Comment on lines +176 to +215
// Save score when game ends
useEffect(() => {
const scoreSaved = fetcher.state === 'idle' && fetcher.data?.success;

if (gameEnded && !scoreSaved && fetcher.state === 'idle') {
// Calculate total issues across all problems
const allProblems = getProblems(codeLanguage, 1)
.concat(getProblems(codeLanguage, 2))
.concat(getProblems(codeLanguage, 3));

const usedProblems = allProblems.filter((p) =>
gameState.usedProblemIds.includes(p.id)
);

const totalIssues = usedProblems.reduce(
(sum, problem) => sum + problem.issues.length,
0
);

const issuesFound = gameState.allSolvedIssueIds.length;
const accuracy = totalIssues > 0 ? issuesFound / totalIssues : 0;

// Prepare result data
const resultData = {
score: gameState.score,
issuesFound: issuesFound,
totalIssues: totalIssues,
accuracy: accuracy,
uiLanguage: lang,
codeLanguage: codeLanguage,
};

// Submit to result/create action
fetcher.submit(
{ payload: JSON.stringify(resultData) },
{ method: 'post', action: '/result/create' }
);
}
}, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Guard score-saving effect to avoid endless resubmits on failure

When /result/create returns { success: false, error: ... } (or any falsy success), fetcher.state goes back to 'idle' with fetcher.data set, so:

  • scoreSaved is false, and
  • if (gameEnded && !scoreSaved && fetcher.state === 'idle') becomes true again,

causing the effect to immediately re-call fetcher.submit. This will loop indefinitely on persistent failures, spamming your D1 endpoint and re-triggering the error UI.

You already surface errors nicely in the second effect and in the game-over UI, so it’s better to auto-submit only once and let the user decide if they want to retry.

One simple fix is to treat “has we ever tried to save?” as a separate guard derived from fetcher.state/fetcher.data, and only auto-submit when there has been no prior attempt:

   // Save score when game ends
   useEffect(() => {
-    const scoreSaved = fetcher.state === 'idle' && fetcher.data?.success;
-
-    if (gameEnded && !scoreSaved && fetcher.state === 'idle') {
+    const scoreSaved = !!fetcher.data?.success;
+    const hasAttemptedSave = fetcher.state !== 'idle' || fetcher.data != null;
+
+    if (gameEnded && !scoreSaved && !hasAttemptedSave) {
       // Calculate total issues across all problems
       const allProblems = getProblems(codeLanguage, 1)
         .concat(getProblems(codeLanguage, 2))
         .concat(getProblems(codeLanguage, 3));
@@
       // Submit to result/create action
       fetcher.submit(
         { payload: JSON.stringify(resultData) },
         { method: 'post', action: '/result/create' }
       );
     }
-  }, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);
+  }, [gameEnded, fetcher.state, fetcher.data, gameState, codeLanguage, lang, fetcher]);

This keeps your existing success navigation and error handling intact, but prevents uncontrolled retries. If you later want a manual “Retry saving score” button, it can call fetcher.submit directly without changing this effect.

Also applies to: 216-229, 344-380


I'll verify the review comment by examining the actual codebase. Let me start by inspecting the file and understanding the complete context.

Now let me examine the actual file and related code sections.


🏁 Script executed:

#!/bin/bash
# Examine the actual file to verify the code matches the review comment
cat "app/routes/\$lang.\$codeLanguage.play.tsx" | head -230 | tail -80

Length of output: 2451


Let me examine the other locations mentioned in the review.


🏁 Script executed:

#!/bin/bash
# Check lines 344-380 mentioned in "Also applies to"
sed -n '340,380p' "app/routes/\$lang.\$codeLanguage.play.tsx"

Length of output: 1651


Let me check the /result/create action handler to confirm the failure response format:


🏁 Script executed:

#!/bin/bash
# Find the result/create action file
fd -type f -name "*.tsx" -o -name "*.ts" | xargs rg -l "result/create|export.*action" | head -20

Length of output: 288


🏁 Script executed:

#!/bin/bash
# Examine the result.create action handler
cat "app/routes/result.create.tsx"

Length of output: 2232


Fix the infinite loop in score submission on API failure

The infinite loop bug is confirmed and real. When /result/create fails and throws an error, fetcher.data becomes { error: '...' } and fetcher.state becomes 'idle'. Since the current logic checks only fetcher.state === 'idle' && fetcher.data?.success, the condition remains true when success is missing/falsy, triggering an immediate resubmit. This cycles indefinitely, spamming the D1 endpoint.

The proposed fix correctly guards against this by tracking whether a save attempt has ever been made, allowing the action to submit once and letting error handling (via the second effect) take over.

Apply the suggested diff to lines 176-215 as shown in the review comment.

🤖 Prompt for AI Agents
In app/routes/$lang.$codeLanguage.play.tsx around lines 176-215, the effect
resubmits score repeatedly when the API returns an error because it only checks
fetcher.state and fetcher.data?.success; fix by adding a one-time guard (e.g., a
useRef or boolean state like saveAttempted) initialized false, change the effect
condition to require !saveAttempted (so it only submits once), set saveAttempted
to true immediately when calling fetcher.submit, and keep the rest of the logic
(calculating totalIssues, accuracy, and submitting payload) the same so
subsequent failures are handled by the existing error-handling effect rather
than causing infinite resubmits.

@goofmint goofmint merged commit e479312 into main Nov 22, 2025
1 check passed
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.

3 participants