chore(scanner): delete the unused transcript reader, refuse a non-path (#591) - #593

ZETETIC-REVIEW: APPROVE

Summary

Two independent, well-scoped follow-ups from #592's review: delete dead code (scanner.iter_tool_uses) and change a crash into a refusal (transcript_activity on a non-path transcript_path). Both claims in the PR description hold up under independent verification.

Stakes

Medium: touches mcp_server/infrastructure/, a shared layer with several importers, but the change is a pure deletion plus a narrow type-guard, not new business logic. Reviewed at full Move 1-6 depth anyway given the hook-path crash surface.

Move 0: ledger / seen-defect check

No unmapped branches; no "pre-existing/unrelated/out of scope" rationalization anywhere in the PR body or commit message. Proceeds normally.

(1) Is iter_tool_uses truly unreferenced?

Confirmed dead at commit 8239287d:

• git grep -n "iter_tool_uses" 8239287d -- . returns only prose: CHANGELOG.md:24, docs/adr/ADR-0595-scanner.md:23, docs/adr/ADR-1073-...md:21,27, wiki/adr/cortex/1073-...md:19,25, wiki/adr/migration/0595-scanner.md:21: no code reference anywhere.
• No __all__ in mcp_server/infrastructure/scanner.py (checked with grep -n "__all__", empty) that would signal a public re-export contract.
• No getattr(scanner, ...), no importlib dynamic lookup, no entry-point in pyproject.toml referencing it (checked).
• git grep -n "from.*scanner import\|import scanner" 8239287d shows the only live scanner imports are read_head_tail (backfill_memories.py, import_sessions.py, rebuild_profiles.py, test_scanner.py) and discover_conversations_for_projects (explore_features.py, rebuild_profiles.py, test_scanner.py): never iter_tool_uses.
• git log -S"iter_tool_uses" confirms it arrived once in bulk commit dc6876c9 and was never referenced afterward, matching the PR's own claim.

(2) Public-surface change for importers of scanner.py?

None of the four live importers reference iter_tool_uses; deleting it does not change any importer's usable surface. No __all__/re-export contract exists to break.

(3) Baseline entry removal legitimate?

wc -l mcp_server/infrastructure/scanner.py = 271 after the deletion (was over the 300-line cap before, hence the baseline entry). python scripts/check_craftsmanship.py --base origin/main → Craftsmanship gate: OK on this worktree. The removed .craftsmanship-baseline.json entry (scanner.py, file-size, "exceeds 300-line cap") is exactly the entry this file-size reduction earns removal of: legitimate, not a silent baseline weakening elsewhere (diff touches only that one entry).

(4) Path guard: does it accept every legitimate shape?

transcript_activity.py:64: if not transcript_path or not isinstance(transcript_path, (str, PathLike)): return empty. Verified directly:

issubclass(pathlib.PurePath, os.PathLike) -> True
issubclass(<custom __fspath__ class>, os.PathLike) -> True
isinstance(pathlib.PurePosixPath(...), os.PathLike) -> True

os.PathLike.__subclasshook__ checks for __fspath__, so Path, PurePath/PurePosixPath/PureWindowsPath, and any third-party path-like object implementing __fspath__ are all accepted; only non-path-like truthy values (int, float, bool, list, dict) are refused. tests_py/infrastructure/test_transcript_activity.py adds exactly this matrix: test_a_path_object_still_works (a real Path) plus five refused non-path shapes (int, float, bool, list, dict): no legitimate shape is refused, no gap found needing a new test.

(5) Mutation check

Mutated mcp_server/infrastructure/transcript_activity.py:64 twice, both restored via git checkout --:

• Drop the isinstance guard entirely (if not transcript_path:) → 5/17 new tests fail (all five non-path parametrized cases), exactly as expected.
• Weaken (str, PathLike) to str only → test_a_path_object_still_works fails (AssertionError: assert [] == ['Read']).
  Both mutants killed. git status --short clean after restore.

(6) Gates + full suite

• .venv/bin/python -m pyright mcp_server/ → 0 errors, 0 warnings, 0 informations.
• uv run ruff check . → All checks passed. uv run ruff format --check . → 1465 files already formatted.
• uv run python scripts/check_craftsmanship.py --base origin/main → OK.
• uv run pytest -q (full suite, no -k) → 8736 passed, 8 skipped, 2 warnings, 459 subtests passed in 263.73s: matches the PR's claimed 8736 passed, 8 skipped exactly.

SOLID / layering / wiring

No layer-boundary change (pure deletion + guard inside the same infrastructure/ module). No new public symbol added. os.PathLike import is stdlib, no new dependency. No OCP/SRP/LSP/ISP/DIP concerns: the guard is a single added isinstance check, not a new branch of business logic.

Issues

Blocking

None.

Non-blocking

None: the PR is exactly the two follow-ups it claims to be, both independently reproduced.

Verdict

APPROVE. All six checklist items verified directly against the commit; gates, tests, and two independent mutation kills confirm the fix is real and the deletion is safe.