fix(store): reads exclude soft-deleted collections' items (BUG-3425) (#1820)

* test(store): ListItems, its search and the graph edges must exclude soft-deleted collections (BUG-3425, red)

* test(server): graph nodes exclude soft-deleted collections; the account export keeps them (BUG-3425)

* fix(store): ListItems, its search and the graph exclude soft-deleted collections; the account export opts back in (BUG-3425)

* test(store): role counts exclude soft-deleted collections; search honours the opt-out (BUG-3425, codex r1, red)

* fix(store): role counts exclude soft-deleted collections, as the role lanes do (BUG-3425, codex r1)

* docs(store): doneFiltersForWorkspace's comment no longer names GetRoleBreakdown as counting deleted collections (BUG-3425, codex r2)

* fix(store): keep SQL comments out of the role-count query (an apostrophe broke PG rebind); BUG-3333 expects a deleted collection's doc unlisted (BUG-3425)

internal/models/item.go

@@ -1835,6 +1835,11 @@ type ItemListParams struct {
 	// an outgoing parent/implements item_link. Incoming links do not count.
 	Unparented      bool
 	IncludeArchived bool
+	// IncludeDeletedCollections returns items whose collection is
+	// soft-deleted. Every read leaves them out by default (BUG-3425): a
+	// collection's delete leaves its items live. Only the account data
+	// export sets it, so a user's export still holds everything they own.
+	IncludeDeletedCollections bool
 	// NonTerminal, when true, restricts results to items whose resolved
 	// done-field value is NOT one of their collection's terminal options.
 	// Each collection is evaluated against its OWN terminal_options (falling

internal/server/bug3333_deleted_collection_grant_test.go

@@ -120,9 +120,12 @@ func TestBUG3333_DeletedCollectionNotVisibleThroughAGrant(t *testing.T) {
 		}
 	}
 
-	after := []string{"Doc in a granted collection", "Task in an assigned collection"}
+	// Nothing from either deleted collection lists: BUG-3425 leaves a
+	// soft-deleted collection's items out of every ListItems reader, the
+	// whole-collection grant's doc included (it listed here before BUG-3425).
+	after := []string{"Task in an assigned collection"}
 	if got := listTitles(); !bug3333Equal(got, after) {
-		t.Errorf("after deleting the collections the member lists %v, want %v (nothing from the item-granted collection)", got, after)
+		t.Errorf("after deleting the collections the member lists %v, want %v (nothing from either deleted collection)", got, after)
 	}
 	for _, it := range []*models.Item{hidden, granted} {
 		if canOpen(it) {

internal/server/bug3425_deleted_collection_reads_test.go

@@ -0,0 +1,104 @@
+package server
+
+import (
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// BUG-3425: the workspace graph builds its nodes from ListItems, so a
+// soft-deleted collection's items showed up as nodes for an unrestricted
+// viewer. The live task is the control.
+func TestGraphExcludesItemsOfSoftDeletedCollections(t *testing.T) {
+	srv := testServer(t)
+	slug := createWSWithCollections(t, srv)
+	ws, err := srv.store.GetWorkspaceBySlug(slug)
+	if err != nil || ws == nil {
+		t.Fatalf("workspace: %v", err)
+	}
+	doomColl, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{Name: "Drafts", Slug: "drafts", Prefix: "DRF"})
+	if err != nil {
+		t.Fatalf("create collection: %v", err)
+	}
+
+	keep := createItem(t, srv, slug, "tasks", map[string]interface{}{
+		"title": "Kept task", "fields": map[string]interface{}{"status": "open"},
+	})
+	doomed := createItem(t, srv, slug, "drafts", map[string]interface{}{"title": "Draft in a doomed collection"})
+	createBlocksLink(t, srv, slug, keep.Slug, doomed.ID)
+
+	before := getGraph(t, srv, slug, "")
+	if graphNode(before, keep.Ref) == nil || graphNode(before, doomed.Ref) == nil {
+		t.Fatalf("control: both nodes should be in the graph before the delete")
+	}
+
+	if err := srv.store.DeleteCollection(doomColl.ID, ""); err != nil {
+		t.Fatalf("soft-delete collection: %v", err)
+	}
+	after := getGraph(t, srv, slug, "")
+	if graphNode(after, doomed.Ref) != nil {
+		t.Errorf("graph still has a node for an item of a soft-deleted collection")
+	}
+	if graphNode(after, keep.Ref) == nil {
+		t.Errorf("graph lost the live node")
+	}
+	if len(after.Edges) != 0 {
+		t.Errorf("graph still has an edge to the deleted collection's item: %+v", after.Edges)
+	}
+}
+
+// The account data export keeps a user's items from collections they deleted,
+// exactly as before BUG-3425 (it opts back in with IncludeDeletedCollections).
+func TestExportAccount_KeepsItemsOfSoftDeletedCollections(t *testing.T) {
+	srv := testServer(t)
+	owner, err := srv.store.CreateUser(models.UserCreate{
+		Email: "export-3425@example.com", Name: "Export Owner", Username: "export-3425",
+		Password: "pw-test-12345",
+	})
+	if err != nil {
+		t.Fatalf("create owner: %v", err)
+	}
+	ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Export3425", OwnerID: owner.ID})
+	if err != nil {
+		t.Fatalf("create workspace: %v", err)
+	}
+	if err := srv.store.AddWorkspaceMember(ws.ID, owner.ID, "owner"); err != nil {
+		t.Fatalf("add member: %v", err)
+	}
+	liveColl, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{Name: "Notes", Slug: "notes", Prefix: "NOT"})
+	if err != nil {
+		t.Fatalf("create live collection: %v", err)
+	}
+	doomColl, err := srv.store.CreateCollection(ws.ID, models.CollectionCreate{Name: "Drafts", Slug: "drafts", Prefix: "DRF"})
+	if err != nil {
+		t.Fatalf("create doomed collection: %v", err)
+	}
+	if _, err := srv.store.CreateItem(ws.ID, liveColl.ID, models.ItemCreate{Title: "Export live marker"}); err != nil {
+		t.Fatalf("create live item: %v", err)
+	}
+	if _, err := srv.store.CreateItem(ws.ID, doomColl.ID, models.ItemCreate{Title: "Export doomed marker"}); err != nil {
+		t.Fatalf("create doomed item: %v", err)
+	}
+	if err := srv.store.DeleteCollection(doomColl.ID, ""); err != nil {
+		t.Fatalf("soft-delete collection: %v", err)
+	}
+	sessTok, err := srv.store.CreateSession(owner.ID, "go-test", "192.0.2.1", "", 24*time.Hour)
+	if err != nil {
+		t.Fatalf("CreateSession: %v", err)
+	}
+
+	rr := doRequestWithCookie(srv, "GET", "/api/v1/auth/export", nil, sessTok)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("export: %d: %s", rr.Code, rr.Body.String())
+	}
+	body := rr.Body.String()
+	if !strings.Contains(body, "Export live marker") {
+		t.Fatalf("control: the export lost the live item")
+	}
+	if !strings.Contains(body, "Export doomed marker") {
+		t.Errorf("the account export dropped an item of a soft-deleted collection")
+	}
+}

internal/server/handlers_account.go

@@ -391,7 +391,9 @@ func (s *Server) handleExportAccount(w http.ResponseWriter, r *http.Request) {
 			wsData["collections"] = collections
 
 			// Stream items per workspace (each workspace loaded individually, then GC'd)
-			items, err := s.store.ListItems(ws.ID, models.ItemListParams{IncludeArchived: true})
+			// IncludeDeletedCollections: a user's own export keeps the items
+			// of collections they deleted, as it always has (BUG-3425).
+			items, err := s.store.ListItems(ws.ID, models.ItemListParams{IncludeArchived: true, IncludeDeletedCollections: true})
 			if err != nil {
 				slog.Error("export: failed to list items", "workspace", ws.Slug, "error", err)
 				wsData["items"] = []interface{}{}

internal/store/agent_roles.go

@@ -62,12 +62,17 @@ func (s *Store) GetAgentRole(workspaceID, idOrSlug string) (*models.AgentRole, e
 }
 
 func (s *Store) ListAgentRoles(workspaceID string) ([]models.AgentRole, error) {
+	// item_count leaves out a soft-deleted collection's items: the role board
+	// lists its items through ListItems, which does (BUG-3425), and the count
+	// must agree. The check sits in the LEFT JOIN's ON so a role with no
+	// items still lists. (A comment inside this SQL broke it on Postgres.)
 	rows, err := s.db.Query(s.q(`
 		SELECT r.id, r.workspace_id, r.slug, r.name, r.description, r.icon, r.tools, r.sort_order,
 		       r.created_at, r.updated_at,
 		       COUNT(i.id) as item_count
 		FROM agent_roles r
 		LEFT JOIN items i ON i.agent_role_id = r.id AND i.deleted_at IS NULL
+			AND EXISTS (SELECT 1 FROM collections rc WHERE rc.id = i.collection_id AND rc.deleted_at IS NULL)
 		WHERE r.workspace_id = ?
 		GROUP BY r.id
 		ORDER BY r.sort_order ASC, r.name ASC
@@ -190,6 +195,7 @@ func (s *Store) GetRoleBreakdown(workspaceID string) ([]RoleBreakdown, error) {
 	rows, err := s.db.Query(s.q(fmt.Sprintf(`
 		SELECT i.agent_role_id, COUNT(*) as cnt, %s as users
 		FROM items i
+		JOIN collections rc ON rc.id = i.collection_id AND rc.deleted_at IS NULL
 		LEFT JOIN users u ON u.id = i.assigned_user_id
 		WHERE i.workspace_id = ? AND i.deleted_at IS NULL
 		  AND NOT %s

internal/store/bug3425_deleted_collection_reads_test.go

@@ -0,0 +1,258 @@
+package store
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+// BUG-3425: soft-deleting a collection sets deleted_at on the collection row
+// only, so its items stay live. ListItems, its FTS path and the graph's edges
+// joined collections without checking c.deleted_at, so an unrestricted reader
+// (who has no collection filter to catch it) kept seeing them. Every test
+// keeps a live item beside the doomed one, so a query that returns nothing
+// cannot pass.
+
+type bug3425World struct {
+	s                  *Store
+	ws                 *models.Workspace
+	liveColl, doomColl *models.Collection
+	live, doomed       *models.Item
+}
+
+func newBug3425World(t *testing.T) bug3425World {
+	t.Helper()
+	s := testStore(t)
+	ws := createTestWorkspace(t, s, "Deleted collection reads")
+	w := bug3425World{s: s, ws: ws}
+	w.liveColl = createTestCollection(t, s, ws.ID, "Notes")
+	w.doomColl = createTestCollection(t, s, ws.ID, "Drafts")
+	w.live = createTestItem(t, s, ws.ID, w.liveColl.ID, "Zephyr live row", "")
+	w.doomed = createTestItem(t, s, ws.ID, w.doomColl.ID, "Zephyr doomed row", "")
+	return w
+}
+
+func (w bug3425World) deleteDoomed(t *testing.T) {
+	t.Helper()
+	if err := w.s.DeleteCollection(w.doomColl.ID, ""); err != nil {
+		t.Fatalf("soft-delete collection: %v", err)
+	}
+}
+
+func hasItem(items []models.Item, id string) bool {
+	for _, it := range items {
+		if it.ID == id {
+			return true
+		}
+	}
+	return false
+}
+
+func TestListItems_ExcludesSoftDeletedCollections(t *testing.T) {
+	t.Parallel()
+	w := newBug3425World(t)
+
+	before, err := w.s.ListItems(w.ws.ID, models.ItemListParams{})
+	if err != nil {
+		t.Fatalf("ListItems before: %v", err)
+	}
+	if !hasItem(before, w.doomed.ID) || !hasItem(before, w.live.ID) {
+		t.Fatalf("control: both items should be listed before the delete")
+	}
+
+	w.deleteDoomed(t)
+	after, err := w.s.ListItems(w.ws.ID, models.ItemListParams{})
+	if err != nil {
+		t.Fatalf("ListItems after: %v", err)
+	}
+	if hasItem(after, w.doomed.ID) {
+		t.Errorf("ListItems still lists an item from a soft-deleted collection")
+	}
+	if !hasItem(after, w.live.ID) {
+		t.Errorf("ListItems lost the live item")
+	}
+
+	// The account data export keeps exporting everything the user owns.
+	optOut, err := w.s.ListItems(w.ws.ID, models.ItemListParams{IncludeArchived: true, IncludeDeletedCollections: true})
+	if err != nil {
+		t.Fatalf("ListItems opt-out: %v", err)
+	}
+	if !hasItem(optOut, w.doomed.ID) || !hasItem(optOut, w.live.ID) {
+		t.Errorf("IncludeDeletedCollections must keep the deleted collection's items")
+	}
+}
+
+func TestListItemsSearch_ExcludesSoftDeletedCollections(t *testing.T) {
+	t.Parallel()
+	w := newBug3425World(t)
+	search := func() []models.Item {
+		t.Helper()
+		got, err := w.s.ListItems(w.ws.ID, models.ItemListParams{Search: "Zephyr"})
+		if err != nil {
+			t.Fatalf("ListItems search: %v", err)
+		}
+		return got
+	}
+
+	if before := search(); !hasItem(before, w.doomed.ID) || !hasItem(before, w.live.ID) {
+		t.Fatalf("control: search should find both items before the delete")
+	}
+	w.deleteDoomed(t)
+	after := search()
+	if hasItem(after, w.doomed.ID) {
+		t.Errorf("search still finds an item from a soft-deleted collection")
+	}
+	if !hasItem(after, w.live.ID) {
+		t.Errorf("search lost the live item")
+	}
+	optOut, err := w.s.ListItems(w.ws.ID, models.ItemListParams{Search: "Zephyr", IncludeDeletedCollections: true})
+	if err != nil {
+		t.Fatalf("ListItems search opt-out: %v", err)
+	}
+	if !hasItem(optOut, w.doomed.ID) {
+		t.Errorf("search ignores IncludeDeletedCollections")
+	}
+}
+
+func TestListWorkspaceGraphLinks_ExcludesEdgesTouchingSoftDeletedCollections(t *testing.T) {
+	t.Parallel()
+	w := newBug3425World(t)
+	s, ws := w.s, w.ws
+	partner := createTestItem(t, s, ws.ID, w.liveColl.ID, "Partner", "")
+
+	link := func(src, tgt *models.Item, typ string) {
+		t.Helper()
+		if _, err := s.CreateItemLink(ws.ID, models.ItemLinkCreate{TargetID: tgt.ID, LinkType: typ}, src.ID); err != nil {
+			t.Fatalf("link %s -> %s: %v", src.Title, tgt.Title, err)
+		}
+	}
+	wiki := func(src, tgt *models.Item) {
+		t.Helper()
+		body := "See [[" + refOf(tgt) + "]]."
+		if _, err := s.UpdateItem(src.ID, models.ItemUpdate{Content: &body}); err != nil {
+			t.Fatalf("wiki %s -> %s: %v", src.Title, tgt.Title, err)
+		}
+	}
+	// Live control, then each edge shape touching the doomed collection from
+	// either end.
+	link(w.live, partner, "blocks")
+	link(w.doomed, partner, "related")
+	link(partner, w.doomed, "blocks")
+	wiki(w.live, partner)
+	wiki(w.doomed, w.live)
+	wiki(partner, w.doomed)
+
+	touches := func(links []GraphLink, id string) []string {
+		var out []string
+		for _, l := range links {
+			if l.SourceID == id || l.TargetID == id {
+				out = append(out, l.Type)
+			}
+		}
+		return out
+	}
+	liveEdges := func(links []GraphLink) (blocks, wikiLink bool) {
+		for _, l := range links {
+			if l.SourceID == w.live.ID && l.TargetID == partner.ID {
+				blocks = blocks || l.Type == "blocks"
+				wikiLink = wikiLink || l.Type == "wiki-link"
+			}
+		}
+		return
+	}
+
+	before, err := s.ListWorkspaceGraphLinks(ws.ID)
+	if err != nil {
+		t.Fatalf("graph links before: %v", err)
+	}
+	// Control: every doomed-collection edge shape is present before the
+	// delete (item_links both ways, wiki both ways).
+	if got := touches(before, w.doomed.ID); len(got) != 4 {
+		t.Fatalf("control: want 4 edges touching the doomed item before the delete, got %v", got)
+	}
+
+	w.deleteDoomed(t)
+	after, err := s.ListWorkspaceGraphLinks(ws.ID)
+	if err != nil {
+		t.Fatalf("graph links after: %v", err)
+	}
+	if got := touches(after, w.doomed.ID); len(got) != 0 {
+		t.Errorf("graph still has edges touching a soft-deleted collection's item: %s", strings.Join(got, ", "))
+	}
+	if b, wl := liveEdges(after); !b || !wl {
+		t.Errorf("graph lost a live edge: blocks=%v wiki-link=%v", b, wl)
+	}
+}
+
+// The title-rename cascade deliberately still rewrites link text inside a
+// soft-deleted collection's items, so a restored collection's links keep
+// working. BUG-3425 must not narrow it.
+func TestTitleRenameCascade_StillRewritesSoftDeletedCollectionSources(t *testing.T) {
+	t.Parallel()
+	w := newBug3425World(t)
+	body := "See [[Zephyr live row]]."
+	if _, err := w.s.UpdateItem(w.doomed.ID, models.ItemUpdate{Content: &body}); err != nil {
+		t.Fatalf("seed link: %v", err)
+	}
+	w.deleteDoomed(t)
+
+	title := "Zephyr renamed row"
+	if _, err := w.s.UpdateItem(w.live.ID, models.ItemUpdate{Title: &title}); err != nil {
+		t.Fatalf("rename target: %v", err)
+	}
+	got, err := w.s.GetItem(w.doomed.ID)
+	if err != nil || got == nil {
+		t.Fatalf("read source: %v", err)
+	}
+	if !strings.Contains(got.Content, "[[Zephyr renamed row]]") {
+		t.Errorf("cascade no longer rewrites a soft-deleted collection's source: %q", got.Content)
+	}
+}
+
+// Codex r1: the role lanes list their items through ListItems, so the role
+// COUNTS must leave out a soft-deleted collection's items too, or a lane
+// reports more items than it shows.
+func TestRoleCounts_ExcludeSoftDeletedCollections(t *testing.T) {
+	t.Parallel()
+	w := newBug3425World(t)
+	role, err := w.s.CreateAgentRole(w.ws.ID, models.AgentRoleCreate{Name: "Implementer"})
+	if err != nil {
+		t.Fatalf("create role: %v", err)
+	}
+	for _, it := range []*models.Item{w.live, w.doomed} {
+		if _, err := w.s.UpdateItem(it.ID, models.ItemUpdate{AgentRoleID: &role.ID}); err != nil {
+			t.Fatalf("assign role to %s: %v", it.Title, err)
+		}
+	}
+	counts := func() (listed, breakdown int) {
+		t.Helper()
+		roles, err := w.s.ListAgentRoles(w.ws.ID)
+		if err != nil {
+			t.Fatalf("ListAgentRoles: %v", err)
+		}
+		for _, r := range roles {
+			if r.ID == role.ID {
+				listed = r.ItemCount
+			}
+		}
+		rb, err := w.s.GetRoleBreakdown(w.ws.ID)
+		if err != nil {
+			t.Fatalf("GetRoleBreakdown: %v", err)
+		}
+		for _, b := range rb {
+			if b.RoleID != nil && *b.RoleID == role.ID {
+				breakdown = b.ItemCount
+			}
+		}
+		return
+	}
+
+	if l, b := counts(); l != 2 || b != 2 {
+		t.Fatalf("control: want 2 and 2 before the delete, got ListAgentRoles=%d GetRoleBreakdown=%d", l, b)
+	}
+	w.deleteDoomed(t)
+	if l, b := counts(); l != 1 || b != 1 {
+		t.Errorf("after the delete: want 1 and 1 (the live item only), got ListAgentRoles=%d GetRoleBreakdown=%d", l, b)
+	}
+}

internal/store/export_soft_deleted_collection_test.go

@@ -135,7 +135,8 @@ func TestRoundTripPreservesItemsUnderSoftDeletedCollection(t *testing.T) {
 		t.Fatalf("ImportWorkspace: %v", err)
 	}
 
-	items, err := s.ListItems(imported.ID, models.ItemListParams{})
+	// The collection is soft-deleted on both ends, so readers hide its items (BUG-3425); this asserts storage.
+	items, err := s.ListItems(imported.ID, models.ItemListParams{IncludeDeletedCollections: true})
 	if err != nil {
 		t.Fatalf("ListItems: %v", err)
 	}
@@ -215,7 +216,8 @@ func TestRoundTripPreservesDependentsUnderSoftDeletedCollection(t *testing.T) {
 	if err != nil {
 		t.Fatalf("ImportWorkspace: %v", err)
 	}
-	items, err := s.ListItems(imported.ID, models.ItemListParams{})
+	// The collection is soft-deleted on both ends, so readers hide its items (BUG-3425); this asserts storage.
+	items, err := s.ListItems(imported.ID, models.ItemListParams{IncludeDeletedCollections: true})
 	if err != nil {
 		t.Fatalf("ListItems: %v", err)
 	}

internal/store/graph.go

@@ -48,6 +48,8 @@ func graphEdgeType(linkType string) string {
 // Both queries exclude edges whose source or target item is soft-deleted,
 // matching GetParentMap's posture (BUG-734) — a dangling edge to an
 // archived item would render as a node the items query never returned.
+// For the same reason they exclude an endpoint whose COLLECTION is
+// soft-deleted, since ListItems leaves those items out (BUG-3425).
 //
 // Wiki-link rows are deduplicated per (source, target) pair — an item
 // that mentions [[TASK-5]] three times is still one edge — and
@@ -62,6 +64,8 @@ func (s *Store) ListWorkspaceGraphLinks(workspaceID string) ([]GraphLink, error)
 		FROM item_links il
 		JOIN items src ON src.id = il.source_id AND src.deleted_at IS NULL
 		JOIN items tgt ON tgt.id = il.target_id AND tgt.deleted_at IS NULL
+		JOIN collections sc ON sc.id = src.collection_id AND sc.deleted_at IS NULL
+		JOIN collections tc ON tc.id = tgt.collection_id AND tc.deleted_at IS NULL
 		WHERE il.workspace_id = ?
 	`), workspaceID)
 	if err != nil {
@@ -89,6 +93,8 @@ func (s *Store) ListWorkspaceGraphLinks(workspaceID string) ([]GraphLink, error)
 		FROM item_wiki_links wl
 		JOIN items src ON src.id = wl.source_item_id AND src.deleted_at IS NULL
 		JOIN items tgt ON tgt.id = wl.target_item_id AND tgt.deleted_at IS NULL
+		JOIN collections sc ON sc.id = src.collection_id AND sc.deleted_at IS NULL
+		JOIN collections tc ON tc.id = tgt.collection_id AND tc.deleted_at IS NULL
 		WHERE src.workspace_id = ?
 		  AND wl.target_item_id IS NOT NULL
 		  AND wl.target_workspace_id IS NULL

internal/store/items.go

@@ -1299,6 +1299,11 @@ func (s *Store) ListItems(workspaceID string, params models.ItemListParams) ([]m
 	if !params.IncludeArchived {
 		query += " AND i.deleted_at IS NULL"
 	}
+	// A soft-deleted collection leaves its items live (BUG-3425): every
+	// reader leaves them out unless it opts in.
+	if !params.IncludeDeletedCollections {
+		query += " AND c.deleted_at IS NULL"
+	}
 
 	if params.CollectionSlug != "" {
 		query += " AND c.slug = ?"
@@ -2076,6 +2081,10 @@ func (s *Store) listItemsFTS(workspaceID string, params models.ItemListParams) (
 		// the OR-combined plainto_tsquery in the dialect (BUG-842).
 		args = []interface{}{workspaceID, sanitizeFTSQuery(params.Search)}
 	}
+	// As in ListItems (BUG-3425).
+	if !params.IncludeDeletedCollections {
+		query += " AND c.deleted_at IS NULL"
+	}
 
 	if params.CollectionSlug != "" {
 		query += " AND c.slug = ?"
@@ -4653,11 +4662,11 @@ func (s *Store) childrenDoneFiltersForParent(parentItemID string) []collectionDo
 // breakdowns) that need to evaluate "is done?" for every item regardless
 // of which collection it belongs to.
 //
-// Includes soft-deleted collections: callers (e.g. GetRoleBreakdown)
-// count items in the workspace without filtering by collection
-// deleted_at, so excluding soft-deleted collections here would leave
-// their items without a matching per-collection clause and cause them
-// to always register as non-terminal.
+// Includes soft-deleted collections, so that a caller counting items
+// without a collection deleted_at filter still finds a per-collection
+// clause for every item (an item with none would always register as
+// non-terminal). GetRoleBreakdown now excludes those items itself
+// (BUG-3425); keeping them here is harmless for it.
 func (s *Store) doneFiltersForWorkspace(workspaceID string) []collectionDoneFilter {
 	rows, err := s.db.Query(
 		s.q(`SELECT id, schema, settings FROM collections WHERE workspace_id = ?`),