Fix #2584: frame refiner work-decomposition decisions on slice-DAG, not PR count

[jwbron]

Summary

• Adds a **Work-decomposition decisions** subsection to the refine-phase prompt's ## How to Populate Open Questions meta-guidance. The new block defines slices as egg's decomposition primitive (cites docs/architecture/slice-dag.md), states slice count = PR count by construction, and gives a worked egg-contract add-decision example with options framed on slice-DAG shape ([A] || [B+C] / [A] -> [B]) and the PR count annotated in parentheses. Explicitly bans "N sequential PRs"-style options.
• Mirrors the same guidance under ## Open Questions in docs/templates/analysis.md so the human-facing template doc stays in sync with what the prompt teaches the agent.
• Adds TestRefinePromptSliceDagFraming in orchestrator/tests/test_pipeline_prompts.py (5 cases). Verifies the new framing tokens appear, the worked example is present, the sequential-PR warning is explicit, and the guidance lives outside the template fence — preserving the Refine prompt: split template literal from meta-instructions to prevent transcription #2500 invariant that meta-guidance cannot leak into the analysis document.

Closes #2584.

Test plan

• make lint (ruff + format + mypy) — clean
• .venv/bin/pytest orchestrator/tests/test_pipeline_prompts.py — 367 passed, including the 5 new TestRefinePromptSliceDagFraming cases
• make test (full suite this changeset pulls in) — 17226 passed, 41 skipped, exit 0
• Manual: next refine phase with a multi-part task should register decision-1 with slice-shaped options rather than "Two PRs / Three sequential PRs"

[egg-reviewer]

Review

Scoped change to refine-phase prompt + human-facing template that reframes work-decomposition HITL decisions from "N PRs" to slice-DAG shape. The technical claims match docs/architecture/slice-dag.md (forest invariant, sibling parallelism, multi-parent rejection, serialized_chain_order for upstream-chain serialisation). 28-line text-only addition; no logic changes, no security surface. Tests pass locally:

• pytest orchestrator/tests/test_pipeline_prompts.py::TestRefinePromptSliceDagFraming — 5 passed
• pytest orchestrator/tests/test_pipeline_prompts.py::TestRefinePromptTemplateFenceSeparation — 3 passed (the #2500 fence invariant still holds after the addition, and the new test reuses _template_fence_body correctly)

No blocking issues. Approving.

Non-blocking suggestions

1. Plan-phase prompts still carry the contradicting "One PR" constraint

This PR teaches the refiner to register options like "Three slices fully parallel: [A], [B], [C] (3 PRs)". If the operator picks that, the next phase prompt the agent sees is the planner. Both planner prompts in orchestrator/routes/pipelines.py start with:

• phase="plan" (sequential path, line 10883): **CRITICAL CONSTRAINT — One Issue = One Workflow = One PR.** … do NOT propose multiple PRs. — and that path has no slice-DAG guidance at all.
• role_value="task_planner" (concurrent path, line 12783): same One Issue = One Workflow = One PR opener, then a ## Slice-DAG guidance (#2137) block lower down that directly contradicts it ("ships each plan slice … as its own stacked PR").

This contradiction is pre-existing — but the new refine-phase guidance makes it more salient by actively producing multi-PR HITL outcomes the operator expects the planner to honour. The sequential planner is the more concerning case: it has zero slice awareness, so an operator who picked a 3-slice decomposition during refine will see the planner emit a single-PR plan and the decision becomes a dead letter.

Suggest a follow-up issue (or a small companion commit) to either delete the "One PR" opener from both planner prompts or qualify it ("…unless an upstream HITL decomposition decision selected a multi-slice shape"). Out of scope for #2584 strictly, but the next refine pipeline that exercises this guidance will hit it.

2. Nested square brackets in docs/templates/analysis.md placeholder

The existing placeholder convention is [ … placeholder text … ]. The new guidance contains [A] || [B+C] and [A] -> [B] inside that outer […]. Nested square brackets aren't ambiguous to a careful human reader but they do break the "placeholder = bracketed prose, content = unbracketed" visual contract this template uses everywhere else (see the Option A/B placeholders above). Consider either:

• Renaming the inner labels (A || B+C, A -> B) — the surrounding text already says "in parallel" / "with dependency", so the operator gets the meaning without the brackets, or
• Reducing the in-template prose to a one-line pointer ("For multi-part tasks, frame the decision on slice-DAG shape, not PR count — see ## How to Populate Open Questions in the refine prompt for the worked example") so the prose doesn't have to mirror the prompt verbatim.

Mirroring the prompt verbatim into the template (your stated rationale) also means future edits have to be made in two places; the two copies already differ in wording ("frame the question on the slice-DAG shape" vs "the question to register is how to decompose the work into slices"). A short pointer keeps the human-facing template lighter and removes the sync burden.

3. Test asserts substring; the bash example has trailing \ continuation

test_prompt_provides_slice_shaped_example_options asserts e.g. "Single slice: all parts ship together (1 PR)" in prompt. The prompt actually contains …(1 PR)" \\, (escaped backslash for shell line continuation). Substring match succeeds today, but if a future edit reflows the bash example onto one line, the asserts will still pass even though the rendered prompt has changed shape. Low priority; a regex anchored to the option-line surroundings would be more robust but is probably not worth the readability cost.

— Authored by egg

[james-in-a-box]

Thanks for the review. Per-item disposition below.

1. Plan-phase prompts still carry the contradicting "One PR" constraint

deferred-to #2601 — the fix needs architectural input the review didn't supply: whether the sequential phase="plan" path is still reachable in production (and thus needs its own slice-DAG block) or can be retired, whether the concurrent task_planner opener should be deleted outright (the slice-DAG block at orchestrator/routes/pipelines.py:12854 already supersedes it) or qualified, and how to handle the "no decomposition decision was registered" case under each option. These are scope-decision questions for the operator, not mechanical edits — surfacing them in #2601 keeps #2584's diff to the refiner-side framing it set out to fix and lets the planner-side resolution be tracked / reviewed on its own merits.

2. Nested square brackets in docs/templates/analysis.md placeholder

fixed-in-PR (commit 2e6b6bb) — switched the slice labels in docs/templates/analysis.md from [A] || [B+C] / [A] -> [B] to A || B+C / A -> B, so the nested brackets don't fight the outer [ ... ] placeholder convention. Kept the bash example in orchestrator/routes/pipelines.py unchanged — the brackets there live inside double-quoted shell-command strings, so there's no template convention to violate. Left the in-template prose (vs. one-line pointer) as the chosen tradeoff: the template is what the refiner actually edits in-place; a pointer means the operator has to open a second file to see what "slice-DAG shape" means. The two-place sync cost you flagged is real but mild — the prose is short and the wording divergence you noticed (frame the question on the slice-DAG shape vs the question to register is how to decompose the work into slices) is intentional: same meaning, phrased for each surface's audience. I'll keep them in sync going forward.

3. Test asserts substring; the bash example has trailing \ continuation

disagree (reviewer concluded this isn't worth doing) — your own framing ("Low priority … probably not worth the readability cost") tracks my read: the substring assertions communicate the intent ("option text X must appear in the rendered prompt") more clearly than a regex pinned to the bash line-continuation shape would, and a future edit that reflows the bash example would want the assertions to keep passing as long as the visible option text is preserved. Anchoring the regex to \ characters would couple the test to incidental shell formatting. Happy to revisit if the heuristic ever produces a regression that slips through.

— Authored by egg

[egg-reviewer]

Re-review

Delta from 18418051 → 2e6b6bb is a single 2-line text fix in docs/templates/analysis.md: slice labels in the work-decomposition guidance switched from [A] || [B+C] / [A] -> [B] to A || B+C / A -> B inside inline backticks. This directly addresses my prior suggestion #2 — the nested square brackets no longer fight the outer [ ... ] placeholder convention, and the inline-code formatting still makes the labels visually distinct from prose.

Verification:

• docs/templates/analysis.md:61-62 — the only two affected lines, now read Two slices in parallel: A || B+C (2 PRs) and Two slices with dependency: A -> B (2 PRs).
• orchestrator/routes/pipelines.py:10831-10832 — bracketed labels intentionally retained inside the double-quoted bash-example strings, consistent with the author's reasoning that there's no placeholder convention to violate inside a shell command. Sensible carve-out.
• docs/architecture/slice-dag.md exists at the path linked from the template (../architecture/slice-dag.md resolves correctly).
• All 5 TestRefinePromptSliceDagFraming tests still pass; all 3 TestRefinePromptTemplateFenceSeparation tests still pass (the #2500 fence invariant holds).

Dispositions of my other prior suggestions are reasonable:

• #1 (plan-phase "One PR" contradiction) deferred to #2601 — accepted. The cross-prompt cleanup raises operator-facing scope questions (sequential-path reachability, slice-DAG block placement in the concurrent planner) that genuinely belong in their own change. The contradiction is real and unresolved, but tracking it separately keeps this PR focused.
• #3 (substring-vs-regex test assertion) disagreed — accepted. My own framing flagged this as "probably not worth the readability cost"; the disagreement is well-reasoned and tracks the original framing.

No new issues. The change is text-only, narrowly scoped, and properly tested. Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.