Review: harden WebHarbor task validation (#45) - #91
Open
jackjin1997 wants to merge 5 commits into
Open
jackjin1997 wants to merge 5 commits into
jackjin1997 wants to merge 5 commits into
Conversation
Contributor
Author
|
Independent blind first-pass review (PR #45 validator scenarios)
Unexecuted checks:
Non-blocking notes: TV-C11 message says "site order differs" where the site sets differ (wording only); TV-C07 counts an unparseable line as Scope: verdict covers only the frozen executions; it does not certify validator source or broader site behavior. |
jackjin1997
marked this pull request as ready for review
September 10, 2026 07:26
evanz37
added a commit
to evanz37/WebHarbor
that referenced
this pull request
Sep 15, 2026
…tic avatars), asset inventory and gate The mirror shipped no images at all: static/ held two .gitkeep files, base.html carried one inline stylesheet, and every card image was a flat colour plus a text label. That is a real fidelity gap for a mirror of berkeley.edu, where every section page leads with a photograph, and the README called it "by design". 164 files, 6,991,525 bytes (6.67 MiB) on disk, all declared in generated_asset_inventory.json and gated by check_generated_assets.py: campus 8 1024x768 WebP section heroes and page-header banners college 14 1024x768 WebP one per college; odd ids exterior, even interior research 25 1024x768 WebP one per research centre news 21 1024x768 WebP 7 categories x 3 variants, variant = id % 3 + 1 event 14 1024x768 WebP 7 categories x 2 variants, variant = id % 2 + 1 faculty 82 256x256 PNG deterministic Pillow initials avatars Scenes come from fal.ai FLUX.1 [schnell] (4 inference steps, one fixed seed per slot derived from the slug); the avatars are drawn offline by Pillow 11.0.0 from the faculty row's initials and id, and regenerate byte-identically. Variants bind to the row's primary key, never to a render index, so a listing card and a detail banner agree on the same file and pagination cannot reselect one. The plan and the generators live in sites/berkeley/scripts/ (the tracked location sites/compass/scripts/ uses), not in scripts_dev/, which stays a local scratch directory. Prompt policy: every prompt is one subject clause plus a fixed style suffix plus a fixed negative list — no faces, no portraits, no text, no lettering, no logos, no watermarks, no posters, no framed pictures, no screens with visible content, no recognizable landmarks or signage, unlabeled containers. The exact prompt is recorded per file. Slots are occupancy-classified: labs, research interiors, news and event venues are prompted unoccupied; campus and college-exterior slots permit at most two or three figures in the middle distance, backs turned, faces hidden. No image depicts a real person, a real face or a real landmark — the campus scenes are generic institutional architecture, deliberately not the Campanile, Sather Gate or the Golden Gate, and the avatars are monogram discs. Answer-leak safety: every alt is built from the same seed fields the page already renders — college and centre names, category only for news and events — never a title, director, founding year or focus area, which are the graded answers on the detail pages. test_answer_leaks passes unchanged, including the two entity-bound assertions that inspect a +-700-character window around every /research/<slug> link, which now contains the new card image. Verification: - 569 tests pass: sites/berkeley/tests (answer leaks, app robustness, integration, generated assets) and sites/berkeley/verify/tests. - check_generated_assets.py verified 164 assets at build time: exact coverage, per-file SHA-256, decode at the planned dimensions, letterbox test. - OCR pass and manual face review. The OCR pass runs over every scene and has a positive control that proves it can fire on rendered text; it is NOT a text guarantee. It returned zero tokens for the word STCK rendered in large red capitals across a window in colleges/chemistry.webp and missed a placard in research/bair.webp entirely, and it was flagging clean scenes on three-character junk (aif, hea, saks) until the confidence floor was raised to 4 chars / 50. The face pass is a frontal-face cascade, not a person detector: the OpenCV defaults gave 15 false positives on five clean pilot scenes (foliage, mown grass) and were replaced with settings that score zero there and detect a generated face control — which is exactly why it also misses the small, profile faces the rule cares about. scripts/IMAGE_PLAN.md §9 records every measurement and every miss. - The by-eye review is what actually bounds this, across five passes: a seated person in a research interior; crowds with faces to camera in the event family (which moved to unoccupied prompts — an empty hall still reads as a venue); a framed portrait in a faculty-office scene (fixed with anti-wall-art clauses on the 24 office and reading-room motifs that invite one); three letterboxed frames (now a checked defect class, self-healing via is_accepted); and four slots reported on the review contact sheet — chemistry (STCK), the newsroom (watermark-like label), journalism (a face to camera) and law (faces) — each regenerated on a new seed to its instruction: law unoccupied, journalism back-view figures only. The contact sheet is gen_images.py --contact-sheet, default scripts_dev/contact_sheet.png. - §9 UI sweep at 1440/768/390/320 over 424 routes: no overflow, no broken images, no console errors, no aria or heading-order findings. The only findings are net::ERR_ABORTED on lazy images cancelled by navigation, with no 4xx and no external requests. - Contrast is arithmetic, not measured. The pixel-sampling sweep reported no measured failure before or after this change, but every finding on both sides carries the sampler's "could not isolate a background" signature (bg_used == sampled, ratio 1.0) — it went blind exactly where the photos are, so it cannot corroborate photo-backed text. What the claim rests on is the computed worst case against a pure-white pixel behind: hero scrim white 8.6:1, the 18px paragraph 7.3:1, the gold eyebrow 4.8:1, page-header 8.3:1, and .card-img-label given its own scrim (>= 8:1). The gold eyebrow was already 3.25:1 on the old opaque gradient, so this improves it rather than fixing a regression. - Container: 29/29 sites HTTP 200; /health 29 alive + ready; POST /reset/berkeley byte-identical (f2f0187c... == instance_seed) before, after reset and after docker restart; no-op matrix 22/22 FAIL, 0 mismatches; broken-image sweep over 424 routes x 2 widths inside the image, 0 broken. Assets. The bundle ships through the pinned Hugging Face tarball, not git: berkeley.tar.gz, 6,951,483 bytes, sha256 ab9d2716ae8d06540a181b5e60c37f613d87b103864b467511da546b1b173789, 171 managed members, validated by scripts/validate_asset_archive.py. Uploaded as https://huggingface.co/datasets/ChilleD/WebHarbor/discussions/91 (head commit 4529b18c6fc23f9b203a281b2d1e1940ba93853b), and .assets-revision pins `revision: refs/pr/91` as an explicitly INTERIM pin — a PR ref, so it moves if the PR branch is updated. Maintainers should re-pin `revision:` to the merged commit sha once PR aiming-lab#91 is merged, and delete the interim paragraph; the "never a moving branch name" rule in that file then applies again. 86ba06e's fetch_assets.sh build-generated exemption is no longer exercised by berkeley: the archive now exists, so the download branch is taken and the exemption remains only as the general fallback for a build-generated site with no archive at the pinned revision. Nothing needs removing; this records that berkeley stopped being the case that motivated it. run_matrix.py gains --cells so the no-op matrix can be run on its own; that is the only non-imagery change here, and it is additive. Co-Authored-By: Claude Code <noreply@anthropic.com>
Raibows
added a commit
to evanz37/WebHarbor
that referenced
this pull request
Sep 15, 2026
HF dataset PR aiming-lab#91 ("berkeley: synthetic imagery bundle (164 files)") is merged, so the interim `refs/pr/91` pin is replaced by the immutable head commit of the dataset's `main`, c32018ca3b3d67e7b858b1b85fb101aea5090cd7 (merged 2026-09-15T04:00:54Z). A `refs/pr/<n>` ref moves if its branch is updated; the merged commit does not, so this removes the drift the PR ref carried. Verified while changing the pin: - `berkeley.tar.gz` on c32018ca is 6951483 bytes, sha256 ab9d2716ae8d06540a181b5e60c37f613d87b103864b467511da546b1b173789, byte-identical (cmp) to the archive the `refs/pr/91` pin served, and passes `scripts/validate_asset_archive.py` (171 managed members, exit 0); - all 30 registered site archives on c32018ca have the same size and LFS oid as on `refs/pr/91` (30/30 identical, 0 differences), and the same two unregistered archives (bandcamp.tar.gz, drugs_com.tar.gz) are present; - a clean re-fetch from the new pin reproduces sites/berkeley/static/images (164 files) and instance_seed/berkeley.db byte for byte. `.assets-revision`'s comment block now records the merged pin, keeps the previous pins as history, and no longer describes the revision as an interim PR ref. README.md's asset-delivery section states the new pin and the merge.
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
1 participant
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.
Summary
Reviewer-owned continuation of #45, preserving XuanRui LI / @Lxr-max's original commit
6b2a41a600bd0e1b260c3c80494f2d50f2b1d2faand attribution before current-mainintegration and reviewer fixes.
The review hardens the task validator for stable invalid-input diagnostics, registry
consistency, answer-key exclusion, complete site-local grading contracts, exact task
identity, verifier uniqueness, and lower-noise heuristics.
Detailed review report and reproduction commands
Verified candidate
fdc58de0f179eec31da85f6094d78f57b69b5ac1142bae2c32c4f3fc8b1ceae51b1b63511b401f7d36004932bdf82afbe36dc14e00f66841eccf9946ruff check,ruff format --check,pyright, andgit diff --checkpassedThe final commit after the blind-reviewed head changes only the public report; validator,
tests, registries, task corpus, and verifier files are unchanged.
Scope
This is repository tooling, not a mirror contribution. It changes no site UI, route, seed,
database, Docker behavior, asset archive, or Hugging Face revision; those checks are not
applicable and are not reported as passing. No HF action is required.
Ready for maintainer review. This review does not merge GitHub or Hugging Face PRs.