Skip to content

Preserve Library view state and playback queues - #33

Merged
rianjs merged 28 commits into
ui-tweaksfrom
feature/library-view-state
Aug 16, 2026
Merged

Preserve Library view state and playback queues#33
rianjs merged 28 commits into
ui-tweaksfrom
feature/library-view-state

Conversation

@rianjs

@rianjs rianjs commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • preserve each Library source's drilled-down view and add playlist Back navigation
  • give playlists independent persisted column layouts with the approved defaults
  • remove genre override dots and label missing Music genres as muted No Genre
  • keep active playback queues independent from UI navigation

This PR intentionally includes the preceding Last.fm and diagnostics stack already on this branch line.

Validation

  • documentation checks
  • frontend tests, lint, TypeScript, and production build
  • Rust formatting, Clippy, and workspace tests
  • native release build and install
  • native navigation state checks for Music, Podcasts, and playlists

Closes #32

@rianjs

rianjs commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

TDD assessment: PR #33, issue #32 scope

Major

  • The active-playback-queue invariant has no regression test. The change deletes the replace path that rewrote the active queue when the browse projection changed; navigation now only starts playback from the explicit facet-play path (App.tsx:424-438). The former focused replacementQueue test was removed, and the current UI tests contain no playback/navigation assertion (ui.test.ts:120-190). Add one test at the reducer/player boundary that starts a library or playlist queue, changes source/facets/search or opens and backs out of a playlist, and asserts the current queue and origin remain unchanged. This is a stated issue invariant and a regression here would silently change what plays next.

  • Per-source restoration and playlist Back are tested only as disconnected helpers, not through the reducer/UI transition that implements them. rememberSelection/restoreSelection are unit-tested (ui.test.ts:140-148), but the behavior under review is in source, select, browserPanes, and playlist reducer actions (App.tsx:159-165, App.tsx:215-218) plus the Back callback (App.tsx:831-840). Add a transition-level test that verifies Music selection survives Music -> Podcasts -> Music, pane removal updates the saved selection, and Playlist Back clears only selectedPlaylist while retaining the current library selection. The helper test would still pass if any of those action branches regressed.

  • Playlist layout persistence is not covered end-to-end. The TypeScript test proves playlistOverride keys/deletes an in-memory map (ui.test.ts:157-171); Rust tests prove load-time normalization and validation of one serialized map (store.rs:1243-1290). Neither proves that changes wired from PlaylistView (App.tsx:849-862) are saved, reloaded, and applied independently to two playlist IDs. Add one focused settings-store round-trip with two playlists covering order, widths, and hidden columns, plus a UI/reducer-level assertion that a playlist uses its own layout rather than library layout/defaults.

Minor

  • The genre presentation tests do not cover the rendered cell or the removed override indicator. The only test checks facetLabel in isolation (ui.test.ts:187-190). It does not render TrackCell, where No Genre is selected from the facet title and the override dot was removed (libraryViews.tsx:98-117). Add a small component test for a Music Uncategorized track with overridden: true: it should render No Genre, the metadata title, and no ; non-Music should retain its source label.

Migration coverage

The two corrective migration-test commits are represented by the current canonical legacy-order test: it supplies the actual prior visual order and expects the current library default (store.rs:1190-1209). That is useful coverage and no new migration defect is evident. The playlist settings normalization tests also cover malformed/partial persisted values and default-override compaction (store.rs:1211-1290). The missing coverage is behavior after persistence and across independent playlist IDs, not migration parsing itself.

Verdict

Coverage is not adequate for merge as-is. The migration tests are sound, but the primary navigation and queue invariants are untested at their owning transitions; add the three Major tests before merge. No tests were run for this assessment.

@rianjs

rianjs commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed all three Major assessment gaps in 61be568: transition-level queue/origin preservation, source/pane/playlist Back state restoration, keyed playlist layout resolution, and two-playlist settings round-trip coverage. Focused frontend tests: 36/36 passed. Focused Rust store tests: 2/2 passed. The Minor rendered-cell test was intentionally skipped because this repository has no component-test harness and the pure label behavior plus native validation cover it without adding infrastructure.

@rianjs
rianjs force-pushed the feature/library-view-state branch from 88fa901 to b7123ba Compare August 16, 2026 17:28

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: b7123ba6cbed
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer Findings
architecture:seams 1
frontend:view-state 1
rust:implementation-tests 1
security:credential-boundary 1
tauri:config-ipc 0
architecture:seams (1 finding)

Major - apps/desktop/src-tauri/src/playback/reducer.rs:420

This violates Retune's provider-isolation rule: the canonical playback reducer now depends directly on crate::lastfm, stores lastfm_* state, and emits LastFmActions. Consequently, changing or removing the Last.fm contract requires modifying the shared queue/state-transition path used by every backend. Keep only generation-scoped neutral listening facts in playback (track started, cumulative forward time, seek/discontinuity, completion), and let the shell translate those facts into the Last.fm service, where scrobble_threshold_ms and provider-specific eligibility belong.

frontend:view-state (1 finding)

Minor - apps/desktop/test/ui.test.ts:228

The keyed-layout regression test never has two playlists with custom values: every playlistOverride call starts from {}, and the only second playlist assertion checks default fallback. A future update that replaces the entire persisted keyed map when saving playlist B would erase playlist A's layout yet still pass, violating independent persisted playlist layouts. Apply an override for a second playlist to the first call's result, verify both custom layouts resolve, then restore one and verify the other remains.

rust:implementation-tests (1 finding)

Major - apps/desktop/src-tauri/src/lastfm.rs:740

enqueue performs the queue's atomic filesystem write (including sync_all) while holding the async runtime mutex on the Tauri async runtime. The same pattern also holds this mutex across keyring/filesystem work in finish, disconnect, and flush_once. Slow storage or a credential-store prompt blocks an executor worker and prevents all Last.fm state/queue operations from progressing. Move blocking persistence/keyring calls to spawn_blocking and use a snapshot/commit sequence so the async mutex is not held during I/O; add a regression test that verifies concurrent state work can proceed while persistence is stalled.

security:credential-boundary (1 finding)

Blocking - apps/desktop/src-tauri/src/lib.rs:2128

load_diagnostics exposes every current-session log message to the WebView (and then the support-email path) without a redaction boundary. This includes errors logged by notify_error; Spotify's Error::Http formats the complete provider response body, including failures from the refresh-token endpoint. An unexpected or echoed credential-bearing response can therefore cross into ordinary frontend/support data. Redact credential/query/header/body patterns before constructing diagnostic entries (or retain only structured safe error fields), and add canary tests proving access, refresh, playback, Last.fm session, and authorization-token values cannot appear in diagnostics.

Reviewer Coverage

  • architecture:seams — complete (broad); inspected 17 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/src/App.css, apps/desktop/src/App.tsx, apps/desktop/src/appState.ts, apps/desktop/src/diagnostics.ts, apps/desktop/src/dialogViews.tsx, apps/desktop/src/libraryViews.tsx, apps/desktop/src/spotifyViews.tsx, apps/desktop/src/types.ts, apps/desktop/src/ui.ts, docs/architecture/persistence.md, docs/architecture/playback.md; skipped: none; constraints: Review was limited to architecture seams in the assigned files; the adjacent Last.fm module was consulted only to reconstruct the dependency boundary.
  • frontend:view-state — complete (constrained); inspected 1 assigned file (22 inspected across reviewers): apps/desktop/test/ui.test.ts; skipped: none; constraints: Review limited to the assigned UI regression test file; implementation files were outside this review scope.
  • rust:implementation-tests — complete (broad); inspected 9 assigned files (22 inspected across reviewers): Cargo.lock, apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs; skipped: none; constraints: Cargo is not installed in this review environment, so Rust tests and compilation could not be executed. Review scope was limited to the assigned Rust and Cargo changes.
  • security:credential-boundary — complete (constrained); inspected 6 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/store.rs, docs/DEVELOPMENT.md, docs/architecture/persistence.md; skipped: none; constraints: Review scoped to credential, sensitive-data, and lifecycle boundaries in the assigned files; Last.fm and diagnostics implementation was read only as necessary context. The focused Rust test command could not run because the required cargo executable was unavailable through the mandated shell wrapper.
  • tauri:config-ipc — complete (constrained); inspected 9 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/test/ui.test.ts; skipped: none; constraints: Narrow review limited to Tauri configuration, IPC, DTO, event, and desktop security boundaries; general playback correctness was out of scope. Rust test execution was unavailable because the cargo executable could not be started in this environment.
  • unassigned⚠️ unassigned; skipped: .github/workflows/release.yml, .gitignore, scripts/build-install.sh, scripts/check-release.mjs; changed files were not assigned to a selected reviewer
Inspected files (22)
  • Cargo.lock
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/src/diagnostics.rs
  • apps/desktop/src-tauri/src/lastfm.rs
  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/playback/mod.rs
  • apps/desktop/src-tauri/src/playback/reducer.rs
  • apps/desktop/src-tauri/src/playback_commands.rs
  • apps/desktop/src-tauri/src/store.rs
  • apps/desktop/src/App.css
  • apps/desktop/src/App.tsx
  • apps/desktop/src/appState.ts
  • apps/desktop/src/diagnostics.ts
  • apps/desktop/src/dialogViews.tsx
  • apps/desktop/src/libraryViews.tsx
  • apps/desktop/src/spotifyViews.tsx
  • apps/desktop/src/types.ts
  • apps/desktop/src/ui.ts
  • apps/desktop/test/ui.test.ts
  • docs/DEVELOPMENT.md
  • docs/architecture/persistence.md
  • docs/architecture/playback.md

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 5m 22s | gpt-5.6-sol, gpt-5.6-terra | cr 0.10.288
Field Value
Model gpt-5.6-sol, gpt-5.6-terra
Reviewers architecture:seams, frontend:view-state, rust:implementation-tests, security:credential-boundary, tauri:config-ipc
Engine codex_cli · gpt-5.6-sol, gpt-5.6-terra
Reviewed by cr · rianjs-bot[bot]
Duration 5m 22s wall · 14m 01s compute
Cost unavailable
Tokens 4.9M in / 39.9k out

Per-workstream usage

  • orchestrator-selection — gpt-5.6-terra
    • In: 17.4k
    • Out: 796
    • Cache read: 11.0k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 18s
  • architecture:seams — gpt-5.6-sol
    • In: 1.1M
    • Out: 7.6k
    • Cache read: 946.4k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 2m 47s
  • frontend:view-state — gpt-5.6-terra
    • In: 232.3k
    • Out: 3.2k
    • Cache read: 199.9k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 09s
  • rust:implementation-tests — gpt-5.6-terra
    • In: 813.9k
    • Out: 6.1k
    • Cache read: 720.4k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 2m 22s
  • security:credential-boundary — gpt-5.6-terra
    • In: 1.9M
    • Out: 13.6k
    • Cache read: 1.7M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 4m 37s
  • tauri:config-ipc — gpt-5.6-terra
    • In: 826.9k
    • Out: 7.6k
    • Cache read: 724.7k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 2m 38s
  • orchestrator-rollup — gpt-5.6-terra
    • In: 38.9k
    • Out: 974
    • Cache read: 28.2k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 7s

Comment thread apps/desktop/src-tauri/src/lib.rs
Comment thread apps/desktop/src-tauri/src/playback/reducer.rs Outdated
Comment thread apps/desktop/test/ui.test.ts
Comment thread apps/desktop/src-tauri/src/lastfm.rs Outdated

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: 8bb1be0b1053
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer Findings
architecture:seams 0
frontend:view-state 0
rust:implementation-tests 0
security:credential-boundary 1
tauri:config-ipc 1
security:credential-boundary (1 finding)

Major - apps/desktop/src-tauri/src/lib.rs:2126

The Last.fm completion path can persist a newly authorized session before clearing the pending authorization token. If pending-token cleanup fails, finish returns with the old session still active in memory while the native credential store now contains the new account’s session. Playback can continue scrobbling as the old account until restart, then silently switch accounts. Clear the pending token before saving the replacement session (or commit the new session into runtime even when pending cleanup fails), and add a failure-injection test for pending-token removal during a cross-account completion.

tauri:config-ipc (1 finding)

Major - apps/desktop/src-tauri/src/diagnostics.rs:68

The diagnostic DTO still leaks conventional hyphenated credential fields. For example, X-Api-Key: canary is not matched by api key/api_key; the fallback key match is rejected because is_key_boundary treats - as part of a word. load_diagnostics therefore exposes the value to the WebView and email_diagnostics forwards it unchanged. Add hyphenated sensitive-key variants (including access/refresh/session/client-secret forms) or normalize - and _ before matching, with regression tests for X-Api-Key.

Reviewer Coverage

  • architecture:seams — complete (constrained); inspected 17 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/src/App.css, apps/desktop/src/App.tsx, apps/desktop/src/appState.ts, apps/desktop/src/diagnostics.ts, apps/desktop/src/dialogViews.tsx, apps/desktop/src/libraryViews.tsx, apps/desktop/src/spotifyViews.tsx, apps/desktop/src/types.ts, apps/desktop/src/ui.ts, docs/architecture/persistence.md, docs/architecture/playback.md; skipped: none; constraints: Review was limited to ownership, dependency direction, lifecycle, provider isolation, persistence, and playback seams in the assigned files. The workspace was read-only; review used source and diff inspection only.
  • frontend:view-state — complete (constrained); inspected 1 assigned file (22 inspected across reviewers): apps/desktop/test/ui.test.ts; skipped: none; constraints: Review limited to the assigned UI regression test file; implementation files were outside this review scope.
  • rust:implementation-tests — complete (constrained); inspected 9 assigned files (22 inspected across reviewers): Cargo.lock, apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs; skipped: none; constraints: Cargo is not installed in this review environment, so Rust tests and compilation could not be executed. Review scope was limited to the assigned Rust and Cargo changes.
  • security:credential-boundary — complete (constrained); inspected 6 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/store.rs, docs/DEVELOPMENT.md, docs/architecture/persistence.md; skipped: none; constraints: none
  • tauri:config-ipc — complete (constrained); inspected 9 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/test/ui.test.ts; skipped: none; constraints: Read-only Tauri/IPC boundary review; no local test execution was performed.
  • unassigned⚠️ unassigned; skipped: .github/workflows/release.yml, .gitignore, scripts/build-install.sh, scripts/check-release.mjs; changed files were not assigned to a selected reviewer
Inspected files (22)
  • Cargo.lock
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/src/diagnostics.rs
  • apps/desktop/src-tauri/src/lastfm.rs
  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/playback/mod.rs
  • apps/desktop/src-tauri/src/playback/reducer.rs
  • apps/desktop/src-tauri/src/playback_commands.rs
  • apps/desktop/src-tauri/src/store.rs
  • apps/desktop/src/App.css
  • apps/desktop/src/App.tsx
  • apps/desktop/src/appState.ts
  • apps/desktop/src/diagnostics.ts
  • apps/desktop/src/dialogViews.tsx
  • apps/desktop/src/libraryViews.tsx
  • apps/desktop/src/spotifyViews.tsx
  • apps/desktop/src/types.ts
  • apps/desktop/src/ui.ts
  • apps/desktop/test/ui.test.ts
  • docs/DEVELOPMENT.md
  • docs/architecture/persistence.md
  • docs/architecture/playback.md

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 2m 47s | gpt-5.6-sol, gpt-5.6-terra | cr 0.10.288
Field Value
Model gpt-5.6-sol, gpt-5.6-terra
Reviewers architecture:seams, frontend:view-state, rust:implementation-tests, security:credential-boundary, tauri:config-ipc
Engine codex_cli · gpt-5.6-sol, gpt-5.6-terra
Reviewed by cr · rianjs-bot[bot]
Duration 2m 47s wall · 7m 26s compute
Cost unavailable
Tokens 10.1M in / 59.5k out

Per-workstream usage

  • architecture:seams — gpt-5.6-sol
    • In: 1.9M
    • Out: 10.5k
    • Cache read: 1.7M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 05s
  • frontend:view-state — gpt-5.6-terra
    • In: 335.0k
    • Out: 3.8k
    • Cache read: 285.4k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 15s
  • rust:implementation-tests — gpt-5.6-terra
    • In: 2.3M
    • Out: 10.8k
    • Cache read: 2.1M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 46s
  • security:credential-boundary — gpt-5.6-terra
    • In: 3.7M
    • Out: 20.1k
    • Cache read: 3.4M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 2m 18s
  • tauri:config-ipc — gpt-5.6-terra
    • In: 1.9M
    • Out: 13.2k
    • Cache read: 1.8M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 55s
  • orchestrator-rollup — gpt-5.6-terra
    • In: 63.4k
    • Out: 1.1k
    • Cache read: 49.4k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 5s

Comment thread apps/desktop/src-tauri/src/diagnostics.rs
Comment thread apps/desktop/src-tauri/src/lib.rs

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: b71bd09e3ee8
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer Findings
architecture:seams 0
frontend:view-state 0
rust:implementation-tests 1
security:credential-boundary 0
tauri:config-ipc 0
rust:implementation-tests (1 finding)

Major - apps/desktop/src-tauri/src/lastfm.rs:755

commit_session destructively clears the pending token and may clear a different account's durable queue before save_session writes the replacement session. If keyring/session persistence then fails, the old session remains installed while its queue and the retry token have already been removed; after restart the user cannot retry completion and queued scrobbles are lost. Keep a rollback snapshot and restore pending/queue on a later failure (or otherwise make the credential and cleanup steps recoverable as one transaction); add a failing-session-save regression test for the cross-account path.

Reviewer Coverage

  • architecture:seams — complete (constrained); inspected 17 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/src/App.css, apps/desktop/src/App.tsx, apps/desktop/src/appState.ts, apps/desktop/src/diagnostics.ts, apps/desktop/src/dialogViews.tsx, apps/desktop/src/libraryViews.tsx, apps/desktop/src/spotifyViews.tsx, apps/desktop/src/types.ts, apps/desktop/src/ui.ts, docs/architecture/persistence.md, docs/architecture/playback.md; skipped: none; constraints: Review was limited to architecture seams in the assigned files. The dossier's reducer thread reflects an earlier revision; the current head emits provider-neutral ListeningFact values and contains no Last.fm dependency in the reducer. The workspace was read-only; review used source and diff inspection only.
  • frontend:view-state — complete (constrained); inspected 1 assigned file (22 inspected across reviewers): apps/desktop/test/ui.test.ts; skipped: none; constraints: Review limited to the assigned UI regression test file; implementation files were outside this review scope.
  • rust:implementation-tests — complete (constrained); inspected 9 assigned files (22 inspected across reviewers): Cargo.lock, apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs; skipped: none; constraints: Cargo is not installed in this review environment, so Rust tests and compilation could not be executed. Review scope was limited to the assigned Rust and Cargo changes.
  • security:credential-boundary — complete (constrained); inspected 6 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/store.rs, docs/DEVELOPMENT.md, docs/architecture/persistence.md; skipped: none; constraints: Targeted Rust test execution could not run because Cargo is unavailable in the review environment.
  • tauri:config-ipc — complete (constrained); inspected 9 assigned files (22 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/test/ui.test.ts; skipped: none; constraints: Read-only Tauri/IPC boundary review; no local test execution was performed.
  • unassigned⚠️ unassigned; skipped: .github/workflows/release.yml, .gitignore, scripts/build-install.sh, scripts/check-release.mjs; changed files were not assigned to a selected reviewer
Inspected files (22)
  • Cargo.lock
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/src/diagnostics.rs
  • apps/desktop/src-tauri/src/lastfm.rs
  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/playback/mod.rs
  • apps/desktop/src-tauri/src/playback/reducer.rs
  • apps/desktop/src-tauri/src/playback_commands.rs
  • apps/desktop/src-tauri/src/store.rs
  • apps/desktop/src/App.css
  • apps/desktop/src/App.tsx
  • apps/desktop/src/appState.ts
  • apps/desktop/src/diagnostics.ts
  • apps/desktop/src/dialogViews.tsx
  • apps/desktop/src/libraryViews.tsx
  • apps/desktop/src/spotifyViews.tsx
  • apps/desktop/src/types.ts
  • apps/desktop/src/ui.ts
  • apps/desktop/test/ui.test.ts
  • docs/DEVELOPMENT.md
  • docs/architecture/persistence.md
  • docs/architecture/playback.md

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 3m 18s | gpt-5.6-sol, gpt-5.6-terra | cr 0.10.288
Field Value
Model gpt-5.6-sol, gpt-5.6-terra
Reviewers architecture:seams, frontend:view-state, rust:implementation-tests, security:credential-boundary, tauri:config-ipc
Engine codex_cli · gpt-5.6-sol, gpt-5.6-terra
Reviewed by cr · rianjs-bot[bot]
Duration 3m 18s wall · 5m 23s compute
Cost unavailable
Tokens 13.1M in / 71.7k out

Per-workstream usage

  • architecture:seams — gpt-5.6-sol
    • In: 2.5M
    • Out: 12.4k
    • Cache read: 2.4M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 44s
  • frontend:view-state — gpt-5.6-terra
    • In: 462.0k
    • Out: 4.1k
    • Cache read: 401.7k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 12s
  • rust:implementation-tests — gpt-5.6-terra
    • In: 2.8M
    • Out: 12.9k
    • Cache read: 2.6M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 45s
  • security:credential-boundary — gpt-5.6-terra
    • In: 4.5M
    • Out: 25.7k
    • Cache read: 4.2M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 2m 44s
  • tauri:config-ipc — gpt-5.6-terra
    • In: 2.7M
    • Out: 15.4k
    • Cache read: 2.4M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 52s
  • orchestrator-rollup — gpt-5.6-terra
    • In: 90.6k
    • Out: 1.2k
    • Cache read: 73.7k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 5s

Comment thread apps/desktop/src-tauri/src/lastfm.rs Outdated

@rianjs-bot rianjs-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR Review

Reviewed commit: d2bcc1737262
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer Findings
architecture:seams 0
frontend:view-state 0
rust:implementation-tests 0
security:credential-boundary 0
tauri:config-ipc 0

Reviewer Coverage

  • architecture:seams — complete (constrained); inspected 19 assigned files (24 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/build.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/src-tauri/windows-app-manifest.xml, apps/desktop/src/App.css, apps/desktop/src/App.tsx, apps/desktop/src/appState.ts, apps/desktop/src/diagnostics.ts, apps/desktop/src/dialogViews.tsx, apps/desktop/src/libraryViews.tsx, apps/desktop/src/spotifyViews.tsx, apps/desktop/src/types.ts, apps/desktop/src/ui.ts, docs/architecture/persistence.md, docs/architecture/playback.md; skipped: none; constraints: Review was limited to architecture seams in the assigned files. The new Windows manifest/build-script changes are platform build configuration; Tauri-specific correctness remains outside this review's narrow scope. The workspace was read-only; review used source and diff inspection only.
  • frontend:view-state — complete (constrained); inspected 1 assigned file (24 inspected across reviewers): apps/desktop/test/ui.test.ts; skipped: none; constraints: Review limited to the assigned UI regression test file; implementation files were outside this review scope.
  • rust:implementation-tests — complete (constrained); inspected 9 assigned files (24 inspected across reviewers): Cargo.lock, apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs; skipped: none; constraints: Cargo is not installed in this review environment, so Rust tests and compilation could not be executed. The latest commit changes only unassigned Windows build/manifest files; assigned Rust/Cargo files were unchanged from the prior reviewed revision.
  • security:credential-boundary — complete (constrained); inspected 6 assigned files (24 inspected across reviewers): apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/store.rs, docs/DEVELOPMENT.md, docs/architecture/persistence.md; skipped: none; constraints: none
  • tauri:config-ipc — complete (constrained); inspected 9 assigned files (24 inspected across reviewers): apps/desktop/src-tauri/Cargo.toml, apps/desktop/src-tauri/src/diagnostics.rs, apps/desktop/src-tauri/src/lastfm.rs, apps/desktop/src-tauri/src/lib.rs, apps/desktop/src-tauri/src/playback/mod.rs, apps/desktop/src-tauri/src/playback/reducer.rs, apps/desktop/src-tauri/src/playback_commands.rs, apps/desktop/src-tauri/src/store.rs, apps/desktop/test/ui.test.ts; skipped: none; constraints: Read-only Tauri/IPC boundary review; no local test execution was performed.
  • unassigned⚠️ unassigned; skipped: .github/workflows/release.yml, .gitignore, scripts/build-install.sh, scripts/check-release.mjs; changed files were not assigned to a selected reviewer
Inspected files (24)
  • Cargo.lock
  • apps/desktop/src-tauri/Cargo.toml
  • apps/desktop/src-tauri/build.rs
  • apps/desktop/src-tauri/src/diagnostics.rs
  • apps/desktop/src-tauri/src/lastfm.rs
  • apps/desktop/src-tauri/src/lib.rs
  • apps/desktop/src-tauri/src/playback/mod.rs
  • apps/desktop/src-tauri/src/playback/reducer.rs
  • apps/desktop/src-tauri/src/playback_commands.rs
  • apps/desktop/src-tauri/src/store.rs
  • apps/desktop/src-tauri/windows-app-manifest.xml
  • apps/desktop/src/App.css
  • apps/desktop/src/App.tsx
  • apps/desktop/src/appState.ts
  • apps/desktop/src/diagnostics.ts
  • apps/desktop/src/dialogViews.tsx
  • apps/desktop/src/libraryViews.tsx
  • apps/desktop/src/spotifyViews.tsx
  • apps/desktop/src/types.ts
  • apps/desktop/src/ui.ts
  • apps/desktop/test/ui.test.ts
  • docs/DEVELOPMENT.md
  • docs/architecture/persistence.md
  • docs/architecture/playback.md

0 PR discussion threads considered. 0 summarized; 0 resolved.


Completed in 1m 51s | gpt-5.6-sol, gpt-5.6-terra | cr 0.10.288
Field Value
Model gpt-5.6-sol, gpt-5.6-terra
Reviewers architecture:seams, frontend:view-state, rust:implementation-tests, security:credential-boundary, tauri:config-ipc
Engine codex_cli · gpt-5.6-sol, gpt-5.6-terra
Reviewed by cr · rianjs-bot[bot]
Duration 1m 51s wall · 3m 29s compute
Cost unavailable
Tokens 17.6M in / 86.6k out

Per-workstream usage

  • architecture:seams — gpt-5.6-sol
    • In: 3.6M
    • Out: 15.1k
    • Cache read: 3.4M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 44s
  • frontend:view-state — gpt-5.6-terra
    • In: 759.1k
    • Out: 4.9k
    • Cache read: 683.3k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 11s
  • rust:implementation-tests — gpt-5.6-terra
    • In: 3.9M
    • Out: 16.7k
    • Cache read: 3.7M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 20s
  • security:credential-boundary — gpt-5.6-terra
    • In: 5.3M
    • Out: 30.4k
    • Cache read: 5.0M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 41s
  • tauri:config-ipc — gpt-5.6-terra
    • In: 3.8M
    • Out: 18.3k
    • Cache read: 3.5M
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 1m 26s
  • orchestrator-rollup — gpt-5.6-terra
    • In: 120.3k
    • Out: 1.2k
    • Cache read: 100.1k
    • Cache create: unavailable
    • Cost: unavailable
    • Duration: 4s

@rianjs
rianjs marked this pull request as ready for review August 16, 2026 19:28
@rianjs
rianjs merged commit cc0d126 into ui-tweaks Aug 16, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant