From 3eb76e9b5caafb1ff29388cc4f8cc7355dd0b2f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Mar 2026 06:34:44 +0000 Subject: [PATCH 1/2] Fix API remove parameter and redesign cart/search feedback system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session analysis revealed the model burned 74 minutes debugging issues that better tool feedback would have prevented. This commit addresses the top 9 improvements from that analysis: A. Cart returns total_units (sum of qty) alongside items_count (unique line items) — fixes user-vs-model count confusion B. Add/remove/add_plan return cart state (total, items, units) after every mutation — eliminates verify-cart round-trips C. Store mismatch detection: products table tracks store_id, search warns when cached SKUs are from a different store D. search_batch tool: search up to 15 products in one call instead of N sequential search calls E. Price sanity checking: flags implausible unit prices (e.g. $0.18/lb chicken) with PRICE_DATA_SUSPECT warning F. Remove returns failure reason: not_in_cart (with cart_skus for context), api_error, or cart_fetch_failed — model stops retrying blind G. cart_diff tool: compare expected SKUs against actual cart, showing missing items, extra items, and quantity mismatches H. MCP workflow instructions updated: always check favorites first, use search_batch for multi-item searches, verify cart state from mutation responses, handle store_warning and price_suspect flags I. Blacklist integration: search results flag products matching blacklist notes or avoid_brands from memory 30 new tests covering all features. All 121 non-async tests pass. (38 pre-existing async test failures due to missing pytest-asyncio in this environment are unchanged.) https://claude.ai/code/session_01XuoAmJMDKPhHc7zViFVLQw --- raley_assistant/api.py | 19 ++ raley_assistant/db.py | 53 +++- raley_assistant/mcp_server.py | 433 +++++++++++++++++++++++--- raley_assistant/reasoning.py | 36 +++ tests/conftest.py | 12 + tests/test_new_features.py | 566 ++++++++++++++++++++++++++++++++++ 6 files changed, 1073 insertions(+), 46 deletions(-) create mode 100644 tests/conftest.py create mode 100644 tests/test_new_features.py diff --git a/raley_assistant/api.py b/raley_assistant/api.py index 66d5489..2b5573d 100644 --- a/raley_assistant/api.py +++ b/raley_assistant/api.py @@ -738,6 +738,25 @@ def get_cart(client: CurlClient) -> dict: # ============================================================================ +def get_store_id(client: CurlClient) -> str: + """Get the current store/fulfillment location ID from the session. + + Returns store identifier string, or empty string if unavailable. + """ + status, data = client.get(f"{BASE_URL}/api/auth/session") + if status != 200 or not isinstance(data, dict): + return "" + # The session API nests store info in various places depending on fulfillment type + user = data.get("user", {}) + store = user.get("store", {}) + store_id = store.get("id", "") or store.get("key", "") + if not store_id: + # Fallback: check fulfillment location + fulfillment = user.get("fulfillment", {}) + store_id = fulfillment.get("storeId", "") or fulfillment.get("locationId", "") + return str(store_id) if store_id else "" + + def check_session(client: CurlClient) -> dict | None: """Check if session is valid. Returns user data or None.""" status, data = client.get(f"{BASE_URL}/api/auth/session") diff --git a/raley_assistant/db.py b/raley_assistant/db.py index d020557..9f0462a 100644 --- a/raley_assistant/db.py +++ b/raley_assistant/db.py @@ -30,7 +30,8 @@ unit_oz REAL, price_per_oz REAL, last_seen TEXT NOT NULL, - first_seen TEXT NOT NULL + first_seen TEXT NOT NULL, + store_id TEXT DEFAULT '' ); CREATE TABLE IF NOT EXISTS price_history ( @@ -111,7 +112,7 @@ def get_connection() -> sqlite3.Connection: return conn -def sync_products_from_search(conn: sqlite3.Connection, products: list) -> int: +def sync_products_from_search(conn: sqlite3.Connection, products: list, store_id: str = "") -> int: """Sync Product objects from a search into the local DB. Upserts product records and appends price history observations. @@ -127,8 +128,8 @@ def sync_products_from_search(conn: sqlite3.Connection, products: list) -> int: conn.execute( """ INSERT INTO products (sku, name, brand, price_cents, sale_price_cents, - size, unit_oz, price_per_oz, last_seen, first_seen) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + size, unit_oz, price_per_oz, last_seen, first_seen, store_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(sku) DO UPDATE SET name = excluded.name, brand = excluded.brand, @@ -137,7 +138,8 @@ def sync_products_from_search(conn: sqlite3.Connection, products: list) -> int: size = excluded.size, unit_oz = excluded.unit_oz, price_per_oz = excluded.price_per_oz, - last_seen = excluded.last_seen + last_seen = excluded.last_seen, + store_id = CASE WHEN excluded.store_id != '' THEN excluded.store_id ELSE products.store_id END """, ( p.sku, @@ -150,6 +152,7 @@ def sync_products_from_search(conn: sqlite3.Connection, products: list) -> int: p.price_per_oz, now, now, + store_id, ), ) @@ -385,6 +388,46 @@ def is_good_deal( return False, f"Near average price (${avg/100:.2f})." +def check_store_mismatch(conn: sqlite3.Connection, current_store_id: str) -> dict | None: + """Check if cached products are from a different store. + + Returns a warning dict if most cached products have a different store_id, + or None if no mismatch detected. + """ + if not current_store_id: + return None + + row = conn.execute( + """ + SELECT store_id, COUNT(*) as cnt + FROM products + WHERE store_id != '' AND store_id != ? + GROUP BY store_id + ORDER BY cnt DESC + LIMIT 1 + """, + (current_store_id,), + ).fetchone() + + if not row: + return None + + total = conn.execute("SELECT COUNT(*) as c FROM products WHERE store_id != ''").fetchone()["c"] + mismatched = row["cnt"] + + if mismatched > 0 and total > 0 and mismatched / total > 0.5: + return { + "warning": "store_changed", + "cached_store": row["store_id"], + "current_store": current_store_id, + "stale_products": mismatched, + "message": f"Cache has {mismatched} products from a different store. " + f"Run price clear=true to refresh.", + } + + return None + + def search_products_local( conn: sqlite3.Connection, query: str, limit: int = 10 ) -> list[dict]: diff --git a/raley_assistant/mcp_server.py b/raley_assistant/mcp_server.py index 34a381c..7596e51 100644 --- a/raley_assistant/mcp_server.py +++ b/raley_assistant/mcp_server.py @@ -46,46 +46,66 @@ ## Tools -| Tool | What it does | -|-----------|-------------| -| search | Find products. Returns best match with SKU, price, sale status, unit price | -| add | Add single item to cart. Requires `sku` and `cents` from a search result | -| add_plan | Bulk add items: pass 'sku:cents,sku:cents:qty,...' from plan results | -| remove | Remove item from cart by SKU | -| cart | View current cart. Use `summary_only=true` for a quick total | -| offers | Coupon management: `list` unclipped, `clip_all`, or `sync` to local DB | -| plan | Parse a freeform grocery list, find matches + totals. Does not add to cart | -| price | Price history for a SKU, search local DB, or `clear=true` to reset cache | -| orders | Past order history with totals | -| favorites | Purchase history: `products` (recent), `brands` (by product count), `sync` (refresh) | -| deals | Best-value items this week: clipped coupons + sale prices + price history | -| memory | Read/write shopping memory. Use `section=t1d/shopping/notes` to filter | -| knowledge | Search T1D books. Use `book`+`heading` to fetch full section after searching | -| read_saved| Read a previously saved file (from `save_to_file=true`) | -| auth | Check if session cookies are valid | +| Tool | What it does | +|--------------|-------------| +| search | Find products. Returns best match with SKU, price, sale status, unit price | +| search_batch | Search multiple products at once (up to 15). Returns best match per query | +| add | Add single item to cart. Returns cart state after mutation | +| add_plan | Bulk add items: pass 'sku:cents,sku:cents:qty,...' from plan results | +| remove | Remove item from cart by SKU. Returns reason on failure + cart state | +| cart | View current cart. Includes both `items_count` (unique) and `total_units` (sum of qty) | +| cart_diff | Compare expected SKUs against actual cart. Shows missing, extra, qty mismatches | +| offers | Coupon management: `list` unclipped, `clip_all`, or `sync` to local DB | +| plan | Parse a freeform grocery list, find matches + totals. Does not add to cart | +| price | Price history for a SKU, search local DB, or `clear=true` to reset cache | +| orders | Past order history with totals | +| favorites | Purchase history: `products` (recent), `brands` (by product count), `sync` (refresh) | +| deals | Best-value items this week: clipped coupons + sale prices + price history | +| memory | Read/write shopping memory. Use `section=t1d/shopping/notes` to filter | +| knowledge | Search T1D books. Use `book`+`heading` to fetch full section after searching | +| read_saved | Read a previously saved file (from `save_to_file=true`) | +| auth | Check if session cookies are valid | ## Workflow -**Planning a trip**: Use `offers sync` first to pull fresh coupons, then `plan` -with the grocery list. Check `deals` for this week's best values. Confirm items -with the user before calling `add`. +**Planning a trip**: ALWAYS start by checking `favorites products` and +`favorites brands` to learn the user's preferred brands and purchase patterns. +Then `offers sync` + `offers clip_all` for fresh coupons. Use `plan` with the +grocery list, or `search_batch` for multiple queries at once (much faster than +calling `search` in a loop). Check `deals` for this week's best values. Confirm +items with the user before calling `add`. **Finding best value**: `search` returns unit pricing ($/oz, $/lb). Always check if the recommended item is on sale or has a clipped coupon via `deals`. For -price history context, call `price` with the SKU. +price history context, call `price` with the SKU. If a price looks implausibly +low (e.g. $0.18/lb for chicken), the result will include `price_suspect` — do +NOT recommend suspect prices without asking the user to verify on the website. **Coupon workflow**: `offers list` shows unclipped coupons. `offers clip_all` clips everything at once. After clipping, relevant discounts apply automatically at checkout — no promo code needed. -**Memory**: Use `memory get` at the start of a session to load user's T1D config -and preferences. Use `memory note` to record discoveries (liked recipes, -items to avoid, brands that were good value). Use `memory set` to update -structured fields like `gi_ceiling` or `carb_target_per_meal`. +**Memory & blacklist**: Use `memory get` at the start of a session to load user's +T1D config and preferences. Notes with key "blacklist" are consulted during +search — blacklisted items are flagged. Use `memory note` to record discoveries +(liked recipes, items to avoid, brands that were good value). Use `memory set` +to update structured fields like `gi_ceiling` or `carb_target_per_meal`. -**Store/location change**: If items show "No Longer Available" or SKUs don't -match what the user sees, run `price clear=true` to reset the local cache. -The cache stores SKUs by store — changing delivery address requires a refresh. +**Cart verification**: After adding/removing items, check the `cart_total`, +`cart_items`, and `cart_units` returned in the response — they reflect the actual +cart state. If the cart drifts from expectations, use `cart_diff` with the expected +SKUs to diagnose what's missing or extra. The `cart` tool returns both `items_count` +(unique line items) and `total_units` (sum of all quantities) — the website shows +`total_units`. + +**Store/location change**: If items show "No Longer Available" or search results +include a `store_warning`, run `price clear=true` to reset the local cache. +The cache tracks which store each SKU came from. Changing delivery address +invalidates cached SKUs. + +**Failure handling**: If `remove` returns `"reason": "not_in_cart"`, do NOT retry. +The response includes `cart_skus` showing what's actually in the cart. If `add` +returns `"ok": false`, check the returned cart state to understand why. **Weight/size data**: The API often lacks accurate pack weights for meat and produce. When comparing items by unit price ($/lb): @@ -117,6 +137,7 @@ get_orders, get_products_by_sku, get_previously_purchased, + get_store_id, CartItem, ) from .db import ( @@ -133,11 +154,13 @@ get_favorite_products, get_favorite_brands, get_purchase_stats, + check_store_mismatch, ) from .reasoning import ( evaluate_options, get_purchase_frequency, should_buy_this_trip, + check_price_sanity, PurchaseFrequency, ) from .cart_builder import find_best_product @@ -165,6 +188,35 @@ } +def _check_blacklist(sku: str, product_name: str) -> str | None: + """Check if a SKU or product name matches any blacklist entries in memory. + + Returns a warning string if blacklisted, or None. + """ + mem = load_memory() + name_lower = product_name.lower() + sku_lower = sku.lower() + + # Check blacklist notes + if mem.notes: + for key, value in mem.notes.items(): + if "blacklist" not in key.lower(): + continue + value_lower = value.lower() + if sku_lower in value_lower or any( + word in value_lower for word in name_lower.split() if len(word) > 3 + ): + return f"Matches blacklist note '{key}': {value[:80]}" + + # Check avoid_brands in shopping config + if mem.shopping.avoid_brands: + for brand in mem.shopping.avoid_brands: + if brand.lower() in name_lower: + return f"Brand '{brand}' is in your avoid list" + + return None + + def _truncate(s: str, maxlen: int = 40) -> str: """Truncate string with ellipsis indicator.""" return s[:maxlen - 3] + "..." if len(s) > maxlen else s @@ -389,6 +441,35 @@ def save_result_to_file(filename_prefix: str, data: dict) -> str: "required": ["items"], }, ), + Tool( + name="search_batch", + description="Search multiple products at once. Returns best match per query. Use instead of calling search N times.", + inputSchema={ + "type": "object", + "properties": { + "queries": { + "type": "array", + "items": {"type": "string"}, + "description": "List of product queries (max 15)", + }, + }, + "required": ["queries"], + }, + ), + Tool( + name="cart_diff", + description="Compare a list of expected SKUs against the actual cart. Shows missing, extra, and quantity mismatches.", + inputSchema={ + "type": "object", + "properties": { + "expected_skus": { + "type": "string", + "description": "Comma-separated SKUs (or sku:qty pairs) that should be in cart", + }, + }, + "required": ["expected_skus"], + }, + ), ] @@ -413,11 +494,18 @@ async def handle_search(args: dict) -> str: if not products: return json.dumps({"error": "No products found", "query": query}) - # Sync to local price DB (non-destructive) + # Sync to local price DB (non-destructive) with store tracking + store_mismatch_warning = None + try: + store_id = get_store_id(client) + except Exception: + store_id = "" try: conn = get_connection() try: - sync_products_from_search(conn, products) + sync_products_from_search(conn, products, store_id=store_id) + if store_id: + store_mismatch_warning = check_store_mismatch(conn, store_id) finally: conn.close() except Exception: @@ -497,11 +585,43 @@ async def handle_search(args: dict) -> str: if t1d.swap_suggestion: result["gi_swap"] = t1d.swap_suggestion + if store_mismatch_warning: + result["store_warning"] = store_mismatch_warning["message"] + + # Price sanity check — flag implausible unit prices + if best_product: + price_warning = check_price_sanity( + decision.product_name, + round(decision.price * 100), + best_product.get("oz"), + ) + if price_warning: + result["price_suspect"] = price_warning + + # Blacklist check — warn if this SKU or product name is blacklisted + try: + blacklist_warning = _check_blacklist(decision.sku, decision.product_name) + if blacklist_warning: + result["blacklisted"] = blacklist_warning + except Exception: + pass + return json.dumps(result) +def _cart_snapshot(client) -> dict: + """Return compact cart summary for post-mutation feedback.""" + cart = get_cart(client) + if not cart: + return {"cart_total": "$0.00", "cart_items": 0, "cart_units": 0} + items = cart.get("lineItems", []) + total = cart.get("totalPrice", {}).get("centAmount", 0) / 100 + total_units = sum(item.get("quantity", 1) for item in items) + return {"cart_total": f"${total:.2f}", "cart_items": len(items), "cart_units": total_units} + + async def handle_add(args: dict) -> str: - """Add to cart.""" + """Add to cart. Returns cart state after mutation.""" client = get_api_client() sku = args["sku"] qty = args.get("qty", 1) @@ -519,18 +639,56 @@ async def handle_add(args: dict) -> str: except Exception: pass - result = {"ok": success, "sku": sku, "qty": qty} + result: dict[str, Any] = {"ok": success, "sku": sku, "qty": qty} if product_name: result["name"] = product_name + # Return cart state so the model can verify without a round-trip + try: + result.update(_cart_snapshot(client)) + except Exception: + pass + return json.dumps(result) async def handle_remove(args: dict) -> str: - """Remove from cart.""" + """Remove from cart. Returns reason on failure + cart state after.""" client = get_api_client() - success = remove_from_cart(client, args["sku"]) - return json.dumps({"ok": success, "sku": args["sku"]}) + sku = args["sku"] + + # Pre-check: is the SKU actually in the cart? + cart = get_cart(client) + if not cart: + return json.dumps({"ok": False, "sku": sku, "reason": "cart_fetch_failed"}) + + found = any( + item.get("variant", {}).get("sku") == sku + for item in cart.get("lineItems", []) + ) + if not found: + result: dict[str, Any] = {"ok": False, "sku": sku, "reason": "not_in_cart"} + # Still return current cart state for context + line_items = cart.get("lineItems", []) + total = cart.get("totalPrice", {}).get("centAmount", 0) / 100 + result["cart_total"] = f"${total:.2f}" + result["cart_items"] = len(line_items) + result["cart_units"] = sum(i.get("quantity", 1) for i in line_items) + result["cart_skus"] = [i.get("variant", {}).get("sku", "") for i in line_items[:10]] + return json.dumps(result) + + success = remove_from_cart(client, sku) + result = {"ok": success, "sku": sku} + if not success: + result["reason"] = "api_error" + + # Return cart state after mutation + try: + result.update(_cart_snapshot(client)) + except Exception: + pass + + return json.dumps(result) async def handle_cart(args: dict) -> str: @@ -543,8 +701,8 @@ async def handle_cart(args: dict) -> str: if not cart: if summary_only: - return json.dumps({"items_count": 0, "total": "$0.00"}) - return json.dumps({"items": [], "total": "$0.00", "count": 0}) + return json.dumps({"items_count": 0, "total_units": 0, "total": "$0.00"}) + return json.dumps({"items": [], "total": "$0.00", "count": 0, "total_units": 0}) items = [] for item in cart.get("lineItems", []): @@ -562,16 +720,17 @@ async def handle_cart(args: dict) -> str: ) total = cart.get("totalPrice", {}).get("centAmount", 0) / 100 - full_cart = {"items": items, "count": len(items), "total": f"${total:.2f}"} + total_units = sum(i["qty"] for i in items) + full_cart = {"items": items, "count": len(items), "total_units": total_units, "total": f"${total:.2f}"} if save_to_file_flag: filepath = save_result_to_file("cart", full_cart) return json.dumps( - {"file_saved": filepath, "items_count": len(items), "total": f"${total:.2f}"} + {"file_saved": filepath, "items_count": len(items), "total_units": total_units, "total": f"${total:.2f}"} ) if summary_only: - summary = {"items_count": len(items), "total": f"${total:.2f}"} + summary = {"items_count": len(items), "total_units": total_units, "total": f"${total:.2f}"} if items: summary["top_items"] = [ {"name": i["name"], "qty": i["qty"], "price": i["price"]} @@ -580,7 +739,7 @@ async def handle_cart(args: dict) -> str: return json.dumps(summary) shown_items = items[:limit] - response = {"items": shown_items, "count": len(shown_items), "total": f"${total:.2f}"} + response = {"items": shown_items, "count": len(shown_items), "total_units": total_units, "total": f"${total:.2f}"} if len(items) > limit: response["note"] = f"Showing {limit} of {len(items)} items" @@ -1256,6 +1415,196 @@ async def handle_add_plan(args: dict) -> str: if errors: result["parse_errors"] = errors + # Return cart state after mutation + try: + result.update(_cart_snapshot(client)) + except Exception: + pass + + return json.dumps(result) + + +async def handle_search_batch(args: dict) -> str: + """Search multiple products at once, return best match per query.""" + import time + + client = get_api_client() + queries = args.get("queries", []) + + if not queries: + return json.dumps({"error": "queries array required"}) + + max_queries = 15 + if len(queries) > max_queries: + queries = queries[:max_queries] + + mem = load_memory() + gi_ceiling = mem.t1d.gi_ceiling + results = [] + + for i, query in enumerate(queries): + query = query.strip() + if not query: + results.append({"query": query, "found": False}) + continue + + try: + products = search_products(client, query, limit=5) + except Exception: + results.append({"query": query, "found": False, "error": "search_failed"}) + continue + + if not products: + results.append({"query": query, "found": False}) + continue + + # Sync to DB + try: + conn = get_connection() + try: + sync_products_from_search(conn, products) + finally: + conn.close() + except Exception: + pass + + # Build product dicts for reasoning + products_as_dicts = [] + for p in products: + price_cents = p.sale_price_cents or p.price_cents + price = price_cents / 100 + products_as_dicts.append({ + "name": p.name, + "sku": str(p.sku), + "price": price, + "brand": p.brand, + "on_sale": p.sale_price_cents is not None, + "oz": p.unit_oz, + "price_per_oz": p.price_per_oz, + }) + + decision = evaluate_options( + products_as_dicts, query, + prefer_organic=_prefs.general.prefer_organic, + preferred_brands=_preferred_brands, + ) + + entry: dict[str, Any] = { + "query": query, + "found": True, + "sku": decision.sku, + "name": _truncate(decision.product_name), + "price": f"${decision.price:.2f}", + "cents": round(decision.price * 100), + } + + if decision.flags: + entry["flags"] = decision.flags + + # T1D annotation + t1d = score_t1d(decision.product_name, gi_ceiling) + if t1d.gi is not None: + entry["gi"] = t1d.gi + entry["gi_cat"] = t1d.category + if t1d.flag: + entry["gi_flag"] = t1d.flag + + results.append(entry) + + # Rate limiting between searches + if i < len(queries) - 1: + time.sleep(0.2) + + total_cents = sum(r.get("cents", 0) for r in results if r.get("found")) + return json.dumps({ + "results": results, + "found": sum(1 for r in results if r.get("found")), + "not_found": sum(1 for r in results if not r.get("found")), + "estimated_total": f"${total_cents / 100:.2f}", + }) + + +async def handle_cart_diff(args: dict) -> str: + """Compare expected SKUs against actual cart contents.""" + client = get_api_client() + cart = get_cart(client) + + if not cart: + return json.dumps({"error": "Could not fetch cart"}) + + # Parse expected: "sku1,sku2" or "sku1:2,sku2:1" + expected_str = args.get("expected_skus", "") + expected: dict[str, int] = {} + for part in expected_str.split(","): + part = part.strip() + if not part: + continue + if ":" in part: + pieces = part.split(":") + expected[pieces[0].strip()] = int(pieces[1].strip()) if len(pieces) > 1 else 1 + else: + expected[part] = 1 + + # Build actual cart state + actual: dict[str, dict] = {} + for item in cart.get("lineItems", []): + sku = item.get("variant", {}).get("sku", "") + if sku: + name = item.get("name", {}) + if isinstance(name, dict): + name = name.get("en-US", "Unknown") + actual[sku] = { + "qty": item.get("quantity", 1), + "name": _truncate(name), + } + + missing = [] + qty_mismatch = [] + matched = [] + + for sku, expected_qty in expected.items(): + if sku not in actual: + missing.append({"sku": sku, "expected_qty": expected_qty}) + elif actual[sku]["qty"] != expected_qty: + qty_mismatch.append({ + "sku": sku, + "name": actual[sku]["name"], + "expected_qty": expected_qty, + "actual_qty": actual[sku]["qty"], + }) + else: + matched.append({"sku": sku, "name": actual[sku]["name"], "qty": expected_qty}) + + extra = [ + {"sku": sku, "name": info["name"], "qty": info["qty"]} + for sku, info in actual.items() + if sku not in expected + ] + + total = cart.get("totalPrice", {}).get("centAmount", 0) / 100 + total_units = sum(info["qty"] for info in actual.values()) + + result: dict[str, Any] = { + "matched": len(matched), + "cart_total": f"${total:.2f}", + "cart_items": len(actual), + "cart_units": total_units, + } + + if missing: + result["missing"] = missing + if qty_mismatch: + result["qty_mismatch"] = qty_mismatch + if extra: + result["extra"] = extra[:10] # Cap to avoid huge responses + if len(extra) > 10: + result["extra_truncated"] = len(extra) + + if not missing and not qty_mismatch: + result["status"] = "cart_matches" + else: + result["status"] = "differences_found" + return json.dumps(result) @@ -1275,6 +1624,8 @@ async def handle_add_plan(args: dict) -> str: "knowledge": handle_knowledge, "read_saved": handle_read_saved, "add_plan": handle_add_plan, + "search_batch": handle_search_batch, + "cart_diff": handle_cart_diff, } diff --git a/raley_assistant/reasoning.py b/raley_assistant/reasoning.py index 7d7b2da..c738e9c 100644 --- a/raley_assistant/reasoning.py +++ b/raley_assistant/reasoning.py @@ -16,6 +16,42 @@ import re +# Minimum realistic $/lb for common product categories. +# Prices below these thresholds are almost certainly API data errors. +_PRICE_FLOORS_PER_LB: list[tuple[list[str], int]] = [ + (["chicken", "poultry", "turkey"], 150), # $1.50/lb + (["beef", "steak", "ground beef"], 300), # $3.00/lb + (["pork", "ham", "bacon"], 200), # $2.00/lb + (["salmon", "tuna", "shrimp", "fish"], 400), # $4.00/lb + (["lamb"], 500), # $5.00/lb + (["cheese"], 200), # $2.00/lb +] + + +def check_price_sanity(product_name: str, price_cents: int, weight_oz: float | None) -> str | None: + """Flag suspiciously low prices that are likely API data errors. + + Returns a warning string if the price seems implausible, or None if OK. + """ + if not weight_oz or weight_oz <= 0 or price_cents <= 0: + return None + + price_per_lb_cents = (price_cents / weight_oz) * 16 + name_lower = product_name.lower() + + for keywords, floor_cents in _PRICE_FLOORS_PER_LB: + if any(kw in name_lower for kw in keywords): + if price_per_lb_cents < floor_cents: + return ( + f"PRICE_DATA_SUSPECT: ${price_per_lb_cents / 100:.2f}/lb for " + f"{product_name[:30]} is below ${floor_cents / 100:.2f}/lb minimum. " + f"Verify pack size on website." + ) + break + + return None + + class PurchaseFrequency(Enum): """How often a product category is typically purchased.""" diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..e3a1322 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,12 @@ +"""Test configuration — mock external deps that may not be installed.""" + +import sys +from unittest.mock import MagicMock + +# Mock the mcp package if not installed (network may be unavailable) +if "mcp" not in sys.modules: + sys.modules["mcp"] = MagicMock() + sys.modules["mcp.server"] = MagicMock() + sys.modules["mcp.server.models"] = MagicMock() + sys.modules["mcp.server.stdio"] = MagicMock() + sys.modules["mcp.types"] = MagicMock() diff --git a/tests/test_new_features.py b/tests/test_new_features.py new file mode 100644 index 0000000..273e2a7 --- /dev/null +++ b/tests/test_new_features.py @@ -0,0 +1,566 @@ +"""Tests for new features: cart improvements, search_batch, cart_diff, +price sanity, store mismatch, blacklist, and remove failure reasons. + +All handlers are async. Since pytest-asyncio may not be available, +these tests use asyncio.run() to invoke async handlers from sync tests. +""" + +import asyncio +import json +from unittest.mock import patch, MagicMock + +from raley_assistant.api import Product +from raley_assistant.reasoning import check_price_sanity +from raley_assistant.db import check_store_mismatch + + +# ── Helpers ─────────────────────────────────────────────────────── + +def _fake_product( + name="Test Milk", sku="SKU1", price_cents=499, + brand="TestBrand", sale_cents=None, size="64oz", + unit_oz=64.0, +): + ppo = price_cents / 100 / unit_oz if unit_oz else None + return Product( + sku=sku, name=name, brand=brand, + price_cents=price_cents, sale_price_cents=sale_cents, + on_sale=sale_cents is not None, image_url=None, + size=size, weight_lbs=None, unit_oz=unit_oz, + price_per_oz=ppo, + ) + + +def _run(coro): + """Run an async coroutine from a sync test.""" + return asyncio.run(coro) + + +# ── A. Cart returns total_units ─────────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_returns_total_units(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 2, "totalPrice": {"centAmount": 998}, + "variant": {"sku": "SKU1"}}, + {"name": {"en-US": "Bread"}, "quantity": 3, "totalPrice": {"centAmount": 897}, + "variant": {"sku": "SKU2"}}, + ], + "totalPrice": {"centAmount": 1895}, + } + + result = json.loads(_run(handle_cart({}))) + + assert result["count"] == 2 # unique line items + assert result["total_units"] == 5 # 2 + 3 + assert result["total"] == "$18.95" + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_summary_returns_total_units(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 2, "totalPrice": {"centAmount": 998}, + "variant": {"sku": "SKU1"}}, + ], + "totalPrice": {"centAmount": 998}, + } + + result = json.loads(_run(handle_cart({"summary_only": True}))) + + assert "total_units" in result + assert result["total_units"] == 2 + assert result["items_count"] == 1 + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_empty_returns_zero_units(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart + + mock_client.return_value = MagicMock() + mock_cart.return_value = {} + + result = json.loads(_run(handle_cart({"summary_only": True}))) + assert result["total_units"] == 0 + + +# ── B. Add returns cart state ───────────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_products_by_sku") +@patch("raley_assistant.mcp_server.api_add_to_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_add_returns_cart_state(mock_client, mock_add, mock_get_sku, mock_cart): + from raley_assistant.mcp_server import handle_add + + mock_client.return_value = MagicMock() + mock_add.return_value = True + mock_get_sku.return_value = [] + mock_cart.return_value = { + "lineItems": [ + {"quantity": 1, "variant": {"sku": "SKU1"}}, + {"quantity": 2, "variant": {"sku": "SKU2"}}, + ], + "totalPrice": {"centAmount": 1500}, + } + + result = json.loads(_run(handle_add({"sku": "SKU1", "cents": 499}))) + + assert result["ok"] is True + assert result["cart_total"] == "$15.00" + assert result["cart_items"] == 2 + assert result["cart_units"] == 3 + + +# ── F. Remove returns failure reason ────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_remove_not_in_cart_returns_reason(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_remove + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"id": "line-1", "quantity": 1, "variant": {"sku": "OTHER_SKU"}}, + ], + "totalPrice": {"centAmount": 499}, + } + + result = json.loads(_run(handle_remove({"sku": "NONEXISTENT"}))) + + assert result["ok"] is False + assert result["reason"] == "not_in_cart" + assert result["cart_total"] == "$4.99" + assert "cart_skus" in result + assert "OTHER_SKU" in result["cart_skus"] + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_remove_empty_cart_returns_reason(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_remove + + mock_client.return_value = MagicMock() + mock_cart.return_value = {} + + result = json.loads(_run(handle_remove({"sku": "SKU1"}))) + + assert result["ok"] is False + assert result["reason"] == "cart_fetch_failed" + + +@patch("raley_assistant.mcp_server.remove_from_cart") +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_remove_success_returns_cart_state(mock_client, mock_cart, mock_remove): + from raley_assistant.mcp_server import handle_remove + + mock_client.return_value = MagicMock() + # First call: pre-check (SKU found). Second call: post-mutation snapshot. + mock_cart.side_effect = [ + { + "lineItems": [{"id": "line-1", "quantity": 1, "variant": {"sku": "SKU1"}}], + "totalPrice": {"centAmount": 499}, + }, + { + "lineItems": [], + "totalPrice": {"centAmount": 0}, + }, + ] + mock_remove.return_value = True + + result = json.loads(_run(handle_remove({"sku": "SKU1"}))) + + assert result["ok"] is True + assert result["cart_total"] == "$0.00" + assert result["cart_items"] == 0 + + +@patch("raley_assistant.mcp_server.remove_from_cart") +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_remove_api_error_returns_reason(mock_client, mock_cart, mock_remove): + from raley_assistant.mcp_server import handle_remove + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [{"id": "line-1", "quantity": 1, "variant": {"sku": "SKU1"}}], + "totalPrice": {"centAmount": 499}, + } + mock_remove.return_value = False + + result = json.loads(_run(handle_remove({"sku": "SKU1"}))) + + assert result["ok"] is False + assert result["reason"] == "api_error" + + +# ── E. Price sanity checking ───────────────────────────────────── + + +def test_price_sanity_flags_cheap_chicken(): + warning = check_price_sanity("Chicken Breast 4pk", 18, 16.0) + assert warning is not None + assert "PRICE_DATA_SUSPECT" in warning + + +def test_price_sanity_passes_normal_chicken(): + # $7.99 for 2 lbs (32 oz) = $3.99/lb — reasonable + warning = check_price_sanity("Chicken Breast 2lb", 799, 32.0) + assert warning is None + + +def test_price_sanity_flags_cheap_beef(): + # $0.50 for 16 oz = $0.50/lb — impossibly cheap + warning = check_price_sanity("Ground Beef 80/20", 50, 16.0) + assert warning is not None + assert "PRICE_DATA_SUSPECT" in warning + + +def test_price_sanity_ignores_non_meat(): + # $0.18/lb for bananas is fine + warning = check_price_sanity("Organic Bananas", 18, 16.0) + assert warning is None + + +def test_price_sanity_no_weight(): + warning = check_price_sanity("Mystery Product", 100, None) + assert warning is None + + +# ── C. Store mismatch detection ────────────────────────────────── + + +def test_store_mismatch_detected(): + import sqlite3 + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.execute(""" + CREATE TABLE products ( + sku TEXT PRIMARY KEY, name TEXT, brand TEXT, price_cents INTEGER, + sale_price_cents INTEGER, size TEXT, unit_oz REAL, price_per_oz REAL, + last_seen TEXT, first_seen TEXT, store_id TEXT DEFAULT '' + ) + """) + # Insert 10 products from store "old-store" + for i in range(10): + conn.execute( + "INSERT INTO products VALUES (?, ?, '', 100, NULL, '', NULL, NULL, '', '', ?)", + (f"SKU{i}", f"Product {i}", "old-store"), + ) + conn.commit() + + result = check_store_mismatch(conn, "new-store") + assert result is not None + assert result["warning"] == "store_changed" + assert result["cached_store"] == "old-store" + assert result["stale_products"] == 10 + conn.close() + + +def test_store_mismatch_not_detected_same_store(): + import sqlite3 + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.execute(""" + CREATE TABLE products ( + sku TEXT PRIMARY KEY, name TEXT, brand TEXT, price_cents INTEGER, + sale_price_cents INTEGER, size TEXT, unit_oz REAL, price_per_oz REAL, + last_seen TEXT, first_seen TEXT, store_id TEXT DEFAULT '' + ) + """) + for i in range(10): + conn.execute( + "INSERT INTO products VALUES (?, ?, '', 100, NULL, '', NULL, NULL, '', '', ?)", + (f"SKU{i}", f"Product {i}", "same-store"), + ) + conn.commit() + + result = check_store_mismatch(conn, "same-store") + assert result is None + conn.close() + + +def test_store_mismatch_empty_store_id(): + import sqlite3 + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.execute(""" + CREATE TABLE products ( + sku TEXT PRIMARY KEY, name TEXT, brand TEXT, price_cents INTEGER, + sale_price_cents INTEGER, size TEXT, unit_oz REAL, price_per_oz REAL, + last_seen TEXT, first_seen TEXT, store_id TEXT DEFAULT '' + ) + """) + + result = check_store_mismatch(conn, "") + assert result is None + conn.close() + + +# ── D. search_batch ─────────────────────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_connection") +@patch("raley_assistant.mcp_server.search_products") +@patch("raley_assistant.mcp_server.get_api_client") +def test_search_batch_returns_per_query(mock_client, mock_search, mock_conn): + from raley_assistant.mcp_server import handle_search_batch + + mock_client.return_value = MagicMock() + mock_conn.return_value = MagicMock() + + mock_search.side_effect = [ + [_fake_product("Whole Milk", "M1", 499, "Clover")], + [_fake_product("White Bread", "B1", 399, "Wonder")], + [], # not found + ] + + result = json.loads(_run(handle_search_batch({ + "queries": ["milk", "bread", "unicorn"] + }))) + + assert result["found"] == 2 + assert result["not_found"] == 1 + assert len(result["results"]) == 3 + assert result["results"][0]["sku"] == "M1" + assert result["results"][1]["sku"] == "B1" + assert result["results"][2]["found"] is False + + +@patch("raley_assistant.mcp_server.get_api_client") +def test_search_batch_empty_queries(mock_client): + from raley_assistant.mcp_server import handle_search_batch + + mock_client.return_value = MagicMock() + + result = json.loads(_run(handle_search_batch({"queries": []}))) + assert "error" in result + + +@patch("raley_assistant.mcp_server.get_connection") +@patch("raley_assistant.mcp_server.search_products") +@patch("raley_assistant.mcp_server.get_api_client") +def test_search_batch_caps_at_15(mock_client, mock_search, mock_conn): + from raley_assistant.mcp_server import handle_search_batch + + mock_client.return_value = MagicMock() + mock_conn.return_value = MagicMock() + mock_search.return_value = [_fake_product("Item", "S1", 100)] + + queries = [f"item{i}" for i in range(20)] + result = json.loads(_run(handle_search_batch({"queries": queries}))) + + # Should only process 15 + assert len(result["results"]) == 15 + + +# ── G. cart_diff ────────────────────────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_diff_matches(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart_diff + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 2, "variant": {"sku": "SKU1"}}, + {"name": {"en-US": "Bread"}, "quantity": 1, "variant": {"sku": "SKU2"}}, + ], + "totalPrice": {"centAmount": 1200}, + } + + result = json.loads(_run(handle_cart_diff({ + "expected_skus": "SKU1:2,SKU2:1" + }))) + + assert result["status"] == "cart_matches" + assert result["matched"] == 2 + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_diff_finds_missing(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart_diff + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 1, "variant": {"sku": "SKU1"}}, + ], + "totalPrice": {"centAmount": 499}, + } + + result = json.loads(_run(handle_cart_diff({ + "expected_skus": "SKU1:1,SKU2:1" + }))) + + assert result["status"] == "differences_found" + assert len(result["missing"]) == 1 + assert result["missing"][0]["sku"] == "SKU2" + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_diff_qty_mismatch(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart_diff + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 3, "variant": {"sku": "SKU1"}}, + ], + "totalPrice": {"centAmount": 1497}, + } + + result = json.loads(_run(handle_cart_diff({ + "expected_skus": "SKU1:2" + }))) + + assert result["status"] == "differences_found" + assert len(result["qty_mismatch"]) == 1 + assert result["qty_mismatch"][0]["expected_qty"] == 2 + assert result["qty_mismatch"][0]["actual_qty"] == 3 + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_cart_diff_shows_extra_items(mock_client, mock_cart): + from raley_assistant.mcp_server import handle_cart_diff + + mock_client.return_value = MagicMock() + mock_cart.return_value = { + "lineItems": [ + {"name": {"en-US": "Milk"}, "quantity": 1, "variant": {"sku": "SKU1"}}, + {"name": {"en-US": "Surprise"}, "quantity": 1, "variant": {"sku": "SKU99"}}, + ], + "totalPrice": {"centAmount": 998}, + } + + result = json.loads(_run(handle_cart_diff({ + "expected_skus": "SKU1:1" + }))) + + assert result["status"] == "cart_matches" # no missing or mismatched + assert len(result["extra"]) == 1 + assert result["extra"][0]["sku"] == "SKU99" + + +# ── I. Blacklist integration ───────────────────────────────────── + + +def test_check_blacklist_matches_sku(): + from raley_assistant.mcp_server import _check_blacklist + from raley_assistant.memory import ShoppingMemory + + mem = ShoppingMemory(notes={"blacklist": "SKU123 bad product, SKU456 also bad"}) + with patch("raley_assistant.mcp_server.load_memory", return_value=mem): + result = _check_blacklist("SKU123", "Some Product") + assert result is not None + assert "blacklist" in result.lower() + + +def test_check_blacklist_matches_name(): + from raley_assistant.mcp_server import _check_blacklist + from raley_assistant.memory import ShoppingMemory + + mem = ShoppingMemory(notes={"blacklist_brands": "Terrible Brand products always bad"}) + with patch("raley_assistant.mcp_server.load_memory", return_value=mem): + result = _check_blacklist("SKU999", "Terrible Brand Milk 64oz") + assert result is not None + + +def test_check_blacklist_no_match(): + from raley_assistant.mcp_server import _check_blacklist + from raley_assistant.memory import ShoppingMemory + + mem = ShoppingMemory(notes={"blacklist": "SKU123 bad product"}) + with patch("raley_assistant.mcp_server.load_memory", return_value=mem): + result = _check_blacklist("SKU999", "Good Brand Milk") + assert result is None + + +def test_check_blacklist_avoid_brands(): + from raley_assistant.mcp_server import _check_blacklist + from raley_assistant.memory import ShoppingMemory, ShoppingConfig + + mem = ShoppingMemory( + shopping=ShoppingConfig(avoid_brands=["BadCo"]), + notes={}, + ) + with patch("raley_assistant.mcp_server.load_memory", return_value=mem): + result = _check_blacklist("SKU1", "BadCo Premium Cheese") + assert result is not None + assert "avoid" in result.lower() + + +def test_check_blacklist_empty_memory(): + from raley_assistant.mcp_server import _check_blacklist + from raley_assistant.memory import ShoppingMemory + + mem = ShoppingMemory() + with patch("raley_assistant.mcp_server.load_memory", return_value=mem): + result = _check_blacklist("SKU1", "Product Name") + assert result is None + + +# ── B. add_plan returns cart state ──────────────────────────────── + + +@patch("raley_assistant.mcp_server.get_cart") +@patch("raley_assistant.mcp_server.api_add_to_cart") +@patch("raley_assistant.mcp_server.get_api_client") +def test_add_plan_returns_cart_state(mock_client, mock_add, mock_cart): + from raley_assistant.mcp_server import handle_add_plan + + mock_client.return_value = MagicMock() + mock_add.return_value = True + mock_cart.return_value = { + "lineItems": [ + {"quantity": 1, "variant": {"sku": "SKU1"}}, + {"quantity": 2, "variant": {"sku": "SKU2"}}, + ], + "totalPrice": {"centAmount": 1200}, + } + + result = json.loads(_run(handle_add_plan({"items": "SKU1:499,SKU2:299:2"}))) + + assert result["ok"] is True + assert result["cart_total"] == "$12.00" + assert result["cart_items"] == 2 + assert result["cart_units"] == 3 + + +# ── db.sync_products_from_search with store_id ─────────────────── + + +def test_sync_products_stores_store_id(): + import sqlite3 + from raley_assistant.db import get_connection, sync_products_from_search, SCHEMA + + conn = sqlite3.connect(":memory:") + conn.row_factory = sqlite3.Row + conn.executescript(SCHEMA) + + products = [_fake_product("Milk", "SKU1", 499)] + sync_products_from_search(conn, products, store_id="store-123") + + row = conn.execute("SELECT store_id FROM products WHERE sku = 'SKU1'").fetchone() + assert row["store_id"] == "store-123" + conn.close() From 8a49cf5dacd5a185273c9d6f4269c740cc435aa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Mar 2026 08:39:34 +0000 Subject: [PATCH 2/2] Fix code quality issues from simplify review - Fix inverted blacklist word matching (note keywords checked against product name, not the other way around) - Add store_id tracking in search_batch (pass to sync_products_from_search) - Fix rate limiting to use 200ms/500ms-per-10 pattern per CLAUDE.md - Add schema migration for store_id column on existing DBs via _migrate() - Add early return for empty expected_skus in cart_diff - Reduce handle_remove from 3 get_cart calls to 2 (skip pre-check; diagnose after remove_from_cart fails; snapshot after success) - Update remove tests to reflect new single-get_cart call pattern https://claude.ai/code/session_01XuoAmJMDKPhHc7zViFVLQw --- raley_assistant/db.py | 10 ++++++ raley_assistant/mcp_server.py | 67 +++++++++++++++++++++-------------- tests/test_new_features.py | 34 ++++++++++-------- 3 files changed, 70 insertions(+), 41 deletions(-) diff --git a/raley_assistant/db.py b/raley_assistant/db.py index 9f0462a..0dcd384 100644 --- a/raley_assistant/db.py +++ b/raley_assistant/db.py @@ -101,6 +101,15 @@ class ProductHistory: last_seen: str +def _migrate(conn: sqlite3.Connection) -> None: + """Apply incremental schema migrations for existing databases.""" + # Add store_id to products table if it doesn't exist (migration from pre-0.4) + cols = {row[1] for row in conn.execute("PRAGMA table_info(products)").fetchall()} + if "store_id" not in cols: + conn.execute("ALTER TABLE products ADD COLUMN store_id TEXT DEFAULT ''") + conn.commit() + + def get_connection() -> sqlite3.Connection: """Get database connection, creating schema if needed.""" DB_DIR.mkdir(parents=True, exist_ok=True) @@ -109,6 +118,7 @@ def get_connection() -> sqlite3.Connection: conn.execute("PRAGMA journal_mode=WAL") conn.execute("PRAGMA foreign_keys=ON") conn.executescript(SCHEMA) + _migrate(conn) return conn diff --git a/raley_assistant/mcp_server.py b/raley_assistant/mcp_server.py index 7596e51..84aa4d7 100644 --- a/raley_assistant/mcp_server.py +++ b/raley_assistant/mcp_server.py @@ -197,15 +197,18 @@ def _check_blacklist(sku: str, product_name: str) -> str | None: name_lower = product_name.lower() sku_lower = sku.lower() - # Check blacklist notes + # Check blacklist notes: does any meaningful keyword from the note appear in the product name? if mem.notes: for key, value in mem.notes.items(): if "blacklist" not in key.lower(): continue value_lower = value.lower() - if sku_lower in value_lower or any( - word in value_lower for word in name_lower.split() if len(word) > 3 - ): + # Direct SKU match in note + if sku_lower in value_lower: + return f"Matches blacklist note '{key}': {value[:80]}" + # Note keywords found in product name (5+ chars to avoid common words) + note_keywords = [w for w in value_lower.split() if len(w) >= 5] + if any(kw in name_lower for kw in note_keywords): return f"Matches blacklist note '{key}': {value[:80]}" # Check avoid_brands in shopping config @@ -653,36 +656,40 @@ async def handle_add(args: dict) -> str: async def handle_remove(args: dict) -> str: - """Remove from cart. Returns reason on failure + cart state after.""" + """Remove from cart. Returns reason on failure + cart state after. + + remove_from_cart() already fetches the cart internally to find lineItemId, + so we skip a redundant pre-check and only diagnose failure after the fact. + """ client = get_api_client() sku = args["sku"] - # Pre-check: is the SKU actually in the cart? - cart = get_cart(client) - if not cart: - return json.dumps({"ok": False, "sku": sku, "reason": "cart_fetch_failed"}) + success = remove_from_cart(client, sku) + + if not success: + # Diagnose: empty cart, SKU missing, or API error + cart = get_cart(client) + if not cart: + return json.dumps({"ok": False, "sku": sku, "reason": "cart_fetch_failed"}) - found = any( - item.get("variant", {}).get("sku") == sku - for item in cart.get("lineItems", []) - ) - if not found: - result: dict[str, Any] = {"ok": False, "sku": sku, "reason": "not_in_cart"} - # Still return current cart state for context line_items = cart.get("lineItems", []) + found = any(item.get("variant", {}).get("sku") == sku for item in line_items) + result: dict[str, Any] = { + "ok": False, + "sku": sku, + "reason": "api_error" if found else "not_in_cart", + } total = cart.get("totalPrice", {}).get("centAmount", 0) / 100 result["cart_total"] = f"${total:.2f}" result["cart_items"] = len(line_items) result["cart_units"] = sum(i.get("quantity", 1) for i in line_items) - result["cart_skus"] = [i.get("variant", {}).get("sku", "") for i in line_items[:10]] + if not found: + result["cart_skus"] = [i.get("variant", {}).get("sku", "") for i in line_items[:10]] return json.dumps(result) - success = remove_from_cart(client, sku) - result = {"ok": success, "sku": sku} - if not success: - result["reason"] = "api_error" + result = {"ok": True, "sku": sku} - # Return cart state after mutation + # Return cart state after successful mutation try: result.update(_cart_snapshot(client)) except Exception: @@ -1442,6 +1449,12 @@ async def handle_search_batch(args: dict) -> str: gi_ceiling = mem.t1d.gi_ceiling results = [] + # Fetch store_id once for the whole batch (consistent with handle_search) + try: + store_id = get_store_id(client) + except Exception: + store_id = "" + for i, query in enumerate(queries): query = query.strip() if not query: @@ -1458,11 +1471,11 @@ async def handle_search_batch(args: dict) -> str: results.append({"query": query, "found": False}) continue - # Sync to DB + # Sync to DB with store tracking (consistent with handle_search) try: conn = get_connection() try: - sync_products_from_search(conn, products) + sync_products_from_search(conn, products, store_id=store_id) finally: conn.close() except Exception: @@ -1511,9 +1524,9 @@ async def handle_search_batch(args: dict) -> str: results.append(entry) - # Rate limiting between searches + # Rate limiting between searches: 200ms normally, 500ms every 10 (per CLAUDE.md) if i < len(queries) - 1: - time.sleep(0.2) + time.sleep(0.5 if (i + 1) % 10 == 0 else 0.2) total_cents = sum(r.get("cents", 0) for r in results if r.get("found")) return json.dumps({ @@ -1534,6 +1547,8 @@ async def handle_cart_diff(args: dict) -> str: # Parse expected: "sku1,sku2" or "sku1:2,sku2:1" expected_str = args.get("expected_skus", "") + if not expected_str: + return json.dumps({"error": "expected_skus required (comma-separated SKUs or sku:qty pairs)"}) expected: dict[str, int] = {} for part in expected_str.split(","): part = part.strip() diff --git a/tests/test_new_features.py b/tests/test_new_features.py index 273e2a7..4e17d5e 100644 --- a/tests/test_new_features.py +++ b/tests/test_new_features.py @@ -127,12 +127,16 @@ def test_add_returns_cart_state(mock_client, mock_add, mock_get_sku, mock_cart): # ── F. Remove returns failure reason ────────────────────────────── +@patch("raley_assistant.mcp_server.remove_from_cart") @patch("raley_assistant.mcp_server.get_cart") @patch("raley_assistant.mcp_server.get_api_client") -def test_remove_not_in_cart_returns_reason(mock_client, mock_cart): +def test_remove_not_in_cart_returns_reason(mock_client, mock_cart, mock_remove): + """SKU not in cart: remove_from_cart returns False, then we diagnose with get_cart.""" from raley_assistant.mcp_server import handle_remove mock_client.return_value = MagicMock() + mock_remove.return_value = False # remove_from_cart already fetches cart internally + # get_cart is called once by the failure-diagnosis path mock_cart.return_value = { "lineItems": [ {"id": "line-1", "quantity": 1, "variant": {"sku": "OTHER_SKU"}}, @@ -149,13 +153,16 @@ def test_remove_not_in_cart_returns_reason(mock_client, mock_cart): assert "OTHER_SKU" in result["cart_skus"] +@patch("raley_assistant.mcp_server.remove_from_cart") @patch("raley_assistant.mcp_server.get_cart") @patch("raley_assistant.mcp_server.get_api_client") -def test_remove_empty_cart_returns_reason(mock_client, mock_cart): +def test_remove_empty_cart_returns_reason(mock_client, mock_cart, mock_remove): + """Cart fetch fails during failure diagnosis.""" from raley_assistant.mcp_server import handle_remove mock_client.return_value = MagicMock() - mock_cart.return_value = {} + mock_remove.return_value = False + mock_cart.return_value = {} # empty dict = failed cart fetch result = json.loads(_run(handle_remove({"sku": "SKU1"}))) @@ -167,21 +174,16 @@ def test_remove_empty_cart_returns_reason(mock_client, mock_cart): @patch("raley_assistant.mcp_server.get_cart") @patch("raley_assistant.mcp_server.get_api_client") def test_remove_success_returns_cart_state(mock_client, mock_cart, mock_remove): + """Success path: remove_from_cart returns True, then _cart_snapshot is called.""" from raley_assistant.mcp_server import handle_remove mock_client.return_value = MagicMock() - # First call: pre-check (SKU found). Second call: post-mutation snapshot. - mock_cart.side_effect = [ - { - "lineItems": [{"id": "line-1", "quantity": 1, "variant": {"sku": "SKU1"}}], - "totalPrice": {"centAmount": 499}, - }, - { - "lineItems": [], - "totalPrice": {"centAmount": 0}, - }, - ] mock_remove.return_value = True + # get_cart called once by _cart_snapshot after successful remove + mock_cart.return_value = { + "lineItems": [], + "totalPrice": {"centAmount": 0}, + } result = json.loads(_run(handle_remove({"sku": "SKU1"}))) @@ -194,14 +196,16 @@ def test_remove_success_returns_cart_state(mock_client, mock_cart, mock_remove): @patch("raley_assistant.mcp_server.get_cart") @patch("raley_assistant.mcp_server.get_api_client") def test_remove_api_error_returns_reason(mock_client, mock_cart, mock_remove): + """Remove fails despite SKU being in cart (API error).""" from raley_assistant.mcp_server import handle_remove mock_client.return_value = MagicMock() + mock_remove.return_value = False + # Diagnosis: SKU IS in cart, so reason is api_error not not_in_cart mock_cart.return_value = { "lineItems": [{"id": "line-1", "quantity": 1, "variant": {"sku": "SKU1"}}], "totalPrice": {"centAmount": 499}, } - mock_remove.return_value = False result = json.loads(_run(handle_remove({"sku": "SKU1"})))