Skip to content

fix(auto-model): preserve explicit timestamp and speaker text - #3553

Merged
LauraGPT merged 1 commit into
modelscope:mainfrom
John6666cat:fix/auto-model-pred-timestamp
Aug 30, 2026
Merged

fix(auto-model): preserve explicit timestamp and speaker text#3553
LauraGPT merged 1 commit into
modelscope:mainfrom
John6666cat:fix/auto-model-pred-timestamp

Conversation

@John6666cat

Copy link
Copy Markdown
Contributor

Summary

Fix two related timestamp/speaker-output contract mismatches in the Paraformer + AutoModel path.

  • If pred_timestamp is explicitly provided, honor that value.
  • Only fall back to output_timestamp when pred_timestamp is not specified.
  • In the no-timestamp speaker/VAD fallback, expose text while preserving the legacy sentence field.
  • Add CPU-only, network-free regression coverage for both behaviors.

This is intentionally limited to the first of the two fixes discussed with the maintainer (PR A).

Type of change

  • Bug fix
  • Documentation
  • Example or demo
  • Runtime or deployment
  • Benchmark or evaluation
  • Model/training change

Validation

Focused regression tests:

python -m pytest -q tests/test_paraformer_timestamp_contract.py tests/test_punc_model_none.py

Result:

16 passed, 12 subtests passed

The new regressions fail on the unmodified baseline and pass with this change.

  • python -m compileall funasr examples tests
  • Docs or links checked
  • Runtime/deployment command tested

Also checked:

git diff --check

User impact

This fixes cases where an explicit pred_timestamp setting could be overridden by the surrounding output_timestamp behavior, and keeps the speaker/VAD fallback result compatible with consumers expecting the documented text field.

Existing callers using the legacy sentence field remain compatible.

Notes for reviewers

The scope is intentionally narrow.

This PR does not include the separate dynamic-batch timestamp-tail fix (PR B), EParaformer changes, English/BPE-specific changes, or broader timestamp-flag normalization.

The regression tests are CPU-only and do not require network or model downloads.

@LauraGPT LauraGPT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation and focused regressions look correct: I reproduced 16 passed in the isolated NumPy 1.26 environment, and the combined #3553 + #3554 patch applies cleanly. The remaining merge blocker is DCO: commit 0ef4a425d887e906a40d68003e01065377cb34c8 is GitHub-verified but has no Signed-off-by trailer. Please run git commit --amend --signoff --no-edit and git push --force-with-lease; I will re-check the exact replacement SHA.

@John6666cat

Copy link
Copy Markdown
Contributor Author

Thanks. I amended the commit with my DCO sign off and force pushed the replacement head. New SHA: 4494146.

@LauraGPT LauraGPT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replacement implementation is now correct. I applied exact heads #3553 449414665d36953f3bd83e514c31125a228c0fe4 and #3554 6042ace22d617a69c47735503cb67be04c7825fd together on current main@55b662ccf9ea77237ba9253b3bddd953d4184f84; all 24 focused timestamp, padding, punctuation-none, and timestamp-tool tests pass. py_compile and git diff --check also pass.

The only remaining blocker is the trailer syntax. The commit currently contains:

Signed-off-by: John6666 186692226+John6666cat@users.noreply.github.com

DCO requires angle brackets:

Signed-off-by: John6666 <186692226+John6666cat@users.noreply.github.com>

Please ensure user.name and user.email are configured, then run git commit --amend --no-edit --signoff and git push --force-with-lease. Please verify the resulting message with git log -1 --format=%B. No code changes are requested.

Signed-off-by: John6666 <186692226+John6666cat@users.noreply.github.com>
@John6666cat
John6666cat force-pushed the fix/auto-model-pred-timestamp branch from 4494146 to 391fb2b Compare August 30, 2026 11:43

@LauraGPT LauraGPT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revalidated the exact replacement head 391fb2b5138fffe524b9e07fa99669e1d1ccf53d on current main@917ca2f4a673b4ee8b0aea4b64fbe2d6c2034a67.

  • the source tree is identical to the previously reviewed implementation
  • the corrected Signed-off-by trailer satisfies DCO
  • 18/18 focused timestamp-contract, punctuation-none, and timestamp-tool tests pass
  • py_compile and git diff --check pass
  • the current #3554 head also applies cleanly after this PR and the combined suite passes

Approved for merge.

@LauraGPT
LauraGPT merged commit 062b51d into modelscope:main Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants