Skip to content

Commit 42d782e

Browse files
dougborgclaude
andcommitted
feat(mcp)!: migrate catalog call sites to typed cache + decommission CatalogCache (#472 Phase D)
Phase D of the cache-unification effort. Migrates ~33 catalog read call sites from the legacy ``CatalogCache`` to ``TypedCacheEngine`` via the ``CatalogQueries`` adapter (``services.typed_cache.catalog``) and decommissions the legacy modules. Source migration: - Foundation tools: ``items``, ``inventory``, ``customers``, ``reference``, ``reporting``, ``orders``, ``manufacturing_orders``, ``sales_orders``, ``purchase_orders``, ``stock_transfers`` all call ``services.typed_cache.catalog.*(Cached*Class, ...)`` instead of ``services.cache.*(EntityType, ...)``. Dict access (``v["sku"]``) flips to attribute access (``v.sku``) with an ``_attr`` helper bridging dict / attrs / SQLModel call shapes in helper paths that may receive either a cache row or an API-fallback attrs model. - Decorator ``@cache_read`` keys off the ``Cached*`` SQLModel class (e.g. ``CachedVariant``) instead of an ``EntityType`` enum value; fans out to typed-cache sync helpers only. Legacy ``@cache_write`` / ``mark_dirty`` mechanism removed — typed cache pulls incremental deltas via ``updated_at_min`` on every ``@cache_read``-decorated call, so explicit invalidation is no longer needed. - ``resolve_entity_name`` takes a ``Cached*`` class plus the ``CatalogQueries`` adapter; tolerates both rows and dicts during the transition. - ``Services`` dataclass: ``cache`` field removed; only ``client`` and ``typed_cache`` remain. Lifespan in ``server.py`` initializes only the typed cache. - ``services.typed_cache.catalog`` is now the only catalog read path; the inline negative-lookup cache in reporting tightened to ``if variant_id not in variant_cache`` so ``None`` stays a valid cached negative result. Decommission: - Delete ``katana_mcp/cache.py`` (~870 LOC) and ``katana_mcp/cache_sync.py``. - Delete ``tests/test_cache.py`` and ``tests/test_cache_sync.py``. - ``EntityType`` enum disappears with ``cache.py``. Soft-state filter opt-ins on direct-lookup paths: - ``get_by_id`` / ``get_by_sku`` callers that need to surface archived or soft-deleted rows for inspection (e.g. ``check_inventory`` on an archived SKU, ``modify_*`` flows resolving a tombstoned entity) pass ``include_archived=True`` / ``include_deleted=True`` explicitly, matching the legacy cache's "show every row regardless of soft-state" semantics for direct lookups. The search-list paths (``smart_search`` / ``search_fuzzy`` / ``get_all``) keep the adapter's filtered defaults — the papercut fix from #472 Phase B. Tests: - ``conftest.create_mock_context`` exposes the six ``CatalogQueries`` methods on ``lifespan_ctx.typed_cache.catalog``; legacy ``lifespan_ctx.cache`` removed. - All ~13 mock test files migrated. Mock helpers accept ``**_kw`` to swallow the adapter's ``include_archived`` / ``include_deleted`` kwargs. ``CachedCustomer`` / ``CachedSupplier`` / ``CachedLocation`` / ``CachedVariant`` factories replace dict-shaped fixtures. Docs: - ADR-0018 status flipped — the "follow-up epic" is complete; the dual-cache rollout section is now historical. - ``architecture.md`` cache section rewritten for the single-cache reality; "Adding a new tool" checklist updated to direct contributors at ``@cache_read(CachedEntity)``. - CLAUDE.md "Known Pitfalls" updated — the cache-ID uniqueness note no longer points at the retired ``CatalogCache``; the soft-state flags note now describes the ``CatalogQueries`` adapter wiring. BREAKING CHANGE: ``Services.cache`` field removed. ``katana_mcp.cache`` and ``katana_mcp.cache_sync`` modules removed. ``EntityType`` enum removed. Third-party consumers of ``katana_mcp_server`` that touched any of these will need to migrate to ``services.typed_cache.catalog`` and the ``Cached*`` SQLModel classes from ``katana_public_api_client.models_pydantic._generated``. Closes #472. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 63359e1 commit 42d782e

38 files changed

Lines changed: 1518 additions & 2941 deletions

CLAUDE.md

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,15 @@ Common mistakes to avoid:
163163
soft-deleted. Two MCP-side conventions surface this; keep them symmetric:
164164

165165
- **Query-param flags** for opting into surfacing soft-state rows. **Default
166-
`False`.** Items use `include_archived` (`search_items`, catalog cache); the
167-
canonical wiring (landed in #526) is `cache.py`'s denormalized
168-
`entity_index.is_archived` column populated during sync, plus the
169-
`idx.is_archived = 0` predicate in `cache.search` / `search_fuzzy`. Transactional
166+
`False`.** Items use `include_archived` (`search_items`, catalog cache); after #472
167+
Phase D the canonical wiring is the typed cache's `CatalogQueries` adapter —
168+
`parent_archived_at` is denormalized onto `CachedVariant` at sync time (via the
169+
variant `attrs_postprocess` hook in `typed_cache/sync.py`) and the adapter's default
170+
`include_archived=False` / `include_deleted=False` filters push the
171+
`archived_at IS NULL` / `deleted_at IS NULL` predicates down to SQL. Transactional
170172
entities use `include_deleted` on `list_purchase_orders` / `list_sales_orders` /
171-
`list_manufacturing_orders` / `list_stock_adjustments`, filtering at the typed-cache
172-
query layer.
173+
`list_manufacturing_orders` / `list_stock_adjustments`, filtering at the same
174+
typed-cache query layer.
173175
- **Response-side derived booleans**: every response model that exposes `archived_at`
174176
/ `deleted_at` should also expose a convenience `is_archived` / `is_deleted` bool
175177
derived from `<timestamp> is not None`, saving callers from the timestamp/null
@@ -243,18 +245,18 @@ Common mistakes to avoid:
243245
`.claude/worktrees/` as off-limits for destructive operations.
244246

245247
- **Cache IDs are not globally unique — never merge cross-entity maps by numeric ID
246-
alone** - The legacy `CatalogCache` (`katana_mcp/cache.py`) — and `services.cache` by
247-
extension — keys rows by `(entity_type, id)`, so a product with `id=42` and a material
248-
with `id=42` are both legal. When enriching a list of variants with parent context (or
249-
any other cross-entity batch fetch), keep separate per-type maps (`products`,
250-
`materials`) and select based on which ID the variant carries (`v.product_id` vs
251-
`v.material_id`). Merging into a single dict via `{**products, **materials}`
252-
mis-attaches parents on collision — Python dict-unpack iterates left-to-right and
253-
later keys win, so the material entry silently overwrites the product entry on shared
254-
IDs. The bug is symmetric in practice: every product variant whose ID also exists as a
255-
material ID looks up the material's data instead (and vice versa if you reorder the
256-
unpack). Caught in #542 (variant card redesign) by Copilot review; regression test
257-
pins the case in
248+
alone** - The typed cache stores each entity type in its own table (`product`,
249+
`material`, `supplier`, ...), so a product with `id=42` and a material with `id=42`
250+
are both legal. When enriching a list of variants with parent context (or any other
251+
cross-entity batch fetch), keep separate per-type maps (`products`, `materials`) and
252+
select based on which ID the variant carries (`v.product_id` vs `v.material_id`).
253+
Merging into a single dict via `{**products, **materials}` mis-attaches parents on
254+
collision — Python dict-unpack iterates left-to-right and later keys win, so the
255+
material entry silently overwrites the product entry on shared IDs. The bug is
256+
symmetric in practice: every product variant whose ID also exists as a material ID
257+
looks up the material's data instead (and vice versa if you reorder the unpack).
258+
Caught in #542 (variant card redesign) by Copilot review; regression test pins the
259+
case in
258260
`test_items.py::test_enrich_variants_keeps_product_and_material_maps_separate`.
259261

260262
- **First push of a feature branch — use `HEAD:refs/heads/<name>`, not bare branch

katana_mcp_server/docs/adr/0018-sqlmodel-typed-cache.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ Accepted
66

77
Date: 2026-04-23
88

9+
**Update (2026-05-11, #472 Phase D):** the "follow-up epic" called out below — unifying
10+
the catalog tier under `TypedCacheEngine` and retiring `CatalogCache` — is **complete**.
11+
`katana_mcp.cache` and `katana_mcp.cache_sync` are removed; the typed cache now backs
12+
both transactional and catalog reads via the `CatalogQueries` adapter
13+
(`services.typed_cache.catalog`). The "dual-cache coexistence during rollout" section
14+
below describes a transient state that no longer exists. The "Scope 2" question
15+
(replacing the attrs API transport) remains open.
16+
917
## Context
1018

1119
Analytical workflows over Katana data were making large numbers of sequential API calls

katana_mcp_server/docs/architecture.md

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ calls with its own retry / rate-limit logic.
3131
│ Services / dependencies (services/dependencies.py) │
3232
│ get_services(context) → KatanaClient + caches │
3333
├────────────────────────────────────────────────────────────┤
34-
│ Caches (two, by design — see ADR-0018) │
35-
│ CatalogCache (cache.py) — reference entities │
36-
│ TypedCacheEngine (typed_cache/) — transactional entities │
34+
│ Cache (unified — see ADR-0018 + #472 Phase D) │
35+
│ TypedCacheEngine (typed_cache/) │
36+
│ - catalog tier — variants/products/materials/... │
37+
│ - transactional — sales/manufacturing/purchase orders │
3738
├────────────────────────────────────────────────────────────┤
3839
│ KatanaClient (katana_public_api_client) │
3940
│ - transport-layer resilience │
@@ -82,15 +83,15 @@ sync when tool surface changes.
8283

8384
### Cache-aware decorators
8485

85-
`tools/decorators.py` provides `@cache_read(CachedVariant, ...)` and
86-
`@cache_write("entity_a", "entity_b")` decorators. `cache_read` keys off the typed-cache
87-
`Cached*` row class (e.g. `CachedVariant`, `CachedProduct`) and triggers an incremental
88-
sync of the named entity before invoking the tool. During the #472 unification rollout
89-
(Phase C) the decorator fans each sync out to BOTH the legacy `CatalogCache` helper and
90-
the typed-cache helper so tool bodies see fresh data on either path; Phase D drops the
91-
legacy half. `cache_write` invalidates the listed entities after a mutating call so the
92-
next list/get returns fresh data. Tool implementations stay focused on business logic;
93-
sync orchestration lives in the decorator.
86+
`tools/decorators.py` provides `@cache_read(CachedVariant, CachedProduct, ...)`.
87+
`cache_read` triggers an incremental sync of the named typed-cache entities before
88+
invoking the tool. Tool implementations stay focused on business logic; sync
89+
orchestration lives in the decorator.
90+
91+
Cache invalidation after writes is **implicit**: the typed cache pulls incremental
92+
deltas via `updated_at_min` on every `@cache_read`-decorated call, so a freshly-created
93+
or modified entity is picked up automatically by the next read. The legacy `cache_write`
94+
/ `mark_dirty` mechanism was retired alongside `CatalogCache` (#472 Phase D).
9495

9596
## Resources
9697

@@ -116,37 +117,36 @@ from katana_mcp.services import get_services
116117

117118
services = get_services(context)
118119
client = services.client # KatanaClient
119-
catalog_cache = services.cache # CatalogCache
120120
typed_cache = services.typed_cache # TypedCacheEngine
121+
catalog = services.typed_cache.catalog # CatalogQueries adapter
121122
```
122123

123124
Lifespan management (engine open/close, client cleanup) is handled by `server.py`.
124125

125-
## Caches
126-
127-
The MCP server runs **two** complementary caches. They serve different needs and both
128-
are permanent — neither is a temporary stepping stone toward the other
129-
([ADR-0018](adr/0018-sqlmodel-typed-cache.md)).
126+
## Cache
130127

131-
### CatalogCache (`katana_mcp/cache.py`, `cache_sync.py`)
132-
133-
A generic SQLite + FTS5 store for the 10 reference entity types: variants, products,
134-
materials, services, suppliers, customers, locations, tax rates, operators, factories.
135-
Every row projects into a three-text-column `entity_index` (name, description, code) for
136-
cheap full-text search across heterogeneous types. Powers `search_items` and
137-
`get_variant_details`-style lookup tools.
128+
The MCP server runs a single SQLModel-backed cache covering both catalog and
129+
transactional tiers (see [ADR-0018](adr/0018-sqlmodel-typed-cache.md) for the original
130+
typed-cache architecture and #472 Phase D for the catalog unification).
138131

139132
### TypedCacheEngine (`katana_mcp/typed_cache/`)
140133

141-
SQLModel-backed per-entity tables for transactional types: sales orders, manufacturing
142-
orders, purchase orders, stock adjustments, stock transfers, manufacturing-order recipe
143-
rows. Each entity has its own table with proper FK relationships and JSON columns;
144-
nested rows (sales-order rows, MO recipe rows, …) become child tables with FKs back to
145-
the parent.
134+
SQLModel-backed per-entity tables for every cached type. Each entity has its own table
135+
with proper FK relationships and JSON columns; nested rows (sales-order rows, MO recipe
136+
rows, ...) become child tables with FKs back to the parent.
137+
138+
**Catalog tier** (11 entity types): variants, products, materials, services, suppliers,
139+
customers, locations, tax rates, operators, factories, additional costs. Search via
140+
per-entity FTS5 sidecar tables (`<entity>_fts`) wired through a `CatalogQueries` adapter
141+
exposed at `services.typed_cache.catalog`. The adapter provides typed `get_by_id` /
142+
`get_by_sku` / `get_many_by_ids` / `get_all` / `smart_search` / `search_fuzzy` methods
143+
that return `Cached*` SQLModel instances directly (not dict shims), with default
144+
`include_archived=False` / `include_deleted=False` filters.
146145

147-
The transactional types' filter shape (status enums, date ranges, customer/ supplier
148-
IDs, variant-id-via-rows) and 30+-field schemas don't fit `CatalogCache`'s
149-
three-text-column projection — hence the dedicated typed store.
146+
**Transactional tier** (10 entity types counting child rows): sales orders,
147+
manufacturing orders (+ recipe rows), purchase orders (+ rows), stock adjustments (+
148+
rows), stock transfers (+ rows). Searched via SQL `WHERE` clauses; no FTS sidecar —
149+
these tables don't carry free-text fields.
150150

151151
### EntitySpec — the generic sync driver
152152

@@ -213,9 +213,13 @@ bugs at the client/generator layer").
213213
integration; use elicitation for any state-changing operation.
214214
1. **Follow ADR-0019** for naming (`<entity>_<field>s` for batch list filters, singular
215215
for `get_*`) and the docstring opening sentence.
216-
1. **If the tool reads from cache,** add `@cache_read(CachedEntity)` keyed by the typed
217-
`Cached*` row class. If it writes, add `@cache_write("entity_a", "entity_b")` listing
218-
every entity whose cache should be invalidated.
216+
1. **If the tool reads from cache,** add `@cache_read(CachedEntity, ...)` keyed by the
217+
typed-cache `Cached*` SQLModel class (e.g. `CachedVariant`, `CachedProduct`).
218+
Mutating tools do **not** need an explicit invalidation decorator — the typed cache
219+
pulls incremental deltas via `updated_at_min` on every `@cache_read`-decorated call,
220+
so a freshly-created or modified entity is picked up by the next read. The legacy
221+
`@cache_write` / `mark_dirty` mechanism was retired with `CatalogCache` (#472 Phase
222+
D).
219223
1. **For new transactional list tools backed by typed cache:** add an `EntitySpec`
220224
literal in `typed_cache/sync.py` and a thin `ensure_<entity>_synced` wrapper. The
221225
`Cached<Entity>` row class is auto-generated from the spec by the next regen.

0 commit comments

Comments
 (0)