Skip to content

fix(billing): reconcile Cloudflare usage per meter run - #4897

Merged
pandemicsyn merged 6 commits into
mainfrom
fix/cloudflare-reconciliation-run-variance
Jul 30, 2026
Merged

fix(billing): reconcile Cloudflare usage per meter run#4897
pandemicsyn merged 6 commits into
mainfrom
fix/cloudflare-reconciliation-run-variance

Conversation

@pandemicsyn

@pandemicsyn pandemicsyn commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reconcile Cloudflare billing usage per continuous-awake meter generation instead of aggregating every generation sharing a Cloudflare instanceId. Each provider query is keyed by meter interval and bounded by its start/end with a five-second retrieval tolerance, preventing later wakes of the same Durable Object from contaminating earlier runs.
  • Normalize Cloudflare memory and disk byte-seconds by the configured instance capacity, then report their absolute and percentage variance against accepted meter seconds. CPU remains available as a secondary diagnostic because Kilo and Cloudflare bill different CPU concepts.
  • Reduce subject usage result pages from 25 to 15 records so reconciliation can batch the full displayed page into one Cloudflare usage request with 15 independently filtered runs. Retain per-run partial/raw-response attribution and server-side subject/window validation.
  • Make both usage-window fields open the native date/time picker and give their complete values enough space across mobile and desktop layouts.

Architecturally, the Cloudflare Analytics helper now accepts run-scoped queries and combines up to 15 independent filters through GraphQL aliases. Provider rows and partial-result state carry a run key through the router so reused instance IDs remain isolated without one HTTP request per run.

Verification

  • Re-ran read-only Cloudflare probes for two controlled generations using exact boundaries and ±5-second boundaries; both returned identical row counts and memory/disk totals.
  • Queried production meter records and confirmed that the same Cloudflare instanceId can map to separate continuous-awake generations with distinct start/end boundaries.
  • Opened the Cloud Billing usage-records UI locally and verified both start/end controls display complete values, invoke their native picker wiring, and stack without horizontal overflow at mobile width.
  • Additional manual verification:

Reviewer Notes

  • Focus on run-boundary attribution: Cloudflare instanceId identifies the Durable Object but can recur across multiple meter generations.
  • The ±5-second tolerance affects only provider retrieval. Meter accepted seconds and displayed meter boundaries remain exact and unpadded.
  • Reconciliation is capped at the displayed 15 records. A full page uses one aliased usage request plus the settings request; Cloudflare responses remain read-only and are labeled by batch in the raw-response disclosure.

Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts
Comment thread apps/web/src/routers/admin/cloud-billing-skus-router.ts
Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts Outdated
Comment thread apps/web/src/routers/admin/cloud-billing-skus-router.ts
Comment thread apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

Dropping the isolation retry removed the fan-out, but the replacement makes any path-less provider error abort the entire reconciliation and discard rows and raw diagnostics that the single-run path would have returned as partial.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 521 One unscoped GraphQL error in a multi-run batch now throws graphql_error, which the router maps to BAD_GATEWAY, discarding fully populated alias rows, accumulated issues, and the rawResponses disclosure. The identical error on a single-run batch still degrades to partial with usable rows, so behavior depends only on batch.length.

SUGGESTION

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 14 Open thread: MAX_RUN_WINDOWS_PER_REQUEST remains a bare 15 rather than derived from settings.maxNumberOfFields. The author states Cloudflare applies that budget per aliased dataset selection; that claim is not verifiable from this repository, so the constant stays an undocumented provider assumption.
Previously reported issues now resolved
  • The unscoped-error retry no longer re-fans a failed batch into one request per plan, so throttling and complexity errors are no longer amplified and a retry timeout can no longer abort reconciliation mid-recovery.
  • The original provider error is no longer dropped from the result; it is now surfaced in the thrown error message.
Files Reviewed (2 files)
  • apps/web/src/lib/cloudflare/container-usage-analytics.ts - 2 issues
  • apps/web/src/lib/cloudflare/container-usage-analytics.test.ts - 0 issues
Notes and assumptions
  • Incremental pass over f45e618f..506a044c (avoid provider retry fanout) at HEAD 506a044c. Files unchanged in this range were not re-reviewed.
  • Verified correct: dropping (!account && errors.length > 0) from the guard is safe, because an alias-scoped-only error set with null data still throws at the !account check; the batch.length > 1 guard keeps the single-run path on the existing aliasPartial degradation; the alias filter's errorAlias === undefined branch is now reachable only for single-run batches, matching the new throw.
  • No memory-leak surface added: the change removes a recursive call and adds no listeners, timers, or subscriptions; retained response bytes stay bounded by MAX_RETAINED_RAW_BYTES and the retry no longer consumes that budget.
  • No security findings: the new error message quotes only GraphQL error messages, never tokens, headers, or response bodies.
  • Reviewed read-only; tests were not executed in this environment.

Fix these issues in Kilo Cloud

Previous Review Summaries (4 snapshots, latest commit f45e618)

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

Previous review (commit f45e618)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The new per-run isolation retry fixes the batch-wide partial marking, but it re-fans a failed batch into one request per plan with no backoff, which amplifies the throttling and complexity errors that trigger it and can abort the whole reconciliation on a retry timeout or retained-byte overflow.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 521 Unscoped-error retry re-issues the batch as up to 15 back-to-back isolated requests (up to 100 at the plan cap) with no backoff or retry cap; rate-limit and complexity errors cannot be fixed by isolation, and a retry timeout / raw_response_too_large is rethrown and discards all rows already collected.

SUGGESTION

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 535 When isolation succeeds, the early return drops the original unscoped GraphQL error from issues and leaves partial false, so the anomaly survives only inside the raw JSON disclosure.
apps/web/src/lib/cloudflare/container-usage-analytics.ts 14 Open thread: MAX_RUN_WINDOWS_PER_REQUEST is still a bare 15 rather than derived from settings.maxNumberOfFields. The author states Cloudflare applies that budget per aliased dataset selection; that claim could not be verified from this repository, so the constant remains an undocumented provider assumption.
Previously reported issues now resolved
  • A single path-less GraphQL error no longer marks all batched runs partial; unscoped failures are isolated per run and only genuinely failing runs reach partialIds.
  • Per-run issue text now quotes only alias-scoped errors, so one run no longer reports another run's provider error.
  • The raw-response label now counts query windows instead of mislabeling them as runs.
Files Reviewed (3 files)
  • apps/web/src/lib/cloudflare/container-usage-analytics.ts - 3 issues
  • apps/web/src/lib/cloudflare/container-usage-analytics.test.ts - 0 issues
  • apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx - 0 issues
Notes and assumptions
  • Incremental pass over 51bd3405..f45e618f (isolate batched provider errors) at HEAD f45e618f. Files unchanged in this range were not re-reviewed.
  • Verified correct: the retry guard is unreachable for single-plan batches, so per-run degradation still happens through aliasPartial; errorAlias === undefined in the alias filter is only reachable for batch.length === 1, consistent with the retry branch; the failed batch's parse loop is skipped before any row accumulation, so isolation cannot double-count usage into the runKey\0applicationId\0instanceId map; responseIndex keeps batchIndex unique across retried responses; recursion is one level deep and terminates.
  • No memory-leak surface added: no listeners, timers, or subscriptions; retained response bytes stay bounded by MAX_RETAINED_RAW_BYTES, though isolation retries now consume that budget faster.
  • No security findings: no token, header, or response body reaches issues or error messages; retry variables still pass instance IDs and timestamps as GraphQL variables, with only the validated maxPageSize integer interpolated into the query text.
  • Reviewed read-only; tests were not executed in this environment.

Fix these issues in Kilo Cloud

Previous review (commit 51bd340)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The new 15-alias batching fixes all five previous findings, but a single Cloudflare GraphQL error without a path now marks every run in the batch as provider_partial, erasing valid comparisons for up to 15 records.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 537 hasUnscopedError is OR-ed into every alias, so one path-less GraphQL error (throttling, complexity, generic dataset errors) marks all 15 batched runs partial instead of only the failing one.

SUGGESTION

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 555 Per-run issue text joins the batch-wide errors list, so each partial run quotes other runs' provider errors in the admin panel.
apps/web/src/lib/cloudflare/container-usage-analytics.ts 13 MAX_RUN_WINDOWS_PER_REQUEST is a bare 15 while a full batch selects 90 leaf fields; validateSettings still only checks maxNumberOfFields >= 6, so the guard no longer describes the request.
apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx 1025 Raw-response label reports queries.length as "runs", but those entries are query windows; a multi-window run inflates the count.
Previously reported issues now resolved
  • Out-of-retention runs no longer throw; they degrade per run via usageUnavailableRuns and surface as comparison_unavailable.
  • The grouped reconciliation query now has a deterministic ORDER BY started_at, id.
  • Sequential per-run fan-out is replaced by aliased batching (a full 15-row page is now 1 settings + 1 usage request).
  • The run cap constant is single-sourced and matches the Zod bound; it still guards direct callers of the exported reconcileUsageWithCloudflare, so it is no longer dead duplication.
  • Run keys are now bare interval IDs, so no run\0 composite key reaches the UI.
Files Reviewed (5 files)
  • apps/web/src/lib/cloudflare/container-usage-analytics.ts - 3 issues
  • apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx - 1 issue
  • apps/web/src/routers/admin/cloud-billing-skus-router.ts - 0 issues
  • apps/web/src/lib/cloudflare/container-usage-analytics.test.ts - 0 issues
  • apps/web/src/routers/admin/cloud-billing-skus-router.test.ts - 0 issues
Notes and assumptions
  • Incremental pass over commit 51bd3405 (batched Cloudflare run queries) at HEAD 51bd3405.
  • Verified correct: alias-to-plan index mapping across query text, variables, rawResponses.queries, and the parse loop; batches() chunking (16 plans to 15+1) with no dropped tail; row aggregation keyed by runKey\0applicationId\0instanceId so reused instance IDs cannot merge across runs; only the validated integer limit is interpolated into the GraphQL text, with run keys, instance IDs, and timestamps passed as variables; retention clipping vs. full expiry are mutually exclusive per run.
  • No memory-leak surface added: no listeners, timers, or subscriptions; retained raw bytes remain bounded by MAX_RETAINED_RAW_BYTES, and batching reduces the number of retained responses.
  • No security findings: intervalIds still reach PostgreSQL through a parameterized inArray, subject scoping is server-side, the endpoint remains adminProcedure, and no token or header material enters issues, rawResponses, or error messages.
  • Nullability of viewer/accounts in UsageResponseSchema is .optional() but not .nullable(); that gap predates this PR, so it is not reported inline.
  • Reviewed read-only; tests were not executed in this environment.

Fix these issues in Kilo Cloud

Previous review (commit 20d2835)

Status: 5 Issues Found | Recommendation: Address before merge

Executive Summary

No application code changed since the previous review; the per-run Cloudflare retention check still throws instead of degrading, so one out-of-retention interval in the displayed page blocks reconciliation of every other selected record.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 415 Per-run outside_retention throw aborts the whole reconciliation; previously the window was clamped and marked partial. The UI has no way to deselect the offending record and the error is not retryable.
apps/web/src/routers/admin/cloud-billing-skus-router.ts 514 Grouped reconciliation query has no ORDER BY, so per-run row order is nondeterministic in the admin table and the order-sensitive test assertion can flake.

SUGGESTION

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 443 1 batched request becomes 25+ strictly sequential upstream calls at 10s timeout each inside one tRPC mutation; consider bounded concurrency.
apps/web/src/routers/admin/cloud-billing-skus-router.ts 538 MAX_RECONCILIATION_RUNS check is unreachable via the endpoint because Zod already caps intervalIds at 25; the accompanying test asserts a path no API caller can reach.
apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx 1024 Raw-response heading and provider issues render the internal run\0<intervalId> composite key, embedding a literal NUL character in visible text.
Files Reviewed (5 files)
  • apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx - 1 issue
  • apps/web/src/lib/cloudflare/container-usage-analytics.ts - 2 issues
  • apps/web/src/routers/admin/cloud-billing-skus-router.ts - 2 issues
  • apps/web/src/lib/cloudflare/container-usage-analytics.test.ts - 0 issues
  • apps/web/src/routers/admin/cloud-billing-skus-router.test.ts - 0 issues (see the ordering warning above, which affects an assertion in this file)
Notes and assumptions
  • Incremental pass: the two commits added since the last reviewed commit add and then delete scripts/probe-cloudflare-container-usage.mjs, so the net diff versus the previously reviewed tree is empty and no new code was introduced.
  • All five prior findings were re-verified against current HEAD (20d2835b) and still reproduce at the same lines, so no new inline comments were posted.
  • No new memory-leak surface: no added listeners, timers, or subscriptions; retained raw-response bytes stay bounded by MAX_RETAINED_RAW_BYTES.
  • No security findings: intervalIds reach the database through a parameterized inArray, server-side subject scoping is preserved, and the endpoint remains adminProcedure.
  • Reviewed read-only; tests were not executed in this environment.

Fix these issues in Kilo Cloud

Previous review (commit b860dc0)

Status: 5 Issues Found | Recommendation: Address before merge

Executive Summary

The new per-run Cloudflare retention check throws instead of degrading, so a single out-of-retention interval in the displayed page blocks reconciliation of every other selected record.

Overview

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

WARNING

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 415 Per-run outside_retention throw aborts the whole reconciliation; previously the window was clamped and marked partial. UI has no way to deselect the offending record and PRECONDITION_FAILED is not retryable.
apps/web/src/routers/admin/cloud-billing-skus-router.ts 514 Grouped reconciliation query has no ORDER BY, so per-run row order is nondeterministic in the admin table and the new order-sensitive test assertion can flake.

SUGGESTION

File Line Issue
apps/web/src/lib/cloudflare/container-usage-analytics.ts 443 1 batched request becomes 25+ strictly sequential upstream calls at 10s timeout each inside one tRPC mutation; consider bounded concurrency.
apps/web/src/routers/admin/cloud-billing-skus-router.ts 538 MAX_RECONCILIATION_RUNS check is unreachable via the endpoint because Zod already caps intervalIds at 25; the accompanying test asserts a path no API caller can reach.
apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx 1024 Raw-response heading and provider issues render the internal run\0<intervalId> composite key, embedding a literal NUL character in visible text.
Files Reviewed (5 files)
  • apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx - 1 issue
  • apps/web/src/lib/cloudflare/container-usage-analytics.ts - 2 issues
  • apps/web/src/routers/admin/cloud-billing-skus-router.ts - 2 issues
  • apps/web/src/lib/cloudflare/container-usage-analytics.test.ts - 0 issues
  • apps/web/src/routers/admin/cloud-billing-skus-router.test.ts - 0 issues (see the ordering warning above, which affects an assertion in this file)
Notes and assumptions
  • Verified no new memory-leak surface: no added listeners, timers, or subscriptions; retained raw-response bytes stay bounded by MAX_RETAINED_RAW_BYTES.
  • No security findings: intervalIds reach the database through a parameterized inArray, server-side subject scoping is preserved, and the endpoint remains adminProcedure.
  • Reviewed read-only; tests were not executed in this environment.
  • The capacityCrossCheck branch now assigns compared in both arms and only varies statusDetail, which matches the PR's stated intent, so it is not reported as a defect.

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 34 · Output: 9.8K · Cached: 814.5K

Review guidance: REVIEW.md from base branch main

Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts Outdated
Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts Outdated
const MAX_INSTANCE_BATCH_SIZE = 50;
const MAX_PROVIDER_REQUESTS = 100;
const MAX_PROVIDER_QUERY_PLANS = 100;
const MAX_RUN_WINDOWS_PER_REQUEST = 15;

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.

SUGGESTION: Batch size is not derived from the dataset's advertised field budget

A full batch selects 6 leaf fields per alias (dimensions.applicationId, dimensions.instanceId, and 4 sums), i.e. 90 for 15 aliases, while validateSettings still only asserts maxNumberOfFields >= REQUIRED_FIELDS.length (6) and the test fixture advertises maxNumberOfFields: 30. If Cloudflare applies that limit per request rather than per node, a full batch is rejected outright and takes 15 runs down together, even though the settings check reported the account as usable. Deriving the cap (e.g. Math.min(MAX_RUN_WINDOWS_PER_REQUEST, Math.max(1, Math.floor(settings.maxNumberOfFields / REQUIRED_FIELDS.length)))) would make the request provably within the advertised budget.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No behavior change: Cloudflare applies maxNumberOfFields per aliased dataset selection, not cumulatively across aliases. The account advertises 30 fields and a live 15-alias request selecting 6 fields per alias succeeds. Added an explanatory comment in f45e618.

Comment thread apps/web/src/app/admin/cloud-billing-skus/UsageRecordsContent.tsx Outdated
Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts Outdated
Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts Outdated
Comment thread apps/web/src/lib/cloudflare/container-usage-analytics.ts
@pandemicsyn
pandemicsyn merged commit 73034e1 into main Jul 30, 2026
21 checks passed
@pandemicsyn
pandemicsyn deleted the fix/cloudflare-reconciliation-run-variance branch July 30, 2026 20:55
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