Skip to content

Add Best Buy WebHarbor mirror - #48

Closed
Lxr-max wants to merge 1 commit into
aiming-lab:mainfrom
Lxr-max:add-bestbuy-site
Closed

Lxr-max wants to merge 1 commit into
aiming-lab:mainfrom
Lxr-max:add-bestbuy-site

Conversation

@Lxr-max

@Lxr-max Lxr-max commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Add Best Buy WebHarbor mirror.

Status

Closed as superseded by Review PR #135 (Best Buy mirror takeover).

Original implementation lived on branch add-bestbuy-site.

@MufanQiu

MufanQiu commented Jul 1, 2026

Copy link
Copy Markdown

Review — bestbuy (PR #48)

Verdict: REQUEST CHANGES.

We deployed the environment locally and ran the full review checklist; every finding below was independently reproduced, and fidelity was checked against the real upstream site.

Issues (summary)

  • BLOCKER: .assets-revision pins 'main' but bestbuy.tar.gz only exists on the HF PR ref -> CI fail-closed
  • MAJOR: 73/156 product images broken: unescaped '&' in generated SVGs (XML invalid)
  • MINOR: Task BestBuy--11 is ill-posed: no deal title mentions a 'big screen event'
  • MINOR: Answer-leak + single-result: Task 0 (price) readable on results card; product searches return 1 result
  • MINOR: Port-slot collision with released merriam_webster (both claim slot 15 / :40015)
  • MINOR: Task 3 amenity leaked on stores list page

Mechanical checks: FAIL

Most mechanics are clean: byte-identical idempotent reset (instance md5 == seed md5 == 71c41f6f..., function-level idempotency gate at seed_data.py:1515), no cross-site imports (from sites./import sites = 0 hits), no AppleDouble files, no scraped_data reads in handlers, .gitignore matches .assetpaths, and py_compile passes on app.py + seed_data.py.

Visual fidelity: FAIL

Brand fidelity is strong: yellow/blue palette (#0046be/#fff200), category nav, hero, deals row, featured grid, store cards, footer.

Functional depth: PASS (with nits)

Every flow was driven live. Login (alice.j@test.com → /account "Jordan"; bad password rejected), register (success + mismatched-password and duplicate-email validation), rewards (alice 1840), wishlist, cart add + reload-persistence, full checkout end-to-end as david producing deterministic order BBY-240061, compare page with alice's 3 seeded items, order lookup/detail.

Task quality: FAIL

19 tasks over a deep, balanced catalog (156 products = 13×12 categories, 25 brands, 15 stores, 24 support articles, 60 orders, 339 reviews, 312 protection plans), so per-category browse distractors are real.

Required fixes before approval

  1. (BLOCKER) Repin .assets-revision from revision: main to the merged HuggingFace dataset SHA that actually contains bestbuy.tar.gz (after HF PR 31 merges).
  2. (MAJOR) Fix _write_svg (seed_data.py:258-292) to XML-escape title/subtitle/badge (xml.sax.saxutils.escape/html.escape) before interpolation; regenerate all SVGs and reseed.
  3. (MAJOR) Fix the Task 0 price leak and the single-result pattern: hide price on result cards for price-answer tasks and/or phrase product tasks with looser queries that return ≥6 results with near-misses, so the answer requires opening the PDP and the agent must disambiguate.
  4. (MAJOR) Resolve the :40015 slot collision with the released merriam_webster: append the later-merged site at index 16 (:40016) and bump EXPOSE 40000-40016 in websyn_start.sh, control_server.py, and Dockerfile.
  5. (MINOR) Repair Task 11's wording/location mismatch: rename deal 14's title/badge to mention "Big Screen Event," or surface product.deal_badge on the /deals page, or reword the task to "find the product tagged Big Screen Event" without constraining it to the deals page.

@jackjin1997

Copy link
Copy Markdown
Contributor

I'm taking over the review and remediation of this Best Buy contribution. I'll build a separate Review: PR on top of @Lxr-max's original commit, preserve the original authorship, and link the reviewer PR here.

The review will cover current-main integration (port slot/registry and the .assets-revision pin against the paired HF asset PR #31), the findings already raised by @MufanQiu, browser/UI fidelity against bestbuy.com, task solvability and quality, deterministic verifiers backed by recorded UI executions, positive/negative and valid-alternative grading tests, and an independent review of the frozen runs. Maintainers retain final approval and merge responsibility. Please avoid duplicating this review while it is in progress.

@jackjin1997

jackjin1997 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Reviewer takeover is now tracked in Draft Review PR #135: #135

It preserves @Lxr-max’s Best Buy contribution and links the reviewer-owned HF asset candidate. Blind review is still pending; please do not merge either artifact yet.

@Lxr-max

Lxr-max commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by Review PR #135 (Best Buy mirror takeover).

@Lxr-max Lxr-max closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants