anthropic changes - #3227
Closed
akshaydeo wants to merge 1 commit into
Closed
Conversation
|
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change replaces model-parameter overrides with provider-scoped model capabilities. It adds capability caching, datasheet hydration, Anthropic override-aware behavior, provider-aware token defaults, and updated reasoning and tool conversion paths. ChangesProvider-aware model capabilities
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Datasheet
participant CapabilityCache
participant AnthropicHelpers
participant RequestConversion
Datasheet->>CapabilityCache: publish provider/model capabilities
RequestConversion->>AnthropicHelpers: resolve provider-aware capability
AnthropicHelpers->>CapabilityCache: look up capability record
CapabilityCache-->>AnthropicHelpers: return override or fallback inputs
AnthropicHelpers-->>RequestConversion: return thinking, speed, effort, or tool decision
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ❌ 5❌ Failed checks (4 warnings, 1 inconclusive)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
akshaydeo
marked this pull request as ready for review
May 5, 2026 11:15
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. This stack of pull requests is managed by Graphite. Learn more about stacking. |
![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
🤖 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 `@core/providers/anthropic/utils.go`:
- Around line 699-705: The current override handling in
providerUtils.GetBifrostOverrides treats any non-new literal as old-gen, causing
future/newer server_tools versions to be downgraded; change the logic in the
computer_use branch to explicitly check for known constants (e.g., if value ==
AnthropicToolTypeComputer20251124 return ComputerUseGen20251124; else if value
== AnthropicToolTypeComputer20250124 return ComputerUseGen20250124) and for any
unknown/ newer values default to the new-gen path (return
ComputerUseGen20251124) or fall back to name-detection, and apply the same
explicit-constant checks and default-new behavior to the analogous text_editor
branch (the constants and return values around
ComputerUseGen20251124/ComputerUseGen20250124 and the text-editor equivalents).
🪄 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: 26d112fe-4ebd-4b8d-92a5-3ef3a0e7e5de
📒 Files selected for processing (2)
core/providers/anthropic/capability_overrides_test.gocore/providers/anthropic/utils.go
Contributor
Confidence Score: 4/5This is close, but the override lookup should be fixed before merging.
core/providers/utils/modelparamscache.go Important Files Changed
Reviews (5): Last reviewed commit: "anthropic changes" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
July 7, 2026 10:59
2693433 to
4c11b11
Compare
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 7, 2026 10:59
f7c5554 to
1b95946
Compare
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
July 9, 2026 11:13
4c11b11 to
aa762f1
Compare
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 9, 2026 11:13
1b95946 to
889d59d
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
framework/modelcatalog/datasheet/types.go (1)
745-819: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a drift-guard test for
isEmptyBifrostOverrides.Field-by-field check confirms this correctly covers all 70 current
BifrostOverridesfields. But it's a hand-maintained conjunction in a different file from the struct it mirrors — a future field added toBifrostOverrideswithout a matching clause here would silently make populated overrides look "empty" and get dropped bybifrostOverridesIfPresent, with no compiler error.Consider a small test using
reflect.NumField(reflect.TypeOf(schemas.BifrostOverrides{}))compared against a hardcoded expected count (bumped whenever a field is added), or a reflection-based zero-value walk, purely as a regression guard — not a change to the runtime check itself.🤖 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 `@framework/modelcatalog/datasheet/types.go` around lines 745 - 819, Add a drift-guard test for isEmptyBifrostOverrides in the datasheet package to ensure its hand-maintained nil/empty-field conjunction stays in sync with schemas.BifrostOverrides. The issue is that a future field added to BifrostOverrides could be omitted here and still be treated as empty by bifrostOverridesIfPresent; fix it by adding a regression test that compares the struct field count via reflection or walks the zero-value struct, and keep the expected count or coverage aligned with BifrostOverrides as fields change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@framework/modelcatalog/datasheet/types.go`:
- Around line 745-819: Add a drift-guard test for isEmptyBifrostOverrides in the
datasheet package to ensure its hand-maintained nil/empty-field conjunction
stays in sync with schemas.BifrostOverrides. The issue is that a future field
added to BifrostOverrides could be omitted here and still be treated as empty by
bifrostOverridesIfPresent; fix it by adding a regression test that compares the
struct field count via reflection or walks the zero-value struct, and keep the
expected count or coverage aligned with BifrostOverrides as fields change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0edb2176-c9a3-4260-8828-92b8ae7ebffe
📒 Files selected for processing (12)
core/providers/anthropic/capability_overrides_test.gocore/providers/anthropic/chat.gocore/providers/anthropic/responses.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/bedrock/responses.gocore/providers/bedrock/utils.gocore/providers/utils/modelparamscache.gocore/providers/utils/modelparamscache_bifrost_test.gocore/schemas/bifrostoverrides.goframework/modelcatalog/datasheet/sync.goframework/modelcatalog/datasheet/types.go
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 9, 2026 13:48
889d59d to
429fd9b
Compare
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 9, 2026 13:57
429fd9b to
5a351e1
Compare
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
July 9, 2026 13:57
aa762f1 to
fdf209c
Compare
Comment on lines
313
to
318
| if base := schemas.BaseModelName(model); base != model { | ||
| if ov := GetBifrostOverrides(OverrideCacheKey(base, provider)); ov != nil { | ||
| return ov | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // candidateBifrostOverrideKeys returns the provider-conventional datasheet | ||
| // keys to try for a bare model name (after the verbatim lookup misses). | ||
| // Order matters — the most likely match comes first. Used by | ||
| // GetBifrostOverridesForRequest. | ||
| func candidateBifrostOverrideKeys(provider schemas.ModelProvider, model string) []string { | ||
| switch provider { | ||
| case schemas.Vertex: | ||
| return []string{"vertex_ai/" + model} | ||
| case schemas.Azure: | ||
| return []string{"azure/" + model} | ||
| case schemas.Bedrock: | ||
| // Bedrock prefixes are family-stamped on the datasheet: | ||
| // anthropic.<...>-v1:0 for Claude | ||
| // meta.<...>-v1:0 for Llama | ||
| // mistral.<...>-v1:0 for Mistral / Codestral | ||
| // amazon.<...> for Nova / Titan | ||
| // ai21.<...> for Jamba | ||
| // cohere.<...> for Command R / Embed | ||
| // stability.<...> for Stable Diffusion | ||
| switch { | ||
| case schemas.IsAnthropicModel(model): | ||
| return []string{"anthropic." + model} | ||
| case schemas.IsLlamaModel(model): | ||
| return []string{"meta." + model} | ||
| case schemas.IsMistralModel(model): | ||
| return []string{"mistral." + model} | ||
| case schemas.IsNovaModel(model): | ||
| return []string{"amazon." + model} | ||
| } | ||
| return nil | ||
| } | ||
| return nil |
Contributor
There was a problem hiding this comment.
This fallback still does not normalize provider-prefixed runtime IDs to the base key used when overrides are stored. The cache writer stores datasheet overrides under keys like claude-sonnet-4|bedrock, but this lookup only tries the raw model and schemas.BaseModelName(model). For a Bedrock request using anthropic.claude-sonnet-4-20250514-v1:0 or a regional us.anthropic... ID, BaseModelName removes version suffixes but leaves the Bedrock prefix, so it never probes claude-sonnet-4|bedrock. Vertex and Azure IDs with vertex_ai/ or azure/ can miss the same way when storage used the bare base_model. Those requests still fall back to substring gates instead of the provider-specific datasheet override.
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 27, 2026 04:52
5a351e1 to
ea85d4e
Compare
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
July 27, 2026 04:52
fdf209c to
5573659
Compare
Contributor
There was a problem hiding this comment.
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/responses.go (1)
3342-3345: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuthorization Bypass (CWE-863): Incorrect Authorization
Reachability: External
● Entry core/providers/anthropic/utils_test.go:2175 BuildAnthropicResponsesRequestBody │ ▼ ● Hop core/providers/anthropic/requestbuilder.go:137 BuildAnthropicResponsesRequestBody: capModel is the canonical model used for capability gating in the raw-body │ ▼ ● Sink core/providers/anthropic/responses.goSurface Anthropic native fallbacks to policy checks
Nativefallbacksare copied throughExtraParamsand rebuilt only for the Anthropic wire request, so allowlist/governance checks that inspectBifrostResponsesRequest.Fallbacksnever see them. That lets a caller hide alternate models — includingdefault— behind a server-side fallback and have Anthropic execute them outside Bifrost’s normal policy path. Reject or normalize these entries through the same model/parameter checks before storing them inExtraParams.🤖 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 `@core/providers/anthropic/responses.go` around lines 3342 - 3345, Update the fallback handling around req.nativeFallbacks() so native fallback entries are first exposed through BifrostResponsesRequest.Fallbacks and passed through the same model/parameter policy validation as configured fallbacks, including rejecting or normalizing default and alternate models. Only after validation should the approved fallback representation be stored in params.ExtraParams["fallbacks"], preserving the existing preset handling.
🤖 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.
Outside diff comments:
In `@core/providers/anthropic/responses.go`:
- Around line 3342-3345: Update the fallback handling around
req.nativeFallbacks() so native fallback entries are first exposed through
BifrostResponsesRequest.Fallbacks and passed through the same model/parameter
policy validation as configured fallbacks, including rejecting or normalizing
default and alternate models. Only after validation should the approved fallback
representation be stored in params.ExtraParams["fallbacks"], preserving the
existing preset handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e890b780-9434-418a-a787-b2f1a97b9592
📒 Files selected for processing (23)
core/providers/anthropic/capability_overrides_test.gocore/providers/anthropic/chat.gocore/providers/anthropic/requestbuilder.gocore/providers/anthropic/responses.gocore/providers/anthropic/text.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/bedrock/responses.gocore/providers/bedrock/utils.gocore/providers/cohere/chat.gocore/providers/cohere/responses.gocore/providers/gemini/chat.gocore/providers/gemini/responses.gocore/providers/gemini/utils.gocore/providers/openai/chat.gocore/providers/openai/responses.gocore/providers/utils/modelparamscache.gocore/providers/utils/modelparamscache_bifrost_test.gocore/providers/utils/modelparamscache_test.gocore/schemas/chatcompletions.gocore/schemas/modelcapabilities.gocore/schemas/modelcapabilities_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- core/providers/anthropic/chat.go
- core/providers/anthropic/capability_overrides_test.go
- core/providers/anthropic/utils.go
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
July 27, 2026 05:14
5573659 to
7b9c917
Compare
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 27, 2026 05:14
ea85d4e to
4f7b64d
Compare
Contributor
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 `@framework/configstore/migrations.go`:
- Around line 455-458: Register migrationAddBudgetOverrideAnchorColumns in
configstoreMigrationSteps immediately after add_budget_override_columns, using a
unique migration ID that identifies the anchor-column migration. Preserve the
existing migration order and ensure the step runs for existing databases before
UpdateBudgetOverride writes the new columns.
In `@framework/modelcatalog/datasheet/params.go`:
- Around line 244-245: Update the capability-processing logic around
ModelCapabilities unmarshalling to track successful capability decodes
separately from the generic applied count. Log records whose capability fields
fail to decode, and only replace the working overrides cache when the feed
contains at least one successfully decoded capability record; otherwise preserve
the existing cache.
🪄 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 Plus
Run ID: 5c64470d-ca23-48a7-9e36-f96424c4f1ed
📒 Files selected for processing (28)
core/providers/anthropic/capability_overrides_test.gocore/providers/anthropic/chat.gocore/providers/anthropic/requestbuilder.gocore/providers/anthropic/responses.gocore/providers/anthropic/text.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/bedrock/responses.gocore/providers/bedrock/utils.gocore/providers/cohere/chat.gocore/providers/cohere/responses.gocore/providers/gemini/chat.gocore/providers/gemini/responses.gocore/providers/gemini/utils.gocore/providers/openai/chat.gocore/providers/openai/responses.gocore/providers/utils/modelparamscache.gocore/providers/utils/modelparamscache_bifrost_test.gocore/providers/utils/modelparamscache_test.gocore/schemas/chatcompletions.gocore/schemas/modelcapabilities.gocore/schemas/modelcapabilities_test.goframework/configstore/migrations.goframework/configstore/rdb.goframework/modelcatalog/datasheet/params.goframework/modelcatalog/datasheet/sync.goframework/modelcatalog/datasheet/types.go
🚧 Files skipped from review as they are similar to previous changes (22)
- core/providers/anthropic/text.go
- core/providers/anthropic/requestbuilder.go
- core/providers/gemini/chat.go
- core/providers/openai/chat.go
- core/providers/anthropic/types.go
- core/providers/openai/responses.go
- core/providers/cohere/chat.go
- core/schemas/modelcapabilities.go
- core/providers/bedrock/utils.go
- core/providers/utils/modelparamscache_test.go
- core/providers/gemini/responses.go
- framework/modelcatalog/datasheet/sync.go
- core/schemas/modelcapabilities_test.go
- core/providers/utils/modelparamscache.go
- core/providers/gemini/utils.go
- core/providers/utils/modelparamscache_bifrost_test.go
- core/providers/anthropic/chat.go
- core/providers/anthropic/responses.go
- core/providers/anthropic/utils_test.go
- core/providers/bedrock/responses.go
- core/providers/anthropic/utils.go
- core/providers/anthropic/capability_overrides_test.go
Comment on lines
+244
to
+245
| var ov schemas.ModelCapabilities | ||
| if err := json.Unmarshal(rawData, &ov); err == nil { |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avoid clearing capabilities after capability decode failures.
applied is incremented after the generic decode, while ModelCapabilities decode errors are ignored. A feed with invalid capability field types can therefore build an empty overrides map and replace the working cache. Count successful capability decodes separately and only replace from a capability-valid feed; log rejected records.
Also applies to: 293-294
🤖 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 `@framework/modelcatalog/datasheet/params.go` around lines 244 - 245, Update
the capability-processing logic around ModelCapabilities unmarshalling to track
successful capability decodes separately from the generic applied count. Log
records whose capability fields fail to decode, and only replace the working
overrides cache when the feed contains at least one successfully decoded
capability record; otherwise preserve the existing cache.
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
July 27, 2026 05:27
4f7b64d to
061be54
Compare
This was referenced Jul 28, 2026
Open
Closed
TejasGhatte
force-pushed
the
05-05-anthropic_changes
branch
from
August 3, 2026 05:17
061be54 to
53f788c
Compare
TejasGhatte
force-pushed
the
datasheet-backed-compatibility-flows
branch
from
August 3, 2026 05:17
7b9c917 to
54ea738
Compare
Contributor
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)
core/providers/gemini/responses.go (1)
2811-2833: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign annotation
Textnullability between streaming and non-streaming grounding output.Line 2825 unconditionally sets
Text: schemas.Ptr(support.Segment.Text), so an emptysupport.Segment.Textbecomes a pointer to"". The streaming counterpart inemitAnnotationsFromGroundingSupports(lines 4069-4071) only setsTextwhensupport.Segment.Text != "", leaving itnilotherwise.A client that reconstructs the final response from streaming
annotation.addedevents seesText == nilfor an empty-segment citation, but the same conceptual case from a non-streaming call seesTextpointing to"". Make both paths use the same nullability rule forText.🔧 Proposed fix to align non-streaming with the streaming nil-check
annotation := schemas.ResponsesOutputMessageContentTextAnnotation{ Type: "url_citation", - Text: schemas.Ptr(support.Segment.Text), StartIndex: schemas.Ptr(int(support.Segment.StartIndex)), EndIndex: schemas.Ptr(int(support.Segment.EndIndex)), URL: schemas.Ptr(source.URL), } + if support.Segment.Text != "" { + annotation.Text = &support.Segment.Text + } annotation.Title = source.Title🤖 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 `@core/providers/gemini/responses.go` around lines 2811 - 2833, Update the annotation construction in the non-streaming grounding path to match emitAnnotationsFromGroundingSupports: only assign Text when support.Segment.Text is non-empty, leaving it nil for empty text while preserving the existing citation fields and source handling.
🧹 Nitpick comments (1)
framework/modelcatalog/datasheet/params.go (1)
174-184: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueSkip building
recordswhen a miss handler is registered.
recordsandrecordSourceare populated for every capability row on every sync, but they are consumed only inside the!providerUtils.HasCacheMissHandler()branch at line 279. In the config-store deployment mode a miss handler is always registered byframework/modelcatalog/main.go, so the two maps are allocated and filled across the whole datasheet and then discarded.Read the handler flag once before the loop and skip the record bookkeeping when it is set. The alias index must still be built in both modes.
Also applies to: 186-197
🤖 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 `@framework/modelcatalog/datasheet/params.go` around lines 174 - 184, Update Store.applyModelParameters to read providerUtils.HasCacheMissHandler() once before processing rows, and only allocate/populate records and recordSource when no miss handler is registered; preserve the existing alias index construction in both modes and keep the non-handler cache behavior unchanged.
🤖 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 `@core/providers/utils/modelcapabilities_test.go`:
- Around line 178-192: Update TestGetMaxOutputTokensOrDefault to exercise
GetMaxOutputTokensOrDefault with an uncached Anthropic/Claude model, asserting
the production static fallback value instead of invoking the test-only
staticAnthropicFallback or knownAnthropicMaxOutputTokens duplicate. Keep the
existing cached-value and non-Claude default assertions unchanged.
In `@core/providers/utils/modelcapabilities.go`:
- Around line 191-210: Update modelCapabilitiesCache.fetch so cleanup is
registered with defer immediately after creating and storing the inflightCall:
always close call.done and remove rowKey from c.inflight, including when
handler(rowKey) panics. Preserve the existing result assignment, waiter
behavior, and return path.
- Around line 216-236: Update candidateRowKeys and the GetModelParametersByModel
lookup flow so bare model candidates cannot return rows belonging to another
provider; validate each fetched row’s provider against the requested provider
before caching or returning it, while preserving alias-based lookups for
provider-specific keys.
In `@framework/modelcatalog/datasheet/params.go`:
- Around line 274-285: Update the sync publish guard to use the parsed
capability records map, requiring len(records) to be nonzero before updating
aliases, invalidating, or bulk-storing; otherwise retain the existing cache and
warning behavior. In the no-miss-handler path, add or use a bulk-store operation
that stamps the next capability generation and publishes the seeded records
atomically with invalidation, preventing requests from observing an empty cache
during replacement.
In `@framework/modelcatalog/main.go`:
- Around line 114-128: Update SetCacheMissHandler and CapabilitiesFor so
transient store errors and JSON unmarshal failures are logged and are not cached
as “no capabilities”; extend the miss-handler result with a cacheable indicator
if needed, tombstoning only confirmed missing or empty rows while preserving
successful capability caching.
---
Outside diff comments:
In `@core/providers/gemini/responses.go`:
- Around line 2811-2833: Update the annotation construction in the non-streaming
grounding path to match emitAnnotationsFromGroundingSupports: only assign Text
when support.Segment.Text is non-empty, leaving it nil for empty text while
preserving the existing citation fields and source handling.
---
Nitpick comments:
In `@framework/modelcatalog/datasheet/params.go`:
- Around line 174-184: Update Store.applyModelParameters to read
providerUtils.HasCacheMissHandler() once before processing rows, and only
allocate/populate records and recordSource when no miss handler is registered;
preserve the existing alias index construction in both modes and keep the
non-handler cache behavior unchanged.
🪄 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 Plus
Run ID: 6b986cfe-e3d2-459c-836e-6a025f3d27be
📒 Files selected for processing (32)
core/providers/anthropic/capability_overrides_test.gocore/providers/anthropic/chat.gocore/providers/anthropic/requestbuilder.gocore/providers/anthropic/responses.gocore/providers/anthropic/text.gocore/providers/anthropic/types.gocore/providers/anthropic/utils.gocore/providers/anthropic/utils_test.gocore/providers/bedrock/responses.gocore/providers/bedrock/utils.gocore/providers/cohere/chat.gocore/providers/cohere/responses.gocore/providers/gemini/chat.gocore/providers/gemini/responses.gocore/providers/gemini/utils.gocore/providers/openai/chat.gocore/providers/openai/responses.gocore/providers/utils/modelcapabilities.gocore/providers/utils/modelcapabilities_cache_test.gocore/providers/utils/modelcapabilities_test.gocore/providers/utils/modelparamscache.gocore/providers/vertex/utils_test.gocore/schemas/chatcompletions.gocore/schemas/modelcapabilities.gocore/schemas/modelcapabilities_test.goframework/configstore/migrations.goframework/configstore/rdb.goframework/modelcatalog/datasheet/capabilities_test.goframework/modelcatalog/datasheet/params.goframework/modelcatalog/datasheet/sync.goframework/modelcatalog/datasheet/types.goframework/modelcatalog/main.go
💤 Files with no reviewable changes (2)
- framework/modelcatalog/datasheet/sync.go
- core/providers/utils/modelparamscache.go
🚧 Files skipped from review as they are similar to previous changes (20)
- core/providers/cohere/responses.go
- core/schemas/chatcompletions.go
- core/providers/anthropic/text.go
- core/providers/gemini/chat.go
- core/providers/openai/responses.go
- core/providers/anthropic/types.go
- core/providers/openai/chat.go
- core/providers/bedrock/responses.go
- core/providers/bedrock/utils.go
- core/providers/anthropic/requestbuilder.go
- core/providers/anthropic/chat.go
- core/providers/cohere/chat.go
- core/providers/gemini/utils.go
- core/schemas/modelcapabilities_test.go
- framework/configstore/migrations.go
- core/providers/anthropic/utils_test.go
- core/providers/anthropic/responses.go
- framework/configstore/rdb.go
- core/providers/anthropic/utils.go
- core/providers/anthropic/capability_overrides_test.go
Comment on lines
178
to
192
| func TestGetMaxOutputTokensOrDefault(t *testing.T) { | ||
| cache := getModelParamsCache() | ||
| cache.Set("test-or-default", ModelParams{MaxOutputTokens: intPtr(16384)}) | ||
| key := CapabilityCacheKey("test-or-default", schemas.Anthropic) | ||
| SetModelCapability(key, capsWithMax(16384)) | ||
| t.Cleanup(func() { DeleteModelCapability(key) }) | ||
|
|
||
| val := GetMaxOutputTokensOrDefault("test-or-default", 4096) | ||
| val := GetMaxOutputTokensOrDefault(schemas.Anthropic, "test-or-default", 4096) | ||
| if val != 16384 { | ||
| t.Errorf("expected cached value 16384, got %d", val) | ||
| } | ||
|
|
||
| val = GetMaxOutputTokensOrDefault("missing-model-default", 4096) | ||
| val = GetMaxOutputTokensOrDefault(schemas.OpenAI, "missing-model-default", 4096) | ||
| if val != 4096 { | ||
| t.Errorf("expected default 4096 for missing non-claude model, got %d", val) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate staticAnthropicFallback and compare it with the inline table lookup.
set -euo pipefail
rg -n -C 12 'staticAnthropicFallback' --glob '*.go'
rg -n -C 3 'knownAnthropicMaxOutputTokens' --glob '*.go'Repository: maximhq/bifrost
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)modelcapabilities(_test)?\.go$'
printf '%s\n' '--- relevant symbols ---'
rg -n -C 8 'staticAnthropicFallback|knownAnthropicMaxOutputTokens|GetMaxOutputTokensOrDefault|TestGetMaxOutputTokensOrDefaultStaticFallback' \
core/providers/utils 2>/dev/null || true
printf '%s\n' '--- package files ---'
git ls-files core/providers/utils | sed -n '1,120p'Repository: maximhq/bifrost
Length of output: 12869
Test the production static fallback.
staticAnthropicFallback is a test-only duplicate that reads knownAnthropicMaxOutputTokens; calling it does not test GetMaxOutputTokensOrDefault. Exercise the production function with an uncached Claude model instead.
🤖 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 `@core/providers/utils/modelcapabilities_test.go` around lines 178 - 192,
Update TestGetMaxOutputTokensOrDefault to exercise GetMaxOutputTokensOrDefault
with an uncached Anthropic/Claude model, asserting the production static
fallback value instead of invoking the test-only staticAnthropicFallback or
knownAnthropicMaxOutputTokens duplicate. Keep the existing cached-value and
non-Claude default assertions unchanged.
Comment on lines
+191
to
+210
| func (c *modelCapabilitiesCache) fetch(rowKey string, handler func(string) *schemas.ModelCapabilities) *schemas.ModelCapabilities { | ||
| c.inflightMu.Lock() | ||
| if call, ok := c.inflight[rowKey]; ok { | ||
| c.inflightMu.Unlock() | ||
| <-call.done | ||
| return call.result | ||
| } | ||
| call := &inflightCall{done: make(chan struct{})} | ||
| c.inflight[rowKey] = call | ||
| c.inflightMu.Unlock() | ||
|
|
||
| call.result = handler(rowKey) | ||
| close(call.done) | ||
|
|
||
| c.inflightMu.Lock() | ||
| delete(c.inflight, rowKey) | ||
| c.inflightMu.Unlock() | ||
|
|
||
| return call.result | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Release the inflight slot with defer.
If handler panics, close(call.done) and delete(c.inflight, rowKey) never run. Every later caller for the same rowKey then blocks forever on <-call.done, and the key stays poisoned for the process lifetime. The handler registered in framework/modelcatalog/main.go performs a DB read and a JSON unmarshal, so a panic is reachable.
Move the cleanup into deferred calls so the waiters are always released.
🔒️ Proposed fix to make the inflight slot panic-safe
call := &inflightCall{done: make(chan struct{})}
c.inflight[rowKey] = call
c.inflightMu.Unlock()
- call.result = handler(rowKey)
- close(call.done)
-
- c.inflightMu.Lock()
- delete(c.inflight, rowKey)
- c.inflightMu.Unlock()
+ defer func() {
+ close(call.done)
+ c.inflightMu.Lock()
+ delete(c.inflight, rowKey)
+ c.inflightMu.Unlock()
+ }()
+ call.result = handler(rowKey)
return call.result📝 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
| func (c *modelCapabilitiesCache) fetch(rowKey string, handler func(string) *schemas.ModelCapabilities) *schemas.ModelCapabilities { | |
| c.inflightMu.Lock() | |
| if call, ok := c.inflight[rowKey]; ok { | |
| c.inflightMu.Unlock() | |
| <-call.done | |
| return call.result | |
| } | |
| call := &inflightCall{done: make(chan struct{})} | |
| c.inflight[rowKey] = call | |
| c.inflightMu.Unlock() | |
| call.result = handler(rowKey) | |
| close(call.done) | |
| c.inflightMu.Lock() | |
| delete(c.inflight, rowKey) | |
| c.inflightMu.Unlock() | |
| return call.result | |
| } | |
| func (c *modelCapabilitiesCache) fetch(rowKey string, handler func(string) *schemas.ModelCapabilities) *schemas.ModelCapabilities { | |
| c.inflightMu.Lock() | |
| if call, ok := c.inflight[rowKey]; ok { | |
| c.inflightMu.Unlock() | |
| <-call.done | |
| return call.result | |
| } | |
| call := &inflightCall{done: make(chan struct{})} | |
| c.inflight[rowKey] = call | |
| c.inflightMu.Unlock() | |
| defer func() { | |
| close(call.done) | |
| c.inflightMu.Lock() | |
| delete(c.inflight, rowKey) | |
| c.inflightMu.Unlock() | |
| }() | |
| call.result = handler(rowKey) | |
| return call.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 `@core/providers/utils/modelcapabilities.go` around lines 191 - 210, Update
modelCapabilitiesCache.fetch so cleanup is registered with defer immediately
after creating and storing the inflightCall: always close call.done and remove
rowKey from c.inflight, including when handler(rowKey) panics. Preserve the
existing result assignment, waiter behavior, and return path.
Comment on lines
+216
to
+236
| func (c *modelCapabilitiesCache) candidateRowKeys(model string, provider schemas.ModelProvider) []string { | ||
| candidates := []string{model} | ||
| add := func(candidate string) { | ||
| if candidate != "" && !slices.Contains(candidates, candidate) { | ||
| candidates = append(candidates, candidate) | ||
| } | ||
| } | ||
|
|
||
| add(c.aliasFor(CapabilityCacheKey(model, provider))) | ||
|
|
||
| // normalizeClaudeModelName is Claude-specific — it strips everything before | ||
| // the last ".", which mangles names like "gpt-4.1-2025-04-14" into "1". | ||
| if strings.Contains(model, "claude") { | ||
| if base := normalizeClaudeModelName(model); base != model { | ||
| add(c.aliasFor(CapabilityCacheKey(base, provider))) | ||
| add(base) | ||
| } | ||
| } | ||
|
|
||
| return candidates | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect datasheet row-key construction and provider scoping for capability rows.
set -euo pipefail
# Row key extraction used when publishing capability records.
rg -n -C 10 'func extractModelName|func normalizeProvider' --glob '*.go'
# Miss-handler row lookup: confirm whether GetModelParametersByModel is provider-scoped.
rg -n -C 10 'GetModelParametersByModel' --glob '*.go'Repository: maximhq/bifrost
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidateRowKeys and fetch call sites ---'
rg -n -C 12 'candidateRowKeys|c\.fetch\(|func \(c \*modelCapabilitiesCache\) fetch|CapabilityCacheKey' core/providers/utils/modelcapabilities.go
printf '%s\n' '--- related model-catalog and datasheet symbols ---'
rg -n -C 10 'GetModelParametersByModel|extractModelName|normalizeProvider|ModelParameters|datasheet|model.*provider|provider.*model' --glob '*.go' --glob '!vendor/**'
printf '%s\n' '--- relevant tests ---'
rg -n -C 14 'NoCrossProviderCollision|miss handler|miss-handler|CapabilitiesFor' core/providers/utils --glob '*_test.go'Repository: maximhq/bifrost
Length of output: 7159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository files related to model capabilities and model catalog ---'
git ls-files | rg -i 'model|capabilit|datasheet|catalog|pricing' | head -200
printf '%s\n' '--- all references to miss-handler registration and lookup APIs ---'
rg -n -i -C 8 'SetCacheMissHandler|GetModelParametersByModel|ModelParametersByModel|modelcatalog|datasheet' . --glob '!*.sum' --glob '!*.mod' --glob '!vendor/**' || true
printf '%s\n' '--- all definitions of model capability schemas and key construction ---'
rg -n -C 8 'type ModelCapabilities|CapabilityCacheKey|normalizeClaudeModelName|extractModelName|normalizeProvider' . --glob '*.go' || trueRepository: maximhq/bifrost
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- datasheet package files ---'
git ls-files framework/modelcatalog/datasheet
printf '%s\n' '--- miss-handler registration and model-parameter lookup ---'
rg -n -C 12 'SetCacheMissHandler|GetModelParametersByModel|ResolveModelParameters|ModelParameters' framework core --glob '*.go' || true
printf '%s\n' '--- row-key and provider extraction in datasheet package ---'
rg -n -C 12 'extractModelName|normalizeProvider|provider.*model|model.*provider|rowKey|RowKey|CapabilityCacheKey' framework/modelcatalog/datasheet --glob '*.go' || trueRepository: maximhq/bifrost
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- capability row construction ---'
sed -n '164,287p' framework/modelcatalog/datasheet/params.go
printf '%s\n' '--- cache miss handler registration ---'
sed -n '55,130p' framework/modelcatalog/main.go
printf '%s\n' '--- config-store GetModelParametersByModel implementations ---'
rg -n -l 'GetModelParametersByModel' framework --glob '*.go' | while read -r f; do
echo "FILE: $f"
rg -n -C 18 'GetModelParametersByModel' "$f"
done
printf '%s\n' '--- relevant capability cache tests ---'
rg -n -C 18 'NoCrossProviderCollision|SetCacheMissHandler|CacheMiss|CapabilitiesFor' core/providers/utils --glob '*_test.go' || trueRepository: maximhq/bifrost
Length of output: 46519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- extractModelName and provider normalization definitions ---'
rg -n -C 12 'func extractModelName|func normalizeProvider|extractModelName|normalizeProvider' framework/modelcatalog/datasheet framework --glob '*.go' | head -300
printf '%s\n' '--- model-parameters fixture provider/model relationships ---'
python3 - <<'PY'
import json
from pathlib import Path
p = Path("framework/modelcatalog/datasheet/testdata/model-parameters.json")
data = json.loads(p.read_text())
rows = []
for model, value in data.items():
if isinstance(value, dict):
provider = value.get("provider", "")
base = value.get("base_model", "")
if "claude" in model.lower() or "claude" in str(base).lower():
rows.append((model, provider, base))
for row in sorted(rows):
print("\\t".join(row))
print(f"claude_rows={len(rows)} total_rows={len(data)}")
PY
printf '%s\n' '--- model-parameter table schema and key constraints ---'
rg -n -C 10 'type TableModelParameters|TableModelParameters' framework/configstore --glob '*.go' | head -200Repository: maximhq/bifrost
Length of output: 38429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- complete model-parameters fixture keys and providers ---'
python3 - <<'PY'
import json
from pathlib import Path
data = json.loads(Path("framework/modelcatalog/datasheet/testdata/model-parameters.json").read_text())
for model in sorted(data):
row = data[model]
print(f"{model}\tprovider={row.get('provider')}\tbase_model={row.get('base_model')}")
PY
printf '%s\n' '--- deterministic miss-handler reproduction ---'
python3 - <<'PY'
# Reproduce the relevant data-shape and lookup behavior without importing
# repository code: the DB lookup is keyed only by TableModelParameters.Model,
# while provider is stored inside the JSON blob.
rows = {
"claude-opus-4-8": {"provider": "anthropic", "supports_fast_mode": True},
}
def get_model_parameters_by_model(model):
return rows.get(model)
def miss_handler(row_key):
row = get_model_parameters_by_model(row_key)
return row
runtime_provider = "vertex"
runtime_model = "claude-opus-4-8"
record = miss_handler(runtime_model)
assert record is not None
assert record["provider"] == "anthropic"
assert record["provider"] != runtime_provider
print({
"requested_provider": runtime_provider,
"requested_model": runtime_model,
"db_lookup_key": runtime_model,
"returned_row_provider": record["provider"],
"cross_provider": True,
})
PYRepository: maximhq/bifrost
Length of output: 710
Scope miss-handler results to the requested provider. GetModelParametersByModel queries only model, so a bare Vertex lookup can return and cache an Anthropic row. Validate the row provider before caching it, or skip the bare candidate.
🤖 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 `@core/providers/utils/modelcapabilities.go` around lines 216 - 236, Update
candidateRowKeys and the GetModelParametersByModel lookup flow so bare model
candidates cannot return rows belonging to another provider; validate each
fetched row’s provider against the requested provider before caching or
returning it, while preserving alias-based lookups for provider-specific keys.
Comment on lines
+274
to
285
| if applied > 0 { | ||
| providerUtils.SetModelCapabilitiesAliases(aliases) | ||
| providerUtils.InvalidateCapabilities() | ||
| // With no miss handler nothing can refill an evicted entry, so the cache | ||
| // is the only copy of the data: drop the bound and seed the full set. | ||
| if !providerUtils.HasCacheMissHandler() { | ||
| providerUtils.SetCapabilitiesCacheCapacity(0) | ||
| providerUtils.BulkSetModelCapabilities(records) | ||
| } | ||
| } else if s.logger != nil { | ||
| s.logger.Warn("model-parameters-sync: no parseable records, keeping existing model capabilities") | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Gate the cache replacement on parsed capability records, not on decoded rows.
applied counts every row that unmarshals, including rows that IsEmptyModelCapabilities rejects at line 234 and rows with no provider. A feed whose rows all decode but carry no capability fields therefore reaches the applied > 0 branch. It then publishes an empty alias index, bumps the generation, and bulk-stores an empty records map. The working capability cache is wiped, and the "no parseable records" guard never fires.
Gate on len(records) so an empty capability feed keeps the previous cache.
A second point: InvalidateCapabilities() runs before BulkSetModelCapabilities(records). In the no-miss-handler mode nothing can refill the cache during that window, so concurrent requests read nil capabilities and fall back to the hardcoded helpers. Seeding before invalidating does not help either, because the bulk store stamps the old generation. Consider adding a bulk-store variant that stamps the next generation and publishes atomically.
🛡️ Proposed fix for the publish guard
- if applied > 0 {
+ if len(records) > 0 {
providerUtils.SetModelCapabilitiesAliases(aliases)
providerUtils.InvalidateCapabilities()
// With no miss handler nothing can refill an evicted entry, so the cache
// is the only copy of the data: drop the bound and seed the full set.
if !providerUtils.HasCacheMissHandler() {
providerUtils.SetCapabilitiesCacheCapacity(0)
providerUtils.BulkSetModelCapabilities(records)
}
} else if s.logger != nil {
- s.logger.Warn("model-parameters-sync: no parseable records, keeping existing model capabilities")
+ s.logger.Warn("model-parameters-sync: no capability records parsed, keeping existing model capabilities")
}🤖 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 `@framework/modelcatalog/datasheet/params.go` around lines 274 - 285, Update
the sync publish guard to use the parsed capability records map, requiring
len(records) to be nonzero before updating aliases, invalidating, or
bulk-storing; otherwise retain the existing cache and warning behavior. In the
no-miss-handler path, add or use a bulk-store operation that stamps the next
capability generation and publishes the seeded records atomically with
invalidation, preventing requests from observing an empty cache during
replacement.
Comment on lines
+114
to
+128
| providerUtils.SetCacheMissHandler(func(rowKey string) *schemas.ModelCapabilities { | ||
| missCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second) | ||
| defer cancel() | ||
| params, err := mc.datasheet.GetModelParametersByModel(missCtx, model) | ||
| params, err := mc.datasheet.GetModelParametersByModel(missCtx, rowKey) | ||
| if err != nil || params == nil { | ||
| return nil | ||
| } | ||
| var p struct { | ||
| MaxOutputTokens *int `json:"max_output_tokens"` | ||
| VertexMultiRegionOnly *bool `json:"vertex_multi_region_only"` | ||
| } | ||
| if err := json.Unmarshal([]byte(params.Data), &p); err != nil { | ||
| var caps schemas.ModelCapabilities | ||
| if err := json.Unmarshal([]byte(params.Data), &caps); err != nil { | ||
| return nil | ||
| } | ||
| if p.MaxOutputTokens == nil && p.VertexMultiRegionOnly == nil { | ||
| if datasheet.IsEmptyModelCapabilities(&caps) { | ||
| return nil | ||
| } | ||
| return &providerUtils.ModelParams{ | ||
| MaxOutputTokens: p.MaxOutputTokens, | ||
| IsVertexMultiRegionOnly: p.VertexMultiRegionOnly, | ||
| } | ||
| return &caps |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A transient store error is cached as "no capabilities" for the whole generation.
The handler returns nil for three distinct outcomes: a store error, a missing row, and an undecodable payload. CapabilitiesFor in core/providers/utils/modelcapabilities.go tombstones every nil result at the current generation. A single DB error or a 3s timeout therefore suppresses capabilities for that model until the next sync bumps the generation. Anthropic requests then fall back to the static token table and the model-name heuristics, with no signal to the operator.
Two changes are needed:
- Log the error and the unmarshal failure. Right now both paths are silent.
- The handler contract returns only
*schemas.ModelCapabilities, so it cannot express "miss, but do not cache". Consider extending the handler signature to return a cacheable flag, so only a confirmed missing row is tombstoned.
🐛 Proposed minimum fix: log the failure paths
providerUtils.SetCacheMissHandler(func(rowKey string) *schemas.ModelCapabilities {
missCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
params, err := mc.datasheet.GetModelParametersByModel(missCtx, rowKey)
- if err != nil || params == nil {
+ if err != nil {
+ logger.Warn("model capability lookup failed for %s: %v", rowKey, err)
+ return nil
+ }
+ if params == nil {
return nil
}
var caps schemas.ModelCapabilities
if err := json.Unmarshal([]byte(params.Data), &caps); err != nil {
+ logger.Warn("model capability payload for %s is not decodable: %v", rowKey, err)
return nil
}🤖 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 `@framework/modelcatalog/main.go` around lines 114 - 128, Update
SetCacheMissHandler and CapabilitiesFor so transient store errors and JSON
unmarshal failures are logged and are not cached as “no capabilities”; extend
the miss-handler result with a cacheable indicator if needed, tombstoning only
confirmed missing or empty rows while preserving successful capability caching.
This was referenced Aug 3, 2026
Merged
18 tasks
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
3 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
Briefly explain the purpose of this PR and the problem it solves.
Changes
Type of change
Affected areas
How to test
Describe the steps to validate this change. Include commands and expected outcomes.
If adding new configs or environment variables, document them here.
Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
Breaking changes
If yes, describe impact and migration instructions.
Related issues
Link related issues and discussions. Example: Closes #123
Security considerations
Note any security implications (auth, secrets, PII, sandboxing, etc.).
Checklist
docs/contributing/README.mdand followed the guidelines