fix: treat watchdog as always-on hard dependency (#31)#33
Merged
Conversation
- Move Observer import to module top level, remove dead except ImportError guard - Simplify _stop_observer to call .stop()/.join() directly (no getattr dance) - Fix README: remove misleading 'uv add watchdog' conditional phrasing - Update implementation.md and plan.md docs to remove optional language - Add test_start_observer_returns_running_observer unit test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns the codebase and documentation with the fact that watchdog>=4 is a hard production dependency, removing now-dead “optional dependency” handling and clarifying auto-refresh behavior.
Changes:
- Simplifies watchdog observer lifecycle in the CLI by importing
Observerat module scope and removing deadImportError/getattrindirection. - Updates README and internal docs to describe auto-refresh as always-on (with the existing 2s debounce).
- Adds a unit test to assert
_start_observerreturns a running observer and cleans it up via_stop_observer.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/copilot_usage/cli.py |
Makes watchdog observer usage unconditional and simplifies start/stop logic. |
tests/copilot_usage/test_cli.py |
Adds coverage for starting/stopping the watchdog observer. |
README.md |
Removes misleading optional-install instructions for watchdog. |
src/copilot_usage/docs/plan.md |
Removes “if watchdog is installed” phrasing from the plan. |
src/copilot_usage/docs/implementation.md |
Updates the observer description to match always-on behavior and existing directory check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use object types for observer (watchdog has no type stubs) with type: ignore on method calls. Fixes CI failure on PR #33. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Closes #31
Problem
watchdog>=4is listed as a hard production dependency inpyproject.toml, but two places treat it as optional:except ImportErrorguard in_start_observer— unreachable because watchdog is always installeduv add watchdogfor a feature that's always active_stop_observergetattrdance — unnecessary indirection since the watchdog API is always presentSolution (Option B — always-on)
cli.py: ImportObserverat module top level; removetry/except ImportErrorin_start_observer; simplify_stop_observerto call.stop()/.join()directlyREADME.md: Replace conditional phrasing with "The display auto-refreshes when session files change (2-second debounce)."implementation.md/plan.md: Remove "if watchdog is installed" language from docsTests
test_start_observer_returns_running_observer— asserts_start_observerreturns a non-None, alive observer for an existing directory, then cleans up via_stop_observerWarning
The following domains were blocked by the firewall during workflow execution:
astral.shpypi.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.