Canonicalize classified stream failures - #438
Conversation
Add canonical determinate and indeterminate stream failure chunks, regenerate every configured model target, and define shared behavior vectors while retaining ErrorChunk compatibility. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the published handwritten stream API while mapping classified failures to generated models, consuming shared vectors, and enforcing reconciliation without completion commits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Emit generated failure chunks from provider streams and surface partial content plus reconciliation requirements without committing successful turns. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| if (data.TryGetValue("kind", out var kindValue) && kindValue is not null) | ||
| { | ||
| instance.Kind = kindValue?.ToString()!; |
There was a problem hiding this comment.
This file is generated by Typra 0.4.2, and the same ?.ToString()! emitter pattern occurs broadly across the generated C# target (359 occurrences in 130 files). Hand-editing this one generated class would be overwritten by deterministic regeneration and would make this schema-only PR non-reproducible. The coordinated Typra 0.4.4 consolidation will update the emitter pin and regenerate the combined schema after this PR lands; CodeQL itself passes in this PR.
|
|
||
| if (data.TryGetValue("outcome", out var outcomeValue) && outcomeValue is not null) | ||
| { | ||
| instance.Outcome = StreamFailureOutcomeParser.Parse(outcomeValue?.ToString()!); |
There was a problem hiding this comment.
This is generated Typra 0.4.2 output, not handwritten runtime code. The same null-conditional emission pattern is emitter-wide (359 occurrences across 130 generated C# files), so a local generated-file edit would be non-deterministic and outside this focused schema contract. The coordinated Typra 0.4.4 follow-up will pin the newer emitter and regenerate all targets after this PR lands; CodeQL passes here.
|
|
||
| if (data.TryGetValue("message", out var messageValue) && messageValue is not null) | ||
| { | ||
| instance.Message = messageValue?.ToString()!; |
There was a problem hiding this comment.
This is the same emitter-wide Typra 0.4.2 generated-code pattern (359 occurrences across 130 generated C# files). A schema-specific hand edit would be overwritten on regeneration and break reproducibility. We are keeping this PR on the published 0.4.2 baseline and coordinating the 0.4.4 emitter/regeneration follow-up after merge; CodeQL passes on this change.
There was a problem hiding this comment.
🟡 Not ready to approve
The TypeScript streaming processors should explicitly cancel/close the underlying async iterator when emitting terminal FailureChunks to avoid leaking streaming resources.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR introduces a canonical, discriminator-backed model for classified terminal streaming failures (StreamFailureOutcome, StreamFailure, FailureChunk) and wires it through the schema, generated runtimes, vectors, and TypeScript streaming processors so partial content can be surfaced while enforcing reconciliation/no-completion semantics.
Changes:
- Extend the canonical stream-chunk schema with
failurediscriminator plusStreamFailure{outcome,message}and emit docs/JSON Schemas accordingly. - Add shared process vectors for determinate/indeterminate stream failures and update TypeScript/Rust processors + core pipeline to surface
requiresReconciliationand partial content without committing successful turns. - Regenerate Typra outputs across runtimes (TS/Python/Go/C#/Rust) and add/adjust corresponding generated-model tests.
File summaries
| File | Description |
|---|---|
| web/src/content/docs/reference/StreamFailure.md | New generated reference doc for StreamFailure. |
| web/src/content/docs/reference/StreamChunk.md | Documents FailureChunk as a StreamChunk subtype. |
| web/src/content/docs/reference/FailureChunk.md | New generated reference doc for FailureChunk. |
| vscode/prompty/schemas/StreamFailure.yaml | New JSON Schema for StreamFailure for the VS Code extension. |
| vscode/prompty/schemas/StreamChunk.yaml | Adds failure to the StreamChunk.kind discriminator schema. |
| vscode/prompty/schemas/FailureChunk.yaml | New JSON Schema for FailureChunk. |
| spec/vectors/process/stream_failure_vectors.json | Adds deterministic/indeterministic stream-failure processing vectors. |
| schema/tsp-output/.typra-generated/manifest.json | Tracks new generated outputs/tests for the added failure contracts. |
| schema/tsp-output/.typra-generated/export-surfaces.json | Exposes FailureChunk/StreamFailure in generated export surfaces. |
| schema/model/events/stream-chunks.tsp | Canonical TypeSpec additions: StreamFailureOutcome, StreamFailure, FailureChunk, discriminator update. |
| runtime/typescript/packages/openai/tests/stream-failure-vectors.test.ts | OpenAI processor conformance against shared stream-failure vectors. |
| runtime/typescript/packages/openai/src/processor.ts | Emits FailureChunk for refusal/transport errors during streaming. |
| runtime/typescript/packages/core/tests/stream-failures.test.ts | Core turn() reconciliation/no-commit semantics validated via vectors. |
| runtime/typescript/packages/core/tests/model/events/stream-failure.test.ts | Generated model serialization/roundtrip tests for StreamFailure. |
| runtime/typescript/packages/core/tests/model/events/failure-chunk.test.ts | Generated model construction/load/save tests for FailureChunk. |
| runtime/typescript/packages/core/src/model/index.ts | Exports StreamFailure and FailureChunk from the model surface. |
| runtime/typescript/packages/core/src/model/events/stream-failure.ts | New generated StreamFailure model/type. |
| runtime/typescript/packages/core/src/model/events/stream-chunk.ts | Adds FailureChunk discriminator branch + generated class. |
| runtime/typescript/packages/core/src/model/events/index.ts | Re-exports StreamFailure/FailureChunk from events index. |
| runtime/typescript/packages/core/src/index.ts | Re-exports StreamFailureError and stream failure model types. |
| runtime/typescript/packages/core/src/core/pipeline.ts | Introduces StreamFailureError, consumes streams, enforces reconciliation/no-success semantics. |
| runtime/typescript/packages/core/src/core/index.ts | Re-exports StreamFailureError from core entry point. |
| runtime/typescript/packages/anthropic/src/processor.ts | Emits FailureChunk on streaming transport errors. |
| runtime/rust/prompty/tests/stream_chunk_compatibility.rs | Validates handwritten Rust stream failure compatibility ↔ generated model bridge. |
| runtime/rust/prompty/tests/model/events/stream_failure_test.rs | Generated Rust model tests for StreamFailure. |
| runtime/rust/prompty/tests/model/events/mod.rs | Registers the new generated Rust stream_failure_test module. |
| runtime/rust/prompty/src/types.rs | Adds StreamFailure ↔ generated model::StreamFailure conversions and derives. |
| runtime/rust/prompty/src/pipeline/live_turn.rs | Adds determinate/indeterminate stream failure live-turn tests driven by vectors. |
| runtime/rust/prompty/src/model/events/stream_failure.rs | New generated Rust StreamFailure + StreamFailureOutcome. |
| runtime/rust/prompty/src/model/events/stream_chunk.rs | Adds FailureChunk discriminator and nested StreamFailure handling. |
| runtime/rust/prompty/src/model/events/mod.rs | Exports the new stream_failure generated module. |
| runtime/rust/prompty-openai/tests/stream_failure_vectors.rs | Rust OpenAI stream processor conformance against shared vectors. |
| runtime/rust/prompty-openai/src/processor.rs | Emits classified StreamChunk::Failure(StreamFailure::Determinate(..)) for refusal cases. |
| runtime/python/prompty/tests/model/events/test_stream_failure.py | Generated Python model tests for StreamFailure. |
| runtime/python/prompty/prompty/model/events/_StreamFailure.py | New generated Python StreamFailure model. |
| runtime/python/prompty/prompty/model/events/_StreamChunk.py | Adds generated FailureChunk discriminator branch/class. |
| runtime/python/prompty/prompty/model/events/init.py | Re-exports StreamFailure/FailureChunk from Python events package. |
| runtime/python/prompty/prompty/model/init.py | Re-exports StreamFailure/FailureChunk from Python model surface. |
| runtime/go/prompty/model/stream_failure.go | New generated Go StreamFailure and StreamFailureOutcome. |
| runtime/go/prompty/model/stream_failure_test.go | Generated Go tests for StreamFailure load/roundtrip helpers. |
| runtime/go/prompty/model/stream_chunk.go | Adds generated Go FailureChunk and discriminator loader branch. |
| runtime/go/prompty/model/failure_chunk_test.go | Placeholder generated Go test file for FailureChunk. |
| runtime/csharp/Prompty.Core/Model/events/StreamFailureOutcome.cs | New generated C# enum for StreamFailureOutcome. |
| runtime/csharp/Prompty.Core/Model/events/StreamFailure.cs | New generated C# model for StreamFailure. |
| runtime/csharp/Prompty.Core/Model/events/StreamChunk.cs | Adds failure discriminator branch to generated C# StreamChunk. |
| runtime/csharp/Prompty.Core/Model/events/FailureChunk.cs | New generated C# model for FailureChunk. |
| runtime/csharp/Prompty.Core.Tests/Model/events/StreamFailureConversionTests.cs | Generated C# conversion/roundtrip tests for StreamFailure. |
| runtime/csharp/Prompty.Core.Tests/Model/events/FailureChunkConversionTests.cs | Placeholder generated C# conversion test file for FailureChunk. |
Review details
Files not reviewed (4)
- runtime/go/prompty/model/failure_chunk_test.go: Generated file
- runtime/go/prompty/model/stream_chunk.go: Generated file
- runtime/go/prompty/model/stream_failure.go: Generated file
- runtime/go/prompty/model/stream_failure_test.go: Generated file
Suppressed comments (1)
runtime/typescript/packages/openai/src/processor.ts:145
- On refusal, the generator returns early after yielding a FailureChunk but does not cancel the underlying response iterator. Explicitly calling iterator.return() before yielding ensures the upstream stream is closed even if the consumer stops immediately after receiving the failure chunk.
- Files reviewed: 44/48 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| let next: IteratorResult<unknown>; | ||
| try { | ||
| next = await iterator.next(); | ||
| } catch (error) { | ||
| yield failureChunk("indeterminate", errorMessage(error)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fixed in 22351d0. The OpenAI processor now calls iterator.return() before yielding a terminal FailureChunk for both transport failures and refusals; cleanup failures are debug-logged so they cannot mask the canonical stream failure. Added focused tests that assert the upstream iterator is closed.
| let next: IteratorResult<unknown>; | ||
| try { | ||
| next = await iterator.next(); | ||
| } catch (error) { | ||
| yield new FailureChunk({ | ||
| failure: new StreamFailure({ | ||
| outcome: "indeterminate", | ||
| message: error instanceof Error ? error.message : String(error), | ||
| }), | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Fixed in 22351d0. The Anthropic processor now calls iterator.return() before yielding the indeterminate terminal FailureChunk; cleanup failures are debug-logged without replacing the primary failure. Added a focused transport-failure test that asserts closure.
Cancel upstream async iterators before yielding terminal failure chunks so SSE resources are released without masking the classified failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Record permission denials as committed tool results across reference runners without emitting tool execution events. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove trailing spaces from the canonical instruction sample so generated JSON round-trip fixtures agree across runtimes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Apply the configured Ruff formatting and assert the nullable strict-output wire shape already produced for optional fields. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Not ready to approve
The typra-emitter generated Go and C# FailureChunk conversion test files are empty, leaving the new FailureChunk helpers without direct cross-language conversion/round-trip coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (5)
- runtime/go/prompty/model/failure_chunk_test.go: Generated file
- runtime/go/prompty/model/prompty_test.go: Generated file
- runtime/go/prompty/model/stream_chunk.go: Generated file
- runtime/go/prompty/model/stream_failure.go: Generated file
- runtime/go/prompty/model/stream_failure_test.go: Generated file
Suppressed comments (2)
runtime/csharp/Prompty.Core.Tests/Model/events/FailureChunkConversionTests.cs:11
- This generated test file defines an empty
FailureChunkConversionTestsclass, so C# has no direct JSON/YAML/round-trip coverage for the newFailureChunkconversion helpers (unlike other generated conversion tests in this directory). Since the header indicates this is typra-emitter output, the fix should be in the emitter’s C# test template (or by adding a minimal conversion test generation forFailureChunk) and then regenerating outputs.
public class FailureChunkConversionTests
{
}
runtime/go/prompty/model/failure_chunk_test.go:4
- This generated Go test file contains only the header/package declaration and no tests, so there’s no Go-side JSON/YAML/round-trip coverage for the new
FailureChunkhelpers (contrast with other generated *_test.go files like error_chunk_test.go). Since this is typra-emitter output, update the emitter’s Go test template to generate basic conversion tests forFailureChunk(or omit generating this file) and regenerate.
- Files reviewed: 56/63 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Add a canonical nested failure sample so Typra emits load/save and JSON/YAML conversion coverage for FailureChunk across configured runtimes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed the generated conversion-coverage finding in |
There was a problem hiding this comment.
🟡 Not ready to approve
The TypeScript provider processors’ manual iterator.next() loops risk leaving upstream streaming iterators unclosed when consumers cancel early, which can leak streaming resources in real deployments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (5)
- runtime/go/prompty/model/failure_chunk_test.go: Generated file
- runtime/go/prompty/model/prompty_test.go: Generated file
- runtime/go/prompty/model/stream_chunk.go: Generated file
- runtime/go/prompty/model/stream_failure.go: Generated file
- runtime/go/prompty/model/stream_failure_test.go: Generated file
Suppressed comments (2)
runtime/typescript/packages/openai/src/processor.ts:99
streamGeneratorswitched fromfor await...ofto manualiterator.next()reads; this loses the automaticiterator.return()call thatfor await...ofperforms when the consumer stops early (e.g., caller breaks out of the loop or cancels a request). That can leave SSE/ReadableStream-backed providers open longer than necessary unless the generator is explicitly closed.
Consider wrapping the body in try { ... } finally { await closeIterator(iterator); } (and keeping the existing pre-yield closes for refusal/transport errors) so upstream resources are released even on early consumer cancellation.
runtime/typescript/packages/anthropic/src/processor.ts:88
streamGeneratoruses a manualiterator.next()loop. Unlikefor await...of, this won’t automatically calliterator.return()when the consumer stops iteration early, which can keep a streaming connection open unnecessarily.
Recommend adding a try { ... } finally { await closeIterator(iterator); } around the loop (and/or ensuring early cancellation paths always close) so upstream streams are reliably released even when callers abort consumption.
- Files reviewed: 57/64 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Sample the canonical failure discriminator so generated conversion tests assert it survives JSON and YAML round trips in every configured runtime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Run upstream iterator cleanup from async-generator finally blocks so consumer cancellation releases SSE resources without double-closing failure paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed the latest Copilot cancellation finding in |
There was a problem hiding this comment.
🟡 Human review recommended
It introduces cross-runtime schema and streaming-semantic changes that require final human verification beyond spot-checking diffs and unit tests.
Review details
Files not reviewed (5)
- runtime/go/prompty/model/failure_chunk_test.go: Generated file
- runtime/go/prompty/model/prompty_test.go: Generated file
- runtime/go/prompty/model/stream_chunk.go: Generated file
- runtime/go/prompty/model/stream_failure.go: Generated file
- runtime/go/prompty/model/stream_failure_test.go: Generated file
- Files reviewed: 57/64 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| kind: string = "failure"; | ||
| failure!: StreamFailure; | ||
|
|
There was a problem hiding this comment.
FailureChunk.failure intentionally has no schema default: neither a determinate nor indeterminate outcome, nor a fabricated message, is safe to invent. Typra-generated model constructors currently support partial construction across required non-collection fields and omit absent values from save(); this is emitter-wide model behavior, not specific to FailureChunk. Every runtime production construction in this PR supplies a canonical StreamFailure, and the generated/load-save plus shared vectors cover populated instances. Changing constructor validity or required-field enforcement belongs in the coordinated Typra 0.4.4 consolidation, not as a hand edit or schema default in this focused PR.
Summary
StreamFailureOutcome,StreamFailure, and discriminator-backedFailureChunkcontracts while retainingErrorChunkrequiresReconciliationwithout committing successful turnsCompatibility
ErrorChunkremains in the canonical schemaStreamChunk::Error(String)remains constructible and matchablemodel::StreamFailureValidation
schema:npm run build; regeneration is deterministic (git diff --exit-code)cargo test --workspacepassestsc --noEmitpass; OpenAI vectors 2/2 and core reconciliation vectors 2/2 passExisting unrelated suite failures
Full-suite runs still expose existing failures outside this change: the shared permission-denied journal vector omits
tool_resultin TypeScript/Python/Go/C#, and older Typra-generated Prompty/property conversion fixtures fail in Python/Go/C#. The classified stream-failure tests and all changed runtime paths pass; this PR does not fold those emitter/harness defects into the schema-contract change.