Skip to content

Chore/code quanlity 2#3

Merged
nickroci merged 20 commits into
mainfrom
chore/code-quanlity-2
May 22, 2026
Merged

Chore/code quanlity 2#3
nickroci merged 20 commits into
mainfrom
chore/code-quanlity-2

Conversation

@nickroci

Copy link
Copy Markdown
Owner

No description provided.

nickroci and others added 20 commits May 22, 2026 09:47
Coverage uplift to a hard 90% pre-push gate (pytest-cov, statement+branch):
- daemon  76% -> 90.78%  (449 tests, was 256)
- search  63% -> 96.12%  (171 tests, was 62)
- hooks   57% -> 91.97%  (213 tests, was 38 + 7 broken)

Test infrastructure: per-package conftest fixtures (tmp knowledge dirs,
fake SDK MagicMock via mock.patch.dict, fake priming sockets, hook
runner). Hyphenated hook entrypoints (pre-tool-use.py et al) refactored
to 8-line shims importing matching underscore modules — the only way
to exercise them in-process since hyphens aren't legal identifiers.

Complexity violations cleared (ruff C901/PLR0912/PLR0915):
- Extract Method for sequential pipelines (scholar.review, ingest.poll_once,
  library_tools._move_entries_impl, _priming_client._local_priming, ...)
- Strategy chain for parsers (_response_parser.extract_json_blob,
  _blockers._parse_block_triggers, _events.append_event)
- StrEnum dispatch dict for tools/search cli main (Subcommand) and
  daemon priming_rpc handler (RpcOp)
- Rule-list refactor for scholar_prompt.check_invariants (complexity 34
  -> four pure check functions composed at the call site)

Python floor unified to 3.12 across all three packages (was 3.10/3.11/3.12),
pyright pythonVersion and ruff target-version aligned. Authors set on each
package pyproject.

No test-detection branches added to production code. All helpers
underscore-prefixed. Public API unchanged. Behavior preserved — every
existing test still passes against the refactored code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small typing-hygiene moves, no mode change (still basic):

- venvPath/venv pointed at each package's .venv so pyright's LSP can
  resolve claude_agent_sdk imports without uv's VIRTUAL_ENV. Fixes the
  editor-only "reportMissingImports" diagnostics that pre-commit's
  pyright (running under uv) never saw.
- reportUnusedImport promoted to "error" — baseline was 2 violations,
  both fixed in this commit (real dead-import cleanup, not suppression).
- reportPrivateUsage at "warning" — surfaces _underscore imports
  crossing module boundaries (mostly tests reaching into internals);
  informational only, doesn't block.

Cleanups for the two reportUnusedImport hits:
- priming_rpc.py used "from embeddings import load_or_build as _emb_check"
  purely to test importability; switched to __import__("embeddings") which
  doesn't bind a name.
- test_events.py had a redundant `import _events` followed by
  importlib.import_module("_events") on the next line; dropped the bare
  import.

Not adding a claude_agent_sdk .pyi stub — the SDK ships py.typed with
full type info in its own types.py. A local stub would duplicate
upstream and drift over time. The IDE "missing imports" warnings were
about venv discovery, not type info.

Held back on reportArgumentType / reportOptionalMemberAccess /
reportAttributeAccessIssue — those have unknown baselines once Unknown
cascades come in, and turning them on without first sizing the
backlog would brick pre-commit. Follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "(bm25 backend unavailable)" / "(embeddings not importable)"
silent-fallback shape masked real packaging bugs by degrading at
runtime. All 7 sites wrapped imports of HARD-declared deps
(bm25, embeddings, markdown_it, sentence_transformers,
claude_agent_sdk) so the guards never legitimately fired.

Removed the try/except wrappers; imports now fail loudly when
the dep is genuinely missing. Where collision exists across
the agent-mem-search flat layout (bm25.load_or_build vs
embeddings.load_or_build) names are aliased at module top.

Three dead tests (asserting the now-removed fallback strings)
deleted. Test patches that targeted sys.modules now patch the
binding on the importing module — proper monkeypatch pattern.
tools/search adds a `fake_sdk` conftest fixture replacing the
old `_install_fake_sdk` context manager so cli.py's now-module-top
SDK imports are still mockable.

Ruff PLC0415 (import-not-at-top-of-file) added to all three
pyprojects, with tests/conftests excepted. Hoisted a few
additional inline imports the new rule surfaced (yaml in
priming, claude_agent_sdk in library_tools, .paths in priming).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-commit pyright runs on staged .py files only, so a toml-only
commit doesn't trigger pyright. After this lands, every Python
edit touched by pre-commit must pass strict-mode pyright.

Baselines to fix in subsequent parallel passes:
- daemon: 399 errors across 19 source files
- src:    337 errors across 25 source files  (hooks + scripts)
- tools/search: 106 errors across 5 source files
Total: 842 strict-mode errors.

Tests are excluded from pyright entirely (still ruff-checked,
pytest-run, coverage-counted). Per user direction:
"lets not bother with strict types on tests, lets just do
standard."

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pre-push was running pyright on every .py file changed since
origin, which blocks pushes when strict-mode is on top of an
existing baseline of unfixed errors. Pyright already gates at
pre-commit; pre-push retains pytest. No loss in coverage —
just removes a redundant gate that conflicts with the parallel
strict-mode fixup workflow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eliminates 133 strict-mode pyright errors across the seven CLI script
entrypoints. The core moves:

- Add a ``State`` / ``IngestedEntry`` TypedDict in utils.py and thread it
  through compile.py, query.py, lint.py, flush.py so the on-disk
  state.json schema is enforced statically.
- Add a ``FlushState`` TypedDict in flush.py for the dedup sidecar file.
- Add an ``Issue`` TypedDict (severity Literal + NotRequired auto_fixable)
  in lint.py; replace every ``list[dict]`` return / accumulator with
  ``list[Issue]``.
- Replace scope.py's untyped ``os.PathLike`` with a parameterised
  ``Union[str, os.PathLike[str]]``.
- Replace flush.py's untyped ``kwargs: dict`` Popen spread with explicit
  per-platform Popen calls — ``creationflags`` (win) vs
  ``start_new_session`` (POSIX) — keeping every argument concretely typed.
- Lift the lazy ``claude_agent_sdk`` / ``subprocess`` / ``traceback`` /
  ``hashlib`` imports out of function bodies so PLC0415 doesn't trip when
  ruff re-checks the touched files.

No new ``# type: ignore``, no ``Any`` shortcuts, no behaviour changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… runs, buffer, main, ...)

- scheduler.py: type the tailer (JsonlTailer), event payload, in-flight
  Future set, and debounce callbacks. Replace the redundant
  `isinstance(packet, dict)` with a `cast(object, …)` round-trip so the
  runtime guard against contract-violating librarian callables survives
  pyright's narrowing (test_librarian_non_dict_return_is_dropped still
  covers it).
- runs.py: parameterise the cost-state dicts (`Dict[str, Any]`) and cast
  the json.loads result through the isinstance check.
- buffer.py: parameterise the dataclass default_factory calls so deque /
  list / dict fields carry real element types.
- __main__.py: signal handler now takes `(int, FrameType | None) -> None`.
  Hoist `knowledge_dir` to a top-level import and rename the
  `_prewarm_indexes` parameter to avoid shadowing; the optional
  `bm25`/`embeddings` imports stay local with documented `noqa: PLC0415`
  since they live in a sibling search package not always on sys.path.

0 strict-mode errors across all 8 files in the slice; full test suite
(447 tests) green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- frontmatter.py: introduce `Frontmatter = dict[str, object]` as the
  in-memory YAML shape; tighten `read`/`write`/`set_status` signatures.
  Cast PyYAML's untyped `represent_scalar` boundary cleanly.
- bm25.py: add `BM25Hit` NamedTuple for typed search returns (tuple
  subclass — daemon's existing `(p, score, _snippet)` destructure keeps
  working). Type rank_bm25 surface via a local `_BM25Backend` Protocol
  + cast at the boundary. Expose non-private aliases
  (`build_snippet`, `frontmatter_search_text`, `iter_markdown`,
  `strip_and_extract_frontmatter`) so embeddings.py can import them
  without tripping reportPrivateUsage.
- embeddings.py: switch to the new public aliases; annotate the
  embedding matrix as `npt.NDArray[np.float32]`; parameterise the
  dataclass `field(default_factory=list[_DocRecord])` so element type
  isn't lost.
- cli.py: thread `Frontmatter`/`Mapping[str, object]` through every fm
  helper. Add `_as_float` to coerce JSON-typed cost-state values
  safely. Cast `list` narrowings when iterating `applies-when`.
- aliases.py: type the json.loads boundary via `object` + isinstance,
  then cast the narrowed dict for typed iteration.

No new try/except, no `Any` shortcuts (only a single documented cast
through Any at the PyYAML `represent_scalar` boundary). 170 tests
pass; pyright clean on the slice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_schemas.py:
- Pyright cannot infer the parameter of ``list`` from a bare
  ``default_factory=list``; spell it out as ``default_factory=list[T]``
  for every non-``str`` element type (EvidenceItem, ProposedAction,
  LibrarianInterrupt, ScholarDecision, ScholarInterruptDecision,
  Dict[str, object]). Same shape, just an explicit factory generic.
- Annotated the ``_coerce_evidence`` field_validator: ``v: object`` in,
  ``object`` out. Replaced ``isinstance(v, dict)`` with the wider
  ``Mapping`` check (any user-supplied mapping passes); narrowed via
  ``cast`` where pyright keeps the type opaque inside ``isinstance``
  branches.
- Replaced the legacy ``Dict[str, Any]`` shapes on ``LibrarianResponse``
  and ``ScholarResponse`` with ``Dict[str, object]`` — semantically the
  same catch-all but without ``Any``.

llm.py:
- The Claude Agent SDK ships ``py.typed``. Threaded the proper types
  through every SDK boundary instead of bare ``dict``: ``ClaudeAgentOptions``
  for options, ``McpServerConfig`` for the mcp_servers dict,
  ``ToolPermissionContext`` on the can_use_tool callback's third arg, and
  an explicit ``Callable[..., Awaitable[PermissionResult*]]`` alias on
  ``_make_path_guard``'s return.
- Replaced ``opts: dict = dict(...); ClaudeAgentOptions(**opts)`` in
  ``run_librarian_call`` with a direct ``ClaudeAgentOptions(...)``
  construction. The previous pattern hid every keyword's type behind a
  generic ``dict`` and produced ~80 of the 111 errors here. ``cwd`` and
  ``can_use_tool`` are passed as ``None`` when no knowledge root is
  supplied — semantically identical to omitting them (both default to
  ``None`` on the dataclass).
- ``_recursion_guard_env`` now returns ``dict[str, str]`` (not bare
  ``dict``); ``_run_with_timeout`` is generic over the coroutine's
  return type via TypeVar so callers don't lose the inner result type.
- Added ``# noqa: PLC0415`` to the existing lazy SDK imports — those
  are deliberate (keep daemon startup light if the SDK is absent), and
  ruff's strict in-function-import rule trips once the file is touched.

_response_parser.py: had zero strict errors and remains untouched
(re-verified post-change).

Two ``reportPrivateUsage`` warnings on ``library_tools._SERVER_NAME``
remain — pre-existing and out of slice (would need a public alias added
in library_tools.py).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Zero strict-mode pyright errors across all 12 source files in src/hooks/
(was 204). 213 hook tests still pass; coverage holds at 91.99% (>= 90% gate).

Shared TypedDicts hoisted into _events.py so every hook in the slice
annotates the same stdin shape:
- HookPayload (total=False) — session_id, cwd, transcript_path,
  tool_name, tool_input, tool_response, error, prompt, source, reason.
- EventLine — one serialised event-line schema.
- EventPayload — the small dict shipped on the wire as the event payload.

_priming_client.py also gained PrimingRequest / PrimingResponse TypedDicts
matching daemon/agent_mem_daemon/priming_rpc.py's wire shapes, plus a
Frontmatter alias for the lite-YAML scan path and a ScoredEntry tuple
alias for the rank pipeline. The connection helper was split into
_send_request + _parse_response to keep cyclomatic complexity under
ruff's C901 cap.

scope.current_project_slug is owned by another slice and still uses an
untyped os.PathLike parameter, so its inferred signature leaks Unknown
into every caller. session_start.py, session_end.py, pre_compact.py
and user_prompt_submit.py now re-bind it through getattr + cast to a
clean str | None -> str callable at module top — keeps strict mode
happy without touching upstream and also resolves ruff's PLC0415
local-import warning these files were already tripping.

_nudges.take_nudges's per-session budget loader extracted to a helper
(_load_consumed) for the same complexity reason.

No new try/except wrappers, no # type: ignore, no Any shortcuts —
narrow Any locals appear only at JSON-parse boundaries where the input
genuinely is unknown.
Worktree agents fanned out from f25f01e, each fixing a disjoint
slice of source files under typeCheckingMode='strict':

  A (09005fa) daemon: llm + _response_parser + _schemas       (133 → 0)
  B (fe6804e) daemon: priming + priming_rpc                    ( 89 → 0)
  C (a228c8f) daemon: librarian + librarian_prompt + library_tools (60 → 0)
  D (51f7175) daemon: scholar + scholar_prompt + ingest        ( 74 → 0)
  E (8408e16) daemon: scheduler + runs + buffer + main + ...   ( 44 → 0)
  F (5054780) src/hooks/* (12 files)                           (204 → 0)
  G (c23b73a) src/scripts/* (7 files)                          (133 → 0)
  H (b246143) tools/search source modules (5 files)            (106 → 0)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Agent F's worktree pre-merged needed local getattr+cast wrappers
in pre_compact.py, session_start.py, session_end.py, and
user_prompt_submit.py because scope.current_project_slug's
original signature used a bare os.PathLike and leaked Unknown.

Agent G's parallel worktree fixed scope.py to use
os.PathLike[str], which makes those workarounds redundant.
Replace each with a direct `from scope import current_project_slug`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The constant is the MCP server registration key — daemon-wide
config that llm.py looks up in two places. Leading underscore
was a lie about its scope; dropping it makes the cross-module
access legitimate and clears the last two pyright warnings
(reportPrivateUsage) in daemon. _BM25_TOOL_NAME and
_MOVE_TOOL_NAME remain private (only used through the public
fully_qualified_bm25_name / fully_qualified_move_name helpers).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts:
#	src/hooks/test_user_prompt_submit.py
@nickroci nickroci merged commit f11df02 into main May 22, 2026
2 of 3 checks passed
@nickroci nickroci deleted the chore/code-quanlity-2 branch May 27, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant