diff --git a/katana_mcp_server/src/katana_mcp/tools/_modification.py b/katana_mcp_server/src/katana_mcp/tools/_modification.py index 42bd4ab5..7de84cad 100644 --- a/katana_mcp_server/src/katana_mcp/tools/_modification.py +++ b/katana_mcp_server/src/katana_mcp/tools/_modification.py @@ -294,6 +294,13 @@ class ModificationResponse(BaseModel): next_actions: list[str] = Field(default_factory=list) katana_url: str | None = None message: str + # Entity-view extras keyed by an entity-specific contract. Today + # ``product_bom`` uses ``resolved_ingredients`` (int → {sku, display_name}) + # so ``build_bom_modify_ui`` can render the per-row identity for *added* + # rows (the prior_state snapshot already resolves SKUs for existing rows). + # Other entity types may add their own keys without polluting the universal + # response shape. The renderer accesses them by name; absence is graceful. + extras: dict[str, Any] = Field(default_factory=dict) DEFAULT_EXCLUDED: ClassVar[tuple[str, ...]] = ("id", "preview") @@ -575,6 +582,7 @@ def to_tool_result( every #721 child PR has shipped. """ from katana_mcp.tools.prefab_ui import ( + build_bom_modify_ui, build_modification_preview_ui, build_modification_result_ui, build_po_modify_ui, @@ -582,8 +590,8 @@ def to_tool_result( response_dict = response.model_dump() - # Per-entity dispatch — PO migrated in #722; remaining entities - # (SO, MO, stock_transfer, item) fall through to the legacy + # Per-entity dispatch — PO migrated in #722, BOM in #811; remaining + # entities (SO, MO, stock_transfer, item) fall through to the legacy # builders until their respective child PRs land. if response.entity_type == "purchase_order": ui = build_po_modify_ui( @@ -593,6 +601,14 @@ def to_tool_result( ) return make_tool_result(response, ui=ui) + if response.entity_type == "product_bom": + ui = build_bom_modify_ui( + response_dict, + confirm_request=confirm_request, + confirm_tool=confirm_tool, + ) + return make_tool_result(response, ui=ui) + # Legacy path — preserves today's behavior for not-yet-migrated # entity types so #722 can land without cross-entity test churn. if response.is_preview: diff --git a/katana_mcp_server/src/katana_mcp/tools/_modification_dispatch.py b/katana_mcp_server/src/katana_mcp/tools/_modification_dispatch.py index d34b4e69..cb99653d 100644 --- a/katana_mcp_server/src/katana_mcp/tools/_modification_dispatch.py +++ b/katana_mcp_server/src/katana_mcp/tools/_modification_dispatch.py @@ -704,6 +704,7 @@ async def run_modify_plan( plan: list[ActionSpec], has_get_endpoint: bool = True, cache_merge: CacheMerge | None = None, + extras: dict[str, Any] | None = None, ) -> ModificationResponse: """Wrap a built plan in a preview-or-execute :class:`ModificationResponse`. @@ -733,6 +734,10 @@ async def run_modify_plan( ``@cache_read`` tools see fresh data without ``rebuild_cache``. Omit for tools without a GET-by-id (stock_transfers) — the cache stays stale on modify until the next sync window. + extras: Entity-specific extras for the renderer (e.g. BOM's + ``resolved_ingredients`` lookup). Threaded onto + :attr:`ModificationResponse.extras` on both the preview and + apply branches. Defaults to an empty dict. """ entity_type = naming.entity_type entity_label = naming.entity_label @@ -813,6 +818,7 @@ async def run_modify_plan( ], katana_url=katana_url, message=f"Preview: {len(plan)} action(s) planned for {entity_label}", + extras=extras or {}, ) actions = await execute_plan(plan) @@ -868,6 +874,7 @@ async def run_modify_plan( next_actions=next_actions, katana_url=katana_url, message=message, + extras=extras or {}, ) diff --git a/katana_mcp_server/src/katana_mcp/tools/foundation/bom.py b/katana_mcp_server/src/katana_mcp/tools/foundation/bom.py index db9044ca..7c848549 100644 --- a/katana_mcp_server/src/katana_mcp/tools/foundation/bom.py +++ b/katana_mcp_server/src/katana_mcp/tools/foundation/bom.py @@ -542,6 +542,37 @@ async def apply() -> None: # ---------------------------------------------------------------------------- +def _collect_ingredient_ids( + request: ManageProductBomRequest, + existing_snapshot: GetProductBomResponse | None, +) -> set[int]: + """Union of every ingredient_variant_id touched by the plan. + + Adds + updates contribute their request-side ingredient id (when the + update swaps the ingredient). Updates + deletes also contribute the + *existing* row's ingredient id (resolved via the snapshot) so the + builder can render the pre-patch identity. Falls back gracefully + when the snapshot is None (skip the existing-row contributions). + """ + ids: set[int] = set() + for add in request.add_bom_rows or []: + ids.add(add.ingredient_variant_id) + for patch in request.update_bom_rows or []: + if patch.ingredient_variant_id is not None: + ids.add(patch.ingredient_variant_id) + if existing_snapshot is not None: + rows_by_id = {row.id: row for row in existing_snapshot.rows} + for patch in request.update_bom_rows or []: + row = rows_by_id.get(str(patch.id)) + if row is not None: + ids.add(row.ingredient_variant_id) + for row_id in request.delete_bom_row_ids or []: + row = rows_by_id.get(str(row_id)) + if row is not None: + ids.add(row.ingredient_variant_id) + return ids + + async def _modify_product_bom_impl(request: ManageProductBomRequest, context: Context): services = get_services(context) @@ -555,19 +586,65 @@ async def _modify_product_bom_impl(request: ManageProductBomRequest, context: Co # ``prior_state`` for manual revert if the plan partially applies # (fail-fast halts after the first error). Best-effort: a list # failure shouldn't block the modify call itself. + # + # Also resolve the parent variant + product (same path + # ``_get_product_bom_impl`` uses) so the modify card's tier-1 header + # carries ``product_name``, ``variant_sku``, ``uom`` and a + # ``katana_url``. Without these the card falls back to a bare + # "BOM for variant {id}" header — usable but not user-identifiable. + # + # The row-fetch and parent-resolution failure modes are independent + # (split try blocks): a typed-cache miss on the parent shouldn't + # discard rows we already successfully listed (loses diff context + + # revert reference). Each call falls back to its own degraded state: + # row-fetch failure → ``existing_snapshot=None`` (the dispatcher + # warns "could not fetch"); parent-resolution failure → snapshot + # with rows + None header fields (card renders the placeholder + # title but the table + revert reference stay intact). + existing_rows: list[BomRowInfo] | None = None try: existing_rows = await _fetch_bom_row_infos(services, request.id) - existing_snapshot: GetProductBomResponse | None = GetProductBomResponse( - product_variant_id=request.id, - rows=existing_rows, - total_count=len(existing_rows), - ) except asyncio.CancelledError: # Never swallow cooperative cancellation — request timeouts and # shutdown have to propagate cleanly. raise except Exception: + existing_rows = None + + existing_snapshot: GetProductBomResponse | None + if existing_rows is None: existing_snapshot = None + else: + try: + variant, product = await _resolve_parent_for_card(services, request.id) + except asyncio.CancelledError: + raise + except Exception: + variant, product = None, None + existing_snapshot = GetProductBomResponse( + product_variant_id=request.id, + rows=existing_rows, + total_count=len(existing_rows), + product_id=(getattr(product, "id", None) if product is not None else None), + product_name=( + getattr(product, "name", None) if product is not None else None + ), + variant_sku=( + getattr(variant, "sku", None) if variant is not None else None + ), + variant_display_name=( + getattr(variant, "display_name", None) if variant is not None else None + ), + is_producible=( + getattr(product, "is_producible", None) if product is not None else None + ), + uom=getattr(product, "uom", None) if product is not None else None, + katana_url=( + katana_web_url("product", product.id) + if product is not None and getattr(product, "id", None) is not None + else None + ), + ) # Adds need the parent product/material id; fetch once if needed. product_item_id: int | None = None @@ -620,6 +697,25 @@ async def _modify_product_bom_impl(request: ManageProductBomRequest, context: Co ) ) + # Resolve ingredient SKU / display_name for every variant id touched + # by the plan so ``build_bom_modify_ui`` can render user-facing row + # identities on *added* rows (the prior_state snapshot already + # resolves SKUs for existing rows via ``_fetch_bom_row_infos``). One + # batched cache lookup; misses degrade to ``(None, None)``. + ingredient_ids = _collect_ingredient_ids(request, existing_snapshot) + resolved_pairs = ( + await _resolve_ingredient_fields(services, ingredient_ids) + if ingredient_ids + else {} + ) + # Flatten the (sku, display_name) tuple into a serializable dict so + # the wire shape carries through ``model_dump`` cleanly and the + # renderer reads typed string fields. + resolved_ingredients: dict[int, dict[str, str | None]] = { + vid: {"sku": sku, "display_name": display_name} + for vid, (sku, display_name) in resolved_pairs.items() + } + # No web URL for variant-scoped BOMs (Katana's URL pattern is # /product/{id} on the product id, not the variant id; the BOM tab # is reached by navigating from the product page). Skip katana_url @@ -630,7 +726,7 @@ async def _modify_product_bom_impl(request: ManageProductBomRequest, context: Co # dispatcher's labels match ``entity_id=product_variant_id`` — the # whole BOM is the entity being modified; individual rows are the # per-action ``operation`` targets. - return await run_modify_plan( + response = await run_modify_plan( request=request, naming=EntityNaming( entity_type="product_bom", @@ -646,8 +742,119 @@ async def _modify_product_bom_impl(request: ManageProductBomRequest, context: Co # "could not fetch" warning is informative when that happens. has_get_endpoint=True, cache_merge=None, + # Thread the resolved ingredients map onto the response so + # ``build_bom_modify_ui`` can render added-row SKUs + display + # names without a second cache hit at render time. + extras={"resolved_ingredients": resolved_ingredients}, ) + # On the apply path, precompute the post-apply DataTable rows + the + # apply-outcome chrome (Tier-1 header Badge label/variant, failed- + # row Alert summary) and stuff into ``response.extras``. From there + # the apply-time call to ``build_bom_modify_ui`` seeds these into + # its OWN PrefabApp ``state.*`` slots, and the preview iframe's + # ``on_success`` SetState chain reads off ``{{ $result.state. }}`` + # (the apply tool's wire envelope is keyed by ``$prefab`` / ``view`` + # / ``state`` — not ``extras`` — which is why ``$result.state`` is + # the correct path; documented in + # ``test_apply_button_morphs_card_to_applied_state``). + # + # The slots that flow extras → state → preview-morph: + # + # - ``applied_plan_rows`` — DataTable row dicts with per-row + # APPLIED / FAILED Status decoration. + # - ``applied_outcome_label`` — Tier-1 state Badge text + # (APPLIED / PARTIAL FAILURE / FAILED). Without this the badge + # would stay frozen on the preview-time "APPLIED" default even + # when actions failed. + # - ``applied_outcome_variant`` — pairs with the label so the + # renderer picks ``default`` vs ``destructive`` based on the + # actual outcome. + # - ``applied_failed_count`` / ``applied_failed_summary`` — drives + # the consolidated failed-row Alert in the morphed state. We + # pre-format the summary string server-side (one line per failed + # row) because Prefab's Alert children are fixed at build time — + # a state-driven list of AlertDescription rows is not expressible + # in the current component vocabulary. + # + # The underscore-helper imports from prefab_ui are a known coupling + # smell — tracked in #850 for a follow-up extraction into a shared + # non-UI module. + if not response.is_preview: + from katana_mcp.tools.prefab_ui import ( + _merge_bom_rows_for_modify_card, + _prepare_bom_table_rows, + _summarize_apply_outcome, + ) + + # ``execute_plan`` is fail-fast: ``response.actions`` ends at the + # first failed action; plan entries past that point are never + # attempted and so don't appear in ``response.actions``. Without + # synthesizing them here the morphed table would silently HIDE + # the never-run rows — the user sees "1 succeeded, 1 failed" but + # the original plan was "1 succeeded, 1 failed, 3 not run". The + # not-run rows still belong on the table so the operator knows + # what's still pending. We synthesize ``succeeded=None`` entries + # for the plan tail (matches the preview "PLANNED" status, which + # is correct semantically — those rows ARE still planned). + executed_results = list(response.actions) + not_run_specs = plan[len(executed_results) :] + not_run_actions = [ + { + "operation": spec.operation, + "target_id": spec.target_id, + "succeeded": None, + "error": None, + "changes": [ + c.model_dump() if hasattr(c, "model_dump") else dict(c) + for c in spec.diff + ], + "status_label": "NOT RUN", + } + for spec in not_run_specs + ] + actions_dicts = [a.model_dump() for a in executed_results] + not_run_actions + merged = _merge_bom_rows_for_modify_card( + response.prior_state, actions_dicts, resolved_ingredients + ) + applied_plan_rows = _prepare_bom_table_rows(merged) + # Summarize against the EXECUTED actions only — "PARTIAL FAILURE" + # vs "FAILED" buckets on what was attempted, not the full plan. + outcome_label, outcome_variant = _summarize_apply_outcome( + [a.model_dump() for a in executed_results] + ) + failed_rows = [ + r for r in applied_plan_rows if r.get("status_label") == "FAILED" + ] + failed_summary_lines: list[str] = [] + for r in failed_rows: + sku = r.get("sku") or f"variant {r.get('ingredient_variant_id')}" + err = r.get("error") or "unknown error" + failed_summary_lines.append(f"Failed — {sku}: {err}") + if not_run_specs: + failed_summary_lines.append( + f"({len(not_run_specs)} planned action(s) NOT RUN — " + f"fail-fast halted the plan; re-issue manage_product_bom " + f"after fixing the failure to apply the remaining changes.)" + ) + # Footer ``applied_verb`` mirrors the Tier-1 outcome label so the + # in-place morph after Confirm reads the right verb instead of + # the build-time "applied" default. See the comment in + # ``build_bom_modify_ui``'s ``extra_on_success`` for the path. + verb_map = { + "APPLIED": "applied", + "FAILED": "failed", + "PARTIAL FAILURE": "partially applied", + } + response.extras["applied_plan_rows"] = applied_plan_rows + response.extras["applied_outcome_label"] = outcome_label + response.extras["applied_outcome_variant"] = outcome_variant + response.extras["applied_failed_count"] = len(failed_rows) + response.extras["applied_failed_summary"] = "\n".join(failed_summary_lines) + response.extras["applied_verb"] = verb_map.get(outcome_label, "applied") + + return response + @observe_tool @unpack_pydantic_params diff --git a/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py b/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py index 52a549f3..ecb7ae3b 100644 --- a/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py +++ b/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py @@ -106,9 +106,12 @@ from prefab_ui.rx import EVENT, RESULT, Rx from pydantic import BaseModel +from katana_mcp.logging import get_logger from katana_mcp.tools.tool_result_utils import BLOCK_WARNING_PREFIX from katana_mcp.web_urls import EntityKind, katana_web_url +logger = get_logger(__name__) + def _split_warnings( warnings: list[str] | None, @@ -4100,6 +4103,784 @@ def build_po_modify_ui( return app +# ============================================================================ +# BOM modify card — table-as-entity-view variant of the modify-card family +# (#811). Unlike PO/SO/MO/etc. which modify scalar header fields, a BOM +# modify plan is N row creates / row updates / row deletes on a list. The +# entity view here IS the table — each plan action projects onto a row in +# the merged before-state + planned-changes view. +# ============================================================================ + + +# Per-row status pill variants. Mirrors the action status_label vocabulary +# from ``_modification._derive_status_label`` but bucketed for Badge variants +# (success / warn / fail / neutral) so the per-row rendering is consistent +# across preview and applied states. +_BOM_ROW_STATUS_VARIANTS: dict[str, str] = { + "PLANNED": "secondary", + "APPLIED": "default", + "APPLIED (verified)": "default", + "APPLIED (verification mismatch)": "destructive", + "FAILED": "destructive", + # ``execute_plan`` is fail-fast — plan entries past the first failure + # are never attempted. ``_modify_product_bom_impl`` synthesizes these + # into the rendered ``actions`` list with status_label="NOT RUN" so + # the morphed table shows what didn't execute (instead of silently + # hiding the never-run rows). + "NOT RUN": "secondary", + # Existing rows that aren't part of the plan render with no status — + # we use an empty string so the DataTable cell stays empty rather than + # showing a misleading "PLANNED" badge for context rows. + "": "outline", +} + + +# Wire shape of an existing row in ``prior_state.rows`` — pre-serialized by +# ``serialize_for_prior_state`` from ``BomRowInfo.model_dump()``. Carries +# ``id`` (UUID string), ``ingredient_variant_id``, ``sku``, ``display_name``, +# ``quantity``, ``notes``, ``rank``. +_BomMergedRowKind = Literal["existing", "added", "updated", "deleted"] + + +def _bom_change_lookup( + changes: list[dict[str, Any]] | None, +) -> dict[str, dict[str, Any]]: + """Index an action's ``changes`` list by field name for quick lookup. + + Each entry is the raw ``FieldChange`` dict (``field``, ``old``, ``new``, + plus the ``is_*`` flags). Used by the row-merge helper to pluck out + quantity / notes / ingredient_variant_id changes per action. + """ + out: dict[str, dict[str, Any]] = {} + for c in changes or []: + if isinstance(c, dict) and isinstance(c.get("field"), str): + out[c["field"]] = c + return out + + +def _format_bom_quantity(value: Any) -> str: + """Render a BOM-row quantity for the table cell. + + Quantity is a numeric field in Katana but the wire shape may serialize + floats as strings (decimal-padded) or as Python floats. ``None`` renders + as em-dash. Floats with an integer value render without the trailing + ``.0`` (so ``2.0`` reads as ``2`` — typical recipe quantities are whole + units; the decimal only matters when it's non-trivial). + """ + if value is None: + return "—" + if isinstance(value, str): + # Strip pure-integer decimal strings like "2.0000000000". + try: + numeric: float = float(value) + except (TypeError, ValueError): + return value + if numeric.is_integer(): + return str(int(numeric)) + return f"{numeric:g}" + if isinstance(value, (int, float)): + if float(value).is_integer(): + return str(int(value)) + return f"{value:g}" + return str(value) + + +def _format_bom_quantity_diff(old: Any, new: Any, *, unknown_prior: bool) -> str: + """Render a quantity update as ``old → new`` (or ``(prior unknown) → new``). + + Used in the Quantity cell for updated rows. + """ + after = _format_bom_quantity(new) + if unknown_prior: + return f"(prior unknown) → {after}" + return f"{_format_bom_quantity(old)} → {after}" + + +def _bom_existing_row_from_snapshot(row: dict[str, Any]) -> dict[str, Any]: + """Project a snapshot row into the merge-row shape with ``kind=existing``. + + Helper for :func:`_merge_bom_rows_for_modify_card` — extracted so the + main merge stays under ruff's complexity threshold. The snapshot row + is the wire shape of ``BomRowInfo.model_dump()``. + """ + return { + "id": row.get("id"), + "ingredient_variant_id": row.get("ingredient_variant_id"), + "sku": row.get("sku"), + "display_name": row.get("display_name"), + "rank": row.get("rank"), + "rank_label": (str(row["rank"]) if isinstance(row.get("rank"), int) else "—"), + "quantity_label": _format_bom_quantity(row.get("quantity")), + "notes": row.get("notes"), + "notes_label": row.get("notes") or "—", + "kind": "existing", + "status_label": "", + "status_variant": _BOM_ROW_STATUS_VARIANTS[""], + "status_prefix": " ", + "error": None, + } + + +def _bom_synth_orphan_row(target_id: str) -> dict[str, Any]: + """Synthesize a minimal row for an update/delete target absent from the + snapshot (rare — partial fetch failure or stale id). Lets the action + surface in the table even without resolved identity context. + """ + return { + "id": target_id, + "ingredient_variant_id": None, + "sku": None, + "display_name": None, + "rank": None, + "rank_label": "—", + "quantity_label": "—", + "notes": None, + "notes_label": "—", + "kind": "existing", + "status_label": "", + "status_variant": _BOM_ROW_STATUS_VARIANTS[""], + "status_prefix": " ", + "error": None, + } + + +def _bom_add_action_to_row( + *, + changes: dict[str, dict[str, Any]], + resolved_ingredients: dict[int, dict[str, str | None]], + status_label: str, + status_variant: str, + error: str | None, +) -> dict[str, Any]: + """Synthesize an ``added``-kind merge row from an ``add_bom_row`` action.""" + ingredient_id_change = changes.get("ingredient_variant_id") + ingredient_id = ingredient_id_change.get("new") if ingredient_id_change else None + quantity_change = changes.get("quantity") + new_qty = quantity_change.get("new") if quantity_change else None + notes_change = changes.get("notes") + new_notes = notes_change.get("new") if notes_change else None + resolved = ( + resolved_ingredients.get(int(ingredient_id)) + if isinstance(ingredient_id, int) + else None + ) + return { + "id": "", # No UUID yet — server assigns on POST. + "ingredient_variant_id": ingredient_id, + "sku": (resolved or {}).get("sku"), + "display_name": (resolved or {}).get("display_name"), + "rank": None, + "rank_label": "—", + "quantity_label": _format_bom_quantity(new_qty), + "notes": new_notes, + "notes_label": new_notes or "—", + "kind": "added", + "status_label": status_label, + "status_variant": status_variant, + "status_prefix": "+ ", + "error": error, + } + + +def _apply_bom_update_to_row( + row: dict[str, Any], + *, + changes: dict[str, dict[str, Any]], + resolved_ingredients: dict[int, dict[str, str | None]], + status_label: str, + status_variant: str, + error: str | None, +) -> None: + """Decorate a snapshot row in-place with an ``update_bom_row`` action's diff. + + Flips ``kind`` to ``updated``, applies quantity/notes/ingredient_swap + decoration, and overlays the per-row status. Existing identity stays + unless the ingredient_variant_id is swapped. + """ + row["kind"] = "updated" + row["status_label"] = status_label + row["status_variant"] = status_variant + row["status_prefix"] = "~ " + row["error"] = error + qty_change = changes.get("quantity") + if qty_change is not None: + row["quantity_label"] = _format_bom_quantity_diff( + qty_change.get("old"), + qty_change.get("new"), + unknown_prior=bool(qty_change.get("is_unknown_prior")), + ) + notes_change = changes.get("notes") + if notes_change is not None: + new_notes = notes_change.get("new") + if notes_change.get("is_unknown_prior"): + row["notes_label"] = f"(prior unknown) → {new_notes or '—'}" + else: + old_notes = notes_change.get("old") or "—" + row["notes_label"] = f"{old_notes} → {new_notes or '—'}" + ingredient_change = changes.get("ingredient_variant_id") + if ingredient_change is not None: + new_ingredient = ingredient_change.get("new") + if isinstance(new_ingredient, int): + resolved = resolved_ingredients.get(new_ingredient) + row["ingredient_variant_id"] = new_ingredient + row["sku"] = (resolved or {}).get("sku") + row["display_name"] = (resolved or {}).get("display_name") + + +def _apply_bom_delete_to_row( + row: dict[str, Any], + *, + status_label: str, + status_variant: str, + error: str | None, +) -> None: + """Decorate a snapshot row in-place with a ``delete_bom_row`` action.""" + row["kind"] = "deleted" + row["status_label"] = status_label + row["status_variant"] = status_variant + row["status_prefix"] = "- " + row["error"] = error + + +def _merge_bom_rows_for_modify_card( + prior_state: dict[str, Any] | None, + actions: list[dict[str, Any]], + resolved_ingredients: dict[int, dict[str, str | None]] | None, +) -> list[dict[str, Any]]: + """Project the existing BOM snapshot + plan actions into a unified row list. + + Each row carries everything the DataTable needs: + + - ``kind``: ``existing`` | ``added`` | ``updated`` | ``deleted`` + - ``rank`` / ``rank_label``: rank for sortable column; ``rank_label`` is + the display text (``"—"`` for adds with no rank yet, the rank number + for everything else). + - ``ingredient_variant_id``, ``sku``, ``display_name``: row identity. + Adds resolve SKU / display_name from ``resolved_ingredients``; existing + rows pull from the snapshot (already resolved by ``_fetch_bom_row_infos``). + - ``quantity_label``: cell text — bare number for existing/added, + ``old → new`` diff for updated. For deleted rows the snapshot + quantity is preserved (so the user sees *what* is going away — + the ``- `` SKU-column gutter + ``deleted`` kind already signal + the action). + - ``notes`` / ``notes_label``: notes value, with diff decoration on update. + - ``status_label`` / ``status_variant``: per-row Badge text + variant. + - ``error``: failure message (None when not failed). Surfaced in the + consolidated bottom Alert; not rendered inline. + - ``status_prefix``: ``"+ "`` / ``"- "`` / ``" "`` — leading 2-char + gutter on the SKU / Display Name cells so adds and deletes are + visually distinct without depending on row-styling that Prefab + doesn't currently expose. Same layout-stability trick as + ``_render_field_diff_line``. + + Match rules: + + - delete actions are looked up by ``target_id`` (UUID string) against the + snapshot rows. + - update actions are looked up the same way; the resolved ingredient + reflects the post-patch ingredient (when the update swaps it). + - add actions are synthesized fresh from the action's ``changes`` — + they have no ``target_id`` in the plan. SKU/display_name come from + ``resolved_ingredients``; missing entries gracefully degrade to + "(unresolved)" so the row still renders with the ingredient_variant_id. + + Empty plan + empty existing → empty list (the caller renders a friendly + placeholder). The merge is robust against partial data: missing + ``prior_state`` reduces existing rows to zero but still surfaces planned + adds + updates + deletes (the latter two with no resolved row to base + off of — they degrade gracefully). + """ + resolved_ingredients = resolved_ingredients or {} + prior_rows: list[dict[str, Any]] = [] + if isinstance(prior_state, dict): + candidate = prior_state.get("rows") + if isinstance(candidate, list): + prior_rows = [r for r in candidate if isinstance(r, dict)] + + # Build a working copy keyed by UUID string for fast update/delete + # lookup. ``existing_by_id`` is mutated in place as we apply plan + # decorations, so the final pass over it yields rows in the snapshot's + # original rank order regardless of plan iteration order. + existing_by_id: dict[str, dict[str, Any]] = {} + for row in prior_rows: + row_id = row.get("id") + if not isinstance(row_id, str): + continue + existing_by_id[row_id] = _bom_existing_row_from_snapshot(row) + + added_rows: list[dict[str, Any]] = [] + + for action in actions: + op = str(action.get("operation") or "").lower() + status_label = action.get("status_label") or _derive_status_label(action) or "" + status_variant = _BOM_ROW_STATUS_VARIANTS.get(status_label, "secondary") + error = action.get("error") if action.get("succeeded") is False else None + changes = _bom_change_lookup(action.get("changes")) + target_id = action.get("target_id") + target_str = str(target_id) if target_id is not None else None + + if op == "add_bom_row": + added_rows.append( + _bom_add_action_to_row( + changes=changes, + resolved_ingredients=resolved_ingredients, + status_label=status_label, + status_variant=status_variant, + error=error, + ) + ) + continue + + if op == "update_bom_row" and target_str: + row = existing_by_id.get(target_str) + if row is None: + row = _bom_synth_orphan_row(target_str) + existing_by_id[target_str] = row + _apply_bom_update_to_row( + row, + changes=changes, + resolved_ingredients=resolved_ingredients, + status_label=status_label, + status_variant=status_variant, + error=error, + ) + continue + + if op == "delete_bom_row" and target_str: + row = existing_by_id.get(target_str) + if row is None: + row = _bom_synth_orphan_row(target_str) + existing_by_id[target_str] = row + _apply_bom_delete_to_row( + row, + status_label=status_label, + status_variant=status_variant, + error=error, + ) + continue + + # Unmatched: either an unknown ``operation`` string (future + # ``reorder_bom_rows`` etc.) or a known op with a missing + # ``target_id``. Either way the action would silently vanish from + # the rendered card — log so the gap is visible during dev. + logger.warning( + "BOM merge dropped action — unknown operation or missing target", + operation=op, + target_id=target_str, + succeeded=action.get("succeeded"), + ) + + # Materialize: existing-rows (snapshot order, then rank), then adds + # appended at the end (server assigns the rank, so we don't know + # where they slot yet). Stable sort on rank: existing rows preserve + # their snapshot order on equal ranks; adds always trail. + def _sort_key(r: dict[str, Any]) -> tuple[int, int]: + rank = r.get("rank") + return (0, rank) if isinstance(rank, int) else (1, 0) + + materialized = sorted(existing_by_id.values(), key=_sort_key) + materialized.extend(added_rows) + return materialized + + +def _bom_row_summary(rows: list[dict[str, Any]]) -> str: + """Build the ``+N added, ~M updated, -K deleted`` summary line. + + Only emits buckets that have non-zero counts so the line stays compact + on simpler plans. Returns the empty string when the plan is a no-op + (existing-only rows) — the caller skips rendering in that case. + """ + added = sum(1 for r in rows if r.get("kind") == "added") + updated = sum(1 for r in rows if r.get("kind") == "updated") + deleted = sum(1 for r in rows if r.get("kind") == "deleted") + parts: list[str] = [] + if added: + parts.append(f"+{added} added") + if updated: + parts.append(f"~{updated} updated") + if deleted: + parts.append(f"-{deleted} deleted") + return ", ".join(parts) + + +# DataTable columns for the BOM modify card. The Status column renders +# plain text (PLANNED / APPLIED / FAILED) — DataTable doesn't expose a +# Badge component per cell, so kind discrimination is carried by the +# SKU column's leading 2-char gutter (``+ ``/``- ``/``~ ``/`` ``) glued +# on in ``_prepare_bom_table_rows``. ``status_variant`` is still computed +# per merged row, but currently only consumed by the consolidated failure +# Alert (``_render_bom_failed_rows_block``), not the table itself. +_BOM_MODIFY_COLUMNS: list[DataTableColumn] = [ + DataTableColumn(key="rank_label", header="Rank", width="4rem"), + DataTableColumn(key="sku_label", header="Ingredient SKU"), + DataTableColumn(key="display_name", header="Display Name"), + DataTableColumn( + key="quantity_label", header="Quantity", align="right", width="9rem" + ), + DataTableColumn(key="notes_label", header="Notes"), + DataTableColumn(key="status_label", header="Status", width="7rem"), +] + +_BOM_MODIFY_PLAN_KEY = "plan_rows" +_BOM_MODIFY_PLAN_REF = f"{{{{ {_BOM_MODIFY_PLAN_KEY} }}}}" + + +def _prepare_bom_table_rows(merged: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Compose the final per-row dicts passed to the DataTable. + + The SKU column shows the kind prefix (``+ ``/``- ``/``~ ``/`` ``) inline + so adds vs deletes are visually obvious even without per-row styling. + The display_name cell carries the resolved name with a ``(unresolved)`` + fallback so unresolved adds (missing from cache) still render meaningfully. + """ + out: list[dict[str, Any]] = [] + for r in merged: + sku = r.get("sku") or "(unresolved)" + display_name = r.get("display_name") or "" + if not display_name and r.get("ingredient_variant_id") is not None: + display_name = f"variant {r['ingredient_variant_id']}" + out.append( + { + **r, + "sku_label": f"{r['status_prefix']}{sku}", + "display_name": display_name, + # For deleted rows, the row identity carries the strike + # semantically via the kind prefix + status pill — text- + # decoration isn't available in DataTable cells, so we + # encode the signal lexically (``- ``) instead. + } + ) + return out + + +def _resolve_bom_table_rows( + *, + is_preview: bool, + prior_state: dict[str, Any] | None, + actions: list[dict[str, Any]], + resolved_ingredients: dict[int, dict[str, str | None]], + extras: dict[str, Any], +) -> list[dict[str, Any]]: + """Pick the right source for the BOM DataTable rows. + + On the apply path ``_modify_product_bom_impl`` already ran the same + ``_merge → _prepare`` pipeline against the resolved actions and + stuffed the result into ``extras["applied_plan_rows"]`` (also visible + to the LLM via the response.content channel). Reuse it instead of + recomputing — avoids drift between two computation paths. + + Preview path has no precomputed list, so fall back to local merge. + Also fall back if the extras list is missing/malformed (defensive + — shouldn't happen with the current impl but keeps the renderer + robust to future schema changes). + """ + extras_applied_plan_rows = extras.get("applied_plan_rows") + if ( + not is_preview + and isinstance(extras_applied_plan_rows, list) + and all(isinstance(r, dict) for r in extras_applied_plan_rows) + ): + return extras_applied_plan_rows + merged_rows = _merge_bom_rows_for_modify_card( + prior_state, actions, resolved_ingredients + ) + return _prepare_bom_table_rows(merged_rows) + + +def build_bom_modify_ui( + response: dict[str, Any], + *, + confirm_request: BaseModel, + confirm_tool: str, +) -> PrefabApp: + """Build the manage-product-BOM modify card (#811). + + Table-as-entity-view variant of the modify-card family (#721). Unlike + PO/SO/MO/etc. which modify scalar header fields, a BOM modify plan is + N row creates / updates / deletes on a list. The entity view IS the + table — existing rows render unchanged, ``add_bom_row`` actions appear + as new rows with an ``added`` kind, ``update_bom_row`` actions show + the original row with diff-decorated quantity / notes / ingredient, + and ``delete_bom_row`` actions render with a ``deleted`` kind. + + The pre-action BOM snapshot arrives in ``response["prior_state"]`` + (populated by ``_modify_product_bom_impl`` via ``existing_snapshot``). + The ingredient SKU + display_name resolution for *added* rows comes + from ``response["extras"]["resolved_ingredients"]`` — the impl batches + a single cache lookup across every variant id touched by the plan, + so the renderer reads the result without a second hit at build time. + + Four-tier structure (#537): + + - Tier 1 — Identity: product name (linked to Katana), variant SKU pill, + UoM badge, state badge (PREVIEW / APPLIED / PARTIAL / FAILED). + - Tier 2 — Decision metrics: ``+N added, ~M updated, -K deleted`` + summary line above the table. + - Tier 3 — Reference: the diff-decorated DataTable. + - Tier 4 — Actions: Confirm / Cancel (preview); applied-state footer + (applied / failed / cancelled). + """ + actions = response.get("actions") or [] + is_preview = bool(response.get("is_preview", True)) + prior_state: dict[str, Any] | None = response.get("prior_state") + extras: dict[str, Any] = response.get("extras") or {} + resolved_ingredients_raw = extras.get("resolved_ingredients") or {} + # JSON keys arrive as strings (pydantic round-trips dict[int, Any] as + # str keys on the wire); coerce back to int for in-Python lookup. + resolved_ingredients: dict[int, dict[str, str | None]] = {} + for key, value in resolved_ingredients_raw.items(): + try: + int_key = int(key) + except (TypeError, ValueError): + continue + if isinstance(value, dict): + resolved_ingredients[int_key] = { + "sku": value.get("sku"), + "display_name": value.get("display_name"), + } + + table_rows = _resolve_bom_table_rows( + is_preview=is_preview, + prior_state=prior_state, + actions=actions, + resolved_ingredients=resolved_ingredients, + extras=extras, + ) + # ``_bom_row_summary`` reads ``kind`` per row which is preserved + # through ``_prepare_bom_table_rows``, so it works against + # table_rows directly. + summary_line = _bom_row_summary(table_rows) + + # Header content — pulled from the prior_state snapshot (the wire + # shape of ``GetProductBomResponse.model_dump()``, populated by + # ``_modify_product_bom_impl``). Falls back gracefully when fetch + # failed (prior_state is None). + snapshot = prior_state or {} + product_name = ( + snapshot.get("product_name") + or snapshot.get("variant_display_name") + or f"BOM for variant {response.get('entity_id')}" + ) + variant_sku = snapshot.get("variant_sku") + uom = snapshot.get("uom") + katana_url = snapshot.get("katana_url") + + # Footer ``applied_verb`` is passed to ``_render_preview_footer`` as + # a mustache template (``"{{ applied_verb }}"``); the underlying + # ``Muted(content=f"{title_prefix} {applied_verb}.")`` then resolves + # against ``state.applied_verb`` at iframe render time. This makes + # the footer body morph in lockstep with the Tier-1 state Badge — + # otherwise the build-time-fixed verb would read "BOM applied." + # even when ``applied_outcome_label`` morphed to FAILED / PARTIAL + # FAILURE on the preview iframe's in-place apply. + + n_actions = len(actions) + confirm_label = ( + f"Confirm {n_actions} BOM change{'s' if n_actions != 1 else ''}" + if n_actions + else "Confirm Changes" + ) + + # The apply response (when the iframe re-issues with preview=false) + # carries multiple precomputed extras the iframe needs to morph the + # whole applied-state chrome, not just the table rows. + # + # Data flow: + # 1. ``_modify_product_bom_impl`` (apply path) computes + # ``response.extras.applied_plan_rows``, ``applied_outcome_label``, + # ``applied_outcome_variant``, ``applied_failed_count``, and + # ``applied_failed_summary``. + # 2. The apply-time call to ``build_bom_modify_ui`` seeds these + # values into its OWN ``state.*`` slots (see ``state[...]`` block + # below). So the apply card's PrefabApp envelope carries them in + # ``state``, NOT ``extras``. + # 3. The preview iframe's ``on_success`` SetState chain reads + # ``{{ $result.state. }}`` and writes into the preview + # iframe's matching state slot, morphing the rendered chrome. + # + # Without the SetStates below the preview iframe would morph + # ``state.applied=True`` but every other applied-state visual would + # stay frozen at preview-time defaults: + # - DataTable stuck on PLANNED rows. + # - Header Badge stuck on "APPLIED" / default even when some or all + # actions failed. + # - Failed-row Alert never rendered because the build-time guard + # was ``if not is_preview`` (which is False under the preview + # render). Now we render the Alert under ``If("applied")`` driven + # by ``state.applied_failed_count`` and its summary description. + # + # ``$result`` in the on_success Rx context resolves to the apply + # tool's wire-shape ``structured_content`` — a PrefabApp envelope + # keyed by ``$prefab`` / ``view`` / ``state``, NOT the raw + # :class:`~katana_mcp.tools._modification.ModificationResponse`. Same + # limitation documented in ``test_apply_button_morphs_card_to_applied_state``. + # So we read from ``$result.state.`` — never ``$result.extras``, + # which doesn't exist at the envelope's top level. + apply_action = _build_apply_action( + confirm_tool, + confirm_request, + extra_on_success=[ + SetState(_BOM_MODIFY_PLAN_KEY, "{{ $result.state.plan_rows }}"), + SetState( + "applied_outcome_label", + "{{ $result.state.applied_outcome_label }}", + ), + SetState( + "applied_outcome_variant", + "{{ $result.state.applied_outcome_variant }}", + ), + SetState( + "applied_failed_count", + "{{ $result.state.applied_failed_count }}", + ), + SetState( + "applied_failed_summary", + "{{ $result.state.applied_failed_summary }}", + ), + SetState( + "applied_verb", + "{{ $result.state.applied_verb }}", + ), + ], + ) + cancel_action = _build_cancel_action("those BOM changes") + + state = _init_modify_card_state(response) + state[_BOM_MODIFY_PLAN_KEY] = table_rows + # Seed the apply-outcome state slots with preview-time defaults so the + # If-branches below render cleanly even before the apply lands. The + # apply ``on_success`` chain overwrites these with the real outcome + # values from ``$result.state.`` (read off the apply tool's + # envelope — see the rationale on ``extra_on_success`` above). + state["applied_outcome_label"] = ( + response.get("extras", {}).get("applied_outcome_label") or "APPLIED" + ) + state["applied_outcome_variant"] = ( + response.get("extras", {}).get("applied_outcome_variant") or "default" + ) + state["applied_failed_count"] = ( + response.get("extras", {}).get("applied_failed_count") or 0 + ) + state["applied_failed_summary"] = ( + response.get("extras", {}).get("applied_failed_summary") or "" + ) + state["applied_verb"] = response.get("extras", {}).get("applied_verb") or "applied" + + with PrefabApp(state=state, css_class="p-4") as app, Card(): + # Tier 1 — Identity header. Same shape as the read-side BOM card + # (``_bom_header_section``): product name linked to Katana, SKU + # pill, UoM badge, then the state badge so the modify-vs-preview + # signal lives in the same visual slot as the other modify cards. + # + # The applied-state Badge fans out across three branches so the + # variant tracks the outcome (default for success, destructive + # for PARTIAL FAILURE / FAILED). Badge.variant isn't reactive on + # its own — we render parallel components and let the If chain + # pick the right one at morph time. + with CardHeader(), Row(gap=2): + with CardTitle(): + if katana_url: + Link(content=product_name, href=katana_url, target="_blank") + else: + Text(content=product_name) + if variant_sku: + Badge(label=variant_sku, variant="outline") + if uom: + Badge(label=uom, variant="secondary") + with If("applied"): + with If(Rx("applied_outcome_variant") == "destructive"): + Badge( + label="{{ applied_outcome_label }}", + variant="destructive", + css_class="min-w-32 text-center", + ) + with Else(): + Badge( + label="{{ applied_outcome_label }}", + variant="default", + css_class="min-w-32 text-center", + ) + with Else(): + Badge( + label="PREVIEW", + variant="secondary", + css_class="min-w-32 text-center", + ) + + # Tier 2 — Decision summary. Reads "+N added, ~M updated, -K + # deleted" so the user knows the shape of the plan before + # scanning the table. + with CardContent(), Column(gap=3): + if response.get("message"): + Muted(content=response["message"]) + if summary_line: + Text(content=summary_line) + + # Tier 3 — Diff-decorated table. Bound to ``state.plan_rows`` + # via mustache (the same contract every state-bound DataTable + # in this module uses; bare-string refs crash the renderer). + if table_rows: + DataTable( + columns=_BOM_MODIFY_COLUMNS, + rows=_BOM_MODIFY_PLAN_REF, + paginated=True, + pageSize=20, + ) + else: + Muted( + content=( + "No rows in the BOM and no planned changes. " + "Provide add_bom_rows / update_bom_rows / " + "delete_bom_row_ids to manage the recipe." + ) + ) + + block_warnings = _render_warnings_block(response.get("warnings")) + + # Consolidated failed-rows Alert — driven by state so the + # preview iframe's in-place morph after Confirm can surface + # failures (the previous ``if not is_preview`` guard was + # build-time and stayed False through the morph). Pre-formatted + # ``applied_failed_summary`` string seeded into the apply + # card's ``state`` slot by the builder below; the morph picks + # it up via SetState from ``$result.state.applied_failed_summary``. + # Each line is ``Failed — : ``. + with ( + If(Rx("applied_failed_count") > 0), + Alert(variant="destructive", icon="circle-alert"), + ): + AlertTitle(content="{{ applied_failed_count }} failed row(s)") + AlertDescription(content="{{ applied_failed_summary }}") + + if is_preview: + with If("error"): + Separator() + with Alert(variant="destructive", icon="circle-alert"): + AlertTitle(content="Apply failed") + AlertDescription(content="{{ error }}") + + # Tier 4 — Footer. Reuse the shared preview-footer helper so the + # apply/cancel/morph state machine and the next-action buttons + # match the rest of the modify-card family. No next-action + # buttons today — BOM operations don't have a deterministic + # follow-up tool (the user already had the recipe they wanted to + # change). + _render_preview_footer( + title_prefix="BOM", + block_warnings=block_warnings, + confirm_label=confirm_label, + apply_action=apply_action, + cancel_action=cancel_action, + next_action_buttons=(), + # Mustache template against ``state.applied_verb`` (seeded + # above + overwritten by the on_success chain) so the footer + # body morphs to "BOM partially applied." / "BOM failed." in + # lockstep with the Tier-1 outcome Badge. + applied_verb="{{ applied_verb }}", + ) + return app + + def build_so_create_ui( response: dict[str, Any], *, diff --git a/katana_mcp_server/tests/browser/render_test_server.py b/katana_mcp_server/tests/browser/render_test_server.py index 254f31b4..6f1e6e34 100644 --- a/katana_mcp_server/tests/browser/render_test_server.py +++ b/katana_mcp_server/tests/browser/render_test_server.py @@ -27,6 +27,7 @@ ) from katana_mcp.tools.prefab_ui import ( build_batch_recipe_update_ui, + build_bom_modify_ui, build_customer_create_ui, build_inventory_at_ui, build_inventory_check_ui, @@ -769,6 +770,160 @@ def _stock_adjustment_delete_response(*, is_preview: bool) -> dict: } +def _bom_modify_response(*, is_preview: bool, succeeded: bool | None) -> dict: + """Build a canned BOM modify response with adds + updates + deletes. + + Mirrors the production shape for the #811 BOM modify card: 5+ mixed + actions (1 add + 1 update + 2 delete + 3 existing-untouched), the + pre-action snapshot threaded as ``prior_state``, and + ``resolved_ingredients`` in ``extras`` so ``build_bom_modify_ui`` + can render the added row's SKU + display name. + """ + existing_rows = [ + { + "id": "11111111-1111-1111-1111-111111111111", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 401, + "sku": "BLT-M5-10", + "display_name": "M5 chainring bolt", + "quantity": 6.0, + "notes": None, + "rank": 10000, + }, + { + "id": "22222222-2222-2222-2222-222222222222", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 402, + "sku": "BLT-M6-12", + "display_name": "M6 hex screw", + "quantity": 4.0, + "notes": "optional", + "rank": 20000, + }, + { + "id": "33333333-3333-3333-3333-333333333333", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 403, + "sku": "FRM-AL-140", + "display_name": "Aluminum 140 frame raw", + "quantity": 1.0, + "notes": None, + "rank": 30000, + }, + { + "id": "44444444-4444-4444-4444-444444444444", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 404, + "sku": "PNT-MTB", + "display_name": "Matte black paint", + "quantity": 2.0, + "notes": None, + "rank": 40000, + }, + { + "id": "55555555-5555-5555-5555-555555555555", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 405, + "sku": "DCL-MAYHEM", + "display_name": "Mayhem decal sheet", + "quantity": 1.0, + "notes": None, + "rank": 50000, + }, + ] + actions = [ + # Add: new chain pin ingredient. + ActionResult( + index=1, + operation="add_bom_row", + target_id=None, + changes=[ + FieldChange( + field="ingredient_variant_id", + old=None, + new=301, + is_added=True, + ), + FieldChange(field="quantity", old=None, new=2.0, is_added=True), + ], + succeeded=succeeded, + ).model_dump(), + # Update: bump chainring bolt quantity from 6 → 8. + ActionResult( + index=2, + operation="update_bom_row", + target_id="11111111-1111-1111-1111-111111111111", + changes=[ + FieldChange( + field="quantity", + old=None, + new=8.0, + is_unknown_prior=True, + ), + ], + succeeded=succeeded, + verified=True if succeeded else None, + ).model_dump(), + # Delete: drop the paint row. + ActionResult( + index=3, + operation="delete_bom_row", + target_id="44444444-4444-4444-4444-444444444444", + changes=[], + succeeded=succeeded, + ).model_dump(), + # Delete: drop the decal row. + ActionResult( + index=4, + operation="delete_bom_row", + target_id="55555555-5555-5555-5555-555555555555", + changes=[], + succeeded=succeeded, + ).model_dump(), + ] + return { + "entity_type": "product_bom", + "entity_id": 200, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions, + "prior_state": { + "product_variant_id": 200, + "product_id": 100, + "product_name": "Mayhem 140 Frame", + "variant_sku": "MA14025RTLG", + "variant_display_name": "Mayhem 140 Frame / Large", + "is_producible": True, + "uom": "pcs", + "katana_url": "https://factory.katanamrp.com/product/100", + "total_count": 5, + "rows": existing_rows, + }, + "extras": { + "resolved_ingredients": { + 301: {"sku": "FS90250", "display_name": "M5 chain pin"}, + 401: {"sku": "BLT-M5-10", "display_name": "M5 chainring bolt"}, + 402: {"sku": "BLT-M6-12", "display_name": "M6 hex screw"}, + 403: {"sku": "FRM-AL-140", "display_name": "Aluminum 140 frame raw"}, + 404: {"sku": "PNT-MTB", "display_name": "Matte black paint"}, + 405: {"sku": "DCL-MAYHEM", "display_name": "Mayhem decal sheet"}, + }, + }, + "warnings": [], + "next_actions": [], + "katana_url": None, + "message": ( + "Preview: 4 action(s) planned" if is_preview else "Applied 4 action(s)" + ), + } + + SCENARIOS: dict[str, Callable[[], PrefabApp]] = { # The bug-repro: 12 mixed actions on the preview card. Pre-fix this # rendered as a blank iframe; post-fix it renders one DataTable with 12 @@ -904,6 +1059,19 @@ def _stock_adjustment_delete_response(*, is_preview: bool) -> dict: confirm_request=_StubRequest(), confirm_tool="modify_item", ), + # #811 — BOM modify card with adds + updates + deletes against a + # realistic 5-row existing recipe. Pins the row-content + status-pill + # contract end-to-end in a real browser render. + "bom_modify_mixed_preview": lambda: build_bom_modify_ui( + _bom_modify_response(is_preview=True, succeeded=None), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ), + "bom_modify_mixed_applied": lambda: build_bom_modify_ui( + _bom_modify_response(is_preview=False, succeeded=True), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ), } diff --git a/katana_mcp_server/tests/browser/test_modification_render.py b/katana_mcp_server/tests/browser/test_modification_render.py index ba0dde7a..eebee47f 100644 --- a/katana_mcp_server/tests/browser/test_modification_render.py +++ b/katana_mcp_server/tests/browser/test_modification_render.py @@ -71,6 +71,48 @@ def test_modify_twelve_actions_applied_renders(self, render_scenario): # Every action shows APPLIED (verified). assert frame.locator("td").filter(has_text="APPLIED").count() == 12 + def test_bom_modify_mixed_preview_renders_all_kinds(self, render_scenario): + """#811 BOM modify card — a 5-row recipe + (1 add + 1 update + 2 + delete) plan renders the full table with every kind visible: + existing rows untouched, added row's resolved SKU surfaces, + updated row shows the diff arrow, deleted rows carry their + original SKU + a PLANNED status. + """ + frame = render_scenario("bom_modify_mixed_preview") + assert frame.locator("table").count() == 1 + # Header product identity. + assert frame.locator("text=Mayhem 140 Frame").count() >= 1 + # Added row's resolved SKU (the user-centric piece — #811's + # acceptance criterion: no "Add Bom Row" placeholder). + assert frame.locator("td").filter(has_text="FS90250").count() >= 1 + # Existing-untouched SKUs still in the table (frame raw + 2 + # surviving rows). + assert frame.locator("td").filter(has_text="FRM-AL-140").count() >= 1 + # Deleted SKUs (still in the table with deleted kind). + assert frame.locator("td").filter(has_text="PNT-MTB").count() >= 1 + # Plan summary line shows the per-kind tally. + assert frame.locator("text=+1 added").count() >= 1 + assert frame.locator("text=~1 updated").count() >= 1 + assert frame.locator("text=-2 deleted").count() >= 1 + # Confirm button (preview state). + assert frame.locator("button").filter(has_text="Confirm").count() == 1 + # Anti-pattern guard: no internal-ActionResult-model labels. + assert frame.locator("td").filter(has_text="Add Bom Row").count() == 0 + assert frame.locator("td").filter(has_text="Update Bom Row").count() == 0 + assert frame.locator("td").filter(has_text="field(s) set").count() == 0 + + def test_bom_modify_mixed_applied_renders_per_row_status(self, render_scenario): + """Result card after a successful apply — every plan-derived row + shows APPLIED in the Status column. Existing-untouched rows show + no status (empty cell). Aggregate header badge says APPLIED. + """ + frame = render_scenario("bom_modify_mixed_applied") + assert frame.locator("table").count() == 1 + # Per-row APPLIED pills land in the Status column. The plan has + # 4 actions (1 add + 1 update + 2 delete), so 4 APPLIED cells in + # the rendered table. + assert frame.locator("td").filter(has_text="APPLIED").count() >= 4 + def test_apply_button_morphs_card_to_applied_state(self, render_scenario): """Click-through: Confirm fires the apply call, the on_success chain flips ``state.applied=True``, and the button morphs from diff --git a/katana_mcp_server/tests/test_prefab_ui.py b/katana_mcp_server/tests/test_prefab_ui.py index 97a89e4d..019b8c62 100644 --- a/katana_mcp_server/tests/test_prefab_ui.py +++ b/katana_mcp_server/tests/test_prefab_ui.py @@ -15,7 +15,9 @@ from katana_mcp.tools.prefab_ui import ( PREVIEW_APPLY_COACHING, _format_money, + _merge_bom_rows_for_modify_card, build_batch_recipe_update_ui, + build_bom_modify_ui, build_fulfill_preview_ui, build_fulfill_success_ui, build_inventory_check_batch_ui, @@ -2744,6 +2746,762 @@ def test_state_seeds_result_on_applied_path(self): assert result["entity_id"] == 9001 +class TestMergeBomRowsForModifyCard: + """``_merge_bom_rows_for_modify_card`` projects ``prior_state.rows`` + + plan actions into the row-shape the BOM modify DataTable consumes. + These tests pin the kind-discriminator + cell-decoration contract so + downstream renderer changes don't regress the user-facing shape. + """ + + _EXISTING_ROW_A: ClassVar[dict[str, Any]] = { + "id": "11111111-1111-1111-1111-111111111111", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 401, + "sku": "BLT-M5-10", + "display_name": "M5 chainring bolt", + "quantity": 6.0, + "notes": None, + "rank": 10000, + } + _EXISTING_ROW_B: ClassVar[dict[str, Any]] = { + "id": "22222222-2222-2222-2222-222222222222", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 402, + "sku": "BLT-M6-12", + "display_name": "M6 hex screw", + "quantity": 4.0, + "notes": "optional", + "rank": 20000, + } + _PRIOR_STATE: ClassVar[dict[str, Any]] = { + "rows": [_EXISTING_ROW_A, _EXISTING_ROW_B], + } + _RESOLVED: ClassVar[dict[int, dict[str, str | None]]] = { + 301: {"sku": "FS90250", "display_name": "M5 chain pin"}, + 401: {"sku": "BLT-M5-10", "display_name": "M5 chainring bolt"}, + 402: {"sku": "BLT-M6-12", "display_name": "M6 hex screw"}, + } + + def test_existing_rows_carry_existing_kind_with_empty_status(self): + """No actions → both rows render as existing-untouched with empty + status (the per-row Status badge would mislead if it said + PLANNED/APPLIED for rows that aren't part of the plan).""" + rows = _merge_bom_rows_for_modify_card(self._PRIOR_STATE, [], self._RESOLVED) + assert len(rows) == 2 + assert all(r["kind"] == "existing" for r in rows) + assert all(r["status_label"] == "" for r in rows) + + def test_add_row_synthesized_with_resolved_sku_and_display_name(self): + """``add_bom_row`` actions have no target_id — the row is synthesized + from the action's changes; SKU/display_name come from + ``resolved_ingredients``. The row appends after existing rows.""" + action = { + "operation": "add_bom_row", + "target_id": None, + "succeeded": None, + "changes": [ + { + "field": "ingredient_variant_id", + "old": None, + "new": 301, + "is_added": True, + }, + {"field": "quantity", "old": None, "new": 2.0, "is_added": True}, + ], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + assert len(rows) == 3 + # Adds trail existing rows. + added = rows[-1] + assert added["kind"] == "added" + assert added["sku"] == "FS90250" + assert added["display_name"] == "M5 chain pin" + assert added["quantity_label"] == "2" + assert added["status_label"] == "PLANNED" + assert added["status_prefix"] == "+ " + + def test_add_row_with_unresolved_ingredient_degrades_to_null_sku(self): + """If the cache miss prevented resolution, the row still renders + with the ingredient_variant_id so the user has *something* — + ``sku=None`` and ``display_name=None`` mean the SKU column shows + "(unresolved)" via the prepare-rows step (covered in builder + tests). Here we pin the underlying merge contract.""" + action = { + "operation": "add_bom_row", + "target_id": None, + "succeeded": None, + "changes": [ + { + "field": "ingredient_variant_id", + "old": None, + "new": 999, + "is_added": True, + }, + {"field": "quantity", "old": None, "new": 1.0, "is_added": True}, + ], + } + rows = _merge_bom_rows_for_modify_card(self._PRIOR_STATE, [action], {}) + added = rows[-1] + assert added["kind"] == "added" + assert added["ingredient_variant_id"] == 999 + assert added["sku"] is None + assert added["display_name"] is None + + def test_update_row_decorates_quantity_with_unknown_prior(self): + """``update_bom_row`` actions emit ``is_unknown_prior=True`` + (BOM has no GET-by-id). The merged row reflects the targeted + existing row's identity with a diff-decorated quantity cell.""" + action = { + "operation": "update_bom_row", + "target_id": "11111111-1111-1111-1111-111111111111", + "succeeded": None, + "changes": [ + { + "field": "quantity", + "old": None, + "new": 8.0, + "is_unknown_prior": True, + }, + ], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + updated = next(r for r in rows if r["kind"] == "updated") + # Identity preserved from the snapshot. + assert updated["sku"] == "BLT-M5-10" + assert updated["display_name"] == "M5 chainring bolt" + # Quantity diff renders with prior-unknown prefix. + assert updated["quantity_label"] == "(prior unknown) → 8" + assert updated["status_label"] == "PLANNED" + assert updated["status_prefix"] == "~ " + + def test_update_row_with_ingredient_swap_surfaces_new_sku(self): + """Patching ``ingredient_variant_id`` swaps the row's SKU / + display_name to the post-patch identity so users see what the + row will *become*. Existing rank + status reflect the patch.""" + action = { + "operation": "update_bom_row", + "target_id": "11111111-1111-1111-1111-111111111111", + "succeeded": None, + "changes": [ + { + "field": "ingredient_variant_id", + "old": None, + "new": 301, + "is_unknown_prior": True, + }, + ], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + updated = next(r for r in rows if r["kind"] == "updated") + assert updated["sku"] == "FS90250" + assert updated["display_name"] == "M5 chain pin" + + def test_delete_row_flips_kind_and_status(self): + """``delete_bom_row`` actions flip the matched row's kind to + ``deleted`` and pull the action's status_label onto it. The row's + original identity stays so the user sees *what* is going away.""" + action = { + "operation": "delete_bom_row", + "target_id": "22222222-2222-2222-2222-222222222222", + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + deleted = next(r for r in rows if r["kind"] == "deleted") + assert deleted["sku"] == "BLT-M6-12" # original identity preserved + assert deleted["status_label"] == "PLANNED" + assert deleted["status_prefix"] == "- " + + def test_applied_failure_carries_error_per_row(self): + """When an action fails, its row carries the FAILED status and the + error message (rendered in the consolidated bottom Alert; not + inline). Verified ``True`` vs ``None`` is not relevant here — + only succeeded/error/status_label.""" + action = { + "operation": "update_bom_row", + "target_id": "11111111-1111-1111-1111-111111111111", + "succeeded": False, + "error": "422 Unprocessable: quantity must be > 0", + "changes": [ + {"field": "quantity", "old": None, "new": -1, "is_unknown_prior": True} + ], + "status_label": "FAILED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + failed = next(r for r in rows if r["status_label"] == "FAILED") + assert failed["kind"] == "updated" + assert failed["error"] == "422 Unprocessable: quantity must be > 0" + assert failed["status_variant"] == "destructive" + + def test_empty_prior_state_with_only_adds_renders_added_rows_only(self): + """A BOM that doesn't exist yet (or fetch failed) with an add-only + plan → only the added rows surface; no existing rows.""" + action = { + "operation": "add_bom_row", + "target_id": None, + "succeeded": None, + "changes": [ + { + "field": "ingredient_variant_id", + "old": None, + "new": 301, + "is_added": True, + }, + {"field": "quantity", "old": None, "new": 5.0, "is_added": True}, + ], + } + rows = _merge_bom_rows_for_modify_card(None, [action], self._RESOLVED) + assert len(rows) == 1 + assert rows[0]["kind"] == "added" + assert rows[0]["sku"] == "FS90250" + + def test_empty_plan_with_no_existing_rows_returns_empty(self): + """No prior + no plan → empty list (caller renders a placeholder).""" + rows = _merge_bom_rows_for_modify_card(None, [], {}) + assert rows == [] + + def test_existing_rows_preserve_rank_order(self): + """Snapshot order (by rank) must survive the merge — DataTable + renders rows in input order; users expect rank-1 above rank-2.""" + reversed_prior = { + "rows": [self._EXISTING_ROW_B, self._EXISTING_ROW_A] # rank 20000, 10000 + } + rows = _merge_bom_rows_for_modify_card(reversed_prior, [], self._RESOLVED) + # Sorted by rank ascending — A before B. + assert rows[0]["sku"] == "BLT-M5-10" + assert rows[1]["sku"] == "BLT-M6-12" + + def test_update_row_with_orphan_target_id_synthesizes_placeholder(self): + """``update_bom_row`` targeting an id not in the snapshot still + surfaces — the merge synthesizes a placeholder so the action is + visible. Rare in practice (stale snapshot or partial fetch), + but the alternative (silent drop) would be worse.""" + orphan_id = "99999999-9999-9999-9999-999999999999" + action = { + "operation": "update_bom_row", + "target_id": orphan_id, + "succeeded": None, + "changes": [ + { + "field": "quantity", + "old": None, + "new": 3.0, + "is_unknown_prior": True, + }, + ], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + # 2 existing + 1 synthesized orphan with updated decoration. + assert len(rows) == 3 + orphan = next(r for r in rows if r["id"] == orphan_id) + assert orphan["kind"] == "updated" + assert orphan["sku"] is None # no identity to resolve from + assert orphan["display_name"] is None + assert orphan["quantity_label"] == "(prior unknown) → 3" + assert orphan["status_label"] == "PLANNED" + assert orphan["status_prefix"] == "~ " + + def test_delete_row_with_orphan_target_id_synthesizes_placeholder(self): + """``delete_bom_row`` targeting an id not in the snapshot still + surfaces — same rationale as the update-orphan case.""" + orphan_id = "99999999-9999-9999-9999-999999999999" + action = { + "operation": "delete_bom_row", + "target_id": orphan_id, + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + assert len(rows) == 3 + orphan = next(r for r in rows if r["id"] == orphan_id) + assert orphan["kind"] == "deleted" + assert orphan["sku"] is None + assert orphan["status_label"] == "PLANNED" + assert orphan["status_prefix"] == "- " + + def test_all_deletes_plan_renders_every_row_as_deleted(self): + """A "clear the recipe" plan — every existing row marked for + deletion. Symmetric inverse of the all-adds-to-empty-BOM case; + validates that the merge handles homogeneous delete plans.""" + delete_a = { + "operation": "delete_bom_row", + "target_id": "11111111-1111-1111-1111-111111111111", + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + } + delete_b = { + "operation": "delete_bom_row", + "target_id": "22222222-2222-2222-2222-222222222222", + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + } + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [delete_a, delete_b], self._RESOLVED + ) + assert len(rows) == 2 + assert all(r["kind"] == "deleted" for r in rows) + assert all(r["status_prefix"] == "- " for r in rows) + # Identity preserved so the user sees *what* is being removed. + skus = {r["sku"] for r in rows} + assert skus == {"BLT-M5-10", "BLT-M6-12"} + + def test_unknown_operation_is_logged_and_dropped(self, caplog): + """Future operation strings (e.g. ``reorder_bom_rows``) that the + merge doesn't know about should emit a warning rather than + silently vanishing. Surfaces gaps during dev when a new op lands + on the planner side without a renderer update. + """ + import logging + + from katana_mcp.logging import setup_logging + + # Force structlog through stdlib + JSONRenderer so caplog sees the + # emitted warning. Without this the test depends on whether some + # earlier test in the same xdist worker happened to call + # ``setup_logging`` — flaky between sequential and parallel runs. + setup_logging(log_level="WARNING", log_format="json") + action = { + "operation": "reorder_bom_rows", + "target_id": "11111111-1111-1111-1111-111111111111", + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + } + with caplog.at_level(logging.WARNING, logger="katana_mcp.tools.prefab_ui"): + rows = _merge_bom_rows_for_modify_card( + self._PRIOR_STATE, [action], self._RESOLVED + ) + # The unknown action doesn't pollute the rendered rows. + assert len(rows) == 2 + assert all(r["kind"] == "existing" for r in rows) + # The merge logs a warning so the dev sees the dropped action. + matched = [ + rec + for rec in caplog.records + if rec.name == "katana_mcp.tools.prefab_ui" + and rec.levelname == "WARNING" + and "reorder_bom_rows" in rec.getMessage() + ] + assert matched, ( + "Expected a WARNING log record mentioning reorder_bom_rows; " + f"saw {len(caplog.records)} record(s): " + f"{[(r.name, r.levelname, r.getMessage()[:80]) for r in caplog.records]}" + ) + + +class TestBuildBOMModifyUI: + """``build_bom_modify_ui`` (#811) — the diff-decorated BOM modify card. + + Table-as-entity-view variant of the modify-card family: existing rows + render unchanged, plan adds appear with a ``+`` prefix, plan updates + show ``old → new`` quantity diffs, plan deletes carry a ``-`` prefix + + FAILED/APPLIED status. The summary line under the header reads + ``+N added, ~M updated, -K deleted``. + """ + + _BOM_PRIOR: ClassVar[dict[str, Any]] = { + "product_variant_id": 200, + "product_id": 100, + "product_name": "Mayhem 140 Frame", + "variant_sku": "MA14025RTLG", + "variant_display_name": "Mayhem 140 Frame / Large", + "is_producible": True, + "uom": "pcs", + "katana_url": "https://factory.katanamrp.com/product/100", + "total_count": 2, + "rows": [ + { + "id": "11111111-1111-1111-1111-111111111111", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 401, + "sku": "BLT-M5-10", + "display_name": "M5 chainring bolt", + "quantity": 6.0, + "notes": None, + "rank": 10000, + }, + { + "id": "22222222-2222-2222-2222-222222222222", + "product_item_id": 100, + "product_variant_id": 200, + "ingredient_variant_id": 402, + "sku": "BLT-M6-12", + "display_name": "M6 hex screw", + "quantity": 4.0, + "notes": "optional", + "rank": 20000, + }, + ], + } + _RESOLVED: ClassVar[dict[int, dict[str, str | None]]] = { + 301: {"sku": "FS90250", "display_name": "M5 chain pin"}, + 302: {"sku": "FS90251", "display_name": "M6 hex screw v2"}, + 401: {"sku": "BLT-M5-10", "display_name": "M5 chainring bolt"}, + 402: {"sku": "BLT-M6-12", "display_name": "M6 hex screw"}, + } + + @classmethod + def _preview( + cls, + actions: list[dict[str, Any]] | None = None, + **overrides: Any, + ) -> dict[str, Any]: + return { + "entity_type": "product_bom", + "entity_id": 200, + "is_preview": True, + "actions": actions or [], + "prior_state": dict(cls._BOM_PRIOR), + "extras": {"resolved_ingredients": dict(cls._RESOLVED)}, + "warnings": [], + "next_actions": [], + "message": "Preview", + "katana_url": None, + **overrides, + } + + @classmethod + def _applied( + cls, + actions: list[dict[str, Any]] | None = None, + **overrides: Any, + ) -> dict[str, Any]: + from katana_mcp.tools.prefab_ui import ( + _merge_bom_rows_for_modify_card, + _prepare_bom_table_rows, + _summarize_apply_outcome, + ) + + actions_list = actions or [] + merged = _merge_bom_rows_for_modify_card( + cls._BOM_PRIOR, actions_list, cls._RESOLVED + ) + applied_plan_rows = _prepare_bom_table_rows(merged) + outcome_label, outcome_variant = _summarize_apply_outcome(actions_list) + failed_rows = [ + r for r in applied_plan_rows if r.get("status_label") == "FAILED" + ] + summary_lines: list[str] = [] + for r in failed_rows: + sku = r.get("sku") or f"variant {r.get('ingredient_variant_id')}" + err = r.get("error") or "unknown error" + summary_lines.append(f"Failed — {sku}: {err}") + + base = cls._preview(actions_list, is_preview=False, **overrides) + # Mirror what ``_modify_product_bom_impl`` packs into + # ``response.extras`` on the apply branch so the state-driven + # Tier-1 Badge + failed-row Alert have the same data they'd get + # in production. Callers that pass an explicit ``extras=`` retain + # full control. + if "extras" not in overrides: + base["extras"] = { + **base["extras"], + "applied_plan_rows": applied_plan_rows, + "applied_outcome_label": outcome_label, + "applied_outcome_variant": outcome_variant, + "applied_failed_count": len(failed_rows), + "applied_failed_summary": "\n".join(summary_lines), + } + return base + + @staticmethod + def _add_action(*, ingredient_id: int, quantity: float, **overrides: Any) -> dict: + return { + "operation": "add_bom_row", + "target_id": None, + "succeeded": None, + "changes": [ + { + "field": "ingredient_variant_id", + "old": None, + "new": ingredient_id, + "is_added": True, + }, + { + "field": "quantity", + "old": None, + "new": quantity, + "is_added": True, + }, + ], + "status_label": "PLANNED", + **overrides, + } + + @staticmethod + def _update_action( + *, target_id: str, new_quantity: float, **overrides: Any + ) -> dict: + return { + "operation": "update_bom_row", + "target_id": target_id, + "succeeded": None, + "changes": [ + { + "field": "quantity", + "old": None, + "new": new_quantity, + "is_unknown_prior": True, + }, + ], + "status_label": "PLANNED", + **overrides, + } + + @staticmethod + def _delete_action(*, target_id: str, **overrides: Any) -> dict: + return { + "operation": "delete_bom_row", + "target_id": target_id, + "succeeded": None, + "changes": [], + "status_label": "PLANNED", + **overrides, + } + + def test_preview_with_mixed_plan_renders_all_kinds_and_summary(self): + """The canonical multi-kind scenario: 1 add + 1 update + 1 delete + + 2 existing untouched. Pins every kind of row appearing in the card + and the summary line counting them.""" + actions = [ + self._add_action(ingredient_id=301, quantity=2.0), + self._update_action( + target_id="11111111-1111-1111-1111-111111111111", new_quantity=8.0 + ), + self._delete_action(target_id="22222222-2222-2222-2222-222222222222"), + ] + app = build_bom_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + _assert_valid_prefab(app) + rendered = str(app.to_json()) + # Header carries the product identity. + assert "Mayhem 140 Frame" in rendered + assert "MA14025RTLG" in rendered + assert "pcs" in rendered + # Added row's resolved SKU surfaces (the user-centric piece). + assert "FS90250" in rendered + assert "M5 chain pin" in rendered + # Updated row's quantity diff arrow. + assert "(prior unknown) → 8" in rendered + # Deleted row's original identity surfaces. + assert "BLT-M6-12" in rendered + # Summary line. + assert "+1 added" in rendered + assert "~1 updated" in rendered + assert "-1 deleted" in rendered + + def test_added_row_with_unresolved_ingredient_degrades_gracefully(self): + """If the cache miss prevented resolution (extras lookup is empty), + the added row shows ``(unresolved)`` in the SKU column and falls + back to ``variant `` in the display name — the user still sees + *something* to identify the row by.""" + actions = [self._add_action(ingredient_id=99999, quantity=1.0)] + app = build_bom_modify_ui( + self._preview(actions, extras={"resolved_ingredients": {}}), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + assert "(unresolved)" in rendered + assert "variant 99999" in rendered + + def test_applied_state_renders_per_row_status_pills(self): + """The applied card carries per-row APPLIED / FAILED pills on each + plan-derived row. The card-level state badge tracks the aggregate + outcome (APPLIED for all-success, FAILED for all-failure, PARTIAL + FAILURE for mixed).""" + actions = [ + self._add_action( + ingredient_id=301, + quantity=2.0, + succeeded=True, + status_label="APPLIED", + ), + self._update_action( + target_id="11111111-1111-1111-1111-111111111111", + new_quantity=8.0, + succeeded=True, + status_label="APPLIED", + ), + ] + app = build_bom_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + # Aggregate header badge. + assert "APPLIED" in rendered + # Per-row status pills land in plan_rows state so they ride the + # DataTable rows binding. Two rows = two APPLIED pills minimum. + state_rows = app.state.get("plan_rows") if app.state else [] + assert isinstance(state_rows, list) + statuses = [r["status_label"] for r in state_rows] + assert statuses.count("APPLIED") == 2 + + def test_partial_failure_surfaces_failed_error_in_alert(self): + """A mixed applied outcome (1 success + 1 fail) renders the + consolidated failed-rows Alert at the bottom of the table so the + actual error message reaches the user without crowding the row + cells. The row's per-row pill says FAILED.""" + actions = [ + self._add_action( + ingredient_id=301, + quantity=2.0, + succeeded=True, + status_label="APPLIED", + ), + self._update_action( + target_id="11111111-1111-1111-1111-111111111111", + new_quantity=8.0, + succeeded=False, + error="422 Unprocessable: quantity must be > 0", + status_label="FAILED", + ), + ] + app = build_bom_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + # Card-level state badge. + assert "PARTIAL FAILURE" in rendered + # Error surfaces in the consolidated Alert at the bottom. + assert "422 Unprocessable" in rendered + + def test_empty_plan_with_existing_rows_renders_table_with_no_summary(self): + """A no-op plan (no actions) → the table shows existing rows + without the +/~/- summary line (empty plan, nothing to count).""" + app = build_bom_modify_ui( + self._preview([]), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + # Existing rows surface. + assert "BLT-M5-10" in rendered + assert "BLT-M6-12" in rendered + # No summary fragments. + assert "+0 added" not in rendered + assert "~0 updated" not in rendered + + def test_empty_prior_with_only_adds_renders_added_rows(self): + """First-time BOM (no existing rows, prior_state is None or empty) + + add-only plan → the card renders the added rows with their + resolved identities. Common case: setting up a recipe for a new + product.""" + actions = [ + self._add_action(ingredient_id=301, quantity=2.0), + self._add_action(ingredient_id=302, quantity=4.0), + ] + app = build_bom_modify_ui( + self._preview(actions, prior_state=None), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + assert "FS90250" in rendered + assert "FS90251" in rendered + assert "+2 added" in rendered + + def test_confirm_label_pluralizes_with_action_count(self): + """Confirm button label reads ``Confirm 3 BOM changes`` so the user + knows what they're committing to. Singular form for 1 action.""" + single = build_bom_modify_ui( + self._preview([self._add_action(ingredient_id=301, quantity=1.0)]), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + assert "Confirm 1 BOM change" in str(single.to_json()) + + multi = build_bom_modify_ui( + self._preview( + [ + self._add_action(ingredient_id=301, quantity=1.0), + self._add_action(ingredient_id=302, quantity=1.0), + self._add_action(ingredient_id=303, quantity=1.0), + ] + ), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + assert "Confirm 3 BOM changes" in str(multi.to_json()) + + def test_card_does_not_leak_internal_action_labels(self): + """Anti-pattern guard (per ``feedback-user-centric-card-content``): + the rendered card MUST NOT carry the internal ActionResult model's + ``Add Bom Row`` / ``Update Bom Row`` / ``N field(s) set`` labels — + those are exactly what #811 exists to eliminate.""" + actions = [ + self._add_action(ingredient_id=301, quantity=2.0), + self._update_action( + target_id="11111111-1111-1111-1111-111111111111", new_quantity=8.0 + ), + ] + app = build_bom_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + rendered = str(app.to_json()) + # Anti-pattern strings — these would indicate the legacy + # _ACTION_COLUMNS shape leaked through. + assert "Add Bom Row" not in rendered + assert "Update Bom Row" not in rendered + assert "Delete Bom Row" not in rendered + assert "field(s) set" not in rendered + assert "field(s) changed" not in rendered + + def test_state_seeds_plan_rows_for_datatable_binding(self): + """The DataTable binds rows via ``{{ plan_rows }}`` — the merged + row list must seed ``state.plan_rows`` so the mustache reference + resolves at render time. ``_assert_state_bindings_resolve`` checks + the mustache form; this test pins the underlying state shape.""" + actions = [self._add_action(ingredient_id=301, quantity=2.0)] + app = build_bom_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="manage_product_bom", + ) + assert app.state is not None + plan_rows = app.state.get("plan_rows") + assert isinstance(plan_rows, list) + # 2 existing + 1 added. + assert len(plan_rows) == 3 + kinds = [r["kind"] for r in plan_rows] + assert kinds.count("existing") == 2 + assert kinds.count("added") == 1 + + class TestPOEntityViewSharedBetweenCreateAndModify: """``_render_po_entity_view`` is called by BOTH ``build_po_create_ui`` (with ``changes=None``) and ``build_po_modify_ui`` (with a diff diff --git a/katana_mcp_server/tests/tools/test_bom.py b/katana_mcp_server/tests/tools/test_bom.py index a3b6d213..06bcbac8 100644 --- a/katana_mcp_server/tests/tools/test_bom.py +++ b/katana_mcp_server/tests/tools/test_bom.py @@ -385,6 +385,247 @@ async def fake_delete(*, id, client): assert response.prior_state["rows"][0]["sku"] == "SK74001" +@pytest.mark.asyncio +async def test_manage_bom_response_carries_resolved_ingredients_for_renderer(): + """#811: the BOM modify card needs ingredient SKU + display_name for + rows that *don't* have the identity inline on the snapshot. + + Renderer contract (per ``_merge_bom_rows_for_modify_card``): + - **Existing / deleted rows** — read SKU / display_name directly off + the snapshot row dict. The snapshot already resolved them via + ``_fetch_bom_row_infos`` so no further lookup is needed. + - **Added rows** — synthesized from the ``add_bom_row`` action's + ``changes``; the new ingredient_variant_id has no snapshot row to + reference, so the SKU / display_name come from + ``resolved_ingredients[ingredient_id]``. + - **Updated rows with an ingredient swap** — the post-patch identity + replaces the snapshot identity, again sourced from + ``resolved_ingredients`` for the swapped-in variant id. + + The impl (see :func:`_collect_ingredient_ids`) batches a single + cache lookup across the targeted union: every add's target ingredient, + every update's NEW ingredient when swap-shaped, plus every + update / delete TARGET row's existing ingredient (resolved via the + snapshot so the renderer can render the pre-patch identity). Rows + that aren't touched by the plan don't contribute — their identity + is already in the snapshot's ``rows[*].sku`` / ``display_name``. + This test pins that union: one add + one update + one delete → + those three ingredient_variant_ids are in the map. + """ + from katana_mcp.tools.foundation.bom import BomRowInfo + + context, lifespan = create_mock_context() + existing_rows = [ + BomRowInfo( + id="11111111-1111-1111-1111-111111111111", + product_item_id=100, + product_variant_id=200, + ingredient_variant_id=401, # gets updated + sku="BLT-M5-10", + quantity=6.0, + ), + BomRowInfo( + id="22222222-2222-2222-2222-222222222222", + product_item_id=100, + product_variant_id=200, + ingredient_variant_id=402, # gets deleted + sku="BLT-M6-12", + quantity=4.0, + ), + ] + + def _resolve(model, variant_ids, **kwargs): + return { + vid: _mock_variant(id=vid, sku=f"SKU-{vid}", product_id=100) + for vid in variant_ids + } + + lifespan.typed_cache.catalog.get_many_by_ids = AsyncMock(side_effect=_resolve) + + with patch( + "katana_mcp.tools.foundation.bom._fetch_bom_row_infos", + new_callable=AsyncMock, + return_value=existing_rows, + ): + request = ManageProductBomRequest( + id=200, + add_bom_rows=[BomRowAdd(ingredient_variant_id=301, quantity=2.0)], + update_bom_rows=[ + BomRowUpdate( + id=UUID("11111111-1111-1111-1111-111111111111"), + quantity=8.0, + ) + ], + delete_bom_row_ids=[UUID("22222222-2222-2222-2222-222222222222")], + preview=True, + ) + response = await _modify_product_bom_impl(request, context) + + resolved = response.extras.get("resolved_ingredients") + assert isinstance(resolved, dict) + # Every ingredient_variant_id from add + delete + update target's + # existing row appears in the resolved map. (Updates that don't swap + # the ingredient still need the existing row's id resolved so the + # row identity surfaces in the card.) + assert 301 in resolved # add + assert 401 in resolved # update target's existing ingredient + assert 402 in resolved # delete target's existing ingredient + # Resolved entries carry the typed sku/display_name shape the + # renderer reads. + assert resolved[301]["sku"] == "SKU-301" + # Preview response: applied_plan_rows NOT populated (computed only + # on the apply branch). The iframe seeds state.plan_rows from the + # preview table_rows at first render; applied_plan_rows is reserved + # for the on_success SetState morph. + assert "applied_plan_rows" not in response.extras + + +@pytest.mark.asyncio +async def test_manage_bom_apply_response_carries_applied_plan_rows(): + """#811 follow-up: the apply response must precompute the + resolved-status table rows so the iframe's Confirm chain can + SetState the DataTable bindings to the actual outcome (APPLIED / + FAILED) instead of leaving the seeded preview rows stuck on + PLANNED. Reads from ``response.extras["applied_plan_rows"]``; + each row carries the merged-row shape (kind / status_prefix / + sku_label / status_label / etc.) — i.e. the same row dicts + ``_prepare_bom_table_rows`` produces. + """ + from katana_mcp.tools.foundation.bom import BomRowInfo + + context, lifespan = create_mock_context() + lifespan.typed_cache.catalog.get_many_by_ids = AsyncMock( + return_value={ + 200: _mock_variant(id=200, sku="SP0502", product_id=17092695), + } + ) + + async def fake_create(*, client, body): + resp = MagicMock() + resp.status_code = 204 + resp.parsed = None + return resp + + existing_rows = [ + BomRowInfo( + id="11111111-1111-1111-1111-111111111111", + product_item_id=100, + product_variant_id=200, + ingredient_variant_id=401, + sku="BLT-M5-10", + quantity=6.0, + ), + ] + + with ( + patch( + "katana_mcp.tools.foundation.bom._fetch_bom_row_infos", + new_callable=AsyncMock, + return_value=existing_rows, + ), + patch( + "katana_mcp.tools.foundation.bom.api_create_bom_row.asyncio_detailed", + side_effect=fake_create, + ), + ): + request = ManageProductBomRequest( + id=200, + add_bom_rows=[BomRowAdd(ingredient_variant_id=301, quantity=2.0)], + preview=False, + ) + response = await _modify_product_bom_impl(request, context) + + assert response.is_preview is False + applied_rows = response.extras.get("applied_plan_rows") + assert isinstance(applied_rows, list) + # One existing row (untouched) + one added row. + assert len(applied_rows) == 2 + # The added row reflects the resolved status (APPLIED) — not the + # PLANNED-stuck-on-preview state. ``_prepare_bom_table_rows`` glues + # the kind-prefix gutter onto sku_label. + added = next(r for r in applied_rows if r["kind"] == "added") + assert added["status_label"] == "APPLIED" + assert added["status_prefix"] == "+ " + # The existing untouched row stays as such — applied_plan_rows + # carries the FULL post-merge table, not just the planned actions. + untouched = next(r for r in applied_rows if r["kind"] == "existing") + assert untouched["sku"] == "BLT-M5-10" + + +@pytest.mark.asyncio +async def test_manage_bom_apply_synthesizes_not_run_for_fail_fast_tail(): + """``execute_plan`` is fail-fast: ``response.actions`` ends at the + first failed action. The impl must synthesize NOT RUN entries for + plan tail entries that were never attempted, otherwise the morphed + DataTable would silently hide them — the operator sees "1 added" + when the plan was actually "1 added, 2 never-attempted". + + Pins ``applied_plan_rows`` includes the never-run entries with + ``status_label="NOT RUN"`` and ``applied_failed_summary`` calls + out the count. + """ + context, lifespan = create_mock_context() + lifespan.typed_cache.catalog.get_many_by_ids = AsyncMock( + return_value={ + 200: _mock_variant(id=200, sku="SP0502", product_id=17092695), + } + ) + + call_count = {"n": 0} + + async def fake_create(*, client, body): + call_count["n"] += 1 + if call_count["n"] == 1: + # First add fails — execute_plan halts here. + raise RuntimeError("422 ingredient_variant_id is required") + # Should never be reached because fail-fast halts after the first. + resp = MagicMock() + resp.status_code = 204 + resp.parsed = None + return resp + + with ( + patch( + "katana_mcp.tools.foundation.bom._fetch_bom_row_infos", + new_callable=AsyncMock, + return_value=[], + ), + patch( + "katana_mcp.tools.foundation.bom.api_create_bom_row.asyncio_detailed", + side_effect=fake_create, + ), + ): + request = ManageProductBomRequest( + id=200, + add_bom_rows=[ + BomRowAdd(ingredient_variant_id=301, quantity=1.0), + BomRowAdd(ingredient_variant_id=302, quantity=1.0), + BomRowAdd(ingredient_variant_id=303, quantity=1.0), + ], + preview=False, + ) + response = await _modify_product_bom_impl(request, context) + + # Only the first action was attempted (and failed); the rest were + # never run. + assert call_count["n"] == 1 + assert len(response.actions) == 1 + assert response.actions[0].succeeded is False + + applied_rows = response.extras.get("applied_plan_rows") + assert isinstance(applied_rows, list) + # 3 added rows (one FAILED, two NOT RUN). Existing snapshot was empty. + assert len(applied_rows) == 3 + statuses = [r.get("status_label") for r in applied_rows] + assert statuses.count("FAILED") == 1 + assert statuses.count("NOT RUN") == 2 + # The summary tells the operator how to recover. + assert "NOT RUN" in response.extras.get("applied_failed_summary", "") + assert "re-issue manage_product_bom" in response.extras.get( + "applied_failed_summary", "" + ) + + @pytest.mark.asyncio async def test_manage_bom_apply_calls_create_for_each_add(): """Confirm mode executes per-action POST /bom_rows for each add.