Remove the persisted sort-sequence cache that nothing could restore

[mark-sil]

Start here: the removed RestoreFrom in Src/xWorks/RecordList.cs (git show main:Src/xWorks/RecordList.cs). It read the cache file, deleted it and returned false; the rest of the branch is the write and purge machinery around that dead read.

Removes the on-disk sort-sequence cache (a file cache, not the LcmCache): the <clerkId>_SortSeq.fwss files every browse view (RecordBrowseView) wrote to the language project's Temp folder on close, the read attempt when a clerk was created, the serializer in Filters, and the four purges of that folder (discarded settings, Send/Receive, interesting-texts changes, every PropertyChanged broadcast reaching the reversal clerk).

Does behaviour change? No list has come back from this cache in any FieldWorks 9 release: the 2015 Db4o removal (ddf9648) deleted the lines that installed the restored list along with the version-stamp check it meant to remove, so the restore has returned false ever since. The only observable change is that nothing is written, parsed or deleted under <language project>\Temp. Worth your time: did any removed code have a side effect beyond the file system? Two candidates were checked (Evidence).

Where to look:

• RecordView.InitBase: the restored-list branch is gone; the load-if-needed line keeps its other two conditions.
• EntriesOrChildClassesRecordList: the removed override also set m_prevFlid; RecordBrowseView.Init overwrites it with the same value before anything can read it (Evidence).
• FwXWindow.DiscardProperties deleted the whole Temp folder, which Send/Receive also uses for a schema copy; its post-Send/Receive call ran after that copy's scope closed.
• InterestingTextList: the LT-13217 hack deleted related clerks' cache files; with no restore, those clerks reload regardless.
• Public surface removed: IManyOnePathSortItem.PersistData and the static serializer; no caller in the repo.

Deliberately not here: existing .fwss files in users' Temp folders (deleting the language project or restoring a backup removes them); liblcm's ksSortSequenceTempDir and *.fwss sweep in ProjectRestoreService; FwDeleteProjectDlg, which still deletes the Temp folder Send/Receive uses.

Off main, independent of #1179; it removes the one name-independent OnPropertyChanged side effect the PropertyChanged move to Pub/Sub would otherwise have to preserve. Build with both hygiene gates: clean. xWorksTests 1777 passed, 2 skipped, as on main; FiltersTests 24 passed (the two serializer tests went with the serializer). Not run: LexEdDllTests, xCoreTests, manual checks. By hand:

1. Open a language project; list <language project>\Temp.
2. Switch between two browse tools, then close FieldWorks.
3. Confirm no new _SortSeq.fwss file appeared.

Next: approve.

---

Reading this a year from now -- start here

No working documents were on this branch; the investigation ran in a side session and its notes live outside the repo. This body is the only written record of why the cache was dead and why it was removed rather than revived.

Surprising findings

• The break was an over-cut, not a decision. The 2015 commit's message mentions only Db4o and the -app switch. The surviving return false; kept the four-tab indentation of the deleted if (items == null) body, the method closed on the three-tab brace left from a 2012 block, the doc comment still promised to return true on success, and the parsed items were read into a local that nothing used. No commit since touched the method.
• The breakage never reached an 8.x release: it is absent from the FieldWorks8.3.13 tag and first appears in FieldWorks9.0.0-alpha1 (2018-01-09).
• The write side has run the whole time. The projects folder on the development machine held cache files for about 110 language projects, from 26 bytes to 1 MB (one lexicon's entries list, 17,133 lines), with writes dated the day before this branch.
• The purge wiped a folder another feature uses: FLExBridgeListener.CopyDictionaryConfigFileToTemp puts the dictionary-configuration schema in the same Temp folder for FLEx Bridge validation.

Decisions, and why

• Remove rather than revive. No release since 9.0 has had the behaviour, so revival is a new feature. Its live history (FWR-1110, FWR-1128, FWR-2890, LT-11169, LT-11240, LT-11395, LT-11446, LT-11647, LT-12870, LT-13217, LT-13348, LT-13428) is a list of staleness and corruption fixes, and its guards (a check that no object of the list's class was created this session, and GUID validity on read) never checked that the saved order was still current. Its only benefit, when it worked, was skipping the first sort of a large list at startup.
• Remove the XWindow.DiscardProperties hook. Its only override was the purge; a no-op virtual with no override is the same dead weight.
• Remove the serializer and its two tests. WriteItems, ReadItems, LazyManyOnePathSortItem and PersistData had no production caller left, and the tests exercised only them.
• Leave existing cache files alone. Deleting the language project removes them, restoring a backup removes them, and a startup sweep would add code to delete leftovers of code that no longer exists.
• Leave liblcm and FwDeleteProjectDlg unchanged. The constant is liblcm's; the backup-restore sweep is harmless; the Delete Project dialog's handling of Temp is still right because Send/Receive creates the folder.

Paths not taken

• Reviving the restore. It would need the 2015 tail back (null check, list assignment, pending-load reset, current-index restore, list-changed notification), new validation that the saved order is still current after edits made in other tools or merged from outside FieldWorks, and a test strategy for a path that had none. If first-sort time on very large language projects is a real cost today, that deserves its own measured ticket.
• A one-time sweep of stale .fwss files at startup. Rejected as above.
• Keeping the serializer for a future consumer. Nothing else in the repo persists sort items, and the file format carried only GUID paths.

Evidence

• Breaking commit: ddf9648. The diff removes string versionStamp, the out versionStamp argument, Cache.VersionStamp from the writer, and then from RestoreFrom the if (items == null) line, the versionStamp block, m_sortedObjects = items, m_requestedLoadWhileSuppressed = false, the CurrentIndex restore, SendPropChangedOnListChange and return true, leaving one return false.
• m_prevFlid side effect: the restore attempt ran inside RecordView.InitBase, before RecordBrowseView.Init triggers the bulk-edit bar's class change, and that change goes through ReloadList(int, int, bool), which sets m_prevFlid = m_flid itself before changing m_flid. Before that point GetNewCurrentIndex can only reach the m_prevFlid comparison when the obsolete current object's class differs from the list class, which cannot happen for a list that has not changed class.
• Post-Send/Receive purge: OnFLExBridge wraps the FLEx Bridge launch in using (CopyDictionaryConfigFileToTemp(projectFolder)) and calls RefreshCacheWindowAndAll after that block.
• XWindow.DiscardProperties overrides: FwXWindow is the only subclass of XWindow under Src.
• Remaining references: a UTF-16-aware scan of every .cs, .xml, .csproj, .resx, .axaml, .ps1, .cpp and .h under Src for the removed identifiers, fwss and _SortSeq finds only the two ksSortSequenceTempDir uses in FwDeleteProjectDlg, which stay.
• Test runs: test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject xWorksTests: 1779 total, 1777 passed, 2 skipped; -NoBuild -TestProject FiltersTests: 24 passed.

Preflight review details

Code Review Summary

Branch: claude/recursing-williams-5c8755
Base: main
Date: 2026-10-09
Review model: Claude Fable 5.1 (the four policy passes run by an independent read-only subagent; synthesis by the authoring session)
Files changed: 13

Overview

The branch removes the on-disk sort-sequence cache: the <clerkId>_SortSeq.fwss files that RecordBrowseView wrote into the language project's Temp folder on dispose, the read attempt in RecordView.InitBase when a clerk was created, the serializer in Filters, and the purges of that folder in FwXWindow, ReversalClerk.OnPropertyChanged, FLExBridgeListener and InterestingTextList. The restore has returned false unconditionally since the 2015 Db4o removal (ddf9648) cut the lines that installed the restored list, so no FieldWorks 9 release ever restored a list from it.

The analysis verified that claim against the 2015 diff, found no Critical or Important issues, and confirmed that the two candidate surviving side effects (the m_prevFlid assignment in EntriesOrChildClassesRecordList.RestoreFrom, and the creation and wholesale deletion of the Temp folder) leave no behavioural trace. Developer-only change: no Jira ticket needed. The only thing a tester could observe is the absence of new .fwss files in a language project's Temp folder.

Contract/API Changes

• Filters (public): IManyOnePathSortItem.PersistData removed from the interface; ManyOnePathSortItem.PersistData, WriteItems, ReadItems, the internal LazyManyOnePathSortItem and InvalidObjectGuidException removed.
• xCore: XWindow.DiscardProperties (protected virtual) removed. Its only override was FwXWindow; its only callers were RestoreWindowSettings (Shift at startup, or the safe-mode prompt answered Yes) and FwXWindow.ClearInvalidatedStoredData, whose callers in FLExBridgeListener.RefreshCacheWindowAndAll and ReversalClerk.OnPropertyChanged are removed too.
• xWorks: RecordView.PersistSortSequence (protected virtual), the RecordBrowseView.OnParentChanged override, and the internal RecordClerk.PersistListOn, RecordClerk.RestoreListFrom, ListUpdateHelper.ListWasRestored, RecordList.PersistOn, RecordList.RestoreFrom and RecordView.GetSortFilePersistPathname.
• File footprint: <language project>\Temp\<clerkId>_SortSeq.fwss is no longer written, read or deleted; <language project>\Temp is no longer created on clerk creation nor deleted wholesale.
• None of Filters, xCore or xWorks is NuGet-packaged, and FLEx Bridge and liblcm do not reference them. No plausible external consumer.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• Src/xWorks/InterestingTextList.cs: the removed InvalidateRelatedSortSequences also carried Debug.Fail("We may need to add a new RelatedClerkId."), which fired in Debug builds when the texts changed while a clerk outside the four listed ids was active. Release behaviour unchanged; the commit message does not mention dropping the assertion. (The assertion guarded the clerk-id table that existed only to choose which cache files to delete. With the table gone there is nothing left to assert, so it goes with the mechanism.)
• Src/FwCoreDlgs/FwDeleteProjectDlg.cs:303,334: still uses liblcm's LcmFileHelper.ksSortSequenceTempDir, whose name is now misleading. Harmless: it deletes or ignores the folder and tolerates its absence. (The constant is liblcm's and still names the language project's Temp folder, which Send/Receive uses for its schema copy. Renaming it and dropping liblcm's ProjectRestoreService sweep of *.fwss is a liblcm follow-up, not this PR.)

Required Validation / Evidence

• .\build.ps1 -CommentHygiene -TokenHygiene -StartedBy agent: build succeeded, 0 warnings, 0 errors, both hygiene gates clean.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -StartedBy agent -TestProject xWorksTests: 1779 total, 1777 passed, 2 skipped, the same counts as main. Covers BulkEditBarTests, RecordListTests and InterestingTextsTests.
• .\test.ps1 -CommentHygiene -TokenHygiene -SkipNative -NoBuild -StartedBy agent -TestProject FiltersTests: 24 passed. The two removed tests covered only the serializer round trip and bad-GUID rejection.
• Manual smoke (open a browse tool, switch tools, close FieldWorks, confirm no new Temp\*_SortSeq.fwss): not performed.
• LexEdDllTests and xCoreTests: not run. The reviewer found nothing in them that reaches the removed paths; XWorksAppTestBase calls RestoreWindowSettings(false), which never reached the removed branch.

Positive Observations

• The central claim holds: git show ddf964846 -- Src/xWorks/RecordList.cs replaced the m_sortedObjects = items ... return true tail of RestoreFrom with return false; the only override returned base, so RestoreListFrom never returned true and ListWasRestored was unreachable. The InitBase simplification is exact.
• m_prevFlid: the removed assignment ran before RecordBrowseView.Init triggers OnChangeListItemsClass, which assigns m_prevFlid = m_flid itself before changing m_flid; while the class is unchanged the only reader, GetNewCurrentIndex, returns -1 either way.
• Temp folder: the only other writer, FLExBridgeListener.CopyDictionaryConfigFileToTemp, creates the folder itself and removes its file through TempFile. Dropping RobustIO.DeleteDirectoryAndContents(Temp) on every reversal-clerk PropertyChanged is strictly safer for that copy.
• No remaining reference to any removed member under Src (including the six UTF-16 files), DistFiles, Docs or Build. GlobalSettingServices.PersistData is an unrelated HomographConfiguration member.
• The removed OnParentChanged override reached PersistSortSequence through the Clerk getter, which creates a clerk when none exists; its removal closes a clerk-creation path during teardown.
• The new InterestingTextList.Cache doc comment is accurate: assigned only in GetCache() from the property table, read only by PropChanged's Virtuals.LangProjTexts check.
• ManyOnePathSortItem construction, key and path semantics stay covered by TestManyOneBrowse, FindResultsSorterTests, OccurrenceInContextFinderTests, RecordListTests, WordsUsedOnlyElsewhereFilterTests, ItemClickedTests and InterlinearTextRecordClerkTests.
• Commit message within the gitlint limits (title 67 characters, body lines under 72).

Interview Notes

• Remove rather than revive: decided after the history write-up. No release since 9.0 has had the behaviour; its live history is a list of staleness and corruption fixes; its guards never checked that the saved order was still current; its only benefit, when it worked, was skipping the first sort of a large list at startup.
• "Is this just dead code?": the restore half is strictly dead; the write, parse and purge sides were live but useless. The one side effect with a possible functional trace (m_prevFlid) was traced through RecordBrowseView.Init and found masked; the post-Send/Receive purge ran after the schema copy's scope had closed.
• Scope addition: the first build failed on InterestingTextList, which the initial reference sweep had hidden behind its own exclusion filter. Its LT-13217 invalidation of related clerks' cache files is the same dead mechanism and was removed; a UTF-16-aware re-sweep found nothing else.
• Deliberate non-goals: existing .fwss files in users' Temp folders are left in place (no startup sweep); liblcm's constant and *.fwss restore sweep are untouched; FwDeleteProjectDlg is untouched. The XWindow.DiscardProperties hook was removed with its only override.
• Jira: developer-only, no ticket needed.
• Unresolved items: none. No lack-of-understanding notes.

In-Review Quality Check

No in-review changes.

Suggested Review Focus

• RecordView.InitBase: the restored-list branch is gone and the load-if-needed condition lost only the restored flag.
• EntriesOrChildClassesRecordList: removal of the override that set m_prevFlid.
• FwXWindow and XWindow: removal of DiscardProperties and ClearInvalidatedStoredData, and the Temp folder's remaining user.
• IManyOnePathSortItem.PersistData: public interface member removed.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   10m 34s ⏱️ - 3m 6s
6 597 tests  - 2  6 512 ✅  - 2  85 💤 ±0  0 ❌ ±0 
6 606 runs   - 2  6 521 ✅  - 2  85 💤 ±0  0 ❌ ±0 

Results for commit b49d9b9. ± Comparison against base commit bd0608d.

This pull request removes 2 tests.

SIL.FieldWorks.Filters.ManyOnePathSortItemsPersistenceTests ‑ PersistMopsiList
SIL.FieldWorks.Filters.ManyOnePathSortItemsPersistenceTests ‑ PersistMopsiList_BadGUID

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 39.50%. Comparing base (bd0608d) to head (b49d9b9).

Files with missing lines	Patch %	Lines
Src/xWorks/RecordView.cs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1180      +/-   ##
==========================================
- Coverage   39.51%   39.50%   -0.01%     
==========================================
  Files        1537     1537              
  Lines      354706   354424     -282     
  Branches    41096    41062      -34     
==========================================
- Hits       140160   140015     -145     
+ Misses     185154   185033     -121     
+ Partials    29392    29376      -16     

Files with missing lines	Coverage Δ
Src/Common/Filters/ManyOnePathSortItem.cs	51.31% <ø> (-24.07%)	⬇️
Src/LexText/Lexicon/FLExBridgeListener.cs	4.42% <ø> (+<0.01%)	⬆️
Src/LexText/Lexicon/ReversalListener.cs	22.61% <ø> (+0.28%)	⬆️
Src/XCore/xWindow.cs	36.03% <ø> (+0.21%)	⬆️
Src/xWorks/FwXWindow.cs	11.78% <ø> (+0.07%)	⬆️
Src/xWorks/InterestingTextList.cs	56.49% <ø> (-1.95%)	⬇️
Src/xWorks/RecordBrowseView.cs	52.74% <ø> (-0.19%)	⬇️
Src/xWorks/RecordClerk.cs	44.89% <ø> (+0.31%)	⬆️
Src/xWorks/RecordList.cs	53.63% <ø> (+1.46%)	⬆️
Src/xWorks/RecordView.cs	72.17% <0.00%> (+6.14%)	⬆️

... and 1 file 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.