Cleanup backend hotspots and apply code quality fixes - #701
Conversation
🚀 Preview Deployment ReadyVercel is rebuilding the frontend with the new Convex backend URL. Vercel will post the preview URL automatically. Convex Preview Backend
🤖 Deployed automatically by GitHub Actions |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| } | ||
|
|
||
| function isRecord(value: unknown): value is Record<string, unknown> { | ||
| return typeof value === "object" && value !== null; |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const serverSet = new Set(serverFavorites ?? []); | ||
| const serverSnapshot = serializeFavorites(serverSet); | ||
| const userChanged = activeUserIdRef.current !== convexUserId; | ||
| const serverSnapshotChanged = serverSnapshot !== lastServerSnapshotRef.current; |
There was a problem hiding this comment.
🟡 Medium hooks/use-favorite-models.ts:74
When a user toggles a favorite while serverFavorites is still loading, the optimistic update is lost once the server data arrives. The condition at lines 76-80 uses serverSnapshotChanged (true because lastServerSnapshotRef.current is null when the user first authenticates), which overwrites the pending optimistic state with the stale server snapshot. Consider tracking whether an optimistic update is pending separately from snapshot comparisons, or ensure serverSnapshotChanged alone doesn't trigger a reset when updates are in flight.
- const serverSnapshotChanged = serverSnapshot !== lastServerSnapshotRef.current;
+ const serverSnapshotChanged = lastServerSnapshotRef.current !== null && serverSnapshot !== lastServerSnapshotRef.current;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/hooks/use-favorite-models.ts around line 74:
When a user toggles a favorite while `serverFavorites` is still loading, the optimistic update is lost once the server data arrives. The condition at lines 76-80 uses `serverSnapshotChanged` (true because `lastServerSnapshotRef.current` is `null` when the user first authenticates), which overwrites the pending optimistic state with the stale server snapshot. Consider tracking whether an optimistic update is pending separately from snapshot comparisons, or ensure `serverSnapshotChanged` alone doesn't trigger a reset when updates are in flight.
Evidence trail:
apps/web/src/hooks/use-favorite-models.ts lines 44-47 (applyFavorites sets hasPendingOptimisticUpdateRef.current = markOptimistic, default false), lines 60-67 (early return when serverFavorites undefined leaves lastServerSnapshotRef.current = null), lines 73-81 (the OR condition where serverSnapshotChanged alone triggers applyFavorites), lines 96-97 in toggleFavorite (sets hasPendingOptimisticUpdateRef.current = true).
There was a problem hiding this comment.
🟠 High
openchat/apps/server/convex/promptTemplates.ts
Lines 172 to 174 in a16f4b4
getByCommand queries with the raw args.command (e.g., "TEST" or "test"), but templates are stored with sanitized commands like "/test". The by_command_not_deleted index lookup uses exact string matching, so queries with unformatted commands return null even when a matching template exists. Consider sanitizing the command with sanitizeCommand() before the lookup.
- const templates = await getActiveTemplateByCommand(ctx, userId, args.command);
+ const templates = await getActiveTemplateByCommand(ctx, userId, sanitizeCommand(args.command));🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/convex/promptTemplates.ts around lines 172-174:
`getByCommand` queries with the raw `args.command` (e.g., `"TEST"` or `"test"`), but templates are stored with sanitized commands like `"/test"`. The `by_command_not_deleted` index lookup uses exact string matching, so queries with unformatted commands return `null` even when a matching template exists. Consider sanitizing the command with `sanitizeCommand()` before the lookup.
Evidence trail:
- apps/server/convex/promptTemplates.ts lines 16-30: `sanitizeCommand()` definition - transforms "TEST" to "/test"
- apps/server/convex/promptTemplates.ts line 174: `getByCommand` passes `args.command` directly to `getActiveTemplateByCommand` without sanitization
- apps/server/convex/promptTemplates.ts lines 80-89: `getActiveTemplateByCommand` uses exact string match `.eq("command", command)`
- apps/server/convex/promptTemplates.ts line 205: `create` mutation sanitizes command with `sanitizeCommand(args.command)`
- apps/server/convex/promptTemplates.ts line 237: `create` stores `command: sanitizedCommand` (the sanitized version)
There was a problem hiding this comment.
🟠 High hooks/use-prompt-draft.ts:45
skipNextSaveRef.current is set to true unconditionally when chatId changes, but textInputController.setInput is only called when savedDraft is truthy. When navigating to a chat with no saved draft, skipNextSaveRef remains true even though no programmatic setInput occurred, causing the first user keystroke to be skipped and lost.
// Restore draft on mount or when chatId changes
useEffect(() => {
// Reset restoration flag when chatId changes
hasRestoredRef.current = false;
- skipNextSaveRef.current = true;
const savedDraft = getDraft(chatId);
if (savedDraft) {
+ skipNextSaveRef.current = true;
textInputController.setInput(savedDraft);
}
hasRestoredRef.current = true;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/hooks/use-prompt-draft.ts around lines 45-62:
`skipNextSaveRef.current` is set to `true` unconditionally when `chatId` changes, but `textInputController.setInput` is only called when `savedDraft` is truthy. When navigating to a chat with no saved draft, `skipNextSaveRef` remains `true` even though no programmatic `setInput` occurred, causing the first user keystroke to be skipped and lost.
Evidence trail:
apps/web/src/hooks/use-prompt-draft.ts lines 45-52 (first useEffect sets skipNextSaveRef unconditionally at line 47, but setInput is conditional on savedDraft at lines 50-52), lines 74-82 (second useEffect checks skipNextSaveRef and returns early if true, skipping the save). Commit: REVIEWED_COMMIT
Greptile SummaryThis PR performs a broad code-quality sweep across the backend Convex modules and several frontend hooks. The core server-side changes replace all Key concerns to address before merging:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[editAndRegenerate or retryMessage mutation] --> B[completeActiveAndPendingStreams]
A --> C[softDeleteMessagesAfter]
B --> D[(streamJobs: running/pending to completed)]
C --> E[(messages: createdAt after cutoff gets deletedAt)]
F[chats.remove or chats.removeBulk] --> G[assertOwnsChat]
G --> H[softDeleteChatWithMessages]
H --> I[softDeleteAllMessagesInChat]
H --> J[(chats: deletedAt + messageCount=0)]
I --> E
K[streamExecution.executeStream action] -->|FunctionReference cast| L[backgroundStream.failStream]
K -->|FunctionReference cast| M[backgroundStream.updateStreamContent]
K -->|FunctionReference cast| N[backgroundStream.completeStream]
K -->|FunctionReference cast| O[users.incrementAiUsage]
P[typing API POST and GET] --> Q[parseTypingBody or parseChatId]
Q --> R{valid input?}
R -- no --> S[400 error]
R -- yes --> T[chats.get ownership check]
T --> U[redis typing set or get]
V[useFavoriteModels] --> W{convexUserId changed?}
W -- yes --> X[reset refs + applyFavorites empty set]
W -- no --> Y{serverFavorites loaded?}
Y -- yes --> Z[applyFavorites with server data]
Y -- no --> AA[await next render]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
✅ No security issues found — scanned commits: |
| status: v.string(), | ||
| model: v.string(), | ||
| provider: v.string(), | ||
| options: v.optional(streamJobOptionsReturnValidator), | ||
| options: v.optional(streamOptionsValidator), |
There was a problem hiding this comment.
streamJobReturnShape may silently strip existing options data
Before this PR, the options field was validated as v.record(v.string(), v.any()) — accepting any key/value map. It now uses the strict streamOptionsValidator, which only allows five specific keys.
Convex's return-type validator strips unknown fields rather than throwing. This means any streamJob document in the database whose options object contains keys not in streamOptionsValidator (e.g., a key added in a past release or from an earlier migration) will silently have those keys dropped when the record is read back through getJobInternal or similar queries.
Before landing this change, confirm that no production streamJob records carry options keys outside the five defined in streamOptionsValidator. A one-time migration query or a temporary v.record(v.string(), v.any()) union branch would allow a safe rollout. (Score: 3/5)
1 participant
Automated PR for Greptile review
Note
Fix backend hotspots and apply code quality improvements across server and web
internal.*API references with typedFunctionReferenceconstants across multiple Convex modules (streamExecution.ts, chatTitle.ts, userDelete.ts, etc.) to improve type safetyby_chat_not_deletedandby_command_not_deletedsecondary indexes to improve query efficiency in files.ts and promptTemplates.tsuseFavoriteModelsto track per-user state and reconcile server favorites with optimistic updates; fixesUserSyncProviderandOpenRouterKeyStatusProviderto run once per authenticated user and reset on logoutparseStreamMetain redis.ts, andusePromptDraftto avoid unnecessary storage writes after draft restorationmessages.getActiveStreamnow uses theby_chat_statusindex without descending order, which may return a different message when multiple streaming messages exist simultaneouslyMacroscope summarized bba3999.