feat(ux): synthesis log export, UI context, and edge error hardening - #80
Conversation
… streaming to prevent CLS
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Reviewer's GuideAdds synthesis log export UX, centralizes root landing page into a static marketing component, introduces UI state context and Atlas ingestion flow via SSE, and hardens edge/global graph behavior to avoid crashes while improving video player seek handling and skeleton/loading behavior. Sequence diagram for Atlas SSE ingestion flow from AtlasPagesequenceDiagram
actor User
participant AtlasPage
participant useSSEStream
participant EdgeWorker as analyze-llm-stream
participant Router
User->>AtlasPage: submit form
AtlasPage->>AtlasPage: handleAnalyze
AtlasPage->>AtlasPage: setIsSubmitting(true)
alt videoUrl empty
AtlasPage-->>User: return (no-op)
else videoUrl present
AtlasPage->>useSSEStream: startAnalysis(videoUrl, timeZone)
useSSEStream->>EdgeWorker: POST /analyze-llm-stream
EdgeWorker-->>useSSEStream: SSE synthesis stream
useSSEStream-->>AtlasPage: resolve startAnalysis
AtlasPage->>Router: push(/dashboard)
end
opt error during startAnalysis
useSSEStream-->>AtlasPage: throw error
AtlasPage->>AtlasPage: console.error
AtlasPage->>AtlasPage: setIsSubmitting(false)
end
Sequence diagram for useGlobalGraph fetching hardened atlas global graphsequenceDiagram
participant Dashboard as DashboardContainer
participant useGlobalGraph
participant API as /api/atlas/global-graph
Dashboard->>useGlobalGraph: mount hook
useGlobalGraph->>API: GET
alt auth success and graph ok
API-->>useGlobalGraph: {nodes, edges}
useGlobalGraph-->>Dashboard: set graph data
else unauthorized
API-->>useGlobalGraph: [] status 401
useGlobalGraph-->>Dashboard: error Failed to fetch global knowledge graph
else processing error
API-->>useGlobalGraph: {nodes: [], edges: []}
useGlobalGraph-->>Dashboard: set empty graph
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughSimplifies ChangesConsole UI, Authentication, and API Improvements
Sequence DiagramsequenceDiagram
participant User
participant SigninForm as signin/form.tsx
participant AuthCallback as auth/callback
participant Supabase as Supabase Auth
participant AtlasPage as atlas/page.tsx
User->>SigninForm: Click Sign In
SigninForm->>SigninForm: Compute callbackUrl<br/>(origin + /auth/callback<br/>+ next=pathname)
SigninForm->>Supabase: signInWithOAuth(redirectTo: callbackUrl)
Supabase-->>User: Redirect to provider
User->>AuthCallback: OAuth callback<br/>(code + next params)
AuthCallback->>AuthCallback: Create redirect response early
AuthCallback->>Supabase: exchangeCodeForSession(code)
Supabase-->>AuthCallback: Auth cookies
AuthCallback->>AuthCallback: cookies.setAll<br/>(writes to response)
AuthCallback-->>User: Redirect to next<br/>(default /atlas)
User->>AtlasPage: Load /atlas
AtlasPage->>Supabase: getSession()
Supabase-->>AtlasPage: session + sessionError
AtlasPage->>AtlasPage: Log session status
alt Has Session
AtlasPage-->>User: Render Atlas
else No Session
AtlasPage->>AtlasPage: Log redirect
AtlasPage-->>User: Redirect to /auth/signin
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
analyze-llm-streamthe empty-transcript case was changed from a 400 to a 500 while still representing a client-side issue; consider keeping this as a 4xx (or otherwise aligning with existing retry/alerting semantics) so monitoring and clients don’t treat it as a transient server failure. - The
/api/atlas/global-graphroute now returns[]with a 401, butuseGlobalGraphstill treats any non-OK response as an error; either return a 200 with an empty graph for unauthenticated users or update the hook to handle 401 specifically and surface an empty graph without throwing. - The updated
Skeletoncomponent always interpolatesclassNameinto a template literal, which will render the string "undefined" when no className is passed; consider defaultingclassNameto an empty string or conditionally concatenating it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `analyze-llm-stream` the empty-transcript case was changed from a 400 to a 500 while still representing a client-side issue; consider keeping this as a 4xx (or otherwise aligning with existing retry/alerting semantics) so monitoring and clients don’t treat it as a transient server failure.
- The `/api/atlas/global-graph` route now returns `[]` with a 401, but `useGlobalGraph` still treats any non-OK response as an error; either return a 200 with an empty graph for unauthenticated users or update the hook to handle 401 specifically and surface an empty graph without throwing.
- The updated `Skeleton` component always interpolates `className` into a template literal, which will render the string "undefined" when no className is passed; consider defaulting `className` to an empty string or conditionally concatenating it.
## Individual Comments
### Comment 1
<location path="web/components/ui/skeleton.tsx" line_range="1-3" />
<code_context>
-export function Skeleton() {
+export function Skeleton({ className }: { className?: string }) {
return (
- <div className="animate-pulse space-y-4 p-4 border border-gray-700 rounded-lg">
+ <div className={`animate-pulse space-y-4 p-4 border border-gray-700 rounded-lg ${className}`}>
<div className="h-4 bg-gray-700 rounded w-3/4"></div>
<div className="h-64 bg-gray-700 rounded w-full"></div>
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Passing an undefined `className` will render the literal string "undefined" in the class list.
Since `className` is optional, omitting it will produce `"... rounded-lg undefined"` in the DOM. Consider defaulting it to an empty string or guarding it in the template literal, e.g. `export function Skeleton({ className = '' } ...)` or `className={`... rounded-lg ${className ?? ''}`}`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
worker/src/worker.ts (1)
418-425:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHTTP 500 is semantically incorrect for empty transcript — this is a client input error.
An empty or missing transcript is a validation failure of the client-provided payload, not an internal server error. HTTP 400 (Bad Request) correctly signals that the request cannot be processed due to invalid input, while 500 implies a server-side failure that may be retryable.
This change will:
- Trigger server-error monitoring alerts for what is actually a client-side issue
- Potentially cause clients to retry requests that will never succeed
- Obscure actual server failures in error metrics
Recommended: keep 400 status, improve message if needed
if (!req.transcript || req.transcript.trim().length === 0) { console.error('[analyze-llm-stream] Empty transcript received at edge'); return c.json({ - error: 'Transcript unavailable', + error: 'Transcript unavailable. The video may lack captions or transcript extraction failed.', code: 'ERR_EDGE_EMPTY_SOURCE' - }, 500); + }, 400); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@worker/src/worker.ts` around lines 418 - 425, The empty transcript validation check in the analyze-llm-stream handler is returning HTTP status code 500, which incorrectly signals an internal server error when the actual issue is a client-side validation failure. Change the status code from 500 to 400 (Bad Request) in the json response for the empty transcript check, as HTTP 400 is the semantically correct status for invalid client input rather than server failures, which will prevent false server-error alerts and avoid triggering unnecessary client retries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/api/atlas/global-graph/route.ts`:
- Around line 11-13: The 401 response when authError or !user is true returns an
empty array [] instead of the expected KnowledgeGraph object shape. Change the
return statement in the authError/!user check block to return { nodes: [],
edges: [] } to match the shape returned by the successful response path and
error fallback, ensuring downstream code like the useGlobalGraph hook and
GlobalKnowledgeMap component can properly access the nodes and edges properties
without encountering undefined errors.
In `@web/app/atlas/page.tsx`:
- Around line 22-37: In the handleAnalyze function, setIsSubmitting(false) is
only called in the catch block. If router.push throws an error or navigation is
cancelled, the form will remain permanently disabled. Add a finally block to the
try-catch statement in handleAnalyze and move the setIsSubmitting(false) call
there to ensure the form state is always reset regardless of whether the
operation succeeds or fails.
In `@web/app/page.tsx`:
- Around line 3-13: The RootPage component has `export const dynamic =
'force-dynamic'` which forces server-side rendering on every request, but the
page has no dynamic data dependencies (no API calls, cookies, or
request-specific data). Either remove the dynamic export statement to allow
Next.js static optimization since the landing page should be static, or if
dynamic behavior is actually required, update the comment block above RootPage
to clearly document what dynamic requirements justify this setting.
In `@web/components/templates/console/ProcessingLog.tsx`:
- Around line 27-30: The handleCopy function doesn't handle the Promise returned
by navigator.clipboard.writeText(), which can reject due to permissions or
browser support issues. Make the handleCopy function async, await the
navigator.clipboard.writeText call, and wrap it in a try-catch block to
gracefully handle any failures (such as logging an error message or showing user
feedback when the clipboard write fails).
- Around line 98-99: The copy and download buttons in the ProcessingLog
component lack aria-label attributes, which reduces accessibility for screen
reader users. Although these buttons have title attributes for tooltips, screen
readers need explicit aria-label attributes. Add aria-label="Copy logs" to the
button with handleCopy onClick handler and aria-label="Download MD" to the
button with handleDownload('md') onClick handler to match the accessibility
pattern already used on the collapse button elsewhere in the component.
- Around line 32-46: The handleDownload function has a race condition where
URL.revokeObjectURL is called immediately after a.click(), but since a.click()
can be asynchronous in some browsers, the URL may be revoked before the browser
finishes fetching the Blob, causing silent download failures. Defer the
URL.revokeObjectURL(url) call to execute after a delay (e.g., using setTimeout
with a small timeout value like 100ms) to ensure the browser has sufficient time
to read the Blob before the object URL is revoked.
In `@web/components/ui/skeleton.tsx`:
- Line 3: The className concatenation in the Skeleton component at
web/components/ui/skeleton.tsx line 3 renders the string "undefined" as a CSS
class when the className prop is not provided. To fix this, replace the direct
template literal concatenation of the className prop with the cn utility
function (the standard shadcn/ui pattern for merging class names), which will
safely handle undefined values and prevent semantic noise in the rendered class
attribute.
In `@web/lib/stores/ui-state-context.tsx`:
- Line 1: The file ui-state-context.tsx uses client-side React APIs (useState,
useContext, createContext) but lacks the required 'use client' directive for
Next.js App Router. Add 'use client'; as the very first line of the file, before
any imports, to explicitly mark this as a Client Component and ensure it runs in
the browser environment.
---
Outside diff comments:
In `@worker/src/worker.ts`:
- Around line 418-425: The empty transcript validation check in the
analyze-llm-stream handler is returning HTTP status code 500, which incorrectly
signals an internal server error when the actual issue is a client-side
validation failure. Change the status code from 500 to 400 (Bad Request) in the
json response for the empty transcript check, as HTTP 400 is the semantically
correct status for invalid client input rather than server failures, which will
prevent false server-error alerts and avoid triggering unnecessary client
retries.
🪄 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: 4fc5270a-7008-44b4-b30d-9fdf28369499
📒 Files selected for processing (13)
docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.mddocs/audit/10X_PREFLIGHT_REPORT_2026_06_14.mdweb/app/api/atlas/global-graph/route.tsweb/app/atlas/page.tsxweb/app/page.tsxweb/components/containers/DashboardContainer.tsxweb/components/templates/console/ProcessingLog.tsxweb/components/templates/console/VideoPlayerCard.tsxweb/components/ui/skeleton.tsxweb/hooks/useGlobalGraph.tsweb/lib/stores/ui-state-context.tsxweb/store/useVideoStore.tsworker/src/worker.ts
There was a problem hiding this comment.
16 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/components/containers/DashboardContainer.tsx">
<violation number="1" location="web/components/containers/DashboardContainer.tsx:306">
P2: Skeleton dimension keys are not compatible with final dimension keys. After projection loads, persisted selection can point to a non-existent key and the detail panel loses auto-selection.</violation>
</file>
<file name="web/components/templates/console/ProcessingLog.tsx">
<violation number="1" location="web/components/templates/console/ProcessingLog.tsx:29">
P2: `navigator.clipboard.writeText()` returns a Promise that can reject (permissions denied, non-HTTPS, browser support). Without awaiting or catching it, failures are completely silent with no user feedback.</violation>
<violation number="2" location="web/components/templates/console/ProcessingLog.tsx:45">
P2: `URL.revokeObjectURL(url)` is called synchronously after `a.click()`. Since the browser reads the blob asynchronously, immediate revocation can race with the download fetch, causing silent failures in some browsers. Defer revocation with `setTimeout(() => URL.revokeObjectURL(url), 100)`.</violation>
<violation number="3" location="web/components/templates/console/ProcessingLog.tsx:98">
P2: New icon buttons miss `type="button"` and explicit accessible labels, causing possible accidental form submits and weaker accessibility.</violation>
</file>
<file name="web/app/page.tsx">
<violation number="1" location="web/app/page.tsx:3">
P2: `/` is forced to dynamic rendering despite being a static landing page. This adds avoidable per-request render overhead and prevents static cache optimization.</violation>
</file>
<file name="web/app/atlas/page.tsx">
<violation number="1" location="web/app/atlas/page.tsx:29">
P2: Navigation is blocked by awaiting a long-running stream operation. This keeps users on Atlas until analysis fully settles instead of moving to dashboard when analysis is initiated.</violation>
</file>
<file name="docs/audit/10X_PREFLIGHT_REPORT_2026_06_14.md">
<violation number="1" location="docs/audit/10X_PREFLIGHT_REPORT_2026_06_14.md:5">
P3: Recorded HEAD commit is not resolvable from the repository state. This weakens audit traceability for the report.</violation>
</file>
<file name="web/components/templates/console/VideoPlayerCard.tsx">
<violation number="1" location="web/components/templates/console/VideoPlayerCard.tsx:14">
P3: This change removes the last usage of `jumpToTimestamp`, leaving an unused store action. Dead store APIs add maintenance noise and can mislead future callers.</violation>
</file>
<file name="web/app/api/atlas/global-graph/route.ts">
<violation number="1" location="web/app/api/atlas/global-graph/route.ts:12">
P3: 401 now returns an array instead of the standard `{ error: ... }` object. This creates an inconsistent API contract for unauthorized errors.</violation>
<violation number="2" location="web/app/api/atlas/global-graph/route.ts:12">
P1: Inconsistent response shape on 401: returns `[]` instead of `{ nodes: [], edges: [] }`. The successful path and error fallback both return the `{ nodes, edges }` shape, but the 401 path returns a bare array. If the hook's error handling changes or another consumer reads this response, accessing `.nodes` on `[]` will be `undefined`, breaking downstream components.</violation>
<violation number="3" location="web/app/api/atlas/global-graph/route.ts:25">
P1: Catch path now returns 200, so fetch treats real server failures as successful "no data". This hides production errors and breaks client error handling based on response status.</violation>
</file>
<file name="web/lib/stores/ui-state-context.tsx">
<violation number="1" location="web/lib/stores/ui-state-context.tsx:18">
P2: New UI state context is never consumed, so the feature is effectively inactive dead code in this PR.</violation>
<violation number="2" location="web/lib/stores/ui-state-context.tsx:30">
P2: Provider `value` is not memoized, causing avoidable context-wide rerenders on parent/provider rerenders.</violation>
</file>
<file name="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md">
<violation number="1" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:33">
P2: N18 row is factually outdated: no API GET `route.ts` currently uses `await import()` for Supabase. This can misdirect audit follow-up to the wrong file/status.</violation>
<violation number="2" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:34">
P3: N19 row overstates current risk; tracing include is narrowly scoped in `next.config.ts`. Keeping it red as “broad includes” makes the audit status inaccurate.</violation>
</file>
<file name="web/components/ui/skeleton.tsx">
<violation number="1" location="web/components/ui/skeleton.tsx:3">
P2: When `className` is omitted, the template literal evaluates to `"...rounded-lg undefined"`, rendering the literal string "undefined" as a CSS class in the DOM. Use a fallback like `${className || ''}` or the `cn` utility for safe className merging.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // If projection isn't ready but we're analyzing, show all 11 as idle/streaming skeletons | ||
| if (!nucleus.projection && (status === 'analyzing' || status === 'downloading')) { | ||
| return Array.from({ length: 11 }, (_, i) => ({ | ||
| key: `dim-skeleton-${i + 1}`, |
There was a problem hiding this comment.
P2: Skeleton dimension keys are not compatible with final dimension keys. After projection loads, persisted selection can point to a non-existent key and the detail panel loses auto-selection.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/components/containers/DashboardContainer.tsx, line 306:
<comment>Skeleton dimension keys are not compatible with final dimension keys. After projection loads, persisted selection can point to a non-existent key and the detail panel loses auto-selection.</comment>
<file context>
@@ -302,6 +300,20 @@ export function DashboardContainer({ profile }: DashboardContainerProps) {
+ // If projection isn't ready but we're analyzing, show all 11 as idle/streaming skeletons
+ if (!nucleus.projection && (status === 'analyzing' || status === 'downloading')) {
+ return Array.from({ length: 11 }, (_, i) => ({
+ key: `dim-skeleton-${i + 1}`,
+ label: DIMENSION_LABELS[i + 1] || `Dimension ${i + 1}`,
+ icon: DIMENSION_ICONS[i + 1] || "solar:bolt-linear",
</file context>
| key: `dim-skeleton-${i + 1}`, | |
| key: `dim-${i + 1}`, |
| export function VideoPlayerCard() { | ||
| const playerRef = useRef<any>(null); | ||
| const { isPlaying, setPlaying, seekTo, jumpToTimestamp } = useVideoStore(); | ||
| const { isPlaying, setPlaying, seekTo, clearSeek } = useVideoStore(); |
There was a problem hiding this comment.
P3: This change removes the last usage of jumpToTimestamp, leaving an unused store action. Dead store APIs add maintenance noise and can mislead future callers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/components/templates/console/VideoPlayerCard.tsx, line 14:
<comment>This change removes the last usage of `jumpToTimestamp`, leaving an unused store action. Dead store APIs add maintenance noise and can mislead future callers.</comment>
<file context>
@@ -11,7 +11,7 @@ const Player = dynamic(() => import('react-player'), { ssr: false }) as any;
export function VideoPlayerCard() {
const playerRef = useRef<any>(null);
- const { isPlaying, setPlaying, seekTo, jumpToTimestamp } = useVideoStore();
+ const { isPlaying, setPlaying, seekTo, clearSeek } = useVideoStore();
const videoMetadata = useAnalysisStore((s) => s.videoMetadata);
const nucleusVideoId = useSynthesisNucleus((s) => s.analysis?.videoId);
</file context>
|
|
||
| if (authError || !user) { | ||
| return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); | ||
| return NextResponse.json([], { status: 401 }); |
There was a problem hiding this comment.
P3: 401 now returns an array instead of the standard { error: ... } object. This creates an inconsistent API contract for unauthorized errors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/app/api/atlas/global-graph/route.ts, line 12:
<comment>401 now returns an array instead of the standard `{ error: ... }` object. This creates an inconsistent API contract for unauthorized errors.</comment>
<file context>
@@ -9,7 +9,7 @@ export async function GET() {
if (authError || !user) {
- return NextResponse.json({ error: 'Unauthorized' }, { status: 401 });
+ return NextResponse.json([], { status: 401 });
}
</file context>
| return NextResponse.json([], { status: 401 }); | |
| return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); |
| | **L2** | Snyk scans stale | Monorepo | ✅ | Resolved via package upgrades (June 10). | | ||
| | **L4** | Fragmented prompts | Monorepo | 🟡 | Centralization started in `cascade.ts`. | | ||
| | **N18**| Dynamic import in GET | `route.ts` | 🔴 | Still using `await import()` for Supabase. | | ||
| | **N19**| broad tracing includes | `next.config.ts` | 🔴 | Potentially bloating bundle. | |
There was a problem hiding this comment.
P3: N19 row overstates current risk; tracing include is narrowly scoped in next.config.ts. Keeping it red as “broad includes” makes the audit status inaccurate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md, line 34:
<comment>N19 row overstates current risk; tracing include is narrowly scoped in `next.config.ts`. Keeping it red as “broad includes” makes the audit status inaccurate.</comment>
<file context>
@@ -0,0 +1,44 @@
+| **L2** | Snyk scans stale | Monorepo | ✅ | Resolved via package upgrades (June 10). |
+| **L4** | Fragmented prompts | Monorepo | 🟡 | Centralization started in `cascade.ts`. |
+| **N18**| Dynamic import in GET | `route.ts` | 🔴 | Still using `await import()` for Supabase. |
+| **N19**| broad tracing includes | `next.config.ts` | 🔴 | Potentially bloating bundle. |
+
+## NEW FINDINGS (FROM RECENT WORK)
</file context>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/components/templates/console/VideoPlayerCard.tsx">
<violation number="1" location="web/components/templates/console/VideoPlayerCard.tsx:14">
P3: This change removes the last usage of `jumpToTimestamp`, leaving an unused store action. Dead store APIs add maintenance noise and can mislead future callers.</violation>
</file>
<file name="web/lib/stores/ui-state-context.tsx">
<violation number="1" location="web/lib/stores/ui-state-context.tsx:18">
P2: New UI state context is never consumed, so the feature is effectively inactive dead code in this PR.</violation>
</file>
<file name="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md">
<violation number="1" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:33">
P2: N18 row is factually outdated: no API GET `route.ts` currently uses `await import()` for Supabase. This can misdirect audit follow-up to the wrong file/status.</violation>
<violation number="2" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:34">
P3: N19 row overstates current risk; tracing include is narrowly scoped in `next.config.ts`. Keeping it red as “broad includes” makes the audit status inaccurate.</violation>
</file>
<file name="web/components/containers/DashboardContainer.tsx">
<violation number="1" location="web/components/containers/DashboardContainer.tsx:306">
P2: Skeleton dimension keys are not compatible with final dimension keys. After projection loads, persisted selection can point to a non-existent key and the detail panel loses auto-selection.</violation>
</file>
<file name="web/app/api/atlas/global-graph/route.ts">
<violation number="1" location="web/app/api/atlas/global-graph/route.ts:12">
P3: 401 now returns an array instead of the standard `{ error: ... }` object. This creates an inconsistent API contract for unauthorized errors.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
✅ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/api/atlas/global-graph/route.ts`:
- Around line 22-26: The catch block in the error handling for the global graph
API endpoint is returning an inconsistent response shape. Instead of returning
`{ error: 'Internal server error' }`, it should return `{ nodes: [], edges: []
}` to match the 401 error response shape and align with PR objectives of
preventing dashboard crashes by returning empty graph data. Update the
NextResponse.json call in the catch block to return the empty graph structure
with empty nodes and edges arrays, maintaining consistency with the 401 error
handling path and ensuring the frontend can safely access the graph.nodes
property without encountering undefined.
🪄 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: a85df1cb-f4ec-4dbb-9957-4e8e01ff2732
📒 Files selected for processing (4)
web/app/api/atlas/global-graph/route.tsweb/app/page.tsxweb/components/templates/console/ProcessingLog.tsxworker/src/worker.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/api/atlas/global-graph/route.ts`:
- Around line 22-26: The catch block in the error handling for the global graph
API endpoint is returning an inconsistent response shape. Instead of returning
`{ error: 'Internal server error' }`, it should return `{ nodes: [], edges: []
}` to match the 401 error response shape and align with PR objectives of
preventing dashboard crashes by returning empty graph data. Update the
NextResponse.json call in the catch block to return the empty graph structure
with empty nodes and edges arrays, maintaining consistency with the 401 error
handling path and ensuring the frontend can safely access the graph.nodes
property without encountering undefined.
🪄 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: a85df1cb-f4ec-4dbb-9957-4e8e01ff2732
📒 Files selected for processing (4)
web/app/api/atlas/global-graph/route.tsweb/app/page.tsxweb/components/templates/console/ProcessingLog.tsxworker/src/worker.ts
🛑 Comments failed to post (1)
web/app/api/atlas/global-graph/route.ts (1)
22-26:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win500 response shape inconsistent with PR objectives and 401 fix.
The PR objectives state the goal is to "prevent dashboard crashes when global graph API returns 401 or 500 errors by returning empty graph data instead of errors." The 401 path correctly returns
{ nodes: [], edges: [] }, but the 500 path returns{ error: 'Internal server error' }which lacksnodesandedgesproperties. WhenuseGlobalGraphreceives this response, accessinggraph.nodeswill beundefined, causing the same crash pattern the 401 fix addresses.Proposed fix
} catch (error) { console.error('[atlas/global-graph] Processing error:', error); - // Return error status to allow telemetry to capture the failure - return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); + // Return empty graph to prevent dashboard crashes; status 500 still signals failure for telemetry + return NextResponse.json({ nodes: [], edges: [] }, { status: 500 }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/app/api/atlas/global-graph/route.ts` around lines 22 - 26, The catch block in the error handling for the global graph API endpoint is returning an inconsistent response shape. Instead of returning `{ error: 'Internal server error' }`, it should return `{ nodes: [], edges: [] }` to match the 401 error response shape and align with PR objectives of preventing dashboard crashes by returning empty graph data. Update the NextResponse.json call in the catch block to return the empty graph structure with empty nodes and edges arrays, maintaining consistency with the 401 error handling path and ensuring the frontend can safely access the graph.nodes property without encountering undefined.
|
✅ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/reference/PR_REVIEW_WORKFLOW_V2.md">
<violation number="1" location="docs/reference/PR_REVIEW_WORKFLOW_V2.md:93">
P2: Mandatory trigger line conflicts with ledger-first startup protocol; required order should be explicit.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| --- | ||
|
|
||
| ## ⚠️ MANDATORY TRIGGER | ||
| Invoke `/pr_review_workflow` (which points to this V2 spec) at the start of every session. |
There was a problem hiding this comment.
P2: Mandatory trigger line conflicts with ledger-first startup protocol; required order should be explicit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/reference/PR_REVIEW_WORKFLOW_V2.md, line 93:
<comment>Mandatory trigger line conflicts with ledger-first startup protocol; required order should be explicit.</comment>
<file context>
@@ -0,0 +1,93 @@
+---
+
+## ⚠️ MANDATORY TRIGGER
+Invoke `/pr_review_workflow` (which points to this V2 spec) at the start of every session.
</file context>
| Invoke `/pr_review_workflow` (which points to this V2 spec) at the start of every session. | |
| Invoke `/pr_review_workflow` (which points to this V2 spec) after reading/updating `.memory/AGENT_LEDGER.md` at the start of every session. |
|
✅ Pipeline Status: All checks passed. Ready to merge. |
|
✅ Pipeline Status: All checks passed. Ready to merge. |
|
✅ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/app/auth/signin/form.tsx">
<violation number="1" location="web/app/auth/signin/form.tsx:15">
P1: OAuth `next` param captures sign-in page path instead of the original referrer, causing post-auth redirect back to `/auth/signin`</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
✅ Pipeline Status: All checks passed. Ready to merge. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/auth/callback/route.ts`:
- Around line 8-9: The `next` parameter in the callback route is user-controlled
and needs validation to prevent open redirect attacks. Remove the redundant
decodeURIComponent call since searchParams.get() already returns the decoded
value. Instead, validate that the next parameter is a safe relative path
(starting with / and not containing protocol characters like :) before using it
in the redirect. This prevents attackers from redirecting users to arbitrary
external domains after authentication. Apply this validation logic wherever the
next parameter is processed in the callback handler.
In `@web/app/auth/signin/form.tsx`:
- Line 15: The callbackUrl construction is missing URL encoding for the pathname
parameter. When window.location.pathname contains special characters like
spaces, ampersands, equals signs, or other URL-reserved characters, they will
not be properly escaped in the query string, causing malformed URLs or parameter
truncation. Wrap window.location.pathname with encodeURIComponent() before
inserting it into the next query parameter to ensure special characters are
properly encoded.
In `@web/components/templates/console/ProcessingLog.tsx`:
- Around line 100-101: The handleCopy button (line 100) and handleDownload
button (line 101) in ProcessingLog.tsx are using inline style props which
violates the repo's styling policy. Remove the style prop from both buttons and
replace the inline styles with equivalent Tailwind CSS classes: the transparent
background becomes bg-transparent, the removed border becomes border-none, the
muted text color becomes text-[var(--ink-muted)], and the pointer cursor becomes
cursor-pointer. Alternatively, consider using shadcn/ui Button components with
appropriate variants if they better align with the component library's design
patterns.
🪄 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: e4919097-1372-4c34-851c-5d5a058cc623
📒 Files selected for processing (9)
docs/reference/PR_REVIEW_WORKFLOW_V2.mdweb/app/api/atlas/global-graph/route.tsweb/app/atlas/page.tsxweb/app/auth/callback/route.tsweb/app/auth/signin/form.tsxweb/components/templates/console/ProcessingLog.tsxweb/components/ui/skeleton.tsxweb/lib/stores/ui-state-context.tsxweb/middleware.ts
💤 Files with no reviewable changes (1)
- web/middleware.ts
There was a problem hiding this comment.
3 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/components/templates/console/VideoPlayerCard.tsx">
<violation number="1" location="web/components/templates/console/VideoPlayerCard.tsx:14">
P3: This change removes the last usage of `jumpToTimestamp`, leaving an unused store action. Dead store APIs add maintenance noise and can mislead future callers.</violation>
</file>
<file name="web/lib/stores/ui-state-context.tsx">
<violation number="1" location="web/lib/stores/ui-state-context.tsx:18">
P2: New UI state context is never consumed, so the feature is effectively inactive dead code in this PR.</violation>
</file>
<file name="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md">
<violation number="1" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:33">
P2: N18 row is factually outdated: no API GET `route.ts` currently uses `await import()` for Supabase. This can misdirect audit follow-up to the wrong file/status.</violation>
<violation number="2" location="docs/audit/10X_AUDIT_MASTER_CHECKLIST_2026_06_14.md:34">
P3: N19 row overstates current risk; tracing include is narrowly scoped in `next.config.ts`. Keeping it red as “broad includes” makes the audit status inaccurate.</violation>
</file>
<file name="web/components/containers/DashboardContainer.tsx">
<violation number="1" location="web/components/containers/DashboardContainer.tsx:306">
P2: Skeleton dimension keys are not compatible with final dimension keys. After projection loads, persisted selection can point to a non-existent key and the detail panel loses auto-selection.</violation>
</file>
<file name="web/app/api/atlas/global-graph/route.ts">
<violation number="1" location="web/app/api/atlas/global-graph/route.ts:12">
P3: 401 now returns an array instead of the standard `{ error: ... }` object. This creates an inconsistent API contract for unauthorized errors.</violation>
</file>
<file name="docs/reference/PR_REVIEW_WORKFLOW_V2.md">
<violation number="1" location="docs/reference/PR_REVIEW_WORKFLOW_V2.md:93">
P2: Mandatory trigger line conflicts with ledger-first startup protocol; required order should be explicit.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
✅ Pipeline Status: All checks passed. Ready to merge. |
|
✅ Pipeline Status: All checks passed. Ready to merge. |
…80) * fix: restore landing page and move atlas to authenticated route * fix(ui): restore public landing page, isolate atlas, improve API resilience * feat(ui): implement 11-dimension skeleton loaders and refine markdown streaming to prevent CLS * fix(remediation): fix atlas API route, auth guard, and searchParams typing * fix(ui): hard de-couple root route and restore public landing page * feat(ux): implement synthesis log export, UI state context, and edge error hardening * fix(atlas): remove unused store import * fix(atlas): remove unused analysis store import * fix: zero-trust manual override of P1/P2 violations * fix: zero-trust manual override * chore: add auth debugging * fix(auth): preserve redirect path in oauth callback * fix(auth): enforce strict server-side cookie exchange and path persistence * fix(auth): implement strict same-origin sanitizer for oauth callback * fix(auth): capture correct referrer and handle exchange exceptions --------- Co-authored-by: Kelly Bakri <noreply@github.com>
Implements UX upgrades (log export, UI state management) and stabilizes the edge ingestion pipeline with improved error handling.
Summary by Sourcery
Refine the Atlas and console UX while hardening graph and synthesis handling across the web app and edge worker.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Summary by cubic
Adds synthesis log export, a small UI state context, and safer Atlas/global graph and edge error handling. Restores the public landing page, moves Atlas behind auth, and hardens OAuth with server-side cookie exchange, strict same-origin redirect sanitization, and robust exception handling with path persistence.
New Features
mdorjson.Skeletonand shows 11-dimension placeholders during analysis.Bug Fixes
/api/atlas/global-graph; returns{nodes:[],edges:[]}on 401/500 to avoid crashes;useGlobalGraphupdated.ERR_EDGE_EMPTY_SOURCE) with "Transcript unavailable" when transcript is missing.nextfrom sign-in, strictly sanitizes to same-origin paths (incl. encoded forms), and handles exchange exceptions; addedvitestunit tests.Written for commit e69675f. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements
{ nodes: [], edges: [] }.Documentation