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)