Skip to content

Fix analytics follow-up regressions from #2904 - #2918

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.7from
htkarabulut:fix/analytics-gemini-followup-v387
May 29, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.7from
htkarabulut:fix/analytics-gemini-followup-v387

Conversation

@htkarabulut

Copy link
Copy Markdown
Contributor

Summary

  • Follow-up to Fix analytics history and log token accounting #2904 after review comments identified regressions in the merged analytics/cleanup changes.
  • Prevent analytics raw + aggregate double-counting by splitting the unified source at a whole-day raw-retention boundary: daily_usage_summary owns dates before the raw cutoff day, and usage_history owns the cutoff day forward.
  • Avoid API-key filtered analytics leaking global aggregate totals by disabling daily_usage_summary rows whenever an API-key filter is active, since daily_usage_summary does not store API-key attribution.
  • Restore sidebar settings schema fields (sidebarSectionOrder, sidebarItemOrder, sidebarActivePreset) and the SIDEBAR_SECTIONS import that were accidentally dropped.
  • Prevent cleanup data loss by aborting usage_history deletion when the pre-delete rollup reports errors.
  • Make the usage-history rollup sargable by replacing WHERE DATE(timestamp) < ? with WHERE timestamp < ?.

Related Issues

Validation

  • npm run lint
  • npm run test:unit
  • npm run test:coverage
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Targeted validation run locally:

  • npx eslint src/app/api/usage/analytics/route.ts src/lib/db/cleanup.ts src/lib/usage/aggregateHistory.ts src/shared/validation/settingsSchemas.ts tests/unit/usage-analytics-route.test.ts
  • node --import tsx --import ./open-sse/utils/setupPolyfill.ts --test tests/unit/usage-analytics-route.test.ts tests/unit/token-accounting-input-fix.test.ts tests/unit/database-settings-maintenance.test.ts

Both passed.

Full npm run lint, npm run test:unit, and npm run test:coverage were not run for this follow-up.

Tests Added Or Updated

  • tests/unit/usage-analytics-route.test.ts
    • Adds regression coverage that raw usage_history rows and daily_usage_summary rows are not double-counted.
    • Adds regression coverage that API-key-filtered analytics omit global aggregate rows from daily_usage_summary.
  • Existing targeted tests also cover the token-accounting regression from Fix analytics history and log token accounting #2904 and database maintenance / aggregation behavior.

Coverage Notes

  • src/app/api/usage/analytics/route.ts is covered by the updated analytics route unit tests.
  • src/lib/db/cleanup.ts and src/lib/usage/aggregateHistory.ts are covered by tests/unit/database-settings-maintenance.test.ts for aggregation/maintenance behavior.
  • src/shared/validation/settingsSchemas.ts change restores existing sidebar settings schema fields; no new dedicated schema test was added.
  • Coverage was not regenerated locally, so any coverage movement is unknown.

Reviewer Notes

  • This PR intentionally disables daily_usage_summary rows when apiKeyIds filtering is active. Because summary rows do not store API-key attribution, including them would mix other keys’ aggregate usage into a filtered response.
  • The raw/aggregate split uses a whole-day boundary (YYYY-MM-DD) rather than a partial timestamp. This keeps daily summary rows and raw rows from sharing the same day and avoids partial-day gaps or overlaps.
  • Cleanup now aborts deletion if rollup reports errors. This may leave old usage_history rows in place until the next successful cleanup, but avoids permanent loss of usage data.
  • No migrations or feature flags are included.

@htkarabulut
htkarabulut requested a review from diegosouzapw as a code owner May 29, 2026 19:52

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the usage analytics route to prevent double-counting of raw and aggregated rows and to gate off aggregated rows when filtering by API key to prevent data leakage. It also refactors the database cleanup process to abort deletion if the pre-delete rollup fails, and adds settings schema fields for sidebar customization along with corresponding unit tests. Feedback on the changes highlights a critical runtime ReferenceError in cleanup.ts due to the removal of cutoffDateStr from cleanupUsageHistory while still being referenced, as well as its accidental and unused definition in cleanupQuotaSnapshots.

Comment thread src/lib/db/cleanup.ts Resolved
Comment thread src/lib/db/cleanup.ts Resolved

@diegosouzapw diegosouzapw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after local verification, quality checks and merging into release/v3.8.7.

@diegosouzapw
diegosouzapw merged commit 2467a06 into cheaper-inference:release/v3.8.7 May 29, 2026
3 checks passed
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…ytics-gemini-followup-v387

Fix analytics follow-up regressions from cheaper-inference#2904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants