[Evals] Security LLM performance matrix generator in @kbn/evals-extensions - #283586
[Evals] Security LLM performance matrix generator in @kbn/evals-extensions#283586patrykkopycinski wants to merge 3 commits into
Conversation
|
|
||
| const evaluationsKbnUrl = | ||
| flagsReader.string('kbn-url') ?? | ||
| profileEnv.EVALUATIONS_KBN_URL ?? |
There was a problem hiding this comment.
This was recently updated to EVAL_KBN_URL
There was a problem hiding this comment.
Question on the @kbn/evals side of this - Do we need to define a new client and export all of that?
createEvaluationsEvalsClient, CreateEvaluationsEvalsClientParams, and DEFAULT_EVALUATIONS_KBN_URL exist only to keep @kbn/kbn-client out of kbn-evals-extensions, but that package is devOnly, so depending on it directly is fine. That's what @kbn/evals already does. Taking the dependency lets you drop all three and leave just getEvaluationsKbnClient exported.
Unless there's something I'm missing as to why that dependency can't be added.
Added a comment about listExperiments as well.
| * Lists every experiment matching the given filters, paging through the | ||
| * experiments API until the full set has been retrieved. | ||
| */ | ||
| async listExperiments(filters?: ListExperimentsFilters): Promise<EvaluationExperimentSummary[]> { |
There was a problem hiding this comment.
Does this paging loop complete in practice? The experiments route is not a paged search. It re-runs an aggregation that grows with the page number, so it'll hit ES bucket limits long before the 100-page bound.
I don't think the matrix needs it. queryMatrixScores only wants the newest experiment per (suite, model), and the route already supports filtering by model_id, so a single per_page: 1 request per pair gets exactly that. Or am I missing something related to your use case?
Also, the branch filter is a case-insensitive wildcard, not an exact match, so branch: "main" also matches things like feature/main-cleanup. Worth tightening that before this feeds published docs, wdyt?
|
/ci |
…me (#284661) ## Summary The persona-matrix `SkillInvoked` evaluator matched only `attributes.gen_ai.tool.name == "filestore.read"`, but the agent loads skills via the `load_skill` tool. Verified against the golden cluster over a 7-day window: | tool name | spans (7d) | | --- | --- | | `filestore.read` | 0 | | `load_skill` | 7,991 | Because `total_tool_spans` is still non-zero for these traces, the existing "no tool spans → `unavailable`" guard does not trip. The evaluator therefore returned a confident `score: 0` instead of `null` — silently zeroing the **Skill Invoked** column for every model in every persona-matrix run, not just for one model. This surfaced while generating a matrix column for a model that appeared to never route into any skill. Trace data showed the opposite: it invoked `load_skill` and called security skills directly. The reported zero was a measurement artifact, not a model capability result. ## Why only the tool name changed `load_skill` arguments carry the same `SKILL.md` path the existing predicate already matches: ```json {"skill":"/skills/platform/evals/eval-experiment-authoring/SKILL.md"} ``` so only the tool-name check needed widening. No dataset annotations change. `filestore.read` is retained for backwards compatibility with older traces. ## Verification Same-trace baseline on real golden-cluster data, holding the trace and skill predicate fixed and varying only the tool name: ``` OLD (filestore.read only) total_tool_spans=2 skill_invoked=0 NEW (load_skill accepted) total_tool_spans=2 skill_invoked=1 ``` The added regression test asserts the generated ES|QL includes `load_skill`. It was confirmed to fail on the pre-fix predicate by reverting the one-line change and re-running (`1 failed, 13 skipped`), then pass after restoring. Local gates: ``` tsc : exited with 0 eslint : no eslint errors found jest : 14 passed, 14 total ``` ## Note for reviewers This changes what the Skill Invoked column reports for **all** models. Existing persona-matrix results showing `SkillInvoked = 0` should be treated as unreliable and regenerated rather than compared against post-fix runs. ## Relationship to #283586 This fix is also cherry-picked onto #283586 (`feat/evals-extensions-matrix`), which generates the security LLM performance matrix, so that PR's generated matrix does not ship a Skill Invoked column built from the pre-fix predicate. This PR is the standalone version, based directly on `main`, and can merge independently in either order.
…me (elastic#284661) ## Summary The persona-matrix `SkillInvoked` evaluator matched only `attributes.gen_ai.tool.name == "filestore.read"`, but the agent loads skills via the `load_skill` tool. Verified against the golden cluster over a 7-day window: | tool name | spans (7d) | | --- | --- | | `filestore.read` | 0 | | `load_skill` | 7,991 | Because `total_tool_spans` is still non-zero for these traces, the existing "no tool spans → `unavailable`" guard does not trip. The evaluator therefore returned a confident `score: 0` instead of `null` — silently zeroing the **Skill Invoked** column for every model in every persona-matrix run, not just for one model. This surfaced while generating a matrix column for a model that appeared to never route into any skill. Trace data showed the opposite: it invoked `load_skill` and called security skills directly. The reported zero was a measurement artifact, not a model capability result. ## Why only the tool name changed `load_skill` arguments carry the same `SKILL.md` path the existing predicate already matches: ```json {"skill":"/skills/platform/evals/eval-experiment-authoring/SKILL.md"} ``` so only the tool-name check needed widening. No dataset annotations change. `filestore.read` is retained for backwards compatibility with older traces. ## Verification Same-trace baseline on real golden-cluster data, holding the trace and skill predicate fixed and varying only the tool name: ``` OLD (filestore.read only) total_tool_spans=2 skill_invoked=0 NEW (load_skill accepted) total_tool_spans=2 skill_invoked=1 ``` The added regression test asserts the generated ES|QL includes `load_skill`. It was confirmed to fail on the pre-fix predicate by reverting the one-line change and re-running (`1 failed, 13 skipped`), then pass after restoring. Local gates: ``` tsc : exited with 0 eslint : no eslint errors found jest : 14 passed, 14 total ``` ## Note for reviewers This changes what the Skill Invoked column reports for **all** models. Existing persona-matrix results showing `SkillInvoked = 0` should be treated as unreliable and regenerated rather than compared against post-fix runs. ## Relationship to elastic#283586 This fix is also cherry-picked onto elastic#283586 (`feat/evals-extensions-matrix`), which generates the security LLM performance matrix, so that PR's generated matrix does not ship a Skill Invoked column built from the pre-fix predicate. This PR is the standalone version, based directly on `main`, and can merge independently in either order.
|
buildkite test this |
…me (elastic#284661) ## Summary The persona-matrix `SkillInvoked` evaluator matched only `attributes.gen_ai.tool.name == "filestore.read"`, but the agent loads skills via the `load_skill` tool. Verified against the golden cluster over a 7-day window: | tool name | spans (7d) | | --- | --- | | `filestore.read` | 0 | | `load_skill` | 7,991 | Because `total_tool_spans` is still non-zero for these traces, the existing "no tool spans → `unavailable`" guard does not trip. The evaluator therefore returned a confident `score: 0` instead of `null` — silently zeroing the **Skill Invoked** column for every model in every persona-matrix run, not just for one model. This surfaced while generating a matrix column for a model that appeared to never route into any skill. Trace data showed the opposite: it invoked `load_skill` and called security skills directly. The reported zero was a measurement artifact, not a model capability result. ## Why only the tool name changed `load_skill` arguments carry the same `SKILL.md` path the existing predicate already matches: ```json {"skill":"/skills/platform/evals/eval-experiment-authoring/SKILL.md"} ``` so only the tool-name check needed widening. No dataset annotations change. `filestore.read` is retained for backwards compatibility with older traces. ## Verification Same-trace baseline on real golden-cluster data, holding the trace and skill predicate fixed and varying only the tool name: ``` OLD (filestore.read only) total_tool_spans=2 skill_invoked=0 NEW (load_skill accepted) total_tool_spans=2 skill_invoked=1 ``` The added regression test asserts the generated ES|QL includes `load_skill`. It was confirmed to fail on the pre-fix predicate by reverting the one-line change and re-running (`1 failed, 13 skipped`), then pass after restoring. Local gates: ``` tsc : exited with 0 eslint : no eslint errors found jest : 14 passed, 14 total ``` ## Note for reviewers This changes what the Skill Invoked column reports for **all** models. Existing persona-matrix results showing `SkillInvoked = 0` should be treated as unreliable and regenerated rather than compared against post-fix runs. ## Relationship to elastic#283586 This fix is also cherry-picked onto elastic#283586 (`feat/evals-extensions-matrix`), which generates the security LLM performance matrix, so that PR's generated matrix does not ship a Skill Invoked column built from the pre-fix predicate. This PR is the standalone version, based directly on `main`, and can merge independently in either order.
|
/ci |
There was a problem hiding this comment.
Reviewed the matrix generator, @kbn/evals client changes, and CI wiring. The core matrix build/render/query logic is well-tested and reads cleanly. Left two inline comments: one on what look like unrelated agent-harness files (scripts/kbn-verify.sh / .hermes/environment.json) possibly committed by accident, and one on a trace-key mismatch that makes the opt-in --html trace feature ineffective for real configs.
Generated by Claude Reviewer for #283586 · 170.4 AIC · ⌖ 16.4 AIC · ⊞ 4.6K
| # Explicit filter mode | ||
| FILTER="$2" | ||
| echo "🔍 Explicit filter: $FILTER" | ||
| if node scripts/type_check.js "$FILTER" 2>&1; then |
There was a problem hiding this comment.
scripts/kbn-verify.sh and .hermes/environment.json (which invokes this script as its test phase) don't appear related to the Security LLM matrix feature this PR describes — they look like agent-harness verification scaffolding that may have been committed accidentally. Neither is referenced by the matrix module or the new Buildkite pipelines, and the PR description doesn't mention them. If intentional, worth calling out; if not, they should be dropped from this PR.
Separately, if this script is kept: node scripts/type_check.js "$FILTER" (and the per-package "$PKG" call below) passes a package directory as a positional argument, but the repo convention (.claude/CLAUDE.md) is to scope type-checking with --project <path/to/tsconfig.json>. As written this likely won't scope to the intended package.
| ? '<span class="status err">failed</span>' | ||
| : '<span class="status err">missing</span>'; | ||
|
|
||
| const trace = traces?.[traceKey(row.modelId, col.id)]; |
There was a problem hiding this comment.
The --html trace feature looks like it can never resolve a trace for a realistic config. queryMatrixTraces keys entries by model:exampleId, model:datasetId, and model:suiteId (query_matrix_traces.ts), but here the lookup is traceKey(row.modelId, col.id) where col.id is the matrix column id (e.g. alert_triage, multi_step_execution). Those column ids don't match any suite/dataset/example id — suites are e.g. security-alert-triage — and matrix.ts does no column→suite mapping before calling renderMatrixHtml, so every lookup misses and each prompt renders "Trace unavailable".
The unit test only passes because it uses a contrived config where col.id === suiteId ('alert'). Consider mapping column ids to their config.columns[].suites before the lookup (or keying traces by column id in queryMatrixTraces).
b24fd81 to
f8454e3
Compare
|
/ci |
5 similar comments
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
@elasticmachine merge upstream |
|
/ci |
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
🤖 Prompt Changes DetectedChanges have been detected to one or more prompt files in the Elastic Assistant plugin. Please remember to update the integrations repository with your prompt changes to ensure consistency across all deployments. Next Steps:
This is an automated reminder to help maintain prompt consistency across repositories. |
|
/ci |
1331a4e to
17d96b5
Compare
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
|
/ci |
de16bef to
3ff8f20
Compare
|
PR run: bk-01a014c7-65ab-4b12-83fa-de765d9b74a7::smoke-tests::anthropic-claude-4.5-haiku | Baseline (main): bk-019fcd42-db9f-4d5f-9354-681126bed71a::smoke-tests::anthropic-claude-4.5-haiku
Summary View full comparison in UI | Refresh baseline against latest main (click Unblock in the eval build) No significant changes (5 rows)
|
…sions Adds an HTML matrix report generator to kbn-evals-extensions: - queryMatrixScores: latest experiment per (suite, model) from golden cluster - queryMatrixTraces: latest complete score docs with fallback to earlier experiments - renderMatrixHtml: agent_eval_smoke.html-style report with questions, answers, tool trails - Column-to-suite trace lookup with inline markdown rendering - goldenClusterPrivileges fix: create_doc → create for serverless bulk[s] compatibility - kbn-evals exports: getEvaluationsKbnClient, envFromDatasetsProfile, listExperiments
|
/ci |
cc50c24 to
7ed63d3
Compare
|
/ci |
💔 Build Failed
Failed CI StepsMetrics [docs]Async chunks
History
|
…lastic-kibana-283586
|
Superseded by #285833 — clean rebuild against upstream/main with stale globby migration removed |
Summary
Adds the producer side of the Security LLM performance matrix to
@kbn/evals-extensions, so the matrix artifacts thatelastic/docs-content#6960consumes are generated by CI rather than by hand.The matrix has been iterated on out-of-tree for a while; this lands the generator in Kibana next to the eval suites it reads from.
What's included
Matrix module —
x-pack/platform/packages/shared/kbn-evals-extensions/src/matrix/load_matrix_config— parse/validate the column, composite, layout and model taxonomyquery_matrix_scores— pull scores for the configured suites via the evals experiments APIbuild_matrix— aggregate into per-model/per-column cells, including composites and the "not recommended" floorrender_matrix— emitproprietary-models.csv,open-source-models.csv,matrix.md,matrix.jsonCLI —
node scripts/evals ext matrix --config <path> --out <dir> [--branch <branch>], registered alongside the existingred_teamcommand.@kbn/evalssupportEvalsClient.listExperiments()— pages through the experiments route until the full set is retrieved (the matrix needs every experiment for a branch/build, not one page)createEvaluationsEvalsClient()— thin factory wiring the default client through the existing URL/API-key/version/retry handling, so@kbn/evals-extensionscan build a client without taking a direct@kbn/kbn-clientdependencyBuildkite
security_matrix.ymlpipeline + resource definition (registered inlocations.yml)generate_matrix.sh/upload_matrix_artifacts.sh, publishing togs://<bucket>/<domain>/<latest|version>/— the layout the docs-content sync workflow readsRelationship to other PRs
main. This PR re-lands that work on currentmain, reconciled against changes that have since landed in@kbn/evals(evaluations_kbn_client.tsand theexecutionIdexperiment filters already exist upstream, so only the genuinely additive pieces are included here).#280812added —security-persona-matrixandsecurity-persona-matrix-attack-discovery— into themulti_step_executionandattack_discoverycolumns. They were not represented in any matrix column before.MATRIX_GCS_BUCKETis unset by default, so the upload step is skipped and only a Buildkite artifact is produced until the bucket is configured.Verification
Run locally against this branch:
The end-to-end CI path (pipeline run → GCS upload → docs-content sync) still needs a bucket to be provisioned and cannot be exercised from a PR branch; that is the remaining follow-up before
docs-content#6960can pull real artifacts.