Skip to content

Commit bcf879f

Browse files
Copilothotlong
andauthored
refactor: address code review — extract stream helpers, improve type narrowing
Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/7b5017d9-7b75-41e6-a0ad-6eb250b8d770 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 27f18e4 commit bcf879f

4 files changed

Lines changed: 48 additions & 30 deletions

File tree

packages/services/service-ai/src/__tests__/chatbot-features.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,13 @@ describe('AIService.chatWithTools', () => {
259259
// The error message should be in the tool result
260260
const secondCallMessages = (adapter.chat as any).mock.calls[1][0] as ModelMessage[];
261261
const toolMsg = secondCallMessages.find(m => m.role === 'tool');
262-
const toolContent = Array.isArray(toolMsg?.content) ? (toolMsg.content[0] as any)?.output?.value : toolMsg?.content;
262+
let toolContent: string | undefined;
263+
if (toolMsg?.role === 'tool' && Array.isArray(toolMsg.content)) {
264+
const firstResult = toolMsg.content[0];
265+
if ('output' in firstResult && firstResult.output && typeof firstResult.output === 'object' && 'value' in firstResult.output) {
266+
toolContent = String(firstResult.output.value);
267+
}
268+
}
263269
expect(toolContent).toContain('Tool crashed');
264270
});
265271

packages/services/service-ai/src/__tests__/objectql-conversation-service.test.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,12 @@ describe('ObjectQLConversationService', () => {
265265

266266
const updated = await service.addMessage(conv.id, msg);
267267
expect(updated.messages).toHaveLength(1);
268-
const toolContent = updated.messages[0].content as any[];
269-
expect(toolContent[0].toolCallId).toBe('call_abc');
268+
const firstMsg = updated.messages[0];
269+
if (firstMsg.role === 'tool' && Array.isArray(firstMsg.content)) {
270+
expect(firstMsg.content[0].toolCallId).toBe('call_abc');
271+
} else {
272+
throw new Error('Expected tool message with array content');
273+
}
270274
});
271275

272276
it('should add an assistant message with toolCalls', async () => {
@@ -280,10 +284,14 @@ describe('ObjectQLConversationService', () => {
280284

281285
const updated = await service.addMessage(conv.id, msg);
282286
expect(updated.messages).toHaveLength(1);
283-
const assistantContent = updated.messages[0].content as any[];
284-
const toolCallParts = assistantContent.filter((p: any) => p.type === 'tool-call');
285-
expect(toolCallParts).toHaveLength(1);
286-
expect(toolCallParts[0].toolName).toBe('get_weather');
287+
const firstMsg = updated.messages[0];
288+
if (firstMsg.role === 'assistant' && Array.isArray(firstMsg.content)) {
289+
const toolCallParts = firstMsg.content.filter((p) => p.type === 'tool-call');
290+
expect(toolCallParts).toHaveLength(1);
291+
expect(toolCallParts[0].toolName).toBe('get_weather');
292+
} else {
293+
throw new Error('Expected assistant message with array content');
294+
}
287295
});
288296

289297
it('should throw when adding message to non-existent conversation', async () => {

packages/services/service-ai/src/ai-service.ts

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,25 @@ import { ToolRegistry } from './tools/tool-registry.js';
1919
import type { ToolExecutionResult } from './tools/tool-registry.js';
2020
import { InMemoryConversationService } from './conversation/in-memory-conversation-service.js';
2121

22+
// ── Stream event helpers ──────────────────────────────────────────
23+
// These helpers construct properly-typed Vercel AI SDK stream parts
24+
// to avoid repeated `as unknown as TextStreamPart<ToolSet>` casts.
25+
26+
/** Create a text-delta stream part. */
27+
function textDeltaPart(id: string, text: string): TextStreamPart<ToolSet> {
28+
return { type: 'text-delta', id, text } as TextStreamPart<ToolSet>;
29+
}
30+
31+
/** Create a finish stream part from an AIResult. */
32+
function finishPart(result?: AIResult): TextStreamPart<ToolSet> {
33+
return {
34+
type: 'finish',
35+
finishReason: 'stop',
36+
totalUsage: result?.usage ?? { promptTokens: 0, completionTokens: 0, totalTokens: 0 },
37+
rawFinishReason: 'stop',
38+
} as unknown as TextStreamPart<ToolSet>;
39+
}
40+
2241
/**
2342
* Configuration for AIService.
2443
*/
@@ -92,13 +111,8 @@ export class AIService implements IAIService {
92111
if (!this.adapter.streamChat) {
93112
// Fallback: emit the entire response as a single text-delta + finish
94113
const result = await this.adapter.chat(messages, options);
95-
yield { type: 'text-delta', id: 'fallback', text: result.content } as TextStreamPart<ToolSet>;
96-
yield {
97-
type: 'finish',
98-
finishReason: 'stop' as const,
99-
totalUsage: result.usage ?? { promptTokens: 0, completionTokens: 0, totalTokens: 0 },
100-
rawFinishReason: 'stop',
101-
} as unknown as TextStreamPart<ToolSet>;
114+
yield textDeltaPart('fallback', result.content);
115+
yield finishPart(result);
102116
return;
103117
}
104118

@@ -287,13 +301,8 @@ export class AIService implements IAIService {
287301

288302
if (!result.toolCalls || result.toolCalls.length === 0) {
289303
// Final round — return the probed result without an extra model call
290-
yield { type: 'text-delta', id: 'stream', text: result.content } as TextStreamPart<ToolSet>;
291-
yield {
292-
type: 'finish',
293-
finishReason: 'stop' as const,
294-
totalUsage: result.usage ?? { promptTokens: 0, completionTokens: 0, totalTokens: 0 },
295-
rawFinishReason: 'stop',
296-
} as unknown as TextStreamPart<ToolSet>;
304+
yield textDeltaPart('stream', result.content);
305+
yield finishPart(result);
297306
return;
298307
}
299308

@@ -342,12 +351,7 @@ export class AIService implements IAIService {
342351
}
343352
const finalOptions = { ...chatOptions, tools: undefined, toolChoice: undefined };
344353
const result = await this.adapter.chat(conversation, finalOptions);
345-
yield { type: 'text-delta', id: 'stream', text: result.content } as TextStreamPart<ToolSet>;
346-
yield {
347-
type: 'finish',
348-
finishReason: 'stop' as const,
349-
totalUsage: result.usage ?? { promptTokens: 0, completionTokens: 0, totalTokens: 0 },
350-
rawFinishReason: 'stop',
351-
} as unknown as TextStreamPart<ToolSet>;
354+
yield textDeltaPart('stream', result.content);
355+
yield finishPart(result);
352356
}
353357
}

packages/services/service-ai/src/conversation/objectql-conversation-service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ export class ObjectQLConversationService implements IAIConversationService {
183183
contentStr = message.content;
184184
} else {
185185
const parts = message.content;
186-
const textParts = parts.filter(p => p.type === 'text').map(p => 'text' in p ? p.text : '');
186+
const textParts = parts.filter((p): p is { type: 'text'; text: string } => p.type === 'text').map(p => p.text);
187187
const toolCalls = parts.filter(p => p.type === 'tool-call');
188188
contentStr = textParts.join('');
189189
if (toolCalls.length > 0) toolCallsJson = JSON.stringify(toolCalls);
@@ -280,7 +280,7 @@ export class ObjectQLConversationService implements IAIConversationService {
280280
}
281281
case 'tool': {
282282
const toolResults = this.safeParse<ToolResultPart[]>(row.content);
283-
if (toolResults && Array.isArray(toolResults) && toolResults.length > 0 && toolResults[0]?.type === 'tool-result') {
283+
if (toolResults && toolResults.length > 0 && toolResults[0]?.type === 'tool-result') {
284284
return { role: 'tool', content: toolResults };
285285
}
286286
// Backward compat: old format was a plain string

0 commit comments

Comments
 (0)