fix(web): key extract cache on metadata.sourceURL too, pin redirect case
Follow-up to the cherry-picked "cache extracts by returned URL": Keenable and Firecrawl report the post-redirect address in `url` and the REQUESTED URL in `metadata.sourceURL`, so matching on `url` alone left every redirected page uncached. Accept either field, as long as it names a URL from this batch; anything else is served but never cached (a miss re-fetches, a mis-key poisons the cache for the whole TTL). Docs: say the cache key is the requested URL the provider reports, not the batch position. Co-authored-by: nemofq <5635994+nemofq@users.noreply.github.com> Co-authored-by: wooyongbin3-cpu <256294002+wooyongbin3-cpu@users.noreply.github.com>
This commit is contained in:
@@ -172,6 +172,25 @@ class TestWebExtractCacheAttribution:
|
||||
"https://example.com/second", "second page", "Second", format=None, provider="tavily"
|
||||
)
|
||||
|
||||
def test_redirected_page_caches_under_requested_source_url(self):
|
||||
"""Keenable/Firecrawl report the post-redirect address in ``url`` and the requested URL in
|
||||
``metadata.sourceURL``; the cache key must stay the requested URL, never the redirect target."""
|
||||
from tools import web_tools_extract as wte
|
||||
|
||||
class _RedirectProvider:
|
||||
name = "keenable"
|
||||
|
||||
async def extract(self, urls, format=None):
|
||||
return [{"url": "https://www.example.com/moved", "raw_content": "moved page", "title": "Moved",
|
||||
"metadata": {"sourceURL": urls[0]}}]
|
||||
|
||||
with patch("tools.web_result_cache.extract_cache_put") as cache_put:
|
||||
asyncio.run(wte._dispatch_extract(_RedirectProvider(), ["https://example.com/old"], None))
|
||||
|
||||
cache_put.assert_called_once_with(
|
||||
"https://example.com/old", "moved page", "Moved", format=None, provider="keenable"
|
||||
)
|
||||
|
||||
|
||||
# ─── availability / auto-detect ───────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -180,12 +180,18 @@ async def _dispatch_extract(provider, fetch_urls: List[str], format: Optional[st
|
||||
if results and all(r.get("error") for r in results) and _rescue_eligible(provider):
|
||||
return await asyncio.to_thread(_rescue_extract, provider.name, fetch_urls, results)
|
||||
|
||||
# Cache each successful fetch under the URL that the provider actually returned.
|
||||
# Providers may omit failed URLs or return successful results out of request order.
|
||||
# Cache each successful fetch under the REQUESTED url it reports as its own — never by list
|
||||
# position: providers omit failed URLs or return successes out of request order, and a positional
|
||||
# write filed one page's text under another URL's key for the whole TTL. ``metadata.sourceURL``
|
||||
# counts because Keenable/Firecrawl put the requested URL there when ``url`` is the redirect target.
|
||||
# An entry naming no requested URL is served but not cached (a miss re-fetches; a mis-key poisons).
|
||||
requested = set(fetch_urls)
|
||||
for fetched in results:
|
||||
url = fetched.get("url")
|
||||
meta = fetched.get("metadata")
|
||||
source = meta.get("sourceURL") if isinstance(meta, dict) else None
|
||||
url = next((u for u in (fetched.get("url"), source) if u in requested), None)
|
||||
_content = fetched.get("raw_content", "") or fetched.get("content", "")
|
||||
if url in fetch_urls and _content and not fetched.get("error"):
|
||||
if url and _content and not fetched.get("error"):
|
||||
extract_cache_put(url, _content, fetched.get("title", ""), format=format, provider=provider.name)
|
||||
return results
|
||||
|
||||
|
||||
@@ -76,7 +76,7 @@ Repeat web calls within a short window are served from cache instead of the paid
|
||||
|
||||
Concurrent identical searches (a parallel subagent fan-out firing the same query at once) are **coalesced into a single backend request** — the first caller pays; the rest share the response. Requested search limits are bucketed up to 10/20/50/100 so near-identical requests (`limit=5` vs `limit=8`) share one entry, with each caller receiving its requested count.
|
||||
|
||||
Only successful responses are cached. Failures always retry the backend, responses served by the one-shot keyless rescue are never cached (the next call attempts your chosen backend again), and URLs matched by your `security.website_blocklist` are never served from cache. Cached extracts re-run the normal truncation pipeline, so a different `char_limit` on the second call works off the same stored scrape.
|
||||
Only successful responses are cached, each under the requested URL the provider reports for it (a page the provider returns without naming a requested URL is served but not cached, so a partial or reordered batch never files one page under another URL's key). Failures always retry the backend, responses served by the one-shot keyless rescue are never cached (the next call attempts your chosen backend again), and URLs matched by your `security.website_blocklist` are never served from cache. Cached extracts re-run the normal truncation pipeline, so a different `char_limit` on the second call works off the same stored scrape.
|
||||
|
||||
**Local development URLs are never cached.** Anything on `localhost`, `127.0.0.1`, `*.local`, single-label LAN hostnames, or private/link-local IP ranges (`192.168.*`, `10.*`, `172.16-31.*`) bypasses the extract cache entirely — dev servers, hot-reload builds, and chat-GUI artifact previews change on every save, and a cached copy would show you a stale build. Every fetch of a local page is live. (These URLs are only reachable at all when `security.allow_private_urls` is enabled.)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user