Skip to content

LT-22883: Show blanks, not LCM placeholders, in Go-dialog results - #1218

Draft
thejambi wants to merge 1 commit into
LT-22671-go-dialog-ws-selectorfrom
LT-22883-go-results-placeholders
Draft

thejambi wants to merge 1 commit into
LT-22671-go-dialog-ws-selectorfrom
LT-22883-go-results-placeholders

Conversation

@thejambi

@thejambi thejambi commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Start here: Src/LexText/LexTextControls/Avalonia/EntryGoLauncherShared.cs -- FirstText and its callers are the fix; LcmMergeEntryDialogLauncher.ConfirmName is the one exception.

Stacked on #1217. Review only the top commit; this retargets to main once #1217 merges.

What it does

The Avalonia Go-family results lists (Go to Entry, Merge, Add/Link Allomorph, Link MSA, Link Entry or Sense, and Insert Entry's matching list) showed LCM's missing-value placeholders where the legacy lists show a blank: *** in Glosses for an unglossed sense, and ??? as the headword of an entry with no form. They now show blanks. The row's Lexeme Form value gets the same fix, though no dialog shows that column today.

"Why is there still a ????"

A sense row with no gloss falls back to LCM's sense chooser label, as the legacy Link Entry or Sense sense list does; LCM writes ??? there for a missing part of speech, in both UIs. The Merge confirmation also keeps ??? for a formless entry, as the legacy Merge dialog does, so the sentence never has a blank name. Everything else reads stored strings (StringOrNull) instead of LCM's display shortcuts (Best*Alternative, HomographForm).

Where to look

  • FirstText writing-system order: analysis-first for glosses, vernacular-first for forms, as the Best* properties it replaces -- GlossesText_*, LexemeFormText_NoFormInAnyWs_IsBlank.
  • HeadwordText returns empty for a formless entry -- HeadwordText_NoFormInAnyWs_IsBlank.
  • The Merge exception -- ConfirmMessage_EntryWithNoForm_KeepsLcmsHeadwordPlaceholder.

Not here

Verification

LexTextControlsTests: 387 passed, 3 older skips; the 8 new tests each failed before the fix. Hygiene and gitlint clean. Manual checks, New UI matching legacy:

  1. In the legacy UI, clear an entry's Lexeme Form (the New UI detail view rejects a blank one).
  2. New UI: Ctrl+F, choose English in Writing System, search that entry's gloss -- its Headword is blank.
  3. Search an entry whose first sense has no gloss -- Glosses lists only the glossed senses.
  4. Reversal Indexes > Senses ... -- the unglossed sense row shows its chooser label, as legacy does.
  5. Tools > Merge with entry... from the formless entry -- the confirmation names it ???.

Next: review the top commit; approve, or name a list cell that should keep LCM's placeholder.


Reading this a year from now -- start here

LT-22883 was filed from findings during LT-22671 (#1217, the Go-dialog writing-system selector). There, an entry with no default-vernacular form showed ??? as its headword; #1217 added HeadwordText's fallback across writing systems, and the placeholders that remained -- ??? for an entry with no form at all, *** for unglossed senses, already on main -- became this ticket. The record of why lists show blanks while the Merge sentence keeps ??? lives here and in dialog-conversion.md §6; no working notes were committed.

Decisions, and why
  • Blank, not a placeholder, in list cells. The legacy browse views render the stored string, so a missing value is an empty cell (confirmed manually: a formless entry's Headword and Lexeme Form are blank in the legacy record list). Parity is the goal of the Avalonia dialogs.
  • Read stored strings rather than strip *** / ???. The placeholders come from LCM's display properties; filtering their text would also hide a stored value that happens to be those characters. FirstText walks the current analysis and vernacular writing systems with StringOrNull and returns the first non-empty value.
  • The Merge confirmation keeps ???. A blank leaves a gap in "X will be merged into Y". The legacy MergeEntryDlg names entries by ILexEntry.HeadWord, which is ??? for a formless entry, so ConfirmName falls back to HeadWord only when HeadwordText is empty.
  • Sense rows fall back to ChooserNameTS. That fallback was always in SenseGloss; BestAnalysisAlternative returned *** and never let it fire. The chooser label is what the legacy sense list shows, including LCM's ??? for an empty part-of-speech slot.
Paths not taken
  • Blank names in the Merge confirmation (full consistency with the lists): rejected because it creates a new difference from the legacy dialog and reads as a broken sentence.
  • Stripping the placeholder text after calling Best*Alternative: rejected for the false-positive reason above.
Evidence

New tests (LexTextControlsTests):

  • EntryGoDialogLauncherTests: GlossesText_UnglossedSense_IsBlank, GlossesText_UnglossedSenseAmongGlossedOnes_IsSkipped, SharedSearch_UnglossedSense_DescriptionShowsNoPlaceholder, SharedEntryOrSenseSearch_UnglossedSense_RowFallsBackToChooserName, LexemeFormText_NoFormInAnyWs_IsBlank, HeadwordText_NoFormInAnyWs_IsBlank.
  • LcmMergeEntryDialogLauncherTests: ConfirmMessage_NamesBothEntriesByHeadword, ConfirmMessage_EntryWithNoForm_KeepsLcmsHeadwordPlaceholder.

HeadwordText callers, all checked: the entry and sense result rows and their description (EntryGoLauncherShared), Insert Entry's matching list (LcmInsertEntryDialogLauncher), and the Merge confirmation (LcmMergeEntryDialogLauncher, which substitutes HeadWord).

The row description text is not shown by any product dialog today (no launcher sets DescriptionLabel), so only the unit test covers it.

Preflight review details

Code Review Summary

Branch: LT-22883-go-results-placeholders
Base: LT-22671-go-dialog-ws-selector (stacked on #1217; retarget to main after #1217 merges)
Date: 2026-10-30
Review model: Claude Opus 5.5
Files changed: 5 (1 commit) vs the stacked base; 20 files / 4 commits vs origin/main

Overview

LT-22883: the Avalonia Go-family results list (Go to Entry, Merge, Add/Link Allomorph,
Link MSA, Link Entry or Sense, plus Insert Entry's matching list) showed LCM's missing-value
placeholders -- *** from Best*Alternative for an unglossed sense or an empty lexeme
form, and ??? from HomographForm for an entry with no form -- where the WinForms lists
show a blank. A new EntryGoLauncherShared.FirstText returns the first stored value across
the current writing systems, or empty; the Glosses, Lexeme Form and sense-row text use it,
and HeadwordText now returns empty. An unglossed sense row therefore falls back to LCM's
sense chooser label, as the code always intended and as the WinForms sense list shows.

The Merge confirmation is the one deliberate exception: a sentence with a blank name reads
badly, and the WinForms MergeEntryDlg names a formless entry by HeadWord (???), so
ConfirmMessage keeps LCM's headword for that case (full parity, author's choice).

Contract/API Changes

  • EntryGoLauncherShared.HeadwordText returns "" (was HomographForm, i.e. ???) for an
    entry with no form in any current writing system. Internal; callers are the results rows,
    Insert Entry's matching list, and Merge's confirmation (which now substitutes HeadWord).
  • New internal EntryGoLauncherShared.FirstText and LcmMergeEntryDialogLauncher.ConfirmMessage.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

  • A formless entry's results row now has an empty accessible name (EntryGoDialogView.axaml.cs:255 uses row.Text; was ???) (accepted: rare data, WinForms browse rows expose no better name; not worth a change in FwAvaloniaDialogs here)
  • SenseGloss / DescriptionText now try vernacular writing systems before the chooser-label fallback (was BestAnalysisAlternative, analysis only) (accepted: matches the Glosses column, which already used analysis-then-vernacular; a vernacular-ws gloss is unusual)
  • FirstText drops LCM's last-resort user-interface-writing-system fallback, so a value stored only there shows blank (accepted: lists show current writing systems, as WinForms columns do)

Required Validation / Evidence

  • .\test.ps1 -TestProject LexTextControlsTests -CommentHygiene -TokenHygiene: 390 total,
    387 passed, 3 skipped (pre-existing skips). The 8 new tests failed before the fix.
  • Comment hygiene (-BaseRef origin/main) and token hygiene clean; gitlint on
    origin/LT-22671-go-dialog-ws-selector..HEAD exit 0.
  • Manual (author-confirmed, New UI vs WinForms):
    1. Ctrl+F: an unglossed sense contributes nothing to Glosses (second, not ***, second).
    2. Ctrl+F with English selected: a formless entry shows a blank Headword, matching the
      WinForms record list.
    3. Link Entry or Sense (Reversal Indexes > Senses): the unglossed sense row shows LCM's
      chooser label 1 ??? a test definition -- the ??? is the empty part-of-speech slot,
      identical in the WinForms sense list.
    4. Merge confirmation names a formless entry ???, matching WinForms.
  • Not covered manually: the row description text (no product dialog shows the description
    pane), covered by SharedSearch_UnglossedSense_DescriptionShowsNoPlaceholder; and the
    row's Lexeme Form value (every launcher uses BuildDefaultResultColumns, Headword and
    Glosses only), covered by LexemeFormText_NoFormInAnyWs_IsBlank.

Positive Observations

  • Test-first: every changed path has a test that failed on the old code.
  • The Merge exception is pinned in both directions (normal names, formless placeholder).
  • No new user-visible strings; no WinForms code touched.
  • The gotcha is recorded in dialog-conversion.md §6 so the next Avalonia list avoids it.

Interview Notes

  • Purpose: display parity with the WinForms lists for missing values (LT-22883, filed from
    findings during LT-22671 / LT-22671: Add a writing-system selector to the Go-family dialogs #1217).
  • Merge confirmation: author chose full parity -- keep ??? in the sentence while lists show
    blanks.
  • Minor findings: all three accepted as-is (reasons above).
  • Testers: a formless entry cannot be made in the New UI (the detail view rejects a blank
    Lexeme Form); create it in the WinForms UI. Author had nothing further to flag.
  • Jira: user-visible, ticket LT-22883 exists; no new ticket needed.

Suggested Review Focus

🤖 Generated with Claude Code


This change is Reviewable

The entry-search dialogs' results showed "***" for a sense with no
gloss and "???" as the headword of an entry with no form, where the
WinForms dialogs show a blank. The Glosses and Lexeme Form columns,
the sense rows and the description text took LCM's best-alternative
strings, which substitute a placeholder when nothing is stored.

They now read the first stored value across the current writing
systems through a new FirstText helper, and HeadwordText returns an
empty string when no writing system has a form. An unglossed sense
row falls back to the sense's chooser name, as intended.

The Merge confirmation still names a formless entry "???", as the
WinForms Merge dialog does, rather than leaving a blank in the
sentence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thejambi
thejambi added this pull request to stack #1219 October 30, 2026 19:43
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files  ± 0      1 suites  ±0   11m 17s ⏱️ - 3m 49s
6 893 tests  - 78  6 808 ✅  - 78  85 💤 ±0  0 ❌ ±0 
6 902 runs   - 78  6 817 ✅  - 78  85 💤 ±0  0 ❌ ±0 

Results for commit 3f3ee7e. ± Comparison against base commit ad464aa.

@codecov-commenter

codecov-commenter commented Oct 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.02%. Comparing base (ad464aa) to head (3f3ee7e).

Files with missing lines Patch % Lines
.../LexTextControls/Avalonia/EntryGoLauncherShared.cs 80.00% 1 Missing and 3 partials ⚠️
...xtControls/Avalonia/LcmMergeEntryDialogLauncher.cs 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@                        Coverage Diff                         @@
##           LT-22671-go-dialog-ws-selector    #1218      +/-   ##
==================================================================
- Coverage                           40.38%   40.02%   -0.36%     
==================================================================
  Files                                1551     1545       -6     
  Lines                              355742   355454     -288     
  Branches                            41508    41392     -116     
==================================================================
- Hits                               143650   142262    -1388     
- Misses                             182770   183848    +1078     
- Partials                            29322    29344      +22     
Files with missing lines Coverage Δ
...xtControls/Avalonia/LcmMergeEntryDialogLauncher.cs 36.17% <62.50%> (+3.94%) ⬆️
.../LexTextControls/Avalonia/EntryGoLauncherShared.cs 85.35% <80.00%> (+2.31%) ⬆️

... and 62 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.

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.

2 participants