feat(coderd): accumulate user daily AI spend on token usage - #26741
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
cc3228d to
314d33b
Compare
bb18cf7 to
b2c90c2
Compare
c39f028 to
8b7c712
Compare
8b7c712 to
111133e
Compare
|
/coder-agents-review |
|
Chat: Review posted | View chat Review history
deep-review v0.9.0 | Round 1 | Last posted: Round 1, 7 findings (1 P2, 3 P3, 1 Nit, 2 Note), COMMENT. Review Finding inventoryFindings
Round logRound 1Panel. Netero first pass clean (P3 + Nit). Panel: Bisky, Chopper, Ging-Go, Gon, Hisoka, Kite, Komugi, Leorio, Mafu-san, Mafuuu, Meruem, Pariston, Knov (wildcard). 1 P2, 3 P3, 2 Nit, 2 Note. 1 dropped (CRF-5 subsumed by CRF-1). Reviewed against b2c90c2..111133e. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean design. The transaction wrapping is correct (both writes use tx, not s.store), the skip condition covers all four budget-by-price states, and the test matrix exercises every branch, both happy-path resolution strategies (group lookup + user override), and both error paths. The integration test verifies the full write path through dbauthz and reads the spend row back from a real database. 80% test density, 60 production LOC, no scope drift.
As Pariston put it: "I tried to build a case against this change and could not."
Severity count: 1 P2, 3 P3, 2 Nit, 2 Note.
The P2 is the doc comment on recordTokenUsageAndSpend claiming unconditional accumulation when the function conditionally skips. The remaining findings are comment quality, a missing log field, and a minor code duplication.
🤖 This review was automatically generated with Coder Agents.
| @@ -1176,71 +1178,16 @@ func TestRecordTokenUsage(t *testing.T) { | |||
| }, | |||
There was a problem hiding this comment.
Note [CRF-8] Test comment bloat is a pattern in this PR. 14 of 17 test comments restate the test name (// Budget resolves via group lookup, model is priced. on a test named "with effective group and cost"), the mock expectation (// Spend update is skipped because cost is NULL. on IncrementUserAIDailySpend.Times(0)), or use more words than needed. The test names are already descriptive and the assertions are clear. Comments that add the skip-reason (which branch of the guard) are the valuable ones; the rest could be trimmed or removed. (Gon)
🤖
There was a problem hiding this comment.
I find these comments relevant, and they helped me when restructuring the tests to make sure all cases are covered. But happy to remove them if they are not useful for others 🙂
111133e to
e769e76
Compare
b2c90c2 to
38c650b
Compare
| EffectiveGroupID: cost.effectiveGroupID.UUID, | ||
| // Day is derived from the record usage request CreatedAt | ||
| // so it matches the token usage row's created_at column. | ||
| Day: dbtime.StartOfDay(createdAt.UTC()), |
There was a problem hiding this comment.
Initially, I was using a new clock for Day, but then realized the request's CreatedAt already carries the time the token usage was created on the aibridge side. To keep the database consistent and the spend Day deterministic from the aibridge_token_usages table (source of truth), I switched to deriving it from CreatedAt. This handles the edge case where token usage is created on the aibridge side on day X, but the request reaches coderd after midnight, so time.Now() would return day X+1. Let me know if you see any issues.
e769e76 to
ce79640
Compare
2fbae8e to
ec306e5
Compare
888ac85 to
dee171f
Compare
ec306e5 to
f1c6469
Compare
dee171f to
6e8896d
Compare
f1c6469 to
13eb001
Compare
6e8896d to
41ad5f2
Compare
13eb001 to
dbb6f44
Compare
41ad5f2 to
ae35c71
Compare
dbb6f44 to
04e366f
Compare
Merge activity
|
04e366f to
4aa77ba
Compare
2 participants
Description
Adds post-response spend accumulation to
RecordTokenUsage.Changes
Depends on #26562
Closes https://linear.app/codercom/issue/AIGOV-427/add-post-response-spend-accumulation
Note
Initially generated by Claude Opus 4.7, modified and reviewed by @ssncferreira