Skip to content

feat(desktop): add hunk-level review progress - #1218

Draft
bzy-debug wants to merge 4 commits into
mainfrom
codex/hunk-mark-viewed-core
Draft

feat(desktop): add hunk-level review progress#1218
bzy-debug wants to merge 4 commits into
mainfrom
codex/hunk-mark-viewed-core

Conversation

@bzy-debug

@bzy-debug bzy-debug commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review now tracks changed-line coverage across Line, Token, and Tree modes. Marking every hunk completes the file; marking the file reviewed marks every hunk, and unmarking a hunk returns the file to partial or unreviewed state. The toolbar shows the active hunk position, including a global position across semantic sections.

Coverage uses only real original/modified changed ranges, excluding joined navigation context. Active-hunk updates are fenced by changed-file identity, diff generation, and surface generation. Whole-file coverage comes from the current ready logical Line diff, so opening another file while Token or Tree remains selected works even when the hidden Line editor has never been laid out. Semantic filters do not redefine the whole-file range.

Rebased onto cd0d5f97 (main). Conflict resolution preserves the new Visible/Ignored diff model: only Visible changes become review hunks, first/last navigation targets, or file review coverage; Ignored changes retain their layout role. Generated interfaces retain the upstream package-graph messages. The four commits retain their original responsibilities; git range-diff shows only these compatibility adaptations and the existing test fixture's migration to DocumentDiffChange.

Validation on rebased head 0522b684:

  • moon info && moon fmt; the JS-only fileeditor interface was also generated and checked from the JS backend output.
  • just check and just build passed.
  • just test: 3,252 Native tests, 3,225 JS tests, 24 cram cases, and both real CLI lifecycle checks passed.
  • just editor-test: 1,090 wasm, 1,876 JS, and 1,224 Native tests passed.
  • just editor-test-browser: 107 passed.
  • just desktop-test-browser: 64 passed, including Token/Tree file-switch regressions for automatic file completion and hunk unmarking after whole-file completion.

Earlier packaged macOS QA on bd8b9409:

  • Rebuilt the real SeekMoon app with moon -C desktop run --target native package/macos -- --release --target app --no-open; bundle signature verification passed. Tested Proton/CEF with the native Git/file-watch bridge.
  • Walked and marked all 40 hunks in a large Token diff, checking progress after every action: only the final hunk completed the file. Unmarking the last hunk in Tree preserved the other viewed hunks.
  • Verified cross-mode and cross-file coverage, Split/Unified and File/Diff transitions, whole-file mark/unmark, insertions/deletions, added/deleted files, F7/Shift+F7 wraparound, and invalidation after disk edits and HEAD changes. No page errors or console warnings/errors were captured. This packaged QA predates the rebase; the current head was validated by the checks above.

Known issue — pointer selection does not update the active hunk:

  1. Open a Line diff with multiple hunks and select the first using change navigation.
  2. Scroll to a later hunk and click its changed code. The toolbar still identifies the first hunk.
  3. Click Mark viewed: the first hunk becomes Viewed while the clicked hunk remains Unviewed.

The same issue reproduces in Token and Tree when one semantic section contains multiple hunks. Clicking a different semantic section updates the section, but clicking another hunk inside that section retains the old navigation index. Use explicit F7/Shift+F7 or toolbar change navigation before marking. This known issue is not fixed by the rebase.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-02T08:51:48.258189Z 6e3dbcf PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@bzy-debug
bzy-debug marked this pull request as draft September 2, 2026 08:47

@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: 6e3dbcf971

ℹ️ 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 +336 to +337
guard self.current_review_generation(change, active_file) ==
Some(review_generation) else {

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 Fence queued hunk updates by the visible surface

When an active-hunk event has already been queued and the user switches between Line/Token/Tree or collapses the active semantic section, those actions clear review_active_hunk but do not change the file's review generation. This guard therefore accepts the late event because it checks only the file and generation, restoring a hunk from the now-hidden surface and allowing “Mark viewed” to update the wrong hunk. Carry and validate the surface/semantic-input identity in the message, or otherwise invalidate the generation when the visible hunk universe changes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d58bcb00. Active-hunk callbacks now carry a monotonic visible-surface generation. Line/Token/Tree switches, semantic section/filter changes, Source/Diff transitions, and semantic-input replacement invalidate it, and the reducer validates row, diff, and surface generations. This rejects a late callback even after a Line → Token → Line ABA round trip. I added a focused white-box reducer regression for that case while preserving exact-review reselection idempotency. Rebased onto current main and validated just check, just test (3410 native, 3183 JS, 28 cram), just build, desktop Playwright (31/31), and a --no-open SeekMoon package.

@bzy-debug
bzy-debug force-pushed the codex/hunk-mark-viewed-core branch 3 times, most recently from d58bcb0 to bd8b940 Compare September 9, 2026 02:11
@bzy-debug
bzy-debug force-pushed the codex/hunk-mark-viewed-core branch from bd8b940 to 0522b68 Compare September 9, 2026 08:14
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