Repository navigation
feat(web-extract): benchmark self-hosted engines with bounded execution - #258
lanyue-llk wants to merge 107 commits into
Conversation
![lexmount-ocr[bot]](https://avatars.githubusercontent.com/u/277637559?s=60&v=4){kind=small}
There was a problem hiding this comment.
This review found 5 issue(s), including 5 blocking issue(s).
src/harness/web_extract/certification.py(token_budget_reached): skipped round 3 of group "src/harness/web_extract/certification.py,src/harness/web_extract/cli.py,src/harness/web_extract/pipeline.py,src/harness/web_extract/self_hosted.py": used 500533 tokens exceeds budget 500000
There was a problem hiding this comment.
This review found 3 issue(s), including 3 blocking issue(s).
There was a problem hiding this comment.
This review found 7 issue(s), including 7 blocking issue(s).
src/harness/web_extract/self_hosted.py(token_budget_reached): stopped group "src/harness/web_extract/self_hosted.py" mid-review: used 540788 tokens exceeds budget 500000
There was a problem hiding this comment.
This review found 4 issue(s), including 4 blocking issue(s).
There was a problem hiding this comment.
This review found 1 issue(s), including 1 blocking issue(s).
There was a problem hiding this comment.
This review found 1 issue(s), including 1 blocking issue(s).
There was a problem hiding this comment.
This review found 2 issue(s), including 2 blocking issue(s).
There was a problem hiding this comment.
This review found 2 issue(s), including 2 blocking issue(s).
There was a problem hiding this comment.
✅ Re-review complete · 1ec4b58
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 0d3376b
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · d48847c
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 953295c
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 42ef96c
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 00de044
No findings in this incremental review.
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
Reviewed against origin/main at fe47b246. The PR merges cleanly, also with #257, and CI is green.
The approach is sound. The service validates input well: allowed snapshot origins, JSON Schema checks, a body-size limit and constant-time token comparison. Process cleanup has a real design (a subreaper on Linux, suspend then kill). The Crawl4AI and ScrapeGraphAI protocol fixes are tested against the real pinned SDKs. The cost disclosure is tied to the source plan's hash and the population size. The browser test drives the new panel navigation from start to finish.
Inline findings, in priority order:
- Must fix before leaving draft: the engine list is copied in several places (
pipeline.py). - Must fix before leaving draft:
package-checknow depends on the extraction service's SDKs (Makefile). - Must fix before leaving draft: the post-extraction runtime check does nothing for three engines (
self_hosted.py). - Cheap hardening: the container build isn't reproducible (
Dockerfile). - Cheap hardening: ScrapeGraphAI's input decoding may garble non-UTF-8 snapshots.
- Cheap hardening:
/healthexposes internal addresses without authentication. - Nit: the ScrapeGraphAI model timeout disagrees with the worker deadline.
General notes:
- Old links break. The
result-{method}-{case}.htmlroutes are removed. That's fine for new builds, since published reports stay as they are, but republishing to an existing site breaks outside links to those pages. Worth a line in the release notes. - No picture in the PR body. Please add one picture of the main content, for example the topology of the facade, the engines and the private Firecrawl backend, or the new per-case result page.
lanyue-llk
changed the title
There was a problem hiding this comment.
This review found 8 issue(s), including 8 blocking issue(s).
There was a problem hiding this comment.
This review found 4 issue(s), including 4 blocking issue(s).
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
Follow-up review of 040e50a0 against origin/main.
Addressed since the last review: thanks.
package-checkno longer depends ontest-extraction-sdk.worker()now clearslocal_runtime_identity's cache before the post-extraction check.
Still open from the last review:
- The engine names are still copied in
pipeline.py(method list, method→engine dict,normalizetuple). - The base image comes from an unpinned mirror (
docker.1ms.run), andapt-get install chromiumis unpinned. - ScrapeGraphAI still gets
decode("utf-8", errors="replace")input. /healthis still unauthenticated and returns internal addresses.- The ScrapeGraphAI model timeout is still
timeout=150.
New concern: the PR has grown from +4,884 to about +7,600 lines. Commits 315038a9, d36d594b, e0162b58 and 040e50a0 add harness/benchmark_execution.py, a scheduler shared by all tracks. It brings a policy object with purposes and named resource capacities, a dispatcher using threads or forked processes, and a required trial run and wall-time budget before collection. It also:
- moves the Web Search, Web Fetch, Browser Use and Trajectory Replay runners onto the scheduler;
- changes
configs/report-templates.jsonand the golden files for Web Fetch and Web Search; - adds a 144-line
docs/benchmark-execution.mdand a new AGENTS.md rule binding every track.
This is a separate architecture change inside a Web Extract feature PR, and much of it is over-design. Details are inline.
The new opt-in keepalive streaming mode adds a second response protocol: HTTP 200 is sent first, and failures go into the JSON body. It works around gateway timeouts that the README already handles by raising the proxy read timeout. Unless a proxy you can't configure requires it, prefer the configuration fix and keep one response shape.
The $(MAKE) -j2 test test-extraction-sdk line also conflicts with #257 once both merge (see the inline comment).
Recommendation:
- Take the shared scheduler out of this PR and leave the other tracks' runners and golden files unchanged.
- If the self-hosted engines need concurrency, use a bounded
ThreadPoolExecutorinside Web Extract's own collector, with one concurrency setting per method. - Propose a shared scheduler separately, with an issue, if more than one track needs it.
- Then fix the open items above. The engine-name duplication still blocks leaving draft.
There was a problem hiding this comment.
This review found 3 issue(s), including 3 blocking issue(s).
There was a problem hiding this comment.
This review found 5 issue(s), including 5 blocking issue(s).
There was a problem hiding this comment.
✅ Re-review complete · 7feb28c
No findings in this incremental review.
There was a problem hiding this comment.
This review found 4 issue(s), including 4 blocking issue(s).
There was a problem hiding this comment.
This review found 2 issue(s), including 2 blocking issue(s).
There was a problem hiding this comment.
✅ Re-review complete · f5a03fd
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · e48bdf4
No findings in this incremental review.
There was a problem hiding this comment.
This review found 1 issue(s), including 1 blocking issue(s).
There was a problem hiding this comment.
This review found 2 issue(s), including 2 blocking issue(s).
There was a problem hiding this comment.
This review found 2 issue(s), including 2 blocking issue(s).
There was a problem hiding this comment.
✅ Re-review complete · 9a8614b
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 5fa3b26
No findings in this incremental review.
There was a problem hiding this comment.
✅ Re-review complete · 654d11f
No findings in this incremental review.
Add native, self-hosted Crawl4AI, ScrapeGraphAI, Stagehand and Firecrawl to the existing Zyte and SWDE reports. They contribute 3,760 certified results. With the retired extruct baseline removed, the reports contain 9,680 reviewed results, zero unresolved items and zero accepted unreviewed passes. Completed calls and reviews are reused; declared repairs retain every physical predecessor. Extruct has no active adapter, dependency, registry entry or report row. Frozen source provenance remains intact.
All seven candidates have qualification evidence. Scrapy and Scrapling parse the frozen pages but require site rules or another extractor/Agent for generic schema extraction. Crawlee's Stagehand integration returned fields through the Responses protocol; it qualifies as a combination using Stagehand, with a different configuration from the directly evaluated Python SDK.
The shared execution contract covers bounded concurrency, resource admission, preflight, pacing, durable interruption/resume, account blockers and timing scope. The six-service deployment uses fresh supervised extraction processes with deadlines and crash cleanup. Startup compiles and attests installed files; requests reuse signature-checked hash hints and rehash changed files. Firecrawl binds the native server and actual external browser. Startup identities reject runtime drift, endpoint mismatches and host-generation changes. The extraction components are pinned to the qualified EPYC node after an unchanged, four-core, no-model diagnostic measured 16.223 seconds on the older Xeon node versus 4.703 seconds on EPYC. Keepalive returns explicit failure status for launch, decode, serialization and timeout failures.
A separate 12-page diagnostic compares DeepSeek's default and disabled-thinking modes with Gemma 4 e4b, Gemma 4 26B and Qwen 3.5 9B. It retains all 60 comparisons and five preflights, streamed first-output/first-answer/full-JSON timings and field scores. DeepSeek remains the default: smaller models respond faster but lose accuracy on this sample. These diagnostics do not replace full-cohort timings or establish native SDK rankings.
The deployment and reproduction guide includes the locked six-service stack, Rainbond setup, credentials, Cloudflare snapshots, both full collection commands and offline report reproduction. Services run in Efficiency / office; only the authenticated extraction API is exposed. Reports retain their original private destinations: Zyte and SWDE. They distinguish open-source participants, shorten names and cost labels, explain all candidate decisions, and document Firecrawl's AGPL commercial-use requirements.
Validation: