Repository navigation
[8] Report PR costs as work progresses - #1352
tautvydasLiekis wants to merge 67 commits into
Conversation
Kimchi Code Review
Summary🚫 Merge request too large to review 📏 Size: 21 file(s) changed, +2311 / −9 lines 🧠 Cognitive load: 5/5 (1 = trivial, 5 = extremely demanding) An entire new subsystem (snapshot schema, durable queue, HTTP worker, extension wiring) lands at once, interleaved with provider-ID and cost-report refactors in existing modules. Subtle ACK/revision/durability semantics plus ~1000 lines of dense tests make the blast radius very hard to hold in mind. ✂️ Suggested split
👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
There was a problem hiding this comment.
🚫 Merge request too large to review
📏 Size: 21 file(s) changed, +2311 / −9 lines
🧠 Cognitive load: 5/5 (1 = trivial, 5 = extremely demanding)
An entire new subsystem (snapshot schema, durable queue, HTTP worker, extension wiring) lands at once, interleaved with provider-ID and cost-report refactors in existing modules. Subtle ACK/revision/durability semantics plus ~1000 lines of dense tests make the blast radius very hard to hold in mind.
✂️ Suggested split
- Capture stable provider IDs in pull request status — Stores target-repo PR and repository IDs and exposes repository identity lookup.
src/extensions/pull-request-status/pull-requests.tssrc/extensions/pull-request-status/pull-requests.test.ts
- Expose validated work cost report from source journals — Single validated inventory reused by local summaries and later reporting.
src/extensions/work-attribution/cost-sync.tssrc/extensions/work-attribution/cost-sync.test.tssrc/extensions/work-attribution/costs.test.ts
- Add PR cost snapshot schema and builder — Allowlisted wire format with strict validation; private local data never crosses.
src/extensions/pr-cost-reporting/snapshot.tssrc/extensions/pr-cost-reporting/snapshot.test.ts
- Add durable opt-in reporting queue with acknowledgements — Persistent consent, revisions, membership hashes and retry state under agent dir.
src/extensions/pr-cost-reporting/queue.tssrc/extensions/pr-cost-reporting/queue.test.ts
- Add reporting worker, extension wiring and docs — Bounded authenticated delivery, /pr-reporting command, CLI registration, docs.
src/extensions/pr-cost-reporting/worker.tssrc/extensions/pr-cost-reporting/worker.test.tssrc/extensions/pr-cost-reporting/index.tssrc/extensions/pr-cost-reporting/index.test.tssrc/cli.tsdocs/pr-cost-reporting.md
👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them.
3aec50c to
d28b459
Compare
6fd33f8 to
e06294e
Compare
d28b459 to
c2a41b9
Compare
e06294e to
9ed94c7
Compare
c2a41b9 to
6969473
Compare
f582727 to
3200418
Compare
6969473 to
dc4fe8b
Compare
3200418 to
0f37820
Compare
dc4fe8b to
c07737f
Compare
0f37820 to
5af1491
Compare
c07737f to
3b72231
Compare
5af1491 to
0d23713
Compare
3b72231 to
5f61465
Compare
aa5d147 to
ec7ef7b
Compare
ddd1ee6 to
fc128db
Compare
ec7ef7b to
d6990b3
Compare
fc128db to
00cb976
Compare
d6990b3 to
0b97b26
Compare
8eae8f4 to
da5d1ce
Compare
de9c8d4 to
9282247
Compare
…pr-costs-integrated
…pr-costs-integrated
…pr-costs-integrated
…pr-costs-integrated
Kimchi now uploads PR cost reports to the user's own account while work is in progress, so the backend can show what each pull request cost from its own bills. Until now, PR costs stayed on the developer's machine. Reporting follows the existing SaaS telemetry setting, shows a one-time notice, and can be turned off with
/pr-reporting off.Depends on #1351 and the reporting API [7]. The dashboard is a separate change, [9] !948. Merge and deployment are separate steps.
What users get
/pr-reporting on|off|status. The choice survives restart.offcancels delivery and deletes pending uploads; local history and reports the server already accepted remain.statusshows the setting, pending reports, repositories waiting for their upload window, server limits and delivery problems. Typing/pr-reportingsuggests the three arguments.statusreads as a small table (PR reporting Enabled (SaaS default),Repositories 1 queued · 0 acknowledged). Delivery problems and server limits come first and make it a warning; history Kimchi could not report, such as requests without their original account or repository, is listed after the table as information.KIMCHI_PR_COST_REPORTING=0turns uploads off for a process. They keep recording locally, so a later interactive session reports the same work./resourcesalso stops uploads.How it works
After each main-agent turn, at startup and every 30 seconds, Kimchi rebuilds one snapshot per account and repository from its local journals and uploads it at most every five minutes. Journals are read in the background. While they and the local queue are unchanged, a pass skips the rebuild; it rebuilds after an hour anyway so the upload window still moves. A PR opening, merging, closing or reopening, a
/work linkorunlink, a withdrawal and/pr-reporting ongo out at once.Each upload replaces the previous one for that repository under a higher revision. Revision 1 may have requests but no PR; revision 2 adds the discovered PR; revision 3 adds billing IDs. Replacing a report never counts a charge twice, and the server reads prices from its own bills.
flowchart TD A[Recorded request] --> B{Original account and repository known?} B -- No --> Local[Keep locally] B -- Yes --> C{PR ownership evidence} C -- No PR yet --> Unknown[Report as unlinked] C -- One PR --> E{Why?} E -- Its edits landed, or explicit choice --> Confirmed[Confirmed] E -- Same work or model guess --> Inferred[Inferred] C -- Several PRs --> Shared[Shared] Unknown --> Send[Upload when enabled and the account matches] Confirmed --> Send Inferred --> Send Shared --> SendWhat is sent: repository and PR metadata, request and billing IDs, timestamps, coverage, the matching method and correction receipts. What stays local: prompts, plans, paths, work and session IDs, commit hashes and credentials. Health metrics use the existing telemetry setting and carry no user, request, repository or PR IDs.
Which work is sent: open PRs and their work, work from the last 32 days, and finished PRs for 32 days plus two days of clock grace. Older finished PRs are named in
windowedPullRequestIds, so the server keeps their claims. The server freezes a PR's total 32 days after it finishes and keeps frozen totals indefinitely; explicit corrections can still change one until day 90.Limits and failures:
PR_COST_LIMIT) waits about six hours. A snapshot limit also teaches a smaller cap that fits after one rejection; a producer limit holds only that repository; a contributor or organization limit pauses the account, while withdrawals still go out.Retry-Afteror back off.Queue state is in
~/.config/kimchi/harness/pr-cost-reporting/state.json. The reporting docs describe the payload, limits and health metrics.Evidence
Review fix on
d97fe9c55(merged up as55671c371): argument completions, and the skipped-history count saved as its own state field instead of inside the error text, sostatusno longer shows it as a warning. 14 new or changed tests fail without the change;pnpm check, 252 reporting tests and the TUI and ACPpr-cost-reportinge2e tests pass.12 October retest of this head's binary (
55671c371) with real GLM-5.3 and GLM-5.3 Flash sessions and real bills, uploading to the backend from !55779 (16d4018ad, running locally): all 3 uploads were accepted, and the backend's row for PR #927 equals Kimchi's split to the nano-dollar ($0.037085120 confirmed, $0 inferred, 1 contributor).On
da5d1ce61, after the final review fixes, with fixtures for accounts, replies and bills; no paid inference or shared-service upload:pr-checks,master-checks, TUI e2e in 4 shards, ACP e2e and MCP e2e pass on this head. A duplicatetui-e2eentry from a superseded workflow run shows as cancelled.validate-skill.mjsand a binary rebuilt for the smoke test.95614e7c6(#1389), which contains this PR: 73/73 default scenarios pass, including default-on reporting, startup backfill, replacement before a PR exists and opt-out./work linkcorrection and bills that arrived late). All 7 uploads were accepted. Each request's assignment and price, the backend's totals and the console's figures matched the expected result.The notice and
/pr-reporting statusfrom the 12 October run of this head:Risks and remaining work
/pr-reporting offare the opt-out.