Skip to content

fix(trade): scale BUY fills down to available cash instead of rejecting - #86

Merged
xingwudao merged 2 commits into
xingwudao:mainfrom
codingDuan:fix/scale-down-insufficient-cash
Sep 29, 2026
Merged

xingwudao merged 2 commits into
xingwudao:mainfrom
codingDuan:fix/scale-down-insufficient-cash

Conversation

@codingDuan

Copy link
Copy Markdown
Contributor

Linked Issue

No issue filed; found while reconciling a production XQuant spec replica against its legacy implementation.

Linked Docs / Plans

  • agent/skills/configure-trade-execution/SKILL.md: documents execution.insufficient_cash_policy
  • docs/architecture.md: spec example updated

Requirement Background

SimBroker.fill_market_orders rejected a market BUY outright whenever its cost exceeded available cash at fill time.

Orders are sized from the signal bar's close but fill at a later price (next_open, next_hl2, ...). TopNRanking targets sum to 1.0 with no cash buffer. So on a fully invested rotation, the last BUY leg is often a few units short of cash and the whole order is dropped. The target position then stays empty until the next rebalance, and the next attempt must buy the full weight in one order, so it tends to fail again.

Observed on a production spec (3-ETF rotation, 10-day rebalance, next_hl2, lot size 100, 2025-01-02 to 2026-09-28): 17 of 87 orders were rejected with insufficient_cash, several in a row for the same symbol. Signals were identical to a legacy implementation that scales buys to available cash, yet cumulative return was +65.3% vs +80.4%.

Design

  • SimBroker(insufficient_cash_policy=...), default "scale_down": when a BUY costs more than available cash, fill the largest whole-lot quantity the cash covers, fees included (binary search over lot counts; cost is monotonic in shares). Reject with insufficient_cash only when not even one lot is affordable. "reject" keeps the previous all-or-nothing behavior.
  • Lot size comes from the engine: Engine calls broker.set_lot_size(self._lot_size) each bar, next to set_current_date, so there is one source of truth and every broker construction path (compile_run, tools/engine) picks it up.
  • OrderBook.fill(..., shares=None): the ManagedOrder keeps the requested quantity; the returned Fill carries the filled quantity. So orders.csv shows shares (requested) and filled_shares (actual), and scaled orders are marked status_reason=scaled_to_available_cash. _apply_fill already reads fill.order.shares, so portfolio and broker cash stay consistent.
  • Spec: new execution.insufficient_cash_policy, validated by insufficient_cash_policy_invalid and recorded in compiled_plan.json, execution_assumptions.json, and the generated strategy.py.

Design Constraints

  • Existing run directories must keep auditing clean. The new field is part of the effective spec, which would change every spec hash and make audit_reproducibility fail spec_hash and compiled_plan_consistency for runs written before this change. Following the existing precedents (default metrics is dropped from the hash; suspension_policy / lag_bars are back-filled in plan normalization):
    • compute_hash() omits the field when it is the default scale_down. An explicit reject still changes the hash.
    • _normalize_compiled_plan_material_fields back-fills execution.insufficient_cash_policy = "scale_down" for older plans.
  • Target weights / signals are unaffected; only fills change.

Data Flow

strategy_spec.yaml (execution.insufficient_cash_policy) → _parse_execution → validate → compile_run → SimBroker(insufficient_cash_policy=...); Engine.step → broker.set_lot_size; fill_market_orders → _affordable_buy_shares → OrderBook.fill(shares=...) → Fill → _apply_fill → orders.csv / trades.csv / equity curve.

Non-goals

  • No change to order generation (sizing still uses the signal bar's close), SELL handling, or non-market orders.
  • No per-symbol lot sizes (lot_size_config.by_symbol stays non-executable).

Compatibility

  • Behavior change: specs that relied on the implicit all-or-nothing BUY now scale down by default. Set execution.insufficient_cash_policy: reject to keep the old fills.
  • Spec hash: unchanged for specs at the default. Verified by auditing a run directory produced by the pinned pre-change commit 6af7fa77 with this branch: pass (it failed both hash checks before the compatibility fix).
  • SimBroker constructor: new keyword argument with a default; existing callers are unaffected.
  • OrderBook.fill: new optional keyword argument; existing callers are unaffected.

Verification

  • TDD: 20 new tests and 4 updated ones (broker scaling, lot rounding, fees, same-batch cash, reject policy, invalid policy / lot size, engine gap-up scale vs reject, lot size wiring, spec parse / validate, compiled plan / assumptions / strategy.py, legacy hash and legacy plan compatibility). Every test asserting new behavior was run red before its implementation. The guard tests (valid values accepted, explicit reject changes the hash, a legacy plan does not match a reject plan) already held and are there to lock the boundary. The existing gap-up rejection test now passes insufficient_cash_policy="reject" explicitly.
  • tests/spec tests/trade tests/core tests/audit: 792 passed.
  • Full suite: 4699 passed, 9 skipped, 1 failed. The failure, tests/operators/test_installed_certification.py::test_installed_wheel_certifies_without_source_checkout, also fails on main in the same environment, so it predates this change. (tests/agent/test_round_21_monitor_contracts.py shells out to uv run oxq and passes when uv resolves the test environment.)
  • ruff check src tests and mypy on the touched modules: no findings beyond those already on main.
  • Production replay (same spec and market data as production, 2025-01-02 to 2026-09-28):
before (6af7fa77) this branch legacy implementation
cumulative return +65.32% +78.79% +80.41%
max drawdown -10.06% -15.12% -14.94%
rejected / scaled BUYs 17 / 0 0 / 12 n/a

Target weights are byte-identical before and after. On the window where the legacy implementation uses the same market-value NAV method (2026-03-02 onward), the median absolute daily return difference is 0.0008% (max 0.04%). The remaining gap before that window comes from a legacy-side NAV formula change that was never back-filled, not from this engine.

🤖 Generated with Claude Code

SimBroker rejected a whole market BUY when its cost exceeded available
cash at fill time. Orders are sized from the signal bar's close but fill
at a later price, and TopNRanking targets sum to 1.0, so fully invested
rotations routinely dropped their last BUY leg and left the target
position empty until the next rebalance (17 of 87 orders on a production
3-ETF rotation spec).

- SimBroker(insufficient_cash_policy="scale_down" | "reject"), default
  scale_down: fill the largest whole-lot quantity the cash covers,
  including fees; reject only when not even one lot is affordable.
- Engine passes its lot size to the broker via set_lot_size().
- OrderBook.fill(shares=...): the managed order keeps the requested
  quantity, the Fill carries the filled one; scaled fills are marked
  status_reason=scaled_to_available_cash.
- Spec: execution.insufficient_cash_policy, validated and recorded in
  compiled_plan.json, execution_assumptions.json and strategy.py.
- Reproducibility: the default is left out of compute_hash() and
  back-filled when normalizing older compiled plans, so run directories
  written before this change still audit clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xingwudao

Copy link
Copy Markdown
Owner

@codex review

@xingwudao

Copy link
Copy Markdown
Owner

审查结论:建议 Request changes。发现 1 个阻塞问题和 1 个兼容性缺口。

[P1] 旧产物被错误解释为新的 scale_down 语义

位置:

  • [schema.py](
    if self.execution.insufficient_cash_policy == "scale_down":
    # Added after 0.1.0: keep the default out of the hash so run
    # directories written before the field existed still audit clean.
    canonical_obj["execution"].pop("insufficient_cash_policy", None)
    )
  • [compiler.py](
    execution = normalized.get("execution")
    if isinstance(execution, dict):
    execution.setdefault("insufficient_cash_policy", "scale_down")
    )

旧版本缺少该字段时,实际行为是 reject。PR 却同时:

  • 从 spec_hash 中删除默认的 scale_down。
  • 将旧 compiled plan 缺失字段补成 scale_down。
  • 让旧产物继续通过 reproducibility audit。

实际复现结果:

main:
spec_hash sha256:0efe3fa9ecee6ac5
status rejected insufficient_cash

PR #86:
spec_hash sha256:0efe3fa9ecee6ac5
status filled scaled_to_available_cash
filled_shares 5

即相同 spec、相同 hash、审计可通过,但成交结果不同。这破坏了
spec_hash 和 reproducibility audit 的核心保证。

建议将旧计划缺失字段解释为 reject,并通过 schema/version 或显式性标记
区分新默认;或者让新语义进入 hash,避免两种行为共享同一身份。

[P2] 工具入口无法恢复旧的 reject 行为

位置:

  • sim_broker.py
  • [tools/engine.py](
    def engine_run(
    strategy: str,
    start: str,
    end: str,
    symbols: list[str] | None = None,
    initial_cash: float = 100_000.0,
    run_through: Literal["indicator", "signal"] | None = None,
    data_dir: str | None = None,
    fee_rate: float | None = None,
    fee_min: float | None = None,
    slippage_rate: float | None = None,
    lot_size: int = 1,
    cash_annual_return: float = 0.0,
    data_start: str | None = None,
    fill_price_mode: Literal["close", "mid", "next_avg", "next_close", "next_hl2", "next_mid", "next_open"] | None = None,
    market_calendar: str | None = "XNYS",
    ) -> dict[str, Any]:
    """Run a strategy through the engine and store the result."""
    strat = session._strategies.get(strategy)
    if strat is None:
    return {"error": f"Strategy '{strategy}' not found"}
    supported_fill_modes = {"close", "mid", "next_avg", "next_close", "next_hl2", "next_mid", "next_open"}
    next_session_fill_modes = {"next_avg", "next_close", "next_hl2", "next_mid", "next_open"}
    if fill_price_mode is not None and fill_price_mode not in supported_fill_modes:
    valid = ", ".join(sorted(supported_fill_modes))
    return {"error": f"Unsupported fill_price_mode '{fill_price_mode}'. Valid: {valid}"}
    if fill_price_mode in next_session_fill_modes and not market_calendar:
    return {"error": f"market_calendar is required when fill_price_mode='{fill_price_mode}'"}
    # Use symbols as a per-run universe override; otherwise use the session's
    # default run universe. Do not mutate the reusable Strategy object.
    run_universe = StaticUniverse(tuple(symbols)) if symbols else session._strategy_universes.get(strategy)
    if run_universe is None:
    run_universe = getattr(strat, "_legacy_universe", None)
    if run_universe is None or not getattr(run_universe, "symbols", ()):
    return {"error": "No universe set for this run. Use strategy_set_universe first, or pass symbols parameter."}
    path = resolve_data_dir(Path(data_dir) if data_dir else None)
    provider_calendar = market_calendar if fill_price_mode in next_session_fill_modes else None
    market = LocalMarketDataProvider(path, calendar=provider_calendar)
    # Build fee and slippage models from params
    fee_model = None
    if fee_rate is not None:
    fee_model = PercentageFee(
    rate=Decimal(str(fee_rate)),
    min_fee=Decimal(str(fee_min)) if fee_min is not None else Decimal("0"),
    )
    slippage_model = None
    if slippage_rate is not None:
    slippage_model = PercentageSlippage(rate=Decimal(str(slippage_rate)))
    broker_kwargs: dict[str, Any] = {}
    if fee_model is not None:
    broker_kwargs["fee_model"] = fee_model
    if slippage_model is not None:
    broker_kwargs["slippage_model"] = slippage_model
    if fill_price_mode is not None:
    broker_kwargs["fill_price_mode"] = FillPriceMode(fill_price_mode)
    if fill_price_mode in next_session_fill_modes:
    broker_kwargs["market_calendar"] = market_calendar
    broker = SimBroker(**broker_kwargs)
    )
  • [tools/optimize.py](
    ps.add_constraint(expr)
    return ps
    def _broker_factory(
    fee_rate: float | None,
    fee_min: float | None,
    slippage_rate: float | None,
    ) -> Callable[[], SimBroker]:
    """Return a factory that creates fresh SimBroker instances."""
    def factory() -> SimBroker:
    fee_model = None
    if fee_rate is not None:
    fee_model = PercentageFee(
    rate=Decimal(str(fee_rate)),
    min_fee=Decimal(str(fee_min)) if fee_min is not None else Decimal("0"),
    )
    slippage_model = None
    if slippage_rate is not None:
    slippage_model = PercentageSlippage(rate=Decimal(str(slippage_rate)))
    return SimBroker(fee_model=fee_model, slippage_model=slippage_model)
    )

SimBroker 的默认值全局改成了 scale_down,但 engine_run 和优化工具
没有暴露或传递 insufficient_cash_policy。这些入口的用户会静默改变成交
结果,并且无法选择 PR 所称的兼容模式 reject。

建议为相关工具参数、broker factory 和测试补齐该字段。

验证结果:

  • 聚焦测试:429 passed
  • ruff check:通过
  • git diff --check:通过
  • GitHub 当前仅有文档构建检查,没有完整测试门禁
  • 未向 GitHub 发布 review/comment

@xingwudao
xingwudao merged commit eba4ac6 into xingwudao:main Sep 29, 2026
1 check failed
@codingDuan
codingDuan deleted the fix/scale-down-insufficient-cash branch September 29, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants