From 7c601d0988fc265ca51ed15bd0a58c5634fc071a Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Sat, 19 Sep 2026 19:31:49 +0200 Subject: [PATCH] refactor(autonomous): split todo_manager into cohesive submodules (PLF-044) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract todo_ownership (marker/legacy splitting), todo_planning (todo derivation) and todo_render (block rendering) from todo_manager, keeping TodoManager as the execution façade. Decompose the legacy classifier into focused predicates (CC 18 → within limits). Add focused regression tests for ownership and rendering. Closes #39 Co-authored-by: Koru Agent --- src/prefact/autonomous/todo_manager.py | 364 +++-------------------- src/prefact/autonomous/todo_ownership.py | 127 ++++++++ src/prefact/autonomous/todo_planning.py | 182 ++++++++++++ src/prefact/autonomous/todo_render.py | 62 ++++ tests/test_todo_ownership.py | 148 +++++++++ 5 files changed, 567 insertions(+), 316 deletions(-) create mode 100644 src/prefact/autonomous/todo_ownership.py create mode 100644 src/prefact/autonomous/todo_planning.py create mode 100644 src/prefact/autonomous/todo_render.py create mode 100644 tests/test_todo_ownership.py diff --git a/src/prefact/autonomous/todo_manager.py b/src/prefact/autonomous/todo_manager.py index eaba21a..7e4c9eb 100644 --- a/src/prefact/autonomous/todo_manager.py +++ b/src/prefact/autonomous/todo_manager.py @@ -1,41 +1,36 @@ """TODO management for autonomous prefact. -Ownership contract: prefact owns ONLY the block between ``PREFACT_BEGIN`` and -``PREFACT_END`` markers in TODO.md. Everything outside those markers is -operator-maintained content (action plans, notes, manual checklists) and is -preserved verbatim on every rewrite. Checkbox lines outside the block are -never parsed as prefact tickets — prefact manages only the tickets it created -itself. - -Legacy TODO.md files (written before the markers existed) are migrated on the -first rewrite: the generated header and the known prefact sections are -recognised as owned, everything else is kept as manual content, and the new -file wraps the owned part in markers. +The manager is the façade wiring together three cohesive submodules: + +- :mod:`prefact.autonomous.todo_ownership` — splitting TODO.md into manual + and prefact-owned regions (including legacy migration); +- :mod:`prefact.autonomous.todo_planning` — deriving todo state from the + owned block; +- :mod:`prefact.autonomous.todo_render` — rendering the owned block. + +Execution (running fixes for pending tasks) stays here because it owns the +scanner/fixer wiring. """ -from datetime import datetime from pathlib import Path from typing import Any, Dict, List, Tuple -from prefact import __version__ from prefact.config import Config from prefact.config_extended import ExtendedConfig from prefact.fixer import Fixer from prefact.scanner import Scanner from ._base import BaseManager, console - -CONSTANT_6 = 6 - -PREFACT_BEGIN = "" -PREFACT_END = "" - -# Section headings prefact has historically generated (legacy files only). -_OWNED_HEADINGS = ( - "## ✅ Completed Tasks", - "## 📋 Current Issues", - "## 📋 Task Status", +from .todo_ownership import PREFACT_BEGIN, PREFACT_END, split_existing +from .todo_planning import ( + find_completed_tasks, + generate_current_todos, + parse_existing_todos, + parse_todo_tasks, ) +from .todo_render import build_execution_block, build_todo_block + +__all__ = ["PREFACT_BEGIN", "PREFACT_END", "TodoManager"] class TodoManager(BaseManager): @@ -46,87 +41,14 @@ def __init__(self, project_root: Path): self.issues_found: List[Dict[str, Any]] = [] # ------------------------------------------------------------------ - # Ownership: split TODO.md into (manual before, prefact-owned, manual after) + # Ownership: read/write the prefact-owned block, preserving the rest # ------------------------------------------------------------------ def _split_existing(self) -> Tuple[str, str, str]: - """Return ``(before, owned, after)`` of the current TODO.md. - - ``owned`` is the only region prefact may parse or rewrite; ``before`` - and ``after`` are operator content preserved verbatim. - """ + """Return ``(before, owned, after)`` of the current TODO.md.""" if not self.todo_path.exists(): return "", "", "" - text = self.todo_path.read_text() - if PREFACT_BEGIN in text and PREFACT_END in text: - before, rest = text.split(PREFACT_BEGIN, 1) - owned, after = rest.split(PREFACT_END, 1) - return before, owned, after - # Marker variants from other versions: match on the stable prefix. - if "", 1) - owned, after = (marker_rest[1] if len(marker_rest) > 1 else rest).split( - PREFACT_END, 1 - ) - return before, owned, after - return self._split_legacy(text) - - def _split_legacy(self, text: str) -> Tuple[str, str, str]: - """Split a pre-marker TODO.md into manual and prefact-owned parts.""" - lines = text.split("\n") - flags = self._classify_legacy_lines(lines) - if not any(flags): - # Nothing recognisably prefact-generated: whole file is manual. - return text, "", "" - first_owned = flags.index(True) - before = "\n".join(lines[:first_owned]) - owned = "\n".join( - line for line, flag in zip(lines[first_owned:], flags[first_owned:]) if flag - ) - after = "\n".join( - line - for line, flag in zip(lines[first_owned:], flags[first_owned:]) - if not flag - ) - return before, owned, after - - @staticmethod - def _classify_legacy_lines(lines: List[str]) -> List[bool]: - """Flag lines that belong to prefact-generated content (legacy files).""" - flags = [False] * len(lines) - mode: str | None = None # "header" | "section" | None - for idx, line in enumerate(lines): - stripped = line.strip() - if mode is not None: - if stripped.startswith("## ") or stripped.startswith("" +PREFACT_END = "" + +# Section headings prefact has historically generated (legacy files only). +_OWNED_HEADINGS = ( + "## ✅ Completed Tasks", + "## 📋 Current Issues", + "## 📋 Task Status", +) + +# Legacy generated-header signature, matched within a short lookahead. +_HEADER_SIGNATURE = "**Generated by:** prefact" +_HEADER_LOOKAHEAD = 6 + +_FOOTER_PREFIX = "*To execute all tasks" + + +def split_existing(text: str) -> Tuple[str, str, str]: + """Return ``(before, owned, after)`` regions of a TODO.md document. + + ``owned`` is the only region prefact may parse or rewrite; ``before`` + and ``after`` are operator content preserved verbatim. Marker-less + legacy documents are migrated via :func:`split_legacy`. + """ + if PREFACT_BEGIN in text and PREFACT_END in text: + before, rest = text.split(PREFACT_BEGIN, 1) + owned, after = rest.split(PREFACT_END, 1) + return before, owned, after + # Marker variants from other versions: match on the stable prefix. + if "", 1) + owned, after = (marker_rest[1] if len(marker_rest) > 1 else rest).split( + PREFACT_END, 1 + ) + return before, owned, after + return split_legacy(text) + + +def split_legacy(text: str) -> Tuple[str, str, str]: + """Split a pre-marker TODO.md into manual and prefact-owned parts.""" + lines = text.split("\n") + flags = classify_legacy_lines(lines) + if not any(flags): + # Nothing recognisably prefact-generated: whole file is manual. + return text, "", "" + first_owned = flags.index(True) + before = "\n".join(lines[:first_owned]) + owned = "\n".join( + line for line, flag in zip(lines[first_owned:], flags[first_owned:]) if flag + ) + after = "\n".join( + line + for line, flag in zip(lines[first_owned:], flags[first_owned:]) + if not flag + ) + return before, owned, after + + +def classify_legacy_lines(lines: List[str]) -> List[bool]: + """Flag lines that belong to prefact-generated content (legacy files).""" + flags = [False] * len(lines) + mode: str | None = None # "header" | "section" | None + for idx, line in enumerate(lines): + stripped = line.strip() + if mode is not None: + if _is_boundary_line(stripped): + mode = None # boundary — reclassify this line below + else: + flags[idx] = True + if mode == "header" and stripped == "---": + mode = None + continue + if stripped == "# TODO": + if _has_prefact_signature(lines, idx): + flags[idx] = True + mode = "header" + continue + if _is_owned_heading(stripped): + flags[idx] = True + mode = "section" + continue + if stripped.startswith(_FOOTER_PREFIX): + _claim_footer(lines, flags, idx) + return flags + + +def _is_boundary_line(stripped: str) -> bool: + """A heading or comment line closes the current owned region.""" + return stripped.startswith("## ") or stripped.startswith(" +- [ ] manual item + + +## ✅ Completed Tasks + +- [x] src/pkg/old.py:5 - Unused import: 'os' + +## 📋 Current Issues (showing 2 of 2) + +- [ ] src/pkg/a.py:10 - Unused import: 'sys' +- [ ] src/pkg/b.py:20 - String concatenation can be converted to f-string + +--- + +*To execute all tasks, run: `prefact -a --execute-todos`*""" + + +class TestClassifyLegacyLines: + def test_generated_header_is_owned(self): + lines = ["# TODO", "", "**Generated by:** prefact v1.0", "**Generated on:** x", "---"] + assert classify_legacy_lines(lines) == [True, True, True, True, True] + + def test_plain_todo_heading_is_manual(self): + lines = ["# TODO", "", "my own note"] + assert classify_legacy_lines(lines) == [False, False, False] + + def test_owned_sections_and_their_bodies(self): + lines = [ + "## ✅ Completed Tasks", + "", + "- [x] a.py:1 - fixed", + "## 📋 Current Issues (showing 2 of 2)", + "- [ ] b.py:2 - issue", + ] + assert classify_legacy_lines(lines) == [True, True, True, True, True] + + def test_boundary_comment_closes_owned_section(self): + lines = [ + "## 📋 Current Issues", + "- [ ] b.py:2 - issue", + "", + "- [ ] manual item", + ] + assert classify_legacy_lines(lines) == [True, True, False, False] + + def test_footer_claims_separator_above_it(self): + lines = [ + "trailing manual note", + "", + "---", + "", + "*To execute all tasks, run: `prefact -a --execute-todos`*", + ] + assert classify_legacy_lines(lines) == [ + False, # manual note + False, # blank above the separator stays manual + True, # --- + True, # blank + True, # footer + ] + + def test_section_claims_lines_until_boundary(self): + lines = [ + "## 📋 Current Issues", + "- [ ] b.py:2 - issue", + "still owned (no boundary yet)", + "", + "manual item", + ] + assert classify_legacy_lines(lines) == [True, True, True, False, False] + + def test_no_prefact_content_means_all_manual(self): + lines = ["# My notes", "- [ ] my task"] + assert not any(classify_legacy_lines(lines)) + + +class TestSplitLegacy: + def test_manual_plan_moves_after_owned_block(self): + before, owned, after = split_legacy(LEGACY_TODO) + # Header at line 0 → nothing before the owned region. + assert before == "" + assert "**Generated by:** prefact" in owned + assert "## 📋 Current Issues (showing 2 of 2)" in owned + assert "- [ ] src/pkg/a.py:10 - Unused import: 'sys'" in owned + # Manual content outside the historically generated regions survives. + assert "" in after + assert "- [ ] manual item" in after + assert "MANUAL" not in owned + + def test_fully_manual_file_returns_empty_owned(self): + before, owned, after = split_legacy("# Notes\n\n- [ ] mine\n") + assert owned == "" and after == "" + assert "# Notes" in before + + +class TestSplitExisting: + def test_marker_block_round_trip(self): + text = f"# Top\n\n{PREFACT_BEGIN}\nowned body\n{PREFACT_END}\n\n# Bottom\n" + before, owned, after = split_existing(text) + assert before == "# Top\n\n" + assert owned == "\nowned body\n" + assert after == "\n\n# Bottom\n" + + def test_marker_variant_prefix_is_matched(self): + text = "# Top\n\nowned\n\ntail" + before, owned, after = split_existing(text) + assert before == "# Top\n" + assert owned.strip() == "owned" + assert "tail" in after + + +class TestRenderBlocks: + def test_build_todo_block_counts_and_footer(self): + block = build_todo_block( + ["- [ ] a.py:1 - msg"], ["- [x] b.py:2 - done"], 2, 3 + ) + assert "**Total issues:** 2 active, 3 completed" in block + assert "## ✅ Completed Tasks (showing 1 of 3)" in block + assert "## 📋 Current Issues (showing 1 of 2)" in block + assert block.endswith("*To execute all tasks, run: `prefact -a --execute-todos`*") + + def test_build_execution_block_reports_processed_counts(self): + block = build_execution_block( + ["- [x] a.py:1 - msg ✅"], executed_count=1, processed_tasks=2, total_tasks=4 + ) + assert "**Total issues:** 2 processed of 4 active, 1 fixed" in block + assert "**Last executed:**" in block + assert "## 📋 Task Status" in block