HPNEX-9: Add promptfoo behavioral evals for plugins - #464
Conversation
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Promptfoo-based behavioral eval system: Makefile orchestration, promptfoo + Claude SDK dev deps, per-plugin eval YAMLs and fixtures, an eval budget/config and docs, .gitignore updates, and a GitHub Actions workflow that detects changed plugins and runs per-plugin evals (publishing JUnit XML artifacts). ChangesEvaluation infrastructure and plugin evals
Plugin eval configurations and fixtures
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner
participant Repo as Repository
participant Make as Makefile
participant PF as Promptfoo
participant GCP as GCP (Vertex)
participant Art as Artifact storage
GH->>Repo: issue_comment "/run-evals" on PR
GH->>Runner: start workflow
Runner->>Repo: resolve refs & checkout PR merge ref
Runner->>Repo: run detect-changed-plugins -> plugins JSON
GH->>Runner: start behavioral-evals matrix rgba(100,149,237,0.5)
Runner->>Repo: checkout code for plugin
Runner->>GCP: authenticate (GOOGLE_APPLICATION_CREDENTIALS)
Runner->>Make: make eval-plugins EVAL_PLUGIN=...
Make->>PF: npx promptfoo eval (per-config)
PF->>GCP: Vertex calls via claude-agent-sdk rgba(144,238,144,0.5)
PF-->>Make: XML results
Make->>Runner: write eval-results/*.xml
Runner->>Art: upload eval-results/*.xml (artifact)
Art-->>GH: artifact available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
.gitignore (1)
40-40: ⚡ Quick winDo not ignore
package-lock.jsonif this repo now relies on Node-based eval tooling.Committing the lockfile keeps CI/local dependency trees deterministic.
Suggested change
- package-lock.json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore at line 40, Remove package-lock.json from .gitignore so the repository will track and commit the lockfile; edit the .gitignore entry that currently lists "package-lock.json" (the symbol package-lock.json) and delete that line so CI/local dependency trees remain deterministic when using Node-based tooling.package.json (1)
3-3: ⚡ Quick winPin
@anthropic-ai/claude-agent-sdkto an exact version for stable eval baselines.Using
^0.2.132allows dependency drift between runs, which can make behavioral eval outcomes flaky.Suggested change
- "@anthropic-ai/claude-agent-sdk": "^0.2.132", + "@anthropic-ai/claude-agent-sdk": "0.2.132",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 3, Update the dependency declaration for "@anthropic-ai/claude-agent-sdk" in package.json to an exact version (e.g., "0.2.132") instead of a caret range so CI/evaluations use a stable, reproducible package; after updating package.json, regenerate the lockfile (npm/yarn/pnpm install) to persist the exact resolved version in package-lock.json or yarn.lock.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/eval-plugins.yml:
- Around line 40-65: The GCP auth step (uses: google-github-actions/auth@...) in
the behavioral-evals job runs unconditionally and fails on forked PRs without
secrets; add a guard to the Authenticate to GCP step so it only runs when the
needed secret is present (e.g., add an if condition checking that
secrets.GOOGLE_APPLICATION_CREDENTIALS is non-empty or that the PR is not from a
fork using github.event.pull_request.head.repo.fork == false), ensuring the step
is skipped for forked PRs and CI no longer fails due to missing secrets.
In `@evals/AGENTS.md`:
- Around line 46-59: The fenced code block in AGENTS.md lacks a language tag for
markdownlint; update the opening fence to include a tag (e.g., change "```" to
"```text" or "```bash") so the file-structure block is annotated; edit the block
in AGENTS.md around the "plugins/" listing (the fenced code block starting at
the example structure) and add the language tag to the opening fence only.
In `@Makefile`:
- Around line 45-46: The Makefile currently suppresses and ignores failures in
the dependency install step (`@npm install 2>/dev/null || true`), which hides
real errors before running `$(EVAL_TARGETS)`; replace that line so install
failures propagate (e.g., run `npm install` or `npm ci` without `2>/dev/null`
and without `|| true`) so the recipe fails fast and shows install output before
invoking `@$(MAKE) -j$(words $(EVAL_CONFIGS)) --no-print-directory
$(EVAL_TARGETS)`.
- Around line 43-47: The eval-plugins target currently runs silently when no
eval configs are discovered; modify the Makefile so eval-plugins validates that
EVAL_CONFIGS (or the computed list used to form EVAL_TARGETS) is non-empty and
fails fast if none found—e.g., check $(words $(EVAL_CONFIGS)) (or the same
expression that builds $(EVAL_TARGETS)) and call $(error ...) or exit non-zero
with a clear message before running npm install/$(MAKE) to prevent silent
success; update the eval-plugins target to perform this pre-check and only
proceed to `@npm` install and @$(MAKE) when configs exist.
In `@plugins/jira/evals/fixtures/deprecated-endpoints.md`:
- Around line 1-9: The fixture file uses a YAML-style
"Summary/Description/Technical Details" block which violates the plain Markdown
fixture convention; replace the YAML-like structure in deprecated-endpoints.md
with direct Markdown sections (e.g., a top-level title, "Context", "Acceptance
Criteria", and "Technical Details" headings) and list the items plainly so the
eval loader can parse it, and ensure the endpoints to remove (/v2/users/legacy,
/v2/auth/token-v1) and the acceptance criteria referencing pkg/api/routes.go and
docs/migration-v3.md remain as plain markdown bullet points or paragraphs rather
than YAML keys.
In `@plugins/jira/evals/fixtures/multi-area-feature.md`:
- Around line 3-15: The fixture contains indented markdown headings ("##
Context", "## Acceptance Criteria", "## Technical Details") which trigger MD023;
remove the leading indentation so these headings start at column 1 or convert
them to non-heading labels (e.g., "Context:", "Acceptance Criteria:", "Technical
Details:") to flatten the block; update the three occurrences referenced by the
exact heading texts so the file no longer contains indented "##" lines and
re-run markdownlint to verify the MD023 violation is resolved.
In `@plugins/jira/evals/fixtures/rbac-middleware.md`:
- Around line 1-12: The fixture file
plugins/jira/evals/fixtures/rbac-middleware.md currently uses a YAML-style
wrapper; change it to plain markdown by removing the YAML wrapper and ensuring
all sections (Summary, Description, Context, Acceptance Criteria, Technical
Details) are formatted as normal markdown headings and lists; keep the same
content but convert the block that begins with "Summary: Add RBAC middleware..."
into plain markdown headings and bullet lists and ensure there is no
leading/trailing YAML fence or colon-prefixed key syntax so the fixture is a
valid .md file.
In `@plugins/jira/evals/solve.yaml`:
- Around line 3-30: The eval currently does not ensure the agent actually
invokes the /jira:solve skill, so add a concrete assertion and/or prompt
requirement: in the prompts block require the agent to call "/jira:solve" (e.g.,
explicit instruction in prompts) and in defaultTest.add an assert entry that
verifies a tool/skill call to "/jira:solve" (use an assert type that checks tool
invocations or output contains a tool-call record), referencing the existing
prompts and defaultTest.assert sections so the test fails unless the /jira:solve
skill is exercised.
---
Nitpick comments:
In @.gitignore:
- Line 40: Remove package-lock.json from .gitignore so the repository will track
and commit the lockfile; edit the .gitignore entry that currently lists
"package-lock.json" (the symbol package-lock.json) and delete that line so
CI/local dependency trees remain deterministic when using Node-based tooling.
In `@package.json`:
- Line 3: Update the dependency declaration for "@anthropic-ai/claude-agent-sdk"
in package.json to an exact version (e.g., "0.2.132") instead of a caret range
so CI/evaluations use a stable, reproducible package; after updating
package.json, regenerate the lockfile (npm/yarn/pnpm install) to persist the
exact resolved version in package-lock.json or yarn.lock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5000a689-8227-432f-8b9c-34fd565aa10e
📒 Files selected for processing (17)
.github/workflows/eval-plugins.yml.gitignoreMakefileevals/AGENTS.mdevals/promptfooconfig.yamlpackage.jsonplugins/code-review/evals/classify-review-comment.yamlplugins/hello-world/evals/echo.yamlplugins/jira/evals/fixtures/deprecated-endpoints.mdplugins/jira/evals/fixtures/empty-context.mdplugins/jira/evals/fixtures/missing-ac.mdplugins/jira/evals/fixtures/multi-area-feature.mdplugins/jira/evals/fixtures/rbac-middleware.mdplugins/jira/evals/fixtures/sparse-bug.mdplugins/jira/evals/fixtures/well-groomed.mdplugins/jira/evals/ready-to-solve.yamlplugins/jira/evals/solve.yaml
| behavioral-evals: | ||
| needs: detect-changed-plugins | ||
| if: needs.detect-changed-plugins.outputs.has_plugins == 'true' | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| plugin: ${{ fromJson(needs.detect-changed-plugins.outputs.plugins) }} | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 | ||
| with: | ||
| node-version: '22' | ||
|
|
||
| - name: Authenticate to GCP | ||
| uses: google-github-actions/auth@ba79af03959ebeac9769e648f473a284504d9193 # v2.1.10 | ||
| with: | ||
| credentials_json: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Do GitHub Actions workflows triggered by pull_request from forks have access to repository secrets by default?
💡 Result:
No. By default, GitHub Actions workflows triggered by the pull_request event from a fork do not receive repository (or environment) secrets. GitHub’s secrets documentation explicitly states that “Actions secrets are not passed to workflows that are triggered by a pull request from a fork.” [1] The “Using secrets in GitHub Actions” guide also reiterates that, with the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository. [2] What you still get by default: - The workflow does get a GITHUB_TOKEN, but it is not the same as your repository secrets, and its permissions can be restricted. [2] How to make secrets available (not “by default”): - You would need to use other approaches (commonly pull_request_target plus careful checkout/controls, or enabling additional fork settings such as approval and token-write options for private scenarios), but that is an explicit change to your security model rather than default behavior. [3]
Citations:
- 1: https://help.github.com/en/code-security/reference/secret-security/understanding-github-secret-types
- 2: https://docs.github.com/actions/security-guides/using-secrets-in-github-actions
- 3: https://stackoverflow.com/questions/76952023/how-to-make-github-actions-safely-access-secrets-for-prs-created-from-forks
Guard eval execution when secrets are unavailable (fork PRs).
This workflow runs on pull_request but unconditionally performs GCP auth. Fork PRs do not receive repository secrets by default, so the google-github-actions/auth step will fail and create red CI unrelated to code quality.
Suggested guard
behavioral-evals:
needs: detect-changed-plugins
- if: needs.detect-changed-plugins.outputs.has_plugins == 'true'
+ if: needs.detect-changed-plugins.outputs.has_plugins == 'true' && github.event.pull_request.head.repo.fork == false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/eval-plugins.yml around lines 40 - 65, The GCP auth step
(uses: google-github-actions/auth@...) in the behavioral-evals job runs
unconditionally and fails on forked PRs without secrets; add a guard to the
Authenticate to GCP step so it only runs when the needed secret is present
(e.g., add an if condition checking that secrets.GOOGLE_APPLICATION_CREDENTIALS
is non-empty or that the PR is not from a fork using
github.event.pull_request.head.repo.fork == false), ensuring the step is skipped
for forked PRs and CI no longer fails due to missing secrets.
There was a problem hiding this comment.
@coderabbitai Is there a way to allow this for collaborators?
There was a problem hiding this comment.
@stbenjam, yes! There are a few approaches depending on how much you want to open it up:
Option 1: Allow collaborators/members/owners (simplest)
You can check github.event.pull_request.author_association to permit collaborators even when they submit from a fork:
behavioral-evals:
needs: detect-changed-plugins
if: |
needs.detect-changed-plugins.outputs.has_plugins == 'true' &&
(github.event.pull_request.head.repo.fork == false ||
contains(fromJson('["COLLABORATOR","MEMBER","OWNER"]'), github.event.pull_request.author_association))There are two ways to check permissions — author association or user permissions. Author association tells us if they have any permissions on the repository (they'll be marked as a COLLABORATOR no matter what permissions they have). The author_association values COLLABORATOR, MEMBER, and OWNER all indicate the PR author has repo access.
Note: This still means the fork PR runner won't receive secrets automatically. Since this is
pull_request(notpull_request_target), the workflow runs in the fork's context where secrets aren't available. So even collaborator fork PRs would fail the GCP auth step. The cleanest path is Option 2 below.
Option 2: pull_request_target + GitHub Environment with required reviewers (safest)
To make this safe, run any job that uses secrets and is triggered by pull_request_target under a GitHub Environment. Make sure the environment is configured for a team or set of maintainers to be required to review. This blocks execution of those jobs until a repo maintainer inspects the code, verifies it isn't going to do anything malicious with the secrets, then "approves" the environment deployment.
Change the trigger:
on:
pull_request_target:And add an environment to the behavioral-evals job:
behavioral-evals:
environment: gcp-evals # requires maintainer approval for fork PRsYou can use a pull_request_target trigger to run a workflow from the context of your repo's main branch, but check out the fork PR's code. A limitation: changes to the workflow itself will not be seen until they are in your default branch, but this will run the workflow that exists on your default branch against any changes on the fork PR, using your repo's secrets.
Option 3: Repository settings — "Send secrets to workflows" (private repos only)
In 2020, GitHub introduced an option to send secrets to workflows from fork pull requests, but it only has effect on fork pull requests from private repositories.
Recommendation: Since these evals are expensive (GCP/Vertex AI costs) and only relevant for trusted contributors, Option 2 (pull_request_target + an approval environment) is the best approach — it lets collaborators get secrets while requiring a human review gate for truly external PRs.
| # --- Golden tests: known-correct classifications from SKILL.md examples --- | ||
|
|
||
| - description: "classify/nitpick-style — variable grouping nit" | ||
| metadata: &meta-fast |
There was a problem hiding this comment.
this would be computed/enforced via linter+skill
There was a problem hiding this comment.
♻️ Duplicate comments (4)
.github/workflows/eval-plugins.yml (2)
10-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet explicit least-privilege permissions on
detect-changed-plugins.This job currently relies on repository defaults for
GITHUB_TOKENpermissions. Make it explicit (contents: read) to avoid permission drift.Suggested fix
detect-changed-plugins: runs-on: ubuntu-latest + permissions: + contents: read outputs: plugins: ${{ steps.detect.outputs.plugins }} has_plugins: ${{ steps.detect.outputs.has_plugins }}Based on learnings: In GitHub Actions workflows, ensure required permissions (e.g.,
contents: read) are explicitly included whenever apermissionsblock exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/eval-plugins.yml around lines 10 - 12, The detect-changed-plugins job lacks an explicit permissions block; add a permissions: contents: read entry to the detect-changed-plugins job definition so the job uses least-privilege for GITHUB_TOKEN (reference job name detect-changed-plugins).
42-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard eval execution for fork PRs where secrets are unavailable.
On
pull_request, forked PRs won’t receive repository secrets, so auth at Line 61 fails and produces noisy red CI.Suggested fix
behavioral-evals: needs: detect-changed-plugins - if: needs.detect-changed-plugins.outputs.has_plugins == 'true' + if: needs.detect-changed-plugins.outputs.has_plugins == 'true' && github.event.pull_request.head.repo.fork == false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/eval-plugins.yml around lines 42 - 64, The workflow currently attempts to run the "Authenticate to GCP" step (uses: google-github-actions/auth) on pull_request events even for forked PRs where repository secrets are not available; add a conditional to skip authentication for forked PRs (e.g. set the step-level or job-level if to run only when not a fork: if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }}), so the google-github-actions/auth step (and/or the whole eval job) only runs when secrets are accessible.evals/AGENTS.md (1)
162-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the file-structure fenced block.
Line 162 opens a fenced code block without a language, which still trips markdownlint (MD040).
Suggested fix
-``` +```text plugins/ hello-world/evals/echo.yaml # 3 command output tests code-review/evals/classify-review-comment.yaml # 15 skill classification tests @@ .github/workflows/eval-plugins.yml # CI: evals on PRs with changed plugins</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@evals/AGENTS.mdat line 162, The fenced code block opened with triple
backticks in AGENTS.md (the block starting at the code fence around the
file-structure listing) lacks a language tag; change the opening fence fromtotext so the block is explicitly marked as plain text (i.e., update the
opening fence for the file-structure code block).</details> </blockquote></details> <details> <summary>Makefile (1)</summary><blockquote> `44-46`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **Fail fast when no eval configs are found and stop masking install failures.** Line 45 suppresses dependency install errors, and Line 46 can invoke sub-make with an empty target list, creating misleading green runs. <details> <summary>Suggested fix</summary> ```diff .PHONY: eval-plugins eval-plugins: ## Run plugin behavioral evals (EVAL_PLUGIN, EVAL_FILTER, EVAL_OUTPUT, EVAL_REPEAT) - `@npm` install 2>/dev/null || true + `@test` -n "$(strip $(EVAL_CONFIGS))" || { echo "No eval configs found"; exit 1; } + `@npm` install @$(MAKE) -j$(words $(EVAL_CONFIGS)) --no-print-directory $(EVAL_TARGETS) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 44 - 46, The eval-plugins Make target is masking failures and can invoke a sub-make with no targets; remove the "|| true" that suppresses npm install failures and add a guard that fails fast when $(EVAL_TARGETS) (or $(EVAL_CONFIGS)) is empty. Concretely: in the eval-plugins recipe, call npm install without "|| true", then test -n "$(EVAL_TARGETS)" (or test "$(words $(EVAL_TARGETS))" -gt 0) and if empty echo an error and exit 1, otherwise invoke $(MAKE) -j$(words $(EVAL_CONFIGS)) --no-print-directory $(EVAL_TARGETS); keep references to the eval-plugins target, EVAL_CONFIGS and EVAL_TARGETS so the change is easy to locate. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Duplicate comments:
In @.github/workflows/eval-plugins.yml:
- Around line 10-12: The detect-changed-plugins job lacks an explicit
permissions block; add a permissions: contents: read entry to the
detect-changed-plugins job definition so the job uses least-privilege for
GITHUB_TOKEN (reference job name detect-changed-plugins).- Around line 42-64: The workflow currently attempts to run the "Authenticate to
GCP" step (uses: google-github-actions/auth) on pull_request events even for
forked PRs where repository secrets are not available; add a conditional to skip
authentication for forked PRs (e.g. set the step-level or job-level if to run
only when not a fork: if: ${{ github.event_name != 'pull_request' ||
github.event.pull_request.head.repo.full_name == github.repository }}), so the
google-github-actions/auth step (and/or the whole eval job) only runs when
secrets are accessible.In
@evals/AGENTS.md:
- Line 162: The fenced code block opened with triple backticks in AGENTS.md (the
block starting at the code fence around the file-structure listing) lacks a
language tag; change the opening fence fromtotext so the block is
explicitly marked as plain text (i.e., update the opening fence for the
file-structure code block).In
@Makefile:
- Around line 44-46: The eval-plugins Make target is masking failures and can
invoke a sub-make with no targets; remove the "|| true" that suppresses npm
install failures and add a guard that fails fast when $(EVAL_TARGETS) (or
$(EVAL_CONFIGS)) is empty. Concretely: in the eval-plugins recipe, call npm
install without "|| true", then test -n "$(EVAL_TARGETS)" (or test "$(words
$(EVAL_TARGETS))" -gt 0) and if empty echo an error and exit 1, otherwise invoke
$(MAKE) -j$ (words$(EVAL_CONFIGS)) --no-print-directory $ (EVAL_TARGETS); keep
references to the eval-plugins target, EVAL_CONFIGS and EVAL_TARGETS so the
change is easy to locate.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `ffaf2a13-6790-4946-b1ce-d9667c8be42d` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1e5c22b0b136a3f1c588704afaf9081aaeda9c28 and 18b78ff185985b98b2406be84c9164256c9acacd. </details> <details> <summary>📒 Files selected for processing (17)</summary> * `.github/workflows/eval-plugins.yml` * `.gitignore` * `Makefile` * `evals/AGENTS.md` * `evals/promptfooconfig.yaml` * `package.json` * `plugins/code-review/evals/classify-review-comment.yaml` * `plugins/hello-world/evals/echo.yaml` * `plugins/jira/evals/fixtures/deprecated-endpoints.md` * `plugins/jira/evals/fixtures/empty-context.md` * `plugins/jira/evals/fixtures/missing-ac.md` * `plugins/jira/evals/fixtures/multi-area-feature.md` * `plugins/jira/evals/fixtures/rbac-middleware.md` * `plugins/jira/evals/fixtures/sparse-bug.md` * `plugins/jira/evals/fixtures/well-groomed.md` * `plugins/jira/evals/ready-to-solve.yaml` * `plugins/jira/evals/solve.yaml` </details> <details> <summary>✅ Files skipped from review due to trivial changes (10)</summary> * .gitignore * package.json * plugins/jira/evals/fixtures/empty-context.md * plugins/jira/evals/fixtures/multi-area-feature.md * plugins/hello-world/evals/echo.yaml * plugins/jira/evals/fixtures/rbac-middleware.md * plugins/jira/evals/fixtures/sparse-bug.md * plugins/jira/evals/fixtures/well-groomed.md * plugins/jira/evals/fixtures/missing-ac.md * plugins/jira/evals/fixtures/deprecated-endpoints.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (4)</summary> * evals/promptfooconfig.yaml * plugins/jira/evals/ready-to-solve.yaml * plugins/code-review/evals/classify-review-comment.yaml * plugins/jira/evals/solve.yaml </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
♻️ Duplicate comments (3)
evals/AGENTS.md (1)
179-195:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced code block.
Markdownlint requires fenced code blocks to specify a language. Use
text(orbash) for the file structure listing.Suggested fix
-``` +```text plugins/ hello-world/evals/ echo.yaml # 3 command output tests🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@evals/AGENTS.md` around lines 179 - 195, The fenced code block in AGENTS.md (the file-structure listing) lacks a language tag; update the opening fence from ``` to ```text (or ```bash) so Markdownlint passes—locate the multi-line block under the "plugins/" listing in AGENTS.md and add the language identifier on its starting backticks.Makefile (2)
43-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast when no eval configs are discovered.
If
EVAL_CONFIGSis empty due to typos or misconfiguration, the target silently succeeds without running any evaluations.Suggested fix
.PHONY: eval-plugins eval-plugins: ## Run plugin behavioral evals (EVAL_PLUGIN, EVAL_FILTER, EVAL_OUTPUT, EVAL_REPEAT) + `@test` -n "$(strip $(EVAL_CONFIGS))" || { echo "No eval configs found"; exit 1; } `@npm` install 2>/dev/null || true @$(MAKE) -j$(words $(EVAL_CONFIGS)) --no-print-directory $(EVAL_TARGETS)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 43 - 46, The eval-plugins target currently silently succeeds when EVAL_CONFIGS is empty; update the eval-plugins recipe to fail fast by checking EVAL_CONFIGS and exiting non-zero with a clear error message if it's empty. Modify the eval-plugins target (the rule named "eval-plugins" that references $(EVAL_CONFIGS) and $(EVAL_TARGETS)) to perform a guard at the start of the recipe (e.g., a shell test or Make conditional) that prints a helpful error like "No EVAL_CONFIGS found" and exits non-zero so the make invocation fails instead of proceeding.
45-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't suppress dependency install failures.
Silencing
npm installerrors with2>/dev/null || truecan cause evals to run with missing dependencies, producing misleading results or hiding actionable failures.Suggested fix
- `@npm` install 2>/dev/null || true + `@npm` ciUse
npm cifor reproducible installs in CI/make targets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` at line 45, Replace the silenced install command "@npm install 2>/dev/null || true" with a proper failing, reproducible install using "npm ci" (keep the leading @ if you want to suppress echoing); remove the stderr redirection and the "|| true" so failures surface in CI and local runs, ensuring missing dependency errors are not hidden.
🧹 Nitpick comments (1)
plugins/jira/evals/ready-to-solve.yaml (1)
64-67: 💤 Low valueConsider aligning fixture filename with test description.
The test description uses "short-context" but the fixture is named "empty-context.md". While both convey similar meaning, consistent naming between test descriptions and their fixtures improves maintainability.
📝 Suggested alignment
- description: "ready-to-solve/short-context — fails readiness" + description: "ready-to-solve/empty-context — fails readiness"or rename the fixture to
short-context.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/evals/ready-to-solve.yaml` around lines 64 - 67, The test description "ready-to-solve/short-context" and the fixture reference vars.description file://fixtures/empty-context.md are inconsistent; update either the description to "ready-to-solve/empty-context" or rename the fixture to short-context.md and update vars.description accordingly so the description and fixture name match (referencing the description string "ready-to-solve/short-context" and the vars.description file://fixtures/empty-context.md in the YAML).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@evals/AGENTS.md`:
- Around line 179-195: The fenced code block in AGENTS.md (the file-structure
listing) lacks a language tag; update the opening fence from ``` to ```text (or
```bash) so Markdownlint passes—locate the multi-line block under the "plugins/"
listing in AGENTS.md and add the language identifier on its starting backticks.
In `@Makefile`:
- Around line 43-46: The eval-plugins target currently silently succeeds when
EVAL_CONFIGS is empty; update the eval-plugins recipe to fail fast by checking
EVAL_CONFIGS and exiting non-zero with a clear error message if it's empty.
Modify the eval-plugins target (the rule named "eval-plugins" that references
$(EVAL_CONFIGS) and $(EVAL_TARGETS)) to perform a guard at the start of the
recipe (e.g., a shell test or Make conditional) that prints a helpful error like
"No EVAL_CONFIGS found" and exits non-zero so the make invocation fails instead
of proceeding.
- Line 45: Replace the silenced install command "@npm install 2>/dev/null ||
true" with a proper failing, reproducible install using "npm ci" (keep the
leading @ if you want to suppress echoing); remove the stderr redirection and
the "|| true" so failures surface in CI and local runs, ensuring missing
dependency errors are not hidden.
---
Nitpick comments:
In `@plugins/jira/evals/ready-to-solve.yaml`:
- Around line 64-67: The test description "ready-to-solve/short-context" and the
fixture reference vars.description file://fixtures/empty-context.md are
inconsistent; update either the description to "ready-to-solve/empty-context" or
rename the fixture to short-context.md and update vars.description accordingly
so the description and fixture name match (referencing the description string
"ready-to-solve/short-context" and the vars.description
file://fixtures/empty-context.md in the YAML).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b4b54ea4-78e7-4c94-85c8-599ba70b0ed1
📒 Files selected for processing (18)
.github/workflows/eval-plugins.yml.gitignoreMakefileevals/AGENTS.mdevals/budget.yamlevals/promptfooconfig.yamlpackage.jsonplugins/code-review/evals/classify-review-comment.yamlplugins/hello-world/evals/echo.yamlplugins/jira/evals/fixtures/deprecated-endpoints.mdplugins/jira/evals/fixtures/empty-context.mdplugins/jira/evals/fixtures/missing-ac.mdplugins/jira/evals/fixtures/multi-area-feature.mdplugins/jira/evals/fixtures/rbac-middleware.mdplugins/jira/evals/fixtures/sparse-bug.mdplugins/jira/evals/fixtures/well-groomed.mdplugins/jira/evals/ready-to-solve.yamlplugins/jira/evals/solve.yaml
✅ Files skipped from review due to trivial changes (10)
- .gitignore
- package.json
- plugins/jira/evals/fixtures/empty-context.md
- plugins/jira/evals/fixtures/missing-ac.md
- plugins/hello-world/evals/echo.yaml
- evals/budget.yaml
- plugins/jira/evals/fixtures/well-groomed.md
- plugins/jira/evals/fixtures/rbac-middleware.md
- plugins/jira/evals/fixtures/deprecated-endpoints.md
- plugins/jira/evals/fixtures/multi-area-feature.md
🚧 Files skipped from review as they are similar to previous changes (4)
- plugins/jira/evals/fixtures/sparse-bug.md
- evals/promptfooconfig.yaml
- plugins/jira/evals/solve.yaml
- plugins/code-review/evals/classify-review-comment.yaml
|
/run-evals |
4617d34 to
b5dcc95
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
plugins/jira/evals/solve.yaml (1)
3-30:⚠️ Potential issue | 🟠 MajorThese analysis-only tests don't verify actual skill invocation.
The past review comment concern remains unaddressed: these evals test the agent's reasoning about
/jira:solvebut never verify the skill executes correctly. All four test prompts explicitly instruct "do NOT actually implement" or "Just analyze", soskill-used: jira:solvewould fail.While phase-level analysis tests have value, they create a testing gap — the actual jira:solve skill behavior is never exercised in this eval file. A passing eval suite gives false confidence that the plugin works when only its documentation/planning was tested.
Do you want me to help design complementary integration tests that invoke
/jira:solvewith controlled fixtures to verify end-to-end behavior?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/evals/solve.yaml` around lines 3 - 30, The evals currently only run analysis-only prompts and never exercise the actual jira:solve skill; update the eval suite to include complementary integration-style tests that call the skill end-to-end: add new prompt cases (in the same prompts/defaultTest group) that do not contain "do NOT actually implement" language, include assertions that check skill invocation (e.g., set "skill-used: jira:solve" and validate expected outputs/state), and provide controlled fixtures/inputs to deterministically exercise the /jira:solve handler so the evaluation verifies real behavior rather than only planning; keep the existing analysis tests but add these integration tests alongside them so both planning and execution are validated.evals/AGENTS.md (1)
179-195:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to satisfy markdownlint.
The fenced code block showing file structure lacks a language identifier.
Suggested fix
-``` +```text plugins/ hello-world/evals/ echo.yaml # 3 command output tests🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@evals/AGENTS.md` around lines 179 - 195, The markdown fenced code block showing the file tree is missing a language tag; update the opening fence (the ``` line before the "plugins/" tree) to include a language identifier (e.g., ```text) so markdownlint passes; ensure you only change the opening fence for the code block in AGENTS.md that wraps the plugins/ ... package.json listing.Makefile (2)
45-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't hide npm dependency install failures.
The suppression of stderr and
|| truemasks real installation errors, which can cause evals to fail for the wrong reason or produce confusing errors downstream.Suggested fix
- `@npm` install 2>/dev/null || true + `@npm` ciUse
npm cifor reproducible CI-friendly installs that respectpackage-lock.jsonand fail fast on errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` at line 45, The Makefile currently suppresses npm install failures by using the command string "@npm install 2>/dev/null || true"; update this to run a reproducible, CI-safe install and fail fast: replace the suppressed install with "npm ci" (keep the leading @ if you want to silence echoing) and remove the stderr redirection and "|| true" so the rule fails on error and surfaces install problems immediately.
43-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast when no eval configs are discovered.
Currently, if
EVAL_CONFIGSis empty (due to typo inEVAL_PLUGINor missing eval files), the target completes successfully without running any tests. This hides configuration mistakes.Suggested guard
.PHONY: eval-plugins eval-plugins: ## Run plugin behavioral evals (EVAL_PLUGIN, EVAL_FILTER, EVAL_OUTPUT, EVAL_REPEAT) + `@test` -n "$(strip $(EVAL_CONFIGS))" || { echo "ERROR: No eval configs found for EVAL_PLUGIN=$(or $(EVAL_PLUGIN),*)"; exit 1; } `@npm` install 2>/dev/null || true @$(MAKE) -j$(words $(EVAL_CONFIGS)) --no-print-directory $(EVAL_TARGETS)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 43 - 46, The eval-plugins target currently silently succeeds when EVAL_CONFIGS is empty; update the eval-plugins recipe to fail fast by testing EVAL_CONFIGS at the start and exiting non-zero with a clear error message if it is empty (e.g., check EVAL_CONFIGS in the eval-plugins target and abort with an error to prevent running zero tests); reference the Makefile target name eval-plugins and the variables EVAL_CONFIGS and EVAL_TARGETS when making the change.
🧹 Nitpick comments (1)
Makefile (1)
49-62: 💤 Low valueConsider validating required environment variables early.
The eval runs expect
ANTHROPIC_VERTEX_PROJECT_IDto be set (referenced in the eval configs and docs). Currently, if it's unset, the expensive eval runs start and fail mid-flight with cryptic errors.Suggested validation
$(EVAL_TARGETS): $(eval CONFIG := $(subst __,/,$(patsubst _run-eval__%,%,$@))) $(eval EVAL_NAME := $(basename $(notdir $(CONFIG)))) + `@test` -n "$(ANTHROPIC_VERTEX_PROJECT_ID)" || { echo "ERROR: ANTHROPIC_VERTEX_PROJECT_ID must be set"; exit 1; } `@echo` "=== Running eval: $(CONFIG) ===" `@CLAUDE_CODE_USE_VERTEX`=true \Alternatively, add the check once in the
eval-pluginstarget before spawning parallel jobs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 49 - 62, The Makefile runs under the $(EVAL_TARGETS) recipe but doesn't validate that required env vars like ANTHROPIC_VERTEX_PROJECT_ID are set, causing long failing evals; add an early check in the Makefile (either at the top of the $(EVAL_TARGETS) recipe or in the eval-plugins target invoked before parallel jobs) that tests required variables (e.g., ANTHROPIC_VERTEX_PROJECT_ID) and prints a clear error and exits nonzero if missing so the npx promptfoo eval command block is never executed with missing configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/eval-plugins.yml:
- Around line 11-16: The detect-changed-plugins job lacks an explicit
permissions block, so add one granting the needed GitHub token scopes (e.g., set
permissions: contents: read) because this job runs gh pr view and uses
actions/checkout; update the job named detect-changed-plugins to include a
permissions section that at minimum declares contents: read (and any other
minimal scopes required) so the commands succeed even when other workflow jobs
define permissions.
- Around line 67-89: The workflow currently checks out untrusted PR code and
runs it with GCP secrets in the "Checkout PR code" and "Run behavioral evals for
${{ matrix.plugin }}" steps (invoking make eval-plugins), which enables
exfiltration of secrets.GOOGLE_APPLICATION_CREDENTIALS and
ANTHROPIC_VERTEX_PROJECT_ID; fix by removing execution of checked-out PR code
with secrets: either (A) replace the checkout+make run with a pinned/trusted
Docker eval-runner image that contains the eval logic (mount only plugin configs
read-only and invoke evaluation inside that image) so PRs cannot change runtime
code, or (B) change the job to run under pull_request_target and require a
GitHub Environment/manual approval before using secrets, ensuring the job uses
the base repo image/entrypoint (not PR Makefile) to invoke evals (reference the
"Checkout PR code", "Run behavioral evals for ${{ matrix.plugin }}", and the
make eval-plugins invocation).
In `@evals/AGENTS.md`:
- Around line 102-133: The documentation has a test-count mismatch: the
inventory table's "Total" cell shows 25 while the summary line reads "Measured
cost per full run (26 tests, opus agent, Vertex AI)"; verify the actual test
list (hello-world/echo, classify golden, classify ambiguous,
jira/ready-to-solve, jira/solve) and then either (A) update the table "Total"
cell to **26** to match the measured-cost line or (B) change the measured-cost
parenthetical from "26 tests" to "25 tests" (or add a clarifying note if an
extra test is intentionally excluded/included) so both counts align. Ensure the
numeric change is applied to the text "Total" in the inventory table and the
measured-cost sentence text.
---
Duplicate comments:
In `@evals/AGENTS.md`:
- Around line 179-195: The markdown fenced code block showing the file tree is
missing a language tag; update the opening fence (the ``` line before the
"plugins/" tree) to include a language identifier (e.g., ```text) so
markdownlint passes; ensure you only change the opening fence for the code block
in AGENTS.md that wraps the plugins/ ... package.json listing.
In `@Makefile`:
- Line 45: The Makefile currently suppresses npm install failures by using the
command string "@npm install 2>/dev/null || true"; update this to run a
reproducible, CI-safe install and fail fast: replace the suppressed install with
"npm ci" (keep the leading @ if you want to silence echoing) and remove the
stderr redirection and "|| true" so the rule fails on error and surfaces install
problems immediately.
- Around line 43-46: The eval-plugins target currently silently succeeds when
EVAL_CONFIGS is empty; update the eval-plugins recipe to fail fast by testing
EVAL_CONFIGS at the start and exiting non-zero with a clear error message if it
is empty (e.g., check EVAL_CONFIGS in the eval-plugins target and abort with an
error to prevent running zero tests); reference the Makefile target name
eval-plugins and the variables EVAL_CONFIGS and EVAL_TARGETS when making the
change.
In `@plugins/jira/evals/solve.yaml`:
- Around line 3-30: The evals currently only run analysis-only prompts and never
exercise the actual jira:solve skill; update the eval suite to include
complementary integration-style tests that call the skill end-to-end: add new
prompt cases (in the same prompts/defaultTest group) that do not contain "do NOT
actually implement" language, include assertions that check skill invocation
(e.g., set "skill-used: jira:solve" and validate expected outputs/state), and
provide controlled fixtures/inputs to deterministically exercise the /jira:solve
handler so the evaluation verifies real behavior rather than only planning; keep
the existing analysis tests but add these integration tests alongside them so
both planning and execution are validated.
---
Nitpick comments:
In `@Makefile`:
- Around line 49-62: The Makefile runs under the $(EVAL_TARGETS) recipe but
doesn't validate that required env vars like ANTHROPIC_VERTEX_PROJECT_ID are
set, causing long failing evals; add an early check in the Makefile (either at
the top of the $(EVAL_TARGETS) recipe or in the eval-plugins target invoked
before parallel jobs) that tests required variables (e.g.,
ANTHROPIC_VERTEX_PROJECT_ID) and prints a clear error and exits nonzero if
missing so the npx promptfoo eval command block is never executed with missing
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 68847dcd-08c5-4992-9426-1e479ab4f6ec
📒 Files selected for processing (18)
.github/workflows/eval-plugins.yml.gitignoreMakefileevals/AGENTS.mdevals/budget.yamlevals/promptfooconfig.yamlpackage.jsonplugins/code-review/evals/classify-review-comment.yamlplugins/hello-world/evals/echo.yamlplugins/jira/evals/fixtures/deprecated-endpoints.mdplugins/jira/evals/fixtures/empty-context.mdplugins/jira/evals/fixtures/missing-ac.mdplugins/jira/evals/fixtures/multi-area-feature.mdplugins/jira/evals/fixtures/rbac-middleware.mdplugins/jira/evals/fixtures/sparse-bug.mdplugins/jira/evals/fixtures/well-groomed.mdplugins/jira/evals/ready-to-solve.yamlplugins/jira/evals/solve.yaml
✅ Files skipped from review due to trivial changes (10)
- plugins/jira/evals/fixtures/deprecated-endpoints.md
- package.json
- plugins/jira/evals/fixtures/sparse-bug.md
- plugins/jira/evals/fixtures/multi-area-feature.md
- plugins/jira/evals/fixtures/missing-ac.md
- plugins/jira/evals/fixtures/well-groomed.md
- plugins/jira/evals/fixtures/empty-context.md
- plugins/jira/evals/fixtures/rbac-middleware.md
- .gitignore
- evals/budget.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- evals/promptfooconfig.yaml
- plugins/hello-world/evals/echo.yaml
- plugins/jira/evals/ready-to-solve.yaml
- plugins/code-review/evals/classify-review-comment.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/eval-plugins.yml (1)
79-103:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftUntrusted code execution with credentials remains unaddressed.
The past review correctly identified that this workflow checks out PR code (including the Makefile) and executes it with GCP credentials. CodeQL's TOCTOU warning confirms this risk. While the MEMBER/COLLABORATOR gate (lines 20-21) provides some protection, a compromised or malicious collaborator could modify the Makefile to exfiltrate
GOOGLE_APPLICATION_CREDENTIALSandANTHROPIC_VERTEX_PROJECT_ID.The past review suggested two mitigation approaches:
- Use a pinned container image that only mounts configs read-only
- Switch to
pull_request_targetwith a manual approval environment🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/eval-plugins.yml around lines 79 - 103, The workflow currently checks out PR code and runs the Makefile target "make eval-plugins" in the "Run behavioral evals for ${{ matrix.plugin }}" step while authenticated via google-github-actions/auth and exposing secrets GOOGLE_APPLICATION_CREDENTIALS and ANTHROPIC_VERTEX_PROJECT_ID, which allows a malicious PR to exfiltrate credentials; fix by removing execution of untrusted PR code with secrets: either run the evals inside a pinned, minimal container image (instead of checking out PR content) that mounts repository/configs read-only and does not receive secrets, or switch the workflow trigger to pull_request_target and gate the eval job behind a required manual approval environment so only trusted code runs with google-github-actions/auth; ensure the "Checkout code" / actions/checkout step no longer exposes secrets to untrusted refs and that "make eval-plugins" is executed only from trusted/main branch or inside the pinned container.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/eval-plugins.yml:
- Around line 79-103: The workflow currently checks out PR code and runs the
Makefile target "make eval-plugins" in the "Run behavioral evals for ${{
matrix.plugin }}" step while authenticated via google-github-actions/auth and
exposing secrets GOOGLE_APPLICATION_CREDENTIALS and ANTHROPIC_VERTEX_PROJECT_ID,
which allows a malicious PR to exfiltrate credentials; fix by removing execution
of untrusted PR code with secrets: either run the evals inside a pinned, minimal
container image (instead of checking out PR content) that mounts
repository/configs read-only and does not receive secrets, or switch the
workflow trigger to pull_request_target and gate the eval job behind a required
manual approval environment so only trusted code runs with
google-github-actions/auth; ensure the "Checkout code" / actions/checkout step
no longer exposes secrets to untrusted refs and that "make eval-plugins" is
executed only from trusted/main branch or inside the pinned container.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 16f48b76-52f1-4107-8194-4fc2b4e7f153
📒 Files selected for processing (18)
.github/workflows/eval-plugins.yml.gitignoreMakefileevals/AGENTS.mdevals/budget.yamlevals/promptfooconfig.yamlpackage.jsonplugins/code-review/evals/classify-review-comment.yamlplugins/hello-world/evals/echo.yamlplugins/jira/evals/fixtures/deprecated-endpoints.mdplugins/jira/evals/fixtures/empty-context.mdplugins/jira/evals/fixtures/missing-ac.mdplugins/jira/evals/fixtures/multi-area-feature.mdplugins/jira/evals/fixtures/rbac-middleware.mdplugins/jira/evals/fixtures/sparse-bug.mdplugins/jira/evals/fixtures/well-groomed.mdplugins/jira/evals/ready-to-solve.yamlplugins/jira/evals/solve.yaml
✅ Files skipped from review due to trivial changes (8)
- plugins/jira/evals/fixtures/deprecated-endpoints.md
- .gitignore
- plugins/jira/evals/fixtures/empty-context.md
- plugins/jira/evals/fixtures/missing-ac.md
- plugins/jira/evals/fixtures/well-groomed.md
- plugins/jira/evals/fixtures/sparse-bug.md
- plugins/jira/evals/fixtures/multi-area-feature.md
- evals/AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (7)
- plugins/jira/evals/fixtures/rbac-middleware.md
- package.json
- plugins/hello-world/evals/echo.yaml
- plugins/code-review/evals/classify-review-comment.yaml
- plugins/jira/evals/ready-to-solve.yaml
- evals/promptfooconfig.yaml
- evals/budget.yaml
Add a promptfoo-based eval framework for testing plugin skills and commands. Uses the claude-agent-sdk provider with Vertex AI auth and co-locates eval configs inside each plugin directory. Behavioral evals cover three plugins: - hello-world: 3 command output tests - code-review: 15 skill classification tests with skill-used assertions - jira: 7 tests across ready-to-solve and solve commands Makefile targets: eval-plugins (EVAL_PLUGIN, EVAL_FILTER, EVAL_TIER, EVAL_REPEAT). CI workflow runs changed-plugin evals on PRs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
025df35 to
09a2c4e
Compare
- hello-world: 1.0.1 → 1.0.2 - code-review: 0.0.7 → 0.0.8 - jira: 0.4.4 → 0.4.5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This looks like a great foundation I'll have a PR up later today for enforcing the budgets /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@enxebre: This pull request references HPNEX-9 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Summary
Add a promptfoo-based eval framework for testing plugin skills and commands. Uses the
anthropic:claude-agent-sdkprovider with Vertex AI auth, which spawns the Claude Code CLI as a subprocess for full agent behavior (tools, skills, plugins).Design decisions
plugins/<name>/evals/*.yaml), not in a central directoryskill-used/not-skill-usedassertions that verify skill routingoutput_format: structured output bypasses Skill tool invocation, breakingskill-usedassertions — we useicontains+llm-rubricinsteadfile://fixtures/<name>.md— plain markdown, not JSONmake eval-pluginsruns all eval files simultaneously via$(MAKE) -j— wall-clock time equals the slowest plugin, not the sumevals/budget.yamldefines token-usage sizes, tier computation rules, and per-plugin budget capsevals/AGENTS.mdfor full architecture documentationBehavioral evals (3 plugins, 25 test cases)
skill-used,not-skill-used, andicontainsgolden test assertionsllm-rubricjudgingAssertion types used
skill-used/not-skill-used— skill routing verificationicontains/not-icontains— deterministic output matchingllm-rubric— LLM-judged quality (graded byvertex:claude-opus-4-6)cost/latency— regression guards with per-plugin thresholdsTest metadata and tiering
Every test carries per-test metadata for filtering by cost profile via
--filter-metadata:Tier computation:
Current inventory:
Cost and latency thresholds
Per-plugin budgets (
evals/budget.yaml)allowed= admin-set cap.current= sum of cost thresholds across all tests.Measured actual cost per full run: ~$6 (thresholds are 2x safety margin over actuals)
Makefile targets
All eval files run in parallel by default via
$(MAKE) -j.CI workflow
GH Actions runs on every PR:
evals/directoriesdorny/test-reporterTest plan
make eval-plugins EVAL_PLUGIN=hello-world— 3/3 passedmake eval-plugins EVAL_PLUGIN=code-review— 15/15 passedmake eval-plugins EVAL_PLUGIN=jira— 7/7 passedmake eval-pluginsfull parallel run — 25/25 passedmake eval-plugins EVAL_PLUGIN=jira EVAL_FILTER=commit-strategy EVAL_REPEAT=10 EVAL_PASS_RATE_THRESHOLD=95— 10/10 passedmake eval-plugins EVAL_TIER=fast— 16/16 passed (tier filtering works)make eval-plugins EVAL_PLUGIN=code-review EVAL_TIER=medium— 2/2 passedEVAL_OUTPUT_DIR=./eval-results make eval-plugins— JUnit XML files generatedmake lintpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores