fix: preserve multi-machine submissions on the leaderboard#376
Open
haridigresses wants to merge 7 commits intojunhoyeo:mainfrom
Open
fix: preserve multi-machine submissions on the leaderboard#376haridigresses wants to merge 7 commits intojunhoyeo:mainfrom
haridigresses wants to merge 7 commits intojunhoyeo:mainfrom
Conversation
Contributor
|
@haridigresses is attempting to deploy a commit to the Inevitable Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
There was a problem hiding this comment.
6 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/auth.rs">
<violation number="1" location="crates/tokscale-cli/src/auth.rs:153">
P1: Lock ownership is not tracked: stale-lock stealing plus unconditional lock-file deletion can break mutual exclusion and allow concurrent source-id writes.</violation>
<violation number="2" location="crates/tokscale-cli/src/auth.rs:171">
P2: Source-ID lock recovery window is inconsistent: acquisition times out (~2.5s) before stale-lock cleanup (10s), causing predictable transient failures after crashes.</violation>
</file>
<file name="packages/frontend/__tests__/api/submitSourceScope.test.ts">
<violation number="1" location="packages/frontend/__tests__/api/submitSourceScope.test.ts:385">
P2: Test coverage misses the existing `(userId, sourceId)` re-submit path, so overwrite/replace behavior can regress undetected.</violation>
<violation number="2" location="packages/frontend/__tests__/api/submitSourceScope.test.ts:410">
P2: Test is brittle because it depends on `whereCalls[0]` (exact call order) instead of asserting that the expected submissions lookup exists anywhere in captured calls.</violation>
</file>
<file name="packages/frontend/src/app/api/submit/route.ts">
<violation number="1" location="packages/frontend/src/app/api/submit/route.ts:88">
P1: `loadUserSubmitMetrics` reads Drizzle `select` results as objects instead of row arrays, causing response metrics to fall back to zero/null.</violation>
<violation number="2" location="packages/frontend/src/app/api/submit/route.ts:227">
P2: Concurrent first submissions can race between lookup and insert, causing unique-constraint errors to surface as 500 due to missing conflict handling/retry.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
5 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/frontend/src/lib/validation/submission.ts">
<violation number="1" location="packages/frontend/src/lib/validation/submission.ts:87">
P2: Optional source metadata currently rejects blank/whitespace values, causing full payload validation failure instead of treating metadata as absent.</violation>
</file>
<file name="packages/frontend/src/app/api/submit/route.ts">
<violation number="1" location="packages/frontend/src/app/api/submit/route.ts:67">
P2: `loadUserSubmitMetrics` counts all distinct `dailyBreakdown.date` rows, but elsewhere active days are defined as days with `tokens > 0`. If any zero-token daily rows exist, this will over-report active days and make the response inconsistent with the rest of the metrics logic.</violation>
</file>
<file name="crates/tokscale-cli/src/auth.rs">
<violation number="1" location="crates/tokscale-cli/src/auth.rs:216">
P1: Lock liveness check treats command execution failures as "owner dead", enabling false stale-lock removal and possible concurrent entry.</violation>
<violation number="2" location="crates/tokscale-cli/src/auth.rs:370">
P1: Submit can fail entirely when optional source-id lock/file operations fail, because source metadata errors are propagated with `?` instead of degrading to no sourceId.</violation>
</file>
<file name="packages/frontend/__tests__/lib/getUserEmbedStats.test.ts">
<violation number="1" location="packages/frontend/__tests__/lib/getUserEmbedStats.test.ts:128">
P2: Tests only validate numeric rank mocks and miss string-return normalization, allowing runtime type regressions in `rank` to slip through.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/auth.rs">
<violation number="1" location="crates/tokscale-cli/src/auth.rs:309">
P2: Stale lock cleanup no longer runs when the liveness probe fails. `lock_owner_is_alive` returns `None` on command errors, but `owner_is_dead` only matches `Some(false)`, so a stale lock with a valid state will never be removed if the probe command is unavailable. That can cause lock acquisition to time out and block submissions in constrained environments.</violation>
</file>
<file name="packages/frontend/__tests__/lib/validateSubmission.test.ts">
<violation number="1" location="packages/frontend/__tests__/lib/validateSubmission.test.ts:83">
P3: Optional chaining in these assertions allows the test to pass even if `result.data` is missing. Add an explicit assertion that `result.data` exists (or use a non-null assertion) before checking nested fields so the test enforces the valid-submission contract.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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
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 fixes the leaderboard overwrite case for users who submit from more than one machine.
Instead of merging every submit into a single
submissionsrow per user, submissions can now be optionally source-scoped:meta.sourceIdwrite to a(user_id, source_id)rowsourceIdkeep the existing unsourced behaviorThis keeps the fix modest and aligned with the existing schema and aggregation patterns, without introducing nested per-source instances into
daily_breakdown.source_breakdown.Repro
Before this patch:
Xfrom machine A withclaudeactivity.Xfrom machine B withclaudeactivity.Expected behavior:
What changed
source_idandsource_nametosubmissions/api/submitto target the correct submission row by source when providedsubmitpayloads onlyTOKSCALE_SOURCE_IDandTOKSCALE_SOURCE_NAMEValidation
Automated:
DATABASE_URL=postgres://localhost:5432/tokscale bun run db:generatebunx vitest run __tests__/api/submitSourceScope.test.ts __tests__/lib/getUserEmbedStats.test.ts __tests__/api/usersProfile.test.ts __tests__/api/submitAuth.test.ts __tests__/lib/getLeaderboardAllTime.test.ts __tests__/lib/getLeaderboard.test.tsBoth passed.
Notes
TOKSCALE_SOURCE_NAMEis stored persourceId; a later submit with the samesourceIdand a new non-empty name updates that source's display name.cargowas unavailable here.bunx tsc --noEmitstill fails on an unrelated existing import inpackages/frontend/src/components/BlackholeHero.tsx.Summary by cubic
Preserves multi‑machine submissions by scoping per source and aggregating totals across a user’s sources. Adds robust CLI source ID persistence with a stale‑lock cleanup fallback and normalizes profile/leaderboard/embed aggregates.
Bug Fixes
submissionsare source‑scoped: rows keyed by(user_id, source_id); unsourced rows remain; empty/whitespacesourceId/sourceNameare ignored; optional source metadata failures are tolerated./api/submittargets the correct per‑source row, updatessourceNameonly when provided, handles missing/invalid source metadata, and computes response metrics after commit.submissionCountuses sums;updatedAtuses the latest submit; embed ranks are normalized from string results.crates/tokscale-cliincludessourceId/sourceNamein submit meta, persists a stable per‑machine source ID with a small file lock, and cleans up stale locks by age if needed; supportsTOKSCALE_SOURCE_ID,TOKSCALE_SOURCE_NAME, andTOKSCALE_API_URL.Migration
source_id/source_nameand replace single‑row uniqueness with per‑source partial unique indexes.Written for commit 9eda26d. Summary will update on new commits.