Skip to content

feat(review): unified PR comment parity — collapsibles + CI chips + reviewer count - #1051

Merged
JSONbored merged 1 commit into
mainfrom
feat/review-comment-parity
Jun 22, 2026
Merged

feat(review): unified PR comment parity — collapsibles + CI chips + reviewer count#1051
JSONbored merged 1 commit into
mainfrom
feat/review-comment-parity

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

Completes the converged unified PR comment (#1011) by passing the three args the renderer + bridge already supported but processors.ts never wired:

  • extraCollapsibles — the public-safe sections lifted from the legacy buildPublicPrIntelligenceComment: Signal definitions, Review context, Contributor next steps, Review details. The private "Maintainer notes" section is never included (asserted by test).
  • reviewerCount — surfaced from runGittensoryAiReview (reviewsForNotes.length); drives the "N AI reviewers" chip + the code-review evidence row.
  • mergeReadiness — CI chips derived from listCheckSummaries (success→passed, failure/timed_out/cancelled/action_required→failed, else unverified) + mergeStateLabel from pr.mergeableState.

Safety / parity

  • The legacy public-findings filter (critical-severity drop, linked-issue gate, private-bounty-lifecycle, private-term scrub, 2/5 slice) is extracted into a shared publicSafePreflightFindings() so the legacy builder and the new collapsibles are single-source.
  • Flag-OFF output verified byte-identical to the previous buildPublicPrIntelligenceComment (captured HEAD vs working-tree output, same fixtures, diff empty).
  • New unified-comment-parity.test.ts asserts the flag-ON body contains the public sections and never contains "Maintainer notes"; legacy panel still renders it inline.

Typecheck clean; full unit suite (3292 tests) green. Part of #1011 / convergence epic #983.

…iewer count

Wire the three args the unified-comment renderer/bridge already supported but the processor never passed:

- extraCollapsibles: public-safe Signal definitions / Review context / Contributor next steps / Review details, extracted from the legacy buildPublicPrIntelligenceComment (NEVER the private 'Maintainer notes').
- reviewerCount: surfaced from runGittensoryAiReview (reviewsForNotes.length).
- mergeReadiness: CI chips derived from listCheckSummaries + pr.mergeableState.

Shared the legacy public-findings filter into publicSafePreflightFindings() so the legacy builder and the new collapsibles are single-source; flag-OFF output (buildPublicPrIntelligenceComment) verified byte-identical. Adds a parity test. Part of #1011 / epic #983.
@dosubot dosubot Bot added the size:L label Jun 22, 2026
@ghost

ghost commented Jun 22, 2026

Copy link
Copy Markdown

Note

🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦🟦

💡 Gittensory review — advisory only

0 files · no blockers · readiness 0/100

💡 Advisory only — no action taken

Signal Result Evidence
Code review ✅ No blockers synthesized
Gate result ⚠️ Skipped #1051 is no longer open.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 22, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.76%. Comparing base (5bc8140) to head (1d3fed5).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/queue/processors.ts 30.76% 3 Missing and 6 partials ⚠️
src/signals/engine.ts 75.00% 1 Missing and 8 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1051       +/-   ##
===========================================
+ Coverage   71.78%   96.76%   +24.97%     
===========================================
  Files         141      141               
  Lines       17506    17548       +42     
  Branches     6334     6351       +17     
===========================================
+ Hits        12567    16980     +4413     
+ Misses       3785      108     -3677     
+ Partials     1154      460      -694     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 3c20514 into main Jun 22, 2026
25 of 28 checks passed
@JSONbored
JSONbored deleted the feat/review-comment-parity branch June 22, 2026 18:03
JSONbored added a commit that referenced this pull request Jun 22, 2026
…ke timers)

The test drove an async flow — real WebCrypto token-decrypt + async D1/fetch interleaved with one setTimeout(sleep 3000) — via a fake-timer pump loop. Under CI full-shard coverage load the real macrotask lagged the microtask flush the pump relied on, so the scheduled sleep was never fired and the test HUNG → 15s timeout (the CI-only flake that blocked #1050/#1051/#1052/#1056).

Fix: drop fake timers entirely. Mock setTimeout so the sleep(3000) backoff fires on a real 0ms tick, then await processSubmitDraft directly — the flow runs to completion on the real event loop (probe 404 → instant backoff → probe 200 → open PR) with no pump, no race, no real 3s wait, no weakened assertions. Verified stable across 6+ consecutive plain + coverage runs locally.
JSONbored added a commit that referenced this pull request Jun 22, 2026
…ke timers) (#1057)

The test drove an async flow — real WebCrypto token-decrypt + async D1/fetch interleaved with one setTimeout(sleep 3000) — via a fake-timer pump loop. Under CI full-shard coverage load the real macrotask lagged the microtask flush the pump relied on, so the scheduled sleep was never fired and the test HUNG → 15s timeout (the CI-only flake that blocked #1050/#1051/#1052/#1056).

Fix: drop fake timers entirely. Mock setTimeout so the sleep(3000) backoff fires on a real 0ms tick, then await processSubmitDraft directly — the flow runs to completion on the real event loop (probe 404 → instant backoff → probe 200 → open PR) with no pump, no race, no real 3s wait, no weakened assertions. Verified stable across 6+ consecutive plain + coverage runs locally.
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