Remove dead OnPropertyChanged traffic in both directions

[mark-sil]

Start here: Src/Common/Controls/XMLViews/XmlSeqView.cs, the one file where more than a trailing flag changes. The other 52 files each change one trailing true to false.

What it does. Every PropertyTable.SetProperty with the broadcast flag queues a Mediator OnPropertyChanged job even when no colleague has a branch for that name. An inventory of all 177 production broadcasting sites against all 23 handlers found 113 whose name nothing handles; those now pass false. Values, settings groups and persistence calls are untouched. The two handler branches whose names nothing broadcasts are gone too: XmlBrowseView's readOnlyBrowse branch and XmlSeqView's ShowFailingItems-<tool> branch, with the two tests that drove the latter directly; the field that branch updated is now a MakeRoot local.

What breaks? Nothing: every colleague received each dead job and ignored it. The review question is whether the dead set is right; the evidence accordion has the derivation, re-run on current main before editing.

Where to look

• A flipped name that some handler does react to: the handler map was re-derived on main (same 23 handlers, same names), and the extractor, re-run afterwards, reports exactly the 113 inventoried rows changed.
• A flip that touched a value or persistence argument: every PropertyTable overload takes the flag last; a mechanical scan shows each changed pair differs by one trailing token.
• Two sites dead by context rather than by name: InfoPane's ActiveClerk (its value is never the ReversalClerk that handles it) and XmlDocConfigureDlg's layout property (only NotebookPublicationLayout or null is reachable).
• The listener removals: no readOnlyBrowse setter exists; the only ShowFailingItems- button belongs to a tool whose view is XhtmlDocView, and XmlSeqView is hosted only by the Notebook Document tool.
• ReversalClerk purges the sort-sequence cache folder on every broadcast it receives (reversal tools only), so dead broadcasts used to trigger it too; harmless, since RecordList.RestoreFrom never restores anything.

Not here. The 63 live sites, the four XML-driven family sites (Choice.OnClick, ChoiceGroup, xWindow.LoadDefaultProperties, LinkListener.FollowActiveLink) and RecordClerk.ResetStatusBarPanel keep broadcasting; they convert per workflow later. The inventory lives in the gitignored Docs/migration/working/ folder.

Verification. Off main, not stacked. build.ps1 -CommentHygiene -TokenHygiene: 0 warnings, 0 errors. test.ps1 per project: XMLViewsTests 117, xCoreInterfacesTests 19, DetailControlsTests 124 (1 skipped), LexTextDllTests 10, xWorksTests 1777 (2 skipped), all passed. Not run: the full managed suite and the native suites. Nothing a tester can reach changes, so no Jira ticket.

Next: approve, or name a flipped site you want argued.

---

Reading this a year from now -- start here

The inventory behind this PR (every SetProperty/SetDefault site under Src, every OnPropertyChanged handler and the names it reacts to, the 63 live sites classified by reach and timing, the deferral conditions and the workflow index) is a working document in the gitignored Docs/migration/working/ folder: propertytable-broadcast-buckets.md, propertytable-dead-sites.tsv and the extractor propertytable-broadcast-extract.py. It was never on a branch, so there was nothing to delete here; the parts a reader of this PR needs are summarized in the sections below.

Decisions, and why

• Per-site flips, not a filter in PropertyTable.BroadcastPropertyChange. A central list of names with no handler would be a second copy of the handler map, and it would rot silently. The flag at the call site is the existing contract, and the next step converts the live sites one workflow at a time, so each site is visited anyway.
• Dead code first, in both directions. Removing the dead broadcasts and the dead listeners before any ordering or timing work means the later per-workflow conversion reasons only about jobs that do something.
• Two commits split by direction, so the mechanical flips and the handler removals can be read and reverted separately.
• The XML-driven families stay. Choice.OnClick, ChoiceGroup and xWindow.LoadDefaultProperties broadcast whatever name the configuration gives them, and LinkListener.FollowActiveLink sets every property a link carries; their members are decided one by one in the live-site work.
• RecordClerk.ResetStatusBarPanel keeps its flag. Three of its four callers reach xWindow's status panels; only the DialogFilterStatus caller is dead.
• The leftover XmlSeqView field became a local during review rather than in a separate commit, so the listener commit shows the whole of what that handler left behind.
• Two handlers that handle nothing stay. TemporaryRecordClerk.OnPropertyChanged is an intentional empty override, and Ticker is an xCore sample class that production never instantiates.

Surprising findings

• XmlSeqView is constructed only by XmlDocView (the Notebook Document tool, XmlDocView.cs:605) and reaches the Mediator through XmlDocView.GetMessageAdditionalTargets. RecordDocView hosts XmlDocItemView, not XmlSeqView.
• The one configured RecordDocView subclass is RecordDocXmlView, the segment preview inside the modal Find Example Sentences dialog (Lexicon/areaConfiguration.xml). Its parameters carry no layoutProperty, so RecordDocView.RunConfigureDialog passes null to the configure dialog (a Debug.Assert fires in debug builds). The Notebook-only CmdConfigureXmlDocView cannot reach it.
• RecordList.RestoreFrom reads the persisted sort sequence, deletes the file and returns false unconditionally, so the persist/restore pair and the FwXWindow.DiscardProperties purge are dead weight today. A separate session is looking at it.
• The extractor's test-code heuristic misses Src/xWorks/xWorksTests/Avalonia/Hosting/TestLocalizationManagerBootstrap.cs, which is why it reports 178 production broadcasting sites on main against 177 inventoried.

Deferred, and what would unblock it

• The 63 live sites. Each is classified by reach (direct: every reacting handler is reachable from the set site; publish: at least one is not) and timing (immediate, or deferred with the numbered conditions it fails: set during construction, ordering within the action, handler tears down the sender, re-entry, needs layout or focus settled, thread, teardown, exception ownership, fixed slot among the workflow's other delayed paths). They are grouped by workflow (window initialization, area switch, tool switch, record navigation, clerk activation, follow link, reversal index change, dictionary configuration, writing system and style, view toggles, interlinear tab, UI mode, parser status, respeller apply, filters, Pathway export) and each workflow converts as a unit, coordinated with the JumpToRecord Phase 2, FollowLink and MasterRefresh Phase C plans that share three of them.
• CheckParserUpdatesAnalyses has no handler but is set only through Choice.OnClick; it goes with that family.
• The dead sort-sequence restore is being investigated in its own session; this PR does not touch it.

Evidence

Counts. 427 SetProperty/SetDefault sites under Src: 179 in test code, 70 production sites with a literal false flag, 177 production broadcasting sites (the extractor reports 178; the extra row is the misclassified test bootstrap file), of which one is excluded because its flag comes from callers that all pass false (DictionaryConfigurationListener.cs:407), leaving 176 live: 113 dead (this PR) and 63 live (deferred).

Inventory versus main. The inventory was taken on a tree at the pre-squash form of #1177. Main has since added #1163, #1177 and #1176; the files that differ are Avalonia detail files and Hermit Crab parser files. None of the 23 handler files, no GetMessageTargets chain and no file under DistFiles/Language Explorer/Configuration changed. All 113 dead rows matched main by file, line, member and name before editing.

Handlers on main (23), and the names they react to. xWindow/FwXWindow: currentContentControl, ShowRecordList, WritingSystemHvo, BestStyleName, StatusPanel<id>. AreaListener: currentContentControlObject, areaChoice. NavBarAdapter: areaChoice, ToolForAreaNamed_<current area>. PaneBar: the boolProperty of its own buttons. MultiPane: ActiveClerkSelectedObject, ToolForAreaNamed_lexicon, Show_<first pane id>. XhtmlDocView: SelectedPublication, DictionaryPublicationLayout, ReversalIndexPublicationLayout, ActiveClerkSelectedObject, ShowFailingItems-lexiconClassifiedDictionary. RecordEditView: ShowHiddenFields-*, UIMode, UIModeDisabledTools. InterlinMaster: InterlinearTab. RecordClerk: ShowRecordList, SelectedTreeBarNode, SelectedListBarNode, currentFilterForRecordClerk_<id>, <providing clerk id>-selected. ReversalClerk: ReversalIndexGuid, ToolForAreaNamed_lexicon, ActiveClerk, ReversalIndexPublicationLayout. TemporaryRecordClerk: nothing. DataTree: ShowHiddenFields, ShowHiddenFields-*, currentContentControlObject. MultiStringSlice, ReversalIndexEntrySlice: SelectedWritingSystemHvosForCurrentContextMenu. BrowseViewer: SortedFromEnd, SortedByLength. XmlBrowseView: <browse id>_readOnlyBrowse (removed here). XmlBrowseRDEView: areaChoiceParameters, currentContentControlParameters. SimpleRootSite: WritingSystemHvo, BestStyleName. XmlSeqView: ShowFailingItems-<current tool> (removed here). RawTextPane: WritingSystemHvo, ActiveClerkSelectedObject, ShowInvisibleSpaces, ClickInvisibleSpace. InterlinDocForAnalysis: ITexts_AddWordsToLexicon. Ticker: everything, never instantiated.

Dead-listener derivation. Every handler name above, minus the names broadcast with true on main (the 63 live sites and the computed names they produce), minus the XML-driven family members, leaves exactly <browse id>_readOnlyBrowse and ShowFailingItems-<current tool> outside the classified-dictionary tool.

Computed names at flipped sites, resolved on main. xWindow.CreateStatusBar sets the raw panel ids (Message, Progress, Area, ProgressBar, Sort, Filter, ParsingDev, RecordNumber), not StatusPanel<id>. The clerk filter and sorter ids come from RecordList.PropertyTableId (LexDb.Entries_filter and the like), not currentFilterForRecordClerk_<id>. PersistedIndexProperty is <id>-Index; StoreClerkInPropertyTable sets RecordClerk-<id>. Search-engine names are the Go-dialog engine names; dynamic list keys are the numeric keys in the configuration; PersistenceProvider sets <context>-<id>-<label>.

Flipped sites per file (113). Slice.cs 2; BrowseViewer.cs 2; ReallySimpleListChooser.cs 1; SearchEngine.cs 1; XmlBrowseViewBaseVc.cs 2; FieldWorks.cs 2; LexicalProviderImpl.cs 6; MergeObjectDlg.cs 2; LexEntryUi.cs 2; FwFindReplaceDlg.cs 2; ConstituentChart.cs 2; FlexPathwayPlugin.cs 1; ComplexConcControl.cs 1; FilterTextsDialog.cs 1; InfoPane.cs 1; InterlinDocForAnalysis.cs 1; InterlinDocRootSiteBase.cs 1; RawTextPane.cs 1; SandboxBase.designer.cs 1; BaseGoDlg.cs 2; CombineImportDlg.cs 1; NotebookImportWiz.cs 3; LiftImportDlg.cs 2; MasterCategoryListDlg.cs 2; MasterListDlg.cs 2; MsaCreatorDlg.cs 1; MsaInflectionFeatureListDlg.cs 2; PhonologicalFeatureChooserDlg.cs 2; AreaListener.cs 2; FLExBridgeListener.cs 5; ReversalListener.cs 1; SwapLexemeWithAllomorphDlg.cs 1; ConcordanceDlg.cs 2; MorphologyListener.cs 1; RespellerDlg.cs 1; AdapterBase.cs 1; ToolbarAdapter.cs 1; PersistenceProvider.cs 1; xWindow.cs 11; ExportDialog.cs 5; FwXWindow.cs 4; GlobalSettingServices.cs 1; InterestingTextList.cs 2; LinkListener.cs 3; RecordBrowseView.cs 2; RecordClerk.cs 13; RecordEditView.cs 1; RecordList.cs 2; TextListeners.cs 1; XhtmlDocView.cs 1; XmlDocConfigureDlg.cs 1; XmlDocView.cs 1.

Mechanical checks. Every PropertyTable overload (SetProperty 3- and 4-argument, SetDefault 3- and 4-argument) takes the broadcast flag last, and SetPropertyInternal consults it only to decide whether to call BroadcastPropertyChange. A scan of git diff -U0 found 113 pairs whose lines differ by one trailing true becoming false, pure deletions in the three listener files, and nothing else. The extractor, re-run after the change, reports 427 rows before and after, 113 rows differing, every difference true to false with the value and settings-group columns unchanged, and the changed set equal to the inventory's dead list. No comment near a flipped site claims the broadcast matters.

Mediator timing. One queued job is processed per posted WM_BROADCAST_ITEM_INQUEUE message, but removing an item from that FIFO queue does not reorder the live jobs or the BeginInvoke callbacks around them, so no ordering changes for the sites that still broadcast.

Tests. test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject <name>: XMLViewsTests 117 passed; xCoreInterfacesTests 19 passed; DetailControlsTests 124 passed, 1 skipped; LexTextDllTests 10 passed; xWorksTests 1777 passed, 2 skipped. After the XmlSeqView local cleanup: rebuilt with tests, XMLViewsTests 117 passed again.

Preflight review details

Code Review Summary

Branch: claude/pubsub-dead-broadcasts-8a2456
Base: main
Date: 2026-10-09
Review model: Claude Fable 5.1 (Claude Code)
Files changed: 55

Overview

The branch is the first step of moving xCore PropertyTable change notification (PropertyTable.SetProperty with the broadcast flag, which queues a Mediator "OnPropertyChanged" job) to the FwUtils publish/subscribe system: it removes the dead traffic in both directions before any ordering or timing work starts. An inventory of every production SetProperty/SetDefault site against every OnPropertyChanged handler (kept in a gitignored working folder, not in the tree) classified 113 of 176 live broadcasting sites as dead, meaning no registered colleague has a branch for the name. Those 113 sites now pass false for the broadcast flag and nothing else on the line changes. In the other direction, the two handler branches whose names nothing broadcasts are removed, with the two tests that drove one of them directly.

The analysis found no Critical or Important issue. The diff was verified mechanically: every changed line in the first commit differs only by one trailing true becoming false; every PropertyTable overload takes that flag last; the extractor that produced the inventory, re-run on the branch, reports exactly the 113 inventoried rows changed and nothing else. The one name-independent handler side effect in the code base (ReversalClerk purging the sort-sequence cache folder on every broadcast it receives) changes frequency in the reversal tools but has no observable effect, because the cache's restore path never restores anything.

Contract/API Changes

• XmlBrowseView.OnPropertyChanged(string) and XmlSeqView.OnPropertyChanged(string) (public overrides) are removed. The inherited SimpleRootSite.OnPropertyChanged serves both types; the Mediator resolves the method by name on the type, so no caller or reflection path breaks. No signature, serialized format, settings key, resource key or script contract changed. Settings persistence at every flipped site is unchanged (value, settings group and persistence calls stay).

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• Src/LexText/Lexicon/ReversalListener.cs:581: the first commit's body says only the queued job goes away, but ReversalClerk calls window.ClearInvalidatedStoredData() before examining the name, so in the two reversal tools dead broadcasts also purged the sort-sequence cache folder; now only live ones do. (author: state it in the PR description, not the commit message; no runtime effect because RecordList.RestoreFrom never restores)
• Src/Common/Controls/XMLViews/XmlSeqView.cs: with its handler gone, m_fShowFailingItems was written and read only inside MakeRoot. (fixed during review: the field became a local in MakeRoot; the author asked for the cleanup and for it to be mentioned in the PR description)
• Src/xWorks/XmlDocConfigureDlg.cs:1198: this flip rests on configuration facts (the only reachable layout-property names are NotebookPublicationLayout and null) rather than on an absent handler; RecordDocView.OnConfigureXmlDocView still falls back to DictionaryPublicationLayout, which XhtmlDocView handles. (author: no change needed; the commit body records the reasoning)

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene: both hygiene gates clean, build succeeded, 0 warnings, 0 errors.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject <name> (the first run built the test assemblies, later runs used -NoBuild): XMLViewsTests 117 passed; xCoreInterfacesTests 19 passed; DetailControlsTests 124 passed, 1 skipped; LexTextDllTests 10 passed; xWorksTests 1777 passed, 2 skipped. No failures.
• After the in-review cleanup: rebuilt with tests (0 warnings, 0 errors) and XMLViewsTests rerun, 117 passed.
• Manual validation: none needed. The change is developer-only; nothing a tester can reach changes. No Jira ticket needed.
• Not run: the full managed suite and the native suites (the diff touches no native code).

Positive Observations

• Minimal, mechanical scope: 113 one-token flips and three pure removals, no refactoring, no comment or behavior changes beyond the stated purpose.
• The dead classification was re-derived on main before editing: the same 23 handlers with the same name sets, every computed property name resolved, and the configuration XML checked for the two listener removals.
• Two commits split by direction (broadcasts, then listeners), each with a body that explains the rule applied and the deliberate exceptions.
• The tests removed covered only the removed handler branch; the surviving refresh-policy tests still cover XmlSeqView's root-box behavior.

Interview Notes

• Author reviewed the full diff and the two commit messages before the commits were made, then asked for an independent review; its three Minor findings and the author's decisions are recorded above.
• Asked for the non-obvious part in their own words: nothing additional; the commit messages make it clear.
• Anything else to flag: nothing.
• Jira: developer-only, no ticket needed.

In-Review Quality Check

The XmlSeqView field-to-local cleanup was made before the commits were created (the author asked for the review first), so it is part of the second commit rather than a separate fix-up. Rebuilt with tests and reran XMLViewsTests afterwards: 0 warnings, 0 errors, 117 passed.

Suggested Review Focus

• The derivation of the dead set: handler names on main minus the names broadcast with true minus the XML-driven family members.
• The two dead-by-context sites (InfoPane.cs:103, XmlDocConfigureDlg.cs:1198) and the one site deliberately left broadcasting (RecordClerk.ResetStatusBarPanel).
• The hosting facts behind the two listener removals: no readOnlyBrowse setter anywhere; the only ShowFailingItems- button belongs to a tool whose view is XhtmlDocView, and XmlSeqView is hosted only by the Notebook Document tool.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 45s ⏱️ - 2m 55s
6 597 tests  - 2  6 512 ✅  - 2  85 💤 ±0  0 ❌ ±0 
6 606 runs   - 2  6 521 ✅  - 2  85 💤 ±0  0 ❌ ±0 

Results for commit 92d6bcf. ± Comparison against base commit bd0608d.

This pull request removes 2 tests.

XMLViewsTests.XmlViewRefreshPolicyTests ‑ XmlSeqViewOnPropertyChanged_ShowFailingItemsChange_ReconstructsRootBox
XMLViewsTests.XmlViewRefreshPolicyTests ‑ XmlSeqViewOnPropertyChanged_ShowFailingItemsUnchanged_DoesNotReconstructRootBox

[codecov-commenter]

Codecov Report

❌ Patch coverage is 25.21739% with 86 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.50%. Comparing base (bd0608d) to head (92d6bcf).

Files with missing lines	Patch %	Lines
Src/XCore/xWindow.cs	18.18%	9 Missing ⚠️
.../FieldWorks/LexicalProvider/LexicalProviderImpl.cs	0.00%	6 Missing ⚠️
Src/LexText/Lexicon/FLExBridgeListener.cs	0.00%	5 Missing ⚠️
Src/xWorks/ExportDialog.cs	0.00%	5 Missing ⚠️
.../LexTextControls/DataNotebook/NotebookImportWiz.cs	0.00%	3 Missing ⚠️
Src/xWorks/LinkListener.cs	0.00%	3 Missing ⚠️
Src/Common/Controls/XMLViews/XmlSeqView.cs	0.00%	2 Missing ⚠️
Src/Common/FieldWorks/FieldWorks.cs	0.00%	2 Missing ⚠️
Src/FdoUi/Dialogs/MergeObjectDlg.cs	0.00%	2 Missing ⚠️
Src/FdoUi/LexEntryUi.cs	0.00%	2 Missing ⚠️
... and 36 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1179      +/-   ##
==========================================
- Coverage   39.51%   39.50%   -0.01%     
==========================================
  Files        1537     1537              
  Lines      354706   354674      -32     
  Branches    41096    41092       -4     
==========================================
- Hits       140160   140131      -29     
+ Misses     185154   185153       -1     
+ Partials    29392    29390       -2     

Files with missing lines	Coverage Δ
Src/Common/Controls/DetailControls/Slice.cs	57.78% <100.00%> (ø)
Src/Common/Controls/XMLViews/XmlBrowseView.cs	24.46% <ø> (-1.06%)	⬇️
Src/LexText/Interlinear/InterlinDocForAnalysis.cs	16.85% <100.00%> (ø)
Src/LexText/Interlinear/SandboxBase.designer.cs	78.00% <100.00%> (ø)
Src/LexText/Lexicon/ReversalListener.cs	22.33% <100.00%> (ø)
Src/xWorks/FwXWindow.cs	11.71% <100.00%> (ø)
Src/xWorks/InterestingTextList.cs	58.43% <100.00%> (ø)
Src/xWorks/RecordEditView.cs	59.51% <100.00%> (ø)
Src/Common/Controls/XMLViews/BrowseViewer.cs	50.89% <50.00%> (ø)
...ommon/Controls/XMLViews/ReallySimpleListChooser.cs	0.00% <0.00%> (ø)
... and 44 more

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