Conversation
…dit P2-11 - sanitize_brief_text: strip C0 controls (preserve \t \n \r), bound length - verify_image_bytes: allow-list PNG/JPEG by magic; reject SVG/GIF/other - strip_exif: copy() round-trip drops exif/xmp/icc, preserves pixels - strip_dangerous_html: remove script/style/iframe/object/embed, drop on*= handlers, neutralize javascript: URLs in href/src/action Uses html.parser (stdlib) per repo convention, not lxml. 21 new tests; full suite 105 passing in 0.92s.
- Text patches resolve via slot.selector; missing selectors logged + skipped - word-wrappers structure-aware split for staggered hero h1 - Palette regex covers from/to/via/bg/text/border/shadow/ring × 400/500/600 - Images written as PNG to <out>/assets/gen_<slot_id>.png + src updated - All LLM-derived values run through strip_dangerous_html (defense-in-depth) - Uses html.parser (stdlib) per repo convention 7 tests; full suite 112 passing.
- structure_brief: gpt-5-mini Responses API + brand_brief strict schema - personalize: multimodal (logo + slots) + dynamic closed-enum schema for patches[].slot_id and images[].slot_id (deterministic resolution; no selector hallucinations) - generate_images_parallel: AsyncOpenAI gpt-image-1 medium; first call sets style, rest run via asyncio.gather with first as input_images - Hard budget cap (default $1.00) enforced BEFORE every call; raises BudgetExceededError without burning the API. Total spent tracked. - All user inputs sanitized; logo verified + EXIF-stripped before upload - pytest-asyncio added (strict mode); 12 mocked tests cover all 3 paths Full suite 124 passing.
- run_pipeline() chains slot extraction → structure_brief → personalize → async image gen → apply_personalization - Fail-fast on bad logo bytes BEFORE any API call (SVG/empty rejected) - Inventory + index.html missing → FileNotFoundError before LLM - structured_brief_override skips Step 2 (UI lets user edit fields) - dry_run=True validates inputs and logs slot summary, no API calls - Each step's failure logged with step= key before re-raising 7 tests; full suite 131 passing.
Routes: - GET /personalize — render intake form (template added in Task 10) - POST /api/personalize/structure — JSON brief → structured fields (5/min) - POST /api/personalize/run — multipart brief+logo+html_dir → personalized.html (2/min) Hardening: - Global MAX_CONTENT_LENGTH bumped 1 MiB → 8 MiB to fit 2 MiB logo upload; per-route caps stay strict (4 KiB structure, 5 MiB run, 2 MiB logo) - html_dir confined to downloads/ via realpath check (no traversal) - Lazy import of personalize.* inside handlers — keeps import app side-effect-free - Smoke verified: threading.active_count() unchanged on import 8 route tests; full suite 139 passing.
- templates/personalize.html: 3-step UI (brief textarea + logo + html_dir → edit-extracted-fields → run + result panel), no separate JS file - Browser observability logger copied from index.html (same _rawFetch capture-before-wrap pattern; PR #1 lesson) - Smoke verified: GET /personalize returns 200 with all selectors present
- argparse: html_dir positional + --brief --logo --budget --dry-run - python-dotenv loads OPENAI_API_KEY from .env if present - --dry-run validates inputs and emits slot summary, zero API calls - Smoke-tested: dry-run on sample fixture extracts 12 slots in <1s
- test_structure_brief_live: ~$0.005, 10s — validates brand_brief schema - test_personalize_live_with_vision_no_image_gen: ~$0.10, 70s — validates multimodal personalize call + site_personalization closed-enum schema - Validated locally: gpt-5-mini available, ~$0.105 spent total - Image generation not exercised live (gpt-image-1 $0.07/call) — covered by mocked tests; live path is identical SDK pattern Default pytest run skips both: 139 passed, 2 skipped.
- docs/AUDIT.md: P2-11 marked RESOLVED with module pointer + test count - ROADMAP.md: Phase 4 SHIPPED block with landed-in module map - TODO.md: Phase 4 done entry; Later section now Phase 5-6 only - CLAUDE.md: new 'Personalization module' section with conventions (always inject OpenAI client in tests, budget guard fires before call, closed-enum slot IDs, no synthetic faces, Responses not Assistants) - docs/superpowers/plans/: 13-task TDD plan that drove the implementation Branch state: 11 commits, 139 tests + 2 live (gated), all green.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Phase 4 introduced these deps but the CI install lines are explicit (legacy pattern, not pip install -e .) so each new dep needs to be added. pytest job also needs pytest-asyncio for the new async tests. Affects: pytest job (collection-time PIL ImportError), pip-audit job (installed-package set), smoke job (defensive — kratos_clone doesn't import PIL today but the env should match).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements Phase 4 of the project, adding a personalization module that leverages OpenAI's gpt-5-mini and gpt-image-1 to customize captured websites. The changes include a new "personalize" package for slot extraction, security hardening, and HTML patching, alongside new Flask routes and a frontend intake form. The review highlights a potential issue with global regex replacements in the palette application logic, suggesting direct DOM manipulation for better correctness and performance. It also recommends making the pipeline orchestrator asynchronous to avoid event loop conflicts when integrated into async environments.
| html = str(soup) | ||
| html = re.sub( | ||
| rf"\b({_PREFIX_GROUP})-orange-500\b", | ||
| rf"\1-[#{primary}]", | ||
| html, | ||
| ) | ||
| html = re.sub( | ||
| rf"\b({_PREFIX_GROUP})-orange-600\b", | ||
| rf"\1-[#{primary_pressed}]", | ||
| html, | ||
| ) | ||
| html = re.sub( | ||
| rf"\b({_PREFIX_GROUP})-orange-400\b", | ||
| rf"\1-[#{primary_hover}]", | ||
| html, | ||
| ) | ||
| return BeautifulSoup(html, "html.parser") |
There was a problem hiding this comment.
The current implementation of _apply_palette performs a global string replacement on the entire HTML document using regex and then re-parses the result. This approach has two significant drawbacks:
- Correctness: It may inadvertently modify text content, IDs, or other attributes that happen to match the Tailwind class pattern (e.g., a paragraph explaining Tailwind classes or an element with a matching ID).
- Efficiency: Converting the entire
BeautifulSoupobject to a string and re-parsing it multiple times is computationally expensive, especially for large documents.
Consider iterating over elements that have a class attribute and updating the classes directly in the DOM. For example:
for tag in soup.find_all(class_=True):
classes = tag.get("class", [])
new_classes = []
for cls in classes:
# Apply replacement logic to individual class names
new_classes.append(cls.replace("orange-500", f"[#{primary}]"))
tag["class"] = new_classes|
|
||
| # Step 6 — async image generation. | ||
| try: | ||
| images = asyncio.run(client.generate_images_parallel(plan, slots)) |
There was a problem hiding this comment.
Using asyncio.run() inside a synchronous function like run_pipeline can lead to a RuntimeError if the code is executed in an environment where an event loop is already running (e.g., within an async web framework or a task runner).
To improve robustness and future-proof the pipeline, consider making run_pipeline an async function and allowing the caller to manage the event loop execution, or providing an asynchronous version of the orchestrator.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c17e5afa16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {"type": "input_text", "text": user_payload}, | ||
| { | ||
| "type": "input_image", | ||
| "image_url": f"data:image/png;base64,{logo_b64}", |
There was a problem hiding this comment.
OpenAIBrandClient.personalize accepts both PNG and JPEG logos (verify_image_bytes), but this payload is always sent as data:image/png to Responses. For valid JPEG uploads, that MIME/content mismatch can make the vision input fail to decode, so /api/personalize/run may return a 502 for inputs the API explicitly claims to support. Build the data URL from the detected format (image/png vs image/jpeg) to keep accepted JPEG uploads functional.
Useful? React with 👍 / 👎.
| @limiter.limit(os.getenv("PERSONALIZE_RUN_RATE_LIMIT", "2 per minute")) | ||
| def personalize_run(): | ||
| """Steps 4–8 — apply personalization to a captured site.""" | ||
| if (request.content_length or 0) > _PERSONALIZE_RUN_MAX_BYTES: |
There was a problem hiding this comment.
This size gate only checks request.content_length, which is unset for chunked uploads; in that case it evaluates as 0 and skips the 5 MiB guard. A client can therefore bypass this per-route limit and still force multipart parsing up to the global 8 MiB cap, increasing memory/CPU exposure on /api/personalize/run. Mirror the /api/client-errors pattern by checking the buffered body length (or equivalent) instead of relying solely on Content-Length.
Useful? React with 👍 / 👎.
(c1) DOM-walk palette swap (was global regex on serialized HTML) - _apply_palette now iterates every element with a 'class' attribute and rewrites individual class tokens via match-or-keep. Previously a regex against str(soup) could match 'from-orange-500' inside text content, IDs, aria-labels, etc. Test case test_palette_swap_does_not_touch_text_content asserts <code>from-orange-500</code>, id=, aria-label= are preserved. - Class normalization handles both bs4 multi-valued (list) and html.parser (str) shapes; explicit type annotations satisfy mypy. (c2) async pipeline variant (was bare asyncio.run inside sync function) - arun_pipeline: async public API; awaits client.generate_images_parallel directly. Composes inside any caller-managed event loop (FastAPI etc.). - run_pipeline: sync wrapper. Detects a running loop via asyncio.get_running_loop and raises a clear RuntimeError pointing at arun_pipeline instead of dying inside asyncio.run with the confusing 'cannot be called from a running event loop' trace. - 2 new tests: arun works inside event loop; run refuses inside event loop. Tests: 183 → 186 passing. mypy clean on personalize/. bandit HIGH=0.
1 participant
Closes Phase 4 of
ROADMAP.md. Implementsdocs/PERSONALIZATION.mdas code.Summary
personalize/package:slots,sanitize,openai_client,patcher,pipeline,cli(~750 LoC).GET /personalize+POST /api/personalize/{structure,run}(rate-limited 5/min and 2/min/IP).templates/personalize.html) with browser observability logger reused.RUN_OPENAI_LIVE=1.Architecture (per docs/PERSONALIZATION.md)
Live validation
```
RUN_OPENAI_LIVE=1 uv run pytest tests/integration -v -s
test_structure_brief_live PASSED (
$0.005, 10s)$0.10, 70s)test_personalize_live_with_vision_no_image_gen PASSED (
spent: $0.1050; remaining: $0.3950
```
Real gpt-5-mini accepts the schemas. Closed-enum slot_id honored — every patch came back with a slot id from the list we passed.
Security (P2-11 closure)
Test plan
Out of scope