Skip to content

feat(review): local review notes across app, CLI, and agents (#88)#105

Closed
hewigovens wants to merge 1 commit into
mainfrom
issue-88
Closed

feat(review): local review notes across app, CLI, and agents (#88)#105
hewigovens wants to merge 1 commit into
mainfrom
issue-88

Conversation

@hewigovens

@hewigovens hewigovens commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #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; reconciliation replays the exact GUI render pipeline.
  • 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 --repo . --format json and jayjay review resolve-note <id>, 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 (⌘↩ saves), note-count badge that filters the file list, stale-note banner, widened gutter with hover highlight, and selection that stays off the marker columns.
  • 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; the store preserves unparseable or newer-version notes across saves (atomic writes, .json.corrupt preservation).

Also

  • agents/ docs split per area (architecture, swiftui, gpui, review-state, website coverage in help-book).
  • SwiftUI sources grouped into responsibility folders (Repo/DAG, Repo/Bookmarks, Repo/StackedPr, Shared/CommandPalette, Detail/Evolog).
  • Test audit removed six constant-mirroring tests; new regression coverage for the store, reconciliation, and gutter rendering.

Testing

  • just test (43 suites), just test-app (97), just test-gpui (12), JayJayDiffUI package (33), clippy -D warnings + SwiftLint clean.
  • just test-ui needs a local run (automation permission); ReviewNotesScene was updated for the gutter-marker flow.

@hewigovens hewigovens marked this pull request as ready for review July 5, 2026 03:02

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed4a2eb3b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/mac/Sources/JayJay/Shared/ReviewStore.swift
Comment thread crates/jayjay-review/src/store.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0add07229

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/jayjay-review/src/store.rs Outdated
@hewigovens hewigovens force-pushed the issue-88 branch 2 times, most recently from 1ab07ae to 7741813 Compare July 5, 2026 11:44

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

pub fn is_reviewed(&self, change_id: &str, path: &str, identity: &str) -> bool {
self.review_store
.borrow()
.is_reviewed(change_id, path, identity)
}

P2 Badge Refresh GPUI review marks before rendering them

When another surface (the CLI or SwiftUI app) updates review_store.json while a GPUI window is open, this read path still answers from the process-global store snapshot; only GPUI mutations call refresh_from_disk(). That leaves file rows and hide-reviewed filtering stale, and if the user clicks a stale unchecked checkbox, toggle_reviewed refreshes first and then toggles the now-reviewed mark back off. Refresh the shared store before read-side review queries (or at the review-state refresh boundary) so the rendered state and the toggle base agree.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 601344bb48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/mac/Sources/JayJay/Shared/ReviewStore.swift Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

pub fn is_reviewed(&self, change_id: &str, path: &str, identity: &str) -> bool {
self.review_store
.borrow()
.is_reviewed(change_id, path, identity)

P2 Badge Refresh GPUI review state before rendering marks

When the GPUI shell stays open while another process writes review_store.json (for example jayjay review resolve-note or the SwiftUI shell toggling a mark), this accessor reads the process-lifetime ReviewStore snapshot without refresh_from_disk(). I checked the GPUI consumers in shell/gpui/src/diff/file_column/flat.rs and shell/gpui/src/repo/window/render/layout.rs; both render badges/counts and hide-reviewed filtering through is_reviewed, so they stay stale until this GPUI window performs a mutation (the only path that refreshes) or restarts. Refresh on the read boundary or add an explicit store invalidation before rendering review marks.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

self.review_store
.borrow()
.is_reviewed(change_id, path, identity)

P2 Badge Refresh GPUI review reads from disk

When another surface writes review_store.json while a GPUI window is open, the file-column render path still calls this read against the process-global store loaded at startup; only GPUI mutations go through mutate() and call refresh_from_disk(). In that scenario badges, reviewed counts, and hide-reviewed filtering stay stale until this GPUI window toggles a mark or restarts, so refresh on the read/render boundary or track the store mtime before answering is_reviewed.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@hewigovens

Copy link
Copy Markdown
Owner Author

Codex P2 (stale GPUI review reads) fixed in 25d2378: ReviewStore now keeps an (mtime, len) stamp of the loaded store file and exposes refresh_if_stale(), and the GPUI is_reviewed read path calls it — so file badges, reviewed counts, and hide-reviewed filtering pick up marks written by the CLI or the SwiftUI shell on the next render instead of waiting for a local mutation. Mutations still refresh unconditionally via refresh_from_disk(). Covered by refresh_if_stale_picks_up_external_writes_and_skips_when_unchanged in jayjay-review.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e847507c01

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/mac/Sources/JayJay/Detail/DetailHeader.swift
@hewigovens hewigovens force-pushed the issue-88 branch 2 times, most recently from 213c8ef to 9681a0e Compare July 6, 2026 04:56

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9681a0e92b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread shell/mac/Sources/JayJay/Diff/DiffSection.swift

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce0f961712

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +177 to +178
grouping: reviewStore.listNotes(changeId: reviewChangeId)
.filter { visiblePaths.contains($0.path) },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exclude stale notes from active note counts

When a note's file still exists but the note becomes stale after that file is edited, listNotes still returns the unresolved note here and this filter only checks the path. That keeps the file-row badge and “show noted files only” filter active even though loadedReviewNoteSummaries() filters by the current review identity, so no gutter marker or inline note can appear and the stale-note banner is the only actionable surface. Fresh evidence: the fixed code still counts raw listNotes results rather than reconciled .current statuses; derive these counts from reviewNoteStatuses or also match the current hunk identity/status.

Useful? React with 👍 / 👎.

@hewigovens hewigovens closed this Jul 6, 2026
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.

Local review notes store for agent workflows

1 participant