Repository navigation
feat: scope settings and state across workspace, device, and URL layers - #391
Merged
Merged
Conversation
Settings and state were split between one shared server store and ad-hoc localStorage, with no principle behind the split. Switching profiles on a phone switched them on the desktop, column widths followed users between screen sizes, and browsing position was persisted as if it were a setting. Introduce three explicit scopes, decided by one question — "would copying this to another device be wrong?": - Workspace (server): profile list, default profile, plugin registrations, saved searches, Browser/Recently Added filter tag structure, and which columns the song table shows. - Device (localStorage, behind a new DeviceSettingsRepository port): the selected profile, column widths, the common table's sort order, locale, theme, pane split ratios, and Advanced Search settings. - View (URL): Browser and Recently Added selections, so back/forward, duplicated tabs, and shared links all work. Selections serializing past 2,000 characters fall back to a server-side blob referenced by a short content-hash token, swept after 90 days. Sort order is the one field whose scope depends on context: it is Device for the common song table, but part of the saved search itself for Search, since a saved search must return songs in the same order on every device. Supporting changes: - Persistence gains a versioned migration chain applied to raw JSON before deserialization, atomic temp-file writes, and a one-time backup of db/ taken before the first write. - MCP tools accept an optional profile argument and fall back to the workspace default; MpdProfileState.current_profile now means "the default" rather than "the current selection". - Config saves broadcast to other connected clients, which refetch the affected document, so deliberate shared changes propagate live. - Settings is reorganised into Profiles / Library / This device / Raw Data, with each section stating whether it is shared or local. The only proto change is an additive schema_version field on six top-level messages. No existing field is renamed, removed, or retyped, so persisted user state is preserved: the fields whose ownership moved to the device layer are simply no longer written, and existing values are migrated into device settings once on first run.
"Library" read as the music library, and neither it nor "This device" said what belonged inside. Rename the two tabs to Shared Settings and Device Settings so the pair reads as the scope contrast it is, and give each one a sentence saying what that scope means. The slugs move with them (library -> shared, this-device -> device); both were introduced in the preceding commit and never released, so no existing URL breaks. State the scope once per tab rather than once per section. Repeating it placed the note above the heading of every section that renders its own title, which is how Locale ended up labelled before it was named. A profile can be both in use on this device and the workspace default. The two badges sat in a row and clipped each other, so stack them, shorten them to "This device" and "Default", and move the full meaning into a tooltip. Note on the Advanced Search section that only the query limit is device- local: Scan, Vacuum, and Analyze act on the shared library index, so they affect every device. Finally, drop the comments' references to the design document behind this work, which is not part of the repository — each comment now carries its reasoning on its own.
The existing rule said comments go where the why is non-obvious, which did not stop justifications of the shape chosen over alternatives, restatements of a signature, or notes on which bug a change fixed. Name those three.
Selections were written with replace, so choosing a genre overwrote the current history entry instead of adding one — there was nothing to go back to. They now push, and an update that leaves the query unchanged is dropped so the filter panels re-emitting on every interaction does not bury Back under duplicate entries. The rest was self-inflicted. A device-local cache of the last position meant an empty query had two meanings, "nothing selected" and "we have not been told yet", which the URL cannot distinguish. Every attempt to tell them apart guessed at history direction and got it wrong: pressing Back out of a selection restored the position just left and redirected forward to it. Drop the cache. Opening a browsing page from the side navigation now starts with nothing selected; reload, Back, Forward, bookmarks and shared links all still carry the position because it is in the URL. That removes the per-profile device setting entirely, and with it the sweep that cleaned those keys up when a profile was deleted. What is left needs no history-awareness: an inline selection is derived from the URL, and a ?vs= token is fetched whenever the query changes.
…very other table A saved search's columns had two homes: the form values and the atom the song table reads. Opening a saved search filled the form but not the atom, so the table fell back to the common song table's columns and the sort stored with the search — the half of it that is shared across devices — was never applied. It also meant one search's columns lingered when another was opened. Drop the field from the form values, leaving the atom as the only holder, and load it when a search is opened. That exposed a second gap. AG Grid reads a column definition's sort only when it creates the column and keeps its own state afterwards, so columns arriving from elsewhere left whatever the user had last sorted by in place, mixed with the incoming sort. Push the columns' sort into the grid whenever the two differ — only then, or the sort the user makes would be re-applied to itself. In the other direction, a width drag in Search wrote the whole layout entry for each column, sort included, into the device layout. That layout is where the common song table reads its sort from, so sorting a saved search quietly re-sorted Browser and everything else. Add applyColumnWidthsToLayout, which overlays widths and leaves the recorded sort alone, and use it on the width-only path — width is Device-owned everywhere, sort only for the common table.
The list was handed an empty selection unconditionally, so nothing ever appeared selected, and the sync that mirrors selectedValues onto the grid cleared the row the user had just clicked. Derive the selection from the name in the query editor rather than storing which search is open. One write site — the editor, which already tracks the name for the saved/unsaved indicator — covers opening a search, renaming out of one, saving under a new name, and deleting the open one. Holding it separately would need all four kept in step, which is the shape of bug this branch has already fixed once.
Biome has no rule for this and its GritQL plugins cannot see trivia, so the check walks the TypeScript CST. It takes paths the way biome does, and pnpm lint hands it the same ones, so the two never drift apart on what they cover. Counts prose only: delimiters, blank lines and everything from the first JSDoc tag on do not count, since @PARAM and friends document an API rather than explain code. A warning, not a gate.
Narration, restatements of the function name, justifications of the shape chosen over alternatives, and archaeology about a refactor's blast radius. One comment described a sweep that no longer exists.
Section labels naming the variable declared under them, sentences paraphrasing the statement they precede, and JSDoc summaries repeating the function name. Three were also wrong: PlayerSeekBar named onChange when the slider is wired to onChangeEnd, convertConditionToString's operator list omitted ADDED_SINCE, and the migration table claimed "Phase 1 introduces no real migrations yet". Barrel index.ts grouping labels are left alone.
The renderer subscribes to socketio_config_changed on every transport, but the preload allowlist rejects an unlisted channel by throwing. On Electron that rejected the dependency-injection promise the app suspends on, so the window opened and rendered nothing at all. Also allow the two view state blob channels. The main process has handled them since they were introduced, but the preload closed them off, which would have surfaced only once a selection grew past the inline URL limit.
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
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.
Settings and state were split between one shared server store and ad-hoc
localStorage, with no principle behind the split. Switching profiles on a phone switched them on the desktop, column widths followed users between screen sizes, and browsing position was persisted as if it were a setting.This introduces three explicit scopes, each decided by one question: "would copying this to another device be wrong?"
db/*.jsonlocalStorageSort order is the one field whose scope depends on context: Device for the common song table, but part of the saved search itself for Search, since a saved search must return songs in the same order on every device.
Scoping
DeviceSettingsRepository, a port overlocalStorage, so device-scoped values have one typed home instead of scattered direct access.MpdProfileState.current_profilenow means "the workspace default" rather than "the current selection".updateSongTableStateActionAtom: tag list and order go to the server, sort and width to the device.?bsel=,?rasel=) so reload, Back/Forward, duplicated tabs, bookmarks, and shared links all carry the position.?vs=) when a selection would serialize past 2,000 characters, swept after 90 days.Persistence and sync
rename, so an interrupted write cannot truncate a document.db/once before the first write of a run.schema_versionto six top-level protobuf messages. Additive only — no field is renamed, removed, or retyped, so existing user state is preserved.MCP
Settings UI
Fixes
Refactoring
lib/into the features that own them, and place every new file where the folder contract says it belongs.features/commona barrel, like every other feature.Tooling and docs
CLAUDE.mdwhat not to write in a comment: narration, justifications of the shape chosen over alternatives, and archaeology.pnpm lint_comments, which walks the TypeScript CST and warns on comments longer than four lines of prose. It takes paths the waybiomedoes andpnpm linthands it the same ones, so the two never drift apart on coverage.Verification
pnpm fmt && pnpm lint && pnpm test && pnpm buildall pass. 731 tests (shared 50, subsonic 13, backend 210, frontend 458).🤖 Generated with Claude Code