feat: add write_state_file functionality for external integrations#184
feat: add write_state_file functionality for external integrations#184mossglenn wants to merge 7 commits intoMaciek-roboblog:mainfrom
Conversation
…tate_file
Fixed two critical bugs that caused monitor to crash on startup:
1. burn_rate type error: `calculate_hourly_burn_rate()` returns a float
(tokens per minute), not a dict. Changed from `burn_rate.get("tokens_per_minute")`
to directly using the float value.
2. logger not defined: Added `logger = logging.getLogger(__name__)` inside
the except block following the codebase's local logger pattern.
These bugs prevented the monitor from displaying data when --write-state
flag was enabled.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…tions Added complete documentation for the --write-state feature to support external tool development (VS Code extensions, status bars, dashboards). New Documentation: - STATE_FILE.md: Complete JSON schema, field descriptions, and type definitions * TypeScript and Python type definitions * File watching examples in multiple languages * VS Code extension integration example * Best practices for consuming the state file * Update frequency and staleness detection * Troubleshooting guide README.md Updates: - Added --write-state flag to CLI parameters table - Added "External Tool Integration" usage example section - Added state file to saved parameters list - Added STATE_FILE.md to Additional Documentation section - Included state file location and quick overview This documentation enables developers to build external tools that consume real-time Claude usage data from ~/.claude-monitor/reports/current.json. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…isplay The _write_state_file method was incorrectly calculating reset time based on reset_hour instead of using the actual session end time from the API. This caused current.json to show incorrect reset data that didn't match the terminal display. Changes: - Modified _process_active_session_data to include reset_time in the returned processed_data dictionary - Updated _write_state_file to use the pre-calculated reset_time from processed_data instead of manually calculating it - Added proper timezone conversion and time formatting to match terminal display logic - Added null-check error handling for robustness The reset time now correctly reflects the session's endTime from the API (when the current 5-hour session expires), not a daily reset hour. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add 80 test cases achieving high coverage for the write_state_file functionality. Tests cover basic functionality, data accuracy, reset time calculation, edge cases, error handling, and integration scenarios. Test files: - src/tests/test_write_state_file.py (new, 55 tests) - src/tests/test_display_controller.py (updated, 14 new tests) - src/tests/test_settings.py (updated, 11 new tests) Key test categories: - Basic Functionality: File creation, JSON validation, field presence - Data Accuracy: Percentages, burn rate type, timestamp formats - Reset Time Calculation: Regression test for commit 06e880f - Edge Cases: Zero limits, percentages >100%, various plan types - Error Handling: Missing directories, permissions, malformed data - Integration: write_state_enabled flag, CLI flag, Settings field All tests pass and meet contributor guidelines for core functionality coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a CLI flag --write-state and wiring to write a structured JSON state file (current.json) to a reports directory; includes environment variable for reports dir, settings persistence, state-serialization logic, documentation (STATE_FILE.md), and comprehensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (args)
participant Settings as Settings/Namespace
participant Display as DisplayController
participant FS as FileSystem (reports dir)
CLI->>Settings: parse args (--write-state)
Settings-->>Display: args.write_state -> write_state_enabled
Display->>Display: _process_active_session_data()
Display-->>FS: _write_state_file(processed_data) [if write_state_enabled]
FS-->>Display: write result / error
Display->>CLI: log success or warning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/claude_monitor/ui/display_controller.py`:
- Around line 392-393: The lastUpdate field is generated with
datetime.now().isoformat() and lacks timezone info, causing inconsistency with
reset.timestamp which uses reset_time.isoformat() (UTC); change the generation
of lastUpdate in the display_controller (the dict key 'lastUpdate') to use an
aware UTC timestamp (e.g., datetime.now(timezone.utc).isoformat() or convert to
UTC like reset_time) so both timestamps include the same timezone information
and format.
In `@src/tests/test_display_controller.py`:
- Around line 1256-1263: The test's side_effect function capture_write has the
wrong signature and will receive the Path instance as the first argument; update
capture_write to accept the Path receiver (e.g., self: Path) and the content (or
use *args, **kwargs) so it matches Path.write_text's signature; ensure this
patched function stores the actual text passed and then run
controller._write_state_file(sample_processed_data, sample_args) as before
(referencing capture_write, Path.write_text, and controller._write_state_file to
locate the change).
- Around line 1328-1335: The test's capture_write mock used for Path.write_text
has the wrong signature: the function capture_write defined for the patch of
Path.write_text is missing the first positional parameter that the real method
receives (self); update the capture_write used around
controller._write_state_file to accept (self, content: str) -> None so the
patched call matches Path.write_text, and apply the identical change to the
other capture_write defined at the later occurrence (the one mentioned around
Line 1358) so both mocks mirror Path.write_text's signature.
In `@STATE_FILE.md`:
- Line 328: Update the inaccurate documentation line about the default timezone:
change the stated default from "America/New_York" to "Europe/Warsaw" to match
the actual fallback used in the code (see formattedTime in display_controller.py
which uses "Europe/Warsaw" as the fallback timezone).
- Around line 397-398: The STATE_FILE.md entry for "Reset time" is incorrect:
the state file's reset_time is taken from processed_data (the value computed
from a session's end_time_str in display_controller.py at the code handling
processed_data->reset_time), not from the --reset-hour setting; update the
documentation to state that reset_time is sourced from processed_data (e.g.,
session end_time_str) and that --reset-hour does not change the state file's
reset_time (referenced by
test_reset_time_uses_processed_data_not_reset_hour_calculation and the
processed_data/reset_time field).
🧹 Nitpick comments (12)
src/claude_monitor/ui/display_controller.py (2)
319-322: Remove redundant imports inside the method.
jsonandosare already imported at the module level (lines 6, 8).timedeltais imported at line 9. OnlyPathLibalias forPathis unique but unnecessary sincePathis already imported at line 10.♻️ Suggested cleanup
def _write_state_file(self, processed_data: Dict[str, Any], args: Any) -> None: """Write current state to JSON file for external consumers. Args: processed_data: Processed session data from _process_active_session_data args: Command line arguments """ try: - import json - import os - from datetime import timedelta - from pathlib import Path as PathLib - # Extract values from processed_dataAnd update line 396:
- state_file = PathLib(os.environ["CLAUDE_MONITOR_REPORT_DIR"]) / 'current.json' + state_file = Path(os.environ["CLAUDE_MONITOR_REPORT_DIR"]) / 'current.json'
396-397: Consider defensive handling for missing environment variable.While
CLAUDE_MONITOR_REPORT_DIRis set viaos.environ.setdefaultinbootstrap.py, if someone calls this code path without going through the bootstrap (e.g., in tests or direct API usage), aKeyErrorwould be raised and caught by the generic exception handler, silently failing. Consider using.get()with a fallback.🛡️ Proposed defensive access
- state_file = PathLib(os.environ["CLAUDE_MONITOR_REPORT_DIR"]) / 'current.json' + report_dir = os.environ.get( + "CLAUDE_MONITOR_REPORT_DIR", + str(Path.home() / ".claude-monitor" / "reports") + ) + state_file = Path(report_dir) / 'current.json'STATE_FILE.md (2)
376-381: Add language specifier to fenced code block.The data flow diagram is missing a language specifier for the code block.
📝 Suggested fix
- ``` + ```text MonitoringOrchestrator (every --refresh-rate seconds)
387-389: Clarify atomic write behavior.
Path.write_text()is not truly atomic on all platforms. While it typically completes in a single operation, it doesn't use the rename-from-temp-file pattern that guarantees atomicity. The current behavior is generally safe but could leave a partial file on system crash. Consider updating the documentation to be more precise, or implementing a true atomic write if reliability is critical.📝 Suggested documentation clarification
-3. **Atomic writes**: File is written atomically using `Path.write_text()` - - No partial writes under normal conditions +3. **File writes**: File is written using `Path.write_text()` + - Typically completes without partial writes under normal conditionssrc/tests/test_write_state_file.py (8)
76-94: Test does not actually verify the intended behavior.This test creates a controller with
write_state_enabled = Falsebut never calls_write_state_file. The assertion that the file doesn't exist is trivially true since no write was attempted. The comment acknowledges this but the test provides no actual verification.Consider either:
- Removing this test since the behavior is tested elsewhere in
test_display_controller.py- Testing that calling
_write_state_filedirectly still works (it does) but the integration path increate_data_displaydoesn't call it♻️ Suggested improvement
def test_state_file_not_created_when_disabled( self, temp_report_dir: Path, sample_processed_data: Dict[str, Any], sample_args: Mock, ) -> None: - """Test state file is NOT created when write_state_enabled is False.""" - with patch("claude_monitor.ui.display_controller.NotificationManager"): - controller = DisplayController() - controller.write_state_enabled = False - - with patch.dict( - os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} - ): - # This should not create the file - # Note: The method won't even be called if write_state_enabled is False - # in the actual implementation, but we test the flag here - state_file = temp_report_dir / "current.json" - assert not state_file.exists() + """Test _write_state_file is not invoked via create_data_display when disabled.""" + # This behavior is already tested in test_display_controller.py + # TestWriteStateIntegration.test_write_state_file_not_called_when_disabled + pytest.skip("Covered by test_display_controller.py integration tests")
268-287: Test name and comment are misleading.
Path.write_text()is not atomic - it opens the file, writes content, and closes it. A crash mid-write could leave partial content. True atomic writes require writing to a temp file then renaming (which is atomic on POSIX).The test actually verifies "file is complete and parseable after write" rather than atomicity. Consider renaming for accuracy.
♻️ Suggested rename
- def test_state_file_written_atomically( + def test_state_file_written_completely( self, controller: DisplayController, temp_report_dir: Path, sample_processed_data: Dict[str, Any], sample_args: Mock, ) -> None: - """Test file is written atomically (using Path.write_text).""" + """Test file is written completely and is valid JSON.""" with patch.dict( os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} ): - # The implementation uses Path.write_text which is atomic controller._write_state_file(sample_processed_data, sample_args)
292-313: Test verifies the result but not the calculation logic.The test name implies
_write_state_filecalculates the percentage fromtokens_used/token_limit, but the implementation actually readsusage_percentagedirectly fromprocessed_data. The test passes becausesample_processed_datafixture setsusage_percentage=30.0, which matches the expected value.This is technically correct but the docstring "Test token percentage calculated correctly: (used/limit)*100" is misleading since that calculation happens upstream, not in
_write_state_file.
520-523: No-op string replacement.Line 522 has
timestamp.replace("+00:00", "+00:00")which replaces a string with itself, doing nothing. This appears to be a copy-paste artifact.♻️ Suggested fix
# Should be parseable as ISO 8601 timestamp = data["reset"]["timestamp"] - parsed = datetime.fromisoformat(timestamp.replace("+00:00", "+00:00")) + parsed = datetime.fromisoformat(timestamp) assert parsed.tzinfo is not None
584-587: Fragile decimal place verification.The string-based check
len(str(...).split(".")[-1]) <= 2has edge cases:
- Integer
25→['25']→[-1]is'25'with length 2 (passes but checks integer part)- Float
25.0→['25', '0']→ works correctlyA more robust approach would compare against the rounded value directly.
♻️ Alternative approach
# Verify rounding by checking value equality assert data["tokens"]["percent"] == round(data["tokens"]["percent"], 2) assert data["cost"]["percent"] == round(data["cost"]["percent"], 2) assert data["messages"]["percent"] == round(data["messages"]["percent"], 2)
686-700: Test uses realtime.sleep()which slows execution.While this test correctly verifies time-dependent behavior, using real delays slows the test suite. Consider mocking
datetime.now()to simulate time passage without actual waiting.
722-727: Overly permissive assertion.In January 2026, Europe/Warsaw is UTC+1 (no DST), so 23:00 UTC should always convert to 00:00 Warsaw time. The assertion accepts either "00:00" or "23:00", which could mask conversion bugs.
♻️ Tighter assertion
# 23:00 UTC = 00:00 next day Warsaw time - # Could be either depending on DST formatted = data["reset"]["formattedTime"] - assert "00:00" in formatted or "23:00" in formatted + assert "00:00" in formatted # January = UTC+1, no DST
1134-1161: Test lacks concrete assertion on logger call.The test patches
logging.getLoggerbut doesn't assert that the error method was called. The comment "Logger should have been called" isn't enforced. This test may pass even if logging is broken.♻️ Add assertion
try: controller._write_state_file(sample_processed_data, sample_args) except Exception: pass - # Logger should have been called for error - # (either in exception handler or for warnings) + # Verify logger.error was called + mock_log_instance.error.assert_called_once() # Restore permissions temp_report_dir.chmod(0o755)
| written_content = None | ||
|
|
||
| def capture_write(content: str) -> None: | ||
| nonlocal written_content | ||
| written_content = content | ||
|
|
||
| with patch.object(Path, "write_text", side_effect=capture_write): | ||
| controller._write_state_file(sample_processed_data, sample_args) |
There was a problem hiding this comment.
Inconsistent mock signature may cause test failure.
The capture_write function at Line 1258 only accepts content: str, but Path.write_text is an instance method that receives self as the first argument. When used with side_effect, all arguments are passed, so self (the Path instance) would be assigned to content.
Compare with Line 1234 which correctly includes self: Path in the signature.
🔧 Suggested fix
- def capture_write(content: str) -> None:
+ def capture_write(self: Path, content: str) -> None:
nonlocal written_content
written_content = content📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| written_content = None | |
| def capture_write(content: str) -> None: | |
| nonlocal written_content | |
| written_content = content | |
| with patch.object(Path, "write_text", side_effect=capture_write): | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| written_content = None | |
| def capture_write(self: Path, content: str) -> None: | |
| nonlocal written_content | |
| written_content = content | |
| with patch.object(Path, "write_text", side_effect=capture_write): | |
| controller._write_state_file(sample_processed_data, sample_args) |
🤖 Prompt for AI Agents
In `@src/tests/test_display_controller.py` around lines 1256 - 1263, The test's
side_effect function capture_write has the wrong signature and will receive the
Path instance as the first argument; update capture_write to accept the Path
receiver (e.g., self: Path) and the content (or use *args, **kwargs) so it
matches Path.write_text's signature; ensure this patched function stores the
actual text passed and then run
controller._write_state_file(sample_processed_data, sample_args) as before
(referencing capture_write, Path.write_text, and controller._write_state_file to
locate the change).
| written_content = None | ||
|
|
||
| def capture_write(content: str) -> None: | ||
| nonlocal written_content | ||
| written_content = content | ||
|
|
||
| with patch.object(Path, "write_text", side_effect=capture_write): | ||
| controller._write_state_file(sample_processed_data, sample_args) |
There was a problem hiding this comment.
Same mock signature issue as noted earlier.
The capture_write function is missing the self parameter. This pattern repeats here and at Line 1358.
🔧 Suggested fix
- def capture_write(content: str) -> None:
+ def capture_write(self: Path, content: str) -> None:
nonlocal written_content
written_content = contentApply the same fix at Line 1358.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| written_content = None | |
| def capture_write(content: str) -> None: | |
| nonlocal written_content | |
| written_content = content | |
| with patch.object(Path, "write_text", side_effect=capture_write): | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| written_content = None | |
| def capture_write(self: Path, content: str) -> None: | |
| nonlocal written_content | |
| written_content = content | |
| with patch.object(Path, "write_text", side_effect=capture_write): | |
| controller._write_state_file(sample_processed_data, sample_args) |
🤖 Prompt for AI Agents
In `@src/tests/test_display_controller.py` around lines 1328 - 1335, The test's
capture_write mock used for Path.write_text has the wrong signature: the
function capture_write defined for the patch of Path.write_text is missing the
first positional parameter that the real method receives (self); update the
capture_write used around controller._write_state_file to accept (self, content:
str) -> None so the patched call matches Path.write_text, and apply the
identical change to the other capture_write defined at the later occurrence (the
one mentioned around Line 1358) so both mocks mirror Path.write_text's
signature.
Code changes: - Fix timezone inconsistency: use UTC for lastUpdate timestamp to match reset.timestamp Documentation changes: - Fix documentation: correct default timezone from America/New_York to Europe/Warsaw - Fix documentation: clarify reset_time is from processed_data, not --reset-hour setting Test updates: - Update test_last_update_is_recent to use UTC timestamps matching new implementation - Note: CodeRabbit's suggestion to add 'self' parameter to mock functions was incorrect and has not been applied (patch.object side_effect doesn't receive self) Resolves CodeRabbit review feedback on PR Maciek-roboblog#184 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Code improvements (display_controller.py):
- Remove redundant imports (json, os, timedelta, Path already at module level)
- Add defensive environment variable handling with fallback path
Documentation improvements (STATE_FILE.md):
- Add language specifier to code block for better markdown rendering
- Clarify file write behavior (not truly atomic, but typically complete)
Test improvements (test_write_state_file.py):
- Fix no-op string replacement (.replace("+00:00", "+00:00"))
- Replace misleading test with pytest.skip (behavior tested elsewhere)
- Rename test_state_file_written_atomically → test_state_file_written_completely
- Fix misleading docstring about percentage calculation
- Use more robust decimal verification (value == round(value, 2))
- Mock time instead of sleep for faster, more deterministic tests
- Tighten timezone assertion (January = UTC+1, no DST ambiguity)
- Add missing logger.error assertion
Result: Cleaner code, more accurate documentation, more robust tests
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@src/tests/test_write_state_file.py`:
- Around line 1206-1223: The test
test_graceful_failure_when_processed_data_malformed currently swallows
exceptions with try/except and has no assertions; remove the try/except so the
call to DisplayController._write_state_file runs normally and assert observable
behavior instead — for example, assert that no state file was created under
temp_report_dir after calling controller._write_state_file(malformed_data,
sample_args) and/or patch the controller's logger (or processLogger) and assert
logger.error was called; reference DisplayController._write_state_file,
test_graceful_failure_when_processed_data_malformed, sample_args, and
temp_report_dir when making these changes.
- Around line 1112-1125: The test test_graceful_failure_when_env_var_not_set
currently swallows all exceptions with a try/except: pass which negates its
assertion; update the test to explicitly assert no exception by either removing
the try/except so any exception fails the test or catching Exception and calling
pytest.fail with the exception message; target the call to
controller._write_state_file(sample_processed_data, sample_args) (and keep the
patch.dict(os.environ, {}, clear=True) setup) so failures surface instead of
being silently ignored.
- Around line 1072-1087: The test
test_graceful_failure_when_directory_doesnt_exist currently swallows any
exception with a try/except, making it vacuous; remove the try/except and simply
invoke controller._write_state_file(sample_processed_data, sample_args) inside
the patched environment (with patch.dict(os.environ,
{"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir})) so the test will fail if an
exception propagates (mirroring the pattern used in
test_no_exception_raised_on_write_failure).
- Around line 1089-1110: The test test_graceful_failure_on_permission_errors
currently wraps controller._write_state_file(...) in a try/except which swallows
any exception and makes the test always pass; remove the try/except and call
controller._write_state_file(sample_processed_data, sample_args) directly (with
the patch.dict setting CLAUDE_MONITOR_REPORT_DIR to str(temp_report_dir)) so any
permission-related exception will fail the test, keeping the subsequent
temp_report_dir.chmod(0o755) restore for cleanup.
In `@STATE_FILE.md`:
- Around line 424-425: The doc claims "atomic writes" but the implementation
uses Path.write_text(), which is not atomic; update the wording in the sentence
referencing "atomic writes" to accurately state that writes are non-atomic (or
may be mid-write) and that readers could observe partial content, and adjust the
adjacent guidance to recommend retrying reads after a brief delay (10–50ms) or
using a truly atomic write method (e.g., write to temp file + rename) — locate
and edit the text mentioning "atomic writes" and the referenced
Path.write_text() to keep the description consistent.
- Line 41: Update the "lastUpdate" example value in STATE_FILE.md to include the
timezone offset shown by datetime.now(timezone.utc).isoformat(); replace the
naive timestamp example "2026-01-09T13:30:22.976757" with an ISO8601 string that
includes the +00:00 offset (matching the output of
datetime.now(timezone.utc).isoformat()), so consumers are not misled into
treating it as a naive timestamp.
- Around line 95-97: The example for the `lastUpdate` field in STATE_FILE.md is
missing the UTC offset; update the `lastUpdate` example value to include the
timezone offset (e.g., append "+00:00") so it matches ISO 8601 with offset and
is consistent with the other examples that include `+00:00` for UTC; edit the
`lastUpdate` example string in the markdown to include the "+00:00" suffix.
🧹 Nitpick comments (4)
src/claude_monitor/ui/display_controller.py (2)
390-396: Missingmkdir— write fails silently if the report directory doesn't exist.If a user sets
CLAUDE_MONITOR_REPORT_DIRto a new path, or the default~/.claude-monitor/reportshasn't been created yet,write_textwill raiseFileNotFoundErrorand the feature silently does nothing. Adding amkdircall makes the feature self-contained.📝 Suggested fix
report_dir = os.environ.get( "CLAUDE_MONITOR_REPORT_DIR", str(Path.home() / ".claude-monitor" / "reports") ) state_file = Path(report_dir) / 'current.json' + state_file.parent.mkdir(parents=True, exist_ok=True) state_file.write_text(json.dumps(state, indent=2))
343-346:secondsRemainingcan be negative whenreset_timeis in the past.If the session reset has already occurred but the monitor hasn't refreshed yet,
reset_time - current_timeproduces a negative value. Consumers building countdown timers may not expect negative seconds. Consider clamping tomax(0, ...).📝 Suggested fix
- reset_seconds = int((reset_time - current_time).total_seconds()) + reset_seconds = max(0, int((reset_time - current_time).total_seconds()))src/tests/test_write_state_file.py (2)
1127-1154: This test is fragile:chmod(0o444)doesn't restrict root, andtry/exceptweakens the assertion.In CI environments running as root, the permission change won't trigger a write error, so
logger.errorwon't be called and the assertion will fail. Consider usingpatch.objectonPath.write_textto raisePermissionErrorinstead for a deterministic test. Also remove thetry/exceptwrapper.📝 Suggested fix
def test_logger_called_on_errors( self, controller: DisplayController, temp_report_dir: Path, sample_processed_data: Dict[str, Any], sample_args: Mock, ) -> None: """Test logger is called on errors.""" - # Make directory read-only to trigger error - temp_report_dir.chmod(0o444) - with patch.dict( os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} ): - with patch("logging.getLogger") as mock_logger: - mock_log_instance = Mock() - mock_logger.return_value = mock_log_instance - - try: - controller._write_state_file(sample_processed_data, sample_args) - except Exception: - pass - - # Verify logger.error was called - mock_log_instance.error.assert_called_once() - - # Restore permissions - temp_report_dir.chmod(0o755) + with patch("pathlib.Path.write_text", side_effect=PermissionError("mocked")): + with patch("logging.getLogger") as mock_logger: + mock_log_instance = Mock() + mock_logger.return_value = mock_log_instance + + controller._write_state_file(sample_processed_data, sample_args) + + mock_log_instance.error.assert_called_once()
282-303: Token percentage test doesn't exercise_write_state_file's own calculation.
tokens_percentin the state file comes fromprocessed_data["usage_percentage"], not fromtokens_used / token_limit. This test sets bothtokens_used=15000andtoken_limit=50000, then asserts(15000/50000)*100, but the actual written value is whateverusage_percentageis (which is30.0from the fixture). The test passes coincidentally because the fixture'susage_percentageequals the expected calculation. Renaming or adjusting the test to make clear it validates the pass-through ofusage_percentagewould be more accurate.
| def test_graceful_failure_when_directory_doesnt_exist( | ||
| self, | ||
| controller: DisplayController, | ||
| sample_processed_data: Dict[str, Any], | ||
| sample_args: Mock, | ||
| ) -> None: | ||
| """Test graceful failure when directory doesn't exist.""" | ||
| nonexistent_dir = "/nonexistent/directory/that/should/not/exist" | ||
|
|
||
| with patch.dict(os.environ, {"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir}): | ||
| # Should not raise exception | ||
| try: | ||
| controller._write_state_file(sample_processed_data, sample_args) | ||
| except Exception: | ||
| # Should handle gracefully | ||
| pass |
There was a problem hiding this comment.
try/except makes this test vacuous — it passes even if the method raises.
The intent is to verify no exception propagates, but wrapping in try/except: pass means the test always passes. Simply call the method without the try/except, like test_no_exception_raised_on_write_failure (Line 1156) does correctly.
📝 Suggested fix
def test_graceful_failure_when_directory_doesnt_exist(
self,
controller: DisplayController,
sample_processed_data: Dict[str, Any],
sample_args: Mock,
) -> None:
"""Test graceful failure when directory doesn't exist."""
nonexistent_dir = "/nonexistent/directory/that/should/not/exist"
with patch.dict(os.environ, {"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir}):
- # Should not raise exception
- try:
- controller._write_state_file(sample_processed_data, sample_args)
- except Exception:
- # Should handle gracefully
- pass
+ # Should not raise exception - fail silently
+ controller._write_state_file(sample_processed_data, sample_args)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_graceful_failure_when_directory_doesnt_exist( | |
| self, | |
| controller: DisplayController, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure when directory doesn't exist.""" | |
| nonexistent_dir = "/nonexistent/directory/that/should/not/exist" | |
| with patch.dict(os.environ, {"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir}): | |
| # Should not raise exception | |
| try: | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| except Exception: | |
| # Should handle gracefully | |
| pass | |
| def test_graceful_failure_when_directory_doesnt_exist( | |
| self, | |
| controller: DisplayController, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure when directory doesn't exist.""" | |
| nonexistent_dir = "/nonexistent/directory/that/should/not/exist" | |
| with patch.dict(os.environ, {"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir}): | |
| # Should not raise exception - fail silently | |
| controller._write_state_file(sample_processed_data, sample_args) |
🤖 Prompt for AI Agents
In `@src/tests/test_write_state_file.py` around lines 1072 - 1087, The test
test_graceful_failure_when_directory_doesnt_exist currently swallows any
exception with a try/except, making it vacuous; remove the try/except and simply
invoke controller._write_state_file(sample_processed_data, sample_args) inside
the patched environment (with patch.dict(os.environ,
{"CLAUDE_MONITOR_REPORT_DIR": nonexistent_dir})) so the test will fail if an
exception propagates (mirroring the pattern used in
test_no_exception_raised_on_write_failure).
| def test_graceful_failure_on_permission_errors( | ||
| self, | ||
| controller: DisplayController, | ||
| temp_report_dir: Path, | ||
| sample_processed_data: Dict[str, Any], | ||
| sample_args: Mock, | ||
| ) -> None: | ||
| """Test graceful failure on permission errors.""" | ||
| # Make directory read-only | ||
| temp_report_dir.chmod(0o444) | ||
|
|
||
| with patch.dict( | ||
| os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} | ||
| ): | ||
| # Should not raise exception | ||
| try: | ||
| controller._write_state_file(sample_processed_data, sample_args) | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Restore permissions for cleanup | ||
| temp_report_dir.chmod(0o755) |
There was a problem hiding this comment.
Same try/except issue as above — test always passes regardless of behavior.
📝 Suggested fix
def test_graceful_failure_on_permission_errors(
self,
controller: DisplayController,
temp_report_dir: Path,
sample_processed_data: Dict[str, Any],
sample_args: Mock,
) -> None:
"""Test graceful failure on permission errors."""
- # Make directory read-only
temp_report_dir.chmod(0o444)
with patch.dict(
os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)}
):
- # Should not raise exception
- try:
- controller._write_state_file(sample_processed_data, sample_args)
- except Exception:
- pass
+ controller._write_state_file(sample_processed_data, sample_args)
- # Restore permissions for cleanup
temp_report_dir.chmod(0o755)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_graceful_failure_on_permission_errors( | |
| self, | |
| controller: DisplayController, | |
| temp_report_dir: Path, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure on permission errors.""" | |
| # Make directory read-only | |
| temp_report_dir.chmod(0o444) | |
| with patch.dict( | |
| os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} | |
| ): | |
| # Should not raise exception | |
| try: | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| except Exception: | |
| pass | |
| # Restore permissions for cleanup | |
| temp_report_dir.chmod(0o755) | |
| def test_graceful_failure_on_permission_errors( | |
| self, | |
| controller: DisplayController, | |
| temp_report_dir: Path, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure on permission errors.""" | |
| temp_report_dir.chmod(0o444) | |
| with patch.dict( | |
| os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} | |
| ): | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| temp_report_dir.chmod(0o755) |
🤖 Prompt for AI Agents
In `@src/tests/test_write_state_file.py` around lines 1089 - 1110, The test
test_graceful_failure_on_permission_errors currently wraps
controller._write_state_file(...) in a try/except which swallows any exception
and makes the test always pass; remove the try/except and call
controller._write_state_file(sample_processed_data, sample_args) directly (with
the patch.dict setting CLAUDE_MONITOR_REPORT_DIR to str(temp_report_dir)) so any
permission-related exception will fail the test, keeping the subsequent
temp_report_dir.chmod(0o755) restore for cleanup.
| def test_graceful_failure_when_env_var_not_set( | ||
| self, | ||
| controller: DisplayController, | ||
| sample_processed_data: Dict[str, Any], | ||
| sample_args: Mock, | ||
| ) -> None: | ||
| """Test graceful failure when environment variable not set.""" | ||
| # Remove the environment variable | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| # Should not raise exception | ||
| try: | ||
| controller._write_state_file(sample_processed_data, sample_args) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Same pattern — try/except: pass negates the test assertion.
📝 Suggested fix
with patch.dict(os.environ, {}, clear=True):
- # Should not raise exception
- try:
- controller._write_state_file(sample_processed_data, sample_args)
- except Exception:
- pass
+ controller._write_state_file(sample_processed_data, sample_args)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_graceful_failure_when_env_var_not_set( | |
| self, | |
| controller: DisplayController, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure when environment variable not set.""" | |
| # Remove the environment variable | |
| with patch.dict(os.environ, {}, clear=True): | |
| # Should not raise exception | |
| try: | |
| controller._write_state_file(sample_processed_data, sample_args) | |
| except Exception: | |
| pass | |
| def test_graceful_failure_when_env_var_not_set( | |
| self, | |
| controller: DisplayController, | |
| sample_processed_data: Dict[str, Any], | |
| sample_args: Mock, | |
| ) -> None: | |
| """Test graceful failure when environment variable not set.""" | |
| # Remove the environment variable | |
| with patch.dict(os.environ, {}, clear=True): | |
| controller._write_state_file(sample_processed_data, sample_args) |
🤖 Prompt for AI Agents
In `@src/tests/test_write_state_file.py` around lines 1112 - 1125, The test
test_graceful_failure_when_env_var_not_set currently swallows all exceptions
with a try/except: pass which negates its assertion; update the test to
explicitly assert no exception by either removing the try/except so any
exception fails the test or catching Exception and calling pytest.fail with the
exception message; target the call to
controller._write_state_file(sample_processed_data, sample_args) (and keep the
patch.dict(os.environ, {}, clear=True) setup) so failures surface instead of
being silently ignored.
| def test_graceful_failure_when_processed_data_malformed( | ||
| self, | ||
| controller: DisplayController, | ||
| temp_report_dir: Path, | ||
| sample_args: Mock, | ||
| ) -> None: | ||
| """Test graceful failure when processed_data is malformed.""" | ||
| # Missing required fields | ||
| malformed_data = {"tokens_used": 100} | ||
|
|
||
| with patch.dict( | ||
| os.environ, {"CLAUDE_MONITOR_REPORT_DIR": str(temp_report_dir)} | ||
| ): | ||
| # Should handle gracefully | ||
| try: | ||
| controller._write_state_file(malformed_data, sample_args) | ||
| except Exception: | ||
| pass # Expected to fail, but shouldn't crash the app |
There was a problem hiding this comment.
Same try/except: pass issue — and the test doesn't assert anything.
This test should verify that the method doesn't raise (remove the try/except) and ideally assert on some observable behavior (e.g., file not created, or logger called).
🤖 Prompt for AI Agents
In `@src/tests/test_write_state_file.py` around lines 1206 - 1223, The test
test_graceful_failure_when_processed_data_malformed currently swallows
exceptions with try/except and has no assertions; remove the try/except so the
call to DisplayController._write_state_file runs normally and assert observable
behavior instead — for example, assert that no state file was created under
temp_report_dir after calling controller._write_state_file(malformed_data,
sample_args) and/or patch the controller's logger (or processLogger) and assert
logger.error was called; reference DisplayController._write_state_file,
test_graceful_failure_when_processed_data_malformed, sample_args, and
temp_report_dir when making these changes.
| "tokens": 32859.04, | ||
| "messages": 0 | ||
| }, | ||
| "lastUpdate": "2026-01-09T13:30:22.976757" |
There was a problem hiding this comment.
lastUpdate example is missing timezone offset.
The code uses datetime.now(timezone.utc).isoformat() (display_controller.py Line 387), which produces a string with +00:00 suffix. The example here omits it, which could mislead consumers into treating this as a naive timestamp.
📝 Suggested fix
- "lastUpdate": "2026-01-09T13:30:22.976757"
+ "lastUpdate": "2026-01-09T13:30:22.976757+00:00"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "lastUpdate": "2026-01-09T13:30:22.976757" | |
| "lastUpdate": "2026-01-09T13:30:22.976757+00:00" |
🤖 Prompt for AI Agents
In `@STATE_FILE.md` at line 41, Update the "lastUpdate" example value in
STATE_FILE.md to include the timezone offset shown by
datetime.now(timezone.utc).isoformat(); replace the naive timestamp example
"2026-01-09T13:30:22.976757" with an ISO8601 string that includes the +00:00
offset (matching the output of datetime.now(timezone.utc).isoformat()), so
consumers are not misled into treating it as a naive timestamp.
| - **`lastUpdate`** (string): ISO 8601 timestamp when this file was last written | ||
| - Example: `"2026-01-09T13:30:22.976757"` | ||
| - Useful for detecting stale data |
There was a problem hiding this comment.
Same issue: lastUpdate example lacks +00:00.
Consistent with the fix above, update this example too.
📝 Suggested fix
-- **`lastUpdate`** (string): ISO 8601 timestamp when this file was last written
- - Example: `"2026-01-09T13:30:22.976757"`
+- **`lastUpdate`** (string): ISO 8601 UTC timestamp when this file was last written
+ - Example: `"2026-01-09T13:30:22.976757+00:00"`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **`lastUpdate`** (string): ISO 8601 timestamp when this file was last written | |
| - Example: `"2026-01-09T13:30:22.976757"` | |
| - Useful for detecting stale data | |
| - **`lastUpdate`** (string): ISO 8601 UTC timestamp when this file was last written | |
| - Example: `"2026-01-09T13:30:22.976757+00:00"` | |
| - Useful for detecting stale data |
🤖 Prompt for AI Agents
In `@STATE_FILE.md` around lines 95 - 97, The example for the `lastUpdate` field
in STATE_FILE.md is missing the UTC offset; update the `lastUpdate` example
value to include the timezone offset (e.g., append "+00:00") so it matches ISO
8601 with offset and is consistent with the other examples that include `+00:00`
for UTC; edit the `lastUpdate` example string in the markdown to include the
"+00:00" suffix.
| - File may be mid-write when read (very rare with atomic writes) | ||
| - Retry read after brief delay (10-50ms) |
There was a problem hiding this comment.
"Atomic writes" claim is inaccurate — the implementation uses Path.write_text().
Line 388 correctly states that Path.write_text() is used, but this section refers to "atomic writes." Path.write_text() is not atomic — a reader could observe a partial file if it reads during a write. Consider changing the wording to be consistent with Line 388–389.
📝 Suggested fix
-- File may be mid-write when read (very rare with atomic writes)
-- Retry read after brief delay (10-50ms)
+- File may be mid-write when read (rare under normal conditions, but possible since writes are not atomic)
+- Retry read after brief delay (10-50ms)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - File may be mid-write when read (very rare with atomic writes) | |
| - Retry read after brief delay (10-50ms) | |
| - File may be mid-write when read (rare under normal conditions, but possible since writes are not atomic) | |
| - Retry read after brief delay (10-50ms) |
🤖 Prompt for AI Agents
In `@STATE_FILE.md` around lines 424 - 425, The doc claims "atomic writes" but the
implementation uses Path.write_text(), which is not atomic; update the wording
in the sentence referencing "atomic writes" to accurately state that writes are
non-atomic (or may be mid-write) and that readers could observe partial content,
and adjust the adjacent guidance to recommend retrying reads after a brief delay
(10–50ms) or using a truly atomic write method (e.g., write to temp file +
rename) — locate and edit the text mentioning "atomic writes" and the referenced
Path.write_text() to keep the description consistent.
1 participant
Summary
Adds
--write-stateflag to enable writing current monitoring state to~/.claude-monitor/reports/current.jsonfor consumption by external applications (VS Code extensions, status bar widgets, dashboards).Motivation
External tools (IDE extensions, system widgets) need access to real-time Claude usage statistics without parsing terminal output. This feature provides a clean JSON API for integration.
Changes
write_stateboolean field to Settings with CLI flag support_write_state_file()method in DisplayControllerFiles Modified
src/claude_monitor/cli/bootstrap.py- Environment variable setupsrc/claude_monitor/cli/main.py- Flag passing to DisplayControllersrc/claude_monitor/core/settings.py- Settings field definitionsrc/claude_monitor/ui/display_controller.py- Core implementationSTATE_FILE.md- Comprehensive integration documentation (NEW)README.md- Feature documentation and examplessrc/tests/test_write_state_file.py- Core tests (NEW, 55 tests)src/tests/test_display_controller.py- Integration tests (14 new tests)src/tests/test_settings.py- Settings tests (14 new tests)Type of Change
Testing
Documentation
Checklist
Example Output
```json
{
"messages": {"used": 140, "limit": 250, "percent": 56.0},
"tokens": {"used": 17582, "limit": 38705, "percent": 45.43},
"cost": {"used": 5.13, "limit": 50.0, "percent": 10.25},
"reset": {
"timestamp": "2026-01-10T18:00:00+00:00",
"secondsRemaining": 7920,
"formattedTime": "6:00 PM"
},
"burnRate": {"tokens": 32859.04, "messages": 0},
"lastUpdate": "2026-01-10T15:48:00.123456"
}
```
Breaking Changes
None - feature is completely opt-in via
--write-stateflag.Use Cases
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests