Skip to content

Comments

feat(pricing): add provider-level per-model pricing overrides#1726

Open
jerkeyray wants to merge 3 commits intomainfrom
feat/provider-pricing-overrides
Open

feat(pricing): add provider-level per-model pricing overrides#1726
jerkeyray wants to merge 3 commits intomainfrom
feat/provider-pricing-overrides

Conversation

@jerkeyray
Copy link
Contributor

@jerkeyray jerkeyray commented Feb 21, 2026

Summary

Implements provider-level per-model pricing overrides with pattern-based matching and field-level patching over datasheet pricing.

Users can now define pricing_overrides under a provider and override selected pricing fields (token, cache, batch, image, tiered pricing, etc.) without changing remote datasheet sync or core cost formulas.

What Changed

1. Schema & Config Support

  • Added pricing override schema support in provider config:
    • ProviderPricingOverride
    • PricingOverrideMatchType (exact, wildcard, regex)
    • pricing_overrides in provider config

2. Configstore Persistence

  • Added DB migration + column: pricing_overrides_json
  • Added serialization/deserialization in provider table hooks
  • Wired through create/update/get provider config flows

3. Override Engine (Model Catalog)

  • Compile + validate rules
  • Matching precedence:
    • exact > wildcard > regex
    • Request-type filtered overrides beat generic
    • Wildcard specificity tie-break
    • Config-order tie-break
  • Partial patching:
    • Only specified fields override base pricing

4. Pricing Resolution Integration

  • Applied after existing fallback resolution
  • No changes to cost calculation formulas

5. Runtime Lifecycle Wiring

  • Seed overrides on startup
  • Refresh on provider reload/update
  • Clear on provider delete

6. API Validation

  • Match type and pattern validation
  • Request type validation
  • Non-negative pricing field validation

7. UI Support

  • New Pricing Overrides tab in provider config
  • JSON editor + validation

8. Tests Added / Expanded

  • Precedence and tie-break behavior
  • Fallback-order application
  • No-match behavior
  • Delete override behavior
  • Partial patch behavior

Type of Change

  • Feature

Affected Areas

  • core (Go)
  • framework (configstore / modelcatalog)
  • transports (bifrost-http)
  • ui (Next.js)

How to Test

Automated Tests

# framework
cd framework
go test ./modelcatalog
go test ./configstore/...

# transports
cd ../transports
go test ./bifrost-http/server
go test ./bifrost-http/handlers

# ui typecheck
cd ../ui
pnpm exec tsc --noEmit --pretty false

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Provider-level pricing overrides: model-pattern matching (exact, wildcard, regex) with tiered thresholds and many cost types (tokens, characters, images, video, audio, cache, batch).
    • UI: new "Pricing Overrides" tab and JSON editor for managing overrides.
  • Improvements

    • Overrides are validated, persisted, applied to pricing resolution, seeded on startup, and can be set/removed per provider with non-fatal error handling.

Walkthrough

Adds provider-level pricing overrides across schema, DB, config, model-catalog, API handlers, framework wiring, and UI: new types/schemas, migration and persistence, compiled override logic in the model catalog, validation and propagation via HTTP handlers, initialization seeding, and a UI tab to edit overrides.

Changes

Cohort / File(s) Summary
Core schema
core/schemas/provider.go
Adds PricingOverrideMatchType (exact,wildcard,regex), ProviderPricingOverride struct with many tiered pricing fields, and PricingOverrides on ProviderConfig.
Config store & DB
framework/configstore/clientconfig.go, framework/configstore/migrations.go, framework/configstore/rdb.go, framework/configstore/tables/provider.go
Adds PricingOverrides field propagation, JSON column pricing_overrides_json with (de)serialization in TableProvider, includes field in redaction/hash, and a migration to add the column.
Model catalog
framework/modelcatalog/main.go, framework/modelcatalog/overrides.go, framework/modelcatalog/overrides_test.go, framework/modelcatalog/main_test.go
Introduces compiled overrides store (compiledOverrides) with overridesMu, SetProviderPricingOverrides and DeleteProviderPricingOverrides, compilation/validation (exact/wildcard/regex), selection/patching logic, and tests.
Pricing lookup
framework/modelcatalog/pricing.go
Centralizes multi-layer pricing resolution into resolvePricingEntryLocked and applies pricing overrides after base resolution.
API handlers
transports/bifrost-http/handlers/providers.go
Extends add/update/get provider flows to accept and validate pricing_overrides, persists them, calls ModelCatalog.SetProviderPricingOverrides, includes overrides in responses, and adds validation helpers.
Framework wiring
transports/bifrost-http/lib/config.go, transports/bifrost-http/server/server.go
Seeds/applies pricing overrides during model-catalog init/reload/force-reload, clears overrides on provider removal, and logs non-fatal errors when seeding.
Config schema
transports/config.schema.json
Adds provider_pricing_override schema, enums for match/request types, and pricing_overrides array on provider variants (provider, bedrock, azure, vertex).
UI types & validation
ui/lib/types/config.ts, ui/lib/types/schemas.ts
Adds TypeScript types and Zod schemas for pricing overrides (match/request types, numeric fields, superRefine pattern/regex validation); integrates into provider add/update schemas.
UI components
ui/app/workspace/providers/dialogs/providerConfigSheet.tsx, ui/app/workspace/providers/fragments/index.ts, ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx
Adds "Pricing Overrides" tab and PricingOverridesFormFragment JSON editor with validation, dirty state tracking, RBAC-aware save/reset, and server update integration.

Sequence Diagram

sequenceDiagram
    participant UI as UI (Pricing Overrides Form)
    participant API as Bifrost HTTP Handler
    participant ConfigStore as Config Store
    participant DB as Database
    participant ModelCatalog as Model Catalog

    UI->>API: POST/PUT provider (with pricing_overrides)
    API->>API: validatePricingOverrides()
    API->>ConfigStore: UpdateProvider (includes PricingOverrides)
    ConfigStore->>DB: Persist pricing_overrides_json
    API->>ModelCatalog: SetProviderPricingOverrides(provider, overrides)
    ModelCatalog->>ModelCatalog: Compile & store compiledOverrides
    API-->>UI: Return ProviderResponse (includes pricing_overrides)

    Client->>API: Request pricing for model
    API->>ModelCatalog: getPricing(provider, model, requestType)
    ModelCatalog->>ModelCatalog: resolvePricingEntryLocked()
    ModelCatalog->>ModelCatalog: selectBestOverride() / apply override patch
    ModelCatalog-->>API: Patched pricing
    API-->>Client: Return pricing
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I hopped through patterns, exact and wild,
I stitched token costs with a twitch and a smile,
From JSON to DB, into catalog's care,
Overrides compiled — I guard them with flair! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main feature being added: provider-level per-model pricing overrides with pattern-based matching.
Description check ✅ Passed The PR description is comprehensive, covering summary, changes across all layers (schema, configstore, override engine, pricing, UI, tests), affected areas, and testing instructions.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/provider-pricing-overrides

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1726

@jerkeyray jerkeyray marked this pull request as ready for review February 21, 2026 22:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
framework/modelcatalog/main.go (1)

694-703: ⚠️ Potential issue | 🔴 Critical

NewTestCatalog is missing compiledOverrides initialization — will panic on override operations.

SetProviderPricingOverrides and applyPricingOverrides write to / read from mc.compiledOverrides. Since this exported constructor doesn't initialize the map, any caller using it (e.g., external test packages) will get a nil-map write panic.

🐛 Proposed fix
 func NewTestCatalog(baseModelIndex map[string]string) *ModelCatalog {
 	if baseModelIndex == nil {
 		baseModelIndex = make(map[string]string)
 	}
 	return &ModelCatalog{
-		modelPool:      make(map[schemas.ModelProvider][]string),
-		baseModelIndex: baseModelIndex,
-		pricingData:    make(map[string]configstoreTables.TableModelPricing),
-		done:           make(chan struct{}),
+		modelPool:         make(map[schemas.ModelProvider][]string),
+		baseModelIndex:    baseModelIndex,
+		pricingData:       make(map[string]configstoreTables.TableModelPricing),
+		compiledOverrides: make(map[schemas.ModelProvider][]compiledProviderPricingOverride),
+		done:              make(chan struct{}),
 	}
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/modelcatalog/main.go` around lines 694 - 703, NewTestCatalog
currently returns a ModelCatalog without initializing the compiledOverrides map,
causing nil-map write panics when SetProviderPricingOverrides or
applyPricingOverrides touch mc.compiledOverrides; update NewTestCatalog to
initialize the compiledOverrides field to an empty map of the same type those
methods expect (i.e., make(map[...]{...}) using the exact key/value types used
by compiledOverrides in the ModelCatalog) so callers (including external tests)
won't panic when overrides are applied.
🧹 Nitpick comments (4)
core/schemas/provider.go (1)

393-399: PricingOverrides added to ProviderConfig — verify deep-copy needs.

The CheckAndSetDefaults() method deep-copies ExtraHeaders (a map) to prevent data races. PricingOverrides is a slice of structs containing *float64 pointers. If the same ProviderConfig is shared across goroutines and overrides are mutated (e.g., appended to), the slice header could be a shared-state issue. Since the coding guideline specifically calls out map fields, this is lower risk, but worth noting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/schemas/provider.go` around lines 393 - 399, ProviderConfig now contains
a PricingOverrides []ProviderPricingOverride which may be shared across
goroutines; update CheckAndSetDefaults in ProviderConfig to perform a deep copy
of PricingOverrides: allocate a new slice, copy each ProviderPricingOverride
struct into the new slice, and for any pointer fields inside
ProviderPricingOverride (e.g., *float64) allocate new memory and copy the
pointed values so the resulting slice and pointer targets are independent;
reference the PricingOverrides field, the ProviderPricingOverride type, and the
ProviderConfig.CheckAndSetDefaults method when making this change.
framework/modelcatalog/overrides.go (1)

86-92: literalChars for regex patterns is the raw pattern length, not actual literal characters.

For regex match type, literalChars is set to len(pattern) which includes metacharacters (^, $, ., *, etc.). Since literalChars is only used as a tie-breaker among overrides of the same match type, and regex is the lowest priority, this is unlikely to cause issues in practice. However, it could produce surprising tie-break results if two regex overrides compete (e.g., ^gpt-.*$ vs ^gpt-4o$ — the longer pattern wins, not the more specific one).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/modelcatalog/overrides.go` around lines 86 - 92, The literalChars
field is being set to len(pattern) for regex matches, which counts
metacharacters and misorders tie-breaks; update the regex branch in
compiledProviderPricingOverride construction to compute literalChars as the
number of literal characters in the regex (not raw pattern length). Parse the
pattern with regexp/syntax (syntax.Parse) and walk the parsed AST to count
Rune/Char/Literal nodes (and count characters in escaped sequences) while
ignoring metacharacter/operator nodes (^ $ . * + ? | [] {} () etc.), then assign
that count to result.literalChars instead of len(pattern); keep the existing
regexp.Compile and error handling for re and retain result.regex assignment.
framework/modelcatalog/overrides_test.go (1)

25-27: Consider using schemas.Ptr() instead of the local floatPtr helper.

The floatPtr function at lines 25–27 is redundant. The codebase convention is to use schemas.Ptr() (or bifrost.Ptr()) for pointer creation, which is already available through your imports and does exactly the same thing.

♻️ Proposed change
-func floatPtr(v float64) *float64 {
-	return &v
-}

Replace the 4 calls to floatPtr(...) with schemas.Ptr(...):

  • Line 337: CacheReadInputImageTokenCost: floatPtr(0.2)schemas.Ptr(0.2)
  • Line 343: InputCostPerToken: floatPtr(3)schemas.Ptr(3)
  • Line 344: CacheReadInputTokenCost: floatPtr(0.9)schemas.Ptr(0.9)
  • Line 345: OutputCostPerImageToken: floatPtr(1.2)schemas.Ptr(1.2)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/modelcatalog/overrides_test.go` around lines 25 - 27, Remove the
redundant local helper floatPtr and replace its usages with the shared helper
schemas.Ptr: delete the floatPtr function and change calls like floatPtr(0.2) to
schemas.Ptr(0.2) (and similarly for floatPtr(3), floatPtr(0.9), floatPtr(1.2));
ensure replacements are applied where fields like CacheReadInputImageTokenCost,
InputCostPerToken, CacheReadInputTokenCost, and OutputCostPerImageToken are
initialized so imports providing schemas.Ptr are used consistently.
ui/app/workspace/providers/dialogs/providerConfigSheet.tsx (1)

11-11: Consider importing from the barrel export for consistency with Line 8.

PricingOverridesFormFragment is now re-exported from ../fragments/index.ts. The three fragments imported on Line 8 all come from the barrel; consolidating here avoids a mixed pattern within the same file.

♻️ Proposed refactor
- import { ApiStructureFormFragment, GovernanceFormFragment, ProxyFormFragment } from "../fragments";
- import { NetworkFormFragment } from "../fragments/networkFormFragment";
- import { PerformanceFormFragment } from "../fragments/performanceFormFragment";
- import { PricingOverridesFormFragment } from "../fragments/pricingOverridesFormFragment";
+ import {
+   ApiStructureFormFragment,
+   GovernanceFormFragment,
+   NetworkFormFragment,
+   PerformanceFormFragment,
+   PricingOverridesFormFragment,
+   ProxyFormFragment,
+ } from "../fragments";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/app/workspace/providers/dialogs/providerConfigSheet.tsx` at line 11, The
import for PricingOverridesFormFragment should be changed to use the barrel
re-export like the other fragments to keep imports consistent; locate the import
statement that currently reads "import { PricingOverridesFormFragment } from
\"../fragments/pricingOverridesFormFragment\"" and update it to import from the
barrel (the same module used by the other three fragments) so all fragment
imports come from "../fragments" (i.e., import { PricingOverridesFormFragment }
from "../fragments").
🤖 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/handlers/providers.go`:
- Around line 227-229: Change the error message passed to SendError so it starts
with a lowercase letter per Go error conventions: replace "Invalid pricing
overrides: %v" with "invalid pricing overrides: %v" wherever SendError is called
after validatePricingOverrides(payload.PricingOverrides) and the other
occurrence later in the file (both calls that pass the formatted error string).
Ensure you update both places that construct that message so the error string
begins with "invalid".

In `@transports/bifrost-http/lib/config.go`:
- Line 2170: When rebuilding the provider config in loadDefaultProviders,
include the PricingOverrides field so
applyProviderPricingOverrides(config.ModelCatalog, config.Providers) can see and
apply stored overrides; update the ProviderConfig construction in
loadDefaultProviders to copy the existing PricingOverrides from the persisted
config (or config.Providers entry) into the new ProviderConfig instance so
overrides are not dropped on restart.

In `@transports/config.schema.json`:
- Around line 1620-1668: The request_types array in the
provider_pricing_override schema currently accepts any string; tighten it by
adding an enum of allowed request types and referencing that enum in the
request_types items schema. Update the "provider_pricing_override" ->
"request_types" -> "items" to use a $ref to a new or existing definition (e.g.,
add a "$defs/request_type" or "$defs/pricing_request_type" listing the supported
strings such as "chat", "completion", "embedding", "image", "audio", "video",
etc.), so the schema enforces only supported values across the stack.

In `@ui/app/workspace/providers/dialogs/providerConfigSheet.tsx`:
- Around line 39-42: The new "pricing-overrides" tab trigger is missing a
data-testid; update the shared TabsTrigger rendering (in the ProviderConfigSheet
component where tabs are mapped/rendered) to include a data-testid attribute
built from the tab id using the project's convention (e.g.,
data-testid={`provider-tab-${tab.id}`}), so every tab (including the
"pricing-overrides" tab from the tabs array) gets a consistent test id; ensure
the attribute is added to the TabsTrigger element (the shared template that
renders each tab) rather than per-tab instances.

In `@ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx`:
- Around line 105-108: The Reset Button in pricingOverridesFormFragment (the
<Button> element that uses onClick={onReset} and props
disabled={!hasUpdateProviderAccess || !isDirty}) is missing a data-testid; add a
data-testid attribute (e.g., data-testid="pricing-overrides-reset" or matching
existing workspace test id conventions) to that Button so all interactive
workspace elements include a test id while keeping the existing onClick,
variant, type and disabled logic unchanged.
- Around line 31-34: The effect currently resets overridesJSON and clears
validation errors whenever initialValue or provider.name changes, which will
clobber in-progress edits; modify the useEffect (the effect that calls
setOverridesJSON and setValidationError) to first check the form's dirty state
(e.g., form.formState.isDirty) and only perform the reset when the form is not
dirty (clean), otherwise skip the update so user edits are preserved; ensure you
reference initialValue and provider.name as the triggering deps but gate the
reset behind the form.formState.isDirty check.

---

Outside diff comments:
In `@framework/modelcatalog/main.go`:
- Around line 694-703: NewTestCatalog currently returns a ModelCatalog without
initializing the compiledOverrides map, causing nil-map write panics when
SetProviderPricingOverrides or applyPricingOverrides touch mc.compiledOverrides;
update NewTestCatalog to initialize the compiledOverrides field to an empty map
of the same type those methods expect (i.e., make(map[...]{...}) using the exact
key/value types used by compiledOverrides in the ModelCatalog) so callers
(including external tests) won't panic when overrides are applied.

---

Nitpick comments:
In `@core/schemas/provider.go`:
- Around line 393-399: ProviderConfig now contains a PricingOverrides
[]ProviderPricingOverride which may be shared across goroutines; update
CheckAndSetDefaults in ProviderConfig to perform a deep copy of
PricingOverrides: allocate a new slice, copy each ProviderPricingOverride struct
into the new slice, and for any pointer fields inside ProviderPricingOverride
(e.g., *float64) allocate new memory and copy the pointed values so the
resulting slice and pointer targets are independent; reference the
PricingOverrides field, the ProviderPricingOverride type, and the
ProviderConfig.CheckAndSetDefaults method when making this change.

In `@framework/modelcatalog/overrides_test.go`:
- Around line 25-27: Remove the redundant local helper floatPtr and replace its
usages with the shared helper schemas.Ptr: delete the floatPtr function and
change calls like floatPtr(0.2) to schemas.Ptr(0.2) (and similarly for
floatPtr(3), floatPtr(0.9), floatPtr(1.2)); ensure replacements are applied
where fields like CacheReadInputImageTokenCost, InputCostPerToken,
CacheReadInputTokenCost, and OutputCostPerImageToken are initialized so imports
providing schemas.Ptr are used consistently.

In `@framework/modelcatalog/overrides.go`:
- Around line 86-92: The literalChars field is being set to len(pattern) for
regex matches, which counts metacharacters and misorders tie-breaks; update the
regex branch in compiledProviderPricingOverride construction to compute
literalChars as the number of literal characters in the regex (not raw pattern
length). Parse the pattern with regexp/syntax (syntax.Parse) and walk the parsed
AST to count Rune/Char/Literal nodes (and count characters in escaped sequences)
while ignoring metacharacter/operator nodes (^ $ . * + ? | [] {} () etc.), then
assign that count to result.literalChars instead of len(pattern); keep the
existing regexp.Compile and error handling for re and retain result.regex
assignment.

In `@ui/app/workspace/providers/dialogs/providerConfigSheet.tsx`:
- Line 11: The import for PricingOverridesFormFragment should be changed to use
the barrel re-export like the other fragments to keep imports consistent; locate
the import statement that currently reads "import { PricingOverridesFormFragment
} from \"../fragments/pricingOverridesFormFragment\"" and update it to import
from the barrel (the same module used by the other three fragments) so all
fragment imports come from "../fragments" (i.e., import {
PricingOverridesFormFragment } from "../fragments").

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
framework/modelcatalog/overrides_test.go (1)

47-65: Prefer pointer helpers over address-of literals for overrides.

Use a pointer helper (already in scope via schemas.Ptr) instead of &value for consistency with repo conventions.

♻️ Example (apply similarly to other override literals)
-			InputCostPerToken: &wildcard,
+			InputCostPerToken: schemas.Ptr(wildcard),

Based on learnings: In the maximhq/bifrost repository, prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically. Apply this consistently across all code paths, including test utilities, to improve consistency and readability. Replace occurrences of &value where a *T is expected with bifrost.Ptr(value) (or an equivalent call) and ensure the function is in scope and used correctly for the target pointer type.

Also applies to: 86-99, 118-132, 151-158, 177-184, 205-212, 234-241, 267-280, 299-312

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@framework/modelcatalog/overrides_test.go` around lines 47 - 65, Replace
address-of literals used for pointer fields in the pricing override test with
the pointer helper available in scope (schemas.Ptr) for consistency: where the
diff shows &wildcard, &regex, &exact (and the other occurrences noted), call
schemas.Ptr(wildcard), schemas.Ptr(regex), schemas.Ptr(exact) (and similarly for
other numeric/string variables used as *T in SetProviderPricingOverrides calls).
Update the literals passed to mc.SetProviderPricingOverrides and any other
override structs (e.g., InputCostPerToken, OutputCostPerToken, etc.) to use
schemas.Ptr(...) instead of &value so the tests use the repository pointer
helper consistently.
ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx (1)

3-8: Use relative imports for ui/lib modules.

Switch @/lib/* imports to relative paths from this file.

♻️ Suggested import adjustment
-import { getErrorMessage, setProviderFormDirtyState, useAppDispatch } from "@/lib/store";
-import { useUpdateProviderMutation } from "@/lib/store/apis/providersApi";
-import { ModelProvider } from "@/lib/types/config";
-import { providerPricingOverrideSchema } from "@/lib/types/schemas";
+import { getErrorMessage, setProviderFormDirtyState, useAppDispatch } from "../../../../lib/store";
+import { useUpdateProviderMutation } from "../../../../lib/store/apis/providersApi";
+import { ModelProvider } from "../../../../lib/types/config";
+import { providerPricingOverrideSchema } from "../../../../lib/types/schemas";

As per coding guidelines: ui/**/*.{ts,tsx}: TypeScript/React code must use Prettier formatting and follow Next.js App Router patterns. Use relative imports from the ui/lib directory for constants, utilities, and types.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx` around
lines 3 - 8, Replace the absolute imports that reference "@/lib/*" with relative
imports that point to the ui/lib directory for the utilities and types used in
this fragment: getErrorMessage, setProviderFormDirtyState, useAppDispatch,
useUpdateProviderMutation, ModelProvider, and providerPricingOverrideSchema;
update the import statements in pricingOverridesFormFragment.tsx so they use
relative paths instead of "@/lib/..." to comply with the ui/ project import
conventions and Prettier/Next.js App Router patterns.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx`:
- Around line 29-38: The effect that resets the form is gating on hasUserEdits,
which stays true after the first edit and blocks sync when provider changes;
change the guard to use isDirty (computed as hasUserEdits && overridesJSON !==
initialValue) so the form only blocks reset when truly dirty. Update the
useEffect callback to `if (isDirty) return;` and ensure the dependency array
includes isDirty (and provider.name) so when overridesJSON or initialValue
revert and isDirty flips false the effect will run and call
setOverridesJSON(initialValue) and setValidationError("").

---

Nitpick comments:
In `@framework/modelcatalog/overrides_test.go`:
- Around line 47-65: Replace address-of literals used for pointer fields in the
pricing override test with the pointer helper available in scope (schemas.Ptr)
for consistency: where the diff shows &wildcard, &regex, &exact (and the other
occurrences noted), call schemas.Ptr(wildcard), schemas.Ptr(regex),
schemas.Ptr(exact) (and similarly for other numeric/string variables used as *T
in SetProviderPricingOverrides calls). Update the literals passed to
mc.SetProviderPricingOverrides and any other override structs (e.g.,
InputCostPerToken, OutputCostPerToken, etc.) to use schemas.Ptr(...) instead of
&value so the tests use the repository pointer helper consistently.

In `@ui/app/workspace/providers/fragments/pricingOverridesFormFragment.tsx`:
- Around line 3-8: Replace the absolute imports that reference "@/lib/*" with
relative imports that point to the ui/lib directory for the utilities and types
used in this fragment: getErrorMessage, setProviderFormDirtyState,
useAppDispatch, useUpdateProviderMutation, ModelProvider, and
providerPricingOverrideSchema; update the import statements in
pricingOverridesFormFragment.tsx so they use relative paths instead of
"@/lib/..." to comply with the ui/ project import conventions and
Prettier/Next.js App Router patterns.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
transports/bifrost-http/handlers/providers.go (1)

1052-1089: Replace non-deterministic map iteration with an ordered field list.

validatePricingOverrideNonNegativeFields uses map[string]*float64 to iterate over optional cost fields. Go map iteration order is random, so if multiple fields are negative, the reported field name varies between calls, making errors harder to debug consistently.

All 26 *float64 fields from schemas.ProviderPricingOverride are covered in validation, but consider using an ordered slice of field structs instead:

♻️ Proposed refactor
+type pricingField struct {
+	name string
+	ptr  *float64
+}
+
 func validatePricingOverrideNonNegativeFields(index int, override schemas.ProviderPricingOverride) error {
-	optionalValues := map[string]*float64{
-		"input_cost_per_token":                              override.InputCostPerToken,
-		"output_cost_per_token":                             override.OutputCostPerToken,
-		// ... 26 total entries ...
-		"cache_read_input_image_token_cost":                 override.CacheReadInputImageTokenCost,
-	}
-
-	for fieldName, value := range optionalValues {
-		if value != nil && *value < 0 {
-			return fmt.Errorf("override[%d]: %s must be non-negative", index, fieldName)
-		}
-	}
+	fields := []pricingField{
+		{"input_cost_per_token", override.InputCostPerToken},
+		{"output_cost_per_token", override.OutputCostPerToken},
+		// ... ordered list of all 26 fields ...
+		{"cache_read_input_image_token_cost", override.CacheReadInputImageTokenCost},
+	}
+
+	for _, f := range fields {
+		if f.ptr != nil && *f.ptr < 0 {
+			return fmt.Errorf("override[%d]: %s must be non-negative", index, f.name)
+		}
+	}
 
 	return nil
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@transports/bifrost-http/handlers/providers.go` around lines 1052 - 1089, The
function validatePricingOverrideNonNegativeFields uses an unordered map to check
many optional *float64 fields which causes non-deterministic error reporting;
replace the map with a deterministic ordered slice (e.g., []struct{name string;
val *float64}) listing each ProviderPricingOverride field in the desired order
and iterate that slice to check for nil/negative values, returning the formatted
error on the first failure; update references to the fields (the 26 keys
currently used) in the new slice so behaviour and messages remain identical but
deterministic.
🤖 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/handlers/providers.go`:
- Around line 1052-1089: The function validatePricingOverrideNonNegativeFields
uses an unordered map to check many optional *float64 fields which causes
non-deterministic error reporting; replace the map with a deterministic ordered
slice (e.g., []struct{name string; val *float64}) listing each
ProviderPricingOverride field in the desired order and iterate that slice to
check for nil/negative values, returning the formatted error on the first
failure; update references to the fields (the 26 keys currently used) in the new
slice so behaviour and messages remain identical but deterministic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant