Repository navigation
[Feat] Re-score saved public231 decisions - #69
Conversation
Re-score pinned public231 probabilities and verify ticket decisions against accepted actions and committed environment state. Changed lines relative to stacked base: 2264 authored code/test lines and 98 documentation lines (2362 total additions + deletions). No third-party source copied in this commit. Depends on public231 PR ThinkFlowLab#52.
Birfy
left a comment
There was a problem hiding this comment.
Scoped independent review of 7421849, including the new consumer slice and its #52 integration. No confirmed routing/scoring correctness defect from this pass; ticket verification still has the reproducibility gap below, so this is not overall approval.
Validation on Linux/Python 3.12:
- Exact source tree matched GitHub tree cae572d6396e7836e2d9d09f5699018293d7d701.
- Default suite: 757 passed, 163 skipped, 144 subtests passed.
- With all eight hash-pinned JevBench files, the exact companion #131 collector, and a locally reconstructed public231 input matching the published SHA-256: benchmark suite 91 passed, 40 skipped. All 9 new public231 consumer tests passed. These are synthetic saved-response checks, not revalidation of the reported model-quality numbers.
- Repository-wide Ruff check/format and configured ty check passed; smoke passed without live model calls.
All remaining 40 benchmark skips are ticket tests. The original ticket artifacts remain unavailable pending the stated redistribution permission; this review does not ask for restricted data to be published. Please make a redistributable synthetic fixture/contract available so the committed negative regressions execute in a clean contributor/CI checkout. The existing #52 dependency, pending replay evidence and exact-head CI gaps remain; keep Draft.
Birfy
left a comment
There was a problem hiding this comment.
Approve — offline public231 quality consumer (depends on #52).
Reviewed quality_public231.py, accuracy leaderboard, CI expansion (tests/benchmarks + Omni collector pin), and tests. Consumer correctly keeps planned denominators, separates wire validity from scorer validity, and rejects mismatched frozen input/collector/config. Locally: public231 suite 88 passed, 3 skipped (skips need OMNI_BENCH_SOURCE; CI supplies it and was green with zero skips).
Merge order: land #52 first, then refresh this branch if needed and merge. Maintainer demo/exception items in the description remain process follow-ups, not correctness blockers for this offline scorer.
Birfy
left a comment
There was a problem hiding this comment.
Follow-up (Birfy) — 2026-10-17
Prior approval still stands on the design. Current GitHub status is CONFLICTING with main — please rebase (likely CHANGELOG / public231 paths) and confirm CI is green on the new head before merge. I will not re-approve until the conflict-free head is up.
Signed-off-by: linear3735 <heshming@qq.com>
Birfy
left a comment
There was a problem hiding this comment.
Re-review (Birfy), head 04037db — 2026-10-19
Thanks for the rebase. Re-checked the two new commits against main 0fe20f5:
- Merge of
upstream/main(7614801): the diff againstmainis back to the PR's own scope (7 files:benchmarks/quality_public231.py, its tests,benchmarks/README.md, the leaderboard,benchmarks/evidence.py, the workflow leg and one CHANGELOG line). The CHANGELOG entry sits cleanly above thes1a-mcpannotations entry from #84. Fix frozen collector execution to use verified source bytes(04037db): good catch.spec.loader.exec_module()re-reads the file (and may pick up a timestamp-valid.pyc), so the hash check and the executed code could diverge. Compiling and executing the exactsource_bytesthat were hashed closes both the TOCTOU window and the stale-bytecode path, and the two new tests pin each case.
Local check: full suite on this head, 991 passed, 127 skipped. CI is green on all legs, the PR is not a draft, and GitHub reports it mergeable.
Non-blocking nit for later: the collector module is still not registered in sys.modules (same as before this commit), so if the frozen collector ever grows a @dataclass or pickling, exec will need sys.modules[spec.name] = collector first. Not needed for the current file.
Approving this head.
Keep both Unreleased Added entries: BrowseSafe-Bench sample / clip coverage and the offline public231 quality consumer from #69.
…ality ThinkFlowLab#69) Branch refresh requested by Birfy's approval review: ThinkFlowLab#52 has landed, so the replay collector is merged forward onto the current main before ready-for- review. No conflicts. Co-Authored-By: Claude Code <noreply@anthropic.com>
Required human contributor answers
What outcome are you trying to achieve?
Answer: Recompute accuracy and calibration from saved public231 responses while retaining failed and unprocessed questions in the denominator.
What is the highest-leverage variable for that outcome?
Answer: Replayability of the scoring evidence.
How will this work move that variable rather than create busywork?
Answer: Validate the input, raw archive and original collector; reuse the pinned scorer and reject skipped benchmark tests in CI.
Who is the customer or user, and what do they actually need?
Answer: Contributors and reviewers need one command to recompute results and inspect the request and task counts.
After answering the four questions: is a PR needed at all to achieve this outcome?
Yes. A shared, versioned tool and CI checks let future contributions reuse this workflow.
For a bug fix: link actual observed failure evidence.
Evidence: This is primarily a saved-response scoring feature; no live runtime path changes (
s1a, the agent loop, inference backends and serving code are untouched). The conflict-refresh review also reproduced a collector source-loading defect: hashing source alone did not ensure that the executed content was exactly those verified bytes. The follow-up reads source once, verifies those bytes and executes only that same object. Two regressions cover a timestamp-valid untrusted bytecode cache and source replacement after hashing; both failed on the prior candidate and pass after the fix.Why
Closes #68. Saved decision responses need reproducible accuracy scores that keep
failed and unprocessed questions in the denominator.
#52 merged on 2026-10-14. This branch now contains upstream main
0fe20f5044d455126c81f668ac9d28d706e6d3c0through a normal merge, preserving existing history. The #52 collector is therefore in the base rather than an inherited change in this PR. Both the offline-quality and upstream MCP changelog entries are retained; the shared CI keeps the pinned Omni collector, all benchmark tests and the zero-skip guard while adopting main’s setup-uv version.How
Check the saved workload, collector version and response evidence before calling
the pinned JevBench scorer. Preserve original probabilities and all 231 planned
questions. Report response validity, accuracy and calibration separately.
The benchmark CI job fetches the fixed public reference and collector,
builds the input fixture and fails if any benchmark test skips.
What
Review
quality_public231.py, its tests and the CI job in that order. This changedoes not alter
s1a, the agent loop, inference backends or serving code.The historical ticket auditor is outside this PR; its original source and evidence
are retained separately. This tool does not validate ticket episode completion.
Verification
Submitted head:
04037db6cff781084d944495ef61433990f9e0f6; base/merge base:0fe20f5044d455126c81f668ac9d28d706e6d3c0. Full diff: 7 files, +666/−3 (669 changed lines). This update resolves the CI/CHANGELOG conflicts and executes exactly the verified collector source bytes, preventing cached or reopened content from bypassing that check. The normal merge and follow-up fix preserve the earlier evidence-bound commits.Local CPU checks on merge candidate
7614801343171efa0f7c2cda0954186bcf3f3c70: benchmark suite 91 passed, 0 skipped, including its JUnit zero-skip guard; full suite 1,069 passed / 46 skipped / 1 deselected / 203 subtests passed, with that GIF case run separately and skipped because the report extra was absent (47 skips in total). Ruff format/check,ty, smoke anduv pip checkpassed in the existing environment. On final head04037db, the pinned benchmark suite passed 93 tests / 0 skipped, including the two new regressions; the JUnit zero-skip guard, Ruff format/check,tyand diff checks passed. The full suite was not repeated for this two-file loader repair. Skips remain unverified; this was not a clean install or a live model/GPU run.Fresh independent reviews cover the final revision: Codex R1 found the source-loading P2, the two-file repair closed it, and both Codex R2 and a fresh Kimi R2 static review passed at
04037db6cff781084d944495ef61433990f9e0f6. R1 reviews cover unchanged merge/feature content; R2 reviews cover the exact repair and its interaction with the supplied final files. Codex independently reran all 93 benchmark tests and the old-red/new-green probes; Kimi reviewed frozen source and command records without executing tests. These are contributor reviews, not maintainer approval.As of 2026-10-17 17:07:41 UTC, GitHub head matches
04037db6cff781084d944495ef61433990f9e0f6and is MERGEABLE (no conflicts; overall merge stateCLEAN). Exact-head Actions: test: SUCCESS. Head check snapshot: 2 SKIPPED, 4 SUCCESS. The earlier green Actions run for66636afand Birfy’s approval on that head are historical, not validation or approval of the new head. Birfy’s 2026-10-17 follow-up requests a conflict-free head and new-head CI before re-approval. Windows/live-browser coverage is not established by skipped jobs.Historical consumer replay of the saved campaign (2026-10-13; head
66636af9): the consumerat that frozen head re-scored the frozen campaign-416135 archives on CPU and reproduced
the published numbers — Open-Jev-9B 179/231 correct in every round (537/693
pooled, accuracy 0.7748917748917749) and Open-Jev-27B-v1.1 198/231 in every round
(594/693 pooled, 0.8571428571428571); 231 distinct tasks, 0
failed/incomplete/unattempted; Brier 0.320752 / 0.240493 and
ECE 0.086786 / 0.130333 (full precision in the receipt). A second local re-execution
reproduced both
summary.jsonandrecords.jsonlbyte-for-byte (original hashes in thereceipt). This repeat checks reproducibility of the local run; it is not external
reviewer reproduction or authentication of the historical raw archive. The historical benchmark suite at
66636af9with the pinnedreference and frozen collector: 91 passed, 0 skipped (see the prepared JUnit
XML in the verification package). These replays score historical saved responses; they are not new model
runs, current-GPU validation or evidence of the application path. Quality timestamps
and latencies are null. Deployment identity comes from saved metadata; build
ancestry is not independently established.
Image/VL, screenshots, browser tasks and higher concurrency are outside this scope.
Demo / evidence
Five-model report
Pinned JevBench reference
Public five-model summary bundle from #52: campaign 415076 aggregate summaries, run metadata and collector replay receipt. Per-question predictions, probabilities and raw responses are omitted; this is not a complete archive for independent re-scoring.
Frozen-consumer verification package: replay of saved campaign-416135 responses through this PR's head
66636af9— receipt (source head, collector hash5aa20a2c…, input payload hash, per-arm round/pooled counts, output hashes), the two replayedsummary.jsonfiles (local paths normalized; original hashes in the receipt), the recorded commands with local paths and private endpoint identity redacted (verification/execution.json) and the 91-passed/0-skipped JUnit XML. Questions, raw requests/responses and per-question records are omitted under the same publication boundary as [Feat] Add a reproducible public231 decision benchmark #52; the full bundle is preserved locally. Sharing it requires confirmation of redistribution rights and a separately authorized disclosure route; none is established here.That [Feat] Add a reproducible public231 decision benchmark #52 bundle covers the [Feat] Add a reproducible public231 decision benchmark #52 collector, not this PR's
quality_public231.pyconsumer; this package documents execution of the frozen66636af9consumer. It is a verification report, not an executable replay archive: the input manifest, collector/reference files and raw rounds are absent. The historical ticket replay does not validate this consumer, and neither package is the full raw archive required for fully independent re-scoring.uv run ruff format --check . && uv run ruff check . && uv run ty checkuv run pytest -qandscripts/smoke.shCHANGELOG.mdand the docs say what the code does nowMaintainer decisions requested