DiunDashboard/.planning/research/PITFALLS.md

Domain Pitfalls

Domain: Go dashboard — SQLite to dual-database (SQLite + PostgreSQL) migration + dashboard UX improvements Researched: 2026-03-23 Confidence: HIGH for SQLite/Go-specific pitfalls (sourced directly from codebase evidence); MEDIUM for PostgreSQL dialect differences (from training knowledge, verified against known Go database/sql contract)

---

Critical Pitfalls

Mistakes that cause rewrites, data loss, or silent test passes.

---

Pitfall 1: Leaking SQLite-specific SQL into "shared" query layer

What goes wrong: When adding a PostgreSQL path, developers copy existing SQLite queries and swap the driver — but keep SQLite-isms in the SQL itself. The two most common in this codebase: datetime('now') (SQLite built-in, line 225) and INSERT OR REPLACE (SQLite only, lines 109 and 352). Both fail silently or loudly on PostgreSQL. PostgreSQL uses NOW() and INSERT ... ON CONFLICT DO UPDATE.

Why it happens: The queries are embedded as raw strings throughout handler functions rather than in a dedicated SQL layer. Each query must be individually audited and conditionally branched or abstracted.

Consequences: PostgreSQL path silently produces wrong results (datetime('now') evaluates as a column name or throws an error) or panics on INSERT OR REPLACE (PostgreSQL does not support this syntax at all).

Warning signs:

• Any raw db.Exec or db.Query call with datetime(, OR REPLACE, AUTOINCREMENT, PRAGMA, or ? placeholders — all must be replaced or branched for PostgreSQL.
• ? is the SQLite/MySQL placeholder; PostgreSQL requires $1, $2, etc.

Prevention:

• Define a Store interface with methods (UpsertEvent, GetUpdates, DismissImage, etc.) and provide two concrete implementations: sqliteStore and pgStore.
• Never write raw SQL in HTTP handlers. All SQL lives in the store implementation only.
• Add an integration test that runs against both stores for every write operation; if the schema or SQL diverges the test fails at the driver level.

Phase mapping: Must be resolved before any PostgreSQL code is written — this is the foundational refactor that makes dual-DB possible without a maintenance nightmare.

---

Pitfall 2: INSERT OR REPLACE silently deletes tag assignments before PostgreSQL is even added

What goes wrong: UpdateEvent() (line 109) uses INSERT OR REPLACE INTO updates. SQLite implements this as DELETE + INSERT when a conflict is found. Because tag_assignments.image is a foreign key referencing updates.image, the DELETE step removes the child row — unless PRAGMA foreign_keys = ON is active (it is not, confirmed at line 58-103). Even with FK enforcement, the CASCADE would delete the assignment rather than preserve it. The result: every time DIUN sends a new event for a tracked image, its tag assignment vanishes.

Why it happens: The intent of INSERT OR REPLACE is to update existing rows, but the mechanism is destructive. The UPSERT syntax (INSERT ... ON CONFLICT(image) DO UPDATE SET ...) is the correct tool and has been available since SQLite 3.24 (2018).

Consequences: This bug is already in production. Users lose tag assignments every time an image receives a new DIUN event. This directly contributed to the trust erosion described in PROJECT.md. Adding PostgreSQL without fixing this first means the bug ships in both DB paths.

Warning signs:

• Tag assignments disappear after DIUN reports a new update for a previously-tagged image.
• TestDismissHandler_ReappearsAfterNewWebhook tests the acknowledged-state reset correctly, but no test asserts that the tag survives a second UpdateEvent call on the same image.

Prevention:

• Replace line 109 with: INSERT INTO updates (...) VALUES (...) ON CONFLICT(image) DO UPDATE SET diun_version=excluded.diun_version, ... (preserves all other columns, including nothing that touches tag_assignments).
• Add PRAGMA foreign_keys = ON immediately after sql.Open in InitDB().
• Add a regression test: UpdateEvent twice on the same image with a tag assigned between calls; assert tag survives.

Phase mapping: Fix before any other work — this is a data-correctness bug affecting existing users.

---

Pitfall 3: Global package-level state makes database abstraction structurally impossible without a refactor

What goes wrong: The codebase uses var db *sql.DB and var mu sync.Mutex at package level (lines 48-52). The InitDB function sets the global db. Adding PostgreSQL means calling a different sql.Open and storing it — but there is only one db variable. You cannot run SQLite and PostgreSQL tests in the same process, cannot dependency-inject the store into handlers, and cannot test the two stores independently.

Why it happens: The package was written as a single-instance tool, which was appropriate at first. Dual-DB support requires the concept of a "store" that can be swapped — which requires struct-based design.

Consequences: If you try to add PostgreSQL without refactoring, you end up with if dbType == "postgres" { ... } else { ... } branches scattered across every handler. This is unmaintainable, untestable, and will break if a third DB is ever added.

Warning signs:

• Any attempt to pass a PostgreSQL *sql.DB to the existing handlers requires changing the global variable, which breaks concurrent tests.
• The test file uses UpdatesReset() to reset global state between tests — a design smell that signals the global state problem.

Prevention:

• Introduce type Server struct { store Store; secret string } where Store is an interface.
• Move all handler functions to methods on Server.
• InitDB becomes a factory: NewSQLiteStore(path) or NewPostgresStore(dsn) returning the interface.
• Tests construct a fresh Server with an in-memory SQLite store; no global state to reset.

Phase mapping: This refactor is the prerequisite for dual-DB. Do it as the first step of the milestone, before any PostgreSQL driver work.

---

Pitfall 4: Schema migration strategy does not scale to dual-DB or multi-version upgrades

What goes wrong: The current migration strategy is a single silent ALTER TABLE at line 87: _, _ = db.Exec("ALTER TABLE updates ADD COLUMN acknowledged_at TEXT"). This works for one SQLite column addition but fails in two ways when expanded: (1) PostgreSQL requires different syntax and error handling, (2) there is no version tracking, so there is no way to know which migrations have already run on an existing database.

Why it happens: The approach was acceptable for a single-column addition in a personal project. It does not generalise.

Consequences:

• On PostgreSQL, ALTER TABLE ... ADD COLUMN IF NOT EXISTS is available but the silent _, _ error swallow pattern will hide real migration failures.
• If a second column is added in a future milestone, there is no mechanism to skip it on databases that already have it (SQLite's IF NOT EXISTS on ADD COLUMN is only available in SQLite 3.37+).
• Existing user databases upgrading from the current version need all migrations to run in order and idempotently.

Warning signs:

• More than one ALTER TABLE in InitDB().
• Any _, _ = db.Exec(...) where the underscore discards an error on a DDL statement.

Prevention:

• Introduce a schema_migrations table with a single version INTEGER column.
• Write migrations as numbered functions: migration001, migration002, etc.
• InitDB reads the current version and runs only pending migrations.
• Keep migrations simple: pure SQL, no application logic.
• A lightweight library (golang-migrate/migrate) can handle this, but for this project's scale a 30-line hand-rolled runner is sufficient and avoids a new dependency.

Phase mapping: Implement alongside the Store interface refactor. The migration runner must support both SQLite and PostgreSQL SQL dialects.

---

Moderate Pitfalls

---

Pitfall 5: PostgreSQL connection pooling behaves differently than SQLite's forced single connection

What goes wrong: The SQLite configuration uses db.SetMaxOpenConns(1) to serialize all DB access (line 64). This was the correct choice for SQLite's single-writer model. For PostgreSQL, MaxOpenConns(1) is a severe bottleneck and eliminates one of the primary reasons to use PostgreSQL. However, removing the constraint also removes the sync.Mutex, which must be eliminated correctly — not just the SetMaxOpenConns(1) call.

Why it happens: The mutex was added as belt-and-suspenders to the SetMaxOpenConns(1) constraint. For PostgreSQL, transactions handle isolation and the driver manages connection pooling correctly. The mutex is not needed and actively harmful at scale.

Consequences: Keeping SetMaxOpenConns(1) on PostgreSQL caps throughput to sequential queries. Removing it without reviewing the mutex usage can cause incorrect locking (the mutex guards writes, but PostgreSQL transactions should guard atomicity instead).

Warning signs:

• The pgStore implementation sets MaxOpenConns(1) — that is wrong.
• The pgStore implementation acquires a sync.Mutex around individual db.Exec calls instead of using transactions.

Prevention:

• In sqliteStore: keep SetMaxOpenConns(1) and the mutex (SQLite needs it).
• In pgStore: use PostgreSQL's default pooling (SetMaxOpenConns appropriate to load, e.g. 10-25), use db.BeginTx for operations that require atomicity, no application-level mutex.
• Document the difference in code comments.

Phase mapping: During the pgStore implementation phase.

---

Pitfall 6: Optimistic UI updates in assignTag have no rollback on failure

What goes wrong: assignTag() in useUpdates.ts (lines 60-84) applies the state change optimistically before the API call. If the PUT/DELETE fails, the UI shows the new tag state but the server retained the old one. The next poll at most 5 seconds later will overwrite the optimistic state with the real server state — but during that window the user sees incorrect data. Worse, the error is only console.error, so the user gets no feedback that their action failed.

Why it happens: Optimistic updates are a good UX pattern, but require pairing with: (a) rollback on failure, and (b) user-visible error feedback.

Consequences:

• During a 5-second window after a failed tag assignment, the UI shows the wrong tag.
• If the backend is down and the user assigns multiple tags, all changes appear to succeed. The next poll resets all of them silently.

Warning signs:

• No try/catch that restores prev state on assignTag failure.
• No error toast or inline error state for tag assignment failures.

Prevention:

• Capture prevState before the optimistic update.
• In the catch block: restore prevState and surface an error message to the user (inline or toast).
• Example pattern: const prev = updates[image]; setUpdates(optimistic); try { await api() } catch { setUpdates(restore(prev)); showError() }.

Phase mapping: Part of the UX improvements phase.

---

Pitfall 7: Bulk acknowledge actions hitting the backend sequentially instead of in a single operation

What goes wrong: "Dismiss all" and "dismiss by group" are planned features. The naive implementation fires one PATCH /api/updates/{image} per image from the frontend. For a user with 30 tracked images, this is 30 sequential API calls. Each call acquires the mutex and executes a SQL UPDATE. This is fine for single-user loads but is the wrong pattern: it creates 30 round trips, 30 DB transactions, and 30 state updates in the React UI (causing 30 re-renders).

Why it happens: The existing dismiss path is single-image by design; bulk is an afterthought unless an explicit bulk endpoint is designed from the start.

Consequences:

• 30 re-renders in rapid succession cause visible UI flickering.
• If one request fails in the middle, some images are acknowledged and others are not, with no clear feedback to the user.

Warning signs:

• A "dismiss all" button that loops over updates calling acknowledge(image) in sequence or in Promise.all.

Prevention:

• Add a POST /api/updates/acknowledge-bulk endpoint that accepts an array of image names and wraps all UPDATEs in a single transaction.
• The frontend calls one endpoint and updates state once.
• For "dismiss by group": pass tag_id as the filter parameter so the backend does UPDATE updates SET acknowledged_at = NOW() WHERE image IN (SELECT image FROM tag_assignments WHERE tag_id = ?).

Phase mapping: Design the bulk endpoint before implementing the frontend bulk UI; the API contract drives the UI, not the other way around.

---

Pitfall 8: No rollback path for existing SQLite users upgrading to a version with dual-DB

What goes wrong: When an existing user upgrades their Docker image to the version that includes PostgreSQL support, they continue using SQLite. If the migration runner runs new DDL migrations on their existing SQLite database (e.g., a new column added for PostgreSQL compatibility), and the migration fails silently due to the _, _ pattern, they are left with a database in an intermediate state. On the next restart the migration runner does not know whether to retry or skip.

Why it happens: No migration version tracking means "already migrated" cannot be distinguished from "never migrated."

Consequences: Database schema becomes inconsistent. Queries that expect the new column fail. The user has no recourse except to delete the database (losing all data) or manually run SQL.

Warning signs:

• InitDB has no SELECT version FROM schema_migrations step.
• Migration SQL errors are swallowed.

Prevention:

• Implement the versioned migration runner (see Pitfall 4).
• Log migration progress visibly at startup: INFO: running migration 002 (add_xyz_column).
• For the column that already exists implicitly (acknowledged_at), migration 001 is ALTER TABLE updates ADD COLUMN IF NOT EXISTS acknowledged_at TEXT with the result logged regardless of whether the column existed.

Phase mapping: Part of the store interface refactor phase, before any new schema changes land.

---

Minor Pitfalls

---

Pitfall 9: Drag handle invisible by default breaks tag reorganization discoverability

What goes wrong: The GripVertical icon in ServiceCard.tsx (line 96) has opacity-0 group-hover:opacity-100. On touch devices, on keyboard navigation, and for users who do not hover over each card, the drag-to-regroup feature is entirely invisible. Drag-and-drop is the only way to assign a tag to an image (the assignTag API is only called from the drag-and-drop handler).

Why it happens: The design prioritized a clean visual for non-interactive browsing, but made the interactive feature undiscoverable.

Consequences: Users who cannot use hover (touch devices, keyboard-only) have no way to reorganize images. As noted in CONCERNS.md, the delete button on TagSection.tsx has the same problem.

Warning signs:

• The drag handle has opacity-0 without a focus-visible:opacity-100 counterpart.
• No alternative assignment mechanism exists (e.g., a dropdown on the card).

Prevention:

• Make the grip handle always visible at reduced opacity (e.g., opacity-30 group-hover:opacity-100), or make it visible on focus.
• Add an accessible fallback: a "Move to group" dropdown on the card's context menu or ... menu. This also gives keyboard and touch users the ability to assign tags.

Phase mapping: UX improvements phase. Not a blocker for DB work but should be addressed before the milestone closes.

---

Pitfall 10: datetime('now') in DismissHandler produces SQLite-only timestamps

What goes wrong: DismissHandler (line 225) writes acknowledged_at using datetime('now'), a SQLite built-in. This is a SQL dialect issue distinct from the INSERT OR REPLACE problem. When the PostgreSQL path is added, this query must become NOW() or an application-layer timestamp.

Why it happens: It is a small single-line SQL call, easy to overlook during the migration to dual-DB.

Consequences: DismissHandler breaks entirely on PostgreSQL; datetime('now') is not a valid PostgreSQL function call and will produce a column-name error.

Warning signs:

• Any raw datetime( in query strings.

Prevention:

• In the Store interface, the DismissImage(image string) error method takes no timestamp argument — the store implementation generates NOW() in SQL or passes time.Now() as a parameter from Go. Passing the timestamp from Go (? / $1) is the most portable approach: both SQLite and PostgreSQL accept a bound time.Time value, removing all dialect issues for timestamps.

Phase mapping: Resolve during the pgStore implementation. Can be fixed in sqliteStore at the same time for consistency.

---

Pitfall 11: AUTOINCREMENT in SQLite schema vs PostgreSQL SERIAL or GENERATED ALWAYS AS IDENTITY

What goes wrong: The tags table uses INTEGER PRIMARY KEY AUTOINCREMENT (line 90). PostgreSQL does not have AUTOINCREMENT; it uses SERIAL, BIGSERIAL, or GENERATED ALWAYS AS IDENTITY. When writing the CREATE TABLE DDL for PostgreSQL, this must be translated.

Why it happens: A detail that is invisible in the SQLite path because CREATE TABLE IF NOT EXISTS never re-runs.

Consequences: CREATE TABLE fails on PostgreSQL if the SQLite DDL is used verbatim.

Warning signs:

• A single schema.sql file used for both databases.

Prevention:

• Store DDL per-driver: schema_sqlite.sql and schema_pg.sql, or generate DDL in code with driver-specific constants.
• For PostgreSQL, use id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY.

Phase mapping: Part of the initial pgStore schema setup.

---

Phase-Specific Warnings

Phase Topic	Likely Pitfall	Mitigation
Fix SQLite bugs (UPSERT + FK enforcement)	INSERT OR REPLACE deletes tag assignments (Pitfall 2)	Use ON CONFLICT DO UPDATE; add PRAGMA foreign_keys = ON
Store interface refactor	Global state prevents dual-DB (Pitfall 3)	Struct-based Server with Store interface before any PostgreSQL work
Migration runner	Silent failures leave DB in unknown state (Pitfalls 4, 8)	Versioned migrations with visible logging; never swallow DDL errors
PostgreSQL implementation	SQLite SQL dialect in shared queries (Pitfall 1)	All SQL in store implementations, never in handlers; integration test both stores
PostgreSQL connection setup	Single-connection constraint applied to Postgres (Pitfall 5)	pgStore uses pooling and transactions, not mutex + MaxOpenConns(1)
Timestamp writes	datetime('now') fails on PostgreSQL (Pitfall 10)	Pass time.Now() as a bound parameter from Go instead of using SQL built-ins
Schema creation	AUTOINCREMENT not valid PostgreSQL syntax (Pitfall 11)	Separate DDL per driver
Bulk acknowledge UI	Sequential API calls cause flickering and partial state (Pitfall 7)	Design bulk endpoint first; one API call, one state update
Tag UX improvements	Optimistic updates without rollback confuse users (Pitfall 6)	Always pair optimistic updates with catch rollback and user-visible error
Accessibility improvements	Drag handle invisible; keyboard users cannot reorganize (Pitfall 9)	Always-visible handle at reduced opacity + dropdown alternative

---

Sources

• Codebase analysis: /pkg/diunwebhook/diunwebhook.go, lines 48-117, 225, 352 (HIGH confidence — direct code evidence)
• Codebase analysis: /frontend/src/hooks/useUpdates.ts, lines 60-84 (HIGH confidence — direct code evidence)
• Codebase analysis: /frontend/src/components/ServiceCard.tsx, line 96 (HIGH confidence — direct code evidence)
• .planning/codebase/CONCERNS.md — confirmed INSERT OR REPLACE and FK enforcement issues (HIGH confidence — prior audit)
• Go database/sql package contract and SQLite vs PostgreSQL dialect differences (MEDIUM confidence — training knowledge, no external verification available; recommend verifying PostgreSQL placeholder syntax $1 format before implementation)