Skip to content

Commit 25d2378

Browse files
committed
feat(review): local review notes across app, CLI, and agents (#88)
Adds persistent review notes anchored to diff change groups, shared by the SwiftUI shell, the jayjay CLI, and agent workflows through one store. Core: new jayjay-review crate (marks, notes, reconciliation) and jayjay-primitives (jj-lib-free shared types). Notes anchor to jj-diff's canonical Histogram change groups and record their whitespace mode; the CLI reconciles through the same jayjay-core provider as the GUI, so rename detection, LFS placeholders, and group indices agree across surfaces. Statuses: current, stale, orphaned, resolved. CLI: jayjay review notes --format json and jayjay review resolve-note, scoped to the working-copy change, documented in AGENTS.md for agent workflows. UI: per-line note markers in the diff gutter with a click-to-edit popover, note-count badge that filters the file list, stale-note banner, and a widened gutter with hover highlight. Review store keys use the real change id (divergent-safe), the SwiftUI facade caches per-file marks with observable invalidation, GPUI refreshes from disk before mutating, and the store preserves unparseable or newer-version notes across saves. Also: agents/ docs split per area (architecture, shells, review-state, website), SwiftUI sources grouped into responsibility folders, and a test audit removing six constant-mirroring tests.
1 parent ce82da2 commit 25d2378

132 files changed

Lines changed: 4833 additions & 1366 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ Keep this file as always-loaded guidance. Load focused docs only when the task t
88

99
- [Release Workflow](agents/release.md) - version bumps, notarization, appcast, GitHub release, Homebrew tap.
1010
- [Testing Guide](agents/testing.md) - Rust/Swift/GPUI test placement, fixtures, UI test rules.
11-
- [Architecture Guide](agents/architecture.md) - MVVM boundaries, file layout, review state, presentation surfaces.
11+
- [Architecture Guide](agents/architecture.md) - workspace crates, dependency rules, MVVM boundaries, core module layout.
12+
- [SwiftUI Shell Guide](agents/swiftui.md) - shell/mac file layout, view-model and caching conventions, presentation surfaces.
13+
- [GPUI Shell Guide](agents/gpui.md) - shell/gpui file layout, state ownership, globals, caches, rendering tips.
14+
- [Review State Guide](agents/review-state.md) - review store, marks, notes, and the reconciliation contract.
1215
- [Design Guide](agents/design.md) - JayJay product context, visual direction, interaction principles.
13-
- [Help Book Guide](agents/help-book.md) - bundled macOS Help Book, web guide reuse, Help Viewer cache, and Apple Help pitfalls.
16+
- [Help Book Guide](agents/help-book.md) - public website (`docs/`), bundled macOS Help Book, web guide reuse, Help Viewer cache, and Apple Help pitfalls.
1417
- [Pull Request Workflow](agents/pull-requests.md) - bookmark-based GitHub and Codeberg PRs, review updates, landing.
1518
- [Code Review Guide](agents/code-review.md) - repo-specific review setup, adversarial checks, severity, and reporting.
1619

@@ -50,7 +53,7 @@ just release # Sign, notarize, package; read agents/release.md first
5053

5154
Business logic lives in Rust core. UniFFI bridges types. SwiftUI and GPUI shells render state and dispatch actions.
5255

53-
Load [Architecture Guide](agents/architecture.md) before changing ownership boundaries, review state, presentation surfaces, or large file layout.
56+
Load [Architecture Guide](agents/architecture.md) before changing crate or ownership boundaries; load the [SwiftUI](agents/swiftui.md) or [GPUI](agents/gpui.md) shell guide before large file-layout or convention changes in that shell; load [Review State Guide](agents/review-state.md) before touching review marks or notes.
5457

5558
## Testing
5659

@@ -105,6 +108,17 @@ Do not use `git commit`, `git add`, `git push`, `git stash`, `git branch`, or `g
105108

106109
Do not add AI attribution to commits or PRs — no `Generated with`, `Co-Authored-By`, or assistant/session trailers — unless the user explicitly asks.
107110

111+
## Local Review Notes
112+
113+
JayJay can store local review notes on the current working-copy change. Agents should read them before finalizing issue work:
114+
115+
```bash
116+
jayjay review notes --repo . --format json
117+
jayjay review resolve-note <id> --repo .
118+
```
119+
120+
Treat `current` notes as actionable, `stale` notes as needing re-check against the changed diff, and `orphaned` notes as comments whose original file/anchor disappeared. Resolve notes only after the underlying feedback is addressed.
121+
108122
## UI And Design
109123

110124
JayJay is a macOS-native developer tool for jj users. Keep UI changes:

Cargo.lock

Lines changed: 34 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[workspace]
2-
members = ["crates/jj-diff", "crates/jj-test", "crates/jayjay-network", "crates/jayjay-core", "crates/jayjay-uniffi", "crates/jayjay-cli", "shell/gpui"]
2+
members = ["crates/jj-diff", "crates/jj-test", "crates/jayjay-network", "crates/jayjay-primitives", "crates/jayjay-review", "crates/jayjay-core", "crates/jayjay-uniffi", "crates/jayjay-cli", "shell/gpui"]
33
resolver = "2"
44

55
[workspace.package]
@@ -26,6 +26,7 @@ serde = { version = "1", features = ["derive"] }
2626
serde_json = "1"
2727
sha2 = "0.11.0"
2828
hex = "0.4"
29+
uuid = { version = "1", features = ["v4"] }
2930
similar = "3.1.1"
3031
unicode-width = "0.2"
3132
unicode-segmentation = "1"

agents/architecture.md

Lines changed: 28 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,82 +1,42 @@
11
# Architecture Guide
22

3-
Load this file before changing module ownership, repo state, review state, presentation surfaces, or large file layout.
3+
Load this file before changing crate ownership, module boundaries, or cross-shell contracts. For shell file layouts and conventions load [SwiftUI Shell Guide](swiftui.md) or [GPUI Shell Guide](gpui.md); for review marks and notes load [Review State Guide](review-state.md).
44

5-
## MVVM
6-
7-
```text
8-
Rust Core -> UniFFI -> ViewModel -> SwiftUI/GPUI Views
9-
^ | | |
10-
| v v v
11-
business type bridge actions rendering
12-
```
5+
## Workspace Crates
136

14-
- **Model** (`crates/jayjay-core/`): jj-lib wrapper, diff engine, tree-sitter syntax, review state, repo operations. Pure Rust, no platform code.
15-
- **Bindings** (`crates/jayjay-uniffi/`): thin UniFFI layer. Convert types and expose core APIs; do not add business logic.
16-
- **SwiftUI ViewModel** (`Repo/RepoViewModel.swift` and `RepoViewModel+Actions.swift`): owns `JayJayRepo`. jj operations go through here. Async work uses `Task.detached` then `MainActor.run`.
17-
- **Views**: feature folders in SwiftUI and GPUI. Views render state and call callbacks; they should not know jj internals.
18-
19-
## Core Modules
7+
| Crate / target | Responsibility |
8+
| --- | --- |
9+
| `jayjay-primitives` | jj-lib-free domain types: `Change`, `Bookmark`, `DiffHunk`, review types, hashing |
10+
| `jj-diff` | Diff engine: Histogram line diff, word diff, tree-sitter syntax, context collapse, wrapping, conflict display, canonical change groups |
11+
| `jayjay-review` | Local review store: marks, notes, reconciliation (uses `jj-diff`) |
12+
| `jayjay-network` | Shared blocking HTTP client helpers |
13+
| `jayjay-core` | jj-lib wrapper and repo operations; re-exports `jj-diff` as `jayjay_core::diff` |
14+
| `jayjay-uniffi` | UniFFI bindings for the SwiftUI shell only; no business logic |
15+
| `jayjay-cli` | `jayjay` binary: app launcher plus agent-facing `review` subcommands |
16+
| `jj-test` | Shared jj repo fixtures for integration and component tests |
17+
| `shell/gpui` | Cross-platform GPUI shell; links the Rust crates directly (no UniFFI) |
18+
| `shell/mac` | SwiftUI shell + the `JayJayDiffUI` Swift package (AppKit diff renderer) |
2019

21-
Keep core logic split by responsibility:
20+
Dependency direction (never invert): `primitives` and `jj-diff` are leaves → `jayjay-review``jayjay-core``jayjay-uniffi` / `jayjay-cli` / `shell/gpui`.
2221

23-
- `repo/mod.rs`
24-
- `repo/log.rs`
25-
- `repo/diff.rs`
26-
- `repo/mutations.rs`
27-
- `repo/bookmarks.rs`
28-
- `repo/git.rs`
29-
- `repo/working_copy.rs`
30-
- `repo/config.rs`
31-
- `repo/environment.rs`
32-
- `repo/resolve.rs`
33-
- `repo/conflicts.rs`
34-
- `repo/annotate.rs`
35-
- `diff/compute.rs`
36-
- `review.rs`
37-
- `hash.rs`
22+
- New shared types go in `jayjay-primitives`, not `jayjay-core`, so review/CLI code stays jj-lib-light.
23+
- Anything two surfaces must agree on (change groups, review identity, note reconciliation) lives at or below `jayjay-review`/`jj-diff` and is consumed by all surfaces. Do not re-implement a diff or identity computation per surface; the GUI and `jayjay review notes` must reconcile through the same provider or notes silently report stale.
3824

39-
## SwiftUI File Layout
25+
## MVVM
4026

4127
```text
42-
shell/mac/Sources/JayJay/
43-
App/
44-
Config/ AppSettings, AppearanceTypes, EditorTypes, TerminalTypes, AppSettingsTools, FontEnvironment, AppMetadata, JJEnvironment
45-
Window/ RepoWindowManager, RepositoryCommands, RepositoryFocus, RepositoryActions
46-
Watcher/ RepoFSWatcher
47-
JayJayApp.swift, CLIInstaller.swift, DebugBadge.swift, LaunchArguments.swift, SparkleUpdater.swift
48-
Repo/ RepoWindow, RepoSidebar, RepoViewModel, RepoViewModel+Actions, DAGView, DAGLayout, DAGRow, DAGRowViewModel, RepoPresentation, RepoToast, CommitBox, BookmarkPicker, UndoView
49-
Detail/ DetailView, DetailHeader, FileColumn, FileListView, AnnotateView, FileHistoryView
50-
Diff/ DiffSection, DiffColors, NativeDiffView, SideBySideDiffView
51-
Onboarding/ OnboardingView, WelcomeView
52-
Settings/ SettingsView, JJConfigView, AboutView, SettingsComponents
53-
Shared/ ChangeActions, ErrorMessages, ReviewStore, SheetViews, CommandPalette
28+
Rust Core -> UniFFI -> ViewModel -> SwiftUI Views
29+
|
30+
+------------- direct link ---------> GPUI shell (view_model + window)
5431
```
5532

56-
## Review State
57-
58-
Review state is persistent across app restarts and local to the user.
59-
60-
- Canonical implementation: `jayjay_core::review::ReviewStore`.
61-
- SwiftUI still has `Shared/ReviewStore.swift` with the same shape; GPUI uses the Rust store.
62-
- Identity is caller-supplied. The store is a pure keyed store with no disk access and no hashing.
63-
- Review identity is computed in `jayjay_core::repo::diff::entry::compute_review_identity` from `MergedTreeValue` blob IDs.
64-
- Keying: `(changeId, path) -> { identity, file_marked, hunks }`.
65-
- Valid marks require the stored identity to match the current `hunk.review_identity`.
66-
- Same blob IDs preserve review across rebases or amends that do not change file bytes.
67-
- Any byte change in the old or new side invalidates only that file's review.
68-
- `is_hunk_reviewed(idx)` is true when the file is marked or the hunk set contains `idx`.
69-
- Marking every hunk promotes to file-level review. Unmarking a hunk drops the file flag.
70-
- Persistence path: `~/Library/Application Support/dev.hewig.jayjay/review_store.json` for Rust and `UserDefaults` for Swift.
71-
72-
## Presentation Surfaces
33+
- **Model** (`crates/`): all business logic. Pure Rust, no platform code.
34+
- **Bindings** (`crates/jayjay-uniffi/`): convert types and expose core APIs; do not add business logic. Bindings regenerate during `just build`.
35+
- **ViewModels** own the repo handle and all jj operations: `Repo/ViewModel/` in SwiftUI, `repo/view_model/` in GPUI.
36+
- **Views**: feature folders in SwiftUI and GPUI. Views render state and call callbacks; they should not know jj internals.
7337

74-
Use repo-level presentation types from `RepoPresentation.swift` instead of ad hoc booleans.
38+
Async conventions (both shells): heavy jj work runs off the UI thread (`Task.detached``MainActor.run` in Swift; `cx.background_spawn``this.update` in GPUI) and every in-flight result is guarded by a supersession check — a token, generation counter, or commit-id compare against current `@State`/VM state — so a slow stale result can never overwrite a newer one. Both shells also suppress the FS-watcher echo of their own writes (`lastInternalMutationAt` / `last_internal_mutation_at`).
7539

76-
- **Inline state**: pane-scoped no-data, first-run guidance, and recoverable section errors.
77-
- **Toast** (`RepoOverlayState.toast` / `RepoToast`): non-blocking action feedback, success messages, conflict follow-up, and lightweight warnings. Keep it short and allow at most one direct action.
78-
- **HUD** (`RepoOverlayState.loading`): temporary blocking busy states where further interaction would be misleading or unsafe. Prefer quiet refreshes.
79-
- **Alert** (`RepoAlertState`): short blocking interruptions that need acknowledgement or a simple binary choice. No forms, long copy, or more than two meaningful actions.
80-
- **Sheet** (`RepoModalState` + `SheetContainer`): forms, previews, richer explanations, multi-step flows, or confirmations needing more context than an alert.
40+
## Core Modules
8141

82-
Do not escalate inline states into alerts or sheets just because they are errors. Scope the surface to the problem.
42+
Keep `jayjay-core` logic split by responsibility under `repo/` (one file or folder per operation family: `log`, `diff/`, `mutations`, `bookmarks`, `git/`, `working_copy`, `resolve/`, `conflicts`, `annotate`, `evolog`, `diffedit/`, `stacked_pr/`, `pull_requests/`, `review_notes`, `undo`, `workspace`). Top-level modules (`dag`, `file_tree`, `fuzzy`, `palette`, `theme`, `commit_message`) are repo-free helpers.

agents/code-review.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ The focused docs remain the source of truth. This guide points review attention
99
## Review Setup
1010

1111
1. Read `AGENTS.md`.
12-
2. Load the focused guide for the changed area: `agents/architecture.md`, `agents/testing.md`, `agents/design.md`, `agents/pull-requests.md`, or `agents/release.md`.
12+
2. Load the focused guide for the changed area: `agents/architecture.md`, `agents/swiftui.md`, `agents/gpui.md`, `agents/review-state.md`, `agents/testing.md`, `agents/design.md`, `agents/pull-requests.md`, or `agents/release.md`.
1313
3. Inspect `jj st` and the diff, then read the full changed files and nearby patterns before judging the patch.
1414
4. Flag direct edits to generated files, bindings, fixtures, release outputs, or documentation assets unless they trace back to source inputs.
1515
5. Identify the changed behavior, affected user path, verification that would catch a regression, and whether the change is non-trivial enough to need adversarial review.

agents/design.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ Clean, modern, approachable. The blue jaybird mascot adds personality without ma
1818
- Support light and dark modes through system preference.
1919
- Prefer macOS-native feel: SF Symbols, system fonts, native controls.
2020

21+
## Website
22+
23+
The public site at [jayjay.hewig.dev](https://jayjay.hewig.dev) (sources in `docs/`) is a brand surface: the landing page, FAQ, and user guide follow the same jaybird palette, voice, and screenshot style as the app. Load [Help Book Guide](help-book.md) for the `docs/` file layout and what to update together when user-facing features change.
24+
2125
## Interaction Principles
2226

2327
1. **Native first** - Use SwiftUI forms, system fonts, SF Symbols, and platform conventions.
2428
2. **Keyboard-driven** - Every action should be reachable through command palette or shortcut.
2529
3. **Dense, not cluttered** - Optimize for scanning, comparison, and repeated developer workflows.
2630
4. **Performance is UX** - Prefer quiet refreshes over loading spinners where possible.
2731
5. **Jujutsu-native** - Embrace changes, bookmarks, revsets, and working-copy semantics rather than forcing git branch/commit mental models.
28-
29-
## GPUI Layout
30-
31-
- Row-like controls should usually set `.w_full()` before centering content; `justify_center()` only centers within the element's own width.

agents/gpui.md

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# GPUI Shell Guide
2+
3+
Load this file before changing the GPUI shell's layout, state ownership, globals, caches, or rendering conventions. Crate boundaries live in [Architecture Guide](architecture.md); review marks and notes in [Review State Guide](review-state.md).
4+
5+
`shell/gpui` is the cross-platform shell (macOS + Linux; packaged as an AppImage via `just gpui-appimage`). It links the Rust crates directly — no UniFFI, no Swift.
6+
7+
## File Layout
8+
9+
```text
10+
shell/gpui/src/
11+
├── app/ process-level: actions, TOML AppConfig store, Theme, native menus,
12+
│ fs_watcher/ (notify-based repo watcher), telemetry, editor/terminal launchers
13+
├── diff/ pure view components: diff_view/ (unified + side-by-side, find bar, wrap_cache),
14+
│ file_column/ (flat + tree list, tree_cache), selection, annotate, image diff
15+
├── platform/ cfg-gated macos.rs vs linux.rs seam: MOD_KEY, toolbar insets, menu-bar
16+
│ strategy (native vs in-window), reveal-in-file-manager
17+
├── repo/ the main repo window feature
18+
│ ├── revset/ change/compare/endpoint helpers over jj revsets
19+
│ ├── toolbar/ top toolbar and buttons
20+
│ ├── view_model/ RepoViewModel: domain state, loaders, mutations, tasks
21+
│ └── window/ RepoWindow view: render, actions, sidebar, DAG, detail, review, status bar
22+
├── ui/ reusable widgets: text_area/, input/, avatar/ (+cache), context_menu, scrollbar
23+
└── windows/ secondary GPUI windows: settings/, bookmark_manager, operation_log/,
24+
command_palette/, evolog, file_history — each opened via a static `open(...)`
25+
```
26+
27+
## Conventions
28+
29+
- **State ownership** mirrors the SwiftUI split: `RepoWindow` (`repo/window/view.rs`) owns UI state — focus, pane/layout, find, diff selection, scroll handles, toasts, commit-box `TextArea`s, cache slots, the fs watcher, and the shared review-store handle. `RepoViewModel` (`repo/view_model/`) owns domain state — `Arc<Repo>`, graph data, selection, loaded diffs, compare state, loading flags. Input flows window → `vm.update(cx, …)``jayjay_core::Repo`.
30+
- **Async** is centralized in `view_model/tasks.rs` (`background_update`, `repo_write_task`, `core_result_task`): heavy jj work on `cx.background_spawn`, re-entry via `this.update`, with per-section generation counters (`change_gen`, `diff_gen`, `pr_gen`, `refresh_gen`) dropping stale results.
31+
- **Globals** (the only ones): `Theme`, `AppConfigStore` (TOML config), `ReviewStoreHandle` (process-wide review store — mutate only through `window/review.rs::mutate`), and the test-only `WatcherSuppressed`. Tests replace all of them via `tests/support::install_test_globals` (ephemeral config, light theme, in-memory review store, suppressed watcher).
32+
- **Render caches** live in `Rc<RefCell<…>>` slots so re-renders reuse work: `DiffWrapCache` (wrapped lines keyed on `Arc<FileDiff>` identity + columns), `FileTreeCache` (flattened tree keyed on hunks identity + visibility + collapsed dirs), and the VM's `diff_cache`. Canvas prepaint writes panel bounds into `Rc<Cell<…>>` slots that mouse handlers read.
33+
- **FS watcher** (`app/fs_watcher/`): notify events are classified into op-heads vs working-copy changes, debounced (1s/2s), relevance-filtered through `has_unignored_working_copy_paths`, and delivered over a flume channel into `vm.handle_working_copy_change`.
34+
35+
## Rendering Tips
36+
37+
- Row-like controls should usually set `.w_full()` before centering content; `justify_center()` only centers within the element's own width.

agents/help-book.md

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
# Help Book Guide
22

3-
Load this file before changing the bundled macOS Help Book, the public user guide, help screenshots, command-palette help entries, or Help menu behavior.
3+
Load this file before changing the bundled macOS Help Book, the public website or user guide, help screenshots, command-palette help entries, or Help menu behavior.
4+
5+
## Website
6+
7+
`docs/` is the GitHub Pages root, served at `jayjay.hewig.dev` (see `docs/CNAME`):
8+
9+
- `docs/index.html` is the landing page with the FAQ (`/#faq`), linked from `README.md`.
10+
- `docs/guide.html` + `docs/guide.css` are the user guide; screenshots live in `docs/imgs` and double as Help Book sources.
11+
- `docs/llms.txt` is the machine-readable project summary; `docs/sitemap.xml` and `docs/robots.txt` cover indexing.
12+
- `docs/appcast.xml` is the Sparkle update feed — owned by the release flow; load [Release Workflow](release.md) before touching it.
13+
14+
When a user-facing feature ships, update together: the guide page, the llms.txt summary if the feature list changed, the FAQ if it answers a common question, and the Help Book (below) which reuses the same content and screenshots.
415

516
## Source Layout
617

0 commit comments

Comments
 (0)