feat(mcp): wire Factory.base_currency_code through variant details card (#753)

Closes #751.

Variant prices (``sales_price`` / ``purchase_price``) don't carry a
per-record currency on the wire — they're denominated in
``Factory.base_currency_code``. Pre-this-change the variant detail
card's ``_price_display`` fell back to USD, which was wrong for non-USD
tenants (EUR-base tenants saw ``$12.99`` instead of ``€12.99``; JPY-base
saw ``$1500.00`` instead of ``¥1,500``).

Wire path:
- New ``resolve_factory_base_currency(catalog) -> str | None`` helper
  in ``tool_result_utils`` reads the cached ``CachedFactory`` singleton
  (id=1, synthesized by the sync pipeline since ``GET /factory`` has no
  ``id`` on the wire). Best-effort: swallows cache exceptions / returns
  ``None`` on cold cache so card rendering stays robust.
- ``VariantDetailsResponse`` gains a ``base_currency_code: str | None``
  field (response-dict wire path — same style as the existing
  ``currency`` field on ``SalesOrderResponse`` / ``PurchaseOrderResponse``,
  no new kwarg threading through the builder signature).
- ``_get_variant_details_impl`` resolves the base currency once per batch
  via ``asyncio.gather`` alongside parent/supplier enrichment, then
  stamps it onto every ``VariantDetailsResponse``. Adds ``CachedFactory``
  to the ``@cache_read`` decoration so the sync runs before the lookup.
- ``build_variant_details_ui`` reads ``variant["base_currency_code"]``
  and passes it to ``_format_money``. Missing field falls back to USD
  (cold cache / pre-#751 fixtures).

Deferred per the issue's "consider" criteria:
- Surfacing ``*_in_base_currency`` Metric alongside transaction-currency
  Total: skipped. SO/PO cards are already information-dense, and the
  issue suggests these add value only with ``conversion_rate`` +
  ``conversion_date`` captions for audit-trail provenance — a richer
  redesign deserves its own card iteration.
- ``_format_cost`` (bare-decimal per-row DataTable cell): kept as-is.
  Adding Babel-aware formatting per row would clutter dense tables with
  repeated symbols (``$437.50``, ``$5.10``, ``$92.00`` x N rows) and
  duplicate information the column header already carries. Decision
  recorded in the docstring; revisit only if multi-currency mixing
  within a single table causes confusion.

Tests: 10 new cases across ``test_prefab_ui.py`` (EUR / JPY / fallback),
``test_tool_result_utils.py`` (5 cases — hit, dict-shape, miss, exception,
empty-string), and ``test_inventory.py`` (impl-level plumbing
verification + cold-cache tolerance).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: dougborg

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

@@ -45,6 +45,7 @@
 from katana_mcp.tools.tool_result_utils import (
     UI_META,
     make_tool_result,
+    resolve_factory_base_currency,
 )
 from katana_mcp.unpack import Unpack, unpack_pydantic_params
 from katana_mcp.web_urls import EntityKind, katana_web_url
@@ -92,6 +93,7 @@
     Variant,
 )
 from katana_public_api_client.models_pydantic._generated import (
+    CachedFactory,
     CachedMaterial,
     CachedProduct,
     CachedSupplier,
@@ -1647,6 +1649,18 @@ class VariantDetailsResponse(BaseModel):
     # Pricing
     sales_price: float | None = None
     purchase_price: float | None = None
+    base_currency_code: str | None = None
+    """Tenant-wide base currency (``Factory.base_currency_code``).
+
+    Variant prices don't carry a per-record currency on the wire —
+    they're denominated in the tenant's base currency. Plumbed through
+    from :func:`resolve_factory_base_currency` so the card builder can
+    render the price symbol/decimals correctly (``$1,500.00`` for USD
+    tenants, ``€1,500.00`` for EUR tenants, ``¥1,500`` for JPY tenants
+    with no decimals) instead of falling back to ``USD``. ``None`` when
+    the factory record isn't cached yet — callers fall back to ``USD``
+    via :func:`_format_money`.
+    """
 
     # Classification
     type: str | None = None
@@ -1742,6 +1756,7 @@ def _dict_to_variant_details(
     *,
     parent: Any | None = None,
     supplier: Any | None = None,
+    base_currency_code: str | None = None,
 ) -> VariantDetailsResponse:
     """Build a VariantDetailsResponse from a variant cache row or attrs model.
 
@@ -1751,6 +1766,12 @@ def _dict_to_variant_details(
     supplied. Both are optional; missing parents/suppliers (cold cache,
     miss) gracefully degrade to ``None`` for those fields.
 
+    ``base_currency_code`` is the tenant's ``Factory.base_currency_code``;
+    pass it from the caller (resolved once per batch via
+    :func:`resolve_factory_base_currency`) so the variant card renders
+    the price symbol correctly. ``None`` falls back to ``USD`` in the
+    UI layer.
+
     Accepts either a ``CachedVariant`` SQLModel (the cache hit path)
     or a generated ``Variant`` attrs model (the API-fallback path)
     via ``_attr``, which unwraps ``UNSET`` for the attrs side and
@@ -1821,6 +1842,7 @@ def _dump_list(items: Any) -> list[Any]:
         display_name=display_name_value,
         sales_price=_attr(v, "sales_price"),
         purchase_price=_attr(v, "purchase_price"),
+        base_currency_code=base_currency_code,
         type=type_str,
         product_id=product_id,
         material_id=material_id,
@@ -2006,6 +2028,7 @@ def _partition_variant_lookups(
     CachedProduct,
     CachedMaterial,
     CachedSupplier,
+    CachedFactory,
 )
 async def _get_variant_details_impl(
     request: GetVariantDetailsRequest, context: Context
@@ -2050,10 +2073,15 @@ async def _get_variant_details_impl(
 
     # Bulk-fetch parents + suppliers once for the whole batch so the
     # response includes UoM, default supplier name, and batch-tracked
-    # status without a per-variant follow-up call (#538).
-    products, materials, supplier_by_id = await _enrich_variants_with_parent(
-        services, hits
-    )
+    # status without a per-variant follow-up call (#538). The factory
+    # base-currency lookup goes alongside so the variant card renders
+    # prices with the right symbol/decimals (#751) — variants don't
+    # carry a per-record currency on the wire.
+    enrichment, base_currency_code = await asyncio.gather(
+        _enrich_variants_with_parent(services, hits),
+        resolve_factory_base_currency(catalog),
+    )
+    products, materials, supplier_by_id = enrichment
     found: list[VariantDetailsResponse] = []
     for v in hits:
         # Pick the parent map by which ID the variant carries — product
@@ -2068,7 +2096,14 @@ async def _get_variant_details_impl(
             parent = None
         sup_id = _attr(parent, "default_supplier_id")
         supplier = supplier_by_id.get(sup_id) if sup_id else None
-        found.append(_dict_to_variant_details(v, parent=parent, supplier=supplier))
+        found.append(
+            _dict_to_variant_details(
+                v,
+                parent=parent,
+                supplier=supplier,
+                base_currency_code=base_currency_code,
+            )
+        )
 
     return GetVariantDetailsResult(found=found, not_found=not_found)
 

katana_mcp_server/src/katana_mcp/tools/prefab_ui.py

@@ -824,19 +824,23 @@ def build_variant_details_ui(
     needs first (UoM, default supplier, batch tracking, config
     attributes), with raw IDs deprioritized to a footer-style row. See
     #538 for the design rationale.
+
+    Variant prices (``sales_price`` / ``purchase_price``) are tenant-wide
+    and don't carry a per-record currency on the wire — they're
+    denominated in ``Factory.base_currency_code``. The caller threads
+    the resolved value through ``variant["base_currency_code"]`` (see
+    :func:`resolve_factory_base_currency`); the card falls back to USD
+    when the field is missing (cold cache / pre-#751 fixtures), so
+    rendering stays robust.
     """
     uom = variant.get("uom")
+    base_currency = variant.get("base_currency_code")
 
     def _price_display(p: float | None) -> str:
         if p is None:
             return "N/A"
         suffix = f" / {uom}" if uom and uom not in ("pcs", "ea") else ""
-        # Variant prices are tenant-wide and don't carry a per-record
-        # currency on the wire; they're denominated in
-        # ``Factory.base_currency_code``. The factory record isn't
-        # plumbed into this builder yet, so the format falls back to
-        # USD via :func:`_format_money` — wiring tracked in #751.
-        return f"{_format_money(p, None)}{suffix}"
+        return f"{_format_money(p, base_currency)}{suffix}"
 
     with PrefabApp(state={"variant": variant}, css_class="p-4") as app, Card():
         with CardHeader(), Column(gap=1):
@@ -1339,7 +1343,21 @@ def _format_qty_change(qty: float) -> str:
 
 
 def _format_cost(cost: float | int | None) -> str:
-    """Format a per-unit cost; ``—`` when omitted."""
+    """Format a per-unit cost; ``—`` when omitted.
+
+    Intentionally bare-decimal — no currency symbol, no ISO code. This
+    is the per-row DataTable cell formatter (Stock Adjustment rows,
+    PO/SO/MO rows in cards), where the column header already conveys
+    "Cost / unit" and the parent card carries the currency. Adding
+    Babel-aware formatting here would clutter dense rows with
+    repeated symbols (``$437.50``, ``$5.10``, ``$92.00`` x N rows) and
+    duplicate information the header already provides.
+
+    Per #751 acceptance criterion 5: revisit only if multi-currency
+    mixing within a single table causes confusion. The Metric-level
+    "Total" still uses :func:`_format_money` for the parent-card
+    aggregate, so the currency stays visible at the right granularity.
+    """
     if cost is None:
         return "—"
     return f"{cost:.2f}"

katana_mcp_server/src/katana_mcp/tools/tool_result_utils.py

@@ -49,6 +49,49 @@
 BLOCK_WARNING_PREFIX = "BLOCK:"
 
 
+async def resolve_factory_base_currency(catalog: Any) -> str | None:
+    """Look up the tenant's ``Factory.base_currency_code`` from the typed cache.
+
+    Katana's factory record is a singleton (``GET /factory``) — the typed
+    cache stores it under ``CachedFactory.id == 1`` (synthesized by the
+    sync pipeline since the wire shape carries no ``id``). This helper
+    centralizes the lookup so callers can plumb the tenant's base
+    currency through to UI builders without re-deriving the singleton
+    convention each time.
+
+    Returns ``None`` (rather than raising) when the cache is cold,
+    unhealthy, or the record is missing — callers should treat it as
+    "currency unknown" and fall back to a default (most card builders
+    fall back to ``USD`` via :func:`_format_money`). The MCP layer's
+    other resolvers (``resolve_entity_name``) follow the same
+    best-effort shape; the live API stays the authority for correctness.
+
+    Use this when formatting amounts denominated in the tenant's base
+    currency: variant ``sales_price`` / ``purchase_price`` and any
+    ``*_in_base_currency`` field on transactional responses. Transaction
+    currency (``SalesOrder.currency``, ``PurchaseOrder.currency``) is
+    per-record and should be read off the response directly — don't
+    confuse the two.
+    """
+    from katana_public_api_client.models_pydantic._generated import CachedFactory
+
+    try:
+        row = await catalog.get_by_id(
+            CachedFactory, 1, include_archived=True, include_deleted=True
+        )
+    except Exception as exc:
+        logger.warning(
+            "resolve_factory_base_currency: cache lookup failed",
+            error=str(exc),
+        )
+        return None
+    if row is None:
+        return None
+    if isinstance(row, dict):
+        return row.get("base_currency_code") or None
+    return getattr(row, "base_currency_code", None) or None
+
+
 async def resolve_entity_name(
     catalog: Any,
     cached_cls: Any,

katana_mcp_server/tests/test_prefab_ui.py

@@ -287,6 +287,66 @@ def test_omits_uom_when_unset(self):
         # No `/ <uom>` suffix on the bare price.
         assert "$10.00 /" not in rendered
 
+    def test_price_uses_factory_base_currency_eur(self):
+        """#751 — variant prices render with the tenant's base currency.
+
+        Pre-#751 the builder hardcoded USD; an EUR-base tenant saw
+        ``$12.99`` instead of ``€12.99``. The caller now threads
+        ``base_currency_code`` (resolved once per batch via
+        :func:`resolve_factory_base_currency`) into the variant dict
+        before building the card.
+        """
+        variant = {
+            "id": 100,
+            "sku": "WIDGET-EU",
+            "name": "Widget (EU)",
+            "sales_price": 12.99,
+            "purchase_price": 7.50,
+            "base_currency_code": "EUR",
+        }
+        app = build_variant_details_ui(variant)
+        _assert_valid_prefab(app)
+        rendered = str(app.to_json())
+        assert "€12.99" in rendered  # EUR symbol
+        assert "€7.50" in rendered
+        # USD must not leak in when the base is non-USD.
+        assert "$12.99" not in rendered
+
+    def test_price_uses_factory_base_currency_jpy_no_decimals(self):
+        """#751 — JPY tenants get ``\xa51,500`` (no decimals) per ISO 4217.
+
+        Verifies Babel's per-ISO decimal-digit rule round-trips through
+        the variant builder, not just the standalone ``_format_money``
+        helper.
+        """
+        variant = {
+            "id": 100,
+            "sku": "WIDGET-JP",
+            "name": "Widget (JP)",
+            "sales_price": 1500.0,
+            "base_currency_code": "JPY",
+        }
+        app = build_variant_details_ui(variant)
+        _assert_valid_prefab(app)
+        rendered = str(app.to_json())
+        assert "\xa51,500" in rendered  # JPY yen symbol, no decimals
+        assert "\xa51,500.00" not in rendered  # Babel drops the decimals
+
+    def test_price_falls_back_to_usd_when_base_currency_missing(self):
+        """Cold-cache / pre-#751 fixtures: missing ``base_currency_code``
+        falls back to USD so the card still renders cleanly."""
+        variant = {
+            "id": 100,
+            "sku": "SKU-001",
+            "name": "Widget",
+            "sales_price": 10.0,
+            # No base_currency_code field.
+        }
+        app = build_variant_details_ui(variant)
+        _assert_valid_prefab(app)
+        rendered = str(app.to_json())
+        assert "$10.00" in rendered
+
     def test_includes_purchase_uom_when_set_and_different_from_stock(self):
         """Purchase UoM row renders the kit-size conversion when the item is
         purchased in a different unit than it's stocked in (SP0502 case: stock

katana_mcp_server/tests/tools/test_inventory.py

@@ -1035,6 +1035,88 @@ async def test_get_variant_details():
     assert result.custom_fields[0]["field_name"] == "Warranty"
 
 
+@pytest.mark.asyncio
+async def test_get_variant_details_plumbs_factory_base_currency():
+    """#751 — ``_get_variant_details_impl`` resolves
+    ``Factory.base_currency_code`` from the typed cache and stamps it
+    onto :class:`VariantDetailsResponse.base_currency_code` so the
+    variant card can render prices with the correct currency symbol
+    (not the USD fallback)."""
+    context, lifespan_ctx = create_mock_context()
+
+    cached_variant = {
+        "id": 123,
+        "sku": "WIDGET-EU",
+        "type": "product",
+        "display_name": "Widget EU",
+        "sales_price": 12.99,
+        "purchase_price": 7.50,
+    }
+
+    lifespan_ctx.typed_cache.catalog.get_by_sku = AsyncMock(return_value=cached_variant)
+    # Factory singleton (id=1) returns an EUR-base tenant. The shape
+    # mirrors a real CachedFactory row — a stub object with the
+    # ``base_currency_code`` attribute, as the cache helper accepts
+    # either SQLModel rows or dicts.
+
+    class _StubFactory:
+        base_currency_code = "EUR"
+
+    async def _get_by_id(cls, entity_id, **_kwargs):
+        # Only the factory lookup uses get_by_id in this impl; everything
+        # else goes through get_by_sku / get_many_by_ids.
+        if entity_id == 1:
+            return _StubFactory()
+        return None
+
+    lifespan_ctx.typed_cache.catalog.get_by_id = AsyncMock(side_effect=_get_by_id)
+
+    request = GetVariantDetailsRequest(sku="WIDGET-EU")
+    _var_results = await _get_variant_details_impl(request, context)
+    result = _var_results.found[0]
+
+    assert result.base_currency_code == "EUR"
+
+
+@pytest.mark.asyncio
+async def test_get_variant_details_tolerates_missing_factory():
+    """Cold-cache path: when the factory record isn't synced yet, the
+    impl still returns the variant with ``base_currency_code=None`` so
+    the card render falls back to USD via :func:`_format_money` rather
+    than blowing up."""
+    context, lifespan_ctx = create_mock_context()
+
+    cached_variant = {
+        "id": 123,
+        "sku": "WIDGET-001",
+        "type": "product",
+        "display_name": "Widget",
+        "sales_price": 29.99,
+    }
+
+    lifespan_ctx.typed_cache.catalog.get_by_sku = AsyncMock(return_value=cached_variant)
+    # Replace get_by_id with a spy that returns None — pins both the
+    # "cold cache → None resolution" behavior AND the (CachedFactory, 1)
+    # singleton-lookup contract, so a future refactor that drops the
+    # factory call or changes the id wouldn't slip through silently.
+    lifespan_ctx.typed_cache.catalog.get_by_id = AsyncMock(return_value=None)
+
+    request = GetVariantDetailsRequest(sku="WIDGET-001")
+    _var_results = await _get_variant_details_impl(request, context)
+    result = _var_results.found[0]
+
+    assert result.base_currency_code is None
+    # Verify the factory lookup actually fired with the singleton id.
+    factory_calls = [
+        call
+        for call in lifespan_ctx.typed_cache.catalog.get_by_id.call_args_list
+        if len(call.args) >= 2 and call.args[1] == 1
+    ]
+    assert factory_calls, (
+        "expected resolve_factory_base_currency to call get_by_id(CachedFactory, 1)"
+    )
+
+
 @pytest.mark.asyncio
 async def test_get_variant_details_case_insensitive():
     """Test get_variant_details with case-insensitive SKU matching."""