Repository navigation
refactor(memory): remove global+topic trees in favor of source trees - #3059
Merged
senamakel merged 11 commits intoMay 31, 2026
Merged
Conversation
…per day The global tree is a true singleton in the DB (UNIQUE(kind,scope)), but its on-disk summaries were laid out as wiki/summaries/global-<yyyy-mm-dd>/L<level>/, so every end-of-day digest (L0) and every higher-level seal (keyed by its range-start date) wrote into a brand-new date-stamped folder. In an Obsidian vault that reads as a fresh "global tree" every single day, shattering one logical tree into N look-alike folders. Lay the global tree out like source/topic trees instead: a single wiki/summaries/global/L<level>/ folder, with the day/week/month/year grouping carried by the L<level> subdir and each node's time range — never the folder name. Drop the now-purposeless date_for_global parameter from summary_rel_path, summary_abs_path, and stage_summary, and update all call sites + tests. Migration of existing global-*/ folders follows in a subsequent commit.
…p to source trees First stage of deleting the global (time-axis) and topic (subject-axis) memory trees in favour of the source trees, which already hold all content. - Drop the query_global / query_topic JSON-RPC methods, schemas, handlers, and the memory_tree agent-tool modes (search_entities + query_source + drill_down + fetch_leaves + ingest_document + walk remain). - Collapse the query backend to a single source-scope query. - Repoint the two live consumers of the global recap to source trees: agent/tree_loader (session memory prefetch) and the subconscious situation-report recap window now walk per-source summaries across the window via query_source. - Trim the topic/global-specific retrieval benchmark + integration tests. Module deletion, cron/ingest teardown, storage + enum cleanup, and the on-disk/data migration follow in subsequent commits.
…job kinds Removes the production + orchestration for the global (time-axis) and topic (subject-axis) memory trees; source trees plus the entity index remain the substrate. - Delete memory/tree_global/ and memory/tree_topic/ modules. - Remove the daily-digest cron loop + manual trigger/backfill helpers; the scheduler now only enqueues flush_stale for source-tree sealing. - Remove the memory.trigger_digest RPC (schema + handler + rpc body). - Drop the DigestDaily/TopicRoute JobKind variants, their payloads and NewJob constructors, and the per-kind handlers; legacy queue rows of those kinds now parse to an error (drained/purged rather than crashing the loop). - Stop bucket_seal from fanning a source seal out to topic routing. - Stop handle_extract from enqueuing topic_route follow-ups. - Subconscious situation report: drop the global-L0-digest and entity-hotness sections (the hotness signal was a topic-curator byproduct); the source-summaries + repointed recap-window sections remain. - Trim the lib unit/e2e tests (tree_e2e, sync_pipeline_e2e, mod_tests) of their global/topic assertions; keep the source-tree coverage. Lib + lib unit tests compile. Integration-test binaries, the data-migration, and the TreeKind/SummaryTreeKind enum-variant cleanup follow next.
…obal-tree-single-folder
Trim/delete the global+topic-specific integration coverage; keep source-tree coverage. Drop the retired memory_tree_query_global / query_topic / trigger_digest method-registry assertions and the DigestDaily/TopicRoute job payload asserts. Also fixes two stale 3-arg stage_summary call sites in memory_artifacts_e2e and memory_sync_tree_round21 left by the global-folder fix. All test targets compile (cargo test --no-run clean).
Adds purge_global_topic_trees, a version-gated (PRAGMA user_version=2) one-shot migration wired into the memory_tree DB init right after the tinyhumansai#1574 embedding migration. It deletes the orphaned global/topic tree rows, their summaries + sidecar vectors + entity-index rows + buffers, drains any leftover topic_route/digest_daily job rows, and removes the on-disk wiki/summaries/global*/ (both legacy per-day folders and the singleton) and topic-*/ summary folders. Source trees and non-retired jobs are untouched; filesystem errors are best-effort so they can't wedge the version bump. Covered by a unit test asserting selective deletion + the run-once gate.
Remove get_or_create_global_tree / get_or_create_topic_tree / force_create_topic_tree / list_topic_trees / archive_topic_tree — all now have zero callers, and keeping the creation wrappers would leave an easy path to resurrect the removed trees. The generic list_trees_by_kind / archive_tree stay. The TreeKind::Global/Topic enum variants are intentionally retained as inert serialization plumbing so the purge migration can still read legacy rows; they are no longer constructible via any helper.
…tions - legacy_embeddings_migrate_to_sidecar_once: a full init now runs the global/topic purge after the embedding migration, so the user_version gate lands on GLOBAL_TOPIC_PURGE_MIGRATION_VERSION (2), not 1. - stage_summary_global_uses_singleton_folder_no_date: assert the L1 path that mk_summary_input (level=1) actually produces; the point is the singleton global/ folder with no date segment.
turn_runs_full_tool_cycle_with_context_and_hooks and turn_uses_cached_transcript_prefix_on_first_iteration asserted exact memory context, but turn() runs the memory-tree eager prefetch against the real ~/.openhuman workspace via load_config_with_timeout (not the injected loader). They only passed because the prior query_global path was empty there; the repoint to query_source now surfaces real source-tree summaries. Pre-set last_tree_prefetch_at so should_prefetch is false — these tests target the injected memory context + tool cycle, not the prefetch, so skipping it is deterministic and removes the latent home-dir dependency.
bench_cross_chat_recall (8.3.1), bench_stale_preference_newer_supersedes (8.3.5), bench_contradiction_surfaces_both_with_provenance (8.3.6), and bench_drill_down_isolates_children (8.3.8) were deleted with the topic tree; remove their now-dangling matrix rows.
Contributor
📝 WalkthroughWalkthroughThis PR removes Global and Topic tree projections from the memory system, consolidating to Source trees only. The entity index now reconstructs cross-cutting queries. Includes removal of ChangesGlobal and Topic Tree Removal and Source-Only Consolidation
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
|
coderabbitai
Bot
added
memory
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gitbooks/developing/architecture/memory-tree.md (1)
35-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove stale
memory::tree_{global,topic}ownership references.This section still says policy/re-exports live in
memory::tree_globalandmemory::tree_topic, which conflicts with the source-only architecture described above and in retrieval docs. Please reword these lines to reference source-tree/orchestrator policy only (or mark those paths explicitly as legacy-only if they still exist as shims).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gitbooks/developing/architecture/memory-tree.md` around lines 35 - 50, Update the paragraph that describes re-exports in mod.rs: remove or reword the claim that policy/re-exports live in memory::tree_global and memory::tree_topic and instead state that policy and source-tree orchestration live in the orchestrator/source-tree layer (e.g., memory::tree_{global,topic} behavior is handled by the orchestrator), or explicitly mark memory::tree_global and memory::tree_topic and the legacy memory_tree::tree_{global,topic} paths as legacy shims only; make the change where mod.rs and related re-export text reference those symbols so it aligns with the "No policy here" invariant and the retrieval docs.src/openhuman/memory/schema.rs (1)
852-853:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop swallowing invalid RPC params via default fallback.
These handlers currently convert deserialization failures into default requests, so malformed inputs can succeed with unintended behavior instead of returning
invalid params.Proposed fix
fn handle_list_sources(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { @@ - let req = parse_value::<Req>(Value::Object(params)).unwrap_or_default(); + let req = parse_value::<Req>(Value::Object(params))?; to_json(read_rpc::list_sources_rpc(&config, req.user_email_hint).await?) }) } @@ fn handle_graph_export(params: Map<String, Value>) -> ControllerFuture { @@ - let req = parse_value::<Req>(Value::Object(params)).unwrap_or_default(); + let req = parse_value::<Req>(Value::Object(params))?; to_json(read_rpc::graph_export_rpc(&config, req.mode.unwrap_or_default()).await?) }) } @@ fn handle_obsidian_vault_status(params: Map<String, Value>) -> ControllerFuture { @@ - let req = parse_value::<Req>(Value::Object(params)).unwrap_or_default(); + let req = parse_value::<Req>(Value::Object(params))?; to_json(read_rpc::obsidian_vault_status_rpc(&config, req.obsidian_config_dir).await?) }) }Also applies to: 953-954, 966-967
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/schema.rs` around lines 852 - 853, The handler currently swallows deserialization failures by calling parse_value::<Req>(...).unwrap_or_default(), causing malformed input to be treated as valid; instead, change these call sites (the parse_value::<Req> usage that feeds read_rpc::list_sources_rpc and the similar occurrences around the other reported locations) to propagate a deserialization error back to the JSON-RPC layer: attempt parse_value::<Req>(Value::Object(params)) and if it Err, convert/map that error into an "invalid params" RPC error and return it rather than falling back to Default::default(); on success pass req.user_email_hint into read_rpc::list_sources_rpc as before.
🧹 Nitpick comments (7)
src/openhuman/memory_queue/handlers/mod.rs (2)
286-292: 💤 Low valueStale comment referencing removed
topic_routejob.The comment at lines 287-291 references "topic_route" being "advisory" and the source-tree path, but
TopicRoutejobs have been removed. Consider updating this comment to reflect that only source-tree appends remain.📝 Suggested comment update
let Some(tree) = tree else { - // Target topic tree doesn't exist (e.g. archived between - // topic_route and this append). Drop on the floor — the - // topic_route was advisory and the source-tree path already - // ran for this leaf. + // Target tree doesn't exist (e.g. archived). Drop the job — + // source-tree is the only remaining tree kind and stale + // references can be safely discarded. return Ok(JobOutcome::Done); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_queue/handlers/mod.rs` around lines 286 - 292, Update the stale comment inside the match that checks `let Some(tree) = tree else { ... return Ok(JobOutcome::Done); }` to remove references to the removed `TopicRoute` job and instead clearly state that the target topic tree may no longer exist (e.g., archived between routing and this append) and that this handler now only deals with source-tree appends, so it safely drops the job and returns `JobOutcome::Done`.
393-398: 💤 Low valueStale comment referencing removed
digest_dailyhandler.Line 395-396 states "Global trees never reach here —
digest_dailyhandles them" butdigest_dailyhas been removed along with global trees. Update this comment to reflect the current architecture where only source trees exist.📝 Suggested comment update
// Pick the labeling strategy for this tree kind. Source trees mint // emergent themes via the seal-time extractor; topic trees stay empty - // by design (scope already pins the canonical id). Global trees never - // reach here — `digest_daily` handles them — but Empty is a safe - // defensive default. + // by design (scope already pins the canonical id). With global/topic + // trees removed, only source trees remain, but the label strategy + // abstraction is retained for extensibility. let strategy = TreeFactory::from_tree(&tree).label_strategy(config);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_queue/handlers/mod.rs` around lines 393 - 398, Comment references removed digest_daily and global trees; update the comment near the TreeFactory::from_tree(&tree).label_strategy(config) call to reflect that only source trees are present and that topic trees remain empty by design, removing any mention of digest_daily or global trees and clarifying that Empty is a defensive default for unexpected kinds. Locate the comment immediately above the label_strategy invocation and replace the stale sentence with a concise note about current architecture (source trees mint via seal-time extractor; topic trees stay empty; Empty is a defensive fallback).src/openhuman/memory_queue/mod.rs (1)
12-22: ⚡ Quick winModule docstring describes removed job kinds.
The ASCII diagram at lines 14-22 still references
topic_routeanddigest_dailyjob kinds, and mentions the "daily wall-clock tick" scheduler behavior that has been removed. Update this docstring to reflect the simplified architecture where onlyFlushStaleis scheduled periodically.📝 Suggested docstring update
//! ```text //! ingest::persist //! └── writes chunk row (lifecycle = pending_extraction) //! enqueues `extract_chunk` //! //! worker pool (3 tasks) ──► claims jobs by kind: //! extract_chunk → LLM extraction → admission decision → enqueue append_buffer -//! append_buffer → push to L0 → enqueue seal if gate met → enqueue topic_route +//! append_buffer → push to L0 → enqueue seal if gate met //! seal → seal one level → enqueue parent seal if cascading -//! topic_route → match topics → enqueue per-topic append_buffer -//! digest_daily → call tree_global::digest::end_of_day_digest //! flush_stale → enqueue seals for time-stale buffers +//! reembed_backfill → re-embed chunks/summaries at active signature //! -//! scheduler (1 task) ──► daily wall-clock tick: -//! enqueues digest_daily(yesterday) + flush_stale(today) +//! scheduler (1 task) ──► 3-hour periodic tick: +//! enqueues flush_stale so low-volume L0 buffers seal promptly //! ```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_queue/mod.rs` around lines 12 - 22, Update the module docstring to remove references to the removed job kinds (topic_route and digest_daily) and the old “daily wall-clock tick” scheduler; reflect the current job kinds and scheduler behavior by listing extract_chunk, append_buffer, seal, flush_stale, and reembed_backfill (if present) and describe the scheduler as a 3-hour periodic tick that enqueues FlushStale so low-volume L0 buffers seal promptly; ensure symbols like extract_chunk, append_buffer, seal, FlushStale (or flush_stale), and reembed_backfill are mentioned exactly as in the code to make the mapping clear.src/openhuman/memory_queue/types.rs (1)
175-183: 💤 Low valueAppendTarget::Topic is still used; clarify the docstring instead of treating it as dead code.
File:
src/openhuman/memory_queue/types.rs(175-183, also 200-203)
AppendTarget::Topic { tree_id }is actively constructed and matched/handled (e.g., intests/memory_threads_raw_coverage_e2e.rs,src/openhuman/memory_queue/types.rs, andsrc/openhuman/memory_queue/handlers/mod.rsviaget_tree(config, tree_id)), so it’s not dead code.- If the docstring wording implies a relationship to the retired topic-route job types, update it to clearly state this variant’s purpose as the destination for topic-tree appends (including any legacy/serde rationale if applicable).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory_queue/types.rs` around lines 175 - 183, The doc comment above the AppendTarget enum incorrectly implies the Topic variant is dead/related to retired topic-route jobs; update the comment to clearly state that AppendTarget::Topic { tree_id: String } is actively used as the destination for topic-tree appends (distinct from Source { source_id }) and note any legacy/serde rationale (e.g., preserved shape for backward compatibility) so readers understand why Topic exists; ensure the comment references AppendBuffer jobs and how handlers like get_tree(...) consume the tree_id so future maintainers won't treat Topic as dead code.src/openhuman/memory/schema.rs (1)
836-1005: 🏗️ Heavy liftAdd structured debug/trace diagnostics to the new handler paths.
The new/changed controller flows are missing debug/trace logs with stable prefixes and correlation fields, which makes runtime diagnosis harder.
As per coding guidelines: In Rust, default to verbose diagnostics on new/changed flows using
log/tracingatdebug/tracelevels with stable grep-friendly prefixes and correlation fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/memory/schema.rs` around lines 836 - 1005, The new handler functions (e.g., handle_list_chunks, handle_list_sources, handle_search, handle_recall, handle_entity_index_for, handle_chunks_for_entity, handle_top_entities, handle_chunk_score, handle_delete_chunk, handle_graph_export, handle_obsidian_vault_status, handle_flush_now, handle_wipe_all, handle_reset_tree, handle_pipeline_status, handle_set_enabled) lack structured debug/trace diagnostics; add tracing/log statements at entry, after parsing the request, and around the RPC call using a stable, grep-friendly prefix like "controller:<handler_name>" and include correlation fields (e.g., a request id or a config identifier) and key request parameters (but not secrets); ensure logs use trace/debug levels and log both success (with summary) and error paths (with error details) so each handler emits consistent diagnostics for runtime troubleshooting.tests/agent_retrieval_e2e.rs (1)
304-310: 💤 Low valueStale doc comment references
query_topic. The test no longer derives chunk ids fromquery_topic; it lists chunks directly. Update the comment to avoid misleading future readers.📝 Suggested wording
-/// This is the "memory retrieval returns provenance and can hydrate cited -/// chunks" feature (issue#1538): chunk_ids from query_topic are fed into -/// fetch_leaves and each returned leaf must carry `source_ref` when one was -/// set at ingest time. +/// This is the "memory retrieval returns provenance and can hydrate cited +/// chunks" feature (issue#1538): chunk_ids listed directly from the chunk +/// store are fed into fetch_leaves and each returned leaf must carry +/// `source_ref` when one was set at ingest time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/agent_retrieval_e2e.rs` around lines 304 - 310, The doc comment above the test describing fetch_leaves still mentions deriving chunk ids from `query_topic`; update that comment to reflect the current test behavior: state that chunk IDs are listed directly (not derived from `query_topic`) and that the test verifies each hydrated chunk returned by `fetch_leaves` carries a `source_ref` when set at ingest time. Edit the comment text referencing `query_topic` to remove or replace that wording and keep references to `fetch_leaves` and `source_ref` so readers know what the test validates.tests/memory_tree_sync_deep_raw_coverage_e2e.rs (1)
150-227: ⚡ Quick winRemove unused
seed_topic_summary/one_hothelpers frommemory_tree_sync_deep_raw_coverage_e2e
seed_topic_summaryandone_hotare no longer referenced intests/memory_tree_sync_deep_raw_coverage_e2e.rs, so they should be deleted (or explicitly#[allow(dead_code)]) to avoiddead_codewarnings in that test target. Also,TreeKind::Topicis still present insrc/openhuman/memory_store/trees/types.rs, so the “no longer constructible” compile-break concern doesn’t apply here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/memory_tree_sync_deep_raw_coverage_e2e.rs` around lines 150 - 227, Delete the unused helper functions seed_topic_summary and one_hot from tests/memory_tree_sync_deep_raw_coverage_e2e.rs (or alternatively annotate each with #[allow(dead_code)]) to silence dead_code warnings; locate the functions by name (seed_topic_summary and one_hot) and either remove their full definitions or add the allow attribute directly above them, ensuring no other references remain in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/memory/sync_pipeline_e2e_tests.rs`:
- Around line 296-297: Update the test file comments and banners to match the
source-only pipeline by removing or changing references to the removed
global/topic flow: edit the module header and the "Test 2" banner in
src/openhuman/memory/sync_pipeline_e2e_tests.rs so they no longer mention
topic_route or global-digest and instead describe the source-only pipeline
(trees + entity index) used by the tests; search for strings "topic_route",
"global-digest", "global", and the "Test 2" banner to locate the exact comment
blocks to update.
In `@src/openhuman/memory/tree_e2e_tests.rs`:
- Around line 72-82: Update the module-level doc comment above the test in
tree_e2e_tests.rs to reflect the new source-only pipeline: remove or replace any
text claiming the test exercises "all three tree kinds" or references to
global-digest/topic-spawn, and ensure the steps list matches the actual flow
(ingest → seal → source retrieval → entity search) and mentions only source
trees and sealed summaries; adjust any summary sentence to explicitly state this
is a source-only end-to-end pipeline test so readers aren't misled.
---
Outside diff comments:
In `@gitbooks/developing/architecture/memory-tree.md`:
- Around line 35-50: Update the paragraph that describes re-exports in mod.rs:
remove or reword the claim that policy/re-exports live in memory::tree_global
and memory::tree_topic and instead state that policy and source-tree
orchestration live in the orchestrator/source-tree layer (e.g.,
memory::tree_{global,topic} behavior is handled by the orchestrator), or
explicitly mark memory::tree_global and memory::tree_topic and the legacy
memory_tree::tree_{global,topic} paths as legacy shims only; make the change
where mod.rs and related re-export text reference those symbols so it aligns
with the "No policy here" invariant and the retrieval docs.
In `@src/openhuman/memory/schema.rs`:
- Around line 852-853: The handler currently swallows deserialization failures
by calling parse_value::<Req>(...).unwrap_or_default(), causing malformed input
to be treated as valid; instead, change these call sites (the parse_value::<Req>
usage that feeds read_rpc::list_sources_rpc and the similar occurrences around
the other reported locations) to propagate a deserialization error back to the
JSON-RPC layer: attempt parse_value::<Req>(Value::Object(params)) and if it Err,
convert/map that error into an "invalid params" RPC error and return it rather
than falling back to Default::default(); on success pass req.user_email_hint
into read_rpc::list_sources_rpc as before.
---
Nitpick comments:
In `@src/openhuman/memory_queue/handlers/mod.rs`:
- Around line 286-292: Update the stale comment inside the match that checks
`let Some(tree) = tree else { ... return Ok(JobOutcome::Done); }` to remove
references to the removed `TopicRoute` job and instead clearly state that the
target topic tree may no longer exist (e.g., archived between routing and this
append) and that this handler now only deals with source-tree appends, so it
safely drops the job and returns `JobOutcome::Done`.
- Around line 393-398: Comment references removed digest_daily and global trees;
update the comment near the TreeFactory::from_tree(&tree).label_strategy(config)
call to reflect that only source trees are present and that topic trees remain
empty by design, removing any mention of digest_daily or global trees and
clarifying that Empty is a defensive default for unexpected kinds. Locate the
comment immediately above the label_strategy invocation and replace the stale
sentence with a concise note about current architecture (source trees mint via
seal-time extractor; topic trees stay empty; Empty is a defensive fallback).
In `@src/openhuman/memory_queue/mod.rs`:
- Around line 12-22: Update the module docstring to remove references to the
removed job kinds (topic_route and digest_daily) and the old “daily wall-clock
tick” scheduler; reflect the current job kinds and scheduler behavior by listing
extract_chunk, append_buffer, seal, flush_stale, and reembed_backfill (if
present) and describe the scheduler as a 3-hour periodic tick that enqueues
FlushStale so low-volume L0 buffers seal promptly; ensure symbols like
extract_chunk, append_buffer, seal, FlushStale (or flush_stale), and
reembed_backfill are mentioned exactly as in the code to make the mapping clear.
In `@src/openhuman/memory_queue/types.rs`:
- Around line 175-183: The doc comment above the AppendTarget enum incorrectly
implies the Topic variant is dead/related to retired topic-route jobs; update
the comment to clearly state that AppendTarget::Topic { tree_id: String } is
actively used as the destination for topic-tree appends (distinct from Source {
source_id }) and note any legacy/serde rationale (e.g., preserved shape for
backward compatibility) so readers understand why Topic exists; ensure the
comment references AppendBuffer jobs and how handlers like get_tree(...) consume
the tree_id so future maintainers won't treat Topic as dead code.
In `@src/openhuman/memory/schema.rs`:
- Around line 836-1005: The new handler functions (e.g., handle_list_chunks,
handle_list_sources, handle_search, handle_recall, handle_entity_index_for,
handle_chunks_for_entity, handle_top_entities, handle_chunk_score,
handle_delete_chunk, handle_graph_export, handle_obsidian_vault_status,
handle_flush_now, handle_wipe_all, handle_reset_tree, handle_pipeline_status,
handle_set_enabled) lack structured debug/trace diagnostics; add tracing/log
statements at entry, after parsing the request, and around the RPC call using a
stable, grep-friendly prefix like "controller:<handler_name>" and include
correlation fields (e.g., a request id or a config identifier) and key request
parameters (but not secrets); ensure logs use trace/debug levels and log both
success (with summary) and error paths (with error details) so each handler
emits consistent diagnostics for runtime troubleshooting.
In `@tests/agent_retrieval_e2e.rs`:
- Around line 304-310: The doc comment above the test describing fetch_leaves
still mentions deriving chunk ids from `query_topic`; update that comment to
reflect the current test behavior: state that chunk IDs are listed directly (not
derived from `query_topic`) and that the test verifies each hydrated chunk
returned by `fetch_leaves` carries a `source_ref` when set at ingest time. Edit
the comment text referencing `query_topic` to remove or replace that wording and
keep references to `fetch_leaves` and `source_ref` so readers know what the test
validates.
In `@tests/memory_tree_sync_deep_raw_coverage_e2e.rs`:
- Around line 150-227: Delete the unused helper functions seed_topic_summary and
one_hot from tests/memory_tree_sync_deep_raw_coverage_e2e.rs (or alternatively
annotate each with #[allow(dead_code)]) to silence dead_code warnings; locate
the functions by name (seed_topic_summary and one_hot) and either remove their
full definitions or add the allow attribute directly above them, ensuring no
other references remain in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 025580c7-4524-41e0-b71c-e0b93bf9da72
📒 Files selected for processing (58)
docs/TEST-COVERAGE-MATRIX.mdgitbooks/developing/architecture/memory-tree.mdsrc/openhuman/agent/harness/session/turn_tests.rssrc/openhuman/agent/tree_loader.rssrc/openhuman/memory/mod.rssrc/openhuman/memory/query/backend.rssrc/openhuman/memory/query/ingest_document.rssrc/openhuman/memory/query/mod.rssrc/openhuman/memory/query/query_global.rssrc/openhuman/memory/query/query_source.rssrc/openhuman/memory/query/query_topic.rssrc/openhuman/memory/schema.rssrc/openhuman/memory/sync_pipeline_e2e_tests.rssrc/openhuman/memory/tree_e2e_tests.rssrc/openhuman/memory/tree_global/digest.rssrc/openhuman/memory/tree_global/mod.rssrc/openhuman/memory/tree_global/recap.rssrc/openhuman/memory/tree_global/seal.rssrc/openhuman/memory/tree_topic/backfill.rssrc/openhuman/memory/tree_topic/curator.rssrc/openhuman/memory/tree_topic/hotness.rssrc/openhuman/memory/tree_topic/mod.rssrc/openhuman/memory/tree_topic/routing.rssrc/openhuman/memory_queue/handlers/mod.rssrc/openhuman/memory_queue/handlers/mod_tests.rssrc/openhuman/memory_queue/mod.rssrc/openhuman/memory_queue/scheduler.rssrc/openhuman/memory_queue/types.rssrc/openhuman/memory_store/chunks/connection.rssrc/openhuman/memory_store/chunks/store.rssrc/openhuman/memory_store/chunks/store_tests.rssrc/openhuman/memory_store/content/atomic.rssrc/openhuman/memory_store/content/paths.rssrc/openhuman/memory_store/content/read.rssrc/openhuman/memory_store/trees/mod.rssrc/openhuman/memory_store/trees/registry.rssrc/openhuman/memory_tree/retrieval/benchmarks.rssrc/openhuman/memory_tree/retrieval/global.rssrc/openhuman/memory_tree/retrieval/integration_tests.rssrc/openhuman/memory_tree/retrieval/mod.rssrc/openhuman/memory_tree/retrieval/rpc.rssrc/openhuman/memory_tree/retrieval/schemas.rssrc/openhuman/memory_tree/retrieval/topic.rssrc/openhuman/memory_tree/tree/bucket_seal.rssrc/openhuman/memory_tree/tree/bucket_seal_tests.rssrc/openhuman/memory_tree/tree/rpc.rssrc/openhuman/subconscious/situation_report/digest.rssrc/openhuman/subconscious/situation_report/hotness.rssrc/openhuman/subconscious/situation_report/mod.rssrc/openhuman/subconscious/situation_report/query_window.rstests/agent_retrieval_e2e.rstests/json_rpc_e2e.rstests/memory_artifacts_e2e.rstests/memory_core_threads_raw_coverage_e2e.rstests/memory_sync_tree_round21_raw_coverage_e2e.rstests/memory_threads_raw_coverage_e2e.rstests/memory_tree_sync_deep_raw_coverage_e2e.rstests/worker_c_modules_e2e.rs
💤 Files with no reviewable changes (26)
- src/openhuman/memory_tree/retrieval/topic.rs
- src/openhuman/memory_tree/retrieval/global.rs
- src/openhuman/memory/tree_global/recap.rs
- src/openhuman/memory/tree_global/seal.rs
- src/openhuman/memory/tree_topic/routing.rs
- src/openhuman/subconscious/situation_report/hotness.rs
- src/openhuman/memory/query/query_topic.rs
- src/openhuman/memory/tree_topic/curator.rs
- src/openhuman/memory/tree_global/mod.rs
- docs/TEST-COVERAGE-MATRIX.md
- src/openhuman/memory_store/content/read.rs
- src/openhuman/memory/query/query_global.rs
- src/openhuman/memory/tree_topic/hotness.rs
- src/openhuman/memory/tree_topic/backfill.rs
- src/openhuman/subconscious/situation_report/digest.rs
- src/openhuman/memory/tree_global/digest.rs
- src/openhuman/memory/tree_topic/mod.rs
- src/openhuman/memory_queue/handlers/mod_tests.rs
- tests/memory_sync_tree_round21_raw_coverage_e2e.rs
- tests/json_rpc_e2e.rs
- tests/memory_artifacts_e2e.rs
- tests/memory_core_threads_raw_coverage_e2e.rs
- src/openhuman/memory_tree/tree/rpc.rs
- src/openhuman/memory_tree/tree/bucket_seal_tests.rs
- src/openhuman/memory_tree/retrieval/rpc.rs
- tests/worker_c_modules_e2e.rs
Comment on lines
+296
to
+297
| // (The global-digest and topic-spawn steps were removed with those | ||
| // trees — source trees plus the entity index are the substrate.) |
Contributor
There was a problem hiding this comment.
Make test docs consistent with the source-only pipeline.
This new note says global/topic flow is removed, but the module header and Test 2 banner still describe topic_route/global-digest behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/memory/sync_pipeline_e2e_tests.rs` around lines 296 - 297,
Update the test file comments and banners to match the source-only pipeline by
removing or changing references to the removed global/topic flow: edit the
module header and the "Test 2" banner in
src/openhuman/memory/sync_pipeline_e2e_tests.rs so they no longer mention
topic_route or global-digest and instead describe the source-only pipeline
(trees + entity index) used by the tests; search for strings "topic_route",
"global-digest", "global", and the "Test 2" banner to locate the exact comment
blocks to update.
Comment on lines
+72
to
82
| /// Full pipeline: ingest → seal → source retrieval → entity search. | ||
| /// | ||
| /// Steps: | ||
| /// 1. Ingest heavy batches from two distinct sources so the source trees' | ||
| /// L0 buffers cross the token-budget seal threshold. | ||
| /// 2. Drain the async job queue so extract / admit / buffer / seal jobs run. | ||
| /// 3. Verify source-tree retrieval returns sealed summaries. | ||
| /// 4. Run the end-of-day digest and assert it emits a global node. | ||
| /// 5. Verify global retrieval returns at least one hit. | ||
| /// 6. Pre-seed hotness counters and call `force_recompute` to spawn the | ||
| /// topic tree for `email:alice@example.com`. | ||
| /// 7. Verify topic-tree retrieval returns results. | ||
| /// 8. Verify `search_entities("alice")` resolves to alice's canonical id. | ||
| /// 4. Verify `search_entities("alice")` resolves to alice's canonical id. | ||
| /// | ||
| /// (The global-digest and topic-spawn steps were removed with those trees.) | ||
| #[tokio::test] |
Contributor
There was a problem hiding this comment.
Align module-level docs with the updated source-only flow.
The updated step list removes global/topic stages, but the top-of-file description still says this test exercises all three tree kinds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/memory/tree_e2e_tests.rs` around lines 72 - 82, Update the
module-level doc comment above the test in tree_e2e_tests.rs to reflect the new
source-only pipeline: remove or replace any text claiming the test exercises
"all three tree kinds" or references to global-digest/topic-spawn, and ensure
the steps list matches the actual flow (ingest → seal → source retrieval →
entity search) and mentions only source trees and sealed summaries; adjust any
summary sentence to explicitly state this is a source-only end-to-end pipeline
test so readers aren't misled.
Merged
6 tasks done
Merged
9 tasks done
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 6, 2026
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wiki/summaries/global-<yyyy-mm-dd>/folder per day, making one logical tree look like dozens in an Obsidian vault. (Now moot — global gone — but the path/param cleanup ships with it.)query_source.memory.trigger_digestRPC, thequery_global/query_topicretrieval methods + agent-tool modes, the topic-routing fan-out, and theDigestDaily/TopicRoutejob kinds.global*/+topic-*/folders.Problem
The global and topic trees were a second and third copy of retrieval structure layered over the source trees. They added a daily cron, a job-queue fan-out, an entity-hotness curator, extra RPC surface, and per-day folder clutter — but no information that walking the source trees + the entity index can't reconstruct. The global tree's per-day folder layout (
global-2026-05-28/,global-2026-05-29/, …) also actively mislead users into thinking a new tree was created every day.Solution
summary_rel_pathnow lays the (removed) global kind out as a singleglobal/L<level>/folder and drops thedate_for_globalparameter threaded throughstage_summary/summary_abs_path.memory/tree_global/andmemory/tree_topic/; the scheduler now only enqueuesflush_stalefor source-tree sealing.agent/tree_loaderandsubconscious/situation_report/query_windowaggregate recent source-tree summaries across the window (sameQueryResponseshape) instead of reading a pre-folded global node. The hotness-delta + global-L0 situation-report sections (topic-curator byproducts) were dropped.JobKind::parsetolerates legacytopic_route/digest_dailyrows (returns an error so the worker loop drains rather than crashes); the purge migration removes them.TreeKind::Global/Topicenum variants are intentionally retained as inert serialization plumbing so the migration can read legacy rows; no helper can construct them anymore.Submission Checklist
global_topic_purge_removes_only_global_and_topicmigration test (selective-delete + run-once gate); integration + e2e + turn tests updated for the removalpnpm test:rustrun locally (see below)8.3.xrows for the deleted topic-retrieval benchmarks indocs/TEST-COVERAGE-MATRIX.md## Related)Impact
app/srcreferences to the removed RPC methods).PRAGMA user_version=2, best-effort on the filesystem step so it can't wedge the gate).Related
TreeKind::Global/Topicenum variants (deferred — they're load-bearing for DB round-trip tests / the purge migration and deliver no functional value now that the trees are unbuildable).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/global-tree-single-folder123d22ed78fd9932dbb97ccae448228cd6890ae1Validation Run
format:checknot run (Rust-only change;cargo fmtapplied per commit)pnpm typechecknot run (no TypeScript changed)cargo test --lib -- memory(2026 passed), migration + turn tests greencargo fmt -p openhuman+ pre-pushcargo checkpassedValidation Blocked
command:N/Aerror:N/Aimpact:N/A — fullpnpm test:rustrun locally: 10626 passed, 1 pre-existing unrelated flaky (cost::rpc::dashboard_query_includes_persisted_record, a process-global race; green in isolation)Behavior Changes
global-*vault folders disappear; agent/subconscious memory recap is reconstructed from source trees.Parity Contract
memory_tree_recallwas already source-only;query_source+search_entities+drill_down+fetch_leaves+walkunchanged.JobKind::parsetolerates retired kinds; migration is idempotent + version-gated.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Refactor
Tests
Chores