mirror of
https://github.com/deepset-ai/haystack.git
synced 2026-10-12 02:12:50 +00:00
docs: extend AGENTS.md with conventions mined from PR reviews (#12352)
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
df306d3e44
commit
ad04d23925
@@ -59,3 +59,70 @@ Every user-facing PR (not docs, not CI) must include a release note:
|
||||
hatch run release-note SHORT_DESCRIPTION
|
||||
|
||||
Edit the generated file in `releasenotes/notes/`. Release notes use reStructuredText formatting; see the [release notes section in CONTRIBUTING.md](CONTRIBUTING.md#release-notes) for details.
|
||||
|
||||
In addition, there are rules inferred from previous code reviews. Follow them like a
|
||||
reviewer's note: they encode reasons, not ceremony, so when a rule genuinely does not
|
||||
fit the change at hand, say why rather than contorting the code to satisfy it.
|
||||
|
||||
Also see directory-specific guidelines:
|
||||
|
||||
- [docs-website/AGENTS.md](docs-website/AGENTS.md)
|
||||
- [docs-website/docs/AGENTS.md](docs-website/docs/AGENTS.md)
|
||||
- [haystack/hooks/compaction/AGENTS.md](haystack/hooks/compaction/AGENTS.md)
|
||||
- [releasenotes/notes/AGENTS.md](releasenotes/notes/AGENTS.md)
|
||||
- [test/AGENTS.md](test/AGENTS.md)
|
||||
|
||||
## API Design
|
||||
|
||||
- Target the 3.0 API: `ToolInvoker`, `AsyncPipeline`, and non-chat generators were removed — let `Agent` own tool calls, use `Pipeline.run()`/`Pipeline.run_async()`, and use ChatGenerators like `OpenAIChatGenerator`
|
||||
- Serialize with the Haystack helpers (`component_to_dict`, `default_from_dict`): in `from_dict()` pass only the fields present to `__init__()` so defaults stay centralized, shallow-copy the payload instead of `deepcopy()`, and keep tools/toolsets nested under `data` — preserves wire compatibility and round-trip fidelity
|
||||
- Keep public signatures explicit and compatible: optional params keyword-only after `*` and appended rather than inserted, mirrored constructor params spelled out instead of `*args`/`**kwargs`, and chat generators keeping the `run(messages, *, streaming_callback, generation_kwargs, tools)` order — avoids breaking positional callers
|
||||
- Create API clients and load models in `warm_up()`, not `__init__`; implement `warm_up()` only for real setup and make it idempotent — keeps components constructible and serializable without credentials or network
|
||||
- Implement `run_async` only when there is a real async execution path — `Pipeline.run_async()` already falls back to `asyncio.to_thread(component.run, ...)` when it is missing; where it exists, keep `run_async`/`warm_up_async`/`close_async` genuinely async with separate async state and hooks, and share sync/async logic through private helpers — prevents event-loop blocking and sync/async drift
|
||||
- Pass explicit `required_variables` for optional prompt vars — avoids requiring optional inputs
|
||||
- Subclass `Toolset` only when inherited collection APIs match — prefer composition or raise `NotImplementedError`
|
||||
- Read live Haystack `State` resources via `state.data.get(...)` or `state.data[...]` — avoids deep-copy bugs
|
||||
- Prefer existing `haystack/core/pipeline` APIs or inline logic — avoid duplicate or one-off public APIs
|
||||
- Reuse splitter chunk metadata in `haystack/components/preprocessors/` — keep traceability fields like `page_number`, `source_id`, `header`, `parent_headers`, and split positions; add new keys only for existing downstream contracts
|
||||
|
||||
## Documentation
|
||||
|
||||
- Keep inline comments and private-helper docs to what is non-obvious — remove restatements, keep durable caveats and rationale
|
||||
- Keep docstrings current with signatures and behavior, in the existing Haystack style: each public `:param` by meaning, default, and constraints; `:returns:` contracts; exceptions in the existing `:raises ValueError:` style; aliases like `ToolsType` reflected — stale docs mislead users and assistants
|
||||
- Keep doc examples minimal, runnable, and local: default constructors with required env vars like `OPENAI_API_KEY` noted nearby, only the imports the snippet uses, no restated defaults (name a model only for model-specific behavior), expected output as comments, generally `# >> ...`
|
||||
- When behavior, fields, or names change, update every surface in the same PR: `haystack/components/` docstrings and examples, `docs-website/docs/` plus the current `versioned_docs/version-*/` page (e.g. `concepts/data-classes.mdx`), and `experimental` wording, `pydoc` IDs, and generated markdown filenames when promoting features
|
||||
|
||||
## Code Style
|
||||
|
||||
- Use keyword args for multi-parameter calls — improves readability and prevents mix-ups
|
||||
- Keep lint suppressions exact and current — a `# noqa: PLR0915` only while the code needs it
|
||||
- Scope diffs to the stated goal and leave them clean: no unrelated refactors or formatting, no stray `print()`, no filename-only header comments, no unused private helpers or redundant branches, no args that only restate callee defaults
|
||||
- Use `{placeholder}` logger templates with kwargs — preserves structured logs and avoids eager formatting
|
||||
- Prefix internal helpers and module-level constants with `_` and keep private constants local; never name locals after imported functions, decorators, or utilities — clarifies API boundaries and avoids shadowing
|
||||
|
||||
## Type System
|
||||
|
||||
- Fix types at the source; where a suppression is unavoidable use a coded `# type: ignore[...]` with a nearby why-safe comment, and add `@overload` only for real API variants
|
||||
- Resolve callable annotations with `typing.get_type_hints()` — avoids bugs from raw or string `inspect.signature()` annotations
|
||||
- Keep union aliases non-redundant and concept-named — include base types only when subclasses inherit
|
||||
- Use `T | None`, not `Optional[T]`, in annotations and casts — keeps typing concise
|
||||
- Annotate helper params broadly and accurately — `type | types.UnionType` for class-or-PEP-604 inputs, `Any` for arbitrary typing objects
|
||||
|
||||
## Imports
|
||||
|
||||
- Import via public APIs in the narrowest clear form (keep module imports like `import httpx` when namespaces matter); keep only used imports plus deliberate compatibility re-exports
|
||||
- Use top-level absolute `haystack...` imports in `haystack/components/` and `haystack/core/component/*.py` — keeps component dependencies traceable
|
||||
- Use `LazyImport` only for optional third-party deps — otherwise import directly
|
||||
|
||||
## Config
|
||||
|
||||
- Pass `allowed_modules=` or set `HAYSTACK_DESERIALIZATION_ALLOWLIST` for YAML loads — never widen `haystack/core/serialization_security.py` allowlists in library code
|
||||
|
||||
## Testing
|
||||
|
||||
- Centralize `DocumentStore` tests in `haystack/testing/document_store.py` mixins — use `DocumentStoreBaseTests` as the minimum suite and compose capability mixins into `DocumentStoreBaseExtendedTests`; add explicit integration skips only for unsupported backend features.
|
||||
|
||||
## General
|
||||
|
||||
- Define package exports in `__init__.py` via `__all__`; avoid `__all__` in implementation modules — keeps public APIs intentional
|
||||
- Update `pyproject.toml` for new package APIs — declare deps and minimum versions used
|
||||
|
||||
@@ -1,3 +1 @@
|
||||
# CLAUDE.md
|
||||
|
||||
Before you start working on this repository, read the AGENTS.md file and follow all the instructions.
|
||||
@AGENTS.md
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
# docs-website/ Guidelines
|
||||
|
||||
## Documentation
|
||||
|
||||
- Keep `docs-website` API names, import paths, and links current with public exports — link data-class symbols to anchored API docs and verify every MDX link (internal routes and external URLs) resolves
|
||||
- Omit explicit `.warm_up()` in docs unless required — lazy/idempotent warm-up handles it
|
||||
- Keep setup/usage for maintained APIs and integrations in `docs-website/docs/` — it is the authoritative, navigable source; add pages to `docs-website/sidebars.js` when needed
|
||||
@@ -0,0 +1 @@
|
||||
@AGENTS.md
|
||||
@@ -0,0 +1,9 @@
|
||||
# docs-website/docs/ Guidelines
|
||||
|
||||
## Documentation
|
||||
|
||||
- Edit `docs-website/docs/pipeline-components/` pages only for outdated, incorrect, or materially useful guidance; keep examples concise and `Agent`-level and polish prose before merge — avoids churn while keeping docs copyable
|
||||
- Add `## Overview` near the top of `docs-website/docs/pipeline-components/**` pages — explains what the component does and why to use it before details
|
||||
- Document component outputs, extractor side effects, and exact `doc.meta` keys; link producers, API references, and the authoritative reference for any partial config summary
|
||||
- Use current chat APIs in new docs pipelines — `ChatPromptBuilder`, `ChatMessage`, chat generators, and `result["last_message"]` for agent output — matching wiring, edge names like `prompt`, and declared variables; keep YAML examples on current default model names
|
||||
- Mark joiners/adapters optional where smart pipeline connections already handle the composition
|
||||
@@ -0,0 +1 @@
|
||||
@AGENTS.md
|
||||
@@ -0,0 +1,10 @@
|
||||
# haystack/hooks/compaction/ Guidelines
|
||||
|
||||
## API Design
|
||||
|
||||
- Use provider-compatible roles in `haystack/hooks/compaction/` — prefer `user` for synthetic markers
|
||||
- Name and document compaction settings by their real semantics — retention in `turns`/`steps`, not `messages`; state when compaction runs, what is retained, and which tool-call context is preserved
|
||||
|
||||
## General
|
||||
|
||||
- Keep `haystack/hooks/compaction/` compactors narrative — move shared indexing, grouping, token counting, and helpers into focused utils
|
||||
@@ -0,0 +1 @@
|
||||
@AGENTS.md
|
||||
@@ -0,0 +1,8 @@
|
||||
# releasenotes/notes/ Guidelines
|
||||
|
||||
## Documentation
|
||||
|
||||
- Add `upgrade` notes for breaking/user-visible changes in `releasenotes/notes/` — explain affected users, old/new behavior, and migration steps
|
||||
- Write one concise, user-facing note file per PR, only for in-scope changes and only under sections from `releasenotes/config.yaml`; name the affected APIs/configs and the old and new behavior, describe impact in user terms rather than private helpers, and leave unrelated note files untouched
|
||||
- Highlight APIs in `releasenotes/notes/` only with examples or clear use cases — shows practical value
|
||||
- Check the PR's own release note against the shipped behavior — API names, and reStructuredText formatting with double backticks for inline code; leave existing notes alone
|
||||
@@ -0,0 +1 @@
|
||||
@AGENTS.md
|
||||
@@ -0,0 +1,18 @@
|
||||
# test/ Guidelines
|
||||
|
||||
## Testing
|
||||
|
||||
- Name tests after the behavior they verify and group them by behavior in the existing file mirroring the source layout (`test/components/<group>/test_{component}.py`); keep the suite minimal — one smoke/contract test for brittle live paths such as chat generators, preprocessors, and `test_agent.py` — and split a file only when it becomes hard to navigate
|
||||
- Keep test imports at module scope and share `pytest` fixtures/helpers/constants only for truly common setup — prevents brittle coupling
|
||||
- Test serialization via public round trips — use `component_to_dict`/`component_from_dict`, `to_dict()`/`from_dict()`, or constructors; avoid hardcoded deep dicts
|
||||
- In splitter tests use real multi-chunk fixtures and assert exact ordered content, metadata, and per-source `split_id`s; assert the joined split content equals the input text; check `split_idx_start`/`_split_overlap` as character ranges into the original text for each `split_unit`
|
||||
- Assert exception messages with `pytest.raises(..., match=...)`; fully match one related invalid case — catches user-visible error regressions while keeping tests readable
|
||||
- Cover `haystack/core/pipeline/` changes in the matching pipeline test module with end-to-end edge cases (shorthands, early returns, errors, empty inputs, outputs); add regression tests for socket metadata around `Variadic`/`GreedyVariadic`; assert fan-in order from runtime semantics and use joiners when a custom order matters
|
||||
- Pair sync `run` integration tests with `run_async` tests — keep async behavior covered
|
||||
- Skip OpenAI integration tests without `OPENAI_API_KEY`; use dummy keys for non-live tests — keeps local and CI runs credential-free unless testing live
|
||||
- Test PEP 604 unions (`X | Y`, `X | None`) with `typing.Union`/`Optional` — catches annotation-compat bugs
|
||||
- Assert full dict shapes in `test/components/generators/chat/` — catches schema regressions
|
||||
|
||||
## General
|
||||
|
||||
- Avoid `# type: ignore` in tests; narrow with `hasattr(...)` or `assert isinstance(...)` first — exposes real API mismatches
|
||||
@@ -0,0 +1 @@
|
||||
@AGENTS.md
|
||||
Reference in New Issue
Block a user