Uh oh!
There was an error while loading. Please reload this page.
fix(sdk): correct JSON:API mapper relationship/attr paths + restore shipping-line capability flags - #275
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…lamp page size + wire mapper includes (#278) * fix(sdk): stop sending unsupported container/shipment filter keys + drop `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> * fix(sdk): reconcile stale filter tests + wire container includes 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> * docs(sdk): regenerate reference for filter-correctness API changes Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e94b0b86d6
ℹ️ 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.
… + type restored fields Address Codex review on PR #275. - ContainerInclude: replace the non-existent `destination_terminal` relationship with the real `pickup_facility` relationship, which the container mapper now reads to resolve `destinationTerminal`. Containers have no `destination_terminal` relationship (that one lives on shipments). - Request `pickup_facility` by default in ContainerManager.get/list and Terminal49Client.getContainer so mapped `destinationTerminal` is populated out of the box. Update affected mock-fetch handler URLs/assertions. - Type the restored ShippingLine capability fields (alternativeScacs, billOfLadingTrackingSupport, bookingNumberTrackingSupport, containerNumberTrackingSupport) and Container.currentStatus explicitly so they are no longer reachable only via the index signature; drop the `as ShippingLine` cast in mapShippingLines in favour of a typed predicate. - Regenerate SDK reference docs for the changed public types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dodeja
commented
Jun 26, 2026
Triaged the Codex review + known residual type-hygiene findings. Pushed Addressed
Skipped
Green gate: SDK build + type-check + 57 pass/1 skip; MCP build + type-check + 77 pass. oxfmt --check clean on the changed files. |
Preview deployment for your docs. Learn more about Mintlify Previews.
|
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>
…hipping-line capability flags The mapped SDK responses dropped or nulled several real Terminal49 fields because the mappers read relationship/attribute paths that do not exist in the JSON:API payloads: - mapRoute read a non-existent `port` relationship on each route_location, so every leg's `port` was always null. Route legs expose their port via the `location` relationship (type port|terminal) per the OpenAPI schema. - Port resources expose `code` (e.g. KRPUS), not `locode`, so the `locode` output fields were always undefined. mapShipment, mapTransportEvents, and the container inline transport-event location now read `code` (falling back to `locode` defensively). - mapContainer read a non-existent `attrs.status` (real attribute is `current_status`) and a non-existent `destination_terminal` relationship, while the real `pickup_facility` relationship was never read. It now maps `current_status` to `status`/`currentStatus` and surfaces `pickup_facility` as the destination terminal. - The raw `attrCamel` spread clobbered curated nested fields and emitted ~19 duplicate top-level scalars per container row. A new `omitKeys` helper drops the curated-source attribute keys from the spread so curated nests win and no duplicate scalars leak. - The shipping-line mapper dropped `alternative_scacs` and the three `*_tracking_support` booleans; they are restored. Adds golden-fixture tests (seeded from real API response shapes and the OpenAPI route_location schema) covering each path. Closes DEV-10662 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + type restored fields Address Codex review on PR #275. - ContainerInclude: replace the non-existent `destination_terminal` relationship with the real `pickup_facility` relationship, which the container mapper now reads to resolve `destinationTerminal`. Containers have no `destination_terminal` relationship (that one lives on shipments). - Request `pickup_facility` by default in ContainerManager.get/list and Terminal49Client.getContainer so mapped `destinationTerminal` is populated out of the box. Update affected mock-fetch handler URLs/assertions. - Type the restored ShippingLine capability fields (alternativeScacs, billOfLadingTrackingSupport, bookingNumberTrackingSupport, containerNumberTrackingSupport) and Container.currentStatus explicitly so they are no longer reachable only via the index signature; drop the `as ShippingLine` cast in mapShippingLines in favour of a typed predicate. - Regenerate SDK reference docs for the changed public types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
43c0650 to
e31876dComparedodeja
commented
Jul 1, 2026
Rebased onto current Rebase conflicts resolved in Review feedback: The one substantive automated finding (Codex, P2, Checks: |
Summary
The mapped SDK responses silently dropped or nulled several real Terminal49 fields because the JSON:API mappers read relationship/attribute paths that don't exist in the payloads. This corrects every path against the real API response shapes (verified from existing fixtures and the generated OpenAPI schema):
mapRouteroute-leg port — read a non-existentportrelationship on eachroute_location, so every leg'sportwas alwaysnull. Route legs expose their port via thelocationrelationship (typeport|terminal) per the OpenAPI schema. Now readslocation.codevslocode— Port resources exposecode(e.g.KRPUS), neverlocode, so the mappedlocodefields were alwaysundefined.mapShipment,mapTransportEvents, and the container inline transport-event location now readcode(with a defensive?? locodefallback).mapContainerstatus + facility — read a non-existentattrs.status(real attribute iscurrent_status) and a non-existentdestination_terminalrelationship, while the realpickup_facilityrelationship was never read. Now mapscurrent_status→status/currentStatusand surfacespickup_facilityas the destination terminal.attrCamelclobber — the raw camelCased attribute spread clobbered curated nested fields and emitted ~19 duplicate top-level scalars per container row. A new pureomitKeyshelper (injsonapi.ts) drops the curated-source keys from the spread so curated nests win and no duplicate scalars leak.alternative_scacsand the three*_tracking_supportbooleans; they're restored asalternativeScacs,billOfLadingTrackingSupport,bookingNumberTrackingSupport,containerNumberTrackingSupport.TDD
Golden-fixture tests were written first (red), then the mappers were fixed until green. New fixtures
containers.route.jsonandcontainers.get.pickup.jsonare seeded from the real JSON:API shapes / OpenAPIroute_locationschema. The previously-skipped route test now runs (and its port lookup was corrected to use the reallocationrelationship).Issues
Closes DEV-10662
Green gate
SDK gained +6 passing mapping tests (baseline was 51 pass / 2 skip → now 57 pass / 1 skip). MCP unchanged (77 pass).
oxlint+oxfmt --checkclean.Notes / follow-ups
ContainerIncludeinsrc/types/options.tsstill advertises the wrongdestination_terminalinclude value and omitspickup_facility. That type is owned by another PR, so this PR passes the container include as a string in tests and notes it here. A follow-up should update that enum topickup_facility.This is an AI-drafted PR for human review.
🤖 Generated with Claude Code
Greptile Summary
This PR corrects a batch of wrong JSON:API attribute/relationship paths across the SDK mappers — fixing silently-null port codes,
current_statusvsstatus,pickup_facilityvsdestination_terminal, and a raw-attribute spread that clobbered curated nested objects — and restores four missing shipping-line capability fields.mapContainer:current_status→status/currentStatus,pickup_facilityrelationship replaces the non-existentdestination_terminal, andomitKeysprevents the camelCase attribute spread from clobbering curated nested objects (equipment,location,demurrage,terminals,rail).mapRoute: Route-leg port now read via thelocationrelationship (typeport|terminal), which is the actual JSON:API relationship name; the oldportrelationship never existed onroute_location.mapShipment/mapTransportEvents: Portlocodefields now readattributes.code(with a?? locodedefensive fallback) since Terminal49 port resources exposecode, notlocode.mapShippingLines:alternativeScacs,billOfLadingTrackingSupport,bookingNumberTrackingSupport, andcontainerNumberTrackingSupportrestored. However, these fields are not yet declared on theShippingLineTypeScript interface, so they are inaccessible to typed consumers.Confidence Score: 3/5
The mapper fixes are mostly correct, but two gaps prevent a clean merge:
mapTrackingRequeststill readscontainer.attributes?.status(will always beundefined), and the four new shipping-line fields are not declared in theShippingLineinterface, making them invisible to typed consumers.Most of the mapper corrections are accurate and well-tested. However,
mapTrackingRequestrepeats the exact same wrong attribute name (statusinstead ofcurrent_status) that this PR set out to fix inmapContainer, so tracking-request responses will continue to silently drop the container status. Separately, the restored shipping-line capability fields are added to the runtime object but omitted from theShippingLineTypeScript interface, meaning SDK consumers using typed access cannot reach them without an extra cast.sdks/typescript-sdk/src/client/mappers.ts— themapTrackingRequestfunction and theShippingLineinterface insrc/types/models.tsboth need attention.Important Files Changed
mapContainercorrectly fixed;mapTrackingRequeststill readscontainer.attributes?.status(should becurrent_status). NewShippingLinefields are missing from the interface type. Routelocationtype (port vs terminal) is unchecked.omitKeyshelper is correct and well-guarded. No issues.mapTrackingRequestcontainer status regression.current_status,pickup_facility, and excludes a non-existentstatusattribute.locationrelationship (notport) on eachroute_location, matching the fixed mapper logic.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph mapContainer A[container resource] -->|pickup_facility rel ✅| B[destinationTerminal] A -->|current_status attr ✅| C[status / currentStatus] A -->|omitKeys spread ✅| D[equipment / location / demurrage / rail nested objects] end subgraph mapRoute E[route_location resource] -->|location rel ✅| F[port or terminal] F -->|.code ✅| G[leg port.code] end subgraph mapShipment_mapTransportEvents H[port resource] -->|.code ?? .locode ✅| I[locode field] end subgraph mapShippingLines J[shipping_line resource] -->|alternative_scacs ✅| K[alternativeScacs] J -->|tracking_support ✅| L[3 capability flags] L -.->|not in ShippingLine type ❌| M[TypeScript interface gap] end subgraph mapTrackingRequest N[container relationship] -->|.status ❌ should be current_status| O[nested container.status always undefined] end%%{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 subgraph mapContainer A[container resource] -->|pickup_facility rel ✅| B[destinationTerminal] A -->|current_status attr ✅| C[status / currentStatus] A -->|omitKeys spread ✅| D[equipment / location / demurrage / rail nested objects] end subgraph mapRoute E[route_location resource] -->|location rel ✅| F[port or terminal] F -->|.code ✅| G[leg port.code] end subgraph mapShipment_mapTransportEvents H[port resource] -->|.code ?? .locode ✅| I[locode field] end subgraph mapShippingLines J[shipping_line resource] -->|alternative_scacs ✅| K[alternativeScacs] J -->|tracking_support ✅| L[3 capability flags] L -.->|not in ShippingLine type ❌| M[TypeScript interface gap] end subgraph mapTrackingRequest N[container relationship] -->|.status ❌ should be current_status| O[nested container.status always undefined] endComments Outside Diff (3)
sdks/typescript-sdk/src/client/mappers.ts, line 430 (link)statusinmapTrackingRequestcontainer.attributes?.statusis the same wrong attribute name thatmapContainerjust fixed — the real field on a container resource iscurrent_status. When aTrackingRequestincludes a related container, the nestedstatusfield will always beundefined, silently dropping the container's current state from every tracking-request response.Prompt To Fix With AI
sdks/typescript-sdk/src/client/mappers.ts, line 151-165 (link)ShippingLinefields missing from the interface typealternativeScacs,billOfLadingTrackingSupport,bookingNumberTrackingSupport, andcontainerNumberTrackingSupportare written into the returned object but are not declared in theShippingLineinterface insrc/types/models.ts. Theas ShippingLinecast silences the TypeScript error, so SDK consumers who type their variable asShippingLinecannot access these fields without another cast. The runtime data is present but the public contract doesn't expose it.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
sdks/typescript-sdk/src/client/mappers.ts, line 91-106 (link)locationwill produce an undefinedcodein the port objectThe
locationrelationship on aroute_locationcan resolve to either aportor aterminal(as noted in the OpenAPI schema and the PR description). When the resolved resource is a terminal,port.attributes?.codewill beundefinedbecause terminals usefirms_code, notcode. The leg's port object would still be truthy (non-null), so callers cannot distinguish a proper port from a structurally empty one. A type-check on the resolved resource (resource.type === 'port') before constructing the object would prevent this.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(sdk): correct JSON:API mapper relati..." | Re-trigger Greptile