Experimented with AI to add more Features #1
Conversation
📝 WalkthroughWalkthroughThe PR introduces multi-character dialogue support with customizable subtitle styling across the backend and web UI. Changes include dynamic character loading from resource folders, per-character TTS generation, dialogue parsing from story text, character image overlays in videos, and new web endpoints for character and subtitle configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Web UI
participant Parser as Story Parser
participant TTS as TTS Generator
participant CharCfg as Character Config
participant VidGen as Video Generator
participant Output as Output Video
User->>Parser: Submit story with dialogue
Parser->>Parser: parse_dialogue()
rect rgb(200, 220, 240)
Note over Parser: Detect [Character]: text format
Parser->>Parser: Extract dialogue segments & narrative
end
Parser-->>User: is_dialogue=true, dialogue[], narrative
User->>TTS: Generate TTS (is_dialogue, dialogue)
loop For each dialogue segment
TTS->>CharCfg: Resolve character voice config
CharCfg-->>TTS: voice, display_name
TTS->>TTS: Generate per-character MP3
TTS->>TTS: Append to dialogue_tts[]
end
TTS-->>User: dialogue_tts paths & metadata
User->>VidGen: create_short_video(dialogue_tts, is_dialogue)
rect rgb(220, 240, 220)
Note over VidGen: Dialogue mode processing
VidGen->>VidGen: Generate manual SRT from segments
VidGen->>VidGen: Apply subtitle styling
VidGen->>VidGen: Overlay character images by timing
end
VidGen->>VidGen: Concat audio/video, apply filters
VidGen-->>Output: Final video with dialogue, character images, styled subtitles
sequenceDiagram
participant WebUI as Web UI
participant Server as Server Routes
participant Config as Config Module
participant FS as File System
WebUI->>Server: GET /api/characters
rect rgb(200, 220, 240)
Note over Server: Character Endpoint
Server->>Config: Access characters list
Config->>FS: Scan character_resources_path/*/
FS-->>Config: Subdirectory list
loop For each character folder
Config->>FS: Load config.json, select image
FS-->>Config: Character data
end
Config-->>Server: characters[] with name, image, voice, display_name
end
Server-->>WebUI: JSON array of characters
WebUI->>Server: GET /api/subtitle-defaults
rect rgb(220, 240, 220)
Note over Server: Subtitle Defaults Endpoint
Server->>Config: Access subtitle constants
Config-->>Server: SUBTITLE_* values
end
Server-->>WebUI: Subtitle styling defaults
WebUI->>WebUI: Render character cards
WebUI->>WebUI: Populate subtitle form fields with defaults
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
|
after testing i will let you know. thanks for the pr |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web_ui/routes.py (2)
103-103: Remove redundant parentheses.The double parentheses in
Voice((code))are unnecessary and likely a typo. This should beVoice(code).🔎 Proposed fix
- voice_enum_member = Voice((code)) # Get enum member by value (code) + voice_enum_member = Voice(code) # Get enum member by value (code)
217-264: Missing route decorator - endpoint not exposed.The
generate_video()function has no@main_bp.route()decorator, so this endpoint is not accessible. Based on the AI summary indicating the old decorator was removed, this appears to be an accidental deletion.🔎 Proposed fix - add route decorator
+@main_bp.route('/api/generate', methods=['POST']) def generate_video(): """Generate a video from the provided script and settings"""
🧹 Nitpick comments (7)
README.md (1)
121-130: Minor inconsistency in image file naming guidance.The documentation states the image should be named
image.jpgorimage.png, but the code inconfig.py(lines 123-126) searches for any file ending with.png,.jpg, or.jpegand uses the first one found. This is more flexible than documented.Consider updating the documentation to reflect this flexibility, or keep it simple as-is since the current guidance works correctly.
reddit_shorts/make_tts.py (1)
176-185: Character lookup can be simplified using the mapping pattern.The loop iterates through all characters for each dialogue segment. Since you're importing
charactersfrom config, consider using a dictionary lookup for O(1) access instead of O(n) per segment.Suggested optimization
from reddit_shorts.config import characters - # Create a mapping of character names to their voices and info - char_map = {char['name']: char for char in characters} + # Create mappings for efficient character lookup (case-insensitive) + char_by_name = {char['name'].lower(): char for char in characters} + char_by_display = {char['display_name'].lower(): char for char in characters} for i, dialogue_segment in enumerate(dialogue): char_name = dialogue_segment['character'] char_text = dialogue_segment['text'] # Find character config - char_config = None - for char in characters: - if char['display_name'].lower() == char_name.lower() or char['name'].lower() == char_name.lower(): - char_config = char - break + char_name_lower = char_name.lower() + char_config = char_by_display.get(char_name_lower) or char_by_name.get(char_name_lower)reddit_shorts/config.py (1)
116-139: Character loading logic is sound with appropriate fallbacks.The dynamic character loading correctly handles missing directories and invalid configurations with warnings. A few observations:
- The
breakon line 126 means only the first matching image file is used, which is reasonable.- The broad
Exceptioncatch on line 138 is acceptable here since it's configuration loading with a warning, not silent failure.One edge case: if
config.jsonexists butimage_fileisNone(no image found), the character is silently skipped. Consider adding a warning for this scenario.Optional: Add warning for missing image files
if image_file and os.path.exists(config_file): try: with open(config_file, 'r') as f: char_config = json.load(f) characters.append({ 'name': item, 'image': image_file, 'voice': char_config.get('voice', 'en_us_002'), 'display_name': char_config.get('display_name', item) }) except Exception as e: print(f"Warning: Could not load character config for {item}: {e}") + elif os.path.exists(config_file) and not image_file: + print(f"Warning: Character '{item}' has config.json but no image file.")web_ui/static/index.html (1)
167-208: Subtitle settings form is well-structured.The form provides comprehensive control over subtitle styling with appropriate input types (
type="number"for numeric fields). Thev-model.numbermodifier ensures values are parsed as numbers.Consider adding basic input validation or constraints (e.g.,
min="0"for font size, outline thickness) to prevent invalid values from being submitted.Optional: Add min constraints for numeric inputs
<div> <label class="block mb-2">Font Size</label> - <input v-model.number="script.subtitle_font_size" type="number" class="w-full bg-gray-700 rounded p-2"> + <input v-model.number="script.subtitle_font_size" type="number" min="1" class="w-full bg-gray-700 rounded p-2"> </div>reddit_shorts/create_short.py (3)
11-11: Consider splitting the long import for readability.The import line has grown quite long with the subtitle constants. While functional, splitting it would improve maintainability.
🔎 Proposed refactor
-from reddit_shorts.config import project_path as default_project_path, footage, music, output_video_path as global_output_video_path, SUBTITLE_FONT, SUBTITLE_FONT_SIZE, SUBTITLE_MARGIN_V, SUBTITLE_BOLD, SUBTITLE_OUTLINE_COLOUR, SUBTITLE_BORDER_STYLE, SUBTITLE_OUTLINE, SUBTITLE_SHADOW, SUBTITLE_SHADOW_COLOUR +from reddit_shorts.config import ( + project_path as default_project_path, + footage, + music, + output_video_path as global_output_video_path, + SUBTITLE_FONT, + SUBTITLE_FONT_SIZE, + SUBTITLE_MARGIN_V, + SUBTITLE_BOLD, + SUBTITLE_OUTLINE_COLOUR, + SUBTITLE_BORDER_STYLE, + SUBTITLE_OUTLINE, + SUBTITLE_SHADOW, + SUBTITLE_SHADOW_COLOUR, +)
202-220: Catch specific exceptions and consider making Whisper model configurable.The broad
Exceptioncatch at line 216 can hide unexpected errors. Additionally, the hardcoded "tiny.en" model at line 206 limits flexibility.🔎 Proposed improvements
else: # Whisper transcription for regular subtitles try: print("Starting Whisper transcription for subtitles...") writer_options = {"max_line_count": 1, "max_words_per_line": 1} - whisper_model = whisper.load_model("tiny.en", device="cpu") # Consider base or small for better accuracy if tiny is too basic + whisper_model_name = kwargs.get('whisper_model', 'tiny.en') + whisper_model = whisper.load_model(whisper_model_name, device="cpu") tts_combined_transcribed = whisper_model.transcribe(tts_combined_path, language="en", fp16=False, word_timestamps=True, task="transcribe") srt_writer = get_writer("srt", os.path.dirname(tts_combined_srt_path)) # Pass directory to writer srt_writer(tts_combined_transcribed, os.path.basename(tts_combined_srt_path), writer_options) print(f"Subtitles generated: {tts_combined_srt_path}") if not os.path.exists(tts_combined_srt_path): print("Warning: SRT file was not created by Whisper.") # Fallback: create an empty SRT file to prevent ffmpeg error if subtitles are mandatory in filter graph with open(tts_combined_srt_path, 'w') as f: f.write("") - except Exception as e: + except (OSError, RuntimeError, whisper.DecodingError) as e: print(f"Error during Whisper transcription: {e}. Subtitles might be missing.") # Fallback: create an empty SRT file with open(tts_combined_srt_path, 'w') as f: f.write("")Note: Verify the specific exception types that Whisper can raise by checking the library documentation.
367-403: Verify character image paths and consider extracting overlay logic.The character configuration structure at
reddit_shorts/config.py:113-139correctly populateschar['image']with full filesystem paths usingos.path.join(char_dir, file). The existence check at line 386 increate_short.pyis appropriate and provides defensive validation before using the image path.Consider extracting the overlay logic into a helper function to reduce complexity and improve maintainability:
💡 Optional refactor to reduce complexity
Extract character overlay creation into a helper function:
def add_character_overlays(main_stream, dialogue_tts, char_image_map, char_display_map, start_time, space_between_tts): """Add character image overlays for dialogue segments.""" current_time = start_time for dialogue_segment in dialogue_tts: char_name = dialogue_segment['character'] segment_path = dialogue_segment['path'] char_key = char_display_map.get(char_name, char_name) char_image_path = char_image_map.get(char_key) if char_image_path and os.path.exists(char_image_path): segment_duration = get_audio_duration(segment_path) if segment_duration > 0: char_overlay = ( ffmpeg .input(char_image_path) .filter('scale', w='min(400,iw)', h='-1') ) main_stream = ffmpeg.overlay( main_stream, char_overlay, x='W-w-20', y='H-h-20', enable=f'between(t,{current_time},{current_time + segment_duration})' ) print(f"Added character overlay for '{char_name}' from {current_time:.1f}s to {current_time + segment_duration:.1f}s") current_time += segment_duration + space_between_tts return main_stream
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.mdreddit_shorts/config.pyreddit_shorts/create_short.pyreddit_shorts/get_reddit_stories.pyreddit_shorts/main.pyreddit_shorts/make_tts.pystories.txtweb_ui/routes.pyweb_ui/static/index.html
🧰 Additional context used
🧬 Code graph analysis (1)
reddit_shorts/make_tts.py (1)
reddit_shorts/tiktok_voice/src/voice.py (1)
Voice(4-121)
🪛 LanguageTool
stories.txt
[style] ~8-~8: Consider using a more formal and expressive alternative to ‘amazing’.
Context: ...th multiple characters! [Alice]: That's amazing! I can't wait to try it out. [Bob]: Me ...
(AWESOME)
[style] ~9-~9: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 547 characters long)
Context: ...'t wait to try it out. [Bob]: Me neither! Let's make some viral content.
(EN_EXCESSIVE_EXCLAMATION)
🪛 Ruff (0.14.10)
reddit_shorts/create_short.py
175-175: Undefined name title_tts_duration
(F821)
175-175: Undefined name title_tts_duration
(F821)
216-216: Do not catch blind exception: Exception
(BLE001)
reddit_shorts/make_tts.py
174-174: Local variable char_map is assigned to but never used
Remove assignment to unused variable char_map
(F841)
193-193: Do not use bare except
(E722)
218-218: Do not catch blind exception: Exception
(BLE001)
reddit_shorts/config.py
138-138: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (13)
reddit_shorts/get_reddit_stories.py (1)
92-99: LGTM!The dialogue parsing integration is well-structured. The
parse_dialoguefunction is correctly called after assembling the story text, and the returned dialogue data is properly added to the story dictionary with clear keys (is_dialogue,dialogue,narrative).reddit_shorts/main.py (1)
36-44: LGTM!The new
dialogueandis_dialogueparameters are correctly passed through to the TTS generation function with appropriate default values. This cleanly integrates the dialogue-aware TTS path without disrupting the existing flow.README.md (1)
9-27: LGTM - Clear quick setup instructions!The Quick Setup section provides a good onboarding experience with practical commands for creating required directories and setting up character resources. The example
config.jsonstructure aligns with what the code expects inreddit_shorts/config.py.reddit_shorts/make_tts.py (1)
168-221: Consider consolidating error handling for dialogue TTS generation.The dialogue TTS block properly generates per-character audio segments. The implementation correctly falls back to the default voice when character configuration is missing or invalid.
One consideration: if TTS generation fails for a dialogue segment (line 218), the loop continues but the failed segment won't be in
dialogue_tts. Downstream code increate_short.pyshould handle gaps gracefully.reddit_shorts/config.py (1)
98-107: LGTM - Well-structured subtitle configuration constants.The subtitle configuration constants provide sensible defaults and clear documentation through naming. The ASS/SSA color format (
&HFF000000) is correctly used for FFmpeg subtitle styling.web_ui/static/index.html (2)
140-157: LGTM - Clean character display implementation.The Available Characters section correctly renders character cards with images and voice info. The
v-if="characters.length > 0"guard ensures the section only appears when characters are configured.
295-303: Default value assignment logic handles edge cases appropriately.The conditional checks properly distinguish between empty strings and
nullvalues when applying defaults:
- String fields use
!this.script.subtitle_font(falsy check)- Numeric fields use
=== nullto preserve explicit0valuesThis is correct behavior for fields where
0might be a valid user input (e.g.,subtitle_bold).web_ui/routes.py (2)
171-186: LGTM - Character enumeration endpoint looks good.The endpoint correctly retrieves character data from config and returns appropriate metadata including the image URL route.
202-216: LGTM - Subtitle defaults endpoint is correct.The endpoint appropriately exposes subtitle configuration constants from config. The long import line is acceptable given it's used immediately.
reddit_shorts/create_short.py (4)
52-53: LGTM - Dialogue parameters are well-defined.The new
dialogue_ttsandis_dialoguekwargs cleanly extend the function signature for dialogue support.
112-126: LGTM - Dialogue TTS processing is well-structured.The dialogue segment processing correctly handles per-segment audio concatenation with appropriate silence between segments.
128-143: LGTM - Narration fallback preserves backward compatibility.The non-dialogue path correctly maintains the original narration logic with helpful warning messages for missing tracks.
405-424: LGTM - Configurable subtitle styling is well-implemented.The subtitle filter correctly applies configurable styling with sensible fallbacks to config defaults. The conditional check for SRT existence prevents errors.
| if is_dialogue and dialogue_tts: | ||
| # For dialogue, create subtitles manually with character names | ||
| print("Creating manual subtitles for dialogue...") | ||
| with open(tts_combined_srt_path, 'w', encoding='utf-8') as srt_file: | ||
| subtitle_index = 1 | ||
| current_time = title_tts_duration + (space_between_tts if title_tts_duration > 0 and dialogue_tts else 0) | ||
|
|
||
| for dialogue_segment in dialogue_tts: | ||
| char_name = dialogue_segment['character'] | ||
| segment_text = dialogue_segment['text'] | ||
| segment_path = dialogue_segment['path'] | ||
|
|
||
| if os.path.exists(segment_path): | ||
| segment_duration = get_audio_duration(segment_path) | ||
| if segment_duration > 0: | ||
| # Format timestamps for SRT (HH:MM:SS,mmm) | ||
| start_time = current_time | ||
| end_time = current_time + segment_duration | ||
|
|
||
| start_str = f"{int(start_time//3600):02d}:{int((start_time%3600)//60):02d}:{start_time%60:06.3f}".replace('.', ',') | ||
| end_str = f"{int(end_time//3600):02d}:{int((end_time%3600)//60):02d}:{end_time%60:06.3f}".replace('.', ',') | ||
|
|
||
| # Write SRT entry | ||
| srt_file.write(f"{subtitle_index}\n") | ||
| srt_file.write(f"{start_str} --> {end_str}\n") | ||
| srt_file.write(f"[{char_name}]: {segment_text}\n\n") | ||
|
|
||
| subtitle_index += 1 | ||
| current_time += segment_duration + space_between_tts | ||
|
|
||
| print(f"Manual subtitles created: {tts_combined_srt_path}") |
There was a problem hiding this comment.
Critical: Undefined variable title_tts_duration at line 175.
The variable title_tts_duration is used at line 175 but not defined until line 323. This will cause a NameError when executing dialogue mode.
🔎 Proposed fix - move title_tts_duration calculation earlier
# --- End Audio Processing ---
soundduration = get_audio_duration(tts_combined_path) # This is the duration of combined narration
if soundduration == 0.0:
print("Error: Combined TTS audio has zero duration. Cannot proceed.")
shutil.rmtree(temp_processing_dir, ignore_errors=True)
return None
+
+ # Calculate title TTS duration early for subtitle timing
+ title_tts_duration = get_audio_duration(narrator_title_track_path) if narrator_title_track_path else 0
# Subtitle generation
if is_dialogue and dialogue_tts:Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.10)
175-175: Undefined name title_tts_duration
(F821)
175-175: Undefined name title_tts_duration
(F821)
🤖 Prompt for AI Agents
In reddit_shorts/create_short.py around lines 170 to 200, the code references
title_tts_duration before it is defined which will raise a NameError; move the
calculation that sets title_tts_duration to a point earlier (before the
dialogue/manual subtitles block) or compute a safe default (e.g., 0) there so
the variable always exists when used; ensure the moved calculation derives the
duration from the title TTS audio (or sets 0 if no title TTS) and keep any
spacing logic unchanged.
| def parse_dialogue(text: str) -> dict: | ||
| """ | ||
| Parse dialogue format like [Character 1]: <lines>, [Character 2]: <lines> | ||
| Returns a dict with 'dialogue' key containing list of (character, text) tuples, | ||
| and 'narrative' key containing non-dialogue text. | ||
| """ | ||
| dialogue_pattern = r'\[([^\]]+)\]:\s*(.+?)(?=\[|$)' # Match [Character]: text until next [ or end | ||
| matches = re.findall(dialogue_pattern, text, re.DOTALL) | ||
|
|
||
| if matches: | ||
| # This is a dialogue story | ||
| dialogue = [] | ||
| for char, line in matches: | ||
| dialogue.append({ | ||
| 'character': char.strip(), | ||
| 'text': line.strip() | ||
| }) | ||
|
|
||
| # Remove dialogue from text to get narrative | ||
| narrative = re.sub(dialogue_pattern, '', text, flags=re.DOTALL).strip() | ||
|
|
||
| return { | ||
| 'is_dialogue': True, | ||
| 'dialogue': dialogue, | ||
| 'narrative': narrative | ||
| } | ||
| else: | ||
| # Regular narrative story | ||
| return { | ||
| 'is_dialogue': False, | ||
| 'dialogue': [], | ||
| 'narrative': text | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Test the regex pattern behavior with various edge cases
python3 << 'EOF'
import re
dialogue_pattern = r'\[([^\]]+)\]:\s*(.+?)(?=\[|$)'
test_cases = [
"[Alice]: Hello\n[Bob]: World",
"[Alice]: I saw [something]!\n[Bob]: What?",
"[Alice]: Line 1\nLine 2\n[Bob]: Response",
"[Alice]:\n[Bob]: Only Bob speaks"
]
print("Testing regex pattern: " + repr(dialogue_pattern))
print("=" * 60)
for i, text in enumerate(test_cases):
print(f"\nTest {i+1}: {repr(text)}")
matches = re.findall(dialogue_pattern, text, re.DOTALL)
if matches:
for char, line in matches:
print(f" Character: '{char}', Text: '{line.strip()}'")
else:
print(" No matches")
EOFRepository: egebese/brainrot-generator
Length of output: 156
🏁 Script executed:
git ls-files | grep -E "reddit_shorts|get_reddit_stories"Repository: egebese/brainrot-generator
Length of output: 1408
🏁 Script executed:
head -60 reddit_shorts/get_reddit_stories.py | cat -nRepository: egebese/brainrot-generator
Length of output: 2671
🏁 Script executed:
python3 << 'EOF'
import re
# Test the regex pattern behavior
dialogue_pattern = r'\[([^\]]+)\]:\s*(.+?)(?=\[|$)'
# Test case 2: dialogue containing brackets
text = "[Alice]: I saw [something]!\n[Bob]: What?"
print("Test: Dialogue with brackets in text")
print(f"Input: {repr(text)}")
matches = re.findall(dialogue_pattern, text, re.DOTALL)
print(f"Matches: {matches}")
EOFRepository: egebese/brainrot-generator
Length of output: 206
🏁 Script executed:
cat -n tests/get_reddit_stories_test.pyRepository: egebese/brainrot-generator
Length of output: 1058
🏁 Script executed:
python3 << 'EOF'
import re
dialogue_pattern = r'\[([^\]]+)\]:\s*(.+?)(?=\[|$)'
test_cases = [
("[Alice]: Hello\n[Bob]: World", "Normal dialogue"),
("[Alice]: I saw [something]!\n[Bob]: What?", "Brackets in dialogue"),
("[Alice]: Line 1\nLine 2\n[Bob]: Response", "Multi-line dialogue"),
("[Alice]:\n[Bob]: Only Bob", "Empty dialogue"),
("[Alice]: Text with [bracket and more text\n[Bob]: Response", "Unclosed bracket in dialogue"),
]
print("Regex pattern:", repr(dialogue_pattern))
print("=" * 70)
for text, description in test_cases:
print(f"\n{description}:")
print(f"Input: {repr(text)}")
matches = re.findall(dialogue_pattern, text, re.DOTALL)
print(f"Output: {matches}")
if matches:
for char, line in matches:
print(f" - '{char.strip()}': '{line.strip()}'")
EOFRepository: egebese/brainrot-generator
Length of output: 1058
🏁 Script executed:
# Check if parse_dialogue is used elsewhere in the codebase
rg "parse_dialogue" --type pyRepository: egebese/brainrot-generator
Length of output: 243
Dialogue parsing regex breaks when dialogue text contains bracket characters.
The regex pattern r'\[([^\]]+)\]:\s*(.+?)(?=\[|$)' with re.DOTALL stops at any [ character, not just dialogue entry markers. Testing confirms:
- Input:
[Alice]: I saw [something]!\n[Bob]: What?→ Parses as[('Alice', 'I saw '), ('Bob', 'What?')], truncating Alice's dialogue at[something] - Input:
[Alice]:\n[Bob]: Only Bob→ Parses as[('Alice', '[Bob]: Only Bob')], incorrectly capturing the next entry
The non-greedy .+? combined with the lookahead (?=\[|$) causes premature termination whenever dialogue text contains a literal bracket or when dialogue is empty.
Multi-line dialogue without brackets is handled correctly, but edge cases with brackets or empty entries will produce incorrect results.
| from reddit_shorts.config import characters | ||
|
|
||
| # Create a mapping of character names to their voices and info | ||
| char_map = {char['name']: char for char in characters} |
There was a problem hiding this comment.
Remove unused variable char_map.
The variable char_map is assigned but never used. The character lookup on lines 182-185 iterates over characters directly instead of using this mapping.
Suggested fix
from reddit_shorts.config import characters
- # Create a mapping of character names to their voices and info
- char_map = {char['name']: char for char in characters}
-
for i, dialogue_segment in enumerate(dialogue):📝 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.
| char_map = {char['name']: char for char in characters} | |
| from reddit_shorts.config import characters | |
| for i, dialogue_segment in enumerate(dialogue): |
🧰 Tools
🪛 Ruff (0.14.10)
174-174: Local variable char_map is assigned to but never used
Remove assignment to unused variable char_map
(F841)
🤖 Prompt for AI Agents
In reddit_shorts/make_tts.py around line 174, the variable `char_map =
{char['name']: char for char in characters}` is created but never used; either
remove this unused assignment or refactor the character lookup at lines 182-185
to use `char_map` (e.g., look up by name instead of iterating), and ensure any
references to the old iteration are updated accordingly so there are no unused
variables left.
| try: | ||
| char_voice = Voice(char_config['voice']) | ||
| except: | ||
| print(f"Warning: Invalid voice '{char_config['voice']}' for character '{char_name}'. Using default.") | ||
| char_voice = active_voice_enum |
There was a problem hiding this comment.
Avoid bare except clause - catch specific exceptions.
Using a bare except: catches all exceptions including KeyboardInterrupt and SystemExit, which can mask bugs and make debugging difficult. The Voice enum raises ValueError for invalid values.
Suggested fix
if not char_config:
print(f"Warning: Character '{char_name}' not found in character configs. Using default voice.")
char_voice = active_voice_enum
else:
try:
char_voice = Voice(char_config['voice'])
- except:
+ except (ValueError, KeyError):
print(f"Warning: Invalid voice '{char_config['voice']}' for character '{char_name}'. Using default.")
char_voice = active_voice_enum🧰 Tools
🪛 Ruff (0.14.10)
193-193: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In reddit_shorts/make_tts.py around lines 191-195, the code uses a bare except
when constructing Voice(char_config['voice']); replace it with a specific
exception handler (e.g., except ValueError as e:) so only invalid enum values
are caught (optionally also handle KeyError if 'voice' may be missing). Capture
the exception object and include it in the warning message, then fall back to
active_voice_enum as before.
| I’m just a solo creative, but a big brand posted they were looking for a “design agency with a proven ad portfolio.” I had none. But I had time, ambition… and a secret weapon. | ||
| Title: Sample Dialogue Story |
There was a problem hiding this comment.
Missing blank line separator between stories may cause parsing issues.
According to the README documentation (line 159), stories should be "Separate[d] multiple stories with at least one blank line." The transition from the first story directly to Title: Sample Dialogue Story without a blank line could cause the parser in parse_stories_from_file to incorrectly append lines 4-9 to the first story's body.
Suggested fix
I'm just a solo creative, but a big brand posted they were looking for a "design agency with a proven ad portfolio." I had none. But I had time, ambition… and a secret weapon.
+
Title: Sample Dialogue Story🤖 Prompt for AI Agents
In stories.txt around lines 3 to 4, the first story and the next story header
run together without the required blank-line separator, which can cause
parse_stories_from_file to append the subsequent Title and lines to the first
story; fix by inserting a single blank line between the end of the first story
and the "Title: Sample Dialogue Story" header (and re-save the file), then run
the story parser/unit tests to confirm the stories are split correctly.
| @main_bp.route('/api/character-image/<character_name>', methods=['GET']) | ||
| def get_character_image(character_name): | ||
| """Serve character images""" | ||
| from reddit_shorts.config import character_resources_path | ||
| char_dir = os.path.join(character_resources_path, character_name) | ||
|
|
||
| if os.path.exists(char_dir): | ||
| for file in os.listdir(char_dir): | ||
| if file.lower().endswith(('.png', '.jpg', '.jpeg')): | ||
| image_path = os.path.join(char_dir, file) | ||
| return send_file(image_path, mimetype=f'image/{file.split(".")[-1].lower()}') | ||
|
|
||
| return jsonify({"error": "Character image not found"}), 404 |
There was a problem hiding this comment.
Path traversal vulnerability - validate character_name parameter.
The character_name parameter is used directly in path construction without validation, allowing potential directory traversal attacks (e.g., ../../etc/passwd). Additionally, iterating all files to find the first image is inefficient.
🔎 Proposed fix with validation and efficiency improvement
@main_bp.route('/api/character-image/<character_name>', methods=['GET'])
def get_character_image(character_name):
"""Serve character images"""
from reddit_shorts.config import character_resources_path
+
+ # Validate character_name to prevent path traversal
+ if not character_name or '/' in character_name or '\\' in character_name or '..' in character_name:
+ return jsonify({"error": "Invalid character name"}), 400
+
char_dir = os.path.join(character_resources_path, character_name)
- if os.path.exists(char_dir):
- for file in os.listdir(char_dir):
- if file.lower().endswith(('.png', '.jpg', '.jpeg')):
- image_path = os.path.join(char_dir, file)
- return send_file(image_path, mimetype=f'image/{file.split(".")[-1].lower()}')
+ if os.path.exists(char_dir) and os.path.isdir(char_dir):
+ # Use glob for efficiency
+ import glob
+ for ext in ['*.png', '*.jpg', '*.jpeg']:
+ matches = glob.glob(os.path.join(char_dir, ext), flags=glob.IGNORECASE)
+ if matches:
+ return send_file(matches[0], mimetype=f'image/{os.path.splitext(matches[0])[1][1:].lower()}')
return jsonify({"error": "Character image not found"}), 404🤖 Prompt for AI Agents
In web_ui/routes.py around lines 188 to 200, the handler uses character_name
directly in os.path.join and lists the directory which permits path traversal
and is inefficient; validate and sanitize character_name (e.g. allow only a
whitelist of safe characters like alphanumerics, hyphen/underscore, or reject on
disallowed chars), then build a secure path and verify the resolved path is
inside character_resources_path (use realpath/abspath and compare prefixes)
before accessing the filesystem; to find an image, avoid naive os.listdir
scanning by searching for the first matching extension via a glob-like pattern
restricted to the safe directory (or check specific expected filenames) and
confirm the found path is a file, then determine the mimetype safely (use
mimetypes.guess_type) and return it; on any validation failure or missing file
return the 404 JSON.
aight man thanks, from my testing there is a slight delay with the audio and the subtitles but the functionality is there, inside the characters folder are subfolders with the character's name and each characters subfolder should have 
the actual image file and a config.json file
and the contents of the file should be somewhat along the sides of { |
2 participants
Uhh so I added some features like Character Overlay . multiple character/voice support using AI spaghetti code and it works for some reason , wanted to try my luck test with a pull request, try it out ig? if you want to else decline
Summary by CodeRabbit
New Features
[Character]: dialogue textDocumentation
✏️ Tip: You can customize this high-level summary in your review settings.