fix: lyrics language verdicts without destructive default repair - #141
Merged
Conversation
Empty vocal lyrics and unsupported languages are invalid or unevaluable, never a silent pass. Repair returns a proposal and keeps the original. Protected spans must appear exactly. Shared Python/TS corpus.
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.8/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.0 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
Owner
Author
|
cursor review |
1 similar comment
Owner
Author
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Protected spans skipped when unevaluable
- Moved the language-independent protected-span presence check before unknown/unsupported language early returns in both Python and TypeScript validators so a missing required span is invalid even for French, Estonian, or unrecognized codes.
Or push these changes by commenting:
@cursor push 8ee83b95bb
Preview (8ee83b95bb)
diff --git a/app/services/lyrics_language.py b/app/services/lyrics_language.py
--- a/app/services/lyrics_language.py
+++ b/app/services/lyrics_language.py
@@ -239,6 +239,14 @@
reasons=["A vocal song must contain lyrics."],
)
+ missing = [span for span in protected if span not in text]
+ if missing:
+ return _report(
+ verdict="invalid",
+ lyrics=text,
+ reasons=["A required verbatim span is missing from the lyric."],
+ )
+
code = canonical_lyrics_language(lyrics_language)
if not code:
return _report(
@@ -253,14 +261,6 @@
reasons=[f"Language {code!r} is not scored by this guard."],
)
- missing = [span for span in protected if span not in text]
- if missing:
- return _report(
- verdict="invalid",
- lyrics=text,
- reasons=["A required verbatim span is missing from the lyric."],
- )
-
masked, _present = _mask_protected(text, protected)
sample = _strip_section_tags(masked)
sample = PROTECTED_TOKEN_RE.sub(" ", sample)
diff --git a/ui/src/lib/lyricsLanguageGuard.ts b/ui/src/lib/lyricsLanguageGuard.ts
--- a/ui/src/lib/lyricsLanguageGuard.ts
+++ b/ui/src/lib/lyricsLanguageGuard.ts
@@ -165,15 +165,15 @@
if (!text.trim()) {
return report('invalid', text, ['A vocal song must contain lyrics.'])
}
+ const protectedList = protectedTexts(options.protectedSegments)
+ if (protectedList.some(span => !text.includes(span))) {
+ return report('invalid', text, ['A required verbatim span is missing from the lyric.'])
+ }
const code = canonicalLyricsLanguage(lyricsLanguage)
if (!code) return report('unevaluable', text, ['The requested lyrics language is not recognized.'])
if (!SCORED.has(code)) {
return report('unevaluable', text, [`Language '${code}' is not scored by this guard.`])
}
- const protectedList = protectedTexts(options.protectedSegments)
- if (protectedList.some(span => !text.includes(span))) {
- return report('invalid', text, ['A required verbatim span is missing from the lyric.'])
- }
const masked = maskProtected(text, protectedList)
const sample = stripTags(masked).replace(/\{\{PROTECTED_\d+\}\}/g, ' ')
let reasons: string[] = []You can send follow-ups to the cloud agent here.
Verbatim presence is language-independent. Returning unevaluable first hid missing required spans when lyrics_language was unsupported.
Owner
Author
|
cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b1a2c3d. Configure here.
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.
Resumen ejecutivo
Qué cambia
El validador de letras distingue
valid/invalid/unevaluable.No da por buena una letra vocal vacía ni un idioma sin soporte (francés,
estonio). La reparación ya no pisa el original: propone un diff.
Para qué sirve
#137/#139 cubrían contaminación española y alias. Seguían pasando en silencio
casos vacíos o no evaluables, y
repairpodía dejar una letra vacía como OK.Impacto para el usuario
Ninguno visible todavía: la librería no está cableada a Generate (fase 6).
Riesgo
Estado
Summary
Phase 2 remainder after merged #139. Library only. No launch / StoryLabPanel.
Validation
tests/test_lyrics_language.py: 20 passedbash scripts/validate_local.sh: passed (no GPU)d4263ce6Follow-up
Phase 6 wires the guard. Phase 3 is independent.
Base:
d4263ce6. Head:d58d05686b6d48abf251dcdf64c763c25670d18f.Generación real: NO EJECUTADA.
Note
Medium Risk
Contract and default behavior change for a shared validation library (verdicts, stricter empty/unsupported handling, non-destructive repair); impact is limited today because the guard is not yet wired into enqueue paths.
Overview
Tightens the provider-free lyrics language guard (Python + mirrored TS) so validation cannot silently pass empty vocals or unsupported languages, and repair no longer overwrites user text.
Reports now expose
verdict(valid|invalid|unevaluable);okis true only forvalid. Empty vocal lyrics are invalid; recognized but unscored languages (e.g. French, Estonian) are unevaluable instead of implied success. Onlyesandenare scored. Requiredprotected_segmentsmust appear verbatim (including newlines) before language checks; missing spans are invalid even when the language is unevaluable.repair_lyrics_languagekeepslyricsas the original and returnsproposal/proposal_diffsfor stripped foreign-script runs; a repair that would empty the vocal lyric stays invalid.assert_lyrics_languagedefaults torepair=False.Adds a shared JSON corpus exercised by Python and TS tests, plus updated
LYRICS_LANGUAGE.mdand phase-2 checklist notes. No wiring to Generate / Story Lab yet (phase 6).Reviewed by Cursor Bugbot for commit b1a2c3d. Configure here.