feat(evaluator-sdk): agent-skill A/B evaluation for Fabric agent-eval - #625
Merged
Conversation
Contributor
|
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
from
July 14, 2026 11:41
8d57c62 to
b74828a
Compare
SandyChapman
changed the base branch from
main
to
fabric-client-to-fabric/schapman
July 14, 2026 11:41
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
from
July 14, 2026 11:42
b74828a to
ae72018
Compare
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
2 times, most recently
from
July 14, 2026 13:27
b79565c to
d7401b3
Compare
SandyChapman
changed the title
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
from
July 14, 2026 15:47
d7401b3 to
f681e57
Compare
SandyChapman
marked this pull request as ready for review
July 14, 2026 15:47
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds Fabric agent-skill injection with native and Codex modes, provenance tracking, usage metrics, run-scoped evidence isolation, and an A/B evaluation example using the bundled ChangesAgent skill evaluation
Sequence Diagram(s)sequenceDiagram
participant EvaluationExample
participant FabricAgentRuntime
participant FabricPlanner
participant SkillInstaller
participant FabricClient
participant SkillUsedMetric
EvaluationExample->>FabricAgentRuntime: create baseline and treated runs
FabricAgentRuntime->>FabricPlanner: probe skill capability
FabricPlanner-->>FabricAgentRuntime: return injection mode
FabricAgentRuntime->>SkillInstaller: stage and install skill
SkillInstaller-->>FabricAgentRuntime: return profiles and provenance
FabricAgentRuntime->>FabricClient: run evaluation task
FabricClient-->>SkillUsedMetric: provide trial metadata and evidence
SkillUsedMetric-->>EvaluationExample: return presence and usage results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nemo_evaluator_sdk/examples/skill_eval/README.md (1)
1-95: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSplit this into a HOW-TO and an EXPLANATION.
Move causal rationale and metric caveats to an explanation page. Start the HOW-TO with prerequisites, provide Python SDK and CLI examples in tab sets, use configured product substitutions, and end with Next Steps.
🤖 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 `@packages/nemo_evaluator_sdk/examples/skill_eval/README.md` around lines 1 - 95, Split the skill_eval README content into separate HOW-TO and EXPLANATION documentation pages. Make the HOW-TO begin with prerequisites, include Python SDK and CLI examples in tab sets, replace hard-coded product references with configured product substitutions, and end with a Next Steps section; move the causal rationale, metric caveats, and other explanatory material into the EXPLANATION page.Source: Coding guidelines
🤖 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 `@packages/nemo_evaluator_sdk/examples/skill_eval/README.md`:
- Around line 34-52: Update the “Run it” prerequisites in the skill evaluation
README to establish the import path for nemo_evaluator_sdk before the shown
module command. Document the supported workspace install or dependency-sync
step, or provide an invocation that sets the package path, while preserving the
existing NVIDIA_API_KEY and ADAPTER_PYTHON guidance.
In `@packages/nemo_evaluator_sdk/examples/skill_eval/run_skill_eval.py`:
- Around line 68-90: The guideline metric currently searches the entire response
and applies the same parameter rule to every task. In
packages/nemo_evaluator_sdk/examples/skill_eval/run_skill_eval.py lines 68-90,
update compute_scores and its regex logic to extract the returned function
signature and validate parameters there; in lines 112-115, make the second task
index-based or provide task-specific expected-parameter rules so gcd does not
require enieme while positional-index tasks do.
---
Outside diff comments:
In `@packages/nemo_evaluator_sdk/examples/skill_eval/README.md`:
- Around line 1-95: Split the skill_eval README content into separate HOW-TO and
EXPLANATION documentation pages. Make the HOW-TO begin with prerequisites,
include Python SDK and CLI examples in tab sets, replace hard-coded product
references with configured product substitutions, and end with a Next Steps
section; move the causal rationale, metric caveats, and other explanatory
material into the EXPLANATION page.
🪄 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: 1726fadf-56b3-4856-a67b-fa3070d1f2e4
⛔ Files ignored due to path filters (3)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/metrics.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/skills.pyis excluded by!sdk/**
📒 Files selected for processing (10)
packages/nemo_evaluator_sdk/examples/skill_eval/.gitignorepackages/nemo_evaluator_sdk/examples/skill_eval/README.mdpackages/nemo_evaluator_sdk/examples/skill_eval/run_skill_eval.pypackages/nemo_evaluator_sdk/examples/skill_eval/skills/supercool-guidelines/SKILL.mdpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/metrics.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/skills.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_skills.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_skill_used_metric.py
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
from
July 14, 2026 18:33
f681e57 to
4a69366
Compare
Contributor
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 `@packages/nemo_evaluator_sdk/examples/skill_eval/run_skill_eval.py`:
- Around line 240-271: Move the AgentSkill.from_directory call into the existing
try block, or extend the exception handling to include SkillInjectionError, so
missing bundled skills follow the same friendly error path as runtime failures.
Preserve the existing message and return behavior in the run_skill_eval flow.
In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/metrics.py`:
- Around line 167-181: Update the exception handling in _skill_used to catch
trace validation failures from Trajectory.model_validate and file-read errors in
addition to the existing KeyError and ValueError cases. Preserve the warning log
and return False best-effort fallback for all expected trace-reading failures.
🪄 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: adcebd6e-e668-4a38-bf7d-d03d3bf6e1e2
⛔ Files ignored due to path filters (3)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/metrics.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/skills.pyis excluded by!sdk/**
📒 Files selected for processing (10)
packages/nemo_evaluator_sdk/examples/skill_eval/.gitignorepackages/nemo_evaluator_sdk/examples/skill_eval/README.mdpackages/nemo_evaluator_sdk/examples/skill_eval/run_skill_eval.pypackages/nemo_evaluator_sdk/examples/skill_eval/skills/supercool-guidelines/SKILL.mdpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/metrics.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/skills.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_skills.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_skill_used_metric.py
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/nemo_evaluator_sdk/examples/skill_eval/.gitignore
- packages/nemo_evaluator_sdk/examples/skill_eval/skills/supercool-guidelines/SKILL.md
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_skills.py
- packages/nemo_evaluator_sdk/examples/skill_eval/README.md
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/skills.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_skill_used_metric.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py
ngoncharenko
approved these changes
Jul 15, 2026
Add agent-skill (agentskills.io) injection to FabricAgentRuntime so an A/B eval can score the same taskset with and without a skill: build one runtime with `skill=None` and one via `with_skill(skill)` over the same tasks, then diff the scores. Each run gets its own run-id evidence subtree so a baseline and a skilled variant sharing a work_root don't collide, and a SkillUsedMetric scores whether the agent actually engaged the injected skill. How the skill reaches the harness is decided by querying Fabric's own capability planner at runtime rather than a hardcoded adapter list: the runtime plans a copy of the config with a sentinel skill path attached and reads the resulting RunPlan.capability_plan routes. A `skills` route targeting `harness_native` means the adapter accepts the native Fabric `skills` config, so the bundle is handed over natively; otherwise a codex harness falls back to the `.agents/skills/` self-discovery convention, and anything else fails fast rather than run a skill-free trial mislabeled "with skill". Driving the decision from Fabric means it tracks whatever the installed adapters declare — including end-user adapters the platform doesn't ship (e.g. the `claude` adapter) — instead of a list that silently goes stale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
agent-skill-eval/schapman
branch
from
July 15, 2026 11:20
4a69366 to
93f637c
Compare
SandyChapman
enabled auto-merge
July 15, 2026 11:26
This was referenced Jul 21, 2026
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
2 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.
What
Adds agent-skill evaluation to the Fabric agent-eval runtime: inject an
agentskills.io skill into an eval and A/B the same
taskset with vs. without it, to measure the skill's effect.
AgentSkill+install_skill(runtimes/fabric/skills.py) — adirectory-backed agentskills bundle, staged per task and wired to the harness.
Namespaced under
<name>/so it can't collide with task-seeded workspace files.SkillProvenance(name, content hash, mode, adapter, location) is stamped intotrial metadata for an auditable A/B diff.
How the skill reaches the harness is decided by querying Fabric's own
capability planner at runtime, not a hardcoded adapter list. The runtime plans
a copy of the config with a sentinel skill path attached and reads
RunPlan.capability_planroutes:skillsroute targetingharness_native(the adapter accepts the nativeFabric
skillsconfig, e.g. Hermes/Claude) → nativeskills.pathsoverlay;codexharness →.agents/skills/<name>/placement, which theCodex CLI self-discovers;
skill" (which would corrupt the A/B).
Driving the decision from Fabric means it tracks whatever the installed adapters
declare — including end-user adapters we don't ship (e.g. the
claudeadapter, which a static allow-list would miss) — instead of a list that silently
goes stale.
FabricAgentRuntimegains askill=param and awith_skill()clone forconfound-free A/B: baseline (
with_skill(None)) vs. treated (with_skill(skill))differ in exactly the skill and nothing else.
SkillUsedMetric—skill_present/skill_usedbooleans, to flag a failureto engage an injected skill (best-effort trajectory detection; strongest for
Codex filesystem discovery).
run_idis folded into the Fabric evidence path (<root>/<run_id>/<task>) soan A/B baseline and skilled variant sharing a
work_rootdon't collide.examples/skill_eval/— a runnable baseline-vs-treated A/B where the skill(the "Supercool Coding Guidelines":
supercool_prefix + Frencheniemeparam)is required to pass
follows_guidelines.Notes
FabricClient→FabricSDK migration (refactor(evaluator-sdk): migrate Fabric agent-eval runtime to the updated Fabric SDK #648), so this isbuilt on the current config-first Fabric run model (
Fabric().run(...)/Fabric().plan(...)). No longer blocked on that migration.resolve_skill_mode/native_skills_route) is pureand unit-testable without the native SDK; the actual
client.plan(...)probe wasverified live against a NeMo-Fabric checkout (hermes→native, codex→unsupported,
claude→native).
Verification
test_fabric_skills.py,test_fabric_runtime.py,test_skill_used_metric.pyand the surrounding agent-eval suite — 160 passed, 2 skipped.
ruff,ty(only the expectedunused-ignorewarnings CI relies on),lint-sdk-vendored, andlint-cliall clean; vendored SDK mirror regeneratedvia
make vendor.🤖 Generated with Claude Code
Summary by CodeRabbit
with_skill(...), including per-run evidence/run isolation and skill provenance metadata.skill_present/skill_usedreporting via a new metric.supercool-guidelinesskill bundle.