Repository navigation
Conversation
lanyue-llk
left a comment
There was a problem hiding this comment.
Reviewed the current head (d6259010) against the base, including configured selection, native request/response contracts, retry/deadline handling, evidence retention and report integration. Targeted offline tests passed; no paid provider calls were made. One reproducible diagnostic regression needs correction (inline).
The shared hosted adapter is a reasonable abstraction at this size; splitting it into five classes would not address an observed problem.
Nonblocking repository requirement: CONTRIBUTING.md requires a conventional PR title. Suggested title: feat(webfetch): add reusable ten-method native configuration.
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 1 issue(s), including 1 blocking issue(s).
There was a problem hiding this comment.
✅ Re-review complete · 2e45c59
No findings in this incremental review.
lanyue-llk
left a comment
There was a problem hiding this comment.
Fairness audit of current head 2e45c593: checked the selected ten methods' input URLs, grading hints, output projection, caller deadline, retry selection, known origin statuses and scorer integration. The runner, offline rescore and report paths reuse the shared scorer/task predicate. Fixed-native requests reject per-case retrieval assistance; native output routes follow declared task kinds. Frozen proxy/render/cache options and different product internals are disclosed configuration differences, not evidence of grader-assisted retrieval.
One inconsistency in the selected cohort affects both acceptance and retries (inline). The cause is inherited in Firecrawl; selecting it alongside the new hosted adapters now combines incompatible interpretations of known target-site errors.
Controlled native HTTP responses and an executable Moli fixture went through the actual run_evaluation -> score_attempt -> task_passed path. With correct body text and origin HTTP 200, all ten methods pass in one physical call. Change the disclosed origin status to 404: Moli, Cloudflare, Browser Use, Browserbase and Crawl4AI Cloud fail and use the configured retry; Firecrawl still passes without retry. Tavily, Exa, Bright Data's raw route and DeepSeek do not expose origin status in these response contracts and were not assigned an invented status.
Related targeted tests: 204 passed, 1 skipped, 61 subtests passed after supplying the locked optional dependency in the isolated test environment. No live provider calls were made. Keep one scorer and add a shared known-status policy plus a cross-adapter matrix; there is no need for another runner or a larger class hierarchy. The scoring-package dependency remains pinned to its merged version, so this PR does not yet execute PR12's proposed rule revision.
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.
✅ Re-review complete · cc35b96
No findings in this incremental review.
lanyue-llk
left a comment
There was a problem hiding this comment.
Re-reviewed cc35b960 against base efb02b96, including the shared origin-status correction, strict JSON decoding, native evidence projection and live/offline consistency.
The previous body-task status issue is fixed: six methods exposing origin status reject disclosed HTTP404 and use the configured retry; methods with unavailable origin status retain that distinction. Native timeout versus caller deadline classification also remains covered.
Two issues remain (inline): resource/file routes still omit observed origin statuses, and strict JSON acquisition disagrees with the saved-evidence parser on BOM/UTF16 responses. Both were reproduced through the actual runner, scorer and public artifact-verification boundary. The JSON matrix covers all four hosted JSON adapters with normal UTF8, invalid UTF8, UTF8 BOM and UTF16 controls.
Relevant local checks passed, including 279 tests/45 subtests for the boundary, retries, hosted adapters, saved routes and public logs. Additional certification and live-to-offline fixture checks passed. No paid service calls or source edits were made.
There was a problem hiding this comment.
✅ Re-review complete · 28a12de
No findings in this incremental review.
lanyue-llk
left a comment
There was a problem hiding this comment.
Re-reviewed current head 28a12de8 against base efb02b96, including the latest shared decoder and resource/file status fixes. The two previous findings are verified fixed: all four hosted JSON methods now preserve verifiable BOM/UTF16/invalid-UTF8 failure evidence, and Cloudflare resource/Moli raw-file routes propagate observed origin status before the shared acceptance/retry decision. Paired controls cover HTTP200/404/503, unavailable/invalid statuses and response encodings through the actual runner, scorer and public evidence boundary.
No new actionable defect was found in this revision. Relevant adapter checks passed 351 tests and 46 subtests; separate shared-record/projection/certification/CLI/dependency checks passed 296 tests and 2 subtests. No paid provider calls or source edits were made.
lanyue-llk
left a comment
There was a problem hiding this comment.
Reviewed the current native-method configuration and shared execution/scoring boundaries alongside PR12's latest dataset. One remaining method-comparability defect is reproducible: the five new hosted methods reject document-body tasks before making a native request. The shared timeout, declared retry/final-response policy, artifact verification, and scorer binding otherwise behaved consistently in the reviewed controls. No provider credentials or paid calls were used.
|
|
||
| def fetch(self, case: EvalCase, timeout: float, *, native_route_ledger=None) -> ProviderResult: | ||
| self.check_native_call(case) | ||
| if case.task_kind not in {"body", "structure"} or case.track not in {"web", "dynamic", "challenge", "risk_control", "articles"}: |
There was a problem hiding this comment.
[P1] Let document-body tasks reach the native URL endpoint
The current dataset contains ten document body tasks, including the plain-text URL https://www.rfc-editor.org/rfc/rfc9110.txt and HTML document pages. This track whitelist forces all five new hosted adapters to return unsupported_task before contacting their services. In a controlled public-runner test with the same URL, input-media metadata, expected body and native response, changing only track from web to document changes every method from PASS/one transport call to FAIL/zero calls; the resulting failure artifacts still pass verification. Thus the comparison records an adapter-imposed label restriction as provider failure. This is particularly clear for Crawl4AI, whose existing /scrape endpoint also documents PDF-to-text support (https://api.crawl4ai.com/docs/#scrape).
Allow document body/structure tasks through the same native URL request and score the actual response/error. This needs no helper downloader or converter. If the intended cohort is webpage-only, predeclare and apply that population to all ten methods instead of hard-rejecting document labels for only these five.
Summary
Web Fetch now has a reusable ten-method native configuration. Select
--providers configured --provider-config configs/providers.example.jsonthrough the existing validation, diagnostic, benchmark or certification entry point. The shared default is 30 seconds per physical call and one failure retry. Each retry receives a fresh budget; the final response is selected, all physical calls are retained, and total retry cost remains separate from final-call latency.Add one shared Markdown adapter for Bright Data, Browser Use Fetch, Browserbase Fetch, DeepSeek Fetch API and Crawl4AI Cloud. Register their interface examples, account requirements and bilingual method descriptions through the existing catalog templates.
Configure these five methods plus Moli, Firecrawl, Cloudflare Markdown, Tavily and Exa with fixed native parameters and caller debug logs. Exa uniformly requests up to 100 native rich links; Crawl4AI Cloud uses its native residential proxy. Remove Moli's per-host private-network exceptions while retaining security checks and the pinned binary identity.
Reject per-case retrieval assistance before transport. Use native URL routes without helper document downloads or scoring-driven parameter changes. Keep the 20 MB HTTP response/Moli output ceiling and product-native capabilities.
Stop fixed-native timing after the complete response and required decoding, before scoring preparation and evidence persistence. Do not override Browser Use's internal timeout. Quarantine still-running timed-out workers instead of launching overlapping retries.
Preserve exact raw responses and sanitized caller diagnostics. Distinguish native timeout failures from caller-enforced deadlines, strictly decode hosted JSON and retain invalid UTF-8 as raw evidence with an empty answer, and prioritize explicit service errors over missing success fields. Invalid Crawl4AI engine/proxy types produce validation errors instead of crashing validation.
Preserve observed target-site statuses on body, image/video resource and raw-file routes. Share strict hosted JSON decoding between acquisition and saved-evidence verification, including BOM/UTF16 failures.
Normalize explicitly disclosed target-site HTTP failures in the shared call boundary before acceptance and retry selection. Preserve service API status separately from target-site status; absent target status stays unknown. Keep response evidence and historical records unchanged.
Scope and diagram clarifications
The configured cohort is reusable configuration, not a report instance. Publishable cohorts still require frozen task selection and the existing certified pipeline. Explicit provider lists and timeout/retry overrides remain available and recorded. Historical diagnostic adapters remain selectable; the no-argument CLI remains an offline fixture.
The five new hosted adapters implement Markdown webpage body/structure output. Existing methods retain task-specific text, resource or file routes; all ten are not universally Markdown-only. “Removed assistance” in the diagram refers to the selected fixed-native execution path, not deletion of every legacy diagnostic route. Removing an internal timeout override is a configuration choice, not evidence that a product-native timeout is cheating.
Requests must match the predeclared task-family contract, not identical parameters across products. Each attempt is scored before the failure-retry decision. No hosted JSON/schema output or parallel collection runner is introduced; the existing
--measure-onlydiagnostic entry remains supported.Local public-cohort execution requires usable public DNS. The inspected environment resolved one benchmark domain to a private address; retain the preflight rejection and fix the environment rather than adding host exceptions or replacing benchmark cases without justification.
Validation
28a12de8. Complete CI passed: 7,118 tests and 2,981 subtests passed (157 skipped); rendered reference checks passed 78 tests (1 skipped). Packaging checks passed. Latest OCR re-review passed with no findings. The shared decoder lives inharness/record_io.pyto keep evidence readers independent of provider imports.efb02b96, retaining its shared source-validation changes and absolute imports. Before refreshing the route baseline, substituting the previous projection-source identity reproduced the prior synthetic fingerprints; request parameters did not change.proxy=residentialreturned content within the 30-second deadline and retained response/log evidence. It verifies that the configuration is callable, not IP rotation, improved success rate or monetary cost. Review corrections made no live service calls.