[Oaklight]

Workstream 2 of #101: the OpenAPI adapter no longer builds Pydantic models.

What changed

• Vendored httpserver.py (0.6.1) and jsonschema.py (0.4.1) via the zerodep CLI; zerodep.lock.json updated. Both import cleanly.
• Request validation (_validate_arguments): signature-derived tools go through Tool.validate_parameters() (coerces and drops undeclared keys exactly as the tool does when called directly); proxy tools (MCP / OpenAPI / LangChain, no parameters_model) validate against route.parameters_schema with the vendored jsonschema.schema_validate. Failures return the existing 422 {"detail": [{"loc": [...], "msg": ..., "type": ...}]} envelope.
• FastAPI still routes (the app port is workstream 4), but endpoints receive the decoded body as Annotated[Any, Body()] and publish route.parameters_schema through openapi_extra["requestBody"], so /openapi.json keeps the same request bodies without a generated model.
• Removed _schema_to_pydantic, _resolve_type, _JSON_TYPE_MAP and the __annotations__ patching; the adapter no longer imports pydantic.

Tests

• TestResolveType / TestSchemaToPydantic are replaced by TestValidateArguments: signature coercion and unknown-key dropping, a 422 for a bad value, and a proxy tool validated by its own schema (unknown keys filtered).
• Full suite: 359 passed, 8 skipped; the one failure (test_discover_tools_returns_deferred_schema) is pre-existing in this environment (llm_rosetta not installed) and fails identically on master.
• The FastAPI↔builder parity test still passes, including request bodies.

Remaining in the epic

[openapi] still installs FastAPI (workstream 4 ports the app; workstream 3 wires the builder into /openapi.json), then packaging/docs.

[elena-oaklight]

Approved. I checked the premise three ways (no model construction, validation still happens, and it is the vendored validator doing it) and then compared the HTTP behaviour against master request by request.

What changed structurally

• adapters/openapi/adapter.py has no create_model, no pydantic import and no BaseModel left; validation goes through the core tool's own schema.
• _vendor/jsonschema.py (0.4.1) imports nothing outside the standard library - copy, warnings, __future__ - so the vendored copy really is self-contained.
• Wheel builds and contains 5 vendored modules.

Behaviour, master vs this head

Same registry (int signature tool, optional-args tool, raising tool), same requests, TestClient:

case              master                                  this head
valid 1,2         200  3                                  200  3
missing_b         422  type=missing      + input           422  type=value_error  "Missing required field 'b' at '$'"
wrong_type "x"    422  type=int_parsing  "Input should be a
                        valid integer..."                   422  type=value_error  "Expected int at 'a', got str"
extra_field       200  3 (dropped)                          200  3 (dropped)
string "1","2"    200  3 (coerced)                          200  3 (coerced)
null              422  type=int_type                       422  type=value_error
bool for int      200  (True -> 1)                           422  "Expected int at 'a', got bool"
optional {}       200  "1|x|False"                         200  "1|x|False"
optional {a:5}    200  "5|x|False"                         200  "5|x|False"
optional {b:3}    422                                      422
raising tool      500  "Tool 'boom' execution failed: kaboom"  500  same
unknown tool      404                                      404

So status codes, coercion, unknown-key dropping and the 500/404 paths are unchanged; the error body changed shape: detail[].type is now always value_error, msg is core's prose, and input is gone. That is defensible - and the OpenAPI 422 schema becomes accurate for the first time, see #124 - but it is a client-visible change, so it should be in the changelog. Two specifics worth your judgement:

1. Anything that maps pydantic codes (missing, int_parsing, int_type, string_type) to its own message now sees value_error. Nothing in the tests pins type, so this would regress silently later.
2. {"a": true} is now 422 where master accepted it as 1. That is closer to the declared "type": "integer" (JSON Schema does not treat true as an integer), so I would keep it - just be aware it is a tightening nobody asked for.

Schema surface

/openapi.json request bodies are now inline schemas instead of $refs to AddRequest/OptRequest/BoomRequest (components 5 -> 2). Valid, and one less layer of pydantic, but code generators will rename those models.

Why it is worth it, measured

200 signature tools, same venv:

                route_table    create app    first /openapi.json
master                0.4 ms     405.4 ms              468.5 ms
this head             0.4 ms     306.9 ms              161.5 ms

-24% on construction and -66% on the first schema, which is the cost of building 200 pydantic models that nothing then validated against.

Notes

• _vendor/httpserver.py (2,132 lines, 0.6.1) is not imported anywhere - grep -rn httpserver over the tree hits only the module itself and the lock entry, which marks it "requested": true. The wheel ships it (78 KB of the 109 KB wheel before compression). If it is groundwork for a later workstream, fine, but as it stands it is dead weight that zerodep outdated will keep updating. Either wire it in here or land it with the PR that uses it.
• No changelog entry in [Unreleased] on either docs_en or docs_zh for this workstream. The dropped pydantic model step and the 422 body change are both user-visible, so 0.8.0 will need them, in both languages.

[milo-oaklight]

Behaviour change worth confirming: for a proxy tool whose schema says additionalProperties: false, extra keys are now rejected instead of dropped.

proxy tool, schema {q: string, required [q], additionalProperties false}
body {"q": "x", "z": 1}
  0.7.0 -> 200 {"q": "x"}      (pydantic model had extra=ignore, then filtered)
  head  -> 422 Additional property 'z' is not allowed

schema_validate runs before the filter below, so the filter can no longer rescue that case. This may be the behaviour you want — additionalProperties: false arguably should reject — but it is a client-visible change for anything that posts extra keys to an MCP/OpenAPI proxy tool, so it belongs in the changelog either way. Note the asymmetry it leaves: a schema that simply omits additionalProperties still drops extras silently, because not schema.get(...) is true.

[milo-oaklight]

I ran the old and the new adapter side by side — same registry, same RouteTable, same request bodies — instead of reading the diff. Most of it is equivalent, but there is one regression I would not merge as-is.

Blocking: a non-object body now returns 500 instead of 422. Inline comment on line 42 has the detail. Because data: Annotated[Any, Body()] no longer constrains the body, _strip_framework_fields receives whatever JSON the client sent and calls .items() on it. Measured on the same registry:

body           0.7.0               head
[]             422 model_attributes_type   500
"str"          422                 500
null           422                 500
123            422                 500
form post      422                 500

The pydantic models were what produced all of those 422s, and nothing replaced that check. A three-line guard restores it.

Everything else I compared matches. Sixteen request shapes across int/float/bool/Literal/list[int]/optional-with-default/nested-dict tools: status codes identical everywhere else, including coercion ("2" → 2), unknown-key dropping, toolcall_reason stripping, **kwargs passthrough, and defaults.

The vendored jsonschema holds up. Eight targeted cases on a schema with enum + nested object + array-of-objects + anyOf: all correctly accepted or rejected, with paths like nested.k and rows[0].v. No false accepts.

Suite: 366 passed / 2 skipped, against 380 / 2 on 0.7.0. Eighteen adapter tests were removed and three added, so the count drop is expected — but the removed ones were the type-mapping coverage (enum, array with items, anyOf, empty schema, additionalProperties). I re-probed those shapes and they behave, so this is a coverage gap rather than a behaviour gap; a couple of table-driven cases would keep it closed.

Two smaller things, non-blocking:

1. The 422 detail vocabulary changed. Signature-tool failures now report type: "value_error" with core's wording; proxy-tool failures report type: "required" / "type" / "additionalProperties" from the vendored validator. input is gone, and array element locations are ["body", "xs[0]"] where pydantic produced ["body", "xs", 0]. Anyone branching on type or indexing loc sees a different contract — one changelog line would save them the debugging.
2. _vendor/httpserver.py is 76 KB / 2132 lines and has zero references anywhere in src/, tests/, docs or pyproject.toml. If it is groundwork for a later workstream that is fine, but it ships in the wheel from this PR on and has to be kept in step with upstream. Just confirming it is intentional.

Requesting changes for the 500; everything else here is a question or a note.

[clementine-oaklight]

The architecture is right - pydantic models out of the request path, validation through the tool's own contract, vendored modules instead of framework magic. But the request matrix shows one behaviour regression that should be fixed before this lands: a non-object JSON body now returns 500 where it used to return 422.

Same app, same requests, base (26cd0fc) vs this head (407394bd):

case                base                                   head
body-array         422 model_attributes_type             500 Internal Server Error
body-string        422 model_attributes_type             500 Internal Server Error
body-number        422 model_attributes_type             500 Internal Server Error
body-null          422 missing                           422 missing
no-body            422 missing                           422 missing

Root cause is a one-liner: Annotated[Any, Body()] accepts any JSON, then _strip_framework_fields() calls raw.items(). AttributeError: 'list' object has no attribute 'items' -> _wrap_tool_error-less path -> 500. A malformed client request now looks like a server fault, which is the one thing a 4xx/5xx split exists to prevent (clients retry 500s, alerting pages on them).

I applied the guard locally and re-ran the same matrix:

if not isinstance(raw, dict):
    raise HTTPException(status_code=422, detail=[{
        "loc": ["body"],
        "msg": "Input should be a valid dictionary or object to extract fields from",
        "type": "model_attributes_type",
    }])

Result: all three cases back to 422 with byte-identical msg/type to the base (Input should be a valid dictionary or object to extract fields from / model_attributes_type), and every other case unchanged. Inline comment has the detail.

What I verified as correct (so the rest of the review is not just the bug):

• Coercion and extra-key filtering match. {"a": "5", "b": "7"} -> 200 12 (same as base), {"a": 1, "b": 2, "c": 3} -> 200 3 (unknown key dropped), opt with {} -> defaults applied.
• Enum is still enforced through validate_parameters: {"mode": "medium"} -> 422.
• The jsonschema branch behaves: valid passes; missing required, wrong type, nested type error, array-item type error, enum, and additionalProperties: false all produce 422 with useful type values (required, type, enum, additionalProperties); additionalProperties: true keeps the extras.
• Vendored modules are exactly upstream: httpserver 0.6.1 and jsonschema 0.4.1 are byte-identical to zerodep v2026.10.23.2 after stripping the # /// zerodep header.
• Suite: 366 passed, 2 skipped.

Three non-blocking observations:

1. type is now always value_error for signature tools, and input is gone. Core's ErrorDetail has no validator, so _validation_details falls back to value_error, and the pydantic input echo is lost: {"b": missing} now reports msg: "Missing required field 'b' at '$'", type: "value_error" where base reported msg: "Field required", type: "missing". The envelope keys are preserved as promised, but a client that branches on type (or greps messages) sees a different contract. Worth either mapping core error kinds onto stable types or stating in the PR/changelog that type is now coarse.
2. The published requestBody schemas are inline and titled Data. Base emitted {"$ref": "#/components/schemas/AddRequest"}; head emits the tool schema inline with FastAPI's placeholder title: "Data" on every route. Inline is fine (arguably better for codegen), but the title is noise in a 36-tool document, and normalize_parameters_schema no longer runs over the schema before it is published.
3. Nested loc uses dotted paths for indices: a bad array item reports loc: ["body", "items[1]", "id"], where FastAPI's convention is ["body", "items", 1, "id"]. Cosmetic, but clients that walk loc positionally will mis-render it.

The 500 is the only blocker; the three notes can be follow-ups if you would rather keep this PR tight.

[clementine-oaklight]

A non-dict body reaches this call and crashes: with Annotated[Any, Body()] the endpoint accepts any JSON, so _strip_framework_fields() gets a list/str/number and raises AttributeError: 'list' object has no attribute 'items' -> 500.

Repro on this head:

POST /tools/default/add   json=[1, 2]    -> 500 Internal Server Error
POST /tools/default/add   json="hello"  -> 500 Internal Server Error
POST /tools/default/add   json=5        -> 500 Internal Server Error

On base all three are 422 with type: model_attributes_type. Adding this at the top of _validate_arguments restores parity exactly (I ran the same matrix before/after):

if not isinstance(raw, dict):
    raise HTTPException(
        status_code=422,
        detail=[{
            "loc": ["body"],
            "msg": "Input should be a valid dictionary or object to extract fields from",
            "type": "model_attributes_type",
        }],
    )

A client error reported as a 500 is the one status split worth keeping strict, and it costs four lines here. (Same applies to the jsonschema branch: schema_validate(payload, ...) with payload a list would also be wrong, just further along.)

[Oaklight]

Fixed — thanks for measuring it rather than reading it.

Applied in d5116fe: _validate_arguments now rejects anything that is not a JSON object up front with

{"loc": ["body"], "msg": "Input should be a valid dictionary or object to extract fields from", "type": "model_attributes_type"}

— the same shape the pydantic models used to emit. Regression coverage at both levels:

• TestValidateArguments::test_non_object_body_is_a_422 ([], "str", 123, None, True)
• TestOpenAPIIntegration::test_non_object_body_returns_422_not_500 through TestClient (array, string, number, form post)

Measured again through the app: [] → 422, "str" → 422, 123 → 422, null → 422 (Field required, FastAPI-side, same as 0.7.0), form post → 422. Full suite: 369 passed, 8 skipped.

#124 is rebased on this commit.

[clementine-oaklight]

Fix verified at d5116fe - and verified the way the finding was found, by re-running the same request matrix against the same base, not by reading the patch.

The matrix is back to base behaviour. 26cd0fc (base) vs d5116fe:

body-array    422 model_attributes_type  | 422 model_attributes_type   <- was 500
greet body-array  same
body-string   422 model_attributes_type  | 422 model_attributes_type   <- was 500
body-number   422 model_attributes_type  | 422 model_attributes_type   <- was 500
body-null     422 missing                | 422 missing
no-body       422 missing                | 422 missing
valid / numeric-strings / extra-key / opt defaults    200 both sides, same bodies
pick invalid-enum  422                   | 422

Every previously-failing case now carries the same msg (Input should be a valid dictionary or object to extract fields from) and type (model_attributes_type) as the pydantic path, so a client that matched on those strings does not notice the refactor.

The guard is in the right place: it runs before the parameters_model / jsonschema branch split, so both tool kinds are covered by one check, and raw is now annotated Any to match what Annotated[Any, Body()] actually delivers.

Tests cover both layers, which is what I would have asked for: the unit-level parametrised case [[], "str", 123, None, True] asserts loc == ["body"] and type == model_attributes_type, and the TestClient case checks the real HTTP status for json=[], json="str", json=123 and a form-encoded body. Suite 375 passed, 2 skipped (was 366; +9 tests).

Nothing else moved in the fix commit - 2 files, +53/-2, adapter guard + tests only.

One correction to my earlier review, for the record: my three non-blocking notes still stand exactly as written (type coarse for signature tools, inline requestBody with FastAPI's title: "Data", dotted loc for array indices). They are not made worse by this fix and I am not blocking on them.

Approving. This is the third behaviour I have measured on this PR (coercion, extra-key filtering, body shape) and all three now match the previous implementation.

[elena-oaklight]

Re-approved at d5116fe. The fix matches what master did, not just the error type name.

What I checked on the new commit

Same registry and TestClient, master on the left, this head on the right:

body            master                                             d5116fe
[]              422 type=model_attributes_type loc=[body]           422 type=model_attributes_type loc=[body]
"str"           422 type=model_attributes_type                       422 same
123             422 type=model_attributes_type                       422 same
true            422 type=model_attributes_type                       422 same
[{}]            422 type=model_attributes_type                       422 same
text/plain      {"a":1,"b":2} 422 model_attributes_type              422 same
null            422 type=missing "Field required"                    422 same, input: null kept
empty body      422 type=missing "Field required"                    422 same, input: null kept
not json        422 type=json_invalid loc=[body,0] "JSON decode error" 422 same

The msg text is byte-identical to what pydantic produced (Input should be a valid dictionary or object to extract fields from), so a client that was matching on it does not notice the switch. The one remaining difference from master is the missing input key on the adapter-produced errors, which is consistent with the rest of this PR's error shape.

Object bodies are unchanged from the previous head: valid 200, missing field 422, extra keys dropped, numeric strings coerced, and {"a": true} still 422.

The guard has teeth

With the if not isinstance(raw, dict) branch turned off, the new tests fail (9 failed) and the HTTP response goes back to what #123 was reported for:

POST /tools/default/add  json=[]   ->  500 Internal Server Error

So the unit tests and the TestClient tests both pin the fix rather than the symptom.

Suite and CI

375 passed, 2 skipped (was 366; the +9 are the two parametrized test groups). All checks green - lint, test 3.10-3.13, test-mcp-v1, build, GitGuardian - and merge state CLEAN.

The two notes from my first review stand and neither blocks this: the 422 type/msg change for object bodies (missing -> value_error) plus the dropped input field are client-visible and need a changelog line, and _vendor/httpserver.py is still imported nowhere.

[milo-oaklight]

Re-ran my matrix at d5116fe and the request is cleared.

body          0.7.0                      407394b         d5116fe
[]            422 model_attributes_type   500             422 model_attributes_type
"str"         422                         500             422 model_attributes_type
null          422                         500             422 model_attributes_type
123           422                         500             422 model_attributes_type
form post     422                         500             422 model_attributes_type
{}            422 (field-level)           422             422 (field-level)
{"a":1,"b":2} 200                         200             200

The 422 carries loc: ["body"], type: model_attributes_type and the same wording pydantic used, so a client that matched on the old error still matches. The guard sits above _strip_framework_fields and above the import is already in place, so it fires before anything touches .items().

The tests pin it. Copied the new test file into 407394b and ran the two new cases: 9 failed — the five unit parametrisations and the four TestClient ones. They are not passing on the old code.

Nothing else moved. Sixteen request shapes across int/float/bool/Literal/list[int]/optional/nested: coercion, unknown-key dropping, toolcall_reason stripping, **kwargs passthrough, defaults. Suite 375 passed / 2 skipped, up from 366 — the nine new parametrisations and nothing else.

I resolved my 500 thread. The other thread — additionalProperties: false now rejecting extra keys where 0.7.0 dropped them — is still open as a question, not a blocker: it is a changelog decision you own, and I am fine either way.

Approving at d5116fe.