Skip to content

Commit f93c838

Browse files
dougborgclaude
andcommitted
docs(mcp): purge stale markdown/format references after #719
Three doc files still described the dual markdown/JSON content contract and the ``make_simple_result`` helper that #719 removed; flagged by Copilot review on PR #719 but never addressed because that PR's code landed via PR #728's merge train instead. Sweep on top of current main: - ``katana_mcp_server/docs/prefab/README.md``: ``make_simple_result`` → ``make_json_result``; clarify the "tools that emit no UI" guidance to point at the actual JSON-content helper. - ``katana_mcp_server/docs/adr/0019-tool-description-batch-conventions.md``: three references to "summary table" replaced with "JSON envelope of rows" — matches what ``check_inventory`` batch + ``list_*`` tools actually return post-#719. - ``katana_mcp_server/docs/examples.md``: rewrite the "Response Format" section + token-efficiency bullet so neither describes a dual markdown/JSON surface; explicit pointer to #719 for the removal of ``format``. - ``katana_mcp_server/docs/adr/0020-consistent-tool-surface-and-cache-unification.md``: drop ``format: "markdown" | "json"`` from the "param shapes" list, note it was removed in #719. No code change; pinning the docs to current reality so future tool- description work doesn't follow stale guidance and reintroduce markdown/format promises (the exact failure mode ADR-0019 is meant to prevent). Closes the three threads on #728 / #719 about doc drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 70c9ce4 commit f93c838

4 files changed

Lines changed: 160 additions & 125 deletions

File tree

katana_mcp_server/docs/adr/0019-tool-description-batch-conventions.md

Lines changed: 96 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -16,27 +16,26 @@ shape exists. Three things drove that fallback.
1616
**Batch capability buried mid-docstring.** First-line summaries said what the tool
1717
*returned* but not that it accepted a batch. Agents sized the input to the field they
1818
saw on the first read pass and didn't scroll for a "Note: this also accepts a list"
19-
buried five paragraphs later. PR #393 moved batch hints into the first sentence;
20-
without a written rule the next tool will drift back.
19+
buried five paragraphs later. PR #393 moved batch hints into the first sentence; without
20+
a written rule the next tool will drift back.
2121

22-
**Parallel singular + plural fields.** Tools like the original `check_inventory`
23-
exposed `sku` + `skus` + `variant_id` + `variant_ids` — four fields, mutually
24-
exclusive, runtime-validated. Agents guessed the wrong combination, sent both, or
25-
sent neither. PR #397 collapsed those four into a single
26-
`skus_or_variant_ids: list[str | int]`. Same fork in the road for every future tool;
27-
without a rule, contributors will reach for the four-field pattern again because
28-
"singular + plural" looks more discoverable.
22+
**Parallel singular + plural fields.** Tools like the original `check_inventory` exposed
23+
`sku` + `skus` + `variant_id` + `variant_ids` — four fields, mutually exclusive,
24+
runtime-validated. Agents guessed the wrong combination, sent both, or sent neither. PR
25+
#397 collapsed those four into a single `skus_or_variant_ids: list[str | int]`. Same
26+
fork in the road for every future tool; without a rule, contributors will reach for the
27+
four-field pattern again because "singular + plural" looks more discoverable.
2928

3029
**`get_*` tools didn't advertise their `list_*` partner.** When an agent had multiple
3130
IDs in hand and reached for `get_sales_order`, nothing in the docstring pointed them at
3231
`list_sales_orders(ids=[...])` for a single round-trip. PR #397 added the cross-
3332
reference; without a rule, future tool pairs won't get it.
3433

3534
**Help-resource drift.** `katana_mcp_server/src/katana_mcp/resources/help.py` mirrors
36-
the tool docstrings as a top-level resource. The PR #397 review caught it
37-
out-of-sync with the source-file docstrings — three workflow examples still used the
38-
old `{"sku": "WIDGET"}` shape after the schema collapse. Without a written sync
39-
expectation, this will recur on every tool change.
35+
the tool docstrings as a top-level resource. The PR #397 review caught it out-of-sync
36+
with the source-file docstrings — three workflow examples still used the old
37+
`{"sku": "WIDGET"}` shape after the schema collapse. Without a written sync expectation,
38+
this will recur on every tool change.
4039

4140
## Decision
4241

@@ -57,16 +56,16 @@ class CheckInventoryRequest(BaseModel):
5756
min_length=1,
5857
description=(
5958
"SKUs (strings) or variant IDs (integers) to check — mix freely. "
60-
"Pass one for a detailed stock card; pass many for a summary table. "
61-
"Batching N items in a single call beats N separate invocations. "
62-
"Output order matches input order."
59+
"Pass one for a detailed Prefab stock card; pass many for a JSON "
60+
"envelope of stock rows. Batching N items in a single call beats N "
61+
"separate invocations. Output order matches input order."
6362
),
6463
)
6564
```
6665

6766
The field is **required** (no `default_factory=list` — see PR #397 review) so the MCP
68-
schema and Pydantic validation agree the field is required. The implementation
69-
branches on `isinstance(item, str)` to dispatch.
67+
schema and Pydantic validation agree the field is required. The implementation branches
68+
on `isinstance(item, str)` to dispatch.
7069

7170
**Homogeneous batch on a `list_*` tool — pass `ids=[...]`.**
7271

@@ -78,20 +77,20 @@ class ListSalesOrdersRequest(BaseModel):
7877
)
7978
```
8079

81-
This is the cache-back pattern (PR #388 onward): every cache-backed `list_<entity>`
82-
tool exposes an `ids` filter that runs as `WHERE id IN (...)` against the typed
83-
cache. Agents holding a batch of IDs use the list tool instead of calling
84-
`get_<entity>` N times. **Exception:** `list_stock_transfers` does not yet expose
85-
`ids` (tracked as a follow-up — agents fall back to per-transfer
86-
`stock_transfer_number` filters until it lands). When adding `ids` to a list tool,
87-
update its docstring to the standard `ids=[1,2,3]` opening (see section 2).
80+
This is the cache-back pattern (PR #388 onward): every cache-backed `list_<entity>` tool
81+
exposes an `ids` filter that runs as `WHERE id IN (...)` against the typed cache. Agents
82+
holding a batch of IDs use the list tool instead of calling `get_<entity>` N times.
83+
**Exception:** `list_stock_transfers` does not yet expose `ids` (tracked as a follow-up
84+
— agents fall back to per-transfer `stock_transfer_number` filters until it lands). When
85+
adding `ids` to a list tool, update its docstring to the standard `ids=[1,2,3]` opening
86+
(see section 2).
8887

89-
**What we don't do: parallel singular + plural fields.** No `sku` + `skus`,
90-
`variant_id` + `variant_ids`, `customer_id` + `customer_ids` on the request schema.
88+
**What we don't do: parallel singular + plural fields.** No `sku` + `skus`, no
89+
`variant_id` + `variant_ids`, no `customer_id` + `customer_ids` on the request schema.
9190
The four-field shape is mutually-exclusive at runtime, opaque at the MCP-schema layer,
9291
and the source of the agent fallback that motivated PR #397. Always a single field —
93-
either a list (always — `min_length=1` enforces non-empty when the field is required)
94-
or a heterogeneous list with a discriminator type.
92+
either a list (always — `min_length=1` enforces non-empty when the field is required) or
93+
a heterogeneous list with a discriminator type.
9594

9695
### 2. Docstring opening-sentence pattern
9796

@@ -107,36 +106,34 @@ The em-dash (`—`, U+2014) separates the purpose clause from the batch hook. Th
107106
parenthetical notes the implementation (cache-backed, indexed SQL, etc.) when it
108107
materially affects the call shape.
109108

110-
For `list_*` tools that don't yet expose `ids` (currently only
111-
`list_stock_transfers`), the hook describes the value of getting multiple rows
112-
back — keeping the same em-dash shape so the convention is recognizable:
109+
For `list_*` tools that don't yet expose `ids` (currently only `list_stock_transfers`),
110+
the hook describes the value of getting multiple rows back — keeping the same em-dash
111+
shape so the convention is recognizable:
113112

114113
```
115114
List stock transfers with filters — returns multiple transfers for discovery or bulk review.
116115
```
117116

118-
This is a **documented exception**, not a target. Adding `ids` to
119-
`list_stock_transfers` (and migrating to the standard `ids=[1,2,3]` form) is
120-
out-of-scope for this ADR — file a feature issue if/when the migration is
121-
prioritised.
117+
This is a **documented exception**, not a target. Adding `ids` to `list_stock_transfers`
118+
(and migrating to the standard `ids=[1,2,3]` form) is out-of-scope for this ADR — file a
119+
feature issue if/when the migration is prioritised.
122120

123-
**`check_*` / `search_*` / single-item tools — short purpose line, then an
124-
input-shape paragraph that includes the em-dash batch hint.**
121+
**`check_*` / `search_*` / single-item tools — short purpose line, then an input-shape
122+
paragraph that includes the em-dash batch hint.**
125123

126124
```
127125
Check current stock levels for one or more SKUs or variant IDs.
128126
129127
Pass a list of SKUs (strings) or variant IDs (integers) — or mix both — to
130-
``skus_or_variant_ids``. A single item returns a rich stock card; multiple
131-
items return a summary table. Batching N checks in one call is faster than
132-
N separate invocations.
128+
``skus_or_variant_ids``. A single item returns a rich Prefab stock card;
129+
multiple items return a JSON envelope of stock rows (see ADR-0020 / #719).
130+
Batching N checks in one call is faster than N separate invocations.
133131
```
134132

135-
Two paragraphs: a short purpose line first (no em-dash on this line — it just
136-
states what the tool does), then the input-shape paragraph that uses the em-dash
137-
to introduce the batch hint and the field name. `get_*` tools follow the same
138-
two-paragraph shape but their input-shape paragraph is the cross-reference
139-
described in section 3.
133+
Two paragraphs: a short purpose line first (no em-dash on this line — it just states
134+
what the tool does), then the input-shape paragraph that uses the em-dash to introduce
135+
the batch hint and the field name. `get_*` tools follow the same two-paragraph shape but
136+
their input-shape paragraph is the cross-reference described in section 3.
140137

141138
### 3. `get_*``list_*` cross-references
142139

@@ -145,67 +142,67 @@ cross-reference as the second paragraph of its docstring:
145142

146143
```
147144
For multiple <entities> at once, use ``list_<entity>(ids=[...])`` —
148-
it returns a summary table and supports all the same filters.
145+
it returns a JSON envelope of rows and supports all the same filters.
149146
```
150147

151-
Use double-backticks (RST inline-literal form) for the call; em-dash
152-
continuation. The cross-reference lives *after* the first-line summary and
153-
*before* the parameters / returns sections.
148+
Use double-backticks (RST inline-literal form) for the call; em-dash continuation. The
149+
cross-reference lives *after* the first-line summary and *before* the parameters /
150+
returns sections.
154151

155152
When a `get_*` tool's per-call cost is non-trivial (e.g.,
156153
`get_manufacturing_order_recipe` fetches inline rows that aren't cached), the cross-
157-
reference also explains the cost trade-off: agents holding a batch of IDs should
158-
prefer the list tool's summary path unless they specifically need the rich detail.
154+
reference also explains the cost trade-off: agents holding a batch of IDs should prefer
155+
the list tool's summary path unless they specifically need the rich detail.
159156

160157
**Exception**`get_manufacturing_order_recipe` cross-references
161-
`get_manufacturing_order` (another `get_*` tool, not a `list_*`) because no
162-
`list_*` partner exists for inline recipe rows. The cross-reference still lives
163-
in the second paragraph; only the target tool differs from the standard form.
158+
`get_manufacturing_order` (another `get_*` tool, not a `list_*`) because no `list_*`
159+
partner exists for inline recipe rows. The cross-reference still lives in the second
160+
paragraph; only the target tool differs from the standard form.
164161

165162
### 4. Help-resource sync expectation
166163

167164
`katana_mcp_server/src/katana_mcp/resources/help.py` is a hardcoded mirror of the tool
168-
docstrings — it powers the `katana://help` resource that agents read during
169-
onboarding. When a tool docstring or request schema changes:
165+
docstrings — it powers the `katana://help` resource that agents read during onboarding.
166+
When a tool docstring or request schema changes:
170167

171-
- **Workflow examples** that reference the changed tool must be updated to use the
172-
new field names / signatures.
168+
- **Workflow examples** that reference the changed tool must be updated to use the new
169+
field names / signatures.
173170
- **Parameter sections** that enumerate the tool's fields must be re-aligned.
174171
- The change lands in the same PR as the tool change — never as a follow-up.
175172

176-
PR #397 review caught three stale `{"sku": "WIDGET"}` workflow examples after the
177-
schema collapse landed in the source-file docstrings; treat that as the canonical
178-
miss to avoid. The `pr-preparer` agent's project-specific PR-readiness check is
179-
expected to flag help-resource drift; if a future drift slips past it, fold the
180-
detection into the agent's brief.
173+
PR #397 review caught three stale `{"sku": "WIDGET"}` workflow examples after the schema
174+
collapse landed in the source-file docstrings; treat that as the canonical miss to
175+
avoid. The `pr-preparer` agent's project-specific PR-readiness check is expected to flag
176+
help-resource drift; if a future drift slips past it, fold the detection into the
177+
agent's brief.
181178

182179
## Consequences
183180

184181
### Positive
185182

186-
- Agents see batch capability in the first sentence, before deciding on N
187-
single-item calls.
183+
- Agents see batch capability in the first sentence, before deciding on N single-item
184+
calls.
188185
- Heterogeneous batch fields stop generating four-way fork choices for contributors
189186
adding new tools.
190-
- `get_*``list_*` partner discovery is part of the tool itself, not the agent's
191-
prior knowledge.
187+
- `get_*``list_*` partner discovery is part of the tool itself, not the agent's prior
188+
knowledge.
192189
- Help-resource drift is closed at PR review time, before it ships.
193190

194191
### Negative
195192

196-
- One more thing for new tools to comply with — easy to miss until a reviewer
197-
mentions it.
198-
- Existing tools that don't yet match the conventions look like outliers; back-fill
199-
is gradual, not all-at-once.
193+
- One more thing for new tools to comply with — easy to miss until a reviewer mentions
194+
it.
195+
- Existing tools that don't yet match the conventions look like outliers; back-fill is
196+
gradual, not all-at-once.
200197
- The "single field" rule means that adding a homogeneous batch shape to a tool that
201-
currently takes a single value is a breaking change to the request schema — see
202-
PR #397 for the precedent.
198+
currently takes a single value is a breaking change to the request schema — see PR
199+
#397 for the precedent.
203200

204201
### Neutral
205202

206-
- The conventions are codified in this ADR, not in lint rules. Future contributors
207-
rely on review and on the `code-modernizer` agent's brief; if drift recurs, escalate
208-
to a `code-modernizer` rewrite pass.
203+
- The conventions are codified in this ADR, not in lint rules. Future contributors rely
204+
on review and on the `code-modernizer` agent's brief; if drift recurs, escalate to a
205+
`code-modernizer` rewrite pass.
209206

210207
## Examples in tree
211208

@@ -214,12 +211,12 @@ The following tools follow all four conventions and serve as canonical reference
214211
- `check_inventory` (`katana_mcp_server/.../foundation/inventory.py`) — heterogeneous
215212
collapse (`skus_or_variant_ids: list[str | int]`), order-preserving impl,
216213
required-field semantics.
217-
- `list_sales_orders` (`.../foundation/sales_orders.py`) — first-line `ids=[...]`
218-
hook on a cache-backed list tool.
214+
- `list_sales_orders` (`.../foundation/sales_orders.py`) — first-line `ids=[...]` hook
215+
on a cache-backed list tool.
219216
- `get_sales_order` (same file) — second-paragraph cross-reference to
220217
`list_sales_orders(ids=[...])`.
221-
- `list_manufacturing_orders` / `get_manufacturing_order` — same pair pattern with
222-
the recipe-row caveat called out in `get_manufacturing_order_recipe`.
218+
- `list_manufacturing_orders` / `get_manufacturing_order` — same pair pattern with the
219+
recipe-row caveat called out in `get_manufacturing_order_recipe`.
223220

224221
When adding a new tool, copy the shape from one of these and adjust.
225222

@@ -228,36 +225,35 @@ When adding a new tool, copy the shape from one of these and adjust.
228225
### Alternative 1: Lint rule (ruff custom check / generated docstring linter)
229226

230227
Catch first-sentence drift, parallel singular+plural fields, and missing cross-
231-
references mechanically. **Rejected for now** — the conventions aren't yet stable
232-
enough across the surface area of the tool set, and a lint rule that fires on every
233-
new tool's first commit would generate more friction than value. Revisit if drift
234-
recurs after this ADR lands; the bar for upgrading is "we reverted to the four-field
235-
pattern in a real PR."
228+
references mechanically. **Rejected for now** — the conventions aren't yet stable enough
229+
across the surface area of the tool set, and a lint rule that fires on every new tool's
230+
first commit would generate more friction than value. Revisit if drift recurs after this
231+
ADR lands; the bar for upgrading is "we reverted to the four-field pattern in a real
232+
PR."
236233

237234
### Alternative 2: Tool-description templates / scaffolding
238235

239-
Add a `scripts/new_tool.py` that emits a docstring shell with the conventions baked
240-
in. **Rejected** — the surface area of differences between tools is too high for one
241-
scaffold to be useful (`list_*` vs. `check_*` vs. `get_*` vs. `create_*` /
242-
`update_*` / `delete_*` all have different opening shapes). Revisit when the
243-
write-tool conventions are also documented.
236+
Add a `scripts/new_tool.py` that emits a docstring shell with the conventions baked in.
237+
**Rejected** — the surface area of differences between tools is too high for one
238+
scaffold to be useful (`list_*` vs. `check_*` vs. `get_*` vs. `create_*` / `update_*` /
239+
`delete_*` all have different opening shapes). Revisit when the write-tool conventions
240+
are also documented.
244241

245242
### Alternative 3: Generate help.py from the tool docstrings
246243

247244
Eliminate the manual sync requirement (section 4) by extracting the workflow examples
248-
and parameter sections from the tool docstrings at build time. **Rejected for
249-
now**would require docstring sub-section markers (`:: Examples ::`,
250-
`:: Workflows ::`) that don't exist today, and the `katana://help` resource needs
251-
high-level workflow narrative that doesn't belong in any single tool's docstring.
252-
Track separately if the manual-sync bug recurs.
245+
and parameter sections from the tool docstrings at build time. **Rejected for now**
246+
would require docstring sub-section markers (`:: Examples ::`, `:: Workflows ::`) that
247+
don't exist today, and the `katana://help` resource needs high-level workflow narrative
248+
that doesn't belong in any single tool's docstring. Track separately if the manual-sync
249+
bug recurs.
253250

254251
## References
255252

256253
- PR #393 — surfaced batch shapes via first-line docstring updates (precursor)
257-
- PR #397 — schema collapse for `check_inventory`; reviewer flagged the missing
258-
ADR; this ADR is the deferred Phase 1 work
259-
- ADR-0017 — automated tool documentation (the layered docs strategy this ADR
260-
refines)
254+
- PR #397 — schema collapse for `check_inventory`; reviewer flagged the missing ADR;
255+
this ADR is the deferred Phase 1 work
256+
- ADR-0017 — automated tool documentation (the layered docs strategy this ADR refines)
261257
- ADR-0016 — tool interface pattern (the Annotated/Unpack/Pydantic shape these
262258
conventions live on top of)
263259
- Issue #398 — this ADR's tracking issue

katana_mcp_server/docs/adr/0020-consistent-tool-surface-and-cache-unification.md

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,23 @@ tracker), referenced from each umbrella issue.
207207
- **Bigger maintained surface.** Going from "modify is consistent" to "the whole surface
208208
is consistent" means more tools, more tests, more help-resource entries. The
209209
discipline is to keep them mechanically uniform — same param shapes (especially
210-
`ids: CoercedIntListOpt`, `format: "markdown" | "json"`, `limit` / `page` semantics),
211-
same response shapes, same empty-state UX.
210+
`ids: CoercedIntListOpt`, `limit` / `page` semantics), same response shapes, same
211+
empty-state UX. The `format: "markdown" | "json"` parameter that originally appeared
212+
here was removed in #719 — every tool now returns JSON-only content. When a response
213+
emits a Prefab card, `structured_content` carries the `PrefabApp` envelope (built via
214+
`make_tool_result(response, ui=app)`); when a response is data-only,
215+
`structured_content` carries the payload dict — typically built via
216+
`make_json_result(response)`, or an inline `ToolResult(...)` when the tool needs to
217+
thread request-driven kwargs through `model_dump_json` / `model_dump` (e.g.,
218+
`get_manufacturing_order` composes an `exclude={...}` selector + `exclude_none=True`
219+
from its include/verbose flags; the batch branches of `check_inventory` and
220+
`get_variant_details` build `ToolResult` inline to share the JSON content text with
221+
their single-item Prefab path). The `meta=UI_META` registration flag advertises that a
222+
tool *may* emit a Prefab card — not that it always does (e.g., `check_inventory` and
223+
`get_variant_details` attach Prefab on single-item requests and return a payload dict
224+
on batches). Promising a card via `meta=UI_META` and then never emitting one crashes
225+
MCP-Apps hosts looking for the widget they were advertised; the contract is the
226+
runtime payload, not the registration flag.
212227
- **Cache migration is a multi-week lift on the most-used cache entity.** Variants are
213228
touched by every `search_items` / `get_variant_details` / `check_inventory` call.
214229
Migration risk is real — incremental rollout per entity with fallback to legacy until

0 commit comments

Comments
 (0)