From f19f71574528437062f3f1957c64f55e28e9ab81 Mon Sep 17 00:00:00 2001 From: Emmanuel Sandorfi Date: Fri, 25 Sep 2026 16:24:48 +0200 Subject: [PATCH] feat(content_type)!: type the taxonomy batch actions and results `ContentType.batch()` now takes `ContentTypeAction` and returns `ContentTypeResult`, matching `File.batch_facets()`. Raw dicts still work. BREAKING CHANGE: results are no longer subscriptable, `r["status"]` becomes `r.status`. Taken over a dict-like shim, which would have left one batch endpoint's results permanently unlike the other's. --- AGENTS.md | 132 +++++++----- README.md | 42 +++- lighton/__init__.py | 8 + lighton/content_type.py | 408 ++++++++++++++++++++++++++++++++----- lighton/enums.py | 17 ++ tests/e2e/cli.py | 31 ++- tests/test_content_type.py | 268 ++++++++++++++++++++++-- 7 files changed, 763 insertions(+), 143 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 846c97b..c57a3ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,19 +75,21 @@ a field whose full domain is known, `workspace_type`/`document_upload_method` st - **Async jobs.** `parse`/`extract` take `mode: ExecMode` (default `ExecMode.SYNC`); `ExecMode.ASYNC` (uppercase members, value `"async"`, and lowercase `async` can't be a member name) sends `options={"async": true}`. `ExecMode` lives in `enums.py` (StrEnum, exported). Async returns a **pollable job handle** (`job.py`): `parse(mode=ASYNC)` → `ParseJob`, `extract(mode=ASYNC)` → `ExtractJob`; sync returns the full response model as before. Each verb has two `@overload`s keyed on `mode: Literal[ExecMode.SYNC|ASYNC]` so callers get the exact return type (`ParseResponse` vs `ParseJob`) instead of the union, the impl signature keeps the `ExecMode` default and the `... | ...Job` return. `Job.poll(page=None)` GETs `/`, absorbs the response onto itself in place (mirrors `_ActiveRecord._absorb`), returns self; `.done` (terminal, `completed_at` set) and `.succeeded` (`status == completed`) read state. `_Job` is a hand-written curated model (`extra="ignore"`) holding the shared plumbing + fields; `ParseJob`/`ExtractJob` subclass it ONLY because `result` differs (`ParseResult.pages` vs `ExtractResult.data`, whose optional fields make a union ambiguous), parse also has `error`. The job binds to the client via the `_VerbClient` transport surface (all it needs is `_request`), not a full `LightOn` (keeps the mixin's `self` assignable without a cast). `JobStatus` (enums.py) has only the documented `pending`/`completed`, the API doesn't publish the failure vocab, so it's for call-site comparison (StrEnum, unknown server values compare unequal, never validated onto the field), and the "poll until `.succeeded`, raise once `.done`" pattern keys off `completed_at`, not a failure string. `_Job.wait(timeout=300, poll=2)` is the auto-wait: a `File.wait`-style poll loop (no webhook exists) that returns self once terminal, raises `TimeoutError` past the deadline and `LightOnError` if `not .succeeded` (detail from `error` when the subclass has one, `getattr`, since only `ParseJob` does). The verbs expose it as `wait=False`/`timeout=300.0` (**same pair as `Workspace.ingest`**), declared **only on the ASYNC `@overload`** so `wait=True` without `mode=ASYNC` is a static error *and* a `ValueError` (sync already blocks); the two negative tests carry a `# ty: ignore[no-matching-overload]`. `wait=True` still returns the job (not the sync response model), so the return-type overloads stay two. No `poll` knob on the verbs, callers who need one use `job.wait(poll=...)`. - **Taxonomy writes** live on `ContentType` as **classmethods** (it isn't an `_ActiveRecord`: the endpoint returns a nested tree, not a paginated flat list, so - there is nothing to bind). One `_action()` helper posts `{action, ...}` to + there is nothing to bind). One `_action()` helper posts a `ContentTypeAction` to `_BASE`, mirroring `File._facet`, and the named methods (`define`/`undefine`/ - `define_attribute`/`undefine_attribute`/`adopt`) just name their fields; `batch()` - posts an `actions` list to `/content-types/batch`. Every action is idempotent, so + `define_attribute`/`undefine_attribute`/`adopt`) just build one; `batch()` posts + an `actions` list to `/content-types/batch`. Every action is idempotent, so `define()` doubles as rename. `undefine` **cascades the subtree**. Node/path arguments take a `ContentType` or a path string via `_path()` in `utils.py`, the scalar sibling of `_paths()` (`File._facet` was switched onto it too, it had the same coercion inlined). `AttributeType` (enums.py) enumerates the documented type - vocabulary; `choices` is **required** for select/multi-select and raises a - `ValueError` client-side rather than spending a round trip on the API's 422. - `batch()` returns the raw `{"status", "data"}` results: `data` is a node for the - content-type actions and an attribute for the attribute ones, so there is no one - model to validate it into (same reasoning as the `ParseJob`/`ExtractJob` split). + vocabulary; `choices` is **required** for select/multi-select and raises client-side + rather than spending a round trip on the API's 422 (from the `ContentTypeAction` + validator now, so it arrives as the `ValueError` subclass `ValidationError`). + `ContentTypeResult.data` stays a raw dict: it's a node for the content-type actions + and an attribute for the attribute ones, so there is no one model to validate it + into (same reasoning as the `ParseJob`/`ExtractJob` split), but the *envelope* is + typed, same as `FacetResult`. - **`Template` subclasses `ContentType`** solely to retype `attributes`: the templates endpoint hangs the **whole subtree's** attributes off the root as a `{path: [Attribute]}` **map**, where a live node carries its own flat list. It @@ -362,53 +364,74 @@ attribute value under an assigned type, T3), each accepting a `ContentType` or a string. `facets()` GETs the file's assigned types as `list[Facet]`. Like tags, File models no facet fields locally. -- **`batch_facets(actions)`** is the batch sibling (POST `/files//facets/batch`, - max **50**), and unlike `ContentType.batch` it is **typed both ways**. That - divergence is the point, not an oversight: a taxonomy action spans five different - shapes (`adopt` takes a path list, `define_content_type` code/label/parent, - `define_attribute` seven fields), so modelling it means five models or one wide - model whose valid fields depend on the action; a file-facet action has exactly - **one** shape (four fields, four verbs), which `FacetAction` models cleanly. Raw - dicts are still accepted alongside `FacetAction`, so the escape hatch is what the - two surfaces share. `ContentType.batch` was deliberately left untouched: it is - shipped public API and retyping its return would break `r["status"]` for everyone. - `FacetAction` is the one curated model with **`extra="forbid"`**: `ignore` suits - read models (drop response noise), but on a write model it would silently drop a - misspelled field from the body, so a typo raises instead (a test pins it). Unmodelled - fields go through a raw dict. -- **Naming split.** The `FacetAction` constructors carry the **SDK's** method names - (`classify`/`unclassify`/`set_attribute`/`clear_attribute`) so a batch is a - mechanical transcription of the one-by-one calls it replaces; `FacetActionType` - (enums.py, its full domain is documented, mirrors the generated - `FileFacetActionRequestActionEnum`) and the wire carry the **API's** - (`set_value`/`clear_value`). The enum exists partly to make that mapping - discoverable. Method name is `batch_facets`, not `facets_batch`: every write on - `File` is verb-first, and bare `batch` is unusable because `Workspace`/`batch.py` - already own a batch *ingest* concept. -- **One wire encoder.** `FacetAction._body()` is the single place that knows the - body field names, and `_facet` now takes a `FacetAction`, so the four single-action - methods and the batch cannot drift; a test pins single-action bodies equal to batch - bodies. `FacetAction`/`FacetResult`/`MAX_FACET_ACTIONS` live in `content_type.py` - next to `Facet`/`Attribute` (the `Template` precedent for taxonomy-adjacent data - models); `types/` would invert the dependency by pulling `ContentType` down into it. -- **The 50 cap raises a `ValueError` client-side**, the same trade as +- **Both batch endpoints are typed both ways, and symmetrically.** + `File.batch_facets(actions)` (POST `/files//facets/batch`) and + `ContentType.batch(client, actions)` (POST `/content-types/batch`), max **50** + each, take `FacetAction`/`ContentTypeAction` and return + `FacetResult`/`ContentTypeResult`. The taxonomy side used to stay raw dicts on the + grounds that a taxonomy action spans five shapes (`adopt` takes a path list, + `define_content_type` code/label/parent, `define_attribute` seven fields) where a + file-facet action has exactly one. That was **reversed**: the API itself models the + five as *one wide class plus a per-action validator* + (`ContentTypeActionRequest`, whose schema says so and names `FileFacetActionRequest` + as the same pattern), so the SDK follows, and `ContentTypeAction`'s + `@model_validator` enforces the narrow contract per verb exactly as + `FacetAction`'s does for the value verbs. The cost was a breaking change, + `r["status"]` became `r.status`, taken deliberately rather than carrying a mapping + shim that would have made one result model unlike the other. On a wide model the + **field descriptions name the verbs each field belongs to**; that is what keeps it + readable. Raw dicts are still accepted alongside both action models, so the escape + hatch is what the two surfaces share. Both action models are the curated models + with **`extra="forbid"`**: `ignore` suits read models (drop response noise), but on + a write model it would silently drop a misspelled field from the body, so a typo + raises instead (a test pins each). Unmodelled fields go through a raw dict. +- **Naming split, on both sides.** The constructors carry the **SDK's** method names + (`classify`/`unclassify`/`set_attribute`/`clear_attribute`; + `adopt`/`define`/`undefine`/`define_attribute`/`undefine_attribute`) so a batch is a + mechanical transcription of the one-by-one calls it replaces; `FacetActionType` and + `ContentTypeActionType` (enums.py, both domains documented, mirroring the generated + `FileFacetActionRequestActionEnum`/`ContentTypeActionRequestActionEnum`) and the + wire carry the **API's** (`set_value`/`clear_value`; + `define_content_type`/`undefine_content_type`). The enums exist partly to make that + mapping discoverable. Method name is `batch_facets`, not `facets_batch`: every write + on `File` is verb-first, and bare `batch` is unusable there because + `Workspace`/`batch.py` already own a batch *ingest* concept; on `ContentType` it is + free, so `batch` it is. +- **One wire encoder per surface.** `FacetAction._body()` and + `ContentTypeAction._body()` are the single places that know the body field names, + and `File._facet`/`ContentType._action` take an action object, so the single-action + methods and their batch cannot drift; a test on each side pins single-action bodies + equal to batch bodies. `ContentTypeAction._body()` is just + `model_dump(exclude_none=True)`: it reproduces the `_compact` semantics the named + methods used before (unset drops, `False` survives), and no content-type field is + meaningfully null on the wire the way `FacetAction.value` is for `set_value`, which + is the one reason that sibling hand-lists its fields. All of + `ContentTypeAction`/`ContentTypeResult`/`FacetAction`/`FacetResult` and the two + `MAX_*_ACTIONS` caps live in `content_type.py` next to `Facet`/`Attribute` (the + `Template` precedent for taxonomy-adjacent data models); `types/` would invert the + dependency by pulling `ContentType` down into it. Two constants, not one shared 50: + they are two endpoints' caps, and a facet-named constant guarding a taxonomy call + would be a naming lie. +- **The 50 caps raise a `ValueError` client-side**, the same trade as `define_attribute`'s missing-`choices` check, and empty is a local no-op (the - `tag`/`untag`/`delete_many` convention). It is deliberately **not chunked**: the - endpoint fails fast at the offending action and commits everything before it, so - chunking would both scatter that boundary (the reported `index` would be relative - to a batch the caller never wrote) and forfeit the single BM25 reindex that is the + `tag`/`untag`/`delete_many` convention). Neither is **chunked**: the endpoints fail + fast at the offending action and commit everything before it, so chunking would both + scatter that boundary (the reported `index` would be relative to a batch the caller + never wrote) and, on the facet side, forfeit the single BM25 reindex that is the whole reason the endpoint exists. -- **`FacetResult` is curated, not the generated `BatchResultItem`**: the generated - name is anonymous and can't document which status belongs to which verb. Its `data` - stays a raw dict for the reason `ContentType.batch` already gives (a classification - for `classify`, an attribute for `set_value`, null for the 204 verbs, so no one - model fits), but the *envelope* is uniform, which is what `FacetResult` buys. No - back-reference to the action: results only come back on success, complete and in - order, so `results[i]` is `actions[i]` by construction. +- **`FacetResult`/`ContentTypeResult` are curated, not the generated + `BatchResultItem`**: the generated name is anonymous and serves both endpoints, so it + can't document which status belongs to which verb. `data` stays a raw dict on both + (a classification for `classify`, an attribute for `set_value`, a node for + `define_content_type`, null for the 204 verbs, so no one model fits), but the + *envelope* is uniform, which is what the two result models buy. No back-reference to + the action: results only come back on success, complete and in order, so `results[i]` + is `actions[i]` by construction. - **Partial commit, unlike `delete_many`.** `/files/bulk-delete` is all-or-nothing - server-side, so it raises and there is no per-item report to give. This endpoint - commits the prefix before the failure and does **not** return those results, so the - *position* is the payload, and it rides on the exception (`LightOnAPIError.index`). + server-side, so it raises and there is no per-item report to give. Both batch + endpoints commit the prefix before the failure and do **not** return those results, + so the *position* is the payload, and it rides on the exception + (`LightOnAPIError.index`). If adding new resources, subclass `_ActiveRecord`: set `_base`/`_resource`, declare the field schema (narrow `id`), and add `create()`/`save()`. Everything else is inherited. @@ -470,8 +493,9 @@ throwaway workspace, runs every SDK verb against `tests/e2e/documents/`, and del it made. Add a step there when you add a feature. The `facet_filters` step needs a classified file, so `content_types` leaves the file classified (with an attribute set) the way `tags` leaves it tagged; on a tenant with an empty taxonomy `_seed_taxonomy()` -defines two throwaway content types straight through `client._request` (the SDK models -the taxonomy read-only) and registers their teardown. +builds two throwaway roots through the taxonomy writes themselves (`ContentType.define`/ +`define_attribute`/`batch`, which doubles as their live check) and registers the +cascading `undefine` teardown. - **Tooling**: ruff (lint + format), ty (type check), pytest, all enforced via pre-commit. `ty` has no autofix; it blocks on errors. - **uv.lock**: re-stage it after any dependency change before committing, or the ty pre-commit hook (which runs through `uv` and re-resolves) will report a lockfile modification and fail the commit. - New deps: prefer stdlib → installed dep → a few lines, before adding anything. Mark deliberate simplifications with `ponytail:` comments. diff --git a/README.md b/README.md index 23fbfe8..9e067d1 100644 --- a/README.md +++ b/README.md @@ -950,23 +950,43 @@ ContentType.undefine_attribute(client, audit, "fiscal_year") ContentType.undefine(client, "compliance") # also removes compliance:audit-report ``` -Building a tree is several calls, so `batch()` sends them in one request. Each entry -is the body a single method would send, and you get one result per action, in order: +Building a tree is several calls, so `batch()` sends up to 50 of them in a **single +request**. Build the list with `ContentTypeAction`, whose constructors take the same +arguments as the methods above, so a batch is a transcription of the calls it +replaces: ```python +from lighton import AttributeType, ContentTypeAction + results = ContentType.batch(client, [ - {"action": "adopt", "content_types": ["legal"]}, - {"action": "define_attribute", "content_type_path": "legal", - "name": "jurisdiction", "attribute_type": "select", "choices": ["FR", "US"]}, + ContentTypeAction.adopt(["legal"]), + ContentTypeAction.define("compliance", "Compliance"), + ContentTypeAction.define("audit-report", "Audit Report", parent="compliance"), + ContentTypeAction.define_attribute( + "legal", "jurisdiction", AttributeType.select, choices=["FR", "US"] + ), ]) -print([r["status"] for r in results]) # [201, 201] +print([r.status for r in results]) # [201, 201, 201, 201] ``` -This is the taxonomy-side sibling of -[`batch_facets()`](#many-writes-in-one-request). The entries stay raw dicts here -because the taxonomy spans five different action shapes (`adopt` takes a path list, `define_content_type` -takes code/label/parent, `define_attribute` takes seven fields), whereas a file -facet action has exactly one shape, which is what `FacetAction` models. +You get one `ContentTypeResult` per action, in the order sent, so `results[i]` belongs +to `actions[i]`. Each carries a `status` (201 created, 200 already applied) and `data`, +which is `None` for the 204 removals (`undefine`, `undefine_attribute`) and otherwise a +node for the content-type verbs or a definition for the attribute ones. + +The list is inert until it reaches `batch()`, so the same one seeds many tenants. The +failure rules are the taxonomy-side copy of +[`batch_facets()`](#many-writes-in-one-request): not transactional, a domain error stops +at that action and leaves the ones before it applied, the raised `LightOnAPIError` +carries the 0-based position on `.index`, every action is idempotent so the fix is to +correct that one and resend the whole list. Empty is a local no-op, and past 50 actions +`batch()` raises a `ValueError` before the request goes out. + +`ContentTypeAction` is one model covering all five verbs rather than five models, which +is how the API models it too: the fields are the union of the action shapes and a +validator enforces the narrow contract per verb, so a `define` without a `label` or a +`select` without `choices` is refused locally. A raw dict still works as an entry, for a +verb the SDK does not model yet. ## API keys diff --git a/lighton/__init__.py b/lighton/__init__.py index 0a85e41..80dd402 100644 --- a/lighton/__init__.py +++ b/lighton/__init__.py @@ -4,9 +4,12 @@ from lighton.apikey import ApiKey, ApiKeyScope from lighton.batch import BatchIngest, BatchIngestJob, BatchProgress, FailedIngest from lighton.content_type import ( + MAX_CONTENT_TYPE_ACTIONS, MAX_FACET_ACTIONS, Attribute, ContentType, + ContentTypeAction, + ContentTypeResult, Facet, FacetAction, FacetResult, @@ -14,6 +17,7 @@ ) from lighton.enums import ( AttributeType, + ContentTypeActionType, DownloadPurpose, ExecMode, FacetActionType, @@ -57,6 +61,9 @@ "BatchIngestJob", "BatchProgress", "ContentType", + "ContentTypeAction", + "ContentTypeActionType", + "ContentTypeResult", "DoneEvent", "DownloadPurpose", "ExecMode", @@ -72,6 +79,7 @@ "JobStatus", "LightOn", "LightOnConfiguration", + "MAX_CONTENT_TYPE_ACTIONS", "MAX_FACET_ACTIONS", "ParseJob", "RelevanceScoring", diff --git a/lighton/content_type.py b/lighton/content_type.py index 42a4517..13738eb 100644 --- a/lighton/content_type.py +++ b/lighton/content_type.py @@ -7,19 +7,21 @@ `Facet` is a content type *assigned to a file* together with the file's attribute values on it (see `File.classify()` / `File.facets()`). `Attribute` is the shared -name/type/value shape used by both. `FacetAction`/`FacetResult` are the write and -result shapes of `File.batch_facets()`. +name/type/value shape used by both. `ContentTypeAction`/`ContentTypeResult` and +`FacetAction`/`FacetResult` are the write and result shapes of the two batch +endpoints, `ContentType.batch()` and `File.batch_facets()`. """ from __future__ import annotations # The list() classmethod shadows builtin list in annotations (class scope). from builtins import list as _list +from collections.abc import Sequence from typing import TYPE_CHECKING, Any from pydantic import BaseModel, ConfigDict, Field, model_validator -from lighton.enums import AttributeType, FacetActionType +from lighton.enums import AttributeType, ContentTypeActionType, FacetActionType from lighton.utils import _compact, _path if TYPE_CHECKING: @@ -27,6 +29,9 @@ _BASE = "/api/v3/content-types" +MAX_CONTENT_TYPE_ACTIONS = 50 +"""Actions per `ContentType.batch()` call, the API's cap. Split a longer job yourself.""" + MAX_FACET_ACTIONS = 50 """Actions per `File.batch_facets()` call, the API's cap. Split a longer job yourself.""" @@ -105,10 +110,12 @@ def list( # --- taxonomy writes --------------------------------------------------- # Mirrors File._facet: one helper posts the action, the named methods just - # name their fields. Every action is idempotent server-side. + # build it. Every action is idempotent server-side. @classmethod - def _action(cls, client: LightOn, action: str, **fields: Any) -> Any: - return client._request("POST", _BASE, json=_compact(action=action, **fields)) + def _action(cls, client: LightOn, action: ContentTypeAction) -> Any: + # ContentTypeAction._body() is the single wire encoder, shared with + # batch(), so the single-action and batch bodies cannot drift apart. + return client._request("POST", _BASE, json=action._body()) @classmethod def templates(cls, client: LightOn) -> _list[Template]: @@ -136,7 +143,7 @@ def adopt(cls, client: LightOn, paths: _list[str]) -> _list[ContentType]: Returns: The imported top-level nodes. """ - data = cls._action(client, "adopt", content_types=paths) + data = cls._action(client, ContentTypeAction.adopt(paths)) return [cls.model_validate(n) for n in data["content_types"]] @classmethod @@ -171,12 +178,13 @@ def define( return cls.model_validate( cls._action( client, - "define_content_type", - code=code, - label=label, - parent_path=_path(parent) if parent is not None else None, - description=description, - inherit_attributes=inherit_attributes, + ContentTypeAction.define( + code, + label, + parent=parent, + description=description, + inherit_attributes=inherit_attributes, + ), ) ) @@ -191,9 +199,7 @@ def undefine(cls, client: LightOn, content_type: ContentType | str) -> None: Returns: None. """ - cls._action( - client, "undefine_content_type", content_type_path=_path(content_type) - ) + cls._action(client, ContentTypeAction.undefine(content_type)) @classmethod def define_attribute( @@ -227,23 +233,21 @@ def define_attribute( Raises: ValueError: If a select/multi-select is missing `choices`, which the API rejects with a 422 anyway, caught here to save the round trip. + Raised by `ContentTypeAction`, so it arrives as the pydantic + `ValidationError` subclass. """ - if ( - attribute_type in (AttributeType.select, AttributeType.multi_select) - and not choices - ): - raise ValueError(f"{attribute_type} needs choices") return Attribute.model_validate( cls._action( client, - "define_attribute", - content_type_path=_path(content_type), - name=name, - attribute_type=attribute_type, - choices=choices, - label=label, - description=description, - required=required, + ContentTypeAction.define_attribute( + content_type, + name, + attribute_type, + choices=choices, + label=label, + description=description, + required=required, + ), ) ) @@ -261,41 +265,63 @@ def undefine_attribute( Returns: None. """ - cls._action( - client, - "undefine_attribute", - content_type_path=_path(content_type), - name=name, - ) + cls._action(client, ContentTypeAction.undefine_attribute(content_type, name)) @classmethod def batch( - cls, client: LightOn, actions: _list[dict[str, Any]] - ) -> _list[dict[str, Any]]: - """Apply several taxonomy actions in one request (POST /content-types/batch). + cls, client: LightOn, actions: Sequence[ContentTypeAction | dict[str, Any]] + ) -> _list[ContentTypeResult]: + """Apply up to 50 taxonomy actions in one request (POST /content-types/batch). - Each entry is the body a single-action method would send, so a tree and + The batch form of adopt/define/undefine/define_attribute/ + undefine_attribute. Build the list with `ContentTypeAction`, whose + constructors take the same arguments as those methods, so a whole tree and its attributes land together instead of one round trip each: ContentType.batch(client, [ - {"action": "adopt", "content_types": ["legal"]}, - {"action": "define_attribute", "content_type_path": "legal", - "name": "jurisdiction", "attribute_type": "select", - "choices": ["FR", "US"]}, + ContentTypeAction.adopt(["legal"]), + ContentTypeAction.define_attribute( + "legal", "jurisdiction", AttributeType.select, + choices=["FR", "US"], + ), ]) + The list is inert until it gets here, so the same one seeds many tenants. + + **Not transactional.** Field-level mistakes are caught up front, so a + malformed action applies nothing. A domain error (an unknown parent path, + a permission denial) stops at that action: everything before it is already + applied, and the raised error carries its 0-based position on `.index`. + Every action is idempotent, so correct that one and resend the whole list. + Args: client: The client to write with. - actions: The action bodies, in order. + actions: The actions, in order, at most 50. `ContentTypeAction` + objects, or raw action bodies as dicts for a verb the SDK doesn't + model yet (mix freely). Empty is a local no-op. A longer job is + yours to split: chunking here would report an index relative to a + batch you never wrote. Returns: - One `{"status": ..., "data": ...}` entry per action, in the same - order. `data` is a node for the content-type actions and an attribute - for the attribute ones, so it's left as raw dicts rather than guessed - into one model. + One ContentTypeResult per action, in the order sent, so `results[i]` + belongs to `actions[i]`. Only ever complete: a failure raises instead. + + Raises: + ValueError: If more than 50 actions are passed (refused here to save + the round trip). + LightOnAPIError: If an action fails. `.index` is the one that did, and + the actions before it are already applied. """ - data = client._request("POST", f"{_BASE}/batch", json={"actions": actions}) - return data["results"] + if not actions: + return [] + if len(actions) > MAX_CONTENT_TYPE_ACTIONS: + raise ValueError( + f"a batch takes at most {MAX_CONTENT_TYPE_ACTIONS} actions, got " + f"{len(actions)}; send them {MAX_CONTENT_TYPE_ACTIONS} at a time" + ) + bodies = [a._body() if isinstance(a, ContentTypeAction) else a for a in actions] + data = client._request("POST", f"{_BASE}/batch", json={"actions": bodies}) + return [ContentTypeResult.model_validate(r) for r in data["results"]] class Template(ContentType): @@ -312,6 +338,288 @@ class Template(ContentType): ) +class ContentTypeAction(BaseModel): + """One taxonomy write, applied by `ContentType.batch()`. + + Build these with the constructors, not the fields: each takes the same + arguments in the same order as the `ContentType` classmethod of the same name + (minus `client`), so a batch is a transcription of the single-action calls. + + ContentType.batch(client, [ + ContentTypeAction.define("compliance", "Compliance"), + ContentTypeAction.define_attribute( + "compliance", "owner", AttributeType.text + ), + ]) + + One wide model rather than five, which is how the API models it too + (`ContentTypeActionRequest`): the fields are the union of the five action + shapes, and a validator enforces the narrow contract per verb. Each field + below names the verbs it belongs to. + + Nothing is sent until the list reaches `batch()`, so one list seeds many + tenants. + """ + + # forbid, not ignore: this is a write model, a misspelled field must fail + # loudly rather than vanish from the body. Raw dicts are the unmodelled escape. + model_config = ConfigDict(extra="forbid") + + action: ContentTypeActionType = Field( + description="The write to perform, the API's verb (see ContentTypeActionType)." + ) + content_types: _list[str] | None = Field( + None, description="Template root paths to import; adopt only." + ) + parent_path: str | None = Field( + None, description="Parent node path; define only, omitted for a root." + ) + code: str | None = Field( + None, + description="The node's own segment, lowercase with hyphens; define only.", + ) + content_type_path: str | None = Field( + None, + description=( + "Node the action applies to, e.g. legal:contract:nda; every verb but " + "adopt and define." + ), + ) + label: str | None = Field( + None, + description=( + "Human-readable label, required by define, optional on define_attribute." + ), + ) + description: str | None = Field( + None, description="Free-text description; define and define_attribute." + ) + inherit_attributes: bool | None = Field( + None, + description=( + "Whether children inherit this node's attributes (server default " + "True); define only." + ), + ) + name: str | None = Field( + None, + description=( + "Attribute identifier in snake_case; define_attribute and " + "undefine_attribute." + ), + ) + attribute_type: AttributeType | str | None = Field( + None, + description=( + "AttributeType, or the equivalent string (the API also accepts the " + "multi_select/multiselect/rich_text/richtext aliases, which skip the " + "local choices check); define_attribute only." + ), + ) + required: bool | None = Field( + None, + description=( + "Whether the schema requires a value (server default False); " + "define_attribute only." + ), + ) + choices: _list[str] | None = Field( + None, + description=( + "Allowed values, **required** for select and multi-select and " + "rejected for every other type; define_attribute only." + ), + ) + + @model_validator(mode="after") + def _each_verb_needs_its_fields(self) -> ContentTypeAction: + """Refuse locally what the API would 422: the narrow contract per verb.""" + needed = { + ContentTypeActionType.adopt: ("content_types",), + ContentTypeActionType.define_content_type: ("code", "label"), + ContentTypeActionType.undefine_content_type: ("content_type_path",), + ContentTypeActionType.define_attribute: ( + "content_type_path", + "name", + "attribute_type", + ), + ContentTypeActionType.undefine_attribute: ("content_type_path", "name"), + } + missing = [f for f in needed[self.action] if not getattr(self, f)] + if missing: + raise ValueError(f"{self.action} needs {', '.join(missing)}") + # ponytail: `AttributeType` only, which a StrEnum makes cover the canonical + # strings too. The API additionally accepts the multi_select/multiselect/ + # rich_text/richtext aliases; those reach its 422. Matching them here would + # mean hand-keeping a copy of a vocabulary we don't own, to save a round + # trip for a caller who deliberately went around the enum. + if ( + self.action == ContentTypeActionType.define_attribute + and self.attribute_type + in (AttributeType.select, AttributeType.multi_select) + and not self.choices + ): + raise ValueError(f"{self.attribute_type} needs choices") + return self + + @classmethod + def adopt(cls, paths: _list[str]) -> ContentTypeAction: + """Import starter trees, the batch form of `ContentType.adopt()`. + + Args: + paths: Template root paths to import, e.g. `["legal", "finance"]` + (see `ContentType.templates()`). + + Returns: + The action, unsent. + """ + return cls(action=ContentTypeActionType.adopt, content_types=paths) + + @classmethod + def define( + cls, + code: str, + label: str, + *, + parent: ContentType | str | None = None, + description: str | None = None, + inherit_attributes: bool | None = None, + ) -> ContentTypeAction: + """Create or update a node, the batch form of `ContentType.define()`. + + Args: + code: This node's own segment, lowercase alphanumeric with hyphens. + label: Human-readable label. + parent: Parent node or path; omit for a root node. + description: Optional free-text description. + inherit_attributes: Whether children inherit this node's attributes + (server default True). + + Returns: + The action, unsent. + """ + return cls( + action=ContentTypeActionType.define_content_type, + code=code, + label=label, + parent_path=_path(parent) if parent is not None else None, + description=description, + inherit_attributes=inherit_attributes, + ) + + @classmethod + def undefine(cls, content_type: ContentType | str) -> ContentTypeAction: + """Delete a node and its subtree, the batch form of `ContentType.undefine()`. + + Args: + content_type: The node to delete (object or path string). + + Returns: + The action, unsent. + """ + return cls( + action=ContentTypeActionType.undefine_content_type, + content_type_path=_path(content_type), + ) + + @classmethod + def define_attribute( + cls, + content_type: ContentType | str, + name: str, + attribute_type: AttributeType | str, + *, + choices: _list[str] | None = None, + label: str | None = None, + description: str | None = None, + required: bool | None = None, + ) -> ContentTypeAction: + """Add an attribute, the batch form of `ContentType.define_attribute()`. + + Args: + content_type: The node to define it on (object or path string). + name: Attribute identifier in snake_case. + attribute_type: AttributeType, or the equivalent string. + choices: Allowed values; **required** for select and multi-select, + and rejected by the server for every other type. + label: Human-readable label (defaults to a title-cased `name`). + description: Optional description. + required: Whether the schema requires a value (server default False). + + Returns: + The action, unsent. + + Raises: + ValueError: If a select/multi-select is missing `choices`, which the + API rejects with a 422 anyway, caught here to save the round trip. + """ + return cls( + action=ContentTypeActionType.define_attribute, + content_type_path=_path(content_type), + name=name, + attribute_type=attribute_type, + choices=choices, + label=label, + description=description, + required=required, + ) + + @classmethod + def undefine_attribute( + cls, content_type: ContentType | str, name: str + ) -> ContentTypeAction: + """Remove an attribute, the batch form of `ContentType.undefine_attribute()`. + + Args: + content_type: The node it's defined on (object or path string). + name: Attribute identifier to remove. + + Returns: + The action, unsent. + """ + return cls( + action=ContentTypeActionType.undefine_attribute, + content_type_path=_path(content_type), + name=name, + ) + + def _body(self) -> dict[str, Any]: + # The one place that knows the wire field names: the single-action + # classmethods post exactly this too, so single and batch can't drift. + # Every unset field simply stays out, which is what _compact did here + # before, and no content-type field is meaningfully null on the wire the + # way FacetAction's `value` is, so there is no special case to carry. + return self.model_dump(exclude_none=True) + + +class ContentTypeResult(BaseModel): + """What one action in a `ContentType.batch()` returned, in request order. + + Every result you receive succeeded: the endpoint fails fast, so a failing + action raises (see `LightOnAPIError.index`) and no results come back at all. A + returned list is therefore always complete and in order, so `results[i]` is the + outcome of `actions[i]`. + """ + + model_config = ConfigDict(extra="ignore") + + status: int = Field( + description=( + "Per-action status: 201 created, 200 already applied/updated, 204 for " + "the removals (undefine, undefine_attribute)." + ) + ) + data: dict[str, Any] | None = Field( + None, + description=( + "What the action returned, None for the 204 verbs. The content-type " + "verbs give a node, the attribute ones an attribute definition, and " + "adopt the imported roots. Left a raw dict: it differs per verb, so " + "there is no one model to validate it into." + ), + ) + + class Facet(BaseModel): """A content type assigned to a file, with the file's attribute values on it.""" diff --git a/lighton/enums.py b/lighton/enums.py index 5745f96..b7ee9bc 100644 --- a/lighton/enums.py +++ b/lighton/enums.py @@ -48,6 +48,23 @@ class AttributeType(StrEnum): rich_text = "rich-text" +class ContentTypeActionType(StrEnum): + """The wire verb in a taxonomy action (`ContentType.batch`). + + `define_content_type`/`undefine_content_type` are the API's names for what + `ContentType.define()` and `ContentType.undefine()` do. The + `ContentTypeAction` constructors keep the SDK's names so a batch reads like + the single-action calls it replaces; this enum keeps the API's, which is what + goes over the wire. + """ + + adopt = "adopt" + define_content_type = "define_content_type" + undefine_content_type = "undefine_content_type" + define_attribute = "define_attribute" + undefine_attribute = "undefine_attribute" + + class FacetActionType(StrEnum): """The wire verb in a file-facet action (`File.batch_facets`). diff --git a/tests/e2e/cli.py b/tests/e2e/cli.py index d104853..ca57115 100644 --- a/tests/e2e/cli.py +++ b/tests/e2e/cli.py @@ -36,6 +36,7 @@ Attribute, AttributeType, ContentType, + ContentTypeAction, DoneEvent, DownloadPurpose, ExecMode, @@ -43,6 +44,7 @@ FacetAction, File, LightOn, + MAX_CONTENT_TYPE_ACTIONS, MAX_FACET_ACTIONS, RelevanceScoring, Role, @@ -346,12 +348,8 @@ def _seed_taxonomy(c: Ctx) -> list[ContentType]: results = ContentType.batch( c.client, [ - { - "action": "define_content_type", - "parent_path": code, - "code": "batched", - "label": "Batched", - }, + ContentTypeAction.define("batched", "Batched", parent=code), + # a raw dict stays a valid entry, the escape hatch both batches share { "action": "define_attribute", "content_type_path": code, @@ -360,11 +358,28 @@ def _seed_taxonomy(c: Ctx) -> list[ContentType]: }, ], ) - assert all(r["status"] < 300 for r in results), f"batch failed: {results}" + assert all(r.status < 300 for r in results), f"batch failed: {results}" + assert results[1].data is not None, "define_attribute returns the definition" _say( f"defined {code} (+child, +batched), attributes {attr.name}/{sel.name}" - f"/{results[1]['data']['name']}" + f"/{results[1].data['name']}" ) + + try: + ContentType.batch(c.client, [ContentTypeAction.undefine("nope:not-a-type")]) + except LightOnAPIError as e: + assert e.index == 0, f"the failing action's position should be 0, got {e.index}" + else: + raise AssertionError("an unknown content type should have failed the batch") + + assert not ContentType.batch(c.client, []), "an empty batch should not have hit API" + with_too_many = [ContentTypeAction.undefine(code)] * (MAX_CONTENT_TYPE_ACTIONS + 1) + try: + ContentType.batch(c.client, with_too_many) + except ValueError: + pass # refused client-side, no round trip + else: + raise AssertionError("past the cap the batch should have been refused") return ContentType.list(c.client, include_attributes=True) diff --git a/tests/test_content_type.py b/tests/test_content_type.py index 9915e9d..daaf010 100644 --- a/tests/test_content_type.py +++ b/tests/test_content_type.py @@ -5,7 +5,16 @@ import httpx import pytest -from lighton import AttributeType, ContentType, LightOn, LightOnConfiguration +from lighton import ( + MAX_CONTENT_TYPE_ACTIONS, + AttributeType, + ContentType, + ContentTypeAction, + ContentTypeActionType, + LightOn, + LightOnConfiguration, +) +from lighton.exceptions import NotFoundError def make_client(handler) -> LightOn: @@ -135,7 +144,14 @@ def test_define_attribute_rejects_a_select_without_choices(): def handler(req: httpx.Request) -> httpx.Response: raise AssertionError("no request should be made") - for kind in (AttributeType.select, AttributeType.multi_select, "select"): + # the enum members and, a StrEnum making them equal, the canonical strings too. + # The API's aliases (multi_select, multiselect) are deliberately not matched. + for kind in ( + AttributeType.select, + AttributeType.multi_select, + "select", + "multi-select", + ): with pytest.raises(ValueError, match="choices"): ContentType.define_attribute(make_client(handler), "legal", "x", kind) @@ -197,34 +213,246 @@ def handler(req: httpx.Request) -> httpx.Response: assert tpl.attributes["legal"][0].name == "jurisdiction" -def test_batch_posts_every_action_and_returns_per_action_results(): - actions = [ - {"action": "adopt", "content_types": ["legal"]}, - { - "action": "define_attribute", - "content_type_path": "legal", - "name": "flag", - "attribute_type": "boolean", - }, - ] - client, sent = _writer( +def test_batch_posts_every_action_in_one_request(): + requests = [] + + def handler(req: httpx.Request) -> httpx.Response: + requests.append(req) + return httpx.Response( + 200, + json={ + "results": [ + {"status": 201, "data": {"path": "legal"}}, + {"status": 201, "data": {"name": "flag"}}, + ] + }, + ) + + ContentType.batch( + make_client(handler), + [ + ContentTypeAction.adopt(["legal"]), + ContentTypeAction.define_attribute("legal", "flag", AttributeType.boolean), + ], + ) + + assert len(requests) == 1, "a batch must be exactly one round trip" + assert requests[0].url.path == "/api/v3/content-types/batch" + assert json.loads(requests[0].content) == { + "actions": [ + {"action": "adopt", "content_types": ["legal"]}, + { + "action": "define_attribute", + "content_type_path": "legal", + "name": "flag", + "attribute_type": "boolean", + }, + ] + } + + +def test_batch_sends_the_same_bodies_as_the_single_action_methods(): + """ContentTypeAction._body() is the one wire encoder, so the paths can't drift.""" + single, batched = [], [] + + def handler(req: httpx.Request) -> httpx.Response: + body = json.loads(req.content) + if req.url.path.endswith("/batch"): + batched.extend(body["actions"]) + return httpx.Response( + 200, json={"results": [{"status": 200, "data": None}] * 5} + ) + single.append(body) + # one body the node, attribute and adopt parsers can all read + return httpx.Response( + 200, + json={ + "content_types": [], + "path": "x", + "code": "x", + "label": "X", + "name": "region", + }, + ) + + client = make_client(handler) + ContentType.adopt(client, ["legal"]) + ContentType.define(client, "nda", "NDA", parent="legal", inherit_attributes=False) + ContentType.define_attribute( + client, "legal", "region", AttributeType.select, choices=["FR"], required=False + ) + ContentType.undefine_attribute(client, "legal", "region") + ContentType.undefine(client, "legal") + ContentType.batch( + client, + [ + ContentTypeAction.adopt(["legal"]), + ContentTypeAction.define( + "nda", "NDA", parent="legal", inherit_attributes=False + ), + ContentTypeAction.define_attribute( + "legal", + "region", + AttributeType.select, + choices=["FR"], + required=False, + ), + ContentTypeAction.undefine_attribute("legal", "region"), + ContentTypeAction.undefine("legal"), + ], + ) + + assert single == batched + + +def test_batch_parses_results_in_order(): + client, _ = _writer( { "results": [ - {"status": 201, "data": {"path": "legal"}}, - {"status": 201, "data": {"name": "flag"}}, + {"status": 201, "data": {"path": "compliance"}}, + {"status": 204, "data": None}, + {"status": 200, "data": {"name": "owner", "type": "text"}}, ] } ) - results = ContentType.batch(client, actions) + results = ContentType.batch( + client, + [ + ContentTypeAction.define("compliance", "Compliance"), + ContentTypeAction.undefine_attribute("compliance", "stale"), + ContentTypeAction.define_attribute( + "compliance", "owner", AttributeType.text + ), + ], + ) + + assert [r.status for r in results] == [201, 204, 200] + assert results[1].data is None, "the 204 verbs carry no data" + assert results[2].data == {"name": "owner", "type": "text"} + + +def test_batch_refuses_more_than_fifty_actions(): + action = ContentTypeAction.undefine("legal") + + def _no_request(req: httpx.Request) -> httpx.Response: + raise AssertionError(f"no request should be made, got {req.url}") + + with pytest.raises(ValueError, match="50"): + ContentType.batch( + make_client(_no_request), [action] * (MAX_CONTENT_TYPE_ACTIONS + 1) + ) + + # and the boundary itself is accepted: an off-by-one here is the plausible bug + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={ + "results": [{"status": 204, "data": None}] * MAX_CONTENT_TYPE_ACTIONS + }, + ) + + assert ( + len( + ContentType.batch(make_client(handler), [action] * MAX_CONTENT_TYPE_ACTIONS) + ) + == MAX_CONTENT_TYPE_ACTIONS + ) + + +def test_batch_is_a_local_no_op_when_empty(): + def _no_request(req: httpx.Request) -> httpx.Response: + raise AssertionError(f"no request should be made, got {req.url}") + + assert ContentType.batch(make_client(_no_request), []) == [] + - assert sent["path"] == "/api/v3/content-types/batch" - assert sent["body"] == {"actions": actions} - assert [r["status"] for r in results] == [201, 201] - assert results[1]["data"]["name"] == "flag" +def test_batch_passes_raw_dicts_through(): + client, sent = _writer({"results": [{"status": 200, "data": None}] * 2}) + + raw = {"action": "some_future_verb", "content_type_path": "legal", "extra": 1} + ContentType.batch(client, [ContentTypeAction.undefine("legal"), raw]) + + assert sent["body"]["actions"][1] == raw, "a raw dict must reach the wire untouched" + + +def test_batch_reports_the_failing_action_index(): + """`index` rides on the status-mapped class, so `except NotFoundError` still works.""" + + def handler(req: httpx.Request) -> httpx.Response: + return httpx.Response(404, json={"detail": "unknown parent", "index": 1}) + + actions = [ + ContentTypeAction.define("compliance", "Compliance"), + ContentTypeAction.define("audit", "Audit", parent="nope:not-a-type"), + ] + with pytest.raises(NotFoundError) as excinfo: + ContentType.batch(make_client(handler), actions) + + assert excinfo.value.index == 1 + assert "action 1" in str(excinfo.value), "the position belongs in the message too" + # which is what makes the offender addressable, and the prefix already applied + assert actions[excinfo.value.index].parent_path == "nope:not-a-type" + + +def test_content_type_action_rejects_a_verb_missing_its_fields(): + # pydantic's ValidationError subclasses ValueError; refused before any request + with pytest.raises(ValueError, match="code, label"): + ContentTypeAction(action=ContentTypeActionType.define_content_type) + + with pytest.raises(ValueError, match="content_type_path"): + ContentTypeAction(action=ContentTypeActionType.undefine_content_type) + + with pytest.raises(ValueError, match="content_types"): + ContentTypeAction(action=ContentTypeActionType.adopt, content_types=[]) + + +def test_content_type_action_rejects_a_misspelled_field(): + # a write model: a typo must not silently vanish from the request body + with pytest.raises(ValueError, match="atribute_type"): + ContentTypeAction( + action=ContentTypeActionType.define_attribute, + content_type_path="legal", + name="region", + attribute_type=AttributeType.text, + atribute_type="typo", # ty: ignore[unknown-argument] + ) def test_define_omits_what_you_did_not_set(): client, sent = _writer({"path": "x", "code": "x", "label": "X"}) ContentType.define(client, "x", "X") assert sent["body"] == {"action": "define_content_type", "code": "x", "label": "X"} + + +def test_a_false_flag_is_sent_not_dropped(): + """The other half of "omits what you did not set": False is a value, not unset. + + `_body()` drops on None alone. Were it ever to drop on falsiness (or on + `exclude_defaults`), `inherit_attributes=False` would vanish and the server + would apply its default of True, silently inheriting attributes the caller + asked it not to — and it would vanish from the batch path identically, so the + single-equals-batch test would stay green. Hence a body assertion on each. + """ + client, sent = _writer({"path": "x", "code": "x", "label": "X"}) + ContentType.define(client, "x", "X", inherit_attributes=False) + assert sent["body"]["inherit_attributes"] is False + + client, sent = _writer({"name": "region", "type": "text"}) + ContentType.define_attribute( + client, "legal", "region", AttributeType.text, required=False + ) + assert sent["body"]["required"] is False + + client, sent = _writer({"results": [{"status": 201, "data": None}] * 2}) + ContentType.batch( + client, + [ + ContentTypeAction.define("x", "X", inherit_attributes=False), + ContentTypeAction.define_attribute( + "legal", "region", AttributeType.text, required=False + ), + ], + ) + assert sent["body"]["actions"][0]["inherit_attributes"] is False + assert sent["body"]["actions"][1]["required"] is False