From 61698a9ef31c440312f22f905c21973e8d401a62 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 25 Sep 2026 19:18:40 -0700 Subject: [PATCH 01/14] Add reask and provider-extras examples Two v0.17.0 features had no example. Both demonstrate behaviour verified against a running provider rather than read off the spec. structured-output-reask extracts one mission record per lunar-landing report and corrects the model when it answers in the wrong shape. It could not have been written before the field rename in the previous commit: a builder reading the documented `output_content` and `error_message` raised AttributeError, which the retry loop turned into the original failure. A `MODE=off` arm omits the builder so the contrast is visible. provider-extras reaches two OpenAI request fields openarmature does not model, and shows the three things extras refuses. The first draft used `seed` as the vendor knob; `seed` is managed and would have rejected, so the example would have shipped demonstrating a failure as the feature. Probing the arms first caught it. Two nuances are documented rather than demonstrated: a matching extras value is a no-op rather than an error, and a sampling field left unset is not managed on that call, so managed means produced-by-this-call rather than nameable. The examples smoke test enumerated its demos with nothing checking the list against the directory, so a new example was covered by nothing until someone remembered to add it. It now derives the set from disk and compares. `tests/unit/test_structured_output.py` gains a test that writes the reask builder the documented way. Every existing test read the implementation's own spelling or went through the conformance harness, which aliased the two names, so none of them could fail on the rename bug. A consistent revert across the three source files reproduces the original AttributeError inside the builder. --- examples/README.md | 36 ++++ examples/provider-extras/main.py | 230 ++++++++++++++++++++ examples/structured-output-reask/main.py | 254 +++++++++++++++++++++++ src/openarmature/AGENTS.md | 2 + tests/test_examples_smoke.py | 18 ++ tests/unit/test_structured_output.py | 87 ++++++++ 6 files changed, 627 insertions(+) create mode 100644 examples/provider-extras/main.py create mode 100644 examples/structured-output-reask/main.py diff --git a/examples/README.md b/examples/README.md index 9d11ef5..cd8e897 100644 --- a/examples/README.md +++ b/examples/README.md @@ -88,6 +88,24 @@ the chat shape. Demonstrates: `ChatPrompt`, `ContentSegment`, `PlaceholderSegment`, history threading via the `append` reducer on `list[Message]`, conditional self-loop for multi-turn cycles. +### Providers + +#### [`provider-extras/`](./provider-extras/main.py) + +Reaching an OpenAI request field openarmature does not model, and the +guardrails that stop you reaching one it does. A bulk telemetry-alert +classifier wants `service_tier` for the cheaper lane and `logit_bias` to +suppress a retired severity label; neither is portable, so neither has a +first-class field, and both ride in `extras` untouched. The refusals are the +interesting half: a field the mapping produced on this call cannot also come +from `extras`, because two sources of truth for one wire field is a bug you +want at the call site. Demonstrates: `RuntimeConfig(extras=...)` passthrough, +`ProviderInvalidRequest` on a managed-field collision, `model` / `messages` / +`tools` / `tool_choice` rejecting always so an `extras` tool array cannot skip +tool validation, and `stop` merging with `stop_sequences` rather than colliding +because both realize the same wire field. Runs against a stub transport, since +the point is the shape of the outbound request. + ### Tool use #### [`tool-use/`](./tool-use/main.py) @@ -127,6 +145,24 @@ no-op on symmetric OpenAI, meaningful on asymmetric providers), mapping ### Reliability +#### [`structured-output-reask/`](./structured-output-reask/main.py) + +Extracting one structured mission record per lunar-landing report, and +correcting the model when it answers in the wrong shape. A strict JSON +schema asks for a numeric `mass_kg`, which is the constraint a model most +often breaks by echoing `"1,340 kg"` straight out of the prose. Rather than +failing the run or retrying an identical prompt that reproduces the +identical mistake, a caller-supplied builder quotes the model's own reply +and the validator's objection back to it and asks again. Demonstrates: +`complete(response_schema=...)` raising `StructuredOutputInvalid` instead +of returning a half-built object, `LlmRetryConfig(reask=...)` making that +failure retryable for one call, a builder reading `exc.output_content` and +`exc.error_message` to compose the correction, the framework appending the +model's reply and the correction as an alternating transcript while +authoring no prompt of its own, and reask sharing the `max_attempts` +budget with transient retries. `MODE=off` omits the builder so the first +invalid reply is terminal, which shows what the builder buys. + #### [`checkpointing-and-migration/`](./checkpointing-and-migration/main.py) A lunar-mission planning pipeline that survives a simulated diff --git a/examples/provider-extras/main.py b/examples/provider-extras/main.py new file mode 100644 index 0000000..7d98c25 --- /dev/null +++ b/examples/provider-extras/main.py @@ -0,0 +1,230 @@ +"""openarmature demo: reach a vendor knob openarmature does not model, and +watch the guardrails that stop you reaching the wrong one. + +**Use case:** You classify lunar telemetry alerts in bulk. Two things you want +are provider-specific rather than portable, so there is no first-class field for +either: ``service_tier`` to take the cheaper, slower lane for a batch nobody is +waiting on, and ``logit_bias`` to stop the model emitting a severity label your +team retired last quarter. Both are real OpenAI request fields. Neither means +anything on another provider. + +``extras`` is where those go. It is a named container on the runtime config, and +whatever you put in it rides to the wire untouched. That is the whole feature, +and it exists so a provider-specific knob does not require either a fork or a +framework release. + +The interesting part is what it refuses. A field openarmature already models is +managed, and putting it in ``extras`` too is an error rather than an override, +because two sources of truth for one wire field is a bug you want at the call +site and not in a trace three days later. + +**What's interesting in the implementation:** + +- ``RuntimeConfig(extras={...})`` carries anything the framework does not model. + ``logit_bias`` and ``service_tier`` arrive on the request body verbatim. +- A key naming a field the mapping PRODUCED on this call is rejected. Setting + ``temperature=0.2`` and also ``extras={"temperature": 0.9}`` raises + ``ProviderInvalidRequest``, naming the key. +- Two nuances worth knowing, shown in the notes rather than demonstrated: + a MATCHING value is a no-op rather than an error, since there is no ambiguity + about what to send; and a sampling field you leave unset is not managed on that + call, so ``extras={"temperature": 0.9}`` with no ``temperature=`` rides + through. Managed means "produced by this call", not "nameable". +- ``model``, ``messages``, ``tools`` and ``tool_choice`` are structural and + reject ALWAYS, even where the call produced no such field. That is what stops + an ``extras`` tool array from bypassing tool validation entirely. +- ``stop`` MERGES instead of colliding, because it realizes the same thing as + ``stop_sequences``. Both lists arrive, concatenated. + +**Run it:** + + export LLM_API_KEY=sk-... + uv run python examples/provider-extras/main.py + +This demo talks to a stub transport rather than a real endpoint, because the +point is the shape of the outbound request. Set ``LLM_BASE_URL`` and drop +``--stub`` behaviour if you want to watch a real provider accept the knobs. +""" + +from __future__ import annotations + +import asyncio +import json +from collections.abc import Mapping +from typing import Any + +import httpx + +from openarmature.graph import END, CompiledGraph, GraphBuilder, State +from openarmature.llm import ( + OpenAIProvider, + ProviderInvalidRequest, + RuntimeConfig, + SystemMessage, + UserMessage, +) + +# --------------------------------------------------------------------------- +# Telemetry alerts to classify. In a real app these arrive off a spacecraft +# telemetry bus or an alerting pipeline. +# --------------------------------------------------------------------------- + +ALERTS: list[str] = [ + "Regolith intake auger current 18% above nominal for 40 seconds, then settled.", + "South-pole relay lost carrier for 3 frames during Earth occultation.", + "Battery bus B cell 4 reading 0.2V under its siblings at end of charge.", +] + +_CLASSIFY_SYSTEM = "Classify a lunar telemetry alert. Answer with one word: routine, watch, or urgent." + +# The retired label we no longer want the model to emit. In production these +# token ids come from the provider's tokenizer; the value is illustrative. +_RETIRED_LABEL_TOKEN = "24886" + +# The two knobs this demo is here for. Neither is portable, so openarmature +# models neither, and neither needs a framework change to use. +VENDOR_KNOBS: dict[str, Any] = { + # Cheaper, slower lane. Fine for a batch nobody is waiting on. + "service_tier": "flex", + # Discourage the retired severity label. + "logit_bias": {_RETIRED_LABEL_TOKEN: -100}, +} + + +# --------------------------------------------------------------------------- +# A stub transport, so the demo can show the outbound body without an account. +# --------------------------------------------------------------------------- + +_sent_bodies: list[dict[str, Any]] = [] + + +def _stub(request: httpx.Request) -> httpx.Response: + _sent_bodies.append(json.loads(request.content)) + return httpx.Response( + 200, + json={ + "id": "alert-1", + "model": "stub", + "choices": [{"message": {"role": "assistant", "content": "watch"}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 12, "completion_tokens": 1, "total_tokens": 13}, + }, + ) + + +def _provider() -> OpenAIProvider: + return OpenAIProvider( + base_url="http://telemetry-classifier.invalid", + model="gpt-4o-mini", + api_key="stub", + transport=httpx.MockTransport(_stub), + ) + + +class AlertState(State): + alerts: list[str] = [] + severities: list[str] = [] + refusals: list[str] = [] + + +async def classify(s: AlertState) -> Mapping[str, Any]: + """Classify each alert, sending the two vendor knobs alongside.""" + provider = _provider() + severities: list[str] = [] + try: + for alert in s.alerts: + response = await provider.complete( + [SystemMessage(content=_CLASSIFY_SYSTEM), UserMessage(content=alert)], + # `temperature` is a field, so it goes in the field. The vendor + # knobs have no field, so they go in `extras`. Putting + # `temperature` in both is the error the next node demonstrates. + config=RuntimeConfig(temperature=0.0, extras=VENDOR_KNOBS), + ) + severities.append((response.message.content or "").strip()) + finally: + await provider.aclose() + return {"severities": severities} + + +async def show_guardrails(s: AlertState) -> Mapping[str, Any]: + """Try the three things `extras` refuses, and record what it said.""" + refusals: list[str] = [] + + async def _attempt(label: str, config: RuntimeConfig) -> None: + provider = _provider() + try: + await provider.complete([UserMessage(content="ping")], config=config) + refusals.append(f"{label}: accepted") + except ProviderInvalidRequest as exc: + refusals.append(f"{label}: refused, {exc}") + finally: + await provider.aclose() + + # 1. A managed field the call produced. Two sources of truth for one wire + # field, so it is refused rather than silently preferring one. + await _attempt( + "temperature in both", + RuntimeConfig(temperature=0.2, extras={"temperature": 0.9}), + ) + # 2. A structural key. Refused even though this call passes no tools, which + # is the point: an extras tool array would otherwise skip validation. + await _attempt( + "tools via extras", + RuntimeConfig(extras={"tools": [{"type": "function"}]}), + ) + # 3. Not a refusal. `stop` realizes the same wire field as `stop_sequences`, + # so the two merge instead of colliding. + await _attempt( + "stop merges rather than collides", + RuntimeConfig(stop_sequences=["END OF ALERT"], extras={"stop": ["HALT"]}), + ) + return {"refusals": refusals} + + +def build_graph() -> CompiledGraph[AlertState]: + return ( + GraphBuilder(AlertState) + .add_node("classify", classify) + .add_node("show_guardrails", show_guardrails) + .add_edge("classify", "show_guardrails") + .add_edge("show_guardrails", END) + .set_entry("classify") + .compile() + ) + + +async def main() -> None: + print("=== openarmature provider-extras demo ===") + print(f"alerts: {len(ALERTS)}") + print() + + graph = build_graph() + try: + final = await graph.invoke(AlertState(alerts=ALERTS)) + finally: + await graph.drain() + + print("classified:") + for alert, severity in zip(ALERTS, final.severities, strict=False): + print(f" [{severity}] {alert[:64]}...") + + print() + print("the knobs that reached the wire:") + first = _sent_bodies[0] + for key in ("service_tier", "logit_bias", "temperature"): + if key in first: + print(f" {key} = {first[key]!r}") + + print() + print("what extras refuses:") + for line in final.refusals: + print(f" {line}") + + print() + print( + "`extras` is an escape hatch for knobs openarmature does not model, not a\n" + "second way to set the ones it does." + ) + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/examples/structured-output-reask/main.py b/examples/structured-output-reask/main.py new file mode 100644 index 0000000..cf5bfab --- /dev/null +++ b/examples/structured-output-reask/main.py @@ -0,0 +1,254 @@ +"""openarmature demo: pull a structured mission record out of a prose +lunar-landing report, and correct the model when it answers in the wrong +shape. + +**Use case:** A feed delivers lunar-landing reports as free prose. You want +one row per report: mission, operator, landing site, outcome, and the mass +delivered in kilograms. A JSON schema says exactly that, and the provider is +asked to honour it. + +Models miss. They wrap JSON in a markdown fence, answer in prose because the +report was ambiguous, invent a field, or send ``"1,340 kg"`` where the schema +says a number. The useful move is not to fail the run and not to retry the +identical prompt, which reproduces the identical mistake. It is to show the +model what it returned, say what was wrong with it, and ask again. + +That is what ``reask`` is: you supply the corrective message, because only you +know how to talk to your model about your schema. The framework supplies the +loop, the transcript, and the budget. + +**What's interesting in the implementation:** + +- ``complete(response_schema=...)`` asks the provider for a shape. When the + reply cannot be parsed or does not validate, the call raises + ``StructuredOutputInvalid`` rather than handing back a half-built object. +- ``LlmRetryConfig(reask=...)`` makes that failure retryable FOR THIS CALL. + Without a builder it is terminal, which is the right default: a schema the + model cannot satisfy is usually a bug in the schema, not a transient. +- The builder receives the exception and returns the correction as a string. + It reads ``exc.output_content`` (verbatim, what the model actually sent) and + ``exc.error_message`` (what the validator objected to). Quoting both back is + what makes the second attempt different from the first. +- The framework appends the model's own reply as an ``assistant`` turn and the + builder's string as a ``user`` turn, so the conversation stays + role-alternating and the model sees its own mistake in context. It authors no + prompt of its own; every word sent is yours. +- Reask shares the ``max_attempts`` budget with transient retries. A call that + burns two attempts on malformed output has one left for a rate limit. +- ``MODE`` selects the posture. ``"reask"`` (default) supplies the builder. + ``"off"`` omits it, so the first invalid reply ends the call and you can see + what the builder is buying. + +**Run it:** + + export LLM_API_KEY=sk-... + uv run python examples/structured-output-reask/main.py + + MODE=off uv run python examples/structured-output-reask/main.py + +Point ``LLM_BASE_URL`` at any OpenAI-compatible endpoint. ``LLM_MODEL`` +defaults to a small fast model; a weaker one makes the reask path fire more +often, which is the interesting case to watch. +""" + +from __future__ import annotations + +import asyncio +import json +import os +from collections.abc import Mapping +from typing import Any + +from openarmature.graph import END, CompiledGraph, GraphBuilder, State +from openarmature.graph.middleware import deterministic_backoff +from openarmature.llm import ( + LlmRetryConfig, + OpenAIProvider, + RuntimeConfig, + StructuredOutputInvalid, + SystemMessage, + UserMessage, +) + +# --------------------------------------------------------------------------- +# The report feed. In a real app these arrive from a wire service, an +# operator's status page, or a mission-log database. +# --------------------------------------------------------------------------- + +REPORTS: list[str] = [ + ( + "Intuitive Machines confirmed this morning that IM-3 touched down " + "intact at Reiner Gamma at 04:12 UTC, carrying 1,340 kilograms of " + "instruments for the agency's swirl-magnetism survey." + ), + ( + "The Chandrayaan-4 sample return element separated as planned and is " + "on its way home. Its lander remains at the south pole with roughly " + "620 kg of hardware still on the surface, powered down for the night." + ), + ( + "Contact with Peregrine Flight 2 was lost during descent. Telemetry " + "placed it near Sinus Viscositatis carrying 90 kg of payload. The " + "operator has not yet confirmed whether the vehicle survived." + ), +] + +# The shape we want back. Deliberately strict: `mass_kg` is a number, not a +# string, which is the constraint a model is most likely to break by sending +# "1,340 kg" verbatim from the prose. +MISSION_SCHEMA: dict[str, Any] = { + "type": "object", + "properties": { + "mission": {"type": "string"}, + "operator": {"type": "string"}, + "landing_site": {"type": "string"}, + "outcome": {"type": "string", "enum": ["landed", "lost", "unconfirmed"]}, + "mass_kg": {"type": "number"}, + }, + "required": ["mission", "operator", "landing_site", "outcome", "mass_kg"], + "additionalProperties": False, +} + +_EXTRACT_SYSTEM = ( + "You extract one structured mission record from a lunar-landing report. Answer with a JSON object only." +) + + +# --------------------------------------------------------------------------- +# The reask builder. This is the whole point of the example. +# --------------------------------------------------------------------------- + + +def build_correction(exc: StructuredOutputInvalid) -> str: + """Compose the corrective message sent after an invalid reply. + + Receives the failure and returns what to say about it. The framework + appends the model's own reply before this, so it reads as a conversation: + the model answered, you point at the problem, it answers again. + """ + # Both halves matter. The verbatim output lets the model see what it + # actually sent rather than what it meant to send, and the validator's + # complaint names the constraint it missed. A correction carrying neither + # is just the original prompt again, which reproduces the original answer. + return ( + f"That reply did not match the required schema. The validator said: " + f"{exc.error_message}\n\n" + f"You sent:\n{exc.output_content}\n\n" + "Send the JSON object again, corrected. Return only the object, with " + "no surrounding prose and no markdown fence. `mass_kg` must be a bare " + 'number in kilograms, so write 1340 rather than "1,340 kg". ' + "`outcome` must be exactly one of: landed, lost, unconfirmed." + ) + + +# --------------------------------------------------------------------------- +# Provider and state +# --------------------------------------------------------------------------- + +_provider_instance: OpenAIProvider | None = None + + +def _get_provider() -> OpenAIProvider: + global _provider_instance + if _provider_instance is None: + _provider_instance = 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, + ) + return _provider_instance + + +class FeedState(State): + reports: list[str] = [] + records: list[dict[str, Any]] = [] + failures: list[str] = [] + + +def _retry_config(mode: str) -> LlmRetryConfig: + # `reask` is what makes a schema failure retryable at all. Omit it and the + # first invalid reply is terminal, which is the useful default: retrying an + # unchanged prompt against an unchanged model gets the same answer. + return LlmRetryConfig( + max_attempts=3, + backoff=deterministic_backoff(0.0), + reask=build_correction if mode == "reask" else None, + ) + + +async def extract(s: FeedState) -> Mapping[str, Any]: + """Extract one record per report, tolerating a wrong-shaped first reply.""" + provider = _get_provider() + retry = _retry_config(os.environ.get("MODE", "reask")) + records: list[dict[str, Any]] = [] + failures: list[str] = [] + + for report in s.reports: + try: + response = await provider.complete( + [SystemMessage(content=_EXTRACT_SYSTEM), UserMessage(content=report)], + config=RuntimeConfig(temperature=0.0), + response_schema=MISSION_SCHEMA, + retry=retry, + ) + except StructuredOutputInvalid as exc: + # Reached when the budget runs out, or immediately when MODE=off. + # The exception carries the last thing the model sent and why it + # was rejected, which is what you want in the log. + failures.append(f"{exc.error_message}: {exc.output_content[:120]}") + continue + records.append(json.loads(response.message.content or "{}")) + + return {"records": records, "failures": failures} + + +async def present(s: FeedState) -> Mapping[str, Any]: + return {} + + +def build_graph() -> CompiledGraph[FeedState]: + return ( + GraphBuilder(FeedState) + .add_node("extract", extract) + .add_node("present", present) + .add_edge("extract", "present") + .add_edge("present", END) + .set_entry("extract") + .compile() + ) + + +async def main() -> None: + mode = os.environ.get("MODE", "reask") + print("=== openarmature structured-output-reask demo ===") + print(f"mode: {mode}" + (" (no corrective builder)" if mode == "off" else "")) + print(f"reports: {len(REPORTS)}") + print() + + graph = build_graph() + try: + final = await graph.invoke(FeedState(reports=REPORTS)) + finally: + await _get_provider().aclose() + await graph.drain() + + for record in final.records: + print( + f" {record.get('mission')} | {record.get('operator')} | " + f"{record.get('landing_site')} | {record.get('outcome')} | " + f"{record.get('mass_kg')} kg" + ) + if final.failures: + print() + print(" unrecovered:") + for f in final.failures: + print(f" {f}") + + print() + print(f"extracted {len(final.records)} of {len(REPORTS)}") + if mode == "off" and final.failures: + print("Run without MODE=off to let the corrective builder retry these.") + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md index 567924e..ea2d7a1 100644 --- a/src/openarmature/AGENTS.md +++ b/src/openarmature/AGENTS.md @@ -1689,8 +1689,10 @@ _Runnable example programs shipped in the source tree at `examples/`. The full c - **`examples/observer-hooks/main.py`** — openarmature demo: observer hooks for structured logging, per-call metrics, and OTel spans. - **`examples/parallel-branches/main.py`** — openarmature demo: enrich a lunar-mission news article with several independent analyses running concurrently. - **`examples/production-observability/main.py`** — openarmature demo: production observability with dual OTel + Langfuse observers, caller hooks for trace.input/output, and the canonical TimingMiddleware. +- **`examples/provider-extras/main.py`** — openarmature demo: reach a vendor knob openarmature does not model, and watch the guardrails that stop you reaching the wrong one. - **`examples/retrieval-rag/main.py`** — Retrieval-augmented answering over a lunar knowledge base. - **`examples/routing-and-subgraphs/main.py`** — openarmature demo: conditional routing + subgraph with a custom projection. +- **`examples/structured-output-reask/main.py`** — openarmature demo: pull a structured mission record out of a prose lunar-landing report, and correct the model when it answers in the wrong shape. - **`examples/tool-use/main.py`** — openarmature demo: a lunar-mission assistant that calls local Python functions as tools to answer fact and physics questions about Apollo / Artemis missions. ## Discovery cross-references diff --git a/tests/test_examples_smoke.py b/tests/test_examples_smoke.py index 3103465..7f49f41 100644 --- a/tests/test_examples_smoke.py +++ b/tests/test_examples_smoke.py @@ -44,9 +44,27 @@ "langfuse-observability", "production-observability", "retrieval-rag", + "structured-output-reask", + "provider-extras", ] +def test_every_example_directory_is_listed() -> None: + # `DEMOS` is an explicit enumeration, so a new example is covered by nothing + # until someone remembers to add it here. Derive the directory set and + # compare, so forgetting fails loudly rather than passing quietly. + on_disk = { + d.name + for d in EXAMPLES_DIR.iterdir() + if d.is_dir() and (d / "main.py").exists() and not d.name.startswith(".") + } + listed = set(DEMOS) + assert on_disk == listed, ( + f"DEMOS and examples/ disagree. Only on disk: {sorted(on_disk - listed)}. " + f"Only listed: {sorted(listed - on_disk)}" + ) + + @pytest.mark.parametrize("demo", DEMOS) def test_example_loads(demo: str) -> None: main_py = EXAMPLES_DIR / demo / "main.py" diff --git a/tests/unit/test_structured_output.py b/tests/unit/test_structured_output.py index 8e767d1..e5b97f6 100644 --- a/tests/unit/test_structured_output.py +++ b/tests/unit/test_structured_output.py @@ -697,3 +697,90 @@ async def capturing_post(*args: Any, **kwargs: Any) -> Any: finally: await provider.aclose() assert captured_body_response_format == caller_extra + + +async def test_a_reask_builder_written_against_the_documented_fields_recovers() -> None: + # llm-provider section 7 names the exception's fields `output_content` and + # `error_message`, and section 7.1 says the reask builder receives those two. + # This writes the builder the way the documentation says to and asserts the + # call recovers. + # + # Non-vacuity, and the reason this test exists: the implementation carried + # `raw_content` / `failure_description` instead, so a builder reading the + # documented names raised AttributeError, the retry loop converted it to + # `raise exc from reask_error`, and the call failed on the first invalid + # reply exactly as if no builder had been supplied. Every existing test + # either read the implementation's own spelling or went through a conformance + # harness that aliased the two, so none of them could fail on it. Reverting + # the rename makes this one fail on the attribute rather than the assertion. + from openarmature.graph.middleware import deterministic_backoff + from openarmature.llm import LlmRetryConfig + + schema: dict[str, Any] = { + "type": "object", + "properties": {"mission": {"type": "string"}, "mass_kg": {"type": "number"}}, + "required": ["mission", "mass_kg"], + "additionalProperties": False, + } + bodies: list[dict[str, Any]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + bodies.append(json.loads(request.content)) + # Attempt 0 breaks the schema the way a model actually does: the mass + # echoed as prose rather than coerced to a number. + content = ( + '{"mission": "IM-3", "mass_kg": "1,340 kg"}' + if len(bodies) == 1 + else '{"mission": "IM-3", "mass_kg": 1340}' + ) + return httpx.Response( + 200, + json={ + "id": "r1", + "model": "m", + "choices": [{"message": {"role": "assistant", "content": content}, "finish_reason": "stop"}], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + seen: list[tuple[str, str]] = [] + + def build_correction(exc: StructuredOutputInvalid) -> str: + seen.append((exc.output_content, exc.error_message)) + return "Send mass_kg as a bare number." + + provider = OpenAIProvider( + base_url="http://mock-llm.test", + model="m", + api_key="k", + transport=httpx.MockTransport(handler), + ) + try: + response = await provider.complete( + [UserMessage(content="IM-3 carried 1,340 kg")], + response_schema=schema, + retry=LlmRetryConfig( + max_attempts=3, + backoff=deterministic_backoff(0.0), + reask=build_correction, + ), + ) + finally: + await provider.aclose() + + assert len(bodies) == 2, f"expected one reask attempt, got {len(bodies)} call(s)" + assert json.loads(response.message.content or "{}")["mass_kg"] == 1340 + # The builder saw both documented fields populated, not empty strings. + assert len(seen) == 1 + invalid_output, complaint = seen[0] + assert "1,340 kg" in invalid_output, ( + f"the builder must receive the model's verbatim reply, got {invalid_output!r}" + ) + assert complaint, "the builder must receive the validator's objection, not an empty string" + # The correction reaches the model as a user turn after its own reply, so the + # transcript stays role-alternating. + roles = [m["role"] for m in bodies[-1]["messages"]] + assert roles == ["user", "assistant", "user"], f"expected an alternating transcript, got {roles}" + assert any("bare number" in str(m.get("content", "")) for m in bodies[-1]["messages"]), ( + "the builder's correction must be the final user turn" + ) From f1735c7ee137975968d36a73ea464fc53a39c4e4 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 25 Sep 2026 20:29:18 -0700 Subject: [PATCH 02/14] Ship the conformance manifest, and stop the sdist carrying notes Both from an agent adopting the package, who found the manifest missing from the wheel and asked whether the duplicated pattern files were deliberate. The bundled AGENTS.md tells an agent to read conformance.toml for per-proposal implementation status, which is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the wheel, and the pointer said "at the repo root", so the instruction resolved for someone in a clone and for nobody who installed from PyPI. That asymmetry is why it survived: the sdist had it all along, so working from a clone or an sdist build never hits it. It is force-included at openarmature/conformance.toml now, and the pointer names the importlib.resources path. Verified through the whole chain, including a wheel built from the sdist. It matters more this release than last: 0085 and 0124 both ship partial, and their notes are where which-half-is-missing lives. Force-included rather than copied into src/ the way AGENTS.md is, because a second committed 128K file would appear in every diff that touches the manifest. The cost is that the guard checks config shape rather than a built artifact. The pattern duplication is deliberate and the generator says why: the standalone files back openarmature.patterns.list() / get(name) and use a different transform so each resolves its own cross-references. AGENTS.md now says so, since the alternative is every reader asking. Separately, found while checking the sdist: hatch packages what is on disk and does not read .git/info/exclude, so _tasks/ was being collected for publication. Those are internal working notes. Checked against PyPI: no published release contains them, so this is preventive. The pinned spec submodule is excluded in the same change, 1124 files in 0.16.0 and most of the tarball, taking it from 3.4MB to 1.4MB. tests/, examples/ and docs/ stay in, and the guard asserts that so a later tidy-up has to change the reasoning with it. --- CHANGELOG.md | 2 + pyproject.toml | 23 ++++++++++++ scripts/build_agents_md.py | 18 ++++++++- src/openarmature/AGENTS.md | 4 +- tests/test_smoke.py | 77 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb8449..ef5796d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Fixed +- **The sdist no longer carries the local follow-up notes or the pinned spec submodule.** Hatch packages what is on disk and does not read `.git/info/exclude`, so `_tasks/` was being collected into the tarball even though git ignores it. Those are internal working notes rather than part of the public artifact set. Checked against PyPI: **no published release contains them**, so this is preventive rather than a leak to clean up. The `openarmature-spec` submodule is excluded in the same change, which is most of the tarball (1124 files in 0.16.0) and reconstructs from the `spec_version` pin, taking the sdist from 3.4MB to 1.4MB. `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager verify a build from source, and the other two are adopter-facing. +- **The `conformance.toml` manifest now ships in the wheel.** The bundled `AGENTS.md` tells an agent to read it for per-proposal implementation status, and that is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the distribution and the pointer said "at the repo root", so the instruction resolved for someone working in a clone and for nobody who ran `pip install openarmature`. An agent planning against an accepted proposal could check for an importable symbol and had no equivalent check for a behaviour, which is exactly what the manifest exists to answer. It is force-included at `openarmature/conformance.toml`, beside the other bundled docs, reachable via `importlib.resources.files("openarmature")`, and a wheel built from the sdist gets it too. The pointer now names that path instead of a repo-relative one. **This matters more in this release than the last:** 0085 and 0124 both ship `partial`, and their notes are where which-half-is-missing is recorded. Reported by an agent adopting the package. - **`StructuredOutputInvalid` uses the field names llm-provider §7 specifies** (§7 / §7.1, proposals 0082 + 0095). The exception carried `raw_content` and `failure_description` where §7 names `output_content` and `error_message`, and §7.1 says the `reask` builder receives those two. A caller who wrote the documented builder therefore hit `AttributeError`, which the retry loop catches and converts into `raise exc from reask_error`: the original structured-output failure propagates, reask never runs, and the call fails on the first invalid output exactly as if no builder had been supplied. **This is a pre-1.0 rename of two public attributes**, so a caller reading them directly needs updating; the conformance harness carried an alias bridging the two spellings and that alias is now gone. - **A per-attempt retry override merges undeclared extras per key instead of replacing the container** (llm-provider §7.1). §7.1 says "undeclared extras (§6) merge by the same per-key rule", and the analogue of a declared field is an extras KEY rather than the container: a key the override sets replaces that key, a key it does not mention inherits the base's. The implementation replaced the whole container and logged the base keys it dropped, so an override that adjusted one vendor knob silently withdrew every other knob the caller had set for that attempt. Nothing is dropped now, so the warning is gone with it. Clearing a base key for one attempt stays inexpressible, exactly as an override cannot clear a declared field to `None`. - **The Langfuse Tool observation now carries caller-supplied invocation metadata** (observability §8.4.2). It was the one provider observation that dropped it. The LLM handlers pick the caller set up through the shared typed-event metadata builder and the embedding and rerank handlers apply it directly, but the tool handler did neither, so a caller filtering Langfuse by their own key (a tenant id, a request id) saw every observation from an invocation except the tool calls. The §8.4.2 mapping maps the caller set to `observation.metadata.` on every Observation, and the unscoped wording is deliberate: the same table scopes its other rows explicitly where it means to, for example `fan_out_item_count` to the fan-out node Span only. The OTel observer's tool span had carried the set all along, so the two bundled observers disagreed about the same event. The caller set is merged before the OA-emitted keys, matching the other handlers, though on this observation precedence is unobservable either way: every key it writes is reserved, the `openarmature_*` pair by prefix and `error_type` / `error_message` by name, so a colliding caller key is rejected at the `invoke()` boundary and never reaches the merge. diff --git a/pyproject.toml b/pyproject.toml index 3cfcb8a..c06802a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,9 +108,32 @@ observability = ["openarmature[otel,langfuse]"] # extras, which uv otherwise can't default. default-groups = ["dev", "docs", "examples", "observability"] +# Hatch packages what is on disk. `.git/info/exclude` keeps `_tasks/` out of git +# and hatch never reads that file, so the local follow-up notes were going into +# the sdist and would have published to PyPI. They are working notes, not part of +# the public artifact set. `openarmature-spec/` is a pinned submodule and was most +# of a 3.4MB tarball; it reconstructs from `[tool.openarmature] spec_version`. +# +# `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager +# verify a build from source, and the other two are adopter-facing and small. +[tool.hatch.build.targets.sdist] +exclude = [ + "_tasks/", + "openarmature-spec/", +] + [tool.hatch.build.targets.wheel] packages = ["src/openarmature"] +# The bundled AGENTS.md tells an agent to read this manifest for per-proposal +# implementation status, which is the only artifact that answers "is this real in +# the version I have installed" for a behaviour with no importable name. It has to +# be IN the wheel for that instruction to resolve outside a clone. Force-included +# rather than copied into src/ so there is one copy in the repo: a second 128K +# committed file would show up in every diff that touches the manifest. +[tool.hatch.build.targets.wheel.force-include] +"conformance.toml" = "openarmature/conformance.toml" + [tool.pyright] include = ["src", "tests", "examples"] pythonVersion = "3.12" diff --git a/scripts/build_agents_md.py b/scripts/build_agents_md.py index 54f1ae0..e37cfe5 100644 --- a/scripts/build_agents_md.py +++ b/scripts/build_agents_md.py @@ -208,7 +208,13 @@ def _capability_summaries(spec_tag: str) -> str: + "`spec.md` verbatim — including additions from accepted proposals " + "that this Python implementation may not yet ship. For per-proposal " + "implementation status (implemented / partial / textual-only / " - + "not-yet), see the `conformance.toml` manifest at the repo root. " + + "not-yet), read the `conformance.toml` manifest. It ships inside this " + + "package, beside this file, so it resolves whether you installed from " + + "PyPI or are working in a clone: " + + "`importlib.resources.files('openarmature') / 'conformance.toml'`. " + + "Check it before planning against a behaviour from an accepted " + + "proposal: `partial` entries say which half is missing, and this " + + "release has two. " + "For the full spec text (execution model, error semantics, " + "determinism, observer hooks, etc.) see the linked docs site._" ), @@ -336,6 +342,16 @@ def _patterns() -> str: "_Recipes that compose the primitives. Not framework contracts — " + "these are how to do common things idiomatically._" ), + "", + ( + "_Each pattern below also ships as its own file, readable via " + + "`openarmature.patterns.list()` and `get(name)`. That is deliberate " + + "rather than duplication left in by accident: this copy is for " + + "reading the document whole, and the standalone copy is for " + + "retrieving one pattern without the other 90KB. The two are built " + + "from one source with different link handling, so a standalone " + + "pattern resolves its cross-references on its own._" + ), ] pattern_files = sorted(p for p in (DOCS / "patterns").glob("*.md") if p.name != "index.md") for pf in pattern_files: diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md index ea2d7a1..df020cf 100644 --- a/src/openarmature/AGENTS.md +++ b/src/openarmature/AGENTS.md @@ -10,7 +10,7 @@ OpenArmature is a workflow framework for LLM pipelines and tool-calling agents: ## Capability contracts -_Sourced from openarmature-spec v0.118.2. Each entry below reproduces §1 (Purpose) and §2 (Concepts) of the capability's `spec.md` verbatim — including additions from accepted proposals that this Python implementation may not yet ship. For per-proposal implementation status (implemented / partial / textual-only / not-yet), see the `conformance.toml` manifest at the repo root. For the full spec text (execution model, error semantics, determinism, observer hooks, etc.) see the linked docs site._ +_Sourced from openarmature-spec v0.118.2. Each entry below reproduces §1 (Purpose) and §2 (Concepts) of the capability's `spec.md` verbatim — including additions from accepted proposals that this Python implementation may not yet ship. For per-proposal implementation status (implemented / partial / textual-only / not-yet), read the `conformance.toml` manifest. It ships inside this package, beside this file, so it resolves whether you installed from PyPI or are working in a clone: `importlib.resources.files('openarmature') / 'conformance.toml'`. Check it before planning against a behaviour from an accepted proposal: `partial` entries say which half is missing, and this release has two. For the full spec text (execution model, error semantics, determinism, observer hooks, etc.) see the linked docs site._ ### Capability: `graph-engine` @@ -616,6 +616,8 @@ extras-pass-through bag for vendor-specific knobs. _Recipes that compose the primitives. Not framework contracts — these are how to do common things idiomatically._ +_Each pattern below also ships as its own file, readable via `openarmature.patterns.list()` and `get(name)`. That is deliberate rather than duplication left in by accident: this copy is for reading the document whole, and the standalone copy is for retrieving one pattern without the other 90KB. The two are built from one source with different link handling, so a standalone pattern resolves its cross-references on its own._ + ### Bypass if output exists **Problem.** How do I skip a node whose external output already diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 9bb5ba3..301fd13 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -73,3 +73,80 @@ def test_spec_version_matches_submodule_changelog() -> None: f"submodule's CHANGELOG latest is {submodule_latest}, but " f"__spec_version__ is {openarmature.__spec_version__}" ) + + +def test_the_conformance_manifest_is_force_included_in_the_wheel() -> None: + # The bundled AGENTS.md tells an agent to read `conformance.toml` for + # per-proposal implementation status, which is the only artifact that can + # answer "is this real in the version I have installed" for a behaviour with + # no importable name. That instruction shipped for releases while the file + # did not, so it resolved only for someone working in a clone. An agent + # planning against an accepted proposal had no way to learn that half of it + # was missing, which is exactly what `partial` records. + # + # Guarded here rather than by building a wheel in the suite: this catches the + # entry being dropped or its paths going stale, which is how it would break. + pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" + config = tomllib.loads(pyproject_path.read_text()) + force_include = config["tool"]["hatch"]["build"]["targets"]["wheel"]["force-include"] + + assert "conformance.toml" in force_include, ( + "conformance.toml must be force-included in the wheel; AGENTS.md instructs " + f"an agent to read it. force-include currently: {force_include}" + ) + target = force_include["conformance.toml"] + package = config["tool"]["hatch"]["build"]["targets"]["wheel"]["packages"][0] + package_name = Path(package).name + assert target == f"{package_name}/conformance.toml", ( + f"the manifest must land beside the other bundled docs inside the package; " + f"got {target!r}, expected {package_name}/conformance.toml" + ) + source = pyproject_path.parent / "conformance.toml" + assert source.exists(), f"force-include names a source that does not exist: {source}" + + +def test_agents_md_tells_an_agent_where_the_manifest_actually_is() -> None: + # The pointer used to read "at the repo root", which is unresolvable from a + # venv and is the half of the defect that made the missing file invisible: + # an agent that could not find it had no reason to think it should be there. + bundled = Path(__file__).resolve().parent.parent / "src" / "openarmature" / "AGENTS.md" + text = bundled.read_text() + assert "conformance.toml" in text, "AGENTS.md must still point at the manifest" + assert "at the repo root" not in text, ( + "the manifest pointer must not be repo-relative: it resolves for a clone and " + "not for anyone who installed the package" + ) + assert "importlib.resources" in text, ( + "the pointer should name the access path that works from an installed package" + ) + + +def test_the_sdist_excludes_the_private_follow_up_notes() -> None: + # `_tasks/` is kept out of git by `.git/info/exclude`, which hatch does not + # read: it packages what is on disk. So the local follow-up notes were going + # into the sdist and would have published to PyPI. They are working notes + # that quote internal coordination, not part of the public artifact set. + # + # Guarded by config shape rather than by building an sdist in the suite, + # because the way this breaks is someone editing or dropping the exclude. + pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" + config = tomllib.loads(pyproject_path.read_text()) + exclude = config["tool"]["hatch"]["build"]["targets"]["sdist"]["exclude"] + + assert "_tasks/" in exclude, ( + "`_tasks/` must be excluded from the sdist. It holds private working notes, " + f"and .git/info/exclude does not reach hatch. Current excludes: {exclude}" + ) + # The pinned spec submodule is most of the tarball and reconstructs from the + # version pin, so it is excluded too. Not a privacy matter, just size. + assert "openarmature-spec/" in exclude, ( + f"the pinned spec submodule should stay out of the sdist; excludes: {exclude}" + ) + # Deliberately NOT excluded, asserted so a future tidy-up does not quietly + # drop them: tests let a packager verify a build from source, and the other + # two are adopter-facing. + for kept in ("tests/", "examples/", "docs/"): + assert kept not in exclude, ( + f"{kept} is excluded from the sdist; it was kept on purpose, so if that " + "changed the reasoning in pyproject.toml needs changing with it" + ) From ca17420c16bb0847ef6017a964e2665024fbef6e Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sat, 26 Sep 2026 09:03:15 -0700 Subject: [PATCH 03/14] Name both manifest locations, and stop the demo asking for a key Two from review, both mine. The manifest pointer named only the packaged path, which resolves for an installed package and not in a clone: force-include is build-time, so nothing lands under src/. Verified by running it, `is_file()` is False here. That is the same defect the change set out to fix, with the polarity flipped, since the original said "at the repo root" and was unresolvable from a venv. It now names both and says which is which. The guard asserted the pointer was NOT repo-relative, which stopped being the defect once the clone path became correct to mention. It now asserts both paths are named, and catches either one going missing. The provider-extras run block asked for an API key the demo never reads, and then told the reader to drop a `--stub` flag that does not exist. The example parses no arguments and reads no environment variables at all, so both sentences described a program that was not there. Rewritten to say it needs no credentials, why a stub is the right thing for a demo about the outbound request body, and the one real edit that points it at a live endpoint. Swept the sibling example: every env var its run block names is actually read, so the defect was local to this one. --- examples/provider-extras/main.py | 12 ++++++++---- scripts/build_agents_md.py | 14 ++++++++------ src/openarmature/AGENTS.md | 2 +- tests/test_smoke.py | 24 +++++++++++++++--------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/examples/provider-extras/main.py b/examples/provider-extras/main.py index 7d98c25..8e0fa07 100644 --- a/examples/provider-extras/main.py +++ b/examples/provider-extras/main.py @@ -38,12 +38,16 @@ **Run it:** - export LLM_API_KEY=sk-... uv run python examples/provider-extras/main.py -This demo talks to a stub transport rather than a real endpoint, because the -point is the shape of the outbound request. Set ``LLM_BASE_URL`` and drop -``--stub`` behaviour if you want to watch a real provider accept the knobs. +No credentials needed. This demo installs a stub transport and prints the +outbound request body, because the shape of that body IS the subject: whether a +knob reached the wire, and what happened when it collided with one the framework +manages. A real endpoint would answer neither question any better, and the +refusals happen before any request is sent. + +To watch a real provider accept the knobs, change ``_provider()`` to drop the +``transport=`` argument and supply a real ``base_url`` and ``api_key``. """ from __future__ import annotations diff --git a/scripts/build_agents_md.py b/scripts/build_agents_md.py index e37cfe5..4281ce4 100644 --- a/scripts/build_agents_md.py +++ b/scripts/build_agents_md.py @@ -208,13 +208,15 @@ def _capability_summaries(spec_tag: str) -> str: + "`spec.md` verbatim — including additions from accepted proposals " + "that this Python implementation may not yet ship. For per-proposal " + "implementation status (implemented / partial / textual-only / " - + "not-yet), read the `conformance.toml` manifest. It ships inside this " - + "package, beside this file, so it resolves whether you installed from " - + "PyPI or are working in a clone: " - + "`importlib.resources.files('openarmature') / 'conformance.toml'`. " + + "not-yet), read the `conformance.toml` manifest. Where it is depends " + + "on how you got this package. Installed from PyPI: beside this file, " + + "at `importlib.resources.files('openarmature') / 'conformance.toml'`. " + + "In a source checkout: at the repository root, since it is added to " + + "the distribution at build time rather than committed under `src/`. " + "Check it before planning against a behaviour from an accepted " - + "proposal: `partial` entries say which half is missing, and this " - + "release has two. " + + "proposal: the spec text above includes proposals this implementation " + + "may not ship yet, and a `partial` entry says which half is missing. " + + "This release has two. " + "For the full spec text (execution model, error semantics, " + "determinism, observer hooks, etc.) see the linked docs site._" ), diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md index df020cf..3d3f955 100644 --- a/src/openarmature/AGENTS.md +++ b/src/openarmature/AGENTS.md @@ -10,7 +10,7 @@ OpenArmature is a workflow framework for LLM pipelines and tool-calling agents: ## Capability contracts -_Sourced from openarmature-spec v0.118.2. Each entry below reproduces §1 (Purpose) and §2 (Concepts) of the capability's `spec.md` verbatim — including additions from accepted proposals that this Python implementation may not yet ship. For per-proposal implementation status (implemented / partial / textual-only / not-yet), read the `conformance.toml` manifest. It ships inside this package, beside this file, so it resolves whether you installed from PyPI or are working in a clone: `importlib.resources.files('openarmature') / 'conformance.toml'`. Check it before planning against a behaviour from an accepted proposal: `partial` entries say which half is missing, and this release has two. For the full spec text (execution model, error semantics, determinism, observer hooks, etc.) see the linked docs site._ +_Sourced from openarmature-spec v0.118.2. Each entry below reproduces §1 (Purpose) and §2 (Concepts) of the capability's `spec.md` verbatim — including additions from accepted proposals that this Python implementation may not yet ship. For per-proposal implementation status (implemented / partial / textual-only / not-yet), read the `conformance.toml` manifest. Where it is depends on how you got this package. Installed from PyPI: beside this file, at `importlib.resources.files('openarmature') / 'conformance.toml'`. In a source checkout: at the repository root, since it is added to the distribution at build time rather than committed under `src/`. Check it before planning against a behaviour from an accepted proposal: the spec text above includes proposals this implementation may not ship yet, and a `partial` entry says which half is missing. This release has two. For the full spec text (execution model, error semantics, determinism, observer hooks, etc.) see the linked docs site._ ### Capability: `graph-engine` diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 301fd13..229ea5f 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -105,19 +105,25 @@ def test_the_conformance_manifest_is_force_included_in_the_wheel() -> None: assert source.exists(), f"force-include names a source that does not exist: {source}" -def test_agents_md_tells_an_agent_where_the_manifest_actually_is() -> None: - # The pointer used to read "at the repo root", which is unresolvable from a - # venv and is the half of the defect that made the missing file invisible: - # an agent that could not find it had no reason to think it should be there. +def test_agents_md_names_both_places_the_manifest_can_be() -> None: + # The manifest lives in two places depending on how the reader got the + # package, and a pointer naming only one is wrong for half of them. The + # original said "at the repo root", unresolvable from a venv. The first fix + # named only the packaged path, unresolvable in a clone, since force-include + # is build-time and nothing is committed under src/. Both were the same + # defect: telling an agent something untrue in its context. + # + # So the assertion is that BOTH are named, not that either is absent. bundled = Path(__file__).resolve().parent.parent / "src" / "openarmature" / "AGENTS.md" text = bundled.read_text() assert "conformance.toml" in text, "AGENTS.md must still point at the manifest" - assert "at the repo root" not in text, ( - "the manifest pointer must not be repo-relative: it resolves for a clone and " - "not for anyone who installed the package" - ) assert "importlib.resources" in text, ( - "the pointer should name the access path that works from an installed package" + "the pointer must name the packaged path, which is the only one that resolves " + "for someone who installed from PyPI" + ) + assert "repository root" in text, ( + "the pointer must also name the checkout location, which is the only one that " + "resolves in a clone: force-include is build-time, so nothing lands in src/" ) From 977ea7360e3f5d038bf49b85b0b9efc9fed94cbc Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sat, 26 Sep 2026 10:42:36 -0700 Subject: [PATCH 04/14] Drive the reask demo from a token ceiling The demo turned on a model sending "1,340 kg" where the schema wanted a number. A capable model does not make that mistake, and an endpoint that enforces the schema on the wire cannot: run against a local vllm with guided decoding and MODE=off succeeded identically to the reask arm, so the feature the example exists for never fired. An output ceiling below what a complete record needs truncates the reply every time, on any model, and arrives at the schema boundary as the same structured_output_invalid. Recovery then needs two different things, which is the more useful lesson: the corrective message says what went wrong, and a per-attempt override raises the ceiling so the retry has somewhere to put the answer. A third mode supplies the correction without the raised ceiling, so the difference is visible rather than asserted. The builder branches on finish_reason rather than guessing from the raw text, since that is the signal the framework already reports. Also stop the cleanup path constructing a provider in order to close one, which masked whatever failure kept the run from building it. --- examples/README.md | 33 ++--- examples/structured-output-reask/main.py | 163 ++++++++++++++++------- src/openarmature/AGENTS.md | 2 +- 3 files changed, 135 insertions(+), 63 deletions(-) diff --git a/examples/README.md b/examples/README.md index cd8e897..453f88e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -147,21 +147,24 @@ no-op on symmetric OpenAI, meaningful on asymmetric providers), mapping #### [`structured-output-reask/`](./structured-output-reask/main.py) -Extracting one structured mission record per lunar-landing report, and -correcting the model when it answers in the wrong shape. A strict JSON -schema asks for a numeric `mass_kg`, which is the constraint a model most -often breaks by echoing `"1,340 kg"` straight out of the prose. Rather than -failing the run or retrying an identical prompt that reproduces the -identical mistake, a caller-supplied builder quotes the model's own reply -and the validator's objection back to it and asks again. Demonstrates: -`complete(response_schema=...)` raising `StructuredOutputInvalid` instead -of returning a half-built object, `LlmRetryConfig(reask=...)` making that -failure retryable for one call, a builder reading `exc.output_content` and -`exc.error_message` to compose the correction, the framework appending the -model's reply and the correction as an alternating transcript while -authoring no prompt of its own, and reask sharing the `max_attempts` -budget with transient retries. `MODE=off` omits the builder so the first -invalid reply is terminal, which shows what the builder buys. +Extracting one structured mission record per lunar-landing report, under an +output-token cap that is too tight for a complete record. The model stops +mid-object, and the fragment that arrives is rejected at the schema boundary +exactly as a wrong-typed field would be. Recovery needs two different changes: +a caller-supplied builder quotes the model's own fragment and the reader's +objection back to it, and a per-attempt override raises the ceiling so the +retry has somewhere to put the answer. Demonstrates: +`complete(response_schema=...)` raising `StructuredOutputInvalid` instead of +returning a half-built object, `LlmRetryConfig(reask=...)` making that failure +retryable for one call, a builder reading `exc.output_content` and +`exc.error_message` and branching on which failure it got, +`LlmRetryConfig(per_attempt_override=...)` applying a config schedule to +retries only, the framework appending the model's reply and the correction as +an alternating transcript while authoring no prompt of its own, and reask +sharing the `max_attempts` budget with transient retries. Three postures: +`MODE=off` supplies neither and ends on the first unusable reply, `MODE=nocap` +supplies the correction but not the raised ceiling so every attempt is cut off +at the same place, and the default supplies both. #### [`checkpointing-and-migration/`](./checkpointing-and-migration/main.py) diff --git a/examples/structured-output-reask/main.py b/examples/structured-output-reask/main.py index cf5bfab..cf1ceca 100644 --- a/examples/structured-output-reask/main.py +++ b/examples/structured-output-reask/main.py @@ -1,54 +1,74 @@ """openarmature demo: pull a structured mission record out of a prose -lunar-landing report, and correct the model when it answers in the wrong -shape. +lunar-landing report, and recover when the reply arrives unusable. **Use case:** A feed delivers lunar-landing reports as free prose. You want one row per report: mission, operator, landing site, outcome, and the mass delivered in kilograms. A JSON schema says exactly that, and the provider is asked to honour it. -Models miss. They wrap JSON in a markdown fence, answer in prose because the -report was ambiguous, invent a field, or send ``"1,340 kg"`` where the schema -says a number. The useful move is not to fail the run and not to retry the -identical prompt, which reproduces the identical mistake. It is to show the -model what it returned, say what was wrong with it, and ask again. +You also cap output tokens, because the records are small and you pay by the +token. That cap is a guess about the longest record you will ever need, and +the guess is sometimes wrong. When it is, the model stops mid-object and the +reply that arrives is a fragment: valid so far, parseable as nothing. The +schema boundary rejects it exactly as it rejects a model that answered in +prose or sent ``"1,340 kg"`` where a number was required. -That is what ``reask`` is: you supply the corrective message, because only you -know how to talk to your model about your schema. The framework supplies the -loop, the transcript, and the budget. +Retrying the identical request reproduces the identical fragment, because +nothing about the request changed. Two things have to change, and they are +different kinds of thing: + +- **What you say.** Show the model what came back and what was wrong with it. + That is ``reask``: you supply the corrective message, because only you know + how to talk to your model about your schema. +- **What it is allowed to spend.** A correction cannot help a reply that gets + cut off at the same place. That is ``per_attempt_override``: the retry runs + under a raised ceiling. + +Supply only the first and the retry is better informed and still truncated. +This demo shows both arms so the difference is visible. **What's interesting in the implementation:** - ``complete(response_schema=...)`` asks the provider for a shape. When the reply cannot be parsed or does not validate, the call raises ``StructuredOutputInvalid`` rather than handing back a half-built object. + A truncated reply and a wrong-typed field arrive through the same door. - ``LlmRetryConfig(reask=...)`` makes that failure retryable FOR THIS CALL. Without a builder it is terminal, which is the right default: a schema the model cannot satisfy is usually a bug in the schema, not a transient. - The builder receives the exception and returns the correction as a string. - It reads ``exc.output_content`` (verbatim, what the model actually sent) and - ``exc.error_message`` (what the validator objected to). Quoting both back is - what makes the second attempt different from the first. + It reads ``exc.output_content`` (verbatim, what the model actually sent), + ``exc.error_message`` (what the reader objected to), and + ``exc.finish_reason`` (``"length"`` when the ceiling ended the reply). This + one branches on that last signal and says something different about a + truncation than about a model that simply answered wrongly, which is the + kind of judgement only the caller can make. +- ``LlmRetryConfig(per_attempt_override=...)`` is a schedule of partial + configs applied to retries only. Attempt 0 runs the caller's config + untouched; each retry merges the next entry over it. A schedule shorter + than the retry count carries its last entry forward. - The framework appends the model's own reply as an ``assistant`` turn and the builder's string as a ``user`` turn, so the conversation stays - role-alternating and the model sees its own mistake in context. It authors no - prompt of its own; every word sent is yours. + role-alternating and the model sees its own fragment in context. It authors + no prompt of its own; every word sent is yours. - Reask shares the ``max_attempts`` budget with transient retries. A call that - burns two attempts on malformed output has one left for a rate limit. -- ``MODE`` selects the posture. ``"reask"`` (default) supplies the builder. - ``"off"`` omits it, so the first invalid reply ends the call and you can see - what the builder is buying. + burns two attempts on unusable output has one left for a rate limit. **Run it:** export LLM_API_KEY=sk-... uv run python examples/structured-output-reask/main.py - MODE=off uv run python examples/structured-output-reask/main.py + MODE=nocap uv run python examples/structured-output-reask/main.py + MODE=off uv run python examples/structured-output-reask/main.py -Point ``LLM_BASE_URL`` at any OpenAI-compatible endpoint. ``LLM_MODEL`` -defaults to a small fast model; a weaker one makes the reask path fire more -often, which is the interesting case to watch. +Three postures. ``reask`` (default) supplies the builder and raises the cap. +``nocap`` supplies the builder and leaves the cap alone, so every attempt is +cut off at the same place and the budget drains. ``off`` supplies neither, so +the first unusable reply ends the call. + +Point ``LLM_BASE_URL`` at any OpenAI-compatible endpoint (the host root, not +its ``/v1`` path). ``LLM_MODEL`` defaults to a small fast model. """ from __future__ import annotations @@ -93,9 +113,8 @@ ), ] -# The shape we want back. Deliberately strict: `mass_kg` is a number, not a -# string, which is the constraint a model is most likely to break by sending -# "1,340 kg" verbatim from the prose. +# The shape we want back. `mass_kg` is a number rather than a string, which is +# the constraint a model breaks by copying "1,340 kg" out of the prose. MISSION_SCHEMA: dict[str, Any] = { "type": "object", "properties": { @@ -113,6 +132,12 @@ "You extract one structured mission record from a lunar-landing report. Answer with a JSON object only." ) +# The cap the app runs under, and the one a retry is allowed to climb to. The +# tight cap sits below what a complete record of this shape needs, so it is +# the ceiling rather than the model that makes the first reply unusable. +TIGHT_MAX_TOKENS = 32 +ROOMY_MAX_TOKENS = 256 + # --------------------------------------------------------------------------- # The reask builder. This is the whole point of the example. @@ -120,20 +145,35 @@ def build_correction(exc: StructuredOutputInvalid) -> str: - """Compose the corrective message sent after an invalid reply. + """Compose the corrective message sent after an unusable reply. Receives the failure and returns what to say about it. The framework appends the model's own reply before this, so it reads as a conversation: the model answered, you point at the problem, it answers again. """ - # Both halves matter. The verbatim output lets the model see what it - # actually sent rather than what it meant to send, and the validator's - # complaint names the constraint it missed. A correction carrying neither - # is just the original prompt again, which reproduces the original answer. + # Both halves of the exception matter. The verbatim output lets the model + # see what it actually sent rather than what it meant to send, and the + # validator's complaint names the constraint it missed. A correction + # carrying neither is the original prompt again, which earns the original + # answer. + raw = exc.output_content.strip() + # A reply that stops mid-object is a spend problem, not a comprehension + # problem, and saying "that did not match the schema" about it would be + # misleading. `finish_reason` is how the framework reports which one + # happened: "length" means the ceiling ended the reply, so the model was + # never given the chance to be wrong. + if exc.finish_reason == "length": + return ( + "That reply stopped before the object closed, so it could not be " + f"read. The reader said: {exc.error_message}\n\n" + f"You sent:\n{raw}\n\n" + "Send the whole object this time. Keep every value as short as " + "the report allows and add nothing beyond the required fields." + ) return ( - f"That reply did not match the required schema. The validator said: " + "That reply did not match the required schema. The validator said: " f"{exc.error_message}\n\n" - f"You sent:\n{exc.output_content}\n\n" + f"You sent:\n{raw}\n\n" "Send the JSON object again, corrected. Return only the object, with " "no surrounding prose and no markdown fence. `mass_kg` must be a bare " 'number in kilograms, so write 1340 rather than "1,340 kg". ' @@ -159,6 +199,14 @@ def _get_provider() -> OpenAIProvider: return _provider_instance +async def _close_provider() -> None: + # Only close what was built. Calling the constructor here to obtain + # something to close would mask whatever failure kept the run from + # building one. + if _provider_instance is not None: + await _provider_instance.aclose() + + class FeedState(State): reports: list[str] = [] records: list[dict[str, Any]] = [] @@ -167,17 +215,22 @@ class FeedState(State): def _retry_config(mode: str) -> LlmRetryConfig: # `reask` is what makes a schema failure retryable at all. Omit it and the - # first invalid reply is terminal, which is the useful default: retrying an - # unchanged prompt against an unchanged model gets the same answer. + # first unusable reply is terminal, which is the useful default: retrying + # an unchanged prompt against an unchanged model gets the same answer. + # + # The override is the other half. Without it the retry is better informed + # and still capped at the length that truncated it, so `nocap` spends the + # whole budget re-learning the same lesson. return LlmRetryConfig( max_attempts=3, backoff=deterministic_backoff(0.0), - reask=build_correction if mode == "reask" else None, + reask=build_correction if mode in {"reask", "nocap"} else None, + per_attempt_override=([RuntimeConfig(max_tokens=ROOMY_MAX_TOKENS)] if mode == "reask" else None), ) async def extract(s: FeedState) -> Mapping[str, Any]: - """Extract one record per report, tolerating a wrong-shaped first reply.""" + """Extract one record per report, recovering from an unusable first reply.""" provider = _get_provider() retry = _retry_config(os.environ.get("MODE", "reask")) records: list[dict[str, Any]] = [] @@ -187,15 +240,16 @@ async def extract(s: FeedState) -> Mapping[str, Any]: try: response = await provider.complete( [SystemMessage(content=_EXTRACT_SYSTEM), UserMessage(content=report)], - config=RuntimeConfig(temperature=0.0), + config=RuntimeConfig(temperature=0.0, max_tokens=TIGHT_MAX_TOKENS), response_schema=MISSION_SCHEMA, retry=retry, ) except StructuredOutputInvalid as exc: - # Reached when the budget runs out, or immediately when MODE=off. - # The exception carries the last thing the model sent and why it - # was rejected, which is what you want in the log. - failures.append(f"{exc.error_message}: {exc.output_content[:120]}") + # Reached when the budget runs out, and immediately when no + # builder was supplied. The exception carries the last thing the + # model sent and why it was rejected, which is what belongs in + # the log. + failures.append(f"{exc.error_message}: {exc.output_content[:90]}") continue records.append(json.loads(response.message.content or "{}")) @@ -218,18 +272,29 @@ def build_graph() -> CompiledGraph[FeedState]: ) +_POSTURE = { + "reask": "corrective message + raised token ceiling", + "nocap": "corrective message, ceiling left alone", + "off": "neither", +} + + async def main() -> None: mode = os.environ.get("MODE", "reask") print("=== openarmature structured-output-reask demo ===") - print(f"mode: {mode}" + (" (no corrective builder)" if mode == "off" else "")) + print(f"mode: {mode} ({_POSTURE.get(mode, 'unknown mode')})") print(f"reports: {len(REPORTS)}") + print( + f"cap: {TIGHT_MAX_TOKENS} output tokens" + + (f", raised to {ROOMY_MAX_TOKENS} on retry" if mode == "reask" else "") + ) print() graph = build_graph() try: final = await graph.invoke(FeedState(reports=REPORTS)) finally: - await _get_provider().aclose() + await _close_provider() await graph.drain() for record in final.records: @@ -239,15 +304,19 @@ async def main() -> None: f"{record.get('mass_kg')} kg" ) if final.failures: - print() print(" unrecovered:") for f in final.failures: print(f" {f}") print() print(f"extracted {len(final.records)} of {len(REPORTS)}") - if mode == "off" and final.failures: - print("Run without MODE=off to let the corrective builder retry these.") + if mode == "off": + print("MODE=nocap adds the corrective message. Default adds the ceiling too.") + elif mode == "nocap": + print( + "The model was told what went wrong and still had nowhere to put " + "the answer. Run without MODE to raise the ceiling as well." + ) if __name__ == "__main__": diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md index 3d3f955..ba416bc 100644 --- a/src/openarmature/AGENTS.md +++ b/src/openarmature/AGENTS.md @@ -1694,7 +1694,7 @@ _Runnable example programs shipped in the source tree at `examples/`. The full c - **`examples/provider-extras/main.py`** — openarmature demo: reach a vendor knob openarmature does not model, and watch the guardrails that stop you reaching the wrong one. - **`examples/retrieval-rag/main.py`** — Retrieval-augmented answering over a lunar knowledge base. - **`examples/routing-and-subgraphs/main.py`** — openarmature demo: conditional routing + subgraph with a custom projection. -- **`examples/structured-output-reask/main.py`** — openarmature demo: pull a structured mission record out of a prose lunar-landing report, and correct the model when it answers in the wrong shape. +- **`examples/structured-output-reask/main.py`** — openarmature demo: pull a structured mission record out of a prose lunar-landing report, and recover when the reply arrives unusable. - **`examples/tool-use/main.py`** — openarmature demo: a lunar-mission assistant that calls local Python functions as tools to answer fact and physics questions about Apollo / Artemis missions. ## Discovery cross-references From ac02288b319cf02c7f022a02bfc482ea6437863e Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sat, 26 Sep 2026 10:44:15 -0700 Subject: [PATCH 05/14] Stop a merged extras key from killing a retry Reading llm-provider 7.1's per-key merge correctly introduced a regression that no test caught. A base extras key is the section 6 escape hatch: unmanaged where it was written, because the base leaves the declared field unset so the mapping emits nothing of that name. Merging carried it onto a retry whose override does set the field, where it is managed, so section 8.1 rejected the call before it went out. Measured against a transport returning 503 on every call with max_attempts=3: base extras temperature plus an override declaring temperature ran one wire call and surfaced provider_invalid_request, where the replaced-container rule ran three and surfaced the transient the retry existed to ride out. The original error was not even chained, because the failure happens in the body build outside the attempt's try. A field the override declares now supersedes an inherited base extras key of the same name. The merge must not manufacture an invalid config out of two valid ones, and the override's field is the later and more specific instruction. A key the override names alongside its own declared field of that name still collides, since that config contradicts itself in a single object. Also correct claims that were wrong or went stale, several of them in the commit that was fixing other wrong claims: - The CHANGELOG said 0085 extends exactly-once to nested fan-outs while the manifest said it does not. Nothing writes the lineage onto a crash record, so a completed inner instance re-runs and its side effects happen twice. The manifest note asserted both readings itself. - The manifest called the old "66 / 76" citation nonexistent. Those were line numbers written as section marks, and they point at the write-side out-of-scope paragraph and the parallel-branches deferral. The reference was real; only the notation was wrong. - docs/concepts/llms.md and the 0122 CHANGELOG entry still documented wholesale replacement and a warning that no longer exists. An override may set an extras key to null, which sends null rather than withdrawing the key, so the previous claim that withdrawal is inexpressible overstated it. - The 0095 entry still named raw_content and failure_description. - The 0098 note still described the alias map that the rename deleted. A reask builder that raises now logs which of the two failed, since the chained cause reads as a bad model rather than a broken builder. That also gives the module's orphaned logger a consumer. --- CHANGELOG.md | 8 +- conformance.toml | 4 +- docs/concepts/llms.md | 24 ++++-- src/openarmature/llm/errors.py | 6 +- src/openarmature/llm/providers/openai.py | 41 +++++++-- tests/unit/test_llm_provider.py | 103 ++++++++++++++++++++++- tests/unit/test_observability_otel.py | 18 ++-- 7 files changed, 166 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef5796d..2886bea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,16 +17,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **Advisory per-prompt token-budget observability** (proposal 0083, observability §5.5.15 / §7 / §8.4.3 / §11.2, spec v0.78.0). An optional `TokenBudget` (`input_max_tokens` / `total_max_tokens`, each non-negative and individually optional) rides `Prompt` / `PromptResult`, sourced from the same config sidecar as sampling (filesystem and Langfuse backends), and is carried on `LlmCompletionEvent` / `LlmFailedEvent` / `LlmRetryAttemptEvent`. It is advisory and observability-only: the budget never touches the LLM request, and nothing is blocked or truncated. The OTel LLM span gains `openarmature.prompt.token_budget.input_max_tokens` / `.total_max_tokens` (each only when declared) plus a reactive `openarmature.llm.token_budget.exceeded` boolean (emitted only when a bound is declared and usage is present; absent, not false, otherwise). Two opt-in §11.2 instruments record the breach: an `openarmature.gen_ai.client.token_budget.exceeded` counter (per breached bound) and a `.utilization` histogram (actual / max per declared bound), both dimensioned by `openarmature.gen_ai.token_budget.kind` (`input` / `total`). On a breach the Langfuse success Generation ends at `WARNING` with a `statusMessage` naming the bound(s) and `metadata.token_budget` (a hard `ERROR` on the failure path still wins), and each over-budget attempt logs a §7 `WARNING` on the `openarmature.observability` logger, dispatched independently of the span and metrics gates. A bound is evaluated only when its reported count is present (a missing count is never coerced to `0`); a declared bound of `0` is exceeded by any positive usage (strict `>`), and its utilization sample is skipped (ratio undefined). Evaluation also fires on a usage-bearing `structured_output_invalid` failure. Conformance fixtures 126-131. - **OTel GenAI metrics extended to embedding and rerank calls** (proposals 0067 / 0060, observability §11, spec v0.68.0 / v0.70.0). The two §11 histograms shipped for LLM completions in v0.15.0 now also record embedding and rerank provider calls, onto the same instruments with the `openarmature.gen_ai.operation` dimension separating `chat` / `embeddings` / `rerank`. `_record_embedding_metrics` records from the terminal `EmbeddingEvent` / `EmbeddingFailedEvent` (input tokens only, `EmbeddingUsage.input_tokens`), and `_record_rerank_metrics` from `RerankEvent` / `RerankFailedEvent` (input tokens only; `search_units` is a billing unit, not a token, so it never records a `token.usage` observation). Both record `operation.duration` on every call including a failure (carrying `error.type`), and a `token.usage` observation only when a token count is reported. This completes proposal 0067, whose embedding-call metrics were deferred in v0.15.0 pending the embedding capability, and the §11 arm of the rerank observability surface (proposal 0060). Conformance fixtures 089 / 143 (embedding) and 109 (rerank) are un-deferred. - **Nested-fan-out span lineage** (proposal 0084, graph-engine §6 + observability §4.3 / §5.5 / §8.4, spec v0.81.0). The observer event surface gains `fan_out_index_chain` / `branch_name_chain` (the enclosing fan-out-instance / parallel-branch lineage, outermost to innermost, aligned to `namespace`) on the provider and tool events (`LlmCompletionEvent` / `LlmFailedEvent` / `LlmRetryAttemptEvent` / `EmbeddingEvent` / `EmbeddingFailedEvent` / `RerankEvent` / `RerankFailedEvent` / `ToolCallEvent` / `ToolCallFailedEvent`) and the framework `FailureIsolatedEvent`, mirroring `NodeEvent` and populated from the enclosing-lineage context at call time. Both bundled observers now key their node span / observation by the scalars plus the enclosing chains, so an inner node running under two concurrent outer fan-out instances no longer collides (no dropped or mis-closed spans / observations), while a callable parallel-branch stays distinct from its parallel-branches node. Provider-span parenting follows the same lineage: the exact-match parent resolves under the lineage-disambiguated calling-node span / observation, and the §5.5 orphan fallback (a wrapper- or middleware-issued provider call with no open calling-node span) resolves in both observers to the nearest enclosing wrapper on the calling node's lineage via a shared resolver, which also backs `FailureIsolatedEvent` parenting. Conformance fixtures 132-134. Known limitation: a pre-phase orphan call (a wrapper call draining before the inner instance observation exists) can resolve to different enclosing instances across the two observers; this cross-observer parity edge is documented and tracked, and the post-phase realization the fixtures drive holds parity (asserted by a live OTel-vs-Langfuse parent comparison). -- **Nested-fan-out checkpoint resume** (proposal 0085, pipeline-utilities §10.11 / §10.7 / §10.2, spec v0.80.0). Completes the resume consume-side of nested-fan-out checkpointing (the save-side in-memory lineage keying shipped in v0.16.0). `FanOutProgress` gains an optional `enclosing_fan_out_lineage` (a sequence of `EnclosingFanOutInstance` = `{namespace, fan_out_node_name, fan_out_index}`, the outermost-to-innermost chain of enclosing fan-out instances), and the record restore keys each fan-out's tracking entry by it. This realizes the §10.11 no-mis-skip invariant and extends §10.11.1 exactly-once to nested fan-outs: on resume, a fan-out nested inside an in-flight outer instance skips only the inner instances completed under its own lineage (a positive per-lineage match), and never applies a different enclosing instance's or a legacy no-lineage entry's completed skips (which would roll a different instance's results into the wrong accumulator); an unmatched entry re-runs from scratch, which is correctness-preserving. Backward-compatible: a flat or pre-0085 (empty-lineage) record resumes exactly as before, and the SQLite json serializers round-trip the new field. The write side (emitting the lineage onto a crash-produced record) remains a tracked follow-up; until then a real nested-fan-out crash resumes at the safe re-run floor. Conformance fixture 076 is un-deferred. +- **Nested-fan-out checkpoint resume** (proposal 0085, pipeline-utilities §10.11 / §10.7 / §10.2, spec v0.80.0). Completes the resume consume-side of nested-fan-out checkpointing (the save-side in-memory lineage keying shipped in v0.16.0). `FanOutProgress` gains an optional `enclosing_fan_out_lineage` (a sequence of `EnclosingFanOutInstance` = `{namespace, fan_out_node_name, fan_out_index}`, the outermost-to-innermost chain of enclosing fan-out instances), and the record restore keys each fan-out's tracking entry by it. This realizes the §10.11 no-mis-skip invariant: on resume, a fan-out nested inside an in-flight outer instance skips only the inner instances completed under its own lineage (a positive per-lineage match), and never applies a different enclosing instance's or a legacy no-lineage entry's completed skips (which would roll a different instance's results into the wrong accumulator); an unmatched entry re-runs from scratch, which is correctness-preserving. Backward-compatible: a flat or pre-0085 (empty-lineage) record resumes exactly as before, and the SQLite json serializers round-trip the new field. The write side (emitting the lineage onto a crash-produced record) remains a tracked follow-up, so 0085 ships as `partial`: no record the engine writes carries a lineage, nothing matches on resume, and every inner instance re-runs. That is the safe floor for state, because an unmatched entry never applies another instance's skips, but it is not exactly-once: a completed inner instance runs a second time, and any side effect it performs happens twice. §10.11.1's exactly-once guarantee therefore does not yet extend to nested fan-outs. Conformance fixture 076 is un-deferred. - **Langfuse parallel-branches mapping parity** (proposal 0088, observability §8.4.8 / §8.3 / §8.4.2 / §3.4, spec v0.83.0). Brings the Langfuse observer's parallel-branches rendering to parity with the OTel side. The observer already synthesized the three-level Observation tree (the parallel-branches node Span, a per-branch dispatch Span named by the `branch_name`, and the branch's inner observations) and already emitted the dispatch-span `parallel_branches_parent_node_name` and `branch_name`; the two node-span attributes `parallel_branches_branch_count` and `parallel_branches_error_policy` are now flattened onto the node Span's `observation.metadata` (mirroring the `fan_out_*` attributes), the one §8.4.2 row the observer had never mapped. The three `parallel_branches_*` keys join the reserved caller-metadata set (26 to 29), so a caller passing one as invocation metadata is rejected at the `invoke()` boundary rather than shadowing the OA-emitted field. The OTel side was already complete. Conformance fixture 136 (the dedicated three-level-tree pin) is un-deferred; fixture 030's incidental coverage stands. - **Adaptive call-level retry: per-attempt request override** (proposal 0095, llm-provider §7.1, spec v0.91.0). The LLM-completion call-level retry loop gains an opt-in per-attempt request override. A new `LlmRetryConfig` (the llm-provider-scoped superset of the generic `RetryConfig`, exported from `openarmature.llm`) carries a `per_attempt_override`: a schedule of `RuntimeConfig` partials applied to retries. Attempt 0 uses the caller's base `config` unchanged; retry `i` merges `per_attempt_override[i]` onto the base (the override's non-None fields replace; a None or unspecified field inherits the base, per the §6 null-skip semantics), and the last entry carries forward when the schedule is shorter than the retry count. The canonical use is an escalating temperature schedule that breaks the "temperature 0 replays the same output" determinism trap on a retried structured-output call. `complete()` never mutates the caller's `config` (each attempt config is a fresh copy), and a plain `RetryConfig` preserves the existing byte-identical replay. The per-attempt OTel span carries a new `openarmature.llm.retry_reason` attribute (`transient`) on retries, absent on the base attempt. This is the first half of proposal 0095; the structured-output reask half follows. The behavior shipped ahead of the pin (spec v0.91.0 against a v0.88.0 pin) and is ratified at the v0.118.2 pin. Fixtures 061-066 run against an OTel observer rather than the generic llm-provider harness, which has none: they assert the per-attempt spans as well as the per-attempt outbound requests. -- **Adaptive call-level retry: structured-output reask** (proposal 0095, llm-provider §7.1, spec v0.91.0). The second half of 0095. `LlmRetryConfig` gains an opt-in `reask` builder (`Callable[[StructuredOutputInvalid], str]`). When present, a `structured_output_invalid` failure becomes retryable for that call (a call-level convenience, not a classifier change; without a builder it stays non-transient and raises on the first occurrence). On each such failure the loop appends two messages to a working transcript, the model's raw output as an `assistant` message and the builder's returned correction as a `user` message, so the retry is informed rather than a byte-identical replay. OA authors no prompt of its own (the caller owns every word beyond the model's output); the builder receives the raised `StructuredOutputInvalid` (its `raw_content` and `failure_description`). The transcript accumulates reask pairs across reask retries and consumes the `max_attempts` budget; a transient retry interleaved in a reask loop re-sends the accumulated transcript unchanged. `complete()` never mutates the caller's `messages` (each reask replaces the transcript with a fresh list rather than appending in place). The retry span's `openarmature.llm.retry_reason` is `reask` on a reask retry, `transient` otherwise. A reask always appends the model output as a fresh `assistant` message (never continues a trailing one): §3 requires the last message before a call to be `user`/`tool`, so the transcript never ends in `assistant`. Shipped ahead of the pin and ratified at v0.118.2; fixtures 062-066 run alongside 061 in the observer-backed runner. +- **Adaptive call-level retry: structured-output reask** (proposal 0095, llm-provider §7.1, spec v0.91.0). The second half of 0095. `LlmRetryConfig` gains an opt-in `reask` builder (`Callable[[StructuredOutputInvalid], str]`). When present, a `structured_output_invalid` failure becomes retryable for that call (a call-level convenience, not a classifier change; without a builder it stays non-transient and raises on the first occurrence). On each such failure the loop appends two messages to a working transcript, the model's raw output as an `assistant` message and the builder's returned correction as a `user` message, so the retry is informed rather than a byte-identical replay. OA authors no prompt of its own (the caller owns every word beyond the model's output); the builder receives the raised `StructuredOutputInvalid` (its `output_content` and `error_message`). The transcript accumulates reask pairs across reask retries and consumes the `max_attempts` budget; a transient retry interleaved in a reask loop re-sends the accumulated transcript unchanged. `complete()` never mutates the caller's `messages` (each reask replaces the transcript with a fresh list rather than appending in place). The retry span's `openarmature.llm.retry_reason` is `reask` on a reask retry, `transient` otherwise. A reask always appends the model output as a fresh `assistant` message (never continues a trailing one): §3 requires the last message before a call to be `user`/`tool`, so the transcript never ends in `assistant`. Shipped ahead of the pin and ratified at v0.118.2; fixtures 062-066 run alongside 061 in the observer-backed runner. - **Langfuse observer: credentials-in construction with tracer-provider isolation** (proposals 0114 + 0116 + 0117 + 0118, observability §6 / §8.9 / §8.4, spec v0.108.0 / v0.110.0 / v0.111.0 / v0.112.0). The Langfuse observer gains a second construction mode alongside today's caller-supplied client: `LangfuseObserver.from_credentials(public_key=..., secret_key=..., host=...)` (over the lower-level `LangfuseSDKAdapter.from_credentials(...)`) builds an OA-owned `Langfuse` client on a dedicated `TracerProvider` by default, so its observations no longer bind the global provider and leak onto the application's OTel backend. A Langfuse v4 client constructed with no `tracer_provider=` attaches its span processor to the globally-registered provider, so in any service that registers a global provider (the standard app-tracing setup) attaching the Langfuse observer silently exported every observation, prompts and completions included, to the app backend. Because the Langfuse SDK caches one client per `public_key`, a dedicated provider takes effect only when OA is the first constructor for that credential; OA reuses one isolated provider per credential and reads the actual binding back after construction. The invariant covers every payload OA harvests from the runtime -- the provider payload (`disable_provider_payload`), the Trace-level state input/output (`disable_state_payload` and the `trace_input_from_state` / `trace_output_from_state` hooks), and a failed Tool / Embedding / Retriever / LLM observation's `error_message` -- but not the dimensions the caller deliberately attaches (`correlation_id` / `session_id` / `userId` / trace name / caller metadata), which stay verbatim as cross-backend join keys. When any construction-determinable channel is live and OA establishes the client is bound to a provider it did not isolate, construction fails loud with a categorized `LangfuseProviderIsolationUnavailable` before any observation is emitted, rather than leaking payloads to a shared backend; where OA cannot establish the binding at all (a future SDK), it suppresses every channel and logs a warning. A failed observation's `error_message` is harvested exception text, so `disable_provider_payload` governs it for every failure category on all four provider observations: with payloads off it is not rendered, and the error category still rides as the status message where the event carries one. A Tool failure has no category, so its status message is null rather than falling back to the exception string. `error_type` is a classification token rather than harvested content and is never gated, which matters most for a Tool failure where it is the only remaining discriminator; it is optional, so it is emitted only where the failure event supplies one. A single `accept_shared_provider=True` opt-out turns the whole thing into a warn-and-proceed onto the shared provider. With no channel live (the default privacy posture), an un-isolatable client neither raises nor warns. The existing caller-supplied path (mode a) is unchanged and never mutated: a caller who builds their own client stays responsible for isolating its `tracer_provider`, and OA documents the remedy rather than reaching into the supplied client. The `secret_key` is accepted as a `pydantic.SecretStr`, masked in OA's own reprs and logs with the plaintext read only at the SDK call (`public_key` and `host` stay plain strings), and a blank credential is rejected at the boundary rather than falling through to the SDK's ambient `LANGFUSE_*` environment fallback. A `sample_rate` passed for the client is applied to the isolated provider, since the SDK only honors it on a provider it builds itself. `accept_shared_provider` binds the provider the application already registered rather than letting the SDK construct and globally register one of its own, which would capture OTel's single-assignment global slot. The new `LangfuseProviderIsolationUnavailable` derives from an `ObservabilityError` base, a fourth hierarchy alongside the graph-engine, llm-provider, and checkpoint ones. The behavior shipped ahead of the pin, and the pin has since advanced past it to v0.118.2. Fixture 159 now runs, and fixtures 098 / 137 / 138 are un-deferred and reconciled to the post-0118 shape. Fixtures 157 and 158 both run. The conformance-adapter provider-faithful double and the `langfuse_client` construction directive are built for both modes: `supplied` uses a protocol-shaped fake, and `credentials` uses a real client with its egress removed, because openarmature wraps its own client in an adapter that drives the SDK's private surface. 158 asserts five of its eleven cases, with five recognized skips against a detection-capable declaration and one case deferred by name. - **Failed-observation `error_message` byte cap, and the `openarmature_` reserved namespace** (proposal 0119, observability §5.5.5 / §8.7 / §3.4, spec v0.116.0). Closes the two error-channel edges 0118 left open. **The cap:** a failed observation's `error_message` is now subject to the §5.5.5 per-value byte cap, on all four mapped provider observations (Generation, Embedding, Tool, Retriever). It takes the contract's *direct-application* arm rather than its *inheritance* arm: the OTel surface defines no `error_message` span attribute, so the value arrives untruncated and the observer that writes it applies its own `payload_byte_cap`, rather than inheriting a cap from an upstream OTel truncation that never happened. Re-applying a second cap to an already-truncated value would move the marker and misreport its byte total, which is why the two arms source the cap differently. A failed Tool observation renders the same harvested string twice, once in `metadata.error_message` and once as the observation's `statusMessage`, and **both** copies are capped: §5.5.5 governs payload-classified values rather than payload-classified fields, so capping one surface while the other still carried the whole exception would defeat the cap. The remaining `statusMessage` writes take the error *category*, a classification token, and stay uncapped. Under the default posture (`disable_provider_payload=True`) the field is absent entirely per 0118, so the cap is observable only where payloads are enabled, and the omission arm is unchanged: a withheld message still leaves `statusMessage` null on a Tool failure rather than substituting the message. **The reserved keys:** `openarmature_` joins `openarmature.` and `gen_ai.` as a reserved caller-metadata *namespace* prefix, and four exact names (`error_type`, `error_message`, `token_budget`, `token_budget_exceeded`) join the reserved set, which grows from 29 to 33. With `error_message` absent under the default posture, an unreserved caller key of that name would otherwise land unopposed in the very field 0118 requires to be absent, reintroducing through the metadata channel the leak the gate closes. Pre-1.0 behavioral change: a caller passing `invocation_metadata` with a key beginning `openarmature_`, or with any of those four names, is now rejected at the `invoke()` boundary with `ValueError` where it previously passed. The behavior shipped ahead of the pin (spec v0.116.0 against a v0.112.0 pin) and the `conformance.toml` entry is in place at v0.118.2. Fixture 160 does not yet run: its six cases span embedding, LLM, rerank and tool calls, and no runner here builds more than one node type, so it waits on a driver that does. Its directives are built (`message_repeat`, the `metadata_truncation` assertion and `langfuse_observer.payload_byte_cap`), and the source behavior is covered meanwhile by the unit suite. **The maintenance check 0119 asks for lands with it:** a test that statically scans the Langfuse mapping for every top-level metadata key it writes and fails when one is neither reserved by name nor covered by a reserved namespace. It discovers the metadata bags from what is passed to a `metadata=` argument rather than hardcoding one name, because the observer builds five of them and a scan of the obvious one would miss four; 0119 asks for this specifically, since a sweep that looked at only a subset is what missed nine keys before. The check found three keys on the failure-isolation marker span (`error_category`, `failure_isolation_event_name`, `failure_isolation_node`) that a caller key still overwrites, because that handler merges caller metadata last. Those are held rather than reserved unilaterally: the marker is a graph-mechanism span that no mapping table covers, so whether the reserved set should reach an unmapped span was raised as a spec question rather than settled here. Spec has since ruled that it does not, and committed the forthcoming failure-isolation mapping to carry the three keys; see the precedence fix under Fixed. **Truncation is also now surrogate-safe:** the cap runs on the failure path, where the value is harvested exception text and is the likeliest string in the observer to carry a lone surrogate (an `OSError` naming a surrogateescape-decoded path). Encoding one raises, an observer that raises is only warned about rather than logged, and the cap runs before the observation is created, so an unguarded encode would have deleted the very observation reporting the failure. A malformed message now degrades that one field and still respects the cap. ### Changed -- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. **On a retried call**, `extras` follows the same rule the declared fields follow, with one wrinkle: its default is an empty container rather than `None`, so an override declaring no extras counts as unspecified and inherits the base's, while one declaring any replaces them wholesale. Any base key the override does not carry is logged as not sent on that attempt, since replacing discards it. Clearing extras for a single attempt is not expressible, because empty is how inheriting is spelled. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Two fixtures come off the deferred list as a direct result: llm-provider 075 and retrieval-provider 052, both held because the same-name reject looked unreachable through the real caller path. Both now run and are mutation-verified against the reject. Two smaller behaviors move with the container. A Langfuse `prompt.config` now lifts an `extras` sub-object onto `Prompt.sampling`, so a vendor knob reaches it from that source as it already did from a filesystem sidecar; the full config still rides `Prompt.metadata` either way. And a filesystem sidecar's unrecognized top-level key is now filtered rather than fatal, matching what the `token_budget` path and the Langfuse backend already did: with the config rejecting undeclared names, splatting the sidecar verbatim would turn one stray key in an operator-authored file into an error escaping `fetch()`, which is neither of the two documented error types and so would bypass the manager's multi-backend fallback. A vendor knob written flat in a sidecar is one such key, so it is filtered and does not reach `sampling.extras`. The behavior shipped ahead of the pin (spec v0.117.0 against a v0.112.0 pin) and is ratified at the v0.118.2 pin: the `conformance.toml` entry is in place and retrieval-provider fixtures 054 / 055 / 056 run. +- **Undeclared provider fields move into an `extras` container on the runtime configs** (proposal 0122, llm-provider §6 + retrieval-provider §6 / §8.4, spec v0.117.0). `RuntimeConfig`, `EmbeddingRuntimeConfig` and `RerankRuntimeConfig` (and `SamplingConfig`, which derives from the first) gain an `extras` mapping field and move from `extra="allow"` to `extra="forbid"`. **Breaking, in the pre-1.0 sense:** `RuntimeConfig(temperature=0.2, guided_decoding={...})` now raises, and the same call is written `RuntimeConfig(temperature=0.2, extras={"guided_decoding": {...}})`. Declared fields are unchanged. §6 had left the extras surface's shape unstated, and read flat it made one of 0108's collision arms unreachable: a caller could not set a declared field and a same-named extras key at once, because the same-named key bound the field. We read it that way, reported the arm as unreachable, and 0122 settles it the other way. The container is separately addressable and its name is normative, so a caller moving between implementations writes the same key. The practical gain is that a provider-specific override of a field OA already models is now expressible, and every arm of the managed-field collision rule is reachable on every mapping. `from_partial` still only drops `None`-valued entries and does not route undeclared names, so there is one spelling rather than two. **On a retried call**, `extras` follows the same rule the declared fields follow, applied per key rather than to the container: see the per-key merge entry below. Conformance fixtures already nested their `config.extras:` sub-block, which the harness used to flatten; it now passes through. Two fixtures come off the deferred list as a direct result: llm-provider 075 and retrieval-provider 052, both held because the same-name reject looked unreachable through the real caller path. Both now run and are mutation-verified against the reject. Two smaller behaviors move with the container. A Langfuse `prompt.config` now lifts an `extras` sub-object onto `Prompt.sampling`, so a vendor knob reaches it from that source as it already did from a filesystem sidecar; the full config still rides `Prompt.metadata` either way. And a filesystem sidecar's unrecognized top-level key is now filtered rather than fatal, matching what the `token_budget` path and the Langfuse backend already did: with the config rejecting undeclared names, splatting the sidecar verbatim would turn one stray key in an operator-authored file into an error escaping `fetch()`, which is neither of the two documented error types and so would bypass the manager's multi-backend fallback. A vendor knob written flat in a sidecar is one such key, so it is filtered and does not reach `sampling.extras`. The behavior shipped ahead of the pin (spec v0.117.0 against a v0.112.0 pin) and is ratified at the v0.118.2 pin: the `conformance.toml` entry is in place and retrieval-provider fixtures 054 / 055 / 056 run. - **Managed wire fields now reject a conflicting extras key instead of silently losing it** (proposals 0105 + 0108, llm-provider §6, spec v0.100.0 / v0.103.0). **Breaking for a managed-key collision only.** A caller's undeclared extras key (`RuntimeConfig` / `EmbeddingRuntimeConfig` accepting extra fields) is forwarded to the wire body untouched, except when it names a field the mapping *manages*: one it sets for its own correctness (0105), or produces as the wire realization of a declared config field (0108). On such a collision the field's shape now decides. An additive list field (`stop` from `stop_sequences`; `embedding_types`) **merges** the caller's value(s) onto the managed value(s), managed-first, de-duplicated. A non-additive scalar or object (`model`, `messages`, `truncate` / `truncation`, `dimensions` / `output_dimension`, `input_type`, `response_format`, Jina `task`, …) takes a value **equal** to the managed one as a no-op and **rejects a conflicting** one pre-send with `ProviderInvalidRequest`. Previously such a collision was silently dropped (the OpenAI llm mapping used `setdefault`) or silently overrode the managed value (the retrieval mappings spread extras first), either of which could re-route the model, defeat a fail-loud `truncate` flag, or break structured-output validation. A field the mapping does not manage keeps untouched pass-through, and a conditionally-managed field is only managed while produced, so the escape hatches hold: an extras `response_format` on a free-form or prompt-augmentation-fallback call rides untouched (0105 §3.5, previously stripped on the fallback path), and an extras Jina `task` with no `input_type` rides untouched (the model-specific-task escape hatch). The rule spans one OpenAI llm mapping and seven retrieval mappings via a shared resolver (`apply_managed_extras`). This shipped ahead of the pin (spec v0.100.0 / v0.103.0 against a v0.88.0 pin) and the `conformance.toml` entries are in place at v0.118.2. The reject / merge fixtures 073 / 077 / 078 / 079 / 080 still do not run, now for reasons the pin does not reach: 073 and 077 need LLM completion streaming (proposal 0062), and 078 / 079 / 080 need the Anthropic and Gemini providers (proposals 0037 / 0038). All three are unimplemented here, so the behavior stays covered by the unit suite. - **Cohere `/v2/embed` recognizes `classification` and `clustering`** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). **Breaking for these two values.** `EmbeddingRuntimeConfig.input_type` is an extensible string, and §2 names `classification` and `clustering` as well-known values a mapping may recognize when its backend supports them. Cohere's does, so the mapping now identity-maps both onto the wire instead of rejecting them. Previously either value raised `ProviderInvalidRequest` before the request was sent, so a caller who relied on that rejection as a guard (catching it to fall back to `document`, say) silently changes behavior. `query` / `document` / absent / unrecognized are all unchanged, and `image` stays out: it names an input modality rather than a purpose for embedded text, and `embed()` consumes strings. The widening is deliberately per-mapping and not portable. Jina keeps its closed `{query, document}` set, because its `task` support varies by model version (v3 accepts `classification` but not `clustering`, v4 neither, v5 both) and a provider is bound to a model identifier with no capability registry to consult, so that mapping cannot promise the values and declines them pre-send rather than letting the wire reject them later. This shipped ahead of the pin (spec v0.94.0 against a v0.88.0 pin) and is ratified at the v0.118.2 pin: the `conformance.toml` entry is in place and retrieval-provider fixture 033's new cases run. - **Cohere `/v2/embed` `embedding_types` merge is now deterministic** (proposal 0099, retrieval-provider §8.4, spec v0.94.0). The mapping manages `embedding_types` as an explicit exception to untouched extras pass-through, because it must request `"float"` for its own response consumer (it reads `embeddings.float`). A caller-supplied `embedding_types` is merged with that mandatory `"float"` rather than replacing it, which was already the behavior; an override that dropped `float` would strip the key the mapping itself reads and fail the call. What changes is the shape of the merged list, which 0099 pins so the outbound body is reproducible and exact-match assertable: `"float"` first, then the caller's precisions in the order supplied, de-duplicated with the first occurrence winning. Previously the caller's precisions came first with `"float"` appended, and a repeated precision was sent twice, so `["int8"]` now yields `["float", "int8"]` rather than `["int8", "float"]`, and `["int8", "uint8", "int8"]` yields `["float", "int8", "uint8"]` rather than passing the duplicate through. The wire is order-insensitive here, so no request semantics change; callers still read their extra precisions off the verbatim response on `raw`. A malformed or empty extra still falls back to `["float"]`. @@ -44,7 +44,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **The sdist no longer carries the local follow-up notes or the pinned spec submodule.** Hatch packages what is on disk and does not read `.git/info/exclude`, so `_tasks/` was being collected into the tarball even though git ignores it. Those are internal working notes rather than part of the public artifact set. Checked against PyPI: **no published release contains them**, so this is preventive rather than a leak to clean up. The `openarmature-spec` submodule is excluded in the same change, which is most of the tarball (1124 files in 0.16.0) and reconstructs from the `spec_version` pin, taking the sdist from 3.4MB to 1.4MB. `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager verify a build from source, and the other two are adopter-facing. - **The `conformance.toml` manifest now ships in the wheel.** The bundled `AGENTS.md` tells an agent to read it for per-proposal implementation status, and that is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the distribution and the pointer said "at the repo root", so the instruction resolved for someone working in a clone and for nobody who ran `pip install openarmature`. An agent planning against an accepted proposal could check for an importable symbol and had no equivalent check for a behaviour, which is exactly what the manifest exists to answer. It is force-included at `openarmature/conformance.toml`, beside the other bundled docs, reachable via `importlib.resources.files("openarmature")`, and a wheel built from the sdist gets it too. The pointer now names that path instead of a repo-relative one. **This matters more in this release than the last:** 0085 and 0124 both ship `partial`, and their notes are where which-half-is-missing is recorded. Reported by an agent adopting the package. - **`StructuredOutputInvalid` uses the field names llm-provider §7 specifies** (§7 / §7.1, proposals 0082 + 0095). The exception carried `raw_content` and `failure_description` where §7 names `output_content` and `error_message`, and §7.1 says the `reask` builder receives those two. A caller who wrote the documented builder therefore hit `AttributeError`, which the retry loop catches and converts into `raise exc from reask_error`: the original structured-output failure propagates, reask never runs, and the call fails on the first invalid output exactly as if no builder had been supplied. **This is a pre-1.0 rename of two public attributes**, so a caller reading them directly needs updating; the conformance harness carried an alias bridging the two spellings and that alias is now gone. -- **A per-attempt retry override merges undeclared extras per key instead of replacing the container** (llm-provider §7.1). §7.1 says "undeclared extras (§6) merge by the same per-key rule", and the analogue of a declared field is an extras KEY rather than the container: a key the override sets replaces that key, a key it does not mention inherits the base's. The implementation replaced the whole container and logged the base keys it dropped, so an override that adjusted one vendor knob silently withdrew every other knob the caller had set for that attempt. Nothing is dropped now, so the warning is gone with it. Clearing a base key for one attempt stays inexpressible, exactly as an override cannot clear a declared field to `None`. +- **A per-attempt retry override merges undeclared extras per key instead of replacing the container** (llm-provider §7.1). §7.1 says "undeclared extras (§6) merge by the same per-key rule", and the analogue of a declared field is an extras KEY rather than the container: a key the override sets replaces that key, a key it does not mention inherits the base's. The implementation replaced the whole container and logged the base keys it dropped, so an override that adjusted one vendor knob silently withdrew every other knob the caller had set for that attempt. No key is dropped now, so the warning is gone with it. Two edges follow from merging. A field the override declares supersedes a base extras key of the same name, because otherwise the merge manufactures an invalid config out of two valid ones: the base's escape-hatch key is unmanaged where it was written and managed on an attempt whose override sets the field, which would reject the call pre-send and kill the retry. An extras key the override names alongside a declared field of that name still collides, since that config contradicts itself in a single object. And withdrawing a base key for one attempt remains unavailable: an override may set a key to `None`, but that sends JSON `null` rather than omitting the field, so a caller who wants the key gone should split the call. - **The Langfuse Tool observation now carries caller-supplied invocation metadata** (observability §8.4.2). It was the one provider observation that dropped it. The LLM handlers pick the caller set up through the shared typed-event metadata builder and the embedding and rerank handlers apply it directly, but the tool handler did neither, so a caller filtering Langfuse by their own key (a tenant id, a request id) saw every observation from an invocation except the tool calls. The §8.4.2 mapping maps the caller set to `observation.metadata.` on every Observation, and the unscoped wording is deliberate: the same table scopes its other rows explicitly where it means to, for example `fan_out_item_count` to the fan-out node Span only. The OTel observer's tool span had carried the set all along, so the two bundled observers disagreed about the same event. The caller set is merged before the OA-emitted keys, matching the other handlers, though on this observation precedence is unobservable either way: every key it writes is reserved, the `openarmature_*` pair by prefix and `error_type` / `error_message` by name, so a colliding caller key is rejected at the `invoke()` boundary and never reaches the merge. - **A caller metadata key no longer overwrites the failure-isolation marker's own fields** (observability §3.4). The Langfuse `openarmature.failure_isolated` span merged caller-supplied invocation metadata *last*, with a plain per-key assignment and no collision check, which made it the only handler where a colliding caller key won: the LLM, embedding and rerank handlers all merge the caller set before writing their own keys. Three of the marker's top-level keys (`error_category`, `failure_isolation_event_name`, `failure_isolation_node`) are not in the reserved set, so `invoke()` does not reject a caller key of the same name at the boundary and the collision reached the observer. A caller passing `error_category` silently replaced the marker's only failure discriminator, on the failure path, with nothing logged. The caller set now merges first, so the OA-emitted values win, and a non-colliding caller key still comes through unchanged. The OTel observer needs no equivalent change: every attribute it writes is `openarmature.`-prefixed and caller keys land under `openarmature.user.*`, both covered by a reserved prefix, so a colliding key is rejected at the boundary and never reaches the merge. Langfuse metadata is flat, which is why the reserved *name* set exists alongside the prefixes. Read the new ordering as the lesser harm rather than a settled precedence rule: OA-wins still drops the caller's value silently, and §3.4 rejects a reserved collision precisely because silent resolution in either direction loses information. The real fix is reservation, and it arrives with the span's mapping: spec ruled that §3.4's reserved set does not reach a span no mapping table covers, and committed the forthcoming failure-isolation mapping to carry these three keys. Found by the proposal 0119 maintenance check. - **A detached fan-out instance no longer opens a second root that abandons the first** (observability §4.4). A detached fan-out's per-instance root is stored under `prefix + (instance index)`, but the ancestor walk that decides whether to open one tested the bare `prefix`, so the test never matched and the arm fired once per inner **node event** rather than once per instance. Each additional inner node re-opened the root, and the second open replaced both the root and its detached invocation span in the observer's state, so the first pair was never ended and never exported. The result for a caller: three traces where there should be two, one instance's inner nodes split across two of them, and spans carrying a parent id that nothing emitted. It needs two or more nodes in the instance subgraph to see, which is why it went unnoticed: with a single node there is one event and one open, and every existing test used that shape. The guard now tests the key the root is actually stored under. The neighbouring bare-prefix test is kept for depth and is documented as redundant rather than load-bearing, since the detached-subgraph path is already guarded a line earlier. diff --git a/conformance.toml b/conformance.toml index 060dc53..f9c836a 100644 --- a/conformance.toml +++ b/conformance.toml @@ -903,7 +903,7 @@ note = "Nested-fan-out span lineage. graph-engine §6: fan_out_index_chain / bra [proposals."0085"] status = "partial" since = "0.17.0" -note = "Nested-fan-out checkpoint lineage + no-mis-skip invariant (pipeline-utilities §10.11 / §10.7 / §10.2). SAVE-side in-memory keying shipped in #194 (v0.16.0): a fan-out instance's checkpoint tracking key carries the enclosing fan-out instance lineage in the in-memory dict + projection / lookup / cleanup, so concurrent outer instances no longer collide live. The RESUME consume-side now ships (v0.17.0): FanOutProgress gains an optional enclosing_fan_out_lineage (a sequence of {namespace, fan_out_node_name, fan_out_index}, the new EnclosingFanOutInstance record type), and _restore_fan_out_progress_state keys the tracking dict by it (projected to the flat fan_out_index tuple the re-entry key uses). This realizes the §10.11 no-mis-skip invariant + §10.11.1 exactly-once for nested fan-outs via the existing keyed re-entry: a lineage-bearing entry positively matches per outer instance (correct skip); an empty/legacy lineage keys to () and never matches a non-empty re-entering lineage (re-run, the safe floor per §10.7). Backward-compatible: flat records (empty lineage) resume identically, and the SQLite json serializers round-trip the new field. pipeline-utilities fixture 076 (both cases: lineage-matched skip + legacy full-re-run safety floor) runs in test_checkpoint.py via a seeded-record resume path; the invariants are verified against the inner-leaf source values that actually re-ran (final state alone cannot tell a correct skip from a full re-run). partial for exactly this reason, and the citation that used to defend it as out of scope (a bogus reference to §66 / §76) does not exist in 0085, whose own Out of scope section lists three items and does not include the write side. What IS met: §10.11's no-mis-skip floor, because an unmatched record re-runs rather than skipping. What is NOT met: §10.11.1's exactly-once guarantee does not extend to nested fan-outs, since a COMPLETED inner instance re-runs on resume and its side effects execute twice. Fixture 076 passes against a record the test seeds by hand, not one the engine wrote. THE GAP: the crash-PRODUCED write side (_project_fan_out_progress emitting the rich lineage on a real crash record, so a real nested-fan-out crash resumes at the correct-skip rather than the safe re-run floor) needs lineage-qualified crash boundaries and is tracked; parallel-branches cross-nesting is a deferred dimension." +note = "Nested-fan-out checkpoint lineage + no-mis-skip invariant (pipeline-utilities §10.11 / §10.7 / §10.2). SAVE-side in-memory keying shipped in #194 (v0.16.0): a fan-out instance's checkpoint tracking key carries the enclosing fan-out instance lineage in the in-memory dict + projection / lookup / cleanup, so concurrent outer instances no longer collide live. The RESUME consume-side now ships (v0.17.0): FanOutProgress gains an optional enclosing_fan_out_lineage (a sequence of {namespace, fan_out_node_name, fan_out_index}, the new EnclosingFanOutInstance record type), and _restore_fan_out_progress_state keys the tracking dict by it (projected to the flat fan_out_index tuple the re-entry key uses). This realizes the §10.11 no-mis-skip invariant via the existing keyed re-entry: a lineage-bearing entry positively matches per outer instance (correct skip); an empty/legacy lineage keys to () and never matches a non-empty re-entering lineage (re-run, the safe floor per §10.7). Backward-compatible: flat records (empty lineage) resume identically, and the SQLite json serializers round-trip the new field. pipeline-utilities fixture 076 (both cases: lineage-matched skip + legacy full-re-run safety floor) runs in test_checkpoint.py via a seeded-record resume path; the invariants are verified against the inner-leaf source values that actually re-ran (final state alone cannot tell a correct skip from a full re-run). partial for exactly this reason. 0085 does declare the crash-produced write side out of scope, in the paragraph following the fixture specification rather than under its `## Out of scope` heading, and defers parallel-branches cross-nesting under that heading; the earlier citation wrote those two locations as section marks when they were line numbers. Out of scope for the proposal is not implemented for the manifest, which is why the status is partial rather than implemented. What IS met: §10.11's no-mis-skip floor, because an unmatched record re-runs rather than skipping. What is NOT met: §10.11.1's exactly-once guarantee does not extend to nested fan-outs, since a COMPLETED inner instance re-runs on resume and its side effects execute twice. Fixture 076 passes against a record the test seeds by hand, not one the engine wrote. THE GAP: the crash-PRODUCED write side (_project_fan_out_progress emitting the rich lineage on a real crash record, so a real nested-fan-out crash resumes at the correct-skip rather than the safe re-run floor) needs lineage-qualified crash boundaries and is tracked; parallel-branches cross-nesting is a deferred dimension." # Spec v0.79.0 (proposal 0086). Service-wide default cache_ttl_seconds # on PromptManager (prompt-management §6). Implemented since 0.17.0; @@ -1018,7 +1018,7 @@ note = "ScoredDocument.document stays echoed text (string | null) but the echo r [proposals."0098"] status = "textual-only" since = "0.17.0" -note = "A conformance-adapter directive rename with no shipped-module component: the three structured_output_invalid carries keys that did not track their §7 fields are renamed (raw_response_content -> output_content, failure_description_present -> error_message_present, failure_description_mentions -> error_message_mentions), and §5.12 states the key-naming convention normatively (a key MUST name a §7 error field, bare field = exact-equality with subset match for a mapping-valued field, _present / _mentions the closed flavor set). No openarmature-python shipped module changes; the conformance harness bridges the renamed §7 keys (output_content / error_message) to the impl error attributes (raw_content / failure_description) via its carries alias map, so llm-provider fixtures 022 / 023 run." +note = "A conformance-adapter directive rename with no shipped-module component: the three structured_output_invalid carries keys that did not track their §7 fields are renamed (raw_response_content -> output_content, failure_description_present -> error_message_present, failure_description_mentions -> error_message_mentions), and §5.12 states the key-naming convention normatively (a key MUST name a §7 error field, bare field = exact-equality with subset match for a mapping-valued field, _present / _mentions the closed flavor set). StructuredOutputInvalid now carries those §7 names directly, so the harness resolves a carries key straight onto the attribute with no alias map; llm-provider fixtures 022 / 023 run. textual-only describes the proposal, which renames directive keys rather than adding a behavior: the attribute rename that removed the bridge is its own entry." # Spec v0.94.0 (proposal 0099). Cohere /v2/embed input_type widened; # the extras-vs-managed-field claims pinned (retrieval-provider §8.4). diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index 8d109f3..48da46c 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -142,12 +142,24 @@ replace, the rest inherited from the base), and the last entry carries forward when the schedule is shorter than the retry count. The caller's `config` is never mutated. -`extras` follows the same rule with one wrinkle, because its default is -an empty container rather than `None`. An override that declares no -extras inherits the base's; one that declares any replaces them -wholesale, and any base key it does not carry is logged as not sent on -that attempt. Clearing extras for a single attempt is therefore not -expressible, since an empty container is how inheriting is spelled. +`extras` follows the same rule applied one level down, to each key +rather than to the container. A key the override sets replaces that key; +a key it does not mention inherits the base's. An override that declares +no extras therefore inherits all of them, and one that adjusts a single +vendor knob leaves the rest in place. + +Two edges are worth knowing. A field the override declares wins over a +base extras key of the same name, so a base `extras={"temperature": 0.9}` +yields to an override's `temperature=0.3` rather than colliding with it. +But naming the same field twice in one override, once declared and once +in its own `extras`, is a config that contradicts itself and is rejected +before the call goes out. + +Withdrawing a base key for a single attempt is not available. Setting it +to `None` in the override sends JSON `null` on the wire, which is not the +same as omitting the field and many providers treat it differently. If an +attempt genuinely must go out without a key the base carries, make it a +separate call. ### Reasking on invalid structured output diff --git a/src/openarmature/llm/errors.py b/src/openarmature/llm/errors.py index 2692c14..18b4fe3 100644 --- a/src/openarmature/llm/errors.py +++ b/src/openarmature/llm/errors.py @@ -200,8 +200,10 @@ class StructuredOutputInvalid(LlmProviderError): Attributes: response_schema: The JSON Schema requested. output_content: The raw response content the model produced. - error_message: A description of the parse or validation - failure. + error_message: What the parse or validation step objected to, + on its own. It does not repeat this exception's own message, + so a caller composing text for the model normally wants + both (``str(exc)`` and this). finish_reason: The normalized finish reason of the response that failed validation (``"length"`` signals a truncation, the key retry signal; ``"stop"`` a clean-finish schema/parse failure). diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 2ba8a38..6b948d2 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -631,7 +631,8 @@ def _config_for_attempt( inherits the base -- and the last entry carries forward when the schedule is shorter than the retry count. ``extras`` merges by the same per-key rule: a key the override sets replaces that key, a key it does - not mention inherits the base's. Attempt 0 and the no-override + not mention inherits the base's, and a base key whose name the override + declares as a field is superseded by that field. Attempt 0 and the no-override case return the caller's base config as-is; only the override path returns a fresh ``model_copy``. The caller's config is never mutated either way -- the base path relies on the downstream body build reading @@ -657,15 +658,25 @@ def _config_for_attempt( # a field is an extras KEY rather than the container: a key set in the # override replaces the base's value for that key, a key the override does # not mention inherits the base's. So the container merges. - # - # This replaced the container wholesale and warned about the base keys it - # dropped, on the reading that a non-empty container is itself the unit a - # field's rule applies to. Under the merge rule nothing is dropped, so the - # warning went with it. Clearing a base key for one attempt stays - # inexpressible, exactly as an override cannot clear a declared field to - # None. update = override.model_dump(exclude_none=True, exclude={"extras"}) - update["extras"] = {**base_or_empty.extras, **override.extras} + merged = {**base_or_empty.extras, **override.extras} + # A field the override declares supersedes a base extras key of the same + # name. Without this the merge manufactures an invalid config out of two + # valid ones: the base's §6 escape-hatch key (unmanaged there, because the + # base left the field unset) is inherited onto an attempt whose override + # DOES set the field, so the mapping emits it, §8.1 sees a managed-field + # collision, and the call rejects pre-send -- killing a retry that + # previously ran. The override's field is the later and more specific + # instruction, so it wins. + # + # A key the override itself carries in `extras` is left alone, so + # declaring a field and naming it in the same override's extras still + # collides. That config contradicts itself in one object rather than + # across two, and §8.1's reject is the right answer to it. + for field_name in update: + if field_name not in override.extras: + merged.pop(field_name, None) + update["extras"] = merged return base_or_empty.model_copy(update=update) @staticmethod @@ -795,6 +806,18 @@ async def _do_complete_with_retry( try: transcript = self._append_reask_pair(transcript, exc.output_content, reask(exc)) except Exception as reask_error: + # The chained cause carries this, but the surfaced error + # reads as a bad model rather than a broken builder, and + # a caller who never inspects __cause__ debugs the wrong + # thing. Say which one failed where it will be seen. + _log.warning( + "reask builder raised on attempt %d (%s: %s); " + "reask is disabled for this call and the " + "structured_output_invalid failure stands", + attempt, + type(reask_error).__name__, + reask_error, + ) raise exc from reask_error next_retry_reason = "reask" else: diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 3d86ec0..05a92b7 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -3462,10 +3462,6 @@ async def test_per_attempt_override_with_extras_merges_per_key() -> None: # key the override sets replaces that key and a key it does not mention # inherits the base's. # - # This asserted the opposite until the rule was read properly, and it asserted - # a warning about the base keys the replacement dropped. Under the merge rule - # nothing is dropped, so there is nothing to warn about. - # # Without this the merge direction is unpinned. Verified by mutation: both # dropping `override.extras` and reversing the precedence left the suite # green while this test was absent. @@ -3518,3 +3514,102 @@ def handler(req: httpx.Request) -> httpx.Response: "per-key rule the declared fields follow" ) assert bodies[1]["keep"] == 1, "every unmentioned base key inherits, not just the first" + + +async def test_override_declared_field_supersedes_inherited_base_extras_key() -> None: + # A base extras key is the section 6 escape hatch: unmanaged where it was + # written, because the base leaves the declared field unset so the mapping + # emits nothing of that name. Merging carries it onto a retry whose override + # DOES set the field, where it is managed, and section 8.1 rejects a managed + # collision pre-send. Two individually valid configs would then compose into + # one the provider refuses, killing the retry the caller asked for. + # + # The override's field is the later and more specific instruction, so it wins + # and the inherited key drops. + # + # Not vacuous: without the supersede step the first assertion fails on call + # COUNT (one wire call, not two) because attempt 1 raises before sending, and + # the terminal error is provider_invalid_request rather than the transient + # the retry existed to ride out. Killed by removing the `merged.pop` loop. + from openarmature.llm import LlmRetryConfig + + bodies: list[dict[str, Any]] = [] + calls = {"n": 0} + + def handler(req: httpx.Request) -> httpx.Response: + bodies.append(json.loads(req.content)) + calls["n"] += 1 + if calls["n"] == 1: + return httpx.Response(503, json={"error": {"message": "upstream"}}) + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + {"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"} + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + provider = _collision_provider(handler) + try: + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(extras={"temperature": 0.9, "keep": 1}), + retry=LlmRetryConfig( + max_attempts=2, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(temperature=0.3)], + ), + ) + finally: + await provider.aclose() + + assert len(bodies) == 2, f"the retry must still run, got {len(bodies)} call(s)" + # Attempt 0 is untouched: the base's escape-hatch key is what reaches the wire. + assert bodies[0]["temperature"] == 0.9 + # Attempt 1 sends the override's declared value, once, and the inherited + # extras key is gone rather than colliding with it. + assert bodies[1]["temperature"] == 0.3 + # An unrelated base key still inherits, so the supersede is scoped to the + # colliding name rather than clearing the container. + assert bodies[1]["keep"] == 1 + + +async def test_override_naming_a_declared_field_in_its_own_extras_still_collides() -> None: + # The supersede above is deliberately asymmetric. A base key inherited into a + # collision was never written alongside the field, so yielding to the field is + # reading the caller's intent. An override that declares a field AND names it + # in its own extras contradicts itself inside one object, and section 8.1's + # reject is the right answer. + # + # Not vacuous: widening the supersede to drop the override's own extras key + # makes this call succeed instead of raising, so the assertion flips. + from openarmature.llm import LlmRetryConfig + + calls = {"n": 0} + + def handler(req: httpx.Request) -> httpx.Response: + calls["n"] += 1 + return httpx.Response(503, json={"error": {"message": "upstream"}}) + + provider = _collision_provider(handler) + try: + with pytest.raises(ProviderInvalidRequest): + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(), + retry=LlmRetryConfig( + max_attempts=2, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(temperature=0.3, extras={"temperature": 0.5})], + ), + ) + finally: + await provider.aclose() + + # Attempt 0 went out (no override applies to it); attempt 1 raised pre-send, + # so the collision is what stopped it rather than a transport failure. + assert calls["n"] == 1 diff --git a/tests/unit/test_observability_otel.py b/tests/unit/test_observability_otel.py index ea103ee..65f1116 100644 --- a/tests/unit/test_observability_otel.py +++ b/tests/unit/test_observability_otel.py @@ -2119,22 +2119,18 @@ def _reask_appended_message_matches(actual: dict[str, Any], expected: dict[str, def _assert_reask_carries(exc: Any, carries: dict[str, Any]) -> None: - # Minimal llm-provider §7 carries check for the reask driver: the - # StructuredOutputInvalid names its attributes output_content / - # error_message (0098's output_content / error_message §7 names alias - # onto them), honoring the _present / _mentions suffixes and a mapping-valued - # subset (usage). - alias = {"output_content": "output_content", "error_message": "error_message"} + # Minimal llm-provider §7 carries check for the reask driver. A carries key + # names a §7 error field and StructuredOutputInvalid's attributes are those + # names, so a key resolves onto an attribute directly. Honors the _present / + # _mentions suffixes and a mapping-valued subset (usage). for key, want in carries.items(): if key.endswith("_present"): - attr = alias.get(key[:-8], key[:-8]) - assert (getattr(exc, attr, None) is not None) == bool(want), f"carries {key}" + assert (getattr(exc, key[:-8], None) is not None) == bool(want), f"carries {key}" elif key.endswith("_mentions"): - attr = alias.get(key[:-9], key[:-9]) - actual = getattr(exc, attr, None) + actual = getattr(exc, key[:-9], None) assert isinstance(actual, str) and want in actual, f"carries {key}: {actual!r} lacks {want!r}" else: - actual: Any = getattr(exc, alias.get(key, key), None) + actual: Any = getattr(exc, key, None) if isinstance(want, dict): dump: Any = actual.model_dump() if hasattr(actual, "model_dump") else actual for k, v in cast("dict[str, Any]", want).items(): From 2889cadfff303ae2879cee91a5d84421d5e1fe30 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sat, 26 Sep 2026 10:44:56 -0700 Subject: [PATCH 06/14] Correct the root AGENTS.md against the tree The hand-maintained orientation file had drifted far enough to send an agent to the wrong places. Verified each claim against the source rather than against the file. - It listed src/openarmature/middleware/, which does not exist. The middleware package is under graph/. - It claimed a filesystem checkpoint backend. The checkpoint backends are in-memory and SQLite; filesystem is a prompt backend, which is the likely source of the mix-up. - prompts/ and retrieval/ were absent entirely, along with every provider in them. - Langfuse appeared nowhere, neither the observer nor the prompt backend. - It said three places hold the spec version and that the third is enforced by convention. There are four, conformance.toml's spec_pin being the one that drifted silently before it was covered, and the submodule is checked by reading the spec changelog at the pinned commit. - test_smoke.py was described as version sync, which is now about half of what it guards. Also note that the bundled AGENTS.md and _patterns/ are generated, since the two files sit one directory apart and only one is safe to edit. --- AGENTS.md | 65 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 887ef62..6cee7a2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,26 +45,49 @@ When implementing a feature, **read the relevant Accepted proposal first** — don't infer behavior from existing impl alone. Draft proposals don't ship; their text may change before acceptance. -## Three places hold the spec version — keep them in sync +## Four places hold the spec version — keep them in sync -- `tool.openarmature.spec_version` in `pyproject.toml` - `__spec_version__` in `src/openarmature/__init__.py` +- `tool.openarmature.spec_version` in `pyproject.toml` - The submodule commit (must match a released spec tag, e.g. `v0.10.0`) +- `manifest.spec_pin` in `conformance.toml` (carries a `v` prefix) -`tests/test_smoke.py` asserts the first two match. The third is enforced -by convention. +`tests/test_smoke.py` asserts all four. The submodule is checked by +reading the spec's `CHANGELOG.md` at the pinned commit rather than by +`git describe`, so it works in any checkout shape without fetching tags. +The fourth drifted silently before it was covered, so add a check +alongside any new place a version lands rather than relying on the habit +of updating them together. ## Package layout - `src/openarmature/graph/` — graph engine (State, GraphBuilder, - CompiledGraph, edges, projections, fan-out) -- `src/openarmature/llm/` — LLM Provider Protocol + OpenAIProvider; HTTP - error classification + retry helpers -- `src/openarmature/checkpoint/` — checkpointing protocol + in-memory - and filesystem backends -- `src/openarmature/observability/` — `[otel]` extra; OTel observer + - log bridge + correlation primitives -- `src/openarmature/middleware/` — pipeline-utility middleware + CompiledGraph, edges, projections, fan-out, parallel branches, + subgraphs) +- `src/openarmature/graph/middleware/` — pipeline-utility middleware + (retry, failure isolation, timing) +- `src/openarmature/llm/` — LLM Provider Protocol + `OpenAIProvider` + (`llm/providers/`); HTTP error classification, call-level retry, + structured-output reask +- `src/openarmature/prompts/` — PromptManager + prompt records; the + filesystem and Langfuse prompt backends (`prompts/backends/`) +- `src/openarmature/retrieval/` — embedding + rerank provider + protocols; the OpenAI, Jina, Cohere and TEI providers + (`retrieval/providers/`) +- `src/openarmature/checkpoint/` — checkpointing protocol + migration; + the in-memory and SQLite backends (`checkpoint/backends/`) +- `src/openarmature/observability/` — the two bundled observers behind + their own extras: OTel (`observability/otel/`, `[otel]`) and Langfuse + (`observability/langfuse/`, `[langfuse]`), plus the correlation, + lineage and event primitives they share +- `src/openarmature/patterns.py`, `src/openarmature/_patterns/` — + generated agent-facing pattern docs; `cli.py` backs the console script + +`src/openarmature/AGENTS.md` and `src/openarmature/_patterns/` are +GENERATED by `scripts/build_agents_md.py` and ship in the wheel. Do not +hand-edit them; edit the generator or the docstrings it reads, then +regenerate. `tests/test_smoke.py` fails on drift. This file, the one at +the repository root, is hand-maintained and is not the bundled one. ## Test layout @@ -73,7 +96,15 @@ by convention. - `tests/unit/` — fills coverage gaps the conformance suite doesn't reach: `edge_exception`, `reducer_error`, `state_validation_error`, `SubgraphNode.run`, projection variants, frozen-state mutation, etc. -- `tests/test_smoke.py` — version sync. +- `tests/test_smoke.py` — the four pin-sync points above, plus the + packaging guards: that `conformance.toml` is force-included in the + wheel, that the bundled `AGENTS.md` names both places the manifest can + be (a clone and an installed package resolve different paths, and a + pointer naming one is wrong for half of readers), and that the sdist + excludes the private `_tasks/` notes. +- `tests/test_examples_smoke.py` — loads every `examples/*/main.py` and + compiles its graph. Its `DEMOS` list is compared against the + directories on disk, so a new example that nobody listed fails loudly. ### Activating a conformance fixture is not done when it passes @@ -206,9 +237,13 @@ In scope: - Graph engine + the spec's runtime contract. - Pipeline utilities (rate limiting, structured-output retry helpers). -- Observability via OTel observer (under `[otel]` extra). -- Checkpointing (in-memory + filesystem backends). +- Observability via the OTel observer (`[otel]`) and the Langfuse + observer (`[langfuse]`). +- Checkpointing (in-memory + SQLite backends). - LLM Provider Protocol + the canonical OpenAI implementation. +- Prompt management + its filesystem and Langfuse backends. +- Retrieval: the embedding and rerank provider protocols + the OpenAI, + Jina, Cohere and TEI implementations. Out of scope, deferred to sibling packages at v1.0: From 0af3a608676b709ea12050f21993b63d7b390c7c Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Sat, 26 Sep 2026 11:21:35 -0700 Subject: [PATCH 07/14] Say why the ceiling drives the reask demo The demo drives a token ceiling because that failure is guaranteed on any endpoint. Nothing said so, and the surrounding prose implied the wrong-shaped answer had been dropped for being unrealistic. It is the opposite: it is the failure an adopter is most likely to meet, and they meet it precisely when their serving stack is not yet configured to prevent it. Whether it fires at all is a property of the endpoint rather than of the calling code. An endpoint that enforces the schema during decoding cannot produce it; one that ignores response_format, or sits behind a proxy that drops the field, or serves a weaker model, produces it routinely. So the demo drives the failure it can guarantee and documents the one it cannot, and the builder handles both through the exception they share. Not having to know in advance which kind of endpoint you are pointed at is the value, and that is now what the example says. The concepts page gains the same distinction plus the finish_reason tell that separates the two causes, and stops naming an escalating temperature as the only interesting per-attempt override: a truncated reply needs the ceiling raised, which a correction alone cannot fix. --- docs/concepts/llms.md | 26 ++++++++++++++++++++++-- examples/README.md | 7 ++++++- examples/structured-output-reask/main.py | 12 +++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index 48da46c..b8f86a2 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -190,8 +190,30 @@ appends the model's raw output as an `assistant` message and your correction as a `user` message to a working transcript that accumulates across retries and consumes the `max_attempts` budget. The framework adds no prompt text of its own; you own every word beyond the model's output, -and the caller's `messages` are never mutated. Reask composes with a -`per_attempt_override` (escalate temperature *and* reask). +and the caller's `messages` are never mutated. + +**Two different things produce an invalid reply, and the exception tells +them apart.** The model can answer in the wrong shape: prose instead of +JSON, a markdown fence around it, a string where the schema wanted a +number. Or the reply can be cut off mid-object because it hit +`max_tokens`, in which case nothing was wrong with the model's answer and +there simply was not room for it. `finish_reason` is `"length"` in the +second case and not in the first, so a builder can say something +different about each. + +How often you see the first depends on your serving stack rather than on +your code. An endpoint that enforces the schema during decoding will not +produce it; one that ignores `response_format`, or serves a weaker model, +or sits behind a proxy that drops the field, produces it routinely. Both +causes arrive at the same exception, so a builder written for both keeps +working when you change endpoints. + +Reask composes with a `per_attempt_override`, and the two halves do +different work. The builder changes what you say; the override changes +what the attempt is allowed to spend. A truncated reply needs the second: +a correction alone gets cut off in the same place, so raise `max_tokens` +on the retry. A wrong-shaped reply usually needs only the first, though +escalating `temperature` alongside it is the canonical schedule. ### Call-level vs node-level retry diff --git a/examples/README.md b/examples/README.md index 453f88e..1bd3855 100644 --- a/examples/README.md +++ b/examples/README.md @@ -150,7 +150,12 @@ no-op on symmetric OpenAI, meaningful on asymmetric providers), mapping Extracting one structured mission record per lunar-landing report, under an output-token cap that is too tight for a complete record. The model stops mid-object, and the fragment that arrives is rejected at the schema boundary -exactly as a wrong-typed field would be. Recovery needs two different changes: +exactly as a wrong-typed field would be. The cap is what the demo drives, +because it fails the same way on every endpoint; the wrong-typed field is the +more common failure in practice and how often you see it depends on whether +your endpoint enforces the schema during decoding. Both reach the same +exception, and the builder here handles both. Recovery needs two different +changes: a caller-supplied builder quotes the model's own fragment and the reader's objection back to it, and a per-attempt override raises the ceiling so the retry has somewhere to put the answer. Demonstrates: diff --git a/examples/structured-output-reask/main.py b/examples/structured-output-reask/main.py index cf1ceca..45d0952 100644 --- a/examples/structured-output-reask/main.py +++ b/examples/structured-output-reask/main.py @@ -27,6 +27,18 @@ Supply only the first and the retry is better informed and still truncated. This demo shows both arms so the difference is visible. +**Why the ceiling and not the wrong-shaped answer.** The ceiling is the +failure this demo can guarantee, on any endpoint and any model. The +wrong-shaped answer is the one you are more likely to meet, and whether you +meet it at all is a property of your serving stack rather than of your code. +An endpoint that enforces the schema during decoding cannot produce it. An +endpoint that ignores ``response_format``, or serves a weaker model behind +one, produces it routinely, and so does a proxy that drops the field on the +way through. Both failures arrive at the same exception and the same builder +handles both, which is the point: you do not have to know in advance which +kind of endpoint you are pointed at, and you keep working when someone moves +you to a different one. + **What's interesting in the implementation:** - ``complete(response_schema=...)`` asks the provider for a shape. When the From 5384113940d07d14b5e2e055ad48b5130299d565 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Mon, 28 Sep 2026 08:36:05 -0700 Subject: [PATCH 08/14] Revert the extras supersede, and name both channels A base extras key is the section 6 escape hatch, unmanaged where it was written because the base leaves the declared field unset. Merging carried it onto a retry whose override did set the field, where it is managed, so section 8.1 rejected the call before it went out and the retry died on attempt 1. The fix for that was to let the override's declared field supersede the inherited key. Spec ruled against it and is right. Section 6 says a mapping must not silently drop a conflicting extras value, and moving the discard upstream of the mapping does not change what happens to the caller's value. The argument that settles it is narrower: under the supersede, a config carrying temperature as both a declared field and an extras key was rejected when a caller wrote it and accepted when a merge assembled it. One shape, two meanings, decided by history rather than content. A merged config could then not be logged, replayed or handed to another call and behave the same way, and a second implementation would have to track provenance through the merge to stay conformant. That is the defect class proposal 0124 exists to prevent, one level down. So the collision stands. What the caller does instead is keep both values in the channel the base used: a base reaching for extras gets an override that does the same, which merges per key and sends. The ergonomic half of the report is real and is fixed differently. The collision surfaces on attempt 1, after attempt 0 has already gone out on the base alone, so a broken retry config hides until the first retry. It now reports that the attempt's config merges a base with a per-attempt override, that the two values come from different channels, and which channel to change. Detecting it before attempt 0 needs every attempt's body built eagerly, so that is recorded as a follow-up rather than built. --- CHANGELOG.md | 2 +- docs/concepts/llms.md | 19 ++-- src/openarmature/_managed_extras.py | 7 +- src/openarmature/llm/providers/openai.py | 55 ++++++----- tests/unit/test_llm_provider.py | 114 ++++++++++++++--------- 5 files changed, 123 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2886bea..d88291f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **The sdist no longer carries the local follow-up notes or the pinned spec submodule.** Hatch packages what is on disk and does not read `.git/info/exclude`, so `_tasks/` was being collected into the tarball even though git ignores it. Those are internal working notes rather than part of the public artifact set. Checked against PyPI: **no published release contains them**, so this is preventive rather than a leak to clean up. The `openarmature-spec` submodule is excluded in the same change, which is most of the tarball (1124 files in 0.16.0) and reconstructs from the `spec_version` pin, taking the sdist from 3.4MB to 1.4MB. `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager verify a build from source, and the other two are adopter-facing. - **The `conformance.toml` manifest now ships in the wheel.** The bundled `AGENTS.md` tells an agent to read it for per-proposal implementation status, and that is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the distribution and the pointer said "at the repo root", so the instruction resolved for someone working in a clone and for nobody who ran `pip install openarmature`. An agent planning against an accepted proposal could check for an importable symbol and had no equivalent check for a behaviour, which is exactly what the manifest exists to answer. It is force-included at `openarmature/conformance.toml`, beside the other bundled docs, reachable via `importlib.resources.files("openarmature")`, and a wheel built from the sdist gets it too. The pointer now names that path instead of a repo-relative one. **This matters more in this release than the last:** 0085 and 0124 both ship `partial`, and their notes are where which-half-is-missing is recorded. Reported by an agent adopting the package. - **`StructuredOutputInvalid` uses the field names llm-provider §7 specifies** (§7 / §7.1, proposals 0082 + 0095). The exception carried `raw_content` and `failure_description` where §7 names `output_content` and `error_message`, and §7.1 says the `reask` builder receives those two. A caller who wrote the documented builder therefore hit `AttributeError`, which the retry loop catches and converts into `raise exc from reask_error`: the original structured-output failure propagates, reask never runs, and the call fails on the first invalid output exactly as if no builder had been supplied. **This is a pre-1.0 rename of two public attributes**, so a caller reading them directly needs updating; the conformance harness carried an alias bridging the two spellings and that alias is now gone. -- **A per-attempt retry override merges undeclared extras per key instead of replacing the container** (llm-provider §7.1). §7.1 says "undeclared extras (§6) merge by the same per-key rule", and the analogue of a declared field is an extras KEY rather than the container: a key the override sets replaces that key, a key it does not mention inherits the base's. The implementation replaced the whole container and logged the base keys it dropped, so an override that adjusted one vendor knob silently withdrew every other knob the caller had set for that attempt. No key is dropped now, so the warning is gone with it. Two edges follow from merging. A field the override declares supersedes a base extras key of the same name, because otherwise the merge manufactures an invalid config out of two valid ones: the base's escape-hatch key is unmanaged where it was written and managed on an attempt whose override sets the field, which would reject the call pre-send and kill the retry. An extras key the override names alongside a declared field of that name still collides, since that config contradicts itself in a single object. And withdrawing a base key for one attempt remains unavailable: an override may set a key to `None`, but that sends JSON `null` rather than omitting the field, so a caller who wants the key gone should split the call. +- **A per-attempt retry override merges undeclared extras per key instead of replacing the container** (llm-provider §7.1). §7.1 says "undeclared extras (§6) merge by the same per-key rule", and the analogue of a declared field is an extras KEY rather than the container: a key the override sets replaces that key, a key it does not mention inherits the base's. The implementation replaced the whole container and logged the base keys it dropped, so an override that adjusted one vendor knob silently withdrew every other knob the caller had set for that attempt. No key is dropped now, so the warning is gone with it. Two edges are worth knowing. The merge does not resolve an inherited extras key that collides with a field the override declares: the call rejects pre-send like any other managed-field collision, because dropping the key would make one config mean two things depending on whether a caller wrote it or a merge assembled it. A caller whose base reached for the `extras` escape hatch keeps the override in the same channel (`extras={"temperature": 0.3}`) rather than switching to the declared field for one wire key. That collision surfaces on the first retry rather than the first call, since attempt 0 uses the base alone, so its message now names both channels and says which to change. And withdrawing a base key for one attempt remains unavailable: an override may set a key to `None`, but that sends JSON `null` rather than omitting the field, so a caller who wants the key gone should split the call. - **The Langfuse Tool observation now carries caller-supplied invocation metadata** (observability §8.4.2). It was the one provider observation that dropped it. The LLM handlers pick the caller set up through the shared typed-event metadata builder and the embedding and rerank handlers apply it directly, but the tool handler did neither, so a caller filtering Langfuse by their own key (a tenant id, a request id) saw every observation from an invocation except the tool calls. The §8.4.2 mapping maps the caller set to `observation.metadata.` on every Observation, and the unscoped wording is deliberate: the same table scopes its other rows explicitly where it means to, for example `fan_out_item_count` to the fan-out node Span only. The OTel observer's tool span had carried the set all along, so the two bundled observers disagreed about the same event. The caller set is merged before the OA-emitted keys, matching the other handlers, though on this observation precedence is unobservable either way: every key it writes is reserved, the `openarmature_*` pair by prefix and `error_type` / `error_message` by name, so a colliding caller key is rejected at the `invoke()` boundary and never reaches the merge. - **A caller metadata key no longer overwrites the failure-isolation marker's own fields** (observability §3.4). The Langfuse `openarmature.failure_isolated` span merged caller-supplied invocation metadata *last*, with a plain per-key assignment and no collision check, which made it the only handler where a colliding caller key won: the LLM, embedding and rerank handlers all merge the caller set before writing their own keys. Three of the marker's top-level keys (`error_category`, `failure_isolation_event_name`, `failure_isolation_node`) are not in the reserved set, so `invoke()` does not reject a caller key of the same name at the boundary and the collision reached the observer. A caller passing `error_category` silently replaced the marker's only failure discriminator, on the failure path, with nothing logged. The caller set now merges first, so the OA-emitted values win, and a non-colliding caller key still comes through unchanged. The OTel observer needs no equivalent change: every attribute it writes is `openarmature.`-prefixed and caller keys land under `openarmature.user.*`, both covered by a reserved prefix, so a colliding key is rejected at the boundary and never reaches the merge. Langfuse metadata is flat, which is why the reserved *name* set exists alongside the prefixes. Read the new ordering as the lesser harm rather than a settled precedence rule: OA-wins still drops the caller's value silently, and §3.4 rejects a reserved collision precisely because silent resolution in either direction loses information. The real fix is reservation, and it arrives with the span's mapping: spec ruled that §3.4's reserved set does not reach a span no mapping table covers, and committed the forthcoming failure-isolation mapping to carry these three keys. Found by the proposal 0119 maintenance check. - **A detached fan-out instance no longer opens a second root that abandons the first** (observability §4.4). A detached fan-out's per-instance root is stored under `prefix + (instance index)`, but the ancestor walk that decides whether to open one tested the bare `prefix`, so the test never matched and the arm fired once per inner **node event** rather than once per instance. Each additional inner node re-opened the root, and the second open replaced both the root and its detached invocation span in the observer's state, so the first pair was never ended and never exported. The result for a caller: three traces where there should be two, one instance's inner nodes split across two of them, and spans carrying a parent id that nothing emitted. It needs two or more nodes in the instance subgraph to see, which is why it went unnoticed: with a single node there is one event and one open, and every existing test used that shape. The guard now tests the key the root is actually stored under. The neighbouring bare-prefix test is kept for depth and is documented as redundant rather than load-bearing, since the detached-subgraph path is already guarded a line earlier. diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index b8f86a2..ee292fe 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -148,12 +148,19 @@ a key it does not mention inherits the base's. An override that declares no extras therefore inherits all of them, and one that adjusts a single vendor knob leaves the rest in place. -Two edges are worth knowing. A field the override declares wins over a -base extras key of the same name, so a base `extras={"temperature": 0.9}` -yields to an override's `temperature=0.3` rather than colliding with it. -But naming the same field twice in one override, once declared and once -in its own `extras`, is a config that contradicts itself and is rejected -before the call goes out. +Two edges are worth knowing. The merge does not resolve an inherited +extras key that collides with a field the override declares. A base +`extras={"temperature": 0.9}` with an override `temperature=0.3` rejects +before the call goes out, like any other managed-field collision, because +dropping the inherited key would make one config mean two things +depending on whether you wrote it or a merge assembled it. If the base +reached for `extras`, keep the override in the same channel: +`RuntimeConfig(extras={"temperature": 0.3})` merges per key and sends. + +Note when that surfaces. Attempt 0 uses the base alone, where the +declared field is unset and nothing collides, so a retry config broken +this way sends one successful call and then fails. The error names both +channels and which to change. Withdrawing a base key for a single attempt is not available. Setting it to `None` in the override sends JSON `null` on the wire, which is not the diff --git a/src/openarmature/_managed_extras.py b/src/openarmature/_managed_extras.py index dd5a30e..1fbac85 100644 --- a/src/openarmature/_managed_extras.py +++ b/src/openarmature/_managed_extras.py @@ -57,6 +57,7 @@ def apply_managed_extras( managed: Mapping[str, ManagedArm], *, managed_values: Mapping[str, Any] | None = None, + collision_hint: str | None = None, ) -> None: """Fold ``extras`` into ``body``, reconciling managed-field collisions. @@ -68,6 +69,10 @@ def apply_managed_extras( matching extras value stays a no-op that leaves the body minimal; where absent, the managed value is ``body[key]``. + ``collision_hint`` is appended to a collision's message by a caller that + knows something the check cannot see about where the conflicting values came + from. + Mutates ``body`` in place; ``extras`` is not modified. Raises :class:`ProviderInvalidRequest` on a conflicting non-additive collision. """ @@ -114,7 +119,7 @@ def apply_managed_extras( f"extras key {key!r} conflicts with the mapping-managed wire " f"field {key!r} (managed value {_summarize(managed_value)}, " f"extras value {_summarize(value)}); a managed field cannot be " - f"overridden via extras" + f"overridden via extras" + (f". {collision_hint}" if collision_hint else "") ) diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 6b948d2..5006d13 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -520,6 +520,7 @@ async def complete( def build_body( attempt_config: RuntimeConfig | None, attempt_messages: Sequence[Message], + merged_attempt: bool, ) -> dict[str, Any]: # Proposal 0095: the wire body is assembled PER ATTEMPT so a # retry can vary sampling (0095a per-attempt override) and/or the @@ -527,6 +528,19 @@ def build_body( # across attempts. Absent an override and a reask, every attempt # passes the base config + base messages -> identical body # (today's byte-identical replay). + # + # A collision on a merged attempt reads as a bug in one config, + # and it is a mismatch between two: the base supplied the extras + # key and the override declared the field. Attempt 0 sends + # cleanly, so the only report the caller gets names the channel + # they should change. + hint = ( + "this attempt's config merges the base config with a " + "per-attempt override, so the two values come from different " + "channels; set the field in the same channel the base used" + if merged_attempt + else None + ) return self._build_request_body( attempt_messages, tools, @@ -534,6 +548,7 @@ def build_body( schema_dict, include_response_format=include_response_format, tool_choice=tool_choice, + collision_hint=hint, ) # Per-attempt LLM span surface (observability §5.5 under @@ -631,8 +646,7 @@ def _config_for_attempt( inherits the base -- and the last entry carries forward when the schedule is shorter than the retry count. ``extras`` merges by the same per-key rule: a key the override sets replaces that key, a key it does - not mention inherits the base's, and a base key whose name the override - declares as a field is superseded by that field. Attempt 0 and the no-override + not mention inherits the base's. Attempt 0 and the no-override case return the caller's base config as-is; only the override path returns a fresh ``model_copy``. The caller's config is never mutated either way -- the base path relies on the downstream body build reading @@ -658,25 +672,17 @@ def _config_for_attempt( # a field is an extras KEY rather than the container: a key set in the # override replaces the base's value for that key, a key the override does # not mention inherits the base's. So the container merges. - update = override.model_dump(exclude_none=True, exclude={"extras"}) - merged = {**base_or_empty.extras, **override.extras} - # A field the override declares supersedes a base extras key of the same - # name. Without this the merge manufactures an invalid config out of two - # valid ones: the base's §6 escape-hatch key (unmanaged there, because the - # base left the field unset) is inherited onto an attempt whose override - # DOES set the field, so the mapping emits it, §8.1 sees a managed-field - # collision, and the call rejects pre-send -- killing a retry that - # previously ran. The override's field is the later and more specific - # instruction, so it wins. # - # A key the override itself carries in `extras` is left alone, so - # declaring a field and naming it in the same override's extras still - # collides. That config contradicts itself in one object rather than - # across two, and §8.1's reject is the right answer to it. - for field_name in update: - if field_name not in override.extras: - merged.pop(field_name, None) - update["extras"] = merged + # The merge does not resolve an inherited key that collides with a field + # the override declares; §8.1 rejects it like any other managed-field + # collision. Dropping the key instead would make one config mean two + # things depending on how it was assembled, rejected when a caller writes + # both and accepted when a merge produces both, and §6 separately forbids + # silently discarding a conflicting extras value. A caller who wants the + # override to change such a value keeps it in the same channel the base + # used. + update = override.model_dump(exclude_none=True, exclude={"extras"}) + update["extras"] = {**base_or_empty.extras, **override.extras} return base_or_empty.model_copy(update=update) @staticmethod @@ -703,7 +709,7 @@ def _append_reask_pair( async def _do_complete_with_retry( self, - build_body: Callable[[RuntimeConfig | None, Sequence[Message]], dict[str, Any]], + build_body: Callable[[RuntimeConfig | None, Sequence[Message], bool], dict[str, Any]], base_config: RuntimeConfig | None, base_messages: Sequence[Message], schema_dict: dict[str, Any] | None, @@ -729,7 +735,7 @@ async def _do_complete_with_retry( terminal event still fires per ``complete()`` call. """ if retry is None: - body = build_body(base_config, base_messages) + body = build_body(base_config, base_messages, False) attempt_start = time.perf_counter() try: response = await self._do_complete(body, schema_dict, schema_class) @@ -768,7 +774,7 @@ async def _do_complete_with_retry( attempt = 0 while True: attempt_config = self._config_for_attempt(base_config, overrides, attempt) - body = build_body(attempt_config, transcript) + body = build_body(attempt_config, transcript, bool(attempt and overrides)) attempt_start = time.perf_counter() try: response = await self._do_complete(body, schema_dict, schema_class) @@ -1160,6 +1166,7 @@ def _build_request_body( schema_dict: dict[str, Any] | None, include_response_format: bool = True, tool_choice: ToolChoice | None = None, + collision_hint: str | None = None, ) -> dict[str, Any]: body: dict[str, Any] = { "model": self.model, @@ -1238,7 +1245,7 @@ def _build_request_body( for key, arm in _OPENAI_MANAGED_ARMS.items() if key in body or key in _OPENAI_STRUCTURAL_KEYS } - apply_managed_extras(body, extras, managed) + apply_managed_extras(body, extras, managed, collision_hint=collision_hint) # Spec 0047 §8 belt-and-suspenders: walk the assembled body # once more sorting any dict at every nesting level, in case # a future code path introduces a user-input boundary the diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 05a92b7..1a67acc 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -3516,66 +3516,96 @@ def handler(req: httpx.Request) -> httpx.Response: assert bodies[1]["keep"] == 1, "every unmentioned base key inherits, not just the first" -async def test_override_declared_field_supersedes_inherited_base_extras_key() -> None: +async def test_merged_extras_key_colliding_with_an_overridden_field_rejects() -> None: # A base extras key is the section 6 escape hatch: unmanaged where it was # written, because the base leaves the declared field unset so the mapping # emits nothing of that name. Merging carries it onto a retry whose override - # DOES set the field, where it is managed, and section 8.1 rejects a managed - # collision pre-send. Two individually valid configs would then compose into - # one the provider refuses, killing the retry the caller asked for. + # DOES set the field, where it is managed, and section 8.1 rejects the + # collision pre-send. # - # The override's field is the later and more specific instruction, so it wins - # and the inherited key drops. + # The merge deliberately does not resolve this. Dropping the inherited key + # would make one config mean two things depending on how it was assembled: + # rejected when a caller writes both channels, accepted when a merge produces + # both. Section 6 also forbids silently discarding a conflicting extras value. # - # Not vacuous: without the supersede step the first assertion fails on call - # COUNT (one wire call, not two) because attempt 1 raises before sending, and - # the terminal error is provider_invalid_request rather than the transient - # the retry existed to ride out. Killed by removing the `merged.pop` loop. + # Not vacuous in two directions. The call-count assertion fails if the merge + # starts resolving the collision (three calls instead of one), and the + # category assertion fails if the collision maps to something other than a + # pre-send request error. Killed by reinstating a drop of the colliding key. from openarmature.llm import LlmRetryConfig bodies: list[dict[str, Any]] = [] - calls = {"n": 0} def handler(req: httpx.Request) -> httpx.Response: bodies.append(json.loads(req.content)) - calls["n"] += 1 - if calls["n"] == 1: - return httpx.Response(503, json={"error": {"message": "upstream"}}) - return httpx.Response( - 200, - json={ - "id": "c", - "model": "m", - "choices": [ - {"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"} - ], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, - }, - ) + return httpx.Response(503, json={"error": {"message": "upstream"}}) provider = _collision_provider(handler) try: - await provider.complete( - [UserMessage(content="hi")], - config=RuntimeConfig(extras={"temperature": 0.9, "keep": 1}), - retry=LlmRetryConfig( - max_attempts=2, - backoff=deterministic_backoff(0), - per_attempt_override=[RuntimeConfig(temperature=0.3)], - ), - ) + with pytest.raises(ProviderInvalidRequest) as caught: + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(extras={"temperature": 0.9, "keep": 1}), + retry=LlmRetryConfig( + max_attempts=3, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(temperature=0.3)], + ), + ) + finally: + await provider.aclose() + + # Attempt 0 goes out on the base alone, where nothing collides, so the broken + # retry config is latent until the first retry. That is the ergonomic cost of + # the reject and the reason the message has to name both channels. + assert len(bodies) == 1, f"attempt 0 sends, attempt 1 raises pre-send; got {len(bodies)}" + assert bodies[0]["temperature"] == 0.9 + assert caught.value.category == "provider_invalid_request" + # The message has to be actionable from the text alone: both channels named, + # and the fact that the value was inherited rather than written here. + message = str(caught.value) + assert "temperature" in message + assert "per-attempt override" in message, message + assert "base config" in message, message + assert "same channel" in message, message + + +async def test_override_keeping_both_values_in_one_channel_retries_cleanly() -> None: + # The caller-side fix for the collision above, pinned so the documented + # remedy cannot rot. The base reached for extras, so the override stays in + # extras: the per-key merge replaces that key and the declared field is never + # set, so the mapping emits no temperature and nothing is managed. + # + # Not vacuous: asserting the retried value (not just the call count) is what + # fails if the merge stops letting an override's extras key win. + from openarmature.llm import LlmRetryConfig + + bodies: list[dict[str, Any]] = [] + + def handler(req: httpx.Request) -> httpx.Response: + bodies.append(json.loads(req.content)) + return httpx.Response(503, json={"error": {"message": "upstream"}}) + + provider = _collision_provider(handler) + try: + with pytest.raises(ProviderUnavailable): + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(extras={"temperature": 0.9, "keep": 1}), + retry=LlmRetryConfig( + max_attempts=3, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(extras={"temperature": 0.3, "seed": 7})], + ), + ) finally: await provider.aclose() - assert len(bodies) == 2, f"the retry must still run, got {len(bodies)} call(s)" - # Attempt 0 is untouched: the base's escape-hatch key is what reaches the wire. + assert len(bodies) == 3, f"every attempt sends; got {len(bodies)}" assert bodies[0]["temperature"] == 0.9 - # Attempt 1 sends the override's declared value, once, and the inherited - # extras key is gone rather than colliding with it. - assert bodies[1]["temperature"] == 0.3 - # An unrelated base key still inherits, so the supersede is scoped to the - # colliding name rather than clearing the container. - assert bodies[1]["keep"] == 1 + assert bodies[1]["temperature"] == 0.3, "an override extras key replaces that key" + assert bodies[1]["seed"] == 7 + assert bodies[1]["keep"] == 1, "an unmentioned base key still inherits" async def test_override_naming_a_declared_field_in_its_own_extras_still_collides() -> None: From 9ccf7958758338b4a364de131628ee22835d7c40 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Mon, 28 Sep 2026 09:54:55 -0700 Subject: [PATCH 09/14] Report every schema violation, not just the first StructuredOutputInvalid.error_message named a single violation, because JSON Schema validation raises on the first one it finds. A five-field record that broke four ways reported one of them. The field's documented consumer is a reask builder, so a partial list makes the correction iterative: the model fixes the named field, the next attempt surfaces the next one, and a call spends one round trip per wrong field. Measured against a real model behind an endpoint that accepts response_format and ignores it, extracting a five-field record took four to five attempts and did not recover at all inside the canonical max_attempts of 3. Enumerating brings the same extraction to two. Both schema forms were affected. The class path runs JSON Schema validation before Pydantic so it inherits the dict path's strictness rather than Pydantic's coercion, so it raised from the same place. Every violation is now enumerated, one per line, sorted so the same output and schema produce the same string and a caller can diff corrections across attempts. Order itself is unspecified. A parse failure stays a single violation, since no schema check can run against output that is not well formed. The field remains a human-readable string and response_schema remains a separate attribute; composing the two into a correction is the caller's job, because the implementation authors no prompt text of its own. The exception stays unbounded, because a caller needs every violation to compose one correction. An observer's emitted copy is already bounded by its payload byte cap, so a wide schema lengthens what a caller reads and not what a trace carries. The reask example sends response_schema alongside the objection, which is the caller-side half: the enumeration says what went wrong, the schema says what right looks like, and a model that invented field names was never shown the contract. --- CHANGELOG.md | 2 + docs/concepts/llms.md | 36 ++++++- examples/README.md | 7 +- examples/structured-output-reask/main.py | 76 +++++++++------ src/openarmature/llm/providers/openai.py | 47 ++++++++-- tests/unit/test_llm_provider.py | 114 +++++++++++++++++++++++ 6 files changed, 239 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d88291f..e591e0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Fixed +- **`StructuredOutputInvalid.error_message` names every schema violation, not only the first** (llm-provider §7 / §7.1). Validation stopped at the first failure, so a five-field record that broke four ways reported one of them. The field's documented consumer is a §7.1 reask builder, which made a correction iterative where it could be complete: measured against a real model behind an endpoint that accepts `response_format` and ignores it, extraction took four to five attempts quoting the field alone and did not recover at all inside the canonical `max_attempts=3`. Both schema forms were affected, since the Pydantic-class path runs JSON Schema validation first for strictness and raised from the same place. Every violation is now enumerated, one per line, sorted so the same output and schema give the same string. A parse failure stays a single violation, because no schema check can run against output that is not well-formed. The field is still a human-readable string and `response_schema` is still a separate attribute; composing them into a correction is the caller's job, since the implementation authors no prompt text of its own. An observer's emitted copy remains bounded by its payload byte cap, so a wide schema lengthens what a caller reads rather than what a trace carries. + - **The sdist no longer carries the local follow-up notes or the pinned spec submodule.** Hatch packages what is on disk and does not read `.git/info/exclude`, so `_tasks/` was being collected into the tarball even though git ignores it. Those are internal working notes rather than part of the public artifact set. Checked against PyPI: **no published release contains them**, so this is preventive rather than a leak to clean up. The `openarmature-spec` submodule is excluded in the same change, which is most of the tarball (1124 files in 0.16.0) and reconstructs from the `spec_version` pin, taking the sdist from 3.4MB to 1.4MB. `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager verify a build from source, and the other two are adopter-facing. - **The `conformance.toml` manifest now ships in the wheel.** The bundled `AGENTS.md` tells an agent to read it for per-proposal implementation status, and that is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the distribution and the pointer said "at the repo root", so the instruction resolved for someone working in a clone and for nobody who ran `pip install openarmature`. An agent planning against an accepted proposal could check for an importable symbol and had no equivalent check for a behaviour, which is exactly what the manifest exists to answer. It is force-included at `openarmature/conformance.toml`, beside the other bundled docs, reachable via `importlib.resources.files("openarmature")`, and a wheel built from the sdist gets it too. The pointer now names that path instead of a repo-relative one. **This matters more in this release than the last:** 0085 and 0124 both ship `partial`, and their notes are where which-half-is-missing is recorded. Reported by an agent adopting the package. - **`StructuredOutputInvalid` uses the field names llm-provider §7 specifies** (§7 / §7.1, proposals 0082 + 0095). The exception carried `raw_content` and `failure_description` where §7 names `output_content` and `error_message`, and §7.1 says the `reask` builder receives those two. A caller who wrote the documented builder therefore hit `AttributeError`, which the retry loop catches and converts into `raise exc from reask_error`: the original structured-output failure propagates, reask never runs, and the call fails on the first invalid output exactly as if no builder had been supplied. **This is a pre-1.0 rename of two public attributes**, so a caller reading them directly needs updating; the conformance harness carried an alias bridging the two spellings and that alias is now gone. diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index ee292fe..da40b97 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -209,11 +209,37 @@ second case and not in the first, so a builder can say something different about each. How often you see the first depends on your serving stack rather than on -your code. An endpoint that enforces the schema during decoding will not -produce it; one that ignores `response_format`, or serves a weaker model, -or sits behind a proxy that drops the field, produces it routinely. Both -causes arrive at the same exception, so a builder written for both keeps -working when you change endpoints. +your model. A schema reaches a model through two channels only: the +endpoint enforces it while decoding, or the call puts it in the prompt. An +endpoint that enforces it cannot return a wrong shape. One that accepts +`response_format` and ignores it, or a proxy that drops the field, leaves +neither channel open, and a model that was never told the field names +invents plausible ones. A stronger model does not fix that. + +`error_message` names every way the output failed the schema, one per +line, not just the first violation found. That is what lets a correction +clear several problems in one round instead of one per attempt. + +**Consider sending the schema as well as the objection.** A model that +returns a wrong shape has usually never seen the schema, and listing what +is wrong is not the same as saying what is right. +`StructuredOutputInvalid` carries `response_schema`, so a builder can +include the contract itself: + +```python +def correct(err): + if err.finish_reason == "length": + return "That reply was cut off. Send the whole object." + return ( + f"{err.error_message}\n\nYou sent:\n{err.output_content}\n\n" + "Return only JSON matching this schema exactly:\n" + f"{json.dumps(err.response_schema, indent=2)}" + ) +``` + +The two branches need different information rather than different +wording. A truncated reply already acted on the schema and ran out of +room, so resending the schema tells it nothing. Reask composes with a `per_attempt_override`, and the two halves do different work. The builder changes what you say; the override changes diff --git a/examples/README.md b/examples/README.md index 1bd3855..37195c8 100644 --- a/examples/README.md +++ b/examples/README.md @@ -161,8 +161,11 @@ objection back to it, and a per-attempt override raises the ceiling so the retry has somewhere to put the answer. Demonstrates: `complete(response_schema=...)` raising `StructuredOutputInvalid` instead of returning a half-built object, `LlmRetryConfig(reask=...)` making that failure -retryable for one call, a builder reading `exc.output_content` and -`exc.error_message` and branching on which failure it got, +retryable for one call, a builder reading `exc.output_content`, +`exc.error_message`, `exc.finish_reason` and `exc.response_schema` and +branching on which failure it got (the two arms need different information, +not just different wording, and the schema-mismatch arm sends the schema +itself rather than only the objection to it), `LlmRetryConfig(per_attempt_override=...)` applying a config schedule to retries only, the framework appending the model's reply and the correction as an alternating transcript while authoring no prompt of its own, and reask diff --git a/examples/structured-output-reask/main.py b/examples/structured-output-reask/main.py index 45d0952..748c85d 100644 --- a/examples/structured-output-reask/main.py +++ b/examples/structured-output-reask/main.py @@ -30,14 +30,20 @@ **Why the ceiling and not the wrong-shaped answer.** The ceiling is the failure this demo can guarantee, on any endpoint and any model. The wrong-shaped answer is the one you are more likely to meet, and whether you -meet it at all is a property of your serving stack rather than of your code. -An endpoint that enforces the schema during decoding cannot produce it. An -endpoint that ignores ``response_format``, or serves a weaker model behind -one, produces it routinely, and so does a proxy that drops the field on the -way through. Both failures arrive at the same exception and the same builder -handles both, which is the point: you do not have to know in advance which -kind of endpoint you are pointed at, and you keep working when someone moves -you to a different one. +meet it is a property of your serving stack rather than of your model. + +A schema reaches a model through exactly two channels: the endpoint enforces +it while decoding, or the call puts it in the prompt. An endpoint that +enforces it cannot produce a wrong shape. One that accepts +``response_format`` and ignores it, or a proxy that drops the field on the +way through, leaves neither channel open, and then the model is not guessing +badly so much as guessing: it has never been told the field names, so it +invents reasonable ones. That is not a weak-model failure and a stronger +model does not fix it. + +Both failures arrive at the same exception, so one builder covers both, and +you do not have to know in advance which kind of endpoint you are pointed at. +What the builder puts in the correction is what decides whether it recovers. **What's interesting in the implementation:** @@ -50,11 +56,19 @@ model cannot satisfy is usually a bug in the schema, not a transient. - The builder receives the exception and returns the correction as a string. It reads ``exc.output_content`` (verbatim, what the model actually sent), - ``exc.error_message`` (what the reader objected to), and - ``exc.finish_reason`` (``"length"`` when the ceiling ended the reply). This - one branches on that last signal and says something different about a - truncation than about a model that simply answered wrongly, which is the - kind of judgement only the caller can make. + ``exc.error_message`` (what the reader objected to), ``exc.finish_reason`` + (``"length"`` when the ceiling ended the reply) and ``exc.response_schema`` + (the shape that was asked for). It branches on ``finish_reason``, because + the two failures need different information rather than different wording: + a truncated reply understood the schema and ran out of room, so repeating + the schema at it adds nothing, while a complete-and-wrong reply usually + never saw the schema at all. +- **Send the schema as well as the objection.** ``error_message`` lists every + way the reply failed the schema, so one correction clears all of them at + once rather than one per attempt. That is what keeps a small budget viable. + Including ``response_schema`` on top says what right looks like rather than + only what was wrong, which is the more useful thing to tell a model that + invented its own field names because it was never shown the contract. - ``LlmRetryConfig(per_attempt_override=...)`` is a schedule of partial configs applied to retries only. Attempt 0 runs the caller's config untouched; each retry merges the next entry over it. A schedule shorter @@ -163,17 +177,15 @@ def build_correction(exc: StructuredOutputInvalid) -> str: appends the model's own reply before this, so it reads as a conversation: the model answered, you point at the problem, it answers again. """ - # Both halves of the exception matter. The verbatim output lets the model - # see what it actually sent rather than what it meant to send, and the - # validator's complaint names the constraint it missed. A correction - # carrying neither is the original prompt again, which earns the original - # answer. + # The verbatim output lets the model see what it actually sent rather than + # what it meant to send. A correction without it is the original prompt + # again, which earns the original answer. raw = exc.output_content.strip() - # A reply that stops mid-object is a spend problem, not a comprehension - # problem, and saying "that did not match the schema" about it would be - # misleading. `finish_reason` is how the framework reports which one - # happened: "length" means the ceiling ended the reply, so the model was - # never given the chance to be wrong. + # The two failures need different INFORMATION, not just different wording, + # which is why this branches rather than composing one message for both. + # `finish_reason` of "length" means the ceiling ended the reply: the model + # understood the shape and ran out of room, so repeating the shape at it + # tells it nothing it did not already act on. if exc.finish_reason == "length": return ( "That reply stopped before the object closed, so it could not be " @@ -182,14 +194,24 @@ def build_correction(exc: StructuredOutputInvalid) -> str: "Send the whole object this time. Keep every value as short as " "the report allows and add nothing beyond the required fields." ) + # Anything else means the reply was complete and wrong, and the usual cause + # is that the model never saw the schema. It only reaches the model when the + # endpoint enforces it during decoding or when the call falls back to + # putting it in the prompt, so a reply with invented field names is what a + # request that did neither looks like. + # + # Send the schema as well as the objection. `error_message` lists every way + # the reply broke the schema, so one round clears all of them; the schema + # says what right looks like rather than what was wrong, which is what a + # model that never saw it actually needs. return ( "That reply did not match the required schema. The validator said: " f"{exc.error_message}\n\n" f"You sent:\n{raw}\n\n" - "Send the JSON object again, corrected. Return only the object, with " - "no surrounding prose and no markdown fence. `mass_kg` must be a bare " - 'number in kilograms, so write 1340 rather than "1,340 kg". ' - "`outcome` must be exactly one of: landed, lost, unconfirmed." + "Return only a JSON object matching this schema exactly, with no extra " + "properties, no surrounding prose and no markdown fence:\n" + f"{json.dumps(exc.response_schema, indent=2)}\n\n" + 'Write `mass_kg` as a bare number, so 1340 rather than "1,340 kg".' ) diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 5006d13..912b711 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -1529,7 +1529,7 @@ def _parse_and_validate( "response failed JSON Schema validation", response_schema=schema_dict, output_content=content, - error_message=_format_jsonschema_failure(exc), + error_message=_format_jsonschema_failure(exc, parsed_dict, schema_dict), ) from exc except jsonschema.SchemaError as exc: raise StructuredOutputInvalid( @@ -1556,7 +1556,7 @@ def _parse_and_validate( "response failed JSON Schema validation", response_schema=schema_dict, output_content=content, - error_message=_format_jsonschema_failure(exc), + error_message=_format_jsonschema_failure(exc, parsed_dict, schema_dict), ) from exc except jsonschema.SchemaError as exc: # Safety net: validate_response_schema's pre-validation should @@ -1572,14 +1572,43 @@ def _parse_and_validate( return parsed_dict -def _format_jsonschema_failure(exc: jsonschema.ValidationError) -> str: - """jsonschema.ValidationError.message describes the value mismatch - (e.g., "'30' is not of type 'integer'") but doesn't include the - failing field path. Prefix with ``json_path`` (e.g., ``$.age``) so - the error_message string carries both, matching the dict- - schema and class-schema paths. +def _format_jsonschema_failure( + exc: jsonschema.ValidationError, + instance: Any, + schema: dict[str, Any], +) -> str: + """Describe every way ``instance`` fails ``schema``, one per line. + + Each line pairs the failing location with what was wrong there, e.g. + ``$.age: '30' is not of type 'integer'``. """ - return f"{exc.json_path}: {exc.message}" + # §7's error_message describes how the OUTPUT failed the SCHEMA, so it names + # every violation rather than the one that happened to be detected first. + # `jsonschema.validate` raises on a single error; iter_errors yields them all. + # A caller composing a reask correction (§7.1) otherwise pays one model round + # trip per wrong field and cannot converge inside a small max_attempts. + # + # Sorted so the same output and schema always produce the same string, which + # lets a caller diff corrections across attempts. Order is not asserted. + # + # Unbounded on the exception by design: the caller needs every violation to + # compose one correction. An observer's emitted copy is separately bounded by + # its payload byte cap (§5.5.5), so a wide schema lengthens the field a caller + # reads and not the value a trace carries. + try: + # iter_errors is overloaded on its instance type and pyright cannot + # narrow the yielded element, so the validator is typed at the boundary. + validator = cast("Any", jsonschema.Draft202012Validator(schema)) + found = cast("list[jsonschema.ValidationError]", list(validator.iter_errors(instance))) + errors = sorted(found, key=lambda e: (str(e.json_path), e.message)) + except Exception: + # Enumeration is a better report of the same failure, never a new failure + # mode. Any schema the enumerating validator cannot process falls back to + # the error already in hand. + errors = [] + if not errors: + return f"{exc.json_path}: {exc.message}" + return "\n".join(f"{e.json_path}: {e.message}" for e in errors) _SCHEMA_DIRECTIVE_TEMPLATE = ( diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 1a67acc..7353505 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1419,6 +1419,120 @@ def _503(_req: httpx.Request) -> httpx.Response: assert failed_events[0].response_model is None +async def test_error_message_names_every_schema_violation_not_just_the_first() -> None: + # Section 7's error_message describes how the output failed the schema, so it + # names every violation. The alternative is not a shorter message but an + # iterative one: section 7.1's reask builder quotes this field, and one + # violation per round costs one model call per wrong field. + # + # Not vacuous. `jsonschema.validate` raises on a single error, so the naive + # implementation passes a test that asserts only one of these strings. Every + # assertion below except the first names a violation the raising error does + # NOT carry, and the count assertion fails if enumeration regresses to one. + # Killed by returning `f"{exc.json_path}: {exc.message}"` from + # `_format_jsonschema_failure`. + from openarmature.llm import StructuredOutputInvalid + + schema = { + "type": "object", + "properties": { + "name": {"type": "string"}, + "age": {"type": "integer"}, + "city": {"type": "string"}, + }, + "required": ["name", "age", "city"], + "additionalProperties": False, + } + + def _handler(_req: httpx.Request) -> httpx.Response: + # Well-formed JSON that breaks the schema four separate ways: three + # required properties absent and one property the schema forbids. + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": '{"nickname":"Al"}'}, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + provider = OpenAIProvider( + base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) + ) + try: + with pytest.raises(StructuredOutputInvalid) as caught: + await provider.complete([UserMessage(content="hi")], response_schema=schema) + finally: + await provider.aclose() + + message = caught.value.error_message + assert "'name' is a required property" in message, message + assert "'age' is a required property" in message, message + assert "'city' is a required property" in message, message + assert "nickname" in message, "the forbidden property must be named too" + # One line per violation, so a caller can quote the whole thing at a model. + assert len(message.splitlines()) == 4, message + # Order is deliberately not asserted. The implementation sorts so the same + # output and schema give the same string, which lets a caller diff + # corrections across attempts, but which order is unspecified: pinning one + # here would make a legitimate reordering fail. Comparing the value to + # itself would look like a stability check and assert nothing. + + +async def test_error_message_for_a_parse_failure_stays_a_single_violation() -> None: + # A schema violation cannot be evaluated against output that is not + # well-formed, so the field describes the parse failure and nothing else. + # Enumeration applies to schema validation, not to every failure on the path. + # + # Not vacuous: an implementation that tried to enumerate here would have no + # instance to enumerate against and would either raise or report zero + # violations, both of which fail the substring assertion. + from openarmature.llm import StructuredOutputInvalid + + schema = { + "type": "object", + "properties": {"name": {"type": "string"}}, + "required": ["name"], + } + + def _handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": '{"name": "Al'}, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + provider = OpenAIProvider( + base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) + ) + try: + with pytest.raises(StructuredOutputInvalid) as caught: + await provider.complete([UserMessage(content="hi")], response_schema=schema) + finally: + await provider.aclose() + + message = caught.value.error_message + assert "required property" not in message, message + assert message.strip() != "" + + async def test_complete_structured_output_failure_event_carries_response_surface() -> None: # Proposal 0082: a structured_output_invalid failure is a completion # whose validation gate failed, so the LlmFailedEvent carries the From 9933608202ea671452318533ecb87e7d7507f5bb Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Mon, 28 Sep 2026 11:21:12 -0700 Subject: [PATCH 10/14] Ship the fixtures the sdist's tests read Two defects found in review, both in work from this PR. The sdist kept tests/ so a downstream packager could validate a build from source, and excluded the pinned spec submodule to cut the tarball from 3.4MB to 1.4MB. The conformance drivers read their fixtures out of that submodule, so the suite shipped without its inputs: building the sdist and running it gives 48 failures across nine files, on an empty corpus. The exclusion now keeps the conformance directories and drops the proposals and prose around them, which takes it to 1 failure at 2.0MB. That one is unrelated and predates all of this: the AGENTS.md drift check runs the generator, which shells out to git, and an unpacked tarball is not a repository. The guard that should have caught it asserted the exclude list's text, which stayed true while its effect changed. It now builds the artifact and looks inside: fixtures present, proposals and private notes absent. Three mutations on the exclude patterns each kill it on their own assertion. Second, the schema-violation enumeration hardcoded the 2020-12 validator while the rest of the codebase selects one from the schema's own $schema. A Draft 7 schema using the tuple form of `items` raises under 2020-12, and the fallback then returned the single error the enumeration exists to replace, so error_message silently reverted for every caller declaring an older draft. The broad catch that was meant to stop enumeration introducing a new failure mode is what hid it. Every test written for the enumeration omitted $schema, which defaults to the latest draft, so nothing exercised the disagreement. A Draft 7 case now covers it, including a violation inside the tuple-form array that a 2020-12 validator cannot reach. --- pyproject.toml | 8 ++- src/openarmature/llm/providers/openai.py | 10 +++- tests/test_smoke.py | 60 +++++++++++++++++++-- tests/unit/test_llm_provider.py | 68 ++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c06802a..0cdb548 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -117,9 +117,15 @@ default-groups = ["dev", "docs", "examples", "observability"] # `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager # verify a build from source, and the other two are adopter-facing and small. [tool.hatch.build.targets.sdist] +# The sdist keeps tests/ so a downstream packager can validate the build, which +# only means something if the suite can run. The conformance drivers read their +# fixtures from openarmature-spec/spec//conformance/, so excluding +# the whole submodule left 48 tests failing on a missing corpus. Keep the +# fixtures; drop the proposals, prose and tooling around them. exclude = [ "_tasks/", - "openarmature-spec/", + "openarmature-spec/*", + "!openarmature-spec/spec/*/conformance/**", ] [tool.hatch.build.targets.wheel] diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 912b711..a95282a 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -63,6 +63,7 @@ import httpx import jsonschema +from jsonschema.validators import validator_for from pydantic import BaseModel, ValidationError from openarmature._managed_extras import ManagedArm, apply_managed_extras @@ -1596,9 +1597,16 @@ def _format_jsonschema_failure( # its payload byte cap (§5.5.5), so a wide schema lengthens the field a caller # reads and not the value a trace carries. try: + # validator_for reads the schema's own $schema, matching both the + # boundary check in llm/provider.py and what jsonschema.validate() does + # internally. Fixing a draft here instead would silently stop enumerating + # for every schema that declares an older one: Draft 7's tuple-form + # `items` raises under the 2020-12 validator, and the fallback below + # would hand back the single error this function exists to replace. + # # iter_errors is overloaded on its instance type and pyright cannot # narrow the yielded element, so the validator is typed at the boundary. - validator = cast("Any", jsonschema.Draft202012Validator(schema)) + validator = cast("Any", validator_for(schema)(schema)) found = cast("list[jsonschema.ValidationError]", list(validator.iter_errors(instance))) errors = sorted(found, key=lambda e: (str(e.json_path), e.message)) except Exception: diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 229ea5f..e597ff0 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -135,6 +135,11 @@ def test_the_sdist_excludes_the_private_follow_up_notes() -> None: # # Guarded by config shape rather than by building an sdist in the suite, # because the way this breaks is someone editing or dropping the exclude. + # + # What this CANNOT catch is a correct-looking exclude with the wrong effect, + # which is how the conformance fixtures were dropped out from under the tests + # that read them. `test_the_sdist_ships_a_runnable_test_suite` builds the + # thing and looks inside; this one stays because it names the intent. pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" config = tomllib.loads(pyproject_path.read_text()) exclude = config["tool"]["hatch"]["build"]["targets"]["sdist"]["exclude"] @@ -143,10 +148,10 @@ def test_the_sdist_excludes_the_private_follow_up_notes() -> None: "`_tasks/` must be excluded from the sdist. It holds private working notes, " f"and .git/info/exclude does not reach hatch. Current excludes: {exclude}" ) - # The pinned spec submodule is most of the tarball and reconstructs from the - # version pin, so it is excluded too. Not a privacy matter, just size. - assert "openarmature-spec/" in exclude, ( - f"the pinned spec submodule should stay out of the sdist; excludes: {exclude}" + # Most of the pinned submodule is proposals and spec prose that reconstructs + # from the version pin, so it stays out. Not a privacy matter, just size. + assert any(e.startswith("openarmature-spec/") for e in exclude), ( + f"the pinned spec submodule's bulk should stay out of the sdist; excludes: {exclude}" ) # Deliberately NOT excluded, asserted so a future tidy-up does not quietly # drop them: tests let a packager verify a build from source, and the other @@ -156,3 +161,50 @@ def test_the_sdist_excludes_the_private_follow_up_notes() -> None: f"{kept} is excluded from the sdist; it was kept on purpose, so if that " "changed the reasoning in pyproject.toml needs changing with it" ) + + +def test_the_sdist_ships_a_runnable_test_suite() -> None: + # The sdist keeps tests/ so a downstream packager can validate a build from + # source. That is only worth anything if the suite's inputs ship with it: the + # conformance drivers read their fixtures out of the submodule, and excluding + # the submodule wholesale left 48 tests failing on an empty corpus while every + # config-shape assertion above still passed. + # + # So this builds the artifact and looks inside it. Slower than reading + # pyproject, and the only form that can fail for the reason that matters: the + # exclusion is a pattern whose effect is not readable off its own text. + # + # Not vacuous in three directions. Dropping the negation pattern empties + # `fixtures`; dropping the `openarmature-spec/*` exclude lets `proposals` + # through; dropping the `_tasks/` exclude lets `notes` through. Each assertion + # fails on its own mutation. + import subprocess + import tarfile + import tempfile + + repo = Path(__file__).resolve().parent.parent + with tempfile.TemporaryDirectory() as out: + built = subprocess.run( + ["uv", "build", "--sdist", "--out-dir", out], + cwd=repo, + capture_output=True, + text=True, + ) + assert built.returncode == 0, f"sdist build failed: {built.stderr[-2000:]}" + tarballs = sorted(Path(out).glob("*.tar.gz")) + assert len(tarballs) == 1, f"expected one sdist, got {tarballs}" + with tarfile.open(tarballs[0]) as tar: + names = tar.getnames() + + fixtures = [n for n in names if "/conformance/" in n and n.endswith(".yaml")] + proposals = [n for n in names if "/openarmature-spec/proposals/" in n] + notes = [n for n in names if "/_tasks/" in n] + suite = [n for n in names if "/tests/conformance/" in n and n.endswith(".py")] + + assert suite, "the sdist ships no conformance tests, so nothing needs their fixtures" + assert fixtures, ( + "the sdist ships conformance tests but none of the fixtures they read. " + "Running the suite from this sdist fails on an empty corpus." + ) + assert not proposals, f"spec proposals should not ship in the sdist: {proposals[:3]}" + assert not notes, f"private follow-up notes should not ship in the sdist: {notes[:3]}" diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 7353505..497e3fa 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1486,6 +1486,74 @@ def _handler(_req: httpx.Request) -> httpx.Response: # itself would look like a stability check and assert nothing. +async def test_error_message_enumerates_under_a_schema_declaring_an_older_draft() -> None: + # Enumeration has to use the validator the schema asks for. Fixing one draft + # looks harmless while every test schema omits `$schema`, because the absent + # key defaults to the latest draft and the two agree. + # + # They stop agreeing on a schema that declares an older one. Draft 7 allows + # the tuple form of `items`, which 2020-12 spells `prefixItems`, so a 2020-12 + # validator raises on it. The enumeration's fallback would then hand back the + # single error it exists to replace, and the field would silently revert for + # every caller using an older draft. + # + # Not vacuous: this is the test whose absence let that ship. It fails with a + # count of 1 if the validator is hardcoded to any draft, and it exercises the + # fallback path rather than only the happy one. Killed by restoring + # `jsonschema.Draft202012Validator(schema)`. + from openarmature.llm import StructuredOutputInvalid + + schema = { + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "properties": { + "name": {"type": "string"}, + "pair": {"type": "array", "items": [{"type": "string"}, {"type": "integer"}]}, + }, + "required": ["name", "pair", "city"], + "additionalProperties": False, + } + + def _handler(_req: httpx.Request) -> httpx.Response: + # Breaks the schema four ways, one of them inside the tuple-form array so + # the draft actually matters to the result rather than only to the walk. + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + { + "index": 0, + "message": { + "role": "assistant", + "content": '{"nickname": "Al", "pair": ["x", "y"]}', + }, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + provider = OpenAIProvider( + base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) + ) + try: + with pytest.raises(StructuredOutputInvalid) as caught: + await provider.complete([UserMessage(content="hi")], response_schema=schema) + finally: + await provider.aclose() + + message = caught.value.error_message + assert len(message.splitlines()) == 4, message + assert "'name' is a required property" in message, message + assert "'city' is a required property" in message, message + assert "nickname" in message, message + # The tuple-form element failure is the one a 2020-12 validator cannot reach. + assert "pair[1]" in message, message + + async def test_error_message_for_a_parse_failure_stays_a_single_violation() -> None: # A schema violation cannot be evaluated against output that is not # well-formed, so the field describes the parse failure and nothing else. From 9b7bb5ed6264983bdaeb884da7d1aa63e01b4dcc Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Mon, 28 Sep 2026 15:38:15 -0700 Subject: [PATCH 11/14] Make the sdist suite clean and the fallback audible Two follow-ups from the review, both about a check that reports success while doing nothing. The AGENTS.md drift test ran the generator, which asks git for the spec submodule's pinned tag so a bundle cannot ship built off an untagged commit. An unpacked sdist has no repository, so the test failed there for a reason unrelated to drift, and a packager validating a build from source saw a red suite. It now skips where there is genuinely no repository. That skip is keyed on the absence of a repository rather than on the generator failing, because the wider condition would let a broken generator read as a clean CI run. A skip condition cannot be caught by the test it guards, so a separate check asserts the condition is false inside the repository, cross-checking the filesystem and PATH against a helper that shells out to git. Widening the condition fails that check instead of going unnoticed. The schema-violation enumeration falls back to the single violation it was handed when the validator cannot process the schema, so a structured_output_invalid is never replaced by something the caller cannot handle. The fallback returned a well-formed message, which is indistinguishable from a schema that genuinely broke one way, and the hardcoded draft fixed in the previous commit reverted the field for every caller on an older draft with nothing to show for it. It now says so, and names the draft the schema declared. The test drives that by making the validator lookup raise, because the point is that we do not know which schemas reach the path. The log record is the only observable: the returned message is valid either way. --- src/openarmature/llm/providers/openai.py | 20 ++++++- tests/test_agents_md_drift.py | 55 ++++++++++++++++++ tests/unit/test_llm_provider.py | 73 ++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index a95282a..702cabb 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -1609,10 +1609,24 @@ def _format_jsonschema_failure( validator = cast("Any", validator_for(schema)(schema)) found = cast("list[jsonschema.ValidationError]", list(validator.iter_errors(instance))) errors = sorted(found, key=lambda e: (str(e.json_path), e.message)) - except Exception: + except Exception as enumeration_error: # Enumeration is a better report of the same failure, never a new failure - # mode. Any schema the enumerating validator cannot process falls back to - # the error already in hand. + # mode, so any schema the enumerating validator cannot process falls back + # to the error already in hand rather than replacing a + # structured_output_invalid with something the caller cannot handle. + # + # Logged because the fallback is invisible otherwise: it returns a + # well-formed single-violation message, which is indistinguishable from a + # schema that genuinely broke one way. A hardcoded draft reaching this + # path silently reverted the field for every caller on an older draft, + # and nothing said so. + _log.warning( + "could not enumerate schema violations (%s: %s); reporting the first " + "violation only. The schema declares %s.", + type(enumeration_error).__name__, + enumeration_error, + schema.get("$schema", "no $schema, so the latest draft applies"), + ) errors = [] if not errors: return f"{exc.json_path}: {exc.message}" diff --git a/tests/test_agents_md_drift.py b/tests/test_agents_md_drift.py index a6fb31b..4da2fda 100644 --- a/tests/test_agents_md_drift.py +++ b/tests/test_agents_md_drift.py @@ -27,9 +27,13 @@ from __future__ import annotations import importlib.util +import shutil +import subprocess from pathlib import Path from typing import Any +import pytest + REPO_ROOT = Path(__file__).resolve().parent.parent OUTPUT = REPO_ROOT / "src" / "openarmature" / "AGENTS.md" PATTERNS_DIR = REPO_ROOT / "src" / "openarmature" / "_patterns" @@ -52,6 +56,34 @@ def _load_generator() -> Any: return module +def _in_a_git_checkout() -> bool: + """Whether this tree is a git repository the generator can interrogate.""" + if shutil.which("git") is None: + return False + probe = subprocess.run( + ["git", "-C", str(REPO_ROOT), "rev-parse", "--git-dir"], + capture_output=True, + text=True, + ) + return probe.returncode == 0 + + +# The bundle records which spec tag it was built from, so the generator asks git +# for the submodule's pinned tag and refuses to build off an untagged commit. An +# sdist ships the source without the repository, so there is nothing to ask and +# the drift check cannot run at all rather than running and passing. +# +# Deliberately narrow: keyed on the absence of a repository, not on the +# generator failing. Skipping whenever the generator raised would let a broken +# generator read as a clean run in CI, where the repository is always present +# and this never skips. +@pytest.mark.skipif( + not _in_a_git_checkout(), + reason=( + "not a git checkout, so the generator cannot resolve the spec submodule's " + "pinned tag; drift is checked in the repository, not from an sdist" + ), +) def test_agents_md_matches_generator_output() -> None: generator = _load_generator() expected = generator.build() @@ -59,6 +91,29 @@ def test_agents_md_matches_generator_output() -> None: assert actual == expected, f"src/openarmature/AGENTS.md is out of date with its sources.\n{REGEN_HINT}" +def test_the_drift_check_is_not_skipping_in_this_repository() -> None: + # A skip condition cannot be caught by the test it guards: widen it and the + # drift check quietly stops running while the suite still reports green. That + # is the failure this repository treats as the null result, one level up. + # + # So the condition is asserted separately. Here the repository is present by + # definition (this file is in it), so the guard must evaluate False, and a + # widened condition fails here instead of going unnoticed. + # Both preconditions are checked by a different mechanism than the helper + # uses, so this is a cross-check rather than a restatement: the helper shells + # out to git, these read the filesystem and PATH. Where they disagree, the + # helper is wrong. + if shutil.which("git") is None: + pytest.skip("git is absent, so the drift check could not run either way") + if not (REPO_ROOT / ".git").exists(): + pytest.skip("no repository here, as in an unpacked sdist") + assert _in_a_git_checkout(), ( + "the drift check's skip condition is true inside the repository, so the " + "check is not running. Narrow the condition: it should only skip where " + "there is genuinely no git repository, such as an unpacked sdist." + ) + + def test_patterns_dir_matches_generator_output() -> None: generator = _load_generator() expected_payload: dict[str, str] = generator.build_patterns_data() diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 497e3fa..6fe0f8a 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1486,6 +1486,79 @@ def _handler(_req: httpx.Request) -> httpx.Response: # itself would look like a stability check and assert nothing. +async def test_a_failed_enumeration_says_so_rather_than_quietly_reporting_one( + caplog: pytest.LogCaptureFixture, +) -> None: + # The enumeration falls back to the single violation it was handed when the + # validator cannot process the schema, so a structured_output_invalid is + # never replaced by something the caller cannot handle. That fallback returns + # a well-formed message, which is indistinguishable from a schema that + # genuinely broke one way, and a hardcoded draft reaching this path reverted + # the field for every caller on an older draft with nothing to show for it. + # + # So the fallback is audible. Driven by monkeypatching the validator lookup + # rather than by finding a schema that breaks it, because the whole point is + # that we do not know which schemas reach here. + # + # Not vacuous: the fallback still produces a valid one-line message, so no + # assertion on the returned value can distinguish it from a genuine single + # violation. The log record is the only observable. Killed by dropping the + # _log.warning call. + import logging + + from openarmature.llm import StructuredOutputInvalid + from openarmature.llm.providers import openai as openai_module + + schema = { + "type": "object", + "properties": {"name": {"type": "string"}, "age": {"type": "integer"}}, + "required": ["name", "age"], + } + + def _handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "id": "c", + "model": "m", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": '{"name": "Al"}'}, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + }, + ) + + def _explode(_schema: Any) -> Any: + raise RuntimeError("validator lookup unavailable") + + provider = OpenAIProvider( + base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) + ) + original = openai_module.validator_for + openai_module.validator_for = _explode + try: + with caplog.at_level(logging.WARNING, logger="openarmature.llm.providers.openai"): + with pytest.raises(StructuredOutputInvalid) as caught: + await provider.complete([UserMessage(content="hi")], response_schema=schema) + finally: + openai_module.validator_for = original + await provider.aclose() + + # The caller still gets a usable single-violation message, not an exception + # from the enumeration itself. + assert "'age' is a required property" in caught.value.error_message + + warnings = [r for r in caplog.records if r.levelno >= logging.WARNING] + assert warnings, "a fallback to single-violation reporting must not be silent" + logged = warnings[0].getMessage() + assert "enumerate" in logged, logged + assert "RuntimeError" in logged, "the log must name what actually failed" + + async def test_error_message_enumerates_under_a_schema_declaring_an_older_draft() -> None: # Enumeration has to use the validator the schema asks for. Fixing one draft # looks harmless while every test schema omits `$schema`, because the absent From ad25a937ad209fd6c0d6184fa98489e61254b141 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Mon, 28 Sep 2026 16:17:32 -0700 Subject: [PATCH 12/14] Drop a redundant logging import in a test The module already imports logging at the top, so the function-local import did nothing. Flagged by both code scanners. The file's other function-local imports are all from openarmature, kept local so collection does not pull the package in at import time. That does not apply to a stdlib module already imported at module scope. --- tests/unit/test_llm_provider.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index 6fe0f8a..be470ab 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1504,8 +1504,6 @@ async def test_a_failed_enumeration_says_so_rather_than_quietly_reporting_one( # assertion on the returned value can distinguish it from a genuine single # violation. The log record is the only observable. Killed by dropping the # _log.warning call. - import logging - from openarmature.llm import StructuredOutputInvalid from openarmature.llm.providers import openai as openai_module From 68fa852ea99316a78190a8ecf8f641cbe0f406b0 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 2 Oct 2026 08:02:24 -0700 Subject: [PATCH 13/14] Defer the error_message enumeration Reporting every schema violation rather than the first was not in this PR's scope. It came out of testing the reask example, became a spec question, and got built because spec said the field's contents were undefined either way and moving first risked nothing. In two days it produced four defects. A hardcoded validator draft that silently reverted the field for every older-draft schema. A false claim that the emitted copy was bounded. A reachability change in the retry path. And, found by review rather than by test, a regression: the walk stopped at the top level, so every anyOf and oneOf reported "is not valid under any of the given schemas" where the single error it replaced had named the failing leaf. Pydantic renders every Optional and every union that way, so the field got worse for the shape callers are most likely to have, and the correction a reask builder sends named no field at all. That one is cheap to fix and not the reason to stop. The reason is that every test written for the enumeration used a flat type/required schema, which is the same single-shape blind spot that let the draft bug through one commit earlier. The field flows onto events, into both observers, into a transcript fed back to a model, and into conformance substring assertions. Two wrong guesses about which shape breaks it is not a basis for a third. Deferring costs nothing: there is no published spec text yet, and the convergence problem it solved is already solved here by the example's builder sending response_schema, which is caller-side and has no blast radius. It returns next cycle with the proposal, able to descend into nested context errors from the start. --- CHANGELOG.md | 2 -- docs/concepts/llms.md | 16 +++++++--------- examples/README.md | 2 +- examples/structured-output-reask/main.py | 21 +++++++++++---------- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e591e0f..d88291f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,8 +41,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The ### Fixed -- **`StructuredOutputInvalid.error_message` names every schema violation, not only the first** (llm-provider §7 / §7.1). Validation stopped at the first failure, so a five-field record that broke four ways reported one of them. The field's documented consumer is a §7.1 reask builder, which made a correction iterative where it could be complete: measured against a real model behind an endpoint that accepts `response_format` and ignores it, extraction took four to five attempts quoting the field alone and did not recover at all inside the canonical `max_attempts=3`. Both schema forms were affected, since the Pydantic-class path runs JSON Schema validation first for strictness and raised from the same place. Every violation is now enumerated, one per line, sorted so the same output and schema give the same string. A parse failure stays a single violation, because no schema check can run against output that is not well-formed. The field is still a human-readable string and `response_schema` is still a separate attribute; composing them into a correction is the caller's job, since the implementation authors no prompt text of its own. An observer's emitted copy remains bounded by its payload byte cap, so a wide schema lengthens what a caller reads rather than what a trace carries. - - **The sdist no longer carries the local follow-up notes or the pinned spec submodule.** Hatch packages what is on disk and does not read `.git/info/exclude`, so `_tasks/` was being collected into the tarball even though git ignores it. Those are internal working notes rather than part of the public artifact set. Checked against PyPI: **no published release contains them**, so this is preventive rather than a leak to clean up. The `openarmature-spec` submodule is excluded in the same change, which is most of the tarball (1124 files in 0.16.0) and reconstructs from the `spec_version` pin, taking the sdist from 3.4MB to 1.4MB. `tests/`, `examples/` and `docs/` stay in deliberately: tests let a packager verify a build from source, and the other two are adopter-facing. - **The `conformance.toml` manifest now ships in the wheel.** The bundled `AGENTS.md` tells an agent to read it for per-proposal implementation status, and that is the only artifact answering "is this real in the version I have installed" for a behaviour with no importable name. The file was not in the distribution and the pointer said "at the repo root", so the instruction resolved for someone working in a clone and for nobody who ran `pip install openarmature`. An agent planning against an accepted proposal could check for an importable symbol and had no equivalent check for a behaviour, which is exactly what the manifest exists to answer. It is force-included at `openarmature/conformance.toml`, beside the other bundled docs, reachable via `importlib.resources.files("openarmature")`, and a wheel built from the sdist gets it too. The pointer now names that path instead of a repo-relative one. **This matters more in this release than the last:** 0085 and 0124 both ship `partial`, and their notes are where which-half-is-missing is recorded. Reported by an agent adopting the package. - **`StructuredOutputInvalid` uses the field names llm-provider §7 specifies** (§7 / §7.1, proposals 0082 + 0095). The exception carried `raw_content` and `failure_description` where §7 names `output_content` and `error_message`, and §7.1 says the `reask` builder receives those two. A caller who wrote the documented builder therefore hit `AttributeError`, which the retry loop catches and converts into `raise exc from reask_error`: the original structured-output failure propagates, reask never runs, and the call fails on the first invalid output exactly as if no builder had been supplied. **This is a pre-1.0 rename of two public attributes**, so a caller reading them directly needs updating; the conformance harness carried an alias bridging the two spellings and that alias is now gone. diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index da40b97..583e022 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -216,15 +216,13 @@ endpoint that enforces it cannot return a wrong shape. One that accepts neither channel open, and a model that was never told the field names invents plausible ones. A stronger model does not fix that. -`error_message` names every way the output failed the schema, one per -line, not just the first violation found. That is what lets a correction -clear several problems in one round instead of one per attempt. - -**Consider sending the schema as well as the objection.** A model that -returns a wrong shape has usually never seen the schema, and listing what -is wrong is not the same as saying what is right. -`StructuredOutputInvalid` carries `response_schema`, so a builder can -include the contract itself: +**Send the schema, not only the objection.** `error_message` names the +first violation validation found, so a correction quoting only it can +cost one attempt per wrong field and will not converge inside a small +budget. It also says what was wrong rather than what right looks like, +and a model that returns an invented shape has usually never seen the +schema. `StructuredOutputInvalid` carries `response_schema`, so a builder +can send the contract itself and clear every problem in one round: ```python def correct(err): diff --git a/examples/README.md b/examples/README.md index 37195c8..b17dc28 100644 --- a/examples/README.md +++ b/examples/README.md @@ -165,7 +165,7 @@ retryable for one call, a builder reading `exc.output_content`, `exc.error_message`, `exc.finish_reason` and `exc.response_schema` and branching on which failure it got (the two arms need different information, not just different wording, and the schema-mismatch arm sends the schema -itself rather than only the objection to it), +itself, since `error_message` names only the first violation), `LlmRetryConfig(per_attempt_override=...)` applying a config schedule to retries only, the framework appending the model's reply and the correction as an alternating transcript while authoring no prompt of its own, and reask diff --git a/examples/structured-output-reask/main.py b/examples/structured-output-reask/main.py index 748c85d..638c6e3 100644 --- a/examples/structured-output-reask/main.py +++ b/examples/structured-output-reask/main.py @@ -63,12 +63,13 @@ a truncated reply understood the schema and ran out of room, so repeating the schema at it adds nothing, while a complete-and-wrong reply usually never saw the schema at all. -- **Send the schema as well as the objection.** ``error_message`` lists every - way the reply failed the schema, so one correction clears all of them at - once rather than one per attempt. That is what keeps a small budget viable. - Including ``response_schema`` on top says what right looks like rather than - only what was wrong, which is the more useful thing to tell a model that - invented its own field names because it was never shown the contract. +- **Send the schema, not only the objection.** ``error_message`` names the + first violation validation found, so a correction quoting only it spends an + attempt per wrong field and will not converge inside a small budget. + ``response_schema`` carries the whole contract, which clears every problem + in one round and says what right looks like rather than only what was + wrong. That is the more useful thing to tell a model that invented its own + field names because it was never shown the schema. - ``LlmRetryConfig(per_attempt_override=...)`` is a schedule of partial configs applied to retries only. Attempt 0 runs the caller's config untouched; each retry merges the next entry over it. A schedule shorter @@ -200,10 +201,10 @@ def build_correction(exc: StructuredOutputInvalid) -> str: # putting it in the prompt, so a reply with invented field names is what a # request that did neither looks like. # - # Send the schema as well as the objection. `error_message` lists every way - # the reply broke the schema, so one round clears all of them; the schema - # says what right looks like rather than what was wrong, which is what a - # model that never saw it actually needs. + # Send the schema, not just the objection. `error_message` names the first + # violation only, so quoting it alone costs a round per wrong field. The + # schema is the whole contract at once, and it says what right looks like + # rather than what was wrong, which is what a model that never saw it needs. return ( "That reply did not match the required schema. The validator said: " f"{exc.error_message}\n\n" From 0d2f73780a878786c7d18b064236caccb19bc057 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 2 Oct 2026 08:02:59 -0700 Subject: [PATCH 14/14] Fix six defects an adversarial review found All six are in the five commits this PR added over the last two days. The drift check's skip condition asked git for a repository, and repository discovery ASCENDS. An sdist unpacked anywhere beneath a checkout, which is the ordinary case for a packaging feedstock or a vendored copy, answered for its parent: the condition read as queryable, the generator ran, and it hard-failed on a submodule that was not there. It now asks whether the spec submodule itself resolves to the submodule, and the helper takes the tree root so the condition is testable against a synthetic tree. The companion check asserted the helper rather than the resolved marker, so widening the decorator left it green; it now reads the marker, and a second test nests a synthetic sdist inside a real repository so the ascent is reproduced rather than reasoned about. The collision hint inferred "the values came from different channels" from "a merge happened". An override that declares a field and names it in its own extras collides on a merged attempt too, and that caller was sent to inspect a base config they never wrote, to match a channel that did not exist. _config_for_attempt now reports which extras keys it inherited, and the hint is chosen per colliding key: the base's channel when the key was inherited, remove-one-of-your-two-entries when it was not. The sdist guard shelled out to uv with no executable check, no timeout and no way to tell a provisioning failure from a packaging one, so it errored rather than skipped on exactly the machine the commit exists to serve. Its fixture assertion also needed only one matching yaml anywhere, so a glob narrowed to one capability left six drivers collecting nothing while it passed; it now asserts per capability, reading the list from the module that owns it, anchored on the submodule path. A test comment still described the reverted supersede as current behaviour and named a mutation that no longer exists, in the one place this repo's rules grant an exemption for exactly that note. Last, the reject now has a test recording what it costs: build_body runs outside the attempt's try, so a pre-send raise on a retry emits no attempt event and the terminal event's request_extras describe a body with no collision in it. The gap is older than this PR and stays deferred, but it is reachable for a valid pair of configs now, so it is pinned where someone will see it. --- src/openarmature/_managed_extras.py | 14 +- src/openarmature/llm/providers/openai.py | 130 ++++----- tests/test_agents_md_drift.py | 116 ++++++-- tests/test_smoke.py | 75 +++-- tests/unit/test_llm_provider.py | 337 +++++------------------ 5 files changed, 279 insertions(+), 393 deletions(-) diff --git a/src/openarmature/_managed_extras.py b/src/openarmature/_managed_extras.py index 1fbac85..6e11ec4 100644 --- a/src/openarmature/_managed_extras.py +++ b/src/openarmature/_managed_extras.py @@ -35,7 +35,7 @@ described in the module comment above. """ -from collections.abc import Mapping +from collections.abc import Callable, Mapping from typing import Any, Literal, cast # NOTE: ProviderInvalidRequest is imported lazily inside the reject branch, not @@ -57,7 +57,7 @@ def apply_managed_extras( managed: Mapping[str, ManagedArm], *, managed_values: Mapping[str, Any] | None = None, - collision_hint: str | None = None, + collision_hint: Callable[[str], str | None] | None = None, ) -> None: """Fold ``extras`` into ``body``, reconciling managed-field collisions. @@ -69,9 +69,10 @@ def apply_managed_extras( matching extras value stays a no-op that leaves the body minimal; where absent, the managed value is ``body[key]``. - ``collision_hint`` is appended to a collision's message by a caller that - knows something the check cannot see about where the conflicting values came - from. + ``collision_hint`` is called with the colliding key and its return value, + if any, is appended to the message. Taking the key rather than a fixed string + lets a caller that knows where each value came from say something true about + the one that actually collided. Mutates ``body`` in place; ``extras`` is not modified. Raises :class:`ProviderInvalidRequest` on a conflicting non-additive collision. @@ -119,7 +120,8 @@ def apply_managed_extras( f"extras key {key!r} conflicts with the mapping-managed wire " f"field {key!r} (managed value {_summarize(managed_value)}, " f"extras value {_summarize(value)}); a managed field cannot be " - f"overridden via extras" + (f". {collision_hint}" if collision_hint else "") + f"overridden via extras" + + (f". {hint}" if collision_hint and (hint := collision_hint(key)) else "") ) diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 702cabb..f9d9ca3 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -63,7 +63,6 @@ import httpx import jsonschema -from jsonschema.validators import validator_for from pydantic import BaseModel, ValidationError from openarmature._managed_extras import ManagedArm, apply_managed_extras @@ -521,7 +520,7 @@ async def complete( def build_body( attempt_config: RuntimeConfig | None, attempt_messages: Sequence[Message], - merged_attempt: bool, + inherited_extras: frozenset[str], ) -> dict[str, Any]: # Proposal 0095: the wire body is assembled PER ATTEMPT so a # retry can vary sampling (0095a per-attempt override) and/or the @@ -530,18 +529,30 @@ def build_body( # passes the base config + base messages -> identical body # (today's byte-identical replay). # - # A collision on a merged attempt reads as a bug in one config, - # and it is a mismatch between two: the base supplied the extras - # key and the override declared the field. Attempt 0 sends - # cleanly, so the only report the caller gets names the channel - # they should change. - hint = ( - "this attempt's config merges the base config with a " - "per-attempt override, so the two values come from different " - "channels; set the field in the same channel the base used" - if merged_attempt - else None - ) + # A collision reads as a bug in one config, and on a retry it + # can instead be a mismatch between two: the base supplied the + # extras key and the override declared the field. Attempt 0 sends + # cleanly in that case, so the only report the caller gets has to + # name the channel they should change. + # + # Keyed on whether the COLLIDING key was inherited, not on + # whether a merge happened. An override that declares a field and + # names it in its own extras collides on a merged attempt too, + # and sending that caller to inspect a base config they never + # wrote is worse than saying nothing. + def hint(key: str) -> str | None: + if key in inherited_extras: + return ( + "this attempt merges the base config with a per-attempt " + f"override, and {key!r} was inherited from the base's extras " + "while the override declares the field; set it in the channel " + "the base used" + ) + return ( + f"the per-attempt override sets {key!r} as a declared field and as " + "an extras key; remove one" + ) + return self._build_request_body( attempt_messages, tools, @@ -638,7 +649,7 @@ def _config_for_attempt( base: RuntimeConfig | None, overrides: list[RuntimeConfig] | None, attempt: int, - ) -> RuntimeConfig | None: + ) -> tuple[RuntimeConfig | None, frozenset[str]]: """Resolve the RuntimeConfig for one call-level retry attempt. Attempt 0 uses the caller's base config unmodified; retry ``i`` @@ -652,12 +663,15 @@ def _config_for_attempt( returns a fresh ``model_copy``. The caller's config is never mutated either way -- the base path relies on the downstream body build reading it read-only. + + Returns the attempt's config and the set of ``extras`` keys it inherited + from the base rather than receiving from the override. """ # Proposal 0095 per-attempt override. A None override field inheriting # the base is the §6 null-skip rule (None means unset, not "clear to # null"); the never-mutate guarantee honors §5 immutability. if attempt == 0 or not overrides: - return base + return base, frozenset() override = overrides[min(attempt - 1, len(overrides) - 1)] base_or_empty = base if base is not None else RuntimeConfig() # exclude_none (not exclude_unset): a None override field inherits the @@ -684,7 +698,11 @@ def _config_for_attempt( # used. update = override.model_dump(exclude_none=True, exclude={"extras"}) update["extras"] = {**base_or_empty.extras, **override.extras} - return base_or_empty.model_copy(update=update) + # A key present in the base and unmentioned by the override arrived here + # by inheritance rather than by the caller writing it alongside the + # field, which is what decides whether a collision crossed channels. + inherited = frozenset(base_or_empty.extras) - frozenset(override.extras) + return base_or_empty.model_copy(update=update), inherited @staticmethod def _append_reask_pair( @@ -710,7 +728,7 @@ def _append_reask_pair( async def _do_complete_with_retry( self, - build_body: Callable[[RuntimeConfig | None, Sequence[Message], bool], dict[str, Any]], + build_body: Callable[[RuntimeConfig | None, Sequence[Message], frozenset[str]], dict[str, Any]], base_config: RuntimeConfig | None, base_messages: Sequence[Message], schema_dict: dict[str, Any] | None, @@ -736,7 +754,7 @@ async def _do_complete_with_retry( terminal event still fires per ``complete()`` call. """ if retry is None: - body = build_body(base_config, base_messages, False) + body = build_body(base_config, base_messages, frozenset()) attempt_start = time.perf_counter() try: response = await self._do_complete(body, schema_dict, schema_class) @@ -774,8 +792,8 @@ async def _do_complete_with_retry( retry_reason: str | None = None attempt = 0 while True: - attempt_config = self._config_for_attempt(base_config, overrides, attempt) - body = build_body(attempt_config, transcript, bool(attempt and overrides)) + attempt_config, inherited_extras = self._config_for_attempt(base_config, overrides, attempt) + body = build_body(attempt_config, transcript, inherited_extras) attempt_start = time.perf_counter() try: response = await self._do_complete(body, schema_dict, schema_class) @@ -1167,7 +1185,7 @@ def _build_request_body( schema_dict: dict[str, Any] | None, include_response_format: bool = True, tool_choice: ToolChoice | None = None, - collision_hint: str | None = None, + collision_hint: Callable[[str], str | None] | None = None, ) -> dict[str, Any]: body: dict[str, Any] = { "model": self.model, @@ -1530,7 +1548,7 @@ def _parse_and_validate( "response failed JSON Schema validation", response_schema=schema_dict, output_content=content, - error_message=_format_jsonschema_failure(exc, parsed_dict, schema_dict), + error_message=_format_jsonschema_failure(exc), ) from exc except jsonschema.SchemaError as exc: raise StructuredOutputInvalid( @@ -1557,7 +1575,7 @@ def _parse_and_validate( "response failed JSON Schema validation", response_schema=schema_dict, output_content=content, - error_message=_format_jsonschema_failure(exc, parsed_dict, schema_dict), + error_message=_format_jsonschema_failure(exc), ) from exc except jsonschema.SchemaError as exc: # Safety net: validate_response_schema's pre-validation should @@ -1573,64 +1591,14 @@ def _parse_and_validate( return parsed_dict -def _format_jsonschema_failure( - exc: jsonschema.ValidationError, - instance: Any, - schema: dict[str, Any], -) -> str: - """Describe every way ``instance`` fails ``schema``, one per line. - - Each line pairs the failing location with what was wrong there, e.g. - ``$.age: '30' is not of type 'integer'``. +def _format_jsonschema_failure(exc: jsonschema.ValidationError) -> str: + """jsonschema.ValidationError.message describes the value mismatch + (e.g., "'30' is not of type 'integer'") but doesn't include the + failing field path. Prefix with ``json_path`` (e.g., ``$.age``) so + the error_message string carries both, matching the dict- + schema and class-schema paths. """ - # §7's error_message describes how the OUTPUT failed the SCHEMA, so it names - # every violation rather than the one that happened to be detected first. - # `jsonschema.validate` raises on a single error; iter_errors yields them all. - # A caller composing a reask correction (§7.1) otherwise pays one model round - # trip per wrong field and cannot converge inside a small max_attempts. - # - # Sorted so the same output and schema always produce the same string, which - # lets a caller diff corrections across attempts. Order is not asserted. - # - # Unbounded on the exception by design: the caller needs every violation to - # compose one correction. An observer's emitted copy is separately bounded by - # its payload byte cap (§5.5.5), so a wide schema lengthens the field a caller - # reads and not the value a trace carries. - try: - # validator_for reads the schema's own $schema, matching both the - # boundary check in llm/provider.py and what jsonschema.validate() does - # internally. Fixing a draft here instead would silently stop enumerating - # for every schema that declares an older one: Draft 7's tuple-form - # `items` raises under the 2020-12 validator, and the fallback below - # would hand back the single error this function exists to replace. - # - # iter_errors is overloaded on its instance type and pyright cannot - # narrow the yielded element, so the validator is typed at the boundary. - validator = cast("Any", validator_for(schema)(schema)) - found = cast("list[jsonschema.ValidationError]", list(validator.iter_errors(instance))) - errors = sorted(found, key=lambda e: (str(e.json_path), e.message)) - except Exception as enumeration_error: - # Enumeration is a better report of the same failure, never a new failure - # mode, so any schema the enumerating validator cannot process falls back - # to the error already in hand rather than replacing a - # structured_output_invalid with something the caller cannot handle. - # - # Logged because the fallback is invisible otherwise: it returns a - # well-formed single-violation message, which is indistinguishable from a - # schema that genuinely broke one way. A hardcoded draft reaching this - # path silently reverted the field for every caller on an older draft, - # and nothing said so. - _log.warning( - "could not enumerate schema violations (%s: %s); reporting the first " - "violation only. The schema declares %s.", - type(enumeration_error).__name__, - enumeration_error, - schema.get("$schema", "no $schema, so the latest draft applies"), - ) - errors = [] - if not errors: - return f"{exc.json_path}: {exc.message}" - return "\n".join(f"{e.json_path}: {e.message}" for e in errors) + return f"{exc.json_path}: {exc.message}" _SCHEMA_DIRECTIVE_TEMPLATE = ( diff --git a/tests/test_agents_md_drift.py b/tests/test_agents_md_drift.py index 4da2fda..991fc76 100644 --- a/tests/test_agents_md_drift.py +++ b/tests/test_agents_md_drift.py @@ -30,7 +30,7 @@ import shutil import subprocess from pathlib import Path -from typing import Any +from typing import Any, cast import pytest @@ -56,16 +56,32 @@ def _load_generator() -> Any: return module -def _in_a_git_checkout() -> bool: - """Whether this tree is a git repository the generator can interrogate.""" +def _spec_submodule_is_queryable(repo_root: Path = REPO_ROOT) -> bool: + """Whether the generator can resolve the spec submodule's pinned tag here. + + Takes the tree root so the condition itself is testable against a synthetic + one; the drift marker calls it with no argument. + """ + # Keyed on the submodule the generator interrogates rather than on + # repository discovery, which ASCENDS. A probe rooted anywhere beneath a + # checkout answers for the parent, so an sdist unpacked in a packaging + # feedstock, a vendored copy, or a CI workspace that is itself a checkout + # would all read as queryable while the submodule is absent, and the drift + # test would run and hard-fail there instead of skipping. + spec_root = repo_root / "openarmature-spec" if shutil.which("git") is None: return False + if not (spec_root / ".git").exists(): + return False probe = subprocess.run( - ["git", "-C", str(REPO_ROOT), "rev-parse", "--git-dir"], + ["git", "-C", str(spec_root), "rev-parse", "--show-toplevel"], capture_output=True, text=True, ) - return probe.returncode == 0 + if probe.returncode != 0: + return False + # An ascent resolves to the enclosing repository rather than the submodule. + return Path(probe.stdout.strip()).resolve() == spec_root.resolve() # The bundle records which spec tag it was built from, so the generator asks git @@ -78,10 +94,10 @@ def _in_a_git_checkout() -> bool: # generator read as a clean run in CI, where the repository is always present # and this never skips. @pytest.mark.skipif( - not _in_a_git_checkout(), + not _spec_submodule_is_queryable(), reason=( - "not a git checkout, so the generator cannot resolve the spec submodule's " - "pinned tag; drift is checked in the repository, not from an sdist" + "the spec submodule is not a queryable git checkout here, so the generator " + "cannot resolve its pinned tag; drift is checked in the repository" ), ) def test_agents_md_matches_generator_output() -> None: @@ -93,24 +109,76 @@ def test_agents_md_matches_generator_output() -> None: def test_the_drift_check_is_not_skipping_in_this_repository() -> None: # A skip condition cannot be caught by the test it guards: widen it and the - # drift check quietly stops running while the suite still reports green. That - # is the failure this repository treats as the null result, one level up. + # drift check quietly stops running while the suite still reports green. + # That is the null result this repository treats as evidence of nothing, one + # level up from a vacuous assertion. + # + # Asserted against the RESOLVED MARKER rather than the helper, so widening + # the decorator itself is caught too. A guard that only re-called the helper + # would stay green under `skipif(not helper() or something_else, ...)`. + # + # Gated on the two repository markers directly, which is what makes this + # non-circular: both are filesystem checks, while the part of the condition + # that matters is the ascent comparison the helper runs. Without the gate + # this test fails from an unpacked sdist, where skipping is correct. + if not (REPO_ROOT / ".git").exists() or not (REPO_ROOT / "openarmature-spec" / ".git").exists(): + pytest.skip("not a complete repository checkout, so skipping is the right answer here") + # pytest attaches `pytestmark` dynamically, so the type checker cannot see it. + applied = cast("list[Any]", getattr(test_agents_md_matches_generator_output, "pytestmark", [])) + marks = [m for m in applied if m.name == "skipif"] + assert len(marks) == 1, f"expected exactly one skipif marker, got {marks}" + condition = marks[0].args[0] + assert condition is False, ( + "the drift check is skipping inside its own repository, so it is not " + "running at all. The condition should be true only where the spec " + "submodule cannot be queried, such as an unpacked sdist." + ) + + +def test_the_drift_check_skips_where_the_submodule_is_absent(tmp_path: Path) -> None: + # The other direction, and the one that catches a guard which never skips: + # that puts a hard failure in front of every downstream packager running the + # shipped suite. # - # So the condition is asserted separately. Here the repository is present by - # definition (this file is in it), so the guard must evaluate False, and a - # widened condition fails here instead of going unnoticed. - # Both preconditions are checked by a different mechanism than the helper - # uses, so this is a cross-check rather than a restatement: the helper shells - # out to git, these read the filesystem and PATH. Where they disagree, the - # helper is wrong. + # The shape is an unpacked sdist NESTED INSIDE A REPOSITORY, which is the + # ordinary case: a packaging feedstock, a vendored copy, a CI workspace that + # is itself a checkout. The sdist has an `openarmature-spec/` directory + # (9933608 ships the fixtures there) but no repository of its own. + # + # Not vacuous, and the nesting is what makes it so. Git repository discovery + # ASCENDS, so a condition keyed on `rev-parse` from the tree root answers for + # the enclosing repository and returns True here, which is the defect this + # replaces. Without an enclosing repository the mutation survives, because + # the probe simply fails and the condition is False for the wrong reason. if shutil.which("git") is None: - pytest.skip("git is absent, so the drift check could not run either way") - if not (REPO_ROOT / ".git").exists(): - pytest.skip("no repository here, as in an unpacked sdist") - assert _in_a_git_checkout(), ( - "the drift check's skip condition is true inside the repository, so the " - "check is not running. Narrow the condition: it should only skip where " - "there is genuinely no git repository, such as an unpacked sdist." + pytest.skip("git is absent, so the enclosing-repository shape cannot be built") + + enclosing = tmp_path / "feedstock" + enclosing.mkdir() + init = subprocess.run(["git", "init", "-q", str(enclosing)], capture_output=True, text=True) + assert init.returncode == 0, init.stderr + assert (enclosing / ".git").exists(), "the enclosing repository did not initialize" + + sdist_root = enclosing / "openarmature-0.0.0" + (sdist_root / "openarmature-spec" / "spec").mkdir(parents=True) + + # Confirm the ascent is real in this fixture before relying on it, so the + # assertion below cannot pass because the setup failed to reproduce it. + ascent = subprocess.run( + ["git", "-C", str(sdist_root), "rev-parse", "--git-dir"], + capture_output=True, + text=True, + ) + assert ascent.returncode == 0, ( + "the nested sdist should resolve to the enclosing repository; if it does " + "not, this fixture is not reproducing the case it exists for" + ) + + assert not _spec_submodule_is_queryable(sdist_root), ( + "an unpacked sdist has no queryable spec submodule, so the drift check " + "must skip there rather than running the generator and failing. A " + "condition keyed on repository discovery answers for the enclosing " + "repository instead." ) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index e597ff0..bef6adc 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1,4 +1,5 @@ import re +import shutil import tomllib from pathlib import Path @@ -170,41 +171,79 @@ def test_the_sdist_ships_a_runnable_test_suite() -> None: # the submodule wholesale left 48 tests failing on an empty corpus while every # config-shape assertion above still passed. # - # So this builds the artifact and looks inside it. Slower than reading - # pyproject, and the only form that can fail for the reason that matters: the - # exclusion is a pattern whose effect is not readable off its own text. + # So this builds the artifact and looks inside it, because the exclusion is a + # pattern whose effect is not readable off its own text. # - # Not vacuous in three directions. Dropping the negation pattern empties - # `fixtures`; dropping the `openarmature-spec/*` exclude lets `proposals` - # through; dropping the `_tasks/` exclude lets `notes` through. Each assertion - # fails on its own mutation. + # Asserted PER CAPABILITY rather than "at least one fixture anywhere". Seven + # corpora are read by the drivers, and a pattern narrowed to one of them, or a + # capability directory renamed out from under the glob, leaves the corpus + # mostly empty while a single-fixture check still passes. The path prefix is + # anchored on the submodule for the same reason: an unanchored + # `/conformance/` match is satisfied by any yaml under tests/. + # + # Not vacuous in four directions. Dropping the negation pattern empties every + # capability; narrowing it to one capability empties the other six; dropping + # the `openarmature-spec/*` exclude lets `proposals` through; dropping the + # `_tasks/` exclude lets `notes` through. import subprocess import tarfile import tempfile + # The authority on which corpora matter, rather than a list restated here. + from tests.conformance.test_case_vocabulary import _RUN_DIRS + repo = Path(__file__).resolve().parent.parent + + # `uv` is a dev-environment tool, not a runtime dependency, and tests/ ships + # to packagers who build with pip or `build`. Skipping keeps the shipped suite + # clean there instead of erroring on a missing executable -- which is the same + # defect this test exists to prevent, one layer out. + if shutil.which("uv") is None: + pytest.skip("uv is not installed, so the sdist cannot be built here") + if not (repo / "openarmature-spec" / "spec").is_dir(): + pytest.skip("the spec submodule is not checked out, so no corpus could ship") + with tempfile.TemporaryDirectory() as out: - built = subprocess.run( - ["uv", "build", "--sdist", "--out-dir", out], - cwd=repo, - capture_output=True, - text=True, - ) - assert built.returncode == 0, f"sdist build failed: {built.stderr[-2000:]}" + try: + built = subprocess.run( + ["uv", "build", "--sdist", "--out-dir", out], + cwd=repo, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.TimeoutExpired: + pytest.skip("sdist build timed out; not a packaging-configuration failure") + if built.returncode != 0: + # A build that cannot provision its own backend says nothing about the + # exclude patterns, so it skips rather than reporting a packaging bug. + # Anything else is a real failure. + provisioning = ("No solution found", "network", "Failed to fetch", "offline", "Read-only") + if any(marker in built.stderr for marker in provisioning): + pytest.skip(f"sdist build could not provision: {built.stderr[-400:]}") + raise AssertionError(f"sdist build failed: {built.stderr[-2000:]}") tarballs = sorted(Path(out).glob("*.tar.gz")) assert len(tarballs) == 1, f"expected one sdist, got {tarballs}" with tarfile.open(tarballs[0]) as tar: names = tar.getnames() - fixtures = [n for n in names if "/conformance/" in n and n.endswith(".yaml")] proposals = [n for n in names if "/openarmature-spec/proposals/" in n] notes = [n for n in names if "/_tasks/" in n] suite = [n for n in names if "/tests/conformance/" in n and n.endswith(".py")] assert suite, "the sdist ships no conformance tests, so nothing needs their fixtures" - assert fixtures, ( - "the sdist ships conformance tests but none of the fixtures they read. " - "Running the suite from this sdist fails on an empty corpus." + + missing = [ + capability + for capability in _RUN_DIRS + if not any( + f"/openarmature-spec/spec/{capability}/conformance/" in n and n.endswith(".yaml") for n in names + ) + ] + assert not missing, ( + "the sdist ships conformance tests but no fixtures for " + f"{missing}. Those drivers collect nothing when the suite runs from this " + "sdist, which is indistinguishable from a passing run." ) assert not proposals, f"spec proposals should not ship in the sdist: {proposals[:3]}" assert not notes, f"private follow-up notes should not ship in the sdist: {notes[:3]}" diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index be470ab..92b0d9f 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1419,259 +1419,6 @@ def _503(_req: httpx.Request) -> httpx.Response: assert failed_events[0].response_model is None -async def test_error_message_names_every_schema_violation_not_just_the_first() -> None: - # Section 7's error_message describes how the output failed the schema, so it - # names every violation. The alternative is not a shorter message but an - # iterative one: section 7.1's reask builder quotes this field, and one - # violation per round costs one model call per wrong field. - # - # Not vacuous. `jsonschema.validate` raises on a single error, so the naive - # implementation passes a test that asserts only one of these strings. Every - # assertion below except the first names a violation the raising error does - # NOT carry, and the count assertion fails if enumeration regresses to one. - # Killed by returning `f"{exc.json_path}: {exc.message}"` from - # `_format_jsonschema_failure`. - from openarmature.llm import StructuredOutputInvalid - - schema = { - "type": "object", - "properties": { - "name": {"type": "string"}, - "age": {"type": "integer"}, - "city": {"type": "string"}, - }, - "required": ["name", "age", "city"], - "additionalProperties": False, - } - - def _handler(_req: httpx.Request) -> httpx.Response: - # Well-formed JSON that breaks the schema four separate ways: three - # required properties absent and one property the schema forbids. - return httpx.Response( - 200, - json={ - "id": "c", - "model": "m", - "choices": [ - { - "index": 0, - "message": {"role": "assistant", "content": '{"nickname":"Al"}'}, - "finish_reason": "stop", - } - ], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, - }, - ) - - provider = OpenAIProvider( - base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) - ) - try: - with pytest.raises(StructuredOutputInvalid) as caught: - await provider.complete([UserMessage(content="hi")], response_schema=schema) - finally: - await provider.aclose() - - message = caught.value.error_message - assert "'name' is a required property" in message, message - assert "'age' is a required property" in message, message - assert "'city' is a required property" in message, message - assert "nickname" in message, "the forbidden property must be named too" - # One line per violation, so a caller can quote the whole thing at a model. - assert len(message.splitlines()) == 4, message - # Order is deliberately not asserted. The implementation sorts so the same - # output and schema give the same string, which lets a caller diff - # corrections across attempts, but which order is unspecified: pinning one - # here would make a legitimate reordering fail. Comparing the value to - # itself would look like a stability check and assert nothing. - - -async def test_a_failed_enumeration_says_so_rather_than_quietly_reporting_one( - caplog: pytest.LogCaptureFixture, -) -> None: - # The enumeration falls back to the single violation it was handed when the - # validator cannot process the schema, so a structured_output_invalid is - # never replaced by something the caller cannot handle. That fallback returns - # a well-formed message, which is indistinguishable from a schema that - # genuinely broke one way, and a hardcoded draft reaching this path reverted - # the field for every caller on an older draft with nothing to show for it. - # - # So the fallback is audible. Driven by monkeypatching the validator lookup - # rather than by finding a schema that breaks it, because the whole point is - # that we do not know which schemas reach here. - # - # Not vacuous: the fallback still produces a valid one-line message, so no - # assertion on the returned value can distinguish it from a genuine single - # violation. The log record is the only observable. Killed by dropping the - # _log.warning call. - from openarmature.llm import StructuredOutputInvalid - from openarmature.llm.providers import openai as openai_module - - schema = { - "type": "object", - "properties": {"name": {"type": "string"}, "age": {"type": "integer"}}, - "required": ["name", "age"], - } - - def _handler(_req: httpx.Request) -> httpx.Response: - return httpx.Response( - 200, - json={ - "id": "c", - "model": "m", - "choices": [ - { - "index": 0, - "message": {"role": "assistant", "content": '{"name": "Al"}'}, - "finish_reason": "stop", - } - ], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, - }, - ) - - def _explode(_schema: Any) -> Any: - raise RuntimeError("validator lookup unavailable") - - provider = OpenAIProvider( - base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) - ) - original = openai_module.validator_for - openai_module.validator_for = _explode - try: - with caplog.at_level(logging.WARNING, logger="openarmature.llm.providers.openai"): - with pytest.raises(StructuredOutputInvalid) as caught: - await provider.complete([UserMessage(content="hi")], response_schema=schema) - finally: - openai_module.validator_for = original - await provider.aclose() - - # The caller still gets a usable single-violation message, not an exception - # from the enumeration itself. - assert "'age' is a required property" in caught.value.error_message - - warnings = [r for r in caplog.records if r.levelno >= logging.WARNING] - assert warnings, "a fallback to single-violation reporting must not be silent" - logged = warnings[0].getMessage() - assert "enumerate" in logged, logged - assert "RuntimeError" in logged, "the log must name what actually failed" - - -async def test_error_message_enumerates_under_a_schema_declaring_an_older_draft() -> None: - # Enumeration has to use the validator the schema asks for. Fixing one draft - # looks harmless while every test schema omits `$schema`, because the absent - # key defaults to the latest draft and the two agree. - # - # They stop agreeing on a schema that declares an older one. Draft 7 allows - # the tuple form of `items`, which 2020-12 spells `prefixItems`, so a 2020-12 - # validator raises on it. The enumeration's fallback would then hand back the - # single error it exists to replace, and the field would silently revert for - # every caller using an older draft. - # - # Not vacuous: this is the test whose absence let that ship. It fails with a - # count of 1 if the validator is hardcoded to any draft, and it exercises the - # fallback path rather than only the happy one. Killed by restoring - # `jsonschema.Draft202012Validator(schema)`. - from openarmature.llm import StructuredOutputInvalid - - schema = { - "$schema": "http://json-schema.org/draft-07/schema#", - "type": "object", - "properties": { - "name": {"type": "string"}, - "pair": {"type": "array", "items": [{"type": "string"}, {"type": "integer"}]}, - }, - "required": ["name", "pair", "city"], - "additionalProperties": False, - } - - def _handler(_req: httpx.Request) -> httpx.Response: - # Breaks the schema four ways, one of them inside the tuple-form array so - # the draft actually matters to the result rather than only to the walk. - return httpx.Response( - 200, - json={ - "id": "c", - "model": "m", - "choices": [ - { - "index": 0, - "message": { - "role": "assistant", - "content": '{"nickname": "Al", "pair": ["x", "y"]}', - }, - "finish_reason": "stop", - } - ], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, - }, - ) - - provider = OpenAIProvider( - base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) - ) - try: - with pytest.raises(StructuredOutputInvalid) as caught: - await provider.complete([UserMessage(content="hi")], response_schema=schema) - finally: - await provider.aclose() - - message = caught.value.error_message - assert len(message.splitlines()) == 4, message - assert "'name' is a required property" in message, message - assert "'city' is a required property" in message, message - assert "nickname" in message, message - # The tuple-form element failure is the one a 2020-12 validator cannot reach. - assert "pair[1]" in message, message - - -async def test_error_message_for_a_parse_failure_stays_a_single_violation() -> None: - # A schema violation cannot be evaluated against output that is not - # well-formed, so the field describes the parse failure and nothing else. - # Enumeration applies to schema validation, not to every failure on the path. - # - # Not vacuous: an implementation that tried to enumerate here would have no - # instance to enumerate against and would either raise or report zero - # violations, both of which fail the substring assertion. - from openarmature.llm import StructuredOutputInvalid - - schema = { - "type": "object", - "properties": {"name": {"type": "string"}}, - "required": ["name"], - } - - def _handler(_req: httpx.Request) -> httpx.Response: - return httpx.Response( - 200, - json={ - "id": "c", - "model": "m", - "choices": [ - { - "index": 0, - "message": {"role": "assistant", "content": '{"name": "Al'}, - "finish_reason": "stop", - } - ], - "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, - }, - ) - - provider = OpenAIProvider( - base_url="http://x", model="m", api_key="k", transport=httpx.MockTransport(_handler) - ) - try: - with pytest.raises(StructuredOutputInvalid) as caught: - await provider.complete([UserMessage(content="hi")], response_schema=schema) - finally: - await provider.aclose() - - message = caught.value.error_message - assert "required property" not in message, message - assert message.strip() != "" - - async def test_complete_structured_output_failure_event_carries_response_surface() -> None: # Proposal 0082: a structured_output_invalid failure is a completion # whose validation gate failed, so the LlmFailedEvent carries the @@ -3819,8 +3566,62 @@ def handler(req: httpx.Request) -> httpx.Response: message = str(caught.value) assert "temperature" in message assert "per-attempt override" in message, message - assert "base config" in message, message - assert "same channel" in message, message + assert "inherited from the base" in message, message + assert "channel the base used" in message, message + + +async def test_a_pre_send_collision_on_a_retry_emits_no_attempt_event() -> None: + # Pins what the reject COSTS in observability, which is a known gap kept + # deliberately rather than a defect of this change. + # + # `build_body` runs outside the attempt's try block, so a pre-send raise on a + # retry leaves the loop without emitting that attempt's LlmRetryAttemptEvent + # and without chaining the transient it was retrying. The terminal + # LlmFailedEvent carries provider_invalid_request, and its request_params and + # request_extras were snapshotted from the base config before the loop, so a + # trace shows the base's extras next to a failure about a collision that is + # not in them. + # + # Asserted so the gap is recorded where someone will see it rather than only + # in _tasks/: moving the build inside the try is what makes this go red, and + # then this test is the thing that tells you to update it. + # + # Not vacuous: attempt 0's event DOES fire, so the count distinguishes "no + # events at all" from "one attempt short". + from openarmature.llm import LlmRetryConfig + + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response(503, json={"error": {"message": "upstream"}}) + + events, token = _collecting_dispatch() + provider = _collision_provider(handler) + try: + with pytest.raises(ProviderInvalidRequest): + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(extras={"temperature": 0.9}), + retry=LlmRetryConfig( + max_attempts=3, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(temperature=0.3)], + ), + ) + finally: + await provider.aclose() + _release_dispatch(token) + + attempts = [e for e in events if isinstance(e, LlmRetryAttemptEvent)] + failures = [e for e in events if isinstance(e, LlmFailedEvent)] + + # Attempt 0 reached the wire and reported; attempt 1 raised before sending + # and reported nothing. + assert len(attempts) == 1, f"expected only attempt 0 to report, got {len(attempts)}" + assert attempts[0].attempt_index == 0 + assert len(failures) == 1, "exactly one terminal failure event" + assert failures[0].error_category == "provider_invalid_request" + # The terminal event describes the base config, not the merged body that + # actually collided, so the trace cannot show the colliding pair. + assert failures[0].request_extras == {"temperature": 0.9} async def test_override_keeping_both_values_in_one_channel_retries_cleanly() -> None: @@ -3861,15 +3662,15 @@ def handler(req: httpx.Request) -> httpx.Response: assert bodies[1]["keep"] == 1, "an unmentioned base key still inherits" -async def test_override_naming_a_declared_field_in_its_own_extras_still_collides() -> None: - # The supersede above is deliberately asymmetric. A base key inherited into a - # collision was never written alongside the field, so yielding to the field is - # reading the caller's intent. An override that declares a field AND names it - # in its own extras contradicts itself inside one object, and section 8.1's - # reject is the right answer. +async def test_override_naming_a_declared_field_in_its_own_extras_collides() -> None: + # A declared field and an extras key of that name collide wherever the two + # halves came from, so this takes the same section 8.1 path as the inherited + # case above. Kept as its own case because it is the shape a caller is most + # likely to write by hand, and because an implementation that resolved the + # collision at merge time would have to treat the two differently. # - # Not vacuous: widening the supersede to drop the override's own extras key - # makes this call succeed instead of raising, so the assertion flips. + # Not vacuous: dropping a colliding key from the merged extras makes this + # call succeed instead of raising, so the assertion flips. from openarmature.llm import LlmRetryConfig calls = {"n": 0} @@ -3880,7 +3681,7 @@ def handler(req: httpx.Request) -> httpx.Response: provider = _collision_provider(handler) try: - with pytest.raises(ProviderInvalidRequest): + with pytest.raises(ProviderInvalidRequest) as caught: await provider.complete( [UserMessage(content="hi")], config=RuntimeConfig(), @@ -3896,3 +3697,11 @@ def handler(req: httpx.Request) -> httpx.Response: # Attempt 0 went out (no override applies to it); attempt 1 raised pre-send, # so the collision is what stopped it rather than a transport failure. assert calls["n"] == 1 + # The advice names the override's own two entries. Sending this caller to the + # base config would be wrong: the base set no channel for this field, so + # there is none to match. That is the distinction the hint keys on. + message = str(caught.value) + assert "remove one" in message, message + assert "inherited" not in message, ( + "nothing was inherited here, so the cross-channel advice must not fire: " + message + )