Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
@claude review |
|
Claude finished @mxkaske's task in 3m 13s —— View job PR Review: chore/improve-onboarding-flow
Overall this is a solid, well-structured refactor. The architecture decisions are good: moving onboarding out of the SecurityNo rate limiting on Any authenticated user can POST to this endpoint in a tight loop. Each call fans out to all ~28 checker regions in parallel. There's no per-user throttle, IP limit, or cooldown. The auth check is present, but a single bad actor (or a bug in client retry logic) could saturate your checker fleet. A simple mitigation is a short-lived Redis/KV key keyed on Correctness
createFeedbackMutation.mutate({ source: "onboarding-completed", ... });
// ...
if (intentMessage) {
createFeedbackMutation.mutate({ source: "onboarding-intent", ... });
}
The ref prevents double-firing, which is correct for the current linear flow. But the "Retry" button calls DesignDouble cast in const COLUMNS = getColumns([]) as unknown as ColumnDef<OnboardingChecksRow>[];The comment explains the list vs. get shape difference, and the adapter ensures compatible structure at runtime. This is currently safe but creates a silent TS blind spot. If
The inline comment acknowledges this is a landmine — good documentation. Just note that it's also present in
const now = Date.now();This is module-level, so the timestamps on the placeholder rows are frozen at first import time. In SSR, this means the demo table shows timestamps from when the Node process started, not from the user's page load. If any column renders a relative time ("3 hours ago"), it will drift. If columns show an absolute timestamp this is fine, but worth a check. Minor / Nits
The cookie value decoding handles
const STATUS_PAGE_DOMAIN =
process.env.NEXT_PUBLIC_STATUS_PAGE_DOMAIN ?? "openstatus.dev";When running locally without the env var, the demo iframe (and the user's freshly-published page preview in step 2) will point to production
const requestStatus =
result.status >= 400 ? "error" : result.status >= 200 ? "success" : "error";1xx responses map to "error". Unlikely to occur in practice since the Go checker probably follows redirects, but if What's well done
|
Deploying openstatus-docs with
|
| Latest commit: |
8c1399f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://00685dfe.openstatus-1sy.pages.dev |
| Branch Preview URL: | https://chore-improve-onboarding-flo.openstatus-1sy.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 43 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="apps/dashboard/src/components/onboarding/checks-table-adapter.ts">
<violation number="1" location="apps/dashboard/src/components/onboarding/checks-table-adapter.ts:77">
P2: Do not replace the measured latency with summed phase durations; this can show incorrect latency values in onboarding results.</violation>
</file>
<file name="apps/dashboard/src/components/onboarding/checks-table.tsx">
<violation number="1" location="apps/dashboard/src/components/onboarding/checks-table.tsx:159">
P2: Use the count of visible columns for `colSpan` instead of total defined columns.</violation>
</file>
<file name="apps/dashboard/src/data/onboarding-checks.ts">
<violation number="1" location="apps/dashboard/src/data/onboarding-checks.ts:38">
P2: Avoid using `Date.now()` for pre-rendered mock rows; it can produce different server/client timestamp output during hydration. Use a fixed seed timestamp so initial HTML stays stable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // and the column looks broken. Use the phase sum as the denominator | ||
| // so the bar always fills the cell. | ||
| const phases = result.timing ? calculatePhases(result.timing) : null; | ||
| const phaseLatency = phases ? sumPhases(phases) : 0; |
There was a problem hiding this comment.
P2: Do not replace the measured latency with summed phase durations; this can show incorrect latency values in onboarding results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/components/onboarding/checks-table-adapter.ts, line 77:
<comment>Do not replace the measured latency with summed phase durations; this can show incorrect latency values in onboarding results.</comment>
<file context>
@@ -0,0 +1,100 @@
+ // and the column looks broken. Use the phase sum as the denominator
+ // so the bar always fills the cell.
+ const phases = result.timing ? calculatePhases(result.timing) : null;
+ const phaseLatency = phases ? sumPhases(phases) : 0;
+ const latency = phaseLatency > 0 ? phaseLatency : result.latency;
+
</file context>
| {isStreaming && rows.length < totalRegions ? ( | ||
| <TableRow className="hover:bg-transparent"> | ||
| <TableCell | ||
| colSpan={COLUMNS.length} |
There was a problem hiding this comment.
P2: Use the count of visible columns for colSpan instead of total defined columns.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/components/onboarding/checks-table.tsx, line 159:
<comment>Use the count of visible columns for `colSpan` instead of total defined columns.</comment>
<file context>
@@ -0,0 +1,177 @@
+ {isStreaming && rows.length < totalRegions ? (
+ <TableRow className="hover:bg-transparent">
+ <TableCell
+ colSpan={COLUMNS.length}
+ className="text-center text-muted-foreground text-xs"
+ >
</file context>
| { region: "eze", status: 200, latency: 245 }, | ||
| ]; | ||
|
|
||
| const now = Date.now(); |
There was a problem hiding this comment.
P2: Avoid using Date.now() for pre-rendered mock rows; it can produce different server/client timestamp output during hydration. Use a fixed seed timestamp so initial HTML stays stable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/dashboard/src/data/onboarding-checks.ts, line 38:
<comment>Avoid using `Date.now()` for pre-rendered mock rows; it can produce different server/client timestamp output during hydration. Use a fixed seed timestamp so initial HTML stays stable.</comment>
<file context>
@@ -0,0 +1,101 @@
+ { region: "eze", status: 200, latency: 245 },
+];
+
+const now = Date.now();
+
+// Deterministic pseudo-random in [0, 1) seeded by string — keeps SSR/CSR stable.
</file context>
1 participant
No description provided.