fix(shared-metrics): keep a committed fallback's class on hermes.compression
finish_compression_attempt reported every committed attempt as outcome=success with failure_class=none, so the metric could not tell a clean summary from a commit that went through a fallback: a deterministic summary (summary model failed, pre-LLM feasibility skip, repeated stall, sustained overload) or the main model writing the summary after a separate summary model failed. A committed attempt still reads success (it did commit and break the prompt cache), but now keeps the attempt log's own class when it has one: summary_generation_failed, feasibility_skip, stall_deterministic_fallback, summary_overload_degraded or aux_model_fallback. summary_model_benched, the deterministic commit after the summary model is benched for repeated fallbacks, maps to feasibility_skip, which is the pre-LLM skip it is. Any other class reads other. No new enum values, so the v4 package schema is unchanged and a fallback row still validates. Consumers that read failure_class != none as a failure must now also check outcome: on a success row it means "committed with a fallback". Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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:<Type>`` / ``rollback:<Type>`` 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:<Type>`` / ``rollback:<Type>``
|
||||
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"
|
||||
|
||||
68
tests/hermes_cli/test_shared_metrics_compression_outcome.py
Normal file
68
tests/hermes_cli/test_shared_metrics_compression_outcome.py
Normal file
@@ -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"
|
||||
@@ -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`. |
|
||||
|
||||
Reference in New Issue
Block a user