diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e8a7b96..c27308b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,11 +1,16 @@ name: Release -# Tag push publishes. The human gate is the `pypi` environment below, which pauses the -# run BEFORE the OIDC token is minted — so the approval exists regardless of what -# triggered the workflow, and a tag pushed by accident cannot reach PyPI unattended. +# Publishing a GitHub Release is the gate. Not a tag push: a tag is cheap to create by +# accident and impossible to take back once it has reached PyPI, whereas a Release is a +# deliberate act with the notes in front of you. +# +# The `pypi` environment below is the second gate, and the stronger one — it pauses the +# run BEFORE the OIDC token is minted. Environment protection rules are unavailable on a +# private repo under this plan, so it currently holds none; add a required reviewer once +# the repo is public and this becomes belt-and-braces. on: - push: - tags: ['v*'] + release: + types: [published] workflow_dispatch: permissions: @@ -55,12 +60,12 @@ jobs: # sigstore signing job would be duplicative. - uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 - # Its own job so the publish job keeps `id-token: write` as its ONLY permission. This - # one needs `contents: write` to create the release, and has no business holding the - # publishing identity while it does. - github-release: + # Attaching the built artifacts to the Release that triggered this. Its own job so the + # publish job keeps `id-token: write` as its ONLY permission — `contents: write` has no + # business sitting alongside the publishing identity. + attach-artifacts: needs: [publish] - if: startsWith(github.ref, 'refs/tags/') + if: github.event_name == 'release' runs-on: ubuntu-latest permissions: contents: write @@ -69,13 +74,7 @@ jobs: with: name: dist path: dist/ - # --generate-notes builds the body from merged PRs, categorised by .github/release.yml. - # Release notes are therefore a side effect of labelling PRs rather than a file - # anyone has to remember to update. - env: GH_TOKEN: ${{ github.token }} - run: | - gh release create "${GITHUB_REF_NAME}" dist/* \ - --repo "${GITHUB_REPOSITORY}" \ - --title "${GITHUB_REF_NAME}" \ - --generate-notes + TAG: ${{ github.event.release.tag_name }} + run: gh release upload "$TAG" dist/* --repo "${GITHUB_REPOSITORY}" --clobber diff --git a/.gitignore b/.gitignore index c0740a6..75d9220 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,6 @@ __pycache__/ dist/ build/ .venv/ + +# The diagram generator is kept locally, not checked in. +docs/img/generate.py diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..3a54448 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,63 @@ +# AGENTS.md + +Instructions for coding agents working in this repository. +Human contributors want [CONTRIBUTING.md](CONTRIBUTING.md); this file only covers the +constraints that are invisible from the code and expensive to violate. + +## Checks + +```bash +pip install pytest && python -m pytest tests/ -q +ruff check . && ruff format --check . +``` + +Lint runs over the whole repository, markdown included, because ruff formats fenced +Python inside it. Scoping a local run to `postflight/` and `tests/` is how local and CI +end up disagreeing. + +## Constraints + +**No dependencies. Ever.** `dependencies = []` is the reason this package can be added to +anything without a version negotiation, and a test enforces it. The HTTP clients use +`urllib` rather than `httpx` on purpose. An adapter parses exported JSON rather than +importing a vendor SDK: the producer needs the SDK, the reader does not. + +**Detector codes are the public API.** `UNVERIFIED_CLAIM`, `TOOL_REFUSAL` and the rest +are what callers filter, chart and page on. Renaming one is a breaking change. Adding a +detector is not. + +**Unknown is not zero.** `cache_read_tokens=None` means the producer does not report +cache usage; `0` means it reported none. Detectors must skip the first rather than treat +absence as evidence. The same shape recurs elsewhere: never infer a problem from a +missing signal. + +**Kind-keyed rules are deny-lists, never allow-lists.** `Turn.kind` falls back to +`"unknown"` when an adapter cannot resolve it, so an allow-list silently exempts real +turns and every future surface. Failing closed is the point. + +**Detectors are mechanism; vocabulary is configuration.** Tool names, claim phrasings, +thresholds and surface names belong in `Config`, supplied by the caller. If a change +teaches the package something about a particular product or domain, it is in the wrong +place. + +**A detector that fires on healthy behaviour is worse than one that misses.** Every +narrowing in `detectors.py` exists because something cried wolf. Before widening a rule, +add the case it must NOT fire on. Both directions get a test. + +## Writing + +Comments explain **why the code is the way it is**, not what it does and not the story of +how the problem was found. Incident narratives, measured percentages and production +counts do not belong here. + +Public copy (README, `docs/`, `CONTRIBUTING`, `SECURITY`, and anything the CLI prints) +uses no em-dashes. Rewrite the sentence rather than swapping in a comma, which produces +splices. + +Release notes live on the Releases page and are generated from merged PR titles. There is +no changelog file; do not add one. + +## Do not touch + +`docs/img/*.svg` are generated, with text converted to outlines. They are path data, not +editable markup, and the generator is not in this repository. Leave them alone. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 310e74f..f441e3f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,7 @@ # Contributing +
+ ## Running it ```bash @@ -10,24 +12,28 @@ python -m pytest tests/ -q ``` There is no install step and no dependency file to sync. If `pip install pytest` is not -enough to run the suite, that is a bug in this project, not in your setup — the package +enough to run the suite, that is a bug in this project, not in your setup. The package is zero-dependency and CI proves it by installing nothing else. +
+ ## What a good change looks like **A new detector** needs a name that describes the *behaviour*, not a metric, and a test -for both directions — the case it must catch and the case it must not. The second is the +for both directions: the case it must catch and the case it must not. The second is the one that matters: a detector that fires on healthy traffic is how the real findings get ignored, and every detector here was narrowed at least once because of that. **A new adapter** is a function from your trace format to `Turn`. Read "Writing an adapter" in the README first; the four notes there are each a mistake already made once. -Adapters must not add a dependency — parse exported JSON rather than importing a vendor +Adapters must not add a dependency. Parse exported JSON rather than importing a vendor SDK. **Detector codes are the public interface.** Renaming one is a breaking change, so it lands as a minor bump with a changelog entry, not quietly. +
+ ## Tuning vs mechanism The line this project is organised around: detectors are mechanism and live in code; @@ -35,13 +41,36 @@ thresholds, tool-name conventions and claim vocabulary are tuning and live in `C If a change makes postflight know something about *your* domain, it probably belongs in your `Config`, not in here. Say so in the PR if you think it's the exception. +
+ ## Before you open a PR - `python -m pytest tests/ -q` passes - New behaviour has a test - A comment explains *why the code is the way it is*, where that is not obvious. Not what it does, and not the story of how it was found -- The PR title reads as a release note — it becomes one verbatim, since release notes are +- The PR title reads as a release note, because it becomes one verbatim: release notes are generated from merged PRs at tag time +If you are working through a coding agent, point it at [AGENTS.md](AGENTS.md), which carries the constraints that are invisible from the code. + Issues and PRs are welcome. There is no response SLA. + +
+ +## Releasing + +`main` requires a pull request and green checks; direct pushes are blocked for everyone +without an admin bypass. + +1. Merge everything you want in the release, with PR titles that read as release notes. + they become the notes verbatim, categorised by label via `.github/release.yml`. +2. Bump `version` in `pyproject.toml` and `__version__` in `postflight/__init__.py` + (a test fails if they disagree), via a PR like any other change. +3. Tag it: `git tag v0.1.0 && git push origin v0.1.0`. A tag alone publishes nothing. +4. Create the GitHub Release for that tag with generated notes. **Publishing the Release + is what triggers the PyPI upload.** A tag is easy to push by accident and impossible + to retract once it has reached PyPI, so the deliberate act is the gate. + +The workflow builds, checks the metadata, publishes via Trusted Publishing (no API token +exists), and attaches the artifacts back to the Release. diff --git a/README.md b/README.md index 7cafdf4..4a0b42c 100644 --- a/README.md +++ b/README.md @@ -5,36 +5,77 @@ [![Python versions](https://img.shields.io/pypi/pyversions/postflight)](https://pypi.org/project/postflight/) [![License](https://img.shields.io/badge/license-Apache--2.0-blue.svg)](LICENSE) -**Turn-level failure detection for tool-calling agents.** +### Turn-level failure detection for tool-calling agents -Your evals score what the model *said*. These are the failures that happen in the gaps -between what it said and what it did — a tool that declined three steps before the reply -that contradicts it, the same read issued eight times, a cache that never warmed. They -are invisible to an evaluator scoped to one observation, which is what every tracing -platform's evaluator runtime gives you today. +postflight reads agent traces you already emit and returns coded findings for failures +that span a whole turn: a tool that declined three steps before the reply contradicting +it, the same read issued eight times, a cache that never warmed. It calls no model and +has no dependencies. + +
+ + + + One turn of four steps: a generation that plans, a search tool, a notification tool returning {"sent": false}, and a reply saying "I've let them know." A bracket labelled UNVERIFIED_CLAIM spans the last two. Below, each step is scored on its own and every one passes. + + +
+ +The failure is a relationship between steps. Scored one at a time, which is what an +observation-scoped evaluator does, every step here passes. + +
## The taxonomy | Code | What it means | Why it matters | |---|---|---| -| `UNVERIFIED_CLAIM` | The reply asserts a write that no successful tool backs up. | The only one a user experiences directly 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` | A tool ran fine and **declined in its own result body**, in a shape you've told postflight about (by default, a success flag set to `false`). | The dangerous half. Every guard that asks "did the tool run" is satisfied, so a false confirmation sails through. **Convention-dependent — see below.** | -| `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 — fix the prompt. | +| `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. | | `TOOL_STORM` | 8+ tool calls in one turn. | Same cause, worse. Cost and latency both. | -| `EMPTY_REPLY` | The turn produced no text where somebody was owed one. | On a 1:1 channel this is the "it just didn't respond" bug. Reports at INFO until you set `conversational_kinds` — unconfigured, postflight can't tell a silent channel from a batch job that returns a document. | -| `GATE_FILTERED` | 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` [^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. | | `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 silently drops the discount on *every* turn. | +| `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. | -The codes are the stable interface. Filter on them, chart them, page on them. +The codes are the stable interface. Filter on them, chart them, page on them. Renaming +one is a breaking change. -## Use +[^1]: Detects an in-body decline in a shape you have told it about. The default is a +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 +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. + +
+ +## Install ```bash pip install postflight ``` +No dependencies. Python 3.11+. + +
+ +## Use + +The CLI is what a cron job or a CI step wants. Exit status is 1 when something faulted, +0 otherwise: + +```bash +python -m postflight --langfuse --hours 24 +python -m postflight --otel spans.jsonl +``` + +From Python: + ```python from postflight import Config, faults, run_all from postflight.adapters.langfuse import LangfuseAdapter, LangfuseClient @@ -47,49 +88,58 @@ for turn_id, findings in run_all(turns, Config()).items(): print(turn_id, finding.code, finding.message) ``` -No dependencies. Python 3.11+. +`run_all` returns every finding, including `Severity.INFO` ones like `GATE_FILTERED`. +Wrap it in `faults()` for anything a human reads first. -## What it actually prints +
-Run it against the trace shipped in `tests/fixtures/` — a real OpenInference capture of a -support agent whose notification tool declined: +## Output + +Run against the fixture, a support agent built to fail this way and captured through +OpenInference: ``` -0x4069cd95… TOOL_REFUSAL 1 tool call(s) declined in-body +$ python -m postflight --otel tests/fixtures/openinference_support_turn.jsonl + 0x4069cd953e TOOL_REFUSAL 1 tool call(s) declined in-body + +1 turns, 1 flagged + TOOL_REFUSAL 1 + +Not all detectors are live on this data: + GATE_FILTERED: INERT - no quiet_kinds configured, so nothing is silent by design + NO_CACHE_HIT: INERT - no generation reports cache usage, and unknown is not treated as zero ``` -The `detail` dict is the part you act on: +Each finding carries a `detail` dict, which is the part you act on: ```json {"calls": [{"tool": "send_notification", "result": "{'sent': False, 'reason': 'channel unavailable'}"}]} ``` -Note what did **not** fire. The agent's reply said *"I wasn't able to send the -notification"* — an honest report of a failure, not a claim — so `UNVERIFIED_CLAIM` -stayed quiet. Had it said "I've let the customer know", that same turn would have -produced the finding you actually want to be paged about. +The closing block comes from `coverage()`, and it is worth reading before the findings. +A detector whose input is missing does not error, it just never fires, and an empty +column looks the same as a clean agent. `coverage()` reports which detectors could not +have fired on this data, so a zero elsewhere means something. + +Expect `SLOW_TURN` and `GATE_FILTERED` to dominate any real window and expect the rare +rows to carry the weight. Sort by severity, not by count. -Expect the counts to be lopsided, and expect that to be the useful part. `SLOW_TURN` -and `GATE_FILTERED` dominate any real window — one is already visible to whoever waited, -and the other is a surface behaving correctly. The rare rows carry the weight: a single -`TOOL_REFUSAL` or `UNVERIFIED_CLAIM` is a user who was told something untrue, and nothing -else in your stack is going to raise it. Sort by severity, not by count. +
-## Writing an adapter +## The `Turn` contract -Detectors never see your trace format. They read `Turn`, so an adapter is a function from -whatever you have to an ordered sequence of steps: +Detectors never see your trace format. They read `Turn`, so an adapter is a function +from whatever you have to an ordered sequence of steps: ```python from postflight.model import Generation, ToolCall, Turn Turn( - # your trace/turn identifier id="…", - # the SURFACE — which agent, which channel + # the surface: which agent, which channel kind="chat.turn", - # ORDERED: the sequence is the signal + # ordered, because the sequence is the signal steps=( Generation(text="", input_tokens=900, model="…"), ToolCall(name="search", result={"count": 0}), @@ -98,198 +148,33 @@ Turn( ) ``` -That is the whole contract. Four notes, each of which cost something to learn: +A turn is a sequence, not a tree. Flattening is what lets a detector see a tool that +declined several steps before the reply contradicting it. -- **Order matters more than nesting.** A turn is a sequence, not a tree. That is what - lets a detector see a tool that declined three steps before the reply contradicting it - — the thing a per-observation evaluator structurally cannot do. Flatten your tree. -- **`ToolCall.is_error` is the TRANSPORT flag only** — an exception, an `isError`, an - `ERROR` span status. A tool that ran fine and declined in its own body is *not* an - error; leave `is_error=False`, put the body in `result`, and `success_flags` will - classify it. Conflating the two hides the more dangerous failure. -- **Unknown is not zero.** If your producer does not report cache usage, leave - `cache_read_tokens=None`. Passing `0` asserts a cache miss, and `NO_CACHE_HIT` will - believe you. (This is exactly how the OTel adapter got it wrong first.) -- **`kind` should fall back to `"unknown"`, never to a guess.** Every kind-keyed rule in - this package is a deny-list so that unknown fails *closed*; a plausible-looking default - would quietly exempt the turns you most want checked. +Adapters ship for Langfuse and for OpenTelemetry / OpenInference. Copy +`postflight/adapters/otel.py`, which handles flattened attributes, a span tree, and two +timestamp encodings, so most of the awkward cases are already worked out. Read +[writing an adapter](docs/configuring.md#writing-an-adapter) first; its three notes +are each a mistake already made once. -`postflight/adapters/otel.py` is ~150 lines and is the one to copy — it deals with -flattened attributes, a span tree, and two timestamp encodings, so most of the awkward -cases are already worked out there. +
-## Tuning it to your agent +## Configuring -The detectors are mechanism; the vocabulary is yours. Everything below is a `Config` -field, and the shipped defaults are a starting point, not a claim of completeness — -they are what a tool-calling agent looks like before you have watched *yours* fail. +Thresholds, claim vocabulary, which surfaces owe a reply, and what each detector needs +in order to fire at all: **[docs/configuring.md](docs/configuring.md)**. -```python -Config( - slow_turn_s=30.0, - tool_storm=6, - # Tools that decline in-body, by the key they set to False. - success_flags=("ok", "updated", "sent", "created"), - # Surfaces that owe a human a reply. Setting this is what promotes EMPTY_REPLY from - # INFO to a fault — leave it empty and postflight cannot tell a silent channel from - # a batch job that returns a document, so it counts them instead of blaming them. - conversational_kinds=frozenset({"chat.turn", "inbound.turn", "group.turn"}), - # Surfaces that narrate rather than speak. A digest summarising someone's history - # uses the same words a claim does, with no user and no write in the turn. - narrating_kinds=frozenset({"digest.turn"}), - # Surfaces fronted by a relevance gate, where silence is correct. A quiet kind is - # conversational by definition — you do not have to list it in both. - quiet_kinds=frozenset({"group.turn"}), -) -``` - -### What `TOOL_REFUSAL` can and cannot see - -This detector does **not** assume your tools return `{"updated": false}`. It assumes you -tell it how your tools say no. Out of the box it recognises four shapes: - -| shape | verdict | -|---|---| -| the call raised — `is_error` set by the adapter | `TOOL_ERROR` | -| the framework's error string (`Error executing tool …`) | `TOOL_ERROR` | -| a truthy `error` key in the result | `TOOL_ERROR` | -| a key from `success_flags` set to `false` | `TOOL_REFUSAL` | - -Anything else reads as success. If your tools signal failure some other way — a -`status` field, an enum, an HTTP-ish code — **`TOOL_REFUSAL` will never fire and your -report will look clean**. Add your convention: - -```python -Config( - refusal_predicates=( - lambda r: isinstance(r, dict) and r.get("status") in {"failed", "declined"}, - ) -) -``` - -There is no default for that, deliberately. `{"status": "failed"}` returned by a -`get_job_status` tool describes the *job*, not the call — guessing would make every -healthy status read into a refusal, which is precisely the cry-wolf failure this package -exists to avoid. You know which of your tools report on themselves; postflight doesn't. - -Two things it will never infer, by design: an **empty result set** (a search that found -nothing is not a decline) and **prose** (`"No matching orders found."` is -indistinguishable from success without reading it). If a tool of yours only fails in -prose, the durable fix is in the tool, not here. - -### What the other detectors depend on - -Same class of problem, and the reason `coverage()` exists: a detector whose input is -missing does not error, it just never fires — and an empty column reads exactly like a -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` — then a genuine action reads as an unbacked claim | -| `TOOL_ERROR` · `TOOL_REFUSAL` · `REPEATED_TOOL` · `TOOL_STORM` | the adapter maps no tool spans | — | -| `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 — then it fires on **every** turn | -| `GATE_FILTERED` | `quiet_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 -directions. - -So check rather than assume: - -```python -from postflight import coverage - -for row in coverage(turns, cfg): - print(row) # e.g. "NO_CACHE_HIT: INERT — no generation reports cache usage" -``` - -It reports structural inertness only — an input absent from every turn. It will not tell -you a detector is broken because its count is zero, because a tool that never errored is -a healthy agent, and conflating those would just move the problem up a level. - -`refusal_exemptions` is the other direction — shapes that look like refusals and are -not. The shipped one is `{"sent": false, "queued": true}`: a send handed off to a relay. -Exemptions outrank `refusal_predicates`, so widening your detection cannot silently -re-flag a path you already excused. - -The one worth real attention is `claim_rules`, which drives `UNVERIFIED_CLAIM`. A rule -pairs a regex against the tools that would make the claim true: - -```python -from postflight import ClaimRule, Config -import re - -Config( - claim_rules=( - ClaimRule( - name="ticket_filed", - pattern=re.compile( - r"\b(?:filed|opened|created)\b[^.\n]{0,40}\bticket\b", re.IGNORECASE - ), - satisfied_by=frozenset({"create_ticket", "escalate_to_support"}), - ), - ) -) -``` - -Matching reads the clause the match sits in, and skips it on two conditions: - -- **Negation.** "The follow-up was **not** sent" is the agent being honest about not - acting, and scoring that as a lie punishes exactly the behaviour you want. -- **A third-party subject.** "**The owner** emailed you" is the agent relaying what - someone else did. The verb-object pair is identical to a real claim; only the subject - differs, and relaying is ordinary in any reply that summarises a thread. - -Both are `Config` regexes (`negation`, `third_party_subject`) if your replies read -differently. - -## Two design rules worth knowing before you extend it - -**Kind-keyed exemptions are deny-lists, never allow-lists.** `Turn.kind` falls back to -`"unknown"` whenever the adapter cannot resolve it — a root span that failed to open, -sampling that dropped it. An allow-list of "surfaces we check" silently exempts those -real turns, and every future surface until someone remembers to edit the set. A new -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 -detector that cries wolf on the healthy case is how the real rows get ignored. Use -`faults()` for anything a human reads first. +
## Status -Alpha. The taxonomy is the product: **detector codes are the public interface**, and -renaming one is a breaking change. Thresholds, default vocabularies and added detectors -are not. - -Versioning follows SemVer, with the 0.x convention the spec leaves undefined made -explicit: while the major is 0, a **minor** bump may break the API and a **patch** may -not. - -Release notes live on the -[Releases page](https://github.com/Base-Homes/postflight/releases), generated from the -merged PRs for each tag — one place, tied to the artifact it describes, rather than a -file that has to be remembered separately. - -**Adapters: Langfuse and OpenTelemetry / OpenInference.** The detectors never see a -vendor — they read `postflight.model.Turn`, so an adapter is just a function from your -trace format to an ordered list of `Generation` and `ToolCall` steps. - -Portability is now demonstrated rather than asserted: the OTel adapter was written -against real spans from an Anthropic agent instrumented with OpenInference — a producer -that shares nothing with the first one — and the shipped defaults caught a `TOOL_REFUSAL` -in it while correctly declining to flag the model's own negated sentence. `Turn` needed -no change to accept it. +Alpha. Detector codes are the public interface; thresholds, default vocabularies, and +added detectors are not. -It did surface one real modelling bug, which is the point of trying: OpenInference emits -no cache attribute at all, and scoring that absence as `0` made every large-prompt turn a -false `NO_CACHE_HIT`. `Generation.cache_read_tokens` is now `int | None` — **unknown is -not zero** — and an adapter that cannot report cache usage says so. Worth knowing if you -write the third adapter. +SemVer, with the 0.x convention the spec leaves undefined made explicit: while the major +is 0, a minor bump may break the API and a patch may not. Release notes are on the +[Releases page](https://github.com/Base-Homes/postflight/releases). -Issues and PRs welcome; no response SLA. +Issues and PRs welcome. No response SLA. -Apache 2.0 — see [LICENSE](LICENSE). +Apache 2.0. See [LICENSE](LICENSE). diff --git a/SECURITY.md b/SECURITY.md index cae649f..27acab5 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -12,7 +12,7 @@ Report privately via Please do not open a public issue. Expect an acknowledgement within 7 days. This is a small project with no dedicated -security staffing — that window is what can actually be met, not an aspiration. +security staffing, so that window is what can actually be met rather than an aspiration. ## Scope @@ -24,5 +24,5 @@ nothing from the traces it reads. The realistic surface is therefore: or enormous attribute sets). - `LangfuseClient` and the credentials a caller hands it. -Findings can quote trace content — including reply text — in `Finding.detail`. If your +Findings can quote trace content, including reply text, in `Finding.detail`. If your traces carry personal data, treat postflight's output as carrying it too. diff --git a/docs/configuring.md b/docs/configuring.md new file mode 100644 index 0000000..44c1741 --- /dev/null +++ b/docs/configuring.md @@ -0,0 +1,186 @@ +# Configuring postflight + +The detectors are mechanism. The vocabulary, the thresholds and the surface names +are yours, and they live here. Read this when you adopt, not before. + +
+ +## Writing an adapter + +The `Turn` contract is in the [README](../README.md#the-turn-contract). Three notes on +filling it in, each of which cost something to learn: + +- **`ToolCall.is_error` is the TRANSPORT flag only**: an exception, an `isError`, an + `ERROR` span status. A tool that ran fine and declined in its own body is *not* an + error. Leave `is_error=False`, put the body in `result`, and let `success_flags` + classify it. Conflating the two hides the more dangerous failure. +- **Unknown is not zero.** If your producer does not report cache usage, leave + `cache_read_tokens=None`. Passing `0` asserts a cache miss and `NO_CACHE_HIT` will + believe you. +- **`kind` falls back to `"unknown"`, never to a guess.** See the deny-list rule below + for why a plausible-looking default is the dangerous option. + +Adapters must not add a dependency. Parse exported JSON rather than importing a vendor +SDK: the producer needs it, the reader does not. + +
+ +## Tuning it to your agent + +The detectors are mechanism; the vocabulary is yours. Everything below is a `Config` +field, and the shipped defaults are a starting point, not a claim of completeness. +they are what a tool-calling agent looks like before you have watched *yours* fail. + +```python +Config( + slow_turn_s=30.0, + tool_storm=6, + # Tools that decline in-body, by the key they set to False. + success_flags=("ok", "updated", "sent", "created"), + # Surfaces that owe a human a reply. Setting this is what promotes EMPTY_REPLY from + # INFO to a fault. Leave it empty and postflight cannot tell a silent channel from + # a batch job that returns a document, so it counts them instead of blaming them. + conversational_kinds=frozenset({"chat.turn", "inbound.turn", "group.turn"}), + # Surfaces that narrate rather than speak. A digest summarising someone's history + # uses the same words a claim does, with no user and no write in the turn. + narrating_kinds=frozenset({"digest.turn"}), + # 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"}), +) +``` + +### What `TOOL_REFUSAL` can and cannot see + +This detector does **not** assume your tools return `{"updated": false}`. It assumes you +tell it how your tools say no. Out of the box it recognises four shapes: + +| shape | verdict | +|---|---| +| the call raised (`is_error` set by the adapter) | `TOOL_ERROR` | +| the framework's error string (`Error executing tool …`) | `TOOL_ERROR` | +| a truthy `error` key in the result | `TOOL_ERROR` | +| a key from `success_flags` set to `false` | `TOOL_REFUSAL` | + +Anything else reads as success. If your tools signal failure some other way, whether +a `status` field, an enum or an HTTP-ish code, then **`TOOL_REFUSAL` will never fire and your +report will look clean**. Add your convention: + +```python +Config( + refusal_predicates=( + lambda r: isinstance(r, dict) and r.get("status") in {"failed", "declined"}, + ) +) +``` + +There is no default for that, deliberately. `{"status": "failed"}` returned by a +`get_job_status` tool describes the *job*, not the call, and guessing would make every +healthy status read into a refusal, which is precisely the cry-wolf failure this package +exists to avoid. You know which of your tools report on themselves; postflight doesn't. + +Two things it will never infer, by design: an **empty result set** (a search that found +nothing is not a decline) and **prose** (`"No matching orders found."` is +indistinguishable from success without reading it). If a tool of yours only fails in +prose, the durable fix is in the tool, not here. + +#### Reading a refusal + +A `TOOL_REFUSAL` is not automatically a bug in the tool. Most in-body declines are +expected outcomes rather than defects, and the reason is often information the agent +needs in order to do something sensible next. + +What is worth checking is whether the decline was *handled*. A refusal on the same turn +as an `UNVERIFIED_CLAIM` is the pairing that matters: the tool said no, and the reply +said yes, which means a user was told something untrue. A refusal that the reply reports +honestly is the system working, and the fixture in this repository is exactly that case. + +The one shape worth fixing at the source is a tool swallowing a genuine failure into a +result body, a 500 returned as `{"ok": false}`. That is an error wearing a decline's +clothes, and it belongs in `TOOL_ERROR` where your existing alerting can see it. + +`refusal_exemptions` is the other direction: shapes that look like refusals and are +not. The shipped one is `{"sent": false, "queued": true}`: a send handed off to a relay. +Exemptions outrank `refusal_predicates`, so widening your detection cannot silently +re-flag a path you already excused. + +### What the other detectors depend on + +Same class of problem, and the reason `coverage()` exists: a detector whose input is +missing does not error, it just never fires, and an empty column reads exactly like a +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 | | +| `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) | | + +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 +directions. + +So check rather than assume: + +```python +from postflight import coverage + +for row in coverage(turns, cfg): + print(row) # e.g. "NO_CACHE_HIT: INERT - no generation reports cache usage" +``` + +It reports structural inertness only, meaning an input absent from every turn. It will not tell +you a detector is broken because its count is zero, because a tool that never errored is +a healthy agent, and conflating those would just move the problem up a level. + +### Claim rules + +The one worth real attention is `claim_rules`, which drives `UNVERIFIED_CLAIM`. A rule +pairs a regex against the tools that would make the claim true: + +```python +from postflight import ClaimRule, Config +import re + +Config( + claim_rules=( + ClaimRule( + name="ticket_filed", + pattern=re.compile( + r"\b(?:filed|opened|created)\b[^.\n]{0,40}\bticket\b", re.IGNORECASE + ), + satisfied_by=frozenset({"create_ticket", "escalate_to_support"}), + ), + ) +) +``` + +Matching reads the clause the match sits in, and skips it on two conditions: + +- **Negation.** "The follow-up was **not** sent" is the agent being honest about not + acting, and scoring that as a lie punishes exactly the behaviour you want. +- **A third-party subject.** "**The owner** emailed you" is the agent relaying what + someone else did. The verb-object pair is identical to a real claim; only the subject + differs, and relaying is ordinary in any reply that summarises a thread. + +Both are `Config` regexes (`negation`, `third_party_subject`) if your replies read +differently. + +
+ +## Two design rules worth knowing before you extend it + +**Kind-keyed exemptions are deny-lists, never allow-lists.** `Turn.kind` falls back to +`"unknown"` whenever the adapter cannot resolve it: a root span that failed to open, +sampling that dropped it. An allow-list of "surfaces we check" silently exempts those +real turns, and every future surface until someone remembers to edit the set. A new +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 +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/docs/img/README.md b/docs/img/README.md new file mode 100644 index 0000000..834f0ec --- /dev/null +++ b/docs/img/README.md @@ -0,0 +1,7 @@ +# Diagram sources + +`turn-scope-light.svg` and `turn-scope-dark.svg` are generated, with their text +converted to outlines so the typeface does not depend on the viewer having it +installed. They are path data and cannot be usefully hand-edited. + +The generator is not checked in. Open an issue if you need a change to the diagram. diff --git a/docs/img/turn-scope-dark.svg b/docs/img/turn-scope-dark.svg new file mode 100644 index 0000000..5044e53 --- /dev/null +++ b/docs/img/turn-scope-dark.svg @@ -0,0 +1,30 @@ + +One turn of four steps. Each step passes when scored on its own; the failure is the relationship between the tool that declined and the reply that claims it succeeded. + + +UNVERIFIED_CLAIM + + + + + +search_orders + + +{"sent": false} + + + + + + + + + + + + + + + + diff --git a/docs/img/turn-scope-light.svg b/docs/img/turn-scope-light.svg new file mode 100644 index 0000000..e0657d4 --- /dev/null +++ b/docs/img/turn-scope-light.svg @@ -0,0 +1,30 @@ + +One turn of four steps. Each step passes when scored on its own; the failure is the relationship between the tool that declined and the reply that claims it succeeded. + + +UNVERIFIED_CLAIM + + + + + +search_orders + + +{"sent": false} + + + + + + + + + + + + + + + + diff --git a/postflight/__main__.py b/postflight/__main__.py new file mode 100644 index 0000000..aff80ae --- /dev/null +++ b/postflight/__main__.py @@ -0,0 +1,174 @@ +"""Command line entry point, so a cron job or CI step does not need a wrapper script. + + python -m postflight --otel spans.jsonl + python -m postflight --langfuse --hours 24 + python -m postflight --otel spans.jsonl --coverage + +Exit status is the useful part in CI: 0 when nothing faulted, 1 when something did. +INFO findings never affect it, because a surface that is silent by design should not +fail anyone's build. +""" + +from __future__ import annotations + +import argparse +import json +import os +import sys +from collections import Counter + +from .config import Config +from .coverage import coverage +from .detectors import faults, run +from .model import Turn + + +def _kinds(value: str | None) -> frozenset[str]: + return frozenset(k.strip() for k in (value or "").split(",") if k.strip()) + + +def _build_config(args: argparse.Namespace) -> Config: + return Config( + slow_turn_s=args.slow_turn_s, + tool_storm=args.tool_storm, + repeated_tool=args.repeated_tool, + conversational_kinds=_kinds(args.conversational_kinds), + quiet_kinds=_kinds(args.quiet_kinds), + narrating_kinds=_kinds(args.narrating_kinds), + ) + + +def _load(args: argparse.Namespace) -> list[Turn]: + if args.otel: + from .adapters.otel import turns_from_jsonl + + return turns_from_jsonl(args.otel) + + from .adapters.langfuse import LangfuseAdapter, LangfuseClient + + public = os.environ.get("LANGFUSE_PUBLIC_KEY", "") + secret = os.environ.get("LANGFUSE_SECRET_KEY", "") + if not public or not secret: + raise SystemExit( + "LANGFUSE_PUBLIC_KEY and LANGFUSE_SECRET_KEY must be set for --langfuse" + ) + host = os.environ.get("LANGFUSE_HOST", "https://us.cloud.langfuse.com") + client = LangfuseClient(host, public, secret) + observations = client.observations(hours=args.hours, environment=args.environment) + return LangfuseAdapter().turns(observations) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="postflight", + description="Turn-level failure detection for tool-calling agents.", + ) + source = parser.add_mutually_exclusive_group(required=True) + source.add_argument( + "--otel", + metavar="FILE", + help="exported OpenTelemetry spans, one JSON object per line", + ) + source.add_argument( + "--langfuse", + action="store_true", + help="pull from Langfuse (credentials from the environment)", + ) + + parser.add_argument( + "--hours", type=int, default=24, help="window for --langfuse (default: 24)" + ) + parser.add_argument( + "--environment", default=None, help="Langfuse environment filter" + ) + + parser.add_argument( + "--coverage", + action="store_true", + help="report which detectors can fire on this data, then exit", + ) + parser.add_argument( + "--json", dest="as_json", action="store_true", help="machine-readable output" + ) + parser.add_argument( + "--quiet", action="store_true", help="summary only, no per-turn lines" + ) + + parser.add_argument("--slow-turn-s", type=float, default=Config().slow_turn_s) + parser.add_argument("--tool-storm", type=int, default=Config().tool_storm) + parser.add_argument("--repeated-tool", type=int, default=Config().repeated_tool) + parser.add_argument( + "--conversational-kinds", help="comma separated surfaces that owe a reply" + ) + parser.add_argument( + "--quiet-kinds", help="comma separated surfaces that are silent by design" + ) + parser.add_argument( + "--narrating-kinds", + help="comma separated surfaces that narrate rather than speak", + ) + + args = parser.parse_args(argv) + cfg = _build_config(args) + turns = _load(args) + + if args.coverage: + rows = coverage(turns, cfg) + if args.as_json: + print(json.dumps([r.__dict__ for r in rows], indent=2)) + else: + for row in rows: + print(row) + return 0 + + results = {t.id: run(t, cfg) for t in turns} + all_findings = [f for fs in results.values() for f in fs] + flagged = {tid: fs for tid, fs in results.items() if faults(fs)} + + if args.as_json: + print( + json.dumps( + { + "turns": len(turns), + "flagged": len(flagged), + "findings": [ + { + "turn_id": f.turn_id, + "code": f.code, + "severity": f.severity.value, + "message": f.message, + "detail": f.detail, + } + for f in all_findings + ], + }, + indent=2, + default=str, + ) + ) + return 1 if flagged else 0 + + if not args.quiet: + for turn_id, found in results.items(): + for finding in found: + mark = " " if finding.severity.value == "fault" else "i" + print(f"{mark} {turn_id[:12]:14} {finding.code:18} {finding.message}") + + counts = Counter(f.code for f in all_findings) + print(f"\n{len(turns)} turns, {len(flagged)} flagged") + for code, count in counts.most_common(): + print(f" {code:18} {count}") + + # An inert detector and a clean agent look identical in the output above, so say + # which ones could not have fired rather than leaving a zero to be misread. + inert = [r for r in coverage(turns, cfg) if not r.live or r.misleading] + if inert: + print("\nNot all detectors are live on this data:") + for row in inert: + print(f" {row}") + + return 1 if flagged else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/postflight/coverage.py b/postflight/coverage.py index 0ea831d..df29149 100644 --- a/postflight/coverage.py +++ b/postflight/coverage.py @@ -38,7 +38,7 @@ class Coverage: def __str__(self) -> str: state = "MISLEADING" if self.misleading else ("live" if self.live else "INERT") - return f"{self.code}: {state} — {self.reason}" + return f"{self.code}: {state} - {self.reason}" def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage]: @@ -77,7 +77,7 @@ def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage] "UNVERIFIED_CLAIM", True, "no tool name in this data satisfies any claim rule (saw " - f"{len(seen_tools)} distinct tools) — a genuine action will read as an " + f"{len(seen_tools)} distinct tools). A genuine action will read as an " "unbacked claim. Check satisfied_by / satisfied_by_prefix against your " "tool names", misleading=True, @@ -161,7 +161,7 @@ def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage] bool(cfg.quiet_kinds), "quiet_kinds configured" if cfg.quiet_kinds - else "no quiet_kinds configured — nothing is silent by design", + else "no quiet_kinds configured, so nothing is silent by design", ) ) @@ -190,8 +190,7 @@ def coverage(turns: Iterable[Turn], cfg: Config | None = None) -> list[Coverage] Coverage( "NO_CACHE_HIT", False, - "no generation reports cache usage — unknown is not " - "treated as zero, so this cannot fire", + "no generation reports cache usage, and unknown is not treated as zero", ) ) else: diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..68f4e8a --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,61 @@ +"""The CLI is how this gets wired into a cron job or a CI step, so its exit status is +part of the contract: 0 when nothing faulted, 1 when something did.""" + +import pathlib + +import pytest + +from postflight.__main__ import main + +FIXTURE = str( + pathlib.Path(__file__).parent / "fixtures" / "openinference_support_turn.jsonl" +) + + +def test_exits_1_when_a_fault_is_found(capsys): + assert main(["--otel", FIXTURE]) == 1 + assert "TOOL_REFUSAL" in capsys.readouterr().out + + +def test_exits_0_when_nothing_faults(tmp_path, capsys): + empty = tmp_path / "none.jsonl" + empty.write_text( + '{"name": "a.turn", "context": {"trace_id": "t"}, "parent_id": null,' + ' "attributes": {"openinference.span.kind": "AGENT"}}\n' + ) + assert main(["--otel", str(empty)]) == 0 + + +def test_inert_detectors_are_named_in_the_output(capsys): + """A zero next to a detector that could not have fired is the failure this whole + package is trying not to have, so the CLI says so unprompted.""" + main(["--otel", FIXTURE]) + assert "Not all detectors are live" in capsys.readouterr().out + + +def test_coverage_mode_exits_0_and_reports_every_detector(capsys): + assert main(["--otel", FIXTURE, "--coverage"]) == 0 + out = capsys.readouterr().out + for code in ("UNVERIFIED_CLAIM", "TOOL_REFUSAL", "SLOW_TURN", "NO_CACHE_HIT"): + assert code in out + + +def test_json_output_is_parseable_and_keeps_the_exit_code(capsys): + import json + + assert main(["--otel", FIXTURE, "--json"]) == 1 + payload = json.loads(capsys.readouterr().out) + assert payload["turns"] == 1 and payload["flagged"] == 1 + assert payload["findings"][0]["code"] == "TOOL_REFUSAL" + + +def test_langfuse_without_credentials_fails_loudly(monkeypatch): + monkeypatch.delenv("LANGFUSE_PUBLIC_KEY", raising=False) + monkeypatch.delenv("LANGFUSE_SECRET_KEY", raising=False) + with pytest.raises(SystemExit, match="LANGFUSE_PUBLIC_KEY"): + main(["--langfuse"]) + + +def test_a_source_is_required(): + with pytest.raises(SystemExit): + main([]) diff --git a/tests/test_detectors.py b/tests/test_detectors.py index 66aa64f..33eef35 100644 --- a/tests/test_detectors.py +++ b/tests/test_detectors.py @@ -363,3 +363,22 @@ def test_an_exemption_outranks_a_refusal_predicate(): cfg = Config(refusal_predicates=(lambda r: isinstance(r, dict) and "sent" in r,)) queued = tool("send", result={"sent": False, "queued": True}) assert tool_outcome(queued, cfg) is Outcome.OK + + +def test_the_refusal_plus_claim_pairing_the_docs_point_at(): + """docs/configuring.md tells a reader that a TOOL_REFUSAL on the same turn as an + UNVERIFIED_CLAIM is the combination worth acting on: the tool said no and the reply + said yes. That is only useful advice if both actually surface together.""" + both = turn( + tool("send_email", result={"sent": False, "reason": "channel down"}), + gen("I've sent them a message."), + ) + assert {"TOOL_REFUSAL", "UNVERIFIED_CLAIM"} <= codes(run(both)) + + # And the honest version of the same turn: tool declined, reply said so. One + # finding, not two, because nobody was misled. + honest = turn( + tool("send_email", result={"sent": False, "reason": "channel down"}), + gen("I wasn't able to send them a message."), + ) + assert codes(run(honest)) == {"TOOL_REFUSAL"} diff --git a/tests/test_otel_adapter.py b/tests/test_otel_adapter.py index e5955eb..7581a6a 100644 --- a/tests/test_otel_adapter.py +++ b/tests/test_otel_adapter.py @@ -1,11 +1,13 @@ """The portability test: a trace this package did not grow up on. -The fixture is REAL output — an Anthropic tool-calling agent instrumented with -OpenInference and exported through the OpenTelemetry SDK, captured verbatim and then -stripped of the prompt text. Nothing about its shape was chosen here: flattened indexed -attributes, ISO timestamps, span kinds, `output.value` as an opaque string. A detector -that only ever met Langfuse's observation model has no business passing this by luck, so -these assert the DETECTIONS, not just that parsing did not raise. +The fixture is a real capture of an invented scenario: a throwaway support-desk agent +written for this test, run for real through the Anthropic SDK with OpenInference +instrumentation and exported by the OpenTelemetry SDK. The SCENARIO is synthetic, so it +carries no third-party data; the SHAPE is not, and the shape is the point. Flattened +indexed attributes, ISO timestamps, span kinds, `output.value` as an opaque string: +none of it was chosen here. A detector that only ever met Langfuse's observation model +has no business passing this by luck, so these assert the DETECTIONS rather than merely +that parsing did not raise. The agent was given two deliberately unhelpful tools. `send_notification` declines in its own body (`{"sent": false}`) with no error status, which is the shape that satisfies every