feat(frontend): isolate public trust and add adjudication#445
Open
ShivamB25 wants to merge 14 commits intojunhoyeo:mainfrom
Open
feat(frontend): isolate public trust and add adjudication#445ShivamB25 wants to merge 14 commits intojunhoyeo:mainfrom
ShivamB25 wants to merge 14 commits intojunhoyeo:mainfrom
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Author
|
This PR is intentionally stacked. If it were marked as a standalone ready-to-merge change, reviewers would be looking at a diff that conceptually depends on code still under review in the earlier PR(s). Reviewing it in order keeps the trust-boundary changes, replay semantics, and public-surface changes easier to reason about. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
7 issues found across 45 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/leaderboard/getLeaderboard.ts">
<violation number="1" location="packages/frontend/src/lib/leaderboard/getLeaderboard.ts:354">
P1: All-time leaderboard and user-rank now rank the full verified-user set in memory on each cache miss instead of using DB-side pagination/ranking.</violation>
</file>
<file name="packages/frontend/src/lib/submissionReviews.ts">
<violation number="1" location="packages/frontend/src/lib/submissionReviews.ts:192">
P2: List endpoint returns detail-shaped review artifacts (including payload/audit/submissionHash) even though it is typed and intended as a summary list, exposing unnecessary data and bloating responses.</violation>
</file>
<file name="packages/frontend/src/lib/submissionPersistence.ts">
<violation number="1" location="packages/frontend/src/lib/submissionPersistence.ts:163">
P1: Check-then-insert on `submissions.userId` is race-prone; concurrent trusted submits can hit a unique-violation instead of reusing the same submission row.</violation>
<violation number="2" location="packages/frontend/src/lib/submissionPersistence.ts:329">
P3: Unsorted Set materialization makes persisted client/model arrays nondeterministic across identical submissions.</violation>
</file>
<file name="packages/frontend/src/lib/validation/submissionTrust.ts">
<violation number="1" location="packages/frontend/src/lib/validation/submissionTrust.ts:33">
P1: Unvalidated `timestampMs` can throw inside `toISOString()`, crashing trust assessment for schema-valid submissions.</violation>
</file>
<file name="packages/frontend/src/app/u/[username]/ProfilePageClient.tsx">
<violation number="1" location="packages/frontend/src/app/u/[username]/ProfilePageClient.tsx:235">
P2: Locale/timezone-dependent `toLocaleString()` can cause server/client hydration mismatches and unstable displayed timestamps.</violation>
</file>
<file name="packages/frontend/__tests__/api/settingsSubmittedDataDelete.test.ts">
<violation number="1" location="packages/frontend/__tests__/api/settingsSubmittedDataDelete.test.ts:145">
P3: Use different mocked row counts for submissions vs. reviews so the test can catch a mix-up in which delete result is used to compute `deletedSubmissions`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -1,7 +1,8 @@ | |||
| import { unstable_cache } from "next/cache"; | |||
Contributor
There was a problem hiding this comment.
P1: All-time leaderboard and user-rank now rank the full verified-user set in memory on each cache miss instead of using DB-side pagination/ranking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/leaderboard/getLeaderboard.ts, line 354:
<comment>All-time leaderboard and user-rank now rank the full verified-user set in memory on each cache miss instead of using DB-side pagination/ranking.</comment>
<file context>
@@ -397,59 +307,65 @@ async function fetchLeaderboardData(
- const totalUsers = Number(globalStats[0]?.uniqueUsers) || 0;
- const totalPages = Math.ceil(totalUsers / limit);
+ const rankedUsers = rankLeaderboardUsers(allTimeUsers, sortBy);
+ const filteredUsers = rankedUsers.filter((user) =>
+ matchesLeaderboardSearch(user, search)
</file context>
| }, | ||
| }; | ||
|
|
||
| const [existingSubmission] = await tx |
Contributor
There was a problem hiding this comment.
P1: Check-then-insert on submissions.userId is race-prone; concurrent trusted submits can hit a unique-violation instead of reusing the same submission row.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/submissionPersistence.ts, line 163:
<comment>Check-then-insert on `submissions.userId` is race-prone; concurrent trusted submits can hit a unique-violation instead of reusing the same submission row.</comment>
<file context>
@@ -0,0 +1,353 @@
+ },
+ };
+
+ const [existingSubmission] = await tx
+ .select({ id: submissions.id })
+ .from(submissions)
</file context>
| const TRUSTED_RETROACTIVE_WINDOW_DAYS = 30; | ||
|
|
||
| function getUtcDateStringFromTimestamp(timestampMs: number): string { | ||
| return new Date(timestampMs).toISOString().slice(0, 10); |
Contributor
There was a problem hiding this comment.
P1: Unvalidated timestampMs can throw inside toISOString(), crashing trust assessment for schema-valid submissions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/validation/submissionTrust.ts, line 33:
<comment>Unvalidated `timestampMs` can throw inside `toISOString()`, crashing trust assessment for schema-valid submissions.</comment>
<file context>
@@ -0,0 +1,154 @@
+const TRUSTED_RETROACTIVE_WINDOW_DAYS = 30;
+
+function getUtcDateStringFromTimestamp(timestampMs: number): string {
+ return new Date(timestampMs).toISOString().slice(0, 10);
+}
+
</file context>
| : await baseQuery.orderBy(desc(submissionReviews.updatedAt)); | ||
|
|
||
| return rows.map((row) => { | ||
| const artifact = mapSubmissionReviewArtifact(row); |
Contributor
There was a problem hiding this comment.
P2: List endpoint returns detail-shaped review artifacts (including payload/audit/submissionHash) even though it is typed and intended as a summary list, exposing unnecessary data and bloating responses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/submissionReviews.ts, line 192:
<comment>List endpoint returns detail-shaped review artifacts (including payload/audit/submissionHash) even though it is typed and intended as a summary list, exposing unnecessary data and bloating responses.</comment>
<file context>
@@ -0,0 +1,365 @@
+ : await baseQuery.orderBy(desc(submissionReviews.updatedAt));
+
+ return rows.map((row) => {
+ const artifact = mapSubmissionReviewArtifact(row);
+ return artifact;
+ });
</file context>
| <ReviewMetaList> | ||
| <ReviewMetaItem> | ||
| <ReviewMetaLabel>Updated</ReviewMetaLabel> | ||
| <ReviewMetaValue>{new Date(data.nonCompetitive.updatedAt).toLocaleString()}</ReviewMetaValue> |
Contributor
There was a problem hiding this comment.
P2: Locale/timezone-dependent toLocaleString() can cause server/client hydration mismatches and unstable displayed timestamps.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/app/u/[username]/ProfilePageClient.tsx, line 235:
<comment>Locale/timezone-dependent `toLocaleString()` can cause server/client hydration mismatches and unstable displayed timestamps.</comment>
<file context>
@@ -164,6 +190,80 @@ const EARLY_ADOPTERS = ["code-yeongyu", "gtg7784", "qodot"];
+ <ReviewMetaList>
+ <ReviewMetaItem>
+ <ReviewMetaLabel>Updated</ReviewMetaLabel>
+ <ReviewMetaValue>{new Date(data.nonCompetitive.updatedAt).toLocaleString()}</ReviewMetaValue>
+ </ReviewMetaItem>
+ {data.nonCompetitive.reasonCodes.length > 0 ? (
</file context>
Suggested change
| <ReviewMetaValue>{new Date(data.nonCompetitive.updatedAt).toLocaleString()}</ReviewMetaValue> | |
| <ReviewMetaValue> | |
| {new Intl.DateTimeFormat("en-US", { | |
| dateStyle: "medium", | |
| timeStyle: "short", | |
| timeZone: "UTC", | |
| }).format(new Date(data.nonCompetitive.updatedAt))} | |
| </ReviewMetaValue> |
| @@ -0,0 +1,353 @@ | |||
| import { eq, inArray, sql } from "drizzle-orm"; | |||
Contributor
There was a problem hiding this comment.
P3: Unsorted Set materialization makes persisted client/model arrays nondeterministic across identical submissions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/src/lib/submissionPersistence.ts, line 329:
<comment>Unsorted Set materialization makes persisted client/model arrays nondeterministic across identical submissions.</comment>
<file context>
@@ -0,0 +1,353 @@
+ reasoningTokens: totalReasoning,
+ dateStart: aggregates.dateStart,
+ dateEnd: aggregates.dateEnd,
+ sourcesUsed: Array.from(allClients),
+ modelsUsed: Array.from(allModels),
+ cliVersion: data.meta.version,
</file context>
| }); | ||
| mockState.setDeletedRows([{ id: "submission-1" }]); | ||
| mockState.setDeletedSubmissionRows([{ id: "submission-1" }]); | ||
| mockState.setDeletedReviewRows([{ id: "review-1" }]); |
Contributor
There was a problem hiding this comment.
P3: Use different mocked row counts for submissions vs. reviews so the test can catch a mix-up in which delete result is used to compute deletedSubmissions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/__tests__/api/settingsSubmittedDataDelete.test.ts, line 145:
<comment>Use different mocked row counts for submissions vs. reviews so the test can catch a mix-up in which delete result is used to compute `deletedSubmissions`.</comment>
<file context>
@@ -125,7 +141,8 @@ describe("DELETE /api/settings/submitted-data", () => {
});
- mockState.setDeletedRows([{ id: "submission-1" }]);
+ mockState.setDeletedSubmissionRows([{ id: "submission-1" }]);
+ mockState.setDeletedReviewRows([{ id: "review-1" }]);
const response = await DELETE(createRequest());
</file context>
Suggested change
| mockState.setDeletedReviewRows([{ id: "review-1" }]); | |
| mockState.setDeletedReviewRows([{ id: "review-1-1" }, { id: "review-1-2" }]); |
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
Changes
/api/leaderboardTesting
Depends on #444
Refs #441