[mark-sil]

Start here: Src/xWorks/RecordClerk.cs, the IsControllingTheRecordTreeBar setter: its two broadcasting sets now come last, in their old order, and every other move follows that pattern.

Six broadcasting PropertyTable sets sat inside a unit of work or mid-sequence. They now run after the unit of work ends or at the end of their sequence: ReversalIndexGuid in ReversalListener.Init, DictionaryPublicationLayout and ReversalIndexPublicationLayout in the configuration migrator, ActiveClerk and ActiveClerkSelectedObject in the clerk setter, ShowRecordList in XmlDocView. EditingHelper sets its WritingSystemHvo suppress flag before the broadcast. One ReversalIndexGuid set, ReversalClerk's fallback to the first index, stops broadcasting; its only listener is the clerk already doing that work.

Does anything behave differently? No. Every notification still goes through the deferred Mediator queue in the same order; only the place in the call stack where the set runs has changed. That clears the exception-ownership condition for these sites: a synchronous handler that throws could no longer roll back data or truncate a sequence. The inventory's other conditions remain, so none of these sets can be delivered synchronously yet. Review question: is there a reader or a second broadcast between each old and new position?

Where to look:

• RecordClerk.cs: both sets moved together past ActiveClerkOwningObject and AddNotification, neither of which reads them.
• PreHistoricMigrator.cs: selections collected and written after each unit of work, in a finally; a pending selection shadows the table for later reads. Two new tests.
• XmlDocView.cs: the set follows ActivateUI(false) and precedes UpdateList, the first reader.
• ReversalListener.cs: the guid chosen inside the task is set after EndNonUndoableTask; one new test.
• The dropped broadcast: one ReversalClerk is a target at a time, and its handler only re-entered ChangeOwningObject.

Deliberately not here: RecordView's two ShowRecordList sets, which PopulateRecordBar reads during activation; the reversal-index deletion path, unreachable since LT-16385 and removed in a follow-up PR; and the migrator's broadcast flags, whose deferred handlers still act after a migration (accordion below).

Off main, independent of PR #1185. Build with both hygiene gates clean; xWorksTests, LexEdDllTests, RootSiteTests and SimpleRootSiteTests pass (counts in the evidence accordion). Manual checks done for the record list, the reversal index at window load, Notebook Document and the writing system combo; not done: a pre-8.3 project migration. One test-host crash in the UIA smoke test on the first run, not reproduced in three later runs.

Next: approve, or name the move whose reader check you want strengthened.

---

Reading this a year from now -- start here

This is step 2 of the plan for converting PropertyTable change notifications from the xCore Mediator queue to Pub/Sub. The inventory that classified every broadcasting set lives in a gitignored working document, so the reasoning behind each move is recorded here rather than in the tree. The branch added no markdown files.

Decisions, and why

Both clerk sets moved, not only ActiveClerk. The brief suggested moving only the ActiveClerk set to the end of the setter. That would have put it behind the ActiveClerkSelectedObject broadcast in the queue. XhtmlDocView answers ActiveClerkSelectedObject by rendering the clerk's list, and ReversalClerk answers ActiveClerk by possibly changing that list, so for a cached reversal clerk whose stored index disagrees with its owning object the first render could have shown the old index before a second notification corrected it. Moving the pair keeps the queue identical.

The finally in MigrateInUnitOfWork. Before the change, the property was already set when a later layout type threw and the unit of work rolled back; the converted file was saved before the set. Writing the pending selections in a finally reproduces that exactly. Discarding them on failure would be a behaviour change on the error path and is deferred to the conversion of these rows.

The migrator's broadcasts stay. A trace of the first open after a migration: the sets run after the window is shown but before the message pump turns, so the initial view is built from the migrated values and the notifications drain afterwards. The DictionaryPublicationLayout handler validates the publication and, when SelectedPublication is the All Entries sentinel, switches it to the first publication and reloads. The ReversalIndexPublicationLayout handler is the only startup code that makes ReversalIndexGuid follow the layout; everything that runs at view creation goes the other way. Both effects need the deferral to reach a view that did not exist when the set ran.

Base main, not #1185. The status-panel PR touches RecordClerk.cs only in ResetStatusBarPanel and its callers; this branch touches only the activation setter.

Reversals

The reversal-index deletion path, ReallyDeleteReversalIndex, was moved and flipped first: the new index carried out of the lambda, the set after the unit of work, the broadcast dropped, with a RecordListTests test that deleted an index through a test wrapper and used a spy colleague to assert that no ReversalIndexGuid notification was delivered. It was removed when the path turned out to be unreachable: DeleteReversalIndex appears only in ReversalListener.cs, and commit 2161fc2 (LT-16385, December 2015) removed both CmdInsertReversalIndex and CmdDeleteReversalIndex from the Lexicon menus when reversal indexes became one per analysis writing system. The removal of the dead handlers is a separate PR after this one, because both touch the same region of the file. The spy-colleague pattern (register an IxCoreColleague that records OnPropertyChanged names, drain with ProcessPendingItems, assert on the names) worked and is worth reusing for later flips.

Evidence -- the rule checks behind each move

Three rules were applied before each move: (a) nothing between the old and new position reads the property; (b) no other broadcasting set lies between them, or the two handlers are independent; (c) no exception path between them leaves the property unset where it used to be set.

• ReversalListener.Init: (a) and (b) hold trivially; (c) only EndNonUndoableTask throwing differs, which aborts window load in both versions.
• PreHistoricMigrator: later layout types read the two properties through MigratePublicationLayoutSelection only, now served by the shadow read; LegacyConfigurationUtils has no property-table access; no view exists at project open, so no PropChanged listener reads them. (c) holds through the finally.
• RecordClerk setter: (a) SetPropertyInternal reads no other property; (c) only AddNotification is passed, which throws only with a disposed cache.
• XmlDocView: ActivateUI(false) populates no bar; UpdateList reaches PopulateRecordBar, which reads ShowRecordList through IsShowing, so the set stops before it. In the Notebook Document tool the two broadcasts it now passes have no acting handler.
• RecordView, not moved: ActivateUI(true) calls PopulateRecordBar, which reads the property, so no later position exists.
• EditingHelper: nothing between the flag and the set reads the flag.

Test counts on the final tree: xWorksTests 1785 (1783 passed, 2 skipped, 0 failed), LexEdDllTests 20 passed, RootSiteTests 106 (105 passed, 1 skipped), SimpleRootSiteTests 115 passed.

Test-host crash: the first xWorksTests run aborted the host five times (exit 0xc0000409 from ucrtbase, UIAutomationCore and views.dll loaded) during BulkEditBarUiaSmokeTests.FilterBar_RealizedWindow_ExposesTargetCombosThroughUIA, identified from the execution order of the later passing run. That test and its fixture then passed in isolation, and the full project passed in three later runs with the same binaries.

Preflight review details

Code Review Summary

Branch: claude/pubsub-ordering-prep-deb002
Base: main
Date: 2026-10-14
Review model: Claude Fable 5.1
Files changed: 7

Overview

Step 2 of the agreed plan for converting PropertyTable change notifications
from the xCore Mediator queue to Pub/Sub. The inventory flagged sets that sit
inside a unit of work or in the middle of a multi-step sequence (condition 8,
exception ownership), plus one whose suppress flag was set after the broadcast
(condition 2, ordering). This branch moves the sets that can move to after their
unit of work or to the end of their sequence, sets the WritingSystemHvo suppress
flag first, and drops one ReversalIndexGuid broadcast whose only listener is the
clerk already doing the work. Delivery stays on the deferred queue, so under
today's mechanism nothing observable changes. Two candidates stay as they are:
RecordView's ShowRecordList sets, because ActivateUI's PopulateRecordBar reads
the property before the view could set it any later, and the reversal-index
deletion path, which turned out to be unreachable from the UI and will be
removed in a separate PR instead.

The analysis found no Critical or Important finding. The seven Minor findings
below are design choices and residuals the author reviewed and decided.

Contract/API Changes

• PreHistoricMigrator.MigratePublicationLayoutSelection goes from private to
  internal, and ApplyPendingLayoutSelections is a new internal method; both
  exist for xWorksTests (InternalsVisibleTo already in place).
• No public API, serialized format, configuration, resource, script or
  installer contract changes.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs: ApplyPendingLayoutSelections runs in a finally, so DictionaryPublicationLayout and ReversalIndexPublicationLayout are written even when the migration throws and its unit of work rolls back. (author: keep; it reproduces the previous behaviour exactly, the converted file is already saved before the set, and the question is revisited when these publish-reach rows convert)
• Src/LexText/Lexicon/ReversalListener.cs: the empty-guid ReversalIndexGuid set no longer broadcasts, so a ChangeOwningObject failure swallowed by ChangeOwningObjectIfPossible loses the one deferred retry the queued handler used to give it. (author: accepted; the retry would hit the same failure)
• Src/xWorks/XmlDocView.cs: ShowRecordList is now set after TriggerMessageBoxIfAppropriate, so a configured msgBoxTrigger's modal loop would drain queued broadcasts before the false value is in place. (author: accepted; the only XmlDocView tool, Notebook Document, has no trigger, and xWindow has already set the property false at the switch)
• Src/LexText/Lexicon/ReversalListener.cs: if EndNonUndoableTask throws in Init, ReversalIndexGuid stays unset where it used to be set. (author: accepted; the exception aborts window load in both versions)
• Src/LexText/Lexicon/ReversalListener.cs: the deletion path originally moved and flipped here is unreachable from the UI (DeleteReversalIndex has had no XML command and no caller since LT-16385 removed it in 2015). (fixed during review: the deletion-path change and its RecordListTests test, wrapper and spy were removed from the branch; a separate removal PR follows this one)
• Src/xWorks/RecordClerk.cs: the setter reorder, the XmlDocView move and the EditingHelper flag order have no automated test. (author: accepted; asserting queue order needs a view-hosting fixture with a Mediator spy, and the manual steps cover them)
• Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs: CurrentLayoutSelection searches a List with LastOrDefault although at most one entry per name can exist. (author: accepted; the list preserves encounter order, which fixes the apply order)

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene -StartedBy agent: both gates
  clean, 0 warnings, 0 errors, on the final tree.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject xWorksTests:
  1785 tests, 1783 passed, 2 skipped, 0 failed, on the final tree (the
  TearDown SynchronizationLockException in LexiconFirstSliceEditContextEdgeCaseTests
  printed as it does on main; not a failed test).
• -TestProject LexEdDllTests: 20 passed, including the new
  ReversalListenerTests test, on the final tree.
• -TestProject RootSiteTests: 106, 105 passed, 1 skipped; and
  -TestProject SimpleRootSiteTests: 115 passed. Both on the branch base; the
  last revert touched only ReversalListener.cs and RecordListTests.cs, which
  neither project references.
• Manual, performed by the author: (1) reversal index chosen at window load
  when the ReversalIndexGuid setting is absent; (2) Lexicon Edit, Browse,
  Dictionary and Reversal Indexes tool switches; (3) Notebook Edit and Document
  record list hide and show; (4) writing system combo following the cursor and
  changing the selection.
• Manual, not performed: opening a pre-8.3 language project with
  .fwlayout files to see the migrated configuration selected. No such
  project was available; the migrator's pending/apply logic is covered by
  the two new PreHistoricMigratorTests.
• Test-host crash, not reproduced: the first xWorksTests run crashed the host
  five times in BulkEditBarUiaSmokeTests.FilterBar_RealizedWindow_ExposesTargetCombosThroughUIA
  (abort in ucrtbase with UIAutomationCore loaded). That test then passed in
  isolation and the full project passed in three later runs on the same
  binaries, so it is classified as desktop/UI Automation flakiness.
• Jira: a tester can reach the record list and reversal index behaviour, but
  the branch changes none of it observably; the author was offered a ticket and
  did not ask for one.

Positive Observations

• Every move was checked against three explicit rules (no reader between the
  old and new position, no reordering of notifications in the queue, no
  exception path that leaves the property unset), and the moves that failed a
  rule were left in place with the reason recorded.
• The clerk setter moves both broadcasting sets together, keeping their order,
  so the queue is identical to before.
• New tests pin the migrator's deferred write and shadow-read rules and the
  ReversalListener startup selection; comments state the rule, not the history.
• Scope stayed minimal: seven files, no configuration or resource changes.

Interview Notes

• Author reviewed each production change and asked for the mechanism behind
  the EditingHelper, RecordClerk and XmlDocView changes; the explanations were
  accepted.
• Decision: keep the finally in MigrateInUnitOfWork for this PR; revisit when
  the publish-reach rows convert.
• Decision: the reversal-index deletion path (candidate 2) leaves this PR and
  is removed in a separate PR after this one merges, not before and not in
  parallel, because both touch the same region of ReversalListener.cs.
• Decision: the migrator's two sets keep their broadcast flag. A trace showed
  the deferred handlers still do work at the first open after a migration:
  the DictionaryPublicationLayout branch can change SelectedPublication from
  All Entries to the first publication, and the ReversalIndexPublicationLayout
  branch is the only startup code that makes ReversalIndexGuid follow the
  layout rather than the other way round.
• Commit message: the author asked that it state up front that only the
  exception-ownership condition is cleared and the other conditions remain,
  and that the RecordView explanation move from the commit to the PR body.
• Base: main, not PR Replace the StatusPanel property broadcasts with direct window calls #1185, because the two touch different regions of
  RecordClerk.cs; the branch was fast-forwarded to main after LT-22689: Give detail section headers a +/- box and lasting state #1184 by another
  session and re-verified byte for byte.

In-Review Quality Check

The candidate-2 revert was rebuilt with both hygiene gates, and xWorksTests and
LexEdDllTests were rerun afterwards (results above).

Suggested Review Focus

• Src/xWorks/RecordClerk.cs setter: both broadcasting sets now follow
  AddNotification, in their old order.
• Src/xWorks/DictionaryConfigurationMigrators/PreHistoricMigrator.cs:
  the pending list, the shadow read, and the finally.
• Src/xWorks/XmlDocView.cs: the set sits after ActivateUI and before
  UpdateList, the first reader.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   14m 23s ⏱️ + 3m 20s
6 662 tests +3  6 577 ✅ +3  85 💤 ±0  0 ❌ ±0 
6 671 runs  +3  6 586 ✅ +3  85 💤 ±0  0 ❌ ±0 

Results for commit 647bda2. ± Comparison against base commit e8487e6.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 97.67442% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 39.65%. Comparing base (2c2a4c3) to head (647bda2).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/xWorks/XmlDocView.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1187      +/-   ##
==========================================
+ Coverage   39.52%   39.65%   +0.12%     
==========================================
  Files        1538     1541       +3     
  Lines      354569   354905     +336     
  Branches    41090    41178      +88     
==========================================
+ Hits       140159   140747     +588     
+ Misses     185034   184777     -257     
- Partials    29376    29381       +5     

Files with missing lines	Coverage Δ
Src/Common/SimpleRootSite/EditingHelper.cs	38.90% <100.00%> (ø)
Src/LexText/Lexicon/ReversalListener.cs	37.62% <100.00%> (+15.01%)	⬆️
...onaryConfigurationMigrators/PreHistoricMigrator.cs	90.02% <100.00%> (+1.46%)	⬆️
Src/xWorks/RecordClerk.cs	44.79% <100.00%> (-0.11%)	⬇️
Src/xWorks/XmlDocView.cs	7.41% <0.00%> (ø)

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