Skip to content

fix(core): avoid float drift in venue decimal math#955

Open
realfishsam wants to merge 2 commits into
mainfrom
fix/easy-float-decimal-arithmetic
Open

fix(core): avoid float drift in venue decimal math#955
realfishsam wants to merge 2 commits into
mainfrom
fix/easy-float-decimal-arithmetic

Conversation

@realfishsam

@realfishsam realfishsam commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add decimal string arithmetic helpers for rounding, complements, ratios, and scaled integer conversion
  • use decimal-safe helpers in Gemini Titan, Kalshi, Opinion, Polymarket US, and Smarkets float-sensitive paths
  • add focused unit tests for the decimal helpers

Fixes #718
Fixes #714
Fixes #709
Fixes #674
Fixes #671
Fixes #666
Fixes #664
Fixes #295
Fixes #286
Fixes #229
Fixes #713

Test Plan

  • npm test --workspace=pmxt-core -- decimal-math.test.ts --runInBand
  • npm run build --workspace=pmxt-core (attempted; process was killed with exit 137/OOM before TypeScript diagnostics)

github-code-quality[bot]
github-code-quality Bot found potential problems Jun 8, 2026
View reviewed changes
Comment thread core/src/exchanges/gemini-titan/normalizer.ts
const bestAsk = contract.prices?.bestAsk ? parseFloat(contract.prices.bestAsk) : 0.5;
const bestBidRaw = contract.prices?.bestBid ?? '0.5';
const bestAskRaw = contract.prices?.bestAsk ?? '0.5';
const bestBid = parseFloat(bestBidRaw);
Comment thread core/src/exchanges/gemini-titan/normalizer.ts
const bestBidRaw = contract.prices?.bestBid ?? '0.5';
const bestAskRaw = contract.prices?.bestAsk ?? '0.5';
const bestBid = parseFloat(bestBidRaw);
const bestAsk = parseFloat(bestAskRaw);
@realfishsam

realfishsam commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Introduces decimal-string helpers and applies them in venue price/amount arithmetic to reduce floating-point drift in financial fields.

Blast Radius

Core normalizers/order helpers for Gemini Titan, Kalshi, Opinion, Polymarket US, Smarkets, plus new decimal-math tests.

Consumer Verification

Before (base branch):
Base used JS floating-point arithmetic for midpoint, complement, subtraction, proportional volume, and scaled integer calculations in several venue paths.

After (PR branch):
PR replaces those paths with BigInt/string decimal helpers and adds core/test/utils/decimal-math.test.ts. Core build passed and Jest passed (25 suites, 647 tests); root verification later failed only at missing pytest.

Test Results

  • Build: PASS
  • Unit tests: CORE PASS (25 suites, 647 tests; root verification blocked by missing pytest)
  • Server starts: PASS during root verification
  • E2E smoke: NOT VERIFIED through live venue API

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: OK — targeted decimal helper tests passed
  • Type safety: OK
  • Auth safety: N/A

Semver Impact

patch -- financial precision bug fix without API shape change.

Risk

Live venue payloads were not sampled, so this review verifies the arithmetic/unit path rather than real exchange data.

@realfishsam
Merge remote-tracking branch 'origin/main' into fix/easy-float-decima…
8ef1395 
…l-arithmetic

# Conflicts:
#	core/src/exchanges/gemini-titan/normalizer.ts
#	core/src/exchanges/kalshi/normalizer.ts
@realfishsam

realfishsam commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Maintenance automation follow-up: I refreshed this branch with origin/main and resolved the two mechanical conflicts in Gemini Titan/Kalshi normalizers, then ran local targeted validation:

  • npm test --workspace=pmxt-core -- decimal-math.test.ts --runInBand ✅
  • npm run build --workspace=pmxt-core ✅

I did not merge because generated-sync checks are now red after the base refresh. Running the client/docs generators locally produces broad hosted-client/doc drift unrelated to this focused float-safety PR, so I left that generated drift out of the branch per the focused-PR generator-drift policy.

@realfishsam

realfishsam commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Adds decimal-string arithmetic helpers and replaces several floating-point price/amount calculations across Gemini Titan, Kalshi, Opinion, Polymarket US, and Smarkets. This reduces precision drift in prices, balances, order books, and PnL calculations that SDK consumers receive.

Blast Radius

Core exchange normalizers/price helpers for five venues plus new core/src/utils/decimal-math.ts and unit tests. SDK surfaces are affected only through normalized response values.

Consumer Verification

Before (base branch):
Examples such as 1 - 0.425, (0.43 + 0.45) / 2, Math.round(probability * 10000), and (currentPrice - entryPrice) * size used binary floating-point arithmetic in venue normalizers/price helpers.

After (PR branch):
The new decimal helpers cover fixed rounding, complements, averages, products, division, tick rounding, and scaled integer conversion. The added decimal-math unit tests passed as part of core tests, including cases like toFixedDecimal(0.575, 2) === "0.58", complementDecimal("0.425", 4) === 0.575, and toScaledInteger(0.10015, 10000) === 1002.

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Core unit tests: PASS (28 suites / 655 tests passed; 1 suite / 3 tests skipped)
  • Decimal utility tests: PASS (included in core test count)
  • Full npm test: FAIL in this checkout during Python SDK collection because pmxt_internal / eth_account are not available.
  • Server starts: Not separately run for this PR
  • E2E smoke: Not run against live affected venues

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: OK (targeted decimal helpers and tests cover the changed arithmetic)
  • Type safety: OK
  • Auth safety: N/A

Semver Impact

patch -- precision bug fix with unchanged API shape.

Risk

Live venue payloads were not exercised; this review verifies helper behavior and call-site replacement, not every external exchange edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment