LT-22673: Convert the Reversal Entries slice to Avalonia

[papeh]

• Fill out the placeholder ReversalIndexEntryPlugin to work similarly to the WinForms slice. Jira: LT-22673.
• Add a mechanism to hide ifdata slices (rather than reporting "unsupported")

Difference from Legacy:

• Slots don't wrap inside themselves. A long entry moves whole to the next line; one wider than the line scrolls inside its slot.
• When unlinking and deleting a subentry cascade deletion as long as the next parent RIE has neither subentries nor linked senses. (Legacy deleted only the unlinked entry itself, if it was no longer used.) This prevents leaving orphan parents.

Future work:

• Fix Ctrl+Z and Ctrl+Y. Presently, within any Avalonia slice, these Un- and Redo text changes within the focused slice or slot, but do not work with the main FieldWorks undo stack. Jira: LT-22820

---

Claude's description

What changed

• New FwReversalEntriesField control (FwAvalonia, LCModel-free). It shows one group per visible analysis-writing-system reversal index, labelled with the writing system's abbreviation.
  • Entries sit on one wrapping line, separated by bars, followed by an add slot that fills the rest of the line.
  • Typing in the add slot opens another add slot right away.
  • Each entry's forms in other writing systems show as a read-only suffix.
  • Keys:
    • Left/Right at a slot's edge (Ctrl allowed) move into the next slot, across groups, mirrored in right-to-left groups.
    • Up/Down move between visual lines at the same horizontal position, across groups; that position holds across a run of them.
    • Home/End go to the ends of the visual line; with Ctrl, to the first and last slot of the whole field.
    • Tab/Shift+Tab visit every slot, then continue to the next or previous slice. (Legacy: tab navigates between slices)
    • An arrow pressed with a selection collapses it and goes no further, the way a text box does; Shift+arrow still extends.
    • Enter does nothing; Escape discards unsaved text.
  • Ctrl+click, or the right-click "Show in Reversal Index", jumps to the entry, or for a subentry its main entry, in the Reversal Index tool.
• Saving (ReversalDetailEditContext, plugin side)
  • The field saves once, when focus leaves it, as one undo step ("Undo change to Reversal Entries").
  • Each changed row finds or creates its colon chain (body: arm: hand, LT-4665) and links the deepest entry; an existing entry is never renamed.
  • Emptying a row unlinks its entry. The entry is deleted if it has no senses and no subentries left, and so is each parent that leaves empty.
  • Rows are matched in NFD, so accented text finds its stored entry.
  • No reversal index is created just to show a group (LT-4480). Entries in hidden writing systems are left alone.
• Shared detail-view plumbing (general mechanisms, recorded in the control exemplar map):
  • DetailField.ControlFactory now receives the render-time SliceFactoryContext; plugins reach it as SlicePluginBuildContext.Render (jump callback, abbreviation-column width). It also carries the view's Cancel, so a plugin that cannot finish a write cancels the way Escape does and the view re-shows.
  • Plugin rows honour visibility="ifdata"; the row is hidden when the sense has no entries.
  • DetailEditContextBase.AddPendingEditFlush, with DetailEditContextHolder.Settle() flushing first. An editor that saves only on focus loss still gets its edits in when the host saves with focus inside it (navigation, refresh, tool switch). Registrations are keyed by row, so a rebuilt row replaces its earlier one.
  • Replaced editors are disposed. The view disposes the ones a section toggle rebuilds and implements IDisposable; its WinForms host disposes the content each re-show or message replaces, and its own content when it is disposed. Every editor's Dispose only detaches its own handlers, and what a rebuild or swap replaces is disposed after it leaves the tree, so a focus loss its removal raises still lands.
  • New theme token DataTree.CaretAllowance.

Reviewer notes

• Editor disposal reaches every detail editor, not just this slice's: the ordering in DataTree.RebuildItems and AvaloniaHostControlBase.ReplaceContent is worth a look.
• Most complex parts: the two-phase batch commit in ReversalDetailEditContext.TryCommitRows, row-key rebinding after a commit, and add-slot growth and removal in FwReversalEntriesField.
• Two Devin review rounds are folded in. Fixed: the stale Up/Down column, a failed batch leaving half-written links, save hooks piling up on section toggles, and (from the re-review) a failed batch's rollback leaving the view stale; replaced editors are now disposed as well. Its severe Undo finding is fixed too: clicking Undo while a slot still holds typed text now saves that text as its own step and cancels the Undo, as it already did for a Gloss edit, and an Undo spent on held text that then fails to save no longer undoes the step before as well. Text that changes nothing, such as spaces, holds nothing, and a failed write takes only one Undo. A row that cannot be resolved fails its whole batch before anything is written.
• A failed batch now cancels the session through the view, as Escape does: it rolls back, discarding whatever else it held, and the view re-shows from the model so no field is left showing rolled-back text. Discarding the rest matches the policy the holder already applies to an edit that fails validation.

Validation

• build.ps1 -CommentHygiene -TokenHygiene: clean.
• FwAvaloniaTests: 846 of 847 passed, 0 failed, 1 skipped.
• xWorksTests: 1725 of 1733 passed, 0 failed; the rest are tests that don't run by default.
• New headless control tests and LCModel tests cover grouping, commit batching, cascade deletion, NFD, undo/redo, RTL, keyboard navigation (arrows, Home/End, Tab -- including inside a real DataTree), selection handling, a failed batch cancelling through the view, a rebuilt row keeping only its live control, editor disposal on rebuilds and content swaps, the host settle flush, and Undo saving text a slot still holds, including when that save fails.
• The author tested this manually, but before the preflight fixes and the rebase onto LT-22688. A manual re-check is still wanted.

Preflight review details

Code Review Summary

Branch: feature/LT-22673-Avalonia-convert-reversal-slice
Base: origin/main (b6cbf8e, after the in-review rebase)
Date: 2026-09-25
Review model: Claude Opus 5.5
Files changed: 22 (5 commits; the Tab change is b2d6b92)

Overview

The author's purpose: port the sense-side Reversal Entries slice
(ReversalIndexEntrySlice, field ReferringReversalIndexEntries) to the
Avalonia detail view with WinForms parity, replacing the reduced-scope
ReversalIndexEntryPlugin (Jira LT-22673). The new LCModel-free
FwReversalEntriesField control shows one group per visible analysis
reversal index, with separator-barred slots on one wrapping line, an add slot
that grows as you type, arrow/Home/End navigation across slots, and a
Ctrl+click / context-menu jump to the Reversal Index tool. The plugin's
ReversalDetailEditContext resolves colon chains find-or-create (LT-4665),
never renames an existing entry, unlinks emptied rows, and cascades deletion
of emptied ancestors. The row hides when the sense has no entries (ifdata).

The analysis found that the first design committed row by row and on the
field's own focus loss. That lost data when a user swapped or shifted text
between slots, and when the host saved or re-showed while focus was still in
the field. It also missed Unicode normalization and a deleted-sense guard.
All of these were fixed during the review.

Contract/API Changes

• New IReversalEntryEditing (FwAvalonia.Detail): TryCommitRows (batch),
  TryCommitRow, IssueAddRowKey, TryResolveMainEntryGuid. It is
  obtained with ctx as IReversalEntryEditing; the core IDetailEditContext
  is unchanged.
• New public types FwReversalEntriesField, DetailReversalGroup,
  DetailReversalRow, DetailReversalAlternative.
• DetailField.ControlFactory is now Func<SliceFactoryContext, Control>
  (was a parameterless factory); SliceFactory.CreateCustom passes the
  render context through.
• SlicePluginBuildContext gains Render (the SliceFactoryContext) and
  VisibleWritingSystems.
• DetailEditContextBase gains AddPendingEditFlush / FlushPendingEdits;
  DetailEditContextHolder.Settle() now flushes before checking IsOpen.
• New theme token DataTree.CaretAllowance, exposed as
  FwAvaloniaDensity.CaretAllowance.
• New resx strings ReversalShowInReversalIndex, ReversalAddEntryName.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

• Row-by-row commits lose data when text moves between slots
  (ReversalIndexEntryPlugin.cs, FwReversalEntriesField.cs). Swapping two
  slots' text, or shifting text up a slot, deleted an entry that another row
  was about to take over, because each row released its old entry before the
  next row linked. (fixed during review: the control sends every changed
  slot as one TryCommitRows batch; the plugin links every target first,
  then unlinks only released entries no row still shows. Tests:
  SwappingTwoRows_KeepsBothEntriesLinked,
  ShiftingTextUpARow_DeletesOnlyTheEntryNoRowKeeps.)
• Saves and re-shows run out of order with the field's focus-loss
  commit (FwReversalEntriesField.cs, DetailEditContextHolder.cs).
  Escape (the view's cancel) still saved the typed text on the focus loss that
  followed the re-show, and a navigation, refresh or tool switch settled the
  session before the field had staged its edits, so they were lost or landed
  after the recompose. (fixed during review: Escape restores every slot to
  its saved text; the plugin registers CommitPendingEdits with the host
  context, and Settle() flushes it before deciding whether a session is
  open. Tests: Escape_RestoresEverySlotsSavedText_AndSavesNothing,
  CommitPendingEdits_SavesWhileFocusIsStillInside,
  Settling_SavesWhatTheFieldHolds_WhileFocusIsStillInIt.)
• No Unicode normalization when matching typed text to stored forms
  (ReversalIndexEntryPlugin.cs). Precomposed typed text never matched
  decomposed stored forms, so an unchanged row looked changed and a
  find-or-create made a duplicate entry. (fixed during review: SplitForms,
  ChainMatches and FindDeepest compare NFD forms. Test:
  PrecomposedTyping_MatchesADecomposedStoredForm.)
• No guard for a deleted sense, and write exceptions go unhandled
  (ReversalIndexEntryPlugin.cs). (fixed during review: TryCommitRows and
  TryResolveMainEntryGuid return false/null for an invalid sense; a failed
  write logs, restores the row bindings and returns false. Test:
  ACommitForADeletedSense_ChangesNothing.)
• Branch was behind origin/main, which adds
  IDetailEditContext.TryResetReferenceOrder and conflicts in
  ReversalIndexEntryPlugin.cs (e278320). (fixed during review:
  rebased onto origin/main, resolved the conflict, and added
  TryResetReferenceOrder => false to ReversalDetailEditContext and the
  test fake. The branch was already on origin at 87d1b66, so it needs a
  force-push with lease.)

Minor - Consider

• SlicePluginBuildContext was growing one constructor parameter per
  render-time service (SlicePlugins.cs). (fixed during review: it takes
  the SliceFactoryContext as Render instead of separate link-callback and
  column-width parameters.)
• A write that throws inside an already-open session leaves its partial
  writes in that session. (fixed in the Devin rounds below: a failed batch
  now cancels the session through the view.)
• Flush registrations build up per rendered control. (fixed in the
  Devin rounds below: registrations are keyed by row.)

Devin review (2026-09-30, head db4e96f)

Five bugs and one flag, all verified against the code before acting.

• Severe: Undo undid the wrong step while a slot held typed text. The
  undo guard checked for an open session, which this field opens only on
  focus loss, so clicking Undo undid the step before and the held text was
  saved after it. (fixed in c3d9db7: the guard asks editors for what they
  hold first, so held text opens the session and the guard saves it as its
  own step and cancels that Undo, as for a Gloss edit. Tests:
  AnUndoGesture_SavesWhatTheFieldHolds_BeforeItRuns,
  UndoGuard_WithAnEditorHoldingAnEdit_SavesItInsteadOfUndoingTheStepBefore.
  This finding was first described as a Ctrl+Z problem; the guard covers the
  Undo command itself. Ctrl+Z inside a text box is a separate, still-open
  issue.)
• Severe, from the re-review of c3d9db7: a failed save undid an older
  step. When saving held text failed, the failure closed the session --
  the one the save opened, or another field's -- so the guard let the Undo
  through and it undid the step before. (fixed in 2a50804: a pending-edit
  flush reports whether its editor held anything, and the guard cancels the
  Undo whenever one did, saved or not. A before-and-after check of the
  session would miss the case where the failing save opened the session
  itself. Tests: AnUndo_WhoseHeldEditFailsToSave_LeavesTheStepBeforeAlone,
  AnUndo_WhoseHeldEditFailsInAnotherFieldsSession_LeavesTheStepBeforeAlone.)
• From the re-review of 2a50804: Undo stayed blocked by unsaved text.
  A slot whose text the commit could not stage kept differing from its saved
  text, so it was reported held on every flush and every Undo was cancelled
  -- for text that changes nothing (spaces in an empty add slot) and for a
  failed write. (fixed in 02431f6: the batch commit reports Staged,
  Unchanged or Failed. Unchanged text becomes the slot's saved text and is
  not held. A failed write takes the one Undo that asked for it, then the slot
  shows its saved text again, matching the view's re-show when a failure
  cancels another field's session. Tests:
  TextThatChangesNothing_IsNotHeld_AndIsNotOfferedAgain,
  AFailedSave_IsHeldOnce_ThenTheSlotShowsWhatIsSaved,
  AnUndo_WithOnlySpacesInAnAddSlot_UndoesTheStepBefore, and a second Undo
  in both failed-save tests.)
• From the re-review of 02431f6: unresolvable rows appeared saved.
  The scan skipped an edit whose row key was unknown or whose index's writing
  system did not resolve, and reported the rows unchanged, so the field took
  the text as saved; in a mixed batch the skipped row's text passed as saved
  with the valid rows'. (fixed in 081de97: such a row fails the whole
  batch before anything is written, keeping the batch all-or-nothing as a
  mid-write failure already does. Test:
  ARowThatCannotBeResolved_FailsTheBatch_WritingNothing.)
• Shift+Up/Down left a stale column for the next plain arrow. Took
  Devin's one-line fix: any arrow with a modifier ends the run. Test:
  AModifiedArrow_RestartsThePositionUpAndDownNavigateBy.
• A failed batch could leave half-written links. A batch that throws
  now cancels the session, discarding whatever else it held -- the policy the
  holder already applies to an edit that fails validation. The row scan moved
  inside the same guard, since it reads an index that may be deleted. Test:
  AFailedBatch_ClosesTheSession_LeavingNothingToSave. (Revised in the next
  round to cancel through the view.)
• Escape leaves two empty add slots until the re-show (author
  confirmed this is not a problem)
• A section toggle registered another save hook and kept the discarded
  control. AddPendingEditFlush now takes the row's own id as a key, so a
  rebuild's registration replaces the one before it and the context holds
  only the live control. Test:
  ARebuiltRow_LeavesOnlyTheLiveControlHoldingEdits. Devin's second half --
  having the detail view dispose replaced editors -- was done next round.
• The key-map comment exceeds the 200-character budget (the repo's
  own checker raises the budget to 600 in dense branching code and passes it
  clean)

Author's own change in the same round: an arrow pressed with a selection now
collapses it and goes no further, at the end the arrow points at, mirrored in
a right-to-left group; the next press moves. Shift+arrow still extends.

Devin re-review (2026-10-01, head 6b2962b)

Five bugs and two flags; four bugs and one flag were carried over from the
first round and are dispositioned above.

• Severe, new: earlier reversal edits vanished after a failed batch's
  rollback. Caused by the first-round fix, which cancelled the session
  directly, behind the view. No re-show followed, so the reversal field kept
  treating its rolled-back edits as saved and never retried them -- and every
  other field kept showing edits the cancel had rolled back. (fixed: the view
  now hands its controls its own cancel through SliceFactoryContext, and a
  failed batch uses it whenever a session is still open, so the session rolls
  back and the view re-shows from the model, as Escape does. Without a view
  the session is cancelled directly, as before. The host queues the re-show
  until the call stack unwinds, so a cancel fired inside a focus loss or a
  Settle() flush is safe. Tests:
  AFailedBatch_InAnotherFieldsSession_CancelsThroughTheView,
  AFailedBatch_WithNothingElseStaged_LeavesTheViewAlone,
  CustomField_FactoryReceivesTheViewsCancel_WhichAlsoCompletesTheEdit.)
• Collapsed rows keep their save hook until the section reopens.
  (fixed with editor disposal: the view disposes the editors a rebuild
  replaces and implements IDisposable; its WinForms host disposes the
  content a swap replaces, and its current content on its own disposal. What
  a rebuild or swap replaces is disposed only after it is out of the tree, so
  a focus loss its removal raises still reaches its handlers. The Reversal
  Entries field raises
  Disposed, and the plugin drops its flush from the host then, leaving any
  later registration for the row in place. Tests:
  CollapsingASection_DisposesTheEditorsItRemoves,
  DisposingTheView_DisposesEachOfItsEditorsOnce,
  SwappingContent_DisposesWhatItReplaces_AndNothingElse,
  DisposingTheHost_DisposesWhatItShows,
  ADisposedField_LetsGoOfOnlyItsOwnRegistration.)

Required Validation / Evidence

Run on the rebased branch, with the in-review fixes staged:

• .\build.ps1 -CommentHygiene -TokenHygiene: succeeded, 0 warnings,
  0 errors; comment hygiene and token hygiene clean. (Two auto-rewrapped
  comment fragments from the first build were reflowed by hand.)
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -TestProject Src/Common/FwAvalonia/FwAvaloniaTests: 823 total, 822 passed, 0 failed,
  1 skipped.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -TestProject Src/xWorks/xWorksTests: 1725 total, 1717 passed, 0 failed (the rest are
  explicit or ignored tests that don't run by default).
• git diff --cached --check: clean.
• After the Devin round (a09aae9): FwAvaloniaTests 840 total, 839 passed,
  0 failed, 1 skipped; xWorksTests 1726 total, 1718 passed, 0 failed; both
  hygiene checks clean; gitlint clean.
• After removing four redundant tests (78c87ed) and the save-hook fix
  (6b2962b): FwAvaloniaTests 839 total, 838 passed, 0 failed, 1 skipped;
  xWorksTests 1724 total, 1716 passed, 0 failed; hygiene and gitlint clean.
  One run hit a clipboard-contention failure in an unrelated test, which
  passed on its own.
• After cancelling a failed batch through the view (86b8157):
  FwAvaloniaTests 840 total, 839 passed, 0 failed, 1 skipped; xWorksTests
  1726 total, 1718 passed, 0 failed; hygiene and gitlint clean. The held-back
  undo-guard stash still applies cleanly (git apply --check).
• After disposing replaced editors (622cf15): FwAvaloniaTests 844 total,
  843 passed, 0 failed, 1 skipped; xWorksTests 1727 total, 1719 passed,
  0 failed; hygiene and gitlint clean; the stash still applies cleanly.
• After moving from the drawn caret (92093e1): FwAvaloniaTests 845 total,
  844 passed, 0 failed, 1 skipped.
• After settling held edits before an Undo (c3d9db7): xWorksTests 1729
  total, 1720 passed, 1 failed -- the clipboard-contention flake in an
  unrelated test, not re-run; hygiene and gitlint clean. CI on c3d9db7: 6,477
  tests, 0 failed.
• After spending the Undo on a held edit that fails to save (2a50804):
  FwAvaloniaTests 845 total, 844 passed, 0 failed, 1 skipped; xWorksTests
  1731 total, 1723 passed, 0 failed; hygiene and gitlint clean.
• After holding an edit for one Undo only (02431f6): FwAvaloniaTests 847
  total, 846 passed, 0 failed, 1 skipped; xWorksTests 1732 total, 1724
  passed, 0 failed; hygiene and gitlint clean. CI on 2a50804: 6,479 tests,
  0 failed.
• After failing a batch with an unresolvable row (081de97): xWorksTests
  1733 total, 1725 passed, 0 failed; hygiene and gitlint clean.
  FwAvaloniaTests not re-run: only doc comments changed there, and they
  compiled.
• Still needed: a manual re-check in FieldWorks. The author confirmed a full
  manual pass, but it came before the in-review fixes and the rebase (which
  brought in LT-22688 tab navigation). Worth re-checking: swapping two slots,
  Escape after typing, switching entries with focus in a slot, and an
  accented form typed into an add slot where the entry already exists.
• gitlint on origin/main..HEAD (the 3 rebased commits): clean. fb15242
  and the commit for the staged Tab change still need the same check;
  "fix misstaging" and the "in progress" title may be worth rewording or
  squashing before the PR.

Ticket: LT-22673 exists and covers the user-visible change.

Positive Observations

• The control is LCModel-free and tested headlessly through a recording
  fake; the LCModel side has its own fixture against real data, including
  undo/redo, cascade deletion, homographs, RTL and bidi forms.
• Commits ride the host's fenced session, so one field visit is one undo
  step labelled with the field ("Undo change to Reversal Entries").
• No reversal index is created just to show a group (LT-4480), and entries
  in hidden writing systems are left alone.
• ifdata on plugin rows and render-time host services are general
  mechanisms, recorded in the control exemplar map for later slices.

Interview Notes

• Purpose: the author accepted the proposed purpose statement as written.
• Findings 1-4: the author asked for all four to be fixed now.
• Finding 5: the author chose to rebase onto origin/main and force-push later.
• Minor (SlicePluginBuildContext): the author asked for the change now.
• Manual testing: the author said "Yes, fully". This came before the
  in-review fixes; see Required Validation.
• Walkthrough of the most complex part: "I'm not sure."
  Author does not understand: the most complex parts of the change
  (field-level save on focus loss, row-key rebinding after commit, add-slot
  growth).
• Author notes for reviewers:
  • Up and down arrow keys don't move between slots yet.
  • Slots don't wrap inside themselves. A long entry moves whole to a new
    line. If one entry is wider than the line (unlikely), the user can scroll
    through it with the arrow keys or by click-and-drag selecting.
  • Tab navigation: at interview time Tab didn't work for any Avalonia slice.
    After the rebase brought in LT-22688, the author asked for Tab/Shift+Tab
    to visit every slot (add slots opened while typing included) and to
    leave the field only from its last or first slot. Implemented and
    tested; see In-Review Quality Check.

In-Review Quality Check

INTERVIEW_CHANGES (the author committed these as fb15242, "Claude cleaning
up & fixing tests - in progress"; the Tab change at the end is b2d6b92):

• IReversalEntryEditing.cs: TryCommitRows batch contract.

• ReversalIndexEntryPlugin.cs: two-phase batch commit, NFD matching,
  sense guard, failure logging with binding restore, flush registration,
  TryResetReferenceOrder, render context through Render.

• FwReversalEntriesField.cs: per-slot saved-text state, one batch per
  field visit, public CommitPendingEdits, Escape revert.

• DetailEditContextBase.cs, DetailEditContextHolder.cs: pending-edit
  flush hook and the flush in Settle().

• SlicePlugins.cs, DetailComposer.cs: SlicePluginBuildContext.Render.

• Tests: FwReversalEntriesFieldTests.cs (batch count, Escape, explicit
  flush, fake updates), ReversalEntriesComposeTests.cs (swap, shift,
  other-writing-system forms, NFD, deleted sense, settle flush; idempotent
  AddAnalysisWs), LexemeEditorInventoryTests.cs (render context passed
  through).

• FwReversalEntriesField.cs (requested after the rebase): Tab and
  Shift+Tab step slot by slot in reading order and fall through to the
  view's tab walk only at the field's ends; a slot opened while typing takes
  its row's tab index so the walk can leave from it. Six new tests, one of
  them inside a real DataTree. FwAvaloniaTests afterwards: 829 total,
  828 passed, 0 failed, 1 skipped; hygiene clean. xWorksTests not re-run
  (no xWorks change).

The build, both hygiene checks, and both test projects pass on the rebased
branch (see above).

Suggested Review Focus

• ReversalDetailEditContext.TryCommitRows: two-phase link/unlink and
  the stillWanted set, especially with cascade deletion.
• DetailEditContextHolder.Settle() flushing before the IsOpen check:
  every host save path now stages the field's held edits first.
• FwReversalEntriesField focus-loss logic (FocusIsInside, add-slot
  removal) and the Escape revert that leaves the key unhandled for the view.
• Row-key rebinding after a commit, and add-slot key issuing while typing.
• Tab handling in WireSlotNavigation alongside LT-22688's row tab
  indexes, and the tab index copied onto slots opened while typing.

🤖 Generated with Claude Code

---

This change is 

Devin

[github-actions]

NUnit Tests

     2 files  +    1       2 suites  +1   21m 57s ⏱️ + 8m 24s
 6 501 tests +  120   6 415 ✅ +  119   85 💤 ± 0  1 ❌ +1 
13 020 runs  +6 630  12 849 ✅ +6 544  170 💤 +85  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 1cf7df5. ± Comparison against base commit a6d34bd.

This pull request removes 2 and adds 122 tests. Note that renamed tests count towards both.

SIL.FieldWorks.XWorks.DetailComposerTests ‑ Compose_SenseWithReversalEntry_ComposesEditableReversalRow_NotUnsupported
SIL.FieldWorks.XWorks.DetailComposerTests ‑ ReversalPlugin_EditingAForm_StagesAndCommitsThroughTheEditContext

FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AChangedRow_CommitsOnceWhenItLosesFocus
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AClick_RestartsThePositionUpAndDownNavigateBy
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AFailedSave_IsHeldOnce_ThenTheSlotShowsWhatIsSaved
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AGroup_ShowsItsEntries_ThenOneAddRow_UnderOneLabel
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AGroupsSlots_ShareOneLine_WithABarBetweenEachPair
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ ALoneAddSlot_HasNoBar
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ ALongEntry_StaysOnOneLine_InsideItsSlot
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ AModifiedArrow_RestartsThePositionUpAndDownNavigateBy
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ ARightToLeftGroup_FlowsRightToLeft
FwAvaloniaTests.Detail.FwReversalEntriesFieldTests ‑ ARunOfUpAndDown_KeepsTheHorizontalPositionItStartedFrom
…

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 89.43005% with 102 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.53%. Comparing base (b6cbf8e) to head (1cf7df5).
⚠️ Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
...Common/FwAvalonia/Detail/FwReversalEntriesField.cs	90.32%	17 Missing and 39 partials ⚠️
...Works/Avalonia/Plugins/ReversalIndexEntryPlugin.cs	89.74%	9 Missing and 19 partials ⚠️
Src/xWorks/Avalonia/Composer/DetailComposer.cs	57.14%	5 Missing and 4 partials ⚠️
Src/xWorks/Avalonia/DetailEditContextHolder.cs	69.23%	4 Missing ⚠️
Src/xWorks/Avalonia/DetailEditContextBase.cs	85.71%	1 Missing and 2 partials ⚠️
Src/Common/FwAvalonia/AvaloniaHostControlBase.cs	80.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1163      +/-   ##
==========================================
+ Coverage   39.01%   39.53%   +0.52%     
==========================================
  Files        1520     1527       +7     
  Lines      352805   354099    +1294     
  Branches    40692    40966     +274     
==========================================
+ Hits       137632   139992    +2360     
+ Misses     185879   184802    -1077     
- Partials    29294    29305      +11     

Files with missing lines	Coverage Δ
Src/Common/FwAvalonia/Detail/DataTree.cs	96.06% <100.00%> (+0.17%)	⬆️
Src/Common/FwAvalonia/Detail/DetailModel.cs	78.55% <100.00%> (ø)
Src/Common/FwAvalonia/Detail/FwFieldControls.cs	87.10% <100.00%> (+3.64%)	⬆️
Src/Common/FwAvalonia/Detail/SliceFactory.cs	95.55% <100.00%> (+0.10%)	⬆️
Src/Common/FwAvalonia/FwAvaloniaDensity.cs	96.61% <100.00%> (+0.05%)	⬆️
Src/Common/FwAvalonia/FwAvaloniaStrings.cs	98.71% <100.00%> (+0.03%)	⬆️
...AvaloniaTheme/Tokens/DataTree/DataTreeTokens.axaml	100.00% <100.00%> (ø)
Src/xWorks/Avalonia/Plugins/SlicePlugins.cs	98.18% <100.00%> (+0.14%)	⬆️
Src/Common/FwAvalonia/AvaloniaHostControlBase.cs	44.85% <80.00%> (+3.44%)	⬆️
Src/xWorks/Avalonia/DetailEditContextBase.cs	74.74% <85.71%> (+9.36%)	⬆️
... and 4 more

... and 54 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[papeh]

Devin review findings: what happened to each

Finding	Outcome
Undo bypasses pending reversal edits (severe)	Fixed in c3d9db7: the undo guard now asks the slot for the text it holds first, so clicking Undo saves that text as its own step and cancels the Undo, as for a Gloss edit. (Not a problem for Ctrl+Z because this keystroke undoes only text edits within the current slot (LT-22820).)
Failed reversal save undoes an older step (severe)	Fixed in 2a50804: a pending-edit flush now reports whether its editor held anything, and the undo guard cancels the Undo whenever one did, even if saving it failed, so the step before is no longer undone on top of a failed save.
Undo remains blocked by unsaved reversal text	Fixed in 02431f6: the batch commit now reports whether it staged, found nothing to change, or failed. Text that changes nothing is not held, and a failed write takes only the one Undo that asked for it before the slot shows what is saved again.
Unresolvable reversal rows appear saved	Fixed in 081de97: a row whose key or writing system cannot be resolved now fails the whole batch before anything is written, so the field shows what is saved instead of treating the text as saved.
Earlier reversal edits vanish after rollback (severe)	Fixed in 86b8157: a failed batch cancels through the view (SliceFactoryContext.Cancel), so the view re-shows from the model.
Failed reversal batch can commit partial writes	Fixed in a09aae9 (the session is cancelled on failure), revised by 86b8157.
Rebuilt reversal rows retain old controls	Fixed in 6b2962b (pending-edit flushes keyed per row) and 622cf15 (replaced editors are disposed).
Collapsed rows retain their save hooks (flag)	Fixed in 622cf15: the field unregisters its flush when disposed.
Vertical navigation uses a stale column	Fixed in a09aae9 with the suggested change.
Escape leaves duplicate empty add slots	Not a problem (I, @papeh, tested).
Navigation comment exceeds comment budget (flag)	Not changed (the repo's comment checker allows longer comments in dense branching code and passes it).

(coauthored by Claude Opus 5.5)

[jasonleenaylor]

Looks like a good and careful port. Two small changes inline, and one non-blocking question about the ancestor cascade.

This review was assisted by Claude Opus 5.5.

[papeh]

Re Devin's finding "External updates remain hidden after unchanged reversal text" (FwReversalEntriesField.cs:750): confirmed and fixed in 5111f6d.

The gap predates the reversal field: a vector item retyped back to its original text had the same problem. In both cases no session opens, so no edit completes and nothing releases the held refresh until the next click.

DataTree's focus-loss handler now reports the view idle (DeliverWhenIdle -> InteractionCompleted -> ReleaseHeldRefresh) when no session is open and no editor still holds text. Moving between reversal slots still holds the refresh. Releasing is safe because AvaloniaDetailRefreshController re-checks busy before it rebuilds.

New test InTheDetailView_LeavingWithTextThatChangesNothing_ReleasesAHeldRefresh. FwAvaloniaTests (reversal, DataTree, DetailHostControl: 100/100) and xWorksTests (detail editing, scheduling, reversal: 86/86) pass with the hygiene checks clean.