feat(prosody): dictionary strategies, LLM annotation, and generation wiring - #1036
Open
Lvigentini wants to merge 12 commits into
Open
feat(prosody): dictionary strategies, LLM annotation, and generation wiring#1036Lvigentini wants to merge 12 commits into
Lvigentini wants to merge 12 commits into
Conversation
Phase 1 of the prosody transformer: a harness around segment production. It
never synthesises anything -- it decides where to cut a script, what settings
each cut carries, and how the pieces are reassembled.
markup -> parse -> compile(engine) -> RenderPlan -> render -> audio
This commit is everything left of the plan. No renderer yet, so nothing in the
generation path changes.
SSML subset, not brackets
-------------------------
Square brackets are already taken: `[laugh]` is a Chatterbox Turbo
paralinguistic tag, passed *to* the engine, where a directive is intercepted
*before* it -- same delimiter, opposite behaviour, and `_PARA_TAG_RE` in
chunked_tts.py cannot tell them apart. Angle brackets are free.
SSML also already owns the pronunciation vocabulary: `<sub alias>` is the
respelling strategy and `<phoneme ph>` the phoneme one, so the dictionary and
the markup share a language instead of having two for the same idea.
Not an XML parser, though. A script is prose -- it contains `&` and `5 < 6`,
both of which XML rejects. The parser recognises a closed tag set and treats
everything else as literal text, which is strictly more forgiving on real
input: nothing needs escaping unless it happens to spell one of our tags.
Malformed *known* markup is an error rather than passthrough, because
passthrough means the engine reads the tag aloud.
The plan is the seam
--------------------
RenderPlan is plain data with no model behind it: it can be built, asserted on
and shown to a user for free. Its warnings carry what the target engine cannot
honour, so `<emphasis>` on base Qwen says so instead of vanishing -- the silent
drop is what makes a model look like it is refusing instructions (jamiepine#579).
Cutting is the cost, so the compiler avoids it: neighbouring runs with equal
settings coalesce, and punctuation orphaned by a span boundary is absorbed into
its neighbour rather than becoming its own generation -- `</lang>.` would
otherwise spend a model call on ".". Unmarked text compiles to a single trivial
run so the common case stays on the existing single-shot path.
A single short word in its own language span is flagged, since prosody restarts
at each cut and that is where these models are least stable; a clause-length
span is not. That is exactly the tight-vs-clause-aligned distinction the
earlier listening prototype was built to test.
strip_markup() is the invariant that will make LLM annotation safe: strip the
model's output and compare to the input, and a model that rewrote the script
instead of annotating it is rejected. It can fail to help; it cannot mangle.
57 tests, no model, no database, 0.06s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 2: the right-hand side of the transformer. Everything here is assembly.
The engine is called once per speech run with that run's own settings, and
silences, rate and joins are arithmetic on the resulting arrays -- which is why
pauses and per-span language work on all eight engines, including the ones that
accept no directives at all.
`generate_run` is injected rather than imported. That keeps the renderer
testable with no model, and it is also how a long run still goes through
`generate_chunked`: prosody splits by directive, chunking splits by length,
and neither has to know about the other.
Two rules, both measured rather than assumed.
Trim the edges
--------------
Every generation carries ~340ms of leading and ~100ms of trailing silence.
Left in, each cut inserts dead air the author never asked for. Measured through
the renderer with a length-proportional stub, so the two plans speak the same
words:
untrimmed 1 run 1.34s 3 runs 2.12s +0.78s
trimmed 1 run 0.96s 3 runs 0.98s +0.02s
which reproduces the +0.70s / +0.03s measured on real model output in the
earlier listening prototype. Segmentation is duration-neutral once trimmed,
and that is the whole reason this approach is viable.
Do not crossfade into a pause
-----------------------------
A crossfade across a Silence overlaps it from both ends, so a 700ms break would
not last 700ms. Runs joined to each other overlap; runs adjacent to silence are
butted. Tested by asserting the faded and butted renders of the same plan have
equal duration.
Rate uses librosa's phase vocoder, matching the story mixer -- resampling would
transpose the voice, which is not what a rate directive means. Runs returning
different sample rates are resampled to the first, since concatenating
mismatched rates comes out as a pitch shift.
A plan that opens with a break holds the silence until a run establishes the
sample rate, rather than guessing one.
19 renderer tests, 76 across the transformer, all model-free, 2.1s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by generating the listening comparison through the real renderer: the `<sub>` case produced three runs, when the entire point of a respelling is that it needs no cut at all. It changes characters, not settings, so cutting there buys exactly the seams that respelling exists to avoid -- and it would have made the no-cut option sound worse than it is in the very comparison meant to choose between them. Cause was coalescing being blocked whenever `spoken_as` was set. That flag governs *inheritance* -- a substitution must not extend over neighbouring text that was never inside the tag -- but once it has been materialised into the spoken text it is no longer a setting and must stop keeping runs apart. Substitution is now resolved before coalescing, into (spoken, written, attrs) with `spoken_as` cleared, so runs merge on what actually differs at render time. Both forms are carried through, so `source_text` still shows the author what was changed rather than silently handing back something they never typed. `The shot is a <sub alias="ban-DEH-ha">bandeja</sub>, not a smash.` now compiles to one run instead of three. Three existing tests asserted the old shape and were updated to assert the new one, plus a test pinning the no-cut property directly, since that is the property the feature is chosen for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Listening feedback said everything sounded "forced" -- including the plain baseline, which carries no markup at all. That was the tell: an unmarked sentence is a single run, so nothing about it should be processed. Measured against the live engine, the cause was trimming the outer edges. A generation carries its own release after the last word -- 290ms on one voice, 400ms on another -- and the trimmer cut it back to the 30ms cushion. Every clip ended abruptly a moment after its final sound, which is exactly what a forced delivery sounds like. The lead-in was being cut the same way. That silence is the utterance's own boundary, not an artefact of cutting. What actually accumulates is the *join*: run N's release butted against run N+1's lead-in is two lots of dead air nobody asked for, and it compounds with the number of runs. So the renderer now trims where runs meet and leaves the first lead-in and the last release alone. Duration neutrality is unaffected -- both the one-run and three-run plans keep the same outer edges, so the interior saving is the same figure as before. A trivial single-run plan now comes back exactly as the engine produced it, which is what it should always have been: in production such a plan takes the single-shot path, so rendering it must not differ. Three tests: the outer lead-in and release survive at their measured sizes, the interior join is still collapsed, and a single run is returned untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from listening across three voices, now expressed in the code rather than left in a conversation. Tight language spans are fine ----------------------------- The compiler warned that a single-word `<lang>` span would sound seamy and suggested widening it or respelling instead. Listening contradicts that: the tight span was judged good on every voice tried. A warning that steers people away from what works is worse than no warning, so it is gone. That warning existed to hedge the prosody-reset risk that gated this whole design. The risk did not materialise. Heavy respellings do misfire ---------------------------- `ban-DEH-ha` -- syllable hyphens plus capitals for stress -- was judged exaggerated on every voice, and measurably so: ~30% longer than the same sentence unmarked, consistently across two very different voices. Plain `bandeha` sounded most natural and ran ~8% shorter; `ban-deh-ha` was acceptable as a fallback. So it is the *combination* that misfires, not either alone, and the new warning fires only on that shape. Acronym expansions like `W C A G` are capitals without hyphens and are deliberately not flagged. A bare <break/> is 700ms ------------------------ 700ms was judged a good pause on every voice; 1500ms read as too long unless the script genuinely wants a beat to stop and think. A bare `<break/>` now means the good default instead of a no-op, so the common case needs no argument. Worth recording for whoever tunes this later: the natural gap after a full stop is already 210-440ms depending on the voice, so a break adds to a pause rather than creating one -- which is why 700ms can read as barely a change on a voice that already pauses generously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Listening rejected `<prosody rate>` as distorting the audio. The cause was the algorithm, not the idea: `librosa.effects.time_stretch` is a phase vocoder, which reconstructs from magnitudes and re-estimated phase. On speech that smears consonants and leaves the characteristic phasey ring. WSOLA never leaves the time domain. It overlap-adds real waveform segments, choosing each splice point by cross-correlation so successive pitch periods line up, so nothing is resynthesised and consonants stay intact. No new dependency -- it is numpy. Resampling remains the wrong answer for a different reason: it would transpose the voice, which is not what a rate directive means. There is a test for that now, comparing the dominant frequency before and after. A bug the tests caught: the first version folded the search offset back into the read pointer, so a run of forward-biased matches accelerated the read and returned 1.47s where 2.0s was asked for. The nominal pointer has to advance by `analysis_hop` regardless of where the search landed. Ratios are accurate now across 0.5x to 2.0x. Worth recording for whoever picks this up: `services/stories.py` applies per-clip speed with the same phase vocoder, so the story mixer has the same artefact. This function is self-contained and should move there too -- tracked separately since it belongs to jamiepine#1007 rather than here. 4 further tests: ratio accuracy across five rates, pitch preservation, and that silence stays silent rather than ringing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Names, acronyms, brands and loanwords come out wrong and there is no reusable way to fix them (jamiepine#827) -- you edit the text every time, in every script. Adds a term -> respelling map applied just before TTS. Respelling rather than phonemes on purpose: every engine reads plain text, so `bandeja -> ban-DEH-ha` works on all of them, where a phoneme string only works on the engines that accept one. Cruder, portable. Entries are global by default and can be scoped to a language, a voice, or both. Language scope is the one that earns its keep for mixed-language work: a Spanish term needs respelling while the engine is reading English and must be left alone when it is already reading Spanish. A profile-scoped entry beats a global one; a language-specific entry beats a wildcard. Applied at generation time, not when the text is saved. `generations.text` keeps what the author wrote, so History stays readable and editing an entry changes future audio without rewriting the past. `POST /pronunciations/preview` exists because of that -- the rewritten string is never stored, so without it there is no way to see what a rule does short of listening. Matching is a single pass over one alternation of all terms, longest first. That is what stops replacements cascading: with `bandeja -> ban-DEH-ha` and `ha -> hah`, a loop of per-term substitutions produces `ban-DEH-hah`. It also lets a multi-word entry beat the single-word entry inside it. Word boundaries use lookarounds so terms with punctuation still anchor, terms are escaped so a term is text and not a pattern, and `[laugh]`-style tags are skipped because they are engine syntax rather than speech. Capitalisation carries onto the replacement, counting cased characters rather than `str.isupper()` -- that returns True for `C++`, and shouting the replacement would turn `C plus plus` into `C PLUS PLUS`. Duplicate scopes are rejected in the service rather than by a unique constraint, since SQL treats NULLs as distinct and would accept two global entries for the same term. Applies on both `/generate` and `/generate/stream` so a streamed preview matches what the persisted path produces. 28 tests covering matching, the no-cascade rule, capitalisation, scope resolution, degenerate input, CRUD, and the property the design rests on: the engine receives the respelling and the stored row does not. Closes jamiepine#827 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four of the five CodeRabbit findings were real. SQL wildcards in a term ----------------------- `find_duplicate` used `term.ilike(term)`, so `%` and `_` in a term were read as patterns rather than literals -- `band_ja` collided with `bandeja` and was rejected as a duplicate. Compares `lower(term)` instead, trimmed first, since the route stores the trimmed value and an untrimmed lookup missed its own duplicate. Whitespace-only values ---------------------- `min_length=1` accepted `" "`, which the route then stripped and stored as empty -- a no-op entry, or a replacement that deletes the matched speech. Both request models now strip before length-checking, matching the `TrimmedName` approach already used for folders. Preview accepted an unknown profile ----------------------------------- It silently fell back to global scope and reported a result the real generation would not produce, which 404s on an unknown profile. Validates first. Scope uniqueness now enforced by the database --------------------------------------------- `find_duplicate` is check-then-act; two concurrent creates both pass it. My comment claimed a constraint could not express this because SQL treats NULLs as distinct -- that was wrong. A unique expression index over `lower(term), COALESCE(language, ''), COALESCE(profile_id, '')` maps the wildcard scopes onto comparable values and holds. Violations map to 409 rather than 500. `find_duplicate` stays as the early check that can name the existing row in the message. Added a migration for it: `create_all` builds the index with the table, but will not add one to a table that already exists, so a database from an earlier build of this feature would never get it. Pre-existing duplicates are collapsed first, keeping the oldest row per scope, or CREATE UNIQUE INDEX would fail. `IF NOT EXISTS` because the inspector reflects a snapshot and a migration that raises takes startup down with it -- which the full suite reproduced, several modules booting the app in one process. Not taken: logging terms at INFO. Terms are user-supplied and often names, so the count stays at INFO and the values moved to DEBUG. 7 further tests: literal wildcards, untrimmed duplicate lookup, whitespace-only rejection on both fields, trimmed storage, preview validation, the database constraint including the case-differing global pair a plain UNIQUE would let through, and that terms stay out of INFO logs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more review findings, both correct. `MIN(id)` is not "oldest" ------------------------ The dedup step before CREATE UNIQUE INDEX claimed to keep the oldest row in each scope but ordered by `MIN(id)`, and ids are random UUIDs -- so it kept an arbitrary row, and a different one on a different machine. The code did not do what its own comment said. Now orders by `COALESCE(created_at, '') || '|' || id`: timestamp first, id only to break ties deterministically. ISO-8601 text sorts chronologically, so this needs no window function and stays portable across SQLite builds. Weak privacy assertion ---------------------- The test that terms stay out of INFO logs checked only the first name, so a message leaking just the surname would have passed. Checks every fragment of the term and the replacement. Added a test that the dedup keeps the oldest row, inserting the newer entry first so insertion order cannot be what makes it pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 3. The dictionary stops being a private text-substitution pass and becomes a *source of markup*: an entry emits the same directive an author would have typed, and the ordinary parse-compile-render pipeline takes it from there. That is worth more than it first looks. A dictionary term and a hand-written span compose, because by the time the compiler sees them they are the same thing. Every rule the compiler already enforces -- coalescing, orphaned punctuation, engine capability -- applies to dictionary output for free. And the result is showable: a preview can hand back the annotated markup, so a rule firing unexpectedly is visible rather than being an invisible difference between what was typed and what was spoken. The forthcoming LLM annotator emits into exactly the same slot, which is what will keep "with an LLM" and "without an LLM" the same code path downstream. Three strategies ---------------- respell <sub alias="bandeha">bandeja</sub> every engine language <lang xml:lang="es">víbora</lang> every engine, via cutting phoneme <phoneme ph="...">chiquita</phoneme> engines that accept it `replacement` stays required and stays plain text, because it is the fallback whenever the preferred strategy is unavailable -- a phoneme entry on an engine with no phoneme support renders as its respelling rather than doing nothing. Existing rows are all respellings, which is the default, so the migration needs no backfill. Both strategies are now evidence-backed rather than guesses: listening across three voices found tight `<lang>` spans good and minimal respellings most natural, which is why `respell` and `language` are peers rather than one being a workaround for the other. Annotation leaves the author alone ---------------------------------- A term already wrapped by hand is not re-wrapped -- the author has said what they want. Text inside an attribute value is never rewritten, which would corrupt the markup. A rule whose replacement equals the term emits no tag at all rather than an inert one that would cost a cut. Found by its own tests: escaping a quote into `"` on write meant the engine would have spoken the entity, since nothing unescaped it on read. The parser now decodes attribute entities, ampersand last so `&quot;` survives as literal text. A strategy that cannot be realised is rejected at the API rather than stored: `strategy="language"` with no `spoken_language` would show the user the strategy they picked while silently falling back. Updates validate against the row as it will be, since a strategy change can rely on a field set earlier. 22 annotator tests, 8 API tests. 256 backend tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 4. The local LLM drafts markup; it never touches rendering. It reads a script and returns the same script with directives inserted. From there the deterministic pipeline runs exactly as it does for markup typed by hand -- which is what keeps "with an LLM" and "without an LLM" the same code path downstream, and why generation stays reproducible. A model in the render path would make the same script produce different audio on every run. The invariant ------------- Strip the tags from the model's answer and compare to the input. If a single word moved, the model rewrote the script instead of annotating it, and the answer is discarded. It can fail to help; it cannot mangle. Without that check an LLM quietly rephrasing a line would be discovered only by listening. `strip_markup()` has been waiting for this since phase 1; this is what it was for. Small models wrap answers in fences and quotes. Rather than guess which, every plausible unwrapping is offered and the invariant picks the first that is faithful -- so a wrapper is removed only when doing so yields something demonstrably correct, and no unwrapping can launder a changed word into acceptance. A first attempt that fails is retried once with the complaint fed back, since the usual failure is prose around the answer rather than a misunderstanding of the task. `markup` on the result is always safe to use: on rejection it is the original text, so a caller applies it unconditionally and reads `rejected_reason` only to explain why nothing changed. Without an LLM -------------- A missing model is reported, never downloaded on demand -- a feature that silently pulls gigabytes the first time it is used is not optional, and this one is. `GET /prosody/annotate/availability` lets a client hide the action rather than offer something that will fail, and the endpoint answers 409 with a message saying what still works. Preview ------- `POST /prosody/preview` compiles a script to its plan without generating anything: every cut, language, silence, the dictionary terms that fired, the markup they resolved to, and everything the chosen engine cannot honour. The plan being plain data is what makes this free. That closes the loop opened in phase 3 -- the dictionary emits markup, so the preview can show the directives rather than only their effect. 24 tests, model stubbed throughout. 279 backend tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 5. `/generate` and `/generate/stream` now resolve dictionary entries and prosody markup before synthesis. Unmarked text is untouched -------------------------- The property under most scrutiny is the one about *not* changing anything. A script with no markup and no dictionary hits compiles to a single plain run and takes the same single-shot call it always did, with the same arguments. Prose that merely looks like markup -- `5 < 6`, `x > y` -- is literal, because the parser only recognises a closed tag set. Malformed markup falls back to speaking the text literally rather than failing the generation. A stray tag must not be able to break generation for someone who never used the feature; before this existed the text was literal, so that is what it degrades to. Auto-detected rather than opt-in, with `prosody: false` as the escape hatch for a script that genuinely contains something tag-shaped. An opt-in flag would have meant updating every caller -- MCP, API, story regeneration -- or leaving the feature invisible. One entry point, two callers ---------------------------- `generate_with_prosody` serves both generation paths, so "unmarked text behaves as before" is a property of one function rather than a claim repeated twice. `generate_chunked` is passed in rather than imported, so prosody composes with chunking instead of competing: prosody splits by directive, chunking splits by length, and a directive run that is still long goes through both. The stored row keeps the markup, not the resolved text -- consistent with the dictionary (jamiepine#1025) and regenerate (jamiepine#1026): the resolved form is derivable, the author's markup is not, and editing markup to regenerate needs it intact. Two fixes found by building this -------------------------------- `is_trivial` excluded any plan carrying a substitution, so every respelled sentence would have taken the renderer path for nothing -- contradicting the property that makes respelling preferred, that it does not cut. `source_text` is provenance for display; by that point the respelling is already in the text. The migration guard for `uq_pronunciation_scope` never fired: SQLAlchemy cannot reflect an expression-based index and skips it with a warning, so the inspector never reported it and the dedup scan ran on every startup. Reads sqlite_master directly now. `IF NOT EXISTS` had been quietly carrying it. Also drops the direct `apply_pronunciations` call from the generation path: the dictionary now reaches the engine as markup, which is how `language` and `phoneme` entries work at all. 19 pipeline tests. 295 backend tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds scoped pronunciation dictionaries, prosody markup parsing and compilation, local LLM annotation, audio rendering, preview endpoints, and prosody-aware persisted and streaming generation. ChangesPronunciation and Prosody
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GenerateRoute
participant run_generation
participant generate_with_prosody
participant PronunciationService
participant prosody_renderer
participant TTSEngine
GenerateRoute->>run_generation: submit text and prosody setting
run_generation->>generate_with_prosody: pass generation context
generate_with_prosody->>PronunciationService: resolve scoped pronunciation entries
PronunciationService-->>generate_with_prosody: return matching entries
generate_with_prosody->>prosody_renderer: render compiled speech and silence plan
prosody_renderer->>TTSEngine: generate speech runs
TTSEngine-->>prosody_renderer: return audio segments
prosody_renderer-->>generate_with_prosody: return assembled audio
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (15)
backend/tests/test_pronunciation.py (2)
423-435: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the route-level 409 from
_commit_or_conflict.This test exercises the constraint through the ORM session. No test drives the
IntegrityErrorpath in_commit_or_conflictinbackend/routes/pronunciation.pyat Lines 31-37, so the substring match on"uq_pronunciation_scope"and the resulting 409 are unverified. A regression there turns a conflict into a 500.One way to cover it: create an entry directly through the session with a case variant, bypassing
find_duplicate, then POST the colliding term and assert 409.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_pronunciation.py` around lines 423 - 435, Extend the pronunciation route tests to cover `_commit_or_conflict`: seed a case-variant conflicting entry directly through the database session to bypass `find_duplicate`, then POST the colliding term through the route and assert HTTP 409. Verify this exercises the `IntegrityError` handling for the `"uq_pronunciation_scope"` constraint rather than returning 500.
454-491: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the migration restores uniqueness enforcement.
The test drops
uq_pronunciation_scopeand verifies only the row that survives. It does not verify thatrun_migrationsrecreated the index. If theCREATE UNIQUE INDEXstep regressed, this test still passes, and every later test in the module then runs without the constraint the module relies on at Line 423.💚 Proposed addition
db.expire_all() rows = db.query(PronunciationEntry).all() assert len(rows) == 1 assert rows[0].replacement == "KEEP" + + # The other half of the migration: enforcement has to be back. + with engine.connect() as conn: + assert conn.execute( + sql_text( + "SELECT 1 FROM sqlite_master " + "WHERE type = 'index' AND name = 'uq_pronunciation_scope'" + ) + ).first()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_pronunciation.py` around lines 454 - 491, Extend test_dedup_migration_keeps_the_oldest_row to verify that run_migrations recreates uq_pronunciation_scope after deduplication, using the existing database engine and SQLAlchemy inspection/query facilities. Keep the current survivor assertions and ensure the test explicitly confirms the restored uniqueness constraint before completing.backend/services/pronunciation.py (2)
81-98: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueThe ReDoS hint on Line 97 is not applicable.
Every term passes through
re.escape, and the generated pattern is a flat alternation of literals with two lookarounds. It contains no nested quantifiers, so catastrophic backtracking is not reachable. No change is needed.One scaling note: the alternation grows with the number of enabled entries in scope. A large dictionary can reach the
remodule's compiled-pattern size limit and raise at compile time. If the dictionary is expected to hold thousands of entries, cap the pattern size or batch the terms.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/pronunciation.py` around lines 81 - 98, Leave build_pattern unchanged regarding ReDoS concerns; the escaped, flat alternation is safe. If the enabled dictionary can contain thousands of entries, add an appropriate size cap or batch the terms before re.compile, while preserving longest-first matching and the existing empty-input behavior.Source: Linters/SAST tools
69-71: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse
str.isupper()per character on cased letters only.The list is named
cased, butisalpha()also accepts caseless letters. For a term such as中文, the characters are alpha andisupper()returnsFalse, so the all-caps branch never fires. The result is correct today, and the fallback at Line 72 handles it. Filtering onc.islower() or c.isupper()matches the stated intent and the variable name.♻️ Proposed change
- cased = [c for c in source if c.isalpha()] + cased = [c for c in source if c.islower() or c.isupper()] if len(cased) > 1 and all(c.isupper() for c in cased): return replacement.upper()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/pronunciation.py` around lines 69 - 71, Update the cased character collection in the pronunciation replacement logic to include only characters where c.islower() or c.isupper() is true, then retain the existing all-uppercase check and replacement.upper() behavior.backend/database/models.py (1)
191-191: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider defining the delete behavior for
profile_id.
profile_idreferencesprofiles.idwith noondeleterule and no ORM relationship. When a profile is deleted, its scoped entries stay in the table and become unreachable rows. Aondelete="CASCADE"(pluspassive_deletes) or an explicit cleanup in the profile delete path makes the intent visible.Note that SQLite enforces foreign keys only when
PRAGMA foreign_keys=ONis set per connection, so verify the pragma state before relying on cascade.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/database/models.py` at line 191, Define explicit delete behavior for the profile_id foreign key in the model: use database-level cascading with an ORM relationship configured for passive deletes, or add cleanup to the profile deletion flow. If relying on cascading, verify SQLite connections enable PRAGMA foreign_keys=ON and add or update the corresponding schema migration.backend/models.py (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
Annotatedfromtyping.
typing.Annotatedexists since Python 3.9, and this file already relies on 3.10+ syntax elsewhere in the backend. Thetyping_extensionsdependency is not needed here.♻️ Proposed change
from pydantic import BaseModel, Field, StringConstraints -from typing_extensions import Annotated +from typing import AnnotatedMerge it into the existing
typingimport line if one is already present.As per static analysis: Ruff UP035 "Import from
typinginstead:Annotated".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/models.py` at line 6, Update the Annotated import in backend/models.py to use Python’s standard typing module instead of typing_extensions, merging it into the existing typing import if present and removing the unnecessary typing_extensions import.Source: Linters/SAST tools
backend/tests/test_prosody_pipeline.py (2)
144-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant
languageoverride.
BASEalready setslanguage="en". The comprehension stripslanguagefromBASEand the next line adds the same value back. Use**BASEas the other tests do.♻️ Proposed simplification
db=db, engine_languages=["en", "es"], - **{k: v for k, v in BASE.items() if k != "language"}, - language="en", + **BASE, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_prosody_pipeline.py` around lines 144 - 152, In the generate_with_prosody call, replace the filtered BASE expansion and explicit language argument with **BASE, preserving the existing language="en" value from BASE and matching the other tests.
266-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
/generate/stream.The tests cover
generate_with_prosodydirectly and theprosodyflag on/generate. The streaming path inbackend/routes/generations.pybuildsgen_kwargsdifferently, and that difference is where an omittedcrossfade_msbecomesNone. A test that posts to/generate/streamwithoutmax_chunk_charsorcrossfade_ms, using markup that produces more than one run, would catch it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_prosody_pipeline.py` around lines 266 - 278, Add a test alongside test_generate_accepts_the_prosody_flag that posts to /generate/stream without max_chunk_chars or crossfade_ms, using markup that produces multiple runs, and verifies successful streaming behavior. Exercise the streaming route’s generated request path and clean up the created profile and generation resources consistently with the existing test.backend/tests/test_prosody_renderer.py (1)
330-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the local
numpyimports.
numpyis already imported asnpat Line 20. The two function-level imports shadow it with the same binding and add no value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_prosody_renderer.py` around lines 330 - 352, Remove the function-level numpy imports from test_stretching_preserves_pitch and test_stretching_does_not_resynthesise_silence_into_noise, and use the existing module-level np import for their NumPy operations.backend/services/prosody/renderer.py (1)
128-140: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winVectorize the similarity search.
The search evaluates up to
2*search+1candidate offsets per frame in a Python list comprehension. At 24 kHz that is 481np.dotcalls of 720 samples for every 15 ms of output. A minute of audio costs roughly 2 million dot products with per-call Python overhead.np.correlateor a strided view computes the same scores in one call.The scores are also raw dot products, so a loud candidate can win over a better-aligned quiet one. Normalizing by the candidate energy makes the match a true waveform similarity.
♻️ Proposed vectorized search
- candidates = np.arange(lo, hi + 1) - scores = [ - float(np.dot(audio[c : c + frame], expected)) for c in candidates - ] - offset = int(candidates[int(np.argmax(scores))]) + window_view = np.lib.stride_tricks.sliding_window_view( + audio[lo : hi + frame], frame + ) + scores = window_view @ expected + norms = np.sqrt(np.einsum("ij,ij->i", window_view, window_view)) + 1e-9 + offset = lo + int(np.argmax(scores / norms))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/prosody/renderer.py` around lines 128 - 140, Replace the per-candidate np.dot list comprehension in the similarity search with a vectorized correlation or strided-window computation over audio[lo:hi+frame]. Normalize each candidate score by its energy, guarding zero-energy windows, so selection compares waveform alignment rather than loudness; preserve the existing offset bounds and argmax behavior.backend/services/prosody/parser.py (1)
62-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
ms < 0branch is unreachable.
_DURATION_REat line 47 accepts no sign, sofloat(m.group("value"))is never negative. Only the upper bound can fail. Keeping the check is harmless defensive code; simplify it if you prefer the message to name the real failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/prosody/parser.py` around lines 62 - 71, Simplify the range validation in parse_duration by removing the unreachable negative-value check and validating only that ms exceeds MAX_BREAK_MS, updating the error message to describe the upper-bound failure accurately.backend/services/prosody/annotate.py (1)
86-88: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
_escapeand the parser's_unescapeare not inverses.
_escapeonly replaces". The parser's_unescapeinbackend/services/prosody/parser.py(line 78) also decodes&,<,>, and'. A replacement that legitimately contains the text"or&therefore round-trips to a different spoken string. Escaping&first in_escapewould make the pair symmetric.♻️ Proposed change
def _escape(value: str) -> str: """Attribute values are quoted, so a quote inside one would end it early.""" - return value.replace('"', """) + return value.replace("&", "&").replace('"', """)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/prosody/annotate.py` around lines 86 - 88, Update _escape to replace ampersands before replacing double quotes, while preserving the existing quote escaping. This must make annotation values symmetric with the parser’s _unescape by preventing literal entity text such as " and & from being decoded incorrectly.backend/services/prosody/compiler.py (1)
30-42: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
_is_over_articulatedflags hyphenated proper nouns.Any alias with a hyphen and one later capital matches, so
"Mary-Jane"or"San-Jose"produce anover_articulated_respellingwarning. The effect is a spurious warning only. Consider requiring a capitalised segment that is not word-initial, for example testing each hyphen-separated part for an all-caps or capitalised form that follows another part.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/prosody/compiler.py` around lines 30 - 42, Update _is_over_articulated so it only flags hyphenated aliases containing a later capitalized segment that is not a normal proper-noun word segment, while preserving detection of the exaggerated stress-marking pattern. Avoid warning for names such as “Mary-Jane” and “San-Jose,” and retain the existing behavior for acronym-style expansions.backend/tests/test_prosody_llm.py (1)
173-182: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
stub_llmis unused in this test.
test_an_llm_error_is_not_fatalinstalls its own backend throughmonkeypatch, so thestub_llmparameter has no effect. Remove it to keep the setup of each test explicit.♻️ Proposed change
-async def test_an_llm_error_is_not_fatal(stub_llm, monkeypatch): +async def test_an_llm_error_is_not_fatal(monkeypatch):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_prosody_llm.py` around lines 173 - 182, Remove the unused stub_llm parameter from test_an_llm_error_is_not_fatal, leaving its monkeypatched Exploding backend setup and test behavior unchanged.backend/tests/test_prosody_transformer.py (1)
54-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a prose case that begins with a known tag name.
The current strings avoid tag-name prefixes, so they do not exercise
_TAG_RE."<breaking> news"and"<language> barrier"currently do not stay literal. See the comment onbackend/services/prosody/parser.pylines 38-42 for the root cause. Add those strings to this parametrize list after the regex is bounded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/test_prosody_transformer.py` around lines 54 - 69, Update the _TAG_RE handling in the prosody parser to require a complete known tag match so prose beginning with a tag-name prefix remains literal. Then extend test_prose_that_would_break_an_xml_parser_is_literal with “<breaking> news” and “<language> barrier” cases, preserving the existing texts(plan(text)) expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/models.py`:
- Around line 230-235: Update backend/models.py at lines 230-235 in
PronunciationPreviewRequest to apply the same supported-language pattern as
GenerationRequest.language. Also update backend/models.py at lines 254-264 in
ProsodyPreviewRequest to replace engine’s max_length-only validation with the
same engine pattern used by GenerationRequest.engine, keeping preview validation
aligned with generation.
In `@backend/routes/generations.py`:
- Around line 380-394: Update the gen_kwargs construction in the generation
route to omit max_chunk_chars and crossfade_ms when their GenerationRequest
values are None, allowing generate_with_prosody/generate_chunked to apply the
existing defaults. Preserve passing explicitly provided values unchanged.
In `@backend/routes/pronunciation.py`:
- Around line 78-86: Update the filtered path around pronunciation.get_entries
so results are ordered by PronunciationEntry.term consistently with the
unfiltered branch. Apply the ordering at the shared get_entries implementation
if appropriate, preserving the existing filtering and include_disabled behavior.
In `@backend/services/generation.py`:
- Around line 104-118: Update the regenerate flow around generate_with_prosody
so it passes the same effective seed used in gen_kwargs, rather than the
original seed variable. Ensure mode == "regenerate" remains randomized for both
plain and multi-run inputs while preserving deterministic behavior for other
modes.
In `@backend/services/prosody/annotate.py`:
- Around line 56-59: Update the LANGUAGE branch in the annotation method to pass
self.spoken_language through _escape before interpolating it into the xml:lang
attribute, matching the existing phonemes escaping while preserving the current
output structure.
In `@backend/services/prosody/compiler.py`:
- Around line 153-169: In the language normalization flow around
engine_languages and seen_unsupported_language, always assign default_language
whenever the requested language is unsupported, while retaining
seen_unsupported_language only to suppress duplicate warnings. Extend
test_each_unsupported_language_warns_once to assert every Speech node uses the
default language, such as "en", in addition to checking the single warning.
- Around line 138-151: Update the whitespace-gluing logic in the plan-node
compilation flow: when extending the previous Speech node, append raw_text to
source_text whenever source_text is set, preserving text/source_text
synchronization. Also retain whitespace that occurs before the first Speech or
immediately after a Silence instead of dropping it, using the appropriate
existing node representation and preserving current behavior for other
whitespace cases.
In `@backend/services/prosody/llm_annotate.py`:
- Around line 160-169: Bound the local LLM generation in annotate_with_llm by
applying an asyncio.wait_for timeout around backend.generate, using the
service-level timeout configuration if one exists and handling timeout failures
through the existing retry/error path. Keep route-level behavior unchanged and
preserve the intermediate service ownership of the backend call.
In `@backend/services/prosody/parser.py`:
- Around line 38-42: Prevent both prosody regexes, _TAG_RE and _STRIP_RE, from
matching tag-name prefixes by requiring a boundary after the name that excludes
word, colon, dot, and hyphen characters; update
backend/services/prosody/parser.py lines 38-42 and 202-204 consistently. Add
"<breaking> news" and "<language> barrier" to the parameterized cases in
backend/tests/test_prosody_transformer.py lines 54-69 to verify markup stripping
and parsing agree.
In `@backend/services/prosody/pipeline.py`:
- Around line 82-89: The annotate callers omit the engine’s phoneme capability,
preventing phoneme strategy entries from producing phoneme markup. In
backend/services/prosody/pipeline.py lines 82-89, resolve the phoneme capability
for engine alongside supports_instruct via engine_capabilities and pass it to
annotate; in backend/routes/prosody.py lines 43-46, move
engine_capabilities(data.engine) before annotate and pass the resolved
capability so preview and generation behave consistently.
In `@backend/tests/test_prosody_llm.py`:
- Around line 119-121: Add pytest-asyncio as an explicit test dependency in the
project configuration, preferably under [project.optional-dependencies.test] if
supported by the existing package build flow. Keep the asyncio_mode
configuration and `@pytest.mark.asyncio` usage in
test_a_fenced_answer_is_unwrapped unchanged.
---
Nitpick comments:
In `@backend/database/models.py`:
- Line 191: Define explicit delete behavior for the profile_id foreign key in
the model: use database-level cascading with an ORM relationship configured for
passive deletes, or add cleanup to the profile deletion flow. If relying on
cascading, verify SQLite connections enable PRAGMA foreign_keys=ON and add or
update the corresponding schema migration.
In `@backend/models.py`:
- Line 6: Update the Annotated import in backend/models.py to use Python’s
standard typing module instead of typing_extensions, merging it into the
existing typing import if present and removing the unnecessary typing_extensions
import.
In `@backend/services/pronunciation.py`:
- Around line 81-98: Leave build_pattern unchanged regarding ReDoS concerns; the
escaped, flat alternation is safe. If the enabled dictionary can contain
thousands of entries, add an appropriate size cap or batch the terms before
re.compile, while preserving longest-first matching and the existing empty-input
behavior.
- Around line 69-71: Update the cased character collection in the pronunciation
replacement logic to include only characters where c.islower() or c.isupper() is
true, then retain the existing all-uppercase check and replacement.upper()
behavior.
In `@backend/services/prosody/annotate.py`:
- Around line 86-88: Update _escape to replace ampersands before replacing
double quotes, while preserving the existing quote escaping. This must make
annotation values symmetric with the parser’s _unescape by preventing literal
entity text such as " and & from being decoded incorrectly.
In `@backend/services/prosody/compiler.py`:
- Around line 30-42: Update _is_over_articulated so it only flags hyphenated
aliases containing a later capitalized segment that is not a normal proper-noun
word segment, while preserving detection of the exaggerated stress-marking
pattern. Avoid warning for names such as “Mary-Jane” and “San-Jose,” and retain
the existing behavior for acronym-style expansions.
In `@backend/services/prosody/parser.py`:
- Around line 62-71: Simplify the range validation in parse_duration by removing
the unreachable negative-value check and validating only that ms exceeds
MAX_BREAK_MS, updating the error message to describe the upper-bound failure
accurately.
In `@backend/services/prosody/renderer.py`:
- Around line 128-140: Replace the per-candidate np.dot list comprehension in
the similarity search with a vectorized correlation or strided-window
computation over audio[lo:hi+frame]. Normalize each candidate score by its
energy, guarding zero-energy windows, so selection compares waveform alignment
rather than loudness; preserve the existing offset bounds and argmax behavior.
In `@backend/tests/test_pronunciation.py`:
- Around line 423-435: Extend the pronunciation route tests to cover
`_commit_or_conflict`: seed a case-variant conflicting entry directly through
the database session to bypass `find_duplicate`, then POST the colliding term
through the route and assert HTTP 409. Verify this exercises the
`IntegrityError` handling for the `"uq_pronunciation_scope"` constraint rather
than returning 500.
- Around line 454-491: Extend test_dedup_migration_keeps_the_oldest_row to
verify that run_migrations recreates uq_pronunciation_scope after deduplication,
using the existing database engine and SQLAlchemy inspection/query facilities.
Keep the current survivor assertions and ensure the test explicitly confirms the
restored uniqueness constraint before completing.
In `@backend/tests/test_prosody_llm.py`:
- Around line 173-182: Remove the unused stub_llm parameter from
test_an_llm_error_is_not_fatal, leaving its monkeypatched Exploding backend
setup and test behavior unchanged.
In `@backend/tests/test_prosody_pipeline.py`:
- Around line 144-152: In the generate_with_prosody call, replace the filtered
BASE expansion and explicit language argument with **BASE, preserving the
existing language="en" value from BASE and matching the other tests.
- Around line 266-278: Add a test alongside
test_generate_accepts_the_prosody_flag that posts to /generate/stream without
max_chunk_chars or crossfade_ms, using markup that produces multiple runs, and
verifies successful streaming behavior. Exercise the streaming route’s generated
request path and clean up the created profile and generation resources
consistently with the existing test.
In `@backend/tests/test_prosody_renderer.py`:
- Around line 330-352: Remove the function-level numpy imports from
test_stretching_preserves_pitch and
test_stretching_does_not_resynthesise_silence_into_noise, and use the existing
module-level np import for their NumPy operations.
In `@backend/tests/test_prosody_transformer.py`:
- Around line 54-69: Update the _TAG_RE handling in the prosody parser to
require a complete known tag match so prose beginning with a tag-name prefix
remains literal. Then extend
test_prose_that_would_break_an_xml_parser_is_literal with “<breaking> news” and
“<language> barrier” cases, preserving the existing texts(plan(text))
expectation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e7d4dc2-de99-4098-a5b6-0c68f4ff3e73
📒 Files selected for processing (24)
backend/database/__init__.pybackend/database/migrations.pybackend/database/models.pybackend/models.pybackend/routes/__init__.pybackend/routes/generations.pybackend/routes/pronunciation.pybackend/routes/prosody.pybackend/services/generation.pybackend/services/pronunciation.pybackend/services/prosody/__init__.pybackend/services/prosody/annotate.pybackend/services/prosody/compiler.pybackend/services/prosody/ir.pybackend/services/prosody/llm_annotate.pybackend/services/prosody/parser.pybackend/services/prosody/pipeline.pybackend/services/prosody/renderer.pybackend/tests/test_pronunciation.pybackend/tests/test_prosody_annotate.pybackend/tests/test_prosody_llm.pybackend/tests/test_prosody_pipeline.pybackend/tests/test_prosody_renderer.pybackend/tests/test_prosody_transformer.py
Comment on lines
+230
to
+235
| class PronunciationPreviewRequest(BaseModel): | ||
| """Request to see what the dictionary would do to a piece of text.""" | ||
|
|
||
| text: str = Field(..., min_length=1, max_length=50000) | ||
| language: Optional[str] = None | ||
| profile_id: Optional[str] = None |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Preview request models accept values generation rejects. Both preview endpoints exist to show what generation would do. Their request models validate scope fields more loosely than GenerationRequest, so a preview can describe a configuration that /generate refuses with a 422.
backend/models.py#L230-L235: add the supported-language pattern toPronunciationPreviewRequest.language, matchingGenerationRequest.languageat Line 85.backend/models.py#L254-L264: replacemax_length=50onProsodyPreviewRequest.enginewith the engine pattern used byGenerationRequest.engineat Line 89, so an unknown engine fails validation instead of producing a plan for an engine that cannot run.
📍 Affects 1 file
backend/models.py#L230-L235(this comment)backend/models.py#L254-L264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/models.py` around lines 230 - 235, Update backend/models.py at lines
230-235 in PronunciationPreviewRequest to apply the same supported-language
pattern as GenerationRequest.language. Also update backend/models.py at lines
254-264 in ProsodyPreviewRequest to replace engine’s max_length-only validation
with the same engine pattern used by GenerationRequest.engine, keeping preview
validation aligned with generation.
Comment on lines
+380
to
+394
| gen_kwargs=dict( | ||
| language=data.language, | ||
| seed=data.seed, | ||
| instruct=data.instruct, | ||
| max_chunk_chars=data.max_chunk_chars, | ||
| crossfade_ms=data.crossfade_ms, | ||
| trim_fn=trim_fn, | ||
| runaway_detector=runaway_detector, | ||
| ), | ||
| db=db, | ||
| profile_id=data.profile_id, | ||
| supports_instruct=supports_instruct, | ||
| engine_languages=engine_langs, | ||
| seed=data.seed, | ||
| instruct=data.instruct, | ||
| max_chunk_chars=data.max_chunk_chars, | ||
| crossfade_ms=data.crossfade_ms, | ||
| trim_fn=trim_fn, | ||
| runaway_detector=runaway_detector, | ||
| enabled=data.prosody, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the optional defaults on GenerationRequest.
fd -t f 'models.py' backend --max-depth 1 --exec rg -n -A 40 'class GenerationRequest' {}Repository: jamiepine/voicebox
Length of output: 2189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Candidate files"
fd -t f 'generations.py|generation.py|models.py' backend
echo
echo "## backend/routes/generations.py relevant section"
sed -n '340,415p' backend/routes/generations.py
echo
echo "## backend/services/generation.py relevant sections"
sed -n '1,140p' backend/services/generation.py
echo
echo "## Search for generate_with_prosody definition/usages"
rg -n "def generate_with_prosody|generate_with_prosody|def generate_chunked|generate_chunked|crossfade_ms|max_chunk_chars" backendRepository: jamiepine/voicebox
Length of output: 18566
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## backend/services/prosody/pipeline.py relevant section"
sed -n '110,190p' backend/services/prosody/pipeline.py
echo
echo "## backend/services/prosody/renderer.py relevant section"
sed -n '180,310p' backend/services/prosody/renderer.py
echo
echo "## backend/utils/chunked_tts.py relevant section"
sed -n '170,325p' backend/utils/chunked_tts.py
echo
echo "## Behavioral probe: gen_kwargs with None values and defaults"
python3 - <<'PY'
def generate_chunked(**kwargs):
return {"called_generate_chunked": True, **kwargs}
gen_kwargs_none = dict(
language="en",
seed=None,
instruct=None,
max_chunk_chars=None, # optional Field default=800, but omitted by client becomes default 800 then None? Request says data.max_chunk_chars is optional.
crossfade_ms=None,
)
print("gen_kwargs_none_exists=", "crossfade_ms" in gen_kwargs_none, "->", gen_kwargs_none.get("crossfade_ms", 50))
print("gen_kwargs_none_max=", gen_kwargs_none.get("max_chunk_chars", 800))
gen_kwargs_missing = {k: v for k, v in gen_kwargs_none.items() if k not in ("crossfade_ms", "max_chunk_chars")}
print("gen_kwargs_missing_exists=", "crossfade_ms" in gen_kwargs_missing, "->", gen_kwargs_missing.get("crossfade_ms", 50))
print("gen_kwargs_missing_max=", gen_kwargs_missing.get("max_chunk_chars", 800))
PYRepository: jamiepine/voicebox
Length of output: 13230
Do not pass None values for max_chunk_chars or crossfade_ms into gen_kwargs.
generate_chunked requires non-None defaults, and these fields on GenerationRequest already default to 800 and 50. Omitting the keys lets generate_with_prosody use those defaults; passing None drops generate_chunked’s fallback and lets render(..., crossfade_ms=None) reach int(sr * crossfade_ms / 1000).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/generations.py` around lines 380 - 394, Update the gen_kwargs
construction in the generation route to omit max_chunk_chars and crossfade_ms
when their GenerationRequest values are None, allowing
generate_with_prosody/generate_chunked to apply the existing defaults. Preserve
passing explicitly provided values unchanged.
Comment on lines
+78
to
+86
| if language is None and profile_id is None: | ||
| q = db.query(PronunciationEntry) | ||
| if not include_disabled: | ||
| q = q.filter(PronunciationEntry.enabled.is_(True)) | ||
| return q.order_by(PronunciationEntry.term).all() | ||
|
|
||
| return pronunciation.get_entries( | ||
| db, language=language, profile_id=profile_id, include_disabled=include_disabled | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Order the filtered branch as well.
The unfiltered branch orders by term. The pronunciation.get_entries branch returns rows in database order, so the same endpoint returns a different, unspecified order as soon as a caller passes language or profile_id. A management screen then shows an unstable list.
♻️ Proposed change
- return pronunciation.get_entries(
+ entries = pronunciation.get_entries(
db, language=language, profile_id=profile_id, include_disabled=include_disabled
)
+ return sorted(entries, key=lambda e: e.term.lower())Alternatively, add the order_by inside get_entries so both branches and the prosody pipeline share one order.
📝 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.
Suggested change
| if language is None and profile_id is None: | |
| q = db.query(PronunciationEntry) | |
| if not include_disabled: | |
| q = q.filter(PronunciationEntry.enabled.is_(True)) | |
| return q.order_by(PronunciationEntry.term).all() | |
| return pronunciation.get_entries( | |
| db, language=language, profile_id=profile_id, include_disabled=include_disabled | |
| ) | |
| if language is None and profile_id is None: | |
| q = db.query(PronunciationEntry) | |
| if not include_disabled: | |
| q = q.filter(PronunciationEntry.enabled.is_(True)) | |
| return q.order_by(PronunciationEntry.term).all() | |
| entries = pronunciation.get_entries( | |
| db, language=language, profile_id=profile_id, include_disabled=include_disabled | |
| ) | |
| return sorted(entries, key=lambda e: e.term.lower()) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/routes/pronunciation.py` around lines 78 - 86, Update the filtered
path around pronunciation.get_entries so results are ordered by
PronunciationEntry.term consistently with the unfiltered branch. Apply the
ordering at the shared get_entries implementation if appropriate, preserving the
existing filtering and include_disabled behavior.
Comment on lines
+104
to
+118
| audio, sample_rate = await generate_with_prosody( | ||
| text, | ||
| engine=engine, | ||
| language=language, | ||
| generate_chunked_fn=generate_chunked, | ||
| tts_model=tts_model, | ||
| voice_prompt=voice_prompt, | ||
| gen_kwargs=gen_kwargs, | ||
| db=bg_db, | ||
| profile_id=profile_id, | ||
| supports_instruct=supports_instruct, | ||
| engine_languages=engine_langs, | ||
| seed=seed, | ||
| enabled=prosody, | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
regenerate loses its randomized seed on the multi-run path.
gen_kwargs["seed"] is set to None when mode == "regenerate", so a regeneration produces a new variation. Line 116 passes the original seed to generate_with_prosody, and plan.with_seeds(seed) then assigns derived seeds to every run. A script with markup or dictionary hits therefore regenerates deterministically and reproduces the previous take, while plain text still varies. Pass the same effective seed to both.
🐛 Proposed fix
+ effective_seed = seed if mode != "regenerate" else None
gen_kwargs: dict = dict(
language=language,
- seed=seed if mode != "regenerate" else None,
+ seed=effective_seed,
instruct=instruct,- seed=seed,
+ seed=effective_seed,
enabled=prosody,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/generation.py` around lines 104 - 118, Update the regenerate
flow around generate_with_prosody so it passes the same effective seed used in
gen_kwargs, rather than the original seed variable. Ensure mode == "regenerate"
remains randomized for both plain and multi-run inputs while preserving
deterministic behavior for other modes.
Comment on lines
+56
to
+59
| if self.strategy == LANGUAGE and self.spoken_language: | ||
| return f'<lang xml:lang="{self.spoken_language}">{matched}</lang>' | ||
| if self.strategy == PHONEME and self.phonemes and supports_phonemes: | ||
| return f'<phoneme alphabet="ipa" ph="{_escape(self.phonemes)}">{matched}</phoneme>' |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Escape spoken_language in the <lang> attribute.
Line 59 escapes phonemes with _escape, but line 57 interpolates self.spoken_language raw. spoken_language comes from a database row, so a value that contains " produces markup that either fails parse or changes the emitted attributes. Apply the same escaping on both branches.
🛡️ Proposed fix
if self.strategy == LANGUAGE and self.spoken_language:
- return f'<lang xml:lang="{self.spoken_language}">{matched}</lang>'
+ return f'<lang xml:lang="{_escape(self.spoken_language)}">{matched}</lang>'📝 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.
Suggested change
| if self.strategy == LANGUAGE and self.spoken_language: | |
| return f'<lang xml:lang="{self.spoken_language}">{matched}</lang>' | |
| if self.strategy == PHONEME and self.phonemes and supports_phonemes: | |
| return f'<phoneme alphabet="ipa" ph="{_escape(self.phonemes)}">{matched}</phoneme>' | |
| if self.strategy == LANGUAGE and self.spoken_language: | |
| return f'<lang xml:lang="{_escape(self.spoken_language)}">{matched}</lang>' | |
| if self.strategy == PHONEME and self.phonemes and supports_phonemes: | |
| return f'<phoneme alphabet="ipa" ph="{_escape(self.phonemes)}">{matched}</phoneme>' |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/prosody/annotate.py` around lines 56 - 59, Update the
LANGUAGE branch in the annotation method to pass self.spoken_language through
_escape before interpolating it into the xml:lang attribute, matching the
existing phonemes escaping while preserving the current output structure.
Comment on lines
+153
to
+169
| language = attrs.language or default_language | ||
| if ( | ||
| engine_languages | ||
| and language not in engine_languages | ||
| and language not in seen_unsupported_language | ||
| ): | ||
| seen_unsupported_language.add(language) | ||
| warnings.append( | ||
| PlanWarning( | ||
| code="language_unsupported", | ||
| detail=( | ||
| f"Engine {engine!r} cannot generate {language!r}; that run will be " | ||
| f"read as {default_language!r}." | ||
| ), | ||
| ) | ||
| ) | ||
| language = default_language |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
A repeated unsupported language keeps the unsupported code after the first run.
language = default_language on line 169 executes only inside the guarded block, and the guard includes language not in seen_unsupported_language. For the second and later runs in the same unsupported language, the condition is false, so Speech.language stays the unsupported code. The emitted warning states the run "will be read as" the default, so plan and warning disagree, and the renderer passes an unsupported language to the engine.
test_each_unsupported_language_warns_once in backend/tests/test_prosody_transformer.py (line 245) asserts only the warning count, so it does not catch this. Separate the de-duplication of the warning from the fallback.
🐛 Proposed fix
language = attrs.language or default_language
- if (
- engine_languages
- and language not in engine_languages
- and language not in seen_unsupported_language
- ):
- seen_unsupported_language.add(language)
- warnings.append(
- PlanWarning(
- code="language_unsupported",
- detail=(
- f"Engine {engine!r} cannot generate {language!r}; that run will be "
- f"read as {default_language!r}."
- ),
- )
- )
+ if engine_languages and language not in engine_languages:
+ if language not in seen_unsupported_language:
+ seen_unsupported_language.add(language)
+ warnings.append(
+ PlanWarning(
+ code="language_unsupported",
+ detail=(
+ f"Engine {engine!r} cannot generate {language!r}; that run will be "
+ f"read as {default_language!r}."
+ ),
+ )
+ )
language = default_languageAlso extend the test to assert every run's language:
def test_each_unsupported_language_warns_once():
p = plan(
'a <lang xml:lang="sw">x y z</lang> b <lang xml:lang="sw">p q r</lang>',
engine_languages=["en"],
)
assert codes(p).count("language_unsupported") == 1
assert all(n.language == "en" for n in p.nodes if isinstance(n, Speech))📝 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.
Suggested change
| language = attrs.language or default_language | |
| if ( | |
| engine_languages | |
| and language not in engine_languages | |
| and language not in seen_unsupported_language | |
| ): | |
| seen_unsupported_language.add(language) | |
| warnings.append( | |
| PlanWarning( | |
| code="language_unsupported", | |
| detail=( | |
| f"Engine {engine!r} cannot generate {language!r}; that run will be " | |
| f"read as {default_language!r}." | |
| ), | |
| ) | |
| ) | |
| language = default_language | |
| language = attrs.language or default_language | |
| if engine_languages and language not in engine_languages: | |
| if language not in seen_unsupported_language: | |
| seen_unsupported_language.add(language) | |
| warnings.append( | |
| PlanWarning( | |
| code="language_unsupported", | |
| detail=( | |
| f"Engine {engine!r} cannot generate {language!r}; that run will be " | |
| f"read as {default_language!r}." | |
| ), | |
| ) | |
| ) | |
| language = default_language |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/prosody/compiler.py` around lines 153 - 169, In the language
normalization flow around engine_languages and seen_unsupported_language, always
assign default_language whenever the requested language is unsupported, while
retaining seen_unsupported_language only to suppress duplicate warnings. Extend
test_each_unsupported_language_warns_once to assert every Speech node uses the
default language, such as "en", in addition to checking the single warning.
Comment on lines
+160
to
+169
| for attempt in range(1, max_attempts + 1): | ||
| try: | ||
| raw = await backend.generate( | ||
| prompt=prompt, | ||
| system=SYSTEM_PROMPT, | ||
| max_tokens=min(2048, len(text) * 2 + 256), | ||
| temperature=TEMPERATURE, | ||
| model_size=model_size, | ||
| examples=_EXAMPLES, | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)backend/services/prosody/llm_annotate.py$|(^|/)backend/routes/prosody.py$|backend/.+generate.*\.(py)$|backend/.+llm.*\.py$' || true
echo "== llm_annotate outline/section =="
if [ -f backend/services/prosody/llm_annotate.py ]; then
wc -l backend/services/prosody/llm_annotate.py
ast-grep outline backend/services/prosody/llm_annotate.py || true
echo "== llm_annotate relevant =="
sed -n '1,260p' backend/services/prosody/llm_annotate.py | cat -n
fi
echo "== generate/backend definitions usages =="
rg -n "async def generate|def generate|generate\\(" backend -S || true
echo "== prosody route relevant =="
if [ -f backend/routes/prosody.py ]; then
wc -l backend/routes/prosody.py
ast-grep outline backend/routes/prosody.py || true
rg -n "llm_annotate|process|annotate|wait_for|timeout|max_attempts|generate" backend/routes/prosody.py -S -C 4 || true
sed -n '1,260p' backend/routes/prosody.py | cat -n
fiRepository: jamiepine/voicebox
Length of output: 24859
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect backend definitions =="
for f in backend/backends/__init__.py backend/backends/qwen_llm_backend.py backend/services/llm.py backend/services/personality.py backend/services/refinement.py backend/services/prosody/pipeline.py backend/backends/mlx_backend.py backend/backends/chatterbox_backend.py backend/backends/hume_backend.py backend/backends/pytorch_backend.py backend/backends/kokoro_backend.py backend/backends/qwen_custom_voice_backend.py backend/backends/chatterbox_turbo_backend.py backend/backends/luxtts_backend.py backend/routes/llm.py; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
rg -n "async def generate|def generate|generate\\(|model_size|examples|system|max_tokens|timeout|llm_generate|get_llm_model" "$f" -S || true
fi
done
echo "== behavioral token-size verifier =="
python3 - <<'PY'
import math
sizes = [100, 500, 1000, 1500, 2000, 3000, 5000, 10000]
for n in sizes:
cap = min(2048, n * 2 + 256)
can_recover = n <= cap
print(f"input_tokens={n:5d} max_tokens={cap:5d} can_recover={can_recovery}")
PYRepository: jamiepine/voicebox
Length of output: 23699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== token-size verifier =="
python3 - <<'PY'
for n in [100, 500, 1000, 1500, 2000, 3000, 4000, 5000, 10000]:
cap = min(2048, n * 2 + 256)
ok = n <= cap
print(f"input_tokens={n:5d} max_tokens={cap:5d} output_tokens_within_cap_if_needs_approx_2x={ok}")
PY
echo "== inspect call sites and route timeouts =="
for f in backend/backends/__init__.py backend/tests/test_prosody_llm.py backend/routes/llm.py backend/services/personality.py backend/services/refinement.py; do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n "wait_for|timeout|asyncio|generate\\(" "$f" -S -C 3 || true
done
echo "== inspect LLM route request model for timeout input =="
rg -n "LLMGenerateRequest|max_tokens|timeout|system|examples|model_size" backend/backend models backend/models.py backend -g '*models*' -S | head -80 || true
fd -a 'models.py' backend | while read -r f; do
echo "-- $f"
rg -n "class LLMGenerateRequest|timeout|max_tokens|system|examples|model_size" "$f" -S || true
doneRepository: jamiepine/voicebox
Length of output: 14064
Bound the LLM annotation call.
annotate_prosody awaits annotate_with_llm directly, and annotate_with_llm awaits backend.generate without any timeout. A stalled local model can keep the /prosody/annotate request open indefinitely; wrap the annotation request with asyncio.wait_for, or add a route/service timeout that applies to the local LLM path.
The generation style guide also says backends should not be called directly from routes, so an intermediate service that owns the generate call and its timeout is preferable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/prosody/llm_annotate.py` around lines 160 - 169, Bound the
local LLM generation in annotate_with_llm by applying an asyncio.wait_for
timeout around backend.generate, using the service-level timeout configuration
if one exists and handling timeout failures through the existing retry/error
path. Keep route-level behavior unchanged and preserve the intermediate service
ownership of the backend call.
Comment on lines
+38
to
+42
| _TAG_RE = re.compile( | ||
| r"<\s*(?P<closing>/)?\s*(?P<name>" + "|".join(sorted(_ALL_TAGS)) + r")" | ||
| r"(?P<attrs>[^<>]*?)(?P<void>/)?\s*>", | ||
| re.IGNORECASE, | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tag names match word prefixes in both prosody regexes. Neither _TAG_RE nor _STRIP_RE requires a separator after the tag name, so [^<>]*? absorbs the remainder of a longer word. "<breaking> news" becomes a 700 ms silence, and "<language>" raises a parse error, which contradicts the closed-tag-set guarantee in the module docstring.
backend/services/prosody/parser.py#L38-L42: add a boundary such as(?![\w:.-])after thenamegroup.backend/services/prosody/parser.py#L202-L204: add the same boundary, sostrip_markupandparseagree on what a tag is; the LLM invariant inllm_annotate.validate_annotationdepends on that agreement.backend/tests/test_prosody_transformer.py#L54-L69: add"<breaking> news"and"<language> barrier"to the parametrize list.
📍 Affects 2 files
backend/services/prosody/parser.py#L38-L42(this comment)backend/services/prosody/parser.py#L202-L204backend/tests/test_prosody_transformer.py#L54-L69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/prosody/parser.py` around lines 38 - 42, Prevent both
prosody regexes, _TAG_RE and _STRIP_RE, from matching tag-name prefixes by
requiring a boundary after the name that excludes word, colon, dot, and hyphen
characters; update backend/services/prosody/parser.py lines 38-42 and 202-204
consistently. Add "<breaking> news" and "<language> barrier" to the
parameterized cases in backend/tests/test_prosody_transformer.py lines 54-69 to
verify markup stripping and parsing agree.
Comment on lines
+82
to
+89
| if db is not None: | ||
| from ..pronunciation import get_entries | ||
|
|
||
| entries = get_entries(db, language=language, profile_id=profile_id) | ||
| if entries: | ||
| markup, applied = annotate(text, rules_from_entries(entries)) | ||
| if applied: | ||
| logger.info("Dictionary annotated %d term(s)", len(applied)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
No caller forwards engine phoneme capability into annotate. annotate exposes supports_phonemes, and rule.realise() uses it to decide whether a phoneme strategy dictionary entry emits phoneme markup. Both call sites omit the argument, so every entry is realised as if no engine supports phonemes, and the phoneme strategy never takes effect.
backend/services/prosody/pipeline.py#L82-L89: resolve the phoneme capability forengine(next tosupports_instructinengine_capabilities) and pass it toannotate.backend/routes/prosody.py#L43-L46: move theengine_capabilities(data.engine)call aboveannotateand pass the resolved phoneme capability so the preview matches generation.
📍 Affects 2 files
backend/services/prosody/pipeline.py#L82-L89(this comment)backend/routes/prosody.py#L43-L46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/prosody/pipeline.py` around lines 82 - 89, The annotate
callers omit the engine’s phoneme capability, preventing phoneme strategy
entries from producing phoneme markup. In backend/services/prosody/pipeline.py
lines 82-89, resolve the phoneme capability for engine alongside
supports_instruct via engine_capabilities and pass it to annotate; in
backend/routes/prosody.py lines 43-46, move engine_capabilities(data.engine)
before annotate and pass the resolved capability so preview and generation
behave consistently.
Comment on lines
+119
to
+121
| @pytest.mark.asyncio | ||
| async def test_a_fenced_answer_is_unwrapped(stub_llm): | ||
| stub_llm(f"```xml\n{GOOD}\n```") |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the pytest-asyncio dependency and asyncio_mode configuration.
fd -H -t f 'pyproject.toml|setup.cfg|pytest.ini|tox.ini|requirements*.txt' --exec rg -n 'pytest-asyncio|asyncio_mode|\[tool.pytest' {} \;Repository: jamiepine/voicebox
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate config files =="
fd -H -t f 'pyproject.toml|setup.cfg|pytest.ini|tox.ini|requirements*.txt|*requirements*.txt' . | sed 's#^\./##' | sort
echo
echo "== pytest/tool related matches in candidate config files =="
for f in $(fd -H -t f 'pyproject.toml|setup.cfg|pytest.ini|tox.ini|requirements*.txt|*requirements*.txt' . | sort); do
echo "--- $f"
rg -n 'pytest-asyncio|pytest_asyncio|asyncio_mode|\[tool\.pytest|\[pytest|\[tox|testenv|^pytest' "$f" || true
done
echo
echo "== installed pytest plugin availability =="
python3 - <<'PY'
import importlib
mods = ["pytest_asyncio", "pytest"]
for m in mods:
try:
mod = importlib.import_module(m)
print(m, getattr(mod, "__version__", "no __version__"))
except Exception as e:
print(m, "missing:", type(e).__name__, e)
PYRepository: jamiepine/voicebox
Length of output: 1272
🏁 Script executed:
#!/bin/bash
set -u
echo "== find config/dependency files =="
fd 'pyproject\.toml|setup\.cfg|pytest\.ini|tox\.ini|requirements.*\.txt' . || true
echo
echo "== search pytest-asyncio / pytest / asyncio_mode in repository files =="
rg -n --glob 'pyproject.toml' --glob 'setup.cfg' --glob 'pytest.ini' --glob 'tox.ini' --glob 'requirements*.txt' \
'pytest-asyncio|pytest_asyncio|asyncio_mode|\[tool\.pytest|\[pytest|\[tox|testenv|deps|devDependencies|dependency|devDependency' . || true
echo
echo "== relevant pyproject section =="
fd 'pyproject\.toml' . -x sh -c 'echo "--- $1"; sed -n "1,140p" "$1"' sh {}Repository: jamiepine/voicebox
Length of output: 3610
🏁 Script executed:
#!/bin/bash
set -u
echo "== backend pyproject dependency sections =="
fd '^pyproject\.toml$' . -x sh -c 'echo "--- $1"; sed -n "/^\[project\]/, /$/p; /^\[project\.optional-dependencies\]/, /$/p" "$1"' sh {}
echo
echo "== dependency references in backend pyproject and requirements =="
rg -n 'pytest|asyncio|dev|test|dependency|optional-dependencies|scripts|install_requires' ./backend/pyproject.toml ./backend/requirements.txt || true
echo
echo "== test markers/import pytest async usage in tests =="
rg -n '`@pytest`\.mark\.asyncio|pytest_asyncio|pytest-asyncio|import pytest' backend/tests || trueRepository: jamiepine/voicebox
Length of output: 5659
🏁 Script executed:
#!/bin/bash
set -u
echo "== backend pyproject.toml complete file =="
cat -n ./backend/pyproject.toml
echo
echo "== all tox/invoke/make/poetry/uv related files =="
fd '\.(toml|ini|cfg|yaml|yml|mk|py)$' . | rg 'tox|poetry|uv|makefile|justfile|nox|tasks' || trueRepository: jamiepine/voicebox
Length of output: 3439
Declare pytest-asyncio explicitly.
backend/pyproject.toml sets asyncio_mode = "auto", but it does not declare pytest-asyncio under [project], so the @pytest.mark.asyncio tests may run in environments without a transient pytest plugin installed. Add an explicit test dependency, for example via [project.optional-dependencies.test], if supported by the package build flow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/tests/test_prosody_llm.py` around lines 119 - 121, Add pytest-asyncio
as an explicit test dependency in the project configuration, preferably under
[project.optional-dependencies.test] if supported by the existing package build
flow. Keep the asyncio_mode configuration and `@pytest.mark.asyncio` usage in
test_a_fenced_answer_is_unwrapped unchanged.
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
1 participant
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.
Builds on #1035 (the prosody library) and #1025 (the pronunciation dictionary). This is the half that changes behaviour, kept separate so #1035 can be judged without it.
Stacking: this branch carries #1025's three commits and sits on #1035's six, so it shows both until they merge. Merge order #1035 → #1025 → this, though nothing breaks in another order.
The dictionary becomes a source of markup
An entry no longer has a private substitution pass. It emits the same directive an author would have typed:
and the ordinary parse–compile–render pipeline takes it from there. That buys three things at once: dictionary terms and hand-written spans compose, because by the time the compiler sees them they are the same thing; every rule the compiler enforces applies to dictionary output for free; and the result is showable, so a rule firing unexpectedly is visible rather than an invisible gap between what was typed and what was spoken.
Three strategies, with
replacementalways populated as the fallback when the preferred one is unavailable on the target engine:respell<sub alias="bandeha">language<lang xml:lang="es">phoneme<phoneme ph="…">A strategy that cannot be realised is rejected at the API rather than stored —
strategy="language"with nospoken_languagewould show the user the strategy they picked while silently falling back.LLM annotation, never in the render path
The local LLM reads a script and returns the same script with directives inserted. From there the deterministic pipeline runs exactly as for hand-typed markup — which is what keeps "with an LLM" and "without an LLM" the same code path downstream, and why generation stays reproducible. A model in the render path would make the same script produce different audio every run.
The invariant that makes it safe: strip the tags from the model's answer and compare to the input. If a single word moved, the model rewrote the script instead of annotating it and the answer is discarded. It can fail to help; it cannot mangle.
Small models wrap answers in fences and quotes. Rather than guess which, every plausible unwrapping is offered and the invariant picks the first faithful one — so no unwrapping can launder a changed word into acceptance.
Without an LLM it degrades, not breaks. A missing model is reported, never downloaded on demand;
GET /prosody/annotate/availabilitylets a client hide the action rather than offer something that will fail. Annotation is typing assistance — everything else works without it.Wiring
POST /generateand/generate/streamnow resolve dictionary entries and markup before synthesis, through one shared function so "unmarked text behaves as before" is a property of a single place rather than a claim repeated twice.The property under most scrutiny is the one about not changing anything:
5 < 6stays literal — the parser only knows a closed tag set;Auto-detected rather than opt-in, with
prosody: falseas the escape hatch for a script that genuinely contains something tag-shaped. An opt-in flag would have meant updating every caller — MCP, API, story regeneration — or leaving the feature invisible.The stored row keeps the markup, not the resolved text, consistent with #1025 and #1026: the resolved form is derivable, the author's is not, and editing markup to regenerate needs it intact.
POST /prosody/previewCompiles a script to its plan with no audio: every cut, language and silence, which dictionary terms fired, the markup they resolved to, and everything the chosen engine cannot honour. Free, because the plan is plain data.
Two bugs found while building it
is_trivialexcluded any plan carrying a substitution, so every respelled sentence would have taken the renderer path for nothing — contradicting the property that makes respelling preferred, that it does not cut.The migration guard for
uq_pronunciation_scopenever fired: SQLAlchemy cannot reflect an expression-based index, so the inspector never reported it and the dedup scan ran on every startup. Readssqlite_masterdirectly now.Tests
209 on top of #1035's 93 — annotator, LLM invariant with the model stubbed, preview, and the pipeline integration whose largest section asserts that unmarked text is untouched.
No UI yet, deliberately: dictionary and prosody are API-only so the model and the matching rules can be argued about before anything is built on them.
🤖 Generated with Claude Code
Summary by CodeRabbit