feat(project): add project add evaluator code-based - #2144
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Small, focused fix that surfaces two real footguns of the code-based evaluator scaffold:
- Empty stub silently returns
Passfor every session (verified againstsrc/assets/evaluators/python-lambda/lambda_function.py, which returnslabel="Pass"). - Managed code-based evaluators aren't yet provisioned by
project deploy.
Logic in index.ts (lines 158–166) matches the commit message: the "returns Pass" note is gated on !hasLambda && !hasMetric, and the "not yet provisioned" note is gated on !hasLambda, so --lambda-arn (external) correctly prints neither.
Tests in index.test.ts use real temp directories via mkdtemp and drive the handler through the router — no excessive mocking — and cover both the stub and external paths. Telemetry isn't warranted here since this only adds informational stderr output, not a new feature.
Nothing blocking.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2144 +/- ##
============================================
+ Coverage 97.22% 97.24% +0.01%
============================================
Files 507 509 +2
Lines 33809 34009 +200
============================================
+ Hits 32872 33073 +201
+ Misses 937 936 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
303c8db to
9bd7998
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| const context: Record<string, unknown> = { Name: toPythonPackageName(flags["name"]) }; | ||
|
|
||
| if (hasMetric) { | ||
| const raw = flags["metric"]!; |
There was a problem hiding this comment.
Can we use make this helper function and use zod here.
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
e78e9bc to
32a10ef
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| @@ -0,0 +1,15 @@ | |||
| { | |||
There was a problem hiding this comment.
I didn't want add an abstraction to generate common assets like this one due time constraints. We can look for this in the future.
|
Claude Security Review: no high-confidence findings. (run) |
e440cb5 to
7215450
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Declarative code-based evaluators via projects. Mode is inferred from flags (mirrors CodeBasedConfigSchema managed XOR external): --lambda-arn -> external (BYO Lambda) --metric <library.Metric> -> managed 3P (deepeval/autoevals), scaffolded neither -> managed empty stub you fill in Scaffolds app/<name>/ from ported evaluator templates (python/deepeval/autoevals lambda), hardcodes codeLocation, and auto-wires additionalPolicies= [execution-role-policy.json]. Also enables `project remove evaluator`.
…aluators Runtimes, harnesses, and evaluators all scaffold into app/<name>, but the duplicate-name guard is per-resource-type and the tree write happens outside the rollback try/catch. An evaluator whose name matches an existing runtime/ harness dir (or a leftover from a removed evaluator) threw a raw 'File already exists' mid-write and orphaned partial files. Fail up front with a clear InputValidationError when app/<name> already exists.
…or code-based evaluators - Reject a namespaced/multi-dot metric class (e.g. deepeval.metrics.Faithfulness) that would render invalid Python; require a single class identifier. - --model is Bedrock-only: accept a bare model id / inference-profile-or- foundation-model ARN, optionally prefixed with bedrock/, validated via isValidBedrockModelId (same forms the llm-as-a-judge handler accepts). Non-Bedrock or slashless values now error instead of being silently dropped (deepeval) or passed to the wrong client (autoevals). - autoevals template prefixes bedrock/ for litellm routing now that Model is the bare id.
Print notes after add: the empty stub returns Pass for every session until implemented, and managed evaluators are scaffolded but not yet provisioned by 'project deploy' (no CDK/L3 support). External (--lambda-arn) prints neither.
…EOUT const; drop code comments
…templates layer Mirror the runtime layering: the handler now just parses/validates flags and passes a ManagedEvaluatorScaffoldInput; templates/evaluator.ts owns the library registry, per-library timeouts, render context, and buildManagedEvaluatorSpec (parallels buildRuntimeSpec). Also adds "evaluator" to RemoveResourceInput.
…aluators The L3 (@aws/agentcore-cdk) does provision spec.evaluators — synth emits AWS::BedrockAgentCore::Evaluator + Lambda — so the note was inaccurate. Keep the empty-stub 'returns Pass until implemented' note, which is still true.
7215450 to
ef9f5ca
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Command structure
agentcore project add evaluator code-based --help:Commits
c4430c03feat — the command + 3 scaffold templates +remove evaluatorbaec1630fix — guardapp/<name>collisions (up-front, no partial writes)e7bc3675fix — validate--metricclass + require a Bedrock--model9bd79980fix — echo the inferred mode + caveats at add time32a10ef9refactor — sharetoPythonPackageNameviafsUtils;DEFAULT_TIMEOUTconst853dcf86refactor — move template knowledge intotemplates/evaluator.ts(runtime layering)Testing
bun run buildOK ·bun test src/handlers/project src/core/project→ 597 pass / 0 fail.AWS::BedrockAgentCore::Evaluator+ Lambda + role + permissions, and generated Pythonast.parses for deepeval + autoevals (bedrock + openai branches). Full report shared separately.