Integrate Amtrak PRs #39 and #99 with reviewed UI, task, and grading fixes - #133
Merged
Merged
Conversation
…web -> 40024 Reviewer follow-up to the rebase of PR #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 #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
…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
Collaborator
Author
|
Integration verified complete: main is now 454e7a4, with a tree identical to the validated PR head. GitHub reports original #39, review #99, and this PR all MERGED; their actual ancestry is preserved in that order. Required HF PR #96 is merged and pinned; incomplete HF draft #30 is closed with a replacement link. Fresh 40-site fetch/build, all 40 health/homepage checks, 182 expected grading results, four new scripted browser regressions, and byte-identical Amtrak reset/restart passed. Local main is fast-forwarded and clean. The temporary test container is stopped; existing previews remain running. Full report: review-reports/amtrak-pr99-integration.md. Docker Hub publication/deployment was not performed; the local image is 5.34 GB. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Amtrak: PR #39 → PR #99 → reviewed fixes
Integration date: 2026-09-17. Original site contribution: @Lxr-max.
Review continuation: @JeremyJC67. Follow-up fixes are based on PR #99.
History and scope
Regular merge commits preserve original PR #39 head
ab903781556b72f6900c7be2d89c89cc46a6c0d9, review PR #99 head677b87c007bcf7390a6dea51171033e75681bdf6, and fix commita57ad6a.No squash, rebase, or force-push is used for this integration. The original and
review branches both remain ancestors even though the reviewer had previously
rebased/copied the contributor's commits.
Amtrak is appended at index 39, port 40039. All existing 39 site ports and
site implementations are preserved, along with main's seed migrations and
asset pins. The integration is isolated from the user's running previews.
Reviewed fixes
preserve selected fares and reject invalid fare submissions.
comparisons, constraints and multi-page evidence, not click padding.
JSON schemas, internal field names and exact-policy-quotation requirements.
precise saved-state verification. Keep structured-answer backward compatibility.
main's existing pre-action
observed_textand final-observation fields.Merged HF assets
HF PR #96
publishes the original contributor's unchanged
amtrak.tar.gzintoChilleD/WebHarbor, at merged commit6f432484307f76ca3c58b1d5c8f7b614dfb34f54.Incomplete draft #30 contained no Amtrak archive and was closed with a link
to the replacement. All 44 previously existing dataset files are unchanged.
22f0132a5f9bfee45a641e8172050a94bb4fc0de4ad45f1d9e52deb50eb37f34.b245928c8b03741d17aa7c27f863a499e44034af607902cfa9c8d099c93591b4..assets-revision.checkout; assets were not copied manually from the preview.
Verification
explicit overrides, missing-archive behavior and digest rejection.
claims, package/fixture/state checks and explicit before/after URL pairing.
expectations (18 prior browser recordings and 164 synthetic controls).
These regrades are not new browser runs.
webharbor:pr99-integrated, imagesha256:978abcae6f8464b5f8ff31c5ee4138090b8f4cb7053a30c36ac60ef982843758.All 40 sites are healthy and all 40 homepages return HTTP 200.
task 3 (multi-city), 8 (saved profile), 16 (refund/fare comparison), and
17 (completed Business booking). These are scripted checks, not LLM-agent runs.
Mobile homepage has no horizontal overflow; booking panel width is 318 px
at a 390 px viewport. Integrated homepage matches the reviewed preview.
both restore byte identity with the seed:
cb4b6a5939b663017d9774f2d3a86884(MD5).under container Python 3.12. Runtime source candidate:
b6e9ff1; this reportis the only subsequent tracked addition.
Local integration evidence:
.assets/integration-pr99/in the integrationworktree. Earlier task GIFs remain in the fix worktree at
.assets/reviews/pr99-fix-20260917/natural-answers/.The existing dashboard is
http://localhost:43815/natural-answers/and thereviewed standalone preview is
http://localhost:44815/(forward both portswhen accessing the remote workspace). They are not deployed release URLs.
Limits
The mirror uses synthetic schedules, fares and generated artwork, not live
Amtrak data or a faithful copy of every upstream page. The natural-answer parser
has bounded coverage, not unrestricted semantic understanding. No independent
LLM-agent run or secondary LLM-judge run is claimed. Docker Hub publication and
deployment are separate stages, not included in this source/assets merge.
The combined image is 5,342,471,387 bytes (5.34 GB), above the repository's 4 GB
target. Broad inherited image-size cleanup was not part of this integration.