From dbdf0270ba7c0933eb6c95bfc8e593d1ebffb3ae Mon Sep 17 00:00:00 2001 From: Jerel John Velarde Date: Wed, 19 Aug 2026 05:38:05 -0700 Subject: [PATCH 1/3] fix(ai): read every text block from the model, and fail loudly on none MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in one four-line read. `message.content[0].type === 'text' ? message.content[0].text : ''` only ever looked at the FIRST content block. Anthropic returns an array, and any non-text leading block — a tool_use, a thinking block — made the whole response read as empty even though the text was sitting in blocks 1 and 2. Multi-block text answers were silently truncated to the first block. It also indexed `content[0]` without checking the array had an element, so an empty `content` threw a TypeError that the surrounding catch swallowed into the generic fallback. The crash was invisible; only the apology reached the reporter. `extractResponseText` joins every text block in response order and returns '' for an empty array. An empty result is now an explicit throw rather than an empty string handed downstream, which routes it through the existing catch and produces the intended `degraded: true` fallback instead of publishing a blank answer that would still have marked the ticket answered. Split out of #187 on purpose: these are the only non-worker files in that branch, and three response-quality issues live in generator.ts (#149, #146, #178). Landing them separately unblocks that work now rather than after worker correctness. Advances #178 Verified: packages/outpost — 61 files, 1013 tests pass. --- packages/outpost/ai/src/generator.test.ts | 25 +++++++++++++++++++++++ packages/outpost/ai/src/generator.ts | 11 ++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/outpost/ai/src/generator.test.ts b/packages/outpost/ai/src/generator.test.ts index feb43a96..659f319c 100644 --- a/packages/outpost/ai/src/generator.test.ts +++ b/packages/outpost/ai/src/generator.test.ts @@ -5,6 +5,7 @@ import { SYSTEM_PROMPT_PREFIX, ResponseGenerator, buildChannelGuidance, + extractResponseText, } from './generator.js'; import { ConfidenceLevel } from './types.js'; import type { SearchResult } from './types.js'; @@ -61,6 +62,30 @@ describe('ResponseGenerator', () => { }); describe('generate', () => { + it('should extract text across response blocks when the first block is non-text', () => { + const content = [ + { type: 'tool_use', id: 'tool-1', name: 'lookup', input: {} }, + { type: 'text', text: 'First part' }, + { type: 'text', text: ' and second part' }, + ] as unknown as Parameters[0]; + + expect(extractResponseText(content)).toBe('First part and second part'); + expect(extractResponseText([])).toBe(''); + }); + + it('should return the safe fallback when the model produces no usable text', async () => { + mock.onMessage(/./, { + content: '', + usage: { input_tokens: 100, output_tokens: 0 }, + }); + + const result = await generator.generate({ question: 'test' }, sampleSources); + + expect(result.text).toContain('unable to generate'); + expect(result.confidenceScore).toBe(0); + expect(result.degraded).toBe(true); + }); + it('should generate a response with confidence scoring', async () => { mock.onMessage(/./, { content: 'Here is how to use CopilotKit actions...', diff --git a/packages/outpost/ai/src/generator.ts b/packages/outpost/ai/src/generator.ts index c38263b3..0bcde3c4 100644 --- a/packages/outpost/ai/src/generator.ts +++ b/packages/outpost/ai/src/generator.ts @@ -80,6 +80,11 @@ export function buildChannelGuidance(source?: PlatformTarget): string { ].join('\n'); } +/** Extract all text blocks from an Anthropic response in response order. */ +export function extractResponseText(content: Anthropic.ContentBlock[]): string { + return content.map((block) => (block.type === 'text' ? block.text : '')).join(''); +} + /** * Claude response generator for the AI support pipeline. * @@ -119,8 +124,10 @@ export class ResponseGenerator { messages, }); - const responseText = - message.content[0].type === 'text' ? message.content[0].text : ''; + const responseText = extractResponseText(message.content); + if (!responseText.trim()) { + throw new Error('Model response contained no usable text'); + } const tokenUsage: TokenUsage = { inputTokens: message.usage.input_tokens, From 9c4c57e3b5535f04ac4bbc58cad295f231c23d5b Mon Sep 17 00:00:00 2001 From: Jerel John Velarde Date: Thu, 20 Aug 2026 08:35:42 -0700 Subject: [PATCH 2/3] test(ai): pin the call site and the whitespace half of the empty-response guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both blockers were coverage, not logic. Reverting the call site in `generate()` to the pre-PR first-block-only expression left 254/254 passing, because the new test only exercised the exported helper — so a bad merge or a later refactor could restore the exact defect this PR fixes with CI green. #187 touches this file, which is where that would come from. Relaxing `!responseText.trim()` to `!responseText` also left the suite green, and whitespace-only is the mode the API can realistically produce for this call shape, so the untested half was the reachable one. Drives a `[thinking, text]` response through `generate()` via aimock's `reasoning` option, asserting the answer survives and `degraded` is false; and a `' \n '` fixture asserting the fallback. Both new tests now die under their mutation. The existing empty-response test also asserts the reason, not just the fallback text: aimock builds `content: ''` as `[{type:'text', text:''}]`, but a real `content: []` reaches the same fallback via TypeError, so without that assertion the test could pass for the wrong reason. Also from review: - Join text blocks with a blank line instead of concatenating. Two text blocks are only adjacent because something non-text sat between them, so they were separate emissions — gluing them yields `...first step.Next you...`. Filtering explicitly rather than mapping non-text to `''` makes the separator apply where it should and nowhere else. - Type the fixture as `Anthropic.ContentBlock[]` instead of casting through `unknown`. The cast was hiding real drift: `ToolUseBlock` now requires `caller`, and the typecheck said so the moment it was removed. 23 tests in the file, 1015 in the package. Typecheck adds no errors over base (same 10, all unbuilt-`shared` module resolution). --- packages/outpost/ai/src/generator.test.ts | 122 +++++++++++++++------- packages/outpost/ai/src/generator.ts | 41 ++++---- 2 files changed, 105 insertions(+), 58 deletions(-) diff --git a/packages/outpost/ai/src/generator.test.ts b/packages/outpost/ai/src/generator.test.ts index 659f319c..3e8e32da 100644 --- a/packages/outpost/ai/src/generator.test.ts +++ b/packages/outpost/ai/src/generator.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import type Anthropic from '@anthropic-ai/sdk'; import { LLMock } from '@copilotkit/aimock'; import { GROUNDING_RULES, @@ -63,16 +64,63 @@ describe('ResponseGenerator', () => { describe('generate', () => { it('should extract text across response blocks when the first block is non-text', () => { - const content = [ - { type: 'tool_use', id: 'tool-1', name: 'lookup', input: {} }, - { type: 'text', text: 'First part' }, - { type: 'text', text: ' and second part' }, - ] as unknown as Parameters[0]; + // Typed as the real union, no `as unknown` — the point of the fixture is + // that it is a response the SDK could actually hand us, and the cast was + // suppressing the check that proves it. + const content: Anthropic.ContentBlock[] = [ + { + type: 'tool_use', + id: 'tool-1', + name: 'lookup', + input: {}, + caller: { type: 'direct' }, + }, + { type: 'text', text: 'First part', citations: null }, + { type: 'text', text: 'and second part', citations: null }, + ]; - expect(extractResponseText(content)).toBe('First part and second part'); + // Blank line, not concatenation: the blocks are separate emissions. + expect(extractResponseText(content)).toBe('First part\n\nand second part'); expect(extractResponseText([])).toBe(''); }); + // The helper above is well covered on its own, but nothing drove a + // multi-block response through `generate()` — reverting the call site to + // first-block-only left the whole suite green, so a bad merge or a refactor + // could put the original defect back silently. #187 touches this same file. + it('should read past a leading thinking block when generating', async () => { + mock.onMessage(/./, { + content: 'Use useCopilotAction for that.', + reasoning: 'internal thinking that is not the answer', + usage: { input_tokens: 400, output_tokens: 80 }, + }); + + const result = await generator.generate({ question: 'test' }, sampleSources); + + // aimock puts the `thinking` block first, so first-block-only yields '' + // and this lands in the degraded fallback instead. + expect(result.text).toBe('Use useCopilotAction for that.'); + expect(result.degraded).toBe(false); + expect(result.reasoning).not.toContain('no usable text'); + }); + + // The `.trim()` half of the guard. Whitespace-only is the mode the API can + // realistically produce for this call shape, and it was the untested half: + // relaxing the check to `if (!responseText)` left the suite green, so a + // `"\n\n"` completion would publish at whatever the retrieval score was. + it('should return the safe fallback for a whitespace-only response', async () => { + mock.onMessage(/./, { + content: ' \n ', + usage: { input_tokens: 100, output_tokens: 2 }, + }); + + const result = await generator.generate({ question: 'test' }, sampleSources); + + expect(result.text).toContain('unable to generate'); + expect(result.degraded).toBe(true); + expect(result.reasoning).toContain('no usable text'); + }); + it('should return the safe fallback when the model produces no usable text', async () => { mock.onMessage(/./, { content: '', @@ -84,6 +132,10 @@ describe('ResponseGenerator', () => { expect(result.text).toContain('unable to generate'); expect(result.confidenceScore).toBe(0); expect(result.degraded).toBe(true); + // Asserted on the reason, not just the fallback: aimock builds + // `content: ''` as `[{type:'text', text:''}]`, but a real `content: []` + // would reach the same fallback via TypeError. This pins which one. + expect(result.reasoning).toContain('no usable text'); }); it('should generate a response with confidence scoring', async () => { @@ -113,14 +165,11 @@ describe('ResponseGenerator', () => { const highQualitySources: SearchResult[] = [ { title: 'A', content: 'Content A', score: 0.95 }, - { title: 'B', content: 'Content B', score: 0.90 }, + { title: 'B', content: 'Content B', score: 0.9 }, { title: 'C', content: 'Content C', score: 0.88 }, ]; - const result = await generator.generate( - { question: 'test' }, - highQualitySources, - ); + const result = await generator.generate({ question: 'test' }, highQualitySources); expect(result.confidenceLevel).toBe(ConfidenceLevel.HIGH); }); @@ -131,10 +180,7 @@ describe('ResponseGenerator', () => { usage: { input_tokens: 100, output_tokens: 50 }, }); - const result = await generator.generate( - { question: 'test' }, - [], - ); + const result = await generator.generate({ question: 'test' }, []); expect(result.confidenceLevel).toBe(ConfidenceLevel.LOW); }); @@ -142,10 +188,7 @@ describe('ResponseGenerator', () => { it('should return graceful fallback on API error', async () => { mock.nextRequestError(429, { message: 'API rate limited' }); - const result = await generator.generate( - { question: 'test' }, - sampleSources, - ); + const result = await generator.generate({ question: 'test' }, sampleSources); expect(result.text).toContain('unable to generate'); expect(result.confidenceScore).toBe(0); @@ -158,14 +201,10 @@ describe('ResponseGenerator', () => { usage: { input_tokens: 200, output_tokens: 50 }, }); - await generator.generate( - { question: 'What about streaming?' }, - sampleSources, - [ - { role: 'user', content: 'How do I use actions?' }, - { role: 'assistant', content: 'You use useCopilotAction...' }, - ], - ); + await generator.generate({ question: 'What about streaming?' }, sampleSources, [ + { role: 'user', content: 'How do I use actions?' }, + { role: 'assistant', content: 'You use useCopilotAction...' }, + ]); // Verify the request contained conversation history const lastReq = mock.getLastRequest(); @@ -175,14 +214,22 @@ describe('ResponseGenerator', () => { const messages = body!.messages; // Should contain the history messages const allMessages = messages as Array<{ role: string; content: string | null }>; - const userMessages = allMessages.filter(m => m.role === 'user'); - const assistantMessages = allMessages.filter(m => m.role === 'assistant'); - expect(userMessages.some(m => - typeof m.content === 'string' && m.content.includes('How do I use actions?') - )).toBe(true); - expect(assistantMessages.some(m => - typeof m.content === 'string' && m.content.includes('You use useCopilotAction...') - )).toBe(true); + const userMessages = allMessages.filter((m) => m.role === 'user'); + const assistantMessages = allMessages.filter((m) => m.role === 'assistant'); + expect( + userMessages.some( + (m) => + typeof m.content === 'string' && + m.content.includes('How do I use actions?'), + ), + ).toBe(true); + expect( + assistantMessages.some( + (m) => + typeof m.content === 'string' && + m.content.includes('You use useCopilotAction...'), + ), + ).toBe(true); }); }); @@ -288,10 +335,7 @@ describe('ResponseGenerator', () => { mock.nextRequestError(500, { message: 'Stream interrupted' }); const chunks: string[] = []; - for await (const chunk of generator.generateStream( - { question: 'test' }, - [], - )) { + for await (const chunk of generator.generateStream({ question: 'test' }, [])) { chunks.push(chunk); } diff --git a/packages/outpost/ai/src/generator.ts b/packages/outpost/ai/src/generator.ts index 0bcde3c4..5ab7fd2d 100644 --- a/packages/outpost/ai/src/generator.ts +++ b/packages/outpost/ai/src/generator.ts @@ -51,14 +51,10 @@ ${GROUNDING_RULES}`; const CHANNEL_GUIDANCE: Record = { discord: 'This question was asked in the CopilotKit Discord. The user is ALREADY in Discord — never suggest they "join the Discord", never share a Discord invite link, and never tell them to ask in Discord. You may point them to the docs or GitHub if genuinely useful.', - github: - 'This question was asked in a GitHub issue or discussion. The user is ALREADY on GitHub — never suggest they "open an issue", "file a bug report", or "open a GitHub discussion"; they already have. You may point them to the docs or Discord if genuinely useful.', - slack: - 'This question was asked in Slack. The user is ALREADY in Slack — never suggest they reach out or ask again in Slack. You may point them to the docs, Discord, or GitHub if genuinely useful.', - teams: - 'This question was asked in Microsoft Teams. The user is ALREADY in Teams — never suggest they reach out or ask again in Teams. You may point them to the docs, Discord, or GitHub if genuinely useful.', - web: - 'This question was asked through the web support widget. Point the user to the docs, Discord, or GitHub if genuinely useful.', + github: 'This question was asked in a GitHub issue or discussion. The user is ALREADY on GitHub — never suggest they "open an issue", "file a bug report", or "open a GitHub discussion"; they already have. You may point them to the docs or Discord if genuinely useful.', + slack: 'This question was asked in Slack. The user is ALREADY in Slack — never suggest they reach out or ask again in Slack. You may point them to the docs, Discord, or GitHub if genuinely useful.', + teams: 'This question was asked in Microsoft Teams. The user is ALREADY in Teams — never suggest they reach out or ask again in Teams. You may point them to the docs, Discord, or GitHub if genuinely useful.', + web: 'This question was asked through the web support widget. Point the user to the docs, Discord, or GitHub if genuinely useful.', }; /** @@ -80,9 +76,21 @@ export function buildChannelGuidance(source?: PlatformTarget): string { ].join('\n'); } -/** Extract all text blocks from an Anthropic response in response order. */ +/** + * Extract all text blocks from an Anthropic response, in response order. + * + * Joined with a blank line rather than concatenated. Two text blocks are only + * ever adjacent because something non-text sat between them (a `tool_use`, a + * `thinking` block), which means they were separate emissions and not two halves + * of one sentence — concatenating them produces `...first step.Next you...`. + * Filtering explicitly rather than mapping non-text blocks to `''` is what makes + * the separator apply where it should and nowhere else. + */ export function extractResponseText(content: Anthropic.ContentBlock[]): string { - return content.map((block) => (block.type === 'text' ? block.text : '')).join(''); + return content + .filter((block): block is Anthropic.TextBlock => block.type === 'text') + .map((block) => block.text) + .join('\n\n'); } /** @@ -139,10 +147,7 @@ export class ResponseGenerator { // Assessed here (the response and its sources are both in hand) and // applied by the pipeline — exactly once. const groundedness = assessGroundedness(responseText, sources); - const confidenceLevel = this.classifyGroundedConfidence( - confidenceScore, - groundedness, - ); + const confidenceLevel = this.classifyGroundedConfidence(confidenceScore, groundedness); return { text: responseText, @@ -195,10 +200,7 @@ export class ResponseGenerator { }); for await (const event of stream) { - if ( - event.type === 'content_block_delta' && - event.delta.type === 'text_delta' - ) { + if (event.type === 'content_block_delta' && event.delta.type === 'text_delta') { yield event.delta.text; } } @@ -220,7 +222,8 @@ export class ResponseGenerator { buildChannelGuidance(source), '', '--- Documentation Context ---', - sourceContext || '(No relevant documentation found — answer from general CopilotKit knowledge if possible, otherwise say you need to escalate)', + sourceContext || + '(No relevant documentation found — answer from general CopilotKit knowledge if possible, otherwise say you need to escalate)', ].join('\n'); } From 626cf9f3321d0dc39bc4910cb1b29ef5a16bc1fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:37:16 -0400 Subject: [PATCH 3/3] Update packages/outpost/ai/src/generator.test.ts --- packages/outpost/ai/src/generator.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/outpost/ai/src/generator.test.ts b/packages/outpost/ai/src/generator.test.ts index 3e8e32da..84d3fc8f 100644 --- a/packages/outpost/ai/src/generator.test.ts +++ b/packages/outpost/ai/src/generator.test.ts @@ -132,9 +132,11 @@ describe('ResponseGenerator', () => { expect(result.text).toContain('unable to generate'); expect(result.confidenceScore).toBe(0); expect(result.degraded).toBe(true); - // Asserted on the reason, not just the fallback: aimock builds - // `content: ''` as `[{type:'text', text:''}]`, but a real `content: []` - // would reach the same fallback via TypeError. This pins which one. + // Asserted on the reason, not just the fallback: an API error, a + // TypeError and this guard all land on the identical fallback text, + // so `text` alone can't tell them apart. This pins that the guard + // fired. It does not distinguish `content: ''` from a real + // `content: []` — both join to `''` and throw the same error. expect(result.reasoning).toContain('no usable text'); });