Skip to content

refactor(project): simplify code-based evaluator scaffolding - #2255

Merged
jariy17 merged 2 commits into
refactorfrom
remove-code-based-evaluator-model-metric
Sep 9, 2026
Merged

refactor(project): simplify code-based evaluator scaffolding#2255
jariy17 merged 2 commits into
refactorfrom
remove-code-based-evaluator-model-metric

Conversation

@jariy17

@jariy17 jariy17 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove --metric and --model from project add evaluator code-based
  • always scaffold the generic Python evaluator for managed code-based evaluators
  • remove the bundled DeepEval and AutoEvals templates
  • describe --timeout-seconds as the evaluator timeout

Compatibility

  • existing evaluator project specs are unchanged
  • scripts using the removed flags now receive an unknown-option error

Testing

  • bun test (3171 passed)
  • bun run typecheck
  • bun run lint:check
  • bun run format:check
  • bun run build

Live verification

Verified with a temporary evaluator-only project in an exploratory AWS account in us-west-2:

  • scaffolded a managed code-based evaluator with --timeout-seconds 123
  • synthesized CloudFormation with Lambda Timeout and evaluator LambdaTimeoutInSeconds both set to 123
  • deployed successfully; the evaluator reached ACTIVE status
  • confirmed the deployed Lambda was Active with Timeout 123
  • invoked the packaged evaluator Lambda with a representative evaluator event and received a Pass response with HTTP 200
  • removed the project and confirmed the stack reached DELETE_COMPLETE and both resources no longer exist

@github-actions github-actions Bot added the size/m PR size: M label Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 9, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AgentCore Harness Review

Verdict: Looks good

The change is self-consistent and cleanly scoped to what the description promises:

  • --metric and --model flags are fully removed from handlers/project/add/evaluator/code-based/index.ts, along with their parsers (parseMetric, resolveBedrock Model).
  • EVALUATOR_LIBRARIES / EvaluatorLibrary are dropped from handlers/project/types.ts, and ManagedEvaluatorScaffoldInput no longer carries metric/model.
  • core/project/templates/evaluator.ts collapses to a single evaluators/python-lambda asset dir with a Name-only render context.
  • The deepeval-lambda and autoevals-lambda asset trees are removed; no remaining references in src/ (only the test line that asserts the removed flags reject).
  • Tests are updated to cover managed-with-timeout, managed-default, --timeout-seconds incompatibility with --lambda-arn, and rejection of the removed flags via commander.unknownOption.
  • No TUI screen depended on the removed inputs (isValidBedrockModelId is retained for the llm-as-a-judge handler and TUI, which still need it).
  • Existing on-disk evaluator specs are unaffected — metric/model were scaffolder inputs only, not part of the Evaluator schema.

No changes required before merge.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Sep 9, 2026
@codecov-commenter

codecov-commenter commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.03%. Comparing base (d9ab959) to head (52787db).
⚠️ Report is 5 commits behind head on refactor.

Additional details and impacted files
@@             Coverage Diff              @@
##           refactor    #2255      +/-   ##
============================================
- Coverage     97.04%   97.03%   -0.01%     
============================================
  Files           566      566              
  Lines         39409    39345      -64     
============================================
- Hits          38244    38180      -64     
  Misses         1165     1165              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 9, 2026
@jariy17
jariy17 marked this pull request as ready for review September 9, 2026 18:37
Hweinstock
Hweinstock previously approved these changes Sep 9, 2026
Comment thread src/core/project/templates/evaluator.ts Outdated
@github-actions github-actions Bot added size/m PR size: M and removed size/m PR size: M labels Sep 9, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@jariy17
jariy17 requested a review from Hweinstock September 9, 2026 19:00
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 9, 2026
@jariy17
jariy17 merged commit 4fe2c10 into refactor Sep 9, 2026
21 checks passed
@jariy17
jariy17 deleted the remove-code-based-evaluator-model-metric branch September 9, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m PR size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants