feat: add promptfoo integration for prompt evaluation - #1
Open
jcpowermac wants to merge 2 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 30 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR adds a complete Promptfoo-based evaluation framework for the SDLC agent. It includes configurations that define how prompts are evaluated, a custom provider bridging Promptfoo to the project's LLM and template pipeline, assertion validators for output validation, test fixtures exercising repo identification and review workflows, and Makefile targets for running evaluations locally and in CI. ChangesEvaluation Framework
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Author
|
Haven't tested this yet... |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
evals/assertions/check_tier_ordering.py (1)
36-36: 💤 Low value
s["step"]assumes the key always exists.If model output omits
stepfor any entry, this comprehension (and the laterstep['step']on line 50) raisesKeyErrorinstead of producing an assertion failure. Since the goal is to grade output, prefer.get("step")and skip malformed entries, or guard explicitly.🤖 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/assertions/check_tier_ordering.py` at line 36, The comprehension building step_map assumes every entry in steps has a "step" key and will KeyError; change the logic in check_tier_ordering to use s.get("step") and skip entries where it's missing (e.g., only include entries with a truthy step value) and update later access sites that use step['step'] to use step.get("step") or guard with an explicit check so malformed entries produce an assertion failure instead of raising KeyError; specifically update the creation of step_map and any subsequent uses of step['step'] to handle missing keys safely.evals/provider.py (1)
93-93: 💤 Low value
__template_pathpop has no fallback.
vars_.pop("__template_path")raises an uncaughtKeyErrorif a fixture omits the key, surfacing as an opaque provider crash rather than a clear assertion error. A guarded pop with a descriptive error keeps eval failures diagnosable.♻️ Optional guard
- template_path = vars_.pop("__template_path") + template_path = vars_.pop("__template_path", None) + if not template_path: + return {"error": "missing __template_path in test vars"}🤖 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/provider.py` at line 93, The code calls vars_.pop("__template_path") which raises a KeyError if the fixture omits the key; change this to safely retrieve the value and raise a clear assertion on missing input. Replace the direct pop with a guarded retrieval (e.g., use vars_.pop("__template_path", None) or vars_.get("__template_path")) and then assert/template-check for None and raise a descriptive AssertionError mentioning "__template_path" so failures in provider.py (where template_path is used) surface as clear test/fixture errors rather than an opaque KeyError.
🤖 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 `@evals/assertions/check_repo_names.py`:
- Line 27: The list comprehension building invalid repo names uses r.get("name",
"") for the predicate but then accesses r["name"], which can raise KeyError for
repos missing the name key; update the comprehension so it consistently uses
r.get("name", "") (or extract name = r.get("name", "") into a local variable)
when testing against _VALID_ORG_RE and when producing the invalid list
(referencing the variable/ r.get call in the expression that assigns invalid).
In `@evals/promptfooconfig.yaml`:
- Line 13: The config currently sets llm_api_base to an empty string when
LLM_API_BASE is unset, which then overrides BaseAgentSettings.llm_api_base;
change the YAML to stop injecting an explicit empty string (e.g. replace
"${LLM_API_BASE:-}" with "${LLM_API_BASE}" or remove the default) so the value
is not forwarded as "" and/or add normalization in provider._make_settings to
treat "" as None (check llm_api_base and set to None when it is an empty string)
to preserve the intended BaseAgentSettings.llm_api_base default.
In `@evals/provider.py`:
- Around line 67-72: In _make_settings, normalize an empty llm_api_base string
to None so the LiteLLM client gets the provider default instead of an empty URL:
retrieve llm_api_base from config (the "llm_api_base" key) and if it's an empty
string convert it to None before passing it into _EvalSettings (leave None
unchanged and pass through any non-empty string); update the llm_api_base
argument construction in _make_settings accordingly so litellm_model and
llm_api_key behavior remains the same.
In `@Makefile`:
- Around line 166-176: Makefile targets eval, eval-view, eval-compare, and
eval-ci currently call npx -y promptfoo@latest which can drift; update these
targets to use a pinned promptfoo version (e.g., define a PROMPTFOO_VERSION
variable like PROMPTFOO_VERSION := 0.121.14 and replace promptfoo@latest with
promptfoo@$(PROMPTFOO_VERSION)) so CI and local evals are reproducible; ensure
all four targets reference the variable consistently.
---
Nitpick comments:
In `@evals/assertions/check_tier_ordering.py`:
- Line 36: The comprehension building step_map assumes every entry in steps has
a "step" key and will KeyError; change the logic in check_tier_ordering to use
s.get("step") and skip entries where it's missing (e.g., only include entries
with a truthy step value) and update later access sites that use step['step'] to
use step.get("step") or guard with an explicit check so malformed entries
produce an assertion failure instead of raising KeyError; specifically update
the creation of step_map and any subsequent uses of step['step'] to handle
missing keys safely.
In `@evals/provider.py`:
- Line 93: The code calls vars_.pop("__template_path") which raises a KeyError
if the fixture omits the key; change this to safely retrieve the value and raise
a clear assertion on missing input. Replace the direct pop with a guarded
retrieval (e.g., use vars_.pop("__template_path", None) or
vars_.get("__template_path")) and then assert/template-check for None and raise
a descriptive AssertionError mentioning "__template_path" so failures in
provider.py (where template_path is used) surface as clear test/fixture errors
rather than an opaque KeyError.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: adbbc3e1-e213-45c5-bc89-3cd35d5d6f72
📒 Files selected for processing (10)
.gitignoreMakefileevals/assertions/check_repo_names.pyevals/assertions/check_tier_ordering.pyevals/assertions/validate_pydantic.pyevals/fixtures/identify_repos.yamlevals/fixtures/run_review.yamlevals/promptfooconfig.compare.yamlevals/promptfooconfig.yamlevals/provider.py
Adds a promptfoo-based eval harness that tests prompt templates against real LLM outputs using the existing render + complete pipeline via a custom Python provider. Supports model comparison, parameter sweeps, and CI gating. New files: - evals/provider.py — custom provider bridging promptfoo to prompts.render() + llm.complete() - evals/promptfooconfig.yaml — single-model eval config (reads LITELLM_MODEL from env) - evals/promptfooconfig.compare.yaml — multi-model comparison config - evals/assertions/ — Pydantic schema validation, tier ordering, repo name checks - evals/fixtures/ — test cases for identify_repos (3) and run_review (3) Makefile targets: eval, eval-view, eval-compare, eval-ci Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jcpowermac
force-pushed
the
feat/promptfoo-eval
branch
from
June 3, 2026 19:48
3d7f503 to
9d602de
Compare
Adds evals/llama-swap.example.yaml with four models (Qwen3-14B, Qwen3-30B-A3B, Gemma-4-12B, Gemma-3-12B) so eval-compare can run against a single GPU via automatic model swapping. Updates the compare config to default to llama-swap's port (8080). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Adds a promptfoo-based eval harness that tests prompt templates against real LLM outputs using the existing render + complete pipeline via a custom Python provider. Supports model comparison, parameter sweeps, and CI gating.
New files:
Makefile targets: eval, eval-view, eval-compare, eval-ci
Summary by CodeRabbit
New Features
makecommands:eval,eval-view,eval-compare, andeval-ciChores
.gitignoreto exclude evaluation directories and temporary build artifacts