fix: YouTube transcript any-language + delete dialog + rename 500#19
Merged
Merged
Conversation
backend: - youtube_fetcher: use api.list() + find_transcript() to fetch transcript in any available language instead of defaulting to English only — fixes NoTranscriptFound for non-English videos - supervisor: add deterministic Python fast-path using extract_video_id() so YouTube URLs are always routed to youtube_agent without relying on LLM - conversation_service: two-query pattern for update_conversation_title — supabase-py v2 does not support .update().select() chaining (postgrest-py issue #394), causing 500 on rename - main: add global exception handler so unhandled 500s pass through CORSMiddleware — prevents browser from reporting them as 'Failed to fetch' - security: simplify redundant except tuple to bare except Exception - service: emit tool_use SSE event when youtube_agent node starts frontend: - ConversationSidebar: replace 2-click confirm with proper delete dialog (single click → dialog with Cancel / Delete buttons) - ChatWindow: show Play icon (red) for YouTube tool, Database icon for memory - dialog.tsx: new Dialog UI component built on @base-ui/react
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
WalkthroughThis PR enhances the chat agent system with YouTube video integration, improves conversation management UX via dialog modals, and strengthens infrastructure error handling. The supervisor now detects YouTube URLs in user messages and routes them directly to a dedicated YouTube agent without LLM involvement. Conversation deletion uses a dedicated modal dialog. Supabase query chaining limitations are worked around by splitting updates into separate operations. Global exception handling logs unhandled errors consistently. ChangesYouTube Agent Integration
Conversation Management & Dialog UX
Infrastructure & Error Handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/features/chat/components/ConversationSidebar.tsx (1)
250-306: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winSplit this file to satisfy the 300-line limit.
This file now reaches 306 lines (see Line 306). Please extract
ConversationRow/SkeletonRowinto separate feature component files to comply with project constraints.As per coding guidelines,
No file longer than 300 lines — split into smaller modules.🤖 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 `@frontend/features/chat/components/ConversationSidebar.tsx` around lines 250 - 306, This file exceeds the 300-line limit; extract the ConversationRow and SkeletonRow components out of ConversationSidebar into their own feature component files (e.g., ConversationRow.tsx and SkeletonRow.tsx), keep ConversationSidebar as the parent that imports these components, export the new components as default or named exports, update the import statements in ConversationSidebar to import ConversationRow and SkeletonRow, and ensure any props types (e.g., ConversationRow's props and ConversationSidebarProps) are exported or moved to a shared types file if needed so existing usage (onSelect, onDeleted, isActive, conversation) continues to work.
🤖 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 `@backend/app/agents/subagents/youtube_fetcher.py`:
- Around line 71-83: Wrap the body of the _sync (or equivalent) function that
calls api.list(video_id) and
transcript_list.find_transcript(...).fetch().to_raw_data() in a try/except,
catch Exception as e, use the module logger (logging.getLogger(__name__)) to log
an error that includes the video_id and the exception (e) or use
logger.exception for traceback, then re-raise the exception so upstream handlers
still receive it; ensure both the api.list(video_id) call and the subsequent
transcript_list.find_transcript(...).fetch().to_raw_data() are inside the try
block so failures from either are logged with video_id context before being
re-raised.
In `@backend/app/agents/supervisor.py`:
- Around line 190-230: The fast-path currently calls extract_video_id(raw) which
misses common YouTube URL variants (e.g., m.youtube.com/watch, /live/, /embed/)
causing the branch to be skipped; update the logic used in supervisor.py by
either expanding extract_video_id to recognize these additional URL patterns
(including subdomains like m.youtube.com, paths /live/ and /embed/, and query
param forms) or pre-normalize raw before calling extract_video_id (parse any
URLs from last_human.content, normalize host to youtube.com, strip prefixes,
then pass the normalized URL or extracted path to extract_video_id) so the
YouTube handoff triggers for those variants. Ensure you reference
extract_video_id and the last_human/raw extraction flow when making the change.
- Around line 238-241: The model is being bound with transfer_to_youtube_agent
on first pass which reintroduces LLM-controlled YouTube routing; change the
binding logic so that when constructing the model for the first pass (where
agent_ran is False) you only bind transfer_to_memory_agent and explicitly
exclude transfer_to_youtube_agent (leave it unbound), and ensure
_get_model(model_id, ...) and its cache key are adjusted to depend on the actual
set of bound tools (not just the agent_ran boolean) so different tool-sets
produce distinct cached models.
In `@backend/app/features/chat/conversation_service.py`:
- Around line 246-262: The check that treats a successful Supabase update as
"not found" should be removed: delete the conditional that inspects
update_result.data (the "if not update_result.data: return None" block) in the
update_conversation_title flow so the function relies on the subsequent
select/fetch query instead of response.data; leave the update call, exception
handling (logger.exception), and the follow-up select logic intact so the real
existence/result is determined by the explicit select rather than the empty
update_result.data.
---
Outside diff comments:
In `@frontend/features/chat/components/ConversationSidebar.tsx`:
- Around line 250-306: This file exceeds the 300-line limit; extract the
ConversationRow and SkeletonRow components out of ConversationSidebar into their
own feature component files (e.g., ConversationRow.tsx and SkeletonRow.tsx),
keep ConversationSidebar as the parent that imports these components, export the
new components as default or named exports, update the import statements in
ConversationSidebar to import ConversationRow and SkeletonRow, and ensure any
props types (e.g., ConversationRow's props and ConversationSidebarProps) are
exported or moved to a shared types file if needed so existing usage (onSelect,
onDeleted, isActive, conversation) continues to work.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1e6f8487-dedb-4fa4-a6a9-a08a9ee97073
📒 Files selected for processing (9)
backend/app/agents/subagents/youtube_fetcher.pybackend/app/agents/supervisor.pybackend/app/core/security.pybackend/app/features/chat/conversation_service.pybackend/app/features/chat/service.pybackend/app/main.pyfrontend/components/ui/dialog.tsxfrontend/features/chat/components/ChatWindow.tsxfrontend/features/chat/components/ConversationSidebar.tsx
Comment on lines
+190
to
+230
| # -- Deterministic YouTube fast-path ------------------------------------ | ||
| # Detect a YouTube URL in the last HumanMessage without asking the LLM. | ||
| # This avoids the failure mode where the model ignores the routing | ||
| # instruction and hallucinates a summary from its training data. | ||
| if not agent_ran: | ||
| last_human = next( | ||
| (m for m in reversed(messages) if isinstance(m, HumanMessage)), | ||
| None, | ||
| ) | ||
| if last_human: | ||
| raw = ( | ||
| last_human.content | ||
| if isinstance(last_human.content, str) | ||
| else " ".join( | ||
| p | ||
| if isinstance(p, str) | ||
| else p.get("text", "") | ||
| if isinstance(p, dict) | ||
| else getattr(p, "text", "") | ||
| for p in last_human.content | ||
| ) | ||
| ) | ||
| if extract_video_id(raw): | ||
| logger.info( | ||
| "[supervisor] YouTube URL detected — fast-path to youtube_agent" | ||
| ) | ||
| return { | ||
| "messages": [ | ||
| AIMessage( | ||
| content="", | ||
| tool_calls=[ | ||
| { | ||
| "id": str(uuid4()), | ||
| "name": YOUTUBE_HANDOFF_TOOL_NAME, | ||
| "args": {}, | ||
| "type": "tool_call", | ||
| } | ||
| ], | ||
| ) | ||
| ] | ||
| } |
There was a problem hiding this comment.
The deterministic fast-path still misses common YouTube URL forms.
This branch only runs when extract_video_id(raw) matches. That helper currently recognizes watch?v=, youtu.be/, and /shorts/ URLs, so links such as m.youtube.com/watch?v=..., /live/..., or /embed/... still fall back to the LLM and reopen the hallucination path this fast-path is supposed to remove. Expand the extractor or normalize URLs before using it here.
🤖 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 `@backend/app/agents/supervisor.py` around lines 190 - 230, The fast-path
currently calls extract_video_id(raw) which misses common YouTube URL variants
(e.g., m.youtube.com/watch, /live/, /embed/) causing the branch to be skipped;
update the logic used in supervisor.py by either expanding extract_video_id to
recognize these additional URL patterns (including subdomains like
m.youtube.com, paths /live/ and /embed/, and query param forms) or pre-normalize
raw before calling extract_video_id (parse any URLs from last_human.content,
normalize host to youtube.com, strip prefixes, then pass the normalized URL or
extracted path to extract_video_id) so the YouTube handoff triggers for those
variants. Ensure you reference extract_video_id and the last_human/raw
extraction flow when making the change.
backend: - youtube_fetcher: wrap _sync body in try/except, log error with video_id before re-raising so failures from api.list() and find_transcript().fetch() are visible in logs with context - youtube_fetcher: expand _YOUTUBE_URL_RE to match m.youtube.com, /live/, /embed/ paths in addition to existing watch?v=, youtu.be, /shorts/ - supervisor: rebind _get_model to accept tools list instead of bool; on first pass only bind transfer_to_memory_agent (exclude youtube tool so LLM can never hallucinate YouTube routing); cache key is now tuple of tool names - conversation_service: remove early 'if not update_result.data: return None' after update query — supabase-py v2 returns empty data on successful updates; real existence is determined by the subsequent select query frontend: - ConversationSidebar: split SkeletonRow into SkeletonRow.tsx and ConversationRow into ConversationRow.tsx to satisfy 300-line limit; ConversationSidebar is now a lean 70-line orchestrator
This was referenced May 10, 2026
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.
backend:
frontend:
Summary by CodeRabbit
Release Notes
New Features
Improvements