Skip to content

Commit 8664ea3

Browse files
bokelleyclaude
andauthored
perf(server): lazy-load Pydantic schema generation to fix storyboard readiness flake (#435)
* perf(server): lazy-load Pydantic outputSchema generation to fix storyboard readiness flake _generate_pydantic_schemas(), _generate_pydantic_output_schemas(), and _apply_pydantic_schemas() previously ran at module import time, causing heavy Pydantic type imports to race with the storyboard readiness probe and producing "Agent unreachable" failures across PRs #391, #405, #406, #407. Generation is now deferred to the first get_tools_for_handler() call (which fires during create_mcp_tools() at server construction, not at import time). _PYDANTIC_SCHEMAS and _PYDANTIC_OUTPUT_SCHEMAS start as empty dicts and are populated via .update() so external references stay valid. The _schemas_applied sentinel makes subsequent calls no-ops (~0ms overhead on the hot path). Import-time delta: ~4.5s of schema generation is moved from `import adcp.server` to the first `create_mcp_tools()` call. Tests updated: conftest.py gains a session-scoped autouse fixture that triggers lazy init before any test reads ADCP_TOOL_DEFINITIONS schema fields; stale "at import time" references in docstrings and error messages are updated. Closes #412 https://claude.ai/code/session_01NnoQN3c6Wi5LY5DEUBp8W2 * fixup: update stale 'at import time' docstrings and error messages Addresses pre-PR review findings: test_spec_coverage.py assertion message still referenced 'at import time', and _ensure_pydantic_schemas_applied docstring understated the in-place mutation and misdirected to get_tools_for_handler instead of create_mcp_tools. https://claude.ai/code/session_01NnoQN3c6Wi5LY5DEUBp8W2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c5c581d commit 8664ea3

4 files changed

Lines changed: 44 additions & 14 deletions

File tree

src/adcp/server/mcp_tools.py

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,8 +1224,8 @@ def _generate_pydantic_schemas() -> dict[str, dict[str, Any]]:
12241224
spec-accurate schemas with proper field types, descriptions,
12251225
required fields, and nested ``$defs``.
12261226
1227-
The result is applied to ``ADCP_TOOL_DEFINITIONS`` at import time
1228-
by :func:`_apply_pydantic_schemas`. Any tool whose generation
1227+
The result is applied to ``ADCP_TOOL_DEFINITIONS`` lazily on first
1228+
``tools/list`` call by :func:`_ensure_pydantic_schemas_applied`. Any tool whose generation
12291229
fails (or whose request model has no mapping here) silently keeps
12301230
its hand-crafted stub; ``tests/test_mcp_schema_drift.py`` guards
12311231
against that regression by asserting every tool has an entry here.
@@ -1558,9 +1558,12 @@ def _generate_pydantic_output_schemas() -> dict[str, dict[str, Any]]:
15581558
return schemas
15591559

15601560

1561-
# Generate schemas once at import time
1562-
_PYDANTIC_SCHEMAS = _generate_pydantic_schemas()
1563-
_PYDANTIC_OUTPUT_SCHEMAS = _generate_pydantic_output_schemas()
1561+
# Schemas are populated lazily on the first tools/list call to avoid
1562+
# heavy Pydantic type imports at module import time. Use .update() so
1563+
# external references bound before init (e.g. in tests) stay valid.
1564+
_PYDANTIC_SCHEMAS: dict[str, dict[str, Any]] = {}
1565+
_PYDANTIC_OUTPUT_SCHEMAS: dict[str, dict[str, Any]] = {}
1566+
_schemas_applied = False
15641567

15651568

15661569
def _apply_pydantic_schemas() -> None:
@@ -1580,7 +1583,23 @@ def _apply_pydantic_schemas() -> None:
15801583
tool_def["outputSchema"] = _PYDANTIC_OUTPUT_SCHEMAS[name]
15811584

15821585

1583-
_apply_pydantic_schemas()
1586+
def _ensure_pydantic_schemas_applied() -> None:
1587+
"""Lazily populate Pydantic schemas and apply them to tool definitions.
1588+
1589+
Mutates :data:`ADCP_TOOL_DEFINITIONS` in-place, replacing each tool's
1590+
``inputSchema`` with the Pydantic-generated schema and adding
1591+
``outputSchema``. Safe to call multiple times — subsequent calls are
1592+
no-ops. Called automatically by :func:`create_mcp_tools` /
1593+
:func:`get_tools_for_handler`; callers outside those paths (e.g. tests
1594+
or doc generators) must invoke this before reading schema fields.
1595+
"""
1596+
global _schemas_applied
1597+
if _schemas_applied:
1598+
return
1599+
_PYDANTIC_SCHEMAS.update(_generate_pydantic_schemas())
1600+
_PYDANTIC_OUTPUT_SCHEMAS.update(_generate_pydantic_output_schemas())
1601+
_apply_pydantic_schemas()
1602+
_schemas_applied = True
15841603

15851604

15861605
def _is_sdk_base_class(cls_name: str) -> bool:
@@ -1718,6 +1737,7 @@ def get_tools_for_handler(
17181737
Returns:
17191738
Filtered list of tool definitions.
17201739
"""
1740+
_ensure_pydantic_schemas_applied()
17211741
cls = handler if isinstance(handler, type) else type(handler)
17221742
instance = handler if not isinstance(handler, type) else None
17231743

tests/conftest.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@
2727
_INTEGRATION_DIR = (Path(__file__).parent / "integration").resolve()
2828

2929

30+
@pytest.fixture(autouse=True, scope="session")
31+
def _ensure_pydantic_schemas() -> None:
32+
"""Trigger lazy Pydantic schema init so ADCP_TOOL_DEFINITIONS has
33+
inputSchema/outputSchema populated for any test that reads them directly."""
34+
from adcp.server.mcp_tools import _ensure_pydantic_schemas_applied
35+
36+
_ensure_pydantic_schemas_applied()
37+
38+
3039
def _is_integration_test(request: pytest.FixtureRequest) -> bool:
3140
"""Is this test under ``tests/integration/``?
3241

tests/test_mcp_schema_drift.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
33
The MCP tool registry exposes ``inputSchema`` for every ADCP tool via
44
``tools/list``. These schemas are auto-generated from the corresponding
5-
Pydantic request models in ``adcp.types`` at import time
6-
(:func:`adcp.server.mcp_tools._generate_pydantic_schemas`).
5+
Pydantic request models in ``adcp.types`` on first ``tools/list`` call
6+
(:func:`adcp.server.mcp_tools._ensure_pydantic_schemas_applied`).
77
88
This module protects the generation path from regressions:
99
@@ -59,7 +59,7 @@ def test_input_schemas_match_pydantic_generation() -> None:
5959

6060
assert not mismatches, (
6161
"ADCP_TOOL_DEFINITIONS has stale inputSchemas — "
62-
"`_apply_pydantic_schemas()` must run at import time:\n"
62+
"call `_ensure_pydantic_schemas_applied()` (or `create_mcp_tools()`) first:\n"
6363
+ "\n".join(f" - {name}" for name in mismatches)
6464
)
6565

tests/test_spec_coverage.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ class MyGovernanceAgent(GovernanceHandler):
173173
def test_mcp_tool_input_schema_matches_pydantic_models():
174174
"""MCP tool inputSchemas are generated from Pydantic request models.
175175
176-
The ``ADCP_TOOL_DEFINITIONS[*].inputSchema`` is overwritten at import
177-
time by ``_apply_pydantic_schemas()`` with the output of
176+
The ``ADCP_TOOL_DEFINITIONS[*].inputSchema`` is overwritten on first
177+
``tools/list`` call by ``_ensure_pydantic_schemas_applied()`` with the output of
178178
``model_json_schema()`` on the corresponding ``<ToolName>Request``
179179
model. This test is a coarse guard that every tool with a mapped
180180
request model carries a schema advertising every field of that
@@ -213,8 +213,9 @@ def test_mcp_tool_input_schema_matches_pydantic_models():
213213

214214
assert drift == [], (
215215
"MCP tool inputSchema fields have drifted from Pydantic models.\n"
216-
"The inputSchema is auto-generated from the request model at\n"
217-
"import time; this drift shouldn't be possible unless schema\n"
218-
"generation is broken. See tests/test_mcp_schema_drift.py.\n"
216+
"The inputSchema is auto-generated from the request model on first\n"
217+
"tools/list call (_ensure_pydantic_schemas_applied()); this drift\n"
218+
"shouldn't be possible unless schema generation is broken.\n"
219+
"See tests/test_mcp_schema_drift.py.\n"
219220
+ "\n".join(f" - {d}" for d in drift)
220221
)

0 commit comments

Comments
 (0)