fix: report tool accounting — attribute tool-results to their tool

[ranxianglei]

Follow-up on the acp_status accounting defects reported in billion-context#386 (kernel side).

What was wrong

1. tool bucket near-empty: report.ts classified messages with toolName ?? "text". Wire adapters set toolName only on tool-CALLS, never on tool-RESULTS, so every tool result (the bulk of context) landed in the text bucket — the tool bucket showed ~0.6% of the real tool volume.
2. two incompatible tool predicates: report.ts keyed on toolName, recommend.ts had the semantically correct isToolMessage (contentType-based) — private.
3. pct() floor: Math.max(1, ...) forced every non-empty bucket to render ≥1%, so three buckets could sum >100%.
4. ranges mislabel: renderUncompressedRanges labeled a merged range with the first message's tool despite the comment claiming "dominant tool".
5. panel docstrings claimed "chars/4 est." while the numbers come from the core's countTokens (kernel default = CJK-aware defaultCountTokens).

Changes

• src/report.ts
  • collectVisible builds a toolCallId -> toolName map from tool-calls and resolves each tool-result back to its calling tool; unresolvable results fall into a generic tool bucket instead of text.
  • pct() returns plain Math.round((n/total)*100) — no floor.
  • merged ranges track per-tool token totals and render the true dominant tool.
• src/recommend.ts — isToolMessage is now exported; report.ts reuses it (single definition, no cycle).
• src/panel/panel.ts — docstrings corrected to "estimated, kernel countTokens scale".
• tests/report-tool-accounting.test.ts — 5 regression tests: tool-result volume lands in tool bucket (12.5K/99%); results resolve to calling tool's name (Top tools + per-tool drilldown); orphan results stay out of text; 0% for tiny buckets with sum ≤100; ranges labeled with dominant tool.

Verification

• typecheck ✓
• full suite 580/580 ✓ (575 existing + 5 new)
• build ✓

Downstream (billion-context side, separate PR after this ships)

bili's 3 buildStatusReport call sites pass estimateTokensFast (chars/4) while the nudge side uses the kernel default CJK-aware estimator — mixing scales 4x-underestimates CJK. They will be switched to defaultCountTokens + acp-kernel pin bump, tracked in billion-context#386.

[github-actions]

📦 Built Package Artifact

Branch: 2026-09-06_report-tool-accounting (c1634b1)

Option A — Install from npm PR tag (recommended)

In your adapter project:

npm install acp-kernel@pr-203

Each push to this PR publishes a new version under the pr-203 npm tag.

Option B — Download artifact

1. Download the artifact from the Actions run
2. Extract the tarball and install:

tar xzf acp-kernel-pr203.tgz
npm install ./package

---

This comment is automatically updated on each push.

[ranxianglei]

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Review of PR #203 — approved with conditions

I independently re-ran the full verification and audited the diff against the old code and the wire layer. Details below.

What I verified

All 5 reported defects are real, and the root cause is at the wire-codec level (not a symptom):

1. Confirmed in all three converters: tool-RESULT messages carry toolCallId but never toolName (src/wire/anthropic.ts:96-101, src/wire/openai.ts:137-143, src/wire/responses.ts:244-245, 277-278), while tool-CALLS set toolName. So toolName ?? "text" in the old collectVisible genuinely dumped all result volume into the text bucket. The fix targets the cause, not the display.
2. Old code had two predicates confirmed (report.ts keyed on toolName; recommend.ts:isToolMessage was private). No import cycle introduced: report.ts → recommend.ts, and recommend.ts imports only types.js + protected.js.
3. Math.max(1, …) floor removed; pct() now plain rounding.
4. renderUncompressedRanges now accumulates per-tool token totals per merged range and renders the true dominant tool — verified the old code labeled merged ranges with the first message's tool despite the "dominant" comment.
5. Panel docstrings corrected; verified defaultCountTokens is indeed CJK-aware (src/tokenize.ts:5-12: CJK counted 1:1, remainder chars/4), so the new wording matches reality.

Preflight (re-run by me, not taken on faith):

• npm run typecheck ✓
• npm test: 580/580 on the PR branch; baseline on origin/master = 575/575 → the "575 existing + 5 new" claim checks out exactly ✓
• npm run build ✓

The 5 new regression tests are meaningful (exact arithmetic via ASCII padding, no float fragility): volume lands in the tool bucket, results resolve to the calling tool's name in both Top-tools and drilldown, orphans stay out of text, tiny buckets render 0% with sum ≤100, merged ranges labeled with the dominant tool.

⚠️ Duplicate screening: overlap with PR #177

This PR overlaps heavily with open PR #177 (2026-08-31_status-report-accounting, refs billion-context#390): same tool-bucket bug, same isToolMessage unification, same pct-floor removal, same panel docstring fix. Tagged duplicate here because there are differences worth keeping separate:

	#203	#177
Range mislabel (defect #4)	✅ fixed	❌ not touched
Structure	exports isToolMessage from recommend.ts	new leaf module src/message-kind.ts
compress.ts breakdown dedup	no (inline copy remains)	✅ unified onto shared predicate
Public export / isTool flag	no	✅ both
State vs master	mergeable, green	conflicts (stale base)
Agent reviews so far	this one	one LGTM already on file

Recommendation (single path, no fork): ship via #203, then close #177 as superseded and open a small follow-up that ports #177's structural wins (leaf module, compress.ts dedup, isTool flag replacing the "text" sentinels at src/report.ts:161/164, plus its cross-surface agreement + CJK-scale tests). Rationale: #203 covers a strict superset of user-visible defects (it fixes the range labeling #177 misses), is already green and mergeable against current master, while #177's remaining value is non-behavioral refactoring + extra tests — ideal for a low-risk follow-up after the fix lands.

⚠️ Process flag before merge

The PR branch contains the release v0.0.53 commits (0dc35b2 version bump + its merge commit), which are not on origin/master — and PR #202 (release v0.0.53) shows as closed without merge even though tag v0.0.53 exists. Merging #203 as-is would land the version bump into master through this fix PR, breaking the release-commit convention. Please:

1. Rebase/recreate the branch onto current master so only the fix commit rides the PR, and
2. Clarify the state of release v0.0.53 #202 / the v0.0.53 publish (was it closed by mistake?).

Nits (non-blocking)

1. renderOverview still buckets totals via the string sentinels (m.tool !== "text" / === "text") — correct under the new classification, but magic-string coupled; would miscount a tool literally named text. The isTool flag from the planned follow-up resolves this.
2. Exporting isToolMessage from recommend.ts couples report→recommend; the leaf-module layout is cleaner (again, covered by the follow-up).
3. Plain Math.round can still sum >100% in pathological cases (e.g. 33.5%/33.5%/33% → 34+34+33=101). The floor defect is fixed; this is cosmetic and vanishingly rare — not worth blocking on.

Verdict

Approve with conditions: (a) rebase off the release commits, (b) one more independent agent review per AGENTS.md §6 (this counts as one of the required two). After that it's ready for human merge; then close #177 as superseded and file the structural follow-up.

[ranxianglei]

Re: "⚠️ Process flag before merge" — verified stale, no action needed

Re-checked against live state just now:

$ git fetch origin && git log --oneline origin/master -3
f14c1c8 Merge pull request #202 from ranxianglei/2026-09-05_release-v0.0.53
0dc35b2 release v0.0.53
168f59b Merge pull request #201 from ranxianglei/2026-09-05_persist-enoent-retry

$ git log --oneline origin/master..2026-09-06_report-tool-accounting
c1634b1 fix: report tool accounting — attribute tool-results to their tool (#386 follow-up)   ← the ONLY commit on the branch

$ git log --oneline 2026-09-06_report-tool-accounting..origin/master
(empty)

• PR#202 API: "state": "closed", "merged": true, "merged_at": "2026-09-05T14:48:41Z" — closed+merged is GitHub's normal representation of a merged PR; it was merged, not closed unmerged.
• f14c1c8 (the release v0.0.53 #202 merge) is on origin/master.
• npm view acp-kernel version → 0.0.53 live.

So the branch is exactly origin/master + 1 fix commit; the release commits arrive via master's own history, not via this PR. No rebase needed.

Re: overlap with #177

Agreed with the single-path recommendation: ship #203, close #177 as superseded, port its structural wins (leaf module message-kind.ts, compress.ts predicate dedup, isTool flag replacing the "text" sentinels, cross-surface + CJK-scale tests) in a small follow-up. The sentinels/coupling nits noted here are covered by that follow-up.

[ranxianglei]

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Correction: process flag retracted — you're right, no rebase needed.

I re-verified against live GitHub API state (not my local clone, which was serving stale refs):

• branches/master → f14c1c8 (the release v0.0.53 #202 merge commit) ✓
• PR release v0.0.53 #202 → state: closed, merged: true, merged_at: 2026-09-05T14:48:41Z, merge_commit_sha: f14c1c8 ✓
• PR fix: report tool accounting — attribute tool-results to their tool #203 → base f14c1c8, head c1634b1, mergeable, behind_by: 0 — branch is exactly master + the single fix commit ✓
• npm view acp-kernel version → 0.0.53 ✓

So condition (a) from my review verdict is withdrawn. Two things went wrong on my side, for the record: (1) my local clone's origin/master ref was stale and a re-fetch didn't advance it, which anchored the whole flag on bad data; and (2) my first API check had already returned merged_at: 2026-09-05T14:48:41Z for #202 and I misread that output as closed-unmerged. Both were my errors — the flag should never have been raised. Sorry for the noise.

Remaining condition before human merge: exactly one — a second independent agent review per AGENTS.md §6 (my review above counts as one of the required two; the thread currently has no other review). Everything else from the audit stands: all five defects verified real at root cause, preflight green (typecheck / 580-580 vs 575 baseline / build), nits tracked for the follow-up.

#177 path: agreed as stated — once #203 merges, close #177 as superseded and open the small follow-up carrying its structural wins (message-kind.ts leaf module, compress.ts predicate dedup, isTool flag replacing the "text" sentinels at src/report.ts:161/164, plus the cross-surface agreement and CJK-scale tests). Happy to draft that follow-up issue/branch when you give the word.

m00026