diff --git a/.planning/phases/10-schema-foundation-pros-cons-fields/10-RESEARCH.md b/.planning/phases/10-schema-foundation-pros-cons-fields/10-RESEARCH.md new file mode 100644 index 0000000..fea7fcd --- /dev/null +++ b/.planning/phases/10-schema-foundation-pros-cons-fields/10-RESEARCH.md @@ -0,0 +1,417 @@ +# Phase 10: Schema Foundation + Pros/Cons Fields - Research + +**Researched:** 2026-03-16 +**Domain:** Drizzle ORM schema migration + full-stack field addition (SQLite / Hono / React) +**Confidence:** HIGH + +--- + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|-----------------| +| RANK-03 | User can add pros and cons text per candidate displayed as bullet lists | Confirmed: two nullable TEXT columns on `thread_candidates` + textarea inputs in `CandidateForm` + visual indicator on `CandidateCard` | + + +--- + +## Summary + +Phase 10 is a contained, top-to-bottom field-addition task. Two nullable `TEXT` columns (`pros`, `cons`) must be added to the `thread_candidates` table, propagated through every layer that touches that table, and surfaced in the UI as editable text areas with a card-level presence indicator. + +The project uses Drizzle ORM with SQLite. Adding nullable columns via `ALTER TABLE … ADD COLUMN` is safe in SQLite (no default value is required for nullable TEXT). The Drizzle workflow is: edit `schema.ts` → `bun run db:generate` → `bun run db:push`. The generated SQL migration follows the established pattern already used four times in this project. + +There is one mandatory non-obvious step documented in CLAUDE.md: the test helper at `tests/helpers/db.ts` contains a hardcoded `CREATE TABLE thread_candidates` statement that mirrors the production schema. It must be updated in lockstep with `schema.ts` or all existing candidate tests will silently omit the new columns and new service-level tests will fail. + +**Primary recommendation:** Follow the exact field-addition ladder: schema → migration → test helper → service (insert + update + select projection) → Zod schemas → shared types → API route (zValidator) → React hook response type → CandidateForm → CandidateCard indicator. Every rung must be touched — skipping any one causes type drift or runtime failures. + +--- + +## Standard Stack + +### Core + +| Library | Version | Purpose | Why Standard | +|---------|---------|---------|--------------| +| drizzle-orm | installed | ORM + migration generation | Project standard; all migrations use it | +| drizzle-kit | installed | CLI for `db:generate` | Project standard; configured in drizzle.config.ts | +| zod | installed | Schema validation on API boundary | Project standard; `@hono/zod-validator` integration | +| bun:sqlite | runtime built-in | In-memory test DB | Used by `createTestDb()` helper | + +No new dependencies are required for this phase. + +**Installation:** +```bash +# No new packages — all required libraries already installed +``` + +--- + +## Architecture Patterns + +### Established Field-Addition Ladder + +Every field addition in this codebase follows this exact sequence. Previous examples: `status` on candidates, `classification` on `setup_items`, `icon` on categories. + +``` +1. src/db/schema.ts — Drizzle column definition +2. drizzle/ (generated) — bun run db:generate +3. gearbox.db — bun run db:push +4. tests/helpers/db.ts — Raw SQL CREATE TABLE mirrored manually +5. src/server/services/thread.service.ts + a. createCandidate() — values() object + b. updateCandidate() — data type + set() + c. getThreadWithCandidates() — explicit column projection +6. src/shared/schemas.ts — createCandidateSchema + updateCandidateSchema +7. src/shared/types.ts — auto-inferred (no manual edit needed) +8. src/client/hooks/useCandidates.ts — CandidateResponse interface +9. src/client/components/CandidateForm.tsx — FormData + textarea inputs + payload +10. src/client/components/CandidateCard.tsx — visual indicator prop + render +``` + +### Pattern 1: Drizzle Nullable Text Column + +**What:** Add an optional text field to an existing Drizzle table. +**When to use:** When the field is user-provided text, no business logic default applies. + +```typescript +// Source: src/db/schema.ts — pattern already used by notes, productUrl, imageFilename +export const threadCandidates = sqliteTable("thread_candidates", { + // ... existing columns ... + pros: text("pros"), // nullable, no default — mirrors notes/productUrl pattern + cons: text("cons"), // nullable, no default + // ... +}); +``` + +### Pattern 2: Test Helper Table Synchronization + +**What:** Mirror every new column in the raw SQL inside `createTestDb()`. +**When to use:** Every time `schema.ts` is modified. Documented as mandatory in CLAUDE.md. + +```typescript +// Source: tests/helpers/db.ts — existing thread_candidates CREATE TABLE +sqlite.run(` + CREATE TABLE thread_candidates ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + thread_id INTEGER NOT NULL REFERENCES threads(id) ON DELETE CASCADE, + name TEXT NOT NULL, + weight_grams REAL, + price_cents INTEGER, + category_id INTEGER NOT NULL REFERENCES categories(id), + notes TEXT, + product_url TEXT, + image_filename TEXT, + status TEXT NOT NULL DEFAULT 'researching', + pros TEXT, -- ADD THIS + cons TEXT, -- ADD THIS + created_at INTEGER NOT NULL DEFAULT (unixepoch()), + updated_at INTEGER NOT NULL DEFAULT (unixepoch()) + ) +`); +``` + +### Pattern 3: Explicit Select Projection in Service + +**What:** `getThreadWithCandidates` uses an explicit `.select({...})` projection, not `select()`. +**When to use:** New columns MUST be explicitly added to the projection or they will not appear in query results. + +```typescript +// Source: src/server/services/thread.service.ts — getThreadWithCandidates() +const candidateList = db + .select({ + // ... existing fields ... + pros: threadCandidates.pros, // ADD + cons: threadCandidates.cons, // ADD + categoryName: categories.name, + categoryIcon: categories.icon, + }) + .from(threadCandidates) + // ... +``` + +### Pattern 4: Zod Schema Extension + +**What:** Add optional string fields to `createCandidateSchema`; `updateCandidateSchema` is derived via `.partial()` and picks them up automatically. +**When to use:** Any new candidate API field. + +```typescript +// Source: src/shared/schemas.ts +export const createCandidateSchema = z.object({ + name: z.string().min(1, "Name is required"), + // ... existing fields ... + pros: z.string().optional(), // ADD + cons: z.string().optional(), // ADD +}); + +// updateCandidateSchema = createCandidateSchema.partial() — inherits automatically +``` + +### Pattern 5: CandidateForm Textarea Addition + +**What:** Extend `FormData` interface and `INITIAL_FORM` constant, add pre-fill in `useEffect`, add textarea elements, include in payload. + +```typescript +// Source: src/client/components/CandidateForm.tsx — FormData interface +interface FormData { + // ... existing ... + pros: string; // ADD + cons: string; // ADD +} + +const INITIAL_FORM: FormData = { + // ... existing ... + pros: "", // ADD + cons: "", // ADD +}; + +// In useEffect pre-fill: +pros: candidate.pros ?? "", // ADD +cons: candidate.cons ?? "", // ADD + +// In payload construction: +pros: form.pros.trim() || undefined, // ADD +cons: form.cons.trim() || undefined, // ADD +``` + +### Pattern 6: CandidateCard Visual Indicator + +**What:** Show a small badge when a candidate has pros or cons text. The requirement says "visual indicator when a candidate has pros or cons entered" — not a full display of the text (that is the form's job). +**When to use:** When `(pros || cons)` is truthy. + +```tsx +// Source: src/client/components/CandidateCard.tsx — props interface +interface CandidateCardProps { + // ... existing ... + pros?: string | null; // ADD + cons?: string | null; // ADD +} + +// In the card's badge section (alongside weight/price badges): +{(pros || cons) && ( + + Notes + +)} +``` + +The exact styling (color, icon, text) is left to the planner's discretion — the requirement only specifies "visual indicator." + +### Anti-Patterns to Avoid + +- **Forgetting the test helper**: If `tests/helpers/db.ts` is not updated, the in-memory schema won't have `pros`/`cons` columns. Tests that insert or read these fields will get `undefined` instead of the stored value, causing silent failures or column-not-found errors. CLAUDE.md documents this as a known hazard. +- **Using `select()` without explicit fields**: The `getThreadWithCandidates` service function already uses an explicit projection. Adding fields to the schema without adding them to the projection means the client never receives the data. +- **Storing pros/cons as a JSON array of bullet strings**: The requirement says "text per candidate displayed as bullet lists" — the display can parse newlines into bullets from a plain TEXT field. A single multi-line `TEXT` column is correct and consistent with the existing `notes` field pattern. No JSON, no separate table. +- **Adding a separate `candidatePros` / `candidateCons` table**: Massive over-engineering. These are simple annotations on a single candidate, not a many-per-candidate relationship. + +--- + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Schema migration | Custom SQL scripts | `bun run db:generate` + `bun run db:push` | Drizzle-kit generates correct ALTER TABLE, tracks journal, handles snapshot | +| API input validation | Manual checks | Zod via `zValidator` (already wired) | All candidate routes already use `updateCandidateSchema` — just extend it | +| Bullet-list rendering | Custom tokenizer | CSS `whitespace-pre-line` or split on `\n` | Simple text with newlines is sufficient for RANK-03 | + +--- + +## Common Pitfalls + +### Pitfall 1: Test Helper Column Drift + +**What goes wrong:** New columns exist in production schema but are absent from the hardcoded `CREATE TABLE` in `tests/helpers/db.ts`. Tests pass structurally but new-column values are lost. +**Why it happens:** The test helper duplicates the schema in raw SQL, not via Drizzle. There is no automated sync. +**How to avoid:** Update `tests/helpers/db.ts` immediately after editing `schema.ts`, in the same commit wave. +**Warning signs:** `candidate.pros` returns `undefined` in service tests even after saving a value. + +### Pitfall 2: Missing Explicit Column in Select Projection + +**What goes wrong:** `getThreadWithCandidates` uses `.select({ id: threadCandidates.id, ... })` — an explicit map. New columns are silently excluded. +**Why it happens:** Drizzle's explicit projection doesn't automatically include newly-added columns. +**How to avoid:** Search for every `.select({` that references `threadCandidates` and add `pros` and `cons`. +**Warning signs:** API returns candidate without `pros`/`cons` fields even though they're saved in DB. + +### Pitfall 3: updateCandidate Service Type Mismatch + +**What goes wrong:** `updateCandidate` in `thread.service.ts` has a hardcoded `Partial<{ name, weightGrams, ... }>` type rather than using the Zod-inferred type. New fields must be manually added to this inline type. +**Why it happens:** The function was written with an inline type, not `UpdateCandidate`. +**How to avoid:** Add `pros: string` and `cons: string` to the `Partial<{...}>` inline type in `updateCandidate`. +**Warning signs:** TypeScript error when trying to set `pros`/`cons` in the `.set({...data})` call. + +### Pitfall 4: CandidateCard Prop Not Threaded Through Call Sites + +**What goes wrong:** `CandidateCard` receives new `pros`/`cons` props, but the parent component (the thread detail page / candidate list) doesn't pass them. +**Why it happens:** Adding props to a component doesn't update callers. +**How to avoid:** Search for all `.sql +ALTER TABLE `thread_candidates` ADD `pros` text; +ALTER TABLE `thread_candidates` ADD `cons` text; +``` + +SQLite supports `ADD COLUMN` for nullable columns without a default. Confirmed by existing migration pattern (`0003_misty_mongu.sql` uses `ALTER TABLE setup_items ADD classification text DEFAULT 'base' NOT NULL`). + +### Service: createCandidate Values + +```typescript +// Source: src/server/services/thread.service.ts +return db + .insert(threadCandidates) + .values({ + threadId, + name: data.name, + // ... existing fields ... + pros: data.pros ?? null, // ADD + cons: data.cons ?? null, // ADD + }) + .returning() + .get(); +``` + +### Service: updateCandidate Inline Type + +```typescript +// Source: src/server/services/thread.service.ts +export function updateCandidate( + db: Db = prodDb, + candidateId: number, + data: Partial<{ + name: string; + weightGrams: number; + priceCents: number; + categoryId: number; + notes: string; + productUrl: string; + imageFilename: string; + status: "researching" | "ordered" | "arrived"; + pros: string; // ADD + cons: string; // ADD + }>, +) { ... } +``` + +### Hook: CandidateResponse Interface + +```typescript +// Source: src/client/hooks/useCandidates.ts +interface CandidateResponse { + id: number; + // ... existing ... + pros: string | null; // ADD + cons: string | null; // ADD +} +``` + +--- + +## State of the Art + +| Old Approach | Current Approach | Notes | +|--------------|------------------|-------| +| Manual SQL migrations | Drizzle-kit generate + push | Already established — 4 migrations in project | +| `notes` as freeform text | `pros`/`cons` as separate nullable TEXT columns | Matches how existing `notes` field works; no special type | + +**Not applicable in this phase:** +- No new libraries +- No breaking API changes (all new fields are optional) +- Existing candidates will have `pros = null` and `cons = null` after migration — no backfill needed + +--- + +## Open Questions + +1. **Bullet list rendering in CandidateCard** + - What we know: RANK-03 says "displayed as bullet lists" + - What's unclear: The card currently shows the pros/cons indicator; does the card need to render the actual bullets, or does that happen elsewhere (e.g., a tooltip, expanded state, or comparison view in Phase 12)? + - Recommendation: Phase 10 success criteria only requires "visual indicator when a candidate has pros or cons entered." Full bullet rendering can be deferred to Phase 12 (Comparison View) or Phase 11. The form's edit view can display raw textarea text. + +2. **Maximum text length** + - What we know: SQLite TEXT has no practical length limit; the existing `notes` field has no validation constraint + - What's unclear: Should pros/cons have a max length? + - Recommendation: Omit length constraint to stay consistent with the `notes` field. Add if user feedback indicates issues. + +--- + +## Validation Architecture + +`workflow.nyquist_validation` is `true` in `.planning/config.json`. + +### Test Framework + +| Property | Value | +|----------|-------| +| Framework | Bun test runner (built-in) | +| Config file | None — `bun test` discovers `tests/**/*.test.ts` | +| Quick run command | `bun test tests/services/thread.service.test.ts` | +| Full suite command | `bun test` | + +### Phase Requirements → Test Map + +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| RANK-03 | `createCandidate` stores pros/cons and returns them | unit | `bun test tests/services/thread.service.test.ts` | Extend existing | +| RANK-03 | `updateCandidate` can set/clear pros and cons | unit | `bun test tests/services/thread.service.test.ts` | Extend existing | +| RANK-03 | `getThreadWithCandidates` returns pros/cons on each candidate | unit | `bun test tests/services/thread.service.test.ts` | Extend existing | +| RANK-03 | `PUT /api/threads/:id/candidates/:id` accepts pros/cons in body | route | `bun test tests/routes/threads.test.ts` | Extend existing | +| RANK-03 | All existing tests pass (no column drift) | regression | `bun test` | Existing ✅ | + +### Sampling Rate + +- **Per task commit:** `bun test tests/services/thread.service.test.ts` +- **Per wave merge:** `bun test` +- **Phase gate:** Full suite green before `/gsd:verify-work` + +### Wave 0 Gaps + +No new test files need to be created. All tests are extensions of existing files: +- `tests/services/thread.service.test.ts` — add `pros`/`cons` test cases to existing `describe("createCandidate")` and `describe("updateCandidate")` blocks +- `tests/routes/threads.test.ts` — add a test case to existing `PUT` candidate describe block + +None — existing test infrastructure covers all phase requirements (as extensions). + +--- + +## Sources + +### Primary (HIGH confidence) + +- Direct code inspection: `src/db/schema.ts` — current `threadCandidates` column layout +- Direct code inspection: `tests/helpers/db.ts` — `CREATE TABLE thread_candidates` raw SQL +- Direct code inspection: `src/server/services/thread.service.ts` — `createCandidate`, `updateCandidate`, `getThreadWithCandidates` +- Direct code inspection: `src/shared/schemas.ts` — `createCandidateSchema`, `updateCandidateSchema` +- Direct code inspection: `src/client/components/CandidateForm.tsx` — form structure and payload +- Direct code inspection: `src/client/components/CandidateCard.tsx` — props interface and badge rendering +- Direct code inspection: `src/client/hooks/useCandidates.ts` — `CandidateResponse` interface +- Direct code inspection: `drizzle/0003_misty_mongu.sql` — ALTER TABLE migration pattern +- Direct code inspection: `CLAUDE.md` — explicit test-helper sync requirement + +### Secondary (MEDIUM confidence) + +- SQLite docs: `ALTER TABLE … ADD COLUMN` supports nullable columns without default — verified by existing migration pattern in project + +### Tertiary (LOW confidence) + +- None + +--- + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH — no new libraries; all tooling already in use +- Architecture: HIGH — full codebase read confirms exact ladder; no ambiguity +- Pitfalls: HIGH — CLAUDE.md explicitly calls out test helper drift; column projection issue confirmed by reading service code + +**Research date:** 2026-03-16 +**Valid until:** 2026-06-16 (stable stack — 90 days)