Files
FabledScribe/src/scribe/services/placement.py
T
bvandeusenandClaude Opus 5 5c6175ad97
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / integration (push) Successful in 50s
CI & Build / TypeScript typecheck (push) Successful in 52s
CI & Build / Python tests (push) Failing after 1m4s
CI & Build / Build & push image (push) Skipped
feat(placement): a record you only cite carries its status (#4154)
Step 1 made placement cheap for a task whose status CHANGES: create_task
and update_task return where it sits, and the report is written from
that. It did nothing for a task a reply merely cites.

This milestone's own step-6 review reported "#4014 is the open step of
milestone 409". #4014 had been done for four days; the open step was
#4015. The id did not come from a read — it came from a retrieval hint,
which carries an id, a kind and a title and says nothing about status,
while list_milestones said "8 of 9" and would not say which one. The
gap was there to be filled and the nearest-looking id filled it.

Two surfaces, one principle: the status arrives with the id.

1. get_project_milestone_summaries gains next_step — the earliest open
   step, {id, title, status} or None — carried through _BRIEF_FIELDS to
   enter_project, get_project and list_milestones. One extra flat query
   for the whole batch, so #2384's fan-out does not come back.

   OPEN_STEP_STATUSES moves to services/milestones.py and placement.py
   imports it; both surfaces now answer "what is next" and must not
   drift on what counts as open. Both step queries take the same
   readable_notes_clause (rule 78), so a row cannot name a step its own
   progress numbers exclude.

2. _record_kind renders a task's status: [task (done)], [issue (todo)].
   A finished step and an open one read identically before, which is
   exactly the line the misreport was taken from. Only tasks — is_task
   IS status-is-not-None on the model, so there is no fallback branch.

reporting-back gains the practice, owned and registered in the guidance
ownership table: a record you only mention is a record to read.

The guards are structural and each fails on the regression it names:
the query count is asserted rather than the payload shape, and the two
surfaces' agreement is pinned on the rendered ORDER BY, since a mocked
session hands back whatever order the test chose.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
2026-09-18 12:08:00 -04:00

148 lines
6.3 KiB
Python

"""Where a task sits — its project, its milestone, its step position, what is next.
WHY THIS EXISTS (milestone 409 step 1)
An agent reporting finished work to the operator is asked to place it: which
milestone, which step of how many, what comes next. Without those facts in
hand it reconstructs them from memory, and a reconstruction reads exactly like
the real thing while being wrong — the drafting of the feature note that
started this milestone invented a milestone title and named a "next" step that
was already done. So the facts come back on the write that changes a task,
where the report is about to be written, instead of being left to recall.
THE SHAPE
{"project": {"id", "title"},
"milestone": {"id", "title", "status"},
"position": {"step": 3, "of": 6},
"progress": {"completed", "total", "pct"},
"next": {"id", "title", "status"} | None}
`milestone`, `position`, `progress` and `next` appear only for a task in a
milestone; a task with a project and no milestone gets `project` alone; a task
with neither gets no placement at all (None), which the doors omit rather than
send empty.
WHAT "STEP" AND "NEXT" MEAN
Notes carry no order column, so a milestone's steps are in CREATION order
(created_at, then id) — the order a plan's steps are written in, and the order
a batch create inserts them. Deliberately NOT get_milestone's listing order
(status, then last update): that is a display choice, and it reshuffles every
time anything is touched.
`next` is the first open step (todo or in_progress) AFTER this one; when every
later step is closed it falls back to the earliest open step before it, since
that is still the milestone's next piece of work. None when nothing else is open.
ACCESS
Siblings are read through access.readable_notes_clause, so a collaborator on a
shared task is never shown the title of a step they cannot open. Position and
progress are computed over that same readable set — one list, so the counts can
never disagree with the titles they sit beside. For an owner the readable set
is every step, and the numbers equal get_milestone's.
"""
from __future__ import annotations
import logging
from sqlalchemy import select
from scribe.models import async_session
from scribe.models.milestone import Milestone
from scribe.models.note import Note
from scribe.models.project import Project
from scribe.services import access as access_svc
from scribe.services.milestones import OPEN_STEP_STATUSES, _progress_from_counts
# Imported, not restated. The milestone summary names a plan's next open step
# too (#4154), and the two answers must agree about what "open" means.
_OPEN = OPEN_STEP_STATUSES
logger = logging.getLogger(__name__)
def _next_open(steps: list, current_id: int) -> dict | None:
"""The next open step after `current_id`, else the earliest open one before it."""
index = next((i for i, s in enumerate(steps) if s.id == current_id), -1)
later = [s for s in steps[index + 1:] if s.status in _OPEN]
earlier = [s for s in steps[:max(index, 0)] if s.status in _OPEN]
pick = (later or earlier or [None])[0]
if pick is None:
return None
return {"id": pick.id, "title": pick.title, "status": pick.status}
async def task_placement(user_id: int, task) -> dict | None:
"""Placement for `task` as `user_id` may see it, or None when it has none.
`task` is the Note the caller already holds — it was just written or read —
so its own row is not fetched again.
"""
project_id = getattr(task, "project_id", None)
milestone_id = getattr(task, "milestone_id", None)
if not project_id and not milestone_id:
return None
async with async_session() as session:
milestone = None
if milestone_id:
milestone = (await session.execute(
select(Milestone).where(
Milestone.id == milestone_id, Milestone.deleted_at.is_(None),
)
)).scalars().first()
project_id = project_id or (milestone.project_id if milestone else None)
project = None
if project_id and await access_svc.can_read_project(user_id, project_id):
project = await session.get(Project, project_id)
if project is not None and project.deleted_at is not None:
project = None
steps: list = []
if milestone is not None:
steps = list((await session.execute(
select(Note).where(
Note.milestone_id == milestone.id,
Note.status.isnot(None),
Note.deleted_at.is_(None),
access_svc.readable_notes_clause(user_id),
).order_by(Note.created_at.asc(), Note.id.asc())
)).scalars().all())
out: dict = {}
if project is not None:
out["project"] = {"id": project.id, "title": project.title}
# A milestone is shown only to someone who can read its project (or who
# owns it): the task being readable does not make its plan readable.
if milestone is not None and (project is not None or milestone.user_id == user_id):
counts: dict[str, int] = {}
for step in steps:
counts[step.status] = counts.get(step.status, 0) + 1
progress = _progress_from_counts(counts)
position = next((i for i, s in enumerate(steps, start=1) if s.id == task.id), None)
out["milestone"] = {"id": milestone.id, "title": milestone.title, "status": milestone.status}
out["position"] = {"step": position, "of": len(steps)}
out["progress"] = {k: progress[k] for k in ("completed", "total", "pct")}
out["next"] = _next_open(steps, task.id)
return out or None
async def attach_placement(user_id: int, data: dict, task) -> dict:
"""Add `placement` to a task payload the door is about to return.
Fail-open, like every in-band decoration: the write it rides on has
already happened, and a placement lookup that errors must not turn a
successful update into a reported failure. Omitted, never sent empty.
"""
try:
placement = await task_placement(user_id, task)
except Exception: # noqa: BLE001 - a decoration never breaks its payload
logger.warning("placement lookup failed for task %s", getattr(task, "id", None), exc_info=True)
return data
if placement:
data["placement"] = placement
return data