mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(coordination): stop phantom re-delegation from text-vs-id AC-ref mismatch (#259)
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 <rennf93@users.noreply.github.com>
This commit is contained in:
+23
-1
@@ -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``."""
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user