Skip to content

Preserve incremental review log updates#83

Merged
lynnswap merged 18 commits into
mainfrom
codex/complete-review-results
Jul 6, 2026
Merged

Preserve incremental review log updates#83
lynnswap merged 18 commits into
mainfrom
codex/complete-review-results

Conversation

@lynnswap

@lynnswap lynnswap commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Purpose

Fix review log rendering regressions where mirrored Codex reasoning entries could duplicate text, where paragraph spacing around command rows was unstable (command chips merging with neighboring paragraphs, gaps flickering while output streams), and where streaming bursts degraded every display update into a full TextKit reload that recreated command panel views.

Changes

  • Make the log document own inter-block paragraph boundaries: rendered block text is normalized in the styler and visible blocks are always separated by one blank-line gap, so command panels no longer lose the boundary when their block text is replaced by a placeholder, and spacing stays stable while command output streams. Source text stays raw, and expanded command panels read output through the block's source range so captured stdout/stderr keeps its exact newlines.
  • Deduplicate agent_reasoning / reasoning mirror entries by consuming them as pairs per scope and text, so late mirrors that arrive after other items (for example commands) are suppressed while legitimately repeated reasoning still renders. Wrapper payloads prefer the nested item kind over the event envelope type.
  • Apply cross-revision block replacements in the log scroll view: streaming updates supersede in-flight render tasks, so applied display documents can skip revisions; the scroll view now recomputes the replacement against the display document actually on screen instead of falling back to a full reload, and logs when an incremental render still degrades to a reload.
  • Replace full-string replacement validation with UTF-16 segment comparison so source/display validation does not allocate rebuilt documents.
  • Make source document replacement detection target the actual changed block and keep later blocks aligned by range delta.
  • Pass delivery: .inline explicitly on prepared review restarts so the restart path carries the same delivery contract as fresh review starts.
  • Import CodexReviewKit in ReviewRunIDArgument instead of relying on leaky member visibility for the package-scoped nilIfEmpty extension.
  • Commit the resolved files of a checkout without dependencies/CodexKit so fresh clones resolve the pinned remote CodexKit without rewriting Package.resolved.
  • Add regression coverage for mirrored reasoning (including mirrors separated by commands, repeated mirror pairs, and wrapped payload kinds), command-output paragraph boundaries with trailing newlines, adjacent command panels, raw output fidelity in expanded panels, and cross-revision scroll view replacements.

Testing

  • swift test --build-system swiftbuild --no-parallel
  • xcodebuild test -project Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj -scheme CodexReviewMonitor -destination 'platform=macOS,arch=arm64' CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO
  • git diff --check
  • Codex review: no findings

@lynnswap lynnswap marked this pull request as ready for review July 6, 2026 06:22

@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: e7eac9ee97

ℹ️ 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 Sources/ReviewChatLogUI/ReviewMonitorCodexChatLogProjection.swift Outdated
Comment thread Sources/CodexReviewAppServer/AppServerCodexReviewBackend.swift
lynnswap and others added 4 commits July 6, 2026 16:01
…aries

Block text with trailing newlines used to absorb the separator between
blocks, so command panels (which replace block text with a placeholder)
lost the boundary and rendered on the same line as neighboring text,
and streamed output flipped the spacing around command rows.

Normalize block text in the document builder and always separate
visible blocks with one blank-line gap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adjacency-based mirror suppression missed reasoning mirrors that arrive
after an intervening item (for example a command), duplicating the
reasoning text, and compared later entries against a stale previous
key. Track rendered reasoning entries per scope and text, and let each
entry consume exactly one later mirror with the counterpart payload
kind, so late mirrors are suppressed while legitimately repeated
reasoning still renders.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restarted reviews relied on the CodexKit default parameter for the
delivery mode. Pass .inline explicitly so the restart path carries the
same delivery contract as fresh review starts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ReviewRunIDArgument uses the package-scoped nilIfEmpty extension but
relied on leaky member visibility from sibling files' imports.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@lynnswap lynnswap left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@codex 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: b5580ac1d2

ℹ️ 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 Sources/ReviewChatLogUI/ReviewMonitorCodexChatLogProjection.swift Outdated
lynnswap and others added 4 commits July 6, 2026 16:36
A wrapper payload can carry both a top-level event envelope type and
the nested item/payload type. Returning the envelope type first hid
the reasoning payload kind from mirror deduplication, so wrapped live
notifications rendered mirrored reasoning twice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Normalizing block text inside the document builder also rewrote
sourceText, so expanded command panels lost leading/trailing newlines
and an all-newline output collapsed to an empty block. Move rendered
normalization into the styler, keep sourceText raw, and read panel
output through the block's source range.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Streaming updates supersede in-flight render tasks, so applied display
documents can skip revisions. The scroll view only accepted the
single-step lastChange or a pure tail append, so any mid-document
change after a skipped revision fell back to a full reload, recreating
command panel views and rebuilding TextKit layout on every burst.
Recompute the block replacement against the display document that is
actually on screen, and log when an incremental render still degrades
to a reload.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Package.resolved was generated with the local dependencies/CodexKit
checkout present, so fresh checkouts resolving the remote fallback
revision rewrote the resolved files and broke builds with automatic
resolution disabled. Commit the resolved state of a checkout without
the local dependency.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@lynnswap lynnswap left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 0bf7d4ac85

ℹ️ 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".

@lynnswap lynnswap merged commit e812890 into main Jul 6, 2026
1 check passed
@lynnswap lynnswap deleted the codex/complete-review-results branch July 6, 2026 09:02
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