fix(ai): read every text block from the model, and fail loudly on none

[jerelvelarde]

Advances #178. First of the two PRs splitting the remainder of #187 — see "Relationship to #187" below for why this is separate.

Two defects in one four-line read:

const responseText = message.content[0].type === 'text' ? message.content[0].text : '';

1. Only the first content block was ever read. Anthropic returns content as an array. Any non-text leading block — a tool_use, a thinking block — made the entire 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.

2. content[0] was indexed without checking the array had an element. An empty content threw a TypeError, which the surrounding catch swallowed into the generic fallback. The crash was invisible in the logs' shape — you'd see the apology fallback and a Generation failed: reason, with nothing indicating the real cause was our own indexing rather than the model or the API.

The fix

extractResponseText joins every text block in response order and returns '' for an empty array. An empty result is then an explicit throw rather than an empty string handed downstream.

That throw is the deliberate part. It routes through the existing catch and produces the apology fallback — instead of publishing a blank answer that, under the one-response-per-ticket rule, would still have marked the ticket answered and spent the reporter's single response on nothing. Failing loudly here is what makes the ticket recoverable.

One correction to an earlier version of this description, per review: the mechanism doing the work on that path is confidenceScore: 0, not the degraded flag. The generator's degraded is dead — pipeline.ts:218 reads only confidenceAssessment.degraded. That's pre-existing (the base returned it too) and out of scope here, but the flag is not what makes this safe.

Relationship to #187

generator.ts and generator.test.ts are the only non-worker files in #187's remaining nine, and CPK-7925 flagged the question of whether they belong with worker correctness at all. They don't: three response-quality issues live in generator.ts — #149 (temperature sent unconditionally), #146 (streaming path skips the groundedness gate), #178 (empty-response behaviour) — and holding them behind a worker-correctness review round buys nothing.

Landing these two files on their own unblocks that work now. Worker correctness (worker.ts, create-job.ts, health.ts, railway.toml, and the job-claim-token migration) follows as its own PR.

Cherry-picked from fix/ai-response-delivery-durability unmodified, so the diff is byte-identical to what #187 already carried — nothing here is newly written or re-derived.

Verification

packages/outpost: 61 test files, 1015 tests passing. Four tests cover the two halves and their wiring:

• text extracted across blocks when the first is non-text (the pure helper)
• a [thinking, text] response driven through generate(), so the call site itself is pinned — reverting it to first-block-only while leaving the helper intact previously left the whole suite green
• the safe fallback when the model produces no usable text, asserted on the reason (no usable text) rather than only the fallback copy
• the same fallback for a whitespace-only response, which pins the .trim() half of the guard — relaxing it to !responseText previously left the suite green

Note for anyone running this locally: a fresh worktree needs pnpm install and npx prisma generate --schema=db/prisma/schema.prisma before the suite is green — without the generate step queue/src/__tests__/scheduler.test.ts fails to load on .prisma/client/default and reads like a real failure. pnpm's ignored-build-scripts warning is the only hint.

[linear-code]

CPK-7925 3. Open + merge the third split PR — worker correctness (unblocks response quality)

The third of the three PRs split out of #187. Not yet opened — #191's description names it as "third (worker correctness) to follow."

Scope — derived from the files #187 carries that neither split does

apps/worker/railway.toml
apps/worker/src/health.ts
apps/worker/src/__tests__/health.test.ts
packages/outpost/queue/src/worker.ts
packages/outpost/queue/src/create-job.ts
packages/outpost/queue/src/__tests__/worker-concurrency.test.ts
packages/outpost/db/prisma/migrations/20260811220000_add_job_claim_token/migration.sql
packages/outpost/ai/src/generator.ts
packages/outpost/ai/src/generator.test.ts

Roadmap issues it should close or advance

• #181 — shutdown() does not drain in-flight jobs
• #182 — /health reports ok for a stopped or wedged worker
• #184 — worker PORT/HEALTH_PORT precedence inverted (railway.toml + health.ts)
• #153 — per-type claiming serializes job types (worker.ts, create-job.ts, the job-claim-token migration)

Worth confirming that list against the actual diff when the PR is opened rather than assuming it — the file overlap is strong evidence of scope, not a promise of what the code does.

Why this is the response-quality gate

packages/outpost/ai/src/generator.ts is in this set and in no other open PR. Three response-quality issues live in that file:

• #149 — temperature sent unconditionally (generator.ts:117 and :185)
• #146 — the streaming path skips the groundedness gate
• #178 — the generator's empty-response behaviour

Until this lands, work on any of the three either conflicts with this branch or gets rewritten by it.

One thing to decide when opening it: whether the generator.ts changes belong here at all. They are the only non-worker files in the set. If they can be dropped or landed separately, three response-quality issues unblock immediately instead of waiting on worker correctness.

CPK-7981 Review + land PR #223 — generator text blocks / fail loudly (4 findings open)

Reopened 2026-08-20, Nathan owns it. The review is complete; the findings are not addressed and the PR is not merged. Retitled to match.

Must-fix before merge: preserve tokenUsage on the empty-text path, give the failure a signal a consumer actually reads (nothing reads the generator's degraded), and check stop_reason so a truncated response does not pass the blank guard. One product decision is yours, not Jerel's: on empty model text, does the reporter get the public apology or a human escalation? My read is escalation — a model returning zero text is our pipeline failing, not a question that failed.

CopilotKit/outpost#223 by jerelvelarde, opened 2026-08-19 12:38Z off main. +34/-2 across 2 files. CI green. MERGEABLE / BLOCKED on review. Branch jerel/cpk-7925-generator-empty-response.

25+ 0-  packages/outpost/ai/src/generator.test.ts
 9+ 2-  packages/outpost/ai/src/generator.ts

Smallest of the three. Answers outpost#178 — "decide the generator's empty-response behaviour" — and the title says the decision was fail loudly, which is the right direction: the alternative is an empty or apology reply posted publicly as though it were an answer.

Note on how this arrived

I had scoped CPK-7925 as one worker-correctness PR and flagged that generator.ts was the only non-worker file in it — suggesting that if it could be split out, three response-quality issues would unblock immediately instead of waiting on worker work. Jerel did exactly that split: this PR is the generator half, #224 is the worker half. Worth knowing when reviewing, because it means CPK-7930's other two items (#149 temperature, #146 streaming gate) are now unblocked much earlier than the ticket predicted.

Where to look

• "Read every text block" implies the old code took only the first. Confirm the concatenation order is deterministic and that a multi-block response now reads correctly rather than doubling content.
• "Fail loudly on none" — check what the failure does downstream. It must not become the public apology fallback (that is exactly #149's failure mode), and it must not mark the job COMPLETED with nothing delivered (the class #175 covers).
• 25 test lines for 9 source lines is a good ratio; confirm the none-case test asserts the loud behaviour and not merely that it does not crash.

cr-loop complete 2026-08-19 — 4 mandatory findings, do not merge as-is

Tier 1 (36 LOC, single module, no shared mechanism). Panel of 3. Round 1 complete, 3/3. Convergence not declared; no fix cycle run. Review comment posted.

The concatenation half is correct — extractResponseText joins in response order, and the tool_use-first test covers the case that was silently truncating answers. The throw half has four problems sharing one theme: "fail loudly" currently fails loudly at the reporter and silently at us.

1. The throw discards real spend. :127 throws into the existing catch, which hardcodes tokenUsage: { inputTokens: 0, outputTokens: 0 }. The request already happened. The PR's own test supplies input_tokens: 100 and asserts nothing about tokenUsage, so it vanishes — under-reporting the cost metric outpost#217 exists to make measurable.
2. Nothing reads degraded. Verified by grepping every reader in the repo: only pipeline.ts:218 (confidenceAssessment.degraded) and account-scoring.ts:163 (sentiment.degraded). Neither is the generator's. All 3 agents flagged it. So the signal is inert and empty-text is indistinguishable downstream from a 500 or a bad API key.
3. The reporter gets the public apology. The catch posts "I apologize, but I was unable to generate a response at this time" at confidence 0 — the same fallback #149 is filed about — for what is really an internal defect. My read: this case should escalate to a human, not post the apology.
4. The guard misses the likelier failure. stop_reason is never checked (:119), so a max_tokens-truncated or refused response has text, passes the blank check, and publishes as complete. Empty responses are rare; truncation is not.

Pre-existing and flagged for the record, not asked of this PR: :115-116 buildSystemPrompt/buildMessages sit outside the try so a score-less source throws past the "never crash" fallback (same at :185-186) · :205 generateStream yields the raw provider error as content, which pipeline would publish (no production caller; overlaps #146, streaming side filed as #228) · :285-288 clamp checks suppress only while pipeline.ts:206 clamps on suppress || forcesEscalation · tests at :143/:288 inject 429/500 for one request while the SDK retries twice with no maxRetries: 0.

Ledger + full reports: ~/.local/share/copilotkit/cr/pr223-generator/.

Review in Linear

[NathanTarbert]

Hey @jerelvelarde,

Replacing my earlier comment here too, same reason as the other two: re-ran with mutation testing enforced, and it both found blockers I'd missed and retracted two things I'd claimed.

Verdict: NEEDS CHANGES. Two blockers, both coverage rather than logic. The direction of this PR is right and the base behavior was worse, so this is close.

---

Approved

extractResponseText is correct, and it's well covered. Mutating it to first-block-only fails a test, reversing block order fails, join('') to join(' ') fails, returning the last block only fails. Four independent mutations all die, which is what good coverage looks like.

Splitting the generator work out of #187 was the right call. It unblocked #149 and #146 earlier than my ticket predicted.

And the base behavior this replaces was genuinely worse: a blank answer published at full retrieval confidence.

---

Blockers

B1. The helper's wiring into generate() is untested

generator.ts:127. Reverting just the call site to the pre-PR expression (message.content[0]?.type === 'text' ? … : '') while leaving the exported helper untouched leaves 254/254 passing. The new test at generator.test.ts:65 only calls the pure function. Nothing drives a multi-block response through generate().

Why it matters concretely: #187 touches this same file. A bad merge or a later refactor puts the call site back to first-block-only, CI stays green, and the exact defect this PR exists to fix comes back silently.

Fix: aimock already emits a leading non-text block. mock.onMessage(/./, { content: 'the answer', reasoning: 'internal thinking' }) produces [{type:'thinking'},{type:'text'}] (verified in @copilotkit/aimock@1.14.0, dist/messages.js:250-259). Assert result.text === 'the answer' and degraded === false through generate().

B2. The .trim() half of the guard is untested

generator.ts:128. Changing if (!responseText.trim()) to if (!responseText) leaves 254/254 passing. Whitespace-only output is the only trigger of this guard the API can realistically produce for this call shape, and it's the untested half. Someone "simplifies" the truthiness check, a "\n\n" completion sails through and publishes at whatever the retrieval score was, possibly HIGH, no disclaimer, no escalation.

Fix: a fixture with content: ' \n ' asserting degraded === true.

---

Non-blocking

• :85 — join('') glues adjacent text blocks with no separator. On text | tool_use | text the two prose blocks are separate emissions, so you can get …first step.Next you…. Non-text blocks correctly contribute '', which means the map/join is only doing separator work by accident. content.filter(b => b.type === 'text').map(b => b.text).join('\n\n') makes the intent explicit. Flagging as a deliberate-choice question, not uncovered code, since the mutations here do die.
• :128 — the guard targets a near-unreachable mode and misses the live ones. The messages.create call at :119-125 passes no tools and no thinking, so the API returns exactly one text block. The commit message's premise, that a tool_use or thinking block made the whole response read as empty, isn't reachable through this path today. Reasonable future-proofing, but the live unusable-response modes are present-but-unusable and none are guarded: stop_reason === 'max_tokens' publishes a mid-sentence truncated answer as complete at full retrieval confidence, and refusals arrive as ordinary text. If "fail loudly on an unusable response" is the goal, stop_reason !== 'end_turn' is the higher-yield check.
• :158-173 — the throw discards real spend. It routes into the catch, which reports tokenUsage: { inputTokens: 0, outputTokens: 0 }, but the input tokens were billed. Your own new fixture bills 100. Mitigating fact I checked: grep -rn "tokenUsage" packages/outpost/queue/src returns nothing, so nothing persists this today. Observability loss, not a billing bug. Either capture message.usage before the guard and pass it through, or accept the spend is unattributed and say so.
• :163 plus pipeline.ts:~247 — the reporter gets promised a human twice. Output on the throw path is "I apologize, but I was unable to generate a response at this time. A human support agent will follow up shortly" and AI_DISCLAIMER_ESCALATED ("We've escalated this to our engineering team, someone will follow up in this thread shortly"). The suppress path has an explicit comment avoiding exactly this doubling, the generator-fallback path doesn't, and this PR routes more tickets into it.
• :129 — throwing inside the swallowing try forfeits the retry ladder. The throw is caught 30 lines down, so the job returns success. There's a real ladder (MAX_JOB_ATTEMPTS, worker.ts:325-356) and the re-answer guard at handlers/ai-response.ts:105-137 makes a pre-post retry safe, since no BOT message exists yet. An empty completion is the archetypal transient failure, and right now it permanently consumes the ticket's one AI response with an apology. A distinguishable error (EmptyModelResponseError) escaping past the catch, or one in-place retry, would give the ladder a shot. Nothing is lost content-wise on this path since the text is empty by definition, so the only casualty is the retry.
• generator.test.ts:66-70 — hand-written cast literals. as unknown as Parameters<typeof extractResponseText>[0] defeats the type check that would verify the fixture is a real Anthropic.ContentBlock, and ToolUseBlock's shape has moved across SDK majors. Per our aimock convention this wants a deterministic fixture driven through the real client. Keep a pure-function test if you like, but type it as Anthropic.ContentBlock[] without the as unknown.

---

Retracting two things I said earlier

1. I said the empty-text path means the reporter sees an apology while "we get a console.error, no alert, no metric." Not accurate. handlers/ai-response.ts:398-414 does enqueue the ESCALATION job, so the promise of a human is honored. The real defect is the opposite of silence: it's stated twice, per the item above.
2. I tied the discarded tokenUsage to No instrumentation for any of the Build Plan target metrics (10-min response, 80% helpful, $500/mo) #217 as under-reported cost. Nothing in the queue persists tokenUsage today, so it's observability only. Downgraded.

And one clarification on degraded: it is dead, confirmed, pipeline.ts:218 reads only confidenceAssessment.degraded and nothing reads the generator's. But the base already returned it, so it's pre-existing rather than something this PR introduced, and it's out of scope to fix here. Worth knowing the good outcome on this path comes from confidenceScore: 0, not from the flag, so the PR body's justification names the wrong mechanism.

---

Still open from my earlier read, not re-verified this round

Carrying these forward as unconfirmed rather than dropping them:

• :115-116 — buildSystemPrompt / buildMessages sit outside the try, so s.score.toFixed(2) on a score-less source would throw past the "never crash" fallback. Same shape at :185-186 in generateStream.
• :205 — generateStream yielding a raw provider error as ordinary content, which pipeline would then buffer and publish, turning a 500 into a public post. No production caller today, so a trap rather than a live bug. Overlaps Groundedness gate: streaming path, generator level, and QA metadata still read the pre-#143 signal #146; I filed the streaming side as QA chat SSE: no cross-chunk buffer silently drops the metadata frame (sources + confidence) #228.
• :285-288 — the clamp checks suppress only while pipeline.ts:206 clamps on suppress || forcesEscalation, so confidenceLevel could read MEDIUM where the pipeline treats it as LOW.
• Tests at :143 / :288 injecting 429/500 for one request while the SDK retries twice with no maxRetries: 0, so they'd pass on mock-miss behavior rather than on the assertion.

The fresh pass did confirm one adjacent thing: generateStream has the identical empty-response defect unfixed at :197-207, with no fallback and no degraded signal, and no production caller beyond pipeline.generateStreamingResponse at :349.

---

Evidence

vitest 4.1.4 and @copilotkit/aimock 1.14.0 resolved from the lockfile, pnpm 10.33.4, darwin arm64. Base 724a53b8 at 252 passing, head dbdf0270 at 254, so the delta is exactly your two new tests with no pre-existing test changing status. Full package: 1011 to 1013.

Environmental note, not the PR: --frozen-lockfile skips build scripts, so queue/src/__tests__/scheduler.test.ts fails on .prisma/client/default until npx prisma generate --schema=db/prisma/schema.prisma runs. All numbers above are post-generate.

Not verified: ESLint never actually ran (v9 printed the flat-config migration notice and exited 0 without linting), so lint status of the diff is unknown. And the claim that a leading non-text block is unreachable today is read off the call shape at :119-125, not observed against production traffic.

One subtlety worth pinning: aimock builds content: '' as [{type:'text', text:''}], which is why your new test fails at base. A real content: [] would also fail at base, but via TypeError into the same fallback, so the test would pass on base for the wrong reason if aimock ever changes that shape. Asserting on result.reasoning containing no usable text would pin it properly.

---

Both blockers are test additions, not logic changes. Happy to take them as a patch if you'd rather stay on #187.

[jerelvelarde]

Both blockers fixed in 9c4c57e. You were right that they were coverage rather than logic, and right that that's the more dangerous kind here.

B1 — call site untested

Confirmed exactly as you described: reverting generate() to message.content[0]?.type === 'text' ? … : '' while leaving the helper alone left the suite fully green. The helper had four dying mutations and the wiring had none, which is the worst possible split given #187 touches this same file — a merge resolution picks the old expression, CI says fine, and the defect is back with no trace.

Used your aimock recipe. Verified the shape myself before relying on it — dist/messages.js:250-259, reasoning pushes a {type:'thinking'} block ahead of the text block, so first-block-only yields '' and lands in the fallback. Test asserts text, degraded === false, and that the reasoning does not contain no usable text. Reverting the call site now fails it.

B2 — the .trim() half

Confirmed: if (!responseText) left 254/254 green. Added the ' \n ' fixture asserting degraded === true. Dropping .trim() now fails it.

Your aimock subtlety — taken

Good catch, and it applied to the test I already had, not just the new ones. The existing empty-response test now asserts result.reasoning contains no usable text, so it's pinned to the guard rather than to "something threw somewhere". Without it, a future aimock that builds content: '' as a real content: [] would have the test passing via TypeError into the same fallback.

Non-blocking, taken

join(''). Agreed, and I'd rather make the intent explicit than rely on non-text blocks mapping to ''. Now filters to text blocks and joins on \n\n, with the reasoning in the docstring: two text blocks are only adjacent because something non-text sat between them, so they were separate emissions and a blank line is the honest separator. Updated the helper's unit test to expect the paragraph break.

The as unknown cast. Taken, and it earned its keep immediately — the moment I typed the fixture as Anthropic.ContentBlock[], tsc failed with Property 'caller' is missing in type … but required in type 'ToolUseBlock'. Exactly the drift you predicted. Fixture now carries caller: { type: 'direct' }. Typecheck adds nothing over base (same 10 errors, all @copilotkit/outpost/shared unresolved because shared isn't built — matches your environment note).

Not taking here, with reasons

stop_reason !== 'end_turn' instead of the empty check. You're right that this is the higher-yield check and that the commit message's premise (a tool_use block making the response read as empty) isn't reachable through this call shape — no tools, no thinking, so one text block. But it's a behaviour change with a live blast radius: max_tokens currently publishes a truncated answer, and switching it to the apology fallback changes what real reporters see on long answers. That wants its own PR with a decision about which non-end_turn reasons are unusable (max_tokens yes; stop_sequence arguably fine). Filing it, and I think it's the more valuable follow-up of the two.

The double promise of a human (:163 + AI_DISCLAIMER_ESCALATED). Confirmed by reading both paths, and thank you for the retraction on the escalation job — that changes it from "silent" to "said twice", which is a better problem but still a bad reporter experience. It's a pipeline-side fix (the suppress path already has the comment avoiding it), and this PR routes more traffic into it, so it should land soon. Not in a test-only commit though.

The retry ladder at :129. This is the finding I'd most like to act on and least want to rush. You're right that an empty completion is archetypally transient, that the pre-post re-answer guard at handlers/ai-response.ts:105-137 makes a retry safe, and that we currently burn the ticket's one AI response on an apology. But a distinguishable error escaping the catch means the "never crash" contract at :158 no longer holds unconditionally, and every caller assumes it does. Wants its own PR.

tokenUsage discarded on the throw path. Accepting the observability loss for now, given nothing in the queue persists it (confirmed your grep). Noting it in the follow-up rather than capturing message.usage before the guard, since the right fix is probably persisting usage at all.

generateStream's identical unfixed defect at :197-207 — confirmed, and it has no fallback at all. Tracking with #228 rather than widening this PR.

Carried-forward items (buildSystemPrompt/buildMessages outside the try, the raw-provider-error yield, the suppress-only clamp vs pipeline.ts:206, the 429/500 tests passing on SDK retry rather than assertion) — all confirmed as still open, none introduced here, all going in the follow-up.

On degraded being dead: noted, and you're right that the PR body names the wrong mechanism. The good outcome on this path comes from confidenceScore: 0, not the flag. Fixing the body rather than the code, since the base already returned it.

Re-review when you have a moment.

[NathanTarbert]

Hey @jerelvelarde,

Verified 9c4c57e with a fresh reviewer that re-ran both mutations at head and at base. Both blockers are genuinely fixed. Approving.

Mutation	Base dbdf0270	Head 9c4c57e3
B1: revert the call site, leave the helper exported	1013/1013 survived	killed — should read past a leading thinking block when generating
B2: !responseText.trim() → !responseText	1013/1013 survived	killed — should return the safe fallback for a whitespace-only response

Each kills exactly one test, so the new tests are doing the work rather than something incidental catching it. The thinking test calls the real generate() and asserts toBe exact, so a leaked thinking string would fail it, and aimock's source confirms reasoning really does push {type:'thinking'} first. That's the shape under test, which is why the mutation dies.

Your ToolUseBlock catch checks out: @anthropic-ai/sdk@0.89.0, messages.d.ts:1363-1372, caller is required on the block and optional only on the *BlockParam variants. The cast was hiding real drift. Typecheck delta vs base is zero.

Head 1015, base 1013, delta +2. Nothing regressed.

Three notes, none of them gating, all fine as follow-ups:

One correction to the commit message. You wrote that a real content: [] reaches the fallback via TypeError. It doesn't. [].filter().map().join('\n\n') returns '' and throws the same 'Model response contained no usable text', so the assertion can't distinguish '' from []. What it genuinely pins is that the guard fired rather than an API error or a TypeError, all of which land on the identical fallback text. The assertion is worth keeping, the rationale in the commit body and the comment at generator.test.ts:132-134 is worth correcting, otherwise the next reader inherits a wrong model of the code.

generator.ts:89-93 — the '\n\n' join is safe today only because this call sends no tools and no citation-enabled documents. With citations the API returns adjacent text blocks that are continuous prose split at citation boundaries, each carrying its own citations, and this join would inject a paragraph break mid-sentence. That's the mirror image of the ...first step.Next you... bug you just fixed, and your own fixture already writes citations: null on both blocks, so the shape is representable. Worth a comment pinning the assumption to "no tools on this call".

Diff noise. The commit carries prettier reformatting across CHANNEL_GUIDANCE, generateStream and buildSystemPrompt, plus a 0.90 → 0.9 score-fixture edit sitting in the formatting noise. Harmless, but it inflates a commit whose whole point is auditability.

The four non-blocking items are all still open and that's fine, they were never gating: stop_reason still never read, message.usage still discarded on the throw, the double human-follow-up promise still in both files, and the retry ladder still never runs since generate() returns on every path. stop_reason is the one I'd pick up next, since max_tokens truncation is the live failure mode and this guard doesn't cover it.

[NathanTarbert]

Approval stands — I modified a comment in the docs so it's accurate now.

Note the first one is at generator.test.ts:135-137 at head, not 132-134 as I wrote in my last comment.

Once both are committed I'll re-run the B1/B2 mutation matrix at the new head and confirm the test count and typecheck delta are unchanged.