Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 11 additions & 42 deletions .claude/commands/til.md
Original file line number Diff line number Diff line change
@@ -1,51 +1,20 @@
# TIL — Today I Learned

Update `docs/til.md` with learnings from the current session.

## Steps

1. Read `docs/til.md` to understand what's already there.

2. Review recent work — look at the git diff, recent commits, and the current conversation to identify:
- New patterns or concepts introduced
- Errors that were hit and why they happened
- Design decisions made and the reasoning
- Gotchas or non-obvious behaviour

3. Add new content to the appropriate sections. Rules:
- **Don't duplicate.** If a concept is already covered, skip it or add a sub-point only if it adds something new.
- **Add to existing sections** rather than creating new ones unless it's genuinely new territory.
- **One concept per bullet.** Keep entries concise — a sentence or two max.
- **Ground it in the codebase.** Reference the actual file/pattern when helpful.

4. Add a new entry to the **Session Log** at the bottom:
- Today's date
- What was built (one line)
- Key decisions or errors fixed (bullet list)
Use `git diff main..HEAD`, recent commits, and the current conversation to find things worth capturing.

5. Show the user a summary of what was added — list the section and the new bullet(s) so they can review and ask for changes before accepting.
What counts: new language or framework features, query patterns, ORM tricks, async gotchas, type system behaviour, linter rules and what they mean, any time two approaches were weighed and one was chosen for a reason (framework/language level), non-obvious error handling behaviour.

## What to look for
Not here: API contract decisions specific to homik (status codes, endpoint shape, error responses), data model choices, scope changes (v1/v2). Add those to TIL anyway if they come up — but flag them at the end and suggest the user run `/update-spec` to capture them there too.

**Patterns worth capturing:**
- New language or framework features used
- Query patterns, ORM tricks, async gotchas
- Security decisions (auth, isolation, validation)
- Type system gotchas
Where to put things: add to existing sections rather than creating new ones unless it's genuinely new territory. One concept per bullet, one or two sentences max.

**Errors worth capturing:**
- Type errors and why they happened
- Linter rule violations and what they mean
- Runtime errors hit during testing
Also add a new entry to the Session Log at the bottom: today's date, what was built (one line), key learnings or errors fixed (bullet list).

**Design decisions worth capturing:**
- Any time we chose between two approaches and had a reason
- Any time a simpler approach was rejected
- API contract decisions (status codes, field names, optional vs required)
- Wording decisions on error messages (yes, those count)
If nothing qualifies, say so and stop.

## Important
Draft the additions and show them to the user before writing anything to the file. Wait for confirmation.

Keep the tone clear and direct — write as if explaining to a senior developer.
Do not add fluff, do not pad entries, do not repeat things already covered.
This is a learning log, not a changelog — capture the *why*, not just the *what*.
Sharpness rules:
- If it's already covered, skip it — don't restate
- Capture the *why*, not just the *what*
- Don't pad, don't add fluff, don't repeat things already in the file
22 changes: 22 additions & 0 deletions .claude/commands/update-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
Update `docs/spec.md` with decisions made during the current session.

Use `git diff main..HEAD`, recent commits, and the current conversation to find decisions worth recording.

**Rule of thumb:** If the decision is specific to homik's API or data model, it belongs here. If it's a pattern that would apply in any FastAPI project, it belongs in `/til`.

What counts: API contract choices (status codes, endpoint shape, error responses), data model decisions (field added/removed, constraint, denormalization), scope changes (v1/v2), deliberate behaviour that would surprise a reader of the spec (merge instead of reject, cascade in code not DB).

Where to put things:
- **Section 9 Key Decisions Log table** — high-level choice + one-sentence reason
- **Section 9 `### API Contract Decisions`** — implementation-level choices (endpoint behaviour, guard logic, merge vs reject)
- **Section 4 Data Model** — if a model field or constraint changed and the spec doesn't reflect it
- **Section 7 API Design** — if an endpoint signature changed

If nothing in the session qualifies, say so and stop — don't invent entries to justify running the skill.

Draft the additions and show them to the user before writing anything to the file. Wait for confirmation.

Sharpness rules:
- If it's already in the spec, skip it — don't restate
- Don't add exploratory ideas that weren't adopted
- One entry per decision. Decision + the one sharp reason it was made, nothing else.
7 changes: 7 additions & 0 deletions .claude/prompts/skill-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Use this when asking Claude to create or revise a skill file in `.claude/commands/`.

Paste into the conversation before describing the skill you want:

---

Write this as direct instructions to Claude, not a document. No headers. Imperative voice throughout ("do X", "skip if Y"). For each thing that could go wrong, name the failure mode explicitly rather than stating a virtue. Include: where to get inputs, confirm-before-write if it writes to a file, "if nothing qualifies say so and stop" if relevant, and pacing (iterative: wait after each item / single-output: gather → draft → confirm → write).
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ Import `SQLAlchemyUserDatabase` directly from `fastapi_users_db_sqlalchemy`, not
- `/pr-description` — generates a PR description and saves to `docs/pr-description.md`
- `/review-comments` — works through `docs/pr-comments.md` one comment at a time
- `/til` — updates `docs/til.md` with learnings from the current session
- `/update-spec` — updates `docs/spec.md` with project-specific API and data model decisions

## Developer environment

Expand Down
11 changes: 11 additions & 0 deletions docs/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,17 @@ Registration forks into two paths depending on whether an invite token is presen
| Past expiry dates | Allowed | No validation rejecting past `expiry_date` on batch creation — needed for initial inventory setup where users scan items already in the house. These batches appear immediately in the expiring-soon view. |
| Scan-out (deduction) flow | `POST /batches/{id}/adjust` | A dedicated adjust endpoint with a signed delta is simpler for the frontend than requiring it to calculate new quantity and branch between PATCH and DELETE. Auto-deletes the batch when quantity reaches zero. |

### API Contract Decisions

- **POST /items — upsert on barcode match.** Returns the existing item with HTTP 200 if a barcode already exists in the household; 201 for a new item. Client uses the status code: 200 = proceed to add a batch; 201 = fill out the new item form. Avoids a separate lookup-then-create round-trip.
- **`quantity > 0` enforced at two layers.** Pydantic `Field(gt=0)` returns a clean 422. DB `CheckConstraint("quantity > 0")` is the safety net if the API layer is bypassed.
- **Cascade delete on items — explicit in code, not DB.** `DELETE /items/{id}` manually deletes all batches first, then the item. N+1 queries accepted at household scale; a DB cascade would obscure the intent.
- **Merge on POST/PATCH collision instead of rejecting.** Both `POST /items/{id}/batches` and `PATCH /batches/{id}` merge quantities when `(item_id, location_id, expiry_date)` would collide. User intent is "add stock here" — a 409 forces the client to recover from something that isn't an error. Both endpoints return the surviving batch; PATCH may return a batch with a different `id` than the URL.
- **409 for FK-protected deletes.** Deleting a location that has batches, or a category that has items, returns 409. Data is not orphaned; the client gets a clear rejection signal.
- **`DELETE /locations/{id}?move_to={id}` — query param for cascading deletes.** One endpoint covers all cases: no batches → clean delete; has batches + no `move_to` → 409 with instructions; has batches + `move_to` → move then delete.
- **Cannot delete the last location.** Deleting the only location in a household is blocked with 409 — batches would have nowhere to move.
- **FE categorises inventory, not the backend.** `GET /items` returns all items; the frontend sorts into stocked / expiring soon / expired / out of stock from batch data it already has. No `expiry_before` filter on `GET /items` — don't add backend filters until a real screen proves it's needed. `GET /expiring` exists for the dedicated flat-list sorted by date.

---

## 10. Out of Scope for v1
Expand Down
47 changes: 25 additions & 22 deletions docs/til.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,39 +65,42 @@

---

## API Design Decisions

- **POST /items upsert pattern.** If a barcode already exists in the household, return the existing item with 200 instead of creating a duplicate. The client uses the status code to decide: 200 = "item exists, add a batch"; 201 = "new item created". This avoids a separate lookup-then-create flow.
- **Last-used location default.** When creating a batch without an explicit `location_id`, the server looks up the most recent batch for that household and uses its location. Falls back to the first seeded location. Falls back to 400 if no locations exist. This is API logic, not DB state.
- **`quantity > 0` validated at two layers.** Pydantic `Field(gt=0)` gives a clean 422 with a readable message. The DB `CheckConstraint("quantity > 0")` is the safety net if the API layer is ever bypassed.
- **Cascade delete on items.** Deleting an item manually deletes all its batches first, then the item. No DB-level cascade configured — explicit in code. N+1 queries, acceptable for household-scale data.
- **Unique constraint on `(item_id, location_id, expiry_date)`.** Enforces the core batch invariant at DB level: one batch per unique combination. Without it, the location move flow could silently create duplicate batches. See `models/batch.py`.
- **Merge batches on location move.** When moving batches to a target location, check first whether the target already has a batch for the same `(item_id, expiry_date)`. If yes, add quantities and delete the source batch. If no, just update `location_id`. Prevents a unique constraint violation and keeps data clean.
- **Merge batches on POST/PATCH collision instead of rejecting.** Both `POST /items/{id}/batches` and `PATCH /batches/{id}` now merge quantities into the existing batch when `(item_id, location_id, expiry_date)` would collide. User intent is "add stock here" — a 409 forces the client to recover from something that isn't really an error. Both endpoints return the surviving batch so the client has the correct state without a separate fetch. PATCH returns the surviving batch even if its `id` differs from the one in the URL.
- **409 for FK-protected deletes.** Deleting a location that has batches, or a category that has items, returns 409. The data isn't orphaned; the client gets a clear signal about why the delete was rejected.
- **Query parameter for cascading operations.** `DELETE /locations/{id}?move_to={id}` uses a query param to pass the target location. One endpoint handles all cases: no batches → clean delete; has batches + no `move_to` → 409 with instructions; has batches + `move_to` → move then delete. Cleaner than a separate "move" endpoint.
- **Cannot delete the last location.** Guard against deleting the only location in a household — batches would have nowhere to go. Count locations first; 409 if only one remains.
- **`POST /batches/{id}/adjust` — delta pattern.** Preferred over PATCH for scan flows. Client sends `{"delta": -2}`; server handles whether that means update or delete. Removes the client-side burden of branching between PATCH (quantity > 0) and DELETE (quantity = 0). Returns 200 + updated batch if quantity remains; 204 if batch is exhausted.
- **Distinguish `new_quantity < 0` from `== 0`.** In the adjust endpoint: `< 0` means the user is trying to remove more than exists — return 422 with current stock in the message. `== 0` means stock is exhausted — delete the batch and return 204. These are different cases and should return different responses.
- **FE does client-side categorisation.** `GET /items` returns all items; the frontend categorises into stocked / expiring soon / expired / out of stock using the batch data it already fetches. No need for an `expiry_before` filter on the backend — that's the frontend's job. `GET /expiring` is still useful for a dedicated flat list of batches sorted by date.
- **Remove filters that duplicate frontend logic.** `expiry_before` was removed from `GET /items` because the FE already has all the data it needs to categorise. Don't add backend filters until a real screen proves it's needed — adding one later is cheap, maintaining unused code is not.
- **Past expiry dates are allowed.** No validation prevents creating a batch with a past `expiry_date`. Needed for initial inventory setup — scanning items already in the house. These appear immediately in `GET /expiring`.
- **Standard consumer barcodes don't contain expiry dates.** EAN-13/UPC-A encode only the product ID. Expiry must be entered manually (v1) or OCR-scanned from the packaging (v2).

---

## Developer Tooling

- **`set -uo pipefail` without `-e` for lint scripts.** `-e` exits immediately on any non-zero exit code, which would kill the script before capturing lint output — lint failures are the expected case. Drop `-e` and capture output with `|| true` so both tools always run regardless of result.
- **`uv run` via a subshell, not a direct binary path.** Running `(cd backend && uv run ruff check .)` ensures `uv` resolves the correct project virtualenv without hardcoding `.venv/bin/ruff` or assuming anything about PATH. Works identically on any machine with `uv` installed.
- **`gh pr edit --body "$(cat docs/pr-description.md)"`** updates the PR description from a local file. Combine with a shell function (not an alias) so the `$()` expands at call time, not at definition time: `update-pr() { gh pr edit --body "$(cat docs/pr-description.md)"; }` in `~/.zshrc`.
- **Shell functions vs aliases for subshell expansion.** A plain `alias` expands `$()` when the alias is defined, capturing the value at shell startup. A function re-evaluates `$()` every time it's called — necessary whenever the substitution reads a file that changes between calls.
- **Claude Code skill design: classify before acting.** The `/fix-lint` skill explicitly separates "clear errors" (auto-fix: unused imports, missing annotations on private functions, print statements) from "design decisions" (wait for confirmation: `# type: ignore`, public API type changes, logic restructuring). Without this, a skill would either be too timid (ask about everything) or too aggressive (silently change semantics). The classification is defined in the skill file itself so the behaviour is predictable and auditable.
---

## Claude Code Skill Design

- **Write skill files as direct instructions, not documents.** Headers and "when to use" sections are for human readers — at runtime, document structure makes Claude process scaffolding before content. Imperative instructions ("do X", "skip if Y") are more directly actionable.
- **Classify before acting — the core skill pattern.** `/fix-lint` separates "clear errors" (auto-fix) from "design decisions" (wait for confirmation). Without this, a skill is either too timid (asks about everything) or too aggressive (silently changes semantics).
- **Sharpness rules outperform general guidance.** "Skip if already covered" is more effective than "be concise" — it names the specific failure mode rather than the virtue. Anticipate exactly how the skill can go wrong and kill each one by name.
- **Confirm-before-write for file-writing skills.** Draft, show the user, wait for confirmation before touching the file. Without this instruction, Claude writes first and the user reviews a diff after the fact.
- **Explicit "nothing qualifies" instruction prevents invented entries.** Without it, Claude will pad output to justify running. One line eliminates the whole failure mode.
- **Pacing depends on task shape.** Iterative skills (fix-lint, review-comments) pause after each item. Single-output skills (til, update-spec, pr-description) follow gather → draft → confirm → write. Match the pattern to the task.
- **CLAUDE.md is ambient; `.claude/prompts/` is on-demand.** CLAUDE.md loads every session — use it for things needed most sessions. For occasional tasks (e.g. writing a new skill), a prompt template in `.claude/prompts/` costs nothing when not in use and the user pastes it when needed. Frequency of use is the deciding factor.

---

## Session Log

### 2026-06-11 — TIL/spec boundary + skill design (PR: chore/til-spec-split)

Built: moved API Design Decisions from TIL to spec, created `/update-spec` skill, rewrote `/til` and `/update-spec` for better performance.

Key learnings:
- Skill files should be direct instructions — document structure makes Claude process scaffolding before content
- Sharpness rules (name the failure mode) outperform general guidance
- Confirm-before-write keeps user in control without post-hoc diff review
- "Nothing qualifies, say so and stop" prevents skill padding
- Pacing: iterative skills (one-at-a-time) vs single-output skills (gather → draft → confirm → write)
- CLAUDE.md vs `.claude/prompts/`: ambient vs on-demand context — frequency of use determines where an instruction lives

---

### 2026-06-08 — Lint workflow tooling (PR: feat/lint-workflow)

Built: `scripts/run-lint.sh` (Ruff + Pyright → `docs/lint-report.md`) and `/fix-lint` Claude Code skill.
Expand Down
Loading