Skip to content

GO-4284 Remove deprecated object-level relationLinks - #3175

Open
requilence wants to merge 3 commits into
developfrom
go-4284-remove-deprecated-relationlinks
Open

GO-4284 Remove deprecated object-level relationLinks#3175
requilence wants to merge 3 commits into
developfrom
go-4284-remove-deprecated-relationlinks

Conversation

@requilence

Copy link
Copy Markdown
Contributor

Why

Object-level relationLinks (the model.RelationLink list on an object, distinct from dataview relations) are no longer read by any client or by the local indexer — relations are tracked by detail keys (FetchRelationByLinks had zero callers; state.HasRelation/iterateKeys read details). They survived only via auto-add writers.

This caused a real cross-participant bug. When one device sets a detail whose relation link is missing (e.g. discussionId via ObjectAddDiscussion), every other online participant's client runs the link reconciler, sees "detail present, link missing", and emits a RelationAdd change under its own identity, then pushes it. Result: a single local edit produces phantom version-history entries authored by everyone in the space.

Diagnosed from a debug export of a chat object: 6× relationAdd discussionId emitted by 4 different identities within the same second, right after one device's detailsSet discussionId.

What changed

Per design decisions, the system is gutted centrally rather than editing 130+ call sites:

  • Stop writing relation-link data into trees: removed RelationAdd/RelationRemove change generation, the Apply-diff ObjectRelationsAmend/Remove events + undo, and snapshot population. No client emits these changes anymore — this fixes the bug.
  • Neutralize writers (zero caller churn): SetDetailAndBundledRelation → thin alias for SetDetail; AddRelationLinks / AddBundledRelationLinks → no-ops.
  • Remove the state relationLinks field + read accessors (Pick/pick/getRelationLinks), strip link logic from RemoveRelation/filterRelations, remove dead FetchRelationByLinks.
  • Backward compatible: kept pb types and made changeRelationAdd/changeRelationRemove no-op parsers so existing trees still load.
  • Tooling: debugtree -j now emits per-change identity + timestamp (used to trace authorship).

Net −527 lines. Full inventory and execution log: docs/GO-4284-relationlinks-removal.md.

Out of scope (separate, still-live)

model.ObjectType.RelationLinks (a type's recommended relations) is a different concept and is actively consumed (relationutils/objecttype.go, bundledobjecttype.go). Left in place; removing it needs migrating those consumers to read recommendedRelations details.

Verification

  • go build ./... → exit 0
  • Repo-wide test-binary compile (go test ./... -run NONE) → exit 0
  • state, objectlink, templateimpl package tests pass; edited files gofmt-clean

🤖 Generated with Claude Code

debugtree -j now includes the signing identity (account) and timestamp for
each change, so tree authorship can be traced per change. Used to diagnose
cross-participant relationLinks writes.
Object-level relationLinks (the model.RelationLink list on an object, distinct
from dataview relations) are no longer read by any client or by the local
indexer — relations are tracked by detail keys. They were kept alive only by
auto-add writers, which caused a cross-participant bug: when one device set a
detail whose relation link was missing (e.g. discussionId), every other online
participant's client independently emitted a RelationAdd change under its own
identity and pushed it, producing phantom history entries authored by everyone
in the space.

Changes:
- Stop writing relation-link data into trees: remove RelationAdd/RelationRemove
  change generation, the Apply-diff ObjectRelationsAmend/Remove events + undo,
  and snapshot population. No client emits these changes anymore.
- Neutralize writers with no caller churn: SetDetailAndBundledRelation is now a
  thin alias for SetDetail; AddRelationLinks / AddBundledRelationLinks are no-ops.
- Remove the state relationLinks field and read accessors (Pick/pick/get), strip
  link logic from RemoveRelation and filterRelations, remove dead
  FetchRelationByLinks.
- Keep pb types and make changeRelationAdd/Remove no-op parsers so existing
  trees still load (backward compatible).
- Update/remove obsolete tests.

The separate, still-live model.ObjectType.RelationLinks (a type's recommended
relations) is intentionally left in place; see docs/GO-4284-relationlinks-removal.md.
Follow-up from review of the relationLinks removal:

- history: drop isRelationsChange and filterLocalAndDerivedRelations* — they
  processed ObjectRelationsAmend/Remove events that are no longer produced now
  that relation-link change generation is gone. Fix DiffVersions tests: a
  legacy tree's relation-link add/remove changes no longer surface in diffs
  (only detail changes do), so the mixed case yields its 2 detail changes and
  the relation-link-only case yields none.
- smartblock.ResetToVersion: drop the GO-7217 bundled-relation-link guard. It
  is structurally obsolete because RelationRemove changes are never generated
  anymore, so a version restore can no longer emit one that wipes a detail.
- import: drop stale comment about auto-injected relationLinks.
@requilence

Copy link
Copy Markdown
Contributor Author

Mixed-version blast radius (old clients still write relationLinks)

Origin. A non-local relation written with plain SetDetail (no link) lands link-less. An old client that has a reconciler for that key re-adds the link and pushes a RelationAdd under its own identity → phantom co-author history entry. Pre-existing bug (e.g. discussionId via ObjectAddDiscussion); this PR just stops new clients writing links, so old clients hit "link missing" more often during the rollout window.

A storm needs BOTH: (1) link written absent, and (2) a reconciler that re-adds that specific key. Reconcilers are fixed lists only (RequiredInternalRelations + per-type RequiredInternalRelationKeys) — there is no generic detail→link scan.

Class Keys Storms?
Universal (every object) name, description, iconEmoji, iconImage, internalFlags Yes — once per object, only on a writing apply by an old client; converges
Space / system objects discussionId, hasChat, homepage, spaceUxType, guestKey, spaceView/workspace relations Bounded — 1 object per space, once on join
Object-specific recommendedRelations, featuredRelations, createdInContext, setOf, sourceObject No — not on any reconciler list → link stays absent, harmless

No loop / no amplification. Adding a link is idempotent: once any client pushes it, others see it present and stop. Worst case is a one-time concurrent burst of ≤(online old clients) per object, then quiescent.

No data risk. Links only, never detail values; new clients no-op incoming RelationAdd/RelationRemove; convergent across mixed versions.

Verdict: acceptable. Finite, self-clearing cosmetic history noise + tiny tree bloat during the mixed-version window; gone once old clients leave the space. The only multi-object effect is the 5 universal keys on new-version-created objects, once each.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant