Skip to content

[codex] Fix creative transformer follow-ups#5377

Merged
bokelley merged 2 commits into
mainfrom
thoughts-on-5219
Jun 6, 2026
Merged

[codex] Fix creative transformer follow-ups#5377
bokelley merged 2 commits into
mainfrom
thoughts-on-5219

Conversation

@bokelley

@bokelley bokelley commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Fixes the creative transformer follow-ups from PR #5219 by aligning migration/release docs with applies_to_output_format_ids, making list_transformers no-match brief expansion return no options, and validating plural target_format_ids against transformer outputs.

Adds focused regression coverage plus an empty changeset for the docs/training-agent-only patch.

Validation: focused transformer tests, typecheck, precommit, pre-push storyboard matrices, and docs broken-link check.

@mintlify

mintlify Bot commented Jun 6, 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 6, 2026, 12:48 AM

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

# Conflicts:
#	docs/reference/whats-new-in-3-1.mdx
@bokelley bokelley marked this pull request as ready for review June 6, 2026 10:11

@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.

Clean follow-up cleanup from #5219 — docs now match the schema instead of contradicting it, and the plural-target path is no longer a validation gap. The right shape: the migration guide was the last doc still claiming per-output pricing "does not survive the move," and the schema (applies_to_output_format_ids + UNPRICEABLE_OUTPUT) already disagrees.

Things I checked

  • Docs match the wire, verbatim. New prose at creative-transformers.mdx:58 ("unscoped option is the default for any output," reject with UNPRICEABLE_OUTPUT rather than guess) is a near-verbatim restatement of the authoritative descriptions in static/schemas/source/core/vendor-pricing-option.json:17 and static/schemas/source/enums/error-code.json:118. ad-tech-protocol-expert: sound. No spec drift — both fields already shipped, this PR only aligns prose.
  • No dangling "split into per-price-point transformers" guidance. Both rewritten passages (L58, L70) are the only occurrences; bullet #2's "See the pricing note above" cross-ref still resolves. docs-expert: lands-well.
  • Plural target validation is correct. targetIds (task-handlers.ts:6755) is the canonical merge of target_format_ids plural / target_format_id singular, capped at 50. The new findIndex maps invalidTargetIndex to the right target_format_ids[i] slot, and the req.target_format_ids?.length guard picks the right field name. Behavior change is correct: a target outside the transformer's output_format_ids now fails closed regardless of which field carried it.
  • Empty-options change is fail-closed, not a regression. task-handlers.ts:3407 dropping the filtered.length > 0 guard means a zero-match brief returns options: [] instead of silently widening to the full catalog. Paging handles the empty set cleanly (moreOptions = false, no cursor). Tested. The field outlives its data was the old failure mode; this kills it.
  • Empty changeset is correct, not missing. Per .agents/playbook.md:289-293, empty frontmatter is the sanctioned mechanism for changes that don't touch the published protocol surface (static/schemas/source/**). This PR touches none. code-reviewer flagged it Blocker; overruling on the strength of the playbook policy plus zero schema-source files in the diff.

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

  • task-handlers.ts:6794targetIds[0] ?? req.target_format_id ?? ...: the ?? req.target_format_id is dead. targetIds[0] already derives from req.target_format_id in every path, so the fallback can never change the result. Drop it.
  • creative-transformers.mdx:24 — the "What changed" table still describes pricing as "per transformer" without noting per-output scoping survives via applies_to_output_format_ids. Untouched by this PR and not contradictory, just coarse. Optional one-line append.

Minor nits (non-blocking)

  1. Error-message wording shift. task-handlers.ts:6785 moved from target_format_id "X" is not... to Target format "X" is not.... The new test matches on code/field only, so nothing breaks — but any downstream string-match on the old message would. Heads-up, not a fix.

Approving on the strength of the protocol-conformance verdict plus the fail-closed validation. Follow-ups noted.

@bokelley bokelley merged commit 52ccdfd into main Jun 6, 2026
32 of 33 checks passed
@bokelley bokelley deleted the thoughts-on-5219 branch June 6, 2026 10:15
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