Skip to content

Commit 679909b

Browse files
dougborgclaude
andcommitted
feat(mcp): redesign variant_details card with parent-derived context
Variants don't carry uom, default supplier, or batch-tracking on their attrs model — those live on the parent product/material. The previous card forced agents to guess UoM (e.g., "250 = 250ml presumably"). Now the impl bulk-fetches parents + suppliers from the cache and the card surfaces UoM, default supplier, batch-tracked status, and config attributes inline. Raw IDs are deprioritized to a footer-style row. - Add uom, default_supplier_id, default_supplier_name, is_batch_tracked to VariantDetailsResponse - Add _enrich_variants_with_parent helper: 3 bulk cache queries (products, materials, suppliers) per get_variant_details call, regardless of variant count - Rewrite build_variant_details_ui as 4-tier layout (identity → metrics → reference → actions); price formatter shows "\$X / uom" suffix when uom isn't pcs/ea - Add 5 behavior tests covering UoM render/omit, config-attr badges, supplier name, and graceful fallback when parent lookup is empty Closes #538. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 746d17d commit 679909b

5 files changed

Lines changed: 364 additions & 52 deletions

File tree

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

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,12 @@ class VariantDetailsResponse(BaseModel):
12361236
"""Full variant details. Exhaustive — every field Katana exposes on the
12371237
``Variant`` attrs model is surfaced, including nested configuration
12381238
attributes and custom fields, so callers don't need follow-up lookups.
1239+
1240+
A handful of fields (``uom``, ``default_supplier_*``, ``is_batch_tracked``)
1241+
live on the parent product/material rather than on the variant attrs
1242+
model. The impl resolves the parent from the cache and lifts these
1243+
fields onto the variant response so a single ``get_variant_details``
1244+
call surfaces every fact a caller typically needs to act on.
12391245
"""
12401246

12411247
# Core fields
@@ -1253,6 +1259,13 @@ class VariantDetailsResponse(BaseModel):
12531259
material_id: int | None = None
12541260
product_or_material_name: str | None = None
12551261

1262+
# Parent-item context (lifted from the parent product/material —
1263+
# variants don't carry these on the attrs model directly)
1264+
uom: str | None = None
1265+
default_supplier_id: int | None = None
1266+
default_supplier_name: str | None = None
1267+
is_batch_tracked: bool | None = None
1268+
12561269
# Deep-link to the parent product or material — variants don't have
12571270
# their own page in Katana's web app, so callers click through to the
12581271
# parent record.
@@ -1302,11 +1315,19 @@ def _iso_or_none(value: Any) -> str | None:
13021315
return str(value)
13031316

13041317

1305-
def _dict_to_variant_details(v: dict[str, Any]) -> VariantDetailsResponse:
1318+
def _dict_to_variant_details(
1319+
v: dict[str, Any],
1320+
*,
1321+
parent: dict[str, Any] | None = None,
1322+
supplier: dict[str, Any] | None = None,
1323+
) -> VariantDetailsResponse:
13061324
"""Build a VariantDetailsResponse from a cache/API variant dict.
13071325
1308-
Surfaces every field the generated ``Variant`` attrs model exposes, so
1309-
callers get the full shape in a single call.
1326+
Surfaces every field the generated ``Variant`` attrs model exposes
1327+
plus the parent-derived context (``uom``, ``default_supplier_*``,
1328+
``is_batch_tracked``) when ``parent`` and/or ``supplier`` dicts are
1329+
supplied. Both are optional; missing parents/suppliers (cold cache,
1330+
miss) gracefully degrade to ``None`` for those fields.
13101331
"""
13111332
product_id = v.get("product_id")
13121333
material_id = v.get("material_id")
@@ -1317,6 +1338,7 @@ def _dict_to_variant_details(v: dict[str, Any]) -> VariantDetailsResponse:
13171338
parent_url = katana_web_url("product", product_id) or katana_web_url(
13181339
"material", material_id
13191340
)
1341+
parent_dict = parent or {}
13201342
return VariantDetailsResponse(
13211343
id=v["id"],
13221344
sku=v.get("sku") or "",
@@ -1327,6 +1349,10 @@ def _dict_to_variant_details(v: dict[str, Any]) -> VariantDetailsResponse:
13271349
product_id=product_id,
13281350
material_id=material_id,
13291351
product_or_material_name=v.get("parent_name"),
1352+
uom=parent_dict.get("uom"),
1353+
default_supplier_id=parent_dict.get("default_supplier_id"),
1354+
default_supplier_name=(supplier or {}).get("name"),
1355+
is_batch_tracked=parent_dict.get("batch_tracked"),
13301356
katana_url=parent_url,
13311357
internal_barcode=v.get("internal_barcode"),
13321358
registered_barcode=v.get("registered_barcode"),
@@ -1341,6 +1367,47 @@ def _dict_to_variant_details(v: dict[str, Any]) -> VariantDetailsResponse:
13411367
)
13421368

13431369

1370+
async def _enrich_variants_with_parent(
1371+
services: Any, variants: list[dict[str, Any]]
1372+
) -> tuple[
1373+
dict[int, dict[str, Any]],
1374+
dict[int, dict[str, Any]],
1375+
dict[int, dict[str, Any]],
1376+
]:
1377+
"""Bulk-fetch parent products/materials and default suppliers for a
1378+
set of variants. Returns ``(products_by_id, materials_by_id,
1379+
supplier_by_id)`` — separate maps per entity type so callers can
1380+
select the correct parent based on whether the variant carries
1381+
``product_id`` or ``material_id``.
1382+
1383+
Product and material IDs are NOT guaranteed disjoint (the cache
1384+
keys rows by ``(entity_type, id)``), so merging into a single map
1385+
keyed only by numeric ID would mis-associate parents on collision.
1386+
1387+
Splits parent IDs by entity type and uses ``get_many_by_ids`` per
1388+
type so we make at most three cache queries total
1389+
(parents-product, parents-material, suppliers) regardless of how
1390+
many variants are in the input.
1391+
"""
1392+
product_ids = {v.get("product_id") for v in variants if v.get("product_id")}
1393+
material_ids = {v.get("material_id") for v in variants if v.get("material_id")}
1394+
products, materials = await asyncio.gather(
1395+
services.cache.get_many_by_ids(EntityType.PRODUCT, product_ids),
1396+
services.cache.get_many_by_ids(EntityType.MATERIAL, material_ids),
1397+
)
1398+
# Pull supplier IDs from both parent dicts, then bulk-resolve
1399+
# supplier names — usually a small unique set even for big variant lists.
1400+
supplier_ids = {
1401+
p.get("default_supplier_id")
1402+
for p in (*products.values(), *materials.values())
1403+
if p.get("default_supplier_id")
1404+
}
1405+
supplier_by_id = await services.cache.get_many_by_ids(
1406+
EntityType.SUPPLIER, supplier_ids
1407+
)
1408+
return products, materials, supplier_by_id
1409+
1410+
13441411
async def _fetch_variant_by_id(services: Any, variant_id: int) -> dict[str, Any] | None:
13451412
"""Look up a variant by ID — cache first, then API fallback.
13461413
@@ -1404,15 +1471,35 @@ async def _get_variant_details_impl(
14041471
asyncio.gather(*(_fetch_variant_by_id(services, v) for v in variant_ids)),
14051472
)
14061473

1407-
results: list[VariantDetailsResponse] = []
1474+
found: list[dict[str, Any]] = []
14081475
for sku, v in zip(sku_cleaned, sku_variants, strict=True):
14091476
if not v:
14101477
raise ValueError(f"Variant with SKU '{sku}' not found")
1411-
results.append(_dict_to_variant_details(v))
1478+
found.append(v)
14121479
for variant_id, v in zip(variant_ids, id_variants, strict=True):
14131480
if v is None:
14141481
raise ValueError(f"Variant ID {variant_id} not found")
1415-
results.append(_dict_to_variant_details(v))
1482+
found.append(v)
1483+
1484+
# Bulk-fetch parents + suppliers once for the whole batch so the
1485+
# response includes UoM, default supplier name, and batch-tracked
1486+
# status without a per-variant follow-up call (#538).
1487+
products, materials, supplier_by_id = await _enrich_variants_with_parent(
1488+
services, found
1489+
)
1490+
results: list[VariantDetailsResponse] = []
1491+
for v in found:
1492+
# Pick the parent map by which ID the variant carries — product
1493+
# and material IDs may collide, so a merged map would mis-attach.
1494+
if v.get("product_id"):
1495+
parent = products.get(v["product_id"])
1496+
elif v.get("material_id"):
1497+
parent = materials.get(v["material_id"])
1498+
else:
1499+
parent = None
1500+
sup_id = (parent or {}).get("default_supplier_id")
1501+
supplier = supplier_by_id.get(sup_id) if sup_id else None
1502+
results.append(_dict_to_variant_details(v, parent=parent, supplier=supplier))
14161503

14171504
return results
14181505

katana_mcp_server/src/katana_mcp/tools/prefab_ui.py

Lines changed: 130 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -232,66 +232,152 @@ def build_search_results_ui(
232232
return app
233233

234234

235+
def _variant_header_section(variant: dict[str, Any]) -> None:
236+
"""Render variant card header: title, badges, parent, config-attribute pills."""
237+
with Row(gap=2):
238+
CardTitle(content=variant.get("name", "Unknown"))
239+
Badge(label=variant.get("sku", ""), variant="outline")
240+
if variant.get("type"):
241+
Badge(label=variant["type"], variant="secondary")
242+
if variant.get("is_batch_tracked"):
243+
Badge(label="Batch tracked", variant="secondary")
244+
parent_name = variant.get("product_or_material_name")
245+
if parent_name:
246+
Muted(content=f"Part of: {parent_name}")
247+
# Config-attribute pills (size / color / volume) inline so the
248+
# variant's distinguishing axes are visible without scrolling.
249+
config_attrs = variant.get("config_attributes") or []
250+
if config_attrs:
251+
with Row(gap=2):
252+
for attr in config_attrs:
253+
if not isinstance(attr, dict):
254+
continue
255+
label = attr.get("config_name") or ""
256+
value = attr.get("config_value") or ""
257+
if label and value:
258+
Badge(label=f"{label}: {value}", variant="outline")
259+
260+
261+
def _variant_supplier_line(variant: dict[str, Any]) -> None:
262+
"""Render the default-supplier text row when name and/or id is set."""
263+
name = variant.get("default_supplier_name")
264+
sid = variant.get("default_supplier_id")
265+
if name and sid:
266+
Text(content=f"Default Supplier: {name} ({sid})")
267+
elif name:
268+
Text(content=f"Default Supplier: {name}")
269+
elif sid:
270+
Text(content=f"Default Supplier ID: {sid}")
271+
272+
273+
def _variant_barcode_line(variant: dict[str, Any]) -> None:
274+
"""Render the barcode text row when at least one barcode is set."""
275+
parts = []
276+
if variant.get("internal_barcode"):
277+
parts.append(f"internal={variant['internal_barcode']}")
278+
if variant.get("registered_barcode"):
279+
parts.append(f"registered={variant['registered_barcode']}")
280+
if parts:
281+
Text(content=f"Barcodes: {', '.join(parts)}")
282+
283+
284+
def _variant_id_line(variant: dict[str, Any]) -> None:
285+
"""IDs deprioritized to a single Muted row at the bottom — they're
286+
useful for follow-up tool calls but not the primary signal a human
287+
reader needs.
288+
"""
289+
id_parts = [f"variant_id={variant.get('id', 'N/A')}"]
290+
if variant.get("product_id"):
291+
id_parts.append(f"product_id={variant['product_id']}")
292+
if variant.get("material_id"):
293+
id_parts.append(f"material_id={variant['material_id']}")
294+
Muted(content=" · ".join(id_parts))
295+
296+
297+
def _variant_reference_section(variant: dict[str, Any]) -> None:
298+
"""Render the reference data block (UoM, supplier, lead time, codes, IDs)."""
299+
if variant.get("uom"):
300+
Text(content=f"UoM: {variant['uom']}")
301+
_variant_supplier_line(variant)
302+
if variant.get("lead_time") is not None:
303+
Text(content=f"Lead Time: {variant['lead_time']} days")
304+
if variant.get("minimum_order_quantity") is not None:
305+
Text(content=f"Min Order Qty: {variant['minimum_order_quantity']}")
306+
_variant_barcode_line(variant)
307+
if variant.get("supplier_item_codes"):
308+
Muted(content="Supplier Codes:")
309+
with ForEach("variant.supplier_item_codes"):
310+
Text(content="{{ $item }}")
311+
_variant_id_line(variant)
312+
313+
314+
def _variant_footer_section(variant: dict[str, Any]) -> None:
315+
"""Render footer action buttons."""
316+
sku = variant.get("sku", "")
317+
if variant.get("katana_url"):
318+
Button(
319+
label="View in Katana",
320+
variant="outline",
321+
on_click=SendMessage(f"Open the Katana URL: {variant['katana_url']}"),
322+
)
323+
Button(
324+
label="Check Inventory",
325+
variant="outline",
326+
on_click=SendMessage(f"Check inventory for SKU {sku}"),
327+
)
328+
Button(
329+
label="Create Purchase Order",
330+
variant="outline",
331+
on_click=SendMessage(f"Draft a purchase order for SKU {sku}"),
332+
)
333+
if variant.get("type") == "material" and variant.get("id"):
334+
Button(
335+
label="List MOs Using This",
336+
variant="outline",
337+
on_click=SendMessage(
338+
f"List manufacturing orders that use variant_id {variant['id']}"
339+
),
340+
)
341+
342+
235343
def build_variant_details_ui(
236344
variant: dict[str, Any],
237345
) -> PrefabApp:
238-
"""Build a detail card for a variant."""
346+
"""Build a detail card for a variant.
347+
348+
Designed for the "should I order more / where is this used / how is
349+
it tracked" decisions. Surfaces the facts an inventory-aware agent
350+
needs first (UoM, default supplier, batch tracking, config
351+
attributes), with raw IDs deprioritized to a footer-style row. See
352+
#538 for the design rationale.
353+
"""
354+
uom = variant.get("uom")
355+
356+
def _price_display(p: float | None) -> str:
357+
if p is None:
358+
return "N/A"
359+
suffix = f" / {uom}" if uom and uom not in ("pcs", "ea") else ""
360+
return f"${p:,.2f}{suffix}"
361+
239362
with PrefabApp(state={"variant": variant}, css_class="p-4") as app, Card():
240-
with CardHeader(), Row(gap=2):
241-
CardTitle(content=variant.get("name", "Unknown"))
242-
Badge(label=variant.get("sku", ""), variant="outline")
243-
if variant.get("type"):
244-
Badge(
245-
label=variant["type"],
246-
variant="secondary",
247-
)
363+
with CardHeader(), Column(gap=1):
364+
_variant_header_section(variant)
248365

249366
with CardContent(), Column(gap=3):
250367
with Row(gap=4):
251368
Metric(
252369
label="Sales Price",
253-
value=f"${variant.get('sales_price', 0):,.2f}"
254-
if variant.get("sales_price") is not None
255-
else "N/A",
370+
value=_price_display(variant.get("sales_price")),
256371
)
257372
Metric(
258373
label="Purchase Price",
259-
value=f"${variant.get('purchase_price', 0):,.2f}"
260-
if variant.get("purchase_price") is not None
261-
else "N/A",
374+
value=_price_display(variant.get("purchase_price")),
262375
)
263-
264376
Separator()
265-
266-
with Row(gap=4):
267-
Text(content=f"ID: {variant.get('id', 'N/A')}")
268-
if variant.get("product_id"):
269-
Text(content=f"Product ID: {variant['product_id']}")
270-
if variant.get("material_id"):
271-
Text(content=f"Material ID: {variant['material_id']}")
272-
if variant.get("lead_time") is not None:
273-
Text(content=f"Lead Time: {variant['lead_time']} days")
274-
275-
if variant.get("supplier_item_codes"):
276-
Muted(content="Supplier Codes:")
277-
with ForEach("variant.supplier_item_codes"):
278-
Text(content="{{ $item }}")
377+
_variant_reference_section(variant)
279378

280379
with CardFooter(), Row(gap=2):
281-
Button(
282-
label="Check Inventory",
283-
variant="outline",
284-
on_click=SendMessage(
285-
f"Check inventory for SKU {variant.get('sku', '')}"
286-
),
287-
)
288-
Button(
289-
label="Create Purchase Order",
290-
variant="outline",
291-
on_click=SendMessage(
292-
f"Draft a purchase order for SKU {variant.get('sku', '')}"
293-
),
294-
)
380+
_variant_footer_section(variant)
295381
return app
296382

297383

0 commit comments

Comments
 (0)