-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix: core api including config and result handling #970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
stevenh
wants to merge
4
commits into
unclecode:main
Choose a base branch
from
stevenh:fix/config-result-handling
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
Fix the handling of file downloads in AsyncPlaywrightCrawlerStrategy which wasn't waiting for the download to complete before returning, which resulted in race conditions and incomplete or missing downloads. Fix incorrect field use for headers and user_agent in AsyncPlaywrightCrawlerStrategy. Fix the type used for the logger in AsyncPlaywrightCrawlerStrategy to the base class Logger. This allows the logger to be used correctly and prevents type errors when using the logger. Add missing abstract methods to AsyncCrawlerStrategy and implement in its subclasses. Fix the extraction of the error message when processing console error log in log_console. Fix the handling of js_code in AsyncPlaywrightCrawlerStrategy to ensure that the code is executed correctly and the result is returned when the script starts with a variable or constant declaration. Remove unused time variables and commented out code to improve readability. Remove duplicate Exception handling. Fix the handling of raw requests in AsyncHTTPCrawlerStrategy which was previously using the parsed path corrupting the request. Fix the browser manager to ensure that it always returns a valid browser instance. This eliminates the class level playwright instance which wasn't being cleaned up on close, resulting in subsequent callers receiving a closed instance and causing errors. Fix the storage of links, media and metadata to ensure that the correct values are stored and returned. This prevents incorrect results when using the cached results. Use Field for default values in Media, Links and ScrapingResult pydantic models to prevent invalid results. Handle undefined string passed to MediaItem width. Add parameters to the CrawlResult.__init__ method to provide type hints for the fields, which provides better type checking. Fix database query retry handling for the case where the database table is missing when using the SQLite database. This prevents the crawler from failing when the table is not present and allows it to continue crawling. This prevents failing tests after one which drops the database table. Remove commit call for query's which don't need it. Sync table definition in legacy code so prevent mixed legacy and new code tests from conflicting with each other. Fix the caching of markdown field in DB / files which was only storing the single value, which caused failures when using cached results. Export the markdown field in StringCompatibleMarkdown, so we don't need to use a private field to ensure that the value is serialised correctly. Correctly initialise BrowserConfig and CrawlerRunConfig from kwargs to ensure legacy parameters work as expected.
Add relevant tests for the following: - markdown generator - http crawler strategy
Add ruff settings to pyproject.toml to prevent deprecation warnings beyond our control and to disable formatting for now to avoid the noise it would generate.
6 tasks
cab4426
to
4ded3d5
Compare
Fix a large number of issues with the core including config serialisation and crawl result handling, which were all identified during work to ensure that all the existing tests pass consistently. This includes: ------- fix: config serialisation Fix config serialisation by creating a new Serialisable type and adding missing module imports for ScoringStats and Logger. This allows the config to be serialised and deserialised correctly. Add missing initialisation for ScoringStats. Add missing stats parameter to URLScorer and all its subclasses to ensure that the stats are serialisable. ------- fix: parameter definitions, type hints and defaults Fix parameter definitions by adding missing Optional hit to those which default to None. Add type hints to improve linting validation and IDE support. Set default values for parameters which were missing them, ensuring type safety and preventing runtime errors. ------- fix: various comment typos Fix various typos in comments and doc strings to improve clarity. ------- fix: BaseDispatcher missing abstract methods Add missing abstract methods to BaseDispatcher and implement in subclasses. ------- fix: crawl result handling Fix the handling of crawl results, which were using inconsistent types. This now uses CrawlResultContainer for all crawl results, unwrapping as needed when performing deep crawls. This moves CrawlResultContainer into models ensuring it can be imported where needed, avoiding circular imports. Refactor CrawlResultContainer to subclass CrawlResult to provide type hinting in the single result case and ensure consistent handling of both synchronous and asynchronous results. ------- feat: implement run_urls_stream for SemaphoreDispatcher Implement run_urls_stream for SemaphoreDispatcher to allow streaming of URLs to be crawled. ------- chore: translate non english comments Translate non english comments to english to improve readability and maintainability for non native speakers. ------- chore: additional example for arun Add examples for arun to demonstrate usage of streamed and batch processing modes, clarifying the impact of deep crawl on the results. ------- fix: handling of CrawlerRunConfig Fix the handling of CrawlerRunConfig to ensure that the config is correctly initialised from legacy kwargs. ------- fix: invalid screenshot argument to aprocess_html Fix failure caused by generating a screenshot due to unsupported argument to aprocess_html. ------- fix: structured content extraction Fix structured content extraction not being run when NoExtractionStrategy is used. This ensures that the content is extracted correctly and returned in the crawl result. ------- fix: aclear_cache Fix aclear_cache, previously it was just clean up, which just closed active connections. It now calls aclear_db to remove all entries from the cache table. ------- fix: unused imports Remove unused imports to improve readability and reduce clutter. ------- fix: undefined filter_conf Fix use of undefined filter_conf in crawl_cmd when the config is not passed in and neither markdown-fix nor md-fit are specified as output options. ------- fix: bs4 imports Fix bs4 imports to use the correct module name and ensure that the correct classes are imported. This addresses linter errors. ------- fix: BM25Okapi idf calculation Fix the idf calculation in BM25Okapi to use the correct formula and ensure that the idf is calculated correctly. This prevents missing results when using BM25Okapi caused by zero idf values. Removed commented out code to improve readability. Eliminate unnecessary tag_weight calculation when score is 0. ------- fix: relevant content filter Fix the extraction of headers from the response, previously this only handled h1 tags, this now handles all header tags. Fix boundary check for the relevant content filter to ensure that the content is not excluded when the end aligns with 150 character limit. ------- fix: return type for extract_text_chunks Fix the return type for extract_text_chunks to be include the right types and values, so consumers know what to expect. ------- fix: invalid markdown parameter to ScrapingResult Remove all references to markdown for ScrapingResult as the source never returns markdown, so its pointless to include it in the result. ------- fix: closest parent description Clean unnecessary white space the description returned by find_closest_parent_with_useful_text so that the two different strategies return consistent results. ------- fix: potential sources for element extraction Fix the potential sources for element extraction to ensure that the srcset and data-lazy-src are processed instead of srcssetdata-lazy-src due to missing comma. ------- fix: data validation in web scraping Add missing data validation, ensuring that the correct types and only set values are processed. ------- fix: missing json import for AmazonProductCrawler Add missing json import for AmazonProductCrawler. ------- fix: GoogleSearchCrawler checks and errors Correct the result field used to report the error message when arun fails. This ensures that the error message is correctly reported. Add missing check for result.js_execution_result before use. Add missing check for blank cleaned_html. Add validation of cleaned_html to ensure that the value is correctly converted from bytes to str. ------- fix: abstract method definitions Fix the abstract method definitions to ensure that the correct types are used for async generators and iterators. This lint errors for incompatible overrides by their implementers. This is cause be the type being adjusted when a yield is present. ------- fix: invalid use of infinity for int values Fix the use of infinity for int values flagged by the linter. Instead we use -1 to indicate that the value is not set. Fix use of unset url parameter, reported by linter. ------- chore: remove unneeded loop on batch results Remove unnecessary duplicate loop on batch results in _arun_batch to calculate _pages_crawled. ------- fix: invalid use of lambda to define async function Replace the use of lambda to define an async function with a normal function definition. ------- fix: validate use of deep_crawl_strategy before use Validate the type of deep_crawl_strategy before use to ensure that the correct type is used and preventing lint errors on methods calls using it. ------- fix: unprocessed FilterChain tasks Ensure that all tasks in the FilterChain are either processed and returned or cancelled if not needed, preventing runtime warnings. ------- feat: add support for transport to Crawl4aiDockerClient Add the ability to provide a custom transport to Crawl4aiDockerClient which allows easy testing. Set base_url on the httpx.AsyncClient to avoid need for local variable and calculations, simplifying the code. ------- fix: Crawl4aiDockerClient.crawl results on error Correctly handle the async data returned by the crawl method in Crawl4aiDockerClient when the stream results in a non 200 response. ------- fix: linter errors for lxml imports Fix linter errors for lxml imports by using importing the required methods directly. ------- fix: use of unset llm_config Fix the use of unset llm_config in generate_schema to ensure that we don't get a runtime error. ------- fix: missing meta field for hub crawlers Support both ways that meta data is defined by hub crawlers. ------- fix: correct return type for get_cached_url Correct the type and number of returned values for get_cached_url. ------- fix: DocsManager generation Fix the generation performed by DocsManager now docs have a hierarchical structure. ------- fix: linting errors in calculate_semaphore_count Ensure we set a default for the resource values returned by os methods. ------- fix: port handling in get_base_domain Maintain the port for non default ports in get_base_domain to ensure that the correct domain is returned. This prevents local links being incorrectly classified as external. ------- fix: get_title method type Add static method decorator to get_title method to ensure that the method is correctly identified as a static method and not an instance method. This prevents lint errors and ensures that the method is correctly called. ------- chore: add project settings to improve developer experience Add details to pyproject.toml to improve the developer experience including: * Configuring pytest test timeouts, ignoring external warnings and asyncio scope. * Disabling ruff formatting * Creating developer package targets: dev, docker and test * Leverage chaining to simplify maintenance of the all group ------- fix: WebScrapingStrategy ascrap Fix the ascrap command by calling scrap instead of _scrap which misses a lot of the functionality. ------- fix: scraping local sites Fix the scraping of local sites by removing the check for dot in parsed network location. ------- fix: monitor support on pseudo terminals Fix monitor support on pseudo terminals by setting the default for enable_ui to the value of sys.stdin.isatty(), this ensures it works under pytest if not specifically set. ------- fix: imports Add missing and remove unused imports as well as eliminating the use of wildcard imports. ------- fix: dependencies Add missing optional dependencies. ------- chore: remove deprecated licence tags Update setup.py and pyproject.toml to remove deprecated license tags, bumping the dependency to support the new method. The eliminates toml lint warning in pyproject.toml.
4ded3d5
to
e09474f
Compare
This was referenced Apr 11, 2025
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.
Notice
Merge #969 first, as this is based on it.
Summary
Fix a large number of issues with the core including config serialisation and crawl result handling identified during work to ensure that all the existing tests pass consistently.
Details
fix: config serialisation
Fix config serialisation by creating a new
Serialisable
type and adding missing module imports for ScoringStats and Logger.This allows the config to be serialised and deserialised correctly.
Add missing initialisation for
ScoringStats
.Add missing stats parameter to
URLScorer
and all its subclasses to ensure that the stats are serialisable.fix: parameter definitions, type hints and defaults
Fix parameter definitions by adding missing Optional hit to those which default to None.
Add type hints to improve linting validation and IDE support.
Set default values for parameters which were missing them, ensuring type safety and preventing runtime errors.
fix: various comment typos
Fix various typos in comments and doc strings to improve clarity.
fix: BaseDispatcher missing abstract methods
Add missing abstract methods to
BaseDispatcher
and implement in subclasses.fix: crawl result handling
Fix the handling of crawl results, which were using inconsistent types. This now uses
CrawlResultContainer
for all crawl results, unwrapping as needed when performing deep crawls.This moves
CrawlResultContainer
into models ensuring it can be imported where needed, avoiding circular imports.Refactor
CrawlResultContainer
to subclassCrawlResult
to provide type hinting in the single result case and ensure consistent handling of both synchronous and asynchronous results.feat: implement run_urls_stream for SemaphoreDispatcher
Implement
run_urls_stream
forSemaphoreDispatcher
to allow streaming of URLs to be crawled.chore: translate non english comments
Translate non english comments to english to improve readability and maintainability for non native speakers.
chore: additional example for arun
Add examples for arun to demonstrate usage of streamed and batch processing modes, clarifying the impact of deep crawl on the results.
fix: handling of CrawlerRunConfig
Fix the handling of CrawlerRunConfig to ensure that the config is correctly initialised from legacy kwargs.
fix: invalid screenshot argument to aprocess_html
Fix failure caused by generating a screenshot due to unsupported argument to
aprocess_html
.fix: structured content extraction
Fix structured content extraction not being run when
NoExtractionStrategy
is used. This ensures that the content is extracted correctly and returned in the crawl result.fix: aclear_cache
Fix
aclear_cache
, previously it was just clean up, which just closed active connections. It now callsaclear_db
to remove all entries from the cache table.fix: unused imports
Remove unused imports to improve readability and reduce clutter.
fix: undefined filter_conf
Fix use of undefined
filter_conf
incrawl_cmd
when the config is not passed in and neither markdown-fix nor md-fit are specified as output options.fix: bs4 imports
Fix bs4 imports to use the correct module name and ensure that the correct classes are imported. This addresses linter errors.
fix: BM25Okapi idf calculation
Fix the idf calculation in
BM25Okapi
to use the correct formula and ensure that the idf is calculated correctly. This prevents missing results when usingBM25Okapi
caused by zero idf values.Removed commented out code to improve readability.
Eliminate unnecessary
tag_weight
calculation when score is 0.fix: relevant content filter
Fix the extraction of headers from the response, previously this only handled h1 tags, this now handles all header tags.
Fix boundary check for the relevant content filter to ensure that the content is not excluded when the end aligns with 150 character limit.
fix: return type for extract_text_chunks
Fix the return type for
extract_text_chunks
to be include the right types and values, so consumers know what to expect.fix: invalid markdown parameter to ScrapingResult
Remove all references to markdown for
ScrapingResult
as the source never returns markdown, so its pointless to include it in the result.fix: closest parent description
Clean unnecessary white space the description returned by
find_closest_parent_with_useful_text
so that the two different strategies return consistent results.fix: potential sources for element extraction
Fix the potential sources for element extraction to ensure that the
srcset
anddata-lazy-src
are processed instead ofsrcssetdata-lazy-src
due to missing comma.fix: data validation in web scraping
Add missing data validation, ensuring that the correct types and only set values are processed.
fix: missing json import for AmazonProductCrawler
Add missing json import for
AmazonProductCrawler
.fix: GoogleSearchCrawler checks and errors
Correct the result field used to report the error message when arun fails. This ensures that the error message is correctly reported.
Add missing check for
result.js_execution_result
before use.Add missing check for blank
cleaned_html
.Add validation of
cleaned_html
to ensure that the value is correctly converted from bytes to str.fix: abstract method definitions
Fix the abstract method definitions to ensure that the correct types are used for async generators and iterators. This lint errors for incompatible overrides by their implementers. This is cause be the type being adjusted when a yield is present.
fix: invalid use of infinity for int values
Fix the use of infinity for int values flagged by the linter. Instead we use -1 to indicate that the value is not set.
Fix use of unset url parameter, reported by linter.
chore: remove unneeded loop on batch results
Remove unnecessary duplicate loop on batch results in
_arun_batch
to calculate_pages_crawled
.fix: invalid use of lambda to define async function
Replace the use of
lambda
to define an async function with a normal function definition.fix: validate use of deep_crawl_strategy before use
Validate the type of
deep_crawl_strategy
before use to ensure that the correct type is used and preventing lint errors on methods calls using it.fix: unprocessed FilterChain tasks
Ensure that all tasks in the
FilterChain
are either processed and returned or cancelled if not needed, preventing runtime warnings.feat: add support for transport to Crawl4aiDockerClient
Add the ability to provide a custom transport to
Crawl4aiDockerClient
which allows easy testing.Set
base_url
on thehttpx.AsyncClient
to avoid need for local variable and calculations, simplifying the code.fix: Crawl4aiDockerClient.crawl results on error
Correctly handle the async data returned by the crawl method in
Crawl4aiDockerClient
when the stream results in a non 200 response.fix: linter errors for lxml imports
Fix linter errors for lxml imports by using importing the required methods directly.
fix: use of unset llm_config
Fix the use of unset
llm_config
ingenerate_schema
to ensure that we don't get a runtime error.fix: missing meta field for hub crawlers
Support both ways that meta data is defined by hub crawlers.
fix: correct return type for get_cached_url
Correct the type and number of returned values for
get_cached_url
.fix: DocsManager generation
Fix the generation performed by
DocsManager
now docs have a hierarchical structure.fix: linting errors in calculate_semaphore_count
Ensure we set a default for the resource values returned by os methods.
fix: port handling in get_base_domain
Maintain the port for non default ports in
get_base_domain
to ensure that the correct domain is returned. This prevents local links being incorrectly classified as external.fix: get_title method type
Add static method decorator to get_title method to ensure that the method is correctly identified as a static method and not an instance method. This prevents lint errors and ensures that the method is correctly called.
chore: add project settings to improve developer experience
Add details to
pyproject.toml
to improve the developer experience including:fix: WebScrapingStrategy ascrap
Fix the ascrap command by calling
scrap
instead of_scrap
which misses a lot of the functionality.fix: scraping local sites
Fix the scraping of local sites by removing the check for dot in parsed network location.
fix: monitor support on pseudo terminals
Fix monitor support on pseudo terminals by setting the default for enable_ui to the value of
sys.stdin.isatty()
, this ensures it works under pytest if not specifically set.fix: imports
Add missing and remove unused imports as well as eliminating the use of wildcard imports.
fix: dependencies
Add missing optional dependencies.
chore: remove deprecated licence tags
Update setup.py and pyproject.toml to remove deprecated license tags, bumping the dependency to support the new method. The eliminates toml lint warning in pyproject.toml.
fix: delay on browser close
Wait for spawned browser process, preventing it from becoming defunct and delaying the clean up until max retries have been reached.
fix: invalid use of firefox via cdp
Remove firefox from the list of supported internal browsers as it doesn't support CDP via playwright.
feat: add support for ephemeral debug port
Add support for using an ephemeral debug port for the browser, eliminating the need to set a specific port and preventing port conflicts.
core: add TODOs for issues noticed
Add TODOs for issues noticed during implementation of these fixes but not needed to address test failures of the code.
How Has This Been Tested?
This has been tested against the full tests/* which will be added in a follow up PR as discussed on #891.
Checklist