feat(data): align combo fields and history ceilings

[kartojal]

DEV-436: Preserves combo position entry-basis fields and respects Data history pagination ceilings.

Tests: lint, typecheck, build, unit PASS; public live PASS.

---

Note

Medium Risk
Changes portfolio field types/parsing and pagination behavior for widely used history APIs; mis-handling could break consumers expecting numeric decimals or silently truncating history, though the new rejects are intentional contract alignment.

Overview
Aligns the TypeScript SDK with Data API contract updates for combo positions and history pagination.

Combo positions now expose grossEntryCostUsdc and entryFeesUsdc, parsed with DecimalStringSchema so entry-basis USDC values stay exact strings and are not coerced through JSON numbers that can lose precision.

Trades and wallet activity offset pagination enforces documented ceilings (10,000 and 5,000 respectively): decodeOffsetCursor rejects cursors beyond the max with UserInputError before any HTTP call, and docs steer callers toward start/end windows for deeper history. paginate().firstPage() is async so synchronous cursor validation failures reject the promise (tests updated for Perps session pagination).

^{Reviewed by Cursor Bugbot for commit a438dbf. Bugbot is set up for automated code reviews on this repo. Configure here.}

[brunson-bot]

The 10_000 / 5_000 values and the inclusive boundary are exactly right — I verified against the live Data API:

/trades?offset=10000  -> 200
/trades?offset=10001  -> 400 {"error":"max historical trades offset of 10000 exceeded"}
/activity?offset=5000 -> 200
/activity?offset=5001 -> 400 {"error":"max historical activity offset of 5000 exceeded"}

So state.offset > maxOffset and nextOffset <= maxOffset land on the correct side of both caps. Comments below, nothing blocking.

---

[issue] pagination.ts:140 — stopping at the ceiling is indistinguishable from running out of data.

cappedOffsetContinuation returns hasMore: false, nextCursor: undefined, which is byte-identical to genuine exhaustion. A consumer doing for await (const page of client.listTrades({ user })) to total up a wallet's history now gets a clean loop exit at the cap where it previously got a RequestRejectedError from the 400 — the truncation is real but there is no runtime signal for it. Page.hasMore is documented at pagination.ts:14 as "Whether another page may be available", which is not true at the ceiling.

It's also pageSize-dependent, because the cap is on offset alone and not offset + limit — I confirmed /trades?offset=10000&limit=500 returns a full 500 rows. So the same query returns ~10,500 items at pageSize: 500 and ~20,000 at pageSize: 10000, with no way for the caller to tell either result apart from a complete history.

The Data API deliberately picked a 400 over a silent clamp here ("Requests past the cap are rejected with a 400 (never silently clamped)"). Worth deciding whether the SDK wants to keep a runtime signal — a distinct error on the page after the ceiling, or a field on Page — rather than only the action TSDoc, which a for await loop never reads.

[issue] pagination.ts:66 — firstPage() throws synchronously from a method typed Promise<Page<T>>.

firstPage() isn't async and evaluates fetchPage(cursor) in argument position, so decodeOffsetCursor's throw escapes synchronously. client.listTrades({ cursor }).firstPage().catch(handle) won't catch it. That shape is pre-existing for tampered cursors, but this PR routes a new case through it: a cursor at offset 10,500 persisted by an older SDK version used to surface as an async RequestRejectedError and is now a sync UserInputError on upgrade.

The two new integration tests are the only ones in tests/integration using expect(() => …).toThrow(…) instead of await expect(…).rejects — everything else in that suite is async — so they codify the shape. Making it async firstPage() converts the whole decode path to rejections and matches createEmptyPaginator's firstPage at pagination.ts:51, which is already async.

[nit] actions/portfolio.ts:151 — /positions carries the same documented ceiling.

data-openapi.yaml gives /positions an offset maximum: 10000 too, and listPositions still calls the uncapped decodeOffsetCursor. Deliberate scope call? Leaving it also means two idioms for offset continuation live side by side — the new helper in activity.ts, the inline encodeOffsetCursor form in the other ten paginators. (I couldn't reproduce a 400 on /positions with a low-position wallet, so enforcement there is spec-only on my side.)

[nit] actions/activity.ts:89 — the same docs paragraph also documents a default time window.

Docs PR 263 says omitting start (or passing 0) limits both endpoints to "the most recent ~3 years", and for /trades that start can only narrow — not extend — market- and event-scoped requests. The new TSDoc points callers at start/end as the escape hatch for deeper history without mentioning that the default is already windowed, which is the part that would surprise someone reading it. One sentence in the same block would close DEV-436 against the whole paragraph.

[nit] bindings/src/data/portfolio.test.ts — the assertion can't fail.

DecimalishSchema resolves to DecimalStringSchema for strings, which is z.string().transform(toDecimalString) (shared.ts:475) — a pure brand cast, so the string branch preserves any input and this holds for any value. The branch where the docs' "never through a float" warning actually bites is the z.number() arm at shared.ts:491, where JSON.parse has already truncated before Zod runs; that one is untested and unconstrained by the new fields. Per AGENTS.md this is the "narrow schema change" case where typecheck plus the portfolio.test.ts integration assertion you already added is enough — I'd drop the 47-line fixture, or repoint it at the number arm if you think that's a real risk for these fields.

[brunson-bot]

Round 2 — both round-1 [issue]s are closed, and I finished verifying the scope question I raised as a nit. Four small things left, nothing blocking.

Closed:

• async firstPage() (pagination.ts:66) routes the whole decode path through rejections. The async iterator was already safe — a synchronous throw inside an async * body rejects next(), so for await propagates it — and the two integration tests plus the Perps session test now read like the rest of the suite.
• Truncation is loud again. offsetContinuation hands back the over-ceiling cursor and decodeOffsetCursor rejects it on continuation, so a for await loop past 10,000 raises instead of exiting clean. UserInputError is already a member of both ListTradesError and ListActivityError, so it stays inside the documented contract.
• The new TSDoc matches data-openapi.yaml on both endpoints — /trades ("start can only narrow market/event-scoped windows, not extend them") and /activity ("with sortDirection=ASC, omitting start already reads from the beginning"). Accurate on all three claims.

Withdrawing my /positions nit — the scope here is exactly right. I checked all nine documented offset maxima in the spec against the live API:

/trades                   offset=10001  -> 400   <- capped by this PR
/activity                 offset=5001   -> 400   <- capped by this PR
/positions                offset=10001  -> 200 []
/closed-positions         offset=100001 -> 200 []
/v1/market-positions      offset=10001  -> 200 []
/v1/leaderboard           offset=1001   -> 200
/v1/builders/leaderboard  offset=1001   -> 200
/v1/positions/combos      offset=100001 -> 400   (SDK sends the server cursor, never an offset)
/v1/activity/combos       offset=10001  -> 200   (SDK sends the server cursor, never an offset)

Of the endpoints that actually enforce their documented ceiling, the only two the SDK paginates by offset are the two you capped. The rest publish a maximum the service doesn't apply.

---

[nit] pagination.ts:20 — hasMore now has a third outcome its doc doesn't cover.

The comment says a full page reports true and "the follow-up request returns an empty final page". At the ceiling the follow-up request throws instead. Page is publicly exported (index.ts:44, export type *) and this comment is the only place hasMore's contract is written down, so a consumer reading it will not expect the loop to raise. One sentence — on offset-capped history methods the follow-up request rejects once the endpoint maximum is passed — would close it.

[nit] pagination.ts:105 — offsetContinuation carries no cap logic, so it is the plain inline pattern with eleven unconverted copies.

With the ceiling living entirely in decodeOffsetCursor, this helper is identical to what leaderboards.ts:147/286, teams.ts:121, series.ts:133, markets.ts:610, comments.ts:158/307, tags.ts:171, market-clarifications.ts:134 and portfolio.ts:169/288 already do inline (search.ts:148 is the 1-based variant). Two call sites plus eleven copies of the same six lines is the shape the helper rule in AGENTS.md is aimed at — either sweep them onto it or inline it back into activity.ts. Fine as a follow-up.

[nit] websockets/perps/session.test.ts:1509 — the rewrite dropped the instanceof check.

toMatchObject({ name: UserInputError.name, ... }) also passes for a plain Error with name assigned; the old toBeInstanceOf did not. The two integration tests you changed in the same commit kept the instance check via .rejects.toThrow(UserInputError). Adding await expect(...).rejects.toBeInstanceOf(UserInputError) next to the toMatchObject gets both back.

[nit] bindings/src/data/portfolio.ts:215-216 — two of the eight amount fields on this object now parse differently from the other six.

DecimalishSchema's string arm is DecimalStringSchema (shared.ts:475/:489), so against the live response this changes nothing: I pulled /v1/positions/combos and every amount comes back as a JSON string, these two included. The real delta is that a numeric gross_entry_cost_usdc would now fail the whole page with UnexpectedResponseError while a numeric entry_cost_usdc beside it still parses. The spec types all eight as type: string, so either schema is defensible — the split is the odd part. Either note why these two are strict, or leave them on DecimalishSchema like their siblings.

[brunson-bot]

Round 3 (a438dbf) — all four round-2 nits are closed. Verify, Tests, Changeset and CodeQL green.

• pagination.ts:18-20 — hasMore now documents the third outcome: "following a full page past the endpoint maximum rejects instead, so capped results are not reported as complete history." Accurate for both capped methods, and it's the sentence a for await consumer needs.
• offsetContinuation deleted and the continuation inlined back into listTrades (activity.ts:152-161) and listActivity (:279-288). Both now match the hasMore + encodeOffsetCursor shape the other eleven offset paginators use, character for character. No dangling references left anywhere in packages/.
• session.test.ts:1507 — toBeInstanceOf restored alongside the toMatchObject. Awaiting the stored promise twice is fine; the first assertion attaches the handler, so there is no unhandled rejection.
• bindings/src/data/portfolio.ts:215 — the comment answers exactly the question I had about why these two fields are stricter than their six siblings.

One tradeoff worth naming, since inlining was my suggestion: deleting offsetContinuation also deleted the only test that tied the production continuation to the reject-on-continue contract. pagination.test.ts:15 now builds the over-ceiling cursor by hand, so if the inline continuation in listTrades were later clamped back to hasMore: false at the ceiling, nothing would fail — the integration tests only exercise a caller-supplied cursor through firstPage(). I don't think it's worth buying back: a genuine continuation test needs a fixture wallet with 20,000+ trades at pageSize: 10_000, and a fake client is off the table under the testing rules. Flagging it so the gap is a decision rather than an accident.

No further findings from me.

[cesarenaldi]

The ClickHouse fallback serializes these unavailable decimal fields as "". We already have OptionalDecimalStringSchema specifically for this wire behavior, normalizing "" to null while preserving populated decimal strings. Can we use it for gross_entry_cost_usdc and entry_fees_usdc instead of DecimalStringSchema.nullish(), and cover the empty fallback shape? Otherwise "" is branded as a valid DecimalString.

[brunson-bot]

Trial period: automated Brunson reviews on this repo are being evaluated. Suggestions are a work in progress and not necessarily accurate or vetted by the team yet — please verify before acting on them.

Confirmed — [bug]. Verified at the source in Polymarket/data-api:

• packages/api/pkg/repository/clickhouse/v1_combos.go:122 builds CombinatorialPositionPayload without GrossEntryCostUsdc / EntryFeesUsdc — the CH arm never scans them. Only pg_repository.go populates them.
• packages/api/pkg/payload/combos_payloads.go:16-17 declares both as plain string with no omitempty, so a CH-served page emits "gross_entry_cost_usdc": "" — present, non-null, empty.

Two consequences beyond what you flagged:

• .nullish() is dead code on this endpoint. Because those struct fields are non-pointer and lack omitempty, null/absent is unreachable; "" is the only missing encoding. DecimalStringSchema is z.string().transform(toDecimalString) with no non-empty check, so "" is branded DecimalString and flows straight into grossEntryCostUsdc − entryFeesUsdc.
• packages/client/tests/integration/portfolio.test.ts:110-111 asserts expect.any(String), which passes on "". Even a live CH-served page wouldn't fail the suite — so the "cover the empty fallback shape" ask is a real coverage gap, not just belt-and-braces.

OptionalDecimalStringSchema is the right primitive here.

Adjacent, pre-existing, not this PR: first_entry_at has the same hole — formatUTC (v1_combos.go:263) returns "" for a NULL first_entry_at, and IsoDateTimeStringSchema brands without validating, so "" becomes an IsoDateTimeString.

Slack discussion