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/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/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/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/orchestrator/api/widgets/chat.py b/orchestrator/api/widgets/chat.py index 5249c6dc1..51d88188e 100644 --- a/orchestrator/api/widgets/chat.py +++ b/orchestrator/api/widgets/chat.py @@ -27,6 +27,7 @@ from api.widgets.auth import WidgetAuthContext, require_permission, widget_auth from core.database.database import get_db +from integrations import PLUGIN_REGISTRY from modules.tools.widget_callback import ( WIDGET_OPEN_CALLBACK_FORM_NAME, WIDGET_SIGNAL_KEY, @@ -74,376 +75,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, -) -> 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}" - ) - - -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, - *, - 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, -) -> 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 @@ -473,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 # --------------------------------------------------------------------------- @@ -537,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", ) @@ -824,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/context_fields.py b/orchestrator/integrations/shopify/context_fields.py new file mode 100644 index 000000000..f8c2493eb --- /dev/null +++ b/orchestrator/integrations/shopify/context_fields.py @@ -0,0 +1,69 @@ +"""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`` (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 +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 new file mode 100644 index 000000000..3b62bf812 --- /dev/null +++ b/orchestrator/integrations/shopify/tests/conftest.py @@ -0,0 +1,122 @@ +"""Shared test fixtures for the Shopify widget plugin tests. + +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, + 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. + +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``. 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 +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 json +import sys +import types +from pathlib import Path + +import networkx as nx +import pytest +from networkx.readwrite import json_graph + + +_THIS_DIR = Path(__file__).resolve().parent +_FIXTURES_DIR = _THIS_DIR / "fixtures" + + +@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() + + +@pytest.fixture +def real_chat_with_graph(monkeypatch, inbuild_graph): + """Inject a fixture-bound ``GraphifyService`` for the lifted resolvers. + + 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 + 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/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") + + 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 + ) + + return { + "graph_service": fake_graph_service_mod, + } 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/orchestrator/integrations/shopify/tests/test_widget_proactive.py b/orchestrator/integrations/shopify/tests/test_widget_proactive.py index 2ea982fec..e90ccd9f5 100644 --- a/orchestrator/integrations/shopify/tests/test_widget_proactive.py +++ b/orchestrator/integrations/shopify/tests/test_widget_proactive.py @@ -1,32 +1,31 @@ -"""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 - 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. +* 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``. 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 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 +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,15 +47,25 @@ def workspace_id(): @pytest.fixture def fake_chat(monkeypatch): - """Replace ``api.widgets.chat`` in ``sys.modules`` with a fake. + """Replace the plugin's downstream helpers with sentinels. - The fake records every call so tests can assert the shim forwards + All four helpers live on :mod:`widget_proactive` itself: + + * ``_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) + + Each is patched in place via ``monkeypatch.setattr``. The fake + 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. - """ - 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": [], @@ -97,12 +106,26 @@ 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, + ) + monkeypatch.setattr( + widget_proactive, + "_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 @@ -224,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, @@ -264,8 +287,117 @@ 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, } + + +# ---- 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). 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. +# +# These tests must KEEP PASSING through every remaining lift (US-010 in +# 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). + + +@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: 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``. + + 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: 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: 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 plugin's gate is symmetric — either side + short-circuits the rewrite. This is a regression guard for the + chat.py ``is_proactive`` semantics now owned by the plugin. + """ + 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/orchestrator/integrations/shopify/widget_proactive.py b/orchestrator/integrations/shopify/widget_proactive.py index f69fee24e..f58782344 100644 --- a/orchestrator/integrations/shopify/widget_proactive.py +++ b/orchestrator/integrations/shopify/widget_proactive.py @@ -1,52 +1,382 @@ -"""Shopify vertical plugin — TEMPORARY shim delegating to chat.py. - -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 -existing 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 four 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. +"""Shopify vertical plugin — proactive opener + cart-idle directive builders. + +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` — 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` — 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 +import logging from typing import Optional from uuid import UUID 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__) + + +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, + *, + 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_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( @@ -57,24 +387,24 @@ async def handle_widget_message( workspace_id: UUID, db: Session, ) -> WidgetPluginResult: - """Shim — replicates the inline chat.py proactive-rewrite block. + """Build the Shopify proactive opener / cart-idle directive. - Behaviour mirrors ``api/widgets/chat.py`` byte-for-byte: + 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) @@ -82,11 +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, - _resolve_cart_recommendations, - ) - recommendations = await _resolve_cart_recommendations( workspace_str, page_context, ) @@ -96,18 +421,13 @@ 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), }, ) - from api.widgets.chat import ( - _build_proactive_opener_message, - _resolve_graph_related_products, - ) - related_products = await _resolve_graph_related_products( workspace_str, page_context, ) @@ -117,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/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/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 1aa1b8384..d5a9de466 100644 --- a/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md +++ b/scripts/ralph/IMPLEMENTATION_PLAN_prd141.md @@ -16,14 +16,14 @@ 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-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-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 +- [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 +- [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)* +- [x] US-010 — Rewire chat.py to dispatch via PLUGIN_REGISTRY and delete inline Shopify functions +- [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)* @@ -38,8 +38,8 @@ 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 -- [ ] US-019 — Write hypothetical barbershop walkthrough doc as architecture validation +- [x] US-018 — Write docs/integrations/README.md explaining how to add a new vertical +- [x] US-019 — Write hypothetical barbershop walkthrough doc as architecture validation ## Release gate @@ -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)",