Documentation tooling, docstring coverage & staleness, Makefile

[aaronsb]

Summary

• Cross-language docstring coverage tool (scripts/development/lint/docstring_coverage.py) — reports documentation coverage across Python (via interrogate or AST fallback), TypeScript (regex scanner), and Rust (regex scanner)
• Docstring staleness tracking (--staleness) — extracts @verified <commit-hash> tags from docstrings and compares against git history to produce a tristate: current, stale (with drift in days), or unverified
• Makefile as unified developer entry point — make help shows all project verbs: testing, static analysis, doc generation, publishing, platform management, container builds
• Reorganized dev scripts — moved static analysis tools to lint/, removed stale venv-based test runners (tests run in Docker now)
• Stale path cleanup — bulk replacement of src/api → api/app and src.api. → api.app. across ~80 files (scripts, tests, CI config, READMEs, guides, ADRs, source code)
• Removed dead test scripts — 10 venv-based shell scripts (in both scripts/ and operator/) that predate containerization and fail immediately
• Docstring pass — verified planning docs against actual code, filled JSDoc gaps
• CLAUDE.md — added Development Practices section

Commits

1. 0ab02c5b — docstring pass: verify plans against code, fill gaps
2. 3f251817 — cross-language docstring coverage tool
3. 53aaeb91 — docstring staleness tracking, reorganize dev scripts
4. 6658bdd4 — Makefile as unified developer entry point
5. 13652aef — fix stale src/api default path in query linter
6. dd6ae29d — fix stale src/api paths in scripts, tests, and docs
7. a3f78034 — fix stale src/api paths in ADRs and remaining docs
8. c8185313 — fix stale src.api module paths to api.app
9. 11b28776 — remove stale venv test scripts, fix Makefile test targets

Test plan

• make lint-queries passes with corrected path
• python3 api/testing/linters/datetime_linter.py scans api/app/ correctly
• python3 scripts/development/lint/docstring_coverage.py produces coverage report
• make help shows all targets with section headers
• make test runs via docker exec (requires running container)
• No remaining src/api or src.api. references outside of archival examples/

[aaronsb]

Code Review: Documentation tooling, docstring coverage & staleness, Makefile

PR size: 1785 additions / 361 deletions across 76 files. Bulk of the additions is the new docstring_coverage.py tool (950 lines) plus the lint_queries.py rename-with-copy and Makefile. The rest is mechanical src/api -> api/app path corrections.

---

What this changes

1. A new cross-language docstring coverage tool with staleness tracking
2. A Makefile as unified developer entry point
3. Reorganization of lint tools from diagnostics/ to lint/
4. Cleanup of 65 files referencing the old src/api Python path
5. Docstring additions to graph_facade.py and SearchBar.tsx
6. CLAUDE.md developer practices section

Assessment: Sound overall, a few items worth addressing

---

Issue: Logic bug in Rust #[cfg(test)] brace tracking

Location: scripts/development/lint/docstring_coverage.py:469-478

Problem: The #[cfg(test)] exclusion uses simple {/} counting on stripped lines. This breaks on:

• Braces inside string literals ("foo { bar }")
• Braces inside comments (// closing })
• Multiple #[cfg(test)] blocks in one file (the brace_depth_at_test variable gets overwritten by the second occurrence)

Additionally, lines 471-473 set brace_depth_at_test = brace_depth on the #[cfg(test)] line itself, but the opening { for the mod tests typically appears on the next line. This means brace_depth_at_test captures the depth before the module's opening brace, so the exit condition on line 477 (brace_depth < brace_depth_at_test) may work by accident (when the closing } decrements below that level) but the intent is fragile.

Why it matters: For a development-only linting tool, this is unlikely to cause real problems in practice -- #[cfg(test)] mod tests { ... } at file scope is the overwhelming norm, and Rust test modules rarely contain string literals with mismatched braces. But the comment says "Track brace depth for #[cfg(test)] exclusion," which implies precision that the implementation does not deliver.

Suggestion: Document the limitation as a comment (e.g., "Note: naive brace counting; sufficient for standard test module patterns"). Or, if precision matters later, switch to only scanning for pub items (which test modules rarely export) and skip the brace tracking entirely.

---

Issue: Quadratic _is_method in Python AST scanner

Location: scripts/development/lint/docstring_coverage.py:167-173

Problem: _is_method() walks the entire AST for every function/async-function node found in the outer ast.walk() loop at line 129. For a file with N nodes and M functions, this is O(N * M). On large files like age_client.py (1000+ lines, many methods), this could add noticeable overhead.

Why it matters: This is a dev tool, not a hot path, so performance is secondary. But the fix is straightforward: build a parent_map dict in a single pass before the main loop (mapping child node IDs to parent types), then do an O(1) lookup per function.

Suggestion: Not blocking, but worth a note. The current code is correct -- just not optimal for very large files.

---

Issue: Prefix match logic in StalenessEntry.status is tautological

Location: scripts/development/lint/docstring_coverage.py:635-638

v = self.item.verified_commit
f = self.file_last_commit or ''
if f.startswith(v) or v.startswith(f[:len(v)]):
    return 'current'

Problem: The second condition v.startswith(f[:len(v)]) is always true when f is at least as long as v, because f[:len(v)] takes the first len(v) characters of f, and then asks "does v start with those?" -- which is equivalent to asking "does f start with v?", the same as the first condition. When f is shorter than v, f[:len(v)] is just f, so the check becomes v.startswith(f) -- which tests the reverse direction (short file hash is prefix of long verified hash).

The intent appears to be: "either hash is a prefix of the other." The first condition handles that (f.startswith(v) works when v is the short hash). The second condition seems meant to handle the reverse case where v is long and f is short, but in practice f comes from git log -1 --format=%H (always 40 chars) while v comes from @verified tags (typically 7 chars). So f.startswith(v) is the only condition that fires in normal use.

Suggestion: Simplify to if f.startswith(v) or v.startswith(f): -- clearer intent, same behavior for all practical inputs.

---

Issue: No __init__.py in scripts/development/lint/

Location: scripts/development/lint/

Problem: The new lint/ directory has no __init__.py. This is fine since both scripts are invoked as top-level scripts (python3 scripts/development/lint/docstring_coverage.py), not as importable modules. But if anyone later tries to import or pytest these, it will fail.

Why it matters: Low priority. Just noting the pattern is consistent -- diagnostics/ also lacks one. Fine for script-only directories.

---

Positive: Clean src/api -> api/app replacement

Spot-checked across ADR-039, ADR-041, ADR-042, ADR-044, ADR-045, ADR-049, ADR-058. All replacements are correct -- the old src/api/... Python source paths are now api/app/..., matching the actual directory structure. The remaining src/api references in the codebase are all legitimate TypeScript paths (cli/src/api/client.ts, web/src/api/client.ts), which are real file paths that should not be changed. Zero false positives found in the ADR corrections.

---

Positive: Makefile is well-structured

The Makefile at 107 lines is clean and conventional:

• .PHONY declarations cover all targets
• ##@ section headers with awk-based help target is the standard GNU Make pattern
• All recipes use @ to suppress command echo
• The SCRIPTS variable avoids path repetition
• lint correctly depends on lint-queries coverage so make lint runs both

One minor note: test-unit and test-api use docker exec kg-api-dev directly, which assumes the container name. This is fine given operator.sh controls the container naming, but it's worth noting these targets require the platform running (./operator.sh start). The diagnose target already implies this pattern, so it's consistent.

---

Positive: Good separation of concerns in the coverage tool

The architecture of docstring_coverage.py follows a clear scanner-per-language pattern. Despite being 950 lines in a single file, the internal structure is well-organized:

• Shared data structures (DocItem, FileResult, LanguageResult) at the top
• Shared @verified extraction (_extract_verified) used by all scanners
• Three independent language scanners, each following the same pattern
• Git helpers isolated from scanners
• Staleness analysis as a separate layer that consumes scanner output

Each scanner could be extracted into its own module, but at 950 lines with clear section markers and no cross-scanner coupling, a single file is reasonable for a dev tool. The file reads well top-to-bottom.

---

Positive: CLAUDE.md additions are practical

The new "Development Practices" section covers the right things for an agent-first codebase: docstring conventions, testing commands (with the important note about kg-api-dev container), tool locations, and platform management. The distinction between lint/ (no running platform) and diagnostics/ (needs running platform) is documented in the directory tree, which clarifies the reorganization rationale.

---

Summary

Category	Finding
Bug (minor)	Rust #[cfg(test)] brace tracking is fragile for edge cases
Clarity	StalenessEntry.status prefix match has a tautological branch
Performance (minor)	_is_method is O(N*M) per file, could be O(N+M)
No issues	src/api path replacements are all correct
No issues	Makefile conventions are solid
No issues	Tool architecture, CLAUDE.md, SearchBar docstrings

None of these are merge blockers. The cfg(test) brace tracking and the prefix match clarity are the most worth addressing.