refactor(agent): unwire eager 7-day memory-tree digest from turn loop (#3170)

[sanil-23]

Summary

• Remove the unconditional "last 7 days" memory-tree digest that the orchestrator turn loop injected onto every user message (first turn + every 30 min).
• Drop the per-session last_tree_prefetch_at bookkeeping (field, initializer, two test work-arounds).
• Keep the tree_loader module (still exercised by an e2e test) so an opt-in eager digest can be re-wired behind a proper read-side gate later; its docstring now records that it is currently unwired.

Problem

The agent turn loop (src/openhuman/agent/harness/session/turn.rs) called tree_loader::TreeContextLoader::load on a 30-min cadence and prepended a [Memory tree — last 7 days] digest to the turn context. Two issues:

1. Redundant. Since the on-demand memory-tree retrieval tools landed (smart multi-strategy walk, feat(memory): smart multi-strategy retrieval walk #3077), the agent pulls exactly the memory a query needs. A fixed 7-day window force-fed every session is blunt and wastes prompt budget.
2. Ignored the disable toggle. The memory-tree on/off toggle (memory_tree_set_enabled) only flips scheduler_gate.mode (Auto/Off) — i.e. it pauses ingestion workers. The read/prefetch path in turn.rs had no gate check, so even with the tree toggled off, every session still injected the 7-day digest into the prompt.

Solution

• Delete the eager-prefetch block in turn.rs; the turn context is now just the memory_loader.load_context(...) result.
• Remove last_tree_prefetch_at from the session state (types.rs) and builder (builder.rs).
• Remove the two unit-test work-arounds that pre-stamped last_tree_prefetch_at to suppress the prefetch — the tests now pass deterministically because the path no longer exists.
• Retain tree_loader.rs (its public surface is still covered by tests/inference_agent_raw_coverage_e2e.rs); docstring updated to note it is unwired pending a future read-side gate.

On-demand memory retrieval is unaffected — only the unconditional auto-injection is removed.

Submission Checklist

• Tests added or updated — N/A new tests; this removes behavior. The two affected unit tests (turn_runs_full_tool_cycle_with_context_and_hooks, turn_uses_cached_transcript_prefix_on_first_iteration) were simplified and pass without their prefetch-suppression hack.
• Diff coverage ≥ 80% — changed lines are net deletions plus doc comments; no new executable lines to cover.
• Coverage matrix updated — N/A: no row in docs/TEST-COVERAGE-MATRIX.md references the eager digest.
• All affected feature IDs listed under ## Related — N/A: no matrix feature IDs affected.
• No new external network dependencies introduced.
• Manual smoke checklist — N/A: behavior removal, no release-cut surface change.
• Linked issue closed via Closes #NNN.

Impact

• Desktop/CLI: orchestrator sessions no longer auto-inject a 7-day memory digest; the agent retrieves memory on demand instead. Slightly smaller first-turn prompts.
• Behavior fix: disabling the memory tree now actually keeps its contents out of the prompt (the read path was previously ungated).
• No migration, no schema change.

Related

• Closes: refactor(agent): unwire eager 7-day memory-tree digest from the turn loop #3170
• Follow-up PR(s)/TODOs: optional — re-wire an opt-in eager digest behind a read-side gate that respects scheduler_gate.mode.

---

AI Authored PR Metadata

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: refactor/3170-unwire-tree-prefetch
• Commit SHA: ef20f20

Validation Run

• pnpm --filter openhuman-app format:check — N/A: no frontend changes.
• pnpm typecheck — N/A: no TS changes.
• Focused tests: cargo test --lib tree_loader (5 ok), turn_runs_full_tool_cycle_with_context_and_hooks + turn_uses_cached_transcript_prefix_on_first_iteration (2 ok).
• Rust fmt/check: cargo fmt --check clean; cargo check --tests clean (warnings pre-existing, unrelated).
• Tauri fmt/check — N/A: shell unchanged.

Behavior Changes

• Intended behavior change: stop auto-injecting the 7-day memory-tree digest into every orchestrator turn.
• User-visible effect: agent answers from on-demand memory retrieval; disabling the memory tree now keeps its contents out of the prompt.

Parity Contract

• Legacy behavior preserved: on-demand memory-tree retrieval tools unchanged; tree_loader module retained for future re-wiring.
• Guard/fallback/dispatch parity checks: the removed path was already non-fatal/best-effort; memory_loader.load_context remains the context source.

Duplicate / Superseded PR Handling

• Duplicate PR(s): none
• Canonical PR: this
• Resolution: N/A

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Removed the eager memory-tree prefetch path and related timing state so on-demand memory retrieval and its toggle behave consistently; turn processing now uses a simpler, deterministic context flow.

• Documentation

  • Updated docs to note the memory-tree loader is not invoked in the turn loop and explained retained constants for potential future opt-in use.

• Tests

  • Updated tests to match the simplified prefetch behavior and turn flow.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98d8b9cf-e123-4d0a-97f2-e42ac61d4698

📥 Commits

Reviewing files that changed from the base of the PR and between 88b6da9 and ace3c91.

📒 Files selected for processing (5)

• src/openhuman/agent/harness/session/builder.rs
• src/openhuman/agent/harness/session/turn.rs
• src/openhuman/agent/harness/session/turn_tests.rs
• src/openhuman/agent/harness/session/types.rs
• src/openhuman/agent/tree_loader.rs

💤 Files with no reviewable changes (4)

• src/openhuman/agent/harness/session/types.rs
• src/openhuman/agent/harness/session/turn_tests.rs
• src/openhuman/agent/harness/session/builder.rs
• src/openhuman/agent/harness/session/turn.rs

✅ Files skipped from review due to trivial changes (1)

• src/openhuman/agent/tree_loader.rs

---

📝 Walkthrough

Walkthrough

Removes the unconditional eager 7-day memory-tree digest injection from the agent turn loop: deletes session prefetch tracking, removes the prefetch block from Agent::turn, updates builder initialization and tests, and documents that TreeContextLoader is retained but not wired into the turn loop.

Changes

Memory-tree eager prefetch removal

Layer / File(s)	Summary
Agent struct and builder: remove prefetch tracking src/openhuman/agent/harness/session/types.rs, src/openhuman/agent/harness/session/builder.rs	Removed Agent.last_tree_prefetch_at and stopped initializing it; builder now initializes recent conversational state fields (last_memory_context, last_turn_citations, history).
Turn loop: remove prefetch injection block src/openhuman/agent/harness/session/turn.rs	Deleted the conditional eager prefetch/injection of TreeContextLoader; context from memory_loader.load_context(...) is no longer augmented and proceeds to STM preemptive recall.
Test cleanup: remove prefetch suppressions src/openhuman/agent/harness/session/turn_tests.rs	Removed test setups that set last_tree_prefetch_at = Some(Instant::now()); tests now run without suppressing the removed prefetch path.
TreeContextLoader documentation: clarify non-wired status src/openhuman/agent/tree_loader.rs	Added module- and method-level notes explaining the loader is not currently called from the turn loop, why the unconditional 7-day digest was removed, and that the module is retained for tests and potential future re-wiring.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#3012: Refactors Agent::turn orchestration; related because both change turn loop behavior and memory/context orchestration.
• tinyhumansai/openhuman#2123: Changes per-turn context injection logic; related area of per-turn context handling.

Suggested reviewers

• graycyrus
• oxoxDev

Poem

🐰 I nudged the seven-day digest aside,
Let on-demand memories take the ride.
The loader naps beneath the bough,
Waiting to wake when gates allow—
Soft paws, patient for the next rewrite.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: removing eager memory-tree digest injection from the turn loop, matching the changeset across all affected files.
Linked Issues check	✅ Passed	The PR fully implements all coding requirements from issue #3170: removes tree prefetch logic from turn.rs, deletes last_tree_prefetch_at state, updates tests, retains tree_loader.rs, and preserves on-demand retrieval.
Out of Scope Changes check	✅ Passed	All changes directly address the requirements in issue #3170. Documentation updates in tree_loader.rs clarify the unwired state without introducing unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/agent/tree_loader.rs (1)

14-48: 💤 Low value

Consider clarifying that the behavior descriptions are historical.

The NOTE section correctly explains this module is unwired, but the subsequent documentation uses present tense ("The orchestrator answers...", "We pre-load...") which could confuse readers who miss the transition at line 12-13. Similarly, constants like DEFAULT_WINDOW_DAYS and REFRESH_INTERVAL are documented as if actively used.

For clearer documentation:

• Rewrite lines 14-20 in past tense, or add explicit "Historically," / "Previously," markers
• Update constant doc-comments (lines 38-48) to note they're retained for future re-wiring
• Optionally add a brief note to the load function doc (line 79) referencing the module-level NOTE

This would help future maintainers quickly understand the module's current status without needing to carefully parse the NOTE transition.

🤖 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/agent/tree_loader.rs` around lines 14 - 48, Update the module
and constant docs to make the historical status explicit: revise the top-level
doc lines that currently read in present tense ("The orchestrator answers...",
"We pre-load...") to past-tense or prepend "Historically,"/ "Previously," to
indicate the module was once wired; update the `DEFAULT_WINDOW_DAYS` and
`REFRESH_INTERVAL` doc-comments to note these constants are retained for
potential future re-wiring rather than actively used; and add a one-sentence
pointer in `TreeContextLoader::load`'s doc comment referencing the module-level
NOTE about being unwired so callers see the status without parsing the full
header.

🤖 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.

Nitpick comments:
In `@src/openhuman/agent/tree_loader.rs`:
- Around line 14-48: Update the module and constant docs to make the historical
status explicit: revise the top-level doc lines that currently read in present
tense ("The orchestrator answers...", "We pre-load...") to past-tense or prepend
"Historically,"/ "Previously," to indicate the module was once wired; update the
`DEFAULT_WINDOW_DAYS` and `REFRESH_INTERVAL` doc-comments to note these
constants are retained for potential future re-wiring rather than actively used;
and add a one-sentence pointer in `TreeContextLoader::load`'s doc comment
referencing the module-level NOTE about being unwired so callers see the status
without parsing the full header.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fbaf0783-9160-44ea-abe2-eee2e8ebc6d3

📥 Commits

Reviewing files that changed from the base of the PR and between 817c2a0 and ef20f20.

📒 Files selected for processing (5)

• src/openhuman/agent/harness/session/builder.rs
• src/openhuman/agent/harness/session/turn.rs
• src/openhuman/agent/harness/session/turn_tests.rs
• src/openhuman/agent/harness/session/types.rs
• src/openhuman/agent/tree_loader.rs

💤 Files with no reviewable changes (4)

• src/openhuman/agent/harness/session/types.rs
• src/openhuman/agent/harness/session/builder.rs
• src/openhuman/agent/harness/session/turn.rs
• src/openhuman/agent/harness/session/turn_tests.rs

[sanil-23]

@coderabbitai addressed the nitpick in 88b6da9: rewrote the module-level behavior description in past tense, noted that DEFAULT_WINDOW_DAYS / REFRESH_INTERVAL are retained for a future re-wiring rather than actively used, and added a pointer from TreeContextLoader::load to the module-level "unwired" NOTE. Doc-only, no behavior change.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@sanil-23 Thanks for the update! The doc improvements in 88b6da9 look thorough — past-tense phrasing, the explicit callout that DEFAULT_WINDOW_DAYS / REFRESH_INTERVAL are retained for future re-wiring, and the cross-reference pointer from TreeContextLoader::load all make the "unwired" state much clearer for the next reader.