[iscekic]

Evidence

Evidence: demonstrated — an isolated local application and PostgreSQL run exercises personal and organization expiration through the balance callers and HTTP routes. Paid-request controls use a local fixture provider.

Change

Single-account expiration reads current primary account values under a row lock. Balance callers use the returned usage and acquired credits together. Explicit force checks retain their refresh behavior.

Expiration inserts use bounded batches. Baseline changes use a set-based update. Completed expiration checks return current account state without scanning the credit history again. Organization detail responses refresh the complete row.

Expiry calculations use the canonical paid-credit reconciler. Closure queries use bounded lookups through the unique original-transaction link.

Verification

• The focused database-backed expiration, balance, organization router, dispatch, consumption, and repair suites pass.
• The local HTTP scenarios cover current balances, replay, paid-request rejection, and paid-request settlement for personal and organization accounts.
• The contention run completes expiration while usage writes remain active, and the writer continues afterward.
• Large-grant runs cover partially and fully consumed credits.
• Large closed-history checks pass before and after a manual statistics refresh, with query plans using the unique-link index.
• Database, shared web, and web type checks pass.
• Scoped lint and formatting checks pass.

Performance limits

Synthetic stress runs still show owner-specific writer stalls when many consumed grants expire together. The canonical allocation calculation retains quadratic work.

The closed-history query no longer depends on refreshed planner statistics. These isolated measurements do not establish production tail latency or workload frequency.

Production prerequisites

No schema migration, environment variable, secret, feature flag, or external service change is required.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental merge replaces the snapshot-based expiration wrappers with the canonical reconcileCreditExpirationsInTransaction caller-path (userId/organizationId + row lock, onlyIfDue by default), rewrites both fetchExpiringTransactions* closure anti-joins to a correlated LEFT JOIN LATERAL ... LIMIT 1 on original_transaction_id, and updates every balance/router/cron caller to consume the returned state coherently; I found no defect in the changed lines.

Assumptions: [INFERENCE] I did not execute the suites or runtime scenarios. I verified credit_transactions.original_transaction_id is covered by a non-partial global unique index (packages/db/src/schema.ts:382, packages/db/src/migrations/0000_baseline.sql:1221, never dropped), which is what makes the unordered LIMIT 1 closure lookup deterministic and keeps the moved owner/category predicates equivalent to the previous anti-join. Minor observation, not a defect: the cron calls processLocalExpirations/processOrganizationExpirations without force, so a row whose hint advances between the cron SELECT and its turn (e.g. forceImmediateExpirationRecomputation) is skipped for that run; the hint remains due for the next cron run or lazy balance read, matching the intended lazy-expiry design.

Files Reviewed (25 files)

• DEVELOPMENT.md
• apps/web/src/app/admin/api/organizations/[id]/force-expiration-check/route.ts
• apps/web/src/app/api/cron/expire-credits/route.ts
• apps/web/src/app/api/cron/expire-credits/route.test.ts
• apps/web/src/lib/code-reviews/dispatch/dispatch-billing.ts
• apps/web/src/lib/coding-plans/billing-lifecycle-cron.test.ts
• apps/web/src/lib/kilo-pass-org/consumption.test.ts
• apps/web/src/lib/recomputeNextCreditExpiration.test.ts
• apps/web/src/routers/organizations/organization-admin-router.ts
• apps/web/src/routers/organizations/organization-funds-router.ts
• apps/web/src/routers/organizations/organization-funds-router.test.ts
• apps/web/src/routers/organizations/organization-router.ts
• apps/web/src/routers/organizations/organization-router.test.ts
• apps/web/src/scripts/d2026-06-26_expire-internal-free-credits.ts
• apps/web/src/tests/profile-balance.test.ts
• packages/db/src/credit-expiration.ts
• packages/web-shared/src/lib/balance-funding.test.ts
• packages/web-shared/src/lib/creditExpiration.ts
• packages/web-shared/src/lib/creditExpiration.concurrent.test.ts
• packages/web-shared/src/lib/creditExpiration.test.ts
• packages/web-shared/src/lib/creditExpirationOrg.test.ts
• packages/web-shared/src/lib/organizations/organization-usage.ts
• packages/web-shared/src/lib/organizations/organization-usage.test.ts
• packages/web-shared/src/lib/paidCreditAccounting.test.ts
• packages/web-shared/src/lib/user/balance.ts

Previous Review Summaries (4 snapshots, latest commit b22bbb2)

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

Previous review (commit b22bbb2)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit replaces the owner/organization-scoped DISTINCT closure derived tables with a correlated LEFT JOIN LATERAL ... LIMIT 1 on original_transaction_id and moves the owner/organization and category predicates into the join condition; because a global unique index exists on credit_transactions.original_transaction_id, at most one closure row can match a link, so the anti-join semantics for already-closed credits are preserved and no defect appears in the changed lines.

Assumptions: [INFERENCE] I did not execute the suites; packages/db/src/migrations/0000_baseline.sql defines CREATE UNIQUE INDEX "IDX_credit_transactions_original_transaction_id" and no later migration drops it, which is what makes the unordered LIMIT 1 selection deterministic. The open evidence hold was cleared by the author's runtime-evidence comment.

Files Reviewed (4 files)

• DEVELOPMENT.md
• packages/web-shared/src/lib/creditExpiration.ts
• apps/web/src/lib/recomputeNextCreditExpiration.test.ts
• packages/web-shared/src/lib/creditExpirationOrg.test.ts

Previous review (commit c6bf63d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit rewrites the closure lookup in fetchExpiringTransactions and fetchExpiringTransactionsForOrganization from a kilo_user_id/organization_id-scoped join alias to an account-scoped SELECT DISTINCT original_transaction_id derived table joined only on the transaction id; the anti-join semantics for already-closed credits are unchanged and no defect appears in the changed lines.

Assumptions: [INFERENCE] I did not execute the suites; the rewrite is behavior-preserving by inspection (the removed join predicates move into the derived table's own WHERE, with the same category sets and owner scoping) and remains exercised indirectly through processLocalExpirations in creditExpiration.test.ts and directly in creditExpirationOrg.test.ts. The open do-not-merge hold is an evidence/merge-policy matter, not a code defect found here.

Files Reviewed (2 files)

• packages/web-shared/src/lib/creditExpiration.ts
• packages/web-shared/src/lib/creditExpiration.concurrent.test.ts

Previous review (commit 35073eb)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The rewritten single-entity expiration writers (processLocalExpirations, processOrganizationExpirations) now lock the owner row on the primary and compute from that committed state, and every updated caller consumes the returned microdollars_used/total_microdollars_acquired/next_credit_expiration_at coherently; I found no defect in the changed lines across the 16 files.

Assumptions: [INFERENCE] I did not execute the suites or runtime flows; conclusions come from reading the diff and tests. Expiration now holds the owner row lock for the whole transaction (bounded inserts of 1,000 rows), so concurrent usage writes for that account queue behind it — that is the intended serialization, but worst-case lock hold time for very large histories was not measured. Non-code note: the PR description still describes the superseded optimistic-UPDATE implementation ("include the persisted microdollars_used in their optimistic UPDATE predicate", "log the concurrency failure and return null", "no signature or caller changes"), which no longer matches HEAD. The operator hold on this PR asks for runtime evidence through the balance callers; that is an evidence/merge-policy question, not a defect found in the code.

Files Reviewed (16 files)

• DEVELOPMENT.md
• apps/web/src/app/admin/api/organizations/[id]/force-expiration-check/route.ts
• apps/web/src/lib/code-reviews/dispatch/dispatch-billing.ts
• apps/web/src/lib/kilo-pass-org/consumption.test.ts
• apps/web/src/routers/organizations/organization-admin-router.ts
• apps/web/src/routers/organizations/organization-funds-router.ts
• apps/web/src/routers/organizations/organization-router.test.ts
• apps/web/src/routers/organizations/organization-router.ts
• apps/web/src/scripts/d2026-06-26_expire-internal-free-credits.ts
• apps/web/src/tests/profile-balance.test.ts
• packages/web-shared/src/lib/creditExpiration.concurrent.test.ts
• packages/web-shared/src/lib/creditExpiration.test.ts
• packages/web-shared/src/lib/creditExpiration.ts
• packages/web-shared/src/lib/creditExpirationOrg.test.ts
• packages/web-shared/src/lib/organizations/organization-usage.ts
• packages/web-shared/src/lib/user/balance.ts

Previous review (commit da034a9)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The usage-snapshot guard added to both single-entity expiration writers is correct and well covered by the new applied/deferred tests for each entity; no defects found in the changed lines.

Assumptions: [INFERENCE] Balance-check callers can supply microdollars_used from a replica read (getBalanceAndOrgSettings(..., readDb) in the AI gateway, and replica-resolved auth feeding getBalanceForUser(ctx.user)). Under replica lag, the new exact-equality predicate defers the expiration write and reaches the pre-existing error-level concurrency log more often. Deferring is the safe direction (it avoids over-expiring on a stale, under-counted usage snapshot), and the actual frequency of that deferral was not measurable from the diff.

Files Reviewed (3 files)

• packages/web-shared/src/lib/creditExpiration.ts
• packages/web-shared/src/lib/creditExpiration.test.ts
• packages/web-shared/src/lib/creditExpirationOrg.test.ts

---

_{Reviewed by deepseek-flash-latest · Input: 160.7K · Output: 39.6K · Cached: 4.2M}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

bot: Holding this PR during the autonomous merge review. The credit-expiry guard has database-backed tests, but separate runtime evidence through the balance callers is not established. The do-not-merge label records this hold; no code defect is implied where the blocker is missing evidence.

[iscekic]

bot: Runtime evidence through the balance callers is now established, so this clears the evidence hold.

• Personal and organization balances computed under a held usage write return the post-expiration balance.
• Paid requests during that race are rejected and do not reach the local fixture provider.
• Under continuous usage writes, expiration completes and the usage writer keeps making progress.
• The organization detail response returns a consistent refreshed row after expiration.

The checks used synthetic accounts and a disposable local PostgreSQL database.

[iscekic]

bot: The renewed performance hold is cleared after testing the canonical paid-credit integration on the current head.

• Personal and organization HTTP scenarios pass, including paid settlement controls and stale-balance rejection without provider calls.
• Runtime scenarios verify concurrent readers, rollback, exact accounting, replay, and batch-versus-single expiration.
• Closed-history checks pass before and after a manual statistics refresh. Query plans use bounded unique-link lookups.
• CI and the current-head code review pass. No review threads remain unresolved.

Performance limit: large consumed-grant sets still cause owner-specific writer stalls. The canonical allocation calculation retains quadratic work. The PR description records this limit; isolated measurements do not establish production tail latency.

No additional production setup is required. This clears the verification hold, not authorization to merge.