diff --git a/.claude/commands/til.md b/.claude/commands/til.md new file mode 100644 index 0000000..d19699a --- /dev/null +++ b/.claude/commands/til.md @@ -0,0 +1,51 @@ +# 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) + +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 to look for + +**Patterns worth capturing:** +- New language or framework features used +- Query patterns, ORM tricks, async gotchas +- Security decisions (auth, isolation, validation) +- Type system gotchas + +**Errors worth capturing:** +- Type errors and why they happened +- Linter rule violations and what they mean +- Runtime errors hit during testing + +**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) + +## Important + +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*. diff --git a/CLAUDE.md b/CLAUDE.md index adacf7d..2c0fdb8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,7 +60,7 @@ Household → User (many per household) - **Item** = the product definition (name, barcode, brand, image_url from Open Food Facts, category, unit). No location — the same product can live in multiple places. - **Batch** = one group of units sharing the same location and expiry date. A purchase split across two locations creates two batches. -- Expiry granularity is month + year only; defaults to +12 months from today. +- Expiry is stored as a full `date` field; the UI will show month + year only. Defaults to +12 months from today. - `ShoppingListItem` model exists in v1 but has no UI — that's a v2 feature. ### Multi-household data isolation @@ -89,6 +89,35 @@ All `datetime` fields must use `sa_column=sa.Column(sa.DateTime(timezone=True))` No Alembic yet. Schema changes require dropping affected tables in the Neon console and restarting the server to recreate them. Add Alembic before any production data exists. +### SQLModel query conventions + +**`col()` for column expressions:** When using `.ilike()`, `.in_()`, `.desc()`, or `order_by()` on a model attribute, wrap it with `col()` from `sqlmodel`. Without it, Pyright resolves the attribute as a Python type (`str`, `int`, `date`) and flags the SQL method as unknown. + +```python +from sqlmodel import col, select + +select(Item).where(col(Item.name).ilike(f"%{name}%")) +select(Item).where(col(Item.id).in_(subquery)) +select(Batch).order_by(col(Batch.expiry_date)) +``` + +**`AsyncSession` import:** Always import `AsyncSession` from `sqlmodel.ext.asyncio.session`, not from `sqlalchemy.ext.asyncio`. SQLAlchemy's version doesn't have `.exec()`. Also set `class_=AsyncSession` in `async_sessionmaker` so the session factory creates SQLModel sessions: + +```python +from sqlmodel.ext.asyncio.session import AsyncSession +_session_factory = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) +``` + +### API endpoint conventions + +**Household isolation:** Every query must filter by `user.household_id`. Return 404 for both "not found" and "wrong household" — never 403. Returning 403 leaks whether an ID exists. + +**IntegrityError handling:** Let the DB enforce unique constraints rather than doing pre-check queries (race condition + extra round-trip). Catch `sqlalchemy.exc.IntegrityError` after `commit()`, call `session.rollback()`, and raise a 409. Do not attempt to introspect the underlying driver exception (`exc.orig.pgcode`, `isinstance(..., asyncpg.UniqueViolationError)`) — this is fragile across SQLAlchemy/asyncpg versions. Add a comment explaining why the broad catch is safe for that specific endpoint. + +**Response schemas:** Never return raw table models — they expose `household_id` and other internals. Always return a `*Read` schema that explicitly lists the fields the client should see. + +**PATCH pattern:** Use `data.model_dump(exclude_unset=True)` to only update fields explicitly sent in the request. This prevents overwriting existing values with `None` for fields the client didn't mention. + ### fastapi-users imports Import `SQLAlchemyUserDatabase` directly from `fastapi_users_db_sqlalchemy`, not `fastapi_users.db`. The re-export in `fastapi_users.db` uses a `try/except` block that Pylance cannot statically trace, causing false unknown-symbol errors. @@ -99,6 +128,7 @@ Import `SQLAlchemyUserDatabase` directly from `fastapi_users_db_sqlalchemy`, not - `./scripts/fetch-pr-comments.sh` — fetches open PR review comments into `docs/pr-comments.md` - `/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 ## Developer environment diff --git a/backend/app/auth/users.py b/backend/app/auth/users.py index 0ea6933..dc6df77 100644 --- a/backend/app/auth/users.py +++ b/backend/app/auth/users.py @@ -6,7 +6,7 @@ from fastapi_users import BaseUserManager, FastAPIUsers, UUIDIDMixin from fastapi_users.db import BaseUserDatabase from fastapi_users_db_sqlalchemy import SQLAlchemyUserDatabase -from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel.ext.asyncio.session import AsyncSession from app.auth.backend import auth_backend from app.config import get_settings @@ -49,6 +49,9 @@ async def on_after_register( household = Household(name=f"{prefix}'s household") self.session.add(household) await self.session.flush() # writes household row and populates household.id + # flush always populates the PK; the None check is for type narrowing only. + if household.id is None: + raise RuntimeError("Household PK not populated after flush") user.household_id = household.id # session tracks this change automatically self.session.add(Location(household_id=household.id, name="Pantry")) self.session.add(Category(household_id=household.id, name="Food")) diff --git a/backend/app/database.py b/backend/app/database.py index f432fb3..5d6b225 100644 --- a/backend/app/database.py +++ b/backend/app/database.py @@ -1,7 +1,8 @@ from collections.abc import AsyncGenerator -from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from sqlmodel import SQLModel +from sqlmodel.ext.asyncio.session import AsyncSession from .config import get_settings @@ -14,7 +15,9 @@ engine = create_async_engine(DATABASE_URL, echo=settings.environment == "development", pool_pre_ping=True) # keeps objects usable after commit (relevant for async sessions) -_session_factory = async_sessionmaker(engine, expire_on_commit=False) +# class_=AsyncSession uses SQLModel's session subclass, which adds the .exec() method +# that Pylance/Pyright can resolve. SQLAlchemy's default AsyncSession doesn't have it. +_session_factory = async_sessionmaker(engine, class_=AsyncSession, expire_on_commit=False) async def get_session() -> AsyncGenerator[AsyncSession]: diff --git a/backend/app/main.py b/backend/app/main.py index 061ecb1..248463a 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -8,7 +8,7 @@ from app.auth.users import fastapi_users from app.database import create_db_and_tables from app.models.user import UserCreate, UserRead -from app.routers import household +from app.routers import batches, categories, household, items, locations, lookup @asynccontextmanager @@ -34,6 +34,11 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None]: ) app.include_router(household.router) +app.include_router(locations.router) +app.include_router(categories.router) +app.include_router(items.router) +app.include_router(batches.router) +app.include_router(lookup.router) @app.get("/health") diff --git a/backend/app/models/batch.py b/backend/app/models/batch.py index a5812f8..dd12741 100644 --- a/backend/app/models/batch.py +++ b/backend/app/models/batch.py @@ -6,6 +6,11 @@ class Batch(SQLModel, table=True): __tablename__ = "batches" # type: ignore[assignment] + # Enforces the core invariant: one batch per unique (item, location, expiry) combo. + # Prevents duplicate batches from being created directly or via the location move flow. + __table_args__ = ( + sa.UniqueConstraint("item_id", "location_id", "expiry_date", name="uq_batch_item_location_expiry"), + ) id: int | None = Field(default=None, primary_key=True) # Denormalized from Item for query-level household isolation — every batch query can filter diff --git a/backend/app/routers/batches.py b/backend/app/routers/batches.py new file mode 100644 index 0000000..9650f82 --- /dev/null +++ b/backend/app/routers/batches.py @@ -0,0 +1,272 @@ +from datetime import date, datetime, timedelta +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException, Query, Response +from sqlmodel import Field, SQLModel, col, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.auth.users import current_active_user +from app.database import get_session +from app.models.batch import Batch +from app.models.item import Item +from app.models.location import Location +from app.models.user import User + +# No prefix — batch endpoints span two URL patterns (/items/{id}/batches and /batches/{id}). +router = APIRouter(tags=["batches"]) + + +class BatchRead(SQLModel): + id: int + item_id: int + quantity: int + expiry_date: date + location_id: int + created_at: datetime + + +class BatchCreate(SQLModel): + quantity: int = Field(gt=0) + # Both fields are optional: server resolves defaults if not provided. + expiry_date: date | None = None + location_id: int | None = None + + +class BatchUpdate(SQLModel): + quantity: int | None = Field(default=None, gt=0) + expiry_date: date | None = None + location_id: int | None = None + + +class BatchAdjust(SQLModel): + # Positive = scan in (add stock), negative = scan out (use stock). + delta: int + + +async def _default_location_id(session: AsyncSession, household_id: int) -> int | None: + """Return last-used location for this household, or fall back to first seeded location.""" + last = ( + await session.exec( + select(Batch.location_id) + .where(Batch.household_id == household_id) + # col() needed so Pyright treats created_at as a column expression, not a datetime. + .order_by(col(Batch.created_at).desc()) + .limit(1) + ) + ).first() + if last is not None: + return last + first_loc = ( + await session.exec( + select(Location) + .where(Location.household_id == household_id) + # col() needed so Pyright treats id as a column expression, not int | None. + .order_by(col(Location.id)) + .limit(1) + ) + ).first() + return first_loc.id if first_loc is not None else None + + +@router.get("/items/{item_id}/batches") +async def list_item_batches( + item_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> list[BatchRead]: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + item = await session.get(Item, item_id) + if item is None or item.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Item not found") + result = await session.exec( + select(Batch).where(Batch.item_id == item_id).order_by(col(Batch.expiry_date)) + ) + return [BatchRead.model_validate(b) for b in result.all()] + + +@router.post("/items/{item_id}/batches", status_code=201) +async def create_batch( + item_id: int, + data: BatchCreate, + response: Response, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> BatchRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + item = await session.get(Item, item_id) + if item is None or item.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Item not found") + + location_id = data.location_id + if location_id is None: + location_id = await _default_location_id(session, user.household_id) + if location_id is None: + raise HTTPException( + status_code=400, + detail="No location available — please create a location first", + ) + + location = await session.get(Location, location_id) + if location is None or location.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Location not found") + + expiry_date = data.expiry_date or date.today() + timedelta(days=365) + + # If a batch already exists for this (item, location, expiry), merge quantities + # rather than rejecting — user intent is "add stock here", not "create a new record". + existing = ( + await session.exec( + select(Batch).where( + Batch.item_id == item_id, + Batch.location_id == location_id, + Batch.expiry_date == expiry_date, + Batch.household_id == user.household_id, + ) + ) + ).first() + + if existing is not None: + existing.quantity += data.quantity + session.add(existing) + await session.commit() + await session.refresh(existing) + response.status_code = 200 + return BatchRead.model_validate(existing) + + batch = Batch( + item_id=item_id, + household_id=user.household_id, + location_id=location_id, + quantity=data.quantity, + expiry_date=expiry_date, + ) + session.add(batch) + await session.commit() + await session.refresh(batch) + return BatchRead.model_validate(batch) + + +@router.patch("/batches/{batch_id}") +async def update_batch( + batch_id: int, + data: BatchUpdate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> BatchRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + batch = await session.get(Batch, batch_id) + if batch is None or batch.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Batch not found") + + updates = data.model_dump(exclude_unset=True) + + if "location_id" in updates and updates["location_id"] is not None: + location = await session.get(Location, updates["location_id"]) + if location is None or location.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Location not found") + + for field, value in updates.items(): + setattr(batch, field, value) + + # Check whether the updated (item, location, expiry) would collide with an existing batch. + # If so, merge quantities into the surviving batch and delete this one — same behaviour + # as the location-delete move flow. Returns the surviving batch so the client sees the + # correct quantity at the destination without needing a separate fetch. + existing = ( + await session.exec( + select(Batch).where( + Batch.item_id == batch.item_id, + Batch.location_id == batch.location_id, + Batch.expiry_date == batch.expiry_date, + Batch.household_id == user.household_id, + Batch.id != batch_id, + ) + ) + ).first() + + if existing is not None: + existing.quantity += batch.quantity + session.add(existing) + await session.delete(batch) + await session.commit() + await session.refresh(existing) + return BatchRead.model_validate(existing) + + session.add(batch) + await session.commit() + await session.refresh(batch) + return BatchRead.model_validate(batch) + + +@router.delete("/batches/{batch_id}", status_code=204) +async def delete_batch( + batch_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> None: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + batch = await session.get(Batch, batch_id) + if batch is None or batch.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Batch not found") + await session.delete(batch) + await session.commit() + + +@router.post("/batches/{batch_id}/adjust") +async def adjust_batch( + batch_id: int, + data: BatchAdjust, + response: Response, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> BatchRead | None: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + batch = await session.get(Batch, batch_id) + if batch is None or batch.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Batch not found") + + new_quantity = batch.quantity + data.delta + + if new_quantity < 0: + raise HTTPException( + status_code=422, + detail=f"Cannot remove {abs(data.delta)} — only {batch.quantity} in stock", + ) + + if new_quantity == 0: + # Quantity exhausted — delete the batch rather than storing a zero value. + await session.delete(batch) + await session.commit() + response.status_code = 204 + return None + + batch.quantity = new_quantity + session.add(batch) + await session.commit() + await session.refresh(batch) + return BatchRead.model_validate(batch) + + +@router.get("/expiring") +async def get_expiring( + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], + days: Annotated[int, Query(ge=1)] = 30, +) -> list[BatchRead]: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + cutoff = date.today() + timedelta(days=days) + result = await session.exec( + select(Batch) + .where( + Batch.household_id == user.household_id, + Batch.expiry_date <= cutoff, + ) + .order_by(col(Batch.expiry_date)) + ) + return [BatchRead.model_validate(b) for b in result.all()] diff --git a/backend/app/routers/categories.py b/backend/app/routers/categories.py new file mode 100644 index 0000000..abb09f9 --- /dev/null +++ b/backend/app/routers/categories.py @@ -0,0 +1,112 @@ +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.exc import IntegrityError +from sqlmodel import Field, SQLModel, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.auth.users import current_active_user +from app.database import get_session +from app.models.category import Category +from app.models.item import Item +from app.models.user import User + +router = APIRouter(prefix="/categories", tags=["categories"]) + + +class CategoryRead(SQLModel): + id: int + name: str + + +class CategoryCreate(SQLModel): + name: str = Field(max_length=100) + + +class CategoryUpdate(SQLModel): + name: str = Field(max_length=100) + + +@router.get("") +async def list_categories( + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> list[CategoryRead]: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + result = await session.exec( + select(Category).where(Category.household_id == user.household_id) + ) + return [CategoryRead.model_validate(cat) for cat in result.all()] + + +@router.post("", status_code=201) +async def create_category( + data: CategoryCreate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> CategoryRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + category = Category(household_id=user.household_id, name=data.name) + session.add(category) + try: + await session.commit() + except IntegrityError: + # Let the DB enforce the unique constraint on (household_id, name) rather than + # doing a pre-check query. The broad IntegrityError catch is intentional — inspecting + # the underlying driver exception (e.g. asyncpg's UniqueViolationError) is fragile + # across SQLAlchemy versions. In practice, FK and NOT NULL violations are ruled out + # here: household_id is server-set and name is validated by Pydantic before commit. + await session.rollback() + raise HTTPException(status_code=409, detail="Integrity error: a category with this name may already exist in this household") from None + await session.refresh(category) + return CategoryRead.model_validate(category) + + +@router.patch("/{category_id}") +async def update_category( + category_id: int, + data: CategoryUpdate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> CategoryRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + category = await session.get(Category, category_id) + if category is None or category.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Category not found") + category.name = data.name + session.add(category) + try: + await session.commit() + except IntegrityError: + # Same reasoning as create_category — unique constraint on (household_id, name). + await session.rollback() + raise HTTPException(status_code=409, detail="Integrity error: a category with this name may already exist in this household") from None + await session.refresh(category) + return CategoryRead.model_validate(category) + + +@router.delete("/{category_id}", status_code=204) +async def delete_category( + category_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> None: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + category = await session.get(Category, category_id) + if category is None or category.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Category not found") + # Prevent deleting a category that has items — would silently uncategorise inventory. + in_use = ( + await session.exec(select(Item.id).where(Item.category_id == category_id).limit(1)) + ).first() + if in_use is not None: + raise HTTPException( + status_code=409, + detail="Cannot delete a category that has items", + ) + await session.delete(category) + await session.commit() diff --git a/backend/app/routers/household.py b/backend/app/routers/household.py index 2247761..fc6fc7d 100644 --- a/backend/app/routers/household.py +++ b/backend/app/routers/household.py @@ -1,7 +1,7 @@ from typing import Annotated from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy.ext.asyncio import AsyncSession +from sqlmodel.ext.asyncio.session import AsyncSession from app.auth.users import current_active_user from app.database import get_session diff --git a/backend/app/routers/items.py b/backend/app/routers/items.py new file mode 100644 index 0000000..c7c55ab --- /dev/null +++ b/backend/app/routers/items.py @@ -0,0 +1,170 @@ +from datetime import datetime +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException, Response +from sqlmodel import Field, SQLModel, col, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.auth.users import current_active_user +from app.database import get_session +from app.models.batch import Batch +from app.models.category import Category +from app.models.item import Item +from app.models.user import User + +router = APIRouter(prefix="/items", tags=["items"]) + + +class ItemRead(SQLModel): + id: int + name: str + barcode: str | None + brand: str | None + image_url: str | None + category_id: int | None + unit: str + notes: str | None + created_at: datetime + updated_at: datetime + + +class ItemCreate(SQLModel): + name: str = Field(max_length=200) + barcode: str | None = Field(default=None, max_length=50) + brand: str | None = Field(default=None, max_length=200) + image_url: str | None = Field(default=None, max_length=500) + category_id: int | None = None + unit: str = Field(max_length=50) + notes: str | None = Field(default=None, max_length=1000) + + +class ItemUpdate(SQLModel): + name: str | None = Field(default=None, max_length=200) + barcode: str | None = Field(default=None, max_length=50) + brand: str | None = Field(default=None, max_length=200) + image_url: str | None = Field(default=None, max_length=500) + category_id: int | None = None + unit: str | None = Field(default=None, max_length=50) + notes: str | None = Field(default=None, max_length=1000) + + +@router.get("") +async def list_items( + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], + name: str | None = None, + location_id: int | None = None, +) -> list[ItemRead]: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + query = select(Item).where(Item.household_id == user.household_id) + if name is not None: + # col() wraps the model attribute as a SQLAlchemy column expression so + # Pyright knows .ilike() is a SQL method, not a str method. + query = query.where(col(Item.name).ilike(f"%{name}%")) + if location_id is not None: + matched = select(Batch.item_id).where( + Batch.location_id == location_id, + Batch.household_id == user.household_id, + ) + query = query.where(col(Item.id).in_(matched)) + result = await session.exec(query.order_by(Item.name)) + return [ItemRead.model_validate(item) for item in result.all()] + + +@router.get("/{item_id}") +async def get_item( + item_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> ItemRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + item = await session.get(Item, item_id) + if item is None or item.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Item not found") + return ItemRead.model_validate(item) + + +@router.post("", status_code=201) +async def create_item( + data: ItemCreate, + response: Response, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> ItemRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + # If a barcode is provided and already exists in this household, return the existing + # item (200) so the client can proceed to add a new Batch against it. + if data.barcode is not None: + existing = ( + await session.exec( + select(Item).where( + Item.household_id == user.household_id, + Item.barcode == data.barcode, + ) + ) + ).first() + if existing is not None: + response.status_code = 200 + return ItemRead.model_validate(existing) + + if data.category_id is not None: + category = await session.get(Category, data.category_id) + if category is None or category.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Category not found") + + item = Item(household_id=user.household_id, **data.model_dump()) + session.add(item) + await session.commit() + await session.refresh(item) + return ItemRead.model_validate(item) + + +@router.patch("/{item_id}") +async def update_item( + item_id: int, + data: ItemUpdate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> ItemRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + item = await session.get(Item, item_id) + if item is None or item.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Item not found") + + updates = data.model_dump(exclude_unset=True) + + if "category_id" in updates and updates["category_id"] is not None: + category = await session.get(Category, updates["category_id"]) + if category is None or category.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Category not found") + + for field, value in updates.items(): + setattr(item, field, value) + + session.add(item) + await session.commit() + await session.refresh(item) + return ItemRead.model_validate(item) + + +@router.delete("/{item_id}", status_code=204) +async def delete_item( + item_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> None: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + item = await session.get(Item, item_id) + if item is None or item.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Item not found") + # Cascade: remove all batches before deleting the item to respect the FK constraint. + batches = (await session.exec(select(Batch).where(Batch.item_id == item_id))).all() + for batch in batches: + await session.delete(batch) + await session.delete(item) + await session.commit() diff --git a/backend/app/routers/locations.py b/backend/app/routers/locations.py new file mode 100644 index 0000000..3e08b01 --- /dev/null +++ b/backend/app/routers/locations.py @@ -0,0 +1,168 @@ +from typing import Annotated + +from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.exc import IntegrityError +from sqlmodel import Field, SQLModel, select +from sqlmodel.ext.asyncio.session import AsyncSession + +from app.auth.users import current_active_user +from app.database import get_session +from app.models.batch import Batch +from app.models.location import Location +from app.models.user import User + +router = APIRouter(prefix="/locations", tags=["locations"]) + + +class LocationRead(SQLModel): + id: int + name: str + + +class LocationCreate(SQLModel): + name: str = Field(max_length=100) + + +class LocationUpdate(SQLModel): + name: str = Field(max_length=100) + + +@router.get("") +async def list_locations( + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> list[LocationRead]: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + result = await session.exec( + select(Location).where(Location.household_id == user.household_id) + ) + return [LocationRead.model_validate(loc) for loc in result.all()] + + +@router.post("", status_code=201) +async def create_location( + data: LocationCreate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> LocationRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + location = Location(household_id=user.household_id, name=data.name) + session.add(location) + try: + await session.commit() + except IntegrityError: + # Let the DB enforce the unique constraint on (household_id, name) rather than + # doing a pre-check query. The broad IntegrityError catch is intentional — inspecting + # the underlying driver exception (e.g. asyncpg's UniqueViolationError) is fragile + # across SQLAlchemy versions. In practice, FK and NOT NULL violations are ruled out + # here: household_id is server-set and name is validated by Pydantic before commit. + await session.rollback() + raise HTTPException(status_code=409, detail="Integrity error: a location with this name may already exist in this household") from None + await session.refresh(location) + return LocationRead.model_validate(location) + + +@router.patch("/{location_id}") +async def update_location( + location_id: int, + data: LocationUpdate, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], +) -> LocationRead: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + location = await session.get(Location, location_id) + if location is None or location.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Location not found") + location.name = data.name + session.add(location) + try: + await session.commit() + except IntegrityError: + # Same reasoning as create_location — unique constraint on (household_id, name). + await session.rollback() + raise HTTPException(status_code=409, detail="Integrity error: a location with this name may already exist in this household") from None + await session.refresh(location) + return LocationRead.model_validate(location) + + +@router.delete("/{location_id}", status_code=204) +async def delete_location( + location_id: int, + user: Annotated[User, Depends(current_active_user)], + session: Annotated[AsyncSession, Depends(get_session)], + move_to: int | None = None, +) -> None: + if user.household_id is None: + raise HTTPException(status_code=400, detail="No household assigned to this account") + + location = await session.get(Location, location_id) + if location is None or location.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Location not found") + + # Cannot delete the last location — household would have nowhere to store batches. + location_count = len( + (await session.exec( + select(Location).where(Location.household_id == user.household_id) + )).all() + ) + if location_count <= 1: + raise HTTPException( + status_code=409, + detail="Cannot delete the only location in your household", + ) + + has_batches = ( + await session.exec(select(Batch.id).where(Batch.location_id == location_id).limit(1)) + ).first() + + if has_batches is not None: + if move_to is None: + raise HTTPException( + status_code=409, + detail="This location has batches. Provide ?move_to= to move them to another location first.", + ) + if move_to == location_id: + raise HTTPException( + status_code=400, + detail="move_to must be a different location", + ) + target = await session.get(Location, move_to) + if target is None or target.household_id != user.household_id: + raise HTTPException(status_code=404, detail="Target location not found") + + # Move all batches to the target location before deleting. + # If the target already has a batch for the same (item, expiry), merge by + # adding quantities rather than creating a duplicate — which would violate + # the unique constraint on (item_id, location_id, expiry_date). + batches = ( + await session.exec( + select(Batch).where( + Batch.location_id == location_id, + Batch.household_id == user.household_id, + ) + ) + ).all() + for batch in batches: + existing = ( + await session.exec( + select(Batch).where( + Batch.item_id == batch.item_id, + Batch.location_id == move_to, + Batch.expiry_date == batch.expiry_date, + Batch.household_id == user.household_id, + ) + ) + ).first() + if existing is not None: + existing.quantity += batch.quantity + session.add(existing) + await session.delete(batch) + else: + batch.location_id = move_to + session.add(batch) + + await session.delete(location) + await session.commit() diff --git a/backend/app/routers/lookup.py b/backend/app/routers/lookup.py new file mode 100644 index 0000000..f40790b --- /dev/null +++ b/backend/app/routers/lookup.py @@ -0,0 +1,53 @@ +from typing import Annotated + +import httpx +from fastapi import APIRouter, Depends, HTTPException +from sqlmodel import SQLModel + +from app.auth.users import current_active_user +from app.models.user import User + +router = APIRouter(tags=["lookup"]) + + +class BarcodeResult(SQLModel): + barcode: str + name: str | None + brand: str | None + image_url: str | None + # Raw category tag from Open Food Facts (e.g. "en:beverages") — not a Category id. + category_hint: str | None + + +@router.get("/lookup/barcode/{barcode}") +async def lookup_barcode( + barcode: str, + # Dependency used only to require authentication; result not needed in the handler. + _user: Annotated[User, Depends(current_active_user)], +) -> BarcodeResult: + url = f"https://world.openfoodfacts.org/api/v0/product/{barcode}.json" + try: + async with httpx.AsyncClient(timeout=5.0) as client: + response = await client.get(url) + except httpx.HTTPError as exc: + raise HTTPException( + status_code=502, detail="Product lookup service unavailable" + ) from exc + + if response.status_code != 200: + raise HTTPException(status_code=502, detail=f"Product lookup service returned {response.status_code}") + + data = response.json() + if data.get("status") != 1: + raise HTTPException(status_code=404, detail="Product not found") + + product = data["product"] + tags: list[str] = product.get("categories_tags") or [] + + return BarcodeResult( + barcode=barcode, + name=product.get("product_name") or product.get("product_name_en"), + brand=product.get("brands"), + image_url=product.get("image_front_url"), + category_hint=tags[0] if tags else None, + ) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index ec988ce..b6bf3da 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -11,6 +11,7 @@ dependencies = [ "pydantic-settings>=2.14.1", "fastapi-users>=13.0,<14.0", "fastapi-users-db-sqlalchemy>=7.0", + "httpx>=0.27", ] [tool.ruff] target-version = "py313" @@ -39,8 +40,6 @@ extend-select = [ ] ignore = [ - "ANN101", # no type for self — unnecessary - "ANN102", # no type for cls — unnecessary "E501", # line too long — formatter handles this "S311", # pseudo-random ok for non-crypto use "TRY003", # long exception messages ok diff --git a/docs/spec.md b/docs/spec.md index 963d7da..b0661c9 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -15,12 +15,23 @@ A mobile-friendly web app for managing household inventory — food, cleaning pr ## 2. Version Roadmap -### v1 — Inventory App (this spec) +### v1 — Backend (complete) - Auth + household management -- Scan items in/out via barcode -- Inventory browsing + search +- CRUD for locations, categories, items, batches +- Barcode lookup proxy (Open Food Facts) - Expiring soon view +### v1 — Frontend (in progress) +Intentionally a thin slice — get something working and user-friendly first, then add features based on real usage. Feature prioritisation happens after the first working version, not before. + +Minimum to ship: +- Auth (register, login) +- Inventory list +- Item detail + batch list +- Scan flow (barcode → add batch) + +Filters, search, and additional views are deferred until real usage shows what's needed. No v2 FE scope defined yet. + ### v2 — Agent Layer - Weekly agent run: reasons over inventory - Writes shopping list to DB @@ -115,12 +126,15 @@ One group of units sharing the same location and expiry date. |---|---|---| | id | int | Primary key | | item_id | int | FK → Item | +| household_id | int | FK → Household — denormalized from Item for query isolation | | location_id | int | FK → Location | -| quantity | int | | -| expiry_date | date | Month + year precision, defaults to +12 months | +| quantity | int | Must be > 0 | +| expiry_date | date | Stored as full date; UI shows month + year only. Defaults to +12 months | | created_at | datetime | | -**Why Batch exists:** The same product can be bought multiple times with different expiry dates, and/or stored across multiple locations. Each unique combination of location + expiry = one batch. +**Why Batch exists:** The same product can be bought multiple times with different expiry dates, and/or stored across multiple locations. Each unique combination of item + location + expiry = one batch. This is enforced by a DB unique constraint on `(item_id, location_id, expiry_date)`. + +**Merge on move:** When batches are moved to another location (via `DELETE /locations/{id}?move_to=`), if the target already has a batch for the same item + expiry date, the quantities are merged rather than creating a duplicate. **Real-world example:** Buy 6 passatas → create two batches: `{quantity: 4, location: cellar, expiry: June 2027}` and `{quantity: 2, location: pantry, expiry: June 2027}`. Inventory view shows total = 6, broken down by location. @@ -234,16 +248,17 @@ Registration forks into two paths depending on whether an invite token is presen - `DELETE /categories/{id}` ### Items -- `GET /items` (supports search: name, location, expiry) +- `GET /items` (supports search: `?name=`, `?location_id=`) - `GET /items/{id}` -- `POST /items` +- `POST /items` — upsert: if a barcode is provided and already exists in the household, returns the existing item with HTTP 200 so the client can proceed to add a batch. Returns 201 for a new item. - `PATCH /items/{id}` -- `DELETE /items/{id}` +- `DELETE /items/{id}` — cascades to all batches for that item ### Batches - `GET /items/{id}/batches` -- `POST /items/{id}/batches` -- `PATCH /batches/{id}` +- `POST /items/{id}/batches` — `location_id` is optional; server defaults to last-used location for the household, then falls back to the first seeded location +- `POST /batches/{id}/adjust` — preferred endpoint for scan in/out. Accepts `{"delta": int}` (positive = add stock, negative = use stock). Returns 200 + updated batch if quantity > 0 after adjustment; returns 204 and deletes the batch if quantity reaches 0 or below. +- `PATCH /batches/{id}` — direct quantity/location/expiry override; use adjust for scan flows - `DELETE /batches/{id}` ### Expiry @@ -273,7 +288,7 @@ Registration forks into two paths depending on whether an invite token is presen | Database | Postgres via Neon | No inactivity pause, free tier, real-world standard | | Auth | FastAPI Users | Educational, integrates with SQLModel, no black box | | SQLite vs Postgres | Postgres | Multi-user, multi-household, multi-device | -| Expiry granularity | Month + year only | Reduces friction, good enough for pantry use | +| Expiry granularity | Stored as full date, displayed as month + year | Full date allows precise filtering and sorting; month/year display reduces friction for the user | | Expiry default | +12 months from today | Zero friction for items where expiry doesn't matter | | Expiry OCR | v2 | Adds complexity, not needed for v1 | | Stock movements | v2 | Agent needs this to reason intelligently | @@ -287,6 +302,9 @@ Registration forks into two paths depending on whether an invite token is presen | Location on Batch not Item | location_id on Batch | Same product can live in multiple locations (e.g. cellar + pantry); batch = unique combo of location + expiry | | Location default | Last used location | Lowest friction for scanning session | | brand + image_url on Item | Added in v1 | Single fields, zero complexity; brand disambiguates products; retrofitting requires migration | +| Expiry date source | Manual entry in v1, OCR in v2 | Standard consumer barcodes (EAN-13, UPC-A) encode only the product ID — no expiry date. The date must be read from the packaging. Manual entry with a +1 year default covers v1; OCR via camera is the v2 solution. | +| 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. | --- diff --git a/docs/til.md b/docs/til.md new file mode 100644 index 0000000..10a39c2 --- /dev/null +++ b/docs/til.md @@ -0,0 +1,139 @@ +# TIL — Today I Learned + +> Living document — updated after each session via `/til`. +> Captures patterns, decisions, and gotchas from building homik. +> Every entry is grounded in real code in this repo. + +--- + +## FastAPI Patterns + +- **`APIRouter`** groups endpoints by domain. Registered in `main.py` with `app.include_router()`. No router needs to know about others. +- **`Depends()`** is dependency injection. FastAPI calls the function, gets the result, passes it to the handler. The handler declares what it needs; FastAPI wires it up. +- **`Annotated[Type, Depends(...)]`** is the modern DI pattern — bundles the type hint and the injection mechanism together. Pylance can resolve the type; FastAPI reads the `Depends`. +- **FastAPI caches dependencies per request.** If two handlers both depend on `get_session`, the session is created once. Same for `current_active_user`. +- **`status_code` on the decorator** sets the default response code. You can override it at runtime by injecting `Response` as a parameter and setting `response.status_code` in the body. Used in the POST /items upsert: 201 for new, 200 for existing. +- **Query parameters are implicit.** Any function parameter that isn't a path param, a Pydantic model, or a `Depends` is automatically treated as a query parameter. +- **`Query(ge=1)`** adds server-side validation to query params. FastAPI returns 422 automatically if validation fails — no manual check needed. +- **Router without prefix** (`APIRouter(tags=["batches"])`) is valid when your endpoints span multiple URL patterns. Batches use both `/items/{id}/batches` and `/batches/{id}`, so no single prefix fits. + +--- + +## SQLModel & SQLAlchemy Patterns + +- **`session.exec(select(Model).where(...))`** is SQLModel's query API. Think of `select(...)` as building the SQL statement; `session.exec()` runs it. +- **`session.get(Model, id)`** fetches by primary key. Checks the session identity cache before hitting the DB. +- **Write cycle:** `session.add(obj)` → `await session.commit()` → `await session.refresh(obj)`. The `refresh` is important — it reloads the row from the DB to get server-generated values like `id` and `created_at`. +- **`model_dump(exclude_unset=True)`** is the PATCH pattern. Pydantic tracks which fields were explicitly included in the request body. `exclude_unset=True` returns only those — fields the client didn't mention are left untouched on the ORM object. +- **`model_validate(orm_instance)`** converts an ORM object into a Pydantic schema. Used to go from table model → response schema, stripping fields like `household_id` that shouldn't be in the API response. +- **`col(Model.field)`** wraps a model attribute as a SQLAlchemy column expression. Needed for `.ilike()`, `.in_()`, `.desc()`, and `order_by()` — without it, Pyright sees Python types (`str`, `int`, `date`) and complains that those methods don't exist. +- **SQLModel's `AsyncSession` vs SQLAlchemy's.** SQLAlchemy's `AsyncSession` doesn't have `.exec()`. SQLModel's subclass (from `sqlmodel.ext.asyncio.session`) adds it. Configure the session factory with `async_sessionmaker(engine, class_=AsyncSession)` to get SQLModel sessions throughout. +- **Primary keys are `int | None` in SQLModel** before insertion. After `flush()` or `commit()` the PK is populated, but the type stays `int | None`. Use a `if pk is None: raise RuntimeError(...)` guard to narrow the type for Pyright. + +--- + +## Security & Data Isolation + +- **Every query must filter by `household_id`.** This is the most critical invariant in the codebase. A user must never see or modify another household's data. +- **Return 404 for wrong household, not 403.** Returning 403 would tell the caller "this ID exists but isn't yours" — which leaks information. 404 treats "not found" and "not yours" identically. +- **`household_id` is always set server-side.** It comes from `current_active_user`, never from the request body. The client has no say in which household an object belongs to. +- **`household_id` on Batch is denormalized.** It's copied from the Item so that every batch query can filter by household directly, without joining through items. This is a deliberate performance and isolation trade-off. +- **Guard `user.household_id is None` at the top of every endpoint.** The `User` model has `household_id: int | None` (nullable for future invite flow). Without the guard, a user with no household would get confusing 404s. With it, they get a clear 400. The guard also narrows the type for Pyright. +- **Subqueries for filtering should also filter by `household_id`.** In `GET /items?location_id=X`, the subquery finding batches at that location also filters `Batch.household_id == user.household_id` — even though the outer query already filters items by household. Belt and braces. + +--- + +## Python Type System + +- **Pyright doesn't know about SQLAlchemy column instrumentation.** At the class level, `Item.name` looks like `str` to Pyright. `col(Item.name)` is the SQLModel-provided escape hatch that says "treat this as a column expression". +- **`assert` is banned in production code (ruff S101).** Python's `-O` flag strips `assert` statements, so they can't be relied on for safety checks. Use an explicit `if x is None: raise RuntimeError(...)` instead. +- **Type narrowing with `if x is None: raise`.** After `if user.household_id is None: raise HTTPException(...)`, Pyright knows `user.household_id` is `int` for the rest of the function. No need for a separate variable. +- **`int | None` is the modern Python union syntax** (Python 3.10+). Equivalent to `Optional[int]` from `typing`. SQLModel uses it for nullable fields and primary keys. + +--- + +## Error Handling + +- **`IntegrityError`** is the SQLAlchemy exception for any DB constraint violation — unique constraints, FK violations, NOT NULL violations, check constraints. It's not specific to one type. +- **Broad `IntegrityError` catch is acceptable** when other causes are ruled out by design: if `household_id` is server-set (no FK violation), `name` is Pydantic-validated (no NOT NULL violation), and there are no check constraints — the only possible cause is the unique constraint. +- **Don't introspect the driver exception.** Checking `exc.orig.pgcode` (psycopg2 convention) or `isinstance(exc.orig, asyncpg.UniqueViolationError)` is fragile across SQLAlchemy/asyncpg versions. A well-commented broad catch is more honest. +- **Always `session.rollback()` before raising** after a failed commit. SQLAlchemy async sessions don't auto-rollback; leaving the session in a failed state causes subsequent operations to fail. +- **`raise X from None`** explicitly suppresses the exception chain. Right when you're intentionally translating a low-level error (DB constraint) into an API error (HTTP 409) and don't want DB internals in the traceback. +- **`raise X from exc`** preserves the chain. Right when you want the original error context to be visible (e.g., wrapping a network error in a 502). +- **Ruff B904** enforces that `raise` inside `except` blocks must have `from err` or `from None`. Without it, the implicit chaining is ambiguous. + +--- + +## 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. +- **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`. +- **Scan-out (deduction) flow in v1.** No dedicated deduct endpoint. Client fetches batch, calculates new quantity, PATCHes. If quantity hits zero, client deletes the batch. v2 plan: `POST /batches/{id}/adjust` with a signed `delta` that auto-deletes on zero. +- **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. +- **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. + +--- + +## Session Log + +### 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. + +Key decisions: +- `set -uo pipefail` without `-e` — lint exit codes must be captured, not abort the script +- `uv run` via subshell — no PATH assumptions, correct virtualenv always used +- `/fix-lint` skill classifies issues as "clear error" or "design decision" before acting — prevents silent semantic changes +- `docs/lint-report.md` gitignored — same pattern as `pr-comments.md` / `pr-description.md` + +--- + +### 2026-06-08 — CRUD refinements (PR: feat/pr-review-comments) + +Built: location delete with move/merge flow, batch adjust endpoint, unique constraint on batches, spec + docs updates. + +Key decisions: +- `DELETE /locations/{id}?move_to=` — query param pattern for cascading deletes +- Batch merge on move — quantities combined when target already has same (item, expiry) +- `(item_id, location_id, expiry_date)` unique constraint added to Batch model (requires table drop in Neon) +- `POST /batches/{id}/adjust` with delta — replaces PATCH+DELETE client branching +- `new_quantity < 0` → 422 (over-removal error); `== 0` → 204 (batch deleted) +- `expiry_before` filter removed from `GET /items` — FE categorises client-side +- FE v1 defined as intentionally thin slice; feature prioritisation deferred + +--- + +### 2026-06-05 — CRUD endpoints (PR: feat/pr-review-comments) + +Built: Location, Category, Item, Batch CRUD + barcode lookup proxy (`GET /lookup/barcode/{barcode}` → Open Food Facts). + +Key decisions made: +- IntegrityError broad catch pattern agreed and documented +- `household_id` None guard added to all endpoints (fixes type errors + weak spot) +- `col()` introduced to fix Pyright column expression errors +- SQLModel `AsyncSession` swap in `database.py` to expose `.exec()` +- `from None` added to all `raise HTTPException` inside `except` blocks (B904) +- `assert` replaced with `if ... raise RuntimeError` (S101) +- `days` param in `/expiring` bounded with `Query(ge=1)` +- PATCH `IntegrityError` handling added to locations and categories (bug: was missing) + +Errors fixed: 16 Pyright errors, 7 ruff errors.