fix(gateway): cross-team planning fanout + complete tracing-gap hints

Task #157 — spine-cap allows planning fanout across cells:
    main-pm's pattern is to delegate planning to be-pm / fe-pm / ux-pm
    in parallel — each on a different team. The previous spine-cap
    rejected all planning siblings under one parent as
    over-decomposition. New helper _is_cross_team_planning skips the
    cap for planning when both teams are non-empty and distinct. Code
    / documentation stay capped regardless (single repo on one branch
    shouldn't have two simultaneous code subtasks).

Task #159 — tracing-gap remediate hints every requirement:
    journal:during_work>=1, journal:struggle, commits>=1, pr_open,
    and self_verified had no entries in _hint_for_missing_key, so when
    they were missing the agent saw the token in `missing[]` but the
    `remediate` text had no instruction for how to satisfy them.
    Smoke-10's be-dev-1 burned multiple turns retrying i_am_done not
    knowing scope='reflect' doesn't count toward during_work. Now
    every token has a hint, the during_work hint warns that reflect
    doesn't satisfy it, and multi-hint remediate uses a numbered list
    so the model treats each requirement as a distinct step instead
    of a semicolon-blob.

Also coerce convert_plan._coerce_risk formatting (ruff-format follow-up
to 9cd73d0).
This commit is contained in:
Renn F
2026-05-15 08:21:08 +02:00
parent 9cd73d0902
commit 5da909d9d7
4 changed files with 404 additions and 7 deletions
+5 -1
View File
@@ -532,7 +532,11 @@ def convert_plan(plan_data: dict | None) -> TaskPlanResponse | None:
def _coerce_risk(r: object) -> dict[str, str]: def _coerce_risk(r: object) -> dict[str, str]:
if not isinstance(r, dict): if not isinstance(r, dict):
return {"description": str(r) if r else "", "mitigation": "", "severity": "medium"} return {
"description": str(r) if r else "",
"mitigation": "",
"severity": "medium",
}
sev = r.get("severity") sev = r.get("severity")
return { return {
"description": str(r.get("description") or r.get("risk") or ""), "description": str(r.get("description") or r.get("risk") or ""),
+93 -6
View File
@@ -1696,15 +1696,54 @@ class Choreographer:
"cancelled) before this transition; wait for the closure " "cancelled) before this transition; wait for the closure "
"dispatcher to bring you back when ready." "dispatcher to bring you back when ready."
), ),
"journal:during_work>=1": (
"no journal:decision / :learning / :struggle entry exists "
"for this task yet. Pre-gateway parity: developers write "
"at least one work-progress journal entry before submit. "
f"Call note(scope='decision'|'learning'|'struggle', "
f"task_id='{tid}', text='<what you decided or learned>') "
"with a substantive entry, then retry i_am_done. "
"NOTE: scope='reflect' does NOT count for this requirement — "
"reflect is the post-work summary; during_work demands "
"an entry written while the work was happening."
),
"journal:struggle": (
f"call note(scope='struggle', task_id='{tid}', "
"text='<what is blocking you>') with the blocker details, "
"then retry."
),
"commits>=1": (
"no commits linked to this task yet. Use commit(message=...) "
"to record your changes, then retry."
),
"pr_open": (
"no PR has been opened for this task. Call open_pr() to "
"push the branch + open the PR, then retry."
),
"self_verified": (
"task has not been self-verified. i_am_done normally runs "
"submit_verification automatically; if you see this gap, "
"retry i_am_done after the previous call returned."
),
} }
return simple_hints.get(missing_key) return simple_hints.get(missing_key)
async def _build_tracing_gap( async def _build_tracing_gap(
self, agent_id: UUID, task_id: UUID, missing: list[str] self, agent_id: UUID, task_id: UUID, missing: list[str]
) -> Envelope: ) -> Envelope:
"""Translate missing requirement keys into agent-facing hints.""" """Translate missing requirement keys into agent-facing hints.
Task #159: multi-missing remediate uses a numbered list so the
agent sees each requirement as a distinct step instead of a
single semicolon-joined sentence the model parses as one
instruction. Each missing key with no hint in
``_hint_for_missing_key`` still surfaces as a literal
``missing[]`` entry (defense-in-depth: the agent gets at least
the key name even if no hint is registered).
"""
hints: list[str] = [] hints: list[str] = []
unaddressed: list[str] = [] unaddressed: list[str] = []
unhinted: list[str] = []
for m in missing: for m in missing:
if m.startswith("acceptance_criterion:"): if m.startswith("acceptance_criterion:"):
unaddressed.append(m.split(":", 1)[1]) unaddressed.append(m.split(":", 1)[1])
@@ -1712,6 +1751,8 @@ class Choreographer:
hint = self._hint_for_missing_key(m, task_id) hint = self._hint_for_missing_key(m, task_id)
if hint is not None: if hint is not None:
hints.append(hint) hints.append(hint)
else:
unhinted.append(m)
if unaddressed: if unaddressed:
hints.append( hints.append(
hint_for_unaddressed_acceptance_criteria( hint_for_unaddressed_acceptance_criteria(
@@ -1719,9 +1760,25 @@ class Choreographer:
task_id=str(task_id), task_id=str(task_id),
) )
) )
# Fallback hints for missing keys without a registered hint —
# the agent at least sees the literal token instead of nothing.
for token in unhinted:
hints.append(
f"requirement {token!r} not satisfied — see lifecycle docs "
f"or escalate via i_am_blocked if you do not know how to "
f"satisfy this."
)
if len(hints) <= 1:
remediate = hints[0] if hints else ""
else:
numbered = "\n".join(f"{i + 1}. {h}" for i, h in enumerate(hints))
remediate = (
f"Multiple requirements missing — address ALL of the "
f"following before retrying:\n{numbered}"
)
return Envelope.tracing_gap( return Envelope.tracing_gap(
missing=missing, missing=missing,
remediate=" ; ".join(hints), remediate=remediate,
context_briefing=await self._briefing_for(agent_id, task_id), context_briefing=await self._briefing_for(agent_id, task_id),
) )
@@ -2563,14 +2620,38 @@ class Choreographer:
context_briefing={}, context_briefing={},
) )
@staticmethod
def _is_cross_team_planning(new_type: str, new_team: str, sib_team: str) -> bool:
"""Task #157: planning subtasks on different teams are NOT
over-decomposition — main_pm fans planning out to per-cell PMs.
Both teams must be non-empty so an empty-team escape hatch can't
bypass the cap defensively.
"""
return (
new_type == "planning"
and bool(new_team)
and bool(sib_team)
and new_team != sib_team
)
@classmethod @classmethod
def _sibling_dup_envelope( def _sibling_dup_envelope(
cls, sibling: Any, new_type: str, new_assignee: str cls, sibling: Any, new_type: str, new_team: str, new_assignee: str
) -> Envelope | None: ) -> Envelope | None:
"""Apply Rule-1 then Rule-2 against one non-terminal sibling.""" """Apply Rule-1 then Rule-2 against one non-terminal sibling.
``code`` / ``documentation`` stay capped regardless of team —
a single repo on one branch shouldn't have two simultaneous code
subtasks. ``planning`` allows cross-team fanout (see
:meth:`_is_cross_team_planning`).
"""
sib_type = str(getattr(sibling, "task_type", "")) sib_type = str(getattr(sibling, "task_type", ""))
sib_team = str(getattr(sibling, "team", "") or "")
sib_assignee = str(getattr(sibling, "assigned_to", "") or "") sib_assignee = str(getattr(sibling, "assigned_to", "") or "")
if new_type in cls._SPINE_TASK_TYPES and sib_type == new_type: same_spine_type = new_type in cls._SPINE_TASK_TYPES and sib_type == new_type
if same_spine_type and not cls._is_cross_team_planning(
new_type, new_team, sib_team
):
return cls._spine_type_dup_envelope(new_type, sibling, sib_assignee) return cls._spine_type_dup_envelope(new_type, sibling, sib_assignee)
if sib_assignee and sib_assignee == new_assignee and sib_type == new_type: if sib_assignee and sib_assignee == new_assignee and sib_type == new_type:
return cls._same_assignee_dup_envelope(new_type, new_assignee, sibling) return cls._same_assignee_dup_envelope(new_type, new_assignee, sibling)
@@ -2588,6 +2669,9 @@ class Choreographer:
1. **Same-type concurrency cap**: a parent may have AT MOST one 1. **Same-type concurrency cap**: a parent may have AT MOST one
non-terminal subtask of types ``code`` / ``planning`` / non-terminal subtask of types ``code`` / ``planning`` /
``documentation`` at any given time, regardless of assignee. ``documentation`` at any given time, regardless of assignee.
Task #157 exception: ``planning`` subtasks on different
teams are allowed in parallel — that's main_pm's legitimate
cross-cell fanout.
2. **Same-assignee same-type** (fallback): a PM never delegates two 2. **Same-assignee same-type** (fallback): a PM never delegates two
``research``/``design``/``administrative`` subtasks to the same ``research``/``design``/``administrative`` subtasks to the same
agent under the same parent. agent under the same parent.
@@ -2597,11 +2681,14 @@ class Choreographer:
""" """
siblings = await self.task.get_subtasks(parent_task_id) siblings = await self.task.get_subtasks(parent_task_id)
new_type = str(inputs.task_type or "") new_type = str(inputs.task_type or "")
new_team = str(inputs.team or "")
new_assignee = str(inputs.assigned_to or "") new_assignee = str(inputs.assigned_to or "")
for sibling in siblings: for sibling in siblings:
if str(getattr(sibling, "status", "")) in self._TERMINAL_STATUSES: if str(getattr(sibling, "status", "")) in self._TERMINAL_STATUSES:
continue continue
envelope = self._sibling_dup_envelope(sibling, new_type, new_assignee) envelope = self._sibling_dup_envelope(
sibling, new_type, new_team, new_assignee
)
if envelope is not None: if envelope is not None:
return envelope return envelope
return None return None
@@ -0,0 +1,128 @@
"""Task #157: spine-cap allows cross-team planning fanout.
Pre-fix:
main_pm delegates a planning subtask to be-pm (backend cell). When
it then tries to delegate a second planning subtask to fe-pm
(frontend cell), the spine-cap rejects because there's already a
non-terminal task_type='planning' under the parent. Pre-gateway
allowed this parallel cross-cell pattern; the gateway over-applied
the over-decomposition cap.
Fix:
`_sibling_dup_envelope` skips the spine-cap when:
- new task_type == "planning"
- new team != sibling team (both non-empty)
Other combinations stay capped:
- same-team planning: still over-decomposition (real bug)
- code / documentation regardless of team: a single repo on one
branch shouldn't have two simultaneous code subtasks
"""
from __future__ import annotations
from unittest.mock import MagicMock
from roboco.services.gateway.choreographer._impl import Choreographer
def _sibling(*, task_type: str, team: str, status: str = "pending") -> MagicMock:
sib = MagicMock()
sib.id = "11111111-aaaa-bbbb-cccc-dddddddddddd"
sib.status = status
sib.task_type = task_type
sib.team = team
sib.assigned_to = "some-pm"
return sib
def test_cross_team_planning_fanout_is_allowed() -> None:
"""main-pm: planning→be-pm (backend) exists; planning→fe-pm (frontend) must pass."""
sib = _sibling(task_type="planning", team="backend")
env = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="planning",
new_team="frontend",
new_assignee="fe-pm",
)
assert env is None, (
"Cross-team planning fanout (backend↔frontend) must NOT be rejected. "
f"Got envelope: {env}"
)
def test_cross_team_planning_third_cell_also_allowed() -> None:
"""Third cell (ux_ui) is also a valid cross-team planning fanout target."""
sib = _sibling(task_type="planning", team="backend")
env = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="planning",
new_team="ux_ui",
new_assignee="ux-pm",
)
assert env is None, env
def test_same_team_planning_still_rejected() -> None:
"""Two planning subtasks on the SAME team is the real over-decomp pattern —
must still be blocked by the spine-cap."""
sib = _sibling(task_type="planning", team="backend")
env = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="planning",
new_team="backend",
new_assignee="be-pm",
)
assert env is not None
body = env.as_dict()
assert body["error"] == "invalid_state", body
def test_cross_team_code_still_rejected() -> None:
"""Code subtasks stay capped regardless of team — only one code task per
parent at a time. (Cross-team code under one parent is meaningless;
each cell's code work lives under its own cell-PM planning task.)"""
sib = _sibling(task_type="code", team="backend")
env = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="code",
new_team="frontend",
new_assignee="fe-dev-1",
)
assert env is not None
body = env.as_dict()
assert body["error"] == "invalid_state", body
def test_cross_team_documentation_still_rejected() -> None:
"""Documentation subtasks stay capped regardless of team — single doc
pass per parent."""
sib = _sibling(task_type="documentation", team="backend")
env = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="documentation",
new_team="frontend",
new_assignee="fe-doc",
)
assert env is not None
def test_planning_with_missing_team_still_rejected() -> None:
"""If either side has no team attribute (defensive), fall back to the
strict cap. Don't let an empty-team escape hatch sneak past."""
sib_no_team = _sibling(task_type="planning", team="")
env = Choreographer._sibling_dup_envelope(
sibling=sib_no_team,
new_type="planning",
new_team="backend",
new_assignee="be-pm",
)
assert env is not None, "Empty team on sibling must NOT bypass the cap"
sib = _sibling(task_type="planning", team="backend")
env2 = Choreographer._sibling_dup_envelope(
sibling=sib,
new_type="planning",
new_team="",
new_assignee="be-pm",
)
assert env2 is not None, "Empty team on new task must NOT bypass the cap"
@@ -0,0 +1,178 @@
"""Task #159: tracing-gap remediate covers every missing requirement.
Pre-fix:
`journal:during_work>=1` (and a handful of other tokens) had no entry
in `_hint_for_missing_key`'s `simple_hints` dict. When the
requirement was missing, the agent saw the token in `missing[]` but
the `remediate` string contained NO instruction for how to satisfy
it. The agent fixed the items with hints, retried, hit the same
rejection again on the unhinted token, and looped.
Fix:
Register hints for the previously-unhinted tokens
(`journal:during_work>=1`, `journal:struggle`, `commits>=1`,
`pr_open`, `self_verified`). Multi-hint remediate switches to a
numbered list so the model sees each requirement as a distinct
instruction instead of a semicolon-blob.
"""
from __future__ import annotations
from unittest.mock import AsyncMock
from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
_MIN_HINT_LEN = 10
def _build_choreographer() -> Choreographer:
deps = ChoreographerDeps(
task=AsyncMock(),
work_session=AsyncMock(),
git=AsyncMock(),
a2a=AsyncMock(),
journal=AsyncMock(),
audit=AsyncMock(),
evidence_repo=AsyncMock(),
messaging=AsyncMock(),
)
repo = deps.evidence_repo
for method in (
"list_unread_a2a",
"list_unread_mentions",
"list_pending_notifications",
"task_metadata_gaps",
"recent_team_activity",
"blockers_in_lane",
"journal_highlights_for_task",
):
getattr(repo, method).return_value = []
return Choreographer(deps)
# ---------------------------------------------------------------------------
# Hint registration — every previously-unhinted token now has a hint
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
"token",
[
"journal:during_work>=1",
"journal:struggle",
"commits>=1",
"pr_open",
"self_verified",
],
)
def test_hint_registered_for_previously_unhinted_token(token: str) -> None:
"""Every requirement token must have a non-empty hint so the agent
knows how to satisfy it."""
hint = Choreographer._hint_for_missing_key(token, uuid4())
assert hint is not None, (
f"requirement token {token!r} has no hint; agent will see the "
f"token in `missing[]` but no actionable instruction"
)
assert len(hint) > _MIN_HINT_LEN, (
f"hint for {token!r} is suspiciously short: {hint!r}"
)
def test_during_work_hint_warns_reflect_doesnt_count() -> None:
"""The during_work hint must explicitly say reflect doesn't satisfy
it that's the exact confusion smoke-10's be-dev-1 hit."""
hint = Choreographer._hint_for_missing_key("journal:during_work>=1", uuid4())
assert hint is not None
lower = hint.lower()
assert "reflect" in lower and (
"does not count" in lower
or "doesn't count" in lower
or "do not count" in lower
or "not count" in lower
), f"during_work hint must warn that scope='reflect' doesn't satisfy this: {hint!r}"
# ---------------------------------------------------------------------------
# Multi-hint remediate — numbered list, not semicolon-joined
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_single_missing_remediate_is_plain_string() -> None:
"""One missing item: remediate is the hint itself, no numbering."""
c = _build_choreographer()
env = await c._build_tracing_gap(uuid4(), uuid4(), ["journal:reflect"])
body = env.as_dict()
assert body["error"] == "tracing_gap"
assert body["missing"] == ["journal:reflect"]
assert "Multiple requirements missing" not in (body["remediate"] or "")
assert "1." not in (body["remediate"] or "")
@pytest.mark.asyncio
async def test_multi_missing_remediate_is_numbered_list() -> None:
"""Multiple missing items: remediate must be a numbered list so the
agent treats each as a distinct step."""
c = _build_choreographer()
env = await c._build_tracing_gap(
uuid4(),
uuid4(),
["journal:reflect", "journal:during_work>=1", "commits>=1"],
)
body = env.as_dict()
remediate = body["remediate"] or ""
assert "Multiple requirements missing" in remediate, remediate
assert "1." in remediate
assert "2." in remediate
assert "3." in remediate
# All three hints must be substring-present (proves each was emitted).
assert "reflect" in remediate.lower()
assert "during" in remediate.lower() or "decision" in remediate.lower()
assert "commit" in remediate.lower()
@pytest.mark.asyncio
async def test_unhinted_token_still_appears_in_remediate() -> None:
"""If a future requirement is added without a hint (defense), the
agent at least sees the literal token in the remediate not silently
dropped."""
c = _build_choreographer()
env = await c._build_tracing_gap(
uuid4(),
uuid4(),
["unknown_future_requirement"],
)
body = env.as_dict()
remediate = body["remediate"] or ""
assert "unknown_future_requirement" in remediate, (
f"unhinted token must surface in remediate as fallback. Got: {remediate!r}"
)
@pytest.mark.asyncio
async def test_acceptance_criteria_grouped_into_one_hint() -> None:
"""Acceptance criteria entries collapse into a single hint regardless
of how many criteria are missing (so 5 unaddressed criteria don't
produce 5 separate numbered items)."""
c = _build_choreographer()
task_id = uuid4()
env = await c._build_tracing_gap(
uuid4(),
task_id,
[
"acceptance_criterion:Branch named correctly",
"acceptance_criterion:Commit prefix present",
"acceptance_criterion:PR opened",
],
)
body = env.as_dict()
remediate = body["remediate"] or ""
# All three criterion names should appear in the SAME hint section
# (collapsed into one hint, not three separate numbered items).
assert "Branch named correctly" in remediate
assert "Commit prefix present" in remediate
assert "PR opened" in remediate
# Single-hint case → no "Multiple requirements" preamble.
assert "Multiple requirements missing" not in remediate