Conversation
📝 Walkthrough""" Walkthroughこの変更セットは、Bitbank取引所のサポートを大幅に追加し、RESTおよびWebSocketクライアントの実装、関連データ構造、シリアライズ・デシリアライズ処理、テスト、ならびにPythonバインディングへの公開を行っています。また、環境変数や設定ファイルの取り扱い改善、既存取引所のAPIやテストの整理・拡張も含まれます。加えて、JupyterノートブックやPythonスクリプトによるBitbankのバックテスト・フォワードテスト例も新規追加されています。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bitbank
participant BitbankMarket
participant BitbankRestApi
participant BitbankPublicWsClient
User->>Bitbank: new()
User->>Bitbank: open_market("BTC/JPY")
Bitbank->>BitbankMarket: async_new(...)
BitbankMarket->>BitbankRestApi: DB/OrderBook初期化
User->>BitbankMarket: download(...)
BitbankMarket->>BitbankRestApi: get_board_snapshot()/get_recent_trades()
User->>BitbankMarket: open_market_stream()
BitbankMarket->>BitbankPublicWsClient: open_stream()
BitbankPublicWsClient-->>BitbankMarket: 市場データ配信
BitbankMarket-->>User: データ/イベント通知
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (1)
exchanges/bitbank/src/rest.rs (1)
237-268:⚠️ Potential issue
logdf_to_archivedfメソッドが未実装です。このメソッドは
todo!()を返しており、呼び出されるとパニックが発生します。
🧹 Nitpick comments (19)
test/bitbank_test/download.py (1)
3-3: 未使用のimportを削除してください静的解析ツールが指摘している通り、
init_logとinit_debug_logがimportされていますが使用されていません。以下の修正を適用してください：
-from rbot import init_log, init_debug_logもしくは、コメントアウトされているデバッグログ初期化を有効にする予定がある場合は、そのままでも構いません。
🧰 Tools
🪛 Ruff (0.11.9)
3-3:
rbot.init_logimported but unusedRemove unused import
(F401)
3-3:
rbot.init_debug_logimported but unusedRemove unused import
(F401)
doc/environment.md (2)
7-12: コードブロックに言語指定を追加してください。マークダウンのベストプラクティスに従い、コードブロックには言語を指定することを推奨します。
-``` +```text .env/rusty-bot/ └── BITBANK.env BINANCE.env BINANCE_TEST.env<details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 7-7: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> --- `19-22`: **ファイル名のテンプレート表記を明確にすることを検討してください。** コードブロック内で`{取引所名.env}`というテンプレート表記が使われていますが、実際のファイル名の例を示すか、より明確な表記にすることで理解しやすくなります。 例えば以下のような表記を検討してください： ```diff -```{取引所名.env} +```bash +# ファイル名: BITBANK.env API_KEY=取引所から提供されるAPIキー API_SECRET=取引所から提供されるAPIシークレット</blockquote></details> <details> <summary>exchanges/binance/src/ws.rs (2)</summary><blockquote> `90-91`: **ブール値の意図を明示してください** `false` をそのまま位置引数で渡すと可読性が低く、後から見た際に何のフラグか判断しづらくなります。 `socket_io_client: false` の意味が伝わるよう、 * ビルダー／構造体初期化式で名前付き引数にする * もしくは `const USE_SOCKET_IO: bool = false;` を用いる などの方法で自己説明的にすることを推奨します。 Also applies to: 191-192 --- `167-168`: **未使用フィールドはプレフィックスで回避を** `#[allow(dead_code)]` で抑制するより、構造体フィールド名を `_server` としておく方が明確で安全です。 </blockquote></details> <details> <summary>modules/rbot_session/src/runner.rs (1)</summary><blockquote> `926-931`: **借用は問題ありませんが変数は未使用です** `let session = py_session.borrow_mut(py);` を取得後すぐ `agent.call_method1` に渡すのみで、以後 `session` を利用していません。 以下のようにワンライナーで渡すとスコープが短くなり安全です。 ```diff -agent.call_method1("on_init", (session,))?; +agent.call_method1("on_init", (py_session.borrow_mut(py),))?;exchanges/bitbank/src/socket.rs (2)
26-37: 同期テストはブロッキングになります
標準スレッド +sleep()による待機はテスト時間を延ばし CI を不安定にします。
tokio::testとtokio::time::sleepを用いた非同期実装へ変更し、
タイムアウトを短く設定することを推奨します。
41-46: コメントと購読チャンネルが不一致です
コメントではBTC/JPY tradesとありますが、実際には"depth_diff_xrp_jpy"を購読しています。
誤解を避けるためコメントか引数を一致させてください。test/bitbank_test/nullagent.py (2)
2-2: 未使用のインポートを削除してください。静的解析で検出された未使用のインポートを削除する必要があります。
以下の差分を適用して未使用のインポートを削除してください：
-from rbot import Session from rbot import Runner from rbot import Bitbank -from rbot import init_debug_log from rbot import init_log -from rbot import OrderSide from rbot import time_string -from rbot import NOW, DAYSAlso applies to: 7-7, 9-9, 11-11
🧰 Tools
🪛 Ruff (0.11.9)
2-2:
rbot.Sessionimported but unusedRemove unused import:
rbot.Session(F401)
52-60: コメントアウトされたコードを削除してください。使用されていないコメントアウトされたコードは、可読性を低下させます。必要に応じてバージョン管理システムから復元できます。
コメントアウトされたコードブロックを削除することを推奨します。
exchanges/bybit/src/ws.rs (1)
99-99: 新しいパラメータの目的を明確にしてください
AutoConnectClient::newに追加されたfalseパラメータの目的が不明確です。このパラメータが何を制御するのか、コメントまたはドキュメントで説明を追加することをお勧めします。また、このPRの目的は「Bitbank public wsメッセージパーサーの追加」ですが、Bybitのファイルを変更する理由を説明してください。
modules/rbot_lib/src/db/archive.rs (1)
568-638: コメントアウトされたコードを削除してください行606-637にコメントアウトされた古い実装が残っています。これらのコードは混乱を招く可能性があるため、削除することをお勧めします。
CSVからParquetへの変換処理をAPIレイヤーに移動したことは、責任の分離の観点から良い設計変更です。
modules/rbot_lib/src/common/env.rs (1)
55-73: ログレベルの見直しを検討してください環境変数ファイルが見つからない場合（行55）は
warnレベルですが、キーが見つからない場合（行72）はerrorレベルになっています。一貫性の観点から、両方とも
warnレベルにするか、重要度に応じて適切に設定することをお勧めします。- log::error!("key [{}] is not found", key); + log::warn!("key [{}] is not found in env file", key);modules/rbot_lib/src/common/bar.rs (2)
53-58: CString の生成場所と無駄なコピーを整理しましょう
CString::newを GIL 外で呼び、その後as_c_str()をクロージャ内で参照しています。
この方式は動作しますが、毎回ヒープを確保する上に３メソッドで重複しています。
py_script.as_bytes()を直接渡せばCString不要- 共通処理をヘルパー関数に抽出すると可読性と保守性が向上します。
148-152: コード重複の解消を検討してください
PyFileBar::initでも同様のCString生成／from_code呼び出しが繰り返されています。
共通ユーティリティにまとめることでバグ混入を防ぎ、今回のような不一致（PyRunningBarの型間違い）も防げます。modules/rbot_lib/src/common/ccxt_config.rs (1)
150-175: データ正規化の重複を回避するヘルパーの導入を提案
get_market_config_by_native_symbolとget_market_configで
同一のFeeType判定・MarketConfig::new呼び出しが重複しています。
共通関数に切り出すことで将来的なパラメータ追加時の修正漏れを防げます。test/bitbank_test/basilbot_backtest.ipynb (3)
134-179: BasilAgentクラスの実装について改善提案があります実装は論理的に正しいですが、以下の改善を推奨します：
- ポジション判定のマジックナンバー（0.001）は
ORDER_SIZEの一部として定義すべきですclass BasilAgent: def __init__(self): self.OFFSET = 0.00_05 self.EXPIRE_TIME = 600 # 600[sec] = 10[min] self.ORDER_SIZE = 0.01 self.RANGE = 300 + self.MIN_POSITION_THRESHOLD = self.ORDER_SIZE / 10 # 最小ポジション閾値 # ... 省略 ... - if session.position <= 0.001: # ポジションが少ない場合は買い注文を出す。 + if session.position <= self.MIN_POSITION_THRESHOLD: # ポジションが少ない場合は買い注文を出す。 order_price = ohlcv['close'][-1] * (1 - self.OFFSET) - print("BUY ORDER: ", order_price, self.ORDER_SIZE) + session.log_info(f"BUY ORDER: price={order_price}, size={self.ORDER_SIZE - session.position}") session.limit_order('Buy', order_price, self.ORDER_SIZE - session.position) else: # ポジションがある場合は売り注文を出す。 order_price = ohlcv['close'][-1] * (1 + self.OFFSET) - print("SELL ORDER: ", order_price, session.position) + session.log_info(f"SELL ORDER: price={order_price}, size={session.position}") session.limit_order('Sell', order_price, session.position)
188-206: MyAgentクラスのクリーンアップが必要です未使用の変数とコメントアウトされたコードを整理することを推奨します。
-MARGIN = 0.05 class MyAgent: def __init__(self): self.last_update = 0 self.order_size = 0.001 def on_clock(self, session, clock): pass def on_tick(self, session, side, price, size): pass -# def on_update(self, session, updated_order): -# print("on_update", updated_order) def on_account_update(self, session, account): print("account update: ", session.current_time, account)
528-568: 可視化コードの重複を削減することを推奨しますバックテストとフォワードテストで同じ可視化コードが重複しています。共通関数として抽出することを推奨します。
以下のような共通関数を作成することを提案します：
def create_trading_visualization(orders, ohlcv, log, title="Trading Result"): """トレード結果の可視化を行う共通関数""" import plotly.graph_objects as go from plotly.subplots import make_subplots fig = make_subplots(rows=4, cols=1, shared_xaxes=True, vertical_spacing=0.1, row_heights=[0.1, 0.1, 0.1, 0.6], subplot_titles=("range", "psudo-position", "psudo-profit", "candlestick")) # 共通の可視化ロジック # ... (既存のコードを関数化) fig.update_layout(height=800, title_text=title) return figAlso applies to: 645-686
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
Cargo.tomlis excluded by!**/*.tomlexchanges/binance/Cargo.tomlis excluded by!**/*.tomlexchanges/bitbank/Cargo.tomlis excluded by!**/*.tomlmodules/rbot_lib/Cargo.tomlis excluded by!**/*.tomlmodules/rbot_lib/src/common/exchange.jsonis excluded by!**/*.jsonpackage-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackage.jsonis excluded by!**/*.json
📒 Files selected for processing (34)
doc/environment.md(1 hunks)exchanges/binance/src/config.rs(1 hunks)exchanges/binance/src/rest.rs(0 hunks)exchanges/binance/src/ws.rs(6 hunks)exchanges/bitbank/src/lib.rs(1 hunks)exchanges/bitbank/src/market.rs(1 hunks)exchanges/bitbank/src/message.rs(2 hunks)exchanges/bitbank/src/rest.rs(7 hunks)exchanges/bitbank/src/socket.rs(1 hunks)exchanges/bitbank/src/ws.rs(1 hunks)exchanges/bitflyer/src/market.rs(1 hunks)exchanges/bybit/src/config.rs(1 hunks)exchanges/bybit/src/market.rs(3 hunks)exchanges/bybit/src/rest.rs(1 hunks)exchanges/bybit/src/ws.rs(2 hunks)modules/rbot_lib/src/common/bar.rs(4 hunks)modules/rbot_lib/src/common/ccxt_config.rs(4 hunks)modules/rbot_lib/src/common/config.rs(1 hunks)modules/rbot_lib/src/common/env.rs(6 hunks)modules/rbot_lib/src/common/order.rs(1 hunks)modules/rbot_lib/src/db/archive.rs(2 hunks)modules/rbot_lib/src/db/df.rs(2 hunks)modules/rbot_lib/src/net/ws.rs(15 hunks)modules/rbot_market/src/market.rs(2 hunks)modules/rbot_session/src/runner.rs(2 hunks)modules/rbot_session/src/session.rs(2 hunks)python/rbot/__init__.py(2 hunks)src/lib.rs(2 hunks)test/00_suite/10_ccxt/test_ccxt_config_generate.py(1 hunks)test/00_suite/test_00exchange.py(1 hunks)test/00_suite/test_01market_basic.py(2 hunks)test/bitbank_test/basilbot_backtest.ipynb(1 hunks)test/bitbank_test/download.py(1 hunks)test/bitbank_test/nullagent.py(1 hunks)
💤 Files with no reviewable changes (1)
- exchanges/binance/src/rest.rs
🧰 Additional context used
🪛 Ruff (0.11.9)
test/00_suite/test_00exchange.py
3-3: rbot.init_debug_log imported but unused
Remove unused import: rbot.init_debug_log
(F401)
python/rbot/__init__.py
5-5: rbot may be undefined, or defined from star imports
(F405)
6-6: rbot may be undefined, or defined from star imports
(F405)
test/bitbank_test/download.py
3-3: rbot.init_log imported but unused
Remove unused import
(F401)
3-3: rbot.init_debug_log imported but unused
Remove unused import
(F401)
test/bitbank_test/nullagent.py
2-2: rbot.Session imported but unused
Remove unused import: rbot.Session
(F401)
7-7: rbot.init_debug_log imported but unused
Remove unused import: rbot.init_debug_log
(F401)
9-9: rbot.OrderSide imported but unused
Remove unused import: rbot.OrderSide
(F401)
11-11: rbot.NOW imported but unused
Remove unused import
(F401)
11-11: rbot.DAYS imported but unused
Remove unused import
(F401)
🪛 markdownlint-cli2 (0.17.2)
doc/environment.md
7-7: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: linux-simple
🔇 Additional comments (46)
exchanges/bybit/src/config.rs (1)
4-4: 未使用のインポートの削除を承認
FeeTypeのインポートが削除されていますが、このファイル内ではコメントアウトされたコード内でのみ使用されており、アクティブなコードでは使用されていません。これは適切なコードクリーンアップです。modules/rbot_lib/src/common/config.rs (1)
126-126: Pythonバインディングの機能向上を承認
FeeTypeenumにeq_int属性が追加されました。これによりPythonでの等価比較が整数値ベースで実行可能になり、Pythonバインディングの使いやすさが向上します。他のenumでも同様の変更が行われており、一貫性のある改善です。python/rbot/__init__.py (2)
5-6: 型チェック警告の適切な抑制を承認star importsに起因する型チェック警告を
# type: ignoreコメントで適切に抑制しています。これは静的解析ツールのF405警告に対する正しい対処法です。🧰 Tools
🪛 Ruff (0.11.9)
5-5:
rbotmay be undefined, or defined from star imports(F405)
6-6:
rbotmay be undefined, or defined from star imports(F405)
35-35: コピーライト年の更新を承認コピーライト表示が2022-2025年に更新されました。これは適切な更新です。
src/lib.rs (2)
13-13: Bitbankクラスのインポートを承認PRの目的であるBitbank取引所サポートの一環として、
Bitbankクラスが適切にインポートされています。既存のexchangeクラスと同じパターンに従っています。
78-78: PythonバインディングへのBitbankクラス追加を承認
BitbankクラスがPythonモジュールに正しく追加されました。これによりPythonコードからBitbank取引所の機能にアクセス可能になります。既存のBinanceやBybitクラスと一貫したパターンで実装されています。test/00_suite/10_ccxt/test_ccxt_config_generate.py (1)
92-95: Bitbank ブランチの整形と情報不足を修正してください
public_ws_server/private_ws_server行が他のブランチより 4 スペース深く、読みやすさが低下します。historical_web_baseが空文字のままです。他の取引所同様、過去データのベース URL を設定するか、不要ならコメントで理由を補足してください。- elif self.exchange_name == 'bitbank': - public_ws_server = "wss://stream.bitbank.cc/socket.io/?EIO=4&transport=websocket" - private_ws_server = "wss://stream.bitbank.cc/v1/private" + elif self.exchange_name == 'bitbank': + historical_web_base = "https://public.bitbank.cc" # 例: 必要に応じ変更 + public_ws_server = "wss://stream.bitbank.cc/socket.io/?EIO=4&transport=websocket" + private_ws_server = "wss://stream.bitbank.cc/v1/private"[ suggest_nitpick ]
test/00_suite/test_00exchange.py (2)
3-3: 未使用のinit_debug_logを削除してください
init_debug_logは本テスト内で呼び出されておらず、Ruff の F401 警告が出ています。不要なら import を削除、使用する場合はテスト内で呼び出してください。
[ suggest_nitpick ]🧰 Tools
🪛 Ruff (0.11.9)
3-3:
rbot.init_debug_logimported but unusedRemove unused import:
rbot.init_debug_log(F401)
11-14: パラメータ化ケースの順序重複に注意
Binance(True)が 2 度出現しているように見えます（行 10 と 11）。重複するとテスト実行時間が増えるだけなので整理をご検討ください。
[ suggest_nitpick ]modules/rbot_lib/src/common/order.rs (1)
256-265:#[pyclass(eq_int)]追加時は Python 側の型互換性も確認を
eq_intにより Python でLogStatus == 0のような比較が可能になりますが、eqを付けない場合 Rust 同士の比較にしか対応しません。また、Python から整数への暗黙変換を行う__int__実装が無いので、必要なら追加をご検討ください。例:
#[pymethods] impl LogStatus { fn __int__(&self) -> u8 { *self as u8 } }[ request_verification ][ suggest_optional_refactor ]
modules/rbot_lib/src/db/df.rs (2)
5-5: 未使用のimportの削除が適切です
Pathのimportが削除され、実際に使用されているPathBufのみが残されています。コードの整理として良い変更です。
617-617: 不要なclone()呼び出しの削除によりパフォーマンスが向上します
rename()メソッドはselfを返すため、.clone()呼び出しは不要でした。この削除により無駄なメモリ割り当てが回避されます。exchanges/binance/src/config.rs (1)
6-7: importの整理により可読性が向上しています未使用のimportが削除され、実際に使用されている
MarketConfigとExchangeConfigのみが残されています。コードの整理として適切な変更です。test/bitbank_test/download.py (1)
7-15: テストの実装が適切ですBitbank APIの基本的な使用方法が適切に実装されています。データダウンロード機能の動作確認として有用なテストファイルです。
exchanges/bitbank/src/lib.rs (2)
3-5: 新しいモジュールの追加が適切に行われています
market、ws、socketモジュールが追加され、Bitbank取引所の機能が拡張されています。モジュール構成が論理的で整理されています。
9-11: 公開API拡張が適切に実装されています新しく追加されたモジュールが適切に再エクスポートされ、ライブラリの公開APIが拡張されています。Bitbank取引所の完全なサポートに必要な機能が提供されています。
modules/rbot_market/src/market.rs (1)
851-853: 時間範囲フィルタリングの追加は適切です。Klineから変換されたトレードデータが指定された時間範囲内にあることを保証する良い変更です。開始時刻を含み、終了時刻を含まない（半開区間）という一般的なパターンを使用しています。
modules/rbot_session/src/session.rs (1)
6-6: Pythonバインディングの改善が適切に実装されています。
IntoPyObjectのインポート追加と、ExecuteMode列挙型へのeq, eq_int属性の追加により、Python側での等価比較が可能になります。これらの変更はPythonとの相互運用性を向上させます。Also applies to: 28-28
exchanges/binance/src/ws.rs (1)
322-331: テストに検証を追加しましょう
test_make_connect_urlは URL を表示するだけでアサーションが無く、失敗を検知できません。
生成された URL が
"wss://"で始まるlistenKey=を含む
など最低限の条件をassert!で確認してください。
加えて、API キー未設定環境ではcreate_listen_keyが失敗し CI が落ちる可能性があります。
dotenvでキーが無い場合はreturn Ok(())として#[ignore]マークを付ける等の配慮を推奨します。exchanges/bybit/src/rest.rs (1)
168-170: 不要なmutを削除した判断に賛成です
tradesを変更していないため immut 化は適切で、将来の意図しない再代入も防げます。test/00_suite/test_01market_basic.py (3)
2-2: インポートの簡素化を承認します。設定オブジェクトから文字列識別子への移行により、よりシンプルなインターフェースになりました。
9-14: Bitbank取引所のテストケース追加を承認します。文字列識別子を使用した新しいパラメータ形式への移行とBitbank取引所のテストケース追加により、テストカバレッジが向上しています。
Also applies to: 41-46
51-51: init_debug_log()の呼び出し順序を確認してください。
init_debug_log()がopen_market()の後に呼び出されているため、マーケット初期化中のログが記録されない可能性があります。マーケット初期化時のログ出力が必要かどうか確認し、必要であれば以下のように修正してください：
def test_open_market_stream(exchange, config): + init_debug_log() market = exchange.open_market(config) - init_debug_log()exchanges/bitflyer/src/market.rs (1)
111-112:Details
✅ Verification successful
APIシグネチャの変更が他の取引所と一貫性があることを確認してください。
open_marketメソッドのパラメータが&MarketConfigから&strに変更されています。この変更により、インターフェースが簡素化されますが、他の取引所の実装と一貫性があることを確認する必要があります。以下のスクリプトで他の取引所の
open_marketメソッドのシグネチャを確認します：
🏁 Script executed:
#!/bin/bash # Description: 他の取引所のopen_marketメソッドのシグネチャを確認 # 各取引所のopen_marketメソッドを検索 ast-grep --pattern 'pub fn open_market(&self, $_) -> $_'Length of output: 58
前回の
ast-grepでは一致が検出できなかったため、ripgrepを使って他の取引所でもopen_marketメソッドのシグネチャを再検索します。
🏁 Script executed:
#!/bin/bash # exchanges/**/src/market.rs 内の pub fn open_market を検索 rg "pub fn open_market" -n exchanges/Length of output: 478
open_marketメソッドのシグネチャは他の取引所と一致しています– Bybit, Bitbank, Binance でも同様に
pub fn open_market(&self, symbol: &str) -> …
が使われていることを確認しました。この変更はインターフェースの簡素化であり、他の実装と一貫性が取れているため問題ありません。
modules/rbot_lib/src/common/env.rs (3)
23-23: 環境変数ファイルパスの変更を確認してください
RBOT_ENV_FILEが.rusty-bot/.envに設定されていますが、これは相対パスです。dot_env_reader関数ではhome_dirと結合されて使用されているため問題ありませんが、この変更が既存のユーザー設定に影響を与える可能性があります。移行ガイドやドキュメントの更新が必要かもしれません。
28-29: 大文字変換の追加は良い改善です
exchange_nameを大文字に変換することで、環境変数名の一貫性が保たれ、大文字小文字の違いによる設定ミスを防ぐことができます。
156-177: PyO3 APIの更新に対応した適切な変更です
PyModule::from_code_boundからPyModule::from_codeへの変更と、c_str!マクロの使用は、PyO3の新しいAPIに適切に対応しています。exchanges/bybit/src/market.rs (2)
106-114: 新しいclient_order_id引数の追加、良い改善ですPython から
Noneを指定可能になり、柔軟性が向上しました。
pyo3(signature=...)でデフォルト値を明示している点も 👍
121-128: market_order も同様に対応済み
limit_orderとインターフェースを揃えており、一貫性があります。modules/rbot_lib/src/net/ws.rs (4)
66-66: socket.ioクライアントサポートの追加を承認します
SimpleWebsocketとAutoConnectClient構造体にsocket_io_clientフィールドを追加することで、socket.ioプロトコル特有の制御メッセージを適切に処理できるようになります。Also applies to: 354-354
141-154: socket.io接続時の制御メッセージスキップ処理を承認しますsocket.ioクライアントモードでは、接続時の制御メッセージを適切にスキップする実装になっています。また、
message.len() > 0のチェックも追加されており、空のメッセージリストに対する安全性が向上しています。
258-269: close()メソッドの安全性改善を承認します
write_streamがSomeであることを確認してからクローズ処理を行うように修正されており、潜在的なパニックを防ぐ良い改善です。
743-743: テストコードの適切な更新を承認しますすべてのテストケースで
socket_io_clientパラメータにfalseを渡すことで、既存の動作を維持しています。Also applies to: 778-778, 807-807, 852-852
test/bitbank_test/basilbot_backtest.ipynb (1)
261-273:Details
✅ Verification successful
Jupyter環境の設定確認が必要です
IProgress not foundエラーが発生しています。これはJupyter環境の問題です。以下のコマンドを実行して、必要なパッケージをインストールしてください：
また、Jupyter環境で以下を実行してください：
!jupyter nbextension enable --py widgetsnbextensionAlso applies to: 349-361
🏁 Script executed:
#!/bin/bash # Jupyterウィジェットの依存関係を確認 pip list | grep -E "(ipywidgets|jupyter)" || echo "ipywidgets not installed" # 必要なパッケージをインストール pip install --upgrade ipywidgets jupyterLength of output: 19389
Jupyterウィジェット拡張機能を有効化してください
既に
ipywidgetsとjupyterのインストールは完了しているため、後続のエラー解消には nbextension の有効化が必要です。ターミナルで以下を実行してください：jupyter nbextension enable --py widgetsnbextension --sys-prefix• JupyterLab (4.x 以上) をご利用の場合、追加の labextension は不要です。
• 上記実行後はノートブックサーバーを再起動し、キャッシュクリアのためブラウザもリロードしてください。exchanges/bitbank/src/ws.rs (1)
57-67: socket.ioクライアントとしての設定を承認しますBitbankのWebSocketサーバーがsocket.ioプロトコルを使用しているため、
AutoConnectClientにtrueを渡す設定は適切です。exchanges/bitbank/src/message.rs (5)
15-17: タイムスタンプ変換関数は正しく実装されています。ミリ秒からマイクロ秒への変換が適切に行われています。
19-24: REST APIレスポンス構造体の定義は適切です。シンプルで明確な構造になっています。
52-67: 注文ステータスのマッピング関数は適切に実装されています。未知のステータスに対するエラーログ出力も含まれており、デバッグに役立ちます。
68-111: BitbankOrderからOrderへの変換実装は適切です。必要なフィールドが正しくマッピングされています。
333-478: テストモジュールは包括的で適切に実装されています。様々なメッセージタイプのパースをカバーしており、品質保証に貢献しています。
exchanges/bitbank/src/market.rs (2)
551-565: 時間範囲の調整ロジックは適切です。無効な開始時刻や古すぎるデータへのアクセスを防ぐ良い実装です。
482-482:Details
❌ Incorrect review comment
WebSocket接続のコードがコメントアウトされています。
public_ws.connect().awaitがコメントアウトされていますが、WebSocketクライアントの実装によっては明示的な接続が必要な場合があります。
🏁 Script executed:
#!/bin/bash # BitbankPublicWsClientのopen_stream実装を確認して、内部でconnectを呼んでいるか検証 ast-grep --pattern 'impl $_ for BitbankPublicWsClient { $$$ async fn open_stream($$$) { $$$ } $$$ }'Length of output: 114
さらなる検証が必要です。
async fn open_streamの定義箇所とBitbankPublicWsClient内でconnectを呼び出しているかを検索してください。
🏁 Script executed:
#!/bin/bash # open_streamの定義を探す rg "async fn open_stream" -n . # BitbankPublicWsClientの実装を探す rg "struct BitbankPublicWsClient" -n -A3 . # connect呼び出しを含む実装箇所を探す rg "BitbankPublicWsClient.*connect" -n .Length of output: 1048
さらなる情報収集のため、
BitbankPublicWsClientのopen_stream実装を直接確認します。
🏁 Script executed:
#!/bin/bash # impl BitbankPublicWsClientブロック内を表示 rg -n "impl .*BitbankPublicWsClient" -n -A20 -B5 exchanges/bitbank/src/ws.rsLength of output: 2062
BitbankPublicWsClientの実際のopen_stream実装内でconnectが呼ばれているかを確認するため、該当箇所を抜粋表示してください。
🏁 Script executed:
#!/bin/bash # exchanges/bitbank/src/ws.rs の pub async fn open_stream 実装を表示 rg -n "pub async fn open_stream" -n -A50 -B5 exchanges/bitbank/src/ws.rsLength of output: 4336
AutoConnectClientの実装を確認し、open_stream内でconnectが呼ばれているか調べます。
🏁 Script executed:
#!/bin/bash # AutoConnectClientのopen_stream実装を表示 rg -n "pub async fn open_stream" -n -A30 -B5 modules/rbot_lib/src/net/ws.rs # AutoConnectClientのconnect実装を表示 rg -n "async fn connect" -n -A20 -B5 modules/rbot_lib/src/net/ws.rsLength of output: 3655
async_start_market_streamメソッド内でpublic_ws.connect().awaitコメントアウト前後の処理を確認します。
🏁 Script executed:
#!/bin/bash # async_start_market_stream の定義と前後50行を表示 rg -n "async fn async_start_market_stream" -n -A50 -B5 exchanges/bitbank/src/market.rsLength of output: 2093
public_ws.connect() の明示的呼び出しは不要です
AutoConnectClient の
open_stream()内で最初のreceive_text()呼び出し時にself.connect().awaitが実行されるため、public_ws.connect().awaitを別途呼び出す必要はありません。このままコメントアウトで問題ありません。Likely an incorrect or invalid review comment.
exchanges/bitbank/src/rest.rs (4)
76-93:get_recent_tradesメソッドは適切に実装されています。エラーハンドリングも含めて正しく実装されています。
149-151: サポートされていない機能に対する警告は適切です。ユーザーに
client_order_idがサポートされていないことを明確に伝えています。
328-330: APIレスポンスのエラーチェックが適切に実装されています。
successフラグを確認してエラーを返す実装は良いプラクティスです。
441-615: テストモジュールは包括的でよく構成されています。主要なAPIエンドポイントがカバーされており、統合テストとして機能します。
| let kwargs = | ||
| vec![("connect_ws", true), ("verbose", self.verbose)]; | ||
|
|
||
| let kwargs = kwargs.into_py_dict_bound(py); | ||
| let kwargs = kwargs.into_py_dict(py)?; | ||
| market.call_method("download", (5, ), Some(&kwargs))?; |
There was a problem hiding this comment.
into_py_dict の戻り値に ? は不要では？
IntoPyDict::into_py_dict(py) は &PyDict を返し Result ではありません。
? を付けるとコンパイルエラーになります。
-let kwargs = kwargs.into_py_dict(py)?;
+let kwargs = kwargs.into_py_dict(py);コンパイルを確認してください。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let kwargs = | |
| vec![("connect_ws", true), ("verbose", self.verbose)]; | |
| let kwargs = kwargs.into_py_dict_bound(py); | |
| let kwargs = kwargs.into_py_dict(py)?; | |
| market.call_method("download", (5, ), Some(&kwargs))?; | |
| let kwargs = | |
| vec![("connect_ws", true), ("verbose", self.verbose)]; | |
| let kwargs = kwargs.into_py_dict(py); | |
| market.call_method("download", (5, ), Some(&kwargs))?; |
🤖 Prompt for AI Agents
In modules/rbot_session/src/runner.rs around lines 343 to 347, the code calls
into_py_dict(py) on a vector, which returns a reference to PyDict and not a
Result. The use of ? operator is incorrect here and will cause a compilation
error. Remove the ? operator from the line assigning kwargs to avoid this error
and ensure the code compiles correctly.
| bitbank.enable_order_with_my_own_risk = True | ||
|
|
||
| runner = Runner() | ||
| session = runner.real_run( | ||
| exchange=bitbank, market=market, agent=agent, verbose=True, execute_time=60*3, log_memory=True) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
実際の取引を行うテストは財務リスクがあります！
このテストはreal_run()を使用して実際の取引を実行しており、enable_order_with_my_own_risk = Trueが設定されているため、実際の資金損失が発生する可能性があります。
テスト環境では以下のいずれかを使用することを強く推奨します：
back_test(): 過去データでのバックテストdry_run(): 実際の注文を出さないドライラン
安全なテスト方法に変更してください：
-bitbank.enable_order_with_my_own_risk = True
-
-runner = Runner()
-session = runner.real_run(
- exchange=bitbank, market=market, agent=agent, verbose=True, execute_time=60*3, log_memory=True)
+# テスト環境ではドライランを使用
+runner = Runner()
+session = runner.dry_run(
+ exchange=bitbank, market=market, agent=agent, verbose=True, execute_time=60*3, log_memory=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bitbank.enable_order_with_my_own_risk = True | |
| runner = Runner() | |
| session = runner.real_run( | |
| exchange=bitbank, market=market, agent=agent, verbose=True, execute_time=60*3, log_memory=True) | |
| # テスト環境ではドライランを使用 | |
| runner = Runner() | |
| session = runner.dry_run( | |
| exchange=bitbank, market=market, agent=agent, verbose=True, execute_time=60*3, log_memory=True) |
🤖 Prompt for AI Agents
In test/bitbank_test/nullagent.py around lines 63 to 67, the code performs an
actual trading test using real_run() with enable_order_with_my_own_risk set to
True, which poses a financial risk. Replace the real_run() call with either
back_test() or dry_run() to ensure the test is safe and does not involve real
transactions. This change will prevent potential financial loss during testing.
| let py_script = CString::new(py_script.as_str()).unwrap(); | ||
|
|
||
| let py_module = | ||
| PyModule::from_code_bound(py, &py_script, "py_running_bar.py", "py_running_bar"); | ||
| PyModule::from_code(py, &py_script, c_str!("py_running_bar.py"), c_str!("py_running_bar")); | ||
|
|
There was a problem hiding this comment.
致命的: PyRunningBar::init はコンパイル不能です
PyModule::from_code の第 2 引数は &[u8] 互換ですが、現在渡している &CString は AsRef<[u8]> を実装していません。
そのためビルド時に型エラーになります。
- let py_module =
- PyModule::from_code(py, &py_script, c_str!("py_running_bar.py"), c_str!("py_running_bar"));
+ let py_module = PyModule::from_code(
+ py,
+ py_script.as_c_str(), // または `py_script.to_bytes()`
+ c_str!("py_running_bar.py"),
+ c_str!("py_running_bar"),
+ );他の２メソッド（RestBar / FileBar）が as_c_str() を使っている点とも統一してください。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let py_script = CString::new(py_script.as_str()).unwrap(); | |
| let py_module = | |
| PyModule::from_code_bound(py, &py_script, "py_running_bar.py", "py_running_bar"); | |
| PyModule::from_code(py, &py_script, c_str!("py_running_bar.py"), c_str!("py_running_bar")); | |
| let py_script = CString::new(py_script.as_str()).unwrap(); | |
| let py_module = PyModule::from_code( | |
| py, | |
| py_script.as_c_str(), // または `py_script.to_bytes()` | |
| c_str!("py_running_bar.py"), | |
| c_str!("py_running_bar"), | |
| ); |
🤖 Prompt for AI Agents
In modules/rbot_lib/src/common/bar.rs around lines 261 to 265, the code passes a
&CString to PyModule::from_code, but this function expects a &[u8] or similar
type, and &CString does not implement AsRef<[u8]>. To fix this, convert
py_script to a byte slice using py_script.as_bytes() before passing it to
PyModule::from_code. Also, update other methods (RestBar / FileBar) to use
as_c_str() for consistency.
| fn get_market_json_by_native_symbol(exchange_name: &str, category: &str, native_symbol: &str) -> anyhow::Result<MarketJson> { | ||
| let native_symbol = native_symbol.to_uppercase(); | ||
|
|
||
| let exchange_config = get_exchange_config(exchange_name)?; | ||
|
|
||
| for market in exchange_config.markets { | ||
| if market.trade_category == category && market.trade_symbol == native_symbol { | ||
| return Ok(market); | ||
| } | ||
| } | ||
|
|
||
| Err(anyhow!( | ||
| "not found market ({}) in exchange({})", | ||
| native_symbol, | ||
| exchange_name | ||
| )) |
There was a problem hiding this comment.
ネイティブシンボルの前後空白が処理されず、検索に失敗します
native_symbol.to_uppercase() だけでは " BTCUSDT" のような文字列を正規化できず、
JSON 内の "BTCUSDT" と一致しません。結果として本関数は常に Err を返し、
後続の get_market_config_by_native_symbol とテスト test_get_market_config_by_native_symbol が失敗します。
- let native_symbol = native_symbol.to_uppercase();
+ let native_symbol = native_symbol.trim().to_uppercase();同様に category も大小文字を無視した比較 (eq_ignore_ascii_case) を推奨します。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn get_market_json_by_native_symbol(exchange_name: &str, category: &str, native_symbol: &str) -> anyhow::Result<MarketJson> { | |
| let native_symbol = native_symbol.to_uppercase(); | |
| let exchange_config = get_exchange_config(exchange_name)?; | |
| for market in exchange_config.markets { | |
| if market.trade_category == category && market.trade_symbol == native_symbol { | |
| return Ok(market); | |
| } | |
| } | |
| Err(anyhow!( | |
| "not found market ({}) in exchange({})", | |
| native_symbol, | |
| exchange_name | |
| )) | |
| fn get_market_json_by_native_symbol( | |
| exchange_name: &str, | |
| category: &str, | |
| native_symbol: &str, | |
| ) -> anyhow::Result<MarketJson> { | |
| let native_symbol = native_symbol.trim().to_uppercase(); | |
| let exchange_config = get_exchange_config(exchange_name)?; | |
| for market in exchange_config.markets { | |
| if market.trade_category == category && market.trade_symbol == native_symbol { | |
| return Ok(market); | |
| } | |
| } | |
| Err(anyhow!( | |
| "not found market ({}) in exchange({})", | |
| native_symbol, | |
| exchange_name | |
| )) | |
| } |
🤖 Prompt for AI Agents
In modules/rbot_lib/src/common/ccxt_config.rs around lines 103 to 118, the
function get_market_json_by_native_symbol does not trim whitespace from
native_symbol before comparison, causing search failures when native_symbol has
leading or trailing spaces. To fix this, trim whitespace from native_symbol
using trim() and convert to uppercase, then compare with market.trade_symbol
also normalized similarly if needed. Additionally, consider using
eq_ignore_ascii_case for category comparison to ignore case differences. This
will ensure accurate matching regardless of whitespace or case variations.
| Message::Text(mut t) => { | ||
| // SOcket IO response | ||
| if t.starts_with("2") { // ping | ||
| log::warn!("message: {:?}", t); | ||
| self.send_text("3".to_string()).await; | ||
| continue; | ||
| } | ||
|
|
||
| if t.starts_with("0") { // connect | ||
| log::warn!("message: {:?}", t); | ||
| self.send_text("40".to_string()).await; // connect response connect | ||
| continue; | ||
| } | ||
| let mut t = t.to_string(); | ||
| if t.starts_with("42") { // event message | ||
| t = t.trim_start_matches("42").to_string(); | ||
| } | ||
|
|
||
| return Ok(ReceiveMessage::Text(t)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
socket.ioメッセージ処理に改善が必要です
socket.ioプロトコルの処理は正しく実装されていますが、以下の改善を推奨します：
log::warn!ではなくlog::debug!を使用すべきです（これらは通常の動作であり警告ではありません）- socket.io特有の処理は
self.socket_io_clientがtrueの場合のみ実行すべきです
Message::Text(mut t) => {
- // SOcket IO response
- if t.starts_with("2") { // ping
- log::warn!("message: {:?}", t);
- self.send_text("3".to_string()).await;
- continue;
- }
-
- if t.starts_with("0") { // connect
- log::warn!("message: {:?}", t);
- self.send_text("40".to_string()).await; // connect response connect
- continue;
- }
- let mut t = t.to_string();
- if t.starts_with("42") { // event message
- t = t.trim_start_matches("42").to_string();
- }
+ // Socket.IO response handling
+ if self.socket_io_client {
+ if t.starts_with("2") { // ping
+ log::debug!("Socket.IO ping: {:?}", t);
+ self.send_text("3".to_string()).await;
+ continue;
+ }
+
+ if t.starts_with("0") { // connect
+ log::debug!("Socket.IO connect: {:?}", t);
+ self.send_text("40".to_string()).await;
+ continue;
+ }
+
+ if t.starts_with("42") { // event message
+ t = t.trim_start_matches("42").to_string();
+ }
+ }
return Ok(ReceiveMessage::Text(t));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Message::Text(mut t) => { | |
| // SOcket IO response | |
| if t.starts_with("2") { // ping | |
| log::warn!("message: {:?}", t); | |
| self.send_text("3".to_string()).await; | |
| continue; | |
| } | |
| if t.starts_with("0") { // connect | |
| log::warn!("message: {:?}", t); | |
| self.send_text("40".to_string()).await; // connect response connect | |
| continue; | |
| } | |
| let mut t = t.to_string(); | |
| if t.starts_with("42") { // event message | |
| t = t.trim_start_matches("42").to_string(); | |
| } | |
| return Ok(ReceiveMessage::Text(t)); | |
| } | |
| Message::Text(mut t) => { | |
| - // SOcket IO response | |
| - if t.starts_with("2") { // ping | |
| - log::warn!("message: {:?}", t); | |
| - self.send_text("3".to_string()).await; | |
| - continue; | |
| - } | |
| - | |
| - if t.starts_with("0") { // connect | |
| - log::warn!("message: {:?}", t); | |
| - self.send_text("40".to_string()).await; // connect response connect | |
| - continue; | |
| - } | |
| - let mut t = t.to_string(); | |
| - if t.starts_with("42") { // event message | |
| - t = t.trim_start_matches("42").to_string(); | |
| - } | |
| + // Socket.IO response handling | |
| + if self.socket_io_client { | |
| + if t.starts_with("2") { // ping | |
| + log::debug!("Socket.IO ping: {:?}", t); | |
| + self.send_text("3".to_string()).await; | |
| + continue; | |
| + } | |
| + | |
| + if t.starts_with("0") { // connect | |
| + log::debug!("Socket.IO connect: {:?}", t); | |
| + self.send_text("40".to_string()).await; | |
| + continue; | |
| + } | |
| + | |
| + if t.starts_with("42") { // event message | |
| + t = t.trim_start_matches("42").to_string(); | |
| + } | |
| + } | |
| return Ok(ReceiveMessage::Text(t)); | |
| } |
🤖 Prompt for AI Agents
In modules/rbot_lib/src/net/ws.rs around lines 290 to 309, the socket.io message
handling logs messages at the warn level, which is inappropriate for normal
protocol messages, and does not check if the socket.io client mode is active.
Change the log level from warn to debug for these messages, and add a condition
to execute this block only if self.socket_io_client is true to ensure correct
protocol handling.
| let message = message.get(1).unwrap().clone(); | ||
| let room_name = message.get("room_name").unwrap().as_str().unwrap(); |
There was a problem hiding this comment.
unwrap()の使用によりパニックが発生する可能性があります。
メッセージ形式が期待と異なる場合の適切なエラーハンドリングが必要です。
- let message = message.get(1).unwrap().clone();
- let room_name = message.get("room_name").unwrap().as_str().unwrap();
+ let message = message.get(1)
+ .ok_or_else(|| anyhow::anyhow!("Missing message at index 1"))?
+ .clone();
+ let room_name = message.get("room_name")
+ .ok_or_else(|| anyhow::anyhow!("Missing room_name field"))?
+ .as_str()
+ .ok_or_else(|| anyhow::anyhow!("room_name is not a string"))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let message = message.get(1).unwrap().clone(); | |
| let room_name = message.get("room_name").unwrap().as_str().unwrap(); | |
| let message = message.get(1) | |
| .ok_or_else(|| anyhow::anyhow!("Missing message at index 1"))? | |
| .clone(); | |
| let room_name = message.get("room_name") | |
| .ok_or_else(|| anyhow::anyhow!("Missing room_name field"))? | |
| .as_str() | |
| .ok_or_else(|| anyhow::anyhow!("room_name is not a string"))?; |
🤖 Prompt for AI Agents
In exchanges/bitbank/src/message.rs at lines 261-262, the use of unwrap() on
message.get(1) and message.get("room_name") can cause panics if the values are
missing or not in the expected format. Replace unwrap() calls with proper error
handling, such as using match or if let to handle None or unexpected types
gracefully, and provide meaningful error messages or fallback behavior.
| impl Into<BoardTransfer> for BitbankDepth { | ||
| fn into(self) -> BoardTransfer { | ||
| let mut board = BoardTransfer::new(); | ||
| board.asks = self | ||
| .asks | ||
| .into_iter() | ||
| .map(|a| BoardItem { | ||
| price: Decimal::from_str(&a[0]).unwrap(), | ||
| size: Decimal::from_str(&a[1]).unwrap(), | ||
| }) | ||
| .collect(); | ||
| board.bids = self | ||
| .bids | ||
| .into_iter() | ||
| .map(|b| BoardItem { | ||
| price: Decimal::from_str(&b[0]).unwrap(), | ||
| size: Decimal::from_str(&b[1]).unwrap(), | ||
| }) | ||
| .collect(); | ||
| board | ||
| } | ||
| } |
There was a problem hiding this comment.
文字列からDecimalへの変換でunwrap()を使用すると、パニックが発生する可能性があります。
APIレスポンスの形式が変更された場合や不正なデータが含まれている場合、アプリケーションがクラッシュする恐れがあります。エラーハンドリングを追加することを推奨します。
- .map(|a| BoardItem {
- price: Decimal::from_str(&a[0]).unwrap(),
- size: Decimal::from_str(&a[1]).unwrap(),
- })
+ .map(|a| BoardItem {
+ price: Decimal::from_str(&a[0]).map_err(|e| anyhow::anyhow!("Failed to parse ask price: {}", e))?,
+ size: Decimal::from_str(&a[1]).map_err(|e| anyhow::anyhow!("Failed to parse ask size: {}", e))?,
+ })同様の変更をbidsの処理にも適用してください。ただし、IntoトレイトはResultを返せないため、TryFromトレイトの使用を検討してください。
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In exchanges/bitbank/src/message.rs lines 140 to 161, the code uses unwrap() on
the result of Decimal::from_str, which can cause a panic if the string is
invalid. Replace the unwrap() calls with proper error handling, such as using
try_from or returning a Result to handle potential parsing errors gracefully.
Since Into<BoardTransfer> cannot return a Result, consider implementing
TryFrom<BitbankDepth> for BoardTransfer instead to handle errors properly.
| let message: Vec<Value> = json5::from_str(message)?; | ||
|
|
||
| if let Some(event_type) = message.get(0).and_then(|v| v.as_str()) { | ||
| println!("Event type: {}", event_type); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
デバッグ出力にprintln!を使用しています。
本番環境ではlog::debug!やlog::trace!を使用することを推奨します。
- println!("Event type: {}", event_type);
+ log::debug!("Event type: {}", event_type);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| println!("Event type: {}", event_type); | |
| log::debug!("Event type: {}", event_type); |
🤖 Prompt for AI Agents
In exchanges/bitbank/src/message.rs at line 258, the code uses println! for
debugging output, which is not suitable for production. Replace println! with
log::debug! or log::trace! to enable configurable logging levels and better
control over debug information in production environments.
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
exchanges/bitbank/src/rest.rs (2)
236-268:⚠️ Potential issue
logdf_to_archivedfメソッドが未実装
todo!()マクロが使用されており、実行時にパニックが発生します。実装を完成させるか、明確なエラーメッセージを返すようにしてください。このメソッドの実装を支援しますか？BitbankのDataFrameフォーマットをアーカイブ形式に変換する実装を提供できます。
95-108: 🛠️ Refactor suggestion時間範囲でのトレード取得がサポートされていない
警告ログのみで空のベクターを返していますが、呼び出し側に明確にエラーを伝えるべきです。
- log::warn!("Bitbank does not support getting trades by time range"); - Ok((vec![], RestPage::Done)) + Err(anyhow::anyhow!("Bitbank does not support getting trades by time range. Use get_recent_trades() instead."))
♻️ Duplicate comments (5)
exchanges/bitbank/src/message.rs (4)
140-161: 文字列からDecimalへの変換でunwrap()を使用すると、パニックが発生する可能性があります。APIレスポンスの形式が変更された場合や不正なデータが含まれている場合、アプリケーションがクラッシュする恐れがあります。
TryFromトレイトの使用を検討してください。
163-168: JSONデシリアライゼーションでunwrap()を使用しています。データ形式が期待と異なる場合にパニックが発生します。
TryFromトレイトの使用を検討してください。
265-265: デバッグ出力にprintln!を使用しています。本番環境では
log::debug!やlog::trace!を使用することを推奨します。
268-269:unwrap()の使用によりパニックが発生する可能性があります。メッセージ形式が期待と異なる場合の適切なエラーハンドリングが必要です。
exchanges/bitbank/src/rest.rs (1)
213-224:get_accountメソッドが未完成です。現在は空のAccountCoinsを返していますが、実際のアカウントデータをパースする必要があります。
🧹 Nitpick comments (2)
exchanges/bitbank/src/socket.rs (1)
13-37: コメントアウトされたテストコードの削除使用されていないコメントアウトされたテストコードは削除することを推奨します。
exchanges/bitbank/src/rest.rs (1)
149-151: 未サポート機能の警告が不十分
client_order_idがサポートされていないことを警告していますが、呼び出し側にエラーとして通知すべきです。if let Some(id) = client_order_id { - log::warn!("client_order_id is not supported in bitbank"); + return Err(anyhow::anyhow!("client_order_id is not supported in Bitbank")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.tomlis excluded by!**/*.tomlexchanges/bitbank/Cargo.tomlis excluded by!**/*.toml
📒 Files selected for processing (3)
exchanges/bitbank/src/message.rs(2 hunks)exchanges/bitbank/src/rest.rs(7 hunks)exchanges/bitbank/src/socket.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: linux-simple
| use pubnub::PubNubClientBuilder; | ||
| use tracing::subscriber::NoSubscriber; | ||
|
|
||
| const SUBSCRIBER_KEY: &str = "sub-c-ecebae8e-dd60-11e6-b6b1-02ee2ddab7fe"; |
There was a problem hiding this comment.
セキュリティリスク：ハードコードされた認証情報
実際のサブスクライバーキーがハードコードされています。環境変数または設定ファイルから読み込むように変更してください。
-const SUBSCRIBER_KEY: &str = "sub-c-ecebae8e-dd60-11e6-b6b1-02ee2ddab7fe";
+const SUBSCRIBER_KEY: &str = env!("PUBNUB_SUBSCRIBER_KEY");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SUBSCRIBER_KEY: &str = "sub-c-ecebae8e-dd60-11e6-b6b1-02ee2ddab7fe"; | |
| -const SUBSCRIBER_KEY: &str = "sub-c-ecebae8e-dd60-11e6-b6b1-02ee2ddab7fe"; | |
| +const SUBSCRIBER_KEY: &str = env!("PUBNUB_SUBSCRIBER_KEY"); |
🤖 Prompt for AI Agents
In exchanges/bitbank/src/socket.rs at line 4, the subscriber key is hardcoded as
a string literal, which poses a security risk. Modify the code to load this key
from an environment variable or a configuration file instead of embedding it
directly in the source code. Use appropriate Rust crates or standard library
functions to read the environment variable or configuration at runtime and
assign it to the constant or variable accordingly.
| let publish_key = "my_publish_key"; | ||
| let subscribe_key = "my_subscribe_key"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
テスト用のプレースホルダー認証情報
実際のテストではプレースホルダーの値ではなく、適切なモック値または環境変数を使用してください。
- let publish_key = "my_publish_key";
- let subscribe_key = "my_subscribe_key";
+ let publish_key = std::env::var("TEST_PUBNUB_PUBLISH_KEY").unwrap_or_else(|_| "test_publish_key".to_string());
+ let subscribe_key = std::env::var("TEST_PUBNUB_SUBSCRIBE_KEY").unwrap_or_else(|_| "test_subscribe_key".to_string());🤖 Prompt for AI Agents
In exchanges/bitbank/src/socket.rs at lines 56 to 57, the publish_key and
subscribe_key are currently set to placeholder strings. Replace these hardcoded
placeholders with appropriate mock values for testing or retrieve the keys from
environment variables to ensure secure and flexible configuration during tests.
| for kline in candlestick.as_array().unwrap() { | ||
| let open = Decimal::from_str(&kline[0].as_str().unwrap()).unwrap(); | ||
| let high = Decimal::from_str(&kline[1].as_str().unwrap()).unwrap(); | ||
| let low = Decimal::from_str(&kline[2].as_str().unwrap()).unwrap(); | ||
| let close = Decimal::from_str(&kline[3].as_str().unwrap()).unwrap(); | ||
| let volume = Decimal::from_str(&kline[4].as_str().unwrap()).unwrap(); | ||
| let timestamp = bitbank_timestamp_to_microsec(kline[5].as_i64().unwrap()); |
There was a problem hiding this comment.
unwrap()の多用によるパニックリスク
Kline変換処理で複数のunwrap()を使用しています。不正なデータ形式に対する適切なエラーハンドリングが必要です。
- for kline in candlestick.as_array().unwrap() {
- let open = Decimal::from_str(&kline[0].as_str().unwrap()).unwrap();
- let high = Decimal::from_str(&kline[1].as_str().unwrap()).unwrap();
- let low = Decimal::from_str(&kline[2].as_str().unwrap()).unwrap();
- let close = Decimal::from_str(&kline[3].as_str().unwrap()).unwrap();
- let volume = Decimal::from_str(&kline[4].as_str().unwrap()).unwrap();
- let timestamp = bitbank_timestamp_to_microsec(kline[5].as_i64().unwrap());
+ let array = candlestick.as_array()
+ .ok_or_else(|| anyhow::anyhow!("candlestick is not an array"))?;
+ for kline in array {
+ let open = kline[0].as_str()
+ .ok_or_else(|| anyhow::anyhow!("open is not a string"))?;
+ let open = Decimal::from_str(open)
+ .map_err(|e| anyhow::anyhow!("Failed to parse open: {}", e))?;
+ // 同様に他のフィールドも処理Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In exchanges/bitbank/src/message.rs around lines 217 to 223, multiple unwrap()
calls are used on potentially invalid data, which can cause panics. Replace
these unwrap() calls with proper error handling by using match statements or the
? operator to propagate errors. Ensure that invalid data formats are gracefully
handled or returned as errors instead of panicking.
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (6)
exchanges/bitbank/src/ws.rs (2)
32-36: 購読チャンネルがハードコードされています – 前回指摘済み
94-121:println!ではなくlog系マクロを使用してください – 既出のレビューと同内容exchanges/bitbank/src/market.rs (1)
165-199:async_start_user_streamのtodo!()が残っています呼び出されると即時パニックします。以前のレビュー指摘と同一内容です。早急に実装するか、安全にエラーを返すようにしてください。
exchanges/bitbank/src/message.rs (3)
228-240:Decimal::from_str(...).unwrap()によるパニックリスクパース失敗でクラッシュします。過去指摘と同様です。
map_errでanyhow::Errorに変換してください。
345-351: デバッグ出力にprintln!と多重unwrap()本番コードでは
log::debug!を使用し、unwrap()は避けてください。こちらも過去に指摘済みです。
298-305: Kline 変換での連続unwrap()不正データでパニックします。安全なパースに置き換えてください。以前のコメントと重複のため詳細は省略します。
🧹 Nitpick comments (13)
test/bitbank_test/test_market_basic.py (1)
25-35: テスト内sleepは不要 & フレーク化の温床です
CI 上では標準出力がノイズになるうえ、sleep(1)やsleep(5)はネットワーク遅延で失敗したり無駄に時間を伸ばします。
assertに置き換え、必要ならcaplog/capfdで検証- 待ち合わせは
market.download(...)の戻り値や状態をポーリングする方法へテスト実行時間短縮と安定化に繋がります。
CLAUDE.md (2)
70-72: typo: “with in” → “within”
ドキュメント上の誤字です。検索性にも影響するため修正を推奨します。- - Recent(with in a day) historical data is stored in SQLite + - Recent (within a day) historical data is stored in SQLite
78-80: 英文の主語‐動詞一致の誤り
「Usually, its implement with WebSocket」→「Usually, it’s implemented with WebSocket」が自然です。test/00_suite/test_00exchange_order.py (3)
3-3: 未使用インポートの削除
Bybit,Binanceはこのテストで使用されていないため削除してください。-from rbot import Bybit, Binance, Bitbank, ExchangeConfig +from rbot import Bitbank, ExchangeConfig
12-17:is False/Trueより論理否定を使用
PEP8 E712 避けのため、以下のように書き換えを推奨します。- assert exchange.production is False + assert not exchange.production ... - assert exchange.enable_order_with_my_own_risk is True + assert exchange.enable_order_with_my_own_risk
37-38: 例外でスキップする場合は原因をログに残す
pytest.skipに加えpytest.xfailやloggingを使うと、外部 API の不調とコード不具合を区別しやすくなります。exchanges/hyperliquid/src/ws.rs (1)
50-66:add_paramsのパラメータ処理が堅牢でありません
params.get(1).unwrap_or(&"BTC".to_string())は"l2Book"だけ渡された場合に"BTC"を借用させつつ一時オブジェクトを生成しています。
・unwrap_or_else(|| "BTC".into())などでムダなString生成を避ける
・else分岐が無いので不正パラメータ時のエラーログを追加を検討してください。
exchanges/hyperliquid/src/message.rs (1)
72-88:level[2]の値チェックが曖昧です
is_bid判定を"1"固定で行っていますが、API 仕様変更時に壊れます。
明確にドキュメント化された値 ("bid"/"ask"など) かを確認し、列挙体や関数へ抽象化すると保守性が向上します。test/00_suite/test_bitbank_order.py (5)
15-18:True/Falseとの比較は直接ブール値を評価
スタイル上、== True / == Falseは避けて直接ブール値を評価した方が読みやすく、Ruff (E712) も回避できます。-assert(exchange.production == False) -assert(exchange.enable_order_with_my_own_risk == True) +assert not exchange.production +assert exchange.enable_order_with_my_own_risk
52-56: 同上: ブール値比較の簡潔化
下記も同様に修正してください。-assert(exchange.production == False) -assert(exchange.enable_order_with_my_own_risk == True) +assert not exchange.production +assert exchange.enable_order_with_my_own_risk
78-80: 未使用変数とブール比較
configはこのテストでは参照されておらず F841 が発生します。またブール比較も簡潔化してください。-config = BitbankConfig.BTCJPY -assert(exchange.production == False) +assert not exchange.productionAlso applies to: 98-99
20-22: テスト出力に
テスト内のpytestのキャプチャやloggingを用いた方が適切です。-print(order_result) -... -print(cancel_result) -... -print(order_result) -... -print(orders) -... -print(account)Also applies to: 32-34, 57-59, 80-82, 100-103
4-40: 外部 API 依存テストにはマークやモックを検討
実際に注文を発行・取消するテストはネットワークとアカウント状態に依存し不安定になりがちです。
@pytest.mark.integrationで分離するか、HTTP クライアントをモックしてユニットテスト化することを推奨します。
CI での再現性と速度が向上します。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
Cargo.tomlis excluded by!**/*.tomlexchanges/example/Cargo.tomlis excluded by!**/*.tomlexchanges/hyperliquid/Cargo.tomlis excluded by!**/*.tomlpyproject.tomlis excluded by!**/*.toml
📒 Files selected for processing (17)
CLAUDE.md(1 hunks)exchanges/bitbank/src/config.rs(1 hunks)exchanges/bitbank/src/lib.rs(1 hunks)exchanges/bitbank/src/market.rs(1 hunks)exchanges/bitbank/src/message.rs(2 hunks)exchanges/bitbank/src/rest.rs(7 hunks)exchanges/bitbank/src/socket.rs(1 hunks)exchanges/bitbank/src/ws.rs(1 hunks)exchanges/hyperliquid/src/lib.rs(1 hunks)exchanges/hyperliquid/src/market.rs(1 hunks)exchanges/hyperliquid/src/message.rs(1 hunks)exchanges/hyperliquid/src/rest.rs(1 hunks)exchanges/hyperliquid/src/ws.rs(1 hunks)test/00_suite/test_00exchange_order.py(1 hunks)test/00_suite/test_02_config.py(1 hunks)test/00_suite/test_bitbank_order.py(1 hunks)test/bitbank_test/test_market_basic.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- exchanges/hyperliquid/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- exchanges/bitbank/src/socket.rs
- exchanges/bitbank/src/lib.rs
🧰 Additional context used
🧬 Code Graph Analysis (3)
exchanges/bitbank/src/config.rs (3)
exchanges/bitbank/src/market.rs (2)
new(47-67)new(216-221)exchanges/bitbank/src/rest.rs (1)
new(28-32)modules/rbot_lib/src/common/config.rs (1)
open_exchange_market(53-55)
test/00_suite/test_00exchange_order.py (4)
modules/rbot_lib/src/common/config.rs (1)
open_exchange_market(53-55)test/00_suite/test_bitbank_order.py (4)
test_limit_order_and_cancel(4-39)test_market_order(41-67)test_open_orders(69-87)test_get_account(89-109)exchanges/bitbank/src/market.rs (4)
limit_order(91-103)cancel_order(118-124)market_order(106-116)get_open_orders(126-128)exchanges/bitbank/src/rest.rs (2)
cancel_order(174-194)test_get_account(544-552)
test/00_suite/test_02_config.py (5)
modules/rbot_lib/src/common/config.rs (3)
exchanges(58-60)open(48-50)open_market(80-82)exchanges/bitbank/src/market.rs (1)
open_market(74-78)exchanges/hyperliquid/src/market.rs (1)
open_market(101-104)exchanges/bybit/src/market.rs (1)
open_market(89-93)exchanges/binance/src/market.rs (1)
open_market(74-83)
🪛 Ruff (0.11.9)
test/00_suite/test_bitbank_order.py
1-1: pytest imported but unused
Remove unused import: pytest
(F401)
15-15: Avoid equality comparisons to False; use if not exchange.production: for false checks
Replace with not exchange.production
(E712)
18-18: Avoid equality comparisons to True; use if exchange.enable_order_with_my_own_risk: for truth checks
Replace with exchange.enable_order_with_my_own_risk
(E712)
52-52: Avoid equality comparisons to False; use if not exchange.production: for false checks
Replace with not exchange.production
(E712)
55-55: Avoid equality comparisons to True; use if exchange.enable_order_with_my_own_risk: for truth checks
Replace with exchange.enable_order_with_my_own_risk
(E712)
78-78: Avoid equality comparisons to False; use if not exchange.production: for false checks
Replace with not exchange.production
(E712)
98-98: Local variable config is assigned to but never used
Remove assignment to unused variable config
(F841)
99-99: Avoid equality comparisons to False; use if not exchange.production: for false checks
Replace with not exchange.production
(E712)
test/00_suite/test_00exchange_order.py
3-3: rbot.Bybit imported but unused
Remove unused import
(F401)
3-3: rbot.Binance imported but unused
Remove unused import
(F401)
test/00_suite/test_02_config.py
13-13: Found useless expression. Either assign it to a variable or remove it.
(B018)
13-13: Undefined name itewriwrite
(F821)
🪛 Pylint (3.3.7)
test/00_suite/test_02_config.py
[error] 13-13: Undefined variable 'itewriwrite'
(E0602)
🪛 LanguageTool
CLAUDE.md
[style] ~7-~7: Consider shortening or rephrasing this to strengthen your wording.
Context: ...he current state of the project. If you make changes to the codebase, please update this file a...
(MAKE_CHANGES)
[grammar] ~71-~71: Did you mean the adverb/preposition “within”?
Context: ...changed from SQLite in v0.4.0) - Recent(with in a day) historical data is stored in SQL...
(WITH_IN)
[uncategorized] ~78-~78: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ... request for exchange according to each exchanges documentation. - ws.rs implements rea...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~79-~79: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...me stream(public and private). Usually, its implement with WebSocket. ### Testing...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[uncategorized] ~79-~79: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...tream(public and private). Usually, its implement with WebSocket. ### Testing Structure...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~83-~83: You might be missing the article “the” here.
Context: ...Structure - Test files are organized in test/ directory - Exchange-specific tes...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: linux-simple
🔇 Additional comments (2)
exchanges/bitbank/src/config.rs (1)
15-17: ```shell
#!/bin/bash
echo "Showing top 50 lines of config.rs:"
head -n50 exchanges/bitbank/src/config.rs || trueecho -e "\nLooking for pyo3 in the file:"
grep -R "pyo3" -n exchanges/bitbank/src/config.rs || trueecho -e "\nSearching for PyResult in codebase:"
grep -R "PyResult" -n .</details> <details> <summary>exchanges/hyperliquid/src/rest.rs (1)</summary> `90-105`: **ボード取得エンドポイント `/info` は適切ですか？** 通常オーダーブックは `/l2Book` など専用エンドポイントを持つことが多く、`/info` では取得できないケースがあります。 API ドキュメントを再確認し、不一致なら URL とペイロードを修正してください。 </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| let stream = stream! { | ||
| loop { | ||
| match receiver.recv().await { | ||
| Ok(ReceiveMessage::Text(message)) => { | ||
| log::debug!("Hyperliquid Private WS received: {}", message); | ||
|
|
||
| if let Ok(ws_message) = serde_json::from_str::<HyperliquidUserWsMessage>(&message) { | ||
| match ws_message.channel.as_str() { | ||
| "user" => { | ||
| // Handle user updates (orders, fills, etc.) | ||
| // Parse and convert to MarketMessage | ||
| } | ||
| _ => { | ||
| log::debug!("Unknown private channel: {}", ws_message.channel); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
プライベート WS でも同様にメッセージ未送出
HyperliquidPrivateWsClient 側もパース後に yield が無く、ユーザーイベントが上位へ届きません。パブリック側と同様に修正が必要です。
🤖 Prompt for AI Agents
In exchanges/hyperliquid/src/ws.rs between lines 195 and 211, the
HyperliquidPrivateWsClient does not yield parsed user events after deserializing
the WebSocket message, so these events are not propagated upstream. To fix this,
add a yield statement that outputs the converted MarketMessage after
successfully parsing and handling the "user" channel message, similar to the
public WebSocket client implementation.
| let stream = stream! { | ||
| loop { | ||
| match receiver.recv().await { | ||
| Ok(ReceiveMessage::Text(message)) => { | ||
| log::debug!("Hyperliquid WS received: {}", message); | ||
|
|
||
| if let Ok(ws_message) = serde_json::from_str::<HyperliquidPublicWsMessage>(&message) { | ||
| match ws_message.channel.as_str() { | ||
| "l2Book" => { | ||
| // Handle order book updates | ||
| // Parse and convert to MarketMessage | ||
| } | ||
| "trades" => { | ||
| // Handle trade updates | ||
| // Parse and convert to MarketMessage | ||
| } | ||
| _ => { | ||
| log::debug!("Unknown channel: {}", ws_message.channel); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
メッセージが yield されずストリームが永遠に空になります
HyperliquidPublicWsClient::open_stream 内でパースが成功しても yield していないため、正常系の MultiMarketMessage が一切ストリームに流れません。
最低限、ws_message を MarketMessage/MultiMarketMessage に変換し yield Ok(...) する処理を追加してください。
🤖 Prompt for AI Agents
In exchanges/hyperliquid/src/ws.rs between lines 110 and 131, the stream loop
parses incoming WebSocket messages but never yields any results, causing the
stream to remain empty. To fix this, after successfully parsing ws_message and
converting it into MarketMessage or MultiMarketMessage, add a yield statement
that yields Ok(...) with the converted message so that the stream emits these
messages properly.
| fn to_string(&self) -> String { | ||
| return "".to_string(); | ||
| } |
There was a problem hiding this comment.
to_string が空文字を返しています
WsOpMessage::to_string は送信メッセージ生成で呼ばれる想定ですが、空文字列を返しており動作しません。serde_json::to_string(self)? などで正しい JSON を返してください。
🤖 Prompt for AI Agents
In exchanges/bitbank/src/ws.rs around lines 38 to 40, the to_string method
currently returns an empty string, which breaks the expected functionality of
generating a sendable message. Modify the to_string method to serialize self
into a JSON string using serde_json::to_string(self) and handle any potential
errors appropriately to return the correct JSON representation.
| let mut s = Box::pin(self.ws.open_stream().await); | ||
|
|
||
| stream! { |
There was a problem hiding this comment.
await の不要／誤用によりコンパイルエラーの可能性
self.ws.open_stream() が impl Stream を即時返す関数であれば await は不要です。
let mut s = Box::pin(self.ws.open_stream());
await を付けると Stream が Future として扱われず型不一致になります。
🤖 Prompt for AI Agents
In exchanges/bitbank/src/ws.rs around lines 81 to 83, remove the `await` after
`self.ws.open_stream()` because if `open_stream()` returns an `impl Stream`
directly, awaiting it causes a type mismatch error. Change the code to assign
`let mut s = Box::pin(self.ws.open_stream());` without `await` to fix the
compilation issue.
| assert 'bybit' in exchanges | ||
| assert 'binance' in exchanges | ||
|
|
||
| itewriwrite |
There was a problem hiding this comment.
🛠️ Refactor suggestion
不要なトークン itewriwrite により構文エラー
この行は意味を持たずテスト全体を壊します。完全に削除してください。
🧰 Tools
🪛 Ruff (0.11.9)
13-13: Found useless expression. Either assign it to a variable or remove it.
(B018)
13-13: Undefined name itewriwrite
(F821)
🪛 Pylint (3.3.7)
[error] 13-13: Undefined variable 'itewriwrite'
(E0602)
🤖 Prompt for AI Agents
In test/00_suite/test_02_config.py at line 13, there is an invalid token
'itewriwrite' causing a syntax error and breaking the test. Remove this entire
line completely to fix the syntax issue and restore test functionality.
| async fn async_start_user_stream(&mut self) -> anyhow::Result<()> { | ||
| // Implementation for user stream | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
async_start_user_stream が未実装です
現在は空実装で何も行わず Ok(()) を返しているため、ユーザーストリームが機能しません。最低でも todo!() でパニックさせるか、実装を完了してください。
🤖 Prompt for AI Agents
In exchanges/hyperliquid/src/market.rs around lines 270 to 274, the
async_start_user_stream function is currently a no-op returning Ok(()), which
means the user stream functionality is not implemented. Replace the empty
implementation with either a todo!() macro to explicitly panic and indicate
unimplemented code, or provide a proper implementation to start the user stream
as intended.
| async fn async_start_market_stream(&mut self) -> anyhow::Result<()> { | ||
| // Implementation for market stream | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
async_start_market_stream も未実装
マーケットストリームも同様に空実装です。このままだと open_market_stream() から呼ばれても実際のストリーム処理が行われません。実装 or 明確な未対応エラーをお願いします。
🤖 Prompt for AI Agents
In exchanges/hyperliquid/src/market.rs around lines 311 to 314, the
async_start_market_stream function is currently a stub with no implementation,
causing open_market_stream() to not perform any actual streaming. Implement the
real market stream processing logic inside async_start_market_stream or, if not
yet supported, return a clear error indicating that this feature is
unimplemented to avoid silent failures.
| // public_ws.connect().await; | ||
|
|
||
| let _ = self.async_refresh_order_book().await; | ||
|
|
||
| self.public_handler = Some(tokio::task::spawn(async move { | ||
| let ws_stream = public_ws.open_stream().await; | ||
| let mut ws_stream = Box::pin(ws_stream); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
WebSocket 接続処理がコメントアウトされています
public_ws.connect().await がコメントアウトされたままではストリームが開始されません。open_stream() の前に必ず接続を確立してください。
🤖 Prompt for AI Agents
In exchanges/bitbank/src/market.rs around lines 482 to 489, the WebSocket
connection call public_ws.connect().await is commented out, preventing the
stream from starting. Uncomment the public_ws.connect().await line and ensure it
is awaited before calling open_stream() to properly establish the WebSocket
connection.
| let params = format!("pair={}", config.trade_symbol); | ||
|
|
||
| let path = "/v5/order/realtime"; | ||
| let response = self.get_sign(&path, Some(¶ms)).await | ||
| .with_context(|| format!("open_orders error: {}{}", server.get_private_api(), path))?; | ||
|
|
||
| let result = Self::get_sign(&server, path, &query_string) | ||
| .await | ||
| .with_context(|| { | ||
| format!( | ||
| "open_orders: server={:?} / path={:?} / query_string={:?}", | ||
| server, path, query_string | ||
| ) | ||
| })?; | ||
| log::debug!("open_orders response: {:?}", response); | ||
|
|
||
| log::debug!("result.body={:?}", result.body); | ||
| if result.body.is_null() { | ||
| return Ok(vec![]); | ||
| } | ||
|
|
||
| let response = serde_json::from_value::<BybitMultiOrderStatus>(result.body) | ||
| .with_context(|| format!("order status parse error"))?; | ||
|
|
||
| let mut orders: Vec<Order> = response.into(); | ||
|
|
||
| for o in orders.iter_mut() { | ||
| o.update_balance(config); | ||
| } | ||
| let orders: Vec<BitbankOrder> = serde_json::from_value(response.data.get("orders").unwrap().clone())?; | ||
|
|
There was a problem hiding this comment.
response.data["orders"] への unwrap() は危険
open_orders で get("orders").unwrap() を呼んでいるため、API 仕様変更や失敗応答時にパニックします。ok_or_else() でエラーハンドリングし anyhow::Result に変換してください。
🤖 Prompt for AI Agents
In exchanges/bitbank/src/rest.rs around lines 200 to 208, the code uses unwrap()
on response.data["orders"], which can cause a panic if the "orders" key is
missing due to API changes or errors. Replace unwrap() with ok_or_else() to
handle the missing key gracefully by returning an appropriate anyhow::Result
error. This ensures robust error handling without panics.
| @@ -0,0 +1,109 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
未使用の pytest インポートを削除してください
pytest を参照している箇所が無く、Ruff の F401 が発生します。CI を通すためにも削除を推奨します。
-import pytest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import pytest | |
| # (Removed unused import) |
🧰 Tools
🪛 Ruff (0.11.9)
1-1: pytest imported but unused
Remove unused import: pytest
(F401)
🤖 Prompt for AI Agents
In test/00_suite/test_bitbank_order.py at line 1, the import statement for
pytest is unused and triggers a Ruff F401 warning. Remove the line importing
pytest to clean up the code and resolve the unused import issue.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
exchanges/bitbank/src/market.rs (1)
489-491:public_ws.connect().awaitが依然コメントアウトされたままです
過去レビュー指摘と同一内容です。接続がopen_stream()内部で行われない場合、ストリームが開始しません。必要であればコメントを外して await してください。exchanges/bitbank/src/rest.rs (1)
207-208:unwrap()によるパニックの可能性
response.data.get("orders").unwrap()はキー欠如時にパニックします。過去にも指摘済みですが未修正です。ok_or_else(|| anyhow!("missing orders"))?などで安全にハンドリングしてください。
🧹 Nitpick comments (2)
CLAUDE.md (1)
89-100: 表記ゆれと文法ミスを修正してください
- “Recent(with in a day)” → “Recent (within a day)”
- “each exchanges documentation” → “each exchange’s documentation”
- “its implement with WebSocket” → “it’s implemented with WebSocket” もしくは “it implements WebSocket”
読みやすさと正式な文法のために上記のように修正を検討してください。
exchanges/bitbank/src/rest.rs (1)
21-22: 同一定数の重複定義
BITBANK_BOARD_DEPTHは他モジュールでも定義されています。単一箇所に集約しpub useすることでメンテナンス性を向上させてください。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CLAUDE.md(1 hunks)exchanges/bitbank/src/market.rs(1 hunks)exchanges/bitbank/src/rest.rs(7 hunks)modules/rbot_lib/src/common/ccxt_config.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/rbot_lib/src/common/ccxt_config.rs
🧰 Additional context used
🪛 LanguageTool
CLAUDE.md
[style] ~7-~7: Consider shortening or rephrasing this to strengthen your wording.
Context: ...he current state of the project. If you make changes to the codebase, please update this file a...
(MAKE_CHANGES)
[grammar] ~91-~91: Did you mean the adverb/preposition “within”?
Context: ...changed from SQLite in v0.4.0) - Recent(with in a day) historical data is stored in SQL...
(WITH_IN)
[uncategorized] ~98-~98: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ... request for exchange according to each exchanges documentation. - ws.rs implements rea...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~99-~99: “its” (belonging to it) seems less likely than “it’s” (it is)
Context: ...me stream(public and private). Usually, its implement with WebSocket. ### Testing...
(AI_HYDRA_LEO_CPT_ITS_ITIS)
[uncategorized] ~99-~99: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...tream(public and private). Usually, its implement with WebSocket. ### Testing Structure...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[uncategorized] ~103-~103: You might be missing the article “the” here.
Context: ...Structure - Test files are organized in test/ directory - Exchange-specific tes...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: linux-simple
🔇 Additional comments (2)
exchanges/bitbank/src/market.rs (1)
51-55: コメントと実装が矛盾しています
"wss://stream.bitbank.cc/…" // Bitbank doesn't have public websocket
URL をセットしている一方で「公開 WS は無い」とコメントしています。
実際に接続が成立するか確認し、不要なら空文字列にするかコメントを更新してください。exchanges/bitbank/src/rest.rs (1)
372-376: 署名メッセージに?を含める仕様を再確認してくださいBitbank の署名仕様は “timestamp + window + path + body(またはクエリ)” です。POST/GET で JSON ボディを送る場合に
?{json}を連結すると検証に失敗する恐れがあります。公式ドキュメントと実機テストで要確認です。
| loop { | ||
| let message = ws_stream.next().await; | ||
| if message.is_none() { | ||
| log::error!("Error in ws_stream.recv: {:?}", message); | ||
| continue; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
None 受信時にループが継続し、CPU スピンの恐れ
ws_stream.next().await が None を返した場合、接続は終了しています。continue ではなく break して再接続するかタスクを終了しないと無限ループになります。
🤖 Prompt for AI Agents
In exchanges/bitbank/src/market.rs around lines 497 to 503, when
ws_stream.next().await returns None, the code currently continues the loop
causing a potential infinite CPU spin. Instead of continue, replace it with a
break statement to exit the loop so that the connection can be properly handled
by reconnecting or ending the task.
- Update REST API endpoints to use correct v1 paths (/v1/user/spot/order, /v1/user/spot/cancel_order) - Fix API base URL configuration in market.rs - Improve error handling in order tests to gracefully handle expected API errors (20019) - Fix WebSocket message parsing tests with proper JSON structure - Update test assertions for BitbankAssets to match actual API response format - Add debug logging for API signature generation - Fix socket subscription implementation with proper PubNub client usage - Remove path from HMAC signature message to match Bitbank API requirements These changes ensure proper API communication and make tests more robust by handling expected authentication/insufficient funds errors gracefully.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
exchanges/bitbank/src/rest.rs (1)
237-266:logdf_to_archivedfメソッドが未実装です。このメソッドは
todo!()で終わっており、呼び出されるとパニックが発生します。DataFrameの変換ロジックを実装する必要があります。コメントアウトされたコードを参考に、適切なDataFrame変換の実装を支援しますか？
♻️ Duplicate comments (7)
exchanges/bitbank/src/market.rs (2)
489-489: WebSocket接続処理がコメントアウトされています。
public_ws.connect().awaitがコメントアウトされたままではストリームが開始されません。open_stream()の前に必ず接続を確立してください。
497-503:None受信時にループが継続し、CPUスピンの恐れ。
ws_stream.next().awaitがNoneを返した場合、接続は終了しています。continueではなく break して再接続するかタスクを終了しないと無限ループになります。exchanges/bitbank/src/rest.rs (1)
207-207:response.data["orders"]へのunwrap()は危険。
open_ordersでget("orders").unwrap()を呼んでいるため、API 仕様変更や失敗応答時にパニックします。ok_or_else()でエラーハンドリングしanyhow::Resultに変換してください。exchanges/bitbank/src/message.rs (4)
221-242: 文字列からDecimalへの変換でunwrap()を使用すると、パニックが発生する可能性があります。APIレスポンスの形式が変更された場合や不正なデータが含まれている場合、アプリケーションがクラッシュする恐れがあります。エラーハンドリングを追加することを推奨します。
同様の変更を
bidsの処理にも適用してください。ただし、IntoトレイトはResultを返せないため、TryFromトレイトの使用を検討してください。
244-249: JSONデシリアライゼーションでunwrap()を使用しています。データ形式が期待と異なる場合にパニックが発生します。
TryFromトレイトの使用を検討してください。
298-304:unwrap()の多用によるパニックリスク。Kline変換処理で複数の
unwrap()を使用しています。不正なデータ形式に対する適切なエラーハンドリングが必要です。
349-350:unwrap()の使用によりパニックが発生する可能性があります。メッセージ形式が期待と異なる場合の適切なエラーハンドリングが必要です。
🧹 Nitpick comments (2)
exchanges/bitbank/src/market.rs (2)
53-53: 矛盾するコメントを修正してください。WebSocketのURLが設定されているにも関わらず、コメントでは"Bitbank doesn't have public websocket"と記載されています。実装と一致するようコメントを更新してください。
- "wss://stream.bitbank.cc/socket.io/?EIO=3&transport=websocket", // Bitbank doesn't have public websocket + "wss://stream.bitbank.cc/socket.io/?EIO=3&transport=websocket", // Bitbank public websocket URL
539-543: Control messageのエラー処理が未実装です。TODOコメントがあり、statusがfalseの場合のアラートやリカバリー処理が未実装です。
Control messageのエラー処理やリカバリー機能の実装を支援しますか？新しいissueを作成することも可能です。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
exchanges/bitbank/src/market.rs(1 hunks)exchanges/bitbank/src/message.rs(2 hunks)exchanges/bitbank/src/rest.rs(7 hunks)exchanges/bitbank/src/socket.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- exchanges/bitbank/src/socket.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: linux-simple
| async fn post(&self, host: &str, path: &str, headers: Vec<(&str, &str)>, params: &str) -> anyhow::Result<BitbankRestResponse> { | ||
| let server = &self.server_config; | ||
|
|
||
| let response = rest_post(&server.get_private_api(), &path, headers, params) | ||
| .await | ||
| .with_context(|| format!("post_sign error {}/{}", server.get_rest_server(), path))?; | ||
| .with_context(|| format!("post error: {}{}", server.get_private_api(), path))?; | ||
|
|
||
| Self::parse_rest_response(response) | ||
| } | ||
| let v: BitbankRestResponse = serde_json::from_str(&response) | ||
| .with_context(|| format!("parse error in post"))?; | ||
|
|
||
| fn parse_rest_response(response: String) -> anyhow::Result<BybitRestResponse> { | ||
| if response == "" { | ||
| log::warn!("empty response"); | ||
| let response = BybitRestResponse { | ||
| return_code: 0, | ||
| return_message: "Ok".to_string(), | ||
| return_ext_info: Value::Null, | ||
| time: NOW() / 1_000, | ||
| body: Value::Null, | ||
| }; | ||
| return Ok(response); | ||
| if v.success == 0 { | ||
| return Err(anyhow!("post error: status=0, {:?}", v.data)); | ||
| } | ||
|
|
||
| // log::debug!("rest response: {}", response); | ||
|
|
||
| let result = from_str::<BybitRestResponse>(&response) | ||
| .with_context(|| format!("parse error in parse_rest_response: {:?}", response))?; | ||
|
|
||
| ensure!( | ||
| result.is_success(), | ||
| format!("parse rest response error = {}", result.return_message) | ||
| ); | ||
|
|
||
| return Ok(result); | ||
| Ok(v) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
postメソッドのhostパラメータが使用されていません。
メソッドシグネチャでhostパラメータを受け取っているにも関わらず、実際にはself.server_config.get_private_api()を使用しています。パラメータを使用するか、シグネチャから削除してください。
- async fn post(&self, host: &str, path: &str, headers: Vec<(&str, &str)>, params: &str) -> anyhow::Result<BitbankRestResponse> {
- let server = &self.server_config;
-
- let response = rest_post(&server.get_private_api(), &path, headers, params)
+ async fn post(&self, host: &str, path: &str, headers: Vec<(&str, &str)>, params: &str) -> anyhow::Result<BitbankRestResponse> {
+ let response = rest_post(host, &path, headers, params)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In exchanges/bitbank/src/rest.rs around lines 339 to 354, the post method
receives a host parameter but does not use it, instead always using
self.server_config.get_private_api(). To fix this, either replace
self.server_config.get_private_api() with the host parameter in the rest_post
call to use the passed-in host, or if the host parameter is unnecessary, remove
it from the method signature to avoid unused parameters.
1 participant
Add public ws message parser