fix: bound web search result content - #14470
Open
Oxygen56 wants to merge 6 commits into
Open
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe Web Search component adds advanced ChangesWeb search limits
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant WebSearchComponent
participant ParsedResults
participant ResultPages
WebSearchComponent->>ParsedResults: select up to max_results
WebSearchComponent->>ResultPages: fetch selected pages
ResultPages-->>WebSearchComponent: return scraped content
WebSearchComponent-->>WebSearchComponent: truncate content to max_content_length
🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/tests/unit/components/data_source/test_web_search.py`:
- Around line 201-239: Add test coverage around
test_perform_web_search_limits_and_content for zero and negative max_results and
max_content_length values. Verify max_results=0 and -1 fetch no result pages,
and max_content_length=0 and -1 return empty content, while preserving the
existing positive-limit assertions and mocks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 29d3bcf8-f8dc-4e18-aba9-4db17d57f5f3
📒 Files selected for processing (2)
src/backend/tests/unit/components/data_source/test_web_search.pysrc/lfx/src/lfx/components/data_source/web_search.py
Comment on lines
+201
to
+239
| @patch.object(WebSearchComponent, "_safe_get_url") | ||
| @patch("lfx.components.data_source.web_search.requests.get") | ||
| def test_perform_web_search_limits_results_and_content(self, mock_get, mock_safe_get): | ||
| """Web search should bound both fetched result count and returned page text.""" | ||
| component = WebSearchComponent() | ||
| component.query = "test query" | ||
| component.max_results = 2 | ||
| component.max_content_length = 12 | ||
| component.timeout = 5 | ||
|
|
||
| mock_response = Mock() | ||
| mock_response.text = """ | ||
| <html> | ||
| <div class="result"> | ||
| <a class="result__a" href="?uddg=https%3A%2F%2Fexample.com%2F1">First</a> | ||
| </div> | ||
| <div class="result"> | ||
| <a class="result__a" href="?uddg=https%3A%2F%2Fexample.com%2F2">Second</a> | ||
| </div> | ||
| <div class="result"> | ||
| <a class="result__a" href="?uddg=https%3A%2F%2Fexample.com%2F3">Third</a> | ||
| </div> | ||
| </html> | ||
| """ | ||
| mock_response.headers = {"content-type": "text/html"} | ||
| mock_response.raise_for_status.return_value = None | ||
| mock_get.return_value = mock_response | ||
|
|
||
| mock_page_response = Mock() | ||
| mock_page_response.text = "<html><body>abcdefghijklmnopqrstuvwxyz</body></html>" | ||
| mock_page_response.raise_for_status.return_value = None | ||
| mock_safe_get.return_value = mock_page_response | ||
|
|
||
| with patch("lfx.components.data_source.web_search.get_user_agent", return_value="test-agent"): | ||
| result = component.perform_web_search() | ||
|
|
||
| assert len(result) == 2 | ||
| assert mock_safe_get.call_count == 2 | ||
| assert result["content"].tolist() == ["abcdefghijkl", "abcdefghijkl"] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test zero and negative limits.
The implementation clamps both limits to zero. This test only covers positive values. Add cases for max_results=0, max_results=-1, max_content_length=0, and max_content_length=-1. Assert that a zero result limit fetches no result pages and that a zero content limit returns empty content.
As per coding guidelines, backend tests must cover positive, negative, edge, and error cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/tests/unit/components/data_source/test_web_search.py` around
lines 201 - 239, Add test coverage around
test_perform_web_search_limits_and_content for zero and negative max_results and
max_content_length values. Verify max_results=0 and -1 fetch no result pages,
and max_content_length=0 and -1 return empty content, while preserving the
existing positive-limit assertions and mocks.
Source: Coding guidelines
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
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
1 participant
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.
Fixes #14469
Summary
Root Cause
Web mode iterated over every DuckDuckGo result and returned the full extracted text of each linked page. Repeated tool calls therefore accumulated unbounded page content in the agent context.
Verification
Notes for Reviewers
Summary by CodeRabbit
New Features
Bug Fixes