Uh oh!
There was an error while loading. Please reload this page.
Add the three contract members one OpenHuman migration needs - #90
Conversation
…te is wrong Two OpenHuman handlers — `memory.recall_context` and `memory.recall_memories` — still call the engine directly because the contract has no recency path. `MemoryRetrieval::recall_namespace_scored` looks like the twin and is not. The two share a prefix (`load_documents_for_scope` + `kv_records_for_scope`) and diverge after it. The scored path ranks candidates against the query; handed an empty string it still runs the ranking, with nothing to rank against. It does not degrade to recency — it returns hits ordered by a similarity signal computed from nothing. That makes the substitution worse than a missing method: it compiles, returns plausible hits, and quietly changes what the user gets back. So this adds `recall_namespace_recent(namespace, limit)`, resolving to the engine's `recall_namespace_memories`, which orders by freshness and priority. Same `NamespaceMemoryHit` shape as the scored path, so a host re-ranking on engine signals treats both uniformly. Required rather than defaulted, like every other member of this family. A default returning an empty vector would be indistinguishable from a namespace with nothing in it — the silent-empty failure this contract has been bitten by before. The null provider refuses with `Unsupported(Retrieval)`, as its siblings do. Registered in all four lists — the interface block, the module manifest, `METHODS` with its length (94 -> 95), and the loader's `EXPECTED_METHODS`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Warning Your free Security trial is over. An organization admin can activate billing to continue. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
How this change flows1 changed behaviour across 8 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 39 further behaviours left out to keep the diagram readable. flowchart LR
n0["...hod_fails_with_its_advertised_family_name<br/>changed"]:::changed
n1["call"]:::impacted
n2["assert"]:::impacted
n3["Result"]:::impacted
n4["assert_unsupported"]:::impacted
n5["...nt_opens_reuse_the_registered_object_path"]:::impacted
n6["store"]:::impacted
n0 -->|calls| n2
n0 -->|calls| n4
n0 -->|tests| n4
n1 -->|uses| n3
n4 -->|uses| n3
n5 -->|calls| n1
n5 -->|tests| n1
n6 -->|uses| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
…here OpenHuman's `reset_tree` and `flush_now` handlers are the last queue call sites, and neither is a thin call the existing families can express. Both drive raw SQL against `mem_tree_*` from the host — tables this engine owns — so giving them contract members means the logic moves here rather than a signature being added over it. `flush_pending` answers a "flush now" control. Deduplication is the driver's, keyed on date and three-hour block, so two presses inside one window schedule the work once. `FlushOutcome` carries the buffer count beside `enqueued` because either number alone misleads: `enqueued: false` is ambiguous between "nothing to flush" and "already scheduled", and a caller showing the first when it is the second is lying about state the user is watching. `reset_derived_index` discards summaries, buffers, entity indexes and trees, then schedules their re-derivation. It is one operation on purpose — deleting the derived rows without queueing the rebuild leaves a store that answers structural queries with nothing and looks healthy doing it. `mem_tree_chunks` is deliberately not in the table list: the chunks are the source, they are never deleted, and the conformance test asserts the chunk count is unchanged across a reset. A reset that took them too would be data loss wearing the word "reset". Two transactions rather than one, also on purpose: the first drops the job table, so re-enqueueing inside it would race its own truncation. `ResetOutcome` carries three counts because they are not a ratio — rows deleted, chunks returned to scope, and jobs scheduled are independent, and collapsing any pair loses the ability to tell "nothing to re-derive" from "re-derivation was not scheduled". Jobs can be fewer than chunks because the enqueue is keyed. The null provider refuses both rather than taking the trait defaults. The defaults are right for a real driver with nothing buffered or nothing derived; they are wrong for a provider that stores nothing, where "flushed nothing" and "reset nothing" would read as work done rather than as a driver that cannot do it. Registration order matters and the test proved it: the interface serves these with their family, so `METHODS`, the manifest and `EXPECTED_METHODS` list them before `RecallNamespaceRecent`. `the_served_members_are_exactly_the_published_contract` compares sequences, not sets, and caught the mismatch on the first run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two additions the archivist migration cannot ship without, batched here so one release serves the whole OpenHuman PR. `IngestItem` gains `author`, `channel_label` and `platform`, all optional and serde-defaulted. The chat mapping used to manufacture all three: every message was attributed to the batch's owner, the display label collapsed into the dedupe key, and the platform string was rewritten to the enum's name. For single-speaker sources that was merely redundant; for an agent session — owner = the session the memory belongs to, author = the speaking role — it destroys role attribution in the stored transcript, and the platform rewrite would silently change what is on disk for a caller that has always written its own value. Each field falls back to exactly the old behaviour when absent, so existing callers store byte-identical rows; conformance now sets all three so the mapping is exercised rather than trusted. `MemoryEpisodic::insert_event` records one extracted event against its segment. Events are segment-derived episodic artifacts — a summariser reads a closed segment and records the durable facts it found — and the record arrives fully formed because the extraction policy is the caller's, not the driver's. The id is an upsert key, so re-running extraction replaces its own rows rather than duplicating them; conformance pins that. `EventKind` mirrors the engine's `event_log` vocabulary on the wire. Registered in all four lists (METHODS 97 -> 98), and the member checks earned their keep again: the first run failed with "declared in the manifest but not served" because the forwarder edit had not landed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The archivist migration surfaced a linkage the contract silently dropped: the engine's `segment_create` and `segment_append_turn` take a per-session `seq`, and the driver was pinning it to `None` with a comment declaring it "not part of this contract". It has to be. The md-backed archivist store rounds timestamps to milliseconds, which can move a fast turn just before its segment's higher-precision start time — the sequence is the identity that survives the rounding, and segment selection prefers it. A host migrated onto members that cannot carry it would silently degrade every segment filter to the timestamp fallback. `create_segment` gains `start_seq: Option<u32>`, `append_turn` gains `seq: Option<u32>`, and `ConversationSegment` carries the pair back out (serde-defaulted, so older payloads still decode). Conformance pins the round trip: create with `Some(1)`, append with `Some(2)`, read both back off `open_segment`. Changing two members' arity is safe here for the same reason it usually is not: no released artifact will ever face a host built against the widened signatures — hosts pin exact digests and re-pin in lockstep with the release that carries this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Adds
MemoryRetrieval::recall_namespace_recent(namespace, limit)— the recency path, which the contract has never had.This is the gap
openhuman's direct-engine-refs ratchet names as the next upstream ask, and it is the one where the obvious migration is actively harmful rather than merely absent.Why
recall_namespace_scoredis not the twinIt looks like the same call with the query left blank. It is not.
recall_namespace_scored(ns, query, …)query_namespace_hits_excluding_sessionqueryrecall_namespace_recent(ns, limit)recall_namespace_memoriesThe two share a prefix —
load_documents_for_scope+kv_records_for_scope— and diverge after it. Passing""to the scored path does not degrade to recency: it runs the ranking with nothing to rank against and returns hits ordered by a similarity signal computed from nothing.That is why this needed a member rather than a caller convention. The substitution compiles, returns plausible hits, and quietly changes what the user gets back — the worst shape a missing method can take. Two OpenHuman handlers (
memory.recall_context,memory.recall_memories) are blocked on exactly this, and migrating them onto the scored path would have shipped that silent change.Shape
MemoryRetrievalmember is required, and the null provider answersUnsupported(Retrieval). A default returning an empty vector would be indistinguishable from "this namespace is empty" — the silent-empty failure this contract has been bitten by before (see Module bus embedder advertises non-zero dimensions while embedding to empty vectors, so recall silently returns nothing #84).NamespaceMemoryHit, the same shape as the scored path, so a host re-ranking on engine signals treats the two uniformly rather than special-casing one.Registration
All four lists, per the lesson from #86 — the compiler checks only the array length:
#[tinybus::interface]block intinymemory-module/src/service/mod.rs(size-checked like the other list-returning members)methodsintinymemory-module/src/lib.rstinymemory_bus::METHODS, 94 → 95, and the crate doc naming the countEXPECTED_METHODSintests/module_e2e.rsValidation
cargo fmt --all -- --check— both workspacescargo clippy --all-targets --all-features -- -D warnings— root; module workspace likewisecargo test -p tinymemory-tinycortex --test full_provider_conformance— 14 passed, including the newrecency_recall_answers_without_a_querycargo test --manifest-path crates/tinymemory-module/Cargo.toml --lib— 49 passed, includingthe_served_members_are_exactly_the_published_contractdlopen'ed release cdylib, one process per test:every_declared_method_is_actually_routed,the_manifest_declares_every_method_the_module_serves,the_module_advertises_the_complete_tinymemory_api— all passNo dependency changes, so neither lockfile moves.
Second commit: the queue's two compound operations
Extended rather than split into another PR, because these three members are what one OpenHuman PR needs to migrate its remaining queue and recall call sites — landing them separately would cost two releases and two registry re-pins for one migration.
reset_treeandflush_noware the last queue call sites in OpenHuman, and neither is a thin call over an existing family. Both drive raw SQL againstmem_tree_*from the host — tables this engine owns — so a contract member means the logic moves here, not that a signature is added over it.flush_pending() -> FlushOutcomeDeduplication is the driver's, keyed on date + three-hour block, so two presses of a "flush now" control inside one window schedule the work once.
FlushOutcomecarriesstale_buffersbesideenqueuedbecause either alone misleads:enqueued: falseis ambiguous between nothing to flush and already scheduled, and a caller showing the first when it is the second is lying about state the user is watching. Pinned byflushing_twice_in_a_window_schedules_the_work_once.reset_derived_index() -> ResetOutcomeDiscards summaries, buffers, entity indexes and trees, then schedules re-derivation. One operation on purpose — deleting the derived rows without queueing the rebuild leaves a store that answers structural queries with nothing and looks healthy doing it.
mem_tree_chunksis deliberately not in the table list. The chunks are the source; they are never deleted.resetting_the_derived_index_keeps_the_chunks_it_derives_fromasserts the chunk count is unchanged across a reset — a reset that took them too would be data loss wearing the word "reset".Two transactions rather than one, also deliberate: the first drops the job table, so re-enqueueing inside it would race its own truncation.
ResetOutcomecarries three counts because they are not a ratio. Rows deleted, chunks returned to scope, and jobs scheduled are independent; collapsing any pair loses the ability to tell nothing to re-derive from re-derivation was not scheduled. Jobs can be fewer than chunks — the enqueue is keyed.Null provider
Refuses both rather than taking the trait defaults. The defaults are right for a real driver with nothing buffered or nothing derived; they are wrong for a provider that stores nothing, where "flushed nothing" and "reset nothing" read as work done rather than as a driver that cannot do it.
The registration order caught a real mismatch
the_served_members_are_exactly_the_published_contractcompares sequences, not sets, and failed on the first run: the interface serves the two Maintenance members with their family, whileMETHODSlisted them after the Retrieval one. Fixed in all three lists. This is the check #86's commit message said would be the one that matters, and it was.Updated validation
flushing_twice_in_a_window_schedules_the_work_onceandresetting_the_derived_index_keeps_the_chunks_it_derives_fromevery_declared_method_is_actually_routed,the_manifest_declares_every_method_the_module_serves,the_module_advertises_the_complete_tinymemory_api,query_and_maintenance_families_dispatch_typed_requests— all pass at 97 members-D warnings, both workspacesStill not here
The chunk-store and engine-handle buckets (~96 refs between them in OpenHuman).
with_connectionhands out a SQLite handle, which no engine-neutral contract can promise — that is a subsystem move behind the bus, not a member, and it is the multi-thousand-line pivot tracked separately.Deliberately not in this PR
The other two gaps behind openhuman's remaining queue call sites —
reset_treeandflush_now— are not thin methods over an existing family. Both are compound operations that today reachwith_connectionand drive raw SQL againstmem_tree_*from the host. Giving them contract members means moving that logic into the engine, which is a design pass rather than a signature, and folding it in here would make neither half reviewable. Filed as follow-up work rather than half-done alongside this.