fix(backtest): route yfinance crypto to CryptoEngine - #970
Merged
warren618 merged 1 commit intoAug 6, 2026
Conversation
_create_market_engine's yfinance branch handed every symbol to GlobalEquityEngine on the loader name alone, so crypto pairs fetched from Yahoo ran under zero-commission equity rules while the CryptoEngine fee keys (taker_rate/maker_rate/slippage) sat ignored in config. Two runs differing only in fees produced byte-identical metrics (HKUDS#965). Route that branch on the instrument market like the fallback branch already does, and teach the market classifier yfinance's native crypto spelling (BTC-USD) next to the USDT forms so the routing actually covers what Yahoo serves. Fixes HKUDS#965 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
he-yufeng
force-pushed
the
fix/yfinance-crypto-engine-routing
branch
from
August 4, 2026 08:27
40eff7c to
672acca
Compare
p1927
pushed a commit
to p1927/Vibe-Trading
that referenced
this pull request
Aug 21, 2026
_create_market_engine's yfinance branch handed every symbol to GlobalEquityEngine on the loader name alone, so crypto pairs fetched from Yahoo ran under zero-commission equity rules while the CryptoEngine fee keys (taker_rate/maker_rate/slippage) sat ignored in config. Two runs differing only in fees produced byte-identical metrics (HKUDS#965). Route that branch on the instrument market like the fallback branch already does, and teach the market classifier yfinance's native crypto spelling (BTC-USD) next to the USDT forms so the routing actually covers what Yahoo serves. Fixes HKUDS#965 Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.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
2 participants
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 #965.
The yfinance branch of
_create_market_enginerouted on the loader name alone, so every symbol fetched from Yahoo went toGlobalEquityEngine. That engine readsslippage_us/slippage_hk/hk_commission, so a crypto config'staker_rate/maker_rate/slippagewere never looked at, and nothing errored or warned. Two runs differing only in fees came out byte-identical, which is how the reporter caught it.The fix routes that branch on the instrument market, the same way the fallback branch already does for
local/stooq. Crypto pairs now getCryptoEngineand the fee keys actually reach it.One adjacent gap had to come along for the fix to cover what Yahoo really serves: the market classifier only knew
BTC-USDT/BTC/USDT, while Yahoo's native crypto spelling isBTC-USD. Without it,BTC-USDstill missed the crypto branch, so^[A-Z]+-USD$is added next to the USDT patterns. Nothing else in the pattern list uses the dash-USD shape, so no existing classification moves.Verification
New
test_yfinance_crypto_routing.py: USDT and USD spellings route toCryptoEngine,AAPL.USstill routes toGlobalEquityEngine, and an explicit fee config lands on the engine instance (taker_rate/maker_rate/slippage_rate). Routing/engine/composite subset: 769 passed, the only 2 failures (test_akshare_loaderforex) reproduce on a clean main checkout without this change, so they are pre-existing and unrelated.Scope and risk
In scope: engine routing for the yfinance branch and the
-USDcrypto spelling in the market classifier. Deliberately out of scope: composite-engine fee plumbing and any loader changes. No broker, MCP, network, or credential surface is touched; the change is backtest-local. Rollback is a single-commit revert; the pre-change behavior is reachable by reverting without side effects.