Skip to content

Fix Moli benchmark harness and add opt-in layout recovery - #25

Merged
lanyue-llk merged 41 commits into
mainfrom
codex/moli-benchmark-harness-fix
Oct 10, 2026
Merged

lanyue-llk merged 41 commits into
mainfrom
codex/moli-benchmark-harness-fix

Conversation

@lanyue-llk

@lanyue-llk lanyue-llk commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

PR #25:评测工具的问题、改动与影响

Summary

Consolidate the Moli benchmark infrastructure and fixed-layout comparison tooling. No Moli source code or binary is modified.

Infrastructure and reproducibility

  • Keep direct python3 tools/<script>.py entry points. Install the tool modules with the existing editable project installation; scripts do not modify import paths. Test paths are configured once in pytest.
  • Keep each cohort engine selection in its own run file; shared binary links and active-set configuration remain unchanged. Inherit Ruby settings from the caller and reject imports from another checkout.
  • Fix Selenium process tracking and cleanup on macOS, and handle framework download timeouts without unhandled promise rejection.
  • Route Schema.* CDP probes through the page session unless explicitly browser-scoped. Preserve historical measured results while correcting routing notes.
  • Freeze the full 372-task Moli 0.1.1 failure cohort, independent of Chrome outcomes. Bind binary, task, dependency and execution identities for version comparisons.

Fixed layout only

  • Layout is off by default; --moli-layout off and --moli-layout on select one mode for the entire run.
  • Remove --try-layout, failed-case rescheduling, replacement matrices, recovery receipts, retry-only artifact paths, the recovery summarizer and its design documentation.
  • Every case runs its configured repetitions, normally three, whether it passes or fails. Failure never changes layout or adds executions.
  • Fixed-layout summaries read ordinary complete results.jsonl runs. The CLI accepts repeated --source RUN_DIR arguments for disjoint task batches, preserving layout and binary checks.
  • Retained mixed-layout recovery evidence is explicitly rejected by fixed-layout reports; existing measurements are not deleted or rewritten.

Evidence and reports

  • Keep one result per engine/task/attempt and require all three attempts to pass for a successful case.
  • Validate fixed-layout source hashes, task coverage, binary identity, individual attempts and retained error logs.
  • Keep resource populations separate from the 372-case accuracy cohort. Resource comparisons and published measurements are maintained in browser-eval PR #27.
  • benchmarks/moli-0.1.1-failure-task-ids.txt remains the original 372-case input cohort. New-version successes do not prune that historical definition.

Verification

  • Real Moli 1.1.9 release: three cases × three attempts × two fixed layouts = 18 executions. No retry files or additional executions.
Case Fixed off Fixed on
pw_raw_browser_getversion 3/3 3/3
v2_diag_gbcr_fixed 0/3 3/3
v4_cdp_emulation_setemulatedostextscale 0/3 0/3
  • Full local harness tests: 653 passed, 15 skipped with the pinned Selenium 4.46.0 client. Dataset validation: 1,928 tasks; scenario validation: 116 specs / 1,233 bindings.
  • Focused CLI, fixed-layout, summary, cohort and manifest checks pass. Direct summary CLI invocation from another working directory is covered.
  • Removed switch and auto mode are rejected. Fixed off/on remain accepted; legacy mixed-layout evidence is rejected.
  • No benchmark accuracy rerun, no measured output or binaries committed, no Moli changes or merges.

@lanyue-llk
lanyue-llk marked this pull request as ready for review September 17, 2026 15:32
@lanyue-llk lanyue-llk changed the title Fix Moli cohort infrastructure failures and freeze version comparisons Fix Moli benchmark infrastructure and CDP target routing Sep 18, 2026
@lanyue-llk

Copy link
Copy Markdown
Contributor Author

Review: no blocking findings in #25

The 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 Schema.* to the page session corrects the harness target rather than changing the browser's claimed capability. The cohort tooling freezes the historical selection and checks complete attempts and recorded comparison conditions. Keeping the historical result rows intact while correcting their interpretation is appropriate.

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 git diff --check. I did not run a complete binary-backed cohort here; the PR's existing macOS and full-cohort evidence supplies that coverage. #27 has a separate actionable automatic-layout classifier finding and should be resolved before the stacked change is merged.

@lanyue-llk lanyue-llk changed the title Fix Moli benchmark infrastructure and CDP target routing Fix Moli benchmark harness and add opt-in layout recovery Sep 18, 2026
Comment thread runner/run.py Outdated
Comment thread benchmarks/moli-0.1.1-failure-cohort.json
Comment thread test/test_download_completion.py Outdated
@lanyue-llk

Copy link
Copy Markdown
Contributor Author

Chromium baseline correction

The two Chrome 151 non-passes in four_engine_full_20260812 were benchmark defects rather than Chromium capability failures:

  • pw_raw_schema_getdomains: the harness sent Schema.getDomains to the browser root. The corrected page-session route passes.
  • pw_raw_browser_getbrowsercommandline: Chrome exposes this diagnostic command only when launched with --enable-automation. The task now declares a scoped browser_automation launch profile; the corrected task passes.

Targeted verification used Chrome for Testing 151.0.7922.47 and ran each formerly failing task once. Both passed. The other 1,926 historical Chrome task results were not rerun. This yields a corrected Chromium baseline of 1,928 / 1,928 (100.00%), while the original immutable run remains 1,926 / 1,928 (99.90%) in historical evidence.

Verification:

  • 155 related tests passed.
  • Repository validation passed for all 1,928 task definitions.
  • PR CI harness passed.
  • The official Linux Chrome binary was independently downloaded and matched the historical SHA-256 3b0be9872ea937893cb1e1523fde071d38c1ed4ef866b3f7976240094a868c93. The targeted executions were on macOS arm64 with the same Chrome version; they are not represented as a new Linux full run.

@lanyue-llk lanyue-llk left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 x ID → 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())  # False

For 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.

@lanyue-llk

Copy link
Copy Markdown
Contributor Author

已处理独立 review 的两项问题,修复提交 5c620b5:

  1. run_moli_cohort.py 在任何目录/收据操作前复用底层 compact_run_id()。带连字符、大小写/重复下划线和超长 ID 的命令、结果目录、conditions 收据使用同一个名称,重复执行仍拒绝覆盖。
  2. 资源汇总逐条检查 engine_provenance.binary_sha256 与本次 binary receipt 一致;缺失或其他构建的哈希立即拒绝,不再进入可发布汇总。

新增五个参数化回归。独立审查 agent 已复核修改,相关 23 项测试通过;原“错误 binary 仍 publishable”的复现现在按预期抛出 row binary identity mismatch。未重跑评测或改写历史结果。

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
@lanyue-llk

Copy link
Copy Markdown
Contributor Author

已按当前 HEAD 重新核对评论,结论如下:

  • “runner 代码过大”成立。 原先的 layout 失败重试、结果替换和专用恢复文件已移除。当前只有固定 off/on 两种模式,每题按原定次数执行,不根据失败追加执行。
  • “修改文件是否都需要”需要逐项判断。 下载完成/超时测试已收进已有 test_framework_driver.py,不再新增独立测试文件。其余变更对应进程清理、CDP 路由、固定布局及证据校验,并有对应场景测试;没有为了减少文件数删除必要校验。
  • 两份 0.1.1 文件应保留。 TXT 是冻结的 372 题输入;JSON 保存来源版本、选择规则、文件哈希及执行参数。它们承担不同职责,新版本通过的题也不能从固定比较分母中删掉。
  • 独立 review 的两个 P2(run ID 规范化、资源结果逐条二进制绑定)已在 5c620b5 修复,本次相关回归测试再次通过。

本次额外实测发现并修复同类证据缺口:版本比较工具原先只核对运行清单,混入其他引擎、其他运行、其他二进制或缺少二进制来源的单条结果仍会接受。现在逐条核对 engine、run ID 和 binary SHA-256;四个拒绝场景修复前失败、修复后通过,并保留完整有效运行的正向测试。复用现有测试文件。文档同时纠正 372 × 3 = 1,116 条执行记录。

修复提交:797c728。验证:相关测试 28 passed;按仓库指定 Selenium 4.46.0 补齐测试环境后,完整测试 639 passed, 15 skipped。首次完整测试中一项本地 HTTP 服务启动超时,单独复测及完整重跑均通过。未更改已测结果、未重跑准确率、未修改 Moli、未合并 PR。

Comment thread tools/summarize_moli_macos_resources.py Outdated
Comment thread tools/summarize_moli_fixed_layout.py Outdated
Comment thread tools/run_moli_cohort.py Outdated
@lanyue-llk

Copy link
Copy Markdown
Contributor Author

已在 fe14ccd 收回不必要的调用方式变更:保留原有 python3 tools/<script>.py,四份证据/复现指南的命令修改撤回。安装配置将 tools 与 runner 一起纳入现有 editable 安装,因此脚本不需要 sys.path 修改或手动设置 PYTHONPATH。五个脚本从外部目录直接执行已实测通过,完整测试 643 passed、15 skipped。重复测试组合已删,必要的来源校验、固定布局及过期代码清理保留。

@euyis1019

Copy link
Copy Markdown
Collaborator

有三个问题需要处理:

  1. run_moli_cohort.py 会把 moli 软链接指向候选二进制并覆盖 active-set.json,跑完不恢复,之后同机评测会悄悄用上候选 Moli。请结束后(含异常)恢复,或改用参数注入。

  2. 脚本写死了 /opt/homebrew/opt/ruby/bin 等个人机器路径,请改为环境变量。

  3. report_four_engine.py 等工具现在依赖 pip install -e .,否则报 ModuleNotFoundError: runner;多 worktree 时还可能静默导入别处的 runner。建议用 python3 -m tools.xxx,或校验 runner.__file__ 在本仓库内。

注:第 3 条在 Debian 13 (x86_64, Python 3.13.5,未执行 editable install) 上基于 fe14ccd 复现;第 1、2 条为代码阅读确认。

@lanyue-llk

Copy link
Copy Markdown
Contributor Author

已处理这三项,实测结论和修复如下:

  1. 共用配置被修改的问题成立。 不再替换 Moli 软链接,也不再覆盖 build_artifacts/active-set.json。每轮写自己的引擎配置,通过 LEXBENCH_ENGINE_SET 传给子进程。成功和异常退出均验证原链接、原配置及原二进制保持不变;实际子进程读取候选配置也已验证。显式指定的配置缺失时直接报错,不退回默认构建。
  2. 本机 Ruby 路径问题成立。 删除 Homebrew Ruby 和个人 gem 目录的自动注入,直接继承调用者的 PATH、GEM_HOME、GEM_PATH。
  3. 多 worktree 混用代码的风险成立。 五个工具在加载 runner 配置前检查 runner 来自当前仓库;导入另一份 checkout 时明确报错。保留普通 python3 tools/<script>.py 命令。未安装项目时缺少 runner 属于运行环境未准备:现有运行指南已经要求 pip install -e '.[drivers,dev]',本次把同一要求补到 cohort 文档,不引入运行时路径修改。

版本比较同时允许本轮配置文件路径和候选二进制路径不同;每条结果的引擎、运行 ID 和二进制哈希仍严格检查,其他执行条件照常比较。

修复前相关场景有 11 项失败;修复后完整测试 653 passed、15 skipped。未重跑准确率、未改写既有实测结果、未合并 PR。

@lanyue-llk
lanyue-llk requested a review from euyis1019 October 10, 2026 11:03
@lanyue-llk
lanyue-llk merged commit 6c19eeb into main Oct 10, 2026
1 check passed
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.

2 participants