feat(translator): translate rich text one container at a time, behind a flag - #139
Merged
Merged
Conversation
… a flag
A paragraph is translated one text node at a time today, each node in
isolation. Word order therefore stays pinned to the source language and inline
formatting lands on whichever word happens to occupy that position. Measured on
a live model: `a **red** car` into French came back `un **rouge** voiture` —
ungrammatical, and the emphasis on the wrong word.
Container mode sends the whole paragraph as one string with numbered marks,
`<1>a </1><2>red</2><3> car</3>`, and rebuilds the container's children in the
order the reply came back. The same sentence now returns
`une **voiture rouge**`.
Off by default, behind `experimental: { inlineMarks: true }`, and ignored
unless the provider declares `capabilities.inlineMarks` — a transport that is
not a language model would translate or strip the marks, so the capability is
part of the provider contract rather than a config option.
The mark instruction is appended **after** a `systemPrompt` override rather
than inside `defaultPrompt`, because a builder is free to ignore the default
and would otherwise silently drop the one rule the format depends on.
A container the format cannot carry falls back to the per-node path, and says
which: source text that already looks like a mark, a single leaf with nothing
to reorder, no translatable text, or a wrapper shape whose per-leaf copy would
drop a node. A reply whose marks come back damaged leaves that container in its
source language rather than writing half a paragraph into the document —
silently for now, since the report that records such places is the next change.
One source wrapper holding several leaves becomes several adjacent wrappers in
the result and they are not merged back (D12): a link with an emphasised word
inside renders identically as two sibling links with the same href. Confirmed
on a live run — `[read the **manual**](/docs)` came back as two adjacent links,
each keeping the href, with the German word order rebuilt around them.
The write path has one branch. An earlier draft kept a write-in-place fast path
for replies that came back in the original order; it was removed before this
change was split out, because the second branch cost a flag, a four-clause
predicate and three tests that existed only to keep the two branches agreeing,
and rebuilding an array per paragraph costs nothing measurable.
Verification: 1488 unit tests, 19 new; check-types clean; lint 58 warnings and
0 errors, identical to main; declaration build passes. Live on a real key
across French and German: word order rebuilt, emphasis carried to the right
word, an emptied mark's node dropped, and a wrapper's href preserved on both
halves.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
SearheiParkhamchuk
requested review from
ChiefCreator and
dogfrogfog
as code owners
September 11, 2026 15:16
`RichContainerExpander` computed a per-container ordinal and stamped each chunk
with `place: { path, container }`, and `RichContainerChunk` declared the field.
Nothing in this change reads either.
The address exists for the untranslated report, which is the next change and is
where it will arrive together with the code that consumes it. Carrying it early
means a field computed on every container, declared in a public-facing chunk
type, and dead — which is the speculative code this repository's rules forbid,
and which this PR's own message argues against while shipping an instance of it.
Found while explaining the expander rather than by a check: the report was split
out of this PR by searching for `untranslated`, and `place` does not contain
that word.
Verification: 1488 unit tests, unchanged in count and all green; check-types
clean; lint 58 warnings and 0 errors, identical to main; declaration build
passes.
Four `@since 0.12.0` annotations said the version the package is already on. This change is a `feat` on top of 0.12.0, so everything it adds arrives in 0.13.0 — `TranslationRequestOptions`, `TranslationProvider.capabilities`, the transport's pass-through of the same, and the `experimental` plugin option. They were written while 0.12.0 was still the next release; it was taken by the default-model change, and the annotations did not move with it. The package's own rule is to derive the version from the latest tag plus the highest bump in the change, which is what makes them checkable at all. The deprecation register's entry also cited `#134`, the issue, where every other entry cites the PR.
`TranslationStage` put parsed reply marks in `PipelineContext.containerFragments`, a `Record<number, ParsedMark[]>`, and `TranslationMutator.apply` took that map as a third parameter and joined it back to each chunk by `chunk.index`. The reply now lives on `RichContainerChunk.reply`; the context field and the third parameter are gone. `chunk.index` is the chunk's own position in `textChunks` — `TextChunkExpander` runs one dense counter across all fields and every expander increments by one per chunk emitted. So the map was a sparse column over an array already indexed by the same number, and its only job was putting the two halves of one write back together. The chunk already holds every other half: `containerRef`, and each fragment's `node` and `top`. It also settles what comes next. The untranslated report derives a second per-chunk value from the same parse — why a reply could not be used — which under the old shape would have become a fifth index-keyed column in the context. On the chunk it is one more field and the report is a walk over `textChunks`. **A habit is broken here deliberately.** No pipeline stage wrote into a chunk before this: chunks were authored by the expander and read by the applicator, so a stage's return value described everything it did. `TranslationStage` now fills in a field on chunks it did not create, and its return understates it. The cost of keeping the rule was the fourth column now and the fifth next. Two smaller things fall out. `parseContainerReplies` no longer returns anything, so it no longer has two ways to say "nothing here" — `undefined` for no containers and an empty map for none parsed — which no caller could tell apart. And the name `containerFragments` no longer sits next to `chunk.fragments` meaning something else: fragments carry live node references collected before sending, a reply is pure data that came back. Behaviour is unchanged, and the 13 end-to-end container tests were not edited to fit. The container write path had no unit test at all before this, so three were added against the applicator directly: reorder, drop a node whose mark came back empty, and leave the container alone when no reply was parsed. Verification: 1491 unit tests (1488 + 3), 0 failed; check-types clean; lint 58 warnings and 0 errors, identical to main; declaration build passes; diff-hygiene and analyzer-delta gates clean; a fresh review pass found nothing. Two mutations, each red on exactly its own cases.
…unk kind a compile error Three findings from an interface review of this PR, none of them behavioural. **`RichContainerChunk.text` was written and never read.** The expander filled it in and no consumer touched it — the applicator and the translation stage both work from `fragments`, `reply` and `containerRef`. It was also the same field name the sibling chunk kinds use for a different thing: on plain and per-node chunks `text` is the source prose the drift guard hashes, while here it held the marked wire string, which is why it needed a comment saying "never hash or display it". A field that has to warn you about its own name is misnamed. The string it duplicated is already in `textMap`, keyed by the same index, so the assertions that read it now read that instead. **A fourth `TextChunk` kind used to compile clean and be silently skipped.** `apply()` branched with three unconnected `if`s and no final else, so a new kind would fall through both tests, never be written, and produce an untranslated field with no error anywhere. Adding the exhaustiveness guard this codebase already uses elsewhere turns that into a compile error in the applicator — verified by adding a fourth member and reading TS2322. **Three consequences of the experimental flag were undocumented.** Turning it back off stops future translations from splitting wrappers but leaves already translated documents split — the one-way door is the first translated document, not the flag. The next major removes the flag, which is also the only way to turn the behaviour off, so it is a schedule rather than a permanent safety valve. And `capabilities.inlineMarks` is core logic that survives the flag's removal, so a third-party provider that never declares it keeps translating node by node indefinitely, in the mode the register itself calls a defect, with no warning surface. All three are now in the register entry, the first two also on the option's own docblock. The drift guard gained one line of why: it is built from per-node expanders deliberately, because a container expander sends marked strings and the guard compares source prose. Verification: 1491 unit tests, unchanged in count and all green; check-types clean; lint 58 warnings and 0 errors, identical to main; declaration build passes. Integration (17 suites, 82 tests) was run against the default path and is green, but it exercises none of container mode — the flag is off there. That gap is the next piece of work.
…sting depth The unit suite proves container mode in memory. Nothing proved it against a real Payload, a real save and a document read back — which matters here more than usual, because this is the one feature that rewrites the structure of stored content rather than just its text. **The fake translation service no longer reverses strings.** Reversal destroys marks, so with the old fake every container would have fallen back to the per-node path and the feature under test would never have run. The replacement understands marks: each comes back exactly once, its text translated, and by default in reverse order, because reordering is the whole point of the format and a fake that preserved order would leave it untested. It can also keep the order, or return a reply that is damaged in one of three named ways. Values now come back prefixed with the target locale — `de:Title source` rather than `ecruos eltiT`. That reads better, but it also found five existing assertions that compared a French or Spanish result against a German expectation and passed, because a reversed string is the same in every locale. Those five now assert the locale they are actually about. New coverage, all of it against a saved and re-read document: - **Every nesting shape the field walker classifies** — group, array, blocks, a named tab, an unnamed tab, `row`, `collapsible`, and one group → array → blocks combination. The shared fixture carries exactly one rich-text field at the top level, so the three "transparent" shapes had no coverage at all, for this feature or any other. `textarea`, the third translatable leaf type, was also never exercised. - **Every container shape inside a Lexical tree** — a heading and a quote are containers exactly as a paragraph is, each list item is its own container with its own marks numbered from one, the list itself is not a container, and a node carrying no text keeps its place when the rest moves. - **A reply whose marks cannot be parsed** — the container keeps its source text whole, and the rest of the document still translates. - **A provider that never declared it can keep marks** — the flag is on and the core still translates node by node, which is the gate that protects a translation service that is not a language model. - **The non-localized sibling in every shared row** survives. Those rows are one row with per-locale columns, so rewriting a rich-text leaf inside one is where a mistake costs another locale its content. Each suite was checked for discrimination rather than assumed: the nine nesting specs and the five structural ones go red with the flag off, the damaged-reply spec goes red when the damage is removed, and the capability spec goes red when the capability is declared. Verification: 22 suites, 108 tests, all green (17 and 82 before). Run from the repository's main checkout — a linked worktree resolves the plugin through the root `node_modules` symlink and would exercise the wrong code, so these cannot be run from the branch's own worktree.
…text The integration suite proves the feature against a stub. Nothing let a person drive it by hand against a real model, which is where the questions that matter turn up — whether the translation is any good, and whether it is better than what the old path produced. Four changes, each needed before the sandbox could show the feature at all: - **The mode is on by default here**, unlike the plugin, where it stays behind the flag. A sandbox run that silently used the old path looks exactly like a feature that does not work, and that is a bad half-hour to spend. `TRANSLATOR_INLINE_MARKS=0` goes back to translating node by node, which is also how the two modes get compared on the same document. - **The local fake declares the capability it genuinely has.** The real OpenAI provider declares it; the fake did not, so a dry run fell back to the old path no matter what the flag said. - **`playground` keeps drafts and versions.** `pages` already had them, but its shape is flat, so until now no versioned document had nested blocks — and a translation writes into the draft, which is the behaviour worth being able to watch. - **A third seeded article, "Word order".** Every sentence in it is one German forces out of English order: the participle or the infinitive lands at the end of the clause, negation moves behind the object. The formatting and the links sit on exactly those travelling words. The existing two articles cannot show the difference between the modes, because their French and German read in English order anyway. `importMap.js` is deliberately not part of this commit. Regenerating it here drops the analytics plugin's components and `@/lead-actions-admin`, because this machine has no GA4 credentials and the plugin disables itself — a record of a missing environment, not a change to the app.
…tainer `createFieldRoute` accepted `inlineMarks` in its argument type and never passed it to the handler, so `translateContent` saw `undefined` and took the per-node path every time. One of the plugin's three translation surfaces quietly ignored the option this release exists to introduce: an editor pressing translate on a single rich-text field got the old behaviour no matter how the plugin was configured. Proven with the local stub, which reverses mark order, so the two modes are distinguishable without a model: through the field endpoint the order never changed, at either flag value; through document translation it changed with the flag on and not with it off. **The flag is now required rather than optional**, on the field feature's config and on the shared `TranslationContext` it derives from. `plugin.ts` computes `experimental?.inlineMarks === true`, which is always a definite boolean — the value is never genuinely absent, so optionality described a state the system cannot be in, and that is what let a dropped argument compile into a silent fall-back. Deleting the argument at the call site is now an error that names the file. The only production change this forced is the route itself; every other edit is a test constructing a context. **Three specs cover the surface, because nothing did.** All twenty-six existing mark specs boot `levels: [documentLevel()]`, so the field route's wiring was never exercised — which is why a dropped argument survived review, a type check and a full suite. The new ones drive the real endpoint through `callEndpoint` against a booted Payload, one configuration per file as everywhere else here: marks reordered with the mode on, source order with it off, and source order when the provider never declared it can keep marks. The first was red against this defect before the fix. The README gains the option, what the two gates do when they stop it, and why the entry is deprecated on the day it ships while `experimental` itself is not.
…capability Review of the three specs added with the fix found they covered only three corners of (flag × capability) — on/on, on/off, off/off — and that in all three the two signals agree on the answer. A route reading `provider.capabilities?.inlineMarks` in place of the flag its caller passed would have satisfied every one of them, which is the same class of wiring defect the fix exists to close. The fourth corner separates them: the flag off while the provider declares it can keep marks. Verified rather than argued — `createFieldRoute` was temporarily made to read the capability instead of the flag, and that turned only the new spec red while the other three stayed green. Two other things travel with it. The field route now passes the handler's config through whole instead of naming its fields to rebuild the object: seven of the eight route factories in this layer already do that, and the one that did not is the one that dropped a field. And the three specs, which were sixty byte-identical lines apart from their boot options, now share one fixture module — so the note about `payload.create` writing ids into the value it is handed exists once rather than three times. The prompt wording is unchanged, and the docblock above it now says what was measured and rejected: a rule asking the model to keep each mark around the same words tightens mark boundaries and costs target word order. Four live runs of a ten-paragraph German fixture are recorded in the plan document — every one of them returned every mark exactly once, with no duplicated or lost word, so mark integrity was never the weak axis.
… owes callers The per-field translate control had fifteen unit tests and no integration coverage at all. That distinction matters more here than it usually does: the unit tests hand the handler a schema map built by hand, while in service the plugin projects one from the real collection configs — and a path into `blocks` is resolved against the saved document's own `blockType`, with rows, unnamed tabs and collapsibles vanishing from the path entirely. A hand-built map and the real one are exactly the pair that can disagree, and nothing compared them. **The contract came first, and writing it down produced facts that existed nowhere.** The types stated the shapes; nothing stated that the value translated is the *saved* one rather than the form's, that nothing is written in any locale, that a per-field translate has no strategy to choose, that "I cannot translate this" is a success rather than an error in six named situations, or that the one warning among them marks the case where an answer is possible but would be wrong. `FieldTranslation` binds that prose to the method, so the signature is now checked rather than described. **Eighteen checks, written against the contract by an author that could not read the implementation** — it was removed from the checkout that author worked in, so blindness is a fact rather than a request. The red run took three attempts and the first two were dishonest, which is worth recording because both failures look like success from a distance: - the specs died in setup — Payload's `update` loses its receiver when pulled off the object through a cast — so no check reached the code under test; - then sixteen went red and **two passed**: both "writes nothing" checks are satisfied by a handler that does nothing whatsoever. They now assert the request was answered before asserting the document is untouched, and a mutation that makes the handler persist turns exactly one of them red. Four more mutations pin the rest: removing the exclusion branch, the localized-list branch, or the size guard reddens its own checks, and hardcoding the source locale reddens exactly the check that says the locale comes from the request. One check earned a fixture guard rather than a mutation: the 413 case now asserts the stored value really does exceed the cap, because it silently stopped doing so once and passed anyway.
… not just for a reader
`POST {basePath}/field` answers `200` with `status: "noop"` in five situations
where it will not translate. Four carried `level: "info"`, and two carried the
same sentence word for word — so the admin control, which renders that sentence,
could not tell "this field is empty" from "this field opted out", and neither
could a test. The notice gains a `reason` beside its `message`.
**The distinction already existed and was discarded one step early.**
`resolveFieldSubtree` returns five named statuses, and its own docblock records
the intent: `excluded` is kept distinct from `not-translatable` "so the notice
can say *why* (a deliberate opt-out, not a wrong type)". The handler then
collapsed five names into two levels and four strings. This finishes that.
**A union on the wire type, not a class per reason.** The package already has a
code taxonomy in `TranslationFailureCode`, but that one is shaped by being
*thrown* — a class is how a throwable carries structure through a `catch`. These
are `200` bodies, so the same idea costs one union instead of five files. Reusing
the resolver's own status names was rejected for two reasons: the fifth situation
arises after resolution and has no status at all, and it would pin a wire format
to an internal helper's vocabulary.
**No behaviour changes.** Every status, level and message is byte-identical; only
the notice is wider. The two identical messages stay identical — `reason` is what
tells them apart now, and rewording is a copy decision nobody has made.
Writing the codes down settled something the contract had left open, and the
first run found it: a container named directly by the path is judged by its own
type before anything walks inside it, so a group whose only leaf is excluded
answers `not-translatable`, not `nothing-translatable`. The spec that assumed
otherwise now says what it actually pins, and the rule is in the docblock — whose
table also drops from six situations to the five the code has.
|
🎉 This PR is included in version 0.13.0 🎉 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 #134. Builds on the two kernel modules from #137.
The defect
A paragraph is translated one text node at a time, each node in isolation. Word order stays pinned to the source language, and inline formatting lands on whichever word happens to occupy that position.
Measured on a live model,
a **red** carinto French:What this does
Sends the whole container — paragraph, heading, list item — as one string with numbered marks, and rebuilds its children in the order the reply comes back:
Same sentence, same model, flag on:
The third node merged away — the model returned its mark empty — so three children became two. That is the designed behaviour for a merge, not a loss.
Two gates, both closed by default
experimental: { inlineMarks: true }in the plugin config.capabilities.inlineMarks. A transport that is not a language model (DeepL, Google Translate) would translate or strip the marks, so this is part of the provider contract rather than a config option.The mark instruction is appended after a
systemPromptoverride, not insidedefaultPrompt— a builder is free to ignore the default, and would otherwise silently drop the one rule the format depends on, with no way for the operator to see why every container fell back.When it will not use marks
A container falls back to the per-node path, and says which reason applies: source text that already looks like a mark; a single leaf with nothing to reorder; no translatable text; or a wrapper shape whose per-leaf copy would drop a node.
A reply whose marks come back damaged leaves that container in its source language rather than writing half a rebuilt paragraph into the document. Silently, for now — the report that records those places is the next change.
A known structural change (D12)
One source wrapper holding several differently-formatted leaves becomes several adjacent wrappers in the result, and they are not merged back in this version. Confirmed live, German:
One link became two siblings with the same href. It renders identically; merging siblings that share an origin is polish, and correctness does not depend on it.
One write path, not two
An earlier draft kept a write-in-place fast path for replies that came back in the original order. It was removed before this change was split out: the second branch cost a flag, a four-clause predicate and three tests that existed only to keep the two branches agreeing, and rebuilding one array per paragraph costs nothing measurable. Removing it was also what exposed a coverage gap — every pipeline-level container test used flat text nodes, so placing a bare leaf instead of its wrapper kept the whole core suite green. That case is now covered.
Verification
main. Declaration build passes.Review note
The load-bearing pair is
node(where text is written) andtop(what is placed into the rebuilt array) from #137. This PR is what consumes that distinction:TranslationMutator.applyContainerwrites throughnodeand placestop.