[siebe41]

What changed

Tests, plus one consistency fix in src/ from the review: TuningApi's /tuning and /tuning/changes measure a change's age and effect on the router's TimeProvider (the clock RouterTuner stamps changes with, and the one /judge/agreement already used) instead of DateTimeOffset.UtcNow. A coverage run (dotnet test --collect:"XPlat Code Coverage", the whole suite, on Windows) put SemanticRouter.Core and SemanticRouter.AspNetCore at 84.4% of lines (20,880 / 24,729). I ranked the hand-written classes by line coverage. Nested and compiler-generated types were folded into their outer type, and the System.Text.RegularExpressions.Generated and *JsonContext source-generated code was left out. From that list I took the ten least-covered classes that aren't just platform branches or property getters, and added focused tests:

Class	Before	After	What the new tests pin down
MachinePolicyLoader	39.4%	51.5%	Registry values of other kinds (QWORD, binary, null, text that only looks like JSON), a missing macOS property list is no policy, and a policy file that exists but can't be read stops the router with an error naming the file
AnswerCapture	48.6%	100%	All four write overloads reach the client, only the first Limit bytes are kept, Truncated (never set by any test before), and disposing leaves the response stream open
SemanticRouterBuilder	49.4%	97.4%	Every Use*/Add* registers what it says over the default: embedding provider by type and by factory, OpenAI-compatible embeddings, evaluation set, model-family and client-turn files (made absolute), the secret file next to the configuration file, custom stores, connectors, memory sources and models; and each refuses a missing argument
KnowledgeGraphMemorySource	62.3%	100%	Loading from disk: a file with its modification time, a folder's memory.jsonl before memory.json, and the errors for a missing file or a folder without a graph
TuningApi	70.7%	93.9%	The effect of a learned change (decisions and feedback before and after it, rerouted decisions). It was never computed in a test, because a change has to be an hour old. The default threshold touches every decision; a route's threshold touches that route's decisions and the ones it rerouted. Tool continuations and model overrides don't count. A change under an hour old has no effect yet
ProtocolErrors	79.7%	100%	The error type each status maps to for Responses clients (OpenAI envelope) and Anthropic clients; problem details for OpenAI clients; a stream that can't be finished ends with an error event in each protocol; a non-streamed reply is aborted rather than left truncated; the connection is aborted when the client is gone too
StallGuard	80.0%	96.7%	A read that waits past the limit is cancelled and called a stall (both ReadAsync overloads); time the client takes between reads doesn't count; a cancelled request isn't a stall; headers and the whole body come through; synchronous reads pass through unguarded; the duration text
OpenAIRequestBuilder	82.0%	100%	tool_choice from OpenAI (none, required, named) and from Anthropic (none, any, tool); top_p, stop, user; json_object and json_schema (strict, default name and schema); a tool schema that doesn't parse is written as any object
QuotaResetParser (Core)	84.1%	98.6%	12am/12pm, impossible times and dates (25:00, 3:75pm, Feb 30, Sep 31), a dated reset that has passed, a January date read in December, (UTC)/(GMT)/unknown zones, a zero relative reset, and Retry-After HTTP dates in the past and future
ConfigurationIssues	82.6%	95.9%	Field paths built from the validator's own messages (an unnamed budget and its bad limit, an unnamed or repeated memory source, a repeated tier level, a bad complexity rule, the classification cache, a profile rule naming an unknown profile, the selection time zone), and the format hint for a value of the wrong type (number, true/false, text, object, list, enum)

Overall: 85.5% (21,155 / 24,729), 1,562 tests (100 new). I also gave TuningTests.StartTunedAsync an optional TimeProvider, so a test can make changes in the past.

Left out of the ten, and why:

• CliScratch (36.8%) and OwnerOnly (73.7%): the missed lines are the Unix-only paths. Their tests exist (CliScratchTests and others) and return early on Windows, so a Windows run can't see them. Linux and macOS CI run them.
• TuningOptions (70.5%): the missed lines are property getters.
• ServiceHealth (75.5%): what's missed reads the machine's network interfaces and process start time, with no seam for a test.

Why

Overflow task O02 of the usage sprint: find the least-covered non-trivial classes in Core and AspNetCore (protocol translation, routing decisions, budgets, secrets) and add focused tests before 1.0. Most of these classes were tested only end to end, so their error and edge paths (stalls, aborted streams, error envelopes, reset-time edge cases, builder registrations) never ran.

How it was verified

• dotnet build -c Release: 0 warnings, 0 errors. Two analyzer findings on the new tests were fixed; one is suppressed where the test calls the array WriteAsync overload on purpose (CA1835).
• New and changed classes alone: dotnet test -c Release --filter "…ConfigurationIssuesTests|…QuotaResetParserTests|…AnswerCaptureTests|…StallGuardTests|…ProtocolErrorsTests|…OpenAIRequestBuilderTests|…KnowledgeGraphMemorySourceTests|…MachinePolicyTests|…SemanticRouterBuilderTests|…TuningTests": 136 passed.
• Full suite with coverage: 1,559 / 1,562 passed. The 3 failures were ChatProtocolTests.A_large_tool_result_is_searched_in_bounded_time and two socket-bound host tests (LifecycleTests…restarts_the_router…, ServiceTests…address_in_use). They time out when the other sprint lanes load this PC, they test code this PR doesn't touch, and all 3 pass when run alone.
• dotnet format --verify-no-changes: clean.
• No packed library changes, so no dotnet pack.

Linux and macOS (this PC is Windows):

• MachinePolicyTests.A_policy_file_that_cannot_be_read… holds the file open with FileShare.None. On Unix, .NET turns that into an exclusive advisory flock, and File.ReadAllText's shared lock is refused with an IOException: the same failure as on Windows, which is what the loader turns into InvalidDataException. If a CI file system ignores flock (NFS, or DOTNET_SYSTEM_IO_DISABLEFILELOCKING set), this test is the one to look at.
• StallGuardTests uses real timers. The stall tests need a 100 ms limit to fire, which load can only delay. The between-reads test arms a limit on each read that is reset only when the read returns, so a thread held up past the limit would trip it: it uses its own 1 s limit (for a read whose data is already waiting) and a 1.5 s client pause that must not count.
• Paths: temp folders and Path.GetFullPath/Path.Combine only. The time zone tests use a custom +02:00 zone and a zone id that exists on no OS.

Not done

• Two bugs in ConfigurationIssues that these tests found. Each is fixed, with its regression test, in a separate PR stacked on this one (Tie an '@' profile name and a non-text address to the right configuration field (O02) #329), as the task asks:
  1. A profile name with @ is tied to Profiles.@ instead of the profile: the validator's message quotes '@' before the name, and the first quoted text was taken.
  2. A value that doesn't parse as an address (e.g. "BaseAddress": 5) says "is not valid here" instead of the address hint: Type.GetType("System.Uri") returns null, because Uri isn't in CoreLib.
• MachinePolicyLoader's Windows registry path (HKLM) and macOS plutil path stay untested. The first needs a machine-wide registry key, which the sprint rules don't allow on this PC; the second needs macOS.
• No CHANGELOG line: the src/ change is not user-visible (outside tests, the router's TimeProvider is the system clock).

Refs: usage-sprint overflow task O02 (no issue).

Latest integration: merged current main and updated the remaining test calls to its new EvaluationCase, ConfigurationIssue and classifier dependency APIs. The focused coverage/API tests pass (60/60), and changed-file formatting is clean.

Full-suite follow-up: all 1,593 tests passed on the integrated branch (OPENAI_API_KEY unset for test isolation).

[siebe41]

Needs changes, small

Both real problems are in one new test, A_change_an_hour_old_lists_its_effect… in TuningTests. Both were confirmed by changing the production code in a scratch worktree and running the filtered tests.

Medium: the TuningApi "effect" test only works because two clocks disagree (tests/SemanticRouter.Tests/AspNetCore/TuningTests.cs:253)

• The test sets the injected ManualTimeProvider to wall-clock minus 3 h, so RouterTuner stamps the changes 3 h ago.
• The change only looks an hour old because TuningApi.Status ignores that provider and reads DateTimeOffset.UtcNow (src/SemanticRouter.AspNetCore/Dashboard/TuningApi.cs:97). /judge/agreement in the same file (:28) already uses RequestServices.GetService<TimeProvider>() ?? TimeProvider.System.
• When Status takes its now from the injected TimeProvider, which is the consistent fix, the test fails with effect = null (InvalidOperationException … element has type 'Null'), although the behaviour under test is unchanged. So the test pins a clock inconsistency, not the effect calculation.
• Fix: make Status read the injected TimeProvider (one line in each of the two /tuning MapGet lambdas). Then, in the test, run the tuner at the manual clock's time and time.Advance(TimeSpan.FromHours(3)) before calling /router/api/tuning. If src/ has to stay untouched in this PR, add a comment at :253 saying the test depends on Status using the wall clock.

Medium: the "and those it rerouted" part of the route assertion is never exercised (TuningTests.cs:277, :289)

• For a route change, Measure counts decisions on the change's route or with Learned == change.Id (TuningApi.cs:128).
• The only decision with learned: 2 is itself on route "reasoning", so the || e.Learned == change.Id branch adds nothing. With that branch deleted, all 12 TuningTests still pass.
• The assertion message at :289 says this case is covered.
• Fix: let the learned decision land elsewhere, as a raised reasoning threshold would: Judged(Decision(TimeSpan.FromHours(1), "smalltalk", learned: 2));. Both expected tuples stay as they are, (3, 2, 1, 3, 2, 1) and (3, 1, 0, 1, 1, 1), and deleting the branch makes the route assertion fail. Optionally add an unjudged after-change "reasoning" decision, so both halves of the OR are counted.

Low: one StallGuard test can fail under load, despite what the PR body says (tests/SemanticRouter.Tests/AspNetCore/StallGuardTests.cs:14, :45-57)

• The body says these tests use only lower bounds, so "load can make them slower but not fail them". That doesn't hold for Time_the_client_takes_between_reads_does_not_count.
• Every read arms a 100 ms CancelAfter. Only the finally in StallGuard.ReadAsync resets it, once the inner read returns.
• If the test thread is held up for more than 100 ms in that window, Stalled becomes true and the test fails. A GC pause or thread-pool starvation on a loaded CI runner can do that.
• Fix: give this test its own larger limit (say 500 ms to 1 s, with the pause above it), or correct the body.

nit: MachinePolicyTests.cs:113-114. The JsonObject indexer also returns null for a key that isn't there, so root["Binary"].Should().BeNull() would still pass if FromValues dropped the key. If "present with a null value" is the contract, also assert root.ContainsKey("Binary").

Checked

• The diff is tests only: 10 files under tests/, nothing in src/. Per CONTRIBUTING, no CHANGELOG line is needed.
• Proved by mutation that these assertions catch the behaviour they name:
  • Changing AnswerCapture.Keep's bytes.Length > room to >= fails A_reply_of_exactly_the_limit_is_not_truncated.
  • Removing StallGuard's CancelAfter(Timeout.InfiniteTimeSpan) reset fails Time_the_client_takes_between_reads_does_not_count.
• From the production code:
  • The array-overload StallGuard test would fail if GuardedStream didn't override that overload.
  • The ProtocolErrors "client is gone" test would fail without the IOException catch, since GoneStream overrides the overload the production code actually calls.
  • The QuotaResetParser edge cases fail for the right reasons. Feb 30 and Sep 31 hit the DaysInMonth check, the past Oct 8, 9am doesn't trigger the year bump, and the +02:00 case pins the <= boundary.
• Cross-platform:
  • The FileShare.None locked-file test passes on the ubuntu-latest and macos-latest CI runs.
  • All file I/O goes through Directory.CreateTempSubdirectory/TempFolder and is cleaned up.
  • The relative paths in SemanticRouterBuilderTests are only resolved, never written.
  • The default UseSecretFile() store only calls GetFullPath and creates its protector lazily, so nothing reaches the user's key ring.
• Nothing uses the network, a port or a vendor CLI, and no test changes the current directory.
• The body's "never tested before" claims hold on the base: there were no AnswerCapture tests and no assertion on effect.
• The coverage numbers in the body were not re-measured.

CI is green on all checks. Nothing pushed.

[siebe41]

Review addressed in 96e557d (merged origin/main first in 2a32a14, and merged into #329 as 6f54e61).

1. Medium, TuningApi effect test relies on two clocks. Fixed in 96e557d. /tuning and /tuning/changes now take now from the injected TimeProvider (a shared Now(context), which /judge/agreement uses too). The test runs the tuner on the manual clock, then calls time.Advance(TimeSpan.FromHours(3)). Mutation: putting DateTimeOffset.UtcNow back in Status fails the test.
2. Medium, the "and those it rerouted" branch is never exercised. Fixed in 96e557d. The learned decision now lands on smalltalk (Judged(Decision(1h, "smalltalk", learned: 2))), and I added an unjudged after-change reasoning decision at +2 h so both halves of the OR are counted. Expected tuples: (3, 2, 1, 4, 2, 1) and (3, 1, 0, 2, 1, 1). Mutation: deleting || e.Learned == change.Id fails the test.
3. Low, the StallGuard between-reads test can fail under load. Fixed in 96e557d. That test now has its own 1 s limit with a 1.5 s pause. Mutation: removing the CancelAfter(Timeout.InfiniteTimeSpan) reset still fails it. The PR body's Linux/macOS paragraph is corrected to describe this.
4. nit, root["Binary"] null vs absent. Not changed as suggested: present-with-null isn't the contract. FromValues' object goes through ToJsonString() into MachinePolicy.FromJson, where a null member and a missing one both leave the setting unset, so pinning ContainsKey would pin an implementation detail. I reworded the assertion's reason to say "null and absent both leave it unset".

The body now says that src/ has this one consistency change and why there is no CHANGELOG line (it isn't user-visible: outside tests the TimeProvider is the system clock).

Verified: dotnet build -c Release gave 0 warnings and 0 errors. The focused tests (Tuning, StallGuard, MachinePolicy, JudgeAgreement) passed 41/41. The full suite passed 1559/1562. The 3 failures are the known SecretsTests OPENAI_API_KEY one and the #335 Host pair (LifecycleTests…taken_address, ServiceTests…address_in_use), and each passes alone. dotnet format --verify-no-changes is clean. #329 after the merge: its build and 90 focused tests pass, and format is clean. CI is green on both PRs.