You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(router): keep submission order within a priority lane in DispatchBatch - #63
DispatchBatch pops messages from a container/heap whose Less compares priority alone. The heap is not stable, so messages that share a priority were dispatched in an arbitrary order: P1 messages submitted as a, b, c, d, e ran as e, d, a, c, b. Less now breaks ties on the message's position in the batch, so each priority lane is first in, first out. DispatchBatchFor uses the same queue and gets the same fix.
Type of Change
feat — New feature
fix — Bug fix
docs — Documentation only
style — Formatting, no code change
refactor — Code restructuring without behavior change
test — Add or fix tests
build — Build system or dependency changes
ci — CI/CD pipeline changes
perf — Performance improvement
Changes
core/router.go:
priorityQueue.Less compares index (the position in the batch, already stored on each item) when two priorities are equal.
The DispatchBatch doc comment now states the order within a priority.
core/router_test.go: TestRouterBatchKeepsOrderWithinPriority dispatches nine messages across P0/P1/P2 and checks the exact handler order. The existing TestRouterBatchPriorityOrder is unchanged and still passes.
Testing
Unit tests pass (go test -race -count=1 ./...)
Integration tests pass (tests/ package passes; it does not use batches)
Manual testing performed: the new test against the unfixed and the fixed router.go
New tests added for new functionality
macOS arm64, go1.26.10 darwin/arm64 (GOTOOLCHAIN=local). Base 741690e, head 9dba84f.
Check
Command
Result
Fail before
git checkout origin/master -- core/router.go (keeping the new test), go test -count=1 -run TestRouterBatch -v ./core/
…Batch
DispatchBatch pops messages from a container/heap whose Less compared
priority alone. container/heap is not stable, so messages that share a
priority were handed to their handlers in an arbitrary order: five P1
messages submitted as a, b, c, d, e ran as e, d, a, c, b.
Break ties on the message's position in the batch, so each lane is
dispatched first in, first out. DispatchBatchFor shares the same queue
and gets the same fix. core gains a test that fails before this change
and passes after it.
Document submission order within each priority and use the batch index to break priority ties in the shared queue.
core/router.go L63-64, L111-118
Tests
Add a regression test verifying that messages dispatch in submission order within each priority lane while preserving priority ordering.
core/router_test.go L81-121
Tips and commands
Interacting with Sourcery
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
No actionable comments were generated in the recent review.
Walkthrough
DispatchBatch now dispatches messages in priority order and preserves their original batch order when priorities are equal. A new test checks the dispatch order and confirms that results and dispatches match the batch size.
Changes: Batch dispatch ordering
Layer / File(s)
Summary
Define and verify batch ordering core/router.go, core/router_test.go
priorityQueue.Less uses batch indices to order messages with equal priority. The documentation describes this ordering, and a test verifies it across P0, P1, and P2 messages. The test also checks result and dispatch counts.
embeddedos-org/eIPC#56: Adds DispatchBatchFor and shares the batch-dispatch helper whose priority queue ordering this change updates.
Merge Risk: Minimal · up to 9dba8
This change makes batch dispatch keep submission order among messages with the same priority, as the linked issue requests. No merge-blocking risk is evident.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the change: DispatchBatch now keeps submission (FIFO) order within each priority lane. The raw summary and PR objectives both describe a tie-break on batch index in priorityQueue.Less,…
Linked Issues check
Issue #62 requires DispatchBatch and DispatchBatchFor to dispatch by priority (P0, then P1, then P2) and to keep submission order within each priority. The whole-PR diff (mergeBase 741690e to head 9db…
Out of Scope Changes check
The whole-PR changes are limited to core/router.go and core/router_test.go. The router.go hunks are the Less tie-break and the DispatchBatch doc comment. Both directly implement #62. The test file cha…
Full details: Title check
Explanation
The title names the change: DispatchBatch now keeps submission (FIFO) order within each priority lane. The raw summary and PR objectives both describe a tie-break on batch index in priorityQueue.Less, with a matching test and documentation update. The title is specific and matches the main change, so it passes. It is a conventional-commit style title of 74 characters, within a typical length.
Full details: Linked Issues check
Explanation
Issue #62 requires DispatchBatch and DispatchBatchFor to dispatch by priority (P0, then P1, then P2) and to keep submission order within each priority. The whole-PR diff (mergeBase 741690e to head 9dba84f) changes priorityQueue.Less in core/router.go. Less now compares msg.Priority first and, when priorities are equal, compares the item index. Each item index is unique and equals the batch position, so the heap pops a strict total order. Both DispatchBatch and DispatchBatchFor call dispatchBatch, so one change covers both entry points. The issue's reproduction case should yield [p0-a p0-b p1-a p1-b p1-c p1-d p1-e p2-a p2-b]. The added test TestRouterBatchKeepsOrderWithinPriority covers this ordering. I did not read the test file body; its coverage rests on the PR description and the change summary, and the reported before/after run.
Full details: Out of Scope Changes check
Explanation
The whole-PR changes are limited to core/router.go and core/router_test.go. The router.go hunks are the Less tie-break and the DispatchBatch doc comment. Both directly implement #62. The test file change adds a single test for the same ordering, which supports the objective. No unrelated behavior, API, or platform code changes appear in the reviewed evidence. The PR description states that the public API is unchanged, and the diff confirms no signature changes in router.go.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 4ba7df53-db3f-4862-b568-02c3a186ed81
Commits
Reviewing files that changed from the base of the PR and between 741690e and 9dba84f.
Files selected for processing: 2
core/router.go
core/router_test.go
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Summary
DispatchBatchpops messages from acontainer/heapwhoseLesscompares priority alone. The heap is not stable, so messages that share a priority were dispatched in an arbitrary order: P1 messages submitted as a, b, c, d, e ran as e, d, a, c, b.Lessnow breaks ties on the message's position in the batch, so each priority lane is first in, first out.DispatchBatchForuses the same queue and gets the same fix.Type of Change
Changes
core/router.go:priorityQueue.Lesscomparesindex(the position in the batch, already stored on each item) when two priorities are equal.DispatchBatchdoc comment now states the order within a priority.core/router_test.go:TestRouterBatchKeepsOrderWithinPrioritydispatches nine messages across P0/P1/P2 and checks the exact handler order. The existingTestRouterBatchPriorityOrderis unchanged and still passes.Testing
go test -race -count=1 ./...)tests/package passes; it does not use batches)router.gomacOS arm64, go1.26.10 darwin/arm64 (
GOTOOLCHAIN=local). Base741690e, head9dba84f.git checkout origin/master -- core/router.go(keeping the new test),go test -count=1 -run TestRouterBatch -v ./core/dispatch order = [p0-a p0-b p1-e p1-d p1-a p1-c p1-b p2-a p2-b], exit 19dba84fTestRouterBatch*tests pass, exit 0go build ./...go vet ./...gofmt -l .go test -race -count=1 -p 1 ./...(CI has no-p 1; it is used here to run one package at a time)make build-allPre-Submission Checklist
go vetclean)<type>(<scope>): <description>convention741690e)Closing issue
Fixes #62
Screenshots / Logs
Raw logs:
raw-fail-before.log,raw-pass-after.log,raw-ci.log.Additional Notes
The order across priorities (P0 first) is unchanged. Only the order inside one priority changes, from heap order to submission order.
Summary by Sourcery
Ensure batch dispatches maintain FIFO ordering within each priority level without changing priority ordering across lanes.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit