mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(progress): plan-driven progress — % derived from the plan checklist (#173)
Progress was only the synthetic milestone entry (auto-emitted at open_pr/i_am_done); agents never deliberately reported and the % was an ungated free-form guess. Now the plan's sub_tasks ARE the progress skeleton: - progress() gains optional `plan_step` (a sub_task id or its 1-based order). With it, that step is marked completed and the percentage is DERIVED as completed/total (equal weight) via new TaskService.record_plan_progress — the agent cannot set/game it. - A narrative entry WITHOUT plan_step is allowed for important mid-step documentation and carries the current derived % (the bar never regresses). No hard anti-spam gate (would loop minimax) — prompt guidance steers "meaningful moments, not every tool call". - `percentage` is now an optional fallback, used only for tasks with no sub_task checklist (back-compat). v2 ProgressRequest, the do.py route, and the do_server MCP tool updated accordingly. - An unmatched plan_step returns invalid_state listing the valid step refs (resolve by id / order / 1-based index). - developer + documenter prompts updated to the plan_step workflow. - Helpers extracted (_plan_subtasks/_derive_plan_pct/_valid_step_refs/ _mark_subtask_complete) to keep record_plan_progress within the cyclomatic gate. Commit 3 of 3 for the plan/progress quality work (#171/#172/#173).
This commit is contained in:
@@ -709,3 +709,92 @@ async def test_reflect_incomplete_input_includes_call_example() -> None:
|
||||
assert "what_done=" in remediate, remediate
|
||||
assert "what_learned=" in remediate, remediate
|
||||
assert "what_struggled=" in remediate, remediate
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #173: plan-driven progress — progress() marks a plan step + derives %.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _active_task(agent_id: object) -> MagicMock:
|
||||
return MagicMock(id=uuid4(), assigned_to=agent_id, status="in_progress")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_progress_plan_step_resolved_ok_with_derived_pct() -> None:
|
||||
agent_id = uuid4()
|
||||
t = _active_task(agent_id)
|
||||
task = AsyncMock()
|
||||
task.get.return_value = t
|
||||
task.record_plan_progress.return_value = {
|
||||
"task": t,
|
||||
"percentage": 50,
|
||||
"step_resolved": True,
|
||||
"valid_steps": ["s1", "s2"],
|
||||
}
|
||||
actions = ContentActions(_make_deps(task=task))
|
||||
|
||||
env = await actions.progress(
|
||||
agent_id=agent_id, task_id=t.id, message="did step 1", plan_step="s1"
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body.get("error") is None, body
|
||||
assert "50%" in body["next"], body
|
||||
task.record_plan_progress.assert_awaited_once()
|
||||
assert task.record_plan_progress.await_args.kwargs["plan_step"] == "s1"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_progress_unknown_plan_step_invalid_state_lists_valid() -> None:
|
||||
agent_id = uuid4()
|
||||
t = _active_task(agent_id)
|
||||
task = AsyncMock()
|
||||
task.get.return_value = t
|
||||
task.record_plan_progress.return_value = {
|
||||
"task": t,
|
||||
"percentage": 0,
|
||||
"step_resolved": False,
|
||||
"valid_steps": ["s1", "s2"],
|
||||
}
|
||||
actions = ContentActions(_make_deps(task=task))
|
||||
|
||||
env = await actions.progress(
|
||||
agent_id=agent_id, task_id=t.id, message="?", plan_step="bogus"
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state", body
|
||||
assert "s1" in body["remediate"] and "s2" in body["remediate"], body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_progress_narrative_without_plan_step_ok() -> None:
|
||||
agent_id = uuid4()
|
||||
t = _active_task(agent_id)
|
||||
task = AsyncMock()
|
||||
task.get.return_value = t
|
||||
task.record_plan_progress.return_value = {
|
||||
"task": t,
|
||||
"percentage": 25,
|
||||
"step_resolved": None,
|
||||
"valid_steps": ["s1"],
|
||||
}
|
||||
actions = ContentActions(_make_deps(task=task))
|
||||
|
||||
env = await actions.progress(
|
||||
agent_id=agent_id, task_id=t.id, message="midway milestone"
|
||||
)
|
||||
assert env.as_dict().get("error") is None
|
||||
assert task.record_plan_progress.await_args.kwargs["plan_step"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_progress_ownership_enforced() -> None:
|
||||
agent_id = uuid4()
|
||||
t = MagicMock(id=uuid4(), assigned_to=uuid4(), status="in_progress")
|
||||
task = AsyncMock()
|
||||
task.get.return_value = t
|
||||
actions = ContentActions(_make_deps(task=task))
|
||||
|
||||
env = await actions.progress(agent_id=agent_id, task_id=t.id, message="x")
|
||||
assert env.as_dict()["error"] is not None
|
||||
task.record_plan_progress.assert_not_awaited()
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
"""#173: progress % is derived from the plan checklist, not agent-set.
|
||||
|
||||
The plan's sub_tasks ARE the progress skeleton. progress(plan_step=...)
|
||||
marks that step completed and the percentage is computed from
|
||||
completed/total (equal weight) — the agent cannot game it. Narrative
|
||||
entries (no plan_step) carry the current derived %. Tasks with no
|
||||
sub_task checklist fall back to the supplied percentage (back-compat).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.task import TaskService
|
||||
|
||||
_PCT_HALF = 50
|
||||
_PCT_FULL = 100
|
||||
_PCT_NONE = 0
|
||||
_PCT_FALLBACK = 42
|
||||
|
||||
|
||||
def _svc_with_task(task: Any) -> TaskService:
|
||||
svc = TaskService.__new__(TaskService)
|
||||
svc.get = AsyncMock(return_value=task) # type: ignore[method-assign]
|
||||
svc.session = MagicMock()
|
||||
svc.session.flush = AsyncMock()
|
||||
return svc
|
||||
|
||||
|
||||
def _task_with_plan(sub_tasks: list[dict[str, Any]]) -> MagicMock:
|
||||
return MagicMock(
|
||||
id=uuid4(),
|
||||
plan={"text": "p", "sub_tasks": sub_tasks},
|
||||
progress_updates=[],
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_marking_step_by_id_derives_percentage() -> None:
|
||||
sid = str(uuid4())
|
||||
task = _task_with_plan(
|
||||
[
|
||||
{"id": sid, "title": "A", "completed": False},
|
||||
{"id": str(uuid4()), "title": "B", "completed": False},
|
||||
]
|
||||
)
|
||||
svc = _svc_with_task(task)
|
||||
agent = uuid4()
|
||||
|
||||
res = await svc.record_plan_progress(task.id, agent, "did A", plan_step=sid)
|
||||
assert res is not None
|
||||
assert res["step_resolved"] is True
|
||||
assert res["percentage"] == _PCT_HALF # 1 of 2
|
||||
assert task.plan["sub_tasks"][0]["completed"] is True
|
||||
assert task.progress_updates[-1]["percentage"] == _PCT_HALF
|
||||
assert task.progress_updates[-1]["message"] == "did A"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_marking_step_by_one_based_order() -> None:
|
||||
task = _task_with_plan(
|
||||
[
|
||||
{"id": "x", "title": "A", "completed": True},
|
||||
{"id": "y", "title": "B", "completed": False},
|
||||
]
|
||||
)
|
||||
svc = _svc_with_task(task)
|
||||
res = await svc.record_plan_progress(task.id, uuid4(), "did B", plan_step="2")
|
||||
assert res["step_resolved"] is True
|
||||
assert res["percentage"] == _PCT_FULL # both now complete
|
||||
assert task.plan["sub_tasks"][1]["completed"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unknown_step_is_not_resolved_and_lists_valid() -> None:
|
||||
task = _task_with_plan([{"id": "s1", "title": "A", "completed": False}])
|
||||
svc = _svc_with_task(task)
|
||||
res = await svc.record_plan_progress(task.id, uuid4(), "?", plan_step="nope")
|
||||
assert res["step_resolved"] is False
|
||||
assert res["valid_steps"] == ["s1"]
|
||||
# Nothing marked; % still derived from (unchanged) checklist = 0.
|
||||
assert task.plan["sub_tasks"][0]["completed"] is False
|
||||
assert res["percentage"] == _PCT_NONE
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_narrative_entry_carries_current_derived_pct() -> None:
|
||||
task = _task_with_plan(
|
||||
[
|
||||
{"id": "a", "title": "A", "completed": True},
|
||||
{"id": "b", "title": "B", "completed": False},
|
||||
]
|
||||
)
|
||||
svc = _svc_with_task(task)
|
||||
res = await svc.record_plan_progress(task.id, uuid4(), "midway note")
|
||||
assert res["step_resolved"] is None # no plan_step requested
|
||||
assert res["percentage"] == _PCT_HALF # current checklist state
|
||||
assert task.progress_updates[-1]["message"] == "midway note"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_checklist_falls_back_to_supplied_percentage() -> None:
|
||||
task = MagicMock(id=uuid4(), plan="just a string plan", progress_updates=[])
|
||||
svc = _svc_with_task(task)
|
||||
res = await svc.record_plan_progress(
|
||||
task.id, uuid4(), "legacy", fallback_percentage=_PCT_FALLBACK
|
||||
)
|
||||
assert res["percentage"] == _PCT_FALLBACK
|
||||
assert res["valid_steps"] == []
|
||||
assert task.progress_updates[-1]["percentage"] == _PCT_FALLBACK
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_task_returns_none() -> None:
|
||||
svc = TaskService.__new__(TaskService)
|
||||
svc.get = AsyncMock(return_value=None) # type: ignore[method-assign]
|
||||
assert await svc.record_plan_progress(uuid4(), uuid4(), "x") is None
|
||||
Reference in New Issue
Block a user