feat: in-app AI chat assistant backed by the MCP server tools (JEF-12) - #159
Conversation
Adds a chat UI letting users ask natural-language questions across their applications, contacts, and interview rounds. Reuses the MCP server's tool schemas and the use-cases it delegates to (GetApplicationsUseCase, GetApplicationUseCase, GetNotesUseCase, GetContactsUseCase, GetInterviewRoundsUseCase) in-process with normal session auth, rather than round-tripping through the MCP HTTP endpoint (which is API-token authenticated, a separate concern). - ILLMProvider gains completeWithTools(messages, tools, maxTokens?), implemented in all 3 provider classes: OpenAICompatibleLLMProvider (OpenAI tools/tool_calls shape — covers 7 of 9 BYOK providers), AnthropicLLMProvider (tools/tool_use/tool_result), GoogleAILLMProvider (functionDeclarations/functionCall/functionResponse). Each translates the generic LLMMessage[]/tool types to/from its own wire format internally. - ChatWithAssistantUseCase runs a bounded tool-call loop (max 5 round-trips) against completeWithTools, dispatching each requested tool to its matching use-case and feeding results back until the LLM returns a final answer. Rate-limited per user (20 messages / 5 min, reusing the existing IRateLimiter). AI_NOT_CONFIGURED when no BYOK key is set, same as the other 3 AI features. Rejects any history entry with a role other than user/assistant, since allowing arbitrary roles would let a client inject a fake system message. Conversation history is ephemeral (frontend-held, resent each turn) — no new DB table for v1. - sendChatMessage(history, message) GraphQL mutation with a new ChatMessageInput type (first nested-list input in this codebase). - Frontend: new /assistant page (own sidebar/mobile-nav item) with a message list, suggested-question chips, and the same AI_NOT_CONFIGURED -> Account settings prompt as the other AI features. Note: Gemini's function-calling wire format (systemInstruction / functionCall / functionResponse with a "function" role) is implemented to the best of my knowledge but not verified against a live Google AI key — worth a real smoke test before relying on it in production. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N2PBmsuzPhrmNnfZf6C3BM
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughAdds an authenticated assistant chat UI backed by a GraphQL mutation and a rate-limited chat use case. The API supports iterative LLM tool calling, application-related tool dispatch, and Anthropic, Gemini, and OpenAI-compatible provider serialisation. ChangesAssistant chat
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AssistantPage
participant sendChatMessage
participant ChatWithAssistantUseCase
participant LLMProvider
participant ApplicationUseCase
User->>AssistantPage: submit message
AssistantPage->>sendChatMessage: send history and message
sendChatMessage->>ChatWithAssistantUseCase: execute authenticated request
ChatWithAssistantUseCase->>LLMProvider: completeWithTools
LLMProvider-->>ChatWithAssistantUseCase: return tool call
ChatWithAssistantUseCase->>ApplicationUseCase: execute selected tool
ApplicationUseCase-->>ChatWithAssistantUseCase: return tool result
ChatWithAssistantUseCase->>LLMProvider: completeWithTools with tool result
LLMProvider-->>ChatWithAssistantUseCase: return final content
ChatWithAssistantUseCase-->>sendChatMessage: return response
sendChatMessage-->>AssistantPage: return assistant message
AssistantPage-->>User: render response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Preview deployments for this PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
apps/api/src/infrastructure/llm/AnthropicLLMProvider.ts (1)
33-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
complete()doesn't convert'tool'/tool-call messages, unlikecompleteWithTools()andOpenAICompatibleLLMProvider.complete().
conversation.map((m) => ({ role: m.role, content: m.content }))passesm.rolestraight through, whereascompleteWithToolsusestoWireMessagesto properly convert'tool'role entries intotool_resultblocks and assistant tool-calls intotool_useblocks. Anthropic's Messages API only acceptsuser/assistantroles — a literalrole: 'tool'would be rejected. Ifcomplete()is ever invoked with a history that contains prior tool-call turns (e.g. a future caller reusing conversation state built forcompleteWithTools), this will fail with a 400 from Anthropic.OpenAICompatibleLLMProvider.complete()already routes through itstoWireMessages, so this is an inconsistency between the two providers.🐛 Suggested fix
const { system, conversation } = this.splitSystem(messages); const json = await this.post({ model: this.model, max_tokens: maxTokens, ...(system ? { system } : {}), - messages: conversation.map((m) => ({ role: m.role, content: m.content })), + messages: this.toWireMessages(conversation), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/infrastructure/llm/AnthropicLLMProvider.ts` around lines 33 - 42, Update AnthropicLLMProvider.complete() to convert conversation messages through the existing toWireMessages helper instead of mapping m.role and m.content directly. Preserve the current system-message handling and request structure while ensuring tool roles become tool_result blocks and assistant tool calls become tool_use blocks, consistent with completeWithTools() and OpenAICompatibleLLMProvider.complete().apps/api/src/use-cases/ports/ILLMProvider.ts (1)
7-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
LLMMessageisn't a discriminated union, allowing invalid combinations.
role,toolCallId, andtoolCallsare all independent optional fields, so nothing at the type level prevents constructing e.g. a'tool'message withouttoolCallId, or a'user'message withtoolCallsset. BothAnthropicLLMProvider.toWireMessagesandOpenAICompatibleLLMProvider.toWireMessagessilently fall back to''/undefinedfortoolCallIdwhen it's missing rather than failing fast, which would produce a malformed request to the provider if this invariant is ever violated upstream.Consider a discriminated union to make invalid states unrepresentable:
♻️ Suggested discriminated union
export type LLMMessage = | { role: 'system' | 'user'; content: string } | { role: 'assistant'; content: string; toolCalls?: LLMToolCall[] } | { role: 'tool'; content: string; toolCallId: string };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/use-cases/ports/ILLMProvider.ts` around lines 7 - 14, Replace the independent-field LLMMessage interface with a discriminated union: system/user messages should only contain content, assistant messages may optionally contain toolCalls, and tool messages must require toolCallId. Update dependent construction and handling in AnthropicLLMProvider.toWireMessages and OpenAICompatibleLLMProvider.toWireMessages as needed so they rely on the enforced invariant instead of silently defaulting missing toolCallId values.apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts (2)
91-94: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSequential tool execution could be parallelized.
Multiple tool calls in a single LLM turn are awaited one at a time. Since
executeToolalready scopes each call touserIdindependently, running them concurrently withPromise.allwould reduce latency without changing behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts` around lines 91 - 94, Update the tool-call handling around executeTool to run all result.toolCalls concurrently with Promise.all, passing input.userId to each call, then append each corresponding tool result to messages with its original call.id. Preserve the existing result ordering and message structure.
78-83: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winLLM provider call isn't wrapped for graceful degradation.
executeToolcatches errors and feeds them back to the model, butcompleteWithTools(the outer LLM call) is unguarded. A network failure or non-OK provider response bubbles up as a generic uncodedError, whichfromCodedErrorwill map to a plain "Internal server error" rather than the existingERROR_CODES.SERVICE_UNAVAILABLE, losing useful signal for the client/UI.Consider catching provider-level errors here and re-throwing with
ERROR_CODES.SERVICE_UNAVAILABLEfor a clearer, more actionable error surfaced to the user.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts` around lines 78 - 83, Wrap the completeWithTools call inside the ChatWithAssistantUseCase iteration loop with error handling, and rethrow provider-level failures using the existing ERROR_CODES.SERVICE_UNAVAILABLE coded error. Preserve the current tool-call processing and response fallback behavior for successful completions, while ensuring network or non-OK provider errors retain the service-unavailable signal.apps/api/src/http/schema/types/inputs/ChatInputs.ts (1)
3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a GraphQL enum for
roleinstead of a free-form string.
roleaccepts any string here; the actualuser/assistantrestriction is enforced later inChatWithAssistantUseCase.executeand cast unsafely (m.role as 'user' | 'assistant') inchatMutations.ts. A Pothos enum type would make the accepted values self-documenting in the schema and let GraphQL reject invalid roles before they reach the resolver, removing the need for the cast.+export const ChatMessageRole = builder.enumType('ChatMessageRole', { + values: ['user', 'assistant'] as const, +}); + export const ChatMessageInput = builder.inputType('ChatMessageInput', { fields: (t) => ({ - role: t.string({ required: true }), + role: t.field({ type: ChatMessageRole, required: true }), content: t.string({ required: true }), }), });This would also need a corresponding client-side type/enum update in the web app, since the frontend already sends this input.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/http/schema/types/inputs/ChatInputs.ts` around lines 3 - 8, Replace the free-form role field in ChatMessageInput with a Pothos GraphQL enum restricted to user and assistant, and update chatMutations.ts to consume the typed enum without an unsafe cast. Add the corresponding client-side enum/type update in the web app so its ChatMessageInput values match the schema.apps/api/src/infrastructure/llm/GoogleAILLMProvider.ts (1)
42-95: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueTool-call round trip logic and response parsing look sound.
systemInstructionextraction, theidToNamecorrelation map, and thefunctionCall/text parsing all correctly implement theLLMCompletionResultcontract (content/toolCalls). Minor optional note: newer Gemini API responses can carry anidonfunctionCall/functionResponsefor correlating parallel calls of the same tool name — worth keeping in mind if/when the live-key smoke test surfaces issues with multiple simultaneous same-name tool calls, but not blocking given the current single-tool-per-turn usage pattern inChatWithAssistantUseCase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/infrastructure/llm/GoogleAILLMProvider.ts` around lines 42 - 95, The reviewed implementation is correct; no code changes are required. Preserve the existing completeWithTools parsing and single-tool-per-turn behavior, while treating function-call IDs as a future enhancement only if parallel same-name tool calls become necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/infrastructure/llm/AnthropicLLMProvider.ts`:
- Around line 112-129: Update the AnthropicLLMProvider.post method to apply a
finite timeout to the outbound fetch request, using the project’s existing
timeout configuration or convention if available. Ensure the timeout signal is
passed in the fetch options and that timeout failures propagate as errors
instead of allowing the request to hang indefinitely.
In `@apps/api/src/infrastructure/llm/OpenAICompatibleLLMProvider.ts`:
- Around line 110-126: Update OpenAICompatibleLLMProvider.post to bound the
outbound fetch with an AbortController timeout, matching the timeout behavior
used by AnthropicLLMProvider.post. Pass the controller’s signal to fetch and
ensure the timeout is cleaned up after completion while preserving existing
response and error handling.
In `@apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts`:
- Around line 57-76: In ChatWithAssistantUseCase, add validation before
llmProvider creation and message construction to enforce configured maximum
lengths for input.message and input.history, including a cap on each history
entry’s content if the existing validation model supports it. Reject oversized
inputs with ERROR_CODES.VALIDATION, while preserving the existing role
validation and normal flow for inputs within limits.
In `@apps/web/src/routes/_authenticated/assistant.tsx`:
- Around line 37-41: Update the mutationFn request in the assistant route to
send only a capped recent window of complete conversation turns rather than all
messages, preserving chronological order and avoiding partial turns. Apply the
same history limit in the server-side sendChatMessage handling so clients cannot
bypass the bound.
- Line 67: Update the assistant page container height in the main layout div to
reserve space for both the mobile header and the fixed bottom navigation, using
the existing responsive desktop height unchanged. Ensure the composer remains
visible above the mobile navigation while preserving the current desktop layout.
---
Nitpick comments:
In `@apps/api/src/http/schema/types/inputs/ChatInputs.ts`:
- Around line 3-8: Replace the free-form role field in ChatMessageInput with a
Pothos GraphQL enum restricted to user and assistant, and update
chatMutations.ts to consume the typed enum without an unsafe cast. Add the
corresponding client-side enum/type update in the web app so its
ChatMessageInput values match the schema.
In `@apps/api/src/infrastructure/llm/AnthropicLLMProvider.ts`:
- Around line 33-42: Update AnthropicLLMProvider.complete() to convert
conversation messages through the existing toWireMessages helper instead of
mapping m.role and m.content directly. Preserve the current system-message
handling and request structure while ensuring tool roles become tool_result
blocks and assistant tool calls become tool_use blocks, consistent with
completeWithTools() and OpenAICompatibleLLMProvider.complete().
In `@apps/api/src/infrastructure/llm/GoogleAILLMProvider.ts`:
- Around line 42-95: The reviewed implementation is correct; no code changes are
required. Preserve the existing completeWithTools parsing and
single-tool-per-turn behavior, while treating function-call IDs as a future
enhancement only if parallel same-name tool calls become necessary.
In `@apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts`:
- Around line 91-94: Update the tool-call handling around executeTool to run all
result.toolCalls concurrently with Promise.all, passing input.userId to each
call, then append each corresponding tool result to messages with its original
call.id. Preserve the existing result ordering and message structure.
- Around line 78-83: Wrap the completeWithTools call inside the
ChatWithAssistantUseCase iteration loop with error handling, and rethrow
provider-level failures using the existing ERROR_CODES.SERVICE_UNAVAILABLE coded
error. Preserve the current tool-call processing and response fallback behavior
for successful completions, while ensuring network or non-OK provider errors
retain the service-unavailable signal.
In `@apps/api/src/use-cases/ports/ILLMProvider.ts`:
- Around line 7-14: Replace the independent-field LLMMessage interface with a
discriminated union: system/user messages should only contain content, assistant
messages may optionally contain toolCalls, and tool messages must require
toolCallId. Update dependent construction and handling in
AnthropicLLMProvider.toWireMessages and
OpenAICompatibleLLMProvider.toWireMessages as needed so they rely on the
enforced invariant instead of silently defaulting missing toolCallId values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fc97fbf4-b58b-4e9b-84e2-1371039525ac
📒 Files selected for processing (18)
apps/api/src/__tests__/application/chat/ChatWithAssistantUseCase.test.tsapps/api/src/__tests__/helpers/mocks.tsapps/api/src/__tests__/infrastructure/llm/AnthropicLLMProvider.test.tsapps/api/src/__tests__/infrastructure/llm/GoogleAILLMProvider.test.tsapps/api/src/__tests__/infrastructure/llm/OpenAICompatibleLLMProvider.test.tsapps/api/src/constants.tsapps/api/src/http/container.tsapps/api/src/http/schema/index.tsapps/api/src/http/schema/mutations/chatMutations.tsapps/api/src/http/schema/types/inputs/ChatInputs.tsapps/api/src/infrastructure/llm/AnthropicLLMProvider.tsapps/api/src/infrastructure/llm/GoogleAILLMProvider.tsapps/api/src/infrastructure/llm/OpenAICompatibleLLMProvider.tsapps/api/src/use-cases/chat/ChatWithAssistantUseCase.tsapps/api/src/use-cases/ports/ILLMProvider.tsapps/web/src/routeTree.gen.tsapps/web/src/routes/_authenticated/assistant.tsxapps/web/src/routes/_authenticated/route.tsx
| private async post(body: Record<string, unknown>): Promise<AnthropicWireResponse> { | ||
| const response = await fetch(LLM.ANTHROPIC_API_URL, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'x-api-key': this.apiKey, | ||
| 'anthropic-version': LLM.ANTHROPIC_VERSION, | ||
| }, | ||
| body: JSON.stringify({ | ||
| model: this.model, | ||
| max_tokens: maxTokens, | ||
| ...(system ? { system } : {}), | ||
| messages: conversation, | ||
| }), | ||
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const body = await response.text(); | ||
| throw new Error(`Anthropic error ${response.status}: ${body}`); | ||
| const text = await response.text(); | ||
| throw new Error(`Anthropic error ${response.status}: ${text}`); | ||
| } | ||
|
|
||
| const json = (await response.json()) as { | ||
| content?: Array<{ type: string; text?: string }>; | ||
| }; | ||
|
|
||
| return json.content?.find((block) => block.type === 'text')?.text ?? ''; | ||
| return response.json() as Promise<AnthropicWireResponse>; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No timeout on the outbound Anthropic fetch call.
If the Anthropic API hangs, this fetch will wait indefinitely, tying up the request for the chat mutation with no bound. Given this now backs an interactive, rate-limited user-facing chat feature (ChatWithAssistantUseCase), an unresponsive upstream can leave requests hanging and degrade the whole feature.
🛡️ Suggested fix
const response = await fetch(LLM.ANTHROPIC_API_URL, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'x-api-key': this.apiKey,
'anthropic-version': LLM.ANTHROPIC_VERSION,
},
body: JSON.stringify(body),
+ signal: AbortSignal.timeout(30_000),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async post(body: Record<string, unknown>): Promise<AnthropicWireResponse> { | |
| const response = await fetch(LLM.ANTHROPIC_API_URL, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'x-api-key': this.apiKey, | |
| 'anthropic-version': LLM.ANTHROPIC_VERSION, | |
| }, | |
| body: JSON.stringify({ | |
| model: this.model, | |
| max_tokens: maxTokens, | |
| ...(system ? { system } : {}), | |
| messages: conversation, | |
| }), | |
| body: JSON.stringify(body), | |
| }); | |
| if (!response.ok) { | |
| const body = await response.text(); | |
| throw new Error(`Anthropic error ${response.status}: ${body}`); | |
| const text = await response.text(); | |
| throw new Error(`Anthropic error ${response.status}: ${text}`); | |
| } | |
| const json = (await response.json()) as { | |
| content?: Array<{ type: string; text?: string }>; | |
| }; | |
| return json.content?.find((block) => block.type === 'text')?.text ?? ''; | |
| return response.json() as Promise<AnthropicWireResponse>; | |
| } | |
| private async post(body: Record<string, unknown>): Promise<AnthropicWireResponse> { | |
| const response = await fetch(LLM.ANTHROPIC_API_URL, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'x-api-key': this.apiKey, | |
| 'anthropic-version': LLM.ANTHROPIC_VERSION, | |
| }, | |
| body: JSON.stringify(body), | |
| signal: AbortSignal.timeout(30_000), | |
| }); | |
| if (!response.ok) { | |
| const text = await response.text(); | |
| throw new Error(`Anthropic error ${response.status}: ${text}`); | |
| } | |
| return response.json() as Promise<AnthropicWireResponse>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/infrastructure/llm/AnthropicLLMProvider.ts` around lines 112 -
129, Update the AnthropicLLMProvider.post method to apply a finite timeout to
the outbound fetch request, using the project’s existing timeout configuration
or convention if available. Ensure the timeout signal is passed in the fetch
options and that timeout failures propagate as errors instead of allowing the
request to hang indefinitely.
| private async post(body: Record<string, unknown>): Promise<OpenAIWireResponse> { | ||
| const response = await fetch(this.baseUrl, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `${AUTH_HEADER.BEARER_PREFIX}${this.apiKey}`, | ||
| }, | ||
| body: JSON.stringify({ model: this.model, messages, max_tokens: maxTokens }), | ||
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const body = await response.text(); | ||
| throw new Error(`LLM provider error ${response.status}: ${body}`); | ||
| const text = await response.text(); | ||
| throw new Error(`LLM provider error ${response.status}: ${text}`); | ||
| } | ||
|
|
||
| const json = (await response.json()) as { | ||
| choices: Array<{ message: { content: string } }>; | ||
| }; | ||
|
|
||
| return json.choices[0]?.message?.content ?? ''; | ||
| return response.json() as Promise<OpenAIWireResponse>; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No timeout on the outbound provider fetch call.
Same concern as AnthropicLLMProvider.post: a hung upstream response will block this request indefinitely with no bound, which is risky for an interactive chat feature.
🛡️ Suggested fix
const response = await fetch(this.baseUrl, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `${AUTH_HEADER.BEARER_PREFIX}${this.apiKey}`,
},
body: JSON.stringify(body),
+ signal: AbortSignal.timeout(30_000),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async post(body: Record<string, unknown>): Promise<OpenAIWireResponse> { | |
| const response = await fetch(this.baseUrl, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `${AUTH_HEADER.BEARER_PREFIX}${this.apiKey}`, | |
| }, | |
| body: JSON.stringify({ model: this.model, messages, max_tokens: maxTokens }), | |
| body: JSON.stringify(body), | |
| }); | |
| if (!response.ok) { | |
| const body = await response.text(); | |
| throw new Error(`LLM provider error ${response.status}: ${body}`); | |
| const text = await response.text(); | |
| throw new Error(`LLM provider error ${response.status}: ${text}`); | |
| } | |
| const json = (await response.json()) as { | |
| choices: Array<{ message: { content: string } }>; | |
| }; | |
| return json.choices[0]?.message?.content ?? ''; | |
| return response.json() as Promise<OpenAIWireResponse>; | |
| } | |
| private async post(body: Record<string, unknown>): Promise<OpenAIWireResponse> { | |
| const response = await fetch(this.baseUrl, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Authorization: `${AUTH_HEADER.BEARER_PREFIX}${this.apiKey}`, | |
| }, | |
| body: JSON.stringify(body), | |
| signal: AbortSignal.timeout(30_000), | |
| }); | |
| if (!response.ok) { | |
| const text = await response.text(); | |
| throw new Error(`LLM provider error ${response.status}: ${text}`); | |
| } | |
| return response.json() as Promise<OpenAIWireResponse>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/infrastructure/llm/OpenAICompatibleLLMProvider.ts` around lines
110 - 126, Update OpenAICompatibleLLMProvider.post to bound the outbound fetch
with an AbortController timeout, matching the timeout behavior used by
AnthropicLLMProvider.post. Pass the controller’s signal to fetch and ensure the
timeout is cleaned up after completion while preserving existing response and
error handling.
| // Only 'user'/'assistant' are accepted from the client — otherwise a | ||
| // crafted `role: "system"` entry could inject a fake system message. | ||
| if (input.history.some((m) => m.role !== 'user' && m.role !== 'assistant')) { | ||
| throw Object.assign(new Error('Invalid message role in conversation history'), { | ||
| code: ERROR_CODES.VALIDATION, | ||
| }); | ||
| } | ||
|
|
||
| const llmProvider = await this.deps.llmProviderFactory.forUser(input.userId); | ||
| if (!llmProvider) { | ||
| throw Object.assign(new Error('Add your AI API key in Settings to use this feature'), { | ||
| code: ERROR_CODES.AI_NOT_CONFIGURED, | ||
| }); | ||
| } | ||
|
|
||
| const messages: LLMMessage[] = [ | ||
| { role: 'system', content: SYSTEM_PROMPT }, | ||
| ...input.history.map((m) => ({ role: m.role, content: m.content })), | ||
| { role: 'user', content: input.message }, | ||
| ]; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
No size/length limit on message or history before calling the LLM.
The only guard against abuse is the 20-messages-per-5-minutes rate limiter; there's no cap on input.message length or input.history size. A user (or malicious client) can stay under the rate limit while sending very large messages/histories repeatedly, each triggering up to CHAT.MAX_TOOL_ITERATIONS paid LLM calls — this is an unbounded cost/latency exposure and can also blow past provider context-window limits, causing the whole turn to fail with an unhandled error.
🛡️ Suggested guard
+ const MAX_MESSAGE_LENGTH = 4000;
+ const MAX_HISTORY_LENGTH = 50;
+ if (input.message.length > MAX_MESSAGE_LENGTH || input.history.length > MAX_HISTORY_LENGTH) {
+ throw Object.assign(new Error('Message or conversation history is too long'), {
+ code: ERROR_CODES.VALIDATION,
+ });
+ }
// Only 'user'/'assistant' are accepted from the client — otherwise a
// crafted `role: "system"` entry could inject a fake system message.
if (input.history.some((m) => m.role !== 'user' && m.role !== 'assistant')) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Only 'user'/'assistant' are accepted from the client — otherwise a | |
| // crafted `role: "system"` entry could inject a fake system message. | |
| if (input.history.some((m) => m.role !== 'user' && m.role !== 'assistant')) { | |
| throw Object.assign(new Error('Invalid message role in conversation history'), { | |
| code: ERROR_CODES.VALIDATION, | |
| }); | |
| } | |
| const llmProvider = await this.deps.llmProviderFactory.forUser(input.userId); | |
| if (!llmProvider) { | |
| throw Object.assign(new Error('Add your AI API key in Settings to use this feature'), { | |
| code: ERROR_CODES.AI_NOT_CONFIGURED, | |
| }); | |
| } | |
| const messages: LLMMessage[] = [ | |
| { role: 'system', content: SYSTEM_PROMPT }, | |
| ...input.history.map((m) => ({ role: m.role, content: m.content })), | |
| { role: 'user', content: input.message }, | |
| ]; | |
| const MAX_MESSAGE_LENGTH = 4000; | |
| const MAX_HISTORY_LENGTH = 50; | |
| if (input.message.length > MAX_MESSAGE_LENGTH || input.history.length > MAX_HISTORY_LENGTH) { | |
| throw Object.assign(new Error('Message or conversation history is too long'), { | |
| code: ERROR_CODES.VALIDATION, | |
| }); | |
| } | |
| // Only 'user'/'assistant' are accepted from the client — otherwise a | |
| // crafted `role: "system"` entry could inject a fake system message. | |
| if (input.history.some((m) => m.role !== 'user' && m.role !== 'assistant')) { | |
| throw Object.assign(new Error('Invalid message role in conversation history'), { | |
| code: ERROR_CODES.VALIDATION, | |
| }); | |
| } | |
| const llmProvider = await this.deps.llmProviderFactory.forUser(input.userId); | |
| if (!llmProvider) { | |
| throw Object.assign(new Error('Add your AI API key in Settings to use this feature'), { | |
| code: ERROR_CODES.AI_NOT_CONFIGURED, | |
| }); | |
| } | |
| const messages: LLMMessage[] = [ | |
| { role: 'system', content: SYSTEM_PROMPT }, | |
| ...input.history.map((m) => ({ role: m.role, content: m.content })), | |
| { role: 'user', content: input.message }, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/use-cases/chat/ChatWithAssistantUseCase.ts` around lines 57 -
76, In ChatWithAssistantUseCase, add validation before llmProvider creation and
message construction to enforce configured maximum lengths for input.message and
input.history, including a cap on each history entry’s content if the existing
validation model supports it. Reject oversized inputs with
ERROR_CODES.VALIDATION, while preserving the existing role validation and normal
flow for inputs within limits.
| mutationFn: (message: string) => | ||
| gqlClient.request<{ sendChatMessage: string }>(SEND_CHAT_MESSAGE, { | ||
| history: messages.map((m) => ({ role: m.role, content: m.content })), | ||
| message, | ||
| }), |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Bound the client-sent conversation history.
Line 39 serialises every prior message on every request. A long session will continually grow payloads and provider context/token work. Send a capped recent window of complete turns; enforce the same limit server-side.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/_authenticated/assistant.tsx` around lines 37 - 41,
Update the mutationFn request in the assistant route to send only a capped
recent window of complete conversation turns rather than all messages,
preserving chronological order and avoiding partial turns. Apply the same
history limit in the server-side sendChatMessage handling so clients cannot
bypass the bound.
| }; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col h-[calc(100vh-3.5rem)] lg:h-screen max-w-3xl mx-auto p-4 sm:p-8"> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the composer above the mobile navigation.
This height subtracts only the 3.5rem header, while apps/web/src/routes/_authenticated/route.tsx overlays a fixed 4rem bottom nav on mobile. The composer is therefore covered by that nav. Reserve both bars.
Proposed fix
- <div className="flex flex-col h-[calc(100vh-3.5rem)] lg:h-screen max-w-3xl mx-auto p-4 sm:p-8">
+ <div className="flex flex-col h-[calc(100dvh-7.5rem)] lg:h-screen max-w-3xl mx-auto p-4 sm:p-8">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="flex flex-col h-[calc(100vh-3.5rem)] lg:h-screen max-w-3xl mx-auto p-4 sm:p-8"> | |
| <div className="flex flex-col h-[calc(100dvh-7.5rem)] lg:h-screen max-w-3xl mx-auto p-4 sm:p-8"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/_authenticated/assistant.tsx` at line 67, Update the
assistant page container height in the main layout div to reserve space for both
the mobile header and the fixed bottom navigation, using the existing responsive
desktop height unchanged. Ensure the composer remains visible above the mobile
navigation while preserving the current desktop layout.
…(JEF-12) Sending a message can involve multiple tool-call round-trips server-side before an answer comes back, so a static "Thinking..." bubble can read as stalled. Cycle through a few status phrases every 3s instead.
…at-assistant # Conflicts: # apps/api/src/constants.ts # apps/api/src/http/container.ts
1 participant
Summary
Adds an in-app AI chat assistant that lets users ask natural-language questions about their applications, contacts, and interview rounds — powered by the same 5 tools already exposed by the MCP server (
list_applications,get_application,list_notes,list_contacts,list_interview_rounds), reused in-process rather than called over HTTP.ILLMProviderextended with tool-calling support (completeWithTools) alongside the existing single-shotcomplete(). New provider-agnostic types (LLMMessage,LLMToolCall,LLMToolDefinition,LLMCompletionResult) let the orchestration use-case stay fully provider-agnostic while each concrete provider owns translation to/from its own wire format:OpenAICompatibleLLMProvider— covers 7 of 9 BYOK providers sharing OpenAI's/chat/completionsshape (OpenAI, OpenRouter, Mistral, Groq, xAI, DeepSeek, Custom).AnthropicLLMProvider— Messages APItool_use/tool_resultcontent blocks.GoogleAILLMProvider— GeminifunctionDeclarations/functionCall/functionResponse. Caveat: implemented from best understanding of the API, not verified against a live Gemini key — needs a smoke test before relying on it in production.ChatWithAssistantUseCase— a bounded agentic loop (max 5 LLM↔tool round-trips per turn) that feeds the 5 MCP tools to the LLM, executes whichever the model calls, and returns a final text answer. Tool-execution errors (e.g. a badapplicationId) are caught and fed back to the model as an error result rather than thrown, so the model can explain the problem instead of the request 500ing.{role, content}history each turn; tool-call/tool-result messages exist only within a single backendexecute()call and are never persisted or surfaced to the client.role: 'user' | 'assistant'— otherwise a craftedrole: 'system'entry could inject a fake system message.chat:${userId}key, 20 messages / 5 minutes, reusing the existingIRateLimiter.sendChatMessage(history: [ChatMessageInput!]!, message: String!): Stringmutation./assistantpage: chat UI with suggested-question chips, message bubbles, a "Thinking…" state, and anAI_NOT_CONFIGURED→ Account settings error banner (same pattern as the existing cover-letter/resume-match features). Added to both the desktop sidebar and mobile bottom nav.Ref: Linear JEF-12.
Test plan
pnpm --filter @job-finder/api test— 821 tests passing (new:ChatWithAssistantUseCase.test.tswith 15 tests covering rate limiting, role validation,AI_NOT_CONFIGURED, direct responses, tool dispatch for all 5 tools, unknown-tool/tool-error handling, multi-round loops, and max-iteration exhaustion; plus newcompleteWithToolstest blocks across all 3 LLM provider test files)pnpm --filter @job-finder/api typecheckandpnpm --filter @job-finder/web typecheck— cleanpnpm --filter @job-finder/api buildandpnpm --filter @job-finder/web build— cleanpnpm lint(both apps) — cleanAI_NOT_CONFIGUREDwith no key configured,VALIDATIONfor arole: "system"history entry, correct pipeline wiring after saving a BYOK key (fails only on the fake key upstream, proving the request reaches the real provider call), and rate limiting triggering exactly on the 21st request in a 5-minute window.DrizzleDocumentRepository.test.tstimestamp-ordering test) reproduced in full-suite runs but passes in isolation — not caused by this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01N2PBmsuzPhrmNnfZf6C3BM
Summary by CodeRabbit
New Features
Tests