Skip to content

docs: review on every push, now that a push costs a delta #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Svilen-Stefanov merged 2 commits into from
Aug 18, 2026
Merged

docs: review on every push, now that a push costs a delta #87

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c7ddd1
docs: review on every push, now that a push costs a delta
Svilen-Stefanov Aug 18, 2026
1ac9399
docs: serialize reviews per pull request alongside per-push runs
Svilen-Stefanov Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ name: CodeBoarding review

on:
pull_request:
types: [opened, reopened, ready_for_review]
types: [opened, reopened, ready_for_review, synchronize]
Comment thread
Svilen-Stefanov marked this conversation as resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exclude the rolling sync PR from per-push reviews

For repositories following the protected-branch setup at README.md:192-210, every baseline update is force-pushed to the machine-owned codeboarding/sync PR, so adding synchronize here runs the paid review job after every such update even though that PR contains only generated baseline state. The repository's own review workflow explicitly excludes this branch at .github/workflows/codeboarding.yml:35-41 because its diagram is noise; the public example should copy that same-repository github.head_ref exclusion so the two recommended modes do not repeatedly review each other's generated output.

Useful? React with 👍 / 👎.

issue_comment:
types: [created]

Expand All @@ -28,6 +28,16 @@ permissions:
issues: write
id-token: write

# One review at a time per pull request. Two pushes in quick succession would
# otherwise analyze concurrently, and both would start from the same older
# analysis instead of the newer one continuing from its predecessor. They also
# share one sticky comment, so whichever finishes last wins — which can be the
# run for the older commit. Queue rather than cancel, so a /codeboarding command
# waits for a running review instead of killing it.
concurrency:
group: codeboarding-${{ github.event.pull_request.number || github.event.issue.number }}
cancel-in-progress: false
Comment on lines +37 to +39

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep filtered comments out of the concurrency group

When one review is running and a synchronize run is pending, any subsequent issue_comment event—even an ordinary or unauthorized comment—enters this workflow-level group before the job-level if filters it. GitHub documents that an existing pending workflow in the same group is canceled and replaced by the newly queued one, so the comment can cancel the pending push review and then skip the review job itself, leaving the sticky diagram at the older SHA indefinitely. Avoid placing filtered-out comment events in the pull-request review group, for example by applying concurrency at the eligible job or separating their group.

Useful? React with 👍 / 👎.


jobs:
review:
if: >
Expand All @@ -44,6 +54,10 @@ jobs:

Automatic runs update one sticky **CodeBoarding review** comment. A trusted repository owner, member, or collaborator can comment `/codeboarding` to analyze the current PR head again, including on fork PRs; every command creates a new result comment.

`synchronize` re-runs the review on every push to the branch. Each of those runs covers only the commits pushed since the previous one, so a push costs a fraction of a first analysis — and a pushed commit is the only thing that builds the reusable analysis, since GitHub gives comment-triggered runs a read-only cache. Drop `synchronize` from the list if you would rather spend one analysis per pull request than one per push.

Keep the `concurrency` block if you keep `synchronize`: it is what makes a push continue from the push before it, and what stops a slower run for an older commit from overwriting the review comment for a newer one. Set `cancel-in-progress: true` instead to abandon a superseded run rather than queue it, which costs less when branches are pushed to rapidly, at the price of no analysis for the commits in between.

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not rely on concurrency to order push reviews

Even when every queued event is a valid synchronize, cancel-in-progress: false only prevents simultaneous execution; GitHub explicitly states that ordering is not guaranteed for workflows in a concurrency group. Two close pushes can therefore have the newer review execute first and the older one afterward, and because automatic runs select the same codeboarding-review identifier in scripts/action/guard.sh:122-123, that older run can still overwrite the newer diagram. The action needs a head-SHA freshness check before publishing (or another ordering mechanism) before this block can be documented as preventing stale overwrites.

Useful? React with 👍 / 👎.


| Command | What it does |
|---|---|
| `/codeboarding` | Analyzes the current head, reusing this PR's previous analysis when one is available. |
Expand Down
Loading