fix(translator): translate a document's locales in one job, extended rather than replaced - #133
Merged
Merged
Conversation
…parallel jobs Translating one document into several locales lost translations. Measured on the real path — the enqueue endpoint, the jobs runner, and the same `jobs.run` call the autorun cron makes — nine trials of nine across SQLite, Postgres and MongoDB landed one translation of two. Which one varied. No error was raised and every job reported success. Every write Payload makes is a whole-document version snapshot, drafts included: publishing or saving a locale reads the current version as its base and merges that locale onto it. The plugin queued one job per target locale and Payload runs a batch through `Promise.all`, so two locales built their snapshots from the same base and the second silently dropped the first's work. The parallelism was ours, so the fix removes it rather than coordinating it. A document's locales are now one workflow whose handler awaits one task per locale. Ordering comes from `await` rather than from any lock, all the locales finish inside one cron tick, and nothing is asked of the host. Payload's own concurrency control was measured and rejected: a job blocked on a key is not held for it but deferred to the next cron tick — about a minute per locale at the default autorun — and enabling it adds an indexed column to the jobs collection. Recorded in #128. Per-locale detail survives. Payload writes a log entry per task inside a workflow, so the status endpoints rebuild their per-locale rows from it; a locale with no entry yet reports the job's own state. Jobs queued in the old per-locale shape still read correctly, the same expand-and-contract `readCollectionRef` already does for the collection reference. Supersession is now per document and narrowed to work that has not begun. A running workflow holds locales it has already translated, and cancelling it would throw them away — which is the loss this change exists to stop. The suite never caught any of this because every integration spec booted the sync runner, which translates inline and in order. These specs boot the jobs runner, the production default, which had no coverage until the harness gained a runner option. Closes #114
A second request for a document used to cancel its live job and queue a replacement carrying only the locales of that request, so every locale the old job still owed disappeared without a trace. The panel's per-row "re-translate" button sends exactly one locale, which made this the ordinary path, not an edge case. A request now adds its locales to the live job's stored list. The write touches only the input column: the full document operation re-reads and rewrites the whole row, which was measured to revert log entries written in between. After writing it verifies, retries once if a competing append replaced the list, and gives anything still undelivered a job of its own — which races nothing, because that only happens once the old job is done. A job carries one source locale, one strategy and one publish flag for all its locales, so it only takes work from a request that chose the same three. Also fixed, found by review: the panel crashed rendering a failed locale whose job carried no final error yet; /translate/cancel deleted any row in payload-jobs, not only ours; cancel-by-collection skipped jobs waiting to retry; and manual run reported success without running anything, while being unable to retry a failed job at all. When the host enables Payload's enableConcurrencyControl, the workflow declares a per-document concurrency key so the queue holds a second job until the running one finishes. The plugin never sets that flag — it adds an indexed column and needs a migration on SQL, which is the host's call. Closes #114
SearheiParkhamchuk
requested review from
ChiefCreator and
dogfrogfog
as code owners
September 8, 2026 10:00
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ng it away `as never` on the workflow object and on the `jobs.queue` call silenced the whole shape, so a typo in a field name would have compiled. Payload's own `WorkflowConfig<TInput>` and a signature naming what `jobs.queue` actually accepts cover both, and the concurrency key and the handler's `job.input` are now checked rather than opaque. Comments removed from PayloadJobsRunnerProvider.
The runner file carried 64 comment lines over 340 — most of them restating the line below, or a third copy of a fact the task contract and a named test already hold. It is now 39 over 318, and the diff as a whole 138 over 1650. Two of them were stranded rather than merely noisy: cancel-by-collection's docblock still described the "pending" predicate the change removed, and the client's DocumentTranslation type still said one job carries one locale. The pre-workflow task shape now has a deprecation-register entry, so the fallback read paths point at a record of when they may be dropped instead of explaining themselves in three places. A duplicate test case left by an earlier rename is gone, and the "does not enable the host's concurrency control" assertion is its own case rather than a comment inside another.
The file had grown to 340 lines and 64 comments. Four review angles and a complexity pass later it is 302 and 35, with two members fewer and no behaviour change. The substantive removals: extendJob carried an unreachable guard (its job comes from a read that already excludes completed rows) and computed its undelivered set twice; run() normalised a job to read three fields the raw row already has; two one-call private helpers were inlined into their only callers. Two invariants stopped depending on a reader's attention. The grouping key is now the request shape itself, so "the key covers at least what pickHost compares" holds by construction rather than by a comment — that is what keeps parallel appends off the same row. And the stored job input is declared once instead of twice under one name in two files. Enqueue also serves documents in bounded batches rather than all at once, and findByCollection narrows raw jobs before expanding them per locale instead of after. One comment was wrong rather than merely noisy: it said cancelling marks a row so a running handler aborts, while the task contract records the opposite — the delete removes the row first. It now says so and points at the measurement.
|
🎉 This PR is included in version 0.11.4 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
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
1 participant
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.
Closes #114.
The defect
Translating a document into several locales lost translations. Each locale was queued as its own job; Payload runs a batch through
Promise.all, and every write it makes is a whole-document version snapshot — so two locales built their snapshots from the same base and the second silently dropped the first's work. Measured on the plugin's real path: one translation of two landing, nine trials of nine, on all three adapters. No error, both jobs reporting success.The suite never caught it because every integration spec booted the sync runner, which translates inline and in order. The jobs runner — the production default — had no coverage.
The fix, in two parts
A document's locales are one workflow job whose handler awaits one task per locale. Ordering comes from
await, not from a scheduler setting the cron cannot express. Different documents still run in parallel; they share no version chain.A later request extends that job instead of replacing it. The first version of this branch cancelled the live job and queued a replacement carrying only the current request's locales — which quietly dropped everything the old job still owed. Since the panel's per-row "re-translate" button sends exactly one locale, that was the ordinary path, not an edge case.
Extending needed one thing proven rather than assumed: does a locale written into a running job's row reach its handler? It does — Payload re-reads the row onto the live job object after each task settles. Measured on SQLite, Postgres and MongoDB, and pinned by
locale-append.int.test.ts, because that behaviour is an implementation detail rather than a documented contract. If a future Payload stops doing it, the test goes red instead of translations going missing.Two details the measurements decided:
inputcolumn only, through the adapter.payload.updatere-reads and rewrites the whole row, which reverts log entries written in between: 3 of 120 runs on Postgres, 1 of 120 on MongoDB. A narrow write showed none across ~1000 rounds.A job carries one source locale, one strategy and one publish flag for all its locales, so it only takes work from a request that chose the same three.
Also fixed, found by review
/translate/canceldeleted any row inpayload-jobscancel-by-collectionskipped jobs waiting to retrytaskSlug, which workflow jobs do not carryOptional stricter mode
When the host sets
jobs: { enableConcurrencyControl: true }, the workflow declares a per-document concurrency key and the queue itself holds a second job until the running one finishes. The plugin never sets that flag: it adds an indexed column and needs a migration on SQL, which is the host's decision. Without it the plugin adapts and extends the running job instead.The whole integration suite runs in both modes (
EXCLUSIVE_QUEUE=1), so enabling it is covered rather than assumed.Verification
mainas well.mainfor the same files, 0 errors.Left as a follow-up
Several panel rows share one job id, so cancelling one locale's row acts on the whole document. Splitting "unit of work" from "panel row" changes the
TaskRunnercontract, both runners, five handlers and the client — its own change, not the tail of this one.