chore(translator): add the inline-mark codec and the container fragment collector - #137
Merged
Merged
Conversation
…nt collector Groundwork for translating rich text one container at a time. Nothing imports these yet; the pipeline is wired to them in the next change, which is what makes them reachable and what earns a release. Typed `chore` deliberately: a release whose notes announce a feature no install can use is noise. Today a paragraph is translated one text node at a time, so word order stays pinned to the source language and inline formatting lands on whichever word happens to sit in that position. These two modules are the pieces needed to send a whole container instead. `collectInlineFragments` walks a Lexical tree and returns, per container, the fragments it is made of — each carrying the node to write text into and the node to place when the array is rebuilt. Those are two different things for a wrapper: a link around one word places the link, a link around two differently-formatted words gets a separate copy per fragment so the two can move independently. It refuses containers it cannot round-trip and says why: a source that already looks like marked-up text, a single leaf with nothing to reorder, no translatable text, or a wrapper shape it does not handle. `inlineMarks` serialises those fragments as `<1>text</1>` and parses a reply back. Parsing is all-or-nothing by design: seven named failures, and any of them returns no fragments at all, because half a rebuilt paragraph written into a document is the failure nobody notices while untranslated text is obvious. The existing per-node walk is left untouched. It feeds both the per-node translation path and the provenance fingerprint, and the two walks disagree on whitespace — this one glues a whitespace-only node into its neighbour where the old one drops it — so widening the old walk would move stored fingerprint values. Verification: 1469 unit tests, 89 new across the two modules; check-types clean; lint 58 warnings and 0 errors, identical to main; declaration build passes. Four mutations, each red on its own cases: allowing a repeated mark, allowing a lost mark, allowing an unclosed mark, and placing a shared wrapper instead of a per-fragment copy.
SearheiParkhamchuk
requested review from
ChiefCreator and
dogfrogfog
as code owners
September 11, 2026 10:12
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…y are enforced Follow-up on the same modules after auditing the tests, the complexity and the comments. Behaviour is unchanged; three of the findings were defects rather than verbosity. **Citations to a file that does not exist.** Sixteen references to `docs/plans/2026-09-08-richtext-container-granularity-design.md` and its decision ids, mostly in the tests. That document was taken out of version control by 23fc243 and is on no branch, so none of them could be followed. Removed; the rationale they pointed at is now stated where it is needed or not at all. **A docblock that was already false.** `parseInlineMarks` claimed text returned for a fragment sent text-free "is ignored". It is not — the function returns it in `collected`. Corrected. **A guard for a state its caller rules out.** `restoreEdges` began with `if (!translated) return translated;`, but `sameShape` already excludes every mark whose text came back empty. Unexplained defensive code reads as evidence the case is reachable, so the next reader preserves it while changing the condition that made it dead. Removed. **A name that said the wrong thing.** `crossed-marks` reads as interleaving to everyone, and the comment beside it existed to say it is not: the case is a mark closed with a different number than it opened with. Renamed to `mismatched-close`, and the comment went with it. Two rules moved out of prose: - `text` and `node` were documented as "null together" and checked by nothing. They are now one discriminated union, so the pairing is a compile error rather than a promise — confirmed by building the impossible combination and reading TS2322. - The skip decision was computed from two sources — `drafts` in the walk, `fragments` in `skipReasonFor` — with an unwritten precedence between them. One function now holds the whole order, stated in one line. Comment lines: 71 → 43 and 63 → 42 in the two modules, 52 → 3 across their tests. Four comments survived the pass: the backwards glue search, the all-or-nothing rule, the mark pattern's tolerance of whitespace and leading zeros (models emit both), and the one test note explaining why a length assertion sits beside a non-mutation check. Verification: 1469 unit tests, unchanged in count and all green; check-types clean; lint 58 warnings and 0 errors, identical to main; declaration build passes.
This was referenced Sep 11, 2026
|
🎉 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.
Groundwork for issue #134. Nothing imports these yet — the pipeline is wired to them in the next PR, which is what makes them reachable and what earns a release.
Typed
choreon purpose: a release whose notes announce a feature no install can use is noise. The version moves when the wiring lands.The problem they exist to solve
A paragraph is translated one text node at a time today, so each node is translated 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 earlier:
Une **rouge**voiture— words fused, emphasis on the wrong one. The same paragraph sent as one string with numbered marks came backUne **voiture rouge**.collectInlineFragments— 232 linesWalks a Lexical tree and returns, per container, the fragments it is made of. Each fragment carries two node handles, and they are not the same thing:
node— where the translated text is written.top— what gets placed when the container's children are rebuilt.For a bare text child they are the same object. For a link around one word,
topis the link. For a link around two differently-formatted words, each fragment gets its own copy of the wrapper chain, so the two can move independently without aliasing each other or the source tree.It refuses containers it cannot round-trip, and names the reason rather than failing silently: a source that already looks like marked-up text, a single leaf with nothing to reorder, no translatable text, or a wrapper shape it does not handle.
inlineMarks— 207 linesSerialises fragments as
<1>text</1>and parses a reply back.Parsing is all-or-nothing by design. Seven named failures —
missing-mark,unknown-mark,repeated-mark,nested-marks,mismatched-close,unclosed-mark,no-text— and any of them returns no fragments at all. Half a rebuilt paragraph written into a document is the failure nobody notices; untranslated text is obvious.What is deliberately left alone
The existing per-node walk (
collectTextNodes.ts). It feeds both the per-node translation path and the provenance fingerprint, and the two walks disagree on whitespace: this one glues a whitespace-only node into its neighbour's text, the old one drops it. Widening the old walk would move stored fingerprint values and make existing translations look stale.Second commit — audit follow-up
The modules were then put through a test audit, a complexity pass and a comment audit. Behaviour is unchanged; three findings were defects rather than verbosity:
23fc243b, so none of them could be followed. Removed.parseInlineMarksclaimed text returned for a fragment sent text-free "is ignored"; it is returned incollected. Corrected.restoreEdgesopened withif (!translated) return translated;whilesameShapealready excludes every emptied mark. Removed, because unexplained defensive code reads as evidence the case is reachable.Plus a rename —
crossed-marksreads as interleaving to everyone, and the comment beside it existed to say it is not; it is nowmismatched-closeand the comment is gone — and two rules moved out of prose into places that enforce them:text/nodeare one discriminated union instead of a docblock promise (the impossible pairing is now TS2322), and the skip decision, previously computed from two sources with an unwritten precedence, is one ordered function.Comment lines: 71 → 43 and 63 → 42 in the modules, 52 → 3 across their tests.
Verification
main. Declaration build passes.verdict is repeated-mark when an issued number comes back twicea corrupt reply carries no fragments at allverdict is unclosed-mark when a mark is never closedReview note
The pair worth reading closely is
nodevstopincollectInlineFragments, and the copy-chain that separates them. The rest of the feature depends on that distinction being right: getting it wrong moves a link to the wrong word, or aliases two fragments onto one mutable wrapper.