From dfbb8649d0a255321b670ed005bda09dc3e8176c Mon Sep 17 00:00:00 2001 From: Renzo F <45401804+rennf93@users.noreply.github.com> Date: Thu, 25 Jun 2026 05:03:24 +0200 Subject: [PATCH] fix(coordination): stop phantom re-delegation from text-vs-id AC-ref mismatch (#259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A parent's acceptance-criteria coverage is matched by stable criterion id, but a PM may declare covers_parent_criteria on a child by EITHER the criterion's id OR its full text (both happen in practice). _parent_ac_ref_sets unioned the raw refs and matched by id only, so a COMPLETED child that declared coverage by text was invisible to the matcher: the criterion read "uncovered", the roll-up gate refused, and the PM re-delegated the already-finished work as a brand-new empty subtask (0 commits, no PR) that can never close — looping for hours and burning tokens (observed live: a parent's xenon work completed + merged via one child, then re-delegated 2h later as an empty phantom). Normalize every child ref to the criterion id (text -> id via the parent's own criteria) in a small _normalize_ac_refs helper, so coverage counts regardless of how it was declared. Fixes existing mismatched data and future declarations; all three consumers (uncovered/unclaimed/parent_ac_coverage) share the builder. An unknown ref (neither id nor a current criterion text) passes through and matches nothing, exactly as before. Adds two regression tests. Co-authored-by: Renn F --- roboco/services/task.py | 24 ++++++++++++++++++- tests/unit/services/test_task.py | 40 ++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/roboco/services/task.py b/roboco/services/task.py index 6695879b..5f5c6159 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -6577,7 +6577,7 @@ class TaskService(BaseService): verified: set[str] = set() any_declared = False for status, refs in result.all(): - refset = set(refs or []) + refset = self._normalize_ac_refs(parent, refs) any_declared = any_declared or bool(refset) if status != TaskStatus.CANCELLED: claimed |= refset @@ -6585,6 +6585,28 @@ class TaskService(BaseService): verified |= refset return parent, claimed, verified, any_declared + @staticmethod + def _normalize_ac_refs(parent: TaskTable, refs: list[str] | None) -> set[str]: + """Resolve a child's parent_ac_refs to parent criterion ids. + + A PM may declare covers_parent_criteria by either a criterion's stable + id OR its full text — both happen. Coverage is matched by id (see + _criteria_texts_not_in), so without this a text-declared ref from a + COMPLETED child reads "uncovered" and the PM re-delegates the already- + finished work as an empty phantom subtask (0 commits, no PR) that can + never close — observed live looping for hours. Map text -> id (via the + parent's own criteria) so coverage counts regardless of how it was + declared. An unknown ref (neither id nor a current criterion text) + passes through and matches nothing, exactly as before. + """ + ac_ids = parent.acceptance_criteria_ids or [] + valid_ids = set(ac_ids) + ac_texts = parent.acceptance_criteria or [] + text_to_id = { + text: ac_ids[idx] for idx, text in enumerate(ac_texts) if idx < len(ac_ids) + } + return {(r if r in valid_ids else text_to_id.get(r, r)) for r in (refs or [])} + @staticmethod def _criteria_texts_not_in(parent: TaskTable, covered: set[str]) -> list[str]: """Texts of the parent criteria whose id is not in ``covered``.""" diff --git a/tests/unit/services/test_task.py b/tests/unit/services/test_task.py index b74ae634..f2bb035d 100644 --- a/tests/unit/services/test_task.py +++ b/tests/unit/services/test_task.py @@ -672,6 +672,46 @@ async def test_uncovered_parent_acs_empty_when_all_covered() -> None: assert await svc.uncovered_parent_acceptance_criteria(parent.id) == [] +@pytest.mark.asyncio +async def test_uncovered_parent_acs_recognizes_text_declared_coverage() -> None: + # Regression (phantom re-delegation): a PM may declare covers_parent_criteria + # by the criterion's full TEXT instead of its id. Matching is by id, so a + # COMPLETED child that declared coverage by text used to read "uncovered" — + # and the PM re-delegated the already-finished work as an empty phantom + # subtask (0 commits, no PR) that could never close. _parent_ac_ref_sets now + # normalizes text -> id so coverage counts regardless of how it was declared. + parent = _build_task( + acceptance_criteria=["crit a", "crit b"], + acceptance_criteria_ids=["id-a", "id-b"], + ) + svc = _svc_with_children( + parent, + [ + (TaskStatus.COMPLETED, ["crit a"]), # declared by TEXT, not "id-a" + (TaskStatus.COMPLETED, ["id-b"]), # declared by id + ], + ) + assert await svc.uncovered_parent_acceptance_criteria(parent.id) == [] + + +@pytest.mark.asyncio +async def test_parent_ac_coverage_normalizes_text_refs() -> None: + # A text-declared coverage ref from a COMPLETED child surfaces as + # claimed+verified, same as an id-declared one. + parent = _build_task( + acceptance_criteria=["crit a", "crit b"], + acceptance_criteria_ids=["id-a", "id-b"], + ) + svc = _svc_with_children(parent, [(TaskStatus.COMPLETED, ["crit a"])]) + cov = await svc.parent_ac_coverage(parent.id) + assert cov[0] == { + "id": "id-a", + "text": "crit a", + "claimed": True, + "verified": True, + } + + @pytest.mark.asyncio async def test_parent_ac_coverage_maps_claimed_and_verified() -> None: # Per-criterion visibility: a COMPLETED child both claims and verifies its