Skip to content

Replace the StatusPanel property broadcasts with direct window calls - #1185

Merged
mark-sil merged 1 commit into
mainfrom
claude/pubsub-status-panels-1822e2
Oct 14, 2026
Merged

mark-sil merged 1 commit into
mainfrom
claude/pubsub-status-panels-1822e2

Conversation

@mark-sil

@mark-sil mark-sil commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/XCore/xWindow.cs, the new SetStatusPanelText and the OnPropertyChanged branch it replaces.

The record number, record message and parser progress texts in the status bar now reach their panels through one direct window call. A setter used to store StatusPanelRecordNumber, StatusPanelMessage or StatusPanelProgress with the broadcast flag, and the window copied the value into the panel after the Mediator queue drained. Those three properties are gone. Nothing else read them or listened to them. It is a direct call, not Pub/Sub, because every setter can already reach the window.

What could break: timing. The text now appears when the setter runs, not after the current action. The status bar exists from window initialization, every setter runs on the UI thread, and none re-enters, so nothing relied on the delay. A panel id with no panel is ignored.

Where to look

  • XWindow.SetStatusPanelText and the deleted default branch of XWindow.OnPropertyChanged.
  • RecordClerk.ResetStatusBarPanel: its DialogFilterStatus caller is not a panel. The Assign Analysis dialog reads that property, so it keeps a plain set without the broadcast no handler received.
  • ParserListener.UpdateStatusPanelProgress runs on every Idle and timer tick. StatusBarPanel.Text ignores identical text.
  • GeneratedHtmlViewer.Init clears the record-number panel the same way.

Not here: the Filter, Sort and ParsingDev panels, which RecordClerk already sets through their panel objects, and every other PropertyTable broadcast.

Verification: build.ps1 with both hygiene gates is clean. xCoreTests (3 new), xWorksTests, xCoreInterfacesTests and ParserUITests all pass; counts are in the evidence section below. Manual steps:

  1. Lexicon > Lexicon Edit: move between entries. The record-number panel shows "n/N" and the Message panel shows the entry's dates.
  2. Lexicon > Dictionary: the record-number panel goes blank.
  3. Texts & Words, Parser > Parse all words: the Progress panel shows "Queue: (a/b/c)", then "Parsed N word(s)". Close the window mid-parse: no error.
  4. Grammar, Category Edit, then View > Grammar > Grammar Sketch: the record-number panel goes blank.
  5. Texts & Words > Word Analyses, a wordform's "Assign Analysis...": filtering a column shows the dialog's "Filtered" label. Known and unchanged: clearing one of two column filters also clears it.

Next: approve, or tell me what to change.


Reading this a year from now -- start here

No research or working files were on this branch, so nothing was deleted from the tree. This record carries the reasoning that does not fit in the code: why one window method replaced three property broadcasts, what was checked before they were removed, and what the change leaves alone.

Decisions, and why
  • A direct call, not Pub/Sub with delayed execution. Every setter can reach the main window, so no subscription, scope or end-of-action delivery is needed. The only behavioral difference is timing, and no deferral condition applies: the status bar is built during window initialization, the setters run on the UI thread, and the removed handler only set the panel text, never another property.
  • The method lives on XWindow, not on the IxWindow interface. That interface serves code that does not reference xCore, and all three setters reference it.
  • Callers use GetWindow() as XWindow with a null-conditional call. GetValue<T>("window") throws when the property holds another type, and one test fixture stores a plain Form there. The test window MockFwXWindow builds no status bar, so the call has to tolerate a missing panel.
  • The properties are removed, not kept with a false flag. No reader of the three names exists in Src, DistFiles or TestLangProj.
  • No CheckDisposed in the method. ShutDownPart1 clears the panel map before it disposes the panels, so a call on a closing window finds nothing and returns. The Mediator behaved the same way once ProcessMessages was false.
  • DialogFilterStatus keeps a plain property set. RecordClerk.UpdateFilterStatusBarPanel stores it while the dialog's IgnoreStatusPanel flag is set, and ConcordanceDlg reads it as it builds its browse view. The broadcast flag goes false because no handler matched that name.
Paths not taken
  • Writing to the panel object found through the PropertyTable, as RecordClerk already does for the Filter, Sort and ParsingDev panels. It needs no new method. The window method adds one public member and keeps the panel map, which XWindow clears before it disposes the panels, private to the window.
  • Pub/Sub with PublishAtEndOfAction. It keeps today's timing at the cost of a subscriber, a scope and a new event, for a window the setter can already call. Nothing depends on the delay.
  • Keeping the three properties with the broadcast flag false. Nothing reads them.
Surprising findings
  • DialogFilterStatus looks like a dead panel write because no panel has that id, but ConcordanceDlg reads it.
  • StatusBarPanel.Text skips its redraw when the new text equals the current text, checked in the installed framework's IL. The progress text can therefore be set on every Idle without the PropertyTable's change check.
  • Unchanged by this branch: the Assign Analysis dialog sets its "Filtered" label in ConcordanceDlg.BrowseViewer_FilterChanged from the added filter of the last column event. Clearing one of two column filters clears the label while the other filter still applies. The main window's Filter panel uses the combined filter, through RecordClerk.UpdateFilterStatusBarPanel.
Deferred, and what would unblock it

Every other PropertyTable broadcast keeps today's delivery, queued until after the current action. Each one needs its own check of who listens and whether anything depends on the delay.

Evidence
Run Result
build.ps1 -CommentHygiene -TokenHygiene clean, build succeeded
xCoreTests 20 passed, 3 new
xWorksTests 1777 passed, 2 skipped
xCoreInterfacesTests 19 passed
ParserUITests 16 passed

Not run: the other test projects and the full suite.

  • A git grep for the StatusPanel names and DialogFilterStatus over Src, DistFiles and TestLangProj leaves the IgnoreStatusPanel flag, which is a different property and untouched, the names SetStatusPanelText and UpdateStatusPanelProgress, and the DialogFilterStatus setter and reader.
  • ParserListener drives its progress from a System.Windows.Forms.Timer and from the Idle event published by XWindow.SynchronizedOnIdleTime. It unsubscribes from Idle and stops the timer when it is disposed.
  • RecordClerk sets the record number and message inside SelectedRecordChanged, which updates the WinForms record bar in the same call, so the call already needs the UI thread.
Preflight review details

Code Review Summary

Branch: claude/pubsub-status-panels-1822e2
Base: main
Date: 2026-10-14
Review model: Claude Sonnet 5.5 (preflight passes); Claude Fable 5.1 (the earlier code-review pass over the same diff)
Files changed: 7 (1 commit)

Overview

The branch replaces the StatusPanelRecordNumber, StatusPanelMessage and StatusPanelProgress property broadcasts with a direct call on the main window, XWindow.SetStatusPanelText(panelId, text), and deletes the StatusPanel branch of XWindow.OnPropertyChanged that they fed. Three setters changed: RecordClerk.ResetStatusBarPanel, ParserListener.UpdateStatusPanelProgress and GeneratedHtmlViewer.Init. The fourth ResetStatusBarPanel caller sets DialogFilterStatus, which no panel has; the Assign Analysis dialog reads it, so that caller keeps a plain property set without the broadcast.

The analysis found no Critical or Important issue. Every claim in the commit was checked against the code: the only listener for the three names was the removed branch, and nothing in Src, DistFiles or TestLangProj reads their values.

Contract/API Changes

  • Added: public XWindow.SetStatusPanelText(string panelId, string text). Unknown or null ids are ignored.
  • Removed behavior: XWindow.OnPropertyChanged no longer reacts to names starting with StatusPanel, and the three properties are no longer stored in the PropertyTable.
  • Changed: DialogFilterStatus is set with the broadcast flag false; no handler matched that name.
  • Private only: RecordClerk.ResetStatusBarPanel now takes a panel id; its constant StatusBarRecordNumber holds RecordNumber.
  • Comment text only: the status bar comment in Main.xml and a trace comment in XmlBrowseRDEView.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

  • A null panel id threw ArgumentNullException although the method documents ignoring unknown ids (fixed during review: the lookup is guarded and the unknown-id test also passes null)
  • A test literal ran 121 columns against the 98 limit (fixed during review: literal split)
  • SetStatusPanelText does not call CheckDisposed like other public xWindow members (kept: the panel map is cleared before the panels are disposed, so a call on a closing window finds nothing and returns, as the dropped Mediator items did; throwing from a parser timer tick during close would be worse)
  • A second way to reach a status panel now exists beside the panel objects RecordClerk reads from the PropertyTable for Filter, Sort and ParsingDev (kept: one window method keeps the panel map private to XWindow; the alternative is recorded in the PR)
  • The removed prefix branch let any PropertyTable holder set a panel's text (informational: no such setter exists in Src)
  • Panel ids are string literals in three files (kept: same as the existing Filter, Sort and ParsingDev literals; an unknown id is ignored by design)

Required Validation / Evidence

Run on the committed tree, all passing:

  • .\build.ps1 -CommentHygiene -TokenHygiene -StartedBy agent: both gates clean, build succeeded.
  • .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject xCoreTests: 20 passed (3 new).
  • Same switches with -NoBuild and -TestProject xWorksTests: 1777 passed, 2 skipped, 0 failed. The log carries a TearDown note from UnitOfWorkService.Dispose in LexiconFirstSliceEditContextEdgeCaseTests; it is not a failed test and the code is untouched here.
  • -TestProject xCoreInterfacesTests: 19 passed. -TestProject ParserUITests: 16 passed.

Manual: the author confirmed step 5 (Grammar Sketch clears the record-number panel) and exercised step 6 (the dialog's filter label). Steps 1 to 4 are written into the PR for the reviewer or tester and are not recorded here as performed.

Not run: the other test projects and the full suite.

Positive Observations

  • One seam, null-safe, with the three observable cases covered by tests.
  • Threading was verified, not assumed: the parser progress runs from a Windows Forms timer and the Idle publisher, and the clerk updates run on the UI thread.
  • StatusBarPanel.Text compares with the current text before redrawing (checked in the installed framework), so setting the progress text on every tick costs nothing when it has not changed.
  • The listener unsubscribes from Idle and stops its timer on disposal, and window teardown is synchronous, so the close-while-parsing case needs no extra guard.

Interview Notes

  • The author reviewed the diff and the findings above before the commit and raised no objection. No finding was dismissed with an author explanation of its own, and no lack of understanding was recorded.
  • Ticket decision: a tester can reach the status bar in the running application, and the branch carries no LT key. Whether to file one is the author's call.
  • Observed while running step 6, not caused by this branch: the Assign Analysis dialog's "Filtered" label reflects only the last column's filter event, so clearing one of two column filters clears it. ConcordanceDlg.BrowseViewer_FilterChanged is untouched here.

In-Review Quality Check

No changes during preflight. The two fixes above were made in the code-review pass before the commit.

Suggested Review Focus

  • XWindow.SetStatusPanelText and the deleted default branch of XWindow.OnPropertyChanged
  • RecordClerk.ResetStatusBarPanel callers and the DialogFilterStatus caller
  • ParserListener.UpdateStatusPanelProgress setting text on every tick
  • Manual steps 1 to 5 in the PR description

🤖 Generated with Claude Code


This change is Reviewable

The StatusPanelRecordNumber, StatusPanelMessage and StatusPanelProgress
properties carried the record number, the record message and the parser
progress to the main window's status bar: a setter stored the property
with the broadcast flag, the xCore Mediator queued an OnPropertyChanged
item, and after the current action the window copied the property value
into the panel whose id is the name without its StatusPanel prefix.
Nothing else listened to those three names, and nothing read their
values.

The window now exposes SetStatusPanelText(panelId, text), and the
setters call it with the panel id from Main.xml (RecordNumber, Message,
Progress). The StatusPanel branch in XWindow.OnPropertyChanged is gone,
and the three properties are not stored any more. The text appears when
the setter runs instead of when the Mediator queue drains after the
action; the status bar exists from window initialization, every setter
runs on the UI thread, and none of them is re-entered, so no caller
depended on the delay. A panel id with no panel is ignored, which also
covers windows built without a status bar.

DialogFilterStatus went through the same setter but is not a panel: the
Concordance dialog reads it from the PropertyTable to show its own
filter state. It keeps a plain property set without the broadcast, which
no handler ever received.

A direct call is also simpler than moving these properties to Pub/Sub
with delayed execution: each setter can already reach the window, so no
subscription, scope or end-of-action delivery is needed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   13m 44s ⏱️ + 3m 22s
6 598 tests +3  6 513 ✅ +3  85 💤 ±0  0 ❌ ±0 
6 607 runs  +3  6 522 ✅ +3  85 💤 ±0  0 ❌ ±0 

Results for commit 273c699. ± Comparison against base commit a90754c.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.51%. Comparing base (a90754c) to head (273c699).

Files with missing lines Patch % Lines
Src/xWorks/RecordClerk.cs 33.33% 2 Missing and 2 partials ⚠️
Src/xWorks/GeneratedHtmlViewer.cs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1185      +/-   ##
==========================================
+ Coverage   39.49%   39.51%   +0.01%     
==========================================
  Files        1537     1537              
  Lines      354392   354387       -5     
  Branches    41058    41059       +1     
==========================================
+ Hits       139974   140031      +57     
+ Misses     185044   184983      -61     
+ Partials    29374    29373       -1     
Files with missing lines Coverage Δ
Src/Common/Controls/XMLViews/XmlBrowseRDEView.cs 0.00% <ø> (ø)
Src/XCore/xWindow.cs 39.77% <100.00%> (+3.73%) ⬆️
Src/xWorks/GeneratedHtmlViewer.cs 0.00% <0.00%> (ø)
Src/xWorks/RecordClerk.cs 44.79% <33.33%> (-0.11%) ⬇️

... and 4 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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

@mark-sil
mark-sil merged commit e682826 into main Oct 14, 2026
9 checks passed
@mark-sil
mark-sil deleted the claude/pubsub-status-panels-1822e2 branch October 14, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants