Enforce naming, size and separation — and split diffStore - #23
Merged
Conversation
…Store Adopts clean-code-javascript + Node Best Practices as cited references and turns on the parts of them the build can hold. Zero new dependencies: every rule is core ESLint, and the one thing core cannot do (cycle detection) is a script in the existing scripts/check-*.mjs idiom. Enforcement - naming: camelcase, new-cap (capIsNew off — 194 SCREAMING_CASE test factories are not constructors), no-underscore-dangle scoped to src/ - size: max-lines-per-function 60 + max-lines 250 on src/**/*.js, matching the cap .vue already had, with scripts/lib/legacySize.mjs holding one EXACT measurement per over-cap file. One added line fails; beat a cap and the entry must be deleted. --retighten can only lower a number. - lint now fails on warnings (--max-warnings 0), which promotes four vue/*-casing rules that previously exited 0 - scripts/check-structure.mjs: import cycles against a baseline, plus stale legacy entries. A cycle that DISAPPEARS also fails, so removing one is proven rather than claimed. - npm run build joins npm run check Structure - diffStore 1509 -> 796 lines, 54 -> 27 state keys, 93 -> 44 actions - four vertical slices under features/, each reachable only through its index.js: share, pasteToCompare, imageExport, configBackup - utils/commands.js is the single registry every menu, shortcut, palette entry, shelf chip and rail button dispatches through - uiStore for dialog chrome; theme folded into settingsStore - diffStore -> tabsStore cycle removed (5 -> 4 baselined) Tests 1976 -> 2015, none deleted; the eight diffStore*.test.js files followed their subjects into tests/renderer/features/. Eight defects were caught by the new guards during their own construction, three of which were checks that looked enforced while matching nothing: featureStyleDirs resolved the repo root one level short (slice CSS left the token guard silently), a later config block dropped the renderer/main fence for every store, and ESLint's import matcher does not support extglob so the slice-privacy pattern matched nothing. npm run build was also broken while 2015 tests passed — no unit test mounts App.vue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of #23 found the unlink fix applied to one of two clear() call sites. _loadReplacement also clears, and dropFiles routes to it PRECISELY when diffSaved is true — no prompt, because it is saved. Dropping files onto a saved diff left the tab claiming an entry it no longer held, so that saved diff could not be reopened. 2015 unit tests and 309 e2e tests passed over it. Subscribed to the action instead of calling it, so every path gets the unlink and no future caller can forget; the core still cannot reach tabsStore. Also from the review: - the slice-privacy fence matched NOTHING between slices. From outside, the specifier carries `features/`; from inside, a sibling is `../imageExport/imageExportStore`, which does not — and `../*/*` cannot stand in because `*` also matches `..` and would ban the core. Siblings are now named, read from disk so a new slice is covered on creation. 11 cases pinned in eslintFence.test.js. - measure() counted a trailing newline as a line, so the four caps --retighten wrote carried a free one. Every file cap now equals ESLint's own count. - commands.test.js faked a pre-split bundle: no share/imageExport/ configBackup, seven dead diff.* fakes, 5 of 42 handlers ever invoked, and a vacuous guard assertion. Rebuilt to run every action and every CLI command against a live-shaped bundle; verified by re-pointing share-current at the core and watching it fail. Swept six orphaned comments, a typedef path left one level short by the move, the commands.js line in the slice contract (no slice has one), and an empty leftover styles directory. Tests 2015 -> 2027. QA found no regressions: 309/309 e2e (307 Docker + 2 macOS-gated natively), all 42 registry actions and all 96 menu items driven with zero renderer errors. Co-Authored-By: Claude Opus 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
1 participant
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.
Adopts clean-code-javascript and Node.js Best Practices as cited references and turns on the parts of them the build can hold — then applies them to the largest violation in the tree.
Spec:
specs/2026-08-03-code-standards-enforcement/plan.mdZero new dependencies. Every rule is core ESLint; the one thing core cannot do (cycle detection) is a script in the existing
scripts/check-*.mjsidiom.Enforcement
camelcase·new-cap(capIsNew: false— 194 SCREAMING_CASE test factories are not constructors) ·no-underscore-danglescoped tosrc/max-lines-per-function60 +max-lines250 onsrc/**/*.js, matching the cap.vuealready had--max-warnings 0, which promotes fourvue/*-casingrules that previously exited 0scripts/check-structure.mjsagainststructure-baseline.jsonnpm run buildjoinsnpm run checkscripts/lib/legacySize.mjsholds one exact measurement per over-cap file — permission for what exists and not one line more. Beat a cap and the entry must be deleted (check-structure.mjsfails on a stale one);--retightencan only lower a number, never raise one. A baselined cycle that disappears also fails, so removing one is proven rather than claimed.Structure
diffStore1509 → 796 lines, 54 → 27 state keys, 93 → 44 actionsfeatures/, each reachable only through itsindex.js:share212,imageExport236,pasteToCompare115,configBackup64 — all under the 250 caputils/commands.js— the single registry every menu, shortcut, palette entry, shelf chip and rail button dispatches throughuiStorefor dialog chrome; theme folded intosettingsStorediffStore → tabsStorecycle removed (5 → 4 baselined)Tests 1976 → 2015, none deleted — the eight
diffStore*.test.jsfiles followed their subjects intotests/renderer/features/.Eight defects caught by the new guards during their own construction
Three were checks that looked enforced while matching nothing:
featureStyleDirs()resolved the repo root one level short — slice CSS left the token guard silently (stylesheets fell 93 → 92)no-restricted-importsis replaced, not merged by a later block — the layering rule forstores/**dropped the renderer/main fence (hard rule 3) for every store!(index)*matched nothing, so slice privacy was decorationPlus:
npm run buildwas broken while 2015 tests passed (no unit test mountsApp.vue); theclearcommand's guard silently stopped unlinking a saved diff's tab; nine commands pointed at the core for members that had moved.Each is guarded now, and each guard was verified red→green.
Known gaps
diffStoreis 796, not the 700 target — deliberate. The rest is the save→replace→pick gate and file-load flow, the core's own guard on its own document. Extracting them would produce a store that is mostly calls home.make e2enot run (needs the Docker container). No e2e spec was edited and none references a moved member — they drive the app through its UI. Worth a run before merge.🤖 Generated with Claude Code