Skip to content

fix: initialize Probable order book resolver queues#510

Merged
realfishsam merged 3 commits into
pmxt-dev:mainfrom
iccccccccccccc:fix-probable-orderbook-resolvers
May 24, 2026
Merged

fix: initialize Probable order book resolver queues#510
realfishsam merged 3 commits into
pmxt-dev:mainfrom
iccccccccccccc:fix-probable-orderbook-resolvers

Conversation

@iccccccccccccc

@iccccccccccccc iccccccccccccc commented May 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #264

Changes

  • Replace the guarded orderBookResolvers.get(...)! push path with an explicit queue initializer.
  • Add a regression test so Probable websocket resolver queueing does not reintroduce the non-null assertion pattern.
  • Include the current generator fixes from fix: initialize Baozi order book resolver queues #509 so this PR passes the existing exchange/SDK drift gates while main is still out of sync.

Verification

  • npm --workspace=pmxt-core test -- --runTestsByPath test/pipeline/probable-websocket-resolvers.test.ts --runInBand
  • npx tsc -p core/tsconfig.json --noEmit
  • node core/scripts/check-exchange-drift.js
  • node sdks/typescript/scripts/generate-client-methods.js; git diff --exit-code sdks/typescript/pmxt/client.ts
  • node sdks/python/scripts/generate-client-methods.js; git diff --exit-code sdks/python/pmxt/client.py
  • python -m py_compile sdks/python/pmxt/_exchanges.py sdks/python/pmxt/__init__.py sdks/python/pmxt/client.py
  • git diff --check

@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: PASS (NOT VERIFIED)

What This Does

Fixes a non-null assertion bug in Probable's WebSocket order book resolver queueing (issue #264) by extracting queue initialization into a getOrderBookQueue() helper. Also carries forward two generator fixes from #509: Python SDK class naming for underscore-delimited exchanges (Polymarket_us -> PolymarketUS), and hardcoded fetchOrderBook special-case generation in both SDK client generators.

Blast Radius

  • Core: probable/websocket.ts only -- no other exchanges touched
  • Python SDK: _exchanges.py, __init__.py, generate-python-exchanges.js -- class name change is a breaking change for Python consumers using pmxt.Polymarket_us
  • Both SDKs: generate-client-methods.js scripts -- adds fetchOrderBook special-case override to generators (output matches what is already in the committed client files)
  • Docs: API_REFERENCE.md in both SDKs -- updated to reflect FetchOrderBookParams type and close method (replacing stale testDummyMethod reference)

Test Results

  • Build: NOT RUN (worktree has no node_modules, npm install was blocked by permissions)
  • Unit tests: NOT RUN locally
  • CI checks: ALL PASS (6/6 -- exchange drift, SDK sync, API reference, compliance)

Findings

  1. Polymarket_us -> PolymarketUS is a breaking change for Python consumers (sdks/python/pmxt/_exchanges.py:368, __init__.py:20). Any user doing import pmxt; client = pmxt.Polymarket_us() will get an ImportError. The exchange_name="polymarket_us" wire string is unchanged so the sidecar still works, but the public API surface changed. This may warrant a deprecation alias or a semver minor/major bump depending on project policy. Note: the old name Polymarket_us is not Pythonic (PEP 8), so the rename to PolymarketUS is correct -- but it is still a breaking symbol rename.

  2. The test is a source-text regex guard, not a behavioral test (core/test/pipeline/probable-websocket-resolvers.test.ts:5-12). It asserts that the pattern orderBookResolvers.get(...)! does not appear in the source file. This prevents regression of the specific anti-pattern but does not verify the runtime fix. Acceptable for a regression guard, but worth noting.

  3. Seven other websocket files have the identical unsafe pattern (baozi, opinion, kalshi [x2], myriad, polymarket, gemini-titan). These are out of scope for this issue ([non-null] probable/websocket.ts: unsafe Map.get()! assertion on orderBookResolvers #264 is Probable-specific), but the same Map.get()! bug exists there. This is informational, not a blocker.

  4. The fetchOrderBook generator special-case duplicates existing hand-maintained logic (sdks/python/scripts/generate-client-methods.js:390-422, sdks/typescript/scripts/generate-client-methods.js:384-419). The generated output matches what is already committed in client.py and client.ts, so this is a no-op change that ensures future regeneration produces the same output. No issues found.

  5. The toClassName regex change (core/scripts/generate-python-exchanges.js:33) splits on both - and _, and special-cases us -> US. This correctly handles polymarket_us -> PolymarketUS, kalshi-demo -> KalshiDemo, and gemini-titan -> GeminiTitan. No other exchange names contain underscores, so the blast radius is limited to polymarket_us.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A (no new fields)
  • OpenAPI sync: N/A (no BaseExchange.ts changes)
  • Type safety: OK (non-null assertion removed, no any introduced)

Semver Impact

minor (or patch depending on policy) -- the core fix is patch-level, but the Python class rename Polymarket_us -> PolymarketUS is a breaking symbol change for Python SDK consumers. If any users reference Polymarket_us by name, they will break on upgrade. If the project treats SDK class names as part of the public API, this is technically a major change -- but given that Polymarket_us was likely a bug in the first place (non-PEP-8), treating it as minor with a changelog note seems reasonable.

Risk

  • Local build and tests were not run (CI passed all 6 checks)
  • The Python class rename has no deprecation alias -- existing pmxt.Polymarket_us users will break silently on upgrade
  • The 7 other websocket files with the same Map.get()! pattern remain unfixed

@realfishsam realfishsam merged commit a4f50b9 into pmxt-dev:main May 24, 2026
6 checks passed
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.

[non-null] probable/websocket.ts: unsafe Map.get()! assertion on orderBookResolvers

2 participants