Repository navigation
fix(structured): recover most-complete object + unwrap string-literal output in coerceJsonToSchema - #1088
Conversation
…eral output
coerceJsonToSchema() is the fallback that recovers a schema-valid object from
imperfect model text when AI-SDK experimental_output yields nothing. Two
robustness gaps closed (previously each consumer hand-rolled this):
- Among MULTIPLE schema-valid candidates, prefer the most COMPLETE one instead
of breaking on the first match. With nullable fields a lean preamble
({summary, attachment:null}) validates alongside the real
{summary, attachment:{...}}; first-match returned the preamble and dropped the
payload (the classic 'no file delivered' production symptom).
- Unwrap a JSON-string-literal wrapper (providers that double-encode the object
as a JSON string) and scan the inner text for balanced spans.
No behavior change when 0 or 1 candidate validates, or when no schema is given
(first parseable object still wins). Adds a deterministic test suite.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesJSON Coercion Enhancement
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/utils/json/coerce.ts (1)
176-188: ⚡ Quick winConsider caching JSON.stringify lengths to avoid redundant serialization.
The reduce calls
JSON.stringify(cur.value).lengthandJSON.stringify(best.value).lengthon each comparison. For N schema-valid candidates, this results in 2*(N-1) stringify calls. Pre-computing and storing the stringified length for each candidate would reduce this to N calls, which could meaningfully improve performance when objects are large or many candidates are present.⚡ Proposed optimization
+ // Pre-compute stringify lengths for schema-valid candidates + const schemaValidWithSize = schemaValid.map((record) => ({ + ...record, + size: JSON.stringify(record.value).length, + })); + const schemaMatch = - schemaValid.length > 0 - ? schemaValid.reduce((best, cur) => - JSON.stringify(cur.value).length > JSON.stringify(best.value).length - ? cur - : best, - ) + schemaValidWithSize.length > 0 + ? schemaValidWithSize.reduce((best, cur) => + cur.size > best.size ? cur : best, + ) : undefined;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/json/coerce.ts` around lines 176 - 188, The reduce that selects schemaMatch repeatedly calls JSON.stringify on cur.value and best.value causing redundant serializations; fix by precomputing and caching each candidate's serialized length (e.g., map schemaValid to include a computed length for each entry) and then use those cached lengths inside the reduce to compare instead of re-stringifying cur.value/best.value, updating any references from cur.value/best.value length checks to the new cachedLength field on the candidate objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/utils/json/coerce.ts`:
- Around line 176-188: The reduce that selects schemaMatch repeatedly calls
JSON.stringify on cur.value and best.value causing redundant serializations; fix
by precomputing and caching each candidate's serialized length (e.g., map
schemaValid to include a computed length for each entry) and then use those
cached lengths inside the reduce to compare instead of re-stringifying
cur.value/best.value, updating any references from cur.value/best.value length
checks to the new cachedLength field on the candidate objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 598d4fd9-1e87-4138-a0d2-234cae209a57
📒 Files selected for processing (2)
src/lib/utils/json/coerce.tstest/continuous-test-suite-structured-coerce.ts
Tara-ag
left a comment
Tara-ag
left a comment
There was a problem hiding this comment.
Review Summary
Files reviewed: 2
New issues raised: 2 (both MINOR)
Blocking issues: 0
Assessment
This PR is approved for merge. The changes are well-designed, thoroughly tested, and address real production issues.
What Changed
-
src/lib/utils/json/coerce.ts- Enhanced JSON coercion with two robustness improvements:- Most-complete selection: When multiple schema-valid candidates exist, picks the one with the most content (serialized length) instead of first-match. This fixes the "preamble vs payload" issue where a lean
{summary, attachment: null}was incorrectly selected over the full object. - String-literal unwrapping: Handles double-encoded JSON from providers that return objects as JSON strings (
"{\"k\":1}").
- Most-complete selection: When multiple schema-valid candidates exist, picks the one with the most content (serialized length) instead of first-match. This fixes the "preamble vs payload" issue where a lean
-
test/continuous-test-suite-structured-coerce.ts- New deterministic test suite with 7 test cases covering edge cases.
Verification
✅ Security: No hardcoded secrets, no injection risks, no unsafe eval
✅ Architecture: Changes localized to utility function; no circular dependencies introduced
✅ Backward compatibility: Behavior unchanged for single-candidate path (the common case)
✅ Type safety: Proper types used, no any or @ts-ignore
✅ Testing: Comprehensive test coverage for new functionality
✅ CLAUDE.md compliance: No violations of critical rules
Minor Suggestions (non-blocking)
- Performance: The
reducecallback could cacheJSON.stringify()results to avoid redundant serialization (2×(N-1) calls for N candidates) - Developer experience: Consider adding
"test:structured-coerce": "npx tsx test/continuous-test-suite-structured-coerce.ts"to package.json scripts
Code Quality Notes
- Clean implementation with clear comments explaining the rationale
- Good use of existing patterns (
nextBalancedJsonSpan,hasSafeParse) - Proper handling of edge cases (empty input, no schema, parse failures)
- Test assertions are deterministic and don't rely on live API calls
The PR is ready to merge. 🚀
|
|
||
| // Among schema-valid candidates prefer the MOST COMPLETE one. With nullable | ||
| // fields a lean object (e.g. `{summary, attachment: null}`) validates | ||
| // alongside the full object, so breaking on the first match would drop the |
There was a problem hiding this comment.
💡 MINOR: Performance optimization opportunity
The reduce callback calls JSON.stringify() on both cur.value and best.value for every comparison. For N schema-valid candidates, this results in 2*(N-1) serializations.
Suggestion: Cache the serialized length to avoid redundant work:
const schemaMatch =
schemaValid.length > 0
? schemaValid
.map((s) => ({ ...s, serializedLength: JSON.stringify(s.value).length }))
.reduce((best, cur) =>
cur.serializedLength > best.serializedLength ? cur : best
)
: undefined;This is a minor optimization since schemaValid is typically small, but worth considering for consistency with the codebase's performance-conscious patterns.
There was a problem hiding this comment.
💡 MINOR: Test script not registered in package.json
The new test file test/continuous-test-suite-structured-coerce.ts exists but there's no corresponding npm script to run it conveniently. Consider adding to package.json:
"test:structured-coerce": "npx tsx test/continuous-test-suite-structured-coerce.ts"This follows the pattern of other test suites like test:json, test:workflow, etc.
|
🎉 This PR is included in version 9.70.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Problem
coerceJsonToSchema()is NeuroLink's fallback that recovers a schema-valid object from imperfect model text when AI-SDKexperimental_outputyields nothing. It already scans balanced spans + jsonrepairs, but two robustness gaps forced every downstream consumer to hand-roll its own extractor on top:{ summary, attachment: z.object(...).nullable() }), a lean preamble{"summary":"working…","attachment":null}validates alongside the real{"summary":"done","attachment":{…}}. First-match returned the preamble and discarded the payload — the classic "model streamed a short preamble then the real answer, and the file never got delivered" symptom (seen in production traces)."{\"k\":1}"); the balanced scan over the escaped text can't recover it.Fix (additive, low-risk)
In
src/lib/utils/json/coerce.ts:Why this belongs here
This is generic structured-output robustness — the same logic was duplicated in at least one downstream app (Tara/curator). Centralizing it means every NeuroLink consumer gets correct multi-candidate recovery and the app-side extractor can be deleted.
Verification
test/continuous-test-suite-structured-coerce.ts(7/7): preamble-vs-payload both orders, fenced-in-prose, string-literal wrapper, clean object, no-schema first-wins, and no-object→null.pnpm buildclean (lint + typecheck pass).Scope
Single file + test. Behavior is unchanged for the common single-candidate path; only the ambiguous multi-valid-candidate and double-encoded cases change (both strictly toward recovering the intended object).
Summary by CodeRabbit
Bug Fixes
Tests