Skip to content

fix(review): skip re-publishing a PR surface already current at its head (#1615) - #1616

Merged
JSONbored merged 1 commit into
mainfrom
fix/review-skip-republish-unchanged
Jun 27, 2026
Merged

fix(review): skip re-publishing a PR surface already current at its head (#1615)#1616
JSONbored merged 1 commit into
mainfrom
fix/review-skip-republish-unchanged

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

The scheduled re-gate sweep re-reviewed + re-published every open PR's public surface every ~2-min cycle regardless of change (~4292 pr_public_surface_published in 2h for ~64 PRs). The comment was PATCHed unconditionally and there was no per-PR record of the head the surface was last published at, so the sweep couldn't tell a current PR from a changed one. (AI spend was already deduped via the head_sha-keyed ai_review_cache; this was GitHub-write/processing churn.)

This adds a head_sha-keyed published marker and a sweep skip-guard so the engine only re-reviews/re-publishes non-reviewed or changed PRs:

  • pull_requests.last_published_surface_sha (migration 0080) — stamped (head_sha-pinned) after a successful publish. Omitted from the GitHub-sync SET clause like the sibling approved_head_sha/merge_blocked_sha markers, so a later sync can't clobber it.
  • Skip-guard in reReviewStoredPullRequest, placed after the live-head resync (so the compare uses the live head): skip the re-review + re-publish when last_published_surface_sha === head_sha. A never-published PR (NULL marker) or a changed head (push/rebase/force-push → marker ≠ live head) falls through and re-reviews at the new head. The webhook synchronize/opened paths review directly and always re-stamp — this guard only gates the sweep.
  • Comment idempotency — skip the sticky-comment PATCH when the rendered body is byte-identical (defense-in-depth; also collapses duplicate webhook deliveries).
  • Flake fix (folded in, no separate PR)proof-of-power-stats.test.tsx count-up test forced the prefers-reduced-motion path so the value settles synchronously instead of racing a 3s findByText on the rAF tween (it flaked the ui job on fix(selfhost): route single-provider AI through the name-aware router (#1610) #1611 and fix(selfhost): surface the CLI's structured stdout error before the exit code (#1612) #1613). Verified deterministic across 5 runs.

Rebase re-review is explicitly preserved and pinned by a test: a rebased PR's old-head marker ≠ the live head → it resyncs + re-reviews at the new head (the head_sha-keyed AI cache also misses) → it's never skipped. Expected effect: pr_public_surface_published drops from ~1/PR/sweep to ~1/PR/head_sha.

Closes #1615. Also resolves the #7 ui flake (no separate PR).

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused (review-engine over-publish dedup + rebase preservation; the only non-backend file is the flake-fix test, folded per maintainer request — no separate PR).
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch ≥97% — covered: skip-guard (skip / rebase-fall-through / marker-null), markPullRequestSurfacePublished (stamp / null no-op / stale-head no-op), the publish stamp call + its fail-open catch, and both html_url arms of the comment-idempotency early return.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New branches + a regression test for the bug; the flake fix verified deterministic (5 runs)

If any required check was skipped, explain why:

  • No OpenAPI/cf-typegen regeneration: no route/schema or wrangler.jsonc change. The DB change is a hand-written contiguous migration 0080 (the repo's convention for pull_requests columns — see 0053_pr_approved_head_sha.sql, 0062_pr_last_regated_at.sql); db:migrations:check passes.

Safety

  • No secrets, wallets, hotkeys, trust scores, private rankings, or maintainer evidence exposed — the marker is a commit SHA; the skip log carries repo/PR/headSha only.
  • Public GitHub text stays sanitized, low-noise — this strictly reduces GitHub writes.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests — N/A.
  • API/OpenAPI/MCP behavior updated/tested where needed — N/A.
  • UI changes use live API data / real states — N/A (the only UI change is a test-determinism fix).
  • Visible UI changes include UI Evidence — N/A (no visible UI change).
  • Public docs/changelogs updated where needed — N/A.

Notes

  • The marker is intentionally head_sha-keyed (not a content digest): a rebase is the case that MUST re-review, and head_sha is the single freshness key already used by the AI cache and the sibling markers — avoiding a second source of truth.

…ead (#1615)

The scheduled re-gate sweep re-reviewed + re-published every open PR's public
surface every ~2-min cycle regardless of change (~4292 pr_public_surface_published
in 2h for ~64 PRs). The sticky comment was PATCHed unconditionally and no per-PR
marker recorded the head the surface was last published at.

Add pull_requests.last_published_surface_sha (migration 0080), stamped head_sha-
keyed after a successful publish and omitted from the GitHub-sync SET clause like
the sibling approved_head_sha marker. In reReviewStoredPullRequest, after the
live-head resync, skip the re-review + re-publish when the marker equals the live
head. A never-published PR (NULL marker) or a changed head (push/rebase/force-push)
falls through and re-reviews at the new head; the head_sha-keyed AI cache also
misses on a rebase, so rebases are never skipped (pinned by a regression test).
Also skip the comment PATCH when the body is byte-identical.

Folds in the proof-of-power-stats count-up flake fix (force prefers-reduced-motion
so the value settles synchronously instead of racing a 3s findByText on the rAF
tween — it flaked the ui job repeatedly).
@dosubot dosubot Bot added the size:M label Jun 27, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 5e8d319 Commit Preview URL

Branch Preview URL
Jun 27 2026, 07:06 PM

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.51%. Comparing base (050e751) to head (5e8d319).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1616   +/-   ##
=======================================
  Coverage   95.51%   95.51%           
=======================================
  Files         204      204           
  Lines       22041    22050    +9     
  Branches     7963     7966    +3     
=======================================
+ Hits        21052    21061    +9     
  Misses        413      413           
  Partials      576      576           
Files with missing lines Coverage Δ
src/db/repositories.ts 96.14% <100.00%> (+<0.01%) ⬆️
src/db/schema.ts 68.32% <ø> (ø)
src/github/comments.ts 96.77% <100.00%> (+0.10%) ⬆️
src/queue/processors.ts 88.29% <100.00%> (+0.03%) ⬆️
🚀 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 0700d3c into main Jun 27, 2026
21 checks passed
@JSONbored
JSONbored deleted the fix/review-skip-republish-unchanged branch June 27, 2026 19:10
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.

Self-host sweep re-publishes every PR surface every cycle (no 'already published at this head' guard)

1 participant