Review: Amtrak mirror + task verifiers (site by @Lxr-max, verifiers by reviewer) - #99
Review: Amtrak mirror + task verifiers (site by @Lxr-max, verifiers by reviewer)#99JeremyJC67 wants to merge 11 commits into
Conversation
…web -> 40024 Reviewer follow-up to the rebase of PR aiming-lab#39 onto main: the contributor's registration now appends amtrak as the 25th site (index 24, port 40024). Sweep the shared docs (README, AGENTS, CONTRIBUTING, CLAUDE, agent_demo/README) and the registry assertions in the walmart_careers / rotten_tomatoes test suites to the new 40000-40024 range, and point every sites/amtrak/tasks.jsonl row at http://localhost:40024/. The port is provisional until merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…tion harness Reviewer deliverable for PR aiming-lab#39 (CONTRIBUTING "Reviewer role", parts B/C): - sites/amtrak/verify/verify_lib.py: package validation (task id, non-empty answer, completed run, loopback single-origin URLs, decodable PNGs), /booking/results query-parameter gates, ordered-workflow gates, affirmative answer matchers (money / duration / date / code / track / stop order), and SQLite snapshot contract (22-table schema, seeded counts, immutable catalog, exact mutable-row deltas). search_logs is excluded because the site writes it on every search/results request. No verdict depends on an LLM. - verify_0.py .. verify_17.py: one verifier per task with hardcoded ground truth (never in tasks.jsonl); 8 and 17 check DB after-state. - verify/tests: offline unittest matrix (genuine / no-op / shortcut / wrong / other-task / unterminated / foreign-origin / corrupt-png / schema-tamper / collateral-write / state-mismatch / over-reach) and live_matrix.py, a Playwright harness that records agent.py-shaped runs against the running mirror and prints the same matrix. Both excluded from the image by .dockerignore (sites/*/verify/tests/). - verify/README.md documents the per-task contract. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…tasks 2, 3, 5, 16 Adds verifier_path + judge_rubric (rules only, no answers) to all 18 rows. Task wording changes made during review (details in the review comment): - Amtrak--2 / --3: the original flows asked the agent to "reach fare selection" with no verifiable answer; now pick the fastest / cheapest legs and report the per-traveler Business / Value fare shown on the fare page. - Amtrak--5: "next upcoming trip" was ambiguous (two bookings share the earliest date and the dashboard lists Completed trips first); now asks for the upcoming Denver trip on My Trips. - Amtrak--16: rejected (asked whether the mirror connects to a real payment system, a meta question answered by the site banner); replaced by a help-search task anchored on the refunds article title and category. - Amtrak--0, 1, 4, 6, 7, 8, 13, 15: small clarifications (what to report, demo credentials, fare class) so the answer is page-specific and gradable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…nt dashboard Two defects found by the merge-blocker audit, both in the contributor's app.py. log_search() committed a SearchLog row from /search, /help?q= and /booking/results. Those are GET pages, so every agent step dirtied the database: after a 44-page read-only sweep the instance DB no longer matched instance_seed (md5 be90a3ce vs cb4b6a59, 40 rows in search_logs), which breaks reset byte-identity and makes read-only state grading meaningless. The write is removed; the table stays because the 22-table schema is part of the seed contract. account() ordered "Next trips" by departure_date without filtering is_upcoming, so the card led with five Completed Apr 14-17 trips before the first Confirmed one. Both it and the unused dashboard_upcoming_trip() helper are replaced by a single upcoming_bookings_for() that filters before it limits. Verified: instance md5 == instance_seed md5 after a full browse; live validation matrix 90/90, offline matrix green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
Input validation (/booking/results, /schedules, /booking/multi-city) - the handlers
defaulted rather than validated, so bad input succeeded silently:
- /booking/results with no query string at all searched NYP->WAS on the reference
date and returned 13 options; an unparseable departure_date silently became
2026-04-18; unknown fare_class/sort were accepted.
- station_lookup() fell back to a substring match, so origin=a resolved to Albany
and origin=New to Detroit. It is now exact (code, "(CODE)" label, or an
unambiguous station/city name) and an unresolvable value is a 400.
- passengers came through parse_int() unbounded: passengers=100000 rendered
100000 passenger cards, a 245 MB response. Replaced by bounded_int/bounded_passengers
(1..8); out-of-range and non-numeric values are a 400.
Robustness:
- SECRET_KEY was the constant "webharbor-amtrak-demo-key", so a session cookie
could be forged for any account: signing {_user_id: bob} with it returned Bob
Castillo's /account with no password. Now per-process random, overridable via
AMTRAK_SECRET_KEY.
- /logout answered GET and HEAD, so any prefetcher could sign an agent out
mid-task. POST only; GET is 405.
- No CSRF token exists on any form and a cross-origin POST really did rewrite the
profile. Added SameSite=Lax plus an Origin check on unsafe methods (403).
- MAX_CONTENT_LENGTH was unset and a 60 MB multipart POST was accepted; capped at 2 MB.
- PRAGMA foreign_keys was 0, so an orphan ticket row inserted happily. Enabled on connect.
- No commit was wrapped, so an IntegrityError left the session in PendingRollbackError.
register() and booking_checkout() now roll back and flash.
- login?next= was an open redirect (it followed https://example.com/pwned);
now same-site relative targets only.
- 404s were bare Werkzeug pages; added branded 400/403/404/405/413/500 handlers.
- register() defaulted a blank surname to "Traveler", inventing a name for the
account; first and last name are now required.
Verified: live validation matrix 90/90 with 0 deviations, offline matrix green,
instance/instance_seed byte-identical after browsing, all 18 task URLs unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
Form prefill (merge checklist section 1) - fields the tasks tell the agent to supply
arrived pre-filled in a fresh session:
- booking_defaults() shipped origin=NYP, destination=WAS and both dates into the
search widget on / and /booking/search. Tasks 0, 1 and 17 all ask for exactly
"NYP to WAS", so the station pair was typed for the agent. Now empty.
- booking_multi_city.html hardcoded value="2026-04-18"/"2026-04-20"/"2026-04-22"
into the three leg dates - the exact three dates task 3 dictates. Now empty.
- "Start from this station" / "Search fares" / "Search later" / "Search this route"
linked straight to /booking/results with a hardcoded destination=WAS and
departure_date=2026-04-20; from the NYP station page that one click reproduced
the whole of task 0. They now open the empty search form.
UI:
- The homepage feature tiles inherited color:white from .page-hero onto a #e7f3f7
tile: pixel-sampled contrast 1.131:1. Adding an explicit colour takes it to
13.911:1 (WCAG AA needs 4.5:1).
- .media-card .media-top img centre-cropped 1200x560 hero SVGs whose titles start
at x=72, cutting "Acela Express" to "la Express" on 18 of 24 sampled cards.
object-position: left center makes the visible band [0, 899] and all 18 legible.
- The departure tables overflowed the page at 320 and 390 px (/stations/* 434 and
441 px, /schedules 494 px). Grid items default to min-width:auto, so the table's
min-content width stretched its track; min-width:0 plus overflow-x:auto on
.info-table-wrap scrolls the table instead of the document. 0 overflow findings
across 4 widths x 21 routes.
- The option card printed 2h 50m (departure to arrival) while its own segment tag
printed 2h 46m (running time) for the same train. Both are correct but unlabelled;
they now read "2h 50m total" and "2h 46m on board".
- Sign out became a POST form, matching the logout route.
- Added templates/error.html for the branded error handlers.
Verified: live validation matrix 90/90, UI probe 0 overflow / 0 console errors /
0 failed requests / 0 external requests at 1440, 768, 390 and 320 px.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…-only
verify_lib used to exclude search_logs from every immutability check, because the
site committed a row on each /search, /help?q= and /booking/results request. That
write is gone, so the tolerance is now a hole: a route that started writing again
would grade clean. search_logs joins the read-only table set (LOG_TABLES), and
tasks 8 and 17 assert it too - the only writes they authorise are the profile
update and the new booking.
The screenshot check accepted any decodable PNG including a 1x1, so a forged
thumbnail satisfied it. Added a 64x64 floor (MIN_SCREENSHOT_PX) and moved the
offline harness off its 1x1 fixture onto a 320x200 one.
New negative cells, each confirmed to fail on the named check:
- a search_logs row in the after-state of read-only tasks 0, 10 and 16
(read_only_search_logs_unchanged) and of stateful tasks 8 and 17
(search_logs_unchanged);
- 1x1 screenshots (screenshots_decode).
Offline matrix 15 tests OK (was 13); live matrix 90/90 with 0 deviations.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
/trip-lookup shipped placeholder="alice.j@test.com" and placeholder="Jordan". Jordan is benchmark user Alice Jordan's real surname, and on this form a surname is an accepted alternative to the email for authenticating the lookup - so the page handed a visitor one of the two credentials it checks. The booking-code placeholder "Example: ALXXXX" also exposed the seeded code prefix. All three are now generic descriptions of the field. Task 6 supplies the code and the email in its own text, so the task is unaffected; the live matrix cell was re-run to confirm. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
…026) 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
Rebased onto the 26-site
|
| check | result |
|---|---|
| HTTP sweep | 27/27 return 200 |
GET /health |
ok, 27/27 alive and ready |
POST /reset/amtrak → md5 |
cb4b6a5939b663017d9774f2d3a86884, identical to the seed |
POST /reset-all |
ok, 27/27 ready, 1.50 s |
| verifier no-op matrix | 18/18 FAIL |
| offline verifier suite in the image | 15 passed, 119 subtests |
Audit assertions hold: GET /logout 405, branded 404 (10,664 bytes), and the input validation
that replaced the silent fallbacks — an empty booking search, origin=a, and
passengers=100000 all return 400. Three read-only page loads leave the DB byte-identical,
which is the log_search() fix confirmed in the image.
The asset blocker is unchanged and is the only thing left: no ChilleD/WebHarbor revision
contains amtrak.tar.gz. It needs an HF PR against the central dataset before this can merge.
|
The asset blocker on this PR is now open rather than absent — I opened it on @Lxr-max's behalf, since it was the one remaining item that no maintainer could act on and the site author has not been able to. HF PR #77 adds @Lxr-max — if you would rather open this yourself, say so and I will close #77; the point was only to unblock the queue, not to take the step from you. With that, everything on this PR that was not a maintainer action is done: the branch has merged the 26-site |
This PR is the review vehicle for #39. It keeps @Lxr-max's two site commits intact
(authorship preserved, rebased onto current
mainso the PR is mergeable) and addsthe reviewer deliverable: a deterministic grading contract for every task, validated
against the running mirror. Full review with evidence is posted on #39.
What is in here
2af6a47Add Amtrak WebHarbor mirrorsites/amtrak/), registration as the 25th site on port 40024 (rebased; the PR's stale 15/16-site doc, skill and.gitignoreedits were dropped during the rebase becausemainhad moved on)d8d964aFinalize Amtrak asset workflow and checks4f18dcechore(amtrak): sweep docs and tests to 25 sites40000-40024; walmart_careers + rotten_tomatoes registry tests updated;tasks.jsonlweb-> 40024 (port provisional until merge)fcd4697test(amtrak): verifiers, verify_lib, validation harnesssites/amtrak/verify/verify_lib.py,verify_0.py...verify_17.py,verify/README.md,verify/tests/(offline unittest matrix + Playwright live matrix, excluded from the image by.dockerignore)57c92aechore(amtrak): grading contract in tasks.jsonlverifier_path+judge_rubricon all 18 rows; tasks 2, 3, 5 re-anchored, 16 replaced, 8 others clarified (noanswerkey anywhere)Grading contract
Deterministic-first, no LLM dependency (
--no_llm Truechanges nothing):/booking/resultsquery-parameter gates (station aliases, date, sort, fare class, passengers), ordered funnels (round trip, multi-city, sleeper rooms, checkout), login as the named account;search_logsis excluded because the site writes it on every search / results request.Validation
python -m unittest discover -s sites/amtrak/verify/tests): 13 tests, all pass (genuine, no-op, shortcut, wrong answer, other task id, unterminated, foreign origin, corrupt PNG, schema tamper, collateral write, state mismatch, over-reach, phrasing variants).verify/tests/live_matrix.py, real Chromium against the standalone mirror): 18 tasks x {no-op, pass, shortcut, wrong, state-mismatch} = 90 cells, all as expected (every no-op / shortcut / wrong / state-mismatch run FAILs on the intended check; every genuine run PASSes).agent_demo/eval_judge.py --run_dir <run> --verifier Truereturns exit 0 on the genuine runs and exit 1 on the no-op.py_compileok, asset archive validated (308 members, no junk), seed md5cb4b6a5939b663017d9774f2d3a86884byte-identical after first and second boot, no cross-site imports, all 215 internal links 200, no broken images or page errors.Still required from the contributor (details on #39)
amtrak.tar.gzis not in HF discussion Add NBA mirror #30 (refs/pr/30holds only the 15 base tarballs); it exists only on the fork commitlxr-max/WebHarbor@eac108f. Open an HF PR that actually contains the tarball, merge it, then repin.assets-revision(currentlyrevision: main;mainis pinned to65c479f8…).account()"Next trips" lists Completed trips first — filter byis_upcoming./search,/help?q=and/booking/resultscommitsearch_logsrows, so read-only browsing is not byte-identical (the reset invariant still holds; the verifiers tolerate it).Full 25-site Docker environment built from this branch with
amtrak.tar.gzextracted by hand fromlxr-max/WebHarbor@eac108f(noChilleD/WebHarborrevision carries it yet): build ok (4.70 GB), 25/25 sites 200,/health25/25,POST /reset/amtrakbyte-identical (cb4b6a59…) before/after a dirty request and afterdocker restart,/reset-all25/25 in 1.64 s, verifier no-op matrix in the container 18/18 FAIL, registry tests green, offline verifier suite 13 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP