Repository navigation
Conversation
| source: 'sdk', | ||
| sdk_id: `@adcp/sdk@${LIBRARY_VERSION}`, | ||
| field: path.replace(/^\$\./, ''), | ||
| details: { |
There was a problem hiding this comment.
Medium: field is computed against pre-projection product indices, but toCanonicalOnlyResponse can drop a product and compact the output array without throwing. A legacy-only product whose format_ids are unmappable projects to empty format_options → keep=false (augment-response.ts:656), dropped with a LEGACY_FORMAT_ID_DROPPED_UNMAPPED diagnostic, which is explicitly filtered out of blockingDiagnostics (L1004). So a response like [A: legacy-only unmappable, B: divergent dual] drops A, moving B to output products[0], while B's advisory still carries field: 'products[1]' — an index that no longer exists. details.product_id stays authoritative; the field pointer silently lies. Re-map the advisory field after projection, or anchor the advisory on product_id only.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — reject→normalize-and-report conversion of dual-emission format-declaration validation is sound and well-tested.
Checked: non-mutating behavior confirmed (deepEqual(response, before)), legacy-ref evidence survives canonical identity strip via reconciliation pulling from projected.response.errors, dedup by code+field, and enclosingProductId propagation to nested placements. Divergence throw removals are reported loudly via advisories — clears the witness-not-translator bar. Changeset (patch) is defensible for a recovery fix that keeps catalogs available for a previously-rejected case. Coverage is complete.
Medium findings
- src/lib/server/decisioning/runtime/from-platform.ts:1727 — Advisory
fielduses pre-projection product index; projection can compact the array. Non-blocking:details.product_idremains authoritative and the triggering drop (LEGACY_FORMAT_ID_DROPPED_UNMAPPED) is itself non-blocking.
No Critical/High findings. One Medium with no escalation trigger (high_risk false, gated_paths false, no team gate, not data-loss/schema/infra category) — falls through to approve per row 9.
Medium findings
- src/lib/server/decisioning/runtime/from-platform.ts:1727 — Advisory field uses pre-projection product index; projection can compact the array
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean fix, prior Medium resolved.
This subsequent push addresses the prior Medium finding (from-platform.ts:1727 — advisory used pre-projection product index). The reviewer confirms asProductResponseForWire now builds originalToOutputIndex and remapDivergenceAdvisory rewrites the FORMAT_DECLARATION_DIVERGENT advisory products[N] field to the compacted output index after empty-format_ids products are dropped, collapsing to the products collection when the divergent product is itself dropped. Index source matches normalization order (toCanonicalOnlyResponse preserves order/count). No mutation; non-divergence errors untouched. Two new tests cover both omission cases, satisfying the changeset-vs-wire and test requirements.
No current findings. No team gate, gated_paths false, high_risk false. Prior blocking/Medium issue is gone and no new findings surfaced — net improvement. Falls through to row 9.
Keep get_products successful when one product has divergent legacy and canonical format declarations. Treat format_options as authoritative, remove contradictory legacy routes and append FORMAT_DECLARATION_DIVERGENT with SDK provenance and product evidence. Preserve unrelated products and existing diagnostics, and deduplicate by code and field. Remap advisory fields after legacy product filtering and retain product identity when the affected product is omitted.
Validation: 283 packaged catalog, server and version-translation tests pass on Node 24 with the official runner, including strict 3.1 response validation and legacy 3.0 projection. Seven original regressions fail before the fix. Two additional regressions fail before the follow-up and verify advisory paths after 3.0 filtering. Build, typecheck and the patch changeset check pass. The full SDK suite was not completed locally: an earlier run hit a 60-second timeout in an unchanged type fixture, which passed in isolation after warming.
Hosted CI: Generated, Docs & Formatting fails because the live registry sync changes schemas/registry/registry.yaml and src/lib/registry/types.generated.ts. Both files are unchanged by this PR. The official sync generator reproduces the drift from pristine main inputs at ccfc06c.
Fixes #3685. Refs #3642.