Wire the conversion-rule tools — the plugin's first account-level writes - #13
Merged
Merged
Conversation
This was referenced Aug 27, 2026
Closed
Closed
amitl-levi
force-pushed
the
web-fallback
branch
from
August 30, 2026 08:20
d2d3c6f to
998d4b1
Compare
amitl-levi
force-pushed
the
conversion-rule-writes
branch
from
August 30, 2026 08:20
15569bb to
8ea497b
Compare
yanush88
approved these changes
Aug 30, 2026
yanush88
left a comment
yanush88
left a comment
Contributor
There was a problem hiding this comment.
Approved but IMO needs @yuri-g-taboola approval
…evel writes Upstream shipped create_conversion_rule, update_conversion_rule, and get_conversion_rules. Tool counts go from 19 read + 6 write to 20 read + 8 write. Users can now create a conversion rule, change its attribution windows, add revenue value, stop it counting toward Total Conversions, and retire it - all through the existing preview-then- confirm gate. Per the sync policy in CLAUDE.md the writes route exclusively through manage-campaigns, never the agent. No one-line confirm tier for these. Every other write scopes to one campaign or item; a conversion rule feeds attribution and, when include_in_total_conversions is set, the account's Total Conversions, which Target CPA and Maximize Conversions bid against. One boolean moves reported performance and live bidding across every campaign on the account. The duplicate-event guard is the load-bearing rule. Only one ACTIVE rule may hold an event, and the backend accepts a second once the incumbent is DISABLED - which makes "disable the old one and retry" look like a legitimate recovery from a rejected create. It stops conversion reporting for every campaign using the incumbent. Both the skill and the guardrails forbid taking it unilaterally. Merge semantics are inverted from campaign targeting: these tools partial-merge everything including condition and effects, so the read-and-merge reflex the rest of the skill trains overwrites fields nobody asked to change. The read payload is also not a valid update payload - explicit nulls fail validation and seven fields have no parameter on the update tool. Documented from live QA against a real account: ownership runs both directions (a child account returned 62 rules, all owned by the parent network), the read returns ENGAGEMENT rules the write schema cannot express, and some accounts are write-blocked server-side in a way that looks exactly like a validation error. search_conversion_rules is deprecated upstream and removed after 2026-11-01; migrated everywhere. Stale UI-only claims corrected - the triage table, tool-existence boundary, README, and guardrails all still sent users to the UI for work the plugin can now do. W10-W14 remain unrun: both internal test accounts tried are on the server-side write blocklist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review feedback from PR #9: the capability-baseline section narrated when conversion-rule tools arrived instead of just stating what exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field finding after the 20260806.1 MCP release: the read has no pagination and no status filter, so a rule-heavy account (observed: 278 rules / ~270 KB, only 102 ACTIVE) overflows the tool-result cap and arrives as an error plus a dumped-file path. rule_id narrowing is circular - the listing is the only way to learn the IDs. Interim guidance until upstream ships pagination/status filtering: - Recover from the dumped file (read in slices); never re-call unmodified, never abandon, never treat the overflow as "no rules". - The mandatory pre-write read inherits the overflow: event/name collision and ownership checks run against the dumped file, across all statuses (a DISABLED rule still holds its display_name). - User-facing listings default to ACTIVE rules with a mandatory one-line disclosure of skipped counts; non-ACTIVE only on explicit ask. Disclosure phrasing carries no file paths or tool names. - Read Scenario 21 (overflow recovery), new pass criteria on 20A and W10, README troubleshooting entry, CHANGELOG and CLAUDE.md notes marking this guidance stale-capability-claims class once upstream pagination ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings from PR 11's review, applied where the capability actually shipped: - web-fallback's how-vs-do example still cited "create the conversion event" as a UI-only action - the lone claim site the release sweep missed, because it phrases the capability differently than the grep patterns used. Examples replaced with actions that stay UI-only (pixel install, CRM upload), plus an explicit do-not-cite note for conversion rules. - knowledge/bidding.md step 4 still said "Realize UI today; MCP write tool when available" one line after step 3 named the shipped tool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Findings from the requested per-PR review, applied where they belong: - The read count was wrong everywhere it was stated: the rename (search_conversion_rules -> get_conversion_rules) was counted as an addition. It is 19 read + 8 write, not 20 + 8 - fixed in the agent (x2), CLAUDE.md's diagram, the gap doc, and the changelog wording. - docs/realize-best-practices-gap.md still declared conversion tracking "the single largest capability gap" while this release ships four of that section's six wished-for tools. The section now opens with a closed-gaps block, the writes row lists all 8 tools, the header count matches, the view-through unit is corrected (minutes, not 1-24 hours), and the "natural home" example no longer proposes a skill for a capability that shipped. - discovery's ID-type gotcha contradicted the new tool docs: rule ids are integers inside payloads but strings as the rule_id parameter. The gotcha now carries the split, matching the write-surface note. - mcp-write-surface pointed at skills/create-campaign (renamed long ago) and had no field section for the two new write tools - both fixed; new 3b table covers shapes, units, immutables, and the read-only fields the update tool rejects. - Changelog self-contradiction on rule ownership direction resolved (both directions), rename blast-radius corrected to seven files, and the migrated-files list completed. - knowledge/manifest.json's tracking description no longer advertises "Pixel setup", which routed install questions away from the lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amitl-levi
force-pushed
the
web-fallback
branch
from
August 30, 2026 10:43
998d4b1 to
7d87dbe
Compare
amitl-levi
force-pushed
the
conversion-rule-writes
branch
from
August 30, 2026 10:43
8ea497b to
5a28973
Compare
The category example named PURCHASE, which the API rejects — the live create_conversion_rule schema has MAKE_PURCHASE. Replace the two-value example with the full 15-value enum so the doc can't invite this class of guess again. Caught by Yuri in review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Sep 1, 2026
Merged
This was referenced Sep 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 3 of the split of former PRs #9/#10. Wires upstream's
get_conversion_rules/create_conversion_rule/update_conversion_rulebehind the manage-campaigns write gate, plus the overflow survival guidance for the unpaginated read.Per review feedback on #12/#15: all conversion-rule documentation now lives here — the changelog entries, agent triage row + tracking ladder, guardrails blast-radius rules, README scope, tool counts, and test scenario moved out of the web-fallback PR into this one, so #15 is web-fallback only.
Review fixes applied in this PR (Claude-reviewed, two passes):
tests/dataset-creation-prompt.mdis removed from history, not just deleted — it never enters main.search_conversion_rules→get_conversion_rules) is not an addition, so it's 19 read + 8 write, not 20 + 8.docs/realize-best-practices-gap.mdno longer calls conversion tracking "the single largest capability gap" (this PR ships 4 of that section's 6 wished-for tools) and its closed rows are struck; the view-through unit is fixed (minutes, not 1–24 hours).rule_idparameter).web-fallback/bidding.mdUI-only claims fixed; changelog self-contradictions on rule ownership resolved.Stack: based on
web-fallback(PR #15). Merge order: #15 → this → #14 → #11. After merging each PR, delete its branch via the button on the PR page — that's what re-points the next PR at main.🤖 Generated with Claude Code