Review findings & fixes — commit fbae138

All findings below have been fixed and pushed to this branch.

🔴 Critical — Bug

1. Race condition in _persist() (concurrent async persists)

_onReorder called _persist() unconditionally, even when a previous persist was still awaiting network responses. Two concurrent persists both held a reference to _items, both would call _resync() independently, and the _persisting guard in didUpdateWidget could be cleared by the first finishing persist while the second was still in-flight — allowing a stale gateway snapshot to overwrite the optimistic local state.

Fix: Added _pendingPersist flag and _schedulePersist(). If a persist is already running, the new drag is queued; _persist()'s finally block restarts one more persist with the latest _items instead of calling _resync().

2. _recomputeParents() silently reparents uncategorized channels on category drag

The inline drag list places uncategorized channels before all categories in _items. When a user dragged a category to position 0 (before the uncategorized section), the old _recomputeParents() walked the entire list and assigned every uncategorized channel's parentId to that category. _persist() then sent destructive parent_id PATCHes to the API, moving channels the user never touched.

Fix: _recomputeParents now takes the dragged _DragEntry as a parameter and only updates that entry's parentId based on the nearest preceding category. Category drags return early — the category and its children keep their original parentIds.

🟡 Medium — Code quality / maintainability

3. _pos logic duplicated three times

The position-parsing helper (handle int / num / String / null → 0) existed identically in _ChannelDragListState._pos, _ChannelReorderState._pos, and accord_channels.dart's _asInt.

Fix: Extracted to lib/features/channels/utils/channel_sort.dart as parseChannelPosition (and channelListSignature for the signature helper). Both view files now delegate to the shared function.

🟢 Low — Test coverage

4. No tests for the position-parsing / signature logic

The _pos type-coercion and _signatureOf change-detection code had no test coverage. These are the functions that determine whether a reorder triggers a backend PATCH.

Fix: Added test/features/channels/channel_sort_test.dart covering int / double / string / null / non-numeric position parsing and signature order-independence / field-sensitivity.

🟢 Low — Style

5. Multi-line doc comments on private classes

Three new private classes (_ChannelDragList, _DragEntry, _DragUpdate) had multi-line /// blocks explaining what the code does. Per project convention (CLAUDE.md: "one short line max", "Don't explain WHAT the code does"), these were trimmed to single-line summaries. The inline comment describing the ternary in _ChannelListState.build() was also removed for the same reason.

ℹ️ Not fixed — noted for awareness

• _buildChannelEntries (non-manager path) doesn't sort channels by position, while _flatten (manager path) does. This inconsistency pre-dates this PR; the channel controller's _sorted() provides channels in position order so both paths should render correctly in practice.
• The "Reorder channels" modal button in the header is still present alongside the new inline drag. Keeping both is arguably good for accessibility / keyboard users; removing the button would be a separate UX decision.
• onReorderItem semantics — the comment claims it passes a post-removal newIndex (different from the standard onReorder callback, which requires if (newIndex > oldIndex) newIndex--). This matches the code's usage and is consistent with Flutter's intent for onReorderItem. No change made, but worth a manual drag test to confirm correct end-positions.

Generated by Claude Code