Summary
Eight items outside src: a script nothing runs that points at an executable name the project stopped producing, a docs page whose line citations have all drifted, a test polyfill that cannot fire, two hand-rolled reimplementations of things node ships (CRC-32 and a recursive readdir), a test that checks seven keys and then checks all of them again, a render harness written four times, and a dependency nothing imports.
Total estimated saving: about 350 lines, one script, one npm script, one dependency, and 24 stale docs citations and counts.
1. scripts/measure-windows-runtime.ps1 is unreferenced and points at the pre-rename executable
scripts/measure-windows-runtime.ps1:14
128 lines of PowerShell reachable only through npm run benchmark:win (package.json:24), which nothing in CI, the docs, the PR template or CONTRIBUTING mentions. Its default $AppPath is ..\dist\win-unpacked\VS Launcher.exe, a name electron-builder stopped producing when win.executableName became RiftLauncher, so the script throws unless the caller passes -AppPath by hand. It came in with the fork seed commit.
Replace with: nothing. Delete the script and the benchmark:win entry. If a startup or memory number is wanted again, the measurement it does (start the exe, wait for MainWindowHandle, sample the process tree twice) is a dozen lines written fresh against whatever is being measured.
Savings: 128 lines, 1 npm script, 1 file.
Risk and test: losing the process-tree walk used for the 0001 shell ADR comparison. That ADR records its numbers inline and measured them through /proc on Linux anyway, so the measurement it supports survives the script. The only two remaining references are comments (electron-builder.yml:47 and tests/config/electron-builder-icon-files.test.ts:20); that test asserts on the scripts/ exclusion line, which stays.
Worth: high.
2. docs/architecture.md pins line numbers and counts that have all drifted
docs/architecture.md:163 and throughout
The page carries 17 path.ts:NN-NN citations and 7 exact line counts, and measured against origin/dev almost every count is wrong: ListMods.tsx cited at 301 lines is 577, ConfigPage.tsx at 295 is 506, ManageMods.tsx at 261 is 465, ListInstallations.tsx at 257 is 277, ports.ts at 321 is 383, lzma.ts at 670 is 687, extract.ts at 345 is 374, preload/index.ts at 107 is 118. The CI section (:163) is the part that is flatly wrong rather than merely stale: it says five job definitions live in ci.yml when there are seven, never mentions test-matrix, calls build-macos "a sixth", and places the sonarcloud gate at line 49 when it is at line 78. The ci.yml:23-33 citation at :46, offered as where lint:ci is wired in, now points at a setup-node block.
Replace with: strip the :NN-NN suffixes and the raw line counts, keep the paths and the reasoning. A path grep survives any edit, a line number survives none. Rewrite the CI paragraph from the current ci.yml, or drop it and link the workflow.
Savings: 17 stale citations and 7 stale counts, and the page stops needing an edit every time an unrelated file grows.
Risk and test: a reader loses the jump-to-line convenience. Nothing tests these numbers, which is exactly why they drifted; the file paths they sit on are all still correct and are what a reader greps for anyway. The CI paragraph is the part that is actually wrong today, not merely imprecise, so rewrite that one rather than just trimming it.
Worth: medium.
3. The localStorage polyfill in the renderer-dom setup is unreachable
tests/renderer-dom/setup.ts:11
25 lines building a Map-backed Storage behind if (!window.localStorage). Under vitest's default jsdom url (http://localhost:3000, and nothing in this repo overrides environmentOptions.jsdom.url) jsdom 27 always exposes a real localStorage, so the branch is never entered. On the opaque origin the comment worries about, jsdom's own getter throws SecurityError on the property read inside the if, before the branch is evaluated, so the shim could not install itself there either. It came in with PR #415, whose body is entirely about scroll-position preservation and never mentions it.
Replace with: delete the block. The three lines below it that call window.localStorage.removeItem in beforeEach keep working on jsdom's own implementation.
Savings: 25 lines and one hand-rolled Storage implementation.
Risk and test: if someone later sets the jsdom url to about:blank, the removeItem calls throw instead of silently using the shim, and the renderer-dom suite fails loudly on the first test that touches storage, which is the OrderFilter sort-preference test. That is a better failure than a dead branch.
Worth: medium.
4. build-inno-fixtures.ts hand-rolls the CRC-32 its sibling imports from node:zlib
tests/fixtures/build-inno-fixtures.ts:43 (CRC_TABLE) and :53 (crc32)
A 256-entry reflected-IEEE table plus a crc32 function, 16 lines, for two call sites. tests/fixtures/build-fixtures.ts in the same directory already does import { crc32 } from "node:zlib". This is a fixture builder that already imports node:crypto, node:fs and node:child_process, so it is not under the purity constraint that legitimately justifies the hand-rolled CRC in src/domain/inno/crc32.ts; PR #88 documented at length why LZMA had to be hand-written and said nothing about CRC-32.
Replace with: import { crc32 } from "node:zlib", delete CRC_TABLE and the local function.
Savings: 16 lines, one hand-rolled stdlib algorithm.
Risk and test: the Inno block digests in the committed .bin fixtures would change if the two implementations disagreed. They do not: compared over 200 random buffers plus the empty buffer on Node 22, zero mismatches, and regenerating every fixture under tests/fixtures/inno/ produces byte-identical files to the committed ones, so no regeneration is even needed. The CRC guard tests are in tests/domain/inno/extract.test.ts (bad-block-crc.bin, wrong-digest.bin) and fail immediately if anything shifts.
Worth: medium.
5. copy-tone-locales.test.ts checks seven keys, then checks all of them again
tests/copy-tone-locales.test.ts:33
The first describe asserts seven named en-US keys contain no exclamation mark. The second walks every string value in en-US.json with an empty allowlist and asserts the same thing, so the seven are a strict subset and a regression in any of them fails the second block first. Both landed in the same PR (#446, closing #411), whose own body says the second commit "went past those seven and swept the other 40 en-US strings": the wide walk was meant to supersede the narrow one. Its collectStringPaths:53 also reimplements flattenTranslationObject from tests/i18n/helpers.ts (verified identical: both produce the same 855 pairs over the real en-US.json).
Replace with: delete the first describe and the KEYS list, import flattenTranslationObject instead of keeping a second flattener.
Savings: verified by applying it: 80 lines to 40.
Risk and test: losing the #411 trail on those seven keys, so move the issue reference into the surviving describe. The remaining test is strictly stronger and fails on the same strings. The "does a renamed key still exist" check is not lost either: tests/i18n/i18n-parity.test.ts already asserts, codebase wide, that every statically referenced key exists in en-US.json.
Worth: medium.
6. Two test files hand-roll the recursive readdir node ships and a sibling already uses
tests/i18n/helpers.ts:16 (listSourceFiles) and tests/action-label-locales.test.ts (tsxFiles)
One recurses with readdirSync plus statSync, the other with withFileTypes plus flatMap. tests/security-boundaries.test.ts:970 in the same suite already calls readdirSync(root, { recursive: true, encoding: "utf8" }) for the same purpose.
Replace with: readdirSync(dir, { recursive: true, encoding: "utf8" }).filter((f) => extensions.includes(extname(f))).map((f) => join(dir, f)), and delete tsxFiles in favour of the shared helper.
Savings: about 20 lines and one duplicated walker; tests/i18n/helpers.ts drops from 92 to about 80.
Risk and test: the recursive readdir returns paths relative to the root rather than absolute, so the join has to stay or every consumer breaks at readFileSync. Benchmarked against both existing walkers on the real tree (164 .ts/.tsx files, and 87 .tsx for the action-label case): byte-identical sorted lists. CI pins Node 22 on every platform including Windows, where security-boundaries.test.ts already consumes the same call. The check is tests/i18n/i18n-parity.test.ts's own floor assertion, which fails if the collector finds fewer than 50 t() call sites.
Worth: medium.
7. Four copies of the ManageMods render harness in the DOM tests
tests/renderer-dom/manageModsHealth.test.tsx:107, manageMods.test.tsx:191, manageModsDetails.test.tsx:105, manageModsServerMods.test.tsx:84, plus a fifth variant in manageModsUnreadableArchive.test.tsx
Each defines its own renderManageMods(overrides), and each builds the same <Routes><Route path="/installations/mods/:id" element={<TaskProvider><ManageMods /><NotificationsOverlay /></TaskProvider>} /></Routes> at the same route through renderWithProviders.
Replace with: one shared mount primitive in tests/renderer-dom/helpers, next to renderWithProviders which already owns the provider nesting, called by each file's own renderManageMods wrapper.
Savings: about 60 lines, the identical 13-line mount block plus five duplicated imports per file, four route and provider wrappers down to one.
Risk and test: do not try to absorb the whole of each renderManageMods. The five differ in more than the original write-up allowed: the netManager defaults differ substantially per file (full ModDB query simulation, a DETAILS-only stub, an empty listing, a 404), manageModsMissingInstallation.test.tsx renders at a different route with a config holding zero installations, manageModsServerMods returns the mocked bridge where others return void, and manageModsUnreadableArchive waits for and returns a specific row. Keep each file's own defaults, scenario setup and return type local; share only the mount. The four files themselves are the check.
Worth: medium.
8. @electron-toolkit/preload is declared and never imported
package.json:48
Zero imports anywhere in src, tests, scripts or config; the only occurrence of the string in the repo is its own devDependencies line. It is a different package from @electron-toolkit/utils (used in src/main/index.ts) and from the eslint-config and tsconfig packages, all of which are genuinely used. The preload is written against electron's contextBridge directly. Added once in the original electron-vite scaffold commit and never touched.
Replace with: remove the line and regenerate the lockfile (11 lines go with it, and nothing depends on it transitively).
Savings: 1 dependency, 2 lines.
Risk and test: none found. Verified: after removal, all three tsc projects, npm run build:unpack (main, preload, renderer, plus electron-builder packaging) and npm run lint pass with the same pre-existing warning count.
Worth: low.
Suggested order
- Items 1 and 8 (the dead script, the dead dependency): pure deletions, no test rewrites, land them together.
- Item 3 (the polyfill) and item 4 (the CRC table): both self-contained, both verified.
- Item 5 (
copy-tone-locales) and item 6 (the readdir walkers): both touch tests/i18n/helpers.ts's neighbourhood, so one pass.
- Item 7 (the ManageMods harness): the biggest test-side diff, so last among the tests.
- Item 2 (
architecture.md): independent of all the others, but do it after them so the rewritten paragraphs describe the tree as it is once the deletions land.
Out of scope
The rules the pinned tests carry are not what this touches. tests/security-boundaries.test.ts, tests/log-provenance.test.ts, tests/text-contrast.test.ts and tests/i18n/i18n-parity.test.ts keep enforcing log provenance, no HTML sinks, contrast floors and locale parity, and item 5 leans on the parity test rather than weakening it. The hand-rolled CRC-32 and LZMA inside src/domain/inno stay hand-rolled: they exist because that layer is pure, which is the hexagonal split (pure src/domain, src/ipc and src/main as host, the renderer through window.api and feature adapters) doing its job. The path policy, the IPC validation at the boundary, the mutation-tested guards and the accessibility work are deliberate and are not cut here.
Related: #107 tracks the first SonarCloud pass (5 bugs, 199 code smells) and stays the umbrella for Sonar's own list, including whatever duplicate blocks it flags in the test tree.
Summary
Eight items outside
src: a script nothing runs that points at an executable name the project stopped producing, a docs page whose line citations have all drifted, a test polyfill that cannot fire, two hand-rolled reimplementations of things node ships (CRC-32 and a recursive readdir), a test that checks seven keys and then checks all of them again, a render harness written four times, and a dependency nothing imports.Total estimated saving: about 350 lines, one script, one npm script, one dependency, and 24 stale docs citations and counts.
1.
scripts/measure-windows-runtime.ps1is unreferenced and points at the pre-rename executablescripts/measure-windows-runtime.ps1:14128 lines of PowerShell reachable only through
npm run benchmark:win(package.json:24), which nothing in CI, the docs, the PR template or CONTRIBUTING mentions. Its default$AppPathis..\dist\win-unpacked\VS Launcher.exe, a name electron-builder stopped producing whenwin.executableNamebecame RiftLauncher, so the script throws unless the caller passes-AppPathby hand. It came in with the fork seed commit.Replace with: nothing. Delete the script and the
benchmark:winentry. If a startup or memory number is wanted again, the measurement it does (start the exe, wait forMainWindowHandle, sample the process tree twice) is a dozen lines written fresh against whatever is being measured.Savings: 128 lines, 1 npm script, 1 file.
Risk and test: losing the process-tree walk used for the 0001 shell ADR comparison. That ADR records its numbers inline and measured them through
/procon Linux anyway, so the measurement it supports survives the script. The only two remaining references are comments (electron-builder.yml:47andtests/config/electron-builder-icon-files.test.ts:20); that test asserts on thescripts/exclusion line, which stays.Worth: high.
2.
docs/architecture.mdpins line numbers and counts that have all drifteddocs/architecture.md:163and throughoutThe page carries 17
path.ts:NN-NNcitations and 7 exact line counts, and measured againstorigin/devalmost every count is wrong:ListMods.tsxcited at 301 lines is 577,ConfigPage.tsxat 295 is 506,ManageMods.tsxat 261 is 465,ListInstallations.tsxat 257 is 277,ports.tsat 321 is 383,lzma.tsat 670 is 687,extract.tsat 345 is 374,preload/index.tsat 107 is 118. The CI section (:163) is the part that is flatly wrong rather than merely stale: it says five job definitions live inci.ymlwhen there are seven, never mentionstest-matrix, callsbuild-macos"a sixth", and places the sonarcloud gate at line 49 when it is at line 78. Theci.yml:23-33citation at:46, offered as wherelint:ciis wired in, now points at asetup-nodeblock.Replace with: strip the
:NN-NNsuffixes and the raw line counts, keep the paths and the reasoning. A path grep survives any edit, a line number survives none. Rewrite the CI paragraph from the currentci.yml, or drop it and link the workflow.Savings: 17 stale citations and 7 stale counts, and the page stops needing an edit every time an unrelated file grows.
Risk and test: a reader loses the jump-to-line convenience. Nothing tests these numbers, which is exactly why they drifted; the file paths they sit on are all still correct and are what a reader greps for anyway. The CI paragraph is the part that is actually wrong today, not merely imprecise, so rewrite that one rather than just trimming it.
Worth: medium.
3. The localStorage polyfill in the renderer-dom setup is unreachable
tests/renderer-dom/setup.ts:1125 lines building a Map-backed
Storagebehindif (!window.localStorage). Under vitest's default jsdom url (http://localhost:3000, and nothing in this repo overridesenvironmentOptions.jsdom.url) jsdom 27 always exposes a real localStorage, so the branch is never entered. On the opaque origin the comment worries about, jsdom's own getter throwsSecurityErroron the property read inside theif, before the branch is evaluated, so the shim could not install itself there either. It came in with PR #415, whose body is entirely about scroll-position preservation and never mentions it.Replace with: delete the block. The three lines below it that call
window.localStorage.removeIteminbeforeEachkeep working on jsdom's own implementation.Savings: 25 lines and one hand-rolled Storage implementation.
Risk and test: if someone later sets the jsdom url to
about:blank, theremoveItemcalls throw instead of silently using the shim, and the renderer-dom suite fails loudly on the first test that touches storage, which is the OrderFilter sort-preference test. That is a better failure than a dead branch.Worth: medium.
4.
build-inno-fixtures.tshand-rolls the CRC-32 its sibling imports fromnode:zlibtests/fixtures/build-inno-fixtures.ts:43(CRC_TABLE) and:53(crc32)A 256-entry reflected-IEEE table plus a
crc32function, 16 lines, for two call sites.tests/fixtures/build-fixtures.tsin the same directory already doesimport { crc32 } from "node:zlib". This is a fixture builder that already importsnode:crypto,node:fsandnode:child_process, so it is not under the purity constraint that legitimately justifies the hand-rolled CRC insrc/domain/inno/crc32.ts; PR #88 documented at length why LZMA had to be hand-written and said nothing about CRC-32.Replace with:
import { crc32 } from "node:zlib", deleteCRC_TABLEand the local function.Savings: 16 lines, one hand-rolled stdlib algorithm.
Risk and test: the Inno block digests in the committed
.binfixtures would change if the two implementations disagreed. They do not: compared over 200 random buffers plus the empty buffer on Node 22, zero mismatches, and regenerating every fixture undertests/fixtures/inno/produces byte-identical files to the committed ones, so no regeneration is even needed. The CRC guard tests are intests/domain/inno/extract.test.ts(bad-block-crc.bin,wrong-digest.bin) and fail immediately if anything shifts.Worth: medium.
5.
copy-tone-locales.test.tschecks seven keys, then checks all of them againtests/copy-tone-locales.test.ts:33The first
describeasserts seven named en-US keys contain no exclamation mark. The second walks every string value in en-US.json with an empty allowlist and asserts the same thing, so the seven are a strict subset and a regression in any of them fails the second block first. Both landed in the same PR (#446, closing #411), whose own body says the second commit "went past those seven and swept the other 40 en-US strings": the wide walk was meant to supersede the narrow one. ItscollectStringPaths:53also reimplementsflattenTranslationObjectfromtests/i18n/helpers.ts(verified identical: both produce the same 855 pairs over the real en-US.json).Replace with: delete the first
describeand theKEYSlist, importflattenTranslationObjectinstead of keeping a second flattener.Savings: verified by applying it: 80 lines to 40.
Risk and test: losing the #411 trail on those seven keys, so move the issue reference into the surviving
describe. The remaining test is strictly stronger and fails on the same strings. The "does a renamed key still exist" check is not lost either:tests/i18n/i18n-parity.test.tsalready asserts, codebase wide, that every statically referenced key exists in en-US.json.Worth: medium.
6. Two test files hand-roll the recursive readdir node ships and a sibling already uses
tests/i18n/helpers.ts:16(listSourceFiles) andtests/action-label-locales.test.ts(tsxFiles)One recurses with
readdirSyncplusstatSync, the other withwithFileTypesplusflatMap.tests/security-boundaries.test.ts:970in the same suite already callsreaddirSync(root, { recursive: true, encoding: "utf8" })for the same purpose.Replace with:
readdirSync(dir, { recursive: true, encoding: "utf8" }).filter((f) => extensions.includes(extname(f))).map((f) => join(dir, f)), and deletetsxFilesin favour of the shared helper.Savings: about 20 lines and one duplicated walker;
tests/i18n/helpers.tsdrops from 92 to about 80.Risk and test: the recursive readdir returns paths relative to the root rather than absolute, so the
joinhas to stay or every consumer breaks atreadFileSync. Benchmarked against both existing walkers on the real tree (164.ts/.tsxfiles, and 87.tsxfor the action-label case): byte-identical sorted lists. CI pins Node 22 on every platform including Windows, wheresecurity-boundaries.test.tsalready consumes the same call. The check istests/i18n/i18n-parity.test.ts's own floor assertion, which fails if the collector finds fewer than 50t()call sites.Worth: medium.
7. Four copies of the ManageMods render harness in the DOM tests
tests/renderer-dom/manageModsHealth.test.tsx:107,manageMods.test.tsx:191,manageModsDetails.test.tsx:105,manageModsServerMods.test.tsx:84, plus a fifth variant inmanageModsUnreadableArchive.test.tsxEach defines its own
renderManageMods(overrides), and each builds the same<Routes><Route path="/installations/mods/:id" element={<TaskProvider><ManageMods /><NotificationsOverlay /></TaskProvider>} /></Routes>at the same route throughrenderWithProviders.Replace with: one shared mount primitive in
tests/renderer-dom/helpers, next torenderWithProviderswhich already owns the provider nesting, called by each file's ownrenderManageModswrapper.Savings: about 60 lines, the identical 13-line mount block plus five duplicated imports per file, four route and provider wrappers down to one.
Risk and test: do not try to absorb the whole of each
renderManageMods. The five differ in more than the original write-up allowed: thenetManagerdefaults differ substantially per file (full ModDB query simulation, a DETAILS-only stub, an empty listing, a 404),manageModsMissingInstallation.test.tsxrenders at a different route with a config holding zero installations,manageModsServerModsreturns the mocked bridge where others return void, andmanageModsUnreadableArchivewaits for and returns a specific row. Keep each file's own defaults, scenario setup and return type local; share only the mount. The four files themselves are the check.Worth: medium.
8.
@electron-toolkit/preloadis declared and never importedpackage.json:48Zero imports anywhere in
src,tests,scriptsor config; the only occurrence of the string in the repo is its own devDependencies line. It is a different package from@electron-toolkit/utils(used insrc/main/index.ts) and from the eslint-config and tsconfig packages, all of which are genuinely used. The preload is written against electron'scontextBridgedirectly. Added once in the original electron-vite scaffold commit and never touched.Replace with: remove the line and regenerate the lockfile (11 lines go with it, and nothing depends on it transitively).
Savings: 1 dependency, 2 lines.
Risk and test: none found. Verified: after removal, all three tsc projects,
npm run build:unpack(main, preload, renderer, plus electron-builder packaging) andnpm run lintpass with the same pre-existing warning count.Worth: low.
Suggested order
copy-tone-locales) and item 6 (the readdir walkers): both touchtests/i18n/helpers.ts's neighbourhood, so one pass.architecture.md): independent of all the others, but do it after them so the rewritten paragraphs describe the tree as it is once the deletions land.Out of scope
The rules the pinned tests carry are not what this touches.
tests/security-boundaries.test.ts,tests/log-provenance.test.ts,tests/text-contrast.test.tsandtests/i18n/i18n-parity.test.tskeep enforcing log provenance, no HTML sinks, contrast floors and locale parity, and item 5 leans on the parity test rather than weakening it. The hand-rolled CRC-32 and LZMA insidesrc/domain/innostay hand-rolled: they exist because that layer is pure, which is the hexagonal split (puresrc/domain,src/ipcandsrc/mainas host, the renderer throughwindow.apiand feature adapters) doing its job. The path policy, the IPC validation at the boundary, the mutation-tested guards and the accessibility work are deliberate and are not cut here.Related: #107 tracks the first SonarCloud pass (5 bugs, 199 code smells) and stays the umbrella for Sonar's own list, including whatever duplicate blocks it flags in the test tree.