fix: YoutubeChannelSearchTool passes bare handle instead of full URL (#5429)#5829
Closed
NIK-TIGER-BILL wants to merge 4 commits into
Closed
fix: YoutubeChannelSearchTool passes bare handle instead of full URL (#5429)#5829NIK-TIGER-BILL wants to merge 4 commits into
NIK-TIGER-BILL wants to merge 4 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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR normalizes YouTube channel handles into full https://www.youtube.com/... URLs before delegation and adds a fallback to message.reasoning_content for OpenAI chat completions across sync/async and streaming/non-streaming paths, with tests validating both behaviors. ChangesYouTube Channel Search Tool URL Normalization
OpenAI Reasoning Content Fallback Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
+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.
Accept fully-qualified URLs as-is; only normalize bare handles. Fixes review feedback from CodeRabbit on PR crewAIInc#5829. Signed-off-by: NIK-TIGER-BILL <nik.tiger.bill@github.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
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.
Closing this PR as the related issue has been closed and there has been no maintainer review for over 10 days.