crewAI-tools icon indicating copy to clipboard operation
crewAI-tools copied to clipboard

Feature/fix firecrawl search tool bugs #226 and add enhancements.

Open adamzhang1987 opened this issue 10 months ago • 3 comments

  1. Fixed "UnboundLocalError: cannot access local variable 'FirecrawlApp' where it is not associated with a value" bug when initialize FirecrawlSearchTool. Issue #226

Error message:

Traceback (most recent call last):
  File "/Users/adamzhang/PycharmProjects/ai_company/research_crew/src/research_crew/main.py", line 81, in <module>
    tool = FirecrawlSearchTool(query='what is firecrawl?')
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/adamzhang/PycharmProjects/ai_company/research_crew/.venv/lib/python3.12/site-packages/crewai_tools/tools/firecrawl_search_tool/firecrawl_search_tool.py", line 55, in __init__
    self._initialize_firecrawl()
  File "/Users/adamzhang/PycharmProjects/ai_company/research_crew/.venv/lib/python3.12/site-packages/crewai_tools/tools/firecrawl_search_tool/firecrawl_search_tool.py", line 60, in _initialize_firecrawl
    self._firecrawl = FirecrawlApp(api_key=self.api_key)
                      ^^^^^^^^^^^^
UnboundLocalError: cannot access local variable 'FirecrawlApp' where it is not associated with a value
  1. FirecrawlSearchTool Enhancements: Allow parameters to be passed during both initialization or execute the function run().
from crewai_tools import FirecrawlSearchTool

# Initialize the FirecrawlSearchTool with the arguments
tool = FirecrawlSearchTool(query='what is firecrawl?', limit=1)
print(tool.run())

# Or use the `run` method with the arguments
tool = FirecrawlSearchTool()
print(tool.run(query='what is firecrawl?', limit=1))
  1. Added more test cases for FirecrawlSearchTool.

adamzhang1987 avatar Mar 20 '25 17:03 adamzhang1987

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #246 - FirecrawlSearchTool Enhancements

Overview

This pull request introduces several important enhancements to the FirecrawlSearchTool, which include improvements in schema definition, constructor handling, error management, and test coverage. These changes greatly enhance the clarity, robustness, and usability of the code.

Detailed Findings

1. Schema Definition Changes

The new type definitions for scrapeOptions in FirecrawlSearchToolSchema improve the expressiveness of the API:

# Recommended Enhancement:
class ScrapeOptionsFormat(str, Enum):
    MARKDOWN = "markdown"
    HTML = "html"
    RAW_HTML = "rawHtml"
    # ...other formats

class ScrapeOptions(BaseModel):
    formats: Optional[List[ScrapeOptionsFormat]] = Field(default=None, description="Formats to include in the output")

Recommendation: Utilize enumerations for clarity and type safety. This not only improves readability but provides clear structure for API consumers.

2. Class Implementation

  • Constructor Handling: The constructor parameter handling can be significantly simplified with Pydantic’s model features. This reduces boilerplate code and improves maintainability:
# Recommended Change:
class FirecrawlSearchTool(BaseTool):
    def __init__(self, **data):
        super().__init__(**data)
        self._initialize_firecrawl()
  • Error Handling: The _initialize_firecrawl method should raise more specific exceptions to aid in debugging.

3. Parameter Validation

Enhancing input validation helps prevent runtime exceptions:

def _validate_params(self, params: Dict[str, Any]) -> None:
    if "limit" in params and not (1 <= params["limit"] <= 10):
        raise ValueError("Limit must be between 1 and 10")

Recommendation: Comprehensive validation will lead to a more user-friendly API. Ensure all parameters are validated on input to mitigate potential issues.

4. Test Coverage

The introduction of new tests significantly enhances reliability:

def test_no_query():
    with pytest.raises(ValueError):
        tool = FirecrawlSearchTool()
        tool.run()

Recommendation: Continue to enhance test suites by covering edge cases and possible error scenarios to further improve confidence in code reliability.

5. Documentation

The README.md documentation updates provide clarity for users. Including type information for parameters makes it easier for consumers to understand how to integrate with the tool effectively.

| **api_key**     | `string` | **Optional**. Specifies Firecrawl API key. |
| **query** | `string` | **Required**. The search query. |

Recommendation: Keep the documentation up-to-date with any refactoring or enhancements to ensure it matches the evolving codebase.

General Recommendations

  1. Logging: Implement logging for better monitorability and debugging experiences.
  2. Caching: Consider caching results to optimize repeated queries reducing load on external APIs.
  3. Rate Limiting & Retry Logic: Implement mechanisms to handle API rate limits gracefully and retries on transient failures.
  4. Type Hints: Ensure consistent use of type hints across all methods for improved code clarity.

Conclusion

The enhancements in PR #246 significantly contribute to the maintainability and robustness of the FirecrawlSearchTool. These improvements support better usability and prepare the ground for future enhancements. All modifications appear fully backward compatible, thus ensuring a smooth transition for current users of the tool.

I recommend accepting these changes and encourage the team to consider the broader recommendations to drive further improvements.

joaomdmoura avatar Mar 20 '25 17:03 joaomdmoura

@adamzhang1987 I have updated some stuffs and I till thinking this PR is valuable. Could you resolve the conflicts and address the PR comments, pls?

lucasgomide avatar Apr 28 '25 18:04 lucasgomide

@adamzhang1987 I have updated some stuffs and I till thinking this PR is valuable. Could you resolve the conflicts and address the PR comments, pls?

Hi. I've addressed all the PR comments above in new commits.

adamzhang1987 avatar May 12 '25 16:05 adamzhang1987

this pr closes it: https://github.com/crewAIInc/crewAI-tools/pull/351

lorenzejay avatar Jul 02 '25 16:07 lorenzejay