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.
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.
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
cc35b960. Current CI passed, including the fullmake cigate, package checks and rendered reference capture. Incremental OCR re-review passed with no findings.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.