Repository navigation
[Feat] Use native JEV-VL for browser decisions - #78
Conversation
Replace four stale unverified statements with the single-H800 and RECOVERABLE R3 scope. Preserve forced-choice exclusions, failed attempts and the external-evidence publication gap. Runtime, tests and configuration remain at validated 0def927. Complete candidate: 16 files, 1207 changed lines; authored Python and tests: 1143 changed lines.
|
Independent scoped verification of head Personally rerun in an existing cloud Python 3.12 environment, with this head's checkout on the import path:
These are offline checks with mocked HTTP/page fixtures and CLI/MCP smoke, not live browser/model/GPU validation or a clean-install test. Skips remain unverified; no paid/model calls were made. The already-disclosed changelog conflict, absent exact-head GitHub CI and unpublished reviewer-accessible video/raw evidence remain readiness gaps, not new findings. Keep draft while those are unresolved; this comment is not maintainer approval. |
Preserve the upstream Public231 entry and the unchanged JEV-VL entry under Added. Move JEV-VL below the existing Cua entry so the current main merges cleanly without rewriting either entry. No runtime, test or configuration changes.
Birfy
left a comment
There was a problem hiding this comment.
Re-review at ecfa02f against main f91d006.
The only change since my verification of 7250a50 is to CHANGELOG.md (+6/−3). It moves the Cua-S1 4B entry above jev-vl-served and keeps the Public231 entry from main word for word. I checked that by merging locally: git merge-tree --write-tree origin/main ecfa02f exits 0. The merged ### Added list has Public231, then Cua-S1 4B, then jev-vl-served, then desktop text input, then Snake. Nothing is duplicated or dropped. The blank line after the Public231 bullet comes from main, not from this PR. Runtime code and tests are unchanged from the head I reran, and exact-head CI is green: core 3.11 and 3.13, full, and benchmarks. Windows and browser are skipped under the PR configuration.
I found no new code defect. The one remaining blocker is the one the PR already states: the same-trial video and the sanitized raw traces have no URL a reviewer can open. Keeping it in draft until they're linked makes sense. Once they are, I'm ready to re-check and approve.
|
@Birfy The same-trial video and sanitized raw records are now linked in the PR's Demo / evidence section:
Both links were downloaded without GitHub authentication after upload; sizes and SHA-256 match the frozen files. The downloaded ZIP was extracted and its standard-library The packet keeps all three attempts, original/published hashes, 4 non-forced operations versus 6 forced targets, native wire/image links, POST/oracle, READY→RELEASED snapshots and the exact recording-close adjustment. Runtime evidence stays bound to Agents Could you re-check the linked evidence and complete your review if it satisfies the remaining blocker? Keeping this PR Draft; no merge or auto-merge has been performed. |
Birfy
left a comment
There was a problem hiding this comment.
Rechecked the published evidence for unchanged head ecfa02fa903ea4a3eab7f787babeaf7b81935abc.
Downloaded the ZIP without GitHub authentication; its SHA-256 matches 8e3242a6c275c4c1528f8a7643d2b8dcac2db3e9e2ebef4a1195cc8a095d78c6. Independent offline checks verified all 216 bundle-manifest files, 211 source records (107 renderer sources), seven HTTP-200 request/response body hashes, byte-identical decision PNG links, and response argmax choices. The records preserve CLICK → TYPE_TEXT → CLICK → DONE, six forced-target exclusions, the hello world POST/oracle, all three attempts, and the RELEASED lease.
The embedded replay matches the advertised MP4 hash; ffprobe reports 294 frames / 11.76 s. Sampled frames show locked → editing → submitted. The right-hand panel is static trial context, not synchronized server output.
This resolves the previously reported missing reviewer-accessible artifact gap. No new actionable defect found in this evidence check. These are consistency checks of retained, partly redacted records bound to Agents 0def927 / Omni 26dc76b, not independent authentication of the originals or a fresh model/browser/GPU run. Auxiliary-model checkpoint/cost, broad task quality and end-to-end speedup remain unverified; PR Windows/browser CI remains skipped. Formal maintainer acceptance is still separate. Leaving this as COMMENTED; Draft state unchanged.
Birfy
left a comment
There was a problem hiding this comment.
Follow-up review (Birfy)
Re-checked 2026-10-17: still draft and CONFLICTING with main. Prior feedback stands.
Please rebase onto current main (CHANGELOG / decision-model factory churn from recent merges and open stacks like #86) and re-request review once green and conflict-free. Keeping as draft until the native JEV-VL path has a clear validation row in docs/capabilities.md is still appropriate.
Signed-off-by: linear3735 <heshming@qq.com>
Signed-off-by: linear3735 <heshming@qq.com>
There was a problem hiding this comment.
Re-review (Birfy), head 0dd1693 — 2026-10-19
Thanks for the rebase and the evidence write-up. The branch is now conflict-free with main 0fe20f5, CI is green, and the full suite passes locally on this head (1107 passed, 117 skipped). The validation row in docs/capabilities.md and the "Served JEV-VL" section now say clearly what was validated (one bounded R3 fixture on one H800, all three attempts retained, forced choices excluded) and what was not, which is what I asked for last time.
The adapter itself reads well: exactly one PNG/JPEG, Pillow verify() + load() before a forced choice can skip HTTP, a single shared deadline across questions, and no silent re-ask.
A few things I'd like settled before this leaves draft:
decision_rawin every browser tick (s1a/browser/decision_model.py).record["decision_raw"] = decision.rawis gated ondecision.provenance, which is also non-empty for the existing HTTP backendslaya-servedandclm(both seturl/request_id/served_by), not just JEV-VL. So this silently changes step records for the existing backends too:laya-servedticks will now carry its whole response payload. Either scope it to JEV-VL, or call it out in the CHANGELOG and add a test for the existing served backends' record shape.- Full response headers are recorded.
_postreturnsdict(response.headers)and it lands inraw["responses"][i]["headers"], then indecision_rawon disk. Behind a proxy or gateway that can includeset-cookieor auth-related headers. Please keep an allowlist (server-timing,x-request-id, content type/length) rather than everything. - Forced choices get a
request_idfor a request that was never sent. For a one-option choice the record carries a freshuuid4request_idwithforced: true. Minor, but it makes it look like there's a server-side request to correlate;Nonewould be clearer.
None of these block the design. Happy to approve once (1) and (2) are addressed and the PR is marked ready.
Edited: corrected point 1 (plain jev sets no provenance, so it is unaffected) and dropped a note about MAX_BODY_BYTES, which is enforced in _decide.
…ice IDs Addresses the three points of Birfy's 2026-10-19 re-review at 0dd1693: decision_raw is retained only for jev-vl-served ticks so the existing served backends keep their provenance shape without a full payload; per-response headers are restricted to server-timing, x-request-id, content-type and content-length so proxied deployments cannot leak cookies or tokens into evidence; forced choices carry a null request_id (and an all-forced decision a null top-level request_id) because no HTTP request was sent. Co-Authored-By: Claude Code <noreply@anthropic.com>
Birfy
left a comment
There was a problem hiding this comment.
Re-reviewed at 5f6f0d6. All three points from my 2026-10-19 review are addressed:
decision_rawis now kept only forjev-vl-servedticks, so the other served backends keep their record shape.- Response headers are allowlisted (
server-timing,x-request-id,content-type,content-length). httpx lower-cases keys initems(), so the set comparison is correct. - Forced choices carry
request_id: null, and an all-forced decision has a null top-level id; the new tests cover both.
CI is green and the branch merges cleanly. Two small, non-blocking things:
- The check in
s1a/browser/decision_model.pyis a string comparison onself._decision_model.name == "jev-vl-served". A class attribute on the backend (for examplerecords_raw = True) would avoid a renamed backend silently dropping the trace. - The top-level
request_idis a fresh uuid that matches none of the per-question request ids. If it is only a decision id, calling itdecision_idwould avoid someone grepping server logs for it.
Happy to approve once it leaves draft.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Light-impact review (maintainer) — draft, no approval
Tip: 5f6f0d646f984c5f61c508c854242ddd97b95e05 · draft · MERGEABLE · required checks green on this head (core 3.11/3.13, full, benchmarks; browser/Windows skipped by workflow).
Impact
- changed: new
ServedJevVLModel(jev_vl_served.py); factory + browser registration; provenance (decision_rawonly for this backend; header allowlist; nullrequest_idon forced); sharedunit_interval/ token-count overflow order; docs/CHANGELOG - callers/dependents: browser decision path +
build_model("jev-vl-served"); validation helpers shared with other backends - related tests: adapter + registration + browser provenance suites
- not expanded: Omni worker internals; live GPU/browser re-run of frozen R3 evidence on this tip
Risk: high (new decision backend on the browser path; image/HTTP fail-closed)
Missing tests: none obvious for adapter contracts; live tip GPU/browser revalidation remains historical (bound to older frozen Agents/Omni SHAs as documented)
Birfy’s 2026-10-22 re-review of the three prior nits looks correct on this tip. Remaining non-blocking notes (same as Birfy): prefer a backend capability flag over name == "jev-vl-served" for retaining decision_raw; top-level request_id is a fresh uuid (consider decision_id naming).
Verdict: COMMENT only — leave draft; happy to approve once marked ready for review. Not merging, not marking ready.
Preserve main's confidence, model identity, progress reporting and direct Jev model pin alongside JEV-VL provenance and scoped raw browser traces. Retain both documentation histories and add five integration assertions. Restore the upstream local-service timeout example after Kimi review. Frozen reviewed tree: e2d7f53 Parents: 5f6f0d6 and 9b20b98 PR vs main: 16 files, +1255/-16; authored Python/test changes: 1195 lines. The merge also absorbs existing upstream changes; no new runtime artifacts, dependency, GPU kernel or protocol branch is added. Validation: 1258 passed, 130 skipped, 251 subtests; ruff, ty, smoke, offline lock/build and diff checks passed. Independent Codex and Kimi reviews passed; final docs-only delta was rechecked by Kimi. User approved this exact diff for commit and fork push; keep PR Draft.
Birfy
left a comment
There was a problem hiding this comment.
Re-review (Birfy): approve
Head bc750458 (merge of main at 9b20b98 into the branch), now ready for review. Since my 2026-10-22 review of 5f6f0d64, the only change is that merge. I compared the PR's own diff before and after it: the only differences are the conflict resolutions against #93 (the requested_model lines in BrowserDecisionModel are now covered by the existing **decision.provenance spread, so it is still recorded), the extra assertions from main's tests, and the S1A_DECISION_TIMEOUT_S row picking up main's wording. Nothing new in the JEV-VL adapter itself.
Re-run locally on this head: ruff check clean, pytest -q 1258 passed, 130 skipped, 251 subtests passed. CI is green on this head (core 3.11/3.13, full, benchmarks).
The two non-blocking notes from earlier still stand as follow-ups (use a backend capability flag rather than name == "jev-vl-served" for keeping decision_raw, and consider naming the top-level fresh uuid decision_id). Approving.
hsliuustc0106
left a comment
There was a problem hiding this comment.
Light-impact + repo review (maintainer) — approve
Tip: bc750458a40f04ceae91c20a2d8d1ee386a2c162 · ready · MERGEABLE · exact-head checks green (core 3.11/3.13, full, benchmarks; browser/Windows skipped by workflow).
Impact
- changed:
ServedJevVLModel(jev_vl_served.py); factory + browser registration;decision_rawonly for this backend; header allowlist; null forcedrequest_id; shared overflow-order inunit_interval/ token counts; docs/CHANGELOG - callers/dependents: browser decision path +
build_model("jev-vl-served") - related tests: adapter + registration + browser provenance suites
- not expanded: Omni worker internals; live tip GPU/browser re-run (historical R3 evidence remains bound as documented)
Risk: high (new decision backend) but fail-closed image/HTTP path, no silent retry, and Birfy’s prior blockers are addressed on this tip.
Missing tests: none obvious for adapter contracts.
No blocking findings under .claude/skills/review-pr on this tip. Non-blocking follow-ups (same as Birfy): prefer a backend capability flag over name == "jev-vl-served" for retaining decision_raw; consider renaming the top-level fresh uuid to decision_id.
Verdict: APPROVE. Merging with a merge commit to match recent maintainer practice.
Required human contributor answers
The four answers below retain the contributor intent recorded in System1-Omni #139, which includes Agents integration. On 2026-10-25 the contributor explicitly approved this main-integration diff for commit and fork push. This is contributor authorization, not maintainer approval or new runtime validation.
Is a PR needed? Yes. The contributor explicitly requested publishing this reviewed Agents integration after the Omni online-image change.
Observed failure evidence: This is a backend integration feature. Malformed PNG data and huge integer probabilities/usage were observed during local negative probes and are pinned by the adapter regressions. The previously missing backend registration is covered by registration tests. Probe outputs and independent review reports are retained outside Git and are not yet public artifacts.
Why
Browser agents need a registered backend that sends their current screenshots to System1-Omni's native JEV-VL worker. This adds
--model jev-vl-servedwith explicitJEV_VL_URL, so the existing browser policy can use that worker for operation selection. The upstream serving implementation, Omni #139, is now merged; this PR supplies the Agents integration.How
The adapter enforces image/body limits and PNG integrity, preserves option indices, uses one absolute deadline including image preparation, and fails the complete decision on invalid data. It performs no HTTP retry, answer re-ask or partial action. A single-candidate choice is forced locally after image validation and makes no model request. Chat/value still supplies free text and summaries.
Review order:
jev_vl_served.pyand its contract tests; factory/browser registration; browser provenance; then the two shared integer-overflow checks intypes.py/validation.py. Existing browser execution and tool loops are reused. No new dependency, tool/desktop/rail integration, or CLI decide registration is added.What
Current head:
bc750458a40f04ceae91c20a2d8d1ee386a2c162; current main and merge base:9b20b989d919bc69227f81bc0ec571ca56925586. Full PR diff: 16 files, +1,255/−16 (1,271 changed lines). Authored Python source/test changes: 1,195 lines, counted separately from documentation. Main-only changes are inherited upstream work, not newly authored JEV-VL code.Main integration (2026-10-25)
Normal merge commit
bc750458a40f04ceae91c20a2d8d1ee386a2c162has parents5f6f0d646f984c5f61c508c854242ddd97b95e05and9b20b989d919bc69227f81bc0ec571ca56925586; no published history was rewritten. Four conflicts were resolved:CHANGELOG.mdanddocs/capabilities.mdretain both JEV-VL entries and main's new features/evidence, with the original runtime revisions and limits.docs/configuration.mdretains main's direct/OpenRouter Jev model pin and local-server timeout example alongsideJEV_VL_URL, the shared decision deadline and JEV-VL's no-retry rule.s1a/browser/decision_model.pyretains main's model identity, unified confidence/backend-confidence fields and progress/max-steps reporting alongside serving provenance and JEV-VL-onlydecision_raw.max_stepspropagation. The JEV-VL protocol adapter itself is byte-identical to5f6f0d6; all 66 files changed only on main retain main's blobs and modes.Review follow-up (2026-10-21, head
5f6f0d6)Single commit
5f6f0d6resolves all three points of Birfy's 2026-10-19 re-review: (1)decision_rawis retained only forjev-vl-servedbrowser ticks, solaya-served/clmkeep their provenance shape without a full response payload (a regression test covers both existing backends); (2) raw per-response headers are restricted to an allowlist (server-timing,x-request-id,content-type,content-length) so proxied deployments cannot leak cookie/authorization material into evidence (the fixture now plants private headers to prove they are dropped); (3) forced single-option choices record a null per-responserequest_id, and an all-forced decision a null top-levelrequest_id, because no HTTP request is sent. Local verification on this head:python -m pytest -q1,109 passed / 117 skipped / 204 subtests passed;ruff check .andruff format --check .clean. The PR then stayed Draft pending re-review and maintainer approval; no new GPU/browser run was performed for this follow-up.The normal merge preserves both the upstream MCP and JEV-VL CHANGELOG entries. It adds the native JEV-VL row requested in
docs/capabilities.md, binding the existing public evidence and its limits to Agents0def927/ Omni26dc76b. The follow-up documentation commit replaces the obsolete statement that all run-local media are unpublished: the same-trial video and sanitized browser R3 packet are public, while the complete GPU raw campaigns remain unpublished. No evidence archive or historical runtime result is changed.Verification
Local checks ran on frozen candidate tree
a9604be5837b8f37aa142753f4235137de849252, using the existing Python 3.11.15 environment with this checkout on the import path. Final commitbc750458a40f04ceae91c20a2d8d1ee386a2c162has reviewed treee2d7f5395ede38f71e6ee3a4f96411954ec34842: the only subsequent change restores one upstream timeout example indocs/configuration.md. All Python, tests and other documents are byte-identical to the tested tree. Kimi independently rechecked that final documentation delta and reran smoke with explicit Python/CLI overrides. No GPU/model/browser task was rerun for this integration.ruff format --check .,ruff check ., andty check --python <existing-environment-python>: passed.PYTHONPATH=. python -m pytest -q -rs: 1,258 passed / 130 skipped / 251 subtests passed; existingscripts/smoke.sh: passed. The skips are unverified coverage, not passes.CHANGELOG.mdand docs preserve both sides' behavior and the historical evidence boundaries.uv lock --check --offline,uv build --offline, and Git diff checks: passed.tests/system, a real model/GPU service and live browser tasks were not run on this candidate. The historical R3 demo below remains bound to its executed revisions; it is not exact-head runtime or current-ABI acceptance.Exact-head GitHub status
As checked on 2026-10-25 (Hong Kong), GitHub reports head
bc750458a40f04ceae91c20a2d8d1ee386a2c162and merge base9b20b989d919bc69227f81bc0ec571ca56925586, no conflicts (MERGEABLE), and the PR is ready for review / unmerged. Exact-head test run 40119138709 completed SUCCESS: benchmarks, Linux core (Python 3.11), Linux core (Python 3.13), and full all succeeded. Browser and Windows jobs were skipped by workflow conditions; these skips establish neither Windows nor live-browser coverage.The previous test run at
5f6f0d6is historical: four jobs succeeded and browser/Windows were skipped by workflow conditions. It is not CI coverage forbc750458a40f04ceae91c20a2d8d1ee386a2c162. A contributor review or local pass is not maintainer approval. Formal maintainer review remains required; exact-head checks have completed SUCCESS.Demo / evidence
Same-trial R3 evidence packet (prepared offline from the retained records; no GPU/model/browser rerun):
58550ee293bbfb9c5b1f563c03e8cbdb6f6ac2a3ef23080f73ec3435b932a9e4.8e3242a6c275c4c1528f8a7643d2b8dcac2db3e9e2ebef4a1195cc8a095d78c6; 211 retained source/support files plus README, separate original/published hash manifests, redaction notes, all three attempts and a standard-library offline verifier. Extract and runpython3 verify_bundle.py; no key/network/GPU is needed.verify_bundle.pychecks consistency of the published records; it does not rerun model quality or independently authenticate a redacted original.Birfy rechecked the published packet on 2026-10-15 and confirmed the earlier reviewer-access gap was resolved, leaving the review as COMMENTED. The 2026-10-17 follow-up requested a conflict-free main integration, exact-head CI and the native validation row supplied by this update. Formal review is pending; no maintainer approval or PR merge is claimed.
0def92771746ec94adc38bf41be9720bccfbf19d, Omni26dc76bf65c544815e7b285866791da1e703816d; single H800, native Rust/CUDA JEV-VL serving, pinnedautotrust/JEV-27B-VL@f34b598d4ef4bcefd337bee8d8e7ddd3b7733ccc.CLICK → TYPE_TEXT → CLICK → DONE, three executed actions, independenthello worldPOST verified, 12.451 s. Four non-forced operation choices; six forced targets are excluded from learned decisions; seven real visual HTTP requests returned 200 with the original screenshots. Text/summary came from the separately configured chat/value service; its provider alias was recorded, but its actual backend checkpoint and complete cost were not independently established.58550ee293bbfb9c5b1f563c03e8cbdb6f6ac2a3ef23080f73ec3435b932a9e4.Same-trial evidence links are supplied above for independent rechecking. This PR is marked ready; formal review is pending. Exact-head Actions status is reported in Verification above; maintainer approval remains required before merge. This single-fixture smoke does not establish broad task quality, Flights completion, every frontend, or production acceptance. The upstream backend dependencies are now merged: Omni #117 at
0410e841d3ec72c1462609fa7721d0e424ac356cand Omni #139 atfa5582fdd83d48420f351d2cd571eb7105021e74. Their merge does not validate this Agents head or upgrade the historical demo's protocol/model coverage. The contributor marked this PR ready for review on 2026-10-25 (state change only; the reviewed tree e2d7f5395ede38f71e6ee3a4f96411954ec34842 did not change). This update does not merge Agents #78.