feat: add imperative batch-insights command - #2066
Conversation
github-actions
Bot
added
agentcore-harness-reviewing
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2066 +/- ##
============================================
+ Coverage 97.26% 97.40% +0.14%
============================================
Files 398 403 +5
Lines 24194 24321 +127
============================================
+ Hits 23532 23690 +158
+ Misses 662 631 -31 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jariy17
left a comment
jariy17
left a comment
There was a problem hiding this comment.
Left some comments. Pretty good thou
| import { EvalClient } from "./eval"; | ||
| import type { AwsClients } from "./types"; | ||
|
|
||
| describe("EvalClient.startBatchInsights", () => { |
There was a problem hiding this comment.
We don't need this unit test. This should be captured by handler unit tests
| z.string().optional(), | ||
| ), | ||
| flag( | ||
| "online-eval", |
There was a problem hiding this comment.
Can you use online-eval as a data source for batch insight?
There was a problem hiding this comment.
yea you can, the API accepts both online-evals and insights together. Current CLI exposes online-eval for insights as well
| import { parseJsonFlag } from "../utils"; | ||
| import type { SessionSourceValue, SessionWindow } from "./types"; | ||
|
|
||
| export const sessionSourceFlags = [ |
There was a problem hiding this comment.
Encapsulate this as a class please :)
2ec82da to
382e307
Compare
notgitika
left a comment
notgitika
left a comment
There was a problem hiding this comment.
Nice! one comment for behavior I just noticed that wasnt introduced here but worth acting upon
| description: "list batch insights runs", | ||
| flags: [ | ||
| flag("next-token", "pagination token returned by a previous request", z.string().optional()), | ||
| flag("max-results", "maximum number of service items to inspect", z.number().optional()), |
There was a problem hiding this comment.
not introduced in this PR but I just wanted to make a note.
since we filter after fetching the page, --max-results is really "items to look at" not "insights returned", so you can get an empty list back with a nextToken still set. Your help text already says as much and pagination still works, so I'm ok with it here. Only calling it out because the TUI builds on this same filter. Do you want a follow-up for server-side filtering down the line? or maybe just open up an issue for polish later
There was a problem hiding this comment.
Yeah good call out. Filtering between insights and evals seems to be a recurring theme (same issues are coming up in online-insights / online-evals). I'll take an AI to think through this and polish
382e307 to
883630f
Compare
4 participants
Description
Adds imperative batch insights commands for AgentCore evaluations:
Summary of changes
--insightdefaults toBuiltin.Insight.FailureAnalysissessionSourceutilityType of Change
Testing
Added unit tests along with golden fixture tests (recorded against team AWS account). Also ran manual smoke tests against runtime invocations in my personal account, succeeding on all three added commands.
Checklist
Stack created with GitHub Stacks CLI • Give Feedback 💬