[mark-sil]

Start here: Src/XCore/FlexUIAdapter/NavBarAdapter.cs -- the two click handlers and SelectInSidePane, then the three call sites it wraps.

NavBarAdapter shows an areaChoice or ToolForAreaNamed_<area> change by calling SidePane.SelectTab / SelectItem. Those raise TabClicked / ItemClicked, the same events a user click raises, and the adapter answered them by running the tab's or item's choice, which set areaChoiceParameters, areaChoice, currentContentControlParameters and currentContentControl again from inside an OnPropertyChanged handler. A private flag, set around the adapter's three selection calls, now makes the click handlers return without running the choice. SidePane and Choice are untouched; a real click still runs the choice.

What breaks? Nothing visible: every re-run wrote the value already in place (the same strings and, for tools, the same control node AreaListener sets), so the PropertyTable's change-only broadcast swallowed it. The exception was areaChoiceParameters, created from null by the first programmatic SelectTab of a session with no listener alive. The question worth your time is coverage: is every programmatic selection inside the guard, and is no user click?

Where to look

• The guard wraps OnPropertyChanged's SelectTab and SelectItem and SetToolForCurrentArea's SelectItem; FinishInit only adds tabs and items. Pinned by SidebarAdapterSelectionTests.
• First visit of a tab: SidePane auto-selects the first item inside SelectTab, now silently; the adapter's SelectItem(remembered) follows, so the highlight ends on the remembered tool (AreaChoiceChange_SelectsTheRememberedToolWithoutSettingAnyProperty).
• A selection the adapter did not make still sets the property (ItemSelectedOutsideTheAdapter_StillSetsTheProperty).
• Deliberate rule: a remembered tool that no longer exists now opens AreaListener's configured default tool, not the tab's first item. The old outcome was a queue-order accident (see accordion).
• Why now: first step of the tool-switch workflow in the Mediator-to-Pub/Sub migration. Under immediate delivery each re-run would be a real second switch.

Not here: the ToolForAreaNamed_ null reset for a deleted tool still broadcasts from the handler; SetToolForCurrentArea still assumes a current tab; ListPropertyChoice.m_fHandlingClick still lacks try/finally. All noted for the tool-switch step that follows.

Verification. .\build.ps1 -CommentHygiene -TokenHygiene: 0 warnings. xCoreTests 24/24, four new. A control run with the guard disabled fails the two "no property set" tests with 5 queued items. Manual, Sena 3: area tabs, area menus, tool by sidebar and by menu, Show Entry in Concordance / Show Entry in Lexicon, restart, and a first visit to an area whose remembered tool is not its first item -- all pass. Not run: a reversal-entry link (Sena 3 has no reversal entries).

Next: approve.

---

Reading this a year from now -- start here

The analysis behind this change lives in gitignored working documents (Docs/migration/working/propertytable-broadcast-buckets.md, task 6, and propertytable-w3-tool-switch.md), not in the tree. The parts a maintainer needs are below.

Decisions, and why

• Flag in the adapter, not a SidePane API. A SelectTab(tab, raiseEvents) overload was the alternative. The flag keeps SilSidePane untouched and still covers the first-visit auto-select, because SidePane raises that ItemClicked inside the guarded SelectTab.
• Plain set / clear in finally, no save-and-restore. The three guarded calls are leaves; SidePane never calls back into OnPropertyChanged synchronously, so nothing nests.
• Deleted remembered tool opens the configured default. SelectItem fails, SidePane auto-selects the first item, and until now that item's currentContentControlParameters set was the last writer in the Mediator queue, beating AreaListener's fallback to the area's default tool. The sidebar still ends on the default: the currentContentControlObject handler re-sets ToolForAreaNamed_ and the adapter selects it. Preserving "first item" would have needed a new synchronous set in the adapter, the opposite of the migration's direction.

Surprising findings

• ListPropertyChoice.m_fHandlingClick (static) already blocked the nested re-run during a user click, so the re-run only ever happened on the link, Pathway and startup entries.
• Four currentContentControl items used to be queued for one first visit of an area (click, AreaListener, auto-select, remembered); the switch read whichever Parameters value was set last. That "last writer wins" property is what made the re-run harmless and is what immediate delivery removes.
• The side pane's ListViews raise ItemSelectionChanged only with a handle; a hidden child of an unshown form never gets one, so the fixture forces handles by reading Control.Handle down the tree. It never simulates a mouse click: a synthetic WM_LBUTTONDOWN on a native ListView hangs the test host in its drag-detect loop.

Deferred, and what would unblock it

• m_propertyTable.SetProperty(propToolForArea, null, true) after a failed SelectItem (NavBarAdapter.cs:592): a broadcasting set from inside a handler, harmless only because AreaListener's currentContentControlObject handler later overwrites the null. Revisit when ToolForAreaNamed_ is converted.
• SetToolForCurrentArea reads m_sidepane.CurrentItem, which throws when no tab is selected yet. Reachable only if ToolForAreaNamed_<current area> is broadcast before the first areaChoice reaches the adapter.
• m_fHandlingClick has no try/finally; an exception inside a click leaves it set. Belongs with the immediate-delivery change, where handlers can throw into the click.

Evidence

• SidebarAdapterSelectionTests (xCoreTests): bare SidebarAdapter + ChoiceGroupCollection from a two-area, two-tool sidebar XML, no main window. Asserts the selected item per tab list, Mediator.JobItems == 0, and that currentContentControl / areaChoiceParameters are never created by the adapter's own selections.
• Control run with m_selectingInSidePane forced false: AreaChoiceChange_... and ToolForAreaChange_... fail with JobItems == 5; ItemSelectedOutsideTheAdapter_... passes.
• Manual routes (Sena 3): Lexicon > Browse, Grammar tab, Lexicon tab -> Browse highlighted and shown; from Grammar, menu Lexicon > Dictionary -> Lexicon tab and Dictionary highlighted; Lexical Tools > Collect Words, Lexicon Edit; menu Lexicon > Bulk Edit Entries; entry "zinji" right-click > Show Entry in Concordance -> Texts & Words / Concordance; result right-click > Show Entry in Lexicon -> back; Grammar > Natural Classes, Lexicon tab, restart, Grammar tab -> Natural Classes highlighted and shown.

Preflight review details

Code Review Summary

Branch: claude/pubsub-navbar-guard-a93af3
Base: main
Date: 2026-10-27
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 2

Overview

Step 6 of the PropertyTable broadcast conversion plan: remove NavBarAdapter's re-entrant property sets (inventory condition 4) so the tool-switch workflow can later deliver currentContentControl immediately. The analysis found the re-run never changed a value except areaChoiceParameters at session start, and one behavioural edge (deleted remembered tool) that the queue order used to decide.

Contract/API Changes

None. Private flag and private helper in SidebarAdapter.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

• Deleted remembered tool opens the configured default instead of the tab's first item (accepted as the deliberate rule: the old outcome depended on Mediator queue order, and keeping it would add a synchronous set in the adapter)
• No test for returning to a tab whose current item differs from the remembered tool (fixed during review: added ReturnToAnArea_SelectsTheToolRememberedSinceTheLastVisit)

Minor - Consider

• Save/restore of the flag guarded a nesting that cannot happen (fixed during review: plain set and clear in finally)
• SetToolForCurrentArea reads CurrentItem with no tab selected (pre-existing, unchanged line, out of scope; recorded in Deferred)
• Static m_fHandlingClick stays set if OnClick throws under test (pre-existing; the brief excludes Choice.cs; recorded in Deferred)

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene: clean, 0 warnings, 0 errors.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -TestProject xCoreTests: 24/24 passed.
• Control run with the guard disabled: 2 failed as intended, 1 passed.
• Manual checks performed by the author in Sena 3 (list in the Evidence accordion): all passed. Reversal-entry link not run.
• Developer-only in intent, but user-visible if wrong; no ticket by agreement for this migration's PRs.

Positive Observations

• The guard covers all three programmatic selection calls and nothing else.
• The fixture builds the adapter without a main window and proves its own sensitivity through the control run.

Interview Notes

• Author asked whether the deleted-tool edge makes removing the Mediator harder: answered easier (one switch under both deliveries instead of a queue-order-dependent second one); author accepted the default-tool outcome.
• Author asked whether the new tests share the flakiness fixed in Keep UIAdapterInterfaces files in patches until the next base #1208 (keyboard-focus assertion on a shared desktop): no focus, pump or timing dependence; handle creation only.

Suggested Review Focus

• Every SelectTab / SelectItem in NavBarAdapter is inside SelectInSidePane.
• The click handlers run choice.OnClick whenever the flag is clear.
• The deleted-tool rule in "Decisions, and why".

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   13m 36s ⏱️ + 4m 38s
6 866 tests +4  6 781 ✅ +4  85 💤 ±0  0 ❌ ±0 
6 875 runs  +4  6 790 ✅ +4  85 💤 ±0  0 ❌ ±0 

Results for commit 610e43e. ± Comparison against base commit 380cc90.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 40.09%. Comparing base (d0d85cb) to head (610e43e).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/XCore/FlexUIAdapter/NavBarAdapter.cs	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1209      +/-   ##
==========================================
+ Coverage   39.88%   40.09%   +0.20%     
==========================================
  Files        1542     1544       +2     
  Lines      355361   355320      -41     
  Branches    41320    41359      +39     
==========================================
+ Hits       141753   142474     +721     
+ Misses     184253   183543     -710     
+ Partials    29355    29303      -52     

Files with missing lines	Coverage Δ
Src/XCore/FlexUIAdapter/NavBarAdapter.cs	72.03% <93.33%> (+72.03%)	⬆️

... and 53 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 2 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mark-sil).