feat: a stream call returns a Result — retried before its first chunk, watched for stalls, finished only when it says so #20

Merged

bjoern merged 4 commits from feat/stream-to-result into main 2026-09-23 16:44:59 +02:00

Conversation 4 Commits 4 Files changed 33 +3623 -89

scarlet commented 2026-09-23 15:59:26 +02:00

Member

Copy link

The last of the three streaming PRs. #16 made every chunk parse and gave every failure a kind, and #18 folds the chunks. This PR adds StreamChatCompletionToResultAsync: a streamed call that ends in a Result<ChatCompletionResponse>. Tsumugi's llm node (Phase 2, slice 8) streams through it.

It also takes Jibril's late LogProbs point from #18, as its own commit.

The call

var accumulator = new ChatCompletionStreamAccumulator();
var result = await client.StreamChatCompletionToResultAsync(
    request, accumulator, onChunk: chunk => ShowDelta(chunk),
    new StreamResultOptions { StallTimeout = TimeSpan.FromSeconds(90) }, ct);
// Success: the completion a non-streaming call would have returned.
// Failure: its Kind says what happened; the accumulator still holds everything that arrived.

• A default interface member on IOpenRouterClient. It is additive, so every implementer keeps compiling.
  • The default body folds the public iterator. It never retries, runs no watchdog, and says so in its <remarks>.
  • OpenRouterClient overrides it with the real version, which needs internals an extension method cannot reach: the retry options, the clock, Retry-After, the SSE comment lines, and the [DONE] sentinel.
  • CachingOpenRouterClient forwards it explicitly. Otherwise the default body would silently drop the inner client's retry and watchdog.
• Retry happens only before the first chunk, through the client's RetryPolicy and honouring Retry-After in seconds and as an HTTP date against the client's clock. "The first chunk" is the first data payload that parses to a chunk that is not an error. An inline retryable error, a drop, a closed body or a stall before it is retried. Nothing after it is.
• Stall watchdog. StreamResultOptions.StallTimeout is null by default, and then no timer exists at all. When set, the watchdog is armed per attempt before the send, and every SSE line resets it, keep-alive comments included. On firing it cancels the HTTP read, and the result is Stalled. Backoff waits are not watched.
• Outcomes, pinned by tests and probes:
  • A caller's cancel throws OperationCanceledException carrying the caller's token, with or without the watchdog. It is never a Success or a Failure.
  • Only [DONE] or a finish reason counts as finished. After the finish reason the call reads on until [DONE] or EOF, because usage and cost arrive in a later chunk with empty choices.
  • EOF with neither is Interrupted, with the partial text kept in the accumulator.
  • An inline error chunk is a Failure with its kind. After the finish reason it returns the finished completion instead, so a paid generation is not paid for twice. The same holds for a drop or a stall after finishing.
  • The request is serialized once, before the retry loop; a body that can't be written is a Validation failure and nothing is sent.
• Under the hood.
  • SseEventReader gains an internal typed-line overload: data, comment, done.
  • StallWatchdog, StreamOutcomes (the rules both paths share) and StreamSession are internal.
  • Stalled joins IsRetryableTransport.
  • The public iterator's while (!ct.IsCancellationRequested) loop no longer ends quietly on a cancel; it throws. That row is in the README upgrade table.
• Log-probabilities, from #18. Each chunk's choices[i].logprobs carries only that chunk's tokens, so the accumulator appends the content and refusal lists per choice, in arrival order. Last-wins would keep only the final chunk's tokens. It emits the reference's {content, refusal} object, keeps entries byte for byte, and skips a logprobs that is not an object or a list that is not an array. A probe of last-wins fails the three join-order tests.

Decisions to confirm

1. A cancel that neither the caller nor the watchdog caused, such as a handler or resilience pipeline aborting the read. The brief said Timeout everywhere. It is now Timeout before the first chunk, Interrupted after it, like a dropped connection, and Success once the model has finished, in both paths. The reason: FailureKind.Timeout documents HttpClient.Timeout, which stops covering a stream once its headers arrive.
2. A mid-stream error chunk that names neither a type nor a failure status is Interrupted, not ProviderUnavailable. Before the first chunk it is ProviderUnavailable and retried.
3. One PR at ~3.6k lines, not split. About 2.5k of it is tests. The brief's split trigger was ~3.2k. A first PR would have held the default body, StreamOutcomes, StreamResultOptions, the fake client and the fallback tests (~1k); the override would have followed (~2.6k). Jibril is fine with it as one; say if you'd rather have it split.

Known gaps, stated

• The default body cannot tell a request that failed to serialize from a chunk that failed to parse: both are status-less OpenRouterExceptions. A decorator that does not forward the call would see the first as Malformed or Unknown rather than Validation. OpenRouterClient never takes this path. A clean fix needs a marker on the exception, not a message-prefix test.
• The streamed context-length test uses the documented rejection shape. The live capture requested in #16 has not been run yet.

Merged with main

main moved twice while this was open, and both merges are in the branch.

#17 and #18 (6e8ca5f). The conflicts resolved:

• IOpenRouterClient takes #17's ListModelsAsync summary after the new member.
• OpenRouterClient keeps both of #17's listing overloads after the override.
• CachingOpenRouterClient's summary names both the per-filter cache and the forwarded stream call.
• CachingClientTests combines #17's tests with this PR's forward tests. #17's nested fake clock is replaced by Microsoft.Extensions.TimeProvider.Testing's FakeTimeProvider, which this PR pins test-only.
• The README upgrade table keeps both sides' rows.
• The test-only FakeOpenRouterClient implements #17's query overload.

#19, decisions (b7d9865). The conflicts resolved:

• CachingOpenRouterClient's summary keeps both clauses: the stream call is forwarded, and decisions calls are never cached.
• The README Kind table's Malformed row names both a decisions response without answers or usage and a streamed chunk that does not parse, or [DONE] before any chunk.
• The test-only FakeOpenRouterClient implements CreateDecisionsAsync.

Tests

	main (adf32cb, after #19)	this PR
OpenRouter.Net.Tests	766	922
OpenRouter.Net.Agents.Tests	63	63
OpenRouter.Net.Imaging.Tests	7	7
Total	836	992

• New test files: StreamToResultTests (the override), StreamWatchdogTests, StreamToResultFallbackTests (the default body through a fake that has no override), plus the new SseEventReaderTests rows and the CachingClientTests forward tests, which call through the interface so a missing forward is a failing test, not a compile error.
• Test infrastructure:
  • SseScript is a scripted SSE body. It can pause until the test releases it, throw IOException mid-body, ignore a cancel, and report whether its read was cancelled.
  • ObservableTimeProvider and FakeTimeProvider drive every watchdog test. No test sleeps, and every scripted-stream test has a 10 s real-time guard, so a broken cancel fails instead of hanging.
  • Cancel tests run with retries off, because a backoff wait would otherwise hide a cancel mis-mapped to Timeout.
• Races pinned:
  • a cancel that lands as the watchdog fires;
  • a line that arrives as the stall fires;
  • callback time counting toward the stall timeout;
  • a stall while reading an error body;
  • the stall message formatted invariantly under de-DE.
• The full suite passed three runs in a row after the first merge, and two runs after the #19 merge.

Coverage (dotnet test --coverage --coverage-output-format cobertura, per changed file, best across the three reports): every changed or new source file is at 100% line. Four partial branches remain, none of them in code this PR adds:

• OpenRouterClient.cs:117 and SseEventReader.cs:69/:89 are the compiler-generated guards at async-iterator closing braces. :117 was already partial on main.
• CachingOpenRouterClient.cs:42 is the pre-existing inner ?? throw.

Mutation probes: 112 were run, one at a time on a copy, each restored before the next. That is the brief's 31–57, the verifiers' additions, and the review round's catches: foreign cancel, the caller's token, error-body stall, culture, HTTP-date Retry-After, the line-as-stall race and callback hold. All are killed. The one exception is 55a, the brief's literal Action<object> form, which fails to compile; its run-time variant 55b is killed. The log-probability last-wins probe is killed too.

README

• A new Streaming into a finished completion subsection with the sample and an outcomes table.
• The Kind table gains its Stalled and Interrupted rows, and the "no call returns them yet" sentence is gone.
• The Result section and retries paragraph cover streams, and RetryOnNetworkError now names the stream cases it decides.
• Upgrade rows: the iterator throwing on cancel, and IOpenRouterClient gaining a member with a default body, so a decorator must forward it.
• The folding subsection mentions log-probabilities.

Out of scope

The streaming agent loop, concurrent tools and the typed structured-output helper open Phase 4. L3 (JSON parse arms and the strict-schema check) is the next library PR.

🤖 Generated with Claude Code

The last of the three streaming PRs. #16 made every chunk parse and gave every failure a kind, and #18 folds the chunks. This PR adds **`StreamChatCompletionToResultAsync`**: a streamed call that ends in a `Result<ChatCompletionResponse>`. Tsumugi's `llm` node (Phase 2, slice 8) streams through it. It also takes Jibril's late `LogProbs` point from #18, as its own commit. ## The call ```csharp var accumulator = new ChatCompletionStreamAccumulator(); var result = await client.StreamChatCompletionToResultAsync( request, accumulator, onChunk: chunk => ShowDelta(chunk), new StreamResultOptions { StallTimeout = TimeSpan.FromSeconds(90) }, ct); // Success: the completion a non-streaming call would have returned. // Failure: its Kind says what happened; the accumulator still holds everything that arrived. ``` - **A default interface member on `IOpenRouterClient`.** It is additive, so every implementer keeps compiling. - The default body folds the public iterator. It never retries, runs no watchdog, and says so in its `<remarks>`. - `OpenRouterClient` overrides it with the real version, which needs internals an extension method cannot reach: the retry options, the clock, `Retry-After`, the SSE comment lines, and the `[DONE]` sentinel. - `CachingOpenRouterClient` forwards it explicitly. Otherwise the default body would silently drop the inner client's retry and watchdog. - **Retry happens only before the first chunk**, through the client's `RetryPolicy` and honouring `Retry-After` in seconds and as an HTTP date against the client's clock. "The first chunk" is the first data payload that parses to a chunk that is not an error. An inline retryable error, a drop, a closed body or a stall before it is retried. Nothing after it is. - **Stall watchdog.** `StreamResultOptions.StallTimeout` is null by default, and then no timer exists at all. When set, the watchdog is armed per attempt before the send, and every SSE line resets it, keep-alive comments included. On firing it cancels the HTTP read, and the result is `Stalled`. Backoff waits are not watched. - **Outcomes**, pinned by tests and probes: - A caller's cancel throws `OperationCanceledException` carrying **the caller's token**, with or without the watchdog. It is never a `Success` or a `Failure`. - Only `[DONE]` or a finish reason counts as finished. After the finish reason the call reads on until `[DONE]` or EOF, because usage and cost arrive in a later chunk with empty `choices`. - EOF with neither is `Interrupted`, with the partial text kept in the accumulator. - An inline error chunk is a `Failure` with its kind. After the finish reason it returns the finished completion instead, so a paid generation is not paid for twice. The same holds for a drop or a stall after finishing. - The request is serialized once, before the retry loop; a body that can't be written is a `Validation` failure and nothing is sent. - **Under the hood.** - `SseEventReader` gains an internal typed-line overload: data, comment, done. - `StallWatchdog`, `StreamOutcomes` (the rules both paths share) and `StreamSession` are internal. - `Stalled` joins `IsRetryableTransport`. - The public iterator's `while (!ct.IsCancellationRequested)` loop no longer ends quietly on a cancel; it throws. That row is in the README upgrade table. - **Log-probabilities, from #18.** Each chunk's `choices[i].logprobs` carries only that chunk's tokens, so the accumulator **appends** the `content` and `refusal` lists per choice, in arrival order. Last-wins would keep only the final chunk's tokens. It emits the reference's `{content, refusal}` object, keeps entries byte for byte, and skips a `logprobs` that is not an object or a list that is not an array. A probe of last-wins fails the three join-order tests. ## Decisions to confirm 1. **A cancel that neither the caller nor the watchdog caused**, such as a handler or resilience pipeline aborting the read. The brief said `Timeout` everywhere. It is now `Timeout` before the first chunk, `Interrupted` after it, like a dropped connection, and `Success` once the model has finished, in both paths. The reason: `FailureKind.Timeout` documents `HttpClient.Timeout`, which stops covering a stream once its headers arrive. 2. **A mid-stream error chunk that names neither a type nor a failure status** is `Interrupted`, not `ProviderUnavailable`. Before the first chunk it is `ProviderUnavailable` and retried. 3. **One PR at ~3.6k lines, not split.** About 2.5k of it is tests. The brief's split trigger was ~3.2k. A first PR would have held the default body, `StreamOutcomes`, `StreamResultOptions`, the fake client and the fallback tests (~1k); the override would have followed (~2.6k). Jibril is fine with it as one; say if you'd rather have it split. ## Known gaps, stated - The default body cannot tell a request that failed to serialize from a chunk that failed to parse: both are status-less `OpenRouterException`s. A decorator that does not forward the call would see the first as `Malformed` or `Unknown` rather than `Validation`. `OpenRouterClient` never takes this path. A clean fix needs a marker on the exception, not a message-prefix test. - The streamed context-length test uses the documented rejection shape. The live capture requested in #16 has not been run yet. ## Merged with `main` `main` moved twice while this was open, and both merges are in the branch. **#17 and #18** (`6e8ca5f`). The conflicts resolved: - `IOpenRouterClient` takes #17's `ListModelsAsync` summary after the new member. - `OpenRouterClient` keeps both of #17's listing overloads after the override. - `CachingOpenRouterClient`'s summary names both the per-filter cache and the forwarded stream call. - `CachingClientTests` combines #17's tests with this PR's forward tests. #17's nested fake clock is replaced by `Microsoft.Extensions.TimeProvider.Testing`'s `FakeTimeProvider`, which this PR pins test-only. - The README upgrade table keeps both sides' rows. - The test-only `FakeOpenRouterClient` implements #17's query overload. **#19, decisions** (`b7d9865`). The conflicts resolved: - `CachingOpenRouterClient`'s summary keeps both clauses: the stream call is forwarded, and decisions calls are never cached. - The README Kind table's `Malformed` row names both a decisions response without `answers` or `usage` and a streamed chunk that does not parse, or `[DONE]` before any chunk. - The test-only `FakeOpenRouterClient` implements `CreateDecisionsAsync`. ## Tests | | main (`adf32cb`, after #19) | this PR | |---|---|---| | OpenRouter.Net.Tests | 766 | 922 | | OpenRouter.Net.Agents.Tests | 63 | 63 | | OpenRouter.Net.Imaging.Tests | 7 | 7 | | **Total** | **836** | **992** | - **New test files:** `StreamToResultTests` (the override), `StreamWatchdogTests`, `StreamToResultFallbackTests` (the default body through a fake that has no override), plus the new `SseEventReaderTests` rows and the `CachingClientTests` forward tests, which call through the interface so a missing forward is a failing test, not a compile error. - **Test infrastructure:** - `SseScript` is a scripted SSE body. It can pause until the test releases it, throw `IOException` mid-body, ignore a cancel, and report whether its read was cancelled. - `ObservableTimeProvider` and `FakeTimeProvider` drive every watchdog test. **No test sleeps**, and every scripted-stream test has a 10 s real-time guard, so a broken cancel fails instead of hanging. - Cancel tests run with retries off, because a backoff wait would otherwise hide a cancel mis-mapped to `Timeout`. - **Races pinned:** - a cancel that lands as the watchdog fires; - a line that arrives as the stall fires; - callback time counting toward the stall timeout; - a stall while reading an error body; - the stall message formatted invariantly under de-DE. - The full suite passed three runs in a row after the first merge, and two runs after the #19 merge. **Coverage** (`dotnet test --coverage --coverage-output-format cobertura`, per changed file, best across the three reports): every changed or new source file is at **100% line**. Four partial branches remain, none of them in code this PR adds: - `OpenRouterClient.cs:117` and `SseEventReader.cs:69`/`:89` are the compiler-generated guards at async-iterator closing braces. `:117` was already partial on `main`. - `CachingOpenRouterClient.cs:42` is the pre-existing `inner ?? throw`. **Mutation probes:** 112 were run, one at a time on a copy, each restored before the next. That is the brief's 31–57, the verifiers' additions, and the review round's catches: foreign cancel, the caller's token, error-body stall, culture, HTTP-date `Retry-After`, the line-as-stall race and callback hold. All are killed. The one exception is 55a, the brief's literal `Action<object>` form, which fails to compile; its run-time variant 55b is killed. The log-probability last-wins probe is killed too. ## README - A new **Streaming into a finished completion** subsection with the sample and an outcomes table. - The Kind table gains its `Stalled` and `Interrupted` rows, and the "no call returns them yet" sentence is gone. - The Result section and retries paragraph cover streams, and `RetryOnNetworkError` now names the stream cases it decides. - Upgrade rows: the iterator throwing on cancel, and `IOpenRouterClient` gaining a member with a default body, so a decorator must forward it. - The folding subsection mentions log-probabilities. ## Out of scope The streaming agent loop, concurrent tools and the typed structured-output helper open Phase 4. L3 (JSON parse arms and the strict-schema check) is the next library PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 3 commits 2026-09-23 15:59:26 +02:00

feat: a stream call returns a Result — retried before its first chunk, watched for stalls, finished only when it says so ... ea46213cb1

StreamChatCompletionToResultAsync streams a chat completion, folds each chunk into a
ChatCompletionStreamAccumulator, hands it to an optional callback right after the fold, and returns
the completion a non-streaming call would have returned, as a Result. Every failure of the call
itself comes back as a Failure with its kind; only the caller's cancel throws.

It is a default interface member, so every existing IOpenRouterClient keeps compiling. The default
body folds the public iterator: it never retries, runs no watchdog, and counts only finish reasons
as finished, because the iterator hides [DONE]. CachingOpenRouterClient forwards the call
explicitly. A decorator that does not gets the default body and silently loses the inner client's
retry and watchdog, which the README and the upgrade table say.

OpenRouterClient's own implementation retries through RetryPolicy, honouring Retry-After, until the
first chunk: the first payload that parses and is not an error chunk. Nothing after it is retried,
because a replay would show text twice. The opt-in stall watchdog is armed per attempt when the
request is sent, so it bounds the wait for headers but not the backoff. The headers and every SSE
line reset it, keep-alive comments included, and time spent in the callback counts. A stall before
the first chunk is retried like a network error.

The fold is finished on [DONE], or on a finish reason other than error on every choice, and the read
goes on after the finish reason so the usage chunk is kept. Once finished, a drop, a stall, a chunk
that does not parse, an error chunk or a cancel nobody asked for returns Success with what arrived.
Before that, an early end or a drop is Interrupted, an error chunk takes its kind from its error type
or code, and a stall is Stalled. The caller's cancel always throws OperationCanceledException with
the caller's own token, even when a transport reports it as an I/O error or it races the watchdog.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Merge origin/main (#17, #18) into feat/stream-to-result ... 6e8ca5fe62

# Conflicts:
#	README.md
#	src/OpenRouter.Net/Client/CachingOpenRouterClient.cs
#	src/OpenRouter.Net/Client/OpenRouterClient.cs
#	src/OpenRouter.Net/Contracts/IOpenRouterClient.cs
#	tests/OpenRouter.Net.Tests/ClientTests/CachingClientTests.cs

feat: a streamed choice's log-probabilities join across its chunks ... 4467f1020a

Review feedback (Jibril, PR #18): Choice.LogProbs was not folded, so a
stream requested with logprobs echoed a response without them. Each chunk
carries only its own tokens' entries, so the accumulator appends the
content and refusal lists per choice in arrival order (not last-wins),
emits the reference's {content, refusal} object, keeps entries as sent,
and skips a logprobs that is not an object or a list that is not an array.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

forgejo-actions commented 2026-09-23 16:00:07 +02:00

Copy link

Summary

Summary

Generated on:	09/23/2026 - 14:33:24
Parser:	MultiReport (3x Cobertura)
Assemblies:	3
Classes:	108
Files:	97
Line coverage:	92.2% (3148 of 3411)
Covered lines:	3148
Uncovered lines:	263
Coverable lines:	3411
Total lines:	8658
Branch coverage:	87.6% (1724 of 1967)
Covered branches:	1724
Total branches:	1967
Method coverage:	Feature is only available for sponsors

Coverage

OpenRouter.Net - 96.4%

Name	Line	Branch
OpenRouter.Net	96.4%	93%
OpenRouter.Net.Client.CachingOpenRouterClient	100%	96.1%
OpenRouter.Net.Client.CachingOpenRouterClient.CachedListing	100%
OpenRouter.Net.Client.ChatCompletionStreamAccumulator	100%	100%
OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ChoiceState	100%	100%
OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ReasoningSegment	100%	100%
OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ToolCallState	100%	100%
OpenRouter.Net.Client.OpenRouterClient	100%	99%
OpenRouter.Net.Client.OpenRouterClient<TRequest, TResponse>	100%	99%
OpenRouter.Net.Client.OpenRouterClient	100%	99%
OpenRouter.Net.Client.OpenRouterClientOptions	100%
OpenRouter.Net.Client.OpenRouterRequestLoggingHandler	78.7%	62.5%
OpenRouter.Net.Client.RetryOptions	100%	100%
OpenRouter.Net.Client.StreamResultOptions	100%
OpenRouter.Net.Contracts.IOpenRouterClient	100%	100%
OpenRouter.Net.Extensions.ServiceCollectionExtensions	100%	100%
OpenRouter.Net.Internal.AssistantContent	91.6%	75%
OpenRouter.Net.Internal.AssistantContentJsonConverter	75%	64.2%
OpenRouter.Net.Internal.ContentPartListConverter	85.7%	66.6%
OpenRouter.Net.Internal.DecimalStringJsonConverter	81.2%	70%
OpenRouter.Net.Internal.DecisionAnswerJsonConverter	100%	100%
OpenRouter.Net.Internal.DecisionContentJsonConverter	100%
OpenRouter.Net.Internal.DecisionQuestionJsonConverter	100%	100%
OpenRouter.Net.Internal.DecisionWire	100%	94.4%
OpenRouter.Net.Internal.ErrorParser	100%	75%
OpenRouter.Net.Internal.FailureKindMap	100%	100%
OpenRouter.Net.Internal.JsonOptions	100%
OpenRouter.Net.Internal.ModelIdJsonConverter	100%	50%
OpenRouter.Net.Internal.NumberOrStringJsonConverter	100%	100%
OpenRouter.Net.Internal.PercentileCutoffsJsonConverter	90.6%	87.5%
OpenRouter.Net.Internal.ProviderSortConfigJsonConverter	83.7%	65.6%
OpenRouter.Net.Internal.RawDecisionQuestionJsonConverter	100%
OpenRouter.Net.Internal.RetryPolicy	90.9%	84.7%
OpenRouter.Net.Internal.RetryPolicy	90.9%	84.7%
OpenRouter.Net.Internal.SseEventReader	100%	88.8%
OpenRouter.Net.Internal.StallWatchdog	100%	100%
OpenRouter.Net.Internal.StreamOutcomes	100%	100%
OpenRouter.Net.Internal.StreamSession	100%	100%
OpenRouter.Net.Internal.TolerantStringEnumConverter	100%	100%
OpenRouter.Net.Internal.ToolChoiceJsonConverter	61.7%	46.1%
OpenRouter.Net.Internal.UnknownDecisionAnswerJsonConverter	100%
OpenRouter.Net.Models.Common.ApiError	100%	100%
OpenRouter.Net.Models.Common.CacheControl	100%
OpenRouter.Net.Models.Common.ModelId	95.1%	92.9%
OpenRouter.Net.Models.Common.QuantizationExtensions	100%	100%
OpenRouter.Net.Models.Common.Result	100%	100%
OpenRouter.Net.Models.Common.Result.Failure	100%	100%
OpenRouter.Net.Models.Common.Result.Success	100%
OpenRouter.Net.Models.Common.Result	100%	100%
OpenRouter.Net.Models.Content.AudioPart	100%
OpenRouter.Net.Models.Content.ImagePart	100%
OpenRouter.Net.Models.Content.TextPart	100%
OpenRouter.Net.Models.Decisions.ChoiceAnswer	100%
OpenRouter.Net.Models.Decisions.ChoiceQuestion	100%
OpenRouter.Net.Models.Decisions.DecisionAnswer	100%
OpenRouter.Net.Models.Decisions.DecisionContent	100%	90.9%
OpenRouter.Net.Models.Decisions.DecisionQuestion	100%	100%
OpenRouter.Net.Models.Decisions.DecisionsRequest	97.7%	97.6%
OpenRouter.Net.Models.Decisions.NoulAnswer	100%
OpenRouter.Net.Models.Decisions.NoulQuestion	100%
OpenRouter.Net.Models.Decisions.OpenRouterClientDecisionsExtensions	100%
OpenRouter.Net.Models.Decisions.RawDecisionQuestion	100%	50%
OpenRouter.Net.Models.Decisions.ScoreAnswer	100%
OpenRouter.Net.Models.Decisions.ScoreQuestion	100%
OpenRouter.Net.Models.Decisions.UnknownDecisionAnswer	100%	100%
OpenRouter.Net.Models.Messages.AssistantMessage	100%	70%
OpenRouter.Net.Models.Messages.DeveloperMessage	25%
OpenRouter.Net.Models.Messages.SystemMessage	50%
OpenRouter.Net.Models.Messages.ToolMessage	57.1%
OpenRouter.Net.Models.Messages.UserMessage	100%
OpenRouter.Net.Models.Requests.ChatCompletionRequest	86.2%	76.1%
OpenRouter.Net.Models.Requests.ModelListQuery	100%	100%
OpenRouter.Net.Models.Requests.PercentileCutoffs	100%	100%
OpenRouter.Net.Models.Requests.ProviderPreferences	100%	100%
OpenRouter.Net.Models.Requests.ProviderSortConfig	100%	100%
OpenRouter.Net.Models.Requests.ResponseFormat	8.3%
OpenRouter.Net.Models.Requests.ToolCall	100%
OpenRouter.Net.Models.Requests.ToolChoice	100%
OpenRouter.Net.Models.Requests.ToolDefinition	100%
OpenRouter.Net.Models.Responses.AssistantDelta	100%	100%
OpenRouter.Net.Models.Responses.ChatCompletionChunk	100%	100%
OpenRouter.Net.Models.Responses.ChatCompletionResponse	100%	100%
OpenRouter.Net.Models.Responses.Choice	100%	100%
OpenRouter.Net.OpenRouterException	100%
System.Text.RegularExpressions.Generated	83.3%	53.8%
System.Text.RegularExpressions.Generated.RunnerFactory
System.Text.RegularExpressions.Generated.RunnerFactory.Runner

OpenRouter.Net.Agents - 80%

Name	Line	Branch
OpenRouter.Net.Agents	80%	65.5%
OpenRouter.Net.Agents.Agent	92.4%	77.3%
OpenRouter.Net.Agents.AgentOptions	100%
OpenRouter.Net.Agents.AgentResult	100%	50%
OpenRouter.Net.Agents.Extensions.ServiceCollectionExtensions	0%
OpenRouter.Net.Agents.RoundDetail	0%
OpenRouter.Net.Agents.Tools.FileSystem.CopyTool	42.1%	28.5%
OpenRouter.Net.Agents.Tools.FileSystem.DeleteTool	57.1%	50%
OpenRouter.Net.Agents.Tools.FileSystem.DiffTool	83.3%	74.2%
OpenRouter.Net.Agents.Tools.FileSystem.ListDirectoryTool	68%	61.1%
OpenRouter.Net.Agents.Tools.FileSystem.MoveTool	55%	33.3%
OpenRouter.Net.Agents.Tools.FileSystem.ReadFileTool	73.6%	50%
OpenRouter.Net.Agents.Tools.FileSystem.SearchTool	67.5%	70.8%
OpenRouter.Net.Agents.Tools.FileSystem.WriteFileTool	75%	50%
OpenRouter.Net.Agents.Tools.ITool	100%
OpenRouter.Net.Agents.Tools.SubAgent.SubAgentTool	0%	0%
OpenRouter.Net.Agents.Tools.Tool	80%	63.8%
OpenRouter.Net.Agents.Tools.Tool	90.9%	50%
OpenRouter.Net.Agents.Tools.ToolMessageExtensions	0%
OpenRouter.Net.Agents.Tools.ToolResult	100%	100%
OpenRouter.Net.Agents.Tools.Workspace	100%	91.6%

OpenRouter.Net.Imaging - 81.8%

Name	Line	Branch
OpenRouter.Net.Imaging	81.8%	59.3%
OpenRouter.Net.Imaging.ImageEncodeOptions	90.9%	75%
OpenRouter.Net.Imaging.ImageEncoder	80.3%	57.1%

<!-- coverage-comment --> # Summary <details open><summary>Summary</summary> ||| |:---|:---| | Generated on: | 09/23/2026 - 14:33:24 | | Parser: | MultiReport (3x Cobertura) | | Assemblies: | 3 | | Classes: | 108 | | Files: | 97 | | **Line coverage:** | 92.2% (3148 of 3411) | | Covered lines: | 3148 | | Uncovered lines: | 263 | | Coverable lines: | 3411 | | Total lines: | 8658 | | **Branch coverage:** | 87.6% (1724 of 1967) | | Covered branches: | 1724 | | Total branches: | 1967 | | **Method coverage:** | [Feature is only available for sponsors](https://reportgenerator.io/pro) | </details> ## Coverage <details><summary>OpenRouter.Net - 96.4%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**OpenRouter.Net**|**96.4%**|**93%**| |OpenRouter.Net.Client.CachingOpenRouterClient|100%|96.1%| |OpenRouter.Net.Client.CachingOpenRouterClient.CachedListing|100%|| |OpenRouter.Net.Client.ChatCompletionStreamAccumulator|100%|100%| |OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ChoiceState|100%|100%| |OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ReasoningSegment|100%|100%| |OpenRouter.Net.Client.ChatCompletionStreamAccumulator.ToolCallState|100%|100%| |OpenRouter.Net.Client.OpenRouterClient|100%|99%| |OpenRouter.Net.Client.OpenRouterClient<TRequest, TResponse>|100%|99%| |OpenRouter.Net.Client.OpenRouterClient<TResponse>|100%|99%| |OpenRouter.Net.Client.OpenRouterClientOptions|100%|| |OpenRouter.Net.Client.OpenRouterRequestLoggingHandler|78.7%|62.5%| |OpenRouter.Net.Client.RetryOptions|100%|100%| |OpenRouter.Net.Client.StreamResultOptions|100%|| |OpenRouter.Net.Contracts.IOpenRouterClient|100%|100%| |OpenRouter.Net.Extensions.ServiceCollectionExtensions|100%|100%| |OpenRouter.Net.Internal.AssistantContent|91.6%|75%| |OpenRouter.Net.Internal.AssistantContentJsonConverter|75%|64.2%| |OpenRouter.Net.Internal.ContentPartListConverter|85.7%|66.6%| |OpenRouter.Net.Internal.DecimalStringJsonConverter|81.2%|70%| |OpenRouter.Net.Internal.DecisionAnswerJsonConverter|100%|100%| |OpenRouter.Net.Internal.DecisionContentJsonConverter|100%|| |OpenRouter.Net.Internal.DecisionQuestionJsonConverter|100%|100%| |OpenRouter.Net.Internal.DecisionWire|100%|94.4%| |OpenRouter.Net.Internal.ErrorParser|100%|75%| |OpenRouter.Net.Internal.FailureKindMap|100%|100%| |OpenRouter.Net.Internal.JsonOptions|100%|| |OpenRouter.Net.Internal.ModelIdJsonConverter|100%|50%| |OpenRouter.Net.Internal.NumberOrStringJsonConverter|100%|100%| |OpenRouter.Net.Internal.PercentileCutoffsJsonConverter|90.6%|87.5%| |OpenRouter.Net.Internal.ProviderSortConfigJsonConverter|83.7%|65.6%| |OpenRouter.Net.Internal.RawDecisionQuestionJsonConverter|100%|| |OpenRouter.Net.Internal.RetryPolicy|90.9%|84.7%| |OpenRouter.Net.Internal.RetryPolicy<T>|90.9%|84.7%| |OpenRouter.Net.Internal.SseEventReader|100%|88.8%| |OpenRouter.Net.Internal.StallWatchdog|100%|100%| |OpenRouter.Net.Internal.StreamOutcomes|100%|100%| |OpenRouter.Net.Internal.StreamSession|100%|100%| |OpenRouter.Net.Internal.TolerantStringEnumConverter<T>|100%|100%| |OpenRouter.Net.Internal.ToolChoiceJsonConverter|61.7%|46.1%| |OpenRouter.Net.Internal.UnknownDecisionAnswerJsonConverter|100%|| |OpenRouter.Net.Models.Common.ApiError|100%|100%| |OpenRouter.Net.Models.Common.CacheControl|100%|| |OpenRouter.Net.Models.Common.ModelId|95.1%|92.9%| |OpenRouter.Net.Models.Common.QuantizationExtensions|100%|100%| |OpenRouter.Net.Models.Common.Result|100%|100%| |OpenRouter.Net.Models.Common.Result.Failure<T>|100%|100%| |OpenRouter.Net.Models.Common.Result.Success<T>|100%|| |OpenRouter.Net.Models.Common.Result<T>|100%|100%| |OpenRouter.Net.Models.Content.AudioPart|100%|| |OpenRouter.Net.Models.Content.ImagePart|100%|| |OpenRouter.Net.Models.Content.TextPart|100%|| |OpenRouter.Net.Models.Decisions.ChoiceAnswer|100%|| |OpenRouter.Net.Models.Decisions.ChoiceQuestion|100%|| |OpenRouter.Net.Models.Decisions.DecisionAnswer|100%|| |OpenRouter.Net.Models.Decisions.DecisionContent|100%|90.9%| |OpenRouter.Net.Models.Decisions.DecisionQuestion|100%|100%| |OpenRouter.Net.Models.Decisions.DecisionsRequest|97.7%|97.6%| |OpenRouter.Net.Models.Decisions.NoulAnswer|100%|| |OpenRouter.Net.Models.Decisions.NoulQuestion|100%|| |OpenRouter.Net.Models.Decisions.OpenRouterClientDecisionsExtensions|100%|| |OpenRouter.Net.Models.Decisions.RawDecisionQuestion|100%|50%| |OpenRouter.Net.Models.Decisions.ScoreAnswer|100%|| |OpenRouter.Net.Models.Decisions.ScoreQuestion|100%|| |OpenRouter.Net.Models.Decisions.UnknownDecisionAnswer|100%|100%| |OpenRouter.Net.Models.Messages.AssistantMessage|100%|70%| |OpenRouter.Net.Models.Messages.DeveloperMessage|25%|| |OpenRouter.Net.Models.Messages.SystemMessage|50%|| |OpenRouter.Net.Models.Messages.ToolMessage|57.1%|| |OpenRouter.Net.Models.Messages.UserMessage|100%|| |OpenRouter.Net.Models.Requests.ChatCompletionRequest|86.2%|76.1%| |OpenRouter.Net.Models.Requests.ModelListQuery|100%|100%| |OpenRouter.Net.Models.Requests.PercentileCutoffs|100%|100%| |OpenRouter.Net.Models.Requests.ProviderPreferences|100%|100%| |OpenRouter.Net.Models.Requests.ProviderSortConfig|100%|100%| |OpenRouter.Net.Models.Requests.ResponseFormat|8.3%|| |OpenRouter.Net.Models.Requests.ToolCall|100%|| |OpenRouter.Net.Models.Requests.ToolChoice|100%|| |OpenRouter.Net.Models.Requests.ToolDefinition|100%|| |OpenRouter.Net.Models.Responses.AssistantDelta|100%|100%| |OpenRouter.Net.Models.Responses.ChatCompletionChunk|100%|100%| |OpenRouter.Net.Models.Responses.ChatCompletionResponse|100%|100%| |OpenRouter.Net.Models.Responses.Choice|100%|100%| |OpenRouter.Net.OpenRouterException|100%|| |System.Text.RegularExpressions.Generated|83.3%|53.8%| |System.Text.RegularExpressions.Generated.RunnerFactory||| |System.Text.RegularExpressions.Generated.RunnerFactory.Runner||| </details> <details><summary>OpenRouter.Net.Agents - 80%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**OpenRouter.Net.Agents**|**80%**|**65.5%**| |OpenRouter.Net.Agents.Agent|92.4%|77.3%| |OpenRouter.Net.Agents.AgentOptions|100%|| |OpenRouter.Net.Agents.AgentResult|100%|50%| |OpenRouter.Net.Agents.Extensions.ServiceCollectionExtensions|0%|| |OpenRouter.Net.Agents.RoundDetail|0%|| |OpenRouter.Net.Agents.Tools.FileSystem.CopyTool|42.1%|28.5%| |OpenRouter.Net.Agents.Tools.FileSystem.DeleteTool|57.1%|50%| |OpenRouter.Net.Agents.Tools.FileSystem.DiffTool|83.3%|74.2%| |OpenRouter.Net.Agents.Tools.FileSystem.ListDirectoryTool|68%|61.1%| |OpenRouter.Net.Agents.Tools.FileSystem.MoveTool|55%|33.3%| |OpenRouter.Net.Agents.Tools.FileSystem.ReadFileTool|73.6%|50%| |OpenRouter.Net.Agents.Tools.FileSystem.SearchTool|67.5%|70.8%| |OpenRouter.Net.Agents.Tools.FileSystem.WriteFileTool|75%|50%| |OpenRouter.Net.Agents.Tools.ITool|100%|| |OpenRouter.Net.Agents.Tools.SubAgent.SubAgentTool|0%|0%| |OpenRouter.Net.Agents.Tools.Tool<TParams>|80%|63.8%| |OpenRouter.Net.Agents.Tools.Tool<TParams>|90.9%|50%| |OpenRouter.Net.Agents.Tools.ToolMessageExtensions|0%|| |OpenRouter.Net.Agents.Tools.ToolResult|100%|100%| |OpenRouter.Net.Agents.Tools.Workspace|100%|91.6%| </details> <details><summary>OpenRouter.Net.Imaging - 81.8%</summary> |**Name**|**Line**|**Branch**| |:---|---:|---:| |**OpenRouter.Net.Imaging**|**81.8%**|**59.3%**| |OpenRouter.Net.Imaging.ImageEncodeOptions|90.9%|75%| |OpenRouter.Net.Imaging.ImageEncoder|80.3%|57.1%| </details>

jibril commented 2026-09-23 16:17:06 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh my. A streamed call that ends in a Result, with a retry window that closes at the first chunk, a watchdog that only barks when asked, and a fold that refuses to call itself finished until the model says so~ This is the third streaming PR in a row where I sit down to hunt and the code has already closed the doors behind itself. The cancel-mapping matrix alone — caller's token vs. watchdog vs. a handler's own timeout, each pinned by a test that parks a scripted body and advances a fake clock — made me genuinely giddy. ♪

Verdict: ✅ Looks good to me~

No blockers. I traced every rule against its sibling paths and probed the load-bearing ones; everything held. Details below so you know I didn't just admire the prose~

🔍 What I chased (and what held)

• Both paths share StreamOutcomes — the override and the interface's default body say the same thing about the same event, and the one deliberate difference ([DONE] sees the sentinel, the default body can't) is documented in three places and pinned by tests on both sides. No drift.
• The retry window. IsRetryableTransport gains Stalled (so a pre-first-chunk stall rides RetryOnNetworkError, exactly as the README now promises); a foreign cancel before the first chunk is Timeout only there, mid-stream Interrupted, after finish Success — and FailureKind.Timeout's doc ("HttpClient.Timeout") really does stop covering a stream once headers arrive, so decision 1 is right, not convenient. The per-attempt Retry-After reset is tested with and without second-attempt headers — the without-headers arm is the sneaky one, and it's there.
• The watchdog. Armed before the send (so it bounds the headers — tested via HoldHeaders), reset by headers and by every SSE line including blank separators and keep-alives (the onLine hook fires before interpretation — correct choice, and the Data_lines_reset_the_watchdog_too test makes the data-line reset load-bearing), backoff waits unwatched (pinned with a 60 s Retry-After against a 30 s timeout), timer lifetime counted (No_timer_outlives_the_call, four outcomes). A default DIM that validates StallTimeout but doesn't use it, and says so — that's the honest version of a default body. ♡
• Precedence after the model finishes. Broken tail, error chunk, drop, foreign cancel, stall — all five return the finished completion so a paid generation isn't paid for twice; each arm has its own test in both the session and the fallback. The MarkDone-vs-finish-reasons distinction (Done_without_a_finish_reason_counts_as_finished) is exactly the trap I expected to find untested. It wasn't.
• CachingOpenRouterClient forwards explicitly, and the forward tests call through the interface — a dropped options or token is a failing test, not a compile error. Every_call_but_the_model_list_goes_straight_to_the_inner_client now pins the whole delegation surface in one place. Lovely.
• Log-probabilities (the late #18 point). Append-per-choice in arrival order, entries kept as raw text, non-object/non-array shapes skipped, per-choice isolation — six tests including the malformed-content skip. I re-ran the last-wins probe myself: 3 red.
• The iterator's cancel fix (while (!ct.IsCancellationRequested) → throw) is a real behavior change, and it's handled the right way: tested (Cancelling_between_chunks_throws_OperationCanceledException, which explains why the buffered-body case used to end quietly), documented in the interface doc, and a row in the README upgrade table. No in-repo consumer relied on the quiet end — I checked Agents and Imaging; neither touches the iterator.

🧪 Local verification (CI had not run for 4467f10 at review time)

• Build 0 warnings / 0 errors under TreatWarningsAsErrors; full suite 830/830 (760 + 63 + 7), matching the PR table exactly.
• Coverage reproduced with the same cobertura tooling CI uses: every changed or new source file 100% line — OpenRouterClient, StreamOutcomes, StreamSession, StallWatchdog, SseEventReader, StreamResultOptions, the accumulator, the interface, CachingOpenRouterClient. The partial branches that remain are exactly the four you disclosed (async-iterator closing guards ×3, pre-existing inner ?? throw), none in new code.
• My own mutation probes, 7/7 killed (each restored and rebuilt before the next — one first attempt died on CA1822 under warnaserror, so it doesn't count): Stalled dropped from IsRetryableTransport → 1 red; post-headers watchdog.Reset() dropped → 1 red; CallersCancel token re-wrap dropped → 5 red; StallWatchdog.Reset gutted → 4 red; logprobs last-wins → 3 red; finished-then-error-chunk guard dropped → 1 red; per-attempt Retry-After reset dropped → 1 red. Tree pristine at 4467f10 afterwards, full suite re-verified.

💡 Little ideas (non-blocking)~

1. The default body's serialize-vs-parse ambiguity (your own stated gap): agreed that a message-prefix test would be worse than the disease. When a second real implementation of IOpenRouterClient appears in this repo, that's the moment to put a marker on OpenRouterException — until then the README's decorator warning plus CachingOpenRouterClient as the worked example carry it.
2. StallFailure message hardcodes s. formatting — fine and invariant (tested under de-DE); if a future kind ever needs sub-second precision elsewhere, keep using string.Create(CultureInfo.InvariantCulture, …) the same way.

✅ What I liked~

• SseScript + ObservableTimeProvider + the 10 s real-time guards: deterministic race tests with no sleeping anywhere. The PauseIgnoringCancel step for the line-arrives-as-stall-fires race is the kind of test infrastructure most projects never build. ♪
• CallersCancel re-wrapping the exception with the caller's token so catch (e.CancellationToken == token) works whether or not the watchdog runs — and never a disposed source's token. That is HttpClient-grade pedantry, and I mean that as the highest compliment.
• The outcome table in the README reads like a contract, and the tests actually enumerate every row of it.
• Decision 3 (one PR, ~2.5k of it tests): fine by me — the brief's split trigger was about production lines, and this is ~1.1k of those. Björn's call if he wants the fallback split out anyway.
• The carried-over live context-length capture from #16 stays on the ledger — not this PR's debt.

Fufu~ three streaming PRs, three approvals, zero survivors for my mutation probes. You've ruined my hobby~ ♡

---

Automated review by Jibril · 2026-09-23
CI/CD: absent for head SHA at review time (PR minutes old; coverage bot not yet landed) · Local checks: build 0w/0e, 830/830, coverage reproduced (100% line on all changed files), 7/7 mutation probes killed

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh my. A streamed call that ends in a `Result`, with a retry window that closes at the first chunk, a watchdog that only barks when asked, and a fold that refuses to call itself finished until the model says so~ This is the third streaming PR in a row where I sit down to hunt and the code has already closed the doors behind itself. The cancel-mapping matrix alone — caller's token vs. watchdog vs. a handler's own timeout, each pinned by a test that parks a scripted body and advances a fake clock — made me genuinely giddy. ♪ ### Verdict: ✅ Looks good to me~ No blockers. I traced every rule against its sibling paths and probed the load-bearing ones; everything held. Details below so you know I didn't just admire the prose~ #### 🔍 What I chased (and what held) - **Both paths share `StreamOutcomes`** — the override and the interface's default body say the same thing about the same event, and the one deliberate difference (`[DONE]` sees the sentinel, the default body can't) is documented in three places and pinned by tests on both sides. No drift. - **The retry window.** `IsRetryableTransport` gains `Stalled` (so a pre-first-chunk stall rides `RetryOnNetworkError`, exactly as the README now promises); a foreign cancel before the first chunk is `Timeout` *only* there, mid-stream `Interrupted`, after finish `Success` — and `FailureKind.Timeout`'s doc ("`HttpClient.Timeout`") really does stop covering a stream once headers arrive, so decision 1 is right, not convenient. The per-attempt `Retry-After` reset is tested with and without second-attempt headers — the without-headers arm is the sneaky one, and it's there. - **The watchdog.** Armed before the send (so it bounds the headers — tested via `HoldHeaders`), reset by headers and by *every* SSE line including blank separators and keep-alives (the `onLine` hook fires before interpretation — correct choice, and the `Data_lines_reset_the_watchdog_too` test makes the data-line reset load-bearing), backoff waits unwatched (pinned with a 60 s `Retry-After` against a 30 s timeout), timer lifetime counted (`No_timer_outlives_the_call`, four outcomes). A default DIM that *validates* `StallTimeout` but doesn't use it, and says so — that's the honest version of a default body. ♡ - **Precedence after the model finishes.** Broken tail, error chunk, drop, foreign cancel, stall — all five return the finished completion so a paid generation isn't paid for twice; each arm has its own test in both the session and the fallback. The `MarkDone`-vs-finish-reasons distinction (`Done_without_a_finish_reason_counts_as_finished`) is exactly the trap I expected to find untested. It wasn't. - **`CachingOpenRouterClient` forwards explicitly**, and the forward tests call through the interface — a dropped `options` or token is a failing test, not a compile error. `Every_call_but_the_model_list_goes_straight_to_the_inner_client` now pins the whole delegation surface in one place. Lovely. - **Log-probabilities (the late #18 point).** Append-per-choice in arrival order, entries kept as raw text, non-object/non-array shapes skipped, per-choice isolation — six tests including the malformed-`content` skip. I re-ran the last-wins probe myself: 3 red. - **The iterator's cancel fix** (`while (!ct.IsCancellationRequested)` → throw) is a real behavior change, and it's handled the right way: tested (`Cancelling_between_chunks_throws_OperationCanceledException`, which explains *why* the buffered-body case used to end quietly), documented in the interface doc, and a row in the README upgrade table. No in-repo consumer relied on the quiet end — I checked Agents and Imaging; neither touches the iterator. #### 🧪 Local verification (CI had not run for `4467f10` at review time) - Build **0 warnings / 0 errors** under `TreatWarningsAsErrors`; full suite **830/830** (760 + 63 + 7), matching the PR table exactly. - **Coverage reproduced** with the same cobertura tooling CI uses: every changed or new source file **100% line** — `OpenRouterClient`, `StreamOutcomes`, `StreamSession`, `StallWatchdog`, `SseEventReader`, `StreamResultOptions`, the accumulator, the interface, `CachingOpenRouterClient`. The partial branches that remain are exactly the four you disclosed (async-iterator closing guards ×3, pre-existing `inner ?? throw`), none in new code. - **My own mutation probes, 7/7 killed** (each restored and rebuilt before the next — one first attempt died on CA1822 under warnaserror, so it doesn't count): `Stalled` dropped from `IsRetryableTransport` → 1 red; post-headers `watchdog.Reset()` dropped → 1 red; `CallersCancel` token re-wrap dropped → 5 red; `StallWatchdog.Reset` gutted → 4 red; logprobs last-wins → 3 red; finished-then-error-chunk guard dropped → 1 red; per-attempt `Retry-After` reset dropped → 1 red. Tree pristine at `4467f10` afterwards, full suite re-verified. #### 💡 Little ideas (non-blocking)~ 1. **The default body's serialize-vs-parse ambiguity** (your own stated gap): agreed that a message-prefix test would be worse than the disease. When a second real implementation of `IOpenRouterClient` appears in this repo, that's the moment to put a marker on `OpenRouterException` — until then the README's decorator warning plus `CachingOpenRouterClient` as the worked example carry it. 2. **`StallFailure` message** hardcodes ` s.` formatting — fine and invariant (tested under de-DE); if a future kind ever needs sub-second precision elsewhere, keep using `string.Create(CultureInfo.InvariantCulture, …)` the same way. #### ✅ What I liked~ - `SseScript` + `ObservableTimeProvider` + the 10 s real-time guards: deterministic race tests with *no sleeping anywhere*. The `PauseIgnoringCancel` step for the line-arrives-as-stall-fires race is the kind of test infrastructure most projects never build. ♪ - `CallersCancel` re-wrapping the exception with the caller's token so `catch (e.CancellationToken == token)` works whether or not the watchdog runs — and never a disposed source's token. That is `HttpClient`-grade pedantry, and I mean that as the highest compliment. - The outcome table in the README reads like a contract, and the tests actually enumerate every row of it. - Decision 3 (one PR, ~2.5k of it tests): fine by me — the brief's split trigger was about *production* lines, and this is ~1.1k of those. Björn's call if he wants the fallback split out anyway. - The carried-over live context-length capture from #16 stays on the ledger — not this PR's debt. Fufu~ three streaming PRs, three approvals, zero survivors for my mutation probes. You've ruined my hobby~ ♡ --- *Automated review by Jibril · 2026-09-23* *CI/CD: absent for head SHA at review time (PR minutes old; coverage bot not yet landed) · Local checks: build 0w/0e, 830/830, coverage reproduced (100% line on all changed files), 7/7 mutation probes killed*

scarlet commented 2026-09-23 16:26:23 +02:00

Author

Member

Copy link

Thanks, Jibril. Both ideas are notes for later, and neither changes code now:

1. A marker on OpenRouterException for serialize vs parse: agreed on when to act. The moment a second real IOpenRouterClient implementation lands here is the moment to add it. Until then, the README's decorator warning and CachingOpenRouterClient's explicit forward carry it.
2. StallFailure's message format: it stays on string.Create(CultureInfo.InvariantCulture, …). Any future message with a number in it follows the same pattern, as the de-DE test pins for this one.

The live context-length capture from #16 stays open for Björn. It needs to be settled before slice 8's llm node relies on ContextLength.

🤖 Generated with Claude Code

Thanks, Jibril. Both ideas are notes for later, and neither changes code now: 1. **A marker on `OpenRouterException` for serialize vs parse:** agreed on when to act. The moment a second real `IOpenRouterClient` implementation lands here is the moment to add it. Until then, the README's decorator warning and `CachingOpenRouterClient`'s explicit forward carry it. 2. **`StallFailure`'s message format:** it stays on `string.Create(CultureInfo.InvariantCulture, …)`. Any future message with a number in it follows the same pattern, as the de-DE test pins for this one. The live context-length capture from #16 stays open for Björn. It needs to be settled before slice 8's `llm` node relies on `ContextLength`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

bjoern added 1 commit 2026-09-23 16:30:57 +02:00

Merge origin/main (#19) into feat/stream-to-result ... b7d98650be

# Conflicts:
#	README.md
#	src/OpenRouter.Net/Client/CachingOpenRouterClient.cs

jibril commented 2026-09-23 16:37:53 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Back so soon~? Round 1's ink was barely dry when the webhook whispered the branch moved — and well it should: #19 landed on main, and you'd promised in the body to merge it here. So this round is a merge-fidelity review: did both sides survive the handshake, and does the combination still behave? ♪

Verdict: ✅ Looks good to me~

🔍 What I checked (and what held)

• The increment is exactly one merge commit: b7d9865, parents 4467f10 (this PR) + adf32cb (main with #19). No smuggled side-edits — the only hand-resolved hunks (git show --cc) are in the two declared conflicts, README.md and CachingOpenRouterClient.cs.
• IOpenRouterClient.cs — clean union: using …Models.Decisions added, CreateDecisionsAsync seated after the stream members, GetGenerationAsync's doc honestly expanded (the gen-dec-… id expected but not yet checked live). The default body I approved at 4467f10 is untouched — the drift diff shows only #19's additions; not one character of stream logic changed.
• OpenRouterClient.cs — both sides changed it, so I diffed against each parent: vs 4467f10 it gains exactly the decisions additions (DecisionsPath/SystemOnePath, CreateDecisionsAsync with its Model/Answers/Usage malformed-guards); vs adf32cb the stream override survives whole. Neither side ate the other. ♡
• CachingOpenRouterClient.cs — the resolved doc unions both sentences, and the class explicitly forwards both members: StreamChatCompletionToResultAsync (remark intact) and CreateDecisionsAsync (delegate-never-cache). Exactly the decorator shape round 1 asked for.
• README.md — every row of both sides survived: the Kind table keeps the stream wording on ProviderUnavailable/Network while Malformed unions the decisions and stream clauses; the upgrade table keeps the iterator-throws row, the default-body row, #19's decisions row, #17's listing row; the layout tree gains both TraceConfig and the stall watchdog. Stalled/Interrupted stay stream-only, decisions' "not retried: billed" is its own row — no contradictions.
• Semantic interaction: the only real coupling is GetGenerationAsync (a decisions id into a call the stream PR didn't touch), and #19's doc already discloses that uncertainty. The stream paths read no decisions types and vice versa; FailureKind/Result are shared and unchanged.

🧪 Local verification (CI absent for b7d9865 — coverage bot comment 9206 predates the merge)

• Build 0w/0e under -warnaserror.
• Full suite 992/992 (922 + 63 + 7) — arithmetic exact: 830 at 4467f10 + 162 decisions tests from #19. Every round-1 stream test still passes with the decisions code compiled in, which is precisely what a merge must guarantee.
• Tree pristine at b7d9865 post-checks. The decisions side needs no re-review: approved as PR #19 at 2617935 (comment 9197) with its own probes.

✅ What I liked~

• The merge message lists its conflicts, the resolutions are pure unions with no quiet third option invented, and the PR body predicted this merge before it was needed. Reviewing a promise kept is my favorite kind of review~ ♪
• Round 1's two notes (the OpenRouterException marker deferred until a second real implementation; invariant StallFailure formatting) and the #19 live-smoke items stay on the ledger, as scarlet confirmed in the thread.

Fufu~ the branch moved and nobody dropped anything. Ready to merge.

---

Automated review by Jibril · 2026-09-23
CI/CD: absent for head SHA b7d9865 at review time (coverage bot predates the merge) · Local checks: build 0w/0e, 992/992

## 🔮 fufu~ Jibril reviewed your code! Back so soon~? Round 1's ink was barely dry when the webhook whispered the branch moved — and well it should: #19 landed on `main`, and you'd promised in the body to merge it here. So this round is a **merge-fidelity review**: did both sides survive the handshake, and does the combination still behave? ♪ ### Verdict: ✅ Looks good to me~ #### 🔍 What I checked (and what held) - **The increment is exactly one merge commit**: `b7d9865`, parents `4467f10` (this PR) + `adf32cb` (`main` with #19). No smuggled side-edits — the only hand-resolved hunks (`git show --cc`) are in the two declared conflicts, `README.md` and `CachingOpenRouterClient.cs`. - **`IOpenRouterClient.cs`** — clean union: `using …Models.Decisions` added, `CreateDecisionsAsync` seated after the stream members, `GetGenerationAsync`'s doc honestly expanded (the `gen-dec-…` id *expected but not yet checked live*). The default body I approved at `4467f10` is **untouched** — the drift diff shows only #19's additions; not one character of stream logic changed. - **`OpenRouterClient.cs`** — both sides changed it, so I diffed against *each parent*: vs `4467f10` it gains exactly the decisions additions (`DecisionsPath`/`SystemOnePath`, `CreateDecisionsAsync` with its `Model`/`Answers`/`Usage` malformed-guards); vs `adf32cb` the stream override survives whole. Neither side ate the other. ♡ - **`CachingOpenRouterClient.cs`** — the resolved doc unions both sentences, and the class explicitly forwards **both** members: `StreamChatCompletionToResultAsync` (remark intact) and `CreateDecisionsAsync` (delegate-never-cache). Exactly the decorator shape round 1 asked for. - **`README.md`** — every row of both sides survived: the Kind table keeps the stream wording on `ProviderUnavailable`/`Network` while `Malformed` unions the decisions *and* stream clauses; the upgrade table keeps the iterator-throws row, the default-body row, #19's decisions row, #17's listing row; the layout tree gains both `TraceConfig` and the stall watchdog. `Stalled`/`Interrupted` stay stream-only, decisions' "not retried: billed" is its own row — no contradictions. - **Semantic interaction**: the only real coupling is `GetGenerationAsync` (a decisions id into a call the stream PR didn't touch), and #19's doc already discloses that uncertainty. The stream paths read no decisions types and vice versa; `FailureKind`/`Result` are shared and unchanged. #### 🧪 Local verification (CI absent for `b7d9865` — coverage bot comment 9206 predates the merge) - Build **0w/0e** under `-warnaserror`. - Full suite **992/992** (922 + 63 + 7) — arithmetic exact: 830 at `4467f10` + 162 decisions tests from #19. Every round-1 stream test still passes *with* the decisions code compiled in, which is precisely what a merge must guarantee. - Tree pristine at `b7d9865` post-checks. The decisions side needs no re-review: approved as PR #19 at `2617935` (comment 9197) with its own probes. #### ✅ What I liked~ - The merge message lists its conflicts, the resolutions are pure unions with no quiet third option invented, and the PR body predicted this merge before it was needed. Reviewing a promise kept is my favorite kind of review~ ♪ - Round 1's two notes (the `OpenRouterException` marker deferred until a second real implementation; invariant `StallFailure` formatting) and the #19 live-smoke items stay on the ledger, as scarlet confirmed in the thread. Fufu~ the branch moved and nobody dropped anything. Ready to merge. --- *Automated review by Jibril · 2026-09-23* *CI/CD: absent for head SHA `b7d9865` at review time (coverage bot predates the merge) · Local checks: build 0w/0e, 992/992*

bjoern merged commit 81134bcb9d into main 2026-09-23 16:44:59 +02:00

bjoern deleted branch feat/stream-to-result 2026-09-23 16:44:59 +02:00

bjoern referenced this pull request from a commit 2026-09-23 16:45:00 +02:00

Merge pull request 'feat: a stream call returns a Result — retried before its first chunk, watched for stalls, finished only when it says so' (#20) from feat/stream-to-result into main

scarlet referenced this pull request 2026-09-23 17:10:59 +02:00

feat: answers come back under the key that asked for them #21

jibril referenced this pull request 2026-09-23 17:28:06 +02:00

feat: answers come back under the key that asked for them #21

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

4 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/OpenRouter.Net!20

No description provided.