Skip to content

Fix resume invalidation, lazy Kaldi inputs, and strict status - #105

Open
ftshijt wants to merge 1 commit into
wavlab-speech:mainfrom
ftshijt:codex/fix-resume-regressions
Open

ftshijt wants to merge 1 commit into
wavlab-speech:mainfrom
ftshijt:codex/fix-resume-regressions

Conversation

@ftshijt

@ftshijt ftshijt commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Fix three resume regressions introduced in #103:

  • When inputs change, invalidate all old metric completion entries and their score fields before merging fresh results. Previously the first metric pass updated the shared input identity, causing later passes to reuse stale scores. An interruption between passes now leaves those metrics pending.
  • Select pending inputs through a lazy mapping view that preserves the original input references. Kaldi resume now uses the same identity during planning and scoring, loads only pending waveforms, and reuses completed work across metric-oriented and utterance-oriented runs.
  • Restrict resumed status counts to the current metric suite in serial, worker, and multi-source scoring. Failures retained from removed metrics no longer make a successful current configuration fail --strict.

Validation: 512 core tests passed with no skips or failures (pytest -c ci/pytest-core.ini --rootdir=. -q, with model downloads disabled). Added regression coverage for input invalidation across interruptions, partial and complete Kaldi resumes under both identity policies and execution orders, and strict status after removing failed metrics. Black checks on changed files, fatal flake8, all three docstring gates, and git diff --check pass. No GPU or real-model inference was needed.

Summary by CodeRabbit

  • Bug Fixes

    • Resuming runs with changed inputs now correctly recomputes affected metrics while preserving valid results.
    • Historical failures from metrics no longer included in the current metric selection no longer incorrectly fail strict resume checks.
    • Resume operations now preserve input identity and avoid reprocessing unchanged data.
  • Performance

    • Resumed processing loads only newly added or pending inputs, reducing unnecessary data loading and metric computation.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 362dac72-d199-46f3-9ddb-71e6e143bbfd

📥 Commits

Reviewing files that changed from the base of the PR and between c854f61 and d6cae58.

📒 Files selected for processing (5)
  • test/test_completion.py
  • test/test_pipeline/test_mapss_pipeline.py
  • test/test_pipeline/test_resume_contract.py
  • versa/completion.py
  • versa/scorer_shared.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The changes update resume behavior. Changed inputs invalidate stale metrics, pending inputs remain lazy, and run status counts only requested metrics. New tests cover metric recomputation, Kaldi loading, multi-source resume, and removed metric failures.

Changes

Resume and metric state handling

Layer / File(s) Summary
Changed-input metric invalidation
versa/completion.py, test/test_completion.py, test/test_pipeline/test_resume_contract.py
merge_rows clears previous metric entries when input signatures change. Resume tests verify recomputation and checkpoint behavior.
Lazy pending-input selection
versa/scorer_shared.py, test/test_pipeline/test_resume_contract.py
Pending inputs use a lazy key subset. Kaldi resume tests verify that only new entries are loaded.
Requested-metric status accounting
versa/completion.py, versa/scorer_shared.py, test/test_completion.py, test/test_pipeline/test_mapss_pipeline.py, test/test_pipeline/test_resume_contract.py
RunStatus.record_row filters outcomes by requested metric names. Resume call sites pass active metric signatures, and tests cover removed metric failures.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title concisely identifies the three main changes: resume invalidation, lazy Kaldi inputs, and strict status handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 5 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

1 participant