♻️ SQLite state foundation with migrations and full metadata parity

[Robdel12]

Why

This refactor is about making local TDD state durable and predictable as the CLI grows.

The old JSON files worked for early usage, but they were starting to create complexity around consistency, migration, and multi-process behavior. SQLite gives us a real schema, safer state evolution, and cleaner concurrency boundaries.

What changed

• Switched local TDD/reporter state to a SQLite-backed store with schema migrations
  • src/tdd/state-store/sqlite-store.js
  • migration runner in src/tdd/state-store/migrations.js
• Introduced explicit state-store modes
  • mode: 'write' for the TDD writer path
  • mode: 'read' for routers/services/commands that should never mutate state
  • write APIs now throw in read mode
• Wired callsites to match that contract
  • writer: src/server/handlers/tdd-handler.js
  • readers: dashboard/events/health/static-report/context
• Completed metadata parity on SQLite
  • baseline metadata
  • hotspot metadata
  • region metadata
  • baseline build metadata
• Finalized migration behavior
  • one-time legacy JSON migration when state.db is created for the first time
  • bootstrap migration runs on CLI startup when needed (legacy files present + no state.db)
  • no repeated migration runs once DB exists
• Fixed reset semantics
  • baseline reset now clears persisted state and in-memory TDD runtime caches
• Updated SSE route behavior
  • subscription is read-only
  • snapshots read from a fresh read store to avoid stale/missing-db edge cases
• Added/updated tests for mode contract, migration bootstrap behavior, and updated read/write callsites

Migration behavior (final)

1. On any CLI invocation, we check the current project for legacy state.
2. If .vizzly/ exists, state.db does not exist, and legacy state files exist, we bootstrap migration once.
3. After state.db exists, migration does not run again.

This keeps startup safe and predictable, and avoids hidden write behavior in read-only code paths.

Test plan

• npm run lint
• node --test tests/tdd/state-store-mode.test.js tests/tdd/metadata/baseline-metadata.test.js tests/tdd/metadata/hotspot-metadata.test.js tests/tdd/metadata/region-metadata.test.js tests/server/routers/events.test.js tests/tdd/tdd-service.integration.test.js
• node --test tests/server/handlers/tdd-handler.test.js tests/server/http-server.test.js tests/server/routers/dashboard.test.js tests/tdd/tdd-service.test.js tests/utils/context.test.js

Notes:

• Full npm test in this sandbox still reports existing tests/utils/colors.test.js color assertions (TTY/env behavior). The SQLite/state suites above are green.

[claude]

PR Review: SQLite state foundation with migrations and full metadata parity

Overall this is a solid, well-scoped migration. The schema versioning story is clean, the file/SQLite backends share a coherent interface, and the dependency injection pattern preserves testability throughout. A few things worth addressing before merge:

---

Performance: store opened/closed per HTTP request

In dashboard.js and health.js every route handler calls createStateStore(...) / stateStore.close(). Each open runs applySchemaMigrations (queries schema_migrations) plus two legacy-migration guard reads before doing the actual work. Under active TDD polling this adds meaningful overhead.

The events router shows the right pattern — create one store, subscribe, close on disconnect. Dashboard and health should receive the store as a dependency (injected at server startup) the same way the TDD handler does with injectedStateStore.

---

Per-operation store opening in metadata modules

baseline-metadata.js, hotspot-metadata.js, and region-metadata.js all use:

function withStateStore(workingDir, operation) {
  let store = createStateStore({ workingDir });
  try { return operation(store); }
  finally { store.close(); }
}

Every loadBaselineMetadata / saveBaselineMetadata call opens and closes the DB, triggering migration checks each time. During screenshot processing this is called in a hot path. The store should be injected or cached at the call site rather than created per-operation.

---

Module-level stateEmitters Map never cleaned up

let stateEmitters = new Map();   // src/tdd/state-store.js

Entries are added by workingDir but never removed. In tests (and hypothetically in long-running daemon mode cycling across projects) this leaks EventEmitter instances. Adding a stateEmitters.delete(workingDir) in close() — or exposing a clearEmitter(workingDir) helper for tests — would fix this.

---

findAvailablePort fallback silently returns an occupied port

// src/tdd/server-registry.js
for (let i = 0; i < maxAttempts; i++) { ... }
return startPort;   // ← returned even when startPort was occupied

If all 100 candidates are occupied the function returns startPort unconditionally. The server will then fail to bind with a confusing error. Returning null or throwing here would surface the real problem earlier.

---

Legacy migration marks completion even on partial failure

// src/tdd/state-store.js — maybeMigrateLegacyJson
} catch (error) {
  output.debug?.('state', `legacy report JSON migration skipped: ${error.message}`);
} finally {
  setKey('legacy_json_migrated', '1');    // ← set even on error
}

A transient read error (permissions, disk full, malformed JSON mid-write) silently discards the legacy data permanently. Moving the setKey call to the success path, or only marking it done after a successful replaceReportDataInternal, would avoid silent data loss.

---

createSqliteStateStore — no error handling around DB open

let db = new DatabaseImpl(resolvedDbPath);

A locked, corrupted, or read-only DB throws synchronously here with no try/catch. Since this now runs inside HTTP request handlers (dashboard, health), an unhandled throw will crash the request or the process. At minimum, let it propagate with a clear message; ideally surface it as a 500 with a log line.

---

Minor nits

• buildSummary makes four .filter() passes over the same array; a single reduce would be cleaner for larger comparison sets (not a blocker).
• write() in ServerRegistry uses delete-all + re-insert inside a transaction rather than upsert. Correct, but more disruptive than needed for the register use-case it's adjacent to.
• The engines constraint in better-sqlite3 (20.x || 22.x || ...) should be reflected in the root package.json engines field if it isn't already, so consumers get a clear error on unsupported runtimes.

---

What's working well

• Migration versioning with schema_migrations + idempotent IF NOT EXISTS DDL is solid.
• WAL mode, synchronous = NORMAL, busy_timeout = 5000, and foreign_keys = ON are all the right pragmas for this use-case.
• Transactions wrapping every multi-step mutation — good consistency guarantees.
• ON DELETE CASCADE on comparison_details provides a safety net even though the code also deletes explicitly.
• The dual-backend design (SQLite in production, file in tests) keeps the test suite fast without mocking SQLite.
• close() guards (if (!this.db) return) and silent-swallow in the catch are appropriate for cleanup paths.
• The subscribe/unsubscribe pattern through the module-level EventEmitter correctly decouples DB lifecycle from SSE connection lifecycle.

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - Approved

19 comparisons, no changes detected.

View build

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{feat/sqlite-state · 826ba948}