Skip to content

[aw][code health] watchdog hard-dep treated as optional: dead ImportError guard and misleading README #31

@microsasa

Description

@microsasa

Root Cause

watchdog>=4 is listed in [project] dependencies in pyproject.toml — it is always installed as a hard production dependency. Despite this, two places treat it as optional:

1. Dead except ImportError in cli.py (_start_observer, line 128–137):

def _start_observer(session_path: Path, change_event: threading.Event) -> object | None:
    """Start a watchdog observer if available. Returns the observer or None."""
    try:
        from watchdog.observers import Observer  # type: ignore[import-untyped]
        ...
    except ImportError:
        return None   # ← unreachable: watchdog is a hard dep

The except ImportError branch can never execute because watchdog is unconditionally installed. This is dead code that misleads future readers into thinking watchdog might be absent.

2. Misleading README (Installation section):

If watchdog is installed, the display auto-refreshes when session files change (2-second debounce). Install it with uv add watchdog for live-updating views.

This actively misleads users into running a redundant uv add watchdog for a feature that is always active.


Resolution Spec

Option A — keep watchdog optional (recommended if lightweight installs matter):

  • Move watchdog>=4 from [project] dependencies to [project.optional-dependencies] under a key like watch
  • Keep the try/except ImportError guard (it becomes live code again)
  • Update README to explain the optional install

Option B — watchdog is always-on (simpler):

  • Remove the try/except ImportError guard in _start_observer; import at module top level
  • Remove _stop_observer's getattr dance (watchdog API is always present)
  • Fix README: "The display auto-refreshes when session files change (2-second debounce)." — no conditional phrasing

Either option removes the misleading copy. Pick whichever matches project intent.


Testing Requirement

  • Add a unit test for _start_observer that asserts it returns a non-None observer when given an existing directory (proving the live code path works)
  • If going with Option A: add a test that mocks the ImportError and asserts _start_observer returns None gracefully
  • Verify README no longer contains uv add watchdog language

Generated by Code Health Analysis ·

Metadata

Metadata

Assignees

No one assigned

    Labels

    awCreated by agentic workflowcode-healthCode cleanup and maintenance

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions