Skip to content

fix(structured-output): disable structured output with tools on native anthropic provider - #1116

Closed
murdore wants to merge 3 commits into
releasefrom
fix/anthropic-tools-schema
Closed

murdore wants to merge 3 commits into
releasefrom
fix/anthropic-tools-schema

Conversation

@murdore

@murdore murdore commented Jun 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem

experimental_output (AI-SDK JSON-schema enforcement) combined with tools silently drops tool calls on the native Anthropic Messages API surface (provider anthropic/bedrock, including via a proxy / ANTHROPIC_BASE_URL override).

Symptoms observed on an anthropic-via-proxy deployment (Tara):

  • The model returns finishReason=tool-calls but zero tool calls are parsed (has_tools=false), so the agent loop ends at step 1.
  • Only the assistant's preamble text is returned → the model fabricates tool-success ("flag set successfully") while the tool never ran.
  • Structured-output recovery then retry-storms (8–23 calls per turn).

Root cause

isToolsSchemaExclusionInForce() disables structured-output-with-tools only for Gemini; its comment assumed "Anthropic Claude … supports tools and structured output simultaneously." That holds for Vertex+Claude (different transport) but not for the native Anthropic surface.

Fix

Extend the static gate to also exclude the native anthropic surface (anthropic/bedrock) when tools are active, via a new isNativeAnthropicProvider() — mirroring the existing Gemini fallback. Vertex+Claude is intentionally left untouched (provider vertex, claude- model), preserving strict structured output for the primary production config.

Verification

End-to-end on an anthropic-via-proxy deployment with the equivalent change applied:

  • Before: tool never executes; fabricated reply; retry storm.
  • After: tool executes (real return values — e.g. reporting the previous flag value), clean 2-step tool loop, no retry storm.

Downstream validateOrRecoverStructured() recovers the structured shape from text mode, so there is no regression to the response format.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility for structured output when tools are enabled with native Anthropic providers on Bedrock, while keeping Vertex + Claude supported.
    • Added automatic recovery for requests that fail due to deprecated/unsupported temperature by retrying without the temperature setting.
    • Prevented temperature from being sent for Anthropic models that deprecate it.
  • New Features
    • Enhanced Anthropic multimodal prompt conversion for file/image parts (including PDF and media-type detection).
  • Tests
    • Added continuous test coverage for native Anthropic tools policy, temperature deprecation detection, and multimodal conversion.

…e anthropic provider

experimental_output (JSON-schema enforcement) + tools silently drops tool_use blocks on the native Anthropic Messages API surface (provider 'anthropic'/'bedrock', incl. via a proxy/base-URL override): the model returns finishReason=tool-calls but zero tool calls are parsed, so the agent loop ends at step 1 and only the preamble text is returned — surfacing as fabricated tool-success replies and structured-output retry storms.

The static gate previously excluded only Gemini; its comment assumed Anthropic supports both. Extend isToolsSchemaExclusionInForce() to also exclude the native anthropic surface via isNativeAnthropicProvider(), mirroring the Gemini fallback. Vertex+Claude (provider 'vertex', claude- model) is intentionally NOT matched — different transport, no conflict — so the primary production config keeps strict structured output.

Verified end-to-end against an anthropic-via-proxy deployment: tool calls now execute (clean 2-step tool loop, real tool return values) instead of being dropped.
Copilot AI review requested due to automatic review settings June 25, 2026 11:51
@vercel

vercel Bot commented Jun 25, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
neurolink Ready Ready Preview, Comment Jun 25, 2026 6:11pm

@coderabbitai

coderabbitai Bot commented Jun 25, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Structured output policy now covers native Anthropic tool usage and temperature-deprecation detection. GenerationHandler retries once without temperature on matching failures. Anthropic provider conversion now handles file parts and shared image/PDF block helpers, with new continuous tests for both policy and multimodal conversion.

Changes

Structured output, Anthropic conversion, and retry

Layer / File(s) Summary
Provider surface detection
src/lib/core/modules/structuredOutputPolicy.ts
Adds native Anthropic provider detection, extends the tools exclusion rule to Gemini and native Anthropic surfaces, and updates the surrounding policy comments.
Temperature retry path
src/lib/core/modules/GenerationHandler.ts, src/lib/core/modules/structuredOutputPolicy.ts
Adds a temperature-deprecation error predicate and uses it in executeGeneration to retry once without temperature while recording retry telemetry and finish reason data.
Anthropic multimodal block helpers
src/lib/providers/anthropicImageBlocks.ts, src/lib/providers/anthropic.ts
Adds shared image/document conversion helpers, routes AI-SDK file parts through them, and gates Anthropic temperature parameters for models that deprecate them.
Policy and multimodal tests
test/continuous-test-suite-anthropic-tools-policy.ts, test/continuous-test-suite-anthropic-multimodal.ts
Covers native Anthropic provider detection, tool-gated exclusion behavior, Gemini behavior, temperature-deprecation matching, and image/PDF/file conversion cases.

Sequence Diagram(s)

sequenceDiagram
  participant GenerationHandler
  participant structuredOutputPolicy
  participant callGenerateText
  GenerationHandler->>structuredOutputPolicy: isTemperatureDeprecatedError(error)
  structuredOutputPolicy-->>GenerationHandler: true
  GenerationHandler->>callGenerateText: retry with temperature undefined
  callGenerateText-->>GenerationHandler: retry result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • Tara-ag

Possibly related PRs

  • juspay/neurolink#1080: Extends structured-output policy behavior and related retry handling in a closely overlapping area of the codebase.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: disabling structured output with tools on the native Anthropic provider.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/anthropic-tools-schema

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Single Commit Policy - COMPLIANT

Status: Policy requirements met • 1 commit • Valid format • Ready for merge

📊 View validation details

📝 Commit Details

  • Hash: 7ce02f3674c0c24ec033e7428744446c5cf1d83a
  • Message: fix(structured-output): disable structured output with tools on native anthropic provider
  • Author: Sachin Sharma

✅ Validation Results

  • Single commit requirement met
  • No merge commits in branch
  • Semantic commit message format verified
  • Ready for squash merge to release branch

🤖 Automated validation by NeuroLink Single Commit Enforcement

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 AI Review & Build Compliance ✅

Status: AI analysis complete • Build rules validated • Ready for review

📊 View detailed analysis results

🛡️ Analysis Complete

  • ✅ Security scan (vulnerabilities, API keys)
  • ✅ TypeScript safety & code quality
  • ✅ Error handling & best practices
  • ✅ Build rule enforcement validated
  • ✅ Commit format & compliance checks

📋 Ready for Merge When

  • All CI checks passing
  • Manual review approved
  • Any AI-flagged issues resolved

🤖 AI analysis complete - check individual code comments for specific feedback

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/lib/core/modules/structuredOutputPolicy.ts`:
- Around line 58-61: The `isToolsSchemaExclusionInForce` test coverage needs to
be extended for the new native provider branches. Update
`test/continuous-test-suite-json.ts` by adding assertions alongside the existing
Gemini/tools test to verify that `isToolsSchemaExclusionInForce("anthropic",
"claude-3-sonnet", true, 5)` and `isToolsSchemaExclusionInForce("bedrock",
"anthropic.claude-3-sonnet", true, 5)` both return `true`. Use the existing test
block around `isToolsSchemaExclusionInForce` as the insertion point and keep the
assertions consistent with the current `assertEqual` style.
🪄 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: d16c2c4f-7d3c-4806-9dae-d41b056d1de5

📥 Commits

Reviewing files that changed from the base of the PR and between 8f0fda3 and 7ce02f3.

📒 Files selected for processing (1)
  • src/lib/core/modules/structuredOutputPolicy.ts

Comment on lines +58 to +61
(isGeminiProvider(providerName, modelName) ||
isNativeAnthropicProvider(providerName)) &&
shouldUseTools &&
toolCount > 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify that structured-output exclusion tests cover the new native Anthropic surfaces.
# Expect: assertions for anthropic+tools and bedrock+tools returning true, plus vertex+claude+tools returning false.

rg -n -C 3 'isToolsSchemaExclusionInForce\("(anthropic|bedrock|vertex)"' test/continuous-test-suite-json.ts

Repository: juspay/neurolink

Length of output: 655


🏁 Script executed:

#!/bin/bash
# Check for any existing tests involving "anthropic" or "bedrock" in the test file
rg -n "(anthropic|bedrock)" test/continuous-test-suite-json.ts --type ts

Repository: juspay/neurolink

Length of output: 324


Add assertions for native Anthropic and Bedrock to isToolsSchemaExclusionInForce tests.

The current test suite in test/continuous-test-suite-json.ts lacks coverage for the new provider-specific logic. You must add assertions verifying that:

  1. isToolsSchemaExclusionInForce("anthropic", "claude-3-sonnet", true, 5) returns true.
  2. isToolsSchemaExclusionInForce("bedrock", "anthropic.claude-3-sonnet", true, 5) returns true.
<details>
<summary>Current test coverage</summary>
```typescript
// Existing test (Line 58-61)
await test("exclusion fires only for Gemini + tools present", () => {
  // Verifies Vertex+Claude = false, Vertex+Gemini = true
  assertEqual(isToolsSchemaExclusionInForce("vertex", "claude-sonnet-4-6", true, 5), false);
  assertEqual(isToolsSchemaExclusionInForce("vertex", "gemini-2.5-pro", true, 5), true);
});
```

Add similar blocks for the native providers to prevent regression of the new exclusion logic.

🤖 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/core/modules/structuredOutputPolicy.ts` around lines 58 - 61, The
`isToolsSchemaExclusionInForce` test coverage needs to be extended for the new
native provider branches. Update `test/continuous-test-suite-json.ts` by adding
assertions alongside the existing Gemini/tools test to verify that
`isToolsSchemaExclusionInForce("anthropic", "claude-3-sonnet", true, 5)` and
`isToolsSchemaExclusionInForce("bedrock", "anthropic.claude-3-sonnet", true, 5)`
both return `true`. Use the existing test block around
`isToolsSchemaExclusionInForce` as the insertion point and keep the assertions
consistent with the current `assertEqual` style.

@Tara-ag Tara-ag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved

This PR correctly fixes a critical bug where experimental_output combined with tools silently drops tool calls on the native Anthropic Messages API surface.

Review Summary

Files Changed: 1 (src/lib/core/modules/structuredOutputPolicy.ts)
Additions: 28 lines
Deletions: 7 lines

Key Observations

  1. Correct Fix: The change extends the tools↔schema exclusion to native Anthropic providers (anthropic/bedrock) while intentionally preserving Vertex+Claude behavior. This aligns with CLAUDE.md Critical Rule 3's pattern.

  2. Backward Compatible: The change is purely additive - it adds new providers to the exclusion list without affecting existing behavior for other providers. Downstream validateOrRecoverStructured() ensures no regression in response format.

  3. Well-Documented: Comments clearly explain the distinction between:

    • Gemini (has conflict)
    • Native Anthropic/Bedrock (has conflict)
    • Vertex+Claude (no conflict - different transport)
  4. Clean Implementation: The new isNativeAnthropicProvider() function follows the same simple predicate pattern as isGeminiProvider().

Suggestion (Non-blocking)

Consider adding test cases to test/continuous-test-suite-json.ts to verify:

  • isToolsSchemaExclusionInForce("anthropic", "claude-sonnet-4-6", true, 5) returns true
  • isToolsSchemaExclusionInForce("bedrock", "anthropic.claude-sonnet-4-6", true, 5) returns true
  • isToolsSchemaExclusionInForce("anthropic", "claude-sonnet-4-6", false, 0) returns false (no tools)

This ensures the new behavior is covered by the continuous test suite.

Overall: Clean, focused fix that addresses the root cause without introducing risks. Approved for merge.

@murdore

murdore commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Verification update — why disabling (not a different mode) is the right call

Confirmed the conflict is with Anthropic's native structured outputs (output_format), not the jsonTool fallback. getModelCapabilities() in @ai-sdk/anthropic@3.0.64 already returns supportsStructuredOutput: true for claude-{sonnet,haiku}-4-5 / 4-6, so structuredOutputMode: "auto" resolves to outputFormat and no json tool is created. Native output_format constrains the response to be the schema JSON, which is mutually exclusive with the tool_use loop — the model emits the structured object on turn 1 (fabricating any tool actions) and never calls tools. Forcing outputFormat therefore changes nothing; it's already what runs.

End-to-end on an anthropic-via-proxy deployment with this patch, a single turn:

  • executes a real tool (tara_set_feature_flag, returning the actual previous value), and
  • yields a schema-validated structured response — [STRUCTURED_GENERATE] Schema validation passed.

So disabling AI-SDK enforcement does not lose structured output: the consumer's downstream text→schema recovery reconstructs and validates it, exactly as already happens on Gemini. Tools + structured result both work.

The newest Anthropic models (e.g. claude-opus-4-8 with tools + advanced beta features) reject temperature with HTTP 400 ('temperature is deprecated for this model.') in favour of reasoning-effort controls. NeuroLink sent it unconditionally, so the whole turn failed ('All providers failed').

Add isTemperatureDeprecatedError() and, in GenerationHandler's catch, retry the generation once with temperature omitted (mirrors the existing isToolsSchemaConflictError structured-output fallback). Add a deterministic test suite covering the native-anthropic structured-output gate + the temperature detector.
@github-actions

Copy link
Copy Markdown
Contributor

🤖 AI Review & Build Compliance ✅

Status: AI analysis complete • Build rules validated • Ready for review

📊 View detailed analysis results

🛡️ Analysis Complete

  • ✅ Security scan (vulnerabilities, API keys)
  • ✅ TypeScript safety & code quality
  • ✅ Error handling & best practices
  • ✅ Build rule enforcement validated
  • ✅ Commit format & compliance checks

📋 Ready for Merge When

  • All CI checks passing
  • Manual review approved
  • Any AI-flagged issues resolved

🤖 AI analysis complete - check individual code comments for specific feedback

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/lib/core/modules/GenerationHandler.ts`:
- Around line 577-585: The temperature-retry return path in GenerationHandler is
missing the same span telemetry recorded in the primary and structured-output
fallback paths. Before returning the result in the retry branch, use the same
usage/cost attributes as the other paths by setting gen_ai.usage.input_tokens,
gen_ai.usage.output_tokens, and neurolink.cost on the span alongside the
existing finish_reason and retry.count updates. Refer to the result handling
inside GenerationHandler so the retry path mirrors the telemetry behavior of the
other branches.
🪄 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: 47168301-076e-4736-b32d-85ba498a9fd5

📥 Commits

Reviewing files that changed from the base of the PR and between 7ce02f3 and 077ccc0.

📒 Files selected for processing (3)
  • src/lib/core/modules/GenerationHandler.ts
  • src/lib/core/modules/structuredOutputPolicy.ts
  • test/continuous-test-suite-anthropic-tools-policy.ts

Comment on lines +577 to +585
span.setAttribute("retry.count", 1);
if (result.finishReason) {
span.setAttribute(
"gen_ai.response.finish_reason",
result.finishReason,
);
}
span.setStatus({ code: SpanStatusCode.OK });
return result;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Temperature-retry path drops usage/cost span telemetry.

The primary (Lines 404-422) and structured-output fallback (Lines 502-523) paths both set gen_ai.usage.input_tokens, gen_ai.usage.output_tokens, and neurolink.cost on the span. This retry path sets only finish_reason, so traces recovered via the temperature retry report a cost of 0 and no token usage, breaking the "what did this trace cost?" query for that path.

📊 Proposed fix to record usage/cost before returning
             span.addEvent("retry.recovered", {
               "retry.attempts": 2,
               "retry.strategy": "temperature_omitted",
             });
             span.setAttribute("retry.count", 1);
+            if (result.usage) {
+              span.setAttribute(
+                "gen_ai.usage.input_tokens",
+                result.usage.inputTokens || 0,
+              );
+              span.setAttribute(
+                "gen_ai.usage.output_tokens",
+                result.usage.outputTokens || 0,
+              );
+              const retryCost = calculateCost(
+                this.providerName,
+                this.modelName,
+                {
+                  input: result.usage.inputTokens || 0,
+                  output: result.usage.outputTokens || 0,
+                  total:
+                    (result.usage.inputTokens || 0) +
+                    (result.usage.outputTokens || 0),
+                },
+              );
+              span.setAttribute("neurolink.cost", retryCost ?? 0);
+            }
             if (result.finishReason) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
span.setAttribute("retry.count", 1);
if (result.finishReason) {
span.setAttribute(
"gen_ai.response.finish_reason",
result.finishReason,
);
}
span.setStatus({ code: SpanStatusCode.OK });
return result;
span.setAttribute("retry.count", 1);
if (result.usage) {
span.setAttribute(
"gen_ai.usage.input_tokens",
result.usage.inputTokens || 0,
);
span.setAttribute(
"gen_ai.usage.output_tokens",
result.usage.outputTokens || 0,
);
const retryCost = calculateCost(
this.providerName,
this.modelName,
{
input: result.usage.inputTokens || 0,
output: result.usage.outputTokens || 0,
total:
(result.usage.inputTokens || 0) +
(result.usage.outputTokens || 0),
},
);
span.setAttribute("neurolink.cost", retryCost ?? 0);
}
if (result.finishReason) {
span.setAttribute(
"gen_ai.response.finish_reason",
result.finishReason,
);
}
span.setStatus({ code: SpanStatusCode.OK });
return result;
🤖 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/core/modules/GenerationHandler.ts` around lines 577 - 585, The
temperature-retry return path in GenerationHandler is missing the same span
telemetry recorded in the primary and structured-output fallback paths. Before
returning the result in the retry branch, use the same usage/cost attributes as
the other paths by setting gen_ai.usage.input_tokens,
gen_ai.usage.output_tokens, and neurolink.cost on the span alongside the
existing finish_reason and retry.count updates. Refer to the result handling
inside GenerationHandler so the retry path mirrors the telemetry behavior of the
other branches.

@Tara-ag Tara-ag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Files reviewed: 3
New issues raised: 1 (MINOR)
Existing comments acknowledged: 1 (CodeRabbit's test coverage comment - addressed by the new test file)

Assessment

This PR correctly fixes a critical production issue where the native Anthropic Messages API surface (including via proxy) silently drops tool calls when experimental_output (structured output) is combined with tools. The fix properly distinguishes between:

  • Native Anthropic (anthropic/bedrock): Disables structured output when tools are active ✅
  • Vertex+Claude (vertex with claude-* models): Preserves structured output with tools ✅

Changes Analysis

File Assessment
structuredOutputPolicy.ts Clean implementation of isNativeAnthropicProvider() with clear documentation explaining the transport difference. Correctly extends isToolsSchemaExclusionInForce() without breaking existing Gemini logic.
GenerationHandler.ts Temperature deprecation retry follows established fallback patterns. Proper telemetry attribution. No infinite loop risk (single retry only).
continuous-test-suite-anthropic-tools-policy.ts Comprehensive test coverage for all new logic paths. Well-structured using the canonical harness.

Issue Raised

  • MINOR: New test file not registered in package.json scripts. Consider adding "test:anthropic-tools-policy": "npx tsx test/continuous-test-suite-anthropic-tools-policy.ts" and including it in test:unit since it's a pure logic test.

Compliance Check

  • ✅ CLAUDE.md Rule 3 (Gemini tools + JSON schema mutual exclusion) - correctly extended to native Anthropic
  • ✅ Backward compatibility preserved - Vertex+Claude unaffected
  • ✅ Error handling follows existing patterns
  • ✅ Tests included for new behavior

Recommendation: Approve after addressing the minor package.json registration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 MINOR: Test file not registered in package.json scripts

This new test suite is comprehensive and well-structured, but it's not registered in package.json scripts. Per CONTRIBUTING.md:

"Or create a new suite test/continuous-test-suite-<name>.ts and add a matching test:<name> script in package.json"

Suggested addition to package.json scripts:

"test:anthropic-tools-policy": "npx tsx test/continuous-test-suite-anthropic-tools-policy.ts",

Since this is a pure logic test (no API calls), also consider adding it to test:unit:

"test:unit": "pnpm run test:envguard && pnpm run test:bugfixes && ... && pnpm run test:anthropic-tools-policy",

Without registration, these tests won't run in CI and could regress unnoticed.

…mperature proactively

AI-SDK v6 encodes images AND PDFs as `type:"file"` parts in the LanguageModel
prompt that the provider's doGenerate receives. messagesToAnthropic only handled
text/image/image_url, so on the tool-using generate path the image was silently
dropped and the model answered "no image detected". Vertex/Gemini are immune
(they read input.images directly in their own builders).

- Add anthropicImageBlocks.ts: pure file/image -> Anthropic block converters.
  Image media type is taken from the AI-SDK mediaType, then sniffed from magic
  bytes, then defaulted (a hardcoded image/png corrupts JPEG/GIF/WebP -> HTTP 400).
  PDF file parts become document blocks.
- messagesToAnthropic: handle the `type:"file"` content part.
- modelDeprecatesTemperature(): omit `temperature` proactively for opus-4-8+
  (which reject it) instead of relying on the fail-then-retry safety net — the
  guaranteed-to-fail first request was a wasted round-trip on every turn.
- Deterministic suites for both (no API).
@github-actions

Copy link
Copy Markdown
Contributor

🤖 AI Review & Build Compliance ✅

Status: AI analysis complete • Build rules validated • Ready for review

📊 View detailed analysis results

🛡️ Analysis Complete

  • ✅ Security scan (vulnerabilities, API keys)
  • ✅ TypeScript safety & code quality
  • ✅ Error handling & best practices
  • ✅ Build rule enforcement validated
  • ✅ Commit format & compliance checks

📋 Ready for Merge When

  • All CI checks passing
  • Manual review approved
  • Any AI-flagged issues resolved

🤖 AI analysis complete - check individual code comments for specific feedback

@Tara-ag Tara-ag left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Files reviewed: 6 changed files
New issues raised: 2 (both MINOR)
Blocking issues: 0

Assessment

This PR effectively addresses a critical production issue where experimental_output combined with tools silently drops tool calls on the native Anthropic Messages API surface. The fix is well-architected:

  1. Correctly extends the structured output exclusion to native Anthropic providers (anthropic/bedrock) while intentionally preserving Vertex+Claude behavior
  2. Adds temperature deprecation handling for newer Anthropic models (claude-opus-4-8+) with proactive omission and reactive retry
  3. Fixes multimodal image handling for AI-SDK v6's type:"file" encoding that was dropping images on the tool-using generate path
  4. Includes comprehensive test coverage for both the policy logic and multimodal conversion

New Comments Raised

  • 💡 MINOR: Type assertion safety suggestion in anthropic.ts for file part handling
  • 💡 MINOR: Test file registration in package.json scripts (CONTRIBUTING.md compliance)

Already Addressed by Existing Comments

  • Test coverage extension for native Anthropic/Bedrock assertions (CodeRabbit)
  • Temperature-retry telemetry parity (CodeRabbit)
  • Package.json script registration for multimodal test (Tara-ag)

Architectural Verification

✅ Static import from structuredOutputPolicy.ts to anthropic.ts is acceptable - this is a utility module, not a provider (CLAUDE.md Critical Rule 1 applies to provider-to-provider imports only)
✅ No breaking changes to public SDK API
✅ No security vulnerabilities identified
✅ Proper error handling and telemetry throughout

Recommendation: Approve with minor suggestions. The core fix is solid and addresses a real production pain point.

const cc = cacheControlOf(p);
blocks.push(cc ? { ...img, cache_control: cc } : img);
}
} else if (p?.type === "file") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 MINOR: Type assertion safety for file part handling

The type assertion p as { mediaType?: string; data?: unknown } on line 449 assumes the part object has this shape, but p originates from message parsing and may not fully match at runtime. Consider adding a runtime validation check:

} else if (p?.type === "file") {
  // Runtime validation before type assertion
  const filePart = p as { mediaType?: unknown; data?: unknown };
  if (typeof filePart.mediaType === "string" || filePart.mediaType === undefined) {
    const block = fileToAnthropicBlock({
      mediaType: filePart.mediaType,
      data: filePart.data,
    });
    // ... rest of handling
  }
}

This is a minor defensive coding suggestion - the current implementation is functional given the controlled input sources.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 MINOR: Test file not registered in package.json scripts

Per CONTRIBUTING.md, new test suites should be registered in package.json scripts. Consider adding:

"test:anthropic-tools-policy": "npx tsx test/continuous-test-suite-anthropic-tools-policy.ts",
"test:anthropic-multimodal": "npx tsx test/continuous-test-suite-anthropic-multimodal.ts",

And optionally include them in test:unit since they're pure logic tests (no API calls):

"test:unit": "pnpm run test:envguard && pnpm run test:bugfixes && ... && pnpm run test:anthropic-tools-policy && pnpm run test:anthropic-multimodal",

Without registration, these tests won't run in CI and could regress unnoticed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/lib/providers/anthropicImageBlocks.ts`:
- Around line 232-234: The `anthropicImageBlocks` flow is passing every
`image/*` media type into `toAnthropicImageBlock()`, which causes unsupported
hints like `image/svg+xml` or `image/bmp` to be mislabeled as PNG instead of
being omitted. Update the `mediaType.startsWith("image/")` branch to only call
`toAnthropicImageBlock()` for supported image types and return `undefined` for
unsupported ones, so the function’s behavior matches its contract. Use
`toAnthropicImageBlock` and the `mediaType.startsWith("image/")` check as the
main places to adjust.
🪄 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: a96d9efd-459f-42bc-9872-c52fd44110d1

📥 Commits

Reviewing files that changed from the base of the PR and between 077ccc0 and 5fc6374.

📒 Files selected for processing (5)
  • src/lib/core/modules/structuredOutputPolicy.ts
  • src/lib/providers/anthropic.ts
  • src/lib/providers/anthropicImageBlocks.ts
  • test/continuous-test-suite-anthropic-multimodal.ts
  • test/continuous-test-suite-anthropic-tools-policy.ts

Comment on lines +232 to +234
if (mediaType.startsWith("image/")) {
return toAnthropicImageBlock(data, mediaType);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Skip unsupported image/* hints instead of relabeling them as PNG.

Lines 232-234 currently send every image/* file part into toAnthropicImageBlock(). For unsupported hints like image/svg+xml or image/bmp, that helper falls through to the "image/png" default and emits a bad Anthropic block instead of returning undefined, so the request fails even though this function promises unsupported types are omitted.

Suggested fix
   if (mediaType.startsWith("image/")) {
+    if (!normalizeImageMediaType(mediaType)) {
+      return undefined;
+    }
     return toAnthropicImageBlock(data, mediaType);
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (mediaType.startsWith("image/")) {
return toAnthropicImageBlock(data, mediaType);
}
if (mediaType.startsWith("image/")) {
if (!normalizeImageMediaType(mediaType)) {
return undefined;
}
return toAnthropicImageBlock(data, mediaType);
}
🤖 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/providers/anthropicImageBlocks.ts` around lines 232 - 234, The
`anthropicImageBlocks` flow is passing every `image/*` media type into
`toAnthropicImageBlock()`, which causes unsupported hints like `image/svg+xml`
or `image/bmp` to be mislabeled as PNG instead of being omitted. Update the
`mediaType.startsWith("image/")` branch to only call `toAnthropicImageBlock()`
for supported image types and return `undefined` for unsupported ones, so the
function’s behavior matches its contract. Use `toAnthropicImageBlock` and the
`mediaType.startsWith("image/")` check as the main places to adjust.

@murdore

murdore commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1117, which consolidates these fixes (structured-output, temperature, vision, exceljs, mammoth) plus the OAuth-proxy relocation + tracing fixes into a single commit per the single-commit policy.

All review comments from this PR are addressed in #1117:

  • anthropicImageBlocks — unsupported image/* hints (svg/bmp) now fall through to magic-byte salvage instead of being relabeled as PNG.
  • structuredOutputPolicy — added isToolsSchemaExclusionInForce assertions for native anthropic and bedrock (policy suite 9/9 pass).
  • GenerationHandler:585 — temperature-retry span now records gen_ai.usage.* + neurolink.cost.
  • anthropic.ts:449 — runtime guard added before the file-part type assertion.
  • package.json — registered test:anthropic-tools-policy, test:anthropic-multimodal, test:excel-interop and added them to test:unit.

Closing in favour of #1117.

@murdore murdore closed this Jun 26, 2026

This branch was successfully deployed

1 active deployment
Preview — 5fc6374d Deployed Jun 25, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants