Skip to content

Publish JumpToRecord at once from three detail-pane handlers - #1241

Merged
mark-sil merged 1 commit into
mainfrom
claude/pubsub-jump-slice-menu-senders-4b88f2
Nov 11, 2026
Merged

mark-sil merged 1 commit into
mainfrom
claude/pubsub-jump-slice-menu-senders-4b88f2

Conversation

@mark-sil

@mark-sil mark-sil commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Three WinForms detail-pane handlers now publish JumpToRecord to their own window, so the window's active clerk selects the record before the handler returns. Until now each queued it with the Mediator's BroadcastMessageUntilHandled, which ran after the handler had returned and offered the jump to every colleague of the window.

The three senders are ReversalPOSMenuHandler's OnMoveReversalPOS and OnMergeReversalPOS, and Slice.InsertObject, which jumps to a new sub-item that got no slice under the inserting one.

The jump can run at once because nothing in these handlers needs it to be late. The reversal handlers do nothing after it, and in Slice.InsertObject the clerk's part of the jump touches no slice, so the focus step that follows is unaffected. What the unit of work left queued, the detail pane's postponed refresh, does not depend on the jump, and the clerk's display of the record it jumped to stays queued behind it, as before.

When a filter hides the target, the clerk's prompt to turn the filter off now opens inside the handler, so the two reversal handlers publish as their last step. Slice.InsertObject tested the broadcast's result, which was always false; that test is gone and the focus step after it runs as before.

Reviewer notes

Start here: Src/Common/Controls/DetailControls/Slice.cs, InsertObject -- the one sender with code after the jump; the two ReversalEntryPOS.cs changes are the same swap with nothing after it.

The unknown: what does an immediate jump change? The only Pub/Sub subscriber is RecordClerk.JumpToRecordViaPublisher, which acts only in the window's active clerk, and both ends are scoped to the main window.

  • Reversal Index Categories has no other colleague that handled JumpToRecord, so the clerk was already the only receiver there.
  • Slice.InsertObject reaches the publish only when the new object got no slice under the inserting slice. The clerk then either re-shows the record that owns the new object (IndexOfParentOf) or, in a list whose sub-items are records, selects it. Either way it touches no slice before the focus loop runs.
  • The filter prompt is the one visible difference: it now opens while the handler is on the stack.

Where to look.

  • The filter prompt inside OnMoveReversalPOS / OnMergeReversalPOS: nothing follows the publish, and manual testing covered both answers.
  • Slice.InsertObject: the removed if was dead, so return insertionPosition; never ran; the focus loop is unchanged.

Deliberately not here. ReversalIndexEntryMenuHandler.OnMoveReversalindexEntry stays on the Mediator. In Lexicon > Reversal Indexes the preview XhtmlDocView is a High-priority Mediator colleague, so the broadcast reaches its OnJumpToRecord before the clerk, and that handler sets the flag that regenerates the preview. With only the clerk subscribed, a move between entries on the same preview page left the preview stale. It converts once the doc view's JumpToRecord handling moves to Pub/Sub for all senders.

Verification.

  • build.ps1 -CommentHygiene -TokenHygiene: 0 warnings, 0 errors.
  • New SliceInsertJumpTests: the jump arrives before the insert returns, reaches only its own window, and queues nothing on the Mediator. It fails with main's broadcast restored.
  • Targeted runs: LexEdDllTests reversal fixtures 20/20; DetailControls slice and DataTree fixtures 27 passed, 1 skipped; xWorks menu-authority, menu-execution, edit-context and navigation fixtures 109/109.
  • Manual, WinForms detail views, project MXB: move and merge reversal categories, including the one the list is on; a filter-hidden target with Yes and No; a move in one of two windows leaves the other alone; Lexicon Edit Insert > Etymology from Citation Form; Insert Reversal Subentry.
  • No automated test for the two reversal handlers: each opens a modal dialog first.

No Jira ticket: Pub/Sub work gets none.

Next: approve, or tell me what to change.

Preflight review details

Code Review Summary

Branch: claude/pubsub-jump-slice-menu-senders-4b88f2
Base: origin/main (merge base 4e0ba82; merges cleanly with c6cd25f)
Date: 2026-11-11
Review model: Claude Opus 5.5
Files changed: 3 (1 commit)

Overview

Part of retiring the xCore Mediator: three deferred BroadcastMessageUntilHandled("JumpToRecord")
senders become an immediate, window-scoped Publisher.Publish. They are
ReversalPOSMenuHandler.OnMoveReversalPOS and OnMergeReversalPOS (Lists > Reversal Index
Categories) and Slice.InsertObject, which jumps to a new sub-item that got no slice under the
inserting slice. The only subscriber is RecordClerk.JumpToRecordViaPublisher, which acts only in
the window's active clerk.

ReversalIndexEntryMenuHandler.OnMoveReversalindexEntry was converted and then put back on the
Mediator. In Lexicon > Reversal Indexes the preview XhtmlDocView is a High-priority Mediator
colleague, so the broadcast reaches its OnJumpToRecord before the clerk. That handler sets the
flag that regenerates the preview, so with only the clerk subscribed, a move between entries on
the same preview page left the preview stale (seen in manual testing).

Contract/API Changes

None. No public members change; Slice.InsertObject keeps its return value (the removed
return insertionPosition; branch was unreachable because the broadcast always returned false).

Findings

Critical - Must address before merge

None.

Important - Should address before merge

  • Move Entry left the Reversal Indexes preview stale (fixed during review: OnMoveReversalindexEntry stays on the Mediator)

Minor - Consider

  • The filter prompt now opens inside the handler (the two reversal handlers publish as their last step; manual steps A5 and A6 covered both prompt answers)
  • Slice.InsertObject runs its focus loop after an immediate jump (the clerk's part touches no slice; the prompt is unreachable there because the new object's owner chain contains the current record)
  • The two reversal handlers have no automated test (author's decision: they open a modal dialog first, and extracting methods only for a test was rejected; manual run A covers them)

Required Validation / Evidence

  • ./build.ps1 -CommentHygiene -TokenHygiene -BuildTests -SkipNative: clean, 0 warnings.
  • ./test.ps1 targeted: SliceInsertJumpTests 1/1; LexEdDllTests ~Reversal 20/20;
    DetailControls SliceTests|DataTreeTests|DataTreeCommandAdapterTests 27 passed, 1 skipped;
    xWorks menu-authority, menu-execution, edit-context and navigation fixtures 109/109.
  • Control run: SliceInsertJumpTests fails with main's broadcast restored.
  • Manual (author, MXB project, WinForms detail views): move and merge reversal categories,
    including the current one, the filter prompt with both answers, two windows; Insert > Etymology
    from Citation Form in Lexicon Edit; Insert Reversal Subentry. All passed.

Jira: none, per the standing rule that Pub/Sub work gets no tickets.

Positive Observations

  • Each sender diff is the broadcast swapped for a scoped publish, with no restructuring.
  • The new test asserts the jump arrives before the insert returns, reaches only its own window,
    and queues nothing on the Mediator.

Interview Notes

  • The author chose in-place conversion over extracting MoveCategory/MergeCategory/MoveEntry
    for testability.
  • The author chose to leave OnMoveReversalindexEntry on the Mediator until the doc view's
    JumpToRecord handling is converted for all senders together.
  • A one-off hang in the Reversal Indexes preview during testing did not reproduce on main or on
    this branch.

Suggested Review Focus

  • The rationale for running the jump immediately at the two reversal handlers and at
    Slice.InsertObject.

🤖 Generated with Claude Code


This change is Reviewable

Three WinForms detail-pane handlers now publish JumpToRecord to their
own window, so the window's active clerk selects the record before the
handler returns. Until now each queued it with the Mediator's
BroadcastMessageUntilHandled, which ran after the handler had returned
and offered the jump to every colleague of the window.

The three senders are ReversalPOSMenuHandler's OnMoveReversalPOS and
OnMergeReversalPOS, and Slice.InsertObject, which jumps to a new
sub-item that got no slice under the inserting one.

The jump can run at once because nothing in these handlers needs it
to be late. The reversal handlers do nothing after it, and in
Slice.InsertObject the clerk's part of the jump touches no slice, so
the focus step that follows is unaffected. What the unit of work left
queued, the detail pane's postponed refresh, does not depend on the
jump, and the clerk's display of the record it jumped to stays queued
behind it, as before.

When a filter hides the target, the clerk's prompt to turn the filter
off now opens inside the handler, so the two reversal handlers publish
as their last step. Slice.InsertObject tested the broadcast's result,
which was always false; that test is gone and the focus step after it
runs as before.

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

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 54s ⏱️ + 2m 40s
7 164 tests +1  7 079 ✅ +1  85 💤 ±0  0 ❌ ±0 
7 173 runs  +1  7 088 ✅ +1  85 💤 ±0  0 ❌ ±0 

Results for commit 6d2abb7. ± Comparison against base commit c6cd25f.

@codecov-commenter

codecov-commenter commented Nov 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.70%. Comparing base (4e0ba82) to head (6d2abb7).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
Src/LexText/Lexicon/ReversalEntryPOS.cs 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1241      +/-   ##
==========================================
+ Coverage   40.55%   40.70%   +0.14%     
==========================================
  Files        1559     1567       +8     
  Lines      356164   356900     +736     
  Branches    41607    41754     +147     
==========================================
+ Hits       144436   145263     +827     
+ Misses     182425   182306     -119     
- Partials    29303    29331      +28     
Files with missing lines Coverage Δ
Src/Common/Controls/DetailControls/Slice.cs 60.49% <100.00%> (+0.24%) ⬆️
Src/LexText/Lexicon/ReversalEntryPOS.cs 0.00% <0.00%> (ø)

... and 50 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 3 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 21935d3 into main Nov 11, 2026
9 checks passed
@mark-sil
mark-sil deleted the claude/pubsub-jump-slice-menu-senders-4b88f2 branch November 11, 2026 16:13
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