Add Claude PR review caller workflow (pilot)#3277
Draft
dkayiwa wants to merge 3 commits into
Draft
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Size Change: 0 B Total Size: 18.3 MB ℹ️ View Unchanged
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
pull_request doesn't pass org/repo secrets to PRs from forks, so the Anthropic API key would be empty for the bulk of OpenMRS contributions. pull_request_target runs the workflow YAML from the base branch with secrets available; the reusable workflow now explicitly checks out the PR head SHA. No step executes PR code, so the standard pull_request_target footgun doesn't apply. Also declare permissions explicitly at the caller level so the reusable workflow's pull-requests:write / issues:write requests succeed regardless of repo default workflow permissions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
denniskigen
reviewed
May 7, 2026
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| issues: write |
Member
There was a problem hiding this comment.
Small least-privilege thought: the Claude Code Action typically only needs pull-requests: write to post review comments. If claude-pr-review.yml doesn't actually create or edit issues, issues: write could likely be dropped here. Worth a quick check against the upstream workflow's declared needs.
Comment on lines
+17
to
+25
| extra_instructions: | | ||
| This is a TypeScript/React OpenMRS frontend repository (part of OpenMRS 3.x microfrontend architecture). Pay particular attention to: | ||
|
|
||
| - i18n: user-facing strings must use the t() function from useTranslation, not hard-coded text. Flag hard-coded English strings in JSX, alerts, and toasts. | ||
| - Type safety: avoid `any` in new code; prefer specific types or `unknown` with type guards. Flag new uses of `any`. | ||
| - Data fetching: use SWR/React Query patterns consistent with the rest of the codebase; don't introduce ad-hoc fetch wrappers. | ||
| - Public exports: flag breaking changes to anything exported from index.ts or package entry points. | ||
| - Testing: use @testing-library/react idioms; avoid testing implementation details. Flag tests that mock OpenMRS framework primitives instead of using the official testing utilities. | ||
| - Performance: flag obvious re-render hazards (new object/array literals as hook deps, missing useMemo/useCallback on hot paths) but skip nits. |
Member
There was a problem hiding this comment.
Solid starting point for the pilot. After looking at the conventions in o3-docs and the most-used @openmrs/esm-framework symbols across this repo, a few of these bullets restate generic React/TS advice that Claude already applies without prompting, while some of the highest-leverage O3-specific rules are missing. A few suggestions worth considering:
- Data fetching: the line mentions React Query, but this codebase is SWR-only via
openmrsFetch. The convention from o3-docs is to colocate fetching in*.resource.tsfiles with hooks nameduse<Resource>that memoize their return value (especially for hooks consumed in table rows). - Testing: the current bullet says to flag tests that "mock OpenMRS framework primitives," but per o3-docs the framework mocks at
@openmrs/esm-framework/mockare the official path. The rule there is "don't mock components, render real implementations." - i18n: worth adding the
en.json/ Transifex rule, so the reviewer doesn't ask contributors to update other locale files. Also<Trans>for strings containing HTML. - Framework primitives over ad-hoc: judging by import counts across this repo, the most common violations are around
showSnackbar(not custom toasts),showModal(not bespoke modals),launchWorkspace/launchWorkspace2for workspaces,formatDate/parseDatefor dates, anduseConfig+defineConfigSchemafor configurable values. A bullet flagging custom toasts, bespoke modals, rawdayjs.format(), and hardcoded tunables would catch a lot. - Imports: two small but frequent ones worth adding are
lodash-esoverlodash(tree-shaking) and@carbon/reactas the sole UI library (flag MUI/Chakra/Mantine/etc.).
A denser revised draft, dropping the generic React/TS bits to make room:
extra_instructions: |
This is an OpenMRS 3.x frontend repo (TS/React, microfrontend
architecture). Conventions: https://o3-docs.openmrs.org. Flag
deviations from the following project-specific rules; skip generic
React nits.
- Prefer `@openmrs/esm-framework` primitives over ad-hoc equivalents:
`showSnackbar` for toasts, `showModal` for modals, `launchWorkspace`
(or `launchWorkspace2` where the surrounding code uses v2) for
workspaces, `formatDate`/`parseDate`/`formatDatetime` for dates,
`useConfig` + `defineConfigSchema` for configurable values. Flag
custom toast/modal components, raw `dayjs.format()`, and hardcoded
tunables that should be config.
- Data fetching: colocate in `*.resource.ts`. Use `openmrsFetch` with
`useSWR` / `useSWRImmutable` / `useOpenmrsSWR`; no ad-hoc fetch
wrappers. Name hooks `use<Resource>`. Wrap hook return objects in
`useMemo` (especially for hooks consumed in table rows). Return
`isLoading`, `error`, `data` distinctly. Pass `null` URL for
conditional fetching. No infinite retries.
- i18n: user-facing strings via `t()` from `useTranslation`; use
`<Trans>` for strings containing HTML. `en.json` is the source of
truth and is regenerated by `extract-translations`; don't hand-edit,
and don't ask contributors to update other locale files (Transifex
syncs them).
- UI library: `@carbon/react` only. Flag imports from other component
libraries (MUI, Chakra, Mantine, Radix, etc.).
- Imports: use `lodash-es`, not `lodash` (tree-shaking). Use
`import type { … }` for type-only imports.
- Testing: render real components rather than mocking them; use the
mocks from `@openmrs/esm-framework/mock`. Don't test implementation
details.
- Public exports: flag breaking changes to anything exported from
`index.ts` or package entry points.
- Performance: flag missing `useMemo` on hook return values
(especially in row-rendered components).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a thin caller workflow that invokes the reusable Claude PR review workflow in
openmrs/openmrs-contrib-gha-workflows. Triggers on[opened, ready_for_review, reopened](nosynchronize) to avoid re-reviewing on every push during the pilot.Why
Part of a 10-repo pilot of AI-assisted PR review (see openmrs/openmrs-contrib-gha-workflows#36). We're evaluating Claude Code Action as a potential replacement for CodeRabbit, which has been hitting free-tier limits.
Dependencies
@main.Cost guardrails (pilot)
claude-sonnet-4-6(cheaper of the two main options).[opened, ready_for_review, reopened]only — explicitly excludessynchronize, so each PR gets one review per state-change, not one per push.Test plan
🤖 Generated with Claude Code