Fork value-add: markdown TTS normalization + ReDoS hardening + roadmap#4
Fork value-add: markdown TTS normalization + ReDoS hardening + roadmap#4Fieldnote-Echo wants to merge 17 commits into
Conversation
Add handle_markdown() pre-processing pass that strips markdown formatting (headings, bold, italic, links, code blocks, tables, etc.) before TTS synthesis. Preserves code block contents literally and avoids false positives on snake_case identifiers. - New: handle_markdown() function in normalizer.py - New: markdown_normalization toggle in NormalizationOptions (default True) - New: test_normalizer.py with 24 test cases including mixed LLM output Rebase note (v0.6.0): the original commit also stripped trailing whitespace from normalize_text() output; that was dropped because upstream's smart_split() now rejoins custom-phoneme segments with "".join() and relies on boundary whitespace being preserved (test_smart_custom_phenomes / test_smart_split_only_phenomes). Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Re-implementation of the fork's base-image digest pin against upstream's
multi-stage docker/gpu/Dockerfile.optimized + docker-bake.hcl layout
(the old single-stage docker/gpu/Dockerfile was removed upstream).
- Dockerfile.optimized gains CUDA_BUILDER_IMAGE / CUDA_RUNTIME_IMAGE args
defaulting to the unpinned ${CUDA_VERSION} tags, so plain docker/compose
builds keep working.
- docker-bake.hcl overrides them with tag@sha256 index digests for
gpu-amd64 (12.6.3), gpu-arm64 (12.9.1), and gpu-cu128-amd64 (12.8.1),
preventing supply-chain substitution of the mutable version tags.
Digests resolved 2026-07-16 via docker buildx imagetools inspect.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace upstream's multi-arch bake release workflow with a GPU-only, amd64-only build that includes: - Pinned action SHAs (all 11 actions) - step-security/harden-runner (audit mode) - Sigstore build provenance attestation - Trivy CRITICAL+HIGH container scan → GitHub Security tab - CycloneDX SBOM attached to GitHub Release Triggers on tag push (v*) or manual workflow_dispatch. Publishes to ghcr.io/fieldnote-echo/kokoro-fastapi-gpu. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Weekly grouped updates for CI action SHAs only. No pip ecosystem — upstream manages Python deps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Document what Fieldnote-Echo fork adds vs remsky/Kokoro-FastAPI: markdown normalization, supply-chain hardened CI, GHCR images. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move api/src/services/text_processing/test_normalizer.py to api/tests/test_markdown_normalizer.py so pytest's testpaths collects it, and replace the import shim with a direct package import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_MD_BOLD/_MD_ITALIC_STAR/_MD_ITALIC_UNDER/_MD_STRIKETHROUGH used .+? without newline handling, so '**bold\ntext**' passed through unstripped. Emphasis content now matches across single newlines (soft wraps) while refusing to cross blank lines (paragraph breaks), keeping over-matching across paragraphs impossible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Streaming/truncated chunks can open a ``` fence that never closes; the old pattern required a closing fence, leaving literal backticks that got voiced. The fence must now start at a line beginning, and an unclosed fence swallows to end of input: the fence line (with its language info string) is stripped and the content is kept protected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_MD_LINK/_MD_IMAGE stopped at the first ')', so links to URLs like wiki/Foo_(bar) left a stray ')' in the voiced text. The URL part now accepts one nesting level of balanced parentheses, which covers real-world URLs while still stopping at the true closing paren. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
smart_split splits on CUSTOM_PHONEMES and rejoins with ''.join, so a
normalizer that strips segment edges glues words to phoneme tokens
('Say[Kokoro](...)please'). Restore any edge whitespace the normalizer
eats and strip once on the final joined result only. Note: the rebase
onto upstream already dropped normalize_text's trailing .strip(), so
this did not reproduce here — this hardens the join and adds the
phoneme-spacing regression tests so it cannot come back.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
smart_split only ran normalize_text for lang codes a/b/en-us/en-gb, so markdown_normalization was silently ignored for every other language. Markdown syntax is language-independent: when the toggle is enabled, handle_markdown() now runs for non-English languages too (with custom phoneme tokens protected), while the rest of the English-specific normalization stays gated as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- __bold__ double-underscore emphasis is now stripped (word-boundary
aware, like the underscore italic)
- ATX-closed headings drop the trailing hash sequence ('## H ##' -> 'H')
while '#' glued to a word survives ('# Learning C#' -> 'Learning C#')
- list markers are stripped before headings so '- # H' voices 'H';
heading regex also tolerates indentation
- empty-link-text '[](url)' drops entirely instead of surviving verbatim
- NUL bytes are removed from input up front, making the \x00-delimited
code placeholders collision-proof against placeholder-looking input
- table rows still need 2+ pipes but must also be pipe-anchored or sit
next to a separator row, so prose like 'a | b | c' keeps its pipes
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aped pipes Verifier findings on the md normalization pass: - CRLF input defeated every MULTILINE anchor: closing-hash headings leaked '##' into speech and CRLF blank lines stopped acting as paragraph breaks for emphasis. Normalize \r\n/\r to \n up front. - Emphasis patterns scanned O(n^2) on floods of unclosed markers (140KB of '__word ' took 43s inside async smart_split — an event-loop-blocking single-request DoS). Bound emphasis spans at 600 chars: O(n·600), same 140KB now 0.34s. - Fences inside blockquotes were garbled by the inline-code pass; recognize '> ```' openers and dedent the quoted content. - Escaped pipes (\|) were counted as table syntax; treat as literal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-2 verifier profiling found three more super-linear vectors on the
same unbounded request path as the emphasis fix:
- _MD_TABLE_SEP_ROW: '-{3,}' and '[\s:|-]*' both match '-', giving
catastrophic backtracking on long dash lines (80KB line took 43s).
Replaced with an ambiguity-free character-class fullmatch plus a
'---' substring check.
- _MD_LINK/_MD_IMAGE: unbounded '[^\]]*' link text scanned to
end-of-string from every '[' in a bracket flood (400KB took >30s).
Bounded link text at 800 chars and the URL part at 2000.
- Phase-3 placeholder restore looped str.replace per placeholder,
O(placeholders x len): fence/backtick floods were quadratic even
with fast regexes. Replaced with a single indexed-callback re.sub.
All vectors now complete in <0.25s at 400-800KB. Adds flood and
scaling-ratio regression tests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…m handler
Final adversarial-hardening pass found three more O(n^2) vectors on the
default normalize_text path, all reachable with tiny inputs:
- _MD_UNORDERED_LIST/_MD_ORDERED_LIST: '^(\s*)' under MULTILINE let the
line anchor consume every following blank line from each line start
(100KB of newlines -> 65s; the CRLF fix feeds this). '\s' -> '[ \t]'.
- URL_PATTERN / EMAIL_PATTERN: unbounded domain/local runs rescanned the
whole tail from every position of an 'a.a.a...' flood. Added a
token-start negative lookbehind to URL and bounded both to RFC lengths.
- Dotted-acronym handler '(?:[A-Za-z]\.){2,} [a-z]': unbounded '{2,}'
backtracked O(n) per position on 'a.a...' floods. Bounded to {2,12}
(real acronyms are short); 'U.S.A. b' handling preserved.
All vectors now <0.25s at 400KB (were 12-65s). Adds newline/CRLF/word-run
flood regression tests. URL_PATTERN and the acronym handler are pre-existing
upstream code (v0.2.4); worth upstreaming separately.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
State the fork's direction beyond security hardening: correct, open, tunable narration (pronunciation/prosody control for e-learning TTS). - ROADMAP.md: milestones (eval harness next), good-faith upstreaming model - NORTH_STAR.md: the distilled goal and what it explicitly is not - NARRATION_STRATEGY.md: full landscape research + market niche + primitives - README: fork notice now leads with the goal and links the roadmap Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by QodoMarkdown-aware TTS normalization with ReDoS-safe regexes and hardened GPU releases
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
There was a problem hiding this comment.
Code Review
This pull request introduces markdown-aware text normalization to strip formatting syntax (such as bold, italic, headings, and tables) before text-to-speech synthesis, along with ReDoS hardening for existing regex patterns, supply-chain security improvements pinning CUDA base images by digest, and comprehensive unit tests. Feedback on the changes highlights a high-severity ReDoS vulnerability in the URL regex pattern due to an empty alternative group, and points out an edge case in the table normalization logic where certain valid Markdown tables without outer pipes may fail to have their pipes stripped correctly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # run (O(n^2) — a 100KB run of 'a' hung the normalizer). The domain run | ||
| # is also bounded at 253 chars, the DNS name length limit. | ||
| r"(?<![a-zA-Z0-9.-])" | ||
| r"(https?://|www\.|)+(localhost|[a-zA-Z0-9.-]{1,253}(\.(?:" |
There was a problem hiding this comment.
The regex pattern (https?://|www\.|)+ contains a + quantifier applied to a group with an empty alternative (due to the trailing | inside the group). This introduces a risk of catastrophic backtracking (ReDoS) and is highly inefficient. Since the goal is to optionally match https:// and/or www., this can be simplified to a completely deterministic and safer pattern: (?:https?://)?(?:www\.)?.
| r"(https?://|www\.|)+(localhost|[a-zA-Z0-9.-]{1,253}(\.(?:" | |
| r"(?:https?://)?(?:www\\.)?(localhost|[a-zA-Z0-9.-]{1,253}(\\.(?:" |
| lines = text.split("\n") | ||
| is_sep_row = [_is_table_sep_row(line) for line in lines] | ||
| for i, line in enumerate(lines): | ||
| if is_sep_row[i]: | ||
| lines[i] = "" | ||
| continue | ||
| if line.count("|") < 2: | ||
| continue | ||
| stripped = line.strip() | ||
| pipe_anchored = stripped.startswith("|") or stripped.endswith("|") | ||
| near_sep_row = (i > 0 and is_sep_row[i - 1]) or ( | ||
| i + 1 < len(lines) and is_sep_row[i + 1] | ||
| ) | ||
| if pipe_anchored or near_sep_row: | ||
| lines[i] = line.replace("|", " ") | ||
| text = "\n".join(lines).replace("\x00EP\x00", "|") |
There was a problem hiding this comment.
The current table normalization logic only replaces pipes with spaces for rows that are directly adjacent to the separator row or are pipe-anchored (start/end with |). For multi-row tables without outer pipes (which is valid Markdown), rows beyond the first one below the separator row will not have their pipes replaced, leaving raw | characters in the output. Additionally, 2-column tables without outer pipes (which only have 1 pipe) are skipped entirely due to the line.count("|") < 2 check.
We can resolve this robustly by propagating the table status to all contiguous lines containing pipes starting from the separator row.
lines = text.split("\\n")
is_sep_row = [_is_table_sep_row(line) for line in lines]
is_table_row = [False] * len(lines)
for i, sep in enumerate(is_sep_row):
if sep:
is_table_row[i] = True
# Propagate table status to contiguous lines containing pipes
j = i - 1
while j >= 0 and "|" in lines[j]:
is_table_row[j] = True
j -= 1
j = i + 1
while j < len(lines) and "|" in lines[j]:
is_table_row[j] = True
j += 1
for i, line in enumerate(lines):
if is_sep_row[i]:
lines[i] = ""
elif is_table_row[i]:
lines[i] = line.replace("|", " ")
text = "\\n".join(lines).replace("\\x00EP\\x00", "|")
Code Review by Qodo
1. Markdown toggle blocked by normalize
|
| if lang_code in ["a", "b", "en-us", "en-gb"]: | ||
| processed_text = CUSTOM_PHONEMES.split(processed_text) | ||
| for index in range(0, len(processed_text), 2): | ||
| processed_text[index] = normalize_text( | ||
| processed_text[index], normalization_options | ||
| segment = processed_text[index] | ||
| normalized = normalize_text(segment, normalization_options) | ||
| # Segments are rejoined with ''.join below, so any edge | ||
| # whitespace the normalizer eats would glue words onto | ||
| # the adjacent custom phoneme tokens — restore it. | ||
| if segment[:1].isspace() and not normalized[:1].isspace(): | ||
| normalized = " " + normalized | ||
| if segment[-1:].isspace() and not normalized[-1:].isspace(): | ||
| normalized = normalized + " " | ||
| processed_text[index] = normalized | ||
|
|
||
| # Strip once on the final joined result, never per segment | ||
| processed_text = "".join(processed_text).strip() | ||
| elif normalization_options.markdown_normalization: | ||
| # Markdown syntax is language-independent — strip it even | ||
| # when English-specific normalization is skipped. Custom | ||
| # phoneme tokens look like markdown links, so protect them. | ||
| processed_text = CUSTOM_PHONEMES.split(processed_text) | ||
| for index in range(0, len(processed_text), 2): | ||
| processed_text[index] = handle_markdown( | ||
| processed_text[index] | ||
| ) | ||
|
|
||
| processed_text = "".join(processed_text).strip() | ||
| logger.info( | ||
| "Applied markdown normalization only; full text " | ||
| "normalization is only supported for english" | ||
| ) | ||
| else: |
There was a problem hiding this comment.
2. Markdown toggle blocked by normalize 🐞 Bug ≡ Correctness
smart_split() only runs markdown stripping when NormalizationOptions.normalize is true, so requests cannot disable the rest of normalization while still enabling markdown_normalization. This makes markdown_normalization unusable as an independent control knob despite being modeled as a separate option in NormalizationOptions.
Agent Prompt
### Issue description
`markdown_normalization` is a separate flag in `NormalizationOptions`, but `smart_split()` gates the entire normalization block behind `normalization_options.normalize`. As a result, a caller cannot request “strip markdown only” by setting `normalize=False, markdown_normalization=True`.
### Issue Context
- `NormalizationOptions` introduces `markdown_normalization` as its own toggle.
- `normalize_text()` already implements markdown stripping behind `markdown_normalization`.
- `smart_split()` controls whether `normalize_text()` / `handle_markdown()` is invoked at all.
### Fix Focus Areas
- api/src/services/text_processing/text_processor.py[172-207]
- api/src/structures/schemas.py[49-61]
### Suggested fix approach
- Change the gating condition to allow markdown-only normalization, e.g.:
- Enter the normalization block when `normalization_options.normalize` **or** `normalization_options.markdown_normalization` is true.
- Apply `handle_markdown()` (with CUSTOM_PHONEMES protection) when `markdown_normalization` is true.
- Apply the rest of `normalize_text()` only when `normalize` is true.
- Alternatively (if the intended semantics are “normalize=false disables everything”), clarify this explicitly in schema field descriptions / docs so the behavior is not surprising.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…iness Bot findings on the fork PRs (Gemini/Qodo): - URL_PATTERN prefix (https?://|www\.|)+ has a '+' over a group with an empty alternative — a ReDoS smell and non-deterministic. Rewrote as two optionals (?:https?://)?(?:www\.)? — deterministic, same matches. handle_url uses only group(0), so the capturing->non-capturing shift is safe. - Table de-piping missed rows beyond the first in borderless multi-row tables and skipped 2-column borderless tables (1 pipe, failed the count>=2 gate). Replaced the anchored/adjacent heuristic with propagation of table status outward from each separator row; lone prose pipes stay intact. - Perf flood tests asserted <2.0s wall-clock; raised to <8.0s so they still catch the 12-65s O(n^2) blowups but don't flake on loaded CI runners. Adds borderless-table regression tests. 78 md/normalizer tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 043098aa2d
ℹ️ 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".
| for i, sep in enumerate(is_sep_row): | ||
| if not sep: | ||
| continue | ||
| j = i - 1 | ||
| while j >= 0 and "|" in lines[j]: |
There was a problem hiding this comment.
Avoid rescanning the table for every separator
When a request contains many contiguous separator rows, each separator scans the entire surrounding pipe-bearing region in both directions, making this loop quadratic. For example, 8,000 |---|---| rows (roughly 80 KB) take about 9.6 seconds here; because handle_markdown() runs synchronously inside smart_split(), one such request blocks the async event loop. Propagate table membership in linear passes instead of walking outward from every separator.
Useful? React with 👍 / 👎.
| _MD_FENCED_CODE = re.compile( | ||
| r"^[ \t]*(>[ \t]*)?```[^\n]*\n?(.*?)(?:```|\Z)", re.DOTALL | re.MULTILINE | ||
| ) |
There was a problem hiding this comment.
Require closing code fences to start on a line
When fenced code contains a mid-line triple-backtick sequence, this pattern treats it as the closing fence even though Markdown closing fences must be line-oriented. The actual closing fence is then parsed as a new unclosed opener, causing later text to be protected as code; for example, **bold** after such a block remains unnormalized and its markers reach TTS. Constrain the closing alternative to a valid fence line.
Useful? React with 👍 / 👎.
| _MD_UNORDERED_LIST = re.compile(r"^([ \t]*)[-*+][ \t]+", re.MULTILINE) | ||
| _MD_ORDERED_LIST = re.compile(r"^([ \t]*)\d+\.[ \t]+", re.MULTILINE) |
There was a problem hiding this comment.
Consume checklist and parenthesized list markers
For common Markdown list forms, these patterns remove only - , * , + , or a numeric marker ending in a period. Consequently, GFM task items such as - [x] ship leave [x] ship in the synthesized text, while ordered items such as 1) step retain the marker entirely. Consume checkbox tokens and the parenthesized ordered-list form so list syntax does not reach the phonemizer.
Useful? React with 👍 / 👎.
| _MD_HEADING = re.compile( | ||
| r"^[ \t]*#{1,6}[ \t]+(.*?)(?:[ \t]+#+)?[ \t]*$", re.MULTILINE | ||
| ) |
There was a problem hiding this comment.
Strip Setext heading underlines
When input uses a Setext level-one heading such as Heading\n===, this ATX-only handling leaves the underline untouched. With the default replace_remaining_symbols option, every = is subsequently expanded to equals, so the generated speech includes a run of unwanted “equals” words. Recognize Setext underlines as heading syntax before general symbol replacement.
Useful? React with 👍 / 👎.
| _MD_HEADING = re.compile( | ||
| r"^[ \t]*#{1,6}[ \t]+(.*?)(?:[ \t]+#+)?[ \t]*$", re.MULTILINE | ||
| ) | ||
| _MD_BLOCKQUOTE = re.compile(r"^>\s?", re.MULTILINE) |
There was a problem hiding this comment.
Strip every marker from nested blockquotes
For nested blockquotes such as >> quoted text or > > quoted text, this pattern removes only the first > marker. The remaining marker is not handled by SYMBOL_REPLACEMENTS, so it reaches sentence processing and the phonemizer despite Markdown normalization being enabled. Consume the complete sequence of blockquote markers on each line.
Useful? React with 👍 / 👎.
The fork's value-add on top of upstream v0.6.0. Stacked on #3 (base is
sync/upstream-v0.6.0), so this diff shows only the 16 fork commits, not the upstream sync.What this fork does (beyond security hardening)
Goal: correct, open, tunable narration — pronunciation/prosody control for e-learning TTS. See
ROADMAP.md/NORTH_STAR.md. This PR lands the foundation:Markdown-aware TTS normalization
handle_markdown()+markdown_normalizationtoggle (default on): strips**bold**,[links](url), headings, fenced/inline code, tables, blockquotes, etc. so raw LLM output narrates cleanly instead of speaking the syntax. Applies across all languages. Tests relocated intoapi/tests/(the original 224-line suite was never collected by CI) and expanded to 62 cases.ReDoS hardening (found while hardening the above)
Five quadratic/catastrophic-backtracking vectors on the synchronous request path, each of which let a single request pin the async event loop for 10–65s:
All now < 0.25s at 400KB. Covered by flood + linear-scaling regression tests.
Supply-chain hardening
CUDA base images pinned by digest in
docker-bake.hcl, GitHub Actions SHA-pinned, Sigstore provenance, Trivy scan, CycloneDX SBOM, Dependabot.Direction docs
ROADMAP.md,NORTH_STAR.md,NARRATION_STRATEGY.md, and an updated fork notice.Verification
164 passed, 12 deselected(api/tests, no-cov). Rebase is additive-only over upstream (merge-base = upstream HEAD; no upstream code clobbered). Deploy-risk swept against rippling-lms — unaffected.Review notes
.strip()phoneme-gluing regression the audit expected did not reproduce — it self-resolved in the rebase (upstream returns unstripped); guards kept anyway.🤖 Generated with Claude Code