Claude/enable openrouter caching m6yia#27
Merged
epavanello merged 5 commits intomainfrom Feb 18, 2026
Merged
Conversation
Move the dynamic `## Project state` block out of the system prompt and
into a dedicated second message. This keeps the system prompt static so
OpenRouter can cache it across requests:
- Auto-caching models (Moonshot AI, OpenAI-compatible): the unchanged
system prompt prefix is cached automatically with no extra config.
- Anthropic-compatible models: a `cacheControl: { type: 'ephemeral' }`
marker on the project-state user message tells OpenRouter to cache
everything before that point (i.e. the system prompt).
The project state is now prepended as a user+assistant exchange before
the real conversation, keeping full context available to the model while
ensuring the large, expensive system prompt is only processed once per
cache window instead of on every request.
https://claude.ai/code/session_01BUGedtPGoaoXbYcQ9Getdy
📝 WalkthroughWalkthroughAdds typed user roles and DB enum for user.role, admin-only stats endpoint and dashboard, removes temporary layer-ID tracking from AI operations, updates AI prompts and chat UI behavior, and bumps dependencies plus adds date-fns. Changes
Sequence DiagramsequenceDiagram
participant Client as Client (Browser)
participant Auth as Auth Hook
participant AdminPage as Admin Dashboard
participant Guard as checkRole
participant AdminAPI as getAdminStats
participant DB as Database
Client->>Auth: Request page /session info
Auth->>Auth: Read session.user, set event.locals.user.role ('user' default)
Client->>AdminPage: Navigate to /admin (from,to)
AdminPage->>Guard: checkRole('admin')
Guard->>Auth: read event.locals.user.role
alt role === 'admin'
Guard-->>AdminPage: allow
else
Guard-->>Client: redirect to /
end
AdminPage->>AdminAPI: getAdminStats({from,to})
AdminAPI->>DB: query users, projects, ai_usage_log (date range)
DB-->>AdminAPI: result rows
AdminAPI->>AdminAPI: aggregate per-user, per-model, compute costs
AdminAPI-->>AdminPage: stats array
AdminPage-->>Client: render dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/components/ai/ai-chat.svelte (1)
254-254:⚠️ Potential issue | 🟠 Major
submittedstate is inconsistently handled — user can send a second message while waiting for a response.The Stop button appears when
status === 'streaming' || status === 'submitted'(line 259), correctly covering both waiting states. However, the textareadisabledcondition at line 254 only covers'streaming', and theonSubmitguard at line 160 also only checkschat.status === 'streaming'. During thesubmittedwindow (message sent, server hasn't responded yet):
- The textarea remains enabled — user can type.
- Pressing Enter calls
onSubmit, which passes the guard becausestatus !== 'streaming'.chat.sendMessage(...)fires a second time.The one-liner fix on the
disabledprop is sufficient:🐛 Proposed fix
- disabled={chat.status === 'streaming' || projectStore.isRecording} + disabled={chat.status === 'streaming' || chat.status === 'submitted' || projectStore.isRecording}Alternatively (belt-and-suspenders), also update the
onSubmitguard:- if (!prompt.current.trim() || chat.status === 'streaming') return; + if (!prompt.current.trim() || chat.status === 'streaming' || chat.status === 'submitted') return;Also applies to: 259-273
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/ai/ai-chat.svelte` at line 254, The textarea is still enabled during the 'submitted' state allowing duplicate sends; update the disabled prop check and the onSubmit guard to treat chat.status === 'submitted' the same as 'streaming' (i.e., disable input and prevent submission when chat.status is 'streaming' || 'submitted' or when projectStore.isRecording), and ensure any UI controls like the Stop button logic (which already checks 'submitted') remain consistent with the new guard so chat.sendMessage cannot be called while status is 'submitted' or 'streaming'.
🧹 Nitpick comments (11)
src/lib/components/ai/tool-part.svelte (1)
16-17: Loose type assertion forerrorTextaccess.The cast
(tool as { errorText?: string }).errorTextdiscards all other type information. A narrower approach would preserve type safety:- const errorText = $derived(isError ? (tool as { errorText?: string }).errorText : undefined); + const errorText = $derived(isError && 'errorText' in tool ? (tool.errorText as string) : undefined);This is minor since
errorTextis only accessed when the state is alreadyoutput-error, so the property should exist at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/ai/tool-part.svelte` around lines 16 - 17, The code uses a broad cast (tool as { errorText?: string }).errorText which discards other type info; instead add a narrow discriminated type or type-guard and use it when deriving errorText: define an interface like OutputErrorTool { state: 'output-error'; errorText?: string } and a type predicate function isOutputError(tool): tool is OutputErrorTool (or use an inline check `tool.state === 'output-error'` in the derived callback with a cast to OutputErrorTool), then access errorText only after that guard so the access is type-safe in the errorText const.src/app.d.ts (1)
3-4: Import ordering: external package should precede internal alias.
better-auth(external package) is currently imported after$lib/roles(internal), which inverts the prescribed order.♻️ Proposed fix
-import type { UserRole } from '$lib/roles'; import type { Session, User } from 'better-auth'; +import type { UserRole } from '$lib/roles';As per coding guidelines: "Organize imports in order: External packages → SvelteKit → Internal lib → Relative imports."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app.d.ts` around lines 3 - 4, Reorder the imports so external packages come first: move the import of Session and User from 'better-auth' above the internal alias import of UserRole from '$lib/roles'; ensure the top-level imports now read external ('better-auth') → SvelteKit (if any) → internal ('$lib/roles') → relative, keeping the same imported symbols (Session, User, UserRole) intact.src/lib/functions/projects.remote.ts (1)
81-90: Admin gets view-only access to private projects (no edit) — confirm intent.
canEdit: isOwnermeans an admin viewing another user's private project getscanEdit: false. This is likely intentional (read-only moderation), but worth a comment to document the decision.Separately, the hardcoded
'admin'string is inconsistent with theUserRoletype fromsrc/lib/roles.ts. Consider importing and reusing the constant.♻️ Optional: remove string literal via import
+import type { UserRole } from '$lib/roles'; ... - const isAdmin = locals.user?.role === 'admin'; + const isAdmin = (locals.user?.role as UserRole) === 'admin';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/functions/projects.remote.ts` around lines 81 - 90, The current logic sets isAdmin via locals.user?.role === 'admin' and sets canEdit: isOwner which makes admins read-only on others' private projects—add an inline comment near the isAdmin and canEdit assignments (around isAdmin, canEdit and isOwner in projects.remote.ts) stating that admins intentionally have view-only access to private projects; then replace the hardcoded 'admin' string with the canonical role constant from your roles module (import UserRole or the exported ADMIN symbol from src/lib/roles.ts and use locals.user?.role === UserRole.ADMIN) so the role check is type-safe and consistent.src/lib/server/db/schema/auth.ts (1)
14-14: Optional: use a database-levelpgEnumto constrain valid role values.The plain
textcolumn accepts any string. A Postgres enum would reject invalid roles at the DB level.♻️ Proposed refactor
-import { pgTable, text, timestamp, boolean } from 'drizzle-orm/pg-core'; +import { pgTable, text, timestamp, boolean, pgEnum } from 'drizzle-orm/pg-core'; + +export const userRoleEnum = pgEnum('user_role', ['user', 'admin']); export const user = pgTable('user', { ... - role: text('role').default('user').notNull() + role: userRoleEnum('role').default('user').notNull() });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/db/schema/auth.ts` at line 14, Change the plain text role column to use a Postgres enum so invalid roles are rejected at the DB level: replace the current role definition (role: text('role').default('user').notNull()) with a pgEnum-based column (create a pgEnum type with the allowed values like 'user' and 'admin' and reference it for the role column) and update the schema/migrations accordingly so the enum type is created before the table/column is created and defaults to 'user'. Ensure the symbols to modify are the role column definition in auth.ts and the new pgEnum type declaration used by that column.src/lib/ai/ai-operations.svelte.ts (3)
104-113:executeRemoveLayercallsgetContext()twice — second call is redundant.Line 107 constructs a new
MutationContextonly to access.project.layers, which is the same reference asprojectStore.state.layers.✏️ Suggested fix
const result = mutateRemoveLayer(getContext(projectStore), input); if (result.success) { if ( projectStore.selectedLayerId && - getContext(projectStore).project.layers.find((l) => l.id === projectStore.selectedLayerId) === - undefined + projectStore.state.layers.find((l) => l.id === projectStore.selectedLayerId) === undefined ) { projectStore.selectedLayerId = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ai/ai-operations.svelte.ts` around lines 104 - 113, In executeRemoveLayer the second call to getContext(projectStore) is redundant; capture the MutationContext (or reuse projectStore.state) once and reference its project.layers when checking for the selectedLayerId, e.g. store the result of getContext(projectStore) in a local variable (or directly use projectStore.state.layers) and use that variable instead of calling getContext(projectStore) a second time so you don't construct a new MutationContext unnecessarily.
1-6: Stale file-header comment — "layer ID tracking" was removed in this PR.Line 4 still says "Handles progressive tool execution with layer ID tracking across tool calls." The layer-tracking (layerIdMap, layerCreationIndex, resetLayerTracking) was deleted in this PR, so this description is now misleading.
✏️ Suggested fix
/** * AI Tool Executor - Client-side execution of AI tool calls * - * Handles progressive tool execution with layer ID tracking across tool calls. - * Refactored to use shared 'mutations.ts' logic. + * Executes AI tool calls on the client-side by delegating to shared mutation + * logic from 'mutations.ts'. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ai/ai-operations.svelte.ts` around lines 1 - 6, The file header in ai-operations.svelte.ts is stale: remove or replace the sentence "Handles progressive tool execution with layer ID tracking across tool calls." because the layer-tracking logic (symbols layerIdMap, layerCreationIndex, resetLayerTracking) was removed in this PR; update the header to accurately describe current behavior (e.g., client-side AI tool execution using shared mutations.ts logic) or simply delete the obsolete sentence so the header no longer references layer ID tracking.
41-47: Remove the now-empty "Layer ID Tracking" section headers.After removing
layerIdMap,layerCreationIndex, andresetLayerTracking, lines 41–46 are two consecutive empty section-comment blocks with no code between them. They add noise and hint at removed functionality.✏️ Suggested fix
-// ============================================ -// Layer ID Tracking -// ============================================ - -// ============================================ -// Context Helper -// ============================================ +// ============================================ +// Context Helper +// ============================================🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ai/ai-operations.svelte.ts` around lines 41 - 47, Remove the leftover empty section comment blocks labeled "Layer ID Tracking" (the two consecutive comment headers currently between the Context Helper section) since layerIdMap, layerCreationIndex, and resetLayerTracking were removed; delete those header comment lines so there's no empty noise between the surrounding sections (search for the exact text "Layer ID Tracking" to locate and remove the two comment blocks).src/lib/server/auth.ts (1)
65-77: Use an array type forroleto enforce enum-level validation at the better-auth layer.The better-auth docs recommend passing an array of allowed values as the
typefor enum-constrained additional fields — this makes better-auth validate the stored value against the allowed set. Usingtype: 'string'leaves the field unconstrained at the application level.Reusing the
userRolesconstant from$lib/roleskeeps the constraint synchronized:♻️ Suggested fix
+import { userRoles } from '$lib/roles'; // ... user: { additionalFields: { role: { - type: 'string', + type: userRoles, required: true, defaultValue: 'user', input: false } },The
deleteUser: { enabled: true }configuration is safe—all foreign-key relationships toproject,asset,session,account,aiUserUnlock, andaiUsageLogtables are configured withonDelete: 'cascade'in the Drizzle schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/server/auth.ts` around lines 65 - 77, Change the additionalFields.role configuration to use an enum array instead of a plain string so better-auth can validate the stored value: replace type: 'string' with type: userRoles (the exported array constant from $lib/roles), keep required: true, defaultValue: 'user' and input: false, and import or reference userRoles at the top of the module; ensure the symbol names are exactly userRoles and the additionalFields.user.role block remains otherwise unchanged.src/routes/(app)/chat/+server.ts (2)
226-234: Appending raw project JSON as asystemmessage may not be cacheable and could be large.The system prompt was separated from the project state for caching (per the comment on Line 154). However,
JSON.stringify(project)is appended as a final system message on every request with the full project payload (layers, keyframes, etc.). This is correct for giving the AI current project state, but note:
- This message changes on every request (since project state evolves), so it won't benefit from OpenRouter caching — which is the intended design.
- For large projects, this could be a significant token cost. Consider whether a trimmed projection (e.g., only layer names/ids and top-level properties) would suffice for context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(app)/chat/+server.ts around lines 226 - 234, You are appending the entire project JSON as a system message in the agent.stream call (messages array), which is large and defeats caching; instead build and send a compact project summary. Replace JSON.stringify(project) in the system message with JSON.stringify(trimmedProject) (or a call like getProjectSummary(project)), where trimmedProject includes only necessary top-level fields and minimal layer info (e.g., layer ids, names, types, and any top-level project metadata or timestamps) and excludes heavy fields like frames/keyframes/bitmaps; implement the trimming logic in a helper (e.g., getProjectSummary or buildTrimmedProject) and use that helper in place of JSON.stringify(project) when constructing the system message passed to agent.stream.
212-224:enableCacheControloverwrites any existingproviderOptionson messages.The spread
{ ...message, providerOptions: { openrouter: ... } }replaces the entireproviderOptionsobject rather than merging. IfconvertToModelMessagesever attaches provider options, they'll be silently lost. Consider a shallow merge:Suggested merge
return { ...message, providerOptions: { + ...message.providerOptions, openrouter: { cacheControl: { type: 'ephemeral' /* ttl: '1h' */ } } } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(app)/chat/+server.ts around lines 212 - 224, enableCacheControl currently replaces any existing providerOptions on each ModelMessage by setting providerOptions to a new object; instead shallow-merge into the existing message.providerOptions and only set/merge the openrouter key with cacheControl. Update the enableCacheControl function to read existing message.providerOptions (if any), copy its keys, then assign/merge an openrouter object containing cacheControl so other provider options are preserved; reference the message variable and providerOptions/openrouter/cacheControl keys inside enableCacheControl to locate where to change the spread.src/lib/functions/admin.remote.ts (1)
89-91: In-memory re-sort duplicates work already done by the DB.Line 65 orders projects by
updatedAtascending at the DB level, then line 90 re-sorts descending in JS. Push the sort direction into the query to avoid redundant work:Suggested fix
import { eq, sql, count, and, gte, lte } from 'drizzle-orm'; +import { desc } from 'drizzle-orm'; ... .from(project) - .orderBy(project.updatedAt); + .orderBy(desc(project.updatedAt)); ... - projects: (projectsByUser.get(u.id) ?? []).sort( - (a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime() - ) + projects: projectsByUser.get(u.id) ?? []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/functions/admin.remote.ts` around lines 89 - 91, The code is re-sorting projects in memory after the DB already sorts them ascending; update the DB query that builds projectsByUser to order by updatedAt descending instead of ascending (so the DB returns newest-first) and remove the client-side sort applied to projects: (projectsByUser.get(u.id) ?? []).sort(...) in admin.remote.ts (referencing projectsByUser and the projects property) so you avoid redundant JS sorting and rely on the DB-ordered results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/ai/system-prompt.ts`:
- Around line 99-101: The system prompt refers to "PROJECT STATE" but
buildSystemPrompt() deliberately excludes project state for caching, so update
the prompt and tooling to remove broken references or provide alternatives:
either inject the required project state into buildSystemPrompt() so
pre-existing layer and keyframe IDs are available to the model, or (preferred)
update the system prompt text generated by buildSystemPrompt() to document that
layer names can be used with animate_layer and edit_layer and add a clear
alternative for keyframe operations (update_keyframe, remove_keyframe) such as
exposing keyframe lookup via a new tool or allowing keyframe lookup by
(layerId|layerName + timestamp/index) as defined in schemas.ts; ensure the
change references buildSystemPrompt(), animate_layer, edit_layer,
update_keyframe, remove_keyframe and the schema definitions in schemas.ts so the
AI can discover IDs or use the documented fallbacks.
In `@src/lib/functions/admin.remote.ts`:
- Around line 55-65: The query that populates allProjects (the
db.select(...).from(project) call) is unbounded and will load the entire project
table; update this query to use pagination or a filter: add parameters for
page/limit or a cursor and apply .limit(pageSize) plus .offset(page*pageSize)
(or a cursor WHERE clause) and/or a WHERE filter (e.g., project.updatedAt >=
cutoffDate or project.userId IN activeUsers) so only a bounded subset is
returned; ensure the function that calls this code (the handler around
allProjects) accepts and validates the paging/filter inputs and returns
total/next-cursor metadata for clients.
- Around line 20-31: The projectCount in the userStats query (selecting
projectCount via sql`cast(count(distinct ${project.id}) as int)`) is currently
counting all-time projects because there is no date restriction; to fix, accept
start/end date parameters into the function and apply a date-range filter to the
projects when counting (either by using a FILTER/WHERE on ${project.createdAt}
BETWEEN start and end inside the aggregation or by adding the date condition to
the LEFT JOIN), updating the userStats query accordingly so projectCount
reflects the requested range; alternatively, if you prefer not to change the
query signature, rename the UI label to make clear the count is all-time.
In `@src/lib/functions/auth.remote.ts`:
- Line 1: Remove the unused `command` import from the import statement that
currently reads `import { command, form, getRequestEvent, query } from
'$app/server';` in the auth.remote.ts file; update the import to only include
the referenced symbols (`form`, `getRequestEvent`, `query`) so ESLint no longer
reports an unused import.
- Around line 65-70: In checkRole (the exported function using query and
getRequestEvent), remove the inconsistent "throw" before redirect and call
redirect(303, '/') directly when locals.user?.role !== role (i.e., replace
"throw redirect(303, '/')" with "redirect(303, '/')") so the redirect usage
matches the other redirects in this file.
In `@src/routes/`(marketing)/admin/+page.svelte:
- Around line 16-18: The computed derived values assume adminStats is always an
array and will throw if undefined; update the derived calculations (totalUsers,
totalProjects, totalCostCents) to guard against non-array values by using a safe
default (e.g., Array.isArray(adminStats) ? adminStats : []) or fallback values
before calling .length or .reduce, and add a simple loading/error state in the
template where adminStats is used so the UI checks for falsy/adminStats errors
before rendering the derived values; touch the derived declarations (totalUsers,
totalProjects, totalCostCents) and the template that references adminStats to
implement these guards.
- Around line 70-73: The page currently relies on getAdminStats() (which calls
checkRole('admin')) at data-fetch time, allowing the page to render before
access is validated; to fix, add a +page.server.ts for this route and implement
an exported load function that calls your existing checkRole('admin') (or wraps
getAdminStats()) to perform server-side role validation before the page renders
and redirect/throw if unauthorized. Also add audit logging inside that
server-side load (or a dedicated audit function) to record the acting user
id/email and timestamp whenever the admin dashboard is accessed (use the same
identity source used by checkRole, e.g., the request/session/user object) so
accesses are logged for compliance. Ensure the client +page.svelte uses only
data returned from the server load and remove any client-side-only role checks
to avoid duplicate/late validation.
---
Outside diff comments:
In `@src/lib/components/ai/ai-chat.svelte`:
- Line 254: The textarea is still enabled during the 'submitted' state allowing
duplicate sends; update the disabled prop check and the onSubmit guard to treat
chat.status === 'submitted' the same as 'streaming' (i.e., disable input and
prevent submission when chat.status is 'streaming' || 'submitted' or when
projectStore.isRecording), and ensure any UI controls like the Stop button logic
(which already checks 'submitted') remain consistent with the new guard so
chat.sendMessage cannot be called while status is 'submitted' or 'streaming'.
---
Nitpick comments:
In `@src/app.d.ts`:
- Around line 3-4: Reorder the imports so external packages come first: move the
import of Session and User from 'better-auth' above the internal alias import of
UserRole from '$lib/roles'; ensure the top-level imports now read external
('better-auth') → SvelteKit (if any) → internal ('$lib/roles') → relative,
keeping the same imported symbols (Session, User, UserRole) intact.
In `@src/lib/ai/ai-operations.svelte.ts`:
- Around line 104-113: In executeRemoveLayer the second call to
getContext(projectStore) is redundant; capture the MutationContext (or reuse
projectStore.state) once and reference its project.layers when checking for the
selectedLayerId, e.g. store the result of getContext(projectStore) in a local
variable (or directly use projectStore.state.layers) and use that variable
instead of calling getContext(projectStore) a second time so you don't construct
a new MutationContext unnecessarily.
- Around line 1-6: The file header in ai-operations.svelte.ts is stale: remove
or replace the sentence "Handles progressive tool execution with layer ID
tracking across tool calls." because the layer-tracking logic (symbols
layerIdMap, layerCreationIndex, resetLayerTracking) was removed in this PR;
update the header to accurately describe current behavior (e.g., client-side AI
tool execution using shared mutations.ts logic) or simply delete the obsolete
sentence so the header no longer references layer ID tracking.
- Around line 41-47: Remove the leftover empty section comment blocks labeled
"Layer ID Tracking" (the two consecutive comment headers currently between the
Context Helper section) since layerIdMap, layerCreationIndex, and
resetLayerTracking were removed; delete those header comment lines so there's no
empty noise between the surrounding sections (search for the exact text "Layer
ID Tracking" to locate and remove the two comment blocks).
In `@src/lib/components/ai/tool-part.svelte`:
- Around line 16-17: The code uses a broad cast (tool as { errorText?: string
}).errorText which discards other type info; instead add a narrow discriminated
type or type-guard and use it when deriving errorText: define an interface like
OutputErrorTool { state: 'output-error'; errorText?: string } and a type
predicate function isOutputError(tool): tool is OutputErrorTool (or use an
inline check `tool.state === 'output-error'` in the derived callback with a cast
to OutputErrorTool), then access errorText only after that guard so the access
is type-safe in the errorText const.
In `@src/lib/functions/admin.remote.ts`:
- Around line 89-91: The code is re-sorting projects in memory after the DB
already sorts them ascending; update the DB query that builds projectsByUser to
order by updatedAt descending instead of ascending (so the DB returns
newest-first) and remove the client-side sort applied to projects:
(projectsByUser.get(u.id) ?? []).sort(...) in admin.remote.ts (referencing
projectsByUser and the projects property) so you avoid redundant JS sorting and
rely on the DB-ordered results.
In `@src/lib/functions/projects.remote.ts`:
- Around line 81-90: The current logic sets isAdmin via locals.user?.role ===
'admin' and sets canEdit: isOwner which makes admins read-only on others'
private projects—add an inline comment near the isAdmin and canEdit assignments
(around isAdmin, canEdit and isOwner in projects.remote.ts) stating that admins
intentionally have view-only access to private projects; then replace the
hardcoded 'admin' string with the canonical role constant from your roles module
(import UserRole or the exported ADMIN symbol from src/lib/roles.ts and use
locals.user?.role === UserRole.ADMIN) so the role check is type-safe and
consistent.
In `@src/lib/server/auth.ts`:
- Around line 65-77: Change the additionalFields.role configuration to use an
enum array instead of a plain string so better-auth can validate the stored
value: replace type: 'string' with type: userRoles (the exported array constant
from $lib/roles), keep required: true, defaultValue: 'user' and input: false,
and import or reference userRoles at the top of the module; ensure the symbol
names are exactly userRoles and the additionalFields.user.role block remains
otherwise unchanged.
In `@src/lib/server/db/schema/auth.ts`:
- Line 14: Change the plain text role column to use a Postgres enum so invalid
roles are rejected at the DB level: replace the current role definition (role:
text('role').default('user').notNull()) with a pgEnum-based column (create a
pgEnum type with the allowed values like 'user' and 'admin' and reference it for
the role column) and update the schema/migrations accordingly so the enum type
is created before the table/column is created and defaults to 'user'. Ensure the
symbols to modify are the role column definition in auth.ts and the new pgEnum
type declaration used by that column.
In `@src/routes/`(app)/chat/+server.ts:
- Around line 226-234: You are appending the entire project JSON as a system
message in the agent.stream call (messages array), which is large and defeats
caching; instead build and send a compact project summary. Replace
JSON.stringify(project) in the system message with
JSON.stringify(trimmedProject) (or a call like getProjectSummary(project)),
where trimmedProject includes only necessary top-level fields and minimal layer
info (e.g., layer ids, names, types, and any top-level project metadata or
timestamps) and excludes heavy fields like frames/keyframes/bitmaps; implement
the trimming logic in a helper (e.g., getProjectSummary or buildTrimmedProject)
and use that helper in place of JSON.stringify(project) when constructing the
system message passed to agent.stream.
- Around line 212-224: enableCacheControl currently replaces any existing
providerOptions on each ModelMessage by setting providerOptions to a new object;
instead shallow-merge into the existing message.providerOptions and only
set/merge the openrouter key with cacheControl. Update the enableCacheControl
function to read existing message.providerOptions (if any), copy its keys, then
assign/merge an openrouter object containing cacheControl so other provider
options are preserved; reference the message variable and
providerOptions/openrouter/cacheControl keys inside enableCacheControl to locate
where to change the spread.
Comment on lines
+20
to
+31
| const userStats = await db | ||
| .select({ | ||
| id: user.id, | ||
| name: user.name, | ||
| email: user.email, | ||
| createdAt: user.createdAt, | ||
| projectCount: sql<number>`cast(count(distinct ${project.id}) as int)` | ||
| }) | ||
| .from(user) | ||
| .leftJoin(project, eq(project.userId, user.id)) | ||
| .groupBy(user.id, user.name, user.email, user.createdAt) | ||
| .orderBy(user.createdAt); |
There was a problem hiding this comment.
projectCount includes all-time projects, but the UI title says "in range".
The project count via LEFT JOIN on the project table has no date filter, so it reflects all-time totals. Meanwhile, the admin page summary on line 49 of +page.svelte says "{totalProjects} projects … total AI spend in range". The juxtaposition may confuse admins into thinking the project count is also filtered by the date range. Consider either filtering projects by date range or clarifying the label in the UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/functions/admin.remote.ts` around lines 20 - 31, The projectCount in
the userStats query (selecting projectCount via sql`cast(count(distinct
${project.id}) as int)`) is currently counting all-time projects because there
is no date restriction; to fix, accept start/end date parameters into the
function and apply a date-range filter to the projects when counting (either by
using a FILTER/WHERE on ${project.createdAt} BETWEEN start and end inside the
aggregation or by adding the date condition to the LEFT JOIN), updating the
userStats query accordingly so projectCount reflects the requested range;
alternatively, if you prefer not to change the query signature, rename the UI
label to make clear the count is all-time.
Comment on lines
+55
to
+65
| const allProjects = await db | ||
| .select({ | ||
| id: project.id, | ||
| name: project.name, | ||
| isPublic: project.isPublic, | ||
| userId: project.userId, | ||
| updatedAt: project.updatedAt, | ||
| views: project.views | ||
| }) | ||
| .from(project) | ||
| .orderBy(project.updatedAt); |
There was a problem hiding this comment.
Unbounded project query loads every project in the database.
allProjects fetches every row from the project table with no WHERE clause or LIMIT. As the number of projects grows, this will increasingly tax memory and transfer time. Consider adding pagination or, at minimum, a filter (e.g., only projects belonging to users with activity in the date range).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/functions/admin.remote.ts` around lines 55 - 65, The query that
populates allProjects (the db.select(...).from(project) call) is unbounded and
will load the entire project table; update this query to use pagination or a
filter: add parameters for page/limit or a cursor and apply .limit(pageSize)
plus .offset(page*pageSize) (or a cursor WHERE clause) and/or a WHERE filter
(e.g., project.updatedAt >= cutoffDate or project.userId IN activeUsers) so only
a bounded subset is returned; ensure the function that calls this code (the
handler around allProjects) accepts and validates the paging/filter inputs and
returns total/next-cursor metadata for clients.
Comment on lines
+16
to
+18
| const totalUsers = $derived(adminStats.length); | ||
| const totalProjects = $derived(adminStats.reduce((s, u) => s + u.projectCount, 0)); | ||
| const totalCostCents = $derived(adminStats.reduce((s, u) => s + u.totalCostCents, 0)); |
There was a problem hiding this comment.
Derived values assume adminStats is always an array — no loading/error state.
If adminStats is undefined (e.g., during re-fetch or on error), .length, .reduce() on lines 16-18 and the template on line 52 will throw. Consider adding a guard or default value, and handling loading/error states for a better UX.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(marketing)/admin/+page.svelte around lines 16 - 18, The computed
derived values assume adminStats is always an array and will throw if undefined;
update the derived calculations (totalUsers, totalProjects, totalCostCents) to
guard against non-array values by using a safe default (e.g.,
Array.isArray(adminStats) ? adminStats : []) or fallback values before calling
.length or .reduce, and add a simple loading/error state in the template where
adminStats is used so the UI checks for falsy/adminStats errors before rendering
the derived values; touch the derived declarations (totalUsers, totalProjects,
totalCostCents) and the template that references adminStats to implement these
guards.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/lib/components/ai/tool-part.svelte (1)
5-12:OutputErrorToolis redundant — use the SDK's own discriminated union type for narrowing.
ToolUIPartis already a discriminated union, and theoutput-errorvariant carrieserrorText: stringas a required field. The localOutputErrorToolinterface re-declareserrorTextas optional (errorText?: string), weakening the type guarantee, andisOutputErrorsimply re-checksstate === 'output-error'— identical toisErrorderived on line 25.The
errorTextderived value can be obtained by narrowing directly with the existingisErrorflag:♻️ Proposed simplification
- interface OutputErrorTool { - state: 'output-error'; - errorText?: string; - } - - function isOutputError(t: unknown): t is OutputErrorTool { - return typeof t === 'object' && t !== null && (t as OutputErrorTool).state === 'output-error'; - }- const isError = $derived(tool.state === 'output-error'); - const errorText = $derived(isOutputError(tool) ? tool.errorText : undefined); + const isError = $derived(tool.state === 'output-error'); + const errorText = $derived(isError ? (tool as { errorText: string }).errorText : undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/ai/tool-part.svelte` around lines 5 - 12, Remove the redundant local OutputErrorTool interface and its type guard isOutputError, and instead use the existing ToolUIPart discriminated union and its isError narrowing to access errorText; update any code that calls isOutputError or references OutputErrorTool to rely on isError and treat errorText as required per the SDK variant. Specifically, delete OutputErrorTool and isOutputError, replace checks like isOutputError(t) with the existing isError(t) pattern, and read errorText from the narrowed ToolUIPart value (e.g., (tool).errorText) assuming the SDK's 'output-error' variant provides it as a required string.src/routes/(app)/chat/+server.ts (1)
212-230:enableCacheControlcan be a module-level helper.This pure function closes over nothing from the
POSThandler — a new function object is allocated on every request. Extracting it to module scope is a clean improvement.♻️ Proposed fix
+function enableCacheControl(messages: ModelMessage[]) { + return messages.map((message) => { + if (typeof message.content !== 'string') { + return message; + } + return { + ...message, + providerOptions: { + ...message.providerOptions, + openrouter: { + ...(message.providerOptions?.openrouter as Record<string, unknown> | undefined), + cacheControl: { type: 'ephemeral' } + } + } + }; + }); +} + export const POST: RequestHandler = async ({ request, locals }) => { ... - function enableCacheControl(messages: ModelMessage[]) { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/`(app)/chat/+server.ts around lines 212 - 230, Move the pure helper function enableCacheControl out of the POST handler to module scope so it's not reallocated per request; specifically, define enableCacheControl(messages: ModelMessage[]) at top-level (outside the request handler) and keep its existing implementation that shallow-merges into message.providerOptions/openrouter.cacheControl. Update any local references in the POST handler to call the module-level enableCacheControl and ensure ModelMessage type is imported/available at module scope if needed.drizzle/0009_user_role_enum.sql (1)
1-7: Verify no non-enum role values exist before applying this migration.The
USING "role"::"public"."user_role"cast will fail at runtime if any row'srolecolumn contains a value outside('user', 'admin')— includingNULLif the column was previously nullable. Before deploying to production, confirm with:SELECT DISTINCT role FROM "user" WHERE role NOT IN ('user', 'admin') OR role IS NULL;If any rows are returned, backfill them before running the migration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@drizzle/0009_user_role_enum.sql` around lines 1 - 7, Before applying the ALTER TABLE that casts role to the enum, verify there are no invalid or NULL role values by running the suggested DISTINCT check and, if any rows are returned, backfill them to one of the allowed values ('user' or 'admin') or a desired default, then re-run the migration; specifically, check results for the current "user"."role" values referenced by CREATE TYPE "public"."user_role" and the ALTER TABLE ... USING "role"::"public"."user_role" cast, update any offending rows to an allowed value, and only then apply the ALTER TABLE and ALTER COLUMN ... SET DEFAULT 'user'::"public"."user_role".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/lib/ai/system-prompt.ts`:
- Around line 60-63: The system prompt references "shown in **PROJECT STATE**"
but the injected JSON (via chat/+server.ts using JSON.stringify(project)) lacks
that explicit label; update chat/+server.ts to prefix the injected system
message with a short label (e.g., "# PROJECT STATE\n" or similar) before the
JSON.stringify(project) output, and then update the wording in
src/lib/ai/system-prompt.ts (the sentence that says "shown in **PROJECT
STATE**") to match that exact label so the model reliably recognizes the
injected JSON; ensure the label string and the text in the system prompt use the
same token/casing.
In `@src/lib/functions/auth.remote.ts`:
- Around line 65-70: The checkRole validator currently hardcodes roles with
z.enum(['admin','user']); import the shared userRoles constant and derive the
Zod enum from it instead (e.g., replace z.enum(['admin','user']) with
z.enum(userRoles) or z.enum([...userRoles]) depending on its type) so the
validation stays in sync; update the imports at the top to include userRoles and
adjust the checkRole declaration to use that derived enum.
---
Nitpick comments:
In `@drizzle/0009_user_role_enum.sql`:
- Around line 1-7: Before applying the ALTER TABLE that casts role to the enum,
verify there are no invalid or NULL role values by running the suggested
DISTINCT check and, if any rows are returned, backfill them to one of the
allowed values ('user' or 'admin') or a desired default, then re-run the
migration; specifically, check results for the current "user"."role" values
referenced by CREATE TYPE "public"."user_role" and the ALTER TABLE ... USING
"role"::"public"."user_role" cast, update any offending rows to an allowed
value, and only then apply the ALTER TABLE and ALTER COLUMN ... SET DEFAULT
'user'::"public"."user_role".
In `@src/lib/components/ai/tool-part.svelte`:
- Around line 5-12: Remove the redundant local OutputErrorTool interface and its
type guard isOutputError, and instead use the existing ToolUIPart discriminated
union and its isError narrowing to access errorText; update any code that calls
isOutputError or references OutputErrorTool to rely on isError and treat
errorText as required per the SDK variant. Specifically, delete OutputErrorTool
and isOutputError, replace checks like isOutputError(t) with the existing
isError(t) pattern, and read errorText from the narrowed ToolUIPart value (e.g.,
(tool).errorText) assuming the SDK's 'output-error' variant provides it as a
required string.
In `@src/routes/`(app)/chat/+server.ts:
- Around line 212-230: Move the pure helper function enableCacheControl out of
the POST handler to module scope so it's not reallocated per request;
specifically, define enableCacheControl(messages: ModelMessage[]) at top-level
(outside the request handler) and keep its existing implementation that
shallow-merges into message.providerOptions/openrouter.cacheControl. Update any
local references in the POST handler to call the module-level enableCacheControl
and ensure ModelMessage type is imported/available at module scope if needed.
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.
Summary by CodeRabbit
New Features
UX Improvements
Database
Chores