Repository navigation
fix: drop the leading space on Latin sentence timestamps - #3866
Conversation
timestamp_sentence inserts a space before an English word but, unlike timestamp_sentence_en, never removes a leading space. A Chinese utterance that continues in English was stored as " iphone pro max.". Signed-off-by: Ting Zhang <181050363+GPT-AGI@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
LauraGPT
left a comment
There was a problem hiding this comment.
Reviewed exact head 245ee66ac8faa3413c2d03a964c91718c93cdc15 against its original base 20f26b7c4905d010768ec91206149df2a90104b6.
- Baseline with the new regression fails on the synthetic leading space (
' iphone pro max。'). - Exact head: 9/9 timestamp-tool tests pass, covering Latin sentence starts, mixed Chinese/Latin text, all-English/all-Chinese controls, raw text, punctuation, and boundaries.
No exact-head hosted check currently covers this PR, and main has advanced since the tested base. Leaving it open pending current-base revalidation plus hosted coverage. Scope is deterministic CPU text/timestamp postprocessing only; no model, GPU, ASR-quality, or release claim.
Retain exact sentence, raw-text and timestamp assertions for PR modelscope#3866. The corrected expectation fails on the old implementation and passes on the fixed current-base tree; the complete NumPy CI selection passed privately (421 passed, 27 CUDA-only skips). Signed-off-by: LauraGPT <zhifu.gzf@alibaba-inc.com>
LauraGPT
left a comment
There was a problem hiding this comment.
Maintainer validation of corrected head e7e633a2d3d6d304ba4c8b57afc7b7d359a4c19e on the main containing #3863:
- Appended only the stale VAD sentence expectation correction, preserving exact strings, timestamp offsets/boundaries and raw text. The original implementation fails that updated expectation; the corrected implementation passes. Additional commit is verified signed and DCO-signed-off; original commits/authorship preserved.
- New exact-head NumPy 1.26.4 and 2.4.0 jobs both pass: 421 passed and 27 CUDA-only skips each. Actual artifact ZIP digests, source-commit files and JUnit were verified. The old failed run was not rerun.
- The combined tree with #3863 passed the complete union of both CI selections: 468 passed, 33 skips. The first private run had one fresh-process pytorch_wpe import failure (467 passed); pointing only that isolated process's PYTHONUSERBASE to the previously installed private dependency resolved it. No product source, test assertion, shared environment or model weights changed for that repair.
- These are deterministic CPU text/postprocessing and related integration tests, not GPU inference or ASR-quality claims. The prior repository-wide offline attempt remains unpassed at online model lookup and timeout; no full-suite result is asserted.
Code, discussions, complete protection/rules, exact head/main and combined tested-tree equality checked. GitHub's temporary merge reference retained the previous main; an independent file-level three-way merge has no overlapping changed paths and exactly matches the tested combined tree. The normal merge result must match that tree and both actual parents. Normal merge only, no self-approval or admin bypass.
Summary
Input:
timestamp_sentenceinserts a separator before each Latin token, including thefirst token of a sentence. Drop that one leading separator using the same check
as
timestamp_sentence_en; internal English spaces remain unchanged.Type of change
Validation
python -m pytest -q tests/test_timestamp_tools.py— 9 passedpython -m compileall -q funasr examples testsUser impact
sentence_infotext no longer starts with a synthetic space when a sentencestarts with a Latin word.
Notes for reviewers
Four before/after controls covered a Latin-starting sentence, Latin text within
Chinese, all-English, and all-Chinese input. Sentence boundaries, timestamps,
punctuation, and
raw_textwere unchanged.