Meal-Planner/Review/docs-gpt5.5.md

Adversarial Documentation Review

Verdict

Do not move directly into implementation from these docs without resolving the blockers below. The documentation is directionally useful, but it currently mixes aspirational design, stale implementation status, contradictory API/schema definitions, and missing security requirements. If implementation starts as-is, likely outcomes are rework in Phase 2, broken startup/API assumptions, and unauthenticated admin or approval workflows that become hard to retrofit.

Blocking Findings

1. Phase/status docs contradict current implementation state

docs/ORIENTATION.md:21-29 says Phase 1 is complete and SQLAlchemy models are created, while docs/implementation-plan.md:9-19 still lists Phase 1 setup as unchecked. docs/implementation-plan.md:70-97 says Phase 2 models/migrations are not done, but backend/app/models/__init__.py already contains model classes. This creates an immediate execution hazard: implementers cannot tell whether Phase 2 is greenfield, repair work, or migration alignment.

Required before implementation:

• Rewrite phase status into one canonical source of truth.
• Separate “implemented skeleton exists” from “verified and production-ready”.
• Add explicit acceptance criteria for Phase 2 based on current code, not original plan checkboxes.

2. Database schema is internally inconsistent and conflicts with current models

docs/database-schema.md:45-73 defines recipe.ingredients as JSONB, but docs/ARCHITECTURE.md:238-244 and docs/ORIENTATION.md:100-105 describe a recipe_ingredient junction relationship. Current code includes both Recipe.ingredients JSONB and a RecipeIngredient table, while RecipeIngredient.recipe = relationship("Recipe", back_populates="recipe_ingredients") has no matching Recipe.recipe_ingredients relationship in the model. This is not just documentation drift; it points at an implementation design conflict that will break ORM mapping or duplicate ingredient truth.

Required before implementation:

• Choose either normalized recipe_ingredient as canonical or JSONB as canonical.
• Document why the chosen model supports unit conversion, shopping-list aggregation, pantry subtraction, and ingredient matching.
• Update schema docs, architecture diagrams, and model expectations to match one design.

3. API endpoint definitions conflict across docs

docs/implementation-plan.md:110-115 specifies POST /api/meals/{id}/approve, GET /api/deny/{token}, and POST /api/deny/{token}. docs/ARCHITECTURE.md:252-273 specifies POST /api/meals/{id}/deny, GET /api/approve/{token}, and token deny routes. docs/RUNNING.md:140-151 references admin routes that are not protected in the docs. These differences matter because frontend routing, email templates, and backend routers will depend on exact paths.

Required before implementation:

• Publish one OpenAPI-aligned endpoint table.
• Distinguish authenticated web actions from unauthenticated email-token actions.
• Define request/response schemas, status codes, redirects, token expiry errors, and idempotency for approve/deny flows.

4. Security model is missing for externally reachable workflows

The docs state remote access via nginx/Caddy (docs/SPEC.md:127-130) and tokenized email approval links (docs/ARCHITECTURE.md:270-273), but docs/ORIENTATION.md:190-194 says “JWT-free for MVP” without defining replacement controls. Admin endpoints like POST /api/admin/scrape and log views are documented (docs/implementation-plan.md:124-127, docs/RUNNING.md:140-151) but no authentication, authorization, CSRF protection, rate limits, or network restrictions are specified.

Required before implementation:

• Define MVP auth explicitly: session, basic auth behind reverse proxy, signed email tokens, or VPN-only deployment.
• Require strong random token generation, hashing-at-rest or equivalent protection, expiry, single-use/rotation behavior, and audit logging for email approvals.
• Protect admin endpoints before exposing remote access.
• Define CORS, trusted hosts, proxy headers, secure cookies if sessions are used, and secret management for SECRET_KEY.

5. Migration strategy conflicts with runtime table creation

Docs repeatedly require Alembic migrations (docs/RUNNING.md:104-115, docs/implementation-plan.md:91-97), but current backend startup calls Base.metadata.create_all(bind=engine) in backend/app/main.py:17. Starting implementation from the docs without addressing this will create unmanaged tables, then make Alembic autogeneration noisy or unsafe.

Required before implementation:

• Document whether Phase 2 must remove runtime create_all.
• Define initial migration ownership and whether existing dev DBs can be dropped.
• Add a verification command that proves migrations, not app startup, create schema.

6. Current code appears incompatible with documented model layout

docs/implementation-plan.md:72-85 suggests one model per table and backend/app/models/__init__.py with imports. Current backend/app/main.py:4 imports from app.models import family_profile, ingredient, recipe, meal_plan, home_pantry, feedback, grocery_item, scrape_log, email_log, but actual backend/app/models/__init__.py defines all classes in one file and no such submodules exist. The docs do not warn implementers that the current skeleton may not start.

Required before implementation:

• Verify and document current startup status.
• Decide whether model files will be split or imports fixed.
• Make docs describe the actual current state, including known broken skeleton areas.

High-Risk Gaps

7. Scraping feasibility is under-specified

The docs say scrape Lucky California weekly ads and product catalog while respecting robots.txt (docs/SPEC.md:132-136), but there is no feasibility spike, selector strategy, anti-bot handling policy, cache policy, legal/compliance note, or fallback data-entry schema. docs/implementation-plan.md:163 only verifies “10+ sale items”, which does not prove stable mapping to ingredients, price units, sale windows, or aisles.

Recommendation:

• Add a Phase 4 spike before building dependent meal-planning logic.
• Define scraped product normalization rules: package size, price-per-unit, sale terms, loyalty-card requirements, store location, unavailable/out-of-stock states.
• Define manual fallback UX and data model before scraper failure handling is implemented.

8. Meal-planning algorithm lacks deterministic requirements

docs/ARCHITECTURE.md:91-109 and docs/implementation-plan.md:189-215 describe constraints at a high level, but do not define scoring weights, hard vs soft constraints, conflict resolution, reproducibility, or failure behavior when no valid 7-day plan exists. “No mushrooms for 3/4 members” is listed, but not modeled as per-person preferences or household-level exclusion.

Recommendation:

• Define hard constraints, soft preferences, scoring weights, tie-breakers, and fallback behavior.
• Add example input/output fixtures for at least one valid plan and one no-solution case.
• Decide whether disliked ingredients exclude whole-family meals or permit optional substitutions.

9. Shopping-list math is not implementable from current docs

Shopping-list generation requires unit conversion, ingredient normalization, recipe scaling, pantry subtraction, and store item matching. Docs mention these features (docs/implementation-plan.md:180-184, docs/implementation-plan.md:305-327) but do not define canonical units, conversion tables, rounding rules, partial pantry quantities, or substitutions.

Recommendation:

• Add unit normalization requirements before implementing recipes and pantry.
• Define output rules for ambiguous items like “1 bunch cilantro”, “to taste salt”, and recipe-specific prepared ingredients.
• Add tests using real recipe-like data, not only CRUD checks.

10. Deployment docs promise production features that compose file does not provide

docs/RUNNING.md:47-57 documents SSL certs, while docker-compose.yml mounts only ./nginx/nginx.conf and no SSL directory. docs/RUNNING.md:75-80 references docker-compose.prod.yml, which does not exist. docs/ARCHITECTURE.md:329-336 says nginx handles SSL and rate limiting, but current nginx/nginx.conf only proxies /api/ and serves SPA fallback over port 80.

Recommendation:

• Mark production deployment as not implemented or add missing deployment docs later.
• Remove commands referencing nonexistent compose files until they exist.
• Define dev vs prod networking, TLS termination, and rate-limit boundaries explicitly.

11. Environment-variable docs are inconsistent and include unsafe examples

docs/ORIENTATION.md:128-147, docs/RUNNING.md:27-45, docs/implementation-plan.md:516-533, and .env.example use overlapping but not identical environment variables. Examples include API-key-like prefixes (SG..., sk-...) and SECRET_KEY=change-me-to-a-random-secret-key, but docs do not say which vars are required by backend startup, frontend build, or optional integrations.

Recommendation:

• Add a canonical env var table with required/optional/default/scope.
• Avoid realistic secret prefixes in docs where possible.
• Add startup validation expectations and safe local-dev defaults.

Medium-Risk Issues

12. Documentation contains stale and misspelled file paths

docs/implementation-plan.md:417-480 places RUNNING.md at project root, but actual docs list it under docs/RUNNING.md. The same tree misspells implemenation-plan.md at docs/implementation-plan.md:431. This will cause agent/tool confusion during implementation.

13. Tech-stack docs are already version stale

Docs repeatedly say React 18 (README.md:73, docs/ORIENTATION.md:63, docs/ARCHITECTURE.md:168), while current frontend dependencies use React ^18.2.0. That is not wrong today, but the docs lack a dependency-update policy and lockfile expectations. More important: docs/implementation-plan.md:43 suggests Create React App or Vite, but current project uses Vite, so CRA should be removed to avoid divergent setup.

14. Operational docs assume endpoints that are not proven to exist

docs/RUNNING.md:280-288 documents /health/db; current implementation has this route, but its SQLAlchemy call uses a raw string execute in backend/app/main.py:28, which may fail under SQLAlchemy 2 without text("SELECT 1"). Docs should avoid declaring health checks verified until command output is captured.

15. Data retention lacks implementation mechanics

docs/database-schema.md:265-274 defines retention windows, but no job owner, schedule, archival storage, or privacy behavior is described. “Archive to JSON, delete rows” for meal plans can break feedback history if FK behavior is not designed.

16. Accessibility is barely addressed despite family-facing UI

The docs require a non-technical family web UI and email workflow, but do not specify WCAG targets, keyboard behavior, color contrast, readable print views, or email-client fallbacks beyond plain text. Approval/denial links in email need accessible labels and safe confirmation UX.

17. Observability requirements are aspirational

docs/ARCHITECTURE.md:386-404 lists log categories and future monitoring, but no structured log schema, correlation IDs, error taxonomy, or alert thresholds. Scraper/email failures are core product risks, so this should be in MVP docs, not future polish.

Suggested Gate Before Implementation

Resolve these docs before writing Phase 2 code:

• One canonical project status page that reflects actual current code and broken areas.
• One canonical schema with either JSONB recipe ingredients or normalized recipe_ingredient, not both as competing truths.
• One canonical API contract, preferably generated or mirrored from OpenAPI.
• MVP security section covering admin routes, approval tokens, remote access, secrets, CORS, rate limiting, and audit logs.
• Migration policy that removes or explicitly forbids runtime create_all outside throwaway local dev.
• Deployment doc split into current local-dev support vs future production support.
• Verification matrix with exact commands and expected output per phase.

Implementation Readiness Rating

Current documentation readiness: 5/10.

The docs are good enough to understand product intent, but not good enough to drive implementation safely. Main risk is not missing detail; main risk is contradictory detail that will cause agents or engineers to implement incompatible schema/API/security assumptions.