Skip to content

Add major version tags, to allow implicit upgrades for patch & minor versions - #6

Merged
jaredly merged 21 commits into
mainfrom
major-version
May 19, 2022
Merged

Add major version tags, to allow implicit upgrades for patch & minor versions#6
jaredly merged 21 commits into
mainfrom
major-version

Conversation

@jaredly

@jaredly jaredly commented May 19, 2022

Copy link
Copy Markdown
Contributor

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.

Issue: XXX-XXXX

Test plan:

➜ g tag -d full-or-limited-v0.0.0
Deleted tag 'full-or-limited-v0.0.0' (was 4e86abe)

➜ g push origin :full-or-limited-v0.0.0
To github.com:Khan/actions.git
 - [deleted]         full-or-limited-v0.0.0

➜ 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 added 19 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
@jaredly
jaredly requested review from a team, jeresig and k4b7 May 19, 2022 18:42

@k4b7 k4b7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like a great feature to support. I think it would be good if this also supported 0.x in a similar way. Once something hits 1.x then it should be safe to use the most recent .minor.patch, but before then a .minor upgrade could be breaking.

Comment thread utils/publish.mjs
cmds.push(`git push origin ${tag}`);
// This will succeed with a warning if the major tag doesn't exist
cmds.push(`git push origin :refs/tags/${majorTag}`);
cmds.push(`git push origin ${majorTag}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So this overwrite the major tag with whatever the most updated to minor.patch version within that major version is? Nice solution.

@jeresig jeresig left a comment

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.

Very nice, thank you! @kevinbarabash I was talking with Jared about this and I really think that either folks want to pin to a specific version or they want major updates, but we don't need to care about other cases, I think. If we ever do we can add those release tags, too.

Base automatically changed from lints to main May 19, 2022 20:05
@jaredly
jaredly merged commit c558b4c into main May 19, 2022
@jaredly
jaredly deleted the major-version branch May 19, 2022 20:06
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