Skip to content

fix(sdk): compute detailed execution prices locally#1308

Merged
realfishsam merged 1 commit into
mainfrom
fix/issue-1306-local-execution-price
Jun 26, 2026
Merged

fix(sdk): compute detailed execution prices locally#1308
realfishsam merged 1 commit into
mainfrom
fix/issue-1306-local-execution-price

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Make TypeScript getExecutionPriceDetailed a synchronous local order-book walk instead of a sidecar HTTP call.
  • Make Python get_execution_price_detailed compute the same pure result locally.
  • Add focused TS/Python regressions for full and partial fills with sidecar autostart disabled.

Fixes #1306

Test Plan

  • git diff --check HEAD~1..HEAD
  • node sdks/typescript/scripts/generate-client-methods.js
  • node sdks/python/scripts/generate-client-methods.js
  • npm test --workspace=pmxtjs -- --runTestsByPath tests/execution-price-detailed-local.test.ts --runInBand blocked locally: this checkout lacks sdks/typescript/generated/src/index.js; npm run generate --workspace=pmxtjs is blocked by missing java in the cron environment.
  • PYTHONPATH=sdks/python python3 sdks/python/tests/test_execution_price_detailed_local.py blocked locally: this checkout lacks generated pmxt_internal.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Moves get_execution_price_detailed calculation into the Python and TypeScript SDK clients so consumers can compute detailed fill price from a supplied OrderBook without calling the sidecar-only getExecutionPriceDetailed route.

Blast Radius

Python SDK Exchange.get_execution_price_detailed, TypeScript SDK Exchange.getExecutionPriceDetailed, and their local unit-test coverage. No core exchange normalizer/OpenAPI schema changes.

Consumer Verification

Before (base branch):
The SDK method builds a sidecar request to POST /api/{exchange}/getExecutionPriceDetailed with the order book, side, and amount; consumer behavior depends on a running sidecar route.

Polymarket(auto_start_server=False).get_execution_price_detailed(order_book, "buy", 8)
# base implementation attempts sidecar HTTP POST /api/polymarket/getExecutionPriceDetailed

After (PR branch):
Static review of sdks/python/pmxt/client.py and sdks/typescript/pmxt/client.ts shows the method now walks asks for buys and bids for sells, sorts the book side by best execution price, returns partial-fill metadata, and raises on non-positive amount. Example from the added tests:

asks=[(0.52, 4), (0.50, 6)], side="buy", amount=8
# fills 6 @ 0.50 + 2 @ 0.52 => price 0.505, filled_amount=8, fully_filled=True

Runtime SDK verification was blocked by missing generated SDK artifacts in this checkout, not by this PR's changed logic.

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Unit tests: NOT VERIFIED for SDK tests. npx jest -c sdks/typescript/jest.config.cjs sdks/typescript/tests/execution-price-detailed-local.test.ts --runInBand fails before tests run with TS2307: Cannot find module '../generated/src/index.js'; uv run pytest tests/test_execution_price_detailed_local.py -q fails dependency resolution because project requires-python >=3.8 conflicts with urllib3>=2.7.0 requiring Python >=3.10 for the Python 3.9 split.
  • Server starts: NOT RUN (SDK-local behavior change; core build passed)
  • E2E smoke: NOT VERIFIED (not sidecar-observable after the change)

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: OK for existing SDK numeric model; no new float-sensitive venue/API serialization path added.
  • Type safety: OK in changed hand-written code; SDK generated-import blocker prevented full TS test execution.
  • Auth safety: N/A

Semver Impact

patch -- SDK-local bug fix/removal of unnecessary sidecar dependency for an existing helper method.

Risk

The old TypeScript getExecutionPrice helper still walks order-book levels in caller-supplied order, while the new detailed method sorts best-first. That is outside this PR's direct change but means the two helpers can disagree if consumers pass an unsorted book. Full SDK test execution remains unverified until generated artifacts/dependency resolution are available.

@realfishsam realfishsam merged commit 94296db into main Jun 26, 2026
12 checks passed
@realfishsam realfishsam deleted the fix/issue-1306-local-execution-price branch June 26, 2026 06:48
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.

getExecutionPriceDetailed is a synchronous pure function in core but async with sidecar round-trip in both SDKs

1 participant