Wrap up A-share strategy research and gating#32
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
8e08d7c to
3cb6ca6
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
3cb6ca6 to
17fde7e
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Co-Authored-By: Codex <noreply@openai.com>
17fde7e to
377c1c6
Compare
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
With the default simulation_mode='blend', this function now stores the combined return series in blend_slice, but the returned periods block still references the removed combo_slice. After the ETF and stock legs finish, building the payload raises NameError, so the combo research script cannot print or write results unless this reference is changed back to the actual blended series.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| & ~frame["is_st"] | ||
| & frame["adv20_cny"].ge(float(min_adv20_cny)) | ||
| & frame["market_cap_cny"].ge(float(min_market_cap_cny)) | ||
| & frame["revenue_yoy"].ge(float(min_revenue_yoy)) | ||
| & frame["profit_yoy"].ge(float(min_profit_yoy)) | ||
| & frame["mom_12_1"].ge(float(min_momentum)) | ||
| & frame["list_days"].ge(int(min_list_days)) | ||
| & frame[ |
There was a problem hiding this comment.
Filter suspended ChiNext names before ranking
The new ChiNext snapshot contract requires suspension_days_63 and the panel populates it, but the eligibility filter never applies it. Any recently suspended stock that otherwise passes liquidity, growth, and momentum thresholds remains rankable and can be selected, which makes the proxy backtest and generated targets tradability-inaccurate for those snapshots; the dividend snapshot path already excludes names with suspension days before scoring.
Useful? React with 👍 / 👎.
|
|
||
|
|
||
| def run(*, start: str, end: str, target_vol: float = 0.50) -> dict[str, Any]: | ||
| market_history = _download_market_history(start, end) |
There was a problem hiding this comment.
Download warmup history before the reported window
This new proxy downloads data starting exactly at start, while run_proxy_backtest cannot emit a signal until DEFAULT_MIN_HISTORY_DAYS rows have elapsed. For the default start=2021-01-01, the first ~201 trading days included in full_sample and the 2021–2022 period are forced cash rather than strategy returns, so the research evidence is materially biased; fetch a pre-start warmup window and only slice metrics to the requested dates.
Useful? React with 👍 / 👎.
| "research_backtest_only_profiles": [ | ||
| CN_INDUSTRY_ETF_ROTATION_AGGRESSIVE_PROFILE, | ||
| CN_INDEX_ETF_TACTICAL_ROTATION_PROFILE, |
There was a problem hiding this comment.
Do not label the live candidate as research-only
CN_INDUSTRY_ETF_ROTATION_AGGRESSIVE_PROFILE was promoted to catalog status live_candidate in this change, but the live-candidate evaluation payload still emits it under research_backtest_only_profiles. Consumers of this report will see the optional QMT target classified as research-only, which contradicts the catalog gating layer this script is supposed to summarize.
Useful? React with 👍 / 👎.
| return { | ||
| "revenue_yoy": 0.0, | ||
| "profit_yoy": 0.0, | ||
| "revenue_acceleration_2q": 0.0, | ||
| "roe_ttm": 0.0, |
There was a problem hiding this comment.
Treat missing ChiNext financials as ineligible
When AkShare returns no financial rows for a ChiNext name, the panel writes zero growth/ROE values instead of missing values. Those zeros satisfy the strategy's default min_revenue_yoy=-5% and min_profit_yoy=-20% filters, and because earnings_positive is only a small bonus, a stock with no financial coverage can still be selected on momentum/liquidity, contrary to the design's requirement to exclude missing financial fields.
Useful? React with 👍 / 👎.
| growth_score * 0.50 | ||
| + momentum_score * 0.40 | ||
| + liquidity_score * 0.10 | ||
| + earnings_bonus | ||
| - risk_penalty * 0.05 |
There was a problem hiding this comment.
Include the quality factors in the ChiNext score
The final score for the new growth-momentum-quality selector only combines growth, momentum, liquidity, an earnings bonus, and volatility; it never uses the required quality columns such as roe_ttm, roe_stability_3y, or gross_margin_stability_3y. In snapshots where high-momentum names have weak quality, they rank the same as quality names, so the backtest and target weights do not actually evaluate the advertised quality sleeve.
Useful? React with 👍 / 👎.
|
|
||
| import pandas as pd | ||
|
|
||
| from scripts.research_cn_dual_track_combo_proxy_backtest import ( |
There was a problem hiding this comment.
Avoid importing snapshot-pipeline scripts during test collection
In the inspected CI workflow, dependencies install this package, pandas/pytest, and QuantPlatformKit, but not CnEquitySnapshotPipelines. Importing research_cn_dual_track_combo_proxy_backtest at test collection imports research_cn_dividend_quality_snapshot_proxy_backtest, whose top-level imports require cn_equity_snapshot_pipelines, so the new unit test fails before running unless that external repo happens to be checked out locally.
Useful? React with 👍 / 👎.
| if not positive_adv20.empty and float(positive_adv20.median()) < DEFAULT_ADV20_CNY_UNIT_SCALE_THRESHOLD: | ||
| frame["adv20_cny"] = frame["adv20_cny"] * DEFAULT_ADV20_CNY_UNIT_SCALE |
There was a problem hiding this comment.
Do not infer ADV units from median size
When a snapshot is already expressed in CNY but covers a low-liquidity universe whose median positive adv20_cny is below 10m, this heuristic multiplies every ADV value by 10,000 before the liquidity floor is applied. That lets genuinely illiquid names pass the 30m CNY filter and changes scores/weights; the unit conversion should come from explicit snapshot metadata or source-specific normalization rather than the cross-sectional median.
Useful? React with 👍 / 👎.
| "benchmark_symbol": DEFAULT_BENCHMARK_SYMBOL, | ||
| "defensive_symbols": ("510300",), |
There was a problem hiding this comment.
Enable the configured defensive leg
These presets are passed through industry_rotation.build_target_weights, where defensive_symbols are only considered when enable_benchmark_risk_off is true. Because none of the KCB presets set that flag, the listed 510300 defensive sleeve is inert and the research metrics are computed without the intended risk-off behavior.
Useful? React with 👍 / 👎.
Summary
Verification