refactor: reconcile hexagonal architecture, segregate ports, extract usecases and remove db leaks - #61
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.github.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @TechHypeXP, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
Warning Review limit reached
More reviews will be available in 23 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
WalkthroughThis PR executes a multi-wave architectural refactoring toward hexagonal/clean architecture patterns. The codebase transitions from monolithic ports (e.g., ChangesHexagonal Architecture Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
✨ Simplify code
|
|
✅ Pipeline Status: All checks passed. Ready to merge. |
![cubic-dev-ai[bot]](https://avatars.github.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 51 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/app/api/chat/persist/route.ts">
<violation number="1" location="web/app/api/chat/persist/route.ts:37">
P1: Raw adapter exceptions can now leak internal error details in API responses. Return a generic 500 message for persistence failures instead of propagating `error.message` to clients.</violation>
</file>
<file name=".github/SECURITY.md">
<violation number="1" location=".github/SECURITY.md:4">
P3: Email wrapped in square brackets renders as literal `[privacy@hex-yt-intel.com]` in standard markdown. Use `<privacy@hex-yt-intel.com>` or a proper `[email](mailto:...)` link for correct rendering.</violation>
</file>
<file name="web/lib/ports/BillingQuotaPort.ts">
<violation number="1" location="web/lib/ports/BillingQuotaPort.ts:15">
P3: Endpoint union is duplicated inline instead of shared type. This increases risk of type drift across ports/adapters when endpoints change.</violation>
</file>
<file name="web/lib/ports/QuotaPort.ts">
<violation number="1" location="web/lib/ports/QuotaPort.ts:1">
P2: Framework leak: `NextResponse` from `next/server` imported in a port interface. Hexagonal architecture requires ports to be framework-agnostic. The adapter layer should construct framework responses; the port should define domain-level result types (status code, headers, body).</violation>
</file>
<file name="web/lib/ports/ChatPersistencePort.ts">
<violation number="1" location="web/lib/ports/ChatPersistencePort.ts:26">
P1: Conversation/message lookups are keyed by `conversationId` only, so ownership checks are not enforced by contract. This can permit cross-user data access if an adapter queries by ID without extra guards.</violation>
</file>
<file name="web/hooks/useSSEStream.ts">
<violation number="1" location="web/hooks/useSSEStream.ts:227">
P1: Aborted runs can leave global `isLoading` stuck true because final cleanup is gated on `!currentSignal.aborted`. This can keep UI actions disabled after navigation/unmount aborts.</violation>
</file>
<file name="web/lib/ports/CryptographicTokenPort.ts">
<violation number="1" location="web/lib/ports/CryptographicTokenPort.ts:1">
P2: Cross-port type coupling reintroduces boundary leakage. Move StreamToken to a neutral shared contract file and import it from both ports.</violation>
</file>
<file name="LICENSE">
<violation number="1" location="LICENSE:25">
P2: Typo in AGPLv3 preamble: "do you know" should be "you know" — the word "do" is erroneously inserted, breaking the standard preamble text.</violation>
<violation number="2" location="LICENSE:47">
P3: LICENSE file is incomplete — after stating the terms "follow", it only provides a URL placeholder. Standard practice is to include the full AGPLv3 license text verbatim so the legal terms are self-contained in the repository.</violation>
</file>
<file name="web/app/api/billing/webhook/route.ts">
<violation number="1" location="web/app/api/billing/webhook/route.ts:25">
P0: Missing await on Paddle webhook unmarshal leaves event as a Promise and skips all subscription handling.</violation>
</file>
<file name="web/lib/ports/PersistencePort.ts">
<violation number="1" location="web/lib/ports/PersistencePort.ts:134">
P3: `status` was added to the port contract but is not consumed by the adapter. This is dead API surface and can drift from actual persisted state semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!secret) { | ||
| throw new Error('PADDLE_WEBHOOK_SECRET is required'); | ||
| } | ||
| event = paddle.webhooks.unmarshal(body, secret, signature); |
There was a problem hiding this comment.
P0: Missing await on Paddle webhook unmarshal leaves event as a Promise and skips all subscription handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/app/api/billing/webhook/route.ts, line 25:
<comment>Missing await on Paddle webhook unmarshal leaves event as a Promise and skips all subscription handling.</comment>
<file context>
@@ -9,35 +10,40 @@ import { getSupabaseClientWithAuth } from '@/lib/supabase';
+ if (!secret) {
+ throw new Error('PADDLE_WEBHOOK_SECRET is required');
+ }
+ event = paddle.webhooks.unmarshal(body, secret, signature);
+ }
+
</file context>
Suggested change
| event = paddle.webhooks.unmarshal(body, secret, signature); | |
| event = await paddle.webhooks.unmarshal(body, secret, signature); |
| } | ||
| if (!conv || conv.user_id !== userId) { | ||
| console.warn('[chat/persist] Conversation/owner mismatch', { conversationId }); | ||
| const conv = await persistenceAdapter.getConversation({ conversationId }); |
There was a problem hiding this comment.
P1: Raw adapter exceptions can now leak internal error details in API responses. Return a generic 500 message for persistence failures instead of propagating error.message to clients.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/app/api/chat/persist/route.ts, line 37:
<comment>Raw adapter exceptions can now leak internal error details in API responses. Return a generic 500 message for persistence failures instead of propagating `error.message` to clients.</comment>
<file context>
@@ -35,34 +31,21 @@ export async function POST(request: NextRequest) {
- }
- if (!conv || conv.user_id !== userId) {
- console.warn('[chat/persist] Conversation/owner mismatch', { conversationId });
+ const conv = await persistenceAdapter.getConversation({ conversationId });
+ if (!conv || conv.userId !== userId) {
+ console.warn('[chat/persist] Conversation/owner mismatch or not found', { conversationId });
</file context>
| @@ -0,0 +1,82 @@ | |||
| import type { ChatConversation, ChatMessage } from '@/lib/types/chat'; | |||
There was a problem hiding this comment.
P1: Conversation/message lookups are keyed by conversationId only, so ownership checks are not enforced by contract. This can permit cross-user data access if an adapter queries by ID without extra guards.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/ports/ChatPersistencePort.ts, line 26:
<comment>Conversation/message lookups are keyed by `conversationId` only, so ownership checks are not enforced by contract. This can permit cross-user data access if an adapter queries by ID without extra guards.</comment>
<file context>
@@ -0,0 +1,82 @@
+ * Retrieve a single conversation.
+ */
+ getConversation(params: {
+ conversationId: string;
+ }): Promise<ChatConversation | null>;
+
</file context>
Comment on lines
+227
to
+229
| if (!currentSignal.aborted) { | ||
| setIsLoading(false); | ||
| } |
There was a problem hiding this comment.
P1: Aborted runs can leave global isLoading stuck true because final cleanup is gated on !currentSignal.aborted. This can keep UI actions disabled after navigation/unmount aborts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/hooks/useSSEStream.ts, line 227:
<comment>Aborted runs can leave global `isLoading` stuck true because final cleanup is gated on `!currentSignal.aborted`. This can keep UI actions disabled after navigation/unmount aborts.</comment>
<file context>
@@ -200,7 +224,9 @@ export function useSSEStream() {
// action button is never left disabled after a partial/interrupted stream,
// regardless of which exit path ran.
- setIsLoading(false);
+ if (!currentSignal.aborted) {
+ setIsLoading(false);
+ }
</file context>
Suggested change
| if (!currentSignal.aborted) { | |
| setIsLoading(false); | |
| } | |
| if (abortControllerRef.current?.signal === currentSignal) { | |
| setIsLoading(false); | |
| } |
| @@ -0,0 +1,11 @@ | |||
| import type { NextResponse } from 'next/server'; | |||
There was a problem hiding this comment.
P2: Framework leak: NextResponse from next/server imported in a port interface. Hexagonal architecture requires ports to be framework-agnostic. The adapter layer should construct framework responses; the port should define domain-level result types (status code, headers, body).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/ports/QuotaPort.ts, line 1:
<comment>Framework leak: `NextResponse` from `next/server` imported in a port interface. Hexagonal architecture requires ports to be framework-agnostic. The adapter layer should construct framework responses; the port should define domain-level result types (status code, headers, body).</comment>
<file context>
@@ -0,0 +1,11 @@
+import type { NextResponse } from 'next/server';
+
+/** Result of a quota gate check. */
</file context>
| have the freedom to distribute copies of free software (and charge for | ||
| them if you wish), that you receive source code or can get it if you | ||
| want it, that you can change the software or use pieces of it in new | ||
| free programs, and that you do you know you can do these things. |
There was a problem hiding this comment.
P2: Typo in AGPLv3 preamble: "do you know" should be "you know" — the word "do" is erroneously inserted, breaking the standard preamble text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At LICENSE, line 25:
<comment>Typo in AGPLv3 preamble: "do you know" should be "you know" — the word "do" is erroneously inserted, breaking the standard preamble text.</comment>
<file context>
@@ -0,0 +1,47 @@
+have the freedom to distribute copies of free software (and charge for
+them if you wish), that you receive source code or can get it if you
+want it, that you can change the software or use pieces of it in new
+free programs, and that you do you know you can do these things.
+
+ Developers that use our General Public Licenses protect your rights
</file context>
| # Security Policy | ||
|
|
||
| ## Reporting Vulnerabilities | ||
| Please report security vulnerabilities to [privacy@hex-yt-intel.com]. |
There was a problem hiding this comment.
P3: Email wrapped in square brackets renders as literal [privacy@hex-yt-intel.com] in standard markdown. Use <privacy@hex-yt-intel.com> or a proper [email](mailto:...) link for correct rendering.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/SECURITY.md, line 4:
<comment>Email wrapped in square brackets renders as literal `[privacy@hex-yt-intel.com]` in standard markdown. Use `<privacy@hex-yt-intel.com>` or a proper `[email](mailto:...)` link for correct rendering.</comment>
<file context>
@@ -0,0 +1,9 @@
+# Security Policy
+
+## Reporting Vulnerabilities
+Please report security vulnerabilities to [privacy@hex-yt-intel.com].
+
+## Policy on Automated Abuse
</file context>
Suggested change
| Please report security vulnerabilities to [privacy@hex-yt-intel.com]. | |
| Please report security vulnerabilities to <privacy@hex-yt-intel.com>. |
| userId: string; | ||
| tier: UserTier; | ||
| email?: string; | ||
| endpoint: 'analyses' | 'search' | 'checkout'; |
There was a problem hiding this comment.
P3: Endpoint union is duplicated inline instead of shared type. This increases risk of type drift across ports/adapters when endpoints change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/ports/BillingQuotaPort.ts, line 15:
<comment>Endpoint union is duplicated inline instead of shared type. This increases risk of type drift across ports/adapters when endpoints change.</comment>
<file context>
@@ -0,0 +1,22 @@
+ userId: string;
+ tier: UserTier;
+ email?: string;
+ endpoint: 'analyses' | 'search' | 'checkout';
+ }): Promise<QuotaGateResult>;
+
</file context>
|
|
||
| TERMS AND CONDITIONS | ||
|
|
||
| [Full AGPLv3 text: https://www.gnu.org/licenses/agpl-3.0.txt] |
There was a problem hiding this comment.
P3: LICENSE file is incomplete — after stating the terms "follow", it only provides a URL placeholder. Standard practice is to include the full AGPLv3 license text verbatim so the legal terms are self-contained in the repository.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At LICENSE, line 47:
<comment>LICENSE file is incomplete — after stating the terms "follow", it only provides a URL placeholder. Standard practice is to include the full AGPLv3 license text verbatim so the legal terms are self-contained in the repository.</comment>
<file context>
@@ -0,0 +1,47 @@
+
+ TERMS AND CONDITIONS
+
+[Full AGPLv3 text: https://www.gnu.org/licenses/agpl-3.0.txt]
</file context>
| payload: any; | ||
| model: string | null; | ||
| validationPassed: boolean; | ||
| status: 'done' | 'interrupted'; |
There was a problem hiding this comment.
P3: status was added to the port contract but is not consumed by the adapter. This is dead API surface and can drift from actual persisted state semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/lib/ports/PersistencePort.ts, line 134:
<comment>`status` was added to the port contract but is not consumed by the adapter. This is dead API surface and can drift from actual persisted state semantics.</comment>
<file context>
@@ -74,4 +74,64 @@ export interface IPersistencePort {
+ payload: any;
+ model: string | null;
+ validationPassed: boolean;
+ status: 'done' | 'interrupted';
+ validationReport: any;
+ }): Promise<void>;
</file context>
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
web/lib/adapters/SettingsModelAdapter.ts (1)
14-22:⚠️ Potential issue | 🟡 MinorFix misleading tier documentation in SettingsModelAdapter (and wire tier cascade when not in trial mode)
web/lib/adapters/SettingsModelAdapter.ts ignores the
_tierparameter (returns Haiku-only in COMMERCIAL_TRIAL_MODE), yet its JSDoc claims “WorkerIngestionAdapter handles tier logic”—but WorkerIngestionAdapter only fetches metadata/transcript and does not resolve models. Tier-based cascade logic exists in web/lib/services/settings.ts (resolveModelCascade(tier, kind)) but isn’t used by SettingsModelAdapter.🤖 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/adapters/SettingsModelAdapter.ts` around lines 14 - 22, SettingsModelAdapter.resolveModels currently ignores the _tier argument and always returns Haiku when COMMERCIAL_TRIAL_MODE or kind === 'chat'; update it to use the existing tier cascade logic by calling resolveModelCascade(_tier, kind) from web/lib/services/settings.ts when not in COMMERCIAL_TRIAL_MODE so model lists honor tier-based fallbacks, while preserving the special COMMERCIAL_TRIAL_MODE behavior; also update the JSDoc on SettingsModelAdapter to stop claiming "WorkerIngestionAdapter handles tier logic" and state that SettingsModelAdapter resolves models via resolveModelCascade unless in trial mode.package.json (1)
19-21: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPin Node.js version to 24.16.0 LTS for consistency.
The coding guidelines require pinning Node.js to version 24.16.0 LTS for CI/deployment. The current
enginesfield specifies>=24.0.0, which allows any 24.x version. While.nvmrcor.node-versionfiles may pin the version for local development, thepackage.jsonshould also reflect this requirement for clarity.📝 Proposed fix
"engines": { - "node": ">=24.0.0", + "node": "24.16.0", "pnpm": ">=9.0.0" }As per coding guidelines: "{.nvmrc,.node-version,*.json}: Pin Node.js to version 24.16.0 LTS for CI/deployment"
🤖 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 `@package.json` around lines 19 - 21, Update the engines entry in package.json to pin Node.js to 24.16.0 LTS instead of a range: replace the "node": ">=24.0.0" value with "24.16.0" (leave "pnpm" as-is), ensuring the engines field explicitly requires Node 24.16.0 for CI/deployment consistency.Source: Coding guidelines
worker/src/services/PromptBuilder.ts (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale interface reference in documentation comment.
The comment references
IPromptBuilder, but the code now implementsPromptBuilderPort(line 13).📝 Proposed fix
-/** - * PromptBuilder - Pure Service (stateless) - * - * Implements IPromptBuilder. Wraps getUCISPrompt — the prompt IP is constructed - * here and bundled into the worker by esbuild, so it never leaves the server. - * Config-only / stateless: safe to share across requests. - */ +/** + * PromptBuilder - Pure Service (stateless) + * + * Implements PromptBuilderPort. Wraps getUCISPrompt — the prompt IP is constructed + * here and bundled into the worker by esbuild, so it never leaves the server. + * Config-only / stateless: safe to share across requests. + */🤖 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/services/PromptBuilder.ts` at line 4, The top-of-file doc comment incorrectly references the old interface name IPromptBuilder; update the comment to reference the current interface PromptBuilderPort and any related identifiers (e.g., the class PromptBuilder and the getUCISPrompt method) so the documentation matches the implementation; ensure wording reflects that PromptBuilder implements PromptBuilderPort rather than IPromptBuilder.worker/src/services/LLMCascade.ts (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale interface reference in documentation comment.
The comment references
ILLMCascade, but the code now implementsLLMCascadePort(line 31).📝 Proposed fix
-/** - * LLMCascade - LLM Transport Adapter (config-only) - * - * Implements ILLMCascade. Owns the OpenRouter multi-model fallback chain and the - * two transport adapters (streaming + non-streaming). Config-only (apiKey): all - * request-scoped state stays in method locals, so it is race-free when shared. - */ +/** + * LLMCascade - LLM Transport Adapter (config-only) + * + * Implements LLMCascadePort. Owns the OpenRouter multi-model fallback chain and the + * two transport adapters (streaming + non-streaming). Config-only (apiKey): all + * request-scoped state stays in method locals, so it is race-free when shared. + */🤖 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/services/LLMCascade.ts` at line 4, Update the stale doc comment that mentions ILLMCascade to reference the current interface LLMCascadePort: find the top-of-file comment in LLMCascade.ts that starts "Implements ILLMCascade" and change the interface name to "LLMCascadePort" so the documentation matches the implemented symbol (LLMCascade class and its implemented interface LLMCascadePort).worker/src/services/TranscriptExtractor.ts (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale interface reference in documentation comment.
The comment references
ITranscriptProvider, but the code now implementsTranscriptProviderPort(line 26).📝 Proposed fix
/** * TranscriptExtractor - Pure Service * * HEXAGONAL ARCHITECTURE: - * - PORT: ITranscriptProvider (fetch(videoId: string): Promise<string>) + * - PORT: TranscriptProviderPort (fetch(videoId: string): Promise<TranscriptResult>) * - ADAPTER: YouTube Caption API + Decodo fallback * - DOMAIN: Caption parsing, language fallback, timeout protection * * Extracts video transcripts from YouTube's timedtext API with: * - English language prioritization * - Fallback to first available language * - 5-second timeout per request * - XML/JSON parsing and reconstruction */🤖 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/services/TranscriptExtractor.ts` at line 5, Update the stale documentation comment that references ITranscriptProvider to the current port name TranscriptProviderPort; locate the docblock above the TranscriptExtractor class (or where the comment appears) and replace the old interface reference with TranscriptProviderPort and, if desired, confirm the documented method/signature matches the actual port method (fetch(videoId: string): Promise<string>).worker/src/services/UpstashCacheAdapter.ts (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale interface reference in documentation comment.
The comment references
IPersistenceRepository, but the code now implementsPersistenceRepositoryPort(line 13).📝 Proposed fix
/** * UpstashCacheAdapter - Persistence Adapter (config-only) * - * Implements IPersistenceRepository. The SOLE place Upstash REST `fetch` calls + * Implements PersistenceRepositoryPort. The SOLE place Upstash REST `fetch` calls * live — core reasoning never touches the Upstash client directly, only this port. * Config-only (url + token): no request-scoped mutable state, safe to share. */🤖 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/services/UpstashCacheAdapter.ts` at line 4, The doc comment at the top of UpstashCacheAdapter.ts incorrectly references IPersistenceRepository; update that comment to reference the current interface name PersistenceRepositoryPort (and optionally mention the implementing class UpstashCacheAdapter) so the documentation matches the actual implementation (class UpstashCacheAdapter implements PersistenceRepositoryPort).worker/src/services/ReasoningEngine.ts (1)
5-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale interface references in documentation comment.
The comment block references the old
I*interface names (IReasoningEngine,IPromptBuilder,ILLMCascade,IPersistenceRepository), but the code now uses the*Portnaming convention (lines 23-26, 42-47).📝 Proposed fix
/** * ReasoningEngine - Thin Orchestrator (Hexagonal-Lite) * * HEXAGONAL ARCHITECTURE: - * - PORT: IReasoningEngine (executeAndStream(context, handlers) / execute(context)) + * - PORT: ReasoningEnginePort (executeAndStream(context, handlers) / execute(context)) * - DEPENDENCIES (constructor DI): - * IPromptBuilder — UCIS v5.1 prompt synthesis (IP stays server-side) - * ILLMCascade — OpenRouter multi-model fallback (streaming + batch) + * PromptBuilderPort — UCIS v5.1 prompt synthesis (IP stays server-side) + * LLMCascadePort — OpenRouter multi-model fallback (streaming + batch) * ValidationService — 11D structure validation - * IPersistenceRepository — Upstash KV cache (optional; insulated behind port) + * PersistenceRepositoryPort — Upstash KV cache (optional; insulated behind port) * * The engine owns ONLY orchestration + the per-stream BracketBuffer parser. It holds no * request-scoped mutable state on shared sub-services — each is stateless/config-only, * and a fresh BracketBuffer is created per stream — so DI is race-free even though * worker.ts constructs the engine per-request. * * BOUNDARY: Accepts domain objects (transcript, metadata), emits domain events * (delta text, dimension fragments). Never touches raw HTTP Request/Response or SSE. * The orchestrator (worker.ts) owns transport; this engine owns reasoning. */🤖 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/services/ReasoningEngine.ts` around lines 5 - 10, Update the stale interface names in the top comment of ReasoningEngine.ts: replace IReasoningEngine with ReasoningEnginePort, IPromptBuilder with PromptBuilderPort, ILLMCascade with LLMCascadePort, and IPersistenceRepository with PersistenceRepositoryPort; also ensure the listed port methods (executeAndStream, execute) and the constructor DI list match the actual symbols used in the file (the class constructor and methods executeAndStream/execute) so the docblock reflects the current *Port naming and method signatures.worker/src/ports/PromptBuilderPort.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the comment header to match the renamed interface.
The comment header still references
IPromptBuilder, but the interface has been renamed toPromptBuilderPort(Line 10).📝 Proposed fix
/** - * IPromptBuilder — Domain Port (Hexagonal-Lite) + * PromptBuilderPort — Domain Port (Hexagonal-Lite) *🤖 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/ports/PromptBuilderPort.ts` at line 2, The comment header at the top still references the old interface name IPromptBuilder but the interface has been renamed to PromptBuilderPort; update the header text to reference PromptBuilderPort (or a neutral description) so it matches the interface name PromptBuilderPort in this file and avoid stale docs.worker/src/ports/LLMCascadePort.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the comment header to match the renamed interface.
The comment header still references
ILLMCascade, but the interface has been renamed toLLMCascadePort(Line 11).📝 Proposed fix
/** - * ILLMCascade — Domain Port (Hexagonal-Lite) + * LLMCascadePort — Domain Port (Hexagonal-Lite) *🤖 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/ports/LLMCascadePort.ts` at line 2, The file header comment still references the old interface name ILLMCascade; update that top comment text to match the renamed interface LLMCascadePort so the header accurately reflects the code (search for the header/comment block and replace occurrences of ILLMCascade with LLMCascadePort, and verify any related comment lines reference LLMCascadePort consistently with the interface declaration).worker/src/ports/ReasoningEnginePort.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the comment header to match the renamed interface.
The comment header still references
IReasoningEngine, but the interface has been renamed toReasoningEnginePort(Line 91).📝 Proposed fix
/** - * IReasoningEngine — Domain Port (Hexagonal-Lite) + * ReasoningEnginePort — Domain Port (Hexagonal-Lite) *🤖 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/ports/ReasoningEnginePort.ts` at line 2, The file header/comment currently references the old interface name IReasoningEngine; update that comment to reference the new interface name ReasoningEnginePort so the header matches the renamed interface (look for the top-of-file comment and the interface declaration named ReasoningEnginePort) — simply replace IReasoningEngine with ReasoningEnginePort in the header text to keep documentation consistent with the code.worker/src/ports/PersistenceRepositoryPort.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale JSDoc reference.
The comment still references the old interface name
IPersistenceRepository, but the interface was renamed toPersistenceRepositoryPorton Line 8.📝 Proposed fix
-/** - * IPersistenceRepository — Domain Port (Hexagonal-Lite) + * PersistenceRepositoryPort — Domain Port (Hexagonal-Lite)🤖 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/ports/PersistenceRepositoryPort.ts` at line 2, The JSDoc header still refers to the old interface name "IPersistenceRepository"; update the comment to reference the current interface name "PersistenceRepositoryPort" so docs match the code — edit the top-of-file JSDoc (the comment above the PersistenceRepositoryPort interface) to replace "IPersistenceRepository" with "PersistenceRepositoryPort" and ensure any surrounding wording stays accurate.worker/src/ports/TranscriptProviderPort.ts (1)
2-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale JSDoc reference.
The comment still references the old interface name
ITranscriptProvider, but the interface was renamed toTranscriptProviderPorton Line 12.📝 Proposed fix
-/** - * ITranscriptProvider — Domain Port (Hexagonal-Lite) + * TranscriptProviderPort — Domain Port (Hexagonal-Lite)🤖 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/ports/TranscriptProviderPort.ts` at line 2, The file's JSDoc still references the old interface name ITranscriptProvider; update that comment to use the current interface name TranscriptProviderPort so the documentation matches the code (search for the JSDoc block at the top that mentions ITranscriptProvider and replace the name with TranscriptProviderPort, ensuring any descriptions remain accurate).web/lib/ports/PersistencePort.ts (1)
37-76: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMove interface documentation to
/docs/directory.The coding guidelines require documentation to be kept in
/docs/, not in code comments. The JSDoc comments throughout this file should be extracted to architecture documentation.🤖 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/ports/PersistencePort.ts` around lines 37 - 76, The file currently contains JSDoc for the PersistencePort interface and its methods (PersistencePort, findCachedAnalysis, upsertProcessingStub, persistAnalysis); extract those block comments into a new architecture doc under /docs/ (e.g., docs/persistence-port.md) preserving the descriptions, parameter semantics, return contracts, and the 8-dimension validation detail, then remove the multi-line JSDoc blocks from web/lib/ports/PersistencePort.ts so the file only exposes the TypeScript interface and method signatures (you may keep a single-line comment if needed), and ensure the new docs reference the exact symbol names (PersistencePort, findCachedAnalysis, upsertProcessingStub, persistAnalysis) so developers can find the implementation.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 `@docs/audit/10X_REAUDIT_IV_COMBINED_2026_06_10.md`:
- Around line 19-26: Update the audit snapshot table entries for "Root Version",
"Web Version", and "Worker Version" from 1.4.1 / 1.4.6 / 1.5.1 to 1.5.2, and
change the "Version Drift" / H6 status to reflect no drift (or explicitly note
the doc is a pre-PR snapshot) so the doc's metrics match the current monorepo
state; edit the markdown table row labels "Root Version", "Web Version", "Worker
Version", "Version Drift" and the H6 status text in the same document to perform
this correction.
In `@docs/specs/IMPLEMENTATION_PLAN.md`:
- Around line 67-137: The document uses absolute file URIs (strings starting
with "file:///home/kellyb_dev/projects/hex-yt-intel/") which breaks portability;
replace every absolute file:///... reference with a relative repo path (e.g.,
change "file:///home/.../web/app/api/billing/webhook/route.ts" to
"web/app/api/billing/webhook/route.ts") throughout IMPLEMENTATION_PLAN.md,
including all entries under Step 1.1, Step 1.2, Step 1.3, Step 2.1, Step 2.2 and
Wave 3 targets; search for the "file:///" prefix and update referenced paths to
their relative equivalents so links and targets like
web/app/api/analyses/route.ts, web/hooks/useSSEStream.ts,
web/lib/ports/IIngestionPort.ts, web/lib/adapters/WorkerIngestionAdapter.ts,
etc., no longer use absolute file URIs.
In `@web/app/api/analyses/persist/route.ts`:
- Around line 67-75: The code loses type safety by casting row.validationReport
to any and mixes snake_case vs camelCase in the JSON blob; define a
ValidationReport interface (including fields like status, transcript_available,
model_used, valid, etc.), replace the any cast so priorReport is typed as
ValidationReport, and update usages around priorReport/newReport (and the
created newReport object) to use the typed properties consistently (preserve
existing snake_case fields if the stored JSON uses them) so TypeScript enforces
correct property names and types when reading/writing validationReport.
In `@web/app/api/analyses/route.ts`:
- Line 66: Replace the unsafe "as any" cast for explicitPersona by ensuring
validation.data.persona is typed/validated as PersonaId: update
AnalysisCreateSchema to declare persona as PersonaId (or perform a runtime type
guard) and then assign explicitPersona = validation.data.persona (or cast to
PersonaId only after validation); reference the explicitPersona assignment and
the AnalysisCreateSchema/PersonaId types to locate and fix the code path where
validation.data.persona is used.
In `@web/app/api/billing/webhook/route.ts`:
- Around line 19-26: The current development-mode fallback parses the body
without verification when secret is missing, which is risky if NODE_ENV is
misconfigured; update the webhook handling in route.ts so that instead of
silently using JSON.parse when secret is falsey and NODE_ENV === 'development'
you either (a) require an explicit opt-in like a new env flag (e.g.,
DEV_ALLOW_UNVERIFIED_WEBHOOKS) and throw/log a clear warning if that flag is not
set, or (b) always throw and log an error unless PADDLE_WEBHOOK_SECRET is
present; specifically modify the block that references secret, NODE_ENV,
paddle.webhooks.unmarshal, signature and event to check for
PADDLE_WEBHOOK_SECRET and the opt-in flag, emit a prominent warning via your
logger when falling back, and avoid silently accepting unverified payloads.
- Around line 30-47: The webhook currently calls
persistenceAdapter.updateUserTier(...) but that method
(SupabasePersistenceAdapter.updateUserTier) returns Promise<void> and can
silently no-op if no rows match; update
SupabasePersistenceAdapter.updateUserTier to perform the update with RETURNING
or inspect the update response (e.g., rowCount/returned rows) and throw a
descriptive error when zero rows were affected, so the webhook route's switch
(subscription.created/updated/canceled) will surface failures instead of
returning processed:true for unknown userIds; ensure the thrown error includes
the userId and requested tier to aid debugging.
In `@web/app/api/chat/conversations/`[id]/messages/route.ts:
- Around line 63-66: The POST handler in route.ts fetches the conversation via
persistenceAdapter.getConversation but never verifies ownership; update the
handler to retrieve the authenticated user's id (e.g., from the request/session
or existing auth helper used in this file) and compare it to the conversation
owner field on conv (e.g., conv.userId or conv.ownerId). If they don't match,
return a 403 JSON response and do not allow inserting the message; keep the
existing 404 check for missing conv and place the ownership check immediately
after fetching conv in the POST route.
- Around line 31-33: The GET handler currently returns messages from
persistenceAdapter.getMessages({ conversationId: id }) without verifying
ownership; update the handler to fetch the conversation (e.g., via
persistenceAdapter.getConversation or getConversations) for the given id and
compare its userId to identity.userId, and if they differ return a
403/NextResponse.json error instead of returning messages; only call
persistenceAdapter.getMessages and return NextResponse.json({ messages }) after
the ownership check passes.
In `@web/lib/adapters/SettingsModelAdapter.ts`:
- Line 6: The COMMERCIAL_TRIAL_MODE constant in SettingsModelAdapter is
hardcoded; remove the private static readonly COMMERCIAL_TRIAL_MODE and instead
read the flag from configuration or a settings service (e.g., inject a Config or
SettingsService into SettingsModelAdapter or import an app config module),
defaulting appropriately if missing; update any references in
SettingsModelAdapter to use the injected/config value and add a unit-testable
fallback so behavior can be controlled via env/config in production and tests.
In `@web/lib/adapters/SupabasePersistenceAdapter.ts`:
- Around line 145-173: The getUserHistory method (and other new methods listed
such as findAnalysisById, updateUserTier, getConversations, createConversation,
getConversation, updateConversationTitle, getMessages, findMessageByClientMsgId,
createMessage, findAssistantMessageAfter, getAnalysisGrounding,
findAnalysisForPersist, updateAnalysisResult) currently only logs errors with
console.error; update each method to also capture exceptions with Sentry (e.g.,
Sentry.captureException(error)) when an error occurs, and include contextual
metadata (method name and relevant identifiers like userId, analysisId,
conversationId) in the capture to improve observability; keep the existing
console.error and thrown error behavior but add the Sentry.captureException call
before rethrowing so failures are reported to Sentry consistently across the
adapter.
- Around line 165-172: The map callbacks use `any` for DB rows; define explicit
Supabase row interfaces (e.g., AnalysisRow, ConversationRow, MessageRow) that
match your table schemas and replace `any` in the mapping callbacks (the
`analysis` parameter in the analyses mapping, and similar parameters in the
conversation and message mapping blocks referenced at 238-246 and 335-342) with
those types; update the return mapping code to use the typed fields (video_id,
created_at, validation_passed, validation_report, etc.) so the compiler enforces
schema correctness across SupabasePersistenceAdapter.ts.
- Line 464: Replace the unsafe cast of validation_report to any by adding a
proper typed shape and a narrow check: define an interface/type like
ValidationReport with an optional status field, change accesses to use a typed
guard (e.g., check typeof data.validation_report === 'object' &&
data.validation_report !== null) or a small helper isValidationReport(obj): obj
is ValidationReport, then read (data.validation_report as
ValidationReport)?.status || 'incomplete'; update the mapping in
SupabasePersistenceAdapter (where data.validation_report is used) to use this
type/guard instead of (data.validation_report as any).
In `@web/lib/ports/ChatPersistencePort.ts`:
- Around line 3-82: Remove the JSDoc block comments from the ChatPersistencePort
interface and its methods (leave the exported interface and all method
signatures intact) and move the explanatory text into the project’s
documentation directory as an architecture doc; create a markdown entry that
documents the ChatPersistencePort interface and each method
(ChatPersistencePort, getConversations, createConversation, getConversation,
updateConversationTitle, getMessages, findMessageByClientMsgId, createMessage,
findAssistantMessageAfter, getAnalysisGrounding) preserving the original
descriptions and request/response shapes so the code remains unchanged except
for removed comments.
In `@web/lib/ports/CryptographicTokenPort.ts`:
- Around line 3-24: Remove the inline JSDoc comments from the
CryptographicTokenPort interface and its methods signAnalysisToken and
signChatToken and place that documentation into the project architecture
documentation (create a new docs entry describing the interface purpose and the
two method contracts: HMAC-signed streaming tokens bound to their respective IDs
and models). Update the code in CryptographicTokenPort to keep only the type
signatures (interface and method params/return types) with no descriptive
comments, and ensure the new docs clearly mirror the removed comments so
consumers can find the same information outside the source file.
In `@web/lib/ports/MetadataIngestionPort.ts`:
- Around line 5-29: Remove the inline JSDoc blocks from the
MetadataIngestionPort interface and its members (the top-level comment and the
comments for fetch, detectPersona, and buildJobMetadata) and instead add an
entry in the project documentation describing the interface responsibilities and
contracts: list MetadataIngestionPort, its methods fetch(videoId: string):
Promise<IngestionResult> (including the error/quotas behavior and that
transcript may be empty), detectPersona(params: {title, channelTitle,
explicitPersona?: PersonaId}): PersonaId, and buildJobMetadata(metadata:
VideoMetadata): AnalysisJobMetadata; keep type references (IngestionResult,
PersonaId, VideoMetadata, AnalysisJobMetadata) in the doc so implementers
understand expectations and remove the corresponding JSDoc comments from the
source.
In `@web/lib/ports/PersistencePort.ts`:
- Around line 111-136: The findAnalysisForPersist and updateAnalysisResult
method signatures use `any` for `validationReport` and `payload`; update them to
use concrete types: replace `validationReport: any` with a proper type such as
`ValidationReportInput` (or `ValidationReportInput | unknown` if you need a safe
fallback) in both the findAnalysisForPersist return type and the
updateAnalysisResult parameter, and change `payload: any` in
updateAnalysisResult to `payload: UCISPayloadV2 | null` (UCISPayloadV2 is
already imported) so both methods use explicit, safer types instead of `any`.
In `@web/lib/usecases/CreateAnalysisUseCase.ts`:
- Around line 25-28: The UseCaseResult type uses `any` for the `data` field
which erases type safety; replace `any` with explicit interfaces for each
discriminated branch (e.g., define CacheHitData, ProcessingData and use those in
the `UseCaseResult` union) so callers of the UseCase (and route handlers) can
rely on concrete shapes; update the exported `UseCaseResult` union to reference
these new types for `data` (and add or tighten optional `headers` types if
needed) and update impacted call sites to conform to the new data shapes (look
for references to UseCaseResult, cache_hit, and processing handling).
---
Outside diff comments:
In `@package.json`:
- Around line 19-21: Update the engines entry in package.json to pin Node.js to
24.16.0 LTS instead of a range: replace the "node": ">=24.0.0" value with
"24.16.0" (leave "pnpm" as-is), ensuring the engines field explicitly requires
Node 24.16.0 for CI/deployment consistency.
In `@web/lib/adapters/SettingsModelAdapter.ts`:
- Around line 14-22: SettingsModelAdapter.resolveModels currently ignores the
_tier argument and always returns Haiku when COMMERCIAL_TRIAL_MODE or kind ===
'chat'; update it to use the existing tier cascade logic by calling
resolveModelCascade(_tier, kind) from web/lib/services/settings.ts when not in
COMMERCIAL_TRIAL_MODE so model lists honor tier-based fallbacks, while
preserving the special COMMERCIAL_TRIAL_MODE behavior; also update the JSDoc on
SettingsModelAdapter to stop claiming "WorkerIngestionAdapter handles tier
logic" and state that SettingsModelAdapter resolves models via
resolveModelCascade unless in trial mode.
In `@web/lib/ports/PersistencePort.ts`:
- Around line 37-76: The file currently contains JSDoc for the PersistencePort
interface and its methods (PersistencePort, findCachedAnalysis,
upsertProcessingStub, persistAnalysis); extract those block comments into a new
architecture doc under /docs/ (e.g., docs/persistence-port.md) preserving the
descriptions, parameter semantics, return contracts, and the 8-dimension
validation detail, then remove the multi-line JSDoc blocks from
web/lib/ports/PersistencePort.ts so the file only exposes the TypeScript
interface and method signatures (you may keep a single-line comment if needed),
and ensure the new docs reference the exact symbol names (PersistencePort,
findCachedAnalysis, upsertProcessingStub, persistAnalysis) so developers can
find the implementation.
In `@worker/src/ports/LLMCascadePort.ts`:
- Line 2: The file header comment still references the old interface name
ILLMCascade; update that top comment text to match the renamed interface
LLMCascadePort so the header accurately reflects the code (search for the
header/comment block and replace occurrences of ILLMCascade with LLMCascadePort,
and verify any related comment lines reference LLMCascadePort consistently with
the interface declaration).
In `@worker/src/ports/PersistenceRepositoryPort.ts`:
- Line 2: The JSDoc header still refers to the old interface name
"IPersistenceRepository"; update the comment to reference the current interface
name "PersistenceRepositoryPort" so docs match the code — edit the top-of-file
JSDoc (the comment above the PersistenceRepositoryPort interface) to replace
"IPersistenceRepository" with "PersistenceRepositoryPort" and ensure any
surrounding wording stays accurate.
In `@worker/src/ports/PromptBuilderPort.ts`:
- Line 2: The comment header at the top still references the old interface name
IPromptBuilder but the interface has been renamed to PromptBuilderPort; update
the header text to reference PromptBuilderPort (or a neutral description) so it
matches the interface name PromptBuilderPort in this file and avoid stale docs.
In `@worker/src/ports/ReasoningEnginePort.ts`:
- Line 2: The file header/comment currently references the old interface name
IReasoningEngine; update that comment to reference the new interface name
ReasoningEnginePort so the header matches the renamed interface (look for the
top-of-file comment and the interface declaration named ReasoningEnginePort) —
simply replace IReasoningEngine with ReasoningEnginePort in the header text to
keep documentation consistent with the code.
In `@worker/src/ports/TranscriptProviderPort.ts`:
- Line 2: The file's JSDoc still references the old interface name
ITranscriptProvider; update that comment to use the current interface name
TranscriptProviderPort so the documentation matches the code (search for the
JSDoc block at the top that mentions ITranscriptProvider and replace the name
with TranscriptProviderPort, ensuring any descriptions remain accurate).
In `@worker/src/services/LLMCascade.ts`:
- Line 4: Update the stale doc comment that mentions ILLMCascade to reference
the current interface LLMCascadePort: find the top-of-file comment in
LLMCascade.ts that starts "Implements ILLMCascade" and change the interface name
to "LLMCascadePort" so the documentation matches the implemented symbol
(LLMCascade class and its implemented interface LLMCascadePort).
In `@worker/src/services/PromptBuilder.ts`:
- Line 4: The top-of-file doc comment incorrectly references the old interface
name IPromptBuilder; update the comment to reference the current interface
PromptBuilderPort and any related identifiers (e.g., the class PromptBuilder and
the getUCISPrompt method) so the documentation matches the implementation;
ensure wording reflects that PromptBuilder implements PromptBuilderPort rather
than IPromptBuilder.
In `@worker/src/services/ReasoningEngine.ts`:
- Around line 5-10: Update the stale interface names in the top comment of
ReasoningEngine.ts: replace IReasoningEngine with ReasoningEnginePort,
IPromptBuilder with PromptBuilderPort, ILLMCascade with LLMCascadePort, and
IPersistenceRepository with PersistenceRepositoryPort; also ensure the listed
port methods (executeAndStream, execute) and the constructor DI list match the
actual symbols used in the file (the class constructor and methods
executeAndStream/execute) so the docblock reflects the current *Port naming and
method signatures.
In `@worker/src/services/TranscriptExtractor.ts`:
- Line 5: Update the stale documentation comment that references
ITranscriptProvider to the current port name TranscriptProviderPort; locate the
docblock above the TranscriptExtractor class (or where the comment appears) and
replace the old interface reference with TranscriptProviderPort and, if desired,
confirm the documented method/signature matches the actual port method
(fetch(videoId: string): Promise<string>).
In `@worker/src/services/UpstashCacheAdapter.ts`:
- Line 4: The doc comment at the top of UpstashCacheAdapter.ts incorrectly
references IPersistenceRepository; update that comment to reference the current
interface name PersistenceRepositoryPort (and optionally mention the
implementing class UpstashCacheAdapter) so the documentation matches the actual
implementation (class UpstashCacheAdapter implements PersistenceRepositoryPort).
🪄 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: 776c5915-bf59-40d3-a424-703e6dbf4f9a
📒 Files selected for processing (51)
.github/CONTRIBUTING.md.github/SECURITY.mdLICENSEdocs/audit/10X_REAUDIT_IV_COMBINED_2026_06_10.mddocs/legal/LICENSE-ADDENDUM.mddocs/legal/NOTICE.mddocs/specs/IMPLEMENTATION_PLAN.mdpackage.jsonweb/app/api/analyses/persist/route.tsweb/app/api/analyses/route.tsweb/app/api/billing/webhook/route.tsweb/app/api/chat/conversations/[id]/messages/route.tsweb/app/api/chat/conversations/route.tsweb/app/api/chat/persist/route.tsweb/app/api/search/route.tsweb/hooks/useSSEStream.tsweb/lib/adapters/PostgresBillingAdapter.tsweb/lib/adapters/RedisTrafficAdapter.tsweb/lib/adapters/SettingsModelAdapter.tsweb/lib/adapters/StreamTokenAdapter.tsweb/lib/adapters/SupabaseAuthAdapter.tsweb/lib/adapters/SupabasePersistenceAdapter.tsweb/lib/adapters/WorkerIngestionAdapter.tsweb/lib/ports/AuthPort.tsweb/lib/ports/BillingQuotaPort.tsweb/lib/ports/ChatPersistencePort.tsweb/lib/ports/CryptographicTokenPort.tsweb/lib/ports/IIngestionPort.tsweb/lib/ports/IQuotaPort.tsweb/lib/ports/IngestionPort.tsweb/lib/ports/MetadataIngestionPort.tsweb/lib/ports/ModelResolutionPort.tsweb/lib/ports/PersistencePort.tsweb/lib/ports/QuotaPort.tsweb/lib/ports/TrafficGuardPort.tsweb/lib/ports/index.tsweb/lib/types/chat.tsweb/lib/usecases/CreateAnalysisUseCase.tsweb/package.jsonworker/package.jsonworker/src/ports/LLMCascadePort.tsworker/src/ports/PersistenceRepositoryPort.tsworker/src/ports/PromptBuilderPort.tsworker/src/ports/ReasoningEnginePort.tsworker/src/ports/TranscriptProviderPort.tsworker/src/services/LLMCascade.tsworker/src/services/PromptBuilder.tsworker/src/services/ReasoningEngine.tsworker/src/services/TranscriptExtractor.tsworker/src/services/UpstashCacheAdapter.tsworker/src/worker.ts
💤 Files with no reviewable changes (2)
- web/lib/ports/IIngestionPort.ts
- web/lib/ports/IQuotaPort.ts
Comment on lines
+19
to
+26
| | **Root Version** | `1.4.1` | `package.json` | | ||
| | **Web Version** | `1.4.6` | `web/package.json` | | ||
| | **Worker Version**| `1.5.1` | `worker/package.json` | | ||
| | **Version Drift** | ⚠️ **CONFIRMED** | Monorepo versions (1.4.1 vs 1.4.6 vs 1.5.1) are out of sync | | ||
| | **Web LOC** | ~17,400 TS/TSX | Includes billing services and legal templates | | ||
| | **Worker LOC** | ~5,400 TS | Cloudflare Worker Hono code + chat streams | | ||
| | **DB Migrations** | 24 | From baseline to `20260607231000_c1_quota_auth_bypass.sql` | | ||
| | **pnpm Version** | `11.5.1` | `packageManager` drift vs `GEMINI.md` specified `11.1.3` | |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify version alignment across monorepo after this PR
echo "Root version:"
jq -r '.version' package.json
echo "Web version:"
jq -r '.version' web/package.json
echo "Worker version:"
jq -r '.version' worker/package.jsonRepository: Hex-Tech-Lab/hex-yt-intel
Length of output: 130
Fix audit snapshot version metrics to match current monorepo state (all packages are 1.5.2)
docs/audit/10X_REAUDIT_IV_COMBINED_2026_06_10.mdreports Root/Web/Worker versions as1.4.1 / 1.4.6 / 1.5.1with “Version Drift:⚠️ CONFIRMED”, and marks H6 “UNCHANGED”, but the current repo has1.5.2inpackage.json,web/package.json, andworker/package.json—so the drift/H6 statements don’t align with the post-PR state.- Update the version metrics + H6 status in the doc (lines 19-26 / H6 section), or explicitly label the document as intentionally pinned to the pre-PR snapshot it’s describing.
🤖 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 `@docs/audit/10X_REAUDIT_IV_COMBINED_2026_06_10.md` around lines 19 - 26,
Update the audit snapshot table entries for "Root Version", "Web Version", and
"Worker Version" from 1.4.1 / 1.4.6 / 1.5.1 to 1.5.2, and change the "Version
Drift" / H6 status to reflect no drift (or explicitly note the doc is a pre-PR
snapshot) so the doc's metrics match the current monorepo state; edit the
markdown table row labels "Root Version", "Web Version", "Worker Version",
"Version Drift" and the H6 status text in the same document to perform this
correction.
Comment on lines
+67
to
+137
| * **Target File**: [web/app/api/billing/webhook/route.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/billing/webhook/route.ts) | ||
| * **Action**: Restore the signature verification mechanism using `@paddle/paddle-node-sdk`. Gate the JSON parsing fallback strictly behind `process.env.NODE_ENV === 'development'`. | ||
| * **Estimated LLM Effort**: 15 minutes. | ||
|
|
||
| #### Step 1.2: Add AbortController to useSSEStream | ||
| * **Target File**: [web/hooks/useSSEStream.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/hooks/useSSEStream.ts) | ||
| * **Action**: Instantiate and maintain an `AbortController` in the SSE stream initiator. Call `.abort()` on the controller before starting a new stream or when the component unmounts to close old connections. | ||
| * **Estimated LLM Effort**: 20 minutes. | ||
|
|
||
| #### Step 1.3: Parallelize Traffic & Billing Checks in route.ts | ||
| * **Target File**: [web/app/api/analyses/route.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/analyses/route.ts) | ||
| * **Action**: Group `trafficAdapter.checkGate()` and `billingAdapter.checkGate()` calls into a single `Promise.all()` block. Saves ~100ms of latency per POST request. | ||
| * **Estimated LLM Effort**: 15 minutes. | ||
|
|
||
| --- | ||
|
|
||
| ### WAVE 2: Port Segregation (ISP/LSP Resolution) | ||
| * **Goal**: Refactor interfaces to contain only single-responsibility definitions. | ||
| * **Dependencies**: Wave 1 complete. | ||
|
|
||
| #### Step 2.1: Split IIngestionPort | ||
| * **Target Files**: | ||
| * [web/lib/ports/IIngestionPort.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/ports/IIngestionPort.ts) | ||
| * [web/lib/adapters/WorkerIngestionAdapter.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/adapters/WorkerIngestionAdapter.ts) | ||
| * **Action**: | ||
| * Create `IMetadataIngestionPort` containing only `fetch()` and `buildJobMetadata()`. | ||
| * Create `IModelResolutionPort` containing `resolveModels()`. | ||
| * Create `ICryptographicTokenPort` containing `signToken()`. | ||
| * Refactor `WorkerIngestionAdapter` to only implement `IMetadataIngestionPort`, removing the throwing methods. | ||
| * Move resolution and signing implementations into separate adapter classes cleanly. | ||
| * **Estimated LLM Effort**: 45 minutes. | ||
|
|
||
| #### Step 2.2: Split IQuotaPort | ||
| * **Target Files**: | ||
| * [web/lib/ports/IQuotaPort.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/ports/IQuotaPort.ts) | ||
| * [web/lib/adapters/RedisTrafficAdapter.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/adapters/RedisTrafficAdapter.ts) | ||
| * [web/lib/adapters/PostgresBillingAdapter.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/adapters/PostgresBillingAdapter.ts) | ||
| * **Action**: | ||
| * Create `ITrafficGuardPort` with `checkGate()` for DDoS/rate-limiting. | ||
| * Create `IBillingQuotaPort` with `checkGate()` and `refund()` for credit transactions. | ||
| * Refactor `RedisTrafficAdapter` to implement `ITrafficGuardPort` (removing the unused `refund` method). | ||
| * Refactor `PostgresBillingAdapter` to implement `IBillingQuotaPort`. | ||
| * **Estimated LLM Effort**: 30 minutes. | ||
|
|
||
| --- | ||
|
|
||
| ### WAVE 3: Domain UseCase Isolation & Clean Routing | ||
| * **Goal**: Move business orchestration into UseCase classes and unify persistence. | ||
| * **Dependencies**: Wave 2 complete. | ||
|
|
||
| #### Step 3.1: Create CreateAnalysisUseCase | ||
| * **Target File**: `web/lib/usecases/CreateAnalysisUseCase.ts` (New File) | ||
| * **Action**: Write the core analysis creation workflow inside a constructor-configured class using the new segregated ports. Keep it completely free of HTTP inputs or Next.js routing parameters. | ||
| * **Estimated LLM Effort**: 30 minutes. | ||
|
|
||
| #### Step 3.2: Refactor route.ts POST Handler | ||
| * **Target File**: [web/app/api/analyses/route.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/analyses/route.ts) | ||
| * **Action**: Instantiate the adapters, pass them to `CreateAnalysisUseCase`, and execute the UseCase. Map resulting data to `NextResponse.json` payload shapes. | ||
| * **Estimated LLM Effort**: 20 minutes. | ||
|
|
||
| #### Step 3.3: Refactor route.ts GET Handler | ||
| * **Target File**: [web/app/api/analyses/route.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/analyses/route.ts) | ||
| * **Action**: Add a `findAnalysesByUserId(userId: string): Promise<CachedAnalysis[]>` method to [IPersistencePort](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/ports/IPersistencePort.ts). Query history through [SupabasePersistenceAdapter](file:///home/kellyb_dev/projects/hex-yt-intel/web/lib/adapters/SupabasePersistenceAdapter.ts) instead of writing inline database calls in the GET route. | ||
| * **Estimated LLM Effort**: 25 minutes. | ||
|
|
||
| #### Step 3.4: Reconcile Monorepo Versions | ||
| * **Target Files**: | ||
| * [package.json](file:///home/kellyb_dev/projects/hex-yt-intel/package.json) | ||
| * [web/package.json](file:///home/kellyb_dev/projects/hex-yt-intel/web/package.json) | ||
| * [worker/package.json](file:///home/kellyb_dev/projects/hex-yt-intel/worker/package.json) | ||
| * **Action**: Unify monorepo package versions to `1.5.2` (or current clean state) to complete the Housekeeping checks. |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace absolute file paths with relative paths for portability.
The implementation plan contains absolute file system paths (e.g., file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/billing/webhook/route.ts). These paths break portability and will fail when the repository is cloned to different machines or CI environments. Replace them with relative paths from the repository root.
📝 Example fix
-* **Target File**: [web/app/api/billing/webhook/route.ts](file:///home/kellyb_dev/projects/hex-yt-intel/web/app/api/billing/webhook/route.ts)
+* **Target File**: `web/app/api/billing/webhook/route.ts`Apply this pattern to all file references in the document.
🧰 Tools
🪛 LanguageTool
[grammar] ~113-~113: Ensure spelling is correct
Context: ...*: 30 minutes. --- ### WAVE 3: Domain UseCase Isolation & Clean Routing * Goal: M...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~114-~114: Ensure spelling is correct
Context: ...oal**: Move business orchestration into UseCase classes and unify persistence. * **Depe...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
[warning] 71-71: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 76-76: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 83-83: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 87-87: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 99-99: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 113-113: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 117-117: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 122-122: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 127-127: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 132-132: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@docs/specs/IMPLEMENTATION_PLAN.md` around lines 67 - 137, The document uses
absolute file URIs (strings starting with
"file:///home/kellyb_dev/projects/hex-yt-intel/") which breaks portability;
replace every absolute file:///... reference with a relative repo path (e.g.,
change "file:///home/.../web/app/api/billing/webhook/route.ts" to
"web/app/api/billing/webhook/route.ts") throughout IMPLEMENTATION_PLAN.md,
including all entries under Step 1.1, Step 1.2, Step 1.3, Step 2.1, Step 2.2 and
Wave 3 targets; search for the "file:///" prefix and update referenced paths to
their relative equivalents so links and targets like
web/app/api/analyses/route.ts, web/hooks/useSSEStream.ts,
web/lib/ports/IIngestionPort.ts, web/lib/adapters/WorkerIngestionAdapter.ts,
etc., no longer use absolute file URIs.
Comment on lines
+67
to
+75
| const priorReport = (row.validationReport as any) || {}; | ||
| const isInterrupted = status === 'interrupted'; | ||
|
|
||
| // ADR 006: Dual-write - update both markdown and JSON payload columns | ||
| const { error: updateError } = await service | ||
| .from('analyses') | ||
| .update({ | ||
| analysis_markdown: markdown, | ||
| analysis_payload: payload ?? null, // ADR 006: JSONB column for structured payload | ||
| model_used: model || 'edge-stream', | ||
| validation_passed: isInterrupted ? false : !!valid, | ||
| validation_report: { | ||
| ...priorReport, | ||
| status: isInterrupted ? 'interrupted' : 'done', | ||
| model_used: model, | ||
| valid: isInterrupted ? false : !!valid, | ||
| }, | ||
| updated_at: new Date().toISOString(), | ||
| }) | ||
| .eq('id', analysisId); | ||
| const newReport = { | ||
| ...priorReport, | ||
| status: isInterrupted ? 'interrupted' : 'done', | ||
| model_used: model, | ||
| valid: isInterrupted ? false : !!valid, | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Type cast to any and mixed naming conventions in validation report handling.
Line 67 casts row.validationReport to any, losing type safety. Additionally, the code accesses transcript_available (snake_case at line 95) from priorReport, suggesting the JSON blob uses snake_case while adapter row fields use camelCase. Consider defining a ValidationReport type to ensure consistency.
♻️ Suggested type definition
interface ValidationReport {
status?: string;
transcript_available?: boolean;
model_used?: string;
valid?: boolean;
// ... other fields
}
const priorReport: ValidationReport = (row.validationReport as ValidationReport) || {};🤖 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 67 - 75, The code loses
type safety by casting row.validationReport to any and mixes snake_case vs
camelCase in the JSON blob; define a ValidationReport interface (including
fields like status, transcript_available, model_used, valid, etc.), replace the
any cast so priorReport is typed as ValidationReport, and update usages around
priorReport/newReport (and the created newReport object) to use the typed
properties consistently (preserve existing snake_case fields if the stored JSON
uses them) so TypeScript enforces correct property names and types when
reading/writing validationReport.
| url: validation.data.url, | ||
| timezone: validation.data.timezone || 'UTC', | ||
| forceRefresh: validation.data.forceRefresh || false, | ||
| explicitPersona: validation.data.persona as any, |
Contributor
There was a problem hiding this comment.
Unsafe as any cast for persona parameter.
The as any cast bypasses type checking for the persona field. If validation.data.persona doesn't match PersonaId, runtime errors could occur in the use case.
🐛 Suggested fix
- explicitPersona: validation.data.persona as any,
+ explicitPersona: validation.data.persona as PersonaId | undefined,Or update AnalysisCreateSchema to use PersonaId type directly.
📝 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.
Suggested change
| explicitPersona: validation.data.persona as any, | |
| explicitPersona: validation.data.persona as PersonaId | undefined, |
🤖 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/route.ts` at line 66, Replace the unsafe "as any" cast
for explicitPersona by ensuring validation.data.persona is typed/validated as
PersonaId: update AnalysisCreateSchema to declare persona as PersonaId (or
perform a runtime type guard) and then assign explicitPersona =
validation.data.persona (or cast to PersonaId only after validation); reference
the explicitPersona assignment and the AnalysisCreateSchema/PersonaId types to
locate and fix the code path where validation.data.persona is used.
Comment on lines
+19
to
+26
| if (!secret && process.env.NODE_ENV === 'development') { | ||
| event = JSON.parse(body); | ||
| } else { | ||
| if (!secret) { | ||
| throw new Error('PADDLE_WEBHOOK_SECRET is required'); | ||
| } | ||
| event = paddle.webhooks.unmarshal(body, secret, signature); | ||
| } |
Contributor
There was a problem hiding this comment.
Development-mode signature bypass could be risky if NODE_ENV is misconfigured.
The fallback to JSON.parse when PADDLE_WEBHOOK_SECRET is missing in development mode is convenient, but if NODE_ENV is accidentally set to development in a production-like environment, webhook signature verification would be bypassed. Consider adding an explicit check or warning log.
🛡️ Suggested hardening
if (!secret && process.env.NODE_ENV === 'development') {
+ console.warn('[Paddle Webhook] Signature verification BYPASSED - development mode without PADDLE_WEBHOOK_SECRET');
event = JSON.parse(body);
} else {📝 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.
Suggested change
| if (!secret && process.env.NODE_ENV === 'development') { | |
| event = JSON.parse(body); | |
| } else { | |
| if (!secret) { | |
| throw new Error('PADDLE_WEBHOOK_SECRET is required'); | |
| } | |
| event = paddle.webhooks.unmarshal(body, secret, signature); | |
| } | |
| if (!secret && process.env.NODE_ENV === 'development') { | |
| console.warn('[Paddle Webhook] Signature verification BYPASSED - development mode without PADDLE_WEBHOOK_SECRET'); | |
| event = JSON.parse(body); | |
| } else { | |
| if (!secret) { | |
| throw new Error('PADDLE_WEBHOOK_SECRET is required'); | |
| } | |
| event = paddle.webhooks.unmarshal(body, secret, signature); | |
| } |
🤖 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/billing/webhook/route.ts` around lines 19 - 26, The current
development-mode fallback parses the body without verification when secret is
missing, which is risky if NODE_ENV is misconfigured; update the webhook
handling in route.ts so that instead of silently using JSON.parse when secret is
falsey and NODE_ENV === 'development' you either (a) require an explicit opt-in
like a new env flag (e.g., DEV_ALLOW_UNVERIFIED_WEBHOOKS) and throw/log a clear
warning if that flag is not set, or (b) always throw and log an error unless
PADDLE_WEBHOOK_SECRET is present; specifically modify the block that references
secret, NODE_ENV, paddle.webhooks.unmarshal, signature and event to check for
PADDLE_WEBHOOK_SECRET and the opt-in flag, emit a prominent warning via your
logger when falling back, and avoid silently accepting unverified payloads.
Comment on lines
+3
to
+82
| /** | ||
| * Handles all database operations for chat threads and messages, isolating the chat | ||
| * domains from the database engine. | ||
| */ | ||
| export interface ChatPersistencePort { | ||
| /** | ||
| * List conversations for a user. | ||
| */ | ||
| getConversations(userId: string): Promise<ChatConversation[]>; | ||
|
|
||
| /** | ||
| * Start a new chat conversation. | ||
| */ | ||
| createConversation(params: { | ||
| userId: string; | ||
| analysisId: string | null; | ||
| title: string; | ||
| }): Promise<ChatConversation>; | ||
|
|
||
| /** | ||
| * Retrieve a single conversation. | ||
| */ | ||
| getConversation(params: { | ||
| conversationId: string; | ||
| }): Promise<ChatConversation | null>; | ||
|
|
||
| /** | ||
| * Update the title of a conversation. | ||
| */ | ||
| updateConversationTitle(params: { | ||
| conversationId: string; | ||
| title: string; | ||
| }): Promise<void>; | ||
|
|
||
| /** | ||
| * Load history/messages for a thread. | ||
| */ | ||
| getMessages(params: { | ||
| conversationId: string; | ||
| }): Promise<ChatMessage[]>; | ||
|
|
||
| /** | ||
| * Check if a message already exists (idempotency support). | ||
| */ | ||
| findMessageByClientMsgId(params: { | ||
| conversationId: string; | ||
| clientMsgId: string; | ||
| }): Promise<ChatMessage | null>; | ||
|
|
||
| /** | ||
| * Save a chat message. | ||
| */ | ||
| createMessage(params: { | ||
| conversationId: string; | ||
| userId: string; | ||
| role: 'user' | 'assistant'; | ||
| content: string; | ||
| clientMsgId?: string | null; | ||
| }): Promise<ChatMessage>; | ||
|
|
||
| /** | ||
| * Find the first assistant message created after a user message. | ||
| */ | ||
| findAssistantMessageAfter(params: { | ||
| conversationId: string; | ||
| timestamp: string; | ||
| }): Promise<ChatMessage | null>; | ||
|
|
||
| /** | ||
| * Retrieve analysis details for grounding the chat session. | ||
| */ | ||
| getAnalysisGrounding(params: { | ||
| analysisId: string; | ||
| }): Promise<{ | ||
| title: string; | ||
| channelTitle: string | null; | ||
| analysisMarkdown: string | null; | ||
| status: string; | ||
| } | null>; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Move interface documentation to /docs/ directory.
The coding guidelines require documentation to be kept in /docs/, not in code comments. The JSDoc comments throughout this file should be extracted to architecture documentation.
🤖 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/ports/ChatPersistencePort.ts` around lines 3 - 82, Remove the JSDoc
block comments from the ChatPersistencePort interface and its methods (leave the
exported interface and all method signatures intact) and move the explanatory
text into the project’s documentation directory as an architecture doc; create a
markdown entry that documents the ChatPersistencePort interface and each method
(ChatPersistencePort, getConversations, createConversation, getConversation,
updateConversationTitle, getMessages, findMessageByClientMsgId, createMessage,
findAssistantMessageAfter, getAnalysisGrounding) preserving the original
descriptions and request/response shapes so the code remains unchanged except
for removed comments.
Source: Coding guidelines
Comment on lines
+3
to
+24
| /** | ||
| * Handles signing streaming access tokens bound to video, analysis job, and resolved models list. | ||
| */ | ||
| export interface CryptographicTokenPort { | ||
| /** | ||
| * Mint an HMAC-signed streaming token bound to videoId + analysisId + models. | ||
| */ | ||
| signAnalysisToken(params: { | ||
| videoId: string; | ||
| analysisId: string; | ||
| models: string[]; | ||
| }): StreamToken; | ||
|
|
||
| /** | ||
| * Mint an HMAC-signed chat token bound to conversationId + userId + models. | ||
| */ | ||
| signChatToken(params: { | ||
| conversationId: string; | ||
| userId: string; | ||
| models: string[]; | ||
| }): StreamToken; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Move interface documentation to /docs/ directory.
The coding guidelines require documentation to be kept in /docs/, not in code comments. The JSDoc comments at lines 3-5, 7-9, and 16-18 should be extracted to architecture documentation.
🤖 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/ports/CryptographicTokenPort.ts` around lines 3 - 24, Remove the
inline JSDoc comments from the CryptographicTokenPort interface and its methods
signAnalysisToken and signChatToken and place that documentation into the
project architecture documentation (create a new docs entry describing the
interface purpose and the two method contracts: HMAC-signed streaming tokens
bound to their respective IDs and models). Update the code in
CryptographicTokenPort to keep only the type signatures (interface and method
params/return types) with no descriptive comments, and ensure the new docs
clearly mirror the removed comments so consumers can find the same information
outside the source file.
Source: Coding guidelines
Comment on lines
+5
to
+29
| /** | ||
| * Handles only the retrieval of video metadata and transcripts, and persona detection. | ||
| */ | ||
| export interface MetadataIngestionPort { | ||
| /** | ||
| * Fetch video metadata and transcript in parallel. | ||
| * @throws When metadata fetch fails (caller must refund quota). | ||
| * @returns IngestionResult with transcript possibly empty. | ||
| */ | ||
| fetch(videoId: string): Promise<IngestionResult>; | ||
|
|
||
| /** | ||
| * Detect the target persona from video title + channel, or use the explicit override. | ||
| */ | ||
| detectPersona(params: { | ||
| title: string; | ||
| channelTitle: string; | ||
| explicitPersona?: PersonaId; | ||
| }): PersonaId; | ||
|
|
||
| /** | ||
| * Build the canonical AnalysisJobMetadata from raw video metadata. | ||
| */ | ||
| buildJobMetadata(metadata: VideoMetadata): AnalysisJobMetadata; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Move interface documentation to /docs/ directory.
The coding guidelines require documentation to be kept in /docs/, not in code comments. The JSDoc comments at lines 5-7, 9-13, 16-18, and 25-27 should be extracted to architecture documentation.
🤖 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/ports/MetadataIngestionPort.ts` around lines 5 - 29, Remove the
inline JSDoc blocks from the MetadataIngestionPort interface and its members
(the top-level comment and the comments for fetch, detectPersona, and
buildJobMetadata) and instead add an entry in the project documentation
describing the interface responsibilities and contracts: list
MetadataIngestionPort, its methods fetch(videoId: string):
Promise<IngestionResult> (including the error/quotas behavior and that
transcript may be empty), detectPersona(params: {title, channelTitle,
explicitPersona?: PersonaId}): PersonaId, and buildJobMetadata(metadata:
VideoMetadata): AnalysisJobMetadata; keep type references (IngestionResult,
PersonaId, VideoMetadata, AnalysisJobMetadata) in the doc so implementers
understand expectations and remove the corresponding JSDoc comments from the
source.
Source: Coding guidelines
Comment on lines
+111
to
+136
| /** | ||
| * Find analysis row for server-to-server persistence lookup. | ||
| */ | ||
| findAnalysisForPersist(params: { | ||
| analysisId: string; | ||
| videoId: string; | ||
| }): Promise<{ | ||
| id: string; | ||
| userId: string; | ||
| title: string; | ||
| validationReport: any; | ||
| createdAt: string; | ||
| } | null>; | ||
|
|
||
| /** | ||
| * Update the analysis row with the final reasoning results. | ||
| */ | ||
| updateAnalysisResult(params: { | ||
| analysisId: string; | ||
| markdown: string; | ||
| payload: any; | ||
| model: string | null; | ||
| validationPassed: boolean; | ||
| status: 'done' | 'interrupted'; | ||
| validationReport: any; | ||
| }): Promise<void>; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Replace any types with proper type annotations.
Lines 121, 131, and 135 use any for validationReport and payload parameters, reducing type safety. Consider:
validationReport: Use a defined type (e.g.,ValidationReportInputor a union withunknown)payload: UseUCISPayloadV2 | null(already imported at line 3)
🔧 Suggested type improvements
findAnalysisForPersist(params: {
analysisId: string;
videoId: string;
}): Promise<{
id: string;
userId: string;
title: string;
- validationReport: any;
+ validationReport: ValidationReportInput | Record<string, unknown>;
createdAt: string;
} | null>;
updateAnalysisResult(params: {
analysisId: string;
markdown: string;
- payload: any;
+ payload: UCISPayloadV2 | null;
model: string | null;
validationPassed: boolean;
status: 'done' | 'interrupted';
- validationReport: any;
+ validationReport: Record<string, unknown>;
}): Promise<void>;🤖 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/ports/PersistencePort.ts` around lines 111 - 136, The
findAnalysisForPersist and updateAnalysisResult method signatures use `any` for
`validationReport` and `payload`; update them to use concrete types: replace
`validationReport: any` with a proper type such as `ValidationReportInput` (or
`ValidationReportInput | unknown` if you need a safe fallback) in both the
findAnalysisForPersist return type and the updateAnalysisResult parameter, and
change `payload: any` in updateAnalysisResult to `payload: UCISPayloadV2 | null`
(UCISPayloadV2 is already imported) so both methods use explicit, safer types
instead of `any`.
Comment on lines
+25
to
+28
| export type UseCaseResult = | ||
| | { type: 'cache_hit'; data: any; headers?: Record<string, string> } | ||
| | { type: 'processing'; data: any; headers?: Record<string, string> } | ||
| | { type: 'error'; code: string; status: number; message: string; headers?: Record<string, string> }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Strengthen typing for UseCaseResult.data field.
Using any for the data field loses type safety and makes it harder to catch contract mismatches between the use case and route handlers. Consider defining specific data shapes for each result type.
♻️ Suggested type-safe approach
+interface CacheHitData {
+ id: string;
+ analysisId: string;
+ videoId: string;
+ status: 'done';
+ title: string;
+ markdown: string;
+ // ... other fields
+}
+
+interface ProcessingData {
+ id: string;
+ analysisId: string;
+ videoId: string;
+ status: 'processing';
+ stream: { url: string; sig: string; exp: number };
+ // ... other fields
+}
+
export type UseCaseResult =
- | { type: 'cache_hit'; data: any; headers?: Record<string, string> }
- | { type: 'processing'; data: any; headers?: Record<string, string> }
+ | { type: 'cache_hit'; data: CacheHitData; headers?: Record<string, string> }
+ | { type: 'processing'; data: ProcessingData; headers?: Record<string, string> }
| { type: 'error'; code: string; status: number; message: string; headers?: Record<string, string> };📝 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.
Suggested change
| export type UseCaseResult = | |
| | { type: 'cache_hit'; data: any; headers?: Record<string, string> } | |
| | { type: 'processing'; data: any; headers?: Record<string, string> } | |
| | { type: 'error'; code: string; status: number; message: string; headers?: Record<string, string> }; | |
| interface CacheHitData { | |
| id: string; | |
| analysisId: string; | |
| videoId: string; | |
| status: 'done'; | |
| title: string; | |
| markdown: string; | |
| // ... other fields | |
| } | |
| interface ProcessingData { | |
| id: string; | |
| analysisId: string; | |
| videoId: string; | |
| status: 'processing'; | |
| stream: { url: string; sig: string; exp: number }; | |
| // ... other fields | |
| } | |
| export type UseCaseResult = | |
| | { type: 'cache_hit'; data: CacheHitData; headers?: Record<string, string> } | |
| | { type: 'processing'; data: ProcessingData; headers?: Record<string, string> } | |
| | { type: 'error'; code: string; status: number; message: string; headers?: Record<string, string> }; |
🤖 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/CreateAnalysisUseCase.ts` around lines 25 - 28, The
UseCaseResult type uses `any` for the `data` field which erases type safety;
replace `any` with explicit interfaces for each discriminated branch (e.g.,
define CacheHitData, ProcessingData and use those in the `UseCaseResult` union)
so callers of the UseCase (and route handlers) can rely on concrete shapes;
update the exported `UseCaseResult` union to reference these new types for
`data` (and add or tighten optional `headers` types if needed) and update
impacted call sites to conform to the new data shapes (look for references to
UseCaseResult, cache_hit, and processing handling).
…res, and upgrade postcss
|
✅ Pipeline Status: All checks passed. Ready to merge. |
This was referenced Jun 10, 2026
This was referenced Jun 19, 2026
TechHypeXP
pushed a commit
that referenced
this pull request
Jul 23, 2026
…, 16 moderate, 1 low) Queried the Dependabot API directly (gh api repos/.../dependabot/alerts) for exact vulnerable-version ranges and first_patched_version per alert, rather than guessing at bump targets from severity labels alone. - next: 16.2.6 -> 16.2.11 (web/package.json). Single patch-level bump resolves all 16 alerts (#73-90) at once -- SSRF in Server Actions/rewrites, Middleware/Turbopack bypass, DoS in Server Actions and Image Optimization SVGs, cache confusion on invalid-UTF8 request bodies, unauthenticated Server Function endpoint disclosure, unbounded Server Action payload in Edge runtime. Same minor line (16.2.x), no breaking changes -- verified via full build + 861-test suite, not assumed safe from the version number alone. - tar: ^7.5.16 -> ^7.5.19 (pnpm-workspace.yaml override). Resolves 4 cascading CVEs at once (#62 moderate through #64 critical -- decompression/parse DoS via unlimited input, negative-entry-size infinite loop, PAX numeric path type confusion, NUL-byte DoS). - js-yaml: ^4.1.2 -> ^4.3.0 (override). #61: merge-key chains forcing quadratic CPU consumption. - fast-uri: new override ^3.1.4 (was resolving to 3.1.2, no prior override entry). #69, #72: host confusion via failed IDN canonicalization and via a literal backslash authority delimiter. - sharp: new override ^0.35.0 (transitive, no direct dependency anywhere in the workspace). #71: inherited libvips CVEs (2026-33327/33328/35590/35591). - hono: new override ^4.12.27 (root workspace only -- video-pipelines own lockfile already resolved 4.12.31, already patched, no action needed there). #66-68: XSS via cx() escaping bypass, jsx context leaking across requests, API Gateway v1 adapter dropping repeated header values. - dompurify: ^3.4.11 -> ^3.4.12 (override, was one patch version short). #70: CUSTOM_ELEMENT_HANDLING bypasses afterSanitizeElements. - @hono/node-server: ^1.13.7 -> ^2.0.5 (video-pipeline/package.json). #91-92: Windows path traversal in serve-static via encoded backslash (%5C) -- these two were NET NEW from tonights video-pipeline addition, not pre-existing debt. A major version bump (1.x->2.x) for a service that only calls serve({fetch, port}) minimally -- verified beyond typecheck: full Docker build (npm audit inside the build reported "found 0 vulnerabilities"), then ran the container and confirmed /health and the auth-gate 401 both still work correctly post-bump. Duplicate-key fix along the way: pnpm-workspace.yaml already had tar and js-yaml override entries further down the file that my first pass missed, causing two separate "duplicated mapping key" YAML errors on `pnpm install` -- caught immediately (install refuses to run on invalid YAML, not a silent failure) and consolidated onto the pre-existing entries instead of leaving two competing pins for the same package. Verified E2E, not just typecheck, given the scope of what changed: - web: type-check clean, lint clean (0/0), production build clean, full vitest suite 861 passed / 16 pre-existing skips / 0 failed. - worker: typecheck clean, esbuild clean (2.1mb, no size regression). - video-pipeline: typecheck clean, Docker build clean (0 vulnerabilities per npm audit inside the image), container run-and-curl verified /health (all 3 binaries report healthy) and the 401 auth gate, matching pre-bump behavior exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TechHypeXP
pushed a commit
that referenced
this pull request
Jul 31, 2026
…, 16 moderate, 1 low) Queried the Dependabot API directly (gh api repos/.../dependabot/alerts) for exact vulnerable-version ranges and first_patched_version per alert, rather than guessing at bump targets from severity labels alone. - next: 16.2.6 -> 16.2.11 (web/package.json). Single patch-level bump resolves all 16 alerts (#73-90) at once -- SSRF in Server Actions/rewrites, Middleware/Turbopack bypass, DoS in Server Actions and Image Optimization SVGs, cache confusion on invalid-UTF8 request bodies, unauthenticated Server Function endpoint disclosure, unbounded Server Action payload in Edge runtime. Same minor line (16.2.x), no breaking changes -- verified via full build + 861-test suite, not assumed safe from the version number alone. - tar: ^7.5.16 -> ^7.5.19 (pnpm-workspace.yaml override). Resolves 4 cascading CVEs at once (#62 moderate through #64 critical -- decompression/parse DoS via unlimited input, negative-entry-size infinite loop, PAX numeric path type confusion, NUL-byte DoS). - js-yaml: ^4.1.2 -> ^4.3.0 (override). #61: merge-key chains forcing quadratic CPU consumption. - fast-uri: new override ^3.1.4 (was resolving to 3.1.2, no prior override entry). #69, #72: host confusion via failed IDN canonicalization and via a literal backslash authority delimiter. - sharp: new override ^0.35.0 (transitive, no direct dependency anywhere in the workspace). #71: inherited libvips CVEs (2026-33327/33328/35590/35591). - hono: new override ^4.12.27 (root workspace only -- video-pipelines own lockfile already resolved 4.12.31, already patched, no action needed there). #66-68: XSS via cx() escaping bypass, jsx context leaking across requests, API Gateway v1 adapter dropping repeated header values. - dompurify: ^3.4.11 -> ^3.4.12 (override, was one patch version short). #70: CUSTOM_ELEMENT_HANDLING bypasses afterSanitizeElements. - @hono/node-server: ^1.13.7 -> ^2.0.5 (video-pipeline/package.json). #91-92: Windows path traversal in serve-static via encoded backslash (%5C) -- these two were NET NEW from tonights video-pipeline addition, not pre-existing debt. A major version bump (1.x->2.x) for a service that only calls serve({fetch, port}) minimally -- verified beyond typecheck: full Docker build (npm audit inside the build reported "found 0 vulnerabilities"), then ran the container and confirmed /health and the auth-gate 401 both still work correctly post-bump. Duplicate-key fix along the way: pnpm-workspace.yaml already had tar and js-yaml override entries further down the file that my first pass missed, causing two separate "duplicated mapping key" YAML errors on `pnpm install` -- caught immediately (install refuses to run on invalid YAML, not a silent failure) and consolidated onto the pre-existing entries instead of leaving two competing pins for the same package. Verified E2E, not just typecheck, given the scope of what changed: - web: type-check clean, lint clean (0/0), production build clean, full vitest suite 861 passed / 16 pre-existing skips / 0 failed. - worker: typecheck clean, esbuild clean (2.1mb, no size regression). - video-pipeline: typecheck clean, Docker build clean (0 vulnerabilities per npm audit inside the image), container run-and-curl verified /health (all 3 binaries report healthy) and the 401 auth gate, matching pre-bump behavior exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR contains the complete core platform stabilization and hexagonal architecture alignment matching Option B in the implementation plan (Waves 1-3 + remaining refactoring tasks for search, billing, and chat routes).
Summary by cubic
Refactored the app to a clean hexagonal architecture by splitting ports, hiding database access behind adapters, and adding a
CreateAnalysisUseCaseto run analysis creation end-to-end. This removes direct@supabase/supabase-jscalls from API routes, tightens billing/search/chat flows, adds Sentry instrumentation, and fixes SSE abort leaks.Refactors
/api/analyses, chat (conversations/messages), search, and billing webhook routes to use adapters/use case only....Portand updating services; instrumented Sentry via@sentry/honoand added auth-failure logging inweb/middleware.ts.@hex-yt-intel/webandyoutube-intelligence-workerto1.5.2and upgradedpostcss.CreateAnalysisUseCaseto handle traffic/billing gates, ingestion, model resolution, token minting, and persistence.Bug Fixes
AbortControllerto stop duplicate streams and leaks.SupabasePersistenceAdapter.StreamTokenAdapter.signChatTokenand enforced message persistence via the chat persistence port.Written for commit fc0d270. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation