Skip to content

Fix duplicate IBKR execution and market-data error scoping - #359

Merged
Pigbibi merged 1 commit into
mainfrom
codex/fix-market-hours-boundary-20260728
Jul 29, 2026
Merged

Fix duplicate IBKR execution and market-data error scoping#359
Pigbibi merged 1 commit into
mainfrom
codex/fix-market-hours-boundary-20260728

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Why

  • A manual live invocation and the scheduled invocation could execute the same signal because execution deduplication was not consistently propagated to all configured services.
  • Structured failed/blocked execution results could be mistaken for completed execution, while partial accepted orders needed a durable marker.
  • Expected IBKR market-data permission errors were emitted through the global error path and obscured unrelated gateway failures.

What changed

  • Propagate the explicit IBKR_EXECUTION_DEDUP_ENABLED repository/runtime setting without silently enabling it by default; explicit false remains authoritative.
  • Record dedup markers only for accepted/previewed execution outcomes, preserve legacy fallback, and keep failed/blocked runs retryable.
  • Scope expected market-data error suppression to the matching IB connection/request/contract while preserving unrelated errors and concurrency across connections.
  • Update compatibility fixtures and regression coverage.

Verification

  • pytest: 388 passed, 5 skipped, 5 warnings.
  • ruff check: passed for all changed Python files.
  • actionlint: passed for the changed workflow.
  • git diff --check: passed.
  • Codex review (not AIAudit): no actionable findings.

Production safety

  • Live services already have deduplication explicitly enabled as a configuration mitigation.
  • No live /run endpoint or order submission was triggered during verification.

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi marked this pull request as ready for review July 29, 2026 02:31
@Pigbibi
Pigbibi merged commit 2115cde into main Jul 29, 2026
3 of 4 checks passed
@Pigbibi
Pigbibi deleted the codex/fix-market-hours-boundary-20260728 branch July 29, 2026 02:33

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a402c1dab

ℹ️ 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".

"IBKR_MARKET_TIMEZONE",
"IB_GATEWAY_ZONE",
"IB_GATEWAY_IP_MODE",
"IBKR_EXECUTION_DEDUP_ENABLED",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply per-target dedup overrides before the shared fallback

When the repository-level variable is set and a service's runtime_target.overrides specifies the opposite value, adding this setting to SHARED_TARGET_FALLBACK_ENV makes _target_env_value select the shared value first; the override at lines 432–440 is only consulted when that value is None. For example, a shared false silently overrides a live target's explicit true, disabling deduplication and allowing manual and scheduled runs to submit the same signal, so the per-target override must take precedence.

Useful? React with 👍 / 👎.

Comment on lines +43 to +44
symbol = str(getattr(resolved_contract, "symbol", "") or "").strip().upper()
return bool(symbol and symbol in self.requested_symbols)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Match the actual quote request before suppressing errors

During a snapshot fetch, any other outstanding market-data request on the same IB connection whose contract has the same symbol is treated as part of this fetch, even if it has a different request ID, security type, exchange, or is a pre-existing streaming subscription. If that request emits 10089/10168, its error and subsequent 300 are hidden and included in this fetch's fallback summary, so unrelated subscription failures can still be obscured; suppression needs to be limited to request IDs/contracts initiated by the wrapped fetch rather than symbol equality alone.

Useful? React with 👍 / 👎.

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