Skip to content

test(rules): mirror and cover CSDK-021, CSDK-022 - #167

Closed
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/claude-sdk-ts-description-quality
Closed

bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/claude-sdk-ts-description-quality

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#90, 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

CSDK-017/018 cover the Python side of description quality; the TypeScript half was missing. CSDK-014 only checks that a description exists, so "TODO: describe this tool." and "Gets data." pass today — leaving the tool in exactly the state CSDK-014 exists to prevent.

CSDK-014's own explanation says why this costs more on the TS side: there's no docstring fallback, so the description argument is the entire prompt-side account of the tool. CSDK-022 also names the mitigation that isn't one — the Zod input schema constrains the shape of the arguments once the model has decided to call this tool, and says nothing about whether calling it was the right move.

What this PR does

  1. Mirrors claude_sdk/tool_definition.yaml into testdata/rules-fixture/.
  2. Adds cases to policyRuleCases, as TestPolicyRules_AllRulesCovered requires.
case expectation
CSDK-021 — "TODO: describe this tool." fires
CSDK-021 — real description silent
CSDK-022 — "Gets data." fires
CSDK-022 — full sentence silent
CSDK-022 — empty description silent

Five cases rather than four. CSDK-022 pairs description_length_lt: 40 with has_docstring: true so an empty description stays CSDK-014's finding rather than double-reporting — length 0 is also under the threshold. The fifth case pins that guard.

Verification

$ RULES_REPO=../trustabl-rules scripts/check-rules-sync.sh
rules fixture is in sync with production (86 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#90, 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 claude_sdk/tool_definition.yaml into testdata/rules-fixture and
adds cases to policyRuleCases, as TestPolicyRules_AllRulesCovered
requires.

CSDK-017/018 cover Python; the TypeScript half was missing.

Five cases rather than four. CSDK-022 pairs description_length_lt with
has_docstring so an empty description stays CSDK-014's finding instead of
double-reporting, and the fifth case pins that guard.
@jhumel-code

Copy link
Copy Markdown
Collaborator

Thanks @bradAGI. This is the engine half I want to see alongside a rules PR: the fixture copy of claude_sdk/tool_definition.yaml is byte-identical to the one in trustabl/agent-reliability-rules#90, so check-rules-sync.sh stays green as long as the two land together.

The five cases hold up when I walk them. parseTSTool routes KindClaudeSDKTool to DiscoverTSTools, and each snippet imports from @anthropic-ai/claude-agent-sdk and passes four positional arguments, so the tool is genuinely discovered rather than the case passing on a false Applies. The empty-description case is the one I would have asked for: PredHasDocstring trims and compares against the empty string, so has_docstring: true fails and CSDK-022 stays silent while CSDK-014 keeps that finding.

One thing to note rather than change: the CSDK-021 fire snippet is 25 characters, so that tool also trips CSDK-022. CSDK-017 and CSDK-018 already overlap the same way in Python, so I would rather keep the pair consistent than special-case it here.

The blank-line removal in policyRepoRuleCases is unrelated noise, harmless either way. Before these ship they need a rationale doc in trustabl-rulebook, which is on us, and I am 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