fix(token-stats): resolve test failures and lint errors#1485
fix(token-stats): resolve test failures and lint errors#1485UncertaintyDeterminesYou4ndMe wants to merge 11 commits intoMoonshotAI:mainfrom
Conversation
…eekly ledger - Add TokenStats dataclass to kimi-agent-sdk Session for per-session accumulation - Add TokenLedger for persistent daily/weekly token stats (~/.kimi/token-stats.json) - Redesign status bar: Claude Code-style context bar with █/░, token counts, elapsed time - Show context window usage as percentage + absolute tokens (e.g. 27% (45k/128k)) - Show cumulative session token stats (in/out) in status bar - Show daily and weekly token totals below main status line - Add tests for TokenLedger (boundary resets, persistence, resilience) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove unused pytest import from test_token_ledger.py - Fix _session_start_time AttributeError when tests use object.__new__ - Update test_prompt_tips.py assertions to match new status bar design - Restore deleted web/package-lock.json
…sage-stats fix(token-stats): resolve test failures and lint errors
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19fecd767
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| tmp = self._file.with_suffix(".tmp") | ||
| tmp.write_text(json.dumps(data)) | ||
| tmp.rename(self._file) | ||
| except OSError: |
There was a problem hiding this comment.
Switching to tmp.rename(self._file) makes persistence fail on Windows once the destination file already exists, because Path.rename there does not overwrite existing paths. Since the OSError is swallowed, record() silently stops updating token-stats.json after the first write, so daily/weekly totals become stale across sessions. Use Path.replace/os.replace for cross-platform atomic overwrite behavior.
Useful? React with 👍 / 👎.
| current_toast_left = _current_toast("left") | ||
| if current_toast_left is not None: | ||
| tip_text: str | None = current_toast_left.message[:available] |
There was a problem hiding this comment.
The redesigned toolbar now only reads _current_toast("left") and never renders the right queue, so notifications queued with position="right" are effectively invisible. This regresses background MCP feedback (e.g. OAuth/connect toasts emitted from toolset._toast_mcp), leaving users without status/error messages that previously appeared in the right span.
Useful? React with 👍 / 👎.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
UncertaintyDeterminesYou4ndMe
force-pushed
the
feat/token-usage-stats
branch
from
76f8fe7 to
5cf3112
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cf3112b74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/token_ledger.py
Outdated
| # ── public interface ────────────────────────────────────────────────── | ||
|
|
||
| def record(self, usage: TokenUsage) -> None: | ||
| """Add *usage* to both daily and weekly buckets, then persist.""" |
There was a problem hiding this comment.
record() always appends into the day/week keys captured at construction time, so a long-running CLI session that crosses midnight (or a Monday boundary) keeps charging usage to the previous period until the process restarts. That makes the displayed daily/weekly stats incorrect despite the module contract saying boundaries reset automatically.
Useful? React with 👍 / 👎.
src/kimi_cli/token_ledger.py
Outdated
| except (json.JSONDecodeError, OSError): | ||
| return | ||
|
|
||
| if data.get("daily", {}).get("date") == self._today_str: |
There was a problem hiding this comment.
.get access
_load() assumes daily and weekly are mapping objects and immediately calls nested .get(...); if token-stats.json is valid JSON but has an unexpected shape (for example from manual edits or legacy data), this raises AttributeError during startup and prevents the agent from initializing. Since this file is treated as best-effort, malformed-but-parseable shapes should be ignored instead of crashing.
Useful? React with 👍 / 👎.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Remove unused pytest import from test_token_ledger.py - Fix _session_start_time AttributeError when tests use object.__new__ - Update test_prompt_tips.py assertions to match new status bar design - Restore deleted web/package-lock.json - Merge upstream/main and resolve import conflicts Additional fixes from review: - Fix Windows file persistence: use tmp.replace() instead of tmp.rename() - Fix swapped color thresholds in _pct_color() (50-69% yellow, 70-89% orange) - Fix missing right-position toast rendering in status bar Further fixes: - Fix TokenLedger long-running session boundary detection (recompute on record/read) - Fix type validation in _load() to handle malformed JSON gracefully - Fix StatusSnapshot to always include daily/weekly stats (not None when zero)
UncertaintyDeterminesYou4ndMe
force-pushed
the
feat/token-usage-stats
branch
from
5cf3112 to
e2a6320
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2a63202c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/token_ledger.py
Outdated
| self._maybe_reset_boundaries() | ||
| self._daily.add(usage) | ||
| self._weekly.add(usage) | ||
| self._save() |
There was a problem hiding this comment.
record() updates only this process's in-memory _daily/_weekly values and then writes them back, but _maybe_reset_boundaries() only checks date keys and never reloads current totals from disk. If two kimi sessions run concurrently, the later writer overwrites the earlier writer's increments (last-write-wins), so “all sessions” daily/weekly totals become undercounted whenever sessions overlap.
Useful? React with 👍 / 👎.
src/kimi_cli/token_ledger.py
Outdated
| input_other=d.get("input_other", 0), | ||
| output=d.get("output", 0), | ||
| input_cache_read=d.get("input_cache_read", 0), | ||
| input_cache_creation=d.get("input_cache_creation", 0), |
There was a problem hiding this comment.
from_dict() copies JSON values without numeric validation, so valid-but-untyped data like "input_other": "100" gets stored as a string in _PeriodStats; the next record() then hits TypeError when add() does self.input_other += usage.input_other. Because this file is user-editable/best-effort state, non-int numeric fields should be coerced or rejected during load instead of crashing usage recording.
Useful? React with 👍 / 👎.
| if status.daily_token_usage is not None: | ||
| fragments.append(("", "\n")) | ||
| fragments.extend(_ledger_line("today", status.daily_token_usage)) | ||
| if status.weekly_token_usage is not None: | ||
| fragments.append(("", "\n")) | ||
| fragments.extend(_ledger_line("week", status.weekly_token_usage)) |
There was a problem hiding this comment.
🟡 Daily/weekly token stats always shown in toolbar (even when zero) due to is not None check on always-non-None value
The TokenLedger.daily and TokenLedger.weekly properties always return a TokenUsage object (never None), since _PeriodStats.to_token_usage() at src/kimi_cli/token_ledger.py:33-39 always constructs a valid TokenUsage. In KimiSoul.status (src/kimi_cli/soul/kimisoul.py:333-334), these are assigned directly to daily_token_usage and weekly_token_usage. The UI check if status.daily_token_usage is not None is therefore always True, causing two extra toolbar rows displaying all-zero values (today 0 in:0 out:0 / week 0 in:0 out:0) even before any tokens have been used. This is inconsistent with the total_token_usage handling at line 1684 which correctly checks is not None and .total > 0.
Was this helpful? React with 👍 or 👎 to provide feedback.
- Add robust type coercion in from_dict() to handle strings, floats, invalid values - Fix concurrent session data loss by reloading from disk before record() - Extract _read_from_disk() helper for code reuse
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3498df472d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| weekly = None | ||
|
|
||
| # Safely extract daily stats with type checking | ||
| daily_data = data.get("daily") |
There was a problem hiding this comment.
_read_from_disk assumes json.loads returned a mapping and immediately calls data.get("daily"); if token-stats.json is valid JSON with a non-object root (for example [] after manual edits), this raises AttributeError in TokenLedger.__init__ and can block startup. Fresh evidence: the new type checks only validate daily/weekly after the unguarded data.get(...) call.
Useful? React with 👍 / 👎.
| if isinstance(v, int): | ||
| return v | ||
| if isinstance(v, float): | ||
| return int(v) |
There was a problem hiding this comment.
The float coercion path uses int(v) directly, so parseable values like 1e309 (decoded as inf) or NaN can raise OverflowError/ValueError while loading stats, and those exceptions are not suppressed by _read_from_disk; this can still crash ledger loading from a malformed-but-parseable file. Fresh evidence: coercion was added, but the float branch still lacks non-finite guards.
Useful? React with 👍 / 👎.
- Validate JSON root type is dict before accessing (handles [] or primitive roots) - Guard against non-finite numbers (inf, nan) in _to_int() coercion
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 453210016a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| def daily(self) -> TokenUsage: | ||
| """Total token usage for today (all sessions).""" | ||
| self._maybe_reset_boundaries() | ||
| return self._daily.to_token_usage() | ||
|
|
||
| @property | ||
| def weekly(self) -> TokenUsage: | ||
| """Total token usage for the current ISO week (all sessions).""" | ||
| self._maybe_reset_boundaries() | ||
| return self._weekly.to_token_usage() |
There was a problem hiding this comment.
These accessors are documented and surfaced as "all sessions" totals, but they only run _maybe_reset_boundaries() and then return this process's cached _daily/_weekly values. If another kimi process records usage after this instance starts, this instance keeps showing stale day/week numbers until it records locally or restarts, so the new toolbar can report incorrect cross-session usage. Reload current-period values from disk before returning daily/weekly (or invalidate cache on external changes).
Useful? React with 👍 / 👎.
Ensure cross-session usage is accurate by reloading persisted counters before returning daily/weekly totals. This fixes stale data when multiple kimi processes are running concurrently.
Resolve conflicts: - soul/__init__.py: keep both TokenUsage and MCPStatusSnapshot imports - soul/kimisoul.py: include both token usage and mcp_status in StatusSnapshot - ui/shell/prompt.py: keep both _session_start_time and new running prompt fields
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f4866ba3e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| daily_token_usage=self._ledger.daily, | ||
| weekly_token_usage=self._ledger.weekly, |
There was a problem hiding this comment.
This status snapshot now pulls daily_token_usage and weekly_token_usage directly from TokenLedger on every status access, and both properties do synchronous JSON file reads/parses. In the shell UI, status_provider is called during toolbar redraws (e.g., while typing), so this introduces repeated blocking filesystem I/O on the event loop and can noticeably degrade prompt responsiveness, especially on slow or network-backed home directories. Consider caching ledger totals in memory and refreshing them only after record() or on a throttled interval.
Useful? React with 👍 / 👎.
Add _MIN_RELOAD_INTERVAL (1.0s) to prevent blocking filesystem I/O on every status access. Properties (daily/weekly) use throttled reload, while record() forces immediate reload for correctness. Addresses Codex review: 'Avoid disk I/O in hot status path'
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 479e104659
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/token_ledger.py
Outdated
| raw_data = json.loads(self._file.read_text()) | ||
| except (json.JSONDecodeError, OSError): |
There was a problem hiding this comment.
_read_from_disk() is intended to be best-effort, but Path.read_text() can raise UnicodeDecodeError when token-stats.json contains non-UTF8 bytes (for example after file corruption or manual edits). Because that exception is not caught here, TokenLedger initialization can fail and block CLI startup instead of falling back to empty stats. Please treat decode errors the same way as JSON parse errors so damaged stats files do not crash the agent.
Useful? React with 👍 / 👎.
Add UnicodeDecodeError to exception handling in _read_from_disk() and _maybe_reset_boundaries(). Treats encoding errors same as parse errors - fall back to empty stats instead of crashing. Addresses Codex review: 'Handle non-UTF8 token stats files'
1 participant
Fix test failures and lint errors for the token usage stats feature.
Changes
Testing
Related
Part of the token usage stats feature.