fix: post-release review polish - #11
Merged
Merged
Conversation
Five issues caught in a fresh codebase review: - test_stats_with_data and test_stats_reads_timestamp_key were writing event timestamps up to 50 minutes in the past, which could straddle local midnight and drop events from the "today" bucket. Tightened the windows to a few seconds. - Hook grace-period arithmetic assumed NOW >= stored timestamps. A backward clock jump made (NOW - GRACE_LAST_CALL_AT) negative, which bypassed rest indefinitely. Added [ NOW -ge X ] guards so skew falls through to block instead. - uninstall.sh was silently rm -rf'ing ~/.tomato (focus history and checkpoints) on every run. Now preserved by default; --purge deletes. - load_config() silently accepted invalid user values (negative ints, wrong types, bools). Now warns and falls back to the default. - README no longer claims specific millisecond numbers for the hook fast path without a benchmark to back them up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five issues caught in a fresh full-codebase review after v0.1.0 shipped. No behavior changes for the normal path; all fixes target edge cases and UX.
Fixes
tests/test_cli.py):test_stats_with_dataandtest_stats_reads_timestamp_keywrote event timestamps up to 50 min in the past. When the test ran near local midnight, the earliest events landed on "yesterday" and got filtered out of the "today" stats bucket. Tightened windows to a few seconds.skills/tomato/bin/tomato-hook.sh): the grace arithmetic assumedNOW >= GRACE_*_AT. A backward clock jump madeNOW - GRACE_LAST_CALL_ATnegative, which evaluated as "within grace window" and bypassed rest indefinitely. Added[ NOW -ge X ]guards so skew falls through to block.uninstall.shpreserves history by default (uninstall.sh): previouslyrm -rf ~/.tomato/silently deleted focus history, checkpoints, and config on every run. Now preserved; pass--purgeto delete.skills/tomato/bin/tomato-cli.py):load_config()silently accepted invalid values (negative ints, wrong types, bools). Now warns and falls back to the default per-key.README.md): dropped the specific~8ms/~35msnumbers; we don't have a benchmark in CI to back them up yet.Tests
TestConfigValidation::test_invalid_config_falls_back_to_defaultsTest plan
./uninstall.sh(without--purge) keeps~/.tomato/intact./uninstall.sh --purgedeletes~/.tomato/~/.tomato/config.jsonemits a warning and uses defaults🤖 Generated with Claude Code