feat(dashboard): rebuild to V3 mockup, Vite → Next.js 16#25
Conversation
Pre-launch hardening. No-op when DSN is unset (prod-safe on merge). Backend (FastAPI on Railway): - sentry-sdk[fastapi] integration with FastApi + Starlette - send_default_pii=False, max_request_body_size="never" (Stripe PII safety) - include_local_variables=False (no secret leaks in stack frames) - before_send scrubs auth headers, cookies, X-API-Key, X-Stripe-Signature, and known secret keys in extras/contexts - Release tag from RAILWAY_GIT_COMMIT_SHA with "dev" fallback - Logs init status INFO so misconfig is visible in Railway logs Frontend (Vite+React on Cloudflare Pages): - @sentry/react with browserTracing + replay (masked + block media) - replaysSessionSampleRate=0, replaysOnErrorSampleRate=0.5 (cost guard) - beforeSend scrubs Supabase tokens from URLs + breadcrumbs - @sentry/vite-plugin uploads source maps when SENTRY_AUTH_TOKEN set (source maps deleted after upload, not served to browsers) Tests: 11 new (48 total passing). Docs: cloud/SENTRY-SETUP.md. Co-Authored-By: Gradata <noreply@gradata.ai>
Phase 1-3 of the dashboard rebuild (Phase 4 widgets to come). Static export for Cloudflare Pages, Sentry rewired, V3 Dark Cinematic tokens + sidebar matching approved mockup. Migration: - Next.js 16.2 App Router (static export, no Node runtime at edge) - Removed: Vite, React Router, @sentry/vite-plugin, @sentry/react, @fontsource-variable/geist, vite.config.ts, index.html, main.tsx, App.tsx, old src/pages/*.tsx, old Sidebar.tsx + Header.tsx - Added: next, @sentry/nextjs, eslint-config-next, @tailwindcss/postcss - Auth pages ported: login, signup, forgot-password, protected (dashboard) group with auth-guarded layout - useAuth.ts kept unchanged (no router coupling) - lib/supabase.ts + lib/api.ts: VITE_* → NEXT_PUBLIC_* env vars V3 Dark Cinematic theme: - globals.css: 10 color tokens, Space Grotesk + Inter + JetBrains Mono fonts via CSS @import (turbopack font loader path issue workaround), glass utility, scan-line keyframes, text-gradient-brand - NoiseOverlay: fractal noise SVG overlay (opacity 0.035) - GlassCard: reusable primitive with gradTop + scanLine options Shell UI matching mockup: - Sidebar: 3 sections (Brain/Team/Settings), active-route highlight, brand mark with gradient + glow, collapsible scroll - Header: workspace email + sign-out button - DashboardLayout: max-w-1280 content area, 32px padding Sentry: - sentry.client.config.ts + stub server/edge configs for static export - withSentryConfig wraps next.config.ts with conditional source-map upload (auth token optional) - All PII scrubbing from S105 kept: auth headers, tokens, URL params Build: 8 static routes, 0 errors. Known Phase 4 TODOs (per APPENDIX A of plan, sim-validated): - KpiStrip with 6-dim taxonomy (Goal Alignment, Tone & Register, Clarity & Structure, Factual Integrity, Domain Fit, Actionability) - CorrectionDecayCurve (hero viz, Wozniak decay + 95% CI bands) - GraduationProgressBar (INSTINCT 0.40 → PATTERN 0.60 → RULE 0.90) - ActiveRulesPanel (Bayesian conf, decay, zombie/suppression flags) - ABProofPanel, PrivacyPosturePanel, MethodologyLink - Principle lineage, convergence signal, contradiction state Co-Authored-By: Gradata <noreply@gradata.ai>
Implements APPENDIX A of the plan (sim-revised widget set from S101-103,
WAVE2, SIM_A/B, 1760 lines of synthesis). Dashboard reframed as an
observability lens over a local-first system; raw corrections never
surface, synthesized principles only.
New widgets (all in src/components/brain/):
- KpiStrip: 4 sim-validated metrics
· Correction Rate % (the only universally respected metric — S101)
· Sessions to Graduation with 95% CI (differentiator vs Mem0/Letta)
· Misfires absolute (trust signal, S103 "0 across 900+ applications")
· Brain Footprint (observability, not "cloud owns your data")
- CorrectionDecayCurve: hero viz, Wozniak-style exp-decay fit + CI band
(replaces trend bar from design spec per S103_STAT_REPLICATION)
- GraduationProgressBar: 3-tier INSTINCT 0.40 → PATTERN 0.60 → RULE 0.90
(SIM_A: "the graduation pipeline IS the product")
- ActiveRulesPanel: confidence as metadata not primary, recurrence
indicator (SIM16: 80% don't audit raw confidence scores)
- CategoriesChart: 6-dimension taxonomy (WAVE2 §2-3, SIM102 R5) —
Goal Alignment, Tone & Register, Clarity & Structure, Factual
Integrity, Domain Fit, Actionability — folds legacy 5-bucket
categories into the new dims until backend migrates
- MetaRulesGrid: 2-layer grouping (Objective / Subjective) with Goal
Alignment as governing node, tier badges Universal/Strong/Minority
(SIM101 consensus)
- ActivityFeed: graduation/self-healing/recurrence/meta-rule/convergence/
alert color coding per mockup (mock data, backend TODO)
- PrivacyPosturePanel: "raw corrections never leave your device" +
injection cap + footprint (SIM_A §5A trust surface)
- ABProofPanel: principles-vs-baseline with 95% CIs + 70% win rate
headline (S103 validated claims)
- MethodologyLink: modal with cited baselines (Duolingo HLR, Wozniak,
Copilot RCT) + named limitations (truth protocol)
Supporting infra:
- src/lib/analytics-client.ts: client-side KPI computation + decay-curve
fitting (no backend changes required for MVP)
- src/lib/fixtures/mock-{meta-rules,activity,proof}.ts: TODO(backend)
placeholders with sim-validated data shapes
- src/hooks/useApi.ts: accepts null URL for conditional fetching
Old src/components/brain/*Tab.tsx removed (replaced by widget set).
Empty state copy (no brains): "AI that learns the corrections you keep
making" + "Mem0 remembers. Gradata learns." + pip install gradata
(JTBD-validated positioning from S101-103).
Docs: cloud/dashboard/CLOUDFLARE-PAGES.md — deploy settings, env vars,
static-export gotchas.
Build: 8 static routes, 0 errors. Sentry disabled no-op path verified.
Co-Authored-By: Gradata <noreply@gradata.ai>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough
WalkthroughInitializes backend Sentry (with scrubbing and release resolution) and client Sentry stubs; migrates dashboard from Vite to Next.js; restructures dashboard routing/layout/auth; adds many Next.js pages, UI components, analytics helpers, fixtures, billing endpoints/models, new backend routes (activity/meta-rules/rule-patches), tests, and operational docs. Changes
Sequence Diagram(s)sequenceDiagram
actor Browser
participant NextJS as "Next.js App (client)"
participant Auth as "AuthProvider / ProtectedLayout"
participant API as "FastAPI Backend"
participant SentryClient as "Sentry (browser)"
Browser->>NextJS: Request /dashboard
NextJS->>Auth: render ProtectedLayout
Auth->>Auth: useAuth() (client)
alt unauthenticated
Auth-->>Browser: redirect /login
else authenticated
NextJS->>API: fetch /brains, /analytics, /corrections...
API-->>NextJS: responses
NextJS->>NextJS: computeKpis(), computeGraduationCounts()
NextJS->>Browser: render dashboard UI
Browser->>SentryClient: capture error/event (if DSN set)
SentryClient->>SentryClient: beforeSend scrub -> send
end
sequenceDiagram
participant FastAPI as "FastAPI Server"
participant SentrySDK as "sentry_sdk (server)"
participant Scrubber as "before_send scrubber"
participant SentryCloud as "Sentry Cloud"
FastAPI->>SentrySDK: init_sentry(settings)
alt DSN configured
SentrySDK->>SentrySDK: resolve_release()
SentrySDK->>SentrySDK: configure integrations (FastAPI/Starlette)
SentrySDK->>Scrubber: register before_send
FastAPI->>SentrySDK: capture exception/event
SentrySDK->>Scrubber: before_send(event)
Scrubber->>Scrubber: _scrub_event(event)
Scrubber-->>SentrySDK: sanitized event
SentrySDK->>SentryCloud: POST event
SentryCloud-->>SentrySDK: ack
else DSN empty
SentrySDK-->>FastAPI: disabled / no-op
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/app/sentry_init.py`:
- Around line 72-79: _scrub_dict currently only recurses into dict values so
sensitive data nested inside lists (e.g., {"extra": {"tokens": [{"access_token":
"..."}]}}) is not filtered; update _scrub_dict to also descend into iterable
containers: when a value is a list or tuple, iterate its elements and
recursively call _scrub_dict on any element that is a dict (and recursively
handle nested lists/tuples), and likewise handle dict-like mappings; keep the
existing behavior of replacing d[key] with "[Filtered]" when key.lower() is in
_SENSITIVE_KEYS. Target the _scrub_dict function and ensure recursion covers
dicts, lists, and tuples so secrets nested inside arrays are filtered.
In `@cloud/dashboard/app/`(dashboard)/dashboard/page.tsx:
- Around line 29-44: The dashboard is unwrapping only the first page from useApi
(correctionsResp, lessonsResp) and computing KPIs from that single .data slice;
update the data-loading to aggregate all pages for a brain when primaryBrainId
is set: either extend useApi to accept and auto-fetch all pages or add a
pagination loop that uses the paginated response (PaginatedResponse) to request
subsequent pages until exhausted and then set corrections and lessons to the
full concatenated arrays (referencing correctionsResp, lessonsResp, corrections,
lessons and useApi/primaryBrainId to locate the logic). Ensure the final
corrections and lessons arrays contain all items across pages before any
KPI/chart calculations run.
- Around line 26-34: The widgets are rendering a transient empty state because
useApi hooks flip from null to loading after primaryBrainId appears; fix by
gating the widget rendering on the actual dependent responses instead of just
primaryBrainId: keep the existing useApi calls (useApi, analytics,
correctionsResp, lessonsResp, primaryBrainId) but wrap/chart-render blocks so
they only render when the corresponding response is present (e.g. analytics !==
undefined / null before rendering analytics chart, correctionsResp exists before
rendering corrections panel, lessonsResp exists before rendering lessons panel);
apply the same guarding for all dependent widgets referenced in this file (lines
~52-125).
In `@cloud/dashboard/app/forgot-password/page.tsx`:
- Around line 39-41: The current JSX nests the Button component inside Link,
creating nested interactive elements (Link and Button) which is inaccessible;
update the markup to use one interactive element only by either rendering the
Button as the Link using the Button's asChild prop (e.g., pass asChild to Button
and place Link as its child) or by styling the Link to look like the Button and
remove the Button wrapper; change the <Link> / <Button> usage around the "Back
to login" text accordingly (refer to the Link and Button components in page.tsx)
and ensure keyboard focus and semantics remain a single interactive element.
In `@cloud/dashboard/app/globals.css`:
- Around line 3-23: The SCSS linter is flagging Tailwind's custom at-rule
`@theme`; update your Stylelint configuration to teach the scss/at-rule-no-unknown
rule to ignore Tailwind at-rules by adding an ignoreAtRules array that includes
"theme" (and common Tailwind at-rules such as "tailwind", "apply", "screen",
"variants", "responsive", "layer" as needed). Modify the scss/at-rule-no-unknown
rule entry in your Stylelint config (e.g., .stylelintrc.js or package.json
stylelint key) to include ignoreAtRules with "theme" so the `@theme` declaration
in globals.css no longer triggers the error.
In `@cloud/dashboard/CLOUDFLARE-PAGES.md`:
- Around line 38-41: The Sentry smoke-test instruction that asks operators to
trigger a client-side React error on the live site should be removed or
relocated: update the checklist that mentions the browser console message
"[sentry] initialized env=production release=..." and the step to "trigger a
React error" so it either (a) moves into preview/staging guidance, or (b)
replaces the manual error with a non-invasive synthetic verification such as
invoking a controlled Sentry API call (e.g., a documented synthetic event via
Sentry.captureMessage/captureException or server-side test event using the DSN)
so production operators do not create real user-facing errors; ensure references
to DSN and the console log remain but clarify that generation of events on
production is forbidden and point to the staging verification alternative.
In `@cloud/dashboard/package.json`:
- Around line 13-40: Run local compatibility tests for the specified dependency
set: install and build the Next.js app using next@16.0.0, typescript@~6.0.2, and
`@sentry/nextjs`@10.48.0, and resolve any peer dependency conflicts reported by
npm/yarn; if build or tsc fails, either pin `@sentry/nextjs` or typescript to a
compatible minor version or add a package manager resolution/override to force
compatible peer deps, then re-run next build and tsc --noEmit to confirm success
(check package names `@sentry/nextjs`, next, and typescript in package.json and
adjust versions/overrides accordingly).
In `@cloud/dashboard/src/components/brain/ABProofPanel.tsx`:
- Around line 42-57: The progress bar inner divs for baseline and
with-principles (using baselinePct and withPct) are currently plain divs with
aria-labels that won't be announced; update those elements to include
role="progressbar" and the ARIA attributes aria-valuenow={baselinePct} (or
{withPct}), aria-valuemin="0", aria-valuemax="100" while keeping the aria-label
text so assistive tech correctly announces the percentage; locate the two inner
divs in ABProofPanel (the ones with className "h-full bg-[var(--color-body)]"
and "h-full bg-gradient-brand") and add those attributes.
In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx`:
- Around line 20-26: The ago function needs to validate the parsed date before
computing differences: inside ago(iso) check that iso is a non-empty string and
that new Date(iso).getTime() yields a valid number (use Date.parse or
Number.isFinite/Number.isNaN checks); if invalid, return a safe fallback such as
'just now' or 'unknown'; otherwise compute diffMs and proceed as before. Update
the ago helper to perform this validation and early-return to avoid producing
"NaNd ago".
In `@cloud/dashboard/src/components/brain/CategoriesChart.tsx`:
- Around line 45-47: The loop that folds analytics.corrections_by_category
currently defaults unknown keys to 'Factual Integrity' (see the iteration over
analytics.corrections_by_category, LEGACY_MAP lookup, and folded accumulator),
which can misattribute new/misspelled backend keys; change the logic to skip
keys not present in LEGACY_MAP (or aggregate them under an explicit
'Unknown'/'Other' bucket) instead of using the 'Factual Integrity' default, and
optionally emit a console.warn or telemetry for unmapped keys so they are
visible during debugging.
In `@cloud/dashboard/src/components/brain/CorrectionDecayCurve.tsx`:
- Around line 26-28: The headline drop uses the zero-filled edges from
buildDecayCurve (variables first/last/dropPct) which makes sparse ranges report
0%; instead locate the first and last observed empirical values (scan data for
the first element where item.empirical is not null/undefined/0 and the last
element likewise), assign those to firstObserved/lastObserved, and compute
dropPct from those (falling back to 0 if no observed points); update the
references where first/last are used so dropPct reflects actual observed
start/end values rather than zero-filled entries.
- Around line 73-91: The CI band is stacked incorrectly because both Area
components use stackId="ci"; compute a new derived data field ciBand = ciHigh -
ciLow for each datum and update the chart to render ciLow first as a transparent
base Area (e.g., dataKey="ciLow", fillOpacity=0, stroke="none") and then render
ciBand on top (dataKey="ciBand", stackId="ci", fill="#3A82FF", fillOpacity=0.08)
so the shaded region covers ciLow → ciHigh; update any data mapping logic that
builds the series to include the ciBand field before rendering.
In `@cloud/dashboard/src/components/brain/KpiStrip.tsx`:
- Around line 12-30: The delta is being formatted with toFixed(0) for display
but the changeTone decision still uses the original unrounded
metrics.correctionRateDeltaPct, causing mismatched signs; compute a roundedPct =
Math.round(metrics.correctionRateDeltaPct) (or similar) once inside the items
entry and use that roundedPct both to decide value (use fmtDelta(roundedPct) or
inline formatting) and to set changeTone (roundedPct < 0 ? 'pos' : roundedPct >
0 ? 'neg' : 'neu'), keeping the existing zero-display of '—' when roundedPct ===
0 and updating the Correction Rate item's value and changeTone to reference that
rounded value instead of the raw metric.
In `@cloud/dashboard/src/components/brain/MetaRulesGrid.tsx`:
- Around line 27-49: The inline Card component inside MetaRulesGrid causes a new
function on every render; move Card out to module scope (above or below
MetaRulesGrid) and give it a typed props interface (e.g., props: { m: typeof
mockMetaRules[number] }) so it no longer gets recreated each render, then update
MetaRulesGrid to use the top-level Card; optionally wrap the top-level Card with
React.memo to further prevent unnecessary re-renders and keep existing
identifiers like Card, MetaRulesGrid, mockMetaRules and TIER_STYLE unchanged so
keys and styling continue to work.
In `@cloud/dashboard/src/components/brain/MethodologyLink.tsx`:
- Around line 29-94: The modal rendered when open (role="dialog" in
MethodologyLink component) lacks keyboard and focus accessibility: add
Escape-to-close handling, explicit aria-labelledby/aria-modal attributes, and
proper focus management. Implement a ref for the dialog container and focus the
first interactive element (the close button) on open using useEffect, restore
focus to the element that opened the modal on close, and add an onKeyDown
handler that listens for Escape to call setOpen(false); additionally ensure the
heading has an id and the dialog uses aria-labelledby pointing to it, and
consider trapping focus within the modal while open (or use an existing
focus-trap utility) to prevent keyboard users from tabbing out.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx`:
- Around line 43-77: The sidebar in DashboardLayout permanently reserves 240px
(the aside using SECTIONS and Link) which breaks small viewports; change it to a
responsive collapsible/drawer: hide the aside on small screens (e.g., use
utility classes like hidden on sm and show on md) and add a toggle state
(isSidebarOpen) and a hamburger button in the top/header to open a drawer; when
open render the same section list (use SECTIONS, pathname, Link) inside a
full-screen/overlay drawer with appropriate aria attributes and a close control,
and ensure the aside keeps its current layout on larger screens so desktop
behavior is unchanged.
In `@cloud/dashboard/src/hooks/useApi.ts`:
- Around line 31-35: The fetchData callback returns early when url === null but
doesn't clear previously loaded state, so update fetchData (in useApi.ts) to
explicitly clear stored results when url becomes null by calling the appropriate
state setters (e.g., setData and setError) and setLoading(false) before
returning; locate fetchData and the state hooks (setData, setError, setLoading)
and ensure you reset data to an empty/initial value (or null per existing state
shape) so stale analytics/corrections are not shown when url transitions to
null.
In `@cloud/dashboard/src/lib/analytics-client.ts`:
- Around line 132-160: The code builds 24h rolling buckets from `now`
(Date.now()) so bucket boundaries float with the current clock; change the
bucket anchor to a day boundary instead: compute a local "endOfToday" (a Date
set to local midnight for the current day, e.g., new Date(); setHours(24,0,0,0)
or setHours(0,0,0,0) then add dayMs as appropriate) and use that as the
reference instead of `now` when constructing `buckets`, computing `idx`, and
filtering corrections (use the correction timestamp `c.created_at` as you
already do). This ensures buckets align to calendar days and labels from
toLocaleDateString reflect the actual calendar day for each bucket; update uses
of `now` in the loop that builds `buckets` and the index calc (`const idx =
Math.floor((t - (now - rangeDays * dayMs)) / dayMs)`) to use the new
`endOfToday` reference.
In `@cloud/dashboard/src/lib/api.ts`:
- Line 5: The code currently sets baseURL: process.env.NEXT_PUBLIC_API_URL ||
'https://gradata-production.up.railway.app/api/v1', which silently falls back to
production; remove the implicit fallback and instead fail fast when
NEXT_PUBLIC_API_URL is missing by checking process.env.NEXT_PUBLIC_API_URL
during initialization (the baseURL assignment used to configure the API client)
and throwing or logging a clear error if it's undefined so the app does not
accidentally send traffic to prod.
In `@cloud/dashboard/tsconfig.json`:
- Around line 38-45: The tsconfig currently includes ".next/types/**/*.ts" and
".next/dev/types/**/*.ts" but also excludes the entire ".next" directory, which
prevents those included paths from being used; remove ".next" from the "exclude"
array so the explicit includes take effect (update the tsconfig.json exclude
list to only contain "node_modules" and "out"), ensuring Next.js generated
route/type files are available to the TypeScript compiler.
In `@cloud/SENTRY-SETUP.md`:
- Around line 20-24: The fenced code blocks containing environment variable
examples (e.g., the block with GRADATA_SENTRY_DSN,
GRADATA_SENTRY_TRACES_SAMPLE_RATE, GRADATA_ENVIRONMENT) need explicit language
tags to satisfy markdownlint; update each triple-backtick fence (including the
other blocks at ranges referenced) to use an appropriate tag such as ```bash or
```env so the blocks are recognized as shell/environment snippets and the linter
warnings are resolved.
- Around line 3-4: The frontend Sentry runbook still references Vite-era setup
(VITE_* env vars, vite.config.ts, src/main.tsx) but the dashboard now uses
Next.js with public vars renamed to NEXT_PUBLIC_*; update the doc to instruct
operators to set NEXT_PUBLIC_SENTRY_DSN (and any NEXT_PUBLIC_SENTRY_ENV,
NEXT_PUBLIC_SENTRY_RELEASE), redeploy the Next.js dashboard on Cloudflare Pages,
and replace instructions about modifying vite.config.ts/src/main.tsx with
guidance to initialize Sentry in the Next.js entrypoint (e.g., app/layout.tsx or
pages/_app.tsx) and any next.config.js/runtime config changes so client-side
Sentry is actually enabled. Ensure the same corrections are applied to the other
affected sections (lines ~34-50 and ~91-100).
In `@cloud/tests/test_sentry.py`:
- Around line 35-45: The test test_init_sentry_enabled_when_dsn_set calls
init_sentry which invokes the real sentry_sdk.init and leaves a global client
active, polluting other tests; update this test to mock sentry_sdk.init (or
patch it) so init_sentry is exercised without creating a real global client,
assert the mock was called with the expected DSN and environment, and remove any
assertions that rely on sentry_sdk.get_client() being active; reference the test
function name test_init_sentry_enabled_when_dsn_set, the init_sentry function
under test, and the sentry_sdk.init call to locate where to apply the
mock/patch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e623deb7-5935-47ad-9176-2c046b35d2f4
⛔ Files ignored due to path filters (1)
cloud/dashboard/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
cloud/SENTRY-SETUP.mdcloud/app/config.pycloud/app/main.pycloud/app/sentry_init.pycloud/dashboard/.gitignorecloud/dashboard/CLOUDFLARE-PAGES.mdcloud/dashboard/app/(dashboard)/api-keys/page.tsxcloud/dashboard/app/(dashboard)/dashboard/page.tsxcloud/dashboard/app/(dashboard)/layout.tsxcloud/dashboard/app/(dashboard)/settings/page.tsxcloud/dashboard/app/forgot-password/page.tsxcloud/dashboard/app/globals.csscloud/dashboard/app/layout.tsxcloud/dashboard/app/login/page.tsxcloud/dashboard/app/page.tsxcloud/dashboard/app/signup/page.tsxcloud/dashboard/index.htmlcloud/dashboard/instrumentation.tscloud/dashboard/next.config.tscloud/dashboard/package.jsoncloud/dashboard/postcss.config.mjscloud/dashboard/sentry.client.config.tscloud/dashboard/sentry.edge.config.tscloud/dashboard/sentry.server.config.tscloud/dashboard/src/App.tsxcloud/dashboard/src/components/brain/ABProofPanel.tsxcloud/dashboard/src/components/brain/ActiveRulesPanel.tsxcloud/dashboard/src/components/brain/ActivityFeed.tsxcloud/dashboard/src/components/brain/AnalyticsTab.tsxcloud/dashboard/src/components/brain/CategoriesChart.tsxcloud/dashboard/src/components/brain/CorrectionDecayCurve.tsxcloud/dashboard/src/components/brain/CorrectionsTab.tsxcloud/dashboard/src/components/brain/GraduationProgressBar.tsxcloud/dashboard/src/components/brain/KpiStrip.tsxcloud/dashboard/src/components/brain/LessonsTab.tsxcloud/dashboard/src/components/brain/MetaRulesGrid.tsxcloud/dashboard/src/components/brain/MethodologyLink.tsxcloud/dashboard/src/components/brain/OverviewTab.tsxcloud/dashboard/src/components/brain/PrivacyPosturePanel.tsxcloud/dashboard/src/components/layout/DashboardLayout.tsxcloud/dashboard/src/components/layout/GlassCard.tsxcloud/dashboard/src/components/layout/Header.tsxcloud/dashboard/src/components/layout/NoiseOverlay.tsxcloud/dashboard/src/components/layout/Sidebar.tsxcloud/dashboard/src/components/providers/AuthProvider.tsxcloud/dashboard/src/hooks/useApi.tscloud/dashboard/src/index.csscloud/dashboard/src/lib/analytics-client.tscloud/dashboard/src/lib/api.tscloud/dashboard/src/lib/fixtures/mock-activity.tscloud/dashboard/src/lib/fixtures/mock-meta-rules.tscloud/dashboard/src/lib/fixtures/mock-proof.tscloud/dashboard/src/lib/supabase.tscloud/dashboard/src/main.tsxcloud/dashboard/src/pages/ApiKeys.tsxcloud/dashboard/src/pages/BrainDetail.tsxcloud/dashboard/src/pages/Dashboard.tsxcloud/dashboard/src/pages/Settings.tsxcloud/dashboard/tsconfig.app.jsoncloud/dashboard/tsconfig.jsoncloud/dashboard/tsconfig.node.jsoncloud/dashboard/vite.config.tscloud/pyproject.tomlcloud/tests/test_sentry.py
💤 Files with no reviewable changes (17)
- cloud/dashboard/src/App.tsx
- cloud/dashboard/src/main.tsx
- cloud/dashboard/tsconfig.app.json
- cloud/dashboard/index.html
- cloud/dashboard/tsconfig.node.json
- cloud/dashboard/src/pages/BrainDetail.tsx
- cloud/dashboard/src/components/brain/AnalyticsTab.tsx
- cloud/dashboard/src/index.css
- cloud/dashboard/src/pages/Dashboard.tsx
- cloud/dashboard/src/components/layout/Sidebar.tsx
- cloud/dashboard/src/pages/Settings.tsx
- cloud/dashboard/src/components/brain/LessonsTab.tsx
- cloud/dashboard/src/components/layout/Header.tsx
- cloud/dashboard/vite.config.ts
- cloud/dashboard/src/components/brain/CorrectionsTab.tsx
- cloud/dashboard/src/pages/ApiKeys.tsx
- cloud/dashboard/src/components/brain/OverviewTab.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.0)
cloud/SENTRY-SETUP.md
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Stylelint (17.6.0)
cloud/dashboard/app/globals.css
[error] 3-3: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🔇 Additional comments (29)
cloud/dashboard/src/lib/supabase.ts (1)
3-4: Env var migration looks correct.Using
process.env.NEXT_PUBLIC_SUPABASE_URLandprocess.env.NEXT_PUBLIC_SUPABASE_ANON_KEYaligns with the Next.js migration and keeps client config explicit.cloud/dashboard/src/components/providers/AuthProvider.tsx (1)
5-7: Provider wiring is clean and correct.This correctly bridges
useAuthProvider()intoAuthContext.Providerfor app-wide auth state.cloud/dashboard/src/components/layout/NoiseOverlay.tsx (1)
3-12: Overlay implementation is solid.
aria-hidden+pointer-events-noneis the right combination for a purely decorative noise layer.cloud/dashboard/.gitignore (1)
1-31: Ignore rules are aligned with the Next.js/Sentry migration.This is a clean, targeted set for build outputs, env files, and generated artifacts.
cloud/dashboard/src/lib/fixtures/mock-activity.ts (1)
2-20: Fixture typing is strong and UI-friendly.
ActivityKind+MockActivitywith__mocked: truegives clear, safe mock-data boundaries.cloud/dashboard/src/lib/fixtures/mock-meta-rules.ts (1)
5-70: Meta-rule fixture structure is well designed.The typed
layer/tiermodel and source metadata make this dataset robust for grid rendering and future backend swap.cloud/pyproject.toml (1)
14-14: Sentry dependency addition matches backend instrumentation changes.This dependency update is consistent with the FastAPI Sentry integration introduced in this PR.
cloud/dashboard/sentry.edge.config.ts (1)
1-2: Sentry edge stub is correct for static export.This minimal module is appropriate and avoids unnecessary runtime init for this target.
cloud/dashboard/app/(dashboard)/api-keys/page.tsx (1)
1-11: Placeholder route implementation is clear and safe.The page renders a deterministic fallback state and keeps scope explicit for Phase 4.
cloud/dashboard/instrumentation.ts (1)
1-5: No-op instrumentation hook is appropriate here.This keeps Next/Sentry integration expectations satisfied without introducing runtime work.
cloud/dashboard/sentry.server.config.ts (1)
1-2: Server Sentry stub is correctly minimal.This is a valid static-export placeholder and does not introduce server runtime coupling.
cloud/dashboard/postcss.config.mjs (1)
1-7: PostCSS configuration looks good.The plugin wiring is concise and aligned with the new tooling setup.
cloud/app/main.py (1)
26-30: Sentry init placement is correct.Initializing before FastAPI app construction is the right ordering for integration coverage.
cloud/dashboard/app/(dashboard)/settings/page.tsx (1)
1-11: Settings placeholder page is acceptable for phased delivery.The route is explicit, stable, and communicates implementation status clearly.
cloud/app/config.py (1)
23-26: LGTM!The Sentry configuration fields are well-designed with sensible defaults. Empty DSN as the disable mechanism is a standard pattern, and 0.1 trace sample rate is reasonable for production use. The inline comment documenting the release fallback behavior is helpful.
cloud/dashboard/app/page.tsx (1)
7-17: LGTM!The redirect logic is correct:
router.replace()prevents back-navigation to the empty page, and the effect dependencies are complete. The pattern aligns with the protected layout's auth gating.Note: On static export, there may be a brief moment where the page mounts before the redirect fires. This is acceptable since
nullis rendered, but consider adding a loading indicator if users report a flash of blank screen.cloud/dashboard/app/(dashboard)/layout.tsx (1)
8-17: LGTM!The protected layout correctly guards dashboard routes: it returns
nullwhile auth is loading or when unauthenticated (preventing flash of protected content), and the effect handles the redirect. The pattern is consistent with the root page's auth handling.cloud/dashboard/app/layout.tsx (2)
11-28: LGTM!The root layout correctly sets up the app structure with
AuthProviderfor global auth state andNoiseOverlayfor the V3 theme. The Google Fonts approach withdisplay=swapis appropriate given the noted turbopack limitation. Consider revisitingnext/fontwhen the path issue is resolved for automatic subsetting benefits.
14-21: 🧹 Nitpick | 🔵 TrivialFont loading via stylesheet links bypasses
next/fontoptimizations.The comment notes a turbopack path issue necessitating this approach. Be aware this skips Next.js font optimizations (automatic subsetting, self-hosting,
font-displaycontrol). Consider revisiting once the turbopack issue is resolved.For now, adding
font-display=swapto the URL prevents FOIT:🎨 Add font-display parameter
<link rel="stylesheet" - href="https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600&family=Space+Grotesk:wght@500;600;700&family=JetBrains+Mono:wght@400;500&display=swap" + href="https://fonts.googleapis.com/css2?family=Inter:wght@400;500;600&family=Space+Grotesk:wght@500;600;700&family=JetBrains+Mono:wght@400;500&display=swap&font-display=swap" />Note: The
display=swapparameter is already present, which handles this. LGTM on the current implementation.> Likely an incorrect or invalid review comment.cloud/dashboard/src/components/brain/MetaRulesGrid.tsx (1)
51-85: LGTM!The conditional rendering logic for the three layers is clean, and the UI structure follows the documented SIM101/SIM102 consensus design. The TODO comment for backend integration is noted.
cloud/dashboard/src/components/layout/GlassCard.tsx (1)
1-27: LGTM!Clean, reusable component with well-defined props interface. The optional
gradTopandscanLineeffects integrate properly with the global CSS classes defined inglobals.css. Good use ofcnutility for className merging and proper prop spreading.cloud/dashboard/src/lib/fixtures/mock-proof.ts (1)
1-19: LGTM!Well-structured fixture with clear interface definition. The TODO comment appropriately flags this for backend replacement, and the data provenance is documented in comments.
cloud/dashboard/src/components/brain/ABProofPanel.tsx (1)
12-41: LGTM!The component logic for computing deltas and displaying confidence intervals is correct. Good documentation of the differentiator rationale in the JSDoc comment.
Also applies to: 58-69
cloud/dashboard/next.config.ts (1)
1-22: LGTM!Well-configured for static export to Cloudflare Pages. Good practices include:
hideSourceMaps: trueprevents exposing source maps in productionsilent: !process.env.CIkeeps local dev quiet while ensuring CI visibility- Graceful degradation when
SENTRY_AUTH_TOKENis not set (dev-safe)- TypeScript/ESLint checks enforced during builds
The stub files for server/edge configs properly handle the static export case where no runtime exists.
cloud/dashboard/app/login/page.tsx (2)
20-33: LGTM!The session redirect pattern is correctly implemented. The
useEffectwaits for!loading && sessionwhich ensures the redirect only fires after the initial auth state check completes (per theuseAuthhook's initialization sequence). ThehandleSubmitproperly relies on the auth state change subscription to trigger the redirect rather than navigating manually, avoiding race conditions.
1-5: LGTM!Clean migration from React Router to Next.js App Router. Imports correctly updated to
next/linkandnext/navigation, and the component properly uses'use client'directive for client-side interactivity.Also applies to: 12-14, 66-71, 82-83
cloud/dashboard/sentry.client.config.ts (1)
1-40: LGTM!Solid PII scrubbing implementation with comprehensive coverage:
- Sensitive headers filtered (authorization, cookie, x-api-key)
- Token parameters redacted from URLs (query params and hash fragments)
sendDefaultPii: falseas baseline protection- Replay integration configured with
maskAllTextandblockAllMediafor privacy- Graceful no-op when DSN is unset (appropriate for local dev)
The
beforeSendhook correctly handles edge cases with try-catch for URL parsing.cloud/dashboard/src/components/brain/GraduationProgressBar.tsx (1)
50-52: No defensive fallback is needed;avgConfidenceByStateis always fully populated.The
avg()function explicitly returns0for empty arrays, and the object literal incomputeGraduationCountscreates all three state keys (INSTINCT, PATTERN, RULE) with numeric values every time. Accessingcounts.avgConfidenceByState[t.key]will always return a number, so calling.toFixed(2)is safe.> Likely an incorrect or invalid review comment.cloud/dashboard/app/signup/page.tsx (1)
33-37: > Likely an incorrect or invalid review comment.
| def _scrub_dict(d: dict[str, Any]) -> None: | ||
| """Recursively replace values whose keys look sensitive.""" | ||
| for key, value in list(d.items()): | ||
| if isinstance(key, str) and key.lower() in _SENSITIVE_KEYS: | ||
| d[key] = "[Filtered]" | ||
| elif isinstance(value, dict): | ||
| _scrub_dict(value) | ||
|
|
There was a problem hiding this comment.
Recursion stops at lists, so nested secrets still leak.
_scrub_dict() only descends into dict values. Payloads like {"extra": {"tokens": [{"access_token": "..."}]}} pass through untouched, which breaks the “anywhere in extras/contexts” guarantee and can leak credentials.
Suggested fix
+def _scrub_value(value: Any) -> None:
+ if isinstance(value, dict):
+ _scrub_dict(value)
+ elif isinstance(value, list):
+ for item in value:
+ _scrub_value(item)
+
+
def _scrub_dict(d: dict[str, Any]) -> None:
"""Recursively replace values whose keys look sensitive."""
for key, value in list(d.items()):
if isinstance(key, str) and key.lower() in _SENSITIVE_KEYS:
d[key] = "[Filtered]"
- elif isinstance(value, dict):
- _scrub_dict(value)
+ else:
+ _scrub_value(value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/sentry_init.py` around lines 72 - 79, _scrub_dict currently only
recurses into dict values so sensitive data nested inside lists (e.g., {"extra":
{"tokens": [{"access_token": "..."}]}}) is not filtered; update _scrub_dict to
also descend into iterable containers: when a value is a list or tuple, iterate
its elements and recursively call _scrub_dict on any element that is a dict (and
recursively handle nested lists/tuples), and likewise handle dict-like mappings;
keep the existing behavior of replacing d[key] with "[Filtered]" when
key.lower() is in _SENSITIVE_KEYS. Target the _scrub_dict function and ensure
recursion covers dicts, lists, and tuples so secrets nested inside arrays are
filtered.
| const { data: analytics } = useApi<BrainAnalytics>( | ||
| primaryBrainId ? `/brains/${primaryBrainId}/analytics` : null, | ||
| ) | ||
| const { data: correctionsResp } = useApi<PaginatedResponse<Correction> | Correction[]>( | ||
| primaryBrainId ? `/brains/${primaryBrainId}/corrections` : null, | ||
| ) | ||
| const { data: lessonsResp } = useApi<PaginatedResponse<Lesson> | Lesson[]>( | ||
| primaryBrainId ? `/brains/${primaryBrainId}/lessons` : null, | ||
| ) |
There was a problem hiding this comment.
Gate the widgets on the dependent requests, not just /brains.
Because useApi() starts these hooks with url = null and only flips them to loading in the follow-up effect, this path renders zeroed charts/panels for one frame after primaryBrainId appears. That produces a visible false-empty state before analytics, corrections, and lessons resolve.
Suggested fix
- const { data: analytics } = useApi<BrainAnalytics>(
+ const { data: analytics, loading: loadingAnalytics } = useApi<BrainAnalytics>(
primaryBrainId ? `/brains/${primaryBrainId}/analytics` : null,
)
- const { data: correctionsResp } = useApi<PaginatedResponse<Correction> | Correction[]>(
+ const { data: correctionsResp, loading: loadingCorrections } = useApi<PaginatedResponse<Correction> | Correction[]>(
primaryBrainId ? `/brains/${primaryBrainId}/corrections` : null,
)
- const { data: lessonsResp } = useApi<PaginatedResponse<Lesson> | Lesson[]>(
+ const { data: lessonsResp, loading: loadingLessons } = useApi<PaginatedResponse<Lesson> | Lesson[]>(
primaryBrainId ? `/brains/${primaryBrainId}/lessons` : null,
)
@@
- if (loadingBrains) return <LoadingSpinner className="py-20" />
+ if (
+ loadingBrains ||
+ (primaryBrainId && (loadingAnalytics || loadingCorrections || loadingLessons))
+ ) {
+ return <LoadingSpinner className="py-20" />
+ }Also applies to: 52-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/dashboard/page.tsx around lines 26 - 34, The
widgets are rendering a transient empty state because useApi hooks flip from
null to loading after primaryBrainId appears; fix by gating the widget rendering
on the actual dependent responses instead of just primaryBrainId: keep the
existing useApi calls (useApi, analytics, correctionsResp, lessonsResp,
primaryBrainId) but wrap/chart-render blocks so they only render when the
corresponding response is present (e.g. analytics !== undefined / null before
rendering analytics chart, correctionsResp exists before rendering corrections
panel, lessonsResp exists before rendering lessons panel); apply the same
guarding for all dependent widgets referenced in this file (lines ~52-125).
| const { data: correctionsResp } = useApi<PaginatedResponse<Correction> | Correction[]>( | ||
| primaryBrainId ? `/brains/${primaryBrainId}/corrections` : null, | ||
| ) | ||
| const { data: lessonsResp } = useApi<PaginatedResponse<Lesson> | Lesson[]>( | ||
| primaryBrainId ? `/brains/${primaryBrainId}/lessons` : null, | ||
| ) | ||
|
|
||
| const corrections = useMemo<Correction[]>(() => { | ||
| if (!correctionsResp) return [] | ||
| return Array.isArray(correctionsResp) ? correctionsResp : correctionsResp.data | ||
| }, [correctionsResp]) | ||
|
|
||
| const lessons = useMemo<Lesson[]>(() => { | ||
| if (!lessonsResp) return [] | ||
| return Array.isArray(lessonsResp) ? lessonsResp : lessonsResp.data | ||
| }, [lessonsResp]) |
There was a problem hiding this comment.
Paginated responses are being treated as complete datasets.
When these endpoints return PaginatedResponse, this page unwraps a single .data page and computes every KPI/chart from that slice only. Once a brain has more than one page of corrections or lessons, the dashboard becomes silently wrong.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/dashboard/page.tsx around lines 29 - 44, The
dashboard is unwrapping only the first page from useApi (correctionsResp,
lessonsResp) and computing KPIs from that single .data slice; update the
data-loading to aggregate all pages for a brain when primaryBrainId is set:
either extend useApi to accept and auto-fetch all pages or add a pagination loop
that uses the paginated response (PaginatedResponse) to request subsequent pages
until exhausted and then set corrections and lessons to the full concatenated
arrays (referencing correctionsResp, lessonsResp, corrections, lessons and
useApi/primaryBrainId to locate the logic). Ensure the final corrections and
lessons arrays contain all items across pages before any KPI/chart calculations
run.
| <Link href="/login"> | ||
| <Button variant="outline" className="w-full">Back to login</Button> | ||
| </Link> |
There was a problem hiding this comment.
Nested interactive elements create accessibility issues.
Wrapping a <Button> inside a <Link> creates nested interactive elements, which can cause screen reader confusion and keyboard navigation issues. Use either a styled link or a button with onClick navigation.
♿ Proposed fix using Button's asChild pattern (if supported) or direct styling
- <Link href="/login">
- <Button variant="outline" className="w-full">Back to login</Button>
- </Link>
+ <Link href="/login" className="inline-flex w-full items-center justify-center rounded-md border border-input bg-background px-4 py-2 text-sm font-medium hover:bg-accent hover:text-accent-foreground">
+ Back to login
+ </Link>Or if the Button component supports asChild:
<Button variant="outline" className="w-full" asChild>
<Link href="/login">Back to login</Link>
</Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/forgot-password/page.tsx` around lines 39 - 41, The
current JSX nests the Button component inside Link, creating nested interactive
elements (Link and Button) which is inaccessible; update the markup to use one
interactive element only by either rendering the Button as the Link using the
Button's asChild prop (e.g., pass asChild to Button and place Link as its child)
or by styling the Link to look like the Button and remove the Button wrapper;
change the <Link> / <Button> usage around the "Back to login" text accordingly
(refer to the Link and Button components in page.tsx) and ensure keyboard focus
and semantics remain a single interactive element.
| @theme { | ||
| --color-bg: #0C1120; | ||
| --color-surface: #151D30; | ||
| --color-text: #F8FAFC; | ||
| --color-body: #8895A7; | ||
| --color-accent-blue: #3A82FF; | ||
| --color-accent-violet: #7C3AED; | ||
| --color-success: #22C55E; | ||
| --color-warning: #EAB308; | ||
| --color-destructive: #EF4444; | ||
| --color-border: rgba(255, 255, 255, 0.08); | ||
| --color-border-hover: rgba(58, 130, 255, 0.25); | ||
|
|
||
| --font-sans: 'Inter', system-ui, sans-serif; | ||
| --font-heading: 'Space Grotesk', sans-serif; | ||
| --font-mono: 'JetBrains Mono', monospace; | ||
|
|
||
| --radius-card: 0.625rem; | ||
| --radius-item: 0.5rem; | ||
| --radius-badge: 0.25rem; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "(globals\.css|\.stylelintrc)" | head -20Repository: Gradata/gradata
Length of output: 90
🏁 Script executed:
fd -t f "globals.css" --max-results 5Repository: Gradata/gradata
Length of output: 90
🏁 Script executed:
fd -t f "\.stylelintrc" -o "stylelint" --max-results 10Repository: Gradata/gradata
Length of output: 196
🏁 Script executed:
fd ".stylelintrc"Repository: Gradata/gradata
Length of output: 41
🏁 Script executed:
fd "stylelint" -t fRepository: Gradata/gradata
Length of output: 41
🏁 Script executed:
cat -n cloud/dashboard/app/globals.cssRepository: Gradata/gradata
Length of output: 2594
🏁 Script executed:
find . -type f \( -name "package.json" -o -name "stylelint.config.*" -o -name ".stylelintrc.*" \) | grep -v node_modules | head -10Repository: Gradata/gradata
Length of output: 109
🏁 Script executed:
rg "stylelint" --type json --max-count 20Repository: Gradata/gradata
Length of output: 222
🏁 Script executed:
cat -n package.json | head -100Repository: Gradata/gradata
Length of output: 103
🏁 Script executed:
cat -n .stylelintrc.jsonRepository: Gradata/gradata
Length of output: 1643
🏁 Script executed:
cat -n cloud/dashboard/package.json | head -150Repository: Gradata/gradata
Length of output: 1419
🏁 Script executed:
rg "tailwind" cloud/dashboard/package.json -A 2 -B 2Repository: Gradata/gradata
Length of output: 368
🏁 Script executed:
rg "postcss" --type jsonRepository: Gradata/gradata
Length of output: 150
🏁 Script executed:
find . -type f \( -name "postcss.config.*" -o -name "tailwind.config.*" \) | grep -v node_modulesRepository: Gradata/gradata
Length of output: 95
🏁 Script executed:
fd -t f "postcss.config" -o "tailwind.config"Repository: Gradata/gradata
Length of output: 208
🏁 Script executed:
cat -n cloud/dashboard/postcss.config.mjsRepository: Gradata/gradata
Length of output: 199
🏁 Script executed:
find cloud/dashboard -type f -name "*tailwind*"Repository: Gradata/gradata
Length of output: 41
Teach Stylelint about Tailwind's @theme at-rule.
The @theme at-rule in globals.css (line 3) will trigger scss/at-rule-no-unknown from the inherited Stylelint config until explicitly ignored. Add the ignoreAtRules option to allow Tailwind v4's custom at-rules.
Suggested config change
// .stylelintrc.json
{
"extends": [
"stylelint-config-standard-scss"
],
"rules": {
+ "scss/at-rule-no-unknown": [true, { "ignoreAtRules": ["theme"] }],
"selector-id-pattern": null,🧰 Tools
🪛 Stylelint (17.6.0)
[error] 3-3: Unexpected unknown at-rule "@theme" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/globals.css` around lines 3 - 23, The SCSS linter is
flagging Tailwind's custom at-rule `@theme`; update your Stylelint configuration
to teach the scss/at-rule-no-unknown rule to ignore Tailwind at-rules by adding
an ignoreAtRules array that includes "theme" (and common Tailwind at-rules such
as "tailwind", "apply", "screen", "variants", "responsive", "layer" as needed).
Modify the scss/at-rule-no-unknown rule entry in your Stylelint config (e.g.,
.stylelintrc.js or package.json stylelint key) to include ignoreAtRules with
"theme" so the `@theme` declaration in globals.css no longer triggers the error.
|
|
||
| const api = axios.create({ | ||
| baseURL: import.meta.env.VITE_API_URL || 'https://gradata-production.up.railway.app/api/v1', | ||
| baseURL: process.env.NEXT_PUBLIC_API_URL || 'https://gradata-production.up.railway.app/api/v1', |
There was a problem hiding this comment.
Remove implicit production fallback for required API URL.
On Line 5, defaulting to the production Railway URL can silently send non-prod traffic/data to prod when NEXT_PUBLIC_API_URL is misconfigured. Since this env is required, fail fast instead of auto-fallback.
Proposed fix
-const api = axios.create({
- baseURL: process.env.NEXT_PUBLIC_API_URL || 'https://gradata-production.up.railway.app/api/v1',
-})
+const apiBaseUrl = process.env.NEXT_PUBLIC_API_URL
+if (!apiBaseUrl) {
+ throw new Error('Missing required env: NEXT_PUBLIC_API_URL')
+}
+
+const api = axios.create({
+ baseURL: apiBaseUrl,
+})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/lib/api.ts` at line 5, The code currently sets baseURL:
process.env.NEXT_PUBLIC_API_URL ||
'https://gradata-production.up.railway.app/api/v1', which silently falls back to
production; remove the implicit fallback and instead fail fast when
NEXT_PUBLIC_API_URL is missing by checking process.env.NEXT_PUBLIC_API_URL
during initialization (the baseURL assignment used to configure the API client)
and throwing or logging a clear error if it's undefined so the app does not
accidentally send traffic to prod.
| ".next/types/**/*.ts", | ||
| ".next/dev/types/**/*.ts" | ||
| ], | ||
| "exclude": [ | ||
| "node_modules", | ||
| "out", | ||
| ".next" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 16 tsconfig.json recommended exclude settings
💡 Result:
The recommended "exclude" settings in tsconfig.json for Next.js 16 are ["node_modules"]. This is the default that Next.js automatically adds or expects when running next dev or next build if not present, as TypeScript excludes it by default but Next.js explicitly enforces it. In practice, users often extend this for testing files or generated directories, e.g.: "exclude": [ "node_modules", "src//.spec.ts", "src//.test.ts", ".next/types//*.ts" ] No specific changes for Next.js 16 were found; it follows the standard Next.js TypeScript template from create-next-app, which uses TypeScript 5+ and modern defaults like target ES2017 (no es5).
Citations:
- 1: `next dev` rewrites `tsconfig.json` when it shouldn't vercel/next.js#39942
- 2: https://github.com/vercel/next.js/blob/canary/packages/create-next-app/create-app.ts
- 3: create-next-app still generates tsconfig.json with target: es5 vercel/next.js#58640
- 4: Improve initial setup with new App Router TypeScript project vercel/next.js#64826
- 5: Custom tsconfig path breaks next typescript plugin vercel/next.js#79086
- 6: Next.js sets invalid and irrelevant tsconfig options. vercel/next.js#45617
Conflicting include/exclude for .next types directory.
Lines 38-39 include .next/types/**/*.ts and .next/dev/types/**/*.ts, but line 44 excludes the entire .next directory. TypeScript's exclude takes precedence over include, preventing the generated Next.js route types from being included. This defeats the purpose of explicitly including .next/types and may cause missing type definitions for route parameters.
The Next.js 16 standard recommends excluding only node_modules, not .next. Remove the .next exclusion:
Proposed fix
"exclude": [
"node_modules",
- "out",
- ".next"
+ "out"
]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ".next/types/**/*.ts", | |
| ".next/dev/types/**/*.ts" | |
| ], | |
| "exclude": [ | |
| "node_modules", | |
| "out", | |
| ".next" | |
| ] | |
| ".next/types/**/*.ts", | |
| ".next/dev/types/**/*.ts" | |
| ], | |
| "exclude": [ | |
| "node_modules", | |
| "out" | |
| ] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/tsconfig.json` around lines 38 - 45, The tsconfig currently
includes ".next/types/**/*.ts" and ".next/dev/types/**/*.ts" but also excludes
the entire ".next" directory, which prevents those included paths from being
used; remove ".next" from the "exclude" array so the explicit includes take
effect (update the tsconfig.json exclude list to only contain "node_modules" and
"out"), ensuring Next.js generated route/type files are available to the
TypeScript compiler.
| Sentry is wired into the backend (FastAPI on Railway) and the dashboard (Vite+React on Cloudflare Pages). Both are **disabled by default** — they stay no-op until you set the DSN env vars. | ||
|
|
There was a problem hiding this comment.
The frontend Sentry runbook is still Vite-era.
This doc tells operators to set VITE_* vars, rebuild a Vite app, and update vite.config.ts / src/main.tsx, but this PR migrated the dashboard to Next.js and renamed the public env vars to NEXT_PUBLIC_*. Following these steps will leave client-side Sentry disabled on Cloudflare Pages.
Suggested fix
-Sentry is wired into the backend (FastAPI on Railway) and the dashboard (Vite+React on Cloudflare Pages).
+Sentry is wired into the backend (FastAPI on Railway) and the dashboard (Next.js on Cloudflare Pages).-VITE_SENTRY_DSN=https://...@o0.ingest.sentry.io/<project-id>
-VITE_SENTRY_ENVIRONMENT=production # optional, defaults to MODE
-VITE_SENTRY_RELEASE=gradata-dashboard@<ver> # optional
+NEXT_PUBLIC_SENTRY_DSN=https://...@o0.ingest.sentry.io/<project-id>
+NEXT_PUBLIC_SENTRY_ENVIRONMENT=production
+NEXT_PUBLIC_SENTRY_RELEASE=gradata-dashboard@<ver>-- `cloud/dashboard/package.json` — added `@sentry/react`, `@sentry/vite-plugin`
-- `cloud/dashboard/src/lib/sentry.ts` — init + PII scrubber
-- `cloud/dashboard/src/main.tsx` — calls `initSentry()` before `createRoot`
-- `cloud/dashboard/vite.config.ts` — conditional source-map upload plugin
+- `cloud/dashboard/package.json` — added `@sentry/nextjs`
+- `cloud/dashboard/sentry.client.config.ts` — browser init
+- `cloud/dashboard/sentry.server.config.ts` — server init placeholder for static export
+- `cloud/dashboard/sentry.edge.config.ts` — edge init placeholder for static export
+- `cloud/dashboard/instrumentation.ts` — Next.js instrumentation entrypointAlso applies to: 34-50, 91-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/SENTRY-SETUP.md` around lines 3 - 4, The frontend Sentry runbook still
references Vite-era setup (VITE_* env vars, vite.config.ts, src/main.tsx) but
the dashboard now uses Next.js with public vars renamed to NEXT_PUBLIC_*; update
the doc to instruct operators to set NEXT_PUBLIC_SENTRY_DSN (and any
NEXT_PUBLIC_SENTRY_ENV, NEXT_PUBLIC_SENTRY_RELEASE), redeploy the Next.js
dashboard on Cloudflare Pages, and replace instructions about modifying
vite.config.ts/src/main.tsx with guidance to initialize Sentry in the Next.js
entrypoint (e.g., app/layout.tsx or pages/_app.tsx) and any
next.config.js/runtime config changes so client-side Sentry is actually enabled.
Ensure the same corrections are applied to the other affected sections (lines
~34-50 and ~91-100).
| ``` | ||
| GRADATA_SENTRY_DSN=https://...@o0.ingest.sentry.io/<project-id> | ||
| GRADATA_SENTRY_TRACES_SAMPLE_RATE=0.1 # optional, defaults to 0.1 | ||
| GRADATA_ENVIRONMENT=production # already set | ||
| ``` |
There was a problem hiding this comment.
Add language tags to the fenced blocks.
markdownlint is already warning on these fences, so this file will keep failing doc lint until they are marked as bash, text, etc.
Also applies to: 28-30, 38-47, 53-55
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/SENTRY-SETUP.md` around lines 20 - 24, The fenced code blocks
containing environment variable examples (e.g., the block with
GRADATA_SENTRY_DSN, GRADATA_SENTRY_TRACES_SAMPLE_RATE, GRADATA_ENVIRONMENT) need
explicit language tags to satisfy markdownlint; update each triple-backtick
fence (including the other blocks at ranges referenced) to use an appropriate
tag such as ```bash or ```env so the blocks are recognized as shell/environment
snippets and the linter warnings are resolved.
| def test_init_sentry_enabled_when_dsn_set(caplog): | ||
| """Valid DSN must init Sentry and log success (visible misconfig).""" | ||
| # Use an obviously-fake DSN that Sentry SDK accepts syntactically | ||
| fake_dsn = "https://publickey@o0.ingest.sentry.io/0" | ||
| settings = _make_settings(sentry_dsn=fake_dsn, environment="test") | ||
| with caplog.at_level(logging.INFO, logger="app.sentry_init"): | ||
| init_sentry(settings) | ||
| # Client should exist after init (use 2.x API, not deprecated Hub) | ||
| client = sentry_sdk.get_client() | ||
| assert client is not None and client.is_active(), "Sentry SDK client not active" | ||
| assert any("initialized" in rec.message.lower() for rec in caplog.records) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the test file and examine its contents
find . -name "test_sentry.py" -type f | head -5Repository: Gradata/gradata
Length of output: 87
🏁 Script executed:
# Look for conftest or fixtures related to sentry
find . -name "conftest.py" -o -name "*sentry*" | grep -E "(conftest|sentry)" | head -20Repository: Gradata/gradata
Length of output: 284
🏁 Script executed:
# Search for the init_sentry function
rg "def init_sentry" --type py -A 15Repository: Gradata/gradata
Length of output: 1233
🏁 Script executed:
cat -n ./cloud/tests/test_sentry.pyRepository: Gradata/gradata
Length of output: 5508
🏁 Script executed:
cat -n ./cloud/tests/conftest.py | head -100Repository: Gradata/gradata
Length of output: 3256
🏁 Script executed:
cat -n ./cloud/app/sentry_init.pyRepository: Gradata/gradata
Length of output: 4938
🏁 Script executed:
rg "sentry" ./cloud/tests/conftest.py -iRepository: Gradata/gradata
Length of output: 41
🏁 Script executed:
rg "def.*sentry" ./tests/conftest.py -iRepository: Gradata/gradata
Length of output: 41
🏁 Script executed:
# Check if there are any fixtures that might reset sentry state
rg "@pytest.fixture" ./cloud/tests/conftest.py -A 5Repository: Gradata/gradata
Length of output: 721
🏁 Script executed:
# Search for any sentry_sdk cleanup/close calls in tests
rg "sentry_sdk\." ./cloud/tests/test_sentry.pyRepository: Gradata/gradata
Length of output: 95
🏁 Script executed:
# Check Sentry SDK documentation behavior on multiple init calls
rg "close|reset|shutdown" ./cloud/tests/ -iRepository: Gradata/gradata
Length of output: 167
🏁 Script executed:
# Check if sentry_sdk.init is called during app creation
rg "create_app|init_sentry" ./cloud/app/main.py -B 2 -A 8Repository: Gradata/gradata
Length of output: 617
🏁 Script executed:
# Look for any test dependencies or test ordering issues
rg "test_init_sentry|test_app_creates" ./cloud/tests/test_sentry.py -B 1 -A 10Repository: Gradata/gradata
Length of output: 1513
🏁 Script executed:
# Search for any pytest markers or test configuration that might affect test isolation
find ./cloud/tests -name "pytest.ini" -o -name "pyproject.toml" | xargs grep -l "pytest" 2>/dev/nullRepository: Gradata/gradata
Length of output: 43
This test initializes a global Sentry SDK client that is never cleaned up between test runs.
init_sentry() calls the real sentry_sdk.init(), which sets up a process-wide client. The test asserts this client is active, but never resets it. This pollutes subsequent tests—a test designed to check the disabled path will still find an active client if this test ran first, making the suite dependent on test execution order.
Consider mocking sentry_sdk.init() instead of initializing the actual SDK in the test, so the test verifies your code passes the correct parameters to Sentry without persisting global state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/tests/test_sentry.py` around lines 35 - 45, The test
test_init_sentry_enabled_when_dsn_set calls init_sentry which invokes the real
sentry_sdk.init and leaves a global client active, polluting other tests; update
this test to mock sentry_sdk.init (or patch it) so init_sentry is exercised
without creating a real global client, assert the mock was called with the
expected DSN and environment, and remove any assertions that rely on
sentry_sdk.get_client() being active; reference the test function name
test_init_sentry_enabled_when_dsn_set, the init_sentry function under test, and
the sentry_sdk.init call to locate where to apply the mock/patch.
Fills every deferred item from S105 plan. No more fixtures-only pages —
everything wires to a backend endpoint with graceful fallback to demo
data when the brain has no real events yet.
Backend (cloud/app/routes/):
- meta_rules.py: GET /brains/{id}/meta-rules — synthesized principles
from meta_rules table, newest first, no raw correction content
- activity.py: GET /brains/{id}/activity — learning events feed filtered
to visible kinds (graduation, self-healing, recurrence, meta-rule-
emerged, convergence, alert)
- rule_patches.py: GET /brains/{id}/rule-patches — self-healing audit
trail + POST /brains/{id}/rule-patches/{id}/rollback creates inverse
patch and restores lesson description (append-only audit)
- 10 new tests (test_new_routes.py), 58/58 backend tests green
Dashboard pages (cloud/dashboard/app/(dashboard)/):
- settings: Stripe tier pricing cards ($29 Cloud, $99 Team, Enterprise)
with upgrade CTAs routing to /billing/checkout
- api-keys: V3-themed port with dialog flow for key generation + display
- billing: Current plan + Stripe customer portal link + period end
- brain?id=...: Brain detail (query-param, not [id] — simpler under
static export) reusing KpiStrip + GraduationProgressBar + RulesPanel
- corrections: severity-filtered correction list, newest first
- rules: state-filtered lesson list (INSTINCT/PATTERN/RULE), confidence-
sorted
- meta-rules: standalone MetaRulesGrid page
- observability: A/B proof + convergence signal + methodology link
(differentiator page)
- privacy: PrivacyPosturePanel + explicit "what reaches cloud" list
- self-healing: PatchKind diffs with rollback button wired to API
- operator: god-mode for gradata.ai/sprites.ai domains — MRR/ARR/churn/
NRR KPIs, customer list with health + plan + MRR, alerts panel
- setup: SDK install wizard (pip install → API key → init snippet →
verify), progress bar tracks completion
- notifications: alert toggles (correction spike, rule regression, meta
emerged) + digest cadence (weekly default per SIM16) + Slack webhook
- team: KPI strip + leaderboard ("whose AI learned fastest" by Δ %) +
members list with invite dialog. All gated behind PlanGate(requires=team)
- team/members: invite flow + role management (owner/admin/member)
New components + fixtures:
- PlanBadge + PlanGate: tier badge + "upgrade to unlock" blur overlay
for Cloud/Team/Enterprise features
- mock-team.ts, mock-operator.ts, mock-self-healing.ts: fixtures for
surfaces that fall back when backend returns empty
- Sidebar now includes Observability, Privacy, Setup, Operator sections
Data flow:
- MetaRulesGrid + ActivityFeed + SelfHealing now fetch real data and
gracefully fall back to fixtures when brain has no events yet
(avoids empty states during cold start / pre-launch demo)
Build: 21 static routes, 0 errors.
Co-Authored-By: Gradata <noreply@gradata.ai>
Deploying gradata-dashboard with
|
| Latest commit: |
d908a9d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a920b58b.gradata-dashboard.pages.dev |
| Branch Preview URL: | https://worktree-dashboard-nextjs-re.gradata-dashboard.pages.dev |
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Actionable comments posted: 24
♻️ Duplicate comments (3)
cloud/dashboard/src/components/brain/ActivityFeed.tsx (1)
70-75:⚠️ Potential issue | 🟡 MinorGuard
ago()against invalid timestamps.
Dateparsing can returnNaN, which leads to invalid relative-time text. Add a parse guard and safe fallback.Proposed fix
const ago = (iso: string): string => { - const diffMs = Date.now() - new Date(iso).getTime() + const ts = Date.parse(iso) + if (!Number.isFinite(ts)) return 'just now' + const diffMs = Math.max(0, Date.now() - ts) const h = Math.floor(diffMs / 3600_000) if (h < 1) return 'just now' if (h < 24) return `${h}h ago` return `${Math.floor(h / 24)}d ago` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx` around lines 70 - 75, The ago(iso: string) helper can produce NaN-based results when new Date(iso).getTime() fails; add a parse guard in the ago function to detect invalid dates (use Number.isFinite or isNaN on the timestamp) and return a safe fallback like 'unknown' or 'just now' when the timestamp is invalid, otherwise continue computing h and the existing relative strings; update the ago function signature and body (the ago function) accordingly so it handles null/invalid iso inputs safely.cloud/dashboard/src/components/layout/DashboardLayout.tsx (1)
49-83:⚠️ Potential issue | 🟠 MajorRestore a mobile collapse/drawer path for the sidebar.
The layout still hard-reserves
240pxfor navigation on every viewport and always renders the sidebar, so small screens lose too much content width with no way to dismiss nav. This remains a release blocker for phone/narrow-tablet usability.Also applies to: 109-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx` around lines 49 - 83, The sidebar currently always reserves 240px via the aside element (class includes w-[240px] min-w-[240px]) and is always rendered, which breaks small-screen layouts; change the layout to hide the aside on small viewports and provide a toggleable drawer: add a boolean state (e.g., isSidebarOpen) in the DashboardLayout component, replace the static aside rendering with responsive classes (e.g., hidden on sm/md breakpoints and visible on larger screens) and render the aside as a slide-over panel when isSidebarOpen is true; add a header toggle button that flips isSidebarOpen, render an overlay/backdrop that closes the drawer on click/Escape, and reuse the same SECTIONS map/Link items inside the drawer so behavior (active detection via pathname and cn usage) remains identical.cloud/dashboard/src/components/brain/MetaRulesGrid.tsx (1)
98-122: 🧹 Nitpick | 🔵 TrivialMove
Cardto module scope to avoid per-render component recreation.Line 98 defines
CardinsideMetaRulesGrid, which recreates the component function on every parent render. Extracting it to top-level (optionally memoized) keeps identity stable.♻️ Proposed refactor
+function Card({ m }: { m: DisplayMetaRule }) { + return ( + <li className="rounded-[0.5rem] border border-[var(--color-border)] bg-white/[0.02] p-4 transition-all hover:border-[var(--color-border-hover)]"> + <div className="mb-2 flex items-start justify-between gap-3"> + <div className="text-[13px] leading-snug">{m.title}</div> + <span className={`shrink-0 rounded-[0.25rem] px-2 py-0.5 text-[9px] font-semibold uppercase tracking-wider ${TIER_STYLE[m.tier]}`}> + {m.tier} + </span> + </div> + <div className="mb-2 text-[11px] font-mono text-[var(--color-body)]"> + {m.source_description} + </div> + {m.source_categories.length > 0 && ( + <div className="flex flex-wrap gap-1.5"> + {m.source_categories.map((cat) => ( + <span + key={cat} + className="rounded-[0.25rem] bg-[rgba(58,130,255,0.08)] px-2 py-0.5 text-[10px] font-medium text-[var(--color-accent-blue)]" + > + {cat} + </span> + ))} + </div> + )} + </li> + ) +} + export function MetaRulesGrid() { @@ - const Card = ({ m }: { m: DisplayMetaRule }) => ( - ... - )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/brain/MetaRulesGrid.tsx` around lines 98 - 122, The Card component is defined inside MetaRulesGrid causing it to be recreated on every render; move Card to module scope as a top-level React component (export or file-local) that accepts props like ({ m }: { m: DisplayMetaRule }) and uses TIER_STYLE, then import/consume it from MetaRulesGrid (optionally wrap with React.memo for stable identity). Ensure you preserve the JSX structure and classNames, keep the same prop type DisplayMetaRule, and update any references inside MetaRulesGrid to pass the m prop into the relocated Card.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx:
- Around line 34-39: The create flow sets newKey on success but never closes the
create dialog, so showCreate remains true and the create dialog reopens after
the one‑time key is dismissed; update the success path in the POST handler (the
block that calls api.post and then setNewKey, setKeyName, refetch) to also call
setShowCreate(false) (and mirror the same fix in the similar create flow around
the 126-156 block) so the create dialog is explicitly closed when a key is
successfully created.
In `@cloud/dashboard/app/`(dashboard)/billing/page.tsx:
- Around line 21-29: The page currently falls back to a fake "free" plan when
API responses fail; instead, detect API errors from the useApi hook (e.g., the
error fields returned alongside data from useApi for profile and subscription)
and render the existing ErrorState component used in settings rather than
computing currentPlan when either request errored. Specifically, update the
logic around useApi/profile/sub (the variables useApi, profile, sub,
loadingProfile, loadingSub, currentPlan, planMeta) to: if either API reports an
error (profileError || subError) return <ErrorState ...> (matching the pattern
in cloud/dashboard/app/(dashboard)/settings/page.tsx) and only compute
currentPlan/planMeta when no error and loading is false.
In `@cloud/dashboard/app/`(dashboard)/brain/page.tsx:
- Around line 29-52: The page currently treats failed/errored secondary fetches
as empty data; update the useApi result handling so analytics, correctionsResp
and lessonsResp are only considered settled when their respective loading is
false and error is null — i.e., keep their original loading/error flags from
useApi and do not coerce undefined into empty arrays inside the
corrections/lessons useMemo; change the kpis and gradCounts computations
(computeKpis and computeGraduationCounts) to run only when the corresponding
data is present and error-free, and surface or forward the loading/error states
to the KPI/graduation widgets rather than rendering zero-state values.
In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx:
- Around line 20-34: The code hides API failures by treating missing data as
empty state; update the two useApi calls (the one that returns { data: brains,
loading: loadingBrains } and the one that returns { data: resp, loading }) to
also read their error values (e.g., { error: brainsError } and { error:
respError }) and render an error UI when present instead of the EmptyState/empty
corrections: if brainsError show an ErrorState (or similar) with the brainsError
message instead of "No brain yet", and if respError show an ErrorState with
respError text instead of rendering an empty corrections list, preserving
LoadingSpinner while loading and keeping existing primaryId/corrections logic
otherwise.
- Around line 31-45: The displayed "newest first" promise is wrong because
filtered is never sorted; update the logic that builds filtered (based on
corrections and filter) to produce a new array sorted by created_at descending
(most recent first) — e.g., take corrections (or corrections.filter(...)), copy
it (slice or spread) and sort by comparing Date.parse(a.created_at) vs
Date.parse(b.created_at) or numeric timestamps so that the variable filtered
reflects newest-first order; ensure you do not mutate the original corrections
array and keep the existing filter condition using filter and the severities
list.
In `@cloud/dashboard/app/`(dashboard)/meta-rules/page.tsx:
- Line 1: Remove the top-level 'use client' directive from the page component so
the page stays a Server Component; specifically edit page.tsx and delete the
"'use client'" line, keep the page exporting the component that renders
<MetaRulesGrid /> (MetaRulesGrid should remain a Client Component) and ensure
you do not move or remove any client-only hooks or APIs from MetaRulesGrid
itself.
In `@cloud/dashboard/app/`(dashboard)/notifications/page.tsx:
- Around line 39-44: Add a "saving" state and prevent concurrent saves in
handleSave by returning early if saving is true, set saving true at start and
false when complete; store the timeout id returned by setTimeout (used to clear
the saved indicator) in a ref or module-scope variable and call clearTimeout
before setting a new timeout; also clear that timeout in a useEffect cleanup to
avoid state updates after unmount; update the Save button to be disabled when
the saving state is true and show a saving label. Refer to handleSave, setSaved,
(new) setSaving/saving, and the setTimeout call that clears the saved state for
locating where to apply changes.
In `@cloud/dashboard/app/`(dashboard)/observability/page.tsx:
- Around line 71-82: In ConvergenceRow, clamp and sanitize the incoming cosine
before using it for display and the progress width: ensure cosine is numeric and
finite, clamp it to the 0..1 range (e.g., replace NaN/Infinity with 0 or nearest
bound), store the sanitized value in a local variable (e.g., clampedCosine) and
use that for both the toFixed(2) output and the style width calculation so you
never render invalid percentages or NaN.
- Around line 38-40: The paragraph element rendering "TODO(backend): wire to
real cross-user cosine similarity when endpoint ships." is internal debug text
visible to users; update the user-facing copy in the paragraph (the <p
className="mt-6 font-mono text-[11px] text-[var(--color-body)]"> element in
page.tsx) to a neutral user-facing message or remove it entirely—for example
replace with "Cross-user cosine similarity coming soon" or hide it behind a
dev-only flag so internal TODOs are not rendered to end users.
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx:
- Around line 11-15: The operator dataset is being imported at module scope
(OPERATOR_EMAIL_DOMAINS and the mock operator data) inside the client component
page.tsx which bakes sensitive data into the bundle; instead, remove the mock
dataset and OPERATOR_EMAIL_DOMAINS from the client module, implement a
server-side protected API endpoint (e.g., /api/admin/operators) that validates
the authenticated user/role and returns operator data, and update the client
component to fetch operator data from that endpoint at runtime (or convert the
page to a server component and call the server API directly); ensure all
references to OPERATOR_EMAIL_DOMAINS and the in-module mock operators are
deleted so the data is no longer bundled to the client.
In `@cloud/dashboard/app/`(dashboard)/privacy/page.tsx:
- Around line 12-23: The component shows "0 KB" while analytics is still loading
because only brains loading (loadingBrains) is gated; update the analytics
useApi call to also return its loading flag (e.g., const { data: analytics,
loading: loadingAnalytics } = useApi<BrainAnalytics>(primaryId ?
`/brains/${primaryId}/analytics` : null)) and then include loadingAnalytics in
the render gate (e.g., if (loadingBrains || loadingAnalytics) return
<LoadingSpinner ... />) or otherwise avoid computing/printing footprintKb until
analytics is defined; adjust footprintKb (useMemo) to only run when analytics
exists and ensure the UI renders a loader or placeholder until analytics
resolves.
- Around line 79-86: The nested ternary assignments for dot and prefix based on
tone are hard to maintain; replace them with a typed lookup map (e.g., a const
TONE_META: Record<ToneType, { dot: string; prefix: string }>) keyed by the
existing tone variable, then derive dot and prefix from TONE_META[tone] with a
safe fallback for unknown tones; update references where dot and prefix are
defined (the dot and prefix constants) so the logic uses the map and remove the
nested ternaries.
In `@cloud/dashboard/app/`(dashboard)/rules/page.tsx:
- Around line 18-33: The page currently treats failed API calls as empty data
because it only checks loading and the derived primaryId/lessons; update the
handling to surface API errors from useApi instead: read the error fields
returned by useApi for both the brains call (const { data: brains, loading:
loadingBrains, error: brainsError } = useApi<Brain[]>('/brains')) and the
lessons call (const { data: resp, loading, error: lessonsError } =
useApi<...>(primaryId ? `/brains/${primaryId}/lessons` : null)), and if either
brainsError or lessonsError is truthy render the shared ErrorState component
with a helpful message instead of falling through to EmptyState; keep the
existing LoadingSpinner logic (loadingBrains || loading) and only compute
primaryId/lessons after checking brainsError so that backend failures are shown
as ErrorState rather than "No brain yet" or "No rules."
- Around line 24-30: The current computed variable filtered sorts the lessons
array in place, mutating the original lessons reference derived from resp; to
fix, ensure you sort a copy instead of mutating lessons by creating a new array
before sorting (e.g., use lessons.slice() or spread [...lessons] when filter ===
'all', or always build a new array from the filter result) so that the original
lessons/resp data is not mutated; update the logic that computes filtered
(referencing lessons, filtered, and resp) to operate on a copied array prior to
calling .sort().
In `@cloud/dashboard/app/`(dashboard)/self-healing/page.tsx:
- Around line 93-107: The handleRollback function currently treats api.post as
always succeeding; wrap the network call in a try/catch so failures are handled
explicitly: call
api.post(`/brains/${primaryId}/rule-patches/${patchId}/rollback`) inside try, on
success update setRolledBack(patchId) and call refetch(), but in catch set a
failure indicator (e.g., call a toast/notification helper or setRollbackError
state keyed by patchId) so the user sees the rollback failed and do NOT mark the
patch as rolled back; keep the finally block to clear
setRollbackInFlight(patchId). Reference symbols: handleRollback, api.post,
setRolledBack, refetch, setRollbackInFlight and add/use a toast or
setRollbackError state for visible failure feedback.
In `@cloud/dashboard/app/`(dashboard)/settings/page.tsx:
- Around line 70-116: The plan grid is hardcoded to (['free','cloud','team'] as
const) so the enterprise tier from PLANS/PlanTier is never rendered; change the
iteration to derive tiers from PLANS (e.g. Object.keys(PLANS) as PlanTier[] or a
sorted array that includes 'enterprise') so the enterprise card is rendered and
the isCurrent/canUpgrade logic (using currentPlan, PLANS, upgrading,
handleUpgrade) works correctly; ensure the new iteration preserves the existing
key={tier} usage and button behavior.
- Line 26: currentPlan is cast directly from profile.plan to PlanTier and can be
an unexpected string; instead normalize and validate the plan against the PLANS
keys before using it: create a safePlan (e.g., let safePlan = typeof
profile.plan === 'string' ? profile.plan.toLowerCase() : 'free'), check that
PLANS hasOwnProperty(safePlan) (or use Object.keys(PLANS).includes(safePlan))
and if not fallback to 'free', then cast safePlan to PlanTier for subsequent
use; update references to currentPlan and the access of PLANS[currentPlan].rank
(and any other PLANS[currentPlan] usages) to use the validated safePlan so you
never index PLANS with an invalid key.
In `@cloud/dashboard/app/`(dashboard)/setup/page.tsx:
- Around line 22-54: The current onboarding treats fetch failures as empty
state; update the use of useApi for both '/api-keys' and '/brains' to surface
and handle fetch errors explicitly by extracting the error (e.g., keysError,
brainsError) alongside data and loading, then short-circuit normal wizard
rendering when an error exists (render an error message/UI and avoid showing
onboarding steps or assume no keys/brains). Specifically modify the
destructuring around useApi in this file (symbols: useApi, keys, brains,
loadingKeys, loadingBrains, hasKey, hasBrain) to check for error values and
return a clear error component or message when keysError or brainsError is
truthy, and ensure any side effects (like hasKey/hasBrain logic) only run when
there is no error.
- Around line 28-46: The snippet mistakenly interpolates the persisted prefix
(firstKeyPrefix) into the runnable api_key example, which is misleading since
the prefix is not the secret; update the initSnippet generation so
Brain.init(api_key=...) never contains a real-looking key by replacing
interpolation of firstKeyPrefix with a non-authentic placeholder (e.g.,
"gd_YOUR_API_KEY_HERE" or a rendered "gd_{PREFIX}…" string that clearly includes
an ellipsis) and ensure the variables firstKeyPrefix, initSnippet and the
Brain.init call reflect this change so the snippet cannot be mistaken for a
usable secret.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx:
- Around line 47-57: The header "Invite member" Button and the invite modal
(state setter setInviteOpen and any InviteModal/InviteDialog component) must be
gated the same way as the member list: check currentPlan for the "team"
entitlement and either wrap the Button and the modal render in the existing
PlanGate (featureName="Team member management") or disable the Button and avoid
rendering/opening the modal when currentPlan lacks "team"; specifically update
the Button onClick usage (setInviteOpen(true)) and the modal render (the
component that reads inviteOpen) so they are only reachable when PlanGate
permits (or short-circuit with a disabled Button and no modal mount when
currentPlan doesn't include the "team" feature).
- Around line 79-86: The Remove button (rendered where m.role !== 'owner' in
page.tsx) currently has no onClick or disabled state and thus wrongly advertises
a destructive action; either remove the JSX entirely until the backend exists or
render the Button disabled and add explanatory copy/tooltip (e.g., "Coming soon"
or "Not available") and set the disabled prop so it’s inert; if you prefer a
future-enabled stub, keep the element but add a noop onClick that opens a
modal/tooltip explaining the feature is unavailable and ensure className and
aria-disabled reflect the disabled state.
In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx`:
- Around line 85-100: The mapped feed items in the useMemo for items (using
real, normalizeKind, KIND_TITLE, extractDetail and created_at) must be sorted by
created_at (newest first) before applying slice(0, 8); modify the pipeline so
after mapping and filtering you sort by numeric timestamp (e.g., compare new
Date(a.created_at).getTime()) descending and then truncate to the first 8
entries — apply the same sorting+truncation change to the other similar feed
calculation later in the file.
- Around line 86-111: The mapping currently returns mockActivity only when real
is falsy or empty, but if real contains entries whose types all map to null via
normalizeKind the mapped result becomes empty while showingDemo remains false;
update the logic in the memo that builds the display list (the block using
real.map, normalizeKind, KIND_TITLE, extractDetail and filtering to
DisplayActivity) to capture the mapped-and-filtered array into a const (e.g.,
mapped = real.map(...).filter(...)) and if mapped.length === 0 fall back to
returning mockActivity; also ensure showingDemo uses the same condition (i.e.,
showingDemo = !real || mapped.length === 0) so the UI correctly treats “all
events filtered out” as demo.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx`:
- Around line 66-78: The nav links currently rely only on visual styling; update
the Link element (the one rendering item.href/item.label in DashboardLayout) to
set aria-current="page" when active (e.g., aria-current={active ? 'page' :
undefined}) so assistive tech can expose the active nav state, and mark the icon
span (the <span> wrapping item.icon) as decorative to screen readers by adding
aria-hidden="true" (and remove it from tab order if needed) so Unicode glyphs
aren’t announced.
---
Duplicate comments:
In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx`:
- Around line 70-75: The ago(iso: string) helper can produce NaN-based results
when new Date(iso).getTime() fails; add a parse guard in the ago function to
detect invalid dates (use Number.isFinite or isNaN on the timestamp) and return
a safe fallback like 'unknown' or 'just now' when the timestamp is invalid,
otherwise continue computing h and the existing relative strings; update the ago
function signature and body (the ago function) accordingly so it handles
null/invalid iso inputs safely.
In `@cloud/dashboard/src/components/brain/MetaRulesGrid.tsx`:
- Around line 98-122: The Card component is defined inside MetaRulesGrid causing
it to be recreated on every render; move Card to module scope as a top-level
React component (export or file-local) that accepts props like ({ m }: { m:
DisplayMetaRule }) and uses TIER_STYLE, then import/consume it from
MetaRulesGrid (optionally wrap with React.memo for stable identity). Ensure you
preserve the JSX structure and classNames, keep the same prop type
DisplayMetaRule, and update any references inside MetaRulesGrid to pass the m
prop into the relocated Card.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx`:
- Around line 49-83: The sidebar currently always reserves 240px via the aside
element (class includes w-[240px] min-w-[240px]) and is always rendered, which
breaks small-screen layouts; change the layout to hide the aside on small
viewports and provide a toggleable drawer: add a boolean state (e.g.,
isSidebarOpen) in the DashboardLayout component, replace the static aside
rendering with responsive classes (e.g., hidden on sm/md breakpoints and visible
on larger screens) and render the aside as a slide-over panel when isSidebarOpen
is true; add a header toggle button that flips isSidebarOpen, render an
overlay/backdrop that closes the drawer on click/Escape, and reuse the same
SECTIONS map/Link items inside the drawer so behavior (active detection via
pathname and cn usage) remains identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b03cf8d-436d-488e-b31f-a98673af5f6a
📒 Files selected for processing (22)
cloud/dashboard/app/(dashboard)/api-keys/page.tsxcloud/dashboard/app/(dashboard)/billing/page.tsxcloud/dashboard/app/(dashboard)/brain/page.tsxcloud/dashboard/app/(dashboard)/corrections/page.tsxcloud/dashboard/app/(dashboard)/meta-rules/page.tsxcloud/dashboard/app/(dashboard)/notifications/page.tsxcloud/dashboard/app/(dashboard)/observability/page.tsxcloud/dashboard/app/(dashboard)/operator/page.tsxcloud/dashboard/app/(dashboard)/privacy/page.tsxcloud/dashboard/app/(dashboard)/rules/page.tsxcloud/dashboard/app/(dashboard)/self-healing/page.tsxcloud/dashboard/app/(dashboard)/settings/page.tsxcloud/dashboard/app/(dashboard)/setup/page.tsxcloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/app/(dashboard)/team/page.tsxcloud/dashboard/src/components/brain/ActivityFeed.tsxcloud/dashboard/src/components/brain/MetaRulesGrid.tsxcloud/dashboard/src/components/brain/PlanBadge.tsxcloud/dashboard/src/components/layout/DashboardLayout.tsxcloud/dashboard/src/lib/fixtures/mock-operator.tscloud/dashboard/src/lib/fixtures/mock-self-healing.tscloud/dashboard/src/lib/fixtures/mock-team.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
cloud/dashboard/app/(dashboard)/observability/page.tsx (1)
12-67: Clean composition and reusable page structureNice decomposition with
ABProofPanel,GlassCard, andMethodologyLink; this keeps the page readable and easy to evolve.cloud/dashboard/app/(dashboard)/notifications/page.tsx (2)
46-132: LGTM!The JSX structure is well-organized with clear separation of notification preference sections. The use of
GlassCardcontainers and consistent styling aligns with the V3 theme.
135-152: LGTM!
ToggleRowis appropriately defined at module scope, avoiding unnecessary re-creation. The label-wrapped checkbox pattern ensures good accessibility, and the hover state provides clear visual feedback.cloud/dashboard/app/(dashboard)/privacy/page.tsx (1)
37-71: Good clarity in user-facing privacy contract content.The categories are explicit, and the “never sent” items are clearly distinguished from metadata/counters.
| const res = await api.post<ApiKeyCreateResponse>('/api-keys', { | ||
| name: keyName || 'Default', | ||
| }) | ||
| setNewKey(res.data) | ||
| setKeyName('') | ||
| refetch() |
There was a problem hiding this comment.
Reset showCreate on success or the create dialog reopens after dismissing the key.
After a successful POST, newKey is populated but showCreate stays true. That closes the first dialog only because of !newKey; once the user dismisses the one-time key dialog, newKey becomes null and the create dialog opens again immediately.
🐛 Suggested fix
try {
const res = await api.post<ApiKeyCreateResponse>('/api-keys', {
name: keyName || 'Default',
})
setNewKey(res.data)
+ setShowCreate(false)
setKeyName('')
refetch()Also applies to: 126-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx around lines 34 - 39, The
create flow sets newKey on success but never closes the create dialog, so
showCreate remains true and the create dialog reopens after the one‑time key is
dismissed; update the success path in the POST handler (the block that calls
api.post and then setNewKey, setKeyName, refetch) to also call
setShowCreate(false) (and mirror the same fix in the similar create flow around
the 126-156 block) so the create dialog is explicitly closed when a key is
successfully created.
| const { data: profile, loading: loadingProfile } = useApi<UserProfile>('/users/me') | ||
| const { data: sub, loading: loadingSub } = useApi<SubscriptionResponse>('/billing/subscription') | ||
| const [loadingPortal, setLoadingPortal] = useState(false) | ||
| const [portalError, setPortalError] = useState<string | null>(null) | ||
|
|
||
| if (loadingProfile || loadingSub) return <LoadingSpinner className="py-20" /> | ||
|
|
||
| const currentPlan = (sub?.plan ?? (profile?.plan as PlanTier) ?? 'free') as PlanTier | ||
| const planMeta = PLANS[currentPlan] ?? PLANS.free |
There was a problem hiding this comment.
Do not default failed billing/profile fetches into a fake “free plan” view.
If either request errors, currentPlan falls back and the page renders billing controls against incomplete data. That is especially risky here because it can misstate plan status and send users into the wrong Stripe flow. This page should surface ErrorState, like cloud/dashboard/app/(dashboard)/settings/page.tsx Lines 18-20.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/billing/page.tsx around lines 21 - 29, The
page currently falls back to a fake "free" plan when API responses fail;
instead, detect API errors from the useApi hook (e.g., the error fields returned
alongside data from useApi for profile and subscription) and render the existing
ErrorState component used in settings rather than computing currentPlan when
either request errored. Specifically, update the logic around useApi/profile/sub
(the variables useApi, profile, sub, loadingProfile, loadingSub, currentPlan,
planMeta) to: if either API reports an error (profileError || subError) return
<ErrorState ...> (matching the pattern in
cloud/dashboard/app/(dashboard)/settings/page.tsx) and only compute
currentPlan/planMeta when no error and loading is false.
| const { data: brain, loading, error, refetch } = useApi<Brain>(id ? `/brains/${id}` : null) | ||
| const { data: analytics } = useApi<BrainAnalytics>(id ? `/brains/${id}/analytics` : null) | ||
| const { data: correctionsResp } = useApi<PaginatedResponse<Correction> | Correction[]>( | ||
| id ? `/brains/${id}/corrections` : null, | ||
| ) | ||
| const { data: lessonsResp } = useApi<PaginatedResponse<Lesson> | Lesson[]>( | ||
| id ? `/brains/${id}/lessons` : null, | ||
| ) | ||
|
|
||
| const corrections = useMemo<Correction[]>(() => { | ||
| if (!correctionsResp) return [] | ||
| return Array.isArray(correctionsResp) ? correctionsResp : correctionsResp.data | ||
| }, [correctionsResp]) | ||
|
|
||
| const lessons = useMemo<Lesson[]>(() => { | ||
| if (!lessonsResp) return [] | ||
| return Array.isArray(lessonsResp) ? lessonsResp : lessonsResp.data | ||
| }, [lessonsResp]) | ||
|
|
||
| const kpis = useMemo( | ||
| () => (analytics ? computeKpis(analytics, corrections, lessons) : null), | ||
| [analytics, corrections, lessons], | ||
| ) | ||
| const gradCounts = useMemo(() => computeGraduationCounts(lessons), [lessons]) |
There was a problem hiding this comment.
Don't treat failed secondary fetches as real zero-state data.
analytics, correctionsResp, and lessonsResp drive the KPIs and graduation panels, but their loading/error states are ignored. If /brains/:id succeeds while one of those requests fails, this page renders missing KPIs or zero-count widgets as if the brain actually had no data. Surface a partial-error state, or keep the dependent widgets in loading/error until those requests settle successfully.
Possible direction
- const { data: analytics } = useApi<BrainAnalytics>(id ? `/brains/${id}/analytics` : null)
- const { data: correctionsResp } = useApi<PaginatedResponse<Correction> | Correction[]>(
+ const { data: analytics, loading: loadingAnalytics, error: analyticsError } =
+ useApi<BrainAnalytics>(id ? `/brains/${id}/analytics` : null)
+ const {
+ data: correctionsResp,
+ loading: loadingCorrections,
+ error: correctionsError,
+ } = useApi<PaginatedResponse<Correction> | Correction[]>(
id ? `/brains/${id}/corrections` : null,
)
- const { data: lessonsResp } = useApi<PaginatedResponse<Lesson> | Lesson[]>(
+ const {
+ data: lessonsResp,
+ loading: loadingLessons,
+ error: lessonsError,
+ } = useApi<PaginatedResponse<Lesson> | Lesson[]>(
id ? `/brains/${id}/lessons` : null,
)
...
- if (loading) return <LoadingSpinner className="py-20" />
- if (error) return <ErrorState message={error} onRetry={refetch} />
+ if (loading || loadingAnalytics || loadingCorrections || loadingLessons) {
+ return <LoadingSpinner className="py-20" />
+ }
+ if (error || analyticsError || correctionsError || lessonsError) {
+ return <ErrorState message={error || analyticsError || correctionsError || lessonsError} onRetry={refetch} />
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/brain/page.tsx around lines 29 - 52, The
page currently treats failed/errored secondary fetches as empty data; update the
useApi result handling so analytics, correctionsResp and lessonsResp are only
considered settled when their respective loading is false and error is null —
i.e., keep their original loading/error flags from useApi and do not coerce
undefined into empty arrays inside the corrections/lessons useMemo; change the
kpis and gradCounts computations (computeKpis and computeGraduationCounts) to
run only when the corresponding data is present and error-free, and surface or
forward the loading/error states to the KPI/graduation widgets rather than
rendering zero-state values.
| const { data: brains, loading: loadingBrains } = useApi<Brain[]>('/brains') | ||
| const primaryId = brains?.[0]?.id ?? null | ||
| const { data: resp, loading } = useApi<PaginatedResponse<Correction> | Correction[]>( | ||
| primaryId ? `/brains/${primaryId}/corrections` : null, | ||
| ) | ||
|
|
||
| const corrections = useMemo<Correction[]>(() => { | ||
| if (!resp) return [] | ||
| return Array.isArray(resp) ? resp : resp.data | ||
| }, [resp]) | ||
|
|
||
| const filtered = filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter) | ||
|
|
||
| if (loadingBrains || loading) return <LoadingSpinner className="py-20" /> | ||
| if (!primaryId) return <EmptyState title="No brain yet" description="Install the SDK to start logging corrections." /> |
There was a problem hiding this comment.
This page also collapses fetch failures into empty/setup states.
A failed /brains request renders “No brain yet”, and a failed /corrections request renders “No corrections”. Please surface the useApi error path here as well, consistent with cloud/dashboard/app/(dashboard)/api-keys/page.tsx Lines 64-65 and cloud/dashboard/app/(dashboard)/settings/page.tsx Lines 18-20.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx around lines 20 - 34,
The code hides API failures by treating missing data as empty state; update the
two useApi calls (the one that returns { data: brains, loading: loadingBrains }
and the one that returns { data: resp, loading }) to also read their error
values (e.g., { error: brainsError } and { error: respError }) and render an
error UI when present instead of the EmptyState/empty corrections: if
brainsError show an ErrorState (or similar) with the brainsError message instead
of "No brain yet", and if respError show an ErrorState with respError text
instead of rendering an empty corrections list, preserving LoadingSpinner while
loading and keeping existing primaryId/corrections logic otherwise.
| const filtered = filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter) | ||
|
|
||
| if (loadingBrains || loading) return <LoadingSpinner className="py-20" /> | ||
| if (!primaryId) return <EmptyState title="No brain yet" description="Install the SDK to start logging corrections." /> | ||
|
|
||
| const severities: Array<'all' | Correction['severity']> = ['all', 'trivial', 'minor', 'moderate', 'major', 'rewrite'] | ||
|
|
||
| return ( | ||
| <> | ||
| <header className="mb-7 flex items-baseline justify-between"> | ||
| <div> | ||
| <h1 className="text-[22px]">Corrections</h1> | ||
| <p className="mt-1 text-[13px] text-[var(--color-body)]"> | ||
| Every correction, newest first · <span className="font-mono">{filtered.length}</span> shown | ||
| </p> |
There was a problem hiding this comment.
“Newest first” is not actually enforced.
The subtitle promises newest-first ordering, but filtered is never sorted by created_at. Right now the list order depends on whatever the backend happens to return.
🕒 Suggested fix
- const filtered = filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter)
+ const filtered = [...(filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter))]
+ .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const filtered = filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter) | |
| if (loadingBrains || loading) return <LoadingSpinner className="py-20" /> | |
| if (!primaryId) return <EmptyState title="No brain yet" description="Install the SDK to start logging corrections." /> | |
| const severities: Array<'all' | Correction['severity']> = ['all', 'trivial', 'minor', 'moderate', 'major', 'rewrite'] | |
| return ( | |
| <> | |
| <header className="mb-7 flex items-baseline justify-between"> | |
| <div> | |
| <h1 className="text-[22px]">Corrections</h1> | |
| <p className="mt-1 text-[13px] text-[var(--color-body)]"> | |
| Every correction, newest first · <span className="font-mono">{filtered.length}</span> shown | |
| </p> | |
| const filtered = [...(filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter))] | |
| .sort((a, b) => new Date(b.created_at).getTime() - new Date(a.created_at).getTime()) | |
| if (loadingBrains || loading) return <LoadingSpinner className="py-20" /> | |
| if (!primaryId) return <EmptyState title="No brain yet" description="Install the SDK to start logging corrections." /> | |
| const severities: Array<'all' | Correction['severity']> = ['all', 'trivial', 'minor', 'moderate', 'major', 'rewrite'] | |
| return ( | |
| <> | |
| <header className="mb-7 flex items-center justify-between"> | |
| <div> | |
| <h1 className="text-[22px]">Corrections</h1> | |
| <p className="mt-1 text-[13px] text-[var(--color-body)]"> | |
| Every correction, newest first · <span className="font-mono">{filtered.length}</span> shown | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx around lines 31 - 45,
The displayed "newest first" promise is wrong because filtered is never sorted;
update the logic that builds filtered (based on corrections and filter) to
produce a new array sorted by created_at descending (most recent first) — e.g.,
take corrections (or corrections.filter(...)), copy it (slice or spread) and
sort by comparing Date.parse(a.created_at) vs Date.parse(b.created_at) or
numeric timestamps so that the variable filtered reflects newest-first order;
ensure you do not mutate the original corrections array and keep the existing
filter condition using filter and the severities list.
| <header className="mb-7 flex items-baseline justify-between"> | ||
| <div> | ||
| <h1 className="text-[22px]">Members</h1> | ||
| <p className="mt-1 text-[13px] text-[var(--color-body)]"> | ||
| Invite teammates and manage roles | ||
| </p> | ||
| </div> | ||
| <Button onClick={() => setInviteOpen(true)}>Invite member</Button> | ||
| </header> | ||
|
|
||
| <PlanGate current={currentPlan} requires="team" featureName="Team member management"> |
There was a problem hiding this comment.
Gate the invite flow too.
The member list is plan-gated, but the header CTA and modal are always rendered. A user without the team entitlement can still open this dialog and complete the invite workflow, which breaks the feature boundary for team management. Wrap the button/dialog in the same gate or disable them when currentPlan is below team.
Also applies to: 94-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 47 - 57,
The header "Invite member" Button and the invite modal (state setter
setInviteOpen and any InviteModal/InviteDialog component) must be gated the same
way as the member list: check currentPlan for the "team" entitlement and either
wrap the Button and the modal render in the existing PlanGate (featureName="Team
member management") or disable the Button and avoid rendering/opening the modal
when currentPlan lacks "team"; specifically update the Button onClick usage
(setInviteOpen(true)) and the modal render (the component that reads inviteOpen)
so they are only reachable when PlanGate permits (or short-circuit with a
disabled Button and no modal mount when currentPlan doesn't include the "team"
feature).
| {m.role !== 'owner' && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="text-[var(--color-destructive)] hover:text-[var(--color-destructive)]" | ||
| > | ||
| Remove | ||
| </Button> |
There was a problem hiding this comment.
Remove or disable the inert Remove action.
This button has no onClick and no disabled/coming-soon treatment, so it advertises a destructive action that currently does nothing. Hide it until the backend exists, or render it disabled with explanatory copy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/team/members/page.tsx around lines 79 - 86,
The Remove button (rendered where m.role !== 'owner' in page.tsx) currently has
no onClick or disabled state and thus wrongly advertises a destructive action;
either remove the JSX entirely until the backend exists or render the Button
disabled and add explanatory copy/tooltip (e.g., "Coming soon" or "Not
available") and set the disabled prop so it’s inert; if you prefer a
future-enabled stub, keep the element but add a noop onClick that opens a
modal/tooltip explaining the feature is unavailable and ensure className and
aria-disabled reflect the disabled state.
| const items = useMemo<DisplayActivity[]>(() => { | ||
| if (real && real.length > 0) { | ||
| return real | ||
| .map((e) => { | ||
| const kind = normalizeKind(e.type) | ||
| if (!kind) return null | ||
| return { | ||
| id: e.id, | ||
| kind, | ||
| title: KIND_TITLE[kind], | ||
| detail: extractDetail(e), | ||
| created_at: e.created_at, | ||
| } | ||
| }) | ||
| .filter((x): x is DisplayActivity => x !== null) | ||
| } |
There was a problem hiding this comment.
Sort activity by created_at before truncating.
The feed is described as chronological, but current rendering depends on API order. Without explicit sorting, slice(0, 8) can drop recent events.
Proposed fix
const items = useMemo<DisplayActivity[]>(() => {
if (real && real.length > 0) {
- return real
+ return real
.map((e) => {
const kind = normalizeKind(e.type)
if (!kind) return null
return {
id: e.id,
kind,
title: KIND_TITLE[kind],
detail: extractDetail(e),
created_at: e.created_at,
}
})
.filter((x): x is DisplayActivity => x !== null)
+ .sort((a, b) => Date.parse(b.created_at) - Date.parse(a.created_at))
}Also applies to: 122-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx` around lines 85 - 100,
The mapped feed items in the useMemo for items (using real, normalizeKind,
KIND_TITLE, extractDetail and created_at) must be sorted by created_at (newest
first) before applying slice(0, 8); modify the pipeline so after mapping and
filtering you sort by numeric timestamp (e.g., compare new
Date(a.created_at).getTime()) descending and then truncate to the first 8
entries — apply the same sorting+truncation change to the other similar feed
calculation later in the file.
| if (real && real.length > 0) { | ||
| return real | ||
| .map((e) => { | ||
| const kind = normalizeKind(e.type) | ||
| if (!kind) return null | ||
| return { | ||
| id: e.id, | ||
| kind, | ||
| title: KIND_TITLE[kind], | ||
| detail: extractDetail(e), | ||
| created_at: e.created_at, | ||
| } | ||
| }) | ||
| .filter((x): x is DisplayActivity => x !== null) | ||
| } | ||
| // Fallback: demo data so the surface isn't empty | ||
| return mockActivity.map((a) => ({ | ||
| id: a.id, | ||
| kind: a.kind, | ||
| title: a.title, | ||
| detail: a.detail, | ||
| created_at: a.created_at, | ||
| })) | ||
| }, [real]) | ||
|
|
||
| const showingDemo = !real || real.length === 0 |
There was a problem hiding this comment.
Handle “all events filtered out” as a fallback case.
If real.length > 0 but all type values are unmapped, the UI renders an empty list and still shows the non-demo label. Treat empty mapped results as fallback.
Proposed fix
- const items = useMemo<DisplayActivity[]>(() => {
+ const { items, showingDemo } = useMemo(() => {
if (real && real.length > 0) {
- return real
+ const mapped = real
.map((e) => {
const kind = normalizeKind(e.type)
if (!kind) return null
return {
id: e.id,
kind,
title: KIND_TITLE[kind],
detail: extractDetail(e),
created_at: e.created_at,
}
})
.filter((x): x is DisplayActivity => x !== null)
+ if (mapped.length > 0) return { items: mapped, showingDemo: false }
}
- // Fallback: demo data so the surface isn't empty
- return mockActivity.map((a) => ({
+ return {
+ items: mockActivity.map((a) => ({
id: a.id,
kind: a.kind,
title: a.title,
detail: a.detail,
created_at: a.created_at,
- }))
+ })),
+ showingDemo: true,
+ }
}, [real])
-
- const showingDemo = !real || real.length === 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (real && real.length > 0) { | |
| return real | |
| .map((e) => { | |
| const kind = normalizeKind(e.type) | |
| if (!kind) return null | |
| return { | |
| id: e.id, | |
| kind, | |
| title: KIND_TITLE[kind], | |
| detail: extractDetail(e), | |
| created_at: e.created_at, | |
| } | |
| }) | |
| .filter((x): x is DisplayActivity => x !== null) | |
| } | |
| // Fallback: demo data so the surface isn't empty | |
| return mockActivity.map((a) => ({ | |
| id: a.id, | |
| kind: a.kind, | |
| title: a.title, | |
| detail: a.detail, | |
| created_at: a.created_at, | |
| })) | |
| }, [real]) | |
| const showingDemo = !real || real.length === 0 | |
| if (real && real.length > 0) { | |
| const mapped = real | |
| .map((e) => { | |
| const kind = normalizeKind(e.type) | |
| if (!kind) return null | |
| return { | |
| id: e.id, | |
| kind, | |
| title: KIND_TITLE[kind], | |
| detail: extractDetail(e), | |
| created_at: e.created_at, | |
| } | |
| }) | |
| .filter((x): x is DisplayActivity => x !== null) | |
| if (mapped.length > 0) return { items: mapped, showingDemo: false } | |
| } | |
| return { | |
| items: mockActivity.map((a) => ({ | |
| id: a.id, | |
| kind: a.kind, | |
| title: a.title, | |
| detail: a.detail, | |
| created_at: a.created_at, | |
| })), | |
| showingDemo: true, | |
| } | |
| }, [real]) | |
| const { items, showingDemo } = useMemo<DisplayActivity[]>(() => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/components/brain/ActivityFeed.tsx` around lines 86 - 111,
The mapping currently returns mockActivity only when real is falsy or empty, but
if real contains entries whose types all map to null via normalizeKind the
mapped result becomes empty while showingDemo remains false; update the logic in
the memo that builds the display list (the block using real.map, normalizeKind,
KIND_TITLE, extractDetail and filtering to DisplayActivity) to capture the
mapped-and-filtered array into a const (e.g., mapped =
real.map(...).filter(...)) and if mapped.length === 0 fall back to returning
mockActivity; also ensure showingDemo uses the same condition (i.e., showingDemo
= !real || mapped.length === 0) so the UI correctly treats “all events filtered
out” as demo.
| <Link | ||
| key={item.href} | ||
| href={item.href} | ||
| className={cn( | ||
| 'flex items-center gap-2.5 rounded-[0.5rem] px-3 py-2 text-[13px] transition-all', | ||
| active | ||
| ? 'border border-[rgba(58,130,255,0.15)] bg-[rgba(58,130,255,0.12)] text-[var(--color-text)]' | ||
| : 'text-[var(--color-body)] hover:bg-[rgba(58,130,255,0.08)] hover:text-[var(--color-text)]', | ||
| )} | ||
| > | ||
| <span className="w-4 text-center text-sm opacity-50">{item.icon}</span> | ||
| {item.label} | ||
| </Link> |
There was a problem hiding this comment.
Expose the active nav state and hide decorative glyphs from assistive tech.
These links rely on visual styling only. Add aria-current for the active item and mark the icon span as decorative so screen readers don’t announce the Unicode symbols.
♿ Suggested fix
<Link
key={item.href}
href={item.href}
+ aria-current={active ? 'page' : undefined}
className={cn(
'flex items-center gap-2.5 rounded-[0.5rem] px-3 py-2 text-[13px] transition-all',
active
? 'border border-[rgba(58,130,255,0.15)] bg-[rgba(58,130,255,0.12)] text-[var(--color-text)]'
: 'text-[var(--color-body)] hover:bg-[rgba(58,130,255,0.08)] hover:text-[var(--color-text)]',
)}
>
- <span className="w-4 text-center text-sm opacity-50">{item.icon}</span>
+ <span aria-hidden="true" className="w-4 text-center text-sm opacity-50">
+ {item.icon}
+ </span>
{item.label}
</Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx` around lines 66 -
78, The nav links currently rely only on visual styling; update the Link element
(the one rendering item.href/item.label in DashboardLayout) to set
aria-current="page" when active (e.g., aria-current={active ? 'page' :
undefined}) so assistive tech can expose the active nav state, and mark the icon
span (the <span> wrapping item.icon) as decorative to screen readers by adding
aria-hidden="true" (and remove it from tab order if needed) so Unicode glyphs
aren’t announced.
SUPABASE-SETUP.md — Google OAuth provider config, custom email templates, RLS policies for meta_rules/rule_patches/events, auth URL config, auto-workspace-creation trigger verification. RAILWAY-ENV.md — complete env var list including new Stripe price ID vars (GRADATA_STRIPE_PRICE_ID_CLOUD / _TEAM) that the Stripe SKU wiring expects. Includes Stripe product + webhook registration steps. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
…t redirect - public/robots.txt: block crawlers from the authed dashboard; marketing content lives at gradata.ai (separate project) - app/page.tsx: show pulsing brand mark while the auth state resolves instead of a white screen on cold load Co-Authored-By: Gradata <noreply@gradata.ai>
…nterprise) Replaces stale `pro`/`team` checkout request with a typed `PlanTier` enum (`free`/`cloud`/`team`/`enterprise`) matching S104 pricing ($29 Cloud / $99 Team). Enterprise is rejected with a contact-sales 400; free is rejected as a non-checkout tier. Stripe price IDs and credentials now live on the typed `Settings` object via `GRADATA_STRIPE_PRICE_ID_CLOUD` / `GRADATA_STRIPE_PRICE_ID_TEAM` / `GRADATA_STRIPE_SECRET_KEY` / `GRADATA_STRIPE_WEBHOOK_SECRET` rather than being read inline at module import. When a price ID for a requested plan is unset, checkout returns 503 "not configured yet". Adds POST /billing/portal which mints a Stripe customer-portal session for the caller's stripe_customer_id, returning 404 "no active subscription" when none is on file. Webhook handler now also processes `customer.subscription.created`, downgrades to `free` on `customer.subscription.deleted`, marks `past_due` on `invoice.payment_failed`, and resolves plan from metadata, price metadata, then matched price IDs (canonical names instead of legacy nicknames). Tests: 14 new in cloud/tests/test_billing.py — invalid plan rejection, 400 for enterprise, 503 when price IDs unset, happy paths for cloud and team using the correct configured price_id, portal 404 paths and success, and three webhook event handlers including signature failure. Stripe SDK is fully mocked via sys.modules; Supabase via the existing MockSupabaseClient. Total backend tests go from 58 to 72 passing. Co-Authored-By: Gradata <noreply@gradata.ai>
- public/_headers: HSTS, CSP (Supabase + api.gradata.ai + Sentry + Google Fonts), X-Frame-Options DENY, Permissions-Policy, immutable cache for /_next/static/* - public/_redirects: legacy /brains/* and /app/* compat - wrangler.toml: pages_build_output_dir=out, name=gradata-dashboard - .env.example: documents required + optional vars (Supabase, API, Sentry) - .nvmrc: pin Node 20 for Cloudflare build env - CLOUDFLARE-PAGES.md: full project settings, custom domain, verify steps Build verified: pnpm build emits out/_headers + out/_redirects. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Gradata has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Covers KPI logic, graduation pipeline, categories folding, plan tier gating, active rules sort/filter, activity feed normalization, and meta-rule layer grouping. useApi mocked for widget tests. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/OPS.md`:
- Line 61: Replace the hardcoded string "All 58 tests should pass" with a
generic, future-proof phrase such as "All tests should pass" or "All tests
should complete" (retain the performance target "<20s" if desired); locate the
exact text "All 58 tests should pass in <20s" in OPS.md and update it to
something like "All tests should pass in <20s (no real Supabase/Stripe needed —
fully mocked)" so the doc doesn’t require updates when the test count changes.
In `@cloud/RAILWAY-ENV.md`:
- Around line 43-49: Add documentation for the two optional Stripe redirect env
vars by inserting a new "Optional — Stripe redirects" table after the "Optional
— App config" section that documents GRADATA_STRIPE_SUCCESS_URL (default
https://app.gradata.ai/billing?success=1) and GRADATA_STRIPE_CANCEL_URL (default
https://app.gradata.ai/billing?cancel=1) and their purposes (redirect after
successful checkout and redirect after cancelled checkout), ensuring the
variable names and defaults match the billing implementation so operators can
override them for staging or custom domains.
In `@cloud/SUPABASE-SETUP.md`:
- Line 110: Update the sentence that references the internal project name
'project_s104_handoff' in the documentation: either remove that internal
identifier entirely or replace it with a neutral description (e.g., "an internal
project" or "our onboarding project") and add a brief note explaining what that
project is/does (it installs a trigger that creates a workspace and brain row on
user signup) so future maintainers understand the context; locate the sentence
containing 'project_s104_handoff' and edit it to use the clarified phrasing or
remove the reference.
- Line 22: Update the "Authorized redirect URIs:
`https://<supabase-project>.supabase.co/auth/v1/callback`" line to clarify that
`<supabase-project>` is the Supabase project ref (Reference ID) and tell the
reader where to find it (Supabase → Settings → General → Reference ID), and give
a brief example phrasing such as "Replace `<supabase-project>` with your project
ref (e.g., your-project-ref.supabase.co)".
- Around line 69-85: The doc's RLS SQL examples are out-of-sync with the
migration: replace the old SELECT-only examples with the real policies used in
migrations by updating the SQL block to match the migration logic and names: use
meta_rules_via_brain, events_via_brain, and rule_patches_via_lesson, change FOR
SELECT to FOR ALL where appropriate, and include the workspace_members join
conditions for team access (brain_id and workspace_id checks) so the
documentation mirrors cloud/migrations/001_initial_schema.sql exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bec587d1-1e79-447d-a42b-e01982c6bf70
📒 Files selected for processing (3)
cloud/OPS.mdcloud/RAILWAY-ENV.mdcloud/SUPABASE-SETUP.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
cloud/RAILWAY-ENV.md (2)
1-91: LGTM: Comprehensive environment variable documentation.The structure is clear and actionable, with good separation between required/optional variables and helpful source attributions. The verification steps and Railway-specific guidance are valuable for operators.
20-21: The documentation inRAILWAY-ENV.mdis correct and matches the actual code implementation. There is no variable name mismatch.The code in
cloud/app/config.pydefines:
stripe_price_id_cloud(becomesGRADATA_STRIPE_PRICE_ID_CLOUDwith theGRADATA_prefix)stripe_price_id_team(becomesGRADATA_STRIPE_PRICE_ID_TEAM)The tests in
cloud/tests/test_billing.pyconfirm these exact variable names. The environment variables documented inRAILWAY-ENV.mdlines 20–21 and the example on lines 69–70 are correct and will function as expected.There is no
cloud/app/routes/billing.pyfile usingGRADATA_STRIPE_PRICE_PROin the codebase. The referenced "billing.py line 21" does not exist.> Likely an incorrect or invalid review comment.cloud/OPS.md (1)
1-109: LGTM: Excellent operations playbook.This document provides clear, actionable guidance for operators across the entire deployment lifecycle. The structure is logical (setup → day-to-day → emergencies), cross-references are appropriate, and the troubleshooting guidance aligns well with the actual error handling in the codebase (e.g., webhook signature verification, Sentry DSN-based initialization).
The mapping of services to URLs and the emergency contact information make this genuinely useful for on-call scenarios.
| cd cloud | ||
| python -m pytest tests/ -q | ||
| ``` | ||
| All 58 tests should pass in <20s (no real Supabase/Stripe needed — fully mocked). |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid hardcoding test count in documentation.
The phrase "All 58 tests should pass" will become outdated as tests are added or removed. Consider using a more generic statement.
♻️ Suggested wording
-All 58 tests should pass in <20s (no real Supabase/Stripe needed — fully mocked).
+All tests should pass in <20s (no real Supabase/Stripe needed — fully mocked).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| All 58 tests should pass in <20s (no real Supabase/Stripe needed — fully mocked). | |
| All tests should pass in <20s (no real Supabase/Stripe needed — fully mocked). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/OPS.md` at line 61, Replace the hardcoded string "All 58 tests should
pass" with a generic, future-proof phrase such as "All tests should pass" or
"All tests should complete" (retain the performance target "<20s" if desired);
locate the exact text "All 58 tests should pass in <20s" in OPS.md and update it
to something like "All tests should pass in <20s (no real Supabase/Stripe needed
— fully mocked)" so the doc doesn’t require updates when the test count changes.
| ## Optional — App config | ||
|
|
||
| | Var | Default | Purpose | | ||
| |-----|---------|---------| | ||
| | `GRADATA_ENVIRONMENT` | `development` | Set to `production` on Railway | | ||
| | `GRADATA_LOG_LEVEL` | `INFO` | Use `DEBUG` only when actively diagnosing | | ||
| | `GRADATA_CORS_ORIGINS` | `http://localhost:3000,https://app.gradata.ai` | Comma-separated allow-list | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Document optional Stripe redirect URL overrides.
The billing code supports two additional optional Stripe environment variables that aren't documented:
GRADATA_STRIPE_SUCCESS_URL(default:https://app.gradata.ai/billing?success=1)GRADATA_STRIPE_CANCEL_URL(default:https://app.gradata.ai/billing?cancel=1)
These control the redirect URLs after Stripe checkout completion/cancellation. While they have sensible defaults, operators may want to customize them (e.g., for staging environments).
📋 Proposed addition to Optional section
Add after the "Optional — App config" section:
## Optional — Stripe redirects
| Var | Default | Purpose |
|-----|---------|---------|
| `GRADATA_STRIPE_SUCCESS_URL` | `https://app.gradata.ai/billing?success=1` | Where to redirect after successful checkout |
| `GRADATA_STRIPE_CANCEL_URL` | `https://app.gradata.ai/billing?cancel=1` | Where to redirect if user cancels checkout |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/RAILWAY-ENV.md` around lines 43 - 49, Add documentation for the two
optional Stripe redirect env vars by inserting a new "Optional — Stripe
redirects" table after the "Optional — App config" section that documents
GRADATA_STRIPE_SUCCESS_URL (default https://app.gradata.ai/billing?success=1)
and GRADATA_STRIPE_CANCEL_URL (default https://app.gradata.ai/billing?cancel=1)
and their purposes (redirect after successful checkout and redirect after
cancelled checkout), ensuring the variable names and defaults match the billing
implementation so operators can override them for staging or custom domains.
| - Type: Web application | ||
| - Name: `Gradata Supabase` | ||
| - Authorized JS origins: `https://app.gradata.ai`, `http://localhost:3000` | ||
| - Authorized redirect URIs: `https://<supabase-project>.supabase.co/auth/v1/callback` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the Supabase project reference placeholder.
The redirect URI contains <supabase-project> as a placeholder. Consider adding guidance on where to find this value (e.g., "Replace <supabase-project> with your project ref from Supabase → Settings → General → Reference ID").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/SUPABASE-SETUP.md` at line 22, Update the "Authorized redirect URIs:
`https://<supabase-project>.supabase.co/auth/v1/callback`" line to clarify that
`<supabase-project>` is the Supabase project ref (Reference ID) and tell the
reader where to find it (Supabase → Settings → General → Reference ID), and give
a brief example phrasing such as "Replace `<supabase-project>` with your project
ref (e.g., your-project-ref.supabase.co)".
| ```sql | ||
| -- meta_rules: user can SELECT their own brain's meta-rules | ||
| CREATE POLICY "meta_rules_read_own" ON meta_rules FOR SELECT | ||
| USING (brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid())); | ||
|
|
||
| -- rule_patches: user can SELECT patches for their own lessons | ||
| CREATE POLICY "rule_patches_read_own" ON rule_patches FOR SELECT | ||
| USING (lesson_id IN ( | ||
| SELECT l.id FROM lessons l | ||
| JOIN brains b ON b.id = l.brain_id | ||
| WHERE b.user_id = auth.uid() | ||
| )); | ||
|
|
||
| -- events: user can SELECT events for their own brain | ||
| CREATE POLICY "events_read_own" ON events FOR SELECT | ||
| USING (brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid())); | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Extract RLS policy definitions from the migration file
rg -A 10 "CREATE POLICY.*meta_rules" cloud/migrations/001_initial_schema.sql
rg -A 10 "CREATE POLICY.*events" cloud/migrations/001_initial_schema.sql
rg -A 10 "CREATE POLICY.*rule_patches" cloud/migrations/001_initial_schema.sqlRepository: Gradata/gradata
Length of output: 1165
Critical: RLS policy examples don't match the actual migration SQL.
The SQL examples in this section differ significantly from the actual policies in cloud/migrations/001_initial_schema.sql:
Policy names mismatch:
- Documentation:
"meta_rules_read_own","events_read_own","rule_patches_read_own" - Migration:
meta_rules_via_brain,events_via_brain,rule_patches_via_lesson
Permission scope mismatch:
- Documentation:
FOR SELECT(read-only) - Migration:
FOR ALL(read + write via brain/workspace ownership)
The migration policies also include workspace membership logic for team access that's completely missing from the documentation examples.
📝 Proposed fix: Update examples to match migration SQL
-```sql
--- meta_rules: user can SELECT their own brain's meta-rules
-CREATE POLICY "meta_rules_read_own" ON meta_rules FOR SELECT
- USING (brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid()));
-
--- rule_patches: user can SELECT patches for their own lessons
-CREATE POLICY "rule_patches_read_own" ON rule_patches FOR SELECT
- USING (lesson_id IN (
- SELECT l.id FROM lessons l
- JOIN brains b ON b.id = l.brain_id
- WHERE b.user_id = auth.uid()
- ));
-
--- events: user can SELECT events for their own brain
-CREATE POLICY "events_read_own" ON events FOR SELECT
- USING (brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid()));
-```
+```sql
+-- meta_rules: user can read/write their own brain's meta-rules (and team members' via workspace)
+CREATE POLICY meta_rules_via_brain ON meta_rules
+ FOR ALL USING (
+ brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid())
+ OR brain_id IN (
+ SELECT b.id FROM brains b
+ JOIN workspace_members wm ON wm.workspace_id = b.workspace_id
+ WHERE wm.user_id = auth.uid()
+ )
+ );
+
+-- events: user can read/write events for their own brain (and team members' via workspace)
+CREATE POLICY events_via_brain ON events
+ FOR ALL USING (
+ brain_id IN (SELECT id FROM brains WHERE user_id = auth.uid())
+ OR brain_id IN (
+ SELECT b.id FROM brains b
+ JOIN workspace_members wm ON wm.workspace_id = b.workspace_id
+ WHERE wm.user_id = auth.uid()
+ )
+ );
+
+-- rule_patches: user can read/write patches for their own lessons
+CREATE POLICY rule_patches_via_lesson ON rule_patches
+ FOR ALL USING (
+ lesson_id IN (
+ SELECT l.id FROM lessons l
+ JOIN brains b ON b.id = l.brain_id
+ WHERE b.user_id = auth.uid()
+ )
+ );
+```
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @cloud/SUPABASE-SETUP.md around lines 69 - 85, The doc's RLS SQL examples are
out-of-sync with the migration: replace the old SELECT-only examples with the
real policies used in migrations by updating the SQL block to match the
migration logic and names: use meta_rules_via_brain, events_via_brain, and
rule_patches_via_lesson, change FOR SELECT to FOR ALL where appropriate, and
include the workspace_members join conditions for team access (brain_id and
workspace_id checks) so the documentation mirrors
cloud/migrations/001_initial_schema.sql exactly.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| ## 6. Auto-create workspace on signup | ||
|
|
||
| The existing trigger (per `project_s104_handoff`) creates a workspace + brain row on user signup. Verify it's installed: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove or clarify the internal project reference.
The phrase per 'project_s104_handoff' appears to be an internal project reference. Consider either removing it or adding context about what it refers to for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/SUPABASE-SETUP.md` at line 110, Update the sentence that references the
internal project name 'project_s104_handoff' in the documentation: either remove
that internal identifier entirely or replace it with a neutral description
(e.g., "an internal project" or "our onboarding project") and add a brief note
explaining what that project is/does (it installs a trigger that creates a
workspace and brain row on user signup) so future maintainers understand the
context; locate the sentence containing 'project_s104_handoff' and edit it to
use the clarified phrasing or remove the reference.
Sidebar collapses to hamburger drawer below md (768px). Wide row layouts (team leaderboard, members, operator customers/alerts) now wrap on mobile instead of cramming fixed-width columns. KPI value text scales down on small screens to prevent wrapping at <400px. Methodology modal close button is now 44x44 for tap-friendliness. Page headers wrap their title + action pair so filter pills don't get squeezed off-screen. Pure additive className changes — no component restructuring, no behavior changes, no backend touches. Build verified (21 routes), dev server returns 200 on all touched pages. Co-Authored-By: Gradata <noreply@gradata.ai>
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/app/config.py`:
- Around line 28-37: The config exposes stripe_secret_key but the code calls
Stripe APIs without verifying it; update the _stripe() helper to check that
config.stripe_secret_key is non-empty and raise or return an HTTP
503/appropriate error when missing, and ensure callers (create_checkout and
portal routes) use this validation path instead of calling stripe.* directly so
they fail fast with a clear 503 message; reference the _stripe() function,
create_checkout route, and portal route when making the change.
In `@cloud/app/routes/billing.py`:
- Around line 107-113: The 404 error message is misleading when
db.select("workspaces", filters={"owner_id": user_id}) returns no rows; update
the handling in the route so it distinguishes between "no workspace/billing
account" and "no active subscription": if ws_rows is empty, raise HTTPException
with a message like "no billing account found" (or similar) referencing the
ws_rows result from db.select, otherwise keep the existing check of customer_id
= ws_rows[0].get("stripe_customer_id") and raise "no active subscription" when
customer_id is missing; adjust the HTTPException messages in this function
accordingly.
- Around line 98-102: The create_portal_session endpoint has an unused request:
Request parameter—remove the Request parameter from the function signature in
create_portal_session and any corresponding type imports so the function becomes
async def create_portal_session(user_id: str = Depends(get_current_user_id)) ->
PortalResponse; update any internal references (none expected) and remove the
unused Request import from the module to keep imports accurate.
- Around line 176-185: Replace the hardcoded "free" plan string with the enum
value to keep config consistent: in the stripe event handler for etype
"customer.subscription.deleted" (the block that calls db.update on
"workspaces"), change the data payload to use PlanTier.free.value instead of the
literal "free" and import or reference PlanTier as needed so PlanTier.free.value
is available where db.update is called.
In `@cloud/dashboard/.env.example`:
- Around line 12-13: Update the example env value for NEXT_PUBLIC_API_URL so it
matches the runtime fallback used by the API client in
cloud/dashboard/src/lib/api.ts (which appends /api/v1); change the example from
https://api.gradata.ai to include the same base path (e.g.,
https://api.gradata.ai/api/v1) so local/production examples and the
NEXT_PUBLIC_API_URL value used by the application are consistent with the
fallback path referenced in the Api client.
In `@cloud/dashboard/CLOUDFLARE-PAGES.md`:
- Around line 89-90: The documentation currently asserts a console message
"[sentry] initialized env=production release=..." which the client does not
emit; update CLOUDFLARE-PAGES.md to remove that incorrect "initialized"
expectation and either keep only the verifiable disabled-case message "[sentry]
disabled: ..." or replace the check with a verifiable alternative (for example,
instructions to verify event delivery in non-production or to look for a real
emitted log/event), referencing the exact strings "[sentry] initialized
env=production release=..." and "[sentry] disabled: ..." so reviewers can find
and update the sentence accordingly.
- Line 73: The documentation currently claims `'unsafe-eval'` is required for
Next.js hydration; update the CLOUDFLARE-PAGES.md text that mentions
`'unsafe-eval'` so it no longer asserts this for production—either remove the
`'unsafe-eval'` directive entirely from the production CSP example or explicitly
state that `'unsafe-eval'` is only a possible development-time addition and that
production Next.js hydration typically requires only `'unsafe-inline'` for
inline scripts; adjust the sentence referencing Next.js hydration to cite that
`'unsafe-eval'` is not required in production and should be applied
conditionally for development builds.
In `@cloud/dashboard/public/_headers`:
- Line 10: The Content-Security-Policy header's connect-src currently includes a
staging origin (https://gradata-production.up.railway.app) which should be
removed; edit the Content-Security-Policy line to delete that origin from
connect-src so only canonical production origins (e.g., https://api.gradata.ai
and other approved domains like https://*.supabase.co, wss://*.supabase.co,
https://*.ingest.sentry.io) remain, ensuring the CSP in the file's
Content-Security-Policy header no longer permits the staging backend.
- Line 10: Update the Content-Security-Policy entry in the _headers file by
removing 'unsafe-eval' from the script-src directive (the CSP line starting with
"Content-Security-Policy: default-src 'self'; script-src ..."); keep
'unsafe-inline' if required for styles/inline scripts but ensure the script-src
no longer contains 'unsafe-eval' for production builds (or conditionally include
it only in your development config).
In `@cloud/dashboard/public/robots.txt`:
- Around line 3-8: The robots.txt has two separate "User-agent: *" blocks which
violates the spec; merge them into a single "User-agent: *" group and place both
directives (Disallow: / and Allow: /favicon.ico) under that single header so
crawlers apply both rules correctly; update the file by removing the duplicate
"User-agent: *" line and moving the Allow directive into the same group that
contains the Disallow directive.
In `@cloud/tests/test_billing.py`:
- Around line 33-54: Add a test that verifies behavior when
GRADATA_STRIPE_SECRET_KEY is missing while price IDs are configured: create a
new fixture (e.g., stripe_missing_secret) that calls get_settings.cache_clear(),
sets GRADATA_STRIPE_PRICE_ID_CLOUD and GRADATA_STRIPE_PRICE_ID_TEAM to test
values, unsets GRADATA_STRIPE_SECRET_KEY (monkeypatch.delenv with
raising=False), sets GRADATA_STRIPE_WEBHOOK_SECRET, yields, then clears cache;
add a test function (e.g., test_checkout_missing_stripe_secret) that uses this
fixture and invokes the same checkout endpoint exercised by existing tests and
asserts the response is a 503 (and optionally contains the expected
service-unavailable/error message) instead of raising a Stripe auth error.
- Around line 245-257: Remove the redundant "if True:" blocks wrapping test
event setup and posting; locate the three occurrences inside the test methods
that build the "event" dict and call self._post_event (the shown block using
event = {...} and resp = self._post_event(client, fake_stripe, event)), as well
as the similar blocks in test_subscription_deleted_downgrades_to_free and
test_invoice_payment_failed_marks_past_due, and simply delete the "if True:"
line and unindent the inner lines so the event construction and resp =
self._post_event(...) run directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94ffd29b-de78-47ed-b3aa-87fcfbb6057e
📒 Files selected for processing (12)
cloud/app/config.pycloud/app/models.pycloud/app/routes/billing.pycloud/dashboard/.env.examplecloud/dashboard/.nvmrccloud/dashboard/CLOUDFLARE-PAGES.mdcloud/dashboard/app/page.tsxcloud/dashboard/public/_headerscloud/dashboard/public/_redirectscloud/dashboard/public/robots.txtcloud/dashboard/wrangler.tomlcloud/tests/test_billing.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (15)
cloud/dashboard/app/page.tsx (3)
1-6: Client-page setup is correctly wired.
'use client'and hook imports are appropriate for this auth-driven redirect page.
11-14: Redirect flow is correctly sequenced after auth hydration.The
loadingguard beforerouter.replace(...)is correct and prevents premature navigation.
16-23: Loading placeholder implementation looks good.This avoids a blank initial paint and matches the intended branded auth-resolve state.
cloud/dashboard/.env.example (1)
1-11: Clean and well-structured env template.Required vs optional separation is clear, and the Sentry runtime/build-time split matches actual usage in
cloud/dashboard/sentry.client.config.tsandcloud/dashboard/next.config.ts.Also applies to: 15-32
cloud/dashboard/CLOUDFLARE-PAGES.md (1)
91-91: Keep production verification non-invasive.The instruction at Line 91 to trigger a React error on live production is the same concern raised earlier.
cloud/dashboard/.nvmrc (1)
1-1: Node version pin looks good.
.nvmrcaligns with the documented Cloudflare Pages Node 20 requirement.cloud/dashboard/wrangler.toml (1)
1-3: Wrangler Pages config is coherent.The project name, compatibility date, and output directory are consistent with the static-export deployment flow.
cloud/dashboard/public/_redirects (1)
6-9: Legacy redirect rules look correct.The mappings are clear and align with the current
/brain?id=...route model and/app/*prefix cleanup.cloud/app/models.py (2)
186-197: LGTM!The
PlanTierenum is well-documented with clear tier semantics. The docstring accurately describes the business logic (free = no Stripe sub, cloud/team = paid, enterprise = sales-only), which aligns with the route-layer validation inbilling.py.
200-211: LGTM!Typing
CheckoutRequest.planasPlanTierenforces enum validation at the Pydantic layer, andPortalResponsecorrectly models the portal session URL response.cloud/app/config.py (1)
23-26: LGTM!Sentry settings default safely — empty DSN disables initialization gracefully (confirmed in
sentry_init.py), and the trace sample rate default of 0.1 is reasonable for production.cloud/tests/test_billing.py (2)
57-80: LGTM!The
fake_stripefixture is well-designed — it properly stubs the lazy-imported module structure, mockscheckout.Session.create,billing_portal.Session.create,Webhook.construct_event, and provides a customSignatureVerificationErrorexception class for testing the error path.
93-141: LGTM!Comprehensive validation tests covering invalid enum values (422), enterprise/free rejection (400), and missing price ID configuration (503). The assertions correctly verify both status codes and error message content.
cloud/app/routes/billing.py (2)
27-34: LGTM on checkout validation flow.The checkout endpoint properly validates
enterprise(400, sales-only),free(400, no checkout needed), and missing price IDs (503). The_price_id_forhelper cleanly mapsPlanTierto configured Stripe price IDs.Also applies to: 54-95
198-227: LGTM on plan extraction logic.The
_extract_planfunction has a robust 4-level fallback: subscription metadata → price metadata → configured price ID matching → legacy nickname. This handles various Stripe configurations gracefully.
| # Stripe (all optional — empty values mean "not configured") | ||
| # Env vars: GRADATA_STRIPE_SECRET_KEY, GRADATA_STRIPE_WEBHOOK_SECRET, | ||
| # GRADATA_STRIPE_PRICE_ID_CLOUD, GRADATA_STRIPE_PRICE_ID_TEAM | ||
| stripe_secret_key: str = "" | ||
| stripe_webhook_secret: str = "" | ||
| stripe_price_id_cloud: str = "" | ||
| stripe_price_id_team: str = "" | ||
| stripe_success_url: str = "https://app.gradata.ai/billing?success=1" | ||
| stripe_cancel_url: str = "https://app.gradata.ai/billing?cancel=1" | ||
| stripe_portal_return_url: str = "https://app.gradata.ai/billing" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if stripe_secret_key is validated before Stripe API calls
rg -n "stripe_secret_key" cloud/app/routes/billing.pyRepository: Gradata/gradata
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Get context around line 39 where stripe_secret_key is accessed
sed -n '30,60p' cloud/app/routes/billing.pyRepository: Gradata/gradata
Length of output: 1052
🏁 Script executed:
#!/bin/bash
# Check the webhook validation pattern mentioned (lines 132-133)
sed -n '125,145p' cloud/app/routes/billing.pyRepository: Gradata/gradata
Length of output: 876
🏁 Script executed:
#!/bin/bash
# Find the _stripe() function definition and understand its structure
rg -n "def _stripe" cloud/app/routes/billing.py -A 15Repository: Gradata/gradata
Length of output: 622
🏁 Script executed:
#!/bin/bash
# Look for stripe API calls in the checkout/portal routes
rg -n "stripe\." cloud/app/routes/billing.py | head -20Repository: Gradata/gradata
Length of output: 308
🏁 Script executed:
#!/bin/bash
# Get the full create_checkout function to see if there's price_id validation
sed -n '54,95p' cloud/app/routes/billing.pyRepository: Gradata/gradata
Length of output: 1494
🏁 Script executed:
#!/bin/bash
# Get the full portal function to see if there's any validation
sed -n '100,125p' cloud/app/routes/billing.pyRepository: Gradata/gradata
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Check if price_id is validated before the Stripe API call
rg -n "stripe_price_id" cloud/app/routes/billing.py -B 2 -A 2Repository: Gradata/gradata
Length of output: 545
Missing validation for stripe_secret_key before Stripe API calls.
When stripe_secret_key is empty (the default) but a client attempts checkout or portal actions, the calls to stripe.checkout.Session.create() and stripe.billing_portal.Session.create() will proceed with an empty API key, resulting in cryptic authentication errors from Stripe rather than a clean 503 response.
The webhook endpoint properly validates stripe_webhook_secret before calling Stripe (lines 132-135), but similar validation is missing for stripe_secret_key in the _stripe() function. The create_checkout route validates the price_id and returns 503 if missing, but bypasses stripe_secret_key validation. The portal route has no such safety checks.
Add validation in _stripe() to fail fast:
Proposed fix in `cloud/app/routes/billing.py`
def _stripe():
"""Lazy-import stripe so the module loads even when stripe is not installed."""
+ if not get_settings().stripe_secret_key:
+ raise HTTPException(status_code=503, detail="Stripe not configured")
try:
import stripe as _s
_configure_stripe(_s)
return _s
except ImportError as exc: # pragma: no cover
raise HTTPException(status_code=503, detail="Stripe not configured") from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/config.py` around lines 28 - 37, The config exposes
stripe_secret_key but the code calls Stripe APIs without verifying it; update
the _stripe() helper to check that config.stripe_secret_key is non-empty and
raise or return an HTTP 503/appropriate error when missing, and ensure callers
(create_checkout and portal routes) use this validation path instead of calling
stripe.* directly so they fail fast with a clear 503 message; reference the
_stripe() function, create_checkout route, and portal route when making the
change.
| @router.post("/billing/portal", response_model=PortalResponse) | ||
| async def create_portal_session( | ||
| request: Request, | ||
| user_id: str = Depends(get_current_user_id), | ||
| ) -> PortalResponse: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused request: Request parameter.
The request parameter is not used in create_portal_session. Consider removing it to clean up the function signature.
♻️ Proposed fix
`@router.post`("/billing/portal", response_model=PortalResponse)
async def create_portal_session(
- request: Request,
user_id: str = Depends(get_current_user_id),
) -> PortalResponse:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/billing.py` around lines 98 - 102, The create_portal_session
endpoint has an unused request: Request parameter—remove the Request parameter
from the function signature in create_portal_session and any corresponding type
imports so the function becomes async def create_portal_session(user_id: str =
Depends(get_current_user_id)) -> PortalResponse; update any internal references
(none expected) and remove the unused Request import from the module to keep
imports accurate.
| ws_rows = await db.select("workspaces", filters={"owner_id": user_id}) | ||
| if not ws_rows: | ||
| raise HTTPException(status_code=404, detail="no active subscription") | ||
|
|
||
| customer_id = ws_rows[0].get("stripe_customer_id") | ||
| if not customer_id: | ||
| raise HTTPException(status_code=404, detail="no active subscription") |
There was a problem hiding this comment.
Error message is misleading when user has no workspace.
The 404 response at line 109 says "no active subscription" when the user might simply have no workspace record at all. Consider differentiating these cases or using a more accurate message like "no billing account found."
✏️ Proposed fix
ws_rows = await db.select("workspaces", filters={"owner_id": user_id})
if not ws_rows:
- raise HTTPException(status_code=404, detail="no active subscription")
+ raise HTTPException(status_code=404, detail="No billing account found")
customer_id = ws_rows[0].get("stripe_customer_id")
if not customer_id:
- raise HTTPException(status_code=404, detail="no active subscription")
+ raise HTTPException(status_code=404, detail="No active subscription")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ws_rows = await db.select("workspaces", filters={"owner_id": user_id}) | |
| if not ws_rows: | |
| raise HTTPException(status_code=404, detail="no active subscription") | |
| customer_id = ws_rows[0].get("stripe_customer_id") | |
| if not customer_id: | |
| raise HTTPException(status_code=404, detail="no active subscription") | |
| ws_rows = await db.select("workspaces", filters={"owner_id": user_id}) | |
| if not ws_rows: | |
| raise HTTPException(status_code=404, detail="No billing account found") | |
| customer_id = ws_rows[0].get("stripe_customer_id") | |
| if not customer_id: | |
| raise HTTPException(status_code=404, detail="No active subscription") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/billing.py` around lines 107 - 113, The 404 error message is
misleading when db.select("workspaces", filters={"owner_id": user_id}) returns
no rows; update the handling in the route so it distinguishes between "no
workspace/billing account" and "no active subscription": if ws_rows is empty,
raise HTTPException with a message like "no billing account found" (or similar)
referencing the ws_rows result from db.select, otherwise keep the existing check
of customer_id = ws_rows[0].get("stripe_customer_id") and raise "no active
subscription" when customer_id is missing; adjust the HTTPException messages in
this function accordingly.
| elif etype == "customer.subscription.deleted": | ||
| customer_id = data.get("customer") | ||
| status = data.get("status", "canceled") | ||
| if customer_id: | ||
| await db.update( | ||
| "workspaces", | ||
| data={"plan": "free", "subscription_status": status}, | ||
| filters={"stripe_customer_id": customer_id}, | ||
| ) | ||
| _log.info("Subscription deleted customer=%s -> free", customer_id) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use PlanTier.free.value instead of hardcoded "free" string.
For consistency with the rest of the codebase that uses the PlanTier enum, consider using PlanTier.free.value here. This ensures the value stays in sync if the enum value ever changes.
♻️ Proposed fix
elif etype == "customer.subscription.deleted":
customer_id = data.get("customer")
status = data.get("status", "canceled")
if customer_id:
await db.update(
"workspaces",
- data={"plan": "free", "subscription_status": status},
+ data={"plan": PlanTier.free.value, "subscription_status": status},
filters={"stripe_customer_id": customer_id},
)
_log.info("Subscription deleted customer=%s -> free", customer_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/billing.py` around lines 176 - 185, Replace the hardcoded
"free" plan string with the enum value to keep config consistent: in the stripe
event handler for etype "customer.subscription.deleted" (the block that calls
db.update on "workspaces"), change the data payload to use PlanTier.free.value
instead of the literal "free" and import or reference PlanTier as needed so
PlanTier.free.value is available where db.update is called.
| # Backend API base URL (Railway: https://gradata-production.up.railway.app, prod: https://api.gradata.ai) | ||
| NEXT_PUBLIC_API_URL=https://api.gradata.ai |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Align API base URL example with runtime fallback path.
Line 13 sets NEXT_PUBLIC_API_URL=https://api.gradata.ai, but cloud/dashboard/src/lib/api.ts falls back to a base URL that includes /api/v1. This can create environment-specific path drift.
Suggested fix
-# Backend API base URL (Railway: https://gradata-production.up.railway.app, prod: https://api.gradata.ai)
-NEXT_PUBLIC_API_URL=https://api.gradata.ai
+# Backend API base URL (Railway: https://gradata-production.up.railway.app/api/v1, prod: https://api.gradata.ai/api/v1)
+NEXT_PUBLIC_API_URL=https://api.gradata.ai/api/v1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Backend API base URL (Railway: https://gradata-production.up.railway.app, prod: https://api.gradata.ai) | |
| NEXT_PUBLIC_API_URL=https://api.gradata.ai | |
| # Backend API base URL (Railway: https://gradata-production.up.railway.app/api/v1, prod: https://api.gradata.ai/api/v1) | |
| NEXT_PUBLIC_API_URL=https://api.gradata.ai/api/v1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/.env.example` around lines 12 - 13, Update the example env
value for NEXT_PUBLIC_API_URL so it matches the runtime fallback used by the API
client in cloud/dashboard/src/lib/api.ts (which appends /api/v1); change the
example from https://api.gradata.ai to include the same base path (e.g.,
https://api.gradata.ai/api/v1) so local/production examples and the
NEXT_PUBLIC_API_URL value used by the application are consistent with the
fallback path referenced in the Api client.
| 4. Browser console shows `[sentry] initialized env=production release=...` | ||
| (or `[sentry] disabled: ...` if DSN not set — prod-safe) |
There was a problem hiding this comment.
Sentry verification text does not match runtime behavior.
At Line 89, the doc expects an initialization console message that the client config does not emit. Keep only the disabled-case log expectation or replace with a verifiable signal (e.g., test event delivery in non-prod).
Suggested edit
-4. Browser console shows `[sentry] initialized env=production release=...`
- (or `[sentry] disabled: ...` if DSN not set — prod-safe)
+4. If `NEXT_PUBLIC_SENTRY_DSN` is unset, browser console shows
+ `[sentry] disabled: NEXT_PUBLIC_SENTRY_DSN not set`.
+ If DSN is set, verify via Sentry event ingestion instead of console text.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 4. Browser console shows `[sentry] initialized env=production release=...` | |
| (or `[sentry] disabled: ...` if DSN not set — prod-safe) | |
| 4. If `NEXT_PUBLIC_SENTRY_DSN` is unset, browser console shows | |
| `[sentry] disabled: NEXT_PUBLIC_SENTRY_DSN not set`. | |
| If DSN is set, verify via Sentry event ingestion instead of console text. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/CLOUDFLARE-PAGES.md` around lines 89 - 90, The documentation
currently asserts a console message "[sentry] initialized env=production
release=..." which the client does not emit; update CLOUDFLARE-PAGES.md to
remove that incorrect "initialized" expectation and either keep only the
verifiable disabled-case message "[sentry] disabled: ..." or replace the check
with a verifiable alternative (for example, instructions to verify event
delivery in non-production or to look for a real emitted log/event), referencing
the exact strings "[sentry] initialized env=production release=..." and
"[sentry] disabled: ..." so reviewers can find and update the sentence
accordingly.
| X-Frame-Options: DENY | ||
| Referrer-Policy: strict-origin-when-cross-origin | ||
| Permissions-Policy: interest-cohort=(), geolocation=(), camera=(), microphone=(), payment=(), usb=(), accelerometer=(), gyroscope=(), magnetometer=() | ||
| Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com data:; img-src 'self' data: blob: https:; connect-src 'self' https://*.supabase.co wss://*.supabase.co https://api.gradata.ai https://gradata-production.up.railway.app https://*.ingest.sentry.io https://ingest.sentry.io; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests |
There was a problem hiding this comment.
Remove staging backend origin from production connect-src.
At Line 10, permitting https://gradata-production.up.railway.app in prod unnecessarily expands data exfiltration surface. Prefer only canonical production API origin(s).
Suggested edit
- Content-Security-Policy: ... connect-src 'self' https://*.supabase.co wss://*.supabase.co https://api.gradata.ai https://gradata-production.up.railway.app https://*.ingest.sentry.io https://ingest.sentry.io; ...
+ Content-Security-Policy: ... connect-src 'self' https://*.supabase.co wss://*.supabase.co https://api.gradata.ai https://*.ingest.sentry.io https://ingest.sentry.io; ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com data:; img-src 'self' data: blob: https:; connect-src 'self' https://*.supabase.co wss://*.supabase.co https://api.gradata.ai https://gradata-production.up.railway.app https://*.ingest.sentry.io https://ingest.sentry.io; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests | |
| Content-Security-Policy: default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com data:; img-src 'self' data: blob: https:; connect-src 'self' https://*.supabase.co wss://*.supabase.co https://api.gradata.ai https://*.ingest.sentry.io https://ingest.sentry.io; frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'; upgrade-insecure-requests |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/public/_headers` at line 10, The Content-Security-Policy
header's connect-src currently includes a staging origin
(https://gradata-production.up.railway.app) which should be removed; edit the
Content-Security-Policy line to delete that origin from connect-src so only
canonical production origins (e.g., https://api.gradata.ai and other approved
domains like https://*.supabase.co, wss://*.supabase.co,
https://*.ingest.sentry.io) remain, ensuring the CSP in the file's
Content-Security-Policy header no longer permits the staging backend.
🧩 Analysis chain
🌐 Web query:
For Next.js 16 App Router static export production builds, is script-src 'unsafe-eval' required at runtime, or can CSP omit it safely?
💡 Result:
No, script-src 'unsafe-eval' is not required at runtime for Next.js 16 App Router static export production builds. CSP can safely omit it. In production static exports (using output: 'export' in next.config.js), the build generates static HTML, CSS, and JS bundles in the out directory, served without a Node.js server. The client-side JS bundles (e.g., webpack chunks) execute in browsers and do not require eval for core functionality like hydration or routing. Inline scripts such as self.next_f.push(...) are present for router state initialization and client-side navigation but are plain JavaScript without eval. unsafe-eval is only needed in development for features like React Fast Refresh (source maps) or certain debugging tools, as noted in CSP examples distinguishing dev (${isDev ? "'unsafe-eval'" : ''}) from production. Strict CSP using 'self', nonces, hashes, or 'strict-dynamic' works for production static builds, though inline scripts require 'unsafe-inline', nonces, or hashes—not 'unsafe-eval'. Sources confirm static exports support App Router with prerendered Server Components to HTML/JS payloads, Client Components hydrated client-side without server dependencies, and no mentions of production eval needs.
Citations:
- 1: Next Js Content-Security-Policy , script-src requires 'unsafe-inline' in production vercel/next.js#81703
- 2: Inline scripts generated as a result of
appDirpreventing use of strict CSP vercel/next.js#43743 - 3: https://www.reddit.com/r/nextjs/comments/18kzmkk/scriptsrc_self_in_a_static_nextjs_app_error/
- 4: NextJS13 dynamic components injects <script> tag in the DOM causing weird markup and CSP errors even when using next export vercel/next.js#45099
- 5: https://www.reflowhub.dev/en/docs/nextjs/app-guides/app-guides-content-security-policy
- 6: https://nextjs.org/docs-wip/app/building-your-application/deploying/static-exports
- 7: NextJS Static Site Bundling A Bunch Of
self.__next_f.pushInstead Of Raw HTML vercel/next.js#56180 - 8: appDir: Inline Scripts (self.__next_f...) vercel/next.js#43710
- 9: Content-Security-Policy unsafe-eval in Next.js 9.4+ dev env vercel/next.js#14221
Remove unsafe-eval from production CSP—it is not required.
At Line 10, 'unsafe-eval' significantly weakens XSS protections without necessity. Next.js 16 static exports generate pre-built JS bundles that execute without eval; inline scripts use plain JavaScript without eval (e.g., self.next_f.push(...)). unsafe-eval is only needed in development for React Fast Refresh debugging. Remove it from the production CSP header.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/public/_headers` at line 10, Update the
Content-Security-Policy entry in the _headers file by removing 'unsafe-eval'
from the script-src directive (the CSP line starting with
"Content-Security-Policy: default-src 'self'; script-src ..."); keep
'unsafe-inline' if required for styles/inline scripts but ensure the script-src
no longer contains 'unsafe-eval' for production builds (or conditionally include
it only in your development config).
| User-agent: * | ||
| Disallow: / | ||
|
|
||
| # Allow health checks | ||
| User-agent: * | ||
| Allow: /favicon.ico |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
In robots.txt, when there are multiple "User-agent: *" groups, do major crawlers merge them or treat them separately? Is a single combined group recommended for compatibility?
💡 Result:
Major crawlers treat multiple "User-agent: " groups separately rather than merging them. They select only one matching group based on specificity and position according to their parsing rules, typically the first or most specific "' group for non-specific agents, ignoring subsequent "" groups. Google merges multiple groups only if they match the same specific user-agent (e.g., multiple "Googlebot" groups), but explicitly does not combine specific user-agent groups with "" groups. For "", multiple groups are treated as separate, with only one applied per the order of precedence. Bingbot honors only one set of directives: the specific "bingbot" group (if present), or "msnbot", or the first/default "" group, discarding others. The original spec and Yandex indicate only one "*" record is allowed; multiples are invalid, and crawlers pick the first matching one. A single combined "User-agent: *" group is recommended for compatibility across all major crawlers to avoid unpredictable behavior from multiple groups.
Citations:
- 1: https://developers.google.com/crawling/docs/robots-txt/robots-txt-spec
- 2: https://stackoverflow.com/questions/14710324/multiple-user-agents-in-robots-txt
- 3: https://stackoverflow.com/questions/18885682/multiple-user-agents-in-robots-txt
- 4: https://www.conductor.com/academy/robotstxt/
- 5: https://www.bing.com/robots.txt
- 6: https://www.bing.com/webmasters/help/how-to-create-a-robots-txt-file-cb7c31ec?toWww=1&redig=DD7C371F03AB43F1BAC8832DE028EA13
- 7: https://blogs.bing.com/webmaster/2012/05/03/to-crawl-or-not-to-crawl-that-is-bingbots-question/
- 8: https://yandex.com/support/webmaster/en/controlling-robot/robots-txt.html
- 9: https://yandex.com/support/webmaster/robot-workings/user-agent.html
Consolidate duplicate wildcard groups for spec compliance.
The robots.txt specification allows only one User-agent: * group. Using two separate blocks (lines 3 and 7) violates the spec and causes crawlers to apply only the first matching group, ignoring subsequent directives. Merge both Disallow and Allow directives under a single User-agent: * block.
Suggested edit
-User-agent: *
-Disallow: /
-
-# Allow health checks
-User-agent: *
-Allow: /favicon.ico
+User-agent: *
+Disallow: /
+Allow: /favicon.ico📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| User-agent: * | |
| Disallow: / | |
| # Allow health checks | |
| User-agent: * | |
| Allow: /favicon.ico | |
| User-agent: * | |
| Disallow: / | |
| Allow: /favicon.ico |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/public/robots.txt` around lines 3 - 8, The robots.txt has two
separate "User-agent: *" blocks which violates the spec; merge them into a
single "User-agent: *" group and place both directives (Disallow: / and Allow:
/favicon.ico) under that single header so crawlers apply both rules correctly;
update the file by removing the duplicate "User-agent: *" line and moving the
Allow directive into the same group that contains the Disallow directive.
| @pytest.fixture | ||
| def configured_prices(monkeypatch): | ||
| """Set Stripe price IDs in settings (use cache_clear so values stick).""" | ||
| get_settings.cache_clear() | ||
| monkeypatch.setenv("GRADATA_STRIPE_PRICE_ID_CLOUD", "price_cloud_test") | ||
| monkeypatch.setenv("GRADATA_STRIPE_PRICE_ID_TEAM", "price_team_test") | ||
| monkeypatch.setenv("GRADATA_STRIPE_SECRET_KEY", "sk_test_dummy") | ||
| monkeypatch.setenv("GRADATA_STRIPE_WEBHOOK_SECRET", "whsec_test_dummy") | ||
| yield | ||
| get_settings.cache_clear() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def unconfigured_prices(monkeypatch): | ||
| """Explicitly clear price IDs so checkout returns 503.""" | ||
| get_settings.cache_clear() | ||
| monkeypatch.delenv("GRADATA_STRIPE_PRICE_ID_CLOUD", raising=False) | ||
| monkeypatch.delenv("GRADATA_STRIPE_PRICE_ID_TEAM", raising=False) | ||
| monkeypatch.setenv("GRADATA_STRIPE_SECRET_KEY", "sk_test_dummy") | ||
| monkeypatch.setenv("GRADATA_STRIPE_WEBHOOK_SECRET", "whsec_test_dummy") | ||
| yield | ||
| get_settings.cache_clear() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a test for missing stripe_secret_key.
The configured_prices and unconfigured_prices fixtures test price ID presence, but there's no test verifying behavior when stripe_secret_key is empty while price IDs are configured. This scenario would currently result in a Stripe authentication error rather than a clean 503.
Would you like me to generate a test case for this scenario?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/tests/test_billing.py` around lines 33 - 54, Add a test that verifies
behavior when GRADATA_STRIPE_SECRET_KEY is missing while price IDs are
configured: create a new fixture (e.g., stripe_missing_secret) that calls
get_settings.cache_clear(), sets GRADATA_STRIPE_PRICE_ID_CLOUD and
GRADATA_STRIPE_PRICE_ID_TEAM to test values, unsets GRADATA_STRIPE_SECRET_KEY
(monkeypatch.delenv with raising=False), sets GRADATA_STRIPE_WEBHOOK_SECRET,
yields, then clears cache; add a test function (e.g.,
test_checkout_missing_stripe_secret) that uses this fixture and invokes the same
checkout endpoint exercised by existing tests and asserts the response is a 503
(and optionally contains the expected service-unavailable/error message) instead
of raising a Stripe auth error.
| if True: | ||
| event = { | ||
| "type": "customer.subscription.updated", | ||
| "data": { | ||
| "object": { | ||
| "customer": "cus_test_123", | ||
| "status": "active", | ||
| "current_period_end": 1700000000, | ||
| "metadata": {"plan": "team"}, | ||
| } | ||
| }, | ||
| } | ||
| resp = self._post_event(client, fake_stripe, event) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary if True: blocks.
The if True: blocks at lines 245, 276, and 299 serve no purpose and reduce readability. They appear to be debug artifacts or copy-paste remnants.
♻️ Proposed fix
mock_supabase.update = _capture_update # type: ignore[method-assign]
- if True:
- event = {
- "type": "customer.subscription.updated",
- ...
- }
- resp = self._post_event(client, fake_stripe, event)
+ event = {
+ "type": "customer.subscription.updated",
+ ...
+ }
+ resp = self._post_event(client, fake_stripe, event)Apply the same cleanup to test_subscription_deleted_downgrades_to_free (line 276) and test_invoice_payment_failed_marks_past_due (line 299).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if True: | |
| event = { | |
| "type": "customer.subscription.updated", | |
| "data": { | |
| "object": { | |
| "customer": "cus_test_123", | |
| "status": "active", | |
| "current_period_end": 1700000000, | |
| "metadata": {"plan": "team"}, | |
| } | |
| }, | |
| } | |
| resp = self._post_event(client, fake_stripe, event) | |
| event = { | |
| "type": "customer.subscription.updated", | |
| "data": { | |
| "object": { | |
| "customer": "cus_test_123", | |
| "status": "active", | |
| "current_period_end": 1700000000, | |
| "metadata": {"plan": "team"}, | |
| } | |
| }, | |
| } | |
| resp = self._post_event(client, fake_stripe, event) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/tests/test_billing.py` around lines 245 - 257, Remove the redundant "if
True:" blocks wrapping test event setup and posting; locate the three
occurrences inside the test methods that build the "event" dict and call
self._post_event (the shown block using event = {...} and resp =
self._post_event(client, fake_stripe, event)), as well as the similar blocks in
test_subscription_deleted_downgrades_to_free and
test_invoice_payment_failed_marks_past_due, and simply delete the "if True:"
line and unindent the inner lines so the event construction and resp =
self._post_event(...) run directly.
…rktree The 3 backend route files + test_new_routes.py (10 tests) existed in the main checkout but were never committed — they got written to the wrong working tree earlier in the session. Now on the PR branch where they belong. All 72 backend tests pass (48 baseline + 10 new-routes + 14 billing). Co-Authored-By: Gradata <noreply@gradata.ai>
|
Too many files changed for review. ( |
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (6)
cloud/dashboard/app/(dashboard)/corrections/page.tsx (2)
31-45:⚠️ Potential issue | 🟡 Minor“Newest first” is promised but not enforced.
Line 44 says newest-first, but Line 31 only filters and never sorts by
created_at.Suggested fix
- const filtered = filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter) + const filtered = [...(filter === 'all' ? corrections : corrections.filter((c) => c.severity === filter))] + .sort((a, b) => Date.parse(b.created_at) - Date.parse(a.created_at))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx around lines 31 - 45, The UI claims "Newest first" but the list is never sorted; update the filtering logic around the filtered variable (currently defined as const filtered = ...) to sort by Correction.created_at descending after applying the severity filter (e.g., produce a sorted array or replace filtered with a sorted result) so that the corrections shown are in newest-first order; ensure you preserve immutability by creating a new array (use array.slice()/spread before sort) and reference Correction['created_at'] or the created_at field on each correction when sorting.
20-35:⚠️ Potential issue | 🟠 MajorHandle API failures explicitly instead of falling back to empty states.
On Line 20 and Line 22,
useApierrors are ignored; failed requests currently render “No brain yet” / “No corrections,” which is misleading.Suggested fix
- const { data: brains, loading: loadingBrains } = useApi<Brain[]>('/brains') + const { data: brains, loading: loadingBrains, error: brainsError } = useApi<Brain[]>('/brains') const primaryId = brains?.[0]?.id ?? null - const { data: resp, loading } = useApi<PaginatedResponse<Correction> | Correction[]>( + const { data: resp, loading, error: correctionsError } = useApi<PaginatedResponse<Correction> | Correction[]>( primaryId ? `/brains/${primaryId}/corrections` : null, ) if (loadingBrains || loading) return <LoadingSpinner className="py-20" /> + if (brainsError) return <EmptyState title="Failed to load brains" description={brainsError} /> if (!primaryId) return <EmptyState title="No brain yet" description="Install the SDK to start logging corrections." /> + if (correctionsError) return <EmptyState title="Failed to load corrections" description={correctionsError} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx around lines 20 - 35, The component currently ignores API errors from useApi and treats failed requests as empty data (rendering LoadingSpinner or EmptyState); update the logic to read and handle the error fields returned by useApi for both the brains and corrections calls (e.g., the error properties alongside data/loading), and render an explicit error UI/message or ErrorState when either useApi for brains or for corrections reports an error instead of falling back to the "No brain yet" / "No corrections" empty states; locate the useApi calls and the variables primaryId, loadingBrains, loading, resp, corrections, filtered, LoadingSpinner and EmptyState to add conditional branches that surface errors and avoid conflating network failures with legitimately empty data.cloud/dashboard/app/(dashboard)/api-keys/page.tsx (1)
37-39:⚠️ Potential issue | 🟠 MajorClose the create dialog on successful key creation.
At Line 37,
newKeyis set butshowCreateis lefttrue. Because Line 126 gates the dialog withshowCreate && !newKey, closing the one-time key dialog at Line 156 (setNewKey(null)) reopens the create dialog immediately.Proposed fix
try { const res = await api.post<ApiKeyCreateResponse>('/api-keys', { name: keyName || 'Default', }) setNewKey(res.data) + setShowCreate(false) setKeyName('') refetch()Also applies to: 126-127, 156-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx around lines 37 - 39, The create dialog is left open after successful key creation because setNewKey(res.data) is called without closing it; update the success path (where setNewKey(res.data), setKeyName(''), refetch() are called) to also call setShowCreate(false) so the dialog closes on success, and ensure any code that clears the one-time key (setNewKey(null)) does not set showCreate back to true (remove or avoid any setShowCreate(true) around the setNewKey(null) usage) so the dialog doesn't immediately reopen.cloud/dashboard/app/(dashboard)/operator/page.tsx (1)
1-8:⚠️ Potential issue | 🔴 CriticalSensitive mock data bundled in client component — previously flagged.
The
'use client'directive causes the mock operator data (customer names, MRR, health status, alerts) imported at line 8 to be bundled and shipped to every browser, regardless of whether the user passes the domain check. The client-side domain check at lines 32-34 can be trivially bypassed by modifying the response or inspecting the bundle.As noted in the prior review, move operator data retrieval to a server-side protected API endpoint that validates operator role after authentication. The mock fixture should not be imported in client code.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx around lines 1 - 8, The page currently uses the 'use client' directive and directly imports mock operator fixtures (mockGlobalKpis, mockCustomers, mockAlerts, AdminCustomer) which bundles sensitive data into the browser; remove those imports from the client component and instead implement a server-side protected API endpoint that validates the operator role post-authentication and returns the operator data. Update the client component (operator/page.tsx) to call that endpoint via the existing useApi hook (or a fetch wrapper) to retrieve kpis/customers/alerts at runtime, and ensure mock fixtures remain only on the server (or in test/dev-only server code) so no mock data is shipped to the client and the client-side domain check is not relied upon for security.cloud/dashboard/src/components/brain/MethodologyLink.tsx (1)
21-27:⚠️ Potential issue | 🟠 MajorModal still lacks keyboard/focus accessibility essentials.
Line 31 introduces the dialog, but it still has no Escape-to-close, no initial focus/restore focus flow, and no
aria-labelledbyassociation to the heading. This is the same unresolved issue from earlier review.Suggested fix
-'use client' + 'use client' -import { useState } from 'react' +import { useEffect, useRef, useState } from 'react' export function MethodologyLink() { const [open, setOpen] = useState(false) + const triggerRef = useRef<HTMLButtonElement>(null) + const closeRef = useRef<HTMLButtonElement>(null) + + useEffect(() => { + if (!open) return + const prev = document.activeElement as HTMLElement | null + closeRef.current?.focus() + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') setOpen(false) + } + window.addEventListener('keydown', onKeyDown) + return () => { + window.removeEventListener('keydown', onKeyDown) + prev?.focus?.() + } + }, [open]) return ( <> <button + ref={triggerRef} type="button" + aria-haspopup="dialog" + aria-expanded={open} onClick={() => setOpen(true)} className="mt-6 text-[11px] font-mono uppercase tracking-wider text-[var(--color-body)] underline-offset-4 hover:text-[var(--color-accent-blue)] hover:underline" > Methodology & limitations → @@ <div role="dialog" aria-modal="true" + aria-labelledby="methodology-title" className="fixed inset-0 z-[100] flex items-center justify-center bg-black/60 p-4 backdrop-blur-sm" onClick={() => setOpen(false)} > @@ - <h3 className="text-[18px] font-semibold">Methodology & limitations</h3> + <h3 id="methodology-title" className="text-[18px] font-semibold">Methodology & limitations</h3> <button + type="button" + ref={closeRef} onClick={() => setOpen(false)} className="-mr-2 flex h-11 w-11 shrink-0 items-center justify-center rounded-[0.5rem] text-[24px] leading-none text-[var(--color-body)] hover:bg-white/[0.04] hover:text-[var(--color-text)]" aria-label="Close"Also applies to: 31-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/brain/MethodologyLink.tsx` around lines 21 - 27, The modal launched by the MethodologyLink component (button onClick calls setOpen(true)) lacks keyboard and focus semantics; update the dialog implementation to: add an aria-labelledby that points to the modal heading id (ensure the heading element has that id), implement Escape-to-close by handling keydown/Escape to call setOpen(false), set initial focus into a meaningful focusable element inside the dialog when opened and restore focus back to the MethodologyLink button when closed, and ensure focus is trapped inside the dialog while open (use the existing dialog component or a focus-trap utility to manage focus).cloud/dashboard/src/components/brain/KpiStrip.tsx (1)
12-13:⚠️ Potential issue | 🟡 MinorUse the same rounded delta for both label and tone.
Line 13 rounds only for display while Lines 28–30 classify tone from the raw value. Small deltas can render as
-0%/0%but still use non-neutral tone.Suggested fix
export function KpiStrip({ metrics }: { metrics: KpiMetrics }) { - const fmtDelta = (pct: number) => - pct === 0 ? '—' : `${pct > 0 ? '+' : ''}${pct.toFixed(0)}%` + const roundedCorrectionDelta = Math.round(metrics.correctionRateDeltaPct) + const fmtDelta = (pct: number) => + pct === 0 ? '—' : `${pct > 0 ? '+' : ''}${pct}%` @@ { label: 'Correction Rate', - value: metrics.correctionRateDeltaPct === 0 + value: roundedCorrectionDelta === 0 ? '—' - : `${fmtDelta(metrics.correctionRateDeltaPct)}`, + : fmtDelta(roundedCorrectionDelta), change: `${metrics.correctionsThisWeek} this week · ${metrics.correctionsPriorWeek} prior`, changeTone: - metrics.correctionRateDeltaPct < 0 ? 'pos' - : metrics.correctionRateDeltaPct > 0 ? 'neg' + roundedCorrectionDelta < 0 ? 'pos' + : roundedCorrectionDelta > 0 ? 'neg' : 'neu', },Also applies to: 23-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cloud/dashboard/src/components/brain/KpiStrip.tsx` around lines 12 - 13, Compute a single rounded delta and use it for both display and tone: in fmtDelta (or the surrounding component) derive const roundedPct = Math.round(pct) (or parsed Number(pct.toFixed(0))) and use roundedPct for the string output (producing '—' when roundedPct === 0 and avoiding '-0%') and also use roundedPct for the tone classification logic currently using the raw pct (the code around the tone classification lines 28–30) so that small values that round to 0 get the neutral tone.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cloud/app/routes/activity.py`:
- Around line 32-38: The function list_activity declares an unused parameter
request: Request; remove this parameter from the function signature in
cloud/app/routes/activity.py (keep the other parameters: brain_id, credentials:
HTTPAuthorizationCredentials = Depends(_bearer), limit, offset) or, if the
Request is intentionally reserved for future middleware/logging, rename it to
_request to silence linters; update any imports or type hints accordingly so the
signature no longer contains the unused request variable.
In `@cloud/app/routes/meta_rules.py`:
- Around line 20-26: The list_meta_rules route declares an unused parameter
request: Request; remove the unused parameter from the function signature in
list_meta_rules and also remove the corresponding Request import from this
module (or mark it unused/underscore if intentionally kept), ensuring the
function signature becomes async def list_meta_rules(brain_id: str, credentials:
HTTPAuthorizationCredentials = Depends(_bearer), limit: int = Query(50, ge=1,
le=200), offset: int = Query(0, ge=0)) -> list[dict]: and adjust any callers if
necessary.
In `@cloud/app/routes/rule_patches.py`:
- Around line 20-26: Remove the unused Request parameter from both endpoint
signatures: update list_rule_patches(...) and rollback_patch(...) to drop the
request: Request argument and any references to it, and then remove the
now-unused Request import from the module; keep the credentials dependency and
Query params unchanged so endpoint behavior remains the same.
- Around line 86-101: The rollback currently inserts an inverse patch via
db.insert(...) before updating the lesson, risking an inconsistent audit trail
if the update fails; change the order in rule_patches.py to perform the lesson
update (await db.update("lessons", {"description": patch["old_description"]},
filters={"id": patch["lesson_id"]})) first and only after it succeeds insert the
inverse audit record (await db.insert("rule_patches", {...})); also catch and
log any error from the subsequent insert of the audit record (including patch_id
and lesson_id) so a failed audit write is visible for follow-up.
In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx:
- Around line 126-127: Reset the createError state whenever the create dialog is
dismissed by clearing createError inside the Dialog open-change handler: update
the onOpenChange (where Dialog currently uses setShowCreate) to call
setCreateError(null) when the dialog is closed (and keep existing setShowCreate
behavior), and apply the same change to the other Dialog instance referenced
around the 135-139 area so any stale error is cleared each time the create
dialog is closed.
In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx:
- Around line 44-45: The displayed count uses filtered.length while the list
actually renders filtered.slice(0, 50), causing mismatch when >50 items exist;
update the UI text to reflect the number shown versus total (e.g., "showing
{Math.min(filtered.length, 50)} of {filtered.length}") or change the rendered
list to show all items — adjust the JSX where filtered.length is used and where
filtered.slice(0, 50) is rendered (look for the variables/expressions filtered
and filtered.slice(0, 50) in the page component) so the count and displayed
items stay consistent.
In `@cloud/dashboard/app/`(dashboard)/dashboard/page.tsx:
- Around line 83-94: The time-range buttons in dashboard/page.tsx rendering
inside the map lack an accessibility state; update the <button> that sets range
(the one using onClick={() => setRange(r)} and comparing r === range) to include
aria-pressed with a boolean value reflecting selection (e.g., aria-pressed={r
=== range}) so screen readers can detect the active state.
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx:
- Around line 164-170: The formatAgo(iso: string) function must validate the
parsed date and handle future timestamps: first construct the Date from iso and
check isNaN(date.getTime()) — if invalid return a sensible fallback like
'invalid date' or 'unknown'; then compute diffMs and if diffMs < 0 handle future
times (e.g., return 'in Xh' or 'just now' depending on desired UX) instead of
treating them as recent; ensure you use the same math for hours/days (h =
Math.floor(Math.abs(diffMs) / 3600_000)) when formatting future intervals and
clamp/format appropriately so NaN never propagates to the returned string.
- Around line 29-33: The current logic uses useApi<UserProfile>('/users/me') and
treats profile being undefined the same as a non-operator; instead detect and
handle the API error state returned by useApi (e.g., an error or status flag)
rather than only checking loading, and surface an error UI or retry option
before computing domain/isOperator; update the component to branch: if (loading)
show LoadingSpinner, else if (error) show an error message and retry control (or
a dismissible toast), else compute const domain = profile.email.split('@')[1]
and const isOperator = OPERATOR_EMAIL_DOMAINS.includes(domain) so transient
failures don’t incorrectly render the "restricted" view.
In `@cloud/dashboard/app/`(dashboard)/team/page.tsx:
- Around line 11-15: The component TeamOverviewPage currently defaults
currentPlan to 'free' when profile is null; instead destructure error from
useApi (e.g., const { data: profile, loading, error } =
useApi<UserProfile>('/users/me')) and, when error is truthy, render the
ErrorState component (import ErrorState from '@/components/shared/ErrorState')
before computing currentPlan so API failures are surfaced rather than silently
falling back to 'free'; keep the existing loading path for loading and only
compute currentPlan after confirming profile exists.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx`:
- Around line 152-157: The Sign out button element should explicitly declare
type to avoid accidental form submissions; update the JSX for the button that
calls signOut (the onClick={() => signOut()} button in DashboardLayout
component) to include type="button" so it remains a non-submitting control even
if later wrapped in a form.
In `@cloud/dashboard/tests/MetaRulesGrid.test.tsx`:
- Around line 19-22: The useApi mock in the tests currently returns fallback
data for any non-'/brains' URL which masks incorrect endpoint usage; update the
mockImplementation for useApiMock to explicitly branch on each expected URL
(e.g., '/brains', other endpoints used in this test) and return withData(...)
for each expected path, and throw an error (or fail) for any unexpected url to
ensure tests assert the endpoint contract; apply the same strict URL-branching
pattern to the other useApiMock occurrences in this file (the blocks around
lines that currently return fallback []) so each test verifies exact API paths.
---
Duplicate comments:
In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx:
- Around line 37-39: The create dialog is left open after successful key
creation because setNewKey(res.data) is called without closing it; update the
success path (where setNewKey(res.data), setKeyName(''), refetch() are called)
to also call setShowCreate(false) so the dialog closes on success, and ensure
any code that clears the one-time key (setNewKey(null)) does not set showCreate
back to true (remove or avoid any setShowCreate(true) around the setNewKey(null)
usage) so the dialog doesn't immediately reopen.
In `@cloud/dashboard/app/`(dashboard)/corrections/page.tsx:
- Around line 31-45: The UI claims "Newest first" but the list is never sorted;
update the filtering logic around the filtered variable (currently defined as
const filtered = ...) to sort by Correction.created_at descending after applying
the severity filter (e.g., produce a sorted array or replace filtered with a
sorted result) so that the corrections shown are in newest-first order; ensure
you preserve immutability by creating a new array (use array.slice()/spread
before sort) and reference Correction['created_at'] or the created_at field on
each correction when sorting.
- Around line 20-35: The component currently ignores API errors from useApi and
treats failed requests as empty data (rendering LoadingSpinner or EmptyState);
update the logic to read and handle the error fields returned by useApi for both
the brains and corrections calls (e.g., the error properties alongside
data/loading), and render an explicit error UI/message or ErrorState when either
useApi for brains or for corrections reports an error instead of falling back to
the "No brain yet" / "No corrections" empty states; locate the useApi calls and
the variables primaryId, loadingBrains, loading, resp, corrections, filtered,
LoadingSpinner and EmptyState to add conditional branches that surface errors
and avoid conflating network failures with legitimately empty data.
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx:
- Around line 1-8: The page currently uses the 'use client' directive and
directly imports mock operator fixtures (mockGlobalKpis, mockCustomers,
mockAlerts, AdminCustomer) which bundles sensitive data into the browser; remove
those imports from the client component and instead implement a server-side
protected API endpoint that validates the operator role post-authentication and
returns the operator data. Update the client component (operator/page.tsx) to
call that endpoint via the existing useApi hook (or a fetch wrapper) to retrieve
kpis/customers/alerts at runtime, and ensure mock fixtures remain only on the
server (or in test/dev-only server code) so no mock data is shipped to the
client and the client-side domain check is not relied upon for security.
In `@cloud/dashboard/src/components/brain/KpiStrip.tsx`:
- Around line 12-13: Compute a single rounded delta and use it for both display
and tone: in fmtDelta (or the surrounding component) derive const roundedPct =
Math.round(pct) (or parsed Number(pct.toFixed(0))) and use roundedPct for the
string output (producing '—' when roundedPct === 0 and avoiding '-0%') and also
use roundedPct for the tone classification logic currently using the raw pct
(the code around the tone classification lines 28–30) so that small values that
round to 0 get the neutral tone.
In `@cloud/dashboard/src/components/brain/MethodologyLink.tsx`:
- Around line 21-27: The modal launched by the MethodologyLink component (button
onClick calls setOpen(true)) lacks keyboard and focus semantics; update the
dialog implementation to: add an aria-labelledby that points to the modal
heading id (ensure the heading element has that id), implement Escape-to-close
by handling keydown/Escape to call setOpen(false), set initial focus into a
meaningful focusable element inside the dialog when opened and restore focus
back to the MethodologyLink button when closed, and ensure focus is trapped
inside the dialog while open (use the existing dialog component or a focus-trap
utility to manage focus).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 37ab0561-91e8-4bef-8087-d4666fbc2ca3
⛔ Files ignored due to path filters (1)
cloud/dashboard/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
cloud/app/routes/__init__.pycloud/app/routes/activity.pycloud/app/routes/meta_rules.pycloud/app/routes/rule_patches.pycloud/dashboard/app/(dashboard)/api-keys/page.tsxcloud/dashboard/app/(dashboard)/corrections/page.tsxcloud/dashboard/app/(dashboard)/dashboard/page.tsxcloud/dashboard/app/(dashboard)/operator/page.tsxcloud/dashboard/app/(dashboard)/rules/page.tsxcloud/dashboard/app/(dashboard)/team/members/page.tsxcloud/dashboard/app/(dashboard)/team/page.tsxcloud/dashboard/package.jsoncloud/dashboard/src/components/brain/KpiStrip.tsxcloud/dashboard/src/components/brain/MethodologyLink.tsxcloud/dashboard/src/components/layout/DashboardLayout.tsxcloud/dashboard/tests/ActiveRulesPanel.test.tsxcloud/dashboard/tests/ActivityFeed.test.tsxcloud/dashboard/tests/CategoriesChart.test.tsxcloud/dashboard/tests/GraduationProgressBar.test.tsxcloud/dashboard/tests/KpiStrip.test.tsxcloud/dashboard/tests/MetaRulesGrid.test.tsxcloud/dashboard/tests/PlanBadge.test.tsxcloud/dashboard/tests/analytics-client.test.tscloud/dashboard/tests/setup.tscloud/dashboard/vitest.config.tscloud/tests/test_new_routes.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (34)
cloud/dashboard/app/(dashboard)/corrections/page.tsx (1)
22-24: Conditional fetch gating withnullURL is correctly applied.Deferring corrections fetch until
primaryIdis available avoids premature requests and matches the hook contract.cloud/dashboard/app/(dashboard)/api-keys/page.tsx (1)
90-102: Good change: only key prefix is rendered in the list.Showing
gd_{k.key_prefix}…instead of full key material is a solid safety improvement for accidental secret exposure in UI.cloud/dashboard/app/(dashboard)/operator/page.tsx (3)
16-26: LGTM!The style constants are well-typed using
Recordandas const, providing type safety and covering all expected health/alert states.
144-162: LGTM!The
Kpihelper component is clean and properly handles all tone variants with appropriate color styling.
115-115: The type assertionc.plan as PlanTieris redundant. BothAdminCustomer.planandPlanTierare identically defined as'free' | 'cloud' | 'team' | 'enterprise', so the assertion is unnecessary. Consider removing it to let TypeScript infer the correct type automatically.> Likely an incorrect or invalid review comment.cloud/dashboard/src/components/layout/DashboardLayout.tsx (3)
67-80: Addaria-currentandaria-hiddenfor assistive technology support.The past review comment about accessibility has not been addressed. The active nav link should expose its state to screen readers, and the decorative icon glyphs should be hidden from assistive tech.
♿ Suggested fix
<Link key={item.href} href={item.href} onClick={onNavigate} + aria-current={active ? 'page' : undefined} className={cn( 'flex items-center gap-2.5 rounded-[0.5rem] px-3 py-2 text-[13px] transition-all', active ? 'border border-[rgba(58,130,255,0.15)] bg-[rgba(58,130,255,0.12)] text-[var(--color-text)]' : 'text-[var(--color-body)] hover:bg-[rgba(58,130,255,0.08)] hover:text-[var(--color-text)]', )} > - <span className="w-4 text-center text-sm opacity-50">{item.icon}</span> + <span aria-hidden="true" className="w-4 text-center text-sm opacity-50"> + {item.icon} + </span> {item.label} </Link>
97-123: LGTM!The mobile sidebar drawer is well-implemented with proper body scroll lock (storing and restoring previous overflow value), accessible overlay button, and auto-close on navigation.
164-178: LGTM!Clean Next.js layout composition with proper
childrenrendering pattern. The local state for mobile drawer toggle is appropriate, and the integration with the parentProtectedLayout(which handles auth redirect and renders<DashboardLayout>) is correct.cloud/dashboard/tests/setup.ts (1)
1-1: Good shared matcher setup.Keeps test files clean and ensures consistent DOM matcher availability.
cloud/dashboard/vitest.config.ts (1)
4-10: Vitest configuration looks solid.Environment, setup bootstrap, and alias mapping are aligned with the added RTL test suite.
cloud/dashboard/tests/ActiveRulesPanel.test.tsx (1)
23-63: Strong behavioral coverage for panel rules.The suite validates the key rendering contracts (filter/sort/empty-state/confidence suppression) well.
cloud/dashboard/tests/PlanBadge.test.tsx (1)
5-64: Plan badge/gate test coverage is good.It exercises both presentation mapping and rank-based gating behavior in a clear way.
cloud/dashboard/tests/GraduationProgressBar.test.tsx (1)
14-60: Comprehensive progress-bar test set.Covers both arithmetic correctness and UI output for populated and empty states.
cloud/dashboard/package.json (1)
5-48: Version compatibility already flagged in prior review.The dependency versions (next@16.0.0, typescript@6.0.2, vitest@4.1.4, jsdom@29.0.2) are beyond my knowledge cutoff. A prior review comment already requested verification of these package versions and their compatibility.
cloud/dashboard/app/(dashboard)/rules/page.tsx (1)
18-33: API error handling and array mutation issues already flagged.Prior review comments correctly identified:
- API failures silently fall through to empty states instead of showing
ErrorState- The
.sort()call mutates the originallessonsarray whenfilter === 'all'These issues remain unaddressed.
cloud/dashboard/app/(dashboard)/team/members/page.tsx (1)
47-57: Invite flow gating and inert Remove button already flagged.Prior review comments correctly identified:
- The "Invite member" button and dialog (lines 54, 94-134) are outside the
PlanGate, allowing users without theteamentitlement to access the invite workflow- The "Remove" button (lines 79-87) has no
onClickhandler, making it a non-functional UI elementThese issues remain unaddressed.
Also applies to: 79-87, 94-134
cloud/dashboard/tests/ActivityFeed.test.tsx (1)
1-79: LGTM! Well-structured test suite for ActivityFeed.The tests properly mock
useApibefore importing the component and cover the key scenarios: fixture fallback, real data rendering, type normalization, and graceful handling of missing data.cloud/dashboard/tests/CategoriesChart.test.tsx (1)
1-70: LGTM! Thorough coverage of the category folding taxonomy.The tests correctly validate the fixed 6-dimension rendering and legacy category mappings defined in
LEGACY_MAP. The use ofclosest('li')?.textContentto verify counts is a pragmatic approach.cloud/dashboard/tests/KpiStrip.test.tsx (1)
1-52: LGTM! Test assertions align correctly with the KpiStrip implementation.The tests properly verify the conditional CSS classes for tone-based styling, and the placeholder behavior for zero values matches the component logic shown in the context snippets.
cloud/dashboard/tests/analytics-client.test.ts (3)
117-135: LGTM!buildDecayCurvetests verify bucket structure correctly.The parameterized test for bucket sizing and the field presence assertions provide good coverage of the decay curve output shape.
92-115: LGTM!computeGraduationCountstests are well-structured.The tests correctly verify per-state counting,
totalActivecalculation as pattern + rule (excluding instinct), and the edge case of empty lessons returning 0 for all confidence averages. The implementation and comment are consistent and accurate.> Likely an incorrect or invalid review comment.
46-89: No issues found. All test expectations align with thecomputeKpisimplementation: empty inputs correctly return zero values, the negative delta formula matches the test assertion, footprint calculation is11 KB × correction count, andtotalFirescorrectly sums onlyRULEandPATTERNlesson fire counts. The tests are well-written and correct.cloud/dashboard/app/(dashboard)/team/page.tsx (1)
74-124: LGTM! Clean helper components for KPI cards and leaderboard rows.The
KpiandLeaderRowcomponents are well-structured with appropriate prop types and conditional styling logic.cloud/dashboard/app/(dashboard)/dashboard/page.tsx (3)
26-34: Dependent requests' loading states are not tracked, causing a transient empty-state flash.The
loadingstate fromanalytics,correctionsResp, andlessonsResphooks is not destructured. AfterprimaryBrainIdbecomes available, the widgets render withnull/empty data for one frame before the effect triggers and data loads. This was previously flagged.
36-44: Only the first page of paginated data is used for KPI and chart calculations.When endpoints return
PaginatedResponse, lines 38 and 43 extract only the.dataarray from the first page. Once a brain accumulates enough corrections or lessons to span multiple pages, all dashboard metrics become silently incomplete. This was previously flagged.
99-114: LGTM on conditional widget rendering.
KpiStripandCategoriesChartare correctly gated on their data being present. The remaining widgets that receive potentially-empty arrays will benefit from the loading-state gating fix suggested in the earlier comment.cloud/app/routes/activity.py (1)
47-55: LGTM — correct filtering and pagination.The in-memory filtering against
_VISIBLE_EVENT_TYPESand newest-first sorting is correct. The comment inrule_patches.pyacknowledges this is acceptable for MVP volume; consider adding server-side type filtering via PostgRESTinoperator when event volume grows.cloud/app/routes/rule_patches.py (1)
35-51: LGTM — correct lesson-scoped filtering.The two-step query (fetch brain's lessons, then filter patches by lesson_id set) correctly enforces brain-level isolation. The MVP caveat about fetching all patches is documented.
cloud/app/routes/__init__.py (1)
7-7: LGTM!The three new routers are correctly imported and registered with descriptive tags matching their endpoint groups.
Also applies to: 14-15, 27-29
cloud/app/routes/meta_rules.py (1)
36-44: LGTM!Direct filtering by
brain_idat the DB level is efficient. Sorting and pagination are correctly applied.cloud/tests/test_new_routes.py (4)
7-18: LGTM — clean fixture for bypassing auth.The
valid_bearer_patchesfixture correctly stubsget_brain_for_requestacross all three route modules, allowing tests to focus on DB query behavior.
21-80: LGTM!
TestMetaRulesEndpointandTestActivityEndpointprovide good coverage for empty results, ordering, pagination, and type filtering. The visible-types test (lines 57-74) effectively validates that only whitelisted event types are returned.
137-159: LGTM — good 404 coverage.Both edge cases (patch belongs to different brain, patch doesn't exist) correctly verify the 404 response, ensuring the endpoint doesn't leak information about patches from other brains.
113-135: Test doesn't verify lesson update.The rollback test asserts that an inverse patch was inserted (lines 130-135) but doesn't verify that the lesson's
descriptionwas updated toold_description. The endpoint does calldb.update("lessons", ...)on line 97-101, but if that call were accidentally removed, this test would still pass.However, the suggested fix in the original comment references
mock_supabase._updates, which doesn't exist in theMockSupabaseClient— the mock only tracks inserts. Consider either enhancing the mock to track updates, or adding a different verification approach for this behavior.> Likely an incorrect or invalid review comment.
| async def list_activity( | ||
| brain_id: str, | ||
| request: Request, | ||
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | ||
| limit: int = Query(50, ge=1, le=200), | ||
| offset: int = Query(0, ge=0), | ||
| ) -> list[dict]: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused request parameter.
The request: Request parameter is declared but never used in the function body. Remove it unless it's needed for middleware or future logging.
🧹 Proposed fix
`@router.get`("/brains/{brain_id}/activity")
async def list_activity(
brain_id: str,
- request: Request,
credentials: HTTPAuthorizationCredentials = Depends(_bearer),
limit: int = Query(50, ge=1, le=200),
offset: int = Query(0, ge=0),
) -> list[dict]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def list_activity( | |
| brain_id: str, | |
| request: Request, | |
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | |
| limit: int = Query(50, ge=1, le=200), | |
| offset: int = Query(0, ge=0), | |
| ) -> list[dict]: | |
| async def list_activity( | |
| brain_id: str, | |
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | |
| limit: int = Query(50, ge=1, le=200), | |
| offset: int = Query(0, ge=0), | |
| ) -> list[dict]: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/activity.py` around lines 32 - 38, The function
list_activity declares an unused parameter request: Request; remove this
parameter from the function signature in cloud/app/routes/activity.py (keep the
other parameters: brain_id, credentials: HTTPAuthorizationCredentials =
Depends(_bearer), limit, offset) or, if the Request is intentionally reserved
for future middleware/logging, rename it to _request to silence linters; update
any imports or type hints accordingly so the signature no longer contains the
unused request variable.
| async def list_meta_rules( | ||
| brain_id: str, | ||
| request: Request, | ||
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | ||
| limit: int = Query(50, ge=1, le=200), | ||
| offset: int = Query(0, ge=0), | ||
| ) -> list[dict]: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused request parameter.
Same as the other route files — request: Request is declared but unused.
🧹 Proposed fix
`@router.get`("/brains/{brain_id}/meta-rules")
async def list_meta_rules(
brain_id: str,
- request: Request,
credentials: HTTPAuthorizationCredentials = Depends(_bearer),
limit: int = Query(50, ge=1, le=200),
offset: int = Query(0, ge=0),
) -> list[dict]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def list_meta_rules( | |
| brain_id: str, | |
| request: Request, | |
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | |
| limit: int = Query(50, ge=1, le=200), | |
| offset: int = Query(0, ge=0), | |
| ) -> list[dict]: | |
| async def list_meta_rules( | |
| brain_id: str, | |
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | |
| limit: int = Query(50, ge=1, le=200), | |
| offset: int = Query(0, ge=0), | |
| ) -> list[dict]: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/meta_rules.py` around lines 20 - 26, The list_meta_rules
route declares an unused parameter request: Request; remove the unused parameter
from the function signature in list_meta_rules and also remove the corresponding
Request import from this module (or mark it unused/underscore if intentionally
kept), ensuring the function signature becomes async def
list_meta_rules(brain_id: str, credentials: HTTPAuthorizationCredentials =
Depends(_bearer), limit: int = Query(50, ge=1, le=200), offset: int = Query(0,
ge=0)) -> list[dict]: and adjust any callers if necessary.
| async def list_rule_patches( | ||
| brain_id: str, | ||
| request: Request, | ||
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | ||
| limit: int = Query(50, ge=1, le=200), | ||
| offset: int = Query(0, ge=0), | ||
| ) -> list[dict]: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused request parameter in both endpoints.
Both list_rule_patches and rollback_patch declare request: Request but never use it.
🧹 Proposed fix for list_rule_patches
`@router.get`("/brains/{brain_id}/rule-patches")
async def list_rule_patches(
brain_id: str,
- request: Request,
credentials: HTTPAuthorizationCredentials = Depends(_bearer),
limit: int = Query(50, ge=1, le=200),
offset: int = Query(0, ge=0),
) -> list[dict]:🧹 Proposed fix for rollback_patch
`@router.post`("/brains/{brain_id}/rule-patches/{patch_id}/rollback", status_code=204)
async def rollback_patch(
brain_id: str,
patch_id: str,
- request: Request,
credentials: HTTPAuthorizationCredentials = Depends(_bearer),
) -> None:Also applies to: 55-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/rule_patches.py` around lines 20 - 26, Remove the unused
Request parameter from both endpoint signatures: update list_rule_patches(...)
and rollback_patch(...) to drop the request: Request argument and any references
to it, and then remove the now-unused Request import from the module; keep the
credentials dependency and Query params unchanged so endpoint behavior remains
the same.
| # Flip the patch by creating the inverse | ||
| await db.insert( | ||
| "rule_patches", | ||
| { | ||
| "lesson_id": patch["lesson_id"], | ||
| "old_description": patch["new_description"], | ||
| "new_description": patch["old_description"], | ||
| "reason": f"rollback of patch {patch_id}", | ||
| }, | ||
| ) | ||
| # Apply the restored text to the lesson | ||
| await db.update( | ||
| "lessons", | ||
| {"description": patch["old_description"]}, | ||
| filters={"id": patch["lesson_id"]}, | ||
| ) |
There was a problem hiding this comment.
Non-atomic rollback risks partial failure.
The rollback performs two sequential operations: inserting an inverse patch (line 87-95) and updating the lesson (line 97-100). Per cloud/app/db.py, there's no transaction wrapper. If the insert succeeds but the update fails (network issue, constraint violation), the audit trail will show a rollback that never actually applied to the lesson.
Consider one of:
- Reversing the order: update lesson first, then insert audit record (failed audit insert is less harmful than inconsistent lesson state)
- Adding a compensation mechanism if the second operation fails
- Documenting this as a known limitation for MVP
🛡️ Suggested reorder (lower-risk approach)
- # Flip the patch by creating the inverse
- await db.insert(
- "rule_patches",
- {
- "lesson_id": patch["lesson_id"],
- "old_description": patch["new_description"],
- "new_description": patch["old_description"],
- "reason": f"rollback of patch {patch_id}",
- },
- )
# Apply the restored text to the lesson
await db.update(
"lessons",
{"description": patch["old_description"]},
filters={"id": patch["lesson_id"]},
)
+ # Record the inverse patch for audit (after lesson is successfully updated)
+ await db.insert(
+ "rule_patches",
+ {
+ "lesson_id": patch["lesson_id"],
+ "old_description": patch["new_description"],
+ "new_description": patch["old_description"],
+ "reason": f"rollback of patch {patch_id}",
+ },
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Flip the patch by creating the inverse | |
| await db.insert( | |
| "rule_patches", | |
| { | |
| "lesson_id": patch["lesson_id"], | |
| "old_description": patch["new_description"], | |
| "new_description": patch["old_description"], | |
| "reason": f"rollback of patch {patch_id}", | |
| }, | |
| ) | |
| # Apply the restored text to the lesson | |
| await db.update( | |
| "lessons", | |
| {"description": patch["old_description"]}, | |
| filters={"id": patch["lesson_id"]}, | |
| ) | |
| # Apply the restored text to the lesson | |
| await db.update( | |
| "lessons", | |
| {"description": patch["old_description"]}, | |
| filters={"id": patch["lesson_id"]}, | |
| ) | |
| # Record the inverse patch for audit (after lesson is successfully updated) | |
| await db.insert( | |
| "rule_patches", | |
| { | |
| "lesson_id": patch["lesson_id"], | |
| "old_description": patch["new_description"], | |
| "new_description": patch["old_description"], | |
| "reason": f"rollback of patch {patch_id}", | |
| }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/app/routes/rule_patches.py` around lines 86 - 101, The rollback
currently inserts an inverse patch via db.insert(...) before updating the
lesson, risking an inconsistent audit trail if the update fails; change the
order in rule_patches.py to perform the lesson update (await
db.update("lessons", {"description": patch["old_description"]}, filters={"id":
patch["lesson_id"]})) first and only after it succeeds insert the inverse audit
record (await db.insert("rule_patches", {...})); also catch and log any error
from the subsequent insert of the audit record (including patch_id and
lesson_id) so a failed audit write is visible for follow-up.
| <Dialog open={showCreate && !newKey} onOpenChange={setShowCreate}> | ||
| <DialogContent> |
There was a problem hiding this comment.
Reset createError when the dialog is dismissed.
If a create attempt fails once, the old error message persists and is shown again on next open before any new submission.
Proposed fix
- <Dialog open={showCreate && !newKey} onOpenChange={setShowCreate}>
+ <Dialog
+ open={showCreate && !newKey}
+ onOpenChange={(open) => {
+ setShowCreate(open)
+ if (!open) setCreateError(null)
+ }}
+ >Also applies to: 135-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/api-keys/page.tsx around lines 126 - 127,
Reset the createError state whenever the create dialog is dismissed by clearing
createError inside the Dialog open-change handler: update the onOpenChange
(where Dialog currently uses setShowCreate) to call setCreateError(null) when
the dialog is closed (and keep existing setShowCreate behavior), and apply the
same change to the other Dialog instance referenced around the 135-139 area so
any stale error is cleared each time the create dialog is closed.
| const { data: profile, loading } = useApi<UserProfile>('/users/me') | ||
| if (loading) return <LoadingSpinner className="py-20" /> | ||
|
|
||
| const domain = profile?.email?.split('@')[1] ?? '' | ||
| const isOperator = OPERATOR_EMAIL_DOMAINS.includes(domain) |
There was a problem hiding this comment.
Handle API error state.
If the /users/me call fails, loading becomes false but profile remains undefined. This causes domain to be an empty string, showing the "restricted" message even for legitimate operators experiencing transient network issues.
Consider handling the error state explicitly to show an appropriate error message or retry option.
🛡️ Proposed fix
export default function OperatorPage() {
- const { data: profile, loading } = useApi<UserProfile>('/users/me')
+ const { data: profile, loading, error } = useApi<UserProfile>('/users/me')
if (loading) return <LoadingSpinner className="py-20" />
+ if (error) {
+ return (
+ <div className="py-12 text-center">
+ <h1 className="text-[22px]">Operator</h1>
+ <p className="mt-3 text-[13px] text-[var(--color-destructive)]">
+ Failed to load profile. Please refresh to try again.
+ </p>
+ </div>
+ )
+ }
const domain = profile?.email?.split('@')[1] ?? ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: profile, loading } = useApi<UserProfile>('/users/me') | |
| if (loading) return <LoadingSpinner className="py-20" /> | |
| const domain = profile?.email?.split('@')[1] ?? '' | |
| const isOperator = OPERATOR_EMAIL_DOMAINS.includes(domain) | |
| export default function OperatorPage() { | |
| const { data: profile, loading, error } = useApi<UserProfile>('/users/me') | |
| if (loading) return <LoadingSpinner className="py-20" /> | |
| if (error) { | |
| return ( | |
| <div className="py-12 text-center"> | |
| <h1 className="text-[22px]">Operator</h1> | |
| <p className="mt-3 text-[13px] text-[var(--color-destructive)]"> | |
| Failed to load profile. Please refresh to try again. | |
| </p> | |
| </div> | |
| ) | |
| } | |
| const domain = profile?.email?.split('@')[1] ?? '' | |
| const isOperator = OPERATOR_EMAIL_DOMAINS.includes(domain) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx around lines 29 - 33, The
current logic uses useApi<UserProfile>('/users/me') and treats profile being
undefined the same as a non-operator; instead detect and handle the API error
state returned by useApi (e.g., an error or status flag) rather than only
checking loading, and surface an error UI or retry option before computing
domain/isOperator; update the component to branch: if (loading) show
LoadingSpinner, else if (error) show an error message and retry control (or a
dismissible toast), else compute const domain = profile.email.split('@')[1] and
const isOperator = OPERATOR_EMAIL_DOMAINS.includes(domain) so transient failures
don’t incorrectly render the "restricted" view.
| function formatAgo(iso: string): string { | ||
| const diffMs = Date.now() - new Date(iso).getTime() | ||
| const h = Math.floor(diffMs / 3600_000) | ||
| if (h < 1) return 'just now' | ||
| if (h < 24) return `${h}h ago` | ||
| return `${Math.floor(h / 24)}d ago` | ||
| } |
There was a problem hiding this comment.
Handle invalid or future dates gracefully.
If iso is malformed, new Date(iso) returns an invalid date, causing diffMs to be NaN. This propagates through the math operations, resulting in "NaNd ago" being displayed. Additionally, future dates produce negative diffMs, which would incorrectly show "just now".
Consider adding validation:
🛡️ Proposed fix
function formatAgo(iso: string): string {
+ const date = new Date(iso)
+ if (isNaN(date.getTime())) return '—'
- const diffMs = Date.now() - new Date(iso).getTime()
+ const diffMs = Date.now() - date.getTime()
+ if (diffMs < 0) return 'just now'
const h = Math.floor(diffMs / 3600_000)
if (h < 1) return 'just now'
if (h < 24) return `${h}h ago`
return `${Math.floor(h / 24)}d ago`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function formatAgo(iso: string): string { | |
| const diffMs = Date.now() - new Date(iso).getTime() | |
| const h = Math.floor(diffMs / 3600_000) | |
| if (h < 1) return 'just now' | |
| if (h < 24) return `${h}h ago` | |
| return `${Math.floor(h / 24)}d ago` | |
| } | |
| function formatAgo(iso: string): string { | |
| const date = new Date(iso) | |
| if (isNaN(date.getTime())) return '—' | |
| const diffMs = Date.now() - date.getTime() | |
| if (diffMs < 0) return 'just now' | |
| const h = Math.floor(diffMs / 3600_000) | |
| if (h < 1) return 'just now' | |
| if (h < 24) return `${h}h ago` | |
| return `${Math.floor(h / 24)}d ago` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/operator/page.tsx around lines 164 - 170,
The formatAgo(iso: string) function must validate the parsed date and handle
future timestamps: first construct the Date from iso and check
isNaN(date.getTime()) — if invalid return a sensible fallback like 'invalid
date' or 'unknown'; then compute diffMs and if diffMs < 0 handle future times
(e.g., return 'in Xh' or 'just now' depending on desired UX) instead of treating
them as recent; ensure you use the same math for hours/days (h =
Math.floor(Math.abs(diffMs) / 3600_000)) when formatting future intervals and
clamp/format appropriately so NaN never propagates to the returned string.
| export default function TeamOverviewPage() { | ||
| const { data: profile, loading } = useApi<UserProfile>('/users/me') | ||
| if (loading) return <LoadingSpinner className="py-20" /> | ||
|
|
||
| const currentPlan = (profile?.plan?.toLowerCase() ?? 'free') as PlanTier |
There was a problem hiding this comment.
Surface API failure instead of defaulting to 'free' plan.
If the /users/me call fails, profile will be null and currentPlan silently defaults to 'free'. This could incorrectly gate features for users who have a higher plan but experienced a transient API error. Consider destructuring error from useApi and rendering an ErrorState when the profile fetch fails.
🛡️ Proposed fix
export default function TeamOverviewPage() {
- const { data: profile, loading } = useApi<UserProfile>('/users/me')
+ const { data: profile, loading, error } = useApi<UserProfile>('/users/me')
if (loading) return <LoadingSpinner className="py-20" />
+ if (error) return <ErrorState title="Failed to load profile" description="Please try again." />
const currentPlan = (profile?.plan?.toLowerCase() ?? 'free') as PlanTierYou'll also need to import ErrorState:
import { ErrorState } from '@/components/shared/ErrorState'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/app/`(dashboard)/team/page.tsx around lines 11 - 15, The
component TeamOverviewPage currently defaults currentPlan to 'free' when profile
is null; instead destructure error from useApi (e.g., const { data: profile,
loading, error } = useApi<UserProfile>('/users/me')) and, when error is truthy,
render the ErrorState component (import ErrorState from
'@/components/shared/ErrorState') before computing currentPlan so API failures
are surfaced rather than silently falling back to 'free'; keep the existing
loading path for loading and only compute currentPlan after confirming profile
exists.
| <button | ||
| onClick={() => signOut()} | ||
| className="rounded-[0.5rem] border border-[var(--color-border)] px-3 py-1.5 text-[12px] text-[var(--color-body)] transition-all hover:border-[var(--color-border-hover)] hover:text-[var(--color-text)]" | ||
| > | ||
| Sign out | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add type="button" to the sign-out button.
Although not currently inside a form, adding type="button" is a defensive best practice to prevent unintended form submission behavior if the component structure changes.
Suggested fix
<button
+ type="button"
onClick={() => signOut()}
className="rounded-[0.5rem] border border-[var(--color-border)] px-3 py-1.5 text-[12px] text-[var(--color-body)] transition-all hover:border-[var(--color-border-hover)] hover:text-[var(--color-text)]"
>
Sign out
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={() => signOut()} | |
| className="rounded-[0.5rem] border border-[var(--color-border)] px-3 py-1.5 text-[12px] text-[var(--color-body)] transition-all hover:border-[var(--color-border-hover)] hover:text-[var(--color-text)]" | |
| > | |
| Sign out | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => signOut()} | |
| className="rounded-[0.5rem] border border-[var(--color-border)] px-3 py-1.5 text-[12px] text-[var(--color-body)] transition-all hover:border-[var(--color-border-hover)] hover:text-[var(--color-text)]" | |
| > | |
| Sign out | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/src/components/layout/DashboardLayout.tsx` around lines 152 -
157, The Sign out button element should explicitly declare type to avoid
accidental form submissions; update the JSX for the button that calls signOut
(the onClick={() => signOut()} button in DashboardLayout component) to include
type="button" so it remains a non-submitting control even if later wrapped in a
form.
| useApiMock.mockImplementation((url: string | null) => { | ||
| if (url === '/brains') return withData([{ id: 'b1' }]) | ||
| return withData([]) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten useApi mocks to assert expected endpoint contracts.
Current mocks return fallback data for any non-/brains URL, so a wrong API path can still pass tests. Make the mock explicit per URL and fail on unexpected inputs.
Suggested hardening pattern
- useApiMock.mockImplementation((url: string | null) => {
- if (url === '/brains') return withData([{ id: 'b1' }])
- return withData(real)
- })
+ useApiMock.mockImplementation((url: string | null) => {
+ if (url === '/brains') return withData([{ id: 'b1' }])
+ if (url === '/brains/b1/meta-rules') return withData(real)
+ throw new Error(`Unexpected useApi url: ${url}`)
+ })Apply the same strict URL branching in the other tests in this file.
Also applies to: 30-33, 39-42, 65-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cloud/dashboard/tests/MetaRulesGrid.test.tsx` around lines 19 - 22, The
useApi mock in the tests currently returns fallback data for any non-'/brains'
URL which masks incorrect endpoint usage; update the mockImplementation for
useApiMock to explicitly branch on each expected URL (e.g., '/brains', other
endpoints used in this test) and return withData(...) for each expected path,
and throw an error (or fail) for any unexpected url to ensure tests assert the
endpoint contract; apply the same strict URL-branching pattern to the other
useApiMock occurrences in this file (the blocks around lines that currently
return fallback []) so each test verifies exact API paths.
1 participant
Summary
Complete rebuild of the Gradata cloud platform surface — dashboard migration, 17 routes, 11 sim-validated widgets, 3 new backend endpoints, Stripe SKU wiring, Cloudflare Pages config, end-to-end observability. Zero deferrals.
Built against:
03-dashboard-v3-complete.html)brain/stress_test_results/(S101-103, WAVE2, SIM_A/B) — applied as sim-validated revisionsDashboard migration (
cloud/dashboard/)useAuth/useApihooks,lib/{api,supabase,utils}.ts,types/api.tsSim-validated widgets (11)
All widgets fetch real data from backend endpoints and fall back to fixtures gracefully when brain has no events (cold start).
Backend additions (
cloud/app/)3 new routes + 10 new tests:
GET /brains/{id}/meta-rules— synthesized principles, no raw correction contentGET /brains/{id}/activity— learning events feed filtered to visible kindsGET /brains/{id}/rule-patches+POST .../rollback— self-healing audit trail with append-only historyStripe SKU wiring (+14 tests):
CheckoutRequest.planis now aPlanTierenum (cloud/team/enterprise)GRADATA_STRIPE_PRICE_ID_CLOUD/_TEAMsettings (was inlineos.environ)POST /billing/portalendpoint for Stripe customer portalcustomer.subscription.{created,updated,deleted},invoice.payment_failed,checkout.session.completedBackend tests: 72/72 passing (was 58 + 14 new billing tests)
Deploy + ops
wrangler.toml,public/_headers(HSTS, CSP allowing Supabase + Sentry + Google Fonts, immutable cache for_next/static/*),public/_redirects(legacy/brains/*→/brain?id=...),.nvmrc(Node 20)cloud/OPS.md— single-page operations indexcloud/SUPABASE-SETUP.md— Google OAuth provider + email templates + RLScloud/RAILWAY-ENV.md— complete backend env var list with new Stripe varscloud/SENTRY-SETUP.md— error tracking setup (landed earlier)cloud/dashboard/CLOUDFLARE-PAGES.md— updated with new build command + env var listcloud/dashboard/.env.example— documented env var templateBuild + test status
/,/login,/dashboardall respond 200 with correct contentSim deltas applied (vs stale design spec)
Known runbooks to execute (human action required)
SUPABASE-SETUP.mdRAILWAY-ENV.mdapp.gradata.aiin Cloudflare Pages dashboardWhat's still deferred
Truly nothing from the S105 dashboard scope. Marketing site SEO migration (
gradata.ai→ Next.js) is its own separate project in.tmp/website/and requires its own PR.Generated with Gradata