Keep empty Auto-Translate in-place results as no-ops - #155
Draft
Blackspirits wants to merge 1 commit into
Draft
Blackspirits wants to merge 1 commit into
Blackspirits wants to merge 1 commit into
Conversation
Blackspirits
commented
Sep 16, 2026
Blackspirits
left a comment
Owner
Author
There was a problem hiding this comment.
Final adversarial review on 95d3546056de205091f9fa4fd1475f01399a4f36 after CI SubtitleEdit#313: no blocker found. Verified zero-result OK is now a true no-op before original capture, row mutation, detected-language reset, dictionary/spell-check state reset, translator-mode switch and waveform refresh. Positive in-place and capture-original paths remain unchanged. All eight warnings are outside the changed set; upstream is still exactly dda1fce8.... No merge/promotion performed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Current-main follow-up to the new "Translate in place" path merged in upstream commit
dda1fce8f9d4a605b1b6732acc072c7109dc6323(SubtitleEdit#14956 follow-ups).The existing selected-lines flow already treated an OK result containing no translated rows as a no-op when it was about to capture the current subtitle as the original. The new in-place mode accidentally made that guard conditional on
captureOriginal.Current upstream main was rechecked at
dda1fce8f9d4a605b1b6732acc072c7109dc6323, exactly this PR's base.Finding fixed
For a subtitle without an original column:
TranslatedTextempty (for example a partial/cancelled run that produced no row);Textis changed;AutoTranslateSelectedLines()continued intoOnSubtitleLanguageChanged().OnSubtitleLanguageChanged()is intentionally stateful: it clears the cached detected language, clears the dictionary explicitly selected for the current subtitle, drops an in-progress spell-check session and re-runs live-spell-check language selection.That is correct after translated text replaces rows, but not when the operation produced zero changes.
The pre-existing default/capture-original mode did return early for this exact empty-result case; the new in-place branch bypassed that no-op guarantee.
Fix
After OK, selected-lines Auto-Translate now first checks whether any result row contains non-empty
TranslatedText.If none do, it returns immediately before:
OriginalText;Only after a real translation exists does the method decide whether this run should capture the original or remain in-place.
Regression coverage
A headless UI regression:
The existing positive in-place translation and default capture-original tests remain unchanged.
Scope / branch state
dda1fce8f9d4a605b1b6732acc072c7109dc632395d3546056de205091f9fa4fd1475f01399a4f36Acceptance
AI assistance: ChatGPT was used for state-transition review of the new in-place path, spell-check/language side-effect tracing, and focused no-op regression design.
Final CI
Authoritative run: SubtitleEdit#313 on
95d3546056de205091f9fa4fd1475f01399a4f36All eight warnings are outside the two-file #155 changed set.
Final drift gate
Upstream main was rechecked at
dda1fce8f9d4a605b1b6732acc072c7109dc6323, exactly this PR's base. No post-base drift exists.Final adversarial review was performed on this exact HEAD after CI; no blocker found.