Skip to content

perf(organizations): read group policy from replica - #5053

Merged
chrarnoldus merged 1 commit into
mainfrom
use-read-replica-for-org-policy
Aug 6, 2026
Merged

chrarnoldus merged 1 commit into
mainfrom
use-read-replica-for-org-policy

Conversation

@chrarnoldus

@chrarnoldus chrarnoldus commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • run organization group-policy snapshots through readDb instead of the primary client
  • preserve the existing repeatable-read, read-only transaction semantics
  • keep explicit caller-supplied transactions authoritative
  • add regression coverage for replica selection and transaction options

Tradeoff

Policy reads can observe normal replica lag while remaining internally consistent within the repeatable-read snapshot. readDb falls back to primary where no replica is configured.

Validation

  • pnpm --filter web typecheck
  • focused Jest suites: 3 passed, 35 tests
  • changed-file oxlint: 0 warnings, 0 errors
  • pnpm format
  • git diff --check

@chrarnoldus chrarnoldus self-assigned this Aug 5, 2026
@kilo-code-bot

kilo-code-bot Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The new commit opts the global chat route's inference-time policy read into replica-backed snapshots via consistency: 'eventual' — a read-only authorization path with no same-request read-after-write dependency, matching the PR's documented lag trade-off and falling back to primary where no replica exists — and the added test assertion pins the exact call shape against a properly mocked module.

Files Reviewed (2 files)
  • apps/web/src/app/api/openrouter/[...path]/route.test.ts
  • apps/web/src/app/api/openrouter/[...path]/route.ts
Previous Review Summaries (3 snapshots, latest commit 6892a4e)

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

Previous review (commit 6892a4e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The previous finding on hard-coded eventual consistency is resolved: getAvailableModelsForOrganization now defaults to strong consistency and only the three read-only presentation callers (openrouter models catalog, listAvailableModels, model-preference filtering) opt into replica reads, while both mutation-time validation paths (auto-routing pool entries, organization default-model replacement) omit the option and stay on primary, with new test assertions pinning both call shapes.

Files Reviewed (6 files)
  • apps/web/src/app/api/openrouter/models/route.test.ts
  • apps/web/src/app/api/openrouter/models/route.ts
  • apps/web/src/lib/ai-gateway/auto-routing-pool-validation.test.ts
  • apps/web/src/lib/organizations/organization-models.ts
  • apps/web/src/routers/model-preferences-router.ts
  • apps/web/src/routers/organizations/organization-settings-router.ts

Previous review (commit 0183b7e)

Status: 1 Issues Found | Recommendation: Address before merge

Executive Summary

getAvailableModelsForOrganization hard-codes consistency: 'eventual', which also routes two mutation-time validation paths (auto-routing pool entry validation and organization default-model replacement validation) through the read replica, contrary to the consistency contract limiting it to read-only presentation paths.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/organizations/organization-models.ts 29 Hard-coded consistency: 'eventual' also applies to mutation-time validation callers (validatePoolEntries via buildEligibleCatalog, validateOrganizationDefaultReplacement), letting a just-denied model pass save-time validation during the replica-lag window
Files Reviewed (8 files)
  • apps/web/src/app/api/gateway/embedding-models/route.ts
  • apps/web/src/app/api/gateway/transcription-models/route.ts
  • apps/web/src/app/api/openrouter/models-by-provider/route.ts
  • apps/web/src/app/api/organizations/[id]/defaults/route.ts
  • apps/web/src/lib/organizations/group-policies/model-access/model-access.server.ts
  • apps/web/src/lib/organizations/organization-group-policy-context.server.test.ts
  • apps/web/src/lib/organizations/organization-group-policy-context.server.ts
  • apps/web/src/lib/organizations/organization-models.ts - 1 issue

Fix these issues in Kilo Cloud

Previous review (commit 109c795)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Routing organization group-policy snapshot reads through readDb is safe here: all callers are read-only enforcement paths with no same-request read-after-write dependency, readDb falls back to the primary pool when no replica is configured, the repeatable-read/read-only transaction options are preserved, and the narrowed resolveOrganization parameter type matches its only call site; regression coverage asserts replica selection and transaction options.

Files Reviewed (2 files)
  • apps/web/src/lib/organizations/organization-group-policy-context.server.ts
  • apps/web/src/lib/organizations/organization-group-policy-context.server.test.ts

Reviewed by kimi-k3 · Input: 44.4K · Output: 8.9K · Cached: 356.4K

Review guidance: REVIEW.md from base branch main

@chrarnoldus chrarnoldus changed the title perf(organizations): read group policy from replica perf(organizations): use replicas for policy catalogs Aug 5, 2026
@chrarnoldus chrarnoldus changed the title perf(organizations): use replicas for policy catalogs perf(organizations): use replicas for selected policy reads Aug 5, 2026
@chrarnoldus
chrarnoldus force-pushed the use-read-replica-for-org-policy branch from dbd6565 to 109c795 Compare August 6, 2026 09:45
@chrarnoldus chrarnoldus changed the title perf(organizations): use replicas for selected policy reads perf(organizations): read group policy from replica Aug 6, 2026
@chrarnoldus
chrarnoldus merged commit b5a870f into main Aug 6, 2026
26 of 29 checks passed
@chrarnoldus
chrarnoldus deleted the use-read-replica-for-org-policy branch August 6, 2026 10:45
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