From f3d07b17b21de224b60be5875ccc2e579cc9e126 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Tue, 29 Sep 2026 20:37:45 +0800 Subject: [PATCH] Send the provider a combinator-free planner schema Since #1854 expanded the planner acceptance's per-kind oneOf branches, every planner call to the hosted vLLM gateway has failed with an empty HTTP 500 (followed by 429s), the planner node parked on SupervisorInfraPark, and the live planner gates measured nothing on every main run. The same model answers every other caller, and heads without #1854 got 200s. vLLM compiles the forced tool's schema into a decoding grammar, and its error names no keyword. The request now carries an optional WireJsonSchema that both provider clients send as the tool schema, falling back to JsonSchema. The planner sends PlannerSchema.WireSchema: ResponseSchema with oneOf, anyOf, allOf, not, if, then and else stripped at every schema position, which leaves the flat typed acceptance the gateway answered before #1849 while still declaring every acceptance field. Dropping a combinator only removes a constraint, so the wire schema accepts every reply the contract does; validation, the bounded re-ask and the schema quoted in the prompt all keep the full ResponseSchema. A guard pins every schema the code sends to a model as combinator-free on the wire. The park log and the E2E ride's unresolved-park message now carry the fault text, so the next outage shows the gateway's words. --- .../Deciders/LlmSupervisorDecider.cs | 2 +- .../Llm/Anthropic/AnthropicClient.cs | 2 +- .../Workflows/Llm/IStructuredLLMClient.cs | 9 ++ .../Workflows/Llm/JsonSchemaCombinators.cs | 63 ++++++++ .../Workflows/Llm/OpenAi/OpenAiClient.cs | 2 +- .../Services/Workflows/Nodes/InfraPark.cs | 2 +- .../Workflows/Planning/PlannerSchema.cs | 10 ++ .../Planning/Planners/LlmWorkflowPlanner.cs | 1 + .../PlanMapSynthPlannerRequest.cs | 1 + .../Workflows/Supervisor/InfraParkRide.cs | 14 +- .../Supervisor/InfraParkRideTests.cs | 15 ++ .../Workflows/InfraParkTests.cs | 25 ++++ .../Workflows/JsonSchemaCombinatorsTests.cs | 136 ++++++++++++++++++ .../StructuredResponseContractTests.cs | 54 +++++++ .../Workflows/TypedModelSchemaBranchTests.cs | 67 ++++++--- 15 files changed, 378 insertions(+), 25 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Services/Workflows/Llm/JsonSchemaCombinators.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/JsonSchemaCombinatorsTests.cs diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs index 6e117c68d..fbff73a9b 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/Deciders/LlmSupervisorDecider.cs @@ -572,7 +572,7 @@ internal static string MissingPayloadRepairPrompt(string kind, string defect, st /// The fewest decisions worth folding — below this a compaction would not shrink the prompt meaningfully (the overflow has another cause), so the original fault propagates to the clean-stop path. internal const int MinCompactFold = 4; - private static readonly JsonElement TapeSummarySchema = JsonDocument.Parse(""" + internal static readonly JsonElement TapeSummarySchema = JsonDocument.Parse(""" { "type": "object", "additionalProperties": false, "required": ["summary"], "properties": { "summary": { "type": "string", "description": "The compact progress digest." } } } """).RootElement; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/Anthropic/AnthropicClient.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/Anthropic/AnthropicClient.cs index 324f99b75..73d3f5eb7 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/Anthropic/AnthropicClient.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/Anthropic/AnthropicClient.cs @@ -213,7 +213,7 @@ private async Task CompleteStructuredOnceAsync(Structur StopSequences = request.Sampling?.Stop, System = system, Messages = messages, - Tools = new[] { new AnthropicTool { Name = StructuredToolName, Description = "Return the result as structured JSON.", InputSchema = request.JsonSchema } }, + Tools = new[] { new AnthropicTool { Name = StructuredToolName, Description = "Return the result as structured JSON.", InputSchema = request.WireJsonSchema ?? request.JsonSchema } }, ToolChoice = new AnthropicToolChoice { Type = "tool", Name = StructuredToolName } }; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/IStructuredLLMClient.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/IStructuredLLMClient.cs index 2cb1a6fb8..8a99e6613 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/IStructuredLLMClient.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/IStructuredLLMClient.cs @@ -36,6 +36,15 @@ public sealed record StructuredLLMCompletionRequest /// JSON Schema (object) the response MUST conform to. public required JsonElement JsonSchema { get; init; } + /// + /// The schema the PROVIDER is handed as the forced tool's schema, when that must differ from ; + /// null sends itself. It exists for a constrained decoder that cannot compile a construct the + /// contract needs (see ). It must ACCEPT everything + /// accepts: every reply is still validated against , which also stays the schema the prompt + /// quotes, so a narrower wire schema would forbid the model an answer the contract allows. + /// + public JsonElement? WireJsonSchema { get; init; } + /// Server-only validation of the consumer contract, alongside JSON schema. Violations enter the same bounded model re-ask; this callback never rewrites output. [JsonIgnore] public Func>? ResponseValidator { get; init; } diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/JsonSchemaCombinators.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/JsonSchemaCombinators.cs new file mode 100644 index 000000000..e0db008ef --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/JsonSchemaCombinators.cs @@ -0,0 +1,63 @@ +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace CodeSpace.Core.Services.Workflows.Llm; + +/// +/// Derives a schema's COMBINATOR-FREE form: oneOf, anyOf, allOf, not, if, then +/// and else dropped at every schema position, every other keyword, property name and literal kept verbatim. This +/// is the form a provider's constrained decoder is handed as +/// while the full schema keeps validating every reply. +/// +/// Why it exists: a hosted vLLM backend compiles the forced tool's schema into a decoding grammar, and a +/// combinator shape it cannot compile fails the whole call with an EMPTY HTTP 500 — not a 400 the progressive fallback +/// degrades on. The planner's per-kind acceptance branches did exactly that on every call, while every combinator-free +/// schema sent to the same model was answered. The error names no keyword, so every combinator goes, not a guessed one. +/// +/// Why it is safe: each combinator only adds a constraint alongside its siblings, so dropping one can only widen +/// what a schema accepts, and the model is never forbidden a reply the contract allows. What the wire no longer says is +/// still enforced: the bounded re-ask validates against the full schema. The one keyword family this reasoning would +/// not hold for, unevaluatedProperties/unevaluatedItems, reads annotations out of these subschemas; no +/// model-facing schema here uses it. +/// +public static class JsonSchemaCombinators +{ + private static readonly string[] Keywords = ["oneOf", "anyOf", "allOf", "not", "if", "then", "else"]; + + /// Keywords whose value maps NAMES to subschemas — the names are data (a property called not is still a property), only the values are schemas. + private static readonly HashSet NamedSubschemas = ["properties", "patternProperties", "$defs", "definitions", "dependentSchemas"]; + + /// Keywords whose value is a subschema, or an array of them. Anything else (enum, const, default, required, …) is data and is copied untouched. + private static readonly HashSet Subschemas = ["items", "prefixItems", "additionalItems", "additionalProperties", "contains", "propertyNames", "unevaluatedItems", "unevaluatedProperties"]; + + /// A copy of with every combinator removed; the input is never modified. + public static JsonElement Strip(JsonElement schema) + { + var copy = JsonNode.Parse(schema.GetRawText()); + + StripSchema(copy); + + return JsonSerializer.SerializeToElement(copy); + } + + private static void StripSchema(JsonNode? node) + { + if (node is not JsonObject schema) return; + + foreach (var keyword in Keywords) schema.Remove(keyword); + + foreach (var (keyword, value) in schema) StripChildren(keyword, value); + } + + private static void StripChildren(string keyword, JsonNode? value) + { + if (NamedSubschemas.Contains(keyword) && value is JsonObject named) StripEach(named.Select(pair => pair.Value)); + else if (Subschemas.Contains(keyword) && value is JsonArray list) StripEach(list); + else if (Subschemas.Contains(keyword)) StripSchema(value); + } + + private static void StripEach(IEnumerable schemas) + { + foreach (var schema in schemas) StripSchema(schema); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Llm/OpenAi/OpenAiClient.cs b/backend/src/CodeSpace.Core/Services/Workflows/Llm/OpenAi/OpenAiClient.cs index a141413c8..e0d8dd180 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Llm/OpenAi/OpenAiClient.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Llm/OpenAi/OpenAiClient.cs @@ -254,7 +254,7 @@ private async Task CompleteStructuredOnceAsync(Structur Stop = request.Sampling?.Stop, ReasoningEffort = LlmModelCapabilities.SupportsReasoningEffort(request.Model) ? request.ReasoningEffort : null, // sent ONLY to a reasoning model (a plain chat model 400s on it); the value rides verbatim (the API validates it per model) Messages = BuildMessages(system, request.UserPrompt), - Tools = new[] { new OpenAiTool { Function = new OpenAiFunction { Name = StructuredToolName, Description = "Return the result as structured JSON.", Parameters = request.JsonSchema } } }, + Tools = new[] { new OpenAiTool { Function = new OpenAiFunction { Name = StructuredToolName, Description = "Return the result as structured JSON.", Parameters = request.WireJsonSchema ?? request.JsonSchema } } }, ToolChoice = new OpenAiToolChoice { Function = new OpenAiToolChoiceFunction { Name = StructuredToolName } }, }; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/InfraPark.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/InfraPark.cs index a3b618681..2c288f0c5 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/InfraPark.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/InfraPark.cs @@ -54,7 +54,7 @@ public static NodeResult Park(NodeRunContext context, LlmApiException fault, Dat var delay = SupervisorInfraPark.DelayFor(state.Parks); var marker = SupervisorInfraPark.Marker(state, fault.Message); - context.Logger.LogWarning("Node {NodeId}: model call hit a {Category} infra fault — parking {Delay} (park {Parks} since {First:o}) instead of failing the run", context.NodeId, fault.Category, delay, state.Parks, state.FirstParkedAtUtc); + context.Logger.LogWarning("Node {NodeId}: model call hit a {Category} infra fault — parking {Delay} (park {Parks} since {First:o}) instead of failing the run: {Fault}", context.NodeId, fault.Category, delay, state.Parks, state.FirstParkedAtUtc, fault.Message); return NodeResult.Suspend(new SuspensionToken { diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs b/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs index cfec2f25a..3ffa655d4 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Serialization; +using CodeSpace.Core.Services.Workflows.Llm; namespace CodeSpace.Core.Services.Workflows.Planning; @@ -155,6 +156,15 @@ public static class PlannerSchema } """).RootElement.Clone(); + /// + /// What the provider's constrained decoder is handed in place of : the same schema with its + /// combinators stripped, so the acceptance is one flat typed object that still declares every field (formatVersion, + /// kind, argv, artifactPaths, …) but carries no per-kind branch. The branches made a hosted vLLM + /// backend answer every planner call with an empty HTTP 500. still validates each reply and + /// is still the schema the prompt quotes, so the per-kind requirements are enforced by the bounded re-ask instead. + /// + public static readonly JsonElement WireSchema = JsonSchemaCombinators.Strip(ResponseSchema); + /// Deserialization options for mapping a schema-valid object back into PlannedWorkflow. Case-insensitive so the model's lower-camel keys bind to the record's Pascal properties; the string-enum converter binds the acceptance kind ("TestsPass"/"ArtifactPresent") to BenchmarkGradingKind. public static readonly JsonSerializerOptions Options = new() { diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Planning/Planners/LlmWorkflowPlanner.cs b/backend/src/CodeSpace.Core/Services/Workflows/Planning/Planners/LlmWorkflowPlanner.cs index 6dcaf55d7..2f52a06e2 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Planning/Planners/LlmWorkflowPlanner.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Planning/Planners/LlmWorkflowPlanner.cs @@ -91,6 +91,7 @@ public async Task PlanAsync(WorkflowPlanRequest request, Cancel SystemPrompt = SystemPrompt, UserPrompt = BuildUserPrompt(request, catalog, lessons), JsonSchema = PlannerSchema.ResponseSchema, + WireJsonSchema = PlannerSchema.WireSchema, ResponseValidator = ValidateModelResponse, ResponseAdvisor = AdviseModelResponse, MaxOutputTokens = 4096, diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/PlanMapSynthPlannerRequest.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/PlanMapSynthPlannerRequest.cs index 9780698da..b01825b7e 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/PlanMapSynthPlannerRequest.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Infrastructure/PlanMapSynthPlannerRequest.cs @@ -62,6 +62,7 @@ public static async Task BuildAsync(ILifetimeSco SystemPrompt = LlmWorkflowPlanner.SystemPrompt, UserPrompt = LlmWorkflowPlanner.BuildUserPromptForTest(planRequest, catalog), JsonSchema = PlannerSchema.ResponseSchema, + WireJsonSchema = PlannerSchema.WireSchema, }; } diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRide.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRide.cs index f699a64df..6bf976cac 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRide.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRide.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Text.Json; using Autofac; using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Services.Workflows.Engine; @@ -159,7 +160,18 @@ private static async Task PauseAsync(Stopwatch clock, TimeSpan wakePau private static string Unresolved(ParkedCell cell, int wakes, TimeSpan wakePause) => $"node '{cell.NodeId}' was STILL parked on {WorkflowWaitKinds.SupervisorInfraPark} after {wakes} deadline wake(s) over ~{(wakes * wakePause).TotalSeconds:0}s " - + "— the model plane never came back inside the ride's budget. That is INFRA, not a model verdict, and it is NOT a pass: nothing was driven to completion."; + + "— the model plane never came back inside the ride's budget. That is INFRA, not a model verdict, and it is NOT a pass: nothing was driven to completion. " + + $"The park's last fault: {ParkFault(cell)}"; + + /// The fault the park stored on its own marker — the gateway's own words — so a park that outlives the ride names its cause in the job summary, not only its duration. + private static string ParkFault(ParkedCell cell) + { + if (cell.WaitPayloadJson is null) return "(none recorded)"; + + using var marker = JsonDocument.Parse(cell.WaitPayloadJson); + + return marker.RootElement.TryGetProperty("error", out var error) && error.ValueKind == JsonValueKind.String ? error.GetString()! : "(none recorded)"; + } /// Read the run's newest pending infra park plus the projected status of the cell it holds. No park pending ⇒ a Settled cell (there is nothing for the ride to wake). private static async Task ReadCellAsync(PostgresFixture fixture, Guid runId) diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRideTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRideTests.cs index 5c10621f0..5bfe3c280 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRideTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/Supervisor/InfraParkRideTests.cs @@ -1,3 +1,4 @@ +using CodeSpace.Core.Services.Supervisor; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Enums; using Shouldly; @@ -122,6 +123,20 @@ public async Task An_exhausted_budget_yields_the_honest_infra_outcome_never_a_gr RealModelGate.IsGatewayInfraFailure(new AggregateException(ex)).ShouldBeTrue("the await chain can wrap it"); } + [Fact] + public async Task An_unresolved_park_names_the_fault_the_park_stored_on_its_marker() + { + // The planner's park outlived this ride on run after run, and the skip said only THAT the model plane never + // came back. What the gateway actually said sat on the park's own marker, one read away. The marker is minted + // by the production writer, so the key this read depends on cannot drift from it unnoticed. + var marker = SupervisorInfraPark.Marker(SupervisorInfraPark.Next(null, DateTimeOffset.UtcNow), "Anthropic API error (HTTP 500, Transient): Hosted_vllmException"); + var cell = Parked() with { WaitPayloadJson = marker.GetRawText() }; + + var ex = await Should.ThrowAsync(() => InfraParkRide.RideAsync(() => Task.FromResult(cell), _ => Task.CompletedTask, maxWakes: 1, wakePause: TimeSpan.Zero)); + + ex.Message.ShouldContain("Anthropic API error (HTTP 500, Transient): Hosted_vllmException", Case.Sensitive, "the infra skip must carry the gateway's own words, not only the park's duration"); + } + [Fact] public void The_ride_pauses_for_real_between_wakes_so_a_recovery_can_actually_be_observed() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/InfraParkTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/InfraParkTests.cs index 63b540f2d..aeb472fa9 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/InfraParkTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/InfraParkTests.cs @@ -5,6 +5,7 @@ using CodeSpace.Core.Services.Workflows.Runtime; using CodeSpace.Messages.Constants; using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -63,6 +64,18 @@ public void A_first_fault_parks_on_the_shared_wait_kind_with_a_deadline_wake() result.SuspendUntil.TimeoutPayload.ShouldNotBeNull("the wake must carry the ladder position forward"); } + [Fact] + public void The_park_log_carries_the_faults_own_words() + { + // The planner parked on an empty-bodied gateway 500 run after run, and the park line named only the category — + // so the one thing a reader needed, what the gateway actually said, was in no log line at all. + var logger = new CapturingLogger(); + + InfraPark.Park(Context() with { Logger = logger }, Fault(LlmErrorCategory.Transient), DateTimeOffset.UtcNow); + + logger.Messages.ShouldHaveSingleItem().ShouldContain("upstream unavailable"); + } + [Fact] public void The_park_keeps_the_nodes_ambient_cell_so_a_map_branch_stays_in_its_branch() { @@ -113,4 +126,16 @@ public void Past_the_whole_window_the_node_fails_honestly_instead_of_parking_for result.Error.ShouldContain("model plane", Case.Insensitive); result.Retryable.ShouldBeFalse("re-running the node cannot reach a provider that has been down for a day"); } + + /// Keeps every line the park writes, formatted the way a sink would render it. + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + public bool IsEnabled(LogLevel logLevel) => true; + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => Messages.Add(formatter(state, exception)); + + private sealed class NullScope : IDisposable { public static readonly NullScope Instance = new(); public void Dispose() { } } + } } diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/JsonSchemaCombinatorsTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/JsonSchemaCombinatorsTests.cs new file mode 100644 index 000000000..e5ed6eabf --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/JsonSchemaCombinatorsTests.cs @@ -0,0 +1,136 @@ +using System.Reflection; +using System.Text.Json; +using System.Text.Json.Nodes; +using CodeSpace.Core.Services.Agents.ModelCredentials; +using CodeSpace.Core.Services.Learning; +using CodeSpace.Core.Services.Review; +using CodeSpace.Core.Services.Supervisor.Arbiter; +using CodeSpace.Core.Services.Supervisor.Deciders; +using CodeSpace.Core.Services.Tasks.Effort.Classifiers.Llm; +using CodeSpace.Core.Services.Tasks.SpecPreview; +using CodeSpace.Core.Services.Workflows.Llm; +using CodeSpace.Core.Services.Workflows.Planning; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// Pins the combinator-free WIRE form of a structured-output schema: what +/// removes and what it must leave alone, and the portability rule that every schema the code hands a model reaches the +/// provider with no combinator at any depth. A hosted vLLM backend compiles that schema into a decoding grammar, and a +/// combinator shape it cannot compile costs the whole call an empty HTTP 500 — so the rule is pinned per schema rather +/// than rediscovered per outage. +/// +[Trait("Category", "Unit")] +public sealed class JsonSchemaCombinatorsTests +{ + private static readonly string[] Combinators = ["oneOf", "anyOf", "allOf", "not", "if", "then", "else"]; + + /// + /// Every schema backend/src hands a model — the value of each JsonSchema = assignment, plus the coordinator's + /// schema, which reaches one through llm.complete's responseSchema — in the form the provider receives + /// (WireJsonSchema ?? JsonSchema). The planner is the one caller whose wire form differs from its contract. + /// An operator-authored llm.complete schema is the operator's own and is not listed. + /// + private static readonly Dictionary WireForms = new() + { + ["ArbiterDecisionSchema.ResponseSchema"] = ArbiterDecisionSchema.ResponseSchema, + ["CoordinatorSchema.ResponseSchema"] = CoordinatorSchema.ResponseSchema, + ["CriticSchema.GateSchema"] = CriticSchema.GateSchema, + ["CriticSchema.ImproveSchema"] = CriticSchema.ImproveSchema, + ["LessonDistillationSchema.ResponseSchema"] = LessonDistillationSchema.ResponseSchema, + ["LlmEffortClassifierSchema.ResponseSchema"] = LlmEffortClassifierSchema.ResponseSchema, + ["LlmLessonRelevanceEvaluator.ResponseSchema"] = LlmLessonRelevanceEvaluator.ResponseSchema, + ["LlmRubricJudge.RubricVerdictSchema"] = LlmRubricJudge.RubricVerdictSchema, + ["LlmSupervisorDecider.TapeSummarySchema"] = LlmSupervisorDecider.TapeSummarySchema, + ["ModelTieringSchema.ResponseSchema"] = ModelTieringSchema.ResponseSchema, + ["PlannerSchema.WireSchema"] = PlannerSchema.WireSchema, + ["SupervisorDecisionSchema.ResponseSchema"] = SupervisorDecisionSchema.ResponseSchema, + ["TaskSpecCompilerSchema.ResponseSchema"] = TaskSpecCompilerSchema.ResponseSchema, + ["TaskSpecCompilerSchema.ReviewSchema"] = TaskSpecCompilerSchema.ReviewSchema, + }; + + /// Schemas that only ever VALIDATE: their call site hands the provider a wire form instead (), so they may carry the combinators the wire cannot. + private static readonly string[] ValidationOnly = ["PlannerSchema.ResponseSchema"]; + + public static TheoryData WireFormNames => new(WireForms.Keys); + + [Theory] + [MemberData(nameof(WireFormNames))] + public void Every_schema_the_code_sends_to_a_model_is_combinator_free_on_the_wire(string schema) => + CombinatorPaths(WireForms[schema]).ShouldBeEmpty($"{schema} reaches the provider's constrained decoder; carry the combinator in a validation-only JsonSchema and send JsonSchemaCombinators.Strip of it as the WireJsonSchema"); + + [Fact] + public void Every_schema_constant_in_core_is_either_sent_on_the_wire_or_only_validates() + { + // The guard above is only as good as its list. Every schema a model is handed today is a static *Schema field, + // so a new one lands here and must be sorted: sent to a provider (combinator-free) or validation-only. + var declared = typeof(PlannerSchema).Assembly.GetTypes() + .SelectMany(type => type.GetFields(BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic).Where(field => field.FieldType == typeof(JsonElement) && field.Name.EndsWith("Schema", StringComparison.Ordinal)).Select(field => $"{type.Name}.{field.Name}")); + + declared.ShouldBe(WireForms.Keys.Concat(ValidationOnly), ignoreOrder: true); + } + + [Theory] + [InlineData("oneOf")] + [InlineData("anyOf")] + [InlineData("allOf")] + [InlineData("not")] + [InlineData("if")] + [InlineData("then")] + [InlineData("else")] + public void Strip_removes_a_combinator_at_every_schema_position_and_nothing_beside_it(string keyword) + { + var stripped = JsonSchemaCombinators.Strip(JsonDocument.Parse(SchemaAtEveryPosition(keyword)).RootElement); + + CombinatorPaths(stripped).ShouldBeEmpty(); + JsonNode.DeepEquals(JsonNode.Parse(stripped.GetRawText()), JsonNode.Parse(SchemaAtEveryPosition(keyword: null))).ShouldBeTrue("only the combinator goes; every sibling keyword stays exactly as written"); + } + + [Fact] + public void Strip_keeps_property_names_and_literal_values_that_happen_to_spell_a_combinator() + { + // A property called "not" is still a property: dropping its declaration under additionalProperties:false would + // forbid a field the contract allows, which is the one thing a wire schema may never do. Literals are data. + const string schema = """ + { + "type": "object", + "additionalProperties": false, + "properties": { "not": { "type": "string" }, "if": { "type": "boolean" }, "then": { "type": "object", "default": { "oneOf": 1 } } }, + "required": ["not", "if"], + "examples": [{ "not": "x", "if": true, "then": { "anyOf": [] } }] + } + """; + + JsonNode.DeepEquals(JsonNode.Parse(JsonSchemaCombinators.Strip(JsonDocument.Parse(schema).RootElement).GetRawText()), JsonNode.Parse(schema)).ShouldBeTrue(); + } + + /// Every path at which a combinator keyword appears as an object key, at ANY depth. Deliberately blind to what a key means, so it is stricter than the stripper it checks. + internal static IReadOnlyList CombinatorPaths(JsonElement node, string path = "$") => node.ValueKind switch + { + JsonValueKind.Object => node.EnumerateObject().SelectMany(property => (Combinators.Contains(property.Name) ? [$"{path}.{property.Name}"] : Array.Empty()).Concat(CombinatorPaths(property.Value, $"{path}.{property.Name}"))).ToArray(), + JsonValueKind.Array => node.EnumerateArray().SelectMany((item, index) => CombinatorPaths(item, $"{path}[{index}]")).ToArray(), + _ => [], + }; + + /// One schema with beside a sibling at every position a subschema can sit — or, for a null keyword, the same schema written without it. + private static string SchemaAtEveryPosition(string? keyword) + { + var value = keyword?.EndsWith("Of", StringComparison.Ordinal) == true ? """[{ "required": ["a"] }]""" : """{ "required": ["a"] }"""; + var at = keyword is null ? "" : $"\"{keyword}\": {value}, "; + + return $$""" + { + {{at}}"type": "object", + "properties": { "a": { {{at}}"type": "string" }, "list": { "type": "array", "items": { {{at}}"minLength": 1 }, "prefixItems": [{ {{at}}"maxLength": 9 }], "contains": { {{at}}"const": "x" } } }, + "patternProperties": { "^x-": { {{at}}"type": "number" } }, + "additionalProperties": { {{at}}"type": "boolean" }, + "propertyNames": { {{at}}"pattern": "^[a-z-]+$" }, + "dependentSchemas": { "a": { {{at}}"required": ["list"] } }, + "unevaluatedProperties": { {{at}}"type": "null" }, + "$defs": { "d": { {{at}}"type": "integer" } }, + "definitions": { "d": { {{at}}"type": "integer" } } + } + """; + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/StructuredResponseContractTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/StructuredResponseContractTests.cs index 9b625ddcd..0354e3cf1 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/StructuredResponseContractTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/StructuredResponseContractTests.cs @@ -406,6 +406,52 @@ public async Task The_reask_preamble_calls_a_fatal_miss_invalid_and_never_says_t advisory.Bodies[1].ShouldNotContain("previous (invalid) response", customMessage: "a degradable reply is not invalid; calling it that is a lie the model then acts on"); } + [Theory] + [InlineData("Anthropic")] + [InlineData("OpenAI")] + public async Task The_planner_request_sends_the_provider_a_combinator_free_schema_that_still_declares_every_acceptance_field(string provider) + { + // A hosted vLLM backend compiles the forced tool's schema into its decoding grammar. The per-kind oneOf + // branches (#1854) broke that compile: every planner call came back an EMPTY HTTP 500, the planner parked, + // and the live planner gates measured nothing on run after run while every other caller of the same model + // got 200s. The provider sees no combinator at any depth, and still sees every field an acceptance can carry. + var handler = new WireHandler(provider, [PlannerReply("TestsPass", ",\"argv\":[\"sh\"]")]); + + await Client(provider, handler).CompleteStructuredAsync(PlannerRequest(provider), CancellationToken.None); + + var sent = SentToolSchema(provider, handler.Bodies[0]); + JsonSchemaCombinatorsTests.CombinatorPaths(sent).ShouldBeEmpty("the provider's constrained decoder must never be handed a combinator"); + + var acceptance = sent.GetProperty("properties").GetProperty("subtasks").GetProperty("items").GetProperty("properties").GetProperty("acceptance").GetProperty("properties"); + foreach (var field in new[] { "formatVersion", "kind", "argv", "artifactPaths" }) + acceptance.TryGetProperty(field, out _).ShouldBeTrue($"the decoder can only emit acceptance.{field} if the wire schema declares it"); + } + + [Theory] + [InlineData("Anthropic")] + [InlineData("OpenAI")] + public async Task A_reply_the_wire_schema_admits_but_the_contract_rejects_still_earns_the_reask_and_the_fault(string provider) + { + // The wire schema relaxes only what the provider's decoder is told. The contract is still JsonSchema: a reply + // that satisfies the combinator-free wire form but breaks the combinator it dropped is re-asked and, if it + // stays broken, faulted exactly as before. + var request = new StructuredLLMCompletionRequest + { + Model = "wire-test-model", SystemPrompt = "Return data", UserPrompt = "Name exactly one of a or b", + JsonSchema = JsonDocument.Parse("""{"type":"object","oneOf":[{"required":["a"]},{"required":["b"]}]}""").RootElement, + WireJsonSchema = JsonDocument.Parse("""{"type":"object"}""").RootElement, + Credential = new ResolvedModelCredential { Provider = provider, ApiKey = "fixture-key" }, + }; + var handler = new WireHandler(provider, ["{}", "{}"]); + + var error = await Should.ThrowAsync(() => Client(provider, handler).CompleteStructuredAsync(request, CancellationToken.None)); + + error.Category.ShouldBe(LlmErrorCategory.Malformed); + handler.Bodies.Count.ShouldBe(2, "the combinator the wire dropped still earned its one re-ask"); + handler.Bodies[1].ShouldContain("oneOf requires exactly one matching schema"); + JsonSchemaCombinatorsTests.CombinatorPaths(SentToolSchema(provider, handler.Bodies[0])).ShouldBeEmpty("the provider was handed the wire schema, not the contract"); + } + /// The live regression shape: one subtask that names an oracle kind and authors NO payload for it — a consumer-contract defect the model-visible schema ALSO faults (no per-kind oneOf branch matches), which is why the two must be read as one severity. appends raw acceptance keys, so an EMPTY payload can be authored too. private static string PlannerReply(string kind, string payload = "") => PlannerReplyWithAcceptance("{\"formatVersion\":2,\"kind\":\"" + kind + "\"" + payload + "}"); @@ -434,6 +480,14 @@ private static StructuredLLMCompletionRequest PlannerRequest(string provider) => ResponseValidator = json => json.TryGetProperty("argv", out var argv) && argv.ValueKind == JsonValueKind.Array && argv.GetArrayLength() > 0 ? [] : ["consumer requires non-empty argv"], }; + /// The schema the forced tool call carried on the wire: Anthropic's tools[0].input_schema, the OpenAI wire's tools[0].function.parameters. + private static JsonElement SentToolSchema(string provider, string body) + { + var tool = JsonDocument.Parse(body).RootElement.GetProperty("tools")[0]; + + return provider == "Anthropic" ? tool.GetProperty("input_schema").Clone() : tool.GetProperty("function").GetProperty("parameters").Clone(); + } + private static IStructuredLLMClient Client(string provider, WireHandler handler) => provider == "Anthropic" ? new AnthropicClient(new Factory(handler)) : new OpenAiClient(new Factory(handler)); private sealed class Factory(HttpMessageHandler handler) : IHttpClientFactory { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs index 1d9771362..94945feda 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using System.Text.Json.Nodes; using CodeSpace.Core.Services.Tasks.SpecPreview; using CodeSpace.Core.Services.Workflows.Llm; using CodeSpace.Core.Services.Workflows.Planning; @@ -9,24 +10,45 @@ namespace CodeSpace.UnitTests.Workflows; public sealed class TypedModelSchemaBranchTests { [Fact] - public void Every_oracle_branch_is_self_describing_for_structured_output_generators() + public void The_generator_sees_every_field_an_oracle_branch_requires_on_one_flat_acceptance() { + // The per-kind branches no longer reach a structured-output generator: a hosted vLLM backend answered every + // planner call that carried them with an empty HTTP 500, so the provider is handed PlannerSchema.WireSchema. + // The branches still VALIDATE each reply — one per oracle kind — and the generator can only emit a field the + // wire declares, so every field any branch requires must be declared on the wire's flat acceptance. var branches = AcceptanceSchema().GetProperty("oneOf").EnumerateArray().ToArray(); + var kinds = AcceptanceSchema().GetProperty("properties").GetProperty("kind").GetProperty("enum").EnumerateArray().Select(kind => kind.GetString()).ToArray(); - branches.Length.ShouldBe(5); - foreach (var branch in branches) - { - var properties = branch.GetProperty("properties"); - var required = branch.GetProperty("required").EnumerateArray().Select(value => value.GetString()).ToArray(); - var kind = properties.GetProperty("kind").GetProperty("enum")[0].GetString(); - var payload = kind == "TestsPass" ? "argv" : "artifactPaths"; - - properties.TryGetProperty("formatVersion", out _).ShouldBeTrue("a generator may interpret a oneOf branch without merging its parent's properties"); - properties.TryGetProperty(payload, out _).ShouldBeTrue($"the {kind} branch must expose its required payload shape where that requirement is declared"); - required.ShouldContain("formatVersion"); - required.ShouldContain("kind"); - required.ShouldContain(payload); - } + branches.Select(branch => branch.GetProperty("properties").GetProperty("kind").GetProperty("enum")[0].GetString()).ShouldBe(kinds, ignoreOrder: true, "one validating branch per oracle kind"); + + var wire = WireAcceptanceSchema(); + wire.TryGetProperty("oneOf", out _).ShouldBeFalse(); + + foreach (var field in branches.SelectMany(branch => branch.GetProperty("required").EnumerateArray()).Select(name => name.GetString()!).Distinct()) + wire.GetProperty("properties").TryGetProperty(field, out _).ShouldBeTrue($"a branch requires '{field}', so the wire acceptance must declare it or the generator can never emit it"); + + wire.GetProperty("required").EnumerateArray().Select(name => name.GetString()).ShouldBe(new[] { "formatVersion", "kind" }, "the requirement every branch shares stays on the wire"); + } + + [Fact] + public void The_wire_schema_is_the_validation_schema_minus_only_the_acceptance_branches() + { + // Dropping a combinator only removes a constraint, so this equality is what makes the wire a SUPERSET of the + // contract: it can never forbid a reply the validation schema accepts, and it lost nothing else on the way. + var expected = JsonNode.Parse(PlannerSchema.ResponseSchema.GetRawText())!; + expected["properties"]!["subtasks"]!["items"]!["properties"]!["acceptance"]!.AsObject().Remove("oneOf"); + + JsonNode.DeepEquals(JsonNode.Parse(PlannerSchema.WireSchema.GetRawText()), expected).ShouldBeTrue(); + } + + [Theory] + [MemberData(nameof(ValidOracles))] + public void Every_valid_oracle_reply_is_also_valid_on_the_wire(string acceptance) + { + var reply = JsonDocument.Parse($$"""{"goal":"g","subtasks":[{"id":"s1","title":"t","instruction":"i","acceptance":{{acceptance}}}]}""").RootElement; + + JsonSchemaValidator.Validate(reply, PlannerSchema.ResponseSchema).ShouldBeEmpty("precondition: the validation schema accepts this reply"); + JsonSchemaValidator.Validate(reply, PlannerSchema.WireSchema).ShouldBeEmpty("the wire schema may never forbid a reply the validation schema accepts"); } [Theory] @@ -51,12 +73,15 @@ public void The_model_visible_schema_itself_requires_the_selected_oracle_payload public void A_present_but_empty_conflicting_or_incomplete_payload_does_not_satisfy_the_model_schema(string response) => JsonSchemaValidator.Validate(JsonDocument.Parse(response).RootElement, AcceptanceSchema()).ShouldNotBeEmpty(); + public static TheoryData ValidOracles => new( + """{"formatVersion":2,"kind":"TestsPass","argv":["sh",""," ","Δ"]}""", + """{"formatVersion":2,"kind":"ArtifactPresent","artifactPaths":["out.txt"]}""", + """{"formatVersion":2,"kind":"CitationsResolve","artifactPaths":["out.txt"]}""", + """{"formatVersion":2,"kind":"LlmJudge","artifactPaths":["out.txt"],"rubric":{"criteria":[{"id":"a","requirement":"explains findings"}]}}""", + """{"formatVersion":2,"kind":"ArtifactSchema","artifactPaths":["out.txt"],"schema":{"type":"object"}}"""); + [Theory] - [InlineData("""{"formatVersion":2,"kind":"TestsPass","argv":["sh",""," ","Δ"]}""")] - [InlineData("""{"formatVersion":2,"kind":"ArtifactPresent","artifactPaths":["out.txt"]}""")] - [InlineData("""{"formatVersion":2,"kind":"CitationsResolve","artifactPaths":["out.txt"]}""")] - [InlineData("""{"formatVersion":2,"kind":"LlmJudge","artifactPaths":["out.txt"],"rubric":{"criteria":[{"id":"a","requirement":"explains findings"}]}}""")] - [InlineData("""{"formatVersion":2,"kind":"ArtifactSchema","artifactPaths":["out.txt"],"schema":{"type":"object"}}""")] + [MemberData(nameof(ValidOracles))] public void Every_valid_oracle_remains_expressible_without_altering_its_data(string response) => JsonSchemaValidator.Validate(JsonDocument.Parse(response).RootElement, AcceptanceSchema()).ShouldBeEmpty(); @@ -105,6 +130,8 @@ public void Alternative_schemas_enforce_their_actual_matching_semantics(string s private static JsonElement AcceptanceSchema() => PlannerSchema.ResponseSchema.GetProperty("properties").GetProperty("subtasks").GetProperty("items").GetProperty("properties").GetProperty("acceptance"); + private static JsonElement WireAcceptanceSchema() => PlannerSchema.WireSchema.GetProperty("properties").GetProperty("subtasks").GetProperty("items").GetProperty("properties").GetProperty("acceptance"); + [Fact] public void Deeply_branching_schema_validation_is_bounded_and_cannot_turn_exhaustion_into_success() {