diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 2bab231..b17f2fd 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -96,7 +96,11 @@ Plans: 2. A code generator reads Go event constants from `backend/events/events.go` and produces `frontend/src/events.ts`, wired into `go generate` and the pre-commit hook — adding an event in Go without regenerating TypeScript fails the hook 3. Queue batch lookups in `persistence.go` use `sqlc.slice()` for IN clauses where sqlc supports it, replacing `fmt.Sprintf` placeholder construction 4. Every hand-crafted SQL statement that intentionally bypasses sqlc has a `// SAFETY:` comment explaining why (batch INSERT, dynamic IN clauses, etc.) -**Plans:** TBD +**Plans:** 3 plans +Plans: +- [ ] 06-01-PLAN.md — Create track_metadata VIEW and consolidate search queries +- [ ] 06-02-PLAN.md — Event codegen tool (Go→TypeScript) and pre-commit hook wiring +- [ ] 06-03-PLAN.md — Migrate lookupChunk to sqlc.slice() and add SAFETY comments to all hand-crafted SQL ### Phase 7: Backend Performance **Goal:** Queue mutations and library loading are fast — single-track queue changes are O(1) instead of O(n), and the library doesn't block startup with a full data fetch @@ -128,7 +132,7 @@ Plans: | 3. Test Infrastructure | 0/1 | Planned | — | | 4. Queue, Config & Player Tests | 0/2 | Planned | — | | 5. Database & Library Tests | 0/2 | Planned | — | -| 6. SQL Consolidation & Code Quality | 0/? | Not started | — | +| 6. SQL Consolidation & Code Quality | 0/3 | Planned | — | | 7. Backend Performance | 0/? | Not started | — | | 8. Frontend Performance & UX | 0/? | Not started | — | diff --git a/.planning/phases/06-sql-consolidation-code-quality/06-01-PLAN.md b/.planning/phases/06-sql-consolidation-code-quality/06-01-PLAN.md new file mode 100644 index 0000000..feda53a --- /dev/null +++ b/.planning/phases/06-sql-consolidation-code-quality/06-01-PLAN.md @@ -0,0 +1,241 @@ +--- +phase: 06-sql-consolidation-code-quality +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - backend/database/database.go + - backend/database/search.go + - backend/database/sql/schemas/track_metadata_view.sql + - backend/database/sql/sqlcgen/models.go +autonomous: true +requirements: [QUAL-01] + +must_haves: + truths: + - "All FTS5 search queries (SearchFTS, SearchFTSByFilename, SearchFTSTracks) use the track_metadata VIEW instead of inline 5-table JOINs" + - "RebuildSearchIndex SELECTs from track_metadata VIEW instead of duplicating the JOIN" + - "Migration 4 creates the track_metadata VIEW for existing databases" + - "sqlc generate succeeds with the VIEW schema file and produces updated models" + - "Existing FTS5 search tests (15 tests) pass unchanged after VIEW consolidation" + artifacts: + - path: "backend/database/sql/schemas/track_metadata_view.sql" + provides: "VIEW definition for sqlc schema awareness" + contains: "CREATE VIEW IF NOT EXISTS track_metadata" + - path: "backend/database/database.go" + provides: "Migration 4 creating VIEW for existing databases" + contains: "migration4TrackMetadataView" + - path: "backend/database/search.go" + provides: "Consolidated search queries using VIEW" + contains: "track_metadata" + key_links: + - from: "backend/database/search.go" + to: "track_metadata VIEW" + via: "JOIN track_metadata tm ON tm.id = si.rowid" + pattern: "JOIN track_metadata" + - from: "backend/database/database.go" + to: "track_metadata VIEW" + via: "migration 4 CREATE VIEW" + pattern: "CREATE VIEW IF NOT EXISTS track_metadata" +--- + + +Consolidate the duplicated 5-table FTS5 JOIN pattern into a single SQLite VIEW named `track_metadata`, and update all search queries to use it. + +Purpose: Eliminate 4+ copies of the same complex JOIN across search.go and database.go. A single VIEW is the source of truth for audio file metadata JOINs — changes to the schema only need updating in one place. + +Output: Migration 4 (VIEW creation), sqlc schema file, consolidated search.go queries, updated sqlc-generated code. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/06-sql-consolidation-code-quality/06-RESEARCH.md + +@backend/database/database.go +@backend/database/search.go +@backend/database/sql/schemas/ +@backend/database/sqlc.yaml + + + + +From backend/database/database.go: +- Migrations are Go functions registered in a slice, applied sequentially by PRAGMA user_version +- Pattern: `migration2BasenameAndFTS`, `migration3UniqueArtistCreditArtist` — each bumps user_version +- Current highest migration: 3 (user_version=3) +- `//go:generate go tool sqlc generate` directive at line 21 + +From backend/database/search.go: +- `func (d *DB) SearchFTS(query string, limit int) ([]SearchResult, error)` — line 22 +- `func (d *DB) SearchFTSByFilename(query string, limit int) ([]SearchResult, error)` — line 72 +- `func (d *DB) RebuildSearchIndex() error` — line 161 +- `func (d *DB) SearchFTSTracks(query string, limit int) ([]SearchTrackResult, error)` — line 222 +- All 4 functions contain inline 5-table JOINs (audio_files → recordings → artist_credit → release_group_recordings subquery → release_groups) + +From backend/database/sql/schemas/ directory: +- Schema files sorted alphabetically; sqlc processes them in filesystem order +- Tables: artist_credit.sql, artists.sql, audio_files.sql, cover_art.sql, file_types.sql, genres.sql, recordings.sql, release_group_recordings.sql, release_groups.sql, etc. +- `track_metadata_view.sql` will sort after all table schemas (t > all existing prefixes) + + + + + + + Task 1: Create track_metadata VIEW schema and migration + + backend/database/sql/schemas/track_metadata_view.sql + backend/database/database.go + + + 1. Create `backend/database/sql/schemas/track_metadata_view.sql` with the VIEW definition: + ```sql + CREATE VIEW IF NOT EXISTS track_metadata AS + SELECT + af.id, + af.file_path, + af.length_milliseconds, + COALESCE(r.name, '') AS title, + COALESCE(ac.text, '') AS artist_name, + r.track_number, + r.disc_number, + COALESCE(rg.name, '') AS album, + CAST(COALESCE( + (SELECT GROUP_CONCAT(g.name, '||') + FROM recording_genres rg_sub + JOIN genres g ON rg_sub.genre_id = g.id + WHERE rg_sub.recording_id = r.id), + '' + ) AS TEXT) AS genre, + COALESCE(r.year, 0) AS year, + COALESCE(r.composer, '') AS composer, + COALESCE(ft.extension, '') AS file_type, + af.sample_rate, + af.bit_depth, + af.channels, + af.bitrate, + af.file_size + FROM audio_files af + LEFT JOIN recordings r ON af.recording_id = r.id + LEFT JOIN artist_credit ac ON r.artist_credit_id = ac.id + LEFT JOIN ( + SELECT recording_id, + MIN(release_group_id) AS release_group_id + FROM release_group_recordings + GROUP BY recording_id + ) rgr ON r.id = rgr.recording_id + LEFT JOIN release_groups rg ON rgr.release_group_id = rg.id + LEFT JOIN file_types ft ON af.file_type_id = ft.id; + ``` + + 2. In `backend/database/database.go`, add migration 4 (`migration4TrackMetadataView`): + - The migration function should execute `CREATE VIEW IF NOT EXISTS track_metadata AS ...` (same SQL as the schema file) + - Register it in the migrations slice after migration 3 + - Follow the existing migration function pattern (takes `*sql.DB` and `context.Context`, returns `error`) + + 3. Run `go tool sqlc generate` from `backend/database/` to regenerate code with VIEW awareness. + + 4. **CRITICAL:** Do NOT change `migration2BasenameAndFTS` to use the VIEW — migration 2 runs before migration 4 for databases upgrading from version 1. The inline JOIN in migration 2 must stay as-is. + + 5. Verify sqlc generate succeeds without errors. + + + cd backend/database && go tool sqlc generate && echo "sqlc OK" + + + - `track_metadata_view.sql` exists in schemas directory with the VIEW definition + - Migration 4 registered in database.go, creates the VIEW for existing databases + - `sqlc generate` succeeds and recognizes the VIEW + - migration2 code is unchanged (still uses inline JOIN) + + + + + Task 2: Consolidate search queries to use track_metadata VIEW + + backend/database/search.go + + + Update all 4 search functions in `search.go` to use the `track_metadata` VIEW instead of inline JOINs: + + 1. **SearchFTS** (line ~22): Replace the inline 5-table JOIN with: + ```sql + SELECT tm.file_path, tm.length_milliseconds, tm.title, tm.artist_name, tm.album + FROM search_index si + JOIN track_metadata tm ON tm.id = si.rowid + WHERE search_index MATCH ? + ORDER BY rank + LIMIT ? + ``` + Only select the 5 columns the function actually uses — SQLite optimizes away unused VIEW columns. + + 2. **SearchFTSByFilename** (line ~72): Same pattern as SearchFTS but with the filename-specific FTS query logic. Replace the inline JOIN with `JOIN track_metadata tm ON tm.id = si.rowid`. Keep the same column selection. + + 3. **SearchFTSTracks** (line ~222): Replace the inline 6-table JOIN (includes file_types) with the VIEW. The VIEW already includes `file_type` (from the file_types JOIN), so this becomes simpler. Select the columns needed by `SearchTrackResult`: file_path, length_milliseconds, title, artist_name, album, track_number, disc_number, genre, year, composer, file_type, sample_rate, bit_depth, channels, bitrate, file_size. + + 4. **RebuildSearchIndex** (line ~161): Replace the inline JOIN with: + ```sql + INSERT INTO search_index(rowid, file_path, title, artist, album) + SELECT id, file_path, title, artist_name, album + FROM track_metadata + ``` + + **Preserve:** All FTS5 MATCH syntax, ORDER BY rank, LIMIT clauses, error handling, row scanning, and function signatures remain identical. Only the FROM/JOIN clauses change. + + **Do NOT touch:** `InsertSearchIndex`, `DeleteSearchIndex`, `ClearSearchIndex` — these are single-row FTS5 operations that don't use JOINs. + + + cd backend && go test -tags webkit2_41 -race -count=1 -timeout 60s ./database/... + + + - SearchFTS, SearchFTSByFilename, SearchFTSTracks, and RebuildSearchIndex all use `track_metadata` VIEW + - No inline 5-table JOIN patterns remain in search.go (except in comments) + - All 15 existing FTS5 search tests pass with -race + - Function signatures unchanged — callers are unaffected + + + + + + +```bash +# 1. Verify sqlc generates cleanly +cd backend/database && go tool sqlc generate + +# 2. Verify all database tests pass (15 search tests + migrations) +cd backend && go test -tags webkit2_41 -race -count=1 -timeout 60s ./database/... + +# 3. Verify no inline JOIN duplication remains in search.go +grep -c "LEFT JOIN recordings" backend/database/search.go # Should be 0 + +# 4. Verify VIEW is referenced +grep -c "track_metadata" backend/database/search.go # Should be 4+ + +# 5. Verify migration2 is unchanged +grep "LEFT JOIN recordings" backend/database/database.go # Should still exist (migration2 only) + +# 6. Full build check +go build -tags webkit2_41 ./... +``` + + + +- The duplicated 5-table JOIN pattern is eliminated from search.go (0 copies remain) +- All search queries use the `track_metadata` VIEW +- Migration 4 creates the VIEW for existing databases +- sqlc schema file enables future sqlc queries against the VIEW +- All 15 existing database tests pass with -race +- Full project builds without errors + + + +After completion, create `.planning/phases/06-sql-consolidation-code-quality/06-01-SUMMARY.md` + diff --git a/.planning/phases/06-sql-consolidation-code-quality/06-02-PLAN.md b/.planning/phases/06-sql-consolidation-code-quality/06-02-PLAN.md new file mode 100644 index 0000000..a1462e6 --- /dev/null +++ b/.planning/phases/06-sql-consolidation-code-quality/06-02-PLAN.md @@ -0,0 +1,253 @@ +--- +phase: 06-sql-consolidation-code-quality +plan: 02 +type: execute +wave: 1 +depends_on: [] +files_modified: + - backend/events/events.go + - backend/events/cmd/genevents/main.go + - frontend/src/events.ts + - lefthook.yml +autonomous: true +requirements: [QUAL-02] + +must_haves: + truths: + - "Running `go generate ./backend/events/...` produces frontend/src/events.ts that exactly matches the Go constants" + - "The generated events.ts includes LibraryConfigChanged (currently missing from hand-maintained TS file)" + - "The codegen-check pre-commit hook detects stale events.ts and fails" + - "Output is deterministic — running the generator twice produces identical output" + artifacts: + - path: "backend/events/cmd/genevents/main.go" + provides: "Go→TypeScript event constant generator" + contains: "go/ast" + - path: "backend/events/events.go" + provides: "go:generate directive for event codegen" + contains: "go:generate" + - path: "frontend/src/events.ts" + provides: "Generated TypeScript event constants" + contains: "LibraryConfigChanged" + key_links: + - from: "backend/events/events.go" + to: "frontend/src/events.ts" + via: "go:generate directive running genevents" + pattern: "go:generate go run" + - from: "lefthook.yml" + to: "go generate" + via: "codegen-check pre-commit hook" + pattern: "go generate" +--- + + +Build a Go code generator that reads event constants from `backend/events/events.go` using `go/ast` and produces `frontend/src/events.ts`, then wire it into `go generate` and the pre-commit hook. + +Purpose: Eliminate manual synchronization of event names between Go and TypeScript. The generator automatically catches drift (like the missing `LibraryConfigChanged`) and the pre-commit hook prevents stale files from being committed. + +Output: Generator tool, `//go:generate` directive, updated events.ts with missing constant, working codegen-check hook. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/06-sql-consolidation-code-quality/06-RESEARCH.md + +@backend/events/events.go +@frontend/src/events.ts +@lefthook.yml + + + + +From backend/events/events.go (21 constants in 5 groups): +```go +// Playback events (backend → frontend push). +const ( + PlaybackStateChanged = "PlaybackStateChanged" + PlaybackFinished = "PlaybackFinished" + TrackChanged = "TrackChanged" + SeekFailed = "SeekFailed" + VolumeChanged = "VolumeChanged" +) +// Queue events (backend → frontend push). +const ( + QueueChanged = "QueueChanged" + QueueIndexChanged = "QueueIndexChanged" + QueueModeChanged = "QueueModeChanged" + QueueTracksModified = "QueueTracksModified" +) +// Config events. +const ( + LibraryConfigChanged = "LibraryConfigChanged" // <-- MISSING from TS + ThemeConfigChanged = "ThemeConfigChanged" + TrackListConfigChanged = "TrackListConfigChanged" + FavoritesConfigChanged = "FavoritesConfigChanged" +) +// Playlist events. +const ( + PlaylistCreated = "PlaylistCreated" + PlaylistDeleted = "PlaylistDeleted" + PlaylistRenamed = "PlaylistRenamed" + PlaylistTracksChanged = "PlaylistTracksChanged" + PlaylistsRestored = "PlaylistsRestored" + DefaultPlaylistChanged = "DefaultPlaylistChanged" +) +// Library events. +const ( + LibraryScanStarted = "LibraryScanStarted" + LibraryScanComplete = "LibraryScanComplete" +) +``` + +From frontend/src/events.ts (20 constants — missing LibraryConfigChanged): +- Format: `export const Events = { ... } as const;` +- Followed by: `export type EventName = (typeof Events)[keyof typeof Events];` +- Comment groups match Go groups (Playback, Queue, Playlist, Config, Library) + +From lefthook.yml: +- codegen-check hook runs `go generate ./...` then checks `git diff --name-only` +- Hook currently hangs per STATE.md but research shows `go generate ./...` now completes in <1s + +Existing go:generate directives: +- `backend/app.go:4` — `//go:generate go tool templ generate` +- `backend/database/database.go:21` — `//go:generate go tool sqlc generate` + + + + + + + Task 1: Create event codegen tool + + backend/events/cmd/genevents/main.go + backend/events/events.go + + + 1. Create `backend/events/cmd/genevents/main.go` — a standalone Go program (package main) that: + - Uses `go/ast`, `go/parser`, `go/token` to parse `events.go` in the same directory as the source + - Accepts a `-source` flag (path to events.go, default: the events.go file relative to the generator location) and an `-output` flag (path to output .ts file) + - Walks the AST in declaration order (NOT map iteration — deterministic output is critical) + - For each `const` block: extracts the doc comment above the block (e.g., "// Playback events (backend → frontend push).") and each constant name + string value + - Generates TypeScript output matching the current `events.ts` format exactly: + ```typescript + // Code generated by genevents from backend/events/events.go. DO NOT EDIT. + + export const Events = { + // Playback events (backend → frontend push) + PlaybackStateChanged: "PlaybackStateChanged", + ... + } as const; + + export type EventName = (typeof Events)[keyof typeof Events]; + ``` + - Preserves comment group separation with blank lines between groups + - Strips the trailing period from Go doc comments (Go convention) for TypeScript comments + - Writes output atomically (write to temp file, then rename) + + 2. Add `//go:generate` directive to `backend/events/events.go`: + ```go + //go:generate go run ./cmd/genevents -source events.go -output ../../frontend/src/events.ts + ``` + Place it after the package doc comment and before the first const block. Use a relative path from the events package directory to the frontend output. + + 3. Run `go generate ./backend/events/...` and verify the output matches the expected format. + + 4. Verify the generated events.ts now includes `LibraryConfigChanged` (the constant missing from the hand-maintained file). + + **Key constraint:** AST iteration must be in source declaration order (iterate `f.Decls` directly, NOT collect into a map). This ensures deterministic output so the codegen-check hook doesn't produce false diffs. + + + go generate ./backend/events/... && diff <(cat frontend/src/events.ts) <(go run ./backend/events/cmd/genevents -source backend/events/events.go -output /dev/stdout) && echo "Deterministic OK" && grep -q "LibraryConfigChanged" frontend/src/events.ts && echo "Missing constant fixed" + + + - Generator exists at backend/events/cmd/genevents/main.go + - `//go:generate` directive added to events.go + - Running `go generate ./backend/events/...` produces valid events.ts + - Output includes all 21 constants (including LibraryConfigChanged) + - Output is deterministic (running twice produces identical files) + - Comment groups match Go source ordering + + + + + Task 2: Wire codegen-check pre-commit hook + + lefthook.yml + + + 1. The existing `codegen-check` hook in `lefthook.yml` already runs `go generate ./...` and diffs. Per research, `go generate ./...` now completes in <1 second (previous hanging appears resolved). The hook structure should work as-is with the new event generator wired in. + + 2. Test the hook end-to-end: + - Run `go generate ./...` and verify it completes quickly (<5 seconds) + - Verify no unstaged changes exist after generation (all generated code is up-to-date) + - Manually introduce a drift: add a test constant to events.go, verify `go generate` updates events.ts, then verify the hook would detect the diff + + 3. If the hook still hangs (unlikely per research): narrow the `codegen-check` glob to only trigger on event-related files, or split into a separate event-specific check. Update lefthook.yml accordingly. + + 4. Run the full pre-commit hook to verify all hooks pass: + ```bash + LEFTHOOK=1 lefthook run pre-commit + ``` + Note: If the hook takes >10 seconds, investigate and optimize. Expected: <5s total. + + 5. Clean up any test changes (remove test constant if added). + + **Important:** The hook runs `go generate ./...` which triggers ALL generators (templ, sqlc, events). This is the correct behavior — it ensures all generated code is fresh. The <1s completion time makes this acceptable. + + + go generate ./... && test -z "$(git diff --name-only)" && echo "codegen-check would pass" + + + - `go generate ./...` completes in <5 seconds + - codegen-check hook detects stale events.ts (adding Go constant without regenerating TS fails the hook) + - All existing pre-commit hooks still pass + - No leftover test changes in the working tree + + + + + + +```bash +# 1. Generator produces valid output +go generate ./backend/events/... + +# 2. Output includes all 21 constants +grep -c ":" frontend/src/events.ts # Should be 21+ (constants + type line) + +# 3. LibraryConfigChanged is present +grep "LibraryConfigChanged" frontend/src/events.ts + +# 4. Deterministic output +go generate ./backend/events/... +git diff --name-only # Should be empty (no changes on second run) + +# 5. Full generate works +go generate ./... + +# 6. Frontend typecheck passes with new events.ts +cd frontend && ./node_modules/.bin/tsc --noEmit + +# 7. Full build +go build -tags webkit2_41 ./... +``` + + + +- Event codegen tool parses Go constants and generates matching TypeScript +- LibraryConfigChanged gap is automatically fixed +- `go generate` directive wired into events.go +- codegen-check hook works end-to-end (detects drift, passes when clean) +- Frontend TypeScript compiles with generated events.ts +- Output is deterministic across runs + + + +After completion, create `.planning/phases/06-sql-consolidation-code-quality/06-02-SUMMARY.md` + diff --git a/.planning/phases/06-sql-consolidation-code-quality/06-03-PLAN.md b/.planning/phases/06-sql-consolidation-code-quality/06-03-PLAN.md new file mode 100644 index 0000000..fdea4f2 --- /dev/null +++ b/.planning/phases/06-sql-consolidation-code-quality/06-03-PLAN.md @@ -0,0 +1,317 @@ +--- +phase: 06-sql-consolidation-code-quality +plan: 03 +type: execute +wave: 2 +depends_on: [06-01] +files_modified: + - backend/database/sql/queries/audio_files.sql + - backend/database/sql/sqlcgen/audio_files.sql.go + - backend/database/sql/sqlcgen/models.go + - backend/queue/persistence.go + - backend/database/search.go + - backend/library/library.go + - backend/library/rescan.go +autonomous: true +requirements: [QUAL-03, QUAL-04] + +must_haves: + truths: + - "lookupChunk no longer uses fmt.Sprintf for IN clause construction — it calls a sqlc-generated query via the track_metadata VIEW" + - "Every hand-crafted SQL statement that bypasses sqlc has a // SAFETY: comment with two parts: why sqlc can't handle it AND what makes it safe" + - "All 12 identified hand-crafted SQL statements have SAFETY comments" + - "Queue tests and database tests pass unchanged after the migration" + artifacts: + - path: "backend/database/sql/queries/audio_files.sql" + provides: "sqlc query for batch track metadata lookup" + contains: "LookupTrackMetaByPaths" + - path: "backend/queue/persistence.go" + provides: "Updated lookupChunk using sqlc-generated query" + contains: "SAFETY" + - path: "backend/database/search.go" + provides: "SAFETY comments on all FTS5 queries" + contains: "SAFETY" + - path: "backend/library/library.go" + provides: "SAFETY comments on FTS5 insert/delete operations" + contains: "SAFETY" + - path: "backend/library/rescan.go" + provides: "SAFETY comments on FTS5 delete operation" + contains: "SAFETY" + key_links: + - from: "backend/queue/persistence.go" + to: "backend/database/sql/sqlcgen/" + via: "sqlc-generated LookupTrackMetaByPaths query" + pattern: "LookupTrackMetaByPaths" + - from: "backend/database/sql/queries/audio_files.sql" + to: "track_metadata VIEW" + via: "SELECT FROM track_metadata WHERE file_path IN (sqlc.slice)" + pattern: "sqlc.slice" +--- + + +Migrate the queue's `lookupChunk` from hand-crafted SQL with `fmt.Sprintf` to a sqlc-generated query using the `track_metadata` VIEW and `sqlc.slice()`, then add `// SAFETY:` comments to all remaining hand-crafted SQL statements. + +Purpose: Replace the only hand-crafted SQL that CAN be migrated to sqlc (lookupChunk), and document all intentional exceptions so future maintainers understand why each hand-crafted statement exists. + +Output: sqlc query file, regenerated code, updated persistence.go, SAFETY comments on all 12 hand-crafted SQL statements. + + + +@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md +@/home/caleb/.config/opencode/get-shit-done/templates/summary.md + + + +@.planning/PROJECT.md +@.planning/ROADMAP.md +@.planning/STATE.md +@.planning/phases/06-sql-consolidation-code-quality/06-RESEARCH.md +@.planning/phases/06-sql-consolidation-code-quality/06-01-SUMMARY.md + +@backend/queue/persistence.go +@backend/database/search.go +@backend/library/library.go +@backend/library/rescan.go +@backend/database/sql/queries/audio_files.sql +@backend/database/sqlc.yaml + + + + +From backend/queue/persistence.go: +```go +type trackMeta struct { + AudioFileID int64 + FilePath string + Title string + Artist string +} + +// lookupTrackMetaBatch — chunks at maxSQLiteVars (900) and calls lookupChunk per chunk +// lookupChunk — hand-crafted SELECT with fmt.Sprintf IN clause (TARGET for sqlc migration) +// insertTrackBatch — multi-row INSERT with variable VALUES count (STAYS hand-crafted) +const maxSQLiteVars = 900 +``` + +From backend/database/search.go (after Plan 01 consolidation): +- SearchFTS — FTS5 MATCH query using track_metadata VIEW +- SearchFTSByFilename — FTS5 MATCH query using track_metadata VIEW +- InsertSearchIndex — single-row INSERT INTO search_index +- DeleteSearchIndex — DELETE FROM search_index WHERE rowid = ? +- ClearSearchIndex — DELETE FROM search_index +- RebuildSearchIndex — INSERT INTO search_index SELECT FROM track_metadata +- SearchFTSTracks — FTS5 MATCH query using track_metadata VIEW + +From backend/library/library.go: +- commitNewAudioFile (~line 798) — INSERT INTO search_index VALUES (single row) +- updateAudioFileMetadata (~line 879) — DELETE FROM search_index WHERE rowid = ? +- updateAudioFileMetadata (~line 893) — INSERT INTO search_index VALUES (single row) + +From backend/library/rescan.go: +- clearAllLibraryData (~line 165) — DELETE FROM search_index + +Complete SAFETY comment inventory (12 statements): +| # | File | Function | Operation | Why hand-crafted | +|---|------|----------|-----------|-----------------| +| 1 | search.go | SearchFTS | FTS5 MATCH | FTS5 unsupported by sqlc | +| 2 | search.go | SearchFTSByFilename | FTS5 MATCH | FTS5 unsupported by sqlc | +| 3 | search.go | InsertSearchIndex | FTS5 INSERT | FTS5 virtual table | +| 4 | search.go | DeleteSearchIndex | FTS5 DELETE | FTS5 virtual table | +| 5 | search.go | ClearSearchIndex | FTS5 DELETE | FTS5 virtual table | +| 6 | search.go | RebuildSearchIndex | FTS5 INSERT SELECT | FTS5 virtual table | +| 7 | search.go | SearchFTSTracks | FTS5 MATCH | FTS5 unsupported by sqlc | +| 8 | library.go | commitNewAudioFile | FTS5 INSERT | FTS5 virtual table | +| 9 | library.go | updateAudioFileMetadata | FTS5 DELETE | FTS5 virtual table | +| 10 | library.go | updateAudioFileMetadata | FTS5 INSERT | FTS5 virtual table | +| 11 | rescan.go | clearAllLibraryData | FTS5 DELETE | FTS5 virtual table | +| 12 | persistence.go | insertTrackBatch | Variable-count multi-row INSERT | sqlc can't generate variable-length batch INSERTs | + + + + + + + Task 1: Migrate lookupChunk to sqlc with sqlc.slice() + + backend/database/sql/queries/audio_files.sql + backend/database/sql/sqlcgen/audio_files.sql.go + backend/database/sql/sqlcgen/models.go + backend/queue/persistence.go + + + 1. Add the sqlc query to `backend/database/sql/queries/audio_files.sql`: + ```sql + -- name: LookupTrackMetaByPaths :many + SELECT id, file_path, title, artist_name + FROM track_metadata + WHERE file_path IN (sqlc.slice('paths')); + ``` + This uses the `track_metadata` VIEW created by Plan 01. The VIEW's columns `title` and `artist_name` match the data lookupChunk currently fetches via its inline JOIN. + + 2. Run `go tool sqlc generate` from `backend/database/` to generate the Go code. + + 3. Update `backend/queue/persistence.go`: + + a. Replace the `lookupChunk` method body. Instead of building `fmt.Sprintf` placeholders, call the sqlc-generated `LookupTrackMetaByPaths` method: + ```go + func (q *Queue) lookupChunk( + paths []string, + result map[string]trackMeta, + ) { + if len(paths) == 0 { + return + } + + rows, err := q.db.Queries.LookupTrackMetaByPaths(q.db.Ctx, paths) + if err != nil { + q.logger.Error("Batch metadata lookup failed", "err", err) + return + } + + for _, row := range rows { + result[row.FilePath] = trackMeta{ + AudioFileID: row.ID, + FilePath: row.FilePath, + Title: row.Title, + Artist: row.ArtistName, + } + } + } + ``` + + b. The `lookupTrackMetaBatch` function stays unchanged — it still chunks at `maxSQLiteVars` and calls `lookupChunk` per chunk. The chunking is still necessary because `sqlc.slice()` does NOT auto-chunk. + + c. Remove the now-unused imports: `"fmt"` and `"strings"` may become unused if `insertTrackBatch` is the only remaining user. Check import usage — `fmt` is still needed for `insertTrackBatch` (line ~200 `fmt.Errorf`), and `strings` is still needed for `insertTrackBatch` (line ~196 `strings.Join`). Keep both if still referenced. + + 4. Verify the field name mapping is correct: + - VIEW column `id` → sqlc field `ID` → `trackMeta.AudioFileID` + - VIEW column `file_path` → sqlc field `FilePath` → `trackMeta.FilePath` + - VIEW column `title` → sqlc field `Title` → `trackMeta.Title` + - VIEW column `artist_name` → sqlc field `ArtistName` → `trackMeta.Artist` + + 5. Run queue tests to verify the migration doesn't break metadata resolution. + + + cd backend/database && go tool sqlc generate && cd ../.. && go test -tags webkit2_41 -race -count=1 -timeout 60s ./backend/queue/... + + + - sqlc query `LookupTrackMetaByPaths` exists in audio_files.sql + - lookupChunk uses the sqlc-generated query instead of fmt.Sprintf + - lookupTrackMetaBatch still chunks at maxSQLiteVars (900) + - All queue tests pass with -race (29 tests) + - No hand-crafted SQL remains in lookupChunk + + + + + Task 2: Add SAFETY comments to all hand-crafted SQL + + backend/database/search.go + backend/library/library.go + backend/library/rescan.go + backend/queue/persistence.go + + + Add `// SAFETY:` comments to all 12 hand-crafted SQL statements. Each comment has two parts: (1) WHY sqlc can't handle it, and (2) what makes the query safe. Cross-reference related operations where applicable. + + **backend/database/search.go** (7 statements): + + 1. Before SearchFTS query (~line 34): + `// SAFETY: FTS5 MATCH syntax unsupported by sqlc. Query is parameterized; no string interpolation.` + + 2. Before SearchFTSByFilename query (~line 92): + `// SAFETY: FTS5 MATCH syntax unsupported by sqlc. Query is parameterized; no string interpolation.` + + 3. Before InsertSearchIndex query (~line 133): + `// SAFETY: FTS5 virtual table INSERT unsupported by sqlc. All values are parameterized.` + + 4. Before DeleteSearchIndex query (~line 143): + `// SAFETY: FTS5 virtual table DELETE unsupported by sqlc. Rowid is parameterized.` + + 5. Before ClearSearchIndex query (~line 152): + `// SAFETY: FTS5 virtual table DELETE unsupported by sqlc. No parameters; unconditional delete.` + + 6. Before RebuildSearchIndex query (~line 168): + `// SAFETY: FTS5 virtual table INSERT unsupported by sqlc. All values sourced from track_metadata VIEW; no user input.` + + 7. Before SearchFTSTracks query (~line 232): + `// SAFETY: FTS5 MATCH syntax unsupported by sqlc. Query is parameterized; no string interpolation.` + + **backend/library/library.go** (3 statements): + + 8. Before commitNewAudioFile FTS INSERT (~line 798): + `// SAFETY: FTS5 virtual table, see search.go:InsertSearchIndex. All values parameterized.` + + 9. Before updateAudioFileMetadata FTS DELETE (~line 879): + `// SAFETY: FTS5 virtual table, see search.go:DeleteSearchIndex. Rowid parameterized.` + + 10. Before updateAudioFileMetadata FTS INSERT (~line 893): + `// SAFETY: FTS5 virtual table, see search.go:InsertSearchIndex. All values parameterized.` + + **backend/library/rescan.go** (1 statement): + + 11. Before clearAllLibraryData FTS DELETE (~line 165): + `// SAFETY: FTS5 virtual table, see search.go:ClearSearchIndex. No parameters; unconditional delete.` + + **backend/queue/persistence.go** (1 statement): + + 12. Before insertTrackBatch query (~line 195): + `// SAFETY: Multi-row INSERT with variable row count unsupported by sqlc. Placeholder count matches args length; no string interpolation.` + + **Rules:** + - Place each SAFETY comment on the line immediately before the SQL string literal (the query variable or inline string) + - Use the exact `// SAFETY:` prefix (capital, colon, space) + - Two-part format: reason + safety assurance + - Cross-reference related operations in library.go/rescan.go back to search.go + + + test $(grep -r "// SAFETY:" backend/database/search.go backend/library/library.go backend/library/rescan.go backend/queue/persistence.go | wc -l) -eq 12 && echo "All 12 SAFETY comments present" && go build -tags webkit2_41 ./... + + + - All 12 hand-crafted SQL statements have SAFETY comments + - Comments follow two-part format (why + safety assurance) + - Cross-references link library.go/rescan.go back to search.go + - Code compiles without errors + - No SAFETY comments on migration DDL (migration2, migration3, migration4) + + + + + + +```bash +# 1. sqlc generates cleanly +cd backend/database && go tool sqlc generate + +# 2. All queue tests pass +go test -tags webkit2_41 -race -count=1 -timeout 60s ./backend/queue/... + +# 3. All database tests pass +go test -tags webkit2_41 -race -count=1 -timeout 60s ./backend/database/... + +# 4. All library tests pass +go test -tags webkit2_41 -race -count=1 -timeout 60s ./backend/library/... + +# 5. Verify all 12 SAFETY comments exist +grep -r "// SAFETY:" backend/database/search.go backend/library/library.go backend/library/rescan.go backend/queue/persistence.go | wc -l # Should be 12 + +# 6. Verify no fmt.Sprintf remains in lookupChunk +grep -A5 "func.*lookupChunk" backend/queue/persistence.go | grep -c "fmt.Sprintf" # Should be 0 + +# 7. Full build +go build -tags webkit2_41 ./... +``` + + + +- lookupChunk uses sqlc-generated `LookupTrackMetaByPaths` query against track_metadata VIEW +- fmt.Sprintf placeholder construction eliminated from lookupChunk +- Chunking logic preserved (maxSQLiteVars = 900) +- All 12 hand-crafted SQL statements documented with // SAFETY: comments +- All existing tests pass (queue: 29, database: 15, library: 13) +- Full project builds without errors + + + +After completion, create `.planning/phases/06-sql-consolidation-code-quality/06-03-SUMMARY.md` +