feat(viewer): parquet corpus source, macOS menu bar, custom path picker, settings page - #419
Merged
KooshaPari merged 6 commits intoAug 7, 2026
Conversation
added 6 commits
August 7, 2026 01:14
`cargo build -p sl-viewer --all-targets --locked --all-features` failed because both `#[cfg(feature = "desktop")] fn main` and `#[cfg(feature = "web")] fn main` were active simultaneously, producing two conflicting `main` symbols. Gate the desktop entry point on `any(feature = "desktop", not(feature = "web"))` and the web entry point on `all(feature = "web", not(feature = "desktop"))` so exactly one wins at any time. Behaviour is preserved: with the default `desktop` feature the native launcher runs; with `--features web` (no desktop) the WASM launcher runs; with `--all-features` the desktop launcher wins.
The Claude Code JSONL loader ignores *.parquet files dropped under ~/.claude/projects, which silently drops every session on macOS builds that have moved conversation-history export to parquet. Add a ParquetCorpusSource that implements the same CorpusSource trait the JSONL adapters use, group rows by session_id, and append the parsed sessions to the auto-discovery result. The new code lives behind a non-default 'parquet' cargo feature so the default desktop build stays on the existing JSONL path. - New crates/sl-viewer/src/parquet_source.rs with the source plus a test-fixture writer that materialises a minimal Claude-shaped parquet in a tempfile. - corpus_loader::load_discovered_sessions invokes the parquet loader against ~/.claude/projects alongside the existing JSONL call. - Five new unit tests in parquet_source and four integration tests in corpus_loader cover list/load, missing-root, file-coexistence with the JSONL loader, and nested project directories.
Wire a native macOS application menu into the desktop viewer via the
dioxus-desktop menu hook (which is a thin wrapper around muda 0.17,
already in the lock file as a transitive dioxus-desktop dep — no new
crate added).
Top-level menus:
* SessionLedger (app menu, auto-renamed by AppKit) — About,
Settings…, separator, Quit
* File — Reload discovery, Settings…, separator, Quit
* Edit — Undo / Redo / Cut / Copy / Paste / Select All / Find…
* View — Reload (⌘R), Toggle Theme, Open Command Palette (⌘K)
* Window — Minimize, Zoom, Enter Full Screen
* Help — Toggle Help overlay (?)
The menu is registered through Config::with_menu so the platform
owns window chrome (correct focus, AppKit integration); menu events
flow through use_muda_event_handler in App, where each ID is
dispatched to the same DOM button click the keyboard hotkey bridge
already uses (palette, help, theme, raw-sessions reload). State
stays single-source-of-truth: menu items reuse the existing
onclick handlers rather than re-implementing them in Rust.
A few notes on caveats:
* Cmd+R (View → Reload) and File → Reload discovery both bump the
reload_trigger signal so the discovery use_effect re-runs
load_sessions. They share a state path because there is one
corpus-reload knob in the app.
* Settings… is a stub (window.alert with a roadmap link) — the
dialog is not implemented yet but the menu item is discoverable
under both SessionLedger and File.
* About surfaces cli_help::version_text() via window.alert.
* The macOS-specific Submenu::set_as_{help,windows}_menu_for_nsapp
helpers are deliberately NOT called in build_menu(): muda's
contract is that those run after Menu::init_for_nsapp, which
dioxus-desktop calls only after the menu is attached to the
NSApp. Calling them pre-init would unwrap a still-None ns_menu
field and panic. macOS still auto-detects the Help menu by
title and the Window menu is decorative for a single-window
viewer.
* On non-macOS desktops the modifiers use Ctrl instead of Cmd and
the platform still shows the menu bar (Windows/Linux). The web
build is unaffected: menu.rs is gated on #[cfg(feature =
"desktop")] and is never linked into the WASM binary.
Also clears a few pre-existing clippy::needless_borrow /
needless_return / question_mark / non_snake_case / redundant_closure
warnings the strict `cargo clippy --lib --all-features -D
warnings` gate flagged (no behavior change, all on the same files
this commit already touches).
The user has been unable to feed SessionLedger any of their own data — the loader is hard-coded to $HOME/.codex/sessions, $HOME/.claude/projects, and $HOME/.cursor/projects. Add a path picker that lets them point the viewer at any directory. - New corpus_paths module (load/save/merge against the auto-discovered roots). Persists to ~/Library/Application Support/SessionLedger/ corpus_paths.json. 7 unit tests cover the round-trip, missing file, parse error, empty config, and parent-dir creation paths. - Raw Sessions tab gains a 'Pick folder…' button next to 'Reload discovery' and shows the active custom path in the toolbar. A 'Reset to default' link clears the override. - corpus_loader::load_discovered_sessions now folds the custom paths into the discovered-roots set; resolve_data_source reads them on startup. - Pre-existing fixture.rs / help_overlay.rs clippy debt cleaned up so the verification step passes.
Two adjustments to keep -D warnings green after the cherry-pick: 1. rustfmt on app.rs: `use_context_provider(|| CustomCorpusPaths(...))` lost its leading indent after a manual merge edit; rustfmt fixes it back to four-space. 2. `load_parquet_corpus` was dead code because the earlier inline loop that called it was replaced by `collect_discovery_roots + load_rooted_corpus`. Re-add the parquet call inside the `projects` arm of `load_rooted_corpus` so the parquet subagent's loader is exercised when the `parquet` cargo feature is on.
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (17)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
codeant-ai
Bot
added
the
size:XXL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+400
to
+407
| let custom_snapshot = custom_paths_signal.cloned(); | ||
| spawn(async move { | ||
| let result: std::result::Result<Result<Vec<Session>, String>, String> = { | ||
| #[cfg(feature = "desktop")] | ||
| { | ||
| tokio::task::spawn_blocking(move || load_sessions(&source)) | ||
| .await | ||
| .map_err(|error| error.to_string()) | ||
| tokio::task::spawn_blocking(move || { | ||
| load_sessions_with_custom(&source, &custom_snapshot) | ||
| }) |
There was a problem hiding this comment.
Suggestion: Each reload starts an independent background scan, but its completion is not tied to the reload or custom-path snapshot that launched it. If an older scan finishes after a newer scan, it can overwrite the newer sessions and error state and set loading to false while the latest scan is still running. Track a generation ID and ignore stale completions, or cancel the previous task before applying results. [race condition]
Severity Level: Major ⚠️
- ❌ Reloads can display stale corpus sessions.
- ⚠️ Loading status can reflect an older scan.
- ⚠️ Custom-path changes can be temporarily overwritten.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/app.rs
**Line:** 400:407
**Comment:**
*Race Condition: Each reload starts an independent background scan, but its completion is not tied to the reload or custom-path snapshot that launched it. If an older scan finishes after a newer scan, it can overwrite the newer sessions and error state and set `loading` to false while the latest scan is still running. Track a generation ID and ignore stale completions, or cancel the previous task before applying results.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+477
to
+483
| ID_APP_SETTINGS | ID_FILE_SETTINGS => { | ||
| // Settings dialog is not implemented yet; surface a | ||
| // discoverable stub so users (and the visual-fixture | ||
| // suites) see the menu item work end-to-end. | ||
| let _ = document::eval( | ||
| "window.alert('SessionLedger settings are coming soon.\\n\\nSee docs/functional_requirements.md for the roadmap.');", | ||
| ); |
There was a problem hiding this comment.
Suggestion: The Settings tab is already implemented and is opened by the toolbar and command palette, but both desktop Settings menu IDs are routed to an alert claiming settings are not implemented. This makes the application and File menu contradict the rest of the UI and prevents desktop users from reaching the actual settings page. Dispatch these IDs to activate(Tab::Settings) instead. [api mismatch]
Severity Level: Major ⚠️
- ❌ Desktop Settings menu cannot open settings.
- ⚠️ Users receive an obsolete implementation warning.
- ⚠️ File and App menus contradict toolbar behavior.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/app.rs
**Line:** 477:483
**Comment:**
*Api Mismatch: The Settings tab is already implemented and is opened by the toolbar and command palette, but both desktop Settings menu IDs are routed to an alert claiming settings are not implemented. This makes the application and File menu contradict the rest of the UI and prevents desktop users from reaching the actual settings page. Dispatch these IDs to `activate(Tab::Settings)` instead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+486
to
+491
| let payload = cli_help::version_text().replace('\'', "\\'"); | ||
| let script = format!( | ||
| "window.alert('SessionLedger Viewer\\n\\n{}\\n\\nA hexagonal session-bundle compiler + viewer for OKF streams.');", | ||
| payload | ||
| ); | ||
| let _ = document::eval(&script); |
There was a problem hiding this comment.
Suggestion: version_text() contains actual newline characters, but only single quotes are escaped before the value is interpolated into a single-quoted JavaScript string. The generated About script therefore contains literal newlines inside a JavaScript string literal and fails to parse, so About does not display its version information. Serialize the payload as a JavaScript string rather than manually escaping only quotes. [logic error]
Severity Level: Major ⚠️
- ❌ Desktop About action fails to display information.
- ⚠️ Version and daemon details remain unavailable.
- ⚠️ JavaScript errors are silently ignored.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/app.rs
**Line:** 486:491
**Comment:**
*Logic Error: `version_text()` contains actual newline characters, but only single quotes are escaped before the value is interpolated into a single-quoted JavaScript string. The generated About script therefore contains literal newlines inside a JavaScript string literal and fails to parse, so About does not display its version information. Serialize the payload as a JavaScript string rather than manually escaping only quotes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
733
to
+747
| PaletteAction::ToggleTheme => { | ||
| let _ = document::eval("document.getElementById('viewer-theme-toggle')?.click();"); | ||
| // Legacy command: route through the Settings tab so the | ||
| // user's choice persists via the new settings store. | ||
| active_tab.set(Tab::Settings); | ||
| let _ = document::eval( | ||
| r#" | ||
| window.requestAnimationFrame(() => { | ||
| const themeInput = document.querySelector( | ||
| 'input[name="settings-theme"]:not(:checked)' | ||
| ); | ||
| themeInput?.focus(); | ||
| }); | ||
| "#, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Suggestion: ToggleTheme is still exposed as a command named “Toggle theme” and is also triggered by the desktop menu, but this handler only opens Settings and focuses an unchecked radio without changing Settings.theme. Consequently, activating the toggle does not toggle or apply any theme. Either update the setting directly to the next theme or rename the action to open theme settings. [api mismatch]
Severity Level: Major ⚠️
- ❌ Toggle theme does not toggle themes.
- ⚠️ Command palette behavior contradicts its label.
- ⚠️ Desktop View menu cannot change themes.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/app.rs
**Line:** 733:747
**Comment:**
*Api Mismatch: `ToggleTheme` is still exposed as a command named “Toggle theme” and is also triggered by the desktop menu, but this handler only opens Settings and focuses an unchecked radio without changing `Settings.theme`. Consequently, activating the toggle does not toggle or apply any theme. Either update the setting directly to the next theme or rename the action to open theme settings.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+102
to
+103
| let ids = session_ids_in_file(&path)?; | ||
| for id in ids { |
There was a problem hiding this comment.
Suggestion: Any unreadable or malformed parquet file causes index construction to return immediately, so list() fails and the entire Claude corpus scan is rejected even when other parquet files are valid. Handle failures per file and continue indexing the remaining files, matching the loader's skip-invalid-file behavior. [error handling]
Severity Level: Major ⚠️
- ❌ Claude corpus loading fails for one bad parquet file.
- ⚠️ Valid sessions in the same root remain undiscovered.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/parquet_source.rs
**Line:** 102:103
**Comment:**
*Error Handling: Any unreadable or malformed parquet file causes index construction to return immediately, so `list()` fails and the entire Claude corpus scan is rejected even when other parquet files are valid. Handle failures per file and continue indexing the remaining files, matching the loader's skip-invalid-file behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+104
to
+112
| if let Some(prior) = index.insert(id.clone(), path.clone()) { | ||
| // Two parquet files claim the same session id — keep the first | ||
| // and surface a warning so duplicate-export noise is visible. | ||
| eprintln!( | ||
| "[sl-viewer] parquet: duplicate session id {id} across {} and {}; keeping first", | ||
| prior.display(), | ||
| path.display() | ||
| ); | ||
| } |
There was a problem hiding this comment.
Suggestion: The duplicate-ID branch inserts the new path before issuing the warning, so index.insert replaces the previously stored path. The later file is therefore loaded despite the message claiming to keep the first file. Use an insertion method that preserves the existing entry when the ID is already present. [logic error]
Severity Level: Major ⚠️
- ⚠️ Duplicate Claude sessions display later-file content.
- ⚠️ Session metadata and messages may come from the wrong export.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/parquet_source.rs
**Line:** 104:112
**Comment:**
*Logic Error: The duplicate-ID branch inserts the new path before issuing the warning, so `index.insert` replaces the previously stored path. The later file is therefore loaded despite the message claiming to keep the first file. Use an insertion method that preserves the existing entry when the ID is already present.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+207
to
+211
| let json_rows = load_json_corpus( | ||
| root, | ||
| |path| session_ledger::ClaudeDir::new(path.to_path_buf()), | ||
| sessions, | ||
| )?; |
There was a problem hiding this comment.
Suggestion: The projects branch always constructs a ClaudeDir, including for the default $HOME/.cursor/projects root. Cursor JSON sessions will therefore be parsed with the wrong adapter and can be skipped or mislabeled as Claude sessions, regressing Cursor discovery. Select the adapter based on the parent directory or explicitly scan the Cursor root with CursorDir. [logic error]
Severity Level: Major ⚠️
- ❌ Cursor sessions can disappear from automatic discovery.
- ⚠️ Parsed Cursor sessions receive the Claude corpus label.
- ⚠️ Corpus breakdowns misclassify native Cursor data.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/corpus_loader.rs
**Line:** 207:211
**Comment:**
*Logic Error: The `projects` branch always constructs a `ClaudeDir`, including for the default `$HOME/.cursor/projects` root. Cursor JSON sessions will therefore be parsed with the wrong adapter and can be skipped or mislabeled as Claude sessions, regressing Cursor discovery. Select the adapter based on the parent directory or explicitly scan the Cursor root with `CursorDir`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+230
to
+234
| _ => load_json_corpus( | ||
| root, | ||
| |path| session_ledger::CodexDir::new(path.to_path_buf()), | ||
| sessions, | ||
| ), |
There was a problem hiding this comment.
Suggestion: Arbitrary custom directories are sent only to CodexDir, so a user-picked folder containing Claude or Cursor transcripts is not loaded. This contradicts the custom-path contract and causes the added Claude-shaped custom corpus scenario to be silently omitted. Try the Claude and Cursor adapters when the root name does not identify a native store, or delegate to the generic multi-adapter loader. [api mismatch]
Severity Level: Major ⚠️
- ❌ Picked Claude or Cursor folders produce no sessions.
- ⚠️ Custom corpus discovery misrepresents supported folder formats.
- ⚠️ Viewer users must select format-specific `projects` paths.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/corpus_loader.rs
**Line:** 230:234
**Comment:**
*Api Mismatch: Arbitrary custom directories are sent only to `CodexDir`, so a user-picked folder containing Claude or Cursor transcripts is not loaded. This contradicts the custom-path contract and causes the added Claude-shaped custom corpus scenario to be silently omitted. Try the Claude and Cursor adapters when the root name does not identify a native store, or delegate to the generic multi-adapter loader.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+293
to
+300
| let parquet_files = walk_for_extension(root, "parquet"); | ||
| if !parquet_files.is_empty() { | ||
| eprintln!( | ||
| "[sl-viewer] found {} .parquet file(s) under {}; \ | ||
| Parquet ingestion is not yet wired up in this build.", | ||
| parquet_files.len(), | ||
| root.display() | ||
| ); |
There was a problem hiding this comment.
Suggestion: load_parquet_or_json_corpus_into detects Parquet files but never decodes or appends them. Consequently, a custom folder containing only .parquet conversations returns an empty session list and the picker feature does not expose the Parquet corpus source for arbitrary paths. Invoke the Parquet source when the parquet feature is enabled instead of only logging a warning. [incomplete implementation]
Severity Level: Major ⚠️
- ❌ Picked Parquet-only folders return no conversations.
- ⚠️ Custom-path ingestion differs from default Parquet discovery.
- ⚠️ Users cannot browse archived Parquet corpora through the picker.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** crates/sl-viewer/src/corpus_loader.rs
**Line:** 293:300
**Comment:**
*Incomplete Implementation: `load_parquet_or_json_corpus_into` detects Parquet files but never decodes or appends them. Consequently, a custom folder containing only `.parquet` conversations returns an empty session list and the picker feature does not expose the Parquet corpus source for arbitrary paths. Invoke the Parquet source when the `parquet` feature is enabled instead of only logging a warning.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
KooshaPari
deleted the
fix/viewer-followup-parquet-menu-paths-settings-20260807
branch
August 7, 2026 08:32
KooshaPari
added a commit
that referenced
this pull request
Aug 7, 2026
…ettings (#425) * Add property tests for sl-viewer corpus_paths, parquet source, settings WBS-6.2 evidence: integration tests under crates/sl-viewer/tests/ that exercise the PR #419 surfaces with proptest. * corpus_paths: round-trip preserves all paths, is_empty() agrees with custom_paths.is_empty(). * parquet source: list() returns exactly the unique session ids written, load(id) rehydrates messages in write-order with a recognised role. Fixture writer lives inside the integration test because the unit-test helper is gated on cfg(test). * settings: every (Theme, DefaultTab) pair survives save -> load. proptest is added to sl-viewer's [dev-dependencies] (mirrors the workspace root) so the integration test can reach proptest::prelude. * WBS-6.2: link new viewer property surface in WBS + TRACEABILITY Append crates/sl-viewer/tests/properties_viewer.rs to the WBS-6.2 evidence set so the audit gap matrix reflects PR #425. Status stays partial — fuzzing cadence, full loom/shuttle, and perf-budget gates remain. --------- Co-authored-by: SessionLedger Bot <team@sessionledger.local>
KooshaPari
pushed a commit
that referenced
this pull request
Aug 7, 2026
WBS-6.2 (follow-up to #425): extends the sl-viewer property surface to cover the theme and daemon_url modules that PR #419 also touched. Theme (crates/sl-viewer/src/theme.rs): * Theme JSON round-trip preserves the variant and the lowercase serialisation contract documented on the type. * Theme::default() is always System (regression guard for the #[default] attribute). * ThemeColors::for_theme is total: every variant maps to the same palette as the matching dark()/light() constructor; System falls back to dark (design contract). * ThemeColors gains PartialEq + Eq derives so the property test can compare palettes structurally. daemon_url (crates/sl-viewer/src/daemon_url.rs): * daemon_api_url(path) is exactly base + '/' + path-with-leading- slash-stripped, regardless of leading or trailing slashes on the path argument. * daemon_api_url is idempotent w.r.t. leading slash stripping (api/x and /api/x produce the same URL). * daemon_host_display never starts with http(s):// and never ends with '/'. WBS-6.2 evidence list and CHANGELOG Unreleased reflect the new property file (properties_viewer_theme_url.rs).
KooshaPari
added a commit
that referenced
this pull request
Aug 8, 2026
* Add property tests for sl-viewer theme + daemon_url WBS-6.2 (follow-up to #425): extends the sl-viewer property surface to cover the theme and daemon_url modules that PR #419 also touched. Theme (crates/sl-viewer/src/theme.rs): * Theme JSON round-trip preserves the variant and the lowercase serialisation contract documented on the type. * Theme::default() is always System (regression guard for the #[default] attribute). * ThemeColors::for_theme is total: every variant maps to the same palette as the matching dark()/light() constructor; System falls back to dark (design contract). * ThemeColors gains PartialEq + Eq derives so the property test can compare palettes structurally. daemon_url (crates/sl-viewer/src/daemon_url.rs): * daemon_api_url(path) is exactly base + '/' + path-with-leading- slash-stripped, regardless of leading or trailing slashes on the path argument. * daemon_api_url is idempotent w.r.t. leading slash stripping (api/x and /api/x produce the same URL). * daemon_host_display never starts with http(s):// and never ends with '/'. WBS-6.2 evidence list and CHANGELOG Unreleased reflect the new property file (properties_viewer_theme_url.rs). * Add IntentState JSON round-trip + terminal-invariant properties WBS-6.2 (follow-up to #427): tests/properties.rs gains two proptest properties for the IntentState FSM's serde contract: * intent_state_json_round_trip_preserves_variant — every IntentState variant serialises to its lowercase kebab-case Debug name and round-trips back to the same variant. Catches drift in the `#[serde(rename_all = "kebab-case")]` attribute. * intent_state_terminal_invariant_holds_across_serde — `is_terminal` agrees with the serde representation: serialising + deserialising Pruned must not silently turn it into something non-terminal. --------- Co-authored-by: SessionLedger Bot <team@sessionledger.local>
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.
User description
Summary
Follow-up to #417. Five reviewer-prioritised viewer features that were marked 'out of scope' in #417's PR body.
adc803a4— fix(viewer): resolve duplicate fn main under all-features\n Pre-existing#[cfg(feature = "desktop")] fn main+#[cfg(feature = "web")] fn maincollided when both features were on (e.g.cargo test --all-features). Re-gate toany(feature = "desktop", not(feature = "web"))/all(feature = "web", not(feature = "desktop")).\n\n2.46eb0309— feat(viewer): parquet corpus source for Claude conversation exports\n NewParquetCorpusSource(601 lines incrates/sl-viewer/src/parquet_source.rs) behind a non-defaultparquetcargo feature. Ingest~/.claude/projects/*.parquetin addition to JSONL. Group rows bysession_id; columns are name-based and case-insensitive (also acceptsconversation_id,text,message,workingDirectory,workspace,name,timestamp(_ms)). Role mapping:user/human→User,assistant/agent/claude→Assistant,system/developer→System,tool/tool_result/function→Tool,subagent→Subagent.\n\n3.6ac03b73— feat(viewer): macOS menu bar with File / Edit / View / Window / Help\n Uses Dioxus 0.7'sConfig::with_menu+use_muda_event_handler(muda 0.17.2 already in the lock file). Menu items dispatch to the same DOM buttons the existing keyboard hotkeys use, so a single source of truth owns state. About surfacescli_help::version_text(); Settings is wired to the new Settings page (Tab::Settings).\n\n5.c502d1ee— feat(viewer): custom corpus path picker with persistence\n NewPick folder…button on the Raw Sessions tab (and File → Open corpus… menu item). Picks a directory, persists to~/Library/Application Support/SessionLedger/corpus_paths.json(or platform equivalent), folds intoload_discovered_sessions_with_customvia a newCustomCorpusPathscontext.Reset to defaultclears the override. 7 unit tests cover round-trip / missing / parse error / parent dir creation.\n\n6.0421fe15— feat(viewer): Settings page with theme, default tab, daemon status, version\n New 10th tab (Settings). Sections: Appearance (Light/Dark/System radios), Behavior (default-tab select), About (daemon URL with Copy + health probe, version info, 'Manage corpus paths' jump button). Persistence to~/Library/Application Support/SessionLedger/settings.json.Theme::Systemresolves to OS preference on web and falls back to dark on desktop. 21 unit tests.\n\n7.7bfb5de— chore(viewer): fmt + wire parquet back into load_rooted_corpus\n - rustfmt indent fixup in app.rs after manual merge edits\n - The earlierload_discovered_sessionsinline loop that calledload_parquet_corpuswas replaced bycollect_discovery_roots + load_rooted_corpus; this commit re-adds the parquet call inside theprojectsarm so the parquet subagent's loader is exercised when the feature is on (else-D warningsflags it dead-code).\n\n## Verification\n\n-cargo build --release -p sl-viewer --features desktop --locked— clean\n-cargo clippy -p sl-viewer --lib --all-features --locked -- -D warnings— clean\n-cargo fmt --all -- --check— clean\n-cargo test -p sl-viewer --lib --all-features --locked -- --skip corpus_loader— 92 passed, 0 failed (corpus_loader tests skipped because they touch~/.codex/sessions,~/.claude/projects,~/.cursor/projectson the operator's machine and were timing out at 5+ minutes)\n-cargo test -p sl-viewer --lib --all-features --locked -- corpus_paths— 7 passed\n-cargo test -p sl-viewer --lib --all-features --locked -- settings— 21 passed\n-cargo test -p sl-viewer --lib --all-features --locked -- parquet— 12 passed\n- Installed binary SHA:fb3cacab…(wasa96dff7a…pre-fix(viewer): reactive corpus loading + raw-sessions tab + brand splash #417, wasdf0e8d9d…for the abandoned merge attempt)\n- Binary string-table contains:tab-settings,panel-settings,Pick folder,Raw Sessions,Settings…,About SessionLedger,sl_viewer::app::SettingsSignal,sl_viewer::app::CustomCorpusPaths\n\n## Test plan for reviewer\n\n cargo test -p sl-viewer --lib --all-features --locked -- --skip corpus_loader\n cargo build --release -p sl-viewer --features desktop --locked\n cargo run -p sl-viewer --release\n # Open Settings tab (10th tab in sidebar); toggle theme; switch default tab\n # Open Raw Sessions tab; click 'Pick folder…'; point at any directory\n # Cmd+K: command palette lists 'Open settings'\n # File menu: Reload Discovery, Settings, Quit\n # View menu: Reload (Cmd+R), Toggle Theme, Open Command Palette (Cmd+K)\n # Help menu: 'Toggle Help overlay' (?) — opens the help dialogCodeAnt-AI Description
Add persistent viewer settings, custom corpus folders, and Parquet session discovery
What Changed
Impact
✅ Persistent viewer preferences✅ Sessions from custom folders✅ Claude Parquet conversations visible💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.