feat(audio): add transcript normalization stages - #2267
Conversation
4f06835 to
f45fc2f
Compare
mohammadaaftabv
changed the title
7fc15d1 to
cb63d2b
Compare
Greptile SummaryThe PR adds YAML-driven transcript regex substitution and language-aware abbreviation concatenation stages, including public exports and focused tests.
Confidence Score: 4/5The PR is not yet safe to merge because supported-language transcripts containing non-ASCII uppercase vowel-plus-s tokens can still be normalized incorrectly. The abbreviation stage still classifies vowels through an ASCII-only set, so inputs such as German Files Needing Attention: nemo_curator/stages/audio/text_filtering/abbreviation_concat.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[AudioTask pred_text] --> B[RegexSubstitutionStage]
B --> C[text]
C --> D[AbbreviationConcatStage]
D --> E[Normalized text and notes]
B --> F{Empty after cleaning?}
F -->|Yes| G[Set skip reason]
Reviews (7): Last reviewed commit: "fix(audio): address normalization review..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _pattern(language: str) -> re.Pattern[str]: | ||
| char_class = _LANG_CHAR_CLASS.get(language, _LANG_CHAR_CLASS["en"]) | ||
| return re.compile( | ||
| rf"(?<![\w’’’ʼ])({char_class}(?: {char_class}){{1,}}(?:(?<=[A-Z])s)?)(?!\w)" # noqa: RUF001 |
There was a problem hiding this comment.
When a supported-language abbreviation contains non-ASCII uppercase letters and a consonant plural token, the ASCII-only [A-Z] condition can match only a suffix of the sequence, producing partially normalized transcript text instead of joining the complete abbreviation.
Knowledge Base Used: Audio Stages
| for index, rule in enumerate(raw_rules): | ||
| if not isinstance(rule, dict) or "pattern" not in rule or "repl" not in rule: | ||
| msg = f"Regex rule {index} must define pattern and repl" | ||
| raise ValueError(msg) | ||
| re.compile(str(rule["pattern"])) | ||
| self._rules = raw_rules |
There was a problem hiding this comment.
YAML setup validates key presence and regex syntax but not repl or count types, so an invalid count fails later during transcript processing while a null replacement silently inserts the literal string None. Validate these values once during worker setup to reject malformed normalization configuration before processing data.
Knowledge Base Used: Audio Stages
| len(parts) >= _MIN_PARTS | ||
| and len(parts[-1]) == _PLURAL_SUFFIX_LEN | ||
| and parts[-1][1] == "s" | ||
| and (parts[-1][0] in _VOWELS or not parts[-1][0].isupper()) |
There was a problem hiding this comment.
When a supported-language abbreviation ends with a non-ASCII uppercase vowel-plus-s token such as German A Ös, the ASCII-only _VOWELS set classifies Ös as a consonant plural and emits AÖs instead of preserving A Ös, corrupting the normalized transcript.
Knowledge Base Used: Audio Stages
|
@nithinraok, could you please confirm which of the following remaining behavioural differences from Comparison anchors PR head: 3977493
Please confirm whether all six differences are acceptable, or identify the item numbers that should be aligned with the reference. |
| [ | ||
| ("A P I and G P U", ["API", "GPU"]), | ||
| ("a A P I", ["a API"]), | ||
| ("A P Is", ["AP I"]), |
There was a problem hiding this comment.
APIs is the more natural expectation here. However, the current test was matching nithinraok/Curator:nkoluguri/integration-test.
The reference treats terminal Is as a separate word, producing AP Is, and reports it using replaced.strip().rstrip("’s").rstrip("’s"), which produces AP I because rstrip() treats its argument as a character set rather than an exact suffix.
@nkoluguri, can you confirm whether we should intentionally diverge from the reference and make both the normalized text and reported abbreviation APIs, or retain the reference behavior?
There was a problem hiding this comment.
It should be APIs as correctly pointed by Viraj.
| def outputs(self) -> tuple[list[str], list[str]]: | ||
| return [], [self.output_text_key, self.notes_key] | ||
|
|
||
| def process(self, task: AudioTask) -> AudioTask: |
There was a problem hiding this comment.
Do we want to have process_batch as well?
| result = re.sub(r"\s+", " ", result).strip() | ||
| task.data[self.output_text_key] = result | ||
| if not result and had_text: | ||
| task.data[self.skip_me_key] = "Empty after regex cleaning" |
There was a problem hiding this comment.
Can we check is _skipme flag was already there and not to overwrite it?
3977493 to
a55b89a
Compare
|
/ok to test a55b89a |
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
Signed-off-by: aaftaabv@gmail.com <aaftaabv@gmail.com>
a55b89a to
47d7a38
Compare
|
/ok to test 47d7a38 |
5 participants
Summary
pred_text→text) with strict setup-time rule validation, whitespace normalization, skip preservation, and empty-after-cleaning handling.nemo_curator.stages.audio.text_filteringand the lazynemo_curator.stages.audioAPI.process()and inherited batch processing.Validation
Includes and supersedes #2268.