Skip to content

feat: persist UUID episode identifiers - #1707

Merged
malatewang merged 29 commits into
mainfrom
episode_uuid
Oct 7, 2026
Merged

malatewang merged 29 commits into
mainfrom
episode_uuid

Conversation

@malatewang

@malatewang malatewang commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Episodes receive server-generated UUIDs before persistence. A database-backed
increasing sequence number gives every episode a stable tie breaker when event
times match. Both SQLite and PostgreSQL reserve numbers atomically; semantic
history stores the same sequence instead of a batch position. Episode and
semantic history reads order by episode time and sequence.

The episode store, episodic memory, and semantic registration still write
concurrently to avoid serializing request latency on the episode database
commit. Semantic history keeps registration time separate from episode time.
Registration and ingestion age checks use the semantic storage clock, so clock
skew between application workers and the database does not prematurely expire
missing episodes or trigger ingestion. Empty episode metadata is normalized to
None across in-memory and stored read paths.

Delete behavior and partial writes

Deleting a valid but unknown episode UUID is idempotent and returns success.
Deletion attempts cleanup in each configured backend even when the episode
store row is absent. The episode-store existence read has been removed. All
concurrent add failures are logged before the request raises its first error.

Concurrent writes can still leave a partial record if one backend fails. The
semantic ingestion retry window remains in place, is configurable through
semantic_memory.missing_episode_grace_period_sec, and is applied in debug mode.

Compatibility

This PR does not upgrade old integer episode IDs. Existing integer-key episode
tables, semantic history rows, and citations that refer to those IDs require
an explicit data migration before use with UUID episodes. For databases
already using the UUID schema, the PostgreSQL semantic history migration and
the vector semantic storage startup migration rename the batch position column
to the episode sequence column.

Verification

Focused episode, semantic, and episodic tests pass. Ruff checks and formatting
pass on changed Python files. PostgreSQL and Neo4j integration tests could not
run because Docker is unavailable in this environment. The broad type check is
blocked by optional packages absent from the test environment.

The OpenAPI document was regenerated with UUID examples and schemas.

@malatewang
malatewang force-pushed the episode_uuid branch 3 times, most recently from 20918da to 10e0ae2 Compare September 22, 2026 23:53
@malatewang
malatewang requested review from edwinyyyu and marvinyu-memverge and removed request for edwinyyyu September 23, 2026 18:47

@marvinyu-memverge marvinyu-memverge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 10e0ae2. The parallel write and the grace period on missing
episodes both look right to me; three things I'd want settled before this
goes in, all reproduced locally:

  1. Upgrading an existing deployment. The episode store only runs create_all,
    so an existing episodestore keeps its INTEGER id column, and inserting a
    UUID into it fails (SQLite: "IntegrityError: datatype mismatch"; Postgres
    rejects it the same way). Because the store write now runs alongside the
    episodic and semantic writes, each of those failed adds still lands in
    episodic memory and the semantic queue. So on an un-migrated database
    every add errors and leaves an orphan. Could we either ship a migration
    (semantic storage already has one for its history_id -> string change) or
    check the column type at startup and refuse to start? Either way it fails
    loudly instead of half-writing.

  2. Orphans can't be deleted by id. Same shape outside the upgrade case: if
    episode_storage.add_episodes fails (DB timeout, dropped connection), the
    episode is still in episodic memory and shows up in search, but the new
    existence check in delete_episodes raises ResourceNotFoundError for it,
    so it can only be cleared by deleting the whole session. Either
    delete_episodes should go ahead for ids that aren't in the store, or a
    failed store write should undo the episodic write.

  3. Ordering within a batch. History now sorts by (created_at, history_id),
    and every message in one add shares a created_at whenever the client sends
    the same timestamp for all of them (e.g. importing a transcript stamped
    per session). Ties then fall back to the random UUID, so ingestion can
    process the messages out of order and the older fact wins. I ran
    test_ingestion_keeps_latest_value_across_uuid_ordered_batches with every
    entry at the same created_at: it ends on "violet" instead of "red". On
    main the int id kept insertion order. Something that breaks ties by
    position in the batch would restore it.

Verified and fine: created_at reaches semantic history on all four backends,
the 30 s grace period covers ingestion reading an episode before its store
row commits, and CI is green at head.

@malatewang

Copy link
Copy Markdown
Contributor Author

Reviewed at 10e0ae2. The parallel write and the grace period on missing episodes both look right to me; three things I'd want settled before this goes in, all reproduced locally:

  1. Upgrading an existing deployment. The episode store only runs create_all,
    so an existing episodestore keeps its INTEGER id column, and inserting a
    UUID into it fails (SQLite: "IntegrityError: datatype mismatch"; Postgres
    rejects it the same way). Because the store write now runs alongside the
    episodic and semantic writes, each of those failed adds still lands in
    episodic memory and the semantic queue. So on an un-migrated database
    every add errors and leaves an orphan. Could we either ship a migration
    (semantic storage already has one for its history_id -> string change) or
    check the column type at startup and refuse to start? Either way it fails
    loudly instead of half-writing.
  2. Orphans can't be deleted by id. Same shape outside the upgrade case: if
    episode_storage.add_episodes fails (DB timeout, dropped connection), the
    episode is still in episodic memory and shows up in search, but the new
    existence check in delete_episodes raises ResourceNotFoundError for it,
    so it can only be cleared by deleting the whole session. Either
    delete_episodes should go ahead for ids that aren't in the store, or a
    failed store write should undo the episodic write.
  3. Ordering within a batch. History now sorts by (created_at, history_id),
    and every message in one add shares a created_at whenever the client sends
    the same timestamp for all of them (e.g. importing a transcript stamped
    per session). Ties then fall back to the random UUID, so ingestion can
    process the messages out of order and the older fact wins. I ran
    test_ingestion_keeps_latest_value_across_uuid_ordered_batches with every
    entry at the same created_at: it ends on "violet" instead of "red". On
    main the int id kept insertion order. Something that breaks ties by
    position in the batch would restore it.

Verified and fine: created_at reaches semantic history on all four backends, the 30 s grace period covers ingestion reading an episode before its store row commits, and CI is green at head.

We do not support upgrade or backward compatibility.

Comment thread packages/server/src/memmachine_server/common/episode_store/episode_model.py Outdated
@edwinyyyu

edwinyyyu commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Also using UUID types everywhere prevents bugs:

  • For a string representation, you don't know whether it's hex or not (though it probably is), lowercase or uppercase.
  • For a string representation, you don't know whether it has the hyphens.
  • If some code path doesn't do validation or doesn't follow the convention, then there may be a mix of arbitrarily written ids.

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 3755d06 against main 8101f14. Inline comments below. The three points already raised (migration, orphans on partial failure, batch ordering) remain open; I have not repeated them except where the inline comment adds a mechanism.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/main/memmachine.py
Comment thread packages/server/src/memmachine_server/main/memmachine.py
Comment thread packages/server/src/memmachine_server/main/memmachine.py Outdated
Comment thread packages/server/src/memmachine_server/common/episode_store/episode_model.py Outdated

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second pass at 07f4317. Three inline comments below, plus one item on a file outside the diff:

packages/common/src/memmachine_common/api/doc.py:634 still gives EPISODIC_ID = ["123", "345"] and EPISODIC_IDS = [["123", "345"], ["23"]] as the public examples, and docs/openapi.json is generated from them. This PR changes the id format every client sees, so those examples now describe ids that can never match. Please update them to UUID strings and regenerate docs/openapi.json in this PR.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/main/memmachine.py Outdated

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more inline comment on the metadata normalization.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/main/memmachine.py Outdated

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Four more inline comments: the history timestamp doubling as the ingestion debounce clock, and three on the grace-period bookkeeping.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/semantic_memory/semantic_memory.py Outdated
Comment thread packages/server/src/memmachine_server/semantic_memory/semantic_ingestion.py Outdated
Comment thread packages/server/src/memmachine_server/semantic_memory/semantic_ingestion.py Outdated
@edwinyyyu edwinyyyu added horizontal scaling Wrong or unsafe when more than one server process serves the same backends (replicas or workers) and removed horizontal scaling Wrong or unsafe when more than one server process serves the same backends (replicas or workers) labels Sep 29, 2026

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One naming nit.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more on the missing-episode bookkeeping.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/semantic_memory/semantic_ingestion.py Outdated

@leomem leomem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at a2cbde7. Batch delete from the Python client and the CLI fails after this change.

DeleteEpisodicMemorySpec.episodic_id is now UUID | None (packages/common/src/memmachine_common/api/spec.py:693), but the client still sends an empty string by default:

  • packages/client/src/memmachine_client/memory.py:598: delete_episodic(self, episodic_id: str = "", ...) passes episodic_id into the spec unconditionally.
  • packages/client/src/memmachine_client/cli.py:456: --id defaults to "".

So memory.delete_episodic(episodic_ids=[...]) and memmachine delete-episodic --ids <uuid> raise before any request is sent. Reproduced on this branch:

DeleteEpisodicMemorySpec(org_id="o", project_id="p", episodic_id="", episodic_ids=[str(uuid4())])
# ValidationError: episodic_id
#   Input should be a valid UUID, invalid length: expected length 32 for simple format, found 0 [type=uuid_parsing, input_value='']

An older client that sends "episodic_id": "" now gets a 422 from the server for the same reason. The client tests only call delete_episodic with a single id, so CI doesn't catch it.

Suggested fix: default episodic_id to None in delete_episodic and in the CLI's --id, and add a client test that deletes by episodic_ids alone. If old clients should keep working, the spec could also map "" to None before validation.

@edwinyyyu

Copy link
Copy Markdown
Contributor

@malatewang one review thread is still open and may be easy to miss since it sits on an older review: #1707 (comment)

Short version: keeping the store insert concurrent with the memory writes is a correctness tradeoff (partial writes with no scan-free recovery), so it needs a number. The saving is bounded by the store insert time itself, and that is already instrumented as @timed("add_episodes"). Please post p50 and p99 of that timer on Postgres and SQLite for batch sizes 1 and 10. If the saving is under roughly 20 ms, the ask is to restore the store-first order and drop the grace-period machinery with it. Every other thread from this side is resolved.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

@malatewang

Copy link
Copy Markdown
Contributor Author

@malatewang one review thread is still open and may be easy to miss since it sits on an older review: #1707 (comment)

Short version: keeping the store insert concurrent with the memory writes is a correctness tradeoff (partial writes with no scan-free recovery), so it needs a number. The saving is bounded by the store insert time itself, and that is already instrumented as @timed("add_episodes"). Please post p50 and p99 of that timer on Postgres and SQLite for batch sizes 1 and 10. If the saving is under roughly 20 ms, the ask is to restore the store-first order and drop the grace-period machinery with it. Every other thread from this side is resolved.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

That number will not provide any information. Any overhead is really workload and hardware dependent.

@malatewang

Copy link
Copy Markdown
Contributor Author

@malatewang one review thread is still open and may be easy to miss since it sits on an older review: #1707 (comment)
Short version: keeping the store insert concurrent with the memory writes is a correctness tradeoff (partial writes with no scan-free recovery), so it needs a number. The saving is bounded by the store insert time itself, and that is already instrumented as @timed("add_episodes"). Please post p50 and p99 of that timer on Postgres and SQLite for batch sizes 1 and 10. If the saving is under roughly 20 ms, the ask is to restore the store-first order and drop the grace-period machinery with it. Every other thread from this side is resolved.
🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

That number will not provide any information. Any overhead is really workload and hardware dependent. The original implementation also has consistency and correctness problem. To make the episode insert atomic, it requires additional work.

@edwinyyyu edwinyyyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final-state pass at 6f6fa2e. Six inline comments: two correctness, two dead code, one stale docstring, one leftover test setup. Two items on files outside the diff:

  1. PR body, Compatibility. It declares only the episode table incompatible with existing databases. The semantic history and citation tables are too: existing history_id and citation values are integer strings, every reader now does UUID(...) on them, and history rows from before migration 62dff1150a46 have a NULL created_at that the new registration-time lookup cannot handle. Under the stated no-upgrade stance that is fine, but the body should say all three tables, not one.

  2. docs/open_source/configuration.mdx. The semantic_memory parameter table and samples do not list the new missing_episode_grace_period_sec key. The same section's existing field is ingestion_trigger_age_seconds, an integer, so the new key should also follow that spelling and type within the section.

🤖 Written by Claude Fable 5.1 via Claude Code; posted from @edwinyyyu's account.

Comment thread packages/server/src/memmachine_server/main/memmachine.py
Comment thread packages/server/src/memmachine_server/semantic_memory/semantic_ingestion.py Outdated
Comment thread packages/server/src/memmachine_server/semantic_memory/storage/storage_base.py Outdated
Comment thread packages/server/server_tests/memmachine_server/main/test_memmachine_mock.py Outdated
@malatewang

Copy link
Copy Markdown
Contributor Author

On the final-state review summary (#1707 (review)): the documentation gap for missing_episode_grace_period_sec is valid. One naming correction: docs/open_source/configuration.mdx currently documents ingestion_trigger_age as an HH:MM:SS duration, matching the YAML configuration model. ingestion_trigger_age_seconds is the API update field, not the key used in that documentation section. I propose adding missing_episode_grace_period_sec as a numeric seconds field to both YAML samples and the parameter table, while keeping the existing age key unchanged.

@malatewang
malatewang merged commit ad8ff24 into main Oct 7, 2026
54 checks passed
@malatewang
malatewang deleted the episode_uuid branch October 7, 2026 03:21
edwinyyyu added a commit to edwinyyyu/MemMachine that referenced this pull request Oct 7, 2026
…UUIDs and scores, and refuse invalid inputs (MemMachine#1733)

* Look up the segments that derivatives belong to in the segment store

Event memory resolves a search hit, a derivative's vector record, to the
derivative's segment through a `_segment_uuid` property it copies onto
every vector record. The segment store already holds that mapping, on the
derivative's own row.

`SegmentStorePartition.get_segment_uuids_by_derivative_uuids` reads it.
The SQLAlchemy partition answers with one query on the derivative table,
served by its primary key, and checks the partition's liveness in the same
statement, as its other reads do. A derivative the partition does not hold
is left out of the result. The in-memory partition the event memory tests
use implements it too.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Resolve event memory's search hits through the segment store

A derivative's vector record carried its segment's UUID as the
`_segment_uuid` property, and a search read it back from each match. That
copy is only as fresh as the vector store's reads, and the segment store
holds the same mapping on the derivative's row.

A search now asks the vector store for no properties and maps the matched
derivatives to their segments with one
`get_segment_uuids_by_derivative_uuids` call. A match whose derivative the
segment store no longer holds, because its segment was deleted and its
vector outlived it, is dropped. Vector records no longer carry
`_segment_uuid`, and the schema event memory expects of its collection no
longer declares it; a collection created with it keeps declaring it.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Resolve semantic search hits through a vector_uuid column on the feature row

A feature's vector record was keyed by a UUID derived from the feature's
id, and carried eleven properties copied from the feature's row, among
them `feature_id`, which a search read back to resolve a hit to its row.
The caller's feature metadata was merged into the same properties.
- `update_feature` read the stored vector back through
  `VectorStoreCollection.get` to write the record again with fresh
  properties, since an upsert replaces a whole record. The value read is
  written back, so a stale read becomes a lasting wrong write: of two
  concurrent updates of one feature, one with a new embedding and one
  without, the second can write the old embedding over the new one; and a
  read that misses the record fails the update after its row was
  committed (MemMachine#1721).
- Nothing filtered on the copied properties; the row is their authority.

The feature row now carries `vector_uuid`, a fresh UUID that keys its
vector record, and the record carries no properties. A search resolves
its hits through that column, in the order the search returned them,
dropping a hit whose row is gone. `update_feature` writes the vector
store only when given a new embedding, and reads nothing back. The delete
paths take the UUIDs of exactly the rows they delete with RETURNING. The
collection declares no indexed properties.

Breaking: a feature table created before this has no `vector_uuid`
column, and the table is created with `create_all`, which adds none; and
a collection created with the old declared properties no longer matches
the configuration `open_or_create_collection` asks for. No migration is
included.

Fixes MemMachine#1721.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Answer a vector store query with record UUIDs and scores; remove get and close_collection

A query returned each match's record: its properties, and its vector on
request. They were copies of what the callers' own stores hold, only as
fresh as the vector store's reads, and since the previous two commits no
caller reads them. `get` has had no caller since semantic memory stopped
reading its vectors back, and a `get` safe to write from would need every
backend to reflect every write that returned before it, from any process.
Every store's `close_collection` did nothing, and nothing called it.

- `QueryMatch` carries a score and a `record_uuid`, and `query` loses
  `return_vector` and `return_properties`. Properties are still stored
  and filtered on.
- `VectorStoreCollection.get` and `VectorStore.close_collection` go, with
  what only served them: the stores' record parsing,
  `VectorSearchEngine.get_vectors`, and sqlite-vec's vector decoding.
- `Record` is input-only, so its vector is required and its properties
  default to `{}`. The model rejects a missing vector at construction, so
  the stores' checks for one and their coalescing of `None` properties go.

Mechanical: event memory and semantic memory read `match.record_uuid` in
place of `match.record.uuid` and stop passing the flags, and the
in-memory test collection follows the interface.

Tests of the return flags, of `get`, and of values read back through a
query go. Tests that check what a store holds read the backend past the
store: the SQLite stores' records tables, a Qdrant scroll of the
collection's partition, a Milvus `get` on the client. The tests of a
store refusing a `None` vector become tests that the model refuses a
missing one, plus one that properties default to `{}` and one that a
record without properties is stored.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Refuse vector store inputs no collection can hold or search

Each store refuses, with a ValueError and before any backend call:
- a record whose declared property holds a value of another type, since
  the collection indexes and filters the property as its declared type
  (`require_declared_types`: the value's type is the declared type, and a
  bool is not an int);
- a record vector or a query vector whose width is not the collection's
  dimensions;
- a query vector with a coordinate that is not finite, or a score
  threshold that is not finite.

The model checks a record's own vector for finiteness: `Record.vector` is
`list[FiniteFloat]`, so pydantic refuses a NaN or infinite coordinate when
the record is built, in the pass that already validates each coordinate.
A query vector is a plain sequence, so the stores check it. An embedding
endpoint can return NaN whatever the caller does, so both kinds of vector
are checked.

Qdrant and Milvus check a filter's property keys with the other inputs,
before the early return for no query vectors.

`validate_identifier` uses `fullmatch`: `$` also matches before a trailing
newline, so `"name\n"` passed as a namespace, a collection name or a
property key.

The ABC states these refusals in `upsert` and `query`. Tests: each store
refuses each input, and a record refuses a coordinate that is not finite.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* State the vector store's consistency in its contract

The contract said nothing about what a query sees of earlier writes.
`VectorStoreCollection` now states what every store keeps:

  An `upsert` or `delete` is durable once it returns; queries may not
  reflect it right away. A store that guarantees more states it.

Neither event memory nor semantic memory reads its own writes back
through the vector store: a search's hits resolve through the segment
store or the feature row, which hold the mapping. Milvus reads at the
consistency level its collections are configured with, and a replicated
Qdrant may answer a query from a replica that has not applied a write
yet, so a stronger promise would bind every store to read settings with
costs of their own.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Name the derivative-to-segment map segments_by_derivatives

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Drop a None check no property reaches, and a filter branch no caller takes

- The Qdrant store's payload skipped a None property, but a Record's
  properties are PropertyValues, which exclude None, so the model refuses
  one before the store sees it.
- Semantic storage's _apply_feature_filter took a Select or a Delete, but
  only selects reach it: delete_feature_set filters its DELETE ...
  RETURNING itself. It takes and answers a Select, and the cast at its
  caller goes.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Tidy the tests the review found out of place

- The SQLite stores' four input-validation tests sat inside TestFilters,
  under a "Delete" banner meant for TestDelete; they move into
  TestInputValidation, and the banner moves above TestDelete.
- Two event memory schemas still declared `_segment_uuid`, which event
  memory no longer reserves: the context test declares `_timestamp`
  alone, and the missing-base-field test declares nothing.
- The 40,000-feature test's comment says which SQLite the bind limit is
  the feature store's.

Tests only; the same tests run and pass.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Fold the declared-type check into the vector store utilities

declared_properties.py held one function beside utils.py, which holds
the other input checks every store runs (dimensions, query vector,
score threshold, identifiers). It imports only common.data_types,
which imports nothing back, so it moves into utils.py as is.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Resolve semantic search hits in one statement

Since queries answer record UUIDs alone, a semantic search mapped its
hits to feature ids in one select, then loaded the features by id with
the filter in another: two statements in two sessions, and the first
ran even when the search found nothing. One select on the vector_uuid
column with the filter now loads the features, ordered as the hits
were, and no select runs for a search with no hits. Results are
unchanged: the same features, in the same order, under the same
filter.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Refuse a query limit that is not positive

The contract said nothing of a limit at or below zero, and the stores
disagreed: the SQLite stores and Milvus answered empty results, and
Qdrant passed the limit on for the server to reject. A limit that is
not positive is now a ValueError in every store, stated in the query
contract beside the other refused inputs, and checked whether or not
there are query vectors.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Refuse a search for no memories before it does any work

A search whose top_k is not positive asks for nothing, yet it reached
the vector store, which refuses a limit that is not positive, only
after event memory had embedded the query: a request for nothing still
cost an embedding call. SearchMemoriesSpec.top_k is now positive
(gt=0), so request validation refuses it with a 422 naming top_k, the
MCP tool included, and LongTermMemory.search_scored refuses a
num_episodes_limit that is not positive before the query is embedded,
for callers of the Python API. The event backend's window test no
longer runs a limit of 0 on the in-memory vector fake, which does not
check limits; its lower bound is now exercised with a negative
expand_context.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

* Show the positive top_k in the OpenAPI document

docs/openapi.json, as docs/tools/generate_openapi.py writes it for the
search specification's top_k: exclusiveMinimum 0 and the description.
The generator's other differences under the locked FastAPI are left to
the regeneration that carries them.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Rebased onto main. Main refuses a top_k that is not positive since MemMachine#1740,
with ge=1 and its own tests, so this commit keeps that bound and those tests
in place of the gt=0 bound and the test described above, and
docs/openapi.json shows minimum 1. vector_store_semantic_storage.py keeps
importing datetime, which the history methods MemMachine#1707 added use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants