diff --git a/docs/concepts/scopes.md b/docs/concepts/scopes.md index a692c5b..11519bd 100644 --- a/docs/concepts/scopes.md +++ b/docs/concepts/scopes.md @@ -83,7 +83,8 @@ announced purge. If the call fails, the rows stay, and [`purge-scopes`](../operations/commands.md#purge-scopes) catches them later. The global scope `''` is refused. A generation message still queued for the deleted scope is refused before the provider is called, once the host no longer declares the scope, and a -run of that scope already started stops before its next batch. +run of that scope already started stops before its next batch; if the provider fails on the +batch it had sent, the keys never sent get no errored row under the deleted code. `ScopePurger::findStoredScopes()` lists the scopes the bundle stores something for, with what each holds. See the [decision record](../adr/0025-a-deleted-scope-is-purged-journal-kept.md). diff --git a/docs/public-contract.md b/docs/public-contract.md index 86b3b10..d83b859 100644 --- a/docs/public-contract.md +++ b/docs/public-contract.md @@ -162,7 +162,8 @@ and only change in a major version: (`InvalidOverrideException`), when the host declares scopes: a message queued for a scope deleted since bills nothing. The scope is checked again before every batch: a run whose scope is deleted meanwhile stops before its next batch, journaled as a failure, and - throws the same exception. Only the batch already with the provider is billed. + throws the same exception. Only the batch already with the provider is billed; if that + batch fails, the keys never sent get no errored suggestion in the scope that is gone. - **A key that has a pending suggestion is not sent**, an errored one included, unless the run retries errors. - **The keys of a batch are reserved before the provider is called**: stored as errored diff --git a/src/Suggestion/SuggestionGenerator.php b/src/Suggestion/SuggestionGenerator.php index be8b57d..66395db 100644 --- a/src/Suggestion/SuggestionGenerator.php +++ b/src/Suggestion/SuggestionGenerator.php @@ -522,7 +522,12 @@ private function storeFailures(array $texts, array &$done, array &$new, array $e return; } - $this->suggestionRepository->transactional(function () use ($texts, &$done, &$new, $errored, $error, $catalogue, $targetLocale, $sourceLocale, $providerName, $scope, $onlyMissing): void { + // A scope deleted (and purged) while the failing batch was with the provider gets no + // new row: a scope created again under its code would find errored suggestions + // nobody asked it for, and the purge command no longer sees the code. + $scopeKnown = $this->scopes->isKnownScope($scope); + + $this->suggestionRepository->transactional(function () use ($texts, &$done, &$new, $errored, $error, $catalogue, $targetLocale, $sourceLocale, $providerName, $scope, $onlyMissing, $scopeKnown): void { // The rows already stored are re-read and locked: one reviewed while the run // went on (filled by hand and approved, rejected) is never written over. $stillErrored = $this->lockStillPendingByKey(array_intersect_key($errored, $texts)); @@ -535,7 +540,7 @@ private function storeFailures(array $texts, array &$done, array &$new, array $e $this->recordFailure($stillErrored[$key], $error, $providerName, $sourceValue); } elseif (isset($errored[$key])) { $this->logLeftOut('AI translation failure not recorded: the errored suggestion was reviewed while the run went on.', $catalogue, $targetLocale, $scope, $key); - } else { + } elseif ($scopeKnown) { $failed = $new[] = TranslationSuggestion::failed($key, $catalogue, $targetLocale, $sourceValue, $sourceLocale, $providerName, $error, $scope); $failed->setMetadata([self::ONLY_MISSING => $onlyMissing]); $this->suggestionRepository->saveDeferred($failed); diff --git a/tests/Unit/Suggestion/TranslationAiServiceTest.php b/tests/Unit/Suggestion/TranslationAiServiceTest.php index 7f71242..33d8c20 100644 --- a/tests/Unit/Suggestion/TranslationAiServiceTest.php +++ b/tests/Unit/Suggestion/TranslationAiServiceTest.php @@ -488,6 +488,55 @@ public function getAvailableScopes(): array self::assertSame('b2b', $this->recordedLogs[0]->getScope()); } + /** + * The scope deleted (and purged) while the first batch is with the provider, which then + * fails: the keys of the batches never sent get no errored row in the scope that is gone. + */ + public function testAProviderFailureAfterTheScopeIsDeletedLeavesNoNewRowInIt(): void + { + $messages = []; + for ($i = 0; $i < 25; ++$i) { + $messages[\sprintf('k%02d', $i)] = \sprintf('V%02d', $i); + } + + $scopes = new class implements ScopeProviderInterface { + /** @var array */ + public array $available = ['b2b' => 'B2B']; + + public function getScope(): ?string + { + return null; + } + + public function getAvailableScopes(): array + { + return $this->available; + } + }; + $service = $this->createService( + frMessages: $messages, + enMessages: [], + pending: [], + translator: static function () use ($scopes): string { + $scopes->available = ['retail' => 'Retail']; + + throw TranslationProviderException::requestFailed('stub', 'quota exceeded'); + }, + scopeProvider: $scopes, + ); + + try { + $service->generateSuggestions('messages', 'en_US', 'fr_FR', scope: 'b2b'); + self::fail('The run was expected to fail.'); + } catch (TranslationProviderException) { + } + + self::assertCount(1, $this->providerCalls); + self::assertCount(20, $this->stagedRowsByKey(), 'Only the reserved keys of the first batch have a row.'); + self::assertCount(1, $this->recordedLogs); + self::assertTrue($this->recordedLogs[0]->isFailure()); + } + /** An every-key run sends the keys that have a value: one written during the run is no exception. */ public function testAnEveryKeyRunStillSendsAKeyGivenAValueDuringTheRun(): void {