Skip to content

Fix silent map-directive drop: use Channel(UNLIMITED) in HomeViewModel (#1904)#1924

Merged
bmander merged 1 commit into
mainfrom
fix/1904-map-directives-unlimited-channel
Jul 17, 2026
Merged

Fix silent map-directive drop: use Channel(UNLIMITED) in HomeViewModel (#1904)#1924
bmander merged 1 commit into
mainfrom
fix/1904-map-directives-unlimited-channel

Conversation

@bmander

@bmander bmander commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #1904.

Problem

HomeViewModel._mapDirectives was a Channel(capacity = 16) written with trySend. #1865 introduced this Channel (replacing a replay-0 SharedFlow) precisely so a directive emitted while HOME's collector is resubscribing would queue instead of vanish. But trySend fails silently once the bounded channel fills — 17+ directives queued while the user is on a pushed destination reproduces the very drop bug the Channel was introduced to fix, unlogged.

Fix

Switch to Channel(capacity = Channel.UNLIMITED). Directives are tiny and low-frequency, so an unbounded buffer never drops and costs nothing. trySend can then only fail on a closed channel (which this VM never does), so it stays a correct synchronous emit — the comments are updated to reflect that.

Test

Added a regression test that emits 40 directives (past the old bound of 16) with no collector subscribed, then subscribes and asserts all 40 are delivered in order. It fails under capacity = 16 and passes under UNLIMITED.

Verification

./gradlew :onebusaway-android:testObaGoogleDebugUnitTest --tests "org.onebusaway.android.ui.home.HomeViewModelTest" -PwarningsAsErrors=true → BUILD SUCCESSFUL, all tests green, no compiler warnings.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Ensured map actions are reliably queued when the map is not actively listening.
    • Prevented queued route-focus actions from being lost during bursts of activity.
    • Preserved the order of map directives for consistent results.

#1904)

#1865 replaced a replay-0 SharedFlow with Channel(capacity = 16) so a
directive emitted while HOME's collector was resubscribing would queue
instead of vanish. But emitMapDirective uses trySend, which fails
silently once the bounded channel fills — 17+ directives queued while
the user is on a pushed destination reproduced the very drop bug the
Channel was introduced to fix, unlogged.

Switch to Channel(capacity = Channel.UNLIMITED): directives are tiny and
low-frequency, so an unbounded buffer never drops and costs nothing.
trySend can then only fail on a closed channel (which this VM never
does), so it stays a correct synchronous emit.

Add a regression test that emits 40 directives (past the old bound of
16) with no collector subscribed, then subscribes and asserts all 40 are
delivered in order — fails under capacity = 16, passes under UNLIMITED.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7076a839-405a-4038-a26b-b5bf6c0e9c76

📥 Commits

Reviewing files that changed from the base of the PR and between 01c0970 and 702a559.

📒 Files selected for processing (2)
  • onebusaway-android/src/main/java/org/onebusaway/android/ui/home/HomeViewModel.kt
  • onebusaway-android/src/test/java/org/onebusaway/android/ui/home/HomeViewModelTest.kt

📝 Walkthrough

Walkthrough

HomeViewModel now uses an unbounded channel for map directives, and a unit test verifies that directives emitted before collection are all retained and delivered in order.

Changes

Map directive delivery

Layer / File(s) Summary
Unbounded directive queue and regression coverage
onebusaway-android/src/main/java/org/onebusaway/android/ui/home/HomeViewModel.kt, onebusaway-android/src/test/java/org/onebusaway/android/ui/home/HomeViewModelTest.kt
_mapDirectives uses Channel.UNLIMITED, documentation reflects the updated trySend behavior, and a test verifies that 40 queued route directives are received in order.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main fix: preventing silent map-directive drops by switching to an unlimited channel.
Linked Issues check ✅ Passed The code switches _mapDirectives to Channel.UNLIMITED and adds a regression test, matching #1904’s requested fix.
Out of Scope Changes check ✅ Passed Changes stay focused on HomeViewModel and its test, with only related comment updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1904-map-directives-unlimited-channel

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

Comment @coderabbitai help to get the list of available commands.

@bmander
bmander merged commit 2397af9 into main Jul 17, 2026
3 checks passed
@bmander
bmander deleted the fix/1904-map-directives-unlimited-channel branch July 17, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HomeViewModel: mapDirectives trySend can still silently drop directives — the bug the Channel was introduced to fix

1 participant