user-service/roomclient/client_integration_test.go (1)

125-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add missing test parity with GetRoomsInfo coverage.

TestGetRoomsInfo_Integration also covers "no responder" (RPC failure) and "cross-site siteID routing" scenarios, but TestGetThreadRoomInfoBatch_Integration only covers happy path and errcode relay. Since GetThreadRoomInfoBatch has the identical siteID-routing pattern, mirroring the missing subtests would close a coverage gap on a newly introduced fan-out path used by GetThreadUnread.

✅ Suggested additional subtests

 	t.Run("errcode reply relayed", func(t *testing.T) {
 		nc := dial(t)
 		sub, err := nc.Subscribe(subject.ThreadRoomInfoBatch("site-a"), func(m otelnats.Msg) {
 			data, _ := json.Marshal(errcode.BadRequest("bad"))
 			_ = m.Msg.Respond(data)
 		})
 		require.NoError(t, err)
 		t.Cleanup(func() { _ = sub.Unsubscribe() })

 		_, err = New(nc, "site-a").GetThreadRoomInfoBatch(context.Background(), "site-a", []string{"tr1"})
 		var e *errcode.Error
 		require.True(t, errors.As(err, &e))
 		assert.Equal(t, errcode.CodeBadRequest, e.Code)
 	})
+
+	t.Run("no responder — returns error wrapping thread-room-info rpc", func(t *testing.T) {
+		nc := dial(t)
+		ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+		defer cancel()
+
+		_, err := New(nc, "site-a").GetThreadRoomInfoBatch(ctx, "site-a", []string{"tr1"})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "thread-room-info rpc")
+	})
+
+	t.Run("cross-site siteID routing — uses siteID param not c.siteID", func(t *testing.T) {
+		nc := dial(t)
+		sub, err := nc.Subscribe(subject.ThreadRoomInfoBatch("site-b"), func(m otelnats.Msg) {
+			out, _ := json.Marshal(model.ThreadRoomInfoBatchResponse{
+				Threads: []model.ThreadRoomInfo{{ThreadRoomID: "tr2", Found: true, LastMsgAt: 10, RoomType: model.RoomTypeChannel}},
+			})
+			_ = m.Msg.Respond(out)
+		})
+		require.NoError(t, err)
+		t.Cleanup(func() { _ = sub.Unsubscribe() })
+
+		got, err := New(nc, "site-a").GetThreadRoomInfoBatch(context.Background(), "site-b", []string{"tr2"})
+		require.NoError(t, err)
+		require.Len(t, got, 1)
+	})

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@user-service/roomclient/client_integration_test.go` around lines 125 - 159,
`TestGetThreadRoomInfoBatch_Integration` is missing the same coverage that
`TestGetRoomsInfo_Integration` already has: add subtests for the no-responder
RPC failure case and for cross-site `siteID` routing. Use
`New(...).GetThreadRoomInfoBatch`, `subject.ThreadRoomInfoBatch`, and the
existing `dial`/`Subscribe` test pattern to verify the request fails when no
subscriber exists and that a request sent with one site ID does not hit a
subscriber for another site ID.

user-service/service/threadunread_test.go (1)

50-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider a multi-site (2+ sites, mixed success/failure) test case run with -race.

Current failure test only exercises a single site. A test with ≥2 sites (one succeeding, one failing) run under -race would directly validate the per-goroutine results[i] indexing and rule out any loop-variable-capture regression, independent of Go version.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@user-service/service/threadunread_test.go` around lines 50 - 66, The current
GetThreadUnread_SiteFailureDegrades test in threadunread_test.go only covers one
site, so add a multi-site case that exercises both a successful and a failing
site in the same call. Use threadUnreadService.GetThreadUnread with multiple
ThreadSubRef entries and stub RoomClient.GetThreadRoomInfoBatch for at least two
site IDs so one returns data and one returns an error, then assert the mixed
results and unavailable sites. Run the new test under -race to validate the
per-goroutine results[i] handling and guard against loop-variable-capture
issues.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

user-service/service/threadunread_test.go (1)

132-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a size <= 0 case to cover chunkStrings' invalid-input branch. The if size <= 0 { size = len(ids) } path in chunkStrings is untested; a case like {name: "non-positive size", ids: []string{"a","b"}, size: 0, expected: [][]string{{"a","b"}}} closes the gap. As per coding guidelines, "Tests must cover: happy path, error paths, edge cases ..., and invalid input".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@user-service/service/threadunread_test.go` around lines 132 - 163, Add a test
case in TestChunkStrings to cover the invalid-input branch in chunkStrings where
size <= 0, using a non-positive size and asserting it falls back to returning
the full ids slice as a single chunk. Locate the fix in the chunkStrings test
table and ensure the new case exercises the existing size normalization behavior
alongside the current happy-path cases.

Source: Coding guidelines

docs/superpowers/specs/2026-07-01-user-thread-unread-rpc-design.md (1)

136-150: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Tag the data-flow fence with a language.

The fenced diagram currently has no language tag, which is what markdownlint is flagging. text (or a plain list) will keep the docs lint-clean.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/specs/2026-07-01-user-thread-unread-rpc-design.md` around
lines 136 - 150, The fenced data-flow diagram under the thread.unread RPC flow
is missing a language tag, causing markdownlint to fail. Update the fence around
the diagram in the spec so it uses an explicit language like text, or convert it
to a plain list, and keep the content unchanged while ensuring the markdown
remains lint-clean.

Source: Linters/SAST tools

docs/superpowers/plans/2026-07-01-user-thread-unread-rpc.md (1)

27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Unify the subject builder shape.

This plan still introduces ThreadRoomInfoBatchSubscribe, but the spec says to reuse ThreadRoomInfoBatch(siteID) for both publishing and handler registration. Keeping both names will drift the public subject API.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/plans/2026-07-01-user-thread-unread-rpc.md` around lines 27
- 30, The subject API is inconsistent because the plan adds both
ThreadRoomInfoBatch and ThreadRoomInfoBatchSubscribe, but the spec requires a
single ThreadRoomInfoBatch(siteID) shape for both publishing and handler
registration. Update pkg/subject/subject.go to expose only the unified
ThreadRoomInfoBatch builder and have room-service/handler.go registration use
that same symbol, while removing the extra ThreadRoomInfoBatchSubscribe
reference from the plan and any related call sites.

user-service/service/threadunread.go (1)

14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Magic batch-size constant duplicated across services without a shared source of truth.

threadInfoBatchChunk = 500 is a local guess at room-service's MAX_BATCH_SIZE (documented as "default 1000" in the comment). If room-service's configured batch limit is lowered below 500 in some deployment, this constant silently becomes wrong with no compile-time or config-time signal. Consider sourcing this limit from a shared pkg constant (or having the room-service batch RPC report/enforce it back to callers) rather than a comment-documented assumption.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@user-service/service/threadunread.go` around lines 14 - 16, The
threadInfoBatchChunk constant in threadunread.go is a duplicated batch-size
assumption with no shared source of truth. Update the ThreadRoomInfoBatch call
path to use a shared pkg constant or a value provided/enforced by room-service
instead of hardcoding 500, and make the batch sizing logic in the relevant
thread unread helper(s) derive from that centralized limit so it stays
consistent across deployments.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes