[mark-sil]

The reversal entries list now persists its filter, sorter and sort name under a key named by the reversal index's writing system, and the export reads that same key. Until now the view keyed them by the current ReversalIndexPublicationLayout value and the export by the configuration file's path, so an export that walked another reversal first found nothing for the filtered one and wrote the file unfiltered, and the view had lost its filter when the export ended.

Settings found only under one of the older keys are moved to the writing-system key the first time they are read. A persisted filter or sorter that cannot be restored is logged instead of silently replaced by the default.

Reviewer notes

Root cause, from the acceptance project on the ticket. Its Settings.xml holds ReversalIndexPublicationLayout as an empty string (FLEx creates it on any read with an empty default), so the Bulk Edit Reversal Entries view persisted Ken's filter and reverse sort under ReversalIndex.AllEntries-_filter and _sorter, with no writing system in the key. The Word export switches the one reversal clerk through every index in repository order (French, Spanish, English in that project), restoring each index's settings under a key built from the local <ws>.fwdictconfig path. Visiting French cleared the English filter and replaced the sorter with the default; coming back to English found nothing under the path key, so the file was written unfiltered and ascending (the tester's screenshot shows A, C, H, S, Z headings), and the view had no filter afterwards. The same mismatch exists on a single machine whenever the layout property is empty, and Backup/Restore carries the same two settings files, so this was not an artifact of the zipped project.

Not a regression. Every function on this path is unchanged since the September 2025 fix (699d1ad). The 2025 verifications passed because the testers re-applied the filter after switching views, which rewrote it under a path key the export could find.

Deliberate rules.

• AllReversalEntriesRecordList.PropertyTableId keys by the configuration's writing system when one is given, otherwise by the writing system of the index ReversalIndexGuid names. It returns null when no index is current, and the clerk then persists nothing rather than writing under a null name.
• GetPersistedSetting reads the writing-system key first. A value found only under an older key is copied to the writing-system key and the older key is removed, so later switches find it, no other index inherits it, and the settings file keeps no empty entry. The older keys tried are: the layout property's path when its file name is the writing system id or the file declares that writing system; the project's <ws>.fwdictconfig path; every other file in the project's ReversalIndex folder that declares the writing system (copied and imported configurations are named after their label); and the empty-layout key, only for the current index.
• Only the root element's writingSystem attribute is read from a configuration file, from a stream; an unreadable or malformed file counts as not matching. A layout value that cannot be a path is never treated as one.
• PropertyTable.RemoveProperty gains an overload taking the settings group, so the migration can drop a local setting outright instead of leaving it with a null value.
• Failed filter and sorter restores are logged through Logger.WriteEvent; the default still applies.

Tests. New DictionaryExportServiceReversalFilterTests replays the export loop (French, then English) against a clerk whose filter was set in the view, against Ken's exact persisted XML under the empty-layout keys, against settings under a configuration-path key, and against settings under a Copy of English.fwdictconfig key; each expects the English result filtered and, where a sorter was persisted, reverse sorted, with the clerk keeping its filter. A further case covers a filter set with no current index. ReversalEntryBulkEditTests covers the key shape, the null case, key preference, and each fallback source with its migration, including a copy outside the project folder reached only through the layout property and a copy inside it found while the layout points at a French copy. PropertyTableTests covers the new RemoveProperty overload. The export-service, clerk, list and reversal test sets in xWorksTests (149), the LexEdDll tests (8) and the PropertyTable tests (11) pass.

Manual tests (done). With the ticket's project: the filter and reverse sort appear in Bulk Edit Reversal Entries; a Word export from the Dictionary view writes the English reversal filtered and reverse sorted; the view keeps its filter after the export and after switching to French and back; a new filter set in the view is honoured by the next export and survives a restart. With the English keys in db$local$Settings.xml renamed to the old form for a Copy of English.fwdictconfig placed in the project's ReversalIndex folder, and the layout property pointing at the English configuration, the view opens with the filter and reverse sort and the file is rewritten under the -en_ keys.

Out of scope. An export-only clerk, so the export stops mutating the view's clerk at all, is a separate change and is not needed for this ticket. The default sorter the clerk builds for an index with no persisted sort uses the writing system of ReversalIndexGuid, not the index being exported; unchanged here.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ± 0      1 suites  ±0   14m 32s ⏱️ +15s
7 063 tests +12  6 978 ✅ +12  85 💤 ±0  0 ❌ ±0 
7 072 runs  +12  6 987 ✅ +12  85 💤 ±0  0 ❌ ±0 

Results for commit 1d1b831. ± Comparison against base commit 9c931ec.

This pull request removes 1 and adds 13 tests. Note that renamed tests count towards both.

LexEdDllTests.ReversalEntryBulkEditTests ‑ PropertyTableIdContainsWsId

LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_PrefersWritingSystemKey
LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_ReadsConfigurationPathKey
LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_ReadsEmptyLayoutKeyForCurrentIndex
LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_ReadsLayoutPathKeyWhenItNamesTheWritingSystem
LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_ReadsLayoutPathKeyWhenTheFileDeclaresTheWritingSystem
LexEdDllTests.ReversalEntryBulkEditTests ‑ GetPersistedSetting_ReadsProjectConfigurationKeyDeclaringTheWritingSystem
LexEdDllTests.ReversalEntryBulkEditTests ‑ PropertyTableId_UsesWritingSystemOfCurrentReversalIndex
SIL.FieldWorks.XWorks.DictionaryExportServiceReversalFilterTests ‑ GetReversalFilteredAndSortedEntries_KeepsFilterSetInView_AfterVisitingAnotherIndex
SIL.FieldWorks.XWorks.DictionaryExportServiceReversalFilterTests ‑ GetReversalFilteredAndSortedEntries_RestoresSettingsPersistedUnderConfigurationPath
SIL.FieldWorks.XWorks.DictionaryExportServiceReversalFilterTests ‑ GetReversalFilteredAndSortedEntries_RestoresSettingsPersistedUnderCopiedConfigurationPath
…

♻️ This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 83.69565% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.55%. Comparing base (8afbeb5) to head (1d1b831).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/xWorks/RecordClerk.cs	60.00%	6 Missing and 2 partials ⚠️
Src/LexText/Lexicon/ReversalEntryBulkEdit.cs	88.88%	2 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1233      +/-   ##
==========================================
+ Coverage   40.50%   40.55%   +0.04%     
==========================================
  Files        1559     1559              
  Lines      356085   356149      +64     
  Branches    41584    41605      +21     
==========================================
+ Hits       144243   144437     +194     
+ Misses     182531   182411     -120     
+ Partials    29311    29301      -10     

Files with missing lines	Coverage Δ
Src/XCore/xCoreInterfaces/PropertyTable.cs	79.11% <100.00%> (+0.22%)	⬆️
Src/xWorks/RecordList.cs	54.07% <100.00%> (+0.28%)	⬆️
Src/xWorks/ReversalIndexServices.cs	72.85% <ø> (ø)
Src/LexText/Lexicon/ReversalEntryBulkEdit.cs	43.18% <88.88%> (+18.57%)	⬆️
Src/xWorks/RecordClerk.cs	49.24% <60.00%> (+1.41%)	⬆️

... and 20 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]

Should fix: a filter saved under a copied reversal configuration is dropped when an older Settings.xml is first read

Src/LexText/Lexicon/ReversalEntryBulkEdit.cs:188, PathDerivedPropertyTableIds

Older builds saved the Bulk Edit Reversal Entries filter and sort under a key built from ReversalIndexPublicationLayout, which is usually the path of the selected reversal configuration. When the writing-system key is still empty, this method moves an old value to it, but it accepts the layout's key only if the file name is the writing system id (NamesWritingSystem).

Users can copy a reversal configuration in Configure Dictionary. The copy is named after its label (Copy of English.fwdictconfig, or whatever the user renames it to), and it appears in the Reversal Indexes configuration menu. Choosing a copy sets ReversalIndexGuid from the copy's WritingSystem (DictionaryConfigurationUtils.SetReversalIndexGuidBasedOnReversalIndexConfiguration).

Scenario: a user views a copied English configuration, sets a filter and reverse sort in Bulk Edit Reversal Entries, then opens the project in a build with this change. This also happens after restoring a backup or opening a zipped project whose Settings.xml came from an older build.

1. The writing-system key ReversalIndex.AllEntries-en_filter is empty.
2. NamesWritingSystem returns false, because the file name isn't en.
3. The project-path key (…\ReversalIndex\en.fwdictconfig) is empty.

The filter and sort silently revert to the defaults. The view had been honouring that filter until then. The user has to re-apply it once, and after that it persists under the writing-system key.

Suggested fix: match the layout by the writing system its configuration declares, the way SetReversalIndexGuidBasedOnReversalIndexConfiguration already does. Keep the file-name check for paths from another machine:

static bool LayoutIsFor(string layout, string wsId, LcmCache cache) =>
    layout.IndexOfAny(Path.GetInvalidPathChars()) < 0
    && (Path.GetFileNameWithoutExtension(layout) == wsId
        || File.Exists(layout) && new DictionaryConfigurationModel(layout, cache).WritingSystem == wsId);

This runs only when the writing-system key is empty, so it costs nothing once the value has moved. A test with a layout such as …\ReversalIndex\Copy of English.fwdictconfig would pin it down.

If copied reversal configurations are rare enough that a one-time loss is acceptable, please say so in the PR body's deliberate rules instead.

[mark-sil]

Fixed in 1d1b831.

The layout path now counts when its file declares the writing system, as you suggested. The migration also tries every file in the project's ReversalIndex folder that declares the writing system, so a copy's settings are found even after the view has moved on to another configuration, which the layout check alone would miss. Only the root element's writingSystem attribute is read (XmlReader over a stream), so the check stays cheap on the reads that run for an index with nothing persisted.

While here, the old key is now removed rather than set to null. A nulled property was still written to the settings file as a value-less entry, which is confusing to anyone inspecting it. That needed a RemoveProperty overload on PropertyTable that takes the settings group.

Tests cover a layout pointing at a copy outside the project folder, a copy inside the folder while the layout points at a French copy, and the export loop restoring from a "Copy of English.fwdictconfig" key. Also verified by hand on the ticket's project with the English keys renamed to the old copy form: the view opens filtered and reverse sorted, and the file is rewritten under the writing-system keys with the old entries gone.

[thejambi]

Another finding - I will approve and you can consider whether this one should be addressed. Thanks!

Should fix: the migration can restore a filter the user cleared

Src/LexText/Lexicon/ReversalEntryBulkEdit.cs, GetPersistedSetting / PathDerivedPropertyTableIds

Clearing the last filter leaves RecordList.Filter null, and RecordClerk.OnChangeFilter persists DynamicLoader.PersistObject(null), which is null. So "the user cleared the filter" is stored as a key that exists with a null value. The migration loop skips that key (if (value == null) continue;) the same way it skips a missing one, and goes on to the next configuration's key. Since 1d1b831 that includes every other configuration in the folder that declares the writing system.

Scenario (older build, then this one):

1. With the English configuration selected, the user filters Bulk Edit Reversal Entries to ^[^z]. That is saved under the en.fwdictconfig path key.
2. They switch the Reversal Indexes configuration to Copy of English and clear the filter. A null is saved under the copy's path key.
3. In this build, the writing-system key is empty. The layout key exists but is null, so the loop moves on. The en.fwdictconfig key holds ^[^z], which is moved to -en_ and installed.

The filter the user removed is back in the view. Because the export now honours it, the English reversal export silently drops the z entries. The same thing happens if the user moved from one English configuration to another without clearing anything: the old build showed no filter for the configuration in use, and the upgrade brings back whatever another configuration still holds.

Suggested fix: treat the first old key that exists as the answer, even when it is null. When the layout belongs to this index, its key is the one the old view read and wrote, so look at no other keys:

if (layout != null && (layout.Length == 0 ? isCurrentIndex : IsConfigurationFor(layout, wsId)))
    return new[] { KeyFor(layout) };   // the view's own key decides, present or not

In the loop, stop at the first key that exists, null included. To make a migrated "cleared" value stick, check PropertyExists on the writing-system key before scanning, instead of relying on a null result. The folder scan then runs only when the layout belongs to another index. That is the case the new ...ProjectConfigurationKeyDeclaringTheWritingSystem test covers. Restricting the scan this way also keeps the read path from opening every configuration file on each index switch for an index with nothing saved.

A test for the scenario above: layout pointing at an English copy whose key is null, with en.fwdictconfig's key holding a filter. It should restore no filter.

[thejambi]

@thejambi reviewed 8 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on mark-sil).

[thejambi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on mark-sil).