Skip to content

Commit b5f149d

Browse files
dougborgclaude
andauthored
refactor(mcp): extract _format_money helper + standardize currency rendering (#752)
**Helper extraction (closes #744)** Pulled the duplicated ``f"${amount:,.2f} {currency or 'USD'}"`` block out of three create-card builders (PO create, SO create, PO receipt) plus the variant-detail price display into a single ``_format_money(amount, currency)`` helper in ``prefab_ui.py``. **Babel for currency-aware formatting** The old inline format hard-coded ``$`` and 2 decimals — wrong for EUR (``$1,500.00 EUR`` instead of ``€1,500.00``), wrong for JPY (JPY has no decimals on the wire), wrong for any currency whose symbol isn't ``$``. ``_format_money`` now delegates to ``babel.numbers.format_currency`` so the rendered string picks up the right symbol, decimal-digit count, and grouping per ISO 4217. Examples (verified in new ``TestFormatMoney`` cases): - ``_format_money(1500.0, "USD")`` → ``"$1,500.00"`` - ``_format_money(1500.0, "EUR")`` → ``"€1,500.00"`` - ``_format_money(1500, "JPY")`` → ``"¥1,500"`` (no decimals) - ``_format_money(None, "USD")`` → ``"$0.00"`` - Unknown ISO code → ``"XYZ1,500.00"`` (code prefix, doesn't raise) Added ``babel>=2.17`` to ``katana_mcp_server/pyproject.toml`` (it was already a transitive dep via mkdocs-material; promoting to a direct dependency). **Two Katana currency concepts, both surfaced** The helper's docstring records the two-currency model so future callers know which to pass: - **Transaction currency** — ``SalesOrder.currency`` / ``PurchaseOrder.currency``; the currency line totals (``total``, ``price_per_unit``, ``cogs_value``) are denominated in. - **Factory base currency** — ``Factory.base_currency_code``; the tenant home currency. Pass this when formatting ``*_in_base_currency`` amounts. Threading factory data into card builders is the remaining piece — currently only the three migrated Total metrics receive transaction currency; the variant-price builder falls back to USD because the factory record isn't plumbed into it yet. Tracked as a follow-up. **MCP boundary fixes for the #749 schema changes** PR #749 reclassified two MO recipe row fields from ``number`` to ``string`` (their wire format). The consumer in ``_recipe_row_info_from_attrs`` passed the strings straight through to ``RecipeRowInfo``'s ``float | None`` field annotations — it worked via pydantic coercion but was inconsistent with the boundary pattern established in #741 for SalesOrderRow. Wrapped the affected reads with ``float_or_none(unwrap_unset(...))`` to match. Also fixed ``MO operation row`` consumers for the five string-on-wire fields that were already strings before #749. And ``invoicing_status`` on ``SalesOrderSummary`` / the get-SO response was passed bare while its sibling status fields used ``enum_to_str``. After #749 made it an actual enum, the bare pass relied on StrEnum coincidence; brought it in line with ``status`` / ``production_status``. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8c3c69c commit b5f149d

6 files changed

Lines changed: 101 additions & 17 deletions

File tree

katana_mcp_server/pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ dependencies = [
4747
# context for the DB driver. Declared explicitly for the #342 typed
4848
# cache; without it, async session operations raise at runtime.
4949
"greenlet>=3.0.0",
50+
# ISO 4217 currency-aware money formatting (Prefab UI ``Total`` metrics).
51+
# Picks per-currency symbol, decimal digits, and separators so non-USD
52+
# orders render correctly (€1,500.00, ¥1500 with no decimals, etc.).
53+
"babel>=2.17",
5054
]
5155

5256
# Note: Development dependencies like mcp-hmr require Python >=3.12

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
apply_date_window_filters,
6161
coerce_enum,
6262
enum_to_str,
63+
float_or_none,
6364
iso_or_none,
6465
make_json_result,
6566
make_tool_result,
@@ -838,16 +839,20 @@ def _recipe_row_info_from_attrs(
838839
sku=sku,
839840
display_name=display_name,
840841
notes=unwrap_unset(row.notes, None),
841-
planned_quantity_per_unit=unwrap_unset(row.planned_quantity_per_unit, None),
842-
total_actual_quantity=unwrap_unset(row.total_actual_quantity, None),
842+
planned_quantity_per_unit=float_or_none(
843+
unwrap_unset(row.planned_quantity_per_unit, None)
844+
),
845+
total_actual_quantity=float_or_none(
846+
unwrap_unset(row.total_actual_quantity, None)
847+
),
843848
total_consumed_quantity=unwrap_unset(row.total_consumed_quantity, None),
844849
total_remaining_quantity=unwrap_unset(row.total_remaining_quantity, None),
845850
ingredient_availability=unwrap_unset(row.ingredient_availability, None),
846851
ingredient_expected_date=iso_or_none(
847852
unwrap_unset(row.ingredient_expected_date, None)
848853
),
849854
batch_transactions=batch_infos,
850-
cost=unwrap_unset(row.cost, None),
855+
cost=float_or_none(unwrap_unset(row.cost, None)),
851856
created_at=iso_or_none(unwrap_unset(row.created_at, None)),
852857
updated_at=iso_or_none(unwrap_unset(row.updated_at, None)),
853858
deleted_at=iso_or_none(unwrap_unset(row.deleted_at, None)),
@@ -885,13 +890,19 @@ def _operation_row_info_from_attrs(row: Any) -> OperationRowInfo:
885890
assigned_operators=assigned,
886891
completed_by_operators=completed,
887892
active_operator_id=unwrap_unset(row.active_operator_id, None),
888-
planned_time_per_unit=unwrap_unset(row.planned_time_per_unit, None),
889-
planned_time_parameter=unwrap_unset(row.planned_time_parameter, None),
890-
total_actual_time=unwrap_unset(row.total_actual_time, None),
893+
planned_time_per_unit=float_or_none(
894+
unwrap_unset(row.planned_time_per_unit, None)
895+
),
896+
planned_time_parameter=float_or_none(
897+
unwrap_unset(row.planned_time_parameter, None)
898+
),
899+
total_actual_time=float_or_none(unwrap_unset(row.total_actual_time, None)),
891900
total_consumed_time=unwrap_unset(row.total_consumed_time, None),
892901
total_remaining_time=unwrap_unset(row.total_remaining_time, None),
893-
planned_cost_per_unit=unwrap_unset(row.planned_cost_per_unit, None),
894-
total_actual_cost=unwrap_unset(row.total_actual_cost, None),
902+
planned_cost_per_unit=float_or_none(
903+
unwrap_unset(row.planned_cost_per_unit, None)
904+
),
905+
total_actual_cost=float_or_none(unwrap_unset(row.total_actual_cost, None)),
895906
cost_per_hour=unwrap_unset(row.cost_per_hour, None),
896907
cost_parameter=unwrap_unset(row.cost_parameter, None),
897908
group_boundary=unwrap_unset(row.group_boundary, None),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ def _variant_for_row(row: Any) -> Any:
917917
location_id=so.location_id,
918918
status=enum_to_str(so.status),
919919
production_status=enum_to_str(so.production_status),
920-
invoicing_status=so.invoicing_status,
920+
invoicing_status=enum_to_str(so.invoicing_status),
921921
created_at=iso_or_none(so.created_at),
922922
delivery_date=iso_or_none(so.delivery_date),
923923
total=so.total,
@@ -1335,7 +1335,7 @@ def _display_name_for(v: Any) -> str | None:
13351335
order_created_date=iso_or_none(unwrap_unset(so.order_created_date, None)),
13361336
status=enum_to_str(unwrap_unset(so.status, None)),
13371337
production_status=enum_to_str(unwrap_unset(so.production_status, None)),
1338-
invoicing_status=unwrap_unset(so.invoicing_status, None),
1338+
invoicing_status=enum_to_str(unwrap_unset(so.invoicing_status, None)),
13391339
product_availability=enum_to_str(unwrap_unset(so.product_availability, None)),
13401340
product_expected_date=iso_or_none(unwrap_unset(so.product_expected_date, None)),
13411341
ingredient_availability=enum_to_str(

katana_mcp_server/src/katana_mcp/tools/prefab_ui.py

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848

4949
from typing import Any, Literal
5050

51+
from babel.numbers import format_currency
5152
from prefab_ui.actions import Action, SetState, ShowToast
5253
from prefab_ui.actions.mcp import CallTool, SendMessage, UpdateContext
5354
from prefab_ui.actions.navigation import OpenLink
@@ -830,7 +831,12 @@ def _price_display(p: float | None) -> str:
830831
if p is None:
831832
return "N/A"
832833
suffix = f" / {uom}" if uom and uom not in ("pcs", "ea") else ""
833-
return f"${p:,.2f}{suffix}"
834+
# Variant prices are tenant-wide and don't carry a per-record
835+
# currency on the wire; they're denominated in
836+
# ``Factory.base_currency_code``. The factory record isn't
837+
# plumbed into this builder yet, so the format falls back to
838+
# USD via :func:`_format_money` — wiring tracked in #751.
839+
return f"{_format_money(p, None)}{suffix}"
834840

835841
with PrefabApp(state={"variant": variant}, css_class="p-4") as app, Card():
836842
with CardHeader(), Column(gap=1):
@@ -1339,6 +1345,36 @@ def _format_cost(cost: float | int | None) -> str:
13391345
return f"{cost:.2f}"
13401346

13411347

1348+
def _format_money(amount: float | int | None, currency: str | None) -> str:
1349+
"""Format a Metric ``Total`` value using ISO 4217 currency-aware rules.
1350+
1351+
Delegates to :func:`babel.numbers.format_currency` so the rendered string
1352+
picks up the right symbol, decimal-digit count, and grouping for the
1353+
currency (``$1,500.00`` for USD, ``€1,500.00`` for EUR, ``¥1,500`` for
1354+
JPY with no decimals). Integer ``amount`` is passed through unchanged
1355+
— Babel handles ``int`` and ``float`` identically.
1356+
1357+
Katana has two currency concepts:
1358+
1359+
- **Transaction currency** — ``SalesOrder.currency`` / ``PurchaseOrder.currency``;
1360+
the currency the line totals (``total``, ``total_cost``, ``price_per_unit``)
1361+
are denominated in. Pass this when formatting per-order amounts.
1362+
- **Factory base currency** — ``Factory.base_currency_code``; the tenant's
1363+
home currency. Pass this when formatting ``*_in_base_currency`` amounts
1364+
(converted totals).
1365+
1366+
Falls back to ``USD`` when ``currency`` is missing so the widget reads
1367+
cleanly even on the path where the response hasn't populated the field.
1368+
Unlike :func:`_format_cost`, never returns ``—`` — every Total-style
1369+
metric has a value to show (``$0.00`` for unset).
1370+
"""
1371+
return format_currency(
1372+
0 if amount is None else amount,
1373+
currency or "USD",
1374+
locale="en_US",
1375+
)
1376+
1377+
13421378
def _iso_date_only(value: object) -> str:
13431379
"""Trim a serialized ISO datetime to its ``YYYY-MM-DD`` prefix.
13441380
@@ -1850,7 +1886,9 @@ def build_po_create_ui(
18501886
Four-tier framework (#537):
18511887
- Tier 1 — Identity: title, order_number badge, PREVIEW/CREATED state
18521888
badge, status badge, entity_type badge (regular/outsourced).
1853-
- Tier 2 — Decision metrics: Total ($X.XX <currency>), Line Items.
1889+
- Tier 2 — Decision metrics: Total (formatted per
1890+
``response["currency"]`` via :func:`_format_money` — symbol + decimal
1891+
precision picked by Babel from the ISO 4217 code), Line Items.
18541892
- Tier 3 — Reference: supplier, location, notes, plus warnings.
18551893
- Tier 4 — Actions: Confirm + Cancel via the direct-apply rail
18561894
(Confirm fires ``tools/call`` directly and pushes the structured
@@ -1861,7 +1899,7 @@ def build_po_create_ui(
18611899
status = response.get("status")
18621900
entity_type = response.get("entity_type")
18631901
total_cost = response.get("total_cost")
1864-
currency = response.get("currency") or "USD"
1902+
currency = response.get("currency")
18651903
item_count = response.get("item_count")
18661904

18671905
apply_action = _build_apply_action_direct(confirm_tool, confirm_request)
@@ -1885,7 +1923,7 @@ def build_po_create_ui(
18851923
if total_cost is not None or item_count is not None:
18861924
with Row(gap=4):
18871925
if total_cost is not None:
1888-
Metric(label="Total", value=f"${total_cost:,.2f} {currency}")
1926+
Metric(label="Total", value=_format_money(total_cost, currency))
18891927
if item_count is not None:
18901928
Metric(label="Line Items", value=str(item_count))
18911929
_render_party_line(
@@ -1945,7 +1983,7 @@ def build_so_create_ui(
19451983
order_number = response.get("order_number") or "N/A"
19461984
status = response.get("status")
19471985
total = response.get("total")
1948-
currency = response.get("currency") or "USD"
1986+
currency = response.get("currency")
19491987
item_count = response.get("item_count")
19501988
delivery_date = response.get("delivery_date")
19511989

@@ -1966,7 +2004,7 @@ def build_so_create_ui(
19662004
if total is not None or item_count is not None or delivery_date:
19672005
with Row(gap=4):
19682006
if total is not None:
1969-
Metric(label="Total", value=f"${total:,.2f} {currency}")
2007+
Metric(label="Total", value=_format_money(total, currency))
19702008
if item_count is not None:
19712009
Metric(label="Line Items", value=str(item_count))
19722010
if delivery_date:
@@ -2736,7 +2774,9 @@ def build_receipt_ui(
27362774
if response.get("total_cost") is not None:
27372775
Metric(
27382776
label="PO Total",
2739-
value=f"${response['total_cost']:,.2f} {response.get('currency') or 'USD'}",
2777+
value=_format_money(
2778+
response["total_cost"], response.get("currency")
2779+
),
27402780
)
27412781

27422782
block_warnings, regular_warnings = _split_warnings(response.get("warnings"))

katana_mcp_server/tests/test_prefab_ui.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from katana_mcp.tools.prefab_ui import (
1616
PREVIEW_APPLY_COACHING,
1717
PREVIEW_APPLY_DIRECT_COACHING,
18+
_format_money,
1819
build_batch_recipe_update_ui,
1920
build_fulfill_preview_ui,
2021
build_fulfill_success_ui,
@@ -112,6 +113,32 @@ def _assert_valid_prefab(app: PrefabApp) -> None:
112113
_assert_state_bindings_resolve(result)
113114

114115

116+
class TestFormatMoney:
117+
"""``_format_money`` delegates to Babel for currency-aware rendering."""
118+
119+
def test_usd_amount(self):
120+
assert _format_money(1500.0, "USD") == "$1,500.00"
121+
122+
def test_eur_uses_euro_symbol(self):
123+
assert _format_money(1500.0, "EUR") == "€1,500.00"
124+
125+
def test_jpy_has_no_decimals(self):
126+
# JPY's ISO definition has zero decimal places — Babel drops them
127+
# automatically; a hand-rolled formatter would render "¥1500.00".
128+
assert _format_money(1500, "JPY") == "¥1,500"
129+
130+
def test_missing_currency_falls_back_to_usd(self):
131+
assert _format_money(1500.0, None) == "$1,500.00"
132+
133+
def test_none_amount_renders_as_zero(self):
134+
assert _format_money(None, "USD") == "$0.00"
135+
136+
def test_unknown_currency_keeps_code_prefix(self):
137+
# Babel gracefully handles unknown ISO codes by prefixing the code
138+
# instead of raising — keeps the helper total over partial.
139+
assert _format_money(1500.0, "XYZ") == "XYZ1,500.00"
140+
141+
115142
class TestBuildSearchResultsUI:
116143
def test_with_items(self):
117144
items = [

uv.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)