Repository navigation
fix(generate): stop treating ai@6 raw-text output echo as parsed schema output - #1145
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ 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 |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughStructured-output recovery now treats JSON-encoded empty strings as empty completions. Experimental output identical to raw model text is routed through text coercion to avoid double-encoding. ChangesStructured-output recovery
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🤖 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 |
There was a problem hiding this comment.
Pull request overview
Fixes a behavioral change in ai@6 where experimental_output can echo raw model text (when no output spec was actually applied), which previously caused NeuroLink to misclassify raw text as parsed schema output and double-encode responses—most visibly turning empty completions into the literal string "".
Changes:
- In
GenerationHandler.formatEnhancedResult, detect theexperimental_outputraw-text echo and route it through text-mode coercion rather than serializing it as schema output. - Normalize JSON-encoded empty-string scalar outputs (
'""'→ parsed as"") to a true empty completion (content: "", nostructuredData) in bothGenerationHandlerand theneurolink.tsfacade path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/lib/neurolink.ts | Normalizes parsed scalar empty-string ("") to an empty completion for facade-level schema coercion on provider-native generate overrides. |
| src/lib/core/modules/GenerationHandler.ts | Avoids treating experimental_output raw-text echo as parsed schema output; normalizes parsed scalar empty-string to empty completion in text-mode coercion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (scalar === "") { | ||
| // A JSON-encoded empty string is an EMPTY completion, not a | ||
| // recovered scalar — normalize to a true empty ('' content, no | ||
| // structuredData) so callers' empty-response handling fires | ||
| // instead of a literal '""' reaching the user. | ||
| logger.warn( | ||
| "[GenerationHandler] schema requested but the model returned an empty JSON string; normalizing to empty content", | ||
| { provider: this.providerName, model: this.modelName }, | ||
| ); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
Added in 56c1d01 — test/continuous-test-suite-schema-empty-normalization.ts (pure suite, no API) drives formatEnhancedResult directly through all five shapes: empty raw-text echo (fallback result), literal "" completion, non-empty echo (must coerce, not double-encode), genuine Output.object object (preserved), and a parsed string ≠ raw text (z.string() schema, stays structured). Verified it fails 3/5 against the pre-fix code and passes 5/5 with the fix.
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 2 (src/lib/core/modules/GenerationHandler.ts, src/lib/neurolink.ts)
New issues raised this run: 0
Decision: Approve
The fix correctly identifies and routes the ai@6 raw-text echo away from the structured-output path, and the empty-JSON-string normalization addresses the production empty-response bug described in the PR. The changes are surgical, well-commented, and consistent with the existing coercion patterns in coerceJsonToSchema.
No blocking criteria are met: no secrets, no architectural rule violations, no breaking public SDK API changes, and no new MAJOR correctness issues. The existing unresolved review comment from the Copilot reviewer requesting a regression test for the literal "" path is noted; it is not duplicated here per review protocol, but should be resolved before merge.
🤖 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 |
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Reviewed 3 changed files. No new blocking issues found.
Files reviewed
src/lib/core/modules/GenerationHandler.tssrc/lib/neurolink.tstest/continuous-test-suite-schema-empty-normalization.ts
Assessment
- The raw-text echo detection in
formatEnhancedResultcorrectly identifies the ai@6output ?? text()fallback case and routes it through text coercion instead of double-encoding. - The empty-JSON-string (
"") normalization is applied consistently in both theGenerationHandlerpath and the facade scalar path inneurolink.ts, preventing the literal'""'content bug. - The regression test suite covers the key shapes: empty raw-text echo, literal
"", non-empty echo, genuineOutput.object, and parsed string ≠ raw text. - No CLAUDE.md critical-rule violations, security issues, or public SDK API breakage detected.
Existing comment
The existing Copilot review thread requesting a regression test for the literal "" path appears addressed by the added test/continuous-test-suite-schema-empty-normalization.ts.
Decision: Approve.
56c1d01 to
9053210
Compare
🤖 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 |
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 3
New issues raised: 1 (MINOR)
Blocking issues: 0
Assessment
The fix correctly addresses the ai@6 raw-text echo regression:
GenerationHandler.formatEnhancedResultnow detects whenexperimental_outputis the raw model text echo and routes it through text-mode coercion instead of treating it as parsed schema output.- The JSON-encoded empty string (
"") is normalized to a true empty completion withstructuredDataunset, which fixes the literal'""'content bug. - The same guard is applied in
neurolink.tsfor provider-nativegenerate()overrides. - A pure, no-API regression suite covers the five critical shapes: empty raw-text echo, literal
"", non-empty echo coercion, genuineOutput.object, and parsed string ≠ raw text.
No hardcoded secrets, security vulnerabilities, architectural rule violations, or public SDK API breaks were found.
Minor note
The new regression suite is not yet wired into package.json or the main test/continuous-test-suite.ts orchestrator, so it will not run in CI. I left an inline suggestion on the test file to add a matching script. This is non-blocking.
Approving — the core fix is sound and the regression coverage is good.
There was a problem hiding this comment.
💡 The new regression suite is well-scoped and covers the critical edge cases. Per CONTRIBUTING.md, new continuous-test-suite-*.ts files should also get a matching test:<name> script in package.json (or be imported by the main test/continuous-test-suite.ts orchestrator) so this runs in CI and doesn’t silently regress. Consider adding e.g.:
"test:schema-empty-normalization": "npx tsx test/continuous-test-suite-schema-empty-normalization.ts"and wiring it into the relevant aggregate script.
…ma output
Since ai@6 resolves `output ?? text()` inside generateText, a result
produced WITHOUT an output spec — the structured-output fallback retry,
or the tools↔schema exclusion path — no longer throws from the
`experimental_output` getter: it echoes the raw model text as a string.
formatEnhancedResult treated any defined `experimental_output` as the
AI-SDK's parsed+validated object, so on those paths it set
structuredData to the raw TEXT STRING and JSON.stringify'd it into the
content — double-encoding every response, and turning an EMPTY
completion into the literal two-character string '""' with
structuredData ''. Downstream, the facade's string-literal unwrap
accidentally repaired the non-empty case, but the empty case survived
to callers: consumers' empty-response handling never fired and users
saw a literal "".
Three changes:
- formatEnhancedResult: detect the raw-text echo (string identical to
generateResult.text) and route it through text-mode coercion instead
of serialising it as schema output.
- coerceTextMode scalar path: a JSON-encoded empty string ('""') is an
EMPTY completion, not a recovered scalar — normalize to '' content
with no structuredData (+ WARN) so empty-response handling fires.
- facade scalar path (finalizeGenerateRequestResult): same guard for
provider-native generate() overrides (Vertex/Google AI/Anthropic).
Adds test/continuous-test-suite-schema-empty-normalization.ts (pure, no
API) driving formatEnhancedResult through all five shapes; fails 3/5
against the pre-fix code, passes 5/5 with the fix. Existing coercion
suites (structured-coerce, coerce-nested-unwrap, json) pass; tsc clean.
9053210 to
35446e4
Compare
🤖 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 |
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 3
New issues raised: 0
Existing unresolved comments: 1 minor suggestion (package.json wiring for the new test suite — already noted by Tara-ag)
Assessment
The fix correctly addresses the ai@6 raw-text echo behavior in structured-output fallback paths:
GenerationHandler.formatEnhancedResultnow detects the raw-text echo (experimental_outputstring identical togenerateResult.text) and routes it through text-mode coercion instead of serializing it as parsed schema output.- The scalar coercion paths in both
GenerationHandlerandneurolink.tsnormalize a JSON-encoded empty string ('""') to a true empty completion, preventing the literal two-character string from reaching callers. - The regression suite covers the critical shapes: empty raw-text echo, literal
'""', non-empty echo coercion, genuineOutput.objectobjects, and parsed strings that differ from raw text.
Checked against project rules
- CLAUDE.md Critical Rule 5 (backward compatibility): No breaking changes to the public SDK API; the change only corrects edge-case output for schema fallback paths.
- Security: No secrets, credentials, or PII exposure in the new logs;
logger.warnonly includes provider/model identifiers. - Type safety / correctness: No
any/non-null assertions introduced in production code; the logic correctly distinguishes raw-text echoes from genuine parsed scalar strings. - Testing: New pure suite added in the
continuous-test-suite-*.tspattern.
Note
The existing unresolved comment from Tara-ag about adding a matching test:schema-empty-normalization script in package.json remains unaddressed. That is a valid CI/wiring suggestion but is non-blocking; it can be followed up in this PR or a fast-follow.
Approving — the blocking bug is fixed and the change is low-risk.
|
🎉 This PR is included in version 9.86.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Problem
Since ai@6 resolves
output ?? text()insidegenerateText, a result produced without an output spec no longer throws from theexperimental_outputgetter — it echoes the raw model text as a string. Two NeuroLink paths produce such results while a schema is active:NoObjectGeneratedError/ tools↔schema conflict / schema-complexity errors), andformatEnhancedResulttreats any definedexperimental_outputas the AI-SDK's parsed+validated object, so on those paths it:structuredDatato the raw text string, andJSON.stringifys it intocontent— double-encoding every response.For a non-empty response the facade's JSON-string-literal unwrap accidentally repairs the damage downstream. For an EMPTY completion it survives to the caller as content
'""'(the literal two characters) withstructuredData ''— so consumers' empty-response handling never fires and users see a literal"". This was found in production-style E2E testing of curator's empty-response retry (its retry predicate never fired on the schema path).Fix (3 changes)
GenerationHandler.formatEnhancedResulttypeof experimental_output === 'string' && === generateResult.text) and route it through text-mode coercion instead of serialising it as schema outputGenerationHandlercoerceTextModescalar path'""') is an empty completion, not a recovered scalar — normalize to''content, nostructuredData, + WARNneurolink.tsfacade scalar pathgenerate()overrides (Vertex / Google AI / Anthropic native)Genuine
Output.objectresults are unaffected: theirexperimental_outputis a parsed object (or a parsed string ≠ raw text), never the raw-text echo.Verification (stubbed Anthropic API, wire-level)
'""', structuredData'''', structuredData unset ✅""as its completion'""', structuredData'''', structuredData unset ✅''''(no regression) ✅tsc --noEmitcleancontinuous-test-suite-structured-coerce/coerce-nested-unwrap/json— all PASS🤖 Investigated & authored with Claude Code
Summary by CodeRabbit