refactor(store): single module for review-history SQL

[ajianaz]

What

Adds engine::review_store, the single module that owns SQL for reviews / findings / finding_events (schema stays in index/schema.rs, unchanged). It replaces engine::db_writer.

Interface:

• ReviewStore<'a> over a borrowed &Connection (testable with in-memory SQLite): record_review, resolve_stale, list_findings(&FindingFilter), stats, finding_status, dismiss(id, reason), reopen(id) (both return Transition::{NotFound, Unchanged, Applied}), reviews_for_root.
• Connection acquisition: open_read() / open_write() (+ open_read_at(path)), all Result.
• Policy in one place: persist_review_best_effort(&ReviewRecord) / persist_on(&conn, ..). It records the review and auto-resolves stale findings and never fails; errors are logged at debug. review.rs and scan.rs (3 call sites) now make one call instead of an open/save/fingerprint/resolve block each.
• load_debt_rows(project_root) feeds debt_tracker::snapshots_from_rows (pure conversion); debt_tracker no longer has SQL.

Error policy: store methods surface errors to callers that can act on them (the cora findings CLI). Only the review/scan persistence path and the read-only debt report swallow errors, each through one named function.

SQL sites migrated (checklist for reviewers)

Found via grep -rnE 'findings|finding_events|reviews' src filtered to SQL verbs; schema.rs (DDL + its own migration tests) is intentionally left.

Before	Now
commands/findings.rs list query (JOIN + dynamic filters)	ReviewStore::list_findings
commands/findings.rs 5x count(*) (stats)	ReviewStore::stats
commands/findings.rs dismiss: SELECT status, UPDATE, INSERT event	ReviewStore::dismiss
commands/findings.rs reopen: SELECT status, UPDATE, INSERT event	ReviewStore::reopen
engine/db_writer.rs INSERT reviews / findings / finding_events('opened')	ReviewStore::record_review
engine/db_writer.rs SELECT stale + UPDATE resolved + INSERT 'auto_resolved'	ReviewStore::resolve_stale
engine/db_writer.rs open_db, open_db_for_read, open_db_for_write (Option)	open_read / open_write (Result)
engine/debt_tracker.rs projects lookup + reviews SELECT + 2x GROUP BY findings	ReviewStore::reviews_for_root
src/mcp/tools.rs	none: it has no SQL against these tables (debt there reads the JSON snapshots via debt_tracker::load_snapshots)

Why

Closes #571. Review-history SQL was spread over four files, open_db_* swallowed errors via Option, and the status transitions could only be reached through the CLI against the real $HOME database, so they were untested. Now every path is covered by in-memory tests and the best-effort-vs-surface decision is explicit.

Behavior changes (none user-visible on success):

• record_review and status transitions now run in a transaction (a mid-way failure no longer leaves a review row without findings, or a status change without its event). resolve_stale is also a single transaction.
• findings list/stats now propagate a SQL/row error instead of silently dropping rows / showing 0. A missing or unopenable DB still prints the same "could not open cora.db" message with exit 1.
• CLI output, exit codes and DB contents for the same inputs are unchanged (smoke-tested, see below). Dismissing an already-dismissed finding still re-records an event, as before.
• No CHANGELOG edit (not user-visible).

Left out / noticed: cora findings list --severity X never matches anything. Severities are stored lowercase (Severity::to_string) but the filter upper-cases its argument, and the list renderer's CRITICAL/MAJOR colour match is likewise dead. This is pre-existing; I kept it so the refactor stays behavior-preserving and pinned it in a test with a comment. Worth a separate fix.

Testing

• cargo fmt, cargo clippy --all-targets --features tree-sitter -- -D warnings: clean.
• cargo test --features tree-sitter -- --skip index:: --skip watch::: all pass (924 unit + integration). In my sandbox the index:: / commands::watch tests hang (not touched by this change), so I skipped them.
• New in-memory tests (engine::review_store::tests, 13): record + findings + opened events round trip, atomic rollback on failure, list filters (open/all/file/limit), dismiss, dismiss again, reopen, already-open no-op, not-found, reopen after auto-resolve, auto_resolved event and project isolation, stats, debt reads (open-only breakdown, unknown project), best-effort persistence (success + auto-resolve, and broken-schema returns default without error), read-only open of a missing DB errors. Plus debt_tracker::snapshots_from_store_rows.
• Smoke test with the debug binary in a temp HOME (DB created by findings dismiss migrations, then seeded with sqlite3):
  • findings list on no DB: Error: could not open cora.db, exit 1
  • list shows #2 minor src/b.rs :9 | Meh [OPEN] / #1 critical ...
  • dismiss 1 --reason wontfix -> Finding #1 dismissed. exit 0; reopen 2 -> already open. exit 0; reopen 1 -> reopened.; dismiss 42 -> not found. exit 1
  • finding_events: dismissed|wontfix, reopened|Manually reopened via CLI, dismissed|Manually dismissed via CLI
  • stats / list --all --json output as expected.

Closes #571

🤖 Generated with Claude Code