LT-22691: Build owned detail menus from the menu XML, not ChoiceGroup

[mark-sil]

Start here: ResolveMenu and ConvertOwned in Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs, the walk that replaces ChoiceGroup on owned menus. Then LexiconMenuAuthority.Owns, the predicate the next PRs feed.

What it does. Stage 2, PR 1 of the hidden-DataTree retirement: the groundwork the Lexicon menu PRs build on. An owned menu id is now built from its menu XML through the XWindow's loaded configuration and command set; no ChoiceGroup, no ChoiceBase and no mediator call on an owned path. IDetailMenuAuthority.Build takes a DetailMenuLeaf (the resolved Command, its message and label); the five shipped authorities migrated. LexiconMenuAuthority arrives with no answerers and owns by predicate: every leaf's message answered, no list submenu, nothing unanswerable. The owned-population hook #1153 added to ChoiceGroup is removed. No user-visible change is intended.

The question to settle. Does the walk see what ChoiceGroup.Populate saw? A test compares the two for every context-menu id in the window configuration: top-level leaves equal by id and label, nested leaves a subset (the mediator path drops a submenu no colleague shows), list submenus equal. It found two things xCore tolerates until display that the walk now tolerates too: property-toggle items (PaneBar-*, mnuBrowseHeader) and an item naming a command that is commented out (mnuDataTree-FeatureStructure-Feature, Grammar). Both become Unanswerable entries, so no authority can own such a menu and it stays on the mediator path.

Where to look.

• ResolveItem: the precedence of ChoiceBase.Make, item label over command label, localized the same way.
• ConvertOwned: the LT-8791 empty-submenu drop, inline splice, list via BuildList and separator trimming, pinned by the existing owned-menu tests.
• LexiconMenuAuthority.Owns: the pinned owned-id list is empty; LexiconMenuAuthorityTests flips the predicate with fake answerers.
• ChoiceGroup: a pure revert of the LT-22691: Let a menu authority answer a whole menu id, submenus included #1153 threading.
• Mediator.CommandSet is read as a registry; the no-mediator spy tests still pass.

Deliberately not here. Any answerer (PRs A, B and C1 follow). A native property-toggle answerer. Caching resolved menus per window. A shared LoadRecordEditView test helper.

Verification. Build and both hygiene gates clean. 76 Hosting fixture tests and 19 xCoreInterfaces tests green. Manual parity pass against the WinForms menus in Lexicon Edit: owned label menus, chip menus, writing-system toggles, and the Lexeme Form and sense-header menus still on the mediator path. Not run: the full suite; a localized UI, since only strings-en.xml ships.

Next: approve, or say if the ChoiceGroup revert should be its own PR.

---

Reading this a year from now -- start here

This PR is stage 2, PR 1 of the plan that retires the hidden WinForms DataTree the Avalonia detail view keeps alive to answer context-menu commands. Stage 1 answered six ids with five authorities that own by hard-coded id: mnuReorderVector (#1143), mnuDataTree-Object and -Help (#1161), mnuReferenceChoices (#1172), mnuDataTree-MultiStringSlice (#1169), mnuEnvReferenceChoices and the two environment insert menus (#1175). Stage 2 answers the ~30 Lexicon menus through one authority parameterized by each command's message and XML parameters; PRs A, B and C1 add its first answerers, C2, D1 and D2 follow, and E finishes with 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

Command lookup stays on Mediator.CommandSet. It is a hashtable built once at window load, read exactly as CommandChoice.CommandObject reads it; no message is sent. The CmObjectUi jump methods take a Command, so the resolved command has to reach ReferenceItemMenuAuthority regardless.

Build takes a DetailMenuLeaf carrying the Command. The cluster keys answers on the command's message and <parameters>, and the reference-item authority needs the Command object itself; a command id and label alone would not have served either.

Owns(string) and the static OwnsAll are unchanged. The bridge exposes the resolved tree and the cluster authority takes a resolver delegate at construction. Changing Owns to take the menu node would have touched every authority and forced the window into OwnsAll.

The predicate is message-level. An answerer that cannot act on the current row returns a disabled item, which is the disable-and-log rule already in force. Row-conditional ownership stays where it already lives, on the multi-string authority.

The walk tolerates what ChoiceGroup tolerates. A property item, an undefined command or an unknown element becomes an Unanswerable entry rather than an exception, because Owns now runs the walk for every unowned id a row binds, and a throw there would remove the whole row menu where today the menu still shows. Only a truly malformed <item> throws, as ChoiceBase.Make does.

Test files follow the code, not the PR schedule. LexiconMenuAuthorityTests.cs holds the pinned owned-id list and the cluster's contracts; rule classes keep one test file each under FdoUiTests/DetailRules/; a LexiconMenuExecutionTests.cs arrives with the first end-to-end mutation check. The five migrations stayed in DetailObjectCommandExecutionTests.cs.

Paths not taken

• Owns(menuId, menuNode). Rejected: every authority and OwnsAll would change for the benefit of one.
• Throwing on a property item or undefined command. Rejected after the equivalence test found both in shipped configuration: the walk must not be stricter than ChoiceGroup for menus nobody owns.
• Catching ConfigurationException inside Owns. Rejected: exceptions as flow control, and it would have hidden a genuinely unknown menu id.
• One test fixture per track. Rejected once the tracks were agreed to run sequentially; the files follow the code instead.
• Restoring CmdDataTree-Delete-FeatureStructure-Feature. Not done: commented out since before the truncated history; the author ruled no ticket and no change.

Preflight review details

Code Review Summary

Branch: LT-22691m
Base: main (merge base dc0004c, 1 commit ahead)
Date: 2026-10-08
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 15

Overview

Stage 2, PR 1 of the hidden-DataTree menu-id retirement (LT-22691): groundwork the
Lexicon menu PRs build on. An owned context-menu id is now built from its menu XML
through the XWindow's loaded configuration and command set, with no ChoiceGroup,
no ChoiceBase and no mediator call on an owned path. IDetailMenuAuthority.Build
takes a DetailMenuLeaf; the five shipped authorities migrated. LexiconMenuAuthority
arrives with no answerers and owns by predicate. The owned-population hook #1153 added
to ChoiceGroup is removed. No user-visible behaviour change is intended; the author
confirmed parity manually against the WinForms menus.

The analysis found no Critical or Important issue. The equivalence test over every
context-menu id surfaced two configuration facts the walk now tolerates as xCore does:
property-toggle items in PaneBar-* and mnuBrowseHeader, and an item in
mnuDataTree-FeatureStructure-Feature naming a command that is commented out in
Grammar/DataTreeInclude.xml. The author ruled the latter needs no ticket or change.

Contract/API Changes

• IDetailMenuAuthority.Build(string, ChoiceBase) -> Build(string, DetailMenuLeaf).
  All implementers are in xWorks and xWorksTests; updated.
• ChoiceGroup.PopulateNow(bool) (public, added by LT-22691: Let a menu authority answer a whole menu id, submenus included #1153 on 2026-09-25) removed. No
  caller remains in the repository; the parameterless PopulateNow() is unchanged.
• New public types in xWorks: DetailMenuLeaf, DetailMenuDefinition, DetailMenuEntry;
  new public XCoreMenuBridge.ResolveMenu(XWindow, string).
• No native, COM, serialized-format, script or installer change.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• DetailMenuDefinition.Unanswerable is free text that the tests parse
  (UndefinedCommandId, the "property" prefix filter) (deferred: a structured kind
  would remove the coupling; the dependent tests fail loudly if the text changes, so
  nothing breaks silently)
• LexiconMenuAuthority re-walks the XML for every unowned id on every menu
  request (deferred: configuration is immutable per window, so a per-window cache is
  possible; the cost is one XML walk per id per right-click and is not measurable today)
• LoadRecordEditView is copied into a seventh Hosting fixture (deferred: the
  shared home is XWorksAppTestBase; out of scope for a groundwork PR)
• ResolveEntries threw for any child element other than item or menu
  (fixed during review: an unknown element becomes an Unanswerable entry so the
  menu stays on the mediator path, as ChoiceGroup keeps it)
• ResolveMenu ignored a list attribute on the root menu node (fixed during
  review: a list-populated root is one list entry; after Zach's review comment it is
  marked inline so its items splice flat, as ChoiceGroup.PopulateFromList shows them,
  and the comment no longer cites IsAListGroup, which tests behavior, not list)
• ReferenceItemMenuAuthority.ToolOf indexed Parameters[0] with no null
  check (fixed during review: a command with no parameters element yields null)

Required Validation / Evidence

Run:

• .\build.ps1 -CommentHygiene -TokenHygiene: clean.
• .\test.ps1 -TestProject Src/xWorks/xWorksTests -TestFilter over
  DetailObjectCommandExecutionTests, LexiconMenuAuthorityTests,
  DetailEditorParityTests, DetailCommandAdapterHardeningTests,
  DetailWritingSystemStateTests: 76 passed, 0 failed.
• .\test.ps1 -TestProject Src/XCore/xCoreInterfaces/xCoreInterfacesTests: 19 passed.
• Manual (author, 2026-10-08): Lexicon Edit, Avalonia detail view, compared against the
  WinForms menus: owned label menus (multi-string rows, Scientific Name, Semantic
  Domains, Complex Forms, Subentries, allomorph Environments), chip menus, writing-system
  toggles, and the Lexeme Form and sense-header controls on the mediator path.

Not run: the full test suite; a localized UI (only strings-en.xml ships), so label
localization through the walk is covered by code equivalence only.

Positive Observations

• ResolveMenu_ReadsWhatChoiceGroupPopulates_ForEveryContextMenuId compares the walk
  with ChoiceGroup.Populate for every context-menu id in the window configuration.
• The ownership pin in LexiconMenuAuthorityTests makes each later PR's diff show the
  menus it switched over.
• Four of the five authorities no longer reference xCore at all.
• The ChoiceGroup change is a pure revert of the LT-22691: Let a menu authority answer a whole menu id, submenus included #1153 threading.

Interview Notes

• The author reviewed every file in-session. Two notes applied: CreateMenuAuthority
  lists one authority per line with a matching summary; the "No answerer exists yet"
  sentence was removed from LexiconMenuAuthority.
• Decisions settled before coding (user, 2026-10-07): command lookup stays on
  Mediator.CommandSet (a registry read, not a message); Build takes a leaf carrying
  the resolved Command; Owns keeps its signature and the cluster authority takes a
  resolver delegate; the predicate is message-level; the authority is named
  LexiconMenuAuthority; test files follow the code (LexiconMenuAuthorityTests.cs
  now, LexiconMenuExecutionTests.cs when a track needs it).
• The author ruled no Jira ticket and no change for the dangling
  CmdDataTree-Delete-FeatureStructure-Feature reference: it has been that way since
  before the truncated history.
• No unresolved items.

In-Review Quality Check

The three review fixes were made before the commit; build, hygiene gates and the
targeted tests were rerun after them (76 and 19 passed).

Suggested Review Focus

• XCoreMenuBridge.ResolveItem reproduces ChoiceBase.Make precedence and label
  localization.
• XCoreMenuBridge.ConvertOwned keeps the LT-8791 empty-submenu drop, inline splice
  and separator trimming.
• LexiconMenuAuthority.Owns declines a menu with a list submenu or an unanswerable
  item.
• The ChoiceGroup diff is a revert and nothing else.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   13m 46s ⏱️ -15s
6 599 tests +5  6 514 ✅ +5  85 💤 ±0  0 ❌ ±0 
6 608 runs  +5  6 523 ✅ +5  85 💤 ±0  0 ❌ ±0 

Results for commit bb05c39. ± Comparison against base commit 489ed1b.

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 85.71429% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.51%. Comparing base (dc0004c) to head (bb05c39).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/xWorks/Avalonia/Hosting/XCoreMenuBridge.cs	83.03%	12 Missing and 7 partials ⚠️
...rc/xWorks/Avalonia/Hosting/LexiconMenuAuthority.cs	84.09%	2 Missing and 5 partials ⚠️
...rc/xWorks/Avalonia/Hosting/DetailMenuDefinition.cs	86.36%	0 Missing and 6 partials ⚠️
Src/xWorks/Avalonia/Hosting/DetailMenuLeaf.cs	71.42%	0 Missing and 2 partials ⚠️
...rks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs	94.11%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1177      +/-   ##
==========================================
+ Coverage   39.33%   39.51%   +0.18%     
==========================================
  Files        1533     1537       +4     
  Lines      353646   354685    +1039     
  Branches    40869    41092     +223     
==========================================
+ Hits       139094   140160    +1066     
+ Misses     185220   185137      -83     
- Partials    29332    29388      +56     

Files with missing lines	Coverage Δ
Src/XCore/xCoreInterfaces/ChoiceGroup.cs	46.24% <100.00%> (-0.24%)	⬇️
.../xWorks/Avalonia/Hosting/CompositeMenuAuthority.cs	78.94% <ø> (ø)
...Avalonia/Hosting/EnvironmentInsertMenuAuthority.cs	82.14% <100.00%> (+3.57%)	⬆️
...Works/Avalonia/Hosting/MultiStringMenuAuthority.cs	80.48% <100.00%> (ø)
Src/xWorks/Avalonia/Hosting/ObjectMenuAuthority.cs	67.30% <100.00%> (ø)
...xWorks/Avalonia/Hosting/RecordEditView.Avalonia.cs	61.05% <100.00%> (+0.37%)	⬆️
...rks/Avalonia/Hosting/ReorderVectorMenuAuthority.cs	72.50% <100.00%> (ø)
...rks/Avalonia/Hosting/ReferenceItemMenuAuthority.cs	74.03% <94.11%> (+2.34%)	⬆️
Src/xWorks/Avalonia/Hosting/DetailMenuLeaf.cs	71.42% <71.42%> (ø)
...rc/xWorks/Avalonia/Hosting/DetailMenuDefinition.cs	86.36% <86.36%> (ø)
... and 2 more

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

One thing to share @mark-sil . Up to you if you think it needs addressing in this pull request.

1. Root list menus would render wrongly (not reachable today). XCoreMenuBridge.cs:152 treats a list at the root of a menu as a nested submenu instead of splicing its items in. ChoiceGroup shows those items flat, so the flag there should be true. The comment beside it says it reads the root as IsAListGroup does, but IsAListGroup checks the behavior attribute, not list. No shipped context menu has a list at its root.

[thejambi]

and you can consider addressing the other comment I left.

@thejambi reviewed 15 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).

[mark-sil]

Thanks, both points are right: the root list is now marked inline so its items splice flat, and the comment cites PopulateFromList instead of IsAListGroup, which tests behavior. Fixed in bb05c39.

[thejambi]

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