Skip to content

fix(registry): tighten metric_id+accreditation filter to per-metric semantics#3740

Closed
bokelley wants to merge 2 commits into
mainfrom
claude/issue-3733-per-metric-filter-semantics
Closed

fix(registry): tighten metric_id+accreditation filter to per-metric semantics#3740
bokelley wants to merge 2 commits into
mainfrom
claude/issue-3733-per-metric-filter-semantics

Conversation

@bokelley

@bokelley bokelley commented May 1, 2026

Copy link
Copy Markdown
Contributor

Closes #3733

Summary

When /api/registry/agents receives both metric_id and accreditation query params, the old implementation built independent JSONB @> containment predicates ANDed at SQL level. This allowed cross-metric false positives: a vendor with an attention_units metric (not MRC-accredited) and a separately MRC-accredited viewability metric would match ?metric_id=attention_units&accreditation=MRC, even though no single metric satisfies both constraints.

Fix: When both params are present, filterMeasurementAgents now emits one combined JSONB containment probe per (metric_id, accreditation) pair — cross-product AND semantics. Each probe requires a single metrics element to carry both metric_id and the nested accreditations[].accrediting_body. Solo filters (only metric_id or only accreditation) are unchanged and still use the existing @> path.

Also fixed (pre-existing docs error): The metric_id table row and OpenAPI description incorrectly said multiple values were "OR'd within the param". The code has always AND'd them (vendor must carry all named metrics). Both are corrected here.

Changeset: --empty — this is a server-side query behavior fix; no AdCP protocol schema changes.

Non-breaking justification

PR #3726 shipped this endpoint on 2026-05-01 (the same day this issue was filed). Zero production callers have observed the old behavior in steady state. The fix narrows results to what callers actually asked for; any agent using both params with the old code was getting false-positive vendors.

Changes

File Change
server/src/db/agent-snapshot-db.ts Cross-product combined containment when both metric_ids and accreditations are set
docs/registry/index.mdx Documents combined-filter semantics; corrects metric_id table row from "OR'd" to "AND'd"
server/src/routes/registry-api.ts Updates metric_id and accreditation OpenAPI descriptions to reflect AND semantics and cross-product note
server/tests/unit/agent-snapshot-db.test.ts New: 7 regression tests covering solo filters, combined 1×1 (the core regression case), cross-product 2×1, cross-product 1×2, and q filter

Pre-PR review

  • code-reviewer: approved — code logic correct, GIN index still used, no migration needed; flagged docs table contradiction (fixed in second commit)
  • adtech-product-expert: approved — strict per-metric semantics matches real buyer workflows; confirmed AND semantics correct for solo multi-value params; flagged OpenAPI metric_id description still said "OR'd" (fixed in second commit)

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See #3121
for context.

Session: https://claude.ai/code/session_01WtFVR4EFu1UUuLN9PKGVfx


Generated by Claude Code

claude added 2 commits May 1, 2026 10:59
…emantics (#3733)

When both metric_id and accreditation are present in /api/registry/agents,
the same metrics[] element must now satisfy both constraints simultaneously.
Independent JSONB predicates ANDed at SQL level allowed cross-metric false
positives. Cross-product AND semantics now apply for multi-value params.

Adds regression tests for filterMeasurementAgents and documents combined
filter semantics (including the 2×N cross-product warning) in docs.

Closes #3733

https://claude.ai/code/session_01WtFVR4EFu1UUuLN9PKGVfx
…mantics

Multiple metric_id or accreditation values were documented as OR'd but
the SQL has always AND'd them. Corrects table rows in docs/registry/index.mdx
and OpenAPI descriptions in registry-api.ts to accurately say AND'd.

Also adds cross-product AND note to the accreditation OpenAPI description
to match the combined-filter semantics block added in the parent commit.

https://claude.ai/code/session_01WtFVR4EFu1UUuLN9PKGVfx
@bokelley bokelley added the claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage. label May 1, 2026
@bokelley

Copy link
Copy Markdown
Contributor Author

Closing — superseded by merged #3733, which shipped the same per-metric semantics fix for the metric_id+accreditation filter.

@bokelley bokelley closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-triaged Issue has been triaged by the Claude Code triage routine. Remove to re-triage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tighten metric_id+accreditation filter to per-metric semantics (currently matches across different metrics)

2 participants