Skip to content

Revert "Revert "feat(web): serve /usage analytics from Postgres replica"" - #5229

Merged
chrarnoldus merged 3 commits into
mainfrom
revert-5228-revert-5223-chrarnoldus/usage-analytics-postgres
Aug 13, 2026
Merged

Revert "Revert "feat(web): serve /usage analytics from Postgres replica""#5229
chrarnoldus merged 3 commits into
mainfrom
revert-5228-revert-5223-chrarnoldus/usage-analytics-postgres

Conversation

@chrarnoldus

Copy link
Copy Markdown
Contributor

Reverts #5228

Comment thread apps/web/src/routers/usage-analytics-router.test.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental re-review of the new commits (dedicated usage-replica pool on POSTGRES_REPLICA_EU_URL_2 plus the fixed case-insensitive is null test assertion) found no issues; the previous SUGGESTION is resolved and no memory-leak vectors were introduced (the new pool is attached via attachDatabasePool, has an error handler, and is closed in closeAllDrizzleConnections).

Files Reviewed (4 files)
  • apps/web/src/lib/drizzle.ts
  • apps/web/src/lib/drizzle.test.ts
  • apps/web/src/routers/usage-analytics-router.ts
  • apps/web/src/routers/usage-analytics-router.test.ts
Previous Review Summary (commit 8d6b79d)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 8d6b79d)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Re-lands the Snowflake→Postgres replica migration for /usage analytics (unchanged from #5223); the only finding is a case-sensitive test assertion that cannot catch the regression it guards against.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/routers/usage-analytics-router.test.ts 71 not.toContain('IS NULL') is case-sensitive while drizzle emits lowercase is null, so the negative assertion always passes

Notes: scope/SQL changes were verified against packages/db schema — all joins (microdollar_usage_metadata, feature, mode) are 1:1 on primary keys, so no aggregate fan-out; all user input is parameterized (no injection paths); the organization_id IS NULL personal-scope predicate correctly replaces Snowflake's empty-string sentinel. No memory-leak vectors introduced (the AbortController/setTimeout timeout path was removed in favor of SET LOCAL statement_timeout via the pre-existing timedUsageQuery). Behavior changes (caller-supplied ISO date windows instead of day/month-aligned bounds, 500 instead of zeroed summaries when the backend is unreachable) match the intent documented in #5223.

Fix these issues in Kilo Cloud

Files Reviewed (2 files)
  • apps/web/src/routers/usage-analytics-router.ts - 0 issues
  • apps/web/src/routers/usage-analytics-router.test.ts - 1 issue

Reviewed by kimi-k3 · Input: 117.8K · Output: 2.8K · Cached: 135.5K

Review guidance: REVIEW.md from base branch main

Route usage analytics aggregates through POSTGRES_REPLICA_EU_URL_2 so
they do not share the standard readDb pool. Keep org/user lookups on
readDb. Make the self-view IS NULL guard case-insensitive.

Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
@chrarnoldus chrarnoldus self-assigned this Aug 12, 2026
@chrarnoldus
chrarnoldus enabled auto-merge (squash) August 13, 2026 09:19
…23-chrarnoldus/usage-analytics-postgres

# Conflicts:
#	apps/web/src/lib/drizzle.test.ts
#	apps/web/src/routers/usage-analytics-router.ts
@chrarnoldus
chrarnoldus merged commit d83bf8e into main Aug 13, 2026
17 checks passed
@chrarnoldus
chrarnoldus deleted the revert-5228-revert-5223-chrarnoldus/usage-analytics-postgres branch August 13, 2026 09:31
pandemicsyn added a commit that referenced this pull request Aug 28, 2026
Revert #5229 (d83bf8e), retaining user BYOK semantics through Snowflake's user_byok_request_count rollup. Surface unavailable configuration instead of zero usage, avoid logging upstream response bodies, and cover the restored queries with regression tests.
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