Public Access
feat: phase r1+r2 recovery + r3-0 swiftly api ingestion
R1 stabilization: pytest harness with transactional db fixture, smoke + alembic + auth + scrape + approval + swiftly tests, github actions ci yaml. Bearer-token admin auth + signed-cookie session for family ui mutations. Async POST /api/admin/scrape (BackgroundTasks, returns 202). Path canonicalization (no /list, /planned suffixes). DATABASE_URL fail-fast on empty. R2 deferred-risk spikes: live lucky california fetch (R2-A), full email+per-voter approval click round trip with single-use enforcement (R2-B, console email backend, sendgrid stub). R3-0 phase 3 redesign: replaced playwright html scraper with requests based swiftly json api client. 17 categories, ~10k products per scrape, upsert by (source, external_id). 401 surfaces actionable token-refresh message via ScrapeLog.error_message. Pre-existing defects fixed: shopping_list.py syntax error blocking app import, MealPlan.votes orphan relationship, JSONB(astext=True) invalid kwarg, missing requests dep, calorie_target schema drift, every SQLEnum needed values_callable, 0001 had empty downgrade(), seed had duplicate ingredient rows. Migrations added: 0003 grocery_item.description, 0004 family_profile. calorie_target, 0005 grocery_item.external_id + source + composite index. Verified: 31/31 pytest green, alembic upgrade->downgrade->upgrade clean, frontend npm run build clean, live scrape 9,960 grocery_item rows in 36s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,63 @@
|
||||
# Phase R1 + R2 Verification Gate — PASSED
|
||||
|
||||
Date: 2026-05-04
|
||||
|
||||
## Test results
|
||||
|
||||
```
|
||||
docker compose run pytest tests/ -q
|
||||
26 passed in 2.54s
|
||||
```
|
||||
|
||||
Suite: `test_smoke`, `test_alembic` (forward + roundtrip), `test_config`,
|
||||
`test_auth`, `test_scrape_endpoint`, `test_approval`, `test_lucky_ca_scraper`.
|
||||
|
||||
## Verification matrix
|
||||
|
||||
| Item | Result |
|
||||
|------|--------|
|
||||
| `from app.main import app; app.title` | "MealPlanner", 22 routes |
|
||||
| `alembic upgrade head` (fresh DB) | 0001 → 0002 → 0003 → 0004 clean |
|
||||
| `alembic upgrade → downgrade base → upgrade head` | clean (in-test) |
|
||||
| `pytest -q` | 26 passed, 0 failed |
|
||||
| `npm run build` (frontend) | clean, 264 KB |
|
||||
| `POST /api/admin/scrape` no token | 401 |
|
||||
| `POST /api/admin/scrape` bad bearer | 401 |
|
||||
| `POST /api/admin/scrape` good bearer | 202 + scrape_log_id |
|
||||
| `GET /api/profile` no session (read) | 200 |
|
||||
| `PUT /api/profile` no session (mutation) | 401 |
|
||||
| `POST /api/auth/login` good password | 204 + Set-Cookie |
|
||||
| `POST /api/auth/login` bad password | 401 |
|
||||
| Email approval round-trip (R2-B) | OK: send → GET 200 → POST 200 → replay 409, item_status=approved |
|
||||
| Live Lucky CA scrape (R2-A) | 11 coupons parsed, schema survives |
|
||||
|
||||
## Pre-existing defects discovered and fixed during the recovery pass
|
||||
|
||||
1. `backend/app/api/shopping_list.py:59` — `Ingredient.id.in_ all_ingredient_ids` syntax error blocking app import. One-char fix.
|
||||
2. `backend/app/models/__init__.py:201` — `MealPlan.votes` relationship had no FK target. Removed (votes are reachable via `MealPlan.items[*].votes`).
|
||||
3. `backend/alembic/versions/0001_initial_migration.py:101` — `JSONB(astext=True)` invalid kwarg. Dropped.
|
||||
4. `backend/alembic/versions/0001_initial_migration.py:288` — `downgrade()` was `pass`. Replaced with a DO block dropping all non-alembic tables + all public enum types.
|
||||
5. `backend/alembic/versions/0002_seed_data.py` — duplicate Chickpeas + Black Beans seed rows; INSERT not idempotent. Removed dupes, added `ON CONFLICT (name_lower) DO NOTHING`.
|
||||
6. `backend/requirements.txt` — scraper imports `requests` but it wasn't pinned. Added `requests==2.31.0`.
|
||||
7. `backend/app/models/__init__.py` — every `SQLEnum(...)` used the default name-based mapping, but the Postgres enum types use lowercase values. All occurrences now use `values_callable=lambda obj: [e.value for e in obj]`.
|
||||
8. Schema drift: `FamilyProfile.calorie_target` existed in the model but not in the migration. New migration `0004_family_profile_calorie_target.py` adds it.
|
||||
9. `docker-compose.yml` — backend service didn't pass `ADMIN_TOKEN` / `SESSION_PASSWORD` / `EMAIL_BACKEND` env to the container. Added.
|
||||
10. `backend/tests/test_config.py` — `importlib.reload` fired the `RuntimeError` outside `pytest.raises`. Refactored to construct `Settings(_env_file=None, DATABASE_URL="")` inside the assertion.
|
||||
11. `backend/app/api/admin.py` `POST /scrape` ran Playwright synchronously inside the request handler. Now enqueues via `BackgroundTasks` and returns 202.
|
||||
12. `grocery_item.description` column added (migration 0003) — the live scraper produces description but the model/schema didn't have a column.
|
||||
|
||||
## Decisions captured in `.agent/context.md`
|
||||
|
||||
- Auth model: bearer `ADMIN_TOKEN` for admin, signed-cookie session (itsdangerous, key=SECRET_KEY) for family UI mutations. Reads stay open inside the trusted network.
|
||||
- Login bootstrap: signs literal `"bootstrap"` if no FamilyProfile exists yet — first-run hatch. **Worth flagging to the user explicitly.**
|
||||
- Path canonicalization: dropped `/list` and `/planned` suffixes; routers use `@router.get("")`. Frontend updated.
|
||||
- Email backend: `ConsoleEmailBackend` for dev; SendGrid stub raises NotImplementedError until R3-C.
|
||||
|
||||
## Open issues tracked but not blocking the gate
|
||||
|
||||
- Task #8: `ScrapeStatus` enum lacks distinct queued/started states (R1-C reused STARTED).
|
||||
- Task #10: `scraper_service._run_scrape_in_background` line 105 has tz-aware/naive datetime subtraction TypeError. Affects bg task happy-path post-202; admin gate verified independently.
|
||||
|
||||
## Phase R1 + R2 — DONE
|
||||
|
||||
Per `.agent/plan.md`, R3 (Phase 4 recipe engine, Phase 5 planner algo, Phase 6 SendGrid, Phase 8 feedback UI, Phase 9 generation, Phase 10 images) is now unblocked. Schema has survived contact with the deferred-risk spikes the adversarial review demanded.
|
||||
@@ -0,0 +1,29 @@
|
||||
# R1-A — Verification Harness
|
||||
|
||||
## Files created (NEW only — no edits to existing files)
|
||||
|
||||
- `backend/pytest.ini` — testpaths=tests, asyncio auto, deprecation filter.
|
||||
- `backend/tests/__init__.py` — empty package marker.
|
||||
- `backend/tests/conftest.py` — sets `DATABASE_URL` from `TEST_DATABASE_URL` before app import; `requires_postgres` marker auto-skips when PG unreachable; session-scoped `_schema` runs `alembic upgrade head`; per-test `db` fixture uses connection+transaction rollback for isolation; `client` fixture overrides `get_db`.
|
||||
- `backend/tests/test_smoke.py` — `test_app_imports`, `test_health`, `test_health_db`, parametrized `test_router_list_endpoints` over the 8 paths in the spec. Asserts `< 500` and accepts {200, 307, 401, 404, 422}.
|
||||
- `backend/tests/test_alembic.py` — downgrade-base then upgrade-head round-trip.
|
||||
- `backend/requirements-dev.txt` — pytest, pytest-cov, pytest-asyncio, httpx (note: pytest + httpx + pytest-asyncio also pinned in `requirements.txt`; dev file holds the canonical dev set).
|
||||
- `.github/workflows/ci.yml` — `backend` job (postgres:15 service, py3.11, alembic upgrade, pytest -q) + `frontend` job (node20, npm ci, npm run build). Triggers on push + pull_request.
|
||||
|
||||
## Test outcomes (could not execute locally — Bash python denied)
|
||||
|
||||
By design:
|
||||
- `test_app_imports` — passes if `app.main` imports cleanly (DATABASE_URL set in conftest pre-import).
|
||||
- `test_health`, `test_health_db`, `test_router_list_endpoints[*]`, `test_alembic_upgrade_head_roundtrip` — all gated on `requires_postgres`. Pass when CI Postgres service is up; auto-skipped locally without `TEST_DATABASE_URL`.
|
||||
- No xfails added.
|
||||
|
||||
## Blockers / risks found
|
||||
|
||||
- `app/config.py::Settings` has no default for `DATABASE_URL` — any import path without env var crashes. Conftest works around it but production code is fragile. Flag for R1-B.
|
||||
- Alembic migrations use `postgresql.UUID/JSONB/ARRAY` — SQLite fallback impossible. CI requires PG service (already wired).
|
||||
- Spec listed `/api/recipes/ingredients` and `/api/meals` but actual routes are `/api/recipes/ingredients/list` and `/api/meals/planned`. Smoke test still validates router wiring (404 acceptable, no 5xx).
|
||||
- `/health` declares an unused `db` dependency — works but odd.
|
||||
|
||||
## CI yaml one-liner
|
||||
|
||||
Two jobs (`backend` w/ postgres:15 service runs alembic+pytest, `frontend` runs `npm ci && npm run build`) on push/PR.
|
||||
@@ -0,0 +1,39 @@
|
||||
# R1-B+D — Auth + path canonicalization + DATABASE_URL fail-fast
|
||||
|
||||
## Files added
|
||||
- `backend/app/security.py` — `require_admin` (bearer token vs `settings.ADMIN_TOKEN`), `require_session` (signed-cookie via itsdangerous), `issue_session`, `SESSION_COOKIE`, `SESSION_MAX_AGE`.
|
||||
- `backend/app/api/auth.py` — `POST /api/auth/login` (shared `SESSION_PASSWORD`, sets httponly+secure+samesite=lax cookie), `POST /api/auth/logout` (clears cookie). Both return 204.
|
||||
- `backend/tests/test_auth.py` — admin bearer required, session cookie required for mutation, login/logout round-trip.
|
||||
- `backend/tests/test_config.py` — `Settings(_env_file=None, DATABASE_URL="")` raises `RuntimeError("DATABASE_URL is required")`.
|
||||
|
||||
## Files modified
|
||||
- `backend/app/config.py` — added `ADMIN_TOKEN`, `SESSION_PASSWORD`; `DATABASE_URL` no longer typed as required (default `""`) but a `model_validator(mode="after")` raises `RuntimeError` if blank — gives a clear error instead of pydantic's confusing ValidationError.
|
||||
- `backend/app/main.py` — wired `auth.router` at `/api/auth`.
|
||||
- `backend/app/api/admin.py` — `APIRouter(dependencies=[Depends(require_admin)])` so EVERY admin route is bearer-gated.
|
||||
- `backend/app/api/profile.py` — session-gated: `PUT /`, `POST /members`, `DELETE /members/{id}`. GETs open. Trailing slashes dropped.
|
||||
- `backend/app/api/recipes.py` — rewritten so `/ingredients` GET/POST come before `/{recipe_id}`. Session-gated: `POST /`, `DELETE /{id}`, `POST /ingredients`. GETs open. `GET /ingredients/list` renamed → `GET /ingredients`. Trailing slashes dropped.
|
||||
- `backend/app/api/meals.py` — `GET /planned` renamed → `GET ""`. Session-gated: `POST /`, `POST /{id}/lock`, `POST /items/{id}/swap`. Per-voter approval token routes (`GET/POST /items/{id}/vote/{token}`) UNCHANGED.
|
||||
- `backend/app/api/pantry.py` — session-gated: `POST /`, `PUT /{id}`, `DELETE /{id}`. GETs open. Trailing slashes dropped.
|
||||
- `backend/app/api/shopping_list.py` — trailing slash dropped on root GET. No mutations exist.
|
||||
- `backend/tests/test_smoke.py` — canonical paths enforced; 307 (slash redirect) is now a failure.
|
||||
- `backend/requirements.txt` — added `itsdangerous==2.1.2`.
|
||||
- `frontend/src/api/index.ts` — `withCredentials: true`, added `auth.login/logout`, `recipes.listIngredients` → `/recipes/ingredients`, `meals.getPlanned` → `/meals`.
|
||||
- `.env.example` — added `ADMIN_TOKEN`, `SESSION_PASSWORD`.
|
||||
|
||||
## Routes admin-gated (bearer token)
|
||||
All `/api/admin/*`: `POST /scrape`, `GET /logs`, `GET /logs/{id}`, `GET /email-logs`, `GET /meal-plans`, `POST /test-email`, `GET /stats`.
|
||||
|
||||
## Routes session-gated (cookie)
|
||||
- `PUT /api/profile`, `POST /api/profile/members`, `DELETE /api/profile/members/{id}`
|
||||
- `POST /api/recipes`, `DELETE /api/recipes/{id}`, `POST /api/recipes/ingredients`
|
||||
- `POST /api/meals`, `POST /api/meals/{id}/lock`, `POST /api/meals/items/{id}/swap`
|
||||
- `POST /api/pantry`, `PUT /api/pantry/{id}`, `DELETE /api/pantry/{id}`
|
||||
|
||||
## Paths renamed
|
||||
- `GET /api/recipes/ingredients/list` → `GET /api/recipes/ingredients`
|
||||
- `GET /api/meals/planned` → `GET /api/meals`
|
||||
- All routers: `@router.get("/")` → `@router.get("")` (no trailing slash on resource roots)
|
||||
|
||||
## Blockers
|
||||
- Could not execute `pytest` or `python -c "from app.main import app"` locally — Bash python execution denied (same as R1-A). Verification deferred to CI. Logical review of imports/wiring done.
|
||||
- TestClient runs http; the `secure=True` cookie won't auto-roundtrip. `test_session_login_logout` manually re-sets the cookie to validate the signing path. Production (https via nginx) is unaffected.
|
||||
@@ -0,0 +1,64 @@
|
||||
# R1-C — Async scrape + grocery_item.description
|
||||
|
||||
## What changed
|
||||
|
||||
- `backend/app/api/admin.py` — `POST /api/admin/scrape` now returns **202**
|
||||
with body `{"status": "queued", "scrape_log_id": "<uuid>"}`. Endpoint
|
||||
delegates to `enqueue_scrape(db, source, scrape_type, background_tasks)`.
|
||||
- `backend/app/services/scraper_service.py` — split:
|
||||
- `enqueue_scrape()` inserts a `ScrapeLog` row (status=`STARTED`),
|
||||
`db.commit()`, then `background_tasks.add_task(_run_scrape_in_background, log.id, ...)`.
|
||||
Returns the refreshed log row.
|
||||
- `_run_scrape_in_background(log_id, source, scrape_type)` opens its OWN
|
||||
`SessionLocal()` (request session is gone by then), runs the scraper,
|
||||
sets terminal status (`SUCCESS`/`FAILED` + `error_message` + `duration_seconds`),
|
||||
`db.close()` in `finally`. Rolls back before writing FAILED to avoid
|
||||
detached instances.
|
||||
- `ScraperService.run_scrape` retained for direct/test callers; both paths
|
||||
share `_do_scrape()` and `_save_grocery_item()`.
|
||||
- `_save_grocery_item` now maps `item_data["description"]` → `GroceryItem.description`
|
||||
on both insert and update.
|
||||
- `backend/app/models/__init__.py` — added `description = Column(Text, nullable=True)` to `GroceryItem`.
|
||||
- `backend/alembic/versions/0003_grocery_item_description.py` — `revision="0003"`,
|
||||
`down_revision="0002"`. `op.add_column("grocery_item", sa.Column("description", sa.Text(), nullable=True))` / `op.drop_column` on downgrade.
|
||||
- `backend/tests/test_scrape_endpoint.py` — monkeypatches
|
||||
`_run_scrape_in_background` to a no-op, POSTs with valid bearer, asserts
|
||||
202 + `scrape_log_id`, reads back the row through the test session, asserts
|
||||
`status == ScrapeStatus.STARTED` and `completed_at is None`.
|
||||
|
||||
## Endpoint contract
|
||||
|
||||
`POST /api/admin/scrape` (admin bearer required):
|
||||
- Request: optional query `source` (default `lucky_california`), `scrape_type` (default `weekly_ad`).
|
||||
- Response: **202** + `{"status": "queued", "scrape_log_id": "<uuid>"}`.
|
||||
- Poll `GET /api/admin/logs/{id}` for terminal state.
|
||||
|
||||
## Status enum note
|
||||
|
||||
Existing `ScrapeStatus` enum is `started|success|failed` only — no
|
||||
`pending`/`running`/`completed` members. Added migration would have to alter
|
||||
the PG enum type (and seed data assumes 3-value). Reused `STARTED` as the
|
||||
queued/in-flight state; spec semantics map cleanly. If you want explicit
|
||||
`PENDING`/`RUNNING`/`COMPLETED`, that's a separate enum migration.
|
||||
|
||||
## DB session in the bg task
|
||||
|
||||
Imports `SessionLocal` from `app.database` at function call time and opens a
|
||||
fresh session per task; the request-scoped session is closed when the 202 is
|
||||
sent.
|
||||
|
||||
## Verification
|
||||
|
||||
- `pytest tests/test_lucky_ca_scraper.py -q` → 1 passed (R2-A still green).
|
||||
- `tests/test_scrape_endpoint.py` parses and is wired correctly; runs only
|
||||
with `TEST_DATABASE_URL` set (other tests follow the same convention).
|
||||
- `python -c "from app.services.scraper_service import enqueue_scrape, _run_scrape_in_background, ScraperService; from app.models import GroceryItem, ScrapeStatus"` → ok.
|
||||
|
||||
## Blocker (pre-existing, OUT OF R1-C SCOPE)
|
||||
|
||||
`backend/app/api/shopping_list.py:59` has a syntax error from commit c735d21:
|
||||
`Ingredient.id.in_ all_ingredient_ids` — missing parens, should be
|
||||
`Ingredient.id.in_(all_ingredient_ids)`. This blocks `from app.main import app`
|
||||
and therefore `tests/test_smoke.py::test_app_imports`. Untouched by R1-A,
|
||||
R1-B+D, R2-A, or R1-C — they all couldn't actually exercise the import path.
|
||||
Recommend a one-character fix in the next round (it's outside my owned files).
|
||||
@@ -0,0 +1,35 @@
|
||||
# R2-A — Lucky California live-scrape spike (DONE)
|
||||
|
||||
## Outcome
|
||||
|
||||
- ONE live fetch performed: `https://luckysupermarkets.com/coupons/Coupon%2Flu-featured-in-ad`, HTTP 200, 201 KB rendered HTML.
|
||||
- Fixture saved: `backend/tests/fixtures/lucky_ca/weekly_ad.html` + `weekly_ad.png` + `META.md`.
|
||||
- Parser now yields **11 items** from the captured page, each with non-empty `name`, `current_price`, and `image_url`.
|
||||
- Test `backend/tests/test_lucky_ca_scraper.py::test_parse_fixture` passes (`pytest` exit 0, 0.09 s, no network, no Playwright).
|
||||
|
||||
## Selectors (verified 2026-05-04)
|
||||
|
||||
- Card root: `div.coupon-card-wrapper` (Swiftly-rendered tiles).
|
||||
- Price + short name: `.coupon-card-value-text` ("$13.97 Pepsi 24 packs").
|
||||
- Long description: `.coupon-card-short-description`.
|
||||
- Image: nested `<img>` from `cdn.luckysupermarkets.com/loyalty/offer/<id>.jpg`.
|
||||
- No per-card hyperlink in DOM; offers are non-navigable tiles.
|
||||
|
||||
## Scraper diff (minimal, additive)
|
||||
|
||||
`backend/app/scraper/lucky_ca_scraper.py`: added `parse_featured_coupons_html(html)` and `_parse_coupon_card(card)`; rewrote `scrape_featured_coupons` to delegate to the parser. Old `_parse_coupon_item` retained as fallback. The original `h2/h3/a` regex selector was stale — it produced 1 item against the live DOM.
|
||||
|
||||
## Captcha / blocker check
|
||||
|
||||
The HTML contains a `<span id="recaptcha-element">` placeholder element (no challenge served). Title is `"Featured in Ad | Luckys Supermarket"`, all 13 `coupon-card-wrapper` cards rendered. **Not a wall.** The spike script's heuristic flagged the keyword; META.md documents the false positive.
|
||||
|
||||
## Schema impact (grocery_item)
|
||||
|
||||
- Survives contact with reality. Populated columns: `name`, `current_price`, `image_url`, `is_on_sale`, `scraped_at`, `scraped_url`.
|
||||
- `product_url` is **always NULL** on this site — keep nullable, don't index as required.
|
||||
- Parser also produces `description` (long offer text). Recommend either adding `description TEXT NULL` to `grocery_item` or dropping the field — currently no model column exists for it.
|
||||
- 2 of 13 cards filter out (no parseable price), expected (header/footer rows, e.g. "$35 minimum order"). Not a regression.
|
||||
|
||||
## Blockers
|
||||
|
||||
None. Phase R2-A clears the gate. Recommend a tiny migration adding `grocery_item.description TEXT NULL` before R3-A, otherwise the description is dropped silently when persisting.
|
||||
@@ -0,0 +1,82 @@
|
||||
# R2-B blockers — pre-existing defects discovered by the spike
|
||||
|
||||
The R2-B email + per-voter approval round-trip cannot complete the
|
||||
end-to-end DB-backed proof until both of the following pre-existing bugs
|
||||
are fixed. Both are OUT OF SCOPE for R2-B per the task brief.
|
||||
|
||||
## Blocker 1: model relationship bug — `MealPlan.votes`
|
||||
|
||||
**File:** `backend/app/models/__init__.py:201`
|
||||
|
||||
```python
|
||||
class MealPlan(Base):
|
||||
...
|
||||
votes = relationship("MealPlanVote", back_populates="meal_plan", cascade="all, delete-orphan")
|
||||
```
|
||||
|
||||
But `MealPlanVote` has no `meal_plan` relationship and no FK to
|
||||
`meal_plan.id` — only to `meal_plan_item.id` (line 235).
|
||||
|
||||
**Symptom:** `sqlalchemy.exc.NoForeignKeysError: Could not determine join
|
||||
condition between parent/child tables on relationship MealPlan.votes`
|
||||
fires the moment ANY mapper is configured (i.e. on the first ORM use of
|
||||
any model). Blocks every flow, not just the vote flow.
|
||||
|
||||
**Repro (with schema in place):**
|
||||
|
||||
```bash
|
||||
DATABASE_URL='postgresql://...' python -c "
|
||||
from app import models
|
||||
from sqlalchemy.orm import configure_mappers
|
||||
configure_mappers() # raises NoForeignKeysError
|
||||
"
|
||||
```
|
||||
|
||||
**Fix options (for whichever agent owns models):**
|
||||
1. Remove `MealPlan.votes` (votes are reachable via `MealPlan.items[*].votes`).
|
||||
2. Add `meal_plan_id` FK to `MealPlanVote` and a back-ref. Requires migration.
|
||||
3. Specify `primaryjoin="MealPlan.id == foreign(remote(MealPlanVote.meal_plan_item_id))"` via `MealPlanItem` — viewonly only.
|
||||
|
||||
Option 1 is least invasive and matches existing usage in `meals.py` (no
|
||||
code reads `MealPlan.votes`).
|
||||
|
||||
## Blocker 2: migration uses invalid kwarg — `JSONB(astext=True)`
|
||||
|
||||
**File:** `backend/alembic/versions/0001_initial_migration.py:101`
|
||||
|
||||
```python
|
||||
sa.Column('ingredients', postgresql.JSONB(astext=True), nullable=False),
|
||||
```
|
||||
|
||||
`astext` is not a valid `JSONB.__init__` kwarg in SQLAlchemy 2.x.
|
||||
|
||||
**Symptom:** `alembic upgrade head` fails with:
|
||||
`TypeError: JSON.__init__() got an unexpected keyword argument 'astext'`.
|
||||
|
||||
**Impact:** `backend/tests/test_alembic.py::test_alembic_upgrade_head_roundtrip`
|
||||
fails. `backend/tests/conftest.py::_schema` fixture errors, so any test
|
||||
marked `requires_postgres` errors at session bootstrap.
|
||||
|
||||
**Fix:** drop the kwarg. `astext` is a *runtime* attribute on a `JSONB`
|
||||
column expression for casting to text, not a column definition arg.
|
||||
|
||||
```python
|
||||
sa.Column('ingredients', postgresql.JSONB(), nullable=False),
|
||||
```
|
||||
|
||||
## Effect on R2-B
|
||||
|
||||
- Parts 1 (services), 2 (routes), and the unit-test portion of Part 4
|
||||
are complete and verified (3 unit tests pass, 3 DB-backed tests skip
|
||||
cleanly on no-PG environments).
|
||||
- Part 3 `--simulate-click` and the 3 DB-backed tests in Part 4 require
|
||||
a working schema. They will run as soon as Blocker 1 is fixed (the
|
||||
spike script can bootstrap its own schema via SQLAlchemy
|
||||
`Base.metadata.create_all` once the relationship resolves).
|
||||
|
||||
## Sandbox cleanup note
|
||||
|
||||
While diagnosing Blocker 1, this agent ran `Base.metadata.create_all`
|
||||
against `mealplanner-db-1` (192.168.144.2:5432, db `mealplanner`) and
|
||||
then dropped all created tables/types after the model error surfaced.
|
||||
Final state: only `alembic_version` (matches pre-spike state).
|
||||
@@ -0,0 +1,82 @@
|
||||
# R2-B — Email + per-voter approval round-trip spike
|
||||
|
||||
## Schema gap (top, per spec)
|
||||
|
||||
The spike surfaced a pre-existing **model relationship bug** that blocks
|
||||
the round-trip even after a clean schema bootstrap:
|
||||
`backend/app/models/__init__.py:201` declares
|
||||
`MealPlan.votes = relationship("MealPlanVote", back_populates="meal_plan", ...)`
|
||||
but `MealPlanVote` has no `meal_plan` FK or relationship. SQLAlchemy
|
||||
fails at first mapper configure with `NoForeignKeysError`. Details +
|
||||
two pre-existing migration bug in `r2b-blockers.md`. Per scope rule
|
||||
("If the spike reveals the schema can't support the flow, STOP, do not
|
||||
workaround"), I did NOT touch models or migrations.
|
||||
|
||||
## What landed
|
||||
|
||||
- `backend/app/services/email.py` — `EmailBackend` Protocol,
|
||||
`ConsoleEmailBackend` (stdout + JSONL outbox at
|
||||
`backend/var/email_outbox.jsonl`), `SendGridEmailBackend` stub
|
||||
raising `NotImplementedError("Wire SendGrid in R3-C")`,
|
||||
`get_email_backend()` switching on `settings.EMAIL_BACKEND`.
|
||||
- `backend/app/services/approval.py` — itsdangerous
|
||||
`URLSafeTimedSerializer`, salt `meal-approval-v1`, key
|
||||
`settings.SECRET_KEY`. `issue_token`, `verify_token` (raises 401),
|
||||
`consume_token` (verifies, matches URL `item_id`, looks up voter,
|
||||
enforces single-use via existing `MealPlanVote` row → 409). Single-
|
||||
use enforcement lives ONLY in `consume_token`.
|
||||
- `backend/app/api/meals.py` — replaced two vote routes:
|
||||
- `GET /api/meals/vote/{item_id}?token=...` → minimal accessible
|
||||
HTML page (lang attr, contrast, ARIA labels, JS-enhanced JSON
|
||||
POST with form fallback). Token only inside form `action`,
|
||||
never visible body.
|
||||
- `POST /api/meals/vote/{item_id}?token=...` body `{"vote": "approve"|"deny"}`.
|
||||
Records `MealPlanVote` (Boolean: True/False), then applies rule:
|
||||
any deny → DENIED; full electorate approved → APPROVED; else PENDING.
|
||||
Returns `{"status": "recorded", "item_status": "..."}`.
|
||||
- `backend/app/config.py` — added `EMAIL_BACKEND: str = "console"`.
|
||||
- `backend/requirements-dev.txt` — added `freezegun>=1.4`.
|
||||
- `.gitignore` — appended `backend/var/`.
|
||||
- `scripts/send_test_approval.py` — bootstraps profile/voter/recipe/
|
||||
plan/item, issues a token, sends via `ConsoleEmailBackend`, prints
|
||||
the URL. `--simulate-click {approve|deny}` drives a `TestClient`
|
||||
GET (asserts 200 + html) + POST (asserts 200, prints
|
||||
`item_status=...`) + a second POST (asserts 409 single-use).
|
||||
- `backend/tests/test_approval.py` — 6 tests as spec'd; 3 unit tests
|
||||
green; 3 `requires_postgres` tests written and skip cleanly on
|
||||
no-PG. They will pass once Blocker 1 is fixed.
|
||||
|
||||
## URL contract
|
||||
|
||||
`/api/meals/vote/{item_id}?token=<itsdangerous-signed JSON>`.
|
||||
Token payload: `{"item": str(uuid), "voter": str(uuid)}`. TTL 7 days.
|
||||
Single use enforced by `UniqueConstraint(meal_plan_item_id, family_member_id)`
|
||||
on `meal_plan_vote` (already in the schema).
|
||||
|
||||
## What the round-trip proved (and didn't)
|
||||
|
||||
- **Token layer** proved end-to-end (issue → verify → tamper → expire).
|
||||
- **Schema fit (paper)**: `family_member`, `meal_plan_item`,
|
||||
`meal_plan_vote` columns map cleanly to the flow.
|
||||
- **Schema fit (runtime)**: BLOCKED by `MealPlan.votes` defect.
|
||||
Cannot run the `--simulate-click` proof or the 3 DB tests until
|
||||
another agent fixes the model.
|
||||
|
||||
## Verification (what I ran)
|
||||
|
||||
- `pytest backend/tests/test_approval.py -q` → **3 passed, 3 skipped**.
|
||||
- `python -c "from app.services.email import get_email_backend; print(...)"`
|
||||
→ `ConsoleEmailBackend`.
|
||||
- `python scripts/send_test_approval.py --simulate-click approve` →
|
||||
fails at ORM init with the documented Blocker 1.
|
||||
|
||||
## Files
|
||||
|
||||
- created: `backend/app/services/__init__.py`,
|
||||
`backend/app/services/email.py`, `backend/app/services/approval.py`,
|
||||
`scripts/send_test_approval.py`,
|
||||
`backend/tests/test_approval.py`,
|
||||
`.agent/phase-summaries/r2b-blockers.md`,
|
||||
`.agent/phase-summaries/r2b-summary.md`.
|
||||
- modified: `backend/app/api/meals.py`, `backend/app/config.py`,
|
||||
`backend/requirements-dev.txt`, `.gitignore`.
|
||||
@@ -0,0 +1,56 @@
|
||||
# R3-0 (Swiftly API ingestion) — GATE PASSED
|
||||
|
||||
Date: 2026-05-05
|
||||
|
||||
## Live end-to-end scrape
|
||||
|
||||
```
|
||||
POST /api/admin/scrape (Bearer test-admin-token) → 202 + scrape_log_id
|
||||
[bg] Swiftly: discovered 17 categories
|
||||
[bg] Background scrape complete: 10908/10908 items saved
|
||||
final status: success
|
||||
items_scraped: 10908
|
||||
duration_seconds: 36
|
||||
grocery_item count (after upsert): 9960
|
||||
```
|
||||
|
||||
## Tests
|
||||
|
||||
```
|
||||
pytest -q tests/ → 31 passed in 2.41s
|
||||
```
|
||||
|
||||
Includes:
|
||||
- 26 tests from R1+R2 gate
|
||||
- 5 new Swiftly tests: parse categories fixture, parse category JSON, mapping invariants, 401 raises `SwiftlyAuthError`, background runner writes FAILED with actionable message.
|
||||
|
||||
## Schema
|
||||
|
||||
Migration `0005_grocery_item_external_id.py` adds:
|
||||
- `grocery_item.external_id TEXT NULLABLE` (Swiftly product id)
|
||||
- `grocery_item.source TEXT NULLABLE`
|
||||
- composite index `ix_grocery_item_source_external_id`
|
||||
|
||||
Idempotency key for upserts: `(source, external_id)` with fallback `(name, scraped_url)` for legacy R2-A rows.
|
||||
|
||||
## Resolved
|
||||
|
||||
- Task #11 (LuckyCaliforniaScraper `_browser` attr bug): obsoleted — the new client uses `requests`, no Playwright lifecycle.
|
||||
- The "Phase 3 complete on paper, broken in practice" condition flagged in the original synopsis is no longer true.
|
||||
|
||||
## Behavior to know
|
||||
|
||||
- `SWIFTLY_BEARER_TOKEN` is read from env at scraper init. The token in `.env.example` and `.env.test` is the user-supplied one and will expire (~hourly). On 401, scrape fails with `error_message` instructing the admin to capture a fresh token from luckysupermarkets.com's devtools network tab and update the env var.
|
||||
- Bearer header is scoped to `prod.swiftlyapi.net` only; never sent to the public categories page.
|
||||
- `LUCKY_STORE_ID=757` (Lucky California in San Pablo). Switch via env to target a different store.
|
||||
- Playwright is still pinned in `requirements.txt` (other future scrapers may want it). Removable later if no other use case lands.
|
||||
|
||||
## Open items
|
||||
|
||||
- Task #8: ScrapeStatus lacks distinct queued/started states. Cosmetic.
|
||||
- `.env.example` carries the live bearer token; user authorized. Refresh whenever the token expires.
|
||||
|
||||
## Phase 3 status
|
||||
|
||||
Was: Playwright scraper, 11 coupons, broken `_browser` attribute, 0 rows persisted via the endpoint.
|
||||
Now: JSON API client, 17 categories × N products, 9,960 rows persisted live, 31 tests green, 401-handling proven.
|
||||
@@ -0,0 +1,64 @@
|
||||
# R3-0 — Swiftly product API client (replaces Playwright path)
|
||||
|
||||
## Schema migration
|
||||
|
||||
Added `0005_grocery_item_external_id.py` (`down_revision='0004'`):
|
||||
|
||||
- `grocery_item.external_id` (`String(100)`, nullable, indexed)
|
||||
- `grocery_item.source` (`String(50)`, nullable)
|
||||
- composite index `ix_grocery_item_source_external_id`
|
||||
|
||||
Idempotency key for upserts is `(source, external_id)`. Both nullable so legacy R2-A rows (which lacked an external id) keep validating; the upsert path falls back to `(name, scraped_url)` when external_id is absent.
|
||||
|
||||
## Discovery + sample counts
|
||||
|
||||
- Categories page: 17 distinct slugs (e.g. `Product/meat_seafood`, `Product/produce`, `Product/dairy_eggs_cheese`, ...). Selector: `<a class="swiftlyCouponCategory" href="/categories/<urlencoded slug>">`, regex with double-lookahead so attribute order doesn't matter.
|
||||
- `Product/meat_seafood` JSON returned 256 items, all 256 mapped successfully — 82 on sale (`promoArea` present), 174 regular-only.
|
||||
|
||||
## Field mapping
|
||||
|
||||
| Swiftly JSON | grocery_item column |
|
||||
| --- | --- |
|
||||
| `id` | `external_id` (new) |
|
||||
| `name` | `name` |
|
||||
| `description` | `description` |
|
||||
| `brand` | `brand` |
|
||||
| `primaryImage.url` | `image_url` |
|
||||
| `price.ok.regPriceText` (e.g. `"$3.49 /lb"`) | `regular_price`, `unit` |
|
||||
| `price.ok.promoArea.promoText` | `sale_price`, `is_on_sale=True` |
|
||||
| (queried slug → tail) | `aisle` (e.g. `meat_seafood`) |
|
||||
| (constant) | `source = "lucky_california"` |
|
||||
| (none) | `product_url = NULL` (Swiftly exposes none) |
|
||||
| `validityText` | not parsed — `sale_start_date`/`sale_end_date` left null |
|
||||
|
||||
`current_price` = `sale_price` when on sale, else `regular_price`. Prices stored as `Decimal`.
|
||||
|
||||
## 401 handling
|
||||
|
||||
`SwiftlyAuthError` is a custom exception raised:
|
||||
|
||||
1. Up-front when `SWIFTLY_BEARER_TOKEN` is empty (no HTTP call).
|
||||
2. On `response.status_code == 401` BEFORE `raise_for_status` (which would have masked the 401 as a generic `HTTPError`). The new client uses `requests.Session.get` directly — `BaseScraper._get`'s retry-and-swallow path was bypassed deliberately, advisor flagged this as load-bearing.
|
||||
|
||||
`_run_scrape_in_background` catches all exceptions (existing behavior), writes `status=FAILED` + `error_message="SWIFTLY_BEARER_TOKEN expired — request a fresh token from the user (capture from luckysupermarkets.com network tab on a /search/api/v1 request)"`. Admin sees it via `GET /api/admin/logs/<id>`.
|
||||
|
||||
## Auth scoping
|
||||
|
||||
Two `requests.Session` objects: `public_session` (no auth, hits `luckysupermarkets.com`) and `api_session` (Authorization header attached per-request, hits `prod.swiftlyapi.net`). Bearer is never sent to the public host.
|
||||
|
||||
## Verification
|
||||
|
||||
- `pytest -q tests/test_swiftly_api.py` → 5 passed, 1 skipped (Postgres-only).
|
||||
- `pytest -q tests/` → 10 passed, 21 skipped (all skipped because no live Postgres in dev shell — same as R1+R2 gate-pass baseline).
|
||||
- `python scripts/spike_swiftly_ingest.py --confirm-live` → 17 categories discovered, 256 items in meat_seafood, sale + reg-only samples both render correctly with Decimal prices and unit="lb".
|
||||
- Docker stack POST verification deferred: no live Postgres in this shell. The test `test_background_runner_writes_failed_with_token_message` covers that path under `requires_postgres` and will run in CI / `docker compose` env.
|
||||
|
||||
## Caveats
|
||||
|
||||
- **Token in `.env.example` is a real (expiring) credential**, per task spec. It expired 2026-05-05 ~07:13 PT (`exp:1777991217`); I used it during the spike and it still worked. The user explicitly authorized this. Recommend gitignoring `.env.example` or rotating to a placeholder in a future cleanup task.
|
||||
- Old test `tests/test_lucky_ca_scraper.py` was deleted (it asserted on `parse_featured_coupons_html`, which no longer exists). R2-A's fixtures `weekly_ad.html`, `weekly_ad.png`, `META.md` retained per task spec.
|
||||
- `BaseScraper` and `SeleniumScraper` classes in `app/scraper/base.py` are no longer subclassed but kept untouched — they are still imported via `app.scraper.__init__` and may be useful for a future second store. No Playwright code path is exercised by `LuckyCaliforniaScraper` anymore, so the `_browser` attribute bug cannot recur.
|
||||
- Rate limit set to 0.75s/req in the new client (between the spec's 1–2 req/sec). Sequential walk of 17 categories @ ~250 items/category should run in ~15s server-side.
|
||||
- `_save_grocery_item` now `flush()`es instead of `commit()`ting per row; the outer commit happens in `_run_scrape_in_background` / `ScraperService.run_scrape`. Trade-off: a single bad row aborts the whole scrape's transaction. The API data is well-typed so this is acceptable; a future hardening could wrap each row in a savepoint.
|
||||
- Legacy fallback `(name, scraped_url)` upsert path is intentionally non-colliding with new rows: the new scraper writes `scraped_url="LuckyCaliforniaScraper:Product/<slug>"` whereas R2-A wrote `scraped_url=base_url`, so the two epochs of rows coexist without false matches.
|
||||
- Verification #4 (POST `/api/admin/scrape` against the live docker stack) was NOT run from this subagent shell — no live Postgres reachable. The unit test `test_background_runner_writes_failed_with_token_message` (Postgres-required, skips cleanly without it) covers the failure-path persistence; the success-path will run when the parent agent runs the suite inside `docker compose` per the R1+R2 gate-pass precedent.
|
||||
Reference in New Issue
Block a user