evals supports multiple skill sets (path | paths) - #779
Conversation
Resolves #755 finding 6. `evals.path` was a single string, but a persona that wraps several skills has several eval sets — pr-review's held-out sets live at evals/deep-review/ and evals/triage/, and the single-path shape cannot express that, blocking its onboarding. Change: `evals` now requires EXACTLY ONE of `path` (one set, unchanged — qa-lead keeps it) or `paths` (an array, one per wrapped skill). A oneOf enforces mutual exclusivity; verified path-only PASSES, paths-only PASSES, both FAILS, neither FAILS. Backward-compatible: qa-lead's `path: evals/qa-lead/` is untouched, so no cross-repo ordering is needed. Also tightened the min_cases contract: it is per-set (each listed set must independently clear it) and is now described as enforced by validate-personas.py once status reaches required_before — the count half of principle 5, which the companion validator PR in .github-private makes real. Verification - valid Draft 2020-12 schema; diff is additive (no reformat). - oneOf behaviour verified against all four combinations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe persona schema updates the ChangesPersona evaluation validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the persona JSON schema to support either a single evaluation path (path) or multiple evaluation paths (paths) using a oneOf validation constraint. The review feedback suggests two improvements to the schema: adding "uniqueItems": true to the paths array to prevent duplicate entries, and simplifying the oneOf definition by removing redundant not constraints to improve clarity and error messaging.
There was a problem hiding this comment.
Pull request overview
Updates the persona manifest JSON Schema to support personas that reference either a single eval set or multiple eval sets, enabling multi-skill personas to model distinct held-out eval directories while keeping existing single-path personas compatible.
Changes:
- Makes
evalsrequirerequired_beforeandmin_cases, and requires exactly one ofevals.pathorevals.paths. - Adds
evals.paths(array) withminItems: 1and updates schema descriptions to clarify per-setmin_casesexpectations.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@standards/personas/persona.schema.json`:
- Around line 256-259: The oneOf constraints redundantly exclude the alternate
property with explicit not clauses. Simplify the oneOf block in the persona
schema to require path in one branch and paths in the other, relying on oneOf to
reject configurations containing both while preserving the existing
exactly-one-of behavior.
- Around line 267-272: Update the paths array schema to enforce unique entries
by adding the uniqueItems constraint alongside minItems and items. Keep the
existing path descriptions and validation behavior unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72d7fcd4-f3bf-4272-bf41-55136aee36ff
📒 Files selected for processing (1)
standards/personas/persona.schema.json
|
@coderabbitai resolve |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #779 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
✅ Action performedComments resolved and changes approved. |
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: c46f00d287efb6c352b834175453935dc9562949
Review mode: triage-approved (single reviewer)
Summary
Single-file JSON Schema change to standards/personas/persona.schema.json (+16/−5): the evals gate now requires exactly one of path (single eval set) or paths (array, one per wrapped skill) via a oneOf, unblocking multi-skill persona onboarding (pr-review, business-analyst). Backward compatible — existing path-only manifests (qa-lead) validate unchanged.
Linked issue analysis
Linked issue #755, finding 6 ("evals.path is a single path, but the eval tree is skill-keyed — blocks pr-review onboarding. Schema gap.") is substantively and precisely addressed: paths lets a persona reference several skill-keyed eval sets (e.g. evals/deep-review/ + evals/triage/), and min_cases is documented as per-set. The issue covers 8+ findings; this PR correctly scopes to finding 6 only.
Findings
No blocking findings.
- Verified independently: fetched the schema at
c46f00d287efb6c352b834175453935dc9562949and ran the validation matrix with jsonschema Draft 2020-12 — path-only PASS, paths-only PASS, both FAIL, neither FAIL, emptypathsFAIL, duplicatepathsFAIL. Matches the PR description's claims exactly. - Prior bot feedback resolved: CodeRabbit's two actionable comments (add
uniqueItems: true; drop redundantnotclauses insideoneOf) are both applied in the current head; Gemini and Copilot raised the same points. All 5 review threads are resolved and CodeRabbit's changes-requested review was dismissed after resolution. - Secret scan:
run_secret_scanningMCP tool not available in this environment; gitleaks CI check passed, and the diff is schema JSON with no secret-like content. - Risk rationale (MEDIUM): schema contract change affecting org-wide persona validation — non-trivial logic, but additive, mutually-exclusive by construction, and backward compatible. No auth/CI/security surface touched.
- Note:
min_casesper-set enforcement is delegated tovalidate-personas.pyin a companion .github-private PR — the schema-side description is accurate about that division of labor.
CI status
All checks green at c46f00d287efb6c352b834175453935dc9562949: Lint, ShellCheck, CodeQL (actions), SonarCloud quality gate, Agent Security Scan, agent-shield, Secret scan (gitleaks), npm audit — all SUCCESS. Ecosystem-specific audits (pip/cargo/govulncheck/pnpm) skipped (not applicable). No failing or pending checks.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: c46f00d287efb6c352b834175453935dc9562949
Review mode: triage-approved (single reviewer)
Summary
Single-file additive change to standards/personas/persona.schema.json: evals now accepts exactly one of path (single eval set) or paths (array, one per wrapped skill), enforced via oneOf. required_before/min_cases semantics clarified (per-set enforcement by validate-personas.py). Backward compatible — existing single-path personas (qa-lead) are unaffected.
Linked issue analysis
Resolves finding 6 of #755 ("evals.path is a single path, but the eval tree is skill-keyed — blocks pr-review onboarding"). The paths array directly closes that gap by letting multi-skill personas reference their existing skill-keyed eval sets (e.g. evals/deep-review/ + evals/triage/). Substantively addressed.
Findings
- Schema at head SHA is valid JSON; oneOf correctly enforces mutual exclusivity of path/paths (both-present fails, neither fails).
- Bot reviewer suggestions were incorporated: paths has uniqueItems: true and minItems: 1; redundant not-clauses were removed in favor of plain oneOf.
- All 5 review threads (gemini, copilot, coderabbit) are resolved; CodeRabbit's changes-requested review was dismissed after fixes.
- Triage assessment confirmed: additive, docs+validation-shape change with no executable code, no auth/secrets/CI-workflow surface.
- MCP secret-scanning tool unavailable in this run; gitleaks CI check passed (no secret-bearing content in the diff).
CI status
All required checks green: Lint, ShellCheck, CodeQL, Agent Security Scan, gitleaks secret scan, SonarCloud quality gate, agent-shield, npm audit. Ecosystem-specific audits (pip/cargo/go/pnpm) skipped as not applicable.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
* feat(personas): onboard dev-lead, pr-review, business-analyst as personas Formalizes the three existing agents under the persona standard and makes each addressable as @petry-projects/<role> (org team handles — teams created: dev-lead, pr-review, business-analyst; all closed, notifications disabled, 0 members). Closes the manifest half of #1281 / #1282 / #1283. Each persona (all status: draft): - personas/<id>/persona.yml — a compliant manifest documenting the REAL runtime: definition layers point at what exists (dev-lead → prompts/dev-lead/; pr-review → agents/pr-reviewer.md + prompts/; business-analyst → vendored bmad-method analyst v6.8.0 + prompts/incubation/). Triggers reflect reality, advisory by default, every write surface carries its gate_label (§4 rule 2). - prompts/<id>/advisory.md — a role-focused advisory the shared persona runner serves on @-mention (read-only, prints between sentinels, marker-first — the qa-lead contract). dev-lead advises on implementation/scoping (code is still written only by its label-triggered runtime, not this path); pr-review gives review guidance (the full tiered review runs on its existing path); business-analyst advises on framing/research/next-analysis-step. - README.md + seed held-out evals (draft-exempt from the count gate; expand before promotion). pr-review points evals.paths at its EXISTING skill sets (evals/deep-review/, evals/triage/) — the multi-skill case #779 unblocks. Decisions taken (from the onboarding issues): - @-addressability for all three (the initiative's goal). pr-review's legacy @donpetry-bot path is retained (real account, works); this adds the role handle. - business-analyst is the Analyst role (brainstorm/market-research/brief) ONLY; the PRD stays with a separate product-manager persona — the documented Analyst→PM handoff is preserved, not collapsed. - pr-review's canary-rings registration (#710) stays a separate follow-up; draft personas don't require it. Deliberately NOT here (tracked, per-issue): real (non-seed) eval sets; the product-manager persona; pr-review canary registration; and — for the mention paths to go live — enrolling these into the router soak (the framework is live only in the next ring today). MERGE ORDER: after petry-projects/.github#779 (schema: evals.paths) and #1302 (validator: reads paths + count gate). pr-review's manifest uses `paths`, which main's validator cannot parse until #1302 lands. Verification - validate-personas.py (with #779 schema + #1302 validator): 4/4 valid, invariants hold. - validate-cases.py: OK, 7 skills / 54 cases, no cross-split id overlap. - markdownlint: clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: dev-lead update (review-changes) [skip ci-relay] * chore: dev-lead update (review-changes) [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: c46f00d287efb6c352b834175453935dc9562949
Review mode: triage-approved (single reviewer)
Summary
Confirms the triage assessment: a small, additive JSON Schema change to standards/personas/persona.schema.json. The evals gate now accepts exactly one of path (single eval set) or paths (array, one per wrapped skill), enforced by a oneOf; min_cases is pinned as a per-set requirement. Backward-compatible — existing path-only manifests (qa-lead) remain valid.
Linked issue analysis
Resolves #755 finding 6 ("evals.path is a single path, but the eval tree is skill-keyed"). The new paths array lets a persona wrapping multiple skills (e.g. pr-review's deep-review + triage sets) declare all its eval sets, unblocking pr-review and business-analyst onboarding. Substantively addressed.
Findings
No blocking findings.
- Schema at head SHA is valid JSON (verified by parsing).
oneOfsemantics checked: path-only passes, paths-only passes, both fails, neither fails — matching the PR's stated verification. pathshasminItems: 1anduniqueItems: true(addressing an earlier CodeRabbit suggestion); redundantnotclauses flagged by Gemini were removed.- All 5 review threads resolved; CodeRabbit's CHANGES_REQUESTED was dismissed after resolution and superseded.
- Secret scan MCP tool not available in this environment; gitleaks CI check passed (noted, non-blocking).
CI status
All checks green: Lint, ShellCheck, CodeQL, SonarCloud (quality gate passed, 0 new issues), AgentShield, Agent Security Scan, gitleaks secret scan, npm audit — all SUCCESS. Ecosystem-specific audits (pip/cargo/go/pnpm) skipped as not applicable.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
* feat(personas): enforce the eval count gate; read evals.path OR evals.paths Companion to petry-projects/.github#779. Two things, both #755: 1. Read the new evals shape. `evals` now carries `path` (one set) OR `paths` (several, for a persona that wraps multiple skills — pr-review's deep-review + triage). check_evals handles both and requires dev/ + holdout/ splits in every set. Backward-compatible: qa-lead's single `path` still validates against both the live main schema and the #779 branch schema. 2. Make principle 5 partially real (finding 5). "No persona reaches stable without an eval gate" was documented but unenforced. Now: once a persona's status reaches its `evals.required_before` ring, each held-out set MUST carry >= `min_cases` real cases (blank lines ignored). A draft is exempt — draft is exactly when seed cases are placeholders. So a persona can no longer be promoted past required_before on the 5 synthetic starters. The SCORED gate (running the cases against a judge and requiring a pass rate) needs an eval harness and is deliberately NOT here — it is a separate follow-up. What is hermetically checkable (the held-out COUNT at promotion) is enforced now; what needs a runtime is tracked, not faked. Verification - bats tests/test_validate_personas.bats: 22/22 (16 + 6 new: paths all-splits, paths one-split-missing, draft-not-enforced, stable-enforced-fails, enough-cases-passes, blank-lines-ignored). - validate-personas.py against BOTH the live main schema and the #779 branch schema: OK (qa-lead is draft, so the count gate does not bite). Merge after #779 (the validator fetches the schema from .github main; until #779 lands, `evals.paths` is not yet permitted there — but qa-lead uses `path`, so main stays green either way). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Resolves #755 finding 6.
evals.pathwas a single string, but a persona wrapping several skills has several eval sets — pr-review's held-out sets live atevals/deep-review/andevals/triage/, which the single-path shape can't express. This blocks pr-review's onboarding (#1282) and business-analyst's (#1283, brainstorm+research+brief).Change:
evalsnow requires exactly one ofpath(one set — qa-lead keeps it, unchanged) orpaths(array, one per wrapped skill). AoneOfenforces mutual exclusivity.Verified: path-only PASS, paths-only PASS, both FAIL, neither FAIL. Backward-compatible — qa-lead's
pathis untouched, so no cross-repo ordering.Also pinned the
min_casescontract: per-set, enforced byvalidate-personas.pyonce status reachesrequired_before— the count half of principle 5, made real in the companion.github-privatevalidator PR.Valid Draft 2020-12; additive diff.
🤖 Generated with Claude Code
Summary by CodeRabbit