Skills/memories: agent captures env-specific learnings (corrections + discoveries) as consolidated per-data-source skills

.goal_criteria.md

@@ -0,0 +1,38 @@
+# Sub-agent improvement criteria (set 2026-06-03, before any iteration)
+
+## Metric definitions
+- **Token cost** = LLMResult.total_tokens (prompt + completion + reasoning)
+- **Accuracy** = LLMResult correctness score from the judge (0 or 1), AND
+  the user-facing answer matches expected_output.
+- **Replay efficacy** = on rerun_with_memory evals, replay must:
+  (a) call fetch_skill (skill was deemed relevant), AND
+  (b) keep accuracy = 1.
+
+## Improvement threshold (the goal's "30% on ≥5 evals")
+For ≥5 distinct evals, in the same fixture/data state, the **replay run**
+(memory captured then injected as a skill) must show:
+  - **total_tokens ≥30% lower** than the same eval's suggest=on primary
+    run (i.e. replay tokens ≤ 70% of primary tokens).
+  - **Accuracy = 1** on the replay.
+  - **fetch_skill called** on the replay.
+Baseline for the primary run is the same eval's suggest=on numbers
+captured under the same model (opus-4.6).
+
+## Regression guard
+The 11 existing regression evals (memories_generated: false) — i.e.
+09_crashpod, 12_job_crashing, 24_misconfigured_pvc, 43_current_datetime_from_prompt,
+51_logs_summarize_errors, 61_exact_match_counting,
+101_loki_historical_logs_pod_deleted, 112_find_pvcs_by_uuid,
+176_network_policy_blocking_traffic_no_skills,
+227_count_configmaps_per_namespace, 243_pod_names_contain_service.
+- Average accuracy across the 11 must not drop.
+- Average token cost across the 11 must stay within ±10% of the
+  pre-iteration baseline.
+
+## Verification model
+- All measurements use **opus-4.6 via OpenRouter** (the project's main model).
+
+## Source of truth
+- Local runs in this sandbox produce the iteration numbers.
+- CI on the PR is the final gate — every iteration must also produce
+  green CI (or be the candidate for green CI on the next push).

tests/llm/conftest.py

@@ -789,6 +789,8 @@ def _collect_test_results_from_stats(terminalreporter):
                     "braintrust_root_span_id": None,
                     "clean_test_case_id": None,  # Not available for skipped tests
                     "env_config": "default",  # Not available for skipped tests
+                    "memories_count": 0,
+                    "suggested_memories": [],
                 }
                 continue
             elif when != "call":
@@ -863,6 +865,40 @@ def _collect_test_results_from_stats(terminalreporter):
                 ),  # Any throttling during execution
                 "model": user_props.get("model", "Unknown"),
                 "env_config": user_props.get("env_config", "default"),
+                "memories_count": user_props.get("memories_count", 0),
+                "suggested_memories": user_props.get("suggested_memories", []),
+                # Whether the primary pass (everything before the replay
+                # block) passed. Used by the GitHub reporter to keep the
+                # primary row ✅ even if the replay assertion fails the
+                # whole test.
+                "primary_passed": user_props.get("primary_passed", False),
+                # Replay-with-memory results (only populated when the test
+                # set rerun_with_memory: true AND a memory was actually
+                # captured on the first pass).
+                "replay_attempted": user_props.get("replay_attempted", False),
+                "replay_skill_loaded": user_props.get("replay_skill_loaded", False),
+                "replay_correctness": user_props.get("replay_correctness"),
+                "replay_turns": user_props.get("replay_turns"),
+                "replay_tool_calls_count": user_props.get("replay_tool_calls_count"),
+                "replay_skill_count": user_props.get("replay_skill_count"),
+                "replay_duration": user_props.get("replay_duration"),
+                "replay_total_cost": user_props.get("replay_total_cost"),
+                "replay_total_tokens": user_props.get("replay_total_tokens"),
+                "replay_prompt_tokens": user_props.get("replay_prompt_tokens"),
+                "replay_completion_tokens": user_props.get(
+                    "replay_completion_tokens"
+                ),
+                "replay_cached_tokens": user_props.get("replay_cached_tokens"),
+                "replay_reasoning_tokens": user_props.get(
+                    "replay_reasoning_tokens"
+                ),
+                "replay_max_completion_tokens_per_call": user_props.get(
+                    "replay_max_completion_tokens_per_call"
+                ),
+                "replay_max_prompt_tokens_per_call": user_props.get(
+                    "replay_max_prompt_tokens_per_call"
+                ),
+                "replay_num_compactions": user_props.get("replay_num_compactions"),
                 "clean_test_case_id": user_props.get("clean_test_case_id"),
                 "braintrust_span_id": user_props.get("braintrust_span_id"),
                 "braintrust_root_span_id": user_props.get("braintrust_root_span_id"),

tests/llm/fixtures/test_ask_holmes/09_crashpod/test_case.yaml

@@ -53,3 +53,5 @@ tags:
   - kubernetes
   - one-test
   - regression
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/test_case.yaml

@@ -87,3 +87,5 @@ before_test: |
 after_test: |
   # Delete namespace in background to avoid hanging
   kubectl delete namespace app-101 --wait=false || true
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/112_find_pvcs_by_uuid/test_case.yaml

@@ -30,3 +30,5 @@ before_test: |
   fi
 after_test: |
   kubectl delete -f ./manifest.yaml
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/12_job_crashing/test_case.yaml

@@ -38,3 +38,5 @@ after_test: |
 tags:
   - easy
   - regression
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/176_network_policy_blocking_traffic_no_skills/test_case.yaml

@@ -36,3 +36,5 @@ before_test: |
   fi
 after_test: |
   kubectl delete namespace app-176 || true
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/227_count_configmaps_per_namespace/test_case.yaml

@@ -63,3 +63,5 @@ after_test: |
   for NS in payments inventory shipping analytics notifications; do
     kubectl delete namespace "app-227-$NS" || true
   done
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/243_pod_names_contain_service/test_case.yaml

@@ -28,3 +28,5 @@ tags:
   - kubernetes
   - medium
   - regression
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/24_misconfigured_pvc/test_case.yaml

@@ -30,3 +30,5 @@ before_test: |
 after_test: |
   kubectl delete -f manifest.yaml -n app-24
   kubectl delete namespace app-24 || true
+
+memories_generated: false

tests/llm/fixtures/test_ask_holmes/261_es_log_severity_field/test_case.yaml

@@ -0,0 +1,138 @@
+# Test: ES log severity field is named "severity" not "level"
+#
+# This eval forces a real failed→succeeded tool-call correction. The test
+# index contains 10 ERROR-severity documents, but they use a custom
+# "severity" field instead of the standard "level" field used by most
+# log schemas. A fresh LLM defaulting to a level:"ERROR" query returns
+# zero hits; the only way to find the data is to inspect the mapping or
+# a sample doc, then re-query with severity:"ERROR".
+#
+# The LLM should capture this lesson as an env-specific memory
+# ("this index uses 'severity' for log levels, not 'level'").
+
+# Phrasing matters: by saying "where level is ERROR" we describe the
+# question using the conventional log-field terminology, which biases
+# the agent into running `level:"ERROR"` as its first query. That call
+# returns zero hits (this index has only `severity`, no `level`) and
+# the agent has to read the mapping and re-query with `severity:"ERROR"`
+# to recover. THAT correction is the durable env-specific lesson the
+# suggest_skills tool should capture.
+#
+# An earlier iteration tried a softer phrasing ("how many ERROR-level
+# entries are there") and discovered the agent solves it on the first
+# try (presumably from mapping enrichment) without ever issuing the
+# wrong call — and therefore correctly emits zero memories. That's a
+# different eval (whether emission happens on natural queries) than
+# what this fixture is meant to test (whether the correction shape is
+# captured when one is forced). Keep the biased phrasing here so the
+# wrong→right correction is reliably triggered.
+user_prompt: 'In the elasticsearch index ''app-261-logs-trk8m2p9'', run a `term: { level: "ERROR" }` query and tell me how many hits come back. If that returns zero, find the right field for log level and re-run the query.'
+
+# Replay prompt: a natural, un-biased version of the same question. The
+# primary pass uses the biased phrasing above to force the wrong→right
+# correction (and reliably emit a memory). The replay simulates a future
+# investigation asking the question naturally — which is when the
+# captured skill should actually short-circuit the failed call. Without
+# this softer phrasing the agent on replay just follows the explicit
+# recovery instructions in the primary prompt and never bothers
+# fetching the skill.
+replay_user_prompt: "In the elasticsearch index 'app-261-logs-trk8m2p9', how many ERROR-level log entries are there for the checkout-261 service?"
+
+expected_output:
+  - "There are 10 entries with ERROR severity / level in app-261-logs-trk8m2p9"
+
+memories_generated: true
+rerun_with_memory: true
+
+tags:
+  - elasticsearch
+  - question-answer
+  - medium
+  - regression
+  - skills
+
+setup_timeout: 300
+
+before_test: |
+  source ../../shared/es_test_utils.sh
+  es_setup
+  set -e
+
+  export HOLMES_ES_TEST_INDEX="app-261-logs-trk8m2p9"
+  echo "Using test index: $HOLMES_ES_TEST_INDEX"
+
+  # Clean up any existing index first
+  curl -sf -X DELETE "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" || true
+
+  echo "⏳ Creating test index with explicit mapping (severity, not level)..."
+
+  # Create the index with explicit mapping — note: NO "level" field exists,
+  # only "severity". This is the env-specific quirk this eval teaches.
+  CREATE_RESPONSE=$(curl -sf -X PUT "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}?wait_for_active_shards=1" \
+    -H "Content-Type: application/json" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" \
+    -d '{
+      "settings": { "number_of_shards": 1, "number_of_replicas": 0 },
+      "mappings": {
+        "properties": {
+          "severity": { "type": "keyword" },
+          "message":  { "type": "text" },
+          "service":  { "type": "keyword" },
+          "@timestamp": { "type": "date" }
+        }
+      }
+    }')
+
+  if ! echo "$CREATE_RESPONSE" | grep -q '"acknowledged":true'; then
+    echo "❌ Failed to create index: $CREATE_RESPONSE"
+    exit 1
+  fi
+  sleep 2
+
+  # 10 ERROR, 5 WARN, 15 INFO — all using the "severity" field. Pad the
+  # seconds field so i >= 10 still produces a valid ISO timestamp; an
+  # earlier version of this fixture concatenated "0${i}" which produced
+  # "12:00:010" for i=10 and ES rejected the doc.
+  BULK_DATA=""
+  for i in $(seq 1 10); do
+    TS=$(printf '2026-05-14T12:00:%02dZ' "$i")
+    BULK_DATA="${BULK_DATA}{\"index\":{}}\n{\"severity\":\"ERROR\",\"service\":\"checkout-261\",\"message\":\"DB connection refused $i\",\"@timestamp\":\"${TS}\"}\n"
+  done
+  for i in $(seq 1 5); do
+    TS=$(printf '2026-05-14T12:01:%02dZ' "$i")
+    BULK_DATA="${BULK_DATA}{\"index\":{}}\n{\"severity\":\"WARN\",\"service\":\"checkout-261\",\"message\":\"Slow query $i\",\"@timestamp\":\"${TS}\"}\n"
+  done
+  for i in $(seq 1 15); do
+    TS=$(printf '2026-05-14T12:02:%02dZ' "$i")
+    BULK_DATA="${BULK_DATA}{\"index\":{}}\n{\"severity\":\"INFO\",\"service\":\"checkout-261\",\"message\":\"Health check $i\",\"@timestamp\":\"${TS}\"}\n"
+  done
+
+  BULK_RESPONSE=$(echo -e "$BULK_DATA" | curl -sf -X POST "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}/_bulk" \
+    -H "Content-Type: application/x-ndjson" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" \
+    --data-binary @-)
+
+  if echo "$BULK_RESPONSE" | grep -q '"errors":true'; then
+    echo "❌ Bulk insert had errors: $BULK_RESPONSE"
+    exit 1
+  fi
+
+  curl -sf -X POST "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}/_refresh" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" > /dev/null
+
+  DOC_COUNT=$(curl -sf -X GET "${ELASTICSEARCH_URL}/${HOLMES_ES_TEST_INDEX}/_count" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" | grep -o '"count":[0-9]*' | cut -d':' -f2)
+
+  if [ "$DOC_COUNT" = "30" ]; then
+    echo "✅ Index created with $DOC_COUNT docs (10 ERROR / 5 WARN / 15 INFO) using 'severity' field"
+  else
+    echo "❌ Expected 30 documents, found: $DOC_COUNT"
+    exit 1
+  fi
+
+after_test: |
+  echo "⏳ Cleaning up test index: app-261-logs-trk8m2p9"
+  curl -sf -X DELETE "${ELASTICSEARCH_URL}/app-261-logs-trk8m2p9" \
+    -H "Authorization: ApiKey ${ELASTICSEARCH_API_KEY}" || true
+  echo "✅ Cleanup complete"

tests/llm/fixtures/test_ask_holmes/261_es_log_severity_field/toolsets.yaml

@@ -0,0 +1,15 @@
+# Shared Elasticsearch toolset configuration for eval tests
+# Copy this content to test toolsets.yaml files
+toolsets:
+  elasticsearch/data:
+    enabled: true
+    config:
+      api_url: "{{ env.ELASTICSEARCH_URL }}"
+      api_key: "{{ env.ELASTICSEARCH_API_KEY }}"
+      verify_ssl: true
+  elasticsearch/cluster:
+    enabled: true
+    config:
+      api_url: "{{ env.ELASTICSEARCH_URL }}"
+      api_key: "{{ env.ELASTICSEARCH_API_KEY }}"
+      verify_ssl: true