fix(llm): second CoPilot review pass on PR #42

Addresses 19 review threads from the second CoPilot pass; about half
were duplicates of the same underlying issue:

- examples/00-hello-world/main.py + README hello-world: api_key now
  uses `os.environ.get("LLM_API_KEY") or None` so an exported-but-
  empty env var falls through to no-auth (matters for local servers
  that reject an empty bearer header).

- Both examples now close the OpenAIProvider in the finally block
  alongside graph.drain(). Long-running consumers that copy the
  snippet had been leaking the underlying httpx.AsyncClient.

- errors.py header dropped the hard-coded "seven canonical
  categories" count after StructuredOutputInvalid landed.

- strict_mode_supported docstring and the surrounding spec-anchor
  comment block both updated to match the implementation:
  additionalProperties must be EXPLICITLY false (an omitted key
  counts as non-strict, since JSON Schema's default permits extras).

- _resolve_ref now handles ref == "#" as the document root before
  rejecting external refs. Root-recursive schemas that use the bare
  JSON-Pointer-root form now resolve correctly. Unit test added.

- _strict_mode_check tightened to return False on unrecognized
  shapes (empty {}, const-only, enum-only, unknown keywords) instead
  of falling through to True. Primitive types (string/integer/
  number/boolean/null) classified as terminal-strict-compatible.
  Two unit tests added.

- _build_request_body now explicitly strips response_format from the
  body when the provider is in fallback mode. RuntimeConfig is
  extra="allow", so a caller could have piped response_format
  through the extras loop past the include_response_format gate.

- provider.py module docstring's summary signature line updated to
  match the Protocol's response_schema parameter.

- validate_response_schema's spec-anchor comment updated to reflect
  that JSON Schema validity is now checked at the boundary via
  Draft202012Validator.check_schema(), not delegated to parse time.

- test_pydantic_class_wire_body_matches_dict_form: widened the
  assertion from response_format-only to full body equality, so any
  regression in the class-input wire mapping (not just
  response_format) gets caught.

- test_inspect_property_native_default and
  test_inspect_property_fallback_when_forced converted to async
  with try/finally + aclose() to match the rest of the file's
  provider-lifecycle pattern.

Author: chris-colinsky

README.md

@@ -94,7 +94,7 @@ class PipelineState(State):
 provider = OpenAIProvider(
     base_url=os.environ.get("LLM_BASE_URL", "https://api.openai.com"),  # host root; impl adds /v1
     model=os.environ.get("LLM_MODEL", "gpt-4o-mini"),
-    api_key=os.environ.get("LLM_API_KEY"),
+    api_key=os.environ.get("LLM_API_KEY") or None,                      # empty → no-auth
 )
 
 
@@ -168,6 +168,7 @@ async def main() -> None:
             print(f"summary: {final.summary}")
     finally:
         await graph.drain()
+        await provider.aclose()
 
 
 asyncio.run(main())

examples/00-hello-world/main.py

@@ -81,7 +81,10 @@ class PipelineState(State):
 _provider = OpenAIProvider(
     base_url=os.environ.get("LLM_BASE_URL", "https://api.openai.com"),
     model=os.environ.get("LLM_MODEL", "gpt-4o-mini"),
-    api_key=os.environ.get("LLM_API_KEY"),
+    # ``or None`` so an exported-but-empty LLM_API_KEY falls through to
+    # no-auth (matters for local servers like vLLM that reject an empty
+    # bearer header).
+    api_key=os.environ.get("LLM_API_KEY") or None,
 )
 
 
@@ -197,6 +200,7 @@ async def main() -> None:
         print(f"metadata: {final.metadata}")
     finally:
         await graph.drain()
+        await _provider.aclose()
 
 
 if __name__ == "__main__":

src/openarmature/llm/errors.py

@@ -1,15 +1,15 @@
-# Spec: realizes llm-provider §7 (seven canonical error categories).
+# Spec: realizes llm-provider §7 (canonical error categories).
 
 """Errors raised by an llm-provider implementation.
 
-A provider call (``ready()`` or ``complete()``) MAY raise one of
-seven canonical category errors. Each error class carries a
-``category`` class attribute matching the canonical string identifier
-so callers can dispatch on the category without matching exception
-types directly.
+A provider call (``ready()`` or ``complete()``) MAY raise one of the
+canonical category errors documented below. Each error class carries
+a ``category`` class attribute matching the canonical string
+identifier so callers can dispatch on the category without matching
+exception types directly.
 
 This module is also the single source of truth for the canonical
-category strings — :data:`TRANSIENT_CATEGORIES` lives here, and
+category strings; :data:`TRANSIENT_CATEGORIES` lives here, and
 ``openarmature.graph.middleware.retry``'s default classifier imports
 it.
 """

src/openarmature/llm/provider.py

@@ -14,9 +14,11 @@
   A successful return implies the next ``complete()`` would not
   raise errors that surface mismatched configuration or unloaded
   state.
-- ``async complete(messages, tools=None, config=None) -> Response``
-  — performs a single completion. Stateless, reentrant, MUST NOT
-  mutate its inputs.
+- ``async complete(messages, tools=None, config=None, response_schema=None) -> Response``
+  performs a single completion. Stateless, reentrant, MUST NOT mutate
+  its inputs. When ``response_schema`` is supplied (a JSON Schema
+  dict or Pydantic class), the implementation constrains the model's
+  output and populates ``Response.parsed``.
 
 This module also exports :func:`validate_message_list`: a list-level
 invariant check that complements per-message Pydantic validation. A
@@ -177,9 +179,12 @@ def validate_tools(tools: Sequence[Tool] | None) -> None:
 
 # Spec llm-provider §5 requires the response_schema argument to
 # complete() to be a valid JSON Schema with a top-level type "object".
-# The pre-send check here is the structural minimum; deeper validity
-# (recursive JSON Schema correctness, vendor extensions) is delegated
-# to the runtime validator at parse time.
+# The boundary check here validates BOTH constraints: structural
+# (must be a dict with top-level type: "object") AND full JSON Schema
+# validity via Draft202012Validator.check_schema(). The runtime
+# validator on the parse path only handles instance-against-schema
+# failures; malformed schemas fail here rather than escaping at decode
+# time as jsonschema.SchemaError.
 def validate_response_schema(schema: object) -> None:
     """Pre-send validation for a JSON Schema passed as the
     ``response_schema`` argument to ``complete()``.
@@ -224,9 +229,11 @@ def strict_mode_supported(schema: dict[str, Any]) -> bool:
     by native-decoding LLM wire paths.
 
     Returns True iff for every nested (sub)schema in the tree
-    ``additionalProperties`` is not ``true`` and every key in
-    ``properties`` appears in ``required``. False on any violation, on
-    an unresolvable ``$ref``, or on an unknown shape.
+    ``additionalProperties`` is explicitly ``false`` (an omitted key
+    counts as non-strict, since JSON Schema's default is to permit
+    extras) and every key in ``properties`` appears in ``required``.
+    False on any violation, on an unresolvable ``$ref``, or on an
+    unknown shape.
 
     Args:
         schema: The root JSON Schema dict.
@@ -238,6 +245,13 @@ def strict_mode_supported(schema: dict[str, Any]) -> bool:
     return _strict_mode_check(schema, root=schema, visited=set())
 
 
+# JSON Schema primitive types: terminal-strict-compatible because they
+# carry no nested structure to verify. Object/array types have their
+# own branch checks; anything else (const, enum, unknown keywords,
+# empty {}) is conservatively non-strict.
+_PRIMITIVE_TYPES = frozenset({"string", "integer", "number", "boolean", "null"})
+
+
 def _strict_mode_check(
     schema: Any,
     *,
@@ -311,7 +325,23 @@ def _strict_mode_check(
                 if not _strict_mode_check(item, root=root, visited=visited):
                     return False
 
-    return True
+    # Determine whether the schema declared a shape we know how to
+    # verify. Object/array branches above already returned False on
+    # any internal violation; reaching here means all internal checks
+    # passed. Combinators with all branches passing are likewise OK.
+    # Primitive types are terminal. Anything else (empty schema,
+    # `const`/`enum`-only, unknown keywords) is conservatively
+    # non-strict — the walker can't statically verify it.
+    has_combinator = any(k in schema_dict for k in ("anyOf", "oneOf", "allOf"))
+    if is_object_type or is_array_type or has_combinator:
+        return True
+    if isinstance(schema_type, str) and schema_type in _PRIMITIVE_TYPES:
+        return True
+    if isinstance(schema_type, list) and all(
+        isinstance(t, str) and t in _PRIMITIVE_TYPES for t in cast("list[Any]", schema_type)
+    ):
+        return True
+    return False
 
 
 # Internal-only $ref resolver. Handles JSON Pointer fragments rooted
@@ -320,6 +350,11 @@ def _strict_mode_check(
 # None. JSON Pointer escape rules (`~0` for `~`, `~1` for `/`) are
 # unescaped per RFC 6901.
 def _resolve_ref(ref: str, root: dict[str, Any]) -> dict[str, Any] | None:
+    # Bare "#" is the JSON Pointer for the document root; "#/" prefixes
+    # an internal path. Anything else (external URIs, relative refs we
+    # can't resolve without a base) we treat as unresolvable.
+    if ref == "#":
+        return root
     if not ref.startswith("#/"):
         return None
     parts = ref[2:].split("/")

src/openarmature/llm/providers/openai.py

@@ -360,6 +360,13 @@ def _build_request_body(
                     "strict": strict_mode_supported(schema_dict),
                 },
             }
+        elif not include_response_format:
+            # On the fallback path the §8.5.1 contract is "response_format
+            # MUST NOT be on the wire." RuntimeConfig is extra="allow" so
+            # a caller could pass response_format through via the extras
+            # loop above; strip it here so the fallback contract holds
+            # regardless of caller-supplied extras.
+            body.pop("response_format", None)
         return body
 
     # ------------------------------------------------------------------

tests/unit/test_structured_output.py

@@ -178,6 +178,44 @@ def test_strict_mode_unresolvable_ref_fails() -> None:
     assert strict_mode_supported(schema) is False
 
 
+def test_strict_mode_empty_property_schema_fails() -> None:
+    # A property schema of {} (matches anything) cannot be statically
+    # verified as strict-compatible. The walker should return False
+    # rather than fall through to True.
+    schema: dict[str, Any] = {
+        "type": "object",
+        "properties": {"x": {}},
+        "required": ["x"],
+        "additionalProperties": False,
+    }
+    assert strict_mode_supported(schema) is False
+
+
+def test_strict_mode_primitive_property_passes() -> None:
+    # Primitive types (string, integer, number, boolean, null) carry no
+    # nested structure to verify, so they are terminal-strict-compatible.
+    schema: dict[str, Any] = {
+        "type": "object",
+        "properties": {"name": {"type": "string"}, "age": {"type": "integer"}},
+        "required": ["name", "age"],
+        "additionalProperties": False,
+    }
+    assert strict_mode_supported(schema) is True
+
+
+def test_strict_mode_resolves_bare_root_ref() -> None:
+    # JSON Pointer "#" is a valid reference to the document root
+    # (RFC 6901). A schema using $ref: "#" for self-recursion should
+    # resolve through and inherit the root's strict-mode status.
+    schema: dict[str, Any] = {
+        "type": "object",
+        "properties": {"value": {"type": "string"}, "self": {"$ref": "#"}},
+        "required": ["value", "self"],
+        "additionalProperties": False,
+    }
+    assert strict_mode_supported(schema) is True
+
+
 def test_strict_mode_handles_ref_cycle() -> None:
     # Self-referential schema: each entry has a "children" key pointing
     # back to the same definition. Without cycle protection this would
@@ -445,28 +483,34 @@ def handler_dict(request: httpx.Request) -> httpx.Response:
 
     body_class = json.loads(captured_class[0].content)
     body_dict = json.loads(captured_dict[0].content)
-    assert body_class["response_format"] == body_dict["response_format"]
+    assert body_class == body_dict
 
 
 # ---------------------------------------------------------------------------
 # uses_prompt_augmentation_fallback inspect property
 # ---------------------------------------------------------------------------
 
 
-def test_inspect_property_native_default() -> None:
+async def test_inspect_property_native_default() -> None:
     provider = OpenAIProvider(
         base_url="http://mock-llm.test",
         model="test-model",
         api_key="test-key",
     )
-    assert provider.uses_prompt_augmentation_fallback is False
+    try:
+        assert provider.uses_prompt_augmentation_fallback is False
+    finally:
+        await provider.aclose()
 
 
-def test_inspect_property_fallback_when_forced() -> None:
+async def test_inspect_property_fallback_when_forced() -> None:
     provider = OpenAIProvider(
         base_url="http://mock-llm.test",
         model="test-model",
         api_key="test-key",
         force_prompt_augmentation_fallback=True,
     )
-    assert provider.uses_prompt_augmentation_fallback is True
+    try:
+        assert provider.uses_prompt_augmentation_fallback is True
+    finally:
+        await provider.aclose()