From 757d016a0c64cacfd8eada86f21df6014ab70212 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 21 Oct 2026 20:26:50 -0400 Subject: [PATCH] docs(6205): milestones comments state the reason, not the instance that taught it The pool-exhaustion, handshake-size and stepless-roadmap comments state the condition the code guards instead of the measured incident; citations go. Co-Authored-By: Claude Opus 5.5 --- src/scribe/services/milestones.py | 45 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/scribe/services/milestones.py b/src/scribe/services/milestones.py index a36fc3eb..7944ac33 100644 --- a/src/scribe/services/milestones.py +++ b/src/scribe/services/milestones.py @@ -19,14 +19,14 @@ OPEN_STEP_STATUSES = ("todo", "in_progress") def embed_milestone(milestone: Milestone) -> None: - """Refresh a milestone's vectors, fire-and-forget (milestone 415). + """Refresh a milestone's vectors, fire-and-forget. - At the service, so every path that writes a milestone gets it — the lesson - embed_note records (#2056): a record written through a door that forgot the - call stays out of search until a restart. Exceptions are swallowed because + At the service, so every path that writes a milestone gets it: a record + written through a door that forgot the call stays out of search until a + restart. Exceptions are swallowed because a milestone that saved must not fail on its index refresh; no running loop (a script, a unit test) is ordinary. A delete racing the refresh wins: the - upsert claims the milestone's row first (#3262). + upsert claims the milestone's row first. """ try: import asyncio @@ -250,12 +250,12 @@ async def get_project_milestone_summaries( The per-project version below is a nested fan-out: one query to list a project's milestones, then one more per milestone for its progress. Called - for 25 projects concurrently it asked for ~250 pooled connections against a - pool of 15, and every one of them waited out the 30-second checkout timeout - (#2384). This does the same work in a fixed number of queries and one + for many projects concurrently it asks for far more pooled connections than + the pool holds, and every one of them waits out the checkout timeout. This + does the same work in a fixed number of queries and one session: the milestones, their step counts, and their open steps. - Each row carries `next_step` — the earliest open step, or None (#4154). + Each row carries `next_step` — the earliest open step, or None. That is NOT the same question `services/placement.py` answers: placement knows which step you are on and names the next one AFTER it, while a listing has no current step, so the earliest open one is the whole answer. @@ -279,7 +279,7 @@ async def get_project_milestone_summaries( next_step: dict[int, dict] = {} if milestones: milestone_ids = [m.id for m in milestones] - # Both step queries below take the SAME visibility clause (rule 78). + # Both step queries below take the SAME visibility clause. # They have to: `next_step` names a step and the counts beside it # say how many there are, so a row that could name a step its own # progress excludes would be reporting two different milestones. @@ -304,8 +304,8 @@ async def get_project_milestone_summaries( step_touched[milestone_id] = latest # ONE query for every milestone in the batch, not one per row. - # #2384 was exactly this listing fanned out per milestone, and it - # drained the connection pool; a third flat query keeps the cost + # Fanned out per milestone, this listing drains the connection + # pool; a third flat query keeps the cost # constant in the number of plans. Ordered the way # services/placement.py orders steps — creation order, the order a # plan is written and a batch create inserts — so the first row per @@ -331,14 +331,14 @@ async def get_project_milestone_summaries( entry.update(_progress_from_counts(counts.get(m.id, {}))) # A milestone's own updated_at doesn't move when its steps do, so a # plan whose steps closed today would read as untouched since it was - # written (#4045). Touched is the later of the two. + # written. Touched is the later of the two. touched = [t for t in (m.updated_at, step_touched.get(m.id)) if t] entry["last_touched_at"] = max(touched).isoformat() if touched else None # Always present, None included. "8 of 9 done" tells a reader there is # an open step and not WHICH, and a gap that shape gets filled from # whatever id is nearest to hand — a retrieval hint carries an id and a - # title and no status, and that is how a done step was reported as the - # open one (#4154). A listing that names it leaves nothing to guess. + # title and no status, so a done step can be reported as the open one. + # A listing that names it leaves nothing to guess. entry["next_step"] = next_step.get(m.id) out.setdefault(m.project_id, []).append(entry) return out @@ -352,11 +352,10 @@ async def get_project_milestone_summary(user_id: int, project_id: int) -> list[d # What a milestone LISTING needs: enough to say what each plan is, how far # along it is, and which step is next. The plan itself (`body`) is -# get_milestone's job. Summaries once carried it, and on a project with 39 -# milestones enter_project came to ~222k characters, 168k of them plan bodies. That is past what an MCP client will -# accept as a tool result, so the session handshake arrived as a file to page -# through (#4045). user_id / project_id / timestamps repeat what the caller -# already knows. +# get_milestone's job: on a project with a few dozen milestones, plan bodies +# would push enter_project past what an MCP client accepts as a tool result, +# and the session handshake would arrive as a file to page through. +# user_id / project_id / timestamps repeat what the caller already knows. _BRIEF_FIELDS = ( "id", "title", "description", "status", "order_index", "total", "completed", "pct", "status_counts", "next_step", @@ -391,9 +390,9 @@ def unplanned_milestones( A plan written as a milestone with a description and no steps is open work that nothing else names. It is never "touched" — touching is a step changing — so the recency list that brief_milestone_summary(limit=) builds - can never reach it, and progress reads 0% either way. A project whose - roadmap was written that way ended up with every later plan opened as a - new milestone beside the one that already described it (milestone 415). + can never reach it, and progress reads 0% either way. Unlisted, a roadmap + written that way gets every later plan opened as a new milestone beside the + one that already described it. `exclude_ids` drops milestones a caller already listed. Kept in roadmap order (order_index, then creation), the order they were written in.