fix(crdt): route CollectionSetDelta out of GetDelta instead of dropping it

[SAY-5]

Problem

internal/core/crdt/ipld_union.go declares a keyed IPLD union whose
arms are LWWDelta, DocCompositeDelta, CounterDelta, CollectionDelta,
CollectionSetDelta, CollectionDefinitionDelta, FieldDefinitionDelta.

CRDT.GetDelta() dispatches on whichever field is non-nil. Lines 70 and 72
both check the same field:

case c.CollectionDelta != nil:
    return c.CollectionDelta
case c.CollectionDelta != nil:        // copy-paste: should be CollectionSetDelta
    return c.CollectionSetDelta

The second arm is unreachable, so a CRDT carrying only CollectionSetDelta
falls through every case and returns nil. Any downstream code that calls
GetDelta() on such a block observes a nil Delta and silently skips or
mis-handles the merge.

The sibling GetPriority() at lines 93-94 already dispatches on
c.CollectionSetDelta != nil, so the IPLD schema and the priority path
are consistent — only GetDelta is broken.

Fix

Check CollectionSetDelta in the second arm, matching the schema and
GetPriority.

-    case c.CollectionDelta != nil:
+    case c.CollectionSetDelta != nil:
         return c.CollectionSetDelta

Fixes #4720

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR corrects a copy-paste bug in CRDT.GetDelta() where a duplicate case condition checking c.CollectionDelta != nil is fixed to correctly check c.CollectionSetDelta != nil. This ensures the method properly returns the CollectionSetDelta instead of falling through and returning nil.

Changes

Cohort / File(s)	Summary
CRDT Delta Retrieval Fix internal/core/crdt/ipld_union.go	Corrected duplicate case condition in GetDelta() method from c.CollectionDelta != nil to c.CollectionSetDelta != nil to properly return stored CollectionSetDelta instead of nil.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Fix CRDT.GetDelta() to return CollectionSetDelta when non-nil [#4720]	✅
Add unit test for CollectionSetDelta case [#4720]	❓	Test coverage not mentioned in the provided summary; unclear if test file changes are included.
Ensure regression tests for other six delta variants pass [#4720]	❓	Test execution status not documented in the provided summary.

Suggested reviewers

• shahzadlone
• fredcarle

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/core/crdt/ipld_union.go (1)

62-80: Consider adding a regression unit test.

Per the PR objectives, a unit test that constructs CRDT{CollectionSetDelta: &CollectionSetDelta{...}} and asserts GetDelta() returns the non-nil *CollectionSetDelta would guard against this class of copy-paste regression. A table-driven test covering all seven variants would also catch any similar fallthrough in the future.

Additionally, enabling a linter rule such as govet's duplicate-case detection or golangci-lint's dupl/gocritic dupBranchBody could catch identical switch conditions automatically.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/core/crdt/ipld_union.go` around lines 62 - 80, Add a regression unit
test for CRDT.GetDelta: create a table-driven test that iterates over each
variant (LWWDelta, DocCompositeDelta, CounterDelta, CollectionDelta,
CollectionSetDelta, CollectionDefinitionDelta, FieldDefinitionDelta), construct
a CRDT with only that field set (e.g. CRDT{CollectionSetDelta:
&CollectionSetDelta{...}}), call GetDelta() and assert the returned Delta equals
the non-nil concrete pointer for that variant (use pointer equality or type
assertion as appropriate); include at least one explicit test case for
CollectionSetDelta to prevent copy-paste regressions and enable a linter rule
such as govet/duplicate-case or golangci-lint gocritic/dupBranchBody to catch
identical switch branches automatically.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/core/crdt/ipld_union.go`:
- Around line 62-80: Add a regression unit test for CRDT.GetDelta: create a
table-driven test that iterates over each variant (LWWDelta, DocCompositeDelta,
CounterDelta, CollectionDelta, CollectionSetDelta, CollectionDefinitionDelta,
FieldDefinitionDelta), construct a CRDT with only that field set (e.g.
CRDT{CollectionSetDelta: &CollectionSetDelta{...}}), call GetDelta() and assert
the returned Delta equals the non-nil concrete pointer for that variant (use
pointer equality or type assertion as appropriate); include at least one
explicit test case for CollectionSetDelta to prevent copy-paste regressions and
enable a linter rule such as govet/duplicate-case or golangci-lint
gocritic/dupBranchBody to catch identical switch branches automatically.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e6f7dc43-99ba-4154-bff9-3cafcb795c6b

📥 Commits

Reviewing files that changed from the base of the PR and between 266896d and 4d1b6ec.

📒 Files selected for processing (1)

• internal/core/crdt/ipld_union.go

📜 Review details

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-03-27T08:24:16.083Z

Learnt from: islamaliev
Repo: sourcenetwork/defradb PR: 4661
File: http/middleware.go:93-93
Timestamp: 2026-03-27T08:24:16.083Z
Learning: When handling `client.ErrNotAuthorizedToPerformOperation` in the defradb repository, treat it as an unauthenticated/missing-identity condition (not insufficient permissions). Map it to HTTP 401 Unauthorized rather than 403 Forbidden; this mapping is intentional and consistent with prior behavior (pre-PR `#4661`).

Applied to files:

• internal/core/crdt/ipld_union.go

🔇 Additional comments (1)

internal/core/crdt/ipld_union.go (1)

72-73: Correct fix for the copy-paste bug.

The updated case correctly dispatches CollectionSetDelta and matches the handling in NewCRDT (Line 33), GetPriority (Line 93), and the IPLD schema (Line 55). Previously this branch was unreachable due to the duplicate c.CollectionDelta != nil condition, causing GetDelta() to return nil for CRDT blocks carrying only a CollectionSetDelta and silently skipping merges at internal/core/block/store.go (around Line 201).