Skip to content

Make address-review summaries human-ready - #795

Merged
justin808 merged 8 commits into
mainfrom
jg-codex/human-ready-address-review
Sep 10, 2026
Merged

Make address-review summaries human-ready#795
justin808 merged 8 commits into
mainfrom
jg-codex/human-ready-address-review

Conversation

@justin808

@justin808 justin808 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Why

Address-review checkpoint comments use workflow terms such as “cutoff-safe” in their visible status. Humans should see the useful outcome first, while agents retain the detailed review record they need for future scans.

What changed

  • Start each visible checkpoint with 🤖 plus the posting client and model family, supplied from reliable runtime context.
  • Replace internal status language with a plain-language outcome: review follow-up is complete or needs another pass.
  • Move scan metadata, categorized findings, tracking receipts, and rescan instructions into one collapsed Agent details section.
  • Apply the same human-ready format to replacement carryover comments while preserving hidden parser markers.

How to review and verify

  1. Inspect the primary and replacement-carryover templates for the human-visible summary and collapsed agent evidence.
  2. Confirm the marker-first automation contract and required runtime identity remain covered by the focused template test.

Test plan

  • ruby skills/address-review/bin/address-review-summary-template-test.rb — 5 runs, 58 assertions, 0 failures.
  • ruby skills/pr-batch/bin/target-membership-guard-test.rb — 37 runs, 704 assertions, 0 failures.
  • bin/validate — full clean-tree validation passed, including installer and stack suites.
  • Behavior-level checks verify the robot/client/model opening, human-ready outcome language, a single collapsed Agent details section, and preserved hidden state markers.
  • Changelog classification: deferred_to_update_changelog.
Agent details

Runtime note

The validation commands used the repository-compatible Ruby 3.4.8 / JSON 2.21.2 wrapper and cleared interactive shell hooks. The default local Ruby 3.4.6 / JSON 3.0.1 combination changes unrelated guard-test error formatting.

Review decision

The visible outcome no longer repeats SCAN_SCOPE. Exact scan metadata appears once inside Agent details, which also avoids awkward full-history wording while preserving machine-readable evidence.

@github-actions github-actions Bot added the coderabbit:first-pass Triggers CodeRabbit's automatic first-pass pull-request review. label Sep 8, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 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-10T03:09:26.409513Z e2b9197 New commits
ℹ️ 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.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 4bee1e0b-ebd9-41ff-b884-ea789c202a24


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@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: 04a6b19d3a

ℹ️ 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 skills/address-review/references/templates.md Outdated
Comment thread skills/address-review/references/templates.md Outdated
@claude

claude Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review summary

Small, focused doc-only change (76+/52-, 3 files) that matches its stated Why — renaming internal-sounding status language, adding a 🤖 client · model identity line, and collapsing scan metadata/rescan instructions into a single Agent details <details> block. The template and prose (templates.md / workflows/address-review.md) stay consistent with each other, and the required POSTING_CLIENT/POSTING_MODEL_FAMILY guard (fail loudly rather than guess) matches the rationale in the decision log. No security or performance concerns — this only affects generated comment text.

One concrete bug found and left as an inline comment: the new "I checked review activity %s." sentence (templates.md lines 272 and 275) is ungrammatical for the documented full history via check all reviews value of SCAN_SCOPE, which is exactly the value used on every check all reviews run — the case this PR is trying to make more readable.

No scope concerns — didn't find any new helper/flag/schema field with only one caller, and the diff size matches the described change.

…address-review

* origin/main:
  Split always_allowed per kind and unskew the OC-v1 checkpoint buckets (#512)
  Make coordination opt-in for ordinary PR workflows (#451)
  Record partial changelog benchmark and gated runner (#798)
  Document composition with other skill libraries (#797)
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review summary

Reviewed the template/wording changes in skills/address-review/references/templates.md, the mirrored prose in workflows/address-review.md, and the new/updated assertions in address-review-summary-template-test.rb.

Findings: none. Specifically checked:

  • Marker-first contract preserved: the hidden <!-- address-review-summary --> / <!-- address-review-status --> markers remain the literal first line in both the primary and source-carryover blocks; the new 🤖 **<client> · <model family>** line is only the first visible line. Confirmed no other script in the repo (e.g. skills/pr-batch) parses on the old heading text (## Address-review summary, ## Address-review replacement carryover, Status: cutoff-safe...) — downstream automation only checks the marker prefix and the address-review-source-state:v1 block, so removing those headings doesn't break anything.
  • New required vars fail closed: POSTING_CLIENT/POSTING_MODEL_FAMILY use : "${VAR:?...}", which aborts the script rather than silently guessing — matches the PR's stated decision ("a wrong identity is worse than failing before posting"), and both vars are consumed at two call sites (primary + source blocks), not a single-use flag.
  • Template/test/doc consistency: traced the full primary and source-carryover printf sequences against both the new test assertions (assert_in_order, the two <!-- -->/🤖 regexes, the SCAN_SCOPE ordering/count checks) and the mirrored bullet list in workflows/address-review.md (Findings that mattered / Optional suggestions / Skipped items / Agent details) — all line up.
  • Scope: the diff is proportionate to the stated goal (reformatting an existing template + mirroring docs + the matching test), no speculative abstractions introduced.

Nothing further to flag.

@justin808

Copy link
Copy Markdown
Member Author

🤖 Codex · Astra

Review follow-up complete

Every review item in the selected scan has a recorded outcome, so the next routine check can start after this comment.

Agent details

Scan scope: Full PR history via check all reviews.

Findings that mattered

  • Fixed the duplicated scan scope and awkward full-history wording.
  • Replied to and resolved both review threads that reported this issue.

Skipped items

  • Automated review summaries and lifecycle status comments required no action.

Deferred-work tracking: No follow-up issue was needed.

Next scan: Start after this comment. Say check all reviews to rescan the full PR.

…address-review

* origin/main:
  Disable CodeRabbit incremental reviews (#805)
…address-review

* origin/main:
  Report oversized PR diffs as blocked preflight coverage (#748)
  Make restart preparation bounded and verify every task recovers (#800)
  Remove Python from pack validation (#803)

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

ℹ️ 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 skills/address-review/references/templates.md Outdated
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review summary

Reviewed the diff (templates.md, the address-review workflow prose, and the matching Ruby template test) against the stated goal of making address-review checkpoint comments human-readable while keeping the automation contract intact.

Correctness: No bugs found. Traced every printf/argument pairing, <details>/<summary> balance, and the required POSTING_CLIENT/POSTING_MODEL_FAMILY :? guards — they fire before comment construction and correctly trip the existing EXIT cleanup trap. The primary and source-carryover templates consistently move rescan instructions inside the collapsed Agent details block, and workflows/address-review.md's prose mirrors the templates.md rule change exactly.

Scope: Tight and proportionate to the stated Why — three files, all directly tied to the human-readable-summary goal. The only new test (test_posting_identity_is_required_and_not_guessed) directly exercises the new required env vars; no speculative helpers, flags, or schema additions with a single caller.

One non-issue surfaced during cross-file tracing: skills/pr-batch/bin/single_target_entrypoint_test.rb has fixture strings using the old header text (## Address-review replacement carryover), which this PR didn't update. Traced the actual jq filter that test exercises — it keys only on the <!-- address-review-summary -->/<!-- address-review-status --> markers and the address-review-source-state:v1 block, never the header prose — so this is stale cosmetic fixture text with no functional effect. Not flagging as a blocking finding.

No inline comments to post — nothing else stood out.

…address-review

* origin/main:
  Batch installer copies to reduce validation overhead (#807)
…address-review

* origin/main:
  Speed up PR validation for ordinary documentation (#806)
@justin808
justin808 merged commit 48ed4f8 into main Sep 10, 2026
6 checks passed
@justin808
justin808 deleted the jg-codex/human-ready-address-review branch September 10, 2026 03:23
justin808 added a commit that referenced this pull request Sep 10, 2026
…usted-base-provenance

* origin/main:
  Make address-review summaries human-ready (#795)
justin808 added a commit that referenced this pull request Sep 10, 2026
…ical-token-budgets

* origin/main:
  Separate integration checks from full release verification (#808)
  Make address-review summaries human-ready (#795)
  Speed up PR validation for ordinary documentation (#806)
  Batch installer copies to reduce validation overhead (#807)
  Report oversized PR diffs as blocked preflight coverage (#748)
justin808 added a commit that referenced this pull request Sep 11, 2026
…iet-draft-reviews

* origin/main:
  Accept trusted-base provenance for merged changes (#523)
  Limit optional review nits to the initial fix pass (#811)
  Make merge approval receipts human-first (#820)
  Simplify human-facing writing guidance (#821)
  Make ordinary PR closeout lightweight (#818)
  Separate integration checks from full release verification (#808)
  Make address-review summaries human-ready (#795)
justin808 added a commit that referenced this pull request Sep 11, 2026
…data-trust-boundary

* origin/main:
  Accept trusted-base provenance for merged changes (#523)
  Limit optional review nits to the initial fix pass (#811)
  Make merge approval receipts human-first (#820)
  Simplify human-facing writing guidance (#821)
  Make ordinary PR closeout lightweight (#818)
  Separate integration checks from full release verification (#808)
  Make address-review summaries human-ready (#795)

# Conflicts:
#	skills/pr-batch/bin/pr-security-preflight
#	skills/pr-batch/bin/pr-security-preflight-test.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coderabbit:first-pass Triggers CodeRabbit's automatic first-pass pull-request review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant