Fix #2601: drop "One PR" opener from planner prompts; align both paths on slice-DAG

orchestrator/routes/pipelines.py

@@ -11105,11 +11105,12 @@ def _build_phase_prompt(
     elif phase == "plan":
         lines.extend(
             [
-                "Create a detailed implementation plan.",
-                "",
-                "**CRITICAL CONSTRAINT — One Issue = One Workflow = One PR.**",
-                "All tasks belong to a single pull request. Use phases and commits to",
-                "organise the work within that PR — do NOT propose multiple PRs.",
+                "Create a detailed implementation plan, decomposing the work into "
+                "slices per the slice-DAG guidance at the end of this section. The "
+                "implement-phase pipeline ships each slice as its own stacked PR. "
+                "A single-slice plan is fine when the work is cohesive; multi-slice "
+                "plans are required when an upstream refine-phase work-decomposition "
+                "HITL decision selected a multi-slice shape.",
                 "",
                 "Steps:",
                 "1. Review any prior analysis",
@@ -11146,12 +11147,12 @@ def _build_phase_prompt(
                 "    Pre-merge: any required steps before merging",
                 "    Post-merge: any required steps after merging",
                 *_PR_CONTEXT_YAML_EXAMPLE_LINES,
-                "phases:",
+                "slices:",
                 "  - id: 1",
                 "    name: |-",
-                "      Phase Name",
+                "      Slice Name",
                 "    goal: |-",
-                "      What this phase achieves",
+                "      What this slice achieves",
                 "    tasks:",
                 "      - id: TASK-1-1",
                 "        description: |-",
@@ -11166,13 +11167,75 @@ def _build_phase_prompt(
                 "",
                 *_YAML_TASKS_SAFETY_GUIDANCE,
                 "",
-                "Do NOT use a `pr_plan` key or propose multiple PRs.",
+                "Do NOT use a `pr_plan` key — slice packaging is owned by the "
+                "slice-DAG section below, not by an ad-hoc PR list.",
                 "",
                 "The `test_plan` field is **required** — describe both automated test "
                 "coverage and any manual verification steps. The `manual_steps` field "
                 "should list any pre-merge or post-merge actions required by the reviewer "
                 "or deployer; use an empty string if none.",
                 "",
+                # ----------------------------------------------------
+                # #2137 — slice-DAG planner guidance (mirrors the
+                # concurrent task_planner block; keep the two paths
+                # aligned so a refine-phase multi-slice HITL decision
+                # is honoured regardless of which planner runs).
+                # ----------------------------------------------------
+                "## Slice-DAG guidance (#2137)",
+                "",
+                "The implement-phase pipeline ships each plan **slice** (formerly "
+                "**phase**) as its own stacked PR. The plan you emit drives that "
+                "DAG; the rules below are mandatory.",
+                "",
+                "**Yaml key swap**: prefer the canonical ``slices:`` key in your "
+                "``# yaml-tasks`` block (the parser also accepts ``phases:`` for "
+                "backward compatibility). New plans should use ``slices:``.",
+                "",
+                "**Slice-sizing guidance (soft, advisory only)**: target ≤1,000 "
+                "LOC per slice where possible. The plan reviewer flags oversized "
+                "slices as advisory but does NOT reject on size.",
+                "",
+                "**Forest constraint (HARD)**: every slice must have at most ONE "
+                "DAG parent — the implement-phase pipeline ships every slice as a "
+                "stacked PR with exactly one base branch. Multi-parent slices "
+                "break the stacking invariant and are rejected at plan ingestion.",
+                "",
+                "**Auto-serialization rule for would-be multi-parent slices**: "
+                "when a slice would naturally have >1 parents, serialise the "
+                "upstream slices into a linear chain and record the chosen "
+                "ordering on the downstream slice's ``serialized_chain_order`` "
+                "field. The list names the upstream slice IDs in their chosen "
+                "serialization order.",
+                "",
+                "Worked example: if ``slice-3`` would naturally have "
+                "parents ``[slice-1, slice-2]``, instead emit:",
+                "",
+                "```yaml",
+                "  - id: 1",
+                "    name: |-",
+                "      Foundations",
+                "    # ... (root)",
+                "  - id: 2",
+                "    name: |-",
+                "      Middle",
+                "    dependencies:",
+                "      - slice-1",
+                "  - id: 3",
+                "    name: |-",
+                "      Downstream",
+                "    dependencies:",
+                "      - slice-2  # serialised — slice-2 is the only DAG parent",
+                "    serialized_chain_order:",
+                "      - slice-1",
+                "      - slice-2  # records that you deliberately picked",
+                "                 # slice-1 → slice-2 → slice-3",
+                "```",
+                "",
+                "Your judgement is the source of truth. The fallback heuristic "
+                "when you have no preference is: cluster would-be parents by "
+                "``files_affected`` Jaccard overlap (>0.3), then order by "
+                "descending downstream fan-out.",
+                "",
                 f"Write your plan to `{plan_path}`.",
                 "Commit and push the draft when done.",
                 "",
@@ -13031,11 +13094,13 @@ def _build_agent_prompt(
         draft_path = _get_draft_path("plan", issue_number=issue_number, pipeline_id=pipeline_id)
         lines.extend(
             [
-                "Decompose the architecture analysis into a single-PR implementation plan.",
-                "",
-                "**CRITICAL CONSTRAINT — One Issue = One Workflow = One PR.**",
-                "All tasks belong to a single pull request. Use phases and commits to",
-                "organise the work within that PR — do NOT propose multiple PRs.",
+                "Decompose the architecture analysis into a slice-DAG implementation "
+                "plan. The implement-phase pipeline ships each slice as its own "
+                "stacked PR — see the ``## Slice-DAG guidance (#2137)`` section below "
+                "for the slice-shaping rules. A single-slice plan is fine when the "
+                "work is cohesive; multi-slice plans are required when an upstream "
+                "refine-phase work-decomposition HITL decision selected a "
+                "multi-slice shape.",
                 "",
                 "Steps:",
                 "1. Review the architecture analysis from the ARCHITECT agent",
@@ -13073,12 +13138,12 @@ def _build_agent_prompt(
                 "    Pre-merge: any required steps before merging",
                 "    Post-merge: any required steps after merging",
                 *_PR_CONTEXT_YAML_EXAMPLE_LINES,
-                "phases:",
+                "slices:",
                 "  - id: 1",
                 "    name: |-",
-                "      Phase Name",
+                "      Slice Name",
                 "    goal: |-",
-                "      What this phase achieves",
+                "      What this slice achieves",
                 "    tasks:",
                 "      - id: TASK-1-1",
                 "        description: |-",
@@ -13094,7 +13159,8 @@ def _build_agent_prompt(
                 "",
                 *_YAML_TASKS_SAFETY_GUIDANCE,
                 "",
-                "Do NOT use a `pr_plan` key or propose multiple PRs.",
+                "Do NOT use a `pr_plan` key — slice packaging is owned by the "
+                "slice-DAG section below, not by an ad-hoc PR list.",
                 "",
                 "The `test_plan` field is **required** — describe both automated test "
                 "coverage and any manual verification steps. The `manual_steps` field "

orchestrator/tests/test_pipeline_prompts.py

@@ -3289,6 +3289,140 @@ def test_decomposition_guidance_lives_outside_template_fence(self):
             )
 
 
+class TestPlannerPromptSliceDagFraming:
+    """Planner prompts must not contradict slice-DAG decomposition (#2601).
+
+    Both planner paths — ``_build_phase_prompt(phase="plan")`` (sequential)
+    and ``_build_agent_prompt(role_value="task_planner")`` (concurrent) —
+    used to open with ``CRITICAL CONSTRAINT — One Issue = One Workflow =
+    One PR`` and a follow-on ``do NOT propose multiple PRs`` line. That
+    directly contradicts the slice-DAG guidance the concurrent path already
+    carried and silently turned multi-slice refine-phase HITL decisions
+    into dead letters. This class is a negative-regression suite: the
+    opener must not return, and the slice-DAG framing must remain.
+    """
+
+    @staticmethod
+    def _sequential_plan_prompt() -> str:
+        return _build_phase_prompt(
+            phase="plan",
+            pipeline_id="test-pipe",
+            pipeline_mode="issue",
+            prompt="Implement the change.",
+            issue_number=100,
+        )
+
+    @staticmethod
+    def _concurrent_planner_prompt() -> str:
+        return _build_agent_prompt(
+            role_value="task_planner",
+            phase="plan",
+            pipeline_id="test-pipe",
+            pipeline_mode="issue",
+            prompt="Implement the change.",
+            issue_number=100,
+            concurrent=True,
+        )
+
+    def test_sequential_plan_drops_one_pr_opener(self):
+        prompt = self._sequential_plan_prompt()
+        for needle in (
+            "CRITICAL CONSTRAINT",
+            "One Issue = One Workflow = One PR",
+            "do NOT propose multiple PRs",
+        ):
+            assert needle not in prompt, (
+                f"sequential planner still carries removed opener {needle!r} — "
+                "this contradicts slice-DAG decomposition (#2601)"
+            )
+
+    def test_concurrent_planner_drops_one_pr_opener(self):
+        prompt = self._concurrent_planner_prompt()
+        for needle in (
+            "CRITICAL CONSTRAINT",
+            "One Issue = One Workflow = One PR",
+            "do NOT propose multiple PRs",
+        ):
+            assert needle not in prompt, (
+                f"concurrent planner still carries removed opener {needle!r} — "
+                "this contradicts slice-DAG decomposition (#2601)"
+            )
+
+    def test_sequential_plan_includes_slice_dag_guidance(self):
+        prompt = self._sequential_plan_prompt()
+        for needle in (
+            "Slice-DAG guidance (#2137)",
+            "stacked PR",
+            "Forest constraint",
+            "serialized_chain_order",
+        ):
+            assert needle in prompt, (
+                f"sequential planner missing slice-DAG token {needle!r} — "
+                "the two planner paths must stay aligned (#2601)"
+            )
+
+    def test_concurrent_planner_includes_slice_dag_guidance(self):
+        prompt = self._concurrent_planner_prompt()
+        for needle in (
+            "Slice-DAG guidance (#2137)",
+            "stacked PR",
+            "Forest constraint",
+            "serialized_chain_order",
+        ):
+            assert needle in prompt, (
+                f"concurrent planner missing slice-DAG token {needle!r} — "
+                "removing the One-PR opener must not have dropped the "
+                "slice-DAG block (#2601)"
+            )
+
+    def test_sequential_plan_yaml_example_uses_slices_key(self):
+        """The canonical YAML example must use ``slices:`` — agents copy the
+        example verbatim, so leaving ``phases:`` in the example silently
+        teaches the legacy key while the slice-DAG section says to prefer
+        ``slices:`` (review feedback on #2607)."""
+        prompt = self._sequential_plan_prompt()
+        assert "\nslices:\n" in prompt, (
+            "sequential planner YAML example must use 'slices:' as the "
+            "canonical key (parser still accepts 'phases:' as a backward-"
+            "compat alias, but new prompts should teach 'slices:')"
+        )
+        assert "\nphases:\n" not in prompt, (
+            "sequential planner YAML example still uses 'phases:' — switch "
+            "to 'slices:' to match the slice-DAG directive in the same prompt"
+        )
+
+    def test_concurrent_planner_yaml_example_uses_slices_key(self):
+        """Concurrent planner's canonical YAML example must use ``slices:``
+        (review feedback on #2607 — parallel to the sequential path)."""
+        prompt = self._concurrent_planner_prompt()
+        assert "\nslices:\n" in prompt, (
+            "concurrent planner YAML example must use 'slices:' as the "
+            "canonical key (parser still accepts 'phases:' as a backward-"
+            "compat alias, but new prompts should teach 'slices:')"
+        )
+        assert "\nphases:\n" not in prompt, (
+            "concurrent planner YAML example still uses 'phases:' — switch "
+            "to 'slices:' to match the slice-DAG directive in the same prompt"
+        )
+
+    def test_sequential_plan_carries_worked_example_and_jaccard(self):
+        """The sequential planner's slice-DAG block must now include the
+        worked ``serialized_chain_order`` example and the Jaccard fallback
+        heuristic, mirroring the concurrent path (review feedback on
+        #2607 flagged the asymmetry as a likely copy-paste oversight)."""
+        prompt = self._sequential_plan_prompt()
+        for needle in (
+            "Worked example",
+            "Jaccard",
+            "files_affected",
+        ):
+            assert needle in prompt, (
+                f"sequential planner missing concurrent-mirror token "
+                f"{needle!r} — the worked example + Jaccard fallback must "
+                "be present in both planner paths (#2607)"
+            )
+
+
 class TestReviewerBrcPreamble:
     """Tests that reviewer agents receive BRC preamble in concurrent mode."""
 

shared/egg_contracts/plan_parser.py

@@ -262,12 +262,14 @@ def parse_yaml_code_fence(content: str) -> tuple[dict[str, Any] | None, str, lis
     The code fence must be formatted as:
     ```yaml
     # yaml-tasks
-    phases:
+    slices:
       - id: 1
-        name: Phase Name
+        name: Slice Name
         ...
     ```
 
+    The legacy ``phases:`` key is also accepted for backward compatibility.
+
     Args:
         content: The document content
 
@@ -414,10 +416,10 @@ def parse_phases_from_yaml(
     """
     Parse phases and tasks from structured YAML (yaml-tasks code fence format).
 
-    Expected format:
+    Expected format (post-#2137):
     ```yaml
     # yaml-tasks
-    phases:
+    slices:
       - id: 1
         name: Setup
         goal: Initialize the project
@@ -429,6 +431,9 @@ def parse_phases_from_yaml(
               - schema.json
     ```
 
+    The legacy ``phases:`` key is accepted as an alias for ``slices:``;
+    when both are present, ``slices:`` wins.
+
     Args:
         yaml_data: Parsed YAML data from code fence
 
@@ -446,8 +451,10 @@ def parse_phases_from_yaml(
     slices_list = yaml_data.get("slices", [])
     legacy_phases_list = yaml_data.get("phases", [])
 
-    # Reject multi-PR format: pr_plan key indicates the LLM proposed
-    # multiple PRs, which violates the one-issue-one-PR constraint.
+    # Reject ad-hoc multi-PR `pr_plan` format. Slice packaging is owned by
+    # the `slices:` DAG (one slice = one stacked PR, post-#2137); `pr_plan`
+    # is not a supported decomposition format regardless of whether the
+    # plan ships as one or many PRs.
     if "pr_plan" in yaml_data:
         if not slices_list and not legacy_phases_list:
             # pr_plan without slices/phases means the LLM put the task
@@ -456,18 +463,21 @@ def parse_phases_from_yaml(
                 ParseWarning(
                     line_number=None,
                     message="'pr_plan' key found without 'slices' or 'phases' — "
-                    "the plan uses the unsupported multi-PR format. Each issue must "
-                    "produce exactly one PR using the 'slices' (canonical) or "
-                    "'phases' (legacy) key.",
-                    context="The 'pr_plan' multi-PR format is not supported",
+                    "'pr_plan' is not a supported decomposition format. Use the "
+                    "'slices' (canonical, post-#2137) or 'phases' (legacy) key "
+                    "to express the slice DAG; the implement-phase pipeline "
+                    "ships each slice as its own stacked PR.",
+                    context="The 'pr_plan' format is not supported; use 'slices'",
                 )
             ]
         warnings.append(
             ParseWarning(
                 line_number=None,
-                message="'pr_plan' key is not supported — use 'pr' (singular) instead. "
-                "Each issue must produce exactly one PR.",
-                context="The 'pr_plan' multi-PR format will be ignored",
+                message="'pr_plan' key is not supported — use the 'slices' key "
+                "to express the slice DAG, and the singular 'pr' key for the "
+                "per-PR metadata block (title, description, test_plan, "
+                "manual_steps).",
+                context="The 'pr_plan' format will be ignored; use 'slices' + 'pr'",
             )
         )
 
@@ -829,10 +839,12 @@ def extract_pr_metadata_from_yaml(
       manual_steps: |
         Pre-merge: any steps before merging
         Post-merge: any steps after merging
-    phases:
+    slices:
       ...
     ```
 
+    The legacy ``phases:`` key is accepted as an alias for ``slices:``.
+
     Args:
         yaml_data: Parsed YAML data from code fence or frontmatter