Skip to content

Derive default-catalog product ids deterministically (issue #34) - #37

Merged
atc964 merged 2 commits into
IABTechLab:mainfrom
aleksUIX:fix/deterministic-default-catalog-ids
Jul 29, 2026
Merged

Derive default-catalog product ids deterministically (issue #34)#37
atc964 merged 2 commits into
IABTechLab:mainfrom
aleksUIX:fix/deterministic-default-catalog-ids

Conversation

@aleksUIX

Copy link
Copy Markdown
Contributor

Problem

The last live piece of #34 observation 1. In default (non-CSV) mode, build_static_product_catalog mints prod-{uuid4().hex[:8]} ids at build time, and the catalog cache is per process. Any deployment with more than one process reproduces the reported symptom: GET /products served by worker A returns ids that GET /products/{id} on worker B has never seen, so the lookup 404s. The repo's own image runs uvicorn --workers 2 (infra/docker/Dockerfile:82), and a restart between list and get does the same thing. The CSV-mode side of this was already fixed in v2.2.2 by reading ids from the CSV.

Change

Derive each id with uuid5(NAMESPACE_URL, "ad-seller-product:{name}") instead of uuid4. Ids keep the exact prod-[0-9a-f]{8} shape pinned by test_default_mode_ids_stay_uuid_shaped_and_stable, stay unique per product, and are now identical in every process and across restarts.

Adds test_default_mode_ids_survive_cache_reset: rebuilds the catalog after reset_catalog_cache() (the single-process proxy for a second worker) and asserts the id set is unchanged, plus uniqueness across the 13 configs.

Verification

Ran the catalog build in two separate Python processes: identical id sets, all shape-conformant, 13 unique ids. The new test passes; note the full suite needs the iab-agentic-primitives dependency, which public clones cannot currently install.

…b#34)

build_static_product_catalog minted prod-{uuid4} ids per process, so any
multi-worker deployment (the shipped Dockerfile runs uvicorn --workers 2)
or a restart between GET /products and GET /products/{id} could 404 on an
id the server itself had just returned. This is the surviving kernel of
issue IABTechLab#34 observation 1; the CSV-mode side was fixed in v2.2.2.

Derive ids with uuid5 over the config name instead: same prod-[0-9a-f]{8}
shape, unique per product, identical in every process. Adds a regression
test that rebuilds the catalog after a cache reset (the single-process
proxy for a second worker) and asserts the id set is unchanged.
@jaanijuk

jaanijuk commented Jul 28, 2026

Copy link
Copy Markdown

Commenting as the reporter of #34 and good to see the last piece of observation 1 picked up, and thank you for the very clear write-up.

Scope note from the original repro. Our #34 instance was observed on v2.0 (0f544a5), CSV adapter (AD_SERVER_TYPE=csv, the aws_workshop sample set), a single uvicorn process (no --workers), with GET /productsGET /products/{id} run back-to-back in one session, no restart in between. So I can confirm the symptom as reported, but our data can't speak to the per-process / multi-worker mechanism addressed here; the instance we hit sat on the path fixed in v2.2.1. The mechanism as described (build-time uuid4 minting, per-process cache, the image's --workers 2) reads as coherent, though I haven't been able to run this branch to check it, see the CI note below.

Three non-blocking suggestions on the change:

  1. Uniqueness is now name-contingent. Two configs sharing a name collide deterministically (identical uuid5 → identical truncated id), where under uuid4 a duplicate name was harmless. The new test pins uniqueness for the current 13 configs, but a future duplicate name would collide silently. A build-time guard that raises on duplicate names would close that off.

  2. Upgrade path. Every default-catalogue id changes under this fix, so packages/placements persisted before the upgrade that reference pre-fix ids will dangle afterwards. If the 422 introduced in v2.2.x guards creation-time references, rows persisted before the id change would still sit outside it. A startup warning when stored placements reference unknown product ids would cover that gap, plus a release-note line. Related docs note: renaming a product now changes its id.

  3. Test design. The cache-reset proxy is a fair CI stand-in, and your manual two-process check is the actual failure mode, and needs no credentials. A subprocess-based integration test would let CI cover it directly; happy for that to be a follow-up rather than blocking here.

CI. The 3.12 job's failure annotation sits on the Install dependencies step (exit code 1, ~10 seconds in), with the 3.11 job cancelled by fail-fast which is consistent with iab-agentic-primitives failing to install on public runners, per your own verification note. Until that package is publicly installable, external reviewers can't run the suite either.

Offer. The day iab-agentic-primitives becomes publicly installable, I'll rerun the exact curl sequence from #34 against a build containing this merge together with the v2.2.2 CSV-side fix, and replicate your two-process catalogue-build comparison to cover the default-mode path independently and will post the results here.

…ABTechLab#34)

The old test asserted every cache reset mints fresh product ids — the
exact behavior that broke follow-up lookups for external integrators.
Ids are now stable by design; the test asserts identity across resets.
@atc964
atc964 merged commit fe6b093 into IABTechLab:main Jul 29, 2026
4 checks passed
@jaanijuk

Copy link
Copy Markdown

Ran the two-process catalogue-build comparison against v2.3.3 (post-merge): identical id sets across separate processes, 13 ids, shape preserved. Full retest results on #34

atc964 added a commit that referenced this pull request Jul 30, 2026
…ounts, deal-sync docs (#40)

* docs: backfill CHANGELOG into dated 2.1.0–2.3.3 releases; version 2.3.3

Cut the stale [Unreleased] blob into dated release sections assigned by
tag history (git tag dates, first-parent merges per tag range), and add
the missing 2.1.1–2.3.3 entries. [Unreleased] is left empty at the top
for the next release's notes. Bump the long-stale pyproject version
(0.1.0) to the current release, 2.3.3.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: refresh stale surface counts — 46 MCP tools, 87 endpoints, 22 event types

Counts drifted across README and docs while the auto-generated
inventories (docs/reference/) stayed current. Bring every user-facing
count in line with the live registries: 46 MCP tools (was 41), 87 REST
endpoints across 25 OpenAPI tags (was 82/58/59 across 15–19), 22 event
types (was 16/21). Regenerate the README tool-category and
endpoint-group tables from the inventories (the old tables summed to 45
and 76 and cited two tools that no longer exist). Update the Bedrock
model example in the configuration guide to the current id used by
.env.example, and reword the reset_catalog_cache docstring to the
deterministic uuid5 product-id contract (#37 — comment-only change).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: surface reference inventories in nav; document the deal-sync subsystem

Add a Reference section to the mkdocs nav linking the auto-generated
inventories (REST endpoints, MCP tools, event types) that were
previously orphaned, and add the Troubleshooting guide to the Publisher
Guide section. Document the #32 deal-sync connector family
(DealSyncClient / DealSyncRegistry, clients/deal_sync_base.py — a peer
of the SSP registry) in the architecture overview's new Connector Layer
section and in the README architecture diagram, linking the existing
deals-api-mcp integration guide.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to 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.

3 participants