RFC: Address codebase anti-patterns and encapsulation issues #18

Summary

The codebase audit identified several anti-patterns and encapsulation violations that should be addressed to improve maintainability. The most critical issue is that storage._connect() (a private method) is accessed directly from 4 external modules, creating tight coupling.

Problem / Motivation

During /audit-codebase review, these issues were discovered:

P1: Critical

1. Encapsulation violation: queries.py and patterns.py directly call storage._connect(), bypassing the SQLiteStorage public API
2. Hardcoded version: "0.1.0" is hardcoded in server.py:73 rather than reading from package metadata

P2: Code Quality

3. Import aliasing: do_* prefix pattern in server.py adds noise (lines 22-30)
4. Monolithic format_output(): 80-line if-elif chain in cli.py:18-95 violates Open/Closed principle
5. Duplicated query building: Same WHERE clause construction pattern repeated 4 times in queries.py
6. Empty __init__.py: No public API definition, no __all__ exports

P3: Minor

7. Schema migration: SCHEMA_VERSION=1 defined but no migration logic
8. Empty pass branch: Unnecessary code in ingest.py:95-98
9. Timestamp handling: Timezone stripping undocumented

P4: Polish

10. CLI help: Subcommand descriptions not visible in main --help

Context

• Discovered during: /audit-codebase command
• Relevant files: storage.py (588 lines), queries.py (431 lines), patterns.py (369 lines), cli.py (259 lines), server.py (286 lines)
• Related PRs: Phase 7: Polish and CLI tests #16 (Phase 7: Polish)

Proposed Solution

Phase A: Encapsulation Fix (P1)

1. Add SQLiteStorage.execute_query() public method
2. Migrate all _connect() usage in queries.py and patterns.py
3. Read version from importlib.metadata

Phase B: Query Builder (P2)

4. Extract build_where_clause() helper to reduce duplication
5. Define public API in __init__.py with __all__

Phase C: CLI Refactor (P2)

6. Refactor format_output() to use formatter registry pattern
7. Remove do_* import aliasing in server.py

Phase D: Polish (P3-P4)

8. Add schema migration infrastructure
9. Remove empty pass branch
10. Document timestamp handling
11. Improve CLI help text

Assumptions

Open Questions

1. Should execute_query() be the only public query method, or add specialized methods per use case?
2. For schema migrations - use simple version check or full migration framework?

Actionable Requirements

Implementation Checklist

• Add SQLiteStorage.execute_query() public method
• Migrate queries.py to use public API (4 locations)
• Migrate patterns.py to use public API (4 locations)
• Extract build_where_clause() helper
• Read version from package metadata
• Define __all__ in __init__.py
• Refactor format_output() to use registry pattern
• Remove do_* import aliasing in server.py
• Add basic schema migration support
• Remove empty pass branch in ingest.py
• Add timestamp handling documentation
• Improve CLI help text with epilog
• Run make check to verify no regressions