Skip to content

Commit d25c026

Browse files
committed
fix(cli): quick mode runs ad-hoc instead of minting quick-<ts> benchmarks; harden findBenchmark name reuse
Phase 1 of benchmark dedup: stop the two CLI paths that grew the benchmarks list on every invocation of the same command. - Quick mode (bare 'benchmark' with no server running) previously created a fresh 'quick-$(Date.now())' Benchmark doc per run. It now delegates to the unified evaluation-runs API as an ad-hoc run over all stored test cases (sources: test-case-ids), creating zero Benchmark docs. Server lifecycle preserved via forceStopServer. - findBenchmark now falls back to a unique trimmed/case-insensitive name match (exact id > exact name > unambiguous fuzzy), so re-running with a case/whitespace variant reuses the existing benchmark instead of creating a duplicate. Ambiguous fuzzy matches return null (fail loudly, no guessing). Tests: unit (findBenchmark matrix), integration (two identical ad-hoc runs leave the benchmark count unchanged). Signed-off-by: Megha Goyal <goyamegh@amazon.com> (cherry picked from commit f85b49486918a37d40632ed06ee32446e6cc6c1c) Signed-off-by: goyamegh <goyamegh@amazon.com>
1 parent 9e33e19 commit d25c026

7 files changed

Lines changed: 236 additions & 118 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,11 @@ Inspired by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
149149
- **Compare view: no more fabricated 0% pass rate / $0.00 cost / 0ms duration for CLI and untraced runs** ([services/comparisonService.ts](services/comparisonService.ts), [components/comparison/ComparisonPage.tsx](components/comparison/ComparisonPage.tsx)): three distinct lies in the scoreboard. (1) CLI-written run docs keep the verdict on the report doc only, and `calculateRunAggregates` bucketed those results entries as "errored" → 0% pass rate while the per-case table below showed real Passed/Failed; report verdicts are now overlaid before bucketing. (2) The batch metrics API returns a zero-filled `status:'pending'` placeholder when a runId has no spans; those were summed as real data → "$0.00 / 0ms" (reads as *free and instant* instead of *not captured*); pending placeholders are now skipped so the display falls back to "-". (3) Avg Duration now falls back to the per-result `performanceMetrics.durationMs` the benchmark runner already persists, before the coarse run-level fields. The trace-metrics overlay + fallback logic is extracted into a new pure `mergeTraceMetrics()` in `comparisonService.ts` (previously inlined in `ComparisonPage.tsx`'s render path, untestable in isolation). Unit: [tests/unit/services/comparisonService.test.ts](tests/unit/services/comparisonService.test.ts).
150150
- **Claude Code connector: per-execution CLI args no longer duplicate under benchmark concurrency** ([services/connectors/claude-code/ClaudeCodeConnector.ts](services/connectors/claude-code/ClaudeCodeConnector.ts)): the connector registry hands out a singleton instance shared by all concurrent benchmark tasks, and each execution built its args by appending to the shared `this.config.args` — compounding other in-flight executions' flags (spawns observed with `--append-system-prompt`/`--allowed-tools` repeated up to 5x at concurrency 3, i.e. the system prompt text injected multiple times). Executions now build from a pristine constructor-args base captured once; the set→read window is synchronous so it is safe under concurrency. Unit: [tests/unit/services/connectors/claude-code/ClaudeCodeConnector.test.ts](tests/unit/services/connectors/claude-code/ClaudeCodeConnector.test.ts).
151151
- **Classic (non-SDK) runs no longer trace-poll non-traced agents for 10 minutes** ([services/evaluationRunner.ts](services/evaluationRunner.ts)): the unified evaluation-runs path pre-persists a placeholder report with `metricsStatus: 'pending'`, and the classic path's final report carried no `metricsStatus` — the update-merge kept the placeholder's `pending`, so the runner trace-polled a `useTraces: false` agent (demo/mock included) for the full 10-minute timeout and then errored the report. A 3-second demo run took 10 minutes and surfaced as `errored`. The runner now stamps `completed` (or `pending` for `useTraces` agents) when the report lacks the field, mirroring `benchmarkRunner`. Pinned by [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts) (times out again if this regresses).
152-
- **CLI quick mode no longer mints a `quick-<timestamp>` benchmark per invocation** ([cli/commands/benchmark.ts](cli/commands/benchmark.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)): running `agent-health benchmark` bare (quick mode) created a fresh `quick-${Date.now()}` Benchmark doc every time — re-running the same command grew the benchmarks list unbounded. Quick mode now routes through the unified evaluation-runs API as an **ad-hoc run** over all stored test cases (no Benchmark doc created), preserving the start-and-stop server lifecycle. `findBenchmark` also hardens name-keyed reuse: exact id, then exact name, then a **unique** trimmed/case-insensitive match (ambiguity returns null instead of guessing), so `-n "my bench "` reuses `My Bench` instead of minting a duplicate. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts); integration: [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts).
152+
- **CLI quick mode no longer mints a `quick-<timestamp>` benchmark per invocation** ([cli/commands/benchmark.ts](cli/commands/benchmark.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)): running `agent-health benchmark` bare (quick mode) created a fresh `quick-${Date.now()}` Benchmark doc every time — re-running the same command grew the benchmarks list unbounded. Quick mode now routes through the unified evaluation-runs API as an **ad-hoc run** over all stored test cases (no Benchmark doc created), preserving the start-and-stop server lifecycle. `findBenchmark` also hardens name-keyed reuse: exact id, then exact name, then a **unique** trimmed/case-insensitive match, so `-n "my bench "` reuses `My Bench` instead of minting a duplicate. An **ambiguous** fuzzy match (e.g. `Foo` and `FOO` both exist) is a hard error listing the collisions — the CLI neither guesses nor creates a third near-duplicate under the colliding name. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts); integration: [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts).
153153
- **Classic (non-SDK) runs no longer trace-poll non-traced agents for 10 minutes** ([#437](https://github.com/opensearch-project/agent-health/pull/437)) ([services/evaluationRunner.ts](services/evaluationRunner.ts)): the unified evaluation-runs path pre-persists a placeholder report with `metricsStatus: 'pending'`, and the classic path's final report carried no `metricsStatus` — the update-merge kept the placeholder's `pending`, so the runner trace-polled a `useTraces: false` agent (demo/mock included) for the full 10-minute timeout and then errored the report. A 3-second demo run took 10 minutes and surfaced as `errored`. The runner now stamps `completed` (or `pending` for `useTraces` agents) when the report lacks the field, mirroring `benchmarkRunner`. Pinned by [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts) (times out again if this regresses).
154-
- **CLI quick mode no longer mints a `quick-<timestamp>` benchmark per invocation** ([#437](https://github.com/opensearch-project/agent-health/pull/437)) ([cli/commands/benchmark.ts](cli/commands/benchmark.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)): running `agent-health benchmark` bare (quick mode) created a fresh `quick-${Date.now()}` Benchmark doc every time — re-running the same command grew the benchmarks list unbounded. Quick mode now routes through the unified evaluation-runs API as an **ad-hoc run** over all stored test cases (no Benchmark doc created), preserving the start-and-stop server lifecycle. `findBenchmark` also hardens name-keyed reuse: exact id, then exact name, then a **unique** trimmed/case-insensitive match (ambiguity returns null instead of guessing), so `-n "my bench "` reuses `My Bench` instead of minting a duplicate. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts); integration: [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts).
154+
- **CLI quick mode no longer mints a `quick-<timestamp>` benchmark per invocation** ([#437](https://github.com/opensearch-project/agent-health/pull/437)) — **erratum: this CLI change was inadvertently dropped from #437 during a branch repair and did NOT ship in 0.6.0**; it lands in the follow-up PR (see Unreleased) ([cli/commands/benchmark.ts](cli/commands/benchmark.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)): running `agent-health benchmark` bare (quick mode) created a fresh `quick-${Date.now()}` Benchmark doc every time — re-running the same command grew the benchmarks list unbounded. Quick mode now routes through the unified evaluation-runs API as an **ad-hoc run** over all stored test cases (no Benchmark doc created), preserving the start-and-stop server lifecycle. `findBenchmark` also hardens name-keyed reuse: exact id, then exact name, then a **unique** trimmed/case-insensitive match (ambiguity returns null instead of guessing), so `-n "my bench "` reuses `My Bench` instead of minting a duplicate. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts); integration: [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts).
155155
- **`traces` fixture: bare/cache token attrs recognized, computed cost fallback, fail-loud on vacuous zero** ([lib/matchers/traces.ts](lib/matchers/traces.ts), [lib/matchers/tracesPricing.ts](lib/matchers/tracesPricing.ts)): dogfooding a cost-aware eval against Claude Code (`useTraces: true`) found `buildTracesAccessor()` only recognized `gen_ai.usage.*`/`llm.usage.*` token keys — Claude Code's `claude_code.llm_request` spans carry **bare** `input_tokens`/`output_tokens`/`cache_read_tokens`/`cache_creation_tokens` and no cost attribute at all, so `traces.totalTokens` and `traces.totalCost` silently read `0` and `expect(traces.totalCost).lessThan(2)` passed vacuously (same bug class as #230, just one layer deeper). Fixes: (1) the token key lists now also match the bare Claude Code attrs and cache-read/cache-creation tokens (both bare and `gen_ai.usage.cache_*_input_tokens`) are summed into `totalTokens`; (2) a new fallback pricing table (`lib/matchers/tracesPricing.ts`, estimated Bedrock/Anthropic Claude rates by model tier — clearly marked as estimates) computes `totalCost` from tokens + resolved model id (`gen_ai.request.model` / `gen_ai.response.model` / bare `model`) when no span reports cost directly; the accessor exposes a new `costSource: 'reported' | 'computed' | 'mixed' | 'none'` field so callers can tell which happened; (3) reading `totalTokens`/`totalCost` now **throws** (mirroring `unavailableTracesAccessor`) when spans exist but none carry a recognized token attribute, and reading `totalCost` also throws when tokens were found but every span's model is unpriced and no cost attribute exists — a genuine zero-value trace (recognized attrs present, value `0`) does not throw. Unit: [tests/unit/lib/matchers/traces.test.ts](tests/unit/lib/matchers/traces.test.ts), [tests/unit/lib/matchers/tracesPricing.test.ts](tests/unit/lib/matchers/tracesPricing.test.ts).
156+
- **CLI quick mode no longer mints a `quick-<timestamp>` benchmark per invocation** ([cli/commands/benchmark.ts](cli/commands/benchmark.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)): running `agent-health benchmark` bare (quick mode) created a fresh `quick-${Date.now()}` Benchmark doc every time — re-running the same command grew the benchmarks list unbounded. Quick mode now routes through the unified evaluation-runs API as an **ad-hoc run** over all stored test cases (no Benchmark doc created), preserving the start-and-stop server lifecycle. `findBenchmark` also hardens name-keyed reuse: exact id, then exact name, then a **unique** trimmed/case-insensitive match (ambiguity returns null instead of guessing), so `-n "my bench "` reuses `My Bench` instead of minting a duplicate. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts); integration: [tests/integration/cli/adhocRunNoBenchmark.integration.test.ts](tests/integration/cli/adhocRunNoBenchmark.integration.test.ts).
156157
- **Bedrock: `temperature` is no longer sent to models that have deprecated it** ([lib/bedrockCompat.ts](lib/bedrockCompat.ts), [server/services/bedrockService.ts](server/services/bedrockService.ts), [services/skills/evalGenerator.ts](services/skills/evalGenerator.ts), [services/skills/improver.ts](services/skills/improver.ts), [server/services/failureClusterService.ts](server/services/failureClusterService.ts), [server/services/assistantService.ts](server/services/assistantService.ts)) ([#299](https://github.com/opensearch-project/agent-health/issues/299)): newer Anthropic models (Claude Opus 4.5+, Sonnet 4.6+, the Claude 5 family) reject any explicit `temperature` with `ValidationException: 'temperature' is deprecated for this model`, so picking one as the judge — or triggering skill eval generation / improvement / failure clustering / the assistant fallback — failed outright. A shared `buildInferenceConfig()` helper now omits the parameter for those models across **all five** Bedrock Converse call sites; models that still accept it keep their configured value. Unit: [tests/unit/lib/bedrockCompat.test.ts](tests/unit/lib/bedrockCompat.test.ts), [tests/unit/server/services/bedrockService.test.ts](tests/unit/server/services/bedrockService.test.ts).
157158
- **Bedrock: model registry no longer pinned to US-only inference profiles** ([lib/bedrockCompat.ts](lib/bedrockCompat.ts), [server/services/bedrockService.ts](server/services/bedrockService.ts), [server/services/piAgenticJudgeService.ts](server/services/piAgenticJudgeService.ts)) ([#298](https://github.com/opensearch-project/agent-health/issues/298)): every Bedrock `model_id` in `lib/constants.ts` hardcodes the `us.` cross-region inference-profile prefix, which only resolves from US regions — running with `AWS_REGION=eu-central-1` failed with `ValidationException: The provided model identifier is invalid`. A shared `resolveRegionAwareModelId()` now re-homes the prefix (`us.` → `eu.`/`apac.`) to the active region at call time in the judge and all skill/assistant call sites; `global.` profiles, bare model ids, and non-Bedrock ids pass through unchanged. (The invalid `-v1` suffix on the Opus 4.7/4.8 registry entries — the other half of #298 — was already fixed.) Unit: [tests/unit/lib/bedrockCompat.test.ts](tests/unit/lib/bedrockCompat.test.ts).
158159
- **CLI `run`: trace-mode agents are no longer reported FAILED before the judge has run** ([cli/commands/run.ts](cli/commands/run.ts), [cli/utils/apiClient.ts](cli/utils/apiClient.ts)) ([#333](https://github.com/opensearch-project/agent-health/issues/333)): for a `useTraces` agent the server emits the SSE `completed` event **before** the background trace poller runs the judge (`metricsStatus: 'pending'`, no `passFailStatus`), and the CLI collapsed that not-yet-judged snapshot into `FAILED` — contradicting the persisted report, which the judge later marked `passed`. The CLI now keeps polling until the judge verdict lands (`metricsStatus` leaves `pending`), shows a "Waiting for traces / judge verdict..." spinner meanwhile, and renders a `PENDING` (not `FAILED`) status if the wait times out; `pollReportStatus` no longer treats a judge-pending report as terminal. Unit: [tests/unit/cli/utils/apiClient.test.ts](tests/unit/cli/utils/apiClient.test.ts).

0 commit comments

Comments
 (0)