You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
From the review of #223. That PR fixed the empty-text case and pinned it; this is the higher-yield half it deliberately didn't take, plus the adjacent findings.
The main point
generate() calls messages.create with no tools and no thinking, so the API returns exactly one text block. The mode #223 guards — a tool_use or thinking block making the 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.
Refusals arrive as ordinary text and publish normally.
If "fail loudly on an unusable response" is the goal, stop_reason !== 'end_turn' is the check that earns it.
Not a drop-in: it changes what real reporters see on long answers, and it needs a decision about which non-end_turn reasons count as unusable (max_tokens yes; stop_sequence arguably fine). That's why it isn't in #223.
The reporter is promised a human twice
On the generation-failure path the reporter gets "A human support agent will follow up shortly" from the generator fallback andAI_DISCLAIMER_ESCALATED ("We've escalated this to our engineering team, someone will follow up in this thread shortly") from the pipeline. The suppress path has an explicit comment avoiding exactly this doubling; the generator-fallback path doesn't, and #223 routes more tickets into it.
(The escalation job is enqueued, so the promise is honored — the defect is that it's stated twice, not that it's empty.)
An empty completion permanently burns the ticket's one AI response
The throw is caught ~30 lines down, so the job returns success and never sees the retry ladder (MAX_JOB_ATTEMPTS). An empty completion is the archetypal transient failure, and the re-answer guard makes a pre-post retry safe — no BOT message exists yet at that point. Nothing is lost content-wise on this path since the text is empty by definition, so the only casualty is the retry.
A distinguishable error (EmptyModelResponseError) escaping the catch, or one in-place retry, would give the ladder a shot. The catch is the "never crash" contract every caller relies on, so this needs its own change rather than a loosened catch.
generateStream has the identical defect, unfixed
Same empty-response hole, with no fallback and no degraded signal. No production caller beyond pipeline.generateStreamingResponse. Streaming side of this is #228.
Smaller, carried from the same review
The throw discards real spend. The catch reports tokenUsage: { inputTokens: 0, outputTokens: 0 } but the input tokens were billed. Observability only — nothing in the queue persists tokenUsage today. The right fix is probably persisting usage at all.
buildSystemPrompt / buildMessages sit outside the try, so s.score.toFixed(2) on a score-less source throws past the "never crash" fallback. Same shape in generateStream.
The confidence clamp checks suppress only while the pipeline clamps on suppress || forcesEscalation, so confidenceLevel can read MEDIUM where the pipeline treats it as LOW.
Two tests inject 429/500 for a single request without maxRetries: 0, so the SDK's own retries mean they pass on mock-miss behavior rather than on the assertion.
The generator's degraded flag is dead — pipeline.ts reads only confidenceAssessment.degraded. Pre-existing. Worth deleting or wiring, but the good outcome on the failure path comes from confidenceScore: 0, not the flag.
From the review of #223. That PR fixed the empty-text case and pinned it; this is the higher-yield half it deliberately didn't take, plus the adjacent findings.
The main point
generate()callsmessages.createwith notoolsand nothinking, so the API returns exactly one text block. The mode #223 guards — atool_useor thinking block making the 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.If "fail loudly on an unusable response" is the goal,
stop_reason !== 'end_turn'is the check that earns it.Not a drop-in: it changes what real reporters see on long answers, and it needs a decision about which non-
end_turnreasons count as unusable (max_tokensyes;stop_sequencearguably fine). That's why it isn't in #223.The reporter is promised a human twice
On the generation-failure path the reporter gets "A human support agent will follow up shortly" from the generator fallback and
AI_DISCLAIMER_ESCALATED("We've escalated this to our engineering team, someone will follow up in this thread shortly") from the pipeline. The suppress path has an explicit comment avoiding exactly this doubling; the generator-fallback path doesn't, and #223 routes more tickets into it.(The escalation job is enqueued, so the promise is honored — the defect is that it's stated twice, not that it's empty.)
An empty completion permanently burns the ticket's one AI response
The throw is caught ~30 lines down, so the job returns success and never sees the retry ladder (
MAX_JOB_ATTEMPTS). An empty completion is the archetypal transient failure, and the re-answer guard makes a pre-post retry safe — no BOT message exists yet at that point. Nothing is lost content-wise on this path since the text is empty by definition, so the only casualty is the retry.A distinguishable error (
EmptyModelResponseError) escaping the catch, or one in-place retry, would give the ladder a shot. The catch is the "never crash" contract every caller relies on, so this needs its own change rather than a loosenedcatch.generateStreamhas the identical defect, unfixedSame empty-response hole, with no fallback and no
degradedsignal. No production caller beyondpipeline.generateStreamingResponse. Streaming side of this is #228.Smaller, carried from the same review
tokenUsage: { inputTokens: 0, outputTokens: 0 }but the input tokens were billed. Observability only — nothing in the queue persiststokenUsagetoday. The right fix is probably persisting usage at all.buildSystemPrompt/buildMessagessit outside the try, sos.score.toFixed(2)on a score-less source throws past the "never crash" fallback. Same shape ingenerateStream.generateStreamyields a raw provider error as ordinary content, which the pipeline would buffer and publish — turning a 500 into a public post. No production caller, 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.suppressonly while the pipeline clamps onsuppress || forcesEscalation, soconfidenceLevelcan read MEDIUM where the pipeline treats it as LOW.maxRetries: 0, so the SDK's own retries mean they pass on mock-miss behavior rather than on the assertion.degradedflag is dead —pipeline.tsreads onlyconfidenceAssessment.degraded. Pre-existing. Worth deleting or wiring, but the good outcome on the failure path comes fromconfidenceScore: 0, not the flag.