Repository navigation
Fix Moli benchmark harness and add opt-in layout recovery - #25
Conversation
Review: no blocking findings in #25The stated goal matches the changed paths: the macOS Selenium process tracking addresses the platform-specific infrastructure failure; the download progress helper attaches a rejection handler before awaiting a potentially slow click and removes the CDP listener; routing I reviewed the runner, adapter, cohort comparator, task correction, and relevant tests. The consolidated CI check passes. Locally, 26 download/CDP/cohort tests passed on the #27 tree (which contains #25 unchanged), and both PR diffs pass |
Add opt-in layout reruns for failed Moli cases
Chromium baseline correctionThe two Chrome 151 non-passes in
Targeted verification used Chrome for Testing Verification:
|
There was a problem hiding this comment.
Independent review — 1ab9c552077b2d25d1333583c3c3881e8ddb4997
Reviewed the PR diff and the current fixed --moli-layout off/on execution, attempt summaries, resource collection, and evidence validation. I found two reproducible issues:
[P2] Normalize the cohort run ID before deriving paths and receipts
tools/run_moli_cohort.py:82–83 builds its run directory and conditions receipt from the original argument, although line 69 accepts hyphens and arbitrary lengths. The invoked runner applies compact_run_id() in reserve_run_dir(). Directly exercising that real reservation function gives:
release-1_1_9→release_1_1_9; the wrapper's expected directory does not exist.- An 80-character
xID →xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx_d929cdee; the wrapper's expected directory does not exist.
Consequently the benchmark can finish successfully, then the wrapper fails while opening run_manifest.json, with its conditions receipt stored under a different ID. Case folding also changes the receipt's recorded identity even on a case-insensitive filesystem. Normalize once with the runner's canonical function before collision checks, receipt creation, and command construction, or reject noncanonical IDs before any mutation. Add a wrapper-level regression covering a hyphenated ID and a compacted ID.
[P2] Bind each resource measurement to the certified binary
tools/summarize_moli_macos_resources.py:144–145 validates each row's layout but never checks its engine_provenance.binary_sha256 against the manifest/receipt. Using the existing two-task resource fixture, changing one off-run row's binary hash from 999…999 to aaa…aaa, and calling summarize_fixed_pairs() still returns quality.publishable: true and attributes that measurement to 999…999.
A stale or mixed-binary results file is therefore accepted as the certified candidate. Check every row's binary hash against the certified identity and reject missing identities as well. The fixed-layout accuracy summarizer already performs this check. Add negative controls for both mismatched and missing row hashes, alongside the valid cohort case.
Validation performed: 127 passed, 8 skipped across test_moli_fixed_layout.py, test_summarize_moli_fixed_layout.py, test_summarize_moli_macos_resources.py, test_framework_driver.py, and test_resources.py. The two reproductions above were run independently in temporary directories; no repository or measured evidence was changed. No additional actionable finding in the removal of layout retries itself.
Reproduction entry points, from the repository root with its Python dependencies installed:
from pathlib import Path
from tempfile import TemporaryDirectory
from runner.run import reserve_run_dir
with TemporaryDirectory() as tmp:
print(reserve_run_dir(Path(tmp), "release-1_1_9", "error")[0])
print((Path(tmp) / "release-1_1_9").exists()) # FalseFor the second finding, use the _run, _receipt, _protocol, and _write_jsonl fixtures in test/test_summarize_moli_macos_resources.py; create the off/on runs, change off/results.jsonl row 0's engine_provenance.binary_sha256, and call summarize_fixed_pairs(off, on, receipt, protocol, expected_tasks=2, expected_frozen_calls=5). The returned quality.publishable is True despite the mismatch.
|
已处理独立 review 的两项问题,修复提交
新增五个参数化回归。独立审查 agent 已复核修改,相关 23 项测试通过;原“错误 binary 仍 publishable”的复现现在按预期抛出 |
Scroll agent-browser and Ferrum targets into view before clicking
* Apply automation profile to Moli and set explicit viewport * test: cover automation profiles for supported engines
|
已按当前 HEAD 重新核对评论,结论如下:
本次额外实测发现并修复同类证据缺口:版本比较工具原先只核对运行清单,混入其他引擎、其他运行、其他二进制或缺少二进制来源的单条结果仍会接受。现在逐条核对 engine、run ID 和 binary SHA-256;四个拒绝场景修复前失败、修复后通过,并保留完整有效运行的正向测试。复用现有测试文件。文档同时纠正 372 × 3 = 1,116 条执行记录。 修复提交: |
|
已在 fe14ccd 收回不必要的调用方式变更:保留原有 python3 tools/<script>.py,四份证据/复现指南的命令修改撤回。安装配置将 tools 与 runner 一起纳入现有 editable 安装,因此脚本不需要 sys.path 修改或手动设置 PYTHONPATH。五个脚本从外部目录直接执行已实测通过,完整测试 643 passed、15 skipped。重复测试组合已删,必要的来源校验、固定布局及过期代码清理保留。 |
|
有三个问题需要处理:
注:第 3 条在 Debian 13 (x86_64, Python 3.13.5,未执行 editable install) 上基于 |
|
已处理这三项,实测结论和修复如下:
版本比较同时允许本轮配置文件路径和候选二进制路径不同;每条结果的引擎、运行 ID 和二进制哈希仍严格检查,其他执行条件照常比较。 修复前相关场景有 11 项失败;修复后完整测试 653 passed、15 skipped。未重跑准确率、未改写既有实测结果、未合并 PR。 |
Summary
Consolidate the Moli benchmark infrastructure and fixed-layout comparison tooling. No Moli source code or binary is modified.
Infrastructure and reproducibility
python3 tools/<script>.pyentry points. Install the tool modules with the existing editable project installation; scripts do not modify import paths. Test paths are configured once in pytest.Schema.*CDP probes through the page session unless explicitly browser-scoped. Preserve historical measured results while correcting routing notes.Fixed layout only
--moli-layout offand--moli-layout onselect one mode for the entire run.--try-layout, failed-case rescheduling, replacement matrices, recovery receipts, retry-only artifact paths, the recovery summarizer and its design documentation.results.jsonlruns. The CLI accepts repeated--source RUN_DIRarguments for disjoint task batches, preserving layout and binary checks.Evidence and reports
benchmarks/moli-0.1.1-failure-task-ids.txtremains the original 372-case input cohort. New-version successes do not prune that historical definition.Verification
pw_raw_browser_getversionv2_diag_gbcr_fixedv4_cdp_emulation_setemulatedostextscaleautomode are rejected. Fixed off/on remain accepted; legacy mixed-layout evidence is rejected.