UPDATE#1218
Open
Roblmvp wants to merge 19 commits into
Open
Conversation
Adds the Organization model and nullable organizationId FKs on User and Prompt, plus the ExecutionRecord and StageTrace tables and the ExecutionStatus / AdaptationMode enums for the Composition Engine execution traces. Scope is intentionally atomic (Step 1 only): organizationId is nullable; backfill and the NOT NULL flip are deferred to Step 1b. compositionId is a plain indexed column with no FK (Composition model lands in Phase 2). https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Data-only migration: creates the single default "Vyaxis (Internal)" org (slug "internal") and assigns all existing org-less users and prompts to it. Idempotent (ON CONFLICT on slug; UPDATEs scoped to NULLs). organizationId remains nullable and the FK is unchanged; the NOT NULL flip and FK on-delete change are deferred to Step 1c, after the app write-paths populate organizationId. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Adds a Pino structured logger (src/lib/logger.ts) emitting JSON to stdout with service/env base fields and defensive redaction of password/apiKey/token/authorization. proxy.ts now propagates an x-request-id correlation id (reuses an inbound one, else mints a UUID), which route handlers read via requestLogger(). Adopted in the collection route (add/remove) as the first consumer. Path 1: complements the existing Sentry stack, does not replace it. No @vercel/otel (would conflict with Sentry's OTel). Sentry trace correlation deferred (requestId-only already satisfies the core need). https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
The hardcoded DSN was the upstream prompts.chat project's (org "promptschat"); with enabled-in-production + sendDefaultPii:true a fork deploy would send users' errors/traces/logs and PII to the upstream maintainer's Sentry. Move the DSN to env (SENTRY_DSN for server/edge, NEXT_PUBLIC_SENTRY_DSN for the browser client) defaulting to empty, which disables sending until a self-owned project is configured, and set sendDefaultPii:false everywhere. Documented the vars in .env.example. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Closes an active exposure: anon/authenticated held full DML grants on all public tables with RLS disabled, so the public anon key could read/write users.password, accounts OAuth tokens, verification_tokens, etc. via PostgREST over HTTPS (confirmed: anon GET on users returned a password hash). The app is pure-Prisma and connects as the bypass `postgres` role, so this is app-safe. - REVOKE ALL from anon/authenticated on all 26 public tables - ENABLE ROW LEVEL SECURITY (deny-by-default, no policies) on all 26 - ALTER DEFAULT PRIVILEGES FOR ROLE postgres to stop future tables auto-granting anon/authenticated Verified: anon REST read now 401 permission denied; 0 anon grants remain; RLS enabled on 26/26 tables. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
…e 2) Superset/non-destructive schema change: adds Composition and Stage models and the compositionId -> Composition FK (deferred from Step 1). Reuses the existing audited ExecutionRecord/StageTrace tables as-is, adding only ExecutionRecord.userId (NOT NULL) and sessionId (nullable); all Level-3 scoring columns are preserved. Schema only. The additive migration has NOT been applied to production yet (awaiting approval); no execution/trace columns are dropped. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Adds the org FK on compositions.organizationId -> organizations(id) with ON DELETE RESTRICT (deliberate deletion, no cascade), and ships the two migrations applied to prod: the additive composition-engine schema (compositions, stages, execution_records.userId/sessionId, composition FKs) and RLS enablement on the two new tables. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Multi-step chain orchestrator: dagResolver (order stages) -> stageExecutor (LLM via a new provider-agnostic abstraction over the existing OpenAI integration) -> adaptationLayer (extractFields + transformRules) -> thresholdCheck (deterministic v1; Level-3 scoring later) -> traceRecorder (atomic ExecutionRecord + StageTrace persistence). Falls back to fallbackPromptId on below-threshold/error; status success|partial|failed. Engine maps onto the audited trace columns (input->adaptedInput, duration->durationMs, success->thresholdMet, errorMessage->error, status->ExecutionStatus enum), preserving Level-3 scoring fields. Adds 5 routes under /api/v1/compositions (create/list, get, execute, history, test) and an e2e orchestrator test (stub LLM + mocked Prisma) covering the 3-stage chain, fallback, adaptation, and all status paths. https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
Drives one real 3-stage chain through the deployed HTTP routes (create + execute) against a live DB, then asserts the ExecutionRecord and the 3 StageTrace rows landed correctly (incl. adaptation between stages and real token usage). Mints a NextAuth v5 JWT session cookie for a seeded user. Run with: npm run smoke:composition (see header for required env). https://claude.ai/code/session_01HhSSACKMJYh92GqDAakXHT
The five /api/v1/compositions routes trusted organizationId and userId from the request body and only checked that a session existed, letting any authenticated user read, execute, or attribute work to another org. - userId now always comes from the authenticated session - organizationId is resolved server-side from the user's org, falling back to the default internal org (new src/lib/organization.ts) - client-supplied organizationId/userId are optional and rejected with 403 on mismatch; they are never used as the source of truth - list is scoped to the caller's org; all [id] routes return 404 for another org's composition so ids don't leak - 15 new route-level authz tests cover the rejection paths scripts/smoke-composition.ts is unchanged: its payloads supply the seeded user's real org/user ids, which now pass the mismatch checks. https://claude.ai/code/session_01GP2YigWVnAGR3xGQ59N4SK
AnthropicProvider implements the existing LLMProvider interface — no changes to stageExecutor or any call site. Selection is opt-in: the provider is used only when LLM_PROVIDER=anthropic AND ANTHROPIC_API_KEY are both set; otherwise the OpenAI provider runs exactly as before. - model from ANTHROPIC_GENERATIVE_MODEL, default claude-opus-4-8 - prompt caching v1: the stage's system prompt is a stable prefix re-sent on every chain execution, so it carries cache_control (ephemeral) and repeated executions hit the cache - cache read/write tokens fold into inputTokens so StageTrace token accounting stays comparable across providers - temperature is not forwarded (current Claude models reject sampling parameters) - all three env vars documented in .env.example - 9 unit tests with a mocked SDK: completion mapping, token accounting, cache flag, no-sampling-params, and selection logic Citations support is deferred to the Level 3 spec. https://claude.ai/code/session_01GP2YigWVnAGR3xGQ59N4SK
Build-ready spec for the next session. Six sections, each with API sketches and data-flow diagrams: 1. Validation framework: Zod-enforced evaluator contract with UNKNOWN / BLOCKED_BY_MISSING_EVIDENCE states; non-conforming model output is rejected, never coerced; results persist into the reserved StageTrace scoring columns. 2. Governed promotion: candidate -> evaluation -> ChangeRequest -> PromptVersion promotion -> rollback, reusing the existing models; machine improvements never mutate production prompts in place. 3. Computed routing hints: nightly StageTrace rollup by prompt x model into bestWithModels + a new PromptModelStats table. 4. MCP tools: recommend_composition (pgvector over the existing embedding column, org-scoped, ranked by routing hints — with the explicit post-seed embedding backfill prerequisite) and execute_composition. 5. Provider phase 2: citations mode on AnthropicProvider for evidence-backed stages. 6. Two-week sequencing with dependencies, plus explicit non-goals (no swarm, no auto-publishing, nothing bypasses ChangeRequest review). All Level 3 schema changes live in this spec; none land in Level 2 code. https://claude.ai/code/session_01GP2YigWVnAGR3xGQ59N4SK
The Vercel Preview build failed in postinstall: prisma/config's env() helper throws PrismaConfigEnvError at config-load time when DATABASE_URL is unset, so 'prisma generate' (pure codegen, no DB connection) never ran. Read the var directly with a non-functional placeholder fallback so codegen works without the secret; migrate/db push/seed and all runtime code run only where DATABASE_URL is set, so the placeholder is never dialed. Verified: 'env -u DATABASE_URL prisma generate' now exits 0. https://claude.ai/code/session_01GP2YigWVnAGR3xGQ59N4SK
|
@Roblmvp is attempting to deploy a commit to the fkadev Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis change adds a multi-tenant Composition Engine with Anthropic/OpenAI provider support, ordered stage execution, fallbacks, output adaptation, persisted traces, scoped API routes, authorization tests, operational logging, Sentry configuration, and a Level 3 implementation blueprint. ChangesComposition Engine
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CompositionRoute
participant executeComposition
participant LLMProvider
participant Database
Client->>CompositionRoute: POST composition execution
CompositionRoute->>Database: Verify organization-scoped composition
CompositionRoute->>executeComposition: Execute with session-derived context
executeComposition->>LLMProvider: Complete each ordered stage
LLMProvider-->>executeComposition: Output and token usage
executeComposition->>Database: Persist execution record and stage traces
Database-->>CompositionRoute: executionId and status
CompositionRoute-->>Client: Execution result
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
package.jsonOops! Something went wrong! :( ESLint: 9.39.2 A configuration object specifies rule "react-hooks/set-state-in-effect", but could not find plugin "react-hooks". Common causes of this problem include:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (6)
src/lib/ai/llm-provider.ts (2)
45-48: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet explicit timeouts on both LLM clients.
Both
OpenAIandAnthropicclients are constructed without atimeout, leaving them at the SDK defaults (~10 minutes). In a sequential composition engine, a single hanging stage blocks the entire pipeline for that duration. Pass an explicit, shorter timeout (e.g., 30–60 s) to fail fast and let the orchestrator apply fallbacks.⏱️ Proposed fix
this.client = new OpenAI({ apiKey, baseURL: process.env.OPENAI_BASE_URL || undefined, + timeout: 30_000, });- this.client = new Anthropic({ apiKey }); + this.client = new Anthropic({ apiKey, timeout: 30_000 });Also applies to: 88-88
🤖 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 `@src/lib/ai/llm-provider.ts` around lines 45 - 48, Update the OpenAI client construction in the provider initialization and the corresponding Anthropic client construction to pass an explicit 30–60 second timeout. Apply the same timeout policy to both client instances so hanging LLM calls fail promptly and existing orchestration fallback behavior can run.
65-65: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer
max_completion_tokenshere.max_tokensis still accepted in openai@6.x, but it’s deprecated and can break on newer reasoning models.🤖 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 `@src/lib/ai/llm-provider.ts` at line 65, Replace the max_tokens request option with max_completion_tokens in the LLM provider configuration, preserving the existing req.maxTokens fallback value and surrounding request behavior.prisma/migrations/20260606185240_add_multitenancy_and_execution_traces/migration.sql (1)
85-94: 🩺 Stability & Availability | 🔵 TrivialIndex and FK operations on existing tables may block writes in production.
Squawk flags non-concurrent
CREATE INDEXonusers(line 85) andprompts(line 88), and FK constraints withoutNOT VALIDon the same tables (lines 91, 94). These acquire locks that block writes on tables with existing data.CREATE INDEX CONCURRENTLYandADD CONSTRAINT ... NOT VALIDrequire running outside a transaction, which conflicts with Prisma's transactional migration model. If these tables are large in production, consider applying these changes via a manual migration outside Prisma.🤖 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 `@prisma/migrations/20260606185240_add_multitenancy_and_execution_traces/migration.sql` around lines 85 - 94, The migration performs blocking index creation and foreign-key validation on existing users and prompts tables. Do not alter this Prisma migration to use concurrent indexes or NOT VALID constraints; instead, plan and apply these operations through a manual, non-transactional production migration, while preserving the existing schema migration for environments where Prisma’s transactional behavior is acceptable.Source: Linters/SAST tools
prisma/schema.prisma (1)
419-527: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSchema models are well-structured with appropriate indexes and cascade rules.
The multi-tenant composition engine schema correctly uses
onDelete: RestrictonComposition.organization(prevents accidental org deletion),CascadeonStage.compositionandExecutionRecord.composition, and unique constraints on[compositionId, order]and[executionId, stageOrder]. Index coverage matches the API query patterns.One documentation gap:
StageTrace.promptId(line 494) explicitly comments "references prompts.id by value; no FK on the hot prompts table by design," butStage.promptId(line 440) andStage.fallbackPromptId(line 441) follow the same pattern without the same rationale documented. Consider adding a similar comment for consistency.🤖 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 `@prisma/schema.prisma` around lines 419 - 527, Document the intentional lack of foreign-key relations for Stage.promptId and Stage.fallbackPromptId, matching the rationale already stated on StageTrace.promptId: these fields reference prompts.id by value because the prompts table is hot. Add concise comments directly to both Stage fields without changing the schema behavior.src/app/api/v1/compositions/route.ts (1)
28-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract shared auth + org resolution into a helper to reduce duplication.
The same ~12-line block (auth check →
resolveUserOrganizationId→ 401/403 guards) is repeated verbatim across all six composition route handlers. A shared helper would eliminate this boilerplate and ensure consistent error responses.♻️ Proposed helper
// src/lib/composition/auth-context.ts import { NextResponse } from "next/server"; import { auth } from "`@/lib/auth`"; import { resolveUserOrganizationId } from "`@/lib/organization`"; export async function resolveAuthContext() { const session = await auth(); if (!session?.user) { return { response: NextResponse.json({ error: "unauthorized" }, { status: 401 }), }; } const organizationId = await resolveUserOrganizationId(session.user.id); if (!organizationId) { return { response: NextResponse.json({ error: "no_organization" }, { status: 403 }), }; } return { userId: session.user.id, organizationId, session }; }Usage in each route:
- const session = await auth(); - if (!session?.user) { - return NextResponse.json({ error: "unauthorized" }, { status: 401 }); - } - const organizationId = await resolveUserOrganizationId(session.user.id); - if (!organizationId) { - return NextResponse.json({ error: "no_organization" }, { status: 403 }); - } + const ctx = await resolveAuthContext(); + if ("response" in ctx) return ctx.response; + const { userId, organizationId } = ctx;🤖 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 `@src/app/api/v1/compositions/route.ts` around lines 28 - 99, Extract the repeated authentication and organization lookup from POST and the other composition route handlers into a shared resolveAuthContext helper. Have it return the existing 401 or 403 NextResponse for missing users or organizations, and otherwise provide userId, organizationId, and session; update each handler to use this helper while preserving its current response behavior and downstream organization checks.src/lib/organization.ts (1)
15-28: 🚀 Performance & Scalability | 🔵 TrivialLGTM!
Clean tenant resolver with correct fallback semantics. The two-query path (user → default org) only triggers for pre-multi-tenancy users, which is the expected migration cohort.
One operational note: every composition API request makes a DB round-trip here. If these routes become high-traffic, consider adding
organizationIdto the NextAuth session token (the session callback insrc/lib/auth/index.tscurrently omits it) so this resolver can short-circuit on the session value and only hit the DB as a fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/organization.ts` around lines 15 - 28, Optimize resolveUserOrganizationId by first using organizationId from the authenticated NextAuth session when available, avoiding a database query for composition API requests. Update the session callback in auth configuration to include organizationId in the session token, while preserving the existing user-to-default-organization database fallback when the session value is absent.
🤖 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 @.gitignore:
- Around line 52-53: Update the .gitignore environment patterns so the catch-all
.env* rule no longer overrides the existing !.env.example exception. Move .env*
before that negation or add a later !.env.example rule, ensuring .env.example
remains trackable.
In `@docs/level3-plan.md`:
- Line 78: Add language identifiers to every fenced code block in
docs/level3-plan.md, including the referenced locations: use text for diagrams,
ts for TypeScript, prisma for schema, and sql for the aggregation query, while
preserving each block’s contents.
- Around line 96-102: Update the Stage evaluation flow so one authoritative
predicate combines state === "PASS" with mean(scores) >= Stage.successThreshold;
use this predicate for both StageTrace.thresholdMet persistence and the engine
continuation/fallback gate, ensuring below-threshold PASS results follow the
fallback path.
- Around line 415-417: Update the dependency spine summary in the plan to
separate the two MCP tools: state that recommend_composition depends on routing
statistics and embeddings, while execute_composition has no embedding dependency
and may ship before recommendation. Preserve the existing §5 parallelization and
unaffected-work statements.
- Around line 246-260: Update the PromptModelStats model’s promptId field to
define a Prisma relation to Prompt, including the appropriate relation field and
delete behavior, and add the corresponding database foreign-key constraint in
the migration. Preserve the existing uniqueness and index definitions while
enabling organization-scoped prompt relation traversal.
- Around line 140-142: Update the StageTrace aggregation SQL to read state from
the evaluation JSON using evaluation->>'state' rather than a stage_traces.state
column, and use valid FILTER (WHERE ...) syntax for passRate. Explicitly define
whether rows with evaluation IS NULL are included or excluded, preserving the
intended handling of UNKNOWN and BLOCKED_BY_MISSING_EVIDENCE states in both
affected query sections.
- Around line 93-100: Update the synthetic UNKNOWN fallback in the StageTrace
evaluation flow to satisfy StageEvaluationSchema by either making its required
fields optional for this explicit state or supplying complete zero/empty values
for scores, facts, inferences, assumptions, and risks. Ensure StageTrace.score
and ExecutionRecord.overallScore retain clearly defined semantics when the
fallback has no scores, and persist the resulting complete evaluation object.
- Around line 63-67: Update the StageEvaluationSchema validation flow to
JSON-parse the string LLMCompletionResult.output before calling safeParse.
Preserve strict rejection of nonconforming parsed values, and include JSON
parsing failures in the single re-ask error context before recording UNKNOWN
after the second failure.
- Around line 44-57: Update StageEvaluationSchema to use z.strictObject() so
unknown evaluation fields are rejected, and enforce the blockers contract: PASS
evaluations must have no blockers, while non-PASS evaluations must have at least
one. Use a refinement or discriminated PASS/non-PASS union while preserving the
existing fields and score validation.
- Around line 283-289: Update the limit field in the recommend_composition
inputSchema to require values between 1 and 10 by adding the minimum constraint
alongside the existing integer, maximum, and default settings.
In `@scripts/smoke-composition.ts`:
- Around line 44-47: Update fail() to throw an error instead of calling
process.exit(1), allowing control flow to reach cleanup. Hoist the composition,
execution, and prompt identifiers to a scope shared with main(), and move the
existing cleanup logic into the main().catch().finally() chain so it runs after
failures and successful completion.
- Around line 141-154: Update the fetch call in postJson to include an
AbortSignal.timeout with an appropriate timeout duration, ensuring unresponsive
requests terminate instead of hanging indefinitely while preserving the existing
request and response handling.
In `@src/app/api/v1/compositions/`[id]/test/route.ts:
- Around line 9-10: Update the testSchema sampleInputs validation to add a
reasonable maximum array length alongside the existing minimum, preventing
oversized requests from triggering excessive composition executions and LLM
calls.
- Around line 107-109: Replace the message-prefix check in the route’s error
handler with an explicit custom error type exported from the composition module.
Update executeComposition to throw that typed error for missing compositions,
then use instanceof against the shared error class to return the existing 404
response while leaving other errors as 500s.
In `@src/app/api/v1/compositions/route.ts`:
- Around line 121-130: Update the GET compositions handler around
db.composition.findMany to support page/limit pagination consistent with the
sibling history endpoint: parse the query parameters, clamp them to the
established valid bounds, and apply the resulting skip and take values to the
ordered query. Preserve the organization filter, ordering, included
stages/count, and JSON response shape.
- Around line 41-42: Guard the request.json() call in the route handler so
malformed JSON SyntaxErrors return a 400 response instead of reaching the
generic 500 catch path. Keep createSchema.parse validation and existing handling
unchanged for successfully parsed bodies and other errors.
---
Nitpick comments:
In
`@prisma/migrations/20260606185240_add_multitenancy_and_execution_traces/migration.sql`:
- Around line 85-94: The migration performs blocking index creation and
foreign-key validation on existing users and prompts tables. Do not alter this
Prisma migration to use concurrent indexes or NOT VALID constraints; instead,
plan and apply these operations through a manual, non-transactional production
migration, while preserving the existing schema migration for environments where
Prisma’s transactional behavior is acceptable.
In `@prisma/schema.prisma`:
- Around line 419-527: Document the intentional lack of foreign-key relations
for Stage.promptId and Stage.fallbackPromptId, matching the rationale already
stated on StageTrace.promptId: these fields reference prompts.id by value
because the prompts table is hot. Add concise comments directly to both Stage
fields without changing the schema behavior.
In `@src/app/api/v1/compositions/route.ts`:
- Around line 28-99: Extract the repeated authentication and organization lookup
from POST and the other composition route handlers into a shared
resolveAuthContext helper. Have it return the existing 401 or 403 NextResponse
for missing users or organizations, and otherwise provide userId,
organizationId, and session; update each handler to use this helper while
preserving its current response behavior and downstream organization checks.
In `@src/lib/ai/llm-provider.ts`:
- Around line 45-48: Update the OpenAI client construction in the provider
initialization and the corresponding Anthropic client construction to pass an
explicit 30–60 second timeout. Apply the same timeout policy to both client
instances so hanging LLM calls fail promptly and existing orchestration fallback
behavior can run.
- Line 65: Replace the max_tokens request option with max_completion_tokens in
the LLM provider configuration, preserving the existing req.maxTokens fallback
value and surrounding request behavior.
In `@src/lib/organization.ts`:
- Around line 15-28: Optimize resolveUserOrganizationId by first using
organizationId from the authenticated NextAuth session when available, avoiding
a database query for composition API requests. Update the session callback in
auth configuration to include organizationId in the session token, while
preserving the existing user-to-default-organization database fallback when the
session value is absent.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a9608130-7692-4e9f-b7af-ec821d92df50
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
.env.example.gitignoredocs/level3-plan.mdpackage.jsonprisma.config.tsprisma/migrations/20260606185240_add_multitenancy_and_execution_traces/migration.sqlprisma/migrations/20260606192213_backfill_default_org/migration.sqlprisma/migrations/20260606200000_secure_public_tables_revoke_and_rls/migration.sqlprisma/migrations/20260606201500_add_composition_engine/migration.sqlprisma/migrations/20260606201600_enable_rls_compositions_stages/migration.sqlprisma/schema.prismaprompts.config.tsscripts/smoke-composition.tssentry.edge.config.tssentry.server.config.tssrc/__tests__/composition/authz.test.tssrc/__tests__/composition/orchestrator.test.tssrc/__tests__/lib/ai/llm-provider.test.tssrc/app/api/collection/route.tssrc/app/api/v1/compositions/[id]/execute/route.tssrc/app/api/v1/compositions/[id]/history/route.tssrc/app/api/v1/compositions/[id]/route.tssrc/app/api/v1/compositions/[id]/test/route.tssrc/app/api/v1/compositions/route.tssrc/instrumentation-client.tssrc/lib/ai/llm-provider.tssrc/lib/composition/adaptationLayer.tssrc/lib/composition/dagResolver.tssrc/lib/composition/index.tssrc/lib/composition/stageExecutor.tssrc/lib/composition/thresholdCheck.tssrc/lib/composition/traceRecorder.tssrc/lib/composition/types.tssrc/lib/logger.tssrc/lib/organization.tssrc/proxy.ts
Comment on lines
+52
to
+53
| .env.production.vyaxis | ||
| .env* |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
.env* on line 53 shadows the !.env.example negation on line 37, causing .env.example to be ignored.
In .gitignore, the last matching pattern wins. Since .env* (line 53) comes after !.env.example (line 37), it re-ignores .env.example. This means the example env template — which should be tracked — will be excluded from git, and git status may show it as deleted if already tracked.
🐛 Proposed fix
Move the catch-all before the negation, or add a second negation after it:
.env.production.vyaxis
-.env*
+.env*
+!.env.example📝 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
| .env.production.vyaxis | |
| .env* | |
| .env.production.vyaxis | |
| .env* | |
| !.env.example |
🤖 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 @.gitignore around lines 52 - 53, Update the .gitignore environment patterns
so the catch-all .env* rule no longer overrides the existing !.env.example
exception. Move .env* before that negation or add a later !.env.example rule,
ensuring .env.example remains trackable.
Comment on lines
+44
to
+57
| export const StageEvaluationSchema = z.object({ | ||
| state: EvalStateSchema, | ||
| scores: z.object({ | ||
| relevance: z.number().min(0).max(1), | ||
| completeness: z.number().min(0).max(1), | ||
| grounding: z.number().min(0).max(1), // claims traceable to stage input | ||
| formatFit: z.number().min(0).max(1), // matches what the next stage expects | ||
| }), | ||
| facts: z.array(z.string()), // claims in the output supported by the input | ||
| inferences: z.array(z.string()), // claims derived but not directly stated | ||
| assumptions: z.array(z.string()), // claims with no support in the input | ||
| risks: z.array(z.string()), // ways this output could mislead stage N+1 | ||
| blockers: z.array(z.string()), // required only when state != PASS; must be non-empty then | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files | rg '(^|/)docs/level3-plan\.md$|(^|/)package\.json$|(^|/)pnpm-lock\.yaml$|(^|/)package-lock\.json$|(^|/)yarn.lock$|(^|/)schema\.prisma$|(^|/)prisma/|(^|/)src/'
printf '\n== docs/level3-plan.md outline ==\n'
ast-grep outline docs/level3-plan.md --view expanded || true
printf '\n== targeted excerpt around lines 1-220 ==\n'
cat -n docs/level3-plan.md | sed -n '1,220p'Repository: f/prompts.chat
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== docs/level3-plan.md lines 35-75 ==\n'
cat -n docs/level3-plan.md | sed -n '35,75p'
printf '\n== package.json zod references ==\n'
rg -n '"zod"|strictObject|superRefine|discriminatedUnion' package.json packages/**/package.json prisma/schema.prisma docs/level3-plan.md || true
printf '\n== package.json (root) excerpt ==\n'
cat -n package.json | sed -n '1,220p'Repository: f/prompts.chat
Length of output: 7451
Make StageEvaluationSchema enforce the stated hard rules. z.object() will silently drop unknown keys, and blockers is still unconstrained; use z.strictObject() plus a refinement or a PASS/non-PASS union so invalid evaluations are rejected instead of normalized.
🤖 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/level3-plan.md` around lines 44 - 57, Update StageEvaluationSchema to
use z.strictObject() so unknown evaluation fields are rejected, and enforce the
blockers contract: PASS evaluations must have no blockers, while non-PASS
evaluations must have at least one. Use a refinement or discriminated
PASS/non-PASS union while preserving the existing fields and score validation.
Comment on lines
+63
to
+67
| - **Reject, never coerce.** `StageEvaluationSchema.safeParse` on the raw model | ||
| output. On failure: one re-ask with the Zod error appended; on second | ||
| failure the evaluation is recorded as `UNKNOWN` with | ||
| `blockers: ["evaluator_nonconforming_output"]`. We never `.catch()`-default | ||
| scores, never clamp, never strip unknown keys silently. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='docs/level3-plan.md'
echo '--- lines 55-75 ---'
sed -n '55,75p' "$file"
echo
echo '--- lines 350-375 ---'
sed -n '350,375p' "$file"
echo
echo '--- search for StageEvaluationSchema and LLMCompletionResult.output ---'
rg -n 'StageEvaluationSchema|LLMCompletionResult\.output|safeParse|evaluator_nonconforming_output' "$file"Repository: f/prompts.chat
Length of output: 2476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='docs/level3-plan.md'
sed -n '78,98p' "$file"Repository: f/prompts.chat
Length of output: 1295
Parse the provider JSON string before StageEvaluationSchema.safeParse. LLMCompletionResult.output is a string, so passing it directly into the object schema will reject valid JSON text and push evaluations to UNKNOWN. Parse the JSON first, then validate that value and include JSON parse errors in the re-ask.
🤖 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/level3-plan.md` around lines 63 - 67, Update the StageEvaluationSchema
validation flow to JSON-parse the string LLMCompletionResult.output before
calling safeParse. Preserve strict rejection of nonconforming parsed values, and
include JSON parsing failures in the single re-ask error context before
recording UNKNOWN after the second failure.
|
|
||
| ### Data flow | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to all fenced code blocks.
markdownlint-cli2 reports MD040 for these fences. Use text for diagrams, ts for TypeScript, prisma for schema, and sql for the aggregation query.
Also applies to: 137-137, 188-188, 210-210, 265-265, 294-294, 386-386
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/level3-plan.md` at line 78, Add language identifiers to every fenced
code block in docs/level3-plan.md, including the referenced locations: use text
for diagrams, ts for TypeScript, prisma for schema, and sql for the aggregation
query, while preserving each block’s contents.
Source: Linters/SAST tools
Comment on lines
+93
to
+100
| │ { state: UNKNOWN, | ||
| │ blockers: [evaluator_nonconforming_output] } | ||
| v | ||
| persist into StageTrace reserved columns: | ||
| score <- mean(scores) (Float?, exists today) | ||
| thresholdMet <- state == "PASS" (Boolean?, exists today) | ||
| + new column StageTrace.evaluation Json? (full contract object) | ||
| ExecutionRecord.overallScore <- mean of stage scores (Float?, exists today) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the synthetic UNKNOWN value satisfy StageEvaluationSchema.
The fallback object only contains state and blockers, while the schema requires scores, facts, inferences, assumptions, and risks. It also leaves mean(scores) undefined for StageTrace.score. Either make those fields optional for this explicit fallback state or define and persist a complete zero/empty representation with clear overall-score semantics.
🤖 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/level3-plan.md` around lines 93 - 100, Update the synthetic UNKNOWN
fallback in the StageTrace evaluation flow to satisfy StageEvaluationSchema by
either making its required fields optional for this explicit state or supplying
complete zero/empty values for scores, facts, inferences, assumptions, and
risks. Ensure StageTrace.score and ExecutionRecord.overallScore retain clearly
defined semantics when the fallback has no scores, and persist the resulting
complete evaluation object.
Comment on lines
+141
to
+154
| const postJson = async (path: string, body: unknown): Promise<{ status: number; json: unknown }> => { | ||
| const res = await fetch(`${BASE_URL}${path}`, { | ||
| method: "POST", | ||
| headers: { "content-type": "application/json", cookie }, | ||
| body: JSON.stringify(body), | ||
| }); | ||
| let json: unknown = null; | ||
| try { | ||
| json = await res.json(); | ||
| } catch { | ||
| /* non-JSON body */ | ||
| } | ||
| return { status: res.status, json }; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add a timeout to fetch calls to prevent indefinite hangs.
postJson issues fetch requests without a timeout or AbortSignal. If the deployed server is slow or unresponsive, the smoke test will hang indefinitely with no feedback. Adding AbortSignal.timeout() is a one-line fix that prevents stuck runs.
⏱️ Proposed fix
async function postJson(path: string, body: unknown): Promise<{ status: number; json: unknown }> {
const res = await fetch(`${BASE_URL}${path}`, {
method: "POST",
headers: { "content-type": "application/json", cookie },
body: JSON.stringify(body),
+ signal: AbortSignal.timeout(30_000),
});📝 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
| const postJson = async (path: string, body: unknown): Promise<{ status: number; json: unknown }> => { | |
| const res = await fetch(`${BASE_URL}${path}`, { | |
| method: "POST", | |
| headers: { "content-type": "application/json", cookie }, | |
| body: JSON.stringify(body), | |
| }); | |
| let json: unknown = null; | |
| try { | |
| json = await res.json(); | |
| } catch { | |
| /* non-JSON body */ | |
| } | |
| return { status: res.status, json }; | |
| }; | |
| const postJson = async (path: string, body: unknown): Promise<{ status: number; json: unknown }> => { | |
| const res = await fetch(`${BASE_URL}${path}`, { | |
| method: "POST", | |
| headers: { "content-type": "application/json", cookie }, | |
| body: JSON.stringify(body), | |
| signal: AbortSignal.timeout(30_000), | |
| }); | |
| let json: unknown = null; | |
| try { | |
| json = await res.json(); | |
| } catch { | |
| /* non-JSON body */ | |
| } | |
| return { status: res.status, json }; | |
| }; |
🤖 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 `@scripts/smoke-composition.ts` around lines 141 - 154, Update the fetch call
in postJson to include an AbortSignal.timeout with an appropriate timeout
duration, ensuring unresponsive requests terminate instead of hanging
indefinitely while preserving the existing request and response handling.
Comment on lines
+9
to
+10
| const testSchema = z.object({ | ||
| sampleInputs: z.array(z.string().min(1)).min(1), |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Cap sampleInputs array length to prevent LLM quota exhaustion.
sampleInputs has .min(1) but no .max(). A single request with hundreds of inputs triggers hundreds of full composition executions (each with multiple LLM API calls), which can exhaust provider quotas, incur significant cost, and block the request for minutes. Add a reasonable upper bound.
🛡️ Proposed fix
- sampleInputs: z.array(z.string().min(1)).min(1),
+ sampleInputs: z.array(z.string().min(1)).min(1).max(10),📝 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
| const testSchema = z.object({ | |
| sampleInputs: z.array(z.string().min(1)).min(1), | |
| const testSchema = z.object({ | |
| sampleInputs: z.array(z.string().min(1)).min(1).max(10), |
🤖 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 `@src/app/api/v1/compositions/`[id]/test/route.ts around lines 9 - 10, Update
the testSchema sampleInputs validation to add a reasonable maximum array length
alongside the existing minimum, preventing oversized requests from triggering
excessive composition executions and LLM calls.
Comment on lines
+107
to
+109
| if (error instanceof Error && error.message.startsWith("Composition not found")) { | ||
| return NextResponse.json({ error: "not_found" }, { status: 404 }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace fragile string-prefix error matching with a typed error.
error.message.startsWith("Composition not found") couples this handler to the exact error text in executeComposition (src/lib/composition/index.ts). If that message changes, this silently degrades to a 500 instead of 404. A custom error class would make this contract explicit.
♻️ Proposed fix: use a custom error class
In src/lib/composition/index.ts:
+export class CompositionNotFoundError extends Error {
+ constructor(public compositionId: string) {
+ super(`Composition not found: ${compositionId}`);
+ this.name = "CompositionNotFoundError";
+ }
+}Then in the test route:
- if (error instanceof Error && error.message.startsWith("Composition not found")) {
+ if (error instanceof CompositionNotFoundError) {
return NextResponse.json({ error: "not_found" }, { status: 404 });
}🤖 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 `@src/app/api/v1/compositions/`[id]/test/route.ts around lines 107 - 109,
Replace the message-prefix check in the route’s error handler with an explicit
custom error type exported from the composition module. Update
executeComposition to throw that typed error for missing compositions, then use
instanceof against the shared error class to return the existing 404 response
while leaving other errors as 500s.
Comment on lines
+41
to
+42
| const body = await request.json(); | ||
| const data = createSchema.parse(body); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard request.json() to return 400 on malformed JSON.
request.json() throws a SyntaxError for invalid JSON bodies. This is caught by the generic catch block and returns 500, but malformed JSON is a client error and should return 400.
🛡️ Proposed fix
- const body = await request.json();
- const data = createSchema.parse(body);
+ let body: unknown;
+ try {
+ body = await request.json();
+ } catch {
+ return NextResponse.json({ error: "invalid JSON" }, { status: 400 });
+ }
+ const data = createSchema.parse(body);📝 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
| const body = await request.json(); | |
| const data = createSchema.parse(body); | |
| let body: unknown; | |
| try { | |
| body = await request.json(); | |
| } catch { | |
| return NextResponse.json({ error: "invalid JSON" }, { status: 400 }); | |
| } | |
| const data = createSchema.parse(body); |
🤖 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 `@src/app/api/v1/compositions/route.ts` around lines 41 - 42, Guard the
request.json() call in the route handler so malformed JSON SyntaxErrors return a
400 response instead of reaching the generic 500 catch path. Keep
createSchema.parse validation and existing handling unchanged for successfully
parsed bodies and other errors.
Comment on lines
+121
to
+130
| const compositions = await db.composition.findMany({ | ||
| where: { organizationId }, | ||
| orderBy: { createdAt: "desc" }, | ||
| include: { | ||
| stages: { orderBy: { order: "asc" } }, | ||
| _count: { select: { executions: true } }, | ||
| }, | ||
| }); | ||
|
|
||
| return NextResponse.json({ compositions }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Add pagination to the GET list endpoint.
GET /api/v1/compositions returns all compositions for an organization without any skip/take pagination, while the sibling history endpoint ([id]/history/route.ts) already implements page/limit pagination with clamping. As organizations grow, this unbounded query will degrade response times and memory usage.
⚡ Proposed fix: add pagination consistent with the history endpoint
export async function GET(request: NextRequest) {
const log = requestLogger(request.headers.get("x-request-id"));
try {
const session = await auth();
if (!session?.user) {
return NextResponse.json({ error: "unauthorized" }, { status: 401 });
}
const organizationId = await resolveUserOrganizationId(session.user.id);
if (!organizationId) {
return NextResponse.json({ error: "no_organization" }, { status: 403 });
}
const { searchParams } = new URL(request.url);
const requestedOrgId = searchParams.get("organizationId");
if (requestedOrgId && requestedOrgId !== organizationId) {
return NextResponse.json({ error: "organization_mismatch" }, { status: 403 });
}
- const compositions = await db.composition.findMany({
- where: { organizationId },
- orderBy: { createdAt: "desc" },
- include: {
- stages: { orderBy: { order: "asc" } },
- _count: { select: { executions: true } },
- },
- });
+ const page = Math.max(1, parseInt(searchParams.get("page") ?? "1", 10) || 1);
+ const limit = Math.min(100, Math.max(1, parseInt(searchParams.get("limit") ?? "20", 10) || 20));
+
+ const [total, compositions] = await Promise.all([
+ db.composition.count({ where: { organizationId } }),
+ db.composition.findMany({
+ where: { organizationId },
+ orderBy: { createdAt: "desc" },
+ skip: (page - 1) * limit,
+ take: limit,
+ include: {
+ stages: { orderBy: { order: "asc" } },
+ _count: { select: { executions: true } },
+ },
+ }),
+ ]);
- return NextResponse.json({ compositions });
+ return NextResponse.json({
+ compositions,
+ page,
+ limit,
+ total,
+ totalPages: Math.ceil(total / limit),
+ });
} catch (error) {
log.error({ op: "composition.list", err: error }, "failed to list compositions");
return NextResponse.json({ error: "server_error" }, { status: 500 });
}
}📝 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
| const compositions = await db.composition.findMany({ | |
| where: { organizationId }, | |
| orderBy: { createdAt: "desc" }, | |
| include: { | |
| stages: { orderBy: { order: "asc" } }, | |
| _count: { select: { executions: true } }, | |
| }, | |
| }); | |
| return NextResponse.json({ compositions }); | |
| const page = Math.max(1, parseInt(searchParams.get("page") ?? "1", 10) || 1); | |
| const limit = Math.min(100, Math.max(1, parseInt(searchParams.get("limit") ?? "20", 10) || 20)); | |
| const [total, compositions] = await Promise.all([ | |
| db.composition.count({ where: { organizationId } }), | |
| db.composition.findMany({ | |
| where: { organizationId }, | |
| orderBy: { createdAt: "desc" }, | |
| skip: (page - 1) * limit, | |
| take: limit, | |
| include: { | |
| stages: { orderBy: { order: "asc" } }, | |
| _count: { select: { executions: true } }, | |
| }, | |
| }), | |
| ]); | |
| return NextResponse.json({ | |
| compositions, | |
| page, | |
| limit, | |
| total, | |
| totalPages: Math.ceil(total / limit), | |
| }); |
🤖 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 `@src/app/api/v1/compositions/route.ts` around lines 121 - 130, Update the GET
compositions handler around db.composition.findMany to support page/limit
pagination consistent with the sibling history endpoint: parse the query
parameters, clamp them to the established valid bounds, and apply the resulting
skip and take values to the ordered query. Preserve the organization filter,
ordering, included stages/count, and JSON response shape.
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.
Description
Type of Change
Please don't edit
prompts.csvdirectly!Instead, visit prompts.chat and:
This ensures proper attribution, formatting, and keeps the repository in sync. You'll also appear on the Contributors page!
Additional Notes
Summary
DATABASE_URL.