Skip to content

Fix BidCliq compliance suite fixtures#5308

Merged
bokelley merged 2 commits into
mainfrom
bidcliq-compliance-fixes
Jun 3, 2026
Merged

Fix BidCliq compliance suite fixtures#5308
bokelley merged 2 commits into
mainfrom
bidcliq-compliance-fixes

Conversation

@bokelley

@bokelley bokelley commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Updates compliance snapshots and storyboards for BidCliq's 3.0/3.1 runs: refreshes invalid_transitions, makes state_machine create active buys from discovered products, and removes the hardcoded test-product path. Adds seed_account support for deterministic list_accounts pagination and wires the controller schema, advertised scenarios, and lint docs accordingly. Includes an empty changeset because the repo pre-push hook requires one. Validated with targeted storyboard/schema tests, build:compliance, build:schemas, precommit, and the pre-push storyboard matrices for current source and 3.0 compatibility.

@mintlify

mintlify Bot commented Jun 3, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
adcp 🟢 Ready View Preview Jun 3, 2026, 4:20 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Follow-ups noted below. Right shape: new seed_account mirrors the existing seed_creative_format pattern, idempotency contract is explicit, and the 3.0-compat path now prefers session-scoped accounts before falling back to the fixture pool so the seed flow works in both compat windows.

Things I checked

  • seedAccountFixture at server/src/training-agent/account-handlers.ts:456 — auth guard (durableAccountIdentityError) matches handleSyncAccounts / handleListAccounts gate; defaults (billing=operator, status=active, paymentTerms=net_30, accountScope=operator_brand, rateCard=sandbox, sandbox=true) match the sandbox path in handleSyncAccounts.
  • handleListAccounts 3.0-compat ternary at server/src/training-agent/account-handlers.ts:1067-1075preferFixtureAccounts && scopedAccounts.length > 0 now returns seeded accounts; existing fallback to getComplianceAccounts() preserved. Test exercises both 3.1 and 3.0 paths and confirms has_more=true + cursor on max_results=2.
  • Schema if/then at static/schemas/source/compliance/comply-test-controller-request.json:363-383 — same shape as the other seed_* conditionals; requires params.account_id, leaves fixture permissive. scenario is type: string (no enum), so additive by construction.
  • audit-oneof.mjs unaffected — new construct is if/then inside allOf, not a oneOf; baseline scripts/oneof-discriminators.baseline.json does not need to move.
  • start_time: \"asap\" in the state-machine storyboard validates against static/schemas/source/core/start-timing.json (oneOf includes const: \"asap\"). Clean substitution for the hardcoded ISO timestamp.
  • Scenario count assertion at server/tests/unit/comply-test-controller.test.ts:199 lifted from 19 → 20; LOCAL_SCENARIOS, get_adcp_capabilities, and tenant router scenario lists all updated consistently.

Follow-ups (non-blocking — file as issues)

  • Pagination storyboard now under-tests its own name. static/compliance/source/universal/pagination-integrity-list-accounts.yaml dropped both the has_more=false + cursor-absence terminal-page assertion and the total_count == 3 invariant — the field_value_or_absent checks are gone. The justification ("agents may have pre-existing sandbox accounts") is undercut by the same diff turning on controller_seeding: true, which is exactly the session-isolation precondition that made the terminal assertion safe in the sibling pagination-integrity.yaml (list_creatives) storyboard. Either rename to pagination-integrity-list-accounts-continuation to match what it actually validates, or restore the terminal-page + total_count checks under the session-isolation predicate. As-is, the storyboard no longer catches the under-reporting failure mode it exists to detect.
  • Changeset is empty; prior art was minor. feat(compliance): add seed_creative_format (PR #3114, commit 926b07935) shipped a minor changeset for the structurally identical change — new scenario value + new if/then in the published comply controller schema. The "runners MUST accept unknown scenario strings" rule governs consumers, not the spec's own version bookkeeping. Either justify the empty bump in .changeset/wild-rocks-yell.md (the issue-5228 changeset is the precedent for empty-with-prose) or convert to minor. The storyboard and state-machine edits are orthogonal compliance-fixture work and don't change that calculus.
  • Idempotency-equality fragility at server/src/training-agent/account-handlers.ts:511-513. JSON.stringify(accountStateToWire(...)) is order-dependent and silently breaks if accountStateToWire's key emission order changes or if a new conditional field gets added. Swap for a accountStateEquals(a, b) helper that compares semantic fields explicitly. Same gotcha applies to cross-session findAccountByIdAcrossSessions matches — when we early-return without accounts.set(key, state), the current session never gets a local copy. Fine today because handleListAccounts reads via accountsForPrincipal, but a future caller using getAccountMap directly would miss it.
  • 3.0-compat filter chain bypass at server/src/training-agent/account-handlers.ts:1140-1150. When preferFixtureAccounts === true, req.sandbox / req.status / exactAccountFilter are skipped — mirrors prior behavior, but worth a one-line comment so a future reader doesn't "fix" it.

Minor nits (non-blocking)

  1. Double-cast on brand in seedAccountFixture. server/src/training-agent/account-handlers.ts:497 re-casts brand to { domain: string; brand_id?: string; name?: string } even though brand?.domain was narrowed truthy above. A typed local would drop the cast.
  2. The fixture-name domains (pagination-acme-1.example, etc.) drift from the test-kit's own acme-outdoor.example brand. Not wrong — the storyboard just wants three distinct sandbox accounts — but the prefix pagination- makes it harder to grep the storyboard back to its test-kits/acme-outdoor.yaml kit. Non-blocking.

Approving on the strength of the schema-pattern match against existing seed_* scenarios plus the round-trip test covering both 3.0 and 3.1 compat modes.

@bokelley
bokelley merged commit 584b6dd into main Jun 3, 2026
27 checks passed
@bokelley
bokelley deleted the bidcliq-compliance-fixes branch June 3, 2026 18:21
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.

1 participant