Skip to content

fix(core): centralize time defaults#950

Open
realfishsam wants to merge 1 commit into
mainfrom
fix/easy-maintenance-constants
Open

fix(core): centralize time defaults#950
realfishsam wants to merge 1 commit into
mainfrom
fix/easy-maintenance-constants

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Add shared core time utilities for timestamp unit detection and WebSocket reconnect defaults
  • Standardize timestamp seconds/milliseconds heuristics across core and SDK server managers
  • Standardize exchange WebSocket reconnect fallbacks on a shared 5s default and preserve explicit 0 values with nullish coalescing

Fixes #206
Fixes #225

Test Plan

  • git diff --check
  • python3 -m py_compile sdks/python/pmxt/server_manager.py
  • npm run build --workspace=pmxt-core (attempted; tsc was killed by the local environment with exit 137)
  • npx tsc --noEmit --pretty false --incremental false --skipLibCheck (attempted; killed by the local environment with exit 137)

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: FAIL

What This Does

Centralizes reconnect/timestamp defaults across exchange WebSockets, normalizers, and SDK server-manager status conversion. This should be an internal cleanup with no intended SDK response-shape change.

Blast Radius

Core WebSocket clients for Gemini Titan, Kalshi, Limitless, Opinion, Probable; Polymarket/Probable normalizers; Python and TypeScript server-manager status helpers.

Consumer Verification

Before (base branch):
Base branch has duplicated literals such as 5000ms reconnect delays and local timestamp seconds/ms conversion. I could not run a consumer API comparison because the PR branch does not build.

After (PR branch):
PR branch fails before the sidecar can start: npm run build --workspace=pmxt-core reports src/exchanges/probable/websocket.ts(2,33): error TS2440: Import declaration conflicts with local declaration of 'QueuedPromise'.

Test Results

  • Build: FAIL (npm run build --workspace=pmxt-core TypeScript error)
  • Unit tests: NOT RUN (build failed)
  • Server starts: FAIL (server not started because build failed)
  • E2E smoke: FAIL (blocked by build failure)

Findings

  1. core/src/exchanges/probable/websocket.ts:2 imports QueuedPromise from ../../types while the same file still declares local interface QueuedPromise<T> at line 7. TypeScript rejects the duplicate name, so the core package cannot build and the sidecar cannot ship.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: ISSUE — duplicate QueuedPromise declaration prevents compilation
  • Auth safety: N/A

Semver Impact

patch -- intended internal cleanup, but currently unmergeable because it breaks the build.

Risk

No runtime behavior could be verified because the TypeScript build fails.

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.

Magic numbers: WebSocket reconnect intervals Magic numbers: Timestamp conversion thresholds

1 participant