diff --git a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/PlanMap/PlanMapBuilderBase.cs b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/PlanMap/PlanMapBuilderBase.cs index b5583cc3f..80ce9070e 100644 --- a/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/PlanMap/PlanMapBuilderBase.cs +++ b/backend/src/CodeSpace.Core/Services/Tasks/Projection/Builders/PlanMap/PlanMapBuilderBase.cs @@ -253,17 +253,34 @@ private static JsonElement SynthConfig(TaskBuildContext context) /// is the identical serialization of the identical array, so an ordinary run's prompt does not change by a /// character; over budget the model is handed a fair share of every included branch and TOLD, in the prompt's /// first sentence, that it is reading an excerpt. + /// + /// A repo-bound graph's reduce is ALSO handed the integrate node's own account of what landed + /// () and told what to do with it (). + /// The integrate step's outputs used to reach only : nothing the reduce reads said that a + /// candidate conflicted, or that a unit whose own check rejected it was withheld from it, so a partial integration + /// was narrated as a whole deliverable. Whether the graph integrates is a BUILD-time fact (the integrate node exists + /// or it does not), so the conditional is legitimate here — and a repo-less graph keeps both halves of its prompt + /// byte-for-byte. The account itself is a RUN-time fact the node renders; the prompt only binds it. It sits OUTSIDE + /// the map's promptBudgetChars, which bounds the results projection alone — the node bounds the account + /// instead (RunIntegrationSummary.MaxChars, 2,000 characters against a 120,000-character default budget). /// - private static JsonElement SynthInputs(TaskBuildContext context) => JsonSerializer.SerializeToElement(new + private static JsonElement SynthInputs(TaskBuildContext context) { - systemPrompt = SynthSystemPrompt, - // BYTE-IDENTICAL to the pre-continue prompt: the data half of the reduce carries the goal and the results, - // and nothing else. The failure half rides the SYSTEM prompt instead, so a run in which nothing failed is - // never handed a "Subtasks that failed: 0" line — the reduce learns about a failure from the marker actually - // sitting in its results, which is the only place the fact exists per-run. (A build-time conditional cannot - // express this: the failure count is a RUN-time fact and the prompt is frozen into the definition.) - userPrompt = $"Goal: {context.Seed.Goal}\n\nPer-subtask results:\n" + SynthResultsRef, - }); + var integrates = context.AgentProfile?.RepositoryId is not null; + + return JsonSerializer.SerializeToElement(new + { + systemPrompt = integrates ? SynthSystemPrompt + SynthIntegrationInstruction : SynthSystemPrompt, + // BYTE-IDENTICAL to the pre-continue prompt on a repo-less graph: the data half of the reduce carries the + // goal and the results, and nothing else. The failure half rides the SYSTEM prompt instead, so a run in + // which nothing failed is never handed a "Subtasks that failed: 0" line — the reduce learns about a failure + // from the marker actually sitting in its results, which is the only place the fact exists per-run. (A + // build-time conditional cannot express this: the failure count is a RUN-time fact and the prompt is frozen + // into the definition.) A repo-bound graph also carries what landed — on a clean run ONE factual sentence — + // because that is the fact the reduce needs in order to call the work delivered, not failure furniture. + userPrompt = $"Goal: {context.Seed.Goal}\n\nPer-subtask results:\n" + SynthResultsRef + (integrates ? IntegrationOutcomeSection : ""), + }); + } /// /// The reduce's instruction. The failure clause is what continue-on-error requires of it: the run now reaches @@ -278,6 +295,19 @@ private static JsonElement SynthInputs(TaskBuildContext context) => JsonSerializ + "A subtask that FAILED appears in the results as an {\"error\": ...} entry instead of a result: never present its work as done — " + "say which subtasks failed, what they were meant to deliver, and what is therefore missing from the answer."; + /// + /// What a REPO-BOUND reduce is told about the integration outcome it is shown, appended to + /// only when the graph has an integrate node — a repo-less reduce has no such + /// outcome, and a sentence about one would only invite the model to invent it. The two verbs carry the honesty + /// contract: what landed is stated as delivered, and anything conflicted or withheld is named as NOT delivered. + /// + internal const string SynthIntegrationInstruction = + " The integration outcome tells you what actually landed on the integrated branch; state what landed, " + + "and name anything conflicted or withheld as NOT delivered — never narrate withheld work as done."; + + /// The data half's integration section (repo-bound graphs only): the integrate node's own rendering of what landed, bound whole into the prompt. The key is one git.integrate_run declares in its OutputSchema, which DefinitionValidator enforces at build. + private const string IntegrationOutcomeSection = "\n\nIntegration outcome:\n{{nodes.integrate.outputs.summary}}"; + /// The reduce's results binding, composed from so the prompt and the key the reducer writes cannot drift apart. private const string SynthResultsRef = "{{nodes.map.outputs." + WorkflowOutputKeys.MapResultsPrompt + "}}"; diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateNode.cs index 14cdf9176..c370855da 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateNode.cs @@ -156,12 +156,14 @@ private static string BuildIntegrationBranch(NodeRunContext context) => ["appliedCount"] = JsonSerializer.SerializeToElement(result.AppliedCount), ["reason"] = JsonSerializer.SerializeToElement(result.Reason), ["conflicts"] = JsonSerializer.SerializeToElement( - result.Outcomes - .Where(o => o.Disposition != ContributionDisposition.Applied) - .OrderBy(o => o.Skipped) + NotApplied(result) .Select(o => new { label = o.Label, disposition = o.Disposition.ToString(), reason = o.Reason, conflictedFiles = o.ConflictedFiles, fallbackBranch = o.FallbackBranch, skipped = o.Skipped })), }; + /// The outcomes that did NOT apply, a real failure before a bystander (stable — ties keep outcome order). The one ordering conflicts[] above and git.integrate_run's prose summary both read, so what a reader is told first never depends on which of the two it reads. + internal static IEnumerable NotApplied(IntegrationResult result) => + result.Outcomes.Where(o => o.Disposition != ContributionDisposition.Applied).OrderBy(o => o.Skipped); + private static IReadOnlyList ReadContributions(NodeRunContext context) { if (!context.Inputs.TryGetValue("contributions", out var value) || value.ValueKind != JsonValueKind.Array) diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateRunNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateRunNode.cs index e17c191c3..3a78ee3c3 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateRunNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitIntegrateRunNode.cs @@ -31,6 +31,13 @@ namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; /// resumed pass re-derives and RE-INTEGRATES against then-current facts (the human may have pushed a fix or a /// reconciled branch), so an approve after a repair lands the Clean candidate; a still-conflicted retry /// completes honestly with the review trail on its outputs — one park per run, never a loop. +/// +/// The node owns the NARRATIVE of its own outcome. Every pass — clean, conflicted, skipped, resumed — emits +/// summary (: what actually landed, what conflicted and where its work is +/// kept) and withheld (: the units the head gate kept off the +/// candidate because their own definition-of-done rejected them — dropped BEFORE integration, so in no outcome). +/// The plan-map synth is handed the summary: before it, the reduce narrated a partial integration as a whole +/// deliverable, because appliedCount / conflicts / reason reached nothing it reads. /// public sealed class GitIntegrateRunNode : INodeRuntime { @@ -79,7 +86,7 @@ public GitIntegrateRunNode(IBranchIntegrator integrator, IAgentWorkspaceResolver "required": ["repositoryId"] } """), - OutputSchema = SchemaBuilder.Parse(""" + OutputSchema = SchemaBuilder.Parse($$""" { "type": "object", "properties": { @@ -87,7 +94,12 @@ public GitIntegrateRunNode(IBranchIntegrator integrator, IAgentWorkspaceResolver "integratedBranch": { "type": ["string","null"] }, "appliedCount": { "type": "integer" }, "reason": { "type": ["string","null"] }, - "conflicts": { "type": "array" } + "conflicts": { "type": "array" }, + "summary": { "type": "string", "maxLength": {{RunIntegrationSummary.MaxChars}}, "description": "One account of what actually landed on the integrated branch, what conflicted or was skipped (and where its work is kept), and what was withheld — produced on every pass, and read by the plan-map synth." }, + "withheld": { "type": "array", "items": { "type": "object", "properties": { "label": { "type": "string" }, "reason": { "type": "string" } } }, "description": "The units kept off the candidate BEFORE integration because their own acceptance check failed or was waived — in no other output. Empty when nothing was withheld." }, + "reviewApproved": { "type": "boolean", "description": "Resumed pass only: the human's verdict on the parked conflict." }, + "reviewComment": { "type": "string", "description": "Resumed pass only: the reviewer's comment." }, + "reviewedBy": { "type": "string", "description": "Resumed pass only: who reviewed the parked conflict." } } } """) @@ -102,9 +114,10 @@ public async Task RunAsync(NodeRunContext context, CancellationToken var manifests = await _manifests.ListForWorkflowRunAsync(runId, teamId, cancellationToken).ConfigureAwait(false); var agentWork = await LoadAgentWorkAsync(runId, teamId, cancellationToken).ConfigureAwait(false); var contributions = RunIntegrationContributions.Build(repoId, manifests, agentWork); + var withheld = RunIntegrationContributions.Withheld(repoId, manifests, agentWork); if (contributions.Count == 0) - return NodeResult.Ok(SkippedOutputs("the run produced no integrable work for this repository")); + return NodeResult.Ok(SkippedOutputs("the run produced no integrable work for this repository", withheld)); // The ancestor-most base, not the first contribution's: a withheld producer is dropped from the contributions // while its manifest row survives, so the run's root lives in the ledger even when the surviving contributions @@ -112,7 +125,7 @@ public async Task RunAsync(NodeRunContext context, CancellationToken var baseSha = IntegrationBaseAnchor.Resolve(manifests, repoId, contributions.Select(c => c.BaseSha).FirstOrDefault(sha => !string.IsNullOrEmpty(sha))); if (string.IsNullOrEmpty(baseSha)) - return NodeResult.Ok(SkippedOutputs("the produced work recorded no base revision to integrate from")); + return NodeResult.Ok(SkippedOutputs("the produced work recorded no base revision to integrate from", withheld)); WorkspaceRequest? workspace; try @@ -165,7 +178,7 @@ public async Task RunAsync(NodeRunContext context, CancellationToken context.Logger.LogInformation("git.integrate_run on repo {RepoId}: {Status} ({Applied}/{Total} applied)", repoId, result.Status, result.AppliedCount, contributions.Count); - var outputs = GitIntegrateNode.ProjectOutputs(result); + var outputs = WithNarrative(GitIntegrateNode.ProjectOutputs(result), RunIntegrationSummary.ForResult(result, withheld), withheld); // The review trail rides the outputs on the resumed pass — who looked, what they said — so the terminal // (and any downstream consumer) sees the conflict was REVIEWED, never silently narrated past. @@ -219,14 +232,28 @@ private async Task> LoadAgentWorkAsync(Guid runId, G .Select(r => new RunAgentWork(r.Id, r.NodeId, r.IterationKey, r.CreatedDate, r.ResultJson, r.TaskJson)) .ToList(); - private static Dictionary SkippedOutputs(string reason) => new() + private static Dictionary SkippedOutputs(string reason, IReadOnlyList withheld) { - ["status"] = JsonSerializer.SerializeToElement("Skipped"), - ["integratedBranch"] = JsonSerializer.SerializeToElement((string?)null), - ["appliedCount"] = JsonSerializer.SerializeToElement(0), - ["reason"] = JsonSerializer.SerializeToElement(reason), - ["conflicts"] = JsonSerializer.SerializeToElement(Array.Empty()), - }; + var outputs = new Dictionary + { + ["status"] = JsonSerializer.SerializeToElement("Skipped"), + ["integratedBranch"] = JsonSerializer.SerializeToElement((string?)null), + ["appliedCount"] = JsonSerializer.SerializeToElement(0), + ["reason"] = JsonSerializer.SerializeToElement(reason), + ["conflicts"] = JsonSerializer.SerializeToElement(Array.Empty()), + }; + + return WithNarrative(outputs, RunIntegrationSummary.ForSkipped(reason, withheld), withheld); + } + + /// The outcome's prose and the units kept off the candidate, added to EVERY pass's outputs — the one seam, so no arm (clean, conflicted, skipped, resumed) can omit what the synth reads. + private static Dictionary WithNarrative(Dictionary outputs, string summary, IReadOnlyList withheld) + { + outputs["summary"] = JsonSerializer.SerializeToElement(summary); + outputs["withheld"] = JsonSerializer.SerializeToElement(withheld.Select(w => new { label = w.Label, reason = w.Reason })); + + return outputs; + } private static bool TryReadGuid(NodeRunContext context, string key, out Guid id) { diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationContributions.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationContributions.cs index 156a7a991..0deeac336 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationContributions.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationContributions.cs @@ -48,14 +48,7 @@ public static class RunIntegrationContributions { public static IReadOnlyList Build(Guid repositoryId, IReadOnlyList manifests, IReadOnlyList agentWork) { - var workByRunId = agentWork.ToDictionary(w => w.AgentRunId); - - var produced = manifests - .Where(m => m.Kind == PublishManifestKind.Agent && m.AgentRunId is not null && m.RepositoryId == repositoryId && m.PublishStateValue != PublishState.None) - .Where(m => !IsWithheldFromHead(m)) - .Select(m => (Manifest: m, Work: workByRunId.GetValueOrDefault(m.AgentRunId!.Value))) - .Where(pair => pair.Work is not null) - .Select(pair => (pair.Manifest, Work: pair.Work!)); + var produced = ProducedRows(repositoryId, manifests, agentWork).Where(pair => !IsWithheldFromHead(pair.Manifest)); return LatestAttemptPerUnit(produced) .OrderBy(pair => pair.Work.CreatedDate).ThenBy(pair => pair.Work.AgentRunId) @@ -71,6 +64,56 @@ public static IReadOnlyList Build(Guid repositoryId, IReadOn .ToList(); } + /// + /// The other face of : the units the head gate kept OUT of 's + /// contributions, reported instead of discarded. A withheld unit reaches no integrator outcome at all — it is + /// dropped before integration — so without this an outcome that reads Clean over the survivors is + /// indistinguishable from a run that integrated everything it produced, and whoever narrates it (the plan-map synth) + /// calls a partial deliverable whole. + /// + /// A unit is withheld only when NONE of its work landed. A retry respawns a fresh agent run, so a unit whose + /// first attempt flunked and whose second passed has both rows in the ledger — and its work is on the candidate; + /// naming the flunked attempt would tell the reader that delivered work was not. "Did it land" is asked on the same + /// unit the contribution reduction uses: the (node, iteration) cell where the cell IS the unit, the attempt itself + /// in the supervisor lane, where the cell is a turn shared by K concurrent deliverables and a peer that landed must + /// not hide the one that was withheld. One entry per unit, at its latest attempt's verdict, in agent-run creation + /// order — the same total order applies, so the report repeats across builds. + /// + public static IReadOnlyList Withheld(Guid repositoryId, IReadOnlyList manifests, IReadOnlyList agentWork) + { + var produced = ProducedRows(repositoryId, manifests, agentWork).ToList(); + var landed = LatestAttemptPerUnit(produced.Where(pair => !IsWithheldFromHead(pair.Manifest))).Select(pair => UnitKey(pair.Work)).ToHashSet(); + + return produced + .Where(pair => IsWithheldFromHead(pair.Manifest) && !landed.Contains(UnitKey(pair.Work))) + .GroupBy(pair => UnitKey(pair.Work)) + .Select(LatestRowOfUnit) + .OrderBy(pair => pair.Work.CreatedDate).ThenBy(pair => pair.Work.AgentRunId) + .Select(pair => new WithheldContribution(UnitLabel(pair.Work), $"acceptance {pair.Manifest.AcceptanceState}")) + .ToList(); + } + + /// Every row this repository's agents PRODUCED — Agent-kind, in this repository, carrying something (a None-state row left no trace), and joined to the agent-run row that holds its result bytes. Before any verdict is applied: drops the withheld ones, reports them. + private static IEnumerable<(PublishManifest Manifest, RunAgentWork Work)> ProducedRows(Guid repositoryId, IReadOnlyList manifests, IReadOnlyList agentWork) + { + var workByRunId = agentWork.ToDictionary(w => w.AgentRunId); + + return manifests + .Where(m => m.Kind == PublishManifestKind.Agent && m.AgentRunId is not null && m.RepositoryId == repositoryId && m.PublishStateValue != PublishState.None) + .Select(m => (Manifest: m, Work: workByRunId.GetValueOrDefault(m.AgentRunId!.Value))) + .Where(pair => pair.Work is not null) + .Select(pair => (pair.Manifest, Work: pair.Work!)); + } + + /// The unit's newest row — by agent-run creation, then id, then alias — so the pick is total and repeats across builds (the order uses, plus the alias tie-break between one attempt's own sibling rows). + private static (PublishManifest Manifest, RunAgentWork Work) LatestRowOfUnit(IEnumerable<(PublishManifest Manifest, RunAgentWork Work)> unit) => + unit.OrderBy(pair => pair.Work.CreatedDate).ThenBy(pair => pair.Work.AgentRunId).ThenBy(pair => pair.Manifest.RepositoryAlias, StringComparer.Ordinal).Last(); + + private static string UnitLabel(RunAgentWork work) => AgentAcceptanceContract.UnitId(work.NodeId, work.IterationKey ?? ""); + + /// What "this unit" means when asking whether it landed: its where the cell is the unit, the agent run itself where it is not (the fence applies). + private static string UnitKey(RunAgentWork work) => CellIsTheUnit(work.TaskJson) ? UnitLabel(work) : work.AgentRunId.ToString("N"); + /// /// "This unit's work is WITHHELD from the reviewable head" — the ledger-row analogue of the supervisor lane's /// SupervisorOutcome.IsWithheldFromHead (its per-unit grade rejected it, or a human WAIVED its diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationSummary.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationSummary.cs new file mode 100644 index 000000000..7f9b04592 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/RunIntegrationSummary.cs @@ -0,0 +1,74 @@ +using CodeSpace.Messages.Agents; + +namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; + +/// +/// What git.integrate_run says about its own outcome: ONE account of what actually landed, rendered on every +/// pass (clean, conflicted, empty, skipped, resumed) and read by the plan-map synth beside the per-subtask results. +/// Pure, so the exact words pin without a database — the words are the contract: the reduce is told to state what +/// landed and to name anything conflicted or withheld as NOT delivered, and it can only do that if the sentence it +/// reads says so. +/// +/// Landed is not applied. A non-Clean integration published NO branch — +/// is null on every such status and the integrator resets its clone to base on an abort — so AppliedCount is a +/// trial count there, never a partial publish. A conflicted set therefore says that nothing landed, with the applied +/// count as context only, and names each contribution that did not apply beside the branch that still keeps its work. +/// +/// Withheld is not an outcome. A unit whose own definition-of-done rejected it never reaches the integrator +/// (), so it appears in no ; its own +/// clause is appended to every account, because it is true of a Clean candidate and a skipped run alike. +/// +/// Bounded. The text rides a prompt whose own budget is enforced elsewhere (the map's +/// promptBudgetChars), so it carries its own: , cut with an ellipsis — negligible against +/// that budget, and never a silent drop of the head of the account, which is always first. +/// +public static class RunIntegrationSummary +{ + /// The account's length bound, in UTF-16 code units (the cut never splits a surrogate pair). + public const int MaxChars = 2_000; + + /// The account of an integration the integrator actually ran, plus what was withheld before it. + public static string ForResult(IntegrationResult result, IReadOnlyList withheld) => Bounded(Headline(result) + WithheldClause(withheld)); + + /// The account of an integration that never ran — nothing integrable, or no base revision to integrate from — plus what was withheld before it (the usual reason nothing was integrable). + public static string ForSkipped(string reason, IReadOnlyList withheld) => Bounded($"Integration skipped: {reason}." + WithheldClause(withheld)); + + private static string Headline(IntegrationResult result) => result.Status switch + { + IntegrationStatus.Clean => $"Integration: {result.AppliedCount} contribution(s) landed on {result.IntegratedBranch}.", + IntegrationStatus.Conflicted => ConflictedHeadline(result), + IntegrationStatus.Empty => $"Integration landed nothing: {result.Reason ?? "there was nothing to integrate"}.", + _ => $"Integration {result.Status}: {result.AppliedCount} of {result.Outcomes.Count} contribution(s) applied.", + }; + + private static string ConflictedHeadline(IntegrationResult result) + { + var headline = $"Integration conflicted: no integrated branch was published, so none of this run's work landed on one ({result.AppliedCount} of {result.Outcomes.Count} contribution(s) applied cleanly, but integration is all-or-nothing)."; + var notApplied = GitIntegrateNode.NotApplied(result).Select(Describe).ToList(); + + if (notApplied.Count > 0) return $"{headline} Conflicted and withheld from the integrated branch: {string.Join(", ", notApplied)}."; + + return result.Reason is { Length: > 0 } reason ? $"{headline} Reason: {reason}." : headline; + } + + /// One contribution that did not apply, beside where its work is kept. A bystander — never attempted, blocked only because ANOTHER contribution's problem stopped the set — is marked so it never reads as an equal failure. + private static string Describe(ContributionOutcome outcome) + { + var kept = outcome.FallbackBranch is { Length: > 0 } branch ? branch : "no branch to review"; + + return outcome.Skipped ? $"{outcome.Label} → {kept} (not attempted)" : $"{outcome.Label} → {kept}"; + } + + private static string WithheldClause(IReadOnlyList withheld) => + withheld.Count == 0 ? "" : $" Withheld before integration: {string.Join(", ", withheld.Select(w => $"{w.Label} — {w.Reason}"))}."; + + /// Cut to with an ellipsis, stepping back over a high surrogate the cut would otherwise strand — an unpaired half is text System.Text.Json refuses to encode, which would fail the node over prose. + private static string Bounded(string text) + { + if (text.Length <= MaxChars) return text; + + var cut = char.IsHighSurrogate(text[MaxChars - 2]) ? MaxChars - 2 : MaxChars - 1; + + return text[..cut] + "…"; + } +} diff --git a/backend/src/CodeSpace.Messages/Agents/WithheldContribution.cs b/backend/src/CodeSpace.Messages/Agents/WithheldContribution.cs new file mode 100644 index 000000000..2715eaef3 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Agents/WithheldContribution.cs @@ -0,0 +1,10 @@ +namespace CodeSpace.Messages.Agents; + +/// +/// One unit of a run's produced work the integration step left OUT of the reviewable candidate on purpose — a pure +/// data noun (Rule 18.1). is the unit id the integration outcome names its contributions by +/// ({node}#{iteration}), so a reader can line a withheld unit up against the ones that landed; +/// is the verdict that withheld it (acceptance Failed / acceptance Waived), read off the +/// ledger row and never free text. +/// +public sealed record WithheldContribution(string Label, string Reason); diff --git a/backend/tests/CodeSpace.E2ETests/Workflows/PlanMapIntegrateWholeLoopE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Workflows/PlanMapIntegrateWholeLoopE2ETests.cs index d18ef8be3..7c27d8b7c 100644 --- a/backend/tests/CodeSpace.E2ETests/Workflows/PlanMapIntegrateWholeLoopE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Workflows/PlanMapIntegrateWholeLoopE2ETests.cs @@ -105,6 +105,11 @@ public async Task A_repo_bound_fan_out_integrates_both_items_onto_one_reviewable outputs.GetProperty("integrationStatus").GetString().ShouldBe("Clean"); outputs.GetProperty("integratedBranch").GetString().ShouldBe(integrationBranch); outputs.GetProperty("combined").GetString().ShouldNotBeNullOrWhiteSpace("the synth still narrates — the code reduce rides beside it, not instead of it"); + + // …and what the synth was SHOWN: the integrate node's own account of what landed. The fake synth echoes its + // prompt, so `combined` is the output of the real node → VariableResolver → llm.complete path, not a re-derivation. + outputs.GetProperty("combined").GetString()!.ShouldContain($"Integration outcome:\nIntegration: 2 contribution(s) landed on {integrationBranch}.", + customMessage: "the reduce is handed what actually landed, in one factual sentence, beside the results it qualifies"); } /// @@ -114,10 +119,12 @@ public async Task A_repo_bound_fan_out_integrates_both_items_onto_one_reviewable /// candidate and the run's outputs were empty. With the projection declaring continue, the map finishes, /// the integrate step runs over the run's publish ledger, and the reduce narrates with the failure counted. /// - /// Which contributions integrate is a LEDGER question, not an outcome one (RunIntegrationContributions - /// deliberately applies no outcome filter): a unit that captured a diff contributes even if its own gate later - /// flunked it, so a human reviews the produced work instead of losing it. What this test pins is the part that - /// was broken — that the candidate exists at all, and that the SUCCEEDED sibling's work is in its tree. + /// Which contributions integrate is a LEDGER question with exactly ONE verdict gate + /// (RunIntegrationContributions): a unit whose own definition-of-done REJECTED it (or whose verification a + /// human waived) is withheld from the candidate, while a unit that merely ended badly but captured a diff still + /// contributes. This test pins the part that was broken — the candidate exists at all, with the SUCCEEDED sibling's + /// work in its tree and the flunked unit's work off it — and that the reduce is TOLD the flunked unit was withheld, + /// by name, instead of narrating a whole deliverable over a candidate that lacks it. /// [Fact] public async Task A_flunked_item_still_leaves_its_siblings_work_on_one_reviewable_candidate() @@ -189,7 +196,18 @@ public async Task A_flunked_item_still_leaves_its_siblings_work_on_one_reviewabl (await remote.BranchHasFileAsync(integrationBranch, FileWritingFakeCli.FileFor("do the second thing"))) .ShouldBeFalse(customMessage: "the flunked item's work must be withheld from the candidate — continue-on-error must not turn 'keep the siblings' into 'ship the rejected work'"); - outputs.GetProperty("combined").GetString().ShouldNotBeNullOrWhiteSpace("the reduce ran too — the run narrates instead of dying at the map"); + var combined = outputs.GetProperty("combined").GetString(); + + combined.ShouldNotBeNullOrWhiteSpace("the reduce ran too — the run narrates instead of dying at the map"); + + // What the synth was SHOWN: the flunked unit never reached the integrator, so the integration outcome alone + // would read as a clean, complete candidate. The integrate node names it as withheld — the label is the unit + // id of the failed attempts' shared (node, iteration) cell, read off the ledger rather than assumed. + var flunked = agentRuns.First(r => r.Status == AgentRunStatus.Failed); + var flunkedLabel = Core.Services.Agents.AgentAcceptanceContract.UnitId(flunked.NodeId, flunked.IterationKey ?? ""); + + combined.ShouldContain($"Integration outcome:\nIntegration: 1 contribution(s) landed on {integrationBranch}. Withheld before integration: {flunkedLabel} — acceptance Failed.", + customMessage: "the reduce must be told the flunked unit was withheld from the candidate — without it the candidate reads as the whole deliverable"); } finally { @@ -261,6 +279,20 @@ public async Task A_conflicted_candidate_parks_for_review_and_resumes_to_an_hone .ShouldBeFalse("no clean candidate ⇒ no candidate row"); (await remote.RemoteHasBranchAsync($"codespace/integration/{runId:N}")).ShouldBeFalse("nothing was pushed for a conflicted set — the fragments stay the only branches"); + + // What the synth was SHOWN. The two items run in parallel, so which agent run is created — and therefore + // applied first — is not fixed, and neither is WHICH unit conflicts. It is read from ground truth instead: + // the fallback branch the park itself named, mapped back to its unit through the publish ledger. + var conflictedBranch = JsonDocument.Parse(wait.PayloadJson!).RootElement.GetProperty("fallbackBranches")[0].GetString()!; + var conflictedLabel = await UnitLabelOfBranchAsync(verify.Resolve(), runId, conflictedBranch); + var combined = outputs.GetProperty("combined").GetString()!; + + combined.ShouldContain("Integration outcome:\nIntegration conflicted: no integrated branch was published", + customMessage: "the reduce is told the candidate conflicted and that nothing landed on an integrated branch"); + combined.ShouldContain($"{conflictedLabel} → {conflictedBranch}", + customMessage: "…and which contribution conflicted, with the branch that still keeps its work — the fragments the reviewer was asked about"); + combined.ShouldNotContain($"landed on codespace/integration/{runId:N}", + customMessage: "a conflicted set published no branch, so the reduce must never be handed one as where the work landed"); } finally { @@ -269,6 +301,15 @@ public async Task A_conflicted_candidate_parks_for_review_and_resumes_to_an_hone } } + /// The unit label the integration outcome names a contribution by, found from ground truth: the agent attempt that pushed (the run's publish ledger), then that agent run's (node, iteration) cell. + private static async Task UnitLabelOfBranchAsync(CodeSpaceDbContext db, Guid runId, string branch) + { + var manifest = await db.PublishManifest.AsNoTracking().SingleAsync(m => m.WorkflowRunId == runId && m.Kind == PublishManifestKind.Agent && m.Branch == branch); + var agentRun = await db.AgentRun.AsNoTracking().SingleAsync(r => r.Id == manifest.AgentRunId); + + return Core.Services.Agents.AgentAcceptanceContract.UnitId(agentRun.NodeId, agentRun.IterationKey ?? ""); + } + // ─── Projection (the production builder, planner pinned to the work-plan fake, synth retargeted) ─── private async Task ProjectAndStartAsync(Guid teamId, Guid userId, Guid plannerRowId, Guid repoId) diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/GitIntegrateRunNodeFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/GitIntegrateRunNodeFlowTests.cs index 6e4520726..9e45231e5 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/GitIntegrateRunNodeFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/GitIntegrateRunNodeFlowTests.cs @@ -303,6 +303,191 @@ public async Task A_supervisor_turns_parallel_agents_all_reach_the_integrator() integrator.LastRequest.Contributions.Select(c => c.Patch).ShouldBe(new[] { "diff-alpha", "diff-beta" }, customMessage: "and each sibling's own bytes reach the integrator"); } + // ─── The run's own account of the integration: the text the plan-map synth is handed ─── + // + // The integrate node owns the narrative. Its outputs already said `status` / `appliedCount` / `conflicts`, but a + // reader of those could not tell a run that integrated everything from one that integrated everything EXCEPT the + // unit its own definition-of-done rejected — that unit is dropped before the integrator sees it and appears in no + // outcome. `summary` states what landed; `withheld` names what was kept off the candidate on purpose. Both ride + // EVERY pass (clean, conflicted, skipped, resumed), and every key the node emits is one its OutputSchema declares. + + [Fact] + public async Task A_clean_integration_tells_the_reader_what_landed_and_where() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 9, branch: "codespace/agent/a"); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#1", minutesAgo: 3, branch: "codespace/agent/b"); + + using var scope = _fixture.BeginScope(); + var node = NodeOver(scope, new RecordingIntegrator { Result = CleanResult(runId, "agent#map#0", "agent#map#1") }); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + result.Outputs["summary"].GetString().ShouldBe($"Integration: 2 contribution(s) landed on codespace/integration/{runId:N}."); + result.Outputs["withheld"].GetArrayLength().ShouldBe(0, "nothing was withheld ⇒ an empty array, present on every pass"); + EveryEmittedOutputIsDeclared(node, result); + } + + [Fact] + public async Task A_conflicted_integration_says_nothing_landed_and_names_what_did_not() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 9, branch: "codespace/agent/a"); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#1", minutesAgo: 3, branch: "codespace/agent/b"); + + using var scope = _fixture.BeginScope(); + var node = NodeOver(scope, new RecordingIntegrator { Result = ConflictedResult() }); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success, "without the park opt-in a conflict is a routable outcome"); + result.Outputs["summary"].GetString().ShouldBe( + "Integration conflicted: no integrated branch was published, so none of this run's work landed on one (1 of 2 contribution(s) applied cleanly, but integration is all-or-nothing). " + + "Conflicted and withheld from the integrated branch: agent#map#1 → codespace/agent/b."); + EveryEmittedOutputIsDeclared(node, result); + } + + [Fact] + public async Task A_run_that_produced_nothing_integrable_says_why_it_skipped() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + + using var scope = _fixture.BeginScope(); + var integrator = new RecordingIntegrator(); + var node = NodeOver(scope, integrator); + + var result = await node.RunAsync(Context(Guid.NewGuid(), teamId, runId), CancellationToken.None); + + result.Outputs["summary"].GetString().ShouldBe("Integration skipped: the run produced no integrable work for this repository."); + result.Outputs["withheld"].GetArrayLength().ShouldBe(0); + integrator.Calls.ShouldBe(0, "a skipped pass never touches git"); + EveryEmittedOutputIsDeclared(node, result); + } + + [Fact] + public async Task A_run_whose_work_recorded_no_base_revision_says_why_it_skipped() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 5, branch: "codespace/agent/a", baseSha: ""); + + using var scope = _fixture.BeginScope(); + var integrator = new RecordingIntegrator(); + var node = NodeOver(scope, integrator); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success); + result.Outputs["status"].GetString().ShouldBe("Skipped", "fixture check: the no-base arm — not the no-contributions one — is what ran"); + result.Outputs["summary"].GetString().ShouldBe("Integration skipped: the produced work recorded no base revision to integrate from."); + integrator.Calls.ShouldBe(0); + EveryEmittedOutputIsDeclared(node, result); + } + + /// + /// The unit a flunked (or human-waived) definition-of-done withheld is named beside the candidate it is NOT on. + /// Before, it appeared in no output: the outcome said Clean over the survivors, and the synth narrated a whole + /// deliverable. The assertion on the request is the fixture check — the unit really was dropped before the integrator. + /// + [Theory] + [InlineData(PublishAcceptanceState.Failed, "acceptance Failed")] + [InlineData(PublishAcceptanceState.Waived, "acceptance Waived")] + public async Task A_withheld_unit_is_named_beside_the_clean_candidate_it_is_not_on(PublishAcceptanceState state, string reason) + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 9, branch: "codespace/agent/a"); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#1", minutesAgo: 3, branch: "codespace/agent/b", acceptance: state); + + using var scope = _fixture.BeginScope(); + var integrator = new RecordingIntegrator { Result = CleanResult(runId, "agent#map#0") }; + var node = NodeOver(scope, integrator); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + integrator.LastRequest!.Contributions.Select(c => c.Label).ShouldBe(new[] { "agent#map#0" }, "fixture check: the withheld unit never reached the integrator"); + result.Outputs["summary"].GetString().ShouldBe($"Integration: 1 contribution(s) landed on codespace/integration/{runId:N}. Withheld before integration: agent#map#1 — {reason}."); + + var withheld = result.Outputs["withheld"].EnumerateArray().ShouldHaveSingleItem(); + withheld.GetProperty("label").GetString().ShouldBe("agent#map#1"); + withheld.GetProperty("reason").GetString().ShouldBe(reason); + EveryEmittedOutputIsDeclared(node, result); + } + + /// The withheld set is read BEFORE the skip decision: when the only unit the run produced was withheld, the integration skips — and the skip must say WHY, or the synth reads an empty run instead of a rejected one. + [Fact] + public async Task When_every_unit_was_withheld_the_skip_names_them_and_git_is_never_touched() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 5, branch: "codespace/agent/a", acceptance: PublishAcceptanceState.Failed); + + using var scope = _fixture.BeginScope(); + var integrator = new RecordingIntegrator(); + var node = NodeOver(scope, integrator); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + result.Outputs["status"].GetString().ShouldBe("Skipped"); + result.Outputs["summary"].GetString().ShouldBe("Integration skipped: the run produced no integrable work for this repository. Withheld before integration: agent#map#0 — acceptance Failed."); + result.Outputs["withheld"].EnumerateArray().ShouldHaveSingleItem().GetProperty("label").GetString().ShouldBe("agent#map#0"); + integrator.Calls.ShouldBe(0); + EveryEmittedOutputIsDeclared(node, result); + } + + /// The resumed pass re-integrates and states its outcome like any other pass, beside the review trail it already carried — and that trail (reviewApproved / reviewComment / reviewedBy) is declared too, so the keys the pass emits are all bindable. + [Fact] + public async Task The_resumed_pass_states_the_outcome_too_beside_the_review_trail() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 9, branch: "codespace/agent/a"); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#1", minutesAgo: 3, branch: "codespace/agent/b"); + + using var scope = _fixture.BeginScope(); + var node = NodeOver(scope, new RecordingIntegrator { Result = ConflictedResult() }); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId, parkOnConflict: true, resumePayload: """{"approved":false,"comment":"ship the fragments","by":"user-2"}"""), CancellationToken.None); + + result.Outputs["reviewedBy"].GetString().ShouldBe("user-2", "fixture check: this is the resumed pass"); + result.Outputs["summary"].GetString().ShouldStartWith("Integration conflicted: no integrated branch was published"); + result.Outputs.Keys.ShouldContain("withheld"); + EveryEmittedOutputIsDeclared(node, result); + } + + [Fact] + public async Task A_wide_conflict_cannot_bloat_the_summary_past_its_bound() + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var runId = await SeedRunAsync(teamId, userId); + var repositoryId = Guid.NewGuid(); + await SeedProducedUnitAsync(teamId, runId, repositoryId, "map#0", minutesAgo: 5, branch: "codespace/agent/a"); + + var outcomes = Enumerable.Range(0, 120) + .Select(i => new ContributionOutcome { Label = $"agent#map#{i}", Disposition = ContributionDisposition.Conflicted, FallbackBranch = $"codespace/agent/{new string('b', 40)}-{i}", Reason = "textual conflict" }) + .ToList(); + + using var scope = _fixture.BeginScope(); + var node = NodeOver(scope, new RecordingIntegrator { Result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, outcomes, "a contribution conflicted while integrating") }); + + var result = await node.RunAsync(Context(repositoryId, teamId, runId), CancellationToken.None); + + var summary = result.Outputs["summary"].GetString()!; + + summary.Length.ShouldBe(2_000, "the reduce prompt it is appended to is budgeted elsewhere — this text must stay negligible against that budget"); + summary.ShouldEndWith("…"); + result.Outputs["conflicts"].GetArrayLength().ShouldBe(120, "the bound is on the prose, never on the machine-readable conflicts[]"); + } + // ─── Seeds ────────────────────────────────────────────────────────────────── private async Task SeedRunAsync(Guid teamId, Guid userId) @@ -356,6 +541,27 @@ private async Task SeedAgentManifestAsync(Guid teamId, Guid runId, Guid agentRun }, CancellationToken.None); } + /// One produced unit — an agent run plus the publish-manifest row its attempt wrote: the pair the node derives a contribution (or a withheld unit) from. + private async Task SeedProducedUnitAsync(Guid teamId, Guid runId, Guid repositoryId, string iterationKey, int minutesAgo, string branch, PublishAcceptanceState acceptance = PublishAcceptanceState.NotApplicable, string baseSha = "b1") + { + var agentRunId = await SeedAgentRunAsync(teamId, runId, new AgentRunSeed(iterationKey, minutesAgo)); + await SeedAgentManifestAsync(teamId, runId, agentRunId, repositoryId, branch, baseSha, acceptance); + } + + private static GitIntegrateRunNode NodeOver(Autofac.ILifetimeScope scope, RecordingIntegrator integrator) => + new(integrator, new StubResolver(), scope.Resolve(), scope.Resolve()); + + private static IntegrationResult CleanResult(Guid runId, params string[] labels) => + IntegrationResult.Build(IntegrationStatus.Clean, $"codespace/integration/{runId:N}", labels.Select(label => new ContributionOutcome { Label = label, Disposition = ContributionDisposition.Applied }).ToList()); + + /// A key the node emits but its OutputSchema does not declare is unbindable — DefinitionValidator rejects the reference — so every pass's outputs must be a subset of the declared ones. + private static void EveryEmittedOutputIsDeclared(GitIntegrateRunNode node, NodeResult result) + { + var declared = node.Manifest.OutputSchema.GetProperty("properties").EnumerateObject().Select(p => p.Name).ToList(); + + result.Outputs.Keys.Except(declared).ShouldBeEmpty("an output the node emits but its OutputSchema does not declare cannot be bound by any graph"); + } + private static NodeRunContext Context(Guid repositoryId, Guid teamId, Guid runId, bool parkOnConflict = false, string? resumePayload = null) => new() { Inputs = new Dictionary { ["repositoryId"] = JsonSerializer.SerializeToElement(repositoryId.ToString()) }, diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/GitIntegrateRunNodeManifestTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/GitIntegrateRunNodeManifestTests.cs new file mode 100644 index 000000000..752a95fff --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/GitIntegrateRunNodeManifestTests.cs @@ -0,0 +1,58 @@ +using System.Text.Json; +using CodeSpace.Core.Services.Workflows.Engine; +using CodeSpace.Core.Services.Workflows.Nodes; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Messages.Dtos.Workflows; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// The consumer's view of git.integrate_run's declared outputs. DefinitionValidator rejects a +/// {{nodes.X.outputs.Y}} reference to a key X's OutputSchema does not declare, so a key the node EMITS but does +/// not DECLARE is unreachable to every graph that wants it: the plan-map synth cannot bind summary, and no +/// author can bind the review trail a resumed pass already emits (reviewApproved / reviewComment / +/// reviewedBy). Pinned one key per case so a failure names exactly the key that is missing. +/// +[Trait("Category", "Unit")] +public class GitIntegrateRunNodeManifestTests +{ + [Theory] + [InlineData("status")] + [InlineData("integratedBranch")] + [InlineData("appliedCount")] + [InlineData("reason")] + [InlineData("conflicts")] + [InlineData("summary")] + [InlineData("withheld")] + [InlineData("reviewApproved")] + [InlineData("reviewComment")] + [InlineData("reviewedBy")] + public void A_downstream_node_can_bind_every_output_the_node_emits(string output) + { + var validator = new DefinitionValidator(new NodeRegistry(new INodeRuntime[] { new TriggerManualNode(), new GitIntegrateRunNode(null!, null!, null!, null!), new TerminalNode() })); + + var definition = new WorkflowDefinition + { + Nodes = new List + { + Node("start", "trigger.manual", new { }), + Node("integrate", "git.integrate_run", new { repositoryId = Guid.NewGuid().ToString() }), + Node("done", "builtin.terminal", new { bound = "{{nodes.integrate.outputs." + output + "}}" }), + }, + Edges = new List { new() { From = "start", To = "integrate" }, new() { From = "integrate", To = "done" } }, + }; + + var validation = validator.Validate(definition); + + validation.IsValid.ShouldBeTrue(customMessage: $"'{output}' must be a declared output of git.integrate_run — errors: " + string.Join(" | ", validation.Errors)); + } + + private static NodeDefinition Node(string id, string typeKey, object inputs) => new() + { + Id = id, + TypeKey = typeKey, + Config = JsonDocument.Parse("{}").RootElement.Clone(), + Inputs = JsonSerializer.SerializeToElement(inputs), + }; +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapDynamicDefinitionBuilderTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapDynamicDefinitionBuilderTests.cs index 937475e02..42677c49e 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapDynamicDefinitionBuilderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapDynamicDefinitionBuilderTests.cs @@ -167,6 +167,25 @@ public void Synth_is_a_real_llm_reduce_over_the_bounded_results_projection_gener done.Inputs.GetProperty("combined").GetString().ShouldBe("{{nodes.synth.outputs.text}}"); } + /// The dynamic variant shares the base's reduce, so a repo-bound graph hands its synth the integrate node's account of what landed exactly as plan-map-synth does (the sibling's pin carries the exact prompt text) — and a repo-less one keeps its prompt untouched. + [Fact] + public void A_repo_bound_reduce_is_shown_what_actually_landed_like_its_sibling() + { + var bound = Builder.Build(Context(new ResolvedAgentProfile { RepositoryId = Guid.NewGuid(), Harness = "claude-code" })); + var synth = bound.Nodes.Single(n => n.Id == "synth").Inputs; + + synth.GetProperty("userPrompt").GetString()!.ShouldEndWith("\n\nIntegration outcome:\n{{nodes.integrate.outputs.summary}}", + customMessage: "the integrate node's own account of what landed rides the reduce prompt on this variant too"); + synth.GetProperty("systemPrompt").GetString()!.ShouldContain("NOT delivered", customMessage: "and the reduce is told to name anything conflicted or withheld as not delivered"); + + var validation = RealValidator().Validate(bound); + + validation.IsValid.ShouldBeTrue(customMessage: "the binding resolves against the integrate node's declared outputs: " + string.Join(" | ", validation.Errors)); + + Builder.Build(Context()).Nodes.Single(n => n.Id == "synth").Inputs.GetProperty("userPrompt").GetString()! + .ShouldNotContain("Integration outcome", customMessage: "a repo-less graph has no integrate node and keeps its prompt byte-for-byte"); + } + [Fact] public void Profile_maps_onto_the_planner_model_and_the_agent_body() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapSynthDefinitionBuilderTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapSynthDefinitionBuilderTests.cs index 3b1d227cc..d043ff34a 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapSynthDefinitionBuilderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/PlanMapSynthDefinitionBuilderTests.cs @@ -347,6 +347,49 @@ public void The_reduces_user_prompt_carries_no_failure_furniture_so_a_clean_run_ customMessage: "the failed count is a persisted fact on the map bag + the run row — it is not furniture on every happy run's prompt"); } + /// + /// A repo-bound reduce used to narrate over an integration it was never told about: the integrate step's outputs + /// reached only the done terminal, so a candidate that conflicted — or a unit withheld from it — was summarised as + /// a whole deliverable. The integrate node authors one account of the outcome (summary); the data half of + /// the prompt carries it, and the system prompt says what to do with it. + /// + /// That is not the "failure furniture" the test above forbids: on a clean run the account is ONE factual + /// sentence (Integration: N contribution(s) landed on {branch}.) — the fact the reduce needs in order to call + /// the work delivered — and it is a RUN-time fact the node renders, not a build-time conditional. + /// + [Fact] + public void A_repo_bound_reduce_is_shown_what_actually_landed_and_told_to_name_what_did_not() + { + var def = Builder.Build(Context(new ResolvedAgentProfile { RepositoryId = Guid.NewGuid(), Harness = "claude-code" })); + var synth = def.Nodes.Single(n => n.Id == "synth").Inputs; + + synth.GetProperty("userPrompt").GetString().ShouldBe( + $"Goal: Improve the onboarding module\n\nPer-subtask results:\n{{{{nodes.map.outputs.{WorkflowOutputKeys.MapResultsPrompt}}}}}\n\nIntegration outcome:\n{{{{nodes.integrate.outputs.summary}}}}", + customMessage: "the integrate node's own account of what landed rides the data half of the reduce prompt, after the results it qualifies"); + + var systemPrompt = synth.GetProperty("systemPrompt").GetString()!; + + systemPrompt.ShouldStartWith(PlanMapBuilderBase.SynthSystemPrompt, customMessage: "the failure clause is kept verbatim — the integration instruction is added to it, never in place of it"); + systemPrompt.ShouldContain("what actually landed on the integrated branch", customMessage: "the reduce is told what the integration outcome IS"); + systemPrompt.ShouldContain("NOT delivered", customMessage: "anything conflicted or withheld must be named as not delivered"); + systemPrompt.ShouldContain("never narrate withheld work as done", customMessage: "the one thing the reduce must not do"); + + var validation = RealValidator().Validate(def); + + validation.IsValid.ShouldBeTrue(customMessage: "the binding resolves against the integrate node's declared outputs: " + string.Join(" | ", validation.Errors)); + } + + /// The other arm, and the byte pin's reach: a repo-less graph has no integrate node, so neither half of its reduce prompt may mention an integration — a sentence about an outcome that does not exist would only invite the model to invent one. + [Fact] + public void A_repo_less_reduce_is_never_told_about_an_integration_it_does_not_have() + { + var synth = Builder.Build(Context()).Nodes.Single(n => n.Id == "synth").Inputs; + + synth.GetProperty("userPrompt").GetString()!.ShouldNotContain("integrat", Case.Insensitive, customMessage: "no integrate node ⇒ no integration outcome in the data half"); + synth.GetProperty("systemPrompt").GetString().ShouldBe(PlanMapBuilderBase.SynthSystemPrompt, customMessage: "a repo-less reduce keeps its system prompt byte-for-byte"); + synth.GetProperty("systemPrompt").GetString()!.ShouldNotContain("integrat", Case.Insensitive); + } + /// The other half of the same fact: the failed-branch count reaches the RUN ROW beside the combined answer, so a partial result is legible from the run's outcome and not only from the map node's bag (the coverage binding's reasoning, applied to the second way an answer can be less than whole). [Fact] public void The_done_terminal_surfaces_the_failed_branch_count_beside_the_combined_answer() diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationContributionsTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationContributionsTests.cs index 1cc95071e..a2fd82164 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationContributionsTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationContributionsTests.cs @@ -233,6 +233,145 @@ public void A_withheld_unit_does_not_withhold_its_siblings() contributions.ShouldHaveSingleItem().ProducedBranch.ShouldBe("codespace/agent/passed"); } + // ─── What the head withheld is NAMED, not just dropped ─────────────────────────── + // + // The withhold gate above removes a unit from every outcome the integration reports, so a reader of the outcome — + // the plan-map synth first — could not tell a run that integrated everything from one that integrated everything + // except the unit its own definition-of-done rejected. `Withheld` is the gate's other face: the same verdict, the + // same rows, reported instead of discarded. + + [Theory] + [InlineData(PublishAcceptanceState.Failed, "acceptance Failed")] + [InlineData(PublishAcceptanceState.Waived, "acceptance Waived")] + public void A_unit_the_head_withheld_is_named_with_the_verdict_that_withheld_it(PublishAcceptanceState state, string reason) + { + var runId = Guid.NewGuid(); + + var withheld = RunIntegrationContributions.Withheld(Repo, + new[] { Manifest(runId, Repo, PublishState.Pushed, branch: "codespace/agent/a", acceptance: state) }, + new[] { Work(runId, "agent", "map#0", minute: 1) }); + + withheld.ShouldHaveSingleItem().ShouldBe(new WithheldContribution("agent#map#0", reason), + customMessage: "the label is the unit id the integration outcome names its contributions by — so a reader can line the two up"); + } + + [Theory] + [InlineData(PublishAcceptanceState.Passed)] + [InlineData(PublishAcceptanceState.NotApplicable)] + public void A_unit_the_head_kept_is_never_reported_withheld(PublishAcceptanceState state) + { + var runId = Guid.NewGuid(); + + RunIntegrationContributions.Withheld(Repo, + new[] { Manifest(runId, Repo, PublishState.Pushed, branch: "codespace/agent/a", acceptance: state) }, + new[] { Work(runId, "agent", "map#0", minute: 1) }) + .ShouldBeEmpty("a unit that reaches the candidate is not withheld — naming it would contradict the integration outcome"); + } + + /// The two views partition the produced units: every unit is either a contribution or withheld, never both and never neither — which is the property that makes "what landed" plus "what was withheld" a complete account. + [Fact] + public void What_landed_and_what_was_withheld_partition_the_produced_units() + { + var flunked = Guid.NewGuid(); + var passed = Guid.NewGuid(); + var manifests = new[] + { + Manifest(flunked, Repo, PublishState.Pushed, branch: "codespace/agent/flunked", acceptance: PublishAcceptanceState.Failed), + Manifest(passed, Repo, PublishState.Pushed, branch: "codespace/agent/passed", acceptance: PublishAcceptanceState.Passed), + }; + var work = new[] { Work(flunked, "agent", "map#0", minute: 1), Work(passed, "agent", "map#1", minute: 2) }; + + RunIntegrationContributions.Build(Repo, manifests, work).Select(c => c.Label).ShouldBe(new[] { "agent#map#1" }); + RunIntegrationContributions.Withheld(Repo, manifests, work).Select(w => w.Label).ShouldBe(new[] { "agent#map#0" }); + } + + /// A retry respawns a fresh agent run, so a unit whose first attempt flunked and whose second passed has BOTH rows in the ledger — and its work landed. Reporting the flunked attempt would tell the reduce that delivered work was not delivered. + [Fact] + public void A_unit_whose_retry_landed_is_not_reported_withheld_for_its_abandoned_attempt() + { + var abandoned = Guid.NewGuid(); + var respawned = Guid.NewGuid(); + var manifests = new[] + { + Manifest(abandoned, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed), + Manifest(respawned, Repo, PublishState.Pushed, branch: "codespace/agent/a2", acceptance: PublishAcceptanceState.Passed), + }; + var work = new[] { Work(abandoned, "agent", "map#0", minute: 1), Work(respawned, "agent", "map#0", minute: 7) }; + + RunIntegrationContributions.Build(Repo, manifests, work).ShouldHaveSingleItem(); + RunIntegrationContributions.Withheld(Repo, manifests, work).ShouldBeEmpty("the unit's work reached the candidate through its second attempt"); + } + + [Fact] + public void Every_attempt_of_a_withheld_unit_is_reported_once_under_its_latest_verdict() + { + var first = Guid.NewGuid(); + var second = Guid.NewGuid(); + var third = Guid.NewGuid(); + + var withheld = RunIntegrationContributions.Withheld(Repo, + new[] + { + Manifest(first, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed), + Manifest(second, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed), + Manifest(third, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Waived), + }, + new[] { Work(first, "agent", "map#0", minute: 1), Work(second, "agent", "map#0", minute: 4), Work(third, "agent", "map#0", minute: 8) }); + + withheld.ShouldHaveSingleItem(customMessage: "the retries of one unit are one unit — a row per attempt would name it three times") + .Reason.ShouldBe("acceptance Waived", "the unit stands at its latest attempt's verdict"); + } + + /// The supervisor lane's cell is a whole turn, so its K agents share one label while being K distinct deliverables: a peer that landed must not hide the one that was withheld, which is why "did the unit land" is keyed on the agent there and on the cell everywhere else. + [Fact] + public void A_supervisor_turns_withheld_agent_is_named_even_though_a_peer_sharing_its_cell_landed() + { + var alpha = Guid.NewGuid(); + var beta = Guid.NewGuid(); + var manifests = new[] + { + Manifest(alpha, Repo, PublishState.Pushed, branch: "codespace/agent/s1", acceptance: PublishAcceptanceState.Failed), + Manifest(beta, Repo, PublishState.Pushed, branch: "codespace/agent/s2", acceptance: PublishAcceptanceState.Passed), + }; + var work = new[] { PlannedSupervisorWork(alpha, "sup#turn1", "subtask-a", minute: 1), PlannedSupervisorWork(beta, "sup#turn1", "subtask-b", minute: 2) }; + + RunIntegrationContributions.Build(Repo, manifests, work).ShouldHaveSingleItem().ProducedBranch.ShouldBe("codespace/agent/s2"); + RunIntegrationContributions.Withheld(Repo, manifests, work).ShouldHaveSingleItem().ShouldBe(new WithheldContribution("sup#sup#turn1", "acceptance Failed")); + } + + [Fact] + public void Only_work_this_repository_actually_produced_can_be_withheld_from_it() + { + var foreignRepo = Guid.NewGuid(); + var foreign = Guid.NewGuid(); + var nothingProduced = Guid.NewGuid(); + var mine = Guid.NewGuid(); + + var withheld = RunIntegrationContributions.Withheld(Repo, + new[] + { + Manifest(foreign, foreignRepo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed), + Manifest(nothingProduced, Repo, PublishState.None, acceptance: PublishAcceptanceState.Failed), + Manifest(mine, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed), + }, + new[] { Work(foreign, "agent", "map#0", minute: 1), Work(nothingProduced, "agent", "map#1", minute: 2), Work(mine, "agent", "map#2", minute: 3) }); + + withheld.Select(w => w.Label).ShouldBe(new[] { "agent#map#2" }, "another repository's verdict, and a row that produced nothing, are not this repository's withheld work"); + } + + [Fact] + public void Withheld_units_list_in_agent_run_creation_order_whatever_order_the_rows_arrive_in() + { + var late = Guid.NewGuid(); + var early = Guid.NewGuid(); + var middle = Guid.NewGuid(); + var manifests = new[] { late, early, middle }.Select(id => Manifest(id, Repo, PublishState.Pushed, acceptance: PublishAcceptanceState.Failed)).ToArray(); + var work = new[] { Work(late, "agent", "map#2", minute: 9), Work(early, "agent", "map#0", minute: 3), Work(middle, "agent", "map#1", minute: 6) }; + + RunIntegrationContributions.Withheld(Repo, manifests, work).Select(w => w.Label).ShouldBe(new[] { "agent#map#0", "agent#map#1", "agent#map#2" }); + RunIntegrationContributions.Withheld(Repo, manifests.Reverse().ToArray(), work.Reverse().ToArray()).Select(w => w.Label).ShouldBe(new[] { "agent#map#0", "agent#map#1", "agent#map#2" }); + } + [Fact] public void A_surviving_attempts_sibling_alias_rows_all_stay() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationSummaryTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationSummaryTests.cs new file mode 100644 index 000000000..08723d2fa --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RunIntegrationSummaryTests.cs @@ -0,0 +1,182 @@ +using System.Text; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Messages.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// What git.integrate_run says about its own outcome: the text the plan-map synth is handed beside the +/// per-subtask results. Pinned as EXACT text per outcome, because the words are the contract — the reduce is told to +/// state what landed and to name anything conflicted or withheld as NOT delivered, and it can only do that if the +/// sentence it reads says so. +/// +/// The honesty axis these pin: a non-Clean integration published NO branch (IntegrationResult.IntegratedBranch +/// is null on every such status, and the integrator resets its clone to base on an abort), so its applied count is a +/// trial count and never "landed". And a unit withheld before integration appears in no outcome at all — only the +/// withheld clause names it. +/// +[Trait("Category", "Unit")] +public class RunIntegrationSummaryTests +{ + private static readonly IReadOnlyList NoneWithheld = Array.Empty(); + + private static readonly WithheldContribution Flunked = new("agent#map#1", "acceptance Failed"); + + [Fact] + public void A_clean_integration_says_how_many_contributions_landed_and_where() + { + var result = IntegrationResult.Build(IntegrationStatus.Clean, "codespace/integration/run1", new[] { Applied("agent#map#0"), Applied("agent#map#1") }); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldBe("Integration: 2 contribution(s) landed on codespace/integration/run1."); + } + + [Fact] + public void A_withheld_unit_is_named_after_the_outcome_even_when_the_candidate_is_clean() + { + var result = IntegrationResult.Build(IntegrationStatus.Clean, "codespace/integration/run1", new[] { Applied("agent#map#0") }); + var withheld = new[] { Flunked, new WithheldContribution("agent#map#3", "acceptance Waived") }; + + RunIntegrationSummary.ForResult(result, withheld).ShouldBe( + "Integration: 1 contribution(s) landed on codespace/integration/run1. Withheld before integration: agent#map#1 — acceptance Failed, agent#map#3 — acceptance Waived."); + } + + [Fact] + public void A_conflicted_set_says_nothing_landed_and_names_the_conflicted_contribution_with_the_branch_that_keeps_its_work() + { + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, new[] { Applied("agent#map#0"), Conflicted("agent#map#1", "codespace/agent/b") }, "a contribution conflicted while integrating"); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldBe( + "Integration conflicted: no integrated branch was published, so none of this run's work landed on one (1 of 2 contribution(s) applied cleanly, but integration is all-or-nothing). " + + "Conflicted and withheld from the integrated branch: agent#map#1 → codespace/agent/b."); + } + + [Fact] + public void A_conflict_is_named_before_a_bystander_the_set_merely_never_attempted() + { + // The shape LocalGitBranchIntegrator.ResolvedContribution.Skip() produces: no files of its own, and the set-level reason. + var bystander = Conflicted("agent#map#0", "codespace/agent/a") with { Skipped = true, ConflictedFiles = Array.Empty(), Reason = "not integrated — an earlier contribution conflicted" }; + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, new[] { bystander, Conflicted("agent#map#1", "codespace/agent/b") }, "a contribution conflicted while integrating"); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldEndWith( + "agent#map#1 → codespace/agent/b, agent#map#0 → codespace/agent/a (not attempted)."); + } + + [Fact] + public void A_contribution_with_no_branch_to_fall_back_on_is_named_as_having_none() + { + var unintegrable = new ContributionOutcome { Label = "agent#map#2", Disposition = ContributionDisposition.Unintegrable, Reason = "no patch and no branch (the agent's work was not captured)" }; + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, new[] { unintegrable }, "a contribution could not be applied"); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldEndWith("agent#map#2 → no branch to review."); + } + + [Fact] + public void A_conflicted_set_whose_contributions_all_applied_states_the_reason_the_publish_was_refused() + { + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, new[] { Applied("agent#map#0"), Applied("agent#map#1") }, "remote integration branch advanced"); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldBe( + "Integration conflicted: no integrated branch was published, so none of this run's work landed on one (2 of 2 contribution(s) applied cleanly, but integration is all-or-nothing). " + + "Reason: remote integration branch advanced."); + } + + [Fact] + public void A_conflicted_set_names_its_withheld_units_after_its_conflicts() + { + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, new[] { Conflicted("agent#map#0", "codespace/agent/a") }, "a contribution conflicted while integrating"); + + RunIntegrationSummary.ForResult(result, new[] { Flunked }).ShouldEndWith( + "agent#map#0 → codespace/agent/a. Withheld before integration: agent#map#1 — acceptance Failed."); + } + + [Fact] + public void An_integration_that_found_nothing_to_land_says_so_with_the_integrators_reason() + { + var result = IntegrationResult.Build(IntegrationStatus.Empty, null, new[] { Applied("agent#map#0") }, "every contribution was a no-op — nothing to integrate"); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldBe("Integration landed nothing: every contribution was a no-op — nothing to integrate."); + } + + [Fact] + public void A_status_the_integrator_does_not_yet_produce_still_reads_as_applied_counts_and_claims_no_branch() + { + var result = IntegrationResult.Build(IntegrationStatus.Partial, null, new[] { Applied("agent#map#0"), Conflicted("agent#map#1", "codespace/agent/b") }); + + RunIntegrationSummary.ForResult(result, NoneWithheld).ShouldBe("Integration Partial: 1 of 2 contribution(s) applied."); + } + + /// A skipped integration says why — and still names the units withheld before it, which is usually the reason nothing was integrable. + [Theory] + [InlineData(false, "Integration skipped: the run produced no integrable work for this repository.")] + [InlineData(true, "Integration skipped: the run produced no integrable work for this repository. Withheld before integration: agent#map#1 — acceptance Failed.")] + public void A_skipped_integration_says_why_and_names_what_was_withheld_before_it(bool anyWithheld, string expected) + { + var withheld = anyWithheld ? new[] { Flunked } : Array.Empty(); + + RunIntegrationSummary.ForSkipped("the run produced no integrable work for this repository", withheld).ShouldBe(expected); + } + + // ─── The bound: a wide fan-out can never bloat the reduce prompt it is appended to ─── + + [Fact] + public void The_bound_is_two_thousand_characters() + { + RunIntegrationSummary.MaxChars.ShouldBe(2_000); + } + + [Fact] + public void A_conflict_list_past_the_bound_is_cut_with_an_ellipsis_and_keeps_its_head() + { + var outcomes = Enumerable.Range(0, 200).Select(i => Conflicted($"agent#map#{i}", $"codespace/agent/{new string('b', 40)}-{i}")).ToList(); + var result = IntegrationResult.Build(IntegrationStatus.Conflicted, null, outcomes, "a contribution conflicted while integrating"); + + var summary = RunIntegrationSummary.ForResult(result, NoneWithheld); + + summary.Length.ShouldBe(2_000); + summary.ShouldEndWith("…"); + summary.ShouldStartWith("Integration conflicted: no integrated branch was published"); + } + + [Fact] + public void A_withheld_list_past_the_bound_is_cut_the_same_way_on_a_skipped_integration() + { + var withheld = Enumerable.Range(0, 200).Select(i => new WithheldContribution($"agent#map#{i}", "acceptance Failed")).ToList(); + + var summary = RunIntegrationSummary.ForSkipped("the run produced no integrable work for this repository", withheld); + + summary.Length.ShouldBe(2_000); + summary.ShouldEndWith("…"); + } + + [Fact] + public void A_summary_exactly_at_the_bound_is_not_touched() + { + var reason = new string('r', RunIntegrationSummary.MaxChars - "Integration skipped: .".Length); + + var summary = RunIntegrationSummary.ForSkipped(reason, NoneWithheld); + + summary.Length.ShouldBe(2_000); + summary.ShouldEndWith("r."); + } + + /// A cut that lands between the halves of a surrogate pair would leave an unpaired high surrogate, which System.Text.Json refuses to encode — so the same arithmetic that protects the prompt would fail the node. The cut steps back over the pair instead. + [Fact] + public void A_cut_never_splits_a_surrogate_pair() + { + var filler = new string('r', 1_998 - "Integration skipped: ".Length); + + var summary = RunIntegrationSummary.ForSkipped(filler + "\U0001F600 and then some more text", NoneWithheld); + + summary.Length.ShouldBeLessThanOrEqualTo(2_000); + Should.NotThrow(() => new UTF8Encoding(false, throwOnInvalidBytes: true).GetBytes(summary), "the bound must not leave half of a surrogate pair at the cut"); + summary.ShouldEndWith("…"); + } + + private static ContributionOutcome Applied(string label) => new() { Label = label, Disposition = ContributionDisposition.Applied }; + + private static ContributionOutcome Conflicted(string label, string fallbackBranch) => new() + { + Label = label, Disposition = ContributionDisposition.Conflicted, ConflictedFiles = new[] { "shared.txt" }, FallbackBranch = fallbackBranch, Reason = "textual conflict applying the patch", + }; +}