-
Notifications
You must be signed in to change notification settings - Fork 2
Add api-contract-audit skill for type hint checking #962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5c148d4
0d5eaac
dd90515
9b5766b
d13b845
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| version: 1 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, to be fair, some of these are already covered by the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| 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" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requiring
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" | ||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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-auditshould remain as a separate skill underexasol/toolbox/skills/api-contract-audit/SKILL.md, since it is generic and not part of theexasol-python-toolboxskill. The current limitation is thatskills:installonly installs the PTB skill, whileskills:checkalready 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 underexasol-python-toolbox.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:installand documentation in this PR.