Conversation
|
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Anthropic beta-header prefixes and a ContextManagement feature flag; reorders MergeBetaHeaders to take ctx first; changes raw-body stripping to gate the entire Changes
sequenceDiagram
participant Client
participant Bifrost as Bifrost HTTP Transport
participant Adapter as Vertex Adapter
participant AnthropicUtil as anthropic.utils
participant VertexAI as Vertex API
Client->>Bifrost: Send Anthropic-style request (headers + body)
Bifrost->>Adapter: Forward request (passthrough safe headers only)
Adapter->>Adapter: Build Anthropic request body (marshal/remap)
Adapter->>AnthropicUtil: MergeBetaHeaders(ctx, providerExtra)
Adapter->>AnthropicUtil: StripUnsupportedFieldsFromRawBody(rawBody, schemas.Vertex, deployment)
alt strip succeeds
AnthropicUtil-->>Adapter: cleanedRawBody
Adapter->>VertexAI: Send request with cleaned body and filtered betas
else strip fails
AnthropicUtil-->>Adapter: error
Adapter-->>Bifrost: Abort with wrapped BifrostOperationError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
18 tasks
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
18 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
transports/bifrost-http/integrations/anthropic.go (1)
207-207: Keep the unsupported-beta comment exhaustive (or explicitly non-exhaustive).Line 207 omits prefixes already filtered in this function (
skills-,fast-mode-,redact-thinking-on Lines 237-239), which can cause maintenance drift.Suggested comment tweak
-// Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, mcp-client, context-management. +// Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, +// mcp-client, skills, fast-mode, redact-thinking, context-management.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` at line 207, Update the "unsupported-beta" comment in transports/bifrost-http/integrations/anthropic.go to either list all currently filtered prefixes or explicitly mark the list as non-exhaustive; include the prefixes that are filtered later in this function ("skills-", "fast-mode-", "redact-thinking-") so the comment stays accurate with the actual filtering logic (refer to the filtering code that checks those prefixes) and avoid future maintenance drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/vertex/vertex.go`:
- Around line 399-403: The streaming Anthropic branch in ChatCompletionStream is
not sanitizing the marshaled rawBody like the unary path does; update the
Anthropic streaming path to call
anthropic.StripUnsupportedFieldsFromRawBody(rawBody, schemas.Vertex,
request.Model) before sending or marshaling the payload (same as the unary chat
block), handle the returned rawBody and error (returning a wrapped fmt.Errorf on
error), and ensure you reference the same symbols used in the unary fix:
rawBody, anthropic.StripUnsupportedFieldsFromRawBody, schemas.Vertex,
request.Model, and the ChatCompletionStream Anthropic branch.
---
Nitpick comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Line 207: Update the "unsupported-beta" comment in
transports/bifrost-http/integrations/anthropic.go to either list all currently
filtered prefixes or explicitly mark the list as non-exhaustive; include the
prefixes that are filtered later in this function ("skills-", "fast-mode-",
"redact-thinking-") so the comment stays accurate with the actual filtering
logic (refer to the filtering code that checks those prefixes) and avoid future
maintenance drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba59898f-fbb8-4f31-a5ac-12b00fe837b0
📒 Files selected for processing (4)
core/providers/anthropic/types.gocore/providers/anthropic/utils_test.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
079bfdd to
3610c41
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/integrations/anthropic.go (1)
207-240: Consolidate Vertex beta-header blocking into a single source of truth.This transport function hardcodes unsupported prefixes that are also governed in provider-level filtering, which increases drift risk across the stack. The new
context-management-*addition is correct, but this is a good place to delegate toFilterBetaHeadersForProvider(..., schemas.Vertex)(or shared prefix config) and keep only parsing/joining logic here.♻️ Suggested direction
- // transport-owned unsupported prefix checks... - if strings.HasPrefix(beta, anthropic.AnthropicAdvancedToolUseBetaHeaderPrefix) || ... - strings.HasPrefix(beta, anthropic.AnthropicContextManagementBetaHeaderPrefix) { - continue - } - filteredBetas = append(filteredBetas, beta) + // Delegate support policy to provider-level filter to avoid duplicated lists. + parsedBetas = append(parsedBetas, beta) ... + filteredBetas = anthropic.FilterBetaHeadersForProvider(parsedBetas, schemas.Vertex)As per coding guidelines
**: always check the stack and review changes in light of the whole stack.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` around lines 207 - 240, The function filterVertexUnsupportedBetaHeaders currently hardcodes a list of Anthropic beta-prefixes to drop; replace that provider-specific blocking logic by delegating to the central filter (call FilterBetaHeadersForProvider or the shared prefix config) for schemas.Vertex while keeping only the header lookup, comma-splitting and recomposition logic in filterVertexUnsupportedBetaHeaders; specifically, locate filterVertexUnsupportedBetaHeaders, remove the series of strings.HasPrefix checks against anthropic.*BetaHeaderPrefix constants, and call FilterBetaHeadersForProvider(headersToCheck, schemas.Vertex) (or the shared prefix filter) to determine which beta tokens to exclude, then join remaining tokens back into the same header key and return the modified headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Around line 207-240: The function filterVertexUnsupportedBetaHeaders currently
hardcodes a list of Anthropic beta-prefixes to drop; replace that
provider-specific blocking logic by delegating to the central filter (call
FilterBetaHeadersForProvider or the shared prefix config) for schemas.Vertex
while keeping only the header lookup, comma-splitting and recomposition logic in
filterVertexUnsupportedBetaHeaders; specifically, locate
filterVertexUnsupportedBetaHeaders, remove the series of strings.HasPrefix
checks against anthropic.*BetaHeaderPrefix constants, and call
FilterBetaHeadersForProvider(headersToCheck, schemas.Vertex) (or the shared
prefix filter) to determine which beta tokens to exclude, then join remaining
tokens back into the same header key and return the modified headers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 136f74fd-71a9-481f-9006-73a2e0114695
📒 Files selected for processing (4)
core/providers/anthropic/types.gocore/providers/anthropic/utils_test.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.go
🚧 Files skipped from review as they are similar to previous changes (1)
- core/providers/anthropic/types.go
Contributor
Confidence Score: 5/5Safe to merge — the fix is well-scoped, well-tested, and all findings are P2 style issues. No P0 or P1 findings. The two P2 comments are a documentation formatting regression and an unused constant/inconsistent literal usage — neither affects runtime behaviour. docs/changelogs/v1.4.23.mdx and docs/changelogs/v1.5.0-prerelease4.mdx have collapsed code-fence blocks that will break rendered output. Important Files Changed
Reviews (10): Last reviewed commit: "vertex + anthropic fixes" | Re-trigger Greptile |
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
3610c41 to
dab7e1d
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
transports/bifrost-http/integrations/anthropic.go (1)
207-207: Keep the unsupported-header comment synchronized with actual filters.Line 207 lists unsupported betas but omits
skills,fast-mode, andredact-thinking, which are also filtered below. Consider broadening or updating the comment to avoid drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` at line 207, The inline comment "Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, mcp-client, context-management." is out of sync with the actual filter list below (which also filters "skills", "fast-mode", and "redact-thinking"); update that comment to either enumerate the missing flags ("skills", "fast-mode", "redact-thinking") or broaden it (e.g., "Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, mcp-client, context-management, skills, fast-mode, redact-thinking") so the unsupported-header comment matches the filters in this file and won't drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Line 207: The inline comment "Vertex AI doesn't support: structured-outputs,
advanced-tool-use, prompt-caching-scope, mcp-client, context-management." is out
of sync with the actual filter list below (which also filters "skills",
"fast-mode", and "redact-thinking"); update that comment to either enumerate the
missing flags ("skills", "fast-mode", "redact-thinking") or broaden it (e.g.,
"Vertex AI doesn't support: structured-outputs, advanced-tool-use,
prompt-caching-scope, mcp-client, context-management, skills, fast-mode,
redact-thinking") so the unsupported-header comment matches the filters in this
file and won't drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 241683f0-c3ef-4d03-8508-e4c19b59a6c0
📒 Files selected for processing (6)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/anthropic/utils_test.go
- core/providers/anthropic/types.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
dab7e1d to
145de6d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
transports/bifrost-http/integrations/anthropic.go (1)
233-240: Prefer centralized beta filtering to prevent prefix drift.This hardcoded
HasPrefixchain can diverge from the provider-wide beta filtering logic as new prefixes are added (for example, additional families defined incore/providers/anthropic/types.go). Consider routing through a shared filter path (e.g.,anthropic.FilterBetaHeadersForProvider) and then rebuilding theanthropic-betaheader from the filtered tokens.Based on learnings: In maximhq/bifrost’s Vertex anthropic-beta handling, filtering is intended to be centralized via
anthropic.FilterBetaHeadersForProvider(...)to keep prefix-based behavior consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` around lines 233 - 240, Replace the long chain of strings.HasPrefix checks for individual prefixes (e.g., anthropic.AnthropicAdvancedToolUseBetaHeaderPrefix, AnthropicStructuredOutputsBetaHeaderPrefix, AnthropicPromptCachingScopeBetaHeaderPrefix, AnthropicMCPClientBetaHeaderPrefix, AnthropicSkillsBetaHeaderPrefix, AnthropicFastModeBetaHeaderPrefix, AnthropicRedactThinkingBetaHeaderPrefix, AnthropicContextManagementBetaHeaderPrefix) with a call into the shared filter function (anthropic.FilterBetaHeadersForProvider) to centralize prefix logic; pass the list of beta tokens (the variable `beta` or the token slice you currently iterate) into FilterBetaHeadersForProvider, use its returned filtered tokens to rebuild the `anthropic-beta` header value, and remove the per-prefix HasPrefix checks so provider-wide additions are handled consistently.core/providers/vertex/vertex.go (1)
481-483: Preserve underlying strip errors instead of dropping the cause.These branches currently create
BifrostOperationErrorwith anilwrapped error, which loses root-cause detail in logs and tracing.Suggested error-wrapping adjustment
- if stripErr != nil { - return nil, providerUtils.NewBifrostOperationError(stripErr.Error(), nil) - } + if stripErr != nil { + return nil, providerUtils.NewBifrostOperationError("failed to strip unsupported fields", stripErr) + }Also applies to: 770-772
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/vertex/vertex.go` around lines 481 - 483, The code constructs a BifrostOperationError using providerUtils.NewBifrostOperationError(stripErr.Error(), nil) which discards the original error; change these calls to pass the underlying error as the wrapped cause (e.g., providerUtils.NewBifrostOperationError(stripErr.Error(), stripErr)) so the original stripErr is preserved for logging/tracing; update the occurrence shown around the if stripErr != nil block and the similar instance later in the same file (the other NewBifrostOperationError call) to wrap the original error instead of passing nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/anthropic/utils.go`:
- Around line 456-496: Update the typed sanitizer to mirror the new JSON-path
short-circuit: inside stripUnsupportedAnthropicFields (the typed-path sanitizer)
check features.ContextManagementField and if false remove/ignore the entire
context_management payload (same as the JSON-field branch that calls
providerUtils.DeleteJSONField("context_management")); otherwise continue
applying per-edit gates (e.g., only keep edits when features.Compaction or
features.ContextEditing allow them) for context_management.edits so the typed
and raw sanitizers behave identically.
In `@transports/bifrost-http/integrations/anthropic.go`:
- Line 207: Update the inline comment that currently lists unsupported beta
families for Vertex AI to match the actual filter logic: include "skills",
"fast-mode", and "redact-thinking" in the comment so it reflects the families
being stripped by the filtering code that removes unsupported beta families for
the Anthropic/Vertex integration; ensure the comment near the Vertex AI note
mirrors the same list used in the stripping logic.
---
Nitpick comments:
In `@core/providers/vertex/vertex.go`:
- Around line 481-483: The code constructs a BifrostOperationError using
providerUtils.NewBifrostOperationError(stripErr.Error(), nil) which discards the
original error; change these calls to pass the underlying error as the wrapped
cause (e.g., providerUtils.NewBifrostOperationError(stripErr.Error(), stripErr))
so the original stripErr is preserved for logging/tracing; update the occurrence
shown around the if stripErr != nil block and the similar instance later in the
same file (the other NewBifrostOperationError call) to wrap the original error
instead of passing nil.
In `@transports/bifrost-http/integrations/anthropic.go`:
- Around line 233-240: Replace the long chain of strings.HasPrefix checks for
individual prefixes (e.g., anthropic.AnthropicAdvancedToolUseBetaHeaderPrefix,
AnthropicStructuredOutputsBetaHeaderPrefix,
AnthropicPromptCachingScopeBetaHeaderPrefix, AnthropicMCPClientBetaHeaderPrefix,
AnthropicSkillsBetaHeaderPrefix, AnthropicFastModeBetaHeaderPrefix,
AnthropicRedactThinkingBetaHeaderPrefix,
AnthropicContextManagementBetaHeaderPrefix) with a call into the shared filter
function (anthropic.FilterBetaHeadersForProvider) to centralize prefix logic;
pass the list of beta tokens (the variable `beta` or the token slice you
currently iterate) into FilterBetaHeadersForProvider, use its returned filtered
tokens to rebuild the `anthropic-beta` header value, and remove the per-prefix
HasPrefix checks so provider-wide additions are handled consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d585cb8-385b-41b5-9d2d-53d5be0a7127
📒 Files selected for processing (6)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/config.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
145de6d to
1de1e53
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
transports/bifrost-http/integrations/anthropic.go (1)
207-207:⚠️ Potential issue | 🟡 MinorComment still omits three filtered header families.
The comment at line 207 was updated to add
context-management, but it still doesn't listskills,fast-mode, andredact-thinkingwhich are filtered at lines 237-238. This discrepancy was flagged previously.✏️ Suggested update
-// Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, mcp-client, context-management. +// Vertex AI doesn't support: structured-outputs, advanced-tool-use, prompt-caching-scope, +// mcp-client, skills, fast-mode, redact-thinking, context-management.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/integrations/anthropic.go` at line 207, Update the in-file comment that begins "Vertex AI doesn't support:" to list all filtered header families; specifically add "skills", "fast-mode", and "redact-thinking" alongside the existing items (structured-outputs, advanced-tool-use, prompt-caching-scope, mcp-client, context-management) so the comment accurately reflects the header filtering implemented (the code that filters header families including "skills", "fast-mode", and "redact-thinking").
🧹 Nitpick comments (1)
core/providers/anthropic/utils_test.go (1)
1306-1315: Harden Anthropic keep-path assertions beyond top-level field existence.At Line 1313 you only assert
context_managementexists. This can pass even ifeditsare emptied or a clear edit type is accidentally stripped. Consider asserting preserved edit count/types (and includeclear_thinkingtoo) to make this regression-proof.♻️ Suggested test hardening
t.Run("anthropic_keeps_context_management_per_edit_type", func(t *testing.T) { - // Anthropic supports context_management; compact edits are kept, clear edits are also kept. - input := []byte(`{"model":"claude-sonnet-4-6","context_management":{"edits":[{"type":"` + string(ContextManagementEditTypeCompact) + `"},{"type":"` + string(ContextManagementEditTypeClearToolUses) + `"}]}}`) + // Anthropic supports context_management; all edit types should be preserved. + input := []byte(`{"model":"claude-sonnet-4-6","context_management":{"edits":[{"type":"` + string(ContextManagementEditTypeCompact) + `"},{"type":"` + string(ContextManagementEditTypeClearToolUses) + `"},{"type":"` + string(ContextManagementEditTypeClearThinking) + `"}]}}`) result, err := StripUnsupportedFieldsFromRawBody(input, schemas.Anthropic, "claude-sonnet-4-6") if err != nil { t.Fatalf("unexpected error: %v", err) } if !providerUtils.JSONFieldExists(result, "context_management") { t.Errorf("expected context_management to be kept for Anthropic, got: %s", string(result)) } + edits := providerUtils.GetJSONField(result, "context_management.edits").Array() + if len(edits) != 3 { + t.Fatalf("expected 3 edits preserved for Anthropic, got %d: %s", len(edits), string(result)) + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/utils_test.go` around lines 1306 - 1315, The test currently only checks top-level existence of "context_management"; instead update the test for anthopic_keeps_context_management_per_edit_type to parse the result JSON from StripUnsupportedFieldsFromRawBody and assert that context_management.edits contains the expected edits (including types ContextManagementEditTypeCompact, ContextManagementEditTypeClearToolUses and clear_thinking) and that the edits array length matches expected (i.e., none were removed). Use providerUtils.JSONFieldExists only for initial presence, then decode the edits array and assert element types/count to harden the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Line 207: Update the in-file comment that begins "Vertex AI doesn't support:"
to list all filtered header families; specifically add "skills", "fast-mode",
and "redact-thinking" alongside the existing items (structured-outputs,
advanced-tool-use, prompt-caching-scope, mcp-client, context-management) so the
comment accurately reflects the header filtering implemented (the code that
filters header families including "skills", "fast-mode", and "redact-thinking").
---
Nitpick comments:
In `@core/providers/anthropic/utils_test.go`:
- Around line 1306-1315: The test currently only checks top-level existence of
"context_management"; instead update the test for
anthopic_keeps_context_management_per_edit_type to parse the result JSON from
StripUnsupportedFieldsFromRawBody and assert that context_management.edits
contains the expected edits (including types ContextManagementEditTypeCompact,
ContextManagementEditTypeClearToolUses and clear_thinking) and that the edits
array length matches expected (i.e., none were removed). Use
providerUtils.JSONFieldExists only for initial presence, then decode the edits
array and assert element types/count to harden the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c697db2-038a-4ae0-9c55-fb0be1c9b162
📒 Files selected for processing (7)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (1)
- core/providers/anthropic/types.go
This was referenced Apr 26, 2026
Merged
akshaydeo
force-pushed
the
04-26-adds_message_filter_flow_for_log_details_messages
branch
from
cadd20e to
ea715ea
Compare
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
1de1e53 to
650b3a2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/anthropic/utils.go`:
- Around line 481-483: The sanitization currently only drops
"context_management" when all edits are unsupported (len(dropIndices) ==
len(edits) && len(edits) > 0), which misses the case where edits is an empty
array; update the logic in the block around variables dropIndices, edits and
jsonBody (the branch that calls providerUtils.DeleteJSONField(jsonBody,
"context_management")) to also drop "context_management" when edits is present
but empty (len(edits) == 0) so behavior matches the typed sanitizer (len(kept)
== 0). Ensure the condition that invokes providerUtils.DeleteJSONField covers
both "all edits unsupported" and "edits is empty" cases so an empty
context_management object is removed consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0708d6a9-21c2-4090-8732-2a264a92d066
📒 Files selected for processing (7)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/vertex/utils.go
- core/providers/anthropic/types.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
650b3a2 to
db5d8b1
Compare
akshaydeo
force-pushed
the
04-26-adds_message_filter_flow_for_log_details_messages
branch
from
ea715ea to
a7a2deb
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/anthropic/utils.go (1)
481-483:⚠️ Potential issue | 🟡 MinorDrop empty
context_management.editsas well for parity.At Line 481, the
len(edits) > 0guard keepscontext_managementwheneditsis an empty array, while the typed sanitizer drops it when no edits remain. This leaves raw/typed behavior inconsistent.Suggested fix
- if len(dropIndices) == len(edits) && len(edits) > 0 { + if len(dropIndices) == len(edits) { // All edits unsupported — drop the whole context_management. jsonBody, err = providerUtils.DeleteJSONField(jsonBody, "context_management")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/utils.go` around lines 481 - 483, The current branch only deletes "context_management" when all edits were removed and edits was non-empty; change the condition around the providerUtils.DeleteJSONField call (the block referencing dropIndices and edits in core/providers/anthropic/utils.go) so that if len(dropIndices) == len(edits) it deletes the entire "context_management" field even when len(edits) == 0; in practice remove the "&& len(edits) > 0" guard (or add an explicit check for len(edits) == 0) so empty edits arrays are dropped to match the typed sanitizer behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/anthropic/utils.go`:
- Around line 481-483: The current branch only deletes "context_management" when
all edits were removed and edits was non-empty; change the condition around the
providerUtils.DeleteJSONField call (the block referencing dropIndices and edits
in core/providers/anthropic/utils.go) so that if len(dropIndices) == len(edits)
it deletes the entire "context_management" field even when len(edits) == 0; in
practice remove the "&& len(edits) > 0" guard (or add an explicit check for
len(edits) == 0) so empty edits arrays are dropped to match the typed sanitizer
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99b5452f-94ba-4a3f-8e17-e70d946f6bd8
📒 Files selected for processing (7)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (2)
- transports/bifrost-http/lib/config.go
- core/providers/anthropic/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- transports/bifrost-http/integrations/anthropic.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
db5d8b1 to
792ecff
Compare
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
792ecff to
0e71c35
Compare
akshaydeo
force-pushed
the
04-26-adds_message_filter_flow_for_log_details_messages
branch
from
a7a2deb to
43a8f68
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@transports/bifrost-http/integrations/anthropic.go`:
- Around line 239-240: extractPassthroughHeaders currently ignores provider and
does not use filterVertexUnsupportedBetaHeaders, allowing unsupported beta
headers like Anthrop icContextManagementBetaHeaderPrefix to be forwarded; update
extractPassthroughHeaders to call filterVertexUnsupportedBetaHeaders (passing
the provider) when processing passthrough beta headers so that any headers
filtered by filterVertexUnsupportedBetaHeaders are removed, and apply the same
change to the other similar passthrough extraction block referenced by the
second occurrence (the block around the alternate beta handling).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d5baead-222c-4b70-856a-e274f1a609c6
📒 Files selected for processing (7)
core/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.gotransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (1)
- transports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (4)
- core/providers/vertex/utils.go
- core/providers/anthropic/utils.go
- core/providers/anthropic/utils_test.go
- core/providers/anthropic/types.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
0e71c35 to
9516aff
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/providers/anthropic/request_builder.go (1)
312-318:⚠️ Potential issue | 🟠 MajorDelete
anthropic_betawhen filtering strips everything.This only sets the body field when
betaHeadersis non-empty. In the raw-body path, a caller can already haveanthropic_betaon the JSON payload; if filtering removes all entries, the stale field is left intact and Vertex still receives the unsupported beta header in-body. That means the exact 400 this PR is fixing can still happen for passthrough requests.Suggested fix
if cfg.InjectBetaHeadersIntoBody { - if betaHeaders := FilterBetaHeadersForProvider(MergeBetaHeaders(ctx, cfg.ProviderExtraHeaders), cfg.Provider, cfg.BetaHeaderOverrides); len(betaHeaders) > 0 { + if betaHeaders := FilterBetaHeadersForProvider(MergeBetaHeaders(ctx, cfg.ProviderExtraHeaders), cfg.Provider, cfg.BetaHeaderOverrides); len(betaHeaders) > 0 { jsonBody, err = providerUtils.SetJSONField(jsonBody, "anthropic_beta", betaHeaders) if err != nil { return nil, newErr(schemas.ErrProviderRequestMarshal, err, jsonBody) } + } else { + jsonBody, err = providerUtils.DeleteJSONField(jsonBody, "anthropic_beta") + if err != nil { + return nil, newErr(schemas.ErrProviderRequestMarshal, err, jsonBody) + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/request_builder.go` around lines 312 - 318, When cfg.InjectBetaHeadersIntoBody is true you currently only SetJSONField("anthropic_beta", ...) when FilterBetaHeadersForProvider returns non-empty, leaving any existing "anthropic_beta" in raw jsonBody when the filter strips everything; fix by checking betaHeaders := FilterBetaHeadersForProvider(MergeBetaHeaders(ctx, cfg.ProviderExtraHeaders), cfg.Provider, cfg.BetaHeaderOverrides) and if len(betaHeaders) == 0 remove the "anthropic_beta" key from jsonBody (e.g. call providerUtils.DeleteJSONField(jsonBody, "anthropic_beta") or, if that helper doesn't exist, unmarshal to a map, delete the key, and re-marshal) otherwise keep the current SetJSONField behavior and error handling around providerUtils.SetJSONField.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/anthropic/types.go`:
- Around line 119-121: The schemas for Vertex currently claim Compaction support
while ContextManagementField is false (which causes any ContextManagement edits,
including ContextManagementEditTypeCompact under
AnthropicMessageRequest.ContextManagement, to be dropped), so update the Vertex
capability entries in core/providers/anthropic/types.go to make the advertised
capability match actual forwarding: either set ContextManagementField = true for
schemas.Vertex (after confirming upstream support and ensuring
ContextManagementEditTypeCompact will be forwarded) or set Compaction = false
for schemas.Vertex (and the other occurrence around lines 157-168) so compaction
is not advertised; locate the Vertex entries by the symbols Compaction,
ContextManagementField, schemas.Vertex and apply the consistent change in both
places.
---
Outside diff comments:
In `@core/providers/anthropic/request_builder.go`:
- Around line 312-318: When cfg.InjectBetaHeadersIntoBody is true you currently
only SetJSONField("anthropic_beta", ...) when FilterBetaHeadersForProvider
returns non-empty, leaving any existing "anthropic_beta" in raw jsonBody when
the filter strips everything; fix by checking betaHeaders :=
FilterBetaHeadersForProvider(MergeBetaHeaders(ctx, cfg.ProviderExtraHeaders),
cfg.Provider, cfg.BetaHeaderOverrides) and if len(betaHeaders) == 0 remove the
"anthropic_beta" key from jsonBody (e.g. call
providerUtils.DeleteJSONField(jsonBody, "anthropic_beta") or, if that helper
doesn't exist, unmarshal to a map, delete the key, and re-marshal) otherwise
keep the current SetJSONField behavior and error handling around
providerUtils.SetJSONField.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a50e2dc4-237d-4606-864f-68843760998e
📒 Files selected for processing (14)
core/providers/anthropic/anthropic.gocore/providers/anthropic/request_builder.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/azure/azure.gocore/providers/bedrock/bedrock.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.godocs/changelogs/v1.4.23.mdxdocs/changelogs/v1.5.0-prerelease4.mdxtransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/integrations/anthropic_test.gotransports/bifrost-http/lib/config.go
💤 Files with no reviewable changes (1)
- transports/bifrost-http/integrations/anthropic_test.go
✅ Files skipped from review due to trivial changes (3)
- docs/changelogs/v1.5.0-prerelease4.mdx
- docs/changelogs/v1.4.23.mdx
- transports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (3)
- transports/bifrost-http/integrations/anthropic.go
- core/providers/anthropic/utils_test.go
- core/providers/anthropic/utils.go
akshaydeo
force-pushed
the
04-26-vertex_anthropic_fixes
branch
from
9516aff to
663292e
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/providers/anthropic/utils.go (1)
457-497:⚠️ Potential issue | 🟠 MajorMirror this
ContextManagementFieldshort-circuit in the typed sanitizer too.This raw-path fix is still out of sync with
stripUnsupportedAnthropicFields: the typed path only filtersContextManagement.Editsby edit type, so providers like Vertex can still serializecontext_managementfor compact edits and hit the same 400 on non-raw requests. That leaves the stack with different behavior depending on whether the request came through raw-body passthrough.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/anthropic/utils.go` around lines 457 - 497, The typed sanitizer (stripUnsupportedAnthropicFields) must mirror the raw JSON short-circuit for ContextManagement: if features.ContextManagementField is false, remove the ContextManagement field entirely (e.g., via the typed model or DeleteJSONField equivalent), otherwise filter ContextManagement.Edits by the same edit-type whitelist (ContextManagementEditTypeCompact, ContextManagementEditTypeClearToolUses, ContextManagementEditTypeClearThinking) honoring features.Compaction and features.ContextEditing; if filtering removes all edits, delete the whole ContextManagement field. Update stripUnsupportedAnthropicFields to use the same logic and constants so providers like Vertex won’t serialize unsupported context_management edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@core/providers/anthropic/utils.go`:
- Around line 457-497: The typed sanitizer (stripUnsupportedAnthropicFields)
must mirror the raw JSON short-circuit for ContextManagement: if
features.ContextManagementField is false, remove the ContextManagement field
entirely (e.g., via the typed model or DeleteJSONField equivalent), otherwise
filter ContextManagement.Edits by the same edit-type whitelist
(ContextManagementEditTypeCompact, ContextManagementEditTypeClearToolUses,
ContextManagementEditTypeClearThinking) honoring features.Compaction and
features.ContextEditing; if filtering removes all edits, delete the whole
ContextManagement field. Update stripUnsupportedAnthropicFields to use the same
logic and constants so providers like Vertex won’t serialize unsupported
context_management edits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2cc7e37-f660-46f8-9dde-b8d9e0d15bfa
📒 Files selected for processing (14)
core/providers/anthropic/anthropic.gocore/providers/anthropic/request_builder.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/azure/azure.gocore/providers/bedrock/bedrock.gocore/providers/vertex/utils.gocore/providers/vertex/vertex.godocs/changelogs/v1.4.23.mdxdocs/changelogs/v1.5.0-prerelease4.mdxtransports/bifrost-http/integrations/anthropic.gotransports/bifrost-http/integrations/anthropic_test.gotransports/bifrost-http/lib/config.go
💤 Files with no reviewable changes (1)
- transports/bifrost-http/integrations/anthropic_test.go
✅ Files skipped from review due to trivial changes (3)
- docs/changelogs/v1.5.0-prerelease4.mdx
- docs/changelogs/v1.4.23.mdx
- transports/bifrost-http/lib/config.go
🚧 Files skipped from review as they are similar to previous changes (2)
- core/providers/anthropic/request_builder.go
- transports/bifrost-http/integrations/anthropic.go
Contributor
Author
Merge activity
|
akshaydeo
changed the base branch from
04-26-adds_message_filter_flow_for_log_details_messages
to
graphite-base/3055
akshaydeo
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
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
2 participants
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.
Summary
Disables
context-managementbeta header support on Vertex AI after live API errors confirmed it is unsupported. This preventsClearToolUses/ClearThinkingcontext editing fields from being forwarded to Vertex, which would cause 400 errors.Changes
ContextEditing: truefrom the Vertex provider feature support map, reflecting that thecontext-managementbeta is not supported on Vertex (confirmed via live API 400 error).AnthropicContextManagementBetaHeaderPrefixconstant to enable prefix-based filtering of thecontext-managementbeta header.filterVertexUnsupportedBetaHeadersin the HTTP transport to strip anycontext-management-*beta headers before forwarding requests to Vertex.StripUnsupportedFieldsFromRawBodyin the Vertex provider'sChatCompletionpath to remove unsupported fields (e.g., context editing edits) from the raw request body before sending.AnthropicContextManagementBetaHeaderis now in the unsupported set for Vertex rather than the supported set.Type of change
Affected areas
How to test
Send a request with a
context-managementbeta header or context editing fields (e.g.,ClearToolUses/ClearThinking) targeting a Vertex-backed model and confirm no 400 error is returned and the header/fields are stripped before forwarding.Breaking changes
Related issues
References live API error confirming Vertex returns 400 for
context-managementbeta header.Security considerations
None.
Checklist
docs/contributing/README.mdand followed the guidelines