Skip to content

Commit 1ca22bb

Browse files
nictukuclaude
andcommitted
fix: LOWEST_COST_WITH_MIN_ROAS uses bid_constraints, not bid_amount
The reviewer correctly identified that LOWEST_COST_WITH_MIN_ROAS requires bid_constraints={"roas_average_floor": <value>} (scaled 10000x), NOT bid_amount. Removed it from strategies_requiring_bid_amount to avoid producing a misleading validation error. Updated docstrings, error messages, and tests to reflect the correct parameter requirements for each bid strategy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4062eb5 commit 1ca22bb

2 files changed

Lines changed: 29 additions & 29 deletions

File tree

‎meta_ads_mcp/core/adsets.py‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -119,26 +119,28 @@ async def create_adset(
119119
Use targeting_automation.advantage_audience=1 for automatic audience finding
120120
For interest targeting, obtain Interest targeting IDs using the search_interests tool.
121121
bid_amount: Bid amount in account currency (in cents).
122-
REQUIRED for: LOWEST_COST_WITH_BID_CAP, COST_CAP, LOWEST_COST_WITH_MIN_ROAS.
122+
REQUIRED for: LOWEST_COST_WITH_BID_CAP, COST_CAP, TARGET_COST.
123123
NOT REQUIRED for: LOWEST_COST_WITHOUT_CAP (recommended default).
124+
NOT USED by: LOWEST_COST_WITH_MIN_ROAS (uses bid_constraints instead).
124125
Note: May also be required if the parent campaign uses a bid strategy that requires it.
125126
bid_strategy: Bid strategy determines how Meta optimizes your bids.
126127
Valid values:
127128
- 'LOWEST_COST_WITHOUT_CAP' (recommended) - No bid_amount required, most flexible
128129
- 'LOWEST_COST_WITH_BID_CAP' - Sets max bid cap, REQUIRES bid_amount
129130
- 'COST_CAP' - Caps cost per result, REQUIRES bid_amount
130-
- 'LOWEST_COST_WITH_MIN_ROAS' - Sets minimum ROAS, REQUIRES bid_amount
131-
131+
- 'LOWEST_COST_WITH_MIN_ROAS' - Sets minimum ROAS, REQUIRES bid_constraints
132+
with roas_average_floor (not bid_amount). Also requires optimization_goal=VALUE.
133+
132134
IMPORTANT: 'LOWEST_COST' is NOT a valid value - use 'LOWEST_COST_WITHOUT_CAP' instead.
133-
134-
IMPORTANT: Campaign-level bid strategy constrains ad set choices. If the parent campaign
135-
uses LOWEST_COST_WITH_BID_CAP, all child ad sets MUST provide bid_amount regardless of
135+
136+
IMPORTANT: Campaign-level bid strategy constrains ad set choices. If the parent campaign
137+
uses LOWEST_COST_WITH_BID_CAP, all child ad sets MUST provide bid_amount regardless of
136138
their own bid_strategy setting.
137-
139+
138140
Examples:
139141
1. Recommended (no bid amount needed):
140142
bid_strategy="LOWEST_COST_WITHOUT_CAP"
141-
143+
142144
2. With bid cap (bid_amount required):
143145
bid_strategy="LOWEST_COST_WITH_BID_CAP", bid_amount=500
144146
start_time: Start time in ISO 8601 format (e.g., '2023-12-01T12:00:00-0800')
@@ -231,11 +233,10 @@ async def create_adset(
231233
"targeting_automation": {"advantage_audience": 1}
232234
}
233235

234-
# Bid strategies that require bid_amount
236+
# Bid strategies that require bid_amount (not bid_constraints)
235237
strategies_requiring_bid_amount = [
236238
'LOWEST_COST_WITH_BID_CAP',
237239
'COST_CAP',
238-
'LOWEST_COST_WITH_MIN_ROAS',
239240
'TARGET_COST',
240241
]
241242

@@ -251,7 +252,7 @@ async def create_adset(
251252
"LOWEST_COST_WITHOUT_CAP (recommended - no bid_amount required)",
252253
"LOWEST_COST_WITH_BID_CAP (requires bid_amount)",
253254
"COST_CAP (requires bid_amount)",
254-
"LOWEST_COST_WITH_MIN_ROAS (requires bid_amount)"
255+
"LOWEST_COST_WITH_MIN_ROAS (requires bid_constraints with roas_average_floor)"
255256
],
256257
"example": '{"bid_strategy": "LOWEST_COST_WITHOUT_CAP"}'
257258
}, indent=2)
@@ -387,11 +388,12 @@ async def update_adset(adset_id: str, frequency_control_specs: Optional[List[Dic
387388
- 'LOWEST_COST_WITHOUT_CAP' (recommended) - No bid_amount required
388389
- 'LOWEST_COST_WITH_BID_CAP' - REQUIRES bid_amount
389390
- 'COST_CAP' - REQUIRES bid_amount
390-
- 'LOWEST_COST_WITH_MIN_ROAS' - REQUIRES bid_amount
391-
391+
- 'LOWEST_COST_WITH_MIN_ROAS' - REQUIRES bid_constraints (not bid_amount)
392+
392393
IMPORTANT: 'LOWEST_COST' is NOT a valid value - use 'LOWEST_COST_WITHOUT_CAP' instead.
393394
bid_amount: Bid amount in account currency (in cents for USD).
394-
REQUIRED when using LOWEST_COST_WITH_BID_CAP, COST_CAP, or LOWEST_COST_WITH_MIN_ROAS.
395+
REQUIRED when using LOWEST_COST_WITH_BID_CAP, COST_CAP, or TARGET_COST.
396+
NOT USED by LOWEST_COST_WITH_MIN_ROAS (uses bid_constraints instead).
395397
status: Update ad set status (ACTIVE, PAUSED, etc.)
396398
targeting: Complete targeting specifications (will replace existing targeting)
397399
(e.g. {"targeting_automation":{"advantage_audience":1}, "geo_locations": {"countries": ["US"]}})
@@ -416,16 +418,15 @@ async def update_adset(adset_id: str, frequency_control_specs: Optional[List[Dic
416418
"LOWEST_COST_WITHOUT_CAP (recommended - no bid_amount required)",
417419
"LOWEST_COST_WITH_BID_CAP (requires bid_amount)",
418420
"COST_CAP (requires bid_amount)",
419-
"LOWEST_COST_WITH_MIN_ROAS (requires bid_amount)"
421+
"LOWEST_COST_WITH_MIN_ROAS (requires bid_constraints with roas_average_floor)"
420422
],
421423
"example": '{"bid_strategy": "LOWEST_COST_WITHOUT_CAP"}'
422424
}, indent=2)
423-
424-
# Bid strategies that require bid_amount
425+
426+
# Bid strategies that require bid_amount (not bid_constraints)
425427
strategies_requiring_bid_amount = [
426428
'LOWEST_COST_WITH_BID_CAP',
427429
'COST_CAP',
428-
'LOWEST_COST_WITH_MIN_ROAS',
429430
'TARGET_COST',
430431
]
431432

‎tests/test_bid_strategy_validation.py‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -165,25 +165,25 @@ async def test_create_adset_cost_cap_requires_bid_amount(
165165
mock_api_request.assert_not_called()
166166

167167
@pytest.mark.asyncio
168-
async def test_create_adset_lowest_cost_with_min_roas_requires_bid_amount(
168+
async def test_create_adset_lowest_cost_with_min_roas_does_not_require_bid_amount(
169169
self, mock_api_request, basic_adset_params
170170
):
171-
"""Test that LOWEST_COST_WITH_MIN_ROAS requires bid_amount"""
171+
"""Test that LOWEST_COST_WITH_MIN_ROAS does NOT require bid_amount.
172+
173+
This strategy uses bid_constraints with roas_average_floor instead of bid_amount.
174+
The validation should NOT block the request for missing bid_amount.
175+
"""
172176
result = await create_adset(
173177
**basic_adset_params,
174178
bid_strategy="LOWEST_COST_WITH_MIN_ROAS"
175-
# Missing bid_amount
179+
# No bid_amount - should be fine, this strategy uses bid_constraints instead
176180
)
177181

178182
result_data = parse_result(result)
179183

180-
# Should return error
181-
assert "error" in result_data
182-
assert "bid_amount is required" in result_data["error"]
183-
assert "LOWEST_COST_WITH_MIN_ROAS" in result_data["error"]
184-
185-
# API should NOT have been called
186-
mock_api_request.assert_not_called()
184+
# Should NOT return a bid_amount validation error
185+
if "error" in result_data:
186+
assert "bid_amount is required" not in result_data.get("error", "")
187187

188188
@pytest.mark.asyncio
189189
async def test_update_adset_lowest_cost_with_bid_cap_requires_bid_amount(
@@ -432,7 +432,6 @@ async def test_backward_compatibility_all_valid_strategies(
432432
valid_strategies_requiring_bid_amount = [
433433
"LOWEST_COST_WITH_BID_CAP",
434434
"COST_CAP",
435-
"LOWEST_COST_WITH_MIN_ROAS"
436435
]
437436

438437
for strategy in valid_strategies_requiring_bid_amount:

0 commit comments

Comments
 (0)