Repository navigation
Classify the case-level directive mappings - #326
Merged
Merged
Conversation
A sub-key inside a case-level directive mapping was accepted and ignored, so a fixture declaring an assertion there passed with the assertion never made. The nested vocabulary check covered expected, resume, calls[] and node specs. This extends it to every other directive at case level whose value is a mapping or a list of mappings. Each of the 72 such directives is now placed in one group: walked already (expected, resume, nodes, the subgraph bodies, cases and the call lists), open (the keys are data, or the reader checks them itself), out of scope (invariants), or closed. The 52 closed ones become positions of their own, with a vocabulary from their model fields or an explicit key set, so an unknown key fails and a key a running fixture declares must be read. A new mapping fails until it is placed. The read check found real gaps, now wired: - pipeline-utilities expected_error.flaky_call_count (009, 016), through a call counter on the adapter's flaky node, and fan_out_category (023); - observability direct_call.operation (046) and typed_observers[].retains_arrival_order (056); - retrieval-provider typed_observers[].kind, and expected_error.raised_from, which must name the node the harness drives. raised_from was also unread in 23 running pipeline-utilities fixtures, under expected_error and first_run_expected_error. The check missed it because the observability runner loads fixture 031, which made it an owner of every pipeline-utilities fixture. Both pipeline-utilities runners now check the node the error names, and a runner that loads one named fixture from another capability owns that fixture alone. 084's invocations[].session_id joins the unread ledger, since its case waits for the sessions capability. POSITION_BLIND_READS grows from 15 to 107 entries: keys read only through a string that names more than one mapping. Tying each read to its own mapping is a separate change. The extras redundancy check could not fail: it subtracted the extras from the vocabulary before comparing against them. It now compares against the model fields alone.
Both read checks counted a key read anywhere in harness/, adapter.py or middleware_seam.py as read for every capability. The path match was a substring, so test_conformance_adapter.py counted as shared too. A shared read now counts for a capability only when one of its owning runners reaches the function holding it, resolved by name through each runner's imports. A new test pins the import shapes that resolution has to get right. The old rule hid two gaps: - The retrieval-provider runner drives the entry node alone and never builds a graph, so nothing checked the edges its 56 fixtures declare. It now fails unless the graph is the entry node with one edge to END, which every current case is. - No llm-provider runner reads expected.invariants, which 16 running fixtures declare. They are ledgered as known-unread until a pass reads the predicates. A new test requires every block under that entry to carry a directive the runner does read, since predicates alone assert nothing.
Comments in the vocabulary checks and the checkpoint runner cited counts that had drifted, described earlier versions of the code, or kept rationale in docstrings. They now describe the code as it is, with the rationale in comments beside it. No code changes. Some were wrong, not just dated. The after_node test's note said no fixture uses after_node; fixture 070 does, but its target is the entry node, so it passes a double that aborts after any node, and the note now says the test is the only check that catches one. Others said three llm-provider fixtures are deferred to tests/unit (nine are), that test_pipeline_utilities has no deferral registry (it has one beside its numeric cutoff), and that the subgraph walk covers running fixtures (it covers every fixture in a run directory).
There was a problem hiding this comment.
🟡 Changes recommended
Assigned full fixture paths still grant capability-wide ownership, allowing unrelated reads to mask missing assertions.
1 open issue
What changed in this PR
This PR strengthens the conformance harness so fixture directives cannot silently pass without being recognized or read by an owning runner.
Changes:
- Classifies case-level mappings and expands nested vocabulary checks and unread-key tracking.
- Scopes shared-helper reads to reachable definitions and narrows cross-capability fixture ownership.
- Adds missing runner assertions and corrects outdated test comments.
| File | Description |
|---|---|
tests/conformance/test_retrieval_provider.py |
Validates observer kinds, error origins, and supported graph shape. |
tests/conformance/test_pipeline_utilities.py |
Checks error origins, fan-out categories, and flaky-node call counts. |
tests/conformance/test_observability.py |
Validates direct-call operations and observer arrival-order settings. |
tests/conformance/test_nested_vocabulary.py |
Checks classified mappings and guards blocks with unread invariants. |
tests/conformance/test_checkpoint.py |
Checks first-run error origins and updates explanatory comments. |
tests/conformance/test_case_vocabulary.py |
Refines fixture ownership and shared-helper read attribution. |
tests/conformance/harness/vocabulary.py |
Registers mapping vocabularies and expands read-gap ledgers. |
tests/conformance/adapter.py |
Adds flaky-node counters and shared error-origin extraction. |
Review effort: Balanced
A runner that bound a complete fixture path to a name, then loaded it through the name, was recorded as owning the whole capability, so its reads could vouch for keys in every other fixture there. No runner has that shape today. The derivation now takes the fixture's id from the bound expression first, and a test pins each path shape it resolves.
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.

The nested vocabulary check walked
expected,resume,calls[]and node specs. A sub-key inside any other case-level directive mapping was accepted and ignored, so a fixture could declare an assertion there and pass without it being made. Three commits.Classify the case-level directive mappings. Each of the 72 case-level directives whose value is a mapping, or a list of mappings, is placed in one group: walked already, open (the keys are data, or the reader checks them itself), out of scope (
invariants), or closed. The 52 closed ones become positions with their own vocabulary, from their model fields or an explicit key set, so an unknown key fails and a key a running fixture declares must be read. A new mapping fails until it is placed.The read check found these gaps, now wired:
expected_error.flaky_call_count(009, 016), through a call counter on the adapter's flaky node, andfan_out_category(023);direct_call.operation(046) andtyped_observers[].retains_arrival_order(056);typed_observers[].kind, andexpected_error.raised_from, which must name the node the harness drives.raised_fromwas also unread in 23 running pipeline-utilities fixtures, underexpected_errorandfirst_run_expected_error. The check missed it because the observability runner loads fixture 031, which made it an owner of every pipeline-utilities fixture. Both pipeline-utilities runners now check the node the error names, and a runner that loads one named fixture from another capability owns that fixture alone.084's
invocations[].session_idjoins the unread ledger, since its case waits for the sessions capability.POSITION_BLIND_READSgrows from 15 to 107 entries: keys read only through a string that names more than one mapping. The extras redundancy check could not fail, because it subtracted the extras from the vocabulary before comparing against them. It now compares against the model fields alone.Count shared helper reads only for their callers. Both read checks counted a key read anywhere in
harness/,adapter.pyormiddleware_seam.pyas read for every capability, and the path match was a substring, sotest_conformance_adapter.pycounted as shared too. A shared read now counts for a capability only when one of its owning runners reaches the function holding it, resolved by name through each runner's imports. The old rule hid two gaps:END, which every current case is.expected.invariants, which 16 running fixtures declare. They are ledgered as known-unread until a pass reads the predicates. A new test requires every block under that entry to carry a directive the runner does read, since predicates alone assert nothing.Drop dated figures and history from test comments. Comments and docstrings only. Some were wrong, not just dated. The
after_nodetest's note said no fixture usesafter_node: fixture 070 does, but it targets the entry node, so only the direct test catches a double that aborts after any node. Others miscounted the llm-provider fixtures deferred totests/unit, or described registries and walks that no longer match the code.Testing:
raised_from(pipeline-utilities 009 and 071, checkpoint 025 and 048) fail, and their three controls pass.fan_out_categoryonFanOutEmptyfails 023. Each passes once its key is removed from the fixture, so the new read is what catches it.raised_from.raised_fromreads removed in memory, the old ownership lets 22 unread declarations pass, and the narrowed ownership reports all 22.from, 56toand 16invariantsdeclarations above, and each of the other read groups traces to a real call. Breaking each part of the rule (annotation skipping, the reach rule, import resolution, class methods) fails the matching assertion in the new test. The retrieval graph check rejects a second node, an extra edge, an edge to another node, and no edges at all.invariantsalone,responsemade unread (a presence-only check passes this one),raisesmade unread, and a walk that skips call-level blocks.Known limit: for the rejection cases of llm-provider 072, 074 and 075, the guard rests on a position-blind
raisesread. The read is real, but the check cannot tie it to its mapping until position-blind reads are attributed in a later change.