Fix the projects-page pool exhaustion, and cap milestone bars at 10 #95

Merged
bvandeusen merged 2 commits from dev into main 2026-08-02 19:43:00 -04:00
5 changed files with 366 additions and 31 deletions
+62 -3
View File
@@ -113,6 +113,45 @@ function truncate(text: string | null, max = 120): string {
return text.length > max ? text.slice(0, max) + "..." : text; return text.length > max ? text.slice(0, max) + "..." : text;
} }
// A card is a glance, not a report. Roundtable had ~35 milestones and its tile
// ran several viewport-heights tall, which made the grid unreadable (#2391).
const MAX_MILESTONE_BARS = 10;
interface MilestoneBar extends MilestoneSummary {
/** Position in the FULL list, so a bar keeps its colour when another
* milestone is added or finishes. Tying the palette to the visible index
* would recolour the card every time work closed. */
paletteIndex: number;
}
/** Bars to draw per project, plus how many were withheld.
*
* Ordered OPEN WORK FIRST, newest first. Recency alone would be wrong here: a
* long-running project's oldest milestones are usually its finished ones, so
* showing 10 completed bars while hiding the 3 in flight is worse than showing
* nothing. What the card is for is "what is happening", not "what happened".
*
* Computed once per load rather than called from the template — a helper in a
* v-for is re-run on every render, and this one sorts.
*/
const milestoneBars = computed(() => {
const byProject = new Map<number, { bars: MilestoneBar[]; hidden: number }>();
for (const project of projects.value) {
const all = project.summary?.milestone_summary ?? [];
const indexed: MilestoneBar[] = all.map((ms, i) => ({ ...ms, paletteIndex: i }));
const newestFirst = (a: MilestoneBar, b: MilestoneBar) => b.id - a.id;
const ordered = [
...indexed.filter((m) => m.pct < 100).sort(newestFirst),
...indexed.filter((m) => m.pct >= 100).sort(newestFirst),
];
byProject.set(project.id, {
bars: ordered.slice(0, MAX_MILESTONE_BARS),
hidden: Math.max(0, ordered.length - MAX_MILESTONE_BARS),
});
}
return byProject;
});
function overallPct(project: Project): { total: number; pct: number } { function overallPct(project: Project): { total: number; pct: number } {
const counts = project.summary?.task_counts; const counts = project.summary?.task_counts;
if (!counts) return { total: 0, pct: 0 }; if (!counts) return { total: 0, pct: 0 };
@@ -185,11 +224,11 @@ function overallPct(project: Project): { total: number; pct: number } {
<!-- Milestone progress bars --> <!-- Milestone progress bars -->
<div <div
v-if="project.summary?.milestone_summary?.length" v-if="milestoneBars.get(project.id)?.bars.length"
class="milestone-bars" class="milestone-bars"
> >
<div <div
v-for="(ms, i) in project.summary.milestone_summary" v-for="ms in milestoneBars.get(project.id)!.bars"
:key="ms.id" :key="ms.id"
class="milestone-bar-row" class="milestone-bar-row"
:title="`${ms.title} — ${ms.pct}% (${ms.completed}/${ms.total} tasks)`" :title="`${ms.title} — ${ms.pct}% (${ms.completed}/${ms.total} tasks)`"
@@ -198,11 +237,23 @@ function overallPct(project: Project): { total: number; pct: number } {
<div class="milestone-bar-track"> <div class="milestone-bar-track">
<div <div
class="milestone-bar-fill" class="milestone-bar-fill"
:style="{ width: ms.pct + '%', background: milestoneColor(i) }" :style="{ width: ms.pct + '%', background: milestoneColor(ms.paletteIndex) }"
></div> ></div>
</div> </div>
<span class="milestone-bar-pct">{{ ms.pct }}%</span> <span class="milestone-bar-pct">{{ ms.pct }}%</span>
</div> </div>
<!-- Say what is withheld. A list that simply stops reads as a
rendering bug; a count reads as a summary. Plain text, not a
link: the whole card already navigates to this project, and a
link nested inside a clickable region is a trap for keyboard
and screen-reader users. -->
<span
v-if="milestoneBars.get(project.id)!.hidden"
class="milestone-more"
>
+{{ milestoneBars.get(project.id)!.hidden }}
{{ milestoneBars.get(project.id)!.hidden === 1 ? 'more milestone' : 'more milestones' }}
</span>
</div> </div>
<div class="card-footer"> <div class="card-footer">
@@ -492,6 +543,14 @@ function overallPct(project: Project): { total: number; pct: number } {
text-align: right; text-align: right;
} }
/* Deliberately quiet — it is a footnote about what is not shown, not another
row competing with the bars above it. */
.milestone-more {
color: var(--color-text-muted);
font-size: var(--fs-size-tiny);
padding-top: 0.15rem;
}
.card-footer { .card-footer {
margin-top: auto; margin-top: auto;
} }
+21 -9
View File
@@ -1,5 +1,4 @@
"""Project management routes.""" """Project management routes."""
import asyncio
import logging import logging
from quart import Blueprint, jsonify, request from quart import Blueprint, jsonify, request
@@ -13,6 +12,7 @@ from scribe.services.projects import (
delete_project, delete_project,
get_project, get_project,
get_project_for_user, get_project_for_user,
get_project_summaries,
get_project_summary, get_project_summary,
list_projects_for_user, list_projects_for_user,
update_project, update_project,
@@ -31,16 +31,28 @@ async def list_projects_route():
include_summary = request.args.get("include_summary", "").lower() in ("1", "true") include_summary = request.args.get("include_summary", "").lower() in ("1", "true")
projects = await list_projects_for_user(uid, status=status) projects = await list_projects_for_user(uid, status=status)
if include_summary: if include_summary:
# Fetch all summaries in parallel — one backend pass instead of N+1 frontend calls # Batched: four queries plus two, in two sessions, for ALL projects.
async def _attach(project_dict: dict) -> dict: # This replaced an asyncio.gather over a per-project summary that opened
# its own session and then one more per milestone — ~250 concurrent
# checkouts against a pool of 15, all waiting out the 30s timeout and
# starving every other route on the instance (#2384).
#
# Grouped by OWNER because a shared project's counts belong to its
# owner's records, matching what the per-project path passed.
by_owner: dict[int, list[dict]] = {}
for p in projects:
by_owner.setdefault(p.get("user_id") or uid, []).append(p)
for owner_uid, owned in by_owner.items():
try: try:
owner_uid = project_dict.get("user_id") or uid # user_id now in to_dict() summaries = await get_project_summaries(
summary = await get_project_summary(owner_uid, project_dict["id"]) owner_uid, [p["id"] for p in owned]
project_dict["summary"] = summary )
except Exception: except Exception:
pass logger.warning("Project summaries failed", exc_info=True)
return project_dict continue
projects = list(await asyncio.gather(*[_attach(p) for p in projects])) for p in owned:
if p["id"] in summaries:
p["summary"] = summaries[p["id"]]
return jsonify({"projects": projects}) return jsonify({"projects": projects})
+75 -19
View File
@@ -156,26 +156,82 @@ async def get_milestone_progress(milestone_id: int) -> dict:
for status, count in rows.fetchall(): for status, count in rows.fetchall():
status_counts[status] = count status_counts[status] = count
total = sum(status_counts.values()) # Same rule as the batch path, computed in one place so the two cannot
cancelled = status_counts.get("cancelled", 0) # drift on the cancelled-exclusion.
completed = status_counts.get("done", 0) return _progress_from_counts(status_counts)
# Cancelled tasks are resolved work, not pending — exclude them from the
# percent-complete denominator so a milestone whose only open task was
# cancelled still reaches 100% (and auto-collapses) instead of stalling.
active_total = total - cancelled
pct = round(completed / active_total * 100, 1) if active_total > 0 else 0.0
return {
"total": total, def _progress_from_counts(status_counts: dict[str, int]) -> dict:
"completed": completed, """The progress shape, computed from already-fetched counts.
"pct": pct,
"status_counts": { Split out of get_milestone_progress so the batch path can reuse the rule
"todo": status_counts.get("todo", 0), rather than restate it — the cancelled-exclusion below is easy to get
"in_progress": status_counts.get("in_progress", 0), subtly different in a second copy, and then two screens disagree about
"done": status_counts.get("done", 0), whether a milestone is finished.
"cancelled": cancelled, """
}, total = sum(status_counts.values())
} cancelled = status_counts.get("cancelled", 0)
completed = status_counts.get("done", 0)
# Cancelled tasks are resolved work, not pending — excluded from the
# denominator so a milestone whose only open task was cancelled reaches
# 100% instead of stalling.
active_total = total - cancelled
return {
"total": total,
"completed": completed,
"pct": round(completed / active_total * 100, 1) if active_total > 0 else 0.0,
"status_counts": {
"todo": status_counts.get("todo", 0),
"in_progress": status_counts.get("in_progress", 0),
"done": status_counts.get("done", 0),
"cancelled": cancelled,
},
}
async def get_project_milestone_summaries(
user_id: int, project_ids: list[int]
) -> dict[int, list[dict]]:
"""Milestone summaries for MANY projects in two queries total.
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 two queries and one session.
"""
if not project_ids:
return {}
async with async_session() as session:
milestones = list((await session.execute(
select(Milestone).where(
Milestone.user_id == user_id,
Milestone.project_id.in_(project_ids),
Milestone.deleted_at.is_(None),
).order_by(Milestone.order_index.asc(), Milestone.created_at.asc())
)).scalars().all())
counts: dict[int, dict[str, int]] = {}
if milestones:
rows = await session.execute(
select(Note.milestone_id, Note.status, func.count(Note.id))
.where(
Note.milestone_id.in_([m.id for m in milestones]),
Note.status.isnot(None),
Note.deleted_at.is_(None),
)
.group_by(Note.milestone_id, Note.status)
)
for milestone_id, status, count in rows.fetchall():
counts.setdefault(milestone_id, {})[status] = count
out: dict[int, list[dict]] = {pid: [] for pid in project_ids}
for m in milestones:
entry = m.to_dict()
entry.update(_progress_from_counts(counts.get(m.id, {})))
out.setdefault(m.project_id, []).append(entry)
return out
async def get_project_milestone_summary(user_id: int, project_id: int) -> list[dict]: async def get_project_milestone_summary(user_id: int, project_id: int) -> list[dict]:
+80
View File
@@ -127,6 +127,86 @@ async def delete_project(user_id: int, project_id: int) -> bool:
return True return True
async def get_project_summaries(
user_id: int, project_ids: list[int]
) -> dict[int, dict]:
"""Summaries for MANY projects — four queries and one session, total.
Replaces an `asyncio.gather` over the per-project version below, which was
a nested fan-out: each project opened its own session for three queries,
then called the milestone summary, which opened one more per milestone. For
25 projects that asked for roughly 250 pooled connections at once against a
pool of 15 (SQLAlchemy's default 5 + 10 overflow), so most of them sat out
the 30-second checkout timeout and everything else on the instance queued
behind them — including unrelated routes, which is why /api/settings
returned 500 while /api/projects took 30.9s (#2384).
The comment it replaced said "one backend pass instead of N+1 frontend
calls". It did remove the N+1 from the network — and recreated it against
the connection pool, where it is worse: the browser had at least been
serialising those calls.
"""
if not project_ids:
return {}
async with async_session() as session:
task_rows = await session.execute(
select(Note.project_id, Note.status, func.count(Note.id))
.where(
Note.user_id == user_id,
Note.project_id.in_(project_ids),
Note.status.isnot(None),
Note.deleted_at.is_(None),
)
.group_by(Note.project_id, Note.status)
)
task_counts: dict[int, dict[str, int]] = {}
for project_id, status, count in task_rows.fetchall():
task_counts.setdefault(project_id, {})[status] = count
note_rows = await session.execute(
select(Note.project_id, func.count(Note.id))
.where(
Note.user_id == user_id,
Note.project_id.in_(project_ids),
Note.status.is_(None),
Note.deleted_at.is_(None),
)
.group_by(Note.project_id)
)
note_counts = {pid: count for pid, count in note_rows.fetchall()}
# Deliberately NOT filtered by deleted_at, matching the per-project
# version: "last activity" includes trashing something.
activity_rows = await session.execute(
select(Note.project_id, func.max(Note.updated_at))
.where(Note.user_id == user_id, Note.project_id.in_(project_ids))
.group_by(Note.project_id)
)
last_activity = {pid: ts for pid, ts in activity_rows.fetchall()}
from scribe.services.milestones import get_project_milestone_summaries
milestones = await get_project_milestone_summaries(user_id, project_ids)
return {
pid: {
# All three lifecycle keys present so consumers can sum without
# `?? 0` guards — the frontend declares them required, and
# `undefined + N` renders as NaN.
"task_counts": {
"todo": 0, "in_progress": 0, "done": 0,
**task_counts.get(pid, {}),
},
"note_count": note_counts.get(pid, 0),
"last_activity": (
last_activity[pid].isoformat() if last_activity.get(pid) else None
),
"milestone_summary": milestones.get(pid, []),
}
for pid in project_ids
}
async def get_project_summary(user_id: int, project_id: int) -> dict: async def get_project_summary(user_id: int, project_id: int) -> dict:
"""Return task counts by status, note count, and last activity.""" """Return task counts by status, note count, and last activity."""
async with async_session() as session: async with async_session() as session:
+128
View File
@@ -0,0 +1,128 @@
"""Batched project summaries (#2384).
The bug was not wrong output — it was CONNECTION COUNT. A per-project summary
opened its own session and then one more per milestone, and the route fanned
that out with asyncio.gather. For 25 projects that asked for roughly 250
checkouts against a pool of 15, so most waited out the 30-second timeout and
every other route on the instance queued behind them:
QueuePool limit of size 5 overflow 10 reached, connection timed out
GET /api/settings 500 30584.0ms
GET /api/projects 200 30882.9ms
So these tests assert the number of sessions opened, not only the values
returned. A version that produced identical output while opening a session per
project would pass a correctness test and reproduce the outage.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
def _session_factory(counter: list[int], results: list):
"""A session whose .execute() returns queued results, counting opens."""
def _make():
s = AsyncMock()
s.__aenter__ = AsyncMock(return_value=s)
s.__aexit__ = AsyncMock(return_value=False)
counter[0] += 1
async def _execute(*_a, **_kw):
rows = results.pop(0) if results else []
r = MagicMock()
r.fetchall = MagicMock(return_value=rows)
r.scalars = MagicMock(return_value=MagicMock(all=lambda: rows))
return r
s.execute = _execute
return s
return _make
@pytest.mark.asyncio
async def test_summaries_for_many_projects_open_ONE_session():
"""The whole point. Twenty-five projects must not mean twenty-five
checkouts — that is the shape that exhausted the pool."""
from scribe.services import projects as svc
opened = [0]
rows = [
[(1, "todo", 3), (1, "done", 2), (2, "in_progress", 1)], # task counts
[(1, 7)], # note counts
[], # last activity
]
with patch.object(svc, "async_session", _session_factory(opened, rows)), \
patch("scribe.services.milestones.get_project_milestone_summaries",
AsyncMock(return_value={})):
out = await svc.get_project_summaries(1, [1, 2])
assert opened[0] == 1, f"opened {opened[0]} sessions for 2 projects"
assert out[1]["task_counts"] == {"todo": 3, "in_progress": 0, "done": 2}
assert out[1]["note_count"] == 7
# A project with tasks but no notes still reports 0, not a missing key.
assert out[2]["task_counts"] == {"todo": 0, "in_progress": 1, "done": 0}
assert out[2]["note_count"] == 0
@pytest.mark.asyncio
async def test_every_requested_project_gets_an_entry():
"""A project with no notes at all must still appear. The frontend indexes
by id and renders `undefined.task_counts` as a crash, not a blank."""
from scribe.services import projects as svc
opened = [0]
with patch.object(svc, "async_session", _session_factory(opened, [[], [], []])), \
patch("scribe.services.milestones.get_project_milestone_summaries",
AsyncMock(return_value={})):
out = await svc.get_project_summaries(1, [4, 5, 6])
assert sorted(out) == [4, 5, 6]
for entry in out.values():
assert entry["task_counts"] == {"todo": 0, "in_progress": 0, "done": 0}
assert entry["note_count"] == 0
assert entry["last_activity"] is None
@pytest.mark.asyncio
async def test_no_projects_opens_no_session_at_all():
"""`.in_([])` is a valid but pointless query; the guard keeps an empty
install from paying for a connection to learn it has nothing."""
from scribe.services import projects as svc
opened = [0]
with patch.object(svc, "async_session", _session_factory(opened, [])):
assert await svc.get_project_summaries(1, []) == {}
assert opened[0] == 0
@pytest.mark.asyncio
async def test_milestone_summaries_for_many_projects_open_ONE_session():
"""Same property one level down — this was the nested half of the fan-out,
a session per MILESTONE, which is what turned 25 into ~250."""
from scribe.services import milestones as svc
m1 = MagicMock(id=10, project_id=1)
m1.to_dict = MagicMock(return_value={"id": 10, "title": "A"})
m2 = MagicMock(id=11, project_id=2)
m2.to_dict = MagicMock(return_value={"id": 11, "title": "B"})
opened = [0]
rows = [[m1, m2], [(10, "done", 2), (10, "todo", 1), (11, "cancelled", 1)]]
with patch.object(svc, "async_session", _session_factory(opened, rows)):
out = await svc.get_project_milestone_summaries(1, [1, 2])
assert opened[0] == 1
assert out[1][0]["completed"] == 2 and out[1][0]["total"] == 3
# Cancelled is excluded from the denominator, so a milestone whose only
# task was cancelled reads as complete rather than stalled at 0%.
assert out[2][0]["pct"] == 0.0 and out[2][0]["status_counts"]["cancelled"] == 1
def test_both_progress_paths_share_one_rule():
"""get_milestone_progress and the batch path must not compute pct
differently — two screens disagreeing about whether a milestone is done is
exactly the drift this codebase keeps finding."""
from scribe.services.milestones import _progress_from_counts
assert _progress_from_counts({"done": 3, "cancelled": 1})["pct"] == 100.0
assert _progress_from_counts({"cancelled": 2})["pct"] == 0.0
assert _progress_from_counts({})["total"] == 0