Conversation
Add a toggle filter that surfaces usage-log records whose requested billing model differs from the model actually returned by the provider. The SQL condition compares COALESCE(model, original_model) against actual_response_model after trimming whitespace, skipping rows where either side is NULL or empty. This intentionally ignores model redirection (original_model vs model) and focuses on provider-side model substitution. Wired through the full stack: API query schemas and handlers, server actions, repository conditions for list/batch/stats/slim queries, the dashboard filter UI with active-filter display, URL serialization, and i18n strings for all five locales.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (24)
📝 WalkthroughWalkthrough在整个代码栈中端到端新增 ChangesactualResponseModelMismatch 端到端过滤
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new filter, actualResponseModelMismatch, allowing users to filter usage logs where the requested billing model differs from the actual response model. The changes span UI updates (including translations, a new switch component, and active filter display), API schemas, and repository query logic, along with comprehensive integration and unit tests. The reviewer feedback highlights two key improvement opportunities: removing the PostgreSQL-specific btrim function in the SQL query builder to improve database portability and index performance, and explicitly parsing the actualResponseModelMismatch URL search parameter to a boolean to prevent TypeScript type mismatches and runtime bugs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const effectiveRequestModel = fallbackModelColumn | ||
| ? sql`COALESCE(NULLIF(btrim(${modelColumn}), ''), NULLIF(btrim(${fallbackModelColumn}), ''))` | ||
| : sql`NULLIF(btrim(${modelColumn}), '')`; | ||
| const actualResponseModel = sql`NULLIF(btrim(${actualResponseModelColumn}), '')`; |
There was a problem hiding this comment.
The use of the PostgreSQL-specific btrim function on columns in the WHERE clause has two drawbacks:
- Database Portability:
btrimis PostgreSQL-specific. If the application or its tests are run against other databases (such as SQLite for local testing), this query will fail. - Performance / Index Usage: Applying functions directly to columns in a
WHEREclause prevents the database from utilizing standard indexes on those columns (e.g., an index onmodeloractual_response_model).
Since model names are system identifiers and do not contain leading or trailing whitespace, trimming is unnecessary. We can safely use standard NULLIF without btrim:
const effectiveRequestModel = fallbackModelColumn
? sql`COALESCE(NULLIF(${modelColumn}, ''), NULLIF(${fallbackModelColumn}, ''))`
: sql`NULLIF(${modelColumn}, '')`;
const actualResponseModel = sql`NULLIF(${actualResponseModelColumn}, '')`;| endTime: _params.get("endTime") ?? undefined, | ||
| statusCode: _params.get("statusCode") ?? undefined, | ||
| model: _params.get("model") ?? undefined, | ||
| actualResponseModelMismatch: _params.get("actualResponseModelMismatch") ?? undefined, |
There was a problem hiding this comment.
The parameter actualResponseModelMismatch is retrieved directly from URLSearchParams using _params.get("actualResponseModelMismatch") ?? undefined. This returns a string (e.g., "true" or "false") or undefined, rather than a boolean.
This causes a TypeScript type mismatch if the filter is typed as boolean | undefined. Furthermore, it can lead to subtle runtime bugs because the string "false" is truthy in JavaScript, which would cause the filter to be treated as active even when set to false.
We should explicitly parse the string value to a boolean.
| actualResponseModelMismatch: _params.get("actualResponseModelMismatch") ?? undefined, | |
| actualResponseModelMismatch: _params.get("actualResponseModelMismatch") === "true" ? true : undefined, |
| /** 仅筛选请求模型与实际响应模型不一致的记录(不按 originalModel/模型重定向判断) */ | ||
| actualResponseModelMismatch?: boolean; |
There was a problem hiding this comment.
The JSDoc comment says "不按 originalModel/模型重定向判断" (not based on originalModel / model-redirect logic), but the SQL condition produced by
buildActualResponseModelMismatchCondition deliberately uses originalModel as a COALESCE fallback for the effective billing model. The comment contradicts the implementation and will mislead future readers into thinking originalModel is entirely ignored.
| /** 仅筛选请求模型与实际响应模型不一致的记录(不按 originalModel/模型重定向判断) */ | |
| actualResponseModelMismatch?: boolean; | |
| /** 仅筛选请求模型与实际响应模型不一致的记录(按 COALESCE(model, originalModel) 作为有效计费模型与 actualResponseModel 比较) */ | |
| actualResponseModelMismatch?: boolean; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/repository/usage-logs.ts
Line: 39-40
Comment:
The JSDoc comment says "不按 originalModel/模型重定向判断" (not based on originalModel / model-redirect logic), but the SQL condition produced by `buildActualResponseModelMismatchCondition` deliberately uses `originalModel` as a COALESCE fallback for the effective billing model. The comment contradicts the implementation and will mislead future readers into thinking `originalModel` is entirely ignored.
```suggestion
/** 仅筛选请求模型与实际响应模型不一致的记录(按 COALESCE(model, originalModel) 作为有效计费模型与 actualResponseModel 比较) */
actualResponseModelMismatch?: boolean;
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review Summary
This release PR cleanly threads a single optional boolean filter (actualResponseModelMismatch) through the entire stack — Zod schemas, REST handlers, server actions, the repository condition builder, the slim cache key, generated OpenAPI types, dashboard URL state, the active-filter chip, and all five locales. The SQL semantics (COALESCE(NULLIF(btrim(model),''), NULLIF(btrim(original_model),'')) <> NULLIF(btrim(actual_response_model),'') with both sides IS NOT NULL) correctly implement the documented intent: it flags only a true divergence between the billed/requested model and what the upstream actually returned, deliberately excluding pure model redirects. No defects met the reporting threshold after full-context validation.
PR Size: L
- Lines changed: 260 (+259 / -1)
- Files changed: 24
Split note (L-sized): Despite 24 files, ~20 are minimal one-line touch-points wiring a single boolean through the stack, plus 5 mechanical i18n locale additions. The change is one tightly-coupled feature; splitting it (e.g. repo-layer vs. UI-layer) would leave the halves unshippable on their own — the API contract, repository condition, and UI toggle must land together. No practical split recommended.
Verification performed (all paths confirmed covered)
buildActualResponseModelMismatchConditionis applied on both the message-request and usage-ledger sides of every query:findUsageLogsBatch,findUsageLogsForKeySlim/Batch,findReadonlyUsageLogsBatchForKey, andfindUsageLogsStats.- Slim-path total-count cache key includes the flag, preventing stale cross-filter totals.
BooleanQuerySchemacoerces"true"/"false"/absent correctly; URL round-trip (true→actualResponseModelMismatch=true→true) verified inlogs-query.tsand the navigation test.- All three self-usage entry points (
getMyUsageLogs,getMyUsageLogsBatch,getMyUsageLogsBatchFull) forward the filter. - i18n keys resolve under the
dashboard.logs.filtersnamespace inactive-filters-display.tsxandrequest-filters.tsx.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
No issues at confidence >= 80.
Observations (below threshold, no action required)
- The unit test in
tests/unit/repository/usage-logs-actual-model-mismatch-filter.test.tsasserts on the generated SQL string rather than executing against a database. This is the standard approach for Drizzle SQL-builder unit tests here and adequately guards theCOALESCE/<>/original_model-exclusion semantics; it does not exercise the ledger-side inline copies of the condition, but those call the same shared builder. - The generated
actualResponseModelMismatch?: "true" | "false" | booleanunion inopenapi-types.gen.tsis intentionally permissive to mirror the Zod union that accepts both string and boolean query input.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Claude AI
Summary
Release v0.8.8 — merges the
devbranch intomain. This is a single-feature release containing 1 PR: the new usage-log filter for billing/actual model mismatches.Included Changes
feat(usage-logs): filter by billing/actual model mismatch (#1302)
Adds an optional
actualResponseModelMismatchboolean filter to usage-log queries that surfaces records where the effective billing/request model differs from the model the upstream provider actually returned (actualResponseModel).actualResponseModelas a↳ {model}hint, but there was no way to search for rows where the two diverge — making silent upstream model substitutions (e.g. a provider downgrading the model) hard to spot across large datasets.COALESCE(btrim(model), btrim(originalModel))and a row is flagged only when it is non-blank and differs (<>) from the non-blankactualResponseModel. Because the baseline isCOALESCE(model, originalModel), a pure model redirect (originalModel <> model) is not flagged as a mismatch on its own — only a true divergence between what was billed/requested and what the upstream actually returned. Blank/whitespace values collapse toNULL, so rows missing a model are excluded rather than flagged.en,ja,ru,zh-CN,zh-TW).Foundational PRs this builds on:
modelwithoriginalModelfallback) relies on the redirected model those rules produce.Notes for Reviewers
actual_response_model(feat(logs): record and display upstream actual response model #1092),model, andoriginal_model(feat: support provider model redirect rules #993).Testing
Automated (added/updated)
tests/unit/repository/usage-logs-actual-model-mismatch-filter.test.ts— SQL semantics: confirmsCOALESCE/<>comparison againstactual_response_model, assertsoriginal_model <> modelis not the trigger condition, and that the flag absent/false adds no condition.tests/api/v1/usage-logs/usage-logs.test.ts— admin list / stats / async export propagate the filter.tests/api/v1/me/me.test.ts— self usage-logs list / full propagate the filter.usage-logs-view-virtualized.test.tsx— URL state includesactualResponseModelMismatch=true.Manual verification
originalModel <> model, matchingactualResponseModel) is not filtered out.Description enhanced by Claude AI