Wire nine observability fixtures and fix the dead metrics invariants - #274
Merged
Conversation
Activates 145, 146, 147, 149, 150 and 151, cutting the fixtures that run nowhere from fourteen to eight. Adds proposal 0107's raises sub-directive to the retrieval mocks. The category must still come from the declared status, so rather than restate the status-to-category table in the harness, the real classifier runs on a probe response and the harness raises a dynamically-named subclass of whatever it returns: the category is inherited while the class name and message supply the literal values 150 and 151 assert. Adds the five token-budget invariants fixture 146 declares, covering a null prompt_tokens leaving the input bound unevaluable while a sound total bound still evaluates. The exceeded span signal must be absent rather than false, since false would claim the bound was evaluated. Fixtures 144 and 148 stay deferred with reasons naming what they need: both want a driver rather than a dispatch entry.
The metrics driver read expected.metrics but never expected.invariants, and unlike the token-budget driver it had no guard for an invariant it did not handle. Every invariant on every metrics fixture was silently dropped. That mattered most for 145 and 147, whose invariants are the whole assertion: both state in their own comments that the metrics list shape can express presence only, so the negative claims live nowhere else. Both passed while an input token.usage observation was recorded for a null prompt_tokens, which is exactly what they forbid. Adding the guard surfaced nine more dead invariants on four fixtures that predate this cluster. All sixteen are implemented, and the guard now fails loudly on the next unhandled name.
Invariant blocks in this corpus are documentary: they restate in prose what a concrete directive already pins, and runners deliberately do not implement most of them. That convention is only safe while something concrete is present to do the pinning. A case whose expected block carries nothing but invariants has no such backing, so it asserts nothing while reading like coverage. The metrics driver reached that state by another route and two fixtures passed while checking nothing, which is what this guard exists to prevent recurring. It matches no fixture today, so it ships with a test that injects a synthetic case and confirms it fires on both runner paths, plus a non-vacuity check that the corpus it polices is actually reachable.
154 asserts the failure-path counterpart of the embedding metrics fixtures: the duration observation survives a failed embed and carries the error category, while no token usage is recorded because no response arrived. Its two invariants are implemented and mutation-verified. Binding the provider needed a fallback. The harness reads the model off the first mock response body, and a 503 has no body, so a failure-path fixture bound to the harness default and every dimension match failed on a model name the fixture never mentions. It now falls back to the model the expected metric dimensions report. 119, 152 and 153 stay deferred, but with reasons diagnosed rather than guessed: each reaches its sibling's driver and fails on a concrete gap. 119 drives a branch that raises under retry, which that driver has no path for; 152 and 153 need a subgraphs block the orphan-fallback driver does not build.
155 and 156 are near-clones of 130, which already runs in the sibling Langfuse runner, so they belong there rather than needing a driver in the generic one. Both now execute and the generic runner records them as relocated rather than unaccounted. Verified in both directions, since a one-way mutation proves nothing for a pair asserting opposite outcomes: hardcoding the exceeded flag false fails 155, hardcoding it true fails 156.
There was a problem hiding this comment.
Pull request overview
This PR re-enables a batch of previously deferred observability conformance fixtures and tightens the conformance harness so fixture “invariants” are no longer silently ignored or allowed to become vacuous.
Changes:
- Activates additional OTel/Langfuse observability fixtures and updates runner dispatch/deferral bookkeeping accordingly.
- Fixes metrics/token-budget harness logic to actually evaluate expected
invariants, with explicit “unknown invariant” guards. - Adds a guard that rejects cases whose
expectedblock contains onlyinvariants, plus tests to ensure the guard is exercised on the real fixture corpus.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/conformance/test_observability.py | Wires additional observability fixtures and extends harness assertions (metrics invariants, token-budget invariants, retrieval mock raises, model binding fallback). |
| tests/conformance/test_observability_langfuse.py | Adds the invariants-only guard to Langfuse fixture execution and wires additional Langfuse token-budget fixtures. |
| tests/conformance/test_capability_gate_wiring.py | Adds tests covering the invariants-only guard and a corpus non-vacuity check. |
| tests/conformance/harness/capabilities.py | Implements assert_case_asserts_something to prevent invariants-only expected blocks from passing without concrete assertions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The adversarial review found this branch reproducing the defect it was opened to fix. 147 and 149 declared span_tree blocks their drivers never read: corrupting every span name in either left both green. With 144 deferred, that left the whole 0101 span surface with no live coverage while the branch reported nine fixtures wired. 144's deferral reason was also false. It claimed no generic LLM span-tree driver existed; _run_llm_payload_fixture is exactly that, and 144 passes through it unmodified. A reader trusting that reason would have built a driver that already exists. 144 and 149 now run through that driver, and 147 through the token-budget one, which reads observers and span_tree alongside metrics. The two invariant registries validate against their union so a fixture may declare names from both families while each guard implements only its own subset. The absence predicates gain a positive anchor. Every span claim is of the form "X is absent", which an empty span set satisfies trivially: renaming the LLM span made 147 green. The structural fix is a guard asserting each fixture's driver reads every expected key it declares, with the fixture-to-driver map parsed from the dispatch chain rather than hand-maintained. It caught a real error in this very commit, where a move landed in the wrong list.
Two deferral reasons were wrong in the same way as 144's. 148's blamed missing mock_llm plumbing in the Langfuse runner, which drives three such fixtures already; the real gap is that its usage comparator subset-matches expected keys, so an omitted input key cannot be expressed at all. 119's blamed the callable-branch driver, but the adapter never translates a node's YAML middleware block, so the retry is never installed and the suggested fix would not have helped. The invariants-only guard skipped every single-case fixture, because those carry expected at the top level and it iterated `cases or []`. Its non-vacuity test was itself vacuous: it counted fixtures on disk and passed identically with both call sites deleted. It now grafts a synthetic case onto a real fixture and pushes it through each runner's entry point, and fails when either call site is removed. The duration and token-usage count invariants asserted on aggregated histogram data points rather than observation counts, so the double-record regression they are named for could not fail them. 156's no_warning_level_under_budget is implemented because it is unbacked: 155 pins its level concretely so its invariants are documentary, but 156's expected block declares no level. A blanket guard here would be wrong, since it would demand implementation of the documentary majority.
A green conformance run is the null result: a fixture wired into a driver that ignores half its expected block passes exactly like a fully asserted one, so passing discriminates nothing. States the inverted acceptance criterion, that a fixture is wired when the behaviour it covers has been broken and seen to go red, and the traps around it: verify the mutation landed, treat a fixture that needs no harness work as the highest risk rather than the easiest win, diagnose by running instead of reading, and prefer a structural guard over remembering any of it. Written down because the rule already existed as guidance and was still missed repeatedly across one session.
The delta review found the previous fix swapped which half of 149 was dropped rather than fixing it. It moved from a driver reading only observers to one reading only span_tree, and the new guard waved it through because the hand-written map claimed the payload driver reads observers. It does not; it reads span_tree alone. The map now states what that driver actually reads. 149 goes back to deferred, because no driver serves it: one reads observers, one reads span_tree, and the third reads both but builds the graph from a mock shape that does not reproduce the fixture's response model or id, so its span match fails on attributes rather than behaviour. Its event half is the fixture's stated discriminator, so wiring it under a span-only driver drops the assertion it exists for. The guard also iterated `cases or []`, the idiom this branch fixed forty lines below it, making it a no-op for single-case fixtures. Its fail-open bypass is now documented as fail-open rather than left to read as blanket protection: most drivers are unregistered, so the guard is dark for them.
The recognized-invariant check validated against the union of the metrics and token-budget families from inside each guard, but the metrics runner calls only one of them. A token-budget name declared on a metrics fixture was therefore recognized while nothing checked it. Each runner now supplies the union over the guards it actually calls, so the metrics path rejects what it cannot evaluate. The positive span anchor was gated on the span set being non-empty, which short-circuited it in exactly the zero-span case it exists to catch, and keyed on a name prefix that missed both exceeded_span_signal_* claims. It now keys on an enumerated set and fires regardless of how many spans were recorded.
The note said the _DRIVER_EXPECTED_KEYS check makes a fixture wired into a driver that drops a directive impossible rather than merely catchable. That overstates it: the check is fail-open and returns silently for any driver absent from the map, which is 30 of the 36 drivers the dispatch chain reaches. An agent reading the old wording would treat a green run on an unregistered driver as evidence.
Three of the harness's own allowlists had no check that they still describe what its code does, and a conformance run cannot reveal the difference: a dead assertion and a live one produce the same green. _PER_TRACE_INVARIANTS omitted no_warning_level_under_budget, which _assert_trace does check, so the multi-trace path would discard that claim. Both sets are now derived from the function bodies and compared, so the next omission fails instead of going quiet. An unregistered driver is invisible to the mis-routing guard, so a new one goes dark on arrival. Every dispatched driver must now be registered or named in an unguarded list with its reason. Four shared drivers covering 17 fixtures are recorded as a real remaining gap rather than left implicit. The span anchor itself was undetectable by the corpus: both fixtures declaring a span-absence claim also declare span_tree, whose root check fails first, so deleting the anchor left every fixture green. Driving both guards directly with an empty span set closes that.
The test named two fixtures and claimed one exercised the multi-case shape, but both put expected at the top level with no cases list, so the branch reading spec["cases"][0] never ran. It now names a multi-case fixture per runner and asserts each row still has the shape it was chosen for. Both runners skip a fixture they no longer drive, and Skipped derives from BaseException, so it passed straight through pytest.raises and turned the whole test into a silent skip. A skip now fails, naming the fixture, since a test that never ran is the failure mode this file exists to prevent.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
tests/conformance/test_observability.py:436
- The PR description says fixture 149 is activated and fixture 144 remains deferred, but this diff does the opposite: 149 is added to
_DEFERRED_FIXTURES, while 144 is dispatched through_run_llm_payload_fixture. Update the description and verification list to match the implemented fixture set, or wire the originally claimed fixture.
"149-malformed-wire-counter-nulled-through-mapping-to-event-and-span": (
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Activates nine spec conformance fixtures that were switched off, cutting the fixtures running nowhere from fourteen to five. Along the way it fixes a class of test assertions that were passing without checking anything, including two fixtures wired earlier in this same branch.
The fixtures
150 / 151 need proposal 0107's
raises: {error_type, message}sub-directive on amock_embedding/mock_rerankentry, so a failure'serror_typeanderror_messagecan be asserted literally instead of by format. The category must still come from the declared HTTP status, so rather than restate the status-to-category table in the harness where it would drift from src, the real classifier runs on a probe response of that status and the harness raises a dynamically-named subclass of whatever it returns.error_categoryreadsexc.categoryand is inherited;error_typeanderror_messageread the class name and message, which the rename supplies.145 / 146 / 147 / 149 are the proposal 0101 null-counter arms. The mock already supported a null or malformed
prompt_tokens, so these were mostly dispatch, except 146 which needed five token-budget invariants implemented.154 asserts the failure-path counterpart of the embedding metrics fixtures. It also needed a real fix: the harness binds the provider to the model named in the first mock response body, and a 503 has no body, so a failure-path fixture bound to the harness default and every metric dimension mismatched on a model the fixture never mentions. It now falls back to the model the expected dimensions report.
155 / 156 are near-clones of 130 and belong in the sibling Langfuse runner rather than needing a driver in the generic one.
The bug this uncovered
_run_metrics_casereadexpected.metricsbut neverexpected.invariants, and unlike the token-budget driver it had no guard for an invariant it did not handle. Every invariant on every metrics fixture was silently dropped.That mattered because 145 and 147 both state in their own comments that the
metrics:list shape can express presence only, so for those two the invariants were the entire assertion. Both passed while an inputtoken.usageobservation was recorded for a nullprompt_tokens, which is exactly what they forbid. I had already committed them as done.Adding the guard surfaced nine more dead invariants on four fixtures that predate this work: 088, 090, 091 and 125. All sixteen are now implemented.
The guard
A separate, forward-looking check: a case whose
expectedblock carries nothing butinvariantshas no concrete directive doing the pinning, so it asserts nothing. Invariants in this corpus are documentary by design, which is only safe while something concrete is present.It matches no fixture today, so it ships with a test that injects a synthetic case and confirms it fires on both runner paths, plus a non-vacuity check that the corpus it polices is actually reachable.
Worth stating what this guard is not: a blanket "every driver must implement every invariant" rule would fail 92 fixtures that are working as intended, because most invariants deliberately restate what
span_tree/langfuse_trace/event_countalready assert.Verification
2124 passed, 500 skipped. Ruff, pyright and the conformance-manifest check clean. Conformance and unit suites re-run together for order contamination.
Every fixture is mutation-verified rather than trusted because it passed:
token.usagefor a nullprompt_tokensfails 145/147error.typefrom the duration observation fails 090/125/154That last pair needed both directions: 155 asserts the flag is true when over budget and 156 that it is false when under, so a one-way mutation can only kill one of them.
Each mutation was confirmed to have landed before its result was trusted. Two did not apply on the first attempt, once because the target text appears twice and the script asserted it appeared once; those runs were discarded rather than read as survivors.
Still deferred
Five fixtures remain off, each with a diagnosed cause rather than a guess, established by running it against the closest existing driver:
These ride a follow-up PR.