Skip to content
Merged
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
19 changes: 15 additions & 4 deletions .agents/guidelines/python.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,33 @@
# Python guidelines for libraries

Our code runs inside other people's programs. Every choice we make (a dependency, a global, a log line, a warning) becomes theirs too.
Our code runs inside other people's programs. Every choice we make (a dependency, a global, a log line, a warning) becomes theirs too. Where `.agents/project.md` or the surrounding code settles a convention differently (naming, layout, libraries), follow them.

## Code

- Support the Python versions the project declares in `pyproject.toml`, and test on all of them.
- Type-annotate the public API completely and ship the `py.typed` marker. Keep the type checker clean in strict mode for the public modules.
- Do not do work at import time: no network, no file access, no reading environment variables, no logging configuration. Importing the library must be cheap and free of side effects.
- No global mutable state. Configuration goes into objects the caller creates and passes in.
- Raise exceptions from the library's own hierarchy, rooted in one base class, with messages that say what was wrong and what to do. Never `print`, and never call `sys.exit`.
- Log through `logging.getLogger(__name__)` and add no handlers: the application decides where logs go.
- Do not change the arguments the caller passes in; return new values. Builders and accumulators whose job is to be changed are the exception, and say so in their documentation.
- Accept the most general type that works (`Iterable`, `Mapping`, `os.PathLike`) and return the most specific one.
- Async APIs, if any, never block the event loop; do not start event loops or threads behind the caller's back.
- Give closed sets of values an `Enum`. A tuple whose meaning depends on position becomes a named type. A boolean parameter that switches between two behaviours is usually two functions.
- Async APIs, if any, never block the event loop; do not start event loops or threads behind the caller's back. Catch `CancelledError` only to clean up and re-raise.
- Name things after what they mean, put units in names (`timeout_seconds`, `size_bytes`), and use one word for one concept across the public API.
- Give every number with a meaning a named constant and a reason. Leave no dead code: unused functions and parameters, commented-out code, TODOs without an issue.

## Errors and logging

- Raise exceptions from the library's own hierarchy, rooted in one base class, with messages that say what was wrong and what to do. Never `print`, and never call `sys.exit`.
- Raise on input you did not expect. End a chain of known cases with an `else` that raises (`typing.assert_never` lets the type checker prove every case is handled), so an unknown value never falls through to a default; never hide a failure behind a `None` return.
- Raise specific errors where the problem is found. Do not catch `KeyError`, `TypeError` or `AttributeError` far from where they happen as a stand-in for a real condition, and do not let them escape the public API where your own exception is documented.
- Chain the original exception (`raise LibraryError(...) from exc`) when you wrap one, so the caller keeps the cause.
- Log through `logging.getLogger(__name__)` and add no handlers: the application decides where logs go. Never log the caller's data or secrets.

## Dependencies

- Every dependency is imposed on all our users: add one only when it is worth that, and keep version ranges as wide as the code supports. Do not pin exact versions.
- Heavy or rarely needed dependencies go into optional extras, imported lazily with a clear error when missing.
- Prefer the standard library; check that it does not already solve the problem before you write a helper.

## Packaging

Expand Down
7 changes: 6 additions & 1 deletion .agents/guidelines/testing.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Testing guidelines for libraries

Users depend on the behaviour we promise. Tests pin that promise down.
Users depend on the behaviour we promise. Tests pin that promise down. Where `.agents/project.md` or the existing tests settle a convention differently (layout, naming, fixtures, tools), follow them.

## What to test

Expand All @@ -9,6 +9,7 @@ Users depend on the behaviour we promise. Tests pin that promise down.
- Cover edge cases of inputs: empty, very large, unicode, `None` where allowed, wrong types where the API promises an error.
- Check that errors are the documented exception types with useful messages.
- Run the test suite on every supported Python version and operating system the project claims, and against the lowest and the highest versions of dependencies that the version ranges allow.
- Do not test the standard library or the dependencies themselves.

## Kinds of tests

Expand All @@ -20,6 +21,10 @@ Users depend on the behaviour we promise. Tests pin that promise down.
## How to write them

- One behaviour per test, named after what it checks.
- No logic in a test: no `if`, loops over cases, `try`, or skipping depending on the environment beyond the documented platform markers. To cover several inputs, parametrise the test.
- Put the setup in fixtures or small factories, so the test body reads as "call, then check".
- Build expected values by hand. A test that computes them with the code under test proves nothing: do not check a decoder with its own encoder alone.
- Assert exact values and whole results (`assert result == expected`), not fragments (`> 0`, `in`, a substring). Floats compare with a tolerance (`pytest.approx`); an error's message is checked with `pytest.raises(..., match=...)`; property-based tests assert the invariant instead.
- No network and no reliance on the machine's locale, time zone or current time; inject or freeze them.
- Tests pass in any order and in parallel.
- Deprecations: test that the old API still works and emits the documented `DeprecationWarning`, and turn warnings into errors in the test run so new ones are noticed.
Expand Down
30 changes: 30 additions & 0 deletions .agents/prompts/pure-refactor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Check that a change keeps behaviour

Use this on a change that claims to change no behaviour: a refactoring, a move, a rename, a dependency swap. Assume it does change behaviour until you have traced every path. You are checking, not rewriting: do not edit files unless you are asked to.

## What to check

The change you were given; if none was named, the current branch against the default branch, plus uncommitted and untracked changes. Read every changed file in full, old and new, and find every caller of every changed function, including references by string: registries, entry points, configuration, templates, dynamic lookups. Behaviour most often changes at a call site the diff does not show.

## How to work through it

Split the change into slices small enough to trace completely: a function, a class, a group of related functions. If your tool can run sub-agents, give each slice to one and run them in parallel; otherwise work through them one at a time and write down each result before the next. Tell each sub-agent this prompt, its slice and how to get the whole change (a range, or the uncommitted and untracked changes); a sub-agent given one slice traces that slice only, and whoever split the change merges the results.

For every slice, compare the old code with the new, path by path:

- **Interface:** the same importable names and module paths, parameter names (callers may pass them by keyword), configuration keys and environment variables, command-line flags, routes, and names in stored data and events.
- **Inputs:** the same accepted arguments, types, defaults and validation; the same behaviour on `None`, empty and boundary values.
- **Outputs:** the same return values and types, the same order of collections, the same serialised form (field names, missing versus null, number and date formats).
- **Errors:** the same exceptions for the same conditions, the same error codes and messages reaching callers, the same log level and event.
- **Side effects:** the same writes to storage, the same outgoing requests (payload, headers, order, retries, timeouts), the same metrics and their labels.
- **Order and timing:** the same order of operations wherever it can be observed: transaction boundaries, when a lock is taken, when a stream yields, when a task starts.
- **Code that appeared or disappeared:** a check, a default, a conversion or a log line that runs now and did not before, or the other way round.
- **Tests:** a changed assertion or fixture in a change that claims to keep behaviour is a behaviour change until shown otherwise.

## How to report

- **Behaviour changes**: each as `file:line`, the old behaviour, the new one, and how a caller, a user or the stored data would notice.
- **Not fully traced**: what differs and what would settle it.
- **Unchanged**: each slice and what you traced in it.

End with a one-line verdict: behaviour kept, behaviour changes found, or not settled (and how many paths are not fully traced). A change the author intended still counts if the description does not say so.
2 changes: 1 addition & 1 deletion .agents/prompts/refactor-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ Answer these questions before you change the structure of code. If an answer is
5. **Reach.** Does it stay inside the code the task is about? Shared modules, public interfaces and changes across many files need their owners' agreement.
6. **Timing.** Will it collide with open pull requests or a release in progress?

When you go ahead, keep refactoring commits apart from behaviour changes, run the tests after every step, and explain in the pull request what moved where and why behaviour is unchanged.
When you go ahead, keep refactoring commits apart from behaviour changes, run the tests after every step, and explain in the pull request what moved where and why behaviour is unchanged. Before you open it, check the refactoring commits with `.agents/prompts/pure-refactor.md`.
32 changes: 25 additions & 7 deletions .agents/prompts/review.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,35 @@
# Review a change

Review the change as a careful senior engineer of this project would. You are reviewing, not rewriting: do not edit files unless you are asked to.
Review the change as a careful senior engineer of this project would. You are not the author and you owe the change nothing: every addition has to earn its place. You are reviewing, not rewriting: do not edit files unless you are asked to.

## What to review

The change you were given: a pull request, a branch, a commit range or a set of files. If none was named, review the current branch against the default branch, plus uncommitted changes. Read enough of the surrounding code to judge each change in its context, and read `.agents/project.md` for the project's conventions.
The change you were given: a pull request, a branch, a commit range or a set of files. If none was named, review the current branch against the default branch, plus uncommitted and untracked changes. Read `.agents/project.md` for the project's conventions, and the guidelines in `.agents/guidelines/` that cover the code the change touches.

Read every changed file in full, not only the changed lines, and look up the callers of every function whose behaviour changed. Most problems are in what the change does not fit with: a helper that already exists, a pattern the rest of the module follows, a caller that now breaks. Search for them and cite where they are instead of guessing.

## How to work through it

1. **Split the change into slices**, each small enough to read completely: a module or a package with its tests. A small change is one slice.
2. **Review each slice** against the list below. If your tool can run sub-agents, give each slice to one and run them in parallel; otherwise work through the slices one at a time and write down each slice's findings before you start the next. Tell each sub-agent this prompt, its slice and how to get the whole change (a range, or the uncommitted and untracked changes). A sub-agent given one slice does steps 2 and 4 for that slice only; steps 1 and 3 stay with whoever split the change.
3. **Review the change as a whole**, for what no slice can see:
- a second task mixed into the change, or files that do not belong in it (generated output the project does not commit, local notes, debugging leftovers);
- documentation, examples and the changelog that the change makes wrong;
- the same helper written twice, one idea under two names, an abstraction with a single implementation and no test double;
- behaviour that changes although the description says it does not.
4. **Check every finding** at the line you cite. Drop what you cannot confirm, merge duplicates, and list repeats once with their other locations.

## What to look for, most important first

1. **Correctness.** Does the change do what its description says? Check edge cases, error paths, empty and missing inputs, concurrency, time zones and encodings.
1. **Correctness.** Does the change do what its description says? Check edge cases, error paths, empty and missing inputs, concurrency, time zones and encodings. An unexpected input or state must fail loudly, not fall back to a default nobody will notice.
2. **Security.** Input validation, injection, authorisation, secrets in code or logs, unsafe deserialisation, new dependencies and what they can do.
3. **Compatibility and data.** Public APIs, configuration and file formats, database migrations, rollout and rollback.
4. **Tests.** Would a test fail without the change? Are the important cases covered? Do the tests check behaviour rather than implementation details?
5. **Operations.** Logging, metrics, error messages a person can act on, timeouts and retries.
6. **Readability.** Names, structure, duplication, dead code, comments that explain why rather than what.
5. **Necessity.** For every addition, meaning a new public name, parameter, option, configuration key or dependency: what problem does it solve now, and where is it used? "It might be useful later" is a reason to leave it out.
6. **Operations.** Logging, metrics, error messages a person can act on, timeouts and retries.
7. **Readability.** Names, structure, duplication, dead code, comments that explain why rather than what.

Skip style points that a formatter or linter already enforces.
Skip style points that a formatter or linter already enforces. Problems in lines the change adds or modifies are not excused because the code "already was like that"; problems elsewhere in a changed file are at most Consider.

## How to report

Expand All @@ -25,4 +39,8 @@ Group findings by severity:
- **Should fix**: likely problems, missing tests, confusing code.
- **Consider**: suggestions and questions.

For each finding give the file and line, what is wrong, why it matters, and a concrete fix. If you are not sure, say so and say what would settle it. End with a one-line verdict. If you found nothing serious, say so plainly: do not invent findings to fill the list.
For each finding give the file and line, what is wrong, why it matters, and a concrete fix. Where the author has to decide, ask the question they must answer ("Where is this option used?", "What happens when the list is empty?"). If you are not sure, say so and say what would settle it.

Then add **Justify or remove**: every addition (see Necessity), each with the reason you found in the change or its description, or "no reason found". Additions are judged here only, not again among the findings.

End with a one-line verdict: ready, ready after the "should fix" items, or needs rework. If you found nothing serious, say so plainly: do not invent findings to fill the list.
11 changes: 6 additions & 5 deletions .claude/agents/reviewer.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
---
name: reviewer
description: Reviews a change in its own context and reports findings by severity, without editing anything. Use for large changes or when an independent review is wanted.
description: Reviews a change, or one slice of it, in its own context and reports findings by severity, without editing anything. Use for large changes, for each slice of a split review, or when an independent review is wanted.
tools: Read, Grep, Glob, Bash
---

You review code changes in this repository. You never edit files, commit or push.

1. Read `.agents/project.md` for the project's conventions and commands, and `.agents/prompts/review.md` for the checklist. Follow the checklist. Where the profile is blank, find the facts in the repository as `AGENTS.md` says ("Before you start"), and say which ones you found there.
2. Work out what to review from the request. If it names nothing, review the current branch against the default branch, plus uncommitted changes.
3. Use Bash only to read: `git diff`, `git log`, `git show`, and the project's lint or test commands when you were asked to run them.
4. Report findings grouped as Must fix, Should fix and Consider, each with the file and line, the problem, why it matters and a fix. End with a one-line verdict.
1. Read `.agents/project.md` for the project's conventions and commands. Where the profile is blank, find the facts in the repository as `AGENTS.md` says ("Before you start"), and say which ones you found there.
2. Follow the prompt the request names: `.agents/prompts/pure-refactor.md` for a check that behaviour is kept, `.agents/prompts/review.md` otherwise. If the request gives you one slice of a larger change, review that slice completely and read the rest of the change only for context; the whole-change part of the prompt is left to the caller.
3. Work out what to review from the request. If it names nothing, review the current branch against the default branch, plus uncommitted and untracked changes.
4. Use Bash only to read: `git diff`, `git log`, `git show`, and the project's lint or test commands when you were asked to run them.
5. Report as the prompt says, with the file and line of every finding.
5 changes: 3 additions & 2 deletions .claude/skills/pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ Open a pull request for the current branch. $ARGUMENTS

1. Read `.agents/project.md` for the default branch, the language of pull requests and how they refer to issues.
2. Make sure everything is committed and the project's checks pass. Report what fails and ask before going on.
3. Write the title in the project's format. Write the description from the commits and the diff: what changes and why, how it was tested, risks and how to roll back, and the issue it resolves. Fill in the repository's template if it has one (`.github/pull_request_template.md`, `.gitlab/merge_request_templates/`).
4. Show the title and description and wait for the user's approval. Then push the branch and open the pull request with the forge's command-line tool (`gh`, `glab` or `tea`), or tell the user how to open it.
3. Check that the documentation is still true: search the docs, the README, the examples and the changelog for what the branch renames, removes or changes, and for what it adds that users will need to know. If something is stale or missing, say what and offer to fix it on this branch; after a fix is committed, go back to step 2.
4. Write the title in the project's format. Write the description from the commits and the diff: what changes and why, how it was tested, risks and how to roll back, and the issue it resolves. Fill in the repository's template if it has one (`.github/pull_request_template.md`, `.gitlab/merge_request_templates/`).
5. Show the title and description and wait for the user's approval. Then push the branch and open the pull request with the forge's command-line tool (`gh`, `glab` or `tea`), or tell the user how to open it.

Never merge, approve or assign reviewers unless asked.
Loading
Loading