Compare commits
28 commits
feature/de
...
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7b0e22060e | ||
|
|
77276f2a7c | ||
|
|
71bb9a6295 | ||
|
|
03410cb271 | ||
|
|
68a2360811 | ||
|
|
0e833447c0 | ||
|
|
4c19a0fb9d | ||
|
|
014a9cb8ff | ||
|
|
568465f9be | ||
|
|
295305ef2d | ||
|
|
c51e0729ce | ||
|
|
abd36a9abe | ||
|
|
ed46504ac6 | ||
|
|
7d178f11ee | ||
|
|
53ba67c2c0 | ||
|
|
6315393c95 | ||
|
|
fc588c626d | ||
|
|
1057c5660f | ||
|
|
eb7ba38013 | ||
|
|
8e2d970d61 | ||
|
|
1c5dd980d4 | ||
|
|
8e50413b53 | ||
|
|
86dec44124 | ||
|
|
a3b3f45f01 | ||
|
|
45925746cf | ||
|
|
e25006039f | ||
|
|
31b059de79 | ||
|
|
432162f167 |
17 changed files with 2126 additions and 179 deletions
11
.gitignore
vendored
11
.gitignore
vendored
|
|
@ -78,3 +78,14 @@ backend/client_defaults.json
|
|||
backend/config/development.env
|
||||
backend/config/production.env
|
||||
backend/config/box_jwt_config.json
|
||||
|
||||
# Legacy env paths (pre-config/ refactor) still in use on older deploys.
|
||||
# Untracked 2026-05-17 — git reset --hard during deploys was overwriting
|
||||
# rotated secrets with the historical (compromised) values.
|
||||
config.env
|
||||
backend/config.env
|
||||
config/development.env
|
||||
config/production.env
|
||||
|
||||
# Local test fixtures (real HP Source Messaging files; not for commit)
|
||||
backend/tests/fixtures/
|
||||
|
|
|
|||
197
backend/BOX_CLIENT_ONBOARDING.md
Normal file
197
backend/BOX_CLIENT_ONBOARDING.md
Normal file
|
|
@ -0,0 +1,197 @@
|
|||
# Box Client Onboarding Runbook
|
||||
|
||||
Adds a new client to the Box-webhook-driven QC pipeline (Phase 4). Run through this once per client. Most steps need ~5 minutes; total ~30 minutes including Box admin turnaround for collaborator invites.
|
||||
|
||||
Architectural reference: the JWT auth + webhook endpoint live in `backend/box_jwt_client.py` and `backend/api_server.py` (search for `_run_box_triggered_analysis`). The admin CLI is `backend/scripts/box_setup.py`. The JWT auth coexists with an older per-user OAuth flow in `backend/box_client.py` — different code path, dormant scaffolding, not used by this pipeline.
|
||||
|
||||
---
|
||||
|
||||
## What you need before starting
|
||||
|
||||
- **Box admin access** (or someone who can act as one) — to create folders and invite the service account.
|
||||
- **SSH access to the dev server** (`optical-production-dev`) — to run the bootstrap CLI and tail logs.
|
||||
- **Repo write access** — to land the `client_config.py` change as a PR.
|
||||
- **The client's profile decisions** — which profile should be the unattended-run default? (Pick from the client's existing `profiles` list.)
|
||||
|
||||
Already done at the platform level (don't redo per-client):
|
||||
- JWT config JSON at `/opt/ai_qc/backend/config/box_jwt_config.json` on each server
|
||||
- `BOX_WEBHOOK_PRIMARY_KEY` + `BOX_WEBHOOK_SECONDARY_KEY` in each server's env file
|
||||
- ffmpeg installed (for video pre-flight)
|
||||
|
||||
---
|
||||
|
||||
## Step 1 — Box-side prep (admin task)
|
||||
|
||||
For client `<CLIENT>` (e.g. Diageo):
|
||||
|
||||
1. **Create two folders in Box:**
|
||||
- `AI-QC > INCOMING > AI QC <CLIENT> IN` — where source assets land
|
||||
- `AI-QC > REPORTS > AI QC <CLIENT> REPORTS` — where QC reports land
|
||||
|
||||
2. **Invite the JWT service account as a collaborator on BOTH folders.** Role: **Editor** or higher. (Editor lets it read uploads, write reports, and move files into the auto-created `_PROCESSED` subfolder. Co-owner also works.)
|
||||
|
||||
3. **Capture the folder IDs.** Box shows them in the URL when you open a folder, or you can list them programmatically once invites are in:
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
venv/bin/python backend/scripts/box_setup.py list-folder <parent_AI-QC_folder_id>
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Step 2 — Code change
|
||||
|
||||
Edit `backend/client_config.py`, add three optional fields to the client entry:
|
||||
|
||||
```python
|
||||
'<client_id>': {
|
||||
'name': 'Client Display Name',
|
||||
'profiles': ['client_specific_profile', 'static_general', 'video_general'],
|
||||
'display_name': 'Client Display Name',
|
||||
'description': '...',
|
||||
'box_folder_id': '<INCOMING folder ID>',
|
||||
'box_reports_folder_id': '<REPORTS folder ID>',
|
||||
'default_profile': '<one of the profiles above>',
|
||||
},
|
||||
```
|
||||
|
||||
Then:
|
||||
- Push as a small PR → merge to `develop`
|
||||
- On the dev server: `cd /opt/ai_qc && ./backend/scripts/deploy.sh dev`
|
||||
- No env-file backup dance needed (this is a code-only change)
|
||||
|
||||
---
|
||||
|
||||
## Step 3 — Verify the service account got access
|
||||
|
||||
Before registering webhooks, sanity-check that the service account can actually read the folders the admin invited it to:
|
||||
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
venv/bin/python backend/scripts/box_setup.py list-folder <INCOMING folder ID>
|
||||
venv/bin/python backend/scripts/box_setup.py list-folder <REPORTS folder ID>
|
||||
```
|
||||
|
||||
Expected: both print `Folder <id> contains N items:` even if empty.
|
||||
|
||||
**If you get `Access Denied` / HTTP 403**: the service account isn't actually a collaborator yet. Box admin needs to retry the invite. Common causes:
|
||||
- Invite went to the wrong identity (Box has separate "user" and "app" identities — the JWT app is an app)
|
||||
- Invite is pending acceptance somewhere
|
||||
- Folder was created but invite wasn't applied at the right level
|
||||
|
||||
Don't proceed until both `list-folder` calls succeed.
|
||||
|
||||
---
|
||||
|
||||
## Step 4 — Register the V2 webhook
|
||||
|
||||
**Option A: CLI (recommended)** — idempotent, batch-able, lives in version control:
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
venv/bin/python backend/scripts/box_setup.py register-all-clients \
|
||||
https://optical-dev.oliver.solutions/ai_qc/api/box/webhook
|
||||
```
|
||||
|
||||
The script:
|
||||
- Scans `client_config.py` for every client with `box_folder_id` set
|
||||
- For each, checks Box for an existing webhook on that folder pointing at the given URL
|
||||
- Skips ones that already exist
|
||||
- Creates webhooks for any that are missing
|
||||
- Prints `<client> (<folder_id>): CREATED webhook id=<id>` or `SKIP — webhook already exists`
|
||||
|
||||
Safe to re-run any time; it won't duplicate.
|
||||
|
||||
**Option B: Box Developer Console UI** — useful for one-off testing:
|
||||
- Box Developer Console → your Custom App → **Webhooks** tab → **Create Webhook**
|
||||
- URL: `https://optical-dev.oliver.solutions/ai_qc/api/box/webhook`
|
||||
- Content Type: **Folder** → search/pick the client's INCOMING folder
|
||||
- Event Triggers: tick **`FILE.UPLOADED`** only (do not tick others — they'd trigger spurious webhook deliveries)
|
||||
- Save
|
||||
|
||||
No new signing keys to generate — they're app-level, configured once for the whole Custom App.
|
||||
|
||||
---
|
||||
|
||||
## Step 5 — End-to-end test
|
||||
|
||||
Open one terminal:
|
||||
```bash
|
||||
sudo journalctl -u ai-qc.service -f
|
||||
```
|
||||
|
||||
In Box: upload a small test asset (image, PDF, or video) to the client's INCOMING folder.
|
||||
|
||||
Within a few seconds you should see (timestamps abbreviated):
|
||||
```
|
||||
Box webhook: dispatching session=<ts> client=<client_id> profile=<default_profile> file_id=...
|
||||
Box webhook: downloaded <file> → uploads-dev/<ts>/<file>
|
||||
Running check 1/N: <check_name>
|
||||
...
|
||||
Box webhook: uploaded report QC_Report_<ts>_<file>.html → folder <REPORTS folder ID>
|
||||
Box webhook: moved source → _PROCESSED/<ts>_<file>
|
||||
Box webhook: analysis complete for session <ts>, score <N>
|
||||
```
|
||||
|
||||
Then in Box, verify:
|
||||
- A new `QC_Report_<ts>_<original-filename>.html` exists in the REPORTS folder
|
||||
- The source file has been moved into the auto-created `_PROCESSED` subfolder inside INCOMING. Its new name has the session_id prefix, which ties back to the corresponding report.
|
||||
|
||||
---
|
||||
|
||||
## Step 6 — (Optional) Tune the default profile from the UI
|
||||
|
||||
If the team finds that the static `default_profile` in code doesn't match how they want webhook-triggered runs to behave, an admin can change it without a code deploy:
|
||||
|
||||
1. Open the app → pick the client in the picker
|
||||
2. ⚙️ **Settings** → **Default Profile** tab
|
||||
3. Click a different profile → **Set as default**
|
||||
|
||||
The override is persisted to `backend/client_defaults.json` (gitignored, per-server) and takes effect immediately on the next webhook run. **Revert to static default** clears the override.
|
||||
|
||||
---
|
||||
|
||||
## Step 7 — Promote to prod
|
||||
|
||||
After the dev test passes:
|
||||
|
||||
1. PR `develop → main` on Bitbucket. Merge.
|
||||
2. Tag main: e.g. `v1.2.0`, push the tag.
|
||||
3. On the prod server (`optical-production`):
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
./backend/scripts/deploy.sh prod v1.2.0
|
||||
```
|
||||
4. Once-per-environment prod prerequisites (you only do these the first time prod gets Phase 4, never again):
|
||||
- JWT config JSON at `/opt/ai_qc/backend/config/box_jwt_config.json` (scp from your laptop, `chmod 600`)
|
||||
- `BOX_WEBHOOK_PRIMARY_KEY` + `BOX_WEBHOOK_SECONDARY_KEY` in `production.env` — these are the same app-level keys as dev
|
||||
- `sudo apt install ffmpeg` (for video pre-flight)
|
||||
5. Register webhooks pointing at the prod URL (different from dev's URL — each webhook is bound to one address):
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
venv/bin/python backend/scripts/box_setup.py register-all-clients \
|
||||
https://optical-prod.oliver.solutions/ai_qc/api/box/webhook
|
||||
```
|
||||
|
||||
The Box folders themselves are shared — you don't create new prod-only folders. Both dev and prod webhooks fire on the same client folders. If you don't want prod handling uploads yet, just don't register the prod webhooks until you're ready.
|
||||
|
||||
---
|
||||
|
||||
## Common gotchas
|
||||
|
||||
| Symptom | Likely cause | Fix |
|
||||
|---|---|---|
|
||||
| 403 from `list-folder` | Service account isn't a collaborator on that folder yet | Box admin re-invites with Editor role |
|
||||
| `Box webhook: signature verification failed` in logs | Signing keys in env don't match what the Custom App has | Box Developer Console → Manage Signature Keys → regenerate → update env on each server → restart service |
|
||||
| `Box webhook: no client configured for Box folder <id>` | The folder ID Box sent doesn't match any `box_folder_id` in `client_config.py` | Check `client_config.py` against the actual Box folder ID; they're strings, must match exactly |
|
||||
| `Box webhook: skipping non-QC extension <ext>` | User uploaded a file type we don't QC (e.g. `.docx`, `.zip`) | Working as intended; document for the client |
|
||||
| Webhook fires correctly but source file stays in INCOMING | The report-upload step failed earlier; the move is gated on a successful report upload so the user can retry by re-uploading | Look upstream in the log for `failed to upload report to Box: <error>` and fix the cause (usually a permissions issue on the REPORTS folder) |
|
||||
| Re-uploading the same filename doesn't trigger a fresh webhook | This is normal Box V2 behavior — same-name "replace" uploads create new versions of the existing file, which the folder-scoped webhook doesn't fire on | The auto-move-to-`_PROCESSED` step solves this for the happy path. If a file got stuck in INCOMING because of a previous failure, move/delete it manually so the next upload is a genuinely-new file |
|
||||
| Reports folder fills up indefinitely | No auto-cleanup of old reports — by design | Manual cleanup, or add an age-based pruning script as a follow-up |
|
||||
| `_PROCESSED` folder not auto-created | Service account doesn't have Editor (Viewer can't create subfolders) | Box admin upgrades the collaborator role to Editor |
|
||||
|
||||
---
|
||||
|
||||
## What this onboarding does NOT cover
|
||||
|
||||
- **Removing a client from the integration** — to stop processing: delete the webhook in the Box Developer Console (or `box_setup.py delete-webhook <webhook_id>`), then remove the `box_folder_id` field from `client_config.py` in a PR. Existing reports in the REPORTS folder are left alone.
|
||||
- **Multiple webhook-triggered profiles per client** — current schema is one default profile per client. If a client needs `FILE.UPLOADED` in one folder to run profile A and a different folder to run profile B, that's a schema change (one `client_config.py` entry per folder, or extend the schema to `{folder_id: profile_id}` maps).
|
||||
- **Webhook health monitoring** — there's no alert if Box stops delivering. If you suspect webhooks are silent, drop a fresh test asset and watch logs; if nothing fires, check Box Developer Console → Webhooks → the webhook's `App Diagnostics` tab.
|
||||
|
|
@ -8,6 +8,7 @@ import sys
|
|||
import json
|
||||
import base64
|
||||
import collections
|
||||
import html
|
||||
import importlib
|
||||
import traceback
|
||||
import re
|
||||
|
|
@ -858,6 +859,8 @@ def get_client_from_profile(profile_id):
|
|||
return 'amazon'
|
||||
elif profile_lower.startswith('boots'):
|
||||
return 'boots'
|
||||
elif profile_lower.startswith('hp_'):
|
||||
return 'hp'
|
||||
elif profile_lower.startswith(('dow_jones', 'dj_', 'marketwatch', 'mw_', 'wsj')):
|
||||
return 'dow_jones'
|
||||
else:
|
||||
|
|
@ -973,6 +976,12 @@ def generate_html_content(report_data, filename, file_path=None):
|
|||
json_data = check_data.get('json_data', {})
|
||||
response_text = ""
|
||||
|
||||
# Structured findings (e.g. hp_copy_review) render as a table
|
||||
# instead of the default response-text block. If absent, falls
|
||||
# back to the existing text rendering below.
|
||||
findings = (json_data or {}).get('findings') if isinstance(json_data, dict) else None
|
||||
findings_html = _render_findings_table(findings) if findings is not None else None
|
||||
|
||||
# Try to extract detailed analysis from JSON data
|
||||
if json_data:
|
||||
# Look for common detailed fields in the JSON
|
||||
|
|
@ -1099,12 +1108,12 @@ def generate_html_content(report_data, filename, file_path=None):
|
|||
</div>
|
||||
<div class="analysis-section">
|
||||
<h4>Analysis Details:</h4>
|
||||
<div class="response-text">{response_text.replace(chr(10), '<br>')}</div>
|
||||
{f'<div class="response-text">{html.escape(json_data.get("summary", "") or "") if isinstance(json_data, dict) else ""}</div>{findings_html}' if findings_html is not None else f'<div class="response-text">{response_text.replace(chr(10), "<br>")}</div>'}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
"""
|
||||
|
||||
|
||||
# Get summary score result
|
||||
overall_score = report_data['summary']['overall_score']
|
||||
overall_result, overall_color = get_score_result(overall_score/10) # Normalize to 0-10 scale
|
||||
|
|
@ -1163,6 +1172,16 @@ def generate_html_content(report_data, filename, file_path=None):
|
|||
.json-toggle {{ cursor: pointer; color: #FFC407; text-decoration: underline; padding: 15px; text-align: center; font-weight: bold; }}
|
||||
.json-view {{ display: none; margin-top: 20px; }}
|
||||
.json-view pre {{ background-color: #2d3748; color: #e2e8f0; padding: 20px; border-radius: 10px; overflow-x: auto; font-size: 0.9em; }}
|
||||
.findings-table {{ width: 100%; border-collapse: collapse; margin-top: 12px; font-size: 0.92em; }}
|
||||
.findings-table th {{ background: #f1f3f5; color: #495057; text-align: left; padding: 8px 10px; border-bottom: 2px solid #dee2e6; font-weight: 600; }}
|
||||
.findings-table td {{ padding: 8px 10px; border-bottom: 1px solid #e9ecef; vertical-align: top; word-break: break-word; }}
|
||||
.findings-table tr:last-child td {{ border-bottom: none; }}
|
||||
.findings-table code {{ background: #f8f9fa; padding: 2px 5px; border-radius: 4px; font-family: 'SFMono-Regular', Consolas, Menlo, monospace; font-size: 0.9em; color: #c7254e; }}
|
||||
.priority-pill {{ display: inline-block; padding: 3px 8px; border-radius: 10px; color: white; font-weight: 600; font-size: 0.78em; letter-spacing: 0.03em; }}
|
||||
.priority-high {{ background-color: #dc3545; }}
|
||||
.priority-medium {{ background-color: #fd7e14; }}
|
||||
.priority-low {{ background-color: #28a745; }}
|
||||
.muted {{ color: #6c757d; font-size: 0.9em; }}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
|
|
@ -1256,6 +1275,45 @@ def generate_html_response(report_data, filename, save_to_file=False, session_id
|
|||
else:
|
||||
return Response(html_content, mimetype='text/html')
|
||||
|
||||
def _render_findings_table(findings):
|
||||
"""Render an hp_copy_review-style findings array as an HTML table.
|
||||
|
||||
Each finding dict is expected to carry: priority (high|medium|low),
|
||||
category, quote, issue, suggested_fix, source_reference. All string
|
||||
fields are HTML-escaped before interpolation. An empty/None findings
|
||||
list renders a friendly "clean copy" note instead of an empty table.
|
||||
"""
|
||||
if not findings:
|
||||
return '<p class="muted">No findings — copy is clean.</p>'
|
||||
rows = []
|
||||
for f in findings:
|
||||
priority = (f.get('priority') or 'low').lower()
|
||||
pri_class = {
|
||||
'high': 'priority-high',
|
||||
'medium': 'priority-medium',
|
||||
'low': 'priority-low',
|
||||
}.get(priority, 'priority-low')
|
||||
quote_raw = (f.get('quote') or '')[:200]
|
||||
rows.append(
|
||||
'<tr>'
|
||||
f'<td><span class="priority-pill {pri_class}">{html.escape(priority.upper())}</span></td>'
|
||||
f'<td><code>{html.escape(f.get("category", "") or "")}</code></td>'
|
||||
f'<td><code>{html.escape(quote_raw)}</code></td>'
|
||||
f'<td>{html.escape(f.get("issue", "") or "")}</td>'
|
||||
f'<td>{html.escape(f.get("suggested_fix", "") or "")}</td>'
|
||||
f'<td class="muted">{html.escape(f.get("source_reference", "") or "")}</td>'
|
||||
'</tr>'
|
||||
)
|
||||
return (
|
||||
'<table class="findings-table"><thead><tr>'
|
||||
'<th>Priority</th><th>Category</th><th>Quote</th>'
|
||||
'<th>Issue</th><th>Suggested fix</th><th>Source</th>'
|
||||
'</tr></thead><tbody>'
|
||||
+ ''.join(rows) +
|
||||
'</tbody></table>'
|
||||
)
|
||||
|
||||
|
||||
def _render_technical_section_html(report):
|
||||
"""Render the technical pre-flight report as an HTML block. Empty string if no report."""
|
||||
if not report or report.get('kind') in (None, 'unknown'):
|
||||
|
|
@ -1340,7 +1398,14 @@ def generate_comprehensive_html_report(analysis_result, filename, file_path=None
|
|||
score_color = '#28a745' if score >= 6 else '#dc3545'
|
||||
response = result.get('response', 'No response available')
|
||||
display_name = check_name.replace('_', ' ').replace(chr(32).join([w.capitalize() for w in check_name.split('_')]), check_name.replace('_', ' ').title())
|
||||
|
||||
|
||||
# Structured findings (e.g. hp_copy_review) render as a table
|
||||
# instead of the default response-text block. Fallback to the
|
||||
# existing response rendering when 'findings' is absent.
|
||||
json_data = result.get('json_data') if isinstance(result, dict) else None
|
||||
findings = json_data.get('findings') if isinstance(json_data, dict) else None
|
||||
findings_html = _render_findings_table(findings) if findings is not None else None
|
||||
|
||||
# Remove JSON blocks for cleaner display and handle empty responses
|
||||
response = re.sub(r'```json.*?```', '', response, flags=re.DOTALL).strip()
|
||||
if not response:
|
||||
|
|
@ -1367,7 +1432,7 @@ def generate_comprehensive_html_report(analysis_result, filename, file_path=None
|
|||
</div>
|
||||
<div class="analysis-section">
|
||||
<h4>Analysis Details:</h4>
|
||||
<div class="response-text">{response.replace(chr(10), '<br>')}</div>
|
||||
{f'<div class="response-text">{html.escape(json_data.get("summary", "") or "") if isinstance(json_data, dict) else ""}</div>{findings_html}' if findings_html is not None else f'<div class="response-text">{response.replace(chr(10), "<br>")}</div>'}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
|
@ -1421,6 +1486,16 @@ def generate_comprehensive_html_report(analysis_result, filename, file_path=None
|
|||
.check-metadata {{ background: #f8f9fa; padding: 15px; border-radius: 8px; margin-bottom: 15px; }}
|
||||
.analysis-section h4 {{ color: #495057; margin-bottom: 10px; }}
|
||||
.response-text {{ background: #f8f9fa; padding: 15px; border-radius: 8px; line-height: 1.6; font-family: 'Montserrat', Georgia, serif; }}
|
||||
.findings-table {{ width: 100%; border-collapse: collapse; margin-top: 12px; font-size: 0.92em; }}
|
||||
.findings-table th {{ background: #f1f3f5; color: #495057; text-align: left; padding: 8px 10px; border-bottom: 2px solid #dee2e6; font-weight: 600; }}
|
||||
.findings-table td {{ padding: 8px 10px; border-bottom: 1px solid #e9ecef; vertical-align: top; word-break: break-word; }}
|
||||
.findings-table tr:last-child td {{ border-bottom: none; }}
|
||||
.findings-table code {{ background: #f8f9fa; padding: 2px 5px; border-radius: 4px; font-family: 'SFMono-Regular', Consolas, Menlo, monospace; font-size: 0.9em; color: #c7254e; }}
|
||||
.priority-pill {{ display: inline-block; padding: 3px 8px; border-radius: 10px; color: white; font-weight: 600; font-size: 0.78em; letter-spacing: 0.03em; }}
|
||||
.priority-high {{ background-color: #dc3545; }}
|
||||
.priority-medium {{ background-color: #fd7e14; }}
|
||||
.priority-low {{ background-color: #28a745; }}
|
||||
.muted {{ color: #6c757d; font-size: 0.9em; }}
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
|
|
@ -1632,6 +1707,15 @@ def get_reference_asset_content(reference_asset_id):
|
|||
reference_content += f"\nLocalization Matrix: Contains {', '.join(loc_messages)} "
|
||||
reference_content += f"for {len(loc_countries)} markets ({', '.join(loc_countries[:10])}).\n"
|
||||
reference_content += "Expected copy will be cross-referenced with the media plan during analysis.\n"
|
||||
elif file_record.get('summary_path'):
|
||||
# Source-messaging Excel (HP and similar) — inject the Gemini-generated Markdown summary
|
||||
try:
|
||||
with open(file_record['summary_path'], 'r', encoding='utf-8') as f:
|
||||
summary = f.read()
|
||||
reference_content += f"\nSource Messaging Summary (extracted from {original_filename}):\n{summary}\n"
|
||||
except Exception as e:
|
||||
print(f"Failed to read source-messaging summary at {file_record['summary_path']}: {e}")
|
||||
reference_content += f"\nReference file ({file_ext}) uploaded but summary unreadable.\n"
|
||||
else:
|
||||
reference_content += f"\nReference file ({file_ext}) uploaded as reference.\n"
|
||||
else:
|
||||
|
|
@ -4832,15 +4916,15 @@ def upload_brand_guideline():
|
|||
).start()
|
||||
file_record['processing_status'] = 'processing'
|
||||
|
||||
# Trigger localization matrix parsing for Excel files
|
||||
# Trigger Excel processing: try localization matrix first (existing
|
||||
# clients), fall back to Source Messaging summary (HP and similar).
|
||||
elif file_record.get('file_type') in ('.xlsx', '.xls'):
|
||||
import threading
|
||||
def _process_localization_bg(fid, spath, fdir):
|
||||
def _process_excel_bg(fid, spath, fdir):
|
||||
try:
|
||||
from localization_processor import parse_localization_matrix
|
||||
parsed = parse_localization_matrix(spath)
|
||||
if parsed:
|
||||
# Save parsed JSON
|
||||
json_path = os.path.join(fdir, f"{fid}_localization.json")
|
||||
with open(json_path, 'w', encoding='utf-8') as f:
|
||||
json.dump(parsed, f, indent=2, ensure_ascii=False)
|
||||
|
|
@ -4855,25 +4939,34 @@ def upload_brand_guideline():
|
|||
print(f"Localization matrix parsing complete for {fid}: "
|
||||
f"{len(parsed.get('messages', {}))} messages, "
|
||||
f"{len(parsed.get('countries', []))} countries")
|
||||
else:
|
||||
brand_db.update_file_record(fid, {
|
||||
'processed': True,
|
||||
'processed_at': datetime.now().isoformat(),
|
||||
'asset_type': 'excel_file',
|
||||
})
|
||||
print(f"Excel file {fid} is not a localization matrix, stored as-is")
|
||||
return
|
||||
|
||||
# Not a localization matrix — process as Source Messaging
|
||||
# (HP-style structured Markdown summary via Gemini).
|
||||
from excel_processor import process_excel_file
|
||||
summary_text, summary_path = process_excel_file(spath, fid)
|
||||
brand_db.update_file_record(fid, {
|
||||
'processed': True,
|
||||
'processed_at': datetime.now().isoformat(),
|
||||
'summary_path': summary_path,
|
||||
'summary_length': len(summary_text),
|
||||
'cover_image_path': None,
|
||||
'asset_type': 'source_messaging',
|
||||
})
|
||||
print(f"Source-messaging summary complete for {fid}: "
|
||||
f"{len(summary_text)} chars")
|
||||
except Exception as e:
|
||||
print(f"Localization matrix parsing failed for {fid}: {e}")
|
||||
print(f"Excel processing failed for {fid}: {e}")
|
||||
brand_db.update_file_record(fid, {
|
||||
'processed': 'error',
|
||||
'processing_error': str(e)
|
||||
'processing_error': str(e),
|
||||
})
|
||||
|
||||
threading.Thread(
|
||||
target=_process_localization_bg,
|
||||
target=_process_excel_bg,
|
||||
args=(file_record['id'], file_record['stored_path'],
|
||||
str(brand_db.files_dir)),
|
||||
daemon=True
|
||||
daemon=True,
|
||||
).start()
|
||||
file_record['processing_status'] = 'processing'
|
||||
|
||||
|
|
|
|||
|
|
@ -63,9 +63,10 @@ CLIENT_PROFILES = {
|
|||
},
|
||||
'hp': {
|
||||
'name': 'HP',
|
||||
'profiles': ['static_general', 'video_general'],
|
||||
'profiles': ['hp_copy_review', 'static_general', 'video_general'],
|
||||
'display_name': 'HP',
|
||||
'description': 'Demo client — scope pending'
|
||||
'description': 'HP marketing copy QC graded against canonical Source Messaging',
|
||||
'default_profile': 'hp_copy_review',
|
||||
},
|
||||
'ferrero': {
|
||||
'name': 'Ferrero',
|
||||
|
|
|
|||
|
|
@ -1,10 +0,0 @@
|
|||
OPENAI_API_KEY=sk-svcacct-HSREzGYDnN-vCVGAh6LhYqlNcJVF2oefMrY9oCsdDsQFmyVJyHpLb1eSb_mp_vP4YPl4T3BlbkFJzKaOrPghIzx76_22K8VjwO6j2JnoDEvrYDrgfrnA4WjD5sTMnhOqGHXximwGXFhUoYgA
|
||||
GOOGLE_API_KEY=AIzaSyDMWN_PAnyU7bPmtWcEKq4LJfiu1KuwUsU
|
||||
|
||||
# Azure AD / MSAL Authentication Configuration
|
||||
AZURE_TENANT_ID=e519c2e6-bc6d-4fdf-8d9c-923c2f002385
|
||||
AZURE_CLIENT_ID=9079054c-9620-4757-a256-23413042f1ef
|
||||
|
||||
# Flask Security Configuration
|
||||
FLASK_ENV=development
|
||||
SECRET_KEY=your-secret-key-here-change-in-production
|
||||
|
|
@ -15,12 +15,20 @@ Document-scope checks bypass the LLM pipeline entirely (deterministic, $0).
|
|||
Page-level checks plug into `process_checks_in_batches()` exactly as before.
|
||||
"""
|
||||
|
||||
from concurrent.futures import ThreadPoolExecutor, as_completed
|
||||
from datetime import datetime
|
||||
from typing import Callable, Dict, List, Optional
|
||||
|
||||
from . import checks as doc_checks
|
||||
|
||||
|
||||
# Max concurrent page-level LLM calls within a single check, used by Stage 3c
|
||||
# (page_sample) and Stage 3d (page_each). Was sequential; that pinned a 4-page
|
||||
# × 7-check Boots PPack run at ~15 min. Bump if larger docs / paid-tier rate
|
||||
# limits make it safe; keep modest to stay well under Gemini's per-key quota.
|
||||
_PAGE_PARALLEL_WORKERS = 4
|
||||
|
||||
|
||||
def _grade(overall_score: float) -> str:
|
||||
"""Same Pass/Fail rule as single-asset mode: avg per-check ≥ 6 = Pass."""
|
||||
avg_individual = overall_score / 10
|
||||
|
|
@ -199,6 +207,47 @@ def run_document_analysis(
|
|||
'percentage': 12 + (completed / len(enabled_checks)) * 80,
|
||||
})
|
||||
|
||||
# ── Page-type map (shared by 3c page_sample and 3d page_each) ──────────
|
||||
# For profiles that don't tag pages (e.g. AXA), every page is 'artwork',
|
||||
# so the page-type-aware aggregation in Stage 3d falls through cleanly.
|
||||
page_type_map = {p['page_num']: p.get('page_type', 'artwork') for p in pages}
|
||||
artwork_page_nums = {pn for pn, pt in page_type_map.items() if pt == 'artwork'}
|
||||
|
||||
# ── Per-page dispatch helper (used by 3c and 3d in parallel) ───────────
|
||||
# process_checks_in_batches is already reentrant — asset-mode runs it
|
||||
# under its own ThreadPoolExecutor — so it's safe to call concurrently
|
||||
# from a pool. progress_tracker writes are GIL-safe and racy by design
|
||||
# (visual only). page_level_results writes happen from the main thread
|
||||
# after future.result(), so no races on that dict either.
|
||||
def _run_check_on_page(check_name: str, page: Dict):
|
||||
page_num = page['page_num']
|
||||
try:
|
||||
page_check_results = process_checks_in_batches(
|
||||
enabled_checks=[check_name],
|
||||
qc_apps=qc_apps,
|
||||
profile_config=profile_config,
|
||||
profile_weights=profile_weights,
|
||||
file_path=page['image_path'],
|
||||
analysis_reference_asset=analysis_reference_asset,
|
||||
brand_db=brand_db,
|
||||
progress_tracker=progress_tracker,
|
||||
session_id=session_id,
|
||||
batch_size=15,
|
||||
media_plan_context=media_plan_context,
|
||||
ocr_context=ocr_context,
|
||||
)
|
||||
result_for_page = page_check_results.get(check_name, {})
|
||||
except Exception as e:
|
||||
result_for_page = {
|
||||
'check_name': check_name,
|
||||
'score': 0.0,
|
||||
'pass': False,
|
||||
'response': f'Check raised {type(e).__name__}: {e}',
|
||||
'findings': {'error': str(e)},
|
||||
}
|
||||
result_for_page['page_type'] = page_type_map.get(page_num, 'artwork')
|
||||
return page_num, result_for_page
|
||||
|
||||
# ── Stage 3c: page-sample (LLM, sampled pages) ────────────────────────
|
||||
page_level_results: Dict[str, Dict[int, Dict]] = {} # check → page_num → result
|
||||
sample_buckets = scope_buckets['page_sample']
|
||||
|
|
@ -206,30 +255,24 @@ def run_document_analysis(
|
|||
for check_name, scope_args in sample_buckets:
|
||||
n = (scope_args or {}).get('sample_size', 8)
|
||||
page_nums = _evenly_spaced(pages_processed, n)
|
||||
eligible = [pages[pn - 1] for pn in page_nums if pages[pn - 1].get('image_path')]
|
||||
page_level_results[check_name] = {}
|
||||
for page_num in page_nums:
|
||||
page = pages[page_num - 1]
|
||||
if not page.get('image_path'):
|
||||
continue
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name} on page {page_num} (sample)',
|
||||
'current_page': page_num,
|
||||
})
|
||||
page_check_results = process_checks_in_batches(
|
||||
enabled_checks=[check_name],
|
||||
qc_apps=qc_apps,
|
||||
profile_config=profile_config,
|
||||
profile_weights=profile_weights,
|
||||
file_path=page['image_path'],
|
||||
analysis_reference_asset=analysis_reference_asset,
|
||||
brand_db=brand_db,
|
||||
progress_tracker=progress_tracker,
|
||||
session_id=session_id,
|
||||
batch_size=15,
|
||||
media_plan_context=media_plan_context,
|
||||
ocr_context=ocr_context,
|
||||
)
|
||||
page_level_results[check_name][page_num] = page_check_results.get(check_name, {})
|
||||
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name} (sampling {len(eligible)} pages)...',
|
||||
})
|
||||
|
||||
pages_done = 0
|
||||
with ThreadPoolExecutor(max_workers=_PAGE_PARALLEL_WORKERS) as pool:
|
||||
futures = [pool.submit(_run_check_on_page, check_name, p) for p in eligible]
|
||||
for fut in as_completed(futures):
|
||||
pn, result = fut.result()
|
||||
page_level_results[check_name][pn] = result
|
||||
pages_done += 1
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name}: {pages_done} of {len(eligible)} pages',
|
||||
})
|
||||
|
||||
# Aggregate the sampled results into the doc-level entry
|
||||
page_scores = {p: (r.get('score') or 0) for p, r in page_level_results[check_name].items()}
|
||||
scores = list(page_scores.values())
|
||||
|
|
@ -262,39 +305,27 @@ def run_document_analysis(
|
|||
# average that drives the headline score & grade. This implements the
|
||||
# strict-grade exemption requested for Boots Production Packs without
|
||||
# changing AXA-style profiles (which don't tag pages → all pages count).
|
||||
page_type_map = {p['page_num']: p.get('page_type', 'artwork') for p in pages}
|
||||
artwork_page_nums = {pn for pn, pt in page_type_map.items() if pt == 'artwork'}
|
||||
|
||||
if scope_buckets['page_each']:
|
||||
for check_name, _scope_args in scope_buckets['page_each']:
|
||||
page_level_results.setdefault(check_name, {})
|
||||
for page in pages:
|
||||
page_num = page['page_num']
|
||||
if not page.get('image_path'):
|
||||
continue
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name} on page {page_num}',
|
||||
'current_page': page_num,
|
||||
})
|
||||
page_check_results = process_checks_in_batches(
|
||||
enabled_checks=[check_name],
|
||||
qc_apps=qc_apps,
|
||||
profile_config=profile_config,
|
||||
profile_weights=profile_weights,
|
||||
file_path=page['image_path'],
|
||||
analysis_reference_asset=analysis_reference_asset,
|
||||
brand_db=brand_db,
|
||||
progress_tracker=progress_tracker,
|
||||
session_id=session_id,
|
||||
batch_size=15,
|
||||
media_plan_context=media_plan_context,
|
||||
ocr_context=ocr_context,
|
||||
)
|
||||
result_for_page = page_check_results.get(check_name, {})
|
||||
# Tag the per-page result with its page_type so the report
|
||||
# writer can group results by page category.
|
||||
result_for_page['page_type'] = page_type_map.get(page_num, 'artwork')
|
||||
page_level_results[check_name][page_num] = result_for_page
|
||||
eligible_pages = [p for p in pages if p.get('image_path')]
|
||||
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name} across {len(eligible_pages)} pages...',
|
||||
'current_page': 0,
|
||||
})
|
||||
|
||||
pages_done = 0
|
||||
with ThreadPoolExecutor(max_workers=_PAGE_PARALLEL_WORKERS) as pool:
|
||||
futures = [pool.submit(_run_check_on_page, check_name, p) for p in eligible_pages]
|
||||
for fut in as_completed(futures):
|
||||
pn, result = fut.result()
|
||||
page_level_results[check_name][pn] = result
|
||||
pages_done += 1
|
||||
progress_tracker[session_id].update({
|
||||
'current_check_display': f'{check_name}: {pages_done} of {len(eligible_pages)} pages',
|
||||
})
|
||||
|
||||
page_scores = {p: (r.get('score') or 0) for p, r in page_level_results[check_name].items()}
|
||||
artwork_scores = {p: s for p, s in page_scores.items() if p in artwork_page_nums}
|
||||
|
|
|
|||
162
backend/excel_processor.py
Normal file
162
backend/excel_processor.py
Normal file
|
|
@ -0,0 +1,162 @@
|
|||
#!/usr/bin/env python3
|
||||
"""Excel reference-asset processor for HP Source Messaging files.
|
||||
|
||||
Mirrors pdf_processor.py: openpyxl extracts raw cell content from every
|
||||
sheet, Gemini 2.5 Pro summarises the result into structured Markdown
|
||||
under brand_guidelines/files/{file_id}_summary.md. The hp_copy_review
|
||||
check pulls that Markdown into its prompt at QC time.
|
||||
|
||||
Public surface:
|
||||
process_excel_file(file_path, file_id) -> (summary_text, summary_path)
|
||||
|
||||
Behaviour mirrors pdf_processor.summarize_brand_guidelines: on Gemini
|
||||
failure we write a degraded summary containing the raw extraction so
|
||||
the reference asset stays usable downstream. The function does not
|
||||
raise — failures are logged and surfaced via the degraded payload.
|
||||
"""
|
||||
|
||||
import os
|
||||
from typing import Tuple
|
||||
|
||||
from openpyxl import load_workbook
|
||||
|
||||
|
||||
BRAND_GUIDELINES_DIR = os.path.join(
|
||||
os.path.dirname(os.path.abspath(__file__)), 'brand_guidelines', 'files'
|
||||
)
|
||||
|
||||
# Cap raw extraction at ~50K chars to keep the summary prompt bounded.
|
||||
# A 30-row, 12-column workbook is ~10-15K chars in practice; this leaves
|
||||
# headroom for HP's larger source files without blowing the prompt budget.
|
||||
_RAW_EXTRACTION_CAP = 50_000
|
||||
|
||||
|
||||
_SYSTEM_PROMPT = """You're processing an HP Source Messaging Excel into a structured Markdown reference. Output these sections exactly, in this order:
|
||||
|
||||
## Product / Variant
|
||||
(brand, product line, variant if any — e.g. "HP OmniDesk Mini — Core")
|
||||
|
||||
## Key Selling Points (KSPs)
|
||||
For each KSP: heading, value proposition, supporting body copy, message-length variants (ultra-short / short / medium / long if present in the source).
|
||||
|
||||
## Disclaimers / Footnotes
|
||||
Numbered list, exact wording, what claim each footnote anchors to.
|
||||
|
||||
## Approved Brand and Product Names
|
||||
Exact spellings, including trademark glyphs (™, ®, ©).
|
||||
|
||||
## Variant Notes / Watch-outs
|
||||
Anything explicitly marked variant-specific (e.g. "Mainstream only", "Core only", "must not appear in entry tier").
|
||||
|
||||
## Verboten Phrasing
|
||||
Any explicitly disallowed or deprecated phrasing called out in the source.
|
||||
|
||||
Be exhaustive but concise. Quote exactly where the source is explicit. If a section has no content in this source, write 'None specified' under it — do not omit the section heading."""
|
||||
|
||||
|
||||
def process_excel_file(file_path: str, file_id: str) -> Tuple[str, str]:
|
||||
"""Extract + summarise an HP Source Messaging Excel.
|
||||
|
||||
Args:
|
||||
file_path: Path to the .xlsx file on disk.
|
||||
file_id: Stable identifier used for the output filename.
|
||||
|
||||
Returns:
|
||||
Tuple of (summary_text, summary_path). Summary is written to
|
||||
BRAND_GUIDELINES_DIR/{file_id}_summary.md.
|
||||
|
||||
Never raises. On Gemini failure, writes a degraded summary that
|
||||
embeds the raw extraction so the reference asset stays usable.
|
||||
"""
|
||||
try:
|
||||
raw_text = _extract_workbook_text(file_path)
|
||||
except Exception as e:
|
||||
print(f" Excel extraction failed for {file_id}: {type(e).__name__}: {e}")
|
||||
summary = (
|
||||
f"# {os.path.basename(file_path)} (degraded — extraction failed)\n\n"
|
||||
f"openpyxl extraction failed: {type(e).__name__}: {e}\n"
|
||||
)
|
||||
raw_text = ''
|
||||
else:
|
||||
try:
|
||||
summary = _summarise_with_gemini(raw_text, os.path.basename(file_path))
|
||||
except Exception as e:
|
||||
print(f" Gemini summarisation failed for {file_id}: {type(e).__name__}: {e}")
|
||||
summary = (
|
||||
f"# {os.path.basename(file_path)} (degraded — summary failed)\n\n"
|
||||
f"Gemini summarisation failed: {type(e).__name__}: {e}\n\n"
|
||||
f"## Raw extraction\n\n```\n{raw_text}\n```\n"
|
||||
)
|
||||
|
||||
os.makedirs(BRAND_GUIDELINES_DIR, exist_ok=True)
|
||||
summary_path = os.path.join(BRAND_GUIDELINES_DIR, f"{file_id}_summary.md")
|
||||
with open(summary_path, 'w', encoding='utf-8') as f:
|
||||
f.write(summary)
|
||||
return summary, summary_path
|
||||
|
||||
|
||||
def _extract_workbook_text(file_path: str) -> str:
|
||||
"""Read every sheet, dump as 'Sheet: <name>\\n<tab-aligned rows>\\n\\n'.
|
||||
|
||||
Empty rows are skipped. Output is capped at _RAW_EXTRACTION_CAP chars;
|
||||
when exceeded, a truncation marker is appended and the rest is dropped.
|
||||
"""
|
||||
wb = load_workbook(file_path, data_only=True, read_only=True)
|
||||
try:
|
||||
parts = []
|
||||
total_chars = 0
|
||||
for sheet in wb.worksheets:
|
||||
header = f"Sheet: {sheet.title}\n"
|
||||
parts.append(header)
|
||||
total_chars += len(header)
|
||||
for row in sheet.iter_rows(values_only=True):
|
||||
if not any((c is not None and str(c).strip()) for c in row):
|
||||
continue
|
||||
line = '\t'.join(('' if c is None else str(c)) for c in row)
|
||||
parts.append(line + '\n')
|
||||
total_chars += len(line) + 1
|
||||
if total_chars >= _RAW_EXTRACTION_CAP:
|
||||
parts.append(
|
||||
f"\n[truncated — exceeded {_RAW_EXTRACTION_CAP}-char cap]\n"
|
||||
)
|
||||
return ''.join(parts)
|
||||
parts.append('\n')
|
||||
total_chars += 1
|
||||
return ''.join(parts)
|
||||
finally:
|
||||
wb.close()
|
||||
|
||||
|
||||
def _summarise_with_gemini(raw_text: str, source_filename: str) -> str:
|
||||
"""Send the extracted workbook text to Gemini 2.5 Pro for summarisation.
|
||||
|
||||
Mirrors pdf_processor.summarize_brand_guidelines: uses
|
||||
google.generativeai directly with MODEL_VERSIONS.gemini_vision
|
||||
(currently gemini-2.5-pro). Raises on any failure; the caller
|
||||
converts failures into a degraded summary.
|
||||
"""
|
||||
import google.generativeai as genai
|
||||
from llm_config import MODEL_VERSIONS
|
||||
|
||||
api_key = os.getenv("GOOGLE_API_KEY")
|
||||
if not api_key:
|
||||
raise RuntimeError("GOOGLE_API_KEY not configured")
|
||||
|
||||
genai.configure(api_key=api_key)
|
||||
model = genai.GenerativeModel(MODEL_VERSIONS.gemini_vision)
|
||||
|
||||
prompt = (
|
||||
f"{_SYSTEM_PROMPT}\n\n"
|
||||
f"Source filename: {source_filename}\n\n"
|
||||
f"Raw cell content:\n\n```\n{raw_text}\n```"
|
||||
)
|
||||
response = model.generate_content(prompt)
|
||||
|
||||
# Mirror pdf_processor's safety-block handling: surface a useful error.
|
||||
if not getattr(response, 'parts', None):
|
||||
feedback = getattr(response, 'prompt_feedback', 'No specific feedback provided.')
|
||||
raise RuntimeError(
|
||||
f"Gemini response blocked or empty. Feedback: {feedback}"
|
||||
)
|
||||
|
||||
return response.text
|
||||
14
backend/profiles/hp_copy_review.json
Normal file
14
backend/profiles/hp_copy_review.json
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
{
|
||||
"name": "HP Copy Review",
|
||||
"description": "Marketing copy graded against canonical HP Source Messaging",
|
||||
"mode": "asset",
|
||||
"visibility": "client_specific",
|
||||
"visible_to_clients": ["hp"],
|
||||
"checks": {
|
||||
"hp_copy_review": {
|
||||
"weight": 10.0,
|
||||
"llm": "Gemini",
|
||||
"enabled": true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -96,8 +96,13 @@ fi
|
|||
echo "Target: $TARGET_SHORT $(git log -1 --format='%s' "$TARGET_REF")"
|
||||
echo ""
|
||||
echo "Commits to apply:"
|
||||
git log --oneline "$CURRENT_REV..$TARGET_REV" | head -20
|
||||
CHANGE_COUNT=$(git log --oneline "$CURRENT_REV..$TARGET_REV" | wc -l | tr -d ' ')
|
||||
# Use git's own line limit (`-n 20`) rather than `| head -20`: piping to head
|
||||
# closes the pipe after 20 lines and makes git log exit with SIGPIPE (141),
|
||||
# which `set -o pipefail` propagates and `set -e` then uses to kill the
|
||||
# script silently. Only bites when the deploy batch is >20 commits — i.e.
|
||||
# real prod releases. First hit observed on the v1.3.0 prod deploy.
|
||||
git log --oneline -n 20 "$CURRENT_REV..$TARGET_REV"
|
||||
CHANGE_COUNT=$(git rev-list --count "$CURRENT_REV..$TARGET_REV")
|
||||
if [[ $CHANGE_COUNT -gt 20 ]]; then
|
||||
echo " ... and $((CHANGE_COUNT - 20)) more"
|
||||
fi
|
||||
|
|
|
|||
0
backend/visual_qc_apps/hp_copy_review/__init__.py
Normal file
0
backend/visual_qc_apps/hp_copy_review/__init__.py
Normal file
179
backend/visual_qc_apps/hp_copy_review/app.py
Normal file
179
backend/visual_qc_apps/hp_copy_review/app.py
Normal file
|
|
@ -0,0 +1,179 @@
|
|||
"""HP Copy Review — single-call LLM grader against canonical Source Messaging.
|
||||
|
||||
This check compares all visible copy on an HP marketing asset (claims,
|
||||
headlines, body, disclaimers, footnotes, spec call-outs, brand mentions)
|
||||
against the canonical Source Messaging summaries attached as reference
|
||||
assets (.xlsx → Markdown summary via excel_processor).
|
||||
|
||||
It returns a structured JSON object with a 0-10 score, a one-paragraph
|
||||
summary, and a `findings` array (priority / category / quote / issue /
|
||||
suggested_fix / source_reference). Empty findings on a clean asset is a
|
||||
valid result (score 9-10). When no Source Messaging is attached, the
|
||||
LLM is instructed to return score 0 with an explanatory message rather
|
||||
than grade blind.
|
||||
|
||||
Reference assets and media-plan context (including `language`) are
|
||||
injected by `process_single_check` in `api_server.py` — this module
|
||||
exposes only the static prompt template. A standalone `build_prompt()`
|
||||
helper is provided for unit-style smoke tests and for any future caller
|
||||
that wants to assemble the full prompt outside the production path.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from typing import Iterable, Mapping, Optional, Sequence, Tuple
|
||||
|
||||
# Add parent directory to path so we can import shared template
|
||||
sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
|
||||
|
||||
from visual_qc_apps.flask_app_template import FlaskAppTemplate
|
||||
|
||||
|
||||
# --- Canonical prompt template ------------------------------------------------
|
||||
#
|
||||
# The reference-asset summary block ("CANONICAL SOURCE MESSAGING") is
|
||||
# prepended by `process_single_check` in `api_server.py` via
|
||||
# `get_reference_asset_content()`. Likewise the media-plan context block
|
||||
# ("=== MEDIA PLAN CONTEXT ===" with `- Language: <value>`) is appended
|
||||
# by `process_single_check`. We embed instructions that *reference* both
|
||||
# blocks so the LLM knows where to look.
|
||||
|
||||
HP_COPY_REVIEW_PROMPT = """You are a copy reviewer for HP marketing materials. Your job is to compare the marketing asset against the canonical Source Messaging that has been attached as a reference asset, and report every copy discrepancy as a structured finding.
|
||||
|
||||
WHAT YOU WILL BE GIVEN:
|
||||
1. One or more canonical Source Messaging summaries, attached above as REFERENCE ASSET GUIDELINES. Each Source Messaging file (e.g. `messi_core.xlsx`, `messi_mainstream.xlsx`) has been pre-summarised into Markdown and is the single source of truth for product claims, KSPs, disclaimers, spec call-outs, variant naming, and approved tone.
|
||||
2. A media-plan context block (appended below the prompt) which may include `- Language: <value>` and `- Country: <value>`. Treat the language value as the PRODUCT LANGUAGE the asset should be using (e.g. "UK English", "US English", "French (France)").
|
||||
3. The marketing asset image itself.
|
||||
|
||||
WHAT TO DO:
|
||||
For every claim, headline, body line, disclaimer, footnote, spec call-out, and brand mention visible on the asset, evaluate it against the canonical Source Messaging. Flag:
|
||||
- Wording that disagrees with an approved KSP or claim.
|
||||
- Missing or incorrect mandatory disclaimers / legal footnotes / asterisked notes.
|
||||
- Spec call-outs that contradict the canonical spec (wrong number, wrong unit, wrong product variant).
|
||||
- Variant / product-name errors (e.g. "OmniDesk Mini" vs "OmniDesk Mini Core").
|
||||
- Tone / phrasing drift from the approved brand voice described in the source.
|
||||
- Brand-name misuse (HP, sub-brand capitalisation, trademark glyph misuse).
|
||||
- Language / locale mismatch against the media-plan PRODUCT LANGUAGE (e.g. "color" appearing in a UK English asset, or French copy on an asset specified as US English).
|
||||
|
||||
OUTPUT — return ONE JSON object, and nothing else (no prose, no markdown fences outside the JSON code block). The shape:
|
||||
|
||||
```json
|
||||
{
|
||||
"score": <number 0-10>,
|
||||
"summary": "<one-paragraph headline finding>",
|
||||
"findings": [
|
||||
{
|
||||
"priority": "high" | "medium" | "low",
|
||||
"category": "ksp" | "disclaimer" | "spec" | "variant" | "tone" | "brand-name" | "language" | "other",
|
||||
"quote": "<exact quote from the asset>",
|
||||
"issue": "<what's wrong>",
|
||||
"suggested_fix": "<what it should say, citing the canonical source>",
|
||||
"source_reference": "<where in the source messaging this comes from, e.g. file name + section heading>"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
RULES:
|
||||
- If no Source Messaging reference asset is attached (i.e. there is no "REFERENCE ASSET GUIDELINES" block above describing canonical HP messaging), return EXACTLY:
|
||||
{"score": 0, "summary": "No HP Source Messaging reference was attached — cannot grade copy without a canonical source.", "findings": []}
|
||||
Do not attempt to grade copy from prior knowledge.
|
||||
- High-priority findings (factually-wrong claims, missing mandatory disclaimers, wrong product variant, wrong language) weight the score most heavily. A single high-priority finding should typically pull the score below 6.
|
||||
- Medium-priority findings are wording drift that changes nuance but not meaning, or missing optional supporting copy.
|
||||
- Low-priority findings are tone / style nits.
|
||||
- An empty `findings` array is a valid and expected result for a clean asset — in that case score 9 or 10 and write a short, positive summary.
|
||||
- The `quote` field must be the EXACT visible text from the asset, including punctuation. If you can read it, quote it.
|
||||
- `source_reference` should make it easy for a reviewer to verify the finding — name the Source Messaging file and the section/heading you matched against.
|
||||
- Return ONLY the JSON object inside a single ```json ... ``` code block. No surrounding prose, no explanations outside the JSON.
|
||||
"""
|
||||
|
||||
|
||||
def build_prompt(
|
||||
reference_summaries: Optional[Sequence[Tuple[str, str]]] = None,
|
||||
media_plan_row: Optional[Mapping[str, str]] = None,
|
||||
base_prompt: str = HP_COPY_REVIEW_PROMPT,
|
||||
) -> str:
|
||||
"""Assemble a fully-rendered HP copy-review prompt for testing / inspection.
|
||||
|
||||
In production, `process_single_check` (api_server.py) does this
|
||||
assembly itself: it prepends `get_reference_asset_content(...)` and
|
||||
appends `build_media_plan_context(...)`. This helper mirrors that
|
||||
flow so we can smoke-test the prompt assembly without running the
|
||||
full server, and so callers that want to render the exact prompt
|
||||
text for logging / debugging have a single entry point.
|
||||
|
||||
Args:
|
||||
reference_summaries: List of (filename, markdown_summary) tuples,
|
||||
one per attached Source Messaging .xlsx. Each summary is
|
||||
already a Markdown string produced by `excel_processor`.
|
||||
None or [] means "no canonical source attached" — in that
|
||||
case we still build the prompt but omit the canonical block,
|
||||
and the LLM will fall back to the score-0 rule.
|
||||
media_plan_row: Mapping with optional `language`, `country`,
|
||||
`placement`, etc. Only `language` and `country` are
|
||||
rendered into the prompt here; the production flow uses
|
||||
`build_media_plan_context` and includes more fields.
|
||||
base_prompt: Override for the canonical prompt template (used
|
||||
in tests where we want to inject a shorter stub).
|
||||
|
||||
Returns:
|
||||
The fully-assembled prompt string, with the canonical source
|
||||
messaging block (if any) prepended, the media-plan language /
|
||||
country line(s) appended, and the base template in between.
|
||||
"""
|
||||
parts = []
|
||||
|
||||
# 1. Canonical source messaging block — mirrors the shape of
|
||||
# `get_reference_asset_content` so the LLM sees a consistent
|
||||
# "REFERENCE ASSET GUIDELINES" heading whether it's running in
|
||||
# production or via this helper.
|
||||
if reference_summaries:
|
||||
ref_lines = ["\n\n=== REFERENCE ASSET GUIDELINES ===",
|
||||
"CANONICAL SOURCE MESSAGING:"]
|
||||
for filename, summary in reference_summaries:
|
||||
ref_lines.append(f"\n--- File: {filename} ---\n{summary}")
|
||||
ref_lines.append("=== END REFERENCE ASSET GUIDELINES ===\n")
|
||||
parts.append("\n".join(ref_lines))
|
||||
|
||||
# 2. The static prompt template itself.
|
||||
parts.append(base_prompt)
|
||||
|
||||
# 3. Media-plan context (language / country). Production appends
|
||||
# the full `build_media_plan_context` block; here we render just
|
||||
# the language + country fields, which is what Step 5.6 asserts.
|
||||
if media_plan_row:
|
||||
mp_lines = ["\n=== MEDIA PLAN CONTEXT ==="]
|
||||
if media_plan_row.get('language'):
|
||||
mp_lines.append(f"- Language: {media_plan_row['language']}")
|
||||
if media_plan_row.get('country'):
|
||||
mp_lines.append(f"- Country: {media_plan_row['country']}")
|
||||
mp_lines.append("=== END MEDIA PLAN CONTEXT ===")
|
||||
parts.append("\n".join(mp_lines))
|
||||
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
class HpCopyReviewApp(FlaskAppTemplate):
|
||||
"""HP Copy Review — single-call LLM copy grader against Source Messaging.
|
||||
|
||||
Subclasses `FlaskAppTemplate` so the check is auto-discovered by
|
||||
`load_qc_apps()` in `api_server.py`. The class instance exposes
|
||||
`self.prompt` (the canonical template plus the standard scoring
|
||||
instructions appended by the template base class).
|
||||
|
||||
Reference asset summaries and media-plan context are injected at
|
||||
runtime by `process_single_check` — this class does NOT call Gemini
|
||||
directly. Response parsing is handled by
|
||||
`extract_json_from_response` / `extract_score_from_result` in
|
||||
api_server.py, which will lift `score`, `summary`, and `findings`
|
||||
out of the JSON code block returned by the LLM.
|
||||
"""
|
||||
|
||||
def __init__(self):
|
||||
super().__init__(__name__, HP_COPY_REVIEW_PROMPT)
|
||||
|
||||
|
||||
# Allow running this check standalone for ad-hoc testing
|
||||
if __name__ == "__main__":
|
||||
app_instance = HpCopyReviewApp()
|
||||
app_instance.run()
|
||||
10
config.env
10
config.env
|
|
@ -1,10 +0,0 @@
|
|||
OPENAI_API_KEY=sk-svcacct-HSREzGYDnN-vCVGAh6LhYqlNcJVF2oefMrY9oCsdDsQFmyVJyHpLb1eSb_mp_vP4YPl4T3BlbkFJzKaOrPghIzx76_22K8VjwO6j2JnoDEvrYDrgfrnA4WjD5sTMnhOqGHXximwGXFhUoYgA
|
||||
GOOGLE_API_KEY=AIzaSyDMWN_PAnyU7bPmtWcEKq4LJfiu1KuwUsU
|
||||
|
||||
# Azure AD / MSAL Authentication Configuration
|
||||
AZURE_TENANT_ID=e519c2e6-bc6d-4fdf-8d9c-923c2f002385
|
||||
AZURE_CLIENT_ID=9079054c-9620-4757-a256-23413042f1ef
|
||||
|
||||
# Flask Security Configuration
|
||||
FLASK_ENV=development
|
||||
SECRET_KEY=your-secret-key-here-change-in-production
|
||||
|
|
@ -1,43 +0,0 @@
|
|||
# Development Environment Configuration
|
||||
# This file is used for local development testing
|
||||
|
||||
# OpenAI Configuration
|
||||
OPENAI_API_KEY=sk-svcacct-HSREzGYDnN-vCVGAh6LhYqlNcJVF2oefMrY9oCsdDsQFmyVJyHpLb1eSb_mp_vP4YPl4T3BlbkFJzKaOrPghIzx76_22K8VjwO6j2JnoDEvrYDrgfrnA4WjD5sTMnhOqGHXximwGXFhUoYgA
|
||||
GOOGLE_API_KEY=AIzaSyDMWN_PAnyU7bPmtWcEKq4LJfiu1KuwUsU
|
||||
|
||||
# Azure AD / MSAL Authentication Configuration (Development App Registration)
|
||||
# NOTE: You'll need to create a separate app registration for development
|
||||
AZURE_TENANT_ID=e519c2e6-bc6d-4fdf-8d9c-923c2f002385
|
||||
AZURE_CLIENT_ID=9079054c-9620-4757-a256-23413042f1ef
|
||||
AZURE_REDIRECT_URI=http://localhost:7183
|
||||
|
||||
# Flask Configuration
|
||||
FLASK_ENV=development
|
||||
SECRET_KEY=dev-secret-key-change-this-for-security
|
||||
DEBUG_MODE=true
|
||||
PORT=7183
|
||||
|
||||
# Application Configuration
|
||||
ENVIRONMENT=development
|
||||
BASE_URL=http://localhost:7183
|
||||
UPLOAD_FOLDER=uploads-dev
|
||||
OUTPUT_FOLDER=output-dev
|
||||
|
||||
# Development-specific settings
|
||||
LOG_LEVEL=DEBUG
|
||||
ENABLE_DEBUG_ENDPOINTS=true
|
||||
|
||||
# Mailgun / SMTP (for email notifications)
|
||||
SMTP_SERVER=smtp.mailgun.org
|
||||
SMTP_PORT=587
|
||||
SMTP_USER=twist@mail.dev.oliver.solutions
|
||||
SMTP_PASSWORD=102115e9f3b9d7332d0cd1d4329bc0d4-77751bfc-ca066b71
|
||||
SENDER_EMAIL=TWIST-UK-SERVER@oliver.agency
|
||||
ERROR_EMAIL=nick.viljoen@brandtech.plus
|
||||
REPORT_EMAILS=nick.viljoen@brandtech.plus
|
||||
|
||||
# Box.com OAuth (per-creator user authentication for automation folders)
|
||||
# Redirect URI is computed from each request — no need to hardcode it per server.
|
||||
# Set BOX_REDIRECT_URI here only as an override if request-based detection fails.
|
||||
BOX_CLIENT_ID=o9zxyl6j917q0bkndrwfi2x5zbdeanh5
|
||||
BOX_CLIENT_SECRET=yejdbWTeBOcdsDImpNQ7nvLJZad3e0Jm
|
||||
|
|
@ -1,42 +0,0 @@
|
|||
# Production Environment Configuration
|
||||
# This file is used for production deployment on the web server
|
||||
|
||||
# OpenAI Configuration
|
||||
OPENAI_API_KEY=sk-svcacct-HSREzGYDnN-vCVGAh6LhYqlNcJVF2oefMrY9oCsdDsQFmyVJyHpLb1eSb_mp_vP4YPl4T3BlbkFJzKaOrPghIzx76_22K8VjwO6j2JnoDEvrYDrgfrnA4WjD5sTMnhOqGHXximwGXFhUoYgA
|
||||
GOOGLE_API_KEY=AIzaSyDMWN_PAnyU7bPmtWcEKq4LJfiu1KuwUsU
|
||||
|
||||
# Azure AD / MSAL Authentication Configuration (Production)
|
||||
AZURE_TENANT_ID=e519c2e6-bc6d-4fdf-8d9c-923c2f002385
|
||||
AZURE_CLIENT_ID=9079054c-9620-4757-a256-23413042f1ef
|
||||
AZURE_REDIRECT_URI=https://ai-sandbox.oliver.solutions/ai_qc/
|
||||
|
||||
# Flask Configuration
|
||||
FLASK_ENV=production
|
||||
SECRET_KEY=prod-ai-qc-oliver-solutions-2025-secure-key-9f8e7d6c5b4a3
|
||||
DEBUG_MODE=false
|
||||
PORT=7184
|
||||
|
||||
# Application Configuration
|
||||
ENVIRONMENT=production
|
||||
BASE_URL=https://ai-sandbox.oliver.solutions/ai_qc
|
||||
UPLOAD_FOLDER=uploads
|
||||
OUTPUT_FOLDER=output
|
||||
|
||||
# Production-specific settings
|
||||
LOG_LEVEL=INFO
|
||||
ENABLE_DEBUG_ENDPOINTS=false
|
||||
|
||||
# Mailgun / SMTP (for email notifications)
|
||||
SMTP_SERVER=smtp.mailgun.org
|
||||
SMTP_PORT=587
|
||||
SMTP_USER=twist@mail.dev.oliver.solutions
|
||||
SMTP_PASSWORD=102115e9f3b9d7332d0cd1d4329bc0d4-77751bfc-ca066b71
|
||||
SENDER_EMAIL=TWIST-UK-SERVER@oliver.agency
|
||||
ERROR_EMAIL=nick.viljoen@brandtech.plus
|
||||
REPORT_EMAILS=nick.viljoen@brandtech.plus
|
||||
|
||||
# Box.com OAuth (per-creator user authentication for automation folders)
|
||||
# Redirect URI is computed from each request — no need to hardcode it per server.
|
||||
# Set BOX_REDIRECT_URI here only as an override if request-based detection fails.
|
||||
BOX_CLIENT_ID=o9zxyl6j917q0bkndrwfi2x5zbdeanh5
|
||||
BOX_CLIENT_SECRET=yejdbWTeBOcdsDImpNQ7nvLJZad3e0Jm
|
||||
786
docs/superpowers/plans/2026-05-17-hp-cycle-1-onboarding.md
Normal file
786
docs/superpowers/plans/2026-05-17-hp-cycle-1-onboarding.md
Normal file
|
|
@ -0,0 +1,786 @@
|
|||
# HP Onboarding — Cycle 1 Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use `superpowers:subagent-driven-development` (recommended) or `superpowers:executing-plans` to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Implement the `hp_copy_review` check and its supporting infrastructure per `docs/superpowers/specs/2026-05-17-hp-cycle-1-onboarding-design.md`, replacing the deprecated `hp-copy` PHP/Make.com POC.
|
||||
|
||||
**Architecture:** New `excel_processor.py` mirrors `pdf_processor.py` to convert HP Source Messaging Excels into structured Markdown summaries at upload time. A single new `hp_copy_review` QC check assembles those summaries + media-plan language metadata + the asset image into one Gemini prompt and returns a structured findings list. HP gets a real client config entry, a dedicated profile, and routing for `.xlsx` uploads through the existing `/api/brand_guidelines` endpoint.
|
||||
|
||||
**Tech Stack:**
|
||||
- openpyxl 3.x (existing dep, used by `media_plan_processor.py`)
|
||||
- Gemini 2.5 Pro via `llm_config.py` (existing)
|
||||
- Existing reference-asset / brand-guidelines flow
|
||||
- Existing media-plan processor
|
||||
- No new external dependencies
|
||||
|
||||
**Branch:** `feature/hp-cycle-1-onboarding` from `develop`.
|
||||
|
||||
**Testing posture:** This project does not use pytest. Verification matches `backend/scripts/test-system.sh`: `py_compile`, import checks, profile-load tests, and real-asset smoke runs on the dev server. Inline `python3 -c "..."` snippets stand in for unit tests where helpful.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
**New files:**
|
||||
- `backend/excel_processor.py` — Excel ingestion + Gemini summarisation
|
||||
- `backend/profiles/hp_copy_review.json` — new profile
|
||||
- `backend/visual_qc_apps/hp_copy_review/app.py` — new QC check
|
||||
- `backend/visual_qc_apps/hp_copy_review/__init__.py` — empty module marker
|
||||
|
||||
**Modified files:**
|
||||
- `backend/client_config.py` — HP entry promoted from placeholder
|
||||
- `backend/api_server.py` — `.xlsx` dispatch on `/api/brand_guidelines` POST + findings-table rendering in both HTML generators
|
||||
- `backend/media_plan_processor.py` — `language` column extraction + metadata surfacing
|
||||
- `CLAUDE.md` — HP row updated from "_scope pending_" to the new doc reference (small)
|
||||
|
||||
**Test fixtures (placed manually on disk, not committed):**
|
||||
- `backend/tests/fixtures/hp/messi_core_source_messaging.xlsx`
|
||||
- `backend/tests/fixtures/hp/messi_mainstream_source_messaging.xlsx`
|
||||
- `backend/tests/fixtures/hp/gaston_source_messaging.xlsx`
|
||||
|
||||
The user-provided originals live at `/Users/nickviljoen/Desktop/AI_QC_Bitbucket/hp/recieved_docs/excel/` — those get *copied* (not symlinked) into `backend/tests/fixtures/hp/` for repeatable local verification. The directory is gitignored.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Excel processor module
|
||||
|
||||
Implement `excel_processor.py` mirroring `pdf_processor.py`. This is the most foundational change and the largest single module of new code.
|
||||
|
||||
**Files:**
|
||||
- Create: `backend/excel_processor.py`
|
||||
- Create: `backend/tests/fixtures/hp/` (gitignored)
|
||||
- Modify: `.gitignore` (add `backend/tests/fixtures/`)
|
||||
|
||||
- [ ] **Step 1.1: Set up the fixtures directory**
|
||||
|
||||
```bash
|
||||
mkdir -p backend/tests/fixtures/hp
|
||||
cp '/Users/nickviljoen/Desktop/AI_QC_Bitbucket/hp/recieved_docs/excel/26C2 Messi Core HP OmniDesk Mini Desktop PC Source Messaging 04-10 (1).xlsx' backend/tests/fixtures/hp/messi_core.xlsx
|
||||
cp '/Users/nickviljoen/Desktop/AI_QC_Bitbucket/hp/recieved_docs/excel/26C2 Messi Mainstream HP OmniDesk Mini Desktop PC Source Messaging 04-10 (1).xlsx' backend/tests/fixtures/hp/messi_mainstream.xlsx
|
||||
cp '/Users/nickviljoen/Desktop/AI_QC_Bitbucket/hp/recieved_docs/excel/HP AluminiumBook Source Messaging - Gaston 05-06.xlsx' backend/tests/fixtures/hp/gaston.xlsx
|
||||
ls backend/tests/fixtures/hp/
|
||||
```
|
||||
|
||||
Expected: three `.xlsx` files listed.
|
||||
|
||||
- [ ] **Step 1.2: Add gitignore rule for fixtures**
|
||||
|
||||
Add to `.gitignore` near the existing legacy-env block:
|
||||
|
||||
```
|
||||
# Local test fixtures (real HP Source Messaging files; not for commit)
|
||||
backend/tests/fixtures/
|
||||
```
|
||||
|
||||
- [ ] **Step 1.3: Read `pdf_processor.py` as the pattern source**
|
||||
|
||||
```bash
|
||||
wc -l backend/pdf_processor.py
|
||||
```
|
||||
|
||||
Read the file end-to-end. Identify: public surface (`process_pdf_file`), helper for raw extraction, helper for LLM summarisation, file path conventions (`brand_guidelines/files/{file_id}_summary.txt`), error handling shape, retry pattern, return tuple `(summary_text, summary_path)`.
|
||||
|
||||
- [ ] **Step 1.4: Create `excel_processor.py` skeleton**
|
||||
|
||||
Create `backend/excel_processor.py` with:
|
||||
|
||||
```python
|
||||
"""Excel reference-asset processor for HP Source Messaging files.
|
||||
|
||||
Mirrors pdf_processor.py: openpyxl extracts raw cell content from
|
||||
every sheet, Gemini summarises the result into structured Markdown
|
||||
under brand_guidelines/files/{file_id}_summary.md. The check
|
||||
hp_copy_review pulls that Markdown into its prompt at QC time.
|
||||
"""
|
||||
|
||||
import os
|
||||
from typing import Tuple
|
||||
|
||||
from openpyxl import load_workbook
|
||||
|
||||
from llm_config import call_gemini_text # adjust to actual export name
|
||||
|
||||
BRAND_GUIDELINES_DIR = os.path.join(
|
||||
os.path.dirname(os.path.abspath(__file__)), 'brand_guidelines', 'files'
|
||||
)
|
||||
|
||||
# Cap raw extraction at ~50K chars to keep the summary prompt bounded.
|
||||
# A 30-row, 12-column workbook is ~10-15K chars in practice; this leaves
|
||||
# headroom for HP's larger source files without blowing the prompt budget.
|
||||
_RAW_EXTRACTION_CAP = 50_000
|
||||
|
||||
|
||||
def process_excel_file(file_path: str, file_id: str) -> Tuple[str, str]:
|
||||
"""Extract + summarise an HP Source Messaging Excel.
|
||||
|
||||
Returns (summary_text, summary_path). Saves the summary as
|
||||
{file_id}_summary.md under BRAND_GUIDELINES_DIR. Never raises —
|
||||
on failure, writes a degraded summary containing the raw extraction
|
||||
so the reference asset is still usable, and returns that.
|
||||
"""
|
||||
raw_text = _extract_workbook_text(file_path)
|
||||
try:
|
||||
summary = _summarise_with_gemini(raw_text, os.path.basename(file_path))
|
||||
except Exception as e:
|
||||
summary = (
|
||||
f"# {os.path.basename(file_path)} (degraded — summary failed)\n\n"
|
||||
f"Gemini summarisation failed: {type(e).__name__}: {e}\n\n"
|
||||
f"## Raw extraction\n\n```\n{raw_text}\n```\n"
|
||||
)
|
||||
|
||||
os.makedirs(BRAND_GUIDELINES_DIR, exist_ok=True)
|
||||
summary_path = os.path.join(BRAND_GUIDELINES_DIR, f"{file_id}_summary.md")
|
||||
with open(summary_path, 'w', encoding='utf-8') as f:
|
||||
f.write(summary)
|
||||
return summary, summary_path
|
||||
```
|
||||
|
||||
- [ ] **Step 1.5: Implement `_extract_workbook_text`**
|
||||
|
||||
Append:
|
||||
|
||||
```python
|
||||
def _extract_workbook_text(file_path: str) -> str:
|
||||
"""Read every sheet, dump as 'Sheet: <name>\\n<tab-aligned rows>\\n\\n'."""
|
||||
wb = load_workbook(file_path, data_only=True, read_only=True)
|
||||
parts = []
|
||||
total_chars = 0
|
||||
for sheet in wb.worksheets:
|
||||
parts.append(f"Sheet: {sheet.title}\n")
|
||||
for row in sheet.iter_rows(values_only=True):
|
||||
# Skip rows where every cell is None/empty
|
||||
if not any((c is not None and str(c).strip()) for c in row):
|
||||
continue
|
||||
line = '\t'.join(('' if c is None else str(c)) for c in row)
|
||||
parts.append(line + '\n')
|
||||
total_chars += len(line) + 1
|
||||
if total_chars >= _RAW_EXTRACTION_CAP:
|
||||
parts.append(f"\n[truncated — exceeded {_RAW_EXTRACTION_CAP}-char cap]\n")
|
||||
return ''.join(parts)
|
||||
parts.append('\n')
|
||||
wb.close()
|
||||
return ''.join(parts)
|
||||
```
|
||||
|
||||
- [ ] **Step 1.6: Implement `_summarise_with_gemini`**
|
||||
|
||||
Append:
|
||||
|
||||
```python
|
||||
_SYSTEM_PROMPT = """You're processing an HP Source Messaging Excel into a structured Markdown reference. Output these sections exactly, in this order:
|
||||
|
||||
## Product / Variant
|
||||
(brand, product line, variant if any — e.g. "HP OmniDesk Mini — Core")
|
||||
|
||||
## Key Selling Points (KSPs)
|
||||
For each KSP: heading, value proposition, supporting body copy, message-length variants (ultra-short / short / medium / long if present in the source).
|
||||
|
||||
## Disclaimers / Footnotes
|
||||
Numbered list, exact wording, what claim each footnote anchors to.
|
||||
|
||||
## Approved Brand and Product Names
|
||||
Exact spellings, including trademark glyphs (™, ®, ©).
|
||||
|
||||
## Variant Notes / Watch-outs
|
||||
Anything explicitly marked variant-specific (e.g. "Mainstream only", "Core only", "must not appear in entry tier").
|
||||
|
||||
## Verboten Phrasing
|
||||
Any explicitly disallowed or deprecated phrasing called out in the source.
|
||||
|
||||
Be exhaustive but concise. Quote exactly where the source is explicit. If a section has no content in this source, write 'None specified' under it — do not omit the section heading."""
|
||||
|
||||
|
||||
def _summarise_with_gemini(raw_text: str, source_filename: str) -> str:
|
||||
user_prompt = (
|
||||
f"Source filename: {source_filename}\n\n"
|
||||
f"Raw cell content:\n\n```\n{raw_text}\n```"
|
||||
)
|
||||
# call_gemini_text is the existing text-only Gemini wrapper in llm_config.
|
||||
# If the actual export name differs, adjust in Step 1.7 verification.
|
||||
return call_gemini_text(
|
||||
system_prompt=_SYSTEM_PROMPT,
|
||||
user_prompt=user_prompt,
|
||||
model='gemini-2.5-pro',
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 1.7: Verify llm_config exports a usable text-only Gemini wrapper**
|
||||
|
||||
```bash
|
||||
grep -nE "def (call_gemini|gemini_text|generate.*gemini)" backend/llm_config.py | head -20
|
||||
```
|
||||
|
||||
If `call_gemini_text` doesn't exist under that name, find the closest analogue (look at how `pdf_processor.py` calls Gemini) and update the import + call site in `excel_processor.py` accordingly.
|
||||
|
||||
- [ ] **Step 1.8: Syntax + import verification**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -m py_compile excel_processor.py && python3 -c "import excel_processor; print('OK', excel_processor.BRAND_GUIDELINES_DIR)"
|
||||
```
|
||||
|
||||
Expected: `OK <path>/brand_guidelines/files`
|
||||
|
||||
- [ ] **Step 1.9: Run the processor against the Messi-Core fixture**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
import os, sys
|
||||
sys.path.insert(0, '.')
|
||||
from excel_processor import process_excel_file
|
||||
summary, path = process_excel_file('tests/fixtures/hp/messi_core.xlsx', 'test-messi-core')
|
||||
print('summary_path:', path)
|
||||
print('summary_len:', len(summary))
|
||||
print('first 800 chars:')
|
||||
print(summary[:800])
|
||||
"
|
||||
```
|
||||
|
||||
Expected: summary is 1500–4000 chars, contains `## Key Selling Points`, `## Disclaimers`, `## Approved Brand and Product Names`, and at least one KSP-level content snippet referencing "OmniDesk" or "Mini".
|
||||
|
||||
- [ ] **Step 1.10: Commit Task 1**
|
||||
|
||||
```bash
|
||||
git add backend/excel_processor.py .gitignore
|
||||
git commit -m "feat(excel-processor): add openpyxl + Gemini summary pipeline for HP Source Messaging
|
||||
|
||||
Mirrors pdf_processor.py — public process_excel_file() reads any HP
|
||||
Source Messaging Excel, extracts cells via openpyxl (skipping empty
|
||||
rows, capped at 50K chars), and summarises into structured Markdown
|
||||
via Gemini 2.5 Pro. Output saved as brand_guidelines/files/{file_id}_summary.md.
|
||||
|
||||
On Gemini failure the processor writes a degraded summary containing
|
||||
the raw extraction so the reference asset stays usable. Test fixtures
|
||||
(real HP Excels) live under backend/tests/fixtures/hp/ and are gitignored."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: `.xlsx` dispatch on the reference asset upload endpoint
|
||||
|
||||
Wire `excel_processor.process_excel_file` into the `/api/brand_guidelines` POST handler at `backend/api_server.py:4771` so `.xlsx` uploads route correctly.
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/api_server.py` (around the existing `/api/brand_guidelines` POST handler near line 4771)
|
||||
|
||||
- [ ] **Step 2.1: Read the existing handler to find the PDF dispatch**
|
||||
|
||||
```bash
|
||||
sed -n '4760,4900p' backend/api_server.py
|
||||
```
|
||||
|
||||
Identify: where the extension is checked, where `pdf_processor.process_pdf_file` is called, and what's returned to the client.
|
||||
|
||||
- [ ] **Step 2.2: Add the `.xlsx` branch**
|
||||
|
||||
Edit the POST handler to dispatch by extension. The exact change depends on the existing code shape — pattern is:
|
||||
|
||||
- Where the handler currently checks for `.pdf` and calls `pdf_processor.process_pdf_file(...)`, add an `elif filename.lower().endswith('.xlsx')` branch that imports `excel_processor` and calls `excel_processor.process_excel_file(...)` with the same arg signature.
|
||||
- The DB record / response shape should be identical to the PDF path — same `file_id`, same `status`, same return JSON.
|
||||
- Cover image: PDF has one; Excel doesn't. If the DB record assigns a `cover_path`, set it to `None` for Excels.
|
||||
|
||||
- [ ] **Step 2.3: Syntax + import verification**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -m py_compile api_server.py && python3 -c "import api_server; print('api_server OK')"
|
||||
```
|
||||
|
||||
- [ ] **Step 2.4: Commit Task 2**
|
||||
|
||||
```bash
|
||||
git add backend/api_server.py
|
||||
git commit -m "feat(brand-guidelines): route .xlsx uploads to excel_processor
|
||||
|
||||
The /api/brand_guidelines POST handler now dispatches by extension:
|
||||
.pdf → pdf_processor.process_pdf_file (existing), .xlsx →
|
||||
excel_processor.process_excel_file (new). Same DB record shape;
|
||||
cover image is null for Excel since there's no first-page analogue."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Media plan `language` column
|
||||
|
||||
Add `language` to the media-plan column extraction and surface it into the prompt context.
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/media_plan_processor.py`
|
||||
|
||||
- [ ] **Step 3.1: Locate the column-extraction logic**
|
||||
|
||||
```bash
|
||||
grep -n -E "country|placement|vendor|dimensions" backend/media_plan_processor.py | head -10
|
||||
```
|
||||
|
||||
These are the existing matched-row metadata fields. The `language` field will live alongside them.
|
||||
|
||||
- [ ] **Step 3.2: Add `language` to the case-insensitive header match list**
|
||||
|
||||
Edit the column-mapping section to recognise `Language` / `language` / `LANGUAGE` headers and store the value in the matched-row dict under the key `language`.
|
||||
|
||||
- [ ] **Step 3.3: Surface `language` in the prompt context block**
|
||||
|
||||
Locate where the matched-row dict is rendered as text injected into check prompts (the function that returns the "media plan context" string used by `process_single_check`). Add a line:
|
||||
|
||||
```python
|
||||
if row.get('language'):
|
||||
lines.append(f"Language: {row['language']}")
|
||||
```
|
||||
|
||||
— preserving the existing structure (no line if absent).
|
||||
|
||||
- [ ] **Step 3.4: Syntax + import verification**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -m py_compile media_plan_processor.py && python3 -c "import media_plan_processor; print('OK')"
|
||||
```
|
||||
|
||||
- [ ] **Step 3.5: Quick functional test with a synthetic plan**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
# Mock test: build a minimal row dict with a language field and confirm the
|
||||
# prompt-context formatter emits 'Language: <value>'. Exact function name to
|
||||
# locate during Step 3.3 — adjust below.
|
||||
from media_plan_processor import format_matched_row_for_prompt # adjust if named differently
|
||||
row = {'country': 'UK', 'language': 'UK English', 'placement': 'eTail tile'}
|
||||
print(format_matched_row_for_prompt(row))
|
||||
"
|
||||
```
|
||||
|
||||
Expected: output includes a line `Language: UK English`.
|
||||
|
||||
- [ ] **Step 3.6: Commit Task 3**
|
||||
|
||||
```bash
|
||||
git add backend/media_plan_processor.py
|
||||
git commit -m "feat(media-plan): extract and surface 'language' column
|
||||
|
||||
Adds case-insensitive 'language' header recognition to the media-plan
|
||||
column mapper. When present in a matched row, the value flows into
|
||||
the prompt context block as 'Language: <value>'. Absent → no line
|
||||
(graceful no-op for clients whose plans don't include the field).
|
||||
Enables multilingual support for hp_copy_review (Cycle 1) and any
|
||||
future check that wants to reason about asset language."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: HP client config + profile
|
||||
|
||||
Promote HP from placeholder. Create the `hp_copy_review` profile JSON. Ensure the profile loader picks it up.
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/client_config.py`
|
||||
- Create: `backend/profiles/hp_copy_review.json`
|
||||
|
||||
- [ ] **Step 4.1: Update the HP entry in `CLIENT_PROFILES`**
|
||||
|
||||
Edit `backend/client_config.py`. Replace the existing `'hp'` entry with:
|
||||
|
||||
```python
|
||||
'hp': {
|
||||
'name': 'HP',
|
||||
'profiles': ['hp_copy_review', 'static_general', 'video_general'],
|
||||
'display_name': 'HP',
|
||||
'description': 'HP marketing copy QC graded against canonical Source Messaging',
|
||||
'default_profile': 'hp_copy_review',
|
||||
},
|
||||
```
|
||||
|
||||
- [ ] **Step 4.2: Create the profile JSON**
|
||||
|
||||
Create `backend/profiles/hp_copy_review.json`:
|
||||
|
||||
```json
|
||||
{
|
||||
"name": "HP Copy Review",
|
||||
"description": "Marketing copy graded against canonical HP Source Messaging",
|
||||
"mode": "asset",
|
||||
"visibility": "client_specific",
|
||||
"visible_to_clients": ["hp"],
|
||||
"checks": {
|
||||
"hp_copy_review": {
|
||||
"weight": 10.0,
|
||||
"llm": "gemini",
|
||||
"enabled": true
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4.3: Verify client config**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
from client_config import get_client_profiles, get_default_profile
|
||||
print('profiles:', get_client_profiles('hp'))
|
||||
print('default:', get_default_profile('hp'))
|
||||
"
|
||||
```
|
||||
|
||||
Expected:
|
||||
```
|
||||
profiles: ['hp_copy_review', 'static_general', 'video_general']
|
||||
default: hp_copy_review
|
||||
```
|
||||
|
||||
- [ ] **Step 4.4: Verify profile load**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
from profile_config import get_profile
|
||||
p = get_profile('hp_copy_review')
|
||||
print('name:', p.name)
|
||||
print('mode:', getattr(p, 'mode', 'asset'))
|
||||
print('enabled checks:', p.get_enabled_checks())
|
||||
print('strict_grade:', getattr(p, 'strict_grade', False))
|
||||
"
|
||||
```
|
||||
|
||||
Expected: profile loads, mode is `asset`, enabled_checks lists `['hp_copy_review']`. (The check itself doesn't exist yet → may emit a "Loaded profile" line but the check loader fails for `hp_copy_review`; that's expected at this task boundary.)
|
||||
|
||||
- [ ] **Step 4.5: Commit Task 4**
|
||||
|
||||
```bash
|
||||
git add backend/client_config.py backend/profiles/hp_copy_review.json
|
||||
git commit -m "feat(hp): promote HP client + add hp_copy_review profile
|
||||
|
||||
HP is no longer a placeholder. The client gets a new hp_copy_review
|
||||
profile (single weighted check, client-specific visibility) as its
|
||||
default, plus the generic static_general and video_general profiles
|
||||
it already had visibility into."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: `hp_copy_review` check module
|
||||
|
||||
The actual QC check — single LLM call per asset.
|
||||
|
||||
**Files:**
|
||||
- Create: `backend/visual_qc_apps/hp_copy_review/__init__.py` (empty)
|
||||
- Create: `backend/visual_qc_apps/hp_copy_review/app.py`
|
||||
|
||||
- [ ] **Step 5.1: Read `flask_app_template.py` and a comparable real check**
|
||||
|
||||
```bash
|
||||
ls backend/flask_app_template.py 2>/dev/null && wc -l backend/flask_app_template.py
|
||||
ls backend/visual_qc_apps/boots_tandc_wording/app.py && wc -l backend/visual_qc_apps/boots_tandc_wording/app.py
|
||||
```
|
||||
|
||||
Read both. The boots_tandc_wording check is the closest analogue (copy-against-reference, image input, structured findings output). Use it as the implementation pattern.
|
||||
|
||||
- [ ] **Step 5.2: Create the directory + empty `__init__.py`**
|
||||
|
||||
```bash
|
||||
mkdir -p backend/visual_qc_apps/hp_copy_review
|
||||
touch backend/visual_qc_apps/hp_copy_review/__init__.py
|
||||
```
|
||||
|
||||
- [ ] **Step 5.3: Create `app.py` with the standard check skeleton**
|
||||
|
||||
Copy the structure from `boots_tandc_wording/app.py` (Flask blueprint pattern, `run_check(...)` or equivalent entry point, the reference-asset summary injection, the media-plan context injection). Adapt the prompt to:
|
||||
|
||||
```
|
||||
You are a copy reviewer for HP marketing materials. Compare the
|
||||
marketing asset against the canonical Source Messaging provided.
|
||||
|
||||
PRODUCT LANGUAGE: <from media plan, or "not specified">
|
||||
|
||||
CANONICAL SOURCE MESSAGING:
|
||||
<one or more Markdown summaries from attached Excel reference assets,
|
||||
concatenated, each preceded by a header like "--- File: messi_core.xlsx ---">
|
||||
|
||||
MARKETING ASSET:
|
||||
[image]
|
||||
|
||||
For every claim, headline, body line, disclaimer, footnote, spec
|
||||
call-out, and brand mention visible on the asset, evaluate against
|
||||
the canonical source. Output a JSON object with this shape:
|
||||
|
||||
{
|
||||
"score": <number 0-10>,
|
||||
"summary": "<one-paragraph headline finding>",
|
||||
"findings": [
|
||||
{
|
||||
"priority": "high" | "medium" | "low",
|
||||
"category": "ksp" | "disclaimer" | "spec" | "variant" | "tone" | "brand-name" | "language" | "other",
|
||||
"quote": "<exact quote from the asset>",
|
||||
"issue": "<what's wrong>",
|
||||
"suggested_fix": "<what it should say, citing the canonical source>",
|
||||
"source_reference": "<where in source messaging this comes from>"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
Rules:
|
||||
- If no Source Messaging is attached, return {"score": 0, "summary": "No HP Source Messaging reference was attached — cannot grade copy without a canonical source.", "findings": []}
|
||||
- High-priority findings weight the score most heavily
|
||||
- Empty findings (clean asset) is a valid result; score 9-10
|
||||
- Return ONLY the JSON object, no surrounding prose
|
||||
```
|
||||
|
||||
- [ ] **Step 5.4: Implement response parsing**
|
||||
|
||||
The check function must parse the LLM's JSON response. Handle:
|
||||
- Valid JSON with the expected shape → extract `score`, `summary`, `findings` and return them in the standard check result shape (`{'score': ..., 'response': ..., 'findings': ...}` — match the existing checks' return shape so the report renderer can pick up `findings` later).
|
||||
- Malformed JSON → score 0, response = raw LLM text, findings = `[]`, summary = "Failed to parse check output".
|
||||
- The `findings` array gets attached to the check result dict so the report renderer in Task 6 can detect it.
|
||||
|
||||
- [ ] **Step 5.5: Syntax + import + profile load verification**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -m py_compile visual_qc_apps/hp_copy_review/app.py && python3 -c "
|
||||
from profile_config import get_profile
|
||||
from app_discovery import discover_qc_apps # or the actual loader path
|
||||
apps = discover_qc_apps()
|
||||
print('hp_copy_review in apps:', 'hp_copy_review' in apps)
|
||||
p = get_profile('hp_copy_review')
|
||||
print('profile enabled checks:', p.get_enabled_checks())
|
||||
"
|
||||
```
|
||||
|
||||
Expected: `hp_copy_review in apps: True`, profile lists it as enabled.
|
||||
|
||||
- [ ] **Step 5.6: Dry-run prompt-assembly test (no LLM call)**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
# Smoke test: instantiate the check, call its prompt-assembly helper
|
||||
# (without invoking Gemini) with mock reference summaries and a mock
|
||||
# media-plan row including language='UK English'. Confirm output prompt
|
||||
# contains 'Language: UK English', 'CANONICAL SOURCE MESSAGING', and
|
||||
# the findings-format instructions.
|
||||
from visual_qc_apps.hp_copy_review.app import build_prompt # adjust if named differently
|
||||
prompt = build_prompt(
|
||||
reference_summaries=[('messi_core.xlsx', '## Product\nHP OmniDesk Mini Core')],
|
||||
media_plan_row={'language': 'UK English', 'country': 'UK'},
|
||||
)
|
||||
assert 'Language: UK English' in prompt, 'language missing from prompt'
|
||||
assert 'CANONICAL SOURCE MESSAGING' in prompt
|
||||
assert 'findings' in prompt
|
||||
print('prompt assembly OK')
|
||||
"
|
||||
```
|
||||
|
||||
- [ ] **Step 5.7: Commit Task 5**
|
||||
|
||||
```bash
|
||||
git add backend/visual_qc_apps/hp_copy_review/
|
||||
git commit -m "feat(hp_copy_review): single-check LLM grader against Source Messaging
|
||||
|
||||
Single Gemini call per asset. Prompt assembles attached Source
|
||||
Messaging summaries + media-plan language context + the asset image.
|
||||
Returns structured JSON with score, summary, and a findings array
|
||||
(priority, category, quote, issue, suggested fix, source reference).
|
||||
Empty findings = clean asset; missing reference → score 0 with a
|
||||
clear message rather than running blind."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Findings-table rendering in both HTML report generators
|
||||
|
||||
Both HTML generators need a small case to render `findings` as a table.
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/api_server.py` (`generate_html_content` and `generate_comprehensive_html_report` — see [[feedback_multi_html_generators]])
|
||||
|
||||
- [ ] **Step 6.1: Locate both generators**
|
||||
|
||||
```bash
|
||||
grep -n "def generate_html_content\|def generate_comprehensive_html_report" backend/api_server.py
|
||||
```
|
||||
|
||||
Expected: two function definitions, both render check results to HTML.
|
||||
|
||||
- [ ] **Step 6.2: Identify where each renders a per-check response**
|
||||
|
||||
In each generator, find the section that renders the per-check `response` text (often inside an expandable `<details>` block). The new case goes *before* that fallback: if the check's result dict contains a `findings` array, render the table; else fall back to the text response.
|
||||
|
||||
- [ ] **Step 6.3: Implement a shared helper `_render_findings_table(findings)`**
|
||||
|
||||
Add near the existing CSS/render helpers in `api_server.py`:
|
||||
|
||||
```python
|
||||
def _render_findings_table(findings):
|
||||
"""Render an hp_copy_review-style findings array as an HTML table."""
|
||||
if not findings:
|
||||
return '<p class="muted">No findings — copy is clean.</p>'
|
||||
rows = []
|
||||
for f in findings:
|
||||
priority = f.get('priority', 'low')
|
||||
pri_class = {'high': 'score-bad', 'medium': 'score-ok', 'low': 'score-good'}.get(priority, 'muted')
|
||||
rows.append(
|
||||
f'<tr>'
|
||||
f'<td><span class="score-pill {pri_class}">{priority.upper()}</span></td>'
|
||||
f'<td><code>{f.get("category", "")}</code></td>'
|
||||
f'<td><code>{(f.get("quote") or "")[:200]}</code></td>'
|
||||
f'<td>{f.get("issue", "")}</td>'
|
||||
f'<td>{f.get("suggested_fix", "")}</td>'
|
||||
f'<td class="muted">{f.get("source_reference", "")}</td>'
|
||||
f'</tr>'
|
||||
)
|
||||
return (
|
||||
'<table class="findings-table"><thead><tr>'
|
||||
'<th>Priority</th><th>Category</th><th>Quote</th>'
|
||||
'<th>Issue</th><th>Suggested fix</th><th>Source</th>'
|
||||
'</tr></thead><tbody>'
|
||||
+ ''.join(rows) + '</tbody></table>'
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 6.4: Wire the helper into both generators**
|
||||
|
||||
In each generator, where it renders a check's response block, add (in pseudocode):
|
||||
|
||||
```python
|
||||
findings = check_result.get('findings')
|
||||
if findings is not None:
|
||||
body_html += _render_findings_table(findings)
|
||||
else:
|
||||
body_html += render_response_text(check_result.get('response', ''))
|
||||
```
|
||||
|
||||
Match the exact variable names and HTML scaffolding used by each generator.
|
||||
|
||||
- [ ] **Step 6.5: Syntax verification + manual HTML inspection**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -m py_compile api_server.py && python3 -c "
|
||||
from api_server import _render_findings_table
|
||||
html = _render_findings_table([
|
||||
{'priority': 'high', 'category': 'disclaimer', 'quote': 'must be linked to a boots.com account', 'issue': 'Wrong account type', 'suggested_fix': '...linked to an Advantage Card account...', 'source_reference': 'Messi Core T&Cs row 18'},
|
||||
{'priority': 'low', 'category': 'tone', 'quote': 'a tiny powerhouse', 'issue': 'Not approved phrasing', 'suggested_fix': 'Use \"compact and capable\"', 'source_reference': 'KSP 1'},
|
||||
])
|
||||
with open('/tmp/findings_preview.html', 'w') as f:
|
||||
f.write('<!DOCTYPE html><html><head><style>table{border-collapse:collapse}td,th{border:1px solid #ddd;padding:6px}</style></head><body>' + html + '</body></html>')
|
||||
print('wrote /tmp/findings_preview.html')
|
||||
"
|
||||
open /tmp/findings_preview.html
|
||||
```
|
||||
|
||||
Eye-check: table renders, priority pills coloured correctly, quote in monospace.
|
||||
|
||||
- [ ] **Step 6.6: Commit Task 6**
|
||||
|
||||
```bash
|
||||
git add backend/api_server.py
|
||||
git commit -m "feat(report): render hp_copy_review findings as a structured table
|
||||
|
||||
Both HTML report generators (generate_html_content and
|
||||
generate_comprehensive_html_report) get a small case: when a check
|
||||
result has a 'findings' array, render it as a priority-coloured
|
||||
table with quote/issue/suggested-fix/source columns instead of the
|
||||
default response-text block. Fallback to text rendering when
|
||||
findings is absent — every existing check is unaffected."
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Dev smoke test + deployment
|
||||
|
||||
End-to-end verification on the dev server with real assets and real LLM calls.
|
||||
|
||||
- [ ] **Step 7.1: Run the full pre-session checklist**
|
||||
|
||||
```bash
|
||||
cd backend && python3 -c "
|
||||
from profile_config import get_profile
|
||||
for p in ['general_check','static_general','unilever_key_visual','unilever_packaging','diageo_key_visual','diageo_packaging','loreal_static','amazon_static','boots_static','boots_ppack','inclusive_accessibility','video_general','axa_policy_document','axa_policy_document_diff','axa_accessibility','hp_copy_review']:
|
||||
prof = get_profile(p)
|
||||
print(f'OK {prof.name} ({len(prof.get_enabled_checks())} checks)')
|
||||
"
|
||||
cd .. && python3 -m py_compile backend/**/*.py
|
||||
python3 -c "
|
||||
import sys; sys.path.insert(0, 'backend')
|
||||
import api_server, llm_config, profile_config, jwt_validator, auth_middleware
|
||||
print('all imports OK')
|
||||
"
|
||||
```
|
||||
|
||||
Expected: every profile (including new `hp_copy_review`) loads; all syntax + imports green.
|
||||
|
||||
- [ ] **Step 7.2: Push the feature branch**
|
||||
|
||||
```bash
|
||||
git push -u origin feature/hp-cycle-1-onboarding
|
||||
```
|
||||
|
||||
- [ ] **Step 7.3: Open PR `feature/hp-cycle-1-onboarding → develop` via Bitbucket**
|
||||
|
||||
URL: `https://bitbucket.org/zlalani/ai_qc/pull-requests/new?source=feature/hp-cycle-1-onboarding&t=1`. Destination = `develop`. Title: "feat(hp): cycle 1 — hp_copy_review check + excel processor + language field". Body links to the spec.
|
||||
|
||||
- [ ] **Step 7.4: Merge PR, then deploy to dev**
|
||||
|
||||
SSH to `optical-production-dev`:
|
||||
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
backend/scripts/deploy.sh dev
|
||||
sudo journalctl -u ai-qc -n 30 --no-pager
|
||||
```
|
||||
|
||||
Confirm clean deploy + service healthy.
|
||||
|
||||
- [ ] **Step 7.5: Manually upload Source Messaging fixtures to dev**
|
||||
|
||||
Via the UI at `optical-dev.oliver.solutions/ai_qc/`:
|
||||
1. Sign in (admin).
|
||||
2. Settings → Reference Assets (for client `hp`).
|
||||
3. Upload `messi_core.xlsx`, `messi_mainstream.xlsx`, `gaston.xlsx` (from the original locations under `~/Desktop/AI_QC_Bitbucket/hp/recieved_docs/excel/`).
|
||||
4. Watch the status badge — each should flip to `ready` within 60s. If degraded, inspect the saved `_summary.md` to see what failed.
|
||||
|
||||
- [ ] **Step 7.6: Run an HP marketing asset through `hp_copy_review`**
|
||||
|
||||
1. From the HP team, get a real Messi or Gaston marketing image (PNG/JPG).
|
||||
2. Open a QC session as client `hp`, profile `hp_copy_review`.
|
||||
3. Attach the relevant Source Messaging reference (e.g. `messi_core` for a Core-targeted asset).
|
||||
4. (Optional) Upload a media plan with a `language` column populated so the prompt picks it up.
|
||||
5. Run the QC.
|
||||
6. Inspect the report: confirm findings table renders, priority pills coloured correctly, quotes are real text from the asset.
|
||||
|
||||
If output structure is wrong (e.g. LLM returns prose instead of JSON), iterate the prompt — small follow-up PRs against `develop`.
|
||||
|
||||
- [ ] **Step 7.7: PR `develop → main` and tag**
|
||||
|
||||
Once HP-side smoke testing confirms the output is useful:
|
||||
|
||||
```bash
|
||||
# (laptop) sync local develop, open PR via Bitbucket UI:
|
||||
# https://bitbucket.org/zlalani/ai_qc/pull-requests/new?source=develop&dest=main&t=1
|
||||
```
|
||||
|
||||
After merge:
|
||||
|
||||
```bash
|
||||
git fetch origin
|
||||
git tag -a v1.4.0 origin/main -m "v1.4.0 — HP onboarding cycle 1 (hp_copy_review check + excel processor + media-plan language field)"
|
||||
git push origin v1.4.0
|
||||
git rev-parse v1.4.0^{commit}; git rev-parse origin/main # should match
|
||||
```
|
||||
|
||||
- [ ] **Step 7.8: Deploy v1.4.0 to prod**
|
||||
|
||||
SSH to `optical-production`:
|
||||
|
||||
```bash
|
||||
cd /opt/ai_qc
|
||||
backend/scripts/deploy.sh prod v1.4.0
|
||||
sudo journalctl -u ai-qc -n 30 --no-pager
|
||||
```
|
||||
|
||||
No env-file backup dance needed — env files are now permanently gitignored (since v1.3.2).
|
||||
|
||||
- [ ] **Step 7.9: Upload Source Messaging files to prod**
|
||||
|
||||
Repeat Step 7.5 against the prod UI (`optical-prod.oliver.solutions/ai_qc/`). Source Messaging files are *per-server* — they live in `brand_guidelines/files/` on disk and don't sync between dev and prod.
|
||||
|
||||
- [ ] **Step 7.10: Hand off to HP team**
|
||||
|
||||
Confirm HP has access (via per-user client access — `Nick.Viljoen@oliver.agency` adds the HP team's email(s)). Walk them through:
|
||||
1. Where to upload Source Messaging files (Settings → Reference Assets).
|
||||
2. How to run a QC (select hp_copy_review, attach the right reference).
|
||||
3. What feedback to send back (findings missed, findings wrong, output format suggestions).
|
||||
|
||||
Collect first-week feedback before opening Cycle 2 (Word/PPT processor).
|
||||
293
docs/superpowers/specs/2026-05-17-database-design.md
Normal file
293
docs/superpowers/specs/2026-05-17-database-design.md
Normal file
|
|
@ -0,0 +1,293 @@
|
|||
# AI QC Database Design
|
||||
|
||||
**Goal:** Introduce a PostgreSQL database to the AI QC app, dual-written alongside the existing JSONL usage logs, to support a future exec + drill-down dashboard.
|
||||
|
||||
**Architecture:** Postgres 16 in a Docker container on each of the two app VMs (`optical-production-dev`, `optical-production`), accessed by the Flask app via SQLAlchemy + psycopg. Alembic for schema migrations. Every analysis write goes to BOTH the existing JSONL log AND the database; JSONL remains source-of-truth for this cycle. A daily `pg_dump → GCS` job protects against VM loss. After dual-write has run for ~1 week and parity is verified, a one-shot script backfills historical JSONL data.
|
||||
|
||||
**Tech stack:** PostgreSQL 16, SQLAlchemy 2.x, psycopg 3, Alembic, Docker / docker-compose, `gsutil` (for backups).
|
||||
|
||||
**Status:** Phase 5 of Nick's roadmap, cycle 1 of 3 (DB → Docker → Dashboard). Cycles 2 and 3 are out of scope here.
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
Today, the AI QC app records every analysis event as a line in `backend/usage_logs/YYYY-MM-DD.jsonl`. Readers of those files include `backend/generate_usage_report.py` (CLI), the `/api/client_usage_stats` endpoint (powering the "Reporting" tab), and the audit trail for `access_change` / `access_request` events. The JSONL store is battle-tested but limits us in three ways that motivate moving to a relational DB:
|
||||
|
||||
1. **Querying** — anything beyond simple per-client/per-day aggregates means writing one-off scripts. A dashboard with filters, drill-down, and trending will not scale on flat files.
|
||||
2. **Cross-event joins** — "for this analysis, list all checks that scored below 6" requires correlating multiple JSONL events. Trivial in SQL.
|
||||
3. **Schema evolution** — JSONL is shape-free; the DB makes us define what we record and forces consistency across writers.
|
||||
|
||||
The dashboard itself is cycle 3 of Phase 5 — out of scope for this cycle. This cycle just lands the data.
|
||||
|
||||
---
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope (this cycle)
|
||||
|
||||
1. Postgres 16 container running on each VM under compose project name `ai-qc`, with a named volume for data persistence.
|
||||
2. Schema: two tables (`analyses`, `analysis_checks`) plus indexes — design locked during brainstorm (see "Schema" below).
|
||||
3. SQLAlchemy models, a thin repository module for typed writes, an engine factory that reads `DB_URL` from env.
|
||||
4. Alembic baseline migration; the deploy script learns to run `alembic upgrade head` before restarting the service.
|
||||
5. Dual-write integration at every analysis event site in `api_server.py` (UI upload flow, document-mode flow, Box webhook flow, per-check writes, completion writes).
|
||||
6. Feature flag `DB_DUAL_WRITE_ENABLED` (env var) — defaults to `true`. If set to `false`, the app skips DB writes entirely and behaves exactly as it does today. This is the rollback escape hatch.
|
||||
7. Daily `pg_dump → GCS` backup via a systemd timer on each VM. 30-day retention via a GCS bucket lifecycle policy.
|
||||
8. Parity verification script (`verify_dual_write_parity.py`) that compares JSONL records vs DB records for a given date range.
|
||||
9. One-shot backfill script (`backfill_from_jsonl.py`) — idempotent — to populate the DB with historical JSONL records once dual-write parity has been confirmed.
|
||||
|
||||
### Out of scope (handled in follow-up cycles or follow-up tasks)
|
||||
|
||||
- Containerising the Flask app itself (Phase 5 cycle 2 — Docker).
|
||||
- Building the dashboard UI (Phase 5 cycle 3 — Dashboard).
|
||||
- Migrating `generate_usage_report.py` and `/api/client_usage_stats` to read from the DB. This is the natural follow-up to this cycle but ships separately, once the DB has proven itself.
|
||||
- Replicating access events (`access_change`, `access_request`, `access_denied`) to the DB. Stays in JSONL for now.
|
||||
- Moving to managed Cloud SQL. Reasonable future move, but it's a procurement + GCP-admin conversation, not a code change.
|
||||
- Connection pooling, HA / read replicas, encryption-at-rest beyond Docker volume defaults — all defer until we have a real need.
|
||||
|
||||
---
|
||||
|
||||
## Schema
|
||||
|
||||
Two tables, plus indexes. JSON columns are PostgreSQL `jsonb` (queryable, indexable later if needed). All timestamps are `timestamptz` (UTC). The `analyses.id` reuses the existing in-pipeline `session_id` (a UUID generated at upload time) so JSONL and DB rows share the same primary key.
|
||||
|
||||
### `analyses`
|
||||
|
||||
One row per QC run.
|
||||
|
||||
| column | type | nullable | notes |
|
||||
|---|---|---|---|
|
||||
| `id` | `uuid` (pk) | no | matches pipeline `session_id` |
|
||||
| `client_id` | `text` | no | e.g. `loreal`, `boots` |
|
||||
| `user_email` | `text` | yes | `null` for Box webhook runs |
|
||||
| `source_origin` | `text` | no | `ui_upload` \| `box_webhook` |
|
||||
| `profile_id` | `text` | no | e.g. `loreal_static` |
|
||||
| `mode` | `text` | no | `asset` \| `document` \| `document_diff` |
|
||||
| `source_file_name` | `text` | no | original filename |
|
||||
| `source_file_size_bytes` | `bigint` | yes | |
|
||||
| `source_file_type` | `text` | yes | `image` / `video` / `pdf` / etc. |
|
||||
| `started_at` | `timestamptz` | no | |
|
||||
| `completed_at` | `timestamptz` | yes | |
|
||||
| `status` | `text` | no | `pending` \| `running` \| `success` \| `failed` |
|
||||
| `overall_score` | `numeric` | yes | 100-pt scale (120 for Unilever KV) |
|
||||
| `overall_verdict` | `text` | yes | `pass` \| `fail` |
|
||||
| `technical_report` | `jsonb` | yes | Phase 3 output (file inspection result) |
|
||||
| `media_plan_match` | `jsonb` | yes | matched row from media plan if any |
|
||||
| `total_tokens` | `bigint` | yes | sum across all checks |
|
||||
| `estimated_cost_usd` | `numeric` | yes | |
|
||||
| `report_html_path` | `text` | yes | on-disk path to the generated report |
|
||||
| `box_report_file_id` | `text` | yes | Box file id when uploaded back |
|
||||
| `error_message` | `text` | yes | populated when `status = 'failed'` |
|
||||
|
||||
### `analysis_checks`
|
||||
|
||||
One row per check executed. Cascade-delete with the parent analysis.
|
||||
|
||||
| column | type | nullable | notes |
|
||||
|---|---|---|---|
|
||||
| `id` | `uuid` (pk) | no | |
|
||||
| `analysis_id` | `uuid` (fk → `analyses.id`, on delete cascade) | no | |
|
||||
| `check_name` | `text` | no | e.g. `brand_logo_check` |
|
||||
| `llm_provider` | `text` | no | `gemini` \| `openai` |
|
||||
| `status` | `text` | no | `success` \| `failed` |
|
||||
| `score` | `numeric` | yes | |
|
||||
| `weight` | `numeric` | no | profile-defined weight |
|
||||
| `tokens_used` | `integer` | yes | |
|
||||
| `duration_ms` | `integer` | yes | |
|
||||
| `details` | `jsonb` | no | full LLM response — supports drill-down |
|
||||
| `started_at` | `timestamptz` | no | |
|
||||
| `completed_at` | `timestamptz` | yes | |
|
||||
|
||||
The `details` column is the heaviest field by far; it preserves the whole structured LLM response so future drill-down views need no schema change. PostgreSQL TOAST compression handles the storage cost; we can prune the column later if it becomes a problem.
|
||||
|
||||
### Indexes
|
||||
|
||||
- `analyses(client_id, started_at desc)` — primary index for client-scoped chronological queries (the exec dashboard's main shape).
|
||||
- `analyses(user_email, started_at desc)` — for user-scoped views.
|
||||
- `analyses(started_at desc)` — for global trending queries.
|
||||
- `analysis_checks(analysis_id)` — drill-down lookups.
|
||||
|
||||
---
|
||||
|
||||
## Components
|
||||
|
||||
### `backend/db/` (new module)
|
||||
|
||||
- `__init__.py` — public surface: `get_session()` (context manager), `init_engine()`.
|
||||
- `engine.py` — engine factory. Reads `DB_URL` from env. Singleton engine across the Flask app.
|
||||
- `models.py` — SQLAlchemy ORM models for `Analysis` and `AnalysisCheck`.
|
||||
- `repository.py` — typed write helpers, the only surface `api_server.py` touches:
|
||||
- `record_analysis_start(session, *, analysis_id, client_id, user_email, ...) -> Analysis`
|
||||
- `record_analysis_complete(session, analysis_id, *, overall_score, overall_verdict, status, total_tokens, ...) -> None`
|
||||
- `record_analysis_failed(session, analysis_id, *, error_message) -> None`
|
||||
- `record_check_result(session, analysis_id, *, check_name, llm_provider, status, score, weight, details, ...) -> AnalysisCheck`
|
||||
- `parity.py` — pure helpers consumed by both `verify_dual_write_parity.py` and `backfill_from_jsonl.py`. Parses a JSONL line into the same shape the repository functions accept.
|
||||
|
||||
### `backend/migrations/` (new Alembic env)
|
||||
|
||||
Standard Alembic layout. Baseline revision creates both tables + indexes.
|
||||
|
||||
### `backend/scripts/` (additions)
|
||||
|
||||
- `backfill_from_jsonl.py` — one-shot, idempotent. Reads `usage_logs/*.jsonl`, looks up each `session_id` in `analyses`, inserts when missing. Per-row try/except so a single malformed line doesn't abort the run. Logs a summary at the end (read / inserted / skipped / errored).
|
||||
- `verify_dual_write_parity.py` — for a given date range, counts events in JSONL vs rows in DB and surfaces drift. Exits non-zero when drift is detected.
|
||||
- `pg_backup_to_gcs.sh` — `pg_dump` of the AI QC database, gzip, `gsutil cp` to the configured GCS bucket under `pgdumps/<env>/<date>.sql.gz`. Logs to journal via systemd.
|
||||
|
||||
### `deploy/docker-compose.db.yml` (new)
|
||||
|
||||
```yaml
|
||||
name: ai-qc
|
||||
|
||||
services:
|
||||
db:
|
||||
image: postgres:16
|
||||
restart: unless-stopped
|
||||
environment:
|
||||
POSTGRES_USER: ${POSTGRES_USER}
|
||||
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
|
||||
POSTGRES_DB: ${POSTGRES_DB}
|
||||
volumes:
|
||||
- pgdata:/var/lib/postgresql/data
|
||||
ports:
|
||||
- "127.0.0.1:5432:5432" # not exposed externally
|
||||
|
||||
volumes:
|
||||
pgdata:
|
||||
name: ai-qc-pgdata
|
||||
```
|
||||
|
||||
The top-level `name: ai-qc` and the explicit volume `name:` are belt-and-braces against the CLAUDE.md compose-collision warning, even though the AI QC VMs are dedicated.
|
||||
|
||||
### `backend/config/*.env` additions
|
||||
|
||||
Both `development.env` and `production.env` (gitignored) get:
|
||||
|
||||
```
|
||||
# Database
|
||||
DB_URL=postgresql+psycopg://aiqc:<password>@127.0.0.1:5432/aiqc
|
||||
POSTGRES_USER=aiqc
|
||||
POSTGRES_PASSWORD=<generated>
|
||||
POSTGRES_DB=aiqc
|
||||
DB_DUAL_WRITE_ENABLED=true
|
||||
|
||||
# Backups
|
||||
BACKUP_GCS_BUCKET=ai-qc-pg-backups-<env> # name to be confirmed when bucket is provisioned
|
||||
```
|
||||
|
||||
### `api_server.py` modifications
|
||||
|
||||
Five touchpoints, all dual-write. The pattern at every site:
|
||||
|
||||
```python
|
||||
from db import get_session
|
||||
from db.repository import record_analysis_start
|
||||
|
||||
# After the existing JSONL write:
|
||||
if os.environ.get("DB_DUAL_WRITE_ENABLED", "true").lower() == "true":
|
||||
try:
|
||||
with get_session() as db_session:
|
||||
record_analysis_start(db_session, analysis_id=session_id, ...)
|
||||
except Exception:
|
||||
logger.exception("DB write failed (analysis_id=%s); JSONL is authoritative", session_id)
|
||||
```
|
||||
|
||||
Touchpoints:
|
||||
|
||||
1. `/api/start_analysis` (asset analysis kickoff) — `record_analysis_start`.
|
||||
2. `/api/document/start_analysis` (document-mode kickoff) — `record_analysis_start` with `mode='document'`.
|
||||
3. `_run_box_triggered_analysis` (Box webhook flow) — `record_analysis_start` with `source_origin='box_webhook'`, `user_email=None`.
|
||||
4. `process_single_check` (per-check completion path) — `record_check_result`.
|
||||
5. Analysis completion / failure paths — `record_analysis_complete` or `record_analysis_failed`. (Note: per the [[feedback_multi_html_generators]] memory, there are parallel HTML generators; both completion paths need wiring.)
|
||||
|
||||
### Backup setup (per VM, manual one-off)
|
||||
|
||||
- Create GCS bucket `ai-qc-pg-backups-dev` and `ai-qc-pg-backups-prod` (or names agreed with infra).
|
||||
- Apply a 30-day lifecycle deletion rule on each bucket.
|
||||
- Create a GCP service account with `roles/storage.objectCreator` on its corresponding bucket. Key file at `/etc/ai-qc/gcs-backup-sa.json`, chmod 600, owned by the service user.
|
||||
- Install `pg_backup_to_gcs.sh` to `/opt/ai_qc/backend/scripts/`.
|
||||
- Create systemd unit `ai-qc-pg-backup.service` + timer `ai-qc-pg-backup.timer` firing daily at 02:00 UTC.
|
||||
|
||||
The bucket creation and IAM steps depend on Nick's boss / infra owner (Nick isn't a GCP admin). The script and systemd units are in code and reusable across the two envs.
|
||||
|
||||
---
|
||||
|
||||
## Data Flow
|
||||
|
||||
**Normal asset analysis (UI upload):**
|
||||
|
||||
1. User uploads → `/api/start_analysis`.
|
||||
2. JSONL: write `{event: "analysis_start", session_id, client_id, user_email, profile_id, ...}` to today's log file.
|
||||
3. DB: insert row in `analyses` (`status='pending'`, `started_at=now`, all metadata fields populated; `overall_score` / `completed_at` left null).
|
||||
4. Pipeline runs checks in batches. For each check completion:
|
||||
- JSONL: write `{event: "check_complete", session_id, check_name, status, score, ...}`.
|
||||
- DB: insert row in `analysis_checks` (full `details` jsonb, score, tokens, duration).
|
||||
5. All checks done, final scoring + report generation:
|
||||
- JSONL: write `{event: "analysis_complete", session_id, overall_score, overall_verdict, total_tokens, ...}`.
|
||||
- DB: update the `analyses` row (`status='success'`, `completed_at`, `overall_score`, `overall_verdict`, `total_tokens`, `report_html_path`).
|
||||
|
||||
**Document mode and Box webhook flows:** Same dual-write pattern; different entry points. `source_origin` and (for Box) `user_email=None` differentiate the rows.
|
||||
|
||||
**DB write failure (any step):** caught, logged with `exc_info`, swallowed. JSONL has already succeeded; the analysis continues normally and produces the same user-visible output it does today.
|
||||
|
||||
---
|
||||
|
||||
## Error Handling and Rollback
|
||||
|
||||
- **DB connection failure on any single write** — caught, logged, swallowed. JSONL is authoritative.
|
||||
- **DB connection unavailable at app startup** — app still boots. The engine is initialised lazily; any DB write will fail-and-log per the rule above. This means a misconfigured DB never takes the production service down.
|
||||
- **Schema migration failure during deploy** — `deploy.sh` runs `alembic upgrade head` *before* restarting the systemd unit. If migration fails, deploy aborts before restart, and the existing app keeps running on the unmigrated DB (which it doesn't read from yet, so no harm). Existing rollback logic in `deploy.sh` already handles `git reset --hard` of the app code.
|
||||
- **Rollback escape hatch** — flip `DB_DUAL_WRITE_ENABLED=false` and restart the service. The app reverts to the exact pre-cycle behaviour (JSONL only). This is the no-code-changes-required panic button.
|
||||
- **Backup script failure** — logged to journal. No automated alerting in this cycle (see "Out of scope"). Weekly manual spot-check that the latest dump exists in the bucket.
|
||||
- **Parity drift detected** — `verify_dual_write_parity.py` exits non-zero and prints a diff. We investigate manually before proceeding with the backfill.
|
||||
|
||||
---
|
||||
|
||||
## Testing
|
||||
|
||||
Tests run against a dedicated `ai-qc-test` database, brought up via the same docker-compose with a separate database name. No SQLite in tests (per CLAUDE.md Postgres-only policy).
|
||||
|
||||
- **Unit tests** for repository functions — each test creates and tears down its rows in a transaction.
|
||||
- **Migration tests** — `alembic upgrade head` followed by `alembic downgrade base` should both succeed cleanly on an empty DB.
|
||||
- **Dual-write integration test** — mock a fake analysis end-to-end with the DB enabled; assert that both the JSONL file and the DB rows reflect the expected events, and that they agree.
|
||||
- **Parity test** — feed a known JSONL fixture into the backfill script against an empty DB; assert the resulting DB rows match expectations.
|
||||
- **Failure-resilience test** — point `DB_URL` at an unreachable host and run an analysis. Verify the analysis still completes successfully and that the failure is logged.
|
||||
|
||||
---
|
||||
|
||||
## Deployment
|
||||
|
||||
Deploys land in this order on each VM:
|
||||
|
||||
1. Update env file with new variables (`DB_URL`, `POSTGRES_*`, `DB_DUAL_WRITE_ENABLED`, `BACKUP_GCS_BUCKET`).
|
||||
2. Bring up the Postgres container: `docker compose -f deploy/docker-compose.db.yml -p ai-qc up -d`.
|
||||
3. `alembic upgrade head` against the new DB (run from the app venv).
|
||||
4. Deploy the new app code via existing `deploy.sh dev` / `deploy.sh prod <tag>`. Existing flow already runs `pip install` on `requirements.txt` change.
|
||||
5. Smoke-test: trigger one analysis via the UI; confirm the row appears in both `backend/usage_logs/<today>.jsonl` and `analyses`.
|
||||
6. Set up the GCS bucket + service account + systemd timer for backups (per "Backup setup" above) — can be done in parallel with the above once bucket is provisioned.
|
||||
7. Let dual-write run for ~1 week. Run `verify_dual_write_parity.py --last-days 7`. If clean, proceed to step 8; if drift, debug and re-run.
|
||||
8. Run `backfill_from_jsonl.py` once. Re-run `verify_dual_write_parity.py` over the full historical range to confirm.
|
||||
|
||||
The follow-up cycle (migrating the readers to query the DB instead of JSONL) starts only after step 8 lands cleanly on prod.
|
||||
|
||||
---
|
||||
|
||||
## Definition of Done
|
||||
|
||||
- Both VMs running Postgres 16 in a Docker container under `name: ai-qc` with a named volume.
|
||||
- Alembic migrations applied; both tables and all indexes present.
|
||||
- App dual-writing on every analysis touchpoint; failures swallowed and logged.
|
||||
- `DB_DUAL_WRITE_ENABLED=false` confirmed to restore pre-cycle behaviour.
|
||||
- Daily backup job running on both VMs and producing readable dumps in GCS.
|
||||
- Parity script clean over the last 7 days of live traffic.
|
||||
- Historical JSONL backfilled into the DB and parity-verified across the full history.
|
||||
- This spec + the implementation plan committed under `docs/superpowers/`.
|
||||
|
||||
---
|
||||
|
||||
## Deferred decisions (worth surfacing at resume)
|
||||
|
||||
- **GCS bucket naming + provisioning** — depends on infra owner.
|
||||
- **Backup verification automation** — manual spot-check is fine to start; if we add alerting, this is where it goes.
|
||||
- **Connection pooling tuning** — defaults are fine; revisit if the app gets containerised + scaled out (cycle 2 of Phase 5).
|
||||
- **Move to Cloud SQL** — eventual win on managed backups + encryption at rest; out of scope until a clear procurement decision is made.
|
||||
- **Replicating access events to DB** — would let the dashboard surface access history; defer until the dashboard cycle is being scoped.
|
||||
|
|
@ -0,0 +1,280 @@
|
|||
# HP Onboarding — Cycle 1: `hp_copy_review` Check
|
||||
|
||||
**Goal:** Onboard HP onto the AI QC platform with a Source-Messaging-grounded copy review check, replacing the existing `hp-copy` PHP/Make.com POC tool.
|
||||
|
||||
**Architecture:** Single new QC check `hp_copy_review` grades an HP marketing asset's on-asset copy against canonical Source Messaging Excel files uploaded as reference assets. A new `excel_processor.py` mirrors `pdf_processor.py`: openpyxl extracts raw cell content at upload time, Gemini summarises into structured Markdown, saved alongside the file under `brand_guidelines/files/`. At QC time the check prompt assembles the Markdown summary(s) + media-plan language metadata + the asset image and returns a structured findings list. HP gets a real client config entry plus the generic profiles it already has visibility into.
|
||||
|
||||
**Tech stack:** openpyxl 3.x (already a project dep — used by `media_plan_processor.py`), existing `llm_config.py` Gemini integration, existing brand-guidelines flow, existing media-plan processor. **No new external dependencies.**
|
||||
|
||||
**Status:** Cycle 1 of 3 in HP onboarding. Cycles 2 (Word/PPT ingestion) and 3 (Box file picker) are independent and ship later. This cycle is independently shippable.
|
||||
|
||||
---
|
||||
|
||||
## Context
|
||||
|
||||
HP's existing `hp-copy` is a PHP UI wrapping a Make.com webhook (opaque). The PM raised seven concerns; Dave's decision is to deprecate the POC and migrate HP onto AI QC. Of the seven concerns:
|
||||
|
||||
- **Solved natively by AI QC today:** stability, configurable rule sets, accuracy (LLM + reference assets eliminate the false-positives-on-brand-names class of bugs because the canonical source list comes from the Excels), bulk processing (local upload supports multi-file out of the box).
|
||||
- **Cycle 1 (this spec) addresses:** the HP-specific check, the Source-Messaging Excel ingestion pipeline, and multilingual via a media-plan `language` field.
|
||||
- **Other cycles:** Word/PPT support (Cycle 2), Box file picker (Cycle 3).
|
||||
|
||||
The user-visible flow Day 1 after this cycle ships:
|
||||
1. HP user uploads Source Messaging `.xlsx` files (Messi-Core, Messi-Mainstream, Gaston) once via Settings → Reference Assets.
|
||||
2. HP user uploads marketing asset(s) via local upload — same UX as Boots/AXA/LOREAL.
|
||||
3. HP user selects the `hp_copy_review` profile and attaches the relevant Source Messaging reference(s).
|
||||
4. The check returns a structured findings table matching the Messi Copy Review document format (priority, quote, issue, suggested fix, source citation).
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope (this cycle)
|
||||
|
||||
1. **HP client config** promoted from `_scope pending_` to a real entry with `hp_copy_review` as the default profile.
|
||||
2. **`hp_copy_review` profile JSON** — single weighted check, client-specific visibility.
|
||||
3. **`hp_copy_review` QC check** at `backend/visual_qc_apps/hp_copy_review/app.py`.
|
||||
4. **`backend/excel_processor.py`** — new module mirroring `pdf_processor.py`. openpyxl extraction → Gemini summary → Markdown saved as `{file_id}_summary.md`.
|
||||
5. **Reference-asset upload routing** — `.xlsx` uploads route to `excel_processor.process_excel_file`. Existing endpoints (`POST /api/brand_guidelines`, `GET /api/brand_guidelines/<id>/status`, `POST .../reprocess`) work without modification beyond the dispatch line.
|
||||
6. **Media plan `language` field** — free-form text column; surfaced in matched-row metadata; included in the check prompt when present; absent → graceful no-op.
|
||||
7. **Report rendering** — small case in the two HTML report generators so the findings JSON renders as a priority-coloured table instead of a wall of text.
|
||||
8. **Unit + smoke tests** as listed under Testing.
|
||||
|
||||
### Out of scope (other cycles or deferred)
|
||||
|
||||
- Word / PPT ingestion as reference assets — Cycle 2.
|
||||
- Box file picker UI — Cycle 3.
|
||||
- HP master brand guidelines reference — HP hasn't provided one yet.
|
||||
- Briefs (`.pptx`) as reference assets — depends on Cycle 2.
|
||||
- Multi-language Source Messaging variants — HP currently has English-only files. If they later provide Spanish / Dutch versions, no code change is needed; they upload as separate reference assets.
|
||||
- Strict-grade enforcement — the HP Copy Review is a nuanced priority-tiered (High / Medium / Low) review, not pass/fail. Standard 0–100 weighted scoring.
|
||||
- Replacing or modifying the existing `hp-copy` PHP tool. We leave it running; HP migrates traffic at their own pace.
|
||||
|
||||
---
|
||||
|
||||
## Components
|
||||
|
||||
### `backend/client_config.py` — HP entry
|
||||
|
||||
Promote HP from placeholder to a real entry. Add `hp_copy_review` to the profile list, set as default:
|
||||
|
||||
```python
|
||||
'hp': {
|
||||
'name': 'HP',
|
||||
'profiles': ['hp_copy_review', 'static_general', 'video_general'],
|
||||
'display_name': 'HP',
|
||||
'description': 'HP marketing copy QC graded against canonical Source Messaging',
|
||||
'default_profile': 'hp_copy_review',
|
||||
},
|
||||
```
|
||||
|
||||
`box_folder_id` / `box_reports_folder_id` deferred to Cycle 3.
|
||||
|
||||
### `backend/profiles/hp_copy_review.json` — new profile
|
||||
|
||||
```json
|
||||
{
|
||||
"name": "HP Copy Review",
|
||||
"description": "Marketing copy graded against canonical HP Source Messaging",
|
||||
"mode": "asset",
|
||||
"visibility": "client_specific",
|
||||
"visible_to_clients": ["hp"],
|
||||
"checks": {
|
||||
"hp_copy_review": {
|
||||
"weight": 10.0,
|
||||
"llm": "gemini",
|
||||
"enabled": true
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Total weight = 10.0 → scoring uses the `weighted_score × 10` path, max 100. Single check carries the whole score. No `strict_grade`.
|
||||
|
||||
### `backend/visual_qc_apps/hp_copy_review/app.py` — new check
|
||||
|
||||
Standard QC app module following `flask_app_template.py`. Single Gemini call. Returns: `score` (0–10), `summary` (one-paragraph headline), and `findings` (JSON list).
|
||||
|
||||
**Prompt structure** (starting point — expect tuning during smoke testing):
|
||||
|
||||
```
|
||||
You are a copy reviewer for HP marketing materials. Compare the
|
||||
marketing asset against the canonical Source Messaging provided.
|
||||
|
||||
PRODUCT LANGUAGE: <from media plan, or "not specified">
|
||||
|
||||
CANONICAL SOURCE MESSAGING:
|
||||
<one or more Markdown summaries from attached Excel reference assets,
|
||||
concatenated with a `---` separator and a file-name header>
|
||||
|
||||
MARKETING ASSET:
|
||||
<image>
|
||||
|
||||
For every claim, headline, body line, disclaimer, footnote, spec
|
||||
call-out, and brand mention visible on the asset, evaluate against
|
||||
the canonical source. Output a structured findings array:
|
||||
|
||||
[
|
||||
{
|
||||
"priority": "high" | "medium" | "low",
|
||||
"category": "ksp" | "disclaimer" | "spec" | "variant" |
|
||||
"tone" | "brand-name" | "language" | "other",
|
||||
"quote": "<exact quote from the asset>",
|
||||
"issue": "<what's wrong>",
|
||||
"suggested_fix": "<what it should say, citing the canonical source>",
|
||||
"source_reference": "<where in source messaging this comes from,
|
||||
e.g. 'Core sheet row 12 KSP 3'>"
|
||||
},
|
||||
...
|
||||
]
|
||||
|
||||
Then provide a score from 0–10 reflecting overall copy quality
|
||||
(10 = no issues, 0 = severe and pervasive issues). Score should
|
||||
weight high-priority issues most heavily.
|
||||
|
||||
If no Source Messaging is attached, return score 0 with a clear
|
||||
summary explaining that no canonical source was provided.
|
||||
```
|
||||
|
||||
**Empty-findings case** (clean asset): valid result — score 9–10, `findings: []`, summary "no issues identified".
|
||||
|
||||
**No-reference-attached case**: check returns score 0 with the explanatory message, rather than running blind against an empty source.
|
||||
|
||||
### `backend/excel_processor.py` — new module
|
||||
|
||||
Mirrors `pdf_processor.py`. Public surface:
|
||||
|
||||
- `process_excel_file(file_path, file_id) -> tuple[str, str]` — reads `.xlsx`, returns `(summary_text, summary_path)`. Saves `{file_id}_summary.md` under `brand_guidelines/files/`.
|
||||
|
||||
Internal helpers:
|
||||
|
||||
- `_extract_workbook_text(path) -> str` — openpyxl, iterates all sheets, dumps as `"Sheet: <name>\n<row-by-row tab-aligned cell values>\n\n"`. Skips empty rows. Caps at a reasonable cell budget (e.g. 50K chars) to bound prompt size.
|
||||
- `_summarise_with_gemini(raw_text, source_filename) -> str` — Gemini 2.5 Pro call with HP-tuned system prompt (below) producing a structured Markdown summary, ~1500–3000 words.
|
||||
|
||||
**Summary prompt** (Excel-specific):
|
||||
|
||||
```
|
||||
You're processing an HP Source Messaging Excel into a structured
|
||||
Markdown reference. Output these sections:
|
||||
|
||||
## Product / Variant
|
||||
(brand, product line, variant if any — e.g. "HP OmniDesk Mini — Core")
|
||||
|
||||
## Key Selling Points (KSPs)
|
||||
For each KSP: heading, value proposition, supporting body copy,
|
||||
message-length variants (ultra-short / short / medium / long if
|
||||
present in the source).
|
||||
|
||||
## Disclaimers / Footnotes
|
||||
Numbered list, exact wording, what claim each footnote anchors to.
|
||||
|
||||
## Approved Brand and Product Names
|
||||
Exact spellings, including trademark glyphs (™, ®, ©).
|
||||
|
||||
## Variant Notes / Watch-outs
|
||||
Anything explicitly marked variant-specific (e.g. "Mainstream only",
|
||||
"Core only", "must not appear in entry tier").
|
||||
|
||||
## Verboten Phrasing
|
||||
Any explicitly disallowed or deprecated phrasing called out in the source.
|
||||
|
||||
Be exhaustive but concise. Quote exactly where the source is explicit.
|
||||
```
|
||||
|
||||
No cover image (Excel has no analogous concept). The reference-asset DB record schema already permits a null `cover_path`.
|
||||
|
||||
### `backend/media_plan_processor.py` — `language` column
|
||||
|
||||
When parsing media-plan Excel sheets, extract `language` (case-insensitive header match: `language`, `Language`, `LANGUAGE`) into the matched-row metadata dict. The existing media-plan-context block injected into prompts gains a `Language: <value>` line when the field is present; if absent, the line is omitted entirely (graceful no-op for clients whose media plans don't include language).
|
||||
|
||||
### `api_server.py` — reference asset upload routing
|
||||
|
||||
Existing `/api/brand_guidelines` POST routes `.pdf` → `pdf_processor.process_pdf_file`. Extend the dispatch: `.xlsx` → `excel_processor.process_excel_file`. Reuse the existing DB-record shape and the existing `GET .../<id>/status` and `POST .../<id>/reprocess` endpoints unchanged — they're agnostic to processor type.
|
||||
|
||||
### Report rendering — findings table
|
||||
|
||||
Per the [[feedback_multi_html_generators]] memory, there are two HTML generators (`generate_html_content` and `generate_comprehensive_html_report`). Both need a small case for `hp_copy_review`: when the check response contains a `findings` array, render as a table with columns for **Priority** (red/amber/green pill), **Category** (pill), **Quote** (monospace), **Issue**, **Suggested fix**, **Source**. Falls back to the existing plain-text response renderer if `findings` is absent (e.g. malformed LLM response).
|
||||
|
||||
---
|
||||
|
||||
## Data Flow
|
||||
|
||||
**Reference asset upload (one-time per Source Messaging file):**
|
||||
|
||||
1. HP user uploads `.xlsx` via Settings → Reference Assets.
|
||||
2. `api_server.py` routes by extension to `excel_processor.process_excel_file`.
|
||||
3. openpyxl extracts raw cell content from all sheets.
|
||||
4. Gemini summarises into structured Markdown via the HP-specific summary prompt.
|
||||
5. Summary saved at `brand_guidelines/files/{file_id}_summary.md`.
|
||||
6. DB record updated; status flips to `ready`.
|
||||
|
||||
**QC run (per analysis):**
|
||||
|
||||
1. HP user uploads marketing asset (image).
|
||||
2. Selects `hp_copy_review` profile.
|
||||
3. Selects one or more Source Messaging reference assets (Core / Mainstream / Gaston as applicable).
|
||||
4. (Optional) The asset's filename matches a media plan row containing a `language` value.
|
||||
5. `process_single_check` for `hp_copy_review` assembles the prompt: system instructions + concatenated Markdown summaries + media-plan context (with language if present) + asset image.
|
||||
6. Single Gemini call returns score + summary + findings JSON.
|
||||
7. Report renderer presents findings as a Messi-Review-style table.
|
||||
|
||||
---
|
||||
|
||||
## Error Handling
|
||||
|
||||
- **Excel parse failure** (corrupt file, password-protected, etc.) — processor returns an error; DB status = `failed`; user sees the error in the reference-assets list. No app crash.
|
||||
- **Gemini summarisation failure at upload** — retry once with exponential backoff; if still failing, save the raw extraction as the summary and mark status = `degraded`. The check can still use a degraded summary (lower fidelity) rather than blocking.
|
||||
- **Check-time LLM failure or malformed findings JSON** — existing `process_single_check` exception handling captures and records a score-0 result with the error in the response. Standard pattern, no new surface.
|
||||
- **Empty findings** (clean asset) — valid result; score 9–10, `findings: []`, summary "no issues identified".
|
||||
- **No reference asset attached** — check returns score 0 with a clear message ("No HP Source Messaging reference selected — attach a Source Messaging Excel to compare against"). Doesn't run blind.
|
||||
- **Excel processing concurrency** — uploads are independent files; `pdf_processor.py` already handles concurrent uploads safely (per-file_id artefact paths). Same pattern applies.
|
||||
|
||||
---
|
||||
|
||||
## Testing
|
||||
|
||||
Tests run against the project's existing pytest setup. Real Source Messaging Excels live under `tests/fixtures/hp/` (copied from the user-provided originals).
|
||||
|
||||
- **Unit tests** — `excel_processor`:
|
||||
- Happy path: Messi-Core / Messi-Mainstream / Gaston Excels each yield a non-empty `.md` summary containing the expected section headers (`## Key Selling Points`, `## Disclaimers / Footnotes`, etc.) and at least one KSP-level content snippet.
|
||||
- Corrupt file: error returned, no crash.
|
||||
- Empty workbook: graceful degradation with a sensible message.
|
||||
- **Unit tests** — `hp_copy_review/app.py`:
|
||||
- Prompt assembly: given mock reference summaries and a mock media-plan row with `language: "UK English"`, assert the assembled prompt contains the language line, the source-messaging block delimiter, and the findings-format instructions.
|
||||
- Response parsing: given a known Gemini-shape JSON response (fixture), assert findings list extracted correctly with all six fields per finding.
|
||||
- Empty references: score 0 + the explanatory message.
|
||||
- **Integration smoke test**: end-to-end with a real Messi asset (sample PNG of an OmniDesk eTail tile) + the Messi-Core Source Messaging reference attached. Assert the check runs to completion, returns a valid score, returns at least one finding (the Messi Copy Review found 34 — Gemini should surface at least 3 in the deterministic ones).
|
||||
- **Profile load** in the pre-session checklist: add `hp_copy_review` to the loader test.
|
||||
|
||||
---
|
||||
|
||||
## Deployment
|
||||
|
||||
Code-only changes — no infrastructure work, no requirements changes (openpyxl already installed).
|
||||
|
||||
1. PR `feature/hp-cycle-1-onboarding → develop`. Deploy to dev via `deploy.sh dev`.
|
||||
2. **One-time data step on dev:** HP team (or Nick on their behalf) uploads the three Source Messaging Excel files (Messi-Core, Messi-Mainstream, Gaston-v2) via the UI. These land in `brand_guidelines/files/` on dev only — uploads are not synced between dev and prod; the prod uploads happen separately.
|
||||
3. Dev smoke test: run an HP marketing image through `hp_copy_review` with the Messi-Core reference attached. Verify output structure mirrors the Messi Copy Review doc.
|
||||
4. PR `develop → main`. Tag `v1.4.0` (minor — new client capability). Deploy to prod via `deploy.sh prod v1.4.0`.
|
||||
5. HP team uploads Source Messaging files on prod, runs first real QC, provides feedback. Prompt tuning iterations are post-deploy LLM-prompt changes — small follow-up PRs as needed, no spec changes.
|
||||
|
||||
---
|
||||
|
||||
## Definition of Done
|
||||
|
||||
- `hp_copy_review` profile loads cleanly (pre-session checklist passes with the new profile in the loader script).
|
||||
- `client_config.get_client_profiles('hp')` returns `['hp_copy_review', 'static_general', 'video_general']`.
|
||||
- `client_config.get_default_profile('hp')` returns `'hp_copy_review'`.
|
||||
- Uploading a Source Messaging `.xlsx` produces a non-empty `_summary.md` within 60s of upload.
|
||||
- Running `hp_copy_review` on a known Messi asset with the Messi-Core reference attached returns findings overlapping with at least 3 of the 34 issues in the HP-provided Messi Copy Review doc (rough qualitative bar — Gemini scoring varies run-to-run, but the major issues should be detected).
|
||||
- Report renders the findings as a structured table, not free-text.
|
||||
- Media plan parsing extracts `language` when present; the check prompt includes a `Language:` line in that case.
|
||||
- Standard pre-session checklist all green on develop tip.
|
||||
|
||||
---
|
||||
|
||||
## Deferred decisions (worth surfacing at follow-up)
|
||||
|
||||
- **Strict-grade for HP?** Not in V1. If HP wants any High-priority finding to force overall Fail, add `strict_grade: true` to the profile and extend the scoring path (small retrofit).
|
||||
- **HP master brand guidelines** — none today. Whenever HP provides a master brand guide PDF (colour palette, logo usage, typography), it can be attached as an additional reference asset alongside Source Messaging. No code change.
|
||||
- **Prompt template tuning** — the templates above are starting points. Live HP usage will surface what to refine. Iterate via small prompt-only PRs.
|
||||
- **Non-English Source Messaging** — if HP later provides Spanish / Dutch versions, they upload as separate reference assets and select the relevant one(s) per QC run. Works without code change.
|
||||
- **Findings-output schema versioning** — if HP wants additional fields per finding (e.g. screenshot crop region, suggested approval routing), add to the JSON shape and bump renderer.
|
||||
- **Briefs as reference assets** — depends on Cycle 2 (Word/PPT ingestion). Once that lands, HP can attach Gaston/Messi `.pptx` briefs alongside the Excel sources.
|
||||
Loading…
Add table
Reference in a new issue