fix(deep_crawling): BFS/DFS/BestFirst strategies share one default FilterChain instance#2075
Open
chuenchen309 wants to merge 1 commit into
Open
Conversation
…lterChain instance BFSDeepCrawlStrategy.__init__ and BestFirstCrawlingStrategy.__init__ both default filter_chain to `FilterChain()` -- a classic Python mutable-default-argument bug. The default object is instantiated once at function-definition time, not per call, so every strategy instance created without an explicit filter_chain= argument shares the exact same FilterChain object. DFSDeepCrawlStrategy inherits BFS's __init__ unchanged, so it's affected too. FilterChain is genuinely stateful: FilterChain.apply() increments self.stats._counters on every call, and add_filter() mutates self.filters in place. Two unrelated strategy instances (e.g. two concurrent crawls, or sequential crawls reusing default construction) silently share and corrupt each other's filter stats, and adding a filter to one instance's default chain leaks into every other instance using the default. Confirmed via `s1.filter_chain is s2.filter_chain` == True for two independently-constructed BFSDeepCrawlStrategy instances before this fix. Fix: filter_chain: Optional[FilterChain] = None, with `self.filter_chain = filter_chain if filter_chain is not None else FilterChain()` inside __init__, giving each instance a fresh chain when none is explicitly provided. Added tests/deep_crawling/test_default_filter_chain_isolation.py covering BFS, DFS, and BestFirst independently, plus a test confirming add_filter() on one instance's default chain doesn't leak into another's. Full tests/deep_crawling/ suite: 73 passed. py_compile clean on changed files (no lint/format tooling configured in this repo). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BFSDeepCrawlStrategy.__init__andBestFirstCrawlingStrategy.__init__both defaultfilter_chaintoFilterChain()— a classic Python mutable-default-argument bug. The default object is instantiated once at function-definition time, not per call, so every strategy instance created without an explicitfilter_chain=argument shares the exact sameFilterChainobject.DFSDeepCrawlStrategyinherits BFS's__init__unchanged, so it's affected too.FilterChainis genuinely stateful:FilterChain.apply()incrementsself.stats._counterson every call, andadd_filter()mutatesself.filtersin place. Two unrelated strategy instances (e.g. two concurrent crawls, or sequential crawls reusing default construction) silently share and corrupt each other's filter stats, and adding a filter to one instance's default chain leaks into every other instance using the default.Confirmed via
s1.filter_chain is s2.filter_chain==Truefor two independently-constructedBFSDeepCrawlStrategyinstances before this fix.Fix
filter_chain: Optional[FilterChain] = None, withself.filter_chain = filter_chain if filter_chain is not None else FilterChain()inside__init__, giving each instance a fresh chain when none is explicitly provided.List of files changed and why
crawl4ai/deep_crawling/bfs_strategy.py— fix the mutable default (covers BFS and, via inheritance, DFS).crawl4ai/deep_crawling/bff_strategy.py— same fix forBestFirstCrawlingStrategy.tests/deep_crawling/test_default_filter_chain_isolation.py(new) — regression coverage.How Has This Been Tested?
filter_chain is notbetween two instances), plus a test confirmingadd_filter()on one instance's default chain doesn't leak into another's.s1.filter_chain is s2.filter_chain == Trueand the leaked-filter assertion failing; reapplying passes.tests/deep_crawling/suite: 73 passed.pyproject.toml, noMakefile/.pre-commit-config.yaml); confirmed changed files compile cleanly withpython -m py_compile.dispatcher=passthrough parameter, not thefilter_chaindefault — no overlap.Checklist:
AI-Generated disclosure
Found via an AI-assisted code review pass (Claude Code) over
crawl4ai/deep_crawling/. I personally confirmed the shared-instance bug with a standalone repro, verified the fix across all three affected strategy classes, and ran the relevant test suite before submitting.