Repository navigation
Add free-translation and boundary commands with parity tests - #517
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
|
| Layer / File(s) | Summary |
|---|---|
Book access and indexes src/utils/book-indexes.ts, src/hooks/useBookIndexes.ts, src/services/bookService.ts, src/__tests__/utils/book-indexes.test.ts, src/__tests__/services/bookService.test.ts |
Book services expose tokenized books and segments. Shared indexes provide token, segment, and document-order lookups, and tests cover index construction and reuse. |
Phrase segment validation and ordering src/main.ts, src/types/interlinearizer.d.ts, src/__tests__/main.test.ts |
Phrase operations validate tokens against book segments. splitPhrase uses document order from the source book instead of caller-supplied order. |
Staged scope and book-link lookups src/services/commands/changeBuffer.ts, src/services/analysisSessions.ts, src/__tests__/services/commands/changeBuffer.test.ts, src/__tests__/services/analysisSessions.test.ts |
ChangeBuffer stages scope records and overlays book-scoped link lookups. AnalysisSessions.edit uses a staged scope when present. |
Boundary commands and parity src/services/commands/boundaryCommands.ts, src/main.ts, src/types/interlinearizer.d.ts, src/utils/phrase-arc.ts, src/__tests__/services/commands/parity.ts, src/__tests__/services/commands/boundaryCommands*.test.ts, src/__tests__/main.test.ts, AGENTS.md |
Merge, split, and move commands stage segmentation changes. Split and move also split approved phrases that cross the edited boundary. Tests compare host commands with reducer behavior and cover no-op edits. |
Segment free-translation writes src/services/commands/freeTranslationCommands.ts, src/services/commands/staleCommands.ts, src/store/analysisSlice.ts, src/main.ts, src/types/interlinearizer.d.ts, src/__tests__/services/commands/freeTranslationCommands.parity.test.ts |
The command updates, clears, adopts, or creates segment translation records. Reducer updates and parity tests cover approved and stale translations. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant interlinearizer
participant bookService
participant boundaryCommands
participant ChangeBuffer
participant AnalysisSessions
interlinearizer->>bookService: Read the token's verse book
bookService-->>interlinearizer: Return the tokenized book
interlinearizer->>boundaryCommands: Run merge, split, or move
boundaryCommands->>ChangeBuffer: Read scope and stage segmentation
boundaryCommands->>ChangeBuffer: Stage splits for approved phrases across a boundary
AnalysisSessions->>ChangeBuffer: Build and apply the edit change set
Possibly related issues
sillsdev/interlinearizer-extension#445— The change uses phrase splitting during boundary edits, but does not add every phrase command.sillsdev/interlinearizer-extension#451— The change adds phrase splitting for boundary edits, but not the separate rule for overlapping approved phrases.sillsdev/interlinearizer-extension#520— The change breaks phrases that cross new segment boundaries, which relates to the issue's phrase-span behavior.sillsdev/interlinearizer-extension#450— The new commands provide a command layer for segment-page analysis, but do not add segment-page queries.sillsdev/interlinearizer-extension#453— The change adds host commands, but does not wire the WebView to those commands.
Suggested reviewers: alex-rawlings-yyc
Merge Risk: 
Minimal · previous review 502e1
-
This PR adds host commands for segment free translations, segment merge and split, boundary moves and phrase splitting, with parity tests against the WebView behavior. I found no concrete merge-blocking defect in the supplied context. The test and coverage results come from the PR description and were not independently run, so normal CI should still confirm them before merge.
Pre-merge checks: 
5
-
Full details: Linked Issues check
-
Explanation
Direct issue
#446requires segment free-translation and boundary commands with parity tests. The reviewed changes addwriteSegmentFreeTranslation,mergeSegments,splitSegment, andmoveBoundary. Boundary commands stage segmentation and split approved phrases cut by split or move in the sameChangeBufferchange set. The parity suites compare host and WebView results for free-translation and boundary cases. The focused boundary implementation and parity test confirm this behavior.
Full details: Out of Scope Changes check
-
Explanation
The changes support issue
#446.ChangeBuffer, scope staging, book lookup, document indexes, phrase guards, stale-translation helpers, reducer tie rules, WebView parity behavior, and tests provide the command implementation or its parity coverage. The reviewed changes show no demonstrated unrelated product change.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 60 functions across 21 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Open decisions for whoever takes this over. The current branch takes the first option in each.
|
491fb38 to
1c28e44
Compare
a83cb5f to
502e1e5
Compare
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc reviewed 22 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec and jasonleenaylor).
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 22 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on jasonleenaylor).
writeSegmentFreeTranslation ports its reducer step for step, keeping the stale-translation helpers module-private for the stale-handling commands to reuse. mergeSegments, splitSegment and moveBoundary copy the WebView's boundary dispatch: a split or move breaks every approved phrase the new boundary would cut in the same change set as the boundary change, which rides in the scope record. ChangeBuffer stages and reads the scope and finds links by book, and a session edit keeps a staged scope when it lists the analysis language. The host reads the verse-tokenized book through bookService and builds document order with the index builder extracted from useBookIndexes, so splitPhrase no longer takes a caller-supplied token order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The WebView's stale free-translation lookup, used to adopt, keep, clear or discard a stale translation, now picks the stale link an approval tie picks rather than the first one listed. A boundary split or move now breaks the phrases it cuts in link-id order in the WebView as on the host, so when two cut phrases share a payload, the same one forks on both sides. Only tie cases change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The stale commands already find a stale translation and approve it for a segment; the free-translation write adopting one now goes through the same two functions, so keeping and adopting can't drift apart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4e00334 to
13cd768
Compare
Part of #426; closes #446. Stacked on #516; review that first.
Summary
The segment free-translation and boundary commands, with parity tests. A split or move breaks every approved phrase the new boundary cuts, in the same
ChangeSet.writeSegmentFreeTranslationcopies its reducer, adopt-stale branch included. It adopts through Add stale-handling commands with parity tests #511'sfindStaleTranslationand a newapproveStaleTranslation, whichkeepStaleFreeTranslationnow uses too (as noted on #446).segmentationDispatchin the loader, wrapped by the force-break inInterlinearizer.tsx.createPhrase,updatePhrase,mergePhrasesandapprovePhraserefuse tokens that aren't all in one segment of the text, as the staged boundaries segment it. They read the segments through a newbookService.getBookSegments, which shares the page reads' cache.ChangeBuffergainsgetScope,putScopeandtoChangeSetemittingscope;AnalysisSessions.editstamps the analysis language onto a staged scope instead of overwriting it. Undo and redo already invert scope.buildBookIndexesmoves out ofuseBookIndexesintoutils/book-indexes.ts; the hook and the host both call it. The host goes throughbookIndexesOf, which builds once per segments array.preferredLink, andphrasesStraddlingBoundaryreturns cut phrases in link-id order, so the WebView and the host agree whatever order links are stored in. Only tie cases change in the WebView.Evidence
runParity, including stale ties stored loser first, writes that change nothing, and a cleared translation that another link names.runBoundaryParity, which seeds analysis and segmentation, runs the WebView's composition (transform plus force-break through the reducer) against the command, and compares both plus the repository's indexes. They cover splits cutting none, one or several phrases, a discontiguous phrase whose gap holds the boundary, halves of 1 and 2+ tokens, two cut phrases sharing a payload stored out of id order, links in another book, a token the text lacks, moves, merges, edits restoring the default, and no-ops staging nothing.maintests refuse each guarded phrase command over two segments, and over a token the text lacks. They also check that the guard reads the draft's boundaries in the phrase's book, keyed as the page reads key them.tscclean.Decisions with a strong alternative
splitPhrasedropstokenDocOrder(from Add phrase commands with parity tests #516): the host builds it from its verse book, so the WebView needs no book-wide token order. Nothing calls it yet. It now throws when the project has no text for the phrase's book.splitPhraseand the guarded phrase commands. The WebView can't reach that case.getVerseBookandgetBookSegmentsread throughgetSegments' held-book lookup, so a command naming a book that is neither open nor latest makes it the project's latest.newIdgenerator, since the number of phrases to break isn't known up front.Merge Danger
Door: two-way
Nothing calls the new commands. WebView behavior changes in two places. Ties now pick by rule: which stale link a free-translation write or keep adopts through, and which of two cut phrases sharing a payload keeps it. A free-translation write that changes nothing leaves the store as it was, and clearing a translation keeps an analysis that another link names.
Blast Radius:
bookService,AnalysisSessions.edit,ChangeBuffer,useBookIndexes,phrasesStraddlingBoundary, the free-translation reducer, the phrase commands'main.tswrappers. #507 (3j) also changesbookServiceheld books andAnalysisSessions, so whichever lands later takes a merge there. 3k (#450) and 3l (#451) build on the host-side order and scope staging; 4a (#453) must handle the "no text for book" and segment-guard errors. #507 (3j) and this PR: whichever lands later runs the open book's reanchor pass after each boundary command. The WebView re-anchors after every boundary edit, which stales a free translation whose segment a split or merge changed, and 4a turns that pass off.🤖 Generated with Claude Code
This change is
Summary by CodeRabbit