LT-22672: Cover the relevance rule with the most rows riding on it - #1145
Merged
Merged
Conversation
Raised by Jason on #1135, after the merge. MoStemMsa.IsFieldRelevant withholds FromPartsOfSpeech ("Attaches to Categories") unless the owning entry has a proclitic or enclitic, and Morphology.fwlayout:39 declares that part visibility="always" -- so before the relevance gate the row composed on every stem MSA, and now it disappears for every entry without a clitic. That is the gate's most visible consequence and nothing pinned it. The test is Jason's, run and confirmed to bite: suppressing the gate fails its negative half with "Expected: False, But was: True", while the positive control -- add a proclitic allomorph, change nothing else -- keeps passing. Also corrects two comments that claimed more than the code does. The composer implements the SECOND of SliceFilter.IncludeSlice's two gates; the first looks the slice id up in the tool's filter list, and the id never reaches the composer, so that half is LT-22802. The fixture summary said it covered the remainder of the overrides, which was untrue while this test was missing, and still excludes the InflectionClass limb that withholds the row from a compound rule's left/right MSA. xWorksTests filter Avalonia 1642 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1145 +/- ##
=======================================
Coverage 38.99% 38.99%
=======================================
Files 1520 1520
Lines 352711 352711
Branches 40681 40681
=======================================
Hits 137532 137532
Misses 185883 185883
Partials 29296 29296
🚀 New features to boost your workflow:
|
The summary described IsIrrelevantForObject but sat on _propsToMonitor, a field declared between it and the method, which already carries its own comment where it is used. It also asserted that the other gate was absent and named it as tracked elsewhere. That reads as true only until the other gate lands, and the change that lands it branched from a base without this comment, so it would not have corrected the claim. Saying which gate this one IS leaves nothing to fall out of date; LT-22802 is still named, since a Jira id is the pointer that keeps working. Two uses of the banned word went with it, in this file and in the test fixture's summary, along with the "legacy resolves no flid either" aside, which describes this code's own behaviour perfectly well without it. Comments only. DetailFieldRelevanceTests 4 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
thejambi
marked this pull request as ready for review
September 24, 2026 22:21
mark-sil
approved these changes
Sep 26, 2026
mark-sil
left a comment
mark-sil
left a comment
Contributor
There was a problem hiding this comment.
@mark-sil reviewed 2 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on thejambi).
thejambi
pushed a commit
that referenced
this pull request
Sep 28, 2026
…-lists Main now has #1145, which this branch also carried as its first commit. Squash-merging gave it a different identity, so both files it touched conflicted with that commit's own later edits there. Resolved toward main, whose comments had been corrected, with one change: the relevance doc named LT-22802 as covering the other gate. This branch is that gate, so the doc now points at IsFilteredOutByTool, in the same class. The fixture summary keeps its pointer to DetailSliceFilterTests, where that gate is tested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the one composer test that was missing for the field-relevance gate, raised by
@jasonleenaylor after #1135 merged.
MoStemMsa.IsFieldRelevantwithholds Attaches to Categories (FromPartsOfSpeech)unless the owning entry has a proclitic or enclitic allomorph.
Morphology.fwlayoutdeclares that part
visibility="always", so before the relevance gate landed the rowcomposed on every stem MSA. That makes it the gate's most visible consequence, and it
was the only one of its rules with no test.
The test bites
It is Jason's test, run and falsified rather than taken on trust:
Expected: False, But was: TrueThe positive control adds a proclitic allomorph and changes nothing else, so the same
row on the same object is shown to appear as well as to vanish.
Also
Two comments claimed more than the code does, and are corrected:
IsIrrelevantForObjectdescribed itself as the whole ofSliceFilter.IncludeSlice.It implements one of that method's two gates.
_propsToMonitor, a field declared between it and the method itdescribes -- and which already carries its own comment where it is used. Moved onto
IsIrrelevantForObject.true only until LT-22802: Apply a tool's slice filter list in the Avalonia detail view #1146 lands it, and LT-22802: Apply a tool's slice filter list in the Avalonia detail view #1146 branched from a base without this comment,
so it would not have corrected the claim. Saying which gate this one is leaves
nothing to fall out of date, and the two can merge in either order.
LT-22802isstill named: a Jira id is the pointer that keeps working.
IsFieldRelevantoverrides,which was untrue while this test was missing. It now names the one limb still
uncovered: the
InflectionClasscase that withholds the row from a compound rule'sleft or right MSA.
Verification
No behaviour change -- one test and the comments around it.
Merged with
main(8 commits) and re-verified, since a clean merge is not necessarilya compiling one.
TestCategory!=DesktopRequired)build.ps1 -CommentHygiene -TokenHygienecleanAddAllomorphToEntryTest, passes on its own: the SLDRwriting-system repository intermittently cannot write its cache directory during
BackendProvider.EnsureWritingSystemsExist. Seen twice this week on unrelatedtests, including a CI run that went green on a re-run.
The
LexiconFirstSliceEditContextEdgeCaseTeststeardown error in that run ispre-existing and unrelated:
Validate_WhitespaceOnlyLexeme_IsAnErrorstages an edit andnever commits or cancels it, so the unit-of-work write lock is still held when the cache
is disposed.
git blameputs it in LT-22625 (#964). Nothing here touches that file.🤖 Generated with Claude Code
This change is