Skip to content

Commit 5b57bd5

Browse files
dougborgclaude
andcommitted
feat(mcp): per-entity PO modify card with field-level diff overlay — #722
Foundation + first migration for the #721 modify-card umbrella. Replaces the today's ``ActionResult``-shaped modify card (which showed "Update Header / 2 field(s) changed / Operation #1, Target #2641831" — internal model abstractions the user can't action on) with a per-entity entry that mirrors the create-card layout and decorates the changing fields with ``before → after``. What lands: - ``FieldChangeView`` + ``_index_changes_by_field`` + ``_render_field_diff_line`` — renderer-facing field-diff projection, shared by every future modify card. Flattens ``ActionResult.changes`` across all actions into a single field-name keyed map so the entity view's render code can do simple ``changes.get("expected_arrival_date")`` lookups. - ``_render_po_entity_view(entity, *, changes=None)`` — the PO entity-view body, extracted from the old inlined ``build_po_create_ui`` Tier 2/3 content. Called by BOTH the create card (``changes=None``) and the new modify card. Single source of truth for "what fields show in a PO card"; the create-modify pair can't drift over time. - ``build_po_modify_ui`` — the new entrypoint. Handles ``modify_purchase_order`` / ``delete_purchase_order`` / ``correct_purchase_order`` (verb from ``confirm_tool``). Renders the entity view with the diff overlay; the leading ``✗`` glyph + inline Muted error line appears only on failed actions (the agreed hybrid status approach — card-level header Badge carries the all/most-case status, per-field decoration only when it carries information). - Dispatch in ``_modification.to_tool_result`` — switches on ``response.entity_type``. PO routes to the new entrypoint; other entities fall through to the legacy ``build_modification_preview_ui`` / ``build_modification_result_ui`` pair until their child PRs (#723 SO, #724 MO, #725 stock-transfer, #726 item) ship. The legacy builders delete once every entity migrates. Tests: - ``TestFieldDiffIndex`` — pins ``_index_changes_by_field`` projection (multi-action flattening, added/unchanged kinds, failed-action error propagation). - ``TestBuildPOModifyUI`` — smoke + diff-decoration + supplier composite diff + failed-action ``✗`` glyph + delete verb + partial-failure header badge + ``state.result`` seeding on applied path. - ``TestPOEntityViewSharedBetweenCreateAndModify`` — pins the dual-call contract so a future refactor of ``_render_po_entity_view`` can't silently drift the create card away from modify. What this PR does NOT touch (deferred to follow-ups): - Line-item / additional-cost adds/removes (the ``add_row`` / ``delete_row`` / ``add_additional_cost`` operations) — the entity view doesn't yet render those rows. The current PO create card doesn't render them either, so this is consistent with #728's create-card scope, not a regression. Adding the nested-rows section is its own follow-up (probably the next PO modify PR or rolled into the SO migration where the entity-view component for line-items first matters). - Other entities (SO, MO, stock_transfer, item) — child PRs #723-#726. Closes #722 Refs #721 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 96d467e commit 5b57bd5

6 files changed

Lines changed: 1438 additions & 63 deletions

File tree

katana_mcp_server/src/katana_mcp/tools/_modification.py

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -554,25 +554,40 @@ def to_tool_result(
554554
) -> ToolResult:
555555
"""Build a :class:`ToolResult` with a Prefab UI from a ModificationResponse.
556556
557-
Preview branch: emits ``build_modification_preview_ui`` with the
558-
direct-apply rail (Confirm fires ``tools/call`` directly + iframe
559-
pushes the structured result back via ``ui/update-model-context``).
560-
Non-preview branch: emits ``build_modification_result_ui`` summarizing
561-
each action's terminal status.
562-
563-
``confirm_request`` is the original Pydantic request (its ``preview``
564-
field flips to ``False`` when the iframe re-issues for apply);
565-
``confirm_tool`` is the registered MCP tool name to re-call. The
566-
preview branch wires both into the Confirm-button CallTool; the
567-
result branch uses ``confirm_tool`` to derive the title verb so a
568-
delete reads "Product Delete" instead of "Product Modification".
557+
Dispatches by ``response.entity_type`` to a per-entity builder that
558+
handles BOTH preview and applied states (one entrypoint per entity,
559+
matching the create-card pattern in #728 and the modify-card design
560+
in #721). Each per-entity builder shares its entity-view renderer
561+
with its create-card sibling (e.g. ``_render_po_entity_view``) —
562+
create cards call it with no diff overlay; modify cards pass the
563+
per-field diff lookup built from ``response.actions[*].changes``.
564+
565+
Entities not yet migrated fall back to ``build_modification_preview_ui``
566+
/ ``build_modification_result_ui`` — the legacy single-entrypoint pair
567+
today's modify/delete/correct tools render through. Removed once
568+
every #721 child PR has shipped.
569569
"""
570570
from katana_mcp.tools.prefab_ui import (
571571
build_modification_preview_ui,
572572
build_modification_result_ui,
573+
build_po_modify_ui,
573574
)
574575

575576
response_dict = response.model_dump()
577+
578+
# Per-entity dispatch — PO migrated in #722; remaining entities
579+
# (SO, MO, stock_transfer, item) fall through to the legacy
580+
# builders until their respective child PRs land.
581+
if response.entity_type == "purchase_order":
582+
ui = build_po_modify_ui(
583+
response_dict,
584+
confirm_request=confirm_request,
585+
confirm_tool=confirm_tool,
586+
)
587+
return make_tool_result(response, ui=ui)
588+
589+
# Legacy path — preserves today's behavior for not-yet-migrated
590+
# entity types so #722 can land without cross-entity test churn.
576591
if response.is_preview:
577592
ui = build_modification_preview_ui(
578593
response_dict,

katana_mcp_server/src/katana_mcp/tools/_modification_dispatch.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -762,12 +762,23 @@ async def run_modify_plan(
762762
)
763763
raise ValueError(msg)
764764

765+
# ``prior_state`` populated on BOTH branches: apply path uses it for
766+
# the revert reference; preview path uses it for renderer-side entity
767+
# view (the modify-card design in #721 wants the unchanged header /
768+
# reference fields to render as context around the diff-decorated
769+
# changing fields — without prior_state, the card would show only the
770+
# changed fields and a mostly-empty header). ``existing`` may be None
771+
# if the diff fetch failed; ``serialize_for_prior_state`` tolerates
772+
# that and returns ``None`` so the renderer sees no snapshot.
773+
prior_state = serialize_for_prior_state(existing)
774+
765775
if request.preview:
766776
return ModificationResponse(
767777
entity_type=entity_type,
768778
entity_id=request.id,
769779
is_preview=True,
770780
actions=plan_to_preview_results(plan),
781+
prior_state=prior_state,
771782
warnings=warnings,
772783
next_actions=[
773784
f"Review {len(plan)} planned action(s)",
@@ -777,7 +788,6 @@ async def run_modify_plan(
777788
message=f"Preview: {len(plan)} action(s) planned for {entity_label}",
778789
)
779790

780-
prior_state = serialize_for_prior_state(existing)
781791
actions = await execute_plan(plan)
782792
message, next_actions = summarize_modify_outcome(
783793
actions, len(plan), entity_label=entity_label, tool_name=tool_name

katana_mcp_server/src/katana_mcp/tools/foundation/corrections.py

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -578,13 +578,22 @@ async def _correct_manufacturing_order_impl(
578578
close_phase = _build_close_mo_actions(request.id, snapshot, services)
579579
phases = [revert_phase, edit_phase, recreate_phase, close_phase]
580580

581+
# ``prior_state`` populated on BOTH branches: apply path uses it for
582+
# the revert reference; preview path uses it for renderer-side entity
583+
# view (#721 modify-card design — without prior_state, the rendered
584+
# card has only the changed-field diffs and an almost-empty header).
585+
prior_state = _augment_prior_state_with_snapshot(
586+
serialize_for_prior_state(existing_mo), snapshot
587+
)
588+
581589
if request.preview:
582590
full_plan = [action for phase in phases for action in phase]
583591
return ModificationResponse(
584592
entity_type="manufacturing_order",
585593
entity_id=request.id,
586594
is_preview=True,
587595
actions=plan_to_preview_results(full_plan),
596+
prior_state=prior_state,
588597
warnings=_close_state_warnings_mo(snapshot),
589598
next_actions=[
590599
f"Review {len(full_plan)} planned action(s) for MO {request.id}",
@@ -600,10 +609,6 @@ async def _correct_manufacturing_order_impl(
600609
f"({len(full_plan)} action(s))"
601610
),
602611
)
603-
604-
prior_state = _augment_prior_state_with_snapshot(
605-
serialize_for_prior_state(existing_mo), snapshot
606-
)
607612
aggregated, failed = await _run_phases_until_failure(phases)
608613
if failed:
609614
return _build_failure_response(
@@ -1097,13 +1102,21 @@ async def _correct_sales_order_impl(
10971102
close_phase = [_build_close_so_action(request.id, services)]
10981103
phases = [delete_phase, revert_phase, edit_phase, recreate_phase, close_phase]
10991104

1105+
# See #722 note on the MO correction above — prior_state populated
1106+
# on both branches so the per-entity modify card can render the
1107+
# unchanged-field context around the diff overlay on preview too.
1108+
prior_state = _augment_prior_state_with_snapshot(
1109+
serialize_for_prior_state(existing_so), snapshot
1110+
)
1111+
11001112
if request.preview:
11011113
full_plan = [action for phase in phases for action in phase]
11021114
return ModificationResponse(
11031115
entity_type="sales_order",
11041116
entity_id=request.id,
11051117
is_preview=True,
11061118
actions=plan_to_preview_results(full_plan),
1119+
prior_state=prior_state,
11071120
warnings=_close_state_warnings_so(snapshot),
11081121
next_actions=[
11091122
f"Review {len(full_plan)} planned action(s) for SO {request.id}",
@@ -1118,10 +1131,6 @@ async def _correct_sales_order_impl(
11181131
f"{request.id} ({len(full_plan)} action(s))"
11191132
),
11201133
)
1121-
1122-
prior_state = _augment_prior_state_with_snapshot(
1123-
serialize_for_prior_state(existing_so), snapshot
1124-
)
11251134
aggregated, failed = await _run_phases_until_failure(phases)
11261135
if failed:
11271136
return _build_failure_response(
@@ -1537,13 +1546,19 @@ async def _correct_purchase_order_impl(
15371546
]
15381547
phases = [revert_phase, edit_phase, receive_phase]
15391548

1549+
# See #722 note on the MO / SO correction above.
1550+
prior_state = _augment_prior_state_with_snapshot(
1551+
serialize_for_prior_state(existing_po), snapshot
1552+
)
1553+
15401554
if request.preview:
15411555
full_plan = [action for phase in phases for action in phase]
15421556
return ModificationResponse(
15431557
entity_type="purchase_order",
15441558
entity_id=request.id,
15451559
is_preview=True,
15461560
actions=plan_to_preview_results(full_plan),
1561+
prior_state=prior_state,
15471562
warnings=_close_state_warnings_po(snapshot),
15481563
next_actions=[
15491564
f"Review {len(full_plan)} planned action(s) for PO {request.id}",
@@ -1557,10 +1572,6 @@ async def _correct_purchase_order_impl(
15571572
f"{request.id} ({len(full_plan)} action(s))"
15581573
),
15591574
)
1560-
1561-
prior_state = _augment_prior_state_with_snapshot(
1562-
serialize_for_prior_state(existing_po), snapshot
1563-
)
15641575
aggregated, failed = await _run_phases_until_failure(phases)
15651576
if failed:
15661577
return _build_failure_response(

0 commit comments

Comments
 (0)