Skip to content

feat: add path-filtered CLI UX review and scope reviews by change type#822

Merged
stack72 merged 1 commit intomainfrom
better-ci-checks
Mar 22, 2026
Merged

feat: add path-filtered CLI UX review and scope reviews by change type#822
stack72 merged 1 commit intomainfrom
better-ci-checks

Conversation

@stack72
Copy link
Copy Markdown
Contributor

@stack72 stack72 commented Mar 22, 2026

Summary

  • Add new claude-ux-review job that evaluates CLI user experience: help text, error messages, log/JSON output consistency, and behavioral consistency with existing commands (runs on Sonnet for speed)
  • Path-filter all three Claude review jobs so they only run when relevant files change:
    • Standard code review (code): src/**, integration/**, deno.json
    • Adversarial review (core): src/domain/**, src/infrastructure/**, src/libswamp/**
    • UX review (ux): src/cli/commands/**, src/presentation/**, src/domain/errors.ts, src/libswamp/**
  • Fix auto-merge bypassing failed reviews: each review job now writes a /tmp/review-failed sentinel file when requesting changes, and a follow-up step checks for it and fails the job. This ensures --request-changes reviews actually block auto-merge.
  • PRs that only touch docs, skills, workflows, or scripts skip all Claude reviews and auto-merge faster

Test plan

  • Open a PR that only changes README.md — verify all three Claude reviews are skipped and auto-merge proceeds
  • Open a PR that changes src/presentation/ — verify code review and UX review run, adversarial review is skipped
  • Open a PR that changes src/domain/ — verify all three reviews run
  • Verify a review that posts --request-changes creates /tmp/review-failed and the job fails, blocking auto-merge
  • Verify a review that posts --approve does not create the sentinel file and the job succeeds

@stack72 stack72 changed the title feat: add path-filtered CLI UX review and scope existing reviews by change type feat: add path-filtered CLI UX review and scope reviews by change type Mar 22, 2026
@stack72 stack72 force-pushed the better-ci-checks branch 3 times, most recently from 3cd152f to cb49a53 Compare March 22, 2026 15:09
…hange type

## Summary

- Add new `claude-ux-review` job that evaluates CLI user experience: help text, error messages, log/JSON output consistency, and behavioral consistency with existing commands
- Path-filter all three Claude review jobs so they only run when relevant files change:
  - **Standard code review** (`code`): `src/**`, `integration/**`, `deno.json`
  - **Adversarial review** (`core`): `src/domain/**`, `src/infrastructure/**`, `src/libswamp/**`
  - **UX review** (`ux`): `src/cli/commands/**`, `src/presentation/**`, `src/domain/errors.ts`, `src/libswamp/**`
- PRs that only touch docs, skills, workflows, or scripts skip all Claude reviews and auto-merge faster
- UX review uses Sonnet for speed; standard and adversarial reviews remain on Opus

## Test plan

- [ ] Open a PR that only changes README.md — verify all three Claude reviews are skipped and auto-merge proceeds
- [ ] Open a PR that changes a file in `src/presentation/` — verify standard code review and UX review run, adversarial review is skipped
- [ ] Open a PR that changes a file in `src/domain/` — verify all three reviews run
- [ ] Verify UX review approves quickly on a PR that touches `src/libswamp/` internals without changing user-facing output shapes
- [ ] Verify UX review catches a missing `--json` output field or inconsistent flag name
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code Review

Blocking Issues

None.

Suggestions

  1. Consider adding code path filter to claude-review: The comment says it runs on every non-draft PR intentionally for branch protection approval, which makes sense. However, for docs/scripts-only PRs, this means a full Claude Opus code review still runs (and bills API tokens) even when there's no source code to review. If branch protection requires this job to pass, you could have it auto-approve when needs.changes.outputs.code != 'true' with a fast no-op step instead of running the full review. This would save API costs on non-code PRs while still satisfying branch protection.

  2. Sentinel file path is shared across jobs: Each review job writes to /tmp/review-failed in its own runner, so there's no actual collision risk since they run on separate ubuntu-latest instances. Just noting this is fine as-is — no action needed.

Overall this is a clean, well-structured improvement to the CI pipeline. The path filtering will reduce unnecessary review runs, and the sentinel file mechanism correctly ensures that --request-changes reviews block auto-merge.

@stack72 stack72 merged commit 20565b8 into main Mar 22, 2026
10 checks passed
@stack72 stack72 deleted the better-ci-checks branch March 22, 2026 15:27
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