feat(project): add online-eval to project add - #2048
Conversation
github-actions
Bot
added
the
agentcore-harness-reviewing
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2048 +/- ##
=========================================
Coverage 97.14% 97.15%
=========================================
Files 382 383 +1
Lines 22884 22975 +91
=========================================
+ Hits 22231 22321 +90
- Misses 653 654 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
removed
the
agentcore-harness-reviewing
| }); | ||
| break; | ||
| } | ||
| case "online-eval": { |
There was a problem hiding this comment.
Having a default for this makes sense to avoid having cases that all have no scaffolding and directly push. But at the same time, having the default loosens our validations since new resourceTypes will fall through to the default.
Meet in the middle: a resourceType enum, stack cases for new resourceTypes that do not need scaffolding
There was a problem hiding this comment.
I'll look at your config bundle PR and follow that pattern.
I think format is failing. |
| break; | ||
| } | ||
| case "online-eval": { | ||
| // Pure spec-level resource: no files to scaffold, just record the config. |
There was a problem hiding this comment.
nit: i feel like this comment is expressed by the code.
| // OnlineEvalConfigSchema owns every cross-field rule (agent XOR log groups, | ||
| // evaluators XOR insights, endpoint/serviceNames prerequisites). Validate once | ||
| // here so the user gets one actionable message rather than a raw stack later at | ||
| // deploy time — json.write does not re-validate on the way to disk. |
There was a problem hiding this comment.
I don't think this comment is fully accurate since we do actually validate before writing:
agentcore-cli/src/core/project/manager.tsx
Line 169 in 14e0440
Its also referencing other parts of the codebase that may change, so its high risk to become stale.
There was a problem hiding this comment.
I'm just gonna tell claude to stop putting code comments unless there is a good reason too.
7edf52f to
2639903
Compare
| case "online-eval": | ||
| newResources.push(resourceConfig); | ||
| break; |
There was a problem hiding this comment.
lets just stack this on config bundle
2639903 to
a171b74
Compare
Upstream moved the per-resource `project add` tests out of the monolithic project.test.ts into colocated add/<resource>/index.test.ts suites (harness in aws#2034, online-eval in aws#2048). Move the memory tests to match, with the same locally-duplicated run/inProject helpers those suites use. project.test.ts is now identical to upstream/refactor again, so this PR no longer touches it. Also drops the DeserializationError, FsReadWriteJson and ReadWriteJson imports, left dead there once the harness tests that used them moved to add/harness/index.test.ts. No test content changed: 187 project tests still pass, now across 10 files instead of 9.
Upstream moved the per-resource `project add` tests out of the monolithic project.test.ts into colocated add/<resource>/index.test.ts suites (harness in aws#2034, online-eval in aws#2048). Move the memory tests to match, with the same locally-duplicated run/inProject helpers those suites use. project.test.ts is now identical to upstream/refactor again, so this PR no longer touches it. Also drops the DeserializationError, FsReadWriteJson and ReadWriteJson imports, left dead there once the harness tests that used them moved to add/harness/index.test.ts. No test content changed: 187 project tests still pass, now across 10 files instead of 9.
5 participants
Command structure
agentcore project add online-eval --helpRelated: aws/agentcore-l3-cdk-constructs#335 —
OnlineEvaluationConfigagentresolves runtimes only (harness targets unsupported). The--agenthelp text here says "runtime" to match.