From 406744d8cfd1ef8af2ddad04cdf0fc4d90ebdae3 Mon Sep 17 00:00:00 2001 From: Doug Borg Date: Tue, 26 May 2026 20:59:29 -0600 Subject: [PATCH] =?UTF-8?q?feat(mcp):=20build=5Fso=5Fmodify=5Fui=20?= =?UTF-8?q?=E2=80=94=20diff-decorated=20SO=20modify=20card=20(closes=20#72?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the per-entity SO modify card to the #721 modify-card redesign umbrella. Reuses the entity-view diff helpers shared with PO modify (_render_field_diff_line / _render_party_diff_line / _render_failed_changes_block) and follows the BOM modify card's state-driven apply-outcome morph pattern for parallel sub-entity outcomes that build-time rendering cannot predict. - New build_so_modify_ui in prefab_ui.py — dispatched via _modification.to_tool_result on entity_type == "sales_order". Covers modify_sales_order, delete_sales_order, correct_sales_order. - Header scalar field-spec (_SO_HEADER_FIELD_SPEC) drives the per-field diff line emission table-style instead of branchy code. - Per-sub-entity sections (Line items / Addresses / Fulfillments / Shipping fees) — actions render with kind gutter (+ / ~ / -) + per-action APPLIED / FAILED Badge on the applied path. Failed actions get the ✗ gutter for layout-stable per-row failure signal. - Apply-outcome morph state seeded for applied_outcome_label, applied_outcome_variant, applied_subentity_failed_count, applied_subentity_failed_summary, applied_verb so the preview iframe's on_success SetState chain reads $result.state.* (NOT $result. — same envelope-wrap limitation documented on build_bom_modify_ui). - Browser scenario: so_modify_partial_failure_applied — pins the partial-failure render contract end-to-end including the consolidated sub-entity failed-action Alert with retry coaching. 22 new unit tests in TestBuildSOModifyUI mirror the PO modify coverage shape (preview, applied-success, applied-partial-failure, applied-all-failed) and add SO-specific cases (sub-entity grouping, address update unknown-prior handling, customer party diff, failed sub-entity ✗ gutter + Badge). --- .../src/katana_mcp/tools/_modification.py | 15 +- .../tools/foundation/corrections.py | 86 +- .../tools/foundation/sales_orders.py | 35 +- .../src/katana_mcp/tools/prefab_ui.py | 1435 ++++++++++++++- .../tests/browser/render_test_server.py | 712 ++++++++ .../tests/browser/test_modification_render.py | 324 ++++ katana_mcp_server/tests/test_prefab_ui.py | 1588 +++++++++++++++++ .../tests/tools/test_corrections.py | 185 ++ .../tests/tools/test_sales_orders.py | 60 + uv.lock | 4 +- 10 files changed, 4424 insertions(+), 20 deletions(-) diff --git a/katana_mcp_server/src/katana_mcp/tools/_modification.py b/katana_mcp_server/src/katana_mcp/tools/_modification.py index 7de84cad9..c41d9b2f8 100644 --- a/katana_mcp_server/src/katana_mcp/tools/_modification.py +++ b/katana_mcp_server/src/katana_mcp/tools/_modification.py @@ -586,13 +586,14 @@ def to_tool_result( build_modification_preview_ui, build_modification_result_ui, build_po_modify_ui, + build_so_modify_ui, ) response_dict = response.model_dump() - # 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. + # Per-entity dispatch — PO migrated in #722, BOM in #811, SO in #723; + # remaining entities (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( response_dict, @@ -609,6 +610,14 @@ def to_tool_result( ) return make_tool_result(response, ui=ui) + if response.entity_type == "sales_order": + ui = build_so_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/foundation/corrections.py b/katana_mcp_server/src/katana_mcp/tools/foundation/corrections.py index bbe36f9d0..8e076a9b4 100644 --- a/katana_mcp_server/src/katana_mcp/tools/foundation/corrections.py +++ b/katana_mcp_server/src/katana_mcp/tools/foundation/corrections.py @@ -171,22 +171,65 @@ def _augment_prior_state_with_snapshot( async def _run_phases_until_failure( phases: list[list[ActionSpec]], -) -> tuple[list[ActionResult], bool]: +) -> tuple[list[ActionResult], bool, list[ActionSpec]]: """Run each phase via :func:`execute_plan`; halt on the first failed action. - Returns ``(aggregated_results, failed)`` — ``failed=True`` means an - action raised in some phase and subsequent phases were skipped. Callers - use the boolean to branch into success vs failure response building. + Returns ``(aggregated_results, failed, not_run_specs)`` — + ``failed=True`` means an action raised in some phase and subsequent + phases were skipped. ``not_run_specs`` carries every :class:`ActionSpec` + from the unattempted plan tail (the rest of the current failing phase + plus every later phase). Callers use the boolean to branch into success + vs failure response building, and the spec tail to synthesize NOT-RUN + extras for the per-section row morph in :func:`build_so_modify_ui` + (#858 finding B; mirrors ``_modify_sales_order_impl``). + Empty phases are skipped silently. """ aggregated: list[ActionResult] = [] - for phase in phases: + for phase_idx, phase in enumerate(phases): if not phase: continue - aggregated.extend(await execute_plan(phase)) - if any(a.succeeded is False for a in aggregated): - return aggregated, True - return aggregated, False + phase_results = await execute_plan(phase) + aggregated.extend(phase_results) + if any(a.succeeded is False for a in phase_results): + # Unattempted tail = leftover specs in this phase (after the + # failing action) + every spec in every later phase. Mirrors + # ``execute_plan``'s fail-fast contract: it returns results + # only up through the failed action, so any specs past that + # index in the phase never ran. + executed_in_phase = len(phase_results) + not_run_specs = list(phase[executed_in_phase:]) + for later_phase in phases[phase_idx + 1 :]: + not_run_specs.extend(later_phase) + return aggregated, True, not_run_specs + return aggregated, False, [] + + +def _synthesize_correction_not_run_actions( + specs: list[ActionSpec], +) -> list[dict[str, Any]]: + """Build NOT-RUN action dicts for the unattempted phase tail. + + Same shape as :func:`_modify_sales_order_impl`'s NOT-RUN synthesis so + :func:`build_so_modify_ui` (via :func:`_so_actions_with_not_run_tail`) + can merge them into the per-section row morph without distinguishing + apply-vs-correction provenance. ``succeeded=None`` + ``status_label= + "NOT RUN"`` sets the "secondary" Badge variant. + """ + return [ + { + "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 specs + ] def _make_tolerant_patch_apply( @@ -622,8 +665,11 @@ async def _correct_manufacturing_order_impl( f"({len(full_plan)} action(s))" ), ) - aggregated, failed = await _run_phases_until_failure(phases) + aggregated, failed, _not_run_specs = await _run_phases_until_failure(phases) if failed: + # MO modify card doesn't merge NOT-RUN extras yet — drop the spec + # tail here. SO failure path below synthesizes them for the SO + # modify-card morph (#858 finding B). return _build_failure_response( request.id, aggregated, prior_state, katana_url, snapshot ) @@ -1144,11 +1190,22 @@ async def _correct_sales_order_impl( f"{request.id} ({len(full_plan)} action(s))" ), ) - aggregated, failed = await _run_phases_until_failure(phases) + aggregated, failed, not_run_specs = await _run_phases_until_failure(phases) if failed: - return _build_failure_response( + response = _build_failure_response( request.id, aggregated, prior_state, katana_url, snapshot ) + # Synthesize NOT-RUN entries for the unattempted plan tail so + # :func:`build_so_modify_ui` (which handles ``correct_sales_order`` + # alongside ``modify_sales_order``) renders skipped restore / + # recreate / close phases instead of silently overwriting the + # preview's full sub-entity rows with only the executed prefix + # (#858 finding B — Copilot comment 3312071378). Mirrors the + # equivalent synthesis in ``_modify_sales_order_impl``. + not_run_actions = _synthesize_correction_not_run_actions(not_run_specs) + if not_run_actions: + response.extras["not_run_actions"] = not_run_actions + return response return ModificationResponse( entity_type="sales_order", @@ -1585,8 +1642,11 @@ async def _correct_purchase_order_impl( f"{request.id} ({len(full_plan)} action(s))" ), ) - aggregated, failed = await _run_phases_until_failure(phases) + aggregated, failed, _not_run_specs = await _run_phases_until_failure(phases) if failed: + # PO modify card doesn't merge NOT-RUN extras yet — drop the spec + # tail here. The SO failure path above synthesizes them for the SO + # modify-card morph (#858 finding B). return _build_failure_response( request.id, aggregated, prior_state, katana_url, snapshot ) diff --git a/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py b/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py index f79274beb..be0fc3055 100644 --- a/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py +++ b/katana_mcp_server/src/katana_mcp/tools/foundation/sales_orders.py @@ -2417,7 +2417,7 @@ async def _modify_sales_order_impl( ) ) - return await run_modify_plan( + response = await run_modify_plan( request=request, naming=EntityNaming( entity_type="sales_order", @@ -2444,6 +2444,39 @@ async def _modify_sales_order_impl( ), ) + # Apply-path: synthesize NOT-RUN entries for the unattempted plan tail. + # ``execute_plan`` is fail-fast: ``response.actions`` ends at the first + # failed action. Without these synthetic entries the modify card's + # per-section row morph would silently HIDE every planned action past + # the failure — the operator would see "1 succeeded, 1 failed" with + # no indication that 3 more actions were never attempted. Mirror + # ``_modify_product_bom_impl``'s NOT RUN pattern (#811): emit one + # entry per leftover ``ActionSpec`` with ``status_label="NOT RUN"`` + # and ``succeeded=None`` (so per-row chrome renders the "secondary" + # Badge variant via :data:`_SO_SUB_STATUS_VARIANTS`). The renderer + # picks them up via ``response.extras["not_run_actions"]`` and merges + # them into the per-section row bucketing in + # :func:`build_so_modify_ui` — #858 finding B. + if not response.is_preview: + not_run_specs = plan[len(response.actions) :] + 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 + ] + if not_run_actions: + response.extras["not_run_actions"] = not_run_actions + 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 043f90a18..d7b7b2722 100644 --- a/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py +++ b/katana_mcp_server/src/katana_mcp/tools/prefab_ui.py @@ -68,6 +68,8 @@ from __future__ import annotations +from collections.abc import Callable +from dataclasses import dataclass from decimal import Decimal, InvalidOperation from typing import TYPE_CHECKING, Any, Literal @@ -102,7 +104,7 @@ Separator, Text, ) -from prefab_ui.components.control_flow import Elif, Else, If +from prefab_ui.components.control_flow import Elif, Else, ForEach, If from prefab_ui.components.slot import Slot from prefab_ui.rx import EVENT, RESULT, Rx from pydantic import BaseModel @@ -2559,6 +2561,8 @@ class FieldChangeView(BaseModel): def _index_changes_by_field( actions: list[dict[str, Any]], + *, + include_operations: frozenset[str] | None = None, ) -> dict[str, FieldChangeView]: """Flatten ``ActionResult.changes`` lists into a field-name keyed map. @@ -2573,9 +2577,39 @@ def _index_changes_by_field( in two actions (rare) takes the last write — the iteration order matches the action plan's execution order, so the last write is the one that ran (or would have run) most recently. + + ``include_operations`` restricts the flatten to actions whose + ``operation`` value (case-insensitive) is in the set — used by the SO + modify card to keep header-field rendering from picking up sub-entity + changes whose field names overlap header names (``status``, + ``picked_date``, ``tracking_number`` exist on both fulfillments and + the SO header). When ``None`` (the default), every action contributes + — same behavior as before this parameter was added, keeps PO/test + call sites unchanged. + + Synthesized NOT-RUN actions (``status_label == "NOT RUN"``, emitted by + :func:`_synthesize_correction_not_run_actions` and the per-tool + NOT-RUN synthesizers for the unattempted phase tail of a failed + correction) are filtered out — these are placeholders for plan steps + that never ran, not real diffs. Last-write-wins on the field map + would otherwise let a synthesized late ``update_header`` (e.g., the + close-phase header step in a sales-order correction) overwrite an + earlier EXECUTED ``update_header`` diff and render as an applied + scalar change with no NOT RUN indication. NOT RUN status surfaces + via row-level chrome (the NOT RUN Badge on the per-action row), + not via the header field diff map. Caught by Copilot review on #858. """ out: dict[str, FieldChangeView] = {} for action in actions: + if include_operations is not None: + op = str(action.get("operation") or "").lower() + if op not in include_operations: + continue + # NOT-RUN synthesized actions are plan placeholders, not real + # diffs — never let them write into the field map (would mask + # an earlier executed action's diff via last-write-wins). + if str(action.get("status_label") or "") == "NOT RUN": + continue succeeded = action.get("succeeded") action_error = action.get("error") # ``succeeded`` is None during preview, True/False after apply. @@ -4905,6 +4939,1405 @@ def build_so_create_ui( return app +# ============================================================================ +# Sales-order modify card — diff-decorated SO modify/delete/correct card +# (#723). Reuses ``_render_field_diff_line`` / ``_render_party_diff_line`` +# from the PO modify card, but intentionally does NOT call +# ``_render_failed_changes_block`` — all header-failure rendering is +# routed through the state-driven ``applied_header_failed_*`` Alert (seeded +# by :func:`_so_header_op_failure_alert_text`) so the preview→Confirm +# in-place morph has a single source of truth. Adds dedicated sub-sections +# for SO's parallel-outcome sub-entities (rows, addresses, fulfillments, +# shipping fees). Each sub-section follows the BOM modify card's +# table-as-entity-view pattern adapted to a per-section row list with +# per-action status pills. +# ============================================================================ + + +# Per-action sub-entity status variants — same buckets as BOM_ROW_STATUS_VARIANTS +# so SO sub-entity rows render with the same Badge variants as BOM rows. +_SO_SUB_STATUS_VARIANTS: dict[str, str] = { + "PLANNED": "secondary", + "APPLIED": "default", + "APPLIED (verified)": "default", + "APPLIED (verification mismatch)": "destructive", + "FAILED": "destructive", + "NOT RUN": "secondary", + "": "outline", +} + + +# Wire field → user-facing label map for the consolidated failed-changes +# Alert. Reads "Failed — Customer: 422 Bad Request" instead of the wire- +# name "Failed — customer_id: ...". Keys cover every SOHeaderPatch field +# the API can produce a FieldChange for; missing keys fall through to +# a Title-cased version of the wire name inside +# :func:`_so_header_op_failure_alert_text`. +_SO_HEADER_LABEL_OVERRIDES: dict[str, str] = { + "customer_id": "Customer", + "customer_name": "Customer", + "location_id": "Location", + "location_name": "Location", + "additional_info": "Notes", + "notes": "Notes", + "order_no": "Order #", + "order_number": "Order #", + "delivery_date": "Delivery date", + "picked_date": "Picked date", + "order_created_date": "Order created date", + "currency": "Currency", + "conversion_rate": "Conversion rate", + "conversion_date": "Conversion date", + "customer_ref": "Customer ref", + "tracking_number": "Tracking #", + "tracking_number_url": "Tracking URL", + "status": "Status", +} + + +def _normalize_so_prior_state(prior_state: dict[str, Any] | None) -> dict[str, Any]: + """Map an SO ``prior_state`` snapshot from the wire shape produced by + ``SalesOrder.to_dict()`` (server-side) to the response shape + :func:`_render_so_entity_view` consumes. + + Field renames between the wire snapshot and the response shape: + + - ``order_no`` (wire) → ``order_number`` (response shape; SO create + response surfaces it under ``order_number``). + - ``additional_info`` (wire) → also kept under ``notes`` so the + renderer can read either key. + + Derived metrics: + + - ``item_count`` — :class:`ModificationResponse` doesn't carry + ``item_count`` (only :class:`SalesOrderResponse` does), so the + modify card's Tier-2 "Line Items" Metric would render blank on a + real apply response. Derive it from ``len(prior_state["sales_order_rows"])`` + when ``item_count`` is missing so the Metric renders consistently + across the create and modify cards (#858 Copilot finding — + comment 3313163122). + + SO's wire snapshot does NOT carry a nested ``customer`` object the way + PO carries ``supplier``; it only has ``customer_id``. The + ``customer_name`` lookup the response shape may surface is added by + upstream cache-merge; if the snapshot doesn't have it, the renderer + falls back to ``#``. + + Without this adapter the modify card renders mostly-empty header rows + in production: ``entity.get("order_number")`` falls through to ``None`` + because the wire-shape snapshot only has ``order_no``. Same shape- + mismatch bug Copilot caught on PO #755. + """ + if not prior_state: + return {} + out = dict(prior_state) + if "order_no" in prior_state and "order_number" not in out: + out["order_number"] = prior_state["order_no"] + if "additional_info" in prior_state and "notes" not in out: + out["notes"] = prior_state["additional_info"] + if "item_count" not in out: + rows = prior_state.get("sales_order_rows") + if isinstance(rows, list): + out["item_count"] = len(rows) + return out + + +def _so_change_new(changes: list[Any], field: str) -> Any: + """Pluck the ``new`` value for ``field`` from an action's changes list, + or ``None`` if the field isn't present. Helper for the SO sub-entity + summary formatters.""" + for c in changes: + if isinstance(c, dict) and c.get("field") == field: + return c.get("new") + return None + + +def _format_so_diff_pairs( + changes: list[Any], + *, + address_style: bool = False, +) -> list[str]: + """Format an action's changes list into ``field: before → after`` (or + ``field: (prior unknown) → after``) strings. + + Used by the ``update_*`` formatters across rows / addresses / + fulfillments / shipping fees. ``address_style=True`` skips the diff + arrow and renders only the after value — address updates always + carry ``is_unknown_prior=True`` (no per-address GET), so the + arrow form would always read ``(prior unknown) → new``; collapsing + to the bare ``field: new`` form keeps the summary line compact. + """ + diffs: list[str] = [] + for c in changes: + if not isinstance(c, dict): + continue + field = c.get("field") + if not isinstance(field, str): + continue + new = c.get("new") + if address_style: + diffs.append(f"{field}: {_format_diff_value(new)}") + continue + old = c.get("old") + unknown_prior = bool(c.get("is_unknown_prior")) + after = _format_diff_value(new) + if unknown_prior: + diffs.append(f"{field}: (prior unknown) → {after}") + else: + diffs.append(f"{field}: {_format_diff_value(old)} → {after}") + return diffs + + +def _so_anchor_with_diffs( + *, + kind: str, + target_id: Any, + diffs: list[str], +) -> str: + """Compose `` # — , `` (or the bare + anchor when no diffs are present). Shared anchor-builder for the + SO ``update_*`` / ``delete_*`` summary formatters. + """ + anchor = f"{kind} #{target_id}" if target_id is not None else kind + if diffs: + return f"{anchor} — {', '.join(diffs)}" + return anchor + + +def _format_so_add_row(changes: list[Any]) -> str: + """One-line summary for an ``add_row`` action.""" + variant_id = _so_change_new(changes, "variant_id") + quantity = _so_change_new(changes, "quantity") + bits: list[str] = [] + if variant_id is not None: + bits.append(f"variant {variant_id}") + if quantity is not None: + bits.append(f"qty {quantity}") + return ", ".join(bits) or "new line item" + + +def _format_so_add_address(changes: list[Any]) -> str: + """One-line summary for an ``add_address`` action.""" + entity_type = _so_change_new(changes, "entity_type") or "address" + city = _so_change_new(changes, "city") + zip_ = _so_change_new(changes, "zip") + bits: list[str] = [] + if city: + bits.append(str(city)) + if zip_: + bits.append(str(zip_)) + return f"{entity_type}: {', '.join(bits)}" if bits else str(entity_type) + + +def _format_so_add_fulfillment(changes: list[Any]) -> str: + """One-line summary for an ``add_fulfillment`` action. + + Labels are field-specific — ``status`` and ``picked_date`` get their + own ``status `` / ``picked `` prefixes so the rendered + summary doesn't read ``status 2026-05-08T14:30:00Z`` when only the + pick timestamp is supplied (the old first-truthy fallback conflated + the two and surfaced a timestamp under the ``status`` label). + """ + status = _so_change_new(changes, "status") + picked = _so_change_new(changes, "picked_date") + tracking = _so_change_new(changes, "tracking_number") + bits: list[str] = [] + if status: + bits.append(f"status {status}") + if picked: + bits.append(f"picked {picked}") + if tracking: + bits.append(f"tracking {tracking}") + return ", ".join(bits) or "new fulfillment" + + +def _format_so_add_shipping_fee(changes: list[Any]) -> str: + """One-line summary for an ``add_shipping_fee`` action.""" + description = _so_change_new(changes, "description") or "shipping fee" + amount = _so_change_new(changes, "amount") + if amount is not None: + return f"{description}: {amount}" + return str(description) + + +# Dispatch table for ``add_*`` summaries — keeps the main formatter +# under ruff's complexity budget. ``update_*`` and ``delete_*`` ops use +# the shared :func:`_so_anchor_with_diffs` helper so they don't need +# per-kind entries. +_SO_ADD_FORMATTERS: dict[str, Callable[[list[Any]], str]] = { + "add_row": _format_so_add_row, + "add_address": _format_so_add_address, + "add_fulfillment": _format_so_add_fulfillment, + "add_shipping_fee": _format_so_add_shipping_fee, +} + + +# Map of ``update_*`` / ``delete_*`` op → (kind label, address_style). +# ``address_style`` flips the diff renderer to the unknown-prior-friendly +# bare-after form for address updates. +_SO_KIND_FOR_OP: dict[str, tuple[str, bool]] = { + "update_row": ("row", False), + "delete_row": ("row", False), + "update_address": ("address", True), + "delete_address": ("address", False), + "update_fulfillment": ("fulfillment", False), + "delete_fulfillment": ("fulfillment", False), + "update_shipping_fee": ("shipping fee", False), + "delete_shipping_fee": ("shipping fee", False), +} + + +def _format_so_action_summary(action: dict[str, Any]) -> str: + """Build a one-line label summarizing an SO sub-entity action. + + Picks the most identifying field from the action's changes to anchor + the line — e.g. ``variant_id`` for row adds, ``id`` (target_id) for + row updates, etc. Falls back to ``target_id`` (the wire row UUID/id) + when no obvious anchor field is present, then to a generic counter. + + Dispatched via two tables: + + - :data:`_SO_ADD_FORMATTERS` handles ``add_*`` ops (each kind has its + own identifying fields — variant_id+quantity, city+zip, etc.). + - :data:`_SO_KIND_FOR_OP` covers ``update_*`` / ``delete_*`` via the + shared anchor builder. ``update_address`` uses the + ``address_style=True`` path because Katana has no per-address GET, + so every diff carries ``is_unknown_prior=True`` and the bare-after + form reads better than ``(prior unknown) → X`` per field. + + The output is the *body* of the per-action row text — callers prefix + it with the kind gutter (``+ `` / ``~ `` / ``- ``). + """ + op = str(action.get("operation") or "").lower() + target_id = action.get("target_id") + changes = action.get("changes") or [] + + if op in _SO_ADD_FORMATTERS: + return _SO_ADD_FORMATTERS[op](changes) + + if op in _SO_KIND_FOR_OP: + kind, address_style = _SO_KIND_FOR_OP[op] + if op.startswith("delete_"): + # Delete summaries are just the anchor — no field diffs to + # surface (the action carries empty ``changes``; the kind + # gutter already signals the removal). + return _so_anchor_with_diffs(kind=kind, target_id=target_id, diffs=[]) + diffs = _format_so_diff_pairs(changes, address_style=address_style) + return _so_anchor_with_diffs(kind=kind, target_id=target_id, diffs=diffs) + + # Fallback — operation name + target_id when nothing else fits. + if target_id is not None: + return f"{op} #{target_id}" + return op or "(action)" + + +# Per-sub-entity ``operation`` prefix for grouping actions into sections. +# Section headers read "Line items", "Addresses", "Fulfillments", "Shipping +# fees" so the modify card scans naturally; the grouping is keyed off the +# wire ``operation`` enum so future schema additions land in a sensible +# bucket without renderer churn. +_SO_SUBENTITY_GROUPS: tuple[tuple[str, str, tuple[str, ...]], ...] = ( + ( + "rows", + "Line items", + ("add_row", "update_row", "delete_row"), + ), + ( + "addresses", + "Addresses", + ("add_address", "update_address", "delete_address"), + ), + ( + "fulfillments", + "Fulfillments", + ("add_fulfillment", "update_fulfillment", "delete_fulfillment"), + ), + ( + "shipping_fees", + "Shipping fees", + ("add_shipping_fee", "update_shipping_fee", "delete_shipping_fee"), + ), +) + + +# Derived set of every operation that belongs to a sub-entity row group. +# Used by :func:`_so_subentity_failed_summary` to gate the sub-entity Alert +# on operations that actually render in the sub-entity sections — +# ``update_header`` / ``delete_sales_order`` failures are surfaced by the +# state-driven ``applied_header_failed_*`` Alert (seeded by +# :func:`_so_header_op_failure_alert_text`) instead, so including them in +# the sub-entity Alert would double-render the same error. +_SO_SUBENTITY_OPS: frozenset[str] = frozenset( + op for _key, _label, ops in _SO_SUBENTITY_GROUPS for op in ops +) + + +# Header-level operations for the SO modify card. Used to filter +# :func:`_index_changes_by_field` so the header-field scalar-diff +# rendering doesn't pick up sub-entity field changes (sub-entity actions +# for fulfillments / rows / shipping fees can carry field names like +# ``status``, ``picked_date``, ``tracking_number``, ``description``, +# ``amount``, ``quantity`` that overlap or look-like header names — +# flattening them into the header map would render a stale +# "Status: PACKED → DELIVERED" header diff driven by a fulfillment +# update). +# +# ``delete`` is the top-level SO delete operation emitted by +# ``delete_sales_order``; ``update_header`` covers ``modify_sales_order`` +# header writes and the header-touching legs of ``correct_sales_order``. +_SO_HEADER_OPS: frozenset[str] = frozenset({"update_header", "delete"}) + + +def _so_action_kind_gutter(operation: str) -> str: + """Return the 2-char gutter prefix for an SO sub-entity action. + + Mirrors the BOM modify card's ``status_prefix`` convention: adds get + ``+ ``, updates ``~ ``, deletes ``- ``. The leading 2 chars reserve + visual space so a status-pill morph doesn't reflow the row. + """ + if operation.startswith("add_"): + return "+ " + if operation.startswith("update_"): + return "~ " + if operation.startswith("delete_"): + return "- " + return " " + + +def _build_so_subentity_row(action: dict[str, Any]) -> dict[str, Any]: + """Project one sub-entity action onto the row-dict shape consumed by + the state-bound ``ForEach`` renderer (see :func:`_render_so_subentity_section`). + + Each row dict carries the pre-rendered text + Badge metadata so the + DOM tree built by ``ForEach`` is purely state-driven — the preview + iframe's apply-time morph just swaps in a new row list (via + ``SetState`` from ``$result.state.so_
_rows``) and the + per-action chrome (gutter glyph, status label, badge variant) + updates in lockstep with the apply outcome. + + Schema: + + - ``gutter_summary`` — ``""``. Pre-painted so the + ``ForEach`` body is a single ``Text(content="{{ $item.gutter_summary }}")`` + — no Mustache compose needed inside the loop. The gutter encodes + both kind (``+ ``/``~ ``/``- ``) and outcome (``x `` when failed). + - ``status_label`` — ``"PLANNED" / "APPLIED" / "FAILED" / "NOT RUN"`` + or ``""`` when unknown. The build-time render uses + :func:`_derive_status_label` to bucket per-action outcome. + - ``has_badge`` — ``True`` when the row should show a Badge alongside + its text (applied / failed / NOT RUN); ``False`` for preview-time + ``PLANNED`` rows (the card-level state Badge already carries the + planned-state signal and per-row Badges would be noise). + - ``status_variant`` — ``"default" / "destructive" / "secondary"``. + Drives the Badge color via the ``If(Rx("$item.status_variant") == + "destructive")`` branch inside the ``ForEach`` body. + + Shared between preview-time row seeding and apply-time row recompute + so the wire shape matches across the morph. + """ + op = str(action.get("operation") or "").lower() + gutter = _so_action_kind_gutter(op) + succeeded = action.get("succeeded") + # Failed actions get the failure glyph in the gutter slot (replacing + # the +/~/- kind glyph) so the failure is at-a-glance visible. The + # kind is still readable from the section header + the diff body. + if succeeded is False: + gutter = "✗ " + summary = _format_so_action_summary(action) + status_label = action.get("status_label") or _derive_status_label(action) or "" + has_badge = bool(status_label) and status_label != "PLANNED" + status_variant = _SO_SUB_STATUS_VARIANTS.get(status_label, "secondary") + return { + "gutter_summary": f"{gutter}{summary}", + "status_label": status_label, + "has_badge": has_badge, + "status_variant": status_variant, + } + + +def _build_so_subentity_row_lists( + actions: list[dict[str, Any]], +) -> dict[str, list[dict[str, Any]]]: + """Bucket SO actions into per-section row-dict lists keyed by the + :data:`_SO_SUBENTITY_GROUPS` section key (``rows`` / ``addresses`` / + ``fulfillments`` / ``shipping_fees``). + + Threads :func:`_build_so_subentity_row` over each action and groups + by operation prefix. Used by :func:`build_so_modify_ui` to seed + ``state.so_
_rows`` slots at build time; the apply-time call + recomputes against the apply response's actions, and the on_success + ``SetState`` chain reads ``{{ $result.state.so_
_rows }}`` + so the preview iframe morphs each section's rows in lockstep with + the apply outcome. + + The returned dict always contains all four section keys (empty list + when the section had no actions) so the renderer can bind + unconditionally without ``None``-guards. + """ + by_section: dict[str, list[dict[str, Any]]] = { + section_key: [] for section_key, _label, _ops in _SO_SUBENTITY_GROUPS + } + for action in actions: + op = str(action.get("operation") or "").lower() + for section_key, _label, section_ops in _SO_SUBENTITY_GROUPS: + if op in section_ops: + by_section[section_key].append(_build_so_subentity_row(action)) + break + return by_section + + +def _render_so_subentity_section( + section_label: str, + section_key: str, + actions: list[dict[str, Any]], +) -> None: + """Render one sub-entity action section (Line items / Addresses / + Fulfillments / Shipping fees) inside the SO modify card body. + + State-bound: the per-action rows render inside a ``ForEach`` keyed + to ``state.so__rows`` so the preview→Confirm in-place + morph re-paints each row's gutter glyph + Badge label + Badge + variant from the apply response (via the on_success ``SetState`` + chain). Pre-fix the rows were Python-painted at build time and + stayed frozen on the preview-time PLANNED state even after the + apply landed (#858, Copilot finding). + + Each row's content lives in ``state.so__rows[i]`` as + a dict (see :func:`_build_so_subentity_row` for the schema). The + ``ForEach`` body renders ``Text`` for the gutter + summary and a + conditional ``Badge`` gated by ``$item.has_badge`` — the destructive + vs default variant branches on ``$item.status_variant``. + + The failed-row error message is NOT rendered inline — it aggregates + into the consolidated state-driven Alert via + :func:`_so_subentity_failed_summary` (sub-entity ops) and (for + header-level ops) :func:`_so_header_op_failure_alert_text`. Same + layout-stability rule as the PO entity view. + + The ``actions`` arg only gates whether the section header renders + (an empty plan section stays absent so the card doesn't paint + "Fulfillments:" headers with no rows below them). The row content + itself comes from state, not from ``actions`` at render time. + + Called inside ``CardContent`` → ``Column(gap=3)``. + """ + if not actions: + return + + Separator() + Muted(content=f"{section_label}:") + # ForEach iterates over the state-bound row list — the same slot + # the on_success chain writes from ``$result.state.so_
_rows``, + # so the row chrome morphs in-place when the apply lands. Inside the + # loop, ``$item`` resolves per-row to one of the row dicts built by + # :func:`_build_so_subentity_row`. + with ForEach(f"so_{section_key}_rows") as item, Row(gap=2): + Text(content=f"{item.gutter_summary}") + # Per-item Badge: only render when the row has reached a + # post-preview status. ``has_badge`` is False at preview + # time (PLANNED rows hide their per-row Badge); True after + # the morph for any row that reached APPLIED / FAILED / + # NOT RUN. + with If(item.has_badge): + # Badge.variant isn't reactive on its own; render parallel + # If/Else branches so the morph picks the right variant. + with If(item.status_variant == "destructive"): + Badge( + label=f"{item.status_label}", + variant="destructive", + ) + with Elif(item.status_variant == "secondary"): + Badge( + label=f"{item.status_label}", + variant="secondary", + ) + with Else(): + Badge( + label=f"{item.status_label}", + variant="default", + ) + + +def _so_subentity_failed_summary( + actions: list[dict[str, Any]], + *, + so_id: int | None, + confirm_tool: str | None = None, +) -> tuple[int, str]: + """Pre-format the consolidated sub-entity failed-action summary text. + + Walks the applied actions, picks the ones with ``succeeded is False``, + and emits one ``Failed — #: `` line each. Returns + ``(failed_count, summary_lines_joined)``. + + The retry-coaching tail mirrors the SO create card's shipping-fees + summary (#818): tells the operator how to re-run the failed sub- + entity operations through the same tool they invoked. The recovery + tool is derived from ``confirm_tool`` so that a partial failure on + ``correct_sales_order`` doesn't misdirect the operator to + ``modify_sales_order`` (#858 review follow-up). ``delete_sales_order`` + is atomic and shouldn't reach this code path, but if it ever does + we fall back to a generic "Retry the failed action(s)" phrasing. + + Used to seed ``state.applied_subentity_failed_count`` / + ``state.applied_subentity_failed_summary`` so the in-place morph + after Confirm can surface failures (the Python-painted per-action + rows above carry ✗ glyphs but the textual error messages aggregate + into the morph-bound Alert below). + + Filters to operations in :data:`_SO_SUBENTITY_OPS` only — a failed + ``update_header`` or top-level ``delete`` would otherwise render in + BOTH this sub-entity Alert and the state-driven + ``applied_header_failed_*`` Alert (seeded by + :func:`_so_header_op_failure_alert_text`), double-surfacing the same + error under a misleading "sub-entity failure(s)" title. + """ + failed = [ + a + for a in actions + if a.get("succeeded") is False + and str(a.get("operation") or "").lower() in _SO_SUBENTITY_OPS + ] + if not failed: + return 0, "" + lines: list[str] = [] + for action in failed: + op = str(action.get("operation") or "(action)") + target = action.get("target_id") + error = action.get("error") or "unknown error" + anchor = f"{op} #{target}" if target is not None else op + lines.append(f"Failed — {anchor}: {error}") + if so_id is not None: + retry_tool = _so_retry_tool_for(confirm_tool) + lines.append("") + if retry_tool is not None: + lines.append( + f"Retry the failed action(s) via {retry_tool}(id={so_id}, " + "=[...]). The original request kwargs are still " + "valid — re-supply only the failed entries." + ) + else: + lines.append( + "Retry the failed action(s) by re-issuing the original " + "request with only the failed entries." + ) + return len(failed), "\n".join(lines) + + +# Maps the SO write tool the operator invoked to the tool they should +# re-issue to recover from a partial sub-entity failure. ``correct_*`` +# corrections must round-trip back through ``correct_sales_order`` — +# the partial-failure rows belong to the correction plan, not to a +# regular modify, and the two paths have different downstream side +# effects (correction emits a different audit trail). ``delete_*`` is +# atomic; it shouldn't reach the sub-entity failure code path, but if +# it ever does we drop the tool-specific phrasing rather than misdirect. +_SO_RETRY_TOOLS: dict[str, str] = { + "modify_sales_order": "modify_sales_order", + "correct_sales_order": "correct_sales_order", +} + + +def _so_retry_tool_for(confirm_tool: str | None) -> str | None: + """Return the tool name to suggest in the sub-entity retry tail. + + Returns ``None`` when ``confirm_tool`` doesn't have a meaningful + retry path (e.g. ``delete_sales_order``, or an unrecognized tool). + Callers fall back to a generic phrasing in that case. + """ + if confirm_tool is None: + return None + return _SO_RETRY_TOOLS.get(confirm_tool) + + +# Verb labels for the header-level op failure Alert. Keeps the user-facing +# language ("Failed to delete the sales order: ") aligned with the +# tool the operator called; "modify" is the catch-all for the +# update_header leg (used by both ``modify_sales_order`` and the +# header-touching legs of ``correct_sales_order``). +_SO_HEADER_OP_VERBS: dict[str, str] = { + "delete": "delete the sales order", + "update_header": "modify the sales order header", +} + + +def _so_header_op_failure_alert_text( + actions: list[dict[str, Any]], +) -> tuple[int, str]: + """Pre-format the consolidated header-level failed-op summary text. + + Walks the applied actions, picks the ones with ``succeeded is False`` + AND whose operation is in :data:`_SO_HEADER_OPS`. This is the + state-driven Alert that fires on the preview→Confirm morph path + (gated by ``If(Rx("applied_header_failed_count") > 0)``); it owns + every header-op failure surface so the build-time-only + :func:`_render_failed_changes_block` doesn't need to be called from + the SO entity view at all (avoids double-rendering and prevents the + morph path's stale build-time block from misrepresenting the apply + outcome — #858 finding C). + + Two failure shapes: + + - **No-change ops** (``delete``; ``update_header`` with no fields): + one ``"Failed to : "`` line per failed op. + - **update_header with field changes**: one ``"Failed to : + "`` action-level line PLUS one ``"Failed — : + "`` line per changed field carrying a per-field + error. The action-level error already covers the canonical + Katana 4xx for a header PATCH (a single error for the whole + PATCH), but Katana occasionally returns per-field + ``validation_errors`` on a 422 — surface those when present so + the operator sees which field failed validation. Pre-fix #858 + finding C: failed ``update_header`` actions WITH changes morphed + to ``applied=True`` with neither the ✗ gutter nor the field- + level error visible because the build-time + ``_render_failed_changes_block`` was painted from preview + actions (``succeeded=None``) and stayed at "no failures" after + the morph. + + Returns ``(failure_count, alert_text)``. The alert text reads as + ``"Failed to : "`` one line per failed op; the empty + string when no header ops failed (the Alert is gated by + ``If(Rx("applied_header_failed_count") > 0)``). + """ + failed = [ + a + for a in actions + if a.get("succeeded") is False + and str(a.get("operation") or "").lower() in _SO_HEADER_OPS + ] + if not failed: + return 0, "" + lines: list[str] = [] + for action in failed: + op = str(action.get("operation") or "").lower() + verb = _SO_HEADER_OP_VERBS.get(op, op or "(action)") + error = action.get("error") or "unknown error" + lines.append(f"Failed to {verb}: {error}") + # Per-field errors (rare — Katana usually returns a single + # error string on header PATCHes; this picks up the + # ``validation_errors``-style cases). + for change in action.get("changes") or []: + field_error = change.get("error") if isinstance(change, dict) else None + if not field_error: + continue + field_name = ( + change.get("field") if isinstance(change, dict) else None + ) or "(field)" + label = _SO_HEADER_LABEL_OVERRIDES.get( + field_name, field_name.replace("_", " ").title() + ) + lines.append(f"Failed — {label}: {field_error}") + return len(failed), "\n".join(lines) + + +def _so_header_op_skipped_alert_text( + actions: list[dict[str, Any]], +) -> tuple[int, str]: + """Pre-format the consolidated header-level NOT-RUN summary text. + + Walks the merged action list (executed + synthesized NOT-RUN tail, see + :func:`_so_actions_with_not_run_tail`), picks the ones whose + ``status_label == "NOT RUN"`` AND whose operation is in + :data:`_SO_HEADER_OPS`. Emits one ``"Step skipped: (NOT RUN — + earlier phase failed before this step ran)."`` line per skipped op. + + Mirrors :func:`_so_header_op_failure_alert_text`'s shape so the + state-driven Alert pattern is symmetric (failed / skipped). Header + NOT-RUN actions have no other rendering surface today: + :func:`_index_changes_by_field` filters them out (last-write-wins + would let a skipped close-phase update_header overwrite the executed + revert-phase update_header diff — #858 round 7), and + :func:`_build_so_subentity_row_lists` only buckets sub-entity ops + (rows / addresses / fulfillments / shipping fees). Without this + helper a failed ``correct_sales_order`` whose close-phase + ``update_header`` is skipped renders only sub-entity NOT-RUN rows — + the operator can't tell the SO close/restore step never ran. Round-8 + follow-up to #858, restoring the option-(B) surface the round-6 + reviewer originally suggested. + + Returns ``(skipped_count, alert_text)``. Empty string when no header + ops were skipped — the Alert is gated by + ``If(Rx("applied_header_skipped_count") > 0)`` so it stays hidden in + that case. + """ + skipped = [ + a + for a in actions + if str(a.get("status_label") or "") == "NOT RUN" + and str(a.get("operation") or "").lower() in _SO_HEADER_OPS + ] + if not skipped: + return 0, "" + lines: list[str] = [] + for action in skipped: + op = str(action.get("operation") or "").lower() + verb = _SO_HEADER_OP_VERBS.get(op, op or "(action)") + lines.append( + f"Step skipped: {verb} " + "(NOT RUN — earlier phase failed before this step ran)." + ) + return len(skipped), "\n".join(lines) + + +# Header scalar field-spec: (label, change-key fallbacks, value-key +# fallbacks, render_when_unchanged). When ``render_when_unchanged`` is +# True the field renders even with no change present, as long as the +# entity carries a truthy value for it — keeps the card showing +# meaningful context (Notes, Tracking #) without padding it with empty +# "Delivery date: (unset)" lines for fields the user isn't modifying. +# +# A field appears at most once per render: the first change-key that +# resolves wins, and the first value-key that resolves wins. Fall-back +# chains exist because ``additional_info`` (wire) and ``notes`` +# (response shape) both alias to the same display label, and +# ``order_no`` / ``order_number`` likewise. Picking the right value-key +# matters for unchanged fields — both lookups are nearly free, so a +# fall-back chain is cheaper than a normalization pass. +_SO_HEADER_FIELD_SPEC: tuple[ + tuple[str, tuple[str, ...], tuple[str, ...], bool], ... +] = ( + ("Notes", ("additional_info", "notes"), ("notes", "additional_info"), True), + ("Delivery date", ("delivery_date",), (), False), + ("Picked date", ("picked_date",), (), False), + ("Order created date", ("order_created_date",), (), False), + ("Currency", ("currency",), (), False), + # ``conversion_rate`` / ``conversion_date`` are SOHeaderPatch fields but + # they don't surface a steady-state value worth rendering on every card + # (Katana's SO response shape doesn't include them on read), so + # ``render_when_unchanged=False`` — they only appear in the diff body + # when the user is actively patching them. Pre-fix #858 finding A: + # omitting them from the spec meant a header-only conversion-rate / + # conversion-date update rendered NO diff lines on the card. + ("Conversion rate", ("conversion_rate",), (), False), + ("Conversion date", ("conversion_date",), (), False), + ("Customer ref", ("customer_ref",), ("customer_ref",), True), + ("Tracking #", ("tracking_number",), ("tracking_number",), True), + ("Tracking URL", ("tracking_number_url",), (), False), + ("Order #", ("order_no", "order_number"), (), False), + ("Status", ("status",), (), False), +) + + +def _render_so_header_scalar_diffs( + entity: dict[str, Any], + changes: dict[str, FieldChangeView], +) -> None: + """Walk :data:`_SO_HEADER_FIELD_SPEC` and emit a field-diff line per + entry that has either a present change or a truthy unchanged value. + + Each spec row reads ``(label, change_keys, value_keys, render_when_unchanged)``: + + - ``label`` is the user-facing field name. + - ``change_keys`` is the ordered candidates for ``changes.get(...)`` — + first non-None wins. Lets Notes pick up either ``additional_info`` + (wire) or ``notes`` (response shape) without two separate lookups. + - ``value_keys`` is the ordered candidates for the unchanged-value + fall-back; empty tuple means "no fallback — only render on change". + - ``render_when_unchanged`` gates rendering of fields that should + surface their current value even when not part of the modify plan + (Notes, Customer ref, Tracking #). + + Extracted from :func:`_render_so_entity_view` to keep the main entity + view under ruff's complexity budget. The table-driven shape is also + cheaper to evolve as Katana adds new header fields — add a tuple, + no branch. + """ + for label, change_keys, value_keys, render_unchanged in _SO_HEADER_FIELD_SPEC: + change: FieldChangeView | None = None + for key in change_keys: + candidate = changes.get(key) + if candidate is not None: + change = candidate + break + value: Any = None + if render_unchanged: + for key in value_keys: + candidate_value = entity.get(key) + if candidate_value: + value = candidate_value + break + if change is None and value is None: + continue + _render_field_diff_line(label, value=value, change=change) + + +def _render_so_entity_view( + entity: dict[str, Any], + *, + actions: list[dict[str, Any]], + changes: dict[str, FieldChangeView] | None = None, +) -> list[str]: + """Render the sales-order entity view (Tier 2 metrics + Tier 3 + reference fields + per-sub-entity action sections + warnings). + + Companion to :func:`_render_po_entity_view` for the SO modify card. + Unlike the PO entity view which is shared with ``build_po_create_ui``, + the SO create card today uses a tighter inline shape (no notes / + delivery-date diff lines, dedicated shipping-fees section). This + helper is modify-specific — create cards keep their existing inline + rendering until/unless a future consolidation pass merges them. + + Diff-decoration rules (when ``changes`` is set, same as PO): + + - Each rendered field line looks up ``changes.get("")`` + and, if present, swaps its rendering for the before→after form. + - Unchanged fields render the same as the create card. + - Sub-entity actions (rows / addresses / fulfillments / shipping fees) + render grouped by section under their own labelled header, each + action becoming one summary line with kind gutter + per-action + status Badge (applied path only). + + Returns the block-warning list so callers can gate the Confirm + button. Must be called inside + ``with PrefabApp(...) as app, Card(): with CardContent(), Column(gap=3):``. + """ + changes = changes or {} + total = entity.get("total") + currency = entity.get("currency") + item_count = entity.get("item_count") + delivery_date = entity.get("delivery_date") + + # Tier 2 — Decision metrics (un-decorated; the per-field rows below + # carry the diff signal for changed scalars). + if total is not None or item_count is not None or delivery_date: + with Row(gap=4): + if total is not None: + Metric(label="Total", value=_format_money(total, currency)) + if item_count is not None: + Metric(label="Line Items", value=str(item_count)) + if delivery_date: + Metric(label="Delivery", value=str(delivery_date)) + + # Tier 3 — Reference fields. Customer + Location handled as party + # lines; everything else via the shared field-diff helper. + customer_change = changes.get("customer_id") + prior_customer_name = entity.get("customer_name") + if customer_change is not None and customer_change.kind != "unchanged": + _render_party_diff_line( + "Customer", + id_change=customer_change, + name_change=changes.get("customer_name"), + prior_name=prior_customer_name, + ) + else: + _render_party_line( + "Customer", + name=prior_customer_name, + entity_id=entity.get("customer_id"), + entity_kind="customer", + ) + + location_change = changes.get("location_id") + prior_location_name = entity.get("location_name") + if location_change is not None and location_change.kind != "unchanged": + _render_party_diff_line( + "Location", + id_change=location_change, + name_change=changes.get("location_name"), + prior_name=prior_location_name, + ) + else: + # SalesOrderResponse has no location_name today — pass None for + # the name; ``_render_party_line`` falls back to the ID-only + # form. + _render_party_line( + "Location", + name=prior_location_name, + entity_id=entity.get("location_id"), + ) + + # Header scalar diffs — driven by ``_SO_HEADER_FIELD_SPEC``. Each + # entry maps a user-facing label to (change-key candidates, + # entity-value keys, render_value_when_unchanged). The renderer + # emits the diff line only when the field is changing OR carries a + # value worth showing (so the card stays compact on a small modify + # plan but still surfaces unchanged context for the long-form + # update). + _render_so_header_scalar_diffs(entity, changes) + + # Per-sub-entity sections — Line items / Addresses / Fulfillments / + # Shipping fees. Each group reads the operation prefix off the + # action's ``operation`` field and renders the section only when + # actions of that kind are present. The section's rows render from + # ``state.so__rows`` (state-bound via ``ForEach``) so + # the preview→Confirm morph re-paints per-action chrome — see + # :func:`_render_so_subentity_section`. + for section_key, label, ops in _SO_SUBENTITY_GROUPS: + section_actions = [ + a for a in actions if str(a.get("operation") or "").lower() in ops + ] + _render_so_subentity_section(label, section_key, section_actions) + + # NOTE: SO has no build-time ``_render_failed_changes_block`` call. + # The state-driven ``applied_header_failed_*`` Alert (seeded by + # :func:`_so_header_op_failure_alert_text`) is the single source of + # truth for every header-op failure — no-change ops AND update_header + # with field changes — so both the preview→Confirm morph path and the + # standalone-applied path render the failure exactly once, from state. + # Pre-fix #858 finding C: the build-time block was painted from + # preview actions (``succeeded=None``) and stayed at "no failures" + # after the morph, leaving a failed ``update_header`` with changes + # rendering ``applied=True`` chrome with no error text. Sub-entity + # failures still aggregate into their own state-driven Alert (see + # ``build_so_modify_ui``). + + return _render_warnings_block(entity.get("warnings")) + + +# State slots the SO modify card's apply on_success chain must propagate +# from ``$result.state.*`` (the apply tool's PrefabApp envelope) into the +# preview iframe's matching slots so the preview→Confirm in-place morph +# re-paints from the apply outcome. Centralized here so adding a new +# state slot (e.g. ``applied_header_skipped_*`` in #858 round-8) is a +# one-line edit rather than three concurrent edits across seed, render, +# and morph-chain sites. The ``so_*_rows`` slots drive the per-sub-entity +# section ``ForEach`` loops; everything else drives state-bound Badge / +# Alert / mustache strings. +_SO_MODIFY_MORPH_STATE_SLOTS: tuple[str, ...] = ( + "applied_outcome_label", + "applied_outcome_variant", + "applied_subentity_failed_count", + "applied_subentity_failed_summary", + "applied_header_failed_count", + "applied_header_failed_summary", + "applied_header_skipped_count", + "applied_header_skipped_summary", + "so_rows_rows", + "so_addresses_rows", + "so_fulfillments_rows", + "so_shipping_fees_rows", + "applied_verb", +) + + +def _so_modify_morph_setstate_chain() -> list[Action]: + """Build the ``on_success`` SetState chain that morphs the preview + iframe's state slots from the apply tool's ``$result.state.*`` + envelope. + + Mustache form ``{{ $result.state. }}`` is mandatory — bare + ``$result.`` resolves to the apply tool's PrefabApp envelope's + top level (not the raw :class:`ModificationResponse`), which would + leave every slot empty after the morph. Pinned by + ``test_apply_action_morph_chain_writes_*_slots`` in + :file:`test_prefab_ui.py`. + + Returns ``list[Action]`` (not ``list[SetState]``) to match + :func:`_build_apply_action`'s ``extra_on_success`` contract — the + apply-action builder accepts any :class:`Action`, and SetState is + just the concrete one we emit today. + """ + return [ + SetState(slot, f"{{{{ $result.state.{slot} }}}}") + for slot in _SO_MODIFY_MORPH_STATE_SLOTS + ] + + +@dataclass(frozen=True) +class _SOModifyStateSeed: + """Pre-computed state-slot values for the SO modify card. + + Bundled into a dataclass because the slot list has grown over time + (#858 round-8 added ``applied_header_skipped_*``) and a flat keyword + arg list exceeded ruff's :data:`PLR0913` threshold. The dataclass is + construction-only; the caller passes it to + :func:`_seed_so_modify_card_state` which writes the values into the + PrefabApp state dict. + """ + + outcome_label: str + outcome_variant: str + subentity_failed_count: int + subentity_failed_summary: str + header_failed_count: int + header_failed_summary: str + header_skipped_count: int + header_skipped_summary: str + applied_verb: str + subentity_row_lists: dict[str, list[dict[str, Any]]] + + +def _seed_so_modify_card_state( + response: dict[str, Any], + *, + is_preview: bool, + seed: _SOModifyStateSeed, +) -> dict[str, Any]: + """Seed the SO modify card's state slots with build-time pre-morph + defaults. + + Extracted from :func:`build_so_modify_ui` to keep that builder under + ruff's complexity budget — the slot list has grown over time + (#858 round-8 added ``applied_header_skipped_*``) and centralizing + the seeding here means a new slot is a one-line edit, not a 3-site + edit across seed + morph chain + render. + + On the preview path (``is_preview=True``), the outcome slots seed + with ``"APPLIED"`` / ``"default"`` so the preview-time render shows + success chrome by default and the on_success ``SetState`` chain + morphs them to the actual apply outcome. On the standalone-applied + path (``is_preview=False``), the seeded values match the actual + outcome so a fully-failed apply doesn't show success chrome. + + The ``so__rows`` slots are bound 1:1 to the ``ForEach`` + inside :func:`_render_so_subentity_section`; the on_success chain + copies ``$result.state.so_
_rows`` straight in. + """ + state = _init_modify_card_state(response) + state["applied_outcome_label"] = seed.outcome_label if not is_preview else "APPLIED" + state["applied_outcome_variant"] = ( + seed.outcome_variant if not is_preview else "default" + ) + state["applied_subentity_failed_count"] = seed.subentity_failed_count + state["applied_subentity_failed_summary"] = seed.subentity_failed_summary + state["applied_header_failed_count"] = seed.header_failed_count + state["applied_header_failed_summary"] = seed.header_failed_summary + state["applied_header_skipped_count"] = seed.header_skipped_count + state["applied_header_skipped_summary"] = seed.header_skipped_summary + state["applied_verb"] = seed.applied_verb + for section_key, row_list in seed.subentity_row_lists.items(): + state[f"so_{section_key}_rows"] = row_list + return state + + +def _so_actions_with_not_run_tail( + response: dict[str, Any], + *, + is_preview: bool, +) -> list[dict[str, Any]]: + """Return the SO modify card's effective action list, extended (on + the apply path only) with the unattempted plan tail synthesized by + :func:`_modify_sales_order_impl`. + + ``execute_plan`` is fail-fast — ``response.actions`` ends at the + first failed action. The impl stashes the leftover :class:`ActionSpec` + entries under ``response.extras["not_run_actions"]`` so they + participate in per-section row bucketing in :func:`build_so_modify_ui` + (bucketed by ``operation`` prefix the same way executed actions are). + Without this merge the morph path's per-section ``so_
_rows`` + slot would overwrite the preview's full row list with the apply + response's SHORTER list, silently HIDING every planned action past + the failure (#858 finding B; same family as the BOM fix in + ``_modify_product_bom_impl``). + + On the preview path we deliberately ignore the extras (every action + is already PLANNED) — guards against accidental leakage from a test + or future code path that puts NOT-RUN entries on a preview response. + """ + actions: list[dict[str, Any]] = list(response.get("actions") or []) + if is_preview: + return actions + extras = response.get("extras") or {} + not_run_actions = extras.get("not_run_actions") or [] + if not_run_actions: + actions.extend(not_run_actions) + return actions + + +def build_so_modify_ui( + response: dict[str, Any], + *, + confirm_request: BaseModel, + confirm_tool: str, +) -> PrefabApp: + """Build the modify-/delete-/correct-sales-order card (#723). + + Handles every SO write path that returns a :class:`ModificationResponse`: + ``modify_sales_order``, ``delete_sales_order``, ``correct_sales_order``. + Title verb derives from ``confirm_tool`` via :func:`_verb_label` + (``Modify`` / ``Delete`` / ``Correct``). + + SO is more complex than PO because the modify plan can touch multiple + sub-entity types in parallel (header, rows, addresses, fulfillments, + shipping fees). Each sub-entity group renders as its own section + under the entity-view body, grouped by ``operation`` prefix: + + - **Line items** — ``add_row`` / ``update_row`` / ``delete_row``. + - **Addresses** — ``add_address`` / ``update_address`` / + ``delete_address``. Updates render as bare ``field: new`` (no + arrow) because Katana has no per-address GET endpoint to diff + against — every address update would otherwise read + ``(prior unknown) → new`` for every changed field, which is + visual noise that just restates the unknown-prior fact. The + ``address_style=True`` branch in :func:`_format_so_diff_pairs` + collapses to the new-only form so address summary lines stay + compact. + - **Fulfillments** — ``add_fulfillment`` / ``update_fulfillment`` + / ``delete_fulfillment``. + - **Shipping fees** — ``add_shipping_fee`` / ``update_shipping_fee`` + / ``delete_shipping_fee``. + + Apply-state morph (per the BOM modify card's pattern, #811): + + - Header Badge variant: ``state.applied_outcome_label`` / + ``applied_outcome_variant`` flip between APPLIED / PARTIAL + FAILURE / FAILED on the morph; on_success chain SetState reads + ``{{ $result.state.applied_outcome_label }}`` (NOT + ``$result.``; ``$result`` resolves to the apply tool's + PrefabApp envelope, not the raw ``ModificationResponse``). + - Sub-entity failed-action Alert: gated by + ``If(Rx("applied_subentity_failed_count") > 0)``; its summary + text seeds via ``applied_subentity_failed_summary``. + - Footer body verb: passed as ``"{{ applied_verb }}"`` mustache so + it morphs in lockstep with the outcome Badge. + """ + is_preview = bool(response.get("is_preview", True)) + actions: list[dict[str, Any]] = _so_actions_with_not_run_tail( + response, is_preview=is_preview + ) + entity_id = response.get("entity_id") + prior_state = _normalize_so_prior_state(response.get("prior_state")) + + verb_label = _verb_label(confirm_tool) + # Compose the entity view's source-of-truth dict by overlaying the + # response on top of the normalized prior_state. Same shape as the + # PO modify card. + entity = {**prior_state, **{k: v for k, v in response.items() if v is not None}} + if entity_id is not None: + entity.setdefault("id", entity_id) + + # Header-only changes map — drives the header scalar-diff rendering + # (:func:`_render_so_header_scalar_diffs`) only. Header failures are + # NOT painted from this map; they flow through the state-driven + # ``applied_header_failed_*`` Alert seeded by + # :func:`_so_header_op_failure_alert_text`. Sub-entity actions render + # in their own sections via ``_SO_SUBENTITY_GROUPS`` — including their + # field changes here would let sub-entity field names that overlap + # header names (``status``, ``picked_date``, ``tracking_number`` on + # fulfillments) overwrite real header diffs. + changes_by_field = _index_changes_by_field( + actions, include_operations=_SO_HEADER_OPS + ) + + # Sub-entity failed-action summary — pre-formatted at build time and + # seeded into state slots so the preview→Confirm in-place morph can + # surface failures (the Python-painted per-action rows above carry + # ✗ glyphs but the textual errors aggregate here). + subentity_failed_count, subentity_failed_summary = _so_subentity_failed_summary( + actions, + so_id=entity_id if isinstance(entity_id, int) else None, + confirm_tool=confirm_tool, + ) + + # Header-level failed-op summary — the SINGLE source of truth for + # header failures on the SO modify card. Intentionally includes + # failed ``update_header`` actions WITH field changes (round 8) as + # well as failed top-level ``delete`` actions; sub-entity failures + # are surfaced separately by ``_so_subentity_failed_summary`` + # (which filters out header ops to avoid double-rendering). + header_failed_count, header_failed_summary = _so_header_op_failure_alert_text( + actions + ) + + # Header-level NOT-RUN (skipped) summary — covers the round-8 gap + # left by ``_index_changes_by_field`` filtering NOT-RUN ops out of + # the header field map (#858 round 7) combined with + # :func:`_build_so_subentity_row_lists` only bucketing sub-entity + # ops. Without this Alert, a failed ``correct_sales_order`` whose + # close-phase ``update_header`` is skipped renders only sub-entity + # NOT-RUN rows — the operator can't tell the SO close/restore step + # never ran. + header_skipped_count, header_skipped_summary = _so_header_op_skipped_alert_text( + actions + ) + + # Per-sub-entity row dicts — seed every section's row list with the + # build-time view (PLANNED at preview, APPLIED/FAILED on the + # standalone-applied path). The on_success ``SetState`` chain below + # reads ``$result.state.so_
_rows`` to overwrite these on + # the in-place morph so per-row chrome (gutter glyph, status Badge) + # updates from the apply outcome — see #858 Copilot finding A. + subentity_row_lists = _build_so_subentity_row_lists(actions) + + # Outcome bucketing — same vocabulary as PO modify (APPLIED / + # PARTIAL FAILURE / FAILED). + outcome_label, outcome_variant = _summarize_apply_outcome(actions) + + # SetState chain is extracted into ``_so_modify_morph_setstate_chain`` + # so the per-slot ``{{ $result.state. }}`` mustache pattern stays + # centralized. See :data:`_SO_MODIFY_MORPH_STATE_SLOTS` for the + # canonical slot list and the Rx-context contract that drives every + # state-bound Badge, Alert, and section row-list on this card. + apply_action = _build_apply_action( + confirm_tool, + confirm_request, + extra_on_success=_so_modify_morph_setstate_chain(), + ) + # ``_build_cancel_action`` interpolates its arg into "Cancel: do not + # apply X." — noun-phrase forms so the message reads naturally. Same + # pattern as PO #755. + if verb_label == "Delete": + cancel_operation_label = "that sales order deletion" + elif verb_label == "Correct": + cancel_operation_label = "those sales order corrections" + else: + cancel_operation_label = "those sales order changes" + cancel_action = _build_cancel_action(cancel_operation_label) + + # Delete cards say "Deleted" / "DELETED" / "deleted." in applied + # state; modify and correct cards say "Applied" / "APPLIED" / + # "applied.". Failure overrides the applied copy below. + if verb_label == "Delete": + applied_title_suffix = "Deleted" + applied_state_label = "DELETED" + applied_verb = "deleted" + else: + applied_title_suffix = "Applied" + applied_state_label = "APPLIED" + applied_verb = "applied" + applied_state_variant: str = "default" + + # Standalone-applied path (is_preview=False): drive the state + # label + variant + title + verb from the actual outcome so a + # fully-failed apply doesn't render with success chrome. Same + # contract as PO modify (caught by Copilot on #755). + if not is_preview and outcome_label != "APPLIED": + applied_state_label = outcome_label + applied_state_variant = outcome_variant + if outcome_label == "FAILED": + applied_title_suffix = "Failed" + applied_verb = "failed" + else: # PARTIAL FAILURE + applied_title_suffix = "Partially Applied" + applied_verb = "partially applied" + + # State seeding is extracted into ``_seed_so_modify_card_state`` to + # keep this builder under ruff's complexity budget — the slot list + # has grown (#858 round-8 added ``applied_header_skipped_*``) and a + # future addition shouldn't force a refactor each time. The seeded + # values are the build-time pre-morph defaults; the on_success + # ``SetState`` chain (see :func:`_so_modify_morph_setstate_chain`) + # overwrites them from ``$result.state.*`` on the morph. + state = _seed_so_modify_card_state( + response, + is_preview=is_preview, + seed=_SOModifyStateSeed( + outcome_label=outcome_label, + outcome_variant=outcome_variant, + subentity_failed_count=subentity_failed_count, + subentity_failed_summary=subentity_failed_summary, + header_failed_count=header_failed_count, + header_failed_summary=header_failed_summary, + header_skipped_count=header_skipped_count, + header_skipped_summary=header_skipped_summary, + applied_verb=applied_verb, + subentity_row_lists=subentity_row_lists, + ), + ) + + with ( + PrefabApp(state=state, css_class="p-4") as app, + Card(), + ): + _render_preview_header( + title_prefix=f"{verb_label} Sales Order", + entity="sales_order", + order_number=str(entity.get("order_number") or entity_id or "N/A"), + status=entity.get("status"), + applied_title_suffix=applied_title_suffix, + applied_state_label=applied_state_label, + applied_state_variant=applied_state_variant, + ) + with CardContent(), Column(gap=3): + if response.get("message"): + Muted(content=response["message"]) + block_warnings = _render_so_entity_view( + entity, + actions=actions, + changes=changes_by_field, + ) + + # State-driven sub-entity failed-action Alert. Mirrors the + # BOM modify card's ``applied_failed_count`` / ``applied_failed_summary`` + # pattern — gated by ``If(Rx(...) > 0)`` so it stays hidden + # at preview time and after a fully-successful apply, then + # pops in after a partial/full failure morph. Seeded from + # ``$result.state.*`` so the preview→Confirm in-place morph + # surfaces failures the build-time render couldn't predict. + with ( + If(Rx("applied_subentity_failed_count") > 0), + Alert(variant="destructive", icon="circle-alert"), + ): + AlertTitle( + content="{{ applied_subentity_failed_count }} sub-entity failure(s)" + ) + AlertDescription(content="{{ applied_subentity_failed_summary }}") + + # State-driven header-op failure Alert (#858 finding B). + # SINGLE source of truth for header failures on the SO modify + # card — covers BOTH failed top-level ``delete`` actions AND + # failed ``update_header`` actions WITH field changes (round + # 8). ``_so_subentity_failed_summary`` filters out header ops + # so this Alert is the only surface they appear on. Gated + # separately from the sub-entity Alert so header failures + # surface under a truthful "header op" title instead of being + # misattributed as a sub-entity failure. + with ( + If(Rx("applied_header_failed_count") > 0), + Alert(variant="destructive", icon="circle-alert"), + ): + AlertTitle(content="Sales order operation failed") + AlertDescription(content="{{ applied_header_failed_summary }}") + + # State-driven header-op NOT-RUN Alert (#858 round-8 follow-up). + # Mirrors the failed-op Alert above but for skipped header + # steps — fires when a fail-fast ``correct_sales_order`` halts + # before its close-phase ``update_header`` runs. Variant + # ``info`` (neutral, the closest fit in :data:`AlertVariant` to + # "didn't happen") so the destructive variant above keeps + # exclusive ownership of the failure surface. Without this + # Alert the operator couldn't tell the SO close/restore step + # never ran — sub-entity NOT-RUN rows render in their own + # section but a skipped header op had no rendering surface + # (header field map filters NOT-RUN out per round 7, and the + # sub-entity row lists only bucket sub-entity ops). + with ( + If(Rx("applied_header_skipped_count") > 0), + Alert(variant="info", icon="circle-info"), + ): + AlertTitle( + content="{{ applied_header_skipped_count }} header step(s) skipped" + ) + AlertDescription(content="{{ applied_header_skipped_summary }}") + + if is_preview: + with If("error"): + Separator() + with Alert(variant="destructive", icon="circle-alert"): + AlertTitle(content="Apply failed") + AlertDescription(content="{{ error }}") + + # Confirm label scales with the planned-action count. Delete + # cards use "Confirm Delete" to mirror the destructive + # affordance. + n_actions = len(actions) + if verb_label == "Delete": + confirm_label = "Confirm Delete" + elif n_actions > 1: + confirm_label = f"Confirm {n_actions} changes" + else: + confirm_label = "Confirm Changes" + + _render_preview_footer( + title_prefix=f"Sales Order {verb_label}", + block_warnings=block_warnings, + confirm_label=confirm_label, + apply_action=apply_action, + cancel_action=cancel_action, + # No next-action buttons on modify cards by default — same + # rationale as PO. The user already had the SO they wanted + # to change; surfacing "Fulfill Order" here would be noise. + next_action_buttons=(), + # Mustache template against ``state.applied_verb`` (seeded + # above + overwritten by the on_success chain) so the footer + # body morphs to "Sales Order partially applied." / "Sales + # Order failed." in lockstep with the Tier-1 outcome Badge. + applied_verb="{{ applied_verb }}", + ) + return app + + def build_mo_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 012dde736..3437e6d96 100644 --- a/katana_mcp_server/tests/browser/render_test_server.py +++ b/katana_mcp_server/tests/browser/render_test_server.py @@ -37,6 +37,7 @@ build_product_bom_ui, build_search_results_ui, build_so_create_ui, + build_so_modify_ui, build_stock_adjustment_create_ui, build_stock_adjustment_delete_ui, build_stock_adjustment_update_ui, @@ -966,6 +967,487 @@ def _bom_modify_response(*, is_preview: bool, succeeded: bool | None) -> dict: } +def _so_failed_delete_response(*, is_preview: bool = False) -> dict: + """Build a canned SO delete response where the apply fails. + + Mirrors the failure shape Katana returns when the SO is already + gone (404) or in a state that blocks the cascade. Pre-fix #858 + finding B the FAILED chrome rendered without the actual error text + — delete actions carry no field changes (so the header-changes + block was empty) and the sub-entity failure summary filters out + top-level ``delete`` (so that block was empty too). Post-fix the + dedicated header-op Alert surfaces the :attr:`ActionResult.error`. + + ``is_preview`` flips between the pre-Confirm preview shape (delete + pending) and the post-Confirm apply result (delete FAILED). + """ + delete_succeeded = None if is_preview else False + actions = [ + ActionResult( + index=1, + operation="delete", + target_id=42, + changes=[], + succeeded=delete_succeeded, + error=None + if is_preview + else "404 Not Found: sales order 42 does not exist", + status_label="" if is_preview else "FAILED", + ).model_dump(), + ] + return { + "entity_type": "sales_order", + "entity_id": 42, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions, + "prior_state": { + "id": 42, + "order_no": "SO-2026-001", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "NOT_SHIPPED", + "currency": "USD", + "total": 1250.0, + }, + "warnings": [], + "next_actions": [], + "katana_url": "https://factory.katanamrp.com/salesorder/42", + "message": "Preview: 1 action(s) planned" if is_preview else "Delete failed", + } + + +def _so_modify_partial_failure_response(*, is_preview: bool = False) -> dict: + """Build a canned SO modify response with a partial-failure plan. + + Mirrors the production shape for the #723 SO modify card: + + - Header change (status: NOT_SHIPPED → PACKED) — applies cleanly. + - One shipping_fee add — applies cleanly. + - One row delete — FAILS (404 — stale row id) on apply. + + ``is_preview`` flips the response between the pre-Confirm preview + (every action ``succeeded=None``) and the post-Confirm apply result + (header + shipping_fee succeeded, delete_row failed). The same + actions are returned in both cases so the morph test can assert + that the Confirm-time apply call produces the partial-failure + chrome (Alert + per-action FAILED Badge + card-level + PARTIAL FAILURE state) on top of the preview tree. + + On the applied path the card-level state badge reads PARTIAL + FAILURE, the state-driven sub-entity failed-action Alert shows + the failed row's error + retry coaching, and the Python-painted + action rows in the Line items + Shipping fees sections carry + per-action APPLIED / FAILED Badges. + """ + # On preview every action is pending (``succeeded=None``); on apply + # the header + shipping_fee succeed and the row delete fails. The + # failure path is what makes this the "interesting" morph fixture. + header_succeeded = None if is_preview else True + fee_succeeded = None if is_preview else True + row_succeeded = None if is_preview else False + actions = [ + ActionResult( + index=1, + operation="update_header", + target_id=42, + changes=[ + FieldChange(field="status", old="NOT_SHIPPED", new="PACKED"), + ], + succeeded=header_succeeded, + ).model_dump(), + ActionResult( + index=2, + operation="add_shipping_fee", + target_id=None, + changes=[ + FieldChange( + field="description", + old=None, + new="Express ground", + is_added=True, + ), + FieldChange(field="amount", old=None, new="12.99", is_added=True), + ], + succeeded=fee_succeeded, + status_label="" if is_preview else "APPLIED", + ).model_dump(), + ActionResult( + index=3, + operation="delete_row", + target_id=9999, + changes=[], + succeeded=row_succeeded, + error=None + if is_preview + else "404 Not Found: row 9999 does not exist on SO 42", + status_label="" if is_preview else "FAILED", + ).model_dump(), + ] + if is_preview: + message = "Preview: 3 action(s) planned" + else: + message = "Applied 2 of 3 action(s); 1 failure" + return { + "entity_type": "sales_order", + "entity_id": 42, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions, + "prior_state": { + "id": 42, + "order_no": "SO-2026-001", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "NOT_SHIPPED", + "currency": "USD", + "total": 1250.0, + "additional_info": "Customer requested expedited delivery", + "delivery_date": "2026-05-08T14:30:00Z", + }, + "warnings": [], + "next_actions": [], + "katana_url": "https://factory.katanamrp.com/salesorder/42", + "message": message, + } + + +def _so_modify_fail_fast_not_run_response(*, is_preview: bool = False) -> dict: + """Build a canned SO modify response exercising the fail-fast NOT-RUN + tail (#858 finding B). + + The plan has 4 row adds — the first succeeds, the second fails, and + the third + fourth NEVER RUN (fail-fast halts after the second). + The apply path mirrors what ``_modify_sales_order_impl`` produces: + ``response.actions`` holds the 2 executed entries; the unattempted + tail rides on ``response.extras["not_run_actions"]`` as NOT-RUN + action dicts. :func:`build_so_modify_ui` merges the two so the + morphed Line items section renders all 4 rows (APPLIED + FAILED + + NOT RUN + NOT RUN) instead of silently dropping the leftover plan. + + On preview, every action is pending (``succeeded=None``) and the + extras dict is empty — the preview already shows the full 4-row + plan via the regular ``actions`` list. The morph between preview + and apply is the load-bearing assertion: pre-fix the apply morph + would HIDE the NOT-RUN rows; post-fix they remain visible. + """ + succeeded_states = [ + # Action 1: succeeds. + None if is_preview else True, + # Action 2: fails — execute_plan stops here on apply. + None if is_preview else False, + # Actions 3-4: never attempted on apply. + None, + None, + ] + error_states = [ + None, + None if is_preview else "422 Unprocessable: variant 999 archived", + None, + None, + ] + actions_full = [ + ActionResult( + index=i + 1, + operation="add_row", + target_id=None, + changes=[ + FieldChange( + field="variant_id", + old=None, + new=100 + i, + is_added=True, + ), + FieldChange( + field="quantity", + old=None, + new=f"{i + 1}.0", + is_added=True, + ), + ], + succeeded=succeeded_states[i], + error=error_states[i], + ).model_dump() + for i in range(4) + ] + # On apply, ``response.actions`` is truncated to the 2 executed + # entries (fail-fast) and the unattempted tail rides on extras as + # NOT-RUN dicts (matches what ``_modify_sales_order_impl`` produces + # post-#858-fix). + if is_preview: + actions = actions_full + extras: dict[str, Any] = {} + message = "Preview: 4 action(s) planned" + else: + actions = actions_full[:2] + extras = { + "not_run_actions": [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "error": None, + "status_label": "NOT RUN", + "changes": [ + { + "field": "variant_id", + "old": None, + "new": 100 + i, + "is_added": True, + }, + { + "field": "quantity", + "old": None, + "new": f"{i + 1}.0", + "is_added": True, + }, + ], + } + for i in (2, 3) + ] + } + message = "Applied 1 of 4 action(s); 1 failure; 2 not run" + return { + "entity_type": "sales_order", + "entity_id": 42, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions, + "prior_state": { + "id": 42, + "order_no": "SO-2026-001", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "NOT_SHIPPED", + "currency": "USD", + "total": 1250.0, + "additional_info": "Customer requested expedited delivery", + "delivery_date": "2026-05-08T14:30:00Z", + }, + "warnings": [], + "next_actions": [], + "katana_url": "https://factory.katanamrp.com/salesorder/42", + "message": message, + "extras": extras, + } + + +def _so_modify_header_failed_with_changes_response(*, is_preview: bool = False) -> dict: + """Build a canned SO modify response exercising the failed + ``update_header`` WITH field changes morph path (#858 finding C). + + A single ``update_header`` action changes ``status`` from + NOT_SHIPPED → DELIVERED; on apply the Katana API rejects the + transition (e.g. invalid state machine move). Pre-fix, the build- + time ``_render_failed_changes_block`` (read at preview time when + ``succeeded=None``) painted "no failures" into the view tree and + NEVER updated on the morph — the operator saw APPLIED chrome + despite the failure. + + Post-fix, :func:`_so_header_op_failure_alert_text` is the single + source of truth: it includes failed ``update_header`` actions WITH + changes too, so the state-driven Alert pops in after the morph + with the error text. The build-time + ``_render_failed_changes_block`` was removed from the SO entity + view to avoid double-render. + """ + succeeded = None if is_preview else False + error = None if is_preview else "422 Unprocessable: invalid status transition" + actions = [ + ActionResult( + index=1, + operation="update_header", + target_id=42, + changes=[ + FieldChange(field="status", old="PACKED", new="DELIVERED"), + ], + succeeded=succeeded, + error=error, + ).model_dump(), + ] + if is_preview: + message = "Preview: 1 action(s) planned" + else: + message = "Failed: 1 action(s); 1 failure" + return { + "entity_type": "sales_order", + "entity_id": 42, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions, + "prior_state": { + "id": 42, + "order_no": "SO-2026-001", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "PACKED", + "currency": "USD", + "total": 1250.0, + "additional_info": "Customer requested expedited delivery", + "delivery_date": "2026-05-08T14:30:00Z", + }, + "warnings": [], + "next_actions": [], + "katana_url": "https://factory.katanamrp.com/salesorder/42", + "message": message, + } + + +def _so_correct_fail_fast_header_skipped_response(*, is_preview: bool = False) -> dict: + """Build a canned ``correct_sales_order`` response exercising the + header-step NOT-RUN morph path (#858 round-8). + + Plan: delete fulfillment (phase 1, succeeds) → revert SO header + (phase 2, succeeds) → edit SO row (phase 3, FAILS). Phase 4 + (re-create fulfillment) and phase 5 (close SO via update_header) + never run. The close-phase ``update_header`` is the load-bearing + NOT-RUN entry: pre-round-8 it had no rendering surface (sub-entity + row lists only bucket sub-entity ops; the header field map filters + NOT-RUN out per round 7), so the operator couldn't tell the SO + close step was skipped. Post-round-8 the state-driven + ``applied_header_skipped_*`` Alert surfaces the skipped step. + """ + actions_executed: list[dict[str, Any]] = [] + if not is_preview: + actions_executed = [ + ActionResult( + index=1, + operation="delete_fulfillment", + target_id=77, + changes=[], + succeeded=True, + error=None, + ).model_dump(), + ActionResult( + index=2, + operation="update_header", + target_id=99, + changes=[FieldChange(field="status", new="PENDING")], + succeeded=True, + error=None, + ).model_dump(), + ActionResult( + index=3, + operation="update_row", + target_id=10, + changes=[FieldChange(field="variant_id", old=500, new=501)], + succeeded=False, + error="Katana refused the row edit", + ).model_dump(), + ] + else: + # Preview: every action is pending (succeeded=None), no extras. + actions_executed = [ + ActionResult( + index=1, + operation="delete_fulfillment", + target_id=77, + changes=[], + succeeded=None, + error=None, + ).model_dump(), + ActionResult( + index=2, + operation="update_header", + target_id=99, + changes=[FieldChange(field="status", new="PENDING")], + succeeded=None, + error=None, + ).model_dump(), + ActionResult( + index=3, + operation="update_row", + target_id=10, + changes=[FieldChange(field="variant_id", old=500, new=501)], + succeeded=None, + error=None, + ).model_dump(), + ActionResult( + index=4, + operation="add_fulfillment", + target_id=None, + changes=[], + succeeded=None, + error=None, + ).model_dump(), + ActionResult( + index=5, + operation="update_header", + target_id=99, + changes=[FieldChange(field="status", new="DELIVERED")], + succeeded=None, + error=None, + ).model_dump(), + ] + + extras: dict[str, Any] = {} + if not is_preview: + extras = { + "not_run_actions": [ + { + "operation": "add_fulfillment", + "target_id": None, + "succeeded": None, + "error": None, + "status_label": "NOT RUN", + "changes": [], + }, + { + "operation": "update_header", + "target_id": 99, + "succeeded": None, + "error": None, + "status_label": "NOT RUN", + "changes": [ + { + "field": "status", + "old": None, + "new": "DELIVERED", + } + ], + }, + ] + } + + if is_preview: + message = "Preview: 5 action(s) planned" + else: + message = "Applied 2 of 3 action(s); 1 failure; 2 not run" + return { + "entity_type": "sales_order", + "entity_id": 99, + "is_preview": is_preview, + "operation": "", + "changes": [], + "actions": actions_executed, + "prior_state": { + "id": 99, + "order_no": "SO-2026-002", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "DELIVERED", + "currency": "USD", + "total": 1250.0, + "additional_info": "Customer requested expedited delivery", + "delivery_date": "2026-05-08T14:30:00Z", + }, + "warnings": [], + "next_actions": [], + "katana_url": "https://factory.katanamrp.com/salesorder/99", + "message": message, + "extras": extras, + } + + 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 @@ -1201,6 +1683,100 @@ def _bom_modify_response(*, is_preview: bool, succeeded: bool | None) -> dict: confirm_request=_StubRequest(), confirm_tool="manage_product_bom", ), + # #723 — SO modify card with a partial-failure applied state. Pins + # the card-level PARTIAL FAILURE badge, the state-driven sub-entity + # failed-action Alert, the per-action APPLIED / FAILED Badges in + # the Line items + Shipping fees sections, and the failed-row + # ``✗ `` gutter in a real browser render. + "so_modify_partial_failure_applied": lambda: build_so_modify_ui( + _so_modify_partial_failure_response(), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ), + # #723 / #858 — preview side of the same partial-failure plan. Used + # by the click-through morph test: the iframe renders this preview + # tree, the user clicks Confirm, the apply call returns the applied + # response (via the ``modify_sales_order`` stub tool below), and the + # on_success SetState chain flips the state slots so the sub-entity + # failed-action Alert pops in. Catches slot-name-typo regressions + # the standalone-applied test cannot. + "so_modify_partial_failure_preview": lambda: build_so_modify_ui( + _so_modify_partial_failure_response(is_preview=True), + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order", + ), + # #858 finding B — failed top-level delete renders FAILED chrome WITH + # the dedicated header-op Alert surfacing the ActionResult.error text. + # Pre-fix this rendered with no visible error message. + "so_delete_failed_applied": lambda: build_so_modify_ui( + _so_failed_delete_response(is_preview=False), + confirm_request=_StubRequest(id=42), + confirm_tool="delete_sales_order", + ), + "so_delete_failed_preview": lambda: build_so_modify_ui( + _so_failed_delete_response(is_preview=True), + confirm_request=_StubRequest(id=42), + confirm_tool="delete_sales_order", + ), + # #858 finding B (NOT-RUN morph) — fail-fast leaves the unattempted + # plan tail visible as NOT-RUN rows on the morphed card instead of + # silently dropping them. Pre-fix: 4-row plan that fails on row 2 + # morphed to a card showing only 2 rows. + "so_modify_fail_fast_not_run_applied": lambda: build_so_modify_ui( + _so_modify_fail_fast_not_run_response(is_preview=False), + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_fail_fast", + ), + "so_modify_fail_fast_not_run_preview": lambda: build_so_modify_ui( + _so_modify_fail_fast_not_run_response(is_preview=True), + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_fail_fast", + ), + # #858 Copilot follow-up (comment 3312071378) — same NOT-RUN morph + # contract, but for ``correct_sales_order``. ``build_so_modify_ui`` + # handles both tools and merges ``extras["not_run_actions"]`` the + # same way; this scenario pins that ``_correct_sales_order_impl``'s + # failure path populates those extras correctly so the morphed card + # surfaces the unattempted restore / recreate / close phases. + "so_correct_fail_fast_not_run_applied": lambda: build_so_modify_ui( + _so_modify_fail_fast_not_run_response(is_preview=False), + confirm_request=_StubRequest(id=42), + confirm_tool="correct_sales_order", + ), + "so_correct_fail_fast_not_run_preview": lambda: build_so_modify_ui( + _so_modify_fail_fast_not_run_response(is_preview=True), + confirm_request=_StubRequest(id=42), + confirm_tool="correct_sales_order", + ), + # #858 round-8 — fail-fast ``correct_sales_order`` whose close-phase + # ``update_header`` lands in the NOT-RUN tail. Surfaces the + # ``applied_header_skipped_*`` Alert that owns the rendering surface + # for skipped header steps (sub-entity row lists only bucket sub- + # entity ops; the header field map filters NOT-RUN out per round 7). + "so_correct_fail_fast_header_skipped_applied": lambda: build_so_modify_ui( + _so_correct_fail_fast_header_skipped_response(is_preview=False), + confirm_request=_StubRequest(id=99), + confirm_tool="correct_sales_order", + ), + "so_correct_fail_fast_header_skipped_preview": lambda: build_so_modify_ui( + _so_correct_fail_fast_header_skipped_response(is_preview=True), + confirm_request=_StubRequest(id=99), + confirm_tool="correct_sales_order", + ), + # #858 finding C — failed update_header WITH field changes morphs + # to a card with the state-driven header-op Alert visible (was + # invisible pre-fix because the build-time ``_render_failed_changes_block`` + # painted preview-time content into the view tree). + "so_modify_header_failed_with_changes_applied": lambda: build_so_modify_ui( + _so_modify_header_failed_with_changes_response(is_preview=False), + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_header_fail", + ), + "so_modify_header_failed_with_changes_preview": lambda: build_so_modify_ui( + _so_modify_header_failed_with_changes_response(is_preview=True), + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_header_fail", + ), } @@ -1275,6 +1851,142 @@ async def modify_manufacturing_order( return make_tool_result(response, ui=ui) +@mcp.tool(meta={"ui": True}) +async def modify_sales_order( + id: int, + preview: bool = True, + update_header: Any = None, + add_rows: Any = None, + update_rows: Any = None, + delete_row_ids: Any = None, + add_addresses: Any = None, + update_addresses: Any = None, + delete_address_ids: Any = None, + add_fulfillments: Any = None, + update_fulfillments: Any = None, + delete_fulfillment_ids: Any = None, + add_shipping_fees: Any = None, + update_shipping_fees: Any = None, + delete_shipping_fee_ids: Any = None, +) -> ToolResult: + """Stub for the SO modify click-through test — when the Confirm button + on the preview card fires, the iframe calls this with ``preview=False`` + and we return the canned partial-failure apply response so the + on_success SetState chain in :func:`build_so_modify_ui` fires. + + Sub-payload args mirror :class:`ModifySalesOrderRequest` so FastMCP's + signature validator accepts the full Confirm-button payload. All + values are ignored — the stub always returns the same canned envelope. + + Uses ``make_tool_result`` (same helper the real ``modify_sales_order`` + tool uses) so the wire shape matches production exactly: ``content`` + carries the response JSON, ``structured_content`` carries the apply + result card's Prefab envelope (which is what ``$result.state.*`` + resolves against in the on_success chain). + """ + del ( + id, + update_header, + add_rows, + update_rows, + delete_row_ids, + add_addresses, + update_addresses, + delete_address_ids, + add_fulfillments, + update_fulfillments, + delete_fulfillment_ids, + add_shipping_fees, + update_shipping_fees, + delete_shipping_fee_ids, + ) # unused — canned response + response_dict = _so_modify_partial_failure_response(is_preview=preview) + response = ModificationResponse.model_validate(response_dict) + ui = build_so_modify_ui( + response_dict, + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order", + ) + return make_tool_result(response, ui=ui) + + +@mcp.tool(meta={"ui": True}) +async def delete_sales_order( + id: int, + preview: bool = True, +) -> ToolResult: + """Stub for the SO delete click-through test (#858 finding B) — + Confirm on the preview card fires this with ``preview=False`` and + the stub returns the failed-delete apply response so the + state-driven header-op Alert pops in via the on_success chain. + + Wire shape matches the real ``delete_sales_order`` tool exactly so + the morph contract pins the same behavior production does. + """ + del id # unused — canned response + response_dict = _so_failed_delete_response(is_preview=preview) + response = ModificationResponse.model_validate(response_dict) + ui = build_so_modify_ui( + response_dict, + confirm_request=_StubRequest(id=42), + confirm_tool="delete_sales_order", + ) + return make_tool_result(response, ui=ui) + + +@mcp.tool(meta={"ui": True}) +async def modify_sales_order_fail_fast( + id: int, + preview: bool = True, +) -> ToolResult: + """Stub for the #858 finding B (NOT-RUN) click-through test — + distinct ``confirm_tool`` so the click-through test can target a + different canned response than the ``modify_sales_order`` partial- + failure stub above. Confirm on the preview re-issues this with + ``preview=False``; the canned apply response carries 2 executed + actions + 2 NOT-RUN entries on ``extras["not_run_actions"]``, which + :func:`build_so_modify_ui` merges into the Line items section so + all 4 rows remain visible on the morphed card. + """ + del id # unused — canned response + response_dict = _so_modify_fail_fast_not_run_response(is_preview=preview) + response = ModificationResponse.model_validate(response_dict) + ui = build_so_modify_ui( + response_dict, + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_fail_fast", + ) + return make_tool_result(response, ui=ui) + + +@mcp.tool(meta={"ui": True}) +async def modify_sales_order_header_fail( + id: int, + preview: bool = True, +) -> ToolResult: + """Stub for the #858 finding C click-through test — failed + ``update_header`` WITH field changes. Confirm re-issues with + ``preview=False``; the canned apply response has ``succeeded=False`` + on the update_header action with a field change in ``changes``. + + The morph-target assertion is that the state-driven + ``applied_header_failed_*`` Alert pops in (gated by + ``If(Rx("applied_header_failed_count") > 0)``) with the 422 error + text — pre-fix the build-time ``_render_failed_changes_block`` + painted preview-time content into the view tree and stayed at + "no failures" on the morph. + """ + del id # unused — canned response + response_dict = _so_modify_header_failed_with_changes_response(is_preview=preview) + response = ModificationResponse.model_validate(response_dict) + ui = build_so_modify_ui( + response_dict, + confirm_request=_StubRequest(id=42), + confirm_tool="modify_sales_order_header_fail", + ) + return make_tool_result(response, ui=ui) + + class _EchoVariantDetails(BaseModel): """Echoes back the arguments the host actually passed to ``get_variant_details`` — used by the #494 row-click tests to verify diff --git a/katana_mcp_server/tests/browser/test_modification_render.py b/katana_mcp_server/tests/browser/test_modification_render.py index eebee47f6..4bceb5ce4 100644 --- a/katana_mcp_server/tests/browser/test_modification_render.py +++ b/katana_mcp_server/tests/browser/test_modification_render.py @@ -113,6 +113,330 @@ def test_bom_modify_mixed_applied_renders_per_row_status(self, render_scenario): # the rendered table. assert frame.locator("td").filter(has_text="APPLIED").count() >= 4 + def test_so_modify_partial_failure_applied_renders(self, render_scenario): + """#723 SO modify card — partial-failure applied state renders the + card-level PARTIAL FAILURE badge, the per-action APPLIED / FAILED + Badges in the Line items + Shipping fees sections, and the + consolidated sub-entity failed-action Alert with retry coaching. + + Pins end-to-end browser-render correctness for the SO modify + card's most complex state — multiple parallel sub-entity outcomes + where some succeed and some fail. + """ + frame = render_scenario("so_modify_partial_failure_applied") + # Header carries the SO order_no badge. + assert frame.locator("text=SO-2026-001").count() >= 1 + # Card-level PARTIAL FAILURE state Badge (the build-time outcome + # bucketing — ``_summarize_apply_outcome`` reads the response + # actions on the standalone-applied path). + assert frame.locator("text=PARTIAL FAILURE").count() >= 1 + # Per-section headers — Line items and Shipping fees are present + # because the plan touches both; Fulfillments and Addresses are + # not (no actions, no section header). + assert frame.locator("text=Line items:").count() >= 1 + assert frame.locator("text=Shipping fees:").count() >= 1 + # Per-action status pills surface APPLIED on the successful + # actions; FAILED on the failed one. ``count() >= 1`` because + # multiple APPLIED Badges (one for the row that succeeded plus + # potentially the card-level chrome). + assert frame.locator("text=APPLIED").count() >= 1 + assert frame.locator("text=FAILED").count() >= 1 + # Sub-entity failed-action Alert — title + retry-coaching tail. + assert frame.locator("text=404 Not Found").count() >= 1 + assert frame.locator("text=modify_sales_order(id=42").count() >= 1 + + def test_so_modify_partial_failure_morphs_preview_to_applied(self, render_scenario): + """#723 / #858 — Click-through morph test for the SO modify card. + + The standalone-applied scenario seeds the partial-failure state + slots directly in Python; a slot-name typo in the + :func:`build_so_modify_ui` Confirm-button SetState chain would + sneak past that test. This test wires the **preview→Confirm→ + on_success morph path** end to end: + + 1. Render the SO modify preview card via + ``so_modify_partial_failure_preview``. + 2. Confirm a baseline preview shape — Confirm button visible, + no sub-entity failed-action Alert yet, no per-row FAILED + Badge / ``✗`` gutter (preview rows render as PLANNED with no + Badge). + 3. Click Confirm. The iframe re-issues ``modify_sales_order`` + with ``preview=False``; the stub tool in + :mod:`render_test_server` returns the partial-failure apply + response. + 4. After the morph lands, assert (a) the state-driven sub-entity + failed-action Alert has appeared with the error text and (b) + the per-row chrome has morphed — the failed row carries a + FAILED Badge and the ``✗`` gutter (#858 finding A). Without + the row-binding fix the row text + Badge stayed frozen on + the preview-time PLANNED state. + + Each per-section row list lives in ``state.so_
_rows`` + (see ``_build_so_subentity_row_lists``) and renders via + ``ForEach``. The on_success chain SetStates each section list + from ``$result.state.so_
_rows`` so the row chrome + re-paints in lockstep with the apply outcome. + """ + frame = render_scenario("so_modify_partial_failure_preview") + + # Pre-state: Confirm button visible, no failure alert yet (the + # preview seeds ``applied_subentity_failed_count=0`` so the If + # gate is closed). + assert frame.locator("button").filter(has_text="Confirm").first.is_visible() + # The failed-row error text only shows when the Alert is open + # (the morph hasn't fired yet, so the bound text is the empty + # preview seed). + assert frame.locator("text=404 Not Found").count() == 0 + # Per-row chrome: preview-time rows are PLANNED, so no per-row + # FAILED Badge yet and no ✗ gutter. Pre-fix the per-row chrome + # stayed frozen here even after Confirm. + assert frame.locator("text=FAILED").count() == 0 + # The delete_row line item renders with the kind gutter (``-``) + # at preview time, not the failure gutter (``✗``). + assert frame.locator("text=✗ row #9999").count() == 0 + + # Fire the Confirm button — re-issues ``modify_sales_order`` + # with ``preview=False``, the stub returns the partial-failure + # apply response, and the on_success chain seeds the failed- + # subentity state slots from ``$result.state.*``. + frame.locator("button").filter(has_text="Confirm").first.click() + + # Wait for the morph: the failed-action error text from the + # apply response surfaces inside the now-visible Alert. + frame.locator("text=404 Not Found").first.wait_for( + state="visible", timeout=30000 + ) + # Retry-coaching tail from ``_so_subentity_failed_summary`` + # (references the SO id via ``modify_sales_order(id=42, ...)``). + assert frame.locator("text=modify_sales_order(id=42").count() >= 1 + # Row-binding morph (#858 finding A): the failed row's gutter + # glyph flipped to ``✗`` and the per-row FAILED Badge appeared. + # Pre-fix these stayed at the preview-time PLANNED chrome. + frame.locator("text=✗ row #9999").first.wait_for(state="visible", timeout=10000) + assert frame.locator("text=FAILED").count() >= 1 + + def test_so_failed_delete_applied_renders_error_text(self, render_scenario): + """#858 finding B — A failed top-level ``delete`` action renders + the FAILED chrome AND surfaces the :attr:`ActionResult.error` + text via the state-driven header-op Alert. + + Pre-fix the FAILED chrome rendered without the error message + — delete actions have no field changes (so + :func:`_render_failed_changes_block` was empty) and ``delete`` is + filtered out of :func:`_so_subentity_failed_summary` (so that + Alert was empty too). Operator saw "FAILED" with no way to tell + why. + + Pins end-to-end browser-render correctness for the new + header-op Alert. + """ + frame = render_scenario("so_delete_failed_applied") + # FAILED chrome (the state-driven header Badge). + assert frame.locator("text=FAILED").count() >= 1 + # Sales Order Failed title suffix (driven by ``applied_title_suffix`` + # on the standalone-applied failure path). + assert frame.locator("text=Sales Order Failed").count() >= 1 + # The new state-driven header-op Alert surfaces the + # ActionResult.error text verbatim (the load-bearing fix for + # finding B). + assert frame.locator("text=Sales order operation failed").count() >= 1 + assert frame.locator("text=Failed to delete the sales order").count() >= 1 + # And the actual error string from the response. + assert ( + frame.locator("text=404 Not Found: sales order 42 does not exist").count() + >= 1 + ) + + def test_so_failed_delete_morphs_preview_to_applied(self, render_scenario): + """#858 finding B — Click-through morph: the failed-delete error + text surfaces only after Confirm fires, driven by the on_success + ``SetState`` chain seeding ``applied_header_failed_summary`` from + ``$result.state.applied_header_failed_summary``. + + Mistyped slot names in either side of the chain would leave the + Alert empty (the preview seed) even after the apply lands. This + test catches the slot-name typo that the standalone-applied test + couldn't (the standalone path seeds the slot directly in + Python). + """ + frame = render_scenario("so_delete_failed_preview") + # Pre-state: Confirm visible, no error text yet (header-op + # Alert is gated on the ``applied_header_failed_count > 0`` + # condition; preview seeds it to 0). + assert frame.locator("button").filter(has_text="Confirm").first.is_visible() + assert ( + frame.locator("text=404 Not Found: sales order 42 does not exist").count() + == 0 + ) + assert frame.locator("text=Sales order operation failed").count() == 0 + + # Fire Confirm — stub returns the failed-delete apply response, + # on_success chain seeds ``applied_header_failed_*`` from + # ``$result.state.*``. + frame.locator("button").filter(has_text="Confirm Delete").first.click() + + # Wait for the morph: the failed-delete error text appears + # inside the now-visible header-op Alert. + frame.locator( + "text=404 Not Found: sales order 42 does not exist" + ).first.wait_for(state="visible", timeout=30000) + assert frame.locator("text=Sales order operation failed").count() >= 1 + + def test_so_modify_fail_fast_not_run_morphs_preview_to_applied( + self, render_scenario + ): + """#858 finding B (NOT-RUN morph) — Click-through: a 4-row plan + that fails on row 2 must morph to a card showing all 4 rows + (APPLIED + FAILED + NOT RUN + NOT RUN) instead of silently + dropping rows 3-4. + + Pre-fix: ``execute_plan`` fail-fast truncated ``response.actions`` + at row 2 (the failure boundary). The morph overwrote each + section's ``so_
_rows`` slot with the apply response's + shorter list — rows 3-4 silently disappeared from the Line items + section. Operator saw "1 succeeded, 1 failed" with no indication + that 2 more changes were never attempted. + + Post-fix: ``_modify_sales_order_impl`` stashes the unattempted + plan tail under ``response.extras["not_run_actions"]``; + :func:`build_so_modify_ui` merges it back into the per-section + row bucketing. The morph contract is preserved through the same + ``$result.state.so_
_rows`` on_success chain — the + merged actions list seeds those slots build-time + apply-time + identically. + """ + frame = render_scenario("so_modify_fail_fast_not_run_preview") + # Pre-state: preview shows all 4 rows as PLANNED (no Badges). + # The morph hasn't fired yet, so no NOT RUN / APPLIED / FAILED + # chrome on the per-row Badges. + assert frame.locator("button").filter(has_text="Confirm").first.is_visible() + assert frame.locator("text=NOT RUN").count() == 0 + assert frame.locator("text=APPLIED").count() == 0 + assert frame.locator("text=FAILED").count() == 0 + + # Fire Confirm — stub returns the fail-fast apply response with + # 2 executed + 2 NOT-RUN extras. The on_success chain seeds the + # ``so_rows_rows`` slot from ``$result.state.so_rows_rows`` so + # the merged 4-row list re-paints the Line items section. + frame.locator("button").filter(has_text="Confirm").first.click() + + # Wait for the morph: the NOT-RUN rows must materialize. + frame.locator("text=NOT RUN").first.wait_for(state="visible", timeout=30000) + # All four row states are visible on the morphed card — + # APPLIED + FAILED + NOT RUN x2. Pre-fix the NOT RUN entries + # were silently dropped. + assert frame.locator("text=APPLIED").count() >= 1 + assert frame.locator("text=FAILED").count() >= 1 + assert frame.locator("text=NOT RUN").count() >= 2 + + def test_so_correct_fail_fast_not_run_applied_renders(self, render_scenario): + """#858 Copilot follow-up (comment 3312071378) — ``build_so_modify_ui`` + also handles ``correct_sales_order``; its failure path must also + populate ``extras["not_run_actions"]`` so the morph picks up the + unattempted restore / recreate / close phases instead of silently + dropping them. + + Same wire shape as the ``modify_sales_order`` fail-fast scenario + above — proves the row merge in :func:`_so_actions_with_not_run_tail` + is tool-agnostic (it keys on ``response.extras``, not ``confirm_tool``). + Standalone-applied test, not click-through: the impl-side fix is + verified in :file:`test_corrections.py`; this proves the JS-side + render reads the extras correctly for the correction tool too. + """ + frame = render_scenario("so_correct_fail_fast_not_run_applied") + # Pre-fix: failure response would have shown only the executed + # prefix (1 APPLIED + 1 FAILED row); the NOT-RUN tail was hidden. + # Post-fix: all 4 rows visible on the applied card. + assert frame.locator("text=APPLIED").count() >= 1 + assert frame.locator("text=FAILED").count() >= 1 + assert frame.locator("text=NOT RUN").count() >= 2 + + def test_so_correct_fail_fast_header_skipped_applied_surfaces_alert( + self, render_scenario + ): + """#858 round-8 — A fail-fast ``correct_sales_order`` whose + close-phase ``update_header`` lands in the NOT-RUN tail must + surface that skipped step via the ``applied_header_skipped_*`` + Alert. Pre-round-8 the skipped header step had no rendering + surface: sub-entity row lists only bucket sub-entity ops + (rows / addresses / fulfillments / shipping fees), and the + header field map filters NOT-RUN out per round 7. The result + was a failed correction where the operator could see "edit + row failed" but could NOT see that the SO close step never + ran — the SO was left in PENDING status with no indication. + + Post-round-8 the state-driven Alert surfaces the count + the + user-facing summary ("Step skipped: modify the sales order + header (NOT RUN — earlier phase failed before this step ran)."). + """ + frame = render_scenario("so_correct_fail_fast_header_skipped_applied") + # Alert title (gated by ``applied_header_skipped_count > 0``). + assert ( + frame.locator("text=1 header step(s) skipped").count() >= 1 + or frame.locator("text=header step(s) skipped").count() >= 1 + ) + # User-facing verb derived from ``_SO_HEADER_OP_VERBS["update_header"]`` + # — the operator sees the *operation* they care about + # ("modify the sales order header"), not the wire name. + assert ( + frame.locator("text=Step skipped: modify the sales order header").count() + >= 1 + ) + # Causal phrase makes it obvious why the step was skipped + # (otherwise the operator might think the step ran and the + # status was already correct). + assert frame.locator("text=earlier phase failed").count() >= 1 + + def test_so_modify_header_failed_with_changes_morphs_preview_to_applied( + self, render_scenario + ): + """#858 finding C — Click-through: a failed ``update_header`` + action WITH field changes must surface the state-driven + ``applied_header_failed_*`` Alert on the morph, NOT stay frozen + on the preview-time "no failures" state of the build-time + :func:`_render_failed_changes_block`. + + Pre-fix: the build-time block read off the preview ``changes`` + map (every action's ``succeeded=None``), painted "no failures" + into the view tree, and stayed there on the morph. The + ``applied_header_failed_*`` Alert deliberately excluded ops + WITH changes to avoid double-render, leaving the failure + completely invisible on the morphed iframe. + + Post-fix: :func:`_so_header_op_failure_alert_text` is the single + source of truth for header-op failures (no-change ops AND + update_header WITH changes). The build-time block was removed + from the SO entity view so the error surfaces exactly once, + from state, on BOTH the standalone-applied path and the + preview→Confirm morph path. + """ + frame = render_scenario("so_modify_header_failed_with_changes_preview") + # Pre-state: Confirm visible, no error text yet (the state + # Alert is gated on ``applied_header_failed_count > 0`` — the + # preview seeds it to 0). + assert frame.locator("button").filter(has_text="Confirm").first.is_visible() + assert frame.locator("text=422 Unprocessable").count() == 0 + assert frame.locator("text=Sales order operation failed").count() == 0 + + # Fire Confirm — stub returns the failed-update-header apply + # response. The on_success chain copies + # ``$result.state.applied_header_failed_summary`` into the + # preview iframe's matching slot. + frame.locator("button").filter(has_text="Confirm").first.click() + + # Wait for the morph: the 422 error text appears inside the + # now-visible state Alert. + frame.locator("text=422 Unprocessable").first.wait_for( + state="visible", timeout=30000 + ) + # The dedicated header-op Alert title is visible. + assert frame.locator("text=Sales order operation failed").count() >= 1 + # And the user-facing verb derived from the operation + # (``_SO_HEADER_OP_VERBS["update_header"]``). + assert ( + frame.locator("text=Failed to modify the sales order header").count() >= 1 + ) + 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 743ca74ff..baaeacdb6 100644 --- a/katana_mcp_server/tests/test_prefab_ui.py +++ b/katana_mcp_server/tests/test_prefab_ui.py @@ -33,6 +33,7 @@ build_receipt_ui, build_search_results_ui, build_so_create_ui, + build_so_modify_ui, build_variant_details_ui, build_verification_ui, status_badge_variant, @@ -2319,6 +2320,140 @@ def test_unknown_prior_flag_propagates(self): ) assert idx["status"].unknown_prior is True + def test_include_operations_filters_to_header_actions(self): + """``include_operations`` filters the flatten to header-level + actions so sub-entity field changes whose names overlap header + names (``status`` on fulfillments vs SO header status, + ``tracking_number`` on fulfillments, row ``quantity`` / + shipping-fee ``description`` / ``amount``) don't pollute the + header-field map driving :func:`_render_so_header_scalar_diffs` + and the header-level :func:`_render_failed_changes_block`. + + Caught by Copilot review on #858 (PR feat/723-so-modify-card).""" + from katana_mcp.tools.prefab_ui import _index_changes_by_field + + actions = [ + # Header action — should pass through. + { + "operation": "update_header", + "succeeded": True, + "changes": [ + {"field": "status", "old": "NOT_SHIPPED", "new": "PACKED"}, + {"field": "additional_info", "old": "Net-30", "new": "Net-45"}, + ], + }, + # Sub-entity action with overlapping field name — + # ``status`` on fulfillment, NOT the header status. Must + # NOT overwrite the header's status diff. + { + "operation": "update_fulfillment", + "target_id": 555, + "succeeded": True, + "changes": [ + {"field": "status", "old": "PACKED", "new": "DELIVERED"}, + {"field": "tracking_number", "old": None, "new": "1Z999"}, + ], + }, + # Sub-entity action with non-overlapping name (row + # quantity). Also must not appear in the header map. + { + "operation": "update_row", + "target_id": 999, + "succeeded": False, + "error": "422 Unprocessable: quantity > stock", + "changes": [ + {"field": "quantity", "old": 5, "new": 100}, + ], + }, + ] + idx = _index_changes_by_field( + actions, include_operations=frozenset({"update_header", "delete"}) + ) + # Header fields present. + assert "additional_info" in idx + # Status came from the header action, NOT the fulfillment. + assert idx["status"].before == "NOT_SHIPPED" + assert idx["status"].after == "PACKED" + # Sub-entity fields filtered out — quantity / tracking_number + # only appear on row / fulfillment actions. + assert "quantity" not in idx + assert "tracking_number" not in idx + + def test_include_operations_none_preserves_all_actions(self): + """``include_operations=None`` (the default) flattens every + action — preserves the pre-filter behavior PO modify + tests + rely on. Pins the back-compat contract.""" + from katana_mcp.tools.prefab_ui import _index_changes_by_field + + actions = [ + { + "operation": "update_header", + "succeeded": True, + "changes": [{"field": "status", "old": "OPEN", "new": "CLOSED"}], + }, + { + "operation": "update_row", + "succeeded": True, + "changes": [{"field": "quantity", "old": 5, "new": 10}], + }, + ] + idx = _index_changes_by_field(actions) + assert set(idx.keys()) == {"status", "quantity"} + + def test_not_run_synthesized_action_does_not_overwrite_executed_diff(self): + """A synthesized NOT-RUN ``update_header`` (the unattempted + close-phase tail of a failed ``correct_sales_order``) must NOT + overwrite an earlier EXECUTED ``update_header`` diff via + last-write-wins on the field map. NOT RUN actions are plan + placeholders; their "diff" is what *would have* run, not what + ran. Without this filter the rendered header would show the + skipped step's planned change as an applied scalar diff and + suppress the NOT RUN indication entirely. + + Caught by Copilot review on #858 (PR feat/723-so-modify-card). + Repro: a correction sequence with an executed open-phase + ``update_header`` (succeeded=True, status: NOT_SHIPPED → + DELIVERED) followed by a synthesized close-phase + ``update_header`` (succeeded=None, status_label="NOT RUN", + status: DELIVERED → NOT_SHIPPED). Without the filter, + last-write-wins lets the NOT RUN diff masquerade as applied.""" + from katana_mcp.tools.prefab_ui import _index_changes_by_field + + actions = [ + # EXECUTED open-phase header update — real diff. + { + "operation": "update_header", + "target_id": 7777, + "succeeded": True, + "error": None, + "changes": [ + {"field": "status", "old": "NOT_SHIPPED", "new": "DELIVERED"}, + ], + }, + # SKIPPED close-phase header update — synthesized NOT-RUN + # placeholder from a failed correction tail. Its "diff" is + # the plan's intended close-phase revert; it never ran. + { + "operation": "update_header", + "target_id": 7777, + "succeeded": None, + "error": None, + "changes": [ + {"field": "status", "old": "DELIVERED", "new": "NOT_SHIPPED"}, + ], + "status_label": "NOT RUN", + }, + ] + idx = _index_changes_by_field( + actions, include_operations=frozenset({"update_header", "delete"}) + ) + # The EXECUTED diff wins, not the synthesized NOT-RUN tail. + assert idx["status"].before == "NOT_SHIPPED" + assert idx["status"].after == "DELIVERED" + assert idx["status"].failed is False + # The executed action succeeded — no error propagates. + assert idx["status"].error is None + class TestBuildPOModifyUI: """``build_po_modify_ui`` handles preview/applied/partial-failure for @@ -2919,6 +3054,1459 @@ def test_state_seeds_result_on_applied_path(self): assert result["entity_id"] == 9001 +class TestBuildSOModifyUI: + """``build_so_modify_ui`` handles preview/applied/partial-failure for + every SO write tool (``modify_sales_order``, ``delete_sales_order``, + ``correct_sales_order``). Mirrors ``TestBuildPOModifyUI``'s coverage + shape — title verb derivation, diff-decoration, layout-stability, + failure-state chrome, applied-state morph seeding. + + SO is more complex than PO because of the parallel-outcome + sub-entities (rows, addresses, fulfillments, shipping fees). The + sub-entity tests pin each section's rendering contract. + """ + + # Wire-shape prior_state matching ``SalesOrder.to_dict()`` — wire + # keys ``order_no`` / ``additional_info`` (NOT ``order_number`` / + # ``notes`` from the response shape). Catches shape-mismatch bugs + # at the ``_normalize_so_prior_state`` boundary. + _SO_PRIOR: ClassVar[dict[str, Any]] = { + "id": 42, + "order_no": "SO-2026-001", + "customer_id": 1501, + "customer_name": "Sarah Johnson", + "location_id": 1, + "status": "NOT_SHIPPED", + "currency": "USD", + "total": 1250.0, + "additional_info": "Customer requested expedited delivery", + "delivery_date": "2026-05-08T14:30:00Z", + } + + @classmethod + def _preview( + cls, + actions: list[dict[str, Any]] | None = None, + **overrides: Any, + ) -> dict[str, Any]: + return { + "entity_type": "sales_order", + "entity_id": cls._SO_PRIOR["id"], + "is_preview": True, + "actions": actions or [], + "prior_state": dict(cls._SO_PRIOR), + "warnings": [], + "next_actions": [], + "message": "Preview", + "katana_url": "https://factory.katanamrp.com/salesorder/42", + **overrides, + } + + @classmethod + def _applied( + cls, + actions: list[dict[str, Any]] | None = None, + **overrides: Any, + ) -> dict[str, Any]: + return cls._preview(actions, is_preview=False, **overrides) + + def test_smoke_preview_header_only_change(self): + """Single status-update preview renders the title verb, diff line, + and the cancel-phrase that ``_build_cancel_action`` interpolates.""" + actions = [ + { + "operation": "update_header", + "target_id": 42, + "succeeded": None, + "changes": [ + {"field": "status", "old": "NOT_SHIPPED", "new": "PACKED"}, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + _assert_valid_prefab(app) + rendered = str(app.to_json()) + assert "Modify Sales Order" in rendered + assert "NOT_SHIPPED" in rendered and "PACKED" in rendered + assert "→" in rendered + + def test_customer_change_renders_composite_diff(self): + """A customer change surfaces ``Customer: () → `` + — the composite name+ID rendering keeps the diff readable. Same + contract as PO's supplier-change rendering.""" + actions = [ + { + "operation": "update_header", + "target_id": 42, + "succeeded": None, + "changes": [ + {"field": "customer_id", "old": 1501, "new": 1502}, + { + "field": "customer_name", + "old": "Sarah Johnson", + "new": "Mike Smith", + }, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Customer:" in rendered + assert "Sarah Johnson" in rendered + assert "Mike Smith" in rendered + assert "→" in rendered + # Regression-guard: the bug shape (old name on both sides) must + # not appear. + assert "Sarah Johnson (1501) → Sarah Johnson" not in rendered + + def test_delete_card_uses_delete_verb_and_confirm_label(self): + """Delete cards use "Confirm Delete" and the "Delete Sales Order" + title; the cancel phrase reads naturally as a noun.""" + app = build_so_modify_ui( + self._preview([{"operation": "delete", "succeeded": None, "changes": []}]), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + rendered = str(app.to_json()) + assert "Delete Sales Order" in rendered + assert "Confirm Delete" in rendered + + def test_cancel_messages_use_natural_noun_phrases_per_verb(self): + """``_build_cancel_action`` interpolates noun phrases — modify/ + correct cards say "those sales order changes" / "those sales + order corrections"; delete cards say "that sales order deletion".""" + for tool, expected_phrase in [ + ("modify_sales_order", "those sales order changes"), + ("correct_sales_order", "those sales order corrections"), + ("delete_sales_order", "that sales order deletion"), + ]: + app = build_so_modify_ui( + self._preview( + [{"operation": "update_header", "succeeded": None, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool=tool, + ) + rendered = str(app.to_json()) + assert expected_phrase in rendered, ( + f"Cancel message for {tool} should interpolate " + f"{expected_phrase!r}; got rendered tree without it." + ) + + def test_failed_action_surfaces_glyph_inline_and_error_in_alert(self): + """Hybrid status approach — per-field ✗ glyph in the gutter + + consolidated error message in the bottom Alert. Layout-stability + rule from PO #722: failed apply doesn't reflow diff lines.""" + actions = [ + { + "operation": "update_header", + "target_id": 42, + "succeeded": False, + "error": "422 Unprocessable: invalid customer transition", + "changes": [ + { + "field": "status", + "old": "NOT_SHIPPED", + "new": "PACKED", + }, + ], + } + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "✗" in rendered + assert "422 Unprocessable" in rendered + + def test_applied_partial_failure_renders_overall_badge(self): + """A mixed applied outcome (1 success + 1 fail) renders the + ``PARTIAL FAILURE`` state badge with the destructive variant. + Same contract as PO modify.""" + actions = [ + { + "operation": "update_header", + "succeeded": True, + "changes": [ + {"field": "status", "old": "NOT_SHIPPED", "new": "PACKED"}, + ], + }, + { + "operation": "add_shipping_fee", + "succeeded": False, + "error": "422: tax_rate_id required", + "target_id": None, + "changes": [ + { + "field": "description", + "new": "Express ground", + "is_added": True, + }, + {"field": "amount", "new": "12.99", "is_added": True}, + ], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "PARTIAL FAILURE" in rendered + + def test_applied_all_failed_uses_failed_state_label(self): + """Fully-failed apply — header state badge MUST read ``FAILED``, + not ``APPLIED``. ``applied_state_label`` overrides based on + ``_summarize_apply_outcome``.""" + actions = [ + { + "operation": "update_header", + "succeeded": False, + "error": "422: invalid status transition", + "changes": [{"field": "status", "old": "PACKED", "new": "DELIVERED"}], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "FAILED" in rendered + + def test_failed_apply_uses_destructive_badge_variant(self): + """The FAILED state badge MUST render with variant=destructive + (red), NOT default (green). Pinned via envelope walk because + rendered-text alone can't distinguish FAILED-in-red from + FAILED-in-green. Same contract as PO modify.""" + actions = [ + { + "operation": "update_header", + "succeeded": False, + "error": "422: invalid", + "changes": [{"field": "status", "old": "OPEN", "new": "PACKED"}], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + envelope = app.to_json() + found_destructive = False + + def walk(node: Any) -> None: + nonlocal found_destructive + if isinstance(node, dict): + if ( + node.get("type") == "Badge" + and node.get("label") == "FAILED" + and node.get("variant") == "destructive" + ): + found_destructive = True + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + assert found_destructive, ( + "FAILED Badge with variant=destructive not found — the state " + "badge variant must reflect the apply outcome, not the verb." + ) + + def test_failed_delete_overrides_title_and_verb_to_match_failure(self): + """A failed delete reads "Sales Order Failed" / verb="failed" — not + "Sales Order Deleted" / "deleted." which would contradict the + FAILED badge. + + Unlike the PO modify card (which passes a literal verb to the + footer at build time), the SO modify card passes + ``applied_verb="{{ applied_verb }}"`` as a mustache template so + the footer body morphs in lockstep with the state-driven outcome + Badge. The literal verb lives in ``state.applied_verb``. + Same morph contract as ``build_bom_modify_ui`` (#811).""" + actions = [ + { + "operation": "delete", + "succeeded": False, + "error": "404: not found", + "changes": [], + } + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + rendered = str(app.to_json()) + assert "Delete Sales Order Failed" in rendered + assert "FAILED" in rendered + assert "Delete Sales Order Deleted" not in rendered + # State-driven verb: failed apply seeds applied_verb="failed" + # so the morph reads "Sales Order Delete failed." after the + # in-place state seeding lands. + assert app.state is not None + assert app.state["applied_verb"] == "failed" + + def test_applied_state_renders_applied_terminology_not_created(self): + """Modify cards pass ``applied_title_suffix="Applied"`` so the + rendered applied state reads ``"Sales Order Applied"`` — not the + create-card default "Created".""" + app = build_so_modify_ui( + self._applied( + [{"operation": "update_header", "succeeded": True, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Modify Sales Order Applied" in rendered + assert "APPLIED" in rendered + assert "Modify Sales Order Created" not in rendered + + def test_delete_card_renders_deleted_terminology_not_applied(self): + """Delete cards override the applied copy to "Deleted" / "DELETED".""" + app = build_so_modify_ui( + self._applied([{"operation": "delete", "succeeded": True, "changes": []}]), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + rendered = str(app.to_json()) + assert "Delete Sales Order Deleted" in rendered + assert "DELETED" in rendered + + def test_state_seeds_result_on_applied_path(self): + """Standalone-applied path seeds ``state.result`` from the response + so the applied-state Buttons resolve their ``{{ result.X }}`` + templates without an apply round-trip.""" + app = build_so_modify_ui( + self._applied( + [{"operation": "update_header", "succeeded": True, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + assert app.state["applied"] is True + result = app.state.get("result") + assert isinstance(result, dict) + assert result["entity_id"] == 42 + + def test_state_seeds_morph_slots_for_applied_outcome(self): + """The Confirm button's on_success chain morphs the card via + ``SetState`` from ``$result.state.``. The build-time state + MUST seed each morph slot so the bindings have something to + resolve to before the apply lands.""" + app = build_so_modify_ui( + self._preview( + [{"operation": "update_header", "succeeded": None, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + # Preview path — slots seeded with safe defaults so the + # If/Elif/Else branches have something to bind to. + assert app.state["applied_outcome_label"] == "APPLIED" + assert app.state["applied_outcome_variant"] == "default" + assert app.state["applied_subentity_failed_count"] == 0 + assert app.state["applied_subentity_failed_summary"] == "" + assert app.state["applied_verb"] == "applied" + + def test_sub_entity_section_renders_row_adds_with_gutter(self): + """Add-row actions render in the Line items section with a ``+ `` + gutter and the variant identity inline. Mirrors BOM's adds + rendering convention.""" + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "changes": [ + {"field": "variant_id", "old": None, "new": 2101, "is_added": True}, + {"field": "quantity", "old": None, "new": 3.0, "is_added": True}, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + # Section header. + assert "Line items" in rendered + # Add gutter + variant identity. + assert "+ " in rendered + assert "variant 2101" in rendered + assert "qty 3" in rendered + + def test_sub_entity_section_groups_actions_by_kind(self): + """A mixed plan touching rows + addresses + shipping fees renders + three section headers in the entity-view body, each grouping its + own actions. Pins the operation→section bucketing contract.""" + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "changes": [ + {"field": "variant_id", "old": None, "new": 100, "is_added": True}, + {"field": "quantity", "old": None, "new": 1.0, "is_added": True}, + ], + }, + { + "operation": "delete_address", + "target_id": 9001, + "succeeded": None, + "changes": [], + }, + { + "operation": "add_shipping_fee", + "target_id": None, + "succeeded": None, + "changes": [ + { + "field": "description", + "new": "Express", + "is_added": True, + }, + {"field": "amount", "new": "12.99", "is_added": True}, + ], + }, + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Line items" in rendered + assert "Addresses" in rendered + assert "Shipping fees" in rendered + # Fulfillments has no actions — section should not render + # (otherwise the card would have a "Fulfillments:" header with + # no rows below it). + assert "Fulfillments:" not in rendered + + def test_applied_sub_entity_failure_seeds_morph_summary(self): + """Applied path with a failed sub-entity action seeds the + ``applied_subentity_failed_count`` / ``applied_subentity_failed_summary`` + state slots — the in-place morph after Confirm reads ``$result.state.*`` + to surface failures the build-time render couldn't predict. + + Pre-formatted ``Failed — #: `` lines, plus a + retry-coaching tail referencing the SO id so the operator knows + how to recover via ``modify_sales_order``.""" + actions = [ + { + "operation": "add_shipping_fee", + "target_id": None, + "succeeded": True, + "changes": [ + {"field": "description", "new": "Express", "is_added": True}, + {"field": "amount", "new": "12.99", "is_added": True}, + ], + }, + { + "operation": "delete_row", + "target_id": 9999, + "succeeded": False, + "error": "404 Not Found: row 9999", + "changes": [], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + assert app.state["applied_subentity_failed_count"] == 1 + summary = app.state["applied_subentity_failed_summary"] + assert "Failed — delete_row #9999" in summary + assert "404 Not Found" in summary + # Retry coaching references the SO id literally so the operator + # can copy/paste without bouncing through the response object. + assert "modify_sales_order(id=42" in summary + + def test_subentity_failure_retry_text_matches_confirm_tool(self): + """The retry-coaching tail in the sub-entity failure summary must + name the tool the operator actually invoked. ``correct_sales_order`` + partial failures must recommend re-issuing ``correct_sales_order``, + not ``modify_sales_order`` — corrections and modifies have different + audit-trail side effects, so misdirection here would silently + drop a customer's correction off the books (#858 review follow-up). + """ + actions = [ + { + "operation": "delete_row", + "target_id": 9999, + "succeeded": False, + "error": "404 Not Found: row 9999", + "changes": [], + }, + ] + + modify_app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert modify_app.state is not None + modify_summary = modify_app.state["applied_subentity_failed_summary"] + assert "modify_sales_order(id=42" in modify_summary + assert "correct_sales_order(id=42" not in modify_summary + + correct_app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="correct_sales_order", + ) + assert correct_app.state is not None + correct_summary = correct_app.state["applied_subentity_failed_summary"] + assert "correct_sales_order(id=42" in correct_summary + # Regression-guard: the bug shape — a correction partial failure + # MUST NOT direct the operator back through ``modify_sales_order``. + assert "modify_sales_order(id=42" not in correct_summary + + def test_failed_subentity_action_uses_x_gutter_and_status_pill(self): + """On the applied path, a failed sub-entity action row gets the + ``✗ `` gutter + a destructive FAILED Badge alongside the summary.""" + actions = [ + { + "operation": "delete_row", + "target_id": 9999, + "succeeded": False, + "error": "404 Not Found", + "status_label": "FAILED", + "changes": [], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + envelope = app.to_json() + rendered = str(envelope) + assert "✗ row #9999" in rendered + # FAILED Badge alongside the row. + found_failed = False + + def walk(node: Any) -> None: + nonlocal found_failed + if isinstance(node, dict): + if ( + node.get("type") == "Badge" + and node.get("label") == "FAILED" + and node.get("variant") == "destructive" + ): + found_failed = True + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + assert found_failed, ( + "Failed sub-entity action must render with a FAILED + destructive " + "Badge alongside its row text." + ) + + def test_address_update_collapses_to_bare_after_form(self): + """Address updates collapse to the bare ``field: new`` form (no + arrow) because Katana has no per-address GET endpoint — + ``_format_so_diff_pairs(address_style=True)`` deliberately skips + the ``is_unknown_prior`` arrow path so the summary line stays + compact (otherwise every field would read + ``(prior unknown) → new``, which is noise without signal). + + Pins the bare-after contract: the rendered output MUST surface + the new field values and MUST NOT contain ``(prior unknown)``. + A refactor that flips ``address_style=True`` to the arrow form + would regress this test. + """ + actions = [ + { + "operation": "update_address", + "target_id": 9001, + "succeeded": None, + "changes": [ + { + "field": "city", + "old": None, + "new": "Springfield", + "is_unknown_prior": True, + }, + { + "field": "zip", + "old": None, + "new": "12345", + "is_unknown_prior": True, + }, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Addresses" in rendered + # The update summary surfaces the supplied fields' new values + # in the bare ``field: new`` form. + assert "Springfield" in rendered + assert "12345" in rendered + # Pin the bare-after contract: the address-style renderer MUST + # NOT emit ``(prior unknown)`` (the arrow path is skipped for + # address updates, per ``_format_so_diff_pairs(address_style=True)``). + assert "(prior unknown)" not in rendered + # And no arrow glyph either — address rows are bare-after only. + assert "→" not in rendered + + def test_party_id_swap_without_name_falls_back_to_id_only(self): + """When customer_id changes without an accompanying customer_name + FieldChange (the common case — ``customer_name`` isn't a request + field), the after side renders as ``#``, NOT the old name. + Same contract as PO #755.""" + actions = [ + { + "operation": "update_header", + "succeeded": None, + "changes": [ + {"field": "customer_id", "old": 1501, "new": 1502}, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Sarah Johnson (1501)" in rendered + assert "→" in rendered + assert "#1502" in rendered + # Regression-guard: old name must not show on the after side. + assert "Sarah Johnson (1502)" not in rendered + + def test_unchanged_kind_customer_change_falls_through_to_link_render(self): + """A no-op customer_id patch (request set customer_id=1501 when it + was already 1501) emits a ``FieldChangeView(kind="unchanged")``. + The composite diff line MUST NOT render — fall through to the + normal ``_render_party_line`` Link form.""" + actions = [ + { + "operation": "update_header", + "succeeded": True, + "changes": [ + { + "field": "customer_id", + "old": 1501, + "new": 1501, + "is_unchanged": True, + }, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Sarah Johnson" in rendered + assert "Sarah Johnson (1501) → Sarah Johnson (1501)" not in rendered + + def test_diff_lines_use_2char_gutter_for_layout_stability(self): + """Every changed-field line starts with a 2-char gutter (``" "`` + when not failed, ``"✗ "`` when failed). Failed apply doesn't + reflow the field text position.""" + actions = [ + { + "operation": "update_header", + "succeeded": None, + "changes": [ + {"field": "status", "old": "NOT_SHIPPED", "new": "PACKED"}, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert " Status:" in rendered + + def test_warnings_block_warnings_gate_confirm(self): + """``BLOCK:``-prefixed warnings suppress the Confirm button via the + ``_render_warnings_block`` → ``block_warnings`` → footer gating + chain. Same contract as PO modify and every other card.""" + app = build_so_modify_ui( + self._preview( + [{"operation": "update_header", "succeeded": None, "changes": []}], + warnings=["BLOCK: cannot proceed — invalid request shape"], + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + # Block warning surfaces in the body. + assert "cannot proceed" in rendered + # Footer copy switches to the "cannot proceed" muted line. + assert "Cannot proceed" in rendered + + # -------------------------------------------------------------- + # #858 finding A — per-row chrome morphs from state, not from + # Python-painted build-time values. Pin the state-bound row shape + # so a refactor that drops the ``so_
_rows`` slots would + # be caught at unit-test tier (mirroring BOM's ``plan_rows`` + # contract test). + # -------------------------------------------------------------- + + def test_subentity_row_lists_seeded_per_section_on_preview(self): + """Preview-path build seeds one ``so_
_rows`` slot per + sub-entity section in :data:`_SO_SUBENTITY_GROUPS`. Each slot's + list mirrors the actions for that section, projected through + :func:`_build_so_subentity_row` (preview rows carry + ``status_label="PLANNED"`` + ``has_badge=False`` so the + card-level state Badge is the only planned-state signal). + + Pin contract: state slots exist on every preview render so the + on_success ``SetState`` chain reading ``$result.state.so_
_rows`` + always has a destination slot — empty list for sections the plan + doesn't touch (no None-guard needed in the on_success target). + """ + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "changes": [ + {"field": "variant_id", "old": None, "new": 100, "is_added": True}, + {"field": "quantity", "old": None, "new": 1.0, "is_added": True}, + ], + }, + { + "operation": "delete_address", + "target_id": 9001, + "succeeded": None, + "changes": [], + }, + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + # All four section slots exist (empty when no actions). + assert app.state["so_rows_rows"] == [ + { + "gutter_summary": "+ variant 100, qty 1.0", + "status_label": "PLANNED", + "has_badge": False, + "status_variant": "secondary", + } + ] + assert app.state["so_addresses_rows"] == [ + { + "gutter_summary": "- address #9001", + "status_label": "PLANNED", + "has_badge": False, + "status_variant": "secondary", + } + ] + assert app.state["so_fulfillments_rows"] == [] + assert app.state["so_shipping_fees_rows"] == [] + + def test_subentity_row_lists_apply_path_carries_per_action_outcome(self): + """Standalone-applied path seeds row dicts with the apply-time + ``status_label`` + ``status_variant`` so the result card + renders the right per-row chrome without bouncing through the + morph. Catches the symmetry the morph relies on — the apply + tool's envelope ``state.*`` is what ``$result.state.*`` reads, + so apply-time seeding MUST match the morph target's shape.""" + actions = [ + { + "operation": "add_shipping_fee", + "target_id": None, + "succeeded": True, + "status_label": "APPLIED", + "changes": [ + {"field": "description", "new": "Express", "is_added": True}, + {"field": "amount", "new": "12.99", "is_added": True}, + ], + }, + { + "operation": "delete_row", + "target_id": 9999, + "succeeded": False, + "error": "404 Not Found", + "status_label": "FAILED", + "changes": [], + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + # The shipping fee row carries the APPLIED chrome — has_badge=True, + # default variant for the green-success Badge. + shipping_rows = app.state["so_shipping_fees_rows"] + assert len(shipping_rows) == 1 + assert shipping_rows[0]["status_label"] == "APPLIED" + assert shipping_rows[0]["has_badge"] is True + assert shipping_rows[0]["status_variant"] == "default" + # The row delete carries the FAILED chrome — ✗ gutter + destructive + # Badge variant. + rows_rows = app.state["so_rows_rows"] + assert len(rows_rows) == 1 + assert rows_rows[0]["status_label"] == "FAILED" + assert rows_rows[0]["has_badge"] is True + assert rows_rows[0]["status_variant"] == "destructive" + assert rows_rows[0]["gutter_summary"].startswith("✗ ") + + def test_subentity_row_lists_merge_not_run_tail_from_extras(self): + """Apply path: ``response.extras["not_run_actions"]`` carries the + unattempted plan tail (synthesized by + :func:`_modify_sales_order_impl`). The renderer must merge those + into the per-section row bucketing so the morphed card shows + APPLIED + FAILED + NOT-RUN rows instead of silently HIDING the + leftover plan past the fail-fast boundary (#858 finding B). + + Without this merge, a 5-row plan that fails on row 2 morphs to + a card with only 2 rows — operator sees "1 succeeded, 1 failed" + and never realizes 3 more changes were never attempted. + """ + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": True, + "status_label": "APPLIED", + "changes": [ + {"field": "variant_id", "new": 100, "is_added": True}, + {"field": "quantity", "new": "1.0", "is_added": True}, + ], + }, + { + "operation": "add_row", + "target_id": None, + "succeeded": False, + "error": "422 invalid variant", + "status_label": "FAILED", + "changes": [ + {"field": "variant_id", "new": 999, "is_added": True}, + {"field": "quantity", "new": "2.0", "is_added": True}, + ], + }, + ] + not_run = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "error": None, + "status_label": "NOT RUN", + "changes": [ + {"field": "variant_id", "new": 101, "is_added": True}, + {"field": "quantity", "new": "3.0", "is_added": True}, + ], + }, + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "error": None, + "status_label": "NOT RUN", + "changes": [ + {"field": "variant_id", "new": 102, "is_added": True}, + {"field": "quantity", "new": "4.0", "is_added": True}, + ], + }, + ] + # Synthesize the apply-response shape the impl produces post-fix + # (executed actions + ``extras["not_run_actions"]``). + response = self._applied(actions) + response["extras"] = {"not_run_actions": not_run} + app = build_so_modify_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + rows_rows = app.state["so_rows_rows"] + # 2 executed + 2 NOT RUN = 4 rows visible on the morphed card. + assert len(rows_rows) == 4 + # Plan order preserved: APPLIED, FAILED, NOT RUN, NOT RUN. + assert [r["status_label"] for r in rows_rows] == [ + "APPLIED", + "FAILED", + "NOT RUN", + "NOT RUN", + ] + # NOT RUN rows render with the "secondary" Badge variant per + # :data:`_SO_SUB_STATUS_VARIANTS` (neutral chrome — neither + # success nor failure). + assert rows_rows[2]["status_variant"] == "secondary" + assert rows_rows[2]["has_badge"] is True + assert rows_rows[3]["status_variant"] == "secondary" + # And the per-row Badge dispatch inside the so_rows_rows ForEach + # must include a ``variant="secondary"`` branch — the dispatch is + # a parallel If/Elif/Else chain (destructive / secondary / + # default). Without the Elif, NOT RUN rows fall through to the + # ``default`` Else and render success-green instead of neutral. + # The card-level "PREVIEW" badge also uses ``secondary``, so we + # must inspect the ForEach body specifically, not the whole tree. + envelope = app.to_json() + per_row_foreach: dict[str, Any] | None = None + + def find_foreach(node: Any) -> None: + nonlocal per_row_foreach + if per_row_foreach is not None: + return + if isinstance(node, dict): + if node.get("type") == "ForEach" and node.get("key") == "so_rows_rows": + per_row_foreach = node + return + for v in node.values(): + find_foreach(v) + elif isinstance(node, list): + for v in node: + find_foreach(v) + + find_foreach(envelope) + assert per_row_foreach is not None, ( + "Expected so_rows_rows ForEach in the rendered envelope." + ) + found_secondary_branch = False + + def walk_for_secondary_branch(node: Any) -> None: + nonlocal found_secondary_branch + if isinstance(node, dict): + if node.get("type") == "Badge" and node.get("variant") == ("secondary"): + found_secondary_branch = True + for v in node.values(): + walk_for_secondary_branch(v) + elif isinstance(node, list): + for v in node: + walk_for_secondary_branch(v) + + walk_for_secondary_branch(per_row_foreach) + assert found_secondary_branch, ( + "Per-row Badge dispatch inside so_rows_rows ForEach must " + "include a Badge with variant='secondary' — without the " + "Elif branch between destructive and the default Else, " + "NOT RUN rows fall through to the green-success default " + "variant instead of rendering neutral." + ) + + def test_subentity_row_lists_ignore_not_run_on_preview_path(self): + """The NOT-RUN extras only attach to apply responses (the + ``not is_preview`` branch in :func:`_modify_sales_order_impl`). + On preview, every action is already PLANNED so a NOT-RUN merge + would be a duplicate. Guard against accidental leakage by + asserting preview ignores ``extras["not_run_actions"]`` even if + present (e.g. a mock test that doesn't separate paths).""" + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "changes": [ + {"field": "variant_id", "new": 100, "is_added": True}, + {"field": "quantity", "new": "1.0", "is_added": True}, + ], + } + ] + response = self._preview(actions) + response["extras"] = { + "not_run_actions": [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "status_label": "NOT RUN", + "changes": [], + } + ] + } + app = build_so_modify_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + # Preview path: only the one planned action, not the NOT-RUN + # entry from extras. + rows_rows = app.state["so_rows_rows"] + assert len(rows_rows) == 1 + assert rows_rows[0]["status_label"] == "PLANNED" + + def test_subentity_sections_render_state_bound_foreach(self): + """The sub-entity sections render row content via ``ForEach`` + keyed to ``state.so_
_rows`` — NOT via build-time Python + iteration. Pins the morph contract so a refactor that reverts + to static rendering would regress finding A. + + Walks the rendered envelope looking for a ``ForEach`` node keyed + to ``so_rows_rows`` (the Line items section's row slot). The + existence of this node is the load-bearing morph guarantee: + with ``ForEach`` reading state, the apply-time ``SetState`` of + ``$result.state.so_rows_rows`` swaps the row list and Prefab's + renderer re-paints each row's text + Badge from the new dicts. + """ + actions = [ + { + "operation": "add_row", + "target_id": None, + "succeeded": None, + "changes": [ + {"field": "variant_id", "old": None, "new": 100, "is_added": True}, + {"field": "quantity", "old": None, "new": 1.0, "is_added": True}, + ], + } + ] + app = build_so_modify_ui( + self._preview(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + envelope = app.to_json() + found_foreach_keys: set[str] = set() + + def walk(node: Any) -> None: + if isinstance(node, dict): + if node.get("type") == "ForEach": + key = node.get("key") + if isinstance(key, str): + found_foreach_keys.add(key) + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + # At minimum, the Line items section's ForEach must be present. + # (Other sections render their ForEach too, but only when their + # action list is non-empty — gated by the section header guard.) + assert "so_rows_rows" in found_foreach_keys, ( + "Sub-entity rows must render via ForEach(state.so_rows_rows) — " + "without this the per-row chrome can't morph after apply (#858 " + "finding A)." + ) + + def test_apply_action_morph_chain_writes_per_section_row_slots(self): + """The Confirm button's on_success ``SetState`` chain MUST write + ``$result.state.so_
_rows`` into the preview iframe's + matching state slot, for every section in + :data:`_SO_SUBENTITY_GROUPS`. A mistyped slot name in either + side of the chain would leave the preview rows frozen at their + ``PLANNED`` state even after the apply lands. + + Walks the rendered envelope for SetState nodes whose ``key`` + matches each section's row slot and confirms the matching + ``value`` template reads from ``$result.state.``. + """ + app = build_so_modify_ui( + self._preview( + [{"operation": "update_header", "succeeded": None, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + envelope = app.to_json() + morph_targets: dict[str, str] = {} + + def walk(node: Any) -> None: + if isinstance(node, dict): + # SetState actions serialize as {"action": "setState", + # "key": ..., "value": ...} (not as components with a + # "type" field — they're attached to button on_success + # / on_click slots, not rendered as elements). + if node.get("action") == "setState": + key = node.get("key") + value = node.get("value") + if isinstance(key, str) and isinstance(value, str): + morph_targets[key] = value + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + # Each section MUST have a SetState that copies $result.state. + # into the same-name preview slot. Mistyping the template (e.g. + # $result.so_rows_rows instead of $result.state.so_rows_rows) + # would leave the row slot frozen — caught here. + for section_key in ("rows", "addresses", "fulfillments", "shipping_fees"): + slot = f"so_{section_key}_rows" + assert slot in morph_targets, ( + f"on_success chain missing SetState for {slot} — the " + f"{section_key} section's rows would stay frozen at the " + f"preview-time PLANNED state after apply." + ) + assert morph_targets[slot] == ("{{ $result.state." + slot + " }}"), ( + f"SetState for {slot} reads from {morph_targets[slot]!r}; " + f"must read from '$result.state.{slot}' (NOT $result.) " + f"because $result resolves to the apply tool's PrefabApp " + f"envelope, not the raw ModificationResponse." + ) + + # -------------------------------------------------------------- + # #858 finding B — failed top-level delete must surface its error + # in a dedicated state-driven Alert. Pre-fix the FAILED chrome + # rendered without the error text from ActionResult.error because + # delete actions have no field changes (header-changes block + # rendered nothing) and they're filtered out of the sub-entity + # Alert (delete is a top-level op, not a sub-entity op). + # -------------------------------------------------------------- + + def test_failed_delete_seeds_header_failed_summary_with_error_text(self): + """A failed top-level ``delete`` action seeds the + ``applied_header_failed_count`` + ``applied_header_failed_summary`` + state slots so the dedicated header-op Alert can surface the + :attr:`ActionResult.error` text. Pre-fix this slot didn't exist + and the FAILED chrome rendered with no visible error message + (#858 finding B).""" + actions = [ + { + "operation": "delete", + "target_id": 42, + "succeeded": False, + "error": "404 Not Found: sales order 42 does not exist", + "changes": [], + } + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + assert app.state is not None + assert app.state["applied_header_failed_count"] == 1 + summary = app.state["applied_header_failed_summary"] + # User-facing verb (not the bare wire name "delete") + the + # ActionResult.error string verbatim. Pre-fix the error message + # never made it to the rendered card. + assert "Failed to delete the sales order" in summary + assert "404 Not Found: sales order 42 does not exist" in summary + + def test_successful_delete_omits_header_failed_alert(self): + """A successful delete leaves the header-failed slots at 0/empty + so the state-driven Alert stays hidden (the ``If(Rx(...) > 0)`` + gate keeps the card compact in the success case).""" + app = build_so_modify_ui( + self._applied([{"operation": "delete", "succeeded": True, "changes": []}]), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + assert app.state is not None + assert app.state["applied_header_failed_count"] == 0 + assert app.state["applied_header_failed_summary"] == "" + + def test_failed_header_update_with_changes_surfaces_on_state_alert(self): + """A failed ``update_header`` action with field changes must + increment the state-driven ``applied_header_failed_count`` so + the morph path (preview→Confirm) can surface the error. + + Pre-fix #858 finding C: the SO entity view called the build-time + :func:`_render_failed_changes_block` which read off the preview- + time ``changes`` map (every action's ``succeeded=None``). After + Confirm morphed ``state.applied=True``, that block stayed at + preview-time content — the ✗ gutter and the per-field error + Alert never appeared even though the apply had failed. + + Post-fix: :func:`_so_header_op_failure_alert_text` is the single + source of truth for header-op failures (no-change ops AND + update_header with field changes). The build-time block was + removed from the SO entity view so the error renders exactly + once, from state, on BOTH the standalone-applied path and the + preview→Confirm morph path. + """ + actions = [ + { + "operation": "update_header", + "target_id": 42, + "succeeded": False, + "error": "422 Unprocessable: invalid status transition", + "changes": [ + {"field": "status", "old": "PACKED", "new": "DELIVERED"}, + ], + } + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + # State-driven Alert now owns the failure; ``count == 1`` so the + # ``If(Rx("applied_header_failed_count") > 0)`` gate fires on the + # morphed iframe. + assert app.state["applied_header_failed_count"] == 1 + summary = app.state["applied_header_failed_summary"] + assert "Failed to modify the sales order header" in summary + assert "422 Unprocessable: invalid status transition" in summary + # The error must be reachable in the rendered card too — same + # text as the state summary, painted by the state-bound Alert + # (no duplicate build-time block). + rendered = str(app.to_json()) + assert "422 Unprocessable" in rendered + # No double-render in the *visible* view tree: pre-fix, the + # build-time ``_render_failed_changes_block`` painted an + # ``AlertDescription`` with the verbatim error text into the + # view tree, AND the state-driven Alert also picked it up, + # giving the operator two competing error blocks. Post-fix the + # build-time block is removed for SO — the only AlertDescription + # carrying the error text references it via mustache + # ``{{ applied_header_failed_summary }}``, not as a literal. + envelope = app.to_json() + view_str = str(envelope.get("view")) + assert "422 Unprocessable: invalid status transition" not in view_str, ( + "Build-time _render_failed_changes_block must not paint the " + "error literal into the view tree on SO — only the state-driven " + "Alert may surface it (via mustache binding)." + ) + + def test_apply_action_morph_chain_writes_header_failed_slots(self): + """The Confirm button's on_success chain MUST write + ``$result.state.applied_header_failed_count`` / + ``applied_header_failed_summary`` into the preview iframe's + matching slots so the header-op Alert can pop in after the + morph lands. Mistyped slot names would leave a failed delete + rendering FAILED chrome without the error text the operator + needs to diagnose the failure (#858 finding B).""" + app = build_so_modify_ui( + self._preview([{"operation": "delete", "succeeded": None, "changes": []}]), + confirm_request=_StubRequest(), + confirm_tool="delete_sales_order", + ) + envelope = app.to_json() + morph_targets: dict[str, str] = {} + + def walk(node: Any) -> None: + if isinstance(node, dict): + # SetState actions serialize as {"action": "setState", + # "key": ..., "value": ...} (not as components with a + # "type" field — they're attached to button on_success + # / on_click slots, not rendered as elements). + if node.get("action") == "setState": + key = node.get("key") + value = node.get("value") + if isinstance(key, str) and isinstance(value, str): + morph_targets[key] = value + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + for slot in ("applied_header_failed_count", "applied_header_failed_summary"): + assert slot in morph_targets, ( + f"on_success chain missing SetState for {slot} — a failed " + f"delete after Confirm would render with no error text " + f"(#858 finding B)." + ) + assert morph_targets[slot] == ("{{ $result.state." + slot + " }}") + + # -------------------------------------------------------------- + # #858 round-8 — Header NOT-RUN must have a rendering surface. + # _index_changes_by_field filters NOT-RUN out (round 7 fix) and + # _build_so_subentity_row_lists only buckets sub-entity ops, so a + # synthesized NOT-RUN ``update_header`` (e.g. the close-phase step + # of a failed correct_sales_order) had nowhere to surface. The + # state-driven Alert below covers the gap. + # -------------------------------------------------------------- + + def test_skipped_header_seeds_skipped_state_and_alert_text(self): + """A NOT-RUN ``update_header`` action (synthesized when a + fail-fast ``correct_sales_order`` skips its close-phase header + step) seeds ``applied_header_skipped_count`` / + ``applied_header_skipped_summary`` so the dedicated Alert can + surface "Step skipped: modify the sales order header" to the + operator. Pre-fix this slot didn't exist and the skipped close- + phase step had no rendering surface — sub-entity NOT-RUN rows + rendered but the header step was invisible (#858 round-8).""" + actions = [ + { + "operation": "update_row", + "target_id": 10, + "succeeded": False, + "error": "Katana refused the row edit", + "changes": [{"field": "variant_id", "old": 500, "new": 501}], + }, + { + "operation": "update_header", + "target_id": 42, + "succeeded": None, + "error": None, + "changes": [{"field": "status", "old": None, "new": "DELIVERED"}], + "status_label": "NOT RUN", + }, + ] + app = build_so_modify_ui( + self._applied(actions), + confirm_request=_StubRequest(), + confirm_tool="correct_sales_order", + ) + assert app.state is not None + assert app.state["applied_header_skipped_count"] == 1 + summary = app.state["applied_header_skipped_summary"] + # User-facing verb (not the bare wire name "update_header") + + # the "earlier phase failed" causal phrase so the operator can + # tell at a glance why the step didn't run. + assert "Step skipped: modify the sales order header" in summary + assert "NOT RUN" in summary + assert "earlier phase failed" in summary + + def test_no_skipped_header_omits_skipped_alert(self): + """When every header step ran (or there were none), the + skipped slots stay at 0/empty so the ``If(Rx(...) > 0)`` gate + keeps the Alert hidden — same compactness rule as the failed- + op Alert.""" + app = build_so_modify_ui( + self._applied( + [{"operation": "update_header", "succeeded": True, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + assert app.state is not None + assert app.state["applied_header_skipped_count"] == 0 + assert app.state["applied_header_skipped_summary"] == "" + + def test_apply_action_morph_chain_writes_header_skipped_slots(self): + """The Confirm button's on_success chain MUST also propagate the + ``applied_header_skipped_*`` slots from the apply tool's + ``$result.state.*`` envelope into the preview iframe so the + Alert can pop in on the morph. Mistyped slot names would leave + a skipped close-phase step silently invisible after Confirm + (#858 round-8).""" + app = build_so_modify_ui( + self._preview( + [{"operation": "update_header", "succeeded": None, "changes": []}] + ), + confirm_request=_StubRequest(), + confirm_tool="correct_sales_order", + ) + envelope = app.to_json() + morph_targets: dict[str, str] = {} + + def walk(node: Any) -> None: + if isinstance(node, dict): + if node.get("action") == "setState": + key = node.get("key") + value = node.get("value") + if isinstance(key, str) and isinstance(value, str): + morph_targets[key] = value + for v in node.values(): + walk(v) + elif isinstance(node, list): + for v in node: + walk(v) + + walk(envelope) + for slot in ( + "applied_header_skipped_count", + "applied_header_skipped_summary", + ): + assert slot in morph_targets, ( + f"on_success chain missing SetState for {slot} — a " + f"skipped header step after Confirm would have no " + f"rendering surface (#858 round-8)." + ) + assert morph_targets[slot] == ("{{ $result.state." + slot + " }}") + + # -------------------------------------------------------------- + # #858 Copilot 3313163122 — Line Items metric must render on a + # real ModificationResponse (which has no item_count field) by + # deriving from prior_state.sales_order_rows. + # -------------------------------------------------------------- + + def test_item_count_derived_from_prior_state_rows_when_absent(self): + """``ModificationResponse`` doesn't carry ``item_count`` (only + ``SalesOrderResponse`` does), so the modify card's Tier-2 + "Line Items" Metric would render blank on a real apply response. + The build-side derivation in :func:`_normalize_so_prior_state` + falls back to ``len(prior_state["sales_order_rows"])`` so the + Metric renders consistently across create and modify cards + (#858 Copilot 3313163122).""" + prior_with_rows = dict(self._SO_PRIOR) + prior_with_rows["sales_order_rows"] = [ + {"id": 10, "variant_id": 500, "quantity": 1}, + {"id": 11, "variant_id": 501, "quantity": 2}, + {"id": 12, "variant_id": 502, "quantity": 3}, + ] + # Build a response that mimics the real shape: no top-level + # item_count, prior_state carries sales_order_rows. + response = self._applied( + [{"operation": "update_header", "succeeded": True, "changes": []}], + prior_state=prior_with_rows, + ) + assert "item_count" not in response # guardrail + app = build_so_modify_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + # Metric renders "Line Items" label + the derived count value. + # Both must appear in the view tree; pre-fix the value side + # rendered blank because entity.get("item_count") returned None. + assert "Line Items" in rendered + assert "3" in rendered + + def test_item_count_explicit_overrides_derived(self): + """If the response carries an explicit ``item_count`` (rare + but possible for non-MorphedResponse paths), the explicit + value wins — the derivation is a fallback, not an override. + Otherwise a stale prior_state row list could shadow the + canonical response count.""" + prior_with_rows = dict(self._SO_PRIOR) + prior_with_rows["sales_order_rows"] = [ + {"id": 10, "variant_id": 500, "quantity": 1}, + ] + # The explicit item_count (7) must win over the derived count + # (1 row in prior_state) — entity overlays response on top of + # normalized prior_state. + response = self._applied( + [{"operation": "update_header", "succeeded": True, "changes": []}], + prior_state=prior_with_rows, + item_count=7, + ) + app = build_so_modify_ui( + response, + confirm_request=_StubRequest(), + confirm_tool="modify_sales_order", + ) + rendered = str(app.to_json()) + assert "Line Items" in rendered + assert "7" in rendered + + class TestMergeBomRowsForModifyCard: """``_merge_bom_rows_for_modify_card`` projects ``prior_state.rows`` + plan actions into the row-shape the BOM modify DataTable consumes. diff --git a/katana_mcp_server/tests/tools/test_corrections.py b/katana_mcp_server/tests/tools/test_corrections.py index 4f9763e81..d2bf24ae6 100644 --- a/katana_mcp_server/tests/tools/test_corrections.py +++ b/katana_mcp_server/tests/tools/test_corrections.py @@ -779,6 +779,191 @@ async def fake_create_ful(*, client, body): assert response.prior_state is not None +@pytest.mark.asyncio +async def test_correct_so_fail_fast_synthesizes_not_run_tail_for_morph(): + """Fail-fast mid-correction must surface the unattempted phases as + NOT-RUN extras (#858 finding B — Copilot comment 3312071378). + + ``build_so_modify_ui`` handles ``correct_sales_order`` alongside + ``modify_sales_order``; both rely on ``response.extras[\"not_run_actions\"]`` + so the per-section row morph can render skipped restore / recreate / + close phases instead of silently overwriting the preview's full + sub-entity rows with only the executed prefix. + + Plan: delete fulfillment (phase 1, succeeds) → revert SO (phase 2, + succeeds) → edit row (phase 3, FAILS). Phases 4 (recreate fulfillment) + + 5 (close SO) must surface as NOT-RUN entries. + """ + context, _ = create_mock_context() + picked = datetime(2026, 4, 15, 21, 18, 0, tzinfo=UTC) + so = _make_so(status="DELIVERED", picked_date=picked) + so_row = _make_so_row(row_id=10, variant_id=500) + so.sales_order_rows = [so_row] + fulfillments = [ + _make_fulfillment(ful_id=77, so_id=99, row_id=10, picked_date=picked) + ] + + async def fake_delete_ful(*, id, client): + resp = MagicMock() + resp.status_code = 204 + return resp + + async def fake_update_so(*, id, client, body): + resp = MagicMock() + resp.parsed = so + return resp + + async def boom_update_row(*, id, client, body): + raise RuntimeError("Katana refused the row edit") + + with ( + patch( + "katana_mcp.tools.foundation.corrections._fetch_sales_order_attrs", + new_callable=AsyncMock, + return_value=so, + ), + patch( + "katana_mcp.tools.foundation.corrections._fetch_so_fulfillments", + new_callable=AsyncMock, + return_value=fulfillments, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_delete_so_fulfillment.asyncio_detailed", + side_effect=fake_delete_ful, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_update_sales_order.asyncio_detailed", + side_effect=fake_update_so, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_update_so_row.asyncio_detailed", + side_effect=boom_update_row, + ), + patch( + "katana_mcp.tools.foundation.corrections.is_success", + return_value=True, + ), + ): + response = await _correct_sales_order_impl( + CorrectSalesOrderRequest( + id=99, + line_changes=[SOLineCorrection(old_variant_id=500, new_variant_id=501)], + preview=False, + ), + context, + ) + + # Phases 1+2 succeeded, phase 3 (edit) failed → 3 executed actions. + assert response.is_preview is False + assert len(response.actions) == 3 + assert response.actions[0].succeeded is True # delete_fulfillment + assert response.actions[1].succeeded is True # update_header (revert) + assert response.actions[2].succeeded is False # update_row (boom) + + # The two unattempted phases must surface as NOT-RUN extras so the + # SO modify-card morph picks them up via ``_so_actions_with_not_run_tail``. + not_run = response.extras.get("not_run_actions") or [] + assert len(not_run) == 2, ( + f"Expected 2 NOT-RUN entries (recreate + close); got {len(not_run)}: " + f"{[a.get('operation') for a in not_run]}" + ) + assert [a["operation"] for a in not_run] == ["add_fulfillment", "update_header"] + assert all(a["succeeded"] is None for a in not_run) + assert all(a["status_label"] == "NOT RUN" for a in not_run) + + +@pytest.mark.asyncio +async def test_correct_so_fail_fast_morph_renders_not_run_rows(): + """End-to-end check: feed the failed-correction response into + :func:`build_so_modify_ui` and confirm the NOT-RUN tail makes it into + the action list the morph paints from (#858 finding B). + + This is the consumer-side proof — the impl-side test above proves + extras are populated; this one proves the renderer actually reads them. + """ + from katana_mcp.tools.prefab_ui import _so_actions_with_not_run_tail + + context, _ = create_mock_context() + picked = datetime(2026, 4, 15, 21, 18, 0, tzinfo=UTC) + so = _make_so(status="DELIVERED", picked_date=picked) + so_row = _make_so_row(row_id=10, variant_id=500) + so.sales_order_rows = [so_row] + fulfillments = [ + _make_fulfillment(ful_id=77, so_id=99, row_id=10, picked_date=picked) + ] + + async def fake_delete_ful(*, id, client): + resp = MagicMock() + resp.status_code = 204 + return resp + + async def fake_update_so(*, id, client, body): + resp = MagicMock() + resp.parsed = so + return resp + + async def boom_update_row(*, id, client, body): + raise RuntimeError("Katana refused the row edit") + + with ( + patch( + "katana_mcp.tools.foundation.corrections._fetch_sales_order_attrs", + new_callable=AsyncMock, + return_value=so, + ), + patch( + "katana_mcp.tools.foundation.corrections._fetch_so_fulfillments", + new_callable=AsyncMock, + return_value=fulfillments, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_delete_so_fulfillment.asyncio_detailed", + side_effect=fake_delete_ful, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_update_sales_order.asyncio_detailed", + side_effect=fake_update_so, + ), + patch( + "katana_mcp.tools.foundation.corrections." + "api_update_so_row.asyncio_detailed", + side_effect=boom_update_row, + ), + patch( + "katana_mcp.tools.foundation.corrections.is_success", + return_value=True, + ), + ): + response = await _correct_sales_order_impl( + CorrectSalesOrderRequest( + id=99, + line_changes=[SOLineCorrection(old_variant_id=500, new_variant_id=501)], + preview=False, + ), + context, + ) + + # Hand the response to the merge helper exactly as ``build_so_modify_ui`` + # does. Result: 3 executed + 2 NOT-RUN = 5 rows visible on the morph. + response_dict = response.model_dump() + merged = _so_actions_with_not_run_tail(response_dict, is_preview=False) + assert len(merged) == 5 + + # Plan order preserved: APPLIED, APPLIED, FAILED, NOT RUN, NOT RUN. + status_labels = [a.get("status_label") for a in merged] + assert status_labels[-2:] == ["NOT RUN", "NOT RUN"] + # The trailing two are the recreate + close that never ran. + assert [a.get("operation") for a in merged[-2:]] == [ + "add_fulfillment", + "update_header", + ] + + # ============================================================================ # correct_purchase_order — fixtures # ============================================================================ diff --git a/katana_mcp_server/tests/tools/test_sales_orders.py b/katana_mcp_server/tests/tools/test_sales_orders.py index 36a67a7ca..db0adc3ba 100644 --- a/katana_mcp_server/tests/tools/test_sales_orders.py +++ b/katana_mcp_server/tests/tools/test_sales_orders.py @@ -2487,6 +2487,66 @@ async def test_modify_so_fail_fast_halts_on_first_error(): assert "boom" in (response.actions[1].error or "") +@pytest.mark.asyncio +async def test_modify_so_fail_fast_synthesizes_not_run_tail_for_morph(): + """Fail-fast apply truncates ``response.actions`` at the first failure + — but the modify card's per-section row morph needs the unattempted + plan tail so those rows render as NOT RUN instead of silently + disappearing from the post-apply card (#858 finding B). + + Mirrors :func:`_modify_product_bom_impl`'s NOT-RUN synthesis pattern + (#811): the impl pushes one NOT-RUN action dict per leftover + :class:`ActionSpec` into ``response.extras["not_run_actions"]`` so + :func:`build_so_modify_ui` can merge them into the per-section row + buckets. Without this, a 5-row plan that fails on row 2 would morph + to a card showing only rows 1 (APPLIED) + 2 (FAILED), with rows 3-5 + silently dropped from the Line items section. + """ + context, _ = create_mock_context() + existing = _mock_so(order_id=42, order_no="SO-1") + updated_so = _mock_so(order_id=42, order_no="SO-1") + + with ( + patch( + "katana_mcp.tools.foundation.sales_orders._fetch_sales_order_attrs", + new_callable=AsyncMock, + return_value=existing, + ), + patch(f"{_MODIFY_SO_UPDATE}.asyncio_detailed", new_callable=AsyncMock), + patch( + f"{_MODIFY_SO_ROW_CREATE}.asyncio_detailed", + new_callable=AsyncMock, + side_effect=RuntimeError("boom"), + ), + patch(_MODIFY_SO_UNWRAP_AS_LOCAL, return_value=updated_so), + ): + # Plan = 1 header update (succeeds) + 3 row adds (first fails, + # next 2 are NEVER attempted). + request = ModifySalesOrderRequest( + id=42, + update_header=SOHeaderPatch(status="PACKED"), + add_rows=[ + SORowAdd(variant_id=100, quantity=2), + SORowAdd(variant_id=101, quantity=3), + SORowAdd(variant_id=102, quantity=4), + ], + preview=False, + ) + response = await _modify_sales_order_impl(request, context) + + # Fail-fast: 2 actions executed (header succeeded, first row failed). + assert len(response.actions) == 2 + # The 2 leftover row adds must surface as NOT-RUN entries on extras + # so the morph picks them up. Each carries ``succeeded=None`` + + # ``status_label="NOT RUN"`` so the renderer's row-builder sets the + # "secondary" Badge variant. + not_run = response.extras.get("not_run_actions") or [] + assert len(not_run) == 2 + assert all(a["operation"] == "add_row" for a in not_run) + assert all(a["succeeded"] is None for a in not_run) + assert all(a["status_label"] == "NOT RUN" for a in not_run) + + # ============================================================================ # delete_sales_order — destructive sibling # ============================================================================ diff --git a/uv.lock b/uv.lock index f03e2f9f2..a9c831436 100644 --- a/uv.lock +++ b/uv.lock @@ -1329,7 +1329,7 @@ wheels = [ [[package]] name = "katana-mcp-server" -version = "0.97.0" +version = "0.98.0" source = { editable = "katana_mcp_server" } dependencies = [ { name = "aiosqlite" }, @@ -1360,7 +1360,7 @@ requires-dist = [ [[package]] name = "katana-openapi-client" -version = "0.70.0" +version = "0.71.0" source = { editable = "." } dependencies = [ { name = "attrs" },