Add Portuguese normalization for gladia-3 WER scoring.#30
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds a new Portuguese language module to the normalization package, including clitic merging, written-number normalization via text2num, word/sentence replacement dictionaries, a ChangesPortuguese Language Normalization
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant PortugueseOperators
participant PortugueseNumberNormalizer
participant alpha2digit
Caller->>PortugueseOperators: normalize(text)
PortugueseOperators->>PortugueseOperators: expand_contractions (merge_hyphenated_clitics)
PortugueseOperators->>PortugueseNumberNormalizer: expand_written_numbers(text)
PortugueseNumberNormalizer->>alpha2digit: alpha2digit(text, "pt")
alpha2digit-->>PortugueseNumberNormalizer: converted text
PortugueseNumberNormalizer-->>PortugueseOperators: normalized numbers
PortugueseOperators->>PortugueseOperators: fix_one_word_in_numeric_contexts
PortugueseOperators->>PortugueseOperators: get_word_replacements (PORTUGUESE_REPLACEMENTS)
PortugueseOperators-->>Caller: normalized text
Possibly related PRs
Suggested reviewers: 🚥 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 |
egenthon-cmd
force-pushed
the
feat/portuguese-normalization
branch
from
d115d4d to
6cc7867
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
normalization/languages/portuguese/clitics.py (1)
9-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
"lhes"in_CLITIC_SUFFIXES.
"lhes"appears at both Line 16 and Line 25. The regex alternation will match the first occurrence, so this is functionally harmless, but the duplicate should be removed for clarity.♻️ Remove duplicate entry
"lhe", - "lhes", "lhos", "los", "las", "no", "na", "lho", "lha", - "lhes", )🤖 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 `@normalization/languages/portuguese/clitics.py` around lines 9 - 26, The _CLITIC_SUFFIXES tuple in clitics.py contains a duplicate "lhes" entry, so remove the repeated value and keep only one occurrence. Update the _CLITIC_SUFFIXES definition directly to preserve the intended suffix list and keep the regex alternation clear and maintainable.
🤖 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 `@tests/unit/languages/portuguese_operators_test.py`:
- Around line 40-45: The assertions in portuguese_operators_test.py are
tautological because `pipeline.normalize(...)` is compared against itself, so
they never verify that meaningful affirmatives/idioms survive normalization.
Update the tests around the existing `pipeline.normalize` calls to compare each
input string against an explicit expected normalized result (or an expected
output fixture), using the same test cases for “claro estou”, “esta tudo bem”,
and “e pronto foi assim” so the `normalize` behavior is actually validated.
---
Nitpick comments:
In `@normalization/languages/portuguese/clitics.py`:
- Around line 9-26: The _CLITIC_SUFFIXES tuple in clitics.py contains a
duplicate "lhes" entry, so remove the repeated value and keep only one
occurrence. Update the _CLITIC_SUFFIXES definition directly to preserve the
intended suffix list and keep the regex alternation clear and maintainable.
🪄 Autofix (Beta)
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
Run ID: bd9e175d-0105-472e-9fa5-96e2df635674
⛔ Files ignored due to path filters (1)
tests/e2e/files/gladia-3/pt.csvis excluded by!**/*.csv
📒 Files selected for processing (9)
normalization/languages/__init__.pynormalization/languages/portuguese/__init__.pynormalization/languages/portuguese/clitics.pynormalization/languages/portuguese/number_normalizer.pynormalization/languages/portuguese/operators.pynormalization/languages/portuguese/replacements.pynormalization/languages/portuguese/sentence_replacements.pytests/unit/languages/portuguese_number_normalizer_test.pytests/unit/languages/portuguese_operators_test.py
| # Meaningful affirmatives / idioms must survive normalization | ||
| assert pipeline.normalize("claro estou") == pipeline.normalize("claro estou") | ||
| assert pipeline.normalize("esta tudo bem") == pipeline.normalize("esta tudo bem") | ||
| assert pipeline.normalize("e pronto foi assim") == pipeline.normalize( | ||
| "e pronto foi assim" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tautological assertions provide no verification.
Lines 41, 42, and 43–45 compare pipeline.normalize(x) with pipeline.normalize(x) using the same input on both sides. Since normalize is deterministic, these reduce to x == x and will always pass — even if "claro", "bem", or "pronto" were incorrectly stripped as fillers. The comment at Line 40 states the intent ("Meaningful affirmations / idioms must survive normalization") but the tests do not verify that.
🐛 Proposed fix: assert against expected output
# Meaningful affirmatives / idioms must survive normalization
- assert pipeline.normalize("claro estou") == pipeline.normalize("claro estou")
- assert pipeline.normalize("esta tudo bem") == pipeline.normalize("esta tudo bem")
- assert pipeline.normalize("e pronto foi assim") == pipeline.normalize(
- "e pronto foi assim"
- )
+ assert pipeline.normalize("claro estou") == "claro estou"
+ assert pipeline.normalize("esta tudo bem") == "esta tudo bem"
+ assert pipeline.normalize("e pronto foi assim") == "e pronto foi assim"📝 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.
| # Meaningful affirmatives / idioms must survive normalization | |
| assert pipeline.normalize("claro estou") == pipeline.normalize("claro estou") | |
| assert pipeline.normalize("esta tudo bem") == pipeline.normalize("esta tudo bem") | |
| assert pipeline.normalize("e pronto foi assim") == pipeline.normalize( | |
| "e pronto foi assim" | |
| ) | |
| # Meaningful affirmatives / idioms must survive normalization | |
| assert pipeline.normalize("claro estou") == "claro estou" | |
| assert pipeline.normalize("esta tudo bem") == "esta tudo bem" | |
| assert pipeline.normalize("e pronto foi assim") == "e pronto foi assim" |
🤖 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 `@tests/unit/languages/portuguese_operators_test.py` around lines 40 - 45, The
assertions in portuguese_operators_test.py are tautological because
`pipeline.normalize(...)` is compared against itself, so they never verify that
meaningful affirmatives/idioms survive normalization. Update the tests around
the existing `pipeline.normalize` calls to compare each input string against an
explicit expected normalized result (or an expected output fixture), using the
same test cases for “claro estou”, “esta tudo bem”, and “e pronto foi assim” so
the `normalize` behavior is actually validated.
Introduce a dedicated pt language pack with colloquial variants, clitic merging, written-number expansion, and Orbio call-center oriented rules (fillers, oral prepositions, gender/spelling variants, brand names). Co-authored-by: Cursor <cursoragent@cursor.com>
egenthon-cmd
force-pushed
the
feat/portuguese-normalization
branch
from
6cc7867 to
b3361e5
Compare
1 participant
Introduce a dedicated pt language pack with colloquial variants, clitic merging, written-number expansion, and Orbio call-center oriented rules (fillers, oral prepositions, gender/spelling variants, brand names).
What does this PR do?
Type of change
Checklist
Only fill in the section(s) that match your change — delete the rest.
New language
normalization/languages/{lang}/withoperators.py,replacements.py,__init__.pyreplacements.py(not hardcoded inoperators.py)LanguageConfigis filled in with the language's data (separators, currency words, digit words, …)LanguageOperators— only override methods where the logic changes, not just the data@register_languageand imported innormalization/languages/__init__.pytests/unit/languages/tests/e2e/files/{preset}/{lang}.csv(e.g.tests/e2e/files/gladia-3/fr.csv)Edit existing language
replacements.py, not inline inoperators.pyNone: the step reading it still handlesNonegracefullyNew step
nameclass attribute set (this is the key used in YAML presets)@register_stepand imported insteps/text/__init__.pyorsteps/word/__init__.pyoperators.config.*insteadsteps/text/placeholders.pyandpipeline/base.py'svalidate()is updatedtests/unit/steps/uv run scripts/generate_step_docs.pyEdit existing step
nameis unchanged — if the output changes, create a new step name + new preset insteaduv run scripts/generate_step_docs.pyPreset change
pipeline.validate()passes (runs automatically vialoader.py)How was this tested?
Summary by CodeRabbit
New Features
Bug Fixes