Skip to content

ci(deep-review): restructure review output for scannability#2230

Merged
teeohhem merged 1 commit into
mainfrom
teeohhem/deep-review-readability
May 7, 2026
Merged

ci(deep-review): restructure review output for scannability#2230
teeohhem merged 1 commit into
mainfrom
teeohhem/deep-review-readability

Conversation

@teeohhem
Copy link
Copy Markdown
Contributor

@teeohhem teeohhem commented May 7, 2026

Replace single-line **P{n}** file:line - issue -> fix template with a two-line per-finding layout (issue + Fix sub-bullet + optional reviewer credit), severity-emoji section headings, P3 wrapped in a collapsed

Details block, and a footer with reviewer count and testing gaps.

Same content; just easier to scan in a GitHub PR comment.

Summary

Screenshots or video

Before After

How to test locally or on Vercel

References

  • Linear Issue:
  • Related PRs:

Replace single-line `**P{n}** file:line - issue -> fix` template with a
two-line per-finding layout (issue + Fix sub-bullet + optional reviewer
credit), severity-emoji section headings, P3 wrapped in a collapsed
<details> block, and a footer with reviewer count and testing gaps.

Same content; just easier to scan in a GitHub PR comment.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hyperdx-oss Ignored Ignored May 7, 2026 3:14am

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 7, 2026

⚠️ No Changeset found

Latest commit: 5ec6827

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@teeohhem teeohhem merged commit 28b372b into main May 7, 2026
16 of 17 checks passed
@teeohhem teeohhem deleted the teeohhem/deep-review-readability branch May 7, 2026 03:14
@github-actions github-actions Bot added the review/tier-1 Trivial — auto-merge candidate once CI passes label May 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🟢 Tier 1 — Trivial

Docs, images, lock files, or a dependency bump. No functional code changes detected.

Why this tier:

  • All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats
  • Production files changed: 0
  • Production lines changed: 0
  • Branch: teeohhem/deep-review-readability
  • Author: teeohhem

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Deep Review

✅ No critical issues found.

P2 .github/workflows/deep-review.yml:18 — Header says "Per-finding two-line structure" but the example shows three lines (issue bullet, Fix: sub-bullet, <sub> reviewer credit sub-bullet); a literal-following model may collapse to two lines and drop either the fix or the reviewer line -> rename to "Per-finding structure" or "two- to three-line structure" and let the example carry the shape.

P2 .github/workflows/deep-review.yml:46**Testing gaps:** (include only if substantive) one-line bullets. embeds the conditional directive inside the rendered label, so the agent may copy "(include only if substantive) one-line bullets" verbatim into the comment -> move the conditional to the surrounding prose (e.g. "Append **Testing gaps:** followed by one-line bullets, only when there are substantive gaps"), keep the rendered label clean as **Testing gaps:**.

P2 .github/workflows/deep-review.yml:25 — Merging P0 and P1 into a single ### 🔴 P0/P1 -- must fix section while also forbidding P{n} prefixes per finding erases the within-section severity signal, so a P0 data-loss bug renders identically to a P1 contract break under the same heading -> either keep separate ### 🔴 P0 -- critical and ### 🟠 P1 -- high headings, or relax the no-prefix rule for this merged section so each row still leads with its severity.

P3 nitpicks (2)
  • P3 .github/workflows/deep-review.yml:31,44(N) appears as a literal placeholder in <summary>🔵 P3 nitpicks (N)</summary> and **Reviewers (N):**; a strict model could emit the glyph N instead of the count -> annotate as (<count>) or add an explicit "replace N with the actual count" instruction.
  • P3 .github/workflows/deep-review.yml:38-40 — When all findings are P3, the lead-in "✅ No critical issues found. then any P2 advice underneath" combined with the collapsed <details> block hides every finding behind a fold with no visible body -> add one line clarifying behavior for the all-P3 case (e.g., "if all findings are P3, render them as a non-collapsed bullet list under the no-critical-issues lead-in").

Reviewers (1): correctness/maintainability/project-standards (consolidated — instruction-prose-only diff, no executable code, conditional reviewers skipped per the skill's file-type awareness rule).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

E2E Test Results

All tests passed • 165 passed • 3 skipped • 1173s

Status Count
✅ Passed 165
❌ Failed 0
⚠️ Flaky 4
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-1 Trivial — auto-merge candidate once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant