Skip to content

Commit 48cfa71

Browse files
dkackmanclaude
andcommitted
test(mcp): fixed surface ceiling, guide-pointer resolution test, skills under their cap
SURFACE_BUDGET was a ratchet raised to each measurement plus <1 token, so every docstring change paid for itself by trimming another tool's text, and at least once the trim took a pointer agents needed (#574 re-broke #282). It is now a fixed 16_000 with a stated policy: move narrative to a guide and leave a pointer before raising it. tests/test_guide_pointers.py checks that every guide pointer in the MCP instructions, tool descriptions, skills and references opens a real section, and that the tools pointing at a guide today keep their pointer. The three skills within ~100 bytes of the 12 KiB cap move situational detail into references, linked from SKILL.md with when to read them: minimax-h3 staging rules, minimax-music3 memory check, series-episodes cross-episode loudness. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 19474c0 commit 48cfa71

10 files changed

Lines changed: 256 additions & 208 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ one to open.
178178
| `dw_mcp` stays torch-free | `dw_mcp/` | `dw_mcp` reaches `dw.serve` over HTTP and imports no `dw` module, because `dw/__init__.py` pulls in torch. | `tests/test_mcp_server.py::TestStartupWeight::test_the_server_starts_without_importing_the_engine` |
179179
| Tool surface | `dw_mcp/server.py`, `dw_mcp/tools_*.py` | Only these modules import the MCP SDK, and each tool body is a one-line call into a handler. | `tests/test_mcp_server.py::test_the_wiring_table_covers_every_registered_tool`, `tests/test_mcp_server.py::test_the_stated_tool_count_is_the_registered_one` |
180180
| Surface text budget | the tool docstrings in `dw_mcp/tools_*.py`, the instructions in `dw_mcp/server.py` | The instructions and each tool description stay at or under 2,048 characters, and the whole surface stays within its token budget. | `tests/test_mcp_server.py`: `SURFACE_BUDGET`, `CLIENT_TEXT_LIMIT`; `tests/test_mcp_server.py::test_no_text_the_agent_reads_is_cut_off_by_the_client`, `tests/test_mcp_server.py::test_the_tool_surface_fits_the_budget` |
181+
| Guide pointers in resident text | the tool docstrings in `dw_mcp/tools_*.py`, the instructions in `dw_mcp/server.py`, each plugin `SKILL.md` and its `references/` | Every `get_guide(...)` / guide-section pointer opens a real section through `dw/server/guides.py`'s `get_guide`, and the tools that point at a guide keep their pointer when text is trimmed to fit the budget. | `tests/test_guide_pointers.py` (`TOOLS_THAT_POINT`) |
181182
| Spending needs consent | each handler behind a tool that takes `acknowledged_cost` (`dw_mcp/diagnose.py`, `models.py`, `prompts.py`, `workspaces.py`) | Every tool that takes `acknowledged_cost` refuses until it is set; `delete_workspace`'s refusal is the server's 409. | `tests/test_mcp_twins.py::test_every_tool_that_takes_acknowledged_cost_refuses_without_it` |
182183
| Copies of engine rules | the constants and word lists in `dw_mcp/` and `dw/run.py` | `dw_mcp` cannot import its owners, so each copy equals its owner; a copy that needs no twin is deleted, not pinned. | `tests/test_mcp_twins.py` |
183184
| The CLI is a client of the server | `dw/run.py` | `dw.run` queues on `dw.serve` through `dw_mcp.client`, so `dw` imports `dw_mcp` and never the reverse. | `tests/test_mcp_server.py::TestStartupWeight::test_the_server_starts_without_importing_the_engine`, `tests/test_mcp_twins.py` |

‎dw_mcp/CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ Guidance for the `dw_mcp/` stdio MCP server package. What it covers and how its
44

55
## Writing text an agent reads
66

7-
- A tool description (the docstring in `dw_mcp/tools_*.py`) or the instructions in `dw_mcp/server.py` must put what an agent has to act on first, and point at a guide section rather than restate it. `tests/test_mcp_server.py` pins the length (`CLIENT_TEXT_LIMIT`, `SURFACE_BUDGET`) but not the ordering, and the instructions and `list_workflows` sit within a few dozen characters of the limit.
7+
- A tool description (the docstring in `dw_mcp/tools_*.py`) or the instructions in `dw_mcp/server.py` must put what an agent has to act on first, and point at a guide section rather than restate it. `tests/test_mcp_server.py` pins the length (`CLIENT_TEXT_LIMIT`, `SURFACE_BUDGET`) but not the ordering, and the instructions and `list_workflows` sit within a few dozen characters of the limit. `SURFACE_BUDGET` is a fixed ceiling with headroom, not a ratchet: when it fails, move narrative to a guide and leave a pointer before raising it. `tests/test_guide_pointers.py` checks every pointer resolves and that a tool which points at a guide keeps its pointer.
88
- `MAX_WAIT_SECONDS` (`dw_mcp/diagnose.py`) is interpolated into the tool descriptions, so no doc or skill quotes the number.

‎plugins/dw/skills/minimax-h3/SKILL.md‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,9 @@ section).
100100
`validate_workflow` refuses it and warns on a `weight_name` naming neither.
101101
- Smeary faces (int8 base) or draft -> final tiers: `references/quality-and-speed.md`.
102102
- Backend: default `sparse` (dense off CUDA); same one at draft and final.
103-
- Fit a crowd's action to what it holds: candle-holders asked to clap
104-
rendered three-handed people.
105-
- A directed crowd move needs its landmark in frame: "marches up to the
106-
house" put it behind camera. Track from the side, landmark ahead.
107-
- Two people in frame can lip-sync the wrong one, seemingly by whichever
108-
voice enters first: credit a shout to the crowd, spell the sung line in
109-
`<d>` tags, frame the singer face-on at medium close-up.
110-
- State a crowd's age range; unstated skews young.
111-
- A three-reference shot can open on the portraits' backdrop
112-
for ~1.5s (about 1 in 25): check a shot's first frames and say in
113-
`subject_definitions` that the portrait's backdrop, pose and framing are
114-
not reused.
103+
- A crowd, two people in frame, or a three-reference shot: read
104+
`references/staging.md` before the prompt - who lip-syncs, where the
105+
landmark is, and the portrait backdrop that can open a shot.
115106
- Ref2VA reaches 1344x768 too (trade-off: the `recipes` guide's MiniMax-H3
116107
section). There the 24 GB ceiling on the `17n+5` grid drops with each
117108
reference: 243 frames at one, 209 at two, 175 at three, 141 at four. `validate_workflow` refuses
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# MiniMax H3: staging crowds, two people, and reference shots
2+
3+
Part of the `minimax-h3` skill; its *Hard rules* say when to read this. Each
4+
rule came from a render that went wrong on our server (#484).
5+
6+
- Fit a crowd's action to what it holds: candle-holders asked to clap
7+
rendered three-handed people.
8+
- A directed crowd move needs its landmark in frame: "marches up to the
9+
house" put it behind camera. Track from the side, landmark ahead.
10+
- Two people in frame can lip-sync the wrong one, seemingly by whichever
11+
voice enters first: credit a shout to the crowd, spell the sung line in
12+
`<d>` tags, frame the singer face-on at medium close-up.
13+
- State a crowd's age range; unstated skews young.
14+
- A three-reference shot can open on the portraits' backdrop
15+
for ~1.5s (about 1 in 25): check a shot's first frames and say in
16+
`subject_definitions` that the portrait's backdrop, pose and framing are
17+
not reused.

‎plugins/dw/skills/minimax-music3/SKILL.md‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,10 @@ shapes; do not author a new workflow until the shape decision below fails.
1818
2. `list_workflows(shape="audio")`, and `shape="sequence"` for the music video.
1919
Trust the names below only after the listing confirms them.
2020
3. `get_workflow` on the one chosen, for its variables and defaults.
21-
4. Before a long track or the music video - anything near the
22-
card's ceiling - `get_memory` with the server idle and read a `live: true`
23-
reading's `gpu_memory_allocated_mb`. Only a live reading is the worker's own
24-
and only live readings compare: `info: null` means nothing
25-
is resident, go ahead, while a `live: false` reading with a populated `info`
26-
is cached from another moment - it reads low while a job is loading a model,
27-
so ask again once idle rather than trusting it. A non-trivial
28-
idle figure is what an earlier run left behind, coming off the ~22 GB
29-
these templates need. With the server idle, `clear_memory` clears it
30-
(refused while a job is queued or running) - it also drops the step cache,
31-
so the next run, including a seeded rerun, is cold and regenerates. Re-read
32-
`get_memory` afterwards to confirm. Don't retry into a failed attempt
33-
without clearing first - a failed attempt is what leaves weight
34-
resident, so an immediate retry starts from less than the attempt that
35-
failed had.
21+
4. Before a long track or the music video - anything near the card's
22+
ceiling, the ~22 GB these templates need - read `references/memory.md`:
23+
which `get_memory` reading to trust, when `clear_memory` first, and why
24+
never to retry into a failed attempt without it.
3625

3726
## Which shape is the request
3827

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# MiniMax Music 3: checking memory before a long run
2+
3+
Part of the `minimax-music3` skill; step 4 of *Before anything* says when to
4+
read this - before a long track or the music video, anything near the card's
5+
ceiling.
6+
7+
`get_memory` with the server idle, and read a `live: true` reading's
8+
`gpu_memory_allocated_mb`. Only a live reading is the worker's own and only
9+
live readings compare:
10+
11+
- `info: null` means nothing is resident: go ahead.
12+
- A `live: false` reading with a populated `info` is cached from another
13+
moment. It reads low while a job is loading a model, so ask again once idle
14+
rather than trusting it.
15+
- A non-trivial idle figure is what an earlier run left behind, coming off
16+
the ~22 GB these templates need. With the server idle, `clear_memory`
17+
clears it (refused while a job is queued or running). It also drops the
18+
step cache, so the next run, including a seeded rerun, is cold and
19+
regenerates. Re-read `get_memory` afterwards to confirm.
20+
21+
Don't retry into a failed attempt without clearing first: a failed attempt is
22+
what leaves weight resident, so an immediate retry starts from less than the
23+
attempt that failed had.

‎plugins/dw/skills/series-episodes/SKILL.md‎

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -117,26 +117,11 @@ composes into, not something to re-derive:
117117
applied before the score is passed in.
118118
- **normalize**: the mixed world sound and score are normalized together
119119
(`assemble-and-score`'s `balanced` step, -3 dBFS peak ceiling, always
120-
enforced) so one episode is not louder than the next. A shared peak
121-
ceiling does not mean a shared loudness - a sparse, dialogue-only episode
122-
and a dense, score-heavy one can both sit at -3 dBFS peak and still read
123-
as very different volumes. `assemble-and-score`'s `target_lufs` variable
124-
(passed through to `normalize_audio`) gains the mix toward a measured
125-
loudness before the ceiling is applied - but gain down succeeds only
126-
while the mix's own peak is below the ceiling; a single loud peak (a
127-
laugh track, a sting) caps gain up the same way, and can pull a gain-down
128-
episode lower still. Match episodes **downward** anyway: pick one series `target_lufs`
129-
that every episode can reach - in practice, about the quietest episode
130-
the ceiling holds back - and pass that same value on every episode's
131-
`assemble-and-score` run. A `target_lufs_capped` warning on an
132-
episode means that episode's own peak sets the series' ceiling; lower the
133-
series `target_lufs` rather than accepting the mismatch. Only when the
134-
series should sit louder than its most dynamic episode allows (e.g. -16
135-
for streaming) pass `limit: true` beside the same `target_lufs` on every
136-
episode: -3 dBFS becomes a true-peak ceiling a look-ahead limiter holds,
137-
so the laugh is limited rather than setting the gain. A `limiter_heavy`
138-
warning on an episode means the series target is squashing it audibly -
139-
lower the series `target_lufs` rather than living with it.
120+
enforced), but a shared peak is not a shared loudness. Pass one series
121+
`target_lufs` on every episode, matched **downward** to what the quietest
122+
episode can reach; before episode 1, read `references/loudness.md` for how
123+
to pick it and what `target_lufs_capped`, `limit: true` and
124+
`limiter_heavy` mean.
140125
- **pair**: the normalized track is muxed onto the cut - the episode's
141126
deliverable.
142127

@@ -171,16 +156,9 @@ does, `keep_output` it and reference the resulting `asset:` instead.
171156
Beyond `minimax-h3`'s own judging checklist (a character that changes
172157
between shots, a portrait imposing its framing, a voice without affect):
173158
watch two episodes back to back and check the cast reads as the same
174-
people - the failure mode step 0 exists to prevent. `get_gallery_metadata`
175-
on each episode's final file for duration and loudness, so a level
176-
mismatch between episodes shows up before a viewer notices it - its
177-
`peak_dbfs` is a single sample and does not say how loud the episode reads
178-
as a whole; `integrated_lufs` (BS.1770, whole-track) is the field that
179-
answers that, and is what to compare across episodes. A peak-normalized
180-
episode with one loud outlier (a studio-audience laugh, a sting) can sit
181-
1-2 LU quieter than its neighbors even at the same -3 dBFS peak ceiling -
182-
that gap is `target_lufs`'s to close, matched downward (the **normalize**
183-
bullet; `limit: true` for a series that must sit louder). To confirm every
159+
people - the failure mode step 0 exists to prevent. Compare episodes'
160+
`integrated_lufs` (`get_gallery_metadata`), not `peak_dbfs`; the second
161+
half of `references/loudness.md` reads the gap. To confirm every
184162
line rendered rather than judging it by ear, run `templates/check-script`
185163
against the episode's script: `validate_workflow(name="templates/check-script",
186164
arguments={"input_audio": "output:<name>", "lines": [...]})` first (free; it
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# Series episodes: one loudness across episodes
2+
3+
Part of the `series-episodes` skill; its **normalize** beat and *Run and
4+
judge* say when to read this - picking the series `target_lufs` before
5+
episode 1, and comparing finished episodes.
6+
7+
## Picking the series target
8+
9+
`assemble-and-score`'s `balanced` step normalizes the mixed world sound and
10+
score together to a -3 dBFS peak ceiling, always enforced. A shared peak
11+
ceiling does not mean a shared loudness: a sparse, dialogue-only episode and
12+
a dense, score-heavy one can both sit at -3 dBFS peak and still read as very
13+
different volumes.
14+
15+
`assemble-and-score`'s `target_lufs` variable (passed through to
16+
`normalize_audio`) gains the mix toward a measured loudness before the
17+
ceiling is applied. Gain down succeeds only while the mix's own peak is below
18+
the ceiling; a single loud peak (a laugh track, a sting) caps gain up the
19+
same way, and can pull a gain-down episode lower still.
20+
21+
Match episodes **downward** anyway: pick one series `target_lufs` that every
22+
episode can reach - in practice, about the quietest episode the ceiling holds
23+
back - and pass that same value on every episode's `assemble-and-score` run.
24+
25+
- A `target_lufs_capped` warning on an episode means that episode's own peak
26+
sets the series' ceiling; lower the series `target_lufs` rather than
27+
accepting the mismatch.
28+
- Only when the series should sit louder than its most dynamic episode allows
29+
(e.g. -16 for streaming), pass `limit: true` beside the same `target_lufs`
30+
on every episode: -3 dBFS becomes a true-peak ceiling a look-ahead limiter
31+
holds, so the laugh is limited rather than setting the gain.
32+
- A `limiter_heavy` warning on an episode means the series target is
33+
squashing it audibly - lower the series `target_lufs` rather than living
34+
with it.
35+
36+
## Comparing finished episodes
37+
38+
`get_gallery_metadata` on each episode's final file, for duration and
39+
loudness, so a level mismatch between episodes shows up before a viewer
40+
notices it. Its `peak_dbfs` is a single sample and does not say how loud the
41+
episode reads as a whole; `integrated_lufs` (BS.1770, whole-track) is the
42+
field that answers that, and is what to compare across episodes.
43+
44+
A peak-normalized episode with one loud outlier (a studio-audience laugh, a
45+
sting) can sit 1-2 LU quieter than its neighbors even at the same -3 dBFS
46+
peak ceiling. That gap is `target_lufs`'s to close, matched downward as
47+
above; `limit: true` for a series that must sit louder.

‎tests/test_guide_pointers.py‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
"""Every pointer an agent is handed to a guide section opens that section.
2+
3+
The resident text - the MCP server's instructions and tool descriptions, and
4+
each plugin skill's SKILL.md - says what a tool or a family does and points
5+
at a guide for the rest. A pointer is the whole of the discovery path: a
6+
section renamed out from under one, or a pointer trimmed away to fit a
7+
budget (#574 re-broke #282 that way), leaves an agent holding no way to the
8+
detail. So every pointer must resolve through the same `get_guide` the
9+
server answers with, and the tools that carry one now must keep it.
10+
"""
11+
12+
import asyncio
13+
import glob
14+
import os
15+
import re
16+
17+
import pytest
18+
19+
from dw.server import guides
20+
from tests.test_examples import REPO_ROOT
21+
22+
pytest.importorskip("mcp", reason="the mcp extra is not installed")
23+
24+
from tests.test_mcp_server import ok, server_over, tools_of # noqa: E402
25+
26+
SKILL_TEXTS = sorted(
27+
glob.glob(os.path.join(REPO_ROOT, "plugins", "dw", "skills", "*", "SKILL.md"))
28+
+ glob.glob(
29+
os.path.join(REPO_ROOT, "plugins", "dw", "skills", "*", "references", "*.md")
30+
)
31+
)
32+
33+
# The guide a file name stands for, for the "WORKFLOW_GUIDE's ..." form.
34+
_BY_FILE = {
35+
file_name.removesuffix(".md"): name
36+
for name, (file_name, _summary) in guides.GUIDES.items()
37+
}
38+
39+
# The forms a pointer is written in, with whitespace already collapsed:
40+
# get_guide("workflows", section="The loop") get_guide("loras")
41+
# the `acceleration` guide, section "Reading Memory While Offloading"
42+
# WORKFLOW_GUIDE's "The loop"
43+
_CALL = re.compile(
44+
r'get_guide\(\s*"(?P<name>[a-z-]+)"(?:\s*,\s*section\s*=\s*"(?P<section>[^"]+)")?\s*\)'
45+
)
46+
_PROSE = re.compile(r'the `(?P<name>[a-z-]+)` guide, section "(?P<section>[^"]+)"')
47+
_FILE = re.compile(r'(?P<file>[A-Z0-9_]+)\'s "(?P<section>[^"]+)"')
48+
49+
# The tools whose description points at a guide today. A pointer trimmed
50+
# away to fit the surface budget fails here rather than leaving the tool
51+
# with no way to its detail; adding one to a tool means adding it here.
52+
TOOLS_THAT_POINT = {
53+
"get_job",
54+
"get_memory",
55+
"get_output_audio",
56+
"save_lora",
57+
"validate_workflow",
58+
"wait_for_job",
59+
}
60+
61+
62+
def pointers(text):
63+
"""(guide name, section or None) for each pointer in `text`."""
64+
flat = " ".join(text.split())
65+
found = [(m["name"], m["section"]) for m in _CALL.finditer(flat)]
66+
found += [(m["name"], m["section"]) for m in _PROSE.finditer(flat)]
67+
found += [
68+
(_BY_FILE[m["file"]], m["section"])
69+
for m in _FILE.finditer(flat)
70+
if m["file"] in _BY_FILE
71+
]
72+
return found
73+
74+
75+
def unresolved(text):
76+
"""The pointers in `text` that `get_guide` refuses."""
77+
bad = []
78+
for name, section in pointers(text):
79+
try:
80+
guides.get_guide(name, section)
81+
except (guides.GuideError, KeyError) as error:
82+
bad.append((name, section, str(error)[:120]))
83+
return bad
84+
85+
86+
def mcp_texts():
87+
server = server_over(ok({}))
88+
tools = asyncio.run(tools_of(server))
89+
texts = {name: tool.description or "" for name, tool in tools.items()}
90+
texts["<instructions>"] = server.instructions
91+
return texts
92+
93+
94+
def test_every_pointer_in_the_mcp_surface_resolves():
95+
bad = {name: unresolved(text) for name, text in mcp_texts().items()}
96+
assert not {name: b for name, b in bad.items() if b}
97+
98+
99+
def test_the_tools_that_point_at_a_guide_still_do():
100+
texts = mcp_texts()
101+
lost = sorted(name for name in TOOLS_THAT_POINT if not pointers(texts[name]))
102+
assert not lost, f"these tools lost their guide pointer: {lost}"
103+
104+
105+
def test_the_server_instructions_point_at_the_authoring_section():
106+
assert ("workflows", "Authoring a workflow from an agent") in pointers(
107+
mcp_texts()["<instructions>"]
108+
)
109+
110+
111+
@pytest.mark.parametrize(
112+
"path", SKILL_TEXTS, ids=lambda p: os.path.relpath(p, REPO_ROOT)
113+
)
114+
def test_every_pointer_in_a_skill_resolves(path):
115+
with open(path, encoding="utf-8") as handle:
116+
assert not unresolved(handle.read())
117+
118+
119+
def test_the_pointer_forms_are_recognised():
120+
"""The extraction itself, so a form it stops matching cannot pass the
121+
resolution tests by finding nothing."""
122+
text = (
123+
'see get_guide("workflows",\n section="The loop") and the '
124+
'`acceleration` guide, section "Caching", and WORKFLOW_GUIDE\'s '
125+
'"References", and get_guide("loras")'
126+
)
127+
assert pointers(text) == [
128+
("workflows", "The loop"),
129+
("loras", None),
130+
("acceleration", "Caching"),
131+
("workflows", "References"),
132+
]

0 commit comments

Comments
 (0)