You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request contains the user-facing system corrections surfaced by qa-intel in the main app, prioritizing persistence validation, details formatting, stream timeouts, and INP fixes.
Summary by Sourcery
Harden analysis persistence, streaming, and chat flows while tightening QA/Intel and infra integrations across the app.
Bug Fixes:
Gate analysis persistence through WorkflowConductor with stricter schema validation for chunked and full UCIS payloads, safer markdown reconstruction, and structured error handling.
Improve chat SSE handling with typed events, robust timeout and abort detection, and centralized error reporting/state transitions to avoid noisy failures and stale UI state.
Fix transcript handling by moving fetching and validation into the streaming worker with early status/error frames instead of pre-barring requests at the edge.
Correct Atlas auth logging, Stripe webhook persistence (failing on insert errors), and admin traffic bypass so only valid configured admin emails are exempt from limits.
Resolve minor correctness issues across UI and utilities, including safer ANSI parsing, robust cookie typing, non-async Next.js redirects/rewrites, and removing noisy or misleading logs.
Enhancements:
Introduce WorkflowConductor and workflow schemas to formalize single-video, cross-analysis, and persist flows, plus add contract tests to guard these boundaries.
Strengthen JSON payload extraction/repair and chunk schema validation in the worker so partial or truncated UCIS payloads are either safely healed or rejected.
Improve dashboard UX and accessibility with better dimension placeholders, clipboard error reporting, ARIA roles on toasts and skeletons, and non-blocking analysis triggers via startTransition.
Tighten key usage and rendering in console components (graphs, logs, chat options) and simplify no-op billing adapters for S2S flows.
Refine quality-engine and qa-intel tooling by making ts-morph loading resilient to unhoisted installs, changing diff mode to compare against origin/main, and documenting the QA-intel audit and review matrix.
Build:
Centralize the ajv version override in the root package.json and remove the workspace-level override to satisfy ESLint and tooling constraints.
CI:
Ensure all CI workflows fetch full git history for qa-intel diff scans by setting checkout fetch-depth to 0 across jobs.
Documentation:
Add QA-intel review matrix and final audit docs capturing the quality engine behavior, performance characteristics, and resolution status for static-analysis findings.
Tests:
Add WorkflowConductor contract tests to validate workflow schemas, scopes, and result typing for single-video, cross-analysis, and persist paths.
Summary by cubic
Locks down persistence and streaming end-to-end: WorkflowConductor gates writes, validates chunk/full payloads, safely reconstructs markdown on edge/worker, and starts streams faster with immediate SSE headers. Also tightens INP/accessibility, adds Supabase persistence trace telemetry, and seeds hybrid search with sparse vectors while finalizing qa-intel docs and CI diff scans with resilient ts-morph loading.
New Features
Supabase persistence trace logs for stub create/update lifecycle to diagnose INP delays.
Added generateSparseVector for hybrid search; embed pipeline imports it for sparse scoring.
This pull request has been ignored for the connected project adnmbikaqnxivalqoild because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.
Introduces a WorkflowConductor and workflow schemas to gate analysis persistence and cross‑analysis flows, hardens streaming/chat and JSON/markdown handling, and applies INP/accessibility, billing/webhook, CI, and minor UX robustness fixes across the app and worker.
Sequence diagram for persist POST flow via WorkflowConductor
sequenceDiagram
participant Client
participant AnalysesPersistRoute as AnalysesPersistRoute
participant WorkflowConductor as WorkflowConductor
participant SupabasePersistenceAdapter as SupabasePersistenceAdapter
Client->>AnalysesPersistRoute: POST /api/analyses/persist
AnalysesPersistRoute->>WorkflowConductor: routeToRoom(persist, handler)
WorkflowConductor-->>AnalysesPersistRoute: roomResult (success | error)
alt success path
AnalysesPersistRoute->>SupabasePersistenceAdapter: findAnalysisForPersist
SupabasePersistenceAdapter-->>AnalysesPersistRoute: row
AnalysesPersistRoute->>SupabasePersistenceAdapter: persistAnalysisChunk / updateAnalysisResult
SupabasePersistenceAdapter-->>AnalysesPersistRoute: result
AnalysesPersistRoute-->>Client: 200 { ok: true, ... }
else error from handler or conductor
AnalysesPersistRoute-->>Client: error JSON (status from roomResult)
end
Loading
File-Level Changes
Change
Details
Files
Gate analysis persistence and workflows through the new WorkflowConductor and updated persist route.
Add WorkflowConductor service with typed WorkflowContext/WorkflowResult and routeToRoom/executeSingleVideo/executeCrossAnalysis helpers using new workflow Zod schemas.
Update analyses persist API route to execute all logic inside WorkflowConductor.routeToRoom, returning typed room results instead of direct NextResponse branches.
Keep chunked vs full UCIS payload validation, chunk stitching, caching, and validation task publishing but now surfaced as room result variants (ok/chunk_saved/interrupted/error).
Strengthen JSON/markdown extraction and persistence contracts in worker services.
Extend MarkdownReconstructor with bounded JSON repair helpers (trackStringState, trackBracketState, repairUnclosedJson, safeParse, sanitizePersona) and more robust extractJsonPayload that logs to Sentry and gracefully handles partial/bounded JSON blocks.
Introduce ChunkPayloadSchema in ZodSchemas and switch PersistService to choose UCISPayloadSchema vs ChunkPayloadSchema based on chunkIndex for both persist and settleAnalysis paths.
Pass chunk index/total through PersistService timeout fallback and edge/worker paths so chunk metadata is preserved during timeouts.
Harden chat streaming, SSE handling, and error mapping in the chat store and worker.
Refactor readSSE to normalize CRLF, skip malformed JSON frames, enforce a 25s timeout via AbortError semantics, and keep a clean finally block.
Introduce typed ChatSSEEvent union and VALID_PERSIST_STATUSES, and route SSE frames through a type‑safe handler map instead of ad‑hoc switches.
Centralize chat stream error handling (getErrorConfig/handleChatStreamError), ensure AbortErrors do not spam Sentry, and keep outbox entries while marking persist state aborted/failed.
Add immediate status frame and asynchronous transcript fetching in buildStreamResponse, with early error SSE when no transcript is available, and move transcript fetching responsibility from the HTTP handler into the streaming pipeline.
Improve dashboard, console, and marketing UI for INP, accessibility, and UX resilience.
Wrap analyze/reanalyze flows in React startTransition and simplify handlers to avoid blocking the main thread; add a dimension‑selection empty state in SelectedDimensionReadout.
Make dashboard clipboard/export flows safer by using explicit anchors, centralized reportClipboardError with Sentry context, and ARIA‑correct toast notifications (role/aria-live).
Clean up dimension content and markdown stripping without brittle regexes; normalize aria-hidden usage and stabilize list keys across multiple components (ProcessingLog, ChatDock, MindMap, StreamingGrid, FaqAccordion, AnalysisHero, KnowledgeGraphCanvas).
Use safe markdown reconstruction in edge analyses GET route with payload size caps and error handling, and fall back gracefully when only payload is present.
Tighten billing, traffic, Stripe/embed webhooks, and environment handling.
Change SupabaseBillingAdapter and PostgresBillingAdapter methods to be explicit no‑ops or to use unknown in catches with structured Sentry logging and clearer error messages.
Enforce stricter admin bypass via isValidAdminEmail helper and extend embed webhook placeholder detection to treat 'mock' values as placeholders for both URL and token.
Make Stripe webhook event insertions fail‑fast by checking supabase insert errors and throwing on failure instead of logging and continuing.
Adjust env helpers to use Boolean(process.env.VERCEL), and minor refactors to Supabase client cookie typing and JSON/ANSI helpers.
Stabilize quality-engine tooling, CI, and dependency overrides for qa-intel.
Guard ts-morph usage in verify-quality-engine.ts via dynamic import and exit(0) when unavailable, and change diff mode to compare against origin/main.
Ensure all CI jobs fetch full git history (fetch-depth: 0) so diff scans function, and document the qa-intel workflow and final audit in new markdown docs.
Simplify Next.js config (non‑async redirects/rewrites), remove local AJV override from pnpm-workspace.yaml, and move the AJV override to the root package.json alongside undici.
Add a detailed workflow-conductor contract test suite exercising Path A/B schemas, WorkflowResult, and persist gate constraints.
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
Reviewing files that changed from the base of the PR and between 6fc3267 and 9405130.
📒 Files selected for processing (15)
.memory/AGENT_LEDGER.md
package.json
scripts/verify-quality-engine.ts
web/app/api/analyses/persist/route.ts
web/app/api/search/route.ts
web/app/api/webhooks/embed/route.ts
web/components/containers/DashboardContainer.tsx
web/components/templates/console/ChatDock.tsx
web/hooks/useEagerVideoMetadata.ts
web/hooks/useSSEStream.ts
web/lib/__tests__/workflow-conductor.test.ts
web/lib/adapters/SupabaseAnalysisAdapter.ts
web/lib/embeddings.ts
web/lib/services/WorkflowConductor.ts
web/lib/usecases/CreateAnalysisUseCase.ts
Walkthrough
This PR introduces a WorkflowConductor orchestration service with Zod-validated workflow schemas, adds sparse vector generation for hybrid Upstash vector search, refactors chat SSE streaming with typed events and centralized error handling, strengthens worker JSON repair and chunk-aware persistence, removes DecodoPort from CreateAnalysisUseCase, and applies UI responsiveness fixes via useTransition and deferred setTimeout execution.
Changes
Runtime hardening, workflow orchestration, and UI responsiveness
Layer / File(s)
Summary
Workflow schemas, types, and conductor web/lib/types/workflow.ts, web/lib/types/chat.ts, web/lib/services/WorkflowConductor.ts, web/lib/usecases/CreateAnalysisUseCase.ts, web/lib/__tests__/workflow-conductor.test.ts
Adds WorkflowScopeSchema, PathAInputSchema/OutputSchema, PathBInputSchema/OutputSchema, and ChatSSEEvent union types. Introduces WorkflowConductor with routeToRoom, executeSingleVideo, and executeCrossAnalysis. Removes DecodoPort dependency from CreateAnalysisUseCase. Adds 471-line Vitest contract-verification suite.
Centralizes abort/non-abort classification via ErrorConfig/handleChatStreamError; rewrites deliver() using a typed handlers dispatch map; converts readSSE timeout to throw AbortError on post-timeout completion; adds Sentry capture for renameConversation/deleteConversation failures.
Sparse vector generation and hybrid search web/lib/embeddings.ts, web/app/api/webhooks/embed/route.ts, web/app/api/search/route.ts
Adds SparseVector interface and generateSparseVector with stopword filtering and log-scaled TF weighting. Wires sparseVector into the embed webhook upsert. Fixes search route to pass vector field instead of data string cast.
Adds repairUnclosedJson, safeParse, sanitizePersona to MarkdownReconstructor with Sentry reporting. Adds selectPersistSchema for chunk vs UCIS schema selection. Defers transcript resolution into the stream via fetchTranscriptIfMissing.
Wraps POST handler inside WorkflowConductor.routeToRoom; converts all inline NextResponse returns to typed room results (error/chunk_saved/interrupted/ok); maps results to NextResponse in the outer handler.
API route and adapter hardening web/app/api/analyses/[id]/route.ts, web/app/api/analyses/route.ts, web/app/api/stripe/webhook/route.ts, web/app/api/billing/webhook/route.ts, web/lib/adapters/SupabaseAnalysisAdapter.ts, web/lib/services/traffic.ts
Adds edge-runtime safe markdown reconstruction with byte-size guard. Removes DecodoAdapter singleton. Hardens Stripe webhook insert to throw on failure. Adds billing webhook default case. Adds persistence completion logs to SupabaseAnalysisAdapter. Adds isValidAdminEmail validation guard.
Defers startAnalysis pipeline into setTimeout; moves videoId extraction into setTimeout in useEagerVideoMetadata; splits ChatDock into localInput/input with useTransition; adds SelectedDimensionReadout fallback UI; wraps handleAnalyze/handleReanalyze in startTransition.
Dashboard copy/export and stream adapter cleanup web/components/containers/DashboardContainer.tsx, web/lib/adapters/stream-delta-handler.ts, web/lib/adapters/stream-status-tracker.ts, web/lib/adapters/synthesis-stream-adapter.ts
Adds reportClipboardError helper, rewrites cleanDimensionContent as line-based sanitizer, switches panel exports to anchor elements. Removes debug logging from stream adapters; adds synthesis-stream-adapter default switch branch.
Type safety, cosmetic, and CI changes web/lib/adapters/Supabase*.ts, web/lib/env.ts, web/lib/supabase.ts, web/components/templates/..., web/next.config.ts, .github/workflows/ci-cd.yml, package.json, scripts/verify-quality-engine.ts, docs/...
Converts catch (error: any) to unknown across Supabase adapters; replaces !! with Boolean(); shortens aria-hidden JSX; stabilizes React key props; changes var to const in inline script; adds ajv override; adds fetch-depth: 0 to all CI checkout steps; guards ts-morph import in quality engine.
Estimated code review effort
🎯 5 (Critical) | ⏱️ ~120 minutes
Possibly related PRs
Hex-Tech-Lab/hex-yt-intel#57: Both PRs modify worker/src/services/MarkdownReconstructor.ts JSON extraction and persona/dimensions validation logic.
Hex-Tech-Lab/hex-yt-intel#91: Both PRs update worker/src/services/PersistService.ts and web/app/api/analyses/persist/route.ts with chunk-aware schema selection and quorum/stitching behavior.
Hex-Tech-Lab/hex-yt-intel#92: Both PRs modify useChatStore's persistState handling and SSE persist/abort/failed transition logic.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 28.30% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title is concise and accurately reflects the PR’s qa-intel-driven system corrections in the main app.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch fix/system-corrections-main-app
✨ Simplify code
Create PR with simplified code
Commit simplified code in branch fix/system-corrections-main-app
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.
We reviewed changes in 4ba626a...f70de5c on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`repairUnclosedJson` has a cyclomatic complexity of 13 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
`extractJsonPayload` has a cyclomatic complexity of 16 with "high" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected any. Specify a different type
The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
In repairUnclosedJson, the depth variable is unused and the closers stack can grow without limits for malformed input; consider either removing depth or enforcing a max depth/length and using it consistently to avoid over-appending closing brackets.
The new setTimeout wrappers in handleAnalyze/handleReanalyze can schedule startAnalysis calls after the component unmounts or the URL changes; consider cancelling the timeout on unmount or using startTransition directly in the event handler to avoid stale or redundant calls.
In cleanDimensionContent, the two regexes for stripping dimension headers (one gim, one single /i) overlap and may only remove the first header; consider consolidating into a single, clearly scoped pattern so multi-dimension content is cleaned consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `repairUnclosedJson`, the `depth` variable is unused and the closers stack can grow without limits for malformed input; consider either removing `depth` or enforcing a max depth/length and using it consistently to avoid over-appending closing brackets.
- The new `setTimeout` wrappers in `handleAnalyze`/`handleReanalyze` can schedule `startAnalysis` calls after the component unmounts or the URL changes; consider cancelling the timeout on unmount or using `startTransition` directly in the event handler to avoid stale or redundant calls.
- In `cleanDimensionContent`, the two regexes for stripping dimension headers (one `gim`, one single `/i`) overlap and may only remove the first header; consider consolidating into a single, clearly scoped pattern so multi-dimension content is cleaned consistently.
## Individual Comments### Comment 1
<locationpath="worker/src/services/MarkdownReconstructor.ts"line_range="125-134" />
<code_context>
return lines.join('\n');
}
+function repairUnclosedJson(text: string): string | null {
+ let depth = 0;
+ let inStr = false;
+ let esc = false;
+ const closers: string[] = [];
++ for (const char of text) {
+ if (esc) { esc = false; continue; }
+ if (char === '\\' && inStr) { esc = true; continue; }
+ if (char === '"') { inStr = !inStr; continue; }
+ if (inStr) continue;
+ if (char === '{') { depth++; closers.push('}'); }
+ else if (char === '[') { closers.push(']'); }
+ else if (char === '}') { depth--; closers.pop(); }
+ else if (char === ']') { closers.pop(); }
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Align bracket tracking logic or simplify unused `depth` handling in `repairUnclosedJson`.
`depth` is updated only for `{`/`}`, never read, while `[`/`]` only affect `closers`. This inconsistency also means extra `]`/`}` are silently ignored via `closers.pop()`. Either drop `depth` or use it consistently to enforce balanced braces/brackets (e.g. fail early when it goes negative) so the repair logic is clearer and better defined for malformed input.
Suggested implementation:
```typescriptfunction repairUnclosedJson(text:string):string|null {
let inStr =false;
let esc =false;
const closers:string[] = [];
```If you want stricter handling of malformed input (e.g. failing early on extra `]`/`}` or mismatched pairs), you can:1. Introduce a single `depth` counter that is incremented on `{`/`[` and decremented on `}`/`]`, and check `depth<0` inside the loop to return `null` early.2. Replace the existing `closers.pop()` logic (which currently just ignores extra closers) with a check that ensures the top of the `closers` stack matches the current closing bracket; if it does not, return `null`.3. Keep using `closers` at the end of the function to append any remaining required closing brackets to `text`.These changes would need to be applied in the part of the loop that handles `{`, `[`, `}`, and `]`, which is not fully visible in the provided snippet.</issue_to_address>### Comment 2<location path="worker/src/services/PersistService.ts" line_range="29-37" /><code_context> if (extracted) {- const result = UCISPayloadSchema.safeParse(extracted);+ const isChunk = options.chunkIndex !== undefined;+ const schema = isChunk+ ? z.object({+ schemaVersion: z.literal('2.0'),+ dimensions: z.array(z.any())+ }).passthrough()+ : UCISPayloadSchema;++ const result = schema.safeParse(extracted); if (result.success) { jsonPayload = result.data as unknown as Record<string, unknown>;</code_context><issue_to_address>**suggestion:** Deduplicate the ad‑hoc chunk schema definition to avoid divergence.A nearly identical relaxed chunk schema is also defined in `persistChunk`. Consider extracting a shared `ChunkPayloadSchema` so both call sites stay consistent and future changes only need to be made in one place.Suggested implementation:```typescriptimport { UCISPayloadSchema, ChunkPayloadSchema } from'./ZodSchemas';
import { hmacHex } from'../crypto';
import { z } from'zod';
``````typescriptif (extracted) {
const isChunk =options.chunkIndex!==undefined;
const schema =isChunk?ChunkPayloadSchema:UCISPayloadSchema;
``````typescriptconst extracted =extractJsonPayload(options.finalText);
if (extracted) {
const schema =ChunkPayloadSchema;
const result =schema.safeParse(extracted);
```You will also need to define and export `ChunkPayloadSchema` from `worker/src/services/ZodSchemas.ts` (or wherever `UCISPayloadSchema` lives), for example:```tsexportconst ChunkPayloadSchema =z
.object({
schemaVersion: z.literal('2.0'),
dimensions: z.array(z.any()),
})
.passthrough();
```Make sure this matches the "relaxed" chunk schema currently used in `persistChunk`, and then update `persistChunk` to import and use `ChunkPayloadSchema` instead of its own inline `z.object(...)` definition.</issue_to_address>
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
While this PR successfully addresses several system corrections identified by QA-Intel, it introduces significant technical debt through manual JSON repair logic and redundant state handling. The newly added repairUnclosedJson function in MarkdownReconstructor.ts is not only complex but also contains logic errors regarding bracket matching and duplicates logic found elsewhere in the repository.
Furthermore, the implementation of SSE stream timeouts contradicts the stated requirement to use AbortError exclusively for state settlement by retaining a trailing generic Error throw. There is also a configuration conflict in package.json where dependency overrides are placed in a file explicitly ignored by the workspace settings. Addressing these logic and configuration issues is recommended before merging.
About this PR
The PR introduces complex logic for manual JSON repair and critical state-settlement via AbortErrors, but does not include any accompanying unit tests. Given the fragility of the bracket-balancing logic, automated verification is highly recommended.
There is noticeable duplication in the relaxed Zod schema logic in PersistService.ts and the JSON repair logic in MarkdownReconstructor.ts. This repetition increases the risk of validation drift and creates multiple points of failure for LLM-generated JSON reconstruction.
1 comment outside of the diffweb/store/useChatStore.ts
line 93-94 🟡 MEDIUM RISK
This trailing generic Error throw is redundant and contradicts the requirement to use AbortError for correct UI state settlement. The readSSE function already handles the timeout inside the stream loop (line 68) by throwing a DOMException when timedOut is true. This block should be removed to maintain a consistent error interface.
Test suggestions
Verify 'repairUnclosedJson' correctly balances braces/brackets and closes unclosed strings in malformed JSON.
Verify 'extractJsonPayload' successfully strips invalid persona objects while preserving valid dimensions.
Verify 'PersistService' uses the relaxed schema for chunks and the strict schema for full completions.
Verify 'readSSE' throws a DOMException with name 'AbortError' when the 25s timeout is reached.
Verify 'cleanDimensionContent' regex patterns correctly identify and remove various dimension header formats and code blocks.
Verify that 'handleAnalyze' and 'handleReanalyze' yield the main thread before starting the analysis transition.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify 'repairUnclosedJson' correctly balances braces/brackets and closes unclosed strings in malformed JSON.
2. Verify 'extractJsonPayload' successfully strips invalid persona objects while preserving valid dimensions.
3. Verify 'PersistService' uses the relaxed schema for chunks and the strict schema for full completions.
4. Verify 'readSSE' throws a DOMException with name 'AbortError' when the 25s timeout is reached.
5. Verify 'cleanDimensionContent' regex patterns correctly identify and remove various dimension header formats and code blocks.
6. Verify that 'handleAnalyze' and 'handleReanalyze' yield the main thread before starting the analysis transition.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
The bracket repair logic pops from the closers stack without verifying that the closing character matches the expected type (e.g., matching } with {). For mismatched inputs like {"a": [}, the function will pop the wrong closer and return invalid JSON. Ensure that the character being closed matches the expected character at the top of the stack before popping.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
This logic for repairing unclosed JSON by balancing brackets is duplicated in at least two other locations: worker/src/services/BracketBuffer.ts and web/lib/utils/bracket-buffer.ts. Centralizing this into a shared utility would reduce complexity and ensure consistent behavior across the system.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Adding overrides to package.json contradicts the project rule defined in pnpm-workspace.yaml (line 12), which states that pnpm 11 reads overrides from the workspace file and ignores them in package.json. These overrides should be consolidated in the workspace file to ensure they are applied correctly across the system.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: This regex is missing the g (global) and m (multiline) flags. Unlike the replacement on line 61, this will only strip the first header found at the start of the string, which is inconsistent with the multi-line cleaning intent.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Nitpick: The depth variable is unused. The bracket tracking and repair logic already effectively relies on the closers stack length for state tracking.
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/services/MarkdownReconstructor.ts` around lines 199 - 201, The
catch block in the extractJsonPayload method logs the error using console.debug
but does not capture it with Sentry as required by the coding guidelines. Add a
Sentry.captureException(error) call in the catch block to ensure the error is
properly reported to Sentry, and verify that Sentry is imported at the top of
the MarkdownReconstructor.ts file.
Source: Coding guidelines
🤖 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 @.memory/AGENT_LEDGER.md:
- Line 374: The ledger entry at line 374 inaccurately describes the AJV override
disposition and omits critical worker-layer hardening work. Update the entry to
clarify that the AJV override was moved from pnpm-workspace.yaml to package.json
(overrides.ajv: "^8.17.1"), not simply removed. Additionally, expand the entry
to include documentation of the worker-layer hardening: the repairUnclosedJson
helper added to MarkdownReconstructor.ts and the graceful persona fallback
implemented in extractJsonPayload function. These changes ensure the ledger
accurately reflects the complete scope of the technical debt corrections for
future reference.
In `@web/components/containers/DashboardContainer.tsx`:
- Around line 54-63: The cleanDimensionContent function uses hardcoded newline
characters (\n) in the markdown code-fence stripping regex patterns on line 58,
which fails to handle Windows line endings (CRLF). Update both regex replace
calls to use the platform-agnostic pattern (?:\r?\n) instead of \n: replace the
pattern /^```[a-zA-Z]*\n/ with /^```[a-zA-Z]*(?:\r?\n)/ for the opening fence
and /\n```$/ with /(?:\r?\n)```$/ for the closing fence to support both Unix and
Windows line endings.
In `@web/store/useChatStore.ts`:
- Around line 66-71: The catch block handling the stream operation in
useChatStore.ts is treating the intentional AbortError thrown when timedOut is
true as a generic stream failure. Add a specific check at the beginning of the
catch block to detect if the caught error is an AbortError (by checking
error.name === 'AbortError' or if error instanceof DOMException), and if so,
silently skip error logging and Sentry capture to avoid reporting expected
timeout cancellations as errors. For all other errors, apply the standard error
handling pattern: verify error instanceof Error to safely access error.message,
log with structured tags using the format console.error('[tag]', { message, url
}), and include Sentry error capture with appropriate context.
In `@worker/src/services/MarkdownReconstructor.ts`:
- Around line 136-139: In the MarkdownReconstructor.ts file, the bracket/brace
matching logic needs to validate that closing characters match their expected
openers. When processing closing brackets (the else if conditions for '}' and
']'), instead of blindly calling closers.pop(), first verify that the popped
value matches the current closing character. If there's a mismatch (e.g.,
closing '}' when ']' was expected), handle the error appropriately to prevent
malformed repairs. Additionally, remove the unused depth variable that is
incremented and decremented but never referenced after the loop, as it serves no
purpose in the current logic.
In `@worker/src/services/PersistService.ts`:
- Around line 29-37: The Zod schema definition for chunk payloads (with
schemaVersion literal '2.0', dimensions array, and passthrough) is duplicated
inline in two places: in the conditional schema assignment where isChunk is
checked, and again in the settleAnalysis method around lines 129-132. Extract
this schema to a module-level constant named ChunkPayloadSchema at the top of
the file after imports, then replace the inline z.object definition in the
ternary operator (where schema is assigned based on isChunk) with a reference to
ChunkPayloadSchema, and also replace the duplicate schema definition in the
settleAnalysis method with the same constant to ensure consistency and reduce
duplication.
---
Outside diff comments:
In `@worker/src/services/MarkdownReconstructor.ts`:
- Around line 199-201: The catch block in the extractJsonPayload method logs the
error using console.debug but does not capture it with Sentry as required by the
coding guidelines. Add a Sentry.captureException(error) call in the catch block
to ensure the error is properly reported to Sentry, and verify that Sentry is
imported at the top of the MarkdownReconstructor.ts file.
🪄 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: 52565a1e-2add-4c86-8f76-c86f8e8f8f6f
📥 Commits
Reviewing files that changed from the base of the PR and between 4ba626a and ae778fe.
- [2026-06-20T11:30:00+03:00] [Antigravity (Agent)] [DONE] Copied and saved the comprehensive UI/UX report to docs/specs/ and stored the critique snapshot for /impeccable polish.
- [2026-06-20T20:49:00+03:00] [Antigravity (Agent)] [DONE] Completed Quality Engine verification run with concurrency=22 on 295 files. Optimized script execution by skipping third-party libraries in import parser and refactoring GraphAwareBoundaryRule to run in parallel file scope, reducing execution time from timeouts (>120s) to under 45s. Trace findings saved to task-457.log.
- [2026-06-20T21:28:00+03:00] [Antigravity (Agent)] [DONE] Completed technical debt corrections on branch fix/inp-ajv-and-stream-debt. Fixed AJV/ESLint version crash by removing redundant override in workspace. Wrapped console analysis triggers in startTransition in DashboardContainer.tsx to resolve the 200ms INP block. Hardened readSSE timeout in useChatStore.ts to throw AbortError for correct state settlement. Updated final audit report.
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Clarify the AJV override disposition in the ledger entry.
Line 374 states "removing redundant override in workspace," but per package.json snippet context, the override was moved to package.json (overrides.ajv: "^8.17.1"), not simply removed. Ledger readers should understand the complete action for future reference.
Additionally, the entry omits the significant worker-layer hardening: repairUnclosedJson helper in MarkdownReconstructor.ts and graceful persona fallback in extractJsonPayload. This is a material part of the debt correction and should be documented.
Consider revising the entry to:
Clarify that AJV override was moved from pnpm-workspace.yaml → package.json
Include a brief phrase about worker JSON payload hardening (repair + persona fallback)
🤖 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 @.memory/AGENT_LEDGER.md at line 374, The ledger entry at line 374
inaccurately describes the AJV override disposition and omits critical
worker-layer hardening work. Update the entry to clarify that the AJV override
was moved from pnpm-workspace.yaml to package.json (overrides.ajv: "^8.17.1"),
not simply removed. Additionally, expand the entry to include documentation of
the worker-layer hardening: the repairUnclosedJson helper added to
MarkdownReconstructor.ts and the graceful persona fallback implemented in
extractJsonPayload function. These changes ensure the ledger accurately reflects
the complete scope of the technical debt corrections for future reference.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 11 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`repairUnclosedJson` has a cyclomatic complexity of 15 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`repairUnclosedJson` has a cyclomatic complexity of 15 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable
It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.
The reason will be displayed to describe this comment to others. Learn more.
`repairUnclosedJson` has a cyclomatic complexity of 17 with "high" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
`extractJsonPayload` has a cyclomatic complexity of 21 with "high" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Avoid using console in code that runs on the browser
It is considered a best practice to avoid the use of any console methods in JavaScript code that will run on the browser.
NOTE: If your repository contains a server side project, you can add "nodejs" to the environment property of analyzer meta in .deepsource.toml.
This will prevent this issue from getting raised.
Documentation for the analyzer meta can be found here.
Alternatively, you can silence this issue for your repository as shown here.
If a specific console call is meant to stay for other reasons, you can add a skipcq comment to that line.
This will inform other developers about the reason behind the log's presence, and prevent DeepSource from flagging it.
The reason will be displayed to describe this comment to others. Learn more.
Avoid using console in code that runs on the browser
It is considered a best practice to avoid the use of any console methods in JavaScript code that will run on the browser.
NOTE: If your repository contains a server side project, you can add "nodejs" to the environment property of analyzer meta in .deepsource.toml.
This will prevent this issue from getting raised.
Documentation for the analyzer meta can be found here.
Alternatively, you can silence this issue for your repository as shown here.
If a specific console call is meant to stay for other reasons, you can add a skipcq comment to that line.
This will inform other developers about the reason behind the log's presence, and prevent DeepSource from flagging it.
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
In web/app/api/analyses/persist/route.ts you’re instantiating WorkflowConductor with { } as any, which defeats the type-safety and makes the dependency contract unclear; consider either making CreateAnalysisUseCase optional for routeToRoom use, or introducing a lighter-weight context/trace helper instead of passing a bogus use case instance.
The change in web/lib/env.ts introduces a top-level const isVercel = Boolean(process.env.VERCEL); inside the isProduction getter block in a way that looks mis-indented and may shadow or escape the intended function scope; please re-check that isVercel is declared inside the getter and that you don’t end up with a stray top-level constant or syntax error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments- In `web/app/api/analyses/persist/route.ts` you’re instantiating `WorkflowConductor` with `{ } as any`, which defeats the type-safety and makes the dependency contract unclear; consider either making `CreateAnalysisUseCase` optional for `routeToRoom` use, or introducing a lighter-weight context/trace helper instead of passing a bogus use case instance.
- The change in `web/lib/env.ts` introduces a top-level `const isVercel = Boolean(process.env.VERCEL);` inside the `isProduction` getter block in a way that looks mis-indented and may shadow or escape the intended function scope; please re-check that `isVercel` is declared inside the getter and that you don’t end up with a stray top-level constant or syntax error.
## Individual Comments### Comment 1
<locationpath="worker/src/services/MarkdownReconstructor.ts"line_range="179-188" />
<code_context>
+function safeParse(text: string, phase: string): { parsed: Partial<UCISPayloadV2> | null; repaired: string | null } {
</code_context>
<issue_to_address>
**issue (bug_risk):** Sentry is referenced here but not imported in the worker bundle, which will cause a runtime/compile error.
In the worker runtime, `@sentry/nextjs` isn’t suitable either. Please either import a worker-compatible Sentry client (or a small logging wrapper) for these calls, or remove the Sentry usage to prevent build failures.
</issue_to_address>
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Sentry is referenced here but not imported in the worker bundle, which will cause a runtime/compile error.
In the worker runtime, @sentry/nextjs isn’t suitable either. Please either import a worker-compatible Sentry client (or a small logging wrapper) for these calls, or remove the Sentry usage to prevent build failures.
Regroup these imports to match the repository order.
The new WorkflowConductor import extends an already-mixed block where internal @/ aliases surround third-party imports. Keep framework first, then third-party, then internal aliases, with type-only imports separated.
As per coding guidelines, **/*.{ts,tsx,js,jsx} imports must be grouped with blank lines in order: framework/lib, third-party, internal @/ aliases, and type imports separate from value imports.
🤖 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/analyses/persist/route.ts` around lines 5 - 17, The import block
in the persist route is out of repository order because the new
WorkflowConductor import was added into a mixed set of framework, third-party,
and internal aliases. Reorder the imports in this module so Next.js/framework
imports come first, then third-party packages like zod and `@sentry/nextjs`, then
internal `@/` aliases, with type-only imports separated from value imports; update
the block around the existing verifyContentSig, UCISPayloadV2Schema,
SupabasePersistenceAdapter, and WorkflowConductor imports to match the repo’s
standard grouping.
Show a failure toast when async clipboard writes reject.
The .catch(...) branches only report telemetry. The outer try/catch on Lines 143-160 will not catch these promise rejections, so a denied clipboard write fails silently for the user.
🤖 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/components/containers/DashboardContainer.tsx` around lines 146 - 155,
Async clipboard write failures are only being reported to telemetry and never
surfaced to the user. Update the clipboard handling in DashboardContainer’s copy
actions (the branches using navigator.clipboard.writeText for insights,
knowledge-graph, word-cloud, and mind-map) so each .catch(...) also triggers a
failure toast, and keep reportClipboardError for logging. Since the outer
try/catch won’t catch these promise rejections, handle the rejection inside the
same promise chain or with await so the user gets immediate feedback when the
clipboard write is denied.
Use the repository log shape in reportClipboardError.
Line 56 still logs console.error('[DashboardContainer] Clipboard copy failed:', { message, context }), which does not match the required tagged { message, url } structure and omits the page URL. As per coding guidelines, Log errors with structured tags using format: console.error('[tag]', { message, url }).
🤖 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/components/containers/DashboardContainer.tsx` around lines 55 - 56, In
reportClipboardError inside DashboardContainer, the console error is using the
wrong log shape and is missing the page URL. Update the error logging to follow
the repository’s structured format with a tagged console.error call and a
payload containing message and url, using the current page URL instead of
context. Keep the Sentry.captureException behavior intact while aligning the log
output with the required { message, url } structure.
Source: Coding guidelines
web/store/useChatStore.ts (1)
112-115: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Worker error events still get treated as successful delivery.
handlers.error() only mutates local state, and readSSE() currently catches exceptions from onEvent(...) in the same block as JSON parsing. That means a worker-declared failure cannot abort deliver(), so Line 283 still removes the outbox entry after a failed stream.
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/store/useChatStore.ts` around lines 112 - 115, The worker error path is
being swallowed because readSSE() catches exceptions from onEvent(...) in the
same block as JSON parsing, so handlers.error() cannot abort deliver() and the
outbox entry still gets removed. Update readSSE() to distinguish invalid JSON
frames from handler-thrown failures, and make handlers.error() propagate a
failure signal that deliver() can detect and stop before clearing the message;
use readSSE(), handlers.error(), and deliver() to wire the abort through the
stream flow.
🤖 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 @.github/workflows/ci-cd.yml:
- Line 76: Scope the full-history checkout to only the QA/lint job that actually
needs it: keep fetch-depth: 0 only where scripts/verify-quality-engine.ts
compares against origin/main, and change the other checkout steps in the same
workflow to use the default shallow clone. Update the checkout configuration in
the relevant jobs by locating their uses of fetch-depth in the CI/CD workflow so
the jobs that do not consume git history no longer request full history.
In `@scripts/verify-quality-engine.ts`:
- Around line 93-101: The ts-morph import failure in the verify-quality-engine
flow is being treated as success, which lets the qa-intel step pass without
running analysis. Update the catch around the dynamic TsMorphProject import to
capture and report the error with Sentry and a structured console.error tag
using the caught Error message, then fail the process with a non-zero exit
instead of exiting 0. Keep the behavior scoped to the script’s ts-morph setup so
the CI quality gate cannot silently disappear.
In `@web/app/api/analyses/`[id]/route.ts:
- Around line 11-21: The safeReconstructMarkdown helper is swallowing
reconstruction exceptions and returning an empty string, which masks failures
behind a successful response. Update safeReconstructMarkdown and the
getAnalysisMarkdown call path to catch the original error, capture it in Sentry,
and log it with the route tag using structured console.error output that
includes the error message and request URL before falling back to ''. Keep the
fallback behavior, but ensure any catch in this route follows the coding
guideline and preserves enough context to diagnose failures in
reconstructMarkdown and related payload handling.
- Around line 3-5: The import block in the route module mixes a type-only import
with value imports, which violates the grouping convention. Update the imports
near verifyResourceOwnership and reconstructMarkdown so the internal value
imports stay together, then place UCISPayloadV2 in its own type-only import
group separated by a blank line.
In `@web/app/api/analyses/persist/route.ts`:
- Around line 159-170: Validate the incoming status before calling
persistAnalysisChunk in the chunked write path so only supported values can be
stored in analysis_chunks.status. In the route handler that processes analysis
chunks, replace the unchecked status as any usage with explicit
validation/mapping to the allowed set and reject or normalize unexpected values
before persistence. Keep the fix near the chunk persistence logic in the branch
that uses persistAnalysisChunk and ensure the completion check in the same route
can only be reached with a valid status value.
In `@web/app/api/stripe/webhook/route.ts`:
- Around line 75-79: The `stripe_events` insert in the `route.ts` webhook
handler is happening after `dispatchEvent()`, so a failed write can incorrectly
turn a post-processing logging issue into a webhook failure. Update the webhook
flow around `dispatchEvent()` and the `stripe_events` insert so the insert
failure is treated as non-fatal: log the error from the `insertError` path, but
do not throw or return a 500 once `dispatchEvent()` has already completed. Keep
the dedupe logic tied to `stripe_events`, but make the post-dispatch persistence
best-effort only.
In `@web/components/containers/DashboardContainer.tsx`:
- Around line 167-171: The export logic in DashboardContainer creates object
URLs for both the insights and mind-map downloads but never cleans them up, so
update the branches that use URL.createObjectURL and anchor.click to call
URL.revokeObjectURL on the generated url after the download is triggered. Use
the same cleanup pattern in both export paths so repeated exports do not retain
blob memory for the session.
In `@web/components/templates/console/ChatDock.tsx`:
- Around line 281-282: The suggestion chip list in ChatDock’s options rendering
can receive duplicate values from parseAssistant(), so using key={opt} may
collide and break React reconciliation. Update the options.map render to use a
stable unique key such as the item index combined with the option text, or
dedupe the options before mapping, while keeping the submit(opt) and
disabled={sending} behavior unchanged.
In `@web/components/templates/console/MindMap.tsx`:
- Around line 193-198: The link key generation in MindMap’s layout rendering is
using coordinates, which causes unnecessary remounts when the layout changes.
Update the key inside the layout.links map to use the stable targetId already
present on each link instead of the source/target coordinate string, so each
edge keeps a consistent identity across relayouts.
In `@web/lib/__tests__/workflow-conductor.test.ts`:
- Around line 38-49: Update the validProcessingPayload fixture in
workflow-conductor.test to match the new processing contract by making the
processing transcript empty instead of prefilled text. This keeps the test
aligned with CreateAnalysisUseCase.execute() and ensures the workflow-conductor
suite still verifies the worker-first behavior rather than assuming eager
transcript delivery.
In `@web/lib/services/traffic.ts`:
- Line 75: The admin email bypass check in traffic.ts is comparing userEmail
against the raw ADMIN_EMAIL env value, so a trimmed-but-unmodified configured
email can still fail to match. Update the comparison in the traffic service
logic to use the normalized admin email value consistently, reusing the same
trimmed/validated form from isValidAdminEmail() before calling toLowerCase(), so
the ADMIN_EMAIL bypass works even when the env value has surrounding whitespace.
In `@web/lib/services/WorkflowConductor.ts`:
- Around line 16-20: Move the Sentry capture into the catch blocks before any
flush/cleanup happens so the workflow exception is included in the batch, and
update handleWorkflowError in WorkflowConductor to use Sentry.captureException
with contexts instead of tags. Keep the existing workflow/traceId metadata, but
pass it through the required contexts shape and ensure the route handlers that
call handleWorkflowError no longer flush before the exception is captured.
In `@web/lib/types/workflow.ts`:
- Around line 6-14: `PathAInputSchema` currently allows `url` to be omitted,
which lets invalid single-video requests reach `executeSingleVideo()` and fail
later. Update the schema in `PathAInputSchema` so `url` is required (keep the
URL validation), and ensure any callers relying on this input are aligned with
the required field.
In `@web/lib/usecases/AggregateGlobalGraphUseCase.ts`:
- Line 4: The execute() method in AggregateGlobalGraphUseCase currently returns
a raw KnowledgeGraph, which breaks the use-case contract used under
web/lib/usecases. Update the AggregateGlobalGraphUseCase.execute signature and
return path to emit a discriminated union result object with a type field using
cache_hit, processing, or error, and include the graph data inside the
appropriate variant instead of returning it directly. Make sure the result shape
matches the repo’s use-case pattern so callers can handle the outcome
consistently.
In `@worker/src/routes/analysis.ts`:
- Around line 242-245: The transcript-resolution failure path in the analysis
stream exits before the existing try/finally can record a terminal status, so
failed lookups are never persisted. In the analysis route handler, update the
branch that checks resolvedTranscript to persist an interrupted/failed analysis
state before calling controller.close() and returning, using the same
persistence mechanism used later in the flow. Make sure the terminal record is
written even when resolvedTranscript is empty or contains the known failure
messages, and keep the send({ type: "error" ... }) behavior intact.
- Around line 189-195: The timeout cleanup in the persist flow only happens on
the fulfillment path, so a rejected persist can still leave the timer active and
trigger a duplicate failed persist later. Update the analysis route’s persist
handling around the `persistService.persist(...)`, `timeoutPromise`, and
`Promise.race` logic so `clearTimeout(timeoutId)` runs for both resolved and
rejected attempts, using a shared cleanup path (for example in a `finally`-style
branch) to ensure the timer is always cleared.
---
Outside diff comments:
In `@web/app/api/analyses/persist/route.ts`:
- Around line 5-17: The import block in the persist route is out of repository
order because the new WorkflowConductor import was added into a mixed set of
framework, third-party, and internal aliases. Reorder the imports in this module
so Next.js/framework imports come first, then third-party packages like zod and
`@sentry/nextjs`, then internal `@/` aliases, with type-only imports separated from
value imports; update the block around the existing verifyContentSig,
UCISPayloadV2Schema, SupabasePersistenceAdapter, and WorkflowConductor imports
to match the repo’s standard grouping.
---
Duplicate comments:
In `@web/components/containers/DashboardContainer.tsx`:
- Around line 146-155: Async clipboard write failures are only being reported to
telemetry and never surfaced to the user. Update the clipboard handling in
DashboardContainer’s copy actions (the branches using
navigator.clipboard.writeText for insights, knowledge-graph, word-cloud, and
mind-map) so each .catch(...) also triggers a failure toast, and keep
reportClipboardError for logging. Since the outer try/catch won’t catch these
promise rejections, handle the rejection inside the same promise chain or with
await so the user gets immediate feedback when the clipboard write is denied.
- Around line 55-56: In reportClipboardError inside DashboardContainer, the
console error is using the wrong log shape and is missing the page URL. Update
the error logging to follow the repository’s structured format with a tagged
console.error call and a payload containing message and url, using the current
page URL instead of context. Keep the Sentry.captureException behavior intact
while aligning the log output with the required { message, url } structure.
In `@web/store/useChatStore.ts`:
- Around line 112-115: The worker error path is being swallowed because
readSSE() catches exceptions from onEvent(...) in the same block as JSON
parsing, so handlers.error() cannot abort deliver() and the outbox entry still
gets removed. Update readSSE() to distinguish invalid JSON frames from
handler-thrown failures, and make handlers.error() propagate a failure signal
that deliver() can detect and stop before clearing the message; use readSSE(),
handlers.error(), and deliver() to wire the abort through the stream flow.
🪄 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: 3bc952a9-680c-4a89-a3af-eb754918fb9f
📥 Commits
Reviewing files that changed from the base of the PR and between ee03524 and 6fc3267.
fetch-depth: 0 makes sense for the lint job because scripts/verify-quality-engine.ts compares against origin/main, but these other jobs never consume git history in the shown steps. Pulling the full repo in every one of them adds avoidable checkout time and bandwidth to the pipeline.
Also applies to: 144-144, 198-198, 227-227, 260-260, 309-309, 337-337, 376-376
🤖 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 @.github/workflows/ci-cd.yml at line 76, Scope the full-history checkout to
only the QA/lint job that actually needs it: keep fetch-depth: 0 only where
scripts/verify-quality-engine.ts compares against origin/main, and change the
other checkout steps in the same workflow to use the default shallow clone.
Update the checkout configuration in the relevant jobs by locating their uses of
fetch-depth in the CI/CD workflow so the jobs that do not consume git history no
longer request full history.
UCISPayloadV2 is a type import mixed into the internal value-import block. Keep the internal value imports together, then separate the type-only import with its own blank-line group.
As per coding guidelines, **/*.{ts,tsx,js,jsx} imports must be grouped with blank lines and Type imports separate from value imports.
🤖 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/analyses/`[id]/route.ts around lines 3 - 5, The import block in
the route module mixes a type-only import with value imports, which violates the
grouping convention. Update the imports near verifyResourceOwnership and
reconstructMarkdown so the internal value imports stay together, then place
UCISPayloadV2 in its own type-only import group separated by a blank line.
Don't hide reconstruction failures behind a successful empty response.
This helper swallows every reconstruction exception and returns '', so callers get a 200 with blank analysis_markdown while the failure is only emitted as an unstructured warning. Capture the original error in Sentry and log it with the route tag before falling back.
As per coding guidelines, every catch block must include Sentry error capture and log errors with structured tags using console.error('[tag]', { message, url }).
🤖 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/analyses/`[id]/route.ts around lines 11 - 21, The
safeReconstructMarkdown helper is swallowing reconstruction exceptions and
returning an empty string, which masks failures behind a successful response.
Update safeReconstructMarkdown and the getAnalysisMarkdown call path to catch
the original error, capture it in Sentry, and log it with the route tag using
structured console.error output that includes the error message and request URL
before falling back to ''. Keep the fallback behavior, but ensure any catch in
this route follows the coding guideline and preserves enough context to diagnose
failures in reconstructMarkdown and related payload handling.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate status before writing chunk rows.
status still enters as an unconstrained string and is cast to any here before persistence. Any unexpected value will be stored in analysis_chunks.status, but the completion check on Line 173 only counts 'completed' rows, so a bad status can strand an analysis in the chunked path indefinitely.
Suggested fix
- status: z.string().optional().default('completed'),+ status: z.enum(['completed', 'failed', 'interrupted']).optional().default('completed'),
...
- status: status as any,+ status,
📝 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.
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/analyses/persist/route.ts` around lines 159 - 170, Validate the
incoming status before calling persistAnalysisChunk in the chunked write path so
only supported values can be stored in analysis_chunks.status. In the route
handler that processes analysis chunks, replace the unchecked status as any
usage with explicit validation/mapping to the allowed set and reject or
normalize unexpected values before persistence. Keep the fix near the chunk
persistence logic in the branch that uses persistAnalysisChunk and ensure the
completion check in the same route can only be reached with a valid status
value.
Capture before flushing Sentry, and use contexts here.
Each catch block flushes Sentry before handleWorkflowError() calls captureException(), so the workflow exception is not included in the flushed batch. In short-lived route handlers, that can drop the very failures this conductor is centralizing. This helper also uses tags instead of the required contexts.
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/lib/services/WorkflowConductor.ts` around lines 16 - 20, Move the Sentry
capture into the catch blocks before any flush/cleanup happens so the workflow
exception is included in the batch, and update handleWorkflowError in
WorkflowConductor to use Sentry.captureException with contexts instead of tags.
Keep the existing workflow/traceId metadata, but pass it through the required
contexts shape and ensure the route handlers that call handleWorkflowError no
longer flush before the exception is captured.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require url for the single-video workflow input.
PathAInputSchema is the validation gate used by executeSingleVideo(). Leaving url optional lets malformed single-video requests pass schema validation and fail later inside the use case instead of returning the intended 400.
‼️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.
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/lib/types/workflow.ts` around lines 6 - 14, `PathAInputSchema` currently
allows `url` to be omitted, which lets invalid single-video requests reach
`executeSingleVideo()` and fail later. Update the schema in `PathAInputSchema`
so `url` is required (keep the URL validation), and ensure any callers relying
on this input are aligned with the required field.
Return a use-case result object here, not a raw graph.
This class lives under web/lib/usecases, but execute() now returns a bare KnowledgeGraph synchronously. That drifts from the repo's use-case contract and makes this API inconsistent with the rest of the workflow layer.
As per coding guidelines, **/usecases/**/*.{ts,tsx} use case functions must return discriminated union types with type: 'cache_hit' | 'processing' | 'error' structure.
🤖 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/lib/usecases/AggregateGlobalGraphUseCase.ts` at line 4, The execute()
method in AggregateGlobalGraphUseCase currently returns a raw KnowledgeGraph,
which breaks the use-case contract used under web/lib/usecases. Update the
AggregateGlobalGraphUseCase.execute signature and return path to emit a
discriminated union result object with a type field using cache_hit, processing,
or error, and include the graph data inside the appropriate variant instead of
returning it directly. Make sure the result shape matches the repo’s use-case
pattern so callers can handle the outcome consistently.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear the persist timeout on rejected attempts too.
clearTimeout(timeoutId) only runs when persistService.persist(...) fulfills. If that promise rejects quickly, the 15s timer still fires later, flips settled = true, and queues a second "failed" persist for the same analysis.
‼️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.
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/routes/analysis.ts` around lines 189 - 195, The timeout cleanup in
the persist flow only happens on the fulfillment path, so a rejected persist can
still leave the timer active and trigger a duplicate failed persist later.
Update the analysis route’s persist handling around the
`persistService.persist(...)`, `timeoutPromise`, and `Promise.race` logic so
`clearTimeout(timeoutId)` runs for both resolved and rejected attempts, using a
shared cleanup path (for example in a `finally`-style branch) to ensure the
timer is always cleared.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Persist transcript-resolution failures before closing the stream.
This early return bypasses the later try/finally, so no terminal status is recorded when transcript lookup fails. Because finalText is still empty, atomicPersist.flush() would not help here either; the analysis can be left without a failed/interrupted persistence record.
🤖 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/routes/analysis.ts` around lines 242 - 245, The
transcript-resolution failure path in the analysis stream exits before the
existing try/finally can record a terminal status, so failed lookups are never
persisted. In the analysis route handler, update the branch that checks
resolvedTranscript to persist an interrupted/failed analysis state before
calling controller.close() and returning, using the same persistence mechanism
used later in the flow. Make sure the terminal record is written even when
resolvedTranscript is empty or contains the known failure messages, and keep the
send({ type: "error" ... }) behavior intact.
The reason will be displayed to describe this comment to others. Learn more.
Documentation comment not found for arrow function `handleInputChange`
It is recommended to have documentation comments above, or right inside a function/class declaration.
This helps developers, users and even the author understand the purpose of a code snippet or API function in the future.
NOTE: If you want to stop this issue from getting raised on certain constructs (arrow functions, class expressions, methods etc.), consider using the skip_doc_coverage option under the analyzers.meta property in your .deepsource.toml file.
For example, the following configuration will silence this issue for class expressions and method definitions:
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 10 with "medium" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Function has a cyclomatic complexity of 22 with "high" risk
A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.
The reason will be displayed to describe this comment to others. Learn more.
Found `async` function without any `await` expressions
A function that does not contain any await expressions should not be async (except for some edge cases in TypeScript which are discussed below). Asynchronous functions in JavaScript behave differently than other functions in two important ways:
The reason will be displayed to describe this comment to others. Learn more.
Documentation comment not found for arrow function `checkSettleState`
It is recommended to have documentation comments above, or right inside a function/class declaration.
This helps developers, users and even the author understand the purpose of a code snippet or API function in the future.
NOTE: If you want to stop this issue from getting raised on certain constructs (arrow functions, class expressions, methods etc.), consider using the skip_doc_coverage option under the analyzers.meta property in your .deepsource.toml file.
For example, the following configuration will silence this issue for class expressions and method definitions:
The reason will be displayed to describe this comment to others. Learn more.
Documentation comment not found for arrow function `handleStreamError`
It is recommended to have documentation comments above, or right inside a function/class declaration.
This helps developers, users and even the author understand the purpose of a code snippet or API function in the future.
NOTE: If you want to stop this issue from getting raised on certain constructs (arrow functions, class expressions, methods etc.), consider using the skip_doc_coverage option under the analyzers.meta property in your .deepsource.toml file.
For example, the following configuration will silence this issue for class expressions and method definitions:
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
The reason will be displayed to describe this comment to others. Learn more.
Forbidden non-null assertion
Using non-null assertions cancels out the benefits of strict null-checking, and introduces the possibility of runtime errors. Avoid non-null assertions unless absolutely necessary. If you still need to use one, write a skipcq comment to explain why it is safe.
The reason will be displayed to describe this comment to others. Learn more.
Documentation comment not found for arrow function `stopAnalysis`
It is recommended to have documentation comments above, or right inside a function/class declaration.
This helps developers, users and even the author understand the purpose of a code snippet or API function in the future.
NOTE: If you want to stop this issue from getting raised on certain constructs (arrow functions, class expressions, methods etc.), consider using the skip_doc_coverage option under the analyzers.meta property in your .deepsource.toml file.
For example, the following configuration will silence this issue for class expressions and method definitions:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![deepsource-io[bot]](https://avatars.github.com/in/16372?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.github.com/in/56611?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.github.com/u/9919?s=60&v=4){kind=small}
This pull request contains the user-facing system corrections surfaced by qa-intel in the main app, prioritizing persistence validation, details formatting, stream timeouts, and INP fixes.
Summary by Sourcery
Harden analysis persistence, streaming, and chat flows while tightening QA/Intel and infra integrations across the app.
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests:
Summary by cubic
Locks down persistence and streaming end-to-end:
WorkflowConductorgates writes, validates chunk/full payloads, safely reconstructs markdown on edge/worker, and starts streams faster with immediate SSE headers. Also tightens INP/accessibility, adds Supabase persistence trace telemetry, and seeds hybrid search with sparse vectors while finalizingqa-inteldocs and CI diff scans with resilientts-morphloading.New Features
generateSparseVectorfor hybrid search; embed pipeline imports it for sparse scoring.Bug Fixes
WorkflowConductor; validateChunkPayloadSchema/UCISPayloadSchema; HMAC signatures; structured errors.ChatSSEEvent; unified error mapping and Sentry context.analysis_markdownreconstruction with a 100kB cap; stronger JSON repair; chunk-aware timeout fallback.isValidAdminEmailrejects placeholder/dummy/mock/CI emails; embed webhook treatsmockas placeholder; Stripe webhook fails on event insert errors and ignores unknown types.DecodoAdapter; transcript fetch/validation now happens in the worker during streaming.ts-morphimport guard for unhoisted installs; diff scans againstorigin/main;actions/checkoutusesfetch-depth: 0.vector) to return correct results.Written for commit 9405130. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Performance