From b9a08132dc09e67c1924aeaefe4a72eb40c3f971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=98en=20Fylling?= Date: Fri, 8 May 2026 10:40:31 +0200 Subject: [PATCH] docs: add CLAUDE.md and Claude Code skill for new sub-API authoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two pieces of authored context for AI assistants (Claude Code in particular, but the markdown is readable by any agent that loads files): - CLAUDE.md at repo root — repo-wide conventions, the 10 hard rules (sync codegen, doctest registration, no dict[str, Any], read/write split, APIClient helpers, @overload, FeaturePreviewWarning, etc.) and BAD/GOOD anti-pattern pairs from PR #2534 review feedback. - .claude/skills/cognite-sdk-python-add-resource/ — an opt-in Claude Code skill triggered when adding a new resource group / sub-API. 16-step boilerplate checklist, decision tree (flat vs nested, alpha vs GA, chunking), and per-topic references covering the streams PR walkthrough, DTO design, wiring, alpha warnings, tests/doctests. Also gitignores .claude/settings.local.json (Claude Code per-user state). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../cognite-sdk-python-add-resource/SKILL.md | 189 ++++++++++++++++++ .../references/alpha-and-warnings.md | 150 ++++++++++++++ .../references/dto-design.md | 149 ++++++++++++++ .../references/streams-walkthrough.md | 157 +++++++++++++++ .../references/tests-and-doctests.md | 179 +++++++++++++++++ .../references/wiring.md | 177 ++++++++++++++++ .gitignore | 3 + CLAUDE.md | 75 +++++++ 8 files changed, 1079 insertions(+) create mode 100644 .claude/skills/cognite-sdk-python-add-resource/SKILL.md create mode 100644 .claude/skills/cognite-sdk-python-add-resource/references/alpha-and-warnings.md create mode 100644 .claude/skills/cognite-sdk-python-add-resource/references/dto-design.md create mode 100644 .claude/skills/cognite-sdk-python-add-resource/references/streams-walkthrough.md create mode 100644 .claude/skills/cognite-sdk-python-add-resource/references/tests-and-doctests.md create mode 100644 .claude/skills/cognite-sdk-python-add-resource/references/wiring.md create mode 100644 CLAUDE.md diff --git a/.claude/skills/cognite-sdk-python-add-resource/SKILL.md b/.claude/skills/cognite-sdk-python-add-resource/SKILL.md new file mode 100644 index 0000000000..863f326f38 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/SKILL.md @@ -0,0 +1,189 @@ +--- +name: cognite-sdk-python-add-resource +description: Use when adding a new sub-API / resource group / endpoint coverage to cognite-sdk-python — e.g. "wrap the X API", "add support for the Y endpoints", "how do I add Z to this SDK", "what files do I need to touch for a new API". Triggers on any work that creates new files under cognite/client/_api/ or cognite/client/data_classes/, or that mentions patterns specific to new APIs (`_create_multiple`, `_RESOURCE_PATH`, `WriteableCogniteResource`, `FeaturePreviewWarning`, sync codegen, doctest registration). Does NOT trigger on routine bug fixes / refactors that don't introduce a new resource. Even when the user only says "I want to add streams-style endpoints", this skill should fire. +--- + +# Adding a new sub-API to cognite-sdk-python + +This skill walks you through introducing a new resource group (sub-API) to the SDK. The streams PR (#2534) is the worked example throughout — when in doubt, open `cognite/client/_api/data_modeling/streams.py` and copy the shape. + +## When this skill applies + +Use this skill if you're: +- Creating a new file under `cognite/client/_api/` or `cognite/client/_api//` for a CDF resource the SDK doesn't yet wrap +- Adding the parallel DTOs under `cognite/client/data_classes/[/].py` +- Wiring a new API onto `AsyncCogniteClient` (or onto a parent group like `DataModelingAPI`) + +If you're only modifying an existing API, skip this skill. + +## Decision-tree opener — answer these THREE before writing any code + +### 1. Flat or nested? + +- **Flat** (`cognite/client/_api/.py`) — feature stands alone at `client.`. Examples: `assets.py`, `events.py`, `documents.py`. Wired in `cognite/client/_cognite_client.py` `__init__`. +- **Nested** (`cognite/client/_api//.py`) — feature is a sub-resource at `client..`. Examples: everything under `data_modeling/`, `agents/`, `ai/`, `simulators/`. Wired in the group's own `__init__.py`. + +Pick **nested** if a parent group already exists and the feature belongs to it. Streams was originally placed at `cognite/client/_api/streams/__init__.py` and got relocated to `cognite/client/_api/data_modeling/streams.py` during review — costing a relocation churn. Decide before authoring. + +The flat/nested choice mirrors across `cognite/client/data_classes/`, `cognite/client/_sync_api/`, `tests/tests_unit/test_api/`, and `docs/source/`. + +### 2. Alpha/beta or GA? + +Drives whether you instantiate `FeaturePreviewWarning`: + +- **GA API + GA SDK** → no preview warning (don't spam users on stable features). +- **Anything else** → instantiate `FeaturePreviewWarning(api_maturity=..., sdk_maturity=..., feature_name="...")` in `__init__` and call `self._warning.warn()` as the **first line of every public method**. `api_maturity` and `sdk_maturity` are independent dials. + - Streams: `api_maturity="General Availability", sdk_maturity="alpha"` (API is GA, SDK shape may evolve). + - Agents: `api_maturity="beta", sdk_maturity="alpha"` plus `self._api_subversion = "beta"` for beta-route routing. + +See `references/alpha-and-warnings.md` for details. + +### 3. Does the create/delete endpoint chunk? + +If the server accepts only one item per request (or some other small N): + +- **GOOD:** set `self._CREATE_LIMIT = N` and/or `self._DELETE_LIMIT = N` in `__init__`. +- **BAD:** hand-rolling `if len(items) > 1: raise ValueError(...)` — `_create_multiple` / `_delete_multiple` chunk for you. +- **BAD:** dropping the `Sequence[X]` overload — pass-one/get-one and pass-list/get-list ergonomics stay. +- Streams sets both to `1`. Public signature is unchanged from the multi-item case. + +If the create/delete are **not idempotent at the server**, also register the path in `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` (`cognite/client/utils/_url.py`). + +--- + +## The 16-step boilerplate checklist + +Tags: `[U]` universal, `[C]` conditional, `[O]` optional. Each step names the file(s) to edit and points at the streams reference. + +1. **[U] Decide layout.** Flat vs nested (see above). Streams chose nested under `data_modeling/`. + +2. **[U] Author the DTOs first** at `cognite/client/data_classes/[/].py`. Subclass `CogniteResource` / `WriteableCogniteResource[]` / `CogniteResourceList[]` from `cognite.client.data_classes._base`. Implement `_load`; override `dump` only when nesting needs recursion. Add `as_write()` on the read class. Reference: `cognite/client/data_classes/data_modeling/streams.py`. Deep dive: `references/dto-design.md`. + +3. **[U] Re-export DTOs** in the group-level `__init__.py` (alphabetical in both the import block and `__all__`). For nested: `cognite/client/data_classes//__init__.py`. For flat: `cognite/client/data_classes/__init__.py`. Reference: `cognite/client/data_classes/data_modeling/__init__.py:122-132,257-265`. Mypy's `no_implicit_reexport=true` means namespace import alone is not enough. + +4. **[U] Implement the async API class** at `cognite/client/_api/[/].py`. Subclass `APIClient`. Set `_RESOURCE_PATH`. Use `_create_multiple` / `_list` / `_retrieve` / `_delete_multiple` — **do not hand-roll HTTP**. Reference: `cognite/client/_api/data_modeling/streams.py`. The helper signatures are in `cognite/client/_api_client.py`. + +5. **[C] Set `_CREATE_LIMIT` / `_DELETE_LIMIT` / `_LIST_LIMIT`** in `__init__` *only when* the endpoint chunks. Streams sets both `_CREATE_LIMIT = 1` and `_DELETE_LIMIT = 1`. + - **BAD:** writing `if len(items) > 1: raise ValueError(...)` — `_create_multiple` chunks for you. + - **BAD:** dropping the `Sequence[X]` overload — keep the singular/plural pair (step 4 still applies). + - **GOOD:** just set the limit. Public signature, overloads, and body stay identical to the multi-item case. + +6. **[C] Add `FeaturePreviewWarning`** if API or SDK is not GA-stable. Instantiate in `__init__` (`self._warning = FeaturePreviewWarning(...)`); call `self._warning.warn()` as the first line of every public method body. See `references/alpha-and-warnings.md`. + +7. **[U] Wire the new API class into the parent group's `__init__.py`** (nested) — e.g. add `self.streams = StreamsAPI(config, api_version, cognite_client)` to `DataModelingAPI.__init__`. For flat: register on `AsyncCogniteClient.__init__` in `cognite/client/_cognite_client.py`. Reference: `cognite/client/_api/data_modeling/__init__.py:31`. Walkthrough: `references/wiring.md`. + +8. **[U] Add the docs-time import** in `cognite/client/_cognite_client.py`. There's a `BUILD_COGNITE_SDK_DOCS` block (~lines 45-104) that imports nested-API classes so Sphinx can resolve `AsyncCogniteClient..`. Add `from cognite.client._api.. import API` (alphabetical neighbours). Without this, `make html` under `-W --keep-going` fails. Reference line: 55. See `references/wiring.md`. + +9. **[U] Register mocks in `cognite/client/testing.py`** for both the async (`API`) and the sync (`SyncAPI`) class. Add ` = create_autospec(API, instance=True, spec_set=True)` and pass it as a kwarg into the parent group's `create_autospec(...)` so consumer unit tests pick up the new attribute. Streams pattern at lines 23, 107, 229, 240, 430, 441. **Skipping this step does not fail any lint** — but downstream `CogniteClientMock` users will hit `AttributeError`. See `references/wiring.md`. + +10. **[C] Add the resource path to `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS`** in `cognite/client/utils/_url.py` *only when* the create/delete endpoints are not idempotent server-side. Streams adds `"streams"` (line 37). Also append a parametric entry in `tests/tests_unit/test_api_client.py` (the retry-classification list around lines 1841-1843). + +11. **[U] Write unit tests** at `tests/tests_unit/test_api/[/]test_.py` using `pytest_httpx`. Use the `cognite_client` (sync) and `async_client` fixtures from `tests/tests_unit/conftest.py`. Layered fixture pattern: `make__response` (factory) → `_response` (single) → `_list_response` (wrapper) → `_base_url` (derived from live client). Reference: `tests/tests_unit/test_api/test_data_modeling/test_streams.py`. Cover create/list/retrieve/delete + chunking proofs (when `_CREATE_LIMIT=1`/`_DELETE_LIMIT=1`). **Don't write load/dump round-trip tests** — there's an existing parametric harness keyed off `_RESOURCE`. See `references/tests-and-doctests.md`. + +12. **[U] Register doctests** in `tests/tests_unit/test_docstring_examples.py` — extend the import block AND append `run_docstring_tests()` to the appropriate `TestDocstringExamples.test_` method. **Forgetting this was an explicit fix-up commit in PR #2534** (`987b7d1`). Without it your `>>>` examples never run. See `references/tests-and-doctests.md`. + +13. **[U] Add docs** in `docs/source/.rst` (nested) or `docs/source/cognite.rst` / a new top-level rst (flat). Add a `.. autosummary::` block for the API methods + an `.. automodule::` for the data classes. Match neighbour heading levels (`-` top, `^` nested). Reference: `docs/source/data_modeling.rst:289-302`. + +14. **[U] Run sync codegen** from the repo root: + ```bash + python scripts/sync_client_codegen/main.py run --files cognite/client/_api/[/].py + ``` + Commit the generated `cognite/client/_sync_api/[/].py` (and the regenerated `_sync_api//__init__.py`). Pre-commit will run this hook. **Never hand-edit anything under `_sync_api/`.** Run codegen *last*, after the async signature is stable, to avoid repeated regen+lint commits — PR #2534 ate three commits on this loop. + +15. **[U] Pre-flight before opening PR** — see Pre-flight section below. + +16. **[U] Open PR with a Conventional Commit title** (`feat(): …`). + +--- + +## Iteration order — what to do first vs last + +PR #2534 went through three review rounds; most of the churn was avoidable. Distilled lessons: + +1. **Author the DTOs first**, with the maximally-typed shape. No `dict[str, Any]`, no `Any`, use `Literal[...]` for closed enums. This avoids the round-1 typing rewrite. +2. **Implement the API class using helpers from the start.** Set `_CREATE_LIMIT`/`_DELETE_LIMIT` if applicable. Don't write any HTTP yourself. +3. **Write the docstrings (with `>>>` Examples blocks) before the tests.** Forces every `Args:`/`Returns:`/`Examples:` block to exist. No restating the class header; one-line class docstring. +4. **Wire docs (rst) + the docs-time import in `_cognite_client.py` + the doctest registration together.** Three small, easily-missed steps. PR #2534 forgot two of them and burned fix-up commits `1e6937c` ("fix sphinx doc generation") and `987b7d1` ("ensure we run tests on docstrings") on merge day. +5. **Wire mocks early in `testing.py`** (both async and sync) so unit tests that mock the SDK work from day one. +6. **Write the unit tests** — chunking proofs, doctest-friendly fixture names, plus the `test_api_client.py` retry entry. +7. **Run sync codegen LAST.** Once the async signature is stable. PR #2534 burned `5a52db6` → `1e39c1b` → `b25a668` (limit change → regen → fix unused imports) because codegen ran mid-iteration. +8. **Add `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` if applicable** + the matching `test_api_client.py` entry. +9. **Run pre-flight locally.** Open PR with Conventional Commit title. + +The maintainer's pushback concentrates on (a) helper usage, (b) typing strictness, (c) docstring discipline + working `>>>` examples. Address these before requesting review. + +--- + +## Pre-flight checklist + +Run all of these locally before opening the PR: + +```bash +# What CI's lint job runs: +pre-commit run --all-files + +# Type-check explicitly (also runs in pre-commit): +poetry run dmypy run -- cognite tests + +# Fast unit tests: +poetry run pytest tests/tests_unit -n4 --dist loadscope + +# Doctests in your new module specifically: +poetry run pytest tests/tests_unit/test_docstring_examples.py -k -v + +# Sphinx warnings-as-errors build: +cd docs && make html SPHINXOPTS="-W --keep-going" + +# Sync codegen explicit verify: +python scripts/sync_client_codegen/main.py verify +``` + +Item-by-item smoke: +- [ ] `_RESOURCE_PATH` set on the API class. +- [ ] `_CREATE_LIMIT` / `_DELETE_LIMIT` set if endpoint chunks. +- [ ] `FeaturePreviewWarning` instantiated **and** `.warn()` called as the first line of every public method (if alpha/beta). +- [ ] Every public method has Args/Returns/Examples docstring with at least one working `>>>` block. +- [ ] No `dict[str, Any]` in public signatures or DTO `__init__` args. +- [ ] No `Any` returns; `Literal[...]` for closed enums. +- [ ] No internal codenames (e.g. project codenames) anywhere user-facing. +- [ ] `_load` implemented on every DTO; `dump` overridden only when nested CogniteResources need recursion. +- [ ] Read/write split: `` inherits `WriteableCogniteResource[Write]`; both implement `as_write()`. Pure read-only API → inherit `CogniteResource` directly and skip the write half. +- [ ] Re-exports in the data-classes `__init__.py` and `__all__` (alphabetical). +- [ ] Parent group `__init__.py` (nested) or `_cognite_client.py` (flat) instantiates the new API. +- [ ] Docs-time import added in the `BUILD_COGNITE_SDK_DOCS` block of `_cognite_client.py`. +- [ ] Mocks registered in `testing.py` for async **and** sync (twice — once per *Mock class). +- [ ] Sync mirror regenerated and committed (no manual edits). +- [ ] Doctest registered in `test_docstring_examples.py`. +- [ ] Unit tests covering create/list/retrieve/delete + chunking proofs (if applicable). +- [ ] `test_api_client.py` parametric retry list updated. +- [ ] `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` updated if applicable. +- [ ] `docs/source/.rst` adds API + data-classes section. +- [ ] PR title is Conventional Commit (`feat(): …`). +- [ ] **Don't** edit `CHANGELOG.md`. +- [ ] PR description matches the merged code (update as design evolves). + + +--- + +## Canonical reference: streams (PR #2534) + +When you want to see a real, recently-merged example end-to-end: + +- API class: `cognite/client/_api/data_modeling/streams.py` (~160 lines) +- DTOs: `cognite/client/data_classes/data_modeling/streams.py` (~217 lines) +- Sync mirror (auto-generated, do not edit): `cognite/client/_sync_api/data_modeling/streams.py` +- Tests: `tests/tests_unit/test_api/test_data_modeling/test_streams.py` (~149 lines) +- Wiring touchpoints: `cognite/client/_api/data_modeling/__init__.py:12,31` ; `cognite/client/_cognite_client.py:55` ; `cognite/client/testing.py:23,107,229,240,430,441` ; `cognite/client/data_classes/data_modeling/__init__.py:122-132,257-265` ; `cognite/client/utils/_url.py:37` ; `tests/tests_unit/test_docstring_examples.py:39,132` ; `docs/source/data_modeling.rst:289-302` ; `tests/tests_unit/test_api_client.py:1841-1843` + +For a guided tour of how all those pieces fit together, see `references/streams-walkthrough.md`. + +--- + +## References (load on demand) + +- `references/dto-design.md` — DTO authoring deep dive: ``/`Write`/`List` triple, `_load`/`dump` rules, `as_write()`, settings-as-typed-class pattern, when to nest. +- `references/wiring.md` — exhaustive walkthrough of the five wiring locations. +- `references/tests-and-doctests.md` — fixture pattern, `pytest_httpx`, doctest registration. +- `references/alpha-and-warnings.md` — `FeaturePreviewWarning`, beta routing, `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS`. +- `references/streams-walkthrough.md` — guided tour of the streams PR files. + diff --git a/.claude/skills/cognite-sdk-python-add-resource/references/alpha-and-warnings.md b/.claude/skills/cognite-sdk-python-add-resource/references/alpha-and-warnings.md new file mode 100644 index 0000000000..5627735575 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/references/alpha-and-warnings.md @@ -0,0 +1,150 @@ +# Alpha/beta gating, FeaturePreviewWarning, and retry classification + +## `FeaturePreviewWarning` + +Source: `cognite/client/utils/_experimental.py:7-43`. Subclass of `FutureWarning`, picklable (overrides `__reduce__` because `APIClient` instances get pickled across xdist workers). + +```python +class FeaturePreviewWarning(FutureWarning): + def __init__( + self, + api_maturity: Literal["alpha", "beta", "General Availability"], + sdk_maturity: Literal["alpha", "beta"], + feature_name: str, + pluralize: bool = False, + ): ... + def warn(self) -> None: ... # emits via warnings.warn(self, stacklevel=2) +``` + +`pytest.ini` filters this warning class to `ignore`, so raising one is free in tests. The warning is real at runtime and shows up to end users. + +## Two independent dials: `api_maturity` and `sdk_maturity` + +They are deliberately decoupled. + +| `api_maturity` | Meaning | +| --- | --- | +| `"alpha"` | Backend itself unstable; expect endpoint-shape changes. | +| `"beta"` | Backend stable-ish, will get a `DeprecationWarning` before breakages. | +| `"General Availability"` | Backend is GA. | + +| `sdk_maturity` | Meaning | +| --- | --- | +| `"alpha"` | Python interface itself may change — we want freedom to redesign DTOs/method signatures. | +| `"beta"` | Python interface mostly stable. | + +### Streams: GA API + alpha SDK + +```python +self._warning = FeaturePreviewWarning( + api_maturity="General Availability", + sdk_maturity="alpha", + feature_name="Streams", +) +``` + +Suggested in round-2 review (`reviews_and_comments.md` L92): "lets add a 'SDK in alpha' warning to the StreamsAPI itself to give ourselves some freedom in changing/modifying the SDK API design for some time." Adopted verbatim. + +### Agents: beta API + alpha SDK + beta routing + +`cognite/client/_api/agents/agents.py:28-29`: + +```python +class AgentsAPI(APIClient): + def __init__(self, ...) -> None: + super().__init__(...) + self._api_subversion = "beta" # routes requests to /api/beta/... + self._warnings = FeaturePreviewWarning( + api_maturity="beta", sdk_maturity="alpha", feature_name="Agent", + ) +``` + +Note: `_api_subversion = "beta"` is what makes the SDK hit the beta route prefix. Set it when the endpoint lives at `/api/beta/...`. + +## Where to instantiate + +In the API class `__init__`, after `super().__init__(...)`, alongside any `_CREATE_LIMIT`/`_DELETE_LIMIT`: + +```python +def __init__(self, config: ClientConfig, api_version: str | None, cognite_client: AsyncCogniteClient) -> None: + super().__init__(config, api_version, cognite_client) + self._CREATE_LIMIT = 1 + self._DELETE_LIMIT = 1 + self._warning = FeaturePreviewWarning( + api_maturity="General Availability", sdk_maturity="alpha", feature_name="Streams" + ) +``` + +Attribute name: streams uses `_warning`, agents uses `_warnings` (plural). **Pick one and stay consistent within your file.** The name is local; nothing depends on the spelling. + +## Where to call `.warn()` + +**First line of every public method body.** Not in `__init__`. Not in helpers. + +```python +async def list(self) -> StreamList: + """...""" + self._warning.warn() + return await self._list(method="GET", list_cls=StreamList, resource_cls=Stream) +``` + +Every public-facing entry point — `create`, `list`, `retrieve`, `delete`, `update`, etc. — should warn once per call. Internal helpers (anything with a leading underscore) don't. + +## When NOT to use FeaturePreviewWarning + +GA SDK + GA API features: don't add it. Mature features (`assets.py`, `events.py`, `time_series.py`) have no preview warning. Adding one to a stable feature spams users for no reason. + +## `feature_name` + +The public-facing display name: `"Streams"`, not `"streams"`. `"Agent"` (singular), not `"AgentsAPI"`. Set `pluralize=True` only if the warning text reads better with the plural form ("Streams are in alpha" vs "Stream is in alpha"). + +--- + +## Retry classification — `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` + +File: `cognite/client/utils/_url.py`. + +Add the resource path (the `_RESOURCE_PATH` minus the leading slash) to `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` only when **create/delete on the server are not idempotent** — i.e. retrying the same POST may produce duplicates or fail. Streams adds `"streams"` (line 37): + +```python +NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS: tuple[str, ...] = ( + "annotations", + "assets", + ... + "streams", + ... +) +``` + +The matching regex (`NON_IDEMPOTENT_POST_ENDPOINT_REGEX_PATTERN`, lines 51-64) uses `^/(/delete)?(\?.*)?$`. The `(\?.*)?$` was added in commit `11ce01c` so that `_list`'s injected `?limit=...` query string still classifies correctly. + +### When in doubt — leave it off + +Default is "retryable". The only time you absolutely need to register here is when **the server cannot tolerate duplicate POSTs**. The author's review comment claimed streams was added "since we now use `_list`", but the actual reason is server-side non-idempotency on POST. + +### Test entry + +If you add a path here, also add a parametric entry in `tests/tests_unit/test_api_client.py` (around lines 1841-1843): + +```python +("POST", "https://api.cognitedata.com/api/v1/projects/bla/", False), +("POST", "https://api.cognitedata.com/api/v1/projects/bla//delete", False), +``` + +`False` = not retryable. The matching test exercises the regex against the actual URL shape `_list` produces (with the optional `?limit=` suffix). + +## Beta API routing + +If your endpoint lives at `/api/beta/...` rather than `/api/v1/...`: + +```python +class API(APIClient): + _RESOURCE_PATH = "/" + + def __init__(self, ...) -> None: + super().__init__(...) + self._api_subversion = "beta" + ... +``` + +`_api_subversion = "beta"` switches the URL prefix from `/api/v1/projects//` to `/api/beta/projects//`. See `AgentsAPI` for the live pattern. diff --git a/.claude/skills/cognite-sdk-python-add-resource/references/dto-design.md b/.claude/skills/cognite-sdk-python-add-resource/references/dto-design.md new file mode 100644 index 0000000000..c354d85f78 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/references/dto-design.md @@ -0,0 +1,149 @@ +# DTO design deep dive + +Authoritative example: `cognite/client/data_classes/data_modeling/streams.py`. +Base classes live in `cognite/client/data_classes/_base.py`. + +This file focuses on **DTO authoring patterns specific to a new sub-API**. + +## The `` / `Write` / `List` triple + +For any resource with a write path: + +```python +class Stream(WriteableCogniteResource["StreamWrite"]): + """One line. Args/Returns/Examples carry the load.""" + def __init__( + self, + external_id: str, + created_time: int, + created_from_template: str, + type: Literal["Immutable", "Mutable"], + settings: StreamSettings, + ) -> None: + self.external_id = external_id + ... + + @classmethod + def _load(cls, resource: dict[str, Any]) -> Self: + return cls( + external_id=resource["externalId"], + created_time=resource["createdTime"], + created_from_template=resource["createdFromTemplate"], + type=resource["type"], + settings=StreamSettings._load(resource["settings"]), + ) + + def as_write(self) -> StreamWrite: + return StreamWrite( + external_id=self.external_id, + settings=StreamTemplateWriteSettings( + template=StreamTemplate(name=self.created_from_template), + ), + ) + + +class StreamWrite(WriteableCogniteResource["StreamWrite"]): + """One line.""" + def __init__(self, external_id: str, settings: StreamTemplateWriteSettings) -> None: + self.external_id = external_id + self.settings = settings + + @classmethod + def _load(cls, resource: dict[str, Any]) -> Self: ... + + def as_write(self) -> StreamWrite: + return self + + +class StreamList(CogniteResourceList[Stream], ExternalIDTransformerMixin): + _RESOURCE = Stream +``` + +Key points: + +- The **read class** parameterises `WriteableCogniteResource[""]` and its `as_write()` builds a fresh `` from its own fields. +- The **write class** also parameterises `WriteableCogniteResource[""]` and its `as_write()` returns `self`. This is the standard pattern. +- The **list class** subclasses `CogniteResourceList[]` and sets `_RESOURCE = `. Mix in `ExternalIDTransformerMixin` (`_base.py:841`) when items have `external_id` so users can call `list.as_external_ids()`. +- Use `WriteableCogniteResourceList` (`_base.py:455`) only when the list itself participates in a read/write pair (rare; streams does NOT). +- **Pure read-only API**: skip the write half entirely; inherit `CogniteResource` directly and don't add `as_write()`. + +## Naming + +`` (read), `Write` (write payload), `List` (list container). Older areas use `Apply` (`SpaceApply`) or `Upsert` (`AgentUpsert`); for new code prefer `Write`. + +No internal codenames. PR #2534 had to scrub internal product codenames from public docstrings before merge. **Never expose internal product names anywhere user-facing.** + +## `_load` pattern + +Every DTO implements `_load`. Required fields: `resource["…"]`. Optional fields: `resource.get("…")`. Nested CogniteResources: `._load(resource["…"])`. `Optional[Nested]`: `._load_if(resource.get("…"))` (provided by `_base.py:134-137`). + +```python +@classmethod +def _load(cls, resource: dict[str, Any]) -> Self: + return cls( + external_id=resource["externalId"], + consumed=resource.get("consumed"), + settings=StreamSettings._load(resource["settings"]), + meta=Meta._load_if(resource.get("meta")), + ) +``` + +## When to override `dump` + +Default `dump()` (`_base.py:88-97`) calls `basic_instance_dump`, which handles primitives via `convert_all_keys_to_camel_case`. **Override only when the resource holds nested `CogniteResource` children** — the inherited dump won't recursively call `.dump()` on them. + +Streams overrides `dump` on `StreamLimitSettings`, `StreamSettings`, `StreamTemplateWriteSettings`, `Stream`, `StreamWrite` (each has nested `CogniteResource` children). It does **not** override on `StreamLimit`, `StreamLifecycleSettings`, `StreamTemplate` (primitives only). + +Round-1 review: "dump with primitive types is redundant" — three of four overrides got deleted. **Don't add `dump` overrides you don't need.** + +Pattern when overriding: + +```python +def dump(self, camel_case: bool = True) -> dict[str, Any]: + out: dict[str, Any] = { + "external_id": self.external_id, + "settings": self.settings.dump(camel_case=camel_case), + } + return convert_all_keys_to_camel_case(out) if camel_case else out +``` + +## Settings/template pattern (the streams headline lesson) + +PR #2534 originally shipped `StreamWrite.settings: dict[str, Any]`. Review caught this and commit `934d0e1` removed the dict overload. Final shape uses **typed classes all the way down**: + +```python +class StreamWrite(WriteableCogniteResource["StreamWrite"]): + def __init__(self, external_id: str, settings: StreamTemplateWriteSettings) -> None: ... + +class StreamTemplateWriteSettings(CogniteResource): + def __init__(self, template: StreamTemplate) -> None: ... + +class StreamTemplate(CogniteResource): + def __init__(self, name: str) -> None: ... +``` + +Each layer is its own `CogniteResource`. **Mandatory under `.gemini/styleguide.md` — no `dict[str, Any]` in public signatures.** When tempted to "just take a dict for now", don't. + +## When to nest DTOs vs. flatten + +If the API spec describes a sub-object with two-or-more fields, model it as a nested `CogniteResource` (named ``, e.g. `StreamLimitSettings`). If it's a single-field wrapper that's truly opaque (rare, and watch for spec drift), still model it — the round-trip cost is tiny and your users get autocomplete. + +Review caught spec drift on `StreamTemplate.version` not being in the spec → field removed (commits `2c50de3`, `8cee723`). **Verify your fields match the spec; don't invent.** + +## `Literal[...]` for closed enums + +`Stream.type: Literal["Immutable", "Mutable"]` (added in round 2 review). Plain `str` was rejected. If the API has a closed set, type it as `Literal[...]`. + +## `__eq__` / `__str__` / `_repr_html_` + +`CogniteResource` already provides these (`_base.py:76-190`). Don't override unless you have a domain reason. + +## Common pitfalls (what review will catch) + +- `dict[str, Any]` in any public signature → **rewrite as a typed class**. +- `Any` return type → **rewrite**. +- `MutableSequence` where `Sequence` would do → **use `Sequence`**. +- `str` where `Literal[...]` would do → **use `Literal[...]`**. +- A `DeleteItem` class for an endpoint that just takes external IDs → **drop it; `delete(external_id: str | SequenceNotStr[str])` is enough**. +- Bespoke utility functions in the API module → **move to `cognite/client/utils/...`** if truly needed; usually they're not. +- Class-level docstring that just restates the class name → **one informational line; load goes in `Args:`/`Returns:`/`Examples:`**. diff --git a/.claude/skills/cognite-sdk-python-add-resource/references/streams-walkthrough.md b/.claude/skills/cognite-sdk-python-add-resource/references/streams-walkthrough.md new file mode 100644 index 0000000000..617c0d2117 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/references/streams-walkthrough.md @@ -0,0 +1,157 @@ +# Streams PR (#2534) — guided file tour + +A complete, recently-merged sub-API to read end-to-end. Start at the top, work down. All paths absolute. + +## Source surface (the files you'll author for any new sub-API) + +### `cognite/client/_api/data_modeling/streams.py` + +The async API class. ~160 lines covering create / list / retrieve / delete with full Google-style docstrings. Read this first — it's the cleanest small example in the repo. + +Things to notice: +- `_RESOURCE_PATH = "/streams"` is the only required class attr. +- `_CREATE_LIMIT = 1` / `_DELETE_LIMIT = 1` set in `__init__` because the endpoint accepts one item per request. +- `FeaturePreviewWarning(api_maturity="General Availability", sdk_maturity="alpha", feature_name="Streams")` — GA backend, alpha SDK shape. +- Every public method calls `self._warning.warn()` as the first line. +- `create` uses the `@overload` pattern for singular/plural returns. +- `list`, `retrieve`, `create`, `delete` each delegate to a single helper call — no hand-rolled HTTP. +- `retrieve` shows the `params={"includeStatistics": include_statistics} if include_statistics is not None else None` gating pattern for query params. +- `delete` accepts `external_id: str | SequenceNotStr[str]` — no bespoke `DeleteItem` class. + +### `cognite/client/data_classes/data_modeling/streams.py` + +The DTOs. ~217 lines defining nine public classes: +- `Stream` (read), `StreamWrite` (write), `StreamList` (list container) — the canonical triple. +- `StreamSettings`, `StreamLifecycleSettings`, `StreamLimitSettings`, `StreamLimit`, `StreamTemplate`, `StreamTemplateWriteSettings` — typed nested DTOs (no `dict[str, Any]`). + +Patterns to copy: +- `class Stream(WriteableCogniteResource["StreamWrite"])` + `as_write()` returning a fresh `StreamWrite`. +- `class StreamWrite(WriteableCogniteResource["StreamWrite"])` + `as_write()` returning `self`. +- `class StreamList(CogniteResourceList[Stream], ExternalIDTransformerMixin)` with `_RESOURCE = Stream`. +- Every DTO implements `_load`. Only those with nested `CogniteResource` children override `dump`. +- `Stream.type: Literal["Immutable", "Mutable"]` for closed enums. + +### `tests/tests_unit/test_api/test_data_modeling/test_streams.py` + +The unit tests. ~149 lines using `pytest_httpx`. Layered fixture pattern (`make_stream_response`, `stream_response`, `stream_list_response`, `streams_base_url`). Five tests cover list / retrieve / create-single / create-chunked / delete-chunked. + +## Wiring touchpoints (the five edits) + +### `cognite/client/_api/data_modeling/__init__.py` + +Lines 12 and 31. Imports `StreamsAPI` and instantiates it on `DataModelingAPI`. + +### `cognite/client/_cognite_client.py` + +Line 55, inside the `if _should_build_docs := os.getenv("BUILD_COGNITE_SDK_DOCS") == "true":` block. Imports `StreamsAPI` for Sphinx autosummary resolution. + +### `cognite/client/testing.py` + +Six lines: +- 23: `from cognite.client._api.data_modeling.streams import StreamsAPI` +- 107: `from cognite.client._sync_api.data_modeling.streams import SyncStreamsAPI` +- 229: `dm_streams = create_autospec(StreamsAPI, instance=True, spec_set=True)` +- 240: `streams=dm_streams` kwarg passed into `DataModelingAPI`'s `create_autospec`. +- 430: sync mirror of 229. +- 441: sync mirror of 240. + +### `cognite/client/data_classes/data_modeling/__init__.py` + +Lines 122-132 (imports) and 257-265 (`__all__`). Nine names re-exported alphabetically. + +### `cognite/client/utils/_url.py` + +Line 37: `"streams"` added to `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS`. Plus the regex update at lines 51-64 (commit `11ce01c`) to match `?limit=` query strings. + +## Test wiring + +### `tests/tests_unit/test_docstring_examples.py` + +Line 39 (import) and line 132 (`run_docstring_tests(streams)` call inside `TestDocstringExamples.test_data_modeling`). **This was the missed step** caught by commit `987b7d1` ("ensure we run tests on docstrings"). + +### `tests/tests_unit/test_api_client.py` + +Lines 1841-1843. Two parametric entries asserting `/streams` and `/streams/delete` are non-retryable (`False`). + +## Docs wiring + +### `docs/source/data_modeling.rst` + +Lines 287-302. Two sections: + +```rst +.. currentmodule:: cognite.client + +Streams +------- +.. autosummary:: + :methods: + :toctree: generated/ + :template: custom-automethods-template.rst + + AsyncCogniteClient.data_modeling.streams + +Streams data classes +^^^^^^^^^^^^^^^^^^^^ +.. automodule:: cognite.client.data_classes.data_modeling.streams + :members: + :show-inheritance: +``` + +Heading levels matter: `------` for top-level (e.g. "Streams"), `^^^^^^` for nested (e.g. "Streams data classes"). Match neighbours. + +## Auto-generated artifacts (do not edit) + +### `cognite/client/_sync_api/data_modeling/streams.py` + +Generated by `scripts/sync_client_codegen/main.py`. First six lines contain a hash of the async source. Each method calls `run_sync(self.__async_client.data_modeling.streams.(...))`. + +### `cognite/client/_sync_api/data_modeling/__init__.py` + +Also auto-regenerated; new `SyncStreamsAPI` import + `self.streams = SyncStreamsAPI(async_client)` line appear automatically. + +## Commits that distill the lessons + +In chronological order, relevant commits from PR #2534: + +| Commit | What it taught | +| --- | --- | +| `5a52db6` | "Set chunking limits on async" — use `_CREATE_LIMIT=1`/`_DELETE_LIMIT=1` instead of hand-validating `len(items)==1`. | +| `1e39c1b` | "Regenerate sync after the limits change" — sync codegen is a regular cycle, not a one-shot. | +| `b25a668` | "Remove unused imports in sync data modeling API init" — F401 fallout from regen. Run codegen at the END of iteration. | +| `ec2959d` | "Address round-1 review feedback" — the round-1 consolidation commit; helpers, typing, docstrings. | +| `934d0e1` | "Remove support for dict type" — don't accept `dict[str, Any]` in public signatures, ever. | +| `2c50de3` / `8cee723` | Removed `StreamTemplate.version` (not in the API spec). Verify your fields against the spec. | +| `6027ab1` | Switched to `_list` even though streams has no `limit` param. Use the helper. | +| `11ce01c` | Updated the retry regex to tolerate `?limit=` from `_list`. Cross-cutting concern — be aware. | +| `1e6937c` | "Fix sphinx doc generation" — caught only by `make html SPHINXOPTS="-W --keep-going"` on merge day. Run docs locally. | +| `987b7d1` | "Ensure we run tests on docstrings" — registration in `test_docstring_examples.py` was forgotten. Don't forget. | + +## End-user usage post-merge + +```python +from cognite.client import CogniteClient +from cognite.client.data_classes.data_modeling.streams import ( + StreamWrite, StreamTemplate, StreamTemplateWriteSettings, +) + +client = CogniteClient() + +# Create +client.data_modeling.streams.create( + StreamWrite( + external_id="my-stream", + settings=StreamTemplateWriteSettings(template=StreamTemplate(name="ImmutableTestStream")), + ) +) + +# List (no paging — streams are expected to be few per project) +client.data_modeling.streams.list() + +# Retrieve, optionally with statistics +client.data_modeling.streams.retrieve("my-stream", include_statistics=True) + +# Delete (soft delete — capacity retained for an extended period) +client.data_modeling.streams.delete("my-stream") +client.data_modeling.streams.delete(["stream-a", "stream-b"]) # chunked, 1 per request +``` diff --git a/.claude/skills/cognite-sdk-python-add-resource/references/tests-and-doctests.md b/.claude/skills/cognite-sdk-python-add-resource/references/tests-and-doctests.md new file mode 100644 index 0000000000..e240cf6e07 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/references/tests-and-doctests.md @@ -0,0 +1,179 @@ +# Tests and doctests for a new sub-API + +Reference: `tests/tests_unit/test_api/test_data_modeling/test_streams.py` (~149 lines). + +This file focuses on **what to write for a new sub-API**. + +## Layered fixture pattern + +Round-2 review requested splitting mock responses into reusable fixtures rather than inline dicts. Streams's pattern — copy this shape: + +```python +import re +from collections.abc import Callable +import pytest +from pytest_httpx import HTTPXMock + +from cognite.client import AsyncCogniteClient, CogniteClient +from cognite.client.data_classes.data_modeling.streams import ( + Stream, StreamList, StreamWrite, ... +) +from tests.utils import jsgz_load + + +@pytest.fixture +def make_stream_response() -> Callable[..., dict]: + def _make(external_id: str = "st1", created_time: int = 10) -> dict: + return { + "externalId": external_id, + "createdTime": created_time, + "createdFromTemplate": "ImmutableTestStream", + "type": "Immutable", + "settings": {...}, + } + return _make + + +@pytest.fixture +def stream_response(make_stream_response: Callable[..., dict]) -> dict: + return make_stream_response() + + +@pytest.fixture +def stream_list_response(stream_response: dict) -> dict: + return {"items": [stream_response]} + + +@pytest.fixture +def streams_base_url(async_client: AsyncCogniteClient) -> str: + return async_client.data_modeling.streams._base_url_with_base_path + "/streams" +``` + +Three layers: +1. **`make__response`** — factory; takes overrides, returns a fresh dict. +2. **`_response`** — single-instance default, calls the factory. +3. **`_list_response`** — wraps the single in `{"items": [...]}`. + +Plus a **`_base_url`** fixture derived from the live async client. This keeps test URLs in sync with `_RESOURCE_PATH` — if you later change the path, tests don't need URL updates. + +## Top-level fixtures (already provided) + +`tests/tests_unit/conftest.py:35,53` defines: +- `cognite_client` (sync `CogniteClient`) +- `async_client` (`AsyncCogniteClient`) + +Both are class-scoped. `httpx_mock: HTTPXMock` comes from `pytest_httpx` (a dev dep already in `pyproject.toml`). + +The `tests/conftest.py` autouse `require_semaphore_on_every_request` fixture forces every internal HTTP call to use a semaphore — the helpers handle this; if you hand-rolled HTTP it'll fail loudly. To opt out: `@pytest.mark.allow_no_semaphore("")`. + +## What to test + +Streams's tests cover: + +1. **List parses items.** `httpx_mock.add_response(method="GET", url=..., json=stream_list_response)`, call `await async_client.data_modeling.streams.list()`, assert type is `StreamList` and at least one field on the item. +2. **Retrieve passes query params.** Assert `requests[0].url.params["includeStatistics"].lower() == "true"`. +3. **Create posts a single item.** Assert the wire payload via `jsgz_load(requests[0].content)` (helper at `tests/utils.py`). +4. **Create chunks ≥2 items.** Register two responses, assert `len(requests) == 2`, assert each request body matches one of the two inputs. Proves `_CREATE_LIMIT=1`. +5. **Delete chunks ≥2 items.** Same shape, hits `/streams/delete`. Proves `_DELETE_LIMIT=1`. + +URL matching tip — the `_list` helper injects `?limit=...` even when you don't expose it. Match flexibly: + +```python +import re +url_pattern = re.compile(re.escape(streams_base_url) + r"(?:\?.+)?$") +httpx_mock.add_response(method="GET", url=url_pattern, json=stream_list_response) +``` + +## What NOT to test + +**Don't** write load/dump round-trip tests for your DTOs. Round-2 maintainer review: + +> "I think most of these tests are already covered by our automatic testing, at least load/dump roundtrips of varying sorts, meaning most if not all of these tests can be removed." + +PR #2534 had a full `tests/.../test_data_models/test_streams.py` round-trip suite that got **deleted entirely**. There's an existing parametric harness keyed off `_RESOURCE` registries that exercises every `CogniteResource._load(...).dump() == original` round-trip. Look for it before duplicating. + +## Async vs sync — pick your fixture + +The streams tests use **async** (`async_client`) because async is the source of truth and easier to assert against `httpx_mock`. `pytest.ini` has `anyio_mode = auto` so `async def test_…` works without explicit decorators. + +If your scenario only makes sense via the sync path (e.g. testing the sync wrapper itself), use `cognite_client`. But if you can write the test once against async, do — sync is auto-generated and equivalent. + +## Retry-classification test entry + +Append your endpoint(s) to the parametric list in `tests/tests_unit/test_api_client.py` (around lines 1841-1843): + +```python +("POST", "https://api.cognitedata.com/api/v1/projects/bla/streams", False), +("POST", "https://api.cognitedata.com/api/v1/projects/bla/streams/delete", False), +``` + +`False` if the endpoint is in `NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS` (non-idempotent). `True` for retry-safe. + +## Doctest registration + +The trap caught by PR #2534 commit `987b7d1` ("ensure we run tests on docstrings"). File: `tests/tests_unit/test_docstring_examples.py`. + +### Nested API + +Extend the import block (lines 32-41) and the per-group test method: + +```python +# Top of file: +from cognite.client._api.data_modeling import ( + containers, + data_models, + graphql, + instances, + spaces, + statistics, + streams, # <-- add (alphabetical) + views, +) +... + +# Inside class TestDocstringExamples: +def test_data_modeling(self) -> None: + run_docstring_tests(containers) + ... + run_docstring_tests(streams) # <-- add +``` + +### Flat API + +Add an import to the top-of-file `from cognite.client._api import (...)` block (lines 12-31) and a new method on the class: + +```python +def test_(self) -> None: + run_docstring_tests() +``` + +The class is decorated with `@patch("cognite.client.CogniteClient", CogniteClientMock)`, so `>>>` examples that build a real `CogniteClient()` work via the mock. + +### Doctest content (in the API class docstrings) + +Every public method needs an `Examples:` section with at least one `>>>` block. Conventions (also enforced by `scripts/custom_checks/docstr_examples.py`): + +- One `from cognite.client import CogniteClient` per method's `Examples:` block. +- One `client = CogniteClient()` per method's `Examples:` block. +- `>>>` for executable lines, `...` for continuations. +- For async-only methods, hint `>>> # async_client = AsyncCogniteClient()` as a comment but doctest still drives the sync mock. + +See `cognite/client/_api/data_modeling/streams.py:47-67` for the canonical method-level `Examples:` block. + +If a docstring example needs a fixture (env var, credential, special mock shape), patch it in `test_docstring_examples.py` — see `test_credential_providers` for the decorator pattern. + +## Pre-flight commands for tests/doctests + +```bash +# Run only your new tests: +poetry run pytest tests/tests_unit/test_api/test_data_modeling/test_streams.py -v + +# Run only your doctests (filter by group): +poetry run pytest tests/tests_unit/test_docstring_examples.py -k data_modeling -v + +# Full unit suite (parallel): +poetry run pytest tests/tests_unit -n4 --dist loadscope + +# RST doctests (e.g. quickstart): +poetry run pytest docs/source/quickstart.rst +``` diff --git a/.claude/skills/cognite-sdk-python-add-resource/references/wiring.md b/.claude/skills/cognite-sdk-python-add-resource/references/wiring.md new file mode 100644 index 0000000000..cf27075272 --- /dev/null +++ b/.claude/skills/cognite-sdk-python-add-resource/references/wiring.md @@ -0,0 +1,177 @@ +# Wiring a new sub-API — the five locations + +Adding a new sub-API needs edits in five places. Skipping any one of them produces a different failure mode (some loud, some silent). Streams (PR #2534) hit all of them — concrete line references are inline below. + +## 1. Parent group's `__init__.py` (instantiation) + +### Nested API + +Edit `cognite/client/_api//__init__.py`. Streams (`cognite/client/_api/data_modeling/__init__.py`): + +```python +from cognite.client._api.data_modeling.streams import StreamsAPI # line 12 +... +class DataModelingAPI(APIClient): + def __init__(self, config: ClientConfig, api_version: str | None, cognite_client: AsyncCogniteClient) -> None: + super().__init__(config, api_version, cognite_client) + ... + self.streams = StreamsAPI(config, api_version, cognite_client) # line 31 +``` + +Imports go alphabetically; the assignment list inside `__init__` follows the same order. + +### Flat API + +Edit `cognite/client/_cognite_client.py` — the `__init__` of `AsyncCogniteClient` (~lines 126-164): + +```python +from cognite.client._api. import API +... +class AsyncCogniteClient: + def __init__(self, ...) -> None: + ... + self. = API(self._config, self._API_VERSION, self) +``` + +The sync sibling (`_sync_cognite_client.py`) is auto-generated — do not edit. + +## 2. Docs-time import block in `_cognite_client.py` + +Even for **nested** APIs, you also add the import inside the `BUILD_COGNITE_SDK_DOCS` block in `cognite/client/_cognite_client.py` (~lines 45-104): + +```python +if _should_build_docs := os.getenv("BUILD_COGNITE_SDK_DOCS") == "true": + ... + from cognite.client._api.data_modeling.streams import StreamsAPI # streams sits at line 55 + ... +``` + +Why: Sphinx (which sets `BUILD_COGNITE_SDK_DOCS=true`) needs the class symbol importable at the top level so `autosummary :: AsyncCogniteClient.data_modeling.streams` resolves. **Skipping this fails `make html SPHINXOPTS="-W --keep-going"`.** PR #2534 burned commit `1e6937c` ("fix sphinx doc generation") on this miss. + +For flat APIs, the import in step 1 is at module scope and Sphinx finds it; you usually don't need a second block entry — but copy the pattern of nearby flat imports if in doubt. + +## 3. Mocks in `cognite/client/testing.py` + +This file holds **two mirrored mock classes**: `AsyncCogniteClientMock` (~line 200) and `CogniteClientMock` (~line 410). Both need the new API for **nested** APIs. + +### Async mock (lines 23, 229, 240 for streams) + +```python +from cognite.client._api.data_modeling.streams import StreamsAPI # line 23 +... +class AsyncCogniteClientMock(MagicMock): + def __init__(self, *args, **kwargs): + ... + dm_streams = create_autospec(StreamsAPI, instance=True, spec_set=True) # line 229 + ... + self.data_modeling = create_autospec( + DataModelingAPI, + instance=True, + containers=dm_containers, + ... + streams=dm_streams, # line 240 + ) + flip_spec_set_on(self.data_modeling, dm_statistics) +``` + +### Sync mock (lines 107, 430, 441) + +```python +from cognite.client._sync_api.data_modeling.streams import SyncStreamsAPI # line 107 +... +class CogniteClientMock(MagicMock): + def __init__(self, *args, **kwargs): + ... + sync_dm_streams = create_autospec(SyncStreamsAPI, instance=True, spec_set=True) # line 430 + ... + self.data_modeling = create_autospec( + SyncDataModelingAPI, + ... + streams=sync_dm_streams, # line 441 + ) +``` + +### Failure mode + +**Skipping this step does not fail any obvious lint.** It just means consumer code that uses `CogniteClientMock` (e.g. `from cognite.client.testing import monkeypatch_cognite_client`) will hit `AttributeError` on `mock.data_modeling.streams`. Wire it early so your unit tests work end-to-end. + +For flat APIs, the registration is on the top-level `*Mock` class (not a child group). Look at how `AssetsAPI` is registered if in doubt. + +## 4. Re-export DTOs + +Nested: edit `cognite/client/data_classes//__init__.py`. Streams (`cognite/client/data_classes/data_modeling/__init__.py:122-132,257-265`): + +```python +# Imports — alphabetical, both across blocks and within the block. +from cognite.client.data_classes.data_modeling.streams import ( + Stream, + StreamLifecycleSettings, + StreamLimit, + StreamLimitSettings, + StreamList, + StreamSettings, + StreamTemplate, + StreamTemplateWriteSettings, + StreamWrite, +) + +# __all__ — alphabetical +__all__ = [ + ... + "Stream", + "StreamLifecycleSettings", + "StreamLimit", + "StreamLimitSettings", + "StreamList", + "StreamSettings", + "StreamTemplate", + "StreamTemplateWriteSettings", + "StreamWrite", + ... +] +``` + +Flat: same pattern in `cognite/client/data_classes/__init__.py`. + +`mypy.ini` sets `no_implicit_reexport = true`, so importing into the namespace alone is **not** sufficient — you must also append to `__all__`. Failure mode: downstream consumers writing `from cognite.client.data_classes.data_modeling import StreamWrite` get a mypy error. + +## 5. Sync codegen + +Async is the source of truth. Sync mirror is generated by `scripts/sync_client_codegen/main.py`. + +```bash +# Regenerate just your file: +python scripts/sync_client_codegen/main.py run --files cognite/client/_api/data_modeling/streams.py + +# Wholesale: +python scripts/sync_client_codegen/main.py run --all-files + +# CI verification mode (fails if any sync file is stale): +python scripts/sync_client_codegen/main.py verify +``` + +Generated outputs: +- `cognite/client/_sync_api//.py` — every method calls `run_sync(self.__async_client...(...))`. +- `cognite/client/_sync_api//__init__.py` — auto-regenerated to include the new `SyncAPI`. + +The first lines of each generated file include a hash of the async source (e.g. `cognite/client/_sync_api/data_modeling/streams.py:1-6`). If async drifts, the hash mismatches and `verify` fails. + +**Never hand-edit anything under `_sync_api/`.** If a sync file gets corrupt or stale, delete it (or scramble the hash) and rerun `run` — it'll regenerate clean. + +PR #2534 burned three commits on the regen loop because codegen was run too early: +- `5a52db6` — set chunking limits on async +- `1e39c1b` — regenerate sync after the limits change +- `b25a668` — clean up F401 unused-imports the regen left behind + +**Lesson: regenerate sync at the very end**, after the async signature has stabilised. If you're iterating during review, regen each round; the diff stays small. + +## Tables: file-level summary + +| Edit | Path | Why | +| --- | --- | --- | +| Instantiate | `_api//__init__.py` (nested) or `_cognite_client.py` (flat) | Wire the API onto the client at runtime. | +| Docs-time import | `_cognite_client.py` `BUILD_COGNITE_SDK_DOCS` block | Sphinx autosummary resolution. | +| Async mock | `testing.py` (`AsyncCogniteClientMock`) | Consumer test mocks. | +| Sync mock | `testing.py` (`CogniteClientMock`) | Consumer test mocks (sync flavor). | +| DTO re-exports | `data_classes//__init__.py` + `__all__` | Public namespace; `no_implicit_reexport=true`. | +| Sync codegen | (run script) → `_sync_api//.py` + `_sync_api//__init__.py` | Sync mirror; CI `verify` enforces. | diff --git a/.gitignore b/.gitignore index 1e1aed8e0e..d9db9836bf 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,6 @@ my_file.txt # docs docs/source/generated/ + +# Claude Code per-user settings +.claude/settings.local.json diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000..8b2ef736ff --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,75 @@ +# CLAUDE.md — `cognite-sdk-python` + +Repo-wide conventions, traps, and idioms. This file is auto-loaded into every session in this directory. + +For dev setup, the CI gauntlet, the release flow, and PR-title rules, read the canonical files directly: `CONTRIBUTING.md`, `.pre-commit-config.yaml`, `.github/workflows/build.yml`, `.github/workflows/conventional-pr-title.yml`, `release-please-config.json`, `.github/pull_request_template.md`. This file deliberately does not paraphrase them. + +## Mental model + +Async is the source of truth. Every real implementation lives under `cognite/client/_api//.py`; the primary client is `AsyncCogniteClient` (`cognite/client/_cognite_client.py`). The sync flavor under `cognite/client/_sync_api/...` (and the public `CogniteClient` in `cognite/client/_sync_cognite_client.py`) is **auto-generated** from the async sources by `scripts/sync_client_codegen/main.py`. Each generated file carries an md5-style hash banner — never hand-edit. Most users still import `from cognite.client import CogniteClient` and get the sync flavor, so both surfaces must read like a polished public API. + +## Hard rules + +1. **Sync mirror is generated.** After editing any `cognite/client/_api/...` file, run `python scripts/sync_client_codegen/main.py run --files ` (pre-commit also does it). Stage the regenerated `cognite/client/_sync_api/...` file in the same commit. CI runs `... verify` and fails on hash drift. If a sync file gets corrupt, scramble its hash banner or delete it and let `run` regenerate. + +2. **Every public method needs an `Examples:` doctest, AND the module needs to be registered.** Doctests for `cognite/client/...` modules don't auto-collect — they run from `tests/tests_unit/test_docstring_examples.py::TestDocstringExamples::test_` via `run_docstring_tests()`. New module → add the import at the top **and** the call in the matching `test_` method. Forgetting this is silent: PR #2534 commit `987b7d1` exists for exactly this trap. + +3. **No `dict[str, Any]` in public signatures.** Build typed DTOs. Use `Literal[...]` for closed enums (`type: Literal["Immutable", "Mutable"]`), `Sequence[X]` (not `MutableSequence`) in public params, and `cognite.client.utils.useful_types.SequenceNotStr` when you need "list of strings but reject a bare `str`". The canonical anti-example is `StreamWrite.settings: dict | StreamTemplateWriteSettings` — killed in PR #2534 commit `934d0e1`. + +4. **`` / `Write` / `List` naming and read/write split.** Read DTO subclasses `WriteableCogniteResource["Write"]` and implements `as_write() -> Write`; write DTO does too, returning `self`. List class is `List(CogniteResourceList[])` (mix in `ExternalIDTransformerMixin` if items have `external_id`). API class is `API`. Older code may use `Apply` or `Upsert` — for new code use `Write`. The pair is what lets `_create_multiple` accept either flavor. + +5. **Use the `APIClient` helpers; don't roll your own POST.** `_create_multiple`, `_retrieve`, `_retrieve_multiple`, `_list` / `_list_generator`, `_delete_multiple` handle chunking, semaphores, identifier shaping, single-vs-list overloads, and `404 → None` for retrieve. For one-item-per-request endpoints set `_CREATE_LIMIT = 1` (and/or `_DELETE_LIMIT = 1`) in `__init__` and let the helper chunk a `Sequence[X]` into N calls. Don't validate `len(items) == 1` by hand. + +6. **`@overload` the singular/plural pair.** Pass one, return one; pass a list, return a list: + ```python + @overload + async def create(self, items: StreamWrite) -> Stream: ... + @overload + async def create(self, items: Sequence[StreamWrite]) -> StreamList: ... + async def create(self, items: StreamWrite | Sequence[StreamWrite]) -> Stream | StreamList: ... + ``` + +7. **`from __future__ import annotations` at the top of every module.** Universal in this repo and required for the `X | Y` syntax on Python 3.10 (the canonical dev version). + +8. **No internal codenames in the public surface.** Class names, docstrings, parameter names, error messages — all customer-facing. PR #2534 had to scrub internal product codenames everywhere before merge. + +9. **`FeaturePreviewWarning` for alpha/beta features.** Construct in `__init__` as `self._warning = FeaturePreviewWarning(api_maturity=..., sdk_maturity=..., feature_name=...)`. Call `self._warning.warn()` as the **first line** of every public method body. It's a `FutureWarning` subclass at `cognite/client/utils/_experimental.py`, picklable, and `pytest.ini` already filters it to `ignore`. Pick `sdk_maturity="alpha"` whenever you want freedom to redesign the Python interface, even if the backend API is GA. + +10. **Don't edit `CHANGELOG.md` or `cognite/client/_version.py` by hand.** release-please owns both; they're regenerated from Conventional Commit titles on every push to `master`. + +## Anti-patterns (recurring review pushback from PR #2534) + +1. **BAD:** `dict[str, Any]` in a public signature. + **GOOD:** Build a typed DTO (subclass `CogniteResource`). +2. **BAD:** Internal codenames in class names or docstrings. + **GOOD:** Scrub to public-facing terminology before merge. +3. **BAD:** New module with `Examples:` blocks but no entry in `test_docstring_examples.py`. + **GOOD:** Register it (silently never runs otherwise). +4. **BAD:** Hand-rolled `if len(items) > 1: raise ValueError(...)` on a single-item-per-request endpoint. + **GOOD:** `self._CREATE_LIMIT = 1`; the helper chunks. +5. **BAD:** Ad-hoc utility helpers stuffed inside an API module. + **GOOD:** Put them in `cognite/client/utils/...` (or, more often, prove you don't need them). +6. **BAD:** Class docstring that just restates the class name in a sentence. + **GOOD:** Write something informational, or let `Args:` / `Returns:` / `Examples:` carry the load. +7. **BAD:** `MutableSequence[X]` in public surface, or inconsistent `Sequence` / `MutableSequence` between `create()` and `delete()`. + **GOOD:** `Sequence[X]` everywhere. +8. **BAD:** Overriding `dump(self, camel_case=True)` "just in case". + **GOOD:** Only override when nesting needs manual handling — inherited `convert_all_keys_to_camel_case` covers the common case (PR #2534 deleted three of four such overrides). +9. **BAD:** A bespoke `DeleteItem` class for an endpoint that takes only external IDs. + **GOOD:** `delete(external_id: str | SequenceNotStr[str])`. +10. **BAD:** PR description left stale after a redesign. + **GOOD:** Update it before merge. + +## Adding a new sub-API + +If the task is "add SDK support for a new resource group" (a new sub-API like streams), use the **`cognite-sdk-python-add-resource` skill** at `.claude/skills/cognite-sdk-python-add-resource/`. It has the 16-step boilerplate checklist, decision tree (flat vs nested, alpha vs GA, chunking), iteration order, and per-file references. The rules above still apply, but the skill walks the wiring (`testing.py` mocks, docs-time imports, doctest registration, sync codegen) that's easy to miss otherwise. + +## Examples to copy + +- **Alpha sub-API with read/write split + chunking:** `cognite/client/_api/data_modeling/streams.py` (~160 lines), DTOs at `cognite/client/data_classes/data_modeling/streams.py`, tests at `tests/tests_unit/test_api/test_data_modeling/test_streams.py`. +- **Established flat resource:** `cognite/client/_api/assets.py`, `events.py`, `time_series.py`. +- **Beta API + alpha SDK with `_api_subversion`:** `cognite/client/_api/agents/agents.py`. +- **DTO base classes:** `cognite/client/data_classes/_base.py` (`CogniteResource`, `WriteableCogniteResource`, `CogniteResourceList`, `WriteableCogniteResourceList`, `ExternalIDTransformerMixin`). +- **APIClient helpers:** `cognite/client/_api_client.py`. +- **URL retry classification:** `cognite/client/utils/_url.py` (`NON_RETRYABLE_CREATE_DELETE_RESOURCE_PATHS`). +- **Sync codegen:** `scripts/sync_client_codegen/main.py` (CLI: `run --files`, `run --all-files`, `verify`).