From a93ec3152f75f1e730da33b70c9b311f034be743 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Mon, 12 Oct 2026 08:45:11 +0800 Subject: [PATCH] Declare planner acceptance kind ahead of its payload fields The planner's acceptance object listed argv, oraclePaths and artifactPaths before kind. The gateway compiles the wire schema into a decoding grammar, and a grammar-constrained decoder is expected to write properties in declared order: it has to decide on a payload before it has chosen the oracle that says which one is owed. Both payloads are optional, so it skips them, then writes kind and cannot go back. Every acceptance on every main real-model run since the combinator-free wire schema landed was dropped with "requires a non-empty argv|artifactPaths array" while formatVersion was present, so the subtasks ran ungraded. Reorder to formatVersion, kind, argv, artifactPaths, rubric, schema, oraclePaths, description. Each property definition is byte-identical and the wire schema inherits the order through Strip. The cassette key and the planner schema digest both canonicalize with sorted keys, so neither moves. The decoder behaviour is a hypothesis; the live planner arm is what measures it. That arm could not pass as written: a planner-authored ArtifactPresent is promoted to the content oracle it is paired with, or dropped as self-certifying, so a bare ArtifactPresent never survives to be asserted. It now asks for a JSON deliverable paired with a schema, asserts the promoted ArtifactSchema, and reports a payload omitted by the model separately from a drop by the self-certifying policy. --- .../Workflows/Planning/PlannerSchema.cs | 4 +- .../RealModelPlannerAcceptanceE2ETests.cs | 75 +++++++++++++++---- .../Workflows/TypedModelSchemaBranchTests.cs | 23 ++++++ 3 files changed, 84 insertions(+), 18 deletions(-) diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs b/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs index 333a44e2d..6cefffd93 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Planning/PlannerSchema.cs @@ -47,12 +47,12 @@ public static class PlannerSchema "additionalProperties": false, "properties": { "formatVersion": { "type": "integer", "enum": [2], "description": "Typed acceptance wire version. Always 2; legacy command payloads are not accepted from a fresh planner response." }, + "kind": { "type": "string", "enum": ["TestsPass", "ArtifactPresent", "LlmJudge", "CitationsResolve", "ArtifactSchema"], "description": "Required oracle choice based on the evidence needed, independently of task type. TestsPass executes argv and requires exit 0; ArtifactPresent checks file existence; LlmJudge evaluates file content against rubric; CitationsResolve checks citations; ArtifactSchema validates file content against schema. File existence cannot replace a required behavioral or content check. ArtifactPresent is kept only for an operator-declared deliverable path paired with its own rubric or schema; otherwise it is dropped and the subtask is graded by nothing." }, "argv": { "type": "array", "minItems": 1, "items": { "type": "string" }, "description": "Required only for TestsPass. Exact executable followed by its individual arguments; preserve empty and whitespace arguments. Never shell-split or put file obligations here. Omit artifactPaths." }, - "oraclePaths": { "type": "array", "items": { "type": "string" }, "description": "Optional exact relative oracle file paths already present before execution in a local repository-free workspace. The server hashes these explicit files before the agent runs and rejects any changed bytes; no symlinks, globs, git pathspecs or inferred shell dependencies. These are judge inputs, not output artifact obligations." }, "artifactPaths": { "type": "array", "minItems": 1, "items": { "type": "string" }, "description": "Required for ArtifactPresent, LlmJudge, CitationsResolve or ArtifactSchema. Workspace-relative deliverable file paths that this oracle reads. These are literal paths, not executable names, shell commands or argv. Omit argv." }, - "kind": { "type": "string", "enum": ["TestsPass", "ArtifactPresent", "LlmJudge", "CitationsResolve", "ArtifactSchema"], "description": "Required oracle choice based on the evidence needed, independently of task type. TestsPass executes argv and requires exit 0; ArtifactPresent checks file existence; LlmJudge evaluates file content against rubric; CitationsResolve checks citations; ArtifactSchema validates file content against schema. File existence cannot replace a required behavioral or content check. ArtifactPresent is kept only for an operator-declared deliverable path paired with its own rubric or schema; otherwise it is dropped and the subtask is graded by nothing." }, "rubric": { "type": "object", "additionalProperties": false, "properties": { "criteria": { "type": "array", "minItems": 1, "items": { "type": "object", "additionalProperties": false, "properties": { "id": { "type": "string" }, "requirement": { "type": "string", "description": "A concrete requirement judgeable on the deliverable alone, e.g. 'names at least three competitors with sources'." }, "weight": { "type": "number", "description": "Relative weight (omit for 1)." } }, "required": ["id", "requirement"] } }, "threshold": { "type": "number", "description": "Weighted met-fraction (0..1] required to pass. Omit for 1.0 — every criterion." } }, "required": ["criteria"], "description": "REQUIRED for kind=LlmJudge: the weighted BINARY criteria an independent judge grades the deliverables against." }, "schema": { "type": "object", "additionalProperties": true, "description": "REQUIRED for kind=ArtifactSchema: the JSON schema each deliverable file must validate against (required / type / enum / nested properties+items)." }, + "oraclePaths": { "type": "array", "items": { "type": "string" }, "description": "Optional exact relative oracle file paths already present before execution in a local repository-free workspace. The server hashes these explicit files before the agent runs and rejects any changed bytes; no symlinks, globs, git pathspecs or inferred shell dependencies. These are judge inputs, not output artifact obligations." }, "description": { "type": "string", "description": "Optional human-readable description of the subtask's acceptance check." } }, "required": ["formatVersion", "kind"], diff --git a/backend/tests/CodeSpace.E2ETests/Workflows/RealModelPlannerAcceptanceE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Workflows/RealModelPlannerAcceptanceE2ETests.cs index 2db080e7a..b7c92b2ca 100644 --- a/backend/tests/CodeSpace.E2ETests/Workflows/RealModelPlannerAcceptanceE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Workflows/RealModelPlannerAcceptanceE2ETests.cs @@ -23,17 +23,25 @@ namespace CodeSpace.E2ETests.Workflows; /// /// Report-only (gating: false) until it has a passing history — this repo's rule for a NEW arm, and /// this one has no passing run at all. Every live attempt so far has ended in the same place: the model's reply carried -/// no acceptance argv (and no formatVersion), so the typed contract refused to bind it. An arm that has -/// never once passed cannot tell a regression from its own unmet precondition, so gating it only reds the blessed wire -/// for what the model has always done. The verdict is still REPORTED on every run — the contract-miss note included — -/// so the moment the model starts binding, the passing history the rule asks for exists in the job summaries, and the -/// arm can be promoted by a one-word change here. +/// formatVersion but omitted the acceptance payload (argv / artifactPaths), so the typed contract +/// refused to bind it. The wire schema now declares kind ahead of every property that depends on it, because a +/// grammar-constrained decoder writes properties in declared order and so had to decide on the payload before it had +/// chosen the oracle; this arm is where that ordering is measured against a real model. An arm that has never once +/// passed cannot tell a regression from its own unmet precondition, so gating it only reds the blessed wire for what the +/// model has always done. The verdict is still REPORTED on every run — the contract-miss note included — so the moment +/// the model starts binding, the passing history the rule asks for exists in the job summaries, and the arm can be +/// promoted by a one-word change here. /// /// That precondition now arrives as DATA rather than as an exception: an acceptance the contract cannot bind is /// dropped and recorded on the plan instead of failing the planner. The miss is therefore read off /// PlannedWorkflow.DroppedAcceptances and returned as the same informational verdict — otherwise the very /// degrade that keeps live plans alive would turn this arm's oldest known precondition into a hard assertion red. /// +/// The file obligation is a JSON deliverable paired with a schema, not a bare existence check: a planner-authored +/// ArtifactPresent is never kept as authored (PlannerAcceptanceDraft.ReconcileArtifactPresent promotes it +/// to the content oracle it is paired with, or drops it as self-certifying), so the oracle this arm grades is the +/// promoted ArtifactSchema, which only validates JSON. +/// /// Only its GATING is soft. Everything the arm asserts with Shouldly still reds: RealModelGate excludes a /// ShouldAssertException from the report-only catch deliberately, so the grader facts below — the oracle /// accepting the correct fixture and rejecting the wrong one, the persisted evidence artifact — keep their hard @@ -71,7 +79,7 @@ await RealModelGate.AssessLiveAsync("Anthropic", async () => TaskText = "Author exactly two independent subtasks for validating a prepared report.txt file. " + "One subtask MUST use TestsPass to verify the file's entire content equals accepted (with optional trailing newline), and return nonzero on incorrect content. " + "Choose an exact read-only argv using the available /bin/sh, cat, test and printf commands; do not write or repair files. " - + "The second subtask MUST independently require report.txt to exist using ArtifactPresent. " + + "The second subtask MUST independently require a separate deliverable report.json to exist as a JSON object whose status property is the string accepted: use ArtifactPresent for report.json paired with a schema that requires status to be a string whose only allowed value is accepted (a bare existence check is not kept). " + "These are distinct obligations: mere file presence cannot establish correct content. Both acceptance objects are required. " + "The checks will be graded in an explicitly prepared directory. Do not claim that repository-free agent execution or dependency readiness has been verified.", }, CancellationToken.None); @@ -89,15 +97,21 @@ await RealModelGate.AssessLiveAsync("Anthropic", async () => // The same verdict, for the same miss, now that it no longer throws: an acceptance the typed contract // could not bind is dropped and NAMED on the plan. Returning it keeps the arm's hard assertions about // GRADING hard while leaving the model's own contract binding soft, which is the whole demotion. - if (plan.DroppedAcceptances is { Count: > 0 } dropped) - return (false, $"the live planner authored {dropped.Count} acceptance(s) the typed contract could not bind, even after the provider's bounded re-ask — {string.Join("; ", dropped.Select(drop => $"{drop.SubtaskId} ({drop.Kind}): {drop.Reason}"))}"); + if (plan.DroppedAcceptances is { Count: > 0 } dropped) return (false, DescribeDrops(dropped)); plan.AuthoredByModel.ShouldNotBeNullOrWhiteSpace("the response must come from the pinned real provider path"); plan.Subtasks.Count.ShouldBe(2); plan.Subtasks.ShouldAllBe(item => item.Acceptance != null); - var command = plan.Subtasks.Single(item => item.Acceptance!.Kind == BenchmarkGradingKind.TestsPass).Acceptance!; - var files = plan.Subtasks.Single(item => item.Acceptance!.Kind == BenchmarkGradingKind.ArtifactPresent).Acceptance!; - files.Command.ShouldBe(new[] { "report.txt" }, "the model must author literal file obligations, not test/-f argv in the path field"); + plan.Subtasks.ShouldNotContain(item => item.Acceptance!.Kind == BenchmarkGradingKind.ArtifactPresent, "ReconcileArtifactPresent either promotes an ArtifactPresent to its paired content oracle or drops it by name, so none survives a plan that dropped nothing"); + + var command = plan.Subtasks.FirstOrDefault(item => item.Acceptance!.Kind == BenchmarkGradingKind.TestsPass)?.Acceptance; + var content = plan.Subtasks.FirstOrDefault(item => item.Acceptance!.Kind == BenchmarkGradingKind.ArtifactSchema)?.Acceptance; + + if (command is null || content is null) + return (false, $"the live planner's acceptance kinds were [{string.Join(", ", plan.Subtasks.Select(item => item.Acceptance!.Kind))}] where one TestsPass and one ArtifactSchema (an ArtifactPresent paired with a schema, promoted) were asked for"); + + content.Command.ShouldBe(new[] { "report.json" }, "the model must author literal file obligations, not test/-f argv in the path field"); + content.Schema.ShouldNotBeNull("an ArtifactSchema oracle only exists when a schema was authored alongside it"); command.Command.Count.ShouldBeGreaterThan(0); var directory = Path.Combine(Path.GetTempPath(), $"cs-live-planner-grade-{Guid.NewGuid():N}"); @@ -105,28 +119,57 @@ await RealModelGate.AssessLiveAsync("Anthropic", async () => try { var report = Path.Combine(directory, "report.txt"); + var data = Path.Combine(directory, "report.json"); await File.WriteAllTextAsync(report, "accepted\n"); + await File.WriteAllTextAsync(data, """{"status":"accepted"}"""); var grader = scope.Resolve(); var correct = await grader.GradeDirectoryAsync(directory, command, teamId, 20, CancellationToken.None); correct.Passed.ShouldBeTrue(correct.Detail); correct.EvidenceArtifactId.ShouldNotBeNull("an actual command verdict carries persisted oracle evidence"); - (await grader.GradeDirectoryAsync(directory, files, teamId, 20, CancellationToken.None)).Passed.ShouldBeTrue(); + + // The schema is the MODEL's own authoring, so a schema that rejects the very document the task describes is + // a model-quality miss reported like any other, not a grader fault. + var accepted = await grader.GradeDirectoryAsync(directory, content, teamId, 20, CancellationToken.None); + if (!accepted.Passed) return (false, $"the live planner's ArtifactSchema rejected the correct report.json — {accepted.Detail}"); await File.WriteAllTextAsync(report, "incorrect\n"); var wrong = await grader.GradeDirectoryAsync(directory, command, teamId, 20, CancellationToken.None); wrong.Passed.ShouldBeFalse("the exact same model-authored command must reject the negative fixture"); wrong.Class.ShouldBe(GradeFailureClass.Genuine); wrong.EvidenceArtifactId.ShouldNotBeNull(); - (await grader.GradeDirectoryAsync(directory, files, teamId, 20, CancellationToken.None)).Passed.ShouldBeTrue(); - File.Delete(report); - (await grader.GradeDirectoryAsync(directory, files, teamId, 20, CancellationToken.None)).Passed.ShouldBeFalse(); + (await grader.GradeDirectoryAsync(directory, content, teamId, 20, CancellationToken.None)).Passed.ShouldBeTrue("the report.json obligation is independent of report.txt's content"); + File.Delete(data); + (await grader.GradeDirectoryAsync(directory, content, teamId, 20, CancellationToken.None)).Passed.ShouldBeFalse("a missing report.json must fail the schema oracle closed"); } finally { Directory.Delete(directory, recursive: true); } - return (true, $"the live planner authored a {command.Command.Count}-token TestsPass argv that accepted the correct fixture and rejected the wrong one, plus an independent ArtifactPresent obligation on report.txt"); + return (true, $"the live planner authored a {command.Command.Count}-token TestsPass argv that accepted the correct fixture and rejected the wrong one, plus an independent ArtifactSchema obligation on report.json (an ArtifactPresent paired with a schema, promoted) that accepted the correct document and failed closed once the file was gone"); }, gating: false); } + /// + /// The plan's dropped acceptances, told apart by WHO dropped them — the two mean different things to a reader. A + /// payload omitted drop is the model's reply failing the typed contract (the bind refused it, even after the + /// provider's bounded re-ask). A P2.6 policy drop is a reply that BOUND fine: it authored a bare + /// ArtifactPresent (or one for an undeclared path), which the server refuses to keep as self-certifying. + /// + private static string DescribeDrops(IReadOnlyList dropped) + { + var policy = dropped.Where(IsPolicyDrop).ToList(); + var omitted = dropped.Where(drop => !IsPolicyDrop(drop)).ToList(); + var parts = new List(); + + if (omitted.Count > 0) parts.Add($"{omitted.Count} acceptance(s) the typed contract could not bind (payload omitted or malformed), even after the provider's bounded re-ask — {JoinDrops(omitted)}"); + if (policy.Count > 0) parts.Add($"{policy.Count} ArtifactPresent acceptance(s) that bound but were dropped by the P2.6 self-certifying policy (no paired ArtifactSchema or LlmJudge content check) — {JoinDrops(policy)}"); + + return "the live planner authored " + string.Join(", and ", parts); + } + + /// Both P2.6 refusals (PlannerAcceptanceDraft.InadmissibleArtifactPresentReason) open with this phrase; no bind-level reason does. + private static bool IsPolicyDrop(DroppedAcceptance drop) => drop.Reason.StartsWith("Planner acceptance ArtifactPresent for '", StringComparison.Ordinal); + + private static string JoinDrops(IEnumerable dropped) => string.Join("; ", dropped.Select(drop => $"{drop.SubtaskId} ({drop.Kind}): {drop.Reason}")); + /// /// Whether is the live planner's REPLY failing the PLAN contract, rather than infrastructure /// breaking underneath it. Two shapes, both meaning "the model could not author a conforming plan": the provider's diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs index bceb9359c..1c4dda673 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TypedModelSchemaBranchTests.cs @@ -47,6 +47,29 @@ public void The_wire_schema_is_the_validation_schema_minus_only_the_acceptance_b JsonNode.DeepEquals(JsonNode.Parse(PlannerSchema.WireSchema.GetRawText()), expected).ShouldBeTrue(); } + /// + /// A grammar-constrained decoder writes an object's properties in the order the schema declares them, so it must + /// decide on argv / artifactPaths before it has written the kind that says which one is owed, + /// skips both as optional, then writes kind and can never go back — the payload every planner-authored oracle + /// needs is gone and the subtask runs ungraded. kind therefore has to be declared first. Run against the + /// validation schema AND the wire schema the provider is actually handed (which is derived from it). + /// + [Theory] + [InlineData("validation")] + [InlineData("wire")] + public void On_the_wire_acceptance_kind_precedes_every_kind_dependent_property(string schema) + { + var acceptance = schema == "wire" ? WireAcceptanceSchema() : AcceptanceSchema(); + var declared = acceptance.GetProperty("properties").EnumerateObject().Select(property => property.Name).ToArray(); + + declared.ShouldBe(new[] { "formatVersion", "kind", "argv", "artifactPaths", "rubric", "schema", "oraclePaths", "description" }, ignoreOrder: true, "fixture check: all eight acceptance properties are declared, so the order assertions below cannot pass on an empty object"); + + var kindAt = Array.IndexOf(declared, "kind"); + + foreach (var dependent in new[] { "argv", "artifactPaths", "rubric", "schema", "oraclePaths" }) + Array.IndexOf(declared, dependent).ShouldBeGreaterThan(kindAt, $"'{dependent}' is declared before 'kind' on the {schema} acceptance, so a decoder that writes properties in declared order commits to it before it knows which oracle it is writing"); + } + [Theory] [MemberData(nameof(ValidOracles))] public void Every_valid_oracle_reply_is_also_valid_on_the_wire(string acceptance)