Python Code Assistant skill#2
Conversation
oto-macenauer-absa
left a comment
oto-macenauer-absa
left a comment
There was a problem hiding this comment.
While 336 lines is under the limit, the skill is a flat wall of coding standards. There's no decision logic, no branching, no "if X then load Y." Compare with create-issue which has a two-phase architecture and delegates mechanical work.
| --- | ||
| name: python-standards | ||
| description: >- | ||
| Python coding standards for CPS projects. Covers repository-first conventions, type annotations, error handling, testing, logging, documentation, async code, and dependency hygiene. Activate this skill whenever the user is editing or creating .py files, working in a Python project, or asks for help writing functions, classes, tests, modules, or fixing code - even if they do not say "Python" explicitly. Also triggers on questions about coding conventions, type hints, pytest, imports, logging setup, or dependency management in a Python context. |
There was a problem hiding this comment.
Every time someone edits a .py file in a CPS project, this skill injects ~1,300 tokens of mostly-known Python conventions. Over a day of Python work (say 20 skill activations), that's ~26,000 wasted input tokens for information the model already has.
If it's going to be injected very often, I'd make it considerably slimmer. Like 100 lines top, keep only the important rules, models already know how to write Python.
There was a problem hiding this comment.
It this expected to be in review phase or in dev phase?
There was a problem hiding this comment.
Yes, the skill could be shortened as it is not for human but for AI.
Shorting - caveman style.
| - Create custom exception classes for domain-specific errors rather than reusing generic built-ins. | ||
| - Fail fast and fail loudly: validate inputs early and raise immediately on invalid state. | ||
|
|
||
| ```python |
There was a problem hiding this comment.
The skill doesn't require the code examples to follow the guidelines, if it's helpful it should be moved to references and loaded only on demand.
| - For vendor API responses, be lenient about unknown fields (vendors may add them) but strict about expected fields. | ||
| - Define data contracts as explicit models or data classes, not raw dicts. Typed structures make the expected shape discoverable and enforceable. | ||
|
|
||
| ## Testing |
There was a problem hiding this comment.
not every python code contains tests, this should be reference and loaded only for tests
There was a problem hiding this comment.
Maybe the skill could start with kind of decision table. Helping with exact navigation.
There was a problem hiding this comment.
Yup, agree. It's unlikely that 1 common standard will be applicable broadly anyway
| Before applying these standards in an existing repository: | ||
|
|
||
| - Inspect the nearby Python files and project configuration first. | ||
| - Follow the repository's established formatter, import sorter, linter, type checker, test framework, packaging flow, and docstring style unless the user asks you to change them. |
There was a problem hiding this comment.
This is interesting part ... maybe the skill should stop here if the repo contains all of the information already, because then it would add the same info twice.
There was a problem hiding this comment.
Is possible to replace some AI detections by smart scripts?
It woul reduce AI activity and be more determistic.
miroslavpojer
left a comment
miroslavpojer
left a comment
There was a problem hiding this comment.
I like it, see my reactions in comments.
|
|
||
| ## Type Annotations | ||
|
|
||
| Python is dynamically typed, which means entire categories of bugs (type mismatches, contract violations, null references) can reach production undetected. Type annotations are the primary defence against this. |
There was a problem hiding this comment.
This is what an LLM already knows though
|
|
||
| ### Rules | ||
|
|
||
| - Annotate all public function signatures: both parameters and return types. |
There was a problem hiding this comment.
I don't know, I wouldn't be that strict in general. Maybe some people don't want to be bothered for some scripts or some less-production grade code. Maybe let the user choose this during the first phase of the skill - let the user choose and the skill will navigate it based on that
There was a problem hiding this comment.
How about two regimes - it will ask 1st.
|
|
||
| ### Structure | ||
|
|
||
| - Follow the repository's existing test layout. In repos that mirror the source tree, `src/handlers/` maps to `tests/unit/handlers/`. |
There was a problem hiding this comment.
hmm, the sub dir unit/ is not really that commonly used I think
There was a problem hiding this comment.
I would be a bit less strict. Not everyone has integration tests also
There was a problem hiding this comment.
We have, Maybe make it smart using some if section + detect?
| ## Dependencies | ||
|
|
||
| - Follow the repository's existing dependency-management approach. For new projects, or repos that already use modern packaging, prefer `pyproject.toml` with PEP 621 metadata. | ||
| - If the repository uses `requirements.txt`, constraints files, Poetry, uv, or another established workflow, stay consistent unless the user explicitly asks for a migration. |
There was a problem hiding this comment.
I don't understand this sentence I think. What do you mean by constraints files?
|
|
||
| ## Database | ||
|
|
||
| CPS projects prefer to access databases directly using `aiosql` to keep SQL out of Python string literals. |
There was a problem hiding this comment.
This is not what everyone will prefer I think, but can be an optional recdommendation. Maybe we can be strict in a way of not having hard-coded sql statements in python strings, and recommend aiosql, but proabbly not insisting on it
There was a problem hiding this comment.
In this repo no CPS should be present.
| - Use the `secrets` module for generating tokens and random values, not `random` (which is not cryptographically secure). | ||
| - Validate and sanitise all user-provided input before using it in queries, commands, or file paths. | ||
| - Be explicit about timeouts on all network calls. Hanging connections are a denial-of-service vector. | ||
| - Lock or pin dependencies according to the repository's release process, and audit them regularly. Supply-chain attacks through compromised packages are a real and growing threat. |
There was a problem hiding this comment.
Be a bit more procedural. This sounds like a recommendation to the user of the skill
…into feature/python-standards-skill
|
I did read all of yours comments and did a refactored another version of the skill. This one should:
@lsulak @oto-macenauer-absa @miroslavpojer |
4 participants
Background
This pull request introduces a comprehensive Python coding standards skill for CPS projects, including detailed documentation, evaluation scenarios, and context-based triggers for activation. The main changes are the addition of the standards documentation, a set of evaluation prompts to test adherence, and a trigger configuration that ensures the skill activates only in appropriate Python-related contexts.
Related issus
Closes #https://github.com/absa-group/cps-agentic-toolkit/issues/5