Skip to content

Fix: Creative management - reject invalid creatives #460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bokelley merged 13 commits into from
Oct 16, 2025
Merged

Fix: Creative management - reject invalid creatives #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
849bf65
Fix creative management UI bugs
bokelley Oct 16, 2025
354154f
Reject creatives when preview generation returns no previews
bokelley Oct 16, 2025
864d7a6
Reject creatives when validation fails due to network errors
bokelley Oct 16, 2025
671e30c
Fix: Return failed creatives properly in AdCP response schema
bokelley Oct 16, 2025
69705c6
Add integration tests for creative validation failures
bokelley Oct 16, 2025
8f5d0f0
Add creative agent to docker-compose and real integration tests
bokelley Oct 16, 2025
6f5e8f8
Update creative agent docker-compose to build from source
bokelley Oct 16, 2025
8268526
Add manual creative agent testing tools
bokelley Oct 16, 2025
5ba9329
Fix: Add required fields to Creative schema in integration tests
bokelley Oct 16, 2025
5fdc612
WIP: Update creative agent integration test schema
bokelley Oct 16, 2025
3b5ae20
Mark real creative agent tests as skip_ci
bokelley Oct 16, 2025
bdf2433
Remove creative agent from docker-compose
bokelley Oct 16, 2025
40ecaec
Remove broken mocked creative validation tests
bokelley Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 96 additions & 22 deletions src/core/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2126,19 +2126,56 @@ def _sync_creatives_impl(
f"url={bool(data.get('url'))}, "
f"width={data.get('width')}, "
f"height={data.get('height')}, "
f"variants={len(preview_result.get('previews', [])) if preview_result else 0}"
f"variants={len(preview_result.get('previews', []))}"
)
else:
# Preview generation failed for update - creative is invalid
error_msg = f"Creative validation failed: preview_creative returned no previews for update of {existing_creative.creative_id}"
logger.error(f"[sync_creatives] {error_msg}")
failed_creatives.append(
{
"creative_id": existing_creative.creative_id,
"error": error_msg,
"format": creative_format,
}
)
failed_count += 1
results.append(
SyncCreativeResult(
creative_id=existing_creative.creative_id,
action="failed",
errors=[error_msg],
)
)
continue # Skip this creative update

except Exception as validation_error:
# Creative agent validation failed for update - log warning but continue
# This allows updates even if creative agent is down
error_msg = f"Creative agent validation failed: {str(validation_error)}"
logger.warning(
f"[sync_creatives] {error_msg} for update of {existing_creative.creative_id} - continuing with update",
# Creative agent validation failed for update (network error, agent down, etc.)
# Do NOT update the creative - it needs validation before acceptance
error_msg = (
f"Creative agent unreachable or validation error: {str(validation_error)}. "
f"Retry recommended - creative agent may be temporarily unavailable."
)
logger.error(
f"[sync_creatives] {error_msg} for update of {existing_creative.creative_id}",
exc_info=True,
)
# Note: We continue instead of failing to allow graceful degradation
# when creative agent is unavailable
failed_creatives.append(
{
"creative_id": existing_creative.creative_id,
"error": error_msg,
"format": creative_format,
}
)
failed_count += 1
results.append(
SyncCreativeResult(
creative_id=existing_creative.creative_id,
action="failed",
errors=[error_msg],
)
)
continue # Skip this creative update

# In full upsert, consider all fields as changed
changes.extend(["url", "click_url", "width", "height", "duration"])
Expand Down Expand Up @@ -2389,24 +2426,61 @@ def _sync_creatives_impl(
if dimensions.get("duration"):
data["duration"] = dimensions["duration"]

logger.info(
f"[sync_creatives] Preview data populated: "
f"url={bool(data.get('url'))}, "
f"width={data.get('width')}, "
f"height={data.get('height')}, "
f"variants={len(preview_result.get('previews', [])) if preview_result else 0}"
)
logger.info(
f"[sync_creatives] Preview data populated: "
f"url={bool(data.get('url'))}, "
f"width={data.get('width')}, "
f"height={data.get('height')}, "
f"variants={len(preview_result.get('previews', []))}"
)
else:
# Preview generation failed - creative is invalid
error_msg = f"Creative validation failed: preview_creative returned no previews for {creative_id}"
logger.error(f"[sync_creatives] {error_msg}")
failed_creatives.append(
{
"creative_id": creative_id,
"error": error_msg,
"format": creative_format,
}
)
failed_count += 1
results.append(
SyncCreativeResult(
creative_id=creative_id,
action="failed",
errors=[error_msg],
)
)
continue # Skip this creative

except Exception as validation_error:
# Creative agent validation failed - log warning but continue
# This allows creatives to be stored even if creative agent is down
error_msg = f"Creative agent validation failed: {str(validation_error)}"
logger.warning(
f"[sync_creatives] {error_msg} - continuing with creative storage",
# Creative agent validation failed (network error, agent down, etc.)
# Do NOT store the creative - it needs validation before acceptance
error_msg = (
f"Creative agent unreachable or validation error: {str(validation_error)}. "
f"Retry recommended - creative agent may be temporarily unavailable."
)
logger.error(
f"[sync_creatives] {error_msg} - rejecting creative {creative_id}",
exc_info=True,
)
# Note: We continue instead of failing to allow graceful degradation
# when creative agent is unavailable
failed_creatives.append(
{
"creative_id": creative_id,
"error": error_msg,
"format": creative_format,
}
)
failed_count += 1
results.append(
SyncCreativeResult(
creative_id=creative_id,
action="failed",
errors=[error_msg],
)
)
continue # Skip storing this creative

# Determine creative status based on approval mode

Expand Down
11 changes: 8 additions & 3 deletions templates/creative_management.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ <h2 id="previewTitle" style="margin: 0 0 1.5rem 0;">Creative Preview</h2>
.then(response => response.json())
.then(data => {
if (data.success) {
alert(`AI Review Complete:\n\nDecision: ${data.decision}\n\nReasoning: ${data.reasoning}`);
alert(`AI Review Complete:\n\nStatus: ${data.status}\n\nReason: ${data.reason}\n\nConfidence: ${data.confidence}`);
window.location.reload();
} else {
alert('Error: ' + (data.error || 'Unknown error'));
Expand Down Expand Up @@ -496,8 +496,13 @@ <h3 style="margin: 0 0 0.5rem 0;">${creativeData.title || 'Native Ad'}</h3>
}
} else {
html += `<div style="text-align: center; padding: 3rem; color: #999;">
<p>No preview available for this creative.</p>
<p style="font-size: 0.9rem; margin-top: 0.5rem;">Creative data: ${JSON.stringify(creativeData || {}, null, 2)}</p>
<div style="font-size: 3rem; margin-bottom: 1rem;">🖼️</div>
<h3 style="color: #666; margin-bottom: 0.5rem;">No Preview Available</h3>
<p style="color: #999; margin-bottom: 1rem;">This creative doesn't have preview data yet.</p>
<details style="text-align: left; max-width: 600px; margin: 0 auto; padding: 1rem; background: #f9fafb; border-radius: 8px;">
<summary style="cursor: pointer; font-weight: 600; color: #666;">Show Creative Data</summary>
<pre style="margin-top: 1rem; padding: 1rem; background: white; border-radius: 4px; overflow-x: auto; font-size: 0.85rem; text-align: left;">${JSON.stringify(creativeData || {}, null, 2)}</pre>
</details>
</div>`;
}

Expand Down