From 57383f32cde98d6ab0ddff07812d3eb7ed7db060 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 19:27:57 +0100 Subject: [PATCH 01/11] chore(prd-141): reorder US-011 ahead of lifts + fix US-009/US-012 ACs - US-011 (byte-equality snapshot tests) moved ahead of US-005 so the equivalence net guards every lift (US-005-010) in real time instead of landing after the US-010 hard-cut delete. - US-012 grep gate: drop PCRE (?!...) lookahead (incompatible with grep -E; macOS has no grep -P) for a portable two-pass POSIX ERE check. - US-009: Ralph validates the migration offline only (alembic --sql + py_compile + reviewer agent); staging apply + rowcount deferred to human. --- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 5 +++-- scripts/ralph/prd-141-widget-vertical.json | 6 +++--- scripts/ralph/prd.json | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index 1aa1b8384..cbf5de1c2 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -16,13 +16,13 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc ## Phase 1 — Lift Shopify into the plugin (the risky one) - [x] US-004 — Capture proactive opener + cart-idle fixtures *(synthetic-but-realistic acceptable — see PROMPT_build_prd141.md "Special note for US-004"; human reviews after Ralph generates)* +- [ ] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* - [ ] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder - [ ] US-006 — Move _resolve_graph_related_products into Shopify plugin - [ ] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [ ] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin -- [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces +- [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* - [ ] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions -- [ ] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener - [ ] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces ## Phase 2 — SDK sends page_context on regular messages *(automatos-widget-sdk repo)* @@ -49,6 +49,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - Phase 0 stories (US-001/002/003) are safe to run autonomously - US-004 now executable with synthetic fixtures (see PROMPT_build_prd141.md "Special note for US-004"). Human reviews fixtures after this story before US-005 starts. +- US-011 (snapshot tests) is REORDERED ahead of US-005 so the byte-equality net exists before any lift. It runs against the US-003 shim and MUST pass before the lifts start; every lift story (US-005–US-010) must keep `cd orchestrator && python -m pytest integrations/` green or it broke equivalence. - US-013/014/015 are in a different repo (automatos-widget-sdk); Ralph should mark BLOCKED-CROSS-REPO and exit (human will set up a separate Ralph worktree for SDK work) - US-016/017 are in automatos-skills; same — BLOCKED-CROSS-REPO and exit - US-020 is operational; Ralph should mark SKIPPED-HUMAN and exit diff --git a/scripts/ralph/prd-141-widget-vertical.json b/scripts/ralph/prd-141-widget-vertical.json index af386121e..2edfeeb38 100644 --- a/scripts/ralph/prd-141-widget-vertical.json +++ b/scripts/ralph/prd-141-widget-vertical.json @@ -150,7 +150,7 @@ "Migration is idempotent — running twice doesn't change rowcount on second run", "Migration is reviewed by the migration-reviewer agent (no locking risk, no NOT NULL on big table — this is a JSONB update on a small table)", "Log/comment in the migration: 'PRD-141 Phase 1 — backfill workspace.settings.vertical for all existing Shopify workspaces. Verify count matches expected Shopify workspace count after upgrade.'", - "Migration applied successfully on staging; rowcount logged" + "Ralph validates the migration OFFLINE only and MUST NOT connect to or mutate any database: run 'alembic upgrade --sql' to render the SQL without error, and py_compile the migration file. The actual staging apply + rowcount verification is DEFERRED to the human before merge — note this deferral in the commit body" ], "priority": 9, "passes": false, @@ -192,7 +192,7 @@ ], "priority": 11, "passes": false, - "notes": "BLOCKED by US-004. The tests MUST use real fixtures, not synthetic mocks — synthetic data won't catch real-world format bugs. If a snapshot test fails after US-005/006/007/008 lifts, the lift broke equivalence: fix the lift, do not regenerate the fixture. Fixtures are the source of truth for this refactor." + "notes": "REORDERED to run BEFORE US-005 (see IMPLEMENTATION_PLAN_prd141.md) so the byte-equality net guards every lift in real time. US-004 fixtures ARE present (synthetic-but-realistic per PROMPT_build_prd141.md). At this point the Shopify plugin is still the US-003 shim delegating to chat.py inline functions, so these tests exercise the real pre-move logic through handle_widget_message and MUST PASS now — then stay green through US-005/006/007/008/010. If one fails after a lift, the lift broke equivalence: fix the lift, do NOT regenerate the golden files. Use the committed fixtures; do NOT mock the graph snapshot." }, { "id": "US-012", @@ -200,7 +200,7 @@ "description": "As the platform, I need a CI check that fails any PR introducing Shopify-specific identifiers into generic widget/context/RAG/graph code paths.", "acceptanceCriteria": [ "Add a new shell script at scripts/ci/check-no-shopify-in-generic.sh", - "Script runs: grep -rE 'productHandle|productTitle|cartItems|cartItemCount|cartTotalPrice|shopify_(?!plugin|integration|sync)|onlineStoreUrl' orchestrator/api/widgets/ orchestrator/modules/context/ orchestrator/modules/knowledge/graph_service.py orchestrator/consumers/chatbot/ — MUST return non-zero exit code (no matches) for PR to pass", + "Script checks generic surfaces for forbidden Shopify identifiers using POSIX ERE only — NO PCRE lookahead (grep -E has no (?!...) and macOS has no grep -P). Two passes over paths orchestrator/api/widgets/ orchestrator/modules/context/ orchestrator/modules/knowledge/graph_service.py orchestrator/consumers/chatbot/ : pass A = grep -rnE 'productHandle|productTitle|cartItems|cartItemCount|cartTotalPrice|onlineStoreUrl' ; pass B = grep -rnE 'shopify_' piped through grep -vE 'shopify_(plugin|integration|sync)' to allow those three legitimate forms. If EITHER pass yields any line the script exits non-zero and FAILS the PR; zero hits across both = pass", "Exclude orchestrator/integrations/ from the grep (that's where vertical keys are legitimately referenced)", "Exclude orchestrator/api/shopify.py (catalog sync — outside this PRD's scope)", "Exclude graph_extraction.py's map_shopify_catalog (also outside this PRD's scope)", diff --git a/scripts/ralph/prd.json b/scripts/ralph/prd.json index af386121e..2edfeeb38 100644 --- a/scripts/ralph/prd.json +++ b/scripts/ralph/prd.json @@ -150,7 +150,7 @@ "Migration is idempotent — running twice doesn't change rowcount on second run", "Migration is reviewed by the migration-reviewer agent (no locking risk, no NOT NULL on big table — this is a JSONB update on a small table)", "Log/comment in the migration: 'PRD-141 Phase 1 — backfill workspace.settings.vertical for all existing Shopify workspaces. Verify count matches expected Shopify workspace count after upgrade.'", - "Migration applied successfully on staging; rowcount logged" + "Ralph validates the migration OFFLINE only and MUST NOT connect to or mutate any database: run 'alembic upgrade --sql' to render the SQL without error, and py_compile the migration file. The actual staging apply + rowcount verification is DEFERRED to the human before merge — note this deferral in the commit body" ], "priority": 9, "passes": false, @@ -192,7 +192,7 @@ ], "priority": 11, "passes": false, - "notes": "BLOCKED by US-004. The tests MUST use real fixtures, not synthetic mocks — synthetic data won't catch real-world format bugs. If a snapshot test fails after US-005/006/007/008 lifts, the lift broke equivalence: fix the lift, do not regenerate the fixture. Fixtures are the source of truth for this refactor." + "notes": "REORDERED to run BEFORE US-005 (see IMPLEMENTATION_PLAN_prd141.md) so the byte-equality net guards every lift in real time. US-004 fixtures ARE present (synthetic-but-realistic per PROMPT_build_prd141.md). At this point the Shopify plugin is still the US-003 shim delegating to chat.py inline functions, so these tests exercise the real pre-move logic through handle_widget_message and MUST PASS now — then stay green through US-005/006/007/008/010. If one fails after a lift, the lift broke equivalence: fix the lift, do NOT regenerate the golden files. Use the committed fixtures; do NOT mock the graph snapshot." }, { "id": "US-012", @@ -200,7 +200,7 @@ "description": "As the platform, I need a CI check that fails any PR introducing Shopify-specific identifiers into generic widget/context/RAG/graph code paths.", "acceptanceCriteria": [ "Add a new shell script at scripts/ci/check-no-shopify-in-generic.sh", - "Script runs: grep -rE 'productHandle|productTitle|cartItems|cartItemCount|cartTotalPrice|shopify_(?!plugin|integration|sync)|onlineStoreUrl' orchestrator/api/widgets/ orchestrator/modules/context/ orchestrator/modules/knowledge/graph_service.py orchestrator/consumers/chatbot/ — MUST return non-zero exit code (no matches) for PR to pass", + "Script checks generic surfaces for forbidden Shopify identifiers using POSIX ERE only — NO PCRE lookahead (grep -E has no (?!...) and macOS has no grep -P). Two passes over paths orchestrator/api/widgets/ orchestrator/modules/context/ orchestrator/modules/knowledge/graph_service.py orchestrator/consumers/chatbot/ : pass A = grep -rnE 'productHandle|productTitle|cartItems|cartItemCount|cartTotalPrice|onlineStoreUrl' ; pass B = grep -rnE 'shopify_' piped through grep -vE 'shopify_(plugin|integration|sync)' to allow those three legitimate forms. If EITHER pass yields any line the script exits non-zero and FAILS the PR; zero hits across both = pass", "Exclude orchestrator/integrations/ from the grep (that's where vertical keys are legitimately referenced)", "Exclude orchestrator/api/shopify.py (catalog sync — outside this PRD's scope)", "Exclude graph_extraction.py's map_shopify_catalog (also outside this PRD's scope)", From 90095d18a733d852a45396d7a0b0ea1e826bb92b Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 19:48:58 +0100 Subject: [PATCH 02/11] =?UTF-8?q?feat(prd-141):=20US-011=20=E2=80=94=20sna?= =?UTF-8?q?pshot=20equivalence=20tests=20for=20proactive=20openers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds byte-equality tests against the US-004 fixtures that run through the US-003 shim end-to-end. Conftest AST-extracts the four proactive helpers from real chat.py and injects them as a fake api.widgets.chat module plus a fixture-bound GraphifyService stub, so the tests exercise production source without paying the full FastAPI/RAG import cost. The synthetic INBUILD-flavoured graph exercises every branch (FBT walk, collection/vendor fallback, multi-seed cart aggregation, by_vendor- overrides-FBT NetworkX undirected quirk). Now-green snapshot net gates the US-005/006/007/008/010 lifts — must stay green through every move. Validation: pytest orchestrator/integrations/ → 32 passed; py_compile clean on the two changed Python files. `from main import app` is environment-blocked on a missing camelot native dep (unrelated to this PR; same blocker on baseline) so was not re-run as part of US-011. Story: scripts/ralph/prd.json US-011 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- .../integrations/shopify/tests/conftest.py | 227 ++++++++++++++++++ .../shopify/tests/test_widget_proactive.py | 108 +++++++++ scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 3 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 orchestrator/integrations/shopify/tests/conftest.py diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py new file mode 100644 index 000000000..637206a53 --- /dev/null +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -0,0 +1,227 @@ +"""Shared test fixtures for the Shopify widget plugin tests. + +PRD-141 US-011 — the snapshot equivalence tests need three things on top +of the per-test setup already in ``test_widget_proactive.py``: + +1. The US-004 fixture files (synthetic INBUILD graph, two page contexts, + two expected opener strings) loaded into Python objects. +2. A ``GraphifyService`` stub whose ``load_graph`` returns the fixture + graph regardless of workspace id — the lifted resolvers look it up + lazily so we have to inject it before the helpers run. +3. (Through Phase 1 only) a fake ``api.widgets.chat`` module exposing + the four proactive helpers the US-003 shim still imports. We + AST-extract them from real ``chat.py`` so the test exercises the + actual production source — no separate fixture copy that could + silently drift. + +As US-006/007/008 move the helpers into ``integrations/shopify/ +widget_proactive.py``, the ``api.widgets.chat`` injection in +``real_chat_with_graph`` should be tightened (or eventually removed) +in lockstep. The ``GraphifyService`` stub is needed in every phase. + +Synthetic-fixture caveat: the graph is hand-crafted to exercise the +same code branches the production INBUILD data exercises. The +byte-equality contract holds against this fixture; behavioural parity +against real INBUILD data is confirmed during the US-020 canary soak. +See ``fixtures/README.md`` for the full rationale. +""" + +from __future__ import annotations + +import ast +import json +import logging +import sys +import types +from pathlib import Path +from typing import Optional + +import networkx as nx +import pytest +from networkx.readwrite import json_graph + + +_THIS_DIR = Path(__file__).resolve().parent +_FIXTURES_DIR = _THIS_DIR / "fixtures" +# fixtures/ -> tests/ -> shopify/ -> integrations/ -> orchestrator +_ORCH_ROOT = _THIS_DIR.parents[2] +_CHAT_PY = _ORCH_ROOT / "api" / "widgets" / "chat.py" + + +# Names AST-extracted from chat.py. The two non-function entries +# (``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value``) are +# closed over by ``_build_proactive_opener_message``; without them the +# extracted function bodies raise ``NameError`` at call time. +_WANTED_NAMES = frozenset({ + "_OPENER_CONTEXT_FIELDS", + "_format_opener_context_value", + "_build_proactive_opener_message", + "_build_cart_idle_opener_message", + "_resolve_graph_related_products", + "_resolve_cart_recommendations", +}) + + +@pytest.fixture(scope="session") +def fixtures_dir() -> Path: + return _FIXTURES_DIR + + +@pytest.fixture(scope="session") +def inbuild_graph() -> nx.Graph: + """Rehydrate the US-004 synthetic INBUILD-flavoured graph. + + Stored as NetworkX ``node_link_data`` JSON for git-diff readability; + round-trips identically through ``json_graph.node_link_graph``. + """ + data = json.loads((_FIXTURES_DIR / "inbuild_graph_snapshot.json").read_text()) + return json_graph.node_link_graph(data, edges="edges") + + +@pytest.fixture(scope="session") +def product_page_context() -> dict: + return json.loads( + (_FIXTURES_DIR / "product_page_context.json").read_text() + ) + + +@pytest.fixture(scope="session") +def cart_idle_context() -> dict: + return json.loads( + (_FIXTURES_DIR / "cart_idle_context.json").read_text() + ) + + +@pytest.fixture(scope="session") +def expected_product_page_opener() -> str: + return (_FIXTURES_DIR / "expected_product_page_opener.txt").read_text() + + +@pytest.fixture(scope="session") +def expected_cart_idle_opener() -> str: + return (_FIXTURES_DIR / "expected_cart_idle_opener.txt").read_text() + + +def _extract_chat_helpers() -> dict: + """Pull the six proactive helpers out of chat.py via AST. + + Returns a namespace dict containing: + + * ``_OPENER_CONTEXT_FIELDS`` — the field-mapping tuple used by the + product-page directive builder. + * ``_format_opener_context_value`` — single-value formatter. + * ``_build_proactive_opener_message`` — product-page directive. + * ``_build_cart_idle_opener_message`` — cart-idle directive. + * ``_resolve_graph_related_products`` — single-seed FBT/collection/ + vendor walk. + * ``_resolve_cart_recommendations`` — multi-seed cart FBT walk. + + The resolvers do ``from modules.knowledge.graph_service import + GraphifyService`` lazily; the caller (``real_chat_with_graph``) + arranges that fake module before invoking them. + + Why AST instead of ``import``: chat.py is a FastAPI router that drags + in SQLAlchemy, Redis, RAG, multimodal, etc. Loading it just to read + four helpers is wasteful and brittle. AST extraction reads the + source verbatim and execs only the wanted nodes into an isolated + namespace, so the test exercises identical bytes to the running + server without paying the import cost. + """ + src = _CHAT_PY.read_text() + tree = ast.parse(src) + + ns: dict = { + "Optional": Optional, + "logger": logging.getLogger("us011_snapshot"), + "__name__": "_chat_extracted_for_us011", + } + + for node in tree.body: + name: Optional[str] = None + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + name = node.name + elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + name = node.target.id + elif ( + isinstance(node, ast.Assign) + and len(node.targets) == 1 + and isinstance(node.targets[0], ast.Name) + ): + name = node.targets[0].id + + if name in _WANTED_NAMES: + module = ast.Module(body=[node], type_ignores=[]) + code = compile(module, str(_CHAT_PY), "exec") + exec(code, ns) + + missing = _WANTED_NAMES - set(ns) + if missing: + raise RuntimeError( + "chat.py is missing expected proactive helpers " + f"{sorted(missing)}. If a US-005/006/007/008 lift moved them, " + "update conftest.py to point the AST extractor at the new " + "location (likely integrations/shopify/widget_proactive.py)." + ) + return ns + + +@pytest.fixture +def real_chat_with_graph(monkeypatch, inbuild_graph): + """Wire real chat.py helpers + a fixture-bound GraphifyService into sys.modules. + + The US-003 shim's ``handle_widget_message`` does lazy ``from + api.widgets.chat import ...`` at the point of the rewrite, and the + resolvers themselves do ``from modules.knowledge.graph_service + import GraphifyService``. Both have to resolve to something callable + before the test invokes ``widget_proactive.handle_widget_message``. + + This fixture sets up both, scoped to the test (monkeypatch cleans up + automatically), so the snapshot tests exercise the exact code that + runs in production — only the graph source is swapped for the + deterministic US-004 fixture. + + Through US-005/006/007/008 the helpers move into + ``integrations/shopify/widget_proactive.py`` and the shim's + ``api.widgets.chat`` imports go away. Each lift story should narrow + the injection here so it stays a faithful test environment, not a + nostalgic crutch. + """ + ns = _extract_chat_helpers() + + fake_chat = types.ModuleType("api.widgets.chat") + fake_chat._resolve_graph_related_products = ns["_resolve_graph_related_products"] + fake_chat._resolve_cart_recommendations = ns["_resolve_cart_recommendations"] + fake_chat._build_proactive_opener_message = ns["_build_proactive_opener_message"] + fake_chat._build_cart_idle_opener_message = ns["_build_cart_idle_opener_message"] + + fake_graph_service_mod = types.ModuleType("modules.knowledge.graph_service") + + class _FixtureGraphifyService: + async def load_graph(self, workspace_id): # noqa: D401, ARG002 + return inbuild_graph + + fake_graph_service_mod.GraphifyService = _FixtureGraphifyService + + # Parent packages must exist for `from modules.knowledge.graph_service + # import GraphifyService` to walk the chain. ``monkeypatch.setitem`` + # adds-or-replaces and restores cleanly after the test, regardless of + # whether the parent was already in sys.modules. + for parent in ("modules", "modules.knowledge"): + monkeypatch.setitem( + sys.modules, parent, sys.modules.get(parent, types.ModuleType(parent)) + ) + monkeypatch.setitem( + sys.modules, "modules.knowledge.graph_service", fake_graph_service_mod + ) + + for parent in ("api", "api.widgets"): + monkeypatch.setitem( + sys.modules, parent, sys.modules.get(parent, types.ModuleType(parent)) + ) + monkeypatch.setitem(sys.modules, "api.widgets.chat", fake_chat) + + return { + "chat": fake_chat, + "graph_service": fake_graph_service_mod, + "namespace": ns, + } diff --git a/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index 2ea982fec..5b6a47dd1 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -269,3 +269,111 @@ async def test_cart_idle_delegates_to_chat_helpers(fake_chat, db, workspace_id): "trigger_reason": "cart_idle", "related_count": 2, } + + +# ---- US-011 snapshot equivalence (PRD-007 + PRD-008-B byte-equality) -------- +# +# These tests are the byte-equality safety net that gates every Phase 1 +# lift (US-005/006/007/008/010). At US-011 commit time the Shopify plugin +# is still the US-003 shim that delegates back into ``api.widgets.chat``; +# the ``real_chat_with_graph`` fixture in ``conftest.py`` injects the +# REAL helpers (AST-extracted from chat.py) plus a fixture-bound +# GraphifyService so the test exercises the production code path while +# remaining deterministic. +# +# Through US-005/006/007/008 the helpers progressively move into +# ``integrations/shopify/widget_proactive.py``. These tests must KEEP +# PASSING through every lift. If one fails, the lift broke equivalence — +# fix the lift, NOT the golden fixture (per US-011 notes). + + +@pytest.mark.asyncio +async def test_product_page_opener_byte_equality( + real_chat_with_graph, + product_page_context, + expected_product_page_opener, + db, + workspace_id, +): + """PRD-007 product-page opener — byte-equal to the US-004 fixture. + + Exercises the proactive_opener path end-to-end: shim gates on + (trigger_reason, page_context), calls ``_resolve_graph_related_products`` + against the fixture graph, calls ``_build_proactive_opener_message``, + returns the directive as ``result.message``. + + The fixture graph encodes the by_vendor-overrides-FBT quirk + (NetworkX undirected storage) documented in ``fixtures/README.md`` — + that's WHY the expected opener mentions "Hochiki Banshee Wall Sounder" + as a same-vendor sibling rather than as an FBT pair. + """ + result = await widget_proactive.handle_widget_message( + message="(placeholder synthesized by SDK)", + page_context=product_page_context, + trigger_reason="proactive_opener", + workspace_id=workspace_id, + db=db, + ) + + assert result.message == expected_product_page_opener + assert result.context_note == "shopify shim: proactive_opener rewrite" + + +@pytest.mark.asyncio +async def test_cart_idle_opener_byte_equality( + real_chat_with_graph, + cart_idle_context, + expected_cart_idle_opener, + db, + workspace_id, +): + """PRD-008-B cart-idle opener — byte-equal to the US-004 fixture. + + Exercises the cart_idle path end-to-end: multi-seed FBT walk across + every cart line item, aggregation by (paired_with_count, score), + top-3 recommendations rendered into the cart-idle directive. + + The fixture's three cart items (hochiki-aln/-acb/-atg) produce a + deterministic top-3 of (base-ybn, mxpro5, banshee). Banshee is in + the cart-idle output via the elif branch (added together in 15 of + 31 orders) because it pairs with only ONE cart item — the aln-banshee + FBT edge was overwritten by by_vendor in the synthetic graph (a + quirk of NetworkX undirected storage; see fixtures/README.md). + """ + result = await widget_proactive.handle_widget_message( + message="(placeholder synthesized by SDK)", + page_context=cart_idle_context, + trigger_reason="cart_idle", + workspace_id=workspace_id, + db=db, + ) + + assert result.message == expected_cart_idle_opener + assert result.context_note == "shopify shim: cart_idle rewrite" + + +@pytest.mark.parametrize( + "trigger", + [None, "proactive_opener", "cart_idle", "unknown_trigger"], + ids=["no_trigger", "proactive_opener", "cart_idle", "unknown"], +) +@pytest.mark.asyncio +async def test_no_context_no_rewrite(trigger, db, workspace_id): + """US-011 AC test 4: page_context=None ⇒ message returned unchanged. + + Parametrised across every trigger value (including ``None`` and an + unknown trigger) because the shim's gate is symmetric — either side + short-circuits the rewrite. This is a regression guard for the + chat.py ``is_proactive`` semantics replicated in the shim. + """ + result = await widget_proactive.handle_widget_message( + message="user message verbatim", + page_context=None, + trigger_reason=trigger, + workspace_id=workspace_id, + db=db, + ) + + assert result.message == "user message verbatim" + assert result.context_note is None + assert result.telemetry == {} diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index cbf5de1c2..acbb22def 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -16,7 +16,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc ## Phase 1 — Lift Shopify into the plugin (the risky one) - [x] US-004 — Capture proactive opener + cart-idle fixtures *(synthetic-but-realistic acceptable — see PROMPT_build_prd141.md "Special note for US-004"; human reviews after Ralph generates)* -- [ ] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* +- [x] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* - [ ] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder - [ ] US-006 — Move _resolve_graph_related_products into Shopify plugin - [ ] US-007 — Move _resolve_cart_recommendations into Shopify plugin From a104f73d1b7321256ff005e62511a281def8f0c2 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 19:57:23 +0100 Subject: [PATCH 03/11] =?UTF-8?q?feat(prd-141):=20US-005=20=E2=80=94=20lif?= =?UTF-8?q?t=20=5FOPENER=5FCONTEXT=5FFIELDS=20+=20=5Fformat=5Fopener=5Fcon?= =?UTF-8?q?text=5Fvalue=20into=20integrations/shopify/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First ACTUAL move out of orchestrator/api/widgets/chat.py. Creates integrations/shopify/context_fields.py hosting the field-mapping tuple and single-value formatter — verbatim copies, no value or behaviour changes. chat.py now imports both from the new module so _build_proactive_opener_message keeps rendering identical output through US-008. conftest.py and regenerate.py drop the two names from their AST-extract sets and seed them into the exec namespace via a direct import (context_fields has no heavy deps). US-011 byte-equality snapshot tests stay green (14/14). Story: scripts/ralph/prd.json US-005 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- orchestrator/api/widgets/chat.py | 42 ++---------- .../integrations/shopify/context_fields.py | 68 +++++++++++++++++++ .../integrations/shopify/tests/conftest.py | 34 +++++++--- .../shopify/tests/fixtures/README.md | 6 +- .../shopify/tests/fixtures/regenerate.py | 24 ++++++- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 6 files changed, 120 insertions(+), 56 deletions(-) create mode 100644 orchestrator/integrations/shopify/context_fields.py diff --git a/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index 5249c6dc1..b9daa9a8b 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -27,6 +27,10 @@ from api.widgets.auth import WidgetAuthContext, require_permission, widget_auth from core.database.database import get_db +from integrations.shopify.context_fields import ( + _OPENER_CONTEXT_FIELDS, + _format_opener_context_value, +) from modules.tools.widget_callback import ( WIDGET_OPEN_CALLBACK_FORM_NAME, WIDGET_SIGNAL_KEY, @@ -74,44 +78,6 @@ class WidgetChatRequest(BaseModel): }) -# Fields from page_context that the agent gets to ground openers on. -# Order matters — first match per group wins. Numeric/boolean fields are -# coerced to strings only when present + non-default. -_OPENER_CONTEXT_FIELDS: tuple[tuple[str, str], ...] = ( - ("pageType", "page_type"), - ("template", "template"), - # Product - ("productTitle", "product"), - ("productType", "product_type"), - ("productVendor", "vendor"), - ("productPrice", "price"), - ("productAvailable", "in_stock"), - ("productHandle", "product_handle"), - # Collection (when on a collection page) - ("collectionTitle", "collection"), - ("collectionHandle", "collection_handle"), - # Shop / locale - ("shopDomain", "shop"), - ("shopCurrency", "currency"), - ("shopLocale", "locale"), - # Customer / cart - ("customerId", "logged_in_customer_id"), - ("customerTags", "customer_tags"), - ("cartItemCount", "cart_item_count"), - ("cartTotalPrice", "cart_total"), -) - - -def _format_opener_context_value(key: str, value) -> Optional[str]: - """Render a single page-context value into the directive. Returns None - if the value is empty/zero/false and shouldn't be sent to the agent.""" - if value is None or value == "" or value == 0 or value is False: - return None - if isinstance(value, str): - return f'{key}="{value}"' if " " in value or '"' in value else f"{key}={value}" - return f"{key}={value}" - - def _build_proactive_opener_message( page_context: dict, related_products: Optional[list] = None, diff --git a/orchestrator/integrations/shopify/context_fields.py b/orchestrator/integrations/shopify/context_fields.py new file mode 100644 index 000000000..906fea338 --- /dev/null +++ b/orchestrator/integrations/shopify/context_fields.py @@ -0,0 +1,68 @@ +"""Shopify page-context field mapping for proactive openers. + +PRD-141 US-005 — the first ACTUAL move out of +``orchestrator/api/widgets/chat.py``. This module hosts the +Shopify-shaped page-context primitives that the proactive opener +directive builder closes over: + +* :data:`_OPENER_CONTEXT_FIELDS` — ordered Shopify ``page_context`` → + directive-label mapping. Order matters: it dictates the field order + rendered into the proactive directive seen by the agent. +* :func:`_format_opener_context_value` — single-value formatter used + by the directive builder to skip empties and quote string values + that contain whitespace or embedded quotes. + +These were extracted verbatim from chat.py with no value or behaviour +changes. ``_build_proactive_opener_message`` (still in chat.py through +US-008) imports both from here so its rendered output is byte-equal to +the pre-lift baseline captured in +``orchestrator/integrations/shopify/tests/fixtures/``. + +Why this module exists at all: chat.py is the generic widget-chat +router; ``productHandle``, ``cartItems`` and other Shopify-shaped +keys must not appear in generic surfaces (PRD-141 §12). Hosting them +under ``integrations/shopify/`` lets the CI grep gate (US-012) enforce +that boundary. +""" + +from __future__ import annotations + +from typing import Optional + + +# Fields from page_context that the agent gets to ground openers on. +# Order matters — first match per group wins. Numeric/boolean fields are +# coerced to strings only when present + non-default. +_OPENER_CONTEXT_FIELDS: tuple[tuple[str, str], ...] = ( + ("pageType", "page_type"), + ("template", "template"), + # Product + ("productTitle", "product"), + ("productType", "product_type"), + ("productVendor", "vendor"), + ("productPrice", "price"), + ("productAvailable", "in_stock"), + ("productHandle", "product_handle"), + # Collection (when on a collection page) + ("collectionTitle", "collection"), + ("collectionHandle", "collection_handle"), + # Shop / locale + ("shopDomain", "shop"), + ("shopCurrency", "currency"), + ("shopLocale", "locale"), + # Customer / cart + ("customerId", "logged_in_customer_id"), + ("customerTags", "customer_tags"), + ("cartItemCount", "cart_item_count"), + ("cartTotalPrice", "cart_total"), +) + + +def _format_opener_context_value(key: str, value) -> Optional[str]: + """Render a single page-context value into the directive. Returns None + if the value is empty/zero/false and shouldn't be sent to the agent.""" + if value is None or value == "" or value == 0 or value is False: + return None + if isinstance(value, str): + return f'{key}="{value}"' if " " in value or '"' in value else f"{key}={value}" + return f"{key}={value}" diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py index 637206a53..62d96babe 100644 --- a/orchestrator/integrations/shopify/tests/conftest.py +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -48,13 +48,14 @@ _CHAT_PY = _ORCH_ROOT / "api" / "widgets" / "chat.py" -# Names AST-extracted from chat.py. The two non-function entries -# (``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value``) are -# closed over by ``_build_proactive_opener_message``; without them the -# extracted function bodies raise ``NameError`` at call time. +# Names AST-extracted from chat.py — the four still-inline proactive +# helpers. ``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value`` +# were lifted to ``integrations/shopify/context_fields.py`` in +# PRD-141 US-005; they're now imported directly into the exec namespace +# (see ``_extract_chat_helpers``) so the function bodies can close over +# them without NameError. As US-006/007/008 move each remaining helper +# into ``widget_proactive.py``, drop it from this set in lockstep. _WANTED_NAMES = frozenset({ - "_OPENER_CONTEXT_FIELDS", - "_format_opener_context_value", "_build_proactive_opener_message", "_build_cart_idle_opener_message", "_resolve_graph_related_products", @@ -103,13 +104,16 @@ def expected_cart_idle_opener() -> str: def _extract_chat_helpers() -> dict: - """Pull the six proactive helpers out of chat.py via AST. + """Pull the four proactive helpers out of chat.py via AST. Returns a namespace dict containing: - * ``_OPENER_CONTEXT_FIELDS`` — the field-mapping tuple used by the - product-page directive builder. - * ``_format_opener_context_value`` — single-value formatter. + * ``_OPENER_CONTEXT_FIELDS`` — field-mapping tuple, imported from + ``integrations.shopify.context_fields`` (US-005 lifted it out of + chat.py; it's still required in the namespace because the AST- + extracted ``_build_proactive_opener_message`` closes over it). + * ``_format_opener_context_value`` — single-value formatter, same + story as the field mapping. * ``_build_proactive_opener_message`` — product-page directive. * ``_build_cart_idle_opener_message`` — cart-idle directive. * ``_resolve_graph_related_products`` — single-seed FBT/collection/ @@ -125,8 +129,14 @@ def _extract_chat_helpers() -> dict: four helpers is wasteful and brittle. AST extraction reads the source verbatim and execs only the wanted nodes into an isolated namespace, so the test exercises identical bytes to the running - server without paying the import cost. + server without paying the import cost. ``context_fields`` has none + of that baggage, so we import it normally. """ + from integrations.shopify.context_fields import ( + _OPENER_CONTEXT_FIELDS, + _format_opener_context_value, + ) + src = _CHAT_PY.read_text() tree = ast.parse(src) @@ -134,6 +144,8 @@ def _extract_chat_helpers() -> dict: "Optional": Optional, "logger": logging.getLogger("us011_snapshot"), "__name__": "_chat_extracted_for_us011", + "_OPENER_CONTEXT_FIELDS": _OPENER_CONTEXT_FIELDS, + "_format_opener_context_value": _format_opener_context_value, } for node in tree.body: diff --git a/orchestrator/integrations/shopify/tests/fixtures/README.md b/orchestrator/integrations/shopify/tests/fixtures/README.md index 266720c1d..21e09a1db 100644 --- a/orchestrator/integrations/shopify/tests/fixtures/README.md +++ b/orchestrator/integrations/shopify/tests/fixtures/README.md @@ -66,7 +66,7 @@ PYTHONHASHSEED=0 python3 regenerate.py You only need to regenerate if: -- `chat.py`'s proactive helpers change before the lift (e.g. a new field is added to `_OPENER_CONTEXT_FIELDS`), and you want the fixtures to track. **Be careful** — once the lift starts (US-005), the fixtures must NOT be regenerated against the moving code; they are the frozen target. +- The Shopify proactive helpers change before the lift completes (e.g. a new field is added to `_OPENER_CONTEXT_FIELDS` in `integrations/shopify/context_fields.py`, which US-005 lifted out of `chat.py`), and you want the fixtures to track. **Be careful** — the lift is now in progress (US-005+), so the fixtures must NOT be regenerated against the moving code; they are the frozen target. - You want to extend the synthetic graph to cover an additional code path that US-011's snapshot tests should pin. ## Known fixture quirks @@ -74,9 +74,9 @@ You only need to regenerate if: - The graph is `networkx.Graph` (undirected). When two products had both an FBT and a by_vendor edge between them in the source data, the by_vendor edge overwrites the FBT edge in NetworkX undirected storage. This is *deterministic* but means the captured outputs reflect that loss. Production may use `MultiDiGraph` — the byte-equality contract still holds against THIS fixture, which is all US-011 needs. - Iteration order within FBT and by_vendor groups depends on Python dict insertion order, which is stable since 3.7+ and reproduced exactly by `regenerate.py`. -## Files chat.py reads from page_context +## Files the Shopify proactive helpers read from page_context -For reference (extracted from `_OPENER_CONTEXT_FIELDS` in `api/widgets/chat.py`): +For reference (extracted from `_OPENER_CONTEXT_FIELDS` in `integrations/shopify/context_fields.py`, lifted from `api/widgets/chat.py` in PRD-141 US-005): ``` pageType, template, diff --git a/orchestrator/integrations/shopify/tests/fixtures/regenerate.py b/orchestrator/integrations/shopify/tests/fixtures/regenerate.py index 895a438c1..77e17dfbd 100644 --- a/orchestrator/integrations/shopify/tests/fixtures/regenerate.py +++ b/orchestrator/integrations/shopify/tests/fixtures/regenerate.py @@ -232,9 +232,13 @@ def build_graph() -> nx.Graph: # Extract the proactive helpers from chat.py and exec them in isolation # --------------------------------------------------------------------------- +# PRD-141 US-005 moved ``_OPENER_CONTEXT_FIELDS`` and +# ``_format_opener_context_value`` to +# ``orchestrator/integrations/shopify/context_fields.py``. They're no +# longer extractable from chat.py but the AST-exec'd function bodies +# still close over them — ``_extract_chat_helpers`` imports them from +# their new home and seeds the namespace before exec'ing. WANTED_NAMES = { - "_OPENER_CONTEXT_FIELDS", - "_format_opener_context_value", "_build_proactive_opener_message", "_build_cart_idle_opener_message", "_resolve_graph_related_products", @@ -243,12 +247,24 @@ def build_graph() -> nx.Graph: def _extract_chat_helpers(graph: nx.Graph) -> dict: - """Parse chat.py with ast, return a namespace containing the six helpers. + """Parse chat.py with ast, return a namespace containing the four helpers. Stubs ``modules.knowledge.graph_service.GraphifyService`` to hand back the supplied in-memory graph so ``_resolve_graph_related_products`` / ``_resolve_cart_recommendations`` can run without a real workspace. + + Seeds the namespace with ``_OPENER_CONTEXT_FIELDS`` and + ``_format_opener_context_value`` (lifted to + ``integrations.shopify.context_fields`` in US-005) so the exec'd + ``_build_proactive_opener_message`` body can resolve them. """ + if str(ORCHESTRATOR_ROOT) not in sys.path: + sys.path.insert(0, str(ORCHESTRATOR_ROOT)) + from integrations.shopify.context_fields import ( + _OPENER_CONTEXT_FIELDS, + _format_opener_context_value, + ) + src = CHAT_PY.read_text() tree = ast.parse(src) @@ -258,6 +274,8 @@ def _extract_chat_helpers(graph: nx.Graph) -> dict: "Optional": Optional, "logger": logging.getLogger("fixture_generator"), "__name__": "_chat_extracted", + "_OPENER_CONTEXT_FIELDS": _OPENER_CONTEXT_FIELDS, + "_format_opener_context_value": _format_opener_context_value, } for node in tree.body: diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index acbb22def..bc4bf826c 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -17,7 +17,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-004 — Capture proactive opener + cart-idle fixtures *(synthetic-but-realistic acceptable — see PROMPT_build_prd141.md "Special note for US-004"; human reviews after Ralph generates)* - [x] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* -- [ ] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder +- [x] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder - [ ] US-006 — Move _resolve_graph_related_products into Shopify plugin - [ ] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [ ] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin From 2ea609c8d53f625a93831ae1c995b1fdddd4b46e Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:10:06 +0100 Subject: [PATCH 04/11] =?UTF-8?q?feat(prd-141):=20US-006=20=E2=80=94=20lif?= =?UTF-8?q?t=20=5Fresolve=5Fgraph=5Frelated=5Fproducts=20into=20integratio?= =?UTF-8?q?ns/shopify/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the single-seed FBT/collection/vendor walk verbatim from orchestrator/api/widgets/chat.py to orchestrator/integrations/shopify/widget_proactive.py. The shim's product-page path now calls the local function and only lazy-imports _build_proactive_opener_message from chat.py (until US-008). chat.py keeps the existing call site at the proactive_opener branch and imports the resolver from the new home; the function definition is gone from chat.py. Conftest drops the AST extraction for this name and test_widget_proactive's fake_chat fixture monkeypatches the local resolver so delegation tests still record calls. All 32 integrations/ tests stay green including the US-011 byte-equality snapshots. Story: scripts/ralph/prd.json US-006 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- orchestrator/api/widgets/chat.py | 89 +---------------- .../integrations/shopify/tests/conftest.py | 29 +++--- .../shopify/tests/test_widget_proactive.py | 19 +++- .../integrations/shopify/widget_proactive.py | 96 ++++++++++++++++++- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 5 files changed, 129 insertions(+), 106 deletions(-) diff --git a/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index b9daa9a8b..d3276732a 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -31,6 +31,7 @@ _OPENER_CONTEXT_FIELDS, _format_opener_context_value, ) +from integrations.shopify.widget_proactive import _resolve_graph_related_products from modules.tools.widget_callback import ( WIDGET_OPEN_CALLBACK_FORM_NAME, WIDGET_SIGNAL_KEY, @@ -155,94 +156,6 @@ def _build_proactive_opener_message( ) -async def _resolve_graph_related_products( - workspace_id: str, - page_context: dict, - *, - max_per_relation: int = 1, -) -> list[dict]: - """Pull top related products from the workspace knowledge graph. - - Looks up the seed product node by Shopify handle in page_context, then - walks 1-hop edges by relation type: - - frequently_bought_with (highest co_count wins — real customer signal) - - in_collection (most-connected sibling — likely a category anchor) - - by_vendor (any same-vendor sibling) - - Returns at most ``max_per_relation`` entries per relation type. Empty - list when no seed product, no graph, or any failure — caller treats - that as "fall back to plain Layer-1 opener". - """ - handle = (page_context or {}).get("productHandle") - title = (page_context or {}).get("productTitle") - if not handle and not title: - return [] # No seed (cart/checkout/collection page) — nothing to traverse - - try: - from modules.knowledge.graph_service import GraphifyService - - gs = GraphifyService() - graph = await gs.load_graph(workspace_id) - if graph is None: - return [] - - # Find the seed node by handle (preferred) or label match. - seed_id = None - for node_id, attrs in graph.nodes(data=True): - node_attrs = attrs.get("attrs") or {} - if handle and node_attrs.get("handle") == handle: - seed_id = node_id - break - if seed_id is None and title: - for node_id, attrs in graph.nodes(data=True): - if (attrs.get("file_type") == "shopify_product" - and attrs.get("label") == title): - seed_id = node_id - break - if seed_id is None: - return [] - - # Walk 1-hop, group by relation type, sort each group by signal strength. - by_relation: dict[str, list[dict]] = {} - for u, v, edata in graph.edges(seed_id, data=True): - rel = (edata.get("relation") or "").lower() - if rel not in ("frequently_bought_with", "in_collection", "by_vendor"): - continue - other = v if u == seed_id else u - other_attrs = graph.nodes[other] - # Only return product nodes (skip variants/vendors as targets — - # those aren't recommendable on their own to a shopper) - if other_attrs.get("file_type") not in ("shopify_product", "shopify_collection"): - # in_collection targets ARE collections; that's fine for catalog framing. - if rel != "in_collection": - continue - edge_attrs = edata.get("attrs") or {} - by_relation.setdefault(rel, []).append({ - "relation": rel, - "label": other_attrs.get("label") or other, - "type": other_attrs.get("file_type", ""), - "confidence": edata.get("confidence_score", 0), - "co_count": edge_attrs.get("co_count"), - "total_orders": edge_attrs.get("total_orders"), - "weight": edata.get("weight", 0), - }) - - # FBT: sort by co_count (raw signal strength). - by_relation.get("frequently_bought_with", []).sort( - key=lambda p: -(p.get("co_count") or 0) - ) - # Collection / vendor: arbitrary — take first. - - out: list[dict] = [] - for rel in ("frequently_bought_with", "in_collection", "by_vendor"): - out.extend(by_relation.get(rel, [])[:max_per_relation]) - return out - - except Exception as e: # noqa: BLE001 — opener falls back gracefully - logger.warning("_resolve_graph_related_products failed: %s", e) - return [] - - async def _resolve_cart_recommendations( workspace_id: str, page_context: dict, diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py index 62d96babe..b643ce323 100644 --- a/orchestrator/integrations/shopify/tests/conftest.py +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -48,17 +48,20 @@ _CHAT_PY = _ORCH_ROOT / "api" / "widgets" / "chat.py" -# Names AST-extracted from chat.py — the four still-inline proactive -# helpers. ``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value`` +# Names AST-extracted from chat.py — the proactive helpers still inline +# there. ``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value`` # were lifted to ``integrations/shopify/context_fields.py`` in # PRD-141 US-005; they're now imported directly into the exec namespace # (see ``_extract_chat_helpers``) so the function bodies can close over -# them without NameError. As US-006/007/008 move each remaining helper -# into ``widget_proactive.py``, drop it from this set in lockstep. +# them without NameError. ``_resolve_graph_related_products`` was lifted +# to ``integrations/shopify/widget_proactive.py`` in PRD-141 US-006; the +# shim's product-page path now resolves it locally, so it no longer +# needs AST extraction from chat.py. As US-007/008 move each remaining +# helper into ``widget_proactive.py``, drop it from this set in +# lockstep. _WANTED_NAMES = frozenset({ "_build_proactive_opener_message", "_build_cart_idle_opener_message", - "_resolve_graph_related_products", "_resolve_cart_recommendations", }) @@ -116,13 +119,18 @@ def _extract_chat_helpers() -> dict: story as the field mapping. * ``_build_proactive_opener_message`` — product-page directive. * ``_build_cart_idle_opener_message`` — cart-idle directive. - * ``_resolve_graph_related_products`` — single-seed FBT/collection/ - vendor walk. * ``_resolve_cart_recommendations`` — multi-seed cart FBT walk. - The resolvers do ``from modules.knowledge.graph_service import - GraphifyService`` lazily; the caller (``real_chat_with_graph``) - arranges that fake module before invoking them. + ``_resolve_graph_related_products`` was lifted to + ``integrations.shopify.widget_proactive`` in PRD-141 US-006 and is + no longer extracted here — the shim calls the local function and + the local function does the same lazy ``GraphifyService`` import + the caller (``real_chat_with_graph``) arranges. + + The remaining cart resolver does ``from + modules.knowledge.graph_service import GraphifyService`` lazily; + the caller (``real_chat_with_graph``) arranges that fake module + before invoking it. Why AST instead of ``import``: chat.py is a FastAPI router that drags in SQLAlchemy, Redis, RAG, multimodal, etc. Loading it just to read @@ -201,7 +209,6 @@ def real_chat_with_graph(monkeypatch, inbuild_graph): ns = _extract_chat_helpers() fake_chat = types.ModuleType("api.widgets.chat") - fake_chat._resolve_graph_related_products = ns["_resolve_graph_related_products"] fake_chat._resolve_cart_recommendations = ns["_resolve_cart_recommendations"] fake_chat._build_proactive_opener_message = ns["_build_proactive_opener_message"] fake_chat._build_cart_idle_opener_message = ns["_build_cart_idle_opener_message"] diff --git a/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index 5b6a47dd1..2b04befae 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -48,7 +48,18 @@ def workspace_id(): @pytest.fixture def fake_chat(monkeypatch): - """Replace ``api.widgets.chat`` in ``sys.modules`` with a fake. + """Replace the shim's downstream dependencies with fakes. + + The shim calls a mix of LOCAL and chat.py helpers depending on the + trigger path and the current state of the Phase 1 lift: + + * ``_resolve_graph_related_products`` — local to ``widget_proactive`` + since PRD-141 US-006; patched in place via ``monkeypatch.setattr``. + * ``_resolve_cart_recommendations`` — still in ``api.widgets.chat`` + (until US-007); patched via a fake module in ``sys.modules``. + * ``_build_proactive_opener_message`` / + ``_build_cart_idle_opener_message`` — still in ``api.widgets.chat`` + (until US-008); same fake-module treatment. The fake records every call so tests can assert the shim forwards ``workspace_id`` and ``page_context`` correctly, and returns a @@ -97,12 +108,16 @@ def fake_build_cart(page_context, recommendations=None): }) return "FAKE_CART_IDLE_OPENER_MESSAGE" - fake._resolve_graph_related_products = fake_resolve_products fake._resolve_cart_recommendations = fake_resolve_cart fake._build_proactive_opener_message = fake_build_product fake._build_cart_idle_opener_message = fake_build_cart monkeypatch.setitem(sys.modules, "api.widgets.chat", fake) + monkeypatch.setattr( + widget_proactive, + "_resolve_graph_related_products", + fake_resolve_products, + ) return calls diff --git a/orchestrator/integrations/shopify/widget_proactive.py b/orchestrator/integrations/shopify/widget_proactive.py index f69fee24e..a0c0c9961 100644 --- a/orchestrator/integrations/shopify/widget_proactive.py +++ b/orchestrator/integrations/shopify/widget_proactive.py @@ -41,6 +41,7 @@ from __future__ import annotations +import logging from typing import Optional from uuid import UUID @@ -48,6 +49,96 @@ from integrations import WidgetPluginResult +logger = logging.getLogger(__name__) + + +async def _resolve_graph_related_products( + workspace_id: str, + page_context: dict, + *, + max_per_relation: int = 1, +) -> list[dict]: + """Pull top related products from the workspace knowledge graph. + + Looks up the seed product node by Shopify handle in page_context, then + walks 1-hop edges by relation type: + - frequently_bought_with (highest co_count wins — real customer signal) + - in_collection (most-connected sibling — likely a category anchor) + - by_vendor (any same-vendor sibling) + + Returns at most ``max_per_relation`` entries per relation type. Empty + list when no seed product, no graph, or any failure — caller treats + that as "fall back to plain Layer-1 opener". + """ + handle = (page_context or {}).get("productHandle") + title = (page_context or {}).get("productTitle") + if not handle and not title: + return [] # No seed (cart/checkout/collection page) — nothing to traverse + + try: + from modules.knowledge.graph_service import GraphifyService + + gs = GraphifyService() + graph = await gs.load_graph(workspace_id) + if graph is None: + return [] + + # Find the seed node by handle (preferred) or label match. + seed_id = None + for node_id, attrs in graph.nodes(data=True): + node_attrs = attrs.get("attrs") or {} + if handle and node_attrs.get("handle") == handle: + seed_id = node_id + break + if seed_id is None and title: + for node_id, attrs in graph.nodes(data=True): + if (attrs.get("file_type") == "shopify_product" + and attrs.get("label") == title): + seed_id = node_id + break + if seed_id is None: + return [] + + # Walk 1-hop, group by relation type, sort each group by signal strength. + by_relation: dict[str, list[dict]] = {} + for u, v, edata in graph.edges(seed_id, data=True): + rel = (edata.get("relation") or "").lower() + if rel not in ("frequently_bought_with", "in_collection", "by_vendor"): + continue + other = v if u == seed_id else u + other_attrs = graph.nodes[other] + # Only return product nodes (skip variants/vendors as targets — + # those aren't recommendable on their own to a shopper) + if other_attrs.get("file_type") not in ("shopify_product", "shopify_collection"): + # in_collection targets ARE collections; that's fine for catalog framing. + if rel != "in_collection": + continue + edge_attrs = edata.get("attrs") or {} + by_relation.setdefault(rel, []).append({ + "relation": rel, + "label": other_attrs.get("label") or other, + "type": other_attrs.get("file_type", ""), + "confidence": edata.get("confidence_score", 0), + "co_count": edge_attrs.get("co_count"), + "total_orders": edge_attrs.get("total_orders"), + "weight": edata.get("weight", 0), + }) + + # FBT: sort by co_count (raw signal strength). + by_relation.get("frequently_bought_with", []).sort( + key=lambda p: -(p.get("co_count") or 0) + ) + # Collection / vendor: arbitrary — take first. + + out: list[dict] = [] + for rel in ("frequently_bought_with", "in_collection", "by_vendor"): + out.extend(by_relation.get(rel, [])[:max_per_relation]) + return out + + except Exception as e: # noqa: BLE001 — opener falls back gracefully + logger.warning("_resolve_graph_related_products failed: %s", e) + return [] + async def handle_widget_message( *, @@ -103,10 +194,7 @@ async def handle_widget_message( }, ) - from api.widgets.chat import ( - _build_proactive_opener_message, - _resolve_graph_related_products, - ) + from api.widgets.chat import _build_proactive_opener_message related_products = await _resolve_graph_related_products( workspace_str, page_context, diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index bc4bf826c..372f55bab 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -18,7 +18,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-004 — Capture proactive opener + cart-idle fixtures *(synthetic-but-realistic acceptable — see PROMPT_build_prd141.md "Special note for US-004"; human reviews after Ralph generates)* - [x] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* - [x] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder -- [ ] US-006 — Move _resolve_graph_related_products into Shopify plugin +- [x] US-006 — Move _resolve_graph_related_products into Shopify plugin - [ ] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [ ] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin - [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* From e90b7186be5ee5f96db03d13785c183beec40408 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:15:18 +0100 Subject: [PATCH 05/11] =?UTF-8?q?feat(prd-141):=20US-007=20=E2=80=94=20lif?= =?UTF-8?q?t=20=5Fresolve=5Fcart=5Frecommendations=20into=20integrations/s?= =?UTF-8?q?hopify/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the multi-seed cart-idle FBT walk verbatim from orchestrator/api/widgets/chat.py to orchestrator/integrations/shopify/widget_proactive.py. The shim's cart_idle path now calls the local resolver and only lazy-imports _build_cart_idle_opener_message from chat.py (until US-008). chat.py keeps the existing call site at the cart_idle branch and imports the resolver from the new home; the function definition is gone from chat.py. Conftest drops the AST extraction for this name and test_widget_proactive's fake_chat fixture monkeypatches the local resolver so delegation tests still record calls. All 32 integrations/ tests stay green including the US-011 byte-equality snapshots. Story: scripts/ralph/prd.json US-007 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- orchestrator/api/widgets/chat.py | 106 +-------------- .../integrations/shopify/tests/conftest.py | 41 +++--- .../shopify/tests/test_widget_proactive.py | 12 +- .../integrations/shopify/widget_proactive.py | 128 ++++++++++++++++-- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 5 files changed, 143 insertions(+), 146 deletions(-) diff --git a/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index d3276732a..40d749da6 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -31,7 +31,10 @@ _OPENER_CONTEXT_FIELDS, _format_opener_context_value, ) -from integrations.shopify.widget_proactive import _resolve_graph_related_products +from integrations.shopify.widget_proactive import ( + _resolve_cart_recommendations, + _resolve_graph_related_products, +) from modules.tools.widget_callback import ( WIDGET_OPEN_CALLBACK_FORM_NAME, WIDGET_SIGNAL_KEY, @@ -156,107 +159,6 @@ def _build_proactive_opener_message( ) -async def _resolve_cart_recommendations( - workspace_id: str, - page_context: dict, - *, - max_recs: int = 3, -) -> list[dict]: - """Pull cross-sell recommendations for the items currently in the cart. - - PRD-008-B Feature C2 (cart-idle): walks FBT edges across every cart - line-item, aggregates co_count across overlapping recommendations - (an item that pairs with multiple cart items scores higher), removes - products already in the cart, returns the top ``max_recs`` by score. - - Empty list when no cart items, no graph, or any failure — caller - treats that as "fall back to merchant's static greeting". - """ - cart_items = (page_context or {}).get("cartItems") or [] - if not isinstance(cart_items, list) or not cart_items: - return [] - - # Cart items may arrive as [{handle, id, title, qty}, ...] OR as bare - # handles/strings — be defensive about shape. - seed_handles: set[str] = set() - for item in cart_items: - if isinstance(item, dict): - h = item.get("handle") or item.get("product_handle") - if h: - seed_handles.add(str(h)) - elif isinstance(item, str): - seed_handles.add(item) - if not seed_handles: - return [] - - try: - from modules.knowledge.graph_service import GraphifyService - - gs = GraphifyService() - graph = await gs.load_graph(workspace_id) - if graph is None: - return [] - - # Locate every cart-item node by handle. Skip silently if a handle - # isn't in the graph (newly-added product, catalog out of sync). - seed_node_ids: set = set() - cart_node_ids: set = set() # for exclusion from recs - for node_id, attrs in graph.nodes(data=True): - node_attrs = attrs.get("attrs") or {} - if attrs.get("file_type") == "shopify_product": - h = node_attrs.get("handle") - if h and h in seed_handles: - seed_node_ids.add(node_id) - cart_node_ids.add(node_id) - if not seed_node_ids: - return [] - - # Aggregate FBT recommendations across all seeds. Score = sum of - # co_count across seeds it pairs with. A product paired with 2 of - # 3 cart items scores higher than one paired with just 1. - candidates: dict = {} # node_id -> {label, score, total_orders, paired_with} - for seed_id in seed_node_ids: - for u, v, edata in graph.edges(seed_id, data=True): - rel = (edata.get("relation") or "").lower() - if rel != "frequently_bought_with": - continue - other = v if u == seed_id else u - if other in cart_node_ids: - continue # don't recommend what's already in the cart - other_attrs = graph.nodes[other] - if other_attrs.get("file_type") != "shopify_product": - continue - edge_attrs = edata.get("attrs") or {} - co_count = edge_attrs.get("co_count") or 0 - entry = candidates.setdefault(other, { - "label": other_attrs.get("label") or other, - "handle": (other_attrs.get("attrs") or {}).get("handle"), - "score": 0, - "total_orders": edge_attrs.get("total_orders") or 0, - "paired_with_count": 0, - }) - entry["score"] += co_count - entry["paired_with_count"] += 1 - # Track the widest total_orders denominator seen (for citation). - if edge_attrs.get("total_orders", 0) > entry["total_orders"]: - entry["total_orders"] = edge_attrs["total_orders"] - - if not candidates: - return [] - - # Rank: prefer items paired with the MOST cart items (cross-cutting - # adds = strongest signal), break ties on aggregate score. - ranked = sorted( - candidates.values(), - key=lambda c: (-c["paired_with_count"], -c["score"]), - ) - return ranked[:max_recs] - - except Exception as e: # noqa: BLE001 - logger.warning("_resolve_cart_recommendations failed: %s", e) - return [] - - def _build_cart_idle_opener_message( page_context: dict, recommendations: Optional[list] = None, diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py index b643ce323..d6c3ab6ce 100644 --- a/orchestrator/integrations/shopify/tests/conftest.py +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -54,15 +54,14 @@ # PRD-141 US-005; they're now imported directly into the exec namespace # (see ``_extract_chat_helpers``) so the function bodies can close over # them without NameError. ``_resolve_graph_related_products`` was lifted -# to ``integrations/shopify/widget_proactive.py`` in PRD-141 US-006; the -# shim's product-page path now resolves it locally, so it no longer -# needs AST extraction from chat.py. As US-007/008 move each remaining -# helper into ``widget_proactive.py``, drop it from this set in -# lockstep. +# to ``integrations/shopify/widget_proactive.py`` in PRD-141 US-006 and +# ``_resolve_cart_recommendations`` was lifted there in US-007; the +# shim's resolver paths now run locally, so neither needs AST +# extraction from chat.py. As US-008 moves the remaining builders into +# ``widget_proactive.py``, drop them from this set in lockstep. _WANTED_NAMES = frozenset({ "_build_proactive_opener_message", "_build_cart_idle_opener_message", - "_resolve_cart_recommendations", }) @@ -107,7 +106,7 @@ def expected_cart_idle_opener() -> str: def _extract_chat_helpers() -> dict: - """Pull the four proactive helpers out of chat.py via AST. + """Pull the remaining proactive builders out of chat.py via AST. Returns a namespace dict containing: @@ -119,22 +118,17 @@ def _extract_chat_helpers() -> dict: story as the field mapping. * ``_build_proactive_opener_message`` — product-page directive. * ``_build_cart_idle_opener_message`` — cart-idle directive. - * ``_resolve_cart_recommendations`` — multi-seed cart FBT walk. - ``_resolve_graph_related_products`` was lifted to - ``integrations.shopify.widget_proactive`` in PRD-141 US-006 and is - no longer extracted here — the shim calls the local function and - the local function does the same lazy ``GraphifyService`` import - the caller (``real_chat_with_graph``) arranges. - - The remaining cart resolver does ``from - modules.knowledge.graph_service import GraphifyService`` lazily; - the caller (``real_chat_with_graph``) arranges that fake module - before invoking it. + ``_resolve_graph_related_products`` (US-006) and + ``_resolve_cart_recommendations`` (US-007) were lifted to + ``integrations.shopify.widget_proactive`` and are no longer + extracted here — the shim calls the local functions and they do + the same lazy ``GraphifyService`` import the caller + (``real_chat_with_graph``) arranges. Why AST instead of ``import``: chat.py is a FastAPI router that drags in SQLAlchemy, Redis, RAG, multimodal, etc. Loading it just to read - four helpers is wasteful and brittle. AST extraction reads the + two builders is wasteful and brittle. AST extraction reads the source verbatim and execs only the wanted nodes into an isolated namespace, so the test exercises identical bytes to the running server without paying the import cost. ``context_fields`` has none @@ -177,10 +171,10 @@ def _extract_chat_helpers() -> dict: missing = _WANTED_NAMES - set(ns) if missing: raise RuntimeError( - "chat.py is missing expected proactive helpers " - f"{sorted(missing)}. If a US-005/006/007/008 lift moved them, " - "update conftest.py to point the AST extractor at the new " - "location (likely integrations/shopify/widget_proactive.py)." + "chat.py is missing expected proactive builders " + f"{sorted(missing)}. If a US-008 lift moved them, update " + "conftest.py to point the AST extractor at the new location " + "(likely integrations/shopify/widget_proactive.py)." ) return ns @@ -209,7 +203,6 @@ def real_chat_with_graph(monkeypatch, inbuild_graph): ns = _extract_chat_helpers() fake_chat = types.ModuleType("api.widgets.chat") - fake_chat._resolve_cart_recommendations = ns["_resolve_cart_recommendations"] fake_chat._build_proactive_opener_message = ns["_build_proactive_opener_message"] fake_chat._build_cart_idle_opener_message = ns["_build_cart_idle_opener_message"] diff --git a/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index 2b04befae..2448822ae 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -55,11 +55,11 @@ def fake_chat(monkeypatch): * ``_resolve_graph_related_products`` — local to ``widget_proactive`` since PRD-141 US-006; patched in place via ``monkeypatch.setattr``. - * ``_resolve_cart_recommendations`` — still in ``api.widgets.chat`` - (until US-007); patched via a fake module in ``sys.modules``. + * ``_resolve_cart_recommendations`` — local to ``widget_proactive`` + since PRD-141 US-007; patched in place via ``monkeypatch.setattr``. * ``_build_proactive_opener_message`` / ``_build_cart_idle_opener_message`` — still in ``api.widgets.chat`` - (until US-008); same fake-module treatment. + (until US-008); patched via a fake module in ``sys.modules``. The fake records every call so tests can assert the shim forwards ``workspace_id`` and ``page_context`` correctly, and returns a @@ -108,7 +108,6 @@ def fake_build_cart(page_context, recommendations=None): }) return "FAKE_CART_IDLE_OPENER_MESSAGE" - fake._resolve_cart_recommendations = fake_resolve_cart fake._build_proactive_opener_message = fake_build_product fake._build_cart_idle_opener_message = fake_build_cart @@ -118,6 +117,11 @@ def fake_build_cart(page_context, recommendations=None): "_resolve_graph_related_products", fake_resolve_products, ) + monkeypatch.setattr( + widget_proactive, + "_resolve_cart_recommendations", + fake_resolve_cart, + ) return calls diff --git a/orchestrator/integrations/shopify/widget_proactive.py b/orchestrator/integrations/shopify/widget_proactive.py index a0c0c9961..68cb2bb9f 100644 --- a/orchestrator/integrations/shopify/widget_proactive.py +++ b/orchestrator/integrations/shopify/widget_proactive.py @@ -6,23 +6,23 @@ This module is a **shim**, not a rewrite. It encapsulates the dispatch contract — ``handle_widget_message`` matching the :class:`integrations.WidgetPlugin` protocol — and delegates to the -existing inline Shopify helpers still living in +remaining inline Shopify helpers still living in ``orchestrator/api/widgets/chat.py``: -* ``_resolve_graph_related_products`` (product-page FBT / collection / - vendor walk) -* ``_resolve_cart_recommendations`` (cart-idle multi-seed FBT walk) * ``_build_proactive_opener_message`` (product-page directive builder) * ``_build_cart_idle_opener_message`` (cart-idle directive builder) -US-006/007/008 will move those four helpers into this file. US-010 -will delete the chat.py inline dispatch and route every widget chat -request through ``PLUGIN_REGISTRY``. At that point the imports below -become local definitions and this docstring's "shim" framing goes -away. +The two graph resolvers (``_resolve_graph_related_products`` lifted in +US-006, ``_resolve_cart_recommendations`` lifted in US-007) now live +in this file and the shim calls them locally. -The four chat.py imports happen **inside** ``handle_widget_message``, -beneath the early-return gate. Two reasons: +US-008 will move the two builders here. US-010 will delete the chat.py +inline dispatch and route every widget chat request through +``PLUGIN_REGISTRY``. At that point the imports below become local +definitions and this docstring's "shim" framing goes away. + +The remaining chat.py imports happen **inside** +``handle_widget_message``, beneath the early-return gate. Two reasons: 1. Circular-import safety. During Phase 1 there is a window where chat.py imports back from this module (US-006/007/008 move helpers @@ -140,6 +140,107 @@ async def _resolve_graph_related_products( return [] +async def _resolve_cart_recommendations( + workspace_id: str, + page_context: dict, + *, + max_recs: int = 3, +) -> list[dict]: + """Pull cross-sell recommendations for the items currently in the cart. + + PRD-008-B Feature C2 (cart-idle): walks FBT edges across every cart + line-item, aggregates co_count across overlapping recommendations + (an item that pairs with multiple cart items scores higher), removes + products already in the cart, returns the top ``max_recs`` by score. + + Empty list when no cart items, no graph, or any failure — caller + treats that as "fall back to merchant's static greeting". + """ + cart_items = (page_context or {}).get("cartItems") or [] + if not isinstance(cart_items, list) or not cart_items: + return [] + + # Cart items may arrive as [{handle, id, title, qty}, ...] OR as bare + # handles/strings — be defensive about shape. + seed_handles: set[str] = set() + for item in cart_items: + if isinstance(item, dict): + h = item.get("handle") or item.get("product_handle") + if h: + seed_handles.add(str(h)) + elif isinstance(item, str): + seed_handles.add(item) + if not seed_handles: + return [] + + try: + from modules.knowledge.graph_service import GraphifyService + + gs = GraphifyService() + graph = await gs.load_graph(workspace_id) + if graph is None: + return [] + + # Locate every cart-item node by handle. Skip silently if a handle + # isn't in the graph (newly-added product, catalog out of sync). + seed_node_ids: set = set() + cart_node_ids: set = set() # for exclusion from recs + for node_id, attrs in graph.nodes(data=True): + node_attrs = attrs.get("attrs") or {} + if attrs.get("file_type") == "shopify_product": + h = node_attrs.get("handle") + if h and h in seed_handles: + seed_node_ids.add(node_id) + cart_node_ids.add(node_id) + if not seed_node_ids: + return [] + + # Aggregate FBT recommendations across all seeds. Score = sum of + # co_count across seeds it pairs with. A product paired with 2 of + # 3 cart items scores higher than one paired with just 1. + candidates: dict = {} # node_id -> {label, score, total_orders, paired_with} + for seed_id in seed_node_ids: + for u, v, edata in graph.edges(seed_id, data=True): + rel = (edata.get("relation") or "").lower() + if rel != "frequently_bought_with": + continue + other = v if u == seed_id else u + if other in cart_node_ids: + continue # don't recommend what's already in the cart + other_attrs = graph.nodes[other] + if other_attrs.get("file_type") != "shopify_product": + continue + edge_attrs = edata.get("attrs") or {} + co_count = edge_attrs.get("co_count") or 0 + entry = candidates.setdefault(other, { + "label": other_attrs.get("label") or other, + "handle": (other_attrs.get("attrs") or {}).get("handle"), + "score": 0, + "total_orders": edge_attrs.get("total_orders") or 0, + "paired_with_count": 0, + }) + entry["score"] += co_count + entry["paired_with_count"] += 1 + # Track the widest total_orders denominator seen (for citation). + if edge_attrs.get("total_orders", 0) > entry["total_orders"]: + entry["total_orders"] = edge_attrs["total_orders"] + + if not candidates: + return [] + + # Rank: prefer items paired with the MOST cart items (cross-cutting + # adds = strongest signal), break ties on aggregate score. + ranked = sorted( + candidates.values(), + key=lambda c: (-c["paired_with_count"], -c["score"]), + ) + return ranked[:max_recs] + + except Exception as e: # noqa: BLE001 + logger.warning("_resolve_cart_recommendations failed: %s", e) + return [] + + async def handle_widget_message( *, message: str, @@ -173,10 +274,7 @@ async def handle_widget_message( workspace_str = str(workspace_id) if trigger_reason == "cart_idle": - from api.widgets.chat import ( - _build_cart_idle_opener_message, - _resolve_cart_recommendations, - ) + from api.widgets.chat import _build_cart_idle_opener_message recommendations = await _resolve_cart_recommendations( workspace_str, page_context, diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index 372f55bab..e64ea29e9 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -19,7 +19,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-011 — Add snapshot equivalence tests for product-page opener and cart-idle opener *(REORDERED ahead of the lifts — runs against the US-003 shim so byte-equality guards every move; must pass now and stay green through US-005–US-010)* - [x] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder - [x] US-006 — Move _resolve_graph_related_products into Shopify plugin -- [ ] US-007 — Move _resolve_cart_recommendations into Shopify plugin +- [x] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [ ] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin - [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* - [ ] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions From 508cc22b0be7fb8bab1c70dc5cda00310b0be869 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:23:48 +0100 Subject: [PATCH 06/11] =?UTF-8?q?feat(prd-141):=20US-008=20=E2=80=94=20lif?= =?UTF-8?q?t=20proactive=20opener=20+=20cart-idle=20builders=20into=20inte?= =?UTF-8?q?grations/shopify/?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move _build_proactive_opener_message and _build_cart_idle_opener_message verbatim from orchestrator/api/widgets/chat.py to orchestrator/integrations/shopify/widget_proactive.py. chat.py now imports both from the integrations module; widget_proactive's handle_widget_message calls the local builders directly (lazy api.widgets.chat imports dropped). Tests rewired: orchestrator/tests/test_widget_proactive_prd007.py imports the two builders from their new home; integrations/shopify/tests/test_widget_proactive.py's fake_chat fixture now monkey-patches the builders on widget_proactive (consistent with the resolver patches from US-006/007); conftest.py's AST extraction of chat.py is removed — only the GraphifyService stub remains. All 32 integrations tests pass including the byte-equality snapshot tests. Story: scripts/ralph/prd.json US-008 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- orchestrator/api/widgets/chat.py | 149 +----------- .../integrations/shopify/context_fields.py | 7 +- .../integrations/shopify/tests/conftest.py | 163 ++----------- .../shopify/tests/test_widget_proactive.py | 89 ++++---- .../integrations/shopify/widget_proactive.py | 216 ++++++++++++++---- .../tests/test_widget_proactive_prd007.py | 22 +- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 7 files changed, 265 insertions(+), 383 deletions(-) diff --git a/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index 40d749da6..6428e8312 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -27,11 +27,9 @@ from api.widgets.auth import WidgetAuthContext, require_permission, widget_auth from core.database.database import get_db -from integrations.shopify.context_fields import ( - _OPENER_CONTEXT_FIELDS, - _format_opener_context_value, -) from integrations.shopify.widget_proactive import ( + _build_cart_idle_opener_message, + _build_proactive_opener_message, _resolve_cart_recommendations, _resolve_graph_related_products, ) @@ -82,149 +80,6 @@ class WidgetChatRequest(BaseModel): }) -def _build_proactive_opener_message( - page_context: dict, - related_products: Optional[list] = None, -) -> str: - """Synthesize the user-side message for a proactive opener request. - - The widget never sends real user text for proactive openers — instead - we synthesize a directive carrying the FULL page context the agent - needs to ground a contextual one-line opener. - - PRD-007 v0.4: previously only pageType + productTitle/Type leaked into - the directive; agents had to make up everything else (price, vendor, - availability). Now every populated page_context field is forwarded so - the agent can lean on real facts before reaching for a tool call. - - PRD-007 addendum (post PRD-009 Layer 2): when ``related_products`` is - supplied — top FBT pair, top in-collection sibling, top vendor sibling - — they're appended as facts the agent can weave into the opener. The - agent is instructed to lead with FBT signal when present (real - customer co-purchase pattern, highest leverage) and fall back to the - catalog siblings otherwise. Always real data; never invented. - """ - parts: list[str] = [] - for src_key, label in _OPENER_CONTEXT_FIELDS: - rendered = _format_opener_context_value(label, page_context.get(src_key)) - if rendered is not None: - parts.append(rendered) - summary = ", ".join(parts) if parts else "no context" - - related_block = "" - if related_products: - # Render each related product as a one-line fact with provenance, - # so the agent can cite naturally (e.g. "often bought with X — 12 of - # 57 orders"). Order matters: FBT first (strongest signal), then - # collection / vendor siblings as fall-backs. - rel_order = { - "frequently_bought_with": 0, - "in_collection": 1, - "by_vendor": 2, - } - sorted_rel = sorted( - related_products, - key=lambda p: rel_order.get(p.get("relation", ""), 99), - ) - rendered_rel = [] - for p in sorted_rel: - label = p.get("label", "?") - rel = p.get("relation", "") - if rel == "frequently_bought_with" and p.get("co_count"): - rendered_rel.append( - f'"{label}" (bought together in {p["co_count"]} ' - f'of {p.get("total_orders", "?")} orders)' - ) - elif rel == "in_collection": - rendered_rel.append(f'"{label}" (same collection)') - elif rel == "by_vendor": - rendered_rel.append(f'"{label}" (same vendor)') - else: - rendered_rel.append(f'"{label}"') - related_block = ( - " Related from order/catalog graph (use these naturally — " - "prefer the order-pair signal when present, else mention the " - "collection/vendor sibling as a starter for conversation): " - + "; ".join(rendered_rel) - ) - - return ( - "[PROACTIVE_OPENER] Generate a contextual one-sentence opener " - "(≤140 chars). RETURN PLAIN TEXT ONLY — no tool calls, no JSON, " - "no markdown, no greetings. Use the facts below as your source of " - "truth — do NOT invent specs, compatibility, or pricing the context " - "doesn't include. If a fact you'd want isn't here, ask a question " - "instead of fabricating. " - f"Context: {summary}.{related_block}" - ) - - -def _build_cart_idle_opener_message( - page_context: dict, - recommendations: Optional[list] = None, -) -> str: - """Synthesize the directive for a cart-idle proactive popup. - - PRD-008-B Feature C2: a shopper has been idle on the cart page for - `idle_seconds`. We want a graph-grounded nudge that references real - FBT pairings — "customers who bought your stuff also added X" — not - a generic "still there?" line. - - When ``recommendations`` is empty (no graph, cold start, or no FBT - signal for cart items), the directive still produces a contextual - nudge based on cart size/total — never fabricates products. - """ - cart_count = page_context.get("cartItemCount") or 0 - cart_total = page_context.get("cartTotalPrice") - currency = page_context.get("shopCurrency") or "" - - cart_summary_parts: list[str] = [] - if cart_count: - cart_summary_parts.append(f"cart_item_count={cart_count}") - if cart_total: - # Shopify amounts are minor units (cents/pence). Render as e.g. "362.18 GBP". - try: - major = float(cart_total) / 100.0 - cart_summary_parts.append(f"cart_total={major:.2f} {currency}".strip()) - except (TypeError, ValueError): - cart_summary_parts.append(f"cart_total={cart_total}") - cart_summary = ", ".join(cart_summary_parts) if cart_summary_parts else "cart_idle" - - rec_block = "" - if recommendations: - rendered = [] - for r in recommendations: - label = r.get("label", "?") - paired = r.get("paired_with_count", 0) - if paired > 1: - rendered.append( - f'"{label}" (bought with {paired} of the items in this cart)' - ) - elif r.get("score") and r.get("total_orders"): - rendered.append( - f'"{label}" (added together in {r["score"]} ' - f'of {r["total_orders"]} orders)' - ) - else: - rendered.append(f'"{label}"') - rec_block = ( - " Frequently bought with what's in this cart (real order-graph " - "data — pick ONE to mention, prefer the one paired with multiple " - "cart items): " - + "; ".join(rendered) - ) - - return ( - "[PROACTIVE_OPENER] [CART_IDLE] The shopper has been idle on the cart " - "page. Generate a single helpful sentence (≤140 chars) that nudges " - "them toward checkout OR offers a relevant add-on. RETURN PLAIN TEXT " - "ONLY — no tool calls, no markdown, no greetings. Do NOT invent " - "products that aren't named below. If no recommendation is provided, " - "ask if they need help finishing their order — don't fabricate. " - f"Context: {cart_summary}.{rec_block}" - ) - - class WidgetMessageOut(BaseModel): id: str role: str diff --git a/orchestrator/integrations/shopify/context_fields.py b/orchestrator/integrations/shopify/context_fields.py index 906fea338..f8c2493eb 100644 --- a/orchestrator/integrations/shopify/context_fields.py +++ b/orchestrator/integrations/shopify/context_fields.py @@ -13,9 +13,10 @@ that contain whitespace or embedded quotes. These were extracted verbatim from chat.py with no value or behaviour -changes. ``_build_proactive_opener_message`` (still in chat.py through -US-008) imports both from here so its rendered output is byte-equal to -the pre-lift baseline captured in +changes. ``_build_proactive_opener_message`` (lifted to +``integrations/shopify/widget_proactive.py`` in US-008) imports both +from here so its rendered output is byte-equal to the pre-lift +baseline captured in ``orchestrator/integrations/shopify/tests/fixtures/``. Why this module exists at all: chat.py is the generic widget-chat diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py index d6c3ab6ce..7225c4cbc 100644 --- a/orchestrator/integrations/shopify/tests/conftest.py +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -1,6 +1,6 @@ """Shared test fixtures for the Shopify widget plugin tests. -PRD-141 US-011 — the snapshot equivalence tests need three things on top +PRD-141 US-011 — the snapshot equivalence tests need two things on top of the per-test setup already in ``test_widget_proactive.py``: 1. The US-004 fixture files (synthetic INBUILD graph, two page contexts, @@ -8,16 +8,15 @@ 2. A ``GraphifyService`` stub whose ``load_graph`` returns the fixture graph regardless of workspace id — the lifted resolvers look it up lazily so we have to inject it before the helpers run. -3. (Through Phase 1 only) a fake ``api.widgets.chat`` module exposing - the four proactive helpers the US-003 shim still imports. We - AST-extract them from real ``chat.py`` so the test exercises the - actual production source — no separate fixture copy that could - silently drift. -As US-006/007/008 move the helpers into ``integrations/shopify/ -widget_proactive.py``, the ``api.widgets.chat`` injection in -``real_chat_with_graph`` should be tightened (or eventually removed) -in lockstep. The ``GraphifyService`` stub is needed in every phase. +After PRD-141 US-008 every proactive helper +(``_resolve_graph_related_products``, ``_resolve_cart_recommendations``, +``_build_proactive_opener_message``, ``_build_cart_idle_opener_message``) +lives in :mod:`integrations.shopify.widget_proactive`. The fixture +imports them directly — no more AST extraction from chat.py, and no +more ``api.widgets.chat`` injection into ``sys.modules`` (the shim no +longer reaches back into chat.py for anything). Only the +``GraphifyService`` stub is still needed. Synthetic-fixture caveat: the graph is hand-crafted to exercise the same code branches the production INBUILD data exercises. The @@ -28,13 +27,10 @@ from __future__ import annotations -import ast import json -import logging import sys import types from pathlib import Path -from typing import Optional import networkx as nx import pytest @@ -43,26 +39,6 @@ _THIS_DIR = Path(__file__).resolve().parent _FIXTURES_DIR = _THIS_DIR / "fixtures" -# fixtures/ -> tests/ -> shopify/ -> integrations/ -> orchestrator -_ORCH_ROOT = _THIS_DIR.parents[2] -_CHAT_PY = _ORCH_ROOT / "api" / "widgets" / "chat.py" - - -# Names AST-extracted from chat.py — the proactive helpers still inline -# there. ``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value`` -# were lifted to ``integrations/shopify/context_fields.py`` in -# PRD-141 US-005; they're now imported directly into the exec namespace -# (see ``_extract_chat_helpers``) so the function bodies can close over -# them without NameError. ``_resolve_graph_related_products`` was lifted -# to ``integrations/shopify/widget_proactive.py`` in PRD-141 US-006 and -# ``_resolve_cart_recommendations`` was lifted there in US-007; the -# shim's resolver paths now run locally, so neither needs AST -# extraction from chat.py. As US-008 moves the remaining builders into -# ``widget_proactive.py``, drop them from this set in lockstep. -_WANTED_NAMES = frozenset({ - "_build_proactive_opener_message", - "_build_cart_idle_opener_message", -}) @pytest.fixture(scope="session") @@ -105,107 +81,24 @@ def expected_cart_idle_opener() -> str: return (_FIXTURES_DIR / "expected_cart_idle_opener.txt").read_text() -def _extract_chat_helpers() -> dict: - """Pull the remaining proactive builders out of chat.py via AST. - - Returns a namespace dict containing: - - * ``_OPENER_CONTEXT_FIELDS`` — field-mapping tuple, imported from - ``integrations.shopify.context_fields`` (US-005 lifted it out of - chat.py; it's still required in the namespace because the AST- - extracted ``_build_proactive_opener_message`` closes over it). - * ``_format_opener_context_value`` — single-value formatter, same - story as the field mapping. - * ``_build_proactive_opener_message`` — product-page directive. - * ``_build_cart_idle_opener_message`` — cart-idle directive. - - ``_resolve_graph_related_products`` (US-006) and - ``_resolve_cart_recommendations`` (US-007) were lifted to - ``integrations.shopify.widget_proactive`` and are no longer - extracted here — the shim calls the local functions and they do - the same lazy ``GraphifyService`` import the caller - (``real_chat_with_graph``) arranges. - - Why AST instead of ``import``: chat.py is a FastAPI router that drags - in SQLAlchemy, Redis, RAG, multimodal, etc. Loading it just to read - two builders is wasteful and brittle. AST extraction reads the - source verbatim and execs only the wanted nodes into an isolated - namespace, so the test exercises identical bytes to the running - server without paying the import cost. ``context_fields`` has none - of that baggage, so we import it normally. - """ - from integrations.shopify.context_fields import ( - _OPENER_CONTEXT_FIELDS, - _format_opener_context_value, - ) - - src = _CHAT_PY.read_text() - tree = ast.parse(src) - - ns: dict = { - "Optional": Optional, - "logger": logging.getLogger("us011_snapshot"), - "__name__": "_chat_extracted_for_us011", - "_OPENER_CONTEXT_FIELDS": _OPENER_CONTEXT_FIELDS, - "_format_opener_context_value": _format_opener_context_value, - } - - for node in tree.body: - name: Optional[str] = None - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): - name = node.name - elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): - name = node.target.id - elif ( - isinstance(node, ast.Assign) - and len(node.targets) == 1 - and isinstance(node.targets[0], ast.Name) - ): - name = node.targets[0].id - - if name in _WANTED_NAMES: - module = ast.Module(body=[node], type_ignores=[]) - code = compile(module, str(_CHAT_PY), "exec") - exec(code, ns) - - missing = _WANTED_NAMES - set(ns) - if missing: - raise RuntimeError( - "chat.py is missing expected proactive builders " - f"{sorted(missing)}. If a US-008 lift moved them, update " - "conftest.py to point the AST extractor at the new location " - "(likely integrations/shopify/widget_proactive.py)." - ) - return ns - - @pytest.fixture def real_chat_with_graph(monkeypatch, inbuild_graph): - """Wire real chat.py helpers + a fixture-bound GraphifyService into sys.modules. - - The US-003 shim's ``handle_widget_message`` does lazy ``from - api.widgets.chat import ...`` at the point of the rewrite, and the - resolvers themselves do ``from modules.knowledge.graph_service - import GraphifyService``. Both have to resolve to something callable - before the test invokes ``widget_proactive.handle_widget_message``. - - This fixture sets up both, scoped to the test (monkeypatch cleans up - automatically), so the snapshot tests exercise the exact code that - runs in production — only the graph source is swapped for the - deterministic US-004 fixture. - - Through US-005/006/007/008 the helpers move into - ``integrations/shopify/widget_proactive.py`` and the shim's - ``api.widgets.chat`` imports go away. Each lift story should narrow - the injection here so it stays a faithful test environment, not a - nostalgic crutch. + """Inject a fixture-bound ``GraphifyService`` for the lifted resolvers. + + The Shopify shim's resolvers (``_resolve_graph_related_products``, + ``_resolve_cart_recommendations``) do ``from + modules.knowledge.graph_service import GraphifyService`` lazily at + call time. This fixture replaces that import with a deterministic + stub returning the US-004 fixture graph, so the snapshot tests + exercise the exact production code path with one — and only one — + side-channel: the graph source. + + Fixture name kept for git-history continuity through the Phase 1 + lift (US-005/006/007/008). After US-008 there is no longer any + chat.py injection — the shim calls local builders directly — so + "real chat" in the name is now historical. US-010's dispatch + rewire is a candidate moment to rename. """ - ns = _extract_chat_helpers() - - fake_chat = types.ModuleType("api.widgets.chat") - fake_chat._build_proactive_opener_message = ns["_build_proactive_opener_message"] - fake_chat._build_cart_idle_opener_message = ns["_build_cart_idle_opener_message"] - fake_graph_service_mod = types.ModuleType("modules.knowledge.graph_service") class _FixtureGraphifyService: @@ -226,14 +119,6 @@ async def load_graph(self, workspace_id): # noqa: D401, ARG002 sys.modules, "modules.knowledge.graph_service", fake_graph_service_mod ) - for parent in ("api", "api.widgets"): - monkeypatch.setitem( - sys.modules, parent, sys.modules.get(parent, types.ModuleType(parent)) - ) - monkeypatch.setitem(sys.modules, "api.widgets.chat", fake_chat) - return { - "chat": fake_chat, "graph_service": fake_graph_service_mod, - "namespace": ns, } diff --git a/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index 2448822ae..fc5948a80 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -9,24 +9,24 @@ ``trigger_reason in ("proactive_opener", "cart_idle")`` AND ``page_context is not None`` returns the message unchanged. * Delegation contract — when the shim DOES rewrite, it calls the - matching chat.py helper with the right arguments and returns the - builder's output verbatim as ``message``. Combined with the fact - that the chat.py builders themselves are unchanged in US-003, this - is the transitive equivalence guarantee that US-011 will then pin - byte-for-byte against captured INBUILD fixtures. - -The delegation tests inject a **fake** ``api.widgets.chat`` module -into ``sys.modules`` via ``monkeypatch`` rather than loading the real -one. The real module pulls in the entire FastAPI / RAG / multimodal -dependency tree which is overkill for a unit test of a 60-line shim. -US-011 will exercise the real path against captured production -fixtures — that is where byte-equivalence is enforced. + matching resolver + builder with the right arguments and returns the + builder's output verbatim as ``message``. After PRD-141 US-008 the + four helpers all live in :mod:`integrations.shopify.widget_proactive`, + so the delegation contract is now a pure intra-module wiring check; + US-011's snapshot tests pin the actual builder output byte-for-byte + against the US-004 INBUILD fixtures. + +The delegation tests monkey-patch the four helpers directly on +:mod:`widget_proactive` rather than running the real graph/builder +code. The real resolvers traverse the Shopify graph and the real +builders close over the context-field mapping — overkill for a unit +test of the dispatch contract. US-011 exercises the real path against +the deterministic fixture graph; that is where byte-equivalence is +enforced. """ from __future__ import annotations -import sys -import types from unittest.mock import MagicMock from uuid import uuid4 @@ -48,26 +48,26 @@ def workspace_id(): @pytest.fixture def fake_chat(monkeypatch): - """Replace the shim's downstream dependencies with fakes. + """Replace the shim's downstream helpers with sentinels. - The shim calls a mix of LOCAL and chat.py helpers depending on the - trigger path and the current state of the Phase 1 lift: + Since PRD-141 US-008 all four helpers live on + :mod:`widget_proactive` itself: - * ``_resolve_graph_related_products`` — local to ``widget_proactive`` - since PRD-141 US-006; patched in place via ``monkeypatch.setattr``. - * ``_resolve_cart_recommendations`` — local to ``widget_proactive`` - since PRD-141 US-007; patched in place via ``monkeypatch.setattr``. - * ``_build_proactive_opener_message`` / - ``_build_cart_idle_opener_message`` — still in ``api.widgets.chat`` - (until US-008); patched via a fake module in ``sys.modules``. + * ``_resolve_graph_related_products`` (local since US-006) + * ``_resolve_cart_recommendations`` (local since US-007) + * ``_build_proactive_opener_message`` (local since US-008) + * ``_build_cart_idle_opener_message`` (local since US-008) - The fake records every call so tests can assert the shim forwards + Each is patched in place via ``monkeypatch.setattr``. The fake + records every call so tests can assert the shim forwards ``workspace_id`` and ``page_context`` correctly, and returns a sentinel message so tests can assert the shim wires the builder's output into ``WidgetPluginResult.message`` unmodified. - """ - fake = types.ModuleType("api.widgets.chat") + Fixture name kept as ``fake_chat`` for git-history continuity + through the Phase 1 lift; the historical "chat" reference now means + "the proactive helpers that used to live in chat.py". + """ calls: dict[str, list] = { "resolve_products": [], "resolve_cart": [], @@ -108,10 +108,6 @@ def fake_build_cart(page_context, recommendations=None): }) return "FAKE_CART_IDLE_OPENER_MESSAGE" - fake._build_proactive_opener_message = fake_build_product - fake._build_cart_idle_opener_message = fake_build_cart - - monkeypatch.setitem(sys.modules, "api.widgets.chat", fake) monkeypatch.setattr( widget_proactive, "_resolve_graph_related_products", @@ -122,6 +118,16 @@ def fake_build_cart(page_context, recommendations=None): "_resolve_cart_recommendations", fake_resolve_cart, ) + monkeypatch.setattr( + widget_proactive, + "_build_proactive_opener_message", + fake_build_product, + ) + monkeypatch.setattr( + widget_proactive, + "_build_cart_idle_opener_message", + fake_build_cart, + ) return calls @@ -293,17 +299,18 @@ async def test_cart_idle_delegates_to_chat_helpers(fake_chat, db, workspace_id): # ---- US-011 snapshot equivalence (PRD-007 + PRD-008-B byte-equality) -------- # # These tests are the byte-equality safety net that gates every Phase 1 -# lift (US-005/006/007/008/010). At US-011 commit time the Shopify plugin -# is still the US-003 shim that delegates back into ``api.widgets.chat``; -# the ``real_chat_with_graph`` fixture in ``conftest.py`` injects the -# REAL helpers (AST-extracted from chat.py) plus a fixture-bound -# GraphifyService so the test exercises the production code path while -# remaining deterministic. +# lift (US-005/006/007/008/010). After US-008 all four proactive helpers +# live in :mod:`integrations.shopify.widget_proactive` and the snapshot +# tests call ``handle_widget_message`` directly. The +# ``real_chat_with_graph`` fixture in ``conftest.py`` provides only one +# piece of indirection — a fixture-bound ``GraphifyService`` stub that +# returns the US-004 synthetic INBUILD graph — so the test exercises +# the exact production code path while remaining deterministic. # -# Through US-005/006/007/008 the helpers progressively move into -# ``integrations/shopify/widget_proactive.py``. These tests must KEEP -# PASSING through every lift. If one fails, the lift broke equivalence — -# fix the lift, NOT the golden fixture (per US-011 notes). +# These tests must KEEP PASSING through every remaining lift (US-010 in +# particular rewires chat.py to dispatch via the registry — same shim +# entry point, same expected output). If one fails, the lift broke +# equivalence — fix the lift, NOT the golden fixture (per US-011 notes). @pytest.mark.asyncio diff --git a/orchestrator/integrations/shopify/widget_proactive.py b/orchestrator/integrations/shopify/widget_proactive.py index 68cb2bb9f..5af85895d 100644 --- a/orchestrator/integrations/shopify/widget_proactive.py +++ b/orchestrator/integrations/shopify/widget_proactive.py @@ -1,42 +1,33 @@ -"""Shopify vertical plugin — TEMPORARY shim delegating to chat.py. +"""Shopify vertical plugin — proactive opener + cart-idle directive builders. PRD-141 US-003. Registered as ``PLUGIN_REGISTRY["shopify"]`` and used by any workspace whose ``settings.vertical == "shopify"``. -This module is a **shim**, not a rewrite. It encapsulates the dispatch -contract — ``handle_widget_message`` matching the -:class:`integrations.WidgetPlugin` protocol — and delegates to the -remaining inline Shopify helpers still living in -``orchestrator/api/widgets/chat.py``: - -* ``_build_proactive_opener_message`` (product-page directive builder) -* ``_build_cart_idle_opener_message`` (cart-idle directive builder) - -The two graph resolvers (``_resolve_graph_related_products`` lifted in -US-006, ``_resolve_cart_recommendations`` lifted in US-007) now live -in this file and the shim calls them locally. - -US-008 will move the two builders here. US-010 will delete the chat.py -inline dispatch and route every widget chat request through -``PLUGIN_REGISTRY``. At that point the imports below become local -definitions and this docstring's "shim" framing goes away. - -The remaining chat.py imports happen **inside** -``handle_widget_message``, beneath the early-return gate. Two reasons: - -1. Circular-import safety. During Phase 1 there is a window where - chat.py imports back from this module (US-006/007/008 move helpers - progressively). Lazy imports avoid that window without changing - behaviour. -2. Pass-through paths must not pay the cost of loading the FastAPI - router module (which pulls in database / auth dependencies). The - gate is the hot path; the rewrite is the rare path. - -The ``PROACTIVE_TRIGGER_REASONS`` frozenset from chat.py is -intentionally NOT imported here — the two trigger strings are -hardcoded inline so the gate works without touching chat.py. The -duplication disappears in US-010 when the constant moves alongside -the helpers it gates. +The module encapsulates the dispatch contract — ``handle_widget_message`` +matching the :class:`integrations.WidgetPlugin` protocol — together with +the four Shopify-specific helpers it needs: + +* :func:`_resolve_graph_related_products` (lifted in US-006) — single-seed + FBT / collection / vendor traversal for the product-page opener. +* :func:`_resolve_cart_recommendations` (lifted in US-007) — multi-seed + FBT aggregation for the cart-idle nudge. +* :func:`_build_proactive_opener_message` (lifted in US-008) — product-page + directive builder, closes over ``_OPENER_CONTEXT_FIELDS`` and + ``_format_opener_context_value`` from :mod:`.context_fields`. +* :func:`_build_cart_idle_opener_message` (lifted in US-008) — cart-idle + directive builder; no context-field dependency. + +``orchestrator/api/widgets/chat.py`` still drives the proactive rewrite +inline through US-009/010 — it imports the four helpers from this module +to keep production behaviour unchanged until the dispatch is rewired to +``PLUGIN_REGISTRY``. After US-010 the only entry point is +``handle_widget_message`` and chat.py contains zero Shopify identifiers. + +The ``PROACTIVE_TRIGGER_REASONS`` frozenset still lives in chat.py and +is intentionally NOT imported here — the two trigger strings are +hardcoded inline so the gate works without circular imports. The +duplication disappears in US-010 when the constant moves alongside the +dispatch. """ from __future__ import annotations @@ -48,6 +39,10 @@ from sqlalchemy.orm import Session from integrations import WidgetPluginResult +from integrations.shopify.context_fields import ( + _OPENER_CONTEXT_FIELDS, + _format_opener_context_value, +) logger = logging.getLogger(__name__) @@ -241,6 +236,149 @@ async def _resolve_cart_recommendations( return [] +def _build_proactive_opener_message( + page_context: dict, + related_products: Optional[list] = None, +) -> str: + """Synthesize the user-side message for a proactive opener request. + + The widget never sends real user text for proactive openers — instead + we synthesize a directive carrying the FULL page context the agent + needs to ground a contextual one-line opener. + + PRD-007 v0.4: previously only pageType + productTitle/Type leaked into + the directive; agents had to make up everything else (price, vendor, + availability). Now every populated page_context field is forwarded so + the agent can lean on real facts before reaching for a tool call. + + PRD-007 addendum (post PRD-009 Layer 2): when ``related_products`` is + supplied — top FBT pair, top in-collection sibling, top vendor sibling + — they're appended as facts the agent can weave into the opener. The + agent is instructed to lead with FBT signal when present (real + customer co-purchase pattern, highest leverage) and fall back to the + catalog siblings otherwise. Always real data; never invented. + """ + parts: list[str] = [] + for src_key, label in _OPENER_CONTEXT_FIELDS: + rendered = _format_opener_context_value(label, page_context.get(src_key)) + if rendered is not None: + parts.append(rendered) + summary = ", ".join(parts) if parts else "no context" + + related_block = "" + if related_products: + # Render each related product as a one-line fact with provenance, + # so the agent can cite naturally (e.g. "often bought with X — 12 of + # 57 orders"). Order matters: FBT first (strongest signal), then + # collection / vendor siblings as fall-backs. + rel_order = { + "frequently_bought_with": 0, + "in_collection": 1, + "by_vendor": 2, + } + sorted_rel = sorted( + related_products, + key=lambda p: rel_order.get(p.get("relation", ""), 99), + ) + rendered_rel = [] + for p in sorted_rel: + label = p.get("label", "?") + rel = p.get("relation", "") + if rel == "frequently_bought_with" and p.get("co_count"): + rendered_rel.append( + f'"{label}" (bought together in {p["co_count"]} ' + f'of {p.get("total_orders", "?")} orders)' + ) + elif rel == "in_collection": + rendered_rel.append(f'"{label}" (same collection)') + elif rel == "by_vendor": + rendered_rel.append(f'"{label}" (same vendor)') + else: + rendered_rel.append(f'"{label}"') + related_block = ( + " Related from order/catalog graph (use these naturally — " + "prefer the order-pair signal when present, else mention the " + "collection/vendor sibling as a starter for conversation): " + + "; ".join(rendered_rel) + ) + + return ( + "[PROACTIVE_OPENER] Generate a contextual one-sentence opener " + "(≤140 chars). RETURN PLAIN TEXT ONLY — no tool calls, no JSON, " + "no markdown, no greetings. Use the facts below as your source of " + "truth — do NOT invent specs, compatibility, or pricing the context " + "doesn't include. If a fact you'd want isn't here, ask a question " + "instead of fabricating. " + f"Context: {summary}.{related_block}" + ) + + +def _build_cart_idle_opener_message( + page_context: dict, + recommendations: Optional[list] = None, +) -> str: + """Synthesize the directive for a cart-idle proactive popup. + + PRD-008-B Feature C2: a shopper has been idle on the cart page for + `idle_seconds`. We want a graph-grounded nudge that references real + FBT pairings — "customers who bought your stuff also added X" — not + a generic "still there?" line. + + When ``recommendations`` is empty (no graph, cold start, or no FBT + signal for cart items), the directive still produces a contextual + nudge based on cart size/total — never fabricates products. + """ + cart_count = page_context.get("cartItemCount") or 0 + cart_total = page_context.get("cartTotalPrice") + currency = page_context.get("shopCurrency") or "" + + cart_summary_parts: list[str] = [] + if cart_count: + cart_summary_parts.append(f"cart_item_count={cart_count}") + if cart_total: + # Shopify amounts are minor units (cents/pence). Render as e.g. "362.18 GBP". + try: + major = float(cart_total) / 100.0 + cart_summary_parts.append(f"cart_total={major:.2f} {currency}".strip()) + except (TypeError, ValueError): + cart_summary_parts.append(f"cart_total={cart_total}") + cart_summary = ", ".join(cart_summary_parts) if cart_summary_parts else "cart_idle" + + rec_block = "" + if recommendations: + rendered = [] + for r in recommendations: + label = r.get("label", "?") + paired = r.get("paired_with_count", 0) + if paired > 1: + rendered.append( + f'"{label}" (bought with {paired} of the items in this cart)' + ) + elif r.get("score") and r.get("total_orders"): + rendered.append( + f'"{label}" (added together in {r["score"]} ' + f'of {r["total_orders"]} orders)' + ) + else: + rendered.append(f'"{label}"') + rec_block = ( + " Frequently bought with what's in this cart (real order-graph " + "data — pick ONE to mention, prefer the one paired with multiple " + "cart items): " + + "; ".join(rendered) + ) + + return ( + "[PROACTIVE_OPENER] [CART_IDLE] The shopper has been idle on the cart " + "page. Generate a single helpful sentence (≤140 chars) that nudges " + "them toward checkout OR offers a relevant add-on. RETURN PLAIN TEXT " + "ONLY — no tool calls, no markdown, no greetings. Do NOT invent " + "products that aren't named below. If no recommendation is provided, " + "ask if they need help finishing their order — don't fabricate. " + f"Context: {cart_summary}.{rec_block}" + ) + + async def handle_widget_message( *, message: str, @@ -249,9 +387,9 @@ async def handle_widget_message( workspace_id: UUID, db: Session, ) -> WidgetPluginResult: - """Shim — replicates the inline chat.py proactive-rewrite block. + """Replicates the inline chat.py proactive-rewrite block byte-for-byte. - Behaviour mirrors ``api/widgets/chat.py`` byte-for-byte: + Behaviour mirrors ``api/widgets/chat.py``: * ``trigger_reason`` is ``"proactive_opener"`` or ``"cart_idle"`` (the two members of chat.py's ``PROACTIVE_TRIGGER_REASONS`` @@ -274,8 +412,6 @@ async def handle_widget_message( workspace_str = str(workspace_id) if trigger_reason == "cart_idle": - from api.widgets.chat import _build_cart_idle_opener_message - recommendations = await _resolve_cart_recommendations( workspace_str, page_context, ) @@ -292,8 +428,6 @@ async def handle_widget_message( }, ) - from api.widgets.chat import _build_proactive_opener_message - related_products = await _resolve_graph_related_products( workspace_str, page_context, ) diff --git a/orchestrator/tests/test_widget_proactive_prd007.py b/orchestrator/tests/test_widget_proactive_prd007.py index 84da5b630..c068f3301 100644 --- a/orchestrator/tests/test_widget_proactive_prd007.py +++ b/orchestrator/tests/test_widget_proactive_prd007.py @@ -111,7 +111,7 @@ def test_build_widget_config_returns_none_when_no_public_keys_set(): # --------------------------------------------------------------------------- def test_opener_message_includes_product_title_when_present(): - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "product", @@ -125,7 +125,7 @@ def test_opener_message_includes_product_title_when_present(): def test_opener_message_falls_back_to_handle_without_title(): - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "product", @@ -135,7 +135,7 @@ def test_opener_message_falls_back_to_handle_without_title(): def test_opener_message_handles_collection_pages(): - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "collection", @@ -148,7 +148,7 @@ def test_opener_message_handles_collection_pages(): def test_opener_message_includes_full_grounding_context(): """PRD-007 v0.4: rich page context grounds the agent so it stops inventing facts. Every populated field should reach the directive.""" - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "product", @@ -175,7 +175,7 @@ def test_opener_message_includes_full_grounding_context(): def test_opener_message_skips_empty_zero_false_fields(): """Don't pollute the directive with empty / zero / false signals.""" - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "product", @@ -194,7 +194,7 @@ def test_opener_message_skips_empty_zero_false_fields(): def test_opener_message_quotes_values_with_spaces(): """Multi-word values (like product titles) need quoting so the agent parses them as single tokens.""" - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({ "pageType": "product", @@ -204,7 +204,7 @@ def test_opener_message_quotes_values_with_spaces(): def test_opener_message_handles_empty_context(): - from api.widgets.chat import _build_proactive_opener_message + from integrations.shopify.widget_proactive import _build_proactive_opener_message msg = _build_proactive_opener_message({}) assert msg.startswith("[PROACTIVE_OPENER]") @@ -256,7 +256,7 @@ def test_proactive_trigger_reason_constant_includes_cart_idle(): # --------------------------------------------------------------------------- def test_cart_idle_message_includes_cart_summary(): - from api.widgets.chat import _build_cart_idle_opener_message + from integrations.shopify.widget_proactive import _build_cart_idle_opener_message msg = _build_cart_idle_opener_message( {"cartItemCount": 3, "cartTotalPrice": 14299, "shopCurrency": "GBP"}, @@ -269,7 +269,7 @@ def test_cart_idle_message_includes_cart_summary(): def test_cart_idle_message_renders_recommendations_with_provenance(): - from api.widgets.chat import _build_cart_idle_opener_message + from integrations.shopify.widget_proactive import _build_cart_idle_opener_message recs = [ {"label": "Elta 4A Actuator", "paired_with_count": 2, "score": 24, "total_orders": 87}, @@ -288,7 +288,7 @@ def test_cart_idle_message_renders_recommendations_with_provenance(): def test_cart_idle_message_falls_back_when_no_recommendations(): - from api.widgets.chat import _build_cart_idle_opener_message + from integrations.shopify.widget_proactive import _build_cart_idle_opener_message msg = _build_cart_idle_opener_message({"cartItemCount": 1}, recommendations=[]) # Directive still tells the agent to nudge toward checkout without @@ -298,7 +298,7 @@ def test_cart_idle_message_falls_back_when_no_recommendations(): def test_cart_idle_message_skips_zero_cart_total(): - from api.widgets.chat import _build_cart_idle_opener_message + from integrations.shopify.widget_proactive import _build_cart_idle_opener_message msg = _build_cart_idle_opener_message({"cartItemCount": 1}, recommendations=None) assert "cart_total" not in msg diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index e64ea29e9..0985035ab 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -20,7 +20,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-005 — Move PROACTIVE_OPENER_FIELDS constant into the Shopify integration folder - [x] US-006 — Move _resolve_graph_related_products into Shopify plugin - [x] US-007 — Move _resolve_cart_recommendations into Shopify plugin -- [ ] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin +- [x] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin - [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* - [ ] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions - [ ] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces From cbc3b1abe76d8f33541163453faec92d86d7f401 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:29:36 +0100 Subject: [PATCH 07/11] =?UTF-8?q?feat(prd-141):=20US-009=20=E2=80=94=20bac?= =?UTF-8?q?kfill=20workspace.settings.vertical=20for=20Shopify=20workspace?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New Alembic migration sets settings.vertical='shopify' on every workspace whose JSONB settings already contain shopify_domain. Idempotent (WHERE NOT settings ? 'vertical'). Downgrade only strips vertical from rows we set it on. Lands in same PR as US-010 to keep INBUILD's PRD-007 + PRD-008-B dispatch routing intact. Validated OFFLINE only: py_compile OK, `alembic upgrade … --sql` renders upgrade + downgrade cleanly, `pytest integrations/` 32/32 green, migration-reviewer agent PASS. Staging apply + rowcount verification deferred to human before merge (per US-009 AC #7). Story: scripts/ralph/prd.json US-009 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- .../20260528_backfill_workspace_vertical.py | 67 +++++++++++++++++++ scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 2 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 orchestrator/alembic/versions/20260528_backfill_workspace_vertical.py diff --git a/orchestrator/alembic/versions/20260528_backfill_workspace_vertical.py b/orchestrator/alembic/versions/20260528_backfill_workspace_vertical.py new file mode 100644 index 000000000..dfacaa539 --- /dev/null +++ b/orchestrator/alembic/versions/20260528_backfill_workspace_vertical.py @@ -0,0 +1,67 @@ +"""PRD-141 Phase 1 — backfill workspace.settings.vertical for Shopify workspaces. + +Every existing Shopify workspace (identified by the presence of +``settings.shopify_domain``) must have ``settings.vertical = 'shopify'`` +set BEFORE ``orchestrator/api/widgets/chat.py`` is rewired to dispatch by +vertical (US-010). Without this backfill, dispatch would fall through to +the generic plugin for live INBUILD traffic and break PRD-007 product-page +openers + PRD-008-B cart-idle popups. + +Verify count matches expected Shopify workspace count after upgrade: + + SELECT count(*) FROM workspaces + WHERE settings ? 'shopify_domain' + AND settings ->> 'vertical' = 'shopify'; + +should equal:: + + SELECT count(*) FROM workspaces WHERE settings ? 'shopify_domain'; + +Idempotency: the WHERE clause excludes rows already carrying a ``vertical`` +key, so a second run is a no-op (UPDATE rowcount = 0). + +Downgrade only removes the ``vertical`` key from rows we set it on (Shopify +workspaces where ``vertical = 'shopify'``); manually-set values on other +workspaces are preserved. + +Revision ID: prd141_backfill_workspace_vertical +Revises: prd008a4_channel_drivers +Create Date: 2026-05-28 +""" +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "prd141_backfill_workspace_vertical" +down_revision = "prd008a4_channel_drivers" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.execute( + sa.text( + """ + UPDATE workspaces + SET settings = jsonb_set(settings, '{vertical}', '"shopify"', true) + WHERE settings ? 'shopify_domain' + AND NOT settings ? 'vertical' + """ + ) + ) + + +def downgrade() -> None: + op.execute( + sa.text( + """ + UPDATE workspaces + SET settings = settings - 'vertical' + WHERE settings ? 'shopify_domain' + AND settings ->> 'vertical' = 'shopify' + """ + ) + ) diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index 0985035ab..a7f3e4aee 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -21,7 +21,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-006 — Move _resolve_graph_related_products into Shopify plugin - [x] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [x] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin -- [ ] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* +- [x] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* - [ ] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions - [ ] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces From 22b9c3f21222d1a596f59611bbab5ea87184d6a6 Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:40:25 +0100 Subject: [PATCH 08/11] =?UTF-8?q?feat(prd-141):=20US-010=20=E2=80=94=20rew?= =?UTF-8?q?ire=20chat.py=20to=20dispatch=20via=20PLUGIN=5FREGISTRY?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generic widget endpoint now resolves vertical from workspace.settings.vertical (default 'generic') and dispatches the message through PLUGIN_REGISTRY[vertical].handle_widget_message. The Shopify-specific helpers are no longer imported into chat.py — they stay private to integrations/shopify/. The product_context fallback in the open-callback-form SSE bridge dropped its productTitle/ productHandle reads; the LLM tool call is now the only source. Observability preserved: PROACTIVE_REWRITE and UNKNOWN_TRIGGER_REASON log lines rebuilt from plugin telemetry, now carrying vertical=. Plugin docstrings, context_note values, and test fixtures dropped "shim" wording — post-rewire the Shopify plugin owns the path end-to-end. US-009 backfill migration (cbc3b1abe) ships in the same PR for the atomic deploy required by US-010 notes. Validation: pytest integrations/ → 32/32 green (snapshot byte-equality for PRD-007 + PRD-008-B openers held). grep -rE 'productHandle|productTitle|cartItems|shopify_' orchestrator/api/widgets/chat.py orchestrator/modules/context/sections/ returns zero matches. Manual smoke test on INBUILD is deferred to the US-020 canary (human-only). Story: scripts/ralph/prd.json US-010 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- orchestrator/api/widgets/chat.py | 98 +++++++++++-------- orchestrator/integrations/shopify/__init__.py | 9 +- .../integrations/shopify/tests/conftest.py | 12 +-- .../shopify/tests/test_widget_proactive.py | 42 ++++---- .../integrations/shopify/widget_proactive.py | 68 ++++++------- scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 6 files changed, 121 insertions(+), 110 deletions(-) diff --git a/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index 6428e8312..51d88188e 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -27,12 +27,7 @@ from api.widgets.auth import WidgetAuthContext, require_permission, widget_auth from core.database.database import get_db -from integrations.shopify.widget_proactive import ( - _build_cart_idle_opener_message, - _build_proactive_opener_message, - _resolve_cart_recommendations, - _resolve_graph_related_products, -) +from integrations import PLUGIN_REGISTRY from modules.tools.widget_callback import ( WIDGET_OPEN_CALLBACK_FORM_NAME, WIDGET_SIGNAL_KEY, @@ -109,6 +104,22 @@ def _get_widget_user_id(db: Session) -> int: return result[0] +def _resolve_workspace_vertical(db: Session, workspace_id: str) -> str: + """PRD-141: read ``workspace.settings.vertical`` for plugin dispatch. + + Returns ``"generic"`` when the workspace has no row, no settings, + or no ``vertical`` field — that's the default pass-through plugin. + Same raw-SQL pattern as ``channels/sender.py`` so we don't pull the + full ORM object just to read one JSONB key. + """ + row = db.execute( + text("SELECT settings FROM workspaces WHERE id = :ws"), + {"ws": workspace_id}, + ).fetchone() + settings = row.settings if (row and isinstance(row.settings, dict)) else {} + return settings.get("vertical") or "generic" + + # --------------------------------------------------------------------------- # POST /chat — send message, get SSE stream back # --------------------------------------------------------------------------- @@ -173,55 +184,55 @@ async def widget_chat( workspace_id = str(auth.workspace_id) user_id = _get_widget_user_id(db) - # PRD-007 / PRD-008-B: rewrite ``message`` for proactive opener requests - # so the agent sees a directive, not an empty / placeholder user - # utterance from the SDK. + # PRD-141: dispatch the message through the per-workspace vertical + # plugin. Generic surfaces (this file) hold zero vertical-specific + # keys — all rewrite logic lives under ``integrations//``. + # + # ``is_proactive`` stays defined on the request shape (a known + # proactive trigger + populated context) because it controls the + # downstream LLM-call shape (force_text_only, skip_composio); the + # plugin's decision to rewrite or pass through is independent of + # whether the agent should run in opener mode. + vertical = _resolve_workspace_vertical(db, workspace_id) + plugin = PLUGIN_REGISTRY.get(vertical) + if plugin is None: + logger.warning( + "%s UNKNOWN_VERTICAL: %s — falling back to generic plugin", + log_extra, + vertical, + ) + vertical = "generic" + plugin = PLUGIN_REGISTRY["generic"] is_proactive = ( body.trigger_reason in PROACTIVE_TRIGGER_REASONS and body.page_context is not None ) + original_msg_len = len(body.message or "") + plugin_result = await plugin.handle_widget_message( + message=body.message, + page_context=body.page_context, + trigger_reason=body.trigger_reason, + workspace_id=auth.workspace_id, + db=db, + ) + body.message = plugin_result.message if is_proactive: - original_len = len(body.message or "") - pctx = body.page_context or {} - if body.trigger_reason == "cart_idle": - # PRD-008-B Feature C2: walk FBT edges across every cart line - # item, dedupe, score by cross-cutting hits, return top recs. - # Empty list ⇒ generic "anything else?" nudge with no fabricated - # products. Heavier than the product-page resolver (multi-seed - # traversal) — still bounded by 1-hop on a small graph. - recommendations = await _resolve_cart_recommendations( - workspace_id, pctx, - ) - body.message = _build_cart_idle_opener_message( - pctx, - recommendations=recommendations, - ) - related_count = len(recommendations) - else: - # PRD-007 addendum (post PRD-009 Layer 2): single-seed traversal - # — top 1 FBT pair + collection sibling + vendor sibling. - related_products = await _resolve_graph_related_products( - workspace_id, pctx, - ) - body.message = _build_proactive_opener_message( - pctx, - related_products=related_products, - ) - related_count = len(related_products) logger.info( - "%s PROACTIVE_REWRITE: trigger=%s original_msg_len=%d new_msg_len=%d related=%d new_preview=%s", + "%s PROACTIVE_REWRITE: vertical=%s trigger=%s original_msg_len=%d new_msg_len=%d telemetry=%s new_preview=%s", log_extra, + vertical, body.trigger_reason, - original_len, + original_msg_len, len(body.message), - related_count, + plugin_result.telemetry, _short(body.message), ) elif body.trigger_reason: logger.warning( - "%s UNKNOWN_TRIGGER_REASON: %s (page_context=%s) — proceeding as normal chat", + "%s UNKNOWN_TRIGGER_REASON: %s vertical=%s (page_context=%s) — proceeding as normal chat", log_extra, body.trigger_reason, + vertical, "present" if body.page_context else "missing", ) @@ -460,11 +471,14 @@ def _emit_open_callback_form(product_context: Optional[str]) -> str: not callback_form_emitted and inner.get(WIDGET_SIGNAL_KEY) == WIDGET_SIGNAL_OPEN_CALLBACK_FORM ): + # PRD-141: ``product_context`` is sourced from + # the LLM tool call (tool-data first, tool-start + # input args second). The previous Shopify-key + # fallback on ``body.page_context`` is gone — + # generic surfaces hold no vertical knowledge. product_context = ( inner.get("product_context") or callback_form_args.get("product_context") - or (body.page_context or {}).get("productTitle") - or (body.page_context or {}).get("productHandle") ) yield _emit_open_callback_form(product_context) callback_form_emitted = True diff --git a/orchestrator/integrations/shopify/__init__.py b/orchestrator/integrations/shopify/__init__.py index e4fb4c696..be5ed1d6b 100644 --- a/orchestrator/integrations/shopify/__init__.py +++ b/orchestrator/integrations/shopify/__init__.py @@ -6,10 +6,11 @@ calls — its module-level ``handle_widget_message`` satisfies the :class:`integrations.WidgetPlugin` protocol structurally. -US-003 ships this as a SHIM that delegates to existing chat.py -helpers. US-006/007/008 move those helpers into this folder and US-010 -deletes the shim layer. See ``widget_proactive.py`` for the current -behaviour and migration story. PRD-141 US-003. +After PRD-141 US-010 the plugin owns the full proactive opener and +cart-idle path end-to-end: resolvers, builders, and message dispatch. +``orchestrator/api/widgets/chat.py`` calls this module only through +``PLUGIN_REGISTRY["shopify"].handle_widget_message`` — no direct +imports of the underlying helpers from outside this package. """ from __future__ import annotations diff --git a/orchestrator/integrations/shopify/tests/conftest.py b/orchestrator/integrations/shopify/tests/conftest.py index 7225c4cbc..3b62bf812 100644 --- a/orchestrator/integrations/shopify/tests/conftest.py +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -14,8 +14,7 @@ ``_build_proactive_opener_message``, ``_build_cart_idle_opener_message``) lives in :mod:`integrations.shopify.widget_proactive`. The fixture imports them directly — no more AST extraction from chat.py, and no -more ``api.widgets.chat`` injection into ``sys.modules`` (the shim no -longer reaches back into chat.py for anything). Only the +more ``api.widgets.chat`` injection into ``sys.modules``. Only the ``GraphifyService`` stub is still needed. Synthetic-fixture caveat: the graph is hand-crafted to exercise the @@ -85,7 +84,7 @@ def expected_cart_idle_opener() -> str: def real_chat_with_graph(monkeypatch, inbuild_graph): """Inject a fixture-bound ``GraphifyService`` for the lifted resolvers. - The Shopify shim's resolvers (``_resolve_graph_related_products``, + The Shopify plugin's resolvers (``_resolve_graph_related_products``, ``_resolve_cart_recommendations``) do ``from modules.knowledge.graph_service import GraphifyService`` lazily at call time. This fixture replaces that import with a deterministic @@ -94,10 +93,9 @@ def real_chat_with_graph(monkeypatch, inbuild_graph): side-channel: the graph source. Fixture name kept for git-history continuity through the Phase 1 - lift (US-005/006/007/008). After US-008 there is no longer any - chat.py injection — the shim calls local builders directly — so - "real chat" in the name is now historical. US-010's dispatch - rewire is a candidate moment to rename. + lift (US-005/006/007/008/010). After US-008 there is no longer any + chat.py injection — the plugin calls local builders directly — so + "real chat" in the name is now historical. """ fake_graph_service_mod = types.ModuleType("modules.knowledge.graph_service") diff --git a/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index fc5948a80..e90ccd9f5 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -1,22 +1,21 @@ -"""PRD-141 US-003 — Shopify shim plugin behaviour. +"""PRD-141 — Shopify plugin behaviour. This test file pins three things: * Registration — importing ``integrations`` populates ``PLUGIN_REGISTRY["shopify"]`` with this module. -* Pass-through gating — the shim matches the existing chat.py +* Pass-through gating — the plugin matches chat.py's old ``is_proactive`` guard: anything other than ``trigger_reason in ("proactive_opener", "cart_idle")`` AND ``page_context is not None`` returns the message unchanged. -* Delegation contract — when the shim DOES rewrite, it calls the +* Wiring contract — when the plugin DOES rewrite, it calls the matching resolver + builder with the right arguments and returns the - builder's output verbatim as ``message``. After PRD-141 US-008 the - four helpers all live in :mod:`integrations.shopify.widget_proactive`, - so the delegation contract is now a pure intra-module wiring check; - US-011's snapshot tests pin the actual builder output byte-for-byte - against the US-004 INBUILD fixtures. + builder's output verbatim as ``message``. All four helpers live in + :mod:`integrations.shopify.widget_proactive`, so the wiring contract + is a pure intra-module check; US-011's snapshot tests pin the actual + builder output byte-for-byte against the US-004 INBUILD fixtures. -The delegation tests monkey-patch the four helpers directly on +The wiring tests monkey-patch the four helpers directly on :mod:`widget_proactive` rather than running the real graph/builder code. The real resolvers traverse the Shopify graph and the real builders close over the context-field mapping — overkill for a unit @@ -48,10 +47,9 @@ def workspace_id(): @pytest.fixture def fake_chat(monkeypatch): - """Replace the shim's downstream helpers with sentinels. + """Replace the plugin's downstream helpers with sentinels. - Since PRD-141 US-008 all four helpers live on - :mod:`widget_proactive` itself: + All four helpers live on :mod:`widget_proactive` itself: * ``_resolve_graph_related_products`` (local since US-006) * ``_resolve_cart_recommendations`` (local since US-007) @@ -59,9 +57,9 @@ def fake_chat(monkeypatch): * ``_build_cart_idle_opener_message`` (local since US-008) Each is patched in place via ``monkeypatch.setattr``. The fake - records every call so tests can assert the shim forwards + records every call so tests can assert the plugin forwards ``workspace_id`` and ``page_context`` correctly, and returns a - sentinel message so tests can assert the shim wires the builder's + sentinel message so tests can assert the plugin wires the builder's output into ``WidgetPluginResult.message`` unmodified. Fixture name kept as ``fake_chat`` for git-history continuity @@ -249,7 +247,7 @@ async def test_proactive_opener_delegates_to_chat_helpers(fake_chat, db, workspa # 4. Result wires the builder output verbatim into message. assert result.message == "FAKE_PRODUCT_OPENER_MESSAGE" - assert result.context_note == "shopify shim: proactive_opener rewrite" + assert result.context_note == "shopify: proactive_opener rewrite" assert result.telemetry == { "trigger_reason": "proactive_opener", "related_count": 1, @@ -289,7 +287,7 @@ async def test_cart_idle_delegates_to_chat_helpers(fake_chat, db, workspace_id): assert fake_chat["build_product"] == [] assert result.message == "FAKE_CART_IDLE_OPENER_MESSAGE" - assert result.context_note == "shopify shim: cart_idle rewrite" + assert result.context_note == "shopify: cart_idle rewrite" assert result.telemetry == { "trigger_reason": "cart_idle", "related_count": 2, @@ -308,7 +306,7 @@ async def test_cart_idle_delegates_to_chat_helpers(fake_chat, db, workspace_id): # the exact production code path while remaining deterministic. # # These tests must KEEP PASSING through every remaining lift (US-010 in -# particular rewires chat.py to dispatch via the registry — same shim +# particular rewires chat.py to dispatch via the registry — same plugin # entry point, same expected output). If one fails, the lift broke # equivalence — fix the lift, NOT the golden fixture (per US-011 notes). @@ -323,7 +321,7 @@ async def test_product_page_opener_byte_equality( ): """PRD-007 product-page opener — byte-equal to the US-004 fixture. - Exercises the proactive_opener path end-to-end: shim gates on + Exercises the proactive_opener path end-to-end: plugin gates on (trigger_reason, page_context), calls ``_resolve_graph_related_products`` against the fixture graph, calls ``_build_proactive_opener_message``, returns the directive as ``result.message``. @@ -342,7 +340,7 @@ async def test_product_page_opener_byte_equality( ) assert result.message == expected_product_page_opener - assert result.context_note == "shopify shim: proactive_opener rewrite" + assert result.context_note == "shopify: proactive_opener rewrite" @pytest.mark.asyncio @@ -375,7 +373,7 @@ async def test_cart_idle_opener_byte_equality( ) assert result.message == expected_cart_idle_opener - assert result.context_note == "shopify shim: cart_idle rewrite" + assert result.context_note == "shopify: cart_idle rewrite" @pytest.mark.parametrize( @@ -388,9 +386,9 @@ async def test_no_context_no_rewrite(trigger, db, workspace_id): """US-011 AC test 4: page_context=None ⇒ message returned unchanged. Parametrised across every trigger value (including ``None`` and an - unknown trigger) because the shim's gate is symmetric — either side + unknown trigger) because the plugin's gate is symmetric — either side short-circuits the rewrite. This is a regression guard for the - chat.py ``is_proactive`` semantics replicated in the shim. + chat.py ``is_proactive`` semantics now owned by the plugin. """ result = await widget_proactive.handle_widget_message( message="user message verbatim", diff --git a/orchestrator/integrations/shopify/widget_proactive.py b/orchestrator/integrations/shopify/widget_proactive.py index 5af85895d..f58782344 100644 --- a/orchestrator/integrations/shopify/widget_proactive.py +++ b/orchestrator/integrations/shopify/widget_proactive.py @@ -1,33 +1,33 @@ """Shopify vertical plugin — proactive opener + cart-idle directive builders. -PRD-141 US-003. Registered as ``PLUGIN_REGISTRY["shopify"]`` and used by -any workspace whose ``settings.vertical == "shopify"``. +PRD-141. Registered as ``PLUGIN_REGISTRY["shopify"]`` and used by any +workspace whose ``settings.vertical == "shopify"``. The module encapsulates the dispatch contract — ``handle_widget_message`` matching the :class:`integrations.WidgetPlugin` protocol — together with the four Shopify-specific helpers it needs: -* :func:`_resolve_graph_related_products` (lifted in US-006) — single-seed - FBT / collection / vendor traversal for the product-page opener. -* :func:`_resolve_cart_recommendations` (lifted in US-007) — multi-seed - FBT aggregation for the cart-idle nudge. -* :func:`_build_proactive_opener_message` (lifted in US-008) — product-page - directive builder, closes over ``_OPENER_CONTEXT_FIELDS`` and +* :func:`_resolve_graph_related_products` — single-seed FBT / collection / + vendor traversal for the product-page opener. +* :func:`_resolve_cart_recommendations` — multi-seed FBT aggregation for + the cart-idle nudge. +* :func:`_build_proactive_opener_message` — product-page directive + builder, closes over ``_OPENER_CONTEXT_FIELDS`` and ``_format_opener_context_value`` from :mod:`.context_fields`. -* :func:`_build_cart_idle_opener_message` (lifted in US-008) — cart-idle - directive builder; no context-field dependency. - -``orchestrator/api/widgets/chat.py`` still drives the proactive rewrite -inline through US-009/010 — it imports the four helpers from this module -to keep production behaviour unchanged until the dispatch is rewired to -``PLUGIN_REGISTRY``. After US-010 the only entry point is -``handle_widget_message`` and chat.py contains zero Shopify identifiers. - -The ``PROACTIVE_TRIGGER_REASONS`` frozenset still lives in chat.py and -is intentionally NOT imported here — the two trigger strings are -hardcoded inline so the gate works without circular imports. The -duplication disappears in US-010 when the constant moves alongside the -dispatch. +* :func:`_build_cart_idle_opener_message` — cart-idle directive builder; + no context-field dependency. + +After PRD-141 US-010, ``orchestrator/api/widgets/chat.py`` calls this +module only through ``PLUGIN_REGISTRY["shopify"].handle_widget_message`` +— there are no direct imports of the underlying helpers from outside +this package, and chat.py contains zero Shopify identifiers. + +The two proactive trigger strings (``proactive_opener``, ``cart_idle``) +are hardcoded inline below for the gate check. They mirror the +``PROACTIVE_TRIGGER_REASONS`` frozenset in chat.py, which controls the +generic LLM-call shape (text-only, no composio) and is deliberately +kept on the generic side: a barbershop opener would use the same +``proactive_opener`` value to flip the agent into opener mode. """ from __future__ import annotations @@ -387,24 +387,24 @@ async def handle_widget_message( workspace_id: UUID, db: Session, ) -> WidgetPluginResult: - """Replicates the inline chat.py proactive-rewrite block byte-for-byte. + """Build the Shopify proactive opener / cart-idle directive. - Behaviour mirrors ``api/widgets/chat.py``: + Behaviour: * ``trigger_reason`` is ``"proactive_opener"`` or ``"cart_idle"`` - (the two members of chat.py's ``PROACTIVE_TRIGGER_REASONS`` - frozenset) AND ``page_context`` is not ``None`` → call the - matching resolver + builder and return the rewritten directive - as ``message``. + (mirrors chat.py's ``PROACTIVE_TRIGGER_REASONS`` frozenset) AND + ``page_context`` is not ``None`` → call the matching resolver + + builder and return the rewritten directive as ``message``. * any other case (no trigger, unknown trigger, missing context) → return ``message`` unchanged. This includes mid-conversation messages: the Shopify vertical does NOT prepend an opaque - ``(Context: ...)`` block today; that behaviour is owned by the + ``(Context: ...)`` block — that behaviour is owned by the generic plugin only. - ``telemetry`` carries the same counts chat.py's current - ``PROACTIVE_REWRITE`` log line captures so US-010 can rebuild that - log line from the plugin result without losing observability. + ``telemetry`` carries the counts chat.py's ``PROACTIVE_REWRITE`` + log line surfaces (``trigger_reason``, ``related_count``) so the + dispatcher rebuilds the log line from the plugin result without + losing observability. """ if page_context is None or trigger_reason not in ("proactive_opener", "cart_idle"): return WidgetPluginResult(message=message) @@ -421,7 +421,7 @@ async def handle_widget_message( ) return WidgetPluginResult( message=rewritten, - context_note="shopify shim: cart_idle rewrite", + context_note="shopify: cart_idle rewrite", telemetry={ "trigger_reason": trigger_reason, "related_count": len(recommendations), @@ -437,7 +437,7 @@ async def handle_widget_message( ) return WidgetPluginResult( message=rewritten, - context_note="shopify shim: proactive_opener rewrite", + context_note="shopify: proactive_opener rewrite", telemetry={ "trigger_reason": trigger_reason, "related_count": len(related_products), diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index a7f3e4aee..366ba71d1 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -22,7 +22,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-007 — Move _resolve_cart_recommendations into Shopify plugin - [x] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin - [x] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* -- [ ] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions +- [x] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions - [ ] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces ## Phase 2 — SDK sends page_context on regular messages *(automatos-widget-sdk repo)* From 40497ae2030a2445d717c2a0f23ea17ba77440ea Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:47:30 +0100 Subject: [PATCH 09/11] =?UTF-8?q?feat(prd-141):=20US-012=20=E2=80=94=20CI?= =?UTF-8?q?=20grep=20gate=20for=20Shopify-isolation=20in=20generic=20surfa?= =?UTF-8?q?ces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds scripts/ci/check-no-shopify-in-generic.sh (POSIX ERE, two-pass: forbidden field keys + shopify_ prefix with plugin/integration/sync allowlist) and wires it into a minimal .github/workflows/ check-shopify-isolation.yml that runs on push to main/ralph and on PRs to main. Behaviourally verified offline: clean tree passes, injected productHandle fails Pass A, injected shopify_widget_id fails Pass B, and shopify_plugin/integration/sync pass through the allowlist. The 32 US-011 integration tests stay green. Story: scripts/ralph/prd.json US-012 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- .github/workflows/check-shopify-isolation.yml | 26 ++++++ scripts/ci/check-no-shopify-in-generic.sh | 89 +++++++++++++++++++ scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 3 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/check-shopify-isolation.yml create mode 100755 scripts/ci/check-no-shopify-in-generic.sh diff --git a/.github/workflows/check-shopify-isolation.yml b/.github/workflows/check-shopify-isolation.yml new file mode 100644 index 000000000..40dae5440 --- /dev/null +++ b/.github/workflows/check-shopify-isolation.yml @@ -0,0 +1,26 @@ +name: check-shopify-isolation + +# PRD-141 §12 Integration Coupling Rules — CI gate that prevents +# Shopify-specific identifiers from leaking into generic widget / +# context / chatbot surfaces. See +# scripts/ci/check-no-shopify-in-generic.sh for the gated paths and +# allowlist rules. + +on: + push: + branches: + - main + - "ralph/**" + pull_request: + branches: + - main + +jobs: + check-shopify-isolation: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Run Shopify-isolation gate + run: bash scripts/ci/check-no-shopify-in-generic.sh diff --git a/scripts/ci/check-no-shopify-in-generic.sh b/scripts/ci/check-no-shopify-in-generic.sh new file mode 100755 index 000000000..d28b45e3a --- /dev/null +++ b/scripts/ci/check-no-shopify-in-generic.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# scripts/ci/check-no-shopify-in-generic.sh +# +# PRD-141 §12 Integration Coupling Rules — CI gate. +# +# Generic widget / context / chatbot surfaces MUST NOT contain Shopify- +# specific identifiers. Vertical-specific code lives under +# orchestrator/integrations// and is reachable only via the +# PLUGIN_REGISTRY exposed by orchestrator/integrations/__init__.py. +# +# Two passes over the gated paths (POSIX ERE — no PCRE lookahead so the +# gate runs on macOS bsd-grep and GNU grep identically): +# +# Pass A — forbid Shopify field keys +# (productHandle, productTitle, cartItems, cartItemCount, +# cartTotalPrice, onlineStoreUrl) +# +# Pass B — forbid the 'shopify_' prefix, allowing only the three +# legitimate platform forms used for integration wiring: +# shopify_plugin +# shopify_integration +# shopify_sync +# +# Either pass producing a hit fails CI with the offending file:line so +# the reviewer can see exactly what needs to move into integrations/. +# +# Excluded by design (not in the gated path list — kept here for the +# next engineer who wonders why these directories aren't scanned): +# - orchestrator/integrations/ — vertical-specific code lives here +# - orchestrator/api/shopify.py — catalog sync, pre-PRD-141 surface +# - graph_extraction.py — map_shopify_catalog, outside scope +# +# Reference: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md §12 + +set -u + +GATED_PATHS=( + "orchestrator/api/widgets/" + "orchestrator/modules/context/" + "orchestrator/modules/knowledge/graph_service.py" + "orchestrator/consumers/chatbot/" +) + +PASS_A_PATTERN='productHandle|productTitle|cartItems|cartItemCount|cartTotalPrice|onlineStoreUrl' +PASS_B_PATTERN='shopify_' +PASS_B_ALLOWLIST='shopify_(plugin|integration|sync)' + +REPO_ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +cd "$REPO_ROOT" + +EXISTING_PATHS=() +for p in "${GATED_PATHS[@]}"; do + if [ -e "$p" ]; then + EXISTING_PATHS+=("$p") + else + echo "WARN: gated path missing, skipping: $p" >&2 + fi +done + +if [ ${#EXISTING_PATHS[@]} -eq 0 ]; then + echo "ERROR: no gated paths found — check repo layout" >&2 + exit 2 +fi + +EXIT=0 + +PASS_A=$(grep -rnE "$PASS_A_PATTERN" "${EXISTING_PATHS[@]}" 2>/dev/null || true) +if [ -n "$PASS_A" ]; then + echo "FAIL (Pass A) — forbidden Shopify field key in generic surface:" + echo "$PASS_A" + echo + EXIT=1 +fi + +PASS_B=$(grep -rnE "$PASS_B_PATTERN" "${EXISTING_PATHS[@]}" 2>/dev/null \ + | grep -vE "$PASS_B_ALLOWLIST" || true) +if [ -n "$PASS_B" ]; then + echo "FAIL (Pass B) — forbidden 'shopify_' identifier in generic surface:" + echo "(allowed forms: shopify_plugin, shopify_integration, shopify_sync)" + echo "$PASS_B" + echo + EXIT=1 +fi + +if [ $EXIT -eq 0 ]; then + echo "OK — no Shopify identifiers in generic surfaces." +fi + +exit $EXIT diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index 366ba71d1..62ba6c62c 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -23,7 +23,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc - [x] US-008 — Move _build_proactive_opener_message and _build_cart_idle_opener_message into Shopify plugin - [x] US-009 — Add Alembic migration backfilling workspace.settings.vertical for Shopify workspaces *(staging apply deferred to human; Ralph validates offline only)* - [x] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions -- [ ] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces +- [x] US-012 — Add CI grep gate enforcing no Shopify keys in generic surfaces ## Phase 2 — SDK sends page_context on regular messages *(automatos-widget-sdk repo)* From 778c3d688053e2da828b45643ff38278e9538e1b Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:50:45 +0100 Subject: [PATCH 10/11] =?UTF-8?q?feat(prd-141):=20US-018=20=E2=80=94=20doc?= =?UTF-8?q?s/integrations/README.md=20vertical-add=20runbook?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the step-by-step guide a future engineer follows to ship vertical #2 without touching generic surfaces. Covers what a vertical is, when to add one, the copy-paste-modify recipe (with a ~30-line minimal plugin), the §12 MAY-NOT rules, snapshot + grep-gate test requirements, where admin UI goes, and skill registration. Points at integrations/shopify/widget_proactive.py as the working reference. Story: scripts/ralph/prd.json US-018 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md --- docs/integrations/README.md | 269 ++++++++++++++++++++ scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 2 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 docs/integrations/README.md diff --git a/docs/integrations/README.md b/docs/integrations/README.md new file mode 100644 index 000000000..87c088a69 --- /dev/null +++ b/docs/integrations/README.md @@ -0,0 +1,269 @@ +# Vertical Integrations — How to Add a New One + +> Authoritative reference for the coupling rules: **[PRD-141 §12 — Integration Coupling Rules](../PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md#12-integration-coupling-rules-the-principle-written-down)**. This README is the runbook; §12 is the law. + +This guide is for the engineer who needs to ship vertical #2 (and #3, and #N) without touching a single line of generic widget code. If you find yourself editing `orchestrator/api/widgets/chat.py` to make your vertical work, you have drifted off the path — stop and re-read §12. + +--- + +## (a) What is a vertical integration? + +A **vertical** is a label on a workspace that tells the widget chat dispatcher which Python plugin should rewrite the user's message before it reaches the streaming agent. It is stored at `workspace.settings["vertical"]` and read by `orchestrator/api/widgets/chat.py` on every widget message. + +- `"shopify"` — the working reference. See `orchestrator/integrations/shopify/widget_proactive.py`. +- `"generic"` — the default. No vertical-specific key reads; JSON-formats whatever `page_context` arrives and prepends it to the message. See `orchestrator/integrations/generic/widget_proactive.py`. +- `""` — what this doc helps you build. + +A vertical plugin is a Python module implementing the `WidgetPlugin` protocol from `orchestrator/integrations/__init__.py`: + +```python +async def handle_widget_message( + *, + message: str, + page_context: Optional[dict], + trigger_reason: Optional[str], + workspace_id: UUID, + db: Session, +) -> WidgetPluginResult: ... +``` + +`WidgetPluginResult` is a small dataclass — `message` (the possibly-rewritten user message), `context_note` (optional short string for logs), `telemetry` (dict the dispatcher attaches to its structured log line). + +The dispatcher in `chat.py` looks up your plugin by the workspace's vertical string and calls `handle_widget_message`. Whatever you return as `result.message` is what the agent sees. Everything else about the request stays generic. + +--- + +## (b) When to add a new vertical + +Add a new vertical when **all** of the following are true: + +1. **There is a real partner integration behind it.** Shopify, Stripe, HubSpot, Calendly — something with its own catalog, page shapes, or proactive triggers. Do not add a vertical to express a tone-of-voice difference or a single skill swap; those go in the skill catalog. +2. **The host site sends vertical-specific `page_context` shapes** that the generic JSON pass-through can't usefully exploit. If the agent skill can read whatever generic dict arrives and do the right thing, you don't need a plugin — you need a better skill. +3. **You need to rewrite the message before the agent sees it** — typically because a proactive trigger fires (`proactive_opener`, `cart_idle`, or your vertical's new trigger) and the directive needs to embed graph-resolved entities the agent can't fetch on its own. +4. **The behaviour is one-vertical-per-workspace.** PRD-141 explicitly defers multi-vertical workspaces. If your use case is "the same workspace runs Shopify AND bookings", stop and design the multi-vertical PRD — don't smuggle two verticals into one plugin. + +If only (1) and (2) hold, write a generic-but-tagged skill and let the generic pass-through plugin do its job. If (3) holds, you need a plugin. + +--- + +## (c) Step-by-step: add a new vertical + +The mechanical recipe. Replace `` with your vertical's slug (lower-case, no hyphens — e.g. `barbershop`, `stripe`, `calendly`). + +### Step 1 — Copy the generic plugin as your starting point + +```bash +cp -r orchestrator/integrations/generic orchestrator/integrations/ +``` + +You now have: + +``` +orchestrator/integrations// +├── __init__.py # registers PLUGIN_REGISTRY[""] +├── widget_proactive.py # your handle_widget_message lives here +└── tests/ + └── test_widget_proactive.py +``` + +### Step 2 — Update `__init__.py` to self-register your slug + +```python +""" widget plugin package — see widget_proactive.py.""" + +from __future__ import annotations + +from integrations import PLUGIN_REGISTRY + +from . import widget_proactive + +PLUGIN_REGISTRY[""] = widget_proactive + +__all__ = ["widget_proactive"] +``` + +### Step 3 — Wire your package into the top-level registry + +Edit `orchestrator/integrations/__init__.py` and add one line at the bottom alongside the existing `generic` and `shopify` imports: + +```python +from . import generic # noqa: E402,F401 (registers "generic") +from . import shopify # noqa: E402,F401 (registers "shopify") +from . import # noqa: E402,F401 (registers "") +``` + +That's the only edit to a shared file in the whole flow. + +### Step 4 — Implement `handle_widget_message` in `widget_proactive.py` + +Strip the generic JSON-prefix logic and write what your vertical actually needs. The minimal plugin below is ~30 lines and is complete enough to copy-paste-modify: + +```python +""" vertical plugin — proactive opener for page shapes. + +Registered as PLUGIN_REGISTRY[""]. See docs/integrations/README.md +for the recipe; see integrations/shopify/widget_proactive.py for a +working multi-trigger reference. +""" + +from __future__ import annotations + +import logging +from typing import Optional +from uuid import UUID + +from sqlalchemy.orm import Session + +from integrations import WidgetPluginResult + +logger = logging.getLogger(__name__) + + +async def handle_widget_message( + *, + message: str, + page_context: Optional[dict], + trigger_reason: Optional[str], + workspace_id: UUID, + db: Session, +) -> WidgetPluginResult: + if page_context is None or trigger_reason != "proactive_opener": + return WidgetPluginResult(message=message) + + entity = page_context.get("yourPageKey") + if not entity: + return WidgetPluginResult(message=message) + + rewritten = ( + "[PROACTIVE_OPENER] The visitor is looking at " + f"{entity}. Generate one helpful sentence (≤140 chars). " + "RETURN PLAIN TEXT ONLY — no tool calls, no markdown." + ) + return WidgetPluginResult( + message=rewritten, + context_note=": proactive_opener rewrite", + telemetry={"trigger_reason": trigger_reason}, + ) +``` + +Key shape rules — copy them, don't bend them: + +- **Return the input message unchanged** when `trigger_reason` is `None`, when `page_context` is `None`, or when your vertical can't produce a useful rewrite for this request. Mid-conversation messages must pass through verbatim. +- **Never `raise`** out of `handle_widget_message`. Catch your own errors, log them, and return the unchanged message. The dispatcher logs your `telemetry`; it does not catch your exceptions. +- **Keep `telemetry` small and JSON-serialisable** — short string keys, primitive values, no full graph nodes. + +### Step 5 — Turn on the vertical for a workspace + +The vertical label is set in the database, not in a UI control (PRD-141 OS-3). Either backfill via an Alembic migration (see `orchestrator/alembic/versions/` for the Shopify backfill US-009 added) or set the row by hand for the first canary workspace: + +```sql +UPDATE workspaces +SET settings = jsonb_set(coalesce(settings, '{}'::jsonb), '{vertical}', '""') +WHERE id = ''; +``` + +The next widget message that workspace sends will be dispatched through your plugin. + +--- + +## (d) What you MAY NOT do + +The boundary, restated from [PRD-141 §12](../PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md#12-integration-coupling-rules-the-principle-written-down). These are the rules CI enforces; review will revert anything that breaks them. + +- **Do not modify `orchestrator/api/widgets/chat.py`** to special-case your vertical. The dispatcher is generic — it reads `workspace.settings["vertical"]`, looks up the registry, and calls the plugin. If you find yourself adding an `if vertical == ""` branch in `chat.py`, you are doing it wrong. +- **Do not modify `orchestrator/modules/context/`, `orchestrator/consumers/chatbot/`, or `orchestrator/modules/knowledge/graph_service.py`** to add vertical-specific behaviour. These are gated by the same CI grep gate as `chat.py`. +- **Do not add columns or fields with vertical-specific names to generic models** (no `Workspace.barbershop_*`, no `Workspace._*`). Use `settings` JSON for everything vertical-specific the workspace needs. +- **Do not let your vertical's name appear (as a string match OR an import) in any file outside `orchestrator/integrations//`.** A barbershop opener file referenced from `chat.py` defeats the whole abstraction. The only file outside the integration folder that may name your vertical is the one-line `from . import ` in `orchestrator/integrations/__init__.py` (which is itself inside the integrations tree — generic surfaces remain clean). +- **Do not add partner-specific keys to the generic skill prompt.** Skills are per-vertical files; the generic skill stays generic. See section (g). +- **Do not raise out of `handle_widget_message`.** A plugin that crashes blocks the user's chat. Catch, log, return the unchanged message. + +--- + +## (e) Test requirements + +Two things must land alongside your plugin code. + +### Snapshot fixtures and equivalence tests + +Mirror the layout under `orchestrator/integrations/shopify/tests/fixtures/`: + +``` +orchestrator/integrations//tests/ +├── conftest.py +├── fixtures/ +│ ├── _page_context.json # realistic page_context for one trigger +│ ├── __context.json # one file per trigger your plugin handles +│ ├── expected___opener.txt # verbatim string handle_widget_message returns +│ └── README.md # how the fixtures were generated (synthetic vs captured) +└── test_widget_proactive.py +``` + +Each test loads a fixture context, calls `handle_widget_message`, and asserts `result.message == expected_opener_text`. Byte-equality is the bar — a whitespace change or a reordered dict iteration in your rewrite path is a regression. See `orchestrator/integrations/shopify/tests/test_widget_proactive.py` for the working pattern. + +If your rewrite reads a knowledge graph or other workspace state, capture (or hand-craft) a small graph snapshot fixture and load it from the test the same way the Shopify tests load `inbuild_graph_snapshot.json`. + +### Grep gate update + +The CI gate `scripts/ci/check-no-shopify-in-generic.sh` enforces the §12 coupling rule for Shopify. When you ship a new vertical with its own forbidden-key set, **extend the gate**. Add a sibling script (or a generalised gate that takes the vertical as an argument — author's discretion, but keep the existing Shopify gate working). The CI workflow lives at `.github/workflows/check-shopify-isolation.yml` — wire your new check in the same job. + +The gated paths are the same as the Shopify gate's: `orchestrator/api/widgets/`, `orchestrator/modules/context/`, `orchestrator/modules/knowledge/graph_service.py`, `orchestrator/consumers/chatbot/`. Forbid your vertical's distinctive `page_context` keys (the equivalents of `productHandle`, `cartItems`) AND the `_` prefix on identifiers in those paths. + +### Local validation + +Before opening the PR: + +```bash +cd orchestrator && python -m pytest integrations/ -x --timeout=30 +bash scripts/ci/check-no-shopify-in-generic.sh # must still pass +bash scripts/ci/check-no--in-generic.sh # the gate you added +``` + +All three must be green. + +--- + +## (f) Where vertical-specific admin UI surfaces go + +This PRD set `workspace.settings["vertical"]` via API + migration, not a UI control (PRD-141 deferred admin UI as OS-3). When you do need vertical-specific admin UI — a settings panel for the partner's API keys, a status widget for the catalog sync, a connection wizard — keep it folder-isolated the same way the backend plugin is. + +- **Backend admin endpoints** for `` live under a vertical-namespaced router, e.g. `orchestrator/api/.py` for partner-specific CRUD (catalog sync status, OAuth callbacks, webhook registration). Mirror what `orchestrator/api/shopify.py` does for its sync path. The router is opt-in: not registered in the generic dispatch chain, only mounted when relevant. +- **Frontend admin pages** for `` live under a vertical-namespaced route, e.g. `frontend/src/app/integrations//`. Visibility is gated by `workspace.settings["vertical"] === ""` — generic users never see the panel. +- **Shared admin shell** (the page chrome, the workspace selector, the settings nav) stays generic and discovers integration panels via the existing registry pattern. Do not hard-code `` into the shared shell. + +The CI grep gate does **not** scan `orchestrator/api/.py` or `frontend/src/app/integrations//` — those paths are vertical-namespaced and allowed to contain your vertical's identifiers. The gate scans only the generic surfaces. + +--- + +## (g) Skill registration in `automatos-skills` + +Plugins rewrite the user message; **skills** tell the agent what to do with it. Both must ship together for a vertical to behave end-to-end. + +Add your skill to the `automatos-skills` repo under a vertical-namespaced folder: + +``` +automatos-skills//-support/SKILL.md # v1.0 +``` + +The skill prompt must: + +- **Document the page-context shape your plugin emits.** When `chat.py` prepends or your plugin rewrites a `Context: {...}` block, the agent reads it from there — the skill explains what keys to expect and which tools to call against them. +- **Reference only generic platform tools** (`platform_query_graph`, `platform_search_memory`, the rest of the PRD-71 tool catalog). If you find yourself wanting a `_*` tool, ask whether it can be expressed as a generic-but-tagged platform tool first. +- **Stay out of the generic skill.** The `generic/default-widget-support/SKILL.md` file (added in PRD-141 US-017) is the fallback for workspaces without a vertical. Do not add ``-specific instructions to it. + +Once the skill is published, attach it to your vertical's workspace template (or to the canary workspace directly) so the agent loads it on every widget message. + +See `automatos-skills/shopify/shopify-support/SKILL.md` for a working reference — that file is the model the PRD-141 Phase 3 stories use for `` too. + +--- + +## Working reference + +Whenever this doc says "and X" without spelling out the detail, the answer is in **`orchestrator/integrations/shopify/widget_proactive.py`** — it is the production implementation of every pattern this README describes: + +- Two-trigger dispatch (`proactive_opener`, `cart_idle`) with a single `handle_widget_message` +- Graph-resolved entities (single-seed and multi-seed walks via `GraphifyService`) +- Telemetry passed back to the dispatcher's log line without crossing the generic boundary +- Folder-isolated helpers (`context_fields.py`) referenced only from within the package +- Snapshot tests asserting byte-equality against captured fixtures + +Read it, then come back here and ship your vertical. diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index 62ba6c62c..a3a4f54d5 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -38,7 +38,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc ## Phase 4 — Docs -- [ ] US-018 — Write docs/integrations/README.md explaining how to add a new vertical +- [x] US-018 — Write docs/integrations/README.md explaining how to add a new vertical - [ ] US-019 — Write hypothetical barbershop walkthrough doc as architecture validation ## Release gate From 5f9254b226ceb131832c77d41a34b58918aa3fab Mon Sep 17 00:00:00 2001 From: Gerard Kavanagh Date: Thu, 28 May 2026 20:58:11 +0100 Subject: [PATCH 11/11] =?UTF-8?q?feat(prd-141):=20US-019=20=E2=80=94=20bar?= =?UTF-8?q?bershop=20walkthrough=20architecture=20validation;=20PRD-141=20?= =?UTF-8?q?build=20complete=20(cross-repo=20+=20canary=20stories=20remain?= =?UTF-8?q?=20BLOCKED=20for=20human)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds docs/architecture/integration-walkthrough-barbershop.md — a worked end-to-end walkthrough of adding a hypothetical "barbershop" vertical (ClipShop UK booking site) against the Phase 1 plugin protocol. Mirrors Shopify's package layout, handle_widget_message dispatch, and graph- backed message builders to prove the abstraction is not Shopify-shaped in disguise. Produces the PRD §13 target opener byte-for-byte ("Looking at booking with Sarah for a fade — her next opening is Tuesday at 2pm. Want me to hold it?"), routed via PLUGIN_REGISTRY exactly as Shopify is. Catalogues seven Known Abstraction Gaps surfaced by the walkthrough: no convention for partner-API calls inside the rewrite (Gap 1); centralised PROACTIVE_TRIGGER_REASONS in chat.py (Gap 2); no missing- skill detection at dispatch (Gap 3); duplicated _format_opener_context_ value (Gap 4); no page_context schema versioning (Gap 5); multi-location overrides (Gap 6); sync Session in async protocol (Gap 7). Verdict: Phase 1 protocol is vertical-neutral enough to ship; all gaps are additive and defer to the PRD that adds vertical #2. Story: scripts/ralph/prd.json US-019 PRD: docs/PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md Ralph executable scope complete: - US-001..US-012 done (Phase 0/1 scaffolding + lifts + CI gate) - US-018, US-019 done (docs) - US-013/014/015 — BLOCKED-CROSS-REPO (automatos-widget-sdk) - US-016/017 — BLOCKED-CROSS-REPO (automatos-skills) - US-020 — SKIPPED-HUMAN (operational canary) RALPH_COMPLETE --- .../integration-walkthrough-barbershop.md | 567 ++++++++++++++++++ scripts/ralph/IMPLEMENTATION_PLAN_prd141.md | 2 +- 2 files changed, 568 insertions(+), 1 deletion(-) create mode 100644 docs/architecture/integration-walkthrough-barbershop.md diff --git a/docs/architecture/integration-walkthrough-barbershop.md b/docs/architecture/integration-walkthrough-barbershop.md new file mode 100644 index 000000000..5a908f87c --- /dev/null +++ b/docs/architecture/integration-walkthrough-barbershop.md @@ -0,0 +1,567 @@ +# Walkthrough — Adding a Barbershop Vertical (Architecture Validation) + +> Hypothetical exercise from **[PRD-141 §13](../PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md#13-appendix--hypothetical-barbershop-walkthrough-sanity-check)**. Proves the plugin protocol introduced in Phase 0 (`orchestrator/integrations/__init__.py`) is not Shopify-shaped in disguise — and flags the rough edges a real vertical #2 would hit. +> +> Nothing in this document is built. There is no `orchestrator/integrations/barbershop/`. The point is to walk the protocol end-to-end against a vertical the team hasn't shipped, find the friction, and write it down before INBUILD goes live on the Phase 1 refactor. +> +> Companion docs: +> +> - **[PRD-141 §12 — Integration Coupling Rules](../PRDS/141-WIDGET-VERTICAL-AGNOSTIC-REFACTOR.md#12-integration-coupling-rules-the-principle-written-down)** — the boundary. +> - **[docs/integrations/README.md](../integrations/README.md)** — the runbook for actually adding a vertical. +> - **Working reference: `orchestrator/integrations/shopify/widget_proactive.py`** — every pattern below mirrors one in there. + +--- + +## 1. The scenario + +ClipShop UK runs 47 high-street barbershops. They install the Automatos widget on `clipshop.co.uk`. The site is a hand-rolled booking app — not Shopify, not Calendly. Their pages look like this: + +- `/stylists/sarah-chen` — a stylist's profile page (bio, reviews, available services) +- `/book?stylist=sarah-chen&service=fade-cut` — booking widget for a specific stylist + service +- `/locations/manchester-piccadilly` — branch landing page (which stylists work there, opening hours) + +The host theme is configured to embed page metadata into the Automatos widget: + +```js +window.AutomatosWidget.init({ + workspaceId: "a8f7...", + pageContext: { + pageType: "stylist_profile", + stylistId: "sarah-chen", + stylistName: "Sarah Chen", + serviceType: "fade-cut", + nextSlotUtc: "2026-06-02T14:00:00Z", + currentBookings: 12, + locationSlug: "manchester-piccadilly", + avgReviewScore: 4.8, + reviewCount: 137, + }, +}); +``` + +Workspace settings in the database: + +```sql +SELECT settings FROM workspaces WHERE id = 'a8f7...'; +-- { +-- "vertical": "barbershop", +-- "booking_provider": "internal", +-- "locations": ["manchester-piccadilly", "leeds-vicar-lane", ...] +-- } +``` + +Goal: when a visitor lands on `/stylists/sarah-chen` and idles ~7 seconds, the widget fires a `proactive_opener` request. The backend rewrites the message into a directive the agent uses to produce: + +> "Looking at booking with Sarah for a fade — her next opening is Tuesday at 2pm. Want me to hold it?" + +Identical pattern to PRD-007 for Shopify; vertical-specific data; zero changes to the generic dispatcher. + +--- + +## 2. What gets added + +Three new pieces, mirroring the Shopify shape: + +| Where | What | +|---|---| +| `orchestrator/integrations/barbershop/` | The vertical plugin (this walkthrough) | +| `automatos-skills/barbershop/booking-host/SKILL.md` | The agent skill that consumes the rewritten directive | +| `scripts/ci/check-no-barbershop-in-generic.sh` | The CI grep gate (mirrors `check-no-shopify-in-generic.sh`) | + +What gets **modified** in generic surfaces: **one line**, in `orchestrator/integrations/__init__.py`: + +```python +from . import generic # noqa: E402,F401 (registers "generic") +from . import shopify # noqa: E402,F401 (registers "shopify") +from . import barbershop # noqa: E402,F401 (registers "barbershop") ← added +``` + +That is the entire generic-surface diff. No other file outside `orchestrator/integrations/` mentions `barbershop`. If the walkthrough needed more than that line, the abstraction would be broken. + +--- + +## 3. The plugin + +### 3.1 Package layout + +Mirror Shopify's: + +``` +orchestrator/integrations/barbershop/ +├── __init__.py # self-registers as PLUGIN_REGISTRY["barbershop"] +├── widget_proactive.py # handle_widget_message + private helpers +├── context_fields.py # _OPENER_CONTEXT_FIELDS + value formatter (barbershop-shaped) +└── tests/ + ├── conftest.py + ├── fixtures/ + │ ├── stylist_profile_context.json + │ ├── booking_idle_context.json + │ ├── clipshop_graph_snapshot.json + │ ├── expected_stylist_opener.txt + │ └── README.md + └── test_widget_proactive.py +``` + +Folder-isolated. Nothing imports anything from `orchestrator/integrations/shopify/` — the two verticals do not share code (yet — see [Gap 4](#gap-4--_format_opener_context_value-is-duplicated)). + +### 3.2 Self-registration + +`__init__.py` is a copy-paste of Shopify's with one slug change: + +```python +"""Barbershop widget plugin package — see widget_proactive.py.""" + +from __future__ import annotations + +from integrations import PLUGIN_REGISTRY + +from . import widget_proactive + +PLUGIN_REGISTRY["barbershop"] = widget_proactive + +__all__ = ["widget_proactive"] +``` + +### 3.3 Page-context field mapping + +`context_fields.py` is the barbershop equivalent of Shopify's. Same shape, different keys: + +```python +_OPENER_CONTEXT_FIELDS: tuple[tuple[str, str], ...] = ( + ("pageType", "page_type"), + ("stylistName", "stylist"), + ("serviceType", "service"), + ("nextSlotUtc", "next_slot"), + ("avgReviewScore", "avg_review"), + ("reviewCount", "review_count"), + ("locationSlug", "location"), + ("currentBookings", "current_bookings"), +) + + +def _format_opener_context_value(key: str, value) -> Optional[str]: + if value is None or value == "" or value == 0 or value is False: + return None + if isinstance(value, str): + return f'{key}="{value}"' if " " in value or '"' in value else f"{key}={value}" + return f"{key}={value}" +``` + +The formatter is byte-identical to Shopify's. We tolerate the duplication for now (see [Gap 4](#gap-4--_format_opener_context_value-is-duplicated)). + +### 3.4 `handle_widget_message` — proactive_opener path + +The plugin handles two triggers: `proactive_opener` (on stylist profile pages) and `booking_idle` (on the booking page after N seconds without selecting a slot). Pseudocode mirrors Shopify's `_resolve_graph_related_products` + `_build_proactive_opener_message`: + +```python +async def handle_widget_message( + *, + message: str, + page_context: Optional[dict], + trigger_reason: Optional[str], + workspace_id: UUID, + db: Session, +) -> WidgetPluginResult: + if page_context is None or trigger_reason not in ("proactive_opener", "booking_idle"): + return WidgetPluginResult(message=message) + + workspace_str = str(workspace_id) + + if trigger_reason == "booking_idle": + rewritten = _build_booking_idle_message(page_context) + return WidgetPluginResult( + message=rewritten, + context_note="barbershop: booking_idle rewrite", + telemetry={ + "trigger_reason": trigger_reason, + "stylist_id": page_context.get("stylistId"), + "service_type": page_context.get("serviceType"), + }, + ) + + stylist_facts = await _resolve_stylist_facts(workspace_str, page_context) + rewritten = _build_stylist_opener_message( + page_context, stylist_facts=stylist_facts, + ) + return WidgetPluginResult( + message=rewritten, + context_note="barbershop: proactive_opener rewrite", + telemetry={ + "trigger_reason": trigger_reason, + "stylist_id": page_context.get("stylistId"), + "fact_count": len(stylist_facts), + }, + ) + + +async def _resolve_stylist_facts( + workspace_id: str, + page_context: dict, +) -> list[dict]: + """Walk the workspace KB for one stylist's reviews + speciality + branch. + + Looks up the seed node by ``stylistId``, then walks 1-hop edges by relation: + - ``has_review`` — top reviews to cite + - ``has_speciality`` — "specialises in fades", etc. + - ``works_at`` — the branch the stylist anchors to + """ + stylist_id = (page_context or {}).get("stylistId") + if not stylist_id: + return [] + try: + from modules.knowledge.graph_service import GraphifyService + + gs = GraphifyService() + graph = await gs.load_graph(workspace_id) + if graph is None: + return [] + + seed_id = None + for node_id, attrs in graph.nodes(data=True): + node_attrs = attrs.get("attrs") or {} + if attrs.get("file_type") == "stylist_profile" and node_attrs.get("slug") == stylist_id: + seed_id = node_id + break + if seed_id is None: + return [] + + facts: list[dict] = [] + for u, v, edata in graph.edges(seed_id, data=True): + rel = (edata.get("relation") or "").lower() + if rel not in ("has_review", "has_speciality", "works_at"): + continue + other = v if u == seed_id else u + other_attrs = graph.nodes[other] + facts.append({ + "relation": rel, + "label": other_attrs.get("label") or other, + "type": other_attrs.get("file_type", ""), + "weight": edata.get("weight", 0), + }) + facts.sort(key=lambda f: ( + {"has_review": 0, "has_speciality": 1, "works_at": 2}.get(f["relation"], 99), + -f["weight"], + )) + return facts[:6] + + except Exception as e: # noqa: BLE001 — opener falls back gracefully + logger.warning("_resolve_stylist_facts failed: %s", e) + return [] + + +def _build_stylist_opener_message( + page_context: dict, stylist_facts: list[dict], +) -> str: + parts: list[str] = [] + for src_key, label in _OPENER_CONTEXT_FIELDS: + rendered = _format_opener_context_value(label, page_context.get(src_key)) + if rendered is not None: + parts.append(rendered) + summary = ", ".join(parts) if parts else "no context" + + facts_block = "" + if stylist_facts: + rendered_facts = [] + for f in stylist_facts: + label = f.get("label", "?") + rel = f.get("relation", "") + if rel == "has_review": + rendered_facts.append(f'review: "{label}"') + elif rel == "has_speciality": + rendered_facts.append(f"specialises in {label}") + elif rel == "works_at": + rendered_facts.append(f"branch: {label}") + else: + rendered_facts.append(label) + facts_block = ( + " Stylist facts from workspace KB (cite naturally — prefer " + "specialities and reviews, mention next slot only if the " + "visitor leans booking-ward): " + "; ".join(rendered_facts) + ) + + return ( + "[PROACTIVE_OPENER] Generate a contextual one-sentence opener " + "(≤140 chars). RETURN PLAIN TEXT ONLY — no tool calls, no JSON, " + "no markdown, no greetings. Use the facts below as your source of " + "truth — do NOT invent specialities, slot times, or review content " + "the context doesn't include. If a fact you'd want isn't here, ask " + "a question instead of fabricating. " + f"Context: {summary}.{facts_block}" + ) +``` + +Notice what isn't there: + +- **No external API call.** `nextSlotUtc` arrives in `page_context` — the host site embeds it at page-render time. The plugin doesn't call ClipShop's booking API inside the rewrite (see [Gap 1](#gap-1--no-convention-for-partner-api-calls-inside-the-rewrite)). +- **No imports from generic surfaces.** All the plumbing lives inside `integrations/barbershop/`. +- **No new dispatch logic in `chat.py`.** The call site is unchanged: `plugin = PLUGIN_REGISTRY[vertical]; result = await plugin.handle_widget_message(...)`. + +### 3.5 `booking_idle` trigger + +When a visitor opens the booking page and leaves it idle for N seconds, the SDK fires `trigger_reason="booking_idle"`. The plugin synthesizes a nudge referencing the actual stylist + slot the page is configured for: + +```python +def _build_booking_idle_message(page_context: dict) -> str: + stylist = page_context.get("stylistName") or "the stylist" + service = page_context.get("serviceType") or "an appointment" + slot = page_context.get("nextSlotUtc") + slot_part = f" next_slot={slot}." if slot else "" + return ( + "[PROACTIVE_OPENER] [BOOKING_IDLE] The visitor is on the booking " + f"page for {stylist}, service={service}.{slot_part} Generate one " + "sentence (≤140 chars) nudging them to hold the slot OR asking " + "what's blocking them. RETURN PLAIN TEXT ONLY — no tool calls, no " + "markdown. Do NOT invent alternative slot times the context " + "doesn't include." + ) +``` + +Same shape as Shopify's `_build_cart_idle_opener_message` — directive boilerplate is reused; only the data shape changes. + +--- + +## 4. The matching skill + +`automatos-skills/barbershop/booking-host/SKILL.md v1.0` — outline (frontmatter shape is illustrative; the real skill repo's frontmatter rules apply): + +```markdown +--- +name: booking-host +description: Conversational booking assistant for barbershop, salon, and + appointment-based service businesses using the Automatos widget. +version: 1.0 +verticals: [barbershop] +tools: + - platform_query_graph + - platform_search_memory + - platform_calendar_lookup # hypothetical — see Gap 1 + - widget_open_callback_form +--- + +## When to use + +This skill runs when ``workspace.settings.vertical == "barbershop"`` and a +visitor sends a widget message. + +## Proactive openers + +Mid-conversation messages from the widget may arrive with a +``[PROACTIVE_OPENER]`` prefix. Example payload the agent sees: + + [PROACTIVE_OPENER] Generate a contextual one-sentence opener (≤140 chars). + RETURN PLAIN TEXT ONLY ... Context: page_type=stylist_profile, + stylist="Sarah Chen", service=fade-cut, next_slot=2026-06-02T14:00:00Z, + avg_review=4.8, review_count=137. Stylist facts from workspace KB: + specialises in fades; review: "Sarah's fade was the cleanest I've had"; + branch: Manchester Piccadilly. + +When you see ``[PROACTIVE_OPENER]``: + +1. Produce one sentence ≤140 chars, no tool calls. +2. Lead with the strongest grounded fact — a specific review, a speciality, + or the actual slot time. +3. Never invent slot times, reviews, or specialities the directive doesn't + name. +4. End with a soft question that invites a reply ("Want me to hold it?"). + +## Reading mid-conversation context + +On regular messages (no ``[PROACTIVE_OPENER]``), the widget may prepend a +``(Context: {...})`` block. Treat any ``stylistId``, ``serviceType``, or +``locationSlug`` in that block as a hard anchor — call +``platform_query_graph`` with that identifier before falling back to +keyword search. + +## Tools + +- ``platform_query_graph`` — workspace KB (stylists, services, reviews, + branches) +- ``platform_search_memory`` — past conversations the visitor may have had +- ``platform_calendar_lookup`` — *hypothetical* — would let the agent + confirm a slot is still free at answer-time. Does not exist today. See + Gap 1. +- ``widget_open_callback_form`` — human handoff for booking changes the + agent can't make +``` + +What this skill is **not**: + +- It does not reference Shopify, products, or carts. +- It does not pretend `platform_calendar_lookup` exists today — the description marks it hypothetical, and Gap 1 catalogues the missing convention. +- It is published independently of any Phase 1 Shopify skill change. The vertical's skill is in its own folder. + +--- + +## 5. Worked example — end-to-end trace + +A visitor lands on `/stylists/sarah-chen` and idles 7 seconds. + +1. **SDK fires** `POST /api/widgets/chat`: + + ```json + { + "message": "Hi", + "trigger_reason": "proactive_opener", + "page_context": { + "pageType": "stylist_profile", + "stylistId": "sarah-chen", + "stylistName": "Sarah Chen", + "serviceType": "fade-cut", + "nextSlotUtc": "2026-06-02T14:00:00Z", + "avgReviewScore": 4.8, + "reviewCount": 137, + "locationSlug": "manchester-piccadilly", + "currentBookings": 12 + } + } + ``` + +2. **Dispatcher** (`chat.py`) reads `workspace.settings.vertical → "barbershop"`. Calls `PLUGIN_REGISTRY["barbershop"].handle_widget_message(...)`. + +3. **Plugin** walks the KB graph (finds Sarah Chen's node, walks 1-hop to `has_review`/`has_speciality`/`works_at`), returns: + + ```text + message: + "[PROACTIVE_OPENER] Generate a contextual one-sentence opener (≤140 chars). + RETURN PLAIN TEXT ONLY ... Context: page_type=stylist_profile, + stylist=\"Sarah Chen\", service=fade-cut, next_slot=2026-06-02T14:00:00Z, + avg_review=4.8, review_count=137, location=manchester-piccadilly, + current_bookings=12. Stylist facts from workspace KB: specialises in + fades; review: \"Sarah's fade was the cleanest I've had\"; branch: + Manchester Piccadilly." + + telemetry: + {"trigger_reason": "proactive_opener", + "stylist_id": "sarah-chen", + "fact_count": 3} + ``` + +4. **Dispatcher logs**: + + ```text + PROACTIVE_REWRITE: vertical=barbershop trigger=proactive_opener + original_msg_len=2 new_msg_len=... telemetry={...} + ``` + +5. **Streaming agent** runs the `booking-host` skill and produces: + + > "Looking at booking with Sarah for a fade — her next opening is Tuesday at 2pm. Want me to hold it?" + +6. **Visitor receives** the opener. + +Nothing in steps 2–6 named `barbershop` outside `integrations/barbershop/` and the skill file. The dispatcher's behaviour is identical to the Shopify path — only the registry key differs. + +--- + +## 6. What didn't need to change + +For sanity, here is everything the Phase 1 abstraction lets stay put when vertical #2 arrives: + +- `orchestrator/api/widgets/chat.py` — unchanged. The dispatcher reads `workspace.settings.vertical` and looks up the registry. No new branch. +- `orchestrator/api/widgets/auth.py`, `orchestrator/api/widgets/schemas.py` — unchanged. `page_context` is already typed as `Optional[dict[str, Any]]`. +- `orchestrator/modules/context/sections/*` — unchanged. +- `orchestrator/consumers/chatbot/*` — unchanged. +- `orchestrator/modules/knowledge/graph_service.py` — unchanged. +- The SDK contract — designed in US-013/014/015 (Phase 2 SDK work, pending in the SDK repo) to widen to `Record`. Once that ships, the barbershop's `page_context` shape flows through without further SDK change. +- The Alembic column — already there (`workspaces.settings JSONB`). The barbershop backfill is an analogous `UPDATE` per workspace, modelled on the Shopify migration US-009 added. + +The Phase 1 abstraction holds. + +--- + +## 7. Known abstraction gaps + +The walkthrough completes — but here are the rough edges that surfaced. None of these block Phase 1 (Shopify) shipping. They shape the PRD that adds vertical #2. + +### Gap 1 — no convention for partner-API calls inside the rewrite + +The barbershop plugin **assumes** the host site embeds `nextSlotUtc` into `page_context`. That works when the slot is computed at page-render time. It breaks for "is this slot still free *right now*?" — the canonical anti-stale check before saying "want me to hold it?". + +Today there is no pattern for `handle_widget_message` to call a partner API (ClipShop's booking endpoint) inside the rewrite: + +- No latency budget — `handle_widget_message` blocks the visitor's chat response. +- No failure convention — a 502 from the partner API would silently degrade to "pass message through unchanged" (per the Shopify pattern), which loses the slot data the visitor came for. +- No cache layer. + +The skill outline above lists a hypothetical `platform_calendar_lookup` tool — that's the agent-side answer (let the agent fetch fresh slot data when asked). The plugin-side answer (rewrite-time freshness) is a separate question. + +**Path forward:** when vertical #2 needs real-time partner data, add a `PluginPartnerClient` mixin with a timeout-bounded caching wrapper. Defer until needed; do not pre-build it for Phase 1. + +### Gap 2 — `PROACTIVE_TRIGGER_REASONS` is centralised in `chat.py` + +`PROACTIVE_TRIGGER_REASONS` (the frozenset that flips `is_proactive` for the agent's LLM-call shape — text-only, no Composio) lives in `orchestrator/api/widgets/chat.py`. The barbershop plugin's `booking_idle` trigger would need adding to that frozenset — a generic-surface edit. + +Today's workaround: barbershop reuses the existing `cart_idle` string. The plugin sees the trigger name; the dispatcher only checks set membership. Ugly, works. + +**Path forward:** verticals declare their trigger reasons via a module-level `TRIGGER_REASONS: frozenset[str]` constant on the plugin module, and the dispatcher unions all plugin-declared trigger reasons at import time. Mild lift; do it in the same PR that adds vertical #2. + +### Gap 3 — no missing-skill detection at dispatch time + +`workspace.settings.vertical = "barbershop"` activates the plugin. It does **not** activate the `barbershop/booking-host` skill. If the skill is missing or stale, the plugin's rewrite is meaningless — the agent receives a `[PROACTIVE_OPENER]` directive and has no instructions for what to do with it. + +The Shopify path is fine today because `shopify-support` is installed everywhere we care about. The vertical-#2 path has no such guarantee. + +**Path forward:** at workspace setup time, validate that `vertical == "X"` AND `skill X-support is installed`. Two options: + +- a `bootstrap_check()` on the plugin protocol returning the skill name(s) it requires, or +- a static lookup table in `orchestrator/integrations/__init__.py` mapping vertical → required skill slugs. + +The latter is simpler; the former couples better to plugin evolution. + +### Gap 4 — `_format_opener_context_value` is duplicated + +The barbershop `context_fields.py` formatter is byte-identical to Shopify's: + +```python +f'{key}="{value}"' if " " in value or '"' in value else f"{key}={value}" +``` + +Two verticals, same code. That suggests a shared utility — e.g. `integrations/_common/context_fields.py` exporting the formatter. + +**Path forward:** harvest only when vertical #3 lands (rule of three). Two copies is fine; three copies is a smell. + +### Gap 5 — no `page_context` schema versioning + +A host-site rename (`stylistId → professionalId` in v2 of their theme) silently breaks the rewrite. The plugin tolerates missing keys, so the fallback is "drop to a less-grounded opener" — the visitor sees a worse reply without any error logged or alert raised. + +Shopify has the same hazard today (a theme dev renaming `productHandle` to `product_handle_v2` would silently degrade INBUILD). + +**Path forward:** require a `page_context["_v"]: int` per vertical. The plugin asserts the major version it understands and logs a `KEY_DRIFT` warning when it sees a newer version. Telemetry only; not a blocker. + +### Gap 6 — multi-location / per-row overrides + +A 47-branch chain runs as one workspace with `vertical: "barbershop"` and a `locations: [...]` list. The plugin currently has no place to read per-location overrides (each branch has its own opening hours, its own stylist roster, its own pricing). + +The protocol's `db: Session` argument means the plugin *can* read arbitrary state — but there is no shape convention for "per-location settings". A naive plugin would JSON-pack everything into `workspace.settings`; an opinionated one would want a `workspace_locations` table. + +**Path forward:** out of scope for the plugin protocol. This is a product question — solve at the workspace-template level when the partner brief arrives. + +### Gap 7 — `db: Session` is sync; the protocol is async + +The protocol declares `db: Session` (SQLAlchemy sync session) but `handle_widget_message` is `async`. Today both Shopify functions use the session indirectly via `GraphifyService` (which loads its own state). If a vertical needed to run a `SELECT` inside the rewrite, it would be doing sync DB work inside an async function — fine in practice (FastAPI's sync session is threadlocal-backed) but a foot-gun. + +**Path forward:** when a vertical first needs in-rewrite DB reads, either pass `AsyncSession` (PRD-XXX migration) or have the plugin offload via `asyncio.to_thread`. Document the pattern; do not change the protocol until forced. + +--- + +## 8. Verdict + +The Phase 1 plugin protocol is **vertical-neutral enough** to ship production-grade vertical #2 without revising the protocol. Every gap above is incremental — additive constants, additional helpers, additional contracts that bolt on without rewriting `WidgetPlugin`. + +The walkthrough produces the target opener: + +> "Looking at booking with Sarah for a fade — her next opening is Tuesday at 2pm. Want me to hold it?" + +— and routes through the dispatcher the same way Shopify does. The abstraction is not Shopify-shaped in disguise. + +**Recommendation:** + +1. Ship Phase 1 as designed. The abstraction holds. +2. When vertical #2 lands, address Gaps 1, 2, and 3 in the same PR — they have direct partner-brief implications. +3. Reassess Gaps 4–7 only if and when a third vertical materialises. + +--- + +**Last updated:** 2026-05-28 +**Status:** Architecture validation complete. No blocking gaps identified. +**Reviewer:** Auto (peer review pending). diff --git a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md index a3a4f54d5..d5a9de466 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -39,7 +39,7 @@ Single source of truth for Ralph progress. Tick `- [x]` only after a story's acc ## Phase 4 — Docs - [x] US-018 — Write docs/integrations/README.md explaining how to add a new vertical -- [ ] US-019 — Write hypothetical barbershop walkthrough doc as architecture validation +- [x] US-019 — Write hypothetical barbershop walkthrough doc as architecture validation ## Release gate