inspect_ai

# AGENTS.md

This file provides guidance to AI coding agents when working with code in this repository.

## Contribution policy — required checks before opening a PR

These rules are mandatory for AI coding agents (Claude Code, Codex, Cursor,
Devin, and similar) preparing contributions. Human contributors: see
[CONTRIBUTING.md](CONTRIBUTING.md). A deterministic gate enforces them.

1. **Accepted-issue check.** Is the account you are operating for on the
   qualified roster (recorded by account id in `.github/qualified.yml`)? If
   not, every non-trivial PR requires a linked issue labeled `accepted` by a
   maintainer. This applies regardless of the account's merged-PR history and
   regardless of who filed the issue — an issue filed by the same account the
   PR comes from is fine once a maintainer accepts it, and an issue filed by
   anyone else still needs the label. Without an accepted linked issue, your
   PR will be closed automatically. Do not open it: file your evidence on an
   issue and stop; open the PR only after a maintainer accepts the issue.
2. **Deferred check.** Is the issue you are addressing labeled `deferred`?
   The project has decided not to prioritize it. Do not open a PR against it
   (it will be closed automatically). If you have new evidence or demand,
   comment on the issue and stop.
3. **Duplicate check.** Search open PRs (and the linked issue's existing
   PRs) for work addressing the same problem. If a PR already exists, do not
   open a competing one — review it or comment there instead. If you believe
   your approach is materially better, make that case in a comment on the
   existing PR and wait for a maintainer's direction; if invited to proceed,
   link the two PRs in your description.
4. **Trivial-fix exception.** Documentation-only fixes (typo, broken link;
   docs files only, under 25 changed lines) may be opened directly by anyone.
5. **New functionality defaults to an extension, not core.** Do not open
   unrequested PRs adding functionality — providers, tools, scorers, metrics,
   solvers, storage backends, example evals. Some of these do belong in core,
   but that is a maintainer decision made in an issue: if an accepted issue
   calls for it in core, build it there. Otherwise the path is a separate
   extension package (see https://inspect.aisi.org.uk/extensions.html),
   optionally with a one-line PR adding it to the extensions listing.
   Unrequested additions to core are closed without detailed review.
6. **Value re-evaluation.** Before opening any PR, objectively re-assess it:
   does it fix a demonstrated problem, with evidence (a reproduction or a
   failing test)? If the need is speculative or the fix unverified, do not
   proceed. File an issue with your evidence instead.

If you do open a PR: reference the accepted issue (`Fixes #NNN`); run
`make check` and `make test` and report results honestly; for non-trivial
changes, run at least one code review pass in a fresh context on a strong
(frontier-class) model (see "Authoring pull requests" below); disclose
agent involvement in the PR description; one issue per PR — no bundled
drive-by changes.

As part of disclosing agent involvement, fill in the `### Agent review`
section in the PR description. What to disclose, and how it's read:

- **Disclose**: what model/tool reviewed, whether the review ran in a fresh
  context and/or on a different model from the author, how many passes, and
  the findings — issues found, which were fixed, and which were dismissed
  with a one-line reason each.
- **How it's read**: maintainers weight the disclosed reviewer model and
  pass count when deciding how much independent review a PR still needs.
  Multiple passes, each in a fresh context, often catch issues a single
  pass misses — prefer that for non-trivial changes.
- **Honesty**: if no review pass was run, say so explicitly (disclosure is
  not a substitute for running one where required above). Never report a
  review that didn't happen — a fabricated or content-free review claim
  ("reviewed, looks good") is worse than disclosing none.

Keep the disclosure focused on outcomes. For multiple passes, use a short
bullet per round with its focus, findings, and disposition; give a one-line
reason for each dismissed finding. Link detailed review notes if needed, and
do not repeat validation results or re-explain earlier findings in each round.

Example:

```
### Agent review
- Reviewer: Claude Opus 5 via /code-review (fresh context), 2 passes
- Findings: 3 — 2 fixed, 1 dismissed (flagged a missing None check that is
  guarded upstream)
```

## Build/Lint/Test Commands
- Run all tests: `pytest`
- Run a single test: `pytest tests/path/to/test_file.py::test_function_name -v`
- Format code: `ruff format`
- Lint code: `ruff check --fix`
- Type check: `mypy --exclude tests/test_package src tests`
- If a lint or type check fails on lines your diff didn't touch, check whether `main` has the same failure before debugging your change (run the same check on a clean `main` checkout, or look at the latest CI run on `main`) — a new toolchain release (e.g. a mypy major) can turn `main` red with no code change. If `main` is also failing, don't bundle an unrelated fix into your PR; flag it (in the PR description or an issue) and let it be fixed separately.

## Code Style Guidelines
- **Formatting**: Follow Google style convention. Use ruff for formatting
- **Imports**: Use isort order (enforced by ruff)
- **Types**: Strict typing is required. All functions must have type annotations
- **Typed returns**: When a function returns multiple values, prefer a `NamedTuple` (or small dataclass) over a bare tuple. Adjacent same-typed slots (and `bool`/`int` adjacency) make positional mistakes invisible to the type checker; named fields keep construction sites keyword-checked and give call sites self-documenting attribute access.
- **Naming**: Use snake_case for variables, functions, methods; PascalCase for classes
- **Docstrings**: Google-style docstrings required for public APIs
- **Comments at call sites**: Don't describe what a function does at the call site — the function's name and docstring already document that, and the comment will drift if the function evolves. Document rationale in the function's docstring instead. A call-site comment is appropriate only when the *reason this caller specifically invokes it* isn't obvious from surrounding context (eg. an unusual ordering constraint, a workaround for a known bug in this code path). When in doubt, write the docstring and leave the call site uncommented.
- **Comment length**: Sometimes comments in the code are useful to explain the rationale or context of a particular set of code. When this is necessary, be concise. Preserve the important concept and information but don't be pedantic or overly verbose. Especially avoid just replaying a commit description, PR description, or text used elsewhere into a comment.
- **Error Handling**: Use appropriate exception types; include context in error messages
- **Structured error contracts**: When a command supports machine-readable output (e.g. a `--json` flag with a documented error envelope), every terminal failure path must emit the structured error shape — route new error sites through the subsystem's failure helper (e.g. `_fail` in `src/inspect_ai/_cli/ctl/_failure.py`) rather than exiting directly (a bare `click.exceptions.Exit` bypasses the envelope, leaving `--json` consumers stderr prose and an empty stdout). When adding such a contract, add a mechanical guard that catches bypasses (see `test_no_bare_click_exit_in_ctl_error_sites`).
- **Public contracts**: Before changing a public event, log record, result type, or serialized model, describe the semantics you're changing and name every producer and consumer it touches — transcript/log readers, dataframes, hooks, replay code, sibling packages. Add compatibility or round-trip coverage for persisted data and replay paths the change affects.
- **Invalid state**: Don't silently coerce invalid input, an error, or incompatible persisted state into a different result — a permissive default, a swallowed parse failure, a masked provider error. Define the externally observable error or unsupported-state behavior explicitly, and cover it with a test.
- **Testing**: Write tests with pytest; maintain high coverage. See "Testing Async Code" below for async test conventions. Prefer adding tests to an existing test file covering the same area (e.g. eval-level behavior → `tests/test_eval.py`) rather than creating a new file; only add a new file when no existing one is a reasonable fit.

- **Async Concurrency**: Use `inspect_ai._util._async.tg_collect()` instead of `asyncio.gather()` for running concurrent async tasks. Use `inspect_ai.util.collect()` only inside sample subtasks (it adds transcript span grouping).

- **⚠️ Never `to_thread` (or run in a sync FastAPI `def` endpoint) `fsspec` work on a *remote* filesystem** (s3/gcs/azure — i.e. `filesystem(path).is_async()`). Their sync API runs on fsspec's own background event-loop thread, and nesting our threadpool over it can deadlock; use `inspect_ai._util.asyncfiles.AsyncFilesystem` instead. `to_thread` is fine when the path is known-local (`LocalFileSystem` is plain sync, no background loop) or fsspec-free (e.g. local `sqlite`).

- **No speculative locks**: Don't add `threading.Lock` around module-level state by default. Inspect runs on a single event loop thread (control-server handlers included — uvicorn runs as a task on the eval's loop), so async code doesn't interleave between `await` points, and a single dict/list operation is atomic under the GIL regardless. Add a lock only for demonstrated cross-thread access or a multi-operation invariant, and say in a comment/docstring what it protects; conversely, when omitting one where a reader might expect it, note why it isn't needed.

- **File Paths**: All code that handles file paths must support `s3://` URLs, `file://` URIs, and plain local paths. Use `filesystem()` from `inspect_ai._util.file` for filesystem operations and `local_path()` to resolve `file://` URIs to local paths before passing to APIs that only accept local paths (e.g. `ZipFile`).

- **Respect existing patterns**: Respect existing code patterns when modifying files. Run linting before committing changes.

## Writing for users and reviewers

Use concise, direct language in PR descriptions, documentation, changelog
entries, review notes, error messages, and code comments. State the behavior,
evidence, and any action a reader must take plainly. Use established project
terms; do not invent a term when an existing one is accurate, and define a
new term when it is needed. Avoid metaphors, slogans, rhetorical questions,
and decorative prose. Keep the details needed to assess compatibility,
limitations, and validation.

## Suppression gate

- Do NOT suppress lint or type errors (`# noqa`, `# type: ignore`,
  `# pyright: ignore`, or the file-wide `# ruff: noqa` /
  `# mypy: ignore-errors`). Fix the code. A deterministic gate enforces
  this (`make suppressions-check` against `suppressions.json`); maintainers
  reject suppressions that just make an error go away.
- In the rare case a suppression is correct, it requires both: a reason in
  a trailing hash comment segment on the same line, the only style mypy
  accepts (e.g. `x = f()  # type: ignore[assignment]  # stub is wrong
  upstream`), and `make suppressions-update` to record it in
  `suppressions.json`. Always suppress the specific code
  (`# noqa: E501`, `# type: ignore[assignment]`), never the bare code-less
  form.
- Every new suppression requires human maintainer approval; when it changes
  aggregate counts, that approval includes the `suppressions.json` diff.
  Expect the PR to be blocked until then, and say in the PR description why
  no fix is possible.
- The ledger tracks aggregate counts by file and rule, not individual source
  locations. Moving or replacing the same rule within one file does not alter
  the ledger, so reviewers must still inspect suppression changes in the
  source diff.
- If the `suppressions` CI check fails, never hand-edit the ledger to make
  it pass. Run `make suppressions-update` so the change shows in the
  ledger diff. `--update` refuses to grow any rule's repo-wide reason-less
  total (the ratchet): new suppressions must carry a reason, and the
  baselined reason-less ones burn down over time.
- Merges from upstream are the one sanctioned ratchet exception: when a
  sync brings in new reason-less suppressions, do not edit the
  upstream-owned lines to add reasons (that creates permanent merge
  drift). Instead run `python3 .github/scripts/check_suppressions.py
  --update --allow-growth` to record them; it prints each rule that grew,
  and the growth still shows in the ledger diff for maintainer review.

## Testing Async Code

All async test functions automatically run under both asyncio and trio backends via anyio (applied by the `pytest_pycollect_makeitem` hook in `tests/conftest.py`). Trio variants are skipped by default; use `--runtrio` to enable them.

- **Do NOT use `@pytest.mark.asyncio`** — it conflicts with anyio and is blocked by conftest. Just write `async def test_...` and the hook handles the rest.
- **Use `anyio.sleep()` not `asyncio.sleep()`** in tests; `anyio.Event()` not `asyncio.Event()`; `tg_collect()` not `asyncio.gather()`.
- **Don't synchronize concurrent test tasks with a sleep** (e.g. sleeping so a sibling sample "has time to start") — it races on a loaded runner. Have the task being waited on set an `anyio.Event` and await that.
- **Use `@skip_if_trio`** (from `test_helpers.utils`) for tests that cannot run under trio (e.g. they test asyncio-specific fallback paths).
- **`@pytest.mark.anyio`** is not required but harmless — use it to signal intentional dual-backend coverage.
- **Cancellation and ownership**: for an async change, run the affected
  tests with `--runtrio` before opening a PR rather than relying on the
  asyncio-only default. Add a focused test that cancels or fails mid-await,
  not just the success path, and assert that cleanup still runs. A resource
  that crosses an async boundary (a bridge, a spawned process, a background
  task) is owned by whichever component created it — cleanup belongs there,
  not in a caller or bridge that merely passes it through.

## Gated tests (slow, api, flaky, trio)

Plain `pytest` skips the Docker-based slow tests, the live model-provider
tests, flaky tests, and trio variants. PR CI runs only the slow tests under
`tests/tools/` and a short list of other areas; it never runs the live
provider tests, because they need API keys. So a PR that changes model
providers, sandbox or tool code, agents, or async plumbing must run the gated
tests that cover the change locally, and the PR description must report the
run. The `slow-tests` skill (`.agents/skills/slow-tests/SKILL.md`) says which
flags and directories go with which change and what each class needs.

Report the run in a `### Slow tests` section after `### Validation`. Use one
short bullet per run: test class/provider, version when relevant, exact
command, and passed, failed, and skipped counts. End with a "Not run" bullet
naming each relevant class or provider you could not run and why (for
example, no API key, Docker daemon, model access, or local server).

A test that skipped did not run. Say so rather than counting it. If you ran
nothing, say that, so a maintainer with the keys or Docker runs the tests
before merge.

Link detailed logs instead of narrating individual requests, models, or test
assertions. In `### Validation`, give only the conclusion from these runs;
keep their commands and counts here.

## Subsystem Documentation

Additional files provide context when working in specific areas:

- [Sandbox tools: build process, container injection, RPC communication, design patterns](src/inspect_sandbox_tools/AGENTS.md)
- When adding or changing code that creates, adopts or trusts a directory
  inside a sandbox for Inspect's own use (tools, services, checkpoints,
  agent installs), start from the
  [sandbox trust model](design/sandbox-trust-model.md): its principals,
  boundaries and guarantees, and the verified-directory and verify-before-use
  contracts implemented by the
  [host directory helper](src/inspect_ai/util/_sandbox/_framework_directory.py)
  and the [injected `inspect_sandbox_tools` helper](src/inspect_sandbox_tools/src/inspect_sandbox_tools/_util/server_dir.py),
  including their ownership, ancestor and verification-before-use requirements.
  Update the document in the same PR when a change alters a principal,
  boundary or contract it describes.

## Design Documentation

`design/` contains architecture notes, subsystem internals, and documentation of repo/CI/development processes and workflows. Browse it before diving into an unfamiliar area.

## Authoring pull requests

These conventions apply to every PR, whoever authors it. External contributions must also satisfy the contribution policy above.

Write the PR description using the template at `.github/pull_request_template.md`.
Lead with the user-facing problem and outcome; for a long description, keep
those first two sections short and put detailed design or review notes below
the required sections. Link the issue or give a reproduction when applicable.
Fill in compatibility and migration before validation, so reviewers can assess
whether the tests cover the risks. Lead with the action required of existing
users or downstream packages, or say "No migration required." Then describe
only material effects on existing behavior, public APIs, CLI behavior,
configuration, provider and extension interfaces, events, logs, or persisted
formats, grouped by affected user or integration. Include behavior changes
without signature or schema changes. Explain the compatibility boundary and,
for a public contract, name the affected producers and consumers. Summarize
unchanged behavior briefly rather than listing every unaffected path. Move
implementation-only protocol mechanics, test names, project sequencing, and
unrelated out-of-scope work to the result, validation, or a linked design
document. Keep limitations that affect existing users in this section.
A bare "No" is insufficient when a public contract or persisted data changes.

For code changes, run focused tests for the changed behavior and relevant
neighboring paths, as well as `make check` and `make test`. Follow "Gated
tests" above and the `slow-tests` skill for applicable slow, live-provider,
Docker, and Trio runs. Write `### Validation` as a short decision summary:
lead with the evidence that proves the change or checks compatibility beyond
routine CI, such as a before/after reproduction, another SDK version, or a
live provider run. For non-gated local runs CI cannot show, give the command,
environment, and passed, failed, and skipped counts. Routine green CI is
visible on the PR; omit it from the description. Report CI failures and local
failures that differ from CI, with a brief cause and baseline comparison.
State what remains untested and why. Keep exact gated commands and counts in
`### Slow tests`; give only their conclusion in `### Validation`. Use a table
only when its entries are short; put long commands in bullets or linked notes.
Report results for the current PR head; when the branch changes, refresh
results affected by the change rather than leaving historical runs to appear
current. If an agent worked on the PR, fill in `### Agent review` as described
above. Human-only PRs may omit that section. Keep the `### Slow tests` section
for changes in gated-test areas, as described above.

Title the PR with the user-facing outcome — the bug a user hit or the capability they gain — not the mechanism of the fix: "Fix eval hang when resuming with S3 logs", not "Add AsyncFilesystem to log recorder". A good test: would a user scanning titles recognize their problem or their feature request? PRs with no user-facing outcome (refactoring, dev tooling, docs) describe the change itself instead. CHANGELOG entries follow the same outcome-not-mechanism rule; only product-functionality changes get one (see below), so the carve-out doesn't arise there.

Before opening a non-trivial PR, run at least one code review pass in a fresh context — a reviewer that hasn't seen the authoring conversation (e.g. `/code-review` or a subagent) — using a strong (frontier-class) model; in our experience reviews from small fast-tier models rarely surface real issues. Fix or explicitly dismiss each finding before opening, and disclose the pass in the `### Agent review` section.

When asked to open a PR, don't stop at creation — monitor it afterward: watch its CI checks (e.g. `gh pr checks <number> --repo <owner>/<repo> --watch`) until they complete, report the outcome, and investigate/fix any failures. If the branch has fallen behind its base (out of date), update it — merge or rebase the base branch in and push — so CI runs against current code.

For changes to product functionality (not test-only or build-only changes), add a CHANGELOG entry: a single-line, single-sentence item in the `## Unreleased` section at the top of `CHANGELOG.md` (create that section if it doesn't exist), grouped with similar existing items when there are any, otherwise appended to the list. Keep it short (~25 words): state only the user-visible behavior change — what a user can now do or observe — not the mechanism, internal names, or design rationale (those belong in the PR description and `design/` docs). For example: "Fixed sample buffer database growing unboundedly during long evals", not "Add periodic vacuum to buffer SQLite db". Don't reference issue numbers in the entry (e.g. `(#123)` or `(owner/repo#123)`) — the PR description carries the issue link. A merge from the base can silently relocate the entry under a released heading — the merge resolves cleanly (the entry rides along with neighboring lines that the release commit moved), so nothing flags it. Verify placement mechanically after updating a branch against its base, and again before merging any PR — even when someone else updated the branch (e.g. via GitHub's "Update branch" button): run `git diff "$(git merge-base origin/main HEAD)" HEAD -- CHANGELOG.md` and confirm every added entry line sits under `## Unreleased`; move back any that don't.

Never change a submodule gitlink (e.g. `src/inspect_ai/_view/ts-mono`) unless the task is about that submodule. After any merge/rebase, check `git status`; if it shows the submodule modified, reset the pointer to the base and commit: `git checkout origin/main -- src/inspect_ai/_view/ts-mono`. (`git submodule update` will NOT fix this — it syncs the working tree to the already-recorded pointer, not the reverse.) When a change legitimately requires a coordinated ts-mono update (e.g. regenerated types), follow `.agents/skills/land-ts-mono/SKILL.md`.

### Opening an upstream PR from an org fork

Please open upstream PRs from a personal fork rather than an organization
fork, and leave "Allow edits by maintainers" enabled when creating the PR
(it's on by default in the GitHub UI and `gh pr create`). That setting gives
upstream maintainers push access to the PR branch, and they need it: upstream
`main` requires PR branches to be up to date before merging, so every push to
`main` blocks open PRs until their branches are updated. With maintainer
edits enabled, a maintainer can update the branch (or push small fixes) and
merge on their own schedule. GitHub does not support maintainer edits on
organization-owned forks, so a PR from an org fork can only be kept mergeable
by its author — it will sit approved but unmergeable whenever `main` moves
until the author syncs the branch again.

If you must open from an org fork, the following applies.

Before opening the PR, sync the branch with upstream `main` (`git fetch origin main && git merge origin/main`, resolving any conflicts) and push. Otherwise the PR can open with conflicts against its base, and CHANGELOG entries in particular almost always conflict.

Opening a PR from an organization fork to its upstream via the GitHub API/CLI needs an explicit `head_repo` — without it GitHub can't resolve the org fork as the PR head and rejects it with `{"field":"head","code":"invalid"}` (an org fork requires `head_repo`; a personal fork resolves without it). `gh pr create` has no `--head-repo` flag ([cli/cli#6462](https://github.com/cli/cli/issues/6462)), so use `gh api` — e.g. from the `meridianlabs-ai/inspect_ai` fork to upstream `UKGovernmentBEIS/inspect_ai`:

```bash
gh api repos/UKGovernmentBEIS/inspect_ai/pulls -X POST \
  -f title="<title>" -f base="main" \
  -f head="<branch>" -f head_repo="meridianlabs-ai/inspect_ai" \
  -F body=@<body.md>
```

Once the upstream PR is open it's the system of record: close the corresponding org-fork PR, with a close comment linking to the upstream PR.

## Repository skills

Shared skills live in `.agents/skills`. `.claude/skills` links to that directory. Use `$skill-name` in Codex or `/skill-name` in Claude Code.

More agent context in UKGovernmentBEIS/inspect_ai

5 other files this repository gives its agents.