fix(code-reviews): billing error classification#1332
Open
alex-alecu wants to merge 8 commits intomainfrom
Open
fix(code-reviews): billing error classification#1332alex-alecu wants to merge 8 commits intomainfrom
alex-alecu wants to merge 8 commits intomainfrom
Conversation
…pers Add functions to create new top-level comments on GitHub PRs and GitLab MRs, plus deduplication helpers that check for an existing HTML marker before posting.
When a review fails due to a billing error, post a short comment on the PR/MR telling the user their account is out of credits with a link to app.kilo.ai. Skips posting if the notice already exists (deduplication via HTML marker).
Cover GitHub and GitLab billing notice posting, deduplication (skip if already posted), non-billing failures (no notice), and link presence.
Consistent with findKiloReviewNote — iterate pages so the billing notice deduplication check works on MRs with >100 notes.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by gpt-5.4-20260305 · 533,327 tokens |
The wrapper was discarding real billing error messages, passing the generic event type string instead. normalizePayload did not detect billing from error text, so v1 billing errors stayed as 'cancelled' and v2 ones lacked terminal_reason. Fix all three layers: wrapper passes the actual error, normalizePayload infers billing from message patterns, and a backfill migration corrects ~13k historical rows.
alex-alecu
changed the title
| if (isTerminalError(eventType, properties)) { | ||
| callbacks.onTerminalError(eventType); | ||
| const errorText = | ||
| typeof properties.error === 'string' |
Contributor
There was a problem hiding this comment.
WARNING: Structured session.error payloads still lose the real billing message
EventSessionError.properties.error is an object union (APIError, UnknownError, etc.), not just a string. isTerminalError() already stringifies object-shaped errors, so this branch still fires for billing session.error events, but the fallback here stores Insufficient credits: ${eventType} instead of the actual data.message. That recreates the same generic-message problem for the common non-string error shape.
- Paginate hasPRCommentWithMarker using octokit.paginate to match the GitLab counterpart and avoid duplicate billing notices on busy PRs - Preserve structured error objects in wrapper fallback via JSON.stringify instead of discarding them - Improve cross-reference comments between the duplicate terminal reason type definitions in worker-utils and db packages
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Billing errors from code reviews were being misclassified in two ways: v1 billing errors arrived as
interruptedand got normalized tocancelled(hiding them from billing analytics), and v2 billing errors had the real error message discarded by the wrapper, storing the generic stringsession.errorinstead. This PR adds aterminal_reasoncolumn to track failure categories, fixes both classification paths, posts billing notices on PRs/MRs, and backfills ~13k historical rows.Three layers of fixes:
cloud-agent-next): passes the real billing error text fromproperties.errorinstead of discarding it and forwarding just the event type string.terminalReasonis set, reclassifyingcancelledbilling errors asfailedwithterminalReason: 'billing'.prepareSession/initiateSession/sendMessageV2, tags them withterminalReason: 'billing', and prevents the sendMessage fallback from retrying billing failures.Also adds billing PR/MR comment notices, admin dashboard improvements for billing vs. non-billing failure separation, and three backfill migrations for historical data.
Verification
pnpm jest 'code-review-status.*route\.test'— 17 tests pass (4 new: interrupted billing reclassification, failed billing inference, non-billing interrupted preserved, explicit terminalReason preserved)pnpm typecheck— passes across all workspace packages including cloud-agent-next wrapperVisual Changes
N/A
Reviewer Notes
error_message = 'session.error'matching rather than ILIKE patterns to avoid false positives from branch names containing billing keywords.failedbilling rows (23,711 rows). Migration 0057 covers the remaining gaps: v1cancelledbilling rows (~12,461) and v2session.errorrows (742).!terminalReasonguard in normalizePayload ensures the billing inference only fires when no explicit reason was provided — orchestrator-path billing errors (which already sendterminalReason: 'billing') are not double-handled.