feat(backlinks): suppress parent↔child mentions from "Mentioned in" panel (TASK-1607) (#625)

On a parent's page (typically a PLAN with many child TASKs), the
"Mentioned in" panel was dominated by child tasks that wiki-link
back to their parent. Those children are already listed in the
Children section directly above the panel, so the same items
appeared twice on screen and buried genuine cross-references
(sibling plans, retro docs, etc.). Symmetric problem on a child's
page: the parent shows up in "Mentioned in" even though it's
already in the "Parent: …" header.

Suppression is server-side in two parts, modeled on the existing
self-link filter in GetBacklinks (s.id != targetItemID):

1. Children-suppression — always-on. Source rows where
   s.parent_id = targetItemID are dropped. Uses the existing
   targetItemID parameter; no API change. NULL-safe form so
   orphan items still surface (raw `s.parent_id != ?` would
   silently drop NULL parent_id rows under SQL three-valued logic).

2. Parent-suppression — opt-in. New TargetParentID *string field
   on BacklinksVisibility (zero value = nil = no parent
   suppression, keeping the ~50 existing test callsites valid).
   When set, the source row whose id == TargetParentID is dropped.
   The handler passes item.ParentID from the resolved target.

Both apply to CountBacklinks identically so the
handlers_backlinks.go same-ws/cross-ws pagination math
(which depends on count-vs-fetch agreeing) stays correct.
GetCrossWorkspaceBacklinks is unaffected — parent_id is
workspace-scoped, so a cross-ws source can't be the target's
parent or child by construction. Doc comment added noting this.

Tests:
- ChildMentionOfParentSuppressed: headline case
- ParentMentionOnChildPageSuppressed: symmetric case with/without
  TargetParentID
- SiblingMentionsNotSuppressed: control — siblings of a shared
  parent that wiki-link each other still surface
- OrphanBacklinksUnaffected: regression for the NULL-safe form
- PaginationStableAfterSuppression: page 1 + page 2 hit all
  filtered rows exactly once with no duplicates

Promoted from IDEA-1601. Follow-up to [[PLAN-1593]] (which
shipped the original wiki-link backlinks reverse index).
This commit is contained in:
xarmian
2026-05-24 20:52:11 -04:00
committed by GitHub
parent c1c9f2ef97
commit 4aff8c70a0
3 changed files with 329 additions and 4 deletions
+8
View File
@@ -98,6 +98,14 @@ func (s *Server) handleGetItemBacklinks(w http.ResponseWriter, r *http.Request)
vis.FullCollectionIDs = fullCollIDs
vis.GrantedItemIDs = grantedItemIDs
}
// Pass the target's parent_id (when present) so the store layer
// can suppress the parent's own "Mentioned in" entry on the
// target's page — the parent is already on screen in the
// "Parent: …" header above the panel, so its backlink would
// duplicate UI. Children-suppression is unconditional in the
// store; only parent-suppression needs the extra context.
// TASK-1607 / IDEA-1601.
vis.TargetParentID = item.ParentID
// Union pagination across same-ws + cross-ws tiers.
//
+58 -4
View File
@@ -862,6 +862,15 @@ type BacklinksVisibility struct {
Unrestricted bool
FullCollectionIDs []string
GrantedItemIDs []string
// TargetParentID, when non-nil and non-empty, suppresses the
// target's own parent from the backlinks result set. Set by the
// handler from items.ParentID. Rationale: the parent already
// appears in the "Parent: …" header on the target's page, so a
// child's wiki-link mention of its parent only duplicates UI
// already on screen. Symmetric with the always-on children-
// suppression in GetBacklinks/CountBacklinks. TASK-1607 / IDEA-1601.
TargetParentID *string
}
// GetBacklinks returns the items in `workspaceID` that contain a
@@ -875,6 +884,18 @@ type BacklinksVisibility struct {
// "Mentioned in" panel (PLAN-1593 behavior decision). The row stays
// in the index for completeness; the filter is purely cosmetic.
//
// Children-suppression is always-on for the same UI-duplication
// reason: a source item whose parent_id == targetItemID is a direct
// child, and children are already listed above the backlinks panel
// in the Child Items section. The child's body almost always
// references the parent (`[[Parent Title]]` somewhere in the
// narrative), which would otherwise dominate the backlinks list
// for any plan with many children. Parent-suppression is the
// symmetric case but opt-in via vis.TargetParentID: when the caller
// passes the target's parent_id, that one source row (the parent's
// mention of the target) is dropped because the parent appears in
// the target's "Parent: …" header. TASK-1607 / IDEA-1601.
//
// Ordering: most-recently-updated source first; within an updated_at
// tie (e.g. two backlinks land in the same second), break by
// position ASC so the order is at least deterministic.
@@ -900,6 +921,24 @@ func (s *Store) GetBacklinks(targetItemID, workspaceID string, limit, offset int
return nil, nil
}
// Relational-suppression clause. Always-on children-suppression
// (s.parent_id != targetItemID) mirrors the self-link filter
// (s.id != targetItemID) two lines down: both hide rows that
// already appear elsewhere in the target's page UI (Children
// section above the backlinks panel) and so carry no novel
// information. NULL-safe form because parent_id is nullable —
// orphan items have parent_id IS NULL and would otherwise be
// erroneously dropped by `s.parent_id != ?`. Parent-suppression
// is opt-in (only when vis.TargetParentID is set) because callers
// that don't have the target's parent_id handy can skip it
// without changing existing semantics. TASK-1607 / IDEA-1601.
relClause := " AND (s.parent_id IS NULL OR s.parent_id != ?)"
args := []interface{}{targetItemID, workspaceID, targetItemID, targetItemID}
if vis.TargetParentID != nil && *vis.TargetParentID != "" {
relClause += " AND s.id != ?"
args = append(args, *vis.TargetParentID)
}
// Build the visibility predicate. Unrestricted → omit. Otherwise
// `collection_id IN (...)` OR `id IN (...)` — either branch alone
// is acceptable so a granted-item-only access still resolves; an
@@ -907,7 +946,6 @@ func (s *Store) GetBacklinks(targetItemID, workspaceID string, limit, offset int
// predicate evaluates to FALSE for that branch without breaking
// Postgres's empty-list rejection.
visClause := ""
args := []interface{}{targetItemID, workspaceID, targetItemID}
if !vis.Unrestricted {
collClause := "FALSE"
if len(vis.FullCollectionIDs) > 0 {
@@ -940,7 +978,7 @@ func (s *Store) GetBacklinks(targetItemID, workspaceID string, limit, offset int
WHERE wl.target_item_id = ?
AND s.workspace_id = ?
AND s.deleted_at IS NULL
AND s.id != ?`+visClause+`
AND s.id != ?`+relClause+visClause+`
ORDER BY s.updated_at DESC, wl.position ASC
LIMIT ? OFFSET ?
`), args...)
@@ -998,8 +1036,19 @@ func (s *Store) CountBacklinks(targetItemID, workspaceID string, vis BacklinksVi
if !vis.Unrestricted && len(vis.FullCollectionIDs) == 0 && len(vis.GrantedItemIDs) == 0 {
return 0, nil
}
// Relational-suppression clause — must mirror GetBacklinks
// exactly. The handler's same-ws/cross-ws pagination math
// depends on CountBacklinks returning the same number of rows
// GetBacklinks would yield under identical vis; a drift here
// would let suppressed rows consume LIMIT slots and shrink
// pages silently. TASK-1607 / IDEA-1601.
relClause := " AND (s.parent_id IS NULL OR s.parent_id != ?)"
args := []interface{}{targetItemID, workspaceID, targetItemID, targetItemID}
if vis.TargetParentID != nil && *vis.TargetParentID != "" {
relClause += " AND s.id != ?"
args = append(args, *vis.TargetParentID)
}
visClause := ""
args := []interface{}{targetItemID, workspaceID, targetItemID}
if !vis.Unrestricted {
collClause := "FALSE"
if len(vis.FullCollectionIDs) > 0 {
@@ -1030,7 +1079,7 @@ func (s *Store) CountBacklinks(targetItemID, workspaceID string, vis BacklinksVi
WHERE wl.target_item_id = ?
AND s.workspace_id = ?
AND s.deleted_at IS NULL
AND s.id != ?`+visClause+`
AND s.id != ?`+relClause+visClause+`
`), args...).Scan(&n)
if err != nil {
return 0, fmt.Errorf("count backlinks: %w", err)
@@ -1081,6 +1130,11 @@ func (s *Store) CountBacklinks(targetItemID, workspaceID string, vis BacklinksVi
// workspace A still leaked cross-ws backlinks from workspace B
// because the cross-ws path enumerated via the user's full
// workspace list).
//
// No parent↔child suppression here (the GetBacklinks pair adds
// that for same-ws): item.parent_id is workspace-scoped, so a
// cross-ws source row can never be the target's parent or child
// by construction. TASK-1607.
func (s *Store) GetCrossWorkspaceBacklinks(targetWorkspaceID, targetRef, requesterUserID string, allowedWorkspaceSlugs []string, limit, offset int) ([]models.Backlink, error) {
if limit <= 0 || limit > 300 {
limit = 50
+263
View File
@@ -1240,6 +1240,269 @@ func TestWikiLinks_DuplicateSlashTitleNoTheft(t *testing.T) {
}
}
// createChildItem creates a test item with parent_id set in one
// CreateItem call. The wiki_links tests use this for parent↔child
// suppression coverage where createTestItem (which doesn't set
// parent_id) isn't enough. TASK-1607.
func createChildItem(t *testing.T, s *Store, workspaceID, collectionID, parentID, title, content string) *models.Item {
t.Helper()
pid := parentID
item, err := s.CreateItem(workspaceID, collectionID, models.ItemCreate{
Title: title,
Content: content,
Fields: `{"status":"open"}`,
ParentID: &pid,
})
if err != nil {
t.Fatalf("createChildItem: %v", err)
}
return item
}
// TestWikiLinks_ChildMentionOfParentSuppressed: the headline TASK-1607
// case. A direct child of `parent` mentioning `parent` in its body
// should NOT appear in parent's "Mentioned in" panel — children are
// already listed in the Child Items section above. Symmetric with
// the long-standing self-link suppression.
func TestWikiLinks_ChildMentionOfParentSuppressed(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
parent := createTestItem(t, s, ws.ID, col.ID, "Parent plan", "")
// Child links to parent. Without TASK-1607, this would surface
// as a backlink on parent's page.
child := createChildItem(t, s, ws.ID, col.ID, parent.ID, "Child task",
"See [["+refOf(parent)+"]] for context.")
got, err := s.GetBacklinks(parent.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true})
if err != nil {
t.Fatalf("GetBacklinks: %v", err)
}
if len(got) != 0 {
t.Errorf("expected 0 backlinks (child filtered), got %d: %+v", len(got), got)
}
// CountBacklinks must agree — pagination math in
// handlers_backlinks.go's same-ws/cross-ws split depends on it.
n, err := s.CountBacklinks(parent.ID, ws.ID, BacklinksVisibility{Unrestricted: true})
if err != nil {
t.Fatalf("CountBacklinks: %v", err)
}
if n != 0 {
t.Errorf("CountBacklinks: got %d, want 0", n)
}
// Sanity: the wiki-link row IS in the index (we only suppress
// at read time, not at write time — keeps the index complete
// for future broken-link reports etc.).
var raw int
if err := s.db.QueryRow(s.q(`
SELECT COUNT(*) FROM item_wiki_links
WHERE source_item_id = ? AND target_item_id = ?
`), child.ID, parent.ID).Scan(&raw); err != nil {
t.Fatalf("raw count: %v", err)
}
if raw != 1 {
t.Errorf("raw wiki_links row: got %d, want 1 (index stays complete)", raw)
}
}
// TestWikiLinks_ParentMentionOnChildPageSuppressed: the symmetric
// case. When the parent's body mentions its child by wiki-link, that
// mention should NOT appear in the child's "Mentioned in" panel —
// the parent is already on screen in the "Parent: …" header above
// the panel. Requires vis.TargetParentID to be set; the handler
// passes item.ParentID from the resolved target.
func TestWikiLinks_ParentMentionOnChildPageSuppressed(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
parent := createTestItem(t, s, ws.ID, col.ID, "Parent plan", "")
child := createChildItem(t, s, ws.ID, col.ID, parent.ID, "Child task", "")
// Now the parent mentions the child. Without the suppression,
// this would dominate the child's backlinks panel.
body := "Tracking work in [[" + refOf(child) + "]]."
if _, err := s.UpdateItem(parent.ID, models.ItemUpdate{Content: &body}); err != nil {
t.Fatalf("UpdateItem parent: %v", err)
}
// Without TargetParentID: the parent shows up (no suppression).
plain, err := s.GetBacklinks(child.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true})
if err != nil {
t.Fatalf("GetBacklinks (plain): %v", err)
}
if len(plain) != 1 {
t.Fatalf("baseline: expected 1 backlink (parent mention), got %d", len(plain))
}
// With TargetParentID set to the parent's ID: the parent's
// mention is suppressed.
pid := parent.ID
vis := BacklinksVisibility{Unrestricted: true, TargetParentID: &pid}
got, err := s.GetBacklinks(child.ID, ws.ID, 50, 0, vis)
if err != nil {
t.Fatalf("GetBacklinks (suppressed): %v", err)
}
if len(got) != 0 {
t.Errorf("expected 0 backlinks (parent filtered), got %d: %+v", len(got), got)
}
n, err := s.CountBacklinks(child.ID, ws.ID, vis)
if err != nil {
t.Fatalf("CountBacklinks: %v", err)
}
if n != 0 {
t.Errorf("CountBacklinks: got %d, want 0", n)
}
}
// TestWikiLinks_SiblingMentionsNotSuppressed: control case. Sibling
// items (same parent) that wiki-link each other are genuine
// cross-references — they aren't implied by any other UI surface, so
// they must still appear in "Mentioned in".
func TestWikiLinks_SiblingMentionsNotSuppressed(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
parent := createTestItem(t, s, ws.ID, col.ID, "Parent plan", "")
siblingA := createChildItem(t, s, ws.ID, col.ID, parent.ID, "Sibling A", "")
siblingB := createChildItem(t, s, ws.ID, col.ID, parent.ID, "Sibling B",
"Coordinates with [["+refOf(siblingA)+"]] on the API surface.")
// siblingA's backlinks should include siblingB. Children-
// suppression filters by `s.parent_id != targetItemID` — i.e.
// items whose parent IS siblingA. siblingB's parent is the
// parent plan, not siblingA, so siblingB is not a child of
// siblingA and must not be filtered.
got, err := s.GetBacklinks(siblingA.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true})
if err != nil {
t.Fatalf("GetBacklinks: %v", err)
}
if len(got) != 1 {
t.Fatalf("expected 1 sibling backlink, got %d: %+v", len(got), got)
}
if got[0].SourceItemID != siblingB.ID {
t.Errorf("expected sibling B as source, got %s", got[0].SourceItemID)
}
// Symmetric for the parent-suppression direction: if siblingA
// passes TargetParentID=parent.ID (as the handler would since
// siblingA's parent IS parent), siblingB is still NOT the
// parent, so it survives.
pid := parent.ID
vis := BacklinksVisibility{Unrestricted: true, TargetParentID: &pid}
got, err = s.GetBacklinks(siblingA.ID, ws.ID, 50, 0, vis)
if err != nil {
t.Fatalf("GetBacklinks (with parent vis): %v", err)
}
if len(got) != 1 {
t.Errorf("expected 1 backlink with parent vis set, got %d", len(got))
}
}
// TestWikiLinks_OrphanBacklinksUnaffected: regression guard for the
// NULL-safe parent_id predicate. Items with parent_id IS NULL
// (orphan / root-level items) must still surface as backlinks; the
// naive form `s.parent_id != ?` would silently drop them because
// SQL three-valued logic treats `NULL != x` as NULL (not TRUE).
func TestWikiLinks_OrphanBacklinksUnaffected(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
target := createTestItem(t, s, ws.ID, col.ID, "Target", "")
// Plain createTestItem produces an orphan item (no parent_id).
orphan := createTestItem(t, s, ws.ID, col.ID, "Orphan source",
"Mentions [["+refOf(target)+"]] from the root.")
got, err := s.GetBacklinks(target.ID, ws.ID, 50, 0, BacklinksVisibility{Unrestricted: true})
if err != nil {
t.Fatalf("GetBacklinks: %v", err)
}
if len(got) != 1 {
t.Fatalf("expected orphan source to surface, got %d backlinks", len(got))
}
if got[0].SourceItemID != orphan.ID {
t.Errorf("expected orphan as source, got %s", got[0].SourceItemID)
}
}
// TestWikiLinks_PaginationStableAfterSuppression: the suppression
// applies in SQL, so LIMIT/OFFSET counts the filtered set. Mixing
// suppressed and unsuppressed sources, pages 1 and 2 together must
// equal the full set without skipping or doubling. Regression guard
// for the GetBacklinks/CountBacklinks lockstep that the
// handlers_backlinks.go same-ws/cross-ws math depends on.
func TestWikiLinks_PaginationStableAfterSuppression(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
parent := createTestItem(t, s, ws.ID, col.ID, "Parent plan", "")
// 3 non-children that mention parent — these should all surface.
var realRefs []string
for i := 0; i < 3; i++ {
src := createTestItem(t, s, ws.ID, col.ID, "Real referrer "+itoa(i),
"Mentions [["+refOf(parent)+"]].")
realRefs = append(realRefs, src.ID)
}
// 5 children that also mention parent — these should be hidden.
for i := 0; i < 5; i++ {
createChildItem(t, s, ws.ID, col.ID, parent.ID,
"Child "+itoa(i),
"Refs [["+refOf(parent)+"]].")
}
vis := BacklinksVisibility{Unrestricted: true}
// CountBacklinks must reflect the filtered set, not the raw set.
n, err := s.CountBacklinks(parent.ID, ws.ID, vis)
if err != nil {
t.Fatalf("CountBacklinks: %v", err)
}
if n != 3 {
t.Fatalf("CountBacklinks: got %d, want 3 (3 real, 5 children filtered)", n)
}
// Page through with LIMIT=2; combined results must hit all 3
// real referrers exactly once, in updated_at DESC order.
page1, err := s.GetBacklinks(parent.ID, ws.ID, 2, 0, vis)
if err != nil {
t.Fatalf("GetBacklinks page 1: %v", err)
}
page2, err := s.GetBacklinks(parent.ID, ws.ID, 2, 2, vis)
if err != nil {
t.Fatalf("GetBacklinks page 2: %v", err)
}
if len(page1) != 2 {
t.Errorf("page 1: got %d, want 2", len(page1))
}
if len(page2) != 1 {
t.Errorf("page 2: got %d, want 1", len(page2))
}
combined := make(map[string]bool)
for _, bl := range append(page1, page2...) {
if combined[bl.SourceItemID] {
t.Errorf("duplicate source across pages: %s", bl.SourceItemID)
}
combined[bl.SourceItemID] = true
}
if len(combined) != 3 {
t.Errorf("combined unique sources: got %d, want 3", len(combined))
}
for _, want := range realRefs {
if !combined[want] {
t.Errorf("missing real referrer %s from combined pages", want)
}
}
}
// itoa is strconv.Itoa renamed to keep test bodies readable when
// they're already heavy on ref-formatting.
func itoa(n int) string {