Uh oh!
There was an error while loading. Please reload this page.
fix(sdk): stop sending unsupported container/shipment filter keys + clamp page size + wire mapper includes - #278
Conversation
…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>
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Preview deployment for your docs. Learn more about Mintlify Previews.
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 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".
Uh oh!
There was an error while loading. Please reload this page.
| return this.formatResult(raw, options?.format, (doc) => ({ | ||
| ...this.mapListResult(doc, mapContainerList), | ||
| unsupportedFilters, | ||
| })); |
There was a problem hiding this 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.
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.There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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
commented
Jun 26, 2026
Triaged the automated review comments (codex + greptile). Addressed 1, intentionally skipped 3. Addressed
Intentionally skipped (with reasons)
Green gate passes on the updated branch: SDK build/type-check/tests (61 pass, 2 skip), MCP build/type-check/tests (77 pass). |
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
commented
Jun 26, 2026
Triaged the automated review comments and pushed one fix to Addressed
Skipped (with reasons)
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. |
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>
Summary
The Terminal49 v2
/containersand/shipmentslist endpoints do not supportfilter[status],filter[pod_locode](port),filter[line_scac](carrier), orfilter[updated_at](updatedAfter) — verified againstdocs/openapi.jsonand the generated OpenAPI types. The SDK was silently forwarding these asfilter[*]query keys, which the API drops, giving callers the false impression their filters applied.This PR finishes that filter-correctness work:
buildContainerListQuery/buildShipmentListQuery) map only the supported keys (include; plusnumberandfilter[tracking_stopped]for shipments) and report dropped keys via an additiveunsupportedFilters: string[]on the mapped list result. Theas anycasts on the list GET query objects are removed so an unknown filter key now fails typecheck.page[size]is clamped to[1, MAX_PAGE_SIZE=100]viaclampPageSize/applyTypedPagination.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 assertresult.unsupportedFiltersreports the dropped source keys (status/port/carrier/updatedAfter).include+page[number]/page[size]coverage is preserved.ContainerIncludelisted a nonexistentdestination_terminalrelationship; replaced withpickup_facility, the real container relationship perdocs/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}/routeendpoint is not present indocs/openapi.json, so nested JSON:API include support (route_location.location) cannot be verified against the spec, and the route mapper reads each leg'sportrelationship 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— passnpm run build --workspace @terminal49/mcp— passnpm run type-check --workspace @terminal49/sdk— passnpm run type-check --workspace @terminal49/mcp— passnpm test --workspace @terminal49/sdk -- --run— 61 passed, 2 skippednpm 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, capspage[size]at 100, wirespickup_facilityas the correct container include, and reports dropped filters viaunsupportedFilterson mapped results.buildContainerListQuery/buildShipmentListQuerynow construct typed query objects from the generated OpenAPI spec, replacing the previousRecord<string, string>approach withas anycasts; unsupported filter keys are collected and returned asunsupportedFiltersrather than being silently forwarded.clampPageSizefloors at 1 and caps atMAX_PAGE_SIZE = 100, applied via the newapplyTypedPaginationhelper; the oldapplyPagination(string-only) is retained for non-typed call sites.ContainerIncluderemoves the nonexistentdestination_terminaland addspickup_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
unsupportedFiltersdiagnostic is computed in all code paths but only attached to the return value whenformat: 'mapped'is requested. With the SDK defaultformat: 'raw',formatResultreturns before calling the mapper, sounsupportedFiltersis silently discarded — callers in the default format receive an unfiltered list with no indication their filters were dropped.containers.tsandshipments.ts— theunsupportedFiltersplumbing throughformatResultis worth a second look for the raw-format case.Important Files Changed
clampPageSize, andapplyTypedPagination; correctly isolates the four unsupported filter keys and reports them viaunsupportedFilters.as anycast on the GET query;unsupportedFiltersis only surfaced informat: 'mapped'mode.trackingStoppedandnumberfilter support; sameunsupportedFilterssurfacing limitation in raw format.destination_terminalinclude withpickup_facility, matching the actual container relationships in the OpenAPI spec.unsupportedFiltersreporting in mapped mode; assertions use.sort()for order-safety.toEqualwithout.sort()onunsupportedFilters.npm run type-checksteps 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"]%%{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"]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "docs(sdk): regenerate reference for filt..." | Re-trigger Greptile