From dcda69881801bda5314c496902e4d9c2808868b4 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 25 Sep 2026 18:02:08 -0700 Subject: [PATCH 1/2] Correct four release claims and fix two defects From spec's release review. None of the four claims was a regression; each was a statement about the release that was not true. The CHANGELOG was dated 2026-07-20, 67 days stale and unchanged across the commit titled "Bring the CHANGELOG current before tagging". It also said 0124 was `not-yet` twenty-one lines before saying it was `partial`. 0085 was `implemented` with half of it absent: nothing writes `enclosing_fan_out_lineage`, as compiled.py's own comment says, so section 10.11's no-mis-skip floor holds but section 10.11.1's exactly-once guarantee does not extend to nested fan-outs. A completed inner instance re-runs and its side effects execute twice. Fixture 076 passes against a record the test seeds by hand. Now `partial`, and the note states what is met rather than citing two sections of 0085 that do not exist. 0124's note named two unmet arms. It now names three, worst first: the resolver still keys on span openness, which section 5.5 forbids and which no fixture can reach because the adapter attaches instance or branch middleware where section 5.1 specifies node middleware; no subgraph span is synthesized on either observer; and the detached arms stay unmirrored. A fourth arm, a callable branch on Langfuse, is RETRACTED: section 8.4.8 makes that branch's single observation its dispatch observation, so there is no race to lose, and the parent is stable with and without a yield. It came from a review finding that was never reproduced before being written into a shipped note. Two defects, both public API. `StructuredOutputInvalid` carried `raw_content` and `failure_description` where llm-provider section 7 names `output_content` and `error_message`, which section 7.1 says the reask builder receives. A caller writing the documented builder hit AttributeError, the loop converted it to `raise exc from reask_error`, and the call failed on the first invalid output as if no builder had been supplied. Renamed; a pre-1.0 break for anyone reading the two attributes. The conformance harness alias bridging the spellings is removed as dead. A per-attempt retry override replaced the extras container where section 7.1 says undeclared extras merge by the same per-key rule. An override adjusting one vendor knob silently withdrew every other knob the caller had set for that attempt. The test asserting the wrong behaviour is inverted, and now asserts the inherited keys, which is the half that can fail. --- CHANGELOG.md | 8 ++- conformance.toml | 6 +- docs/concepts/llms.md | 12 ++-- src/openarmature/llm/errors.py | 16 ++--- src/openarmature/llm/providers/openai.py | 75 +++++++++++------------- src/openarmature/llm/retry.py | 4 +- tests/conformance/harness/wire.py | 31 +++------- tests/unit/test_llm_provider.py | 69 +++++++++++----------- tests/unit/test_observability_otel.py | 10 ++-- tests/unit/test_structured_output.py | 8 +-- 10 files changed, 110 insertions(+), 129 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e819b7d..5fb8449 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to `openarmature-python` are documented in this file. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The package follows [Semantic Versioning](https://semver.org/); pre-1.0 minor bumps may carry behavioral changes per [spec governance](https://github.com/LunarCommand/openarmature-spec/blob/main/GOVERNANCE.md). -## [0.17.0] - 2026-07-20 +## [0.17.0] - 2026-09-25 ### Added @@ -37,10 +37,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **The OpenAI embedding mapping decodes a base64 `data[].embedding`** (proposal 0106, retrieval-provider §8.3, spec v0.101.0). A caller may set `encoding_format: "base64"` through the extras bag to request the compact wire encoding, so the `/v1/embeddings` response returns each embedding as a base64 string rather than a JSON number array. The mapping now decodes `data[].embedding` by wire shape: a JSON number array is the float vector verbatim (unchanged); a base64 string is decoded as consecutive little-endian IEEE-754 float32 values (the same vector, more compact on the wire); any other shape (null, a number, a bool, an object, an empty value, or an array with a non-number) is a malformed response (`provider_invalid_response`). A base64 value that is not valid base64, or whose decoded byte length is not a positive multiple of 4, is likewise malformed and fails loud rather than returning a truncated or padded vector. The verbatim base64 string is preserved on `raw` (only `vectors` carries the decoded floats), and the decode composes with batch chunking. `encoding_format` stays an ordinary unmanaged extras key, since the consumer keys on the response shape rather than the request parameter, and the wire default remains `"float"`, so existing callers see no change. Previously a base64 request failed `provider_invalid_response` because the consumer could not read a base64 string as a vector, so this makes a previously-broken advertised knob work. This shipped ahead of the pin (spec v0.101.0 against a v0.88.0 pin) and is ratified at the v0.118.2 pin: the base64 round-trip and malformed-boundary fixtures 049 and 050 run. - **Token-budget failure-path parity on Langfuse, and unrecognized `token_budget` keys are tolerated** (proposal 0109, observability §8.4.3 + prompt-management §3 / §5, spec v0.104.0). Two additive edges from the 0083 token-budget work. (1) The Langfuse Generation now carries a flat `metadata.token_budget_exceeded` boolean whenever a budget is declared and at least one bound is evaluable: `true` if any evaluated bound was crossed, `false` if all held, absent when no bound is evaluable (a not-reported counter contributes nothing, per 0101). It is emitted regardless of the observation level, so on a `structured_output_invalid` failure that also exceeds budget the flag survives the `ERROR`-precedence rule (the `ERROR` level and category `statusMessage` still win as the primary signal), giving the Langfuse failure path parity with the OTel `openarmature.llm.token_budget.exceeded` span attribute. (2) The filesystem prompt backend now tolerates and filters an unrecognized key in a `token_budget` config: a stray or future key alongside a valid bound is ignored and the recognized bound applies, rather than rejecting the whole budget to fallback, converging with the Langfuse backend that already field-filters. A malformed value for a recognized bound still fails loud (fallback-eligible), which 0109 deliberately leaves to each backend's trust model. This shipped ahead of the pin (spec v0.104.0 against a v0.88.0 pin) and is ratified at the v0.118.2 pin, with the Langfuse WARNING-level rendering driven in the dedicated Langfuse harness. Fixture 037's unrecognized-key case is still to be wired. -- **Pinned spec advances v0.88.0 to v0.118.2** across the v0.17.0 cycle, in three steps. The first reaches v0.107.0 (proposals 0094-0113); the second reaches v0.112.0, absorbing the Langfuse client-ownership and provider-isolation arc (proposals 0114-0118), which was opened by a registered consumer's leak report and settled across five spec releases; the third reaches v0.118.2 (proposals 0119-0125). Per-proposal implementation status lives in the `conformance.toml` manifest: every proposal across the three bands is `implemented`, except 0098 / 0102 / 0107, which are `textual-only` and carry no implementable surface, and 0124 / 0125, which are `not-yet`. Spec v0.112.0 is flagged breaking for existing fixtures under the pre-1.0 allowance, because narrowing what a failed observation may carry under the default posture contradicted five shipped cases; those are reconciled at this pin. Observability fixtures 098 / 137 / 138 are un-deferred and run against the post-0118 shape, and 150 / 151 / 157 / 158 / 159 are wired. Fixture 160 is the only arrival at this pin that does not yet run; the `error_message` byte cap entry above says what it waits on. +- **Pinned spec advances v0.88.0 to v0.118.2** across the v0.17.0 cycle, in three steps. The first reaches v0.107.0 (proposals 0094-0113); the second reaches v0.112.0, absorbing the Langfuse client-ownership and provider-isolation arc (proposals 0114-0118), which was opened by a registered consumer's leak report and settled across five spec releases; the third reaches v0.118.2 (proposals 0119-0125). Per-proposal implementation status lives in the `conformance.toml` manifest: every proposal across the three bands is `implemented`, except 0098 / 0102 / 0107, which are `textual-only` and carry no implementable surface; 0124, which is `partial`; and 0125, which is `not-yet` because it is fixtures-only and rides with fixture 160. Spec v0.112.0 is flagged breaking for existing fixtures under the pre-1.0 allowance, because narrowing what a failed observation may carry under the default posture contradicted five shipped cases; those are reconciled at this pin. Observability fixtures 098 / 137 / 138 are un-deferred and run against the post-0118 shape, and 150 / 151 / 157 / 158 / 159 are wired. Fixture 160 is the only arrival at this pin that does not yet run; the `error_message` byte cap entry above says what it waits on. ### Fixed +- **`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. - **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. @@ -58,7 +60,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **A nested branch reusing an ancestor's name no longer collapses onto one dispatch span** (observability §10, pipeline-utilities §11.10). A per-branch dispatch span took its branch identity from the scalar `branch_name`, which is always the innermost branch on the event; resolving a dispatch span at an outer namespace has to take the identity from that depth in `branch_name_chain` instead. Where a nested parallel-branches node reused an ancestor's branch name the two keys collapsed into one, so an inner event opened a dispatch span for the *outer* branch of that name, and where that outer branch was gated off by `when` it acquired a span despite never being dispatched, which §11.10 forbids outright. A plain nested graph reaches this: no middleware, no provider call, no failure isolation. Two further defects in the same function are fixed with it: at an empty prefix the enclosing-branch slice dropped the last chain entry instead of yielding none, and the identity read selected on truthiness, so an empty-string branch name in the chain fell back to the scalar and resolved a different branch. `branch_dispatch_key` and its fan-out sibling `dispatch_key` were duplicated verbatim across the two observers, and the same defect had been fixed twice by hand three changes running; both now live in `observability/lineage.py`, so one mutation fails tests on both sides, and a guard fails if either observer reintroduces a local copy. -- **An orphan provider observation resolves its parent structurally on Langfuse too, and the conformance barrier that proves it** (proposal 0124, conformance-adapter §5.1 + observability §5.5, spec v0.118.0). Completes the structural resolution PR #277 began. That change fixed the OTel observer and left the Langfuse one resolving an orphan against whatever observations happened to exist when the event drained, which is the schedule-dependence §5.5 forbids: a call issued from fan-out instance middleware before its inner node ran parented under the **outer** instance instead of the inner one. The Langfuse observer now synthesizes the fan-out instance and per-branch dispatch observations the calling lineage sits inside, mirroring the OTel side, and backfills the subgraph identity from the first node event so that field cannot depend on which event arrived first either. **Two arms stay schedule-dependent and the manifest records 0124 as `partial` for them:** a call inside a DETACHED wrapper, on both observers, because the detached openers assume a trace the synthesis path has not created; and a CALLABLE parallel branch on Langfuse, which is stored as a leaf where OTel renders it as the dispatch span. Rendering it as the dispatch was attempted here and reverted: it loses the §8.4.2 error mapping, so a branch that raised becomes indistinguishable from one that succeeded, and it does not remove the ordering dependence anyway. The shared `LineageEvent` protocol moves into `observability/lineage.py` beside the dispatch-key builders, which live there because the same defect had to be fixed twice by hand in three consecutive changes. **The half that makes it checkable:** `await_event_delivery` (§5.1) is now honored at all five directive blocks across fixtures 133, 134, 152 and 153. It is a barrier over delivery of the call's own provider event, not the scheduler yield 0124 originally prescribed, which §5.1 calls insufficient because a strictly serial delivery queue hands one turn to whichever event is next rather than to this one. A case whose barrier cannot be provided fails rather than running without it. Adopting it also required the hand-built 133 and 134 drivers to honor their fixtures' declared `phase: pre`, which they ignored in favour of a hardcoded `post` where the node body has already synthesized the dispatch span and no ordering is left to pin. The combination is what earns the entry: with the barrier at `phase: pre`, disabling either observer's synthesis fails the fixtures, and with the previous yield in place the same defect passes. The failure-isolation marker resolves through the same synthesizing path as the five provider handlers, so the two observers agree on its parent again. The barrier checks that it bound to the live invocation before trusting the drain summary, since `drain_events_for` returns a clean summary for an unmatched id that is indistinguishable from a satisfied barrier, and records its outcome for assertion after the run rather than raising inside middleware, which a fan-out declared `error_policy: collect` absorbs into a dropped error record. A dispatch observation synthesized from a provider event is back-dated to the same start as the observation it parents, so `LangfuseClient.span()` gains the optional `start_time` its `generation()` counterpart already had; without it the child's whole interval preceded its parent's start. `stored_lineage` joins `LineageEvent` and `fan_out_identity_key` in `observability/lineage.py`, and the guard against either observer reintroducing a local copy now covers all four rather than the two it was written for. +- **An orphan provider observation resolves its parent structurally on Langfuse too, and the conformance barrier that proves it** (proposal 0124, conformance-adapter §5.1 + observability §5.5, spec v0.118.0). Completes the structural resolution PR #277 began. That change fixed the OTel observer and left the Langfuse one resolving an orphan against whatever observations happened to exist when the event drained, which is the schedule-dependence §5.5 forbids: a call issued from fan-out instance middleware before its inner node ran parented under the **outer** instance instead of the inner one. The Langfuse observer now synthesizes the fan-out instance and per-branch dispatch observations the calling lineage sits inside, mirroring the OTel side, and backfills the subgraph identity from the first node event so that field cannot depend on which event arrived first either. **Three arms stay schedule-dependent and the manifest records 0124 as `partial` for them.** The resolver still returns the calling node's span when that span happens to be open, which §5.5 forbids outright, and no fixture can reach that check because the conformance adapter attaches instance or branch middleware where §5.1 specifies node middleware. No subgraph span is synthesized on either observer, so an orphan inside a plain subgraph parents under the invocation span rather than the subgraph span. And the detached arms are unmirrored on both observers, so a call inside a detached wrapper resolves by drain order and lands in a different trace. The shared `LineageEvent` protocol moves into `observability/lineage.py` beside the dispatch-key builders, which live there because the same defect had to be fixed twice by hand in three consecutive changes. **The half that makes it checkable:** `await_event_delivery` (§5.1) is now honored at all five directive blocks across fixtures 133, 134, 152 and 153. It is a barrier over delivery of the call's own provider event, not the scheduler yield 0124 originally prescribed, which §5.1 calls insufficient because a strictly serial delivery queue hands one turn to whichever event is next rather than to this one. A case whose barrier cannot be provided fails rather than running without it. Adopting it also required the hand-built 133 and 134 drivers to honor their fixtures' declared `phase: pre`, which they ignored in favour of a hardcoded `post` where the node body has already synthesized the dispatch span and no ordering is left to pin. The combination is what earns the entry: with the barrier at `phase: pre`, disabling either observer's synthesis fails the fixtures, and with the previous yield in place the same defect passes. The failure-isolation marker resolves through the same synthesizing path as the five provider handlers, so the two observers agree on its parent again. The barrier checks that it bound to the live invocation before trusting the drain summary, since `drain_events_for` returns a clean summary for an unmatched id that is indistinguishable from a satisfied barrier, and records its outcome for assertion after the run rather than raising inside middleware, which a fan-out declared `error_policy: collect` absorbs into a dropped error record. A dispatch observation synthesized from a provider event is back-dated to the same start as the observation it parents, so `LangfuseClient.span()` gains the optional `start_time` its `generation()` counterpart already had; without it the child's whole interval preceded its parent's start. `stored_lineage` joins `LineageEvent` and `fan_out_identity_key` in `observability/lineage.py`, and the guard against either observer reintroducing a local copy now covers all four rather than the two it was written for. ## [0.16.0] — 2026-07-18 diff --git a/conformance.toml b/conformance.toml index 89ed9ad..060dc53 100644 --- a/conformance.toml +++ b/conformance.toml @@ -901,9 +901,9 @@ note = "Nested-fan-out span lineage. graph-engine §6: fan_out_index_chain / bra # Spec v0.80.0 (proposal 0085). Nested-fan-out checkpoint resume # lineage (pipeline-utilities §10.11 enclosing_fan_out_lineage). [proposals."0085"] -status = "implemented" +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). KNOWN follow-up (out of scope per §66 / §76): 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 + §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." # Spec v0.79.0 (proposal 0086). Service-wide default cache_ttl_seconds # on PromptManager (prompt-management §6). Implemented since 0.17.0; @@ -1200,7 +1200,7 @@ note = "conformance-adapter §5.4 *Subgraph declaration placement*: a declaratio [proposals."0124"] status = "partial" since = "0.17.0" -note = "An orphan provider span's parent is resolved structurally: the enclosing wrapper is determined by the call's position in the graph rather than by which spans an observer has materialized, and \u00a76's synthesis trigger moves from the first inner started event to the first event that needs the span. Both observers do this; PR #277 shipped the OTel half, and the Langfuse half lands with this adoption (it had no call-site synthesis at all). partial because two arms of the structural-parent guarantee are unmet, both deliberately. A wrapper-issued call inside a DETACHED wrapper still resolves by drain order on both observers: the detached openers assume a trace already exists, which holds from the node path and not from the synthesis path, so those arms are unmirrored in both synthesizers (the open half of issue #279). And a callable parallel branch on Langfuse still resolves an orphan's parent by event order, because it is stored as a leaf where OTel renders it as the dispatch span; rendering it as the dispatch was attempted and reverted, since it loses the section 8.4.2 error mapping and does not remove the ordering dependence. Tracked, and it needs a spec answer first. The failure-isolation marker resolves through the same synthesizing path as the five provider handlers rather than a bare enclosing-wrapper walk. The await_event_delivery directive is honored at all five blocks across fixtures 133 / 134 / 152 / 153, as a barrier over delivery of this call's provider event rather than the scheduler yield PR #277 implemented, which \u00a75.1 calls insufficient. The barrier verifies it bound to the live invocation before trusting the drain summary, because drain_events_for returns a clean summary for an unmatched invocation_id, and records its outcome for assertion after the run rather than raising inside middleware a collect-policy fan-out would absorb. Adopting it also required the hand-built 133 / 134 drivers to honor the fixtures' declared phase: pre, which they ignored in favor of a hardcoded post, where the node body has already synthesized the dispatch span and no ordering is left for the barrier to pin." +note = "An orphan provider span's parent is resolved structurally: the enclosing wrapper is determined by the call's position in the graph rather than by which spans an observer has materialized, and \u00a76's synthesis trigger moves from the first inner started event to the first event that needs the span. Both observers do this; PR #277 shipped the OTel half, and the Langfuse half lands with this adoption (it had no call-site synthesis at all). partial for three unmet arms of the structural-parent guarantee, listed worst first. (1) THE RESOLVER STILL KEYS ON SPAN OPENNESS: it returns the calling node's span when that span happens to be open, which section 5.5's MUST NOT forbids, and because the OTel observer publishes node spans synchronously a wrapper-issued call whose provider event drains later becomes the node's child. The parent is a function of scheduling, reachable from ordinary node middleware. No fixture can catch it: the adapter attaches instance or branch middleware where section 5.1 says node middleware, so no case reaches the openness check. (2) NO SUBGRAPH SPAN IS SYNTHESIZED, on both observers, so an orphan inside a plain subgraph parents under the invocation span on OTel and at the Trace root on Langfuse; section 5.5 names the subgraph span alongside the other two. (3) THE DETACHED ARMS are unmirrored on both observers, so a call inside a detached wrapper resolves by drain order and differs in trace id (the open half of issue #279). A previously-listed fourth arm, a callable parallel branch on Langfuse, IS RETRACTED: section 8.4.8 makes the callable branch's single observation its per-branch dispatch observation, so there is no leaf-versus-dispatch race to lose, and the parent is stable with and without a yield, reproduced both ways. The residual callable-branch gap is narrower and separate: that observation omits parallel_branches_parent_node_name, which section 8.4.2 requires. The failure-isolation marker resolves through the same synthesizing path as the five provider handlers rather than a bare enclosing-wrapper walk. The await_event_delivery directive is honored at all five blocks across fixtures 133 / 134 / 152 / 153, as a barrier over delivery of this call's provider event rather than the scheduler yield PR #277 implemented, which \u00a75.1 calls insufficient. The barrier verifies it bound to the live invocation before trusting the drain summary, because drain_events_for returns a clean summary for an unmatched invocation_id, and records its outcome for assertion after the run rather than raising inside middleware a collect-policy fan-out would absorb. Adopting it also required the hand-built 133 / 134 drivers to honor the fixtures' declared phase: pre, which they ignored in favor of a hardcoded post, where the node body has already synthesized the dispatch span and no ordering is left for the barrier to pin." [proposals."0125"] status = "not-yet" diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index 933ddf4..eeeac6c 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -166,13 +166,13 @@ response = await provider.complete( response_schema=schema, retry=LlmRetryConfig( max_attempts=3, - reask=lambda err: f"That output was invalid: {err.failure_description}. Return corrected JSON.", + reask=lambda err: f"That output was invalid: {err.error_message}. Return corrected JSON.", ), ) ``` The builder receives the raised `StructuredOutputInvalid` (its -`raw_content` is the model's invalid output, `failure_description` the +`output_content` is the model's invalid output, `error_message` the reason) and returns the correction text. On each invalid attempt the loop appends the model's raw output as an `assistant` message and your correction as a `user` message to a working transcript that accumulates @@ -701,8 +701,8 @@ can handle them. `StructuredOutputInvalid` is the new one and worth a note. It fires when a model returns content that fails to parse as JSON, or parses but fails to validate against the supplied schema. The exception -carries the requested `response_schema`, the `raw_content` the model -produced, and a `failure_description`. It is non-transient by default +carries the requested `response_schema`, the `output_content` the model +produced, and a `error_message`. It is non-transient by default because a model that emits non-conforming output on a given prompt usually emits the same non-conforming output on retry. Useful retry strategies for this case involve changing the prompt or doubling @@ -722,8 +722,8 @@ async def classify_with_diagnostics(state): log.warning( "schema-validation failure on classify", extra={ - "raw_content": exc.raw_content, - "failure": exc.failure_description, + "output_content": exc.output_content, + "failure": exc.error_message, }, ) raise diff --git a/src/openarmature/llm/errors.py b/src/openarmature/llm/errors.py index 95de980..2692c14 100644 --- a/src/openarmature/llm/errors.py +++ b/src/openarmature/llm/errors.py @@ -199,8 +199,8 @@ class StructuredOutputInvalid(LlmProviderError): Attributes: response_schema: The JSON Schema requested. - raw_content: The raw response content the model produced. - failure_description: A description of the parse or validation + output_content: The raw response content the model produced. + error_message: A description of the parse or validation failure. finish_reason: The normalized finish reason of the response that failed validation (``"length"`` signals a truncation, the key @@ -219,8 +219,8 @@ class StructuredOutputInvalid(LlmProviderError): category = STRUCTURED_OUTPUT_INVALID response_schema: dict[str, Any] - raw_content: str - failure_description: str + output_content: str + error_message: str finish_reason: str | None usage: Usage | None response_id: str | None @@ -230,8 +230,8 @@ def __init__( self, *args: Any, response_schema: dict[str, Any], - raw_content: str, - failure_description: str, + output_content: str, + error_message: str, finish_reason: str | None = None, usage: Usage | None = None, response_id: str | None = None, @@ -239,8 +239,8 @@ def __init__( ) -> None: super().__init__(*args) self.response_schema = response_schema - self.raw_content = raw_content - self.failure_description = failure_description + self.output_content = output_content + self.error_message = error_message self.finish_reason = finish_reason self.usage = usage self.response_id = response_id diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 10f9fae..19ff09f 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -651,27 +651,20 @@ def _config_for_attempt( # leaving it in the dump would clear the caller's vendor knobs on any # override that never mentioned them. # - # An EMPTY override container therefore means "unspecified" and inherits, - # matching what None means for the declared fields. A NON-EMPTY one - # replaces, matching the same rule those fields follow. Clearing extras - # for one attempt is not expressible, since empty is how inherit is - # spelled. + # §7.1: "Undeclared extras (§6) merge by the same per-key rule." The + # per-key rule is the one the DECLARED fields follow, and the analogue of + # 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. # - # Replacing discards whatever the base carried, so the keys that go - # missing are logged rather than dropped in silence. + # 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"}) - if override.extras: - dropped = sorted(set(base_or_empty.extras) - set(override.extras)) - if dropped: - _log.warning( - "per-attempt override for attempt %d replaces the base config's " - "extras; %s not sent on this attempt", - attempt, - ", ".join(repr(k) for k in dropped), - ) - update["extras"] = dict(override.extras) - else: - update["extras"] = dict(base_or_empty.extras) + update["extras"] = {**base_or_empty.extras, **override.extras} return base_or_empty.model_copy(update=update) @staticmethod @@ -799,7 +792,7 @@ async def _do_complete_with_retry( # one terminal LlmFailedEvent still fires -- rather than # leaking a non-§7 error and masking the real failure. try: - transcript = self._append_reask_pair(transcript, exc.raw_content, reask(exc)) + transcript = self._append_reask_pair(transcript, exc.output_content, reask(exc)) except Exception as reask_error: raise exc from reask_error next_retry_reason = "reask" @@ -960,15 +953,15 @@ def _build_llm_failed_event( # Empty content projects to None (as the success path does with # ``content or None``), so both observers omit it identically # rather than one rendering "" and the other dropping it. - output_content = exc.raw_content or None + output_content = exc.output_content or None finish_reason = exc.finish_reason usage = exc.usage response_id = exc.response_id response_model = exc.response_model # error_message carries the failing locator (proposal 0082): the # terse category summary alone drops the which-field detail, so - # combine it with the failure_description observers triage on. - error_message = f"{exc}: {exc.failure_description}" + # combine it with the error_message observers triage on. + error_message = f"{exc}: {exc.error_message}" return LlmFailedEvent( invocation_id=invocation_id, correlation_id=current_correlation_id(), @@ -1091,13 +1084,13 @@ def _build_llm_retry_attempt_event( response_fields = { # Empty content -> None (as the success path projects), so both # observers omit it identically. - "output_content": exc.raw_content or None, + "output_content": exc.output_content or None, "finish_reason": exc.finish_reason, "usage": exc.usage, "response_id": exc.response_id, "response_model": exc.response_model, } - error_message = f"{exc}: {exc.failure_description}" + error_message = f"{exc}: {exc.error_message}" return LlmRetryAttemptEvent( **base, error_category=exc.category, @@ -1351,7 +1344,7 @@ def _parse_response( # The wire response is intact; attach its response-side # context so the failed event / §7 error carries finish_reason # (truncation triage), usage, and the response identity - # (proposal 0082). raw_content is already set by the helper. + # (proposal 0082). output_content is already set by the helper. exc.finish_reason = finish_reason_typed exc.usage = usage exc.response_id = response_id @@ -1477,15 +1470,15 @@ def _parse_and_validate( raise StructuredOutputInvalid( "response content is not valid JSON", response_schema=schema_dict, - raw_content=content, - failure_description=str(exc), + output_content=content, + error_message=str(exc), ) from exc if not isinstance(loaded, dict): raise StructuredOutputInvalid( "response JSON is not an object", response_schema=schema_dict, - raw_content=content, - failure_description=f"top-level type is {type(loaded).__name__}, expected object", + output_content=content, + error_message=f"top-level type is {type(loaded).__name__}, expected object", ) parsed_dict = cast("dict[str, Any]", loaded) @@ -1504,15 +1497,15 @@ def _parse_and_validate( raise StructuredOutputInvalid( "response failed JSON Schema validation", response_schema=schema_dict, - raw_content=content, - failure_description=_format_jsonschema_failure(exc), + output_content=content, + error_message=_format_jsonschema_failure(exc), ) from exc except jsonschema.SchemaError as exc: raise StructuredOutputInvalid( "response could not be validated against the supplied schema", response_schema=schema_dict, - raw_content=content, - failure_description=str(exc), + output_content=content, + error_message=str(exc), ) from exc try: return schema_class.model_validate(parsed_dict) @@ -1520,8 +1513,8 @@ def _parse_and_validate( raise StructuredOutputInvalid( "response failed Pydantic validation", response_schema=schema_dict, - raw_content=content, - failure_description=str(exc), + output_content=content, + error_message=str(exc), ) from exc # Dict-schema path: jsonschema validation, return the dict. @@ -1531,8 +1524,8 @@ def _parse_and_validate( raise StructuredOutputInvalid( "response failed JSON Schema validation", response_schema=schema_dict, - raw_content=content, - failure_description=_format_jsonschema_failure(exc), + output_content=content, + error_message=_format_jsonschema_failure(exc), ) from exc except jsonschema.SchemaError as exc: # Safety net: validate_response_schema's pre-validation should @@ -1542,8 +1535,8 @@ def _parse_and_validate( raise StructuredOutputInvalid( "response could not be validated against the supplied schema", response_schema=schema_dict, - raw_content=content, - failure_description=str(exc), + output_content=content, + error_message=str(exc), ) from exc return parsed_dict @@ -1552,7 +1545,7 @@ 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 failure_description string carries both, matching the dict- + the error_message string carries both, matching the dict- schema and class-schema paths. """ return f"{exc.json_path}: {exc.message}" diff --git a/src/openarmature/llm/retry.py b/src/openarmature/llm/retry.py index d4ba418..8931a7c 100644 --- a/src/openarmature/llm/retry.py +++ b/src/openarmature/llm/retry.py @@ -26,8 +26,8 @@ # Proposal 0095b: a caller-supplied corrective-message builder for # structured-output reask. Given the raised StructuredOutputInvalid (the 0082 -# error surface -- ``exc.raw_content`` is the model's invalid output, -# ``exc.failure_description`` the reason), it returns the correction text OA +# error surface -- ``exc.output_content`` is the model's invalid output, +# ``exc.error_message`` the reason), it returns the correction text OA # appends as a user message. OA authors no prompt of its own (charter §3.1 # principle 7); the caller owns every word. Sync, mirroring classifier / backoff # -- pure string rendering. Passing the whole exception matches the classifier / diff --git a/tests/conformance/harness/wire.py b/tests/conformance/harness/wire.py index 97bf521..93d6541 100644 --- a/tests/conformance/harness/wire.py +++ b/tests/conformance/harness/wire.py @@ -220,27 +220,12 @@ def as_record_mapping(value: Any) -> Mapping[str, Any] | None: def _get_carries_attr(exc: BaseException, name: str) -> Any: - # Resolve a carries key to an exception attribute. Proposal 0098 renamed the - # structured_output_invalid carries keys to the llm-provider §7 error field - # names (output_content / error_message), but the Python - # StructuredOutputInvalid names those attributes raw_content / - # failure_description, so the alias below bridges the §7 names to the impl - # attributes. (The pre-0098 wire-side label raw_response_content is fully - # retired from the fixtures.) + # Resolve a carries key straight to an exception attribute. This used to + # carry an alias table bridging llm-provider section 7's `output_content` / + # `error_message` onto the implementation's own attribute names, with a + # comment saying it would stay correct if the implementation ever renamed + # them to match. It has, so the alias resolved to itself and is gone. # - # Resolve DIRECT-FIRST: an attribute named exactly as the carries key wins, - # and the alias is a fallback only when that attribute is absent. This keeps - # the alias from misdirecting a different error that legitimately exposes an - # output_content / error_message attribute, and stays correct if the impl - # ever renames its attributes to the §7 names. - aliases = { - "output_content": "raw_content", - "error_message": "failure_description", - } - # Single lookup for the direct case: a sentinel default avoids the - # hasattr + getattr double access (which would fire a descriptor twice). - missing = object() - direct = getattr(exc, name, missing) - if direct is not missing: - return direct - return getattr(exc, aliases.get(name, name), None) + # (The pre-0098 wire-side label raw_response_content is fully retired from + # the fixtures.) + return getattr(exc, name, None) diff --git a/tests/unit/test_llm_provider.py b/tests/unit/test_llm_provider.py index e2411b2..3d86ec0 100644 --- a/tests/unit/test_llm_provider.py +++ b/tests/unit/test_llm_provider.py @@ -1424,7 +1424,7 @@ async def test_complete_structured_output_failure_event_carries_response_surface # whose validation gate failed, so the LlmFailedEvent carries the # response-side surface (finish_reason for retry triage, output_content, # usage, response identity), and error_message carries the failing - # locator (the failure_description) rather than just the terse summary. + # locator (the error_message) rather than just the terse summary. from openarmature.graph.events import LlmFailedEvent from openarmature.llm import StructuredOutputInvalid @@ -1485,7 +1485,7 @@ def _handler(_req: httpx.Request) -> httpx.Response: def test_structured_output_builder_projects_empty_content_to_none() -> None: # Proposal 0082 cross-observer parity (defensive): a structured failure whose - # raw_content is empty projects output_content to None in the built event + # output_content is empty projects output_content to None in the built event # (like the success path's `content or None`), so both observers omit it # identically rather than one rendering "" and the other dropping it on a # truthiness gate. The OpenAI wire can't currently produce empty content on @@ -1496,7 +1496,7 @@ def test_structured_output_builder_projects_empty_content_to_none() -> None: provider = OpenAIProvider(base_url="http://test", model="m", api_key="k") exc = StructuredOutputInvalid( - "empty content", response_schema={}, raw_content="", failure_description="empty" + "empty content", response_schema={}, output_content="", error_message="empty" ) event = provider._build_llm_failed_event( # noqa: SLF001 exc, @@ -1864,7 +1864,7 @@ async def test_reask_corrects_on_retry() -> None: from openarmature.llm import LlmRetryConfig def reask(exc: Any) -> str: - return f"Your previous output {exc.raw_content} was invalid. Return corrected JSON." + return f"Your previous output {exc.output_content} was invalid. Return corrected JSON." bodies: list[dict[str, Any]] = [] calls = [0] @@ -1997,7 +1997,7 @@ async def test_reask_composes_with_override_and_accumulates() -> None: retry=LlmRetryConfig( max_attempts=3, backoff=deterministic_backoff(0), - reask=lambda e: f"fix {e.raw_content}", + reask=lambda e: f"fix {e.output_content}", per_attempt_override=[RuntimeConfig(temperature=0.3), RuntimeConfig(temperature=0.6)], ), ) @@ -2038,7 +2038,7 @@ async def test_reask_transient_interleave_carries_transcript() -> None: [UserMessage(content="go")], response_schema=_PERSON_SCHEMA, retry=LlmRetryConfig( - max_attempts=3, backoff=deterministic_backoff(0), reask=lambda e: f"fix {e.raw_content}" + max_attempts=3, backoff=deterministic_backoff(0), reask=lambda e: f"fix {e.output_content}" ), ) finally: @@ -2204,8 +2204,8 @@ async def test_call_level_retry_plain_config_replays_identically() -> None: lambda: StructuredOutputInvalid( "boom", response_schema={}, - raw_content="", - failure_description="", + output_content="", + error_message="", ), "StructuredOutputInvalid", "structured_output_invalid", @@ -2236,7 +2236,7 @@ def test_build_llm_failed_event_maps_category_and_type_per_exception( assert event.error_category == expected_category assert event.error_type == expected_cls_name # error_message is str(exc) for all categories except - # structured_output_invalid, which appends the failure_description + # structured_output_invalid, which appends the error_message # locator (proposal 0082); startswith covers both. assert event.error_message.startswith("boom") assert event.latency_ms == 12.0 @@ -3456,13 +3456,15 @@ def handler(req: httpx.Request) -> httpx.Response: assert bodies[1]["temperature"] == 0.6 -async def test_per_attempt_override_with_extras_replaces_and_warns( - caplog: pytest.LogCaptureFixture, -) -> None: - # An override that DOES declare extras replaces the container, following the - # same rule its declared fields follow. The base keys it does not carry are - # dropped for that attempt, so they are logged: replacing is legitimate, but - # losing a key the caller set without saying so is not. +async def test_per_attempt_override_with_extras_merges_per_key() -> None: + # Section 7.1: "Undeclared extras (section 6) merge by the same per-key rule." + # The analogue of a declared field is an extras KEY, not the container, so a + # 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 @@ -3491,29 +3493,28 @@ def handler(req: httpx.Request) -> httpx.Response: provider = _collision_provider(handler) try: - with caplog.at_level(logging.WARNING): - await provider.complete( - [UserMessage(content="hi")], - config=RuntimeConfig(extras={"strict_grammar": "g", "keep": 1}), - retry=LlmRetryConfig( - max_attempts=2, - backoff=deterministic_backoff(0), - per_attempt_override=[RuntimeConfig(extras={"relaxed": True})], - ), - ) + await provider.complete( + [UserMessage(content="hi")], + config=RuntimeConfig(extras={"strict_grammar": "g", "keep": 1}), + retry=LlmRetryConfig( + max_attempts=2, + backoff=deterministic_backoff(0), + per_attempt_override=[RuntimeConfig(extras={"relaxed": True})], + ), + ) finally: await provider.aclose() assert len(bodies) == 2, f"expected a retry, got {len(bodies)} call(s)" - # Attempt 0 carries the base's extras. + # Attempt 0 carries the base's extras, and none of the override's. assert bodies[0]["strict_grammar"] == "g" assert "relaxed" not in bodies[0] - # Attempt 1 carries the override's, and NOT the base's. + # Attempt 1 carries the override's key AND inherits the base's, which is the + # per-key rule. Asserting the inherited keys is the non-vacuous half: the + # override's own key arrives under either reading. assert bodies[1]["relaxed"] is True - assert "strict_grammar" not in bodies[1], "the override's extras must replace, not merge" - assert "keep" not in bodies[1] - # The dropped keys are named rather than lost silently. - warnings_seen = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] - assert any("strict_grammar" in m and "keep" in m for m in warnings_seen), ( - f"expected a warning naming the dropped keys; got {warnings_seen}" + assert bodies[1]["strict_grammar"] == "g", ( + "a base extras key the override does not mention must inherit, per the same " + "per-key rule the declared fields follow" ) + assert bodies[1]["keep"] == 1, "every unmentioned base key inherits, not just the first" diff --git a/tests/unit/test_observability_otel.py b/tests/unit/test_observability_otel.py index 3ca3d3f..ea103ee 100644 --- a/tests/unit/test_observability_otel.py +++ b/tests/unit/test_observability_otel.py @@ -2120,11 +2120,11 @@ 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 raw_content / - # failure_description (0098's output_content / error_message §7 names alias + # 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": "raw_content", "error_message": "failure_description"} + alias = {"output_content": "output_content", "error_message": "error_message"} for key, want in carries.items(): if key.endswith("_present"): attr = alias.get(key[:-8], key[:-8]) @@ -2213,8 +2213,8 @@ def handler(request: httpx.Request) -> httpx.Response: def _reask(exc: StructuredOutputInvalid) -> str: return ( cast("str", reask_template) - .replace("{output_content}", exc.raw_content or "") - .replace("{error_message}", exc.failure_description or "") + .replace("{output_content}", exc.output_content or "") + .replace("{error_message}", exc.error_message or "") ) retry = LlmRetryConfig( diff --git a/tests/unit/test_structured_output.py b/tests/unit/test_structured_output.py index 29f01a9..8e767d1 100644 --- a/tests/unit/test_structured_output.py +++ b/tests/unit/test_structured_output.py @@ -532,8 +532,8 @@ async def test_pydantic_class_path_rejects_coercible_string_for_int() -> None: ) finally: await provider.aclose() - assert "age" in excinfo.value.failure_description - assert "integer" in excinfo.value.failure_description + assert "age" in excinfo.value.error_message + assert "integer" in excinfo.value.error_message async def test_pydantic_validation_failure_wraps_in_structured_output_invalid() -> None: @@ -554,8 +554,8 @@ async def test_pydantic_validation_failure_wraps_in_structured_output_invalid() finally: await provider.aclose() err = excinfo.value - assert err.raw_content == '{"name":"Alice","age":"thirty"}' - assert "age" in err.failure_description + assert err.output_content == '{"name":"Alice","age":"thirty"}' + assert "age" in err.error_message # Proposal 0082: the error carries the intact response's response-side # context (finish_reason for retry triage, usage, response identity), # attached at the parse/validate call site. From e613b88f2e7aa5ad79cefca84bde309b29409b68 Mon Sep 17 00:00:00 2001 From: chris-colinsky Date: Fri, 25 Sep 2026 22:43:08 -0700 Subject: [PATCH 2/2] Fix three docs the rename and the merge change contradicted All three from review, all three documentation left asserting what the code no longer does. The field rename was a mechanical string replacement, which turned "a failure_description" into "a error_message" in published concept docs. Swept the renamed files for any other article broken the same way; this was the only one. Two docstrings still said a non-empty extras container replaces wholesale: `_config_for_attempt`'s and `LlmRetryConfig`'s. The second is the damaging one, since LlmRetryConfig is public API and that text is what a caller reads before writing an override, so it stated the opposite of what the call now does. The inline comment beside the code was updated and these were not. `assert_error_carries`'s docstring documented three attributes that no longer resolve, now that the alias table is gone and the carries key resolves straight to an attribute. Two of the three were broken by the rename; `raw_response_content` was already stale, retired from the fixtures by 0098 and surviving only in that text. --- docs/concepts/llms.md | 2 +- src/openarmature/llm/providers/openai.py | 5 +++-- src/openarmature/llm/retry.py | 5 +++-- tests/conformance/harness/wire.py | 6 +++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/concepts/llms.md b/docs/concepts/llms.md index eeeac6c..8d109f3 100644 --- a/docs/concepts/llms.md +++ b/docs/concepts/llms.md @@ -702,7 +702,7 @@ can handle them. when a model returns content that fails to parse as JSON, or parses but fails to validate against the supplied schema. The exception carries the requested `response_schema`, the `output_content` the model -produced, and a `error_message`. It is non-transient by default +produced, and an `error_message`. It is non-transient by default because a model that emits non-conforming output on a given prompt usually emits the same non-conforming output on retry. Useful retry strategies for this case involve changing the prompt or doubling diff --git a/src/openarmature/llm/providers/openai.py b/src/openarmature/llm/providers/openai.py index 19ff09f..2ba8a38 100644 --- a/src/openarmature/llm/providers/openai.py +++ b/src/openarmature/llm/providers/openai.py @@ -629,8 +629,9 @@ def _config_for_attempt( (attempt ``i > 0``) merges ``overrides[i-1]`` onto the base -- the override's non-None fields replace, a None field (like an absent one) inherits the base -- and the last entry carries forward when the - schedule is shorter than the retry count. ``extras`` replaces when the - override declares any and inherits when it declares none. Attempt 0 and the no-override + 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 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 diff --git a/src/openarmature/llm/retry.py b/src/openarmature/llm/retry.py index 8931a7c..cb2a973 100644 --- a/src/openarmature/llm/retry.py +++ b/src/openarmature/llm/retry.py @@ -47,8 +47,9 @@ class LlmRetryConfig(RetryConfig): partial-overrides applied to RETRIES only. Attempt 0 uses the caller's base ``config`` unmodified; retry ``i`` (attempt ``i+1``) merges ``per_attempt_override[i]`` onto the base (the override's set fields - replace, unspecified fields inherited; ``extras`` counts as unspecified - when empty, so it inherits, and replaces wholesale when not). When the + replace, unspecified fields inherited; ``extras`` merges by that same + per-key rule, so a key the override sets replaces that key and a key it + omits inherits the base's). When the schedule is shorter than the retry count, the last entry carries forward. The canonical form is an escalating temperature schedule (e.g. ``[RuntimeConfig(temperature=0.3), diff --git a/tests/conformance/harness/wire.py b/tests/conformance/harness/wire.py index 93d6541..4d28f5b 100644 --- a/tests/conformance/harness/wire.py +++ b/tests/conformance/harness/wire.py @@ -152,12 +152,12 @@ def assert_error_carries(exc: BaseException, carries: Mapping[str, Any]) -> None - ``_present: true`` — attribute MUST be set to a truthy non-None value (e.g., ``response_schema_present``, - ``failure_description_present``). + ``error_message_present``). - ``: `` — attribute value equals the supplied - value (e.g., ``raw_response_content: '...'``). + value (e.g., ``output_content: '...'``). - ``_mentions: `` — string attribute value contains the supplied substring (e.g., - ``failure_description_mentions: 'age'``). + ``error_message_mentions: 'age'``). """ for key, expected in carries.items(): if key.endswith("_present"):