-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix transcript handling issue in youtube-trend-analysis #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the YouTube trend analysis workflow to process and validate video transcripts after scraping, saving valid transcripts as text files and excluding erroneous ones. The analysis now uses merged transcript contents instead of file paths. Additional improvements include UTF-8 decoding for subprocess output and updated configuration text to reflect the new content-based processing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Youtube-trend-analysis/config.yaml (1)
5-5
: Remove trailing spaces for consistency.The configuration changes correctly update the parameter references from
{file_paths}
to{file_contents}
, which aligns with the implementation. However, there are trailing spaces on these lines that should be removed.Apply this diff to remove trailing spaces:
- Analyze the transcripts of several videos using {file_contents}. + Analyze the transcripts of several videos using {file_contents}.- Conduct a fine-grained analysis of the transcripts of several videos using {file_contents}. + Conduct a fine-grained analysis of the transcripts of several videos using {file_contents}.Also applies to: 37-37
Youtube-trend-analysis/app.py (1)
136-136
: Remove redundant directory creation.The
transcripts
directory is already created at line 21. This redundant creation inside the loop is unnecessary.- os.makedirs("transcripts", exist_ok=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Youtube-trend-analysis/app.py
(3 hunks)Youtube-trend-analysis/brightdata_scrapper.py
(1 hunks)Youtube-trend-analysis/config.yaml
(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
Youtube-trend-analysis/config.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
[error] 37-37: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
Youtube-trend-analysis/brightdata_scrapper.py (1)
108-110
: Good improvements for robustness!The addition of UTF-8 encoding and stdout validation effectively addresses potential decoding issues and prevents errors when parsing empty responses. These changes align well with the PR objectives.
Youtube-trend-analysis/app.py (2)
204-211
: Well-implemented file content merging!The implementation correctly reads all transcript files and merges their contents into a single string, which aligns perfectly with the configuration changes. The UTF-8 encoding ensures proper handling of special characters.
129-166
: Excellent transcript processing implementation!The transcript processing logic effectively handles both structured (list) and unstructured transcript formats. The cleanup of erroneous transcript files and tracking of error indices ensures data integrity for downstream analysis.
with open(file, "w", encoding="utf-8") as f: | ||
transcript = channel_scrapped_output[0][i].get( | ||
"formatted_transcript", [] | ||
) | ||
if isinstance(transcript, list): | ||
for entry in transcript: | ||
text = entry.get("text", "") | ||
start = entry.get("start_time", 0.0) | ||
end = entry.get("end_time", 0.0) | ||
line = f"({start:.2f}-{end:.2f}): {text}\n" | ||
f.write(line) | ||
else: | ||
f.write(str(transcript)) | ||
error_files.append(i) | ||
del st.session_state.all_files[-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for file operations.
The file writing operations should include error handling to gracefully handle potential issues like permission errors or encoding problems.
Wrap the file operations in a try-except block:
- with open(file, "w", encoding="utf-8") as f:
- transcript = channel_scrapped_output[0][i].get(
- "formatted_transcript", []
- )
- if isinstance(transcript, list):
- for entry in transcript:
- text = entry.get("text", "")
- start = entry.get("start_time", 0.0)
- end = entry.get("end_time", 0.0)
- line = f"({start:.2f}-{end:.2f}): {text}\n"
- f.write(line)
- else:
- f.write(str(transcript))
- error_files.append(i)
- del st.session_state.all_files[-1]
+ try:
+ with open(file, "w", encoding="utf-8") as f:
+ transcript = channel_scrapped_output[0][i].get(
+ "formatted_transcript", []
+ )
+ if isinstance(transcript, list):
+ for entry in transcript:
+ text = entry.get("text", "")
+ start = entry.get("start_time", 0.0)
+ end = entry.get("end_time", 0.0)
+ line = f"({start:.2f}-{end:.2f}): {text}\n"
+ f.write(line)
+ else:
+ f.write(str(transcript))
+ error_files.append(i)
+ del st.session_state.all_files[-1]
+ except (IOError, OSError) as e:
+ st.error(f"Failed to write transcript for video {youtube_video_id}: {str(e)}")
+ error_files.append(i)
+ if file in st.session_state.all_files:
+ st.session_state.all_files.remove(file)
📝 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.
with open(file, "w", encoding="utf-8") as f: | |
transcript = channel_scrapped_output[0][i].get( | |
"formatted_transcript", [] | |
) | |
if isinstance(transcript, list): | |
for entry in transcript: | |
text = entry.get("text", "") | |
start = entry.get("start_time", 0.0) | |
end = entry.get("end_time", 0.0) | |
line = f"({start:.2f}-{end:.2f}): {text}\n" | |
f.write(line) | |
else: | |
f.write(str(transcript)) | |
error_files.append(i) | |
del st.session_state.all_files[-1] | |
try: | |
with open(file, "w", encoding="utf-8") as f: | |
transcript = channel_scrapped_output[0][i].get( | |
"formatted_transcript", [] | |
) | |
if isinstance(transcript, list): | |
for entry in transcript: | |
text = entry.get("text", "") | |
start = entry.get("start_time", 0.0) | |
end = entry.get("end_time", 0.0) | |
line = f"({start:.2f}-{end:.2f}): {text}\n" | |
f.write(line) | |
else: | |
f.write(str(transcript)) | |
error_files.append(i) | |
del st.session_state.all_files[-1] | |
except (IOError, OSError) as e: | |
st.error(f"Failed to write transcript for video {youtube_video_id}: {str(e)}") | |
error_files.append(i) | |
if file in st.session_state.all_files: | |
st.session_state.all_files.remove(file) |
🤖 Prompt for AI Agents
In Youtube-trend-analysis/app.py around lines 141 to 155, the file writing
operations lack error handling, which can cause the program to crash on issues
like permission errors or encoding problems. Wrap the entire file open and write
block in a try-except statement to catch exceptions such as IOError or OSError.
In the except block, log or handle the error appropriately to ensure the program
continues running gracefully without abrupt termination.
This pull request fixes transcript handling issues in the youtube-trend-analysis project.
Changes include:
transcripts
directory is created automatically if missingformatted_transcript
is missing by skipping and removing invalid entriesget_output()
to avoid parsing empty responsesconfig.yaml
to switch fromfile_contents
tofile_paths
as task inputSummary by CodeRabbit
New Features
Bug Fixes
Documentation