Skip to content

fix(pr-review-cli): name the distinct bytes on a split review notice too - #987

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/review-split-notice-distinct-bytes
Nov 4, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/review-split-notice-distinct-bytes

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hub L261 add-on. PR Lane's completion check for #984 reads distinct-bytes= from the review split notice and saw none in six hours of reviews (03:5x-09:0xZ on 2026-11-04).

Summary

The pr-review-cli chunks notice printed distinct-bytes= only on #984's one-review branch, where the diff fits once its copies are left out. A split review, such as melodic-software/claude-code-plugins#13097's 40+40+3 run (41780888786), printed chunks= bytes= files= excluded= and no distinct bytes. So the field was missing whenever that branch did not fire.

Fix

  • .github/workflows/pr-review-cli.yml, all three job copies:
    • distinct_bytes defaults to $SCOPE_BYTES before the copy check. With no copies left out, the distinct diff is the whole scope.
    • The split notice now prints distinct-bytes=$distinct_bytes between bytes= and files=.

Verification

  • pr-review-cli.test.cjs: 76/76. The split test asserts the notice line chunks=2 bytes=700000 distinct-bytes=700000 files=2,1 excluded=0, and it fails on main's workflow.
  • biome: no fixes. No trailing whitespace.
  • Left out: a live split review showing the field. The next review over 40 files shows it.

Related

🤖 Generated with Claude Code

…e too

The chunks notice printed distinct-bytes= only on the one-review branch
(the diff fits once its copies are left out), so a split review's notice
carried none and a check that reads the field saw nothing for six hours
of split reviews. distinct_bytes now defaults to the scope's bytes before
the copy check, and the split notice prints it, in all three job copies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 4, 2026 09:08
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 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-11-04T09:11:40.092387Z 7333e16 Draft marked ready
ℹ️ 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.

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

claude-security-review reviewed 033db05be34e82120eed50ce24037ca6cb8919ab: 0 finding(s), 0 as inline comments.

No security findings in the incremental change since 7333e16, which covers .github/scripts/pr-review-cli.test.cjs and .github/workflows/pr-review-cli.yml. I read .claude-lane/incremental.diff and judged the change from the code alone.

  • Workflow change: In the split step, which appears in three places in pr-review-cli.yml, the awk-measured size of the copy-free diff now goes into a new measured_bytes variable. distinct_bytes keeps the whole scope's size ($SCOPE_BYTES) unless copies are actually left out, and only then takes the measured value.
  • What it affects: This only changes the distinct-bytes number that the step reports (its distinct-bytes output and the ::notice text). The chunking decisions, the COPIES validation (^[1-9][0-9]{0,5}$) and the identical.txt regular-file and non-symlink guard are unchanged. The value comes from wc -c on a file the step wrote, so it is numeric and not attacker-controlled text.
  • Test change: The test adds an assertion that a mismatched COPIES count reports distinct-bytes equal to the whole scope. It adds no security surface.
  • Trust controls: No trust control is touched. The change does not widen who is trusted, a token's scope, what untrusted text reaches a lane, or any kill switch or merge hold.
  • Other surfaces: No manifest, instruction-surface or userConfig changes are in this increment. I did not consult the instruction exception register, because no standing instruction was deleted or softened.

critical-findings: 0

Comment thread .github/workflows/pr-review-cli.yml Resolved
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

claude-review reviewed 033db05be34e82120eed50ce24037ca6cb8919ab: 0 finding(s), 0 as inline comments.

No findings. I reviewed only the change since 7333e16 (.claude-lane/incremental.diff), in pr-review-cli.yml and pr-review-cli.test.cjs. I read the code at head but did not run the test suite.

  • Workflow fix: The step used to overwrite distinct_bytes with the measured size of the copy-free diff before it knew the copy count matched. A mismatched COPIES left the final split notice reporting the measured remainder, although nothing was left out. The measured value now sits in measured_bytes. distinct_bytes takes it only inside the distinct_files + COPIES == scope_files branch, where input="$distinct" and excluded=$COPIES. The one-review branch prints measured_bytes, which is the size of the file it reviews. All three job copies (around lines 1537, 3655 and 5745) change identically.
  • Test: The new mismatched-copies assertion fits the fixture. With 45 files and 11 listed as identical, the remainder is 34 files, and 34 + 9 ≠ 45, so the mismatch path runs. The 45 files are over the 40-file cap, so the diff splits and the notice carries bytes=1000 distinct-bytes=1000 excluded=0, which the added regex matches.
  • Closing issue: The PR says "No related issue", so there are no acceptance criteria to check.
  • Pre-existing, unchanged here: The split path writes distinct-bytes only into the notice, not into $GITHUB_OUTPUT. The later DISTINCT_BYTES env therefore reads it only from the one-review branch. This increment doesn't touch that.

The copy check measured the distinct diff into distinct_bytes before the
count test, so a copy list that did not match the scope left the split
notice with the smaller measured size beside excluded=0. The measured size
now has its own variable, and distinct_bytes takes it only where
excluded=$COPIES does, in all three job copies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 6494ed5 Nov 4, 2026
29 checks passed
@kyle-sexton
kyle-sexton deleted the fix/review-split-notice-distinct-bytes branch November 4, 2026 09:27
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