fix(subagents): roll formatTokenShort over to "m" at 1000k#88209
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a rounding edge case in formatTokenShort where values just below 1,000,000 would render as the out-of-scheme "1000k" instead of advancing to the million branch.
Changes:
- Guard the thousands branch so rounded values that reach 1000 fall through to the million branch.
- Add regression tests covering the 999_499 / 999_500 / 999_999 boundary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/shared/subagents-format.ts | Adds fall-through logic when rounding hits 1000 in the thousands branch. |
| src/shared/subagents-format.test.ts | Adds boundary tests for the rounding rollover. |
openclaw-barnacle
Bot
added
the
proof: supplied
|
Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 6:56 PM ET / 22:56 UTC. Summary PR surface: Source +11, Tests +36. Total +47 across 4 files. Reproducibility: yes. from source inspection: current main formats rounded thousands before checking the million rollover, so boundary values like 999,500 can produce Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Land the focused rollover fix after ordinary maintainer checks, keeping both token display surfaces aligned with the existing compact unit scheme. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main formats rounded thousands before checking the million rollover, so boundary values like 999,500 can produce Is this the best way to solve the issue? Yes. The PR changes the narrow formatter branches that own this display behavior and adds boundary regression coverage instead of adding a second formatting path. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 290b19275be6. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +11, Tests +36. Total +47 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Thanks for the tight boundary fix. I found one remaining sibling surface that should be covered before merge.
[P2] Cover compact subagent announce stats too
The PR fixes formatTokenShort, so the subagent list no longer renders 999_999 as 1000k, but the compact subagent completion stats still use the private formatter in src/agents/subagent-announce-output.ts:506. That path is also subagent usage display text, and on this PR head (ec84ca5e850b301c56e7edc4735d128312a39df2) a direct runtime probe now gives:
formatTokenShort(999999) -> 1m
buildCompactAnnounceStatsLine(...) -> Stats: runtime n/a • tokens 1000.0k (in 1000.0k / out 0)
So merging this leaves the same out-of-scheme token unit in parent completion announcements. Please either reuse the shared subagent formatter there or add the same rollover guard plus a focused regression row for buildCompactAnnounceStatsLine.
formatTokenShort emitted an out-of-scheme "1000k" for token counts in [999_500, 999_999]: the n < 1_000_000 branch used Math.round(n / 1_000), which rounds up to 1000 near the boundary, instead of advancing to the million branch. The sibling formatTokenCount already guards this exact case. Surfaces in the subagent list usage display (formatTokenUsageDisplay -> subagent-list.ts), so a subagent that used ~999.5k-999.999k tokens showed "1000k" instead of "1m". Fall through to the "m" branch when the rounded thousands reach 1000. Add rollover-boundary regression rows. Co-Authored-By: Claude Opus 4.8 <[email protected]>
ec84ca5 to
a6a6c10
Compare
openclaw-barnacle
Bot
added
agents
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Maintainer fix pushed to head a6a6c10f96cda74c36c8a00582bb19d208c4d54d.
What changed:
- kept the original
formatTokenShortrollover fix - added the same unit rollover guard for compact subagent announcement stats
- added regression coverage for
buildCompactAnnounceStatsLineat the 999,999-token boundary
Verification:
- focused runtime probe:
formatTokenShort(999999)->1m; compact announce stats ->Stats: runtime n/a • tokens 1.0m (in 1.0m / out 0) oxfmt --checkon the touched formatter/test filesgit diff --check origin/main..HEAD- autoreview local closeout: clean, no accepted/actionable findings; parallel runtime probe passed
Local Vitest note: node scripts/run-vitest.mjs ... entered a non-exiting Vitest path on this machine, so I stopped it and used the focused runtime probe plus exact-head CI for final gate.
clawsweeper
Bot
added
rating: 🦞 diamond lobster
openclaw-barnacle
Bot
removed
the
proof: sufficient
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Final maintainer fix proof for head c053f4737e06b7dffcacd4257982231c229213af.
What changed since the original PR:
- compact subagent announcement stats now roll one-decimal thousands over to the million unit instead of rendering
1000.0k buildCompactAnnounceStatsLinehas a regression row for the 999,999-token boundary- test fixture now matches the typed session-entry shape
Verification:
- focused runtime probe:
formatTokenShort(999999)->1m; compact announce stats ->Stats: runtime n/a • tokens 1.0m (in 1.0m / out 0) oxfmt --checkon touched formatter/test filesgit diff --check origin/main..HEADnode scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test-pr88209.tsbuildinfo- autoreview local closeout: clean, no accepted/actionable findings
- exact-head CI: Real behavior proof, check-test-types, check-prod-types, check-guards, security-fast, preflight passed
Known unrelated current-main reds: check-lint, checks-node-agentic-gateway-methods, and checks-node-agentic-control-plane-agent-chat are also red on current origin/main and do not touch this formatter patch.
Summary
formatTokenShortinsrc/shared/subagents-format.tsemitted an out-of-scheme"1000k"for token counts in[999_500, 999_999]. Then < 1_000_000branch returned`${Math.round(n / 1_000)}k`, andMath.roundrounds those values up to1000, so the formatter produced"1000k"instead of rolling over to the million branch ("1m").The sibling formatter
formatTokenCount(src/utils/usage-format.ts) already guards this exact case (if (Number(formattedThousands) >= 1_000) return …m), so the correct behavior is established in core —formatTokenShortsimply omitted the rollover check.User impact:
formatTokenShortfeedsformatTokenUsageDisplay, rendered in the subagent list usage line (src/agents/subagent-list.ts:246). A subagent that consumed ~999.5k–999.999k tokens displayed"1000k"instead of"1m".Fix
Fall through to the
"m"branch when the rounded thousands reach1000, mirroringformatTokenCount. One small change plus rollover-boundary regression rows.Real behavior proof (required for external PRs)
Behavior addressed: token counts in the 999.5k–999.999k range rendered as
1000kin the subagent usage display because the thousands-rounding branch could reach 1000 without advancing to the million unit. After this patch they render as1m.Real environment tested: local macOS checkout on fresh main. The proof drives the real production
formatTokenShort(the exact function the subagent list calls throughformatTokenUsageDisplay) over its boundary inputs. No mocks of the function under test.Exact steps or command run after this patch: I added rollover-boundary rows to the formatter's table test, ran them against the unpatched
formatTokenShort(they emitted1000k), then applied the fix and re-ran. Command:Evidence after fix: copied live console output from
node scripts/run-vitest.mjs—Observed result after fix: the
node scripts/run-vitest.mjsrun showsformatTokenShort(999_500)andformatTokenShort(999_999)now return"1m"(previously"1000k"), whileformatTokenShort(999_499)is unchanged at"999k".What was not tested: I did not capture a live screenshot of the subagent list TUI; the proof drives the exact formatter that view renders, over the boundary inputs, via
node scripts/run-vitest.mjs.Tests and validation
node scripts/run-vitest.mjs src/shared/subagents-format.test.ts-> 5 passed (new rollover-boundary rows:999_499->999k,999_500/999_999->1m).1000k) on the unpatched source and pass with the fix.oxfmt --checkclean,oxlintclean,tsgo:coreexit 0,tsgo:core:testexit 0,git diff --checkclean. Diff is +11 / -1 across 2 files.