Skip to content

Review: AccuWeather mirror + task verifiers (site by @sarendis56, verifiers by reviewer) - #98

Open
JeremyJC67 wants to merge 9 commits into
aiming-lab:mainfrom
JeremyJC67:review/pr-93-accuweather
Open

Review: AccuWeather mirror + task verifiers (site by @sarendis56, verifiers by reviewer)#98
JeremyJC67 wants to merge 9 commits into
aiming-lab:mainfrom
JeremyJC67:review/pr-93-accuweather

Conversation

@JeremyJC67

Copy link
Copy Markdown
Contributor

What this PR carries

  • 5217874 feat(accuweather): add high-fidelity mirror@sarendis56's site commit from feat(accuweather): add high-fidelity mirror #93, unchanged (authorship preserved).
  • 5bb9889 chore(accuweather): docs/tests sweep for site 25 and asset markers — README / AGENTS / CONTRIBUTING / CLAUDE / agent_demo README moved to 25 sites and 40000-40024; walmart_careers + rotten_tomatoes registry tests accept the 25-site registry (the docs-range test now derives the range from control_server.SITES, as feat(webmd_doctor): add WebMD Doctor mirror (site 25, port 40024) #95 does); sites/accuweather/.build-generated-seed + .requires-images so fetch_assets.sh / check_assets.sh treat the site like rotten_tomatoes / walmart_careers; requirements.txt pins the packages app.py actually imports.
  • feat(accuweather): deterministic verifiers, judge rubrics and task re-anchor — the reviewer grading contract (below).

Full review with evidence is posted on #93. Verdict there: request changes for release mechanics (HF bundle layout, .assets-revision pin, port slot), not for the code.

Grading contract

sites/accuweather/verify/verify_lib.py + verify_0.py … verify_19.py, verifier_path and judge_rubric on every tasks.jsonl row, no answer key. Deterministic-only (--no_llm True accepted for parity; LLM helpers exist for API parity and no verdict depends on them):

  1. Package identity — task id, agent_done, non-empty answer, every recorded URL on the same loopback host+port as start_url, screenshots decode as PNG.
  2. Navigation gates — the page(s) that render the requested facts (both for comparisons); a /search?q= visit that would surface the target when it is not linked from the homepage grid (literal 94102 for task 10); /login + typed account email + required page order for stateful tasks.
  3. Answer facts — label-aware numeric matchers that catch value/label swaps, unit adjacency, whole-number matching, phrase conditions (CloudyMostly cloudy), clock times, day labels, comparison-winner attribution; all negation-aware.
  4. SQLite snapshot contract — six tables + columns, seed row counts, pinned catalog fingerprint (salt-independent), immutable location/forecast/hourly; read-only tasks leave user/saved_location/alert identical; tasks 6/7/8/9/15/18 must show the exact allowed delta (new user's password verified with hashlib) and nothing else. Snapshots from <run_dir>/initial.db|after.db, --initial_db/--after_db, or docker cp from $WH_CONTAINER; missing/invalid input fails closed.

Task 5 was re-anchored: "UV index" is not rendered on any page (the column exists but no template shows it), so the row now asks for the postal code (search results), the pressure reading (/weather/springfield-mo) and the air-quality value (/air-quality/springfield-mo) — same three-page workflow. All other 19 task texts are unchanged.

Validation (CONTRIBUTING §C)

  • verify/tests/ (stdlib-only fixture DB reproducing the frozen seed fingerprint, agent.py-format trajectories): 293 tests OK — genuine PASS, run-dir snapshot discovery, no-op, wrong task id, shortcut, wrong answers, alternative phrasings, unterminated run, mixed origin, corrupt PNG, schema/catalog/seed drift (fail closed), collateral writes / state mismatch.
  • verify/tests/run_matrix.py — real Chromium against a fresh site per task, live DB snapshots, five variants each: 20/20 tasks: pass = PASS; no-op, shortcut, wrong answer(s) and (stateful) state-mismatch = FAIL, each on the intended first failing check.
  • Site: byte-identical instance/ vs instance_seed/ (md5 487f7e40…) after two boots and after driving every read-only page; all 20 locations' rendered values cross-checked against the seed DB (0 mismatches).

verify/tests/ is excluded from the image by .dockerignore.

Required before merge (from the review on #93)

  1. Repack accuweather.tar.gz with ./scripts/extract_assets.sh <staging> accuweather (members rooted at accuweather/, no static/fonts, no instance_seed, no ._*) and re-upload to HF Add Recreation.gov task verifiers (site by @Chubi-alt, verifiers by reviewer) #66 — the current bundle is rejected by scripts/validate_asset_archive.py, so fetch_assets.sh and a clean Docker build fail.
  2. Repin .assets-revision to the merged HF sha once Add Recreation.gov task verifiers (site by @Chubi-alt, verifiers by reviewer) #66 lands.
  3. Port 40024 is also claimed by feat(webmd_doctor): add WebMD Doctor mirror (site 25, port 40024) #95 (webmd_doctor) — maintainer assigns the slot at merge.

Full 25-site Docker environment built from this branch with the bundle's icons copied in by hand (the bundle itself is rejected by validate_asset_archive.py): build ok (4.69 GB), 25/25 sites 200, /health 25/25, POST /reset/accuweather byte-identical (bad5a815…, the build-generated seed) before/after a dirty request and after docker restart, /reset-all 25/25 in 1.52 s, verifier no-op matrix in the container 20/20 FAIL, registry tests green, sites/accuweather suite 292 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP

sarendis56 and others added 8 commits September 10, 2026 03:06
- README / AGENTS / CONTRIBUTING / CLAUDE / agent_demo README: 25 sites,
  ports 40000-40024 (the PR bumped Dockerfile and the registries but not
  the shared docs).
- walmart_careers and rotten_tomatoes registry tests: accept the 25-site
  registry; the shared-docs port-range test now derives the range from
  control_server.SITES (same approach as PR aiming-lab#95) instead of a hardcoded
  count.
- sites/accuweather: add .build-generated-seed and .requires-images so
  fetch_assets.sh / check_assets.sh treat the site like rotten_tomatoes
  and walmart_careers (the Dockerfile regenerates instance_seed at build
  time; weather icons come from the HF bundle).
- sites/accuweather/requirements.txt: pin Flask-SQLAlchemy / SQLAlchemy /
  Werkzeug / Jinja2 that app.py imports (the image already installs them).

Port 40024 is provisional: PR aiming-lab#95 (webmd_doctor) claims the same slot.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…-anchor

Reviewer grading contract for the 20 AccuWeather tasks (CONTRIBUTING
"Reviewer role" B/C):

- sites/accuweather/verify/verify_lib.py: merriam_webster-style API with
  walmart_careers hardening: package identity (task id, agent_done,
  same loopback host+port for every recorded URL, decodable PNGs),
  navigation gates incl. search-surfaces-target, label-aware numeric
  matchers that catch value/label swaps, negation-aware phrase matchers,
  comparison-winner attribution, SQLite snapshot contract (table set,
  columns, row counts, pinned catalog fingerprint, immutable catalog
  tables, read-only tables, exact stateful row deltas), werkzeug hash
  verification with hashlib only. LLM helpers kept for parity; no
  verdict depends on them.
- verify_0.py .. verify_19.py: one deterministic verifier per task,
  ground truth hardcoded (never in tasks.jsonl).
- tasks.jsonl: verifier_path + judge_rubric on every row; task 5
  re-anchored from "UV index" (not rendered on any page) to the pressure
  reading + air-quality value, keeping the 3-page workflow.
- verify/tests: stdlib-only harness (fixture DB reproduces the frozen
  seed fingerprint, agent.py-format trajectories) covering genuine PASS,
  no-op, wrong task id, shortcut, wrong answers, alternative phrasings,
  unterminated run, mixed origin, corrupt PNG, schema/catalog/seed
  drift, collateral writes and stateful mismatch (293 tests), plus
  run_matrix.py which drives every task through Chromium and grades
  pass / noop / shortcut / wrong / mismatch run dirs with live DB
  snapshots (20/20 tasks OK on carcinoma).
- verify/README.md: the contract per task.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…ndling

Second-pass audit findings against the maintainer merge checklist. Each item
was reproduced with a request and re-verified after the fix.

- Open redirect: POST /login?next=https://evil.example returned 302 to that
  URL; /save/<slug> and /units/<unit> did the same via request.referrer.
  Added local_path()/local_referrer() so only same-origin paths are honoured.
- Fixed SECRET_KEY "accuweather-local-benchmark" let a forged session cookie
  read /account as any user. Now os.environ ACCUWEATHER_SECRET_KEY or
  secrets.token_hex(32), matching walmart_careers / rotten_tomatoes.
- A tampered cookie carrying a non-scalar user_id raised on db.session.get and
  returned 500; current_user() now ignores anything that is not an int.
- /register persisted an empty email, "notanemail" and a 200 000-character
  name, all returning 302 to /account. SQLite does not enforce VARCHAR length,
  so the app now bounds email/name/password and returns 400.
- /alerts/<slug> wrote any alert_type the client sent, including a 3 000-char
  value; now whitelisted to severe/rain/temperature, else 400.
- /settings silently ignored unit=K; now 400.
- No MAX_CONTENT_LENGTH: a 60 MB multipart POST was accepted. Now 64 KB.
- No CSRF protection, unlike 23 of the 25 sites. Added CSRFProtect and a token
  to every POST form (login, register, alerts, settings, save, logout).
- No errorhandler and no error template: 404 rendered the bare Werkzeug page.
  Added templates/error.html and handlers for 400/403/404/405/413/500.
- Removed the orphan POST /units/<unit> route (no template ever rendered a
  control for it).
- weather.html/account.html also pick up the unit letter and UV row here
  because these templates are single-line and their CSRF token cannot be
  staged separately.

Flask-WTF is already installed in the image; added to requirements.txt.
Verified: 297-test unit harness OK, 20/20 real-browser task matrix OK.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
- The homepage overflowed 192 px horizontally at 768 px because .layout keeps
  fixed 632px/304px columns until the 700 px breakpoint. Added a 960 px
  breakpoint that collapses the grid; 60/60 route x width cells are now clean.
- .tabs carried margin-bottom:315px (an ad placeholder), leaving an empty band
  between the tabs and the card on every location page. Now 24 px / 18 px.
- The active tab was hardcoded to .tabs a:first-child, so /hourly highlighted
  "Current Weather". The macro now marks the tab by request.endpoint and sets
  aria-current.
- Temperatures rendered no unit letter, so Celsius mode showed a bare "26deg".
  Added the letter on the current, hourly, daily and account surfaces.
- Contrast, measured by sampling rendered pixels rather than reading CSS:
  white on the brand orange #f05514 is 3.49:1 and #f15a18 is 3.38:1, both
  under the 4.5:1 body-text bar, and .news-grid small was 4.14:1. Introduced
  an --action token (#c2410c, 5.18:1) for buttons, .premium and the breaking
  strip, keeping the brand orange for decoration only; the orange "Accu" mark
  on the dark header measures 4.77:1 and is unchanged. Added a :focus-visible
  ring (--focus #1a73e8) that clears 3:1 on both the white page and the #222
  header.
- Made the seven dead "Weather News" / "Top Stories" items real links.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…ch gates

Two gaps in our own grading contract, both found by probing the verifiers
rather than reading them.

- screenshots_decode only required width and height >= 1, so a run whose every
  screenshot was a 1x1 PNG passed verify_0 with "all checks passed". The
  checklist asks for "decodable and above a minimum size"; now 200x150.
  The test fixture PNG moves from 2x2 to 320x200 accordingly.
- check_search_surfaces accepted any query token intersecting the target's
  city/region/country/postal. Every US location carries country "United
  States", so a single q=United+States search satisfied
  searched_for_springfield-mo without the agent disambiguating the three
  Springfields. search_surfaces now ignores tokens shared by more than half
  the catalog, derived from CATALOG rather than a hardcoded stop-list, so
  "united"/"states" no longer count while "springfield" and "65806" do.

Added negative tests for both (test_stub_screenshot_fails,
test_catalog_wide_query_does_not_satisfy_the_search_gate,
test_screenshots_reject_stub_sizes, test_search_gate_ignores_catalog_wide_tokens).
Harness 293 -> 297 tests, OK; 20/20 real-browser matrix still OK.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
- provenance.json gains a sha256 for all eight captured assets, so a future
  re-download can be compared objectively.
- New site README recording what is deliberately simplified (no JavaScript,
  decorative radar, formula-generated forecasts), how the seed is made
  byte-reproducible, and the font notice: Solis-Regular.woff2 ships with SFNT
  name ID 0 ("(c) Copyright AccuWeather, 2019. All rights reserved.") and name
  ID 14 preserved verbatim, carries no licence grant, and can be removed by
  deleting the file and the @font-face rule since the stylesheet already
  declares the Arial fallback. Adds non-affiliation and trademark notices.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…rt 40026)

main gained fedex (40024) and webmd_doctor (40025). Merged rather than rebased so every
contributor and reviewer commit on this branch is preserved verbatim. Shared registry,
Dockerfile EXPOSE, docs and every site-local port reference move to index 26 / port 40026;
the slot stays provisional and the maintainer reassigns it at merge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
@sarendis56

Copy link
Copy Markdown
Contributor

Contributor asset follow-up for the remaining archive-layout finding:

HF PR #66 is updated at 0a73c1c1ac2e47513389a8a1a67601f75c8c4150. It now contains only accuweather/static/images/ (eight SVG icons), with no unsupported fonts root, stale seed database, or AppleDouble/Finder metadata. This matches this branch's build-generated seed contract.

I downloaded the uploaded bundle and confirmed it passes validate_asset_archive.py (10 managed members). Your code, tasks, and verifiers are untouched. HF merge and the subsequent .assets-revision pin remain the release steps.

…erge

accuweather ships no instance_seed: its seed is generated at image build time. Merging the
26-site main took upstream's Dockerfile wholesale, which dropped that RUN step, so
websyn_start.sh failed at 'cp -a .../instance_seed' and the container exited 1 before the
control plane came up. Step restored verbatim ahead of EXPOSE.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
@JeremyJC67

Copy link
Copy Markdown
Contributor Author

Independently validated, and it matches your report. Downloaded refs/pr/66 at 0a73c1c1 on the review host and ran the repo's own validator:

bytes: 5524      md5: 522284a9610b5699d3982b48878b3623
members: 10      AppleDouble/DS_Store: 0
archive roots: accuweather/        second level: accuweather/static
validate_asset_archive.py -> validated 10 managed members for accuweather

That clears the blocker. The archive is rooted at accuweather/, carries only the managed static/images root, and no longer ships the unsupported fonts root or the stale seed database — which is right, since this site generates its seed at image build time.

One thing worth knowing that is on my side, not yours: when I merged the 26-site main into this review branch, my conflict resolution took upstream's Dockerfile wholesale and silently dropped the accuweather build-time seed step. The container then died at boot on cp -a .../instance_seed before the control plane came up. I have restored that step verbatim (e9e0809) and the site is being re-verified now. Nothing in your code or assets caused it.

Remaining is release mechanics only: a maintainer merges HF #66, then I pin .assets-revision to the merge sha.

@JeremyJC67

Copy link
Copy Markdown
Contributor Author

Rebased onto the 26-site main — this branch is now site 27 / port 40026

main merged FedEx (40024) and WebMD Doctor (40025) overnight, which put every open site
branch into conflict. This branch has been brought up to date by merging main rather than
rebasing, so every contributor and reviewer commit is preserved verbatim, and re-slotted to
index 26 / port 40026: registry, Dockerfile EXPOSE, the shared docs and every
site-local port reference follow. The slot is provisional as always — say the word at merge
time and I will re-slot in whatever order you land things.

The whole environment was then rebuilt and re-verified at the new slot:

check result
HTTP sweep 27/27 return 200
GET /health ok, 27/27 alive and ready
POST /reset/accuweather → md5 cbddbe8704539f93e5be0f5f87959099, identical to the seed
POST /reset-all ok, 27/27 ready, 1.63 s
verifier no-op matrix 20/20 FAIL
registry + docs tests 4 passed, 3,343 subtests

Worth recording because it was my fault, not yours: my first attempt at this merge resolved the
Dockerfile conflict by taking upstream's copy wholesale, which dropped this site's build-time
seed step. Since accuweather ships no instance_seed, websyn_start.sh then died on
cp -a .../instance_seed and the container exited six seconds after start. The step is restored
verbatim in e9e0809 and the run above is with it in place.

Still blocked only on HF #66 being merged, after which I repin .assets-revision.

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.

2 participants