fix(generation): guarantee valid JSON + expose structuredData for schema requests - #1080
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
✅ 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 |
📝 WalkthroughWalkthroughProvider-aware tools/schema gating was added, balanced-brace JSON extraction and jsonrepair-backed coercion were implemented, GenerationHandler and neurolink were extended to surface parsed ChangesJSON Validity for Structured Generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
Contributor
🤖 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 |
Contributor
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
This PR tightens structured-output gating so Vertex+Claude can use tools and JSON-schema enforcement together, and adds a provider-agnostic JSON “coercion” layer so generate({ schema }) returns syntactically valid JSON plus an exposed parsed structuredData object across providers.
Changes:
- Introduces a Gemini-only tools↔schema exclusion policy (and runtime conflict detection) to avoid disabling structured output for Vertex+Claude.
- Adds balanced-brace JSON extraction and
jsonrepair-backed coercion to canonical JSON for text-mode/provider override paths. - Threads
structuredDatathrough generation results and adds unit + live e2e test suites for JSON validity.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/continuous-test-suite-json.ts | Deterministic unit tests for policy predicate, extractor, and coercion behavior. |
| test/continuous-test-suite-json-e2e.ts | Live cross-provider matrix validating content JSON-parses and structuredData matches. |
| src/lib/utils/json/extract.ts | Replaces regex JSON extraction with quote/escape-aware balanced span scanning. |
| src/lib/utils/json/coerce.ts | Adds jsonrepair-backed text→canonical-JSON coercion helper. |
| src/lib/types/utilities.ts | Adds JsonCoercionResult type. |
| src/lib/types/generate.ts | Exposes structuredData?: unknown on GenerateResult and TextGenerationResult. |
| src/lib/neurolink.ts | Adds DTO-boundary coercion attempt + forwards structuredData to GenerateResult. |
| src/lib/core/modules/structuredOutputPolicy.ts | New Gemini-only structured output exclusion + conflict detector helpers. |
| src/lib/core/modules/GenerationHandler.ts | Uses new policy, retries on tools↔schema conflicts, and propagates/coerces structured data. |
| package.json | Adds jsonrepair dependency and new JSON test scripts. |
| pnpm-lock.yaml | Locks jsonrepair@3.14.0. |
| docs/superpowers/plans/2026-06-11-neurolink-json-validity.md | Adds an implementation plan document for the JSON validity work. |
| CLAUDE.md | Updates documented rules around Gemini-only tools↔schema limitation and JSON guarantees. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+86
to
+93
| const firstBrace = text.indexOf("{"); | ||
| const lastBrace = text.lastIndexOf("}"); | ||
| if (firstBrace >= 0 && lastBrace > firstBrace) { | ||
| candidates.push(text.slice(firstBrace, lastBrace + 1)); | ||
| } | ||
| if (firstBrace >= 0) { | ||
| candidates.push(text.slice(firstBrace)); | ||
| } |
Comment on lines
+4509
to
+4513
| // Provider-agnostic JSON guarantee: when a schema was requested, ensure | ||
| // `content` is valid JSON conforming to it and expose the parsed object as | ||
| // `structuredData`. Providers that already produced structuredData (AI-SDK | ||
| // experimental_output) pass through untouched; every other provider path — | ||
| // including those that override generate() (Vertex, Anthropic, Bedrock, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
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)
4471-4534:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmit
generation:end/response:endonly after schema coercion.Line 4481 and Line 4503 emit pre-coercion
textResult, but Line 4521+ mutatescontent/structuredData. Event consumers can observe stale/invalid JSON whilegenerate()returns the coerced value.Suggested fix
- if (!nativeAlreadyEmitted) { - this.emitter.emit("generation:end", { - provider: textResult.provider, - responseTime: Date.now() - startTime, - toolsUsed: textResult.toolsUsed, - timestamp: Date.now(), - result: textResult, - prompt: - originalPrompt || - options.input?.text || - (options as Record<string, unknown>).prompt, - temperature: textOptions.temperature, - maxTokens: textOptions.maxTokens, - pipelineAHandled: true, - }); - } - this.emitter.emit("response:end", textResult.content || ""); - this.emitter.emit( - "message", - `Generation completed in ${Date.now() - startTime}ms`, - ); - if ( textOptions.schema && textResult.structuredData === undefined && typeof textResult.content === "string" @@ } } + + if (!nativeAlreadyEmitted) { + this.emitter.emit("generation:end", { + provider: textResult.provider, + responseTime: Date.now() - startTime, + toolsUsed: textResult.toolsUsed, + timestamp: Date.now(), + result: textResult, + prompt: + originalPrompt || + options.input?.text || + (options as Record<string, unknown>).prompt, + temperature: textOptions.temperature, + maxTokens: textOptions.maxTokens, + pipelineAHandled: true, + }); + } + this.emitter.emit("response:end", textResult.content || ""); + this.emitter.emit( + "message", + `Generation completed in ${Date.now() - startTime}ms`, + );🤖 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/neurolink.ts` around lines 4471 - 4534, The events "generation:end" and "response:end" are emitted before the schema coercion mutates textResult.content/structuredData, causing consumers to see stale data; move the emission of this.emitter.emit("generation:end", ...) (respecting the nativeAlreadyEmitted guard and pipelineAHandled flag) and this.emitter.emit("response:end", ...) to after the coerceJsonToSchema(...) block and after textResult is updated so emitted payloads reflect the coerced content/structuredData returned in generateResult.
🧹 Nitpick comments (2)
docs/superpowers/plans/2026-06-11-neurolink-json-validity.md (1)
1086-1108: ⚡ Quick winSelf-review missed the type definition location violation.
The self-review checklist is thorough and covers spec coverage, placeholder scanning, and type consistency. However, it doesn't catch that
CoercionResult(mentioned in line 1099) violates coding guideline rule 2 by being defined locally incoerce.tsinstead of insrc/lib/types/utilities.ts.Consider adding a guidelines compliance check to the self-review section:
- All type definitions in
src/lib/types/per rule 2- All internal type imports from barrel per rule 13
- No
interfaceusage per rule 7🤖 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 `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md` around lines 1086 - 1108, CoercionResult is defined locally in coerce.ts which violates rule 2; move the CoercionResult type definition from coerce.ts into the shared types barrel (add it to src/lib/types/utilities.ts) and update coerce.ts to import CoercionResult from the barrel; also update any other files referencing the local type to import from src/lib/types/utilities.ts and ensure the barrel export is added so internal imports follow rule 13.test/continuous-test-suite-json-e2e.ts (1)
230-235: 💤 Low valueConsider breaking the long regex into multiple patterns for maintainability.
The comprehensive error-detection regex spans 230+ characters on a single line. While functionally correct, breaking it into an array of patterns or adding inline comments would improve readability and future maintenance.
♻️ Optional refactor for readability
function isInfraError(message: string): boolean { - return /api key|apikey|credential|security token|unauthor|permission|quota|rate.?limit|too many requests|429|not found|unknown model|model.*not|region|ENOTFOUND|ECONNREFUSED|ECONNRESET|socket hang|fetch failed|network|timeout|deadline|unavailable|overloaded|throttl|capacity|exhausted|billing|credits|insufficient|payment|402|access|forbidden|invalid.*model|does not exist|status [45]\d\d|bad request|internal server|service unavailable/i.test( - message, - ); + const patterns = [ + /api key|apikey|credential|security token|unauthor|permission/i, + /quota|rate.?limit|too many requests|429|throttl/i, + /not found|unknown model|model.*not|does not exist|invalid.*model/i, + /region|ENOTFOUND|ECONNREFUSED|ECONNRESET|socket hang|fetch failed|network/i, + /timeout|deadline|unavailable|overloaded|service unavailable/i, + /capacity|exhausted|billing|credits|insufficient|payment|402/i, + /access|forbidden|status [45]\d\d|bad request|internal server/i, + ]; + return patterns.some(pattern => pattern.test(message)); }🤖 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 `@test/continuous-test-suite-json-e2e.ts` around lines 230 - 235, The long regex in isInfraError makes maintenance hard; replace the single giant pattern with a collection of smaller, descriptive patterns (e.g., an array named INFRA_ERROR_PATTERNS) and update isInfraError to test the message against them (either by joining them into a single RegExp with the 'i' flag or by iterating and testing each pattern individually), keeping the original anchors/flags and ensuring matching behavior stays the same; include short descriptive comments for groups of related patterns (e.g., auth, rate limit, network, billing) so future edits are localized and readable.
🤖 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.
Inline comments:
In `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md`:
- Around line 764-989: The review notes that the code in
GenerationHandler.formatEnhancedResult uses the wrong type name for the result
of coerceJsonToSchema; update the usage to the correct exported type
JsonCoercionResult (instead of CoercionResult) returned by coerceJsonToSchema,
and ensure the utilities type export matches that name; locate references to
coerceJsonToSchema and the variable receiving its return in formatEnhancedResult
and change the type annotation/alias to JsonCoercionResult (or update the
utilities export to export the expected name) so the import and usage are
consistent with the type declared in utilities.
- Around line 620-726: Move the local type alias CoercionResult out of coerce.ts
into the shared utilities types module as JsonCoercionResult and export it; then
import and use JsonCoercionResult in coerceJsonToSchema (replace the local type
declaration and any references to CoercionResult), ensuring the exported type is
{ content: string; structuredData: unknown } so coerceJsonToSchema,
parseOrRepair, and related symbols continue to type-check against the new
JsonCoercionResult.
- Around line 35-51: The local type CoercionResult in coerceJsonToSchema should
be moved to a shared exported type named JsonCoercionResult in the project’s
utilities types module and imported instead of defining it locally; create and
export JsonCoercionResult in the types utilities module, add it to the barrel
export, then replace the local CoercionResult declaration with an import of
JsonCoercionResult in the coerceJsonToSchema implementation and update any
references (e.g., in coerceJsonToSchema and callers) to use the shared
JsonCoercionResult type.
---
Outside diff comments:
In `@src/lib/neurolink.ts`:
- Around line 4471-4534: The events "generation:end" and "response:end" are
emitted before the schema coercion mutates textResult.content/structuredData,
causing consumers to see stale data; move the emission of
this.emitter.emit("generation:end", ...) (respecting the nativeAlreadyEmitted
guard and pipelineAHandled flag) and this.emitter.emit("response:end", ...) to
after the coerceJsonToSchema(...) block and after textResult is updated so
emitted payloads reflect the coerced content/structuredData returned in
generateResult.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md`:
- Around line 1086-1108: CoercionResult is defined locally in coerce.ts which
violates rule 2; move the CoercionResult type definition from coerce.ts into the
shared types barrel (add it to src/lib/types/utilities.ts) and update coerce.ts
to import CoercionResult from the barrel; also update any other files
referencing the local type to import from src/lib/types/utilities.ts and ensure
the barrel export is added so internal imports follow rule 13.
In `@test/continuous-test-suite-json-e2e.ts`:
- Around line 230-235: The long regex in isInfraError makes maintenance hard;
replace the single giant pattern with a collection of smaller, descriptive
patterns (e.g., an array named INFRA_ERROR_PATTERNS) and update isInfraError to
test the message against them (either by joining them into a single RegExp with
the 'i' flag or by iterating and testing each pattern individually), keeping the
original anchors/flags and ensuring matching behavior stays the same; include
short descriptive comments for groups of related patterns (e.g., auth, rate
limit, network, billing) so future edits are localized and readable.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d69aa770-644b-4ef6-9068-9226dc89d0f2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
CLAUDE.mddocs/superpowers/plans/2026-06-11-neurolink-json-validity.mdpackage.jsonsrc/lib/core/modules/GenerationHandler.tssrc/lib/core/modules/structuredOutputPolicy.tssrc/lib/neurolink.tssrc/lib/types/generate.tssrc/lib/types/utilities.tssrc/lib/utils/json/coerce.tssrc/lib/utils/json/extract.tstest/continuous-test-suite-json-e2e.tstest/continuous-test-suite-json.ts
Contributor
🤖 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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/types/generate.ts (1)
271-301:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the Google tools+schema JSDoc.
These blocks still tell callers that Vertex schema requests must always set
disableTools: true, but this PR narrows the restriction to Gemini-only calls. That will now mislead Vertex+Claude consumers into disabling tools even though this change explicitly restores that combination.Based on learnings: "Public SDK API must not break existing callers."
Also applies to: 324-337
🤖 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/types/generate.ts` around lines 271 - 301, The JSDoc block above the generate/schema definitions incorrectly states Vertex requests must always set disableTools:true; update the comment to say the restriction applies only to Google Gemini (Gemini API) when combining function calling with structured output — callers using Vertex with Claude or other Vertex models can use schemas without disabling tools. Edit the Zod schema JSDoc (the block referencing "Google Gemini Limitation", "disableTools", and examples around the generate call and schema param) to mention Gemini-only limitation and adjust the example text accordingly so references to provider:"vertex" do not imply a universal disableTools requirement.Source: Learnings
🧹 Nitpick comments (1)
test/continuous-test-suite-json-e2e.ts (1)
62-68: ⚡ Quick winMove local type aliases to
src/lib/types/per repo TypeScript rules.
SchemaResult,SchemaCase, andCellare defined in this test file. As per coding guidelines:**/*.ts: “All type definitions must go insrc/lib/types/.”Also applies to: 153-166, 215-228
🤖 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 `@test/continuous-test-suite-json-e2e.ts` around lines 62 - 68, Extract the local type aliases SchemaResult, SchemaCase, and Cell from the test file into a shared types module and import them back into the test: create a types file exporting these interfaces/types (ensuring names and shapes match exactly), replace the inline definitions in the test with imports of SchemaResult, SchemaCase, and Cell, and update any references in the test to use the imported symbols; ensure the new types are exported from the module so other tests can reuse them and run the typechecker.Source: Coding guidelines
🤖 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.
Inline comments:
In `@src/lib/providers/anthropic.ts`:
- Around line 1471-1483: The current logic in generateTimeoutMs (using
params.max_tokens, getTimeoutForOptions, createTimeoutController) forces a
5‑minute floor and can silently override an explicit caller timeout; change it
so we only apply the 5‑minute floor when the caller did NOT provide an explicit
timeout or abortSignal in options. Concretely, detect options.timeout or
options.abortSignal and, if present, use getTimeoutForOptions(options) (and
respect the provided abortSignal) without applying Math.max(..., 300_000); only
compute Math.max(getTimeoutForOptions(options), 300_000) and create the
timeoutController when neither options.timeout nor options.abortSignal were
supplied.
In `@src/lib/providers/googleVertex.ts`:
- Around line 4018-4021: The computed finishReason (mapping lastStopReason ===
"max_tokens" ? "length" : "stop") is not being forwarded to downstream
callbacks/events; find where onFinish is invoked and where the "generation:end"
event is emitted (these currently hardcode "stop") and replace the hardcoded
string with the computed finishReason variable (or recompute the same expression
if out of scope) so truncation is visible to onFinish consumers and
generation:end listeners; ensure the same finishReason is used consistently in
the response object construction (finishReason) and in the payload passed to
onFinish and event emitters.
In `@src/lib/utils/json/coerce.ts`:
- Around line 94-104: The fallback candidate generation in
src/lib/utils/json/coerce.ts (used by coerceJsonToSchema) only scans for object
braces and therefore misses root arrays or truncated/wrapped arrays; update the
candidate extraction logic to also detect square-bracket pairs (firstBracket =
text.indexOf("["), lastBracket = text.lastIndexOf("]")) and push full and
truncated array slices as candidates (mirroring the existing brace-based
pushes), and apply the same array-aware addition to the subsequent fallback
block (the region around lines 106-149) so truncated/prose-wrapped arrays become
repair candidates too.
In `@test/continuous-test-suite-json-e2e.ts`:
- Around line 373-394: The test softens SDK guarantees by converting
structuredData parity to a log for non-strict (breadth) cells and by parsing
JSON only conditionally; instead, always assert SDK behavior for generate({
schema }): in the block using sdPresent, sdConsistent, cell.strictSchema,
replace the console.log branch with the same assertions used for strictSchema so
that res.structuredData is present and equals parsed for all cells, and ensure
the JSON parsing around parsed (lines ~520-527) is unconditional so parsed is
always available for comparison; use the identifiers res.structuredData, parsed,
sdPresent, sdConsistent, and cell.strictSchema to locate and update the checks.
- Around line 333-334: The current infra-skip regex (the large
/.../i.test(message) returned) is too broad—specifically it includes the
substring "bad request" and the generic "status [45]\d\d" which can mask real
SDK request/construction regressions; update that regex by removing "bad
request" and replacing the generic "status [45]\d\d" token with an explicit list
of infra-only status codes (e.g., 429, 502, 503, 504) so only known transient
infra errors are skipped, leaving true 4xx client errors (like 400/401/403/404)
to fail the tests; apply this change to the regex literal used in the return
statement shown (the /api key|apikey|...|context length/i.test(message)
expression).
---
Outside diff comments:
In `@src/lib/types/generate.ts`:
- Around line 271-301: The JSDoc block above the generate/schema definitions
incorrectly states Vertex requests must always set disableTools:true; update the
comment to say the restriction applies only to Google Gemini (Gemini API) when
combining function calling with structured output — callers using Vertex with
Claude or other Vertex models can use schemas without disabling tools. Edit the
Zod schema JSDoc (the block referencing "Google Gemini Limitation",
"disableTools", and examples around the generate call and schema param) to
mention Gemini-only limitation and adjust the example text accordingly so
references to provider:"vertex" do not imply a universal disableTools
requirement.
---
Nitpick comments:
In `@test/continuous-test-suite-json-e2e.ts`:
- Around line 62-68: Extract the local type aliases SchemaResult, SchemaCase,
and Cell from the test file into a shared types module and import them back into
the test: create a types file exporting these interfaces/types (ensuring names
and shapes match exactly), replace the inline definitions in the test with
imports of SchemaResult, SchemaCase, and Cell, and update any references in the
test to use the imported symbols; ensure the new types are exported from the
module so other tests can reuse them and run the typechecker.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45501e79-bf6d-4482-b3f4-8ce931195ba9
📒 Files selected for processing (12)
CLAUDE.mddocs/superpowers/plans/2026-06-11-neurolink-json-validity.mdsrc/lib/core/modules/GenerationHandler.tssrc/lib/neurolink.tssrc/lib/providers/anthropic.tssrc/lib/providers/googleVertex.tssrc/lib/types/generate.tssrc/lib/types/utilities.tssrc/lib/utils/json/coerce.tssrc/lib/utils/tokenLimits.tstest/continuous-test-suite-json-e2e.tstest/continuous-test-suite-json.ts
✅ Files skipped from review due to trivial changes (2)
- CLAUDE.md
- docs/superpowers/plans/2026-06-11-neurolink-json-validity.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/types/utilities.ts
- src/lib/neurolink.ts
Contributor
🤖 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 |
Contributor
Author
Review Feedback Addressed (Cycle 1)All 11 actionable findings from the automated reviewers (Copilot ×3, CodeRabbit ×8) have been addressed — 10 fixed, 1 declined with rationale below. Changes Made
Declined (with rationale)
Validation
Notes
@coderabbitai review |
|
🐇 ✨ ✅ Action performedReview finished.
|
Contributor
🤖 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.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md`:
- Around line 691-730: The coerceJsonToSchema function currently returns {
content, structuredData } but docs also expect repaired and truncated; reconcile
by updating the function signature and return to include optional
boolean/strings for repaired and truncated (e.g., return { content,
structuredData, repaired?: string|null, truncated?: boolean }) or alternatively
update the docs/Phase 2 description to match the existing shape—pick one
consistent contract; modify the function (coerceJsonToSchema) to populate
repaired when parseOrRepair changed the text and truncated when
nextBalancedJsonSpan cut content, and ensure the other documentation section is
updated to match the chosen shape.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5ae7a5de-5f6e-4024-b67e-4173e299e9d5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
CLAUDE.mddocs/superpowers/plans/2026-06-11-neurolink-json-validity.mdpackage.jsonsrc/lib/core/modules/GenerationHandler.tssrc/lib/core/modules/structuredOutputPolicy.tssrc/lib/neurolink.tssrc/lib/providers/anthropic.tssrc/lib/providers/googleVertex.tssrc/lib/types/generate.tssrc/lib/types/utilities.tssrc/lib/utils/json/coerce.tssrc/lib/utils/json/extract.tssrc/lib/utils/tokenLimits.tstest/continuous-test-suite-json-e2e.tstest/continuous-test-suite-json.ts
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (10)
- package.json
- src/lib/utils/json/extract.ts
- src/lib/utils/tokenLimits.ts
- src/lib/core/modules/structuredOutputPolicy.ts
- src/lib/types/generate.ts
- test/continuous-test-suite-json.ts
- src/lib/neurolink.ts
- src/lib/providers/anthropic.ts
- src/lib/core/modules/GenerationHandler.ts
- test/continuous-test-suite-json-e2e.ts
…text truncation
Two related JSON-validity fixes for generate({ schema }), entirely in the SDK.
1) Structured output was wrongly disabled for Vertex+Claude. The tools<->schema
exclusion is a Gemini-only API limitation, but the gate keyed on the whole
Vertex provider, forcing Vertex+Claude+tools (TARA's production config) into
text mode where hand-written JSON broke on a single mis-escaped character.
- structuredOutputPolicy.ts: exclusion gated on isGeminiProvider only;
isToolsSchemaConflictError detects runtime rejections (e.g. Groq) and
transparently retries without structured output.
- Provider-agnostic guarantee at the neurolink.ts boundary: content is always
valid JSON (balanced-brace scan + jsonrepair) and the parsed object is
exposed as result.structuredData -- override providers (vertex/anthropic/
bedrock/google-ai) bypass GenerationHandler, so the guarantee lives at the
SDK boundary.
2) Huge structured responses were silently truncated. The native Claude paths
hard-coded max_tokens to 4096; past ~16KB the JSON was cut mid-stream and
coercion closed it into a valid-but-incomplete object with no signal (the
Vertex native path didn't even surface finishReason).
- resolveClaudeMaxTokens(): model-aware output ceiling (Sonnet 4.x 64K, Opus
4.x 32K, older models at their published limits); clamps over-large caller
values so the native paths never 400.
- Vertex native generate surfaces finishReason (max_tokens -> "length").
- coerceJsonToSchema returns { repaired, truncated }; GenerateResult exposes
jsonRepaired / jsonTruncated plus a WARN -- truncation observable, never silent.
- Anthropic client sets an explicit timeout so the SDK's non-streaming
long-request guard doesn't reject a large max_tokens; generate timeout
scales when a large output budget is in play.
Verified live with tools active and complex Zod schemas (escaping torture,
nested, array-heavy, 200-line huge-output with no maxTokens) across Vertex
(Claude Sonnet/Opus 4.6 + Gemini 2.5), direct Anthropic (Sonnet/Opus 4.6),
Google AI Studio, OpenAI and breadth providers: 33 passed / 0 failed.
Huge outputs return complete valid JSON (16-24KB) where the old cap truncated;
forced truncation yields jsonTruncated=true + finishReason=length. Deterministic
unit suite covers the policy predicate, extractor, coercion and the new flags.
Contributor
🤖 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.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md`:
- Around line 19-29: Retitle the section header to indicate this is a pre-change
baseline and mark the listed bullets as historical context (e.g., prepend
"Pre-change baseline:"), and add a short note clarifying which specific facts
are outdated — namely the Vertex-wide gate behavior (GenerationHandler.ts
useStructuredOutput / isAnthropicProvider), the behavior of
formatEnhancedResult, the existence/handling of structuredData in
GenerateResult, and that jsonrepair is now a dependency — so readers won’t treat
the bullets (including options.schema and NoObjectGeneratedError mentions) as
current implementation facts.
In `@src/lib/utils/tokenLimits.ts`:
- Around line 158-161: The guard in the shown function treats requested===0 as
"not specified" (returns ceiling) which is inconsistent with getSafeMaxTokens
that preserves explicit 0; decide and make it explicit: either (A) preserve 0
like getSafeMaxTokens by removing the requested>0 check so requested===0 is
returned, or (B) treat 0 as invalid for Claude and keep the current behavior but
add an explicit check and comment (e.g., if (requested === 0) { /* Claude
requires max_tokens>0; treat 0 as unspecified */ } ) and add a short doc comment
to the function to document this choice; update the guard accordingly and ensure
getSafeMaxTokens and this function have consistent semantics and documentation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a62a25b-c6aa-4e10-8e77-b80eb78c60be
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
CLAUDE.mddocs/superpowers/plans/2026-06-11-neurolink-json-validity.mdpackage.jsonsrc/lib/core/modules/GenerationHandler.tssrc/lib/core/modules/structuredOutputPolicy.tssrc/lib/neurolink.tssrc/lib/providers/anthropic.tssrc/lib/providers/googleVertex.tssrc/lib/types/generate.tssrc/lib/types/utilities.tssrc/lib/utils/json/coerce.tssrc/lib/utils/json/extract.tssrc/lib/utils/tokenLimits.tstest/continuous-test-suite-json-e2e.tstest/continuous-test-suite-json.ts
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (11)
- src/lib/types/utilities.ts
- package.json
- src/lib/providers/googleVertex.ts
- src/lib/types/generate.ts
- src/lib/utils/json/coerce.ts
- test/continuous-test-suite-json.ts
- src/lib/neurolink.ts
- test/continuous-test-suite-json-e2e.ts
- src/lib/core/modules/structuredOutputPolicy.ts
- src/lib/providers/anthropic.ts
- src/lib/core/modules/GenerationHandler.ts
Comment on lines
+19
to
+29
| **Verified facts this plan relies on:** | ||
|
|
||
| - `GenerationHandler.ts` gate: `const useStructuredOutput = wantsStructuredOutput && !(isGoogleProvider && shouldUseTools && Object.keys(tools).length > 0);` where `isGoogleProvider = providerName === "google-ai" || providerName === "vertex"`. | ||
| - The file already defines (but does not use here) `isAnthropicProvider = ... || (providerName === "vertex" && modelName?.startsWith("claude-"))`. | ||
| - `formatEnhancedResult` sets `content = JSON.stringify(experimental_output)` when present, else strips fences from `generateResult.text` — and **discards** the parsed object. | ||
| - `NoObjectGeneratedError` fallback (re-runs without `experimental_output`) already exists → enabling structured output for Vertex+Claude is strictly safe. | ||
| - TARA runtime defaults: `neurolink-provider=vertex`, `neurolink-model=claude-sonnet-4-6`, tools registered (curator `registry.ts`). | ||
| - `GenerateResult` (src/lib/types/generate.ts) has no `structuredData` field; DTO builder in `neurolink.ts` (`const generateResult: GenerateResult = { content: textResult.content, ... }`) does not set one. | ||
| - `options.schema` type is `ValidationSchema = ZodTypeAny | Schema<unknown>` (Zod schema _or_ AI-SDK JSON schema). | ||
| - `jsonrepair` is NOT yet a dependency. | ||
| - Tests: `import { defineSuite, assert, assertEqual, assertNotNull } from "./helpers/harness.js"`, `const { test, runSuite } = defineSuite("…")`, run via `npx tsx test/<file>.ts`. `tsx` can import `src/**/*.ts` directly (fast TDD, no build). |
There was a problem hiding this comment.
Retitle this as pre-change baseline.
This block still says the old Vertex-wide gate is the verified fact and that jsonrepair is not yet a dependency, both of which are no longer true in the current implementation. That makes the section misleading; either refresh it or label it explicitly as historical baseline context.
🤖 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 `@docs/superpowers/plans/2026-06-11-neurolink-json-validity.md` around lines 19
- 29, Retitle the section header to indicate this is a pre-change baseline and
mark the listed bullets as historical context (e.g., prepend "Pre-change
baseline:"), and add a short note clarifying which specific facts are outdated —
namely the Vertex-wide gate behavior (GenerationHandler.ts useStructuredOutput /
isAnthropicProvider), the behavior of formatEnhancedResult, the
existence/handling of structuredData in GenerateResult, and that jsonrepair is
now a dependency — so readers won’t treat the bullets (including options.schema
and NoObjectGeneratedError mentions) as current implementation facts.
Comment on lines
+158
to
+161
| if (requested !== undefined && requested !== null && requested > 0) { | ||
| return Math.min(requested, ceiling); | ||
| } | ||
| return ceiling; |
There was a problem hiding this comment.
Clarify or align 0-handling with getSafeMaxTokens.
Line 158's guard requested > 0 treats 0 as "not specified" and returns the ceiling, whereas getSafeMaxTokens (lines 45–49) preserves explicit 0 when provided. This inconsistency could surprise callers. Since the Claude API requires max_tokens > 0, returning the ceiling is safer than forwarding 0, but consider documenting this behavior or validating 0 explicitly to make the intent clear.
📝 Suggested documentation addition
/**
* Resolve the `max_tokens` to send on a native Anthropic/Claude request: honour
* the caller's value but clamp it to the model's published ceiling, and default
- * to that ceiling when the caller did not specify one. Prevents both silent
- * truncation (the legacy 4096 default) and 400s from over-large requests.
+ * to that ceiling when the caller did not specify one (or passed 0 or a negative
+ * value, which are invalid for the Claude API). Prevents both silent truncation
+ * (the legacy 4096 default) and 400s from over-large requests.
*/
export function resolveClaudeMaxTokens(🤖 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/tokenLimits.ts` around lines 158 - 161, The guard in the shown
function treats requested===0 as "not specified" (returns ceiling) which is
inconsistent with getSafeMaxTokens that preserves explicit 0; decide and make it
explicit: either (A) preserve 0 like getSafeMaxTokens by removing the
requested>0 check so requested===0 is returned, or (B) treat 0 as invalid for
Claude and keep the current behavior but add an explicit check and comment
(e.g., if (requested === 0) { /* Claude requires max_tokens>0; treat 0 as
unspecified */ } ) and add a short doc comment to the function to document this
choice; update the guard accordingly and ensure getSafeMaxTokens and this
function have consistent semantics and documentation.
Contributor
|
🎉 This PR is included in version 9.70.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced Jun 25, 2026
Closed
Merged
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes two related JSON-validity problems in
generate({ schema }), entirely within the NeuroLink SDK (nothing on the curator side). Verified live across providers with tools active and complex Zod schemas.Commit 1 — structured output wrongly disabled for Vertex+Claude
TARA's production config is Vertex +
claude-sonnet-4-6+ tools, and NeuroLink was disabling structured-output enforcement for the entire Vertex provider whenever tools were active. But the tools↔schema conflict is a Gemini-only API limitation; Vertex+Claude supports both at once. The over-broad gate forced Vertex+Claude into text mode where the model hand-wrote JSON and a single mis-escaped character brokeJSON.parse.Fix:
structuredOutputPolicy.tsgates the exclusion onisGeminiProvider(Gemini only). A provider-agnostic coercion at theneurolink.tsSDK boundary guaranteescontentis valid JSON and exposes a parsedstructuredDataobject for every provider (override providers like Vertex/Anthropic/Bedrock bypassGenerationHandler, so the guarantee lives at the boundary). Runtime conflicts (e.g. Groq) are detected viaisToolsSchemaConflictErrorand retried without structured output.Commit 2 — huge text silently truncated mid-JSON
The dominant real-world failure for large TARA responses. The native Claude paths hard-coded
max_tokensto 4096, bypassing the 64K provider default. Past ~16 KB the JSON was cut mid-stream; the AI SDK skipsparseCompleteOutputonfinishReason="length", so the path fell to text-mode coercion, which closed the dangling JSON into a valid-but-incomplete object with no signal (the Vertex native path didn't even surfacefinishReason).Fix (all in NeuroLink):
resolveClaudeMaxTokens()— model-aware output ceiling (Sonnet 4.x → 64K, Opus 4.x → 32K, older models at their published limits); clamps over-large caller values so the native paths never 400. Applied at both Vertex+Claude and Anthropic native sites.finishReasonon the Vertex native generate path (Anthropicmax_tokens→"length").coerceJsonToSchemareturns{ repaired, truncated };GenerateResultexposesjsonRepaired/jsonTruncated(set onfinishReason="length"or an unclosed span) plus a WARN. No more silent data loss.timeoutso the SDK's "streaming is required for long requests" pre-flight guard doesn't reject a largemax_tokens, and scale the generate timeout when a large output budget is in play (the abort signal stays the real bound).Verification (live)
Matrix with tools active + complex schemas (escaping torture, deeply-nested, array-heavy, huge-output 200-line script with no
maxTokens) across Vertex (Claude Sonnet/Opus 4.6 + Gemini 2.5), direct Anthropic (Sonnet/Opus 4.6), Google AI Studio, OpenAI, plus breadth providers, plus dedicated complete-output and forced-truncation-is-observable tests.jsonTruncated=true, finishReason=length(observable, never silent).repaired/truncatedflags.tscclean · ESLint clean.Guarantee vs. limitation (honest)
generate({ schema }),contentis valid JSON andstructuredDatais the parsed object — across every provider.gpt-4o-mini16384) and 400s when a caller omitsmaxTokens— a pre-existing non-Claude issue for a separate follow-up.Summary by CodeRabbit
New Features
Behavior / Fixes
Tests
Documentation
Chores