feat(ai): real Anthropic/OpenAI streaming + agent routing#117
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces a new AI service layer with provider-agnostic abstractions, concrete implementations for Anthropic and OpenAI clients, a factory for instantiating clients by provider, agent-specific system prompts, and integrates real streaming AI responses into ChatService to replace simulated logic. Changes
Sequence DiagramsequenceDiagram
actor User
participant ChatService
participant CredentialService
participant AIClientFactory
participant AIClient as AIClient<br/>(Anthropic/OpenAI)
participant ExternalAPI as External AI API
User->>ChatService: requestAgentResponse()
ChatService->>CredentialService: resolveAICredentials()
CredentialService-->>ChatService: {provider, apiKey}
ChatService->>AIClientFactory: createAIClient(provider, apiKey)
AIClientFactory-->>ChatService: AIClient instance
ChatService->>ChatService: Build AI messages + system prompt
ChatService->>ChatService: Emit message_start event
ChatService->>AIClient: streamMessage(messages, systemPrompt, onChunk, signal)
AIClient->>ExternalAPI: Stream request with formatted payload
loop For each text delta
ExternalAPI-->>AIClient: Chunk with text content
AIClient->>ChatService: onChunk({text, done: false})
ChatService->>ChatService: Append chunk to message
ChatService->>ChatService: Emit message_delta event
end
ExternalAPI-->>AIClient: Stream complete
AIClient->>ChatService: onChunk({text: '', done: true})
ChatService->>ChatService: Emit message_complete event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Summary of ChangesHello @jbdevprimary, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a fundamental shift in how AI responses are generated within the application, transitioning from simulated interactions to direct, real-time streaming from Anthropic and OpenAI models. It establishes a robust, provider-agnostic AI client architecture and integrates specialized system prompts to empower distinct agent personalities, significantly enhancing the intelligence and responsiveness of the platform's AI-driven features. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR implements real AI integration with Anthropic and OpenAI, replacing mock responses with actual streaming. The architecture is well-designed with proper abstraction layers, and tests provide good coverage.
Critical Issues Found
Security (2): API keys are exposed in browser via dangerouslyAllowBrowser: true in both AI clients. This is a critical security vulnerability that must be addressed before merge.
Logic Errors (2): Credential resolution logic has fall-through bugs when API keys are configured but empty, leading to wrong provider selection or misleading error messages.
Race Condition (1): Direct Zustand store array mutation causes race conditions during streaming, potentially losing message chunks.
Missing Error Handling (2): Stream failures in both AI clients propagate uncaught, leaving UI in broken state.
All 7 critical issues have actionable fixes provided. Please address these before merging.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| this.client = new Anthropic({ | ||
| apiKey, | ||
| dangerouslyAllowBrowser: true, | ||
| }); |
There was a problem hiding this comment.
🛑 Security Vulnerability: API keys are exposed in browser environment via dangerouslyAllowBrowser: true. This bypasses CORS protections and exposes credentials client-side, allowing malicious scripts or browser extensions to steal API keys.1
Implement a secure backend proxy to handle AI API calls instead of calling the Anthropic API directly from the browser.
Footnotes
-
CWE-522: Insufficiently Protected Credentials - https://cwe.mitre.org/data/definitions/522.html ↩
| this.client = new OpenAI({ | ||
| apiKey, | ||
| dangerouslyAllowBrowser: true, | ||
| }); |
There was a problem hiding this comment.
🛑 Security Vulnerability: API keys are exposed in browser environment via dangerouslyAllowBrowser: true. This bypasses CORS protections and exposes credentials client-side, allowing malicious scripts or browser extensions to steal API keys.1
Implement a secure backend proxy to handle AI API calls instead of calling the OpenAI API directly from the browser.
Footnotes
-
CWE-522: Insufficiently Protected Credentials - https://cwe.mitre.org/data/definitions/522.html ↩
| // Update message content in the store | ||
| const store = useChatStore.getState(); | ||
| const threadMessages = store.messages[threadId] || []; | ||
| const messageIndex = threadMessages.findIndex((m) => m.id === messageId); | ||
| if (messageIndex !== -1) { | ||
| threadMessages[messageIndex].content = currentContent; | ||
| } |
There was a problem hiding this comment.
🛑 Race Condition: Directly mutating the Zustand store array causes race conditions during concurrent streaming. Multiple chunks can arrive simultaneously, causing state mutations to be lost or overwrite each other, resulting in incomplete message content.
Use Zustand's immutable update pattern with a dedicated action to ensure atomic updates.
| // Update message content in the store | |
| const store = useChatStore.getState(); | |
| const threadMessages = store.messages[threadId] || []; | |
| const messageIndex = threadMessages.findIndex((m) => m.id === messageId); | |
| if (messageIndex !== -1) { | |
| threadMessages[messageIndex].content = currentContent; | |
| } | |
| // Update message content in the store | |
| useChatStore.getState().updateMessageContent?.(messageId, threadId, currentContent); |
| async streamMessage( | ||
| messages: AIMessage[], | ||
| systemPrompt: string, | ||
| onChunk: (chunk: AIStreamChunk) => void, | ||
| signal?: AbortSignal | ||
| ): Promise<void> { | ||
| const stream = await this.client.chat.completions.create( | ||
| { | ||
| model: this.model, | ||
| stream: true, | ||
| messages: [ | ||
| { role: 'system', content: systemPrompt }, | ||
| ...messages.filter((m) => m.role !== 'system'), | ||
| ], | ||
| }, | ||
| { signal } | ||
| ); | ||
|
|
||
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta?.content; | ||
| if (delta) { | ||
| onChunk({ text: delta, done: false }); | ||
| } | ||
| } | ||
|
|
||
| onChunk({ text: '', done: true }); | ||
| } |
There was a problem hiding this comment.
Missing error handling for API failures. If the stream encounters an error (network failure, rate limiting, invalid API key), the error propagates uncaught, leaving the UI in a pending state with no error message shown to the user.
| async streamMessage( | |
| messages: AIMessage[], | |
| systemPrompt: string, | |
| onChunk: (chunk: AIStreamChunk) => void, | |
| signal?: AbortSignal | |
| ): Promise<void> { | |
| const stream = await this.client.chat.completions.create( | |
| { | |
| model: this.model, | |
| stream: true, | |
| messages: [ | |
| { role: 'system', content: systemPrompt }, | |
| ...messages.filter((m) => m.role !== 'system'), | |
| ], | |
| }, | |
| { signal } | |
| ); | |
| for await (const chunk of stream) { | |
| const delta = chunk.choices[0]?.delta?.content; | |
| if (delta) { | |
| onChunk({ text: delta, done: false }); | |
| } | |
| } | |
| onChunk({ text: '', done: true }); | |
| } | |
| async streamMessage( | |
| messages: AIMessage[], | |
| systemPrompt: string, | |
| onChunk: (chunk: AIStreamChunk) => void, | |
| signal?: AbortSignal | |
| ): Promise<void> { | |
| try { | |
| const stream = await this.client.chat.completions.create( | |
| { | |
| model: this.model, | |
| stream: true, | |
| messages: [ | |
| { role: 'system', content: systemPrompt }, | |
| ...messages.filter((m) => m.role !== 'system'), | |
| ], | |
| }, | |
| { signal } | |
| ); | |
| for await (const chunk of stream) { | |
| const delta = chunk.choices[0]?.delta?.content; | |
| if (delta) { | |
| onChunk({ text: delta, done: false }); | |
| } | |
| } | |
| onChunk({ text: '', done: true }); | |
| } catch (error) { | |
| if (signal?.aborted) { | |
| throw new DOMException('Aborted', 'AbortError'); | |
| } | |
| throw error; | |
| } | |
| } |
| if (openaiMeta) { | ||
| const result = await CredentialService.retrieve('openai'); | ||
| if (result.secret) { | ||
| return { provider: 'openai', apiKey: result.secret }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: If OpenAI credential exists but result.secret is empty/falsy, the function throws a generic "No AI API key configured" error instead of indicating that the OpenAI key is configured but invalid. This misleads users into thinking no key exists when one is actually configured.
| if (openaiMeta) { | |
| const result = await CredentialService.retrieve('openai'); | |
| if (result.secret) { | |
| return { provider: 'openai', apiKey: result.secret }; | |
| } | |
| } | |
| if (openaiMeta) { | |
| const result = await CredentialService.retrieve('openai'); | |
| if (result.secret) { | |
| return { provider: 'openai', apiKey: result.secret }; | |
| } | |
| throw new Error('OpenAI API key is configured but empty. Please update it in Settings.'); | |
| } |
| const stream = this.client.messages.stream( | ||
| { | ||
| model: this.model, | ||
| max_tokens: MAX_TOKENS, | ||
| system: systemPrompt, | ||
| messages: messages | ||
| .filter((m) => m.role !== 'system') | ||
| .map((m) => ({ | ||
| role: m.role as 'user' | 'assistant', | ||
| content: m.content, | ||
| })), | ||
| }, | ||
| { signal } | ||
| ); | ||
|
|
||
| stream.on('text', (textDelta) => { | ||
| onChunk({ text: textDelta, done: false }); | ||
| }); | ||
|
|
||
| // Wait for the stream to complete | ||
| await stream.finalMessage(); | ||
|
|
||
| onChunk({ text: '', done: true }); | ||
| } |
There was a problem hiding this comment.
Missing error handling for API failures. If the stream encounters an error (network failure, rate limiting, invalid API key), the error propagates uncaught, leaving the UI in a pending state with no error message shown to the user.
| const stream = this.client.messages.stream( | |
| { | |
| model: this.model, | |
| max_tokens: MAX_TOKENS, | |
| system: systemPrompt, | |
| messages: messages | |
| .filter((m) => m.role !== 'system') | |
| .map((m) => ({ | |
| role: m.role as 'user' | 'assistant', | |
| content: m.content, | |
| })), | |
| }, | |
| { signal } | |
| ); | |
| stream.on('text', (textDelta) => { | |
| onChunk({ text: textDelta, done: false }); | |
| }); | |
| // Wait for the stream to complete | |
| await stream.finalMessage(); | |
| onChunk({ text: '', done: true }); | |
| } | |
| async streamMessage( | |
| messages: AIMessage[], | |
| systemPrompt: string, | |
| onChunk: (chunk: AIStreamChunk) => void, | |
| signal?: AbortSignal | |
| ): Promise<void> { | |
| try { | |
| const stream = this.client.messages.stream( | |
| { | |
| model: this.model, | |
| max_tokens: MAX_TOKENS, | |
| system: systemPrompt, | |
| messages: messages | |
| .filter((m) => m.role !== 'system') | |
| .map((m) => ({ | |
| role: m.role as 'user' | 'assistant', | |
| content: m.content, | |
| })), | |
| }, | |
| { signal } | |
| ); | |
| stream.on('text', (textDelta) => { | |
| onChunk({ text: textDelta, done: false }); | |
| }); | |
| // Wait for the stream to complete | |
| await stream.finalMessage(); | |
| onChunk({ text: '', done: true }); | |
| } catch (error) { | |
| if (signal?.aborted) { | |
| throw new DOMException('Aborted', 'AbortError'); | |
| } | |
| throw error; | |
| } | |
| } |
| if (anthropicMeta) { | ||
| const result = await CredentialService.retrieve('anthropic'); | ||
| if (result.secret) { | ||
| return { provider: 'anthropic', apiKey: result.secret }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: If both Anthropic and OpenAI credentials exist but result.secret is empty/falsy (e.g., empty string stored), the function falls through to the next provider check instead of throwing an error. This causes the wrong provider to be selected or a misleading error message.
| if (anthropicMeta) { | |
| const result = await CredentialService.retrieve('anthropic'); | |
| if (result.secret) { | |
| return { provider: 'anthropic', apiKey: result.secret }; | |
| } | |
| } | |
| // Try anthropic first, then openai | |
| if (anthropicMeta) { | |
| const result = await CredentialService.retrieve('anthropic'); | |
| if (result.secret) { | |
| return { provider: 'anthropic', apiKey: result.secret }; | |
| } | |
| throw new Error('Anthropic API key is configured but empty. Please update it in Settings.'); | |
| } |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| // We need to get the stream mock to check its call | ||
| const Anthropic = jest.requireMock('@anthropic-ai/sdk').default; | ||
| const instance = new Anthropic(); |
|
🚀 Expo preview is ready!
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces real-time streaming AI responses from Anthropic and OpenAI, replacing the previous mock implementation. It includes a new AI client factory, provider-specific client implementations, and agent-specific system prompts. The integration in ChatService correctly handles credential resolution, message formatting, and streaming updates, following best practices for client-side AI integration using SecureStore for keys. However, there is a security concern regarding error handling where sensitive API keys could be leaked into insecure storage (AsyncStorage) if they are included in error messages from the AI providers.
| const errorContent = | ||
| error.message.includes('API key') || error.message.includes('No AI') | ||
| ? error.message | ||
| : `Failed to get response: ${error.message}`; | ||
|
|
||
| useChatStore.getState().addMessage({ | ||
| threadId, | ||
| sender: 'system', | ||
| content: errorContent, | ||
| contentType: 'text', | ||
| }); |
There was a problem hiding this comment.
The error handling logic here potentially leaks sensitive API keys into insecure storage. If an error occurs that includes the API key in its message (e.g., from the AI SDK), the errorContent will contain the full message and be added to the chatStore. Since chatStore persists messages to AsyncStorage (which is unencrypted), this results in credentials being moved from SecureStore to insecure storage.
Consider sanitizing the error message or using a generic message for authentication-related errors.
| const errorContent = | |
| error.message.includes('API key') || error.message.includes('No AI') | |
| ? error.message | |
| : `Failed to get response: ${error.message}`; | |
| useChatStore.getState().addMessage({ | |
| threadId, | |
| sender: 'system', | |
| content: errorContent, | |
| contentType: 'text', | |
| }); | |
| const errorContent = | |
| error.message.includes('API key') || error.message.includes('No AI') | |
| ? 'Authentication error: Please check your AI API key in Settings.' | |
| : `Failed to get response: ${error.message}`; | |
| useChatStore.getState().addMessage({ | |
| threadId, | |
| sender: 'system', | |
| content: errorContent, | |
| contentType: 'text', | |
| }); |
| constructor(apiKey: string, model: string = DEFAULT_MODEL) { | ||
| this.client = new Anthropic({ | ||
| apiKey, | ||
| dangerouslyAllowBrowser: true, |
There was a problem hiding this comment.
The dangerouslyAllowBrowser: true option is used for the Anthropic SDK. While this might be necessary for certain client-side environments (like React Native's WebView), it's generally discouraged for security reasons as it can expose API keys if not handled with extreme care. Please ensure that API keys are always securely managed and never directly exposed in client-side code bundles. If this is intended for a secure client-side environment, a comment justifying its use would be beneficial.
| constructor(apiKey: string, model: string = DEFAULT_MODEL) { | ||
| this.client = new OpenAI({ | ||
| apiKey, | ||
| dangerouslyAllowBrowser: true, |
There was a problem hiding this comment.
Similar to the Anthropic client, dangerouslyAllowBrowser: true is used here. Please ensure that API keys are securely managed and not exposed in client-side code. If this is a deliberate choice for a secure client-side environment, consider adding a comment to explain the rationale.
| // Get thread context (recent messages for context) | ||
| const messages = this.getMessages(threadId); | ||
| const recentMessages = messages.slice(-10); // Last 10 messages for context | ||
| const recentMessages = messages.slice(-10); |
There was a problem hiding this comment.
The number 10 for recentMessages is a magic number. It would be more maintainable and readable to define this as a named constant, perhaps at the top of the file or in a configuration object, to clearly indicate its purpose and allow for easier modification.
| const recentMessages = messages.slice(-10); | |
| const recentMessages = messages.slice(-MAX_CONTEXT_MESSAGES); |
…ntations Create provider-agnostic AIClient interface with types for messages and streaming chunks. Implement AnthropicClient using @anthropic-ai/sdk with message streaming via the stream() API, and OpenAIClient using the openai SDK with async iterator streaming. Add AIClientFactory for provider-based client creation. Includes comprehensive unit tests for all components. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove simulateAgentResponse() and getAgentResponsePlaceholder() mock methods. Integrate AIClientFactory with CredentialService to resolve the user's AI provider and API key from secure storage. Stream real AI responses through the existing message delta event system. Add error handling that displays helpful messages when no API key is configured. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Create AgentPrompts module with specialized system prompts for each agent type: Architect (system design), Implementer (code generation), Reviewer (code review), and Tester (test writing). Wire ChatService to use agent-specific prompts when calling the AI client, giving each agent a distinct personality and area of expertise. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jbdevprimary
force-pushed
the
ralph/ws-ai-integration
branch
from
19d6483 to
ad02d68
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/services/ai/__tests__/AnthropicClient.test.ts`:
- Around line 141-160: The test currently doesn't assert that the abort signal
is forwarded to the SDK stream; after calling AnthropicClient.streamMessage,
inspect the mocked stream call args and assert the signal was passed.
Concretely, use the mocked SDK (jest.requireMock('@anthropic-ai/sdk').default)
to access the created mock instance (e.g., Anthropic.mock.instances[0]) or
assert against mockStream.mock.calls to find the call from
testClient.streamMessage and expect the last argument to equal
controller.signal; keep existing mocks (mockStreamOn, mockStreamFinalMessage)
and the onChunk assertion but add the signal assertion to verify proper
propagation.
- Around line 111-139: The test currently races handlers vs final message so
text chunks are never captured; update the mocked stream behavior in the test
for client.streamMessage by either calling the provided handler synchronously
inside mockStreamOn (invoke handler('Hello') and handler(' world') immediately
when event === 'text') or by changing mockStreamFinalMessage to resolve only
after the queued text callbacks have executed (e.g., call handlers then resolve
mockStreamFinalMessage), so that onChunk receives the 'Hello' and ' world'
chunks before the final { text: '', done: true } is appended; adjust assertions
on the chunks array accordingly to verify all text chunks plus the done chunk.
In `@src/services/ai/AnthropicClient.ts`:
- Around line 70-79: The final "done" chunk (onChunk({ text: '', done: true }))
is never emitted if stream.finalMessage() rejects (due to abort or error); wrap
the await stream.finalMessage() and subsequent onChunk call in a try/finally so
onChunk({ text: '', done: true }) is always called regardless of errors/abort.
Update the block around stream.on('text', ...) and the await
stream.finalMessage() in AnthropicClient.ts to use try/finally (or equivalent)
so the final done signal is emitted even when stream.finalMessage() throws or
the abort signal fires.
- Around line 19-22: Rename the file containing the AnthropicClient class from
"AnthropicClient.ts" to "anthropic-client.ts" to follow kebab-case; then update
the export in src/services/ai/index.ts to import/export the class from
"anthropic-client" (and update any other imports that reference
"AnthropicClient" accordingly), ensuring the class name AnthropicClient and the
usage of new Anthropic({ apiKey, dangerouslyAllowBrowser: true }) remain
unchanged.
In `@src/services/ai/OpenAIClient.ts`:
- Around line 62-69: The final "done" chunk is skipped if the streaming loop in
OpenAIClient throws or is aborted; wrap the "for await (const chunk of stream)"
loop in a try/finally and call onChunk({ text: '', done: true }) in the finally
block so the done signal is always emitted (refer to the stream variable and
onChunk callback in the OpenAIClient streaming method).
In `@src/services/chat/ChatService.ts`:
- Around line 290-298: Currently the code directly mutates the store array by
assigning to threadMessages[messageIndex].content which bypasses Zustand's
set/subscription mechanism; replace this with a call to a store action (e.g.,
add an updateMessageContent action on useChatStore) that performs an immutable
update via set(...) to replace the message in messages[threadId] (mapping over
state.messages[threadId] and returning { ...m, content } for the matching
messageId), and invoke useChatStore.getState().updateMessageContent(messageId,
threadId, currentContent) instead of mutating threadMessages in-place.
🧹 Nitpick comments (4)
src/services/ai/AgentPrompts.ts (1)
10-10: Use a stricter key type forAGENT_PROMPTS.
Record<string, string>loses compile-time safety. IfMessageSendervalues change, this map won't flag missing or extraneous keys. Consider aPartial<Record<MessageSender, string>>(or a fullRecordif every sender should have a prompt).Proposed fix
-const AGENT_PROMPTS: Record<string, string> = { +const AGENT_PROMPTS: Partial<Record<MessageSender, string>> = {src/services/chat/ChatService.ts (2)
258-261: Hardcoded context window of 10 messages.The
slice(-10)limit should be a named constant for discoverability and tuning. Also, 10 messages may be insufficient for complex multi-turn agent interactions, and there's no token-budget awareness — a few long code messages could blow past the model's context window even within 10 messages.Suggested improvement
+const MAX_CONTEXT_MESSAGES = 10; + // ... - const recentMessages = messages.slice(-10); + const recentMessages = messages.slice(-MAX_CONTEXT_MESSAGES);
320-340: Error handling looks solid but relies on fragile string matching.The
error.message.includes('API key') || error.message.includes('No AI')check on line 324 couples error presentation to exact wording inresolveAICredentials. If that error message changes, users will get the generic fallback instead of the helpful message.Consider introducing a typed error class (e.g.,
AICredentialError) and usinginstanceofinstead:Suggested approach
class AICredentialError extends Error { constructor(message: string) { super(message); this.name = 'AICredentialError'; } }Then in the catch block:
- const errorContent = - error.message.includes('API key') || error.message.includes('No AI') - ? error.message - : `Failed to get response: ${error.message}`; + const errorContent = + error instanceof AICredentialError + ? error.message + : `Failed to get response: ${error.message}`;src/services/ai/__tests__/AnthropicClient.test.ts (1)
54-54: Consider updating to the latest Claude Sonnet model.The test correctly asserts
'claude-sonnet-4-20250514'matching theDEFAULT_MODELconstant inAnthropicClient.ts. The model identifier is valid, but it is not the current Sonnet line—Claude Sonnet 4.5 (claude-sonnet-4-5-20250929) is now the latest generation. The current model has a tentative retirement not sooner than May 14, 2026. If you need the newest features and performance, consider updating toclaude-sonnet-4-5-20250929or the moving aliasclaude-sonnet-4-5.
| it('should stream message chunks and call onChunk', async () => { | ||
| const chunks: AIStreamChunk[] = []; | ||
| const onChunk = (chunk: AIStreamChunk) => chunks.push(chunk); | ||
|
|
||
| // Simulate the stream: when 'text' event handler is registered, call it | ||
| mockStreamOn.mockImplementation(function ( | ||
| this: { on: typeof mockStreamOn; finalMessage: typeof mockStreamFinalMessage }, | ||
| event: string, | ||
| handler: (text: string) => void | ||
| ) { | ||
| if (event === 'text') { | ||
| // Queue the text callbacks | ||
| setTimeout(() => { | ||
| handler('Hello'); | ||
| handler(' world'); | ||
| }, 0); | ||
| } | ||
| return this; | ||
| }); | ||
|
|
||
| mockStreamFinalMessage.mockResolvedValue({ | ||
| content: [{ type: 'text', text: 'Hello world' }], | ||
| }); | ||
|
|
||
| await client.streamMessage(testMessages, testSystemPrompt, onChunk); | ||
|
|
||
| // The done chunk is always sent | ||
| expect(chunks[chunks.length - 1]).toEqual({ text: '', done: true }); | ||
| }); |
There was a problem hiding this comment.
Stream test doesn't actually verify text chunk delivery.
Because mockStreamFinalMessage resolves as a microtask while the setTimeout(() => { handler('Hello'); handler(' world'); }, 0) fires as a macrotask, the text handlers execute after streamMessage has already returned. This means chunks will only contain the final { text: '', done: true } — the 'Hello' and ' world' chunks are never captured.
Consider invoking the handler synchronously within mockStreamOn or using a deferred pattern where finalMessage resolves only after the text callbacks have fired:
Proposed fix
mockStreamOn.mockImplementation(function (
this: { on: typeof mockStreamOn; finalMessage: typeof mockStreamFinalMessage },
event: string,
handler: (text: string) => void
) {
if (event === 'text') {
- // Queue the text callbacks
- setTimeout(() => {
- handler('Hello');
- handler(' world');
- }, 0);
+ // Call handlers synchronously so they fire before finalMessage resolves
+ handler('Hello');
+ handler(' world');
}
return this;
});Then assert all chunks were received:
- // The done chunk is always sent
- expect(chunks[chunks.length - 1]).toEqual({ text: '', done: true });
+ expect(chunks).toEqual([
+ { text: 'Hello', done: false },
+ { text: ' world', done: false },
+ { text: '', done: true },
+ ]);🤖 Prompt for AI Agents
In `@src/services/ai/__tests__/AnthropicClient.test.ts` around lines 111 - 139,
The test currently races handlers vs final message so text chunks are never
captured; update the mocked stream behavior in the test for client.streamMessage
by either calling the provided handler synchronously inside mockStreamOn (invoke
handler('Hello') and handler(' world') immediately when event === 'text') or by
changing mockStreamFinalMessage to resolve only after the queued text callbacks
have executed (e.g., call handlers then resolve mockStreamFinalMessage), so that
onChunk receives the 'Hello' and ' world' chunks before the final { text: '',
done: true } is appended; adjust assertions on the chunks array accordingly to
verify all text chunks plus the done chunk.
| it('should pass the abort signal to the stream', async () => { | ||
| const onChunk = jest.fn(); | ||
| const controller = new AbortController(); | ||
|
|
||
| mockStreamOn.mockReturnThis(); | ||
| mockStreamFinalMessage.mockResolvedValue({ | ||
| content: [{ type: 'text', text: '' }], | ||
| }); | ||
|
|
||
| // We need to get the stream mock to check its call | ||
| const Anthropic = jest.requireMock('@anthropic-ai/sdk').default; | ||
| const instance = new Anthropic(); | ||
|
|
||
| // Create a new client that uses this instance | ||
| const testClient = new AnthropicClient('test-key'); | ||
| await testClient.streamMessage(testMessages, testSystemPrompt, onChunk, controller.signal); | ||
|
|
||
| // Verify stream was called (indirectly through the mock) | ||
| expect(onChunk).toHaveBeenCalledWith({ text: '', done: true }); | ||
| }); |
There was a problem hiding this comment.
Abort signal test doesn't verify signal propagation.
The test is titled "should pass the abort signal to the stream" but never asserts that stream(...) was called with the signal. Lines 151-153 create a separate Anthropic mock instance that is unrelated to the one used internally by testClient. The only assertion (line 159) checks that the done chunk was emitted, which has nothing to do with signal propagation.
To actually verify signal forwarding, assert against the stream mock's call args:
Proposed approach
const testClient = new AnthropicClient('test-key');
await testClient.streamMessage(testMessages, testSystemPrompt, onChunk, controller.signal);
- // Verify stream was called (indirectly through the mock)
- expect(onChunk).toHaveBeenCalledWith({ text: '', done: true });
+ // Verify the abort signal was forwarded to the stream call
+ const streamMock = instance.messages.stream;
+ expect(streamMock).toHaveBeenCalledWith(
+ expect.any(Object),
+ expect.objectContaining({ signal: controller.signal })
+ );🤖 Prompt for AI Agents
In `@src/services/ai/__tests__/AnthropicClient.test.ts` around lines 141 - 160,
The test currently doesn't assert that the abort signal is forwarded to the SDK
stream; after calling AnthropicClient.streamMessage, inspect the mocked stream
call args and assert the signal was passed. Concretely, use the mocked SDK
(jest.requireMock('@anthropic-ai/sdk').default) to access the created mock
instance (e.g., Anthropic.mock.instances[0]) or assert against
mockStream.mock.calls to find the call from testClient.streamMessage and expect
the last argument to equal controller.signal; keep existing mocks (mockStreamOn,
mockStreamFinalMessage) and the onChunk assertion but add the signal assertion
to verify proper propagation.
| this.client = new Anthropic({ | ||
| apiKey, | ||
| dangerouslyAllowBrowser: true, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the services/ai directory to understand the structure
find src/services/ai -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: agentic-dev-library/thumbcode
Length of output: 450
🏁 Script executed:
# Look for AnthropicClient.ts and related AI clients
fd "AnthropicClient|OpenAIClient" --type fRepository: agentic-dev-library/thumbcode
Length of output: 236
🏁 Script executed:
# Check how API keys are passed to AI clients - search for instantiation patterns
rg "new Anthropic|new OpenAI|AnthropicClient|OpenAIClient" -A 3 -B 3 --type ts --type tsxRepository: agentic-dev-library/thumbcode
Length of output: 100
🏁 Script executed:
# Search for expo-secure-store usage in the codebase
rg "expo-secure-store|getSecureData|setSecureData" --type ts --type tsxRepository: agentic-dev-library/thumbcode
Length of output: 100
🏁 Script executed:
# Check filename conventions in src/services, src/components, src/hooks
ls -la src/services/ 2>/dev/null | head -20
ls -la src/components/ 2>/dev/null | head -20Repository: agentic-dev-library/thumbcode
Length of output: 1183
🏁 Script executed:
# Read AnthropicClient.ts to see full context
cat -n src/services/ai/AnthropicClient.tsRepository: agentic-dev-library/thumbcode
Length of output: 2658
🏁 Script executed:
# Read AIClientFactory.ts to understand how API keys are obtained
cat -n src/services/ai/AIClientFactory.tsRepository: agentic-dev-library/thumbcode
Length of output: 1140
🏁 Script executed:
# Search for expo-secure-store usage without type filter
rg "expo-secure-store|getSecureData|setSecureData|SecureStore" -iRepository: agentic-dev-library/thumbcode
Length of output: 9797
🏁 Script executed:
# Check actual component filenames for naming pattern
find src/components -maxdepth 2 -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: agentic-dev-library/thumbcode
Length of output: 753
🏁 Script executed:
# Check if there's a hooks directory and its naming
find src/hooks -type f -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -20Repository: agentic-dev-library/thumbcode
Length of output: 122
🏁 Script executed:
# Search for where createAIClient is called and how the apiKey is obtained
rg "createAIClient" -B 5 -A 2Repository: agentic-dev-library/thumbcode
Length of output: 13604
🏁 Script executed:
# Look for CredentialService usage patterns to see if API keys are retrieved before passing to clients
rg "CredentialService|getCredential|API.*key|apiKey" src/ -i | grep -i "anthropic\|openai\|credential" | head -30Repository: agentic-dev-library/thumbcode
Length of output: 2043
🏁 Script executed:
# Check the specific onboarding flow to see credential retrieval pattern
cat -n app/\(onboarding\)/api-keys.tsx 2>/dev/null || find . -name "api-keys.tsx" -exec cat -n {} \;Repository: agentic-dev-library/thumbcode
Length of output: 11762
🏁 Script executed:
# Check component file naming patterns
find src/components -maxdepth 3 -type f \( -name "*.tsx" -o -name "*.ts" \) | head -30Repository: agentic-dev-library/thumbcode
Length of output: 1133
🏁 Script executed:
# Check services file naming patterns
find src/services -maxdepth 3 -type f \( -name "*.ts" -o -name "*.tsx" \) | head -30Repository: agentic-dev-library/thumbcode
Length of output: 580
🏁 Script executed:
# Look at ChatService.ts to confirm the full API key flow
cat -n src/services/chat/ChatService.ts | head -80Repository: agentic-dev-library/thumbcode
Length of output: 2584
Rename file to anthropic-client.ts to follow kebab-case naming convention.
Per coding guidelines, file names should use kebab-case (e.g., agent-card.tsx). Rename AnthropicClient.ts → anthropic-client.ts. Update the corresponding export in src/services/ai/index.ts.
Note: The dangerouslyAllowBrowser: true flag is necessary and intentional for React Native/Expo context—it's not a security concern. API keys are properly secured via expo-secure-store as required; they flow from CredentialService.retrieve() at runtime and are never stored in code.
🤖 Prompt for AI Agents
In `@src/services/ai/AnthropicClient.ts` around lines 19 - 22, Rename the file
containing the AnthropicClient class from "AnthropicClient.ts" to
"anthropic-client.ts" to follow kebab-case; then update the export in
src/services/ai/index.ts to import/export the class from "anthropic-client" (and
update any other imports that reference "AnthropicClient" accordingly), ensuring
the class name AnthropicClient and the usage of new Anthropic({ apiKey,
dangerouslyAllowBrowser: true }) remain unchanged.
|
|
||
| stream.on('text', (textDelta) => { | ||
| onChunk({ text: textDelta, done: false }); | ||
| }); | ||
|
|
||
| // Wait for the stream to complete | ||
| await stream.finalMessage(); | ||
|
|
||
| onChunk({ text: '', done: true }); | ||
| } |
There was a problem hiding this comment.
onChunk({ done: true }) is skipped if the stream errors or is aborted.
If the abort signal fires or the stream encounters an error, stream.finalMessage() will reject, and the final done chunk on Line 78 is never emitted. Callers relying on the done signal to finalize UI state (e.g., stop a loading indicator) will be left hanging.
Consider wrapping in try/finally:
Proposed fix
stream.on('text', (textDelta) => {
onChunk({ text: textDelta, done: false });
});
- // Wait for the stream to complete
- await stream.finalMessage();
-
- onChunk({ text: '', done: true });
+ try {
+ await stream.finalMessage();
+ } finally {
+ onChunk({ text: '', done: true });
+ }📝 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.
| stream.on('text', (textDelta) => { | |
| onChunk({ text: textDelta, done: false }); | |
| }); | |
| // Wait for the stream to complete | |
| await stream.finalMessage(); | |
| onChunk({ text: '', done: true }); | |
| } | |
| stream.on('text', (textDelta) => { | |
| onChunk({ text: textDelta, done: false }); | |
| }); | |
| try { | |
| await stream.finalMessage(); | |
| } finally { | |
| onChunk({ text: '', done: true }); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/services/ai/AnthropicClient.ts` around lines 70 - 79, The final "done"
chunk (onChunk({ text: '', done: true })) is never emitted if
stream.finalMessage() rejects (due to abort or error); wrap the await
stream.finalMessage() and subsequent onChunk call in a try/finally so onChunk({
text: '', done: true }) is always called regardless of errors/abort. Update the
block around stream.on('text', ...) and the await stream.finalMessage() in
AnthropicClient.ts to use try/finally (or equivalent) so the final done signal
is emitted even when stream.finalMessage() throws or the abort signal fires.
| for await (const chunk of stream) { | ||
| const delta = chunk.choices[0]?.delta?.content; | ||
| if (delta) { | ||
| onChunk({ text: delta, done: false }); | ||
| } | ||
| } | ||
|
|
||
| onChunk({ text: '', done: true }); |
There was a problem hiding this comment.
Same done chunk issue as AnthropicClient — final onChunk({ done: true }) is skipped on error/abort.
If the for await loop throws (e.g., abort signal, network error), the done chunk on Line 69 is never emitted. Wrap in try/finally for consistent behavior.
Proposed fix
- for await (const chunk of stream) {
- const delta = chunk.choices[0]?.delta?.content;
- if (delta) {
- onChunk({ text: delta, done: false });
+ try {
+ for await (const chunk of stream) {
+ const delta = chunk.choices[0]?.delta?.content;
+ if (delta) {
+ onChunk({ text: delta, done: false });
+ }
}
+ } finally {
+ onChunk({ text: '', done: true });
}
-
- onChunk({ text: '', done: true });📝 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.
| for await (const chunk of stream) { | |
| const delta = chunk.choices[0]?.delta?.content; | |
| if (delta) { | |
| onChunk({ text: delta, done: false }); | |
| } | |
| } | |
| onChunk({ text: '', done: true }); | |
| try { | |
| for await (const chunk of stream) { | |
| const delta = chunk.choices[0]?.delta?.content; | |
| if (delta) { | |
| onChunk({ text: delta, done: false }); | |
| } | |
| } | |
| } finally { | |
| onChunk({ text: '', done: true }); | |
| } |
🤖 Prompt for AI Agents
In `@src/services/ai/OpenAIClient.ts` around lines 62 - 69, The final "done" chunk
is skipped if the streaming loop in OpenAIClient throws or is aborted; wrap the
"for await (const chunk of stream)" loop in a try/finally and call onChunk({
text: '', done: true }) in the finally block so the done signal is always
emitted (refer to the stream variable and onChunk callback in the OpenAIClient
streaming method).
| currentContent += chunk.text; | ||
|
|
||
| // Update message content in the store | ||
| const store = useChatStore.getState(); | ||
| const threadMessages = store.messages[threadId] || []; | ||
| const messageIndex = threadMessages.findIndex((m) => m.id === messageId); | ||
| if (messageIndex !== -1) { | ||
| threadMessages[messageIndex].content = currentContent; | ||
| } |
There was a problem hiding this comment.
Direct mutation of Zustand store state — UI won't re-render during streaming.
useChatStore.getState().messages[threadId] returns a reference to the store's internal array. Mutating threadMessages[messageIndex].content in-place bypasses Zustand's subscription mechanism, so components using useChatStore selectors on messages will not re-render as chunks arrive.
You should use a proper store action (e.g., updateMessageContent) that calls set(...) internally:
Proposed fix
- // Update message content in the store
- const store = useChatStore.getState();
- const threadMessages = store.messages[threadId] || [];
- const messageIndex = threadMessages.findIndex((m) => m.id === messageId);
- if (messageIndex !== -1) {
- threadMessages[messageIndex].content = currentContent;
- }
+ // Update message content via store action to trigger re-renders
+ useChatStore.getState().updateMessageContent(messageId, threadId, currentContent);If updateMessageContent doesn't exist yet, add it to the chat store with an immutable update:
updateMessageContent: (messageId: string, threadId: string, content: string) =>
set((state) => ({
messages: {
...state.messages,
[threadId]: state.messages[threadId].map((m) =>
m.id === messageId ? { ...m, content } : m
),
},
})),Based on learnings: "Use Zustand for state management" — Zustand requires immutable state updates via set() for subscriptions to trigger.
🤖 Prompt for AI Agents
In `@src/services/chat/ChatService.ts` around lines 290 - 298, Currently the code
directly mutates the store array by assigning to
threadMessages[messageIndex].content which bypasses Zustand's set/subscription
mechanism; replace this with a call to a store action (e.g., add an
updateMessageContent action on useChatStore) that performs an immutable update
via set(...) to replace the message in messages[threadId] (mapping over
state.messages[threadId] and returning { ...m, content } for the matching
messageId), and invoke useChatStore.getState().updateMessageContent(messageId,
threadId, currentContent) instead of mutating threadMessages in-place.
|
1 participant
Summary
The core feature — real AI responses from user's own API keys.
Test plan
🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
Release Notes
New Features
Tests