[imnasnainaec]

Part of #426; closes #445 and #520.

Summary

The phrase commands, with parity tests.

interlinearizer.createPhrase(sessionId, tokens) -> ChangeSet applied
interlinearizer.updatePhrase(sessionId, phraseId, tokens) -> ChangeSet applied
interlinearizer.deletePhrase(sessionId, phraseId) -> ChangeSet applied
interlinearizer.mergePhrases(sessionId, targetPhraseId, tokens, absorbedPhraseId?) -> ChangeSet applied
interlinearizer.writePhraseGloss(sessionId, phraseId, value) -> ChangeSet applied
interlinearizer.approvePhrase(sessionId, phraseId) -> ChangeSet applied
interlinearizer.splitPhrase(sessionId, phraseId, splitAfterTokenRef, tokenDocOrder) -> ChangeSet applied

src/services/commands/phraseCommands.ts   # one command per phrase reducer, plus split

• Each command copies its reducer step for step: fork a shared payload, fold an edited payload into an identical one, drop a payload once nothing links it, refuse approval over a token already in an approved phrase.
• splitPhrase ports splitPhraseAtBoundary, which the WebView composes from update, create and delete. It stages all three on one buffer, so a split is one ChangeSet. 3g's segment split and move reuse its staging function to break the phrases a new boundary cuts.
• New lookup getPhraseAnalysisLinks(ids) on AnalysisRepository, both implementations, ChangeBuffer, and the conformance suite. Delete, gloss, approve and split get only a link id.
• phraseSurfaceText moves to utils/phrase-text.ts and sliceAtBoundary is exported, so the reducer and the command share one implementation.
• A gloss write keeps the payload's senseRef, blank writes included (#514's rule, noted on #445).

Evidence

• 47 parity cases run one input through the reducer and the command, then compare the analyses, reloaded from storage, in id order, and check that the repository's indexes still match its records. The split cases drive the reducer actions splitPhraseAtBoundary dispatches.
• main.test.ts runs each PAPI command over a session and checks its reply; writePhraseGloss runs over a stored draft in the session's language.
• Full suite at 100% coverage; lint and tsc clean.

Decisions with a strong alternative

• A split is one undo step. The arc split button calls splitPhraseAtBoundary outside asOneStep, so a split that keeps two phrases records two steps today, and the first undo leaves the second half's tokens in no phrase. One command gives one ChangeSet, as the boundary path's split already does. Users see this from [4a] Wire the WebView to the host behind a feature flag #453/[4b] Flip the host write path on by default #454.
• An empty token run is refused on createPhrase and mergePhrases. The reducer keeps a zero-token phrase, which the analysis index can't file (it keys on the first token), so apply threw a TypeError. The WebView can't send one.
• splitPhrase takes the caller's tokenDocOrder, copying splitPhraseAtBoundary's signature. The 3g PR (stacked on this one) builds the order in the host and drops the argument.
• Two reducer quirks fixed in both the reducer and its command: a blank gloss write with no gloss to clear stages nothing, so it neither forks a shared payload nor records an undo step that does nothing; mergePhrases with an unknown target leaves the absorbed occurrence alone instead of deleting it and its gloss.
• The approval rule covers phrases (Refuse phrase edits that put a token in two approved phrases #520), beyond the plan's "commands add nothing": it refuses an edit that puts a token in a second approved phrase, so createPhrase, updatePhrase and mergePhrases can't overlap one. The WebView could send one: a link click whose far neighbor was another fragment of the focused phrase absorbed the token beside the slot, even when another approved phrase held it. TokenLinkIcon now merges that phrase whole instead. main.test.ts refuses each of the three.

Merge Danger

Door: two-way

Nothing calls the new commands. Linking across a phrase that sits in the focused phrase's gap now merges it into the focused phrase, where the token beside the slot used to land in both. The writePhraseGloss and mergePhrases reducers change only for a blank write with nothing to clear and a merge into an unknown target. The approval rule also checks phrase links now; an edit that touches none can't breach it.

Blast Radius: the repository seam gains one method. #497 (3e) and #507 (3j) touch the same main.ts, interlinearizer.d.ts and main.test.ts blocks, so whichever lands later takes a small merge.

🤖 Generated with Claude Code

---

This change is 

Summary by CodeRabbit

• New Features
  • Added phrase editing capabilities, including creating, updating, deleting, merging, splitting, approving, and adding or clearing glosses.
  • Phrase approval now prevents tokens from being included in multiple approved phrases.
• Bug Fixes
  • Merging with an unknown target and clearing a gloss that is already empty now leave the analysis unchanged.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 73c11a0f-e7df-4c03-9dfd-e09ce21339b4

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 05952637-8ac3-49a7-a9a6-875c56bee20e

Commits

Reviewing files that changed from the base of the PR and between c92befb and bb267ec.

Files selected for processing: 23

• src/__tests__/analysis-repository-conformance.ts
• src/__tests__/main.test.ts
• src/__tests__/services/analysisIndex.test.ts
• src/__tests__/services/commands/approvalRule.test.ts
• src/__tests__/services/commands/changeBuffer.test.ts
• src/__tests__/services/commands/parity.ts
• src/__tests__/services/commands/phraseCommands.parity.test.ts
• src/__tests__/services/commands/phraseCommands.test.ts
• src/__tests__/services/projectStorage.test.ts
• src/__tests__/store/analysisSlice.test.ts
• src/main.ts
• src/services/analysisIndex.ts
• src/services/analysisSessions.ts
• src/services/commands/approvalRule.ts
• src/services/commands/changeBuffer.ts
• src/services/commands/phraseCommands.ts
• src/services/indexedRepository.ts
• src/services/projectStorage.ts
• src/store/analysisSlice.ts
• src/types/analysis-repository.ts
• src/types/interlinearizer.d.ts
• src/utils/phrase-arc.ts
• src/utils/phrase-text.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

This pull request adds phrase occurrence commands for creation, editing, deletion, merging, glossing, approval, and splitting. It adds phrase-link lookup to repository implementations and updates reducer behavior for phrase merge and gloss no-op cases.

Changes: Phrase commands

Layer / File(s)	Summary
Phrase-link lookup src/types/analysis-repository.ts, src/services/analysisIndex.ts, src/services/indexedRepository.ts, src/services/projectStorage.ts, src/services/commands/changeBuffer.ts, related tests	Adds getPhraseAnalysisLinks to repository contracts and implementations. The lookup returns requested stored links and skips missing IDs.
Reducer behavior and shared phrase utilities src/store/analysisSlice.ts, src/utils/phrase-text.ts, src/utils/phrase-arc.ts, src/__tests__/store/analysisSlice.test.ts	An unknown merge target and clearing an absent gloss now leave the analysis unchanged. Phrase surface text is provided by a shared utility, and sliceAtBoundary is exported.
Phrase operations and parity src/services/commands/phraseCommands.ts, src/services/commands/approvalRule.ts, src/types/interlinearizer.d.ts, src/__tests__/services/commands/*	Adds phrase occurrence operations with shared-payload handling, gloss updates, approval checks, and document-order splitting. Tests compare command results with reducer behavior and cover payload reuse, no-op cases, and approval constraints.
Session command wiring src/main.ts, src/__tests__/main.test.ts	Registers seven phrase commands with session handlers and parameter schemas. Tests cover returned changes, empty-token rejection, and approval-rule errors.

Priority: Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Main as main.ts
  participant Commands as phraseCommands.ts
  participant Buffer as ChangeBuffer
  participant Repository as AnalysisRepository
  Main->>Commands: Invoke phrase operation with request
  Commands->>Buffer: Read phrase links and stage edits
  Buffer->>Repository: Retrieve requested phrase links

Loading

Possibly related PRs

• sillsdev/interlinearizer-extension#514: Both changes preserve sense references when writing or clearing glosses.
• sillsdev/interlinearizer-extension#487: This pull request extends the indexed repository lookup seam introduced there.
• sillsdev/interlinearizer-extension#485: This pull request routes phrase commands through the session edit flow added there.

Suggested reviewers: alex-rawlings-yyc

Merge Risk: Minimal · previous review bb267

This change adds phrase commands that are not yet called by any consumer, with parity tests against the reducer. No concrete merge-blocking risk was found in the supplied context.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely summarizes the main changes: adding phrase commands and parity tests.
Linked Issues check		#445 requires phrase commands with parity tests. The PR adds create, update, delete, merge, gloss, approve, and split commands in src/services/commands/phraseCommands.ts. The commands stage edits th…
Out of Scope Changes check		The changes remain connected to #445. The repository lookup, phraseSurfaceText refactor, exported sliceAtBoundary, reducer fixes, approval-rule extension, command registrations, and tests support …
Docstring Coverage		Docstring coverage is 81.40% 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 surviv…

Full details: Linked Issues check

Explanation

#445 requires phrase commands with parity tests. The PR adds create, update, delete, merge, gloss, approve, and split commands in src/services/commands/phraseCommands.ts. The commands stage edits through ChangeBuffer, including shared-payload fork and fold behavior. src/main.ts registers all seven commands and returns the applied edit result. The PR adds parity coverage for phrase operations and PAPI session-command tests. The repository seam and getPhraseAnalysisLinks support the command implementations.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to #445. The repository lookup, phraseSurfaceText refactor, exported sliceAtBoundary, reducer fixes, approval-rule extension, command registrations, and tests support phrase command behavior or parity. The approval-rule extension prevents overlapping approved phrases during phrase edits. No unrelated change is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 81.40% 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 43 functions across 23 files.

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[imnasnainaec]

Open decisions for whoever takes this over. The current branch takes the first option in each.

1. A phrase split is one undo step. Today the arc split button calls splitPhraseAtBoundary outside asOneStep. A split that leaves two phrases records two steps, and the first undo leaves the second half's tokens in no phrase.
   • Keep one step: one intent, one ChangeSet, matching the boundary path's split. Users see the change from [4a] Wire the WebView to the host behind a feature flag #453/[4b] Flip the host write path on by default #454.
   • Match today: the WebView would call updatePhrase and createPhrase separately.
2. Empty token run on createPhrase / mergePhrases. The reducer stores a zero-token phrase, which the index can't file (it keys on tokens[0]). Unreachable from the WebView.
   • Refuse with an error at the command boundary (main.ts).
   • Remove the occurrence, as updatePhrase does for an empty run; change the reducer too to keep parity.
3. Reducer quirks the commands copy. For each: keep it, or fix both the reducer and the command in this PR.
   • A blank writePhraseGloss with nothing to clear still restamps updatedAt. On a shared payload it forks, the fork folds straight back, and only the link's updatedAt changes.
   • mergePhrases removes the absorbed occurrence even when the target doesn't exist.
   • createPhrase doesn't refuse tokens already in an approved phrase. Only approvePhrase checks overlap, and the approval rule covers token and segment links only.
4. getPhraseAnalysisLinks(ids) on AnalysisRepository.
   • Keep it: delete, gloss, approve and split get only a link id.
   • Drop it: callers send the link's current tokens and the command finds it through findLinksByToken. Those tokens can go stale between read and edit.

[alex-rawlings-yyc]

Decisions on the open questions above:

1. A phrase split is one undo step. Kept. Two steps let the first undo land in a state no one chose, and the plan's commands go by intent.
2. Empty token run on createPhrase / mergePhrases: refused at the command boundary, as now. For createPhrase there's no occurrence to remove, and for a merge, removing would turn a malformed request into a delete.
3. Reducer quirks:
   • Blank writePhraseGloss with nothing to clear: fixed in both the reducer and the command, so that nothing is staged. Typing only spaces into an empty phrase gloss reaches it, and on the host it would record an undo step that does nothing.
   • mergePhrases with a missing target: fixed in both, so that nothing is staged. It's the one quirk that loses data: a stale target id deletes the neighbor and its gloss.
   • createPhrase overlap: kept. updatePhrase and mergePhrases have the same gap, so the check belongs in the approval rule: Refuse phrase edits that put a token in two approved phrases #520. (Edit: I've decided to include it here after all).
4. getPhraseAnalysisLinks(ids): kept. The WebView names a phrase by its link id, and the index already stores phrase links by id.

[jasonleenaylor]

Looking good. One gap in the main.ts tests below.

This review was assisted by Claude Opus 5.5.

[jasonleenaylor]

LGTM

This review was assisted by Claude Opus 5.5.

[imnasnainaec]

@imnasnainaec reviewed 25 files and all commit messages.
Reviewable status: 0 of 25 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).

[alex-rawlings-yyc]

@alex-rawlings-yyc reviewed 25 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).