diff --git a/.gitignore b/.gitignore index 75d9220..6cc892c 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,9 @@ dist/ build/ .venv/ +# A lockfile for a package with no dependencies pins nothing, and running the tests +# through uv regenerates it. +uv.lock + # The diagram generator is kept locally, not checked in. docs/img/generate.py diff --git a/README.md b/README.md index 5531d04..c5ba8bf 100644 --- a/README.md +++ b/README.md @@ -33,10 +33,11 @@ observation-scoped evaluator does, every step here passes. | `UNVERIFIED_CLAIM` | The reply asserts a write no successful tool backs up. | The only one a user experiences as a lie. They were told something happened that did not happen. | | `TOOL_ERROR` | A tool raised; the framework wrapped it. | The visible half of tool failure, usually already in your dashboards. | | `TOOL_REFUSAL` [^1] | A tool ran fine and **declined in its own result body**, with no error flag. | The dangerous half. Every guard that asks "did the tool run" is satisfied, so a false confirmation ships. | -| `REPEATED_TOOL` | The same tool called 3+ times in one turn. | The model is searching for an argument it was never given. A context gap, not a model failure. | +| `REPEATED_TOOL` [^2] | The same tool called 3+ times **for the same thing**: same arguments, or the same nothing coming back. | The model is searching for an argument it was never given. A context gap, not a model failure. Calling one tool over several ids it was handed is fan-out, and does not count. | | `TOOL_STORM` | 8+ tool calls in one turn. | Same cause, worse. Cost and latency both. | -| `EMPTY_REPLY` [^2] | No text where somebody was owed one. | On a 1:1 channel, the "it just didn't respond" bug. | -| `GATE_FILTERED` [^3] | A turn a relevance gate dropped without doing work. | **Information, not a fault.** Silence is the design. Watch the count for a gate that has started swallowing real traffic. | +| `EMPTY_REPLY` [^3] | No text where somebody was owed one. | On a 1:1 channel, the "it just didn't respond" bug. | +| `GATE_FILTERED` [^4] | A turn a relevance gate dropped without doing work. | **Information, not a fault.** Silence is the design. Watch the count for a gate that has started swallowing real traffic. | +| `ACTED_SILENTLY` [^5] | A turn that did the work and deliberately said nothing. | **Information, not a fault.** On some surfaces "is there work here" and "does anyone need an answer" are separate decisions. Counted rather than merely un-flagged, so the act-only path stays visible. | | `SLOW_TURN` | Wall clock over the threshold. | Usually a storm with a human waiting. | | `NO_CACHE_HIT` | A prompt big enough to cache that read nothing from cache. | Caching is a prefix match, so one volatile byte early in the system prompt drops the discount on *every* turn. | @@ -47,10 +48,18 @@ one is a breaking change. success flag set to `false`. If your tools say no some other way, see [configuring](docs/configuring.md#what-tool_refusal-can-and-cannot-see). -[^2]: Reports at `INFO` until you set `conversational_kinds`, since unconfigured it +[^2]: Keyed on arguments where your adapter maps them, and on tool name alone where it +does not. `coverage()` says which one you are getting. + +[^3]: Reports at `INFO` until you set `conversational_kinds`, since unconfigured it cannot tell a silent channel from a batch job that returns a document. -[^3]: Never fires until you set `quiet_kinds`. Nothing is a gate by default. +[^4]: Never fires until you set `quiet_kinds`. Nothing is a gate by default. + +[^5]: Never fires until you set `act_only_kinds`, which covers turns that acted, not +turns that were idle. Without it, work-then-silence is an `EMPTY_REPLY` everywhere. A +kind can be both this and `conversational_kinds`: the two govern different turns on the +same surface.
@@ -107,6 +116,7 @@ $ python -m postflight --otel tests/fixtures/openinference_support_turn.jsonl Not all detectors are live on this data: GATE_FILTERED: INERT - no quiet_kinds configured, so nothing is silent by design + ACTED_SILENTLY: INERT - no act_only_kinds configured, so acting without replying is scored as EMPTY_REPLY everywhere NO_CACHE_HIT: INERT - no generation reports cache usage, and unknown is not treated as zero ``` diff --git a/docs/configuring.md b/docs/configuring.md index 44c1741..900c1f4 100644 --- a/docs/configuring.md +++ b/docs/configuring.md @@ -47,6 +47,13 @@ Config( # Surfaces fronted by a relevance gate, where silence is correct. A quiet kind is # conversational by definition, so you need not list it in both. quiet_kinds=frozenset({"group.turn"}), + # Surfaces where a turn that ACTS and says nothing is the designed outcome, because + # "is there work here" and "does anyone need an answer" are separate decisions + # there. Those turns report as ACTED_SILENTLY at INFO; work-then-silence stays an + # EMPTY_REPLY everywhere else. Covers acting quietly, not being idle, which is what + # keeps it orthogonal to conversational_kinds: a kind is often both, and a turn here + # that did nothing and said nothing is still that kind's EMPTY_REPLY. + act_only_kinds=frozenset({"group.turn"}), ) ``` @@ -113,11 +120,12 @@ clean agent. | detector | goes quiet if | goes *wrong* if | |---|---|---| | `UNVERIFIED_CLAIM` | the adapter supplies no reply text, or your replies are not in the vocabulary `claim_rules` knows (they are English by default) | your tool names don't match `satisfied_by` / `satisfied_by_prefix`, and a genuine action then reads as an unbacked claim | -| `TOOL_ERROR` · `TOOL_REFUSAL` · `REPEATED_TOOL` · `TOOL_STORM` | the adapter maps no tool spans | | +| `TOOL_ERROR` · `TOOL_REFUSAL` · `REPEATED_TOOL` · `TOOL_STORM` | the adapter maps no tool spans | `REPEATED_TOOL` only: the adapter maps no tool *arguments*, so repeats fall back to keying on tool name and correct fan-out over several ids reads as thrashing | | `SLOW_TURN` | the adapter supplies no timestamps | | | `NO_CACHE_HIT` | no token counts, or the producer reports no cache usage | | | `EMPTY_REPLY` | there are no generations | the adapter fails to extract reply text, and it then fires on **every** turn | | `GATE_FILTERED` | `quiet_kinds` is unset (the default) | | +| `ACTED_SILENTLY` | `act_only_kinds` is unset (the default) | | Note the coupling: a broken reply mapping silences `UNVERIFIED_CLAIM` *and* makes `EMPTY_REPLY` fire on everything. One wrong field, two wrong columns, in opposite @@ -180,7 +188,7 @@ real turns, and every future surface until someone remembers to edit the set. A narrating surface going unflagged is a false positive; a new conversational surface going unflagged is a missed lie. -**Report on faults, not on findings.** `GATE_FILTERED` is `Severity.INFO` because it -fires on correct behaviour. Counting it as a fault makes the headline cry wolf, and a +**Report on faults, not on findings.** `GATE_FILTERED` and `ACTED_SILENTLY` are +`Severity.INFO` because they fire on correct behaviour. Counting it as a fault makes the headline cry wolf, and a detector that cries wolf on the healthy case is how the real rows get ignored. Use `faults()` for anything a human reads first. diff --git a/postflight/config.py b/postflight/config.py index 05d1042..461fb07 100644 --- a/postflight/config.py +++ b/postflight/config.py @@ -230,6 +230,16 @@ class Config: # nothing: the gate passed it, the agent acted, and nobody got an answer. Otherwise # it reports as GATE_FILTERED, which is INFO, not a fault. quiet_kinds: frozenset[str] = frozenset() + # Kinds where a turn that ACTS and says nothing is a designed outcome, because + # "is there work here" and "does anyone need an answer" are separate decisions on + # the surface. Those turns report as ACTED_SILENTLY; everywhere else work-then- + # silence is an EMPTY_REPLY. + # + # Scoped to turns that DID work, which is what keeps it orthogonal to + # `conversational_kinds` — a kind is often both, and a turn here that did nothing + # and said nothing is still that kind's EMPTY_REPLY. `quiet_kinds` cannot express + # any of this: it describes silence BEFORE anything happens. + act_only_kinds: frozenset[str] = frozenset() def __post_init__(self) -> None: unsatisfiable = [ diff --git a/postflight/coverage.py b/postflight/coverage.py index df29149..7b79cb1 100644 --- a/postflight/coverage.py +++ b/postflight/coverage.py @@ -125,8 +125,28 @@ def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage] "tools signal failure another way, add a refusal_predicate", ) ) - for code in ("REPEATED_TOOL", "TOOL_STORM"): - rows.append(Coverage(code, True, f"{len(tool_calls)} tool call(s) visible")) + rows.append( + Coverage("TOOL_STORM", True, f"{len(tool_calls)} tool call(s) visible") + ) + if any(c.arguments is not None for c in tool_calls): + rows.append( + Coverage( + "REPEATED_TOOL", + True, + f"{len(tool_calls)} tool call(s) visible, with arguments to key on", + ) + ) + else: + rows.append( + Coverage( + "REPEATED_TOOL", + True, + "no tool call carries arguments, so repeats can only be keyed on " + "tool NAME, and correct fan-out over several ids reads as " + "thrashing. Check the adapter maps tool input", + misleading=True, + ) + ) # --- EMPTY_REPLY / GATE_FILTERED ----------------------------------------------- if not generations: @@ -164,6 +184,16 @@ def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage] else "no quiet_kinds configured, so nothing is silent by design", ) ) + rows.append( + Coverage( + "ACTED_SILENTLY", + bool(cfg.act_only_kinds), + "act_only_kinds configured" + if cfg.act_only_kinds + else "no act_only_kinds configured, so acting without replying is " + "scored as EMPTY_REPLY everywhere", + ) + ) # --- SLOW_TURN ----------------------------------------------------------------- timed = [t for t in turns if t.duration_s > 0] diff --git a/postflight/detectors.py b/postflight/detectors.py index fb9f4d8..222ebf8 100644 --- a/postflight/detectors.py +++ b/postflight/detectors.py @@ -146,23 +146,112 @@ def detect_unverified_claim(turn: Turn, cfg: Config) -> Iterator[Finding]: ) +def _canonical(value: Any) -> Any: + """Hashable, key-order-insensitive form of an argument or result payload. + + Dict key order is a serialisation artifact, so two calls differing only in key + order have to produce the same key. Sorts on the key alone: sorting on the pair + compares canonicalised values whenever two keys tie, and those are not always + mutually comparable. + """ + if isinstance(value, dict): + return tuple( + sorted( + ((str(k), _canonical(v)) for k, v in value.items()), + key=lambda kv: kv[0], + ) + ) + if isinstance(value, (list, tuple)): + return tuple(_canonical(v) for v in value) + if isinstance(value, (set, frozenset)): + return tuple(sorted(repr(_canonical(v)) for v in value)) + try: + hash(value) + except TypeError: + return repr(value) + return value + + +def _is_empty(result: Any) -> bool: + """True for every shape of nothing: None, or an empty string or container.""" + if result is None: + return True + if isinstance(result, str): + return not result.strip() + if isinstance(result, (bytes, list, tuple, dict, set, frozenset)): + return len(result) == 0 + return False + + +def _argument_key(call: ToolCall) -> Any: + """The grouping key for one call's arguments. + + `arguments is None` means the adapter maps none, not that the call had none. Every + call of a tool then shares one key, which is name-only keying: the detector degrades + to what it did before rather than going silent. + """ + return None if call.arguments is None else _canonical(call.arguments) + + +def _worth_nothing(call: ToolCall, cfg: Config) -> bool: + """True when a call yielded no payload: it errored, declined, or came back empty.""" + return tool_outcome(call, cfg) is not Outcome.OK or _is_empty(call.result) + + +# `{}`, `[]`, `""` and `None` are one answer in different clothes, so they share a key. +_NOTHING = object() + + +def _result_key(result: Any) -> Any: + return _NOTHING if _is_empty(result) else _canonical(result) + + def detect_repeated_tool(turn: Turn, cfg: Config) -> Iterator[Finding]: - """The same tool called N+ times in one turn. + """The same tool called N+ times for the same thing. Usually the model searching for an argument it was never given — a context gap, not a model failure. Fix the prompt, not the temperature. + + Two keys, because a name-only count cannot separate a thrash from fan-out over N + ids the input supplied. + + `arguments` groups calls that asked for the same thing, falling back to name-only + where the adapter maps no arguments. + + `results` groups calls that got the same nothing, whatever they asked for: a thrash + usually varies one id per attempt, so arguments alone would miss it. Restricted to + calls that errored, declined or came back empty, because N identical success bodies + are a bulk write rather than a thrash. The cost of that arm is N searches in one + turn that legitimately found nothing, which reads the same from a trace. """ - repeats = { - name: count - for name, count in Counter(c.name for c in turn.tool_calls).items() - if count >= cfg.repeated_tool - } + calls = turn.tool_calls + if not calls: + return + + repeats: dict[str, int] = {} + basis: dict[str, list[str]] = {} + + def record(name: str, count: int, why: str) -> None: + repeats[name] = max(repeats.get(name, 0), count) + if why not in basis.setdefault(name, []): + basis[name].append(why) + + for (name, _), count in Counter((c.name, _argument_key(c)) for c in calls).items(): + if count >= cfg.repeated_tool: + record(name, count, "arguments") + + for (name, _), count in Counter( + (c.name, _result_key(c.result)) for c in calls if _worth_nothing(c, cfg) + ).items(): + if count >= cfg.repeated_tool: + record(name, count, "results") + if repeats: yield Finding( code="REPEATED_TOOL", turn_id=turn.id, message=f"repeated calls: {repeats}", - detail={"repeats": repeats}, + detail={"repeats": repeats, "basis": basis}, ) @@ -232,18 +321,40 @@ def detect_no_cache_hit(turn: Turn, cfg: Config) -> Iterator[Finding]: def detect_empty_reply(turn: Turn, cfg: Config) -> Iterator[Finding]: """A turn that produced no text where somebody was owed one. - On a `quiet_kind` — a surface fronted by a relevance gate — silence is the product - working, and flagging it drowns the one class this detector exists for, because - most traffic there is dropped. A quiet kind is only suspicious when the gate PASSED it - and the agent did work: tools ran, or the loop went more than one generation, and - then the room got nothing. Otherwise it reports as GATE_FILTERED at INFO, so the - count stays visible for a gate that has started swallowing real traffic. + Two declarations carve out designed silence, and they answer different questions. + + `act_only_kinds` answers "is acting without answering a designed outcome here?" — + that turn, and only that turn, reports as ACTED_SILENTLY at INFO. Checked before the + reply-expectation gate, so declaring the kind is the whole statement; it need not + also be listed as conversational, and often is, since a surface can hold people who + sometimes get an answer and still let the agent act without broadcasting. A turn on + such a kind that did NO work falls through to the rules below, because the + declaration covers acting quietly, not being idle. + + `quiet_kinds` answers "does a relevance gate drop most traffic here?" — silence + WITHOUT work is that gate working and reports as GATE_FILTERED at INFO, keeping the + count visible for a gate that has started swallowing real traffic. + + Everything left is a fault: tools ran, or a gate passed a turn, and a waiting person + got nothing back. """ if not turn.generations or turn.reply.strip(): return + did_work = bool(turn.tool_calls) or len(turn.generations) > 1 + if turn.kind in cfg.act_only_kinds and did_work: + yield Finding( + code="ACTED_SILENTLY", + turn_id=turn.id, + severity=Severity.INFO, + message="acted without replying — declared act-only surface", + detail={ + "tool_calls": len(turn.tool_calls), + "generations": len(turn.generations), + }, + ) + return if not cfg.owes_reply(turn.kind): return - did_work = bool(turn.tool_calls) or len(turn.generations) > 1 if turn.kind in cfg.quiet_kinds and not did_work: yield Finding( code="GATE_FILTERED", diff --git a/tests/test_coverage.py b/tests/test_coverage.py index 363bca4..0a9b0a2 100644 --- a/tests/test_coverage.py +++ b/tests/test_coverage.py @@ -30,7 +30,11 @@ def wired(**kw): cache_read_tokens=0, model="claude-haiku-4-5", ), - ToolCall(name="send_email", result={"sent": True}), + ToolCall( + name="send_email", + arguments={"to": "a@example.com"}, + result={"sent": True}, + ), ), ) return Turn( @@ -46,6 +50,7 @@ def test_a_fully_wired_setup_reports_everything_live(): cfg = Config( conversational_kinds=frozenset({"chat.turn"}), quiet_kinds=frozenset({"group.turn"}), + act_only_kinds=frozenset({"group.turn"}), ) assert all(r.live and not r.misleading for r in coverage([wired()], cfg)) @@ -184,3 +189,27 @@ def test_custom_rules_are_matched_against_your_own_tool_names(): ) def test_gate_filtered_needs_quiet_kinds(configured, live): assert rows([wired()], Config(quiet_kinds=configured))["GATE_FILTERED"].live is live + + +def test_unmapped_tool_arguments_make_repeated_tool_misleading(): + """Without arguments the detector can only key on tool NAME, and correct fan-out + over several ids reads as thrashing — louder than inertness, and just as wrong.""" + got = rows( + [ + wired( + steps=( + Generation(text="hi", input_tokens=10), + ToolCall(name="get_thing", result={"ok": True}), + ) + ) + ] + ) + assert got["REPEATED_TOOL"].live and got["REPEATED_TOOL"].misleading + + +@pytest.mark.parametrize( + "configured,live", [(frozenset({"group.turn"}), True), (frozenset(), False)] +) +def test_acted_silently_needs_act_only_kinds(configured, live): + got = rows([wired()], Config(act_only_kinds=configured))["ACTED_SILENTLY"] + assert got.live is live diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 33eef35..2f6e9d7 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -187,6 +187,99 @@ def test_repeated_and_storm(): assert {"REPEATED_TOOL", "TOOL_STORM"} <= codes(found) +def test_fan_out_over_distinct_ids_is_not_a_repeat(): + """The healthy case the name-only Counter could not see: one tool, N ids the input + named, N different bodies back. That is the model doing what it was told.""" + found = run( + turn( + *[ + tool( + "get_record", + arguments={"record_id": i}, + result={"id": i, "status": "open"}, + ) + for i in (30, 32, 24, 35) + ], + gen("four records"), + ) + ) + assert "REPEATED_TOOL" not in codes(found) + + +def test_distinct_arguments_returning_the_same_nothing_still_repeats(): + """The trap in argument-keying: a real thrash varies one id every attempt. What + separates it from fan-out is that every attempt comes back with the same nothing.""" + found = run( + turn( + *[ + tool("get_record", arguments={"record_id": n}, result={}) + for n in (1, 2, 3) + ], + gen("could not find it"), + ) + ) + assert "REPEATED_TOOL" in codes(found) + assert next(f for f in found if f.code == "REPEATED_TOOL").detail["basis"] == { + "get_record": ["results"] + } + + +def test_distinct_arguments_each_succeeding_identically_is_not_a_repeat(): + """A bulk write returns the same `{"ok": true}` per row. Keying repeats on the + RESULT alone would eat exactly the fan-out this rule exists to stay clear of.""" + found = run( + turn( + *[ + tool( + "update_record", + arguments={"record_id": t}, + result={"ok": True}, + ) + for t in ("a", "b", "c", "d") + ], + gen("noted for all four"), + ) + ) + assert "REPEATED_TOOL" not in codes(found) + + +def test_identical_arguments_still_repeat(): + found = run( + turn( + *[ + tool("search", arguments={"q": "thing"}, result={"hits": [1, 2]}) + for _ in range(3) + ], + gen("done"), + ) + ) + assert "REPEATED_TOOL" in codes(found) + + +def test_argument_key_order_does_not_make_calls_distinct(): + found = run( + turn( + tool("search", arguments={"q": "thing", "limit": 5}, result={"hits": [1]}), + tool("search", arguments={"limit": 5, "q": "thing"}, result={"hits": [1]}), + tool("search", arguments={"q": "thing", "limit": 5}, result={"hits": [1]}), + gen("done"), + ) + ) + assert "REPEATED_TOOL" in codes(found) + + +def test_unmapped_arguments_degrade_to_name_only_keying(): + """An adapter that never populates arguments keeps the detector it had. Silently + losing it would look exactly like an agent that stopped thrashing.""" + found = run( + turn(*[tool("search", result={"hits": [i]}) for i in range(3)], gen("done")) + ) + assert "REPEATED_TOOL" in codes(found) + assert next(f for f in found if f.code == "REPEATED_TOOL").detail["basis"] == { + "search": ["arguments"] + } + + def test_slow_turn_uses_configured_threshold(): slow = turn(gen("done"), seconds=45) assert "SLOW_TURN" not in codes(run(slow)) @@ -296,6 +389,81 @@ def test_quiet_kind_that_did_work_and_said_nothing_is_a_fault(): assert "EMPTY_REPLY" in codes(found) +def test_act_only_kind_that_acted_is_info_not_a_fault(): + """On a surface declared act-only, acting and saying nothing is the SUCCESSFUL + outcome, not a degenerate one.""" + cfg = Config(act_only_kinds=frozenset({"group.turn"})) + found = run( + turn(tool("create_record", result={"ok": True}), gen(""), kind="group.turn"), + cfg, + ) + assert codes(found) == {"ACTED_SILENTLY"} + assert found[0].severity is Severity.INFO + assert faults(found) == [] + + +def test_act_only_kind_with_no_work_still_reports_gate_filtered(): + """The two states are different observations about the same surface — collapsing + them loses the health signal for a gate that has started swallowing real traffic.""" + cfg = Config( + act_only_kinds=frozenset({"group.turn"}), + quiet_kinds=frozenset({"group.turn"}), + ) + assert codes(run(turn(gen(""), kind="group.turn"), cfg)) == {"GATE_FILTERED"} + + +def test_declaring_an_act_only_kind_never_adds_a_fault(): + """A kind nobody declared conversational stays unreported after the declaration. + Making it produce a fault would punish the config that quietens the noise.""" + turn_ = turn(gen(""), kind="bg.turn") + cfg = Config(conversational_kinds=frozenset({"chat.turn"})) + assert codes(run(turn_, cfg)) == set() + declared = Config( + conversational_kinds=frozenset({"chat.turn"}), + act_only_kinds=frozenset({"bg.turn"}), + ) + assert faults(run(turn_, declared)) == [] + + +def test_an_undeclared_kind_that_acted_and_said_nothing_is_still_a_fault(): + """The bug the detector exists for has to survive the new declaration: tools ran and + a waiting person got nothing back.""" + cfg = Config( + conversational_kinds=frozenset({"chat.turn"}), + act_only_kinds=frozenset({"group.turn"}), + ) + found = run( + turn(tool("create_record", result={"ok": True}), gen(""), kind="chat.turn"), cfg + ) + assert codes(found) == {"EMPTY_REPLY"} + assert found[0].severity is Severity.FAULT + + +def test_a_kind_can_be_both_act_only_and_conversational(): + """Not a contradiction: a shared channel holds people who sometimes get an answer + AND lets the agent act without broadcasting. The two declarations govern different + turns on it.""" + cfg = Config( + conversational_kinds=frozenset({"group.turn"}), + act_only_kinds=frozenset({"group.turn"}), + ) + acted = turn(tool("create_record", result={"ok": True}), gen(""), kind="group.turn") + assert codes(run(acted, cfg)) == {"ACTED_SILENTLY"} + assert faults(run(acted, cfg)) == [] + # Nothing done and nothing said is still the bug, because somebody was there. + idle = run(turn(gen(""), kind="group.turn"), cfg) + assert codes(idle) == {"EMPTY_REPLY"} + assert idle[0].severity is Severity.FAULT + + +def test_act_only_kinds_unset_changes_nothing(): + """The default-off guard: every existing path scores exactly as it did before.""" + cfg = Config(conversational_kinds=frozenset({"chat.turn"})) + worked = turn(tool("create_record", result={"ok": True}), gen("")) + assert codes(run(worked, cfg)) == {"EMPTY_REPLY"} + assert run(worked, cfg)[0].severity is Severity.FAULT + + def test_non_conversational_kind_owes_nothing(): cfg = Config(conversational_kinds=frozenset({"chat.turn"})) assert "EMPTY_REPLY" not in codes(run(turn(gen(""), kind="cron.turn"), cfg))