Files
Meal-Planner/Review/repo-gpt5.5.md
T
admin a0b16f7418 fix: address adversarial review blockers
All §1 consensus blockers and §2 high-risk gaps resolved:

Schema fixes:
- Remove RecipeIngredient join table, use JSONB for ingredients
- Add family_member table for per-voter approval tracking
- Add all ENUMs for status fields (no loose VARCHAR)
- Add CHECK constraints (household_size, rating 1-5, day_of_week)
- Add name_lower for case-insensitive ingredient matching
- Add grocery_item → ingredient FK
- Fix day_of_week to ISO-8601 (1=Monday, 7=Sunday)
- Remove calorie_target (nutrition is non-goal)

Approval flow redesign:
- Email link → confirmation page (GET), not auto-approve
- Actual vote is POST from confirmation page
- Per-voter tokens (single-use, 72h TTL)
- Record which member voted

Auth model:
- VPN-only for admin endpoints
- Session-based for family web UI

Docker hardening:
- Remove direct port exposure for backend/frontend
- nginx is sole entrypoint
- Add docker-compose.dev.yml for local dev

Skeleton fixes:
- Add missing Pantry.tsx page
- Add missing index.html (Vite entrypoint)
- Add package-lock.json
- Fix SQLAlchemy 2 text() for raw SQL
- Remove create_all from startup (use migrations)
- Configure Alembic properly

Docs updates:
- Update Lucky URL to luckysupermarkets.com
- Add WCAG 2.1 AA accessibility target
- Update family profile with correct mushroom preferences
- Add external dependencies list to SPEC

Verification:
- docker compose config: PASS
- docker compose build backend: PASS
- docker compose build frontend: PASS
- backend import: PASS
- alembic context: PASS
2026-05-04 20:11:05 -07:00

262 lines
12 KiB
Markdown

# Repository Analysis
## Executive Summary
The repository is an early skeleton, not an implementation-ready Phase 1 baseline. Product intent is well documented, but current code cannot be treated as a verified foundation: backend imports are inconsistent with file layout, frontend imports missing source files, migration infrastructure is absent despite Alembic being documented, and all API endpoints are placeholders.
Readiness rating: 4/10.
Recommended next move: stabilize the skeleton before implementing feature phases. Do not start meal planning, scraping, email, or shopping-list logic until app startup, build, migrations, and API contract are corrected.
## Current Repo Shape
Implemented or present:
- `docker-compose.yml` with `backend`, `frontend`, `db`, and `nginx` services.
- FastAPI app skeleton under `backend/app`.
- SQLAlchemy model classes colocated in `backend/app/models/__init__.py`.
- Placeholder API routers for profile, recipes, meals, shopping list, pantry, and admin.
- Vite/React/Tailwind frontend skeleton.
- Nginx configs under both `nginx/nginx.conf` and `frontend/nginx.conf`.
- Extensive docs under `docs/` plus `README.md`.
Missing or not present:
- `frontend/index.html`.
- `frontend/package-lock.json`, despite Dockerfile using `npm ci`.
- `frontend/src/pages/Pantry.tsx`, despite being imported by `App.tsx`.
- `backend/alembic.ini` and `backend/alembic/`.
- Backend `schemas/`, `services/`, and `scraper/` directories.
- Tests.
- Authn/authz layer.
- OpenAPI contract beyond FastAPI's generated placeholder routes.
## Blocking Findings
### 1. Backend likely fails during import because model module layout is inconsistent
`backend/app/main.py:4` imports `family_profile`, `ingredient`, `recipe`, `meal_plan`, `home_pantry`, `feedback`, `grocery_item`, `scrape_log`, and `email_log` from `app.models`. Those submodules do not exist. All model classes are currently defined in `backend/app/models/__init__.py`.
Impact: backend startup is likely broken before FastAPI can serve `/health`.
Fix direction:
- Either split models into the imported module files, or remove those submodule imports and import the package/classes consistently.
- Add a backend startup smoke test before continuing any feature work.
### 2. SQLAlchemy relationships contain at least one broken back-populates reference
`backend/app/models/__init__.py:247` sets `RecipeIngredient.recipe = relationship("Recipe", back_populates="recipe_ingredients")`, but `Recipe` does not define `recipe_ingredients`. This is likely to fail mapper configuration once SQLAlchemy evaluates relationships.
Impact: even if import paths are fixed, ORM use can fail at runtime.
Fix direction:
- Add `Recipe.recipe_ingredients` or remove/replace the relationship.
- Decide whether recipe ingredients are normalized via `recipe_ingredient`, stored in `recipe.ingredients` JSONB, or intentionally duplicated with clear ownership.
### 3. Frontend build is blocked by a missing imported page
`frontend/src/App.tsx:5` imports `./pages/Pantry`, and `frontend/src/App.tsx:32` routes `/pantry` to it. No `frontend/src/pages/Pantry.tsx` file exists.
Impact: TypeScript/Vite build should fail immediately.
Fix direction:
- Add a minimal `Pantry.tsx` placeholder, or remove the route/import until the page exists.
### 4. Frontend Docker build is blocked by missing npm lockfile
`frontend/Dockerfile:5-6` copies `package*.json` then runs `npm ci`. There is no `frontend/package-lock.json`.
Impact: `npm ci` fails without a lockfile, so the frontend container cannot build reproducibly.
Fix direction:
- Generate and commit `package-lock.json`, or change Dockerfile to use `npm install` for the skeleton phase. Prefer lockfile.
### 5. Vite app is missing required HTML entrypoint
No `frontend/index.html` exists. Vite expects an HTML entrypoint at project root unless customized.
Impact: local dev/build is blocked even after fixing `Pantry.tsx`.
Fix direction:
- Add standard Vite `index.html` pointing to `/src/main.tsx`.
### 6. Alembic is documented and installed but not configured
`backend/requirements.txt:4` includes Alembic and docs repeatedly call `alembic upgrade head`, but no `backend/alembic.ini` or `backend/alembic/` exists. Meanwhile `backend/app/main.py:17` calls `Base.metadata.create_all(bind=engine)` at import/startup.
Impact: schema ownership is confused. Runtime table creation can mask missing migrations and produce unmanaged dev DB state.
Fix direction:
- Create Alembic config and first migration.
- Remove `create_all` from normal app startup once migrations exist.
- Add a documented migration verification path.
## Important Findings
### 7. API routes are placeholders with no schemas or persistence
All API functions currently return static `{"message": ...}` payloads. Examples: `backend/app/api/profile.py:8-15`, `backend/app/api/recipes.py:8-20`, `backend/app/api/meals.py:8-20`, and `backend/app/api/admin.py:8-20`.
Impact: the route shape exists, but no request models, response models, validation, DB reads/writes, or errors exist. Treat these as route stubs, not usable endpoints.
Fix direction:
- Add Pydantic schemas before endpoint implementation.
- Implement one resource end-to-end first, probably profile or ingredients, to establish patterns.
### 8. Admin endpoints are unauthenticated
`backend/app/api/admin.py` exposes scrape trigger, logs, and test email routes without auth. Docs anticipate remote access via reverse proxy.
Impact: once implemented, these routes could trigger external scraping, send email, and expose operational logs.
Fix direction:
- Define auth before implementing admin behavior.
- At minimum, gate admin routes behind a dependency and environment-configured secret/session/proxy auth.
### 9. Email approval security is not implemented and not modeled yet
Models include `approval_token` and `approval_token_expires` in `MealPlanItem`, but no token route, validation, hashing, single-use behavior, or audit trail exists. Current routes approve/deny by meal ID, not token.
Impact: approval workflow cannot safely be exposed via email links.
Fix direction:
- Decide token route contract before frontend/email work.
- Use high-entropy signed or stored tokens with expiry and replay handling.
- Keep meal-ID approval routes authenticated if they remain.
### 10. `SECRET_KEY` has an unsafe default
`backend/app/config.py:13` sets `SECRET_KEY: str = "dev-secret-key"`. `.env.example` says to change it, but code does not enforce change outside dev.
Impact: future signed tokens/sessions could be deployed with a known secret.
Fix direction:
- Make `SECRET_KEY` required for non-development environments.
- Document environment mode and startup validation.
### 11. SQLAlchemy 2 health query may fail
`backend/app/main.py:28` calls `db.execute("SELECT 1")`. With SQLAlchemy 2, textual SQL should use `text("SELECT 1")`.
Impact: `/health/db` can report failure even when DB is reachable.
Fix direction:
- Use `from sqlalchemy import text` and `db.execute(text("SELECT 1"))`.
### 12. Docker Compose exposes backend and frontend directly plus nginx
`docker-compose.yml` maps backend `8000:8000`, frontend `3000:80`, and nginx `80:80`/`443:443`. This is convenient for dev but contradicts a production reverse-proxy boundary if deployed unchanged.
Impact: backend bypasses nginx controls such as future rate limiting/auth headers unless ports are removed or bound to localhost.
Fix direction:
- Split dev and prod compose files.
- In prod, expose only reverse proxy and keep backend/frontend on internal network.
### 13. Nginx SSL and rate limiting are not implemented
`nginx/nginx.conf` and `frontend/nginx.conf` only provide SPA fallback and `/api/` proxying over HTTP. No TLS cert mounts, rate limits, security headers, request size limits, or proxy timeout policy exist.
Impact: deployment docs overstate production readiness.
Fix direction:
- Keep current nginx config labeled as local/dev only.
- Add production config later with TLS, headers, and route-specific limits.
## Maintainability Findings
### 14. Model file is too large for early iteration and hides domain decisions
`backend/app/models/__init__.py` contains all models and several unresolved design choices. This makes migrations, ownership, and imports harder to reason about.
Recommendation:
- Split per aggregate/table after schema ownership is decided, or keep colocated temporarily but remove false submodule imports.
- Add tests that import and configure all mappers.
### 15. Schema constraints are only partially represented
Some documented constraints are in code, such as `day_of_week BETWEEN 0 AND 6` and feedback rating range. Others are missing or underspecified, such as family counts, valid status enums, meal type values, and never-suggest rules.
Recommendation:
- Prefer Python enums plus DB checks for status/meal type/reason fields.
- Add validation at Pydantic layer before DB writes.
### 16. API prefix behavior may produce trailing-slash-only endpoints
Routers are included with prefixes like `/api/profile`, and router paths use `/`. This exposes `/api/profile/` rather than `/api/profile` unless FastAPI redirects. Docs mostly specify no trailing slash.
Recommendation:
- Decide canonical trailing-slash style and update routes/docs together.
### 17. No repository-level quality gates exist
There are no tests, no backend lint/format config, no frontend lockfile, and no CI config. Docs mention pytest, Black, isort, ESLint, and Prettier, but config and scripts are incomplete.
Recommendation:
- Add minimal smoke tests first: backend import/app creation, mapper configuration, frontend build.
- Add formatting/lint configs only when used by scripts/CI.
## Documentation Alignment
The separate documentation review in `Review/docs-gpt5.5.md` covers doc-specific contradictions. Repo analysis confirms several documentation risks against current files:
- Phase status is overstated: skeleton is present but not verified.
- Alembic workflow is documented but absent.
- API route contract differs between docs and code.
- Production deployment docs exceed current compose/nginx capability.
- Model/schema docs do not resolve JSONB ingredients vs normalized ingredients.
## Recommended Remediation Order
### Phase 0: Make skeleton start and build
- Fix backend model imports.
- Fix SQLAlchemy `recipe_ingredients` relationship mismatch.
- Fix `/health/db` SQLAlchemy 2 textual SQL.
- Add `frontend/index.html`.
- Add or remove `Pantry.tsx` route.
- Add frontend lockfile or adjust Dockerfile.
- Run backend import smoke test and frontend build.
### Phase 1: Establish schema/migration baseline
- Decide ingredient representation.
- Configure Alembic.
- Generate initial migration.
- Remove runtime `create_all` from normal startup.
- Add seed-data strategy.
### Phase 2: Establish API and security baseline
- Define canonical OpenAPI contract.
- Add Pydantic schemas.
- Add auth dependency for admin and authenticated web routes.
- Define approval-token routes and behavior.
- Add CORS/trusted-host/proxy assumptions.
### Phase 3: Implement first vertical slice
- Implement profile or ingredient CRUD end-to-end.
- Add DB-backed tests.
- Wire frontend API client for one real endpoint.
- Use this slice as pattern for recipes, pantry, and meal plans.
## Verification Matrix To Add
Minimum commands expected before declaring foundation complete:
```bash
docker compose config
docker compose build backend
docker compose build frontend
docker compose up -d db
docker compose run --rm backend python -c "from app.main import app; print(app.title)"
docker compose run --rm backend alembic upgrade head
docker compose run --rm backend pytest
docker compose run --rm frontend npm run build
```
These commands may not pass today; they are the gate that should define “Phase 1 complete”.
## Final Assessment
The repo is valuable as a product specification plus rough skeleton. It should not be considered a working app foundation yet. Biggest immediate risk is building new features on top of a skeleton that likely does not start or build. Stabilize imports, frontend entrypoints, Docker builds, and migrations first; then implement one small DB-backed vertical slice before expanding into scraper/email/planner complexity.