Stop hand-rolling in the domain what the repo already imports

[Pixnop]

The four findings of #485, in the order the issue suggests. Refactor only: no behaviour change intended anywhere, and no test was relaxed to make a change pass.

What changes

One executable table in the versions domain. src/domain/versions/gameExecutable.ts goes from 77 lines to 62. expectedGameExecutables(os) returned the file names that gameExecutableCandidates(os) then mapped to launch modes, so one table was spelled twice and a test existed only to assert the two agreed with each other. The candidate list is the table now, and gameLanded in src/domain/versions/install.ts reads .fileName off it, which was the only caller outside the pair. tests/domain/versions/gameExecutable.test.ts goes from 64 lines to 35. docs/architecture.md and docs/vintage-story-quirks.md both named the deleted function and were updated.

One definition of an active mod filter. In src/domain/mods/installedFilters.ts, hasActiveInstalledModFilters restated the three-axis OR that countActiveInstalledModFilters sums right below it. It asks the count now (> 0), so a fourth axis cannot land in one and not the other. Two lines, one duplicated definition.

The What's new caps read straight from the module. WhatsNewLimits and SelectReleasesOptions were filled by the test file alone: all three production call sites, in src/renderer/src/features/info/hooks/useWhatsNew.ts at lines 55, 99 and 142, passed no options at all. Both interfaces are gone, with the three parameters that took them. DEFAULT_WHATS_NEW_LIMITS went with them: nothing outside the module imported it, and with no override left the bag was two numbers, so it is MAX_BLOCKS and MAX_BLOCK_LENGTH as plain module constants. capBlockText lost its length parameter for the same reason, one caller and one value. About 15 lines.

semver orders the releases. parseWhatsNewVersion, compareIdentifiers and compareWhatsNewVersions spelled out semver precedence over 65 lines: dotted release parts, prerelease split, numeric identifiers sorting as numbers, numeric before alphanumeric, a plain release outranking its own prerelease. semver is a production dependency that src/domain/versions/detect.ts, src/domain/mods/compatibility.ts and src/domain/mods/health.ts already read versions with in this same layer, so the three functions are one 8-line comparator: semver.compare behind a semver.valid guard, the shape src/renderer/src/utils/gameVersionOrder.ts ships for the same problem. whatsNew.ts goes from 402 lines to 359.

Reviewer's call on that last one, and the reason it is listed last here too: PR #442 recorded a deliberate decision to hand-roll this rather than take the dependency, comparing it to betaUpdates.ts reading a prerelease with a string check. That decision is recent. If it still stands, drop the last commit (Order What's new releases with semver) and the other three are independent of it.

What stays

Nothing was skipped outright, but two halves of the issue's own suggestions were not taken.

The issue proposes moving compareGameVersionsDesc out of the renderer and into the domain so both call sites share one comparator. Not done: the two want opposite directions, and deriving one from the other by swapping the arguments also swaps the both-unparseable tie-break, which gameVersionOrder.ts documents as alphabetical so the game version list renders the same way every time. Negating that is a behaviour change in the renderer for no line saved. The two comparators now have the same shape and the domain one says where the other lives.

The issue's own "not recommended" note on finding 4, un-exporting matchesInstalledModFilters, is not done either, for the reason it gives: routing its twelve direct test call sites through filterInstalledMods costs more code than it saves.

Untouched, as the issue's out-of-scope line requires: the hexagonal split, src/ipc/pathPolicy.ts, the IPC validation at the boundary, the mutation-tested guards and the accessibility wiring. tests/security-boundaries.test.ts, tests/log-provenance.test.ts, tests/text-contrast.test.ts and tests/i18n/i18n-parity.test.ts are unchanged and pass.

One follow-up for someone else: #457 cites expectedGameExecutables by name and by line range as existing infrastructure for its planned feature. It wants gameExecutableCandidates(os).map((candidate) => candidate.fileName) now, or the candidates directly.

Behaviour

Every caller was grepped before each cut. expectedGameExecutables had one caller outside its own pair (install.ts:101) and two docs references. WhatsNewLimits, SelectReleasesOptions, maxReleases and DEFAULT_WHATS_NEW_LIMITS had no production reader left after the parameters went, which grep over src, tests and docs confirms returns nothing.

Five tests were deleted, all in tests/domain/versions/gameExecutable.test.ts and all because the function they tested is gone:

• "expects the Windows executable and nothing else on Windows", "accepts either the native launcher or the mono executable on Linux" and "has no expectation on macOS, which the launcher cannot run yet" assert the same three file-name lists the surviving gameExecutableCandidates cases assert, which are exact deepEqual on the whole list.
• "prefers the native Linux launcher over the mono fallback" is the order assertion; the name moved onto the candidates case, whose deepEqual pins the same order.
• "names the exact same files and order as expectedGameExecutables" had nothing left to compare: there is one table.

No other test was touched to make a change pass. The seven whatsNew assertions that used the option overrides now reach the same branches through the real caps: a body over 2000 characters for the word-boundary cut, 3000 x with no space to back up to, 2000 emoji for the surrogate-pair cut, and 200 bullets for the block cap. The block cap gained its exact-fit boundary case (120 blocks, no ellipsis block), which the cheap override never reached. The two cases that tested maxReleases itself went away with it.

On the semver swap: semver.valid refuses a tag the old parser coerced through Number(part) || 0, and that is stricter rather than the same ranking. An earlier revision of this section claimed the two put a refused tag in the same place. They do not, and the correction is below.

Every tag this project has published parses once stripVersionPrefix has run, so no published release moves: running both comparators over all 41 published tags, every ordered pair agrees. What moves is a tag semver refuses that the old parse could still read a number out of. 1.8 ranked between 1.9.0 and 1.7.0 and now sorts below both. 2024-06-01 ranked above every 1.x tag and now sorts below them too. A tag opening with a word, nightly and the like, coerced to 0 and already sorted last, so those are unchanged, and two of them against each other are alphabetical now rather than the old dead heat.

That fallback is reachable on data fetched from GitHub, because nothing upstream constrains the shape of a tag: src/ipc/handlers/netHandlers.ts:281 keeps any non-empty tag_name of up to 128 characters. It is pinned by two tests now, listed in Check below. tests/domain/appUpdate/whatsNew.test.ts:356 (beta.10 after beta.9, not equal to it) and :364 (a release outranking its own prerelease) pass unchanged, as do all tests under tests/domain/appUpdate/.

Gate, against origin/dev at the same commit:

	dev	here
typecheck	pass	pass
lint:ci	0 errors, 14 warnings	0 errors, 14 warnings, none new
format:check	pass	pass
test files	242 passed	242 passed
tests	4461 passed, 2 skipped	4458 passed, 2 skipped (the five above, less the two added in Check)
statements	94.58% (8866/9374)	94.60% (8836/9340)
branches	91.02% (4952/5440)	91.08% (4906/5386)
functions	95.22% (1876/1970)	95.21% (1871/1965)
lines	96.26% (7471/7761)	96.25% (7447/7737)

Every denominator shrank, which is the point. git merge-tree against all nine open branches reports no conflict, #514 included, which is the only one that also touches src/domain/versions/install.ts.

Check

One finding from review, applied in Pin where a tag semver refuses lands in the What's new order.

The semver fallback reorders a tag semver refuses, the opposite of what this body claimed, and nothing tested it. Correct on both counts, and the claim was the worse half: the coerced parse read the leading run of a tag as a number, so it ranked 1.8 among the tags semver accepts rather than below them. Sorting v1.9.0, v1.8 and v1.7.0 on dev lists all three; before this fix it dropped v1.8 out of the list, and out of the dialog's window, with no test to notice.

• The claim is corrected in the Behaviour section above and in the message of the commit that made the change, which is reworded to say the ranking is stricter than the coerced parse and to name the two shapes that move.
• src/domain/appUpdate/whatsNew.ts:286 says the same thing at the comparator, which described what it does but not what it stopped doing, and records that netHandlers.ts leaves the tag shape unconstrained.
• tests/domain/appUpdate/whatsNew.test.ts:390 pins the exclusion: a release tagged v1.8 stays outside the window between 1.7.0 and 1.9.0 instead of falling inside it.
• tests/domain/appUpdate/whatsNew.test.ts:446 pins the order among refused tags, asserted from both input orders so it holds whichever way GitHub answers. Letters only, no punctuation whose collation could differ between the Linux and Windows runners.

The ranking itself stands rather than being restored. semver.coerce was the obvious way to take the old lenience back and it is far too greedy to use here: it reads untagged-abc123 as 123.0.0 and build 42 as 42.0.0, which would rank a junk tag above every real release and pull it into the dialog. The old parse put those at 0 and so does the guard. Between guessing and refusing, refusing is the one that cannot show the wrong release notes.

src/domain/appUpdate/whatsNew.ts is back to 100% statements and 100% lines, from 97.39% lines on the previous revision, where the three uncovered lines in the file were exactly this fallback.

Closes #485. Part of #492.