Skip to content

Review: Add Healthline mirror + task verifiers (site by @JeremyJC67, verifiers by reviewer) (#59) - #105

Merged
Raibows merged 34 commits into
aiming-lab:mainfrom
TabsPhasers:review/pr-59-healthline
Sep 13, 2026
Merged

Review: Add Healthline mirror + task verifiers (site by @JeremyJC67, verifiers by reviewer) (#59)#105
Raibows merged 34 commits into
aiming-lab:mainfrom
TabsPhasers:review/pr-59-healthline

Conversation

@TabsPhasers

@TabsPhasers TabsPhasers commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Summary

基于当前 main 接管 @DEM1TASSE#59(Healthline 镜像 + task verifiers)。本 PR 为 Draft 占位版本,用于承载 reviewer 的修复与验证结果。

⚠️ Draft / not mergeable yet:本分支尚未重铺到当前 main,GitHub 显示 CONFLICTINGmergeable=false / dirty)。当前 diff = 原贡献 + 本次 reviewer 修复,共 53 个文件。下一次 push 会重铺到当前 main 并解决冲突,diff 收敛为 sites/healthline/** + Dockerfile / control_server.py / websyn_start.sh + tasks.jsonl / verify/*。在重铺与素材合并完成前,请勿将本 PR 作为合并候选。

Original review resolution

  1. 端口修正 40015→40016 仍与 main 冲突:当前 main 的 40016 已归 ikea;改为追加式分配 40024,并保留 main 全部站点(25 站)。
  2. Healthline--3/16 rubric 已补全:对应 verifier 精度不足——离线探针发现 11 个确定性假阳性(数值子串、复合症状重复计数、否定句通过、药物类别颠倒、缺 DB 当 0 等),已全部修复。
  3. .assets-revision:素材 HF PR Add AKC mirror site #40 未合并、原 pin 为 refs/pr/40;本轮不改 pin(保留 main 的稳定 revision),待 HF Add AKC mirror site #40 合并后 pin 到 merge SHA。

Review changes

  • sites/healthline/verify/* + verify_lib.py:数值边界匹配、否定/肯定区分、药物-类别关联、DB 缺失 fail-close、导航门收紧、分组计数。
  • sites/healthline/tasks.jsonl:web 端口 40016 → 40024
  • Dockerfile / control_server.py / websyn_start.sh:保留 main 站点列表并追加 healthline(端口 40024)。

Validation

  • Baseline no-op:20/20 正常 FAIL(无假阳性放行)。
  • 纯视觉原生 actor:20/20 真实轨迹(含 5 次宿主 watchdog infra 重试闭合)。
  • 修复版确定性 verifier(--no_llm):20/20 exit 0。
  • 修复版 verifier(LLM 检查开启):20/20 exit 0。
  • 独立 LLM judge(官方 eval_judge.py 语义,轨迹 + 末 4 帧):20/20 success,无 false checkpoint。
  • 主审逐题读图 + before/after DB + trajectory:20/20 PASS。
  • 完整证据包(per-task 表、原始输出、哈希)保存在 reviewer 本地目录,不在本 PR 提交中。

Asset delivery status

Scope

  • 保留原作者 @DEM1TASSE 及原站点贡献归属。
  • 排除 reviewer 运行产物、截图、缓存与本地证据。
  • 共享框架改动:无。

Blocking checklist

  • 重铺到当前 main 并解决冲突,把 diff 收敛为站点 + 必要集成改动
  • HF PR Add AKC mirror site #40 合并 → .assets-revision pin merge SHA → clean checkout 执行 fetch_assets.sh healthline + build/reset 验证
  • 按 reviewer 口径补登记接管说明(本项目此前已声明跳过 lock comment)

Reviews #59.

JeremyJC67 and others added 5 commits July 2, 2026 00:19
Addresses review on PR aiming-lab#53:
- Rebase onto current main; register healthline as index 16 -> port 40016
  (append after merriam_webster in websyn_start.sh + control_server.py;
  Dockerfile EXPOSE -> 40000-40016). merriam_webster preserved.
- websyn_start.sh site-count comments reconciled to 17.
- .assets-revision pinned to HF PR aiming-lab#40 (clean tarball, no macOS AppleDouble
  junk, based on current main -> all 17 tarballs). Bump to merged SHA once
  HF PR aiming-lab#40 lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewer deliverable for the Healthline mirror (site by @JeremyJC67, PR aiming-lab#53):
one deterministic verifier per task under sites/healthline/verify/, plus
verifier_path + judge_rubric recorded in every tasks.jsonl row. No answer
key in tasks.jsonl — ground truth lives only inside the verifiers.

Deterministic-first: (1) trajectory navigation gate (anti knowledge-shortcut,
important here since several answers are medically recallable), (2) SQLite DB
after-state for the stateful tasks (save / register / password-change, plus
DB cross-checks for saved-count and reading-history), (3) answer vs frozen
ground truth, with the LLM only as an anchored consistency check.

Validated against the official react agent (agent_demo/agent.py): a no-op run
fails all 20 verifiers; a human answer-check confirms every frozen value
matches what the page renders. On the full 20-task run, after fixing one
too-strict nav gate the run itself surfaced, verifier and LLM judge agree
15/20, and all 5 remaining divergences are the deterministic verifier being
correct while the LLM judge false-positives (blank answer / knowledge-shortcut)
or false-negatives (DB-confirmed save / password change).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Search result cards showed drug_class (e.g. 'ACE inhibitor'), letting T16
be answered from the results page without opening the drug pages. Show the
broader category (e.g. 'Heart Medications') instead — same card layout, but
the ACE-inhibitor-vs-statin distinction now requires opening each detail
page. drug_class still shown on the drug detail page; search backend still
matches on drug_class. Addresses DEM1TASSE review note on PR aiming-lab#53.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- web: all 20 tasks pointed at :40015 (Merriam-Webster) after the site was
  rebased to index 16 / :40016; corrected to http://localhost:40016/.
- judge_rubric for Healthline--3 and Healthline--16: spell out the full
  ground-truth answer the page states (T3 symptom list, T16 what each drug
  treats) so the LLM judge grades against complete facts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ree matches its own Dockerfile and registry

Conflict resolutions:
- Dockerfile: keep '<N> Flask mirror sites' as 25 and EXPOSE 8100+healthline at index 24
  (40000-40024); main's per-site build steps for ikea/phys_org/target/ted/osu/
  rotten_tomatoes/compass/walmart_careers are retained.
- control_server.py / websyn_start.sh: keep the 25-entry SITES list with 'healthline'
  appended (index 24 -> container port 40024).
- .assets-revision: keep the immutable sha of HF PR aiming-lab#70 (superset of the dataset main
  branch: 28 tarballs including healthline.tar.gz).
@JeremyJC67

Copy link
Copy Markdown
Contributor

@TabsPhasers thanks for picking this up. A heads-up so you don't redo work that is already
done, and an offer at the end.

This branch is based on #59's July head (ports 40015/40016). The site PR #53 was rebased onto
current main on 2026-09-11: healthline is site 25 / index 24 / port 40024, @DEM1TASSE's two
verifier commits are cherry-picked in with their authorship preserved, and it is MERGEABLE/CLEAN.
So the first item on your blocking checklist is already closed there.

It has since had a second pass against the maintainer merge-blocker checklist, verified on a full
25-site container build: 25/25 sites 200, reset and restart byte-identical (a8a9b54…), reset-all
1.50 s, no-op verifier matrix 20/20 FAIL. Two of those findings were blockers rather than polish:

  • a hardcoded SECRET_KEY — signing {"_user_id":"1"} with the in-repo key and requesting
    /account returned 200 rendering alice.j@test.com;
  • GET /article/<slug> incremented view_count and committed, so read-only browsing changed the
    DB md5 and the 15 read-only tasks could not assert tables_unchanged.

On assets: HF #40 is closed. It was superseded by
#70, same bytes rebased on
current asset main. Keeping .assets-revision at main's revision will not work either — no
merged revision contains healthline.tar.gz yet, so fetch_assets.sh healthline fails until #70
merges. #53 pins refs/pr/70 temporarily and documents the repin, the same way #86 and #95 do.

Three of your verifier predicates are things I do not have, and they are good: count_groups
in verify_3 (so overlapping symptom tokens count once), classes_not_reversed in verify_16 (which
rejects swapping the two drug classes), and amount_with_unit. Your boundary-aware
number_mentioned matches a fix I landed independently — useful corroboration that it was a real
hole.

So rather than two vehicles for one site, either of these works and both credit your review:

  1. you rebase this PR onto feat(healthline): add Healthline mirror + task verifiers (site 25, port 40024) #53's head and it stays the vehicle; or
  2. you review feat(healthline): add Healthline mirror + task verifiers (site 25, port 40024) #53 directly, and I cherry-pick those three predicates from cdf5865 with your
    authorship preserved.

Tell me which you prefer and I will do my side of it today.

One small correction for the record: the Scope section credits @DEM1TASSE as the original author.
They authored the verifiers; the site itself is mine — which your title already states correctly.

@TabsPhasers

Copy link
Copy Markdown
Contributor Author

Thanks — we'll take option 2: #53 stays the merge vehicle. We're closing #105 as superseded; the branch remains at TabsPhasers/WebHarbor:review/pr-59-healthline @ cdf5865 if you want to diff anything.

Requested cherry-pick with authorship preserved — 8 predicate deltas, not just the three helpers you named (we diffed our tree against #53's head 79e03ce):

  1. verify_lib.count_groups + verify_3 — overlapping symptom tokens ("slow-healing" + "sores", "thirst" + "urination") must count once; feat(healthline): add Healthline mirror + task verifiers (site 25, port 40024) #53 still uses count_matches, so a one-symptom answer can pass as three.
  2. verify_lib.amount_with_unit + verify_11 — require a standalone 50 mg with a unit; substring matching still accepts "150 mg".
  3. verify_16.answer_classes_correct (classes_not_reversed) — reject swapping the two drug classes; contains_all alone passes "lisinopril is a statin; atorvastatin is an ACE inhibitor".
  4. verify_6 — fail-close when the initial DB is unavailable (saved_articles_for(...) or [] currently coerces "no DB" into "0 saved", which a wrong answer can satisfy).
  5. verify_8.answer_reviewer — require the reviewer's name (Kim Chin), not just the credential string.
  6. verify_15.answer_affirms — broadened negation/affirmation set; the current four-phrase list passes "No, grapefruit is absent."
  7. verify_17.answer_condition — require the condition identity (hypertension / silent killer), not only the detection method.
  8. verify_19 — same grouping fix as (1) for migraine triggers ("bright light" + "light" double-count, etc.).

number_mentioned no longer needs taking from us — your independent anchoring is equivalent to ours; treat it as corroboration only.

Two things for the record:

  • Your two blockers (hardcoded SECRET_KEY; GET /article/<slug> committing view_count) are real gaps our pass missed. We're recording them as findings on our side; thanks for catching them.
  • Scope credit is as you corrected: site by @JeremyJC67, verifiers by @DEM1TASSE; our reviewer deltas are the 8 above. We've fixed our PR text accordingly.

On assets we're aligned: HF #40 closed → #70; .assets-revision temporary refs/pr/70 is the same pattern as #86/#95, and the one-line repin to the merge SHA follows once #70 lands. We'll re-verify the predicates after your cherry-pick lands and can post a short reviewer confirmation on #53 if useful.

If a direct cherry-pick conflicts, we're happy to push a small patch branch on top of #53's head instead — just say which you prefer.

(This reply was posted automatically via OpenCode.)

JeremyJC67 pushed a commit to JeremyJC67/WebHarbor that referenced this pull request Sep 13, 2026
…ewer

Ports the eight predicate deltas @TabsPhasers requested on aiming-lab#105, with their authorship.
Applied surgically onto this branch's hardened verify_lib rather than cherry-picking their
commit, because their tree predates the evidence-binding and read-only-state work here.

1. count_groups + verify_3: overlapping symptom tokens count once
2. amount_with_unit + verify_11: a standalone 50 mg with a unit, not a substring
3. verify_16: reject swapping the two drug classes
4. verify_6: fail closed when the initial DB is unavailable
5. verify_8: require the reviewer's name, not just the credential
6. verify_15: broadened affirm/negate pair
7. verify_17: require the condition identity
8. verify_19: same grouping fix for migraine triggers

number_mentioned was not taken: this branch's anchoring is equivalent, by their assessment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AkNTAZMziDodR6uqa6QBtP
@Raibows Raibows reopened this Sep 13, 2026
Raibows added a commit to TabsPhasers/WebHarbor that referenced this pull request Sep 13, 2026
…h_assets.sh

(cherry picked from commit c944482a7e91d3dfedd6e65f2baf4eadf7a34cf7)
@Raibows
Raibows force-pushed the review/pr-59-healthline branch from be65778 to 431e8b0 Compare September 13, 2026 13:28
@Raibows
Raibows marked this pull request as ready for review September 13, 2026 13:41
@Raibows

Raibows commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution! @TabsPhasers @JeremyJC67 @DEM1TASSE

@Raibows
Raibows merged commit a437bd4 into aiming-lab:main Sep 13, 2026
jackjin1997 added a commit to jackjin1997/WebHarbor that referenced this pull request Sep 13, 2026
Upstream advanced to a437bd4, merging Healthline (aiming-lab#105), which took index 26 and
port 40026 -- the slot this branch was using.

Resolved by taking upstream's registry verbatim and appending versus after
healthline: healthline keeps 40026, versus takes 40027. No earlier site moves.
Dockerfile site count and EXPOSE range follow the 28-site registry, and the
task rows and site README follow the slot.

The regression test derives the port from both launchers rather than freezing
it, so it follows this move on its own; so does the verifiers' origin check.
Recorded trajectories carry the old port, so the runs are re-recorded at 40027
rather than graded with an origin override.
@JeremyJC67

Copy link
Copy Markdown
Contributor

@Raibows @TabsPhasers — congratulations on landing this, and one follow-up finding on the merged
tree, since it is the single item from the #53 audit that did not come across.

Two unvalidated redirects are live in sites/healthline/app.py on main:

app.py:700   return redirect(request.args.get("next") or url_for("account"))
app.py:846   return redirect(request.referrer or url_for("article_detail", slug=art.slug))

Both follow an attacker-supplied absolute URL. \/login?next=https://evil.example/pwn lands the
signed-in user off-site, and the second does the same through a forged Referer. Everything else
from that audit did make it — HEALTHLINE_SECRET_KEY with a random fallback, the read-only
GET /article/<slug> (the docstring on article_detail states the invariant), MAX_CONTENT_LENGTH,
the branded error handlers, count_groups / amount_with_unit / tables_unchanged — so this looks
like an oversight in the integration rather than a decision.

The fix that was on #53 is small and self-contained:

def safe_redirect_target(target, fallback):
    """Only follow same-origin relative paths; anything else falls back."""
    if not target:
        return fallback
    parsed = urlparse(target)
    if parsed.scheme or parsed.netloc or not target.startswith("/") or target.startswith("//"):
        return fallback
    return target

with the two call sites becoming redirect(safe_redirect_target(request.args.get("next"), url_for("account")))
and the equivalent for the Referer case.

Say the word and I will open a one-file PR against main with exactly that, plus a container run to
confirm both redirects fall back. It is worth noting that the same pattern is likely present on other
merged sites — the audit found it on several — so if you would rather have it as one sweep across the
registry than a per-site fix, that works too.

evanz37 added a commit to evanz37/WebHarbor that referenced this pull request Sep 13, 2026
… into the UC Berkeley branch

Registry: append `berkeley` after `kaggle`, so Healthline keeps index 26 / port
40026 and Kaggle keeps index 27 / port 40027; UC Berkeley moves to index 28 /
port 40028 (29 sites, 40000-40028).

Conflict resolutions (append rule):
- `websyn_start.sh`, `control_server.py`: keep all of upstream's 28 entries in
  order and append `berkeley` last (both registries identical, 29 entries).
- `Dockerfile`: keep every upstream site block (Healthline's migrate + prune step
  included) and berkeley's build-generated seed block; header 28 -> 29 sites;
  `EXPOSE 8101 40000-40027` -> `40000-40028`.
- `README.md`, `AGENTS.md`, `CONTRIBUTING.md`, `CLAUDE.md`, `agent_demo/README.md`,
  `.claude/skills/*`: take upstream's text, then 29 sites, port range
  40000-40028, alt ports 41000-41028, and `UC Berkeley` appended to README's
  mirror list.
- `scripts/fetch_assets.sh`, `scripts/check_assets.sh`: take upstream's
  registry-scoped implementations and keep berkeley's `.build-generated-seed`
  exemption on top.

Follow-on work required by the port move:
- `sites/berkeley/tasks.jsonl` (22 `web` rows), `app.py` PORT default,
  `README.md`, `verify/TASK_REVIEW.md`, `verify/verify_lib.py` comment,
  `tests/test_integration.py` (SITE_INDEX 28 / SITE_PORT 40028) and
  `verify/tests/test_tasks_contract.py`: port 40028.
- `scripts/check_site_registry.py` (upstream's new gate) reports 29 sites
  consistent across both registries, `Dockerfile EXPOSE` and every
  `tasks.jsonl` port.

Verified after the merge: registry gate green; berkeley site 27, berkeley verify
534, healthline 27; walmart_careers 50 passed and rotten_tomatoes 55 passed
(+4129 subtests) each with one expected red that is pre-existing on upstream/main
and reproduced there from a pristine `git archive` tree (VERIFICATION.md
§re-slot); kaggle ships no pytest suite. Container rebuilt and re-run on
44000-44028: 29/29 sites 200, /health 29 alive+ready, `POST /reset/berkeley`
byte-identical (`f2f0187c...`) before and after `docker restart`, no-op matrix
22/22 FAIL with 0 infra errors.

Co-Authored-By: Claude Code <noreply@anthropic.com>
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.

4 participants