Skip to content

[5] Check PR costs against independent receipts - #1350

Open
tautvydasLiekis wants to merge 28 commits into
feat/record-tool-edit-evidencefrom
feat/verify-pr-cost-accuracy
Open

tautvydasLiekis wants to merge 28 commits into
feat/record-tool-edit-evidencefrom
feat/verify-pr-cost-accuracy

Conversation

@tautvydasLiekis

@tautvydasLiekis tautvydasLiekis commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Adds an offline command that checks a saved PR cost report against separate billing receipts and human ownership labels. It answers "are these PR totals right?" with an exit code and a list of differences, so the attribution in #1347–#1349 can be measured instead of trusted.

Linked issue

Depends on #1349. No public issue is linked.

What does this PR do?

pnpm exec tsx src/extensions/work-attribution/accuracy-cli.ts report.json reference.json

report.json is a saved costs.json. reference.json lists each request's receipt amount, billing account and the PR a person says owns it. The command reads both files, prints the differences and exits. It makes no network or model calls and writes nothing.

Exit Meaning
0 Ownership, prices, totals and the confirmed/inferred portions all match the reference.
1 Wrong arguments, or an input file that cannot be read or has an unsupported shape.
2 Empty, incomplete or malformed evidence: missing or unlabelled requests, repeated labels, or missing account, price or ownership evidence.
3 Complete, but some assignments or amounts differ; the differences are listed.

What it checks:

  • Coverage in dollars and in request count. A report that captured USD 1 of a USD 10 task does not pass, even if that dollar was assigned correctly.
  • Wrong assignments do not cancel out: overcharging one PR cannot hide an undercharge on another.
  • Missing evidence is reported separately from a verified zero charge.
  • Inferred spend can match in dollars and still return exit 3 when the label expects a confirmed assignment.

So that one file can be scored, each work's costs.json now also lists all six unallocated buckets and the requests and PR totals of every work connected to it through a shared PR or a work link. /work still counts only the work's own requests in its price, untagged and failed-refresh lines. The guide covers the reference format, shared work, unknown labels and metrics.

Evidence

12 October retest with real GLM models and real bills on a binary of this head (ca92df405): 18 of 18 requests priced and assigned to the PR, the six unallocated buckets written to costs.json, and the total equal to the bills. CI passes on 6bd52ec2a.

On 69cee6180, after the final review fixes:

  • CI: build, pr-checks, master-checks, TUI e2e in 4 shards, ACP e2e and MCP e2e pass on this head. A duplicate tui-e2e entry from a superseded workflow run shows as cancelled.
  • The stack top 95614e7c6 (#1389) contains this PR. On it, pnpm check passes and the full unit suite passes except 5 tests that need tools this machine lacks: a DAP js-debug adapter, a built validate-skill.mjs and a binary rebuilt for the smoke test.
  • New test: the inferred bucket is checked against the independent receipts.
  • Lab on a binary built from 95614e7c6 (scripted model; local GitHub, billing and report fixtures): 73/73 default scenarios pass. Its happy-path scenarios run this command on their saved reports and require exit 0.
  • On an earlier head, the CLI on a one-request fixture exited 0 for a matching report, and 3 with two price-mismatch lines after the request's price was changed.

All inputs are synthetic. This PR provides the measuring tool; it does not report an accuracy rate over real PRs.

Limits

  • The command does not fetch PR state itself; empty groups use the report's recorded merge state.
  • Older label-only reference files still work but cannot check accounts, prices, inferred assignments or PR totals.
  • Connected works repeat the same rows in each per-work costs.json, so its size grows with the connected group. [11] Store work summaries as row logs #1389 limits how often the full file is rewritten.
  • Real receipts and labels must be collected separately and kept outside the repository.

Checklist

  • I have read CONTRIBUTING.md and agree to the CLA
  • This PR links to an open issue above
  • Tests pass locally (pnpm run test)
  • Lint passes (pnpm run check)
  • Documentation updated if behavior changed

@tautvydasLiekis tautvydasLiekis added the new feature Introduces a new feature label Oct 5, 2026
@kimchi-review

kimchi-review Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Kimchi Code Review

Property Value
Commit 4349d37
Author @tautvydasLiekis
Files changed 7
Review status Completed
Comments 0
Duration 29s

Summary

🚫 Merge request too large to review

📏 Size: 7 file(s) changed, +2659 / −0 lines

🧠 Cognitive load: 4/5 (1 = trivial, 5 = extremely demanding)

One cohesive feature but very large: two comparison engines (label-only vs independent reference) with overlapping metrics, dense financial rules (nanos integer math, bucket taxonomies, account scoping), plus ~1.6k lines of edge-case-pinned tests that must be verified line by line.

✂️ Suggested split

  1. Add attribution accuracy metric engine — Core correct/wrong/missed bucketing and percentage metrics for cost reports.
    • src/extensions/work-attribution/accuracy.ts
    • src/extensions/work-attribution/accuracy.test.ts
  2. Add independent reference verification and CLI — Cross-checks reports against external receipts and exposes the accuracy-cli entry point.
    • src/extensions/work-attribution/accuracy-reference.ts
    • src/extensions/work-attribution/accuracy-cli.ts
  3. Cover the CLI end to end and document the workflow — Reference and CLI scenario suites plus the offline verification guide.
    • src/extensions/work-attribution/accuracy-reference.test.ts
    • src/extensions/work-attribution/accuracy-cli.test.ts
    • docs/work-attribution-accuracy.md

👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them.

⚠️ Full review skipped — this merge request exceeds the reviewable size limit. See the split proposal above.

What to expect

Kimchi will analyze the changes in this pull request and post:

  • A summary of the overall changes
  • Inline comments on specific lines with findings categorized by issue type

The review typically completes within a few minutes. This comment will be updated once the review is ready.

Interact with Kimchi
  • @getkimchi review — re-trigger a full review on the latest commit
  • @getkimchi summary — regenerate the PR summary
  • @getkimchi ignore — skip this PR (no review will be posted)
  • Reply to any inline comment to ask follow-up questions or request clarification
Configuration

Reviews are configured by your organization admin.
Review instructions, excluded directories, and severity thresholds can be adjusted per repository in the Kimchi dashboard.


Powered by Kimchi — AI-powered code review by CAST AI

@kimchi-review kimchi-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚫 Merge request too large to review

📏 Size: 7 file(s) changed, +2659 / −0 lines

🧠 Cognitive load: 4/5 (1 = trivial, 5 = extremely demanding)

One cohesive feature but very large: two comparison engines (label-only vs independent reference) with overlapping metrics, dense financial rules (nanos integer math, bucket taxonomies, account scoping), plus ~1.6k lines of edge-case-pinned tests that must be verified line by line.

✂️ Suggested split

  1. Add attribution accuracy metric engine — Core correct/wrong/missed bucketing and percentage metrics for cost reports.
    • src/extensions/work-attribution/accuracy.ts
    • src/extensions/work-attribution/accuracy.test.ts
  2. Add independent reference verification and CLI — Cross-checks reports against external receipts and exposes the accuracy-cli entry point.
    • src/extensions/work-attribution/accuracy-reference.ts
    • src/extensions/work-attribution/accuracy-cli.ts
  3. Cover the CLI end to end and document the workflow — Reference and CLI scenario suites plus the offline verification guide.
    • src/extensions/work-attribution/accuracy-reference.test.ts
    • src/extensions/work-attribution/accuracy-cli.test.ts
    • docs/work-attribution-accuracy.md

👉 Please split this merge request into the smaller merge requests suggested above — the AI review will run automatically on each of them.

⚠️ Full review skipped — this merge request exceeds the reviewable size limit. See the split proposal above.

@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch from f35a09c to 5c7ff23 Compare October 6, 2026 10:06
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch from 4349d37 to a76bbb2 Compare October 6, 2026 10:39
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch from 5c7ff23 to c3fb131 Compare October 6, 2026 11:32
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch 2 times, most recently from 711e09e to 13cb56f Compare October 6, 2026 12:01
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch 2 times, most recently from 65d1089 to 4ddd773 Compare October 6, 2026 16:48
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch 3 times, most recently from fbb4685 to 8a8d570 Compare October 6, 2026 18:28
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch 2 times, most recently from 99886a6 to 5c55112 Compare October 6, 2026 20:47
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch 2 times, most recently from 07e8f4d to eb6f010 Compare October 7, 2026 08:30
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch from 5c55112 to d0a25d4 Compare October 7, 2026 08:30
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch from eb6f010 to 698a241 Compare October 7, 2026 15:40
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch 2 times, most recently from 4d5285c to 324aa28 Compare October 7, 2026 17:51
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch from 698a241 to e715a29 Compare October 7, 2026 17:51
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch from 324aa28 to f2ed5f7 Compare October 7, 2026 20:53
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch 2 times, most recently from a3b50a2 to d9d7a5a Compare October 7, 2026 21:20
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch 2 times, most recently from a8698f1 to a1a89a1 Compare October 8, 2026 04:24
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch 2 times, most recently from 8663368 to 1221090 Compare October 8, 2026 08:51
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/record-tool-edit-evidence branch from 7c4b96a to da9d349 Compare October 10, 2026 09:09
@tautvydasLiekis
tautvydasLiekis force-pushed the feat/verify-pr-cost-accuracy branch from ec3fd92 to 69cee61 Compare October 10, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature Introduces a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants