Code Review — findings & fixes

All fixes are committed to the review branch (be5e8da).

🔴 Critical — ServerUnreachable flashes on every cold start

File: lib/features/events/controllers/connection.dart

isUnreachable was defined as !isReachable, and disconnected was NOT in the isReachable set. ConnectionController's initial state is disconnected (before any gateway event fires). This meant:

1. App launches, connectionControllerProvider == disconnected
2. User opens a space → members == null (not loaded yet)
3. disconnected.isUnreachable == true → ServerUnreachable renders immediately instead of the loading spinner

The same false-positive applied to the channel list in _ChannelList.

Fix: Added disconnected to isReachable:

bool get isReachable =>
    this == ConnectionStatus.disconnected ||  // ← initial / pre-connect state
    this == ConnectionStatus.connecting ||
    this == ConnectionStatus.connected ||
    this == ConnectionStatus.ready;

reconnecting remains the only isUnreachable state, which is correct — it's the only status that means "we were connected and now the gateway is actively failing to recover."

🔴 Critical — Missing imports cause compile error in accord_home_channels.dart

File: lib/features/spaces/views/accord_home.dart

accord_home_channels.dart is a part file — it shares accord_home.dart's library scope. It uses connectionControllerProvider (from events/controllers/connection.dart) and ServerUnreachable (from shared/components/server_unreachable.dart), but neither was imported in accord_home.dart. Dart part files cannot have their own import directives; they rely entirely on the parent library's imports. Both symbols would be undefined at compile time.

Fix: Added the two missing imports to accord_home.dart.

🟡 Medium — Opacity snap instead of animated transition in _SpaceIcon

File: lib/features/spaces/views/accord_home_rail_tiles.dart

The rail wraps AnimatedContainer (120 ms corner-radius and color animation) in a plain Opacity widget. When a server goes offline or comes back online the opacity jumps instantaneously, which looks jarring next to the already-animated border-radius morph.

Fix: Replaced Opacity with AnimatedOpacity(duration: 300ms).

🟡 Medium — Duplicated retry callback

Files: accord_home_channels.dart, accord_member_list.dart

The identical onRetry callback (read accordAuthProvider, cast to AccordAuthLoggedIn, call ensureConnected()) is copy-pasted in both callers. No fix applied in this pass to keep the diff minimal, but extracting it to a small private helper (or a ConsumerWidget wrapper over ServerUnreachable) would remove the duplication.

🟢 Tests added

Two new test files:

test/features/events/connection_status_test.dart

• Covers every value of ConnectionStatus for both isReachable and isUnreachable.
• Includes an explicit regression test that disconnected.isUnreachable == false (the cold-start bug from finding fix: use explicit type for Image.create() to fix export build #1).

test/features/server/space_cache_test.dart

• save / load round-trip with id and name preservation.
• Overwrite semantics, multi-key isolation, empty-list save.
• remove for a present key and a missing key (no-throw).
• clear wipes all entries; no-op on empty cache.
• Load resilience: non-string stored value, non-list JSON, mixed-type JSON array with good/bad entries.

✅ What looks good

• SpaceCache is well-structured: best-effort with logged exceptions, clean static API, JSON encoding chosen deliberately over Hive's lossy nested-map typing (good call).
• ConnectionStatusReachability extension is a clean, testable abstraction.
• The setSpaces seed-before-client pattern in _addConnection correctly seeds the rail before the AccordClient is constructed, so cached spaces appear before the first gateway event.
• unawaited(SpaceCache.clear/remove) on logout/remove is appropriate — these are fire-and-forget cache hygiene operations.
• ServerUnreachable is a clean, stateless widget with sensible defaults.

Generated by Claude Code