fix: YoutubeChannelSearchTool passes bare handle instead of full URL (#5429)#5829
Open
NIK-TIGER-BILL wants to merge 3 commits into
Open
fix: YoutubeChannelSearchTool passes bare handle instead of full URL (#5429)#5829NIK-TIGER-BILL wants to merge 3 commits into
NIK-TIGER-BILL wants to merge 3 commits into
Conversation
added 3 commits
May 7, 2026 23:19
OpenRouter reasoning models (e.g., Anthropic Sonnet 4.5, Gemini 3.1 Pro) return a reasoning_content field when the model produces chain-of-thought output. CrewAI previously only read message.content, which could be None for these responses, resulting in an empty string being returned. This change adds a fallback to getattr(message, 'reasoning_content') in both sync and async non-streaming and streaming chat completion paths, so that reasoning output is correctly surfaced when content is absent. Fixes crewAIInc#5537 Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
…content Replaces getattr(message, 'reasoning_content', '') and getattr(chunk_delta, 'reasoning_content', '') with try/except AttributeError blocks as requested by reviewer. Also ran ruff check/format. Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
When YoutubeChannelSearchTool.add() received a bare channel handle (e.g. '@krishnaik06'), it forwarded the handle unchanged to the RagTool adapter. The downstream YoutubeChannelLoader then failed with 'Invalid YouTube channel URL' because it expects a full https://www.youtube.com/... URL. Convert the handle to a full YouTube URL before passing it to the adapter, matching the pattern already used by YoutubeVideoLoader. Add a unit test that asserts the adapter receives a proper URL. Fixes: crewAIInc#5429 Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.com>
📝 WalkthroughWalkthroughThis PR addresses two separate features: YouTube channel search tool now normalizes channel handles into full URLs before passing them to downstream loaders, and OpenAI completion handler incorporates reasoning content as fallback text when message content is absent, applied consistently across all sync/async and streaming/non-streaming completion paths. ChangesYouTube Channel Search Tool URL Normalization
OpenAI Reasoning Content Fallback Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai-tools/tests/tools/test_youtube_channel_search_tool.py (1)
17-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd regression coverage for already-fully-qualified channel URLs.
The suite validates handle normalization, but it misses the full-URL input path from Issue
#5429. Add a test assertingtool.add("https://www.youtube.com/@krishnaik06")is forwarded unchanged.🤖 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 `@lib/crewai-tools/tests/tools/test_youtube_channel_search_tool.py` around lines 17 - 45, Add a regression test that verifies YoutubeChannelSearchTool.add forwards already-fully-qualified channel URLs unchanged: in the test file add a new test function (similar to test_add_converts_handle_to_full_url) that creates a YoutubeChannelSearchTool, assigns a _DummyAdapter to tool.adapter, calls tool.add("https://www.youtube.com/@krishnaik06") and asserts the adapter received that exact URL (inspect dummy.calls[0] like the other tests). Ensure the assertion checks pos_args[0] == "https://www.youtube.com/@krishnaik06" so the add method does not alter full URLs.
🧹 Nitpick comments (1)
lib/crewai/tests/llms/openai/test_openai.py (1)
30-64: ⚡ Quick winAdd async/streaming parity tests for reasoning fallback.
Nice baseline test, but the changed logic also touches async and streaming paths. Please add tests for: (1) reasoning-only stream fallback, and (2) mixed reasoning+content stream ensuring final output stays content-first.
🤖 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 `@lib/crewai/tests/llms/openai/test_openai.py` around lines 30 - 64, Add two new tests alongside test_openai_completion_reasoning_content_fallback to cover async/streaming parity: (1) test_openai_completion_reasoning_stream_fallback should mock the streaming path (patch _get_sync_client or _get_async_client depending on sync/async implementation) so the chat.completions.create stream yields chunks whose messages only contain reasoning_content and no content, then call the LLM in streaming mode and assert the final returned string equals the reasoning_content; (2) test_openai_completion_reasoning_mixed_stream_content_first should mock a stream that first yields chunks with reasoning_content only and later yields a chunk with content, then call the LLM in streaming mode and assert the final returned string is the content (content-first wins); use the same helper types from the file (OpenAICompletion, ChatCompletionMessage, ChatCompletion) and patch the client methods used in the production code (_get_sync_client/_get_async_client and chat.completions.create) to simulate the chunked responses.
🤖 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
`@lib/crewai-tools/src/crewai_tools/tools/youtube_channel_search_tool/youtube_channel_search_tool.py`:
- Around line 45-50: The code currently always prepends "@" to
youtube_channel_handle then builds channel_url, which corrupts full URLs; update
the logic in the method that sets youtube_channel_handle (the variable used
before calling super().add) to first detect if the input is already a full URL
(e.g., starts with "http://" or "https://" or common YouTube hosts like
"www.youtube.com" / "youtube.com" / "youtu.be") and, if so, use it as
channel_url directly; otherwise normalize bare handles by ensuring they start
with "@" and then build channel_url =
f"https://www.youtube.com/{youtube_channel_handle}" before calling
super().add(..., data_type=DataType.YOUTUBE_CHANNEL).
In `@lib/crewai/src/crewai/llms/providers/openai/completion.py`:
- Around line 1926-1934: The streaming fallback mixes reasoning and final
content because the block using chunk_delta.reasoning_content and
chunk_delta.content appends reasoning into full_response and emits it
immediately; later the actual content may be appended again causing reasoning to
leak. Fix by buffering reasoning separately (e.g., local var reasoning_buffer)
when processing chunks in the streaming path and only append or emit that
buffered reasoning if no content was emitted by the end of the stream; update
the logic around full_response, chunk_delta.content,
chunk_delta.reasoning_content and the _emit_stream_chunk_event calls so emitted
chunks prefer content and only fall back to the buffered reasoning when final
content was never produced.
---
Outside diff comments:
In `@lib/crewai-tools/tests/tools/test_youtube_channel_search_tool.py`:
- Around line 17-45: Add a regression test that verifies
YoutubeChannelSearchTool.add forwards already-fully-qualified channel URLs
unchanged: in the test file add a new test function (similar to
test_add_converts_handle_to_full_url) that creates a YoutubeChannelSearchTool,
assigns a _DummyAdapter to tool.adapter, calls
tool.add("https://www.youtube.com/@krishnaik06") and asserts the adapter
received that exact URL (inspect dummy.calls[0] like the other tests). Ensure
the assertion checks pos_args[0] == "https://www.youtube.com/@krishnaik06" so
the add method does not alter full URLs.
---
Nitpick comments:
In `@lib/crewai/tests/llms/openai/test_openai.py`:
- Around line 30-64: Add two new tests alongside
test_openai_completion_reasoning_content_fallback to cover async/streaming
parity: (1) test_openai_completion_reasoning_stream_fallback should mock the
streaming path (patch _get_sync_client or _get_async_client depending on
sync/async implementation) so the chat.completions.create stream yields chunks
whose messages only contain reasoning_content and no content, then call the LLM
in streaming mode and assert the final returned string equals the
reasoning_content; (2)
test_openai_completion_reasoning_mixed_stream_content_first should mock a stream
that first yields chunks with reasoning_content only and later yields a chunk
with content, then call the LLM in streaming mode and assert the final returned
string is the content (content-first wins); use the same helper types from the
file (OpenAICompletion, ChatCompletionMessage, ChatCompletion) and patch the
client methods used in the production code (_get_sync_client/_get_async_client
and chat.completions.create) to simulate the chunked responses.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d4bf4b14-f4d2-4177-b27a-4c04bc7fdb9f
📒 Files selected for processing (4)
lib/crewai-tools/src/crewai_tools/tools/youtube_channel_search_tool/youtube_channel_search_tool.pylib/crewai-tools/tests/tools/test_youtube_channel_search_tool.pylib/crewai/src/crewai/llms/providers/openai/completion.pylib/crewai/tests/llms/openai/test_openai.py
Comment on lines
45
to
+50
| if not youtube_channel_handle.startswith("@"): | ||
| youtube_channel_handle = f"@{youtube_channel_handle}" | ||
| super().add(youtube_channel_handle, data_type=DataType.YOUTUBE_CHANNEL) | ||
| # Convert handle to a full YouTube URL so downstream loaders receive | ||
| # a valid channel URL instead of a bare handle. | ||
| channel_url = f"https://www.youtube.com/{youtube_channel_handle}" | ||
| super().add(channel_url, data_type=DataType.YOUTUBE_CHANNEL) |
There was a problem hiding this comment.
Handle full YouTube URLs before @ normalization.
Line 45 currently assumes non-@ input is always a handle. A fully-qualified URL gets corrupted and becomes invalid.
Proposed fix
def add( # type: ignore[override]
self,
youtube_channel_handle: str,
) -> None:
- if not youtube_channel_handle.startswith("@"):
- youtube_channel_handle = f"@{youtube_channel_handle}"
- # Convert handle to a full YouTube URL so downstream loaders receive
- # a valid channel URL instead of a bare handle.
- channel_url = f"https://www.youtube.com/{youtube_channel_handle}"
+ # Accept fully-qualified URLs as-is; only normalize bare handles.
+ if youtube_channel_handle.startswith(("http://", "https://")):
+ channel_url = youtube_channel_handle
+ else:
+ if not youtube_channel_handle.startswith("@"):
+ youtube_channel_handle = f"@{youtube_channel_handle}"
+ # Convert handle to a full YouTube URL so downstream loaders receive
+ # a valid channel URL instead of a bare handle.
+ channel_url = f"https://www.youtube.com/{youtube_channel_handle}"
super().add(channel_url, data_type=DataType.YOUTUBE_CHANNEL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if not youtube_channel_handle.startswith("@"): | |
| youtube_channel_handle = f"@{youtube_channel_handle}" | |
| super().add(youtube_channel_handle, data_type=DataType.YOUTUBE_CHANNEL) | |
| # Convert handle to a full YouTube URL so downstream loaders receive | |
| # a valid channel URL instead of a bare handle. | |
| channel_url = f"https://www.youtube.com/{youtube_channel_handle}" | |
| super().add(channel_url, data_type=DataType.YOUTUBE_CHANNEL) | |
| # Accept fully-qualified URLs as-is; only normalize bare handles. | |
| if youtube_channel_handle.startswith(("http://", "https://")): | |
| channel_url = youtube_channel_handle | |
| else: | |
| if not youtube_channel_handle.startswith("@"): | |
| youtube_channel_handle = f"@{youtube_channel_handle}" | |
| # Convert handle to a full YouTube URL so downstream loaders receive | |
| # a valid channel URL instead of a bare handle. | |
| channel_url = f"https://www.youtube.com/{youtube_channel_handle}" | |
| super().add(channel_url, data_type=DataType.YOUTUBE_CHANNEL) |
🤖 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
`@lib/crewai-tools/src/crewai_tools/tools/youtube_channel_search_tool/youtube_channel_search_tool.py`
around lines 45 - 50, The code currently always prepends "@" to
youtube_channel_handle then builds channel_url, which corrupts full URLs; update
the logic in the method that sets youtube_channel_handle (the variable used
before calling super().add) to first detect if the input is already a full URL
(e.g., starts with "http://" or "https://" or common YouTube hosts like
"www.youtube.com" / "youtube.com" / "youtu.be") and, if so, use it as
channel_url directly; otherwise normalize bare handles by ensuring they start
with "@" and then build channel_url =
f"https://www.youtube.com/{youtube_channel_handle}" before calling
super().add(..., data_type=DataType.YOUTUBE_CHANNEL).
Comment on lines
+1926
to
1934
| try: | ||
| reasoning = chunk_delta.reasoning_content or "" | ||
| except AttributeError: | ||
| reasoning = "" | ||
| if chunk_delta.content or reasoning: | ||
| full_response += chunk_delta.content or reasoning | ||
| self._emit_stream_chunk_event( | ||
| chunk=chunk_delta.content, | ||
| chunk=chunk_delta.content or reasoning or "", | ||
| from_task=from_task, |
There was a problem hiding this comment.
Streaming fallback currently mixes reasoning and final content.
When reasoning_content arrives before content, Line 1931 and Line 2242 append reasoning into full_response, then later append actual content too. Non-streaming (Line 1688/Line 2089) returns content-first, so streaming becomes inconsistent and may leak reasoning text unexpectedly. Buffer reasoning separately and only use it if no content was emitted by stream end.
💡 Suggested direction
- full_response = ""
+ full_response = ""
+ reasoning_fallback = ""
+ saw_content = False
- if chunk_delta.content or reasoning:
- full_response += chunk_delta.content or reasoning
- self._emit_stream_chunk_event(chunk=chunk_delta.content or reasoning or "", ...)
+ if chunk_delta.content:
+ saw_content = True
+ full_response += chunk_delta.content
+ self._emit_stream_chunk_event(chunk=chunk_delta.content, ...)
+ elif reasoning:
+ reasoning_fallback += reasoning
# before finalize:
+ if not saw_content and reasoning_fallback:
+ full_response = reasoning_fallback
+ self._emit_stream_chunk_event(chunk=reasoning_fallback, ...)Also applies to: 2237-2245
🤖 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 `@lib/crewai/src/crewai/llms/providers/openai/completion.py` around lines 1926
- 1934, The streaming fallback mixes reasoning and final content because the
block using chunk_delta.reasoning_content and chunk_delta.content appends
reasoning into full_response and emits it immediately; later the actual content
may be appended again causing reasoning to leak. Fix by buffering reasoning
separately (e.g., local var reasoning_buffer) when processing chunks in the
streaming path and only append or emit that buffered reasoning if no content was
emitted by the end of the stream; update the logic around full_response,
chunk_delta.content, chunk_delta.reasoning_content and the
_emit_stream_chunk_event calls so emitted chunks prefer content and only fall
back to the buffered reasoning when final content was never produced.
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.
Purpose
YoutubeChannelSearchTool.add()forwarded a bare channel handle (e.g.@krishnaik06) unchanged to theRagTooladapter. The downstreamYoutubeChannelLoaderthen raisedValueError: Invalid YouTube channel URLbecause it expects a fully-qualifiedhttps://www.youtube.com/...URL.Convert the handle to a full YouTube URL before passing it to the adapter.
Closes #5429
Test Plan
Added
lib/crewai-tools/tests/tools/test_youtube_channel_search_tool.pywith two unit tests that assert the adapter receives a properhttps://www.youtube.com/@handleURL.Test Result
Before fix:
ValueError: Invalid YouTube channel URL: @krishnaik06After fix: adapter receives
https://www.youtube.com/@krishnaik06Summary by CodeRabbit
New Features
Tests