Skip to content

fix(sdk): stop sending unsupported container/shipment filter keys + clamp page size + wire mapper includes - #278

Merged
dodeja merged 3 commits into
mainfrom
fix/sdk-filter-correctness
Jun 26, 2026
Merged

fix(sdk): stop sending unsupported container/shipment filter keys + clamp page size + wire mapper includes#278
dodeja merged 3 commits into
mainfrom
fix/sdk-filter-correctness

Conversation

@dodeja

@dodejadodeja commented Jun 24, 2026

Copy link
Copy Markdown
Member

Summary

The Terminal49 v2 /containers and /shipments list endpoints do not support filter[status], filter[pod_locode] (port), filter[line_scac] (carrier), or filter[updated_at] (updatedAfter) — verified against docs/openapi.json and the generated OpenAPI types. The SDK was silently forwarding these as filter[*] query keys, which the API drops, giving callers the false impression their filters applied.

This PR finishes that filter-correctness work:

  • Filter no-op fix: typed query builders (buildContainerListQuery / buildShipmentListQuery) map only the supported keys (include; plus number and filter[tracking_stopped] for shipments) and report dropped keys via an additive unsupportedFilters: string[] on the mapped list result. The as any casts on the list GET query objects are removed so an unknown filter key now fails typecheck.
  • Page-size clamp:page[size] is clamped to [1, MAX_PAGE_SIZE=100] via clampPageSize / applyTypedPagination.
  • Test reconciliation: the 4 request-builder tests that asserted the OLD buggy behavior (that the unsupported filter[*] keys ARE emitted) are updated to the post-fix contract — handler URLs no longer register the unsupported keys, the tests assert those keys are NOT present on the emitted request, and they assert result.unsupportedFilters reports the dropped source keys (status/port/carrier/updatedAfter). include + page[number]/page[size] coverage is preserved.
  • Container include wiring (request side for DEV-10662, mapper side in fix(sdk): correct JSON:API mapper relationship/attr paths + restore shipping-line capability flags #275):ContainerInclude listed a nonexistent destination_terminal relationship; replaced with pickup_facility, the real container relationship per docs/openapi.json (container relationships: shipment, pickup_facility, pod_terminal, transport_events, raw_events). Container default includes remain sensible (shipment, pod_terminal).
    • containers.route() include is left as-is: the /containers/{id}/route endpoint is not present in docs/openapi.json, so nested JSON:API include support (route_location.location) cannot be verified against the spec, and the route mapper reads each leg's port relationship directly off the route_location.

Closes DEV-10658 (page-size cap). Implements the filter-no-op critical (full filter grammar tracked in DEV-10668) and wires the request-side includes for DEV-10662 (#275).

Green gate

All commands run from the worktree root:

  • npm run build --workspace @terminal49/sdk — pass
  • npm run build --workspace @terminal49/mcp — pass
  • npm run type-check --workspace @terminal49/sdk — pass
  • npm run type-check --workspace @terminal49/mcp — pass
  • npm test --workspace @terminal49/sdk -- --run — 61 passed, 2 skipped
  • npm run test --workspace @terminal49/mcp -- --run — 77 passed

🤖 Generated with Claude Code

Greptile Summary

Stops the SDK from forwarding unsupported filter[*] query parameters (status, pod_locode, line_scac, updated_at) to the Terminal49 v2 list endpoints, caps page[size] at 100, wires pickup_facility as the correct container include, and reports dropped filters via unsupportedFilters on mapped results.

  • Filter correctness:buildContainerListQuery / buildShipmentListQuery now construct typed query objects from the generated OpenAPI spec, replacing the previous Record<string, string> approach with as any casts; unsupported filter keys are collected and returned as unsupportedFilters rather than being silently forwarded.
  • Page-size clamping:clampPageSize floors at 1 and caps at MAX_PAGE_SIZE = 100, applied via the new applyTypedPagination helper; the old applyPagination (string-only) is retained for non-typed call sites.
  • Include fix:ContainerInclude removes the nonexistent destination_terminal and adds pickup_facility, matching the actual container relationships in the OpenAPI spec.

Confidence Score: 4/5

The filter-correctness and page-size clamping changes are safe to ship; the core bug (sending no-op filter params to the API) is fixed for all callers regardless of format.

The unsupportedFilters diagnostic is computed in all code paths but only attached to the return value when format: 'mapped' is requested. With the SDK default format: 'raw', formatResult returns before calling the mapper, so unsupportedFilters is silently discarded — callers in the default format receive an unfiltered list with no indication their filters were dropped.

containers.ts and shipments.ts — the unsupportedFilters plumbing through formatResult is worth a second look for the raw-format case.

Important Files Changed

FilenameOverview
sdks/typescript-sdk/src/client/query.tsIntroduces typed query builders, clampPageSize, and applyTypedPagination; correctly isolates the four unsupported filter keys and reports them via unsupportedFilters.
sdks/typescript-sdk/src/client/managers/containers.tsSwitches to typed query builder; removes as any cast on the GET query; unsupportedFilters is only surfaced in format: 'mapped' mode.
sdks/typescript-sdk/src/client/managers/shipments.tsSame typed-query-builder migration as containers; adds trackingStopped and number filter support; same unsupportedFilters surfacing limitation in raw format.
sdks/typescript-sdk/src/types/options.tsReplaces the nonexistent destination_terminal include with pickup_facility, matching the actual container relationships in the OpenAPI spec.
sdks/typescript-sdk/src/client.filters.test.tsNew test file covering filter-correctness for both managers, page-size clamping, and unsupportedFilters reporting in mapped mode; assertions use .sort() for order-safety.
sdks/typescript-sdk/src/client.request.test.tsUpdated four tests to assert the post-fix contract; one assertion uses order-sensitive toEqual without .sort() on unsupportedFilters.
.github/workflows/ci.ymlAdds npm run type-check steps to both CI jobs to catch type regressions at the SDK and MCP workspace levels.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["caller: list(filters, options)"] --> B["buildContainerListQuery / buildShipmentListQuery"]
B --> C{{"filter key supported?"}}
C -- "include / number / tracking_stopped" --> D["add to typed query object"]
C -- "status / port / carrier / updatedAfter" --> E["record in unsupportedFilters[]"]
D --> F["applyTypedPagination(query, options)\nclamp page[size] to [1, 100]"]
F --> G["transport.client.GET('/containers' or '/shipments', { query })"]
G --> H["formatResult(raw, format, mapper)"]
H --> I{{"format?"}}
I -- "'raw' (default)" --> J["return raw API response\nunsupportedFilters LOST"]
I -- "'mapped'" --> K["mapper(raw) → { items, links, meta, unsupportedFilters }"]
I -- "'both'" --> L["{ raw, mapped: mapper(raw) }\nunsupportedFilters in mapped"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["caller: list(filters, options)"] --> B["buildContainerListQuery / buildShipmentListQuery"]
B --> C{{"filter key supported?"}}
C -- "include / number / tracking_stopped" --> D["add to typed query object"]
C -- "status / port / carrier / updatedAfter" --> E["record in unsupportedFilters[]"]
D --> F["applyTypedPagination(query, options)\nclamp page[size] to [1, 100]"]
F --> G["transport.client.GET('/containers' or '/shipments', { query })"]
G --> H["formatResult(raw, format, mapper)"]
H --> I{{"format?"}}
I -- "'raw' (default)" --> J["return raw API response\nunsupportedFilters LOST"]
I -- "'mapped'" --> K["mapper(raw) → { items, links, meta, unsupportedFilters }"]
I -- "'both'" --> L["{ raw, mapped: mapper(raw) }\nunsupportedFilters in mapped"]
Loading

Fix All in Codex

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 3
sdks/typescript-sdk/src/client/managers/containers.ts:60-63
**`unsupportedFilters` silently unavailable in the default (`raw`) format**`unsupportedFilters` is attached to the return value only inside the mapper callback passed to `formatResult`. When the caller uses the default format (`'raw'`), `formatResult` returns the raw API response immediately and never calls the mapper — so `unsupportedFilters` is computed but then discarded. A caller who passes `{ status: 'in_transit' }` without specifying `format: 'mapped'` will get an unfiltered list back with no indication that the filter was dropped. The same pattern applies in `shipments.ts`. Consider either always returning a wrapper object that includes `unsupportedFilters` alongside the raw response, or logging a warning when dropped filters are detected regardless of format.
### Issue 2 of 3
sdks/typescript-sdk/src/client.request.test.ts:121-126
The `unsupportedFilters` assertion uses `toEqual` without `.sort()`, coupling the test to the iteration order of the internal `UNSUPPORTED_FILTER_KEYS` constant. If the order of that array ever changes, this test fails for a non-functional reason. The sibling test in `client.filters.test.ts` correctly sorts both sides before comparing.
```suggestion expect(result.unsupportedFilters?.slice().sort()).toEqual( ['status', 'port', 'carrier', 'updatedAfter'].sort(), );```### Issue 3 of 3
sdks/typescript-sdk/src/client/query.ts:37-47
**`includeContainers` in `ShipmentListFilters` is a no-op when calling the builder directly**`ShipmentListFilters` (an exported interface) includes `includeContainers?: boolean`, but `buildShipmentListQuery` never reads it. The field only has effect when the `ShipmentManager` computes `defaultInclude` before calling the builder. A caller who invokes the exported `buildShipmentListQuery({ includeContainers: false })` directly will find the flag is a no-op — the builder will fall back to the empty-array default, not `SHIPMENT_INCLUDES_WITHOUT_CONTAINERS`. Consider either removing `includeContainers` from `ShipmentListFilters` or handling it inside the builder.

Reviews (1): Last reviewed commit: "docs(sdk): regenerate reference for filt..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

dodejaand others added 2 commits June 24, 2026 10:52
…rop `as any` + clamp page size
The `/containers` and `/shipments` v2 list endpoints do NOT support
`filter[status]`, `filter[pod_locode]` (port), `filter[line_scac]`
(carrier), or `filter[updated_at]` (updatedAfter) — verified against
docs/openapi.json and the generated OpenAPI types. The SDK was silently
forwarding these as `filter[*]` query keys, which the API drops, giving
callers the false impression their filters applied.
Changes (filter correctness):
- Extract pure, typed query builders `buildContainerListQuery` /
`buildShipmentListQuery` in query.ts. They map ONLY supported keys
(`include`; plus `number` and `filter[tracking_stopped]` for shipments)
into query objects typed against the generated openapi-fetch params, and
report dropped keys via an additive `unsupportedFilters: string[]` field
on the mapped list result so the MCP layer can echo honesty.
- Drop the `as any` casts on the `/containers` and `/shipments` GET query
objects so an unknown filter key now fails typecheck.
- Add `clampPageSize` + `applyTypedPagination`: page[size] is clamped to
[1, MAX_PAGE_SIZE=100]; applyPagination also clamps.
- Keep the public `containers.list` / `shipments.list` signatures (MCP
callers still compile); `shipments.list` additively gains optional
`trackingStopped` and `number`.
- CI: add type-check steps for both @terminal49/sdk and @terminal49/mcp so a
reintroduced bogus filter key fails the build.
- New unit/integration tests in client.filters.test.ts (TDD-first).
Closes DEV-10658 (page-size clamp portion). Supports the filter-correctness
critical; full filter grammar tracked in DEV-10668.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Finish the filter-correctness work that was blocked on 4 request-builder
tests asserting the OLD buggy behavior (that filter[status]/
filter[pod_locode]/filter[line_scac]/filter[updated_at] ARE emitted on
/shipments and /containers). The SDK now correctly OMITS those unsupported
keys, so the tests are updated to the post-fix contract:
- drop the unsupported filter[*] keys from the mock-fetch handler URLs and
assert they are NOT present on the emitted request, and
- assert result.unsupportedFilters reports the dropped source keys
(status/port/carrier/updatedAfter), keeping include + page[number]/
page[size] coverage intact.
Also wire container relationship includes (DEV-10662, mapper-side in #275):
- ContainerInclude listed a nonexistent `destination_terminal` relationship;
replace it with `pickup_facility`, which is the real container relationship
per docs/openapi.json (container relationships: shipment, pickup_facility,
pod_terminal, transport_events, raw_events). Container default includes
remain sensible (shipment, pod_terminal).
containers.route() include is left as-is: the /containers/{id}/route endpoint
is not present in docs/openapi.json, so nested JSON:API include support
(route_location.location) cannot be verified against the spec, and the route
mapper reads each leg's `port` relationship directly off the route_location.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@linear-code

Copy link
Copy Markdown
Contributor

DEV-10658

@vercel

vercelBot commented Jun 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
apiReadyReadyPreview, CommentJun 24, 2026 6:21pm

Request Review

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mintlify

mintlifyBot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

ProjectStatusPreviewUpdated (UTC)
terminal49🟢 ReadyView PreviewJun 24, 2026, 6:27 PM

@dodeja
dodeja marked this pull request as ready for review June 26, 2026 00:23
@dodeja
dodeja merged commit a019b5d into mainJun 26, 2026
12 checks passed

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:1df56a85c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadsdks/typescript-sdk/src/types/options.ts
Comment on lines +60 to +63
return this.formatResult(raw, options?.format, (doc) => ({
...this.mapListResult(doc, mapContainerList),
unsupportedFilters,
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2unsupportedFilters silently unavailable in the default (raw) format

unsupportedFilters is attached to the return value only inside the mapper callback passed to formatResult. When the caller uses the default format ('raw'), formatResult returns the raw API response immediately and never calls the mapper — so unsupportedFilters is computed but then discarded. A caller who passes { status: 'in_transit' } without specifying format: 'mapped' will get an unfiltered list back with no indication that the filter was dropped. The same pattern applies in shipments.ts. Consider either always returning a wrapper object that includes unsupportedFilters alongside the raw response, or logging a warning when dropped filters are detected regardless of format.

Prompt To Fix With AI
This is a comment left during a code review.
Path: sdks/typescript-sdk/src/client/managers/containers.ts
Line: 60-63
Comment:
**`unsupportedFilters` silently unavailable in the default (`raw`) format**`unsupportedFilters` is attached to the return value only inside the mapper callback passed to `formatResult`. When the caller uses the default format (`'raw'`), `formatResult` returns the raw API response immediately and never calls the mapper — so `unsupportedFilters` is computed but then discarded. A caller who passes `{ status: 'in_transit' }` without specifying `format: 'mapped'` will get an unfiltered list back with no indication that the filter was dropped. The same pattern applies in `shipments.ts`. Consider either always returning a wrapper object that includes `unsupportedFilters` alongside the raw response, or logging a warning when dropped filters are detected regardless of format.
How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mechanics here are accurate: in BaseManager.formatResult, raw short-circuits and returns the API response untouched, so the mapper callback that attaches unsupportedFilters only runs for format: 'mapped' and format: 'both'. In raw mode the computed unsupportedFilters is discarded. This is the same shape in shipments.ts.

The intent is that raw returns the API payload verbatim (no SDK-added envelope keys), and unsupportedFilters rides on the mapped wrapper — a deliberate layering choice. However, I want to flag honestly: I could not find JSDoc on the public list methods that actually documents "unsupportedFilters is only surfaced in mapped/both format," so a raw-mode caller passing a dropped filter (e.g. { status: 'in_transit' }) currently gets no signal at all. That's a legitimate DX gap.

Leaving this thread unresolved for a maintainer to decide between (a) documenting the mapped-only behavior explicitly, or (b) logging a warning when filters are dropped regardless of format. Not resolving and not pushing, since this is a merged branch.

Comment threadsdks/typescript-sdk/src/client.request.test.ts
Comment threadsdks/typescript-sdk/src/client/query.ts
dodeja added a commit that referenced this pull request Jun 26, 2026
Match the sibling assertion in client.filters.test.ts so the test no longer
fails if UNSUPPORTED_FILTER_KEYS iteration order changes. Addresses a P2
review comment on PR #278.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dodeja

Copy link
Copy Markdown
MemberAuthor

Triaged the automated review comments (codex + greptile). Addressed 1, intentionally skipped 3.

Addressed

  • client.request.test.ts: the unsupportedFilters assertion now sorts both sides (result.unsupportedFilters?.sort() vs a sorted expected array), so it no longer couples to the iteration order of UNSUPPORTED_FILTER_KEYS. This matches the sibling assertion already in client.filters.test.ts. (commit 8c528e4)

Intentionally skipped (with reasons)

  • Map pickup_facility before exposing it as an include (options.ts): the include is real and works for raw/both formats; adding a pickupFacility field to the mapped Container model + mapper is a product/API-surface decision (model shape) and out of scope for this filter-correctness PR.
  • unsupportedFilters unavailable in raw format (containers.ts): by design. raw returns the verbatim JSON:API document; attaching SDK-only metadata to it would pollute the raw contract. The flag is surfaced on the mapped list wrapper, which is where SDK metadata lives.
  • includeContainers is a no-op when calling buildShipmentListQuery directly (query.ts): intentional. The builder is a low-level primitive; includeContainers is resolved by ShipmentManager, which picks the right default-include constant. Moving that into the builder would duplicate the include constants and change the builder contract — an ergonomics decision, not a correctness bug.

Green gate passes on the updated branch: SDK build/type-check/tests (61 pass, 2 skip), MCP build/type-check/tests (77 pass).

dodeja added a commit that referenced this pull request Jun 26, 2026
Address PR #278 review: greptile flagged that `unsupportedFilters` is
computed in the list mappers but discarded under the default `raw`
format (formatResult short-circuits and never runs the mapper). Rather
than mutate the verbatim raw response (a contract change), document the
intended behavior on ContainerManager.list and ShipmentManager.list:
`unsupportedFilters` is surfaced only with format `'mapped'`/`'both'`.
Regenerated docs/sdk/reference to keep typedoc output in sync (CI check).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dodeja

Copy link
Copy Markdown
MemberAuthor

Triaged the automated review comments and pushed one fix to fix/sdk-filter-correctness (05e7930).

Addressed

  • unsupportedFilters silently unavailable in the default raw format (greptile, containers.ts): real gap — formatResult short-circuits in raw mode and never runs the mapper that attaches unsupportedFilters. Rather than mutate the verbatim raw response (a contract change), documented the intended behavior on ContainerManager.list / ShipmentManager.list: unsupportedFilters is surfaced only with format: 'mapped' | 'both'. Regenerated docs/sdk/reference to keep typedoc in sync.

Skipped (with reasons)

  • Sort the unsupportedFilters assertion in client.request.test.ts (greptile): already addressed by commit 8c528e4 on this branch (both sides now .sort()).
  • includeContainers is a no-op when calling buildShipmentListQuery directly (greptile): buildShipmentListQuery / ShipmentListFilters live in src/client/query.ts, which is not exported from the public index.ts. The only internal caller (ShipmentManager.list) correctly computes defaultInclude from includeContainers before calling the builder, so there is no real no-op path for SDK consumers. Internal-only nitpick; reconciling it would either duplicate the include-list logic into the builder or diverge the interface from the manager signature — out of scope.
  • Map pickup_facility before exposing it via ContainerInclude (codex): valid observation that mapContainer doesn't surface the pickup_facility relationship, but adding it changes the mapped Container shape (new public field + docs surface) and is a product/API-shape decision. Left for a dedicated change; pickup_facility remains usable today via the raw/both formats.

Green gate from the worktree: SDK build/type-check + tests (61 pass / 2 skip), MCP build/type-check + tests (77 pass). oxfmt run only on the two changed source files.

dodeja added a commit that referenced this pull request Jun 26, 2026
Resolve conflicts in the SDK container layer where #278 (already merged to
main) and this PR (#275) both touched the same files.
- types/options.ts ContainerInclude: both sides replaced `destination_terminal`
with `pickup_facility`; kept a single `pickup_facility` member plus #275's
explanatory comment.
- managers/containers.ts list(): kept main's #278 logic
(buildContainerListQuery + unsupported-filter omission, `unsupportedFilters`
on the mapped result, typed query, applyTypedPagination page-size clamp) AND
preserved #275's intent by adding `pickup_facility` to
DEFAULT_CONTAINER_INCLUDES (the default include passed to the list query).
get() already includes `pickup_facility` by default.
Regenerated docs/sdk/reference; green gate (build + type-check + tests for
@terminal49/sdk and @terminal49/mcp) passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actionsgithub-actionsBot mentioned this pull request Aug 22, 2026
Sign up for freeto 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.

1 participant

@dodeja