feat(web): collection page fetches via skinny /items-index endpoint (TASK-1349) (#491)

* feat(web): collection page fetches via skinny /items-index endpoint (TASK-1349)

Replaces every \`api.items.listByCollection(ws, coll)\` call in the
collection page with \`fetchSkinnyItems(ws, coll, includeArchived)\`,
which calls the local-first \`/items-index\` endpoint (TASK-1344)
through the typed client wrapper (TASK-1345). Items now ship
without the rich-text \`content\` body — the bulk of the per-row
wire size — until the user opens an item detail page, which still
goes through its existing full-item fetch.

Call sites updated:
  - loadCollection — primary load + plans-names lookup
  - SSE handler for item_created / item_archived / item_restored / item_updated
  - Sync coordinator's full-refresh fallback

The skinny rows are widened to \`Item[]\` at the boundary by setting
\`content: ''\` on each row. This keeps the existing view component
type contract unchanged and means existing call sites that read
\`item.content\` see an empty string — already a "nothing to do"
sentinel in the markdown-checklist progress branch.

Documented regression — out of scope for this task: non-plans
collections used to display checklist progress derived from item
content's markdown checkboxes. With \`content\` no longer fetched
for the list view, that progress no longer appears. Plans
progress is unaffected (uses /plans-progress, not content
parsing). Re-introducing the feature requires either server-side
progress on the index endpoint or a separate lazy fetch — a
follow-up rather than a blocker for the bandwidth win.

In-scope behavior preserved:
  - Item create/update flow: server still returns full items, dropped
    into the array as-is; sync coordinator's incremental updates
    similarly use the full-item type from the changes feed
  - Server-side FTS search via \`searchResultIds\`: still id-keyed,
    works against skinny rows
  - List / Board / Table view components: already only read fields
    present on the skinny row (title, fields, tags, sort_order…)
  - Detail page fetch: unchanged — still goes through
    \`api.items.get\` which returns the full Item with content

Parent: PLAN-1343.

* fix(api+web): add /collections/{coll}/checkbox-progress endpoint to preserve list-view checklist progress per Codex review (round 1)

Codex round 1 [P2] flagged that the original PR shipped a real
regression: non-plans collections used to compute markdown-checkbox
progress client-side from `item.content`, and the skinny
`/items-index` endpoint dropped `content` from the payload — so
list/board/table progress badges silently stopped appearing on
docs/tasks/custom collections.

This commit closes that gap with a new server endpoint that
computes the same `{item_id, total, done}` counts via
LENGTH/REPLACE arithmetic on the stored content, returning only
the small derived counts. No item bodies cross the wire.

Server (Go):
  - `store.CollectionCheckboxProgress(workspaceID, collectionID)` —
    SQL: `(LENGTH(content) - LENGTH(REPLACE(content, '- [ ]', '')))
    / 5 + (LENGTH(content) - LENGTH(REPLACE(content, '- [x]', '')))
    / 5` for total, the second clause alone for done. Same trick on
    SQLite and PostgreSQL.
  - `handleCollectionCheckboxProgress` — collection-visibility +
    item-grant filter so guests / restricted members can't enumerate
    items they shouldn't see. Mirrors `guestResourceFilter` exactly.
  - Route: `GET /api/v1/workspaces/{ws}/collections/{coll}/checkbox-progress`.
  - Test `TestCollectionCheckboxProgress` covers the math (open +
    done counts), zero-result rows are filtered, unknown collection
    → 404, empty result → 200 + `[]`.

Web:
  - `api.items.collectionCheckboxProgress(ws, coll)`
  - Both call sites in `+page.svelte` (initial `loadCollection`
    non-plans branch + `refreshProgress` non-plans branch) now
    pull from the endpoint instead of parsing `item.content`.
  - Drops the previous "documented regression" comment — the
    feature is fully preserved.

Sub-100-byte response per item (vs. the full content body) so the
bandwidth win from `/items-index` is preserved. The endpoint scans
content server-side, but doesn't transmit it — the original
listByCollection call both scanned AND transmitted content.

Parent: PLAN-1343.

* fix(api+web): plumb include_archived through checkbox-progress per Codex review (round 2)

Codex round 2 [P2] caught that the Archived toggle path lost
checklist progress badges: `CollectionCheckboxProgress` hard-coded
`deleted_at IS NULL`, but the page-side fetch is called with the
same `showArchived` flag that toggles whether archived items
render. With the toggle on, archived non-plan items appeared in
the list but had no `itemProgress` row — the old client-side parse
would have counted them.

Fix: thread `includeArchived` through the call chain.

  - store.CollectionCheckboxProgress(workspaceID, collectionID,
    includeArchived bool) — appends `AND deleted_at IS NULL` only
    when includeArchived is false. Default match the original
    archived-off behavior.
  - handleCollectionCheckboxProgress reads
    ?include_archived=true and forwards.
  - api.items.collectionCheckboxProgress(ws, coll, { includeArchived })
    on the client.
  - +page.svelte's two call sites pass `showArchived` /
    `includeArchived` exactly.

TestCollectionCheckboxProgress now archives one of the seeded
items and asserts:
  - default response excludes the archived item (1 row)
  - ?include_archived=true response includes it (2 rows)

Also clarified the const-doc on `checkboxCountSQL` to reflect the
dynamic deleted-at clause.
This commit is contained in:
xarmian
2026-05-11 11:51:06 -04:00
committed by GitHub
parent f699d3480e
commit d6894def4f
6 changed files with 366 additions and 22 deletions
+97
View File
@@ -1072,6 +1072,103 @@ func (s *Server) publishItemEventWithName(eventType, workspaceID, itemID, title,
})
}
// handleCollectionCheckboxProgress returns per-item markdown-checkbox
// progress for items in a single collection — the bookkeeping that
// powers the list/board/table progress badges for non-plans
// collections. Pairs with /items-index (TASK-1349 / PLAN-1343 Phase 1):
// the index endpoint omits content to keep the wire payload small,
// and this endpoint computes the checkbox counts server-side via
// LENGTH/REPLACE arithmetic so the client doesn't need content at
// all to render the progress UI.
//
// Visibility: enforces the same collection-visibility + item-grant
// rules as handleListItems / handleListItemsIndex. Items the caller
// can't see contribute zero rows to the response.
func (s *Server) handleCollectionCheckboxProgress(w http.ResponseWriter, r *http.Request) {
workspaceID, ok := s.getWorkspaceID(w, r)
if !ok {
return
}
collSlug := chi.URLParam(r, "collSlug")
coll, err := s.store.GetCollectionBySlug(workspaceID, collSlug)
if err != nil {
writeInternalError(w, err)
return
}
if coll == nil {
writeError(w, http.StatusNotFound, "not_found", "Collection not found")
return
}
// Collection-visibility gate.
visibleIDs, err := s.visibleCollectionIDs(r, workspaceID)
if err != nil {
writeInternalError(w, err)
return
}
if !isCollectionVisible(coll.ID, visibleIDs) {
writeError(w, http.StatusNotFound, "not_found", "Collection not found")
return
}
includeArchived := r.URL.Query().Get("include_archived") == "true"
progress, err := s.store.CollectionCheckboxProgress(workspaceID, coll.ID, includeArchived)
if err != nil {
writeInternalError(w, err)
return
}
// Item-grant filtering: if the caller has item-level grants (guest
// / restricted member) and the collection isn't fully granted, drop
// rows for items outside their grant set. This mirrors the same
// filter handleListItems / handleListItemsIndex apply via
// guestResourceFilter — without it, a guest could enumerate the
// existence of items they can't otherwise see by reading their
// checkbox progress.
fullCollIDs, grantedItemIDs, grantErr := s.guestResourceFilter(r, workspaceID)
if grantErr != nil {
writeInternalError(w, grantErr)
return
}
if len(grantedItemIDs) > 0 {
hasFullCollectionGrant := false
for _, id := range fullCollIDs {
if id == coll.ID {
hasFullCollectionGrant = true
break
}
}
if !hasFullCollectionGrant && workspaceRole(r) != "guest" {
memberColls, _ := s.store.GetMemberCollectionAccess(workspaceID, currentUserID(r))
for _, id := range memberColls {
if id == coll.ID {
hasFullCollectionGrant = true
break
}
}
}
if !hasFullCollectionGrant {
granted := make(map[string]struct{}, len(grantedItemIDs))
for _, id := range grantedItemIDs {
granted[id] = struct{}{}
}
filtered := progress[:0]
for _, p := range progress {
if _, ok := granted[p.ItemID]; ok {
filtered = append(filtered, p)
}
}
progress = filtered
}
}
if progress == nil {
progress = []store.ItemCheckboxProgress{}
}
writeJSON(w, http.StatusOK, progress)
}
// handlePlansProgress returns child item completion progress for all non-deleted plans.
// This is a backward-compat endpoint; the general form is per-item via /items/{slug}/children.
func (s *Server) handlePlansProgress(w http.ResponseWriter, r *http.Request) {
+108
View File
@@ -1540,6 +1540,114 @@ func TestListItemsIndex_DoesNotShadowItemSlug(t *testing.T) {
}
}
// TestCollectionCheckboxProgress covers the markdown-checkbox progress
// endpoint that pairs with /items-index (TASK-1349). Verifies SQL
// LENGTH/REPLACE arithmetic produces the same per-item counts the
// client used to compute from item.content before /items-index made
// content unavailable in list view.
func TestCollectionCheckboxProgress(t *testing.T) {
srv := testServer(t)
slug := createWSWithCollections(t, srv)
// Item with 2 open + 1 done checkbox.
mixed := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "Has checklist",
"content": "Do this:\n- [ ] alpha\n- [x] beta\n- [ ] gamma\n",
"fields": `{"status":"open"}`,
})
// Item with no checkboxes — must be excluded from the response.
createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "No checklist",
"content": "Just prose, nothing to count here.",
"fields": `{"status":"open"}`,
})
// Item with only done checkboxes — total == done.
allDone := createItem(t, srv, slug, "tasks", map[string]interface{}{
"title": "All done",
"content": "- [x] one\n- [x] two\n",
"fields": `{"status":"done"}`,
})
rr := doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress", nil)
if rr.Code != http.StatusOK {
t.Fatalf("checkbox-progress: expected 200, got %d: %s", rr.Code, rr.Body.String())
}
type progressRow struct {
ItemID string `json:"item_id"`
Total int `json:"total"`
Done int `json:"done"`
}
var resp []progressRow
parseJSON(t, rr, &resp)
if len(resp) != 2 {
t.Fatalf("expected 2 rows (mixed + allDone), got %d: %+v", len(resp), resp)
}
byID := map[string]progressRow{}
for _, r := range resp {
byID[r.ItemID] = r
}
if r, ok := byID[mixed.ID]; !ok {
t.Fatalf("mixed item missing from response")
} else if r.Total != 3 || r.Done != 1 {
t.Fatalf("mixed item: expected total=3, done=1, got total=%d, done=%d", r.Total, r.Done)
}
if r, ok := byID[allDone.ID]; !ok {
t.Fatalf("allDone item missing from response")
} else if r.Total != 2 || r.Done != 2 {
t.Fatalf("allDone item: expected total=2, done=2, got total=%d, done=%d", r.Total, r.Done)
}
// Unknown collection → 404.
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/nonexistent/checkbox-progress", nil)
if rr.Code != http.StatusNotFound {
t.Fatalf("expected 404 for unknown collection, got %d: %s", rr.Code, rr.Body.String())
}
// Empty result (collection has no items with checkboxes) → 200 + [].
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/ideas/checkbox-progress", nil)
if rr.Code != http.StatusOK {
t.Fatalf("checkbox-progress empty: expected 200, got %d", rr.Code)
}
if !bytes.Contains(rr.Body.Bytes(), []byte(`[]`)) {
t.Fatalf("expected empty array body, got %s", rr.Body.String())
}
// Archive `allDone` and confirm it drops out of the default response
// but reappears with ?include_archived=true. Mirrors the Archived
// toggle on the collection page (Codex round 2 [P2] on PR #491).
rr = doRequest(srv, "DELETE", "/api/v1/workspaces/"+slug+"/items/"+allDone.Slug, nil)
if rr.Code != http.StatusNoContent {
t.Fatalf("archive allDone: expected 204, got %d", rr.Code)
}
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress", nil)
if rr.Code != http.StatusOK {
t.Fatalf("checkbox-progress default: expected 200, got %d", rr.Code)
}
resp = nil
parseJSON(t, rr, &resp)
for _, r := range resp {
if r.ItemID == allDone.ID {
t.Fatalf("archived item should not appear in default checkbox-progress response")
}
}
if len(resp) != 1 {
t.Fatalf("expected 1 row (mixed) after archiving allDone, got %d", len(resp))
}
rr = doRequest(srv, "GET", "/api/v1/workspaces/"+slug+"/collections/tasks/checkbox-progress?include_archived=true", nil)
if rr.Code != http.StatusOK {
t.Fatalf("checkbox-progress include_archived: expected 200, got %d", rr.Code)
}
resp = nil
parseJSON(t, rr, &resp)
if len(resp) != 2 {
t.Fatalf("expected 2 rows with include_archived, got %d", len(resp))
}
}
func firstID(items []models.Item) string {
if len(items) == 0 {
return ""
+5
View File
@@ -1009,6 +1009,11 @@ func (s *Server) setupRouter() {
// Items within collection
r.Get("/items", s.handleListCollectionItems)
r.Post("/items", s.handleCreateItem)
// Pairs with /items-index — server-side checkbox
// progress so the collection page can render
// list/board/table progress badges without
// fetching item content (TASK-1349).
r.Get("/checkbox-progress", s.handleCollectionCheckboxProgress)
// Collection grants
r.Get("/grants", s.handleListCollectionGrants)
r.Post("/grants", s.handleCreateCollectionGrant)
+76
View File
@@ -783,6 +783,82 @@ func (s *Store) ListItemsIndex(workspaceID string, params ItemIndexParams) ([]mo
return scanItemsIndex(rows)
}
// ItemCheckboxProgress is the per-item count of markdown checkboxes
// (`- [ ]` / `- [x]`) extracted from item content. Used by the
// collection page to render checklist progress badges without
// shipping the rich-text body over the wire (PLAN-1343 Phase 1 /
// TASK-1349).
type ItemCheckboxProgress struct {
ItemID string `json:"item_id"`
Total int `json:"total"`
Done int `json:"done"`
}
// checkboxCountSQL is the SQL fragment used to count `- [ ]` and
// `- [x]` markers inside item content. Implemented identically on
// SQLite and PostgreSQL via the LENGTH/REPLACE arithmetic trick —
// both dialects support LENGTH and REPLACE on TEXT, and integer
// division is identical.
//
// The `i.deleted_at` clause is appended dynamically in
// CollectionCheckboxProgress so callers can request progress for
// archived rows (matches /items-index's include_archived semantics).
const checkboxCountSQL = `
SELECT i.id,
(LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [ ]', ''))) / 5
+ (LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [x]', ''))) / 5 AS total,
(LENGTH(i.content) - LENGTH(REPLACE(i.content, '- [x]', ''))) / 5 AS done
FROM items i
WHERE i.workspace_id = ?
AND i.collection_id = ?
AND i.content LIKE '%- [%]%'
`
// CollectionCheckboxProgress returns the per-item checkbox totals for
// every item in a collection whose content has at least one
// `- [ ]` / `- [x]` marker. The query computes counts server-side via
// LENGTH/REPLACE arithmetic so the wire payload stays small (three
// ints per non-zero item) — much cheaper than shipping every item's
// rich-text body just so the client can grep for checkboxes.
//
// includeArchived controls whether soft-deleted items contribute
// rows. The default (false) matches the pre-existing client-side
// parse for the un-toggled view. With the page's Archived toggle
// on, the collection page renders archived items too — passing
// true preserves their progress badges (per Codex round 2 [P2] on
// PR #491).
//
// Items with no markers, or with non-positive totals after subtracting
// done from open, are filtered out. Result order is unspecified.
func (s *Store) CollectionCheckboxProgress(workspaceID, collectionID string, includeArchived bool) ([]ItemCheckboxProgress, error) {
query := checkboxCountSQL
if !includeArchived {
query += " AND i.deleted_at IS NULL"
}
rows, err := s.db.Query(s.q(query), workspaceID, collectionID)
if err != nil {
return nil, fmt.Errorf("collection checkbox progress: %w", err)
}
defer rows.Close()
var result []ItemCheckboxProgress
for rows.Next() {
var p ItemCheckboxProgress
if err := rows.Scan(&p.ItemID, &p.Total, &p.Done); err != nil {
return nil, err
}
// Skip rows with no checkboxes — the LIKE filter is a fast
// preliminary check, but item bodies can contain the substring
// inside a code block or other context that doesn't end up as
// a markdown checkbox; the per-row Total accounts for that.
if p.Total <= 0 {
continue
}
result = append(result, p)
}
return result, rows.Err()
}
// scanItemsIndex scans rows from ListItemsIndex (skinny projection — no
// i.content column).
func scanItemsIndex(rows *sql.Rows) ([]models.Item, error) {
+20
View File
@@ -472,6 +472,26 @@ export const api = {
plansProgress: (ws: string) =>
request<{item_id: string; total: number; done: number}[]>(`/workspaces/${ws}/plans-progress`),
/**
* Markdown-checkbox progress for items in a single collection.
* The server scans `- [ ]` / `- [x]` markers in each item's
* `content` and returns `{item_id, total, done}` for items with
* at least one checkbox. Pairs with `listIndex` (TASK-1349):
* the index endpoint omits content for bandwidth, this endpoint
* supplies the small derived counts the views need to render
* progress badges.
*/
collectionCheckboxProgress: (
ws: string,
coll: string,
opts?: { includeArchived?: boolean }
) =>
request<{item_id: string; total: number; done: number}[]>(
`/workspaces/${ws}/collections/${coll}/checkbox-progress${qs({
include_archived: opts?.includeArchived ? 'true' : undefined,
})}`
),
/** Star an item for the current user (idempotent) */
star: (ws: string, itemSlug: string) =>
request<void>(`/workspaces/${ws}/items/${itemSlug}/star`, {
@@ -290,7 +290,7 @@
case 'item_archived':
case 'item_restored': {
try {
items = await api.items.listByCollection(ws, coll);
items = await fetchSkinnyItems(ws, coll, false);
} catch {
// Ignore fetch errors — will retry on next event
}
@@ -298,7 +298,7 @@
}
case 'item_updated': {
try {
items = await api.items.listByCollection(ws, coll);
items = await fetchSkinnyItems(ws, coll, false);
} catch {
// Ignore fetch errors
}
@@ -357,8 +357,7 @@
// Full refresh fallback
try {
const listParams = showArchived ? { include_archived: true } : undefined;
const freshItems = await api.items.listByCollection(wsSlug, collSlug, listParams);
const freshItems = await fetchSkinnyItems(wsSlug, collSlug, showArchived);
items = freshItems;
await refreshProgress(wsSlug, collSlug, freshItems);
syncService.markSynced(); // Advance cursor now that reload succeeded
@@ -382,6 +381,27 @@
}
});
/**
* Fetch a collection's items via the skinny /items-index endpoint
* (TASK-1349 / PLAN-1343 Phase 1). The endpoint omits the rich-text
* `content` body, which is the bulk of an item's wire size and is
* only needed when the user opens the detail page.
*
* The result is widened to `Item[]` by setting `content: ''` on
* every row. This satisfies the existing type contract — view
* components and progress code already treat empty content as
* "nothing to compute" — without leaking a custom skinny type
* through the whole call graph. The detail-page fetch still
* returns full items, so opening any item rehydrates `content`.
*/
async function fetchSkinnyItems(ws: string, coll: string, includeArchived: boolean): Promise<Item[]> {
const resp = await api.items.listIndex(ws, {
collection: coll,
includeArchived,
});
return resp.items.map((row) => ({ ...row, content: '' }));
}
async function refreshProgress(ws: string, coll: string, itemList: typeof items) {
if (coll === 'plans') {
const progress = await api.items.plansProgress(ws).catch(() => []);
@@ -391,13 +411,23 @@
}
itemProgress = map;
} else {
// Non-plans collections: pull markdown-checkbox progress from
// the new server-side endpoint. Pre-TASK-1349 this loop walked
// `it.content` client-side, but the skinny /items-index
// payload doesn't ship content. The server endpoint computes
// the same counts via LENGTH/REPLACE arithmetic on the
// stored rows — same shape `{item_id, total, done}` as
// /plans-progress.
//
// Pass `includeArchived` so the toggle-on view (which renders
// archived rows alongside live ones) still gets their
// progress badges. Per Codex round 2 [P2] on PR #491.
const map: Record<string, { total: number; done: number }> = {};
for (const it of itemList) {
if (!it.content) continue;
const total = (it.content.match(/- \[[ x]\]/g) ?? []).length;
if (total === 0) continue;
const done = (it.content.match(/- \[x\]/g) ?? []).length;
map[it.id] = { total, done };
const progress = await api.items
.collectionCheckboxProgress(ws, coll, { includeArchived: showArchived })
.catch(() => []);
for (const p of progress) {
map[p.item_id] = { total: p.total, done: p.done };
}
itemProgress = map;
}
@@ -406,10 +436,9 @@
async function loadCollection(ws: string, coll: string, includeArchived = false) {
loading = true;
try {
const listParams = includeArchived ? { include_archived: true } : undefined;
const [collData, itemsData, viewsData, membersData] = await Promise.all([
api.collections.get(ws, coll),
api.items.listByCollection(ws, coll, listParams),
fetchSkinnyItems(ws, coll, includeArchived),
api.views.list(ws, coll).catch(() => [] as View[]),
api.members.list(ws).catch(() => ({ members: [], invitations: [] }))
]);
@@ -433,23 +462,32 @@
itemProgress = {};
}
} else {
// Compute checklist progress from item content (markdown checkboxes)
const map: Record<string, { total: number; done: number }> = {};
for (const it of itemsData) {
if (!it.content) continue;
const total = (it.content.match(/- \[[ x]\]/g) ?? []).length;
if (total === 0) continue;
const done = (it.content.match(/- \[x\]/g) ?? []).length;
map[it.id] = { total, done };
// Non-plans collections: pull progress from the new
// /collections/{coll}/checkbox-progress endpoint instead
// of parsing `item.content` client-side. /items-index
// doesn't ship content, so the old client-side parse
// would be a no-op; the server-side endpoint computes
// the same counts via LENGTH/REPLACE arithmetic on the
// stored rows. `includeArchived` is plumbed through so
// the toggle-on view keeps progress badges on archived
// items (per Codex round 2 [P2] on PR #491).
try {
const progress = await api.items.collectionCheckboxProgress(ws, coll, { includeArchived });
const map: Record<string, { total: number; done: number }> = {};
for (const p of progress) {
map[p.item_id] = { total: p.total, done: p.done };
}
itemProgress = map;
} catch {
itemProgress = {};
}
itemProgress = map;
progressLabel = 'done';
}
// Fetch plan names for relation display on task cards
if (coll === 'tasks') {
try {
const plans = await api.items.listByCollection(ws, 'plans');
const plans = await fetchSkinnyItems(ws, 'plans', false);
const labels: Record<string, string> = {};
for (const p of plans) {
labels[p.id] = p.title;