Skip to content

Commit 1b5fbb0

Browse files
fix: third CoPilot review pass on PR #42
Addresses 5 remaining review threads (3 substantive, 2 stale on already-fixed code): - LlmProviderResponseAssertion (the typed assertion model in harness/expectations.py) now lists `parsed: Any | None`. The runtime assertion in test_llm_provider.py already handled it, but the typed parser had it under extra="forbid" and would have rejected any future case-shape LLM fixture using `parsed`. The 021-028 fixtures slip past today on `calls:` form's permissive `LlmCallSpec.expected: dict[str, Any]`; this lines the two paths up. - docs/model-providers/authoring.md skeleton comment tightened: removed the "ignore it and return free-form text" option from the response_schema guidance. A provider that silently drops the parameter violates the Protocol contract; callers expect either Response.parsed populated or StructuredOutputInvalid raised. Now only two valid options surfaced: raise ProviderInvalidRequest until implemented, or wire it through. - docs/concepts/llms.md softened the static-typing claim in the Pydantic-class form section. Response.parsed is `dict[str, Any] | BaseModel | None`, so a type checker won't narrow from `response_schema=Classification` alone. The page now separates the runtime guarantee (validated instance) from static access (requires cast/isinstance/typed assignment); generic Response[T] flagged as a follow-up. The two stale threads (examples/00-hello-world/main.py provider cleanup, test_structured_output.py provider cleanup) were already fixed in commit 8ed334c; replies sent + threads resolved without code changes.
1 parent 8ed334c commit 1b5fbb0

3 files changed

Lines changed: 26 additions & 10 deletions

File tree

docs/concepts/llms.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,18 @@ async def classify(state):
9797
return {"classification": response.parsed}
9898
```
9999

100-
`Response.parsed` is a validated `Classification` instance. Field
101-
access is statically typed (`response.parsed.intent` returns
102-
`Literal["research", "summarize"]`); the framework calls
103-
`.model_json_schema()` under the hood to derive the wire body and
104-
`.model_validate()` to deserialize the response.
100+
`Response.parsed` is a validated `Classification` instance at
101+
runtime; the framework calls `.model_json_schema()` under the hood
102+
to derive the wire body and `.model_validate()` to deserialize the
103+
response.
104+
105+
Static typing is shallower. `Response.parsed` is annotated as
106+
`dict[str, Any] | BaseModel | None`, so a type checker won't narrow
107+
to `Classification` from the `response_schema=Classification`
108+
argument alone. Callers that want static field access either
109+
`cast(Classification, response.parsed)`, narrow with `isinstance`,
110+
or assign the value into a typed local. Generic `Response[T]` is on
111+
the table as a follow-up.
105112

106113
### JSON Schema dict form
107114

docs/model-providers/authoring.md

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,14 @@ class MyProvider:
6767
config: RuntimeConfig | None = None,
6868
response_schema: dict[str, Any] | type[BaseModel] | None = None,
6969
) -> Response:
70-
# response_schema support is an optional capability; a skeleton
71-
# provider can raise ProviderInvalidRequest when it's set, or
72-
# ignore it and return free-form text. A production provider
73-
# would wire it through to native response_format support or
74-
# the prompt-augmentation fallback. See ``openarmature.llm.OpenAIProvider``.
70+
# response_schema is part of the Protocol; a skeleton provider
71+
# MUST NOT silently ignore it — callers expect either
72+
# Response.parsed populated or a StructuredOutputInvalid raise.
73+
# Until the wire path is implemented, raise
74+
# ProviderInvalidRequest when response_schema is set. A
75+
# production provider wires it through to native response_format
76+
# support or the prompt-augmentation fallback; see
77+
# ``openarmature.llm.OpenAIProvider``.
7578
validate_message_list(messages)
7679
validate_tools(tools)
7780

tests/conformance/harness/expectations.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ class LlmProviderResponseAssertion(_ForbidExtras):
7171
finish_reason: str | None = None
7272
usage: dict[str, Any] | None = None
7373
raw_check: dict[str, Any] | None = None
74+
# `parsed` was introduced by proposal 0016 — the runtime asserts
75+
# equality against ``Response.parsed``. Typed as Any | None because
76+
# the fixture-side value can be a dict (dict-schema input form),
77+
# a model_dump-equivalent dict (class-schema form), or None
78+
# (tool-call response or no-schema call).
79+
parsed: Any | None = None
7480

7581

7682
class LlmProviderRaisesAssertion(BaseModel):

0 commit comments

Comments
 (0)