Skip to content

test(schema): probe every pattern constraint, and hold the mirroring models.py claims - #270

Merged
imran-siddique merged 3 commits into
agentrust-io:mainfrom
chernistry:test/per-constraint-pattern-discrimination
Sep 2, 2026
Merged

test(schema): probe every pattern constraint, and hold the mirroring models.py claims#270
imran-siddique merged 3 commits into
agentrust-io:mainfrom
chernistry:test/per-constraint-pattern-discrimination

Conversation

@chernistry

Copy link
Copy Markdown
Contributor

Closes the generator half of #247.

The matrix in test_the_schema_and_the_models_agree.py answers "do the two validators disagree about any of these values". It cannot answer "is each constraint discriminated at all" — whether a value reaches the constraint's own boundary, or fails so early that both validators reject for an unrelated reason and agree by accident. Two gaps followed from that, and neither produced a failing test.

Half the pattern constraints were unreachable. BASE carries five of the schema's ten. model.weights_digest, delegation.parent_record_hash, references[].retention, references[].digest and signature are optional, absent from the fixture, and therefore never mutated. A sweep cannot disagree about a field it never sets, and a green run says the same thing either way.

The mirroring was claimed and not held. models.py says its pattern constants are "mirrored verbatim in schema/trace-claim.json and its copy, and held there by tests/test_the_schema_and_the_models_agree.py". They were not held here. Two of the ten are pinned in test_references_block.py; the other eight were maintained by hand, under a comment naming this file.

What it does

_pattern_constraints() reads the ten patterns out of the schema rather than listing them, so a pattern added to the published artifact cannot be added without appearing here. FULL extends BASE so every one of those ten paths exists. _probes(pattern, valid) derives boundary values from the pattern text and from a valid instance of it — structural prefixes of the valid value, literal runs taken out of the pattern, case flip, one character shorter and longer, one character outside the class, and the empty/space/tab values a producer sends for an unset field.

Then, per constraint, one of three outcomes and nothing silent:

  • Mirrored — the model constrains the field with the same pattern string the schema publishes. No string can split them, and saying so is a proof rather than an observation about the values that happened to be tried. This is where all ten sit today.
  • Split — a probe the two disagree about, which goes into DECLARED_DIVERGENCES with its reason, not into a second exemption list.
  • Neither — different patterns and nothing here can tell them apart. That is the state models.py already claims is impossible, and the one that produced schema: the subject pattern accepts identities the reference model refuses #244. It fails unless a human writes down why, in UNMIRRORED_AND_UNSPLIT, which is two-sided the same way: an entry that becomes mirrored has to leave the set.

On the three steers

  1. The floor is discrimination, per pattern, and it is stronger than "at least one splitting value". For a mirrored constraint no splitting value can exist, so demanding one would be demanding a value that cannot be constructed. The test asks instead for the fact that makes that true — the schema's pattern string is one the model carries — and falls through to requiring a declared splitter for any constraint where it is not. The old floors are untouched; both sweeps run.
  2. DECLARED_DIVERGENCES is reused, not duplicated. No new entries: the generator finds no split against the current artifacts. UNMIRRORED_AND_UNSPLIT ships empty and exists so the third case cannot pass in silence. I read the computed _URI_FORMAT_ENFORCED exemption before touching the assertions it feeds — format: uri is not a pattern, so it is outside the ten and the new tests do not reach it.
  3. The tagged-artifact sentence — noted, and separately: tests/test_validate.py:142 already pins the two schema copies against each other, so that part of Four surfaces state the rules and nothing says which wins: schema, reference model, normative spec and docs disagree #247's third bullet is covered. What is not covered is a producer's claim against a published version, which is the precedence text rather than a test.

Verification

1027 passed, 1 skipped; ruff check src tests scripts clean; mypy clean on the file.

Each new test was verified to fail when the thing it guards is broken, rather than only to pass today:

counterfactual result
drop signature from FULL coverage floor fails
_DIGEST_RE changed so it no longer mirrors the schema mirror test fails
a stale UNMIRRORED_AND_UNSPLIT entry staleness test fails
subject restored to the pre-#244 prefix pattern ^(spiffe://|did:) fails, and the generator splits the validators on spiffe://

That last row is the one worth reading. spiffe:// is not written anywhere in this file — it comes out of the pattern's own literal runs — and it is the class the hand-written matrix could not reach, because every value in it fails a prefix test as well, so both validators rejected and agreed.

The preimage bytes for #245 are posted on that issue.

…models.py claims

The matrix in this file answers whether the two validators disagree about a list of
values. It cannot answer whether each constraint is discriminated at all, and two gaps
followed from that, neither of which produced a failing test.

Half the pattern constraints were unreachable. BASE carries five of the schema's ten;
model.weights_digest, delegation.parent_record_hash, references[].retention,
references[].digest and signature are optional, absent, and so never mutated. A sweep
cannot disagree about a field it never sets.

The mirroring was claimed and not held. models.py says its pattern constants are
'mirrored verbatim in schema/trace-claim.json and its copy, and held there by
tests/test_the_schema_and_the_models_agree.py'. Two of the ten were pinned, in
test_references_block.py; the other eight were maintained by hand under a comment
naming this file.

The generator reads the ten patterns out of the schema rather than listing them, probes
each at its own boundary with values derived from the pattern and from a valid instance,
and demands one of three outcomes per constraint: the two artifacts hold the same
pattern string, so no string can split them and that is a proof rather than an
observation; or a splitting value exists and is declared in DECLARED_DIVERGENCES; or it
is neither, which fails. UNMIRRORED_AND_UNSPLIT is the two-sided declaration for the
third case and is empty.

Counterfactuals, each verified to fail: dropping signature from the fixture; changing
_DIGEST_RE so it no longer mirrors the schema; a stale UNMIRRORED_AND_UNSPLIT entry; and
restoring the pre-agentrust-io#244 prefix pattern on subject, which the generator splits on
'spiffe://' -- a value nobody wrote into this file, and the class the matrix could not
reach because every value in it fails a prefix test as well.
@chernistry
chernistry requested a review from a team as a code owner August 31, 2026 23:12
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

🟡 Contributor Check: MEDIUM

Check Result
Profile MEDIUM
Credential LOW
Overall MEDIUM

Automated check by AgenTrust Contributor Check.

lywinged
lywinged previously approved these changes Aug 31, 2026

@lywinged lywinged left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. This is the generator from #247, and it does what was asked: it turns the subject finding into a property every pattern constraint is held to, rather than a second fixed matrix.

I ran it against the two regressions it claims to catch, on both sides of the split, since a discrimination test is only worth its own counterfactual.

  • Put the pre-#244 prefix pattern back on subject in the schema: test_every_pattern_is_mirrored_or_split_or_declared fails and names spiffe:// and did:. The matrix carries those two literals today, but only because they were hand-added as the guard after #244 exposed the miss; the generator derives them from the pattern, so the same prefix shape on a field nobody has patched is caught without anyone thinking to list it.
  • Drop the signature pattern from the model, a field the old sweep never touched because it is optional and absent from BASE: the same test fails and names the splitting probes. So the mirroring check has teeth on the five optional pattern fields FULL reaches that BASE did not (weights_digest, parent_record_hash, references[].digest, references[].retention, signature), which is the coverage that did not exist before this.

Two things I checked that hold: the ten patterns are read out of the schema, so a new one cannot be added without this file failing until it is accounted for; and the three-indirection walk in _model_pattern_strings is load-bearing, signature's pattern is found only by the full walk and drops out of a shallow one, which is the miss the docstring describes.

Two non-blocking notes, neither a reason to hold it:

  1. The mirror match at if pattern in model_patterns is by string against a flat set, not by field. It is sound today because every field carrying the shared sha... digest pattern is genuinely constrained by it in the model, which I checked by injecting a malformed digest at each. It would stop being sound if a later field took that pattern in the schema and was left unconstrained in the model: the string would still be in the set and the field would read as mirrored. Matching the pattern to the field's own model constraint would close that, whenever it is convenient rather than now.
  2. A pattern on a field absent from FULL makes _splitters raise KeyError from _get(FULL, path) rather than the clean message test_the_fixture_reaches_every_pattern_in_the_schema is written to give. The reachability test goes red in the same run, so nothing slips, but the message a maintainer sees is the raw exception rather than the field name.

Suite is green here and the change is one file over the current head.

@chernistry

Copy link
Copy Markdown
Contributor Author

Thanks for running both counterfactuals rather than reading the diff — the signature one is the case I most wanted checked, since it is exactly the coverage BASE never reached. On your two notes: I would rather fold both in here than defer them, since each is a few lines — matching the pattern to the field's own model constraint, and catching the missing path so the reachability failure prints the field name instead of a raw KeyError. Say the word if you would prefer the PR stay as approved and both land as a follow-up. Separately, the gate check is red here and fails in about five seconds; it fails the same way on #234, so I suspect it is repo-side rather than this branch — is there something I should do on my end, or does it need a maintainer?

The mirror check read every pattern the model carries into one flat set and
asked whether a schema pattern's string was a member of it. Two fields
sharing a pattern string is not the same fact as one field's schema
constraint mirroring that same field's model constraint, and the flat set
could not tell the two apart: a pattern reassigned onto an unrelated field
would still read as mirrored as long as some other field carried the same
string.

_model_field_patterns keys each pattern to the specific field it
constrains, and test_every_pattern_is_mirrored_or_split_or_declared now
compares a schema path against that same path's own model pattern instead
of set membership. test_the_mirror_check_is_by_field_not_flat_membership
covers the case a flat set would miss: a pattern string genuinely used
elsewhere in the model, reassigned onto a field it does not constrain.

Signed-off-by: chernistry <sanderchernitsky@gmail.com>
A pattern constraint on a field FULL omits made _splitters raise a bare
KeyError out of _get, since it read the path unguarded.
test_the_fixture_reaches_every_pattern_in_the_schema already catches the
same reachability gap cleanly and by field name via _reachable; this was
the other place reading an arbitrary schema path out of FULL, and it
failed worse when it hit the same gap.

_splitters now checks reachability first and raises LookupError naming the
field, and test_every_pattern_is_mirrored_or_split_or_declared catches
that and reports it alongside every other unaccounted pattern instead of
aborting the run. test_a_missing_path_fails_with_the_field_name_not_a_keyerror
covers it directly.

Signed-off-by: chernistry <sanderchernitsky@gmail.com>
@chernistry

Copy link
Copy Markdown
Contributor Author

Both notes are folded in.

  • Mirror match by field, not flat-set membership: e5f6cdb
  • `_splitters` names the field instead of raising a bare KeyError: 4d3efd1

Each commit adds a test that fails for the stated reason on the prior commit and passes after it; full suite, ruff, and mypy are clean on the branch.

@imran-siddique imran-siddique left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging. This is the sharper half of #247 and it is not the half I filed.

I opened #247 on the premise that four surfaces state the rules and nothing says which wins. @lywinged's #248 then audited all ten schema patterns against the model and found one divergence, which I recorded as narrowing the issue. You have found the reason that audit could not have said what I took it to say:

A sweep cannot disagree about a field it never sets, and a green run says the same thing either way.

Five of the ten patterns are on optional fields absent from BASE, so they were never mutated and never reached their own boundary. The test answered "do these two validators disagree about the values I tried" and I read it as answering "are these ten constraints consistent". Those are different questions and only one of them was being asked.

That is the same defect class the test exists to catch, one level up: something that looks like coverage, reports green, and is not discriminating. A test that cannot fail is worse than an absent one, because the absent one does not stop anyone looking.

Probing each constraint's own boundary is the right fix, and holding models.py's mirroring claims rather than trusting the comment that says they mirror is the other half. A constant asserting it is "mirrored verbatim" is a claim, and until something compares the two it is an unverified one.

Merging, and I am updating #247 with what this changes about the issue.

@imran-siddique
imran-siddique merged commit 98aae17 into agentrust-io:main Sep 2, 2026
1 check failed
@chernistry

Copy link
Copy Markdown
Contributor Author

Thanks for the writeup, and for the extra scrutiny on this one — the sweep-vs-audit distinction (a test that can't disagree on a field it never sets vs. one that probes each field's own boundary) is a good general lesson to keep in mind past this specific fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:MEDIUM Contributor check flagged MEDIUM risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants