[mark-sil]

Start here: Src/FdoUi/DetailRules/DetailObjectInsertion.cs. It is the insert rule both detail views now share, and every other change in this PR calls it.

In the Avalonia detail view, Insert Subsense, Insert Sense, Insert Example, Insert Extended Note and Insert Etymology now run from the row itself. The Lexicon menu authority answers every DataTreeInsert leaf that creates a concrete class in an owning field, so 20 more menu ids stop building the hidden DataTree. Five of them are Lexicon menus (-Subsenses, -ExtendedNotes, -Sense-Hotlinks, -ExtendedNote-Hotlinks, -Etymology-Hotlinks) and five are posEdit section menus (Affix Templates, Affix Slots, Inflection Classes, Subclasses, Stem Allomorph Labels). After an insert, the keyboard lands in the new object's first editor.

The question a reviewer starts with is whether the WinForms insert path still puts new objects where it did. Slice.InsertObject and DTMenuHandler.CanInsertFieldIntoObj now call the rule's pieces (field resolution, owner hop, concrete class, position after the current slice) instead of their own copies; a manual WinForms insert pass confirmed the same placement. The review worth your time is the rule itself and the focus change.

Where to look

• DetailObjectInsertion.ResolveTarget: the owner climb, slice="owner", and the position after the clicked object. Pinned by DetailObjectInsertionTests.
• The Insert answerer's scope: the allomorph, SubPossibilities and reference-field inserts keep their ids with the adapter in full. Pinned by InsertLeaves_OutsideTheScope_LeaveTheirIdsWithTheAdapter.
• DetailFocusMemory.RestoreAfterLayout: a requested object now wins over a surviving editor. This also moves focus after Convert to Affix Process. Pinned by three DetailFocusMemoryTests.
• One divergence: Insert Extended Note under an existing note is enabled (WinForms shows it disabled). Pinned in the equivalence test.

Not here: the allomorph inserts, the Insert Variant dialog and pictures/media (stage 2 PR C2); SubPossibilities and DataTreeCopy (stage 4).

Verification: off main. build.ps1 -CommentHygiene -TokenHygiene is clean; 148 targeted tests pass. Fourteen manual tests on Sena 3 passed, covering Lexicon, posEdit, WinForms and Convert. The full suite was not run.

Next: approve.

---

Reading this a year from now -- start here

This PR is stage 2 PR C1 of LT-22691, which retires the hidden WinForms DataTree from the Avalonia detail view one context-menu id at a time. The plan of record and the per-id cycle live in the hidden-datatree-menu-ids skill. The decisions below are the ones the code cannot explain on its own.

Decisions, and why

• A scope on the answerer, not special cases in the rule. LexiconMenuAuthority.Answer takes an optional leaf predicate. A leaf outside it leaves its whole id with the adapter, because an id is answered natively in full or not at all. The scope admits a concrete class created in an owning field; everything with a rule of its own (allomorph visibility, SubPossibilities, the Insert Variant dialog) stays out until the PR that answers it.
• Enablement is "a target resolves", not the WinForms root-or-row check. The native item is enabled when ResolveTarget finds an owning field from the row's object up its owners. That is why Insert Extended Note under a note is enabled: the sense owning the note takes it. WinForms asks only the shown entry and the row's own object, so that hotlink is dead there.
• recomputeVirtual is not read. The WinForms handler splits the attribute and does nothing with it; the Avalonia re-show recomposes every row anyway.
• Focus: an object request beats a surviving editor; a row request does not. Inserts ask the re-show to focus an object; deletes ask it to focus rows. A delete leaves you where you were unless your row vanished; an insert always moves you to the new object, as WinForms does. A row request also wins over a remembered menu button, which is where focus sits after using the Field options link.
• The new object's first editor, not its header. DataTree.FocusFirstRowOf tries the object's editor rows first, matching WinForms, which focuses the first focusable slice of the new object.

Surprising findings

• mnuDataTree-Subsenses reaches only the Subsenses prompt row of a sense without subsenses: nested sense sequences suppress their banner, and their items take mnuDataTree-Sense.
• mnuDataTree-ExtendedNotes is visibility="ifdata", so without Show Hidden Fields there is no prompt row for the first note.
• The owned-id predicate reaches beyond Lexicon: five posEdit section menus each carry a single Insert leaf, so they switch over in this PR. Each was tested manually.
• A WinForms window showing the same entry takes focus after an insert in another window. It is pre-existing (DataTree.RefreshListAndFocus) and not addressed here.

Deferred, and what would unblock it

• Allomorph inserts (-AlternateForms, -AlternateForms-Hotlinks): need the morph-type and first-allomorph visibility rules and the ActiveClerk.ListSize check. Stage 2 PR C2.
• Insert Variant (-VariantForms, -VariantForms-Hotlinks): needs the Insert Variant dialog path. Stage 2 PR C2.
• SubPossibilities inserts: the WinForms rule asks whether the whole tree has a SubPossibilities slice. Stage 4, with posEdit.
• DataTreeCopy: Grammar only (Duplicate Affix Slot/Template). Stage 4.

Evidence

Targeted run on the tree merged with main, 148 of 148 passed:

.\test.ps1 -CommentHygiene -TokenHygiene -TestFilter "FullyQualifiedName~DetailObjectInsertionTests|FullyQualifiedName~DetailRulesBoundaryTests|FullyQualifiedName~DetailJumpTargetsTests|FullyQualifiedName~LexemeFormConversionTests|FullyQualifiedName~LexiconMenuAuthorityTests|FullyQualifiedName~LexiconMenuExecutionTests|FullyQualifiedName~DetailFocusMemoryTests|FullyQualifiedName~DetailObjectCommandExecutionTests"

Manual tests on Sena 3, all passed:

1. Insert Sense and Insert Example from a sense's Field options link (the new sense lands after the clicked one; caret in its Gloss).
2. Insert Subsense from the Subsenses prompt row.
3. Insert Extended Note from the prompt row (Show Hidden Fields), a note's Field options link, and the Extended Notes header.
4. Insert Etymology from an etymology's Field options link (lands after the clicked one).
5. Insert from each posEdit section header: Inflection Classes, Affix Templates, Affix Slots, Subclasses, Stem Allomorph Labels, with focus starting in another editor.
6. WinForms: Insert Sense, Subsense and Etymology land as before.
7. Convert to Affix Process on a prefix, with focus starting in Gloss: the caret lands in the new form.

Preflight review details

Code Review Summary

Branch: claude/lt-22691p-cbd94e
Base: main
Date: 2026-10-15
Review model: Claude Opus 5.5
Files changed: 11

Overview

Stage 2 PR C1 of LT-22691: the E5 insert engine, core half. The Lexicon menu authority answers
every DataTreeInsert leaf that creates a concrete class in an owning field, so
mnuDataTree-Subsenses, mnuDataTree-ExtendedNotes, mnuDataTree-Sense-Hotlinks,
mnuDataTree-ExtendedNote-Hotlinks and mnuDataTree-Etymology-Hotlinks no longer build the
hidden DataTree. The same predicate completes five posEdit section menus
(mnuDataTree-POS-AffixTemplates, -POS-AffixSlots, -POS-InflectionClasses,
-POS-InflectionClass-Subclasses, -POS-StemNames). The shared rule lives in
Src/FdoUi/DetailRules/DetailObjectInsertion.cs, and the WinForms Slice and DTMenuHandler
insert path calls it.

The analysis found no contract, native or localization issues. Two validation gaps (the posEdit
menus, and the focus change's effect on PR B's Convert) were closed by manual tests during the
interview. Five minor items from the agent code review were kept as is by the author.

Contract/API Changes

• New public static class SIL.FieldWorks.Common.DetailRules.DetailObjectInsertion and public
  sealed DetailInsertTarget in FdoUi (additions).
• DTMenuHandler.CanInsertFieldIntoObj (protected) keeps its signature; its body calls the rule.
  The private GetFlidIfPossible helper is removed.
• LexiconMenuAuthority.Answer (internal) gains an optional leaf scope.
• DetailFocusMemory.RestoreAfterLayout (public): a requested object now wins over a surviving
  remembered editor, and a requested row wins over a remembered menu button. A requested row
  still yields to a surviving editor.
• DataTree.FocusFirstRowOf (internal): the object's editor rows first, its header only when no
  editor can take focus.
• Owned menu ids: this PR adds 20 to the authority's owned list; the pin in
  LexiconMenuAuthorityTests lists every owned id by area.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

• Five posEdit section menus switched to the native path with partial evidence (fixed during review: the author manually tested Insert from each of Affix Templates, Affix Slots, Inflection Classes, Subclasses and Stem Allomorph Labels in Category Edit; all passed)
• The focus change alters PR B's Convert to Affix Process (focus to the new form's first editor, winning over an editor focused elsewhere) (fixed during review: the author manually tested Convert on the prefix "da" after clicking into Gloss; focus landed in the new form's Lexeme Form editor, as intended)

Minor - Consider

• The owner hop in Slice.InsertObject accepts a subclass of the field's class where the old code required the exact class (kept as is: no current layout reaches the difference)
• With displayNumber, the Insert label shows the vector end plus one while the insert lands after the clicked item (kept as is: matches the WinForms label; no owned command uses it)
• The insert scope keys on field names across all classes, not on the command's target class (kept as is: no current DataTreeInsert command collides)
• No headless test asserts the focus request an insert makes from the host (kept as is)
• IsOwningFieldName scans the metadata field list once per insert leaf per menu build (kept as is)

Ticket: user-visible, carries LT-22691; no new ticket needed.

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene: clean, on the merged tree (C1 plus main with PR B).
• .\test.ps1 -CommentHygiene -TokenHygiene -TestFilter over DetailObjectInsertionTests,
  DetailRulesBoundaryTests, DetailJumpTargetsTests, LexemeFormConversionTests,
  LexiconMenuAuthorityTests, LexiconMenuExecutionTests, DetailFocusMemoryTests and
  DetailObjectCommandExecutionTests: Total tests 148, passed 148, on the merged tree.
• Manual, by the author on Sena 3: nine Lexicon and posEdit insert tests before the merge (all
  passed after two focus fixes), and the four posEdit section menus plus PR B's Convert after
  the merge (all passed).
• Not run: the full test suite (the author is asked before a full suite).

Positive Observations

• The insert rule is one control-free class shared by both detail views, guarded by the existing
  DetailRules boundary test, with its own test file.
• The leaf scope keeps the allomorph, SubPossibilities and reference-field inserts with the
  adapter in full instead of answering them partially.
• The one divergence from WinForms (Insert Extended Note under an existing note is enabled) is
  pinned in the equivalence test rather than hidden.

Interview Notes

• Q1 (posEdit menus): the author ran the four untested section menus manually; all passed.
• Q2 (Convert focus): the author confirmed the new focus behaviour is intended and tested it.
• Q3 (most complex part), the author's answer verbatim: "Focus needed to move to the first edit
  field in a new item when an item is inserted. We wanted this consistent in all cases. After an
  insert, the new object takes focus even from an editor that is still on screen. After a delete,
  a surviving editor keeps focus."
  Note: the answer states the two rules but not what selects between them (an insert asks the
  re-show to focus an object; a delete asks it to focus rows).
• Minors: all five kept as is by the author.
• A WinForms window showing the same entry takes focus after an insert in another window. It is
  pre-existing (DataTree.RefreshListAndFocus); the author chose not to track it.

Suggested Review Focus

• DetailObjectInsertion.ResolveTarget: owner climb, slice="owner", and the position after the clicked object.
• DetailFocusMemory.RestoreAfterLayout: the new precedence between object requests, row requests and a surviving editor.
• The pinned owned-id list: five posEdit section menus go native with this PR.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ± 0      1 suites  ±0   14m 17s ⏱️ -18s
6 724 tests +36  6 639 ✅ +36  85 💤 ±0  0 ❌ ±0 
6 733 runs  +36  6 648 ✅ +36  85 💤 ±0  0 ❌ ±0 

Results for commit ba00aae. ± Comparison against base commit b1d2482.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.43939% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.74%. Comparing base (b8926da) to head (ba00aae).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/FdoUi/DetailRules/DetailObjectInsertion.cs	87.68%	7 Missing and 10 partials ⚠️
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs	77.61%	8 Missing and 7 partials ⚠️
Src/Common/Controls/DetailControls/Slice.cs	57.14%	5 Missing and 4 partials ⚠️
Src/Common/FwAvalonia/Detail/DataTree.cs	57.14%	2 Missing and 1 partial ⚠️
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs	82.35%	0 Missing and 3 partials ⚠️
...rc/xWorks/Avalonia/Hosting/LexiconMenuAuthority.cs	91.66%	0 Missing and 1 partial ⚠️
Src/xWorks/DTMenuHandler.cs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
+ Coverage   39.70%   39.74%   +0.04%     
==========================================
  Files        1543     1544       +1     
  Lines      355006   355085      +79     
  Branches    41205    41247      +42     
==========================================
+ Hits       140958   141140     +182     
+ Misses     184684   184585      -99     
+ Partials    29364    29360       -4     

Files with missing lines	Coverage Δ
...rc/xWorks/Avalonia/Hosting/LexiconMenuAuthority.cs	86.00% <91.66%> (+1.90%)	⬆️
Src/xWorks/DTMenuHandler.cs	22.71% <50.00%> (-0.86%)	⬇️
Src/Common/FwAvalonia/Detail/DataTree.cs	93.06% <57.14%> (-0.45%)	⬇️
Src/Common/FwAvalonia/Detail/DetailFocusMemory.cs	72.97% <82.35%> (+1.06%)	⬆️
Src/Common/Controls/DetailControls/Slice.cs	57.44% <57.14%> (-0.54%)	⬇️
...ks/Avalonia/Hosting/RecordEditView.LexiconMenus.cs	79.43% <77.61%> (-0.68%)	⬇️
Src/FdoUi/DetailRules/DetailObjectInsertion.cs	87.68% <87.68%> (ø)

... and 9 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]

Should fix: ResolveTarget doesn't check that an owning atomic field is empty (DetailObjectInsertion.cs, the candidate loop in ResolveTarget)

• What's wrong: CanInsertInto returns false when an atomic field already holds an object, and that is how WinForms disabled such inserts. ResolveTarget skips that check: for an atomic field it returns AtomicPosition, and Avalonia uses that result alone to decide whether the item is enabled.

• Scenario: on an entry that already has a Lexeme Form, call ResolveTarget(entry, "LexemeForm", "MoStemAllomorph", null, false). It returns a target, so the native item would be enabled, and running it would replace the existing form instead of being disabled.

• Why it's latent: no row can reach this today. The only atomic DataTreeInsert fields are LexemeForm and Linker, and both happen to be excluded by the allomorph command-id suffix check. PR C2 will open the gap when it answers "Create Stem Form".

• Fix: before building the target, add:

  if (mdc.GetFieldType(flid) == (int)CellarPropertyType.OwningAtomic
      && cache.DomainDataByFlid.get_ObjectProp(owner.Hvo, flid) != 0)
      return null;

  Also add a case to ResolveTarget_IsNull_....

[mark-sil]

@thejambi Fixed in ba00aae: ResolveTarget now returns no target for an owning atomic field that already holds an object, so the native item is disabled as in WinForms. DetailObjectInsertionTests covers both sides: the filled Lexeme Form case in ResolveTarget_IsNull_..., and the empty one in ResolveTarget_AnEmptyAtomicField_TakesTheNewObject.

[thejambi]

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