Skip to content

test(rules): mirror and cover PYD-012 - #150

Closed
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/pydantic-ai-observability
Closed

bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/pydantic-ai-observability

Conversation

@bradAGI

@bradAGI bradAGI commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Engine half of a coordinated pair. Rules half: trustabl/agent-reliability-rules#71, on a branch of the same name, so the rules-sync job resolves the matching pack rather than main. Neither half should merge alone — check-rules-sync.sh fails if they do.

What the pair adds

Pydantic AI had no observability rule; OpenAI ships OAI-010 and ADK ships ADK-009 for the same print() pattern. The framing is Pydantic AI's own rather than a port: Pydantic AI emits OpenTelemetry spans for each run and tool call, so everything around this print is already correlated to a trace — the bare print is the one diagnostic landing outside it, unattached to the run that produced it.

What this PR does

  1. Mirrors the new observability.yaml into testdata/rules-fixture/.
  2. Adds cases to policyRuleCases, as TestPolicyRules_AllRulesCovered requires.
case expectation
print("looking up " + order_id) fires
logger.info("looking up %s", order_id) silent
pprint({...}) silent

Three cases rather than two. The pprint case pins has_print_call's bare-callee behavior — the rule must not regress into substring matching that sweeps in pprint and every other callee whose name merely contains "print". The schema calls that out explicitly as the trap has_body_text falls into, so it's worth a standing test rather than a comment.

The silent case applies the remediation the fix text prescribes (a module logger) rather than deleting the call.

Verification

$ RULES_REPO=../trustabl-rules scripts/check-rules-sync.sh
rules fixture is in sync with production (87 files compared)

$ go vet ./internal/rules/
$ go test ./internal/rules/
ok  	github.com/trustabl/trustabl/internal/rules

Engine half of a coordinated pair with trustabl/agent-reliability-rules#71, on a
branch of the same name so the rules-sync job resolves the matching pack
rather than main. Neither half should merge alone — check-rules-sync.sh
fails if they do.

Mirrors pydantic_ai/observability.yaml into testdata/rules-fixture and adds
cases to policyRuleCases, as TestPolicyRules_AllRulesCovered requires.

Three cases: the print, the module-logger remediation the fix text
prescribes, and a pprint call that must stay silent. The third pins
has_print_call's bare-callee behavior, so the rule cannot regress into
substring matching that sweeps in pprint and every other callee whose
name contains "print".
@jhumel-code

Copy link
Copy Markdown
Collaborator

Thanks, @bradAGI. Pairing the fixture mirror with the tests in one PR, and saying plainly that neither half merges alone, is exactly the right shape. The fixture copy is byte identical to the file in trustabl/agent-reliability-rules#71, so rules-sync has nothing to object to once both land.

I walked the three cases against PredHasPrintCall. The fire case hits the bare print callee inside the discovered function node. The logger case resolves to an attribute callee (logger.info), and the module level getLogger sits outside the function node anyway, so it stays silent for the right reason. The pprint case resolves to the identifier pprint, which is not print. Nothing passes vacuously here, and I agree the third case is worth keeping as a standing guard on the bare callee contract rather than a comment.

The line removed from policyRepoRuleCases is the gofmt fix for that block, so that is a real cleanup rather than stray whitespace.

Only nit, and it is not blocking: the new cases land at the end of policyRuleCases rather than in the PYD-* banner block near PYD-001 through PYD-007, and the file otherwise groups by SDK.

PYD-012 still needs a rationale doc in trustabl-rulebook before it ships. That part is on us, and we are happy to pair on it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants