feat(1.0): IBKR mock + IbTwsClient (ib_insync), tests

[IamJasonBian]

Context

allocation-engine 1.0.0 adds broker-style plumbing: an offline mock and a real TWS / IB Gateway client (paper or live socket).

What was added

Offline

• trading_system/brokers/ibkr_mock.py — MockIbkrClient: in-memory account tags, positions, MKT/LMT/cancel
• tests/brokers/test_ibkr_mock.py

Live TWS (ib_insync)

• trading_system/brokers/ib_tws_client.py — IbTwsClient / TwsClientConfig: connect, account values, positions, limit order, cancel by id
• requirements.txt: ib_insync
• Lazy export in trading_system/brokers/__init__.py so mock-only imports stay cheap; event-loop bootstrap for Python 3.10+ / eventkit
• tests/brokers/test_ib_tws_client_mocked.py — mocked read/write (no TWS)
• tests/brokers/test_ib_tws_live.py — optional live tests (TWS_INTEGRATION=1, optional TWS_PLACE_ORDER=1 on paper 7496 only)

Note: IbTwsClient currently exposes limit orders only (no MarketOrder wrapper). To trigger an immediate-style buy you either use a limit near the last price, extend the client with ib_insync.MarketOrder, or drive MockIbkrClient for simulation.

Version

• trading_system/version.py, package.json, README 1.0.0; README Interactive Brokers (TWS API) section
• pytest.ini: integration marker

Verified

• pytest tests/brokers/test_ibkr_mock.py — pass
• pytest tests/brokers/test_ib_tws_client_mocked.py — pass
• Optional: TWS_INTEGRATION=1 pytest tests/brokers/test_ib_tws_live.py with TWS API enabled

Note: full repo pytest tests/ may still have unrelated failures; broker tests are isolated.

---

If a buy order does not trigger or submit (TWS / API)

Work through these in order; the TWS log (View → Message Log or API messages) often states the exact rejection reason.

1. API session
   • In TWS: File → Global Configuration → API → Settings
   • Enable Enable ActiveX and Socket Clients
   • Socket port matches your code (paper 7496, live 7497)
   • Read-Only API is off if you need trading (your logs showed Read-Only mode: false — keep it that way for orders)
2. Trusted client
   • 127.0.0.1 (or your host) is in the trusted IPs list, or use Allow connections from localhost only (wording varies by TWS build)
3. Client ID
   • Use a unique client_id per concurrent connection. If a stale Python process holds the id, TWS can reject; pick another (e.g. set TWS_CLIENT_ID in the live test env) or close the other app
4. Account and permissions
   • Logged into the intended account (paper vs live). Paper must show paper buying power / not expired
   • In Client Portal / Account Management, confirm trading permissions and market data / country are enabled for the products you trade (e.g. US stocks)
5. Contract and order type
   • place_limit_order must qualify the contract (qualifyContracts). If the symbol fails, check SMART vs required primary exchange, USD currency for US equities, and that the symbol exists (no typo)
   • A limit far from the market may rest on the book without filling; that is not a “failed” submit — use TWS Orders to see it working
   • For an immediate market-style test, you still need a submit that IB accepts: add a MarketOrder path in code or place a limit at/through the last price and watch for a fill
6. Session / hours
   • If outside regular trading hours (RTH) is not enabled for that order, IB may not execute as you expect; check outsideRth on the Order if you add market/limit with extended hours
7. Market data (optional)
   • You do not always need a paid market data subscription to submit an order, but you may need it for the client to display or adaptive logic; for a plain limit, subscription issues more often show as no last price in TWS, not a silent no-submit
8. This PR’s scope
   • IbTwsClient does not yet wire into Executor / SafeCashBot; production buy paths still go through Robinhood in the rest of the repo. To “trigger a buy” from the trading engine you need a follow-up: inject IbTwsClient (or a shared protocol) into execution and map TradeTask → place_limit_order / MarketOrder

Screenshots (last two, 2026-04-22)

On-disk copies are in docs/pr-55-attachments/ on this branch (filenames: ibkr-tws-1.png, ibkr-tws-2.png).

1. Gateway / TWS startup (04:19) — usfarm connect thread, ndc1.ibllc.com:4000, first hop 64.190.197.40, SSL on.

2. API stream (04:22) — market/HMDS/secdef farms OK; AccountCode U24673021, AccountReady true, AccountType INDIVIDUAL, accrued fields 0.00 (BASE/USD) at capture time.

Debug

If anything above blocks you, paste the exact message from TWS (order rejection / API) on the next PR and we can map it to a code or config change.

[IamJasonBian]

Code Review — PR #55

Overall: Clean addition of an IBKR mock client and a live TWS client via ib_insync. Good test layering (mock unit tests, mocked integration, opt-in live). The lazy import pattern in brokers/__init__.py is well done.

Issues

1. cancelOrder call signature is wrong — In ib_tws_client.py:150, self._ib.cancelOrder(t.contract, t.order) passes two arguments, but ib_insync.IB.cancelOrder() takes only an Order object. This will raise TypeError at runtime. Should be self._ib.cancelOrder(t.order).

2. ib_insync is a hard dependency — Added to requirements.txt as required, meaning all environments (CI, publish-only Render deploys) must install it even if they never use IBKR. Consider keeping it optional (commented like alpaca-py) and relying on the existing ImportError guard in ib_tws_client.py.

3. Version bump 0.1.0 → 1.0.0 is premature — This PR adds a mock and a thin TWS wrapper, but IbTwsClient isn't wired into the engine (Executor, SafeCashBot). There are also 8 open PRs with unmerged core features (execution layer, PDT gate, executor pattern). Bumping to 1.0 signals production-readiness — consider deferring the version bump until the IBKR path is integrated end-to-end.

4. MockIbkrClient and IbTwsClient don't share an interface — PR Executor pattern, RobinhoodClient, deferred task scheduler #53 introduced a Brokerage ABC, but neither client here implements it. Return types also differ: MockIbkrClient.place_limit_order returns Dict, while IbTwsClient.place_limit_order returns int. This makes them non-interchangeable without adapters.

5. connect() doesn't clean up on failure — If IB.connect() raises (TWS not running), self._ib is left as a partially-initialized IB() instance. disconnect() would then call _ib.disconnect() on a never-connected object. Consider wrapping in try/except and setting self._ib = None on failure.

6. Account number visible in PR screenshots — The TWS screenshot shows AccountCode U24673021. Same concern raised on PR Executor pattern, RobinhoodClient, deferred task scheduler #53 about hardcoded account numbers in a public repo.

Strengths

• Lazy __getattr__ import keeps MockIbkrClient imports cheap
• Live test guards (TWS_INTEGRATION, TWS_PLACE_ORDER, port 7497 refusal) are well thought out
• pytest.ini integration marker is a good addition
• Mock client's position tracking with avg cost blending is thorough

---

Generated by Claude Code

[IamJasonBian]

Redundancy Review — PR #55

Focused follow-up on redundant code patterns in this PR and across open branches.

Within this PR

1. Action = Literal["BUY", "SELL"] defined twice — Identical type alias in both ibkr_mock.py:15 and ib_tws_client.py:38. Extract to a shared brokers/_types.py or brokers/__init__.py so broker modules import from one place.

2. Connection guard repeated 4× in ib_tws_client.py — get_account_values, get_account_summary_map (via get_account_values), get_positions, place_limit_order, and cancel_order all check if not self._ib or not self._ib.isConnected(): raise RuntimeError(...). Extract a _require_connected() helper:

   def _require_connected(self) -> IB:
       if self._ib is None or not self._ib.isConnected():
           raise RuntimeError("not connected; call connect() first")
       return self._ib

   Then each method becomes ib = self._require_connected() — eliminates 4 duplicate guard blocks.

3. get_positions() return schemas diverge silently — MockIbkrClient returns {symbol, position, avgCost, marketValue, unrealizedPNL}, while IbTwsClient returns {symbol, position, avgCost, conId, exchange, currency}. Only symbol/position/avgCost overlap. Any code that consumes positions from "a broker" will break when switching between them. Define a minimal PositionRecord TypedDict with the shared fields, and have both methods return it.

Cross-PR redundancy (branches #32, #53, #55)

4. Three parallel broker/execution abstractions — The open branches are diverging:

   • PR Add execution layer: order lifecycle management #32 (execution/) adds ExecutionManager, PriceDiscovery, FillMonitor, SlippageController
   • PR Executor pattern, RobinhoodClient, deferred task scheduler #53 (utils/brokerage.py) adds a Brokerage ABC with 8 abstract methods, plus Executor and TradeTask
   • PR feat(1.0): IBKR mock + IbTwsClient (ib_insync), tests #55 (brokers/) adds MockIbkrClient and IbTwsClient with no ABC

   These three branches each define their own view of what a broker looks like and how orders flow. Before any of them merge, it's worth picking one broker protocol (PR Executor pattern, RobinhoodClient, deferred task scheduler #53's Brokerage ABC is the most complete) and having PRs feat(1.0): IBKR mock + IbTwsClient (ib_insync), tests #55 and Add execution layer: order lifecycle management #32 implement/depend on it. Otherwise merging them in sequence will require significant rework to reconcile the interfaces.

5. place_limit_order return type conflict — MockIbkrClient.place_limit_order returns Dict[str, Any], IbTwsClient.place_limit_order returns int (order ID), and PR Executor pattern, RobinhoodClient, deferred task scheduler #53's Brokerage.place_limit_buy returns Optional[Dict]. Three PRs, three return types for the same logical operation. Aligning on a single return type now will prevent adapter code later.

Recommendation

Start by extracting Action, PositionRecord, and the connection guard into shared modules within this PR. For the cross-PR alignment, consider merging #53's Brokerage ABC first (after addressing its review feedback), then rebasing this PR to implement that interface.

---

Generated by Claude Code

[IamJasonBian]

Stale — 19 days, no commits since Apr 22. Two prior reviews identified a runtime bug (cancelOrder call signature in ib_tws_client.py:150 passes (contract, order) but ib_insync expects just (order)), a premature 1.0.0 version bump, and significant cross-PR redundancy.

Redundancy triage (this PR + open branches)

The redundancy review from May 4 still stands — key items to resolve before merging:

1. Within this PR: Extract duplicate Action type alias and the 4× connection guard into shared modules (brokers/_types.py, _require_connected() helper). Low effort, high clarity.

2. Cross-PR alignment is the blocker: PRs Add execution layer: order lifecycle management #32, Executor pattern, RobinhoodClient, deferred task scheduler #53, and feat(1.0): IBKR mock + IbTwsClient (ib_insync), tests #55 each define their own broker/execution interface. place_limit_order alone has three incompatible return types (Dict, int, Optional[Dict]). Recommend landing PR Executor pattern, RobinhoodClient, deferred task scheduler #53's Brokerage ABC first (after its review fixes), then rebasing this PR to implement that interface. This avoids adapter code later and unifies the position/order schemas across MockIbkrClient and IbTwsClient.

3. PRs to close as superseded: Use state-logs blob store for state uploads #42 (subsumed by Fix PDT gate to detect same-day filled orders (critical PDT protection) #45/Reduce maximum order size to 50 shares #50), Reduce maximum order size to 50 shares #50 (strict subset of Fix PDT gate to detect same-day filled orders (critical PDT protection) #45 plus scope creep). Closing those reduces noise and merge-conflict surface.

Please fix the cancelOrder bug and rebase once the broker abstraction direction is decided.

---

Generated by Claude Code

[IamJasonBian]

Stale — clean broker adapters, minor code duplication to fix.

The MockIbkrClient and IbTwsClient are well-structured. The lazy import pattern in __init__.py is a nice touch. Two cleanup items:

1. Shared Action type alias — defined identically in both broker files. Extract to one place.
2. Simplify _ensure_event_loop — the nested try/except is redundant (see inline).
3. Version bump to 1.0.0 is mixed into this broker PR (version.py, package.json, README). Consider separating the version bump into its own commit or PR — it signals a release milestone that's broader than "added IBKR broker adapters."

The MockIbkrClient is 231 lines — consider whether place_market_order and _fill_market could share more logic with place_limit_order via a common _submit_order helper (saves ~20 lines of repeated dict construction).

---

Generated by Claude Code

[IamJasonBian]

Duplicate type alias — share with ib_tws_client.py.

Action = Literal["BUY", "SELL"] is defined identically in both ibkr_mock.py and ib_tws_client.py. Move it to brokers/__init__.py (or a shared brokers/types.py) and import from there. One definition to maintain instead of two.

---

Generated by Claude Code

[IamJasonBian]

_ensure_event_loop has a redundant fallback path.

On Python 3.10+, get_event_loop() in a thread without a running loop raises RuntimeError (or emits a deprecation warning and creates one, depending on version). The inner try/except is effectively the same as the outer one. Simplify to:

def _ensure_event_loop() -> None:
    try:
        asyncio.get_running_loop()
    except RuntimeError:
        asyncio.set_event_loop(asyncio.new_event_loop())

This is clearer and handles all Python versions correctly — if there's a running loop, do nothing; otherwise create one.

---

Generated by Claude Code