feat(accuweather): add high-fidelity mirror - #93
Conversation
|
Taking ownership of the review for this PR (Track B). Plan: build and drive the mirror per the |
…iming-lab#93 aiming-lab#39 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
Review: accuweather (PR #93) — site validation + grading contractReviewer: JeremyJC67 · Site author: @sarendis56 · Head reviewed: Verdict: REQUEST CHANGES. The Flask mirror itself is functionally solid (auth, save/remove, alerts, unit preference, register all persist; byte-identical seed after two boots and read-only driving; 0 broken images) and 20/20 tasks are now covered by a validated deterministic grading contract (verifiers + rubrics, in the How this was reviewed: the site was run standalone on a Linux host ( 1. Mechanical checks — FAIL (asset bundle), otherwise PASS
2. Visual fidelity — PASS with two fixes requested
3. Functional depth — PASS
4. Task quality — 19 accepted as-is, 1 re-anchored (T5)All facts are synthetic constants (no knowledge-shortcut risk); search cards show only city/region/country/postal (no
Cross-cutting findings for the author:
5. Required before merge
Fix-before-merge (non-blocking, site quality): 6. Grading contract (reviewer deliverable, in the
|
| task | pass | no-op | shortcut (first failing check) | wrong answer(s) | state mismatch |
|---|---|---|---|---|---|
| 0 | PASS | FAIL final_answer_nonempty |
FAIL visited_weather_phoenix |
FAIL ×2 | – |
| 1 | PASS | FAIL | FAIL searched_for_portland-me |
FAIL ×2 | – |
| 2 | PASS | FAIL | FAIL visited_hourly_seattle |
FAIL ×2 | – |
| 3 | PASS | FAIL | FAIL visited_daily_miami |
FAIL ×2 | – |
| 4 | PASS | FAIL | FAIL visited_weather_austin |
FAIL ×2 | – |
| 5 | PASS | FAIL | FAIL searched_for_springfield-mo |
FAIL ×2 | – |
| 6 | PASS | FAIL | FAIL visited_login_page |
FAIL | FAIL saved_rows_added |
| 7 | PASS | FAIL | FAIL visited_login_page |
FAIL | FAIL saved_rows_removed |
| 8 | PASS | FAIL | FAIL visited_login_page |
– | FAIL alerts_exactly_severe_and_rain_for_chicago |
| 9 | PASS | FAIL | FAIL visited_login_page |
FAIL ×2 | FAIL carol_unit_is_celsius |
| 10 | PASS | FAIL | FAIL searched_postal_code_94102 |
FAIL ×2 | – |
| 11 | PASS | FAIL | FAIL searched_for_new-orleans-la |
FAIL | – |
| 12 | PASS | FAIL | FAIL searched_for_los-angeles-ca |
FAIL ×2 | – |
| 13 | PASS | FAIL | FAIL visited_daily_boston |
FAIL ×2 | – |
| 14 | PASS | FAIL | FAIL searched_for_london-gb |
FAIL ×2 | – |
| 15 | PASS | FAIL | FAIL visited_login_page |
FAIL | FAIL saved_rows_added |
| 16 | PASS | FAIL | FAIL searched_for_portland-or |
FAIL ×2 | – |
| 17 | PASS | FAIL | FAIL searched_for_toronto-ca |
FAIL ×2 | – |
| 18 | PASS | FAIL | FAIL visited_register |
FAIL | FAIL exactly_one_new_user |
| 19 | PASS | FAIL | FAIL searched_for_new-orleans-la |
FAIL ×2 | – |
One verifier bug was found and fixed by the harness before the matrix run: the city-alias regex matched portland, or as a prefix of portland, oregon, so a swapped-values answer for T16 slipped through; aliases are now longest-first with a trailing word boundary (test_verify_16.test_wrong_answers_fail).
8. Evidence (exact commands / outputs)
$ hf download ChilleD/WebHarbor accuweather.tar.gz --repo-type dataset --revision refs/pr/66 --local-dir tarball
$ md5sum tarball/accuweather.tar.gz
1e2557d50076b2d6e9afb97fc1c81453
$ tar -tzvf tarball/accuweather.tar.gz | awk '{print $6}' | head -4
static/._images
static/images/
static/images/._weather-01.svg
static/images/weather-01.svg
(31 members: roots static/images, static/images/weather/ (empty), static/fonts, instance_seed; 15 are ._* AppleDouble)
$ python3 scripts/validate_asset_archive.py tarball/accuweather.tar.gz accuweather
ValueError: unexpected site root in archive: 'static/images'
$ python3 scripts/extract_asset_archive.py tarball/accuweather.tar.gz sites accuweather
ValueError: unexpected site root in archive: 'static/images'
$ curl -s https://huggingface.co/api/datasets/ChilleD/WebHarbor/discussions/66 | jq '{title,status}'
{"title": "feat(accuweather): add mirror asset bundle", "status": "open"}
$ cat .assets-revision | grep revision
revision: 65c479f894763f64c6073e0d180ebf542d1d2c02
$ tar -xzf tarball/accuweather.tar.gz -C tarball_x --exclude='._*' # manual extraction for the review only
$ md5sum tarball_x/instance_seed/accuweather.db
2701fb49e1edc178024785e6f8601870
$ cd sites/accuweather && rm -rf instance instance_seed && python -c "import app; ...copy instance/accuweather.db -> instance_seed/" # Dockerfile step
AccuWeather seed DB generated at build time.
$ md5sum instance_seed/accuweather.db -> 487f7e400f77f1db51289e9ce60deca0
$ (second generation in a scratch copy) -> 2c010823ea1cda4087049a68ad0e89d2 # random password salts
$ rm -rf instance && cp -a instance_seed instance && python -c "from app import app; app.run(host='127.0.0.1', port=45002, ...)" &
$ curl -so /dev/null -w '%{http_code}\n' http://127.0.0.1:45002/ -> 200
$ curl -s http://127.0.0.1:45002/_health -> {"ok":true,"site":"accuweather"}
$ md5sum instance/accuweather.db instance_seed/accuweather.db # after boot #1, boot #2, read-only Playwright audit
487f7e400f77f1db51289e9ce60deca0 instance/accuweather.db
487f7e400f77f1db51289e9ce60deca0 instance_seed/accuweather.db
$ md5sum instance/accuweather.db # after the stateful audit (save/remove/alerts/settings/register)
185d7b1d08733210c1676ea212f13344
$ uv run python audit_readonly.py # routes / links / responsive / search / 20-location DOM dump
routes: / 200, /search?q=Phoenix 200, /weather/phoenix-az 200, /hourly/seattle-wa 200, /daily/miami-fl 200,
/radar/new-orleans-la 200, /air-quality/london-gb 200, /login 200, /register 200, /about 200, /privacy 200,
/_health 200, /account -> /login?next=/account, /weather/nowhere-xx 404, GET /units/C 405,
/static/fonts/Solis-Regular.woff2 200 (22832 B), icon-gps.svg 200, weather-12.svg 200
home links: 12/12 return 200; news_items_are_links: [false x7]; font_loaded: true
responsive overflow_px: home@1280 0, home@768 192, home@390 0, home@320 0; weather/hourly/search @768/390/320: 0
broken_images: [] on all 16 combinations
$ python3 crosscheck.py # rendered values vs seed DB
locations audited: 20 | DOM-vs-DB mismatches: []
weather page dl keys: ['Humidity', 'Pressure', 'Visibility', 'Wind']
'uv' rendered anywhere on any audited page: False
phoenix hourly first 6: [['Now','103°','Sunny','2% precipitation'], ['1 PM','104°',...], ..., ['4 PM','107°','Showers','46% precipitation'], ...]
phoenix daily first 2: [['Today','102° / 92°','Sunny','6% precipitation'], ['Thu','103° / 91°',...]]
$ uv run python -m unittest discover -s sites/accuweather/verify/tests -p 'test_*.py'
Ran 293 tests in 48.802s
OK
$ uv run python sites/accuweather/verify/tests/run_matrix.py --site_dir sites/accuweather --python <venv>/bin/python --port 45002 --out runs
task 0: {'pass': True, 'noop': False, 'shortcut': False, 'wrong': [False, False]} ...
task 18: {'pass': True, 'noop': False, 'shortcut': False, 'wrong': [False], 'mismatch': False}
MATRIX OK
Screenshots (Playwright, 1280 px unless noted): home_1280.png, home_768.png (overflow), home_390.png, weather_phoenix.png, weather_portland_me.png, weather_london.png, hourly_seattle.png, hourly_toronto.png, daily_miami.png, daily_boston.png, radar_new_orleans.png, aq_london.png, aq_los_angeles.png, search_springfield.png, search_portland.png, search_nyc_empty.png, login.png, login_wrong_password.png, account_alice.png, account_alice_after_save_seattle.png, account_alice_after_remove_boston.png, alerts_chicago_bob.png, settings_carol_celsius.png, weather_ny_celsius.png, account_jamie_after_save_atlanta.png, matrix_task6_account_after_save.png, matrix_task8_alerts_saved.png, matrix_task18_account_atlanta.png.
Screenshots
All cited screenshots: https://github.com/JeremyJC67/WebHarbor/tree/pr-screenshots/reviews/accuweather
| homepage 1280 px | 768 px — 192 px horizontal overflow |
|---|---|
 |
 |
| Phoenix current weather (315 px ad gap under the tabs) | search "Springfield" — 3 results |
|---|---|
 |
 |
Follow-up review: accuweather — second pass (merge-blocker checklist)Reviewer: JeremyJC67 · Site author: @sarendis56 · Reviewed: Verdict: the two release-mechanics blockers from the first review still stand and still need you; everything else this pass found is already fixed on the review branch. This was a second pass against the maintainer's merge-blocker checklist — form-prefill leaks, answer-identifier grading, distractor/ordering defects, verifier-vs-recorder drift, application robustness, asset provenance, test hygiene and accessibility. It found 8 major and 17 minor defects that a normal review passes over. With the site owner's authorisation I fixed all 25 directly in four reviewer commits rather than only reporting them; every one is listed below so you can review what we changed on your behalf. The site itself came out of this well: no answer leaks through form prefill, no leaking static text, neutral search ordering with no truncation risk, no read-only route that writes, no JavaScript to degrade, and the seed/verifier contract fails closed on drift. 1. Still blocking — needs you (unchanged from the first review)
We cannot perform 1 or 2 — both need a write to your Hugging Face bundle. 2. Application robustness — fixed in
|
| Item | Before | After |
|---|---|---|
Open redirect on ?next= |
POST /login?next=https://evil.example/pwn → 302 Location: https://evil.example/pwn; //evil.example likewise |
302 Location: /account; a legitimate next=/settings still honoured |
Open redirect via Referer |
POST /save/<slug> and POST /units/<unit> with Referer: https://evil.example/hook → 302 to that URL |
falls back to a local path; a same-origin Referer is still honoured |
Fixed SECRET_KEY |
a session cookie signed with the literal accuweather-local-benchmark read /account as Alice, no password |
302 /login?next=/account; key is now ACCUWEATHER_SECRET_KEY env or secrets.token_hex(32), matching walmart_careers / rotten_tomatoes |
Tampered user_id |
a list/dict user_id raised in db.session.get → 500 |
200; current_user() ignores non-int ids |
/register validation |
email="", email="notanemail" and a 200 000-char name all persisted and returned 302 /account |
400; email/name/password bounded at 120/80/128. name of exactly 80 is accepted, 81 is rejected. jamie.lee@example.test (task 18) still registers — no email_validator, so RFC 6761 .test is fine |
/alerts/<slug> |
wrote any alert_type, including bogus-type and a 3 000-char value |
whitelisted to `severe |
/settings |
unit=K and an empty POST silently returned 200 |
400 |
MAX_CONTENT_LENGTH |
a 60 MB multipart POST was accepted (200) |
413; cap 64 KB |
| CSRF | absent, unlike 23 of the 25 sites; a cross-origin POST changed state | CSRFProtect plus a token in all six POST forms. POST /login without a token → 400, with one → 302 /account |
| Error pages | no errorhandler, no template; /nosuchpage rendered the bare Werkzeug page |
branded, styled templates/error.html for 400/403/404/405/413/500 |
PRAGMA foreign_keys |
0 |
1 |
| Orphan route | POST /units/<unit> existed with no UI control |
removed; POST /units/C → 404 |
Flask-WTF==1.2.2 is already in the image's global pip install; we added it to sites/accuweather/requirements.txt.
3. Determinism and provenance — fixed in dcbf85e and 5eb5a77
| Item | Before | After |
|---|---|---|
| Build-time seed reproducibility | three generations gave three md5s (487f7e40…, 2c010823…, 58b41314…) because generate_password_hash salts randomly |
the four benchmark hashes are frozen constants; three fresh generations all give e7eaa11e753534719dea8c5d812e0680 |
| Homepage grid ordering | Location.query.limit(8) with no ORDER BY, relying on SQLite insertion order — and those 8 slugs are load-bearing, since every other location is reachable only via search |
.order_by(Location.id); the rendered order is unchanged |
| Asset hashes | none recorded | sha256 for all 8 assets in provenance.json |
| Font licence | no notice anywhere | new site README.md. The font ships with SFNT name ID 0 (© Copyright AccuWeather, 2019. All rights reserved.) and name ID 14 preserved verbatim; name ID 13 is absent upstream, not stripped by you. Added the removal steps, the recorded fallback stack (Solis, Arial, sans-serif) and non-affiliation plus trademark notices |
Note on the checklist's "byte-reproducibility only holds within one SQLite runtime": that is now separable. Within one runtime the file is byte-identical; across runtimes compare the row-level catalog_fingerprint (05807dac…ff3c), which is stable. We measured on SQLite 3.50.4 and could not measure the container's version — no Docker was run in this pass.
4. UI and accessibility — fixed in e901cba
| Item | Before | After |
|---|---|---|
| Responsive | homepage overflowed 192 px at 768 px (.layout keeps fixed 632px 304px until the 700 px breakpoint). Offender ASIDE.card, right: 960 vs clientWidth 768, checkVisibility() == true, no scroll ancestor — a real overflow, not a deliberate scroller |
960 px breakpoint collapses the grid; 60/60 route × width cells clean at 1440/768/390/320 |
| Ad gap | .tabs{margin:0 0 315px} left a dead band under the tabs on every location page |
24 px / 18 px |
| Active tab | hardcoded to .tabs a:first-child, so /hourly highlighted "Current Weather" |
marked by request.endpoint, with aria-current="page"; on /hourly the active tab is now Hourly |
| Unit letter | Celsius mode rendered a bare 26° |
104°F / 26°C on the current, hourly, daily and account surfaces |
| Unused column | location.uv was stored for all 20 rows and never rendered — the reason we re-anchored task 5 |
rendered as a UV Index row on the weather page. We kept the re-anchored task 5 wording (it is validated and passing); restoring the original UV wording is now possible if you prefer it |
| Dead items | 7 "Weather News" / "Top Stories" items were static text | links |
| Contrast | white on the brand orange #f05514 = 3.492:1 and #f15a18 = 3.378:1 (buttons, .premium, the breaking strip), .news-grid small = 4.139:1 — all under the 4.5:1 body-text bar |
introduced an --action token #c2410c (5.178:1) for interactive surfaces, keeping the brand orange for decoration only |
| Focus | the site defined no focus styling at all | --focus #1a73e8, which clears 3:1 against both the white page and the #222 header |
Contrast was measured by sampling rendered pixels, not by reading CSS. Worth saying plainly: our first sampler reported four extra failures that were not real — it had picked the tab's orange underline, the page background outside a button's rounded corner, the blurred radar gradient and an input border instead of the glyphs. A glyph-anchored re-measure cleared all four. In particular the orange Accu mark on the dark header passes at 4.771:1 and we left it alone.
5. Grading contract — two defects in our own artifact, fixed in 94abf80
These were ours, not yours, but they affect how your site is graded.
screenshots_decodeonly required width and height>= 1, so a run whose every screenshot was a 1×1 PNG passed with "all checks passed". Now a 200×150 minimum.check_search_surfacesaccepted any query token matching city/region/country/postal. Every US location carries country "United States", so a singleq=United Statessearch satisfiedsearched_for_springfield-mowithout the agent ever disambiguating the three Springfields. The gate now ignores tokens shared by more than half the catalog, derived from the catalog itself rather than a hardcoded stop-list.
6. What we checked and found clean
Form prefill (login and register arrive empty; no <select> exists anywhere; the alerts checkboxes and the settings unit radio initialise from the user's own stored state, which is the legitimate edit-a-preference exemption — the seed has zero alert rows so task 8's boxes render unchecked, and task 9's target Celsius is the opposite of the stored Fahrenheit); no leaking placeholder / aria-label / title; review-reports/ never enters the image because the Dockerfile only COPYs sites/ plus four files; verifiers enumerate answer fields explicitly, so the "subtracted answer" bug class does not apply; search ordering is neutral (-score, city, region) with no limit, so no truncation risk; read-only routes do not write (md5 unchanged across GETs, and it does change after one POST, so the check fires); logout is POST-only (GET/HEAD → 405); malformed verifier input produces a structured FAIL, never a traceback; a missing after_db fails closed; negated answers are rejected; no JavaScript at all, so no-JS degradation is total.
7. Commands
$ python -m py_compile app.py _health.py verify/verify_lib.py verify/verify_*.py
ok
$ python -m unittest discover -s sites/accuweather/verify/tests -p 'test_*.py'
Ran 297 tests in 47.247s
OK
$ python sites/accuweather/verify/tests/run_matrix.py --site_dir sites/accuweather \
--python <venv>/bin/python --port 45002 --out runs
task 0: {'pass': True, 'noop': False, 'shortcut': False, 'wrong': [False, False]}
...
task 19: {'pass': True, 'noop': False, 'shortcut': False, 'wrong': [False, False]}
MATRIX OK
$ for i in 1 2 3; do rm -rf instance instance_seed; python -c "import app; ..."; md5sum instance_seed/accuweather.db; done
e7eaa11e753534719dea8c5d812e0680
e7eaa11e753534719dea8c5d812e0680
e7eaa11e753534719dea8c5d812e0680
$ python -c "import verify_lib as L; print(L.catalog_fingerprint('instance_seed/accuweather.db'))"
05807dac4bd73c96fa663d5950d2fe98142590875f841f140ae1dad116abff3c # == pinned constant
$ uiaudit2.py # 4 widths x 15 routes, Playwright
OVERFLOW: [] CONSOLE: [] BROKEN IMAGES: [] EXTERNAL REQS: []
CLEAN ROUTE/WIDTH CELLS: 60 of 60
The 297-test harness and the 20-task real-browser matrix were re-run after every change; the matrix is what proves the CSRF tokens and the new 400s did not break a single task workflow.
8. Two content items we did not change
- The forecasts are formula-generated — today's high is
current − 2, the hourly peak is 4 PM and the lowest daily low is Saturday, for all 20 cities. Tasks 2/3/13/17 are list-scan tasks, so once an agent has seen one city the pattern generalises: the tasks stay correct but stop measuring scanning. - Catalog breadth — only Portland (×2) and Springfield (×3) have same-name distractors; every other task query returns exactly one card.
We left both alone deliberately: rewriting the forecast data would invalidate the frozen CATALOG_FINGERPRINT and all 20 verifiers, which is your call rather than ours. Both are now written down in the new sites/accuweather/README.md scope section so the limitation is explicit rather than implicit.
9. Not tested
No Docker was run in this pass, so the image build, the container-built seed, POST /reset byte-identity, reset-all and the container's SQLite version are unverified since 6e2e24b (the first pass ran them green on that commit with hand-placed icons). Our changes add no build step and Flask-WTF is already in the image. We also could not re-download the font or the SVGs to prove a no-modification clause by hash — accuweather.com is unreachable from the review host — so we recorded the observed sha256 of all 8 assets instead.
Container re-verification of this branch (5eb5a77, full 25-site image)
Run after the four reviewer commits, on the same host as the first pass. The HF bundle still cannot be
fetched by scripts/fetch_assets.sh (blocker 1), so the eight icons were placed by hand before the build;
everything else is the standard pipeline.
| check | result |
|---|---|
docker build |
ok |
| HTTP sweep, alt ports 44000-44024 | 25/25 return 200 |
GET /health |
ok, 25/25 alive and ready, accuweather on internal 40024 |
POST /reset/accuweather → md5 instance vs instance_seed |
bad5a81520e92910f2cb262fa5dde0b1 both |
| dirty request then reset | still bad5a815… |
docker restart then md5 |
still bad5a815… |
POST /reset-all |
ok, 25/25 ready, 1.54 s |
verifier no-op matrix, in-container DBs via docker cp |
20/20 FAIL, 0 errors |
walmart_careers / rotten_tomatoes registry + docs tests |
4 passed, 3,343 subtests |
sites/accuweather suite inside the image |
296 passed, 1 skipped, 45 subtests |
The container's seed is still the build-generated one (bad5a815…), which is the expected consequence of
.build-generated-seed; freezing the four password hashes in this branch makes that file reproducible
within a single SQLite runtime, and the row-level catalog fingerprint is stable across runtimes.
Two release-mechanics items remain and both need @sarendis56: repack accuweather.tar.gz so its members
are rooted at accuweather/ (no static/fonts, no instance_seed, no ._*), and re-pin
.assets-revision to the merged revision once HF #66 lands.
Tooling disclosureAdded after the fact, because it belongs in the public record and not only in our internal notes. The second-pass audit had a general-purpose LLM available to it as a judgment aid — used for Where it changed what we did on this PR: asked to triage the ten application defects it returned No call log was kept, so I cannot enumerate every consultation — that is the honest limit of this |
2 participants
Summary
Adds a deterministic AccuWeather mirror as the next WebHarbor site on port
40024.Fidelity
The UI was refined against recorded desktop and mobile references. It now uses the compact weather canvas, dark navigation, alert strip, pale-blue page background, realistic weather cards, forecast tabs, advertising spacing, and responsive location/news lists seen in the source site.
Screenshots
Assets
sites/accuweather/provenance.json.assets-revisionremains unchanged while the HF PR is open. A follow-up commit will pin the immutable HF merge revision.Validation
Seeded state
Detailed audit report