From 64ca0637d546c94f5ea2f09931bd7a6c362e24cd Mon Sep 17 00:00:00 2001 From: rica-v3 Date: Sun, 19 Jul 2026 22:17:45 +0900 Subject: [PATCH 1/2] lifecycle.py: batch planner checkpoint reviews --- tests/test_orchestration_sprint_execution.py | 118 +++++++++++++++++++ tests/test_sprint_lifecycle.py | 47 +++++++- workflows/sprints/lifecycle.py | 18 ++- 3 files changed, 180 insertions(+), 3 deletions(-) diff --git a/tests/test_orchestration_sprint_execution.py b/tests/test_orchestration_sprint_execution.py index 33fb857..9e3aba5 100644 --- a/tests/test_orchestration_sprint_execution.py +++ b/tests/test_orchestration_sprint_execution.py @@ -1,4 +1,5 @@ from teams_runtime.tests.orchestration_test_utils import * +from teams_runtime.workflows.sprints.lifecycle import pending_requirement_candidates_for_planner def _no_subject_definition(rationale="The sprint handoff is repo-local and does not need external research."): @@ -1659,6 +1660,123 @@ async def fake_execute(_sprint_state, todo): self.assertEqual(sprint_state["last_resume_checkpoint_status"], "running") self.assertEqual(str(sprint_state.get("resume_from_checkpoint_requested_at") or ""), "") + def test_manual_daily_sprint_does_not_force_planner_review_without_pending_candidates(self): + with tempfile.TemporaryDirectory() as tmpdir: + scaffold_workspace(tmpdir) + with patch("teams_runtime.core.orchestration.DiscordClient", FakeDiscordClient): + service = TeamService(tmpdir, "orchestrator") + sprint_state = service._build_manual_sprint_state( + milestone_title="batch no-op planner reviews", + trigger="manual_start", + ) + sprint_state["phase"] = "ongoing" + sprint_state["status"] = "running" + sprint_state["last_planner_review_at"] = datetime.now(timezone.utc).isoformat() + sprint_state["todos"] = [ + build_todo_item( + build_backlog_item( + title=f"todo {index}", + summary=f"todo {index}", + kind="enhancement", + source="user", + scope=f"todo {index}", + ), + owner_role="developer", + ) + for index in (1, 2) + ] + + executed_todos: list[str] = [] + + async def fake_execute(_sprint_state, todo): + executed_todos.append(str(todo.get("todo_id") or "")) + todo["status"] = "completed" + + with ( + patch.object(service, "_save_sprint_state", return_value=None), + patch.object(service, "_sync_manual_sprint_queue", return_value=None), + patch.object(service, "_is_manual_sprint_cutoff_reached", return_value=False), + patch.object( + service, + "_run_internal_request_chain", + new=AsyncMock(side_effect=AssertionError("no-op review must not invoke planner")), + ) as planner_chain_mock, + patch.object(service, "_execute_sprint_todo", side_effect=fake_execute), + patch.object(service, "_finalize_sprint", new=AsyncMock(return_value=None)) as finalize_mock, + ): + asyncio.run(service._continue_manual_daily_sprint(sprint_state, announce=False)) + + self.assertEqual(len(executed_todos), 2) + planner_chain_mock.assert_not_awaited() + self.assertEqual(list(service.paths.requests_dir.glob("*.json")), []) + finalize_mock.assert_awaited_once_with(sprint_state) + + def test_manual_daily_sprint_batches_pending_candidates_into_one_checkpoint_review(self): + with tempfile.TemporaryDirectory() as tmpdir: + scaffold_workspace(tmpdir) + with patch("teams_runtime.core.orchestration.DiscordClient", FakeDiscordClient): + service = TeamService(tmpdir, "orchestrator") + sprint_state = service._build_manual_sprint_state( + milestone_title="batch pending requirements", + trigger="manual_start", + ) + sprint_state["phase"] = "ongoing" + sprint_state["status"] = "running" + sprint_state["last_planner_review_at"] = datetime.now(timezone.utc).isoformat() + sprint_state["todos"] = [ + build_todo_item( + build_backlog_item( + title="todo with requirement feedback", + summary="todo with requirement feedback", + kind="enhancement", + source="user", + scope="todo with requirement feedback", + ), + owner_role="developer", + ) + ] + + review_calls: list[tuple[bool, bool, int]] = [] + + async def fake_review(review_state, *, force=False, requirement_checkpoint=False): + review_calls.append( + ( + force, + requirement_checkpoint, + len(pending_requirement_candidates_for_planner(review_state)), + ) + ) + if requirement_checkpoint: + review_state["pending_requirement_candidates"] = [] + + async def fake_execute(execution_state, todo): + todo["status"] = "committed" + execution_state["pending_requirement_candidates"] = [ + { + "candidate_id": "REQ-CAND-001", + "status": "pending", + "candidate_text": "Add keyboard-only acceptance.", + }, + { + "candidate_id": "REQ-CAND-002", + "status": "pending", + "candidate_text": "Preserve mobile approval flow.", + }, + ] + + with ( + patch.object(service, "_save_sprint_state", return_value=None), + patch.object(service, "_sync_manual_sprint_queue", return_value=None), + patch.object(service, "_is_manual_sprint_cutoff_reached", return_value=False), + patch.object(service, "_run_ongoing_sprint_review", side_effect=fake_review), + patch.object(service, "_execute_sprint_todo", side_effect=fake_execute), + patch.object(service, "_finalize_sprint", new=AsyncMock(return_value=None)) as finalize_mock, + ): + asyncio.run(service._continue_manual_daily_sprint(sprint_state, announce=False)) + + self.assertEqual(review_calls, [(False, False, 0), (True, True, 2)]) + finalize_mock.assert_awaited_once_with(sprint_state) + def test_manual_daily_sprint_wraps_up_when_only_terminal_todos_remain(self): with tempfile.TemporaryDirectory() as tmpdir: scaffold_workspace(tmpdir) diff --git a/tests/test_sprint_lifecycle.py b/tests/test_sprint_lifecycle.py index 9601495..1e0dbd1 100644 --- a/tests/test_sprint_lifecycle.py +++ b/tests/test_sprint_lifecycle.py @@ -43,6 +43,7 @@ next_initial_phase_step, prepare_requested_restart_checkpoint, record_sprint_planning_iteration, + requirement_checkpoint_review_due, requirement_traceability_matrix_for_sprint, recover_sprint_todos_from_recovered, render_initial_implementation_plan_markdown, @@ -206,6 +207,12 @@ def test_ongoing_planning_request_exposes_only_pending_requirement_candidates(se "status": "rejected", "candidate_text": "Rejected text.", }, + { + "candidate_id": "REQ-CAND-003", + "status": "pending", + "candidate_text": "Preserve mobile approval flow.", + "created_at": "2026-04-21T19:40:02+09:00", + }, ], } @@ -223,6 +230,7 @@ def test_ongoing_planning_request_exposes_only_pending_requirement_candidates(se self.assertIn("pending_requirement_candidates:", record["body"]) self.assertIn("REQ-CAND-001: Add keyboard-only acceptance.", record["body"]) + self.assertIn("REQ-CAND-003: Preserve mobile approval flow.", record["body"]) self.assertNotIn("Rejected text.", record["body"]) self.assertEqual( record["params"]["pending_requirement_candidates"], @@ -233,7 +241,14 @@ def test_ongoing_planning_request_exposes_only_pending_requirement_candidates(se "raw_body": "", "artifacts": ["docs/a.md"], "created_at": "2026-04-21T19:40:00+09:00", - } + }, + { + "candidate_id": "REQ-CAND-003", + "candidate_text": "Preserve mobile approval flow.", + "raw_body": "", + "artifacts": [], + "created_at": "2026-04-21T19:40:02+09:00", + }, ], ) self.assertTrue(record["params"]["requirement_reconciliation_checkpoint"]) @@ -252,6 +267,36 @@ def test_ongoing_planning_request_exposes_only_pending_requirement_candidates(se self.assertEqual(non_checkpoint_record["params"]["pending_requirement_candidates"], []) self.assertFalse(non_checkpoint_record["params"]["requirement_reconciliation_checkpoint"]) + def test_requirement_checkpoint_review_requires_success_and_valid_pending_candidates(self) -> None: + sprint_state = { + "pending_requirement_candidates": [ + { + "candidate_id": "REQ-CAND-001", + "status": "pending", + "candidate_text": "Add keyboard-only acceptance.", + }, + { + "candidate_id": "REQ-CAND-002", + "status": "rejected", + "candidate_text": "Rejected candidate.", + }, + { + "candidate_id": "REQ-CAND-003", + "status": "pending", + "candidate_text": "", + }, + ] + } + + self.assertTrue(requirement_checkpoint_review_due(sprint_state, todo_status="completed")) + self.assertTrue(requirement_checkpoint_review_due(sprint_state, todo_status=" COMMITTED ")) + for status in ("queued", "running", "blocked", "failed", "uncommitted", ""): + with self.subTest(status=status): + self.assertFalse(requirement_checkpoint_review_due(sprint_state, todo_status=status)) + + sprint_state["pending_requirement_candidates"][0]["status"] = "registered" + self.assertFalse(requirement_checkpoint_review_due(sprint_state, todo_status="completed")) + def test_sprint_research_prepass_body_lines_include_planning_hints(self) -> None: lines = sprint_research_prepass_body_lines( { diff --git a/workflows/sprints/lifecycle.py b/workflows/sprints/lifecycle.py index b8b9a44..0728b74 100644 --- a/workflows/sprints/lifecycle.py +++ b/workflows/sprints/lifecycle.py @@ -1150,6 +1150,17 @@ def pending_requirement_candidates_for_planner(sprint_state: dict[str, Any]) -> return candidates +def requirement_checkpoint_review_due( + sprint_state: dict[str, Any], + *, + todo_status: str, +) -> bool: + return ( + str(todo_status or "").strip().lower() in {"completed", "committed"} + and bool(pending_requirement_candidates_for_planner(sprint_state)) + ) + + def format_requirement_candidate_ref(candidate: dict[str, Any]) -> str: candidate_id = str(candidate.get("candidate_id") or "").strip().upper() text = _normalize_requirement_text(candidate.get("candidate_text") or candidate.get("raw_body") or "") @@ -3946,7 +3957,10 @@ async def continue_manual_daily_sprint( return await service._execute_sprint_todo(sprint_state, next_todo) service._save_sprint_state(sprint_state) - requirement_checkpoint_review = str(next_todo.get("status") or "").strip().lower() in {"completed", "committed"} + requirement_checkpoint_review = requirement_checkpoint_review_due( + sprint_state, + todo_status=str(next_todo.get("status") or ""), + ) force_review = requirement_checkpoint_review @@ -4011,7 +4025,7 @@ async def continue_sprint( todo_status = str(todo.get("status") or "").strip().lower() if todo_status == "uncommitted": return - if todo_status in {"completed", "committed"} and pending_requirement_candidates_for_planner(sprint_state): + if requirement_checkpoint_review_due(sprint_state, todo_status=todo_status): await service._run_ongoing_sprint_review( sprint_state, force=True, From eb40ce5a1ebc23c392b14b15c20c85a9d3b96ea8 Mon Sep 17 00:00:00 2001 From: rica-v3 Date: Sun, 19 Jul 2026 22:17:50 +0900 Subject: [PATCH 2/2] operations_guide.md: document planner review batching --- docs/operations_guide.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/operations_guide.md b/docs/operations_guide.md index ece5a32..042e616 100644 --- a/docs/operations_guide.md +++ b/docs/operations_guide.md @@ -102,6 +102,7 @@ python -m teams_runtime goal cancel - Sprint backlog definition items should carry concrete `acceptance_criteria` plus planner trace in `origin.milestone_ref`, `origin.requirement_refs`, `origin.spec_refs`, `origin.plan_action_refs`, and `origin.research_refs` when a source-backed or local-evidence research report is available. - During an active sprint, clear new user requirements are stored as sprint-local `REQ-CAND-*` entries in `pending_requirement_candidates`. They are not accepted scope, not acknowledged as registered requirements, and not included in role context until planner reaches the next completed/committed TODO checkpoint. - At that checkpoint, planner receives pending candidates in an `ongoing_review` request and may return `proposals.sprint_requirement_reconciliation` with `registered_requirements`, `merged_candidates`, `deferred_candidates`, and `rejected_candidates`. Only registered candidates become `REQ-*`; unresolved candidates expire into `requirement_candidate_archive` at sprint closeout. +- Manual sprints batch all currently pending candidates into that single checkpoint review. A completed or committed TODO does not force an `ongoing_review` when no valid pending candidates exist; interval-based planner reviews continue to use `sprint.interval_minutes`. - Legacy planner aliases such as `planned_backlog_updates` are compatibility inputs only inside role-runtime normalization. They are not accepted by the canonical backlog helper interface. ## Sprint Requirement Feedback