diff --git a/hermes_cli/observability/shared_metrics_events.py b/hermes_cli/observability/shared_metrics_events.py index 87af4fc9a4..b77b87b7ed 100644 --- a/hermes_cli/observability/shared_metrics_events.py +++ b/hermes_cli/observability/shared_metrics_events.py @@ -89,11 +89,9 @@ def finish_compression_attempt( if not pending: return # Classify the stored value, so the outcome and the recorded reason always agree. - reason = fields_.compression_failure_class("failed", failure_class) - outcome = ( - "success" if commit_status == "committed" - else "skipped" if reason in _SKIPPED_COMPRESSION_CLASSES else "failed" - ) + committed = commit_status == "committed" + reason = fields_.compression_failure_class("success" if committed else "failed", failure_class) + outcome = "success" if committed else "skipped" if reason in _SKIPPED_COMPRESSION_CLASSES else "failed" record_compression( trigger=pending[0], outcome=outcome, tokens_before=pending[1], context_length=context_length, failure_class=reason, diff --git a/hermes_cli/observability/shared_metrics_fields.py b/hermes_cli/observability/shared_metrics_fields.py index 0520f4f07d..059ee9bfdd 100644 --- a/hermes_cli/observability/shared_metrics_fields.py +++ b/hermes_cli/observability/shared_metrics_fields.py @@ -131,12 +131,20 @@ _COMPRESSION_OUTCOMES = { } +# Attempt-log classes the closed set names under an existing value: a summary model benched after repeated +# fallbacks is a pre-LLM skip to the deterministic summary, which is what ``feasibility_skip`` counts. +_COMPRESSION_FAILURE_CLASS_ALIASES = {"summary_model_benched": "feasibility_skip"} + + def compression_failure_class(outcome: str, failure_class: Any) -> str: - """The attempt log's class for a skipped/failed attempt, collapsed onto the closed set: - ``exception:`` / ``rollback:`` keep only the prefix (the type name may be a plugin's).""" - if outcome == "success": - return "none" + """The attempt log's class, collapsed onto the closed set: ``exception:`` / ``rollback:`` + keep only the prefix (the type name may be a plugin's). A success reads ``none`` unless it committed + through a fallback (``feasibility_skip``, ``summary_generation_failed``, ``aux_model_fallback``, ...), + which keeps that class so a fallback commit never reads as a clean one.""" value = _norm(failure_class if isinstance(failure_class, str) else None).split(":", 1)[0] + value = _COMPRESSION_FAILURE_CLASS_ALIASES.get(value, value) + if outcome == "success" and value in {"", "none"}: + return "none" if not value: return "unknown" return value if value in contract.COMPRESSION_FAILURE_CLASSES - {"none", "unknown"} else "other" diff --git a/tests/hermes_cli/test_shared_metrics_compression_outcome.py b/tests/hermes_cli/test_shared_metrics_compression_outcome.py new file mode 100644 index 0000000000..929a562674 --- /dev/null +++ b/tests/hermes_cli/test_shared_metrics_compression_outcome.py @@ -0,0 +1,68 @@ +"""``hermes.compression.count`` keeps a committed fallback's class instead of reading as a clean success. + +A committed fallback is either a deterministic summary (the summary model failed, was skipped or benched, +or the attempt stalled) or a main-model summary written after a separate summary model failed. Both commit +and break the prompt cache; reporting them as ``success`` / ``none`` hid them from the metric. +""" + +import json +from pathlib import Path + +import pytest + +from hermes_cli.observability import shared_metrics_events as events +from hermes_cli.observability.shared_metrics import SharedMetricsStore +from hermes_cli.observability.shared_metrics_fields import compression_fields + +SCHEMA_PATH = ( + Path(__file__).resolve().parents[2] / "hermes_cli" / "observability" / "schemas" + / "hermes.shared_metrics.v4.schema.json" +) +RESOURCE = {"architecture": "x86_64", "hermes_version": "test", "install_method": "git", "os_family": "linux"} + + +def _finish(monkeypatch, commit_status, failure_class): + rows = [] + monkeypatch.setattr(events, "record_compression", lambda **kw: rows.append(compression_fields(**kw))) + events.begin_compression_attempt("auto", 80_000) + events.finish_compression_attempt(commit_status, failure_class, 100_000) + return [(row["outcome"], row["failure_class"]) for row in rows] + + +@pytest.mark.parametrize( + ("failure_class", "expected"), + [ + (None, ("success", "none")), + ("summary_generation_failed", ("success", "summary_generation_failed")), + ("feasibility_skip", ("success", "feasibility_skip")), + ("stall_deterministic_fallback", ("success", "stall_deterministic_fallback")), + ("aux_model_fallback", ("success", "aux_model_fallback")), + # A benched summary model is a pre-LLM skip to the deterministic summary. + ("summary_model_benched", ("success", "feasibility_skip")), + # A class outside the closed set still never reads as a clean success. + ("plugin_specific_fallback", ("success", "other")), + ], +) +def test_committed_attempt_keeps_its_fallback_class(monkeypatch, failure_class, expected): + assert _finish(monkeypatch, "committed", failure_class) == [expected] + + +def test_aborted_attempts_keep_their_skip_and_failure_verdicts(monkeypatch): + assert _finish(monkeypatch, "aborted", "lock_contended") == [("skipped", "lock_contended")] + assert _finish(monkeypatch, "aborted", "summary_auth_failure") == [("failed", "summary_auth_failure")] + + +def test_committed_fallback_row_validates_against_the_package_schema(tmp_path): + jsonschema = pytest.importorskip("jsonschema") + store = SharedMetricsStore(tmp_path / "metrics.sqlite3", tmp_path / "outbox") + fields = compression_fields( + trigger="auto", outcome="success", tokens_before=80_000, context_length=100_000, + failure_class="summary_generation_failed", + ) + store.record_counter("hermes.compression.count", fields, RESOURCE) + + [package_path] = store.create_and_export_package() + package = json.loads(package_path.read_text(encoding="utf-8")) + jsonschema.Draft202012Validator(json.loads(SCHEMA_PATH.read_text(encoding="utf-8"))).validate(package) + [row] = [m for m in package["metrics"] if m["name"] == "hermes.compression.count"] + assert row["dimensions"]["failure_class"] == "summary_generation_failed" diff --git a/website/docs/developer-guide/relay-shared-metrics.md b/website/docs/developer-guide/relay-shared-metrics.md index 6fb5c6878f..2ff5610dce 100644 --- a/website/docs/developer-guide/relay-shared-metrics.md +++ b/website/docs/developer-guide/relay-shared-metrics.md @@ -456,7 +456,7 @@ itself ships. | `hermes.setup.completed` | surface (`cli`/`desktop`), provider | Which providers people choose at setup, and on which surface. | | `hermes.model_tokens.sum` | call role, model, provider, auxiliary task, token type | Token volume per model/provider, prompt-cache share, and what auxiliary work (compression, titles, vision, ...) costs. The value is a token sum, not an event count. | | `hermes.model_route.count` `ttft_bucket` | time to first token | Perceived latency per provider/model. | -| `hermes.compression.count` | trigger, outcome, context-fill bucket, failure class | How often compaction runs, how full contexts get, and whether it fails. `skipped` = nothing could fail: lock held elsewhere, nothing summarizable (no model call), user stop, or a newer attempt replaced it. Since package schema v4 the row also carries `failure_class`, the attempt log's own class name from a closed list: the skip reasons (`lock_contended`, `insufficient_messages`, `no_compressible_window`, `empty_post_handoff_window`, `explicit_interrupt`, `attempt_superseded`, `snapshot_stale`) on skipped rows; the failure reasons (`summary_auth_failure`, `summary_network_failure`, `summary_truncated_failure`, `summary_empty_content_failure`, `summary_overload_failure`, `summary_generation_aborted`, `summary_generation_failed`, `summary_overload_degraded`, `aux_model_fallback`, `feasibility_skip`, `stall_deterministic_fallback`, `stall_interrupted`, `would_grow`, `no_progress`, `commit_fence_cancelled`, `pool_saturated`, `session_split_failed`, `exception`, `rollback`) on failed rows; `none` on success. An exception's or rollback's Python type name is dropped (`exception:TimeoutError` reads `exception`), any other value reads `other`, a failure with no class `unknown`. | +| `hermes.compression.count` | trigger, outcome, context-fill bucket, failure class | How often compaction runs, how full contexts get, and whether it fails. `skipped` = nothing could fail: lock held elsewhere, nothing summarizable (no model call), user stop, or a newer attempt replaced it. Since package schema v4 the row also carries `failure_class`, the attempt log's own class name from a closed list: the skip reasons (`lock_contended`, `insufficient_messages`, `no_compressible_window`, `empty_post_handoff_window`, `explicit_interrupt`, `attempt_superseded`, `snapshot_stale`) on skipped rows; the failure reasons (`summary_auth_failure`, `summary_network_failure`, `summary_truncated_failure`, `summary_empty_content_failure`, `summary_overload_failure`, `summary_generation_aborted`, `summary_generation_failed`, `summary_overload_degraded`, `aux_model_fallback`, `feasibility_skip`, `stall_deterministic_fallback`, `stall_interrupted`, `would_grow`, `no_progress`, `commit_fence_cancelled`, `pool_saturated`, `session_split_failed`, `exception`, `rollback`) on failed rows; `none` on a clean success. A success that committed through a fallback keeps that row's class, so `failure_class` other than `none` on a `success` row means "committed with a fallback": a deterministic summary (`summary_generation_failed`, `summary_overload_degraded`, `stall_deterministic_fallback`, or `feasibility_skip`, which also covers a summary model benched after repeated fallbacks) or the main model replacing a failed summary model (`aux_model_fallback`). An exception's or rollback's Python type name is dropped (`exception:TimeoutError` reads `exception`), any other value reads `other`, a failure with no class `unknown`. | | `hermes.model_switch.count` | from/to provider, surface | Which providers people leave and move to. | | `hermes.fallback.count` | from/to provider, error class | How often fallback providers rescue a turn, and from what. | | `hermes.slash_command.count` | command, surface | Which built-in commands are used (`/retry`, `/undo`, `/new` are friction signals). Skill and plugin commands report `skill`/`plugin`. |