Repository navigation
Conversation
LauraGPT
left a comment
There was a problem hiding this comment.
Independently checked exact head 162ef07eb94fddd2ec0adca4892a6108bed46138 against base 14f12f1dc04418c3a2cd28e29623d1ad810c6d35. No blocking correctness finding in this scoped review.
- Ran the submitted timestamp and
AutoModel.inference_with_vadtests against the base implementation: 3 expected failures, 22 passed, 8 subtests passed. The failures reproduce the missing unpunctuated Chinese/English tail and the empty sentence list through the fallback integration path. - Same submitted tests on the PR implementation: 25 passed, 8 subtests passed.
- An independent deterministic matrix exercised both splitters, raw-text on/off, one to four tokens, and all no-punctuation/period combinations: 120 cases passed. Completed sentence dictionaries remained identical to base; valid token timestamps were preserved exactly once with correct boundaries; raw token separators were retained.
- The submitted missing-tail-timestamp cases also pass. This does not establish correctness for every malformed timestamp payload.
This was CPU-only source/test validation with recording/mocked model outputs; I did not download or run punctuation/ASR weights, independently reproduce the reported real-checkpoint experiment, or run the full repository suite. The current head had no hosted check runs at this review checkpoint, which is not a hosted-CI pass. This is a comment review, not approval or a merge.
LauraGPT
left a comment
There was a problem hiding this comment.
Reviewed exact head 162ef07 against current main d98f6ee. The timestamp and AutoModel punctuation suites pass (25 tests and 8 subtests); the combined candidate containing #3758 and #3761 also passes 128 tests and 10 subtests. The earlier independent 120-case splitter review remains applicable to the unchanged implementation. Unpunctuated tails retain their timestamps; incomplete timestamp tails remain omitted and completed prefixes remain unchanged. No blocking findings. This is CPU/parser and recording-double evidence, not checkpoint inference or an accuracy benchmark. No hosted workflow was triggered for this PR.
上游 funasr 的 timestamp_sentence() 只在遇到终止标点时才 flush 一句,于是末尾那段 没有终止标点的文本会被整段丢弃(modelscope/FunASR#3754,截至 1.4.16 未发版)。 说话人说一半被打断、录音到点结束都很常见,表现是字幕尾部凭空少一截, 而 r0 的 text 字段里其实还在。 缓解方式:比对 sentence_info 拼接文本与整段文本的「实义字符」(必须忽略标点—— ct-punc 会给 sentence_info 补句号,直接比原字符串会把标点差异误判成多了内容), 确认确实是前缀关系(丢的是尾巴而非中间)后,把剩下那截按「上一条结束 → 音频结束」 补回去,沿用上一条的说话人标签。 两个刻意取舍: - 时间戳不从 timestamp 数组对齐。实测它按「非标点 token」给(本机 10.3s 样本是 32 条时间戳 vs 36 个非空白字符),硬对齐反而更容易错。宁可时间范围略宽, 也不能把字丢了——这本来就是这个 bug 的本质。 - 对不上就原样返回:中间缺段、非前缀关系、时间上没地方放,一律不猜, 交给原有的兜底路径。 新增 11 条回归测试,含 4 条「不误伤」路径:覆盖完整不得补、纯标点差异不得补、 中间缺段不得猜、时间不够不得硬塞。 文档:engine/README.md 新增 §8 记录该上游问题与缓解取舍;新增 §0 记录两个包 各自验证过的依赖版本(CPU 包 funasr-onnx 0.4.3、CUDA 包 funasr 1.4.16), 并注明 0.4.1 时代的两处 workaround(ContextualParaformer 漏设 language、 model_eb_quant.onnx 需本地复制)在 0.4.3 上依然必需,不能删。 requirements-full.txt 同步说明 funasr 不钉版本、取当前最新。 真机验证:CUDA 包 exe 上跑截断到 7.0s / 8.5s 的样本,末条字幕分别延伸到 6805ms / 8395ms(贴近音频末尾 7000 / 8500ms),确认尾巴被补回而非丢弃。
Summary
Users combining token timestamps with online punctuation can get an empty
sentence_infofor a nonempty transcript when the punctuation model leaves the last token unpunctuated. Both fallback timestamp splitters emitted a segment only after a punctuation ID greater than 1. They now emit a remaining segment when its timestamps are present. The existing surface-text path already emits such a tail.Type of change
Validation
.venv/bin/python -m pytest -q tests/test_timestamp_tools.py tests/test_punc_model_none.py: 25 passed, 8 subtests passed. Restoringtimestamp_tools.pyfrom the base commit makes the three new tail cases fail; 22 tests and 8 subtests pass..venv/bin/python -m compileall -q funasr examples tests: exit 0.iic/punc_ct-transformer_zh-cn-common-vad_realtime-vocab272727checkpoint on CPU with inputhello world. It returned texthello worldand punctuation IDs[1, 1]. Feeding that output with fixed ASR timestamps[[0, 100], [100, 300]]throughAutoModel.inference_with_vadreturnedsentence_info=[]on the base source and onehello worldsentence spanning 0 to 300 ms with the fix. The existing surface-text splitter returned that same sentence for these values. The ASR and VAD outputs in this check were stubs; I did not run ASR weights.User impact
The tested online punctuation checkpoint can leave a final token with ID 1. When a timestamped ASR result has no word list,
AutoModeltakes the fallback splitter and previously lost that token fromsentence_info. The commonct-puncalias uses a different, offline model; I tested the online checkpoint named above. I measured this input, not how often it occurs in other transcripts.Notes for reviewers
The new segment is omitted if its timestamp is missing. The change is limited to the fallback timestamp splitters. Completed sentences and top-level ASR text were unchanged in the fixed-input tests.