fix(observability): enrich NoOutputGeneratedError sentinel chunk metadata - #990
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (25)
WalkthroughThis PR introduces a centralized "no output" sentinel handling system across the streaming pipeline. It replaces provider-local error handling with shared utilities ( Changes
Sequence DiagramsequenceDiagram
participant Client
participant Provider as Provider Stream
participant StreamHandler
participant Utilities as Sentinel Utils
participant Wrapper as Wrapper (neurolink)
Client->>Provider: initiate stream
Provider->>Provider: onError callback (capture upstream error)
loop streaming chunks
Provider-->>StreamHandler: yield chunk/NoOutputGeneratedError
StreamHandler->>StreamHandler: count content chunks
alt NoOutputGeneratedError caught
StreamHandler->>Utilities: buildNoOutputSentinel(error, captured)
Utilities->>Utilities: enrich with provider error, usage
Utilities-->>StreamHandler: NoOutputSentinel
StreamHandler->>Utilities: stampNoOutputSpan(sentinel)
StreamHandler-->>Wrapper: yield sentinel
else chunk received
StreamHandler-->>Wrapper: yield chunk
end
end
alt no chunks yielded
StreamHandler->>Utilities: detectPostStreamNoOutput(result, captured)
Utilities->>Utilities: await finishReason (detect rejection)
Utilities-->>StreamHandler: sentinel (if rejected)
StreamHandler->>Utilities: stampNoOutputSpan(sentinel)
StreamHandler-->>Wrapper: yield sentinel
end
Wrapper->>Wrapper: filter chunks (exclude sentinels)
Wrapper->>Wrapper: count real output (text/media only)
alt realOutputChunks === 0
Wrapper->>Wrapper: trigger fallback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Documentation Validation Results🚀 Documentation validation passed!
📦 Build artifact uploaded successfully. Ready for deployment preview. Commit: |
There was a problem hiding this comment.
Pull request overview
Enhances observability for zero-output streaming failures by enriching the NoOutputGeneratedError sentinel chunk metadata emitted from StreamHandler.createTextStream(), enabling downstream telemetry to understand why a stream produced no output.
Changes:
- Enrich
NoOutputGeneratedErrorsentinel chunk metadata withfinishReason,usage,providerError, andmodelResponseRaw, and stamp the active OTel span with a status message. - Add a continuous test script (plus env/credential guard helpers) to verify the enriched sentinel metadata in the compiled artifact and via best-effort live provider reproduction.
- Add documentation describing the Curator Issue #6 report, fix, and verification steps.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/lib/core/modules/StreamHandler.ts |
Adds best-effort metadata extraction in the NoOutputGeneratedError path and enriches span attributes + sentinel chunk metadata. |
test/helpers/envGuard.ts |
Adds helpers for skipping tests when env vars are missing and classifying expected provider/credential errors. |
test/continuous-test-suite-issue-06-no-output-context.ts |
Adds a reproduction/verification script (static artifact scan + live provider recipes) for Issue #6. |
docs/curator-feedback-fixes/issue-06-no-output-context.md |
Documents the issue, fix, and verification procedure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| > When AI SDK throws `NoOutputGeneratedError`, the sentinel chunk yielded | ||
| > by `StreamHandler.createTextStream` carries only `{ noOutput: true, | ||
| errorType: "NoOutputGeneratedError" }` — no `finishReason`, no `usage`, |
There was a problem hiding this comment.
The blockquote in the Curator report is broken across lines: the wrapped errorType: "NoOutputGeneratedError" line is missing the leading > marker, so it won’t render as part of the quote. Add > to the continuation line (or keep the snippet on one line) to preserve the intended formatting.
| errorType: "NoOutputGeneratedError" }` — no `finishReason`, no `usage`, | |
| > errorType: "NoOutputGeneratedError" }` — no `finishReason`, no `usage`, |
| promptTokens?: number; | ||
| completionTokens?: number; | ||
| }; | ||
| const summary = `Stream produced no output (NoOutputGeneratedError): finishReason=${String(finishReason)}, promptTokens=${u?.promptTokens ?? 0}, completionTokens=${u?.completionTokens ?? 0}`; |
There was a problem hiding this comment.
langfuse.status_message set here is likely overwritten on span end by ContextEnricher.applyNonErrorLangfuseLevel() when it sees neurolink.no_output === true (it unconditionally sets langfuse.status_message to a generic string). To preserve the enriched message, either set langfuse.level here (e.g. WARNING) so applyNonErrorLangfuseLevel won’t run, or update the enricher to only set langfuse.status_message if it isn’t already present.
| const summary = `Stream produced no output (NoOutputGeneratedError): finishReason=${String(finishReason)}, promptTokens=${u?.promptTokens ?? 0}, completionTokens=${u?.completionTokens ?? 0}`; | |
| const summary = `Stream produced no output (NoOutputGeneratedError): finishReason=${String(finishReason)}, promptTokens=${u?.promptTokens ?? 0}, completionTokens=${u?.completionTokens ?? 0}`; | |
| activeSpan.setAttribute("langfuse.level", "WARNING"); |
| let finishReason: unknown = "error"; | ||
| let usage: unknown = { | ||
| promptTokens: 0, | ||
| completionTokens: 0, | ||
| totalTokens: 0, | ||
| }; | ||
| try { | ||
| if (result.finishReason !== undefined) { | ||
| finishReason = await Promise.resolve(result.finishReason); | ||
| } | ||
| } catch { | ||
| // Expected — AI SDK rejects it on no-output. Keep "error" default. | ||
| } | ||
| try { | ||
| if (result.totalUsage !== undefined) { | ||
| usage = await Promise.resolve(result.totalUsage); | ||
| } | ||
| } catch { |
There was a problem hiding this comment.
The new totalUsage field doesn’t appear to exist on the AI SDK StreamTextResult used throughout the codebase (providers and analytics collector use result.usage). As a result this branch will almost always keep the default object and won’t benefit from future SDK improvements. Consider accepting/reading usage (and normalizing it to the repo’s TokenUsage shape via extractTokenUsage/createEmptyTokenUsage) instead of introducing totalUsage + a {promptTokens,...} default that doesn’t match the {input, output, total} convention used elsewhere (e.g. stream span enrichment).
| console.log( | ||
| `\n${colors.bright}Results:${colors.reset} ${passed} passed, ${failed} failed, ${skipped} skipped`, | ||
| ); | ||
| process.exit(0); // bug repro: failed > 0 expected |
There was a problem hiding this comment.
This script always exits with code 0 even when one or more checks record FAIL, which makes it easy for regressions to slip by in CI/automation. Consider exiting non-zero when failed > 0 (similar to other continuous test suite scripts) so the verification command in the PR description actually enforces the expected behavior.
| process.exit(0); // bug repro: failed > 0 expected | |
| process.exit(failed > 0 ? 1 : 0); |
265978b to
80b90a5
Compare
|
Force-pushed addressing reviewer Finding #6 + applying same recipe as #988/#989:
Types live in LiteLLM intentionally still rethrows — its inline comment at line 547 says "propagating to fallback chain" and that's a deliberate choice for fallback orchestration, not a bug. Left untouched. Static artifact check now reads @coderabbitai full review |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
Force-pushed addressing reviewer Finding #6 follow-up: Two providers were still uncovered by the previous round:
All five provider stream-transformers now go through the shared @coderabbitai full review |
80b90a5 to
adfa898
Compare
|
🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
adfa898 to
318eab7
Compare
|
Force-pushed addressing the reviewer's three follow-up concerns:
Static check still PASS; live recipes still SKIP unless a provider trip fires (rare path, expected). @coderabbitai full review |
|
Will do a thorough pass over all five provider sites, the shared helper, the type definitions, and the updated test suite now. 🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
Force-pushed: static verifier now also requires The helper already populates Verified: @coderabbitai full review |
318eab7 to
9795578
Compare
|
🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
9795578 to
47bf52f
Compare
Aggregate validation against Vertex/Gemini-3.1-Pro and Anthropic/Sonnet-4.6Ran every continuous test suite (37 files, 56 suite-runs across two model passes + a no-AI pass) against this branch. Findings: PR-scope test passes cleanly: Wider suite (40 of 56 suite-runs PASS): the 16 residual failures are all categorized as non-PR causes:
None of the residual failures intersect the PR's surface: This commit also includes a separate test-hygiene cleanup (no functional code changes) so the broader suite respects model env vars consistently — @coderabbitai full review |
|
Triggering the full review now. 🧠 Learnings used✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/neurolink.ts (1)
7204-7214:⚠️ Potential issue | 🟠 MajorFallback still treats sentinel-only retry streams as success.
This gate now correctly sends sentinel-only primary streams into
handleStreamFallback(), but the fallback path still treats any yielded chunk as success. If the fallback provider also emits only the shared no-output sentinel,fallbackChunkCountbecomes non-zero and the call returns an effectively empty stream instead of surfacing the failure or continuing orchestration. Please make the fallback path use the same “real output chunk” predicate as this gate.Suggested direction
- let fallbackChunkCount = 0; + let fallbackChunkCount = 0; + let fallbackRealOutputChunks = 0; for await (const fallbackChunk of fallbackResult.stream) { fallbackChunkCount++; + const isNoOutputSentinel = + fallbackChunk !== null && + typeof fallbackChunk === "object" && + "metadata" in fallbackChunk && + (fallbackChunk as { metadata?: Record<string, unknown> }).metadata + ?.noOutput === true; + const hasTextContent = + fallbackChunk && + "content" in fallbackChunk && + typeof fallbackChunk.content === "string" && + fallbackChunk.content.length > 0; + const hasMediaPayload = + fallbackChunk !== null && + typeof fallbackChunk === "object" && + "type" in fallbackChunk && + ((fallbackChunk as { type?: unknown }).type === "audio" || + (fallbackChunk as { type?: unknown }).type === "image"); + if (!isNoOutputSentinel && (hasTextContent || hasMediaPayload)) { + fallbackRealOutputChunks++; + } if ( fallbackChunk && "content" in fallbackChunk && typeof fallbackChunk.content === "string" ) { appendContent(fallbackChunk.content); this.emitter.emit("response:chunk", fallbackChunk.content); } yield fallbackChunk; } if ( - fallbackChunkCount === 0 && + fallbackRealOutputChunks === 0 && fallbackToolCalls.length === 0 && fallbackToolResults.length === 0 ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 7204 - 7214, The fallback path currently counts any yielded chunk (including sentinel-only chunks) as success; update the fallback consumer in handleStreamFallback so it uses the same real-output predicate as the primary gate (i.e. only increment fallbackChunkCount for non-sentinel/non-empty "real" chunks), referencing symbols like realOutputChunks, handleStreamFallback, fallbackChunkCount, and the sentinel detection logic used where realOutputChunks is computed; if the fallback emits only sentinel chunks, treat it as a non-success (do not consider fallbackChunkCount > 0) so the call can surface failure or continue orchestration.
🧹 Nitpick comments (3)
src/lib/core/modules/StreamHandler.ts (2)
138-138: Yielded sentinel doesn't match the declared element type.The generator is typed
AsyncGenerator<{ content: string }>butyield sentinel(and the post-streamyield detected.sentinel) emits aNoOutputSentinel({ content: "", metadata: {...} }). It compiles because excess properties are allowed when assigning from a variable, but downstream consumers can only seemetadataafter a cast — which defeats the centralized sentinel contract.Consider widening the return type to
AsyncGenerator<{ content: string } | NoOutputSentinel>(or to a discriminated union) so the metadata is visible at the type level andmetadata.noOutput === truechecks don't need any-casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/core/modules/StreamHandler.ts` at line 138, The generator signature in StreamHandler.ts currently returns AsyncGenerator<{ content: string }>, but it yields the sentinel value (sentinel and detected.sentinel) of type NoOutputSentinel; update the generator return type to AsyncGenerator<{ content: string } | NoOutputSentinel> (or a discriminated union type that includes NoOutputSentinel) so metadata is visible at the type level; adjust any places that consume the generator to narrow on the discriminant (e.g., metadata.noOutput) instead of casting, and ensure the NoOutputSentinel type is imported/used consistently where sentinel is created and yielded.
175-201: StreamHandler path can't capture upstream provider error like provider-specific paths do.The provider stream-transformers in this PR (
openAI,litellm,huggingFace,openRouter,anthropicBaseProvider,openaiCompatible) all capture the AI SDK's upstream error viastreamText({ onError })and pass it as the thirdunderlyingErrorargument tobuildNoOutputSentinel/detectPostStreamNoOutput, so the sentinel'sproviderError/modelResponseRawcarry the real cause (e.g.content_filter, provider crash) rather than the AI SDK's generic "No output generated" message.
StreamHandler.createTextStreamdoes not accept anunderlyingErrorparameter and calls both helpers without it. Any provider that goes throughStreamHandler(rather than its own transformer) will silently lose this enrichment and emit the generic AI SDK message in telemetry. Consider extending the input contract to also accept an optional underlying-error getter (or value) and threading it through:♻️ Suggested signature/threading
createTextStream(result: { textStream: AsyncIterable<string>; finishReason?: Promise<unknown> | unknown; totalUsage?: Promise<unknown> | unknown; + /** + * Optional accessor for the upstream provider error captured via + * streamText's `onError` callback. Threaded into + * buildNoOutputSentinel / detectPostStreamNoOutput so the sentinel's + * providerError / modelResponseRaw carry the real cause. + */ + getCapturedProviderError?: () => unknown; }): AsyncGenerator<{ content: string }> { const providerName = this.providerName; return (async function* () { ... - const sentinel = await buildNoOutputSentinel(error, result); + const sentinel = await buildNoOutputSentinel( + error, + result, + result.getCapturedProviderError?.(), + ); ... - const detected = await detectPostStreamNoOutput(result); + const detected = await detectPostStreamNoOutput( + result, + result.getCapturedProviderError?.(), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/core/modules/StreamHandler.ts` around lines 175 - 201, createTextStream is losing provider-specific upstream errors because it doesn't accept or forward an underlyingError to buildNoOutputSentinel/detectPostStreamNoOutput; update StreamHandler.createTextStream to accept an optional underlyingError (either an Error or a getter function returning Error/Promise<Error>), thread that value into the catch where buildNoOutputSentinel(...) is called and into the post-stream detection call detectPostStreamNoOutput(result, underlyingError), and ensure any stampNoOutputSpan calls continue to receive the sentinel unchanged; also update all internal callers of createTextStream to pass the provider's underlying error/getter so provider-specific errors flow into providerError/modelResponseRaw.test/continuous-test-suite-issue-06-no-output-context.ts (1)
162-213: Minor: abort timer is never cleared.If the recipe completes before
_abortAfterMs, thesetTimeouthandle (Line 172) is left to fireac.abort()on an already-finished signal and keeps the event loop alive until it does. Not material for this CLI runner (itprocess.exits inmain), but worth tracking the handle and clearing it infinallyfor cleanliness if you revisit this helper.♻️ Optional cleanup
- const abortAfter = (recipe.options as { _abortAfterMs?: number }) - ._abortAfterMs; - if (abortAfter) { - setTimeout(() => ac.abort(), abortAfter); - } + const abortAfter = (recipe.options as { _abortAfterMs?: number }) + ._abortAfterMs; + let abortTimer: ReturnType<typeof setTimeout> | undefined; + if (abortAfter) { + abortTimer = setTimeout(() => ac.abort(), abortAfter); + } try { // … } finally { + if (abortTimer) clearTimeout(abortTimer); await sdk.shutdown?.()?.catch(() => {}); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-issue-06-no-output-context.ts` around lines 162 - 213, In tryRecipe, the abort timer created by setTimeout is never cleared; capture the timeout handle when calling setTimeout (e.g., store it in a variable like abortTimer) and in the finally block clear it (clearTimeout(abortTimer)) before calling sdk.shutdown to prevent the timer from keeping the event loop alive; reference the AbortController ac, the abortAfter variable, and the finally block around sdk.shutdown for where to add the clearTimeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/types/noOutputSentinel.ts`:
- Around line 7-27: Exported stream types use non-prefixed names; rename
NoOutputSentinel to StreamNoOutputSentinel and NoOutputSentinelResultLike to
StreamNoOutputSentinelResultLike, update all type declarations and export
identifiers in this file, and then update any imports/usages across the codebase
to reference the new names (e.g., replace occurrences of NoOutputSentinel and
NoOutputSentinelResultLike with StreamNoOutputSentinel and
StreamNoOutputSentinelResultLike).
In `@src/lib/utils/noOutputSentinel.ts`:
- Around line 71-84: The code reads (error as { cause?: unknown }).cause
unguarded which can throw if error is null/undefined; modify the cause
extraction so you only access error.cause when error is non-null (e.g. check
error != null && typeof error === "object" or use safe optional access) before
falling back to causeFromSource, and leave the modelResponseRaw logic unchanged
so it always slices a string (use String(cause) when present, otherwise use
`${messageSource.name}: ${messageSource.message}`) ensuring modelResponseRaw is
always populated as a string.
- Around line 38-42: The default usage object in noOutputSentinel.ts only
includes v4 keys (promptTokens, completionTokens, totalTokens), so downstream
consumers that expect v6 keys (inputTokens, outputTokens) will see zeros; update
the default `usage` value (the variable named usage in noOutputSentinel.ts) to
include both sets of keys — add inputTokens and outputTokens alongside
promptTokens and completionTokens (keep totalTokens) so callers like
amazonBedrock.ts and streamingClient.ts that read inputTokens/outputTokens ?? 0
get correct defaults; ensure buildNoOutputStatusMessage continues to work with
both shapes.
In `@test/continuous-test-suite-context.ts`:
- Around line 66-75: The current single VERTEX_TEST_MODEL defaults to
"gemini-2.5-pro" which causes Flash tests to run on Pro; split this into two
constants (e.g. VERTEX_PRO_TEST_MODEL and VERTEX_FLASH_TEST_MODEL), each reading
an explicit env override (like process.env.VERTEX_PRO_TEST_MODEL /
process.env.VERTEX_FLASH_TEST_MODEL) falling back to process.env.TEST_MODEL and
then to conservative defaults ("gemini-2.5-pro" for Pro and "gemini-2.5-flash"
for Flash), and replace uses of VERTEX_TEST_MODEL in Flash-specific tests with
VERTEX_FLASH_TEST_MODEL and Pro-specific tests with VERTEX_PRO_TEST_MODEL so
Flash tests no longer silently run on Pro.
In `@test/continuous-test-suite-issue-06-no-output-context.ts`:
- Around line 561-578: The existing doc comment for "Test 6.5 — INVESTIGATION"
is outdated and contradicts the test implementation (which now records FAIL via
calls in detectPostStreamNoOutput), so update the block comment to reflect that
Test 6.5 is now a regression gate that can FAIL CI; remove or rewrite the
"informational / recorded SKIP" wording and explicitly state the test now
asserts post-stream no-output detection and will record FAIL on regressions,
referencing the detectPostStreamNoOutput behavior and the fail-recording logic
used in this test.
- Around line 935-952: The FAIL branch references an undefined variable
usesRealContentChunks which will throw; update the template to use the existing
boolean usesRealOutputChunks (or otherwise define usesRealContentChunks) so the
record(...) call can safely log the two gate booleans; locate the conditional
that sets usesRealOutputChunks and incrementsForRealOnly and fix the template
literal in the record(...) call to reference usesRealOutputChunks and
incrementsForRealOnly alongside testName so failures are recorded instead of
causing a ReferenceError.
---
Outside diff comments:
In `@src/lib/neurolink.ts`:
- Around line 7204-7214: The fallback path currently counts any yielded chunk
(including sentinel-only chunks) as success; update the fallback consumer in
handleStreamFallback so it uses the same real-output predicate as the primary
gate (i.e. only increment fallbackChunkCount for non-sentinel/non-empty "real"
chunks), referencing symbols like realOutputChunks, handleStreamFallback,
fallbackChunkCount, and the sentinel detection logic used where realOutputChunks
is computed; if the fallback emits only sentinel chunks, treat it as a
non-success (do not consider fallbackChunkCount > 0) so the call can surface
failure or continue orchestration.
---
Nitpick comments:
In `@src/lib/core/modules/StreamHandler.ts`:
- Line 138: The generator signature in StreamHandler.ts currently returns
AsyncGenerator<{ content: string }>, but it yields the sentinel value (sentinel
and detected.sentinel) of type NoOutputSentinel; update the generator return
type to AsyncGenerator<{ content: string } | NoOutputSentinel> (or a
discriminated union type that includes NoOutputSentinel) so metadata is visible
at the type level; adjust any places that consume the generator to narrow on the
discriminant (e.g., metadata.noOutput) instead of casting, and ensure the
NoOutputSentinel type is imported/used consistently where sentinel is created
and yielded.
- Around line 175-201: createTextStream is losing provider-specific upstream
errors because it doesn't accept or forward an underlyingError to
buildNoOutputSentinel/detectPostStreamNoOutput; update
StreamHandler.createTextStream to accept an optional underlyingError (either an
Error or a getter function returning Error/Promise<Error>), thread that value
into the catch where buildNoOutputSentinel(...) is called and into the
post-stream detection call detectPostStreamNoOutput(result, underlyingError),
and ensure any stampNoOutputSpan calls continue to receive the sentinel
unchanged; also update all internal callers of createTextStream to pass the
provider's underlying error/getter so provider-specific errors flow into
providerError/modelResponseRaw.
In `@test/continuous-test-suite-issue-06-no-output-context.ts`:
- Around line 162-213: In tryRecipe, the abort timer created by setTimeout is
never cleared; capture the timeout handle when calling setTimeout (e.g., store
it in a variable like abortTimer) and in the finally block clear it
(clearTimeout(abortTimer)) before calling sdk.shutdown to prevent the timer from
keeping the event loop alive; reference the AbortController ac, the abortAfter
variable, and the finally block around sdk.shutdown for where to add the
clearTimeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07f6ad88-51ca-4da0-99ab-11fc6c900a70
📒 Files selected for processing (18)
src/lib/core/modules/StreamHandler.tssrc/lib/neurolink.tssrc/lib/providers/anthropicBaseProvider.tssrc/lib/providers/huggingFace.tssrc/lib/providers/litellm.tssrc/lib/providers/openAI.tssrc/lib/providers/openRouter.tssrc/lib/providers/openaiCompatible.tssrc/lib/services/server/ai/observability/instrumentation.tssrc/lib/types/index.tssrc/lib/types/noOutputSentinel.tssrc/lib/types/stream.tssrc/lib/utils/noOutputSentinel.tstest/continuous-test-suite-autoresearch-live.tstest/continuous-test-suite-context.tstest/continuous-test-suite-dynamic.tstest/continuous-test-suite-issue-06-no-output-context.tstest/continuous-test-suite-proxy.ts
…data + actually trigger the catch path the production bug needs
Curator P3-6: when the AI SDK throws NoOutputGeneratedError on a
zero-output stream, the sentinel chunk yielded `{noOutput: true,
errorType: "NoOutputGeneratedError"}` but no `finishReason`, no `usage`,
no `providerError`, no `modelResponseRaw` — telemetry consumers had no
way to differentiate between content-filter, stop-sequence pre-emption,
or mid-stream provider crash.
Round-1 fix added a `buildNoOutputSentinel` helper and wired it into the
catch block of every provider's `for await (chunk of result.textStream)`
iteration. Round-2 deep investigation found that catch block is dead
code in AI SDK v6.0.141: the SDK rejects the metadata promises
(`result.finishReason`, `result.totalUsage`, `result.steps`) when 0
steps are recorded, but it does NOT throw from textStream itself —
the iteration just completes silently with 0 chunks. Verified by
direct AI SDK probing across 9 different SSE/connection scenarios.
Round-2 fix:
1. New `detectPostStreamNoOutput(result)` helper in noOutputSentinel.ts
awaits `result.finishReason`, returns `{sentinel, error}` when it
rejects with NoOutputGeneratedError, else null.
2. All 7 wired sites (openAI, openaiCompatible, litellm, huggingFace,
openRouter, anthropicBaseProvider, StreamHandler) now call the
helper after the textStream loop completes with 0 chunks. The
existing catch block stays for defensive coverage of the rare path
where AI SDK does throw from textStream.
3. LiteLLM preserves yield-then-throw (yields sentinel, then throws
`detected.error`) so its existing fallback chain still fires.
4. StreamHandler additionally stamps the active OTel span with
`langfuse.status_message` + `neurolink.no_output.finish_reason` so
Pipeline B observability surfaces the right status.
Test (`test/continuous-test-suite-issue-06-no-output-context.ts`):
- 6.0 STATIC: shipped sentinel literal has all 6 enriched keys
- 6.1 STATIC: every wired provider uses the helper (regression guard)
- 6.2 RUNTIME: helper produces full sentinel with correct types
- 6.3 RUNTIME: helper reads partial finishReason/usage from a
result-like, falls back to defaults when promises reject
- 6.4 RUNTIME: helper extracts `error.cause` into modelResponseRaw
- 6.5 END-TO-END: a local HTTP server replays the production trigger
(200 OK + connection-kill before any data). Real NeuroLink
stream consumption finds the enriched sentinel chunk with all
6 keys. This proves the fix actually fires for the bug Curator
captured — round-1's catch block did not.
- 6.x BEST-EFFORT: alphabet-wide stop-sequence recipe per provider
(skips when provider emits any content; expected)
Result: 12 PASS, 0 FAIL, 3 SKIP, exit=0.
47bf52f to
bfa23c8
Compare
Round-N: addressed all 10 reviewer findings (
|
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
🧩 Analysis chain🏁 Script executed: Length of output: 17058
One very minor observation: Triggering the full review now. 🧠 Learnings used✅ Actions performedReview triggered.
|
|
🎉 This PR is included in version 9.59.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Curator P3-6: when the AI SDK throws
NoOutputGeneratedError, the sentinel chunk yielded byStreamHandler.createTextStream()previously carried only{ noOutput: true, errorType }. Downstream telemetry couldn't tell why the stream produced no output.Reproduction (before fix)
After fix:
Fix
src/lib/core/modules/StreamHandler.ts— in theNoOutputGeneratedErrorcatch block:awaitofresult.finishReasonandresult.totalUsage(AI SDK'sStreamTextResultgetters). They reject today; if a future SDK version surfaces partial values the sentinel automatically carries them.finishReason: "error"and zero-usage when those getters reject.error.messageasproviderError,error.cause(truncated to 500 chars) asmodelResponseRaw.langfuse.status_message:Type signature change
createTextStreamnow accepts optionalfinishReasonandtotalUsage:Existing callers don't need changes — both new fields are optional, and providers passing the AI SDK's full
StreamTextResultautomatically benefit.Backward compatibility
Additive only. Listeners that check
noOutput === truecontinue to work unchanged.Verification
Expected:
Results: 1 passed, 0 failed, 18 skipped. The 18 SKIPs are dynamic-trigger recipes that don't fireNoOutputGeneratedErroron the configured providers — the AI SDK only throws when zero text-delta parts are emitted, which is rare in practice. The static check is the deterministic reproduction.Test plan
releaselangfuse.status_messagenoOutput === truepredicate (still set)Summary by CodeRabbit
Bug Fixes
Tests