Skip to content

perf(gamification): add xp_action_counts cache table to fix O(n) COUNT(*) on every XP event (#12546) - #13116

Closed
KooshaPari wants to merge 4 commits into
cheaper-inference:release/v3.8.51from
KooshaPari:pr/resubmit-12712-xp-cache
Closed

perf(gamification): add xp_action_counts cache table to fix O(n) COUNT(*) on every XP event (#12546)#13116
KooshaPari wants to merge 4 commits into
cheaper-inference:release/v3.8.51from
KooshaPari:pr/resubmit-12712-xp-cache

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #12546. Resubmission of #12712 (which was closed for base drift + user_id vs api_key_id column mismatch).

The XP-earning hot path was running SELECT COUNT(*) FROM xp_audit_log ... GROUP BY ... on every event. With large xp_audit_log tables this is the slow path that triggered base-red CI badges.

Fix

Introduces an incremental counter cache keyed on api_key_id (matching the xp_audit_log schema):

  • New table xp_action_counts(api_key_id, action, count, updated_at) via migration 174 (next free after 173 on release/v3.8.51)
  • Backfill via GROUP BY api_key_id, action from existing xp_audit_log rows
  • incrementActionCount() runs in the same transaction as the xp_audit_log INSERT (atomic) — and is wrapped in try/catch so a cache write failure doesn't fail the whole XP grant (cache is a performance optimization, not the source of truth)
  • getActionCountByType() reads from the cache, falls back to COUNT(*) on cache miss (user predates migration 174), also wrapped in try/catch so a cache read failure falls back rather than throws
  • getActionCount() in badges.ts is now a 3-line wrapper calling the cached reader
  • checkActionCountBadges() in events.ts uses the cached reader — O(1) hot path

Behavior

Scenario Before After
Hot path (per XP event) INSERT + full-table COUNT(*) INSERT + UPSERT (cache only)
Bad-count badge threshold check O(audit_log) O(1)
User predates migration 174 n/a fallback to COUNT(*) — same as before
Cache write fails n/a XP grant still succeeds, log + proceed
Cache read fails n/a fallback to COUNT(*) — no regression

Files

File Status LOC
src/lib/db/migrations/174_xp_action_counts.sql new ~25
src/lib/db/gamification.ts modified +45 (increment + getter + transaction wrap)
src/lib/gamification/badges.ts modified -15, +3 (wrapper)
src/lib/gamification/events.ts modified -3, +5 (use cached reader)
tests/unit/gamification/db-gamification.test.ts modified +3 tests

Tests

tests/unit/gamification/db-gamification.test.ts:

  • addXp increments the cache
  • incrementActionCount is idempotent (same api_key+action)
  • getActionCountByType falls back to COUNT(*) on cache miss

All 6 tests pass.

Why this matters

The XP-earning path is hit on every successful LLM request — currently O(audit_log) per call. With migration 174 in place it's O(1), which:

  • Unblocks the base-red CI badge on release/v3.8.51
  • Makes the bad-count thresholds (combo, provider_switch, daily_login) consistent regardless of how big the user's audit_log grows
  • Avoids the cascade where large audit_log → slow count → slow badges → slow UI

Fixes #12546.

@KooshaPari
perf(gamification): add xp_action_counts cache table to fix O(n) COUN…
cd7a090 
…T(*) on every XP event (cheaper-inference#12546, cheaper-inference#12712)

Per Diego's spec on the original issue + his review feedback on the
first PR (cheaper-inference#12712, closed for base drift + column mismatch):

- New table xp_action_counts(api_key_id, action, count, updated_at)
- Migration 174 adds the table + backfills via
  GROUP BY api_key_id, action (NOT user_id - that's the column
  mismatch the first PR had)
- incrementActionCount() runs in the SAME transaction as the
  xp_audit_log INSERT (atomic). Wrapped in try/catch so a cache
  write failure doesn't fail the whole XP grant (cache is a
  performance optimization, not the source of truth).
- getActionCountByType() reads from the cache, falls back to
  COUNT(*) on cache miss (predates migration 174). Also try/catch
  so a cache read failure falls back rather than throwing.
- getActionCount() in badges.ts is now a 3-line wrapper.
- checkActionCountBadges() in events.ts calls the cached reader
  with a comment explaining the O(1) hot-path.

Test: tests/unit/gamification/db-gamification.test.ts
  - addXp increments the cache
  - incrementActionCount is idempotent (same api_key+action)
  - getActionCountByType falls back to COUNT(*) on cache miss

Fixes cheaper-inference#12546
Resubmission of cheaper-inference#12712 (corrected base + correct column)
Copilot AI lite review requested due to automatic review settings September 9, 2026 10:13
Copilot AI reviewed Sep 9, 2026

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@HouMinXi

HouMinXi commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Thanks for the xp_action_counts cache.

Folded into a local recut of release/v3.8.51. The merge itself was clean. After stacking, the recut already had tip's 174_server_tool_executions.sql (#12867, merged) and 176_provider_connection_synced_models_at.sql (#13248), so this PR's 174_xp_action_counts.sql tripped the migration-runner collision guard.

Resolution on the recut only: rename to 177_xp_action_counts.sql. This PR is still MERGEABLE vs tip and does not need a rewrite; once tip moves past 174, a rebase that picks the next free number will land cleanly.

Purpose of the recut: ship current release/v3.8.51 plus open product PRs on a private production host.

@KooshaPari
perf(gamification): add xp_action_counts cache table to fix O(n) COUN…
c542fd3 
…T(*) on every XP event

Fixes cheaper-inference#12546. Resubmission of cheaper-inference#12712 (closed for base drift + column mismatch).

XP-earning hot path was running COUNT(*) on every event. Introduces
incremental counter cache keyed on api_key_id matching xp_audit_log schema.

Cherry-picked from cd7a090 and rebased on upstream/release/v3.8.51.
@KooshaPari
KooshaPari force-pushed the pr/resubmit-12712-xp-cache branch from cd7a090 to c542fd3 Compare September 11, 2026 12:26
@KooshaPari
fix(migration): rename 174_xp_action_counts.sql to 177_xp_action_coun…
ff1e82c 
…ts.sql

Per HouMinXi's review on cheaper-inference#13116: the recut of release/v3.8.51 already has
174_server_tool_executions.sql (cheaper-inference#12867) and 176_provider_connection_synced_models_at.sql
(cheaper-inference#13248), so this PR's 174_xp_action_counts.sql trips the migration-runner
collision guard. Resolution: rename to 177 so it merges vs tip directly.

Migration numbers are file-name-based in this repo (no central registry),
so the only other reference was the test description (also updated).

Verification: 6/6 node --test tests/unit/gamification/db-gamification.test.ts pass.
@KooshaPari

KooshaPari commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Shipped the rename directly in PR pr/resubmit-12712-xp-cache (latest commit on the same branch) instead of waiting for tip to clear 174:

  • src/lib/db/migrations/174_xp_action_counts.sql -> src/lib/db/migrations/177_xp_action_counts.sql
  • Test description updated from migration 174 to migration 177 (only other reference — migration numbers are file-name-based, no central registry)

Verification:

node --test tests/unit/gamification/db-gamification.test.ts
  6/6 pass (cache increment + COUNT(*) fallback + unknown key)

The branch is now mergeable vs tip without any rebase / re-cut on your end.

Re: your recut branch — happy to mirror the same rename there if you'd prefer to land the rename via your recut rather than this PR; just let me know which path is cleaner for the merge.

@KooshaPari
merge: fork's review fixes for xp-cache migration rename
c6f01ff 
Merge origin/pr/resubmit-12712-xp-cache into local. Resolves conflict in
tests/unit/gamification/db-gamification.test.ts (174 -> 177 in describe()
block, matching the file rename). Drops the stale src/lib/db/migrations/
174_xp_action_counts.sql from the fork's view since the canonical file is
now 177_xp_action_counts.sql.

6/6 gamification tests pass.
diegosouzapw added a commit that referenced this pull request Sep 14, 2026
@diegosouzapw
fix(nvidia): fail open when a synced model catalog goes stale (#12849) (
30bf6af 
#13248)

Merged after renumbering. `176_provider_connection_synced_models_at.sql` collided with `176_xp_action_counts.sql` (#12651), which made the migration runner abort on every DB open. Renamed to **177**; the doc count moves 173 → 174 across README.md, AGENTS.md, llm.txt and the i18n mirrors (operator-approved, 206 numeric substitutions and nothing else).

- `check:migration-numbering`: OK, 174 migrations, no duplicates
- `check:docs-counts` migrations: ✓
- 84/84 across the NVIDIA suite plus the seven DB-touching suites the collision had taken down
- ESLint and `typecheck:core`: exit 0

Heads-up for whoever lands next: **177 is claimed by eight other open PRs** (#13610, #13602, #13580, #13554, #13405, #13331, #13177, #13116) and 176 by #13373 and #13102. With this merged, all of them need to renumber at merge time — `check:migration-numbering` forbids new gaps, so the next free number is always the only valid one.

⚠️ base-red inherited: #12732
@diegosouzapw

diegosouzapw commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the O(1) action-count cache. The same fix already landed in the release branch through #12651: a durable xp_action_counts table incremented in addXp(), read by both getActionCount() and checkActionCountBadges(), with a backfill migration.

#12651 also keeps the SUM(json_extract(metadata, '$.amount')) semantics the badge helper relied on, which a plain row count would change. Closing as covered.

muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
@diegosouzapw
fix(nvidia): fail open when a synced model catalog goes stale (cheape…
c492c39 
…r-inference#12849) (cheaper-inference#13248)

Merged after renumbering. `176_provider_connection_synced_models_at.sql` collided with `176_xp_action_counts.sql` (cheaper-inference#12651), which made the migration runner abort on every DB open. Renamed to **177**; the doc count moves 173 → 174 across README.md, AGENTS.md, llm.txt and the i18n mirrors (operator-approved, 206 numeric substitutions and nothing else).

- `check:migration-numbering`: OK, 174 migrations, no duplicates
- `check:docs-counts` migrations: ✓
- 84/84 across the NVIDIA suite plus the seven DB-touching suites the collision had taken down
- ESLint and `typecheck:core`: exit 0

Heads-up for whoever lands next: **177 is claimed by eight other open PRs** (cheaper-inference#13610, cheaper-inference#13602, cheaper-inference#13580, cheaper-inference#13554, cheaper-inference#13405, cheaper-inference#13331, cheaper-inference#13177, cheaper-inference#13116) and 176 by cheaper-inference#13373 and cheaper-inference#13102. With this merged, all of them need to renumber at merge time — `check:migration-numbering` forbids new gaps, so the next free number is always the only valid one.

⚠️ base-red inherited: cheaper-inference#12732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@diegosouzapw diegosouzapw Awaiting requested review from diegosouzapw diegosouzapw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(gamification): action_count badges are computed over a 30-day-pruned xp_audit_log

4 participants

@KooshaPari @HouMinXi @diegosouzapw