Skip to content

fix: make router match params optional#963

Open
realfishsam wants to merge 3 commits into
mainfrom
fix/easy-router-optional-params
Open

fix: make router match params optional#963
realfishsam wants to merge 3 commits into
mainfrom
fix/easy-router-optional-params

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Make compareMarketPrices params optional to match sibling router methods
  • Make fetchRelatedMarkets params optional to remove the no-argument asymmetry

Fixes #448

Test Plan

  • git diff --check

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Makes router match/related-market params optional through BaseExchange signatures, OpenAPI/method-verbs, and both SDK clients/docs.

Blast Radius

Router-facing BaseExchange methods, OpenAPI/method verb schema, Python/TypeScript SDK clients and API references.

Consumer Verification

Before (base branch):
Base required an args/params object for compareMarketPrices and fetchRelatedMarkets, preventing no-argument consumer calls even though the router can supply defaults.

After (PR branch):
PR makes params optional and avoids sending an empty args element when omitted. Core build/Jest passed (24 suites, 644 tests); root verification then failed in Python SDK tests due missing pytest.

Test Results

  • Build: PASS
  • Unit tests: CORE PASS (24 suites, 644 tests; root verification blocked by missing pytest)
  • Server starts: PASS during root verification
  • E2E smoke: NOT VERIFIED with sidecar no-arg router call

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: OK — BaseExchange/OpenAPI/method-verbs/SDK docs updated together
  • Financial precision: N/A
  • Type safety: OK
  • Auth safety: N/A

Semver Impact

patch -- backwards-compatible optional parameter fix.

Risk

No-argument router calls were not executed through the sidecar in this run.

…nal-params

# Conflicts:
#	core/api-doc-config.generated.json
@realfishsam

Copy link
Copy Markdown
Contributor Author

Merge-refresh note from the low-hanging PR automation:

  • Merged current origin/main into this branch and resolved the only textual conflict in core/api-doc-config.generated.json (generated timestamp conflict).
  • Local targeted checks after the merge refresh:
    • npm run build --workspace=pmxt-core
    • python3 -m compileall -q sdks/python/pmxt
    • git diff --check
  • I did not merge this PR: GitHub generated-sync checks turned red after the refresh. Re-running the documented generators locally would produce broad generated/client/docs drift, including removal of existing hosted-mode SDK sections unrelated to this focused router optional-params PR, so I left that drift uncommitted.

Current blocker: generated-sync drift needs a safer generator update or a coordinated generated-output refresh before this can be considered merge-ready.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Makes router match params optional across core, OpenAPI artifacts, and both SDK clients for compareMarketPrices / fetchRelatedMarkets. This lets consumers call the router discovery helpers without passing an empty params object.

Blast Radius

BaseExchange router signatures, server method metadata/OpenAPI, Python and TypeScript SDK method signatures, and generated API docs.

Consumer Verification

Before (base branch):
SDK signatures and OpenAPI required one FetchMatchesParams argument for the affected router methods, so consumers had to pass {}/empty params even for browse-style discovery.

After (PR branch):
Python/TS clients omit the args element when params is absent, and OpenAPI no longer requires the request args array for the changed route. I also started the PR branch sidecar and verified /health returned {"status":"ok"...}. A direct unauthenticated router POST to /api/router/fetchMarketMatches reached the server and returned the expected auth-layer response (Unauthorized: Invalid or missing access token), so authenticated consumer-path behavior remains unverified here.

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Core unit tests: PASS (27 suites / 652 tests passed; 1 suite / 3 tests skipped)
  • Python syntax: PASS (py_compile on changed Python client)
  • Server starts: PASS (/health OK)
  • Full npm test: FAIL in this checkout during Python SDK collection because pmxt_internal / eth_account are not available.
  • E2E smoke: Inconclusive without valid PMXT API auth token for router endpoint

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: OK (BaseExchange, method metadata, OpenAPI/docs, and SDKs changed together)
  • Financial precision: N/A
  • Type safety: OK
  • Auth safety: OK (auth middleware still gates router API)

Semver Impact

patch -- backward-compatible optional-params relaxation.

Risk

I verified artifact consistency and server startup, but did not verify an authenticated router result set through the hosted/local API.

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.

compareMarketPrices and fetchRelatedMarkets take non-optional params while sibling router methods take optional params

1 participant