Uh oh!
There was an error while loading. Please reload this page.
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.29% 97.31% +0.01%
============================================
Files 479 481 +2 Lines 29673 29865 +192 ============================================
+ Hits 28871 29063 +192
Misses 802 802 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Claude Security Review: no high-confidence findings. (run) |
303c8db to
9bd7998CompareClaude 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
32a10efCompareClaude Security Review: no high-confidence findings. (run) |
Claude Security Review: no high-confidence findings. (run) |
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) |
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.
e440cb5 to
7215450CompareClaude Security Review: no high-confidence findings. (run) |
What
Adds
agentcore project add evaluator code-based— declarative code-based evaluators via projects — plus enablesagentcore project remove evaluator. Follows theproject addpattern (#2034 / #2004 / #2035 / #1998 / #2037) and, after review, mirrors the runtime layering (thin handler →templates/evaluator.tsowns the registry +buildManagedEvaluatorSpec, parallelingbuildRuntimeSpec).No mode/selector flag — the mode is inferred from what you pass (mirrors
CodeBasedConfigSchema'smanaged XOR external):--metric <library.Metric>@custom_code_based_evaluator()stub you fill in--lambda-arn <arn>Command structure
agentcore project add evaluator code-based --help:Enforced in-handler (not shown by Commander):
--name+--levelrequired; exactly one of--metric/--lambda-arn;--metricmust be<library>.<Class>where library ∈ {deepeval, autoevals} and Class is a single identifier;--modelmust be a Bedrock model id / inference-profile-or-foundation-model ARN (optionallybedrock/-prefixed) and requires--metric;--timeout-seconds/--model/--metricare managed-only. Managed auto-fillscodeLocation=app/<name>,entrypoint=lambda_function.handler, per-librarytimeoutSeconds(deepeval 300, else 60), andadditionalPolicies=["execution-role-policy.json"].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.Known issues surfaced by the cloud deploy (both OUTSIDE this PR)
The evaluator authoring + synth work; end-to-end
project deployis currently blocked by two pre-existing bugs, neither in this feature's code:src/assets/cdk/lib/cdk-stack.tsimports/instantiatesAgentCorePayments, but no published@aws/agentcore-cdkversion exports that name (alpha.49 and alpha.50 exportAgentCorePaymentManager/Connector). So it's a template code bug, not a stale pin — a version bump can't fix it; the construct must be dropped or gated behind a spec payments-config check.tsc TS2305→ build/synth/deploy fail for every scaffolded project, evaluator or not. Owner: payments.AWS::BedrockAgentCore::Evaluator, which failsCREATE_FAILED:"Access denied for Lambda function …". Confirmed across two runs (not a race): thelambda:InvokeFunction/GetFunctiongrants tobedrock-agentcore.amazonaws.comreachCREATE_COMPLETE~9s before the evaluator, yet the control-plane access check still denies — a deterministic authorization-shape mismatch (likely a requiredSourceAccount/SourceArncondition) in@aws/agentcore-cdk'sAgentCoreEvaluator. Owner: agentcore-cdk L3.🤖 Draft — CLI/authoring layer is complete and green; hold merge until the two deploy blockers (payments #2120, agentcore-cdk L3) land.