Repository navigation
fix(voice): keep link text when the url has parentheses - #8121
Open
ANDROIDHASSAN wants to merge 3 commits into
Open
ANDROIDHASSAN wants to merge 3 commits into
ANDROIDHASSAN wants to merge 3 commits into
Conversation
The link and image patterns stopped the destination at the first ')', so [Python](https://en.wikipedia.org/wiki/Python_(language)) reached TTS as 'Python)'. Allow one level of balanced parentheses in the destination, as CommonMark does, in both the strip patterns and the pending-link check.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The balanced-paren destination read \( as opening a group, so [guide](a\(b) was no longer stripped and the pending check held it to the end of the stream. Match \. as one url char, as CommonMark does.
…arens # Conflicts: # tests/test_transcription_filter.py
This branch has not been deployed
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
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.
Problem
filter_markdowncuts a link or image destination at its first). LLMs often cite Wikipedia URLs like.../Python_(language), soreaches TTS as
read Python) first. For.png) hereit givesthe logo.png) here. When the alt or link text contains a space, the pending-link check has the same problem: it treats the link as finished after the inner).Fix
The destination may now contain one level of balanced parentheses, as CommonMark allows. A backslash-escaped paren (
\(,\)) is read as one destination character and opens no group, so[guide](https://example.com/a\(b)is still stripped toguide, as onmain. The change is in the image and link patterns and in_PENDING_LINK. The alternatives (\\.,[^()\\]and\() never overlap, so the patterns still run in linear time. 20k-character inputs of(,\,\(or plain text after[a](finish in under 3 ms.Tests
test_link_text_keptruns a plain link, a Wikipedia-style link, an image with multi-word alt text, and a link and an image whose destination holds an escaped\(. Each case uses the suite's_outputshelper (from #8510), so every chunk size must give exactly the expected text.main: both paren cases fail, e.g.{'the logo.png) here'} != {'the logo here'}and{'read Python) first'} != {'read Python first'}.mainand guard against reading\(as a group (they failed on this PR's first commit)._PENDING_LINKreverted: the image case fails (.png) herepasses through unchanged at chunk size 1).main:uv run pytest --unit --audio_eot -q: 6235 passed, 4 skipped.ruff format,ruff checkandmake type-checkare clean.Scope
[a](b(c)is no longer stripped toa. It is left as is, which matches CommonMark: it is not a link there either.