Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR refactors the chat architecture from an AI state-based model to an explicit props and hook-based approach, adds new API routes for chat and history operations, introduces a development authentication bypass mechanism, and updates AI SDK dependencies from RSC to React while removing related provider wrappers and hooks. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant MainContent as MainContent<br/>(useChat)
participant ChatAPI as POST /api/chat
participant Embeddings as Embeddings<br/>Service
participant ElasticSearch as ElasticSearch
participant AIModel as AI Model
participant DB as Database
participant Citations as Citations<br/>Transform
Client->>MainContent: onQuestionSubmit(question)
MainContent->>ChatAPI: POST { message, group, focus }
ChatAPI->>ChatAPI: Validate auth & request
ChatAPI->>Embeddings: Compute embedding(question)
Embeddings-->>ChatAPI: embedding vector
ChatAPI->>ElasticSearch: Search with embedding
ElasticSearch-->>ChatAPI: Relevant documents
ChatAPI->>ChatAPI: Construct system message<br/>with context
ChatAPI->>AIModel: Stream with messages
AIModel-->>ChatAPI: Token stream + thoughts
ChatAPI->>Citations: Transform citations
Citations-->>ChatAPI: [^1] references +<br/>Citations section
ChatAPI-->>MainContent: Stream response chunks
MainContent->>MainContent: Update UI messages<br/>& thoughts in real-time
AIModel-->>ChatAPI: completion
ChatAPI->>DB: Save system message,<br/>user input, response
DB-->>ChatAPI: chatId
ChatAPI-->>MainContent: Final metadata { chatId }
MainContent->>MainContent: Auto-navigate to<br/>saved chat URL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (16)
web/src/auth.ts (1)
118-128: Pre-existing type inconsistency:session.userIdis typed asstringbut holds anumberat runtime.Both the SAML path (line 110) and the new bypass path (line 68) store
userIdas anumberin the JWT token. The session callback casts it withas string(line 127), but this is a TypeScript-only assertion — the runtime value remains a number. Downstream consumers expecting a real string (e.g., string comparison with===) could be surprised.This is pre-existing and not introduced by this PR, but worth addressing at some point with an explicit
String(token.userId)conversion.Proposed fix
if (session) { - session.userId = token.userId as string; + session.userId = String(token.userId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/auth.ts` around lines 118 - 128, The session callback in auth.ts currently checks token.userId but then assigns it with a TypeScript assertion (session.userId = token.userId as string), which leaves a numeric runtime value; change the assignment in the async session({ session, token }) callback to coerce token.userId to a real string (e.g., use String(token.userId)) after your existing numeric validation so session.userId is an actual string at runtime and matches its declared type for downstream consumers.web/package.json (1)
6-6: Minor: mismatched quoting inpredevscript.The closing
"after the URL meansto view the appfalls outside the shell quotes.echostill prints it correctly, but the intent seems to be quoting the full sentence. Consider wrapping the entire message or dropping the inner quotes.Proposed fix
- "predev": "echo \"Use http://localhost:3000\" to view the app", + "predev": "echo 'Use http://localhost:3000 to view the app'",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/package.json` at line 6, The predev script in package.json has mismatched quoting around the echo message for "Use http://localhost:3000" causing the trailing text to sit outside the quotes; update the "predev" entry so the entire message is consistently quoted (e.g., wrap the whole sentence in double quotes or single quotes) or remove the inner quotes around the URL so the intent is clear and the shell string is well-formed.web/src/lib/devAuthBypass.ts (1)
56-63: Consider wrapping the Prisma call in a try/catch.If the DB is unreachable during dev, the unhandled rejection from
prisma.users.findUniquewill surface as a 500 rather than a clean auth failure. For a dev-only utility this is low-priority, but a try/catch returningnullwould make failures less noisy.Proposed fix
- const user = await prisma.users.findUnique({ - where: { id: userId }, - select: { id: true, name: true, email: true }, - }); - - if (!user) { - return null; - } + let user; + try { + user = await prisma.users.findUnique({ + where: { id: userId }, + select: { id: true, name: true, email: true }, + }); + } catch { + console.error('Dev bypass: failed to query user from DB'); + return null; + } + + if (!user) { + return null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/devAuthBypass.ts` around lines 56 - 63, Wrap the prisma.users.findUnique(...) call in a try/catch so DB errors return null instead of bubbling as an unhandled rejection; specifically, around the existing call that assigns to `user` (the prisma.users.findUnique where { id: userId } is passed), catch any error and return null (optionally log/debug the error) so the dev auth bypass function fails cleanly.web/src/app/auth/login/page.tsx (2)
78-94: Dev login section renders under "Available Campuses" heading — minor UX ordering issue.The dev section appears between the "Available Campuses"
<h4>(line 77) and the actual campus buttons (line 95). Consider moving it below the campus form or above the "Available Campuses" heading so the two aren't visually interleaved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/auth/login/page.tsx` around lines 78 - 94, The dev-login JSX guarded by devAuthBypassEnabled (the fragment containing the form using devBypassSignInHandler, callbackUrl, and devAuthUserId) is currently rendered between the "Available Campuses" <h4> and the campus buttons; move that entire fragment either above the "Available Campuses" heading or after the campus form/buttons so it no longer interleaves with the campus list—update the JSX order to place the devAuthBypassEnabled block adjacent to other auth actions for clearer UX.
42-56: The inline'use server'directive is redundant.The file already has
'use server'at line 1, which marks all exported/referenced async functions in this module as server actions. The inner directive on line 44 is harmless but unnecessary.Also, the
as anycast on line 55 is a known workaround for passing custom credential fields through NextAuth'ssignIn— acceptable here since the types don't account for provider-specific fields.Remove redundant directive
async function devBypassSignInHandler(formData: FormData) { - 'use server'; if (!devAuthBypassEnabled) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/auth/login/page.tsx` around lines 42 - 56, Remove the redundant inner 'use server' directive from the devBypassSignInHandler function: open the async function devBypassSignInHandler and delete the "'use server';" line inside it (leave the module-level 'use server' intact), keeping the rest of the function (including the signIn call with DEV_AUTH_BYPASS_PROVIDER_ID and the "as any" cast) unchanged.web/eslint.config.mjs (1)
30-32:react-refreshplugin is registered but no rules are enabled.The plugin is added to the
pluginsmap, but there are noreact-refresh/*rules in therulesblock. Typically you'd enable at leastreact-refresh/only-export-componentsto catch HMR-breaking exports.💡 Suggested addition
rules: { + 'react-refresh/only-export-components': [ + 'warn', + { allowConstantExport: true }, + ], eqeqeq: 'error',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/eslint.config.mjs` around lines 30 - 32, The 'react-refresh' plugin is registered in the plugins map but no plugin rules are enabled; add the appropriate react-refresh rules (at minimum enable "react-refresh/only-export-components") to the rules configuration so the plugin actually enforces HMR-safe exports—update the rules block to include "react-refresh/only-export-components": "warn" or "error" (and any other desired react-refresh/* rules) to activate the plugin behavior.web/src/lib/chat/citationsTransform.test.ts (1)
51-162: Good coverage of the core scenarios.The three tests cover the happy path (replacement + footnotes), chunk-boundary handling, and the no-citations edge case. Consider adding a test for non-citation
<characters in the text (e.g.,"Compare <b>bold</b> and <c:0>") to verify they pass through untouched alongside real citations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/chat/citationsTransform.test.ts` around lines 51 - 162, Add a new unit test in the existing describe('createCitationsTransform') suite to verify non-citation "<" sequences are preserved while real citations are transformed: use the existing test helper runTransform with basePolicies and a chunk sequence containing mixed markup like "Compare <b>bold</b> and <c:0>" (or split across chunks to exercise boundary handling), then assert the output contains the unchanged "<b>bold</b>" text and that the citation "[^0]" and the corresponding footnote from basePolicies are present; place this next to the other it(...) cases so it uses the same runTransform helper and policies references.web/src/lib/chat/citationsTransform.ts (2)
18-18: Consider narrowing theanyinStreamTextTransform<any>.If you have a type for the tools/settings used in this project, replacing
anywould improve type safety. Low priority since this is an internal transform.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/chat/citationsTransform.ts` at line 18, The return type uses a wide generic StreamTextTransform<any>; narrow it to the actual payload/interface used by this transform (e.g., a tool/settings or message type) by replacing the any with the specific type (for example ToolSettings, CitationPayload, or the existing project type that represents transform input/output) and update the function signature and any downstream usages to use that type; modify or import the appropriate type and ensure StreamTextTransform<YourType> is used instead of StreamTextTransform<any> in citationsTransform (referencing StreamTextTransform and the function that returns it).
81-83: Buffer is silently discarded onfinish-step— trailing text after lasttext-deltacould be lost.If the final
text-deltachunk ends with a bare<(buffered as a potential citation start), Line 82 clears it without emitting. In practice this is extremely unlikely since it would require the LLM to end output mid-tag, but you could emit the buffer contents as-is before clearing for correctness:💡 Optional defensive fix
if (chunk.type === 'finish-step') { + if (buffer) { + assistantText += buffer; + controller.enqueue({ type: 'text-delta', text: buffer }); + } buffer = '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/chat/citationsTransform.ts` around lines 81 - 83, In the finish-step branch where buffer is reset, don't silently discard any remaining buffered text: before setting buffer = '' (in the block that checks if chunk.type === 'finish-step'), flush/emit the buffer contents as a final text output (e.g., treat it like a final text-delta or append to the output array) when buffer is non-empty so trailing characters (like a lone '<') aren't lost; locate the buffer variable and the finish-step handling in citationsTransform (the chunk/type === 'finish-step' branch) and add a guarded emit of buffer prior to clearing it.web/src/components/chatHistory/chatHistory.tsx (2)
43-45: Unusederrorbinding in catch block.The caught
erroris not logged or inspected. If intentionally ignored, usecatchwithout a binding to avoid lint warnings. If not, consider logging it for observability.💡 Suggested fix
- } catch (error) { + } catch { status = 'error'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chatHistory/chatHistory.tsx` around lines 43 - 45, The catch block in chatHistory.tsx currently declares a wasted error binding ("catch (error) { status = 'error'; }"); either remove the unused binding by switching to a bare catch ("catch { ... }") or use the binding to log/inspect the error (e.g., call console.error or your app logger) before setting status to 'error' so the exception isn't silently ignored; locate the catch in the function that sets the status variable and apply one of these fixes.
72-72:!!chatsis always truthy —chatsdefaults to[](Line 33).The guard can be simplified or removed since
chatsis always a (possibly empty) array at this point.💡 Optional simplification
- {!!chats && <ChatHistoryWrapper group={group} chats={chats} />} + <ChatHistoryWrapper group={group} chats={chats} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chatHistory/chatHistory.tsx` at line 72, The double-negation guard (!!chats) is redundant because chats defaults to an array; remove the boolean coercion and render ChatHistoryWrapper directly as <ChatHistoryWrapper group={group} chats={chats} />; if the intent was to render only when there are items, replace the guard with chats.length > 0 instead — locate the usage of chats and ChatHistoryWrapper to make the change.web/src/app/api/chat/route.ts (1)
86-149: No error handling inside theexecutecallback — stream errors will surface as opaque failures.If
getEmbeddings,getSearchResultsElastic, orconvertToModelMessagesthrow, the error propagates through the stream with no user-friendly message or logging. Consider wrapping the body in a try/catch to write a meaningful error part or close the stream gracefully.💡 Suggested approach
const stream = createUIMessageStream({ execute: async ({ writer }) => { + try { const writeThought = (thought: string) => { // ... }; // ... rest of the execute body ... + } catch (err) { + // eslint-disable-next-line no-console + console.error('Chat stream error', err); + writer.write({ + type: 'error', + errorText: 'Something went wrong. Please try again.', + }); + } }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/chat/route.ts` around lines 86 - 149, Wrap the entire createUIMessageStream execute callback body (the async function passed to execute) in a try/catch so any throw from getEmbeddings, getSearchResultsElastic, convertToModelMessages, streamText, etc. is caught; in the catch block call writer.write to emit a clear error message part (use same message shape as writeThought but non-transient or a distinct type like 'data-error' with the error.message), optionally call writer.write a short user-friendly note, and then close/finish the stream (e.g., ensure writer.merge/finish is invoked or writer is closed) so the UI receives a graceful failure instead of an opaque crash; keep references to createUIMessageStream, execute, writeThought, getEmbeddings, getSearchResultsElastic, convertToModelMessages, streamText, and writer.write/merge when applying the change.web/src/lib/actions.tsx (1)
43-53:deleteChatFromSidebarlacks a return type annotation, unlike its siblings.Minor inconsistency — the other three actions have explicit
Promise<string>orPromise<void>return types.💡 Suggested fix
export const deleteChatFromSidebar = async ( chatId: string, isActiveChat: boolean -) => { +): Promise<void> => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/lib/actions.tsx` around lines 43 - 53, The deleteChatFromSidebar function is missing an explicit return type; update its declaration to include an explicit Promise<void> return type (i.e., change the async arrow signature for deleteChatFromSidebar to end with : Promise<void>) so it matches the other action signatures and keeps type consistency; keep the existing logic involving removeChat, isWonkSuccess, WonkServerError, and redirect unchanged.web/src/app/api/history/route.ts (1)
10-25: Route delegates auth to service layer; consider explicit auth check for consistency withchatroute.The
getChatHistoryForGroupfunction properly handles authentication internally (callsauth()and checkssession.userId), so this route is secure. However, thechatPOST route checkssession?.userIdexplicitly at the route level (lines 45-48), while this route defers to the service layer. For consistency and clarity, adding an explicit check at the route level would make the auth boundary self-documenting.Note: The
Number(result.status)on line 23 is correct—WonkStatusCodesvalues are strings like'401', which convert to numbers properly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/api/history/route.ts` around lines 10 - 25, The GET route delegates auth to getChatHistoryForGroup but should perform an explicit route-level auth check for consistency with the chat POST route: call auth() at the top of the GET handler, verify session?.userId exists, and if missing return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) before validating group or invoking getChatHistoryForGroup; keep existing calls to isValidGroupName, isWonkSuccess and Number(result.status) unchanged.web/src/app/(home)/[group]/share/[shareid]/page.tsx (1)
32-35: Avoid gating shared-chat metadata behind auth if the page is publicly viewable.On Line 33, unauthenticated users get a forced
'Chat'title, while Line 65 still renders shared content. This can degrade link previews and social unfurls for shared URLs.Proposed adjustment
- const session = (await auth()) as WonkSession; - if (!session?.userId) { - return { title: 'Chat' }; - } - const params = await props.params; const { shareid } = params; const result = await getCachedSharedChat(shareid);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/app/`(home)/[group]/share/[shareid]/page.tsx around lines 32 - 35, Don't return a default title when auth() yields no session; instead compute page metadata from the shared chat resource regardless of authentication. Replace the early return that checks session?.userId in page.tsx with logic that calls the same shared-content lookup used for rendering the share page (keep references to auth(), session and the shared chat fetch used later in the file) and set the title from that shared resource, falling back to 'Chat' only if the shared metadata is absent. Ensure metadata generation does not depend on WonkSession being present so public link previews and social unfurls can show the shared chat info.web/src/components/chatHistory/chatHistoryWrapper.tsx (1)
74-76: Don’t fully swallow refresh errors.Ignoring all errors here makes history-refresh failures invisible and harder to debug. At minimum, log at debug/warn level or emit telemetry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/chatHistory/chatHistoryWrapper.tsx` around lines 74 - 76, The catch block in chatHistoryWrapper.tsx currently swallows all errors; modify the catch to capture the error (e.g., catch (err)) and log it at debug/warn level or emit telemetry instead of ignoring it. Locate the try/catch around the history refresh logic (the refresh handling in chatHistoryWrapper) and replace the empty catch with something like logger.warn("chat history refresh failed", { err, stack: err?.stack }) or call your telemetry/metrics helper (e.g., emitTelemetry("chat.history.refresh.failed", { error: err })) so failures are visible while preserving existing control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/chat/answer/feedbackButtons.tsx`:
- Around line 52-55: The catch block in feedbackButtons.tsx currently swallows
submission errors and only calls onReactionUpdate(undefined); update that catch
to (1) capture the thrown error (e), (2) still roll back via
onReactionUpdate(undefined) but also surface a user-facing notification (either
by invoking your existing toast/notification utility or setting local error
state that renders a small inline message in the FeedbackButtons component), and
(3) log the error (console.error or process logger) including e.message/details
for debugging; ensure the notification text is generic/user-friendly and
reference the same handler where onReactionUpdate is called so the UI rollback
and error display happen together.
In `@web/src/components/chat/answer/shareModal.tsx`:
- Around line 47-52: The share/unshare try/finally blocks (around shareChat and
unshareChat) lack a catch so failures silently reset loading; wrap the await
shareChat(chatId) and await unshareChat(chatId) calls in try/catch, call
onShareIdUpdate only on success, and in the catch set an explicit error state
(e.g., setIsLoading('error') or a new setShareError state) and log the error
(console.error or a logger) so the UI can surface the failure to the user before
the existing finally resets loading. Ensure you update both the shareChat path
(shareChat, onShareIdUpdate, setIsLoading) and the unshare path similarly.
In `@web/src/components/chat/main.tsx`:
- Around line 59-68: The local chat state initialized with React.useState(chat)
isn't synced when the initialChat prop changes, causing stale
group/focus/shareId values; update the component to either add a useEffect that
calls setChat(initialChat) with [initialChat] as dependency (referencing the
chat and setChat state variables and initialChat prop) or ensure the parent
forces remounts by adding a key={initialChat.id} to MainContent where it is
rendered so initialMessages (chatMessagesToUIMessages) and subsequent handlers
(onQuestionSubmit, onFocusSelection) always use the current chat.
---
Nitpick comments:
In `@web/eslint.config.mjs`:
- Around line 30-32: The 'react-refresh' plugin is registered in the plugins map
but no plugin rules are enabled; add the appropriate react-refresh rules (at
minimum enable "react-refresh/only-export-components") to the rules
configuration so the plugin actually enforces HMR-safe exports—update the rules
block to include "react-refresh/only-export-components": "warn" or "error" (and
any other desired react-refresh/* rules) to activate the plugin behavior.
In `@web/package.json`:
- Line 6: The predev script in package.json has mismatched quoting around the
echo message for "Use http://localhost:3000" causing the trailing text to sit
outside the quotes; update the "predev" entry so the entire message is
consistently quoted (e.g., wrap the whole sentence in double quotes or single
quotes) or remove the inner quotes around the URL so the intent is clear and the
shell string is well-formed.
In `@web/src/app/`(home)/[group]/share/[shareid]/page.tsx:
- Around line 32-35: Don't return a default title when auth() yields no session;
instead compute page metadata from the shared chat resource regardless of
authentication. Replace the early return that checks session?.userId in page.tsx
with logic that calls the same shared-content lookup used for rendering the
share page (keep references to auth(), session and the shared chat fetch used
later in the file) and set the title from that shared resource, falling back to
'Chat' only if the shared metadata is absent. Ensure metadata generation does
not depend on WonkSession being present so public link previews and social
unfurls can show the shared chat info.
In `@web/src/app/api/chat/route.ts`:
- Around line 86-149: Wrap the entire createUIMessageStream execute callback
body (the async function passed to execute) in a try/catch so any throw from
getEmbeddings, getSearchResultsElastic, convertToModelMessages, streamText, etc.
is caught; in the catch block call writer.write to emit a clear error message
part (use same message shape as writeThought but non-transient or a distinct
type like 'data-error' with the error.message), optionally call writer.write a
short user-friendly note, and then close/finish the stream (e.g., ensure
writer.merge/finish is invoked or writer is closed) so the UI receives a
graceful failure instead of an opaque crash; keep references to
createUIMessageStream, execute, writeThought, getEmbeddings,
getSearchResultsElastic, convertToModelMessages, streamText, and
writer.write/merge when applying the change.
In `@web/src/app/api/history/route.ts`:
- Around line 10-25: The GET route delegates auth to getChatHistoryForGroup but
should perform an explicit route-level auth check for consistency with the chat
POST route: call auth() at the top of the GET handler, verify session?.userId
exists, and if missing return NextResponse.json({ error: 'Unauthorized' }, {
status: 401 }) before validating group or invoking getChatHistoryForGroup; keep
existing calls to isValidGroupName, isWonkSuccess and Number(result.status)
unchanged.
In `@web/src/app/auth/login/page.tsx`:
- Around line 78-94: The dev-login JSX guarded by devAuthBypassEnabled (the
fragment containing the form using devBypassSignInHandler, callbackUrl, and
devAuthUserId) is currently rendered between the "Available Campuses" <h4> and
the campus buttons; move that entire fragment either above the "Available
Campuses" heading or after the campus form/buttons so it no longer interleaves
with the campus list—update the JSX order to place the devAuthBypassEnabled
block adjacent to other auth actions for clearer UX.
- Around line 42-56: Remove the redundant inner 'use server' directive from the
devBypassSignInHandler function: open the async function devBypassSignInHandler
and delete the "'use server';" line inside it (leave the module-level 'use
server' intact), keeping the rest of the function (including the signIn call
with DEV_AUTH_BYPASS_PROVIDER_ID and the "as any" cast) unchanged.
In `@web/src/auth.ts`:
- Around line 118-128: The session callback in auth.ts currently checks
token.userId but then assigns it with a TypeScript assertion (session.userId =
token.userId as string), which leaves a numeric runtime value; change the
assignment in the async session({ session, token }) callback to coerce
token.userId to a real string (e.g., use String(token.userId)) after your
existing numeric validation so session.userId is an actual string at runtime and
matches its declared type for downstream consumers.
In `@web/src/components/chatHistory/chatHistory.tsx`:
- Around line 43-45: The catch block in chatHistory.tsx currently declares a
wasted error binding ("catch (error) { status = 'error'; }"); either remove the
unused binding by switching to a bare catch ("catch { ... }") or use the binding
to log/inspect the error (e.g., call console.error or your app logger) before
setting status to 'error' so the exception isn't silently ignored; locate the
catch in the function that sets the status variable and apply one of these
fixes.
- Line 72: The double-negation guard (!!chats) is redundant because chats
defaults to an array; remove the boolean coercion and render ChatHistoryWrapper
directly as <ChatHistoryWrapper group={group} chats={chats} />; if the intent
was to render only when there are items, replace the guard with chats.length > 0
instead — locate the usage of chats and ChatHistoryWrapper to make the change.
In `@web/src/components/chatHistory/chatHistoryWrapper.tsx`:
- Around line 74-76: The catch block in chatHistoryWrapper.tsx currently
swallows all errors; modify the catch to capture the error (e.g., catch (err))
and log it at debug/warn level or emit telemetry instead of ignoring it. Locate
the try/catch around the history refresh logic (the refresh handling in
chatHistoryWrapper) and replace the empty catch with something like
logger.warn("chat history refresh failed", { err, stack: err?.stack }) or call
your telemetry/metrics helper (e.g.,
emitTelemetry("chat.history.refresh.failed", { error: err })) so failures are
visible while preserving existing control flow.
In `@web/src/lib/actions.tsx`:
- Around line 43-53: The deleteChatFromSidebar function is missing an explicit
return type; update its declaration to include an explicit Promise<void> return
type (i.e., change the async arrow signature for deleteChatFromSidebar to end
with : Promise<void>) so it matches the other action signatures and keeps type
consistency; keep the existing logic involving removeChat, isWonkSuccess,
WonkServerError, and redirect unchanged.
In `@web/src/lib/chat/citationsTransform.test.ts`:
- Around line 51-162: Add a new unit test in the existing
describe('createCitationsTransform') suite to verify non-citation "<" sequences
are preserved while real citations are transformed: use the existing test helper
runTransform with basePolicies and a chunk sequence containing mixed markup like
"Compare <b>bold</b> and <c:0>" (or split across chunks to exercise boundary
handling), then assert the output contains the unchanged "<b>bold</b>" text and
that the citation "[^0]" and the corresponding footnote from basePolicies are
present; place this next to the other it(...) cases so it uses the same
runTransform helper and policies references.
In `@web/src/lib/chat/citationsTransform.ts`:
- Line 18: The return type uses a wide generic StreamTextTransform<any>; narrow
it to the actual payload/interface used by this transform (e.g., a tool/settings
or message type) by replacing the any with the specific type (for example
ToolSettings, CitationPayload, or the existing project type that represents
transform input/output) and update the function signature and any downstream
usages to use that type; modify or import the appropriate type and ensure
StreamTextTransform<YourType> is used instead of StreamTextTransform<any> in
citationsTransform (referencing StreamTextTransform and the function that
returns it).
- Around line 81-83: In the finish-step branch where buffer is reset, don't
silently discard any remaining buffered text: before setting buffer = '' (in the
block that checks if chunk.type === 'finish-step'), flush/emit the buffer
contents as a final text output (e.g., treat it like a final text-delta or
append to the output array) when buffer is non-empty so trailing characters
(like a lone '<') aren't lost; locate the buffer variable and the finish-step
handling in citationsTransform (the chunk/type === 'finish-step' branch) and add
a guarded emit of buffer prior to clearing it.
In `@web/src/lib/devAuthBypass.ts`:
- Around line 56-63: Wrap the prisma.users.findUnique(...) call in a try/catch
so DB errors return null instead of bubbling as an unhandled rejection;
specifically, around the existing call that assigns to `user` (the
prisma.users.findUnique where { id: userId } is passed), catch any error and
return null (optionally log/debug the error) so the dev auth bypass function
fails cleanly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
.gitignoreweb/eslint.config.mjsweb/package.jsonweb/src/app/(home)/[group]/chat/[chatid]/loading.tsxweb/src/app/(home)/[group]/chat/[chatid]/page.tsxweb/src/app/(home)/[group]/share/[shareid]/page.tsxweb/src/app/api/chat/route.tsweb/src/app/api/documents/route.tsweb/src/app/api/history/route.tsweb/src/app/auth/login/page.tsxweb/src/app/global-error.tsxweb/src/auth.tsweb/src/components/chat/answer/chatActions.tsxweb/src/components/chat/answer/feedbackButtons.tsxweb/src/components/chat/answer/shareModal.tsxweb/src/components/chat/answer/wonkMessage.tsxweb/src/components/chat/ask/chatInput.tsxweb/src/components/chat/ask/defaultQuestions.tsxweb/src/components/chat/main.tsxweb/src/components/chatHistory/chatHistory.tsxweb/src/components/chatHistory/chatHistoryWrapper.tsxweb/src/components/chatHistory/deleteChatButton.tsxweb/src/components/layout/collapsibleSidebarProvider.tsxweb/src/lib/actions.tsxweb/src/lib/aiProvider.tsxweb/src/lib/chat/citationsTransform.test.tsweb/src/lib/chat/citationsTransform.tsweb/src/lib/devAuthBypass.tsweb/src/lib/gtagProvider.tsxweb/src/lib/hooks/useStreamableText.tsweb/vitest.config.mts
💤 Files with no reviewable changes (7)
- web/src/app/api/documents/route.ts
- web/src/components/layout/collapsibleSidebarProvider.tsx
- web/src/lib/aiProvider.tsx
- web/src/lib/gtagProvider.tsx
- web/src/app/(home)/[group]/chat/[chatid]/loading.tsx
- web/src/components/chat/ask/defaultQuestions.tsx
- web/src/lib/hooks/useStreamableText.ts
Comment on lines
52
to
55
| } catch (e) { | ||
| // TODO: handle error | ||
| setAIState((currentAIState) => ({ | ||
| ...currentAIState, | ||
| reaction: undefined, // unset the reaction on error | ||
| })); | ||
| onReactionUpdate(undefined); | ||
| } |
Contributor
There was a problem hiding this comment.
Surface submission failures to the user instead of silently reverting.
Current catch path rolls back reaction but gives no feedback. Users can’t tell whether the action failed or was ignored.
Would you like me to open a follow-up issue and draft a small inline error/toast implementation?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/chat/answer/feedbackButtons.tsx` around lines 52 - 55, The
catch block in feedbackButtons.tsx currently swallows submission errors and only
calls onReactionUpdate(undefined); update that catch to (1) capture the thrown
error (e), (2) still roll back via onReactionUpdate(undefined) but also surface
a user-facing notification (either by invoking your existing toast/notification
utility or setting local error state that renders a small inline message in the
FeedbackButtons component), and (3) log the error (console.error or process
logger) including e.message/details for debugging; ensure the notification text
is generic/user-friendly and reference the same handler where onReactionUpdate
is called so the UI rollback and error display happen together.
Comment on lines
+47
to
+52
| try { | ||
| const newShareId = await shareChat(chatId); | ||
| onShareIdUpdate(newShareId); | ||
| } finally { | ||
| setIsLoading(''); | ||
| } |
Contributor
There was a problem hiding this comment.
Add explicit failure handling for share/unshare actions.
finally resets loading, but there’s no catch path to show an error state. Users get no explanation when share operations fail.
Proposed pattern
+ const [actionError, setActionError] = React.useState<string>('');
const handleShare = async (
type: GTagEvents.SHARE | GTagEvents.REGEN_SHARE
) => {
setIsLoading(type);
+ setActionError('');
gtagEvent({ event: type, chat });
try {
const newShareId = await shareChat(chatId);
onShareIdUpdate(newShareId);
+ } catch {
+ setActionError('Unable to share this chat right now. Please try again.');
} finally {
setIsLoading('');
}
};
const handleUnshare = async () => {
setIsLoading(GTagEvents.UNSHARE);
+ setActionError('');
gtagEvent({ event: GTagEvents.UNSHARE, chat });
try {
await unshareChat(chatId);
onShareIdUpdate(undefined);
+ } catch {
+ setActionError('Unable to unshare this chat right now. Please try again.');
} finally {
setIsLoading('');
}
};Also applies to: 58-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/chat/answer/shareModal.tsx` around lines 47 - 52, The
share/unshare try/finally blocks (around shareChat and unshareChat) lack a catch
so failures silently reset loading; wrap the await shareChat(chatId) and await
unshareChat(chatId) calls in try/catch, call onShareIdUpdate only on success,
and in the catch set an explicit error state (e.g., setIsLoading('error') or a
new setShareError state) and log the error (console.error or a logger) so the UI
can surface the failure to the user before the existing finally resets loading.
Ensure you update both the shareChat path (shareChat, onShareIdUpdate,
setIsLoading) and the unshare path similarly.
Comment on lines
+59
to
+68
| const [chat, setChat] = React.useState<ChatHistory>(initialChat); | ||
| const [currentStatusMessage, setCurrentStatusMessage] = | ||
| React.useState<string>(''); | ||
| const lastRedirectedChatIdRef = React.useRef<string | null>(null); | ||
| const didLogNewChatEventRef = React.useRef<boolean>(false); | ||
|
|
||
| const initialMessages = React.useMemo( | ||
| () => chatMessagesToUIMessages(initialChat.messages), | ||
| [initialChat.messages] | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "MainContent call sites (check for key prop):"
rg -nP '<MainContent\b' web/src/app web/src/components
echo
echo "MainContent state init and prop-sync checks:"
rg -nP 'useState<ChatHistory>\(initialChat\)|setChat\(initialChat\)|useEffect\(\(\)\s*=>\s*\{\s*setChat' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 424
🏁 Script executed:
# Get more context on the MainContent file structure and useEffect patterns
cat -n web/src/components/chat/main.tsx | head -100Repository: ucdavis/PolicyWonk
Length of output: 3781
🏁 Script executed:
# Check if there's a useEffect that updates chat state based on initialChat prop
rg -A 5 'useEffect.*initialChat|useEffect.*setChat' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 44
🏁 Script executed:
# Check the full extent of setChat usage in the file
rg -n 'setChat' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 333
🏁 Script executed:
# Look for useEffect hooks in the file
rg -B 2 -A 5 'useEffect' web/src/components/chat/main.tsx | head -50Repository: ucdavis/PolicyWonk
Length of output: 426
🏁 Script executed:
# Check the parent page components to see how MainContent is passed
cat -n web/src/app/\(home\)/\[group\]/chat/\[chatid\]/page.tsx | tail -20Repository: ucdavis/PolicyWonk
Length of output: 604
🏁 Script executed:
# Check the full page component files to see if MainContent has a key prop
cat -n web/src/app/\(home\)/\[group\]/chat/\[chatid\]/page.tsx | grep -B 2 -A 2 'MainContent'Repository: ucdavis/PolicyWonk
Length of output: 395
🏁 Script executed:
# Check the share page too
cat -n web/src/app/\(home\)/\[group\]/share/\[shareid\]/page.tsx | grep -B 2 -A 2 'MainContent'Repository: ucdavis/PolicyWonk
Length of output: 443
🏁 Script executed:
# Verify the full extent of useEffect hooks and their dependencies
sed -n '110,220p' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 3157
🏁 Script executed:
# Get the complete useEffect that handles the redirect and chat id sync
sed -n '110,135p' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 852
🏁 Script executed:
# Check if initialChat.id or other initialChat properties are ever accessed after mount
rg -n 'initialChat\.' web/src/components/chat/main.tsxRepository: ucdavis/PolicyWonk
Length of output: 152
Add a useEffect to sync local chat state when initialChat prop changes, or key MainContent to force remounts across route transitions.
useState(initialChat) only seeds on first mount. Since MainContent is rendered without a key prop, navigating between different chats reuses the same component instance. When initialChat prop changes, the local chat state remains stale with previous chat's group, focus, and shareId values. This causes:
onQuestionSubmitto send requests with stalegroupandfocusvaluesonFocusSelectionto update stale focus state- Router redirect checks to use stale
chat.group
Add React.useEffect(() => { setChat(initialChat); }, [initialChat]) to sync state, or add a key={initialChat.id} prop to MainContent in both page components to force remounts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/chat/main.tsx` around lines 59 - 68, The local chat state
initialized with React.useState(chat) isn't synced when the initialChat prop
changes, causing stale group/focus/shareId values; update the component to
either add a useEffect that calls setChat(initialChat) with [initialChat] as
dependency (referencing the chat and setChat state variables and initialChat
prop) or ensure the parent forces remounts by adding a key={initialChat.id} to
MainContent where it is rendered so initialMessages (chatMessagesToUIMessages)
and subsequent handlers (onQuestionSubmit, onFocusSelection) always use the
current chat.
aristachauhan
approved these changes
Feb 25, 2026
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.
Switching to useChat() like Rocky
Summary by CodeRabbit
New Features
Tests