Skip to content

Add a shared action for gerald, and use it! - #7

Merged
jaredly merged 36 commits into
mainfrom
gerald
May 19, 2022
Merged

Add a shared action for gerald, and use it!#7
jaredly merged 36 commits into
mainfrom
gerald

Conversation

@jaredly

@jaredly jaredly commented May 19, 2022

Copy link
Copy Markdown
Contributor

Summary:

Testing this out

Issue: XXX-XXXX

Test plan:

jaredly added 21 commits May 18, 2022 16:29
Summary:
Setting up this repository! Very excited.

Test plan:
🚫
Summary:
They'd been copied over from graphql-flow, but I figured the Khan/perseus ones might have more recent changes.
The next PR will contain our build & publish scripts.

I also created a 'check-for-changeset' action that I noticed in the perseus pr-actions workflow :)

Test plan:
🤞 not usable/testable quite yet!
…the Khan/actions repo

Summary:
Here's the magic! This will allow us to reference e.g. `Khan/actions@filter-files-v0.0.1` in our workflows (as described in the readme).
I've also added basic workflows, one to check for a changeset, and one to automate releases when a release PR lands.

Test plan:
See the workflow work! I published check-for-changeset-v0.0.0 manually from the CLI to demonstrate.
Summary:
It's very simple for the moment because we're not doing any transpilation. We might decide to adopt typescript or something if we start to have more complicated actions.

Test plan:
`yarn lint` should work!
Also see the github action doing its thing.
…versions

Summary:
it'd be a hassle to have to change all our workflows for patch versions of these.
Now you can do `uses: Khan/actions@filter-files-v1` and it will get all 1.x.x versions.

Test plan:
```bash
actions on  major-version ➜ g tag -d full-or-limited-v0.0.0
Deleted tag 'full-or-limited-v0.0.0' (was 4e86abe)
actions on  major-version ➜ g push origin :full-or-limited-v0.0.0
To github.com:Khan/actions.git
 - [deleted]         full-or-limited-v0.0.0
actions on  major-version ➜ node utils/run-publish.mjs
From .
 * branch            lints      -> FETCH_HEAD
To github.com:Khan/actions.git
 * [new tag]         full-or-limited-v0.0.0 -> full-or-limited-v0.0.0
remote: warning: Deleting a non-existent ref.
To github.com:Khan/actions.git
 - [deleted]         full-or-limited-v0
To github.com:Khan/actions.git
 * [new tag]         full-or-limited-v0 -> full-or-limited-v0
```
@jaredly jaredly self-assigned this May 19, 2022
@khan-actions-bot

khan-actions-bot commented May 19, 2022

Copy link
Copy Markdown
Contributor

Gerald

Required Reviewers
  • @Khan/github-actions for changes to .changeset/smooth-ways-marry.md, .github/NOTIFIED, .github/REVIEWERS, .github/workflows/gerald-pr.yml, actions/gerald-pr/action.yml, actions/gerald-pr/package.json

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@khan-actions-bot
khan-actions-bot requested a review from a team May 19, 2022 19:03
Comment thread .github/workflows/gerald-pr.yml Outdated
with:
token: ${{ secrets.GITHUB_TOKEN }}
admin-token: ${{ secrets.KHAN_ACTIONS_BOT_TOKEN }}
# - name: Get All Changed Files

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whoops, can delete this!

Comment thread actions/gerald/action.yml
Base automatically changed from major-version to main May 19, 2022 20:06
@jaredly
jaredly merged commit 2e50797 into main May 19, 2022
@jaredly
jaredly deleted the gerald branch May 19, 2022 21:09
jeresig added a commit that referenced this pull request Jul 2, 2026
…agging severity-based

Previously every skill violation was blocking. Now the skill-auditor assigns
each violation a severity of `blocking` or `advisory`:

- Source: the skill file's declaration (skill-level default or per-rule
  annotation) when present; otherwise the auditor judges by impact (hard-
  requirement phrasing or correctness/security/data/compat risk -> blocking;
  stylistic/preference -> advisory; ties -> advisory).
- `blocking` -> `issue (blocking, best-practice)` (drives REQUEST_CHANGES).
- `advisory` -> new `suggestion (non-blocking, best-practice)` label (rides
  along with an APPROVE).

The Step 4 verdict is already a mechanical function of posted-comment labels
(#7), so advisory violations no longer force REQUEST_CHANGES; only blocking
ones do. claim-validator can correct an over-/under-stated severity. Skill
violations remain scoped to newly-changed code (#6) regardless of severity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jeresig added a commit that referenced this pull request Jul 2, 2026
…st sub-agent outputs (#194)

* [jeresig/unruffled-keller-15164a] review: skip redundant approvals, drop skipped-dimensions note, persist sub-agent outputs

Three related refinements to the PR review workflow:

- Skip submitting a review when it would be a no-op repeat: an APPROVE with no
  inline comments where the PR's most recent github-actions[bot] review was
  already APPROVED. Steps 7 and 8 still run; only the redundant review
  submission is skipped.
- Remove the "skipped dimensions" note that was appended to the review body
  when a sub-agent's output was unavailable, and all references to it.
- Persist each sub-agent's structured JSON to /tmp/gh-aw/review/out/ and upload
  that directory as a run-scoped artifact (30-day retention) so a human can
  inspect exactly what each reviewer produced when diagnosing or tuning it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: let correctness reviewer import optional repo-specific checks

Add an optional `{{#runtime-import? .github/aw/review/correctness-checks.md}}`
to the correctness-reviewer sub-agent so a host repo can supply extra
correctness things to validate. The `?` variant is silently skipped when the
file is absent, so repos that don't provide it are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: skip PRs labeled skip-ai-review

Add a `skip-ai-review` label check to the workflow's job-level `if:` condition
so a human can opt a specific PR out of automated review. A labeled PR never
starts the agent (zero AI credits) and posts nothing. The label gates each
trigger event going forward; it does not retroactively dismiss a review already
left before the label was added.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: scope inline comments to newly-changed code

Users reported the reviewer re-commenting on code that a previous review
already covered. The workflow reviews the full PR diff on every push, so it
would regenerate comments on unchanged hunks.

Fix it with content-based incremental scoping rather than commit ranges (the
github MCP has no compare-commits tool, and commit-range compares break on
force-push/rebase anyway):

- Step 1 computes a per-file "hunk signature" — one SHA-256 per hunk over its
  added lines only (context/removed lines and line numbers excluded), so the
  hash is stable across force-pushes, rebases, squashes, and base merges.
- It recalls the previous run's signature from cache and derives the set of
  hunks that are new since the last review, written to new-scope.json.
- Step 3 drops candidate correctness findings / skill violations that aren't on
  newly-changed lines, so already-reviewed code is not re-flagged. Narrow
  exception: genuine `issue (blocking)` correctness findings survive even on
  unchanged code. Risk, patterns, and reviewer routing still use the whole PR.
- Step 9 persists reviewedHunks each run.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: only request changes when a posted comment is blocking

A blocking review was observed even though every comment it left was
non-blocking. Step 4's verdict was a category judgment ("any blocking issues")
rather than a check on the labels of the comments actually posted.

Make the verdict a mechanical function of the final comment set's labels:
REQUEST_CHANGES if and only if at least one posted comment carries a blocking
label (`issue (blocking)`, `issue (blocking, best-practice)`, or
`todo (blocking)`); APPROVE otherwise, including when only non-blocking comments
are posted. Never request changes on a purely non-blocking comment set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: pin model versions instead of floating tier aliases

The workflow used gh-aw tier aliases (opus/large/small) that route through a
model gateway matching "anthropic/*opus*", "*sonnet*", "*haiku*" — i.e. any
version, so the reviewer could silently change behavior when a new model ships.

Pin explicit versions:
- Orchestrator engine → claude-opus-4-8 (engine object form)
- opus sub-agents (correctness-reviewer, skill-auditor, thread-reconciler,
  claim-validator) → claude-opus-4-8
- pattern-triage (was `large`, Sonnet-tier) → claude-sonnet-4-6
- reviewer-mapper (was `small`, Haiku-tier) → claude-haiku-4-5

Verified with `gh aw compile`: the engine pin resolves to
ANTHROPIC_MODEL: claude-opus-4-8 in the lock, and full versioned IDs are
accepted for sub-agents (0 errors).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: make skill/best-practice flagging severity-based

Previously every skill violation was blocking. Now the skill-auditor assigns
each violation a severity of `blocking` or `advisory`:

- Source: the skill file's declaration (skill-level default or per-rule
  annotation) when present; otherwise the auditor judges by impact (hard-
  requirement phrasing or correctness/security/data/compat risk -> blocking;
  stylistic/preference -> advisory; ties -> advisory).
- `blocking` -> `issue (blocking, best-practice)` (drives REQUEST_CHANGES).
- `advisory` -> new `suggestion (non-blocking, best-practice)` label (rides
  along with an APPROVE).

The Step 4 verdict is already a mechanical function of posted-comment labels
(#7), so advisory violations no longer force REQUEST_CHANGES; only blocking
ones do. claim-validator can correct an over-/under-stated severity. Skill
violations remain scoped to newly-changed code (#6) regardless of severity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: drop understated-severity aside from claim-validator note

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [jeresig/unruffled-keller-15164a] review: restore the skipped-dimension note

Bring back the note that tells the author which review dimensions were not
assessed when a sub-agent's output was unavailable (or claim validation was
skipped). Step 3 tracks the skipped dimension and Step 6 appends
`Note: <dimension> not assessed this run (<sub-agent> output unavailable).` to
the review body for either verdict.

Reconcile with the redundant no-comment approval skip: a review carrying a
skipped-dimension note is not a bare approval, so it is submitted rather than
skipped. Drops the now-inaccurate "removes the note" line from the
skip-redundant-approval changeset (net-vs-main the note behavior is unchanged).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

3 participants