refactor: use AI budget period from deployment config - #27117
#27117refactor: use AI budget period from deployment config#27117ssncferreira merged 1 commit intomaincoder/coder:mainfrom ssncf/aibridge-budget-periodcoder/coder:ssncf/aibridge-budget-periodCopy head branch name to clipboard
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
28e3a8d to
db14366
Compare
db14366 to
a06d471
Compare
| message IsBudgetExceededRequest { | ||
| string user_id = 1; // UUID | ||
| // The spend aggregation window is [period_start, now]. | ||
| google.protobuf.Timestamp period_start = 2; |
There was a problem hiding this comment.
This parameter was incorrectly introduced in #26915: the period should have been derived from the deployment config from the beginning. Since this change was not part of any release, my understanding is that we can safely remove the field without marking it as deprecated. This would only be a problem if we had a server and a client running with different proto versions: an old server that still requires period_start receiving a request from a new client (which no longer sends it) would reject the request. Let me know if that is not the case.
a06d471 to
a35ad30
Compare
| if slices.Contains(AIBudgetPeriods, s) { | ||
| return AIBudgetPeriod(s) | ||
| } | ||
| return AIBudgetPeriodMonth |
There was a problem hiding this comment.
Shouldn't we error when an unrecognised period is defined? It'd be unfortunate if an operator mispelled daily as daly but silently got monthly.
There was a problem hiding this comment.
That is handled by serpent.EnumOf at config load time:
Line 4611 in 199b593
Any typo in the CLI, env var, or YAML is rejected before this function runs. This matches other usages of Enum in configuration values.
| clock quartz.Clock | ||
| } | ||
|
|
||
| func NewServer(lifecycleCtx context.Context, store store, ps pubsub.Pubsub, logger slog.Logger, accessURL string, |
There was a problem hiding this comment.
Nit: the number of args is starting to get unwieldy.
There was a problem hiding this comment.
Good point 👍 This is a bigger change and touches a lot of files, including tests, so will address this in a follow-up PR.
Refactor `aibridgedserver.NewServer` to take an `Options` struct instead of a long list of positional arguments. Follow-up to review feedback in #27117 (comment)
Description
Read the AI budget period from the deployment config on both the RPC server and the
/users/{user}/ai/spendendpoint, instead of hardcodingmonth. Drops theperiod_startRPC parameter that was incorrectly introduced in #26915: the period should have been derived from the deployment config from the start.Changes
codersdk.NewAIBudgetPeriodFromString, mirroringNewAIBudgetPolicyFromString.aibridgedserver.Servertakes aquartz.Clock, readsBudgetPeriodfrom the deployment config at construction, and computes the period window insideIsBudgetExceeded.period_startfromIsBudgetExceededRequestand stop sending it from the daemon.userAISpendStatusendpoint reads the period fromAIBridgeConfig.BudgetPeriodinstead of hardcoding month.Note
Initially generated by Claude Opus 4.7, modified and reviewed by @ssncferreira