diff --git a/cmd/pad/cmd_server.go b/cmd/pad/cmd_server.go index 3ffc029df..9597bbf66 100644 --- a/cmd/pad/cmd_server.go +++ b/cmd/pad/cmd_server.go @@ -177,6 +177,23 @@ func serveCmd() *cobra.Command { "items_scanned", bf.ItemsScanned, "errors", bf.Errors) } + // The same job for `relation` field values (PLAN-2857 U5). + // Non-fatal for the same reason: a database without the reverse + // index is a database missing a "Referenced by" section, not a + // broken one, and refusing to boot over it would be the worse + // failure. + if rb, err := s.BackfillRelationLinks(); err != nil { + slog.Warn("relation-link backfill failed; non-fatal", "error", err) + } else if rb.LinksInserted > 0 { + slog.Info("Relation-link backfill complete", + "items_scanned", rb.ItemsScanned, + "links_inserted", rb.LinksInserted, + ) + } else if !rb.Skipped { + slog.Debug("Relation-link backfill found nothing to index", + "items_scanned", rb.ItemsScanned) + } + // Backfill: populate status_transitions from the historical // activity log (PLAN-1628 / TASK-1637). Idempotent — gated on an // empty table, so it replays history exactly once on the first diff --git a/internal/server/handlers_relation_backlinks.go b/internal/server/handlers_relation_backlinks.go new file mode 100644 index 000000000..f00254146 --- /dev/null +++ b/internal/server/handlers_relation_backlinks.go @@ -0,0 +1,104 @@ +package server + +import ( + "net/http" + "strconv" + + "github.com/go-chi/chi/v5" + + "github.com/PerpetualSoftware/pad/internal/store" +) + +// handleGetItemRelationBacklinks answers "which items point at this one +// through a relation field?" (PLAN-2857 U5 / TASK-2997). +// +// Separate from handleGetItemBacklinks rather than folded into it. The two +// answer different questions — a `[[wiki-link]]` in prose is not a typed +// field-valued edge — and they have different shapes: a relation backlink +// carries the FIELD KEY it points through and has no snippet, because a field +// value has no surrounding text to quote. Merging them would mean a response +// where half the entries have a snippet and half have a field key, and every +// consumer branching on which. +// +// THE COUNT IS THE VIEWER'S (ratified, day 55). Visibility comes from the same +// guestResourceFilter the wiki backlinks use, so "referenced by 3" means "by 3 +// you can see". A true count would leak the existence of items the viewer has +// no access to. +// +// No cross-workspace tier, unlike wiki backlinks: a relation value names an +// item id, and PLAN-2857 v1 excludes cross-workspace relation targets — a +// carried value is dropped at the boundary rather than resolved. There is +// nothing foreign to page through. +func (s *Server) handleGetItemRelationBacklinks(w http.ResponseWriter, r *http.Request) { + workspaceID, ok := s.getWorkspaceID(w, r) + if !ok { + return + } + + itemSlug := chi.URLParam(r, "itemSlug") + // IncludeDeleted, matching the wiki handler: a soft-deleted item still has + // a page, and "who pointed at this?" is a question that outlives it. + item, err := s.store.ResolveItemIncludeDeleted(workspaceID, itemSlug) + if err != nil { + writeInternalError(w, err) + return + } + if item == nil { + writeError(w, http.StatusNotFound, "not_found", "Item not found") + return + } + if !s.requireItemVisible(w, r, workspaceID, item) { + return + } + + limit := 50 + if v := r.URL.Query().Get("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n > 0 { + limit = n + } + } + if limit > 300 { + limit = 300 + } + offset := 0 + if v := r.URL.Query().Get("offset"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n >= 0 { + offset = n + } + } + + fullCollIDs, grantedItemIDs, gErr := s.guestResourceFilter(r, workspaceID) + if gErr != nil { + writeInternalError(w, gErr) + return + } + vis := store.BacklinksVisibility{Unrestricted: fullCollIDs == nil && grantedItemIDs == nil} + if !vis.Unrestricted { + vis.FullCollectionIDs = fullCollIDs + vis.GrantedItemIDs = grantedItemIDs + } + + total, err := s.store.CountRelationBacklinks(item.ID, workspaceID, vis) + if err != nil { + writeInternalError(w, err) + return + } + links, err := s.store.GetRelationBacklinks(item.ID, workspaceID, limit, offset, vis) + if err != nil { + writeInternalError(w, err) + return + } + if links == nil { + links = []store.RelationBacklink{} + } + + writeJSON(w, http.StatusOK, map[string]any{ + "relation_backlinks": links, + // `total` is what the header renders as "Referenced by N". It is + // computed under the SAME visibility and the SAME filters as the page, + // so the header cannot promise a number the list will not produce. + "total": total, + "limit": limit, + "offset": offset, + }) +} diff --git a/internal/server/relation_backlinks_test.go b/internal/server/relation_backlinks_test.go new file mode 100644 index 000000000..eb498a919 --- /dev/null +++ b/internal/server/relation_backlinks_test.go @@ -0,0 +1,113 @@ +package server + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// The "Referenced by N" door (PLAN-2857 U5 / TASK-2997). +// +// The store tests own the index's correctness. These own the two things only +// the door can answer: that the endpoint is wired to the index at all, and +// that the number it reports is the VIEWER'S — which is the ratified +// visibility decision, and the one with a disclosure consequence. + +type relationBacklinksResponse struct { + RelationBacklinks []struct { + SourceItemID string `json:"source_item_id"` + SourceRef string `json:"source_ref"` + SourceTitle string `json:"source_title"` + FieldKey string `json:"field_key"` + } `json:"relation_backlinks"` + Total int `json:"total"` +} + +// getRelationBacklinks drives the endpoint as `user` and decodes the payload. +func (f *doorFixture) getRelationBacklinks(user *models.User, role string, target *models.Item) relationBacklinksResponse { + f.t.Helper() + var rr = f.callAs(user, role, f.srv.handleGetItemRelationBacklinks, "GET", + "/api/v1/workspaces/"+f.ws.Slug+"/items/"+target.Slug+"/relation-backlinks", + map[string]string{"itemSlug": target.Slug}, nil) + if rr.Code != http.StatusOK { + f.t.Fatalf("relation-backlinks: expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + var out relationBacklinksResponse + if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil { + f.t.Fatalf("decode: %v\nbody: %s", err, rr.Body.String()) + } + return out +} + +// TestRelationBacklinksDoor_ReportsTheEdgeWithItsFieldKey is the wiring claim: +// the endpoint reads the index, and it carries the field key that makes a +// relation backlink different from a wiki one. +func TestRelationBacklinksDoor_ReportsTheEdgeWithItsFieldKey(t *testing.T) { + f := newDoorFixture(t) + source := f.seed(`{"owner_ref":"` + f.target.ID + `"}`) + + got := f.getRelationBacklinks(f.owner, "owner", f.target) + if got.Total != 1 { + t.Fatalf("total = %d, want 1 — the endpoint is not reading the index", got.Total) + } + if len(got.RelationBacklinks) != 1 { + t.Fatalf("want 1 backlink, got %d", len(got.RelationBacklinks)) + } + b := got.RelationBacklinks[0] + if b.SourceItemID != source.ID { + t.Errorf("source = %s, want %s", b.SourceItemID, source.ID) + } + if b.FieldKey != "owner_ref" { + t.Errorf("field_key = %q, want %q — without it the panel cannot say WHICH field points here, which is the whole reason this is not a wiki backlink", b.FieldKey, "owner_ref") + } + if b.SourceTitle == "" || b.SourceRef == "" { + t.Errorf("the backlink lost the source's identity: %+v", b) + } +} + +// TestRelationBacklinksDoor_TheCountIsTheViewersNotTheTruth is the ratified +// visibility decision, and the reason it needs a test is that the "wrong" +// behaviour here looks more correct: reporting the true count is what a +// naive implementation does, and it leaks the existence of items the viewer +// cannot see. +func TestRelationBacklinksDoor_TheCountIsTheViewersNotTheTruth(t *testing.T) { + f := newDoorFixture(t) + + // Two sources point at the same target. The restricted viewer is granted + // exactly one of them. + visibleSource := f.seed(`{"owner_ref":"` + f.target.ID + `"}`) + f.seed(`{"owner_ref":"` + f.target.ID + `"}`) // the one they cannot see + + if got := f.getRelationBacklinks(f.owner, "owner", f.target); got.Total != 2 { + t.Fatalf("the OWNER should see both (total %d); without that the restricted leg proves nothing", got.Total) + } + + blind := mustUser(t, f.srv, "relation-backlinks-blind@example.com", "relbacklinksblind", "") + if err := f.srv.store.AddWorkspaceMember(f.ws.ID, blind.ID, "editor"); err != nil { + t.Fatalf("AddWorkspaceMember: %v", err) + } + // Can see the target's collection, and among the SOURCES only the one + // granted below. + if err := f.srv.store.SetMemberCollectionAccess(f.ws.ID, blind.ID, "specific", + []string{f.people.ID}); err != nil { + t.Fatalf("SetMemberCollectionAccess: %v", err) + } + if _, err := f.srv.store.CreateItemGrant(f.ws.ID, visibleSource.ID, blind.ID, "read", f.owner.ID); err != nil { + t.Fatalf("CreateItemGrant: %v", err) + } + + got := f.getRelationBacklinks(blind, "editor", f.target) + if got.Total != 1 { + t.Errorf("total = %d, want 1 — the count must be the VIEWER'S; reporting 2 tells them an item they cannot see exists", got.Total) + } + if len(got.RelationBacklinks) != 1 || got.RelationBacklinks[0].SourceItemID != visibleSource.ID { + t.Errorf("the page leaked a source the viewer cannot see: %+v", got.RelationBacklinks) + } + // The count and the page must agree. A header promising a number the list + // cannot produce is the drift CountBacklinks documents for the wiki side. + if got.Total != len(got.RelationBacklinks) { + t.Errorf("total %d but %d rows — the count and the page ran under different filters", got.Total, len(got.RelationBacklinks)) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 686eb6f23..d2e761c93 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1807,6 +1807,7 @@ func (s *Server) setupRouter() { r.Get("/children", s.handleGetItemChildren) r.Get("/progress", s.handleGetItemProgress) r.Get("/backlinks", s.handleGetItemBacklinks) + r.Get("/relation-backlinks", s.handleGetItemRelationBacklinks) r.Get("/tasks", s.handleGetItemChildren) // deprecated alias r.Get("/grants", s.handleListItemGrants) r.Post("/grants", s.handleCreateItemGrant) diff --git a/internal/store/attachments.go b/internal/store/attachments.go index e7d518bf9..c5d58c4b9 100644 --- a/internal/store/attachments.go +++ b/internal/store/attachments.go @@ -1502,6 +1502,25 @@ func (s *Store) RemapAttachmentReferencesInWorkspace(workspaceID string, oldToNe } defer tx.Rollback() + // Serialise against concurrent item writes in this workspace (codex round + // 5 on PLAN-2857 U5). This function SCANS every item's fields and content, + // then writes the snapshot back — so without the lock a concurrent update + // can commit newer fields in between and this overwrites them. + // + // That lost update predates the relation index and is not caused by it; + // what U5 adds is that the same stale snapshot is now also used to rebuild + // the reverse index, so the staleness reaches a second place. One lock + // closes both. + // + // It is the same advisory lock ITEM writers take, and taking it first here + // preserves their order (workspace, then rows) so the two cannot invert. + // Comment writers do not take it at all — which is safe, because a lock + // nobody else in that path holds cannot participate in a cycle, and this + // transaction takes no lock a comment writer could be waiting on. + if err := s.acquireWorkspaceSeqLock(tx, workspaceID); err != nil { + return err + } + // ORDER BY id (BUG-2778 class sweep): this gathers rows and then UPDATEs // them one by one inside a transaction, so the scan's order IS the lock // order. Two concurrent remaps in one workspace would otherwise be free to @@ -1518,26 +1537,27 @@ func (s *Store) RemapAttachmentReferencesInWorkspace(workspaceID string, oldToNe // where the second stage locks the renamed row itself and ordering the // cascade therefore cannot help, ordering IS sufficient for the cycle // between these per-row updates. - rows, err := tx.Query(s.q(`SELECT id, content, fields FROM items WHERE workspace_id = ? AND deleted_at IS NULL ORDER BY id`), workspaceID) + rows, err := tx.Query(s.q(`SELECT id, collection_id, content, fields FROM items WHERE workspace_id = ? AND deleted_at IS NULL ORDER BY id`), workspaceID) if err != nil { return fmt.Errorf("scan items for remap: %w", err) } type rowUpdate struct { - id string - content string - fields string + id string + collectionID string + content string + fields string } var updates []rowUpdate for rows.Next() { - var id, content, fields string - if err := rows.Scan(&id, &content, &fields); err != nil { + var id, collectionID, content, fields string + if err := rows.Scan(&id, &collectionID, &content, &fields); err != nil { rows.Close() return fmt.Errorf("scan item: %w", err) } newContent := remapAttachmentRefs(content, oldToNew) newFields := remapAttachmentRefs(fields, oldToNew) if newContent != content || newFields != fields { - updates = append(updates, rowUpdate{id: id, content: newContent, fields: newFields}) + updates = append(updates, rowUpdate{id: id, collectionID: collectionID, content: newContent, fields: newFields}) } } if err := rows.Err(); err != nil { @@ -1620,6 +1640,15 @@ func (s *Store) RemapAttachmentReferencesInWorkspace(workspaceID string, oldToNe u.content, u.fields, u.id); err != nil { return fmt.Errorf("update item %s: %w", u.id, err) } + // This rewrite maps `pad-attachment:` ids and cannot change a relation + // VALUE — but it does write the blob, and the hook is a pure function + // of (blob, schema), so calling it is a no-op while OMITTING it would + // be a standing claim about what remapAttachmentRefs can touch. The + // collection id rides along on the scan above rather than costing a + // lookup per item (PLAN-2857 U5). + if _, err := s.replaceRelationLinks(tx, u.id, workspaceID, u.collectionID, u.fields); err != nil { + return fmt.Errorf("index relation links for item %s: %w", u.id, err) + } } for _, u := range commentUpdates { if _, err := tx.Exec(s.q(`UPDATE comments SET body = ? WHERE id = ?`), diff --git a/internal/store/collections.go b/internal/store/collections.go index b7a39af41..e47684595 100644 --- a/internal/store/collections.go +++ b/internal/store/collections.go @@ -590,7 +590,26 @@ func (s *Store) UpdateCollection(id string, input models.CollectionUpdate) (*mod // without inventing a second ordering rule to keep in sync with the first, // and it is taken BEFORE the row lock, preserving the order this comment // exists to protect. - if len(input.Migrations) > 0 || renaming { + // The relation reverse index is rebuilt below whenever the SCHEMA moves, + // and that rebuild must not interleave with item writes: an item + // transaction that read the OLD schema can commit its own relation hook + // after the reindex has run, leaving the index matching neither the old + // shape nor the new one (codex round 2). So the same workspace lock the + // migrations take is taken for a schema change too. + // + // Gated rather than unconditional, and that is a cost decision as much as + // a correctness one: an icon or description edit changes nothing the index + // depends on, and reindexing a 10k-item collection for it would put ~1.8s + // on an update that has no business paying it. + // CHANGE-SENSITIVE, not merely "a schema pointer was supplied". A client + // that round-trips the collection and submits identical schema bytes would + // otherwise pay the whole-collection reindex for a no-op, which is the + // same cost the icon-edit gate above exists to avoid — and my first + // version of this line claimed to test movement while only testing + // presence (codex round 3). + schemaMoved := input.Schema != nil && *input.Schema != existing.Schema + reindexRelations := schemaMoved || len(input.Migrations) > 0 + if len(input.Migrations) > 0 || renaming || reindexRelations { if err := s.acquireWorkspaceSeqLock(tx, existing.WorkspaceID); err != nil { return nil, err } @@ -695,6 +714,21 @@ func (s *Store) UpdateCollection(id string, input models.CollectionUpdate) (*mod } } + // Rebuild this collection's relation reverse index, LAST in the tx so it + // sees both the new schema and any field-value migration above + // (PLAN-2857 U5, lead ruling). A schema change can move which keys ARE + // relations without touching a single item, so nothing else in this + // transaction would repair the index — and the next per-item write never + // comes for items nobody edits. + // + // Only when the schema or the field VALUES actually moved — see the + // workspace-lock note above for why this is gated. + if reindexRelations { + if err := s.ReindexCollectionRelationLinks(tx, id, existing.WorkspaceID); err != nil { + return nil, fmt.Errorf("reindex relation links: %w", err) + } + } + if err := tx.Commit(); err != nil { return nil, fmt.Errorf("commit collection update: %w", err) } @@ -828,6 +862,15 @@ func (s *Store) MigrateItemFieldValues(collectionID string, migrations []models. return totalAffected, err } + // This function has no non-test caller today, and the hook is here anyway + // (PLAN-2857 U5). It rewrites field VALUES in bulk, so wiring it to a door + // later without this line would stale the relation index for a whole + // collection — a trap that costs one line to remove now and a debugging + // session to find later. + if err := s.ReindexCollectionRelationLinks(tx, collectionID, workspaceID); err != nil { + return totalAffected, err + } + if err := tx.Commit(); err != nil { return totalAffected, err } diff --git a/internal/store/export.go b/internal/store/export.go index cb53e7c4f..2fae4cfa8 100644 --- a/internal/store/export.go +++ b/internal/store/export.go @@ -857,6 +857,22 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow if err != nil { return nil, fmt.Errorf("remap item %s: %w", it.Title, err) } + + // Index the relation edges (PLAN-2857 U5). HERE and not at the + // first-pass INSERT: this pass writes the FINAL blob, with + // source-workspace ids already remapped to their new ones, and it + // visits every inserted item (the guard above skips only items that + // were never inserted). Indexing at the insert would index ids + // belonging to the exporting workspace. + // + // A value the bundle carries that is not an id — a title, say — is + // indexed as written and simply matches no target. That row is inert + // for the reverse query, which is the same disposition migration 088 + // gives every dangling reference. BUG-3014 tracks the separate + // question of how such a value should READ. + if _, err := s.replaceRelationLinks(tx, newItemID, ws.ID, collMap[it.CollectionID], fields); err != nil { + return nil, fmt.Errorf("index relation links for item %s: %w", it.Title, err) + } } // Import comments diff --git a/internal/store/items.go b/internal/store/items.go index 69df15f82..5de1efefa 100644 --- a/internal/store/items.go +++ b/internal/store/items.go @@ -378,6 +378,15 @@ func (s *Store) insertItemTx(tx *sql.Tx, id, workspaceID, collectionID, slug, ts return fmt.Errorf("index wiki links: %w", err) } + // Index this item's `relation` field values, in the same transaction and + // for the same reason as the line above: partial state never lands, and a + // rolled-back write rolls back its index rows. This site covers create AND + // the cross-workspace copy, which reaches it through createItemTxWithID. + // PLAN-2857 U5 / TASK-2997. + if _, err := s.replaceRelationLinks(tx, id, workspaceID, collectionID, fields); err != nil { + return fmt.Errorf("index relation links: %w", err) + } + // Phase 2a (TASK-1595): flip any pre-existing broken `[[Title]]` // rows that have been waiting for an item with this title to // arrive. Cheap when no broken rows match (the common case). @@ -2780,6 +2789,24 @@ func (s *Store) updateItemWithParentLinkOnce( } } + // Re-index `relation` values when the blob was written (PLAN-2857 U5). + // + // Read the blob BACK from the row rather than indexing *input.Fields. An + // update's field write is a server-side field-level MERGE (IDEA-1480), so + // what was submitted is not what was stored — indexing the submission + // would drop the edges of every key the caller did not mention. Reading + // the row makes the index match stored truth by construction, which is the + // only version of this that stays true as the merge rules change. + if input.Fields != nil { + var storedFields string + if err := tx.QueryRow(s.q(`SELECT fields FROM items WHERE id = ?`), id).Scan(&storedFields); err != nil { + return nil, fmt.Errorf("re-read fields for relation index: %w", err) + } + if _, err := s.replaceRelationLinks(tx, id, existing.WorkspaceID, existing.CollectionID, storedFields); err != nil { + return nil, fmt.Errorf("index relation links: %w", err) + } + } + // BUG-2013: apply the parent-link mutation INSIDE this tx, after the // field write. A failure here (cycle detected, DB error) rolls the // whole transaction back, so the caller can never observe the field @@ -3026,6 +3053,21 @@ func (s *Store) restoreItemOnce(id string, opt mutationOptions) (*models.Item, e return nil, sql.ErrNoRows } + // Re-derive the relation index for the item coming back (PLAN-2857 U5). + // + // Restore writes no `fields`, which is why this site does not appear in + // the list of field-writing hooks — and it is a hole exactly because the + // index depends on the item being LIVE as well as on (blob, schema). + // ReindexCollectionRelationLinks deliberately skips soft-deleted items, so + // a schema change that happens WHILE an item is deleted never reaches it: + // the item comes back carrying whatever rows it had before, against a + // schema that has moved. Both directions bite — a field that became a + // relation leaves the restored item missing edges, one that stopped being + // a relation leaves it with edges it should not have (codex round 1). + if _, err := s.replaceRelationLinks(tx, id, existing.WorkspaceID, existing.CollectionID, existing.Fields); err != nil { + return nil, fmt.Errorf("index relation links on restore: %w", err) + } + // item.restored carries the POST-restore snapshot (SPEC-3 v1.1). Read // after the UPDATE, in-tx: the row is live again at this point, so // getItemTx sees it, and the snapshot reflects the state a consumer will @@ -4899,6 +4941,16 @@ func (s *Store) moveItemWithPreCheckOnce( return nil, fmt.Errorf("move item: %w", err) } + // Re-index against the DESTINATION collection (PLAN-2857 U5). A move is + // the one write where the blob and the SCHEMA change together: the + // identical value indexes differently on the other side, because which + // keys are relations is a property of the collection, not of the item. A + // hook that passed the item's old collection here would leave every moved + // item indexed under the shape it just left. + if _, err := s.replaceRelationLinks(tx, itemID, existing.WorkspaceID, targetCollectionID, newFieldsJSON); err != nil { + return nil, fmt.Errorf("index relation links on move: %w", err) + } + // A move can carry a status-changing field override (e.g. // `pad item move ... --field status=done`), which rewrites `fields` // outside the UpdateItemWithPreCheck path. Record the transition here diff --git a/internal/store/migrations/088_item_relation_links.sql b/internal/store/migrations/088_item_relation_links.sql new file mode 100644 index 000000000..47da4040f --- /dev/null +++ b/internal/store/migrations/088_item_relation_links.sql @@ -0,0 +1,73 @@ +-- Migration 088: materialised reverse index for `relation` field values +-- (PLAN-2857 U5 / TASK-2997). +-- +-- A relation field stores the target item's id inside the source item's +-- `fields` JSON blob. Asking the reverse question — "which items point at +-- TASK-5?" — has no answer today short of scanning every blob. +-- +-- WHY MATERIALISED RATHER THAN DERIVED AT READ (ratified Q3). The derived +-- query is an unindexed `json_extract(fields,'$.color') = ?` scan; no index +-- can be added for a USER-DEFINED key without a per-key migration; and on a +-- `multi_relation` array it silently matches nothing rather than failing. +-- That is the same argument migration 061 made for wiki-links, on the same +-- table, against the same alternative. +-- +-- WHY A NEW TABLE rather than extending item_wiki_links: that table's +-- target_kind is CHECK-constrained to three CONTENT-link kinds, and its +-- position / display_text columns are content-shaped. A field-valued edge +-- has no position and no display override — but it does have a source FIELD +-- KEY, which nothing in that schema can carry. +-- +-- Field notes: +-- * source_field_key is the schema key the value sits under. It is what +-- makes this table not item_wiki_links, and it is what lets the reverse +-- side say "referenced by X via `owner`" rather than just "referenced". +-- * target_item_id is NOT FK'd, matching 061's reasoning: a dangling +-- reference is a row worth keeping and rendering honestly on the SOURCE +-- side. Note that such a row is INERT for the reverse query — no +-- target-side lookup can ever find it — so it exists for the source +-- side and a future broken-references report, not for the count. +-- * ordinal is the position within a `multi_relation` array (U4, not yet +-- landed). Single-valued relations are always 0. Present up-front so U4 +-- does not need an ALTER. +-- * workspace_id is denormalised from the source item so the reverse query +-- can scope without a join, exactly as the visibility filter needs it. +-- +-- +-- NUL INVARIANT (migration 084's census). All four TEXT columns here are +-- recorded as unprotected rather than given their own triggers, and the reason +-- is a derivation, not an omission: +-- +-- * target_item_id is extracted from `items.fields`, and source_field_key +-- from `collections.schema`. BOTH of those columns are classJSON-protected +-- already, so a NUL cannot be in the value this table copies. +-- * source_item_id and workspace_id are server-generated ids. +-- +-- The derivation is TRANSACTIONAL, which is the part worth stating: every +-- write hook runs after its own row's INSERT/UPDATE inside the same +-- transaction, so the protecting trigger has already fired and rejected a +-- NUL-bearing blob before this table sees anything derived from it. A hook +-- moved to run BEFORE its row's write would break that, and the census would +-- not notice — it checks columns, not ordering. +-- CASCADE on source_item_id matches every other items-referencing table: an +-- item's OUTGOING edges disappear when it is hard-deleted. A SOFT delete +-- leaves the rows in place and the reverse query filters on the source's +-- deleted_at, so a restore brings the edges back without re-deriving them. + +CREATE TABLE IF NOT EXISTS item_relation_links ( + source_item_id TEXT NOT NULL, + source_field_key TEXT NOT NULL, + target_item_id TEXT NOT NULL, + workspace_id TEXT NOT NULL, + ordinal INTEGER NOT NULL DEFAULT 0, + FOREIGN KEY (source_item_id) REFERENCES items(id) ON DELETE CASCADE +); + +-- The reverse lookup this table exists for. +CREATE INDEX IF NOT EXISTS idx_relation_links_target + ON item_relation_links(target_item_id); + +-- The write hook's own key: every write replaces one (source, field) pair's +-- rows wholesale, so this is the delete's index as well as the forward read's. +CREATE INDEX IF NOT EXISTS idx_relation_links_source_field + ON item_relation_links(source_item_id, source_field_key); diff --git a/internal/store/nul_unprotected_baseline.txt b/internal/store/nul_unprotected_baseline.txt index 3be1529d5..7dc6c4d07 100644 --- a/internal/store/nul_unprotected_baseline.txt +++ b/internal/store/nul_unprotected_baseline.txt @@ -99,6 +99,10 @@ item_links.source_id item_links.target_id item_links.user_id item_links.workspace_id +item_relation_links.source_field_key +item_relation_links.source_item_id +item_relation_links.target_item_id +item_relation_links.workspace_id item_reminders.acked_at item_reminders.created_at item_reminders.fired_at diff --git a/internal/store/pgmigrations/065_item_relation_links.sql b/internal/store/pgmigrations/065_item_relation_links.sql new file mode 100644 index 000000000..b35a428b2 --- /dev/null +++ b/internal/store/pgmigrations/065_item_relation_links.sql @@ -0,0 +1,73 @@ +-- Migration 065 (Postgres; SQLite 088): materialised reverse index for `relation` field values +-- (PLAN-2857 U5 / TASK-2997). +-- +-- A relation field stores the target item's id inside the source item's +-- `fields` JSON blob. Asking the reverse question — "which items point at +-- TASK-5?" — has no answer today short of scanning every blob. +-- +-- WHY MATERIALISED RATHER THAN DERIVED AT READ (ratified Q3). The derived +-- query is an unindexed `json_extract(fields,'$.color') = ?` scan; no index +-- can be added for a USER-DEFINED key without a per-key migration; and on a +-- `multi_relation` array it silently matches nothing rather than failing. +-- That is the same argument migration 061 made for wiki-links, on the same +-- table, against the same alternative. +-- +-- WHY A NEW TABLE rather than extending item_wiki_links: that table's +-- target_kind is CHECK-constrained to three CONTENT-link kinds, and its +-- position / display_text columns are content-shaped. A field-valued edge +-- has no position and no display override — but it does have a source FIELD +-- KEY, which nothing in that schema can carry. +-- +-- Field notes: +-- * source_field_key is the schema key the value sits under. It is what +-- makes this table not item_wiki_links, and it is what lets the reverse +-- side say "referenced by X via `owner`" rather than just "referenced". +-- * target_item_id is NOT FK'd, matching 061's reasoning: a dangling +-- reference is a row worth keeping and rendering honestly on the SOURCE +-- side. Note that such a row is INERT for the reverse query — no +-- target-side lookup can ever find it — so it exists for the source +-- side and a future broken-references report, not for the count. +-- * ordinal is the position within a `multi_relation` array (U4, not yet +-- landed). Single-valued relations are always 0. Present up-front so U4 +-- does not need an ALTER. +-- * workspace_id is denormalised from the source item so the reverse query +-- can scope without a join, exactly as the visibility filter needs it. +-- +-- +-- NUL INVARIANT (migration 084's census). All four TEXT columns here are +-- recorded as unprotected rather than given their own triggers, and the reason +-- is a derivation, not an omission: +-- +-- * target_item_id is extracted from `items.fields`, and source_field_key +-- from `collections.schema`. BOTH of those columns are classJSON-protected +-- already, so a NUL cannot be in the value this table copies. +-- * source_item_id and workspace_id are server-generated ids. +-- +-- The derivation is TRANSACTIONAL, which is the part worth stating: every +-- write hook runs after its own row's INSERT/UPDATE inside the same +-- transaction, so the protecting trigger has already fired and rejected a +-- NUL-bearing blob before this table sees anything derived from it. A hook +-- moved to run BEFORE its row's write would break that, and the census would +-- not notice — it checks columns, not ordering. +-- CASCADE on source_item_id matches every other items-referencing table: an +-- item's OUTGOING edges disappear when it is hard-deleted. A SOFT delete +-- leaves the rows in place and the reverse query filters on the source's +-- deleted_at, so a restore brings the edges back without re-deriving them. + +CREATE TABLE IF NOT EXISTS item_relation_links ( + source_item_id TEXT NOT NULL, + source_field_key TEXT NOT NULL, + target_item_id TEXT NOT NULL, + workspace_id TEXT NOT NULL, + ordinal INTEGER NOT NULL DEFAULT 0, + FOREIGN KEY (source_item_id) REFERENCES items(id) ON DELETE CASCADE +); + +-- The reverse lookup this table exists for. +CREATE INDEX IF NOT EXISTS idx_relation_links_target + ON item_relation_links(target_item_id); + +-- The write hook's own key: every write replaces one (source, field) pair's +-- rows wholesale, so this is the delete's index as well as the forward read's. +CREATE INDEX IF NOT EXISTS idx_relation_links_source_field + ON item_relation_links(source_item_id, source_field_key); diff --git a/internal/store/relation_links.go b/internal/store/relation_links.go new file mode 100644 index 000000000..99a29b99e --- /dev/null +++ b/internal/store/relation_links.go @@ -0,0 +1,441 @@ +package store + +import ( + "database/sql" + "encoding/json" + "fmt" + "strings" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// The materialised reverse index for `relation` field values +// (PLAN-2857 U5 / TASK-2997). See migration 088 for why it is materialised +// and why it is its own table. +// +// THE INDEX IS A FUNCTION OF (BLOB, SCHEMA), and both halves matter. Which +// keys are relations is a property of the COLLECTION, so the same blob +// indexes differently after a move, and a schema change can invalidate every +// item in a collection with no item write at all. That second case is handled +// by ReindexCollectionRelationLinks, called inside the schema-update +// transaction (lead ruling on the U5 contract) — "it fixes itself on the next +// write" is false for items nobody edits, which is the same argument that +// made this index materialised rather than derived. +// +// THIS FUNCTION DOES NOT RESOLVE ANYTHING. Since U6 a relation value may be +// SUPPLIED as a title, but the resolver canonicalises it to an id in place +// before any store write, so the blob reaching here already holds ids. Doing +// its own lookup would be a second, divergent resolver — and would index a +// title on any path where the resolver had not run. + +// replaceRelationLinks rebuilds the reverse-index rows for one source item. +// +// Must run inside `tx` — the caller owns transactionality — and is called at +// every store site that writes an item's `fields` blob, not at the doors. A +// door-level hook is a per-door wiring claim and the door population grows; a +// store-level one cannot be bypassed by a door that does not exist yet. That +// is also the shape replaceWikiLinks established for the content index. +// +// Idempotent: the same (item, blob, schema) twice yields the same row set. +// Calling it on a write that did not touch a relation value is a deliberate +// no-op rather than an omission, so no caller has to reason about which +// writes "can" matter. +// Returns the number of edge rows written, so a caller reporting progress +// counts what LANDED rather than re-deriving a prediction from a schema it read +// earlier — those two disagree the moment a schema changes underneath. +func (s *Store) replaceRelationLinks(tx *sql.Tx, sourceItemID, workspaceID, collectionID, fieldsJSON string) (int, error) { + // Delete first so callers never pre-clear, and so a blob that has lost + // its relation values correctly ends with zero rows. + if _, err := tx.Exec(s.q(`DELETE FROM item_relation_links WHERE source_item_id = ?`), sourceItemID); err != nil { + return 0, fmt.Errorf("delete prior relation links: %w", err) + } + + schema, err := s.relationSchemaForCollectionTx(tx, collectionID) + if err != nil { + return 0, err + } + if len(schema) == 0 { + // The collection declares no relation fields, so there is nothing to + // index. Not an error, and the delete above still ran — which is what + // makes retyping a field away from `relation` self-healing on the + // next write of each item. + return 0, nil + } + + values := relationValuesFromBlob(fieldsJSON, schema) + if len(values) == 0 { + return 0, nil + } + + for _, v := range values { + if _, err := tx.Exec(s.q(` + INSERT INTO item_relation_links + (source_item_id, source_field_key, target_item_id, workspace_id, ordinal) + VALUES (?, ?, ?, ?, ?) + `), sourceItemID, v.fieldKey, v.targetID, workspaceID, v.ordinal); err != nil { + return 0, fmt.Errorf("insert relation link %s/%s: %w", v.fieldKey, v.targetID, err) + } + } + return len(values), nil +} + +// relationLinkRow is one edge about to be written. +type relationLinkRow struct { + fieldKey string + targetID string + ordinal int +} + +// relationValuesFromBlob extracts the relation edges a fields blob carries, +// given the set of keys the collection declares as relations. +// +// A blob that will not parse yields NO edges rather than an error: a corrupt +// blob is a different defect that other code reports, and refusing the write +// here would make this index able to block an item's save. +func relationValuesFromBlob(fieldsJSON string, relationKeys map[string]struct{}) []relationLinkRow { + trimmed := strings.TrimSpace(fieldsJSON) + if trimmed == "" || trimmed == "{}" || trimmed == "null" { + return nil + } + var blob map[string]any + if err := json.Unmarshal([]byte(trimmed), &blob); err != nil { + return nil + } + var out []relationLinkRow + for key := range relationKeys { + raw, present := blob[key] + if !present || raw == nil { + continue + } + switch v := raw.(type) { + case string: + if id := strings.TrimSpace(v); id != "" { + out = append(out, relationLinkRow{fieldKey: key, targetID: id}) + } + case []any: + // `multi_relation` (U4, not yet landed). Indexed now so the table + // does not need a second pass when that type arrives, and so a + // hand-written array value is not silently unindexed today. + for i, entry := range v { + id, isStr := entry.(string) + if !isStr { + continue + } + if id = strings.TrimSpace(id); id != "" { + out = append(out, relationLinkRow{fieldKey: key, targetID: id, ordinal: i}) + } + } + } + } + return out +} + +// relationSchemaForCollectionTx returns the set of field keys the collection +// declares as `relation` (or `multi_relation`). +// +// Reads through the transaction on purpose: a schema change and the reindex it +// triggers happen in ONE transaction, so a pool read here would see the +// pre-change schema and rebuild the index to match the shape being replaced. +func (s *Store) relationSchemaForCollectionTx(tx *sql.Tx, collectionID string) (map[string]struct{}, error) { + var schemaJSON sql.NullString + err := tx.QueryRow(s.q(`SELECT schema FROM collections WHERE id = ?`), collectionID).Scan(&schemaJSON) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("read collection schema for relation index: %w", err) + } + if !schemaJSON.Valid || strings.TrimSpace(schemaJSON.String) == "" { + return nil, nil + } + return relationKeysFromSchemaJSON(schemaJSON.String), nil +} + +// relationKeysFromSchemaJSON returns the field keys a collection schema +// declares as `relation` (or `multi_relation`). +// +// ONE function so the write hook and the backfill cannot disagree about what +// counts as a relation field — two copies of this rule would drift the moment +// a third relation-ish type appears, and the symptom would be an index that is +// correct at write time and wrong after a rebuild. +// +// A schema that will not parse yields no keys rather than an error: that is a +// different defect, reported elsewhere, and the index declines to be the +// second voice. +func relationKeysFromSchemaJSON(schemaJSON string) map[string]struct{} { + if strings.TrimSpace(schemaJSON) == "" { + return nil + } + var schema models.CollectionSchema + if err := json.Unmarshal([]byte(schemaJSON), &schema); err != nil { + return nil + } + keys := map[string]struct{}{} + for _, def := range schema.Fields { + if def.Type == "relation" || def.Type == "multi_relation" { + keys[def.Key] = struct{}{} + } + } + if len(keys) == 0 { + return nil + } + return keys +} + +// ReindexCollectionRelationLinks rebuilds the reverse index for every live +// item in one collection. +// +// WHY THIS EXISTS (lead ruling on the U5 contract). The index is a function of +// (blob, schema), and a SCHEMA change moves the second half without touching a +// single item. Declare a `relation` field on a collection that already holds +// values under that key, or retype one away, and every existing item is +// mis-indexed. "It fixes itself on the next write of each item" is false for +// items nobody edits — which is the same argument that made this index +// materialised rather than derived at read time, so accepting it here would +// undo the decision. +// +// Runs INSIDE the schema-update transaction, so the property is: after a +// committed schema change, no item in the collection is mis-indexed. A failure +// rolls the schema change back with it rather than committing a schema the +// index does not match. +// +// Bounded by the collection's live item count. The cost at 10k items is +// recorded on TASK-2997's trail, because "a rare owner-only operation" is a +// claim about frequency, not about cost, and the two are worth separating. +func (s *Store) ReindexCollectionRelationLinks(tx *sql.Tx, collectionID, workspaceID string) error { + // Ids first, then per-item work: holding a SELECT cursor open while + // writing upsets some drivers, the same reason the wiki backfill collects + // before it writes. + rows, err := tx.Query(s.q(` + SELECT id, fields FROM items + WHERE collection_id = ? AND workspace_id = ? AND deleted_at IS NULL + `), collectionID, workspaceID) + if err != nil { + return fmt.Errorf("scan collection for relation reindex: %w", err) + } + type row struct{ id, fields string } + var items []row + for rows.Next() { + var r row + if scanErr := rows.Scan(&r.id, &r.fields); scanErr != nil { + rows.Close() + return fmt.Errorf("scan relation reindex row: %w", scanErr) + } + items = append(items, r) + } + if rowsErr := rows.Err(); rowsErr != nil { + rows.Close() + return fmt.Errorf("iterate relation reindex rows: %w", rowsErr) + } + rows.Close() + + for _, r := range items { + if _, err := s.replaceRelationLinks(tx, r.id, workspaceID, collectionID, r.fields); err != nil { + return err + } + } + return nil +} + +// RelationBacklink is one item that points at the target through a relation +// field, plus the field it points through. +// +// FieldKey is the part that makes this not a wiki backlink: the reverse side +// can say "referenced by CAR-3 via `color`" rather than just "referenced by", +// which is the whole reason this index is its own table. +type RelationBacklink struct { + SourceItemID string `json:"source_item_id"` + SourceRef string `json:"source_ref"` + SourceTitle string `json:"source_title"` + CollectionSlug string `json:"collection_slug"` + FieldKey string `json:"field_key"` + FieldLabel string `json:"field_label,omitempty"` +} + +// relationBacklinkCap bounds any single page. Matches GetBacklinks' cap for +// the same reason: a caller that asks for everything should not be able to. +const relationBacklinkCap = 300 + +// GetRelationBacklinks returns the live items that reference targetItemID +// through a `relation` field, filtered to what this viewer may see. +// +// VISIBILITY IS THE VIEWER'S, NOT THE TRUTH (ratified, day 55). `vis` is built +// from ResolveBacklinksVisibility — the same resolver the wiki-link reverse +// side uses, reused verbatim rather than reinvented — and the consequence is +// on the record deliberately: "Referenced by 3" means "by 3 you can see". A +// true count would leak the existence of items the viewer has no access to, +// which is the exact leak that resolver exists to close. +// +// Filtering happens in SQL, not after the fetch, so LIMIT counts VISIBLE rows. +// Filtering above the limit would let invisible rows consume page slots and +// silently shrink pages — and worse, a page that came back short would itself +// be a signal about how many hidden rows there are. +// +// Soft-deleted SOURCES are excluded by the join, so a deleted item stops +// referencing and a restore brings it back without re-deriving anything. A +// soft-deleted TARGET keeps its rows: the question "who pointed at this?" +// still has an answer after deletion. +func (s *Store) GetRelationBacklinks(targetItemID, workspaceID string, limit, offset int, vis BacklinksVisibility) ([]RelationBacklink, error) { + if limit <= 0 || limit > relationBacklinkCap { + limit = 50 + } + if offset < 0 { + offset = 0 + } + // Restricted with nothing visible → empty without a query. Postgres + // rejects the `IN ()` form this would otherwise build. + if !vis.Unrestricted && len(vis.FullCollectionIDs) == 0 && len(vis.GrantedItemIDs) == 0 { + return nil, nil + } + + args := []any{targetItemID, workspaceID} + visClause := "" + if !vis.Unrestricted { + collClause := "FALSE" + if len(vis.FullCollectionIDs) > 0 { + ph := make([]string, len(vis.FullCollectionIDs)) + for i, cid := range vis.FullCollectionIDs { + ph[i] = "?" + args = append(args, cid) + } + collClause = "s.collection_id IN (" + strings.Join(ph, ",") + ")" + } + itemClause := "FALSE" + if len(vis.GrantedItemIDs) > 0 { + ph := make([]string, len(vis.GrantedItemIDs)) + for i, iid := range vis.GrantedItemIDs { + ph[i] = "?" + args = append(args, iid) + } + itemClause = "s.id IN (" + strings.Join(ph, ",") + ")" + } + visClause = " AND (" + collClause + " OR " + itemClause + ")" + } + args = append(args, limit, offset) + + rows, err := s.db.Query(s.q(` + SELECT rl.source_item_id, rl.source_field_key, + s.title, s.item_number, s.collection_id, c.prefix, c.slug, c.schema + FROM item_relation_links rl + JOIN items s ON s.id = rl.source_item_id + JOIN collections c ON c.id = s.collection_id + WHERE rl.target_item_id = ? AND rl.workspace_id = ? + AND s.deleted_at IS NULL + -- A soft-deleted COLLECTION leaves its items live, so without this a + -- source in a collection the viewer can no longer open still shows + -- up here — including for an Unrestricted viewer, who has no + -- collection filter to catch it (codex round 2). + AND c.deleted_at IS NULL + AND s.id != rl.target_item_id`+visClause+` + ORDER BY s.updated_at DESC, rl.source_item_id, rl.source_field_key + LIMIT ? OFFSET ? + `), args...) + if err != nil { + return nil, fmt.Errorf("query relation backlinks: %w", err) + } + defer rows.Close() + + // Field LABELS come from the source collection's schema, which the query + // above already carries on every row — parsed once per distinct collection + // here rather than re-queried. + // + // The first version issued a SELECT per distinct collection WHILE THE ROWS + // CURSOR WAS STILL OPEN. That needs a SECOND pool connection, and when the + // pool is saturated the second connection never arrives — an application + // deadlock no SQLSTATE names, and the same class BUG-2409 closed for the + // collection writers (codex round 7). It was also an N+1. Selecting + // `c.schema` in the one query removes both. + labels := map[string]map[string]string{} + labelFor := func(collectionID, schemaJSON, key string) string { + byKey, known := labels[collectionID] + if !known { + byKey = map[string]string{} + var schema models.CollectionSchema + if json.Unmarshal([]byte(schemaJSON), &schema) == nil { + for _, def := range schema.Fields { + if def.Label != "" { + byKey[def.Key] = def.Label + } + } + } + labels[collectionID] = byKey + } + return byKey[key] + } + + var out []RelationBacklink + for rows.Next() { + var b RelationBacklink + // item_number is NULLABLE on legacy rows — scanning it into an int + // errors and would take the whole list down, the same defect U6 shipped + // in hydration (codex round 7 there). + var number sql.NullInt64 + var schemaJSON sql.NullString + var prefix, collectionID string + if err := rows.Scan(&b.SourceItemID, &b.FieldKey, &b.SourceTitle, &number, &collectionID, &prefix, &b.CollectionSlug, &schemaJSON); err != nil { + return nil, fmt.Errorf("scan relation backlink: %w", err) + } + if number.Valid { + b.SourceRef = fmt.Sprintf("%s-%d", prefix, number.Int64) + } + b.FieldLabel = labelFor(collectionID, schemaJSON.String, b.FieldKey) + out = append(out, b) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate relation backlinks: %w", err) + } + return out, nil +} + +// CountRelationBacklinks is GetRelationBacklinks' count, under the SAME +// visibility and the SAME filters. +// +// It exists as its own query rather than len() of the page because the UI +// wants "Referenced by N" without fetching N rows. The filters must stay in +// lockstep with GetRelationBacklinks — a drift would let the header promise a +// number the list cannot produce, which is exactly the failure CountBacklinks +// documents for the wiki side. +func (s *Store) CountRelationBacklinks(targetItemID, workspaceID string, vis BacklinksVisibility) (int, error) { + if !vis.Unrestricted && len(vis.FullCollectionIDs) == 0 && len(vis.GrantedItemIDs) == 0 { + return 0, nil + } + args := []any{targetItemID, workspaceID} + visClause := "" + if !vis.Unrestricted { + collClause := "FALSE" + if len(vis.FullCollectionIDs) > 0 { + ph := make([]string, len(vis.FullCollectionIDs)) + for i, cid := range vis.FullCollectionIDs { + ph[i] = "?" + args = append(args, cid) + } + collClause = "s.collection_id IN (" + strings.Join(ph, ",") + ")" + } + itemClause := "FALSE" + if len(vis.GrantedItemIDs) > 0 { + ph := make([]string, len(vis.GrantedItemIDs)) + for i, iid := range vis.GrantedItemIDs { + ph[i] = "?" + args = append(args, iid) + } + itemClause = "s.id IN (" + strings.Join(ph, ",") + ")" + } + visClause = " AND (" + collClause + " OR " + itemClause + ")" + } + var n int + err := s.db.QueryRow(s.q(` + SELECT COUNT(*) + FROM item_relation_links rl + JOIN items s ON s.id = rl.source_item_id + JOIN collections c ON c.id = s.collection_id + WHERE rl.target_item_id = ? AND rl.workspace_id = ? + AND s.deleted_at IS NULL + -- Same filter as the page query. The two must stay identical or the + -- header promises a number the list cannot produce. + AND c.deleted_at IS NULL + AND s.id != rl.target_item_id`+visClause), args...).Scan(&n) + if err != nil { + return 0, fmt.Errorf("count relation backlinks: %w", err) + } + return n, nil +} diff --git a/internal/store/relation_links_backfill.go b/internal/store/relation_links_backfill.go new file mode 100644 index 000000000..a908159aa --- /dev/null +++ b/internal/store/relation_links_backfill.go @@ -0,0 +1,290 @@ +package store + +import ( + "database/sql" + "errors" + "fmt" +) + +// BackfillRelationLinksResult reports what the backfill did so cmd/pad can log +// a one-line summary at startup. +type BackfillRelationLinksResult struct { + // Skipped is true when the backfill decided it had nothing to do without + // scanning items — see BackfillRelationLinks for the two ways that + // happens. + Skipped bool + + // ItemsScanned counts the live items the backfill walked. + ItemsScanned int + + // LinksInserted is the number of index rows written. + LinksInserted int +} + +// BackfillRelationLinks derives `item_relation_links` for every live item. +// Called from server startup after migrations. +// +// ITS GUARD IS A COMPLETION MARKER, and the two shapes it is not are both +// instructive. +// +// It is not the wiki backfill's per-item EXISTS. That test does not work here: +// most items carry no relation value, so "zero rows" is their correct steady +// state and is indistinguishable from "never indexed" — a per-item EXISTS +// would re-derive nearly every item on every boot, forever. +// +// It is also no longer "does the table have any row", which is what I wrote +// first and which codex round 1 correctly called a P1. Rows commit per item, +// so a crash partway through leaves the table NON-EMPTY AND INCOMPLETE — and +// that guard then skips forever, with the missing edges never derived. The +// "rebuild by deleting the table" story only ever covered deliberate deletion, +// not an interrupted run. +// +// So completion is recorded EXPLICITLY, in platform_settings, and only AFTER a +// full pass finishes. A crash before that leaves no marker and the next boot +// re-derives — which is safe because replaceRelationLinks deletes before it +// inserts, so a repeat pass is idempotent rather than additive. The marker is +// written after the work for the same reason a dedupe token must be: writing +// it first turns a mid-run failure into a permanent silent loss. +// +// A REBUILD is still one line, just a different one: delete the marker (or the +// table and the marker) and restart. +// +// The schema probe stays as a cheap second skip — a database where no +// collection declares a relation field has nothing to derive — and it records +// completion too, because a pass over zero work is a completed pass. A +// relation field arriving later is handled by UpdateCollection's reindex and +// the write hooks, not by this. +// +// TWO SERVERS BOOTING AT ONCE is redundant, not corrupting, and the argument +// is worth writing down because the obvious fix — a startup lock — has worse +// failure modes than the thing it prevents (a crashed holder blocking boot). +// +// Both servers see no marker and both walk items. Each per-item write re-reads +// the blob inside its own transaction and replaces that item's rows wholesale, +// so two passes over one item converge on the same rows rather than doubling +// them. Whichever server finishes first writes the marker, and the marker is +// honest at that moment: a full pass DID complete. The other server's +// remaining writes re-derive rows that are already correct. +// +// The interleaving that WOULD corrupt — an older pass writing a stale blob +// over a newer one — is closed by the in-transaction re-read, not by the +// marker. That is the load-bearing part; the marker only decides whether a +// pass runs at all. +// +// PLAN-2857 U5 / TASK-2997. +func (s *Store) BackfillRelationLinks() (*BackfillRelationLinksResult, error) { + result := &BackfillRelationLinksResult{} + + var marker string + err := s.db.QueryRow(s.q(`SELECT value FROM platform_settings WHERE key = ?`), relationLinksBackfilledFlag).Scan(&marker) + if err == nil && marker == "1" { + result.Skipped = true + return result, nil + } + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("probe relation backfill marker: %w", err) + } + + // Which collections declare a relation field, and what their relation keys + // are. One pass, so the per-item work below is a map lookup rather than a + // schema parse. + relationKeysByCollection, err := s.collectionsWithRelationFields() + if err != nil { + return nil, err + } + if len(relationKeysByCollection) == 0 { + // A pass over zero work is a completed pass. + if err := s.markRelationLinksBackfilled(); err != nil { + return nil, err + } + result.Skipped = true + return result, nil + } + + // IDS ONLY. The scan used to carry every item's `fields` blob, which put + // the whole database's field JSON in memory before a single row was + // written (codex round 6). It was never needed: each write re-reads the + // blob under a lock, precisely because a snapshot written later is a + // snapshot that can be stale. Three ids per row is ~100 bytes, so 100k + // items is ~10MB rather than the sum of every blob. + rows, err := s.db.Query(s.q(` + SELECT id, workspace_id, collection_id + FROM items + WHERE deleted_at IS NULL + `)) + if err != nil { + return nil, fmt.Errorf("scan items for relation backfill: %w", err) + } + var items []itemRow + for rows.Next() { + var r itemRow + if scanErr := rows.Scan(&r.id, &r.workspaceID, &r.collectionID); scanErr != nil { + rows.Close() + return nil, fmt.Errorf("scan relation backfill row: %w", scanErr) + } + items = append(items, r) + } + if rowsErr := rows.Err(); rowsErr != nil { + rows.Close() + return nil, fmt.Errorf("iterate relation backfill rows: %w", rowsErr) + } + rows.Close() + + // BATCHED BY WORKSPACE, in bounded chunks. One transaction per ITEM was + // the first shape and codex round 5 was right to call it out: a 100k-item + // database meant 100k transactions and 100k advisory-lock acquisitions in + // a synchronous startup pause, which is connection churn nobody asked for. + // + // The other extreme — one transaction per workspace — is worse in a + // different way: at the measured ~1.8s per 10k items, a 100k-item + // workspace would hold its write lock for ~18 seconds at boot. Bounded + // chunks keep both the transaction count and the lock hold time small, and + // the per-chunk failure granularity is what the per-item version was + // really buying. + byWorkspace := map[string][]itemRow{} + for _, it := range items { + if _, ok := relationKeysByCollection[it.collectionID]; !ok { + continue + } + byWorkspace[it.workspaceID] = append(byWorkspace[it.workspaceID], it) + } + for workspaceID, wsItems := range byWorkspace { + for start := 0; start < len(wsItems); start += relationBackfillChunk { + end := start + relationBackfillChunk + if end > len(wsItems) { + end = len(wsItems) + } + inserted, err := s.backfillRelationChunk(workspaceID, wsItems[start:end]) + if err != nil { + return nil, err + } + result.ItemsScanned += end - start + result.LinksInserted += inserted + } + } + + // ONLY NOW. Every earlier return path is a failure, and none of them + // records completion — so an interrupted run is re-derived on the next + // boot rather than skipped forever. + if err := s.markRelationLinksBackfilled(); err != nil { + return nil, err + } + return result, nil +} + +// relationLinksBackfilledFlag is the platform_settings key recording that a +// FULL relation-index derivation has completed. Same mechanism the +// webhook-secret encryption migration uses for the same question. +const relationLinksBackfilledFlag = "relation_links_backfilled" + +// markRelationLinksBackfilled records a completed pass. +func (s *Store) markRelationLinksBackfilled() error { + if _, err := s.db.Exec(s.q(` + INSERT INTO platform_settings (key, value, updated_at) VALUES (?, ?, ?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value, updated_at = excluded.updated_at + `), relationLinksBackfilledFlag, "1", now()); err != nil { + return fmt.Errorf("persist relation backfill marker: %w", err) + } + return nil +} + +// collectionsWithRelationFields maps collection id -> its relation field keys, +// for every collection that declares at least one. +func (s *Store) collectionsWithRelationFields() (map[string]map[string]struct{}, error) { + rows, err := s.db.Query(s.q(`SELECT id, schema FROM collections`)) + if err != nil { + return nil, fmt.Errorf("scan collections for relation keys: %w", err) + } + defer rows.Close() + out := map[string]map[string]struct{}{} + for rows.Next() { + var id, schemaJSON string + if err := rows.Scan(&id, &schemaJSON); err != nil { + return nil, fmt.Errorf("scan collection schema: %w", err) + } + keys := relationKeysFromSchemaJSON(schemaJSON) + if len(keys) > 0 { + out[id] = keys + } + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate collection schemas: %w", err) + } + return out, nil +} + +// itemRow is one row of the backfill's initial scan — identity only. The blob +// is deliberately absent: every write re-reads it under a lock, so carrying it +// here would cost memory proportional to the database's total field JSON and +// buy a value that must be discarded anyway. +type itemRow struct{ id, workspaceID, collectionID string } + +// relationBackfillChunk bounds one backfill transaction. Small enough that the +// workspace lock is never held long at boot, large enough that a big database +// does not pay a transaction per item. +const relationBackfillChunk = 500 + +// backfillRelationChunk indexes one bounded batch of items from ONE workspace, +// in a single transaction holding that workspace's lock. +func (s *Store) backfillRelationChunk(workspaceID string, batch []itemRow) (int, error) { + tx, err := s.db.Begin() + if err != nil { + return 0, fmt.Errorf("begin relation backfill tx: %w", err) + } + // Belt and braces on top of the explicit rollbacks: a driver can return a + // RECOVERABLE error from Commit, which no explicit path covers. A rollback + // after a successful commit is a no-op. + defer tx.Rollback() //nolint:errcheck // no-op after commit + + // WORKSPACE LOCK FIRST, then each item row — the order every item write and + // UpdateCollection use, so this cannot invert against them. + // + // The row lock alone serialises against concurrent ITEM writes but not + // against a concurrent SCHEMA change, which is guarded a level up by this + // lock: without it the backfill can read the old schema, watch + // UpdateCollection reindex the collection, then write rows derived from the + // shape it just replaced and commit last (codex rounds 3 and 4). + if err := s.acquireWorkspaceSeqLock(tx, workspaceID); err != nil { + return 0, err + } + + inserted := 0 + for _, it := range batch { + // LOCKED, not just re-read. The re-read closes the gap between the + // SCAN and this transaction; the lock closes the gap between this read + // and the replacement below. SQLite's write lock already serialises + // and it does not take the clause here, hence the dialect gate — the + // same shape UpdateCollection uses for its own re-read. + lockedRead := `SELECT fields, collection_id FROM items WHERE id = ? AND deleted_at IS NULL` + if s.dialect.Driver() == DriverPostgres { + lockedRead += " FOR UPDATE" + } + var fields, collectionID string + if err := tx.QueryRow(s.q(lockedRead), it.id).Scan(&fields, &collectionID); err != nil { + if errors.Is(err, sql.ErrNoRows) { + // Deleted between the scan and now. Not a match any more. + continue + } + return 0, fmt.Errorf("re-read item %s for backfill: %w", it.id, err) + } + + // CALLED UNCONDITIONALLY, even when the blob carries no edges. Skipping + // the zero-edge case was an optimisation that preserved exactly the + // stale rows this pass exists to clear — replaceRelationLinks deletes + // before it inserts, so that case is the one that must run. + // The COUNT comes from the write itself. Deriving it from the + // schema map captured before the pass would disagree with reality + // whenever a schema changed underneath — reporting a number of rows + // that were never written (codex round 6). + n, err := s.replaceRelationLinks(tx, it.id, workspaceID, collectionID, fields) + if err != nil { + return 0, err + } + inserted += n + } + + if err := tx.Commit(); err != nil { + return 0, fmt.Errorf("commit relation backfill tx: %w", err) + } + return inserted, nil +} diff --git a/internal/store/relation_links_cost_test.go b/internal/store/relation_links_cost_test.go new file mode 100644 index 000000000..ee1424702 --- /dev/null +++ b/internal/store/relation_links_cost_test.go @@ -0,0 +1,94 @@ +package store + +import ( + "encoding/json" + "fmt" + "os" + "testing" + "time" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// TestRelationReindexCost measures ReindexCollectionRelationLinks at scale. +// +// The lead asked for this number on TASK-2997's trail before accepting the +// reindex-on-schema-change ruling, and the reason is worth keeping: "a rare +// owner-only operation" is a claim about FREQUENCY, and it says nothing about +// what the operation costs when it does run. The reindex holds the +// schema-update transaction open for its whole duration, so its cost is a +// user-visible latency on an API call, not background work. +// +// Skipped unless RELATION_COST=1 — it builds 10k items, which is minutes of +// setup nobody wants on every `go test ./...`. +func TestRelationReindexCost(t *testing.T) { + if os.Getenv("RELATION_COST") != "1" { + t.Skip("set RELATION_COST=1 to run the 10k reindex measurement") + } + s := testStore(t) + ws, colors, cars, red := relationFixture(t, s) + _ = colors + + const n = 10000 + start := time.Now() + for i := 0; i < n; i++ { + blob, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID}) + if _, err := s.CreateItem(ws.ID, cars.ID, models.ItemCreate{ + Title: fmt.Sprintf("Car %d", i), + Fields: string(blob), + }); err != nil { + t.Fatalf("CreateItem %d: %v", i, err) + } + } + t.Logf("setup: created %d items in %s", n, time.Since(start).Round(time.Millisecond)) + + tx, err := s.db.Begin() + if err != nil { + t.Fatalf("begin: %v", err) + } + reindexStart := time.Now() + if err := s.ReindexCollectionRelationLinks(tx, cars.ID, ws.ID); err != nil { + tx.Rollback() //nolint:errcheck + t.Fatalf("reindex: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit: %v", err) + } + elapsed := time.Since(reindexStart) + + count, err := s.CountRelationBacklinks(red.ID, ws.ID, unrestricted) + if err != nil { + t.Fatalf("count: %v", err) + } + if count != n { + t.Fatalf("reindex produced %d edges, want %d — the measurement below would describe the wrong work", count, n) + } + t.Logf("REINDEX COST: %d items, %d edges, %s", n, count, elapsed.Round(time.Millisecond)) + + // And the BACKFILL over the same data, which is the startup path and the + // one codex round 5 asked about: it batches by workspace in bounded + // chunks, so the numbers that matter are wall time and transaction COUNT + // rather than time alone. + if _, err := s.db.Exec(s.q(`DELETE FROM item_relation_links`)); err != nil { + t.Fatalf("clear index: %v", err) + } + if _, err := s.db.Exec(s.q(`DELETE FROM platform_settings WHERE key = ?`), relationLinksBackfilledFlag); err != nil { + t.Fatalf("clear marker: %v", err) + } + backfillStart := time.Now() + res, err := s.BackfillRelationLinks() + if err != nil { + t.Fatalf("backfill: %v", err) + } + backfillElapsed := time.Since(backfillStart) + after, err := s.CountRelationBacklinks(red.ID, ws.ID, unrestricted) + if err != nil { + t.Fatalf("count after backfill: %v", err) + } + if after != n { + t.Fatalf("the backfill produced %d edges, want %d — the timing below would describe the wrong work", after, n) + } + txns := (res.ItemsScanned + relationBackfillChunk - 1) / relationBackfillChunk + t.Logf("BACKFILL COST: %d items scanned, %d edges, %s, ~%d transactions (chunk=%d)", + res.ItemsScanned, res.LinksInserted, backfillElapsed.Round(time.Millisecond), txns, relationBackfillChunk) +} diff --git a/internal/store/relation_links_test.go b/internal/store/relation_links_test.go new file mode 100644 index 000000000..3882f43d9 --- /dev/null +++ b/internal/store/relation_links_test.go @@ -0,0 +1,713 @@ +package store + +import ( + "encoding/json" + "testing" + "time" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// The materialised reverse index for relation values (PLAN-2857 U5 / +// TASK-2997). Q3's proving test, leg for leg. +// +// EVERY LEG HERE IS A WIRING CLAIM. The index is only ever as good as the +// hooks that maintain it, so a test that exercised the helper directly would +// vouch for nothing — these go through the real write paths (CreateItem, +// UpdateItem, DeleteItem, RestoreItem, MoveItem, UpdateCollection) and read +// through the real query. +// +// The counterfactual that proves they are wired is a MUTATION, not a test in +// this file: replaceRelationLinks is stubbed to return early, the mutant is +// checked to COMPILE, and every leg here must fail. It is recorded in the +// commit messages with its verdict. An earlier version of this comment +// pointed at a TestRelationLinks_RemovedHookGoesStale that does not exist — +// a claim about the file's own contents that nothing checked (codex round 7). + +// unrestricted is the visibility vector for a viewer who sees everything. The +// viewer-scoped legs build their own. +var unrestricted = BacklinksVisibility{Unrestricted: true} + +// relationIndexFixture returns a workspace with Cars (carrying a `color` +// relation) and Colors, plus one colour to point at. +func relationIndexFixture(t *testing.T, s *Store) (*models.Workspace, *models.Collection, *models.Collection, *models.Item) { + t.Helper() + return relationFixture(t, s) +} + +// countFor is the reverse count a viewer who sees everything gets. +func countFor(t *testing.T, s *Store, target *models.Item, ws *models.Workspace) int { + t.Helper() + n, err := s.CountRelationBacklinks(target.ID, ws.ID, unrestricted) + if err != nil { + t.Fatalf("CountRelationBacklinks(%s): %v", target.Title, err) + } + return n +} + +// carPointingAt creates an item in `cars` whose `color` relation names target. +func carPointingAt(t *testing.T, s *Store, ws *models.Workspace, cars *models.Collection, title, targetID string) *models.Item { + t.Helper() + blob, _ := json.Marshal(map[string]any{"status": "open", "color": targetID}) + it, err := s.CreateItem(ws.ID, cars.ID, models.ItemCreate{Title: title, Fields: string(blob)}) + if err != nil { + t.Fatalf("CreateItem(%s): %v", title, err) + } + return it +} + +// TestRelationLinks_WriteThenReadCountsTheSource is the first half of Q3's +// property: a written relation value shows up on the target's reverse side. +func TestRelationLinks_WriteThenReadCountsTheSource(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + + if n := countFor(t, s, red, ws); n != 0 { + t.Fatalf("a fresh target is referenced by %d, want 0 — the fixture is not clean", n) + } + car := carPointingAt(t, s, ws, cars, "Red Car", red.ID) + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("after a write the target is referenced by %d, want 1", n) + } + + links, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted) + if err != nil { + t.Fatalf("GetRelationBacklinks: %v", err) + } + if len(links) != 1 { + t.Fatalf("want 1 backlink, got %d", len(links)) + } + if links[0].SourceItemID != car.ID { + t.Errorf("source = %s, want %s", links[0].SourceItemID, car.ID) + } + // The field key is the whole reason this index is its own table: the + // reverse side must be able to say WHICH field points here. + if links[0].FieldKey != "color" { + t.Errorf("field key = %q, want %q", links[0].FieldKey, "color") + } + if links[0].SourceRef != car.Ref || links[0].SourceTitle != "Red Car" { + t.Errorf("backlink lost the source's identity: %+v", links[0]) + } +} + +// TestRelationLinks_ChangingTheValueMovesTheEdge is Q3's BOTH HALVES leg. +// +// A writer that only ever INSERTs passes "the new target counts it". Only the +// old target dropping to zero proves the edge MOVED rather than accumulated, +// and that is the half a naive hook fails. +func TestRelationLinks_ChangingTheValueMovesTheEdge(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, colors, cars, red := relationIndexFixture(t, s) + blue := createTestItem(t, s, ws.ID, colors.ID, "Blue", "") + + car := carPointingAt(t, s, ws, cars, "Repainted Car", red.ID) + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("setup: red referenced by %d, want 1", n) + } + + blob, _ := json.Marshal(map[string]any{"status": "open", "color": blue.ID}) + fields := string(blob) + if _, err := s.UpdateItem(car.ID, models.ItemUpdate{Fields: &fields}); err != nil { + t.Fatalf("UpdateItem: %v", err) + } + + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("the OLD target is still referenced by %d, want 0 — the hook inserts but does not remove, so edges accumulate", n) + } + if n := countFor(t, s, blue, ws); n != 1 { + t.Errorf("the NEW target is referenced by %d, want 1", n) + } +} + +// TestRelationLinks_SoftDeletedSourceDropsOutAndRestoreBringsItBack. +// +// store/items.go documents that a restore resurrects an item's relationships; +// the reverse side has to match, or a restored item is silently missing from +// the pages that referenced it. +func TestRelationLinks_SoftDeletedSourceDropsOutAndRestoreBringsItBack(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + car := carPointingAt(t, s, ws, cars, "Disappearing Car", red.ID) + + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("setup: referenced by %d, want 1", n) + } + if err := s.DeleteItem(car.ID); err != nil { + t.Fatalf("DeleteItem: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("a soft-deleted source still counts (%d); the reverse side must not cite an item nobody can open", n) + } + if _, err := s.RestoreItem(car.ID); err != nil { + t.Fatalf("RestoreItem: %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("after restore the source is referenced %d times, want 1 — restore resurrects relationships and the reverse side must agree", n) + } +} + +// TestRelationLinks_SoftDeletedTargetKeepsItsRows asserts on the STORE's +// answer rather than on a chip's wording. +// +// Q3's row phrases this leg as "the source's chip renders (deleted)". That +// chip is U2's, and BUG-3013 is that "deleted" overclaims when gone and hidden +// are indistinguishable — so pinning the word here would couple this unit to a +// wording that is already scheduled to change. What U5 owes is that the EDGE +// survives: the question "who pointed at this?" still has an answer after the +// target is deleted. +func TestRelationLinks_SoftDeletedTargetKeepsItsRows(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + carPointingAt(t, s, ws, cars, "Orphaned Car", red.ID) + + if err := s.DeleteItem(red.ID); err != nil { + t.Fatalf("DeleteItem(target): %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("deleting the TARGET dropped its reverse side to %d; the edge belongs to the source, which still points here", n) + } +} + +// TestRelationLinks_MoveReindexesAgainstTheDestinationSchema. +// +// A move is the one write where the blob and the SCHEMA change together. The +// same stored value indexes differently on the other side, because which keys +// are relations belongs to the COLLECTION. +func TestRelationLinks_MoveReindexesAgainstTheDestinationSchema(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + car := carPointingAt(t, s, ws, cars, "Moving Car", red.ID) + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("setup: referenced by %d, want 1", n) + } + + // A collection with NO relation field: the same blob carries no edge here. + plain, err := s.CreateCollection(ws.ID, models.CollectionCreate{ + Name: "Plain", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"}]}`, + }) + if err != nil { + t.Fatalf("CreateCollection: %v", err) + } + // The blob travels unchanged; only the collection differs. + moved, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID}) + if _, err := s.MoveItem(car.ID, plain.ID, string(moved)); err != nil { + t.Fatalf("MoveItem: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("after moving into a collection with no relation field the target is still referenced %d times — the move indexed against the collection it LEFT", n) + } +} + +// TestRelationLinks_SchemaChangeReindexesTheCollection is the lead's D2 ruling +// as a test. +// +// Declaring a relation field over values that already exist must index them. +// Nothing writes those items, so "it fixes itself on the next write" would +// leave the index wrong indefinitely — the same argument that made this index +// materialised rather than derived. +func TestRelationLinks_SchemaChangeReindexesTheCollection(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, _, red := relationIndexFixture(t, s) + + // A collection where `color` is plain TEXT, holding what is already a + // valid item id. + garages, err := s.CreateCollection(ws.ID, models.CollectionCreate{ + Name: "Garages", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"text"}]}`, + }) + if err != nil { + t.Fatalf("CreateCollection: %v", err) + } + blob, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID}) + if _, err := s.CreateItem(ws.ID, garages.ID, models.ItemCreate{Title: "Garage", Fields: string(blob)}); err != nil { + t.Fatalf("CreateItem: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Fatalf("a TEXT field must not be indexed as a relation (count %d)", n) + } + + // Now declare it a relation. No item is written. + reshaped := `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"relation","collection":"colors"}]}` + if _, err := s.UpdateCollection(garages.ID, models.CollectionUpdate{Schema: &reshaped}); err != nil { + t.Fatalf("UpdateCollection: %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("after the field became a relation the target is referenced %d times, want 1 — nothing writes these items, so only the schema-change reindex can find them", n) + } + + // And the reverse direction: retyping it away must remove the edges. + back := `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"text"}]}` + if _, err := s.UpdateCollection(garages.ID, models.CollectionUpdate{Schema: &back}); err != nil { + t.Fatalf("UpdateCollection(back): %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("after retyping the field away from relation the target is still referenced %d times, want 0", n) + } +} + +// TestRelationLinks_BackfillDerivesRowsWrittenBeforeTheIndexExisted is Q3's +// backfill leg. The table is emptied to simulate a pre-migration database. +func TestRelationLinks_BackfillDerivesRowsWrittenBeforeTheIndexExisted(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + carPointingAt(t, s, ws, cars, "Pre-index Car", red.ID) + + // Simulate the pre-migration state: the rows exist, the index does not, + // and no completed pass has been recorded. + if _, err := s.db.Exec(s.q(`DELETE FROM item_relation_links`)); err != nil { + t.Fatalf("clear index: %v", err) + } + if _, err := s.db.Exec(s.q(`DELETE FROM platform_settings WHERE key = ?`), relationLinksBackfilledFlag); err != nil { + t.Fatalf("clear backfill marker: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Fatalf("the index was not actually cleared (count %d), so the leg below would pass without a backfill", n) + } + + res, err := s.BackfillRelationLinks() + if err != nil { + t.Fatalf("BackfillRelationLinks: %v", err) + } + if res.Skipped { + t.Fatal("the backfill skipped: it must not short-circuit when the table is empty and a collection declares a relation field") + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("after the backfill the target is referenced %d times, want 1", n) + } + + // Second run short-circuits, which is what keeps steady-state boots cheap. + again, err := s.BackfillRelationLinks() + if err != nil { + t.Fatalf("BackfillRelationLinks (second): %v", err) + } + if !again.Skipped { + t.Errorf("the second run re-derived instead of short-circuiting (%+v); every boot would pay a full scan", again) + } +} + +// TestRelationLinks_ImportIndexesTheRemappedIDs covers the import hook, which +// none of the legs above reach. +// +// Two things have to be true and only one is obvious. The edge must be indexed +// at all — and it must be indexed with the DESTINATION's id, not the exported +// one. The import rewrites relation values to the new ids in a SECOND pass, so +// a hook at the first-pass INSERT would index ids belonging to the workspace +// that produced the bundle: rows that match nothing, on an index whose whole +// job is matching. +func TestRelationLinks_ImportIndexesTheRemappedIDs(t *testing.T) { + t.Parallel() + s := testStore(t) + owner, err := s.CreateUser(models.UserCreate{ + Name: "Owner", Email: "relation-index-import@example.com", Password: "passw0rd!", + }) + if err != nil { + t.Fatalf("CreateUser: %v", err) + } + + const oldColorID = "old-color-indexed" + export := &models.WorkspaceExport{ + Version: 1, + ExportedAt: "2026-09-11T00:00:00Z", + Workspace: models.WorkspaceExportMeta{Name: "Index Archive", Slug: "index-archive"}, + Collections: []models.CollectionExport{ + { + ID: "old-coll-colors", Name: "Colors", Slug: "colors", Prefix: "COLO", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"}]}`, + CreatedAt: "2026-09-11T00:00:00Z", UpdatedAt: "2026-09-11T00:00:00Z", + }, + { + ID: "old-coll-cars", Name: "Cars", Slug: "cars", Prefix: "CAR", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"relation","collection":"colors"}]}`, + CreatedAt: "2026-09-11T00:00:00Z", UpdatedAt: "2026-09-11T00:00:00Z", + }, + }, + Items: []models.ItemExport{ + { + ID: oldColorID, CollectionID: "old-coll-colors", Title: "Imported Red", Slug: "imported-red", + Fields: `{}`, Tags: `[]`, + CreatedAt: "2026-09-11T00:00:00Z", UpdatedAt: "2026-09-11T00:00:00Z", + }, + { + ID: "old-car-indexed", CollectionID: "old-coll-cars", Title: "Imported Car", Slug: "imported-car", + Fields: `{"color":"` + oldColorID + `"}`, Tags: `[]`, + CreatedAt: "2026-09-11T00:00:00Z", UpdatedAt: "2026-09-11T00:00:00Z", + }, + }, + } + + ws, err := s.ImportWorkspace(export, "Index Archive Target", owner.ID, "") + if err != nil { + t.Fatalf("ImportWorkspace: %v", err) + } + items, err := s.ListItems(ws.ID, models.ItemListParams{}) + if err != nil { + t.Fatalf("ListItems: %v", err) + } + var red, car models.Item + for _, it := range items { + switch it.Title { + case "Imported Red": + red = it + case "Imported Car": + car = it + } + } + if red.ID == "" || car.ID == "" { + t.Fatalf("the bundle did not import both items; the assertion below would be vacuous (got %d items)", len(items)) + } + + n, err := s.CountRelationBacklinks(red.ID, ws.ID, unrestricted) + if err != nil { + t.Fatalf("CountRelationBacklinks: %v", err) + } + if n != 1 { + t.Fatalf("the imported target is referenced %d times, want 1 — the import hook did not run, or ran before the id remap", n) + } + + links, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted) + if err != nil { + t.Fatalf("GetRelationBacklinks: %v", err) + } + if len(links) != 1 || links[0].SourceItemID != car.ID { + t.Errorf("backlink = %+v, want the DESTINATION's car id %s — an index built at the first-pass insert would hold the exported id instead", links, car.ID) + } +} + +// TestRelationLinks_RestoreAfterASchemaChangeReindexes is codex round 1's +// second P1, and it is a hole my own site enumeration could not have found. +// +// I enumerated the sites that WRITE items.fields. Restore writes no fields — +// it clears deleted_at — so it never appeared. But the index depends on +// (blob, schema) AND on the item being live, and the collection reindex +// deliberately skips soft-deleted items. So: delete an item, change the +// collection's schema while it is gone, restore it, and its rows are whatever +// they were before the deletion — stale against a schema that has moved. +// +// Both directions matter. A field that BECAME a relation leaves the restored +// item missing edges; a field that stopped being one leaves it with edges it +// should not have. +func TestRelationLinks_RestoreAfterASchemaChangeReindexes(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + + car := carPointingAt(t, s, ws, cars, "Time Traveller", red.ID) + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("setup: referenced by %d, want 1", n) + } + + // Gone while the schema moves under it. + if err := s.DeleteItem(car.ID); err != nil { + t.Fatalf("DeleteItem: %v", err) + } + plain := `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"text"}]}` + if _, err := s.UpdateCollection(cars.ID, models.CollectionUpdate{Schema: &plain}); err != nil { + t.Fatalf("UpdateCollection: %v", err) + } + + if _, err := s.RestoreItem(car.ID); err != nil { + t.Fatalf("RestoreItem: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("the restored item is referenced %d times, want 0 — `color` is a TEXT field now, and the reindex that would have noticed skipped this item because it was deleted at the time", n) + } + + // The other direction: make it a relation again while the item is gone, + // and the restore must FIND the edge. + if err := s.DeleteItem(car.ID); err != nil { + t.Fatalf("DeleteItem (second): %v", err) + } + back := `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"relation","collection":"colors"}]}` + if _, err := s.UpdateCollection(cars.ID, models.CollectionUpdate{Schema: &back}); err != nil { + t.Fatalf("UpdateCollection (back): %v", err) + } + if _, err := s.RestoreItem(car.ID); err != nil { + t.Fatalf("RestoreItem (second): %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("the restored item is referenced %d times, want 1 — `color` is a relation again and the restore must derive the edge the reindex could not", n) + } +} + +// TestRelationLinks_AnInterruptedBackfillIsRetriedNotSkipped is codex round +// 1's first P1. +// +// The guard used to be "does item_relation_links have any row". Rows commit +// per item, so a crash partway through leaves the table NON-EMPTY AND +// INCOMPLETE — and that guard then skips forever, with the missing edges never +// derived. The rebuild story ("delete the table") only ever covered deliberate +// deletion. +// +// Completion is now recorded explicitly and only after a full pass, so a table +// that is partially populated with no marker must be RE-DERIVED. +func TestRelationLinks_AnInterruptedBackfillIsRetriedNotSkipped(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, colors, cars, red := relationIndexFixture(t, s) + blue := createTestItem(t, s, ws.ID, colors.ID, "Blue", "") + + carPointingAt(t, s, ws, cars, "Indexed Car", red.ID) + carPointingAt(t, s, ws, cars, "Missed Car", blue.ID) + + // The shape an interrupted run leaves behind: one item's rows present, + // another's missing, and NO completion marker. + if _, err := s.db.Exec(s.q(`DELETE FROM item_relation_links WHERE target_item_id = ?`), blue.ID); err != nil { + t.Fatalf("simulate partial index: %v", err) + } + if _, err := s.db.Exec(s.q(`DELETE FROM platform_settings WHERE key = ?`), relationLinksBackfilledFlag); err != nil { + t.Fatalf("clear marker: %v", err) + } + if n := countFor(t, s, blue, ws); n != 0 { + t.Fatalf("the partial state was not established (blue count %d); the assertion below would pass for the wrong reason", n) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("the partial state removed too much (red count %d)", n) + } + + res, err := s.BackfillRelationLinks() + if err != nil { + t.Fatalf("BackfillRelationLinks: %v", err) + } + if res.Skipped { + t.Fatal("the backfill SKIPPED a partially populated table; under the old any-row guard the missing edges would never be derived") + } + if n := countFor(t, s, blue, ws); n != 1 { + t.Errorf("the missed edge is still missing (count %d) after a retry", n) + } + // And the already-indexed one is not duplicated — the pass is idempotent + // because replaceRelationLinks deletes before it inserts. + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("the already-indexed edge is now counted %d times; a repeat pass must replace rather than append", n) + } +} + +// TestRelationLinks_BackfillClearsAStaleEdge is codex round 2's first P1. +// +// The backfill used to SKIP an item whose blob carries no edges, as an +// optimisation. That is wrong for exactly the state it exists to repair: an +// interrupted pass can leave a STALE row for an item whose value has since +// been cleared, and skipping it preserves that row and then writes the +// completion marker over it — permanently. +// +// The retry test above covers a MISSING edge. This one covers the opposite, +// which is the case the optimisation hid. +func TestRelationLinks_BackfillClearsAStaleEdge(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + car := carPointingAt(t, s, ws, cars, "Cleared Car", red.ID) + + // The item's value is gone, but the index still carries the edge — the + // shape an interrupted pass leaves behind. Written directly so the hook + // does not repair it first. + cleared, _ := json.Marshal(map[string]any{"status": "open"}) + if _, err := s.db.Exec(s.q(`UPDATE items SET fields = ? WHERE id = ?`), string(cleared), car.ID); err != nil { + t.Fatalf("clear the value behind the hook's back: %v", err) + } + if _, err := s.db.Exec(s.q(`DELETE FROM platform_settings WHERE key = ?`), relationLinksBackfilledFlag); err != nil { + t.Fatalf("clear marker: %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("the stale state was not established (count %d); the assertion below would pass for the wrong reason", n) + } + + if _, err := s.BackfillRelationLinks(); err != nil { + t.Fatalf("BackfillRelationLinks: %v", err) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("the stale edge survived the backfill (count %d) — an item with zero edges is precisely the one that must still be rewritten, because replaceRelationLinks deletes before it inserts", n) + } +} + +// TestRelationLinks_ASourceInADeletedCollectionIsNotCounted is codex round 2's +// disclosure P1. +// +// DeleteCollection soft-deletes the COLLECTION and leaves its items live. The +// reverse query joins collections, so without a deleted_at filter a source +// sitting in a collection the viewer can no longer open still appears — and +// for an Unrestricted viewer there is no collection filter anywhere else to +// catch it. +func TestRelationLinks_ASourceInADeletedCollectionIsNotCounted(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + + doomed, err := s.CreateCollection(ws.ID, models.CollectionCreate{ + Name: "Doomed", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","type":"relation","collection":"colors"}]}`, + }) + if err != nil { + t.Fatalf("CreateCollection: %v", err) + } + blob, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID}) + if _, err := s.CreateItem(ws.ID, doomed.ID, models.ItemCreate{Title: "Doomed Car", Fields: string(blob)}); err != nil { + t.Fatalf("CreateItem: %v", err) + } + // CONTROL: a source in a LIVE collection, so the assertion below is about + // the deletion rather than about the query returning nothing. + carPointingAt(t, s, ws, cars, "Surviving Car", red.ID) + + if n := countFor(t, s, red, ws); n != 2 { + t.Fatalf("setup: referenced by %d, want 2", n) + } + + if err := s.DeleteCollection(doomed.ID, ""); err != nil { + t.Fatalf("DeleteCollection: %v", err) + } + if n := countFor(t, s, red, ws); n != 1 { + t.Errorf("referenced by %d, want 1 — a source in a soft-deleted collection is still being disclosed; its items stay live, so only a collections.deleted_at filter hides it", n) + } + links, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted) + if err != nil { + t.Fatalf("GetRelationBacklinks: %v", err) + } + if len(links) != 1 { + t.Errorf("the page returned %d rows while the count says 1 — the two ran under different filters", len(links)) + } +} + +// TestRelationLinks_HardDeleteCascadesTheEdges pins a claim the input matrix +// makes and nothing else checked (codex round 4's nit). +// +// The matrix says a hard delete needs no hook because `ON DELETE CASCADE` on +// source_item_id removes the outgoing edges with the row. That is a claim +// about a FOREIGN KEY being enforced — which on SQLite depends on +// `foreign_keys=on` being set by the connection, not on the DDL alone. A +// matrix row resting on "the schema says so" is worth one test. +func TestRelationLinks_HardDeleteCascadesTheEdges(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + car := carPointingAt(t, s, ws, cars, "Doomed Car", red.ID) + + if n := countFor(t, s, red, ws); n != 1 { + t.Fatalf("setup: referenced by %d, want 1", n) + } + + // Hard delete, not soft: the row goes, and the FK must take its edges. + if _, err := s.db.Exec(s.q(`DELETE FROM items WHERE id = ?`), car.ID); err != nil { + t.Fatalf("hard delete: %v", err) + } + + var orphans int + if err := s.db.QueryRow(s.q(`SELECT COUNT(*) FROM item_relation_links WHERE source_item_id = ?`), car.ID).Scan(&orphans); err != nil { + t.Fatalf("count orphan rows: %v", err) + } + if orphans != 0 { + t.Errorf("%d index row(s) survived a hard delete of their source — the cascade is not firing, and the input matrix's 'no hook needed here' reason does not hold", orphans) + } + if n := countFor(t, s, red, ws); n != 0 { + t.Errorf("referenced by %d after the source was hard-deleted, want 0", n) + } +} + +// TestRelationLinks_BacklinkCarriesTheFieldLabel pins codex round 6's nit, +// which was a field declared, documented, consumed by the UI — and never +// populated, so every group rendered its raw key. +// +// The label is what the schema author wrote for humans, so the choice was to +// populate it or delete the field. This asserts the populated version, and the +// fallback case (a field with no label) alongside it, because the UI branches +// on exactly that. +func TestRelationLinks_BacklinkCarriesTheFieldLabel(t *testing.T) { + t.Parallel() + s := testStore(t) + ws, _, _, red := relationIndexFixture(t, s) + + // `owner` carries a label; `sidekick` deliberately does not. + labelled, err := s.CreateCollection(ws.ID, models.CollectionCreate{ + Name: "Labelled", + Schema: `{"fields":[ + {"key":"status","type":"select","options":["open","done"],"default":"open"}, + {"key":"owner","label":"Responsible Colour","type":"relation","collection":"colors"}, + {"key":"sidekick","type":"relation","collection":"colors"} + ]}`, + }) + if err != nil { + t.Fatalf("CreateCollection: %v", err) + } + blob, _ := json.Marshal(map[string]any{"status": "open", "owner": red.ID, "sidekick": red.ID}) + if _, err := s.CreateItem(ws.ID, labelled.ID, models.ItemCreate{Title: "Two Ways", Fields: string(blob)}); err != nil { + t.Fatalf("CreateItem: %v", err) + } + + links, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted) + if err != nil { + t.Fatalf("GetRelationBacklinks: %v", err) + } + if len(links) != 2 { + t.Fatalf("want 2 edges (one per relation field), got %d", len(links)) + } + byKey := map[string]RelationBacklink{} + for _, l := range links { + byKey[l.FieldKey] = l + } + if got := byKey["owner"].FieldLabel; got != "Responsible Colour" { + t.Errorf("owner's label = %q, want %q — the field was declared and consumed while nothing populated it", got, "Responsible Colour") + } + // No label in the schema means an EMPTY one here, not the key: the client + // owns that fallback, and duplicating it server-side would make the two + // disagree the day either changes. + if got := byKey["sidekick"].FieldLabel; got != "" { + t.Errorf("sidekick's label = %q, want empty — a field with no schema label must not have one invented for it", got) + } +} + +// TestRelationBacklinksDoNoSecondPoolRead is the guard for the invariant round +// 7's P1 cost me. +// +// GetRelationBacklinks holds the rows cursor open while it builds the result. +// The first version of the field-LABEL lookup issued a SELECT per distinct +// collection inside that loop — which needs a SECOND pool connection, and when +// the pool is saturated the second connection never arrives. That is an +// application deadlock no SQLSTATE names, the same class BUG-2409 closed for +// the collection writers, and it only shows up under load. +// +// With MaxOpenConns(1) the open cursor owns the only connection, so ANY second +// pool read here deadlocks deterministically instead of rarely. Works on both +// dialects. +func TestRelationBacklinksDoNoSecondPoolRead(t *testing.T) { + s := testStore(t) + ws, _, cars, red := relationIndexFixture(t, s) + + // Two sources in DIFFERENT collections, so a per-collection lookup would + // have to issue more than one — a single-collection fixture would let the + // broken shape pass on a lucky cache hit. + carPointingAt(t, s, ws, cars, "Pool Car", red.ID) + other, err := s.CreateCollection(ws.ID, models.CollectionCreate{ + Name: "Garages Too", + Schema: `{"fields":[{"key":"status","type":"select","options":["open","done"],"default":"open"},{"key":"color","label":"Paint","type":"relation","collection":"colors"}]}`, + }) + if err != nil { + t.Fatalf("CreateCollection: %v", err) + } + blob, _ := json.Marshal(map[string]any{"status": "open", "color": red.ID}) + if _, err := s.CreateItem(ws.ID, other.ID, models.ItemCreate{Title: "Pool Garage", Fields: string(blob)}); err != nil { + t.Fatalf("CreateItem: %v", err) + } + + // From here the pool has exactly one connection. + s.db.SetMaxOpenConns(1) + + done := make(chan error, 1) + go func() { + _, err := s.GetRelationBacklinks(red.ID, ws.ID, 50, 0, unrestricted) + done <- err + }() + select { + case err := <-done: + if err != nil { + t.Fatalf("GetRelationBacklinks: %v", err) + } + case <-time.After(30 * time.Second): + t.Fatal("GetRelationBacklinks did not complete under MaxOpenConns(1): it issued a pool read while holding its own rows cursor, and is waiting for a connection it is itself occupying") + } +} diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index 8e7030970..28e7d7dfe 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -7,6 +7,7 @@ import type { CollectionCreate, CollectionUpdate, Backlink, + RelationBacklinksPage, Item, BulkItemsRequest, BulkItemsResponse, @@ -1370,6 +1371,32 @@ export const api = { })}` ), + /** + * Inbound `relation` FIELD references to an item — the "Referenced + * by" section (PLAN-2857 U5). Distinct from `backlinks`, which is + * `[[...]]` mentions in prose: these are typed field edges and each + * carries the `field_key` it points through. + * + * `total` is the count under the CALLER'S visibility, not the true + * one. A viewer with partial access sees their own number, because a + * true count would disclose the existence of items they cannot read. + * + * No cross-workspace tier: a relation names an item id and PLAN-2857 + * v1 excludes cross-workspace targets, so there is nothing foreign to + * page through. + */ + relationBacklinks: ( + ws: string, + slug: string, + opts?: { limit?: number; offset?: number } + ) => + request( + `/workspaces/${ws}/items/${slug}/relation-backlinks${qs({ + limit: opts?.limit, + offset: opts?.offset, + })}` + ), + /** Get child items linked to a parent item */ children: (ws: string, slug: string) => request(`/workspaces/${ws}/items/${slug}/children`), diff --git a/web/src/lib/components/RelationBacklinksPanel.svelte b/web/src/lib/components/RelationBacklinksPanel.svelte new file mode 100644 index 000000000..5ca24cd24 --- /dev/null +++ b/web/src/lib/components/RelationBacklinksPanel.svelte @@ -0,0 +1,261 @@ + + +{#if shouldRender} +
+
+

Referenced by

+ {#if !loading && !error && total > 0} + {total} + {/if} +
+ + {#if loading} +
Loading references…
+ {:else if error} +
{error}
+ {:else} + {#each groups as group (group.key)} +
+
{group.label}
+ +
+ {/each} + + {#if hasMore} +
+ +
+ {/if} + {/if} +
+{/if} + + diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index f3cfdeff2..22169f08d 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -34,6 +34,7 @@ } from '$lib/components/timeline/feed'; import ChildItems from '$lib/components/ChildItems.svelte'; import BacklinksPanel from '$lib/components/BacklinksPanel.svelte'; + import RelationBacklinksPanel from '$lib/components/RelationBacklinksPanel.svelte'; import { goto } from '$app/navigation'; import { relativeTime, wikiLinksToMarkdown, markdownToWikiLinks, cleanBrokenLinks, unescapeDocLinks } from '$lib/utils/markdown'; import { toastStore } from '$lib/stores/toast.svelte'; @@ -6068,6 +6069,22 @@ onCountChange={(n) => { if (keyedSlug !== itemSlug) return; backlinksCount = n; }} /> + + {/if} diff --git a/web/src/lib/types/index.ts b/web/src/lib/types/index.ts index 8668051b5..2ff9ed92c 100644 --- a/web/src/lib/types/index.ts +++ b/web/src/lib/types/index.ts @@ -1339,6 +1339,43 @@ export interface ItemLinkCreate { * Mirrors `internal/models/backlink.go`. The Phase 3 UI (TASK-1596) renders * these as the "Mentioned in" panel beneath an item's content. */ +/** + * One item that points at another through a `relation` FIELD — the + * "Referenced by" section's data (PLAN-2857 U5). + * + * Deliberately not a `Backlink`. A wiki backlink is a mention in prose and + * carries a `snippet`; this is a typed field-valued edge and carries the + * `field_key` it points through, which is what lets the section say + * "referenced by CAR-3 via Colour" rather than just "referenced by". There is + * no snippet because a field value has no surrounding text to quote. + */ +export interface RelationBacklink { + source_item_id: string; + /** Empty on a legacy item with no item_number — render the title alone. */ + source_ref: string; + source_title: string; + collection_slug: string; + /** The schema key the value sits under. */ + field_key: string; + /** The field's human label, when the schema gives one. */ + field_label?: string; +} + +/** + * The "Referenced by" payload. + * + * `total` is the COUNT UNDER THE VIEWER'S VISIBILITY, not the true one — a + * true count would tell the viewer that items they cannot see exist. So + * "Referenced by 3" means "by 3 you can see", and the number is computed under + * the same filters as the page it accompanies. + */ +export interface RelationBacklinksPage { + relation_backlinks: RelationBacklink[]; + total: number; + limit: number; + offset: number; +} + export interface Backlink { source_item_id: string; source_ref: string;