fix: resolve collaborators presence panel flicker (#523)

[Chris0Jeky]

Summary

• Root cause: onMounted reset presenceMembers to [], then waited for the async SignalR BoardJoined push to arrive before showing the current user — creating a flicker window of several seconds.
• Fix: seed presenceMembers with the current authenticated user immediately on mount (and on board-switch) using useSessionStore, before fetchBoard or realtime.start are called. The SignalR snapshot arriving later simply replaces the seed with the authoritative server list.

Closes #523

Test plan

• TypeScript typecheck passes (npm run typecheck)
• All 1107 unit tests pass (npx vitest --run)
• Two new targeted tests added to BoardView.spec.ts:
  • seeds presence with the current user immediately on mount so the panel never flickers to empty
  • seeds presence with empty array when no user session is active
• Manual: Navigate to any board — presence panel should show the current user immediately, not flicker to "No active collaborators" first

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Self-review: LGTM

Root cause confirmed: onMounted was explicitly setting presence to [], then awaiting two async operations (fetchBoard + realtime.start) before the SignalR BoardJoined push could fire — creating a multi-second empty-state window.

Fix is minimal and surgical:

• currentUserPresenceSeed() reads sessionStore.userId/username synchronously (always available post-login). Returns [] gracefully when no session exists.
• Seed is applied before fetchBoard and realtime.start, eliminating the race window entirely.
• When the server's authoritative BoardPresenceSnapshot arrives (via onPresenceChanged), it replaces presenceMembers.value wholesale — no duplicate entry, no sticky seed.
• Board-switch path (watch on route.params.id) receives the same treatment.

Potential concerns checked:

• Unauthenticated / demo user with null userId: returns [], preserving old behavior.
• Username null: ?? null ensures type safety.
• Double-entry: server snapshot always replaces; no accumulation possible.
• TypeScript: zero errors.
• All 1107 unit tests pass; 2 new targeted tests added and green.

[gemini-code-assist]

Code Review

This pull request addresses a UI flicker issue (#523) where the collaborator panel would briefly appear empty before the SignalR connection was established. It introduces a currentUserPresenceSeed function to immediately populate the presence list with the current user's session data upon mounting or switching boards. Feedback includes suggestions to improve type safety in tests by avoiding 'as unknown' casts and a recommendation to deduplicate the presence seeding logic in the main view component.

[gemini-code-assist]

To avoid the type casting with as unknown as string in your test case for an unauthenticated user, you could define the type of mockSessionStore more explicitly to allow null values for userId and username, which reflects the actual store's state possibilities.

Suggested change

	const mockSessionStore = reactive({
	userId: 'user-abc',
	username: 'alice',
	})
	const mockSessionStore = reactive<{ userId: string | null; username: string | null }>({
	userId: 'user-abc',
	username: 'alice',
	})

[gemini-code-assist]

If you apply the suggestion to add explicit types to mockSessionStore, you can remove the as unknown as string casts here, making the test cleaner.

Suggested change

	mockSessionStore.userId = null as unknown as string
	mockSessionStore.username = null as unknown as string
	mockSessionStore.userId = null
	mockSessionStore.username = null

[gemini-code-assist]

This block of code to seed the presence members is duplicated in the watch block on lines 127-129. To improve maintainability and follow the DRY (Don't Repeat Yourself) principle, you could extract this logic into a separate function and call it from both places.

[Chris0Jeky]

Adversarial Review Pass 2

Checks run: typecheck (vue-tsc, zero errors), vitest unit tests (1109/1109 passed)

Findings:

1. Stale seed — No issue. Router guards ensure the user is authenticated before BoardView mounts, so sessionStore.userId is always populated. The ?? null fallback on username is correct. When the authoritative BoardPresenceSnapshot arrives, onPresenceChanged replaces presenceMembers.value wholesale — stale seed is cleanly overwritten.

2. Duplicate current user — No issue. onPresenceChanged does a full array replace (presenceMembers.value = snapshot.members), so there is no accumulation path.

3. Board-switch correctness — No issue. The watch clears and re-seeds synchronously before realtime.switchBoard, and the snapshot.boardId !== boardId.value guard in onPresenceChanged blocks any late-arriving snapshot from the previous board.

4. setBoardPresenceMembers side effects — None. Confirmed: boardUiStore.ts is a pure state.boardPresenceMembers.value = members assignment with no watchers, events, or persistence.

5. Offline/no-server — The seeded [{current user}] remains if SignalR never fires. This is an improvement over showing "No active collaborators" — the current user is a legitimate collaborator. Acceptable UX.

6. Test coverage — Mount-time seeding covered by both new tests. Board-switch (watch path) is not directly tested, but the code is identical — same currentUserPresenceSeed() call. Minor gap, not blocking.

7. Scope — Changes limited to BoardView.vue and BoardView.spec.ts. Clean.

8. TypeScript — Zero type errors. sessionStore.userId/username are Pinia store refs exposed as plain values; the seed object exactly matches BoardPresenceMember { userId: string; displayName: string | null; editingCardId: string | null }.

9. Minimal fix — One 8-line helper function plus 4 changed lines in two call sites. Appropriately surgical.

Minor note (non-blocking): The test uses null as unknown as string to simulate a missing session. Functional but slightly awkward; a typed string | null field on the mock would be cleaner. Not worth blocking the PR over.

Fixes applied: None needed — no functional bugs found.

Verdict: LGTM. The fix is correct, safe, and well-tested. Ready to merge.

[Chris0Jeky]

Addressed all three Gemini review comments:

1. Explicit nullable type on mockSessionStore — typed as reactive<{ userId: string | null; username: string | null }> to accurately reflect the store's state possibilities.
2. Removed as unknown as string casts — clean null assignments now that the type allows it.
3. Extracted duplicate seed block — introduced applyPresenceSeed() helper called from both onMounted and the board-switch watch, eliminating the 3-line duplication.