docs(25): research catalog enrichment and agent tools phase
This commit is contained in:
@@ -0,0 +1,587 @@
|
||||
# Phase 25: Catalog Enrichment & Agent Tools - Research
|
||||
|
||||
**Researched:** 2026-04-10
|
||||
**Domain:** Drizzle ORM upserts + Hono REST API + MCP tool registration + React detail page
|
||||
**Confidence:** HIGH
|
||||
|
||||
<user_constraints>
|
||||
## User Constraints (from CONTEXT.md)
|
||||
|
||||
### Locked Decisions
|
||||
|
||||
- **D-01:** Add three new columns to `globalItems`: `sourceUrl` (text, nullable), `imageCredit` (text, nullable), `imageSourceUrl` (text, nullable).
|
||||
- **D-02:** No separate `manufacturer` column — existing `brand` field serves this purpose.
|
||||
- **D-03:** Attribution display on catalog detail page: image credit and source link shown inline below the image, not in a collapsible section.
|
||||
- **D-04:** Add a unique constraint on `(brand, model)` to `globalItems`.
|
||||
- **D-05:** Upsert semantics on conflict: `ON CONFLICT (brand, model) DO UPDATE` — update all non-key fields.
|
||||
- **D-06:** `POST /api/global-items/bulk` — accepts array of items, upserts all in a single transaction. Requires API key auth.
|
||||
- **D-07:** All-or-nothing transaction — if any item fails validation, reject the entire batch with detailed error response.
|
||||
- **D-08:** Maximum 100 items per request. Return count of created vs updated items in the response.
|
||||
- **D-09:** Request body shape: `{ items: [{ brand, model, category?, weightGrams?, priceCents?, imageUrl?, description?, sourceUrl?, imageCredit?, imageSourceUrl?, tags?: string[] }] }`.
|
||||
- **D-10:** `POST /api/global-items` — upsert a single catalog item with the same conflict handling as bulk. Also requires auth.
|
||||
- **D-11:** Add `upsert_catalog_item` MCP tool — writes directly to `globalItems` (not user-scoped).
|
||||
- **D-12:** Add `bulk_upsert_catalog` MCP tool — accepts array of items, calls the bulk import service.
|
||||
- **D-13:** MCP catalog tools require authenticated session (API key or OAuth bearer), same as all existing MCP tools.
|
||||
- **D-14:** Register new MCP tools in `src/server/mcp/index.ts` following the existing pattern.
|
||||
|
||||
### Claude's Discretion
|
||||
|
||||
- Drizzle migration approach for new columns and unique constraint
|
||||
- Exact Zod validation schemas for bulk import payload
|
||||
- MCP tool descriptions and parameter documentation
|
||||
- Tag handling in upsert (create-if-not-exists vs require existing tags)
|
||||
- Response shape details for bulk import (created/updated counts, item IDs)
|
||||
|
||||
### Deferred Ideas (OUT OF SCOPE)
|
||||
|
||||
- SEED-04 (actual seeding run with 100+ items)
|
||||
- Admin role for catalog management
|
||||
- Catalog item edit UI
|
||||
|
||||
</user_constraints>
|
||||
|
||||
<phase_requirements>
|
||||
## Phase Requirements
|
||||
|
||||
| ID | Description | Research Support |
|
||||
|----|-------------|------------------|
|
||||
| CATL-01 | Global items have attribution fields (sourceUrl, manufacturer, imageCredit, imageSourceUrl) | D-01: schema migration adds three columns; `brand` already serves manufacturer role (D-02) |
|
||||
| CATL-02 | Global items have a unique constraint on (brand, model) preventing duplicates | D-04: Drizzle `unique()` constraint in schema; migration via `bun run db:generate && db:push` |
|
||||
| CATL-03 | Catalog detail pages display image attribution with credit and source link | D-03: inline display below image in `$globalItemId.tsx`; `GlobalItem` interface needs new fields |
|
||||
| CATL-04 | Bulk import API endpoint accepts multiple catalog items in one request | D-06: `POST /api/global-items/bulk`; zValidator + bulkUpsertGlobalItems service function |
|
||||
| CATL-05 | Bulk import uses upsert semantics (ON CONFLICT update, not fail) | D-05: `onConflictDoUpdate({ target: [brand, model], set: {...} })` — already used elsewhere |
|
||||
| SEED-01 | MCP server has `upsert_catalog_item` tool writing to globalItems (not user-scoped) | D-11/D-14: new `catalog.ts` tool file following `items.ts` pattern; registered in `mcp/index.ts` |
|
||||
| SEED-02 | MCP server has `bulk_upsert_catalog` tool for batch catalog population | D-12: same tool file; calls bulkUpsertGlobalItems service |
|
||||
| SEED-03 | Catalog MCP tools include attribution fields (sourceUrl, manufacturer, imageCredit) as parameters | D-11/D-12: inputSchema includes all attribution fields; same auth model as existing tools |
|
||||
|
||||
</phase_requirements>
|
||||
|
||||
## Summary
|
||||
|
||||
Phase 25 adds write capability to the global items catalog: attribution metadata columns, a uniqueness constraint on `(brand, model)`, single and bulk upsert API endpoints, two new MCP tools, and attribution display on the catalog detail page. All patterns already exist in the codebase — this phase is entirely additive and follows established conventions.
|
||||
|
||||
The DB layer uses Drizzle ORM 0.45.1 with PostgreSQL (via `pg` driver in production, `@electric-sql/pglite` in tests). Drizzle's `.onConflictDoUpdate()` is already used in `auth.service.ts` (single-column conflict) and `settings.ts` (multi-column conflict), so the upsert pattern for `(brand, model)` is proven. The migration workflow is `bun run db:generate` (drizzle-kit) then `bun run db:push`.
|
||||
|
||||
MCP tools follow a three-part pattern: an exported `*ToolDefinitions` array, an exported `register*Tools(db, userId)` factory, and registration loops in `mcp/index.ts`. Catalog tools are unique in that they do NOT need `userId` (the catalog is shared, not user-scoped), but `userId` is still available for auth validation if needed.
|
||||
|
||||
**Primary recommendation:** Create `src/server/mcp/tools/catalog.ts`, extend `global-item.service.ts` with `upsertGlobalItem` and `bulkUpsertGlobalItems`, add `POST` routes to `global-items.ts`, run a schema migration, update the client hook interface, and add attribution display to `$globalItemId.tsx`.
|
||||
|
||||
## Standard Stack
|
||||
|
||||
### Core
|
||||
|
||||
| Library | Version | Purpose | Why Standard |
|
||||
|---------|---------|---------|--------------|
|
||||
| drizzle-orm | 0.45.1 | ORM + upsert via `onConflictDoUpdate` | Project standard; upsert already in use |
|
||||
| drizzle-kit | 0.31.9 | Schema migration generation | Project standard; `bun run db:generate` |
|
||||
| hono | 4.12.8 | HTTP routing for new POST endpoints | Project standard |
|
||||
| @hono/zod-validator | 0.7.6 | Request body validation middleware | Used on every POST/PUT route |
|
||||
| zod | 4.3.6 | Schema definitions for bulk payload | Project standard for shared schemas |
|
||||
| @modelcontextprotocol/sdk | 1.29.0 | MCP tool registration | Project standard; used in `mcp/index.ts` |
|
||||
|
||||
### Supporting
|
||||
|
||||
| Library | Version | Purpose | When to Use |
|
||||
|---------|---------|---------|-------------|
|
||||
| @electric-sql/pglite | 0.4.3 | In-memory Postgres for tests | Tests only — uses `createTestDb()` helper |
|
||||
| @tanstack/react-query | 5.90.21 | Client-side data fetching | Update `useGlobalItem` interface after new fields land |
|
||||
|
||||
**Installation:** No new dependencies needed. All required libraries are already installed.
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### Recommended Project Structure
|
||||
|
||||
The phase touches these existing files (no new directories needed):
|
||||
|
||||
```
|
||||
src/
|
||||
├── db/
|
||||
│ └── schema.ts # Add 3 columns + unique constraint to globalItems
|
||||
├── shared/
|
||||
│ └── schemas.ts # Add upsertGlobalItemSchema + bulkUpsertSchema
|
||||
├── server/
|
||||
│ ├── services/
|
||||
│ │ └── global-item.service.ts # Add upsertGlobalItem + bulkUpsertGlobalItems
|
||||
│ ├── routes/
|
||||
│ │ └── global-items.ts # Add POST / and POST /bulk handlers
|
||||
│ └── mcp/
|
||||
│ ├── index.ts # Register catalog tool group
|
||||
│ └── tools/
|
||||
│ └── catalog.ts # NEW: upsert_catalog_item + bulk_upsert_catalog
|
||||
├── client/
|
||||
│ ├── hooks/
|
||||
│ │ └── useGlobalItems.ts # Add sourceUrl, imageCredit, imageSourceUrl to interface
|
||||
│ └── routes/
|
||||
│ └── global-items/
|
||||
│ └── $globalItemId.tsx # Add attribution display below image
|
||||
drizzle-pg/
|
||||
└── XXXX_catalog_enrichment.sql # Generated migration
|
||||
```
|
||||
|
||||
### Pattern 1: Drizzle Upsert on Multi-Column Conflict
|
||||
|
||||
The `onConflictDoUpdate` API with an array target is already proven in `settings.ts`:
|
||||
|
||||
```typescript
|
||||
// Source: src/server/routes/settings.ts (line 33)
|
||||
await database
|
||||
.insert(settings)
|
||||
.values({ userId, key, value: body.value })
|
||||
.onConflictDoUpdate({
|
||||
target: [settings.userId, settings.key],
|
||||
set: { value: body.value },
|
||||
});
|
||||
```
|
||||
|
||||
For `globalItems` with a unique constraint on `(brand, model)`, the pattern is:
|
||||
|
||||
```typescript
|
||||
// Adapts from settings.ts pattern — for global-item.service.ts
|
||||
const [item] = await db
|
||||
.insert(globalItems)
|
||||
.values(data)
|
||||
.onConflictDoUpdate({
|
||||
target: [globalItems.brand, globalItems.model],
|
||||
set: {
|
||||
category: data.category,
|
||||
weightGrams: data.weightGrams,
|
||||
priceCents: data.priceCents,
|
||||
imageUrl: data.imageUrl,
|
||||
description: data.description,
|
||||
sourceUrl: data.sourceUrl,
|
||||
imageCredit: data.imageCredit,
|
||||
imageSourceUrl: data.imageSourceUrl,
|
||||
},
|
||||
})
|
||||
.returning();
|
||||
return item;
|
||||
```
|
||||
|
||||
**Key insight:** The unique constraint must exist on the table for `.onConflictDoUpdate({ target: [...] })` to reference it. The Drizzle migration (generated from the schema change) creates the constraint — `target` in `onConflictDoUpdate` references the schema columns, not raw strings.
|
||||
|
||||
### Pattern 2: All-or-Nothing Transaction for Bulk Upsert
|
||||
|
||||
Drizzle transactions are used in `setup.service.ts` and `thread.service.ts`. The bulk upsert wraps all inserts in a single transaction:
|
||||
|
||||
```typescript
|
||||
// Adapts from src/server/services/setup.service.ts (line 164)
|
||||
export async function bulkUpsertGlobalItems(
|
||||
db: Db,
|
||||
items: UpsertGlobalItemInput[],
|
||||
): Promise<{ created: number; updated: number; items: GlobalItem[] }> {
|
||||
return await db.transaction(async (tx) => {
|
||||
let created = 0;
|
||||
let updated = 0;
|
||||
const results: GlobalItem[] = [];
|
||||
|
||||
for (const data of items) {
|
||||
// Check if exists to determine created vs updated count
|
||||
const [existing] = await tx
|
||||
.select({ id: globalItems.id })
|
||||
.from(globalItems)
|
||||
.where(and(eq(globalItems.brand, data.brand), eq(globalItems.model, data.model)));
|
||||
|
||||
const [item] = await tx
|
||||
.insert(globalItems)
|
||||
.values(data)
|
||||
.onConflictDoUpdate({
|
||||
target: [globalItems.brand, globalItems.model],
|
||||
set: { /* all non-key fields */ },
|
||||
})
|
||||
.returning();
|
||||
|
||||
if (existing) updated++;
|
||||
else created++;
|
||||
results.push(item);
|
||||
|
||||
// Handle tags if provided
|
||||
if (data.tags && data.tags.length > 0) {
|
||||
await syncGlobalItemTags(tx, item.id, data.tags);
|
||||
}
|
||||
}
|
||||
|
||||
return { created, updated, items: results };
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** If any `.insert()` throws (e.g., Zod fails at service level for a structural error), the transaction rolls back automatically. Zod validation happens at the route level BEFORE the transaction, so the transaction only sees pre-validated data.
|
||||
|
||||
### Pattern 3: MCP Tool Registration (Catalog-Specific)
|
||||
|
||||
The catalog tools differ from other tool groups: they do not scope to `userId`. The `createMcpServer` function signature in `mcp/index.ts` passes `userId` to every tool factory — catalog tools accept it but do not filter by it.
|
||||
|
||||
```typescript
|
||||
// Source: pattern from src/server/mcp/tools/images.ts
|
||||
// images.ts already omits userId from its factory — catalog.ts follows the same approach
|
||||
|
||||
export const catalogToolDefinitions = [
|
||||
{
|
||||
name: "upsert_catalog_item",
|
||||
description: "...",
|
||||
inputSchema: {
|
||||
brand: z.string().describe("Brand or manufacturer name"),
|
||||
model: z.string().describe("Model name — combined with brand as unique identifier"),
|
||||
// ... all fields
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "bulk_upsert_catalog",
|
||||
description: "...",
|
||||
inputSchema: {
|
||||
items: z.array(catalogItemSchema).max(100).describe("Array of catalog items to upsert"),
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
// Factory takes db only (no userId needed — catalog is global)
|
||||
export function registerCatalogTools(db: Db) {
|
||||
return {
|
||||
upsert_catalog_item: async (args: UpsertArgs): Promise<ToolResult> => { ... },
|
||||
bulk_upsert_catalog: async (args: { items: UpsertArgs[] }): Promise<ToolResult> => { ... },
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
In `mcp/index.ts`, register like `imageHandlers` (no userId dependency):
|
||||
|
||||
```typescript
|
||||
// In createMcpServer():
|
||||
const catalogHandlers = registerCatalogTools(db);
|
||||
for (const def of catalogToolDefinitions) {
|
||||
const handler = catalogHandlers[def.name as keyof typeof catalogHandlers];
|
||||
server.tool(def.name, def.description, def.inputSchema, handler);
|
||||
}
|
||||
```
|
||||
|
||||
### Pattern 4: Schema Migration for Columns + Unique Constraint
|
||||
|
||||
Adding columns to an existing table and a unique constraint in Drizzle:
|
||||
|
||||
```typescript
|
||||
// In src/db/schema.ts — globalItems table
|
||||
export const globalItems = pgTable(
|
||||
"global_items",
|
||||
{
|
||||
id: serial("id").primaryKey(),
|
||||
brand: text("brand").notNull(),
|
||||
model: text("model").notNull(),
|
||||
category: text("category"),
|
||||
weightGrams: doublePrecision("weight_grams"),
|
||||
priceCents: integer("price_cents"),
|
||||
imageUrl: text("image_url"),
|
||||
description: text("description"),
|
||||
// NEW attribution columns:
|
||||
sourceUrl: text("source_url"),
|
||||
imageCredit: text("image_credit"),
|
||||
imageSourceUrl: text("image_source_url"),
|
||||
createdAt: timestamp("created_at").defaultNow().notNull(),
|
||||
},
|
||||
(table) => [unique().on(table.brand, table.model)], // NEW unique constraint
|
||||
);
|
||||
```
|
||||
|
||||
Run `bun run db:generate` to generate the migration SQL, then `bun run db:push` to apply. The generated SQL will contain `ALTER TABLE "global_items" ADD COLUMN ...` statements and a `CREATE UNIQUE INDEX` or `CONSTRAINT` statement.
|
||||
|
||||
**Important:** The existing `seedGlobalItems` function in `src/db/seed-global-items.ts` seeds with plain `db.insert()`. After the unique constraint lands, a second seed call would fail for duplicate `(brand, model)` pairs. The seed function is already idempotent by checking `existing.length > 0`, so no changes needed there.
|
||||
|
||||
### Pattern 5: Attribution Display (Client)
|
||||
|
||||
The `$globalItemId.tsx` component renders the image in a `div` below which we add attribution. The `useGlobalItem` hook interface needs updating to include new fields, then the component can conditionally render them:
|
||||
|
||||
```tsx
|
||||
{/* After the image div, before the header */}
|
||||
{(item.imageCredit || item.imageSourceUrl) && (
|
||||
<p className="text-xs text-gray-400 mt-2">
|
||||
{item.imageCredit && <span>Photo: {item.imageCredit}</span>}
|
||||
{item.imageSourceUrl && (
|
||||
<a
|
||||
href={item.imageSourceUrl}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="ml-1 underline hover:text-gray-600"
|
||||
>
|
||||
Source
|
||||
</a>
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
```
|
||||
|
||||
### Pattern 6: Tag Handling in Upsert (Claude's Discretion)
|
||||
|
||||
**Recommendation: create-if-not-exists.** The existing `tags` table has a unique constraint on `name`. Use `onConflictDoUpdate` (or `onConflictDoNothing`) on the tags table to get the tag ID, then upsert the `globalItemTags` junction. This lets agents pass tag names without pre-populating the tags table.
|
||||
|
||||
```typescript
|
||||
async function syncGlobalItemTags(tx: Tx, globalItemId: number, tagNames: string[]) {
|
||||
// Delete existing tags for this item
|
||||
await tx.delete(globalItemTags).where(eq(globalItemTags.globalItemId, globalItemId));
|
||||
|
||||
for (const name of tagNames) {
|
||||
// Upsert tag (create if not exists)
|
||||
const [tag] = await tx
|
||||
.insert(tags)
|
||||
.values({ name })
|
||||
.onConflictDoUpdate({ target: tags.name, set: { name } })
|
||||
.returning({ id: tags.id });
|
||||
|
||||
await tx.insert(globalItemTags).values({ globalItemId, tagId: tag.id });
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
|
||||
- **Do NOT modify `mcp/index.ts` `createMcpServer` signature** to remove `userId` just because catalog tools don't need it. Pass `db` only to `registerCatalogTools` — accept the `userId` parameter in `createMcpServer` and simply not forward it to catalog tools.
|
||||
- **Do NOT validate tags at the Zod level as enum values.** Tags are open-ended strings; only length/format validation is appropriate.
|
||||
- **Do NOT return partial success for bulk upsert.** D-07 mandates all-or-nothing. Zod schema validation at the route level catches structural errors before any DB work begins.
|
||||
- **Do NOT add rate limiting to `POST /api/global-items` or `POST /api/global-items/bulk`.** These are authenticated-write endpoints — the auth middleware already gates them. The rate limiting added in Phase 24 only applies to public GET endpoints.
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| Upsert on conflict | Manual SELECT then INSERT/UPDATE | `onConflictDoUpdate()` | Drizzle handles atomicity; already used in settings.ts and auth.service.ts |
|
||||
| All-or-nothing bulk write | Try/catch with manual rollback | `db.transaction(async (tx) => { ... })` | Drizzle handles rollback on throw; used in setup.service.ts |
|
||||
| Tag create-if-not-exists | Check exists, insert if not | `onConflictDoUpdate` on tags.name | Same conflict mechanism |
|
||||
| Request body validation | Manual type checking in handler | `zValidator("json", schema)` from `@hono/zod-validator` | All POST/PUT routes use this; Hono returns 400 automatically |
|
||||
| Auth on POST endpoints | Custom auth check in handler | Existing `requireAuth` middleware in `src/server/index.ts` | Auth middleware already gates all non-GET `/api/global-items*` after Phase 24 |
|
||||
|
||||
**Key insight:** The auth middleware in `src/server/index.ts` (lines 150-170) already exempts only GET requests on `/api/global-items` from auth. POST requests on that path fall through to `requireAuth` automatically — no changes to `index.ts` needed.
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1: Unique Constraint Not Applied to Existing Data
|
||||
|
||||
**What goes wrong:** If the database already has duplicate `(brand, model)` pairs (e.g., from early seeding), adding a unique constraint via migration will fail with a duplicate key error.
|
||||
|
||||
**Why it happens:** The migration tries to add `UNIQUE(brand, model)` to a table that already has conflicting rows.
|
||||
|
||||
**How to avoid:** Check for existing duplicates before generating the migration. In development with test data, truncate `global_items` or resolve duplicates first. The test database is reset between tests (`TRUNCATE ... RESTART IDENTITY CASCADE`) so tests are not affected.
|
||||
|
||||
**Warning signs:** `drizzle-kit push` fails with "could not create unique index" or similar.
|
||||
|
||||
### Pitfall 2: Client `GlobalItem` Interface Missing New Fields
|
||||
|
||||
**What goes wrong:** `useGlobalItems.ts` defines a local `GlobalItem` interface. After the migration adds columns, the API returns new fields but the TypeScript interface doesn't include them, so the component can't render them.
|
||||
|
||||
**Why it happens:** The interface at the top of `useGlobalItems.ts` (lines 3-14) is manually maintained — it's not generated from Drizzle schema types.
|
||||
|
||||
**How to avoid:** Update the `GlobalItem` interface in `useGlobalItems.ts` to add `sourceUrl: string | null`, `imageCredit: string | null`, `imageSourceUrl: string | null`. The `GlobalItemWithOwnerCount` extends it and gets the fields for free.
|
||||
|
||||
**Warning signs:** TypeScript error `Property 'imageCredit' does not exist on type 'GlobalItemWithOwnerCount'` in `$globalItemId.tsx`.
|
||||
|
||||
### Pitfall 3: Bulk Endpoint Registered Before Auth Middleware Applies
|
||||
|
||||
**What goes wrong:** `/api/global-items/bulk` is a POST endpoint. The auth middleware in `src/server/index.ts` exempts GET on `/api/global-items` (line 165-167). If the route registration order matters, the bulk POST could be accidentally exempt.
|
||||
|
||||
**Why it happens:** The middleware skip check uses `c.req.path.startsWith("/api/global-items") && c.req.method === "GET"` — it's already method-gated, so POST requests are not exempt. No issue in practice.
|
||||
|
||||
**How to avoid:** No special handling needed. The middleware skip condition already checks `c.req.method === "GET"`. Verify this by reading `src/server/index.ts` lines 165-167 before implementing.
|
||||
|
||||
**Warning signs:** POST to `/api/global-items/bulk` returns 200 without `X-API-Key` header.
|
||||
|
||||
### Pitfall 4: MCP Tool for Bulk Returns Success on Partial Failure
|
||||
|
||||
**What goes wrong:** If the service function for bulk upsert silently skips failed items instead of throwing, the MCP tool returns `{ created: X, updated: Y }` even though some items were not persisted.
|
||||
|
||||
**Why it happens:** Swallowing errors inside a loop without re-throwing.
|
||||
|
||||
**How to avoid:** Zod validation happens at the route level (for HTTP) and at the MCP tool handler level (parse inputSchema). The service function should assume pre-validated data and let Drizzle throw naturally inside the transaction. The transaction auto-rolls-back on any thrown error.
|
||||
|
||||
**Warning signs:** Bulk upsert returns counts that don't add up to the input array length, with no error.
|
||||
|
||||
### Pitfall 5: `onConflictDoUpdate` Target Requires Constraint, Not Arbitrary Columns
|
||||
|
||||
**What goes wrong:** Calling `.onConflictDoUpdate({ target: [globalItems.brand, globalItems.model], ... })` without a unique constraint on those columns causes a Postgres error at runtime.
|
||||
|
||||
**Why it happens:** PostgreSQL's `ON CONFLICT (col1, col2) DO UPDATE` requires an existing unique index or constraint. Drizzle passes through to Postgres directly.
|
||||
|
||||
**How to avoid:** The unique constraint must be added to the schema and migration applied BEFORE any upsert code runs. Always run `bun run db:push` before testing the new service functions.
|
||||
|
||||
**Warning signs:** `ERROR: there is no unique or exclusion constraint matching the ON CONFLICT specification`.
|
||||
|
||||
## Code Examples
|
||||
|
||||
Verified patterns from the codebase:
|
||||
|
||||
### Drizzle Upsert (multi-column target)
|
||||
|
||||
```typescript
|
||||
// Source: src/server/routes/settings.ts lines 33-37
|
||||
await database
|
||||
.insert(settings)
|
||||
.values({ userId, key, value: body.value })
|
||||
.onConflictDoUpdate({
|
||||
target: [settings.userId, settings.key],
|
||||
set: { value: body.value },
|
||||
});
|
||||
```
|
||||
|
||||
### Drizzle Transaction
|
||||
|
||||
```typescript
|
||||
// Source: src/server/services/setup.service.ts line 164
|
||||
return await db.transaction(async (tx) => {
|
||||
// ... multiple tx.insert / tx.select calls
|
||||
// Any thrown error rolls back automatically
|
||||
});
|
||||
```
|
||||
|
||||
### Hono Route with zValidator
|
||||
|
||||
```typescript
|
||||
// Source: src/server/routes/setups.ts line 36
|
||||
app.post("/", zValidator("json", createSetupSchema), async (c) => {
|
||||
const db = c.get("db");
|
||||
const userId = c.get("userId")!;
|
||||
const data = c.req.valid("json");
|
||||
// ...
|
||||
});
|
||||
```
|
||||
|
||||
### MCP Tool Definition + Handler (no-userId pattern)
|
||||
|
||||
```typescript
|
||||
// Source: src/server/mcp/tools/images.ts — full file
|
||||
export const imageToolDefinitions = [
|
||||
{
|
||||
name: "upload_image_from_url",
|
||||
description: "...",
|
||||
inputSchema: { url: z.string().describe("...") },
|
||||
},
|
||||
];
|
||||
|
||||
export function registerImageTools() { // no db, no userId
|
||||
return {
|
||||
upload_image_from_url: async (args: { url: string }): Promise<ToolResult> => {
|
||||
try {
|
||||
const result = await fetchImageFromUrl(args.url);
|
||||
return textResult(result);
|
||||
} catch (err) {
|
||||
return errorResult((err as Error).message);
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
### Schema with Unique Constraint (table-level)
|
||||
|
||||
```typescript
|
||||
// Source: src/db/schema.ts line 26-38 (categories table — same pattern)
|
||||
export const categories = pgTable(
|
||||
"categories",
|
||||
{
|
||||
id: serial("id").primaryKey(),
|
||||
// ...
|
||||
},
|
||||
(table) => [unique().on(table.userId, table.name)],
|
||||
);
|
||||
```
|
||||
|
||||
## State of the Art
|
||||
|
||||
| Old Approach | Current Approach | When Changed | Impact |
|
||||
|--------------|------------------|--------------|--------|
|
||||
| Separate `item_global_links` junction table | `globalItemId` FK directly on `items` | Phase migration 0002 | Simpler joins; one less table |
|
||||
| No unique constraint on globalItems | `unique().on(brand, model)` | This phase | Prevents duplicate catalog entries |
|
||||
| Read-only global item API | Read + write (upsert) API | This phase | Enables agent-powered seeding |
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Tag replacement vs merge on upsert**
|
||||
- What we know: D-09 includes `tags?: string[]` as optional field
|
||||
- What's unclear: When tags are omitted on an upsert of an existing item, should existing tags be left untouched, or cleared?
|
||||
- Recommendation: Leave existing tags untouched when `tags` field is absent in the payload. Only sync (replace) tags when `tags` is explicitly provided (even if empty array = clear all tags). This is the least-surprising behavior for agents that send partial updates.
|
||||
|
||||
2. **`imageUrl` field on globalItems vs `imageFilename`**
|
||||
- What we know: `globalItems.imageUrl` stores an absolute URL (unlike `items.imageFilename` which stores a filename for S3). The bulk upsert input accepts `imageUrl`.
|
||||
- What's unclear: Should agents use `upload_image_from_url` first, then pass the returned filename as `imageUrl`? Or pass the original URL directly?
|
||||
- Recommendation: Accept both — agents can pass any URL to `imageUrl`. When the agent wants to mirror the image to S3, they call `upload_image_from_url` first and use the result. The `imageSourceUrl` attribution field is separate and intended to record the original source regardless of where the image is now stored.
|
||||
|
||||
## Environment Availability
|
||||
|
||||
Step 2.6: SKIPPED (no external dependencies identified — this phase is purely code and schema changes within the existing stack; PostgreSQL/PGlite is already operational)
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### 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/global-item.service.test.ts tests/routes/global-items.test.ts tests/mcp/tools.test.ts` |
|
||||
| Full suite command | `bun test` |
|
||||
|
||||
### Phase Requirements → Test Map
|
||||
|
||||
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||
|--------|----------|-----------|-------------------|-------------|
|
||||
| CATL-01 | Attribution columns present and returnable | integration | `bun test tests/services/global-item.service.test.ts` | ✅ (extend) |
|
||||
| CATL-02 | Duplicate (brand, model) upserts rather than errors | integration | `bun test tests/services/global-item.service.test.ts` | ✅ (extend) |
|
||||
| CATL-03 | Attribution rendered in detail page | manual | Visual check in browser | — |
|
||||
| CATL-04 | `POST /api/global-items/bulk` accepts array | integration | `bun test tests/routes/global-items.test.ts` | ✅ (extend) |
|
||||
| CATL-05 | Bulk upsert updates on conflict, returns created/updated counts | integration | `bun test tests/routes/global-items.test.ts` | ✅ (extend) |
|
||||
| SEED-01 | `upsert_catalog_item` MCP tool writes to globalItems | integration | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
|
||||
| SEED-02 | `bulk_upsert_catalog` MCP tool persists all items | integration | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
|
||||
| SEED-03 | Attribution fields available as MCP tool parameters | unit | `bun test tests/mcp/tools.test.ts` | ✅ (extend) |
|
||||
|
||||
### Sampling Rate
|
||||
|
||||
- **Per task commit:** `bun test tests/services/global-item.service.test.ts tests/routes/global-items.test.ts tests/mcp/tools.test.ts`
|
||||
- **Per wave merge:** `bun test`
|
||||
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
|
||||
None — existing test infrastructure covers all phase requirements. The three test files already exist and test the relevant modules; new test cases are added to existing `describe` blocks.
|
||||
|
||||
## Project Constraints (from CLAUDE.md)
|
||||
|
||||
- Use `bun run db:generate` + `bun run db:push` for all schema changes (not raw SQL)
|
||||
- Prices stored as cents (`priceCents: integer`), weights as grams (`doublePrecision`)
|
||||
- Services take `(db, ...)` as first argument — no HTTP awareness
|
||||
- Hono routes delegate to services; use `@hono/zod-validator` for all request validation
|
||||
- Shared Zod schemas live in `src/shared/schemas.ts`
|
||||
- MCP tools: definitions array + register function pattern per domain
|
||||
- Route tree is auto-generated — never edit `routeTree.gen.ts`
|
||||
- Always reuse existing components; check `src/client/components/` before creating new UI elements
|
||||
- `@/*` maps to `./src/*`
|
||||
- Tabs, double quotes, organized imports (Biome lint)
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence)
|
||||
|
||||
- Direct codebase inspection: `src/db/schema.ts` — current globalItems table definition
|
||||
- Direct codebase inspection: `src/server/services/global-item.service.ts` — existing read patterns
|
||||
- Direct codebase inspection: `src/server/routes/global-items.ts` — existing route handlers
|
||||
- Direct codebase inspection: `src/server/mcp/index.ts` — tool registration loop pattern
|
||||
- Direct codebase inspection: `src/server/mcp/tools/items.ts` — tool definition + handler pattern
|
||||
- Direct codebase inspection: `src/server/mcp/tools/images.ts` — no-userId factory pattern
|
||||
- Direct codebase inspection: `src/server/routes/settings.ts` — multi-column `onConflictDoUpdate` pattern
|
||||
- Direct codebase inspection: `src/server/services/auth.service.ts` — single-column `onConflictDoUpdate` pattern
|
||||
- Direct codebase inspection: `src/server/services/setup.service.ts` — `db.transaction()` pattern
|
||||
- Direct codebase inspection: `src/server/index.ts` — auth middleware skip logic for global-items GET
|
||||
- Direct codebase inspection: `tests/helpers/db.ts` — PGlite test setup with migrations
|
||||
- Direct codebase inspection: `tests/services/global-item.service.test.ts` — existing test structure
|
||||
- Direct codebase inspection: `tests/routes/global-items.test.ts` — existing route test structure
|
||||
- Direct codebase inspection: `tests/mcp/tools.test.ts` — MCP tool test structure
|
||||
|
||||
### Secondary (MEDIUM confidence)
|
||||
|
||||
- drizzle-orm 0.45.1 installed version confirmed via `bun pm ls` — upsert API stable since 0.28
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
|
||||
- Standard stack: HIGH — all libraries confirmed installed; APIs confirmed in use
|
||||
- Architecture: HIGH — all patterns directly observed in codebase, not assumed
|
||||
- Pitfalls: HIGH — derived from reading the actual middleware skip logic and schema constraints
|
||||
|
||||
**Research date:** 2026-04-10
|
||||
**Valid until:** 2026-05-10 (stable stack, no fast-moving dependencies)
|
||||
Reference in New Issue
Block a user