Skip to content

Fork value-add: markdown TTS normalization + ReDoS hardening + roadmap#4

Open
Fieldnote-Echo wants to merge 17 commits into
sync/upstream-v0.6.0from
update/upstream-sync
Open

Fork value-add: markdown TTS normalization + ReDoS hardening + roadmap#4
Fieldnote-Echo wants to merge 17 commits into
sync/upstream-v0.6.0from
update/upstream-sync

Conversation

@Fieldnote-Echo

Copy link
Copy Markdown
Owner

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_normalization toggle (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 into api/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:

  • markdown: emphasis spans, link/image brackets, table-separator rows, placeholder-restore loop, list markers
  • upstream patterns (URL/email/dotted-acronym) — isolated and PR'd separately to upstream as remsky#489

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

  • Commits are logically ordered (feature → md fixes → docs) and each is self-contained; reviewable commit-by-commit.
  • The .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

Fieldnote-Echo and others added 16 commits July 18, 2026 09:43
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>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Markdown-aware TTS normalization with ReDoS-safe regexes and hardened GPU releases

✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add markdown-stripping pre-pass for cleaner TTS narration with a default-on toggle
• Harden normalizer regexes against ReDoS on large/pathological inputs
• Ship GPU-only hardened release pipeline (pinned actions/digests, provenance, SBOM, Trivy) plus
 roadmap docs
Diagram

graph TD
A["TTS request"] --> B["smart_split()"] --> C["handle_markdown()"] --> D["normalize_text()"] --> E["phonemize()/synthesis"]
F[".github/workflows/release.yml"] --> G["docker/gpu/Dockerfile.optimized"] --> H{{"GHCR GPU image"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a CommonMark parser instead of regex-based stripping
  • ➕ Much closer to spec-correct markdown handling (nested constructs, edge cases)
  • ➕ Lower risk of regex correctness drift as markdown patterns expand
  • ➖ Adds a dependency and additional runtime overhead on the request path
  • ➖ Harder to guarantee strict linear-time behavior without careful configuration
2. Implement markdown normalization as a single-pass scanner (no regex)
  • ➕ Easier to reason about worst-case complexity (linear by construction)
  • ➕ Avoids subtle regex backtracking and placeholder restore pitfalls
  • ➖ More code to maintain; higher initial implementation complexity
  • ➖ Can still be tricky to get “good enough” markdown heuristics without regressions
3. Apply markdown stripping only at the boundary (e.g., API layer) and keep normalizer upstream-like
  • ➕ Keeps upstream-derived normalizer simpler and easier to rebase
  • ➕ Markdown handling becomes a clearly separated concern
  • ➖ Requires ensuring all entry points consistently invoke the boundary step
  • ➖ Non-English/custom-phoneme token interactions still need careful handling

Recommendation: The PR’s approach (bounded regexes + explicit placeholder protection + regression/perf tests) is a pragmatic choice for a latency-sensitive synchronous path, especially given the stated ReDoS findings. If markdown scope expands further, consider graduating to either a CommonMark parser (if performance budget allows) or a purpose-built linear scanner; for now, the tight bounds and scaling tests materially reduce operational risk.

Files changed (12) +1233 / -270

Enhancement (2) +200 / -5
normalizer.pyAdd markdown stripping + ReDoS-harden URL/email/acronym handling +189/-4

Add markdown stripping + ReDoS-harden URL/email/acronym handling

• Adds handle_markdown() as a pre-pass (behind a default-on markdown_normalization option) to strip markdown syntax while preserving code spans/blocks. Hardens multiple regexes and loops against quadratic behavior by adding bounds, safer anchors/lookbehinds, and replacing quadratic restore logic with placeholder-index substitution.

api/src/services/text_processing/normalizer.py

schemas.pyAdd markdown_normalization option to NormalizationOptions +11/-1

Add markdown_normalization option to NormalizationOptions

• Extends the public schema with a markdown_normalization boolean (default True) to toggle markdown stripping behavior. Adds module docstring noting upstream delta for easier rebases.

api/src/structures/schemas.py

Bug fix (1) +26 / -3
text_processor.pyPreserve whitespace around custom phoneme tokens and enable markdown-only path +26/-3

Preserve whitespace around custom phoneme tokens and enable markdown-only path

• Adjusts smart_split normalization so segment-level normalization cannot eat edge whitespace and glue custom phoneme tokens to adjacent words. Adds a markdown-only normalization path for non-English languages while protecting custom phoneme tokens from being mistaken as markdown links.

api/src/services/text_processing/text_processor.py

Tests (1) +536 / -0
test_markdown_normalizer.pyAdd comprehensive markdown normalizer + ReDoS regression test suite +536/-0

Add comprehensive markdown normalizer + ReDoS regression test suite

• Adds extensive unit/integration tests for handle_markdown() and smart_split interactions, covering common markdown constructs, CRLF handling, placeholder safety, and custom phoneme preservation. Includes performance regression tests to ensure near-linear behavior on adversarial floods and large inputs.

api/tests/test_markdown_normalizer.py

Documentation (4) +328 / -0
NARRATION_STRATEGY.mdAdd narration strategy decision brief and research synthesis +158/-0

Add narration strategy decision brief and research synthesis

• Adds a detailed strategy document explaining the fork’s narration focus, taxonomy (SVS/SVC/STS), and practical constraints/lessons. Establishes rationale for a correctness-first pronunciation/prosody layer and outlines recommended next steps.

NARRATION_STRATEGY.md

NORTH_STAR.mdDefine fork north star and upstream contribution boundaries +98/-0

Define fork north star and upstream contribution boundaries

• Documents the fork’s guiding objective (correct, open, tunable narration) and clarifies what will be upstreamed versus kept product-specific. Summarizes planned pipeline stages and constraints to avoid relearning known limitations.

NORTH_STAR.md

README.mdAdd fork notice and summarize shipped capabilities +16/-0

Add fork notice and summarize shipped capabilities

• Prepends a fork banner describing goals and current shipped features (markdown normalization, ReDoS hardening, supply-chain hardened release images). Links to roadmap/north-star docs and notes intent to upstream generally useful fixes.

README.md

ROADMAP.mdAdd fork roadmap with milestones and rationale +56/-0

Add fork roadmap with milestones and rationale

• Introduces a milestone-based roadmap emphasizing evaluation harness first, then deterministic normalization/lexicon stages. Records constraints and upstreaming philosophy, and marks completed items (supply-chain hardening, ReDoS, markdown normalization).

ROADMAP.md

Other (4) +143 / -262
dependabot.ymlAdd Dependabot updates for GitHub Actions +13/-0

Add Dependabot updates for GitHub Actions

• Introduces a Dependabot configuration that runs weekly and groups GitHub Actions updates. Explicitly avoids Python dependency automation since upstream handles those via sync.

.github/dependabot.yml

release.ymlReplace release workflow with hardened GPU-only GHCR publishing +112/-260

Replace release workflow with hardened GPU-only GHCR publishing

• Reworks the release pipeline to build/push an amd64 GPU image with SHA-pinned actions, runner hardening, provenance attestation, Trivy scanning (SARIF upload), and CycloneDX SBOM attached to the GitHub release. Simplifies triggers to tag pushes (v*) and manual dispatch, and adds a guard against duplicate releases.

.github/workflows/release.yml

docker-bake.hclPin CUDA base images by digest for GPU bake targets +10/-0

Pin CUDA base images by digest for GPU bake targets

• Adds digest-pinned CUDA builder/runtime image references for gpu-amd64, gpu-arm64, and gpu-cu128-amd64 targets to prevent mutable-tag substitution. Keeps default Dockerfile behavior unpinned for local builds while release/bake overrides use digests.

docker-bake.hcl

Dockerfile.optimizedParameterize CUDA base images to support digest pinning +8/-2

Parameterize CUDA base images to support digest pinning

• Introduces CUDA_BUILDER_IMAGE and CUDA_RUNTIME_IMAGE args and uses them in FROM lines, enabling bake/release workflows to inject digest-pinned base images. Maintains tag-based defaults for regular docker build/compose usage.

docker/gpu/Dockerfile.optimized

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}(\.(?:"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

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\.)?.

Suggested change
r"(https?://|www\.|)+(localhost|[a-zA-Z0-9.-]{1,253}(\.(?:"
r"(?:https?://)?(?:www\\.)?(localhost|[a-zA-Z0-9.-]{1,253}(\\.(?:"

Comment on lines +335 to +350
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", "|")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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", "|")

@qodo-code-review

qodo-code-review Bot commented Jul 18, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Markdown toggle blocked by normalize 🐞 Bug ≡ Correctness
Description
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.
Code

api/src/services/text_processing/text_processor.py[R173-204]

                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:
Evidence
smart_split() only enters the normalization path when normalization_options.normalize is true,
while NormalizationOptions defines markdown_normalization as an independent field.

api/src/services/text_processing/text_processor.py[164-207]
api/src/structures/schemas.py[49-61]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### 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


2. Flaky perf timing tests ✓ Resolved 🐞 Bug ☼ Reliability
Description
The new markdown normalizer test suite asserts wall-clock runtimes (e.g., <2s for 100KB payloads)
and a scaling ratio bound, which can intermittently fail on slower or contended CI runners even when
the implementation is correct. This risks destabilizing CI and obscuring real regressions behind
noise.
Code

api/tests/test_markdown_normalizer.py[R440-536]

+def test_unclosed_emphasis_flood_is_fast():
+    """Repeated unclosed emphasis markers must not scan quadratically.
+
+    100KB of '*word ' previously took >10s (O(n^2)); bounded emphasis
+    spans make it linear. Generous ceiling to avoid CI flakiness.
+    """
+    import time
+
+    for marker in ("*word ", "_word ", "__word "):
+        payload = marker * (100_000 // len(marker))
+        start = time.monotonic()
+        handle_markdown(payload)
+        assert time.monotonic() - start < 2.0
+
+
+# ---------------------------------------------------------------------------
+# Blockquoted fences and escaped pipes (verifier minors)
+# ---------------------------------------------------------------------------
+
+
+def test_fenced_code_inside_blockquote():
+    """A fence opened inside a blockquote must not be garbled by the
+    inline-code pass pairing backticks across lines."""
+    result = handle_markdown("> ```\n> code line\n> ```")
+    assert "`" not in result
+    assert "code line" in result
+
+
+def test_escaped_pipes_not_table():
+    """Escaped pipes are literal content, not table syntax."""
+    result = handle_markdown("use a \\| b \\| c here")
+    assert result == "use a | b | c here"
+
+
+def test_adversarial_floods_are_fast():
+    """Every markdown pass must stay near-linear on pathological input.
+
+    Covers the verifier-found quadratic vectors beyond emphasis: long
+    dash lines (table separator backtracking), '[' floods (link/image
+    scan-to-EOS), fence-opener and backtick-pair floods (per-placeholder
+    str.replace restore loop).
+    """
+    import time
+
+    payloads = [
+        "-" * 40_000 + ".",          # table separator backtracking
+        "[word " * (100_000 // 6),   # unclosed-bracket link flood
+        "```\n" * (100_000 // 4),    # fence-opener flood -> placeholder restore
+        "`a`" * (100_000 // 3),      # inline-code flood -> placeholder restore
+    ]
+    for payload in payloads:
+        start = time.monotonic()
+        handle_markdown(payload)
+        assert time.monotonic() - start < 2.0, f"slow on {payload[:20]!r}..."
+
+
+def test_emphasis_flood_scales_linearly():
+    """Scaling ratio guard: 4x input must cost ~4x time (linear), not
+    ~16x (quadratic). Ratio bound is generous for CI noise."""
+    import time
+
+    def cost(n: int) -> float:
+        payload = "__word " * (n // 7)
+        start = time.monotonic()
+        handle_markdown(payload)
+        return time.monotonic() - start
+
+    small, large = cost(100_000), cost(400_000)
+    assert large / max(small, 1e-3) < 9.0
+
+
+def test_newline_and_whitespace_floods_are_fast():
+    """List-marker patterns must not scan quadratically on blank-line
+    floods ('^(\\s*)' + MULTILINE consumed all following newlines from
+    every line anchor; 100KB of newlines took 65s)."""
+    import time
+
+    for payload in ["\n" * 100_000, "\r\n" * 50_000, " \n" * 50_000]:
+        start = time.monotonic()
+        handle_markdown(payload)
+        assert time.monotonic() - start < 2.0
+
+
+def test_long_wordrun_through_normalize_text_is_fast():
+    """URL_PATTERN must not scan quadratically on long word-char runs
+    with no dot-TLD (100KB single line previously hung normalize_text)."""
+    import time
+
+    from api.src.services.text_processing.normalizer import (
+        normalize_text,
+    )
+    from api.src.structures.schemas import NormalizationOptions
+
+    for payload in ["a" * 100_000, "a." * 50_000, "a@" * 50_000]:
+        start = time.monotonic()
+        normalize_text(payload, NormalizationOptions())
+        assert time.monotonic() - start < 2.0
Evidence
The tests use time.monotonic() and assert fixed runtime ceilings and a runtime scaling ratio;
these can fail due to CI performance variance unrelated to correctness.

api/tests/test_markdown_normalizer.py[440-536]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`api/tests/test_markdown_normalizer.py` contains multiple wall-clock timing assertions (e.g. `< 2.0s`) and a runtime scaling ratio assertion. These are inherently environment-dependent and can become flaky on shared/loaded CI runners.

### Issue Context
These tests are valuable as regression guards (ReDoS/near-linear behavior), but they should not fail due to runner performance variability.

### Fix Focus Areas
- api/tests/test_markdown_normalizer.py[440-536]

### Suggested fix approach
- Convert the strict wall-clock assertions into one of the following:
 - Mark as a separate `@pytest.mark.performance`/`@pytest.mark.slow` suite and exclude from default CI, running them on a dedicated schedule or performance job.
 - Increase thresholds substantially and reduce sensitivity (e.g., measure median of N runs, warm up once, and assert under a much higher cap).
 - Prefer deterministic complexity guards where possible (e.g., limit regex backtracking risk via bounded quantifiers—already done—and keep only the scaling-ratio test with very generous bounds and a minimum runtime floor to reduce denominator noise).
- If you keep the ratio test, avoid unstable ratios when `small` is extremely fast (the current `max(small, 1e-3)` helps, but consider a higher minimum floor or larger input sizes so `small` is reliably measurable).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread api/tests/test_markdown_normalizer.py Outdated
Comment on lines 173 to 204
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:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +343 to +347
for i, sep in enumerate(is_sep_row):
if not sep:
continue
j = i - 1
while j >= 0 and "|" in lines[j]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +213 to +215
_MD_FENCED_CODE = re.compile(
r"^[ \t]*(>[ \t]*)?```[^\n]*\n?(.*?)(?:```|\Z)", re.DOTALL | re.MULTILINE
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +256 to +257
_MD_UNORDERED_LIST = re.compile(r"^([ \t]*)[-*+][ \t]+", re.MULTILINE)
_MD_ORDERED_LIST = re.compile(r"^([ \t]*)\d+\.[ \t]+", re.MULTILINE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +248 to +250
_MD_HEADING = re.compile(
r"^[ \t]*#{1,6}[ \t]+(.*?)(?:[ \t]+#+)?[ \t]*$", re.MULTILINE
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant