Fix Bybit order-book stream: map depth to valid v5 spot tiers (1/50/200/1000) - #55
Merged
Conversation
Bybit v5 spot publishes order books only at depths 1/50/200/1000. The topic builder used the raw requested depth, so a non-tier depth (e.g. 20) produced an invalid topic that Bybit rejected (success:false), and no book updates were ever delivered. Round each requested depth UP to the nearest supported tier in BuildTopic, keeping RoutingKeyFor and Classify consistent since both derive from the same topic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…000 tier Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughBybit order book topic generation now maps requested depths to supported tiers, and unit tests were updated to cover rounding, null-depth defaulting, and routing-key consistency. ChangesBybit order book depth mapping
Review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/CryptoExchanges.Net.Bybit/Streaming/BybitStreamProtocol.cs`:
- Around line 156-163: Adjust MapOrderBookDepth in BybitStreamProtocol so it no
longer silently maps unsupported depths to 1000; instead, validate the requested
depth and reject values outside Bybit’s supported range with a failure path or
exception, while only rounding valid in-range inputs to the nearest allowed
topic depth. Keep the existing handling for null/default and the supported
buckets inside the method, but make sure inputs like 0, negative values, and
values above the maximum are surfaced as invalid rather than rewritten.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b59898b-7bb4-4fde-ba3a-0e2b7649dedb
📒 Files selected for processing (2)
src/CryptoExchanges.Net.Bybit/Streaming/BybitStreamProtocol.cstests/CryptoExchanges.Net.Bybit.Tests.Unit/Streaming/BybitStreamProtocolTests.cs
…tead of silent clamp CodeRabbit PR #55: depths above the deepest Bybit v5 spot tier (1000) were silently clamped DOWN to 1000, under-delivering a shallower book than requested with no signal. Since the mapping otherwise rounds UP, an unsatisfiable request (> 1000) now throws ArgumentOutOfRangeException, matching the file's existing unsupported-value style. Round-up for 1..1000 unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes Bybit v5 spot order-book WebSocket subscriptions by mapping requested depths to the nearest supported tier so subscriptions don’t silently fail due to invalid topics (e.g., orderbook.20.*).
Changes:
- Map non-tier order-book depths to valid Bybit v5 spot tiers (1/50/200/1000) when building the topic.
- Throw
ArgumentOutOfRangeExceptionfor depths above the maximum supported tier (1000) instead of subscribing to an invalid/unsatisfiable depth. - Add unit tests covering tier mapping, default-depth handling, and routing-key/topic parity.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/CryptoExchanges.Net.Bybit/Streaming/BybitStreamProtocol.cs | Centralizes order-book topic building to use a depth-tier mapper (1/50/200/1000) and rejects depths > 1000. |
| tests/CryptoExchanges.Net.Bybit.Tests.Unit/Streaming/BybitStreamProtocolTests.cs | Adds theory-based coverage for depth mapping and verifies RoutingKeyFor matches the subscribe topic for non-tier depths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…message Address Copilot nit on PR #55: the ArgumentOutOfRangeException for depth > 1000 now enumerates the discrete supported Bybit v5 spot tiers (1, 50, 200, 1000) instead of only stating the maximum, making the failure actionable. Same exception type, paramName, and actual value preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
morpheums
added a commit
that referenced
this pull request
Jun 27, 2026
Bump version 0.6.0-preview.2 -> 0.6.0-preview.3 and add the dated CHANGELOG section for the Bybit order-book WebSocket non-tier depth fix (PR #55), with comparison links. Build verified 0W/0E. Co-authored-by: Morpheums <morpheums@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes a shipped bug: Bybit order-book WebSocket delivered zero updates for any non-tier depth.
Root cause
BybitStreamProtocol.BuildTopicbuiltorderbook.{depth}.{symbol}from the raw requested depth. Bybit v5 spot only supports order-book depths 1 / 50 / 200 / 1000, so a consumer requesting e.g.depth: 20produced the invalid topicorderbook.20.BTCUSDT→ Bybit replies{"success":false}(classified as an error) → no book data, reconnect.Fix
Map the requested depth up to the nearest supported Bybit tier before building the topic (
20→50,5→50,null→50,1→1,200→200,500→1000, …). The consumer still gets at least the depth requested.RoutingKeyForandClassifystay consistent (both derive from the topic; Bybit echoes the subscribed topic in data frames).Testing
[Theory]covering all tier mappings + a routing-parity test (RoutingKeyFor(20) == subscribe topic == orderbook.50.BTCUSDT).dotnet build CryptoExchanges.Net.slnx0W/0E; non-integration suite green.20to directly proveorderbook.20.Xnow maps toorderbook.50.Xand delivers updates.Other exchanges, Core, and Http are untouched. Ships as v0.6.0-preview.3.
Summary by CodeRabbit
Bug Fixes
Tests