Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/changes/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

## Features

- #940: Added shared validation for packaged agent skills and the `skills:check` Nox session.

## Summary
* #940: Added shared validation for packaged agent skills and the `skills:check` Nox session.
* #942: Added api-contract-audit skill for identifying mismatches between type annotations, docstrings, and runtime
behavior
90 changes: 90 additions & 0 deletions exasol/toolbox/skills/api-contract-audit/SKILL.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This skill is packaged under exasol.toolbox.skills and is discovered by skills:check, but skills:install currently installs only exasol-python-toolbox.
How is a user expected to install api-contract-audit?
Please either extend the installer to support this skill or move/document it as a separately installed skill.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that api-contract-audit should remain as a separate skill under exasol/toolbox/skills/api-contract-audit/SKILL.md, since it is generic and not part of the exasol-python-toolbox skill. The current limitation is that skills:install only installs the PTB skill, while skills:check already discovers all packaged skills. I would keep this location and rely on the installer PR to add support for installing individual or all packaged skills, rather than move this skill under exasol-python-toolbox.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Since you are already handling the installer in #960, should I add documentation for manually installing this skill in the meantime?

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.

We should merge @jana-selva 's first & then adapt the skills:install and documentation in this PR.

Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
---
name: api-contract-audit
description: Audit a Python library's public API for inconsistencies between type annotations, docstrings, user-facing documentation/examples, and actual runtime behavior. Use when reviewing API changes, checking whether public methods accept undocumented parameter shapes, or validating that docs and type hints match enforcement in code.
---

# API Contract Audit

Use this skill when the task is to review a Python package's public API contract rather than implement features.

Focus on externally visible behavior:
- public functions and methods
- exported classes
- user-facing docs and examples
- runtime validation and coercion

Do not assume the annotation is the source of truth. The goal is to find drift between multiple sources of truth.

## Inputs To Compare

For each relevant public API entrypoint, compare:
- signature and type annotations
- docstring parameter and return descriptions
- examples in docs, README, and example scripts
- runtime behavior in the implementation path

Treat these as separate claims. Report when they disagree.

## What To Look For

Prioritize these mismatch patterns:
- annotation says `str`, but implementation accepts or requires tuple-like schema-qualified identifiers
- annotation says one scalar type, but runtime hard-checks another with `isinstance(...)`
- docstring says a parameter or return type that does not match the signature
- docs/examples call the API with arguments that disagree with the annotation or actual signature
- implementation silently accepts more forms than the public docs mention
- wrappers expose narrower types than the lower-level public method they forward to
- runtime coercion like `int(val)` or `str(val)` that makes the public contract broader than the annotation suggests

Typical search signals:
- `isinstance(`
- `type(`
- `raise ValueError`
- identifier-formatting helpers
- tuple-specific branches
- wrapper methods that pass through parameters unchanged

## Workflow

1. Enumerate the public API surface relevant to the request.
2. Read the implementation of each public method and the immediate downstream code it calls.
3. Trace parameter handling until the real runtime constraint is clear.
4. Cross-check docstrings and user-facing docs/examples.
5. Report only concrete inconsistencies or clearly label residual uncertainty.

Prefer `rg` for discovery. Good starter patterns:

```bash
rg -n "^class |^ def " package_dir
rg -n "isinstance\\(|type\\(|raise ValueError|raise TypeError" package_dir
rg -n "function_name\\(" README.md doc examples test
```

## Output Format

Present findings first, ordered by severity.

For each finding include:
- severity: High, Medium, or Low
- affected API
- what the annotation/doc claims
- what the implementation really does
- file references for both sides of the mismatch

After findings, optionally include:
- open questions where intended behavior is unclear
- a short summary of recurring patterns

If no findings are discovered, say that explicitly and mention any coverage limits.

## Severity Guidance

- High: likely to mislead callers, break type-checked usage, or document the wrong accepted input shape
- Medium: accepted behavior is real but under-documented, or docs/examples contradict each other
- Low: naming, docstring argument labels, stale prose, or smaller clarity issues

## Boundaries

- Do not rewrite the API contract on your own. If code, docs, and examples disagree, report the disagreement.
- Do not stop at the first example. Check for the same pattern across sibling APIs.
- Do not treat private helper inconsistencies as findings unless they affect public behavior.
25 changes: 25 additions & 0 deletions test/resources/skills/api-contract-audit/eval_cases.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
version: 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is a skill-specific test needed to validate that this skill’s resource and evaluation cases are available and correctly structured?
The existing tests appear to cover only exasol-python-toolbox; skills:check validates general Markdown rules but not these evaluation cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. Should I add skill-specific tests similar to the ones for exasol-python-toolbox or is there a preferred pattern to follow for new skills?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, please add skill-specific tests similar to the existing exasol-python-toolbox tests.
At minimum, verify that SKILL.md and eval_cases.yml are available, the frontmatter is valid, and the evaluation cases contain the expected structure. This will also establish a reusable pattern for future skills.

@ArBridgeman ArBridgeman Sep 22, 2026

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.

Ah, to be fair, some of these are already covered by the skill:check. (I'd verified that in my initial review). Perhaps, it makes more sense to validate that this particular file is included in a test or CI run of skills:check - right now we mock it in the unit tests, so I would suggest using that to avoid duplicating tests & to not have minor differences over time. This would then be an integration test similar to what we have in nature to the cookiecutter tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, @ArBridgeman , that makes sense.

@Rimsha2535 , could you please update the test to exercise the actual packaged skills, so api-contract-audit is verified as discovered and checked by skills:check instead of duplicating the shared validation tests? The eval_cases.yml validation can be handled separately if needed.

skill: "api-contract-audit"
cases:
- id: "find-type-annotation-mismatch"
category: "audit"
prompt: "Audit the PyExasol public API for mismatches between type annotations and runtime behavior."
expected:
must_include:
- "isinstance"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requiring isinstance makes this evaluation dependent on one implementation detail.
Could this expectation be generalized to runtime validation/behavior evidence, so valid findings based on type(), coercion, or other checks are also accepted?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jana-selva I included isinstance as it is explicitly listed as a typical search signal in the skill itself. But I am happy to generalize it if needed.

- "annotation"
- "severity"
must_not_include:
- "rewrite the API"
- "fix the code"

- id: "find-docstring-mismatch"
category: "audit"
prompt: "Check if PyExasol docstrings match the actual function signatures and runtime behavior."
expected:
must_include:
- "docstring"
- "signature"
- "mismatch"
must_not_include:
- "rewrite the API"