Skip to content

fix(price-basis): explicit fail-closed price_basis for historical loaders - #635

Open
Pigbibi wants to merge 2 commits into
mainfrom
fix/ibkr-price-basis-20261015
Open

Pigbibi wants to merge 2 commits into
mainfrom
fix/ibkr-price-basis-20261015

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Motivation

#602 selected split-only yfinance Close for signals, but missing Close still silently selected Adj Close, with native O/H/L left on a different basis. The IBKR proxy also hard-coded TRADES without an explicit configuration pin. This change makes the historical signal adapter enforce one declared basis without changing strategy or execution controls.

PRICE_BASIS / runtime JSON price_basis accepts exactly raw_unadjusted, split_adjusted, split_dividend_adjusted, and total_return_index. An explicit environment value overrides runtime JSON and manifest defaults; the effective default remains split_adjusted. Invalid values fail configuration validation.

Behavior

Case Before After
Default with complete valid yfinance data Native Open/High/Low/Close and Volume Unchanged, including IEEE-754 float bytes on frozen main fixtures
Default IBKR history Split-adjusted TRADES Same TRADES endpoint and basis, now explicit
Requested split+dividend No configuration pin; Close preferred Adj Close plus row-wise O/H/L multiplied by Adj Close / Close; volume unchanged; IBKR uses ADJUSTED_LAST only
Missing columns/rows, invalid prices/volume, empty/provider failures, unknown or inconsistent OHLC metadata Could silently change basis/provider, return empty, omit invalid rows or synthesize fields Intentional fail-closed change: typed PriceBasisError with bounded diagnostics; no implicit substitution or fabricated values
ADJUSTED_LAST empty when dividend adjustment is requested Could fall back to TRADES Preserve the typed failure even when pinned QPK attempts its legacy retry
raw_unadjusted / total_return_index No explicit contract Valid enum configuration; these endpoints reject unsupported raw history and reject wealth indices for OHLC/history consumers

The local adapter constrains the existing pinned QPK loader. No shared QPK API, dependency pin, shared market-data standard, strategy guard, execution protection, or deployment configuration is changed. No retained fallback is considered necessary for safety. The upstream request failure now stops this history request rather than implicitly switching providers.

Volume is explicitly provider_native_unscaled, never rescaled by the dividend factor. Missing volume is unavailable rather than zero. Provenance v2 records requested/resolved basis, provider, source columns and volume policy; failure logs include a reason and no provider exception text, prices, credentials or account data. External indicators and feature snapshots retain their separate contracts.

Verification

Synthetic fixtures only:

  • All four valid enum values, invalid environment/runtime JSON values, configuration precedence, and propagation into the adapter.
  • Missing Close, missing Adj Close for dividend adjustment, absent O/H/L/volume, invalid rows, and ambiguous multi-symbol columns.
  • Mixed field metadata and inconsistent OHLC detection; a floating-point boundary case preserves valid scaled prices without changing values.
  • A 4:1 split (continuous split-adjusted history versus a contemporaneous raw-unit jump) and a dividend day with coherent O/H/L/C and unchanged volume.
  • Frozen current-main synthetic fixture output compared as IEEE-754 bytes, including both MultiIndex orientations.
  • Real pinned QPK helper integration with fake IBKR: exact TRADES/ADJUSTED_LAST requests, empty ADJUSTED_LAST, transport errors, and native missing-field rejection before legacy synthesis.
  • Typed failures propagate through the runtime historical-price map; unlabelled custom-loader responses are rejected.

Local full suite: 2148 passed, 5 skipped (Python 3.12, pinned dependencies). Full-repo Ruff, dependency pin consistency, uv lock --check, inert Cloud Run startup smoke and git diff --check passed. The 5 existing deprecation warnings concern legacy execution-report environment naming. No market/account data or live broker access was used.

Real paired strategy results remain NOT_COMPUTED: the approved aggregation entry in UESP #542 is unmerged and default-branch gated, and upstream frozen paired replay inputs are still required. Unit tests and CI do not establish real strategy equivalence.

Related vocabulary/design: QPK #678. Research entry: UESP #542.

Needs owner review before IBKR deploy. No merge, auto-merge, deployment, orders, broker writes, or production changes are requested or performed by this PR.

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator Author

Research findings — ibkr_compare_run2

0/8 pairs computed. All eight remain NOT_COMPUTED; no REAL_MISMATCH was assessed, and no zero difference is inferred. A generation/hash-pinned config-manifest GET now succeeds in the approved cloud, but the subsequent bounded metadata LIST returns a confirmed HTTP 403. Cloud steps stopped immediately after that new denial. No IAM/WIF/variables/secrets or trading controls were changed.

Pair / legacy strategy Window Primary class Additional blockers Specific unlocated evidence
T1 soxl_soxx_trend_income W1 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) indicator binding/state/plugins
T1 soxl_soxx_trend_income W2 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) indicator binding/state/plugins; actual first live fill/order date
T2 tqqq_growth_income W1 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) option history/joint accounting
T2 tqqq_growth_income W2 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) option history/joint accounting; actual first live fill/order date
T3 global_etf_rotation W1 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) PIT feature snapshots
T3 global_etf_rotation W2 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) PIT feature snapshots; actual first live fill/order date
T4 russell_top50_leader_rotation W1 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) PIT Top50 membership/snapshots
T4 russell_top50_leader_rotation W2 CONTRACT_GAP DATA_GAP + PERMISSION_DENIED (discovery) PIT Top50 membership/snapshots; actual first live fill/order date

The categories are non-exclusive. CONTRACT_GAP is primary because no inspected default-branch entry produces the required four-target basis-only replay ledgers, and UESP #542 is unmerged. DATA_GAP means inputs are not located/verified, not proved nonexistent. The permission category applies to discovery, not an attempted ledger GET. W1 is trailing two years plus warm-up; W2 requires the first actual live order/fill. No financial window, inception, cost schedule or ledger rows were invented; both-arm metrics and deltas remain null.

Root cause of 0/8: discovery stopped before a real replay, and access is only one blocker. Required frozen prices/actions/calendar, initial state/cash flows, complete numeric 1x/2x costs, historical plugin/option/PIT inputs and independent both-arm ledgers remain unbound. Reproducing #542 confirms exact-size changes within one bucket are missed (synthetic 0.002 vs 0.008 reports 0 difference days), total_return/trade_count are absent, and unknown inception raises INVALID_DATE before W1. Its non-README zh-CN document also remains inconsistent with the supplied documentation rule. #542 was not changed or merged.

A later sanitized pilot receipt locates partial SOXL/SOXX/QQQ/TQQQ history through 2026-08-25 in raw and split+dividend-adjusted bases. It is not treated as missing, but it does not establish split-only input, the current cutoff or paired IBKR ledgers; this job did not revalidate those market objects. Their catalog filename is outside the existing canonical-manifest reader contract, which was not relaxed.

Access evidence:

  • Old UESP inventory: auth succeeded; generic CLI failure does not preserve exact HTTP/client cause.
  • Old UES LIST: 401 and 403 were combined; its historical status cannot be recovered.
  • New exact GET: generation 1792038177321950, 2835 bytes, SHA256 796e80bbbb041aeca2f009cd6d2ae4998780138aae94abbd34d8d545f475e40e verified in cloud. Only sanitized statuses returned, with no referenced inputs read.
  • New default-branch LIST: storage.objects.list, HTTP 403, configured research bucket, request prefix research/v2/input/, using existing vars.GCP_WORKLOAD_IDENTITY_SERVICE_ACCOUNT. Exact principal/resource selectors are frozen by the workflow config and non-secret run logs. A 403 identifies effective operation denial, not the particular IAM/deny/perimeter cause.
  • Diagnostic-only UES #614 preserves LIST status/operation without authentication or retry changes; normally squash-merged after all four CI checks passed on locked head af6b0a7e79c384fd0b3f39622207a721c84050f6. The initial CI also passed before the normal main documentation update. Local discovery tests: 254 passed; repository unittest: 254 passed; Ruff passed.

Exact approval/evidence needed: preferably provide reviewed exact catalog/input object names, generations and SHA256s readable through the existing identity, avoiding LIST. If bucket discovery is essential, separately authorize investigation/resolution of storage.objects.list for that existing principal and configured research bucket, preferably through a role containing only that permission and an owner-approved expiry. The owner-local PERMISSION_REQUEST.md records exact identifiers and proposed CEL. A normal bucket IAM resource.name condition cannot make LIST prefix-only; a bucket-wide listing grant needs an explicit decision (Google Cloud restriction). roles/storage.objectViewer is broader than the minimal LIST permission and is not requested as a blanket grant. GET success is proved for the config manifest only. Access approval alone still does not supply the missing producer, cost/state histories or corrected aggregation contract.

#635 remains open at 39b5f10a504cba039197313022a7976937460f10; no approval, merge, auto-merge or IBKR deployment. Real market/account data remained in approved cloud; no orders or risk-limit changes. The comparison evidence is still insufficient for a deployment decision.

…20261015

Co-Authored-By: Codex <noreply@openai.com>
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.

1 participant