feat(admin): recalculate usage pricing#289
Merged
SantiagoDePolonia merged 12 commits intomainfrom Apr 28, 2026
Merged
Conversation
Add a Settings tool and admin API for recalculating stored usage costs from current model pricing metadata. Support date range, user path, provider/model, and alias filters across SQLite, PostgreSQL, and MongoDB storage backends. Tests: go test ./...; node --test internal/admin/dashboard/static/js/modules/*.test.cjs; curl validation against OpenRouter usage rows.
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds an admin-side usage pricing recalculation feature: new frontend UI and typed-confirmation modal, dashboard JS wiring and tests, a guarded admin POST endpoint, storage-backed recalculation implementations (SQLite/Postgres/Mongo) with factory, config/runtime flag, server/auth changes, and extensive handler/store tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Admin UI
participant DashboardJS as Dashboard JS
participant API as Admin Handler
participant Store as Storage (SQLite/Postgres/Mongo)
UI->>DashboardJS: open pricing UI, select params
UI->>DashboardJS: click "Recalculate Pricing"
DashboardJS->>UI: show typed-confirmation dialog
UI->>DashboardJS: enter "recalculate" and submit
DashboardJS->>DashboardJS: validate typed confirmation
DashboardJS->>API: POST /admin/api/v1/usage/recalculate-pricing (payload)
API->>API: validate confirmation, normalize params, resolve selector/alias
API->>Store: RecalculatePricing(ctx, params, resolver)
Store->>Store: select matching usage entries
Store->>Store: compute per-entry costs via PricingResolver
Store->>Store: update usage records/documents
Store-->>API: return RecalculatePricingResult (counts)
API-->>DashboardJS: respond with summary JSON
DashboardJS->>UI: close dialog, show notice, refresh usage/budgets
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds a "Recalculate Usage Pricing" admin action backed by SQLite, PostgreSQL, and MongoDB recalculators, with date-range, user-path, and provider/model-or-alias filters. It also extracts a reusable typed-confirmation dialog component and refactors the existing budget-reset dialog to use it.
Confidence Score: 4/5Safe to merge for SQLite and PostgreSQL deployments; MongoDB deployments risk partial data corruption on recalculation failure. One P1 finding: the MongoDB recalculator performs individual UpdateByID calls outside a transaction, meaning an error mid-loop leaves cost fields partially updated with no rollback. SQLite and PostgreSQL are implemented correctly with full transactions. Everything else — handler validation, alias resolution, typed-confirmation dialog refactor, tests — is well-implemented. internal/usage/recalculate_pricing_mongodb.go — missing session/transaction wrapping for atomicity. Important Files Changed
Sequence DiagramsequenceDiagram
actor Admin
participant UI as Settings UI (Alpine.js)
participant Dialog as TypedConfirmationDialog
participant API as POST /admin/api/v1/usage/recalculate-pricing
participant Handler as admin.Handler
participant Recalculator as PricingRecalculator (SQLite/PG/Mongo)
participant Registry as ModelRegistry
Admin->>UI: Set filters (date, user_path, selector)
Admin->>UI: Click "Recalculate Pricing"
UI->>Dialog: openTypedConfirmationDialog()
Admin->>Dialog: Type "recalculate" + submit
Dialog->>UI: submitTypedConfirmationDialog() → recalculatePricing()
UI->>API: POST {days/dates, user_path, selector, confirmation}
API->>Handler: RecalculateUsagePricing()
Handler->>Handler: Validate confirmation == "recalculate"
Handler->>Handler: recalculatePricingParams() — parse dates, resolve alias
Handler->>Handler: mutationMu.Lock()
Handler->>Recalculator: RecalculatePricing(ctx, params, registry)
Recalculator->>Recalculator: Query matching usage rows
loop For each row
Recalculator->>Registry: ResolvePricing(model, provider)
Recalculator->>Recalculator: CalculateGranularCost()
Recalculator->>Recalculator: UPDATE usage SET costs... (tx for SQLite/PG)
end
Recalculator-->>Handler: RecalculatePricingResult {matched, recalculated, with_pricing, without_pricing}
Handler-->>API: 200 OK + result JSON
API-->>UI: result
UI->>Dialog: closePricingRecalculateDialog()
UI->>UI: Show summary notice, refresh usage data
Reviews (1): Last reviewed commit: "feat(admin): recalculate usage pricing" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+29
to
+62
| result := RecalculatePricingResult{} | ||
| for cursor.Next(ctx) { | ||
| var row struct { | ||
| ID string `bson:"_id"` | ||
| Model string `bson:"model"` | ||
| Provider string `bson:"provider"` | ||
| Endpoint string `bson:"endpoint"` | ||
| InputTokens int `bson:"input_tokens"` | ||
| OutputTokens int `bson:"output_tokens"` | ||
| RawData map[string]any `bson:"raw_data"` | ||
| } | ||
| if err := cursor.Decode(&row); err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("scan mongodb usage cost row: %w", err) | ||
| } | ||
|
|
||
| update := recalculateEntryCosts(recalculationEntry{ | ||
| ID: row.ID, | ||
| Model: row.Model, | ||
| Provider: row.Provider, | ||
| Endpoint: row.Endpoint, | ||
| InputTokens: row.InputTokens, | ||
| OutputTokens: row.OutputTokens, | ||
| RawData: row.RawData, | ||
| }, resolver) | ||
|
|
||
| if _, err := s.collection.UpdateByID(ctx, update.ID, mongoRecalculationUpdate(update)); err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("update mongodb usage cost %s: %w", update.ID, err) | ||
| } | ||
| updateRecalculatePricingResult(&result, update) | ||
| } | ||
| if err := cursor.Err(); err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("iterate mongodb usage costs for recalculation: %w", err) | ||
| } | ||
| return finalizeRecalculatePricingResult(result), nil |
There was a problem hiding this comment.
The update loop issues individual UpdateByID calls outside a session/transaction. If any call fails after several documents have already been written, there is no rollback path — unlike the SQLite (BeginTx/Commit) and PostgreSQL (pool.Begin/Commit) implementations which both wrap their loops in a transaction. A mid-loop network error or server error leaves the collection in a partially-updated state with some costs at new values and others still at old values, and the caller receives an error with no indication of how many records were actually modified.
Comment on lines
+65
to
+68
| if params.Provider != "" { | ||
| conditions = append(conditions, fmt.Sprintf("(provider = $%d OR provider_name = $%d)", nextIdx, nextIdx+1)) | ||
| args = append(args, params.Provider, params.Provider) | ||
| } |
There was a problem hiding this comment.
nextIdx not incremented after provider condition
nextIdx is consumed as $nextIdx and $nextIdx+1 in the provider OR clause but is never incremented afterward. This works today because the provider filter is always the last condition appended, but it is a latent bug if another condition is added after it in the future. The model filter above correctly increments nextIdx++ after use.
![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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/dashboard/templates/page-settings.html`:
- Around line 147-202: The Usage Pricing Recalculation UI is rendered
unconditionally; gate the entire pricing-recalculate block behind a runtime flag
(e.g., pricingRecalculationEnabled) so the control is hidden/disabled when
recalculation support is absent. Update the template to conditionally render or
disable the section (the div with class "settings-refresh-section
pricing-recalculate-section" and the Recalculate Pricing button) using that
flag, and ensure the front-end handlers referenced
(openPricingRecalculateDialog, pricingRecalculateLoading,
pricingRecalculateNotice, pricingRecalculateError) only run or are callable when
pricingRecalculationEnabled is true; also ensure pricing.js checks the same
runtime flag before opening the dialog or POSTing the recalculation request.
In `@internal/admin/handler_pricing_test.go`:
- Around line 118-137: Add a new test variant to cover the recalculator failure
path: create a mockPricingRecalculator whose RecalculatePricing method returns a
non-nil error, wire it into NewHandler with WithUsagePricingRecalculator, then
call the handler method RecalculateUsagePricing (similar to
TestRecalculateUsagePricingRequiresConfirmation) with a valid confirmation
payload; assert that the HTTP response code maps to an internal server error
(e.g., 500), the response body contains the error message, and that
mockPricingRecalculator.calls == 1 to ensure the recalculator was invoked. Use
the existing test structure (echo.New, httptest.NewRequest/Recorder,
e.NewContext) and the same request path
"/admin/api/v1/usage/recalculate-pricing".
In `@internal/admin/handler.go`:
- Around line 1597-1644: Extract the shared date-parsing logic used in
recalculatePricingDateParams and parseDateRangeParams into a helper (e.g.,
parseDateRange or buildDateRangeParams) that accepts startStr, endStr, days int,
location *time.Location and today time.Time (or returns both start/end and
error), then update recalculatePricingDateParams to call that helper using
req.StartDate, req.EndDate and req.Days and update parseDateRangeParams to call
the same helper with query param values; ensure the helper performs the same
validation/error messages (invalid start_date/end_date format) and preserves
behavior for defaulting to 30 days and setting today as end date.
In `@internal/app/app.go`:
- Around line 817-827: The current code returns an error if
usage.NewPricingRecalculator(store) fails, which blocks the entire admin
surface; instead, catch the error, log or record it, leave pricingRecalculator
nil, and continue startup so only the pricing-recalculation feature is disabled.
Concretely, in the block that calls usage.NewPricingRecalculator(store) (the
pricingRecalculator variable), replace the immediate return on error with code
that logs or notes the error (e.g., via the existing logger or fmt) and sets
pricingRecalculator = nil; ensure callers that perform the recalculation check
for a nil pricingRecalculator and surface an "unsupported" response rather than
assuming it exists.
In `@internal/server/http.go`:
- Line 326: The POST registration for RecalculateUsagePricing is currently
reachable when New() has added /admin/api/v1/* to authSkipPaths (MasterKey ==
""), so require authentication before allowing destructive recalculation: update
the RecalculateUsagePricing handler (cfg.AdminHandler.RecalculateUsagePricing)
to enforce auth/authorization by validating the request context/session or
explicit admin API key check (reusing the same auth logic/middleware used
elsewhere) and return 401/403 if unauthenticated, or alternatively remove this
route from the public adminAPI registration and only register it behind the
authenticated admin middleware; reference New(), authSkipPaths, MasterKey,
adminAPI.POST and cfg.AdminHandler.RecalculateUsagePricing to locate the change.
In `@internal/usage/recalculate_pricing_postgresql.go`:
- Around line 60-68: The Provider condition appends two placeholders using
nextIdx and nextIdx+1 but never advances nextIdx, risking index collisions if
more conditions are added; after appending the provider condition (the block
that uses params.Provider, conditions, args, and nextIdx) increment nextIdx by 2
(e.g., nextIdx += 2) so subsequent conditions use the correct parameter indices.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 450166c5-d71a-4721-9376-2135e095fa20
📒 Files selected for processing (19)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/dashboard.jsinternal/admin/dashboard/static/js/modules/budgets.jsinternal/admin/dashboard/static/js/modules/pricing.jsinternal/admin/dashboard/static/js/modules/pricing.test.cjsinternal/admin/dashboard/templates/layout.htmlinternal/admin/dashboard/templates/page-settings.htmlinternal/admin/dashboard/templates/typed-confirmation-dialog.htmlinternal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/app/app.gointernal/server/http.gointernal/usage/cost.gointernal/usage/factory.gointernal/usage/recalculate_pricing.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_postgresql.gointernal/usage/recalculate_pricing_sqlite.gointernal/usage/recalculate_pricing_sqlite_test.go
Comment on lines
+147
to
+202
| <div class="settings-refresh-section pricing-recalculate-section"> | ||
| <div class="inline-help-section" x-data="{ open: false, copyId: 'pricing-recalculate-help-copy', showLabel: 'Show pricing recalculation help', hideLabel: 'Hide pricing recalculation help', text: 'Recalculate stored usage costs from the current model pricing metadata. Filters are applied to the selected date range, user path subtree, and provider/model selector or alias.' }"> | ||
| <div class="inline-help-title-row"> | ||
| <h3>Usage Pricing Recalculation</h3> | ||
| {{template "inline-help-toggle" .}} | ||
| </div> | ||
| <p :id="copyId" class="inline-help-copy" x-show="open" x-transition.opacity.duration.200ms x-text="text"></p> | ||
| </div> | ||
| <div class="pricing-recalculate-grid"> | ||
| <div class="form-field pricing-recalculate-date-field"> | ||
| <label class="form-field-label" id="pricing-recalculate-date-label">Date Range</label> | ||
| <div aria-labelledby="pricing-recalculate-date-label"> | ||
| {{template "date-picker" .}} | ||
| </div> | ||
| </div> | ||
| <div class="form-field"> | ||
| <label class="form-field-label" for="pricing-recalculate-user-path">User Path</label> | ||
| <input id="pricing-recalculate-user-path" | ||
| class="form-input" | ||
| type="text" | ||
| placeholder="/team/alpha" | ||
| x-model="pricingRecalculateUserPath"> | ||
| </div> | ||
| <div class="form-field"> | ||
| <label class="form-field-label" for="pricing-recalculate-selector">Provider/Model or Alias</label> | ||
| <input id="pricing-recalculate-selector" | ||
| class="form-input" | ||
| type="text" | ||
| placeholder="openai/gpt-4o or smart" | ||
| x-model="pricingRecalculateSelector"> | ||
| </div> | ||
| </div> | ||
| <div class="settings-refresh-actions pricing-recalculate-actions"> | ||
| <button type="button" | ||
| class="pagination-btn pagination-btn-danger pagination-btn-with-icon" | ||
| :disabled="pricingRecalculateLoading" | ||
| :aria-busy="pricingRecalculateLoading ? 'true' : 'false'" | ||
| aria-describedby="pricing-recalculate-help-copy" | ||
| @click="openPricingRecalculateDialog()"> | ||
| <i data-lucide="calculator" class="form-action-icon" aria-hidden="true"></i> | ||
| <span>Recalculate Pricing</span> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <div class="alert alert-success settings-refresh-alert" | ||
| role="status" | ||
| aria-live="polite" | ||
| x-show="pricingRecalculateNotice" | ||
| x-text="pricingRecalculateNotice"></div> | ||
| <div class="alert alert-warning settings-refresh-alert" | ||
| role="alert" | ||
| aria-live="assertive" | ||
| x-show="pricingRecalculateError" | ||
| x-text="pricingRecalculateError"></div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Gate this control on pricing recalculation availability.
Unlike the budget sections, this block renders unconditionally. pricing.js still opens the dialog and POSTs the recalculation request even when usage tracking is disabled or no pricing recalculator was wired, so Settings exposes a dead admin action in those deployments. Please hide or disable this section behind an explicit runtime flag for pricing-recalculation support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/dashboard/templates/page-settings.html` around lines 147 -
202, The Usage Pricing Recalculation UI is rendered unconditionally; gate the
entire pricing-recalculate block behind a runtime flag (e.g.,
pricingRecalculationEnabled) so the control is hidden/disabled when
recalculation support is absent. Update the template to conditionally render or
disable the section (the div with class "settings-refresh-section
pricing-recalculate-section" and the Recalculate Pricing button) using that
flag, and ensure the front-end handlers referenced
(openPricingRecalculateDialog, pricingRecalculateLoading,
pricingRecalculateNotice, pricingRecalculateError) only run or are callable when
pricingRecalculationEnabled is true; also ensure pricing.js checks the same
runtime flag before opening the dialog or POSTing the recalculation request.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/admin/dashboard/templates/page-settings.html (1)
147-202:⚠️ Potential issue | 🟠 MajorGate the pricing recalculation UI behind the runtime capability flag.
This section is still always rendered, so deployments without pricing recalculation support will expose a dead admin action and can still invoke the handler/endpoint. Please hide or disable the whole block with the same availability check used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/dashboard/templates/page-settings.html` around lines 147 - 202, The pricing recalculation UI (the div with class "settings-refresh-section pricing-recalculate-section" and its related alerts) is always rendered but must be gated by the runtime capability flag used elsewhere; wrap this entire block and the alert divs with the same availability check (server/template-side conditional or an x-show bound to the capability) so deployments without the feature neither render nor expose the action. Also ensure the Recalculate Pricing button click handler openPricingRecalculateDialog() and related state variables (pricingRecalculateLoading, pricingRecalculateNotice, pricingRecalculateError) are only reachable when the capability is present (e.g., disable the button or skip binding when not available) to prevent dead/invocable UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/dashboard/static/css/dashboard.css`:
- Around line 3870-3875: The override for .pricing-recalculate-date-field
.date-picker-dropdown prevents the mobile .date-picker-dropdown fixed-bottom
behavior; wrap this rule in a desktop-only media query so it only applies on
wider screens (e.g. `@media` (min-width: 768px) { .pricing-recalculate-date-field
.date-picker-dropdown { top: auto; bottom: calc(100% + 6px); right: auto; left:
0; } }), or use the project’s existing desktop breakpoint if different, ensuring
mobile screens keep the shared .date-picker-dropdown behavior.
---
Duplicate comments:
In `@internal/admin/dashboard/templates/page-settings.html`:
- Around line 147-202: The pricing recalculation UI (the div with class
"settings-refresh-section pricing-recalculate-section" and its related alerts)
is always rendered but must be gated by the runtime capability flag used
elsewhere; wrap this entire block and the alert divs with the same availability
check (server/template-side conditional or an x-show bound to the capability) so
deployments without the feature neither render nor expose the action. Also
ensure the Recalculate Pricing button click handler
openPricingRecalculateDialog() and related state variables
(pricingRecalculateLoading, pricingRecalculateNotice, pricingRecalculateError)
are only reachable when the capability is present (e.g., disable the button or
skip binding when not available) to prevent dead/invocable UI.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: fed347b4-a264-4d49-9b1e-763d5f150b85
📒 Files selected for processing (2)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/templates/page-settings.html
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/handler_pricing_test.go`:
- Around line 120-163: Add two tests exercising the missing branches for the
RecalculateUsagePricing handler: (1) create
TestRecalculateUsagePricingFeatureUnavailable that constructs the handler with
either no usageRecalculator (omit WithUsagePricingRecalculator) or a nil model
registry (pass nil to NewHandler) then POST `{"confirmation":"recalculate"}` and
assert HTTP 503 and that the mock reallocator was not called; (2) create
TestRecalculateUsagePricingInvalidSelector that uses
WithUsagePricingRecalculator(recalculator) but POSTs a body with an invalid
selector value (e.g. `{"confirmation":"recalculate","selector":"invalid"}`) and
assert HTTP 400 and an appropriate error message; reference
RecalculateUsagePricing, NewHandler, WithUsagePricingRecalculator, and
mockPricingRecalculator to locate the code under test.
In `@internal/admin/handler.go`:
- Around line 691-694: The Swagger comment block for the POST
/admin/api/v1/usage/recalculate-pricing endpoint is missing a 500 failure
annotation; update the endpoint's comment above the handler (the block that
contains the `@Router` /admin/api/v1/usage/recalculate-pricing [post]) to include
"@Failure 500 {object} core.GatewayError" so the generated docs match
the handler's behavior (the handler returns http.StatusInternalServerError on
failure). Also make the same addition for the other nearby Swagger block
referenced in the comment region (the block covering lines around the
recalculate-pricing handler).
In `@internal/usage/recalculate_pricing_mongodb.go`:
- Around line 23-40: The current code uses session.WithTransaction which fails
on standalone MongoDB; update the StartSession/WithTransaction flow to detect
the "transactions not supported" capability error (mirror the pattern in
internal/budget/store_mongodb.go) and fall back to the non-transactional path by
calling s.recalculatePricingInMongoTransaction with the original ctx (i.e., run
recalculatePricingInMongoTransaction(ctx, filter, resolver) outside a
transaction) and populate/return the RecalculatePricingResult instead of
returning a 500; keep the existing session start/end handling and only trigger
the fallback when the error clearly indicates transactions are unsupported.
In `@internal/usage/recalculate_pricing_postgresql.go`:
- Around line 16-24: The current flow calls s.postgresRecalculationEntries(...)
before starting the transaction (s.pool.Begin), creating a TOCTOU window; move
the selection of rows into the same transaction that performs updates: begin the
transaction first (use s.pool.Begin), then perform a SELECT ... FOR UPDATE (or
equivalent row-locking query) inside that transaction to fetch the recalculation
entries instead of calling postgresRecalculationEntries outside the tx; perform
the updates inside the same tx and only increment Matched/Recalculated counts
after each successful update (so
finalizeRecalculatePricingResult/RecalculatePricingResult reflect post-update
state), then commit/rollback the tx as needed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 20776192-58b7-4809-9c77-8e05e51496c7
📒 Files selected for processing (14)
internal/admin/dashboard/static/js/modules/pricing.jsinternal/admin/dashboard/static/js/modules/pricing.test.cjsinternal/admin/dashboard/static/js/modules/workflows.jsinternal/admin/dashboard/static/js/modules/workflows.test.cjsinternal/admin/dashboard/templates/page-settings.htmlinternal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/admin/handler_test.gointernal/app/app.gointernal/server/auth.gointernal/server/http.gointernal/server/http_test.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_postgresql.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/handler_pricing_test.go`:
- Around line 141-236: Add unit tests exercising the start_date/end_date parsing
and user_path validation branches for Handler.RecalculateUsagePricing: create
cases that send malformed date strings (e.g., invalid format for
start_date/end_date) and invalid user_path values and assert the handler returns
http.StatusBadRequest with the appropriate error messages, that
mockPricingRecalculator.calls remains 0, and other setup mirrors existing tests
(use NewHandler(nil, providers.NewModelRegistry(),
WithUsagePricingRecalculator(recalculator)) and echo.New/httptest.NewRequest to
post to "/admin/api/v1/usage/recalculate-pricing"). Ensure tests reference
RecalculateUsagePricing, mockPricingRecalculator, NewHandler and
WithUsagePricingRecalculator so they exercise the validation logic without
invoking the recalculator.
In `@internal/admin/handler.go`:
- Around line 381-396: The refactor accidentally made the default end bound
inclusive by setting end = today and shifting the start forward; change the
logic in the block handling startParsed/endParsed and the days-default branch so
omitted end_date continues to mean "next midnight" (i.e., set end =
today.AddDate(0,0,1) when endParsed is false) and compute start relative to that
end (e.g., if !startParsed set start = end.AddDate(0,0,-29) or when using days
default do start = end.AddDate(0,0,-(days-1))). Update the branches that
reference startParsed, endParsed, start, end, today and days so behavior matches
the previous exclusive-end semantics.
- Around line 344-349: Reintroduce an upper bound for requested day ranges by
adding a MAX_DAYS constant and enforcing it inside the shared date-range helper
(buildDateRange) and any callers that parse `days` (the query-parse block that
sets `days := 30` and the body-parsing code paths); after parsing an integer
days value, clamp it with days = min(parsed, MAX_DAYS) (and similarly clamp
body-provided days) so callers cannot request unbounded ranges (choose an
appropriate MAX_DAYS value and apply this clamp wherever buildDateRange is
used).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: e7a9028b-5807-4e5f-98ae-e3e0eeae0f66
📒 Files selected for processing (4)
internal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_postgresql.go
Comment on lines
381
to
+396
| if startParsed || endParsed { | ||
| if !startParsed { | ||
| params.StartDate = params.EndDate.AddDate(0, 0, -29) | ||
| start = end.AddDate(0, 0, -29) | ||
| } | ||
| if !endParsed { | ||
| params.EndDate = today | ||
| end = today | ||
| } | ||
| return params, nil | ||
| return start, end, nil | ||
| } | ||
|
|
||
| days := 30 | ||
| if d := c.QueryParam("days"); d != "" { | ||
| if parsed, err := strconv.Atoi(d); err == nil && parsed > 0 { | ||
| days = parsed | ||
| } | ||
| if days <= 0 { | ||
| days = 30 | ||
| } | ||
| params.EndDate = today | ||
| params.StartDate = today.AddDate(0, 0, -(days - 1)) | ||
|
|
||
| return params, nil | ||
| end = today | ||
| start = today.AddDate(0, 0, -(days - 1)) | ||
| return start, end, nil |
Contributor
There was a problem hiding this comment.
This helper changes the default date window by a full day.
Before this refactor, omitting end_date produced a range ending at the next midnight; now it ends at today and also moves the default start forward by one day. On Tuesday, April 28, 2026, that shifts the default window from March 29, 2026–April 29, 2026 to March 30, 2026–April 28, 2026. That changes every existing admin usage/audit endpoint, and any reader treating EndDate as an exclusive bound will stop including April 28 data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/handler.go` around lines 381 - 396, The refactor accidentally
made the default end bound inclusive by setting end = today and shifting the
start forward; change the logic in the block handling startParsed/endParsed and
the days-default branch so omitted end_date continues to mean "next midnight"
(i.e., set end = today.AddDate(0,0,1) when endParsed is false) and compute start
relative to that end (e.g., if !startParsed set start = end.AddDate(0,0,-29) or
when using days default do start = end.AddDate(0,0,-(days-1))). Update the
branches that reference startParsed, endParsed, start, end, today and days so
behavior matches the previous exclusive-end semantics.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/handler.go`:
- Around line 362-396: The buildDateRange helper currently allows inverted
ranges; after computing/assigning start and end (inside function buildDateRange,
after the block that sets start/end for parsed or default cases but before any
return), add a validation that if start.After(end) then return time.Time{},
time.Time{}, core.NewInvalidRequestError("start_date must be on or before
end_date", nil); this ensures any caller of buildDateRange (and symbols start,
end, startParsed, endParsed) fails fast with a 400 when start > end.
In `@internal/usage/recalculate_pricing_mongodb.go`:
- Around line 110-131: The code passes only row.Provider into
recalculateEntryCosts, which ignores a more specific provider_name present in
the raw Mongo row; update the construction of the recalculationEntry (used by
recalculateEntryCosts) to prefer the explicit provider_name from row.RawData (or
equivalent key) when present and non-empty, falling back to row.Provider
otherwise, so named providers (where provider_name != provider) resolve to the
correct pricing metadata; apply the same change for the other matching block
that builds a recalculationEntry (lines referencing recalculateEntryCosts and
the row struct).
In `@internal/usage/recalculate_pricing_postgresql.go`:
- Around line 62-65: The query builds conditions using params.Provider and
appends a (provider = $N OR provider_name = $N+1) clause but later only
preserves/uses the `provider` value during the SELECT/scan and recalculation
path; update the SELECT/scan and any downstream usage so `provider_name` is
retrieved and carried through (prefer `provider_name` when non-empty, fall back
to `provider`) so recalculation uses the correct provider table; search for the
variables/identifiers `params.Provider`, `provider_name`, `provider`,
`conditions`, `args`, `nextIdx` in this file (and the similar block around lines
72-99) and ensure the row struct/scan includes `provider_name` and the code that
chooses the price table uses `provider_name` when present.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8c19f70f-6a18-49e8-88a6-53582181c28f
📒 Files selected for processing (7)
internal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/templates/page-settings.htmlinternal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/admin/handler_test.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_postgresql.go
Comment on lines
+110
to
+131
| var row struct { | ||
| ID string `bson:"_id"` | ||
| Model string `bson:"model"` | ||
| Provider string `bson:"provider"` | ||
| Endpoint string `bson:"endpoint"` | ||
| InputTokens int `bson:"input_tokens"` | ||
| OutputTokens int `bson:"output_tokens"` | ||
| RawData map[string]any `bson:"raw_data"` | ||
| } | ||
| if err := cursor.Decode(&row); err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("scan mongodb usage cost row: %w", err) | ||
| } | ||
|
|
||
| update := recalculateEntryCosts(recalculationEntry{ | ||
| ID: row.ID, | ||
| Model: row.Model, | ||
| Provider: row.Provider, | ||
| Endpoint: row.Endpoint, | ||
| InputTokens: row.InputTokens, | ||
| OutputTokens: row.OutputTokens, | ||
| RawData: row.RawData, | ||
| }, resolver) |
Contributor
There was a problem hiding this comment.
Resolve pricing with provider_name when it is present.
Lines 152-156 intentionally match rows by provider_name, but Lines 111-130 only pass provider into recalculateEntryCosts. For named providers where provider_name != provider, this recalculates against the generic provider type and can apply the wrong pricing metadata or miss pricing entirely.
🛠️ Suggested fix
var row struct {
ID string `bson:"_id"`
Model string `bson:"model"`
Provider string `bson:"provider"`
+ ProviderName string `bson:"provider_name"`
Endpoint string `bson:"endpoint"`
InputTokens int `bson:"input_tokens"`
OutputTokens int `bson:"output_tokens"`
RawData map[string]any `bson:"raw_data"`
}
@@
+ provider := row.Provider
+ if strings.TrimSpace(row.ProviderName) != "" {
+ provider = row.ProviderName
+ }
+
update := recalculateEntryCosts(recalculationEntry{
ID: row.ID,
Model: row.Model,
- Provider: row.Provider,
+ Provider: provider,
Endpoint: row.Endpoint,
InputTokens: row.InputTokens,
OutputTokens: row.OutputTokens,
RawData: row.RawData,
}, resolver)Also applies to: 152-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/usage/recalculate_pricing_mongodb.go` around lines 110 - 131, The
code passes only row.Provider into recalculateEntryCosts, which ignores a more
specific provider_name present in the raw Mongo row; update the construction of
the recalculationEntry (used by recalculateEntryCosts) to prefer the explicit
provider_name from row.RawData (or equivalent key) when present and non-empty,
falling back to row.Provider otherwise, so named providers (where provider_name
!= provider) resolve to the correct pricing metadata; apply the same change for
the other matching block that builds a recalculationEntry (lines referencing
recalculateEntryCosts and the row struct).
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)
config/config.example.yaml (1)
84-90:⚠️ Potential issue | 🟡 MinorMirror the runtime prerequisite here.
The new flag is useful, but this example block does not mention that
USAGE_ENABLED=trueand supported storage are still required for the action to appear. Copy-paste users may assume the toggle alone is enough.Suggested fix
usage: enabled: true + # Requires USAGE_ENABLED=true and supported storage; false always hides the action. pricing_recalculation_enabled: true enforce_returning_usage_data: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/config.example.yaml` around lines 84 - 90, Update the example usage block to explicitly state the runtime prerequisites: mention that the environment variable USAGE_ENABLED must be set to true and that a supported storage backend must be configured for the usage action to appear; reference the usage block keys (usage.enabled, pricing_recalculation_enabled, enforce_returning_usage_data, buffer_size, flush_interval, retention_days) and add a short comment or descriptive line in that same block explaining these additional requirements so copy-paste users aren’t misled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/handler_pricing_test.go`:
- Around line 269-305: Add a new unit test that verifies the default date-range
translation when no "days", "start_date", or "end_date" are provided: create a
test (e.g., TestRecalculateUsagePricingDefaultsDateRange) that mirrors the
existing TestRecalculateUsagePricingClampsRequestedDays setup (override timeNow
to a fixed date, instantiate mockPricingRecalculator, build Handler with
WithUsagePricingRecalculator, create an HTTP POST to
"/admin/api/v1/usage/recalculate-pricing" with an empty JSON body or only the
confirmation field), invoke h.RecalculateUsagePricing(c), assert no handler
error and http.StatusOK, assert recalculator.calls == 1, and assert
recalculator.params.StartDate and EndDate equal the expected defaults computed
from the fixed timeNow (expectedEnd at midnight of the fixed date and
expectedStart = expectedEnd.AddDate(0,0,-(maxDateRangeDays-1))). Ensure the test
name and assertions reference mockPricingRecalculator, NewHandler, and
h.RecalculateUsagePricing so it’s easy to locate and maintain.
- Around line 120-329: Add a new test that verifies the legacy "confirm" field
is accepted the same as "confirmation": create a test (e.g.,
TestRecalculateUsagePricingAcceptsConfirmAlias) that sets up a
mockPricingRecalculator, builds a Handler with WithUsagePricingRecalculator, and
sends two POST requests to RecalculateUsagePricing—one with body
{"confirm":"recalculate"} expecting HTTP 200 and recalculator.calls == 1, and
one with body {"confirm":"nope"} expecting HTTP 400 and recalculator.calls == 0;
reuse the same echo.New()/httptest setup and assertions style as existing tests
(refer to NewHandler, RecalculateUsagePricing, mockPricingRecalculator) and
ensure content-type is application/json.
In `@internal/admin/handler.go`:
- Around line 727-733: The handler is using h.mutationMu around a long-running
call to h.usageRecalculator.RecalculatePricing which can block unrelated admin
mutations; add a dedicated mutex (e.g., pricingMu) on the handler struct and
replace the h.mutationMu.Lock()/defer Unlock() surrounding the
RecalculatePricing call with pricingMu.Lock()/defer Unlock() so only pricing
recalculation is serialized; ensure you add the new pricingMu field to the
handler type (zero-value sync.Mutex is fine) and only hold that lock for the
duration of the RecalculatePricing invocation and error handling.
In `@internal/usage/recalculate_pricing_mongodb.go`:
- Around line 16-60: The PR lacks unit tests covering MongoDB pricing
recalculation branches; add tests exercising MongoDBStore.RecalculatePricing to
cover: (1) successful transactional flow (mock StartSession/WithTransaction to
invoke recalculatePricingInMongoTransaction and assert
finalizeRecalculatePricingResult behavior), (2) transaction-unavailable fallback
flow (simulate isMongoTransactionCapabilityError/mongoTransactionFallbackCause
by having WithTransaction return the capability error so code takes the fallback
path and calls recalculatePricingInMongoTransaction non-transactionally), and
(3) provider/provider_name resolution during updates (assert the resolver
PricingResolver and normalizedRecalculatePricingParams produce the expected
provider/provider_name field behavior). Use table-driven tests in a new
internal/usage/*_test.go file, mock the collection.Client().StartSession and
session.WithTransaction behaviours, and assert final RecalculatePricingResult
and logged warning path when falling back.
In `@internal/usage/recalculate_pricing_sqlite.go`:
- Around line 17-23: The code currently materializes all rows returned by
sqliteRecalculationEntries into memory (seen where entries, err :=
s.sqliteRecalculationEntries(...) and the later loop over entries), which can
OOM on large ranges; change the implementation to stream/process in bounded
batches using keyset pagination (use the primary id/rowid as the cursor) instead
of returning the full slice—modify sqliteRecalculationEntries (or add a new
paged iterator) to accept a lastID/limit and return each batch, loop fetching
batches until empty, apply updates per-batch, and incrementally update/aggregate
counters passed to finalizeRecalculatePricingResult (don’t accumulate all
entries in memory). Ensure the batch size is configurable and that the cursor
uses a stable unique column like id or rowid to page deterministically.
---
Outside diff comments:
In `@config/config.example.yaml`:
- Around line 84-90: Update the example usage block to explicitly state the
runtime prerequisites: mention that the environment variable USAGE_ENABLED must
be set to true and that a supported storage backend must be configured for the
usage action to appear; reference the usage block keys (usage.enabled,
pricing_recalculation_enabled, enforce_returning_usage_data, buffer_size,
flush_interval, retention_days) and add a short comment or descriptive line in
that same block explaining these additional requirements so copy-paste users
aren’t misled.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: accf531f-cf93-4d56-a132-13b4c7edfa1e
📒 Files selected for processing (20)
.env.templateconfig/config.example.yamlconfig/config.goconfig/config_test.godocs/advanced/configuration.mdxinternal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/templates/page-settings.htmlinternal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/admin/handler_test.gointernal/app/app.gointernal/app/app_test.gointernal/providers/registry.gointernal/providers/registry_metadata_override_test.gointernal/usage/recalculate_pricing.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_postgresql.gointernal/usage/recalculate_pricing_sqlite.gointernal/usage/recalculate_pricing_sqlite_test.gointernal/usage/recalculate_pricing_test.go
Comment on lines
+16
to
+60
| func (s *MongoDBStore) RecalculatePricing(ctx context.Context, params RecalculatePricingParams, resolver PricingResolver) (RecalculatePricingResult, error) { | ||
| if err := recalculatePricingUnavailable(resolver); err != nil { | ||
| return RecalculatePricingResult{}, err | ||
| } | ||
| params = normalizedRecalculatePricingParams(params) | ||
|
|
||
| filter, err := mongoRecalculationFilter(params) | ||
| if err != nil { | ||
| return RecalculatePricingResult{}, err | ||
| } | ||
|
|
||
| session, err := s.collection.Database().Client().StartSession() | ||
| if err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("start mongodb pricing recalculation transaction: %w", err) | ||
| } | ||
| defer session.EndSession(ctx) | ||
|
|
||
| var result RecalculatePricingResult | ||
| _, err = session.WithTransaction(ctx, func(txCtx context.Context) (any, error) { | ||
| next, err := s.recalculatePricingInMongoTransaction(txCtx, filter, resolver) | ||
| if err != nil { | ||
| if isMongoTransactionCapabilityError(err) { | ||
| return nil, &mongoTransactionFallbackError{err: err} | ||
| } | ||
| return nil, err | ||
| } | ||
| result = next | ||
| return nil, nil | ||
| }) | ||
| if err != nil { | ||
| if fallbackErr := mongoTransactionFallbackCause(err); fallbackErr != nil || isMongoTransactionCapabilityError(err) { | ||
| if fallbackErr == nil { | ||
| fallbackErr = err | ||
| } | ||
| slog.Warn("MongoDB transactions unavailable for pricing recalculation; falling back to non-transactional update", "error", fallbackErr) | ||
| result, err := s.recalculatePricingInMongoTransaction(ctx, filter, resolver) | ||
| if err != nil { | ||
| return RecalculatePricingResult{}, fmt.Errorf("recalculate mongodb usage costs without transaction: %w", errors.Join(fallbackErr, err)) | ||
| } | ||
| return finalizeRecalculatePricingResult(result), nil | ||
| } | ||
| return RecalculatePricingResult{}, fmt.Errorf("mongodb pricing recalculation transaction: %w", err) | ||
| } | ||
| return finalizeRecalculatePricingResult(result), nil | ||
| } |
Contributor
There was a problem hiding this comment.
Add MongoDB backend tests for recalculation transaction/fallback branches.
This path now contains critical branching (WithTransaction + capability fallback + conditional field set/unset), but patch coverage indicates this file remains untested. Please add focused tests for (1) transaction-supported flow, (2) transaction-unsupported fallback flow, and (3) provider/provider_name pricing resolution during updates.
As per coding guidelines **/*_test.go: “Add or update tests for behavior changes.”
Also applies to: 101-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/usage/recalculate_pricing_mongodb.go` around lines 16 - 60, The PR
lacks unit tests covering MongoDB pricing recalculation branches; add tests
exercising MongoDBStore.RecalculatePricing to cover: (1) successful
transactional flow (mock StartSession/WithTransaction to invoke
recalculatePricingInMongoTransaction and assert finalizeRecalculatePricingResult
behavior), (2) transaction-unavailable fallback flow (simulate
isMongoTransactionCapabilityError/mongoTransactionFallbackCause by having
WithTransaction return the capability error so code takes the fallback path and
calls recalculatePricingInMongoTransaction non-transactionally), and (3)
provider/provider_name resolution during updates (assert the resolver
PricingResolver and normalizedRecalculatePricingParams produce the expected
provider/provider_name field behavior). Use table-driven tests in a new
internal/usage/*_test.go file, mock the collection.Client().StartSession and
session.WithTransaction behaviours, and assert final RecalculatePricingResult
and logged warning path when falling back.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/admin/handler.go (1)
384-394:⚠️ Potential issue | 🟠 MajorOmitted
end_datestill drops the current day.When
end_dateis absent,end = todaybecomes local midnight at the start of the day. On April 28, 2026, that is2026-04-28 00:00:00, so any caller that treatsEndDateas the upper bound will skip all usage later on April 28. Use the next midnight for the implicit end bound and derive the fallback start from that exclusive bound instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/handler.go` around lines 384 - 394, The current logic uses today (local midnight) as the implicit end when end_date is absent, which unintentionally excludes the current day; change the implicit end to the next midnight (set end = today.AddDate(0,0,1) when endParsed is false) and derive any fallback start from that exclusive end (e.g. when using days: days = normalizeDateRangeDays(days); end = today.AddDate(0,0,1); start = end.AddDate(0,0,-days)), and similarly in the startParsed||endParsed branch, if !endParsed set end = today.AddDate(0,0,1) and if !startParsed set start = end.AddDate(0,0,-29) so all uses of start/end (variables start, end, today, days and function normalizeDateRangeDays) compute the fallback range using the exclusive next-midnight end.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/admin/handler.go`:
- Around line 731-734: The current wrapping of errors from
h.usageRecalculator.RecalculatePricing into core.NewProviderError hides context
cancellations/timeouts and existing gateway errors; change the error handling
after calling h.usageRecalculator.RecalculatePricing(c.Request().Context(),
params, h.registry) to first check for context.Canceled and
context.DeadlineExceeded and for errors of type *core.GatewayError and pass them
directly to handleError(c, err) (or return them unchanged), and only wrap other
unexpected errors with core.NewProviderError("usage",
http.StatusInternalServerError, "failed to recalculate usage pricing", err);
this preserves the original status codes and cancellation semantics.
In `@internal/usage/recalculate_pricing_sqlite_test.go`:
- Around line 21-27: The in-memory SQLite tests use a private ":memory:" DB per
connection which is flaky with database/sql connection pooling; update the test
setup where db is opened (the db variable used to create NewSQLiteStore) to
ensure a single connection is used by calling db.SetMaxOpenConns(1) immediately
after opening the DB (or alternatively change the DSN to a shared-cache memory
DSN like "file::memory:?cache=shared"), so that NewSQLiteStore and subsequent
test steps all operate on the same in-memory database.
---
Duplicate comments:
In `@internal/admin/handler.go`:
- Around line 384-394: The current logic uses today (local midnight) as the
implicit end when end_date is absent, which unintentionally excludes the current
day; change the implicit end to the next midnight (set end =
today.AddDate(0,0,1) when endParsed is false) and derive any fallback start from
that exclusive end (e.g. when using days: days = normalizeDateRangeDays(days);
end = today.AddDate(0,0,1); start = end.AddDate(0,0,-days)), and similarly in
the startParsed||endParsed branch, if !endParsed set end = today.AddDate(0,0,1)
and if !startParsed set start = end.AddDate(0,0,-29) so all uses of start/end
(variables start, end, today, days and function normalizeDateRangeDays) compute
the fallback range using the exclusive next-midnight end.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 8a623f2b-7b8f-493f-affd-c79a12f335cf
📒 Files selected for processing (9)
config/config.example.yamlinternal/admin/handler.gointernal/admin/handler_pricing_test.gointernal/usage/recalculate_pricing_mongodb.gointernal/usage/recalculate_pricing_mongodb_test.gointernal/usage/recalculate_pricing_sqlite.gointernal/usage/recalculate_pricing_sqlite_test.gointernal/usage/store_mongodb.gointernal/usage/store_sqlite.go
Comment on lines
+21
to
+27
| db, err := sql.Open("sqlite", ":memory:") | ||
| if err != nil { | ||
| t.Fatalf("open sqlite: %v", err) | ||
| } | ||
| defer db.Close() | ||
|
|
||
| store, err := NewSQLiteStore(db, 0) |
Contributor
There was a problem hiding this comment.
Pin the in-memory SQLite tests to one connection.
The bare :memory: DSN creates a private database per connection, and database/sql may use pooled connections underneath. That makes these tests flaky once setup, recalculation, and assertions stop reusing the same connection. Call db.SetMaxOpenConns(1) or switch to a shared-cache memory DSN. (sqlite.org)
Minimal fix
db, err := sql.Open("sqlite", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
+ db.SetMaxOpenConns(1)
defer db.Close()Also applies to: 110-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/usage/recalculate_pricing_sqlite_test.go` around lines 21 - 27, The
in-memory SQLite tests use a private ":memory:" DB per connection which is flaky
with database/sql connection pooling; update the test setup where db is opened
(the db variable used to create NewSQLiteStore) to ensure a single connection is
used by calling db.SetMaxOpenConns(1) immediately after opening the DB (or
alternatively change the DSN to a shared-cache memory DSN like
"file::memory:?cache=shared"), so that NewSQLiteStore and subsequent test steps
all operate on the same in-memory database.
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
2 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
Testing
Curl Verification Highlights
POST /admin/api/v1/usage/recalculate-pricingwith OpenRouter filter returnedmatched: 2,recalculated: 2,with_pricing: 2,without_pricing: 0after zeroing test rowsSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation