diff --git a/CLAUDE.md b/CLAUDE.md index faff39d..d156d65 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,7 +81,7 @@ Module-level `@tool` handlers. Call `set_workspace_root()` before use. - Destructive confirms: `classify_danger` + `ToolRegistry(on_confirm=…)`; CLI default deny; config `confirm_destructive` / `path_denylist`. Session YOLO: `/yolo on|off|once` and `--yes` (skip soft confirms; hard denylists unchanged; `once` expires after the next user turn). - **`vcs`**: read-only VCS tools (`vcs_kind` / `vcs_status` / `vcs_diff` / `vcs_log` / `vcs_branch`) via `VcsBackend` protocol; **git** implemented; detectors are pluggable for other systems. - **`chat`**: human prompts as tools — `ask_user_line` / `ask_user_choice` / `ask_user_form` (shared `prompting` core). -- **`todo`**: session sub-task stack — `todo_list` / `todo_push` / `todo_pop` / `todo_update` / `todo_clear`; stored on session row; agent injects a review user message if open todos and no todo tool use in the turn. +- **`todo`**: nested session sub-task stack (frames) — `todo_list` / `todo_push` (multi-title = new frame) / `todo_pop` (leave frame) / `todo_update` / `todo_clear`; pattern push[T1,T2]→push[T1.1…]→pop→push[T2.1…]; stored on session row; agent injects a developer review message if open todos and no todo tool use in the turn. - **`DEFAULT_TOOLS`**: file + process + vcs + chat + todo tool list for a `ToolRegistry`. ### Prompting (`prompting.py`) diff --git a/src/plyngent/agent/todo_stack.py b/src/plyngent/agent/todo_stack.py index 4a0bc69..5aadf46 100644 --- a/src/plyngent/agent/todo_stack.py +++ b/src/plyngent/agent/todo_stack.py @@ -11,7 +11,7 @@ _OPEN: frozenset[str] = frozenset({"pending", "in_progress"}) class TodoItem(Struct, omit_defaults=True): - """One sub-task on the session todo stack.""" + """One sub-task within a stack frame.""" id: str title: str @@ -19,17 +19,29 @@ class TodoItem(Struct, omit_defaults=True): notes: str = "" -class TodoStackData(Struct, omit_defaults=True): - """Serializable stack body (session storage).""" +class TodoFrame(Struct, omit_defaults=True): + """One nesting level: sibling tasks pushed together.""" items: list[TodoItem] = field(default_factory=list) + + +class TodoStackData(Struct, omit_defaults=True): + """Serializable stack body (session storage). + + ``frames`` is bottom→top: root plan first, deepest breakdown last. + Pattern: push [T1,T2] → push [T1.1,T1.2] → pop → push [T2.1] → … + """ + + frames: list[TodoFrame] = field(default_factory=list) next_id: int = 1 class TodoStack: - """Session-local ordered sub-tasks for breaking work into steps. + """Nested todo stack for hierarchical sub-task breakdown. - Maintained mainly by model tools; humans can show/push/pop/clear via slash. + Each ``push`` of one or more titles creates a **new frame** (deeper level). + ``pop`` removes the **top frame** (leave a breakdown level). + Model tools maintain the stack; humans use ``/todos``. """ def __init__(self, data: TodoStackData | None = None) -> None: @@ -37,8 +49,12 @@ class TodoStack: self._touched_this_turn: bool = False @property - def items(self) -> list[TodoItem]: - return self._data.items + def frames(self) -> list[TodoFrame]: + return self._data.frames + + @property + def depth(self) -> int: + return len(self._data.frames) @property def touched_this_turn(self) -> bool: @@ -51,11 +67,14 @@ class TodoStack: def mark_touched(self) -> None: self._touched_this_turn = True + def all_items(self) -> list[TodoItem]: + return [item for frame in self._data.frames for item in frame.items] + def open_items(self) -> list[TodoItem]: - return [item for item in self._data.items if item.status in _OPEN] + return [item for item in self.all_items() if item.status in _OPEN] def is_empty(self) -> bool: - return not self._data.items + return not self._data.frames def needs_review(self) -> bool: """True when open work exists but no todo tool ran this turn.""" @@ -68,6 +87,20 @@ class TodoStack: def from_raw(cls, raw: object | None) -> TodoStack: if raw is None: return cls() + # Legacy flat shape: {items: [...], next_id: N} → single root frame. + if isinstance(raw, dict) and "frames" not in raw and "items" in raw: + try: + flat = cast("dict[str, object]", raw) + items = msgspec.convert(flat.get("items"), type=list[TodoItem]) + next_raw = flat.get("next_id", 1) + next_id = max(1, int(next_raw) if isinstance(next_raw, int | str) else 1) + except (msgspec.ValidationError, TypeError, ValueError): + return cls() + data = TodoStackData( + frames=[TodoFrame(items=items)] if items else [], + next_id=next_id, + ) + return cls(data) try: data = msgspec.convert(raw, type=TodoStackData) except (msgspec.ValidationError, TypeError, ValueError): @@ -79,23 +112,29 @@ class TodoStack: def to_raw(self) -> dict[str, object]: out: object = msgspec.to_builtins(self._data) if not isinstance(out, dict): - return {"items": [], "next_id": 1} + return {"frames": [], "next_id": 1} raw = cast("dict[object, object]", out) return {str(key): value for key, value in raw.items()} def render(self) -> str: - if not self._data.items: + if not self._data.frames: return "(todo stack empty)" - lines: list[str] = [] - for item in self._data.items: - mark = { - "pending": "[ ]", - "in_progress": "[~]", - "done": "[x]", - "cancelled": "[-]", - }.get(item.status, "[?]") - note = f" — {item.notes}" if item.notes else "" - lines.append(f"{mark} {item.id}: {item.title}{note}") + lines: list[str] = [f"(depth={self.depth})"] + for depth, frame in enumerate(self._data.frames): + indent = " " * depth + lines.append(f"{indent}frame {depth}:") + if not frame.items: + lines.append(f"{indent} (empty frame)") + continue + for item in frame.items: + mark = { + "pending": "[ ]", + "in_progress": "[~]", + "done": "[x]", + "cancelled": "[-]", + }.get(item.status, "[?]") + note = f" — {item.notes}" if item.notes else "" + lines.append(f"{indent} {mark} {item.id}: {item.title}{note}") return "\n".join(lines) def review_prompt(self) -> str: @@ -103,46 +142,56 @@ class TodoStack: return ( "Todo stack review (internal control — not a human message).\n" "Open sub-tasks remain and you did not call any todo_* tools this turn.\n" - "Review the stack: mark finished items done, update in_progress, " - "push new sub-tasks, or pop/cancel obsolete ones. Then continue the user work.\n\n" + "Use nested push/pop for breakdown: push [T1,T2], push [T1.1,T1.2], " + "finish children, pop frame, then push children of T2, etc. " + "Mark done/cancelled, push new frames, or pop a finished level.\n\n" f"Current stack:\n{self.render()}" ) - def push(self, title: str, *, notes: str = "", status: TodoStatus = "pending") -> TodoItem: - token = title.strip() - if not token: - msg = "title must not be empty" + def push_titles( + self, + titles: list[str], + *, + notes: str = "", + status: TodoStatus = "pending", + ) -> list[TodoItem]: + """Push a new frame containing *titles* (one or more siblings at this depth).""" + cleaned = [t.strip() for t in titles if t and t.strip()] + if not cleaned: + msg = "at least one non-empty title is required" raise ValueError(msg) - item_id = f"t{self._data.next_id}" - self._data.next_id += 1 - item = TodoItem(id=item_id, title=token, status=status, notes=notes.strip()) - self._data.items.append(item) + note = notes.strip() + created: list[TodoItem] = [] + for title in cleaned: + item_id = f"t{self._data.next_id}" + self._data.next_id += 1 + created.append(TodoItem(id=item_id, title=title, status=status, notes=note)) + self._data.frames.append(TodoFrame(items=created)) self.mark_touched() - return item + return created - def pop(self) -> TodoItem | None: - """Remove and return the last open item, else the last item, else None.""" - if not self._data.items: + def push(self, title: str, *, notes: str = "", status: TodoStatus = "pending") -> TodoItem: + """Push a new frame with a single task (convenience for one title).""" + items = self.push_titles([title], notes=notes, status=status) + return items[0] + + def pop(self) -> TodoFrame | None: + """Pop and return the top frame (leave the current breakdown level).""" + if not self._data.frames: return None - open_ids = {i.id for i in self.open_items()} - for index in range(len(self._data.items) - 1, -1, -1): - if self._data.items[index].id in open_ids: - item = self._data.items.pop(index) - self.mark_touched() - return item - item = self._data.items.pop() + frame = self._data.frames.pop() self.mark_touched() - return item + return frame def clear(self) -> int: - n = len(self._data.items) - self._data.items.clear() + n = sum(len(frame.items) for frame in self._data.frames) + self._data.frames.clear() if n: self.mark_touched() return n def get(self, item_id: str) -> TodoItem | None: - for item in self._data.items: + for item in self.all_items(): if item.id == item_id: return item return None @@ -155,18 +204,42 @@ class TodoStack: status: TodoStatus | None = None, notes: str | None = None, ) -> TodoItem: - for index, item in enumerate(self._data.items): - if item.id != item_id: - continue - new_title = title.strip() if title is not None else item.title - if not new_title: - msg = "title must not be empty" - raise ValueError(msg) - new_status = status if status is not None else item.status - new_notes = notes if notes is not None else item.notes - updated = TodoItem(id=item.id, title=new_title, status=new_status, notes=new_notes) - self._data.items[index] = updated - self.mark_touched() - return updated + for frame in self._data.frames: + for index, item in enumerate(frame.items): + if item.id != item_id: + continue + new_title = title.strip() if title is not None else item.title + if not new_title: + msg = "title must not be empty" + raise ValueError(msg) + new_status = status if status is not None else item.status + new_notes = notes if notes is not None else item.notes + updated = TodoItem(id=item.id, title=new_title, status=new_status, notes=new_notes) + frame.items[index] = updated + self.mark_touched() + return updated msg = f"unknown todo id {item_id!r}" raise KeyError(msg) + + +def parse_push_titles(raw: str) -> list[str]: + """Parse multi-title push input: JSON array, or newline/semicolon-separated.""" + text = raw.strip() + if not text: + return [] + if text.startswith("["): + try: + data: object = msgspec.json.decode(text.encode()) + except (msgspec.DecodeError, UnicodeEncodeError): + data = None + if isinstance(data, list): + out = [item.strip() for item in cast("list[object]", data) if isinstance(item, str) and item.strip()] + if out: + return out + # Newlines, then ``;`` as separators. + parts: list[str] = [] + for line in text.replace(";", "\n").splitlines(): + token = line.strip() + if token: + parts.append(token) + return parts diff --git a/src/plyngent/cli/slash.py b/src/plyngent/cli/slash.py index 19f0e37..a5a9c24 100644 --- a/src/plyngent/cli/slash.py +++ b/src/plyngent/cli/slash.py @@ -954,43 +954,51 @@ def history_cmd( @click.argument("action", required=False, type=str) @click.argument("rest", required=False, nargs=-1, type=str) @click.pass_obj -def todos_cmd( # noqa: C901, PLR0911, PLR0912 +def todos_cmd( # noqa: C901, PLR0911, PLR0912, PLR0915 state: ReplState, action: str | None, rest: tuple[str, ...] ) -> None: - """Show or edit the todo/task stack (sub-tasks). + """Show or edit the nested todo/task stack. - ``/todos`` — list - ``/todos push `` — push pending item - ``/todos pop`` — remove last open item + ``/todos`` — list frames + ``/todos push <title>`` or ``/todos push T1; T2`` — new frame of siblings + ``/todos pop`` — pop top breakdown frame ``/todos done <id>`` / ``/todos cancel <id>`` — set status ``/todos clear`` — wipe stack """ + from plyngent.agent.todo_stack import parse_push_titles + stack = state.todo_stack act = (action or "list").strip().lower() if act in {"list", "show", "ls"}: click.echo(stack.render()) return if act == "push": - title = " ".join(rest).strip() - if not title: - click.echo("error: usage: /todos push <title>") + raw = " ".join(rest).strip() + if not raw: + click.echo("error: usage: /todos push <title> | /todos push T1; T2") + return + titles = parse_push_titles(raw) + if not titles: + click.echo("error: no titles to push") return try: - item = stack.push(title) + items = stack.push_titles(titles) except ValueError as exc: click.echo(f"error: {exc}") return _await(state.persist_todo_stack()) - click.echo(f"pushed {item.id}: {item.title}") + ids = ", ".join(i.id for i in items) + click.echo(f"pushed frame depth={stack.depth} items=[{ids}]") click.echo(stack.render()) return if act == "pop": - item = stack.pop() - if item is None: + frame = stack.pop() + if frame is None: click.echo("todo stack empty") return _await(state.persist_todo_stack()) - click.echo(f"popped {item.id}: {item.title}") + titles = ", ".join(f"{i.id}:{i.title}" for i in frame.items) or "(empty)" + click.echo(f"popped frame ({titles})") click.echo(stack.render()) return if act in {"done", "cancel", "pending", "in_progress"}: diff --git a/src/plyngent/tools/todo.py b/src/plyngent/tools/todo.py index 1d637ca..c1baf37 100644 --- a/src/plyngent/tools/todo.py +++ b/src/plyngent/tools/todo.py @@ -3,6 +3,7 @@ from __future__ import annotations from typing import TYPE_CHECKING, cast from plyngent.agent import tool +from plyngent.agent.todo_stack import parse_push_titles if TYPE_CHECKING: from collections.abc import Callable @@ -41,7 +42,10 @@ def _notify() -> None: @tool(name="todo_list") def todo_list() -> str: - """List the current todo/task stack (sub-tasks for multi-step work).""" + """List the nested todo stack (frames = breakdown levels). + + Pattern: push [T1,T2] → push [T1.1,T1.2] → finish children → pop → push [T2.1]… + """ stack = _require_stack() stack.mark_touched() _notify() @@ -49,26 +53,40 @@ def todo_list() -> str: @tool(name="todo_push") -def todo_push(title: str, notes: str = "") -> str: - """Push a new sub-task onto the todo stack (pending).""" +def todo_push(titles: str, notes: str = "") -> str: + """Push a new breakdown frame with one or more sibling tasks. + + ``titles``: single title, newline-separated list, ``;``-separated, or JSON + string array. Creates a **new nesting level** (does not append to the current + frame). Example sequence: push ``T1\\nT2`` then push ``T1.1\\nT1.2``. + """ stack = _require_stack() + parsed = parse_push_titles(titles) + if not parsed: + return "error: titles must contain at least one non-empty title" try: - item = stack.push(title, notes=notes) + items = stack.push_titles(parsed, notes=notes) except ValueError as exc: return f"error: {exc}" _notify() - return f"pushed {item.id}: {item.title}\n{stack.render()}" + ids = ", ".join(i.id for i in items) + return f"pushed frame depth={stack.depth} items=[{ids}]\n{stack.render()}" @tool(name="todo_pop") def todo_pop() -> str: - """Pop the last open sub-task (or last item if all closed).""" + """Pop the top breakdown frame (leave the current nesting level). + + Use after finishing children of a task (e.g. after T1.1/T1.2, pop back to + the T1/T2 frame). Does not delete sibling frames below. + """ stack = _require_stack() - item = stack.pop() - if item is None: + frame = stack.pop() + if frame is None: return "todo stack empty" _notify() - return f"popped {item.id}: {item.title} ({item.status})\n{stack.render()}" + titles = ", ".join(f"{i.id}:{i.title}" for i in frame.items) or "(empty)" + return f"popped frame ({len(frame.items)} item(s): {titles})\n{stack.render()}" @tool(name="todo_update") @@ -101,7 +119,7 @@ def todo_update( @tool(name="todo_clear") def todo_clear() -> str: - """Clear the entire todo stack.""" + """Clear the entire todo stack (all frames).""" stack = _require_stack() n = stack.clear() _notify() diff --git a/tests/test_agent/test_todo_stack.py b/tests/test_agent/test_todo_stack.py index 3ba1865..322a349 100644 --- a/tests/test_agent/test_todo_stack.py +++ b/tests/test_agent/test_todo_stack.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING, Literal, overload import pytest from plyngent.agent import ChatAgent, ToolRegistry -from plyngent.agent.todo_stack import TodoStack +from plyngent.agent.todo_stack import TodoStack, parse_push_titles from plyngent.config.models import DatabaseConfig from plyngent.lmproto.openai_compatible.model import ( AssistantChatMessage, @@ -23,38 +23,68 @@ if TYPE_CHECKING: from collections.abc import AsyncIterator -def test_todo_stack_push_pop_update() -> None: +def test_parse_push_titles() -> None: + assert parse_push_titles("only") == ["only"] + assert parse_push_titles("T1\nT2") == ["T1", "T2"] + assert parse_push_titles("T1; T2; T3") == ["T1", "T2", "T3"] + assert parse_push_titles('["A", "B"]') == ["A", "B"] + + +def test_nested_push_pop_pattern() -> None: + """(push)[T1,T2] (push)[T1.1,T1.2] (pop) (push)[T2.1] …""" stack = TodoStack() - a = stack.push("one") - b = stack.push("two") - assert a.id == "t1" - assert b.id == "t2" - assert "one" in stack.render() - stack.update("t1", status="done") + root = stack.push_titles(["T1", "T2"]) + assert [i.title for i in root] == ["T1", "T2"] + assert stack.depth == 1 + + children = stack.push_titles(["T1.1", "T1.2"]) + assert stack.depth == 2 + assert [i.title for i in children] == ["T1.1", "T1.2"] + stack.update(children[0].id, status="done") + stack.update(children[1].id, status="done") + popped = stack.pop() assert popped is not None - assert popped.id == "t2" - assert stack.open_items() == [] + assert [i.title for i in popped.items] == ["T1.1", "T1.2"] + assert stack.depth == 1 + assert [i.title for i in stack.frames[0].items] == ["T1", "T2"] + + stack.update(root[0].id, status="done") + _ = stack.push_titles(["T2.1"]) + assert stack.depth == 2 + assert stack.frames[-1].items[0].title == "T2.1" def test_todo_stack_needs_review() -> None: stack = TodoStack() assert not stack.needs_review() - stack.push("work") + _ = stack.push("work") stack.begin_turn() assert stack.needs_review() stack.mark_touched() assert not stack.needs_review() +def test_todo_stack_legacy_flat_raw() -> None: + stack = TodoStack.from_raw( + { + "items": [{"id": "t1", "title": "old", "status": "pending", "notes": ""}], + "next_id": 2, + } + ) + assert stack.depth == 1 + assert stack.all_items()[0].title == "old" + raw = stack.to_raw() + assert "frames" in raw + + def test_todo_stack_roundtrip_raw() -> None: stack = TodoStack() - _ = stack.push("x", notes="n") + _ = stack.push_titles(["x", "y"], notes="n") raw = stack.to_raw() restored = TodoStack.from_raw(raw) - assert len(restored.items) == 1 - assert restored.items[0].title == "x" - assert restored.items[0].notes == "n" + assert restored.depth == 1 + assert [i.title for i in restored.frames[0].items] == ["x", "y"] async def test_todo_tools_and_persist(tmp_path: object) -> None: @@ -65,16 +95,21 @@ async def test_todo_tools_and_persist(tmp_path: object) -> None: stack = TodoStack() set_todo_stack(stack, on_change=None) registry = ToolRegistry(list(TODO_TOOLS)) - assert "pushed t1" in await registry.execute("todo_push", '{"title": "ship it"}') - assert "t1" in await registry.execute("todo_list", "{}") - assert "done" in await registry.execute( - "todo_update", '{"item_id": "t1", "status": "done"}' - ) + out = await registry.execute("todo_push", '{"titles": "T1\\nT2"}') + assert "pushed frame" in out + assert stack.depth == 1 + out2 = await registry.execute("todo_push", '{"titles": "T1.1; T1.2"}') + assert stack.depth == 2 + assert "T1.1" in out2 + out3 = await registry.execute("todo_pop", "{}") + assert "popped frame" in out3 + assert stack.depth == 1 _ = await memory.update_session_todo_stack(session.sid, stack.to_raw()) loaded = await memory.get_session_todo_stack(session.sid) assert loaded is not None again = TodoStack.from_raw(loaded) - assert again.items[0].status == "done" + assert again.depth == 1 + assert [i.title for i in again.frames[0].items] == ["T1", "T2"] finally: set_todo_stack(None) await memory.close() @@ -99,9 +134,7 @@ class ScriptedClient: ) -> ChatCompletionResponse | AsyncIterator[ChatCompletionChunk]: del stream self.calls += 1 - # First call: finish without tools; second: after review inject. text = "ok" if self.calls > 1 else "done without todos" - # Detect review message in history for msg in param.messages: if isinstance(msg, DeveloperChatMessage) and "Todo stack review" in msg.content: text = "reviewed stack"