fix(llm): use non-greedy quantifier when stripping bracket and paren groups from script - #1270
Conversation
…groups from script format_response used re.sub(r"\[.*\]", ...) and re.sub(r"\(.*\)", ...). The greedy .* matches from the first opener to the *last* closer on the same line, silently deleting all intervening content. Examples of silent data loss: "[Intro] Great content [end]" → "." "Save (at least) 10% (monthly)." → "Save ." Replacing .* with .*? makes each substitution non-greedy so that every bracket/paren group is removed individually, leaving surrounding words intact. Add a unit test that feeds a two-group input through generate_script and asserts the text between groups is preserved.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e1add343d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # group is removed independently; the greedy form would eat all text | ||
| # between the first opener and the last closer on the same line. | ||
| response = re.sub(r"\[.*?\]", "", response) | ||
| response = re.sub(r"\(.*?\)", "", response) |
There was a problem hiding this comment.
When the LLM emits a parenthetical group that itself contains parentheses, such as a Markdown URL like [source](https://example.com/Foo_(bar)), this non-greedy pattern stops at the inner ) and leaves a dangling ) in the returned script. The previous greedy pattern removed that whole group, so this change regresses those nested-parenthesis cases while still feeding the sanitized script to downstream narration/subtitle generation.
Useful? React with 👍 / 👎.
|
Thanks for catching this silent script truncation issue and adding a focused regression test. I verified the fix against the latest main, including several bracket and parenthesis cases, and the full test suite passes. Merged! |
2 participants
Summary
format_responseingenerate_scriptusedre.sub(r"\[.*\]", ...)andre.sub(r"\(.*\)", ...). The greedy.*matches from the first openerto the last closer on the same line, deleting all text in between.
"[Intro] Great content [end]"→".""Save (at least) 10% of income (monthly)."→"Save .".*with.*?so each bracket/paren group is removedindependently, leaving surrounding words intact.
Test plan
test_generate_script_strips_each_bracket_group_independentlypassestest_generate_script_sends_custom_prompt_to_llmpasses