core/agent+tools+cli: number todos by listing order, not the internal id

This commit is contained in:
2026-10-16 18:52:36 +08:00
parent ac33e92d33
commit 2eb99a079d
9 changed files with 192 additions and 79 deletions
+43 -30
View File
@@ -14,7 +14,7 @@ _REVIEW_OPEN_TITLE_LIMIT = 8
class TodoItem(Struct, omit_defaults=True):
"""One task inside a group (siblings share a group; not stacked individually)."""
id: str
id: str # stored key only — a listing numbers items by render order, never by id
title: str
status: TodoStatus = "pending"
notes: str = ""
@@ -39,7 +39,8 @@ class TodoStack:
- **Push** always creates **one new group** containing one or more sibling
tasks (``push T1, T2`` is one frame, not two stack levels).
- **Pop** removes the entire **top group**.
- Within a group, update tasks by id (done / in_progress / …).
- **Update** takes the 1-based number a listing shows (TOP group first); the
stored ``id`` never leaves the stack.
Breakdown pattern::
@@ -161,13 +162,26 @@ class TodoStack:
raw = cast("dict[object, object]", out)
return {str(key): value for key, value in raw.items()}
def _render_order(self) -> list[tuple[TodoGroup, int, TodoItem]]:
"""``(group, index, item)`` in listing order — TOP group first.
A listing numbers these 1-based, so an item's position here *is* the
number it is shown (and updated) by.
"""
ordered: list[tuple[TodoGroup, int, TodoItem]] = []
for group in reversed(self._data.groups):
ordered.extend((group, index, item) for index, item in enumerate(group.items))
return ordered
def render(self) -> str:
if not self._data.groups:
return "(todo stack empty — no groups)"
lines: list[str] = [
f"(LIFO of groups: depth={self.depth}; TOP group = current breakdown level)",
]
# Top group first for the model.
# Top group first for the model; the running number is the item's
# position in :meth:`_render_order`, the same list ``update`` resolves.
number = 0
for offset, group in enumerate(reversed(self._data.groups)):
depth = self.depth - 1 - offset
tag = " TOP" if offset == 0 else ""
@@ -176,6 +190,7 @@ class TodoStack:
lines.append(" (empty group)")
continue
for item in group.items:
number += 1
mark = {
"pending": "[ ]",
"in_progress": "[~]",
@@ -183,7 +198,7 @@ class TodoStack:
"cancelled": "[-]",
}.get(item.status, "[?]")
note = f" — {item.notes}" if item.notes else ""
lines.append(f" {mark} {item.id}: {item.title}{note}")
lines.append(f" {mark} {number}. {item.title}{note}")
return "\n".join(lines)
def turn_reminder_prompt(self) -> str:
@@ -220,7 +235,7 @@ class TodoStack:
headline,
action,
"Tools: todo_list | todo_push(titles=[...]) | todo_update | todo_pop | todo_clear",
"Rules: TOP = current level; push = one sibling group; pop = whole TOP group.",
"Rules: TOP = current level; push = one sibling group; pop = whole TOP group; update by listing number.",
"Stack:",
self.render(),
]
@@ -232,7 +247,7 @@ class TodoStack:
n_open = len(open_items)
n_groups = self.depth
if n_open:
open_titles = "; ".join(f"{item.id}:{item.title}" for item in open_items[:_REVIEW_OPEN_TITLE_LIMIT])
open_titles = "; ".join(item.title for item in open_items[:_REVIEW_OPEN_TITLE_LIMIT])
if n_open > _REVIEW_OPEN_TITLE_LIMIT:
open_titles += f"; …(+{n_open - _REVIEW_OPEN_TITLE_LIMIT} more)"
headline = (
@@ -258,7 +273,7 @@ class TodoStack:
headline,
action,
"Tools: todo_list | todo_push(titles=[...]) | todo_update | todo_pop | todo_clear",
"Rules: TOP group = current level; push=one sibling group; pop=remove whole TOP group.",
"Rules: TOP = current level; push = one sibling group; pop = whole TOP group; update by listing number.",
"Stack:",
self.render(),
]
@@ -326,36 +341,34 @@ class TodoStack:
self.mark_touched()
return n
def get(self, item_id: str) -> TodoItem | None:
for item in self.all_items():
if item.id == item_id:
return item
return None
def update(
self,
item_id: str,
number: int,
*,
title: str | None = None,
status: TodoStatus | None = None,
notes: str | None = None,
) -> TodoItem:
for group in self._data.groups:
for index, item in enumerate(group.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)
group.items[index] = updated
self.mark_touched()
return updated
msg = f"unknown todo id {item_id!r}"
raise KeyError(msg)
"""Update the item a listing shows as *number* (1-based, TOP group first).
Numbers are listing positions, not stored ids: a push/pop shifts them, so
they have to come from the latest listing.
"""
ordered = self._render_order()
if not 1 <= number <= len(ordered):
msg = f"no todo item {number}: the list has {len(ordered)} item(s); pick a number from the latest listing"
raise ValueError(msg)
group, index, item = ordered[number - 1]
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)
group.items[index] = updated
self.mark_touched()
return updated
def parse_push_titles(raw: str) -> list[str]:
+18 -13
View File
@@ -420,12 +420,12 @@ def complete_slash_args(
if flags:
return sorted(set(flags))
# Contextual /todos second token: todo ids after done|cancel|pending|in_progress.
# Contextual /todos second token: listing numbers after done|cancel|…
if name == "todos" and prior:
action = prior[0].lower()
if action in {"done", "cancel", "pending", "in_progress"} and len(prior) == 1:
ids = [item.id for item in state.todo_stack.all_items()]
return [i for i in ids if i.startswith(incomplete)]
numbers = [str(n) for n in range(1, len(state.todo_stack.all_items()) + 1)]
return [n for n in numbers if n.startswith(incomplete)]
# Positional arguments: complete the next unset argument.
arg_params = [p for p in cmd.params if isinstance(p, click.Argument)]
@@ -1524,10 +1524,10 @@ def todos_cmd( # noqa: C901, PLR0911, PLR0915 — one branch per /todos action
) -> None:
"""Show or edit the LIFO stack of **task groups**.
``/todos`` — list (top group first)
``/todos`` — list (top group first; items numbered as shown)
``/todos push T1; T2`` — one new group of siblings
``/todos pop`` — pop entire top group
``/todos done <id>`` / ``/todos cancel <id>`` — set status
``/todos done <number>`` / ``/todos cancel <number>`` — set status
``/todos clear`` — wipe stack
"""
from plyngent.agent.todo_stack import parse_push_titles
@@ -1552,8 +1552,8 @@ def todos_cmd( # noqa: C901, PLR0911, PLR0915 — one branch per /todos action
click.echo(f"error: {exc}")
return
_await(state.persist_todo_stack())
ids = ", ".join(i.id for i in group.items)
click.echo(f"pushed group depth={stack.depth} items=[{ids}]")
pushed = ", ".join(i.title for i in group.items)
click.echo(f"pushed group depth={stack.depth} items=[{pushed}]")
click.echo(stack.render())
return
if act == "pop":
@@ -1562,13 +1562,13 @@ def todos_cmd( # noqa: C901, PLR0911, PLR0915 — one branch per /todos action
click.echo("todo stack empty")
return
_await(state.persist_todo_stack())
titles = ", ".join(f"{i.id}:{i.title}" for i in group.items) or "(empty)"
click.echo(f"popped TOP group ({titles})")
popped = ", ".join(i.title for i in group.items) or "(empty)"
click.echo(f"popped TOP group ({popped})")
click.echo(stack.render())
return
if act in {"done", "cancel", "pending", "in_progress"}:
if not rest:
click.echo(f"error: usage: /todos {act} <id>")
click.echo(f"error: usage: /todos {act} <number>")
return
if act == "done":
new_status = "done"
@@ -1579,12 +1579,17 @@ def todos_cmd( # noqa: C901, PLR0911, PLR0915 — one branch per /todos action
else:
new_status = "in_progress"
try:
item = stack.update(rest[0], status=new_status)
except (KeyError, ValueError) as exc:
number = int(rest[0].strip())
except ValueError:
click.echo(f"error: /todos {act} takes an item number from the list (e.g. /todos {act} 2)")
return
try:
item = stack.update(number, status=new_status)
except ValueError as exc:
click.echo(f"error: {exc}")
return
_await(state.persist_todo_stack())
click.echo(f"updated {item.id} → {item.status}")
click.echo(f"updated {number} → {item.status}")
click.echo(stack.render())
return
if act == "clear":
+3
View File
@@ -97,6 +97,9 @@ otherwise a small default is used. Never rely on shell to spoof identity.
### Todos
- Use the todo stack for multi-step work (LIFO groups: push related items, finish/update, \
pop the group). Open items mean unfinished work.
- A listing numbers items in the order it shows them (TOP group first); those numbers are \
positions, not ids, and shift when the stack changes — `todo_update` takes the number from \
the latest listing.
- The stack does **not** auto-clear. When every task is done (all items done/cancelled), \
call `todo_clear` (or pop finished TOP groups) so no hygiene noise is left behind.
"""
+27 -13
View File
@@ -3,6 +3,7 @@ from __future__ import annotations
from typing import TYPE_CHECKING, cast
from plyngent.agent import ToolTag, tool
from plyngent.tools.chat.shape import number_arg
if TYPE_CHECKING:
from collections.abc import Callable
@@ -52,6 +53,10 @@ async def _with_todo_stack(mutator: Callable[[TodoStack], str]) -> str:
async def todo_list() -> str:
"""Show the LIFO stack of **task groups** (TOP group = current breakdown level).
Items are numbered in the order they print (1-based, TOP group first). Those
numbers are listing positions, not stored ids, so they shift whenever the
stack changes — take them from the latest listing.
Non-empty stack with open (pending/in_progress) items = unfinished work.
All-terminal but non-empty = hygiene only (pop/clear). Push creates one
group of siblings; pop removes the whole top group.
@@ -82,8 +87,8 @@ async def todo_push(titles: list[str], notes: str = "") -> str:
group = stack.push_group(parsed, notes=notes)
except ValueError as exc:
return f"error: {exc}"
ids = ", ".join(i.id for i in group.items)
return f"pushed group (depth={stack.depth}) items=[{ids}]\n{stack.render()}"
pushed = ", ".join(i.title for i in group.items)
return f"pushed group (depth={stack.depth}) items=[{pushed}]\n{stack.render()}"
return await _with_todo_stack(_run)
@@ -100,27 +105,36 @@ async def todo_pop() -> str:
group = stack.pop()
if group is None:
return "todo stack empty"
titles = ", ".join(f"{i.id}:{i.title}" for i in group.items) or "(empty)"
popped_titles = ", ".join(i.title for i in group.items) or "(empty)"
top = stack.top_group
top_s = "(empty)" if top is None else ", ".join(i.id for i in top.items)
return f"popped TOP group ({titles}); new top group=[{top_s}]\n{stack.render()}"
top_titles = "(empty)" if top is None else ", ".join(i.title for i in top.items)
return f"popped TOP group ({popped_titles}); new top group=[{top_titles}]\n{stack.render()}"
return await _with_todo_stack(_run)
@tool(name="todo_update", tags=ToolTag.LOCAL | ToolTag.PUBLIC | ToolTag.SESSION_STATE)
async def todo_update(
item_id: str,
number: int,
status: str = "",
title: str = "",
notes: str = "",
) -> str:
"""Update a task by id inside any group.
"""Update a task by the number the listing shows (1-based, TOP group first).
Open items are unfinished work: mark done/cancelled when the work is truly
finished (not just deferred). Pop the TOP group when that breakdown level
is complete so the stack does not linger as false open work.
Numbers come from the latest listing and shift when the stack changes, so
re-list rather than reusing an older number. Open items are unfinished work:
mark done/cancelled when the work is truly finished (not just deferred). Pop
the TOP group when that breakdown level is complete so the stack does not
linger as false open work.
"""
item_number, number_error = number_arg("todo_update", "number", number, example="2")
if number_error is not None:
return f"error: {number_error}"
assert item_number is not None # number_arg returns a value whenever it accepts
if not item_number.is_integer():
return "error: `todo_update` expects `number` as a whole item number; pass e.g. 2"
target = int(item_number)
def _run(stack: TodoStack) -> str:
status_arg: TodoStatus | None = None
@@ -131,14 +145,14 @@ async def todo_update(
status_arg = cast("TodoStatus", token)
try:
item = stack.update(
item_id.strip(),
target,
title=title if title.strip() else None,
status=status_arg,
notes=notes if notes != "" else None,
)
except (KeyError, ValueError) as exc:
except ValueError as exc:
return f"error: {exc}"
return f"updated {item.id} → {item.status}: {item.title}\n{stack.render()}"
return f"updated {target} → {item.status}: {item.title}\n{stack.render()}"
return await _with_todo_stack(_run)
+36 -8
View File
@@ -66,6 +66,29 @@ def test_ids_reuse_after_pop() -> None:
assert g3.items[0].id == "t3"
def test_render_numbers_follow_listing_order() -> None:
"""Numbers are listing positions (TOP group first), never the stored ids."""
stack = TodoStack()
root = stack.push_group(["root", "sibling"]) # stored ids t1, t2
_ = stack.push_group(["child"]) # stored id t3
listing = stack.render()
assert " [ ] 1. child" in listing
assert " [ ] 2. root" in listing
assert " [ ] 3. sibling" in listing
assert "t1" not in listing
assert "t3" not in listing
# update(1) is the first item the listing shows — the one stored as t3.
updated = stack.update(1, status="done")
assert updated.id == "t3"
assert updated.title == "child"
assert updated.status == "done"
assert root.items[0].status == "pending"
with pytest.raises(ValueError, match="no todo item 4"):
stack.update(4, status="done")
def test_push_is_group_not_per_task_stack() -> None:
"""Multi-title push is one group; pop removes the whole group."""
stack = TodoStack()
@@ -94,11 +117,16 @@ def test_dfs_breakdown_with_groups() -> None:
root = stack.push_group(["T1", "T2"])
children = stack.push_group(["T1.1", "T1.2"])
assert stack.depth == 2
stack.update(children.items[0].id, status="done")
stack.update(children.items[1].id, status="done")
# Numbers come from the listing: the pushed children are the TOP group → 1, 2.
stack.update(1, status="done")
stack.update(2, status="done")
assert [i.status for i in children.items] == ["done", "done"]
_ = stack.pop()
assert stack.depth == 1
stack.update(root.items[0].id, status="done")
# Children gone: the root group is the listing again → 1 is T1, 2 is T2.
stack.update(1, status="done")
assert root.items[0].status == "done"
assert root.items[1].status == "pending"
_ = stack.push_group(["T2.1"])
assert stack.depth == 2
assert stack.top_group is not None
@@ -119,14 +147,14 @@ def test_single_title_still_one_group() -> None:
def test_todo_stack_needs_review() -> None:
stack = TodoStack()
assert not stack.needs_review()
item = stack.push("work")
_ = stack.push("work")
stack.begin_turn()
# Non-empty + untouched → nag (open or hygiene).
assert stack.needs_review()
# Any todo_* access this turn suppresses end-of-turn nag, even with open items.
stack.mark_touched()
assert not stack.needs_review()
stack.update(item.id, status="done")
stack.update(1, status="done")
stack.begin_turn()
# Terminal-only stack: same gate (untouched only).
assert stack.needs_review()
@@ -212,7 +240,7 @@ def test_inject_todo_nag_strategies() -> None:
def test_todo_prompts_signal_undone_work() -> None:
stack = TodoStack()
item = stack.push("open work")
_ = stack.push("open work")
reminder = stack.turn_reminder_prompt()
assert "[TODO OPEN WORK]" in reminder
assert "Stack not empty" in reminder
@@ -223,9 +251,9 @@ def test_todo_prompts_signal_undone_work() -> None:
assert "[TODO OPEN WORK]" in review
assert "undone work" in review.lower() or "open item" in review.lower()
assert "open work" in review
assert "t1:open work" in review or "open work" in review
assert "1. open work" in review # the review embeds the numbered listing
stack.update(item.id, status="done")
stack.update(1, status="done")
terminal_review = stack.review_prompt()
assert "[TODO HYGIENE]" in terminal_review
assert "done/cancelled" in terminal_review
+12 -12
View File
@@ -174,28 +174,28 @@ async def test_pretty_todo_push(capsys: pytest.CaptureFixture[str]) -> None:
)
)
result = (
"pushed group (depth=1) items=[a1]\n"
"pushed group (depth=1) items=[T1]\n"
"(LIFO of groups: depth=1; TOP group = current breakdown level)\n"
"group d=0 TOP:\n"
" [ ] a1: T1"
" [ ] 1. T1"
)
await render_events(_aiter([call, _result(result)]))
out = capsys.readouterr().out
assert "* Todo Push:" in out
assert " [ ] a1: T1" in out
assert " [ ] 1. T1" in out
assert "[tool]" not in out
async def test_pretty_todo_batch_merges_to_one_stack(capsys: pytest.CaptureFixture[str]) -> None:
"""A batch of todo calls: one summary line, then the last result's stack."""
stack = "(LIFO of groups: depth=1; TOP group = current breakdown level)\ngroup d=0 TOP:\n [ ] a1: T1"
stack = "(LIFO of groups: depth=1; TOP group = current breakdown level)\ngroup d=0 TOP:\n [ ] 1. T1"
await render_events(
_aiter(
[
_pretty_call("todo_push", '{"titles": ["T1"]}'),
_pretty_call("todo_update", '{"item_id": "a1", "status": "done"}'),
_result(f"pushed group (depth=1) items=[a1]\n{stack}"),
_result(f"updated a1 → done: T1\n{stack}"),
_pretty_call("todo_update", '{"number": 1, "status": "done"}'),
_result(f"pushed group (depth=1) items=[T1]\n{stack}"),
_result(f"updated 1 → done: T1\n{stack}"),
]
)
)
@@ -206,14 +206,14 @@ async def test_pretty_todo_batch_merges_to_one_stack(capsys: pytest.CaptureFixtu
# Only the last result is printed, so the stack renders once.
assert out.count("group d=0 TOP:") == 1
assert "pushed group" not in out
assert "updated a1 → done: T1" in out
assert "updated 1 → done: T1" in out
async def test_pretty_todo_batch_mixed_with_other_tool_stays_per_call(
capsys: pytest.CaptureFixture[str],
) -> None:
"""A batch holding a non-todo call keeps one line (and stack) per todo call."""
stack = "group d=0 TOP:\n [ ] a1: T1"
stack = "group d=0 TOP:\n [ ] 1. T1"
await render_events(
_aiter(
[
@@ -867,13 +867,13 @@ async def test_pretty_vcs_kind_branch_and_diff(capsys: pytest.CaptureFixture[str
async def test_pretty_todo_list_pop_and_clear(capsys: pytest.CaptureFixture[str]) -> None:
stack = " [ ] a1: T1\n [x] a2: T2"
stack = " [ ] 1. T1\n [x] 2. T2"
await render_events(_aiter([_pretty_call("todo_list", "{}"), _result(stack)]))
out = capsys.readouterr().out
assert "* Todo List:" in out
assert " [ ] a1: T1" in out
assert " [ ] 1. T1" in out
await render_events(_aiter([_pretty_call("todo_pop", "{}"), _result(f"popped TOP group (a1:T1)\n{stack}")]))
await render_events(_aiter([_pretty_call("todo_pop", "{}"), _result(f"popped TOP group (T1)\n{stack}")]))
out = capsys.readouterr().out
assert "* Todo Pop:" in out
+3 -3
View File
@@ -194,9 +194,9 @@ def test_complete_slash_args_from_registry(tmp_path: object) -> None:
state.todo_stack = TodoStack()
_ = state.todo_stack.push_group(["alpha", "beta"])
ids = complete_slash_args(state, "/todos", "t", prior_args=["done"])
assert "t1" in ids
assert "t2" in ids
numbers = complete_slash_args(state, "/todos", "", prior_args=["done"])
assert numbers == ["1", "2"]
assert complete_slash_args(state, "/todos", "2", prior_args=["done"]) == ["2"]
state.remember_session_ids([3, 12, 40])
assert complete_slash_args(state, "/resume", "1") == ["12"]
+23
View File
@@ -757,3 +757,26 @@ async def test_btw_read_notice_states_read_only_scope(
assert "read-only" in notice.body
assert "not saved" in notice.body
assert "[notice] aside: read tools" in capsys.readouterr().out
async def test_todos_update_takes_a_listing_number(state: ReplState, capsys: pytest.CaptureFixture[str]) -> None:
assert await handle_slash(state, "/todos push first; second") is True
out = capsys.readouterr().out
assert "pushed group depth=1 items=[first, second]" in out
assert " [ ] 1. first" in out
assert " [ ] 2. second" in out
assert "t1" not in out
assert await handle_slash(state, "/todos done 2") is True
out = capsys.readouterr().out
assert "updated 2 → done" in out
assert " [x] 2. second" in out
# The stored id is not an address: only listing numbers are.
assert await handle_slash(state, "/todos done t1") is True
assert "takes an item number from the list" in capsys.readouterr().out
assert await handle_slash(state, "/todos done 7") is True
assert "no todo item 7" in capsys.readouterr().out
assert await handle_slash(state, "/todos pop") is True
assert "popped TOP group (first, second)" in capsys.readouterr().out
+27
View File
@@ -101,6 +101,33 @@ async def test_session_on_todo_change_fires() -> None:
assert hits == ["session"]
async def test_todo_update_uses_listing_numbers() -> None:
store = MemoryViewStore({})
session = SessionState(session_id="numbers", data=session_data_view(store=store))
registry = ToolRegistry(list(TODO_TOOLS), session_state=session)
out = await registry.execute("todo_push", '{"titles": ["root"]}')
assert "pushed group (depth=1) items=[root]" in out
out = await registry.execute("todo_push", '{"titles": ["child"]}')
assert "pushed group (depth=2) items=[child]" in out
# TOP group first; the stored ids stay out of every result.
assert " [ ] 1. child" in out
assert " [ ] 2. root" in out
assert "t1" not in out and "t2" not in out
numbered = await registry.execute("todo_update", '{"number": 2, "status": "done"}')
assert "updated 2 → done: root" in numbered
assert " [x] 2. root" in numbered
stale = await registry.execute("todo_update", '{"number": 9, "status": "done"}')
assert stale.startswith("error: no todo item 9")
shaped = await registry.execute("todo_update", '{"number": "first"}')
assert shaped.startswith("error: `todo_update` expects `number`")
popped = await registry.execute("todo_pop", "{}")
assert "popped TOP group (child); new top group=[root]" in popped
async def test_todo_without_session_errors() -> None:
registry = ToolRegistry(list(TODO_TOOLS))
out = await registry.execute("todo_list", "{}")