Skip to content

Adapt opt-in PR comment summaries to paginated reporting - #1100

Merged
decyjphr merged 3 commits into
yadhav/fix-recent-issuesfrom
decyjphr-pr-comment-summary-audit
Sep 30, 2026
Merged

decyjphr merged 3 commits into
yadhav/fix-recent-issuesfrom
decyjphr-pr-comment-summary-audit

Conversation

@decyjphr

Copy link
Copy Markdown
Collaborator

Large configuration changes need an at-a-glance summary without changing existing deployments' output. This selectively adapts #1075 onto yadhav/fix-recent-issues, retaining its current subject-aware counts and paginated details rather than restoring the source PR's legacy HTML table.

Approach

  • Add PR_COMMENT_SUMMARY_ENABLED, defaulting to false. When enabled, each page includes the operation-wide error-entry count, affected-plugin list, valid check-run link when available, and an unchecked descriptive review checkbox.
  • Preserve existing repo counts, details, errors, warnings, informational messages, check-run reporting and no-webhook dry runs. CREATE_PR_COMMENT=false still suppresses comments.
  • Reserve footer space before pagination and truncation so each complete comment stays within 55,536 characters. GitHub App subjects do not inflate affected-repository counts.
  • Document the behavior and add isolated smoke phase 27 with owned fixtures and real-Octokit transport coverage.

Source audit

Based directly on target commit 337604d30c447554b8e21d88d5a4eb4bb219d052, with no source-branch merge or unrelated ancestry imported.

Source commit Disposition
f9b4408110599130ca8ebabf90779c76f40c2d9b Adapted as 251e119850440a69196c7a4b2b5b0c76b3d7a0e6; existing default counts and pagination remain intact.
0b8dccec030dc88db0a8a187e106d6fc7697e185 No patch needed: closes the obsolete HTML table, which the target's details renderer already replaced.

These are the complete source-only commits. Source tests expecting absent default counts or a single truncated comment were replaced with tests for the current contract. Error counts represent deduplicated error entries, not just repositories with errors; the checkbox is not enforced or automatically checked.

Validation

  • Node 22.12.0: 962 unit tests passed, 12 skipped; 95 native integration tests passed.
  • Ten pinned-base comparisons confirmed exact default comment equivalence with the flag unset or false, including multi-page and truncated output. Exact size-boundary and missing-URL cases are covered.
  • Six offline startup/auth-boundary tests passed; touched-file lint diagnostics are unchanged from baseline.
  • Exclusive live smoke ran only phases 0, 27 and 9: seven scenarios, seven real check runs, nine posted/read-back comments, and 46 assertions passed. The largest stored comment was exactly 55,536 characters. Owned fixtures were removed; independent snapshots confirmed original resources and private environment restoration.

The live harness exited successfully. A separate wrapper failed a process-name heuristic; retained PID ancestry, inherited guard logs, post-dotenv assertions, process termination and the freed port were independently verified without a rerun. Live coverage verifies generated/stored markup with controlled dry-run inputs, not browser rendering, webhook reconciliation, or compatibility with other independently integrated branches.

Adapt PR #1075 feature commit f9b4408 onto 337604d, preserving default output and subject-aware repository counts. The source table-closing commit 0b8dcce is obsolete with the existing details renderer.

Add bounded page footers, focused regressions, and isolated reporting smoke phase 27 with real-Octokit transport coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Truncation can leave the checkbox inside collapsed markup, and the check-run link promises details that large check runs omit.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

This PR adds an opt-in, at-a-glance summary to paginated PR comments while preserving the default reporting output.

Changes:

  • Adds an environment flag for operation-wide error and plugin summaries, a check-run link, and a review checkbox.
  • Reserves footer space within the comment size limit and adds unit, transport, and smoke coverage.
File Description
test/​unit/​lib/​settings-results.test.js Tests summary content, pagination, and size limits.
test/​unit/​lib/​env.test.js Tests the new flag’s values.
test/​integration/​transport/​pr-comment-summary.test.js Tests smoke-phase transport and cleanup.
smoke-test.js Adds an owned-fixture reporting phase.
README.md Documents the flag and smoke phase.
lib/​settings.js Adds summary metadata and footer handling.
lib/​env.js Defaults the summary flag to false.
.env.example Documents the environment setting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/settings.js
Comment thread lib/settings.js Outdated
decyjphr and others added 2 commits September 30, 2026 14:33
Preserve both opt-in summary and incoming comment-markup regression suites when resolving their shared test-file conflict.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reserve closing markup within the comment limit, avoid splitting details/summary tags, and preserve flag-off output. Cover oversized plugin/error/warning/info sections and footer placement in boundary and transport regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@decyjphr
decyjphr merged commit 543ac5d into yadhav/fix-recent-issues Sep 30, 2026
2 checks passed
@decyjphr
decyjphr deleted the decyjphr-pr-comment-summary-audit branch September 30, 2026 18:48
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.

2 participants