Repository navigation
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| <dd>{formatSessionCost(sessionCostMusd / 1_000_000)}</dd> | ||
| </div> | ||
| )} | ||
| {data.containerCostMusd !== null && ( |
There was a problem hiding this comment.
SUGGESTION: Sub-cent container cost renders as "$0.00"
containerCostMusd !== null shows this row for any positive charge, but formatSessionCost rounds to cents. At the code-review container rate ($0.18/hr) a container shorter than ~100 seconds costs less than $0.005 and will display "Container cost $0.00" for a charge that exists. The model-cost row above (line 288) guards the same situation with isRenderableSessionCost(...). Apply the same guard here (or use formatSessionTotalCost to render "<$0.01"), and use != null to match the sibling row.
| {data.containerCostMusd !== null && ( | |
| {data.containerCostMusd != null && isRenderableSessionCost(data.containerCostMusd / 1_000_000) && ( |
| .from(cloud_billing_sku) | ||
| .where(eq(cloud_billing_sku.id, SANDBOX_USAGE_SKUS.SandboxContainersCodeReview)) | ||
| .limit(1); | ||
| return { enabled: true, hourlyUsd: rate ? Number(rate.rate) * 36 : null }; |
There was a problem hiding this comment.
SUGGESTION: Advertised rate ignores SKU availability
readComputeRates selects accepts_new_usage, and container admission rejects starts when the SKU is disabled. This lookup returns the price unconditionally, so if cloud-agent-code-review-2026-07 is disabled while an account is in paid mode, the settings notice advertises an hourly rate even though every code-review start would be refused at admission. Filtering on cloud_billing_sku.accepts_new_usage (and returning hourlyUsd: null when it is false) would keep the notice consistent with admission.
| ) | ||
| .where( | ||
| and( | ||
| inArray(container_usage_interval.session_id, sessionIds), |
There was a problem hiding this comment.
SUGGESTION: Add a created_at lower bound to enable partition pruning
compute_usage_charge is partitioned monthly on created_at and its payer indexes are (user_id, created_at) / (organization_id, created_at). Without a created_at predicate this lookup must probe the payer index in every month partition that has ever existed, and it runs on the review detail endpoint the page polls. A review's container charges cannot predate the review (or its attempts), so passing that timestamp as a lower bound on compute_usage_charge.created_at would allow partition pruning.
| it.each(['paid', 'shadow'] as const)('reports the admission billing decision: %s', async mode => { | ||
| vi.mocked(resolveBillingMode).mockResolvedValueOnce(mode); | ||
| await expect( | ||
| createMeter().getBillingMode({ |
There was a problem hiding this comment.
SUGGESTION: Test does not assert the forwarded service and subject
This stubs resolveBillingMode and only asserts the resolved mode, so getBillingMode could pass the wrong service or subject and the test would still pass. The core guarantee here is that this read path resolves the same decision as admission, so assert the forwarded arguments, e.g. expect(resolveBillingMode).toHaveBeenCalledWith(expect.anything(), expect.anything(), 'cloud-agent-next-sandbox-containers-code-review', { type: 'user', id: 'oauth/provider/user' }, expect.any(Date)).
Code Review SummaryStatus: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe two highest-risk findings are on the billing read paths: a supplementary container-cost read can turn a healthy review page into a permanent "not found" on any transient DB error, and the settings rate lookup advertises a price without the unit/availability checks that admission enforces. Assumptions: Live end-to-end container-cost linkage was not exercised; the session-id key match is Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit a3ca2c6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a3ca2c6)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (18 files)
Verified: the Note: end-to-end container-cost linkage against a live code-review container was not exercised in the PR; the screenshots use seeded ledger rows. The session-id key match is Reviewed by deepseek-flash-latest · Input: 39.7K · Output: 7.5K · Cached: 194.3K Review guidance: REVIEW.md from base branch |
iscekic
left a comment
iscekic
left a comment
There was a problem hiding this comment.
bot: Automated code review for this pull request, requested by @eshurakov.
This change adds a read-only billing-status surface for code review compute and shows it to customers. The meter gains getBillingMode, which reuses resolveBillingMode extracted from the admission path (including the unreviewed-enterprise exemption), so the read and admission agree. Cloud Agent exposes a new internal read-only route, gated by the existing timing-safe internal-key check and a Zod-validated {type,id} subject. Web adds computeBilling.codeReviewRate, which authorizes org access, asks the Cloud Agent worker, and prices the dedicated cloud-agent-code-review-2026-07 SKU; a new CodeReviewComputeNotice renders enabled/disabled/price-missing/error-with-retry states in the GitHub, GitLab, and Bitbucket config forms. codeReviews.get now sums recorded container charges for the review's session and every attempt session and the detail page shows that as Container cost, with the existing session compute label renamed to match. The head commit is the generated kilo-mcp catalog refresh; the new catalog entry is a query, not a write, so it does not expand credential or mutation reach.
What I checked: the new query reuses the ledger join shape of readSessionComputeCharges (segment idempotency key -> compute_usage_charge.usage_source_id, one delta charge row per settled segment), the payer filter matches the charge-writing code (user_id for user subjects, organization_id for org subjects), and container_usage_interval.session_id is the same cloud-agent session id stored on the review and attempts that readSessionComputeCharges relies on. The meter RPC parses its own input and uses per-call getContainerUsageDb/this.env, and the endpoint requires the internal secret before touching the meter. The extracted resolveBillingMode preserves the admission decision and its enterprise exemption. I could not run the suites or a live container, so the real-session-ledger match and the production SKU row remain unverified at runtime (the PR says the same).
Act first on the failure isolation in code-reviews-router.ts: the new container-cost read is supplementary, but a single query error currently turns a healthy review page into "Code review not found" with polling switched off. Then fix the two display/price issues: sub-cent charges shown as $0.00, and a rate read without the unit/availability checks admission applies.
E2E: not run.
Not run. The checkout has no node_modules and this job does not install dependencies, so no Jest, Vitest, typecheck, or app run was possible. Review was static: the PR diff plus the surrounding repository sources (routers, meter, admission path, schema, UI). I did parse the changed services/kilo-mcp/catalog.json to confirm it is valid JSON and contains the new read-only query entry.
Every remark is attached to the changed line where GitHub accepts one.
This service never pushes, rebases, forces, closes, labels, or merges, and it posts nothing else on this pull request.
| cached: 0, | ||
| }; | ||
|
|
||
| const containerCostMusd = await readCodeReviewContainerCost( |
There was a problem hiding this comment.
WARNING: A transient failure of this supplementary read fails the whole review page.
readCodeReviewContainerCost is awaited inside the get try block. Any error from it falls through to the catch below, which returns failureResult(error.message); CodeReviewDetailClient treats !data?.success as "Code review not found" and its refetchInterval returns false for an unsuccessful result, so the user is stranded on the not-found screen with no automatic retry (and other callers of codeReviews.get receive the raw database message in error). The model-cost read beside it has the opposite contract: getSessionUsageFromBilling catches, reports via captureException, and returns null so a supplementary billing lookup can never fail the review read (apps/web/src/lib/code-reviews/db/code-reviews.ts:2629-2635).
Smallest fix: give the container-cost read the same tolerance — wrap the readCodeReviewContainerCost call in try/catch, log with captureException, and leave containerCostMusd null on failure. While there, move the call into the Promise.all above so the 5-second in-flight poll does not add a serial database round trip.
| .from(cloud_billing_sku) | ||
| .where(eq(cloud_billing_sku.id, SANDBOX_USAGE_SKUS.SandboxContainersCodeReview)) | ||
| .limit(1); | ||
| return { enabled: true, hourlyUsd: rate ? Number(rate.rate) * 36 : null }; |
There was a problem hiding this comment.
WARNING: The advertised price skips the SKU checks that admission enforces.
Number(rate.rate) * 36 is only the cents-per-second -> USD-per-hour conversion. Admission reads the same SKU and rejects a row whose unit !== 'second' (sku_unit_mismatch) or whose acceptsNewUsage is false (sku_not_accepting_new_usage) — services/container-usage-meter/src/postgres.ts:546-569. This lookup filters on id alone, so a row in another unit would be multiplied by 36 and shown as an hourly price, and a disabled SKU would still be advertised as the price of a paid resource. readComputeRates in apps/web/src/lib/cloud-agent/compute-billing.ts already filters cloud_billing_sku.unit = 'second' for the same reason.
Smallest fix: filter on unit = 'second' and accepts_new_usage, or select both and return hourlyUsd: null when the row is not usable, so the notice falls back to "Price unavailable" instead of a wrong or unusable rate.
| {data.containerCostMusd !== null && ( | ||
| <div> | ||
| <dt className="text-muted-foreground">Container cost</dt> | ||
| <dd>{formatSessionCost(data.containerCostMusd / 1_000_000)}</dd> |
There was a problem hiding this comment.
WARNING: A real sub-cent container charge renders as $0.00.
formatSessionCost rounds to whole cents (session-cost-breakdown.ts:25-27), while this row renders for any non-null sum. At the code-review container SKU's cents-per-second rate, a review shorter than about 100 seconds is a positive charge below $0.005, so this line reads "Container cost $0.00" and implies nothing was charged. The model-cost row directly above avoids exactly that with isRenderableSessionCost, and SessionInfoDialog renders the same kind of total with formatSessionTotalCost, which produces <$0.01.
Smallest fix: use formatSessionTotalCost(data.containerCostMusd / 1_000_000) (keeping the non-null gate) so a positive charge is never displayed as $0.00.
| attempts: visibleAttempts, | ||
| tokenUsage, | ||
| sessionCostMusd: billingUsage?.totalCostMusd ?? null, | ||
| containerCostMusd, |
There was a problem hiding this comment.
SUGGESTION: The get wiring for the new field is untested.
readCodeReviewContainerCost itself is unit-tested, but nothing exercises the contract this page depends on: that codeReviews.get passes review.session_id plus every attempt session, picks review.owned_by_organization_id for an org review and the caller for a personal one, and returns the aggregate as containerCostMusd. A wrong owner type or a dropped attempt session would silently show no cost. apps/web/src/routers/code-reviews-router.test.ts already has DB-backed get cost coverage ("returns the billing-derived session cost for a terminal review") to extend.
Smallest addition: seed container_usage_interval + container_usage_segment + compute_usage_charge rows for the review and one attempt session plus a row for another payer, then call get as the owner and as an org member and assert both the amount and that foreign payer rows are excluded; a review with only shadow (no-charge) usage should still return containerCostMusd: null.
eshurakov
changed the title
Summary
The billing service owns the enrollment decision. The new read-only API uses the same decision as admission, including enterprise exemptions.
Verification
Automated checks
git diff --checkpassed.Visual Changes
Screenshots use synthetic local data and a synthetic catalog rate, not production pricing. They show the running application, not mocked browser responses.
Reviewer Notes
Alertprimitive; no dedicated alert recipe exists.