* fix(deriver): truncate oversize observations so one cannot drop the batch
simple_batch_embed raised ValueError when any input exceeded the per-input
token cap, which failed the entire deriver save when a single observation
was over-length. Add on_oversize="truncate": oversize inputs are embedded
from a token-capped prefix (re-encoded until it fits, with a warning),
preserving one vector per input. Default stays "raise" so existing callers
are unchanged. RepresentationManager opts into truncate.
Also add a live embedding test that fails on main (raise / missing kwarg)
and passes once a mixed short+oversize batch survives.
Refs #569
* fix(deriver): surface failure when all observer saves fail
When every observer's save_representation failed (e.g. embedding retries
exhausted under a sustained 429), the deriver logged the error and returned
normally, so the queue marked the work unit processed with zero documents
saved. Collect per-observer errors and, after telemetry is emitted, raise
RepresentationSaveError when no observer succeeded. Partial failures stay
processed (saved observers must not be discarded) and are recorded via an
additive failed_observer_count on RepresentationCompletedEvent.
Refs #728
* fix(embedding): guarantee truncation progress and truncate on re-embed
The retry slice in _truncate_to_token_limit always recomputed the same
keep count, so a slice whose re-encode grew past the cap could oscillate.
Decrement keep after each unsuccessful retry.
Document re-embed in the reconciler used the default on_oversize="raise",
so one oversize document failed every other document in the batch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore: drop ticket ids and shrink comments to one sentence
Comments and docstrings describe current behavior, not the PR that
introduced them. Ticket numbers stay in the commit/PR.
* chore: annotate RepresentationSaveError and assert truncate on re-embed
* fix(embedding): truncate on conclusion create paths and document BPE loop
Storage callers in create_observations (API + agent tools) now pass
on_oversize="truncate" so a single oversize item cannot drop the batch.
Docstring on _truncate_to_token_limit notes why decode/re-encode is load-bearing.
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* telemetry: materialize dropped-event counter children at 0
A labeled Prometheus counter exports no series until its first labels()
call, so telemetry_events_dropped stayed invisible until an event was
actually dropped — impossible to alert on or graph, and "no drops" was
indistinguishable from "metric missing / scrape broken".
Pre-create the (namespace, reason) children at 0 on emitter start, for
each reason the emitter can emit, so the metric is always present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* telemetry: generalize counter zero-init to all bounded-label counters
Extends #927 (which zero-inited telemetry_events_dropped) to every counter
whose label domain is bounded and known at startup, so metrics are present in
Prometheus before their first event — a missing series then signals a broken
scrape rather than "nothing happened yet".
- add initialize_bounded_metrics(instance_type) on PrometheusMetrics; call it
per-process from main.py (api) and deriver/__main__.py (deriver).
- extract a shared _touch() helper; refactor initialize_telemetry_dropped_metrics
onto it (that one stays per-emitter in start() — it's prefix-dependent).
- explicit ALL_EVENT_TYPES / HIGH_VOLUME_EVENT_TYPES registry in telemetry.events,
drift-guarded by tests that walk BaseEvent subclasses.
- only VALID (task_type, token_type, component) tuples for deriver_tokens (the
cartesian product would fabricate impossible always-0 series); only high-volume
event types for sampled_out; high-cardinality labels (endpoint, workspace_name)
left open.
- gauges: zero-init embed_now_tasks_in_flight + telemetry_buffer_size; add a new
message_embeddings_pending backlog gauge, set each reconciliation cycle and
zero-inited at deriver startup (Rajat's pending/in-flight ask).
- backfills the tests #927 shipped without.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: task-aware deriver combos + fail-soft gauge zero-init
I1: _DERIVER_TOKEN_COMBOS was factored task-independently, materializing the
impossible (ingestion, input, previous_summary) series — previous_summary is
summary-only. Make combos task-aware (_DERIVER_TOKEN_COMBOS_BY_TASK) so no
always-0 impossible series is fabricated, matching the PR's own goal. Tests
tightened to assert the ingestion/previous_summary series is absent.
I2: the three gauge .set(0) zero-inits were bare while the counter inits go
through the fail-soft _touch. Add _set_gauge_zero() so a gauge init can't
propagate an exception into process startup either.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(telemetry): isolate zero-init namespaces, add deriver-to-api guard
Global-REGISTRY assertions used a fixed "test" namespace, which several
other suites also pin, so another test's materialized children could
satisfy a presence assertion or break an absence one. Each test now runs
under a unique namespace resolved from settings at read time.
Adds the inverse per-process isolation test: deriver-only init must not
materialize API-only series (dialectic tokens, embed_now).
Co-Authored-By: Claude <noreply@anthropic.com>
* review: per-replica backlog gauge, drop duplicated constants and .meta refs
Addresses Vineeth's review on #927.
Blocking:
- message_embeddings_pending is a DB-global count, so drive it from
ReconcilerScheduler._scheduler_loop (runs on every replica, every
interval) instead of run_vector_reconciliation_cycle (runs off the
queue behind work-unit dedup, so one replica per cycle). Combined with
the zero-init, the old placement made every replica that never won the
work unit export a confident permanent 0. Help string now names the
owner so dashboards don't reach for sum().
- guard initialize_telemetry_dropped_metrics on METRICS.ENABLED,
matching its sibling initializer.
- drop the duplicate REASONING_LEVELS; import the one in src/config.
Non-blocking:
- walk BaseSpecialist recursively via a shared utils.types.walk_subclasses
(replaces the direct-children-only __subclasses__() and the test's
private copy of the same helper).
- derive the specialist assertion from the subclasses instead of
hardcoding two names — the hardcoded pair kept passing after
CardRefreshSpecialist landed, leaving it uncovered.
- inline the zero-init rationale and the multi-instance bucket taxonomy;
removes both pointers to a .meta design doc that is not in the repo.
Tests: new tests/reconciler/test_pending_backlog_gauge.py pins both
halves of the relocation (verified it fails when reverted).
* review: fix inert test guard, stale comments, and the REASONING_LEVELS drift claim
Second review pass on the branch. Findings, most severe first:
- tests/reconciler/test_pending_backlog_gauge.py: the _try_enqueue_task stub
was patched onto the class but declared without `self`, so calling it
raised TypeError — which _scheduler_loop swallows. The guard was inert and
the test passed for the wrong reason. Fixed the arity.
- metrics.py still commented that the backlog gauge is "set live each
reconciliation cycle". That is the exact claim the previous commit
overturned; it now contradicted the help string, the bucket-3 docstring
and sync_vectors.py.
- metrics.py claimed REASONING_LEVELS is "derived from the config Literal so
it never drifts", but config.py hand-listed it, so the earlier dedup had
quietly traded away the guarantee the original get_args() call provided.
Made it true instead: config.REASONING_LEVELS = list(get_args(...)), which
keeps the dedup and restores the invariant.
- dropped _set_gauge_zero: all three gauges it zeroed already have identical
fail-soft setters, so it was a second way to do one thing. Using the
setters also makes _handle_metric_error name the actual gauge.
- record_pending_embeddings_backlog's docstring oversold the covering index
as making the COUNT "negligible". The index makes cost proportional to the
pending backlog, not to the table — which is worst precisely when the
backlog matters. Stated honestly.
- _scheduler_loop's docstring said it only enqueues; it also refreshes the
gauge, at a cadence set by the shortest task interval.
- comment reconciliation: stripped #927 / "the generalization" temporal
anchoring, a CardRefreshSpecialist change-narration clause, and
reviewer-directed phrasing from the test file; disambiguated the
src/utils/summarizer.py path.
- CLAUDE.md had no Prometheus section at all, so the new "add a BaseEvent
subclass -> update ALL_EVENT_TYPES" obligation and the never-sum() rule
for non-additive gauges were undiscoverable from the architecture doc.
Verified: ruff + basedpyright clean (0 errors), tests/telemetry + reconciler
+ dialectic + llm 497 passed, full suite 1768 passed with only the 4
pre-existing test_document failures (OpenAI key required, reproduced on
clean origin/main). Re-confirmed the relocation guard fails when reverted.
* fix: silence the two basedpyright warnings inherited from main
CI runs `uv run basedpyright` bare, and basedpyright exits non-zero on any
warning — so these two have been failing the staticanalysis job on every
branch cut from current main, not just this one:
- src/vector_store/__init__.py:209 implicit string concatenation (#496)
- tests/test_cache_redaction.py:5 private import (#869)
Both predate this branch and are unrelated to the telemetry work; fixed
here only because they block this PR from going green. Verified: clean
origin/main also reports "0 errors, 2 warnings" and exits 1.
basedpyright now 0 errors, 0 warnings, exit 0.
* docs(telemetry): make the bucket-3 aggregation rule precise
The multi-instance taxonomy said a service-scoped non-additive metric has
"no aggregation correct once they disagree", then immediately mandated that
every instance refresh on its own timer. Those undercut each other: staggered
timers ALWAYS disagree slightly, so as written the rule reads as "ensure they
don't", which is unachievable, and it leaves the reader unsure whether max()
and avg() survived the fix.
The actual rule is bounded disagreement plus a scale-preserving aggregator.
Instances are N witnesses to one fact, not N parts of one whole, so sum() can
never be correct (it scales with replica count) while max()/avg()/quantiles
are correct precisely because the per-instance timer bounds the spread.
Wording only; no behavior change. The gauge help string already said
"max() or avg(), never sum()" — this makes the normative docstring agree
with it. Surfaced walking Vineeth's comment 3668208059 for comprehension.
* refactor(bench): import REASONING_LEVELS from config instead of re-listing
Third copy of the constant, missed when ee781c0/694e07f deduped the other
two. This one re-declared the ReasoningLevel Literal as well as the list,
so the type alias could diverge from config's with nothing to catch it —
and the list was hand-written, the variant that typechecks clean while
missing a member.
No import barrier justified it: this module already imports from src, as do
seven of its siblings in tests/bench. Concrete effect of the drift was that
a newly added sixth reasoning level would be rejected by the bench CLI's
argparse choices=.
src.config.REASONING_LEVELS is now the single definition repo-wide.
* test(telemetry): pin the METRICS.ENABLED guard on the per-emitter initializer
initialize_telemetry_dropped_metrics gained a METRICS.ENABLED guard in
ee781c0, addressing Vineeth's asymmetry comment, but nothing asserted it —
it had only the enabled half of the pair its sibling has. Deleting the guard
left the suite green, so the fix closed the asymmetry in the guards and
reproduced it one level up in the tests.
Mirrors test_init_noop_when_metrics_disabled. Verified live rather than
assumed: deleting the two guard lines turns this test red.
Uses a unique namespace, without which the absence assertion would be
satisfied by the enabled test's children rather than by the guard.
* docs(telemetry): fold zero-init why-prose behind # region ai markers
Comment/docstring-only pass over the changed files, per the groudon
comment-marker standard: the terse human-facing "what" stays visible, and
load-bearing "why" (the zero-init / absent-series-means-broken-scrape
rationale, gotchas, receipts) folds into # region ai / # ai: blocks.
Behavior-preserving: AST-identical modulo docstrings/comments vs the
pre-pass merge; ruff, ruff format --check, and basedpyright all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: honor DERIVER_DEDUPLICATE in create_observations
The agent-tool path hardcoded deduplicate=True, so DERIVER_DEDUPLICATE=false
could not disable dedup for observations created through this path. Pass
settings.DERIVER.DEDUPLICATE, matching crud/representation.py.
* test: cover deduplicate setting is forwarded in create_observations
* feat: scopes SDK surface and session allowlist on session context
Exposes the Scopes v1 facade in both SDKs, which until now was reachable
only by hand-rolled HTTP, and closes the Phase 1 gap where the session
allowlist never landed on the context route.
SDKs (DEV-2001, folds in DEV-1996)
New Scope class in both SDKs — addSessions / removeSession / sessions /
status — plus honcho.scope() and honcho.scopes() entry points, a
`scopes` option on session creation, and `scope` + `sessions` read
options on chat, chatStream, representation, and session.context.
`scope` on workspace search. Python covers sync and .aio equally.
`sessions` is sugar, not a new wire field: on the recall endpoints it
goes out as the constrained `filters: {session_id: [...]}` body, never
as a key of its own. Kept separate from the `filters` parameter on the
list/search methods on purpose — that one is the full filter DSL,
whereas the recall endpoints accept a single key and 422 on anything
else, so one name for two grammars would be a trap.
Server (DEV-2357)
`GET /sessions/{id}/context` accepts a `sessions` allowlist confining
the target's representation. Two deliberate choices worth review:
- Sent as a repeated query parameter rather than the `filters` body the
issue specced. The route is a GET and `session_id` is the only
supported key, so a JSON blob in a query string buys nothing.
- The peer card is omitted under an allowlist. Cards key on
(workspace, observer, observed) with no session dimension, so they
cannot be narrowed; returning one would leak exactly what the
allowlist exists to exclude. Same reasoning as ALLOWLIST_SAFE_LEVELS.
`scope` needs no carve-out — it swaps the observer to the scope peer,
so the card read is the scope's own.
`extract_session_allowlist` now delegates to a shared
`normalize_session_allowlist`, so the cap, id charset, and must_include
rule have one implementation across both entry points. Existing error
messages are unchanged.
Also in here
- ConclusionScope renamed to ConclusionsView in both SDKs. "Scope" now
means a named set of sessions, which that class is not — it is a view
over one observer/observed pair. ConclusionScope kept as a deprecated
alias; the package-level import path only.
- The TS HTTP client comma-joined array query params, so any list-valued
parameter arrived as one malformed entry. Fixed at buildURL rather
than the call site.
Not in this PR: the "How Scopes Work" docs guide and the Groudon
dashboard tab (both DEV-2001), and CHANGELOG entries for Phases 2a-2c,
which are still merged-but-unrecorded.
Verified: ruff, basedpyright, tsc --noEmit, biome all clean. New unit
tests cover the SDK option translation and the Scope client, but the
context route's own behavior — the 422s, the 401 membership gate, the
dropped peer card — has no test yet; the analogous chat/representation
cases in tests/test_session_allowlist.py are the place for it.
Refs DEV-2001, DEV-1996, DEV-2357
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Exposes the Scopes v1 facade in both SDKs, which until now was reachable
only by hand-rolled HTTP, and closes the Phase 1 gap where the session
allowlist never landed on the context route.
SDKs (DEV-2001, folds in DEV-1996)
New Scope class in both SDKs — addSessions / removeSession / sessions /
status — plus honcho.scope() and honcho.scopes() entry points, a
`scopes` option on session creation, and `scope` + `sessions` read
options on chat, chatStream, representation, and session.context.
`scope` on workspace search. Python covers sync and .aio equally.
`sessions` is sugar, not a new wire field: on the recall endpoints it
goes out as the constrained `filters: {session_id: [...]}` body, never
as a key of its own. Kept separate from the `filters` parameter on the
list/search methods on purpose — that one is the full filter DSL,
whereas the recall endpoints accept a single key and 422 on anything
else, so one name for two grammars would be a trap.
Server (DEV-2357)
`GET /sessions/{id}/context` accepts a `sessions` allowlist confining
the target's representation. Two deliberate choices worth review:
- Sent as a repeated query parameter rather than the `filters` body the
issue specced. The route is a GET and `session_id` is the only
supported key, so a JSON blob in a query string buys nothing.
- The peer card is omitted under an allowlist. Cards key on
(workspace, observer, observed) with no session dimension, so they
cannot be narrowed; returning one would leak exactly what the
allowlist exists to exclude. Same reasoning as ALLOWLIST_SAFE_LEVELS.
`scope` needs no carve-out — it swaps the observer to the scope peer,
so the card read is the scope's own.
`extract_session_allowlist` now delegates to a shared
`normalize_session_allowlist`, so the cap, id charset, and must_include
rule have one implementation across both entry points. Existing error
messages are unchanged.
Also in here
- ConclusionScope renamed to ConclusionsView in both SDKs. "Scope" now
means a named set of sessions, which that class is not — it is a view
over one observer/observed pair. ConclusionScope kept as a deprecated
alias; the package-level import path only.
- The TS HTTP client comma-joined array query params, so any list-valued
parameter arrived as one malformed entry. Fixed at buildURL rather
than the call site
* fix(scopes): close peer-card leak under limit_to_session, harden SDK inputs
Addresses review findings on the scopes work. All four were verified by
reproducing them, not by reading.
Peer card no longer leaks under any allowlist
The card was dropped when `sessions` was set but returned when
`limit_to_session=true` produced the identical allowlist, so a control
meant to fail closed was defeated by swapping one query parameter. It is
now gated on the effective allowlist, computed once and shared by the
representation call and the card read — the duplicated inline
conditional is what let the two drift apart.
`POST /peers/{id}/chat` still injects an unscoped card under an
allowlist (src/dialectic/chat.py fetches it on peer_card.use alone, with
no reference to session_allowlist). Left alone deliberately: that is a
behavior change to the shipped dialectic and
Scope validation messages survive the option union
ScopeOptionSchema is a union, and Zod collapses a failing union into one
`invalid_union` / "Invalid input" issue, burying the branch errors. Every
invalid scope on chat/representation reported "Invalid input" and told
the caller nothing — including the reserved-prefix case the check order
exists to surface. The rules are now a plain function applied after the
union resolves, so the specific message reaches the caller for bad
charset, reserved prefix, empty and over-cap lists alike.
Empty scope no longer fails open
`session.context({scope: ''})` and `honcho.search(q, {scope: ''})` used
truthiness checks, so an invalid scope was dropped and the call returned
*unscoped* results. Both now test against undefined so the value reaches
the schema.
Session IDs validated before reaching a URL path
`scope.removeSession('valid-session?typo')` addressed `valid-session`
with a stray query string: the wrong session removed, and reconciliation
run against it. Both SDKs now validate the charset first. Python's
`add_sessions` was unvalidated too — harmless in a JSON body, but
leaving one path checked and its sibling unchecked is how this recurs.
Also
- Corrected the `limit_to_session` description: it claimed "only used if
search_query is provided", but the allowlist reaches
_query_documents_recent unconditionally.
- Corrected the documented 1,000-session cap on `sessions`, which is
unreachable via repeated query params — the request line exceeds h11's
16 KB and nginx's 8 KB defaults at a few hundred entries, giving an
opaque 414/431 instead of a 422.
- Removed a dead route builder.
Tests: 7 new TypeScript cases and 2 new Python classes covering all four
findings. The TypeScript unit suite passes 146/146. The context route
itself is still unexercised — the card gate and the 401 membership check
remain verified by reading only.
Refs DEV-2001, DEV-1996, DEV-2357, DEV-2201
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(sdk): align the conclusions-view error message across both SDKs
The ConclusionScope -> ConclusionsView rename updated the identifier but
not the prose inside the thrown message, which the rename pattern
(\bConclusionScope\b) does not match. TypeScript ended up throwing
"managed by this conclusions view" while Python still threw "managed by
this conclusion scope" — the same error, different text per SDK.
Three server-backed conclusions.test.ts cases assert that message by
regex and failed under `pytest -k typescript`. The four equivalent Python
assertions were passing, because they matched Python's unchanged string —
so fixing only the TypeScript tests would have made the suite green with
the divergence still in place.
Brings Python's message, comments and docstring in line with TypeScript,
and updates the assertions in both suites. `grep -ri 'conclusion scope'`
is now empty.
Verified: 64 passed across tests/sdk_typescript/, tests/sdk/test_conclusions.py
and tests/sdk/test_scope_options.py — the last of which had never been run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(llm): forward provider_params.timeout to the OpenAI-compatible embedding client
#832 and #903 added a configurable request timeout for the LLM registry
and the Gemini embedding client respectively, but the OpenAI-compatible
embedding client (src/embedding_client.py) was never wired up. It
constructed AsyncOpenAI with no timeout at all, so a stalled socket
against a slow or contended OpenAI-compatible backend (e.g. a
self-hosted embedding model under load) wedges the deriver worker's
event loop indefinitely — the exact failure #785/#903 describe, just
via a code path #903 didn't cover.
EmbeddingModelConfig now carries provider_params through from
resolve_embedding_model_config, mirroring how resolve_model_config
already does it for ModelConfig, and the OpenAI branch of
_EmbeddingClient.__init__ extracts `timeout` via the existing
request_timeout_from_extra_params helper. Unset stays unset — no
existing behavior changes.
Reproduced and verified against a real self-hosted deployment (local
Ollama backend under load): before this fix, a single stuck embedding
call blocked all deriver queue processing for 20+ minutes with no
error logged, twice in one session.
* fix(embedding): use first-class timeout on embedding model config
provider_params is the LLM per-request escape hatch; embedding timeouts are
client-construction knobs and belong next to max_batch_size. Wire the field
for OpenAI and Gemini, omit the OpenAI kwarg when unset so the SDK default
stays, and keep Gemini's 10-minute floor when unset.
* test(embedding): live coverage for first-class embedding timeout
Exercise EmbeddingModelConfig.timeout on one representative OpenAI and
Gemini model: configured timeout lands on the SDK client, and a near-zero
timeout aborts before the provider answers.
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
* fix(deriver): delimit examples and forbid example leakage
Wrap the minimal deriver EXAMPLES block in <examples> tags and add an explicit negative instruction: the examples are fabricated format illustrations only, and every conclusion must be grounded in the <messages> block. Soft mitigation for example-content leakage; real fixes (sources_indices + SFT / structural enforcement) are follow-ups.
* test(deriver): drop low-value examples-delimiter unit test
* fix: enforce explicit-document session purity in dedup/merge paths
Audit for DEV-2000 (Scopes RFC prerequisite): explicit-level documents must
stay session-pure so scope memory can be built by copying explicit documents
between collections. Two classes of violation were possible:
- Exact-content and semantic dedup in crud/document.py matched candidates
with no level or session scoping, so an explicit document could be
reinforced by — or soft-deleted in favor of — a same-content document from
a different session or a different level (silently merging cross-session
derivations into one row).
- The generic create_observations tool handler accepted level='explicit'
from agents with no message context (dreamer/dialectic), which would mint
session-less explicit documents.
Enforcement (refuse, never rewrite):
- create_documents refuses explicit documents with a null session_name
- exact dedup keys on (content, level, session-for-explicit); derived levels
keep cross-session consolidation
- is_rejected_duplicate scopes candidate search to the same level, and the
same session for explicit documents
- the create_observations tool rejects explicit-level input outside message
ingestion (deriver) context
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add card_refresh dream type for event-driven peer-card updates
Adds a lightweight dream variant (DEV-2000, Scopes RFC prerequisite) that
runs ONLY the peer-card update — for event-driven refreshes such as scope
membership changes and cold starts:
- DreamType.CARD_REFRESH alongside OMNI; dispatched by process_dream to a
new run_card_refresh_dream orchestration
- CardRefreshSpecialist: restricted to get_recent_observations,
search_memory, and update_peer_card (no observation-mutating tools), with
a low tool-iteration cap of min(6, DREAM.MAX_TOOL_ITERATIONS)
- rebuild=True mode carried in the dream payload: the existing card is NOT
injected into the prompt and the specialist rebuilds it solely from
observations present in the collection (for use after removals)
- enqueue-able via the manual enqueue_dream path (bypasses volume gates);
the work-unit key already embeds the dream type so a card refresh never
collides with a pending omni dream. POST /v3/workspaces/{id}/schedule_dream
accepts dream_type=card_refresh plus the rebuild flag
- card refreshes never advance the omni dream guard pair
(last_dream_at / last_dream_document_count)
- shared PEER CARD prompt section extracted (verbatim) from
DeductionSpecialist for reuse; CallPurpose gains dream.card_refresh
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: reserve scope__ peer namespace with kind flag and guardrails
Introduce the scope peer namespace (scope__<name>) and the authoritative
{"kind": "scope"} configuration flag, plus the server-side guardrails:
- src/utils/scopes.py: single source of truth for the prefix, kind flag,
and name helpers (scope_peer_name / is_scope_peer_name /
scope_name_from_peer / validate_no_scope_peer_names)
- reject reserved-prefix names on peer get-or-create (422)
- reject scope peers as message authors in crud.create_messages (422)
- reject scope peers as chat/representation targets (422); a scope peer
as the path-level observer is deferred to Phase 2b
- reject scope peers on the generic session-peer add/set/remove routes
and the session-create peers mapping (422, directing to scopes routes)
- peers.list excludes scope peers by default; new PeerGet.kind option
("scope" | "all") switches the view via a configuration JSONB filter
- schemas: Scope / ScopeCreate / ScopeSessions(Add) and
SessionCreate.scopes (unprefixed scope names, validated)
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add scopes CRUD routes and session-create scopes wiring
New /v3/workspaces/{workspace_id}/scopes facade (workspace-level auth;
peer- and session-scoped keys are rejected):
POST "" create-or-get (201/200)
POST /list paginated scope list
GET /{scope_id} single scope
POST /{scope_id}/sessions add memberships
DELETE /{scope_id}/sessions/{session_id} remove membership
GET /{scope_id}/sessions list member session ids
- crud/scope.py: get_or_create_scopes stamps the backing peer with
{"kind": "scope", "observe_me": false} and refuses to adopt a
legacy peer occupying the reserved name without the flag (409)
- memberships are session_peers rows with observe_others=true /
observe_me=false — identical to a hand-built observer peer
- SessionCreate.scopes: create-or-get each scope peer and add the
membership at session creation (the no-backfill common path)
- crud/session.py: public upsert_session_peers wrapper so the facade
bypasses the route-level guardrails without reaching into privates
Backfill of pre-existing documents and reconciliation on removal land in
DEV-1999; membership only affects messages ingested after the change.
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: cover scopes facade, guardrails, and observer semantics
- create-or-get idempotency, list/get, name validation, legacy-collision
rejection (409), auth scoping (workspace key ok, peer/session keys 401)
- reserved prefix rejected on peer create; peers.list kind filtering
- scope peers rejected as message authors, chat/representation targets,
and on the generic session-peer routes
- membership add/list/remove with observe_others=true / observe_me=false
row shape asserted via DB, and facade-less equivalence with a
hand-built observer peer
- end-to-end litmus: after adding a session to a scope, the deriver
enqueue fan-out includes the scope peer as an observer
- session creation with scopes: [a, b] creates both memberships
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: scope backfill-by-copy and removal reconciliation jobs
Retroactive scope membership changes (DEV-1999):
- New queue task types scope_backfill / scope_removal with payloads,
work-unit keys ({task}:{workspace}:{scope_peer}:{session}), deduped
enqueue (mirrors enqueue_dream), and consumer dispatch.
- Backfill copies a session's explicit documents from each sender's
global (P, P) collection into the scope's (scope_peer, P) collection
with internal_metadata.copied_from as the idempotency marker;
soft-deleted copies from an earlier removal are restored, so
add -> remove -> re-add converges on exactly one live copy. Completion
enqueues one manual omni dream per touched collection.
- Removal soft-deletes the session's explicit documents in the scope's
collections and cascades (fail-closed, transitively) to derived
documents whose source_ids intersect anything removed, deletes the
vectors from the external store, then enqueues a card_refresh dream
with rebuild=True plus a manual omni dream per touched collection.
- Zero LLM re-derivation: explicit documents are session-pure (DEV-2000
invariant); the only external call is re-embedding rows whose
embedding column is NULL (external-store deployments).
- Per-session job status lives in the scope peer's internal_metadata
under backfill_status, written via single-statement JSONB merges
(concurrent-writer safe) and surfaced at
GET /v3/workspaces/{w}/scopes/{scope_id}/status.
- Enqueued from the scopes add-sessions route and SessionCreate.scopes
handling, only when the session already has messages.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: add scope backfill/removal test coverage, fix backfill status JSONB bug
Adds tests/deriver/test_scope_backfill.py covering the DEV-1999 scope
backfill-by-copy and removal reconciliation jobs implemented in a prior
commit: explicit-doc copying, copied_from idempotency (including
add->remove->re-add), multi-peer collection routing, removal cascade to
dependent derived docs, dream enqueues (manual omni on backfill;
card_refresh rebuild + omni on removal), the status route, and add-sessions
route wiring (backfill enqueued only when the session already has messages).
Fixes a real production bug surfaced by these tests: update_scope_backfill_status
passed json.dumps()'d strings through SQLAlchemy cast(..., JSONB), which
double-encodes (psycopg re-serializes the already-JSON string), producing a
JSONB string scalar instead of an object. Postgres's `||` between two
non-array jsonb scalars doesn't merge — it silently wraps both into a
2-element array, corrupting backfill_status into a list. This crashed
clear_scope_backfill_status's `#-` path delete (called on every removal)
with "path element is not an integer" once a session had ever completed a
backfill. Fixed by passing raw Python dicts to cast() instead, mirroring the
working pattern already used in update_collection_internal_metadata.
Also adds src.deriver.scope_backfill.tracked_db to conftest's tracked_db
patch list — the module was missing from that per-import-site allowlist, so
its DB work ran against the real configured database instead of the
isolated per-test database.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(crud): preserve cache invalidation across get_or_create retry
`get_or_create_peers` and `get_or_create_scopes` mutate existing rows, then
insert new ones inside `db.begin_nested()`. When a concurrent writer creates
one of those rows first, the insert raises IntegrityError and the function
retries.
`begin_nested()` autoflushes the pending UPDATEs *before* opening the
savepoint, so the rollback neither undoes them nor expires the now-clean ORM
state. The retry then compared already-updated values, found no change, and
dropped those peers from `changed_peers` — skipping the cache purge while the
row change committed anyway, leaving entries stale until the 300s TTL.
Carry the mutated names into the retry via `_pending_invalidation` so the
purge cannot be lost.
The scopes facade mirrors `get_or_create_peers`, so both copies carried this.
The peer path is pre-existing and runs on every message ingest.
Also add the missing /v3 prefix to `_SCOPES_ROUTE_GUIDANCE`, which pointed
callers at a 404.
Adds tests/crud/test_get_or_create_retry_invalidation.py, which drives a real
racing session and fails without this change.
* fix(scopes): make scope identity unforgeable, unblock non-pattern peer names
Three coupled changes to the scopes facade.
1. `PeerCreate` no longer gates internal lookups. It exists to validate a new,
user-supplied peer id at the API boundary, but crud used it as a DTO for
names that already exist, so any name outside RESOURCE_NAME_PATTERN raised a
raw pydantic ValidationError — which is not a HonchoException, so it fell
through to the catch-all handler as an HTTP 500. Adds `PeerSpec` (same
fields, no charset pattern) as `PeerCreate`'s base, widens
`get_or_create_peers` to accept it, and changes `get_peer` to take a plain
str. All 13 construction sites converted; the create route keeps full
validation.
This unbreaks the Dreamer: DreamScheduler passes `collection.observer`
straight into the specialist preflight, and scope peers have
`observe_others=true`, so every `(scope.x, peer)` dream died there — the
feature scopes exist to enable. It also fixes a pre-existing bug unrelated
to scopes: a peer named `alice.smith` (legal before d429de0e5338, which
validated names by length alone) 500s on message create, session peer add,
and peer update.
2. The `kind` flag moves from `configuration` to `internal_metadata`.
`configuration` is user-writable — `PeerCreate`/`PeerUpdate` accept a
free-form dict and `update_peer` replaces it wholesale — so a legitimate
`{"observe_me": true}` update silently dropped the flag, and a forged
`{"kind": "scope"}` injected an ordinary peer into `POST /scopes/list`.
`internal_metadata` appears in no API schema. `observe_me: false` stays in
`configuration`, where it belongs.
3. Scope identity requires prefix AND flag, via `is_scope_peer()` and
`scope_peer_clause()`. Neither half is forgeable: the prefix sits outside
RESOURCE_NAME_PATTERN, `internal_metadata` is unreachable. Usage-site guards
become flag-based so a legacy peer merely occupying the namespace keeps
working rather than 422-ing on its own traffic; peer create and update stay
name-based, since those must stop new names entering the namespace.
`update_peer` now returns 422 instead of 500.
Also swaps the reserved prefix from `scope__` to `scope.`: `_` is inside
RESOURCE_NAME_PATTERN, so any tenant could already own a `scope__x` peer.
No DB migration — `internal_metadata` already exists on `peers`, and no scope
peers exist in any deployment yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): validate peer names on create, close namespace squatting and upsert race
Addresses three review findings against 48047a6a.
1. `PeerSpec` let API callers create invalid and reserved-prefix peers.
Widening `get_or_create_peers` to accept a pattern-free schema fixed the
lookup 500s but also removed validation from the *insert* path, and
request-controlled names reach it via message authors, session peer maps, and
the chat observer path — none of which carry a charset pattern of their own.
Confirmed: `POST /sessions/{id}/messages` with `peer_id: "scope.x"` returned
201 and minted an unflagged squatter, after which `POST /scopes {id: x}` was
permanently 409-blocked — namespace denial of service by any caller able to
post a message. `peer_id: "not a valid name!@#"` was likewise created.
Fixed by validating only names about to be INSERTed
(`_validate_new_peer_names`), so already-existing names — legacy dotted
names, scope peers — still resolve without a spurious 422. That keeps the
Dreamer fix intact, since it reads through `get_peer`.
2. Existing reserved-prefix squatters could not be updated. The name-based guard
on `PUT /peers/{peer_id}` refused every `scope.` name, contradicting the
invariant that an unflagged squatter stays a normal peer. Now flag-based, so
behavior is three-way: a real scope is refused, an existing unflagged peer
updates, and a missing reserved-prefix name is refused by (1) rather than
minted.
3. Scope checks raced with get-or-create and the membership upsert. The
route-level guards run before peers are resolved, so a scope created
concurrently in that window would be attached by the generic path with a
default `SessionPeerConfig()`, clobbering its observer membership config.
Adds `_reject_resolved_scope_peers`, which runs on the resolved rows in the
same transaction as the upsert — no window, no extra query. The early checks
stay for better error messages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): guard scope membership config, move checks to the mutation point
Addresses a second review pass against 10655792.
1. Scope membership configuration was directly user-mutable.
`PUT /sessions/{id}/peers/{peer_id}/config` had no scope guard at all, and
`crud.set_peer_config` resolved the peer only to discard the row. Confirmed:
posting `{"observe_others": false, "observe_me": true}` for a scope returned
204 and persisted, which silently stops all fan-out into the scope and makes
Honcho form a representation *of* a scope — neither of which is reachable
through the facade. Deterministic, no race required. Now checked on the row
`get_peer` already returns, so it costs nothing and cannot race.
2. Empty and over-long names were still 500s. Removing the charset pattern from
`PeerSpec` fixed one trap but left its length bounds, and request-bound peer
names carry no length limits of their own — so `peer_id: ""` or a 513-char
name reached `PeerSpec(...)` and raised a raw pydantic ValidationError that
the catch-all turned into a 500. `PeerSpec` now carries no constraints at all
(matching its documented purpose) and every rule for a new name lives in
`_validate_new_peer_names` on the insert path.
3. Resolved-row protection generalized. The previous pass applied it only to
membership upserts, leaving check-then-use windows elsewhere: peer update
could have a concurrently-created scope's configuration replaced wholesale
(create-path validation does not fire for a peer that now exists), the chat
observer get-or-create could resolve a fresh scope as its observer, and the
generic session-peer removal could silently detach a scope from its sessions.
Each now inspects the resolved peer immediately before acting; the redundant
name-level guard on the update route is dropped in favor of the race-free one.
`remove_peers_from_session` grows an internal `_allow_scope_peers` flag because
the scopes facade ends membership through that same path and must not be blocked
by its own guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(scopes): enumerate every peer-touching route against a scope policy
Three review passes each found the same class of defect: a route nobody had
checked, rather than logic that was subtly wrong. One of them — `PUT
/sessions/{id}/peers/{id}/config`, which let any caller set a scope to
`observe_others=false` and silently stop all fan-out into it — predated this work
entirely, because the guardrail set was assembled guardrail-by-guardrail instead
of derived from the route list. Sampling review cannot close that kind of gap;
enumeration can.
Derives every route through which a peer name can reach the system and requires
each to be classified as GUARDED or EXEMPT-with-a-reason, so a newly added
peer-touching route fails the suite until someone classifies it. Detection is the
union of path shape and a walk of the dependant tree (including sub-dependency
`Form(...)` params and nested request-body models), because neither signal alone
suffices: parameter names miss `POST /sessions/{id}/peers`, whose peer names are
dict keys, and path shape misses `messages/upload`, whose `peer_id` arrives as a
form field behind a parser dependency.
Both invariants are then asserted behaviorally, by calling the routes rather than
inspecting annotations — the guards deliberately live in crud, which is what makes
`messages/upload` guarded for free via `crud.create_messages`:
- a real scope is refused on all 11 guarded routes, and the rejection must name
the scope, so an unrelated 422 (a malformed body) cannot pass the assertion;
- an *unflagged* peer merely occupying the reserved namespace is unaffected. That
half regressed once already when `update_peer` used a name-based check.
Mutation-tested all three failure modes: disabling the `set_peer_config` guard
fails the guarded test naming that route; regressing `update_peer` to name-based
fails the squatter test; adding an unclassified peer route fails the enumeration.
Covers the HTTP surface only. Peer names also reach the system through the
deriver, dreamer, and queue, which have no route table to enumerate — noted in
the module docstring rather than implied to be covered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse a scope in every observed position; enumerate per position
Addresses a fourth review pass against 62681e4d. The headline finding is that the
previous commit's enumeration test had the wrong *model*, not a missing entry.
1. Manual conclusions could create knowledge about a scope. `POST /conclusions`
validated only that observer_id and observed_id exist, so a scope as
`observed_id` persisted a conclusion about a peer carrying observe_me=false and
created an (observer, scope) collection for it. Confirmed: 201, and it read
back. `POST /schedule_dream` had the same hole via `observed`.
The fix is positional, because the invariant is:
A scope may be an OBSERVER. A scope may never be OBSERVED.
A scope as `observer_id` is how scoped conclusions are stored and must keep
working (verified still 201); as `observed_id` it is now refused. Same split
applied to schedule_dream `observed`, the peer-card `target` (which also covers
a scope's self-card, since target-omitted collapses observed to peer_id), and
session-context `peer_target`.
2. Chat target and both representation roles kept check-to-use races. Only the
chat path-level observer was re-checked on its resolved row; the target was
checked by name and then resolved without inspecting scope identity. Both are
now checked at the dialectic preflight, where observer and observed are already
resolved — an absent name has already failed by then, and an existing squatter
cannot retroactively become a scope.
3. Generic membership removal was still racy. The adjacent SELECT narrowed the
window but could not close it under READ COMMITTED. The UPDATE now carries its
own correlated NOT EXISTS against scope_peer_clause(), so Postgres evaluates
the exclusion as part of the statement and a scope committed after the advisory
check still cannot be detached.
4. New-name validation ran after the name reached Postgres. A NUL byte passed the
request schemas and PeerSpec, then raised psycopg.DataError inside the lookup —
a 500. Values that cannot correspond to a stored row by construction (NUL
bytes, over-length names) are now refused before the query.
(Over-length names already returned 422; only the wasted query was real there.)
The enumeration test is rekeyed from (method, path) to (method, path, position).
A binary per-route verdict cannot express finding 1 at all: `POST /conclusions` is
one route with two positions and opposite verdicts. Detection widens to observer /
observed / target / peer_target / peer_perspective, which surfaced four routes the
previous version never saw — conclusions, schedule_dream, queue/status, and
session context.
Also registers `src.routers.workspaces.tracked_db` in the conftest patch list; the
new guard there would otherwise have run against the real configured database
instead of the per-test one.
Mutation-tested: disabling the conclusions observed-guard fails the positional
test naming that position; adding an unclassified `observed_id` param fails
enumeration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse future scopes in observed positions, preserve scope membership
Addresses a fifth review pass against 14136e5b. All six findings reproduced
locally before fixing.
1. High — generic peer replacement removed scope memberships.
`set_peers_for_session` soft-deleted every active SessionPeer row, and the
request-level guard only inspected names *present* in the replacement map. A
caller detached a scope by simply omitting it, never naming it — so no
request-level guard could ever see it. Reproduced: scope sessions went
['<id>'] -> [] on a 200. The exclusion now lives in the UPDATE itself
(correlated NOT EXISTS against scope_peer_clause), so replacement means
"replace ordinary peers" regardless of request contents or concurrent creation.
2. High — peer cards could be pre-seeded for future scopes.
`set_peer_card` resolves only the observer and writes a JSONB key derived from
an unchecked observed name, and the route guard rejected only *existing*
flagged scopes. Reproduced: PUT card with target=scope.<missing> returned 200,
creating that scope then returned 201, and the card described the real scope.
3. High — dreams could be queued for future scopes.
The route checked `observed` in a read-only session that closed before
`enqueue_dream`, and a missing reserved name passes any is-it-a-scope check.
Reproduced: 204 with observed=scope.<missing>.
2 and 3 share a root cause, so they share a fix: a new `reject_scope_observed`
that is stricter than `reject_scope_peers` in exactly one case — a *missing*
reserved name is refused, because nothing on these paths creates the peer, so
nothing else would ever catch it. Existing unflagged squatters still pass.
Both guards moved to the mutation point: card validation into
`crud.set_peer_card` (same transaction as the JSONB write, so Dreamer and
agent-tool callers are covered), dream validation into `enqueue_dream` (same
transaction as the queue insert). The redundant route-level checks are dropped
rather than left as weaker duplicates.
4. Medium — prefixed NUL names still reached PostgreSQL.
`reject_scope_peers` filtered for the reserved prefix and sent matches to a
text comparison, so "scope.future\0name" raised psycopg.DataError — a 500.
Both guards now share `_reserved_name_candidates`, which materializes the input
once and rejects impossible values before any SQL. Materializing matters
independently: the message-author path passes a generator, and validation
iterates separately from the prefix filter, so a generator would be
half-consumed. `_reject_impossible_peer_names` now takes a Collection so the
type checker enforces that.
5. Medium — representation kept a check-to-use race.
The previous commit claimed both representation roles were rechecked after
resolution; that was wrong — only the dialectic preflight got that check, and
the representation route never goes through it. It now opens one short
read-only session *after* the embedding call, checks both positions, and passes
that same session to `get_working_representation`, so no connection is held
across external work and a scope committed later cannot have conclusions in the
collection being read.
6. Low — policy coverage was not exhaustive. `sender_id` reaches CRUD as
`observed` but was missing from the detected parameter set. ALLOW cases could
also not carry builders, so the suite never proved the other half of the
contract — that legitimate scope *observers* keep working, which a guard
rejecting scopes everywhere would satisfy. Both fixed; observer positions on
conclusions, dreams, cards, session context and queue status are now asserted
behaviorally.
Deliberately not implemented: the scope-creation backstop scanning for
pre-existing card keys and queue items naming a future backing peer. Reasoning is
recorded in `get_or_create_scopes` — no new such state can be created now, any
pre-existing row is coincidental since `scope.` was never a meaningful namespace,
the consequence is inert, and detecting card keys means a full table scan per
scope creation.
Mutation-tested each new guard: removing the replacement exclusion fails both
membership-preservation tests; weakening either observed guard to existing-only
fails the pre-seeding tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): exclude scope memberships from the session observer limit
Scope memberships carry observe_others=true, so every scope counted against
SESSION_OBSERVERS_LIMIT (default 10) — capping scopes-per-session at the limit
minus the session's real observers, and reporting the failure as
`400 Cannot create session <name> with 11 observers. ... Observers are peers
with 'observe_others' set to true.` on a membership call. Wrong on three counts:
the ceiling is undocumented and contradicts RFC §5.1 ("sessions belong to any
number of scopes"), the message describes session creation, and it leaks the
word "observer" through a facade whose entire job is hiding observers (RFC
goal 5). The limit exists to bound per-observer deriver fan-out for real peers;
a scope costs document rows, not LLM calls (RFC §5.2), so it does not belong in
that budget.
Excluded from both halves of the check in `_get_or_add_peers_to_session`: the
incoming names via a flag-based lookup, existing memberships via a correlated
NOT EXISTS on `scope_peer_clause()` — the same pattern the replacement and
removal paths already use, so the exclusion holds regardless of concurrent
scope creation. The early `count_observers_in_config(session.peer_names)` check
in `get_or_create_session` is left alone: `peer_names` cannot contain a scope,
and `scopes` is a separate field.
`reject_scope_peers` is split into a `scope_peer_names()` query helper plus a
two-line raiser so the observer count reuses the authoritative name-AND-flag
predicate instead of growing a third copy of it. Still costs nothing on the
common path — no reserved-prefix name in the input means no query at all.
Also caps `SessionCreate.scopes` at 100, matching `ScopeSessionsAdd.session_ids`.
This belongs in the same commit: the observer limit was the only thing bounding
that list, so removing it turns an unbounded `scopes` array into a peer row and
a membership row per element, committed — the single-request path to the
cardinality anti-pattern RFC §8 warns about. Partly answers OQ6: no per-session
cap, 100 per request.
Tests: a session joins SESSION_OBSERVERS_LIMIT + 2 scopes through both the
facade and session creation; real observers over the limit still 400, so the
carve-out cannot quietly disable the limit; 101 scopes is a 422.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): skip semantic retrieval when the embedding precompute failed
Addresses three open review comments.
1. Major — the representation read could embed inside its DB session. The
route's precompute is suppressed, and both
`RepresentationManager.get_working_representation` and
`crud.query_documents` fall back to embedding when a query arrives without
one, so a failed precompute meant an external call inside the read session
this branch opens for the scope re-check — the connection-holding rule the
route's own comment claimed to satisfy. The innermost fallback also only
catches ValueError, so a provider outage surfaced as a 500. The semantic
query is now passed only when an embedding exists, degrading to
derived+recent retrieval. (`crud.query_documents` embedding inside a caller's
session predates this branch and is left alone.)
2. Minor — `test_resolved_scope_peer_rejected_at_membership_upsert` described a
race it does not perform. It creates an already-flagged scope and calls crud
directly; the unflagged → flagged transition is not simulated. Docstring now
says what the test actually pins.
3. Minor — `test_empty_replacement_preserves_scope_membership` asserted only
half its docstring. It passed if the empty PUT left every ordinary
membership intact; now asserts the ordinary peer's left_at is set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): close auth and observed-position gaps, paginate membership
Review response for #884.
Security:
- gate `SessionCreate.scopes` behind a workspace-level key; the session-create
route is self-authorizing, so a peer- or session-scoped token could mint scope
peers and join sessions to scopes it had no access to via `POST /scopes`
- refuse a reserved-but-nonexistent name in two observed positions that used the
permissive guard: chat `target` and session-context `peer_target`. Both let a
caller act on `scope.X` before it existed, then create the scope
Facade:
- exclude scope peers from `GET /sessions/{id}/peers` and refuse the membership
-config read for a real scope, matching its write side
- replace `GET /scopes/{id}/sessions` with `POST /scopes/{id}/sessions/list`
returning `Page[Session]`; the add route now returns 204. Membership was
unbounded on both, while every other list surface paginates
- rename `crud.get_scope` to `get_scope_or_raise`
Tests:
- add a missing-name axis to the route-policy table (`Case.refuse_missing`), which
is what surfaced the two guard gaps above
- delete 14 hand-written tests the table now enumerates; 52 -> 39 functions in
test_scopes.py with more cases covered
- tighten the squatter assertion from `!= 422` to `< 400`, which was passing on 5xx
- assert the FastAPI-internals traversal still derives positions, so a framework
upgrade can't silently empty the suite
Docs:
- drop internal ticket and RFC references from the published OpenAPI descriptions
and surrounding comments; state the behavior instead
- move implementation reasoning out of the `PUT /peers/{id}` docstring, which
FastAPI publishes, into a comment
* test(scopes): assert exact statuses for permissive missing-name cases
Follow-up review pass on #884.
- add `Case.missing_status` so a permissive missing-name position asserts the
status it should actually get (404, or 200 for the no-op removal) instead of
`!= 422`, which also passed on a 5xx — the same hole already closed in the
squatter assertion
- require it whenever `refuse_missing` is False, and require its absence when
True, so the policy table can't drift from the assertion
- repoint a stale allow-reason at POST /scopes/{scope_id}/sessions/list; the GET
it named was removed
- document the membership list's ordering under `reverse`
* chore: clean up stale docstring language
* fix: don't backfill a session that left the scope
scope_backfill and scope_removal carry different work-unit keys, so
nothing orders them: a removal enqueued right after the add — or one
that lands while the backfill is embedding — sweeps the scope before
the copies exist, leaving a departed session's documents live in the
scope forever.
_run_backfill now re-checks SessionPeer membership inside the write
transaction and returns None; process_scope_backfill then skips both
the dream enqueues and the status write, so a skipped backfill can't
resurrect the status entry removal just cleared.
Adds coverage for the skip, the NULL-embedding re-embed path, and the
failed-status write. Handler-driven tests now stand up the membership
row the guard requires.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(filter): reject unknown operator dicts on scalar columns
An unrecognized operator dict on a non-JSONB column (e.g.
{"session_id": {"operator": "null"}}) fell through to `column == value`,
binding a dict to a VARCHAR parameter. That compiles, then fails in the
driver at execute time with "cannot adapt type 'dict'" — an unhandled
500 for what is invalid input.
Raise FilterError (422) instead. The guard lives in the shared
_build_field_condition, so every route through apply_filter is covered.
It keys on the actual column type rather than the JSONB_COLUMNS name
list, so dict equality still works on JSONB columns reachable through
Document's raw-key fallback (e.g. source_ids), where the driver adapts
dicts fine.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(filter): name psycopg explicitly in comment
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): handle null operands and non-numeric columns in comparisons
Two defects in _build_comparison_conditions, both reachable from any
route that accepts filters:
1. A null operand hit float(None), raising TypeError where only
ValueError was caught — an unhandled 500. A null operand is a null
check, not a value comparison, so {"ne": null} now compiles to
IS NOT NULL and the other operators reject null with a 422. Equality
against null already produced IS NULL via _build_field_condition.
2. Numeric operators float()-cast on every column type, so a string
inequality on a text column ({"session_id": {"ne": "abc"}}) was
rejected as an invalid number. Coercion is now gated on the column
actually being numeric; text columns compare as text. Numeric columns
still validate, and TypeError is caught alongside ValueError.
Existing ne coverage only exercised the JSONB metadata path, which uses
_safe_numeric_cast and handles strings — the scalar column path was
untested. Adds cases for both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): fail closed on unrecognized filter shapes
The filter body is arbitrary client JSON with no schema, so validation
was emergent: any shape the DSL didn't recognize surfaced as an
unhandled 500 from somewhere in SQLAlchemy or psycopg. Fixing individual
shapes doesn't converge — a fuzz over the DSL found five more families
beyond the three already fixed here:
{"AND": [None]} TypeError, non-dict in a logical list
{"AND": [[]]} AttributeError on .items()
{"session_id": {"gte": true}} SQLAlchemy ArgumentError
{"embedding": []} NotImplementedError, no python_type
{"session_id": {"ne": {...}}} execute-time "cannot adapt type 'dict'"
Two generic guards instead:
1. Any operand bound to a non-JSONB column must be a scalar, checked
element-wise for `in`. A dict or list bound to a scalar column
compiles cleanly and only fails in the driver at execute time, so it
has to be rejected during construction. JSONB columns are exempt —
a dict there is a containment match.
2. apply_filter fails closed: FilterError propagates, anything else is
logged with logger.exception (filter shape included) and re-raised as
FilterError. Unknown filter failures become 422s while staying fully
visible as errors rather than being swallowed.
Adds two invariant tests over a generated matrix of filter shapes: every
shape either compiles or raises FilterError, and no non-scalar is ever
bound to a scalar column. Both fail without the guards above. They cover
shapes nobody enumerated, so the next unimagined body fails in CI rather
than in production.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: clear the two remaining basedpyright warnings
`uv run basedpyright tests/ src/` reported two warnings in files that
predate this change. The pre-commit hook is file-scoped, so neither was
visible unless the owning file was touched.
- src/vector_store/__init__.py: join the lancedb error message with
explicit `+` instead of adjacent literals (reportImplicitStringConcatenation).
- tests/test_cache_redaction.py: the test covers a private helper
deliberately, so annotate the import (reportPrivateUsage).
No behavior change; whole-tree check is now clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): keep numeric operands exact instead of coercing to float
float() rounds any integer past 2**53 and flattens a Decimal, so
{"token_count": {"gt": 9007199254740993}} silently compared against
9007199254740992 — a different row set than the client asked for.
_coerce_numeric passes already-numeric operands through untouched and
only parses strings, trying int() before float() so "5" stays exact
while "5.5" still parses. bool narrows to int: it is an int subclass,
but binding it as a boolean against a numeric column produces SQL
Postgres has no operator for.
Not coerced to the column's own type: int(5.5) would turn
{"token_count": {"lt": 5.5}} into `lt 5`, changing which rows match.
Also fixes a vacuous assertion in test_dict_on_jsonb_column_still_works.
It checked for "internal_metadata" in the whole statement, but that name
is in the SELECT projection either way, so the test passed even when no
WHERE clause was applied. Now asserts on stmt.whereclause and that the
filter payload is actually bound.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): bind boolean columns as booleans, numerics without a cast
Boolean columns were treated as numeric because bool subclasses int, so
`{"is_active": {"ne": true}}` coerced true to 1 and Postgres rejected
`boolean <> integer` at execute time. Confirmed against a live database:
every form except bare equality with a native boolean was a 500.
Boolean columns now get their own branch: native true/false bind as
booleans, and any other operand is a FilterError. SQLAlchemy types the
bind from the operand rather than the column, so "true" renders
`is_active = %(param)s::VARCHAR` and Postgres has no such operator — a
422 is the honest answer. String booleans have never worked, are absent
from the docs (every documented boolean is inside metadata, which is
JSONB containment and unaffected), and produced no Sentry events in 90
days, so nothing can depend on the current behavior.
Also corrects the previous commit. Coercing operands to exact ints made
SQLAlchemy render an ::INTEGER cast, so any value past int4 — not 2**53
— started failing with "integer out of range" where float() had silently
compared as a double. Decimal keeps the value exact and renders no cast,
matching what float() did. The `in` branch never went through coercion
at all, so {"token_count": {"in": [1, 2147483648]}} was a 500 before
this PR too; it now takes the same path.
Verified end to end against the live database, not just at compile time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): coerce every operand against its column's type in one place
The DSL had two operand paths with different rules. Comparison operators
parsed datetimes and coerced numbers; bare equality bound whatever it was
handed. SQLAlchemy types a bind from the operand rather than the column
and the psycopg dialect renders that type as an explicit cast, so a
mismatch compiled into valid-looking SQL and failed at execute time:
"operator does not exist: timestamp with time zone = character varying".
A matrix of column type x operand type x operator against a live
database found 462 combinations, of which 55 built cleanly and then
failed. The most plausible was a filter someone would write first try:
{"created_at": "2026-01-01"} 500
{"created_at": {"gte": "2026-01-01"}} worked
_coerce_operand now handles every operand, whatever the operator, keyed
on the column's real type: JSONB takes an object, boolean takes only
true/false, datetime parses strings, numeric goes through _coerce_numeric,
text requires a string, and a column with no python_type (pgvector) is
not filterable. eq/ne/gt/in cannot drift apart because they share the
one call; `in` coerces element-wise, since a single element's type
decides the cast rendered for that parameter. The matrix is now clean.
This is a net deletion: the separate datetime, numeric, in-datetime and
boolean branches, plus _require_bindable_operand, all collapse into it.
Two more execute-time failures fixed on the way. `contains` was keyed on
column_name == "h_metadata", so Document's equally-JSONB
internal_metadata fell through to ILIKE and produced `jsonb ~~* text`;
it now keys on the column type. And {"source_ids": "abc"} was
`jsonb = character varying`.
Closed-set columns are validated against the Literal that defines them,
so declaring a new level or sync state updates filter validation with no
change here. {"level": "banana"} was silently matching nothing.
Empty IN is now always applied rather than skipped. Unifying the branches
inherited a guard that had only ever wrapped the datetime path, which
dropped the condition entirely and widened the query to every row —
fail-open on an empty allowlist, which session scoping relies on to fail
closed (see extract_session_allowlist). Caught by an existing test that
asserts returned rows; the fuzz and the type matrix only check for
errors, so neither would have seen it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): make NOT and ne include rows where the field is unset
NOT (col = v) and col <> v are NULL when col is NULL, so negation
dropped rows whose column is unset — a conclusion with no session is
not "some other session", but was excluded anyway. IS NOT TRUE and
IS DISTINCT FROM behave identically when no NULL is involved.
Adds a row-count test, since this class of bug builds and executes
cleanly, and documents {"ne": null} for excluding unset fields.
* fix(filter): reject session ids that can't name a real session
extract_session_allowlist accepted any non-empty string, so "*" reached
the three consumers of the allowlist — direct IN, the filter DSL, and a
Python membership test — which disagree about it. The DSL reads "*" as
"drop the condition" and matches every session; the others treat it as a
literal name and match none. One /chat request could have some recall
sources unscoped and others scoped to nothing.
Entries are now validated against RESOURCE_NAME_PATTERN, the same pattern
the API requires of session ids, so no session could ever be named "*"
anyway. Wildcards were never part of this endpoint's documented contract
(an id, a list of ids, or {"in": [...]}), and a wildcard alongside a
top-level session_id already 422'd via must_include.
* fix(filter): treat a bare null operand as a null check
Routing every operand through _coerce_operand made bare `None` a type
error rather than a null check, so {"session_id": null} raised FilterError
where it previously built IS NULL: _build_field_condition used to end in
`column == value`, which SQLAlchemy renders as IS NULL. Confirmed 422 on
all five column families (text, numeric, boolean, datetime, JSONB).
Nothing caught it. The docs added in this branch promise
`{"session_id": None}` matches unset rows, the comment in
_build_comparison_conditions claimed the equality path already covered it,
and the DSL-wide invariant test accepts "compiles OR raises FilterError",
so a 422 passed. _coerce_operand's docstring already stated the contract
its caller wasn't honoring — "Callers handle None (a null check) and `*`
(a wildcard) before calling" — so the guard restores that rather than
adding a new rule.
The three null forms now agree: {"col": null} is IS NULL, {"col": {"ne":
null}} is IS NOT NULL, NOT [{"col": null}] is (IS NULL) IS NOT true.
Also from review of #947:
- Log filter keys, not the body. That log line is new in this branch and
operands carry peer/session ids and free-text `contains` values; the
traceback plus the entry shape is what locates a builder bug.
- Assert whereclause in the _where test helper, so a dropped condition
fails instead of returning the whole statement to substring-match.
- Cover the raw-key JSONB path via source_ids, a JSONB column outside
JSONB_COLUMNS reachable through Document's raw-key fallback.
- Drop the orphaned comment left above ENUM_COLUMN_VALUES when
_coerce_operand replaced SCALAR_OPERAND_TYPES.
- Document that a JSONB column takes an object bare or under `contains`
and nothing else. Bare {"metadata": X} is containment, so the `ne` this
branch removed was never its inverse: a row with {"status":"done","x":1}
satisfied both it and {"metadata": {"status":"done"}}. Per-key operators
and NOT cover the real intents.
- Rewrite "Negation and Unset Fields" to lead with the operator rule and a
truth table, forward-linking to Filtering Conclusions instead of using
conclusions ~525 lines before they are introduced. A conclusion's
session_id is the only nullable documented filterable field, verified
across Message/Document/Session/Peer/Workspace.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Import anthropic/openai/google-genai only when a provider is first used
instead of at module import. CLIENTS is now populated lazily via
default_client(), which preserves the patch.dict test seam. The
embedding client defers its SDK imports the same way and dispatches on
transport instead of isinstance.
Cuts idle RSS by ~60MiB per process with all three providers configured
but unused at startup; a process that only ever calls one provider also
never pays for the other two.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: apply session scoping to all working-representation query paths
session_name was only applied to the recent-documents query in
RepresentationManager; the semantic and most-derived paths ignored it,
so limit_to_session leaked cross-session conclusions into perspectives.
- Thread a session allowlist (session_names) uniformly through all
three query paths; pushed down to pgvector and external vector stores
- Accept a list so the upcoming session-allowlist API reuses this path
- Fail closed on an empty allowlist (downstream stores drop empty IN
clauses, which would silently widen scope)
Fixes DEV-1994
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: bare-list membership sugar in the filter DSL
{"session_id": ["s1", "s2"]} is now shorthand for
{"session_id": {"in": [...]}} on regular columns, generically
(peer_id, etc.). JSONB metadata columns are excluded — a bare list
there keeps JSONB containment semantics, unchanged.
Previously a bare list on a regular column compiled to a type-mismatched
equality that matched nothing, so this is strictly additive.
Also translates the same shape in the turbopuffer/lancedb filter
builders, and fixes lancedb dropping empty IN clauses (fail-open) —
an empty membership list now emits an always-false condition.
Groundwork for DEV-1995 (session allowlist via the existing filters
DSL, no new API params)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: session allowlist on dialectic and representation via filters
Adds a constrained 'filters' body to peer.chat and /representation —
the same DSL search and conclusions already accept, supporting only the
session_id key (a session id, a bare list, or {"in": [...]}).
Unsupported keys and shapes are rejected with 422, never silently
ignored. Composes with session_id (must be included in the allowlist
when both are given). Capped at 1,000 sessions per request.
Enforcement is uniform at every recall chokepoint, fail-closed:
- dialectic prefetch + search_memory: conclusion recall restricted to
the allowlist; dream docs (session_name IS NULL) excluded
- message tools (search/grep/date-range/temporal/context/history):
strict intersection of allowlist and observer session membership
- get_reasoning_chain: unavailable under an allowlist (chains traverse
provenance across sessions and cannot be scoped without leaking)
- empty allowlist short-circuits to empty results everywhere
Auth: workspace keys pass the allowlist as-given; peer-scoped JWTs must
be a member of every allowlisted session (403 otherwise), mirroring the
existing single-session check.
Fixes DEV-1995
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: reserve scope__ peer namespace with kind flag and guardrails
Introduce the scope peer namespace (scope__<name>) and the authoritative
{"kind": "scope"} configuration flag, plus the server-side guardrails:
- src/utils/scopes.py: single source of truth for the prefix, kind flag,
and name helpers (scope_peer_name / is_scope_peer_name /
scope_name_from_peer / validate_no_scope_peer_names)
- reject reserved-prefix names on peer get-or-create (422)
- reject scope peers as message authors in crud.create_messages (422)
- reject scope peers as chat/representation targets (422); a scope peer
as the path-level observer is deferred to Phase 2b
- reject scope peers on the generic session-peer add/set/remove routes
and the session-create peers mapping (422, directing to scopes routes)
- peers.list excludes scope peers by default; new PeerGet.kind option
("scope" | "all") switches the view via a configuration JSONB filter
- schemas: Scope / ScopeCreate / ScopeSessions(Add) and
SessionCreate.scopes (unprefixed scope names, validated)
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add scopes CRUD routes and session-create scopes wiring
New /v3/workspaces/{workspace_id}/scopes facade (workspace-level auth;
peer- and session-scoped keys are rejected):
POST "" create-or-get (201/200)
POST /list paginated scope list
GET /{scope_id} single scope
POST /{scope_id}/sessions add memberships
DELETE /{scope_id}/sessions/{session_id} remove membership
GET /{scope_id}/sessions list member session ids
- crud/scope.py: get_or_create_scopes stamps the backing peer with
{"kind": "scope", "observe_me": false} and refuses to adopt a
legacy peer occupying the reserved name without the flag (409)
- memberships are session_peers rows with observe_others=true /
observe_me=false — identical to a hand-built observer peer
- SessionCreate.scopes: create-or-get each scope peer and add the
membership at session creation (the no-backfill common path)
- crud/session.py: public upsert_session_peers wrapper so the facade
bypasses the route-level guardrails without reaching into privates
Backfill of pre-existing documents and reconciliation on removal land in
DEV-1999; membership only affects messages ingested after the change.
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: cover scopes facade, guardrails, and observer semantics
- create-or-get idempotency, list/get, name validation, legacy-collision
rejection (409), auth scoping (workspace key ok, peer/session keys 401)
- reserved prefix rejected on peer create; peers.list kind filtering
- scope peers rejected as message authors, chat/representation targets,
and on the generic session-peer routes
- membership add/list/remove with observe_others=true / observe_me=false
row shape asserted via DB, and facade-less equivalence with a
hand-built observer peer
- end-to-end litmus: after adding a session to a scope, the deriver
enqueue fan-out includes the scope peer as an observer
- session creation with scopes: [a, b] creates both memberships
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(scopes): scope resolution helper and read-route schemas
Add resolve_scope_peers (crud/scope.py) mapping unprefixed scope names to
their backing scope peers, 404 when missing and 422 when a non-scope peer
squats the reserved name. Add the `scope` option to DialecticOptions and
PeerRepresentationGet, and a WorkspaceMessageSearchOptions with `scope`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(scopes): wire the `scope` read option into the routes
Chat and representation: a single scope swaps the observer to the scope
peer (recall confined to the scoped collection and the scope's sessions by
existing observer semantics); a scope list resolves to the union of member
sessions and rides the DEV-1995 dynamic allowlist arm with the path peer as
observer. `scope` is mutually exclusive with `filters`/`session_id` (422)
and requires a workspace/admin key (403 for peer-scoped JWTs).
Session context: `scope` swaps the perspective source for both the working
representation and the peer-card fetch. Workspace search: `scope` injects
the scope's session set into the message-search filter (empty scope -> no
results). Close the carry-over guardrail gap: scope peers are rejected as
peer_target/peer_perspective in session context and as the peer/target in
GET /peers/{id}/context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(scopes): cover the `scope` read option end-to-end
Validation (404/422/403), single-scope observer swap vs. union allowlist,
scoped representation and session-context (scoped collection + scoped card),
workspace search restricted to a scope's sessions, and guardrail closure for
scope peers on the peer/session context surfaces. Patch the workspaces
tracked_db import site so scope resolution reads the per-test database.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(crud): preserve cache invalidation across get_or_create retry
`get_or_create_peers` and `get_or_create_scopes` mutate existing rows, then
insert new ones inside `db.begin_nested()`. When a concurrent writer creates
one of those rows first, the insert raises IntegrityError and the function
retries.
`begin_nested()` autoflushes the pending UPDATEs *before* opening the
savepoint, so the rollback neither undoes them nor expires the now-clean ORM
state. The retry then compared already-updated values, found no change, and
dropped those peers from `changed_peers` — skipping the cache purge while the
row change committed anyway, leaving entries stale until the 300s TTL.
Carry the mutated names into the retry via `_pending_invalidation` so the
purge cannot be lost.
The scopes facade mirrors `get_or_create_peers`, so both copies carried this.
The peer path is pre-existing and runs on every message ingest.
Also add the missing /v3 prefix to `_SCOPES_ROUTE_GUIDANCE`, which pointed
callers at a 404.
Adds tests/crud/test_get_or_create_retry_invalidation.py, which drives a real
racing session and fails without this change.
* fix(scopes): make scope identity unforgeable, unblock non-pattern peer names
Three coupled changes to the scopes facade.
1. `PeerCreate` no longer gates internal lookups. It exists to validate a new,
user-supplied peer id at the API boundary, but crud used it as a DTO for
names that already exist, so any name outside RESOURCE_NAME_PATTERN raised a
raw pydantic ValidationError — which is not a HonchoException, so it fell
through to the catch-all handler as an HTTP 500. Adds `PeerSpec` (same
fields, no charset pattern) as `PeerCreate`'s base, widens
`get_or_create_peers` to accept it, and changes `get_peer` to take a plain
str. All 13 construction sites converted; the create route keeps full
validation.
This unbreaks the Dreamer: DreamScheduler passes `collection.observer`
straight into the specialist preflight, and scope peers have
`observe_others=true`, so every `(scope.x, peer)` dream died there — the
feature scopes exist to enable. It also fixes a pre-existing bug unrelated
to scopes: a peer named `alice.smith` (legal before d429de0e5338, which
validated names by length alone) 500s on message create, session peer add,
and peer update.
2. The `kind` flag moves from `configuration` to `internal_metadata`.
`configuration` is user-writable — `PeerCreate`/`PeerUpdate` accept a
free-form dict and `update_peer` replaces it wholesale — so a legitimate
`{"observe_me": true}` update silently dropped the flag, and a forged
`{"kind": "scope"}` injected an ordinary peer into `POST /scopes/list`.
`internal_metadata` appears in no API schema. `observe_me: false` stays in
`configuration`, where it belongs.
3. Scope identity requires prefix AND flag, via `is_scope_peer()` and
`scope_peer_clause()`. Neither half is forgeable: the prefix sits outside
RESOURCE_NAME_PATTERN, `internal_metadata` is unreachable. Usage-site guards
become flag-based so a legacy peer merely occupying the namespace keeps
working rather than 422-ing on its own traffic; peer create and update stay
name-based, since those must stop new names entering the namespace.
`update_peer` now returns 422 instead of 500.
Also swaps the reserved prefix from `scope__` to `scope.`: `_` is inside
RESOURCE_NAME_PATTERN, so any tenant could already own a `scope__x` peer.
No DB migration — `internal_metadata` already exists on `peers`, and no scope
peers exist in any deployment yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): validate peer names on create, close namespace squatting and upsert race
Addresses three review findings against 48047a6a.
1. `PeerSpec` let API callers create invalid and reserved-prefix peers.
Widening `get_or_create_peers` to accept a pattern-free schema fixed the
lookup 500s but also removed validation from the *insert* path, and
request-controlled names reach it via message authors, session peer maps, and
the chat observer path — none of which carry a charset pattern of their own.
Confirmed: `POST /sessions/{id}/messages` with `peer_id: "scope.x"` returned
201 and minted an unflagged squatter, after which `POST /scopes {id: x}` was
permanently 409-blocked — namespace denial of service by any caller able to
post a message. `peer_id: "not a valid name!@#"` was likewise created.
Fixed by validating only names about to be INSERTed
(`_validate_new_peer_names`), so already-existing names — legacy dotted
names, scope peers — still resolve without a spurious 422. That keeps the
Dreamer fix intact, since it reads through `get_peer`.
2. Existing reserved-prefix squatters could not be updated. The name-based guard
on `PUT /peers/{peer_id}` refused every `scope.` name, contradicting the
invariant that an unflagged squatter stays a normal peer. Now flag-based, so
behavior is three-way: a real scope is refused, an existing unflagged peer
updates, and a missing reserved-prefix name is refused by (1) rather than
minted.
3. Scope checks raced with get-or-create and the membership upsert. The
route-level guards run before peers are resolved, so a scope created
concurrently in that window would be attached by the generic path with a
default `SessionPeerConfig()`, clobbering its observer membership config.
Adds `_reject_resolved_scope_peers`, which runs on the resolved rows in the
same transaction as the upsert — no window, no extra query. The early checks
stay for better error messages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): guard scope membership config, move checks to the mutation point
Addresses a second review pass against 10655792.
1. Scope membership configuration was directly user-mutable.
`PUT /sessions/{id}/peers/{peer_id}/config` had no scope guard at all, and
`crud.set_peer_config` resolved the peer only to discard the row. Confirmed:
posting `{"observe_others": false, "observe_me": true}` for a scope returned
204 and persisted, which silently stops all fan-out into the scope and makes
Honcho form a representation *of* a scope — neither of which is reachable
through the facade. Deterministic, no race required. Now checked on the row
`get_peer` already returns, so it costs nothing and cannot race.
2. Empty and over-long names were still 500s. Removing the charset pattern from
`PeerSpec` fixed one trap but left its length bounds, and request-bound peer
names carry no length limits of their own — so `peer_id: ""` or a 513-char
name reached `PeerSpec(...)` and raised a raw pydantic ValidationError that
the catch-all turned into a 500. `PeerSpec` now carries no constraints at all
(matching its documented purpose) and every rule for a new name lives in
`_validate_new_peer_names` on the insert path.
3. Resolved-row protection generalized. The previous pass applied it only to
membership upserts, leaving check-then-use windows elsewhere: peer update
could have a concurrently-created scope's configuration replaced wholesale
(create-path validation does not fire for a peer that now exists), the chat
observer get-or-create could resolve a fresh scope as its observer, and the
generic session-peer removal could silently detach a scope from its sessions.
Each now inspects the resolved peer immediately before acting; the redundant
name-level guard on the update route is dropped in favor of the race-free one.
`remove_peers_from_session` grows an internal `_allow_scope_peers` flag because
the scopes facade ends membership through that same path and must not be blocked
by its own guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(scopes): enumerate every peer-touching route against a scope policy
Three review passes each found the same class of defect: a route nobody had
checked, rather than logic that was subtly wrong. One of them — `PUT
/sessions/{id}/peers/{id}/config`, which let any caller set a scope to
`observe_others=false` and silently stop all fan-out into it — predated this work
entirely, because the guardrail set was assembled guardrail-by-guardrail instead
of derived from the route list. Sampling review cannot close that kind of gap;
enumeration can.
Derives every route through which a peer name can reach the system and requires
each to be classified as GUARDED or EXEMPT-with-a-reason, so a newly added
peer-touching route fails the suite until someone classifies it. Detection is the
union of path shape and a walk of the dependant tree (including sub-dependency
`Form(...)` params and nested request-body models), because neither signal alone
suffices: parameter names miss `POST /sessions/{id}/peers`, whose peer names are
dict keys, and path shape misses `messages/upload`, whose `peer_id` arrives as a
form field behind a parser dependency.
Both invariants are then asserted behaviorally, by calling the routes rather than
inspecting annotations — the guards deliberately live in crud, which is what makes
`messages/upload` guarded for free via `crud.create_messages`:
- a real scope is refused on all 11 guarded routes, and the rejection must name
the scope, so an unrelated 422 (a malformed body) cannot pass the assertion;
- an *unflagged* peer merely occupying the reserved namespace is unaffected. That
half regressed once already when `update_peer` used a name-based check.
Mutation-tested all three failure modes: disabling the `set_peer_config` guard
fails the guarded test naming that route; regressing `update_peer` to name-based
fails the squatter test; adding an unclassified peer route fails the enumeration.
Covers the HTTP surface only. Peer names also reach the system through the
deriver, dreamer, and queue, which have no route table to enumerate — noted in
the module docstring rather than implied to be covered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse a scope in every observed position; enumerate per position
Addresses a fourth review pass against 62681e4d. The headline finding is that the
previous commit's enumeration test had the wrong *model*, not a missing entry.
1. Manual conclusions could create knowledge about a scope. `POST /conclusions`
validated only that observer_id and observed_id exist, so a scope as
`observed_id` persisted a conclusion about a peer carrying observe_me=false and
created an (observer, scope) collection for it. Confirmed: 201, and it read
back. `POST /schedule_dream` had the same hole via `observed`.
The fix is positional, because the invariant is:
A scope may be an OBSERVER. A scope may never be OBSERVED.
A scope as `observer_id` is how scoped conclusions are stored and must keep
working (verified still 201); as `observed_id` it is now refused. Same split
applied to schedule_dream `observed`, the peer-card `target` (which also covers
a scope's self-card, since target-omitted collapses observed to peer_id), and
session-context `peer_target`.
2. Chat target and both representation roles kept check-to-use races. Only the
chat path-level observer was re-checked on its resolved row; the target was
checked by name and then resolved without inspecting scope identity. Both are
now checked at the dialectic preflight, where observer and observed are already
resolved — an absent name has already failed by then, and an existing squatter
cannot retroactively become a scope.
3. Generic membership removal was still racy. The adjacent SELECT narrowed the
window but could not close it under READ COMMITTED. The UPDATE now carries its
own correlated NOT EXISTS against scope_peer_clause(), so Postgres evaluates
the exclusion as part of the statement and a scope committed after the advisory
check still cannot be detached.
4. New-name validation ran after the name reached Postgres. A NUL byte passed the
request schemas and PeerSpec, then raised psycopg.DataError inside the lookup —
a 500. Values that cannot correspond to a stored row by construction (NUL
bytes, over-length names) are now refused before the query.
(Over-length names already returned 422; only the wasted query was real there.)
The enumeration test is rekeyed from (method, path) to (method, path, position).
A binary per-route verdict cannot express finding 1 at all: `POST /conclusions` is
one route with two positions and opposite verdicts. Detection widens to observer /
observed / target / peer_target / peer_perspective, which surfaced four routes the
previous version never saw — conclusions, schedule_dream, queue/status, and
session context.
Also registers `src.routers.workspaces.tracked_db` in the conftest patch list; the
new guard there would otherwise have run against the real configured database
instead of the per-test one.
Mutation-tested: disabling the conclusions observed-guard fails the positional
test naming that position; adding an unclassified `observed_id` param fails
enumeration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse future scopes in observed positions, preserve scope membership
Addresses a fifth review pass against 14136e5b. All six findings reproduced
locally before fixing.
1. High — generic peer replacement removed scope memberships.
`set_peers_for_session` soft-deleted every active SessionPeer row, and the
request-level guard only inspected names *present* in the replacement map. A
caller detached a scope by simply omitting it, never naming it — so no
request-level guard could ever see it. Reproduced: scope sessions went
['<id>'] -> [] on a 200. The exclusion now lives in the UPDATE itself
(correlated NOT EXISTS against scope_peer_clause), so replacement means
"replace ordinary peers" regardless of request contents or concurrent creation.
2. High — peer cards could be pre-seeded for future scopes.
`set_peer_card` resolves only the observer and writes a JSONB key derived from
an unchecked observed name, and the route guard rejected only *existing*
flagged scopes. Reproduced: PUT card with target=scope.<missing> returned 200,
creating that scope then returned 201, and the card described the real scope.
3. High — dreams could be queued for future scopes.
The route checked `observed` in a read-only session that closed before
`enqueue_dream`, and a missing reserved name passes any is-it-a-scope check.
Reproduced: 204 with observed=scope.<missing>.
2 and 3 share a root cause, so they share a fix: a new `reject_scope_observed`
that is stricter than `reject_scope_peers` in exactly one case — a *missing*
reserved name is refused, because nothing on these paths creates the peer, so
nothing else would ever catch it. Existing unflagged squatters still pass.
Both guards moved to the mutation point: card validation into
`crud.set_peer_card` (same transaction as the JSONB write, so Dreamer and
agent-tool callers are covered), dream validation into `enqueue_dream` (same
transaction as the queue insert). The redundant route-level checks are dropped
rather than left as weaker duplicates.
4. Medium — prefixed NUL names still reached PostgreSQL.
`reject_scope_peers` filtered for the reserved prefix and sent matches to a
text comparison, so "scope.future\0name" raised psycopg.DataError — a 500.
Both guards now share `_reserved_name_candidates`, which materializes the input
once and rejects impossible values before any SQL. Materializing matters
independently: the message-author path passes a generator, and validation
iterates separately from the prefix filter, so a generator would be
half-consumed. `_reject_impossible_peer_names` now takes a Collection so the
type checker enforces that.
5. Medium — representation kept a check-to-use race.
The previous commit claimed both representation roles were rechecked after
resolution; that was wrong — only the dialectic preflight got that check, and
the representation route never goes through it. It now opens one short
read-only session *after* the embedding call, checks both positions, and passes
that same session to `get_working_representation`, so no connection is held
across external work and a scope committed later cannot have conclusions in the
collection being read.
6. Low — policy coverage was not exhaustive. `sender_id` reaches CRUD as
`observed` but was missing from the detected parameter set. ALLOW cases could
also not carry builders, so the suite never proved the other half of the
contract — that legitimate scope *observers* keep working, which a guard
rejecting scopes everywhere would satisfy. Both fixed; observer positions on
conclusions, dreams, cards, session context and queue status are now asserted
behaviorally.
Deliberately not implemented: the scope-creation backstop scanning for
pre-existing card keys and queue items naming a future backing peer. Reasoning is
recorded in `get_or_create_scopes` — no new such state can be created now, any
pre-existing row is coincidental since `scope.` was never a meaningful namespace,
the consequence is inert, and detecting card keys means a full table scan per
scope creation.
Mutation-tested each new guard: removing the replacement exclusion fails both
membership-preservation tests; weakening either observed guard to existing-only
fails the pre-seeding tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): exclude scope memberships from the session observer limit
Scope memberships carry observe_others=true, so every scope counted against
SESSION_OBSERVERS_LIMIT (default 10) — capping scopes-per-session at the limit
minus the session's real observers, and reporting the failure as
`400 Cannot create session <name> with 11 observers. ... Observers are peers
with 'observe_others' set to true.` on a membership call. Wrong on three counts:
the ceiling is undocumented and contradicts RFC §5.1 ("sessions belong to any
number of scopes"), the message describes session creation, and it leaks the
word "observer" through a facade whose entire job is hiding observers (RFC
goal 5). The limit exists to bound per-observer deriver fan-out for real peers;
a scope costs document rows, not LLM calls (RFC §5.2), so it does not belong in
that budget.
Excluded from both halves of the check in `_get_or_add_peers_to_session`: the
incoming names via a flag-based lookup, existing memberships via a correlated
NOT EXISTS on `scope_peer_clause()` — the same pattern the replacement and
removal paths already use, so the exclusion holds regardless of concurrent
scope creation. The early `count_observers_in_config(session.peer_names)` check
in `get_or_create_session` is left alone: `peer_names` cannot contain a scope,
and `scopes` is a separate field.
`reject_scope_peers` is split into a `scope_peer_names()` query helper plus a
two-line raiser so the observer count reuses the authoritative name-AND-flag
predicate instead of growing a third copy of it. Still costs nothing on the
common path — no reserved-prefix name in the input means no query at all.
Also caps `SessionCreate.scopes` at 100, matching `ScopeSessionsAdd.session_ids`.
This belongs in the same commit: the observer limit was the only thing bounding
that list, so removing it turns an unbounded `scopes` array into a peer row and
a membership row per element, committed — the single-request path to the
cardinality anti-pattern RFC §8 warns about. Partly answers OQ6: no per-session
cap, 100 per request.
Tests: a session joins SESSION_OBSERVERS_LIMIT + 2 scopes through both the
facade and session creation; real observers over the limit still 400, so the
carve-out cannot quietly disable the limit; 101 scopes is a 422.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): skip semantic retrieval when the embedding precompute failed
Addresses three open review comments.
1. Major — the representation read could embed inside its DB session. The
route's precompute is suppressed, and both
`RepresentationManager.get_working_representation` and
`crud.query_documents` fall back to embedding when a query arrives without
one, so a failed precompute meant an external call inside the read session
this branch opens for the scope re-check — the connection-holding rule the
route's own comment claimed to satisfy. The innermost fallback also only
catches ValueError, so a provider outage surfaced as a 500. The semantic
query is now passed only when an embedding exists, degrading to
derived+recent retrieval. (`crud.query_documents` embedding inside a caller's
session predates this branch and is left alone.)
2. Minor — `test_resolved_scope_peer_rejected_at_membership_upsert` described a
race it does not perform. It creates an already-flagged scope and calls crud
directly; the unflagged → flagged transition is not simulated. Docstring now
says what the test actually pins.
3. Minor — `test_empty_replacement_preserves_scope_membership` asserted only
half its docstring. It passed if the empty PUT left every ordinary
membership intact; now asserts the ordinary peer's left_at is set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): close auth and observed-position gaps, paginate membership
Review response for #884.
Security:
- gate `SessionCreate.scopes` behind a workspace-level key; the session-create
route is self-authorizing, so a peer- or session-scoped token could mint scope
peers and join sessions to scopes it had no access to via `POST /scopes`
- refuse a reserved-but-nonexistent name in two observed positions that used the
permissive guard: chat `target` and session-context `peer_target`. Both let a
caller act on `scope.X` before it existed, then create the scope
Facade:
- exclude scope peers from `GET /sessions/{id}/peers` and refuse the membership
-config read for a real scope, matching its write side
- replace `GET /scopes/{id}/sessions` with `POST /scopes/{id}/sessions/list`
returning `Page[Session]`; the add route now returns 204. Membership was
unbounded on both, while every other list surface paginates
- rename `crud.get_scope` to `get_scope_or_raise`
Tests:
- add a missing-name axis to the route-policy table (`Case.refuse_missing`), which
is what surfaced the two guard gaps above
- delete 14 hand-written tests the table now enumerates; 52 -> 39 functions in
test_scopes.py with more cases covered
- tighten the squatter assertion from `!= 422` to `< 400`, which was passing on 5xx
- assert the FastAPI-internals traversal still derives positions, so a framework
upgrade can't silently empty the suite
Docs:
- drop internal ticket and RFC references from the published OpenAPI descriptions
and surrounding comments; state the behavior instead
- move implementation reasoning out of the `PUT /peers/{id}` docstring, which
FastAPI publishes, into a comment
* test(scopes): assert exact statuses for permissive missing-name cases
Follow-up review pass on #884.
- add `Case.missing_status` so a permissive missing-name position asserts the
status it should actually get (404, or 200 for the no-op removal) instead of
`!= 422`, which also passed on a 5xx — the same hole already closed in the
squatter assertion
- require it whenever `refuse_missing` is False, and require its absence when
True, so the policy table can't drift from the assertion
- repoint a stale allow-reason at POST /scopes/{scope_id}/sessions/list; the GET
it named was removed
- document the membership list's ordering under `reverse`
* fix: Address Coderabbit Comments
* fix: Address more coderabbit comments
* chore: remove ai artifacting
* fix: remove unused fakeredis client in lieu of in-memory taskless cache for test suite
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat: reserve scope__ peer namespace with kind flag and guardrails
Introduce the scope peer namespace (scope__<name>) and the authoritative
{"kind": "scope"} configuration flag, plus the server-side guardrails:
- src/utils/scopes.py: single source of truth for the prefix, kind flag,
and name helpers (scope_peer_name / is_scope_peer_name /
scope_name_from_peer / validate_no_scope_peer_names)
- reject reserved-prefix names on peer get-or-create (422)
- reject scope peers as message authors in crud.create_messages (422)
- reject scope peers as chat/representation targets (422); a scope peer
as the path-level observer is deferred to Phase 2b
- reject scope peers on the generic session-peer add/set/remove routes
and the session-create peers mapping (422, directing to scopes routes)
- peers.list excludes scope peers by default; new PeerGet.kind option
("scope" | "all") switches the view via a configuration JSONB filter
- schemas: Scope / ScopeCreate / ScopeSessions(Add) and
SessionCreate.scopes (unprefixed scope names, validated)
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add scopes CRUD routes and session-create scopes wiring
New /v3/workspaces/{workspace_id}/scopes facade (workspace-level auth;
peer- and session-scoped keys are rejected):
POST "" create-or-get (201/200)
POST /list paginated scope list
GET /{scope_id} single scope
POST /{scope_id}/sessions add memberships
DELETE /{scope_id}/sessions/{session_id} remove membership
GET /{scope_id}/sessions list member session ids
- crud/scope.py: get_or_create_scopes stamps the backing peer with
{"kind": "scope", "observe_me": false} and refuses to adopt a
legacy peer occupying the reserved name without the flag (409)
- memberships are session_peers rows with observe_others=true /
observe_me=false — identical to a hand-built observer peer
- SessionCreate.scopes: create-or-get each scope peer and add the
membership at session creation (the no-backfill common path)
- crud/session.py: public upsert_session_peers wrapper so the facade
bypasses the route-level guardrails without reaching into privates
Backfill of pre-existing documents and reconciliation on removal land in
DEV-1999; membership only affects messages ingested after the change.
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: cover scopes facade, guardrails, and observer semantics
- create-or-get idempotency, list/get, name validation, legacy-collision
rejection (409), auth scoping (workspace key ok, peer/session keys 401)
- reserved prefix rejected on peer create; peers.list kind filtering
- scope peers rejected as message authors, chat/representation targets,
and on the generic session-peer routes
- membership add/list/remove with observe_others=true / observe_me=false
row shape asserted via DB, and facade-less equivalence with a
hand-built observer peer
- end-to-end litmus: after adding a session to a scope, the deriver
enqueue fan-out includes the scope peer as an observer
- session creation with scopes: [a, b] creates both memberships
Part of DEV-1997 (Scopes RFC DEV-1970).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(crud): preserve cache invalidation across get_or_create retry
`get_or_create_peers` and `get_or_create_scopes` mutate existing rows, then
insert new ones inside `db.begin_nested()`. When a concurrent writer creates
one of those rows first, the insert raises IntegrityError and the function
retries.
`begin_nested()` autoflushes the pending UPDATEs *before* opening the
savepoint, so the rollback neither undoes them nor expires the now-clean ORM
state. The retry then compared already-updated values, found no change, and
dropped those peers from `changed_peers` — skipping the cache purge while the
row change committed anyway, leaving entries stale until the 300s TTL.
Carry the mutated names into the retry via `_pending_invalidation` so the
purge cannot be lost.
The scopes facade mirrors `get_or_create_peers`, so both copies carried this.
The peer path is pre-existing and runs on every message ingest.
Also add the missing /v3 prefix to `_SCOPES_ROUTE_GUIDANCE`, which pointed
callers at a 404.
Adds tests/crud/test_get_or_create_retry_invalidation.py, which drives a real
racing session and fails without this change.
* fix(scopes): make scope identity unforgeable, unblock non-pattern peer names
Three coupled changes to the scopes facade.
1. `PeerCreate` no longer gates internal lookups. It exists to validate a new,
user-supplied peer id at the API boundary, but crud used it as a DTO for
names that already exist, so any name outside RESOURCE_NAME_PATTERN raised a
raw pydantic ValidationError — which is not a HonchoException, so it fell
through to the catch-all handler as an HTTP 500. Adds `PeerSpec` (same
fields, no charset pattern) as `PeerCreate`'s base, widens
`get_or_create_peers` to accept it, and changes `get_peer` to take a plain
str. All 13 construction sites converted; the create route keeps full
validation.
This unbreaks the Dreamer: DreamScheduler passes `collection.observer`
straight into the specialist preflight, and scope peers have
`observe_others=true`, so every `(scope.x, peer)` dream died there — the
feature scopes exist to enable. It also fixes a pre-existing bug unrelated
to scopes: a peer named `alice.smith` (legal before d429de0e5338, which
validated names by length alone) 500s on message create, session peer add,
and peer update.
2. The `kind` flag moves from `configuration` to `internal_metadata`.
`configuration` is user-writable — `PeerCreate`/`PeerUpdate` accept a
free-form dict and `update_peer` replaces it wholesale — so a legitimate
`{"observe_me": true}` update silently dropped the flag, and a forged
`{"kind": "scope"}` injected an ordinary peer into `POST /scopes/list`.
`internal_metadata` appears in no API schema. `observe_me: false` stays in
`configuration`, where it belongs.
3. Scope identity requires prefix AND flag, via `is_scope_peer()` and
`scope_peer_clause()`. Neither half is forgeable: the prefix sits outside
RESOURCE_NAME_PATTERN, `internal_metadata` is unreachable. Usage-site guards
become flag-based so a legacy peer merely occupying the namespace keeps
working rather than 422-ing on its own traffic; peer create and update stay
name-based, since those must stop new names entering the namespace.
`update_peer` now returns 422 instead of 500.
Also swaps the reserved prefix from `scope__` to `scope.`: `_` is inside
RESOURCE_NAME_PATTERN, so any tenant could already own a `scope__x` peer.
No DB migration — `internal_metadata` already exists on `peers`, and no scope
peers exist in any deployment yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): validate peer names on create, close namespace squatting and upsert race
Addresses three review findings against 48047a6a.
1. `PeerSpec` let API callers create invalid and reserved-prefix peers.
Widening `get_or_create_peers` to accept a pattern-free schema fixed the
lookup 500s but also removed validation from the *insert* path, and
request-controlled names reach it via message authors, session peer maps, and
the chat observer path — none of which carry a charset pattern of their own.
Confirmed: `POST /sessions/{id}/messages` with `peer_id: "scope.x"` returned
201 and minted an unflagged squatter, after which `POST /scopes {id: x}` was
permanently 409-blocked — namespace denial of service by any caller able to
post a message. `peer_id: "not a valid name!@#"` was likewise created.
Fixed by validating only names about to be INSERTed
(`_validate_new_peer_names`), so already-existing names — legacy dotted
names, scope peers — still resolve without a spurious 422. That keeps the
Dreamer fix intact, since it reads through `get_peer`.
2. Existing reserved-prefix squatters could not be updated. The name-based guard
on `PUT /peers/{peer_id}` refused every `scope.` name, contradicting the
invariant that an unflagged squatter stays a normal peer. Now flag-based, so
behavior is three-way: a real scope is refused, an existing unflagged peer
updates, and a missing reserved-prefix name is refused by (1) rather than
minted.
3. Scope checks raced with get-or-create and the membership upsert. The
route-level guards run before peers are resolved, so a scope created
concurrently in that window would be attached by the generic path with a
default `SessionPeerConfig()`, clobbering its observer membership config.
Adds `_reject_resolved_scope_peers`, which runs on the resolved rows in the
same transaction as the upsert — no window, no extra query. The early checks
stay for better error messages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): guard scope membership config, move checks to the mutation point
Addresses a second review pass against 10655792.
1. Scope membership configuration was directly user-mutable.
`PUT /sessions/{id}/peers/{peer_id}/config` had no scope guard at all, and
`crud.set_peer_config` resolved the peer only to discard the row. Confirmed:
posting `{"observe_others": false, "observe_me": true}` for a scope returned
204 and persisted, which silently stops all fan-out into the scope and makes
Honcho form a representation *of* a scope — neither of which is reachable
through the facade. Deterministic, no race required. Now checked on the row
`get_peer` already returns, so it costs nothing and cannot race.
2. Empty and over-long names were still 500s. Removing the charset pattern from
`PeerSpec` fixed one trap but left its length bounds, and request-bound peer
names carry no length limits of their own — so `peer_id: ""` or a 513-char
name reached `PeerSpec(...)` and raised a raw pydantic ValidationError that
the catch-all turned into a 500. `PeerSpec` now carries no constraints at all
(matching its documented purpose) and every rule for a new name lives in
`_validate_new_peer_names` on the insert path.
3. Resolved-row protection generalized. The previous pass applied it only to
membership upserts, leaving check-then-use windows elsewhere: peer update
could have a concurrently-created scope's configuration replaced wholesale
(create-path validation does not fire for a peer that now exists), the chat
observer get-or-create could resolve a fresh scope as its observer, and the
generic session-peer removal could silently detach a scope from its sessions.
Each now inspects the resolved peer immediately before acting; the redundant
name-level guard on the update route is dropped in favor of the race-free one.
`remove_peers_from_session` grows an internal `_allow_scope_peers` flag because
the scopes facade ends membership through that same path and must not be blocked
by its own guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(scopes): enumerate every peer-touching route against a scope policy
Three review passes each found the same class of defect: a route nobody had
checked, rather than logic that was subtly wrong. One of them — `PUT
/sessions/{id}/peers/{id}/config`, which let any caller set a scope to
`observe_others=false` and silently stop all fan-out into it — predated this work
entirely, because the guardrail set was assembled guardrail-by-guardrail instead
of derived from the route list. Sampling review cannot close that kind of gap;
enumeration can.
Derives every route through which a peer name can reach the system and requires
each to be classified as GUARDED or EXEMPT-with-a-reason, so a newly added
peer-touching route fails the suite until someone classifies it. Detection is the
union of path shape and a walk of the dependant tree (including sub-dependency
`Form(...)` params and nested request-body models), because neither signal alone
suffices: parameter names miss `POST /sessions/{id}/peers`, whose peer names are
dict keys, and path shape misses `messages/upload`, whose `peer_id` arrives as a
form field behind a parser dependency.
Both invariants are then asserted behaviorally, by calling the routes rather than
inspecting annotations — the guards deliberately live in crud, which is what makes
`messages/upload` guarded for free via `crud.create_messages`:
- a real scope is refused on all 11 guarded routes, and the rejection must name
the scope, so an unrelated 422 (a malformed body) cannot pass the assertion;
- an *unflagged* peer merely occupying the reserved namespace is unaffected. That
half regressed once already when `update_peer` used a name-based check.
Mutation-tested all three failure modes: disabling the `set_peer_config` guard
fails the guarded test naming that route; regressing `update_peer` to name-based
fails the squatter test; adding an unclassified peer route fails the enumeration.
Covers the HTTP surface only. Peer names also reach the system through the
deriver, dreamer, and queue, which have no route table to enumerate — noted in
the module docstring rather than implied to be covered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse a scope in every observed position; enumerate per position
Addresses a fourth review pass against 62681e4d. The headline finding is that the
previous commit's enumeration test had the wrong *model*, not a missing entry.
1. Manual conclusions could create knowledge about a scope. `POST /conclusions`
validated only that observer_id and observed_id exist, so a scope as
`observed_id` persisted a conclusion about a peer carrying observe_me=false and
created an (observer, scope) collection for it. Confirmed: 201, and it read
back. `POST /schedule_dream` had the same hole via `observed`.
The fix is positional, because the invariant is:
A scope may be an OBSERVER. A scope may never be OBSERVED.
A scope as `observer_id` is how scoped conclusions are stored and must keep
working (verified still 201); as `observed_id` it is now refused. Same split
applied to schedule_dream `observed`, the peer-card `target` (which also covers
a scope's self-card, since target-omitted collapses observed to peer_id), and
session-context `peer_target`.
2. Chat target and both representation roles kept check-to-use races. Only the
chat path-level observer was re-checked on its resolved row; the target was
checked by name and then resolved without inspecting scope identity. Both are
now checked at the dialectic preflight, where observer and observed are already
resolved — an absent name has already failed by then, and an existing squatter
cannot retroactively become a scope.
3. Generic membership removal was still racy. The adjacent SELECT narrowed the
window but could not close it under READ COMMITTED. The UPDATE now carries its
own correlated NOT EXISTS against scope_peer_clause(), so Postgres evaluates
the exclusion as part of the statement and a scope committed after the advisory
check still cannot be detached.
4. New-name validation ran after the name reached Postgres. A NUL byte passed the
request schemas and PeerSpec, then raised psycopg.DataError inside the lookup —
a 500. Values that cannot correspond to a stored row by construction (NUL
bytes, over-length names) are now refused before the query.
(Over-length names already returned 422; only the wasted query was real there.)
The enumeration test is rekeyed from (method, path) to (method, path, position).
A binary per-route verdict cannot express finding 1 at all: `POST /conclusions` is
one route with two positions and opposite verdicts. Detection widens to observer /
observed / target / peer_target / peer_perspective, which surfaced four routes the
previous version never saw — conclusions, schedule_dream, queue/status, and
session context.
Also registers `src.routers.workspaces.tracked_db` in the conftest patch list; the
new guard there would otherwise have run against the real configured database
instead of the per-test one.
Mutation-tested: disabling the conclusions observed-guard fails the positional
test naming that position; adding an unclassified `observed_id` param fails
enumeration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): refuse future scopes in observed positions, preserve scope membership
Addresses a fifth review pass against 14136e5b. All six findings reproduced
locally before fixing.
1. High — generic peer replacement removed scope memberships.
`set_peers_for_session` soft-deleted every active SessionPeer row, and the
request-level guard only inspected names *present* in the replacement map. A
caller detached a scope by simply omitting it, never naming it — so no
request-level guard could ever see it. Reproduced: scope sessions went
['<id>'] -> [] on a 200. The exclusion now lives in the UPDATE itself
(correlated NOT EXISTS against scope_peer_clause), so replacement means
"replace ordinary peers" regardless of request contents or concurrent creation.
2. High — peer cards could be pre-seeded for future scopes.
`set_peer_card` resolves only the observer and writes a JSONB key derived from
an unchecked observed name, and the route guard rejected only *existing*
flagged scopes. Reproduced: PUT card with target=scope.<missing> returned 200,
creating that scope then returned 201, and the card described the real scope.
3. High — dreams could be queued for future scopes.
The route checked `observed` in a read-only session that closed before
`enqueue_dream`, and a missing reserved name passes any is-it-a-scope check.
Reproduced: 204 with observed=scope.<missing>.
2 and 3 share a root cause, so they share a fix: a new `reject_scope_observed`
that is stricter than `reject_scope_peers` in exactly one case — a *missing*
reserved name is refused, because nothing on these paths creates the peer, so
nothing else would ever catch it. Existing unflagged squatters still pass.
Both guards moved to the mutation point: card validation into
`crud.set_peer_card` (same transaction as the JSONB write, so Dreamer and
agent-tool callers are covered), dream validation into `enqueue_dream` (same
transaction as the queue insert). The redundant route-level checks are dropped
rather than left as weaker duplicates.
4. Medium — prefixed NUL names still reached PostgreSQL.
`reject_scope_peers` filtered for the reserved prefix and sent matches to a
text comparison, so "scope.future\0name" raised psycopg.DataError — a 500.
Both guards now share `_reserved_name_candidates`, which materializes the input
once and rejects impossible values before any SQL. Materializing matters
independently: the message-author path passes a generator, and validation
iterates separately from the prefix filter, so a generator would be
half-consumed. `_reject_impossible_peer_names` now takes a Collection so the
type checker enforces that.
5. Medium — representation kept a check-to-use race.
The previous commit claimed both representation roles were rechecked after
resolution; that was wrong — only the dialectic preflight got that check, and
the representation route never goes through it. It now opens one short
read-only session *after* the embedding call, checks both positions, and passes
that same session to `get_working_representation`, so no connection is held
across external work and a scope committed later cannot have conclusions in the
collection being read.
6. Low — policy coverage was not exhaustive. `sender_id` reaches CRUD as
`observed` but was missing from the detected parameter set. ALLOW cases could
also not carry builders, so the suite never proved the other half of the
contract — that legitimate scope *observers* keep working, which a guard
rejecting scopes everywhere would satisfy. Both fixed; observer positions on
conclusions, dreams, cards, session context and queue status are now asserted
behaviorally.
Deliberately not implemented: the scope-creation backstop scanning for
pre-existing card keys and queue items naming a future backing peer. Reasoning is
recorded in `get_or_create_scopes` — no new such state can be created now, any
pre-existing row is coincidental since `scope.` was never a meaningful namespace,
the consequence is inert, and detecting card keys means a full table scan per
scope creation.
Mutation-tested each new guard: removing the replacement exclusion fails both
membership-preservation tests; weakening either observed guard to existing-only
fails the pre-seeding tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): exclude scope memberships from the session observer limit
Scope memberships carry observe_others=true, so every scope counted against
SESSION_OBSERVERS_LIMIT (default 10) — capping scopes-per-session at the limit
minus the session's real observers, and reporting the failure as
`400 Cannot create session <name> with 11 observers. ... Observers are peers
with 'observe_others' set to true.` on a membership call. Wrong on three counts:
the ceiling is undocumented and contradicts RFC §5.1 ("sessions belong to any
number of scopes"), the message describes session creation, and it leaks the
word "observer" through a facade whose entire job is hiding observers (RFC
goal 5). The limit exists to bound per-observer deriver fan-out for real peers;
a scope costs document rows, not LLM calls (RFC §5.2), so it does not belong in
that budget.
Excluded from both halves of the check in `_get_or_add_peers_to_session`: the
incoming names via a flag-based lookup, existing memberships via a correlated
NOT EXISTS on `scope_peer_clause()` — the same pattern the replacement and
removal paths already use, so the exclusion holds regardless of concurrent
scope creation. The early `count_observers_in_config(session.peer_names)` check
in `get_or_create_session` is left alone: `peer_names` cannot contain a scope,
and `scopes` is a separate field.
`reject_scope_peers` is split into a `scope_peer_names()` query helper plus a
two-line raiser so the observer count reuses the authoritative name-AND-flag
predicate instead of growing a third copy of it. Still costs nothing on the
common path — no reserved-prefix name in the input means no query at all.
Also caps `SessionCreate.scopes` at 100, matching `ScopeSessionsAdd.session_ids`.
This belongs in the same commit: the observer limit was the only thing bounding
that list, so removing it turns an unbounded `scopes` array into a peer row and
a membership row per element, committed — the single-request path to the
cardinality anti-pattern RFC §8 warns about. Partly answers OQ6: no per-session
cap, 100 per request.
Tests: a session joins SESSION_OBSERVERS_LIMIT + 2 scopes through both the
facade and session creation; real observers over the limit still 400, so the
carve-out cannot quietly disable the limit; 101 scopes is a 422.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): skip semantic retrieval when the embedding precompute failed
Addresses three open review comments.
1. Major — the representation read could embed inside its DB session. The
route's precompute is suppressed, and both
`RepresentationManager.get_working_representation` and
`crud.query_documents` fall back to embedding when a query arrives without
one, so a failed precompute meant an external call inside the read session
this branch opens for the scope re-check — the connection-holding rule the
route's own comment claimed to satisfy. The innermost fallback also only
catches ValueError, so a provider outage surfaced as a 500. The semantic
query is now passed only when an embedding exists, degrading to
derived+recent retrieval. (`crud.query_documents` embedding inside a caller's
session predates this branch and is left alone.)
2. Minor — `test_resolved_scope_peer_rejected_at_membership_upsert` described a
race it does not perform. It creates an already-flagged scope and calls crud
directly; the unflagged → flagged transition is not simulated. Docstring now
says what the test actually pins.
3. Minor — `test_empty_replacement_preserves_scope_membership` asserted only
half its docstring. It passed if the empty PUT left every ordinary
membership intact; now asserts the ordinary peer's left_at is set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(scopes): close auth and observed-position gaps, paginate membership
Review response for #884.
Security:
- gate `SessionCreate.scopes` behind a workspace-level key; the session-create
route is self-authorizing, so a peer- or session-scoped token could mint scope
peers and join sessions to scopes it had no access to via `POST /scopes`
- refuse a reserved-but-nonexistent name in two observed positions that used the
permissive guard: chat `target` and session-context `peer_target`. Both let a
caller act on `scope.X` before it existed, then create the scope
Facade:
- exclude scope peers from `GET /sessions/{id}/peers` and refuse the membership
-config read for a real scope, matching its write side
- replace `GET /scopes/{id}/sessions` with `POST /scopes/{id}/sessions/list`
returning `Page[Session]`; the add route now returns 204. Membership was
unbounded on both, while every other list surface paginates
- rename `crud.get_scope` to `get_scope_or_raise`
Tests:
- add a missing-name axis to the route-policy table (`Case.refuse_missing`), which
is what surfaced the two guard gaps above
- delete 14 hand-written tests the table now enumerates; 52 -> 39 functions in
test_scopes.py with more cases covered
- tighten the squatter assertion from `!= 422` to `< 400`, which was passing on 5xx
- assert the FastAPI-internals traversal still derives positions, so a framework
upgrade can't silently empty the suite
Docs:
- drop internal ticket and RFC references from the published OpenAPI descriptions
and surrounding comments; state the behavior instead
- move implementation reasoning out of the `PUT /peers/{id}` docstring, which
FastAPI publishes, into a comment
* test(scopes): assert exact statuses for permissive missing-name cases
Follow-up review pass on #884.
- add `Case.missing_status` so a permissive missing-name position asserts the
status it should actually get (404, or 200 for the no-op removal) instead of
`!= 422`, which also passed on a 5xx — the same hole already closed in the
squatter assertion
- require it whenever `refuse_missing` is False, and require its absence when
True, so the policy table can't drift from the assertion
- repoint a stale allow-reason at POST /scopes/{scope_id}/sessions/list; the GET
it named was removed
- document the membership list's ordering under `reverse`
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* test(embedding): add reproducer for missing encoding_format on openai paths
The openai SDK defaults encoding_format to base64 when it is not passed. OpenAI-compatible providers that don't support base64 embeddings (e.g. OpenRouter with nvidia/nemotron-3-embed-1b:free) return HTTP 200 with empty data, and every embedding call fails with 'No embedding data received'.
* fix(embedding): request float encoding_format on openai embedding calls
The openai SDK defaults encoding_format to base64 when the caller does not pass one. OpenAI-compatible providers that don't support base64 embeddings (e.g. OpenRouter hosting nvidia/nemotron-3-embed-1b:free) answer HTTP 200 with empty embedding data, and every embedding call fails with 'No embedding data received', breaking conclusions, semantic search, and the deriver. Pass encoding_format='float' explicitly on both the single-query and batch call paths.
* test(embedding): cover openai-compatible providers in the live embedding matrix
The existing openai family runs against real OpenAI, which serves base64
embeddings happily, so the matrix passes with or without the #932 fix. Adds an
`openai_compatible_embedding` family (openai transport, third-party base_url)
so the matrix can reach a provider that rejects base64. Empty default_models
keeps it skipped unless LIVE_EMBEDDING_OPENAI_COMPATIBLE_MODELS is set.
Also adds test_live_openai_float_encoding_matches_base64, which pins the other
direction: switching the wire format must not move vectors on real OpenAI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(embedding): keep an explicit embedding-count check on the openai paths
Passing `encoding_format` disables the openai SDK's own empty-data guard, so a
provider answering 200 with missing embeddings surfaced as `IndexError: list
index out of range` on the single path and `zip() argument 2 is shorter than
argument 1` on the batch path. The latter is also #745's signature, which would
have left it with two unrelated causes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* docs(live-llm): correct the openai-compatible embedding matrix env docs
The documented default dimensions said 2048 after the family moved to 3072, and
LIVE_EMBEDDING_OPENAI_COMPATIBLE_SEND_DIMENSIONS was missing entirely. Also
points the example and the coverage note at a model that is actually reachable,
and records that OpenRouter load-balances, so the base64 failure is per-attempt.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(embedding): resolve openai encoding_format by mode instead of pinning float
Requesting float unconditionally costs ~3.6x the response bytes of base64 and up
to +83% latency on a 500-item batch, which the default deployment on real OpenAI
pays for nothing: only third-party OpenAI-compatible providers reject base64.
Adds EMBEDDING_MODEL_CONFIG__ENCODING_FORMAT_MODE, mirroring dimensions_mode.
`auto` keeps base64 when no base_url override is set or it points at
api.openai.com, and picks float elsewhere. The format is still always sent
explicitly, since the SDK otherwise injects base64 on its own.
Also corrects the _validate_embedding_count docstring, which said "fewer" where
the guard is an inequality.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(embedding): request base64 embeddings by omission, not by name
The openai SDK decodes a base64 response only when it injected the default
itself; naming any format makes it skip the decode and hand back the raw string,
which then fails the dimension check with "Expected 1536, got 8192". base64 mode
therefore has to omit the kwarg rather than pass it.
The unit fake returned float lists whatever was asked for, so it could not catch
this. It now mirrors the SDK and returns a base64 string for a named base64
request, which fails against the previous commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Fix Gemini batch embedding for gemini-embedding-2* models
The google-genai SDK treats embed_content(contents=[list_of_strings]) as a
single multi-part document for gemini-embedding-2* models, silently returning
exactly 1 embedding regardless of input count. This caused a zip(...,
strict=True) ValueError in _process_batch.
Wrap each text in genai_types.Content(parts=[genai_types.Part(text=...)])
so the SDK treats each string as a separate content item. This matches the
workaround used by pydantic-ai (#4873) and graphiti (#1474).
Upstream SDK issue: googleapis/python-genai#2523Fixesplastic-labs/honcho#744
* test(embedding): live embedding coverage for every Gemini and OpenAI model
Adds tests/live_llm/test_live_embeddings.py plus an env-driven embedding
matrix alongside the existing LLM one. Covers single embed, batched embed,
batch-vs-single alignment, chunk-to-id mapping, and the batch-split path.
Only a live call catches the gemini-embedding-2* collapse: the SDK folds a
list of bare strings into one document and returns a single embedding.
Reverting the Content wrapping fails all four batch tests for
gemini-embedding-2-preview and gemini-embedding-2 with the reported
`zip() argument 2 is shorter than argument 1`, while gemini-embedding-001
and text-embedding-3-small stay green.
Also makes the concatenation in the conclusions semantic-search validation
message explicit, so the repo-wide basedpyright pre-push hook passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(embedding): drop the preview twin from the default embedding matrix
gemini-embedding-2 is the GA release of gemini-embedding-2-preview and
behaves identically, so running both by default doubles the Gemini cost for
no extra coverage. The preview stays reachable through
LIVE_EMBEDDING_GEMINI_MODELS.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The error raised when observer/observed are missing from a semantic-search
query states the requirement but not where the values go, so callers can't
tell whether they belong at the top level or inside `filters`. Point at the
`filters` object explicitly, show a minimal well-formed payload, and note
that both the bare and `_id`-suffixed key spellings are accepted (the code
already reads either).
Co-authored-by: hermes <hermes@local>
* fix(deriver): stop extraction examples teaching fabricated inferences
The EXAMPLES block in the minimal deriver prompt demonstrated two
inferences that its own output schema forbids:
- "I just had my 25th birthday last Saturday" -> "alice's birthday is
June 21st". A vague relative reference cannot yield a specific date;
this demonstrates inventing one.
- "I took my dog for a walk in NYC" -> "alice lives in NYC". Visiting a
place is not living there.
A third example invited "+ general knowledge" inference to produce a
deductive conclusion. The deriver has no channel for that output --
PromptRepresentation carries only `explicit`, described as "direct
quotes or clear paraphrases only, no interpretation or inference", and
deductive conclusions are produced by the Dreamer's DeductionSpecialist.
Replace all three with examples that stay inside the schema's contract.
The dog/NYC message is kept and shown extracting correctly, and a third
example shows that "lives in NYC" is valid when actually stated, so the
examples teach the boundary rather than just avoiding it.
Closes#626
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(deriver): keep the stated duration in the NYC residence example
The example dropped "six years" from "I've lived in NYC for six years",
which contradicts the prompt's own rule to extract all observations and
to contextualize each one. Emit both the residence fact and the duration.
Keeping "alice lives in NYC" alongside it is deliberate: that output is
the point of the example, contrasting with the preceding one where the
same conclusion is *not* supported by a single visit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix: resolve tiktoken encoding without constructing the embedding client
EmbeddingClient.encoding forced full client construction, which raises
'OpenAI API key is required' even though tiktoken needs no credentials.
The document dedup tie-break (src/crud/document.py) only needs .encoding
for token counting, so any test hitting that path fails in environments
without embedding keys — notably CI for pull requests from forks, where
repo secrets are unavailable (e.g. #908's test-python job failing on
tests/crud/test_document.py::test_duplicate_rejection_reinforces_existing).
Resolve the encoding from the configured model directly, falling back to
cl100k_base, and only reuse the underlying client's encoding when it has
already been constructed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: make embedding batch size configurable
Add optional max_batch_size to the embedding model config
(EMBEDDING_MODEL_CONFIG__MAX_BATCH_SIZE) to cap texts per request for
OpenAI-compatible providers with smaller limits than OpenAI's, such as
DashScope text-embedding-v4 (10) and Alibaba Bailian
qwen3.7-text-embedding (20). When unset, native provider defaults are
preserved (OpenAI 2048, Gemini 100).
Fixes#687.
* test(embedding): cover Gemini batching and config fallbacks per review
- Gemini transport now tested for configured batch splitting and the 100
default fallback
- OpenAI unset default (2048, single request) explicitly covered
- env-parsing test now asserts the value survives resolve_embedding_model_config
- docs: 100 is the client's conservative Gemini default, not a native limit
* test(embedding): assert provider batch-size defaults
---------
Co-authored-by: adavyas <adavyasharma@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
EmbeddingClient.encoding forced full client construction, which raises
'OpenAI API key is required' even though tiktoken needs no credentials.
The document dedup tie-break (src/crud/document.py) only needs .encoding
for token counting, so any test hitting that path fails in environments
without embedding keys — notably CI for pull requests from forks, where
repo secrets are unavailable (e.g. #908's test-python job failing on
tests/crud/test_document.py::test_duplicate_rejection_reinforces_existing).
Resolve the encoding from the configured model directly, falling back to
cl100k_base, and only reuse the underlying client's encoding when it has
already been constructed.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(llm): set HTTP timeout on Gemini clients (#785)
* fix(embedding): set HTTP timeout on Gemini embedding client (#785)
Same wedge-class failure as the LLM client: a stalled Gemini embedding
socket hangs the in-process reconciler, which shares the deriver worker's
uvloop event loop. Apply the same 10-minute timeout here, in lockstep
with src/llm/registry.py's _build_gemini_http_options.
* style(test): drop extra blank line in test_registry imports
* fix(llm): support per-request provider timeouts
* fix(llm): convert Gemini timeout to milliseconds
* fix(llm): validate Gemini HTTP options
* test(llm): type Anthropic stream context args
* test(llm): live per-request timeout coverage for all providers
Two live checks per provider: a generous timeout asserted at the SDK
call boundary, and a tight timeout that must abort well under the 600s
client default. Gemini's async transport can be aiohttp, so its tight
timeout surfaces as asyncio.TimeoutError rather than httpx.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* style(tests): drop extra blank line in anthropic backend test
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(llm): validate provider_params.timeout at config load
Move the timeout coercion into src.config as coerce_provider_timeout and
run it from a field validator on ModelOverrideSettings.provider_params, so
a bad value in config.toml/env fails at startup with the exact config path
instead of surfacing per-request as a retried 500. Good values normalize
to float seconds at load. The per-request guard in src.llm.backend now
delegates to the same coercion (wrapping ValueError in ValidationException)
and continues to cover extra_params passed programmatically.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs: document provider_params.timeout load-time validation and gotchas
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(llm): address review nits on timeout plumbing
Apply eisene's review feedback:
- Rename PROVIDER_TIMEOUT_ERROR → PROVIDER_TIMEOUT_ERROR_TEXT
- Move request_timeout_from_extra_params from backend.py (pure
dataclasses) to request_builder.py (request assembly)
- Add comment explaining Gemini's ms timeout conversion
- Generalize _normalize_extra_params with _strip_none_params helper
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Aakash Kattelu <aakash@plasticlabs.ai>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: guard against top_k=0 reaching the vector store
Turbopuffer rejects top_k=0 with a 400 ('top_k must be between 1 and
10000'). The fix returns [] for a non-positive top_k, before the embedding call), and
floor the semantic budget at 1 so an explicitly requested search isn't
silently allocated zero.
* fix: comments
* fix: apply session scoping to all working-representation query paths
session_name was only applied to the recent-documents query in
RepresentationManager; the semantic and most-derived paths ignored it,
so limit_to_session leaked cross-session conclusions into perspectives.
- Thread a session allowlist (session_names) uniformly through all
three query paths; pushed down to pgvector and external vector stores
- Accept a list so the upcoming session-allowlist API reuses this path
- Fail closed on an empty allowlist (downstream stores drop empty IN
clauses, which would silently widen scope)
Fixes DEV-1994
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: bare-list membership sugar in the filter DSL
{"session_id": ["s1", "s2"]} is now shorthand for
{"session_id": {"in": [...]}} on regular columns, generically
(peer_id, etc.). JSONB metadata columns are excluded — a bare list
there keeps JSONB containment semantics, unchanged.
Previously a bare list on a regular column compiled to a type-mismatched
equality that matched nothing, so this is strictly additive.
Also translates the same shape in the turbopuffer/lancedb filter
builders, and fixes lancedb dropping empty IN clauses (fail-open) —
an empty membership list now emits an always-false condition.
Groundwork for DEV-1995 (session allowlist via the existing filters
DSL, no new API params)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: session allowlist on dialectic and representation via filters
Adds a constrained 'filters' body to peer.chat and /representation —
the same DSL search and conclusions already accept, supporting only the
session_id key (a session id, a bare list, or {"in": [...]}).
Unsupported keys and shapes are rejected with 422, never silently
ignored. Composes with session_id (must be included in the allowlist
when both are given). Capped at 1,000 sessions per request.
Enforcement is uniform at every recall chokepoint, fail-closed:
- dialectic prefetch + search_memory: conclusion recall restricted to
the allowlist; dream docs (session_name IS NULL) excluded
- message tools (search/grep/date-range/temporal/context/history):
strict intersection of allowlist and observer session membership
- get_reasoning_chain: unavailable under an allowlist (chains traverse
provenance across sessions and cannot be scoped without leaking)
- empty allowlist short-circuits to empty results everywhere
Auth: workspace keys pass the allowlist as-given; peer-scoped JWTs must
be a member of every allowlisted session (403 otherwise), mirroring the
existing single-session check.
Fixes DEV-1995
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: fail closed on empty session allowlist across all filter builders
Empty session allowlists relied solely on the early-return guard in
_get_working_representation_internal. The layers below it were
inconsistent, so a future direct caller (the DEV-1995 allowlist API)
would silently widen scope instead of failing closed:
- _build_filter_conditions used a truthiness check; an empty list was
treated like None and dropped the filter. Now uses `is not None`,
matching the recent/most-derived SQL paths.
- turbopuffer emitted a bare `In []` with undocumented (possibly
fail-open) semantics. Now emits an explicit always-false predicate,
mirroring lancedb's `1 = 0`.
Also extract the duplicated JSONB column tuple in filter.py to a
JSONB_COLUMNS constant.
Tests exercise each fail-closed guarantee at the layer it lives, rather
than masking it behind the early-return guard.
* fix: address tests
* fix(crud): fail closed when session_name is outside the allowlist
search/grep/history helpers scoped to a single session_name ignored the
session_names allowlist entirely — a caller could read a session the
allowlist forbids. The API routes guarded this with a 422, but the
dialectic tools call these CRUD functions directly and bypassed it.
Enforce it at the boundary: return [] when session_name is set and not in
the allowlist, across _semantic_search_messages (covers search_messages +
search_messages_temporal), grep_messages, get_messages_by_date_range,
get_recent_history, and get_observation_context.
Also rename the public param allowed_sessions -> session_names for
consistency with representation.py / chat.py / peers.py; the resolved
intersection keeps its distinct name allowed_session_names.
* fix: test
* fix(scopes): tighten and consolidate session allowlist per review
Addresses review feedback on the session allowlist (DEV-1995).
Behavior changes:
- Auth gate on peers.chat now uses active membership (left_at IS NULL)
via get_peer_session_names(active_only=True), matching the adjacent
is_peer_in_session check on options.session_id. Previously a peer that
had left a session was denied when naming it directly but permitted
when naming it in filters.session_id.
- Scoped conclusion recall is restricted to level == "explicit"
(ALLOWLIST_SAFE_LEVELS). Dream-derived conclusions are stamped with a
single session_name but synthesized across all sessions, so that stamp
can't be scoped on. Applied at all four recall paths. Unscoped recall
is unchanged. Follow-up to give conclusions an authoritative
source-session set is tracked in DEV-2201.
- The allowlist gate checks `is not None` rather than truthiness, so
filters={"session_id": []} reaches it instead of being skipped.
Refactors:
- New crud.message.resolve_session_scope replaces four near-identical
copies of the allowlist-membership intersection. Returns
(allowlist, deny) and never returns an empty list, so the None vs []
distinction that external stores fail open on lives in one tested
place. Takes db=None and opens its own short-lived session only when
distinction that external stores fail open on lives in one tested
place. Takes db=None and opens its own short-lived session only when
an observer lookup is needed, preserving external-lookup-first
ordering on the vector-store path.
- extract_session_allowlist takes must_include, collapsing the
session_id-in-allowlist check duplicated across both peer routes.
- DialecticAgent._select_tools dedupes the two toolset-selection blocks
and drops get_reasoning_chain under an allowlist, rather than paying
for the schema plus a wasted turn to return a refusal.
- Rename session_names -> session_allowlist across crud, agent tools,
dialectic and routes, to remove the one-character ambiguity with
session_name. Internal only; the public filters.session_id surface is
unchanged.
Docs:
- session_allowlist documented across all message and recall entry
points, including the None / [] / populated contract.
- session_name marked deprecated for scoping. Not removed and not
aliased: it also pins the query to one session, bypasses observer
scoping, and drives session-history injection into the dialectic
prompt, so it has no drop-in replacement.
- Note at the Document branch in utils/filter.py that the raw-key
fallback is load-bearing for session scoping.
Tests: 20 -> 39 in tests/test_session_allowlist.py, covering the
peer-scoped JWT gate (member, non-member, left-session, workspace-key
bypass, empty allowlist), the resolve_session_scope tri-state including
the no-DB-checkout path, must_include, and the level narrowing.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: redact Redis password from cache connection logs
The cache client logged the full Redis URL — including the password —
at INFO and WARNING levels on every connection attempt and failure.
This exposed the live Redis credential in stdout/container logs and any
downstream log aggregation.
Add _redact_cache_url() to mask the password component before logging.
URLs without a password are returned unchanged.
Closes#866
* fix: handle malformed URLs and IPv6 in _redact_cache_url
Address review feedback from VVoruganti and CodeRabbit:
- Wrap urlparse/urlunparse in try/except so malformed URLs (e.g. invalid
port) don't raise ValueError inside except blocks, which would crash
startup instead of degrading gracefully
- Preserve IPv6 brackets (e.g. [::1]) in reconstructed URLs
- Add Google-style Args/Returns docstring sections
- Add unit tests for password masking, no-password URLs, IPv6,
malformed inputs, and the invalid-port regression
* test: use real secrets in redaction test fixtures
Three fixtures were weakened by copy-paste mangling: literal '***'
placeholders instead of real passwords (assertions trivially true),
an unescaped '#' that truncated netloc parsing via the URL fragment,
and a no-password case that actually contained userinfo. Restore
inputs that genuinely exercise the masking paths.
* fix: never leak password through malformed-URL fallback
The catch-all fallback returned the original URL when parsing failed,
so a Redis URL with a password and an invalid port (typo, out-of-range)
was logged in clear text - the exact leak #866 exists to fix. Narrow
the handling: .port access gets its own try/except (invalid port is
omitted from the output; userinfo/hostname masking never raises), and
the outer fallback now returns a generic placeholder instead of the
raw input. Tightened the invalid-port test to assert the password is
absent and added out-of-range-port and unparseable-URL cases.
* fix: redact secrets in query params and scheme-less URLs
_redact_cache_url only masked userinfo, but a credential can reach the
URL through two other real configuration paths: redis-py accepts
?password= (all querystring options become client kwargs) and cashews
accepts ?secret= (its HMAC signing key) - and honcho's own default
CACHE.URL already uses a query param (?suppress=true), so this is the
expected configuration style. Separately, a URL missing its scheme
(':pass@host:6379/0') parses with an empty netloc, making the password
invisible to .password and echoing it back verbatim.
Mask sensitive query values in place on the raw query string (no
decode/re-encode, so non-secret params are preserved byte-for-byte)
and return the generic placeholder for @-carrying strings with no
parseable authority. Verified with a 20k-case randomized fuzz run in
addition to the unit tests: no functional credential reaches the
output.
Add a PEP 508 marker so lancedb is not installed on darwin/x86_64, wrap the
LanceDB vector store import in try/except for a clear config error, and
regenerate uv.lock.
Branch rebased onto upstream/main; prior src/utils/clients.py CI tweak is
obsolete because LLM wiring moved under src/llm/.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: apply session scoping to all working-representation query paths
session_name was only applied to the recent-documents query in
RepresentationManager; the semantic and most-derived paths ignored it,
so limit_to_session leaked cross-session conclusions into perspectives.
- Thread a session allowlist (session_names) uniformly through all
three query paths; pushed down to pgvector and external vector stores
- Accept a list so the upcoming session-allowlist API reuses this path
- Fail closed on an empty allowlist (downstream stores drop empty IN
clauses, which would silently widen scope)
Fixes DEV-1994
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: bare-list membership sugar in the filter DSL
{"session_id": ["s1", "s2"]} is now shorthand for
{"session_id": {"in": [...]}} on regular columns, generically
(peer_id, etc.). JSONB metadata columns are excluded — a bare list
there keeps JSONB containment semantics, unchanged.
Previously a bare list on a regular column compiled to a type-mismatched
equality that matched nothing, so this is strictly additive.
Also translates the same shape in the turbopuffer/lancedb filter
builders, and fixes lancedb dropping empty IN clauses (fail-open) —
an empty membership list now emits an always-false condition.
Groundwork for DEV-1995 (session allowlist via the existing filters
DSL, no new API params)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: fail closed on empty session allowlist across all filter builders
Empty session allowlists relied solely on the early-return guard in
_get_working_representation_internal. The layers below it were
inconsistent, so a future direct caller (the DEV-1995 allowlist API)
would silently widen scope instead of failing closed:
- _build_filter_conditions used a truthiness check; an empty list was
treated like None and dropped the filter. Now uses `is not None`,
matching the recent/most-derived SQL paths.
- turbopuffer emitted a bare `In []` with undocumented (possibly
fail-open) semantics. Now emits an explicit always-false predicate,
mirroring lancedb's `1 = 0`.
Also extract the duplicated JSONB column tuple in filter.py to a
JSONB_COLUMNS constant.
Tests exercise each fail-closed guarantee at the layer it lives, rather
than masking it behind the early-return guard.
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix: enforce explicit-document session purity in dedup/merge paths
Audit for DEV-2000 (Scopes RFC prerequisite): explicit-level documents must
stay session-pure so scope memory can be built by copying explicit documents
between collections. Two classes of violation were possible:
- Exact-content and semantic dedup in crud/document.py matched candidates
with no level or session scoping, so an explicit document could be
reinforced by — or soft-deleted in favor of — a same-content document from
a different session or a different level (silently merging cross-session
derivations into one row).
- The generic create_observations tool handler accepted level='explicit'
from agents with no message context (dreamer/dialectic), which would mint
session-less explicit documents.
Enforcement (refuse, never rewrite):
- create_documents refuses explicit documents with a null session_name
- exact dedup keys on (content, level, session-for-explicit); derived levels
keep cross-session consolidation
- is_rejected_duplicate scopes candidate search to the same level, and the
same session for explicit documents
- the create_observations tool rejects explicit-level input outside message
ingestion (deriver) context
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: add card_refresh dream type for event-driven peer-card updates
Adds a lightweight dream variant (DEV-2000, Scopes RFC prerequisite) that
runs ONLY the peer-card update — for event-driven refreshes such as scope
membership changes and cold starts:
- DreamType.CARD_REFRESH alongside OMNI; dispatched by process_dream to a
new run_card_refresh_dream orchestration
- CardRefreshSpecialist: restricted to get_recent_observations,
search_memory, and update_peer_card (no observation-mutating tools), with
a low tool-iteration cap of min(6, DREAM.MAX_TOOL_ITERATIONS)
- rebuild=True mode carried in the dream payload: the existing card is NOT
injected into the prompt and the specialist rebuilds it solely from
observations present in the collection (for use after removals)
- enqueue-able via the manual enqueue_dream path (bypasses volume gates);
the work-unit key already embeds the dream type so a card refresh never
collides with a pending omni dream. POST /v3/workspaces/{id}/schedule_dream
accepts dream_type=card_refresh plus the rebuild flag
- card refreshes never advance the omni dream guard pair
(last_dream_at / last_dream_document_count)
- shared PEER CARD prompt section extracted (verbatim) from
DeductionSpecialist for reuse; CallPurpose gains dream.card_refresh
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: fix tests
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Structured outputs for dialectic
* cleanup
* rename json_schema_to_pydantic to clarify it's not a general schema converter
* clean up schema DoS guards
* simplification and cleanup of schema conversion
* chore: ruff and pyproject toml
* chore: basedpyright cleanup in test
* fix: some needed unrelated test failures
* test(schema_conversion-and-anthropic-backend): expand test coverage
include table tests
* fix(llm): support combined tool calling and structured output across backends
- OpenAI: parse() 500s on non-strict function tools; route tool-carrying
structured requests through create() with an explicit json_schema
response_format (mirrors the streaming path)
- Anthropic: skip the '{' JSON prefill when tools are present so tool_use
blocks stay reachable; make the schema instruction conditional and rely
on parse + repair
- Gemini: native response_schema + function calling is rejected before
Gemini 3; with tools present, inject a schema instruction into the final
turn instead and rely on parse + repair
- All backends: tool-call turns carry no consumable content, so skip
structured-output parsing on them
Extracted from the dialectic structured-output branch (DEV-1652) so the
transport layer can land independently.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(live_llm): exercise combined tools + structured output per provider
Two-turn live flow per backend: a forced tool-call turn (structured
parsing must be skipped) followed by a replay turn that must return a
schema-conforming answer with tools still attached. Asserts the
provider-specific request shaping: no parse() for OpenAI (500s on
non-strict tools), no '{' prefill for Anthropic, no native
response_schema for Gemini.
Verified against live OpenAI (gpt-4.1, gpt-5, gpt-5.4, gpt-5.4-mini)
and Gemini (gemini-2.5-flash).
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(unified): dialectic chat with response_format schema under tool use
Adds response_format pass-through to the unified runner's chat query and
a test case that forces the dialectic tool loop (reasoning off + global
enumeration question) while requiring a schema-conforming JSON answer —
end-to-end coverage of the combined tools + structured output transport
path on whichever provider each level is configured with.
Verified locally against a full harness run (json_match assertions pass;
the llm_judge assertion additionally runs in CI where the Anthropic key
is available).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: some needed unrelated test failures
* ci: add label-triggered live LLM test workflow
Adding the run-live-llm label to a PR (or workflow_dispatch) runs
tests/live_llm/ against real provider APIs — the only place the
--live-llm suite runs in CI. Reuses the unified-tests environment and
its Secrets Manager staging-dotenv resolution for provider keys; runs
on ubuntu-latest (no Fly runner, no Docker — the suite only touches the
LLM backends). Pins LIVE_LLM_ANTHROPIC_45_PLUS_MODELS=claude-sonnet-4-5
since the Anthropic family has no default models and would otherwise
silently collect empty.
Opt-in by design: live model behavior is variable, so this is a signal,
not a required check.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: run live LLM tests on main pushes touching the transport
Mirrors unified-tests' push trigger, scoped to paths that can affect
the live suite (src/llm/, config, the tests, deps, and the workflow
itself) so provider API calls aren't spent on unrelated changes.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: disable auth in live LLM test environment
The staging dotenv sets AUTH_USE_AUTH=true without a usable JWT secret,
and src/config.py validates the pair at import time — the same reason
unified-tests overrides it. This suite never runs the API server.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(live_llm): fix gpt-5.4 reasoning_effort and gemini replay-turn flake
- test_live_openai: gpt-5.4 dropped 'minimal' from the reasoning_effort
vocabulary, so the gpt5 caching test 400'd — and the OpenAI backend's
BadRequestError terminal swallowed it into an empty CompletionResult.
Pick the effort per model generation.
- test_live_tools_structured_output: use tool_choice='auto' on the
replay turn, matching the production dialectic loop (which never
forces 'none') — NONE mode is what provoked gemini-2.5-flash's empty
candidates. Drop the temperature pin so retries actually resample,
and treat a repeat tool call as a retryable attempt.
Verified live: full suite green, gemini 4/4 consecutive passes.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: fail live LLM run when no staging secret was loaded
If the latest-tag fetch fails and no second tag exists, the fallback
step is skipped rather than failed, and the job would proceed without
provider keys — every test then skips via require_provider_key and the
run goes green. Guard on both fetch outcomes so that path fails loudly.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(live-llm-tests-GHA): remove extra comments
* feat(structured-output): enable non-recursive schema references
* docs(structured-outputs): clean up new doc
* test(structured-output): fix caching refs memory leak, add tests
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(llm): support combined tool calling and structured output across backends
- OpenAI: parse() 500s on non-strict function tools; route tool-carrying
structured requests through create() with an explicit json_schema
response_format (mirrors the streaming path)
- Anthropic: skip the '{' JSON prefill when tools are present so tool_use
blocks stay reachable; make the schema instruction conditional and rely
on parse + repair
- Gemini: native response_schema + function calling is rejected before
Gemini 3; with tools present, inject a schema instruction into the final
turn instead and rely on parse + repair
- All backends: tool-call turns carry no consumable content, so skip
structured-output parsing on them
Extracted from the dialectic structured-output branch (DEV-1652) so the
transport layer can land independently.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(live_llm): exercise combined tools + structured output per provider
Two-turn live flow per backend: a forced tool-call turn (structured
parsing must be skipped) followed by a replay turn that must return a
schema-conforming answer with tools still attached. Asserts the
provider-specific request shaping: no parse() for OpenAI (500s on
non-strict tools), no '{' prefill for Anthropic, no native
response_schema for Gemini.
Verified against live OpenAI (gpt-4.1, gpt-5, gpt-5.4, gpt-5.4-mini)
and Gemini (gemini-2.5-flash).
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: some needed unrelated test failures
* ci: add label-triggered live LLM test workflow
Adding the run-live-llm label to a PR (or workflow_dispatch) runs
tests/live_llm/ against real provider APIs — the only place the
--live-llm suite runs in CI. Reuses the unified-tests environment and
its Secrets Manager staging-dotenv resolution for provider keys; runs
on ubuntu-latest (no Fly runner, no Docker — the suite only touches the
LLM backends). Pins LIVE_LLM_ANTHROPIC_45_PLUS_MODELS=claude-sonnet-4-5
since the Anthropic family has no default models and would otherwise
silently collect empty.
Opt-in by design: live model behavior is variable, so this is a signal,
not a required check.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: run live LLM tests on main pushes touching the transport
Mirrors unified-tests' push trigger, scoped to paths that can affect
the live suite (src/llm/, config, the tests, deps, and the workflow
itself) so provider API calls aren't spent on unrelated changes.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: disable auth in live LLM test environment
The staging dotenv sets AUTH_USE_AUTH=true without a usable JWT secret,
and src/config.py validates the pair at import time — the same reason
unified-tests overrides it. This suite never runs the API server.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(live_llm): fix gpt-5.4 reasoning_effort and gemini replay-turn flake
- test_live_openai: gpt-5.4 dropped 'minimal' from the reasoning_effort
vocabulary, so the gpt5 caching test 400'd — and the OpenAI backend's
BadRequestError terminal swallowed it into an empty CompletionResult.
Pick the effort per model generation.
- test_live_tools_structured_output: use tool_choice='auto' on the
replay turn, matching the production dialectic loop (which never
forces 'none') — NONE mode is what provoked gemini-2.5-flash's empty
candidates. Drop the temperature pin so retries actually resample,
and treat a repeat tool call as a retryable attempt.
Verified live: full suite green, gemini 4/4 consecutive passes.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* ci: fail live LLM run when no staging secret was loaded
If the latest-tag fetch fails and no second tag exists, the fallback
step is skipped rather than failed, and the job would proceed without
provider keys — every test then skips via require_provider_key and the
run goes green. Guard on both fetch outcomes so that path fails loudly.
DEV-2035
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(live-llm-tests-GHA): remove extra comments
* ci(CODEOWNERS): introduce CODEOWNERS and gate GHA heavy test runs behind being a CODEOWNER
* ci(GHA-live-LLM-tests): consolidate common GHA steps
* test(test_live_openai): fix reasoning level adjustment for gpt-5
* test(live-llm-tests): temporary removal of gate to test the workflow
* test(live-llm-tests): revert removal of gate
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Split `REPRESENTATION_BATCH_MAX_TOKENS` into a "minimum work unit" setting on the producer side and a "maximum LLM tokens" setting on the consumer side
* feat(telemetry): CloudEvents + Langfuse tracing as projections over a captured LLM stream
Capture each LLM call once (CapturedLLMCall) and fan it out to multiple
exporters -- "one data model, two projections": a CloudEvents trace stream
(llm.call.traced / trace.content) and a Langfuse projection, both reconstructing
trace -> run -> step -> generation from the same source of truth.
- Capture seam (src/llm/capture.py): one canonicalization + content-addressed
hashing point, with an O(N) per-span memo so repeated context isn't re-hashed.
- Session correlation threaded telemetry -> captured call -> exporters,
namespaced only at the Langfuse export boundary.
- Span identity consolidated onto LLMTelemetryContext; dropped TRACE_ENDPOINT.
- Canonical generation/step names; dreamer branches nest under one dream trace;
tool calls become spans under their step.
- LANGFUSE_EXPORTER_MODE toggle ("exporter" default; "inline" kept one release
for side-by-side validation), centralized into computed settings predicates.
- Per-run/per-trace dedup registries (trace_session, langfuse_session) bounded
by an LRU so dedup and span grouping survive long-running workers.
- Embedding-call tracing; deterministic high-volume event sampling.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(telemetry): address trace-review findings (span/step_seq collisions, test, logging)
- Dreamer specialists mint a distinct span_id per execution (trace_id stays the
shared dream run_id), so their CloudEvents trace resource ids no longer collide
between deduction and induction.
- Tool-loop no-tool early-return streams the tail with the next ordinal
(iteration+2) instead of reusing the in-loop call's step_seq, avoiding a
colliding trace resource id; mirrors the synthesis path.
- Tighten test_clips_oversized_string to assert output stays within TRACE_MAX_BYTES.
- emit_trace logs the swallowed exception with exc_info for debuggability.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(telemetry): silence exporter-mode Langfuse warning + drop summarizer run_id placeholder
Two CloudEvents/Langfuse correctness fixes, independent of the trace viewer.
Langfuse exporter-mode gating: annotate_current_generation_io (and its two
executor.py call-site guards) were gated on LANGFUSE_PUBLIC_KEY instead of
langfuse_inline_enabled. In the default `exporter` mode they called
get_client().update_current_generation() with no active @observe span, logging
"No active span in current context" (~14 per dialectic run) and building
throwaway model_dump payloads on every LLM call. The LangfuseExporter projects
I/O from the captured stream, so these helpers must no-op in exporter mode.
Gated all three on langfuse_inline_enabled; added a regression test; fixed a
stale conditional_observe docstring.
Summarizer run_id placeholder: AgentToolSummaryCreatedEvent hardcoded
run_id="deriver"/iteration=0 because summarization is a single LLM call, not an
agentic run. That placeholder pollutes run_id grouping in the CloudEvents stream
(any consumer that groups by run_id sees a phantom "deriver" run). Made
run_id/iteration optional (None) and re-keyed get_resource_id on
message_id:summary_type (the real per-summary identity; run_id/iteration can no
longer identify it); bumped schema_version 2->3. Xatu ingestion stores only the
CloudEvent envelope, so the field/resource_id/version changes are transparent to it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: update docstrings to be less verbose
* fix(telemetry): address PR review on captured-stream tracing
- embedding traces get a fresh span_id under parent_span_id=run_id, so
sibling embeddings in one run no longer share a span/idempotency key
- capture the provider finish_reason from stream chunks instead of
hardcoding "stop" on a successful drain
- gate the Langfuse exporter behind TELEMETRY.ENABLED (master switch) so
disabling telemetry sends no traces at all
- rename _emit_derived_content -> _emit_hashed_content
- inline the _emit_trace wrapper; drop unused trace_session.end_run
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: rename TELEMETRY_TRACE_PAYLOADS to TELEMETRY_TRACE_PAYLOADS_ENABLED
* fix(telemetry): capture provider tool calls in trace stream
The captured trace stream dropped assistant tool calls for openai/gemini:
build_captured_messages only read {role, content, tool_call_id}, but those
providers keep tool calls outside content (openai's tool_calls, gemini's
parts), so replayed tool-call turns landed as empty content and gemini lost
its text and tool results entirely. Anthropic (tool_use in content) was fine.
Normalize each input message per provider into a unified tool_calls
[{id, name, input}] field on CapturedMessage/TraceContentEvent, recovering
gemini text/results along the way, and fold tool_calls into
compute_content_hash so empty-content openai turns no longer collide in the
dedup store. langfuse_exporter._input now surfaces the calls.
Also fix a silent serialization drop: gemini thought_signature is bytes, so
model_dump(mode="json") on the traced event raised UnicodeDecodeError and
emit_trace swallowed it -- dropping the whole tool-calling iteration from the
trace stream (billing and Langfuse were unaffected). base64-encode the
signature on the telemetry path; replay keeps the raw bytes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(telemetry): type replay tool-call dict for bytes signature
thought_signature widened to str | bytes | None, but
_tool_call_result_to_dict's literal was inferred as
dict[str, str | dict[str, Any]], so the bytes assignment failed project-wide
basedpyright (the per-file pre-commit hook didn't catch it). Annotate the
dict as dict[str, Any]; the replay path keeps the raw bytes unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: remove 3 tests
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(conclusions): expose reasoning level + allow filtering by level
The `level` of a conclusion (explicit / deductive / inductive /
contradiction) was filterable server-side but stripped from the
`Conclusion` response and not surfaced in either SDK. This adds it
end-to-end so callers can list explicit-only ("not dreamed on")
conclusions without dropping to raw HTTP.
- api: add `level` to the Conclusion response schema
- python sdk: `ConclusionLevel` type, `level` on Conclusion/response,
`level=` kwarg on ConclusionScope.list() and the async variant
- ts sdk: `ConclusionLevel` type, `level` on Conclusion/response,
`level` option on list()
- tests: assert level is exposed; add level-filter list test
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(conclusions): use generic filters= on list() instead of level= kwarg
Match the documented SDK convention (peers/sessions/messages all take a
generic `filters` dict passed through to the same dynamic server-side
filter logic) instead of a one-off `level=` kwarg. `level` filtering now
works as `list(filters={"level": "explicit"})` alongside any other
supported filter/operator.
The `level` field on the Conclusion response (added in the previous
commit) is kept — it's still not otherwise returned by the API.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(conclusions): allow filtering by level on query() in py + ts SDKs
The branch's level-filter work exposed `filters=` on `list()` but left
`query()` (semantic search) hardcoding `{observer, observed}`, so callers
could filter the list endpoint by reasoning level but not semantic search —
asymmetric in both SDKs.
- Python: add keyword-only `filters` to `ConclusionScope.query` and
`ConclusionScopeAio.query`, merged over the scope's observer/observed.
- TypeScript: add optional `filters` arg to `ConclusionScope.query`,
mirroring the existing `list()` change.
The server `/conclusions/query` endpoint already honors filters in the body
(verified against production), so this is purely SDK surface parity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(filters): document filtering conclusions by reasoning level
The using-filters page covered workspaces/peers/sessions/messages but not
conclusions. Add a "Filtering Conclusions" section showing level-based
filtering on both list() and query(), including the common "explicit only"
(exclude dream-derived) case and the in[deductive,inductive] inverse.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(conclusions): simplify filter merge to a single dict spread
Replace the merged_filters + if-block pattern in list()/query() (py sync,
aio, ts) with a single dict spread that layers the caller's filters over the
scope's observer/observed (and session). No behavior change — same merge
order (caller wins) — just less code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(conclusions): reject scope-managed keys in SDK conclusion filters
The generic filters= argument on ConclusionScope.list()/query() spread
user-supplied filters last, so a stray observer/observed/session key
silently overrode the scope and returned data from a different peer
pair. Add a fail-loud guard in both the Python and TypeScript SDKs that
rejects scope-managed filter keys with a clear error, directing callers
to peer.conclusions / conclusions_of(target) and the session= parameter.
session_id remains a valid filter on query() (which has no dedicated
session parameter).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
* feat: add exact content deduplication in document creation
* feat: add comment for index
* fix: harden times_derived logic across all callers to use max of inputs and existing + 1
---------
Co-authored-by: Vineeth Voruganti <13438633+VVoruganti@users.noreply.github.com>
Aakash Kattelu
Daniel Peng
Phil
Serhii Zghama
Vineeth Voruganti
Joe-Kneeland
Ulysse Pence
Rajat Ahuja
Vansh Sharma
Niyaz Almufti
bbasketballer75
JUNZE
adavyas
PK
Alexei Vedernikov
Leonardo Baray
papesy384
Eugene Eisenstein
Ulysse Pence
ajspig
Chris Caldwell