-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: apply pricing schedules in cost engine #6533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
qixiangyang
wants to merge
13
commits into
maximhq:dev
Choose a base branch
from
qixiangyang:feat/pricing-schedule-cost-engine
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
446dc59
feat: persist billing attempt start time
qixiangyang d23be53
fix: isolate billing attempt timestamps
qixiangyang 16b1aeb
fix: cancel nil-context requests
qixiangyang a55127b
fix: cancel owned streaming contexts
qixiangyang 9b502b9
fix: require context for streaming requests
qixiangyang 90c469f
fix: isolate and restore billing attempt times
qixiangyang 3fcee3b
feat: add generic pricing time schedules
qixiangyang d81b556
fix: allow deterministic pricing schedule overlaps
qixiangyang 93ebd67
fix: harden pricing schedule rules
qixiangyang 13bd375
fix: reject non-finite pricing multipliers
qixiangyang 9e5d987
fix: align schedule lookup with pricing candidates
qixiangyang bc2680b
fix: preserve image stream attempt timestamps on errors
qixiangyang f04dac3
fix: stamp recovered stream billing time
qixiangyang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| package bifrost | ||
|
|
||
| import ( | ||
| "context" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/maximhq/bifrost/core/schemas" | ||
| ) | ||
|
|
||
| // TestClearBillingAttemptStartTimeMasksInheritedValue ensures a child context | ||
| // cannot expose a stale attempt timestamp inherited from its parent. | ||
| func TestClearBillingAttemptStartTimeMasksInheritedValue(t *testing.T) { | ||
| parent := schemas.NewBifrostContext(context.Background(), schemas.NoDeadline) | ||
| stamp := time.Date(2026, 8, 27, 9, 0, 0, 0, time.UTC) | ||
| parent.SetBillingAttemptStartTime(stamp) | ||
|
|
||
| child := schemas.NewBifrostContext(parent, schemas.NoDeadline) | ||
| if got, ok := child.Value(schemas.BifrostContextKeyBillingAttemptStartTime).(time.Time); !ok || !got.Equal(stamp) { | ||
| t.Fatalf("child should inherit the parent timestamp, got %#v", got) | ||
| } | ||
|
|
||
| child.ClearBillingAttemptStartTime() | ||
| if _, ok := child.Value(schemas.BifrostContextKeyBillingAttemptStartTime).(time.Time); ok { | ||
| t.Fatal("cleared child context exposed an inherited billing timestamp") | ||
| } | ||
| } | ||
|
|
||
| // TestNilUnaryRequestsUseIsolatedContexts pins that nil-context callers no longer | ||
| // share the mutable Bifrost-wide context used for request-scoped pricing state. | ||
| func TestNilUnaryRequestsUseIsolatedContexts(t *testing.T) { | ||
| newBifrost := func() *Bifrost { | ||
| return &Bifrost{ | ||
| ctx: schemas.NewBifrostContext(context.Background(), schemas.NoDeadline), | ||
| logger: NewNoOpLogger(), | ||
| account: NewMockAccount(), | ||
| providerMutexes: sync.Map{}, | ||
| } | ||
| } | ||
| req := &schemas.BifrostChatRequest{ | ||
| Provider: schemas.OpenAI, | ||
| Model: "test-model", | ||
| Input: []schemas.ChatMessage{{}}, | ||
| } | ||
| bifrost := newBifrost() | ||
| bifrost.bifrostRequestPool = sync.Pool{New: func() interface{} { return &schemas.BifrostRequest{} }} | ||
| bifrost.llmPlugins.Store(&[]schemas.LLMPlugin{}) | ||
| bifrost.mcpPlugins.Store(&[]schemas.MCPPlugin{}) | ||
| bifrost.pluginPipelinePool = sync.Pool{New: func() interface{} { | ||
| return &PluginPipeline{ | ||
| preHookErrors: make([]error, 0), | ||
| postHookErrors: make([]error, 0), | ||
| } | ||
| }} | ||
| bifrost.tracer.Store(&tracerWrapper{tracer: &schemas.NoOpTracer{}}) | ||
| _, err := bifrost.ChatCompletionRequest(nil, req) | ||
| if err == nil || err.Error == nil { | ||
| t.Fatalf("expected request to fail without providers, got %#v", err) | ||
| } | ||
| if bifrost.ctx.Value(schemas.BifrostContextKeyRequestStartTime) != nil { | ||
| t.Fatal("nil unary caller wrote a request start time to the shared Bifrost context") | ||
| } | ||
| if _, ok := bifrost.ctx.Value(schemas.BifrostContextKeyBillingAttemptStartTime).(time.Time); ok { | ||
| t.Fatal("nil unary caller left a billing timestamp on the shared Bifrost context") | ||
| } | ||
| if v, ok := bifrost.ctx.Value(schemas.BifrostContextKeyRequestID).(string); ok && v != "" { | ||
| t.Fatalf("nil unary caller changed the shared Bifrost request ID: %q", v) | ||
| } | ||
| } |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: maximhq/bifrost
Length of output: 153
🏁 Script executed:
Repository: maximhq/bifrost
Length of output: 50372
🏁 Script executed:
Repository: maximhq/bifrost
Length of output: 50372
🏁 Script executed:
Repository: maximhq/bifrost
Length of output: 47659
🏁 Script executed:
Repository: maximhq/bifrost
Length of output: 18733
Make the nil-context policy consistent across public streaming entrypoints.
ResponsesStreamRequestdereferencesctxbeforehandleStreamRequest, so a nil context can panic.ResponsesRetrieveStreamRequestreplaces nil withbifrost.ctx, so it still accepts nil. Add a common guard before context-dependent validation and document the breaking contract.🤖 Prompt for AI Agents