LT-22691: Answer the Delete leaf of the Lexicon detail menus natively

[mark-sil]

Start here: Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs, the Delete answerer and the host that runs it. Then Src/FdoUi/DetailRules/DetailObjectDeletion.cs, the rule both detail views now call.

What it does. Stage 2, PR A of the hidden-DataTree retirement: the first answerer of LexiconMenuAuthority. Delete Translation and Delete Complex Form Info run from the Avalonia row: hidden on a ghost or object-less row, disabled with the WinForms wording when the object is gone or is the last item of a required field, otherwise deleted through the object's CmObjectUi with its confirmation dialog, in one undo step. The re-show then focuses the nearest surviving neighbour row. mnuDataTree-Translation and mnuDataTree-ComplexFormSpec no longer build the hidden tree.

The question to settle. Message-level ownership reaches further than two ids. Every id whose leaves all send DataTreeDelete is now owned, 17 in all, pinned by area in LexiconMenuAuthorityTests. Two are live posEdit rows (Inflection Class Name, Stem Allomorph Label) and go native with the same engine; the rest are ids no part binds or ids of tools without an Avalonia detail view.

Where to look.

• DetailObjectDeletion: the enablement and wording lifted from Slice.GetCanDeleteNow and DTMenuHandler.OnDisplayDataTreeDelete; both call it. Pinned by DetailObjectDeletionTests and the DetailRules boundary test.
• CmObjectUi.DeleteUnderlyingObject(Func<bool>): the confirm dialog split out behind a delegate; the parameterless method behaves as before. LexiconMenuExecutionTests deletes headlessly through it, confirm and decline.
• RowsToFocusAfterDeleting and DataTree.FocusFirstRow: neighbours nearest first, previous before next, because a section header dies with its last item. DetailFocusMemoryTests pins the fallback.
• Equivalence against the adapter path on every Lexicon row binding an owned id, and a no-mediator spy proving the hidden tree is never built.
• The two posEdit rows have no automated twin; they were tested manually.

Deliberately not here. E3 Move Up/Down and E6 Merge: mnuDataTree-AlternateForm is bound to no WinForms slice and mnuDataTree-CmMedia is unreachable, so both engines move to the PR that owns mnuDataTree-Allomorph. The empty-owned-menu fallback and the record-level delete path, noted in the preflight details.

Verification. Build and both hygiene gates clean. 8 FdoUi, 26 FwAvalonia and 80 xWorks targeted tests green. Manual pass in Lexicon Edit and Category Edit against the WinForms menus, 13 steps, all passed; it found the focus defect this PR then fixed. Not run: the full suite.

Next: approve, or tell me what to change.

---

Reading this a year from now -- start here

This PR is stage 2, PR A of the plan that retires the hidden WinForms DataTree the Avalonia detail view keeps alive to answer context-menu commands. PR 1 (#1177) built owned menus from the menu XML and added an empty LexiconMenuAuthority that owns an id by predicate: when every leaf's message has an answerer. This PR adds the first answerer, Delete (engine E4 of the plan). Later PRs add Insert, Jump, Swap, Move, Merge and the lexical-relation commands, and the last one owns mnuDataTree-Sense. The plan of record lives in gitignored working notes under Docs/migration/working/, so the reasoning is recorded here rather than in the tree.

Decisions, and why

The rule lives in Src/FdoUi/DetailRules, and WinForms calls it too. Slice.GetCanDeleteNow and the label code in DTMenuHandler.OnDisplayDataTreeDelete were replaced by calls to DetailObjectDeletion, so there is one copy of the rule and it keeps working when WinForms is removed. The folder's boundary test forbids any UI dependency.

Confirmation is a delegate on CmObjectUi, not a host-side switch. DeleteUnderlyingObject(Func<bool> confirm) keeps the dialog, the undo task and the related-file prompt where they were; the parameterless method passes ConfirmDelete. The host owns a DeleteConfirmation seam the tests set to answer for the user. The repo already does this on FwWritingSystemSetupModel.ConfirmDeleteWritingSystem, and DummyCmObjectUi already reaches past the dialog in FdoUiTests.

Focus after a delete is a candidate list, not a row. The rows before the deleted object's rows, nearest first, then the rows after; the rebuilt tree focuses the first that exists and can take focus, falling back to the row's field-menu button when its editor is hidden. A single row failed in the manual pass: deleting the last translation removes the "Translations" section header too.

The Delete answerer ignores the command's <parameters>. The WinForms handler reads only the current slice's object, so the row's object is the target. The atomic-wrapping and variant-back-ref target rules are needed by no id in this PR and will be shared when the PRs that need them arrive.

Ownership stays message-level across the whole configuration. Narrowing it to the Lexicon area would reopen the predicate PR 1 settled. The pin lists all 17 ids by area so the reach is visible.

Surprising findings

• mnuDataTree-CmMedia is unreachable in both UIs. The LexPronunciation detail layout's <part ref="MediaFiles" param="FileAndLabel"/> names LexPronunciation-Detail-MediaFiles, which exists in no parts file; DataTree.ProcessPartRefNode and XmlLayoutImporter both omit a missing part. Media rows never render under a pronunciation.
• mnuDataTree-AlternateForm is bound to no WinForms slice. The id sits only on the <seq field="AlternateForms"> node; every allomorph row takes its caller menu mnuDataTree-Allomorph or -AffixProcess from the MoForm layouts. In Avalonia it reaches only the empty-sequence prompt row, which WinForms does not have.
• mnuDataTree-Translation lands on two Avalonia rows, the item header "Translations N" and the multistring row, both targeting the CmTranslation; WinForms has only the latter.
• The "(cannot delete this)" and "(cannot merge this)" strings are in no strings table, so WinForms renders a disabled Delete as Delete X*(cannot delete this)*. The shared rule reproduces that through the same StringTable call; no reachable row in this PR hits the disabled case.
• Only one LCM rule makes a Lexicon object undeletable here: an entry without entry refs requires its Senses, so the only sense stays. No LCM class requires an owning atomic field.

Paths not taken

• A host-side delete delegate instead of the CmObjectUi overload. Rejected: it would skip the CmObjectUi construction the product uses, proving less.
• Narrowing the predicate to Lexicon ids. Rejected: PR 1 settled message-level ownership; the pin makes the reach reviewable instead.
• Keeping E3 and E6 in this PR. Rejected once -AlternateForm proved unreachable: an engine ships with its first consumer, which is mnuDataTree-Allomorph.
• Testing the dialog itself. Not done: ConfirmDeleteObjectDlg is unchanged WinForms code; the seam tests everything around it.

Deferred, and what would unblock it

• Empty owned menu falls back to the adapter menu. Pre-existing in OnDetailMenuRequested; reachable only by an owned id whose every leaf hides on a row with no other menu group. Fix when such an id exists.
• Record-level delete path. DeleteDetailObject returns false for both a cancel and the DeleteRecord delegation; becomes relevant when a record-level row is owned.
• CanDeleteNow's required-atomic branch has no real-data test; needs a stub ICmObject.
• Shared Hosting test helpers. Three fixtures duplicate the spy, Describe and setup code; the shared home is XWorksAppTestBase.

Preflight review details

Code Review Summary

Branch: LT-22691n
Base: main (merge base a90754c, 1 commit ahead)
Date: 2026-10-13
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 14

Overview

Stage 2, PR A of the hidden-DataTree menu-id retirement (LT-22691): the first answerer
of LexiconMenuAuthority. Every DataTreeDelete leaf is answered from the Avalonia row:
hidden on a ghost or object-less row, disabled with the WinForms wording when the object
is gone or is the last item of a required field, otherwise deleted through the object's
CmObjectUi with its confirmation dialog in one undo step. The re-show then focuses the
nearest surviving neighbour row. mnuDataTree-Translation and mnuDataTree-ComplexFormSpec
no longer build the hidden tree. The enablement and wording rule moved to
Src/FdoUi/DetailRules/DetailObjectDeletion.cs, which the WinForms slice and menu
handler now call.

The analysis found no Critical or Important issue. The inventory behind the branch
found two of the four planned ids unreachable (-CmMedia: its pronunciation detail part
does not exist; -AlternateForm: bound to no WinForms slice), so E3 and E6's merge half
moved out of this PR. Message-level ownership makes the authority own 17 ids across the
configuration, two of them live posEdit rows; the author accepted that reach and tested
it manually.

Contract/API Changes

• CmObjectUi.DeleteUnderlyingObject(Func<bool> confirm) and CmObjectUi.ConfirmDelete()
  added (public). The parameterless DeleteUnderlyingObject() delegates to them and
  behaves as before.
• New public SIL.FieldWorks.Common.DetailRules.DetailObjectDeletion (FdoUi).
• FwAvalonia: DetailField.RowAutomationId; DataTree.RequestRowFocusOnRebuild(IReadOnlyList<string>);
  DetailHostControl.FocusRowOnNextShow(IReadOnlyList<string>) and ShownDetail;
  DetailFocusMemory.Memento gains an optional trailing constructor parameter and
  RowAutomationIds.
• xWorks: RecordEditView.CreateLexiconMenuAuthority() becomes (DetailField) (internal).
• No native, COM, serialized-format, script or installer change.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• FocusRow focused a target without checking it was effectively visible, so a
  collapsed row dropped focus silently (fixed during review: falls back to the
  field-menu button and returns whether focus was taken)
• The single focus row named after a delete could vanish with the deleted object
  (the "Translations" header goes when its last item does), found in the manual pass
  (fixed during review: an ordered candidate list, previous rows nearest first then
  following rows; the rebuilt tree focuses the first that exists)
• An owned menu whose every leaf hides builds zero items and the host falls back
  to the adapter menu (author: pre-existing bridge behaviour; no id in this PR can
  reach it)
• DeleteDetailObject cannot tell a cancel from the DeleteRecord delegation
  (author: only matters once a record-level row is owned, in a later PR)
• CanDeleteNow's required-atomic branch has no test (author: no LCM class
  requires an owning atomic field, so the branch is reachable only through a stub)
• No end-to-end assertion that the focus request was made (author: the static
  row choice and the FwAvalonia seam are each tested; accepted)
• Three xWorks fixtures duplicate the spy, Describe and setup helpers (author:
  known debt, tracked at low priority)
• Focus candidates are computed from the shown model, which could lag under a held
  refresh (author: edge case; accepted)
• Production comments compared the code to WinForms ("worded as WinForms words
  it", "as Slice.HandleDeleteCommand does") (fixed during review: removed; the
  rule is now recorded for future work)

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene: clean.
• .\test.ps1 -TestProject Src/FdoUi/FdoUiTests -TestFilter "FullyQualifiedName~DetailObjectDeletionTests|FullyQualifiedName~DetailRulesBoundaryTests": 8 passed.
• .\test.ps1 -TestProject Src/Common/FwAvalonia/FwAvaloniaTests -TestFilter "FullyQualifiedName~DetailFocusMemoryTests|FullyQualifiedName~DetailMenuTests|FullyQualifiedName~DataTreeTabNavigationIntegrationTests|FullyQualifiedName~DetailEditorParityTests": 26 passed.
• .\test.ps1 -TestProject Src/xWorks/xWorksTests -TestFilter "FullyQualifiedName~LexiconMenuAuthorityTests|FullyQualifiedName~LexiconMenuExecutionTests|FullyQualifiedName~DetailObjectCommandExecutionTests|FullyQualifiedName~DetailMenuRequestTests": 80 passed.
• Manual pass by the author (2026-10-13), 13 steps in Lexicon Edit and Category Edit
  against the WinForms menus: all passed after the focus-rule fix.
• Not run: the full suite; CI pending at PR creation.
• Jira: the branch carries LT-22691; no new ticket needed.

Positive Observations

• The shared rule is the only copy of the delete enablement; both UI stacks call it,
  and the DetailRules boundary test keeps it free of UI dependencies.
• Every owned id is pinned, so the diff of each later PR shows exactly which menus it
  switches over.
• The confirmation seam keeps the WinForms dialog in production while letting the
  delete run headlessly end to end for the first time.

Interview Notes

• Author reviewed the full diff and ran the manual pass; the one defect it found (the
  vanishing header) was fixed and re-verified.
• Author accepted the posEdit reach of message-level ownership rather than narrowing
  the predicate, which PR 1 settled.
• Author chose the CmObjectUi overload over a host-side delete delegate, after the
  repo's existing seams (FwWritingSystemSetupModel.ConfirmDeleteWritingSystem,
  DummyCmObjectUi, IEnvironmentMenuHost) were shown as precedent.
• Author asked that production comments stop comparing to WinForms or any old code.
• The PR targets main: PR 1 merged while this branch was in progress, so the branch
  was rebased from LT-22691m onto main (no conflicts) and the stacked-PR plan dropped.

In-Review Quality Check

Gated build and the three targeted test runs above were rerun after the last edit.

Suggested Review Focus

• DetailObjectDeletion.CanDeleteNow against Slice.GetCanDeleteNow at the merge base
• RowsToFocusAfterDeleting ordering and DataTree.FocusFirstRow
• The 17-id pin in LexiconMenuAuthorityTests: is the posEdit reach wanted now

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ± 0      1 suites  ±0   14m 0s ⏱️ -7s
6 635 tests +16  6 550 ✅ +16  85 💤 ±0  0 ❌ ±0 
6 644 runs  +16  6 559 ✅ +16  85 💤 ±0  0 ❌ ±0 

Results for commit 1bf1daf. ± Comparison against base commit 2c2a4c3.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 73.09942% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.59%. Comparing base (2c2a4c3) to head (1bf1daf).

Files with missing lines	Patch %	Lines
Src/FdoUi/FdoUiCore.cs	16.66%	13 Missing and 2 partials ⚠️
Src/Common/FwAvalonia/Detail/DataTree.cs	62.16%	6 Missing and 8 partials ⚠️
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs	85.33%	6 Missing and 5 partials ⚠️
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs	84.61%	1 Missing and 1 partial ⚠️
Src/Common/FwAvalonia/DetailHostControl.cs	0.00%	0 Missing and 2 partials ⚠️
Src/FdoUi/DetailRules/DetailObjectDeletion.cs	90.47%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1182      +/-   ##
==========================================
+ Coverage   39.52%   39.59%   +0.06%     
==========================================
  Files        1538     1540       +2     
  Lines      354569   354689     +120     
  Branches    41090    41123      +33     
==========================================
+ Hits       140159   140454     +295     
+ Misses     185034   184864     -170     
+ Partials    29376    29371       -5     

Files with missing lines	Coverage Δ
Src/Common/Controls/DetailControls/Slice.cs	57.97% <100.00%> (+0.19%)	⬆️
Src/Common/FwAvalonia/Detail/DetailModel.cs	79.02% <100.00%> (+0.02%)	⬆️
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs	61.70% <100.00%> (-0.21%)	⬇️
Src/xWorks/DTMenuHandler.cs	23.57% <100.00%> (+0.18%)	⬆️
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs	71.70% <84.61%> (+0.49%)	⬆️
Src/Common/FwAvalonia/DetailHostControl.cs	67.74% <0.00%> (-4.68%)	⬇️
Src/FdoUi/DetailRules/DetailObjectDeletion.cs	90.47% <90.47%> (ø)
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs	85.33% <85.33%> (ø)
Src/Common/FwAvalonia/Detail/DataTree.cs	93.48% <62.16%> (-1.66%)	⬇️
Src/FdoUi/FdoUiCore.cs	28.23% <16.66%> (+1.32%)	⬆️

... and 11 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.

[thejambi]

@thejambi reviewed 14 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on mark-sil).

---

Src/Common/FwAvalonia/Detail/DataTree.cs line 565 at r1 (raw file):

				editor.Dispose();
			_vectors.Clear();
			_labelBlocks.Clear();

RebuildItems resets the other per-build collections here but not _rowFocusTargets. After a header toggle, the map keeps entries for rows that are no longer built, and those entries point at disposed editors. Today's restore can't reach that because it runs on a fresh tree, but clearing it with its siblings keeps FocusRow safe if something calls it later.

			_labelBlocks.Clear();
			_rowFocusTargets.Clear();

---

Src/FdoUi/FdoUiCore.cs line 1071 at r1 (raw file):

		public bool ConfirmDelete()
		{
			var mainWindow = m_propertyTable.GetValue<Form>("window");

The other public members begin with CheckDisposed(). Should this one?

---

Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs line 78 at r1 (raw file):

				return false;
			}
			var focusRows = RowsToFocusAfterDeleting(m_avaloniaEntryForm?.ShownDetail, target);

Codecov shows DetailHostControl.ShownDetail and FocusRowOnNextShow only ever taking the null branch. So in LexiconMenuExecutionTests, focusRows is always empty and lines 78–89 never run under test. The manual pass caught the vanishing-header bug on this exact path. Would a test asserting that the focus request reaches the host be cheap? I know you accepted this in preflight, so fine to leave it if it isn't.

[thejambi]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on mark-sil).

[mark-sil]

Thanks for the review, @thejambi. Replies to the three discussions:

Src/Common/FwAvalonia/Detail/DataTree.cs line 565 (RebuildItems)
Agreed, done in 09fc9c6: _rowFocusTargets is now cleared with the other per-build collections.

Src/FdoUi/FdoUiCore.cs line 1071 (ConfirmDelete)
Yes, thanks. Added CheckDisposed() in 09fc9c6.

Src/xWorks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs line 78 (focus request coverage)
The coverage report is showing partials there, not misses. Lines 79–88 are hit, and lines 78 and 89 plus the two DetailHostControl members are partial only on the ?. null branch, because the fixture's host always shows a tree. So the focus request does run on that path; it just isn't asserted.

I tried adding the assertion and it isn't cheap. The hand-off relies on the re-show being posted with BeginInvoke, which the product always does while the view is shown. The headless fixture has no window handle, so the re-show runs synchronously inside the delete. Asserting it there would mean either changing the production ordering to suit the harness, which opens a focus jump when a pending-edit re-show lands while the confirm dialog is open and the user then cancels, or asserting on a tree nobody reads. So I'm leaving it as accepted in preflight. The manual pass covers it, including the vanishing-header case.

[thejambi]

@thejambi reviewed 2 files and all commit messages, and resolved 3 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mark-sil).

[thejambi]

@thejambi reviewed 4 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mark-sil).

[thejambi]

@thejambi made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mark-sil).