makiolaj/GearBox
1
0
Fork 0
Code Issues 8 Pull Requests Actions Packages Projects Releases 10 Wiki Activity

docs: complete project research

Browse Source
This commit is contained in:
Jean-Luc Makiola
2026-04-03 22:14:27 +02:00
parent 642ae0d43f
commit 443802fc68
5 changed files with 1642 additions and 1113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

498
.planning/research/PITFALLS.md
View File

@@ -1,368 +1,436 @@
# Pitfalls Research
**Domain:** Adding side-by-side candidate comparison, setup impact preview, and drag-to-reorder ranking to an existing gear management app (GearBox v1.3)
**Researched:** 2026-03-16
**Confidence:** HIGH (derived from direct codebase analysis of v1.2 + verified with dnd-kit GitHub issues, TanStack Query docs, and Baymard comparison UX research)
---
**Domain:** Single-user to multi-user gear platform migration (GearBox v2.0)
**Researched:** 2026-04-03
**Confidence:** HIGH (based on direct codebase analysis of v1.4 + established migration patterns)
## Critical Pitfalls
### Pitfall 1: dnd-kit + React Query Cache Produces Visible Flicker on Drop
### Pitfall 1: Missing userId Filters Leak Data Between Users
**What goes wrong:**
When ranking candidates via drag-to-reorder, the naive approach is to call `mutate()` in `onDragEnd` and apply an optimistic update with `setQueryData` in the `onMutate` callback. Despite this, the item visibly snaps back to its original position for a split second before settling in the new position. The user sees a "jump" on every successful drop, which makes the ranking feature feel broken even though the data is correct.
Every query in the existing codebase operates without a `userId` filter. After adding `userId` columns to `items`, `categories`, `threads`, `setups`, and `settings`, any service function not updated to filter by `userId` will return or mutate other users' data. The current `getAllItems()` returns `db.select().from(items).innerJoin(...)` with zero WHERE clauses. One missed function means User A sees User B's gear.
The surface area is large: 6 service files, 19 MCP tools, 7 route files, aggregate queries in `totals`, the `duplicateItem` function, the `getCollectionSummary` MCP resource, setup-item joins, and thread resolution (which creates a new item).
**Why it happens:**
dnd-kit's `SortableContext` derives its order from React state. When the order is stored in React Query's cache rather than local React state, there is a timing mismatch: dnd-kit reads the list order from the cache after the drop animation, but the cache update triggers a React re-render cycle that arrives one or two frames late. The drop animation briefly shows the item at its original position before the re-render reflects the new order. This is a known, documented issue in dnd-kit (GitHub Discussion #1522, Issue #921) that specifically affects React Query integrations.
Developers add `userId` to the schema, update the obvious CRUD functions, but miss edge cases. The codebase has enough query sites (~30+) that manual "find all queries" misses something. Thread resolution is particularly dangerous because it creates an item as a side effect of updating a thread.
**How to avoid:**
Use a `tempItems` local state (`useState<Candidate[] | null>(null)`) alongside React Query. On `onDragEnd`, immediately set `tempItems` to the reordered array before calling `mutate()`. Render the candidate list from `tempItems ?? queryData.candidates`. In mutation `onSettled`, set `tempItems` to `null` to hand back control to React Query. This approach:
- Prevents the flicker because the component re-renders from synchronous local state immediately
- Avoids `useEffect` syncing (which adds extra renders and is error-prone)
- Stays consistent with the existing React Query + Zustand pattern in the codebase
- Handles drag cancellation cleanly (reset `tempItems` on `onDragCancel`)
Do not store the rank column data in the React Query `["threads", threadId]` cache key in a way that requires invalidation and refetch after reorder — this causes a round-trip delay that amplifies the flicker.
1. Enable Postgres Row-Level Security (RLS) as a safety net -- even if the app filters by `userId`, RLS prevents cross-user access at the database level.
2. Add `userId` as NOT NULL to the Drizzle schema first, then use TypeScript compiler errors to find every query that needs updating (insert calls will fail where `userId` is required but not provided).
3. Write one integration test per entity: create data as User A, query as User B, assert empty results.
4. Grep the codebase for every `.from(items)`, `.from(categories)`, `.from(threads)`, `.from(setups)`, `.from(settings)` and verify each has a `userId` filter.
**Warning signs:**
- Item visibly snaps back to original position for ~1 frame on drop
- `onDragEnd` calls `mutate()` but uses no local state bridge
- `setQueryData` is the only state update on drag end
- Any service function that does not accept a `userId` parameter after migration.
- Tests that pass without specifying which user is performing the action.
- MCP tools that work without user context.
**Phase to address:**
Candidate ranking phase — the `tempItems` pattern must be designed before building the drag UI, not retrofitted after noticing the flicker.
Multi-user data model phase. This is the single most important thing to get right. Do not add public content or discovery features until every query is provably user-scoped.
---
### Pitfall 2: Rank Storage Using Integer Offsets Requires Bulk Writes
### Pitfall 2: Category Name Uniqueness Breaks in Multi-User
**What goes wrong:**
The most obvious approach for storing candidate rank order is adding a `sortOrder INTEGER` column to `thread_candidates` and storing 1, 2, 3... When the user drags candidate #2 to position #1, the naive fix is to update all subsequent candidates' `sortOrder` values to maintain contiguous integers. With 5 candidates, this is 5 UPDATE statements per drop. With rapid dragging, this creates a burst of writes where each intermediate position during the drag fires updates. If the app ever has threads with 10+ candidates (not uncommon for a serious gear decision), this becomes visible latency on every drag.
The current schema has `name: text("name").notNull().unique()` on the `categories` table -- a global unique constraint. When User A creates a "Bikepacking" category, User B cannot. The migration must change this to a composite unique constraint on `(userId, name)`.
**Why it happens:**
Integer rank storage feels natural and maps directly to how arrays work. The developer adds `ORDER BY sort_order ASC` to the candidate query and calls it done. The performance problem is only discovered when testing with a realistic number of candidates and a fast drag gesture.
Single-user apps use simple unique constraints. Developers add `userId` to the table but forget to update the unique constraint from `unique(name)` to `unique(userId, name)`. The migration runs fine on an empty database but fails the moment a second user creates a category with a common name.
**How to avoid:**
Use a `sortOrder REAL` column (floating-point) with fractional indexing — when inserting between positions A and B, assign `(A + B) / 2`. This means a drag requires only a single UPDATE for the moved item. Only trigger a full renumber (resetting all candidates to integers 1000, 2000, 3000... or similar spaced values) when the float precision degrades (approximately after 50+ nested insertions, unlikely in practice for this app). Start values at 1000, 5000 increments to give ample room.
For GearBox's use case (typically 2-8 candidates per thread), integer storage is workable, but the fractional approach is cleaner and avoids the bulk-write problem entirely. The added complexity is minimal: one line of math in the service layer.
Regardless of storage strategy, add an index: `CREATE INDEX ON thread_candidates (thread_id, sort_order)`.
Audit every `.unique()` constraint in the schema during migration. `categories.name` must become a composite unique on `(userId, name)`. The `users.username` unique stays global (desired). No other tables currently have unique constraints, but new tables (reviews, products) should use composite uniqueness from the start.
**Warning signs:**
- `sortOrder` column uses `integer()` type in Drizzle schema
- Reorder service function issues multiple UPDATE statements in a loop
- No transaction wrapping the bulk update
- Each drag event (not just the final drop) triggers a service call
- Database constraint errors when a second user creates categories.
- Tests that only ever use one user.
**Phase to address:**
Schema and service design phase for candidate ranking — the storage strategy must be chosen before building the sort UI, as changing from integer to fractional later requires a migration.
Multi-user data model phase, during schema migration.
---
### Pitfall 3: Impact Preview Reads Stale Candidate Data
### Pitfall 3: Drizzle Schema Rewrite Is a Replacement, Not a Migration
**What goes wrong:**
The impact preview shows "+450g / +$89" next to each candidate — what this candidate would add to the selected setup. The calculation is: `(candidate.weightGrams - null) + setup.totalWeight`. But the candidate card data comes from the `["threads", threadId]` query cache, while the setup totals come from a separate `["setups", setupId]` query cache. These caches can be out of sync: the user edits a candidate's weight in one tab, invalidating the threads cache, but if the setup was fetched earlier and has not been refetched, the "current setup weight" baseline in the delta is stale. The preview shows a delta calculated against the wrong baseline.
Drizzle ORM schemas are dialect-specific. The current schema imports from `drizzle-orm/sqlite-core` and uses `sqliteTable`, `integer().primaryKey({ autoIncrement: true })`, and `real()`. The Postgres schema must import from `drizzle-orm/pg-core` and use `pgTable`, `serial()` or `integer().generatedAlwaysAsIdentity()`, and `doublePrecision()`. This is not a migration Drizzle can auto-generate -- it requires a full schema rewrite and a fresh migration history.
The second failure mode: if `candidate.weightGrams` is `null` (not yet entered), displaying `+-- / +$89` is confusing. Users see "null delta" and assume the comparison is broken rather than understanding that the candidate has no weight data.
Specific differences that will cause bugs if missed:
- `integer("id").primaryKey({ autoIncrement: true })` becomes `serial("id").primaryKey()` or `integer("id").primaryKey().generatedAlwaysAsIdentity()`.
- `integer("created_at", { mode: "timestamp" })` -- SQLite stores timestamps as integers. Postgres has native `timestamp` type. Must decide: keep integer storage or switch to Postgres `timestamp()`.
- `real("weight_grams")` -- SQLite `REAL` is 8-byte float. Postgres `real` is 4-byte float (less precision). Use `doublePrecision()` for equivalent behavior.
- SQLite `text("status")` with string values works as pseudo-enum. Postgres has native `pgEnum` for type safety.
- The `Db` type alias (`typeof prodDb`) changes entirely -- every service file and MCP tool imports this type.
**Why it happens:**
Impact preview feels like pure computation — "just subtract two numbers." The developer writes it as a derived value from two props and does not think about cache coherence. The null case is often overlooked because the developer tests with complete candidate data.
Developers assume Drizzle abstracts away database differences. It does not at the schema layer. The query builder is mostly compatible, but schema definition is dialect-specific by design.
**How to avoid:**
1. Derive the delta from data already co-located in one cache entry where possible. The thread detail query (`["threads", threadId]`) returns all candidates; the setup query (`["setups", setupId]`) returns items with weight. Compute the delta in the component using both: `delta = candidate.weightGrams - replacedItemWeight` where `replacedItemWeight` is taken from the currently loaded setup data.
2. Use `useQuery` for setup data with the setup selector in the same component that renders the comparison, so both data sources are reactive.
3. Handle null weight explicitly: show "-- (no weight data)" not "--g" for candidates without weights. Make the null state visually distinct from a zero delta.
4. Do NOT make a server-side `/api/threads/:id/impact?setupId=:sid` endpoint that computes delta server-side — this creates a third cache entry to invalidate and adds network latency to what should be a purely client-side calculation.
1. Write a new `schema.ts` from scratch using `pg-core`, not edit the existing one.
2. Start a fresh Drizzle migration history for Postgres. SQLite migrations are irrelevant.
3. Write a data migration script that reads from old SQLite and inserts into new Postgres.
4. Update the `Db` type alias in all service files.
5. Use `doublePrecision()` not `real()` for weight values to maintain precision parity with SQLite.
**Warning signs:**
- Impact delta shows stale values after editing a candidate's weight
- Null weight candidates show a numerical delta (treating null as 0)
- Delta calculation is in a server route rather than a client-side derived value
- Setup data is fetched via a different hook than the one used for candidate data, with no shared staleness boundary
- Weight values losing precision (245.5g becoming 245.49999...).
- Timestamps behaving differently (integer epoch vs. native timestamp).
- drizzle-kit refusing to generate migrations against the wrong dialect.
**Phase to address:**
Impact preview phase — establish the data flow (client-side derived from two existing queries) before building the UI so the stale-cache problem cannot arise.
Database migration phase. Must complete before any other v2.0 feature.
---
### Pitfall 4: Side-by-Side Comparison Breaks at Narrow Widths
### Pitfall 4: Test Infrastructure Collapses During Database Switch
**What goes wrong:**
The comparison view is built with a fixed two-or-three-column grid for the candidate cards. On a laptop at 1280px it looks great. On a narrower viewport or when the browser window is partially shrunk, the columns collapse to ~200px each, making the candidate name truncated, the weight/price badges unreadable, and the notes text invisible. The user cannot actually compare the candidates — the view that was supposed to help them decide becomes unusable.
The entire test infrastructure is built on SQLite. `createTestDb()` uses `bun:sqlite` with `Database(":memory:")` and `drizzle-orm/bun-sqlite`. E2E tests use a file-based SQLite (`e2e/test.db`). After switching to Postgres, every test needs a Postgres connection -- no more in-memory databases.
GearBox's existing design philosophy is mobile-responsive, but comparison tables are inherently wide. The tension is real: side-by-side requires horizontal space that mobile cannot provide.
The MCP server hard-codes `db as prodDb` which is an SQLite Drizzle instance. The Hono context variable type for `db` changes. Every route handler that does `c.get("db")` gets a different type.
**Why it happens:**
Comparison views are usually mocked at full desktop width. Responsiveness is added as an afterthought, and the "fix" is often to stack columns vertically on mobile — which defeats the entire purpose of side-by-side comparison.
In-memory SQLite is the best testing story in the Bun ecosystem -- fast, isolated, no external services. Postgres testing requires either: (a) a running Postgres instance, (b) testcontainers with Docker, or (c) PGlite (lightweight Postgres in WebAssembly). Developers delay updating tests and end up with a broken test suite for weeks.
**How to avoid:**
1. Build the comparison view as a horizontally scrollable container on mobile (`overflow-x: auto`). Do not collapse to vertical stack — comparing stacked items is cognitively equivalent to switching between detail pages.
2. Limit the number of simultaneously compared candidates to 3 (or at most 4). Comparing 8 candidates side-by-side is unusable regardless of screen size.
3. Use a minimum column width (e.g., `min-width: 200px`) so the container scrolls horizontally before the column content becomes illegible.
4. Sticky first column for candidate names when scrolling horizontally, so the user always knows which column they are reading.
5. Test at 768px viewport width before considering the feature done.
1. Adopt PGlite (`@electric-sql/pglite`) for unit/integration tests. It provides in-memory Postgres without Docker. Drizzle supports PGlite via `drizzle-orm/pglite`.
2. Update `createTestDb()` to use PGlite instead of bun:sqlite.
3. For E2E tests, use Docker Compose with a test Postgres instance, or PGlite if performance is acceptable.
4. Update the Hono context variable type to the new Postgres Drizzle instance type.
5. Migrate test infrastructure in the same phase as the schema, not after.
**Warning signs:**
- Comparison grid uses percentage widths that collapse below 150px
- No horizontal scroll on the comparison container
- Mobile viewport shows columns stacked vertically
- Candidate name or weight badges are truncated without tooltip
- `bun test` fails across the board after schema change.
- "Type 'BunSQLiteDatabase' is not assignable to type 'PgDatabase'" errors everywhere.
- E2E tests silently skipped or disabled "temporarily."
**Phase to address:**
Side-by-side comparison UI phase — responsive behavior must be designed in, not retrofitted. The minimum column width and scroll container decision shapes the entire component structure.
Database migration phase. Tests must migrate alongside the schema.
---
### Pitfall 5: Pros/Cons Fields Stored as Free Text in Column, Not Structured
### Pitfall 5: Auth Provider Integration Breaks Existing Sessions, API Keys, and MCP
**What goes wrong:**
Pros and cons per candidate are stored as two free-text columns: `pros TEXT` and `cons TEXT` on `thread_candidates`. The user types a multi-line blob of text into each field. The comparison view renders them as raw text blocks next to each other. Two problems emerge:
- Formatting: the comparison view cannot render individual pro/con bullet points because the data is unstructured blobs
- Length: one candidate has a 500-word "pros" essay; another has two words. The comparison columns have wildly unequal heights, making the side-by-side comparison visually chaotic and hard to scan
The deeper problem: free text in a comparison context produces noise, not signal. Users write "it's really lightweight and packable and the color options are nice" when what the comparison view needs is scannable bullet points.
The current auth stores users, sessions, and API keys in the local database. Switching to an external auth provider means: (1) user identity moves external, (2) session management changes (JWT or OAuth flow vs. cookie sessions), (3) existing API keys become orphaned because they reference the old user table, (4) the MCP server authenticates via API keys stored locally, (5) E2E tests authenticate via `POST /api/auth/login` with a seeded user, (6) the onboarding flow (`POST /api/auth/setup`) creates the first user.
**Why it happens:**
Adding two text columns to `thread_candidates` is the simplest possible implementation. The developer tests it with neat, short text and it looks fine. The UX failure is only visible when a real user writes the way real users write.
Auth migration is treated as "swap the login page" when it touches the entire authentication surface: user identity, session lifecycle, API key management, MCP authentication, E2E test setup, and onboarding.
**How to avoid:**
1. Store pros/cons as newline-delimited strings, not markdown or JSON. The UI splits on newlines and renders each line as a bullet. Simple, no parsing, no migration complexity.
2. In the form, use a `<textarea>` with a placeholder of "one item per line." Show a character count.
3. In the comparison view, render each newline-delimited entry as its own row, so columns stay scannable. Use a max of 5 bullet points per field; truncate with "show more" if longer.
4. Cap `pros` and `cons` field length at 500 characters in the Zod schema to prevent essay-length blobs.
5. The comparison view should truncate to the first 3 bullets when in compact comparison mode, with expand option.
1. Keep API keys in the local database even after auth moves external. API keys are long-lived credentials managed by the application, not the auth provider.
2. Map external provider user IDs to a local `users` table. The external provider handles authentication; the local table handles application-level data (userId foreign keys, API keys, preferences). Foreign keys reference local `users.id`, not the provider's UUID.
3. Replace the onboarding flow: instead of "create admin account," it becomes "sign up via external provider, first user gets admin role."
4. Update E2E tests to either mock the auth provider or use API key authentication exclusively for E2E.
**Warning signs:**
- `pros TEXT` and `cons TEXT` added to schema with no length constraint
- Comparison view renders `{candidate.pros}` as a raw string in a `<p>` tag
- One candidate's pros column is 3x taller than another's, making row alignment impossible
- Form shows a full-height textarea with no guidance on format
- MCP server stops working after auth migration.
- E2E tests that log in via `POST /api/auth/login` all fail.
- API keys created before migration stop working.
- No local `users` table -- everything delegated to external provider.
**Phase to address:**
Both the ranking schema phase (when pros/cons columns are added) and the comparison UI phase (when the rendering decision is made). The newline-delimited format must be decided at schema design time.
Auth migration phase. Should be done early because user identity is the foundation.
---
### Pitfall 6: Impact Preview Compares Against Wrong Setup Total When Item Would Be Replaced
### Pitfall 6: Global Item Database Creates a Data Model Fork
**What goes wrong:**
The impact preview shows the delta for each candidate as if the candidate would be *added* to the setup. But the real use case is: "I want to replace my current tent with one of these candidates — which one saves the most weight?" The user expects the delta to reflect `candidateWeight - currentItemWeight`, not just `+candidateWeight`.
When the delta is calculated as a pure addition (no replacement), a 500g candidate looks like "+500g" even though the item it replaces weighs 800g, meaning it would actually save 300g. The user sees a positive delta and dismisses the candidate when they should pick it.
The current `items` table represents user-owned gear. The v2.0 vision includes a "global item database" with manufacturer specs. These are fundamentally different entities: a user's item has quantity, personal notes, setup associations, and belongs to a user. A global item is a product definition with canonical specs, owned by nobody. Conflating them in one table (via `isGlobal` flag or `NULL userId`) creates an unmaintainable mess. Separating them creates a sync problem.
**Why it happens:**
"Impact" is ambiguous. The developer defaults to "how much weight does this add?" because that calculation is simpler (no need to identify which existing item is being replaced). The replacement case requires the user to specify which item in the setup would be swapped out, which feels like additional UX complexity.
It seems efficient to add an `isGlobal` flag. But then queries need to handle both cases, user items need to link to global items for spec inheritance, and the API surface doubles with different permission models.
**How to avoid:**
1. Support both modes: "add to setup" (+delta) and "replace item" (delta = candidate - replaced item). Make the mode selection explicit in the UI.
2. Default to "add" mode if no item in the setup shares the same category as the thread. Default to "replace" mode if an item with the same category exists — offer it as a pre-populated suggestion ("Replaces: Big Agnes Copper Spur 2? Change").
3. The replacement item selector should be a dropdown filtered to setup items in the same category, defaulting to the most likely match.
4. If no setup is selected, show raw candidate weight rather than a delta — do not calculate a delta against zero.
1. Create a separate `products` table for the global database. A product has: name, manufacturer, canonical weight, canonical price, product URL, image, category.
2. User `items` gets a nullable `productId` foreign key. When set, the item inherits specs from the product but can override them (user's measured weight vs. manufacturer spec).
3. User items without a `productId` are standalone (backward-compatible with all existing items).
4. Reviews, owner counts, and setup appearances link to `products`, not user `items`.
**Warning signs:**
- Delta is always positive (never shows weight savings)
- No replacement item selector in the impact preview UI
- Thread category is not used to suggest a candidate's likely replacement item
- Delta is calculated as `candidate.weightGrams` with no baseline
- `items` table query complexity increases beyond what is reasonable.
- Ambiguity about whether an operation affects "my item" or "the global product."
- Permission model becomes unclear (who can edit a global product?).
**Phase to address:**
Impact preview design phase — the "add vs replace" distinction must be designed before building the service layer, because "add" and "replace" produce fundamentally different calculations and different UI affordances.
Global item database phase. Must come after multi-user data model is stable.
---
### Pitfall 7: Schema Change Adds Columns Without Updating Test Helper
### Pitfall 7: Image Storage Migration Breaks Existing URLs and the MCP Tool
**What goes wrong:**
v1.3 requires adding `sortOrder`, `pros`, and `cons` to `thread_candidates`. The developer updates `src/db/schema.ts`, runs `bun run db:push`, and builds the feature. Tests fail with cryptic "no such column" errors — or worse, tests pass silently because they do not exercise the new columns, while the real database has them.
Images are stored in `./uploads/` on the filesystem, served via `app.use("/uploads/*", serveStatic({ root: "./" }))`, and referenced by `imageFilename` in the database. Moving to object storage changes URLs from `/uploads/uuid.jpg` to `https://bucket.s3.region.amazonaws.com/uuid.jpg`. Every existing `imageFilename` reference becomes a broken image.
This pitfall already existed in v1.2 (documented in the previous PITFALLS.md) and the test helper (`tests/helpers/db.ts`) uses raw CREATE TABLE SQL that must be manually kept in sync.
Both `items` and `threadCandidates` have `imageFilename` and `imageSourceUrl` fields. The MCP tool `upload_image_from_url` saves to the local filesystem. The image route `POST /api/images` saves to `./uploads/`.
**Why it happens:**
Under time pressure, the developer focuses on the feature and forgets the test helper update. The error only surfaces when the new service function is called in a test. The CLAUDE.md documents this requirement, but it is easy to miss in the flow of development.
The current design stores only the filename, not the full URL. The serving path is implicit (prepend `/uploads/`). When storage moves to S3, the "prepend `/uploads/`" pattern breaks.
**How to avoid:**
For every schema change in v1.3, update `tests/helpers/db.ts` in the same commit:
- `thread_candidates`: add `sort_order REAL DEFAULT 0`, `pros TEXT`, `cons TEXT`
- Run `bun test` immediately after schema + helper update, before writing any other code
Consider writing a schema-parity test: compare the columns returned by `PRAGMA table_info(thread_candidates)` against a known expected list, failing if they differ. This catches the test-helper-out-of-sync problem automatically.
1. Add a reverse proxy route: keep `/uploads/*` working but proxy to S3 instead of local filesystem. This maintains backward compatibility during transition.
2. Or migrate `imageFilename` to store full URLs. Existing filenames get prefixed with the S3 URL during data migration.
3. Write a migration script that uploads all `./uploads/` files to S3 and updates database references.
4. Update `POST /api/images`, `POST /api/images/from-url`, and the MCP `upload_image_from_url` tool to write to S3.
5. Create an image storage abstraction layer so dev can use local filesystem and production uses S3.
**Warning signs:**
- Tests failing with `SqliteError: no such column`
- New service function works in the running app but throws in `bun test`
- `bun run db:push` was run but `bun test` was not run afterward
- `tests/helpers/db.ts` has fewer columns than `src/db/schema.ts`
- Broken images after deployment.
- Mixed URLs (some `/uploads/`, some `https://s3...`) in the database.
- MCP tool `upload_image_from_url` silently failing.
**Phase to address:**
Every schema-touching phase of v1.3. The candidate ranking schema phase (sortOrder, pros, cons) is the primary risk. Check test helper parity as an explicit completion criterion.
Infrastructure phase. Should be done before discovery/public profiles (which serve images to many users).
---
### Pitfall 8: Comparison View Includes Resolved Candidates
### Pitfall 8: Thread Resolution Creates Items Without Proper User Scoping
**What goes wrong:**
After a thread is resolved (winner picked), the thread's candidates still exist in the database. The comparison view, if it loads candidates from the existing `getThreadWithCandidates` response without filtering, will display the resolved winner alongside all losers — including the now-irrelevant candidates. A user revisiting a resolved thread to check why they picked Option A sees all candidates re-listed in the comparison view with no indication of which was selected, creating confusion.
Thread resolution copies a candidate's data into a new item. In multi-user, the newly created item must inherit the thread owner's `userId`. If the resolution logic does not explicitly set `userId` on the new item, it either fails (NOT NULL constraint) or creates an orphaned item.
The secondary problem: if the app allows drag-to-reorder ranking on a resolved thread, a user could accidentally fire rank-update mutations on a thread that should be read-only.
This is a specific instance of Pitfall 1 but deserves its own callout because resolution is a multi-step transaction: update thread status, set `resolvedCandidateId`, create new item. Any step that forgets `userId` breaks the chain.
**Why it happens:**
The comparison view and ranking components are built for active threads and tested only with active threads. Resolved thread behavior is not considered during design.
The resolution logic is tested as a unit but the test does not set a `userId` because none existed. After adding `userId`, the test still passes if using a default/NULL value. The bug only surfaces with a second user.
**How to avoid:**
1. Check `thread.status === "resolved"` before rendering the comparison/ranking UI. For resolved threads, render a read-only summary: "You chose [winner name]" with the winning candidate highlighted and others shown as non-interactive.
2. Disable drag-to-reorder on resolved threads entirely — don't render the drag handles.
3. In the impact preview, disable the "Impact on Setup" panel for resolved threads and instead show "Added to collection on [date]" for the winning candidate.
4. The API route for rank updates should reject requests for resolved threads (return 400 with "Thread is resolved").
1. Make `userId` NOT NULL on all entity tables from day one.
2. Update `resolveThread` to accept and propagate `userId`.
3. Write a test: resolve thread as User A, verify created item belongs to User A.
**Warning signs:**
- Comparison/ranking UI renders identically for active and resolved threads
- Drag handles are visible on resolved thread candidates
- No `thread.status` check in the comparison view component
- Resolved threads accept rank update mutations
- Items appearing in the wrong user's collection after resolution.
- Thread resolution failing with constraint violations.
**Phase to address:**
Comparison UI phase and ranking phase — both must include a resolved-thread guard. This is a correctness issue, not just a UX issue, because drag mutations on resolved threads corrupt state.
Multi-user data model phase.
---
### Pitfall 9: Public Content Without Explicit Privacy Controls
**What goes wrong:**
The v2.0 plan includes "public user profiles with shared setups" and a "discovery feed." Without explicit visibility controls, the default state is ambiguous: are new setups public? Are all items in a public setup visible? Can someone discover gear a user has not chosen to share? Users expecting a private gear tracker are surprised when their collection appears in search results.
**Why it happens:**
The developer defaults to "everything public" because it is simpler to build discovery features. Privacy controls are added as an afterthought, requiring a retroactive audit of all existing data.
**How to avoid:**
1. Default to private. Every entity (setup, profile) is private unless explicitly published.
2. Add a `visibility` column (`private` | `public`) to setups. Items are visible publicly only through public setups.
3. User profiles are private by default. Public profile is opt-in.
4. Public API endpoints (discovery, search) only query entities with `visibility = 'public'`.
5. Build the visibility model in the data layer before building any discovery UI.
**Warning signs:**
- No `visibility` or `isPublic` column in the schema.
- Discovery queries that do not filter by visibility.
- User complaints about unexpected data exposure.
**Phase to address:**
Multi-user data model phase (add visibility columns) and discovery phase (enforce in queries).
---
### Pitfall 10: SQLite-Specific Patterns That Silently Break on Postgres
**What goes wrong:**
The codebase has SQLite-specific patterns that will not error but will behave differently on Postgres:
- `src/db/index.ts` runs `PRAGMA journal_mode = WAL` and `PRAGMA foreign_keys = ON` -- Postgres has no PRAGMAs. Foreign keys are always enforced. WAL is always on.
- `bun:sqlite` is used as the driver. Postgres needs `postgres` (postgres.js) or `pg` (node-postgres) as the driver.
- The existing Drizzle migrator import is `drizzle-orm/bun-sqlite/migrator`. Postgres uses `drizzle-orm/node-postgres/migrator` or `drizzle-orm/postgres-js/migrator`.
- SQLite allows inserting strings into integer columns silently. Postgres will error.
- SQLite `AUTOINCREMENT` guarantees IDs never reuse. Postgres `serial` reuses IDs after deletions if the sequence is not explicitly configured.
- The test helper's `Database(":memory:")` has no Postgres equivalent without PGlite.
**Why it happens:**
These patterns are invisible in a working SQLite app. They only surface during or after the switch, often as runtime errors in production.
**How to avoid:**
1. Remove all PRAGMA statements when switching to Postgres.
2. Replace `bun:sqlite` driver with `postgres` (postgres.js is recommended for Bun compatibility).
3. Update all migrator imports.
4. Run the full test suite against Postgres to catch type strictness differences.
5. Use `serial` or `identity` columns for auto-increment; accept that IDs may be reused after deletion (this should not matter for a web app).
**Warning signs:**
- "PRAGMA" in the Postgres codebase.
- `bun:sqlite` imports anywhere in production code after migration.
- Tests passing against SQLite but failing against Postgres.
**Phase to address:**
Database migration phase.
---
### Pitfall 11: Setup-Item Delete-All-Reinsert Pattern Causes Phantom Reads
**What goes wrong:**
The current setup item sync uses delete-all-then-re-insert: `DELETE FROM setup_items WHERE setupId = X`, then re-insert all items. In single-user SQLite this is fine. In multi-user Postgres with concurrent writes: (a) race conditions if two users modify setups simultaneously, (b) brief windows where a public setup appears empty to concurrent readers.
**Why it happens:**
The pattern was chosen for simplicity (noted in CLAUDE.md: "Simpler than diffing, atomic in transaction"). "Atomic in transaction" only holds if the transaction isolation level prevents phantom reads, which is not the default in Postgres (`READ COMMITTED`).
**How to avoid:**
1. Wrap in an explicit transaction with `SERIALIZABLE` or `REPEATABLE READ` isolation for the sync operation.
2. Or switch to diff-based approach for public setups: compare existing vs. new list, delete removed, insert added.
3. For private setups, the delete-reinsert pattern with a basic transaction is acceptable.
**Warning signs:**
- Public setups briefly appearing empty.
- Foreign key violations in concurrent scenarios.
**Phase to address:**
Multi-user data model phase, when updating the setup service.
---
### Pitfall 12: Existing Data Has No Owner After Multi-User Migration
**What goes wrong:**
The existing SQLite database has items, categories, threads, setups -- all without a `userId` column. When the schema adds `userId NOT NULL`, the existing data needs an owner. If the migration script does not assign existing data to the original user, the data is either lost (NOT NULL violation prevents migration) or orphaned.
**Why it happens:**
The developer writes the new schema with `userId NOT NULL`, runs `db:push`, and the migration fails because existing rows have no `userId`. The "fix" is to make `userId` nullable, which undermines the entire data isolation model.
**How to avoid:**
1. The data migration script must: (a) create the original user in the new system, (b) assign all existing data to that user's ID, (c) then apply the NOT NULL constraint.
2. Migration order: create tables with `userId` nullable, insert data with the owner's userId, then ALTER to NOT NULL.
3. Verify row counts match before and after migration.
**Warning signs:**
- `userId` column is nullable in the final schema "because of migration."
- Existing data missing after migration.
- Migration script that only handles schema, not data.
**Phase to address:**
Database migration phase, specifically the data migration step.
---
## Technical Debt Patterns
Shortcuts that seem reasonable but create long-term problems.
| Shortcut | Immediate Benefit | Long-term Cost | When Acceptable |
|----------|-------------------|----------------|-----------------|
| Integer `sortOrder` instead of fractional | Simple schema | Bulk UPDATE on every reorder; bulk write latency with 10+ candidates | Acceptable only if max candidates per thread is enforced at 5 or fewer |
| Server-side delta calculation endpoint | Simpler client code | Third cache entry to invalidate; network round-trip on every setup selection change | Never — the calculation is two subtractions using data already in client cache |
| Pros/cons as unstructured free-text blobs | Zero schema complexity | Comparison view cannot render bullets; columns misalign | Never for comparison display — use newline-delimited format from day one |
| Comparison grid with `overflow: hidden` on narrow viewports | Avoids horizontal scroll complexity | Comparison becomes unreadable on laptop with panels open; critical feature breaks | Never — horizontal scroll is the correct behavior for comparison tables |
| Rendering comparison for resolved threads without guard | Simpler component logic | Users can drag-reorder resolved threads, corrupting state | Never — the resolved-thread guard is a correctness requirement |
| `DragOverlay` using same component as `useSortable` | Less component code | ID collision in dnd-kit causes undefined behavior during drag | Never — dnd-kit explicitly requires a separate presentational component for DragOverlay |
---
| Keeping SQLite test infrastructure while developing Postgres features | Tests keep passing during migration | Two database dialects to maintain, false confidence from tests that do not match production | Never -- migrate tests alongside schema |
| Storing both old `/uploads/` paths and new S3 URLs | Avoid data migration script | Every image-rendering component handles both URL formats forever | Only as a 1-2 week transition |
| Using `userId` as nullable during migration | Existing data does not need backfilling | Every query must handle NULL userId, privacy bugs when userId is missing | Only during the migration transaction itself, then enforce NOT NULL |
| Skipping RLS and relying only on app-level userId filtering | Faster to implement | Single missed WHERE clause = data leak | Never for multi-user platforms |
| Deferring visibility controls to "after discovery ships" | Ship discovery faster | Retroactive privacy audit, potential data exposure, user trust damage | Never |
| Keeping the local `users` table password hash after external auth | Avoid migration complexity | Dead column confuses future developers, potential security liability | Never -- remove password hash column after auth migration |
## Integration Gotchas
Common mistakes when connecting new v1.3 features to existing v1.2 systems.
| Integration Point | Common Mistake | Correct Approach |
|-------------------|----------------|------------------|
| Ranking + React Query | Using `setQueryData` alone for optimistic reorder, causing flicker | Maintain `tempItems` local state in the drag component; render from `tempItems ?? queryData.candidates`; clear on `onSettled` |
| Impact preview + weight unit | Computing delta in grams but displaying with `formatWeight` that expects the stored unit | Delta is always computed in grams (raw stored values); apply `formatWeight(delta, unit)` once at display time, same pattern as all other weight displays |
| Impact preview + null weights | Treating `null` weightGrams as 0 in delta calculation | Show "-- (no weight data)" explicitly; never pass null to arithmetic; guard with `candidate.weightGrams != null && setup.totalWeight != null` |
| Pros/cons + thread resolution | Pros/cons text copied to collection item on resolve | Do NOT copy pros/cons to the items table — these are planning notes, not collection metadata. `resolveThread` in `thread.service.ts` should remain unchanged |
| Rank order + existing `getThreadWithCandidates` | Adding `ORDER BY sort_order` to `getThreadWithCandidates` changes the order of an existing query used by other components | Add `sort_order` to the SELECT and ORDER BY in `getThreadWithCandidates`. Audit all consumers of this query to verify they are unaffected by ordering change (the candidate cards already render in whatever order the query returns) |
| Comparison view + `isActive` prop | `CandidateCard.tsx` uses `isActive` to show/hide the "Winner" button. Comparison view must not show "Winner" button inline if comparison has its own resolve affordance | Pass `isActive={false}` to `CandidateCard` when rendering inside comparison view, or create a separate `CandidateComparisonCard` presentational component that omits action buttons |
---
| Integration | Common Mistake | Correct Approach |
|-------------|----------------|------------------|
| External auth provider | Removing the local `users` table entirely | Keep a local `users` table with `externalId` (from auth provider) + local fields (preferences, API keys). Foreign keys reference local `users.id`, not the external provider's UUID. |
| External auth provider | Storing user profile data in the auth provider and querying it at runtime | Store only identity in auth provider. Sync user profile to local `users` table on login. Application queries local table only. |
| External auth provider | Using auth provider's session tokens directly as API authentication | Auth provider handles login/logout. Application mints its own session after verifying the auth provider's token. This decouples session lifecycle from the provider. |
| S3-compatible object storage | Using the S3 SDK directly in route handlers | Create an image storage abstraction (interface with `upload`, `getUrl`, `delete`). Swap implementations (local filesystem for dev, S3 for production) via environment config. |
| Postgres driver | Assuming `bun:sqlite` patterns work with Postgres | Postgres uses `postgres` (postgres.js) or `pg`. Connection pooling, async queries, and error handling differ. SQLite is synchronous; Postgres is async. Service functions may need to become async. |
| Postgres | Assuming SQLite PRAGMA behaviors exist | Postgres has no PRAGMAs. Foreign keys are always on. WAL is always on. Remove all PRAGMA code. |
| Drizzle ORM Postgres driver | Using synchronous `.get()` and `.all()` query methods | SQLite Drizzle uses `.get()` (sync). Postgres Drizzle uses `.execute()` or `await` on queries. Every service function that calls `.get()` or `.all()` must be updated. |
## Performance Traps
Patterns that work at small scale but fail as usage grows.
| Trap | Symptoms | Prevention | When It Breaks |
|------|----------|------------|----------------|
| Bulk integer rank updates on every drag | Visible latency after each drop; multiple UPDATE statements per drag; SQLite write lock held | Use fractional `sortOrder REAL` so only the moved item requires an UPDATE | 8+ candidates per thread with rapid dragging |
| Comparison view fetching all candidates for all threads | Slow initial load; excessive memory for large thread lists | Comparison view uses the already-loaded `["threads", threadId]` query; never fetches candidates outside the active thread's query | 20+ threads with 5+ candidates each |
| Sync rank updates on every `dragOver` event (not just `dragEnd`) | Thousands of UPDATE mutations during a single drag; server overwhelmed; UI lags | Persist rank only on `onDragEnd` (drop), never on `onDragOver` (in-flight hover) | Any usage — `onDragOver` fires on every cursor pixel moved |
| `useQuery` for setups list inside impact preview component | N+1 query pattern: each candidate card fetches its own setup list | Lift setup list query to the thread detail page level; pass selected setup as prop or context | 3+ candidates in comparison view |
---
| N+1 queries in discovery feed | Feed page takes 2+ seconds | Use joins or batch queries for setups with items and categories | 50+ setups in feed, each with 10+ items |
| Unindexed `userId` columns | All queries slow after adding userId filtering | Add indexes on `userId` for every table. Composite indexes for `(userId, categoryId)` on items. | 1000+ items across 50+ users |
| Full-table scans for aggregates | Dashboard slow for large collections | Current aggregates are computed via SQL on read. Add materialized views or cache for public setup totals. | 100+ items per user, or public setups viewed by 100+ visitors |
| Image serving from app server | Server CPU/bandwidth saturated | Serve images from S3/CDN. Current `serveStatic` for uploads hits the app server for every request. | 100+ concurrent users browsing image-heavy pages |
| Global product search without full-text index | Product search slow or inaccurate | Use Postgres full-text search (`tsvector`/`tsquery`) or `pg_trgm` trigram index. | 10,000+ products |
| Synchronous service functions on Postgres | Request timeouts, connection pool exhaustion | SQLite Drizzle is sync. Postgres Drizzle is async. Service functions that were sync must become async. | Any usage under load |
## Security Mistakes
Domain-specific security issues beyond general web security.
| Mistake | Risk | Prevention |
|---------|------|------------|
| `sortOrder` accepts any float value | Malformed values like `NaN`, `Infinity`, or extremely large floats stored in `sort_order` column, corrupting order | Validate `sortOrder` as a finite number in Zod schema: `z.number().finite()`. Reject `NaN` and `Infinity` at API boundary |
| Pros/cons fields with no length limit | Users or automated input can store multi-kilobyte text blobs, inflating the database and slowing candidate queries | Cap at 500 characters per field in Zod: `z.string().max(500).optional()` |
| Rank update endpoint accepts any candidateId | A crafted request can reorder candidates from a different thread by passing a candidateId that belongs to another thread | In the rank update service, verify `candidate.threadId === threadId` before applying the update — same pattern as existing `resolveThread` validation |
---
| No RLS, relying only on app-level userId filtering | Single missed WHERE clause exposes all user data | Enable Postgres RLS on all user-owned tables. App filtering is primary; RLS is safety net. |
| Public setup exposes private item details | Users share a setup but private notes/pricing leak | Public setup views project only public fields (name, weight, category). Define a "public item projection" and enforce it. |
| API keys not scoped to users after auth migration | API key created by User A operates on User B's data | API keys must associate with a userId. After validation, the key's userId scopes all operations. |
| Auth provider misconfigured for open self-registration | Random users create accounts without approval | Configure auth provider for admin-approval or invite-only registration. Test explicitly. |
| Image upload accepts any file type | Stored XSS via SVG uploads, executable content | Validate MIME type on upload (JPEG, PNG, WebP only). Set `Content-Type` and `Content-Disposition` headers. Strip EXIF metadata. |
| External auth provider callback URL not validated | OAuth redirect attack | Whitelist exact callback URLs in auth provider config. Never use wildcard redirect URIs. |
## UX Pitfalls
Common user experience mistakes in this domain.
| Pitfall | User Impact | Better Approach |
|---------|-------------|-----------------|
| Comparison table loses column headers on scroll | User scrolls down to see notes/pros/cons and forgets which column is which candidate | Sticky column headers with candidate name, image thumbnail, and weight. Use `position: sticky; top: 0` on the header row |
| Delta shows raw gram values when user prefers oz | Impact preview shows "+450g" to a user who has set their unit to oz | Apply `formatWeight(delta, unit)` using the `useWeightUnit()` hook, same as all other weight displays in the app |
| Drag-to-reorder with no visual rank indicator | After ranking, it is unclear that the order matters or that #1 is the "top pick" | Show rank numbers (1, 2, 3...) as badges on each candidate card when in ranking mode. Update numbers live during drag |
| Pros/cons fields empty by default in comparison view | Comparison table shows empty cells next to populated ones, making the comparison feel sparse and incomplete | Show a subtle "Add pros/cons" prompt in empty cells when the thread is active. In read-only resolved view, hide the pros/cons section entirely if no candidate has data |
| Impact preview setup selector defaults to no setup | User arrives at comparison view and sees no impact numbers because no setup is pre-selected | Default the setup selector to the most recently viewed/modified setup. Persist the last-selected setup in `sessionStorage` or a URL param |
| Removing a candidate clears comparison selection | User has candidates A, B, C in comparison; deletes C; comparison resets entirely | Comparison state (which candidates are selected) should be stored in local component state keyed by candidate ID. On delete, simply remove that ID from the selection |
---
| Forcing existing single user to re-register via external auth | User loses access to their own data until they figure out new login | Migration path: on first visit after upgrade, guide user to create auth provider account and automatically link to existing data. |
| Public profiles default to showing everything | Users surprised their gear list is public | Default profile to private. Public is opt-in with clear preview of what others see. |
| Review system with only star ratings | Ratings without context are useless for gear decisions | Structured reviews with predefined fields (durability, weight accuracy, value) per category. "Weight is 15g heavier than listed" is actionable; a 4-star rating is not. |
| Discovery feed dominated by one hobby | Users in other hobbies see irrelevant content | Category-based feed filtering. Show content relevant to user's categories. |
| No indication of data ownership when browsing others' setups | User tries to edit someone else's setup and gets error | Clear visual distinction between "my setup" and "someone else's setup." Read-only view with "copy to my setups" action. |
| Settings lost during migration | User's weight unit preference, onboarding state disappear | Migrate the `settings` table data alongside everything else. Map settings to the original user. |
## "Looks Done But Isn't" Checklist
Things that appear complete but are missing critical pieces.
- [ ] **Drag-to-reorder:** Often missing drag handles — verify the drag affordance is visually distinct (grip icon), not just "drag anywhere on the card" which conflicts with the existing click-to-edit behavior
- [ ] **Drag-to-reorder:** Often missing keyboard reorder fallback — verify candidates can be moved with arrow keys for accessibility (dnd-kit's `KeyboardSensor` must be added to `DndContext`)
- [ ] **Drag-to-reorder:** Often missing flicker fix — verify dropping a candidate does not briefly snap back to original position (requires `tempItems` local state, not just `setQueryData`)
- [ ] **Drag-to-reorder:** Often missing resolved-thread guard — verify drag handles are hidden and mutations are blocked on resolved threads
- [ ] **Impact preview:** Often missing the null weight case — verify candidates with no weight show "-- (no weight data)" not "NaNg" or "+0g"
- [ ] **Impact preview:** Often missing the replace-vs-add distinction — verify the user can specify which existing item would be replaced, not just see a pure addition delta
- [ ] **Impact preview:** Often missing unit conversion — verify the delta respects `useWeightUnit()` and `useCurrency()`, not hardcoded to grams/USD
- [ ] **Side-by-side comparison:** Often missing horizontal scroll on narrow viewports — verify the view is usable at 768px without column collapsing
- [ ] **Side-by-side comparison:** Often missing sticky headers — verify candidate names remain visible when scrolling the comparison rows
- [ ] **Pros/cons fields:** Often missing length validation — verify Zod schema caps the field and the textarea shows a character counter
- [ ] **Pros/cons display:** Often missing newline-to-bullet rendering — verify newlines in the stored text render as bullet points in the comparison view, not as `\n` characters
- [ ] **Schema changes:** Often missing test helper update — verify `tests/helpers/db.ts` includes `sort_order`, `pros`, and `cons` columns after the schema migration
---
- [ ] **Multi-user data model:** Often missing userId on the `settings` table -- verify settings are user-scoped (weight unit preference, onboarding state).
- [ ] **Multi-user data model:** Often missing userId filter on `threadCandidates` queries that join through `threads` -- verify candidates are not directly queryable across users.
- [ ] **Multi-user data model:** Often missing userId on thread resolution -- verify `resolveThread` propagates userId to the newly created item.
- [ ] **Auth migration:** Often missing MCP server auth update -- verify MCP tools operate in context of the authenticated user, not as global admin.
- [ ] **Auth migration:** Often missing E2E test auth update -- verify E2E tests authenticate against new auth system or use API keys.
- [ ] **Auth migration:** Often missing API key userId association -- verify API keys created after migration are scoped to the creating user.
- [ ] **Database migration:** Often missing data migration script -- verify existing SQLite data is actually moved to Postgres, not just the schema.
- [ ] **Database migration:** Often missing timestamp conversion -- verify SQLite integer timestamps are correctly handled in Postgres schema.
- [ ] **Database migration:** Often missing weight precision check -- verify `real()` vs `doublePrecision()` does not lose decimal precision.
- [ ] **Database migration:** Often missing sync-to-async conversion -- verify all service functions are async after Postgres switch.
- [ ] **Image migration:** Often missing MCP tool update -- verify `upload_image_from_url` writes to S3, not local filesystem.
- [ ] **Image migration:** Often missing `imageSourceUrl` field -- verify source URL metadata is preserved during migration.
- [ ] **Public content:** Often missing visibility filtering on aggregate endpoints -- verify `/api/totals` only counts requesting user's items.
- [ ] **Reviews:** Often missing rate limiting -- verify a user cannot submit 100 reviews in a minute.
- [ ] **Discovery feed:** Often missing pagination -- verify feed does not load all public setups at once.
- [ ] **Global items:** Often missing product-vs-item distinction -- verify adding a product to global database does not add it to anyone's collection.
## Recovery Strategies
When pitfalls occur despite prevention, how to recover.
| Pitfall | Recovery Cost | Recovery Steps |
|---------|---------------|----------------|
| Drag flicker due to no `tempItems` local state | LOW | Add `tempItems` state to the ranking component. Render from `tempItems ?? queryData.candidates`. No data migration needed. |
| Integer `sortOrder` causing bulk updates | MEDIUM | Add Drizzle migration to change `sort_order` column type from INTEGER to REAL. Update existing rows to spaced values (1000, 2000, 3000...). Update service layer to use fractional logic. |
| Delta treats null weight as 0 | LOW | Add null guards in the delta calculation component. No data changes needed. |
| Pros/cons stored as unformatted blobs | LOW | No migration needed — the data is still correct. Update the rendering component to split on newlines. Add length validation to the Zod schema for new input. |
| Comparison view visible on resolved threads | LOW | Add `if (thread.status === 'resolved') return <ResolvedView />` before rendering the comparison/ranking UI. Add 400 check in the rank update API route. |
| Test helper out of sync with schema | LOW | Update CREATE TABLE statements in `tests/helpers/db.ts`. Run `bun test`. Fix any test that relied on the old column count. |
| Rank update accepts cross-thread candidateId | LOW | Add `candidate.threadId !== threadId` guard in rank update service (same pattern as existing `resolveThread` guard). |
---
| Data leaked between users (missing userId filter) | HIGH | Audit all queries, add RLS immediately, notify affected users, review access logs. Reputation damage is the real cost. |
| Broken images after storage migration | MEDIUM | Keep old uploads directory as fallback. Re-upload missing images. Update database references. |
| Test suite broken for weeks during DB migration | MEDIUM | Pause feature work. Set up PGlite test infrastructure. Port tests one file at a time. |
| Auth migration breaks MCP server | LOW | MCP server can fall back to API key auth (already implemented). Fix isolated to MCP auth middleware. |
| Category unique constraint failures | LOW | Drop old unique constraint, add composite unique. Single transaction. |
| Weight precision loss (SQLite real to Postgres real) | LOW | Alter column to `doublePrecision`. One-time verification script. |
| Public data exposure before visibility controls | HIGH | Emergency: set all entities to private, deploy, then build visibility controls properly. Cannot undo exposure. |
| Existing data orphaned after migration | MEDIUM | Re-run data migration script with correct userId assignment. Verify row counts. |
| Service functions still sync after Postgres switch | MEDIUM | Systematic conversion of all service functions to async. Update all callers. TypeScript will catch most issues. |
## Pitfall-to-Phase Mapping
How roadmap phases should address these pitfalls.
| Pitfall | Prevention Phase | Verification |
|---------|------------------|--------------|
| dnd-kit + React Query flicker | Candidate ranking phase | Drop a candidate, verify no snap-back. Add automated test: mock drag end, verify list order reflects drop position immediately. |
| Bulk integer rank writes | Schema design for ranking | `sortOrder` column is `REAL` type in Drizzle schema. Service layer issues exactly one UPDATE per reorder. Test: reorder 5 candidates, verify only 1 DB write. |
| Stale data in impact preview | Impact preview phase | Change a candidate's weight, verify delta updates immediately. Select a different setup, verify delta recalculates from new baseline. |
| Comparison broken at narrow width | Comparison UI phase | Test at 768px viewport. Verify horizontal scroll is present and content is readable. No vertical stack of comparison columns. |
| Pros/cons as unstructured blobs | Ranking schema phase (when columns added) | Verify Zod schema caps at 500 chars. Verify comparison view renders newlines as bullets. Test: enter 3-line pros text, verify 3 bullets rendered. |
| Impact preview add vs replace | Impact preview design phase | Thread with same-category item in setup defaults to replace mode. Pure-add mode available as alternative. Test: replace mode shows negative delta when candidate is lighter. |
| Comparison/rank on resolved threads | Both comparison and ranking phases | Verify drag handles are absent on resolved threads. Verify rank update API returns 400 for resolved thread. |
| Test helper schema drift | Every schema-touching phase of v1.3 | After schema change, run `bun test` immediately. Zero test failures from column-not-found errors. |
---
| Missing userId filters (P1) | Multi-user data model | Integration tests: create as User A, query as User B, assert empty. RLS policies active. |
| Category uniqueness (P2) | Multi-user data model | Two users create identically-named categories without constraint violations. |
| Drizzle schema rewrite (P3) | Database migration | Schema compiles with pg-core. drizzle-kit generates valid Postgres migrations. Weight values maintain precision. |
| Test infrastructure collapse (P4) | Database migration | `bun test` passes with PGlite. E2E tests pass against Postgres. No SQLite imports in test code. |
| Auth provider breaks sessions/keys (P5) | Auth migration | Existing API keys work. MCP server authenticates. E2E tests pass. First-time setup works via external provider. |
| Global item data model fork (P6) | Global item database | Separate `products` table exists. User items optionally reference a product. CRUD operations distinct. |
| Image URL breakage (P7) | Infrastructure / Image storage | Existing images render. New uploads go to S3. MCP upload tool works. |
| Thread resolution userId (P8) | Multi-user data model | Resolving a thread creates an item owned by the thread's owner. Tested with multiple users. |
| Privacy/visibility (P9) | Multi-user data model + Discovery | Default is private. Public queries filter by visibility. No private data in discovery feed. |
| SQLite-specific patterns (P10) | Database migration | No PRAGMAs in codebase. No bun:sqlite imports. All queries async. |
| Setup sync race conditions (P11) | Multi-user data model | Concurrent setup modifications do not produce empty setups or constraint violations. |
| Existing data ownership (P12) | Database migration | All existing data assigned to original user. Row counts match. userId NOT NULL enforced. |
## Sources
- [dnd-kit Discussion #1522: React Query + DnD flicker](https://github.com/clauderic/dnd-kit/discussions/1522) — `tempItems` solution for React Query cache flicker
- [dnd-kit Issue #921: Sorting not working with React Query](https://github.com/clauderic/dnd-kit/issues/921) — Root cause of the state lifecycle mismatch
- [dnd-kit Sortable Docs: OptimisticSortingPlugin](https://dndkit.com/concepts/sortable) — New API for handling optimistic reorder
- [TanStack Query: Optimistic Updates guide](https://tanstack.com/query/v4/docs/react/guides/optimistic-updates) — `onMutate`/`onSettled` rollback patterns
- [Fractional Indexing: Steveruiz.me](https://www.steveruiz.me/posts/reordering-fractional-indices) — Why fractional keys beat integer reorder for databases
- [Fractional Indexing SQLite library](https://github.com/sqliteai/fractional-indexing) — Implementation reference for base62 lexicographic sort keys
- [Baymard Institute: Comparison Tool Design](https://baymard.com/blog/user-friendly-comparison-tools) — Sticky headers, horizontal scroll, minimum column width for product comparison UX
- [NN/G: Comparison Tables](https://www.nngroup.com/articles/comparison-tables/) — Avoid prose in comparison cells; use scannable structured values
- [LogRocket: When comparison charts hurt UX](https://blog.logrocket.com/ux-design/feature-comparison-tips-when-not-to-use/) — Comparison table anti-patterns
- Direct codebase analysis of GearBox v1.2 (schema.ts, thread.service.ts, setup.service.ts, CandidateCard.tsx, useSetups.ts, useCandidates.ts, tests/) — existing patterns, integration points, and established conventions
- Direct codebase analysis of GearBox v1.4 (schema.ts, services, auth middleware, MCP server, test helpers, db/index.ts, E2E seed)
- [Drizzle ORM PostgreSQL documentation](https://orm.drizzle.team/docs/get-started/postgresql-new)
- [Drizzle ORM SQLite column types](https://orm.drizzle.team/docs/column-types/sqlite)
- [Drizzle ORM migrations documentation](https://orm.drizzle.team/docs/migrations)
- [SQLite to PostgreSQL migration pitfalls (Open WebUI discussion)](https://github.com/open-webui/open-webui/discussions/21609)
- [How to migrate from SQLite to PostgreSQL (Render)](https://render.com/articles/how-to-migrate-from-sqlite-to-postgresql)
- [Multi-tenant architecture guide (WorkOS)](https://workos.com/blog/developers-guide-saas-multi-tenant-architecture)
- [Multi-tenant vs single-tenant SaaS (Clerk)](https://clerk.com/blog/multi-tenant-vs-single-tenant)
- [Migrating file storage to Amazon S3 (DZone)](https://dzone.com/articles/migrating-file-storage-to-amazon-s3)
- [Drizzle ORM PostgreSQL best practices 2025 (GitHub Gist)](https://gist.github.com/productdevbook/7c9ce3bbeb96b3fabc3c7c2aa2abc717)
---
*Pitfalls research for: GearBox v1.3 — Research & Decision Tools (side-by-side comparison, impact preview, candidate ranking)*
*Researched: 2026-03-16*
*Pitfalls research for: GearBox v2.0 -- Single-user to multi-user platform migration*
*Researched: 2026-04-03*