diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..9174f419 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,5 @@ +# Shell entrypoints are executed with sh/dash inside the container image. A +# Windows checkout with core.autocrlf=true rewrites them to CRLF, and dash +# then aborts with "set: Illegal option" because the carriage return becomes +# part of the "-e" flag argument (docker/entrypoint.sh). +*.sh text eol=lf diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..5463b6d0 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,13 @@ +## Description + + + +## Proofs + + + +## Checklist + +- [ ] This PR is correlated to an existing issue, and I understand it will be closed if that issue does not have the `maintainer-approved` label. + + diff --git a/CLAUDE.md b/CLAUDE.md index c9862b59..6a83066d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -187,6 +187,9 @@ The Dreamer is an orchestrated multi-specialist system that runs during schedule - **LLM subsystem** (`src/llm/`): provider-agnostic `honcho_llm_call()`. Backends in `src/llm/backends/` (`anthropic.py`, `gemini.py`, `openai.py`). Includes prompt caching (`caching.py`), structured output (`structured_output.py`), tool loop (`tool_loop.py`), history adapters for cross-provider message formats, and a model registry. Per-retry provider selection is pinned via an `AttemptPlan` so stream-final retries don't bounce back to primary after the tool loop has settled on fallback. - **Per-agent model config**: each agent has its own `MODEL_CONFIG` in `src/config.py` with fallback chains (see `ConfiguredModelSettings`, `FallbackModelSettings`). - **Telemetry**: cloudevents in `src/telemetry/events/` cover API routes, dialectic, dream, deletion, reconciliation, representation, and per-call LLM accounting (`llm.py` — `LLMCallCompletedEvent` fires once per provider hit with full cost-attribution context). High-volume events are sampled deterministically per `run_id` via `TelemetrySettings.HIGH_VOLUME_SAMPLE_RATE`. +- **Prometheus metrics** (`src/telemetry/prometheus/`): every metric carries a `namespace` label and every recorder is fail-soft (a metrics error never propagates into a request or a worker loop). Counter children with a *bounded* label domain are zero-initialized per process at startup — `initialize_bounded_metrics(instance_type=...)`, called from the `src/main.py` lifespan (`api`) and `src/deriver/__main__.py` (`deriver`) — so an absent series means a broken scrape rather than "nothing happened". Two consequences worth knowing before touching telemetry: + - **Adding a `BaseEvent` subclass requires adding its `_event_type` to `ALL_EVENT_TYPES`** in `src/telemetry/events/__init__.py` (and to `HIGH_VOLUME_EVENT_TYPES` if `_volume_class == "high_volume"`). Enforced by the drift guards in `tests/telemetry/test_metric_zero_init.py`, which assert set-equality against the discovered subclasses. + - **A service-wide, non-additive gauge must be refreshed by every replica on its own timer**, and aggregated with `max()`/`avg()`, never `sum()`. `message_embeddings_pending` is the example: it reports a DB-global count, so it is driven from `ReconcilerScheduler._scheduler_loop` (runs on all replicas) rather than from the work-unit-deduped reconciliation cycle — otherwise, combined with the zero-init, every replica that never won the work unit would export a confident permanent `0`. ### Project Structure diff --git a/config.toml.example b/config.toml.example index c5517999..60d9092a 100644 --- a/config.toml.example +++ b/config.toml.example @@ -77,6 +77,8 @@ model = "text-embedding-3-small" # Optional provider request input cap. Useful for OpenAI-compatible embedding # APIs with smaller limits, such as DashScope text-embedding-v4. # max_batch_size = 10 +# Optional client HTTP timeout in seconds (OpenAI + Gemini). +# timeout = 90.0 # Optional module-level endpoint overrides # [embedding.model_config.overrides] diff --git a/docs/v3/contributing/configuration.mdx b/docs/v3/contributing/configuration.mdx index c02a4ebb..3c210e71 100644 --- a/docs/v3/contributing/configuration.mdx +++ b/docs/v3/contributing/configuration.mdx @@ -267,6 +267,7 @@ EMBEDDING_MAX_TOKENS_PER_REQUEST=300000 EMBEDDING_MODEL_CONFIG__TRANSPORT=openai # openai, gemini EMBEDDING_MODEL_CONFIG__MODEL=text-embedding-3-small EMBEDDING_MODEL_CONFIG__MAX_BATCH_SIZE=10 # optional per-request input cap +EMBEDDING_MODEL_CONFIG__TIMEOUT=90.0 # optional client HTTP timeout (seconds) # Optional endpoint overrides EMBEDDING_MODEL_CONFIG__OVERRIDES__BASE_URL=http://localhost:8000/v1 @@ -279,6 +280,13 @@ document a per-request limit. Set it when an OpenAI-compatible embedding provider accepts fewer inputs per request, such as DashScope `text-embedding-v4` with a limit of 10. +`EMBEDDING_MODEL_CONFIG__TIMEOUT` is an optional client HTTP timeout in +seconds. OpenAI-compatible transports receive it as the SDK `timeout` kwarg +(omitted when unset, so the SDK default applies). Gemini converts it to +milliseconds on `http_options.timeout`, and keeps its existing 10-minute +default when unset. The value is validated at config load the same way as +LLM `provider_params.timeout` (positive, finite number of seconds). + Forwarding `dimensions=` to OpenAI-compatible providers is controlled by `EMBEDDING_MODEL_CONFIG__DIMENSIONS_MODE`: - `auto` (default): forwards `dimensions=` when **the operator has explicitly set `EMBEDDING_VECTOR_DIMENSIONS`** — provenance, not value — and the configured model is not on the known-rejecting list (currently `text-embedding-ada-002`). Explicit `EMBEDDING_VECTOR_DIMENSIONS=1536` *does* trigger the forward; this is how `text-embedding-3-large` truncation to 1536 is expressed. Deployments that leave the setting unset get their existing behavior (`dimensions=` is not forwarded). diff --git a/sdks/python/src/honcho/__init__.py b/sdks/python/src/honcho/__init__.py index 239f35ef..a07bf35f 100644 --- a/sdks/python/src/honcho/__init__.py +++ b/sdks/python/src/honcho/__init__.py @@ -41,11 +41,16 @@ from importlib.metadata import PackageNotFoundError, version from pathlib import Path import re -from .aio import ConclusionScopeAio, HonchoAio, PeerAio, SessionAio -from .api_types import MessageCreateParams -from .base import PeerBase, SessionBase +from .aio import ConclusionsViewAio, HonchoAio, PeerAio, ScopeAio, SessionAio +from .api_types import ( + MessageCreateParams, + ScopeBackfillJob, + ScopeResponse, + ScopeStatusResponse, +) +from .base import PeerBase, ScopeBase, SessionBase from .client import Honcho -from .conclusions import Conclusion, ConclusionScope +from .conclusions import Conclusion, ConclusionsView from .http.exceptions import ( APIError, AuthenticationError, @@ -63,6 +68,7 @@ from .http.exceptions import ( from .message import Message from .pagination import AsyncPage, SyncPage from .peer import Peer +from .scope import Scope from .session import Session from .session_context import SessionContext, SessionSummaries, Summary from .types import ( @@ -70,6 +76,12 @@ from .types import ( DialecticStreamResponse, ) +# Deprecated aliases. "Scope" now means a named set of sessions (see `Scope`), +# which these are not — they are views over one observer/observed pair. Kept for +# one more minor version. +ConclusionScope = ConclusionsView +ConclusionScopeAio = ConclusionsViewAio + def _detect_version() -> str: try: @@ -95,23 +107,32 @@ __all__ = [ "Honcho", # Domain classes "Conclusion", - "ConclusionScope", + "ConclusionsView", "Message", "MessageCreateParams", "Peer", + "Scope", "Session", # Aio views (for type hints) - "ConclusionScopeAio", + "ConclusionsViewAio", "HonchoAio", "PeerAio", + "ScopeAio", "SessionAio", # Base classes "PeerBase", + "ScopeBase", "SessionBase", # Response types + "ScopeBackfillJob", + "ScopeResponse", + "ScopeStatusResponse", "SessionContext", "SessionSummaries", "Summary", + # Deprecated aliases + "ConclusionScope", + "ConclusionScopeAio", # Pagination "AsyncPage", "SyncPage", diff --git a/sdks/python/src/honcho/aio.py b/sdks/python/src/honcho/aio.py index 51797c72..29c0f445 100644 --- a/sdks/python/src/honcho/aio.py +++ b/sdks/python/src/honcho/aio.py @@ -3,7 +3,7 @@ This module provides async accessor classes that wrap the main SDK classes and provide async versions of all operations. Access via the `.aio` property -on Honcho, Peer, Session, and ConclusionScope instances. +on Honcho, Peer, Session, and ConclusionsView instances. Example: ```python @@ -24,7 +24,7 @@ from __future__ import annotations import json import logging import warnings -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, Sequence from datetime import datetime from typing import TYPE_CHECKING, Any, ClassVar, Literal, overload @@ -40,15 +40,18 @@ from .api_types import ( PeerResponse, QueueStatusResponse, RepresentationResponse, + ScopeBackfillJob, + ScopeResponse, + ScopeStatusResponse, SessionConfiguration, SessionPeerConfig, SessionResponse, WorkspaceConfiguration, WorkspaceResponse, ) -from .base import PeerBase, SessionBase +from .base import PeerBase, ScopeBase, SessionBase from .conclusions import ( - _SCOPE_RESERVED, + _VIEW_RESERVED, Conclusion, _reject_reserved_filter_keys, ) @@ -64,14 +67,20 @@ from .utils import ( parse_sse_astream, prepare_file_for_upload, resolve_id, + resolve_scope_membership, + resolve_scope_session, + scope_context_fields, + scope_recall_fields, + validate_scope_id, ) if TYPE_CHECKING: from .client import Honcho - from .conclusions import ConclusionScope + from .conclusions import ConclusionsView from .conclusions import ConclusionCreateParams from .peer import Peer, TResponseFormat, serialize_response_format +from .scope import Scope from .session import Session logger = logging.getLogger(__name__) @@ -79,8 +88,9 @@ logger = logging.getLogger(__name__) __all__ = [ "HonchoAio", "PeerAio", + "ScopeAio", "SessionAio", - "ConclusionScopeAio", + "ConclusionsViewAio", ] @@ -267,6 +277,7 @@ class HonchoAio(AsyncMetadataConfigMixin): | list[tuple[PeerBase | str, SessionPeerConfig]] | list[PeerBase | str | tuple[PeerBase | str, SessionPeerConfig]] | None = None, + scopes: Sequence[str | ScopeBase] | None = None, ) -> Session: """ Get or create a session with the given ID asynchronously. @@ -278,6 +289,11 @@ class HonchoAio(AsyncMetadataConfigMixin): peers: Optional peers to attach to the session at creation. Accepts the same shape as Session.add_peers (peer ID string, Peer object, list of either, or tuples with SessionPeerConfig). + scopes: Optional scopes this session should join, as IDs or Scope + objects. Each scope is created if it does not exist yet. Attaching + at creation avoids the asynchronous backfill a later + ``scope.add_sessions()`` triggers, since there is no history to + copy. Returns: A Session object with cached values from the API response. @@ -290,6 +306,8 @@ class HonchoAio(AsyncMetadataConfigMixin): body["configuration"] = configuration.model_dump(exclude_none=True) if peers is not None: body["peers"] = normalize_peers_to_dict(peers) + if scopes is not None: + body["scopes"] = [validate_scope_id(resolve_id(scope)) for scope in scopes] data = await self._honcho._async_http_client.post( routes.sessions(self._honcho.workspace_id), body=body @@ -358,6 +376,88 @@ class HonchoAio(AsyncMetadataConfigMixin): return AsyncPage(data, SessionResponse, transform, fetch_next) + async def scope( + self, + id: str, # noqa: A002 + *, + metadata: dict[str, object] | None = None, + ) -> Scope: + """ + Get or create a scope with the given ID asynchronously. + + A scope is a named set of sessions that acts as a visibility boundary: + recall performed through the scope sees only what happened in its sessions, + while the underlying peer keeps its single unified representation of + everything. + + Args: + id: Unprefixed scope name, unique within the workspace. + metadata: Optional metadata dictionary to associate with this scope. + + Returns: + A Scope object for managing membership. + + Raises: + ValueError: If the scope ID is invalid. + """ + validate_scope_id(id) + await self._honcho._ensure_workspace_async() + body: dict[str, Any] = {"id": id} + if metadata is not None: + body["metadata"] = metadata + + data = await self._honcho._async_http_client.post( + routes.scopes(self._honcho.workspace_id), body=body + ) + scope_data = ScopeResponse.model_validate(data) + return Scope( + id, + self._honcho, + metadata=scope_data.metadata, + created_at=scope_data.created_at, + ) + + async def scopes( + self, + *, + page: int = 1, + size: int = 50, + reverse: bool = False, + ) -> AsyncPage[ScopeResponse, Scope]: + """ + Get all scopes in the current workspace asynchronously. + + Args: + page: Page number (1-indexed). Default: 1. + size: Number of items per page. Default: 50. + reverse: If True, reverses the default ordering. Default: False. + """ + await self._honcho._ensure_workspace_async() + + async def fetch(next_page: int) -> dict[str, Any]: + query: dict[str, Any] = {"page": next_page, "size": size} + if reverse: + query["reverse"] = "true" + return await self._honcho._async_http_client.post( + routes.scopes_list(self._honcho.workspace_id), query=query + ) + + def transform(scope: ScopeResponse) -> Scope: + """Convert a scope API response into a Scope SDK object.""" + return Scope( + scope.id, + self._honcho, + metadata=scope.metadata, + created_at=scope.created_at, + ) + + async def fetch_next(next_page: int) -> AsyncPage[ScopeResponse, Scope]: + return AsyncPage( + await fetch(next_page), ScopeResponse, transform, fetch_next + ) + + return AsyncPage(await fetch(page), ScopeResponse, transform, fetch_next) + async def workspaces( self, filters: dict[str, object] | None = None, @@ -409,12 +509,27 @@ class HonchoAio(AsyncMetadataConfigMixin): limit: int = Field( default=10, ge=1, le=100, description="Number of results to return" ), + *, + scope: str | ScopeBase | None = None, ) -> list[Message]: - """Search for messages in the current workspace asynchronously.""" + """Search for messages in the current workspace asynchronously. + + Args: + query: The search query to use + filters: Filters to scope the search. + limit: Number of results to return (1-100, default: 10) + scope: Optional scope (ID or Scope object) restricting the search to + that scope's member sessions. Mutually exclusive with a + ``session_id`` filter. A scope with no member sessions matches + nothing rather than everything. + """ await self._honcho._ensure_workspace_async() + body: dict[str, Any] = {"query": query, "filters": filters, "limit": limit} + if scope is not None: + body["scope"] = validate_scope_id(resolve_id(scope)) data = await self._honcho._async_http_client.post( routes.workspace_search(self._honcho.workspace_id), - body={"query": query, "filters": filters, "limit": limit}, + body=body, ) return [ Message.from_api_response(MessageResponse.model_validate(item)) @@ -584,6 +699,8 @@ class PeerAio(AsyncMetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[TResponseFormat], @@ -596,6 +713,8 @@ class PeerAio(AsyncMetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: dict[str, Any] | None = None, @@ -608,6 +727,8 @@ class PeerAio(AsyncMetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[BaseModel] | dict[str, Any] | None = None, @@ -623,6 +744,11 @@ class PeerAio(AsyncMetadataConfigMixin): resolved_session_id = resolve_id(session) body: dict[str, Any] = {"query": query, "stream": False} + body.update( + scope_recall_fields( + scope=scope, sessions=sessions, session_id=resolved_session_id + ) + ) if target_id: body["target"] = target_id if resolved_session_id: @@ -651,6 +777,8 @@ class PeerAio(AsyncMetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[BaseModel] | dict[str, Any] | None = None, @@ -666,6 +794,11 @@ class PeerAio(AsyncMetadataConfigMixin): resolved_session_id = resolve_id(session) body: dict[str, Any] = {"query": query, "stream": True} + body.update( + scope_recall_fields( + scope=scope, sessions=sessions, session_id=resolved_session_id + ) + ) if target_id: body["target"] = target_id if resolved_session_id: @@ -826,13 +959,22 @@ class PeerAio(AsyncMetadataConfigMixin): search_max_distance: float | None = Field(None, ge=0.0, le=1.0), include_most_frequent: bool | None = None, max_conclusions: int | None = Field(None, ge=1, le=100), + *, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, ) -> str: - """Get a subset of the representation of the peer asynchronously.""" + """Get a subset of the representation of the peer asynchronously. + + See Peer.representation for parameter details, including the depth caveat + on ``sessions``. + """ await self._peer._honcho._ensure_workspace_async() session_id = resolve_id(session) target_id = resolve_id(target) - body: dict[str, Any] = {} + body: dict[str, Any] = scope_recall_fields( + scope=scope, sessions=sessions, session_id=session_id + ) if session_id: body["session_id"] = session_id if target_id: @@ -1192,6 +1334,14 @@ class SessionAio(AsyncMetadataConfigMixin): None, description="A peer ID to get context from the perspective of.", ), + scope: str | ScopeBase | None = Field( + None, + description="A scope to use as the perspective source instead of a peer.", + ), + sessions: Sequence[str | SessionBase] | None = Field( + None, + description="An allowlist of sessions confining `peer_target`'s representation to that set. This session must be one of them.", + ), limit_to_session: bool = Field( False, description="Whether to limit the representation to this session only.", @@ -1219,7 +1369,11 @@ class SessionAio(AsyncMetadataConfigMixin): description="Maximum number of conclusions to include in the representation.", ), ) -> SessionContext: - """Get optimized context for this session asynchronously.""" + """Get optimized context for this session asynchronously. + + See Session.context for parameter details, including the depth caveat on + ``sessions``. + """ await self._session._honcho._ensure_workspace_async() if peer_target is None and peer_perspective is not None: raise ValueError( @@ -1238,6 +1392,13 @@ class SessionAio(AsyncMetadataConfigMixin): query: dict[str, Any] = { "summary": summary, "limit_to_session": limit_to_session, + **scope_context_fields( + scope=scope, + sessions=sessions, + peer_target=peer_target, + peer_perspective=peer_perspective, + limit_to_session=limit_to_session, + ), } if tokens is not None: query["tokens"] = tokens @@ -1488,19 +1649,19 @@ class SessionAio(AsyncMetadataConfigMixin): return Message.from_api_response(MessageResponse.model_validate(data)) -class ConclusionScopeAio: +class ConclusionsViewAio: """ - Async view of a ConclusionScope. + Async view of a ConclusionsView. - Access via `scope.aio`. Provides async versions of all ConclusionScope methods. - Shares state with the parent ConclusionScope instance. + Access via `view.aio`. Provides async versions of all ConclusionsView methods. + Shares state with the parent ConclusionsView instance. """ - __slots__: ClassVar[tuple[str, ...]] = ("_scope",) - _scope: "ConclusionScope" + __slots__: ClassVar[tuple[str, ...]] = ("_view",) + _view: "ConclusionsView" - def __init__(self, scope: "ConclusionScope") -> None: - self._scope = scope + def __init__(self, view: "ConclusionsView") -> None: + self._view = view async def list( self, @@ -1520,13 +1681,13 @@ class ConclusionScopeAio: https://honcho.dev/docs/v3/documentation/features/advanced/using-filters """ _reject_reserved_filter_keys( - filters, _SCOPE_RESERVED + ("session", "session_id") + filters, _VIEW_RESERVED + ("session", "session_id") ) - await self._scope._honcho._ensure_workspace_async() + await self._view._honcho._ensure_workspace_async() resolved_session_id = resolve_id(session) filters = { - "observer_id": self._scope.observer, - "observed_id": self._scope.observed, + "observer_id": self._view.observer, + "observed_id": self._view.observed, **({"session_id": resolved_session_id} if resolved_session_id else {}), **(filters or {}), } @@ -1534,8 +1695,8 @@ class ConclusionScopeAio: query: dict[str, Any] = {"page": page, "size": size} if reverse: query["reverse"] = "true" - data = await self._scope._honcho._async_http_client.post( - routes.conclusions_list(self._scope.workspace_id), + data = await self._view._honcho._async_http_client.post( + routes.conclusions_list(self._view.workspace_id), body={"filters": filters}, query=query, ) @@ -1549,8 +1710,8 @@ class ConclusionScopeAio: next_query: dict[str, Any] = {"page": next_page, "size": size} if reverse: next_query["reverse"] = "true" - next_data = await self._scope._honcho._async_http_client.post( - routes.conclusions_list(self._scope.workspace_id), + next_data = await self._view._honcho._async_http_client.post( + routes.conclusions_list(self._view.workspace_id), body={"filters": filters}, query=next_query, ) @@ -1575,11 +1736,11 @@ class ConclusionScopeAio: filters: Optional dictionary of additional filter criteria, merged with this scope's observer/observed (e.g. ``{"level": "deductive"}``). """ - _reject_reserved_filter_keys(filters, _SCOPE_RESERVED) - await self._scope._honcho._ensure_workspace_async() + _reject_reserved_filter_keys(filters, _VIEW_RESERVED) + await self._view._honcho._ensure_workspace_async() filters = { - "observer_id": self._scope.observer, - "observed_id": self._scope.observed, + "observer_id": self._view.observer, + "observed_id": self._view.observed, **(filters or {}), } @@ -1591,8 +1752,8 @@ class ConclusionScopeAio: if distance is not None: body["distance"] = distance - data = await self._scope._honcho._async_http_client.post( - routes.conclusions_query(self._scope.workspace_id), + data = await self._view._honcho._async_http_client.post( + routes.conclusions_query(self._view.workspace_id), body=body, ) return [ @@ -1602,9 +1763,9 @@ class ConclusionScopeAio: async def delete(self, conclusion_id: str) -> None: """Delete a conclusion by ID asynchronously.""" - await self._scope._honcho._ensure_workspace_async() - await self._scope._honcho._async_http_client.delete( - routes.conclusion(self._scope.workspace_id, conclusion_id) + await self._view._honcho._ensure_workspace_async() + await self._view._honcho._async_http_client.delete( + routes.conclusion(self._view.workspace_id, conclusion_id) ) async def create( @@ -1612,15 +1773,15 @@ class ConclusionScopeAio: conclusions: list[ConclusionCreateParams | dict[str, Any]], ) -> list[Conclusion]: """Create conclusions in this scope asynchronously.""" - await self._scope._honcho._ensure_workspace_async() + await self._view._honcho._ensure_workspace_async() def build_conclusion_payload( item: ConclusionCreateParams | dict[str, Any], ) -> dict[str, Any]: """Build a single conclusion create payload.""" payload: dict[str, Any] = { - "observer_id": self._scope.observer, - "observed_id": self._scope.observed, + "observer_id": self._view.observer, + "observed_id": self._view.observed, } if isinstance(item, ConclusionCreateParams): payload["content"] = item.content @@ -1636,8 +1797,8 @@ class ConclusionScopeAio: conclusion_params = [build_conclusion_payload(c) for c in conclusions] - data = await self._scope._honcho._async_http_client.post( - routes.conclusions(self._scope.workspace_id), + data = await self._view._honcho._async_http_client.post( + routes.conclusions(self._view.workspace_id), body={"conclusions": conclusion_params}, ) return [ @@ -1654,8 +1815,8 @@ class ConclusionScopeAio: max_conclusions: int | None = None, ) -> str: """Get the computed representation for this scope asynchronously.""" - await self._scope._honcho._ensure_workspace_async() - body: dict[str, Any] = {"target": self._scope.observed} + await self._view._honcho._ensure_workspace_async() + body: dict[str, Any] = {"target": self._view.observed} if search_query is not None: body["search_query"] = search_query if search_top_k is not None: @@ -1667,9 +1828,99 @@ class ConclusionScopeAio: if max_conclusions is not None: body["max_conclusions"] = max_conclusions - data = await self._scope._honcho._async_http_client.post( - routes.peer_representation(self._scope.workspace_id, self._scope.observer), + data = await self._view._honcho._async_http_client.post( + routes.peer_representation(self._view.workspace_id, self._view.observer), body=body, ) response = RepresentationResponse.model_validate(data) return response.representation + + +class ScopeAio: + """ + Async view of a Scope. + + Access via `scope.aio`. Provides async versions of all Scope methods. + Shares state with the parent Scope instance. + """ + + __slots__: ClassVar[tuple[str, ...]] = ("_scope",) + _scope: "Scope" + + def __init__(self, scope: "Scope") -> None: + """Create an async view backed by a sync Scope.""" + self._scope = scope + + @validate_call(config=ConfigDict(arbitrary_types_allowed=True)) + async def add_sessions(self, sessions: Sequence[str | SessionBase]) -> None: + """Add sessions to this scope asynchronously. + + See Scope.add_sessions for details, including the asynchronous backfill + that sessions with existing messages trigger. + """ + session_ids = resolve_scope_membership(sessions) + await self._scope._honcho._ensure_workspace_async() + await self._scope._honcho._async_http_client.post( + routes.scope_sessions(self._scope.workspace_id, self._scope.id), + body={"session_ids": session_ids}, + ) + + @validate_call(config=ConfigDict(arbitrary_types_allowed=True)) + async def remove_session(self, session: str | SessionBase) -> None: + """Remove a session from this scope asynchronously. + + See Scope.remove_session for details on the asynchronous reconciliation. + """ + await self._scope._honcho._ensure_workspace_async() + await self._scope._honcho._async_http_client.delete( + routes.scope_session( + self._scope.workspace_id, self._scope.id, resolve_scope_session(session) + ) + ) + + async def sessions( + self, + page: int = 1, + size: int = 50, + *, + reverse: bool = False, + ) -> AsyncPage[SessionResponse, Session]: + """Get the sessions that are members of this scope asynchronously.""" + await self._scope._honcho._ensure_workspace_async() + + async def fetch(next_page: int) -> dict[str, Any]: + query: dict[str, Any] = {"page": next_page, "size": size} + if reverse: + query["reverse"] = "true" + return await self._scope._honcho._async_http_client.post( + routes.scope_sessions_list(self._scope.workspace_id, self._scope.id), + query=query, + ) + + def transform(response: SessionResponse) -> Session: + return Session( + response.id, + self._scope._honcho, + metadata=response.metadata, + configuration=response.configuration, + created_at=response.created_at, + is_active=response.is_active, + ) + + async def fetch_next(next_page: int) -> AsyncPage[SessionResponse, Session]: + return AsyncPage( + await fetch(next_page), SessionResponse, transform, fetch_next + ) + + return AsyncPage(await fetch(page), SessionResponse, transform, fetch_next) + + async def status(self) -> dict[str, ScopeBackfillJob]: + """Get the backfill/reconciliation progress for this scope asynchronously. + + See Scope.status for details. + """ + await self._scope._honcho._ensure_workspace_async() + data = await self._scope._honcho._async_http_client.get( + routes.scope_status(self._scope.workspace_id, self._scope.id) + ) + return ScopeStatusResponse.model_validate(data).backfill_status diff --git a/sdks/python/src/honcho/api_types.py b/sdks/python/src/honcho/api_types.py index f626c5a6..23692f2f 100644 --- a/sdks/python/src/honcho/api_types.py +++ b/sdks/python/src/honcho/api_types.py @@ -276,6 +276,7 @@ class SessionCreateParams(BaseModel): metadata: dict[str, Any] | None = None peers: dict[str, SessionPeerConfig] | None = None configuration: SessionConfiguration | None = None + scopes: list[str] | None = None class SessionUpdateParams(BaseModel): @@ -295,6 +296,44 @@ class SessionListParams(BaseModel): filters: dict[str, Any] | None = None +# ============================================================================== +# Scope Types +# ============================================================================== + + +class ScopeResponse(BaseModel): + """Scope API response.""" + + model_config = ConfigDict(populate_by_name=True) # pyright: ignore[reportUnannotatedClassAttribute] + + id: str + metadata: dict[str, Any] = Field(default_factory=dict) + created_at: datetime.datetime + + +class ScopeBackfillJob(BaseModel): + """Backfill job state for one session in a scope. + + ``docs_copied`` is present only once the backfill for that session completes. + """ + + model_config = ConfigDict(extra="ignore") # pyright: ignore[reportUnannotatedClassAttribute] + + state: Literal["pending", "completed", "failed"] + updated_at: datetime.datetime + docs_copied: int | None = None + + +class ScopeStatusResponse(BaseModel): + """Scope backfill/reconciliation status API response. + + ``backfill_status`` is keyed by session ID and only contains sessions that + have had a backfill enqueued. + """ + + backfill_status: dict[str, ScopeBackfillJob] = Field(default_factory=dict) + + # ============================================================================== # Summary Types # ============================================================================== diff --git a/sdks/python/src/honcho/base.py b/sdks/python/src/honcho/base.py index a27a64ad..fb80b121 100644 --- a/sdks/python/src/honcho/base.py +++ b/sdks/python/src/honcho/base.py @@ -43,3 +43,20 @@ class SessionBase(BaseModel): workspace_id: str = Field( ..., min_length=1, description="Workspace ID for scoping operations" ) + + +class ScopeBase(BaseModel): + """Base class for Scope objects (sync and async variants). + + Use this type in method signatures to accept either a scope ID string or any + Scope object. + + Attributes: + id: Unprefixed scope name, unique within the workspace + workspace_id: Workspace ID for scoping operations + """ + + id: str = Field(..., min_length=1, description="Unprefixed name of this scope") + workspace_id: str = Field( + ..., min_length=1, description="Workspace ID for scoping operations" + ) diff --git a/sdks/python/src/honcho/client.py b/sdks/python/src/honcho/client.py index 06a2887c..1527792e 100644 --- a/sdks/python/src/honcho/client.py +++ b/sdks/python/src/honcho/client.py @@ -4,7 +4,7 @@ from __future__ import annotations import logging import os -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from typing import Any, Literal import httpx @@ -16,20 +16,22 @@ from .api_types import ( PeerConfig, PeerResponse, QueueStatusResponse, + ScopeResponse, SessionConfiguration, SessionPeerConfig, SessionResponse, WorkspaceConfiguration, WorkspaceResponse, ) -from .base import PeerBase, SessionBase +from .base import PeerBase, ScopeBase, SessionBase from .http import AsyncHonchoHTTPClient, HonchoHTTPClient, routes from .message import Message from .mixins import MetadataConfigMixin from .pagination import SyncPage from .peer import Peer +from .scope import Scope from .session import Session -from .utils import normalize_peers_to_dict, resolve_id +from .utils import normalize_peers_to_dict, resolve_id, validate_scope_id logger = logging.getLogger(__name__) @@ -419,6 +421,10 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul None, description="Optional peers to attach to the session at creation. Accepts the same shape as Session.add_peers.", ), + scopes: Sequence[str | ScopeBase] | None = Field( + None, + description="Optional scopes this session should join. Each scope is created if it does not exist yet.", + ), ) -> Session: """ Get or create a session with the given ID. @@ -433,6 +439,11 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul peers: Optional peers to attach to the session at creation. Accepts the same shape as Session.add_peers (peer ID string, Peer object, list of either, or tuples with SessionPeerConfig). + scopes: Optional scopes this session should join, as IDs or Scope + objects. Each scope is created if it does not exist yet. Attaching + at creation avoids the asynchronous backfill a later + ``scope.add_sessions()`` triggers, since there is no history to + copy. Returns: A Session object with cached metadata, configuration, created_at, and is_active. @@ -445,6 +456,8 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul body["configuration"] = configuration.model_dump(exclude_none=True) if peers is not None: body["peers"] = normalize_peers_to_dict(peers) + if scopes is not None: + body["scopes"] = [validate_scope_id(resolve_id(scope)) for scope in scopes] data = self._http.post(routes.sessions(self.workspace_id), body=body) session_data = SessionResponse.model_validate(data) @@ -514,6 +527,97 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul return SyncPage(data, SessionResponse, transform, fetch_next) + @validate_call + def scope( + self, + id: str = Field( # noqa: A002 + ..., min_length=1, description="Unprefixed name for the scope" + ), + *, + metadata: dict[str, object] | None = Field( + None, + description="Optional metadata dictionary to associate with this scope.", + ), + ) -> Scope: + """ + Get or create a scope with the given ID. + + A scope is a named set of sessions that acts as a visibility boundary: + recall performed through the scope sees only what happened in its sessions, + while the underlying peer keeps its single unified representation of + everything. + + Args: + id: Unprefixed scope name, unique within the workspace. + metadata: Optional metadata dictionary to associate with this scope. + + Returns: + A Scope object for managing membership. + + Raises: + ValueError: If the scope ID is invalid. + + Example: + ```python + therapy = honcho.scope("therapy") + therapy.add_sessions([session_1, session_2]) + ``` + """ + validate_scope_id(id) + self._ensure_workspace() + body: dict[str, Any] = {"id": id} + if metadata is not None: + body["metadata"] = metadata + + data = self._http.post(routes.scopes(self.workspace_id), body=body) + scope_data = ScopeResponse.model_validate(data) + return Scope( + id, + self, + metadata=scope_data.metadata, + created_at=scope_data.created_at, + ) + + def scopes( + self, + *, + page: int = 1, + size: int = 50, + reverse: bool = False, + ) -> SyncPage[ScopeResponse, Scope]: + """ + Get all scopes in the current workspace. + + Args: + page: Page number (1-indexed). Default: 1. + size: Number of items per page. Default: 50. + reverse: If True, reverses the default ordering. Default: False. + + Returns: + A SyncPage of Scope objects representing all scopes in the workspace. + """ + self._ensure_workspace() + + def fetch(next_page: int) -> dict[str, Any]: + query: dict[str, Any] = {"page": next_page, "size": size} + if reverse: + query["reverse"] = "true" + return self._http.post(routes.scopes_list(self.workspace_id), query=query) + + def transform(scope: ScopeResponse) -> Scope: + """Convert a scope API response into a Scope SDK object.""" + return Scope( + scope.id, + self, + metadata=scope.metadata, + created_at=scope.created_at, + ) + + def fetch_next(next_page: int) -> SyncPage[ScopeResponse, Scope]: + return SyncPage(fetch(next_page), ScopeResponse, transform, fetch_next) + + return SyncPage(fetch(page), ScopeResponse, transform, fetch_next) + def workspaces( self, filters: dict[str, object] | None = None, @@ -592,6 +696,11 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul limit: int = Field( default=10, ge=1, le=100, description="Number of results to return" ), + *, + scope: str | ScopeBase | None = Field( + None, + description="Optional scope restricting the search to its member sessions", + ), ) -> list[Message]: """ Search for messages in the current workspace. @@ -602,15 +711,22 @@ class Honcho(BaseModel, MetadataConfigMixin): # pyright: ignore[reportUnsafeMul query: The search query to use filters: Filters to scope the search. See [search filters documentation](https://honcho.dev/docs/v3/documentation/core-concepts/features/using-filters). limit: Number of results to return (1-100, default: 10) + scope: Optional scope (ID or Scope object) restricting the search to + that scope's member sessions. Mutually exclusive with a + ``session_id`` filter. A scope with no member sessions matches + nothing rather than everything. Returns: A list of Message objects representing the search results. Returns an empty list if no messages are found. """ self._ensure_workspace() + body: dict[str, Any] = {"query": query, "filters": filters, "limit": limit} + if scope is not None: + body["scope"] = validate_scope_id(resolve_id(scope)) data = self._http.post( routes.workspace_search(self.workspace_id), - body={"query": query, "filters": filters, "limit": limit}, + body=body, ) return [ Message.from_api_response(MessageResponse.model_validate(item)) diff --git a/sdks/python/src/honcho/conclusions.py b/sdks/python/src/honcho/conclusions.py index 708cc3ed..c9f6a6a9 100644 --- a/sdks/python/src/honcho/conclusions.py +++ b/sdks/python/src/honcho/conclusions.py @@ -15,28 +15,28 @@ from .pagination import SyncPage from .utils import resolve_id if TYPE_CHECKING: - from .aio import ConclusionScopeAio + from .aio import ConclusionsViewAio from .client import Honcho __all__ = [ "Conclusion", - "ConclusionScope", + "ConclusionsView", "ConclusionCreateParams", ] -# Filter keys that define a conclusion scope (the observer/observed peer pair). -# They are set from the scope itself, so a caller must not pass them in `filters`. -_SCOPE_RESERVED = ("observer", "observed", "observer_id", "observed_id") +# Filter keys that define a conclusions view (the observer/observed peer pair). +# They are set from the view itself, so a caller must not pass them in `filters`. +_VIEW_RESERVED = ("observer", "observed", "observer_id", "observed_id") def _reject_reserved_filter_keys( filters: dict[str, Any] | None, reserved: tuple[str, ...] ) -> None: - """Raise if ``filters`` contains keys managed by the conclusion scope. + """Raise if ``filters`` contains keys managed by the conclusions view. The observer/observed peer pair (and, on ``list``, the session) is fixed by - the scope, so letting a user filter override it would silently return data - from a different scope than requested. Fail loud instead. + the view, so letting a user filter override it would silently return data + from a different pair than requested. Fail loud instead. """ if not filters: return @@ -48,7 +48,7 @@ def _reject_reserved_filter_keys( if "session" in reserved or "session_id" in reserved: guidance += "; use the session= parameter to filter by session" raise ValueError( - f"Filter key(s) {clash} are managed by this conclusion scope and " + f"Filter key(s) {clash} are managed by this conclusions view and " + f"cannot be passed in filters. {guidance}." ) @@ -126,7 +126,7 @@ class Conclusion: return self.content -class ConclusionScope: +class ConclusionsView: """ Scoped access to conclusions for a specific observer/observed relationship. @@ -165,7 +165,7 @@ class ConclusionScope: observed: str, ): """ - Initialize a ConclusionScope. + Initialize a ConclusionsView. Args: honcho: The Honcho client instance @@ -179,12 +179,12 @@ class ConclusionScope: self.observed = observed @property - def aio(self) -> "ConclusionScopeAio": + def aio(self) -> "ConclusionsViewAio": """ - Access async versions of all ConclusionScope methods. + Access async versions of all ConclusionsView methods. - Returns a ConclusionScopeAio view that provides async versions of all methods - while sharing state with this ConclusionScope instance. + Returns a ConclusionsViewAio view that provides async versions of all methods + while sharing state with this ConclusionsView instance. Example: ```python @@ -194,9 +194,9 @@ class ConclusionScope: ``` """ # Import here to avoid circular import (aio.py imports from this module) - from .aio import ConclusionScopeAio + from .aio import ConclusionsViewAio - return ConclusionScopeAio(self) + return ConclusionsViewAio(self) def list( self, @@ -226,7 +226,7 @@ class ConclusionScope: Paginated response containing Conclusion objects """ _reject_reserved_filter_keys( - filters, _SCOPE_RESERVED + ("session", "session_id") + filters, _VIEW_RESERVED + ("session", "session_id") ) self._honcho._ensure_workspace() resolved_session_id = resolve_id(session) @@ -288,7 +288,7 @@ class ConclusionScope: Returns: List of matching Conclusion objects """ - _reject_reserved_filter_keys(filters, _SCOPE_RESERVED) + _reject_reserved_filter_keys(filters, _VIEW_RESERVED) self._honcho._ensure_workspace() filters = { "observer_id": self.observer, @@ -443,6 +443,6 @@ class ConclusionScope: def __repr__(self) -> str: return ( - f"ConclusionScope(workspace_id={self.workspace_id!r}, " + f"ConclusionsView(workspace_id={self.workspace_id!r}, " f"observer={self.observer!r}, observed={self.observed!r})" ) diff --git a/sdks/python/src/honcho/http/routes.py b/sdks/python/src/honcho/http/routes.py index 3fdbd677..8f9ee2fb 100644 --- a/sdks/python/src/honcho/http/routes.py +++ b/sdks/python/src/honcho/http/routes.py @@ -102,6 +102,31 @@ def session_peer_config(workspace_id: str, session_id: str, peer_id: str) -> str return f"/{API_VERSION}/workspaces/{workspace_id}/sessions/{session_id}/peers/{peer_id}/config" +# Scope routes +def scopes(workspace_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes" + + +def scopes_list(workspace_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes/list" + + +def scope_sessions(workspace_id: str, scope_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes/{scope_id}/sessions" + + +def scope_sessions_list(workspace_id: str, scope_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes/{scope_id}/sessions/list" + + +def scope_session(workspace_id: str, scope_id: str, session_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes/{scope_id}/sessions/{session_id}" + + +def scope_status(workspace_id: str, scope_id: str) -> str: + return f"/{API_VERSION}/workspaces/{workspace_id}/scopes/{scope_id}/status" + + # Message routes def messages(workspace_id: str, session_id: str) -> str: return f"/{API_VERSION}/workspaces/{workspace_id}/sessions/{session_id}/messages" diff --git a/sdks/python/src/honcho/peer.py b/sdks/python/src/honcho/peer.py index e004db6a..38edf269 100644 --- a/sdks/python/src/honcho/peer.py +++ b/sdks/python/src/honcho/peer.py @@ -6,7 +6,7 @@ from __future__ import annotations import datetime import logging import warnings -from collections.abc import Generator +from collections.abc import Generator, Sequence from typing import TYPE_CHECKING, Any, Literal, TypeVar, overload from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, validate_call @@ -22,14 +22,14 @@ from .api_types import ( SessionConfiguration, SessionResponse, ) -from .base import PeerBase, SessionBase -from .conclusions import ConclusionScope +from .base import PeerBase, ScopeBase, SessionBase +from .conclusions import ConclusionsView from .http import routes from .message import Message from .mixins import MetadataConfigMixin from .pagination import SyncPage from .types import DialecticStreamResponse -from .utils import parse_datetime, parse_sse_stream, resolve_id +from .utils import parse_datetime, parse_sse_stream, resolve_id, scope_recall_fields if TYPE_CHECKING: from .aio import PeerAio @@ -241,6 +241,8 @@ class Peer(PeerBase, MetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[TResponseFormat], @@ -253,6 +255,8 @@ class Peer(PeerBase, MetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: dict[str, Any] | None = None, @@ -265,6 +269,8 @@ class Peer(PeerBase, MetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[BaseModel] | dict[str, Any] | None = None, @@ -285,6 +291,19 @@ class Peer(PeerBase, MetadataConfigMixin): session: Optional session to scope the query to. If provided, only information from that session is considered. Can be a session ID string or a Session object. + scope: Optional scope(s) to confine the query to. A single scope answers + from that scope's own view of the target, including the + higher-order conclusions reasoned within it. A sequence of scopes + restricts recall to the union of their member sessions, which — + like ``sessions`` — yields only directly-stated conclusions. + Mutually exclusive with ``session`` and ``sessions``, and requires + a workspace-level key. + sessions: Optional allowlist of sessions to confine the query to, for + one-off questions spanning a handful of sessions. Recall is + limited to conclusions stated directly in those sessions: + conclusions produced by reasoning across sessions are excluded, + because their provenance cannot be proven to sit inside the + allowlist. Reach for a named ``scope`` when you need that depth. reasoning_level: Optional reasoning level for the query: "minimal", "low", "medium", "high", or "max". Defaults to "low" if not provided. response_format: Optional structure for the answer. Pass a Pydantic @@ -296,12 +315,20 @@ class Peer(PeerBase, MetadataConfigMixin): Response string containing the answer (a JSON string when a schema dict was given), a parsed model instance when a Pydantic model class was given, or None if no relevant information. + + Raises: + ValueError: If ``scope`` is combined with ``session`` or ``sessions``. """ self._honcho._ensure_workspace() target_id = resolve_id(target) resolved_session_id = resolve_id(session) body: dict[str, Any] = {"query": query, "stream": False} + body.update( + scope_recall_fields( + scope=scope, sessions=sessions, session_id=resolved_session_id + ) + ) if target_id: body["target"] = target_id if resolved_session_id: @@ -330,6 +357,8 @@ class Peer(PeerBase, MetadataConfigMixin): *, target: str | PeerBase | None = None, session: str | SessionBase | None = None, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, reasoning_level: Literal["minimal", "low", "medium", "high", "max"] | None = None, response_format: type[BaseModel] | dict[str, Any] | None = None, @@ -350,6 +379,9 @@ class Peer(PeerBase, MetadataConfigMixin): session: Optional session to scope the query to. If provided, only information from that session is considered. Can be a session ID string or a Session object. + scope: Optional scope(s) to confine the query to. See :meth:`chat`. + sessions: Optional allowlist of sessions to confine the query to. See + :meth:`chat` for the depth caveat. reasoning_level: Optional reasoning level for the query: "minimal", "low", "medium", "high", or "max". Defaults to "low" if not provided. response_format: Optional structure for the answer: a Pydantic model @@ -360,12 +392,20 @@ class Peer(PeerBase, MetadataConfigMixin): Returns: DialecticStreamResponse object that can be iterated over and provides final response + + Raises: + ValueError: If ``scope`` is combined with ``session`` or ``sessions``. """ self._honcho._ensure_workspace() target_id = resolve_id(target) resolved_session_id = resolve_id(session) body: dict[str, Any] = {"query": query, "stream": True} + body.update( + scope_recall_fields( + scope=scope, sessions=sessions, session_id=resolved_session_id + ) + ) if target_id: body["target"] = target_id if resolved_session_id: @@ -633,6 +673,9 @@ class Peer(PeerBase, MetadataConfigMixin): search_max_distance: float | None = Field(None, ge=0.0, le=1.0), include_most_frequent: bool | None = None, max_conclusions: int | None = Field(None, ge=1, le=100), + *, + scope: str | ScopeBase | Sequence[str | ScopeBase] | None = None, + sessions: Sequence[str | SessionBase] | None = None, ) -> str: """ Get a subset of the representation of the peer. @@ -646,10 +689,18 @@ class Peer(PeerBase, MetadataConfigMixin): search_max_distance: Maximum semantic distance for search results (0.0-1.0) include_most_frequent: Whether to include the most frequent conclusions max_conclusions: Maximum number of conclusions to include + scope: Optional scope(s) confining the representation. See + :meth:`chat`. Mutually exclusive with ``session`` and ``sessions``. + sessions: Optional allowlist of sessions confining the representation to + directly-stated conclusions from those sessions. See + :meth:`chat` for the depth caveat. Returns: A Representation string + Raises: + ValueError: If ``scope`` is combined with ``session`` or ``sessions``. + Example: ```python # Get global representation @@ -671,7 +722,9 @@ class Peer(PeerBase, MetadataConfigMixin): session_id = resolve_id(session) target_id = resolve_id(target) - body: dict[str, Any] = {} + body: dict[str, Any] = scope_recall_fields( + scope=scope, sessions=sessions, session_id=session_id + ) if session_id: body["session_id"] = session_id if target_id: @@ -764,7 +817,7 @@ class Peer(PeerBase, MetadataConfigMixin): return PeerContextResponse.model_validate(data) @property - def conclusions(self) -> ConclusionScope: + def conclusions(self) -> ConclusionsView: """ Access this peer's self-conclusions (where observer == observed == self). @@ -772,7 +825,7 @@ class Peer(PeerBase, MetadataConfigMixin): has made about themselves. Use this for self-conclusion scenarios. Returns: - A ConclusionScope scoped to this peer's self-conclusions + A ConclusionsView scoped to this peer's self-conclusions Example: ```python @@ -786,9 +839,9 @@ class Peer(PeerBase, MetadataConfigMixin): peer.conclusions.delete("obs-123") ``` """ - return ConclusionScope(self._honcho, self.workspace_id, self.id, self.id) + return ConclusionsView(self._honcho, self.workspace_id, self.id, self.id) - def conclusions_of(self, target: str | PeerBase) -> ConclusionScope: + def conclusions_of(self, target: str | PeerBase) -> ConclusionsView: """ Access conclusions this peer has made about another peer. @@ -799,7 +852,7 @@ class Peer(PeerBase, MetadataConfigMixin): target: The target peer (either a Peer object or peer ID string) Returns: - A ConclusionScope scoped to this peer's conclusions of the target + A ConclusionsView scoped to this peer's conclusions of the target Example: ```python @@ -817,7 +870,7 @@ class Peer(PeerBase, MetadataConfigMixin): ``` """ target_id = target.id if isinstance(target, PeerBase) else target - return ConclusionScope(self._honcho, self.workspace_id, self.id, target_id) + return ConclusionsView(self._honcho, self.workspace_id, self.id, target_id) def __repr__(self) -> str: """ diff --git a/sdks/python/src/honcho/scope.py b/sdks/python/src/honcho/scope.py new file mode 100644 index 00000000..14a5752b --- /dev/null +++ b/sdks/python/src/honcho/scope.py @@ -0,0 +1,233 @@ +# pyright: reportPrivateUsage=false +"""Sync Scope class for Honcho SDK.""" + +from __future__ import annotations + +import logging +from collections.abc import Sequence +from datetime import datetime +from typing import TYPE_CHECKING, Any + +from pydantic import ConfigDict, PrivateAttr, validate_call + +from .api_types import ScopeBackfillJob, ScopeStatusResponse, SessionResponse +from .base import ScopeBase, SessionBase +from .http import routes +from .pagination import SyncPage +from .session import Session +from .utils import resolve_scope_membership, resolve_scope_session + +if TYPE_CHECKING: + from .aio import ScopeAio + from .client import Honcho + +logger = logging.getLogger(__name__) + +__all__ = ["Scope"] + + +class Scope(ScopeBase): + """ + Represents a scope in Honcho. + + A scope is a named set of sessions that acts as a visibility boundary. Recall + performed through a scope sees only what happened in that scope's sessions, + while the underlying peer keeps its single unified representation across + everything it has ever participated in. + + Membership changes are applied asynchronously: adding a session that already + has messages copies its existing conclusions into the scope, and removing one + reconciles them back out. Poll :meth:`status` to watch that settle. + + Attributes: + id: Unprefixed scope name, unique within the workspace + workspace_id: Workspace ID for scoping operations + metadata: Cached metadata for this scope. May be stale if not recently + fetched. + created_at: When this scope was created, if known + + Example: + ```python + therapy = honcho.scope("therapy") + therapy.add_sessions([session_1, session_2]) + + # Ask a question answered only from the therapy sessions + answer = user.chat("What is stressing them out?", scope="therapy") + ``` + """ + + _metadata: dict[str, Any] | None = PrivateAttr(default=None) + _created_at: datetime | None = PrivateAttr(default=None) + _honcho: "Honcho" = PrivateAttr() + + @property + def metadata(self) -> dict[str, Any] | None: + """Cached metadata for this scope. May be stale if not recently fetched.""" + return self._metadata + + @property + def created_at(self) -> datetime | None: + """When this scope was created. Only available if fetched from the API.""" + return self._created_at + + def __init__( + self, + scope_id: str, + honcho: "Honcho", + *, + metadata: dict[str, Any] | None = None, + created_at: datetime | None = None, + ) -> None: + """ + Initialize a new Scope. + + **Do not call this directly — use** ``honcho.scope()``. + + Args: + scope_id: Unprefixed scope name, unique within the workspace + honcho: Honcho client instance + metadata: Cached metadata, if already fetched + created_at: Creation timestamp, if already fetched + """ + super().__init__( + id=scope_id, + workspace_id=honcho.workspace_id, + ) + self._honcho = honcho + self._metadata = metadata + self._created_at = created_at + + @property + def aio(self) -> "ScopeAio": + """ + Access async versions of all Scope methods. + + Returns a ScopeAio view that provides async versions of all methods while + sharing state with this Scope instance. + + Example: + ```python + await scope.aio.add_sessions(["session-1"]) + status = await scope.aio.status() + ``` + """ + # Import here to avoid circular import (aio.py imports this module) + from .aio import ScopeAio + + return ScopeAio(self) + + @validate_call(config=ConfigDict(arbitrary_types_allowed=True)) + def add_sessions(self, sessions: Sequence[str | SessionBase]) -> None: + """ + Add sessions to this scope. + + Every named session must already exist. Adding a session that is already + a member is a no-op. + + Sessions that already hold messages are backfilled into the scope + asynchronously, so recall through this scope may not reflect their history + immediately — poll :meth:`status` to watch that complete. + + Args: + sessions: Sessions to add, as ID strings or Session objects. At most + 100 per call, matching the server's limit; split larger membership + changes into separate calls so a failure names the batch that + failed. + + Raises: + ValueError: If no sessions are given, or more than 100. + """ + session_ids = resolve_scope_membership(sessions) + self._honcho._ensure_workspace() + self._honcho._http.post( + routes.scope_sessions(self.workspace_id, self.id), + body={"session_ids": session_ids}, + ) + + @validate_call(config=ConfigDict(arbitrary_types_allowed=True)) + def remove_session(self, session: str | SessionBase) -> None: + """ + Remove a session from this scope. + + Conclusions copied or derived while the session was a member are + reconciled out asynchronously, and the scope's peer card is rebuilt from + whatever evidence remains. Poll :meth:`status` to watch that settle. + + Args: + session: Session to remove, as an ID string or a Session object + """ + self._honcho._ensure_workspace() + self._honcho._http.delete( + routes.scope_session( + self.workspace_id, self.id, resolve_scope_session(session) + ) + ) + + def sessions( + self, + page: int = 1, + size: int = 50, + *, + reverse: bool = False, + ) -> SyncPage[SessionResponse, Session]: + """ + Get the sessions that are members of this scope. + + Ordered by how long each session has been a member — longest-standing + first, or most recently added first when ``reverse`` is True. + + Args: + page: Page number (1-indexed) + size: Number of results per page + reverse: If True, reverses the default ordering. Default: False. + + Returns: + Paginated response containing Session objects + """ + self._honcho._ensure_workspace() + + def fetch(next_page: int) -> dict[str, Any]: + query: dict[str, Any] = {"page": next_page, "size": size} + if reverse: + query["reverse"] = "true" + return self._honcho._http.post( + routes.scope_sessions_list(self.workspace_id, self.id), + query=query, + ) + + def transform(response: SessionResponse) -> Session: + return Session( + response.id, + self._honcho, + metadata=response.metadata, + configuration=response.configuration, + created_at=response.created_at, + is_active=response.is_active, + ) + + def fetch_next(next_page: int) -> SyncPage[SessionResponse, Session]: + return SyncPage(fetch(next_page), SessionResponse, transform, fetch_next) + + return SyncPage(fetch(page), SessionResponse, transform, fetch_next) + + def status(self) -> dict[str, ScopeBackfillJob]: + """ + Get the backfill/reconciliation progress for this scope. + + Use this after a membership change to tell "the scope knows nothing about + that session yet" apart from "the scope has caught up and there is + genuinely nothing to recall". + + Returns: + Per-session backfill state, keyed by session ID. Only sessions that + have had a backfill enqueued appear; an empty dict means none have. + """ + self._honcho._ensure_workspace() + data = self._honcho._http.get(routes.scope_status(self.workspace_id, self.id)) + return ScopeStatusResponse.model_validate(data).backfill_status + + def __repr__(self) -> str: + return f"Scope(id={self.id!r}, workspace_id={self.workspace_id!r})" + + def __str__(self) -> str: + return self.id diff --git a/sdks/python/src/honcho/session.py b/sdks/python/src/honcho/session.py index 3b1b0f14..2f1838c9 100644 --- a/sdks/python/src/honcho/session.py +++ b/sdks/python/src/honcho/session.py @@ -5,6 +5,7 @@ from __future__ import annotations import json import logging +from collections.abc import Sequence from datetime import datetime from typing import TYPE_CHECKING, Any @@ -20,7 +21,7 @@ from .api_types import ( SessionPeerConfig, SessionResponse, ) -from .base import PeerBase, SessionBase +from .base import PeerBase, ScopeBase, SessionBase from .http import routes from .message import Message from .mixins import MetadataConfigMixin @@ -32,6 +33,7 @@ from .utils import ( normalize_peers_to_dict, prepare_file_for_upload, resolve_id, + scope_context_fields, ) if TYPE_CHECKING: @@ -574,6 +576,14 @@ class Session(SessionBase, MetadataConfigMixin): None, description="A peer ID to get context *from the perspective of*. If given, response will attempt to include representation and card from the perspective of `peer_perspective`. Must be provided with `peer_target`.", ), + scope: str | ScopeBase | None = Field( + None, + description="A scope to use as the perspective source instead of a peer: `peer_target`'s representation and card are read from what that scope observed. Must be provided with `peer_target`; mutually exclusive with `peer_perspective`. Requires a workspace-level key.", + ), + sessions: Sequence[str | SessionBase] | None = Field( + None, + description="An allowlist of sessions confining `peer_target`'s representation to that set. This session must be one of them. Mutually exclusive with `scope` and `limit_to_session`.", + ), limit_to_session: bool = Field( False, description="Whether to limit the representation to this session only. If True, only conclusions from this session will be included.", @@ -616,6 +626,11 @@ class Session(SessionBase, MetadataConfigMixin): peer_target: A peer ID to get context for. search_query: A query string for semantic search. peer_perspective: A peer ID to get context from the perspective of. + scope: A scope to read `peer_target`'s representation and card from. + sessions: An allowlist of sessions confining `peer_target`'s + representation. Recall is limited to conclusions stated directly in + those sessions, and the peer card is omitted, since neither derived + conclusions nor cards carry provable per-session provenance. limit_to_session: Whether to limit the representation to this session only. search_top_k: Number of semantically relevant facts to return. search_max_distance: Maximum semantic distance for search results. @@ -627,6 +642,11 @@ class Session(SessionBase, MetadataConfigMixin): summary, if available, that maximizes conversational context while respecting the token limit + Raises: + ValueError: If `peer_target` is missing when required, or if `scope`, + `sessions`, `peer_perspective`, and `limit_to_session` are combined + in ways the server rejects. + Note: Token counting is performed using tiktoken. For models using different tokenizers, you may need to adjust the token limit accordingly. @@ -650,6 +670,13 @@ class Session(SessionBase, MetadataConfigMixin): query: dict[str, Any] = { "summary": summary, "limit_to_session": limit_to_session, + **scope_context_fields( + scope=scope, + sessions=sessions, + peer_target=peer_target, + peer_perspective=peer_perspective, + limit_to_session=limit_to_session, + ), } if tokens is not None: query["tokens"] = tokens diff --git a/sdks/python/src/honcho/utils/__init__.py b/sdks/python/src/honcho/utils/__init__.py index aac9221b..90810763 100644 --- a/sdks/python/src/honcho/utils/__init__.py +++ b/sdks/python/src/honcho/utils/__init__.py @@ -6,6 +6,15 @@ from .datetime import datetime_to_iso, parse_datetime from .file_upload import normalize_file_input, prepare_file_for_upload from .peers import normalize_peers_to_dict from .resolve import resolve_id +from .scopes import ( + resolve_scope_membership, + resolve_scope_option, + resolve_scope_session, + resolve_session_allowlist, + scope_context_fields, + scope_recall_fields, + validate_scope_id, +) from .sse import SSEStreamParser, parse_sse_astream, parse_sse_chunk, parse_sse_stream __all__ = [ @@ -19,4 +28,11 @@ __all__ = [ "parse_sse_stream", "prepare_file_for_upload", "resolve_id", + "resolve_scope_membership", + "resolve_scope_option", + "resolve_scope_session", + "resolve_session_allowlist", + "scope_context_fields", + "scope_recall_fields", + "validate_scope_id", ] diff --git a/sdks/python/src/honcho/utils/resolve.py b/sdks/python/src/honcho/utils/resolve.py index db70fc01..30eaf811 100644 --- a/sdks/python/src/honcho/utils/resolve.py +++ b/sdks/python/src/honcho/utils/resolve.py @@ -5,7 +5,7 @@ from __future__ import annotations from typing import TYPE_CHECKING, overload if TYPE_CHECKING: - from ..base import PeerBase, SessionBase + from ..base import PeerBase, ScopeBase, SessionBase @overload @@ -17,12 +17,12 @@ def resolve_id(obj: str) -> str: ... @overload -def resolve_id(obj: "PeerBase | SessionBase") -> str: ... +def resolve_id(obj: "PeerBase | SessionBase | ScopeBase") -> str: ... -def resolve_id(obj: "str | PeerBase | SessionBase | None") -> str | None: +def resolve_id(obj: "str | PeerBase | SessionBase | ScopeBase | None") -> str | None: """ - Resolve an ID from a string, PeerBase, SessionBase, or None. + Resolve an ID from a string, PeerBase, SessionBase, ScopeBase, or None. This utility function extracts the ID from an object that may be: - A string (returned as-is) diff --git a/sdks/python/src/honcho/utils/scopes.py b/sdks/python/src/honcho/utils/scopes.py new file mode 100644 index 00000000..4a0f6ef8 --- /dev/null +++ b/sdks/python/src/honcho/utils/scopes.py @@ -0,0 +1,296 @@ +"""Scope and session-allowlist option handling for the Honcho Python SDK. + +The ``scope`` and ``sessions`` options appear on several read surfaces (chat, +representation, session context, search). Their validation and their wire +translation live here so those surfaces cannot drift apart — the server enforces +the same exclusions with a 422, and this raises before the round trip. +""" + +from __future__ import annotations + +import re +from collections.abc import Sequence +from typing import TYPE_CHECKING, Any + +from .resolve import resolve_id + +if TYPE_CHECKING: + from ..base import ScopeBase, SessionBase + +__all__ = [ + "MAX_SCOPES_PER_OPTION", + "MAX_SESSIONS_PER_ADD", + "MAX_SESSION_ALLOWLIST_ENTRIES", + "resolve_scope_membership", + "resolve_scope_option", + "resolve_scope_session", + "resolve_session_allowlist", + "scope_context_fields", + "scope_recall_fields", + "validate_scope_id", +] + +# Scope IDs are stored server-side as peer names with this prefix prepended, so +# they must leave room for it within the 512-character peer name limit. +_SCOPE_PEER_PREFIX = "scope." +MAX_SCOPE_ID_LENGTH = 512 - len(_SCOPE_PEER_PREFIX) + +MAX_SCOPES_PER_OPTION = 100 +MAX_SESSION_ALLOWLIST_ENTRIES = 1000 +# The server accepts at most this many sessions per membership call. +MAX_SESSIONS_PER_ADD = 100 + +_RESOURCE_NAME_PATTERN = r"^[a-zA-Z0-9_-]+$" + + +def validate_scope_id(value: str) -> str: + """Validate an unprefixed scope ID. + + Args: + value: The scope ID as the caller supplied it. + + Returns: + The validated scope ID, unchanged. + + Raises: + ValueError: If the ID is empty, too long, carries the reserved prefix, + or contains characters outside the resource-name charset. + """ + if not 1 <= len(value) <= MAX_SCOPE_ID_LENGTH: + raise ValueError( + f"Scope ID must be between 1 and {MAX_SCOPE_ID_LENGTH} characters" + ) + # Checked before the charset: the reserved prefix contains '.', which is + # itself outside the charset, so a charset-first check would report the + # charset instead of the real mistake for a double-prefixed ID. + if value.startswith(_SCOPE_PEER_PREFIX): + raise ValueError( + f"Scope ID must not start with the reserved prefix '{_SCOPE_PEER_PREFIX}' (scope IDs are unprefixed)" + ) + if not re.fullmatch(_RESOURCE_NAME_PATTERN, value): + raise ValueError(f"Scope ID must match pattern {_RESOURCE_NAME_PATTERN}") + return value + + +def resolve_scope_option( + scope: "str | ScopeBase | Sequence[str | ScopeBase]", +) -> str | list[str]: + """Resolve the ``scope`` read option to its wire value. + + A single scope stays a string; a sequence becomes a list of IDs. The two + shapes mean different things to the server — one scope reads that scope's own + view, a list restricts recall to the union of their member sessions — so the + distinction is preserved rather than normalized away. + + Args: + scope: One scope (ID or ``Scope``) or a sequence of them. + + Returns: + A single validated scope ID, or a list of them. + + Raises: + ValueError: On an empty sequence, an over-cap sequence, or an invalid ID. + """ + # ``str`` is itself a Sequence, so both single-scope forms — an ID and a + # ``Scope`` — are taken first; whatever remains is the list form. + if isinstance(scope, str) or not isinstance(scope, Sequence): + return validate_scope_id(resolve_id(scope)) + + ids = [validate_scope_id(resolve_id(entry)) for entry in scope] + if not ids: + # An empty list would resolve to an empty allowlist server-side and + # silently recall nothing, which is never the intent. + raise ValueError("scope must name at least one scope") + if len(ids) > MAX_SCOPES_PER_OPTION: + raise ValueError(f"scope can name at most {MAX_SCOPES_PER_OPTION} scopes") + return ids + + +def resolve_session_allowlist( + sessions: "Sequence[str | SessionBase]", +) -> list[str]: + """Resolve the ``sessions`` allowlist option to a list of session IDs. + + Args: + sessions: Sessions to allow, as IDs or ``Session`` objects. + + Returns: + The session IDs, in the order given. + + Raises: + ValueError: On an empty list or one over the server's cap. + """ + ids = [resolve_id(entry) for entry in sessions] + if not ids: + # The server treats an empty allowlist as fail-closed (recalls nothing), + # so an empty list here is a caller mistake rather than a query. + raise ValueError("sessions must name at least one session") + if len(ids) > MAX_SESSION_ALLOWLIST_ENTRIES: + raise ValueError( + f"sessions can name at most {MAX_SESSION_ALLOWLIST_ENTRIES} sessions" + ) + return ids + + +def _validate_session_id(value: str) -> str: + """Validate a session ID against the charset the server accepts. + + Args: + value: The session ID as the caller supplied it. + + Returns: + The validated session ID, unchanged. + + Raises: + ValueError: If the ID is empty or contains characters outside the + resource-name charset. + """ + if not value: + raise ValueError("Session ID must be a non-empty string") + if not re.fullmatch(_RESOURCE_NAME_PATTERN, value): + raise ValueError(f"Session ID must match pattern {_RESOURCE_NAME_PATTERN}") + return value + + +def resolve_scope_session(session: "str | SessionBase") -> str: + """Resolve and validate a single session ID for a scope membership change. + + Validated rather than passed through because this ID is interpolated into a + request *path*: an unvalidated value silently changes which resource the + request addresses. ``valid-session?typo`` would target ``valid-session`` + with a stray query string, removing the wrong session from the scope and + triggering reconciliation against it. + + Args: + session: The session, as an ID or a ``Session`` object. + + Returns: + The validated session ID. + + Raises: + ValueError: If the ID is empty or malformed. + """ + return _validate_session_id(resolve_id(session)) + + +def resolve_scope_membership( + sessions: "Sequence[str | SessionBase]", +) -> list[str]: + """Resolve a scope membership change to a list of session IDs. + + Capped at the server's per-call limit rather than silently chunking, so a + rejected batch is the batch the caller passed. + + Args: + sessions: Sessions to add, as IDs or ``Session`` objects. + + Returns: + The session IDs, in the order given. + + Raises: + ValueError: On an empty list, one over the server's per-call cap, or a + malformed session ID. + """ + ids = [_validate_session_id(resolve_id(session)) for session in sessions] + if not ids: + raise ValueError("At least one session must be given") + if len(ids) > MAX_SESSIONS_PER_ADD: + raise ValueError( + f"At most {MAX_SESSIONS_PER_ADD} sessions can be added per call" + ) + return ids + + +def scope_context_fields( + *, + scope: "str | ScopeBase | None", + sessions: "Sequence[str | SessionBase] | None", + peer_target: str | None, + peer_perspective: str | None, + limit_to_session: bool, +) -> dict[str, Any]: + """Build the query fields for ``scope``/``sessions`` on the context route. + + Unlike the recall endpoints, session context takes these as query parameters + — ``sessions`` is sent as a repeated parameter, not as a ``filters`` body. + + Only a single scope is accepted: a scope is the *perspective source* for the + target's representation and card, which is one observer, so a list has no + meaning here. + + Args: + scope: The ``scope`` option, if given. + sessions: The ``sessions`` allowlist option, if given. + peer_target: The observed peer. Required by either option, since both only + reach the representation and there is none without a target. + peer_perspective: The observing peer, if given — a scope replaces it. + limit_to_session: Whether recall is already pinned to this session alone. + + Returns: + The fields to merge into the query. Empty when neither option is set. + + Raises: + ValueError: If either option is combined with something it contradicts, or + used without ``peer_target``. + """ + # A scope already determines what the context can see, and limit_to_session + # already pins recall to this session alone, so combining them with a + # perspective or an allowlist is a contradiction rather than a narrowing. + # Raised here so the caller does not pay a round trip for a 422. + if sessions is not None: + if scope is not None: + raise ValueError("`sessions` and `scope` are mutually exclusive") + if limit_to_session: + raise ValueError("`sessions` and `limit_to_session` are mutually exclusive") + if peer_target is None: + raise ValueError( + "You must provide a `peer_target` when `sessions` is provided" + ) + return {"sessions": resolve_session_allowlist(sessions)} + + if scope is None: + return {} + + if peer_perspective is not None: + raise ValueError("`scope` and `peer_perspective` are mutually exclusive") + if peer_target is None: + raise ValueError("You must provide a `peer_target` when `scope` is provided") + return {"scope": validate_scope_id(resolve_id(scope))} + + +def scope_recall_fields( + *, + scope: "str | ScopeBase | Sequence[str | ScopeBase] | None", + sessions: "Sequence[str | SessionBase] | None", + session_id: str | None = None, +) -> dict[str, Any]: + """Build the request-body fields for the ``scope``/``sessions`` options. + + ``sessions`` is sugar: it goes on the wire as the constrained + ``filters: {"session_id": [...]}`` body the recall endpoints accept, never as + a field of its own, which the server would reject as an unknown key. + + Args: + scope: The ``scope`` option, if given. + sessions: The ``sessions`` allowlist option, if given. + session_id: A single session already set on the request, if any — a scope + already determines what can be seen, so the two conflict. + + Returns: + The fields to merge into the request body. Empty when neither option is + set. + + Raises: + ValueError: If ``scope`` is combined with ``sessions`` or ``session_id``, + or if either option is itself invalid. + """ + if scope is None: + if sessions is None: + return {} + return {"filters": {"session_id": resolve_session_allowlist(sessions)}} + + if sessions is not None: + raise ValueError("`scope` and `sessions` are mutually exclusive") + if session_id is not None: + raise ValueError("`scope` and `session` are mutually exclusive") + return {"scope": resolve_scope_option(scope)} diff --git a/sdks/typescript/__tests__/conclusions.test.ts b/sdks/typescript/__tests__/conclusions.test.ts index 7716d3ac..7beadc34 100644 --- a/sdks/typescript/__tests__/conclusions.test.ts +++ b/sdks/typescript/__tests__/conclusions.test.ts @@ -1,7 +1,7 @@ /** * Conclusions Tests * - * Tests for Conclusion operations via ConclusionScope. + * Tests for Conclusion operations via ConclusionsView. * * Endpoints covered: * - POST /v3/workspaces/:workspaceId/conclusions (create conclusions) @@ -11,7 +11,7 @@ */ import { describe, test, expect, beforeAll, afterAll } from 'bun:test' -import { Honcho, Conclusion, ConclusionScope } from '../src' +import { Honcho, Conclusion, ConclusionsView } from '../src' import { createTestClient, requireServer } from './setup' import { assertConclusionShape } from './helpers' @@ -31,16 +31,16 @@ describe('Conclusions', () => { }) // =========================================================================== - // ConclusionScope Access + // ConclusionsView Access // =========================================================================== - describe('ConclusionScope access', () => { + describe('ConclusionsView access', () => { test('peer.conclusions returns self-scope', async () => { const peer = await client.peer('self-scope-peer') const scope = peer.conclusions - expect(scope).toBeInstanceOf(ConclusionScope) + expect(scope).toBeInstanceOf(ConclusionsView) expect(scope.observer).toBe(peer.id) expect(scope.observed).toBe(peer.id) expect(scope.workspaceId).toBe(client.workspaceId) @@ -289,7 +289,7 @@ describe('Conclusions', () => { for (const key of ['observer', 'observed', 'observer_id', 'observed_id']) { await expect( peer.conclusions.list({ filters: { [key]: 'someone-else' } }) - ).rejects.toThrow(/managed by this conclusion scope/) + ).rejects.toThrow(/managed by this conclusions view/) } }) @@ -298,10 +298,10 @@ describe('Conclusions', () => { await expect( peer.conclusions.list({ filters: { session_id: 'sess' } }) - ).rejects.toThrow(/managed by this conclusion scope/) + ).rejects.toThrow(/managed by this conclusions view/) await expect( peer.conclusions.list({ filters: { session: 'sess' } }) - ).rejects.toThrow(/managed by this conclusion scope/) + ).rejects.toThrow(/managed by this conclusions view/) }) test('query rejects observer/observed scope keys in filters', async () => { @@ -310,7 +310,7 @@ describe('Conclusions', () => { for (const key of ['observer', 'observed', 'observer_id', 'observed_id']) { await expect( peer.conclusions.query('q', 10, undefined, { [key]: 'someone-else' }) - ).rejects.toThrow(/managed by this conclusion scope/) + ).rejects.toThrow(/managed by this conclusions view/) } }) @@ -464,16 +464,16 @@ describe('Conclusions', () => { }) // =========================================================================== - // ConclusionScope toString + // ConclusionsView toString // =========================================================================== - describe('ConclusionScope toString', () => { + describe('ConclusionsView toString', () => { test('returns readable format', async () => { const peer = await client.peer('scope-tostring-peer') const str = peer.conclusions.toString() - expect(str).toContain('ConclusionScope') + expect(str).toContain('ConclusionsView') expect(str).toContain(peer.id) expect(str).toContain(client.workspaceId) }) diff --git a/sdks/typescript/__tests__/http-client.test.ts b/sdks/typescript/__tests__/http-client.test.ts index 06d80230..878869b9 100644 --- a/sdks/typescript/__tests__/http-client.test.ts +++ b/sdks/typescript/__tests__/http-client.test.ts @@ -237,6 +237,39 @@ describe('URL building', () => { expect(url.searchParams.get('present')).toBe('value') expect(url.searchParams.has('missing')).toBe(false) }) + + test('sends array query parameters as repeated params, not comma-joined', async () => { + let capturedURL = '' + globalThis.fetch = async (url) => { + capturedURL = url.toString() + return mockResponse({ ok: true }) + } + + await client.get('/v1/test', { + query: { sessions: ['session-a', 'session-b'] }, + }) + + const url = new URL(capturedURL) + // The API reads list-valued params as ?k=a&k=b. A comma-joined single value + // would arrive as one malformed entry. + expect(url.searchParams.getAll('sessions')).toEqual([ + 'session-a', + 'session-b', + ]) + }) + + test('an empty array query parameter contributes nothing', async () => { + let capturedURL = '' + globalThis.fetch = async (url) => { + capturedURL = url.toString() + return mockResponse({ ok: true }) + } + + await client.get('/v1/test', { query: { sessions: [] } }) + + const url = new URL(capturedURL) + expect(url.searchParams.has('sessions')).toBe(false) + }) }) // ============================================================================= diff --git a/sdks/typescript/__tests__/peer.test.ts b/sdks/typescript/__tests__/peer.test.ts index fca85910..cde6a0ad 100644 --- a/sdks/typescript/__tests__/peer.test.ts +++ b/sdks/typescript/__tests__/peer.test.ts @@ -671,7 +671,7 @@ describe('Peer', () => { // =========================================================================== describe('Conclusion scope access', () => { - test('conclusions property returns ConclusionScope for self', async () => { + test('conclusions property returns ConclusionsView for self', async () => { const peer = await client.peer('self-conclusions-peer') const scope = peer.conclusions @@ -681,7 +681,7 @@ describe('Peer', () => { expect(scope.workspaceId).toBe(client.workspaceId) }) - test('conclusionsOf returns ConclusionScope for target', async () => { + test('conclusionsOf returns ConclusionsView for target', async () => { const observer = await client.peer('obs-conclusions-peer') const target = await client.peer('target-conclusions-peer') diff --git a/sdks/typescript/__tests__/scope.unit.test.ts b/sdks/typescript/__tests__/scope.unit.test.ts new file mode 100644 index 00000000..b4b9ebab --- /dev/null +++ b/sdks/typescript/__tests__/scope.unit.test.ts @@ -0,0 +1,389 @@ +import { describe, expect, test } from 'bun:test' +import { ZodError } from 'zod' +import type { HonchoHTTPClient } from '../src/http/client' +import { Peer } from '../src/peer' +import { Scope } from '../src/scope' +import { Session } from '../src/session' +import type { ScopeStatusResponse } from '../src/types/api' + +/** + * Capture the body of the single request a call makes, so the wire shape the + * server actually receives is asserted rather than the SDK's own options. + */ +function capturingHttp(response: unknown): { + http: HonchoHTTPClient + body: () => Record | undefined + query: () => Record | undefined + path: () => string | undefined +} { + let capturedBody: Record | undefined + let capturedQuery: Record | undefined + let capturedPath: string | undefined + const http = { + post: async ( + path: string, + options?: { body?: Record } + ) => { + capturedPath = path + capturedBody = options?.body + return response + }, + get: async ( + path: string, + options?: { query?: Record } + ) => { + capturedPath = path + capturedQuery = options?.query + return response + }, + delete: async (path: string) => { + capturedPath = path + return undefined + }, + } as unknown as HonchoHTTPClient + return { + http, + body: () => capturedBody, + query: () => capturedQuery, + path: () => capturedPath, + } +} + +describe('sessions allowlist sugar', () => { + test('chat sends `sessions` as a session_id filter, not a bare field', async () => { + const { http, body } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await peer.chat('what happened?', { + sessions: ['session-a', new Session('session-b', 'workspace-1', http)], + }) + + expect(body()).toMatchObject({ + filters: { session_id: ['session-a', 'session-b'] }, + }) + // The sugar must not leak through as its own wire field — the server would + // reject an unknown key. + expect(body()).not.toHaveProperty('sessions') + }) + + test('representation sends `sessions` as a session_id filter', async () => { + const { http, body } = capturingHttp({ representation: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await peer.representation({ sessions: ['session-a'] }) + + expect(body()).toMatchObject({ filters: { session_id: ['session-a'] } }) + }) + + test('an empty allowlist is rejected rather than silently recalling nothing', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await expect(peer.chat('q', { sessions: [] })).rejects.toBeInstanceOf( + ZodError + ) + }) +}) + +describe('scope read option', () => { + test('a single scope passes through as `scope`', async () => { + const { http, body } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await peer.chat('q', { scope: 'therapy' }) + + expect(body()).toMatchObject({ scope: 'therapy' }) + }) + + test('a Scope object resolves to its id', async () => { + const { http, body } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await peer.chat('q', { scope: new Scope('therapy', 'workspace-1', http) }) + + expect(body()).toMatchObject({ scope: 'therapy' }) + }) + + test('a list of scopes passes through as a list', async () => { + const { http, body } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await peer.chat('q', { scope: ['therapy', 'work'] }) + + expect(body()).toMatchObject({ scope: ['therapy', 'work'] }) + }) + + test('scope and sessions are rejected together', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await expect( + peer.chat('q', { scope: 'therapy', sessions: ['session-a'] }) + ).rejects.toBeInstanceOf(ZodError) + }) + + test('scope and a single session are rejected together', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await expect( + peer.chat('q', { scope: 'therapy', session: 'session-a' }) + ).rejects.toBeInstanceOf(ZodError) + }) +}) + +describe('scope id validation', () => { + test('a prefixed name reports the reserved prefix, not the charset', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + // 'scope.therapy' fails both rules; the prefix message is the useful one, + // so it must be the only one raised. + const error = await peer + .chat('q', { scope: 'scope.therapy' }) + .then(() => undefined) + .catch((err: unknown) => err as ZodError) + + expect(error).toBeInstanceOf(ZodError) + const messages = (error as ZodError).issues.map((issue) => issue.message) + expect(messages.some((m) => m.includes('reserved prefix'))).toBe(true) + expect(messages.some((m) => m.includes('may only contain'))).toBe(false) + }) + + test('a name with illegal characters is rejected', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + await expect(peer.chat('q', { scope: 'my scope' })).rejects.toBeInstanceOf( + ZodError + ) + }) + + test('the specific message survives the scope option union', async () => { + // ScopeOptionSchema is a union. Zod collapses a failing union into a single + // `invalid_union` / "Invalid input" issue and buries the branch errors, so + // the rules are applied after the union resolves. Without that, every bad + // scope reports "Invalid input" and the caller learns nothing. + for (const [input, expected] of [ + ['scope.therapy', 'reserved prefix'], + ['my scope', 'may only contain'], + ['', 'non-empty'], + ['a'.repeat(507), 'at most 506'], + ] as const) { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + const error = (await peer + .chat('q', { scope: input }) + .then(() => undefined) + .catch((err: unknown) => err)) as ZodError + + expect(error).toBeInstanceOf(ZodError) + const messages = error.issues.map((i) => i.message).join(' | ') + expect(messages).toContain(expected) + expect(messages).not.toContain('Invalid input') + } + }) + + test('list-form messages also survive the union', async () => { + const { http } = capturingHttp({ content: 'ok' }) + const peer = new Peer('alice', 'workspace-1', http) + + for (const [input, expected] of [ + [[], 'at least one scope'], + [['ok', 'scope.bad'], 'reserved prefix'], + [Array.from({ length: 101 }, (_, i) => `s${i}`), 'at most 100 scopes'], + ] as const) { + const error = (await peer + .chat('q', { scope: input as string[] }) + .then(() => undefined) + .catch((err: unknown) => err)) as ZodError + + expect(error.issues.map((i) => i.message).join(' | ')).toContain(expected) + } + }) +}) + +describe('empty-string options fail closed', () => { + test("session.context rejects scope: '' instead of returning unscoped context", async () => { + const { http, query } = capturingHttp({ + id: 'session-a', + messages: [], + summary: null, + peer_representation: null, + peer_card: null, + }) + const session = new Session('session-a', 'workspace-1', http) + + // A truthiness check here would drop the option and silently return the + // unscoped context — the opposite of what an invalid scope should do. + await expect( + session.context({ peerTarget: 'user', scope: '' }) + ).rejects.toBeInstanceOf(ZodError) + expect(query()).toBeUndefined() + }) +}) + +describe('scope membership ids are validated before reaching a URL', () => { + test('removeSession rejects an id that would alter the request path', async () => { + const { http, path } = capturingHttp(undefined) + const scope = new Scope('therapy', 'workspace-1', http) + + // `valid-session?typo` would address `valid-session` with a stray query + // string, removing the wrong session and reconciling against it. + await expect( + scope.removeSession('valid-session?typo') + ).rejects.toBeInstanceOf(ZodError) + expect(path()).toBeUndefined() + }) + + test('addSessions rejects the same shape', async () => { + const { http, body } = capturingHttp(undefined) + const scope = new Scope('therapy', 'workspace-1', http) + + await expect( + scope.addSessions(['ok-session', 'valid-session?typo']) + ).rejects.toBeInstanceOf(ZodError) + expect(body()).toBeUndefined() + }) +}) + +describe('session.context scoping', () => { + const contextResponse = { + id: 'session-a', + messages: [], + summary: null, + peer_representation: null, + peer_card: null, + } + + test('scope and sessions are sent as their own query params', async () => { + const { http, query } = capturingHttp(contextResponse) + const session = new Session('session-a', 'workspace-1', http) + + await session.context({ + peerTarget: 'user', + sessions: ['session-a', 'session-b'], + }) + + // An array here relies on the HTTP client emitting repeated params; see + // http-client.test.ts. + expect(query()).toMatchObject({ sessions: ['session-a', 'session-b'] }) + }) + + test('sessions without peerTarget is rejected, not silently ignored', async () => { + const { http } = capturingHttp(contextResponse) + const session = new Session('session-a', 'workspace-1', http) + + await expect( + session.context({ sessions: ['session-a'] }) + ).rejects.toBeInstanceOf(ZodError) + }) + + test('sessions and scope are rejected together', async () => { + const { http } = capturingHttp(contextResponse) + const session = new Session('session-a', 'workspace-1', http) + + await expect( + session.context({ + peerTarget: 'user', + scope: 'therapy', + sessions: ['session-a'], + }) + ).rejects.toBeInstanceOf(ZodError) + }) + + test('sessions and limitToSession are rejected together', async () => { + const { http } = capturingHttp(contextResponse) + const session = new Session('session-a', 'workspace-1', http) + + await expect( + session.context({ + peerTarget: 'user', + limitToSession: true, + sessions: ['session-a'], + }) + ).rejects.toBeInstanceOf(ZodError) + }) + + test('scope and peerPerspective are rejected together', async () => { + const { http } = capturingHttp(contextResponse) + const session = new Session('session-a', 'workspace-1', http) + + await expect( + session.context({ + peerTarget: 'user', + peerPerspective: 'assistant', + scope: 'therapy', + }) + ).rejects.toBeInstanceOf(ZodError) + }) +}) + +describe('Scope membership and status', () => { + test('addSessions posts session_ids and resolves Session objects', async () => { + const { http, body, path } = capturingHttp(undefined) + const scope = new Scope('therapy', 'workspace-1', http) + + await scope.addSessions([ + 'session-a', + new Session('session-b', 'workspace-1', http), + ]) + + expect(path()).toBe('/v3/workspaces/workspace-1/scopes/therapy/sessions') + expect(body()).toEqual({ session_ids: ['session-a', 'session-b'] }) + }) + + test('addSessions rejects a batch over the server limit instead of chunking', async () => { + const { http } = capturingHttp(undefined) + const scope = new Scope('therapy', 'workspace-1', http) + + const tooMany = Array.from({ length: 101 }, (_, i) => `session-${i}`) + + await expect(scope.addSessions(tooMany)).rejects.toBeInstanceOf(ZodError) + }) + + test('removeSession targets the session subpath', async () => { + const { http, path } = capturingHttp(undefined) + const scope = new Scope('therapy', 'workspace-1', http) + + await scope.removeSession(new Session('session-b', 'workspace-1', http)) + + expect(path()).toBe( + '/v3/workspaces/workspace-1/scopes/therapy/sessions/session-b' + ) + }) + + test('status maps snake_case job fields to camelCase', async () => { + const response: ScopeStatusResponse = { + backfill_status: { + 'session-a': { + state: 'completed', + updated_at: '2024-01-01T00:00:00Z', + docs_copied: 12, + }, + 'session-b': { state: 'pending', updated_at: '2024-01-02T00:00:00Z' }, + }, + } + const { http } = capturingHttp(response) + const scope = new Scope('therapy', 'workspace-1', http) + + const status = await scope.status() + + expect(status.backfillStatus['session-a']).toEqual({ + state: 'completed', + updatedAt: '2024-01-01T00:00:00Z', + docsCopied: 12, + }) + expect(status.backfillStatus['session-b']?.docsCopied).toBeUndefined() + }) + + test('status on a scope with no backfill is an empty map, not a throw', async () => { + // The server omits the key entirely when nothing was ever enqueued. + const { http } = capturingHttp({} as ScopeStatusResponse) + const scope = new Scope('therapy', 'workspace-1', http) + + const status = await scope.status() + + expect(status.backfillStatus).toEqual({}) + }) +}) diff --git a/sdks/typescript/src/client.ts b/sdks/typescript/src/client.ts index db8e237a..2b66b9c2 100644 --- a/sdks/typescript/src/client.ts +++ b/sdks/typescript/src/client.ts @@ -3,6 +3,7 @@ import { HonchoHTTPClient } from './http/client' import { Message } from './message' import { Page } from './pagination' import { Peer } from './peer' +import { Scope } from './scope' import { Session } from './session' import type { MessageResponse, @@ -11,6 +12,7 @@ import type { QueueStatus, QueueStatusParams, QueueStatusResponse, + ScopeResponse, SessionResponse, WorkspaceResponse, } from './types/api' @@ -32,12 +34,14 @@ import { peerConfigFromApi, peerConfigToApi, type QueueStatusOptions, + ScopeIdSchema, SearchQuerySchema, type SessionConfig, SessionConfigSchema, SessionIdSchema, type SessionMetadata, SessionMetadataSchema, + SessionScopesSchema, sessionConfigFromApi, sessionConfigToApi, type WorkspaceConfig, @@ -254,6 +258,7 @@ export class Honcho { params: { query: string filters?: Record + scope?: string limit?: number } ): Promise { @@ -314,6 +319,39 @@ export class Honcho { ) } + private async _getOrCreateScope( + workspaceId: string, + params: { + id: string + metadata?: Record + } + ): Promise { + return this._http.post( + `/${API_VERSION}/workspaces/${workspaceId}/scopes`, + { body: params } + ) + } + + private async _listScopes( + workspaceId: string, + params?: { + page?: number + size?: number + reverse?: boolean + } + ): Promise> { + return this._http.post>( + `/${API_VERSION}/workspaces/${workspaceId}/scopes/list`, + { + query: { + page: params?.page, + size: params?.size, + reverse: params?.reverse ? 'true' : undefined, + }, + } + ) + } + private async _listSessions( workspaceId: string, params?: { @@ -346,6 +384,7 @@ export class Honcho { string, { observe_me?: boolean | null; observe_others?: boolean | null } > + scopes?: string[] } ): Promise { return this._http.post( @@ -356,6 +395,7 @@ export class Honcho { metadata: params.metadata, configuration: sessionConfigToApi(params.configuration), peers: params.peers, + scopes: params.scopes, }, } ) @@ -504,6 +544,10 @@ export class Honcho { * @param options.peers - Optional peers to attach to the session at creation. * Accepts the same shape as `session.addPeers()` (peer ID strings, * Peer objects, arrays of either, or a record with per-peer config). + * @param options.scopes - Optional scopes this session should join. Each scope is + * created if it does not exist yet. Attaching at creation avoids the + * asynchronous backfill that a later `scope.addSessions()` triggers, + * since there is no history to copy. * @returns Promise resolving to a Session object that can be used to add peers, * send messages, and manage conversation context * @throws Error if the session ID is empty or invalid @@ -514,6 +558,7 @@ export class Honcho { metadata?: SessionMetadata configuration?: SessionConfig peers?: PeerAddition + scopes?: (string | Scope)[] } ): Promise { await this._ensureWorkspace() @@ -528,12 +573,17 @@ export class Honcho { options?.peers !== undefined ? PeerAdditionToApiSchema.parse(options.peers) : undefined + const validatedScopes = + options?.scopes !== undefined + ? SessionScopesSchema.parse(options.scopes.map(resolveId)) + : undefined const sessionData = await this._getOrCreateSession(this.workspaceId, { id: validatedId, configuration: validatedConfiguration, metadata: validatedMetadata, peers: validatedPeers, + scopes: validatedScopes, }) return new Session( validatedId, @@ -547,6 +597,90 @@ export class Honcho { ) } + /** + * Get or create a scope with the given ID. + * + * A scope is a named set of sessions that acts as a visibility boundary: recall + * performed through the scope sees only what happened in its sessions, while the + * underlying peer keeps its single unified representation of everything. + * + * @param id - Unprefixed scope name, unique within the workspace + * @param options.metadata - Optional metadata to associate with this scope + * @returns Promise resolving to a Scope object for managing membership + * @throws Error if the scope ID is empty or invalid, or if a peer already occupies + * the scope's reserved internal name + * + * @example + * ```typescript + * const therapy = await honcho.scope('therapy') + * await therapy.addSessions([session1, session2]) + * ``` + */ + async scope( + id: string, + options?: { + metadata?: Record + } + ): Promise { + await this._ensureWorkspace() + const validatedId = ScopeIdSchema.parse(id) + + const scopeData = await this._getOrCreateScope(this.workspaceId, { + id: validatedId, + metadata: options?.metadata, + }) + return new Scope( + validatedId, + this.workspaceId, + this._http, + scopeData.metadata ?? undefined, + () => this._ensureWorkspace(), + scopeData.created_at + ) + } + + /** + * Get all scopes in the current workspace. + * + * @param options - Pagination options: `page`, `size`, and `reverse` + * @returns Promise resolving to a Page of Scope objects. Returns an empty page if + * no scopes exist + */ + async scopes(options?: { + page?: number + size?: number + reverse?: boolean + }): Promise> { + await this._ensureWorkspace() + const reverse = options?.reverse + const scopesPage = await this._listScopes(this.workspaceId, { + page: options?.page, + size: options?.size, + reverse, + }) + + const fetchNextPage = async ( + page: number, + size: number + ): Promise> => { + return this._listScopes(this.workspaceId, { page, size, reverse }) + } + + return new Page( + scopesPage, + (scope) => + new Scope( + scope.id, + this.workspaceId, + this._http, + scope.metadata ?? undefined, + () => this._ensureWorkspace(), + scope.created_at + ), + fetchNextPage + ) + } + /** * Get all sessions in the current workspace. * @@ -775,6 +909,9 @@ export class Honcho { * * @param query - The search query to use * @param filters - Optional filters to scope the search. See [search filters documentation](https://honcho.dev/docs/v3/documentation/core-concepts/features/using-filters). + * @param options.scope - Optional scope to restrict the search to that scope's member + * sessions. Mutually exclusive with a `session_id` filter. A scope + * with no member sessions matches nothing rather than everything. * @param limit - Number of results to return (1-100, default: 10). * @returns Promise resolving to an array of Message objects representing the search results. * Returns an empty array if no messages are found. @@ -784,6 +921,7 @@ export class Honcho { query: string, options?: { filters?: Filters + scope?: string | Scope limit?: number } ): Promise { @@ -792,12 +930,19 @@ export class Honcho { const validatedFilters = options?.filters ? FilterSchema.parse(options.filters) : undefined + // Checked against undefined, not truthiness: `scope: ''` is invalid, and + // dropping it silently would diverge from the Python SDK, which rejects it. + const validatedScope = + options?.scope !== undefined + ? ScopeIdSchema.parse(resolveId(options.scope)) + : undefined const validatedLimit = options?.limit ? LimitSchema.parse(options.limit) : undefined const response = await this._searchWorkspace(this.workspaceId, { query: validatedQuery, filters: validatedFilters, + scope: validatedScope, limit: validatedLimit, }) return response.map(Message.fromApiResponse) diff --git a/sdks/typescript/src/conclusions.ts b/sdks/typescript/src/conclusions.ts index c9add4e2..44f5e7e1 100644 --- a/sdks/typescript/src/conclusions.ts +++ b/sdks/typescript/src/conclusions.ts @@ -12,10 +12,10 @@ import type { import { normalizeSearchQuery, RepresentationOptionsSchema } from './validation' /** - * Filter keys that define a conclusion scope (the observer/observed peer pair). - * They are set from the scope itself, so a caller must not pass them in `filters`. + * Filter keys that define a conclusions view (the observer/observed peer pair). + * They are set from the view itself, so a caller must not pass them in `filters`. */ -const SCOPE_RESERVED_KEYS = [ +const VIEW_RESERVED_KEYS = [ 'observer', 'observed', 'observer_id', @@ -23,11 +23,11 @@ const SCOPE_RESERVED_KEYS = [ ] /** - * Throw if `filters` contains keys managed by the conclusion scope. + * Throw if `filters` contains keys managed by the conclusions view. * * The observer/observed peer pair (and, on `list`, the session) is fixed by the - * scope, so letting a user filter override it would silently return data from a - * different scope than requested. Fail loud instead. + * view, so letting a user filter override it would silently return data from a + * different pair than requested. Fail loud instead. */ function rejectReservedFilterKeys( filters: Record | undefined, @@ -42,7 +42,7 @@ function rejectReservedFilterKeys( guidance += '; use the session option to filter by session' } throw new Error( - `Filter key(s) ${clash.join(', ')} are managed by this conclusion scope ` + + `Filter key(s) ${clash.join(', ')} are managed by this conclusions view ` + `and cannot be passed in filters. ${guidance}.` ) } @@ -120,7 +120,7 @@ export class Conclusion { /** * Scoped access to conclusions for a specific observer/observed relationship. */ -export class ConclusionScope { +export class ConclusionsView { private _http: HonchoHTTPClient private _ensureWorkspace: () => Promise readonly workspaceId: string @@ -223,14 +223,14 @@ export class ConclusionScope { // =========================================================================== /** - * List conclusions in this scope. + * List conclusions in this view. * * @param options - Optional configuration for the list request * @param options.page - Page number (1-indexed, default: 1) * @param options.size - Number of items per page (default: 50) * @param options.session - Optional session (ID string or Session object) to filter by * @param options.filters - Optional additional filter criteria, merged with - * this scope's observer/observed (and session, if given). Supports the same + * this view's observer/observed (and session, if given). Supports the same * operators as other list endpoints — e.g. `{ level: 'explicit' }` to get * only conclusions extracted directly from messages (i.e. not derived during * dreaming). See @@ -245,7 +245,7 @@ export class ConclusionScope { reverse?: boolean }): Promise> { rejectReservedFilterKeys(options?.filters, [ - ...SCOPE_RESERVED_KEYS, + ...VIEW_RESERVED_KEYS, 'session', 'session_id', ]) @@ -284,13 +284,13 @@ export class ConclusionScope { } /** - * Semantic search for conclusions in this scope. + * Semantic search for conclusions in this view. * * @param query - The search query string * @param topK - Maximum number of results to return (default: 10) * @param distance - Maximum cosine distance threshold (0.0-1.0) * @param filters - Optional additional filter criteria, merged with this - * scope's observer/observed. Supports the same operators as the list + * view's observer/observed. Supports the same operators as the list * endpoint — e.g. `{ level: 'deductive' }` to search only conclusions * derived during dreaming. See * https://honcho.dev/docs/v3/documentation/features/advanced/using-filters @@ -301,7 +301,7 @@ export class ConclusionScope { distance?: number, filters?: Record ): Promise { - rejectReservedFilterKeys(filters, SCOPE_RESERVED_KEYS) + rejectReservedFilterKeys(filters, VIEW_RESERVED_KEYS) const response = await this._query({ query, top_k: topK, @@ -324,7 +324,7 @@ export class ConclusionScope { } /** - * Create conclusions in this scope. + * Create conclusions in this view. */ async create( conclusions: ConclusionCreateParams | ConclusionCreateParams[] @@ -351,7 +351,7 @@ export class ConclusionScope { } /** - * Get the computed representation for this scope. + * Get the computed representation for this view. */ async representation(options?: RepresentationOptions): Promise { const searchQuery = normalizeSearchQuery(options?.searchQuery) @@ -375,6 +375,6 @@ export class ConclusionScope { } toString(): string { - return `ConclusionScope(workspaceId='${this.workspaceId}', observer='${this.observer}', observed='${this.observed}')` + return `ConclusionsView(workspaceId='${this.workspaceId}', observer='${this.observer}', observed='${this.observed}')` } } diff --git a/sdks/typescript/src/http/client.ts b/sdks/typescript/src/http/client.ts index e22480d2..fd5e6379 100644 --- a/sdks/typescript/src/http/client.ts +++ b/sdks/typescript/src/http/client.ts @@ -6,18 +6,27 @@ import { TimeoutError, } from './errors' +/** + * Query parameters for a request. An array value is sent as repeated + * parameters (`?k=a&k=b`), which is how the API reads list-valued parameters. + */ +export type QueryParams = Record< + string, + string | number | boolean | readonly (string | number | boolean)[] | undefined +> + export interface HonchoHTTPClientConfig { baseURL: string apiKey?: string timeout?: number maxRetries?: number defaultHeaders?: Record - defaultQuery?: Record + defaultQuery?: QueryParams } export interface RequestOptions { body?: unknown - query?: Record + query?: QueryParams headers?: Record timeout?: number signal?: AbortSignal @@ -37,7 +46,7 @@ export class HonchoHTTPClient { readonly timeout: number readonly maxRetries: number readonly defaultHeaders: Record - readonly defaultQuery?: Record + readonly defaultQuery?: QueryParams constructor(config: HonchoHTTPClientConfig) { // Remove trailing slash from baseURL @@ -273,21 +282,28 @@ export class HonchoHTTPClient { return JSON.parse(text) as T } - private buildURL( - path: string, - query?: Record - ): string { + private buildURL(path: string, query?: QueryParams): string { const url = new URL(path, this.baseURL) - const mergedQuery: Record = { + const mergedQuery: QueryParams = { ...(this.defaultQuery ?? {}), ...(query ?? {}), } for (const [key, value] of Object.entries(mergedQuery)) { - if (value !== undefined) { - url.searchParams.set(key, String(value)) + if (value === undefined) { + continue } + if (Array.isArray(value)) { + // Repeated params, not a comma-joined value: the API reads list-valued + // query parameters as `?k=a&k=b`, and String([a, b]) would arrive as a + // single malformed entry. + for (const entry of value) { + url.searchParams.append(key, String(entry)) + } + continue + } + url.searchParams.set(key, String(value)) } return url.toString() diff --git a/sdks/typescript/src/index.ts b/sdks/typescript/src/index.ts index 90bff9f2..24ba57b4 100644 --- a/sdks/typescript/src/index.ts +++ b/sdks/typescript/src/index.ts @@ -6,7 +6,13 @@ export { Honcho } from './client' export { Conclusion, type ConclusionCreateParams, - ConclusionScope, + /** + * @deprecated Renamed to `ConclusionsView`. "Scope" now means a named set of + * sessions (see `Scope`), which this class is not — it is a view over one + * observer/observed pair. Kept as an alias for one more minor version. + */ + ConclusionsView as ConclusionScope, + ConclusionsView, } from './conclusions' // HTTP infrastructure export { @@ -30,6 +36,11 @@ export { export { Message, type MessageInput } from './message' export { Page } from './pagination' export { Peer, PeerContext } from './peer' +export { + Scope, + type ScopeBackfillState, + type ScopeStatus, +} from './scope' export { Session } from './session' export { SessionContext, @@ -50,6 +61,9 @@ export type { QueueStatus, QueueStatusResponse, RepresentationOptions, + ScopeBackfillJob, + ScopeResponse, + ScopeStatusResponse, SessionContextResponse, SessionQueueStatus, SessionResponse, diff --git a/sdks/typescript/src/peer.ts b/sdks/typescript/src/peer.ts index adb67cb2..9dab1658 100644 --- a/sdks/typescript/src/peer.ts +++ b/sdks/typescript/src/peer.ts @@ -1,6 +1,6 @@ import { ZodType, z } from 'zod' import { API_VERSION } from './api-version' -import { ConclusionScope } from './conclusions' +import { ConclusionsView } from './conclusions' import type { HonchoHTTPClient } from './http/client' import { createDialecticStream, @@ -8,6 +8,9 @@ import { } from './http/streaming' import { Message, type MessageInput } from './message' import { Page } from './pagination' +// Type-only: scope.ts imports Session, which imports Peer. Importing the type +// keeps that cycle out of the emitted JS. +import type { Scope } from './scope' import { Session } from './session' import type { MessageResponse, @@ -40,6 +43,7 @@ import { peerConfigToApi, RepresentationOptionsSchema, SearchQuerySchema, + scopeRecallFields, sessionConfigFromApi, } from './validation' @@ -251,6 +255,8 @@ export class Peer { stream?: boolean target?: string session_id?: string + scope?: string | string[] + filters?: Record reasoning_level?: string response_format?: Record }): Promise { @@ -265,6 +271,8 @@ export class Peer { query: string target?: string session_id?: string + scope?: string | string[] + filters?: Record reasoning_level?: string response_format?: Record }): Promise { @@ -295,6 +303,8 @@ export class Peer { private async _getRepresentation(params: { session_id?: string + scope?: string | string[] + filters?: Record target?: string search_query?: string search_top_k?: number @@ -365,6 +375,18 @@ export class Peer { * @param options.session - Optional session to scope the query to. If provided, only * information from that session is considered. Can be a session * ID string or a Session object. + * @param options.scope - Optional scope(s) to confine the query to. A single scope answers + * from that scope's own view of the target, including the higher-order + * conclusions reasoned within it. A list of scopes restricts recall to + * the union of their member sessions, which — like `sessions` — yields + * only directly-stated conclusions. Mutually exclusive with `session` + * and `sessions`, and requires a workspace-level key. + * @param options.sessions - Optional allowlist of sessions to confine the query to, for + * one-off questions that span a handful of sessions. Recall is + * limited to conclusions stated directly in those sessions: + * conclusions produced by reasoning across sessions are excluded, + * because their provenance cannot be proven to sit inside the + * allowlist. Reach for a named `scope` when you need that depth. * @param options.reasoningLevel - Optional reasoning level for the query: "minimal", "low", "medium", * "high", or "max". Defaults to "low" if not provided. * @returns Promise resolving to the response string, or null if no relevant information @@ -379,6 +401,16 @@ export class Peer { * target: otherPeer, * reasoningLevel: 'high' * }) + * + * // Answer only from a named scope + * const response = await peer.chat('What is stressing them out?', { + * scope: 'therapy', + * }) + * + * // Answer only from an ad-hoc set of sessions + * const response = await peer.chat('What did we decide?', { + * sessions: [session1, session2], + * }) * ``` */ async chat( @@ -386,6 +418,8 @@ export class Peer { options: { target?: string | Peer session?: string | Session + scope?: string | Scope | (string | Scope)[] + sessions?: (string | Session)[] reasoningLevel?: string responseFormat: ZodType } @@ -395,6 +429,8 @@ export class Peer { options?: { target?: string | Peer session?: string | Session + scope?: string | Scope | (string | Scope)[] + sessions?: (string | Session)[] reasoningLevel?: string responseFormat?: Record } @@ -404,6 +440,8 @@ export class Peer { options?: { target?: string | Peer session?: string | Session + scope?: string | Scope | (string | Scope)[] + sessions?: (string | Session)[] reasoningLevel?: string responseFormat?: ZodType | Record } @@ -423,6 +461,8 @@ export class Peer { query, target: targetId, session: resolvedSessionId, + scope: options?.scope, + sessions: options?.sessions, reasoningLevel: options?.reasoningLevel, responseFormat: options?.responseFormat, }) @@ -437,6 +477,7 @@ export class Peer { stream: false, target: chatParams.target, session_id: chatParams.session, + ...scopeRecallFields(chatParams), reasoning_level: chatParams.reasoningLevel, response_format: Peer.toResponseFormatSchema(options?.responseFormat), }) @@ -465,6 +506,9 @@ export class Peer { * @param options.session - Optional session to scope the query to. If provided, only * information from that session is considered. Can be a session * ID string or a Session object. + * @param options.scope - Optional scope(s) to confine the query to. See {@link Peer.chat}. + * @param options.sessions - Optional allowlist of sessions to confine the query to. + * See {@link Peer.chat} for the depth caveat. * @param options.reasoningLevel - Optional reasoning level for the query: "minimal", "low", "medium", * "high", or "max". Defaults to "low" if not provided. * @returns Promise resolving to a DialecticStreamResponse that can be iterated over @@ -489,6 +533,8 @@ export class Peer { options?: { target?: string | Peer session?: string | Session + scope?: string | Scope | (string | Scope)[] + sessions?: (string | Session)[] reasoningLevel?: string responseFormat?: ZodType | Record } @@ -508,6 +554,8 @@ export class Peer { query, target: targetId, session: resolvedSessionId, + scope: options?.scope, + sessions: options?.sessions, reasoningLevel: options?.reasoningLevel, responseFormat: options?.responseFormat, }) @@ -516,6 +564,7 @@ export class Peer { query: chatParams.query, target: chatParams.target, session_id: chatParams.session, + ...scopeRecallFields(chatParams), reasoning_level: chatParams.reasoningLevel, response_format: Peer.toResponseFormatSchema(options?.responseFormat), }) @@ -846,6 +895,8 @@ export class Peer { */ async representation(options?: { session?: string | Session + scope?: string | Scope | (string | Scope)[] + sessions?: (string | Session)[] target?: string | Peer searchQuery?: string | Message searchTopK?: number @@ -856,6 +907,8 @@ export class Peer { const searchQuery = normalizeSearchQuery(options?.searchQuery) const getRepresentationParams = PeerGetRepresentationParamsSchema.parse({ session: options?.session, + scope: options?.scope, + sessions: options?.sessions, target: options?.target, options: { searchQuery, @@ -878,6 +931,7 @@ export class Peer { const response = await this._getRepresentation({ session_id: sessionId, + ...scopeRecallFields(getRepresentationParams), target: targetId, search_query: searchQuery, search_top_k: getRepresentationParams.options?.searchTopK, @@ -964,7 +1018,7 @@ export class Peer { * This property provides a convenient way to access conclusions that this peer * has made about themselves. Use this for self-conclusion scenarios. * - * @returns A ConclusionScope scoped to this peer's self-conclusions + * @returns A ConclusionsView scoped to this peer's self-conclusions * * @example * ```typescript @@ -978,8 +1032,8 @@ export class Peer { * await peer.conclusions.delete('obs-123') * ``` */ - get conclusions(): ConclusionScope { - return new ConclusionScope( + get conclusions(): ConclusionsView { + return new ConclusionsView( this._http, this.workspaceId, this.id, @@ -995,7 +1049,7 @@ export class Peer { * observer and the target is the observed peer. * * @param target - The target peer (either a Peer object or peer ID string) - * @returns A ConclusionScope scoped to this peer's conclusions of the target + * @returns A ConclusionsView scoped to this peer's conclusions of the target * * @example * ```typescript @@ -1012,9 +1066,9 @@ export class Peer { * const rep = await bobConclusions.representation() * ``` */ - conclusionsOf(target: string | Peer): ConclusionScope { + conclusionsOf(target: string | Peer): ConclusionsView { const targetId = typeof target === 'string' ? target : target.id - return new ConclusionScope( + return new ConclusionsView( this._http, this.workspaceId, this.id, diff --git a/sdks/typescript/src/scope.ts b/sdks/typescript/src/scope.ts new file mode 100644 index 00000000..5a1114fe --- /dev/null +++ b/sdks/typescript/src/scope.ts @@ -0,0 +1,278 @@ +import { API_VERSION } from './api-version' +import type { HonchoHTTPClient } from './http/client' +import { Page } from './pagination' +import { Session } from './session' +import type { + PageResponse, + ScopeStatusResponse, + SessionResponse, +} from './types/api' +import { resolveId } from './utils' +import { + ScopeSessionsSchema, + SessionIdSchema, + sessionConfigFromApi, +} from './validation' + +/** + * Backfill job state for one session in a scope. + */ +export interface ScopeBackfillState { + state: 'pending' | 'completed' | 'failed' + updatedAt: string + /** + * Number of documents copied into the scope. Present only once the backfill + * for this session completes. + */ + docsCopied?: number +} + +/** + * Backfill/reconciliation progress for a scope, keyed by session ID. + * + * Only sessions that have had a backfill enqueued appear. A scope whose + * sessions were all empty when added has an empty `backfillStatus`. + */ +export interface ScopeStatus { + backfillStatus: Record +} + +/** + * Represents a scope in the Honcho system. + * + * A scope is a named set of sessions that acts as a visibility boundary. Recall + * performed through a scope sees only what happened in that scope's sessions, + * while the underlying peer keeps its single unified representation across + * everything it has ever participated in. + * + * Membership changes are applied asynchronously: adding a session that already + * has messages copies its existing conclusions into the scope, and removing one + * reconciles them back out. Poll {@link Scope.status} to watch that settle. + * + * @example + * ```typescript + * const therapy = await honcho.scope('therapy') + * await therapy.addSessions([session1, session2]) + * + * // Ask a question answered only from the therapy sessions + * const answer = await user.chat('What is stressing them out?', { + * scope: 'therapy', + * }) + * ``` + */ +export class Scope { + /** + * Unique identifier for this scope, without the server-side `scope.` prefix. + */ + readonly id: string + /** + * Workspace ID for scoping operations. + */ + readonly workspaceId: string + private _http: HonchoHTTPClient + private _metadata?: Record + private _createdAt?: string + private _ensureWorkspace: () => Promise + + /** + * Cached metadata for this scope. May be stale if the scope was not recently + * fetched from the API. + */ + get metadata(): Record | undefined { + return this._metadata + } + + /** + * Timestamp when this scope was created. Only available if fetched from the API. + */ + get createdAt(): string | undefined { + return this._createdAt + } + + /** + * Initialize a new Scope. **Do not call this directly, use the client.scope() method instead.** + * + * @param id - Unprefixed scope name, unique within the workspace + * @param workspaceId - Workspace ID for scoping operations + * @param http - Reference to the HTTP client instance + * @param metadata - Optional metadata to initialize the cached value + * @param ensureWorkspace - Callback that guarantees the workspace exists + * @param createdAt - Creation timestamp, if already fetched + */ + constructor( + id: string, + workspaceId: string, + http: HonchoHTTPClient, + metadata?: Record, + ensureWorkspace: () => Promise = async () => undefined, + createdAt?: string + ) { + this.id = id + this.workspaceId = workspaceId + this._http = http + this._metadata = metadata + this._ensureWorkspace = ensureWorkspace + this._createdAt = createdAt + } + + // =========================================================================== + // Private API Methods + // =========================================================================== + + private get _basePath(): string { + return `/${API_VERSION}/workspaces/${this.workspaceId}/scopes/${this.id}` + } + + private async _addSessions(sessionIds: string[]): Promise { + await this._ensureWorkspace() + await this._http.post(`${this._basePath}/sessions`, { + body: { session_ids: sessionIds }, + }) + } + + private async _removeSession(sessionId: string): Promise { + await this._ensureWorkspace() + await this._http.delete(`${this._basePath}/sessions/${sessionId}`) + } + + private async _listSessions(params?: { + page?: number + size?: number + reverse?: boolean + }): Promise> { + await this._ensureWorkspace() + return this._http.post>( + `${this._basePath}/sessions/list`, + { + query: { + page: params?.page, + size: params?.size, + reverse: params?.reverse ? 'true' : undefined, + }, + } + ) + } + + private async _getStatus(): Promise { + await this._ensureWorkspace() + return this._http.get(`${this._basePath}/status`) + } + + // =========================================================================== + // Public API Methods + // =========================================================================== + + /** + * Add sessions to this scope. + * + * Every named session must already exist. Adding a session that is already a + * member is a no-op. + * + * Sessions that already hold messages are backfilled into the scope + * asynchronously, so recall through this scope may not reflect their history + * immediately — poll {@link Scope.status} to watch that complete. + * + * @param sessions - Sessions to add, as ID strings or Session objects. At most + * 100 per call, matching the server's limit; split larger + * membership changes into separate calls so a failure names + * the batch that failed. + */ + async addSessions(sessions: (string | Session)[]): Promise { + const sessionIds = ScopeSessionsSchema.parse(sessions.map(resolveId)) + await this._addSessions(sessionIds) + } + + /** + * Remove a session from this scope. + * + * Conclusions copied or derived while the session was a member are + * reconciled out asynchronously, and the scope's peer card is rebuilt from + * whatever evidence remains. Poll {@link Scope.status} to watch that settle. + * + * @param session - Session to remove, as an ID string or a Session object + * @throws If the session ID is malformed + */ + async removeSession(session: string | Session): Promise { + // Validated because this ID is interpolated into a request *path*: an + // unvalidated value silently changes which resource the request addresses. + // `valid-session?typo` would target `valid-session` with a stray query + // string, removing the wrong session and reconciling against it. + await this._removeSession(SessionIdSchema.parse(resolveId(session))) + } + + /** + * Get the sessions that are members of this scope. + * + * Ordered by how long each session has been a member — longest-standing + * first, or most recently added first when `reverse` is true. + * + * @param options - Pagination options: `page`, `size`, and `reverse` + * @returns Promise resolving to a paginated list of member Sessions + */ + async sessions(options?: { + page?: number + size?: number + reverse?: boolean + }): Promise> { + const reverse = options?.reverse + const sessionsPage = await this._listSessions({ + page: options?.page, + size: options?.size, + reverse, + }) + + const fetchNextPage = async ( + page: number, + size: number + ): Promise> => { + return this._listSessions({ page, size, reverse }) + } + + return new Page( + sessionsPage, + (session) => + new Session( + session.id, + this.workspaceId, + this._http, + session.metadata ?? undefined, + sessionConfigFromApi(session.configuration) ?? undefined, + () => this._ensureWorkspace(), + session.created_at, + session.is_active + ), + fetchNextPage + ) + } + + /** + * Get the backfill/reconciliation progress for this scope. + * + * Use this after a membership change to tell "the scope knows nothing about + * that session yet" apart from "the scope has caught up and there is genuinely + * nothing to recall". + * + * @returns Promise resolving to per-session backfill state + */ + async status(): Promise { + const response = await this._getStatus() + return { + backfillStatus: Object.fromEntries( + Object.entries(response.backfill_status ?? {}).map( + ([sessionId, job]) => [ + sessionId, + { + state: job.state, + updatedAt: job.updated_at, + docsCopied: job.docs_copied, + }, + ] + ) + ), + } + } + + toString(): string { + return `Scope(id='${this.id}', workspaceId='${this.workspaceId}')` + } +} diff --git a/sdks/typescript/src/session.ts b/sdks/typescript/src/session.ts index 0f9cf4aa..ed6140a6 100644 --- a/sdks/typescript/src/session.ts +++ b/sdks/typescript/src/session.ts @@ -3,6 +3,9 @@ import type { HonchoHTTPClient } from './http/client' import { Message } from './message' import { Page } from './pagination' import { Peer } from './peer' +// Type-only: scope.ts imports this module. Importing the type keeps that cycle +// out of the emitted JS. +import type { Scope } from './scope' import { SessionContext, SessionSummaries } from './session_context' import type { MessageResponse, @@ -17,7 +20,7 @@ import type { SessionResponse, SessionSummariesResponse, } from './types/api' -import { transformQueueStatus } from './utils' +import { resolveId, transformQueueStatus } from './utils' import { ContextParamsSchema, FileUploadSchema, @@ -220,6 +223,8 @@ export class Session { search_query?: string peer_target?: string peer_perspective?: string + scope?: string + sessions?: string[] limit_to_session?: boolean search_top_k?: number search_max_distance?: number @@ -752,6 +757,17 @@ export class Session { * @param options.tokens - Target token count for the context window * @param options.peerTarget - The peer to get representation for * @param options.peerPerspective - The peer whose perspective to use for representation + * @param options.scope - A scope to use as the perspective source instead of a peer: the + * target's representation and card are read from what that scope + * observed. Requires `peerTarget`, is mutually exclusive with + * `peerPerspective`, and requires a workspace-level key. + * @param options.sessions - Allowlist of sessions confining the target's representation + * to that set. This session must be one of them. Recall is + * limited to conclusions stated directly in those sessions, and + * the peer card is omitted, since neither derived conclusions + * nor cards carry provable per-session provenance. Mutually + * exclusive with `scope` and `limitToSession`; requires + * `peerTarget`. * @param options.limitToSession - Whether to limit representation to this session only * @param options.representationOptions - Options for representation retrieval (searchQuery, searchTopK, etc.) * @returns Promise resolving to a SessionContext with messages, summary, and representation @@ -764,6 +780,12 @@ export class Session { * peerTarget: user * }) * + * // Build the context from what a scope observed + * const ctx = await session.context({ + * peerTarget: user, + * scope: 'therapy', + * }) + * * // Convert to OpenAI format * const messages = ctx.toOpenAI(assistant) * ``` @@ -773,6 +795,8 @@ export class Session { tokens?: number peerTarget?: string | Peer peerPerspective?: string | Peer + scope?: string | Scope + sessions?: (string | Session)[] limitToSession?: boolean representationOptions?: RepresentationOptions }): Promise { @@ -795,6 +819,10 @@ export class Session { tokens: opts.tokens, peerTarget: peerTargetId, peerPerspective: peerPerspectiveId, + // Checked against undefined, not truthiness: `scope: ''` must reach the + // schema and be rejected, not be dropped into an unscoped context. + scope: opts.scope !== undefined ? resolveId(opts.scope) : undefined, + sessions: opts.sessions, limitToSession: opts.limitToSession, representationOptions: opts.representationOptions ? { @@ -810,6 +838,8 @@ export class Session { search_query: searchQuery, peer_target: contextParams.peerTarget, peer_perspective: contextParams.peerPerspective, + scope: contextParams.scope, + sessions: contextParams.sessions, limit_to_session: contextParams.limitToSession, search_top_k: contextParams.representationOptions?.searchTopK, search_max_distance: diff --git a/sdks/typescript/src/types/api.ts b/sdks/typescript/src/types/api.ts index dbe4378d..10ed6037 100644 --- a/sdks/typescript/src/types/api.ts +++ b/sdks/typescript/src/types/api.ts @@ -133,6 +133,28 @@ export interface SessionCreateParams { metadata?: Record configuration?: SessionConfigApi peers?: Record + scopes?: string[] +} + +export interface ScopeResponse { + id: string + metadata: Record + created_at: string +} + +/** + * Per-session backfill job state for a scope. + * + * `docs_copied` is present only once a backfill completes. + */ +export interface ScopeBackfillJob { + state: 'pending' | 'completed' | 'failed' + updated_at: string + docs_copied?: number +} + +export interface ScopeStatusResponse { + backfill_status: Record } export interface SessionUpdateParams { diff --git a/sdks/typescript/src/validation.ts b/sdks/typescript/src/validation.ts index 48255833..832690e2 100644 --- a/sdks/typescript/src/validation.ts +++ b/sdks/typescript/src/validation.ts @@ -159,6 +159,149 @@ export const SessionIdSchema = z */ const SessionIdObjectSchema = z.object({ id: SessionIdSchema }) +/** + * Reserved peer-name prefix the server uses to store a scope. + */ +const SCOPE_PEER_PREFIX = 'scope.' + +/** + * Scope IDs are stored as peer names with the reserved prefix prepended, so + * they must leave room for it within the 512-character peer name limit. + */ +const SCOPE_ID_MAX_LENGTH = 512 - SCOPE_PEER_PREFIX.length + +/** + * The scope ID rules, as a plain function rather than only a schema. + * + * Zod reports a failing union as a single `invalid_union` / "Invalid input" + * issue and buries the branch errors, so a schema alone cannot carry these + * messages out of `ScopeOptionSchema`. Keeping the rules callable lets both the + * bare schema and the union surface the same specific message. + * + * @returns The problems found, or an empty array when the ID is valid. + */ +function scopeIdIssues(value: string): string[] { + if (value.length < 1) { + return ['Scope ID must be a non-empty string'] + } + if (value.length > SCOPE_ID_MAX_LENGTH) { + return [`Scope ID can be at most ${SCOPE_ID_MAX_LENGTH} characters`] + } + // Checked before the charset: the reserved prefix contains '.', which is + // itself outside the charset, so a charset-first check would report the + // charset instead of the real mistake for a double-prefixed name. + if (value.startsWith(SCOPE_PEER_PREFIX)) { + return [ + `Scope ID must not start with the reserved prefix '${SCOPE_PEER_PREFIX}' (scope IDs are unprefixed)`, + ] + } + if (!/^[a-zA-Z0-9_-]+$/.test(value)) { + return [ + 'Scope ID may only contain letters, numbers, underscores, and hyphens', + ] + } + return [] +} + +/** + * Add every scope ID problem in `values` as a top-level issue. + */ +function addScopeIdIssues(values: string[], ctx: z.RefinementCtx): void { + for (const value of values) { + for (const message of scopeIdIssues(value)) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message }) + } + } +} + +/** + * Schema for scope ID validation. + * + * Scope IDs are unprefixed — the `scope.` prefix is a server-side storage + * detail and never appears on the wire. + */ +export const ScopeIdSchema = z.string().superRefine((val, ctx) => { + addScopeIdIssues([val], ctx) +}) + +/** + * Shape-only branch for the `scope` option: an ID string, or an object carrying + * one (so a `Scope` instance is accepted). The ID itself is validated after the + * union resolves — see `ScopeOptionSchema`. + */ +const ScopeIdLikeSchema = z.union([z.string(), z.object({ id: z.string() })]) + +/** + * Schema for the `scope` read option: one scope, or a bounded list of them. + * + * A single scope reads that scope's own view. A list restricts recall to the + * union of the scopes' member sessions. An empty list is rejected rather than + * resolved to an empty allowlist, which would silently recall nothing. + * + * The union discriminates shape only; IDs and list bounds are checked after the + * transform so their messages are not swallowed as `invalid_union`. + */ +export const ScopeOptionSchema = z + .union([ScopeIdLikeSchema, z.array(ScopeIdLikeSchema)]) + .transform((val) => + Array.isArray(val) + ? val.map((entry) => (typeof entry === 'string' ? entry : entry.id)) + : typeof val === 'string' + ? val + : val.id + ) + .superRefine((resolved, ctx) => { + if (Array.isArray(resolved)) { + if (resolved.length === 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'scope must name at least one scope', + }) + } + if (resolved.length > 100) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'scope can name at most 100 scopes', + }) + } + } + addScopeIdIssues(Array.isArray(resolved) ? resolved : [resolved], ctx) + }) + +/** + * Schema for a scope membership change: the sessions to add to a scope. + * + * Capped at 100 to match the server rather than silently chunking, so a + * rejected batch is the batch the caller passed. + */ +export const ScopeSessionsSchema = z + .array(SessionIdSchema) + .min(1, 'At least one session must be given') + .max(100, 'At most 100 sessions can be added per call') + +/** + * Schema for the `scopes` option on session creation: the scopes a new session + * should join. + */ +export const SessionScopesSchema = z + .array(ScopeIdSchema) + .min(1, 'scopes must name at least one scope') + .max(100, 'scopes can name at most 100 scopes') + +/** + * Schema for the `sessions` allowlist option — sugar for the wire-level + * `filters: { session_id: [...] }`. + * + * Capped at 1,000 entries to match the server. An empty list is rejected: the + * server treats an empty allowlist as fail-closed (recalls nothing), which is + * never what a caller passing `sessions: []` intends. + */ +export const SessionAllowlistSchema = z + .array(z.union([SessionIdSchema, SessionIdObjectSchema])) + .min(1, 'sessions must name at least one session') + .max(1000, 'sessions can name at most 1000 sessions') + .transform((vals) => vals.map((v) => (typeof v === 'string' ? v : v.id))) + /** * Schema for session peer configuration. */ @@ -291,6 +434,58 @@ export function normalizeListOptions( return { filters: input as Filters } as T } +/** + * Translate validated `scope` / `sessions` options into their wire fields. + * + * `sessions` is sugar: it goes out as the constrained + * `filters: { session_id: [...] }` body the recall endpoints accept, never as a + * field of its own — the server rejects unknown keys with a 422. Shared by chat, + * chatStream, and representation so the three cannot drift apart. + * + * Purely a translation; the schemas have already rejected the invalid + * combinations by the time this runs. + */ +export function scopeRecallFields(options: { + scope?: string | string[] + sessions?: string[] +}): { scope?: string | string[]; filters?: Record } { + return { + scope: options.scope, + filters: options.sessions ? { session_id: options.sessions } : undefined, + } +} + +/** + * Add issues for the `scope` exclusions the server enforces with a 422. + * + * A scope already determines what a query can see, so combining it with a + * session allowlist or a single session is a contradiction rather than a + * narrowing. Shared by the chat, representation, and context schemas so the + * three surfaces cannot drift apart. + */ +function scopeExclusivityIssues( + data: { scope?: unknown; sessions?: unknown; session?: unknown }, + ctx: z.RefinementCtx +): void { + if (data.scope === undefined) { + return + } + if (data.sessions !== undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'scope and sessions are mutually exclusive', + path: ['sessions'], + }) + } + if (data.session !== undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'scope and session are mutually exclusive', + path: ['session'], + }) + } +} + /** * Schema for chat query parameters. */ @@ -309,6 +504,8 @@ export const ChatQuerySchema = z .transform((val) => val ? (typeof val === 'string' ? val : val.id) : undefined ), + scope: ScopeOptionSchema.optional(), + sessions: SessionAllowlistSchema.optional(), reasoningLevel: z .enum(['minimal', 'low', 'medium', 'high', 'max']) .optional(), @@ -319,6 +516,7 @@ export const ChatQuerySchema = z .optional(), }) .strict() + .superRefine(scopeExclusivityIssues) /** * Schema for representation options. @@ -356,11 +554,40 @@ export const ContextParamsSchema = z tokens: z.int('Token limit must be an integer').optional(), peerTarget: PeerIdSchema.optional(), peerPerspective: PeerIdSchema.optional(), + // Only a single scope is accepted here: the context route uses a scope as + // the *perspective source* for the target's representation and card, which + // is one observer. A list of scopes has no meaning for that. + scope: ScopeIdSchema.optional(), + sessions: SessionAllowlistSchema.optional(), limitToSession: z.boolean().optional(), representationOptions: RepresentationOptionsSchema.optional(), }) .strict() .superRefine((data, ctx) => { + if (data.sessions && !data.peerTarget) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'peerTarget is required when sessions is provided', + path: ['sessions'], + }) + } + + if (data.sessions && data.scope) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'sessions and scope are mutually exclusive', + path: ['sessions'], + }) + } + + if (data.sessions && data.limitToSession) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'sessions and limitToSession are mutually exclusive', + path: ['sessions'], + }) + } + if (data.representationOptions?.searchQuery && !data.peerTarget) { ctx.addIssue({ code: z.ZodIssueCode.custom, @@ -376,6 +603,22 @@ export const ContextParamsSchema = z path: ['peerPerspective'], }) } + + if (data.scope && !data.peerTarget) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'peerTarget is required when scope is provided', + path: ['scope'], + }) + } + + if (data.scope && data.peerPerspective) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'scope and peerPerspective are mutually exclusive', + path: ['scope'], + }) + } }) /** @@ -437,10 +680,13 @@ export const GetRepresentationParamsSchema = z export const PeerGetRepresentationParamsSchema = z .object({ session: z.union([SessionIdSchema, SessionIdObjectSchema]).optional(), + scope: ScopeOptionSchema.optional(), + sessions: SessionAllowlistSchema.optional(), target: z.union([PeerIdSchema, PeerIdObjectSchema]).optional(), options: RepresentationOptionsSchema.optional(), }) .strict() + .superRefine(scopeExclusivityIssues) /** * Schema for peer card target parameter. diff --git a/skills/pre-pr/SKILL.md b/skills/pre-pr/SKILL.md new file mode 100644 index 00000000..0c1e1797 --- /dev/null +++ b/skills/pre-pr/SKILL.md @@ -0,0 +1,63 @@ +--- +name: pre-pr +description: Prepare a Honcho change for a pull request to plastic-labs/honcho. Invoke before opening a PR, when drafting a PR body, when asked if a branch is PR-ready, or when filling the pull request template. Checks the linked issue, required tests and docs, then writes Description / Proofs / Fixes. +--- + +# Pre-PR checklist + +Do this after the change works, before anyone opens the GitHub PR. Output is a filled template body — do not create the PR. + +The template lives at `.github/pull_request_template.md`. Do not add extra sections. + +## 1. Issue gate (hard stop) + +A PR without a maintainer-approved issue will be closed. + +```bash +gh issue view --repo plastic-labs/honcho --json number,title,labels,state +``` + +Stop if any of these fail: + +- no issue number, or the issue is not in `plastic-labs/honcho` +- issue is closed (unless this PR is explicitly reopening it) +- labels do not include `maintainer-approved` + +Say which check failed. Do not draft a PR body around it. + +## 2. Classify the diff + +```bash +git diff main...HEAD --stat +``` + +Pick one primary kind: bug, feature, docs. Then decide layers: + +| Surface touched | Required | +| --- | --- | +| `src/` (non-prompt) | unit tests under the matching `tests/` tree | +| deriver / dialectic / dreamer / LLM path | unit + consider live-llm (`tests/live_llm`) | +| queue, config hierarchy, multi-turn, SDK contract | unified (`uv run python -m tests.unified.run`) | +| `/v3` HTTP or deriver queue behavior | `/verify` skill (runtime, not just pytest) | +| public API, SDK exports, `config.toml` / settings, mintlify `docs/` | documentation in the matching file | + +Skip a layer only with a one-line reason (e.g. "docs-only", "comment-only"). "When appropriate" is not a skip. + +Invoke `/verify` when the runtime surface moved. Do not restate that skill here. + +Lint/type before claiming tests are green: `uv run ruff check src/` → `uv run basedpyright` → the pytest command for the layer. + +## 3. Proofs + +Collect evidence that belongs in the PR, not in the commit: + +- command + pass/fail for what you ran +- a log snippet, screenshot, or file path that shows the new behavior +- for bugs: the failing case before vs after, if you have it + +If `/verify` ran, the proofs *are* that session's output. Do not invent green runs. + +## 4. Write the body + +Fill the description, proofs, checklist portion of the pull request description template. +Make sure to link the related github issue, otherwise the PR will be auto-closed. diff --git a/src/config.py b/src/config.py index d8100694..993f9bfb 100644 --- a/src/config.py +++ b/src/config.py @@ -2,7 +2,7 @@ import logging import math import os from pathlib import Path -from typing import Annotated, Any, ClassVar, Literal, cast +from typing import Annotated, Any, ClassVar, Literal, cast, get_args from urllib.parse import urlparse import tomllib @@ -394,6 +394,15 @@ class ConfiguredEmbeddingModelSettings(BaseModel): dimensions_mode: EmbeddingDimensionsMode = "auto" encoding_format_mode: EmbeddingEncodingFormatMode = "auto" max_batch_size: Annotated[int, Field(gt=0)] | None = None + # Client HTTP timeout in seconds. OpenAI receives seconds; Gemini converts to ms. + timeout: float | None = None + + @field_validator("timeout", mode="before") + @classmethod + def _validate_timeout(cls, v: Any) -> float | None: + if v is None: + return None + return coerce_provider_timeout(v) @model_validator(mode="before") @classmethod @@ -431,6 +440,15 @@ class EmbeddingModelConfig(BaseModel): api_key: str | None = None base_url: str | None = None max_batch_size: Annotated[int, Field(gt=0)] | None = None + # Client HTTP timeout in seconds. OpenAI receives seconds; Gemini converts to ms. + timeout: float | None = None + + @field_validator("timeout", mode="before") + @classmethod + def _validate_timeout(cls, v: Any) -> float | None: + if v is None: + return None + return coerce_provider_timeout(v) @model_validator(mode="before") @classmethod @@ -556,6 +574,7 @@ def resolve_embedding_model_config( api_key=api_key, base_url=configured.overrides.base_url, max_batch_size=configured.max_batch_size, + timeout=configured.timeout, ) @@ -979,15 +998,14 @@ class PeerCardSettings(HonchoSettings): ENABLED: bool = True -# Reasoning levels for dialectic - defined here to avoid circular imports with schemas +# Reasoning levels for dialectic - defined here to avoid circular imports with schemas. +# region ai +# REASONING_LEVELS is derived from the Literal, not hand-listed: the annotation +# rejects an invalid member but not a MISSING one, so a hand-written copy could +# silently drop a level and still typecheck. +# endregion ReasoningLevel = Literal["minimal", "low", "medium", "high", "max"] -REASONING_LEVELS: list[ReasoningLevel] = [ - "minimal", - "low", - "medium", - "high", - "max", -] +REASONING_LEVELS: list[ReasoningLevel] = list(get_args(ReasoningLevel)) class DialecticLevelSettings(BaseModel): diff --git a/src/crud/document.py b/src/crud/document.py index ba80712d..0cec85c9 100644 --- a/src/crud/document.py +++ b/src/crud/document.py @@ -984,7 +984,9 @@ async def create_observations( # Generate embeddings in batch contents = [obs.content for obs in observations] try: - embeddings = await embedding_client.simple_batch_embed(contents) + embeddings = await embedding_client.simple_batch_embed( + contents, on_oversize="truncate" + ) except ValueError as e: raise ValidationException(str(e)) from e diff --git a/src/crud/representation.py b/src/crud/representation.py index 57e6e8e1..3b7070e8 100644 --- a/src/crud/representation.py +++ b/src/crud/representation.py @@ -107,7 +107,7 @@ class RepresentationManager: parent_category="representation", ): embeddings = await embedding_client.simple_batch_embed( - observation_texts + observation_texts, on_oversize="truncate" ) except ValueError as e: raise exceptions.ValidationException( diff --git a/src/deriver/__main__.py b/src/deriver/__main__.py index c56ed6a0..ce3969b5 100644 --- a/src/deriver/__main__.py +++ b/src/deriver/__main__.py @@ -10,6 +10,7 @@ from src.db import engine, register_db_query_instrumentation from src.startup import validate_embedding_schema from src.telemetry import ( initialize_telemetry_async, + prometheus_metrics, register_db_pool_collector, shutdown_telemetry, ) @@ -25,6 +26,12 @@ def start_metrics_server() -> None: # Expose DB connection-pool stats for this deriver instance. register_db_pool_collector("deriver") register_db_query_instrumentation("deriver") + + # region ai + # Zero-init bounded-label counters so a missing series signals a broken scrape, + # not "no events" — see initialize_bounded_metrics. No-op if metrics off. + # endregion + prometheus_metrics.initialize_bounded_metrics(instance_type="deriver") logger.info("Prometheus metrics server started on port 9090") diff --git a/src/deriver/deriver.py b/src/deriver/deriver.py index 2ad7d14f..f76c4d52 100644 --- a/src/deriver/deriver.py +++ b/src/deriver/deriver.py @@ -7,6 +7,7 @@ from src import crud from src.config import ConfiguredModelSettings, settings from src.crud.representation import RepresentationManager from src.dependencies import tracked_db +from src.exceptions import RepresentationSaveError from src.llm import honcho_llm_call from src.llm.types import LLMTelemetryContext from src.models import Message @@ -196,6 +197,7 @@ async def process_representation_tasks_batch( agg_representation_result = crud.CreateDocumentsResult() successful_observer_count = 0 + save_errors: list[tuple[str, Exception]] = [] if observations.is_empty() or not message_ids: logger.warning( "Deriver generated zero observations for messages %s:%s in %s/%s!", @@ -236,10 +238,11 @@ async def process_representation_tasks_batch( representation_result.semantic_dup_replaced_count ) successful_observer_count += 1 - except Exception as e: - logger.error( - "Failed to save representation for observer %s: %s", observer, e + except Exception as e: # noqa: BLE001 + logger.exception( + "Failed to save representation for observer %s", observer ) + save_errors.append((observer, e)) # Log metrics overall_duration = (time.perf_counter() - overall_start) * 1000 @@ -337,5 +340,16 @@ async def process_representation_tasks_batch( exact_dup_in_batch_count=agg_representation_result.exact_dup_in_batch_count, semantic_dup_rejected_count=agg_representation_result.semantic_dup_rejected_count, semantic_dup_replaced_count=agg_representation_result.semantic_dup_replaced_count, + failed_observer_count=len(save_errors), ) ) + + if save_errors and successful_observer_count == 0: + details = "; ".join( + f"{observer}: {exc.__class__.__name__}: {exc}" + for observer, exc in save_errors + ) + raise RepresentationSaveError( + f"save_representation failed for all {len(save_errors)} observer(s): " + + details + ) from save_errors[0][1] diff --git a/src/deriver/queue_manager.py b/src/deriver/queue_manager.py index e4c8124b..1c19c131 100644 --- a/src/deriver/queue_manager.py +++ b/src/deriver/queue_manager.py @@ -478,8 +478,7 @@ class QueueManager: @staticmethod def _is_tenant_work(work_unit_keys: Iterable[str]) -> bool: - """True if any claimed work unit is real tenant work, not housekeeping. - """ + """True if any claimed work unit is real tenant work, not housekeeping.""" for key in work_unit_keys: try: if parse_work_unit_key(key).task_type != "reconciler": diff --git a/src/embedding_client.py b/src/embedding_client.py index 08a4ff04..e55e09fd 100644 --- a/src/embedding_client.py +++ b/src/embedding_client.py @@ -195,13 +195,13 @@ class _EmbeddingClient: from google import genai from google.genai import types as genai_types - # 10-minute HTTP timeout, in lockstep with the LLM registry's Gemini - # client (`src/llm/registry.py:_build_gemini_http_options`). Without - # this, a stalled Gemini embedding socket wedges the deriver worker - # exactly the way #785 describes for the LLM client. + # Default 10-minute HTTP timeout matches the LLM registry Gemini client. + timeout_ms = ( + int(config.timeout * 1000) if config.timeout is not None else 600_000 + ) http_options = genai_types.HttpOptions( base_url=config.base_url, - timeout=600_000, + timeout=timeout_ms, ) self.client: genai.Client | AsyncOpenAI = genai.Client( api_key=config.api_key, @@ -216,10 +216,14 @@ class _EmbeddingClient: raise ValueError("OpenAI API key is required") from openai import AsyncOpenAI - self.client = AsyncOpenAI( - api_key=config.api_key, - base_url=config.base_url, - ) + # Omit timeout when unset so the OpenAI SDK keeps its own default. + client_kwargs: dict[str, Any] = { + "api_key": config.api_key, + "base_url": config.base_url, + } + if config.timeout is not None: + client_kwargs["timeout"] = config.timeout + self.client = AsyncOpenAI(**client_kwargs) self.max_embedding_tokens = max_input_tokens self.max_batch_size = config.max_batch_size or 2048 @@ -317,39 +321,78 @@ class _EmbeddingClient: fn=_call_openai, ) - async def simple_batch_embed(self, texts: list[str]) -> list[list[float]]: + def _truncate_to_token_limit(self, text: str) -> tuple[str, int]: + """Return a prefix of `text` whose re-encoded token count fits the cap. + + Decode/re-encode after slicing: BPE boundaries can re-expand past the cap. """ - Batch-embed a list of text strings. Each input must already fit within - `max_embedding_tokens`; this method does not sub-chunk oversized inputs. + token_ids = self.encoding.encode(text) + keep = self.max_embedding_tokens + while len(token_ids) > self.max_embedding_tokens: + keep = min(keep, len(token_ids) - 1) + if keep < 1: + return "", 0 + text = self.encoding.decode(token_ids[:keep]) + token_ids = self.encoding.encode(text) + keep -= 1 + return text, len(token_ids) + + async def simple_batch_embed( + self, + texts: list[str], + *, + on_oversize: Literal["raise", "truncate"] = "raise", + ) -> list[list[float]]: + """ + Batch-embed a list of text strings. Does not sub-chunk oversized inputs. Internally goes through the same token-aware batching pipeline as `batch_embed()` so the per-request token cap is respected. Args: texts: List of text strings to embed + on_oversize: ``"raise"`` (default) errors; ``"truncate"`` embeds a + token-capped prefix. Returns: List of embedding vectors, one per input text (in order) Raises: - ValueError: If any text exceeds token limits + ValueError: If any text exceeds token limits and `on_oversize` is + ``"raise"`` """ if not texts: return [] - # Validate per-input token limit and collect token counts for batching + # Validate / cap per-input token limit and collect counts for batching + prepared_texts: list[str] = [] token_counts: list[int] = [] for idx, text in enumerate(texts): - tokens = len(self.encoding.encode(text)) - if tokens > self.max_embedding_tokens: - raise ValueError( - f"Text at index {idx} exceeds maximum token limit of {self.max_embedding_tokens} tokens (got {tokens} tokens)" - ) + token_ids = self.encoding.encode(text) + if len(token_ids) > self.max_embedding_tokens: + if on_oversize == "truncate": + original_count = len(token_ids) + text, tokens = self._truncate_to_token_limit(text) + logger.warning( + "truncated oversize embedding input at idx %d: %d->%d tokens", + idx, + original_count, + tokens, + ) + else: + raise ValueError( + f"Text at index {idx} exceeds maximum token limit of " + + f"{self.max_embedding_tokens} tokens (got {len(token_ids)} tokens)" + ) + else: + tokens = len(token_ids) + prepared_texts.append(text) token_counts.append(tokens) # Use positional indices as text_ids so we can reassemble in input order. text_chunks: dict[str, list[tuple[str, int]]] = { - str(i): [(text, token_counts[i])] for i, text in enumerate(texts) + str(i): [(prepared_texts[i], token_counts[i])] + for i in range(len(prepared_texts)) } batches = self._create_batches(text_chunks) @@ -691,9 +734,16 @@ class EmbeddingClient: """Embed a single query string.""" return await self._get_client().embed(query) - async def simple_batch_embed(self, texts: list[str]) -> list[list[float]]: + async def simple_batch_embed( + self, + texts: list[str], + *, + on_oversize: Literal["raise", "truncate"] = "raise", + ) -> list[list[float]]: """Batch embed a list of text strings (each must fit token limit).""" - return await self._get_client().simple_batch_embed(texts) + return await self._get_client().simple_batch_embed( + texts, on_oversize=on_oversize + ) def prepare_chunks(self, id_resource_dict: dict[str, str]) -> dict[str, list[str]]: """Chunk texts using the same rules as `batch_embed` (no network).""" diff --git a/src/exceptions.py b/src/exceptions.py index 129324b6..41d775fe 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -133,6 +133,14 @@ class VectorStoreError(HonchoException): detail = "Vector store operation failed" +@final +class RepresentationSaveError(HonchoException): + """Raised when every observer's representation save fails in a batch.""" + + status_code: int = 500 + detail: str = "Representation save failed for all observers" + + class LLMError(Exception): """Exception raised when an LLM call fails. diff --git a/src/llm/history_adapters.py b/src/llm/history_adapters.py index 02d2ea05..8e3b837e 100644 --- a/src/llm/history_adapters.py +++ b/src/llm/history_adapters.py @@ -121,6 +121,8 @@ class OpenAIHistoryAdapter: } if result.reasoning_details: message["reasoning_details"] = result.reasoning_details + elif result.thinking_content: + message["reasoning_content"] = result.thinking_content return message def format_tool_results( diff --git a/src/llm/tool_loop.py b/src/llm/tool_loop.py index 0feec1d8..783d4965 100644 --- a/src/llm/tool_loop.py +++ b/src/llm/tool_loop.py @@ -212,6 +212,7 @@ def format_assistant_tool_message( tool_calls: list[dict[str, Any]], thinking_blocks: list[dict[str, Any]] | None = None, reasoning_details: list[dict[str, Any]] | None = None, + thinking_content: str | None = None, ) -> dict[str, Any]: """Format an assistant message with tool calls in provider-native shape.""" from .backend import CompletionResult as BackendCompletionResult @@ -229,6 +230,7 @@ def format_assistant_tool_message( ) for tool_call in tool_calls ], + thinking_content=thinking_content, thinking_blocks=thinking_blocks or [], reasoning_details=reasoning_details or [], ) @@ -573,6 +575,7 @@ async def execute_tool_loop( response.tool_calls_made, response.thinking_blocks, response.reasoning_details, + response.thinking_content, ) conversation_messages.append(assistant_message) diff --git a/src/main.py b/src/main.py index 75eac455..6930d4c1 100644 --- a/src/main.py +++ b/src/main.py @@ -110,6 +110,12 @@ async def lifespan(_: FastAPI): register_db_pool_collector("api") register_db_query_instrumentation("api") + # region ai + # Zero-init bounded-label counters so a missing series signals a broken scrape, + # not "no events" — see initialize_bounded_metrics. No-op if metrics off. + # endregion + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + # Validate embedding schema before serving any traffic. Fails closed: if # the configured EMBEDDING_VECTOR_DIMENSIONS does not match the physical # pgvector columns, the process refuses to start rather than silently diff --git a/src/reconciler/scheduler.py b/src/reconciler/scheduler.py index e171c4fa..08941970 100644 --- a/src/reconciler/scheduler.py +++ b/src/reconciler/scheduler.py @@ -21,6 +21,7 @@ from src import models from src.config import settings from src.dependencies import tracked_db from src.models import QueueItem +from src.reconciler.sync_vectors import record_pending_embeddings_backlog logger = logging.getLogger(__name__) @@ -145,15 +146,26 @@ class ReconcilerScheduler: async def _scheduler_loop(self) -> None: """ - Main scheduler loop that enqueues tasks based on their intervals. + Main scheduler loop that enqueues tasks based on their intervals, and + refreshes the service-wide pending-embeddings backlog gauge each pass. Each task has its own interval and the loop checks all tasks on each - iteration, enqueueing any that are due. + iteration, enqueueing any that are due. The loop sleeps until the next + task is due, so the gauge's refresh cadence tracks the SHORTEST task + interval. """ try: while not self._shutdown_event.is_set(): now = datetime.now(timezone.utc) + # region ai + # Refresh on EVERY replica, not just whichever wins the sync_vectors + # work unit: the count is DB-global, so a replica that never ran a + # cycle would otherwise export a stale (or zero-initialized) value + # forever. Full rationale in record_pending_embeddings_backlog. + # endregion + await record_pending_embeddings_backlog() + # Check each task and enqueue if due for task_name, task in RECONCILER_TASKS.items(): next_run = self._next_run.get(task_name, now) diff --git a/src/reconciler/sync_vectors.py b/src/reconciler/sync_vectors.py index 68e9ac59..1a8e99b5 100644 --- a/src/reconciler/sync_vectors.py +++ b/src/reconciler/sync_vectors.py @@ -23,6 +23,7 @@ from src.config import settings from src.dependencies import tracked_db from src.embedding_client import embedding_client from src.exceptions import VectorStoreError +from src.telemetry import prometheus_metrics from src.telemetry.events import EmbeddingCallPurpose from src.utils.types import embedding_call_purpose from src.vector_store import VectorRecord, VectorStore, get_external_vector_store @@ -301,7 +302,9 @@ async def _sync_documents( EmbeddingCallPurpose.VECTOR_SYNC.value, parent_category="reconciliation", ): - new_embeddings = await embedding_client.simple_batch_embed(contents) + new_embeddings = await embedding_client.simple_batch_embed( + contents, on_oversize="truncate" + ) if len(new_embeddings) != len(docs_needing_embed): logger.warning( @@ -700,6 +703,42 @@ async def _cleanup_pgvector_batch( return True +async def record_pending_embeddings_backlog() -> None: + """Set the pending-embeddings backlog gauge to the current count of + MessageEmbedding rows awaiting a vector (sync_state='pending').""" + # region ai + # Called from ``ReconcilerScheduler._scheduler_loop``, deliberately NOT from + # ``run_vector_reconciliation_cycle``: the cycle runs off the queue behind + # work-unit dedup, so exactly one deriver replica executes it. Driving the gauge + # from there would leave every other replica exporting a stale value — or, since + # this metric is zero-initialized, a confident permanent 0 it never measured. The + # count is a property of the database, not the process, so every replica must + # refresh it on its own timer for ``max()``/``avg()`` to mean anything. + # + # Cost: one COUNT per replica per scheduler interval (~5 min by default). + # ``ix_message_embeddings_sync_state_last_sync_at`` keeps the scan proportional to + # the pending backlog, not the whole table — which is not the same as cheap: after + # an embedding outage the backlog is exactly what is large. Still a small duty + # cycle, and the cost shrinks as the reconciler drains. + # + # Best-effort: a metrics/DB hiccup here must never break the scheduler loop. + # endregion + if not settings.METRICS.ENABLED: + return + try: + async with tracked_db("reconciler_pending_count", read_only=True) as db: + count = await db.scalar( + select(func.count()) + .select_from(models.MessageEmbedding) + .where(models.MessageEmbedding.sync_state == "pending") + ) + prometheus_metrics.set_message_embeddings_pending(count=count or 0) + except Exception: + logger.warning( + "Failed to record pending-embeddings backlog gauge", exc_info=True + ) + + async def run_vector_reconciliation_cycle() -> ReconciliationMetrics: """ Run a complete reconciliation cycle. diff --git a/src/routers/sessions.py b/src/routers/sessions.py index 10b769c4..f0d5b056 100644 --- a/src/routers/sessions.py +++ b/src/routers/sessions.py @@ -11,6 +11,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from src import config, crud, schemas from src.cache.client import safe_cache_delete +from src.crud.message import get_peer_session_names from src.crud.session import session_cache_key from src.dependencies import db, read_db from src.deriver.enqueue import enqueue_deletion @@ -23,6 +24,7 @@ from src.exceptions import ( from src.security import JWTParams, require_auth from src.telemetry.events import EmbeddingCallPurpose, GetContextEvent, emit from src.utils import summarizer +from src.utils.filter import normalize_session_allowlist from src.utils.representation import Representation from src.utils.search import search from src.utils.tokens import estimate_tokens @@ -714,9 +716,27 @@ async def get_session_context( None, description="An (unprefixed) scope name to use as the perspective source: the representation and peer card of `peer_target` are read from the scope's observations instead of the global (or `peer_perspective`) view. Must be provided with `peer_target`; mutually exclusive with `peer_perspective`. Requires a workspace- or admin-level key.", ), + sessions: list[str] | None = Query( + None, + description=( + "Optional allowlist of session IDs confining the representation of " + "`peer_target` to those sessions. This session must be one of them. " + "Recall is restricted to conclusions stated directly in the allowed " + "sessions — conclusions synthesized across sessions are excluded, " + "since their provenance cannot be proven to sit inside the allowlist " + "— and the peer card is omitted for the same reason. Mutually " + "exclusive with `scope` and `limit_to_session`. A peer-scoped key " + "must be an active member of every session named. The 1,000-session " + "cap shared with the recall endpoints applies but is not reachable " + "here: these are repeated query parameters, so a long list exceeds " + "the request-line limit of the server or any proxy in front of it " + "(a 414/431, not a 422) at a few hundred entries. Use a named " + "`scope` for large or reusable session sets." + ), + ), limit_to_session: bool = Query( default=False, - description="Only used if `search_query` is provided. Whether to limit the representation to the session (as opposed to everything known about the target peer)", + description="Whether to limit the representation to the session (as opposed to everything known about the target peer). Narrows recall the same way `sessions` does, so the same restrictions apply: explicit-only conclusions, and the peer card is omitted because it carries no per-session provenance.", ), search_top_k: int | None = Query( None, @@ -801,6 +821,47 @@ async def get_session_context( "`scope` requires a workspace- or admin-level key" ) + # The session allowlist confines the representation to a set of sessions this + # one belongs to. `scope` already determines what can be seen and + # `limit_to_session` already pins the set to this session alone, so both are + # contradictions rather than further narrowings — refused rather than given a + # silent precedence order. + session_allowlist: list[str] | None = None + if sessions is not None: + if scope is not None: + raise ValidationException("`sessions` and `scope` are mutually exclusive") + if limit_to_session: + raise ValidationException( + "`sessions` and `limit_to_session` are mutually exclusive" + ) + if not peer_target: + # The allowlist only reaches the representation, and there is no + # representation without a target. Refused rather than accepted and + # silently ignored, which would read as a scoped context. + raise ValidationException( + "peer_target must be provided if sessions is provided" + ) + # `must_include` keeps the allowlist from contradicting the route's own + # session: this session's messages and summary are always part of the + # response, so an allowlist excluding it would describe a context that + # cannot be assembled. + session_allowlist = normalize_session_allowlist( + sessions, field="sessions", must_include=session_id + ) + # A peer-scoped key may only name sessions its peer belongs to. Mirrors + # the chat route's gate (see routers/peers.py), including `active_only`, + # so both answer the same question for a peer that has left a session. + # Reuses the handler's session rather than opening its own: this is a + # DB-only read and the handler already holds a connection. + if jwt_params.p is not None: + member_sessions = set( + await get_peer_session_names( + db, workspace_id, jwt_params.p, active_only=True + ) + ) + if not set(session_allowlist) <= member_sessions: + raise AuthenticationException("JWT not permissioned for this resource") + if not peer_target: # No representation or card needed summary, messages = await _get_session_context_task( @@ -865,6 +926,17 @@ async def get_session_context( ): embedding = await embedding_client.embed(search_query) + # The allowlist recall must respect, whichever way the caller expressed it. + # `sessions` and `limit_to_session` are mutually exclusive (422 above), so at + # most one of these is set. `session_allowlist` is never an empty list here — + # `must_include=session_id` guarantees at least this session — so the + # None-check is the only distinction that matters. + effective_allowlist = ( + session_allowlist + if session_allowlist is not None + else ([session_id] if limit_to_session else None) + ) + # Sequential calls on shared DB session representation = await _get_working_representation_task( db, @@ -872,15 +944,34 @@ async def get_session_context( search_query, observer=observer, observed=observed, - session_allowlist=[session_id] if limit_to_session else None, + session_allowlist=effective_allowlist, search_top_k=search_top_k, search_max_distance=search_max_distance, include_most_derived=include_most_frequent, max_observations=max_conclusions, embedding=embedding, ) - card = await _get_peer_card_task( - db, workspace_id, observer=observer, observed=observed + # A peer card is keyed by (workspace, observer, observed) with no session + # dimension (crud/peer_card.py), so it is synthesized from everything the + # observer has ever seen and cannot be narrowed to an allowlist. Returning it + # would leak exactly what the allowlist exists to exclude, so it is dropped — + # the same fail-closed reasoning that limits allowlisted conclusion recall to + # ALLOWLIST_SAFE_LEVELS. + # + # Gated on the *effective* allowlist, not on `sessions` alone: + # `limit_to_session=true` narrows recall identically, so carving out only the + # newer parameter would leave a control that one parameter swap defeats. + # `scope` needs no carve-out at all — it swaps the observer to the scope peer + # above, so the card read below is the scope's own. + # + # POST /peers/{id}/chat still injects an unscoped card under an allowlist + # (src/dialectic/chat.py) — tracked in DEV-2201, not fixed here. + card = ( + None + if effective_allowlist is not None + else await _get_peer_card_task( + db, workspace_id, observer=observer, observed=observed + ) ) short_summary, long_summary = await _get_both_summaries_task( db, workspace_id, session_id diff --git a/src/telemetry/emitter.py b/src/telemetry/emitter.py index 201d16aa..397ecd90 100644 --- a/src/telemetry/emitter.py +++ b/src/telemetry/emitter.py @@ -172,6 +172,21 @@ class TelemetryEmitter: ) self._running = True self._flush_task = asyncio.create_task(self._periodic_flush()) + + # region ai + # Pre-create the dropped-event counter children at 0: a labeled counter + # exports nothing until its first observation, so this makes the metric + # visible before any drop and lets us tell "no drops" from "metric missing". + # endregion + from src.telemetry.prometheus.metrics import prometheus_metrics + + prometheus_metrics.initialize_telemetry_dropped_metrics( + reasons=[ + f"{self.drop_reason_prefix}buffer_full", + f"{self.drop_reason_prefix}send_failed", + ] + ) + logger.info("Telemetry emitter started, endpoint: %s", self.endpoint) async def shutdown(self) -> None: diff --git a/src/telemetry/events/__init__.py b/src/telemetry/events/__init__.py index 000e70b4..f8950d0f 100644 --- a/src/telemetry/events/__init__.py +++ b/src/telemetry/events/__init__.py @@ -138,9 +138,74 @@ __all__ = [ # Lifecycle "initialize_telemetry_events", "shutdown_telemetry_events", + # Zero-init registry + "ALL_EVENT_TYPES", + "HIGH_VOLUME_EVENT_TYPES", ] +# Explicit registry of CloudEvents `type` values, used to zero-initialize the +# telemetry_events_emitted / telemetry_events_sampled_out counter children. +# region ai +# See metrics.py:initialize_bounded_metrics for why absent and zero are worth +# distinguishing. Explicit literal, not a set derived from BaseEvent subclasses: a +# derived set would follow whatever happens to be imported at init time, so a type +# could drop out of the registry with no code change. A hand-maintained list plus a +# drift-guard test fails loud at the right moment instead. +# +# When you add a BaseEvent subclass, add its `_event_type` here (and to +# HIGH_VOLUME_EVENT_TYPES if `_volume_class == "high_volume"`). The drift-guard test +# tests/telemetry/test_metric_zero_init.py fails until you do — it asserts this +# registry equals the set discovered by walking BaseEvent subclasses. +# endregion +ALL_EVENT_TYPES: tuple[str, ...] = ( + # api + "message.created", + "file.uploaded", + "context.retrieved", + # agent + "agent.iteration", + "agent.tool.conclusions.created", + "agent.tool.conclusions.deleted", + "agent.tool.peer_card.updated", + "agent.tool.summary.created", + "agent.tool.call.completed", + # deletion / dialectic / dream / representation + "deletion.completed", + "dialectic.completed", + "dream.run", + "dream.specialist", + "representation.completed", + # llm / embedding + "llm.call.completed", + "embedding.call.completed", + # reconciliation + "reconciliation.sync_vectors.completed", + "reconciliation.cleanup_stale_items.completed", + # trace stream + # region ai + # Only emitted when TELEMETRY.TRACE_PAYLOADS_ENABLED, but they flow through the + # same emit() path and increment the same counters, so they belong in the set. + # endregion + "llm.call.traced", + "embedding.call.traced", + "trace.content", +) + +# Subset of ALL_EVENT_TYPES whose `_volume_class == "high_volume"`. +# region ai +# Only these can ever be counted by ``telemetry_events_sampled_out``; ground_truth +# events skip the sampler entirely (pre-creating their sampled_out series would be a +# permanently-misleading 0). +# endregion +HIGH_VOLUME_EVENT_TYPES: tuple[str, ...] = ( + "agent.iteration", + "agent.tool.call.completed", + "llm.call.completed", + "embedding.call.completed", +) + + def emit(event: BaseEvent) -> None: """Queue an event for emission to the telemetry backend. diff --git a/src/telemetry/events/representation.py b/src/telemetry/events/representation.py index ad6dfca4..c4291092 100644 --- a/src/telemetry/events/representation.py +++ b/src/telemetry/events/representation.py @@ -154,6 +154,10 @@ class RepresentationCompletedEvent(BaseEvent): default=0, description="Number of observers this representation was saved against", ) + failed_observer_count: int = Field( + default=0, + description="Number of observers whose save_representation failed (partial or total)", + ) def get_resource_id(self) -> str: """Resource ID includes workspace, session, and latest message for uniqueness.""" diff --git a/src/telemetry/prometheus/metrics.py b/src/telemetry/prometheus/metrics.py index 01d7be4f..749591d9 100644 --- a/src/telemetry/prometheus/metrics.py +++ b/src/telemetry/prometheus/metrics.py @@ -20,7 +20,8 @@ from prometheus_client.core import GaugeMetricFamily from starlette.requests import Request from starlette.responses import Response -from src.config import settings +from src.config import REASONING_LEVELS, settings +from src.utils.types import walk_subclasses disable_created_metrics() @@ -66,6 +67,32 @@ class DialecticComponents(Enum): TOTAL = "total" +# Valid (token_type, component) pairs for deriver_tokens_processed, per task_type, +# used to zero-initialize counter children (see initialize_bounded_metrics). +# region ai +# NOT the cartesian product: input tokens only pair with input components, output +# only with OUTPUT_TOTAL, and PREVIOUS_SUMMARY occurs only for summary tasks +# (ingestion has no previous summary). Enumerating anything broader would fabricate +# impossible always-0 series (e.g. output/prompt, or ingestion/previous_summary). +# Explicit literal, drift-guarded by tests/telemetry/test_metric_zero_init.py. +# Sources: track_deriver_input_tokens (src/utils/tokens.py) + the OUTPUT_TOTAL sites +# in src/deriver/deriver.py and src/utils/summarizer.py. +# endregion +_DERIVER_TOKEN_COMBOS_BY_TASK: dict[str, tuple[tuple[str, str], ...]] = { + DeriverTaskTypes.INGESTION.value: ( + (TokenTypes.INPUT.value, DeriverComponents.PROMPT.value), + (TokenTypes.INPUT.value, DeriverComponents.MESSAGES.value), + (TokenTypes.OUTPUT.value, DeriverComponents.OUTPUT_TOTAL.value), + ), + DeriverTaskTypes.SUMMARY.value: ( + (TokenTypes.INPUT.value, DeriverComponents.PROMPT.value), + (TokenTypes.INPUT.value, DeriverComponents.MESSAGES.value), + (TokenTypes.INPUT.value, DeriverComponents.PREVIOUS_SUMMARY.value), + (TokenTypes.OUTPUT.value, DeriverComponents.OUTPUT_TOTAL.value), + ), +} + + api_requests_counter = NamespacedCounter( "api_requests", "Total API requests", @@ -155,6 +182,23 @@ telemetry_buffer_size_gauge = NamespacedGauge( ["namespace"], ) +# Embedding backlog: MessageEmbedding rows still awaiting a vector +# (sync_state='pending'). +# region ai +# Distinct from embed_now_tasks_in_flight (in-flight fast-path work in the API +# process) — this is the durable, DB-wide backlog the reconciler drains. Every +# deriver replica refreshes it on its own timer from +# ReconcilerScheduler._scheduler_loop, so replicas disagree by at most one interval. +# Service-wide, not per-process — hence the help string's "never sum()". +# endregion +message_embeddings_pending_gauge = NamespacedGauge( + "message_embeddings_pending", + "MessageEmbedding rows awaiting embedding (sync_state='pending'). " + + "Service-wide DB count, reported independently by every replica — " + + "aggregate with max() or avg(), never sum()", + ["namespace"], +) + # DB connection-pool health. The in-flight gauge counts statements actually # executing on the wire, so checked_out minus in_flight reveals connections held # but parked (the "idle in transaction during an external call" antipattern). @@ -325,12 +369,159 @@ class PrometheusMetrics: except Exception as e: self._handle_metric_error("record_telemetry_event_dropped", e) + def _touch(self, counter: NamespacedCounter, **labels: str) -> None: + """Pre-create a counter child series at 0 without incrementing it.""" + # region ai + # A labeled Prometheus counter exports no time series until its first + # ``labels(...)`` call, so pre-touching a child keeps it present at 0 — a + # missing series then signals a broken scrape rather than "no events". + # Fail-soft (like the recorders): a bad init must never crash startup. + # endregion + try: + counter.labels(**labels) + except Exception as e: + self._handle_metric_error("_touch", e) + + def initialize_telemetry_dropped_metrics(self, *, reasons: list[str]) -> None: + """Pre-create telemetry_events_dropped ``(namespace, reason)`` children at 0. + + Args: + reasons: The reason label values the calling emitter can produce. + """ + # region ai + # The metric stays invisible in Prometheus/Grafana until an event is actually + # dropped, so materializing the children at startup keeps it present at 0 — a + # missing series then means a broken scrape, not "no drops" (see _touch). + # + # Called per-emitter from ``TelemetryEmitter.start()`` rather than hoisted into + # the process-level ``initialize_bounded_metrics``: the trace emitter (whose + # reasons carry a ``trace_`` prefix) only exists when ``TRACE_PAYLOADS_ENABLED`` + # is set, so hoisting would fabricate ``trace_*`` series on deployments that run + # with tracing off. + # endregion + if not settings.METRICS.ENABLED: + return + + for reason in reasons: + self._touch(telemetry_events_dropped_counter, reason=reason) + + def initialize_bounded_metrics(self, *, instance_type: str) -> None: + """Pre-create bounded-label counter children at 0 for this process, so an + absent series means a broken scrape rather than "nothing happened". + + Args: + instance_type: "api" or "deriver" — selects the process-specific + counters. Event-type and buffer metrics are initialized in both. + """ + # region ai + # A Prometheus counter does not exist until its first increment, so a + # never-yet-incremented metric is indistinguishable from a broken scrape: + # you cannot graph or alert on a series that is absent. Materializing the + # children at 0 inverts that — a missing series now means something is wrong, + # and "no events" reads as a flat 0 instead of a gap. + # + # That only holds for label sets we can enumerate honestly, so a metric is + # initialized here only when its full label domain is bounded, enumerable at + # startup, and actually emitted by THIS process. High-cardinality labels + # (endpoint, workspace_name) and impossible label tuples are deliberately left + # absent — fabricating a permanently-0 series that no code path can ever + # increment is the same lie in the other direction. + # + # Multi-instance safety splits the metrics here into three buckets: + # + # 1. instance-scoped (``telemetry_buffer_size``, ``embed_now_tasks_in_flight``) + # — per-process by nature, so any aggregation is meaningful and zero-init is + # unambiguously right. + # 2. service-scoped additive (the token counters, ``telemetry_events_emitted``) + # — each instance holds a partial count and ``sum()`` reconstructs the whole, + # so multi-instance safe. + # 3. service-scoped non-additive — every instance reports the whole service's + # value, so the instances are N witnesses to one fact rather than N parts of + # one whole. ``sum()`` is therefore never correct here: it scales with the + # replica count. Scale-preserving aggregations (``max()``, ``avg()``, + # quantiles) ARE correct, but only while the witnesses disagree by a bounded + # amount — which requires every instance to refresh on its own timer (see + # ``message_embeddings_pending``, refreshed per replica from + # ``ReconcilerScheduler._scheduler_loop``). A bucket-3 metric that cannot + # meet that bar does not belong in the app at all — it belongs in an exporter + # that yields exactly one series. + # + # Prometheus stamps ``instance``/``job`` at scrape time, which is why buckets 1 + # and 2 need no special handling. ``telemetry_events_dropped`` is handled + # separately, per-emitter, in ``TelemetryEmitter.start()`` (prefix-dependent). + # endregion + if not settings.METRICS.ENABLED: + return + + # ai: lazy import avoids an import-time cycle (metrics is imported widely) + from src.telemetry.events import ALL_EVENT_TYPES, HIGH_VOLUME_EVENT_TYPES + + # region ai + # Common: both processes run a TelemetryEmitter, so both emit their own subset + # of event types. The domain is bounded/low-cardinality (~21 types), so init + # the full set in each process rather than maintain a fragile + # per-event-type -> process map. + # endregion + for event_type in ALL_EVENT_TYPES: + self._touch(telemetry_events_emitted_counter, type=event_type) + for event_type in HIGH_VOLUME_EVENT_TYPES: + self._touch(telemetry_events_sampled_out_counter, type=event_type) + self.set_telemetry_buffer_size(size=0) + + if instance_type == "api": + # dialectic tokens: token_type x component(total) x reasoning_level + for token_type in TokenTypes: + for level in REASONING_LEVELS: + self._touch( + dialectic_tokens_processed_counter, + token_type=token_type.value, + component=DialecticComponents.TOTAL.value, + reasoning_level=level, + ) + # ai: embed_now fast path runs as an API-process background task + self._touch(embed_now_tasks_shed_counter) + self.set_embed_now_tasks_in_flight(0) + + elif instance_type == "deriver": + # deriver tokens: only the valid (token_type, component) tuples per + # task_type (see _DERIVER_TOKEN_COMBOS_BY_TASK). + for task_type_value, combos in _DERIVER_TOKEN_COMBOS_BY_TASK.items(): + for token_type_value, component_value in combos: + self._touch( + deriver_tokens_processed_counter, + task_type=task_type_value, + token_type=token_type_value, + component=component_value, + ) + # dreamer tokens: specialist_name x token_type. + # region ai + # Names come from the concrete BaseSpecialist subclasses (walked recursively + # via walk_subclasses) so a new specialist can't silently miss init. + # endregion + from src.dreamer.specialists import BaseSpecialist + + for specialist in walk_subclasses(BaseSpecialist): + for token_type in TokenTypes: + self._touch( + dreamer_tokens_processed_counter, + specialist_name=specialist.name, + token_type=token_type.value, + ) + # ai: init at 0 so the gauge is visible before its first per-replica refresh + self.set_message_embeddings_pending(count=0) + def set_telemetry_buffer_size(self, *, size: int) -> None: try: telemetry_buffer_size_gauge.labels().set(size) except Exception as e: self._handle_metric_error("set_telemetry_buffer_size", e) + def set_message_embeddings_pending(self, *, count: int) -> None: + try: + message_embeddings_pending_gauge.labels().set(count) + except Exception as e: + self._handle_metric_error("set_message_embeddings_pending", e) + prometheus_metrics = PrometheusMetrics() diff --git a/src/utils/agent_tools.py b/src/utils/agent_tools.py index 9f214009..b8af9eee 100644 --- a/src/utils/agent_tools.py +++ b/src/utils/agent_tools.py @@ -930,7 +930,9 @@ async def create_observations( run_id=run_id, parent_category=parent_category, ): - embeddings = await embedding_client.simple_batch_embed(contents) + embeddings = await embedding_client.simple_batch_embed( + contents, on_oversize="truncate" + ) embeddings_by_index = dict( zip(range(len(normalized_observations)), embeddings, strict=True) ) @@ -1010,7 +1012,7 @@ async def create_observations( workspace_name=workspace_name, observer=observer, observed=observed, - deduplicate=True, + deduplicate=settings.DERIVER.DEDUPLICATE, ) ).created_documents logger.info( diff --git a/src/utils/filter.py b/src/utils/filter.py index 1155d408..22d648af 100644 --- a/src/utils/filter.py +++ b/src/utils/filter.py @@ -279,16 +279,50 @@ def extract_session_allowlist( 'filters.session_id must be a session id, a list of session ids, or {"in": [...]}' ) + return normalize_session_allowlist( + entries, field="filters.session_id", must_include=must_include + ) + + +def normalize_session_allowlist( + entries: Sequence[Any], + *, + field: str, + must_include: str | None = None, +) -> list[str]: + """Validate and de-duplicate a session allowlist. + + Shared by every route-level entry point that accepts one — the ``filters`` + body on the recall endpoints and the ``sessions`` query parameter on session + context — so the cap, the id charset, and the ``must_include`` rule cannot + drift apart between them. Only the parameter *name* in error messages + differs, which is what ``field`` supplies. + + Args: + entries: Raw allowlist entries as the caller supplied them. + field: Caller-facing parameter name, used in error messages. + must_include: A session id that must appear in the allowlist — used by + routes that also carry a session of their own, so the two can't + contradict each other. + + Returns: + The allowlist, de-duplicated, in first-seen order. An empty input yields + an empty list so downstream consumers fail closed. + + Raises: + FilterError: On an over-cap list, a malformed session id, or a + ``must_include`` session missing from the allowlist. + """ if len(entries) > MAX_SESSION_ALLOWLIST_ENTRIES: raise FilterError( - f"filters.session_id supports at most {MAX_SESSION_ALLOWLIST_ENTRIES} sessions per request" + f"{field} supports at most {MAX_SESSION_ALLOWLIST_ENTRIES} sessions per request" ) allowlist: list[str] = [] seen: set[str] = set() for entry in entries: if not isinstance(entry, str) or not entry: - raise FilterError("filters.session_id entries must be non-empty strings") + raise FilterError(f"{field} entries must be non-empty strings") # Only names a session could actually have. The allowlist reaches # queries three ways — direct `IN`, the filter DSL, and a Python # membership test — and they don't agree on a value like "*", which the @@ -298,14 +332,14 @@ def extract_session_allowlist( # {"in": [...]}) which never included wildcards. if not re.fullmatch(RESOURCE_NAME_PATTERN, entry): raise FilterError( - f"Invalid session id in filters.session_id: {entry!r}. Session ids match {RESOURCE_NAME_PATTERN}" + f"Invalid session id in {field}: {entry!r}. Session ids match {RESOURCE_NAME_PATTERN}" ) if entry not in seen: seen.add(entry) allowlist.append(entry) if must_include is not None and must_include not in seen: - raise FilterError("session_id must be included in filters.session_id") + raise FilterError(f"session_id must be included in {field}") return allowlist diff --git a/src/utils/types.py b/src/utils/types.py index 24a13ae6..0bfada75 100644 --- a/src/utils/types.py +++ b/src/utils/types.py @@ -1,4 +1,4 @@ -from collections.abc import Awaitable, Callable, Generator +from collections.abc import Awaitable, Callable, Generator, Iterator from contextlib import contextmanager from contextvars import ContextVar from dataclasses import dataclass, field @@ -6,6 +6,20 @@ from typing import Any, Generic, Literal, TypeVar T = TypeVar("T") + +def walk_subclasses(cls: type[T]) -> Iterator[type[T]]: + """Yield every subclass of ``cls``, recursively.""" + # region ai + # ``type.__subclasses__()`` is direct-children-only, so a grandchild class is + # silently invisible to it. Any registry that enumerates subclasses to decide + # what to initialize or validate wants the transitive closure — otherwise + # subclassing a concrete class is enough to slip past the check. + # endregion + for subclass in cls.__subclasses__(): + yield subclass + yield from walk_subclasses(subclass) + + # Context variable for tracking current iteration in tool execution loop # This is used for telemetry to associate tool calls with their iteration _current_iteration: ContextVar[int] = ContextVar("current_iteration", default=0) diff --git a/tests/bench/runner_common.py b/tests/bench/runner_common.py index be262cb0..b150a3f8 100644 --- a/tests/bench/runner_common.py +++ b/tests/bench/runner_common.py @@ -17,21 +17,18 @@ from dataclasses import dataclass, field from datetime import datetime from logging import Logger from pathlib import Path -from typing import Any, Generic, Literal, TypeVar, cast +from typing import Any, Generic, TypeVar, cast from anthropic import AsyncAnthropic from honcho import Honcho from honcho.api_types import SessionConfiguration, SummaryConfiguration from openai import AsyncOpenAI +from src.config import REASONING_LEVELS, ReasoningLevel from src.telemetry.metrics_collector import MetricsCollector _logger = logging.getLogger(__name__) -# Valid reasoning levels for dialectic chat -ReasoningLevel = Literal["minimal", "low", "medium", "high", "max"] -REASONING_LEVELS: list[str] = ["minimal", "low", "medium", "high", "max"] - # Type variable for result types ResultT = TypeVar("ResultT") diff --git a/tests/conftest.py b/tests/conftest.py index f99859b3..8a078652 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -604,7 +604,9 @@ def mock_openai_embeddings(request: pytest.FixtureRequest): mock_embed.side_effect = embed_side_effect - async def mock_simple_batch_embed_func(texts: list[str]) -> list[list[float]]: + async def mock_simple_batch_embed_func( + texts: list[str], **_kwargs: object + ) -> list[list[float]]: return [_content_to_embedding(text) for text in texts] mock_simple_batch_embed.side_effect = mock_simple_batch_embed_func diff --git a/tests/crud/test_document.py b/tests/crud/test_document.py index 5f03c80b..6686e688 100644 --- a/tests/crud/test_document.py +++ b/tests/crud/test_document.py @@ -1006,6 +1006,46 @@ class TestDocumentCRUD: assert documents[0].content in ["Observation 1", "Observation 2"] assert documents[1].content in ["Observation 1", "Observation 2"] + @pytest.mark.asyncio + async def test_create_observations_embeds_with_truncate_on_oversize( + self, + db_session: AsyncSession, + sample_data: tuple[models.Workspace, models.Peer], + ): + """API conclusion creates must opt into truncation on oversize content.""" + test_workspace, test_peer = sample_data + test_peer2, test_session, _ = await self._setup_test_data( + db_session, test_workspace, test_peer + ) + + with patch( + "src.crud.document.embedding_client.simple_batch_embed", + new=AsyncMock(return_value=[[0.1] * 1536, [0.2] * 1536]), + ) as mock_embed: + created = await crud.create_observations( + db_session, + observations=[ + schemas.ConclusionCreate( + content="short conclusion", + observer_id=test_peer.name, + observed_id=test_peer2.name, + session_id=test_session.name, + ), + schemas.ConclusionCreate( + content="another conclusion", + observer_id=test_peer.name, + observed_id=test_peer2.name, + session_id=test_session.name, + ), + ], + workspace_name=test_workspace.name, + ) + + assert len(created) == 2 + mock_embed.assert_awaited_once_with( + ["short conclusion", "another conclusion"], on_oversize="truncate" + ) + class TestSessionPurityInvariant: """Regression tests for the explicit-document session-purity invariant. diff --git a/tests/crud/test_representation_manager.py b/tests/crud/test_representation_manager.py index 6c5e094c..3c2f57e4 100644 --- a/tests/crud/test_representation_manager.py +++ b/tests/crud/test_representation_manager.py @@ -520,7 +520,9 @@ class TestRepresentationManagerSave: ) assert len(saved.created_documents) == 1 - mock_embed.assert_awaited_once_with(["useful observation"]) + mock_embed.assert_awaited_once_with( + ["useful observation"], on_oversize="truncate" + ) saved_observations = _saved_observations(mock_save) assert len(saved_observations) == 1 assert saved_observations[0].content == "useful observation" @@ -576,7 +578,9 @@ class TestRepresentationManagerSave: ) assert len(saved.created_documents) == 1 - mock_embed.assert_awaited_once_with(["inferred conclusion"]) + mock_embed.assert_awaited_once_with( + ["inferred conclusion"], on_oversize="truncate" + ) saved_observations = _saved_observations(mock_save) assert len(saved_observations) == 1 assert isinstance(saved_observations[0], DeductiveObservation) @@ -630,6 +634,61 @@ class TestRepresentationManagerSave: mock_embed.assert_not_awaited() mock_save.assert_not_awaited() + @pytest.mark.asyncio + async def test_save_representation_embeds_with_truncate_on_oversize(self): + """One oversize observation must not drop the rest of the batch.""" + manager = RepresentationManager( + "workspace", + observer="observer", + observed="observed", + ) + representation = Representation( + explicit=[ + ExplicitObservation( + content="short fact", + created_at=datetime.now(timezone.utc), + message_ids=[1], + session_name="session", + ) + ], + deductive=[ + DeductiveObservation( + conclusion="inferred fact", + premises=["premise"], + source_ids=["doc-a"], + created_at=datetime.now(timezone.utc), + message_ids=[1], + session_name="session", + ) + ], + ) + + with ( + patch("src.crud.representation.tracked_db", _fake_tracked_db), + patch( + "src.crud.representation.embedding_client.simple_batch_embed", + new=AsyncMock(return_value=[[0.1], [0.2]]), + ) as mock_embed, + patch.object( + manager, + "_save_representation_internal", + new=AsyncMock( + return_value=CreateDocumentsResult(created_documents=[MagicMock()]) + ), + ), + ): + await manager.save_representation( + representation, + message_ids=[1], + session_name="session", + message_created_at=datetime.now(timezone.utc), + message_level_configuration=_resolved_config(), + ) + + mock_embed.assert_awaited_once_with( + ["inferred fact", "short fact"], on_oversize="truncate" + ) + class TestVectorQueryTopKFloor: """Regression for HONCHO-19Q / HONCHO-4Q4. diff --git a/tests/deriver/test_deriver_processing.py b/tests/deriver/test_deriver_processing.py index 6785d159..29a983b5 100644 --- a/tests/deriver/test_deriver_processing.py +++ b/tests/deriver/test_deriver_processing.py @@ -7,7 +7,9 @@ import pytest from src import crud, models from src.config import settings +from src.crud.representation import RepresentationManager from src.deriver.deriver import process_representation_tasks_batch +from src.exceptions import RepresentationSaveError from src.llm import HonchoLLMCallResponse from src.utils.representation import ( ExplicitObservationBase, @@ -70,6 +72,116 @@ class TestDeriverProcessing: assert kwargs["model_config"].stop_sequences == expected_config.stop_sequences assert "llm_settings" not in kwargs + async def test_all_observer_saves_failing_surfaces_failure(self): + """When every observer's save_representation fails, the batch must raise.""" + message = Mock( + id=1, + public_id="msg_1", + session_name="session-1", + workspace_name="workspace-1", + peer_name="alice", + content="hello", + token_count=5, + created_at=datetime.now(timezone.utc), + ) + configuration = Mock() + configuration.reasoning.enabled = True + + mock_response = HonchoLLMCallResponse( + content=PromptRepresentation( + explicit=[ + ExplicitObservationBase(content="The user has a dog named Rover") + ] + ), + input_tokens=10, + output_tokens=5, + finish_reasons=["STOP"], + ) + + failing_save = AsyncMock(side_effect=RuntimeError("429 RESOURCE_EXHAUSTED")) + emitted: list[Any] = [] + with ( + patch( + "src.deriver.deriver.honcho_llm_call", + new_callable=AsyncMock, + return_value=mock_response, + ), + patch.object(RepresentationManager, "save_representation", failing_save), + patch("src.deriver.deriver.emit", side_effect=emitted.append), + pytest.raises(RepresentationSaveError, match="save_representation failed"), + ): + await process_representation_tasks_batch( + messages=[message], + message_level_configuration=configuration, + observers=["bob"], + observed="alice", + queue_item_message_ids=[1], + ) + + # Telemetry must fire *before* the raise so a total save failure is still + # visible to metrics. Guards against emit() being moved after the raise. + assert emitted, "expected telemetry to be emitted before the raised failure" + assert emitted[-1].observer_count == 0 + assert emitted[-1].failed_observer_count == 1 + + async def test_partial_observer_failure_is_processed_and_surfaced(self): + """When some observers save and one fails, the batch does NOT raise + (saved observers are kept) and the failure is visible via telemetry. + """ + message = Mock( + id=1, + public_id="msg_1", + session_name="session-1", + workspace_name="workspace-1", + peer_name="alice", + content="hello", + token_count=5, + created_at=datetime.now(timezone.utc), + ) + configuration = Mock() + configuration.reasoning.enabled = True + + mock_response = HonchoLLMCallResponse( + content=PromptRepresentation( + explicit=[ + ExplicitObservationBase(content="The user has a dog named Rover") + ] + ), + input_tokens=10, + output_tokens=5, + finish_reasons=["STOP"], + ) + + # bob succeeds, carol fails. + partial_save = AsyncMock( + side_effect=[ + crud.CreateDocumentsResult(), + RuntimeError("429 RESOURCE_EXHAUSTED"), + ] + ) + emitted: list[Any] = [] + with ( + patch( + "src.deriver.deriver.honcho_llm_call", + new_callable=AsyncMock, + return_value=mock_response, + ), + patch.object(RepresentationManager, "save_representation", partial_save), + patch("src.deriver.deriver.emit", side_effect=emitted.append), + ): + await process_representation_tasks_batch( + messages=[message], + message_level_configuration=configuration, + observers=["bob", "carol"], + observed="alice", + queue_item_message_ids=[1], + ) + + assert emitted, "expected a telemetry event to be emitted" + event = emitted[-1] + assert event.observer_count == 1 + assert event.failed_observer_count == 1 + async def test_process_representation_tasks_batch_passes_custom_instructions_into_prompt( self, ) -> None: diff --git a/tests/deriver/test_vector_reconciliation.py b/tests/deriver/test_vector_reconciliation.py index 5e2e4712..0ecd4d5c 100644 --- a/tests/deriver/test_vector_reconciliation.py +++ b/tests/deriver/test_vector_reconciliation.py @@ -551,9 +551,12 @@ class TestReEmbedding: # Mock embedding client to track batch calls batch_call_count = 0 - async def track_batch_embed(contents: list[str]) -> list[list[float]]: + async def track_batch_embed( + contents: list[str], *, on_oversize: str, **_kwargs: object + ) -> list[list[float]]: nonlocal batch_call_count batch_call_count += 1 + assert on_oversize == "truncate" return [[1.0] * 1536 for _ in contents] with patch("src.reconciler.sync_vectors.embedding_client") as mock_embed_client: diff --git a/tests/live_llm/README.md b/tests/live_llm/README.md index 6da7dfb4..5cf39e4b 100644 --- a/tests/live_llm/README.md +++ b/tests/live_llm/README.md @@ -68,5 +68,5 @@ Coverage by provider: - OpenAI transport → OpenRouter non-reasoning models (e.g. `inception/mercury-2`): non-chat / diffusion architectures must stay on `max_tokens`, no `reasoning_effort`, tool-calling parameter-schema compatibility is the canary for exotic OR-served providers - Gemini 2.5/3.0 classes: structured outputs, cached-content reuse, thought signatures, multi-turn tool replay - Gemini 3.1 class: thinking and tool replay coverage by default; structured-output/caching coverage should only be added once Google documents support for that path -- Embeddings (`test_live_embeddings.py`): single embed, batched embed, batch-vs-single alignment, and chunk-to-id mapping for every configured embedding model. `gemini-embedding-2*` is the reason this exists — those models collapse a list of bare strings into one document (#745), and only a live call catches it +- Embeddings (`test_live_embeddings.py`): single embed, batched embed, batch-vs-single alignment, chunk-to-id mapping, and oversize-truncate survival (`on_oversize="truncate"`) for every configured embedding model. `gemini-embedding-2*` is the reason this exists — those models collapse a list of bare strings into one document (#745), and only a live call catches it. Also covers first-class `EmbeddingModelConfig.timeout` plumbing (one representative model per transport): configured timeout lands on the SDK client, and a near-zero timeout aborts before the provider answers - OpenAI-compatible embedding providers (e.g. OpenRouter's `google/gemini-embedding-001`): the #932 surface. Those providers reject a base64 embedding request outright (HTTP 400) or answer HTTP 200 with empty data, so the whole matrix fails without `encoding_format="float"`. Real OpenAI accepts base64 happily, so only a third-party provider catches it. Note that OpenRouter load-balances across upstreams, so the base64 failure is per-attempt rather than guaranteed: a retry can land on an endpoint that accepts it. `test_live_openai_float_encoding_matches_base64` covers the other side, that the float switch must not move vectors on real OpenAI diff --git a/tests/live_llm/conftest.py b/tests/live_llm/conftest.py index eb7b8dbb..9c58bb87 100644 --- a/tests/live_llm/conftest.py +++ b/tests/live_llm/conftest.py @@ -66,14 +66,23 @@ def require_embedding_key(spec: LiveEmbeddingSpec) -> str: return key +_EMBEDDING_CONFIG_OVERRIDE_KEYS = frozenset({"timeout", "max_batch_size"}) + + def make_embedding_client( spec: LiveEmbeddingSpec, **overrides: Any ) -> _EmbeddingClient: """Build a live embedding client for one matrix entry. Bypasses the `EmbeddingClient` singleton so each spec gets its own client - without mutating global settings. + without mutating global settings. `timeout` and `max_batch_size` land on + `EmbeddingModelConfig`; remaining kwargs go to `_EmbeddingClient`. """ + config_overrides = { + key: overrides.pop(key) + for key in _EMBEDDING_CONFIG_OVERRIDE_KEYS + if key in overrides + } kwargs: dict[str, Any] = { "vector_dimensions": spec.dimensions, "max_input_tokens": 2048, @@ -90,6 +99,7 @@ def make_embedding_client( model=spec.model, api_key=require_embedding_key(spec), base_url=spec.base_url, + **config_overrides, ), **kwargs, ) diff --git a/tests/live_llm/test_live_embeddings.py b/tests/live_llm/test_live_embeddings.py index c8af34b8..7c57a2e3 100644 --- a/tests/live_llm/test_live_embeddings.py +++ b/tests/live_llm/test_live_embeddings.py @@ -1,10 +1,15 @@ from __future__ import annotations +import time from typing import Any, cast +import httpx +import openai import pytest from openai import AsyncOpenAI +from src.config import EmbeddingTransport + from .conftest import cosine_similarity, make_embedding_client from .embedding_matrix import LiveEmbeddingSpec, get_live_embedding_specs @@ -25,6 +30,52 @@ OPENAI_NATIVE_SPECS = tuple( spec for spec in ALL_SPECS if spec.family == "openai_embedding" ) +GENEROUS_TIMEOUT_SECONDS = 120.0 +TIGHT_TIMEOUT_SECONDS = 0.01 +# Well under the client defaults; generous enough to absorb SDK retries. +TIGHT_TIMEOUT_WALL_CLOCK_LIMIT_SECONDS = 30 + +EMBEDDING_TIMEOUT_EXCEPTIONS: dict[ + EmbeddingTransport, tuple[type[BaseException], ...] +] = { + "openai": (openai.APITimeoutError,), + # google-genai raises httpx or aiohttp timeouts depending on its transport; + # aiohttp surfaces as asyncio.TimeoutError (== builtins.TimeoutError). + "gemini": (httpx.TimeoutException, TimeoutError), +} + +TRANSPORT_MARKS = { + "openai": pytest.mark.requires_openai, + "gemini": pytest.mark.requires_gemini, +} + + +def representative_embedding_specs() -> list[Any]: + """One spec per transport — timeout plumbing is client-level, not model-level.""" + params: list[Any] = [] + for transport in ("openai", "gemini"): + specs = get_live_embedding_specs(transport=transport) + if not specs: + continue + # Prefer the native family over openai-compatible proxies. + family = f"{transport}_embedding" + native = next((s for s in specs if s.family == family), specs[0]) + params.append( + pytest.param(native, marks=TRANSPORT_MARKS[transport], id=native.id) + ) + return params + + +def assert_embedding_timeout_on_client( + client: Any, transport: EmbeddingTransport, timeout_seconds: float +) -> None: + if transport == "gemini": + http_options = client.client._api_client._http_options + assert http_options.timeout == int(timeout_seconds * 1000) + return + openai_client = cast(AsyncOpenAI, client.client) + assert openai_client.timeout == timeout_seconds + @pytest.mark.asyncio @pytest.mark.parametrize("spec", ALL_SPECS, ids=lambda spec: spec.id) @@ -146,6 +197,26 @@ async def test_live_openai_float_encoding_matches_base64( ), f"{spec.id}: float encoding diverges from base64 (cosine={similarity:.8f})" +@pytest.mark.asyncio +@pytest.mark.parametrize("spec", ALL_SPECS, ids=lambda spec: spec.id) +async def test_live_batch_embed_truncates_oversize_instead_of_dropping_batch( + spec: LiveEmbeddingSpec, +) -> None: + """on_oversize='truncate' keeps one vector per input when an item exceeds the cap.""" + # Tiny cap so the oversize input stays cheap to tokenize and send. + client = make_embedding_client(spec, max_input_tokens=32) + oversize = " ".join(f"oversize-token-{index}" for index in range(200)) + assert len(client.encoding.encode(oversize)) > client.max_embedding_tokens + + texts = [BATCH_TEXTS[0], oversize, BATCH_TEXTS[1]] + embeddings = await client.simple_batch_embed(texts, on_oversize="truncate") + + assert len(embeddings) == len(texts) + assert all(len(embedding) == spec.dimensions for embedding in embeddings) + # A collapsed or dropped batch would reuse a vector or return fewer. + assert len({tuple(embedding) for embedding in embeddings}) == len(texts) + + @pytest.mark.asyncio @pytest.mark.parametrize("spec", GEMINI_SPECS, ids=lambda spec: spec.id) async def test_live_gemini_batch_embed_survives_batch_split( @@ -160,3 +231,36 @@ async def test_live_gemini_batch_embed_survives_batch_split( assert len(embeddings) == len(BATCH_TEXTS) assert len({tuple(embedding) for embedding in embeddings}) == len(BATCH_TEXTS) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("spec", representative_embedding_specs()) +async def test_live_embedding_timeout_reaches_the_client( + spec: LiveEmbeddingSpec, +) -> None: + """Configured embedding timeout lands on the provider SDK client.""" + client = make_embedding_client(spec, timeout=GENEROUS_TIMEOUT_SECONDS) + + embedding = await client.embed(BATCH_TEXTS[0]) + + assert len(embedding) == spec.dimensions + assert_embedding_timeout_on_client(client, spec.transport, GENEROUS_TIMEOUT_SECONDS) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("spec", representative_embedding_specs()) +async def test_live_tight_embedding_timeout_aborts_request( + spec: LiveEmbeddingSpec, +) -> None: + """A near-zero embedding timeout aborts before the provider can answer.""" + client = make_embedding_client(spec, timeout=TIGHT_TIMEOUT_SECONDS) + + started = time.monotonic() + with pytest.raises(EMBEDDING_TIMEOUT_EXCEPTIONS[spec.transport]): + await client.embed(BATCH_TEXTS[0]) + elapsed = time.monotonic() - started + + assert elapsed < TIGHT_TIMEOUT_WALL_CLOCK_LIMIT_SECONDS, ( + f"tight embedding timeout took {elapsed:.1f}s — client timeout " + f"likely not applied" + ) diff --git a/tests/llm/test_embedding_client.py b/tests/llm/test_embedding_client.py index b50a66f0..fc2ff411 100644 --- a/tests/llm/test_embedding_client.py +++ b/tests/llm/test_embedding_client.py @@ -13,6 +13,7 @@ from src.config import ( ) from src.embedding_client import ( BatchItem, + EmbeddingClient, _EmbeddingClient, # pyright: ignore[reportPrivateUsage] ) @@ -65,7 +66,13 @@ async def test_openai_embedding_client_uses_configured_model_and_dimensions( fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 8) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.api_key: str | None = api_key self.base_url: str | None = base_url self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings @@ -104,7 +111,13 @@ async def test_openai_embedding_client_rejects_dimension_mismatch( fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 7) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) @@ -220,6 +233,118 @@ async def test_gemini_embedding_client_keeps_timeout_without_base_url( assert gemini_client.http_options.timeout == 600_000 +@pytest.mark.asyncio +async def test_openai_embedding_client_forwards_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Configured embedding timeout reaches the OpenAI-compatible client.""" + + class FakeOpenAIClient: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: + self.api_key: str | None = api_key + self.base_url: str | None = base_url + self.timeout: float | None = timeout + self.embeddings: FakeOpenAIEmbeddingsAPI = FakeOpenAIEmbeddingsAPI( + [0.1] * 8 + ) + + monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) + + client = _EmbeddingClient( + EmbeddingModelConfig( + transport="openai", + model="text-embedding-3-small", + api_key="test-key", + timeout=45, + ), + vector_dimensions=8, + max_input_tokens=8192, + max_tokens_per_request=300_000, + send_dimensions=False, + ) + + openai_client = cast(Any, client.client) + assert openai_client.timeout == 45.0 + + +@pytest.mark.asyncio +async def test_openai_embedding_client_omits_timeout_when_unset( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unset timeout omits the kwarg so the OpenAI SDK keeps its default.""" + + missing = object() + + class FakeOpenAIClient: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: object = missing, + ) -> None: + self.api_key: str | None = api_key + self.base_url: str | None = base_url + self.timeout: object = timeout + self.embeddings: FakeOpenAIEmbeddingsAPI = FakeOpenAIEmbeddingsAPI( + [0.1] * 8 + ) + + monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) + + client = _EmbeddingClient( + EmbeddingModelConfig( + transport="openai", + model="text-embedding-3-small", + api_key="test-key", + ), + vector_dimensions=8, + max_input_tokens=8192, + max_tokens_per_request=300_000, + send_dimensions=False, + ) + + openai_client = cast(Any, client.client) + assert openai_client.timeout is missing + + +@pytest.mark.asyncio +async def test_gemini_embedding_client_forwards_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Configured embedding timeout reaches Gemini as milliseconds.""" + + class FakeGeminiClient: + def __init__(self, *, api_key: str | None, http_options: Any) -> None: + self.api_key: str | None = api_key + self.http_options: Any = http_options + self.aio: Any = SimpleNamespace(models=SimpleNamespace()) + + monkeypatch.setattr("google.genai.Client", FakeGeminiClient) + + client = _EmbeddingClient( + EmbeddingModelConfig( + transport="gemini", + model="gemini-embedding-001", + api_key="gemini-key", + timeout=45, + ), + vector_dimensions=8, + max_input_tokens=4096, + max_tokens_per_request=300_000, + send_dimensions=False, + ) + + gemini_client = cast(Any, client.client) + assert gemini_client.http_options.timeout == 45_000 + + def _build_openai_client( monkeypatch: pytest.MonkeyPatch, *, @@ -233,7 +358,13 @@ def _build_openai_client( fake_embeddings = FakeOpenAIEmbeddingsAPI(embedding) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.api_key: str | None = api_key self.base_url: str | None = base_url self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings @@ -707,7 +838,13 @@ async def test_simple_batch_embed_respects_token_budget_per_request( fake_embeddings = FakeOpenAIEmbeddingsAPI([0.5] * 4) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) @@ -745,7 +882,13 @@ async def test_simple_batch_embed_rejects_oversized_input( fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 4) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) @@ -768,6 +911,131 @@ async def test_simple_batch_embed_rejects_oversized_input( await client.simple_batch_embed([too_long]) +@pytest.mark.asyncio +async def test_simple_batch_embed_truncates_oversize_when_requested( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """on_oversize='truncate' embeds a prefix instead of failing the batch.""" + fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 4) + + class FakeOpenAIClient: + def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings + + monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) + + client = _EmbeddingClient( + EmbeddingModelConfig( + transport="openai", + model="text-embedding-3-small", + api_key="test-key", + base_url=None, + ), + vector_dimensions=4, + max_input_tokens=10, + max_tokens_per_request=1000, + send_dimensions=False, + ) + + short = "hello" + too_long = ("word " * 50).strip() + assert len(client.encoding.encode(too_long)) > client.max_embedding_tokens + + out = await client.simple_batch_embed([short, too_long], on_oversize="truncate") + + assert len(out) == 2 + assert fake_embeddings.calls, "expected a provider call after truncation" + received = fake_embeddings.calls[0]["input"] + assert received[0] == short + truncated = received[1] + assert isinstance(truncated, str) + assert truncated != too_long + assert len(client.encoding.encode(truncated)) <= client.max_embedding_tokens + + +@pytest.mark.asyncio +async def test_simple_batch_embed_truncate_reencodes_until_under_cap( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """decode(ids[:n]) can re-encode past n; truncate must re-verify the count.""" + fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 4) + + class FakeOpenAIClient: + def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings + + monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) + + client = _EmbeddingClient( + EmbeddingModelConfig( + transport="openai", + model="text-embedding-3-small", + api_key="test-key", + base_url=None, + ), + vector_dimensions=4, + max_input_tokens=10, + max_tokens_per_request=1000, + send_dimensions=False, + ) + + encode_calls = {"n": 0} + + def encode(text: str) -> list[int]: + encode_calls["n"] += 1 + if text.startswith("LONG"): + # 1: original oversize; 2: still over after first slice; 3+: fits. + if encode_calls["n"] == 1: + return list(range(20)) + if encode_calls["n"] == 2: + return list(range(12)) + return list(range(8)) + return [1] + + def decode(ids: list[int]) -> str: + return "LONG" + "x" * len(ids) + + monkeypatch.setattr(client.encoding, "encode", encode) + monkeypatch.setattr(client.encoding, "decode", decode) + + out = await client.simple_batch_embed(["LONG-input"], on_oversize="truncate") + + assert len(out) == 1 + received = fake_embeddings.calls[0]["input"][0] + assert isinstance(received, str) + # The provider must see the post-loop text, which encodes to 8 (<= cap). + assert encode(received) == list(range(8)) + assert encode_calls["n"] >= 3 + + +@pytest.mark.asyncio +async def test_public_embedding_client_forwards_on_oversize( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The singleton wrapper must forward on_oversize to the inner client.""" + captured: dict[str, object] = {} + + class FakeInner: + async def simple_batch_embed( + self, + texts: list[str], + *, + on_oversize: str = "raise", + ) -> list[list[float]]: + captured["texts"] = texts + captured["on_oversize"] = on_oversize + return [[0.1]] + + wrapper = EmbeddingClient() + monkeypatch.setattr(wrapper, "_get_client", lambda: FakeInner()) + + out = await wrapper.simple_batch_embed(["hi"], on_oversize="truncate") + + assert out == [[0.1]] + assert captured["texts"] == ["hi"] + assert captured["on_oversize"] == "truncate" + + def test_prepare_chunks_returns_ordered_chunks( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -775,7 +1043,13 @@ def test_prepare_chunks_returns_ordered_chunks( fake_embeddings = FakeOpenAIEmbeddingsAPI([0.1] * 4) class FakeOpenAIClient: - def __init__(self, *, api_key: str | None, base_url: str | None) -> None: + def __init__( + self, + *, + api_key: str | None, + base_url: str | None, + timeout: float | None = None, + ) -> None: self.embeddings: FakeOpenAIEmbeddingsAPI = fake_embeddings monkeypatch.setattr("openai.AsyncOpenAI", FakeOpenAIClient) @@ -818,6 +1092,31 @@ def test_embedding_model_config_parses_max_batch_size_from_env( assert resolved.max_batch_size == 10 +def test_embedding_model_config_parses_timeout_from_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + s = _build_embedding_settings( + {"EMBEDDING_MODEL_CONFIG__TIMEOUT": "90.0"}, + monkeypatch, + ) + + assert s.MODEL_CONFIG.timeout == 90.0 + + resolved = resolve_embedding_model_config(s.MODEL_CONFIG) + assert resolved.timeout == 90.0 + + +def test_embedding_model_config_rejects_invalid_timeout() -> None: + with pytest.raises( + ValueError, match=r"provider_params\.timeout must be a positive number" + ): + EmbeddingModelConfig( + transport="openai", + model="text-embedding-3-small", + timeout=-1, + ) + + @pytest.mark.asyncio async def test_gemini_process_batch_wraps_contents_as_content_part( monkeypatch: pytest.MonkeyPatch, diff --git a/tests/llm/test_history_adapters.py b/tests/llm/test_history_adapters.py index 6881df6a..a992398d 100644 --- a/tests/llm/test_history_adapters.py +++ b/tests/llm/test_history_adapters.py @@ -1,3 +1,5 @@ +import pytest + from src.llm.backend import CompletionResult, ToolCallResult from src.llm.history_adapters import ( AnthropicHistoryAdapter, @@ -65,3 +67,49 @@ def test_openai_history_adapter_preserves_reasoning_details() -> None: assert message["role"] == "assistant" assert message["reasoning_details"] == [{"type": "reasoning", "content": "step 1"}] assert message["tool_calls"][0]["function"]["name"] == "search" + + +def test_openai_history_adapter_preserves_thinking_content() -> None: + adapter = OpenAIHistoryAdapter() + result = CompletionResult( + content="Calling a tool", + thinking_content="step 1", + tool_calls=[ + ToolCallResult(id="tool_1", name="search", input={"query": "honcho"}) + ], + ) + + message = adapter.format_assistant_tool_message(result) + + assert message["reasoning_content"] == "step 1" + assert "reasoning_details" not in message + + +def test_openai_history_adapter_prefers_reasoning_details() -> None: + adapter = OpenAIHistoryAdapter() + reasoning_details = [{"type": "reasoning", "content": "step 1"}] + result = CompletionResult( + content="Calling a tool", + thinking_content="duplicate step 1", + reasoning_details=reasoning_details, + ) + + message = adapter.format_assistant_tool_message(result) + + assert message["reasoning_details"] == reasoning_details + assert "reasoning_content" not in message + + +@pytest.mark.parametrize("thinking_content", [None, ""]) +def test_openai_history_adapter_omits_empty_thinking_content( + thinking_content: str | None, +) -> None: + adapter = OpenAIHistoryAdapter() + result = CompletionResult( + content="Calling a tool", + thinking_content=thinking_content, + ) + + message = adapter.format_assistant_tool_message(result) + + assert "reasoning_content" not in message diff --git a/tests/llm/test_tool_loop_reasoning_content.py b/tests/llm/test_tool_loop_reasoning_content.py new file mode 100644 index 00000000..cd4ec91b --- /dev/null +++ b/tests/llm/test_tool_loop_reasoning_content.py @@ -0,0 +1,104 @@ +from __future__ import annotations + +from copy import deepcopy +from typing import Any, cast +from unittest.mock import patch + +import pytest + +from src.config import ModelConfig +from src.llm import tool_loop +from src.llm.runtime import AttemptPlan +from src.llm.tool_loop import execute_tool_loop +from src.llm.types import HonchoLLMCallResponse, ProviderClient + + +def _make_plan() -> AttemptPlan: + return AttemptPlan( + provider="openai", + model="deepseek-v4-pro", + client=cast(ProviderClient, object()), + thinking_budget_tokens=None, + reasoning_effort=None, + selected_config=ModelConfig( + model="deepseek-v4-pro", + transport="openai", + ), + attempt=1, + retry_attempts=1, + is_fallback=False, + ) + + +@pytest.mark.asyncio +async def test_tool_loop_replays_reasoning_content_on_continuation() -> None: + calls: list[list[dict[str, Any]]] = [] + responses = iter( + [ + HonchoLLMCallResponse( + content="", + output_tokens=5, + finish_reasons=["tool_calls"], + tool_calls_made=[ + { + "id": "call_1", + "name": "search", + "input": {"query": "honcho"}, + } + ], + thinking_content="DeepSeek reasoning", + ), + HonchoLLMCallResponse( + content="done", + output_tokens=3, + finish_reasons=["stop"], + tool_calls_made=[], + ), + ] + ) + + async def fake_call(*_args: Any, **kwargs: Any) -> HonchoLLMCallResponse[Any]: + calls.append(deepcopy(kwargs["messages"])) + return next(responses) + + async def execute_search(_name: str, _input: dict[str, Any]) -> str: + return "result" + + with patch.object(tool_loop, "honcho_llm_call_inner", new=fake_call): + result = await execute_tool_loop( + prompt="hi", + max_tokens=64, + messages=[{"role": "user", "content": "hi"}], + tools=[ + { + "name": "search", + "description": "Search", + "input_schema": {"type": "object"}, + } + ], + tool_choice="auto", + tool_executor=execute_search, + max_tool_iterations=5, + response_model=None, + json_mode=False, + temperature=None, + stop_seqs=None, + verbosity=None, + enable_retry=False, + retry_attempts=1, + max_input_tokens=None, + get_attempt_plan=_make_plan, + before_retry_callback=lambda _retry_state: None, + stream_final=False, + telemetry=None, + ) + + assert isinstance(result, HonchoLLMCallResponse) + assert len(calls) == 2 + assert calls[1][1]["reasoning_content"] == "DeepSeek reasoning" + assert calls[1][1]["tool_calls"][0]["function"]["name"] == "search" + assert calls[1][2] == { + "role": "tool", + "tool_call_id": "call_1", + "content": "result", + } diff --git a/tests/reconciler/test_pending_backlog_gauge.py b/tests/reconciler/test_pending_backlog_gauge.py new file mode 100644 index 00000000..226a71e8 --- /dev/null +++ b/tests/reconciler/test_pending_backlog_gauge.py @@ -0,0 +1,86 @@ +"""The pending-embeddings backlog gauge must be refreshed per-replica. + +These tests pin both halves: the scheduler loop drives the refresh, and the +queue-driven reconciliation cycle does not. +""" +# region ai +# ``message_embeddings_pending`` reports a DB-global count, so it is the one gauge +# here whose value is service-wide rather than per-process. It is also zero- +# initialized at startup, which makes a missing refresh actively harmful: a replica +# that never measured the backlog would export a confident, permanently-healthy 0. +# So the count is driven from ``ReconcilerScheduler._scheduler_loop`` (runs on every +# replica, every interval), NOT from ``run_vector_reconciliation_cycle`` (runs off +# the queue behind work-unit dedup, so exactly one replica per cycle executes it). +# endregion + +import asyncio + +import pytest + +from src.reconciler import scheduler as scheduler_module +from src.reconciler import sync_vectors +from src.reconciler.scheduler import ReconcilerScheduler + + +@pytest.fixture(autouse=True) +def _reset_scheduler_singleton(): # pyright: ignore[reportUnusedFunction] + ReconcilerScheduler.reset_singleton() + yield + ReconcilerScheduler.reset_singleton() + + +async def test_scheduler_loop_refreshes_backlog_gauge( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Every scheduler iteration refreshes the gauge, on every replica. + + Patched at the scheduler's own reference so this asserts the call site, not + just that the function exists. + """ + calls = 0 + refreshed = asyncio.Event() + + async def _fake_refresh() -> None: + nonlocal calls + calls += 1 + refreshed.set() + + # region ai + # Patched onto the class, so it is invoked as a bound method — it needs the + # ``self`` parameter or the call raises TypeError, which ``_scheduler_loop`` would + # then swallow, leaving this guard silently inert. + # endregion + async def _never_enqueue(_self: object, _task: object) -> bool: + return False + + monkeypatch.setattr( + scheduler_module, "record_pending_embeddings_backlog", _fake_refresh + ) + monkeypatch.setattr( + ReconcilerScheduler, "_try_enqueue_task", _never_enqueue, raising=True + ) + + scheduler = ReconcilerScheduler() + await scheduler.start() + try: + await asyncio.wait_for(refreshed.wait(), timeout=5.0) + finally: + await scheduler.shutdown() + + assert calls >= 1, "scheduler loop never refreshed the backlog gauge" + + +def test_reconciliation_cycle_does_not_drive_the_gauge() -> None: + """The queue-driven cycle must not be the thing that sets the gauge.""" + # region ai + # If the refresh moves back into ``run_vector_reconciliation_cycle``, only the + # replica that wins the ``sync_vectors`` work unit would ever measure the backlog, + # and the zero-init would go back to lying on all the others. + # + # Structural guard: the cycle is a long DB-driven coroutine, so this inspects the + # global names it references rather than executing it. + # endregion + assert hasattr(sync_vectors, "record_pending_embeddings_backlog") + + referenced = sync_vectors.run_vector_reconciliation_cycle.__code__.co_names + assert "record_pending_embeddings_backlog" not in referenced diff --git a/tests/sdk/test_conclusions.py b/tests/sdk/test_conclusions.py index 9c2cdbc0..314717dc 100644 --- a/tests/sdk/test_conclusions.py +++ b/tests/sdk/test_conclusions.py @@ -6,7 +6,7 @@ from sdks.python.src.honcho.client import Honcho from sdks.python.src.honcho.conclusions import ( Conclusion, ConclusionCreateParams, - ConclusionScope, + ConclusionsView, ) @@ -34,7 +34,7 @@ async def test_observation_create_single( # Get observation scope for observer -> target obs_scope = observer.conclusions_of(target) - assert isinstance(obs_scope, ConclusionScope) + assert isinstance(obs_scope, ConclusionsView) # Create a single observation created = await obs_scope.aio.create( @@ -68,7 +68,7 @@ async def test_observation_create_single( # Get observation scope for observer -> target obs_scope = observer.conclusions_of(target) - assert isinstance(obs_scope, ConclusionScope) + assert isinstance(obs_scope, ConclusionsView) # Create a single observation created = obs_scope.create( @@ -422,7 +422,7 @@ async def test_self_observation_create( # Get self-observation scope obs_scope = peer.conclusions - assert isinstance(obs_scope, ConclusionScope) + assert isinstance(obs_scope, ConclusionsView) assert obs_scope.observer == peer.id assert obs_scope.observed == peer.id @@ -443,7 +443,7 @@ async def test_self_observation_create( # Get self-observation scope obs_scope = peer.conclusions - assert isinstance(obs_scope, ConclusionScope) + assert isinstance(obs_scope, ConclusionsView) assert obs_scope.observer == peer.id assert obs_scope.observed == peer.id @@ -796,7 +796,7 @@ async def test_list_rejects_reserved_scope_filter_keys( target = await honcho_client.aio.peer(id="test-obs-reserved-list-target") obs_scope = observer.conclusions_of(target) for key in reserved: - with pytest.raises(ValueError, match="managed by this conclusion scope"): + with pytest.raises(ValueError, match="managed by this conclusions view"): await obs_scope.aio.list(filters={key: "someone-else"}) # A non-reserved filter (level) is allowed through. await obs_scope.aio.list(filters={"level": "explicit"}) @@ -805,7 +805,7 @@ async def test_list_rejects_reserved_scope_filter_keys( target = honcho_client.peer(id="test-obs-reserved-list-target") obs_scope = observer.conclusions_of(target) for key in reserved: - with pytest.raises(ValueError, match="managed by this conclusion scope"): + with pytest.raises(ValueError, match="managed by this conclusions view"): obs_scope.list(filters={key: "someone-else"}) obs_scope.list(filters={"level": "explicit"}) @@ -827,7 +827,7 @@ async def test_query_rejects_reserved_scope_filter_keys( target = await honcho_client.aio.peer(id="test-obs-reserved-query-target") obs_scope = observer.conclusions_of(target) for key in reserved: - with pytest.raises(ValueError, match="managed by this conclusion scope"): + with pytest.raises(ValueError, match="managed by this conclusions view"): await obs_scope.aio.query("q", filters={key: "someone-else"}) # session_id is a normal filter for query (no dedicated param) — allowed. await obs_scope.aio.query("q", filters={"session_id": "some-session"}) @@ -836,6 +836,6 @@ async def test_query_rejects_reserved_scope_filter_keys( target = honcho_client.peer(id="test-obs-reserved-query-target") obs_scope = observer.conclusions_of(target) for key in reserved: - with pytest.raises(ValueError, match="managed by this conclusion scope"): + with pytest.raises(ValueError, match="managed by this conclusions view"): obs_scope.query("q", filters={key: "someone-else"}) obs_scope.query("q", filters={"session_id": "some-session"}) diff --git a/tests/sdk/test_scope_options.py b/tests/sdk/test_scope_options.py new file mode 100644 index 00000000..fddfe51d --- /dev/null +++ b/tests/sdk/test_scope_options.py @@ -0,0 +1,236 @@ +"""Unit tests for the SDK's scope / session-allowlist option handling. + +Pure logic — no server, no database. These pin the wire translation the server +expects, so a rename or a shape change fails here rather than as a 422 at runtime. +""" + +import sys +from pathlib import Path + +import pytest + +# Add the SDK src to the path to allow imports +sdk_src_path = Path(__file__).parent.parent.parent / "sdks" / "python" / "src" +sys.path.insert(0, str(sdk_src_path)) + +from sdks.python.src.honcho.utils.scopes import ( # noqa: E402 + MAX_SCOPES_PER_OPTION, + MAX_SESSION_ALLOWLIST_ENTRIES, + MAX_SESSIONS_PER_ADD, + resolve_scope_membership, + resolve_scope_option, + resolve_scope_session, + scope_context_fields, + scope_recall_fields, + validate_scope_id, +) + + +def context_fields(**overrides: object) -> dict[str, object]: + """Call scope_context_fields with the neutral defaults filled in.""" + kwargs: dict[str, object] = { + "scope": None, + "sessions": None, + "peer_target": "user", + "peer_perspective": None, + "limit_to_session": False, + } + kwargs.update(overrides) + return scope_context_fields(**kwargs) # pyright: ignore[reportArgumentType] + + +class TestValidateScopeId: + def test_accepts_a_plain_name(self): + assert validate_scope_id("therapy") == "therapy" + + def test_rejects_the_reserved_prefix_by_name(self): + # 'scope.therapy' violates both the prefix rule and the charset. The + # prefix message is the actionable one, so it must be the one raised. + with pytest.raises(ValueError, match="reserved prefix"): + validate_scope_id("scope.therapy") + + def test_rejects_characters_outside_the_charset(self): + with pytest.raises(ValueError, match="must match pattern"): + validate_scope_id("my scope") + + def test_rejects_empty(self): + with pytest.raises(ValueError, match="between 1 and"): + validate_scope_id("") + + def test_rejects_a_name_that_leaves_no_room_for_the_prefix(self): + # 512 - len("scope.") is the ceiling: the server stores the name prefixed + # into a 512-character peer name. + with pytest.raises(ValueError, match="between 1 and"): + validate_scope_id("a" * 507) + + +class TestResolveScopeOption: + def test_a_single_scope_stays_a_string(self): + # The shapes are not interchangeable to the server: one scope reads that + # scope's own view, a list restricts to the union of member sessions. + assert resolve_scope_option("therapy") == "therapy" + + def test_a_sequence_becomes_a_list(self): + assert resolve_scope_option(["therapy", "work"]) == ["therapy", "work"] + + def test_rejects_an_empty_sequence(self): + with pytest.raises(ValueError, match="at least one scope"): + resolve_scope_option([]) + + def test_rejects_an_over_cap_sequence(self): + with pytest.raises(ValueError, match="at most"): + resolve_scope_option([f"s{i}" for i in range(MAX_SCOPES_PER_OPTION + 1)]) + + +class TestScopeRecallFields: + def test_neither_option_contributes_nothing(self): + assert scope_recall_fields(scope=None, sessions=None) == {} + + def test_sessions_becomes_a_session_id_filter(self): + # `sessions` is sugar. It must never reach the wire as its own key — + # the server rejects unknown keys with a 422. + fields = scope_recall_fields(scope=None, sessions=["a", "b"]) + assert fields == {"filters": {"session_id": ["a", "b"]}} + assert "sessions" not in fields + + def test_scope_passes_through_under_its_own_key(self): + assert scope_recall_fields(scope="therapy", sessions=None) == { + "scope": "therapy" + } + + def test_scope_and_sessions_are_mutually_exclusive(self): + with pytest.raises(ValueError, match="mutually exclusive"): + scope_recall_fields(scope="therapy", sessions=["a"]) + + def test_scope_and_a_single_session_are_mutually_exclusive(self): + with pytest.raises(ValueError, match="mutually exclusive"): + scope_recall_fields(scope="therapy", sessions=None, session_id="a") + + def test_sessions_composes_with_a_single_session(self): + # Unlike `scope`, an allowlist may accompany a session_id — the server + # only requires that the session be inside the allowlist. + assert scope_recall_fields(scope=None, sessions=["a", "b"], session_id="a") == { + "filters": {"session_id": ["a", "b"]} + } + + def test_rejects_an_empty_allowlist(self): + # An empty allowlist is fail-closed server-side (recalls nothing), which + # is never what `sessions=[]` intends. + with pytest.raises(ValueError, match="at least one session"): + scope_recall_fields(scope=None, sessions=[]) + + def test_rejects_an_over_cap_allowlist(self): + with pytest.raises(ValueError, match="at most"): + scope_recall_fields( + scope=None, + sessions=[f"s{i}" for i in range(MAX_SESSION_ALLOWLIST_ENTRIES + 1)], + ) + + def test_resolves_objects_with_an_id(self): + class FakeSession: + id: str = "session-a" + + fields = scope_recall_fields(scope=None, sessions=[FakeSession()]) # pyright: ignore[reportArgumentType] + assert fields == {"filters": {"session_id": ["session-a"]}} + + +class TestScopeContextFields: + """The context route takes these as query params, not as a `filters` body.""" + + def test_neither_option_contributes_nothing(self): + assert context_fields() == {} + + def test_scope_passes_through(self): + assert context_fields(scope="therapy") == {"scope": "therapy"} + + def test_sessions_stays_a_plain_list(self): + # Not wrapped in `filters` — this route reads a repeated query parameter. + assert context_fields(sessions=["a", "b"]) == {"sessions": ["a", "b"]} + + def test_scope_and_peer_perspective_are_mutually_exclusive(self): + with pytest.raises(ValueError, match="mutually exclusive"): + context_fields(scope="therapy", peer_perspective="assistant") + + def test_scope_and_sessions_are_mutually_exclusive(self): + with pytest.raises(ValueError, match="mutually exclusive"): + context_fields(scope="therapy", sessions=["a"]) + + def test_sessions_and_limit_to_session_are_mutually_exclusive(self): + with pytest.raises(ValueError, match="mutually exclusive"): + context_fields(sessions=["a"], limit_to_session=True) + + @pytest.mark.parametrize("option", [{"scope": "therapy"}, {"sessions": ["a"]}]) + def test_either_option_requires_a_peer_target(self, option: dict[str, object]): + # Both only reach the representation, and there is none without a target. + # Refused rather than accepted and silently ignored. + with pytest.raises(ValueError, match="peer_target"): + context_fields(peer_target=None, **option) + + def test_limit_to_session_alone_is_untouched(self): + # The neutral case must not start emitting a scope/sessions key. + assert context_fields(limit_to_session=True) == {} + + +class TestResolveScopeMembership: + def test_resolves_ids_and_objects_in_order(self): + class FakeSession: + id: str = "session-b" + + assert resolve_scope_membership(["session-a", FakeSession()]) == [ # pyright: ignore[reportArgumentType] + "session-a", + "session-b", + ] + + def test_rejects_empty(self): + with pytest.raises(ValueError, match="At least one session"): + resolve_scope_membership([]) + + def test_rejects_over_the_per_call_cap_rather_than_chunking(self): + with pytest.raises(ValueError, match="At most"): + resolve_scope_membership([f"s{i}" for i in range(MAX_SESSIONS_PER_ADD + 1)]) + + def test_rejects_a_malformed_id(self): + with pytest.raises(ValueError, match="must match pattern"): + resolve_scope_membership(["ok-session", "valid-session?typo"]) + + +class TestResolveScopeSession: + """Guards the ID that gets interpolated into a scope membership URL path.""" + + def test_resolves_a_plain_id(self): + assert resolve_scope_session("session-a") == "session-a" + + def test_resolves_an_object(self): + class FakeSession: + id: str = "session-a" + + assert resolve_scope_session(FakeSession()) == "session-a" # pyright: ignore[reportArgumentType] + + @pytest.mark.parametrize( + "malformed", + [ + "valid-session?typo", # would address `valid-session` + a query string + "valid-session/../other", # would climb the path + "valid session", + "", + ], + ) + def test_rejects_ids_that_would_alter_the_request_path(self, malformed: str): + # This value lands in a DELETE path. An unvalidated id silently changes + # which session is removed, and removal triggers reconciliation against + # whatever it hits. + with pytest.raises(ValueError, match="Session ID"): + resolve_scope_session(malformed) + + +def test_deprecated_conclusion_scope_aliases_still_resolve(): + """The rename keeps working for callers on the old name.""" + from sdks.python.src.honcho import ( + ConclusionScope, + ConclusionScopeAio, + ConclusionsView, + ConclusionsViewAio, + ) + + assert ConclusionScope is ConclusionsView + assert ConclusionScopeAio is ConclusionsViewAio diff --git a/tests/telemetry/test_metric_zero_init.py b/tests/telemetry/test_metric_zero_init.py new file mode 100644 index 00000000..e69f50df --- /dev/null +++ b/tests/telemetry/test_metric_zero_init.py @@ -0,0 +1,357 @@ +"""Tests for startup zero-initialization of bounded-label metrics. + +Asserts that: +- bounded-label counter children are materialized at 0 before any event, +- high-cardinality / impossible label combinations are deliberately NOT, +- per-process init doesn't materialize the other process's counters, +- the explicit registries stay in sync with the source of truth (drift guards). +""" +# region ai +# Reads use ``REGISTRY.get_sample_value`` (returns the value if a series exists, +# ``None`` if it does not) rather than ``counter.labels(...)``, because ``.labels`` +# would itself materialize the child and destroy the presence/absence signal. +# endregion + +from collections.abc import Iterator +from typing import cast +from uuid import uuid4 + +import pytest +from prometheus_client import REGISTRY + +from src.config import REASONING_LEVELS, settings +from src.dreamer.specialists import BaseSpecialist +from src.telemetry.events import ALL_EVENT_TYPES, HIGH_VOLUME_EVENT_TYPES +from src.telemetry.events.base import BaseEvent +from src.telemetry.prometheus.metrics import ( + _DERIVER_TOKEN_COMBOS_BY_TASK, # pyright: ignore[reportPrivateUsage] + DeriverComponents, + DeriverTaskTypes, + DialecticComponents, + TokenTypes, + prometheus_metrics, +) +from src.utils.types import walk_subclasses + + +def unique_ns(tag: str) -> str: + """A ``namespace`` label value no other test can have materialized under.""" + # region ai + # Every assertion here reads the process-global ``REGISTRY``, which keeps a child + # series for the rest of the session once anything materializes it. A shared + # namespace (several other suites pin ``"test"``) would let another test's children + # satisfy a presence assertion, or break an absence assertion, independently of + # what the initializer under test actually did. + # endregion + return f"test_metric_zero_init_{tag}_{uuid4().hex[:8]}" + + +@pytest.fixture +def metrics_enabled(monkeypatch: pytest.MonkeyPatch) -> Iterator[str]: + """Enable metrics under a namespace unique to the requesting test.""" + ns = unique_ns("enabled") + monkeypatch.setattr("src.config.settings.METRICS.ENABLED", True) + monkeypatch.setattr("src.config.settings.METRICS.NAMESPACE", ns) + yield ns + + +def sample(name: str, **labels: str) -> float | None: + """Value of a series if it exists, else None. Never materializes it. + + Resolves the namespace from settings, so it always reads the unique one the + active test pinned. + """ + ns = cast(str, settings.METRICS.NAMESPACE) + return REGISTRY.get_sample_value(name, {"namespace": ns, **labels}) + + +# --------------------------------------------------------------------------- +# Drift guards (pure logic — no registry). Adding an event type / token component +# without updating the registry fails here, with a pointer to what to fix. +# --------------------------------------------------------------------------- + + +def test_all_event_types_registry_matches_subclasses(): + """ALL_EVENT_TYPES must equal every BaseEvent subclass's _event_type. + + If this fails you added/removed a BaseEvent subclass without updating + ALL_EVENT_TYPES in src/telemetry/events/__init__.py — its Prometheus counter + would not be zero-initialized. Update the registry. + """ + discovered = { + event_type + for cls in walk_subclasses(BaseEvent) + if (event_type := getattr(cls, "_event_type", None)) is not None + } + assert set(ALL_EVENT_TYPES) == discovered + assert len(ALL_EVENT_TYPES) == len(set(ALL_EVENT_TYPES)), "duplicate event types" + + +def test_high_volume_registry_matches_subclasses(): + """HIGH_VOLUME_EVENT_TYPES must equal the high_volume-classed subclasses.""" + discovered = { + event_type + for cls in walk_subclasses(BaseEvent) + if (event_type := getattr(cls, "_event_type", None)) is not None + and getattr(cls, "_volume_class", None) == "high_volume" + } + assert set(HIGH_VOLUME_EVENT_TYPES) == discovered + assert set(HIGH_VOLUME_EVENT_TYPES) <= set(ALL_EVENT_TYPES) + + +def test_deriver_token_combos_are_valid_and_complete(): + """Every combo uses real enum values; the union across tasks covers every + DeriverComponent; and no task enumerates an impossible pair. + + Fails if a DeriverComponent/DeriverTaskType is added without deciding which + task_type + token_type it pairs with in _DERIVER_TOKEN_COMBOS_BY_TASK. + """ + valid_token_types = {t.value for t in TokenTypes} + valid_components = {c.value for c in DeriverComponents} + valid_task_types = {t.value for t in DeriverTaskTypes} + + assert set(_DERIVER_TOKEN_COMBOS_BY_TASK) == valid_task_types + all_components: set[str] = set() + for task_type, combos in _DERIVER_TOKEN_COMBOS_BY_TASK.items(): + assert task_type in valid_task_types + for token_type, component in combos: + assert token_type in valid_token_types + assert component in valid_components + # each task enumerates fewer than its cartesian product (no impossible pairs) + assert len(combos) < len(valid_token_types) * len(valid_components) + all_components.update(comp for _, comp in combos) + + # every component is reachable via some task + assert all_components == valid_components + # previous_summary is summary-only: ingestion must NOT enumerate it + ingestion = _DERIVER_TOKEN_COMBOS_BY_TASK[DeriverTaskTypes.INGESTION.value] + assert ( + TokenTypes.INPUT.value, + DeriverComponents.PREVIOUS_SUMMARY.value, + ) not in ingestion + + +# --------------------------------------------------------------------------- +# API-process zero-init +# --------------------------------------------------------------------------- + + +@pytest.mark.usefixtures("metrics_enabled") +def test_api_init_materializes_event_type_children(): + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + for event_type in ALL_EVENT_TYPES: + assert sample("telemetry_events_emitted_total", type=event_type) is not None + for event_type in HIGH_VOLUME_EVENT_TYPES: + assert sample("telemetry_events_sampled_out_total", type=event_type) is not None + + +@pytest.mark.usefixtures("metrics_enabled") +def test_api_init_materializes_dialectic_and_embed(): + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + for token_type in TokenTypes: + for level in REASONING_LEVELS: + assert ( + sample( + "dialectic_tokens_processed_total", + token_type=token_type.value, + component=DialecticComponents.TOTAL.value, + reasoning_level=level, + ) + is not None + ) + assert sample("embed_now_tasks_shed_total") is not None + assert sample("embed_now_tasks_in_flight") == 0.0 # gauge, explicit .set(0) + + +@pytest.mark.usefixtures("metrics_enabled") +def test_sampled_out_excludes_ground_truth_event_types(): + """Ground-truth events can never be sampled out, so their sampled_out series + must NOT be pre-created (they'd be permanently misleading zeros).""" + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + ground_truth = set(ALL_EVENT_TYPES) - set(HIGH_VOLUME_EVENT_TYPES) + for event_type in ground_truth: + assert sample("telemetry_events_sampled_out_total", type=event_type) is None + + +# --------------------------------------------------------------------------- +# Deriver-process zero-init +# --------------------------------------------------------------------------- + + +@pytest.mark.usefixtures("metrics_enabled") +def test_deriver_init_materializes_token_and_backlog(): + prometheus_metrics.initialize_bounded_metrics(instance_type="deriver") + for task_type, combos in _DERIVER_TOKEN_COMBOS_BY_TASK.items(): + for token_type, component in combos: + assert ( + sample( + "deriver_tokens_processed_total", + task_type=task_type, + token_type=token_type, + component=component, + ) + is not None + ) + # region ai + # Specialist names are derived from the concrete BaseSpecialist subclasses here + # too, rather than hardcoded: a hardcoded list would keep passing when a new + # specialist is added (it only asserts presence), silently leaving it uncovered. + # endregion + specialist_names = { + name + for cls in walk_subclasses(BaseSpecialist) + if (name := getattr(cls, "name", None)) is not None + } + assert {"deduction", "induction", "card_refresh"} <= specialist_names + for specialist_name in specialist_names: + assert ( + sample( + "dreamer_tokens_processed_total", + specialist_name=specialist_name, + token_type=TokenTypes.INPUT.value, + ) + is not None + ), f"specialist {specialist_name!r} was not zero-initialized" + assert sample("message_embeddings_pending") == 0.0 # gauge zero-init + + +@pytest.mark.usefixtures("metrics_enabled") +def test_deriver_init_omits_impossible_token_combos(): + """The cartesian product includes combos that never occur (e.g. output tokens + with an input component). Those must not be materialized.""" + prometheus_metrics.initialize_bounded_metrics(instance_type="deriver") + # output tokens never pair with an input component + assert ( + sample( + "deriver_tokens_processed_total", + task_type=DeriverTaskTypes.INGESTION.value, + token_type=TokenTypes.OUTPUT.value, + component=DeriverComponents.PROMPT.value, + ) + is None + ) + # previous_summary is summary-only — ingestion must not materialize it + assert ( + sample( + "deriver_tokens_processed_total", + task_type=DeriverTaskTypes.INGESTION.value, + token_type=TokenTypes.INPUT.value, + component=DeriverComponents.PREVIOUS_SUMMARY.value, + ) + is None + ) + # base specialist is abstract and never emits — must not be materialized + assert ( + sample( + "dreamer_tokens_processed_total", + specialist_name="base", + token_type=TokenTypes.INPUT.value, + ) + is None + ) + + +# --------------------------------------------------------------------------- +# High-cardinality counters are left open, and per-process isolation holds +# --------------------------------------------------------------------------- + + +@pytest.mark.usefixtures("metrics_enabled") +def test_high_cardinality_counters_not_materialized(): + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + prometheus_metrics.initialize_bounded_metrics(instance_type="deriver") + # no endpoint/workspace_name series fabricated + assert ( + sample( + "api_requests_total", + method="GET", + endpoint="/v3/does-not-exist", + status_code="200", + ) + is None + ) + assert sample("messages_created_total", workspace_name="nope_ws") is None + + +@pytest.mark.usefixtures("metrics_enabled") +def test_api_init_does_not_touch_deriver_counters(): + """api-only init must not materialize or change a deriver-only counter. + + Delta-based (before == after) so it's robust to prior tests having + materialized the series. + """ + labels = dict( + task_type=DeriverTaskTypes.INGESTION.value, + token_type=TokenTypes.INPUT.value, + component=DeriverComponents.PROMPT.value, + ) + before = sample("deriver_tokens_processed_total", **labels) + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + after = sample("deriver_tokens_processed_total", **labels) + assert before == after + + +@pytest.mark.usefixtures("metrics_enabled") +def test_deriver_init_does_not_touch_api_counters(): + """The inverse: deriver-only init must not materialize an API-only counter. + + Without this, a deriver-startup regression could silently fabricate API + series (permanently-0 dialectic tokens on a process that never serves chat). + """ + labels = dict( + token_type=TokenTypes.INPUT.value, + component=DialecticComponents.TOTAL.value, + reasoning_level=REASONING_LEVELS[0], + ) + before = sample("dialectic_tokens_processed_total", **labels) + prometheus_metrics.initialize_bounded_metrics(instance_type="deriver") + after = sample("dialectic_tokens_processed_total", **labels) + assert before == after + # the API-process embed_now counters are equally off-limits + assert sample("embed_now_tasks_shed_total") is None + assert sample("embed_now_tasks_in_flight") is None + + +# --------------------------------------------------------------------------- +# telemetry_events_dropped: per-emitter child materialization +# --------------------------------------------------------------------------- + + +@pytest.mark.usefixtures("metrics_enabled") +def test_dropped_counter_children_materialized(): + prometheus_metrics.initialize_telemetry_dropped_metrics( + reasons=["buffer_full", "send_failed"] + ) + assert sample("telemetry_events_dropped_total", reason="buffer_full") is not None + assert sample("telemetry_events_dropped_total", reason="send_failed") is not None + + +def test_dropped_counter_init_noop_when_metrics_disabled( + monkeypatch: pytest.MonkeyPatch, +): + """The per-emitter initializer must no-op when metrics are disabled.""" + # region ai + # The enabled/disabled pair above and below this line exists for + # ``initialize_bounded_metrics`` (see ``test_init_noop_when_metrics_disabled``); + # without this test the sibling initializer had only the enabled half, so its + # ``METRICS.ENABLED`` guard could be deleted with the suite staying green. The + # unique namespace is what makes the absence assertion mean anything — the enabled + # test above materializes these same two reason values under a different one. + # endregion + monkeypatch.setattr("src.config.settings.METRICS.ENABLED", False) + monkeypatch.setattr( + "src.config.settings.METRICS.NAMESPACE", unique_ns("dropped_disabled") + ) + prometheus_metrics.initialize_telemetry_dropped_metrics( + reasons=["buffer_full", "send_failed"] + ) + assert sample("telemetry_events_dropped_total", reason="buffer_full") is None + assert sample("telemetry_events_dropped_total", reason="send_failed") is None + + +def test_init_noop_when_metrics_disabled(monkeypatch: pytest.MonkeyPatch): + """With metrics disabled, init must not fabricate series for a fresh label.""" + monkeypatch.setattr("src.config.settings.METRICS.ENABLED", False) + monkeypatch.setattr("src.config.settings.METRICS.NAMESPACE", unique_ns("disabled")) + prometheus_metrics.initialize_bounded_metrics(instance_type="api") + assert sample("telemetry_events_emitted_total", type="message.created") is None diff --git a/tests/telemetry/test_representation_v2_fields.py b/tests/telemetry/test_representation_v2_fields.py index a41bed41..6f2251ea 100644 --- a/tests/telemetry/test_representation_v2_fields.py +++ b/tests/telemetry/test_representation_v2_fields.py @@ -51,6 +51,7 @@ class TestRepresentationV2AdditiveFields: assert event.exact_dup_existing_count == 0 assert event.semantic_dup_rejected_count == 0 assert event.semantic_dup_replaced_count == 0 + assert event.failed_observer_count == 0 def test_input_tokens_semantics_preserved(self): """The downstream metering key must remain 'queued-message tokens'. @@ -161,6 +162,7 @@ class TestRepresentationV2AdditiveFields: "exact_dup_existing_count", "semantic_dup_rejected_count", "semantic_dup_replaced_count", + "failed_observer_count", ): assert field in data, f"missing field: {field}" assert data["hit_batch_token_cap"] is True diff --git a/tests/utils/test_agent_tools.py b/tests/utils/test_agent_tools.py index c13fb1e7..ac45cabf 100644 --- a/tests/utils/test_agent_tools.py +++ b/tests/utils/test_agent_tools.py @@ -334,7 +334,9 @@ class TestCreateObservations: """If batch embedding fails but individual embeds succeed, all observations are created.""" workspace, peer1, peer2, session, _, _ = tool_test_data - async def fail_batch_embed(_texts: list[str]) -> list[list[float]]: + async def fail_batch_embed( + _texts: list[str], **_kwargs: object + ) -> list[list[float]]: raise RuntimeError("embedding provider timeout") async def succeed_single_embed(_content: str) -> list[float]: @@ -393,7 +395,9 @@ class TestCreateObservations: """If batch embedding fails and some individual embeds also fail, only successful ones are created.""" workspace, peer1, peer2, session, _, _ = tool_test_data - async def fail_batch_embed(_texts: list[str]) -> list[list[float]]: + async def fail_batch_embed( + _texts: list[str], **_kwargs: object + ) -> list[list[float]]: raise RuntimeError("embedding provider timeout") async def embed_per_observation(content: str) -> list[float]: @@ -458,7 +462,9 @@ class TestCreateObservations: workspace, peer1, peer2, session, _, _ = tool_test_data created_documents: list[Any] = [] - async def fake_batch_embed(texts: list[str]) -> list[list[float]]: + async def fake_batch_embed( + texts: list[str], **_kwargs: object + ) -> list[list[float]]: assert texts == ["trimmed observation"] return [[0.4, 0.5, 0.6]] @@ -504,6 +510,60 @@ class TestCreateObservations: assert len(created_documents) == 1 assert created_documents[0].content == "trimmed observation" + async def test_create_observations_embeds_with_truncate_on_oversize( + self, + tool_test_data: Any, + monkeypatch: pytest.MonkeyPatch, + ): + """Storage path must opt into truncation so one long obs cannot drop the batch.""" + workspace, peer1, peer2, session, _, _ = tool_test_data + captured: dict[str, object] = {} + + async def fake_batch_embed( + texts: list[str], *, on_oversize: str = "raise", **_kwargs: object + ) -> list[list[float]]: + captured["texts"] = texts + captured["on_oversize"] = on_oversize + return [[0.1] for _ in texts] + + async def fake_create_documents( + _db: AsyncSession, + documents: list[Any], + workspace_name: str, + *, + observer: str, + observed: str, + deduplicate: bool = False, + ) -> crud.CreateDocumentsResult: + _ = (workspace_name, observer, observed, deduplicate) + return crud.CreateDocumentsResult(created_documents=documents) + + monkeypatch.setattr( + "src.utils.agent_tools.embedding_client.simple_batch_embed", + fake_batch_embed, + ) + monkeypatch.setattr( + "src.utils.agent_tools.crud.create_documents", fake_create_documents + ) + + result = await create_observations( + observations=[ + schemas.ObservationInput(content="short fact", level="explicit"), + schemas.ObservationInput(content="long fact", level="explicit"), + ], + observer=peer1.name, + observed=peer2.name, + session_name=session.name, + workspace_name=workspace.name, + message_ids=[], + message_created_at=str(datetime.now(timezone.utc)), + ) + + assert isinstance(result, ObservationsCreatedResult) + assert result.created_count == 2 + assert captured["on_oversize"] == "truncate" + assert captured["texts"] == ["short fact", "long fact"] + async def test_create_observations_skips_all_blank_content( self, tool_test_data: Any, @@ -541,6 +601,62 @@ class TestCreateObservations: batch_embed.assert_not_awaited() create_documents.assert_not_awaited() + @pytest.mark.parametrize("deduplicate_setting", [True, False]) + async def test_create_observations_honors_deduplicate_setting( + self, + tool_test_data: Any, + monkeypatch: pytest.MonkeyPatch, + deduplicate_setting: bool, + ): + """create_observations forwards settings.DERIVER.DEDUPLICATE to create_documents. + + Guards against reintroducing a hardcoded deduplicate=True, which made + DERIVER_DEDUPLICATE=false unable to disable dedup on this path (#989). + """ + workspace, peer1, peer2, session, _, _ = tool_test_data + monkeypatch.setattr(settings.DERIVER, "DEDUPLICATE", deduplicate_setting) + + captured: dict[str, Any] = {} + + async def fake_batch_embed(texts: list[str]) -> list[list[float]]: + return [[0.1, 0.2, 0.3] for _ in texts] + + async def fake_create_documents( + _db: AsyncSession, + documents: list[Any], + workspace_name: str, + *, + observer: str, + observed: str, + deduplicate: bool = False, + ) -> crud.CreateDocumentsResult: + _ = (workspace_name, observer, observed) + captured["deduplicate"] = deduplicate + return crud.CreateDocumentsResult(created_documents=documents) + + monkeypatch.setattr( + "src.utils.agent_tools.embedding_client.simple_batch_embed", + fake_batch_embed, + ) + monkeypatch.setattr( + "src.utils.agent_tools.crud.create_documents", fake_create_documents + ) + + result = await create_observations( + observations=[ + schemas.ObservationInput(content="An observation", level="explicit"), + ], + observer=peer1.name, + observed=peer2.name, + session_name=session.name, + workspace_name=workspace.name, + message_ids=[], + message_created_at=str(datetime.now(timezone.utc)), + ) + + assert isinstance(result, ObservationsCreatedResult) + assert captured["deduplicate"] is deduplicate_setting + class TestNormalizeObservationId: """Unit tests for _normalize_observation_id."""