Skip to content

feat(context): global context management module#504

Merged
senamakel merged 15 commits into
tinyhumansai:mainfrom
senamakel:feat/context-mgmt
Apr 11, 2026
Merged

feat(context): global context management module#504
senamakel merged 15 commits into
tinyhumansai:mainfrom
senamakel:feat/context-mgmt

Conversation

@senamakel
Copy link
Copy Markdown
Member

@senamakel senamakel commented Apr 11, 2026

Summary

  • New src/openhuman/context/ module owns system prompt assembly, in-flight history reduction, and LLM-backed summarization in one place — previously these lived across agent/context_pipeline/, agent/harness/context_guard.rs, agent/prompt.rs, channels/prompt.rs, and agent/harness/subagent_runner.rs.
  • Agents now hold a single ContextManager per session (replacing the old context_pipeline: ContextPipeline + prompt_builder: SystemPromptBuilder pair on Agent). The manager exposes build_system_prompt, reduce_before_call (async, dispatches the summarizer internally), record_usage, tick_turn, and stats.
  • Autocompaction actually runs now: when the pipeline returns AutocompactionRequested, ContextManager dispatches an LLM summarization via a new Summarizer trait (default impl ProviderSummarizer wraps an Arc<dyn Provider>). Before this PR the agent just logged a warning and did nothing.
  • Global ContextConfig section on the root config with env overrides (OPENHUMAN_CONTEXT_*). Deprecated agent.tool_result_budget_bytes auto-migrates to context.tool_result_budget_bytes at load time with a tracing warning so existing configs keep working.
  • +11 new unit tests (6 for ContextManager with a mock summarizer, 5 for ProviderSummarizer including the snap-split-forward logic that preserves AssistantToolCalls ↔ ToolResults pairs). Full crate sweep: 2253 tests passing.

Problem

Context management is one concern — "what's in the model's context window?" — but three separate implementations had grown up around it:

  1. agent/context_pipeline/ owned microcompact + tool-result budget + session-memory triggers, deliberately pure (no LLM calls); it returned PipelineOutcome::AutocompactionRequested and trusted callers to dispatch.
  2. agent/harness/context_guard.rs tracked per-model utilization and the compaction circuit breaker.
  3. Summarization dispatch was gone entirely — PR feat(agent): simplify harness + trim bundled prompts #500 removed auto_compact_history without a replacement, so when the pipeline asked for summarization the agent loop just logged a warning.

On top of that, system prompt assembly was duplicated: agent/prompt.rs had a nice SystemPromptBuilder, but channels/prompt.rs reimplemented ~130 lines of nearly identical logic because it consumed &[(&str, &str)] tool tuples, and subagent_runner::render_subagent_system_prompt bypassed the builder entirely. Any new agent archetype or delegation tool had to re-wire all of it.

Solution

One module, three responsibilities:

```
src/openhuman/context/
├── mod.rs # re-exports
├── manager.rs # ContextManager — the per-session keystone
├── config.rs # Mapped from config::schema::context
├── guard.rs # (moved from agent/harness/context_guard.rs)
├── pipeline.rs # (moved from agent/context_pipeline/pipeline.rs)
├── microcompact.rs # (moved)
├── tool_result_budget.rs # (moved)
├── session_memory.rs # (moved; Serialize/Deserialize/JsonSchema added)
├── summarizer.rs # NEW — Summarizer trait + ProviderSummarizer
├── prompt.rs # (moved from agent/prompt.rs + PromptTool<'a>)
└── channels_prompt.rs # (moved from channels/prompt.rs)
```

ContextManager is the single surface agents interact with. Constructed once at session start via `AgentBuilder::context_config(&config.context)`, it owns the pipeline, the compaction circuit breaker, the prompt builder, and an `Arc`. `Agent::turn` calls `self.context.reduce_before_call(&mut self.history).await?` before every provider hit; on `AutocompactionRequested` the manager dispatches the summarizer and records success/failure on the guard's breaker (3 consecutive failures still trip it).

Type harmonization via PromptTool<'a>: the new `PromptTool { name, description, parameters_schema: Option }` lets main agents (which own `Box`) and channels (which only have tuples) feed the same `ToolsSection` implementation. `PromptTool::from_tools(&tools)` is the one-line adapter.

Sub-agent prompt renderer moved to `context::prompt::render_subagent_system_prompt` — same byte-stable implementation, now living alongside the rest of the prompt code so all assembly has one home.

Channel system prompt moved to `context::channels_prompt` wholesale. The bespoke layout (Tool Use Protocol section, Channel Capabilities section, timezone-only datetime for KV-cache stability) is preserved byte-for-byte — consolidating the file location without rebuilding sections was the lower-risk move since channel prompts are live in production. A doc comment flags the builder migration as future work.

Summarization strategy: `ProviderSummarizer::summarize` keeps the `keep_recent = 10` most-recent messages untouched, renders the older head as a plain-text transcript, asks the LLM to compress it into a dense note via a pinned short system prompt, and replaces the head in place with a single `system` `ConversationMessage`. The split point is snapped forward past any `AssistantToolCalls ↔ ToolResults` pair so the API invariant can't break mid-pair. Failures don't partially mutate history.

Explicitly deferred: process-wide session coordination + the `context.get_stats` RPC endpoint. Those require a session registry the codebase doesn't have today; the scope answer was "shared services" (per-agent state, shared logic), not "process-wide coordinator". Observability is covered by the Rust `ContextManager::stats()` method for now.

Submission Checklist

  • Unit tests — 11 new tests (6 `manager`, 5 `summarizer`), all existing `context_pipeline` + `guard` + `channels::prompt` tests moved with their files and still pass. `cargo test --lib` → 2253 passing.
  • E2E / integration — No new E2E; the summarization path needs a long real-session smoke test which is harder to automate. All existing E2E specs continue to pass the build step.
  • Doc comments — `//!` module header on every new file explaining its responsibility + cache-safety / ordering contracts; `///` on every public `ContextManager` / `Summarizer` method; inline comments at the non-obvious integration sites (snap-split logic, migration shim).
  • Inline comments — Added at `turn.rs` reduction site (async dispatch), `builder.rs` manager construction, `config/schema/load.rs` migration shim.

Impact

  • Runtime: desktop (the only surface that runs agents). No mobile/web impact.
  • Behavior change (intended): autocompaction now actually summarizes history instead of logging a warning and stalling. Long sessions that previously would hit the hard limit and abort will now get compacted automatically.
  • Migration: `agent.tool_result_budget_bytes` is deprecated in favor of `context.tool_result_budget_bytes`. Config loader copies it forward at load time with a `tracing::warn!`; existing `config.toml` files continue to work unchanged.
  • Performance: summarization adds one LLM call per autocompaction trigger (previously zero), but only when the pipeline would otherwise have aborted the turn. Cheap models can be configured via `context.summarizer_model` to reduce cost.
  • Clippy: one real bug fixed in `summarizer::snap_split_forward` — a `while` that was secretly an `if` (caught by `-W clippy::never_loop`).

Related

  • Issue(s): none filed — feature work
  • Follow-up PR(s)/TODOs:
    • Cross-agent budget coordinator / process-wide quota pool
    • `context.get_stats` JSON-RPC endpoint (once session coordination lands)
    • Migrate `channels_prompt` + `render_subagent_system_prompt` onto a shared `SystemPromptBuilder` with custom `PromptSection` impls for channel capabilities and sub-agent calling conventions
    • Replace char-heuristic token counting with a real tokenizer (`tiktoken-rs`) if token accuracy becomes a pain point

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added unified ContextManager and public ContextConfig with env overrides (OPENHUMAN_CONTEXT_*) and summarizer model override.
    • Introduced channel-aware system prompt (optional channel name) and prompt-tool metadata for clearer tool rendering.
    • Centralized subagent prompt renderer with render options.
  • Improvements

    • Consolidated context reduction and session-memory controls with richer outcomes and stats.
    • Exposed additional context guard metrics.
  • Documentation

    • Removed emoji usage rules from SOUL.md.

…agement

- Introduced a new context pipeline orchestrator that manages context reduction stages before each provider call, including tool-result budgeting, microcompaction, and session memory management.
- Added `ContextGuard` to monitor context utilization and trigger auto-compaction when thresholds are exceeded.
- Implemented `microcompact` to replace older tool result payloads with placeholders, preserving API invariants while managing memory effectively.
- Created `SessionMemory` to maintain persistent notes across sessions, updated by a background process to avoid impacting performance during user interactions.
- Enhanced overall context management to improve memory efficiency and ensure smoother operation of the agent's functionality.
- Introduced a new `context` module to centralize context management for agent sessions, enhancing organization and maintainability.
- Updated various files to replace references to the deprecated `context_pipeline` with the new `context` module, ensuring consistency across the codebase.
- Bumped the version of the `openhuman` package to 0.52.4 in `Cargo.lock` to reflect these changes.
- Refactored import statements across multiple files to replace references to the deprecated `agent::prompt` module with the new `context::prompt` module.
- This change enhances code organization and aligns with the recent restructuring of the context management system.
- Added a new `ContextConfig` struct to manage global context settings, including budget thresholds, summarization triggers, and session-memory extraction parameters.
- Implemented environment variable overrides for context management settings, allowing dynamic configuration.
- Refactored the configuration loading process to accommodate the new context management structure, ensuring backward compatibility with existing configurations.
- Introduced a `Summarizer` trait and default implementation for handling conversation summarization, enhancing the agent's ability to manage context effectively.
- Updated related modules to integrate the new context management features, improving overall organization and maintainability of the codebase.
…handling

- Introduced a new `ContextManager` struct to manage session context, including prompt assembly, context reduction, and summarization dispatch.
- Added methods to `ContextGuard` for tracking input and output token counts, as well as the context window size.
- Updated the `mod.rs` files to include the new `manager` module and its exports, improving organization and maintainability of the context management system.
- Enhanced overall context management capabilities, allowing for more efficient handling of conversation context during agent interactions.
…imes

- Moved the system prompt construction logic for channel runtimes into a new `channels_prompt` module within the `context` directory, ensuring all prompt-building code is organized in one location.
- Updated various files to replace deprecated references to the old prompt module, enhancing code clarity and maintainability.
- Introduced a new `build_system_prompt` function tailored for channel-specific requirements, including tool descriptions and channel-specific preambles, while maintaining byte-stability for cache efficiency.
- Refactored related imports and tests to align with the new structure, improving overall organization of the context management system.
Clippy caught a never-looping `while` in `snap_split_forward` that was
really an `if` in disguise — collapse it. Everything else is pure
`cargo fmt` cleanup on files touched during the context/ refactor.

Full `cargo test --lib` sweep: 2253 tests passing.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

📝 Walkthrough

Walkthrough

Consolidates context management into a new openhuman::context module, introducing ContextManager, summarizer and pipeline modules, migrating prompt tooling, and removing the legacy agent::context_pipeline surface; agent builder/session code and config/schema updated to use the new context system.

Changes

Cohort / File(s) Summary
Module deletion
src/openhuman/agent/context_pipeline/mod.rs
Removed legacy agent::context_pipeline module and its public re-exports (microcompact, pipeline, session_memory, tool_result_budget).
Context module & APIs
src/openhuman/context/mod.rs, src/openhuman/context/manager.rs, src/openhuman/context/summarizer.rs, src/openhuman/context/guard.rs, src/openhuman/context/pipeline.rs, src/openhuman/context/prompt.rs, src/openhuman/context/session_memory.rs, src/openhuman/context/microcompact.rs, src/openhuman/context/tool_result_budget.rs, src/openhuman/context/channels_prompt.rs
Added centralized openhuman::context module with ContextManager, ReductionOutcome, summarizer trait and ProviderSummarizer, expanded prompt helpers (PromptTool, subagent renderer), session-memory serde defaults, guard accessors, pipeline session-handle API, and re-exports for microcompact/tool-budget APIs.
Agent harness migration
src/openhuman/agent/harness/session/builder.rs, src/openhuman/agent/harness/session/types.rs, src/openhuman/agent/harness/session/turn.rs, src/openhuman/agent/harness/subagent_runner.rs, src/openhuman/agent/harness/mod.rs, src/openhuman/agent/harness/tool_loop.rs
Replaced prompt_builder + ContextPipeline with single ContextManager; added context_config() builder method; updated turn lifecycle to call ContextManager methods and map new ReductionOutcome variants; moved subagent prompt rendering to context::prompt; adjusted imports (removed context_guard submodule).
Module surface & imports
src/openhuman/agent/mod.rs, src/openhuman/mod.rs, src/openhuman/channels/mod.rs, src/openhuman/channels/runtime/startup.rs, src/openhuman/channels/tests/*, src/openhuman/learning/prompt_sections.rs
Removed agent::context_pipeline and agent::prompt exports; added pub mod context; migrated imports and re-exports to use crate::openhuman::context (e.g., build_system_prompt re-export).
Configuration additions & migration
src/openhuman/config/schema/context.rs, src/openhuman/config/schema/types.rs, src/openhuman/config/schema/mod.rs, src/openhuman/config/mod.rs, src/openhuman/config/schema/load.rs, src/openhuman/config/schema/agent.rs
Introduced ContextConfig with compaction/autocompaction toggles, thresholds, reserve tokens, session-memory and summarizer model fields; wired into top-level Config; added env override handling and migration/deprecation for tool_result_budget_bytes.
Prompt and tooling changes
src/openhuman/context/prompt.rs, src/openhuman/agent/harness/subagent_runner.rs, src/openhuman/context/channels_prompt.rs, src/openhuman/context/prompt.rs
Added PromptTool<'a> and SubagentRenderOptions; changed PromptContext.tools to &[PromptTool]; moved and generalized subagent renderer; extended channel prompt to accept optional channel_name.
Docs & minor content edits
src/openhuman/agent/prompts/SOUL.md, src/openhuman/context/session_memory.rs
Removed the “Emoji” section from SOUL.md; made SessionMemoryConfig serde/json-schema-capable with serde default helpers.

Sequence Diagram

sequenceDiagram
    actor User
    participant Agent as Agent Turn
    participant ContextMgr as ContextManager
    participant Pipeline as ContextPipeline
    participant Summarizer as ProviderSummarizer
    participant Provider as LLM Provider

    User->>Agent: Send message
    Agent->>Agent: Process input
    Agent->>ContextMgr: tick_turn()
    ContextMgr->>Pipeline: Update turn state

    Agent->>ContextMgr: build_system_prompt()
    ContextMgr->>ContextMgr: Use SystemPromptBuilder
    ContextMgr-->>Agent: System prompt

    Agent->>Provider: Chat request
    Provider-->>Agent: Response + usage

    Agent->>ContextMgr: record_usage(usage)
    ContextMgr->>Pipeline: Update token tracking
    Agent->>ContextMgr: record_tool_calls(n)
    ContextMgr->>Pipeline: Update session memory counters

    Agent->>ContextMgr: reduce_before_call(history)
    ContextMgr->>Pipeline: Check utilization/triggers

    alt Microcompaction possible
        Pipeline-->>ContextMgr: Microcompact outcome
    else Autocompaction needed
        ContextMgr->>Summarizer: summarize(history, model)
        Summarizer->>Provider: Provider chat_with_history
        Provider-->>Summarizer: Summary
        Summarizer->>ContextMgr: SummaryStats
        ContextMgr->>Pipeline: Update guard/breaker
    else Disabled or below threshold
        Pipeline-->>ContextMgr: NoOp / AutocompactionDisabled
    end

    ContextMgr-->>Agent: ReductionOutcome
    Agent->>ContextMgr: stats()
    ContextMgr-->>Agent: ContextStats
    Agent-->>User: Final response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 Hopped through code with nimble paws,

I stitched context into tidy laws,
One Manager now, no scattered seams,
Summaries neat and prompt-building dreams,
A carrot-cheer for cleaner streams!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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 primary change: introducing a global context management module that consolidates system prompt assembly, in-flight history reduction, and LLM-backed summarization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

- Deleted the `config.toml` file for user `69ccc8e95692bb0ddd56c10f`, which was no longer needed, streamlining the user configuration management.
… instructions

- Deleted the section on emoji usage to simplify the prompt guidelines, focusing on clarity and direct communication with users. This change aims to enhance the overall effectiveness of the prompts by reducing unnecessary complexity.
@senamakel senamakel marked this pull request as ready for review April 11, 2026 06:39
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/context/channels_prompt.rs (1)

166-175: ⚠️ Potential issue | 🟠 Major

Channel Capabilities section is hardcoded for Discord only.

This build_system_prompt function is used by all channel runtimes (Telegram, Slack, Signal, etc.), but the "Channel Capabilities" section explicitly states "running as a Discord bot" (line 169). This will confuse the model when handling messages from non-Discord channels.

Consider parameterizing the channel name or removing channel-specific statements from this shared builder.

🐛 Suggested approach
 pub fn build_system_prompt(
     workspace_dir: &Path,
     model_name: &str,
     tools: &[(&str, &str)],
     skills: &[crate::openhuman::skills::Skill],
     bootstrap_max_chars: Option<usize>,
+    channel_name: Option<&str>,
 ) -> String {

Then conditionally render channel-specific capabilities, or use a generic statement like "You are running as a messaging bot."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/context/channels_prompt.rs` around lines 166 - 175, The Channel
Capabilities text in build_system_prompt currently hardcodes "Discord" and
should be made channel-agnostic: update build_system_prompt to accept a channel
identifier (e.g., channel_name) or a boolean/enum for channel-specific
rendering, then replace the literal "You are running as a Discord bot" line with
either a generic "You are running as a messaging bot." or a formatted string
that inserts the channel_name when provided; keep the other capability bullets
intact and only render channel-specific lines conditionally so prompt (the
string accumulator) remains correct for Telegram/Slack/Signal runtimes.
🧹 Nitpick comments (1)
src/openhuman/context/prompt.rs (1)

437-446: Consider defensive bounds checking for allowed_indices.

Direct indexing with parent_tools[i] will panic if allowed_indices contains an out-of-bounds index. While the caller contract presumably guarantees valid indices, a defensive check would prevent session crashes from misconfigured sub-agent definitions.

🛡️ Optional: Add bounds check or use get()
     for &i in allowed_indices {
-        let tool = &parent_tools[i];
+        let Some(tool) = parent_tools.get(i) else {
+            tracing::warn!("render_subagent_system_prompt: index {i} out of bounds, skipping");
+            continue;
+        };
         let _ = writeln!(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/context/prompt.rs` around lines 437 - 446, The loop directly
indexes parent_tools with values from allowed_indices which can panic if an
index is out of range; change the iteration to defensively access parent_tools
using get(i) (or check i < parent_tools.len()) and skip or log any invalid
indices before calling tool.name()/tool.description()/tool.parameters_schema()
and writing to out, ensuring the function that contains allowed_indices and
parent_tools (the loop using writeln! and variables out, allowed_indices,
parent_tools) no longer can panic from bad indices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/agent/harness/session/turn.rs`:
- Around line 802-810: The code currently calls
self.context.mark_session_memory_complete() immediately after taking
stats_snapshot, which prematurely clears deltas and extraction_in_progress;
change this to call self.context.mark_session_memory_started() instead and move
the mark_session_memory_complete() (and any mark failed) into the background
task's result handling path so the session state is only flipped after the
archivist task returns success or failure; update the background task code that
currently handles archivist results (the closure or async block that uses
stats_snapshot/archivist) to call mark_session_memory_complete() on success and
the appropriate failure path on error, ensuring no other places still clear
extraction_in_progress prematurely.
- Around line 529-531: The code is using the deprecated agent-level field
self.config.tool_result_budget_bytes when calling apply_tool_result_budget;
instead retrieve the resolved context budget from the session's context
manager/config (e.g. use the resolved ContextConfig::tool_result_budget_bytes
from your ContextManager or session.resolved_context) and pass that value as
budget_bytes to crate::openhuman::context::apply_tool_result_budget(raw_result,
budget_bytes) so inline truncation honors the new context setting rather than
the old agent field.

In `@src/openhuman/agent/harness/subagent_runner.rs`:
- Around line 235-242: The code currently discards `definition` and thus ignores
per-definition flags like `omit_identity`, `omit_memory_context`,
`omit_safety_preamble`, and `omit_skills_catalog` when calling
`render_subagent_system_prompt`; instead, thread the relevant options from
`definition` into the call to
`crate::openhuman::context::prompt::render_subagent_system_prompt` (e.g., add
parameters or pass a small options struct created from `definition`) so the
function receives those flags along with `&parent.workspace_dir`, `&model`,
`&allowed_indices`, `&parent.all_tools`, and `&archetype_prompt_body`, and
remove the `_ = definition` discard so the per-definition render options are
honored at runtime.

In `@src/openhuman/config/schema/load.rs`:
- Around line 918-931: The current env parsing for
OPENHUMAN_CONTEXT_COMPACTION_TRIGGER_PCT and OPENHUMAN_CONTEXT_HARD_LIMIT_PCT
only checks 0..=100 and allows 0 and inverted ranges; change the logic to reject
0 (use 1..=100) and enforce semantic ordering (compaction_trigger_pct <
hard_limit_pct). Parse each var into a temporary, validate range (1..=100), then
if both temps exist ensure temp_compaction < temp_hard_limit before assigning to
self.context.compaction_trigger_pct and self.context.hard_limit_pct; on
validation failure, do not overwrite the existing fields and emit a clear
warning log mentioning the env var and invalid value.
- Around line 949-966: The migration treats
self.context.tool_result_budget_bytes == DEFAULT_TOOL_RESULT_BUDGET_BYTES as
“unset” even when the user explicitly set
OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES to that same default; update the
migration in load.rs so it only copies from self.agent.tool_result_budget_bytes
when the env var OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES is not present.
Concretely, in the block that currently checks
self.context.tool_result_budget_bytes ==
crate::openhuman::context::DEFAULT_TOOL_RESULT_BUDGET_BYTES and
self.agent.tool_result_budget_bytes != context_default, also ensure
std::env::var_os("OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES").is_none() before
assigning self.context.tool_result_budget_bytes =
self.agent.tool_result_budget_bytes and emitting the deprecation warning.

In `@src/openhuman/context/manager.rs`:
- Around line 70-72: The enum variant NotAttempted in
PipelineOutcome/ReductionOutcome is never constructed by reduce_before_call,
making its match arm in turn.rs unreachable; either remove the NotAttempted
variant and its matches (including the arm in turn.rs that handles NotAttempted)
or implement the missing path in reduce_before_call to return NotAttempted when
an AutocompactionRequested pipeline result is observed but autocompaction is
disabled by config (propagate config check into reduce_before_call and construct
NotAttempted { utilisation_pct } accordingly), and update any related docs/tests
to reflect the chosen approach.

---

Outside diff comments:
In `@src/openhuman/context/channels_prompt.rs`:
- Around line 166-175: The Channel Capabilities text in build_system_prompt
currently hardcodes "Discord" and should be made channel-agnostic: update
build_system_prompt to accept a channel identifier (e.g., channel_name) or a
boolean/enum for channel-specific rendering, then replace the literal "You are
running as a Discord bot" line with either a generic "You are running as a
messaging bot." or a formatted string that inserts the channel_name when
provided; keep the other capability bullets intact and only render
channel-specific lines conditionally so prompt (the string accumulator) remains
correct for Telegram/Slack/Signal runtimes.

---

Nitpick comments:
In `@src/openhuman/context/prompt.rs`:
- Around line 437-446: The loop directly indexes parent_tools with values from
allowed_indices which can panic if an index is out of range; change the
iteration to defensively access parent_tools using get(i) (or check i <
parent_tools.len()) and skip or log any invalid indices before calling
tool.name()/tool.description()/tool.parameters_schema() and writing to out,
ensuring the function that contains allowed_indices and parent_tools (the loop
using writeln! and variables out, allowed_indices, parent_tools) no longer can
panic from bad indices.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: a9f81925-b784-498a-a956-0b934fdb48c3

📥 Commits

Reviewing files that changed from the base of the PR and between 462b4d2 and 680ca55.

📒 Files selected for processing (32)
  • path=/Users/cardinal/.openhuman/users/69ccc8e95692bb0ddd56c10f/config.toml
  • src/openhuman/agent/context_pipeline/mod.rs
  • src/openhuman/agent/harness/mod.rs
  • src/openhuman/agent/harness/session/builder.rs
  • src/openhuman/agent/harness/session/turn.rs
  • src/openhuman/agent/harness/session/types.rs
  • src/openhuman/agent/harness/subagent_runner.rs
  • src/openhuman/agent/harness/tool_loop.rs
  • src/openhuman/agent/mod.rs
  • src/openhuman/agent/prompts/SOUL.md
  • src/openhuman/channels/mod.rs
  • src/openhuman/channels/runtime/startup.rs
  • src/openhuman/channels/tests/identity.rs
  • src/openhuman/channels/tests/prompt.rs
  • src/openhuman/config/mod.rs
  • src/openhuman/config/schema/agent.rs
  • src/openhuman/config/schema/context.rs
  • src/openhuman/config/schema/load.rs
  • src/openhuman/config/schema/mod.rs
  • src/openhuman/config/schema/types.rs
  • src/openhuman/context/channels_prompt.rs
  • src/openhuman/context/guard.rs
  • src/openhuman/context/manager.rs
  • src/openhuman/context/microcompact.rs
  • src/openhuman/context/mod.rs
  • src/openhuman/context/pipeline.rs
  • src/openhuman/context/prompt.rs
  • src/openhuman/context/session_memory.rs
  • src/openhuman/context/summarizer.rs
  • src/openhuman/context/tool_result_budget.rs
  • src/openhuman/learning/prompt_sections.rs
  • src/openhuman/mod.rs
💤 Files with no reviewable changes (4)
  • src/openhuman/agent/harness/mod.rs
  • src/openhuman/agent/prompts/SOUL.md
  • src/openhuman/agent/mod.rs
  • src/openhuman/agent/context_pipeline/mod.rs

Comment thread src/openhuman/agent/harness/session/turn.rs Outdated
Comment thread src/openhuman/agent/harness/session/turn.rs Outdated
Comment thread src/openhuman/agent/harness/subagent_runner.rs Outdated
Comment thread src/openhuman/config/schema/load.rs Outdated
Comment thread src/openhuman/config/schema/load.rs
Comment thread src/openhuman/context/manager.rs
…ompaction settings

- Improved the parsing and validation of environment variables for `OPENHUMAN_CONTEXT_COMPACTION_TRIGGER_PCT` and `OPENHUMAN_CONTEXT_HARD_LIMIT_PCT`, ensuring that the compaction trigger percentage is strictly less than the hard limit.
- Added warnings for invalid percentage values and retained existing configuration values on failure.
- Updated the context management logic to handle migration from deprecated fields, ensuring backward compatibility and clearer user guidance.
- Changed the source of the tool result budget to be derived from the ContextManager, ensuring it reflects the resolved `context.tool_result_budget_bytes` with any environment or configuration overrides.
- This update removes reliance on the deprecated `agent.tool_result_budget_bytes` field, centralizing budget management for improved consistency across the context management system.
…t construction

- Introduced `SubagentRenderOptions` to manage per-definition rendering flags, allowing for more flexible control over the inclusion of identity, safety preamble, and skills catalog in the system prompt.
- Updated `render_subagent_system_prompt` to utilize these options, ensuring that the rendering behavior aligns with the specified flags.
- Modified `build_system_prompt` to accept an optional `channel_name`, improving clarity in channel capabilities messaging and removing hardcoded references to "Discord".
- Adjusted tests to reflect changes in prompt construction, ensuring comprehensive coverage of the new rendering logic.
- Updated session memory methods in `ContextManager` to use a locked reference for thread safety during extraction state changes.
- Introduced a new method `session_memory_handle` to facilitate background task management of session memory extraction.
- Adjusted the context statistics to utilize a snapshot of session memory, enhancing data integrity and consistency.
- Updated the extraction state handling to transition to "in-progress" instead of "complete" immediately, allowing for better control during background extraction tasks.
- Introduced a shared handle for session memory to manage extraction completion and failure states, ensuring deltas are preserved for retries on failure.
- Enhanced logging to provide clearer insights into the extraction process outcomes.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/config/schema/load.rs`:
- Around line 996-1009: The migration wrongly treats an explicitly set
context.tool_result_budget_bytes equal to DEFAULT_TOOL_RESULT_BUDGET_BYTES as
"unset" and so still overwrites it from the deprecated agent value; change the
logic to use a presence flag from the TOML parser (or make the field an Option)
rather than comparing against DEFAULT_TOOL_RESULT_BUDGET_BYTES. Concretely: add
a boolean like context_tool_result_budget_bytes_present (set when parsing
OPENHUMAN [context] in the TOML) and replace the current check that uses
self.context.tool_result_budget_bytes == DEFAULT_TOOL_RESULT_BUDGET_BYTES with
!context_tool_result_budget_bytes_present (keep the existing env check
OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES and the migration of
self.context.tool_result_budget_bytes = self.agent.tool_result_budget_bytes and
the tracing::warn using self.agent.tool_result_budget_bytes).

In `@src/openhuman/context/manager.rs`:
- Around line 187-192: The manager's should_extract_session_memory currently
delegates to self.pipeline.should_extract_session_memory() and can trigger
archivist extractions even when the manager is disabled; modify
should_extract_session_memory to return false when the manager is disabled
(check the manager's enabled flag first) so it short-circuits pipeline calls,
and similarly update session-memory bookkeeping entry points (tick_turn,
record_usage, record_tool_calls, and reduce_before_call) to no-op when the
manager is disabled to prevent background archivist runs and bookkeeping side
effects.

In `@src/openhuman/context/pipeline.rs`:
- Around line 141-170: The session-memory Mutex guard is currently ignored on
poisoning (using if let Ok or unwrap_or(false)), causing updates and checks in
record_usage, tick_turn, record_tool_calls, should_extract_session_memory, and
session_memory_snapshot to be silently dropped; change each lock() call to
recover the poisoned guard with .lock().unwrap_or_else(|e| e.into_inner()) (and
optionally emit a warning via the logger) so the guard is accessed even after a
panic; apply the same recovery pattern in the background task in turn.rs where
session_memory is locked.

In `@src/openhuman/context/prompt.rs`:
- Around line 549-561: The branch controlled by options.include_skills_catalog
only emits a placeholder and doesn't render actual skills, so thread the real
skills data into this renderer or disable the flag: update the caller
(run_typed_mode) to pass a &[Skill] (or Option<&[Skill]>) down into this module
and change the renderer to, when include_skills_catalog is true and a skills
slice is provided, call the same rendering logic used by SkillsSection to emit
each skill name/path and description instead of the generic text; alternatively,
if you cannot plumb skills now, make include_skills_catalog false/unavailable by
removing exposure from run_typed_mode/definition.omit_skills_catalog so callers
can't opt in until full support is implemented.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 56a2543c-659a-4b6d-804c-06806f845a6e

📥 Commits

Reviewing files that changed from the base of the PR and between 680ca55 and 0b2b13a.

📒 Files selected for processing (10)
  • src/openhuman/agent/harness/session/turn.rs
  • src/openhuman/agent/harness/subagent_runner.rs
  • src/openhuman/channels/runtime/startup.rs
  • src/openhuman/channels/tests/identity.rs
  • src/openhuman/channels/tests/prompt.rs
  • src/openhuman/config/schema/load.rs
  • src/openhuman/context/channels_prompt.rs
  • src/openhuman/context/manager.rs
  • src/openhuman/context/pipeline.rs
  • src/openhuman/context/prompt.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/openhuman/channels/runtime/startup.rs
  • src/openhuman/channels/tests/identity.rs
  • src/openhuman/context/channels_prompt.rs
  • src/openhuman/channels/tests/prompt.rs

Comment on lines +996 to +1009
let context_default = crate::openhuman::context::DEFAULT_TOOL_RESULT_BUDGET_BYTES;
let context_env_set =
std::env::var_os("OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES").is_some();
if !context_env_set
&& self.context.tool_result_budget_bytes == context_default
&& self.agent.tool_result_budget_bytes != context_default
{
tracing::warn!(
old = self.agent.tool_result_budget_bytes,
"[context:config] `agent.tool_result_budget_bytes` is \
deprecated — please move it to \
`context.tool_result_budget_bytes` in your config.toml"
);
self.context.tool_result_budget_bytes = self.agent.tool_result_budget_bytes;
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.

⚠️ Potential issue | 🟠 Major

Explicit [context] config set to the default still gets clobbered by the deprecated field.

This migration now preserves env precedence, but it still treats context.tool_result_budget_bytes == DEFAULT_TOOL_RESULT_BUDGET_BYTES as “unset”. A config file that explicitly sets [context].tool_result_budget_bytes = 16384 will still be overwritten by a stale non-default [agent].tool_result_budget_bytes, which defeats the new source of truth. This needs a presence-based check from the parsed TOML, not a value-equality check.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/config/schema/load.rs` around lines 996 - 1009, The migration
wrongly treats an explicitly set context.tool_result_budget_bytes equal to
DEFAULT_TOOL_RESULT_BUDGET_BYTES as "unset" and so still overwrites it from the
deprecated agent value; change the logic to use a presence flag from the TOML
parser (or make the field an Option) rather than comparing against
DEFAULT_TOOL_RESULT_BUDGET_BYTES. Concretely: add a boolean like
context_tool_result_budget_bytes_present (set when parsing OPENHUMAN [context]
in the TOML) and replace the current check that uses
self.context.tool_result_budget_bytes == DEFAULT_TOOL_RESULT_BUDGET_BYTES with
!context_tool_result_budget_bytes_present (keep the existing env check
OPENHUMAN_CONTEXT_TOOL_RESULT_BUDGET_BYTES and the migration of
self.context.tool_result_budget_bytes = self.agent.tool_result_budget_bytes and
the tracing::warn using self.agent.tool_result_budget_bytes).

Comment on lines +187 to +192
/// Whether the caller should spawn a background session-memory
/// extraction this turn. Delegates to the underlying pipeline
/// state; the manager does not spawn the extraction itself.
pub fn should_extract_session_memory(&self) -> bool {
self.pipeline.should_extract_session_memory()
}
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.

⚠️ Potential issue | 🟠 Major

Disabled context can still trigger archivist extractions.

enabled only short-circuits reduce_before_call(). turn.rs still calls tick_turn(), record_usage(), record_tool_calls(), and then should_extract_session_memory(), so once the thresholds are crossed a background archivist run can still happen with context.enabled = false. This method should return false when the manager is disabled, and ideally the session-memory bookkeeping methods should no-op as well.

🛡️ Minimal guard
     pub fn should_extract_session_memory(&self) -> bool {
+        if !self.enabled {
+            return false;
+        }
         self.pipeline.should_extract_session_memory()
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Whether the caller should spawn a background session-memory
/// extraction this turn. Delegates to the underlying pipeline
/// state; the manager does not spawn the extraction itself.
pub fn should_extract_session_memory(&self) -> bool {
self.pipeline.should_extract_session_memory()
}
/// Whether the caller should spawn a background session-memory
/// extraction this turn. Delegates to the underlying pipeline
/// state; the manager does not spawn the extraction itself.
pub fn should_extract_session_memory(&self) -> bool {
if !self.enabled {
return false;
}
self.pipeline.should_extract_session_memory()
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/context/manager.rs` around lines 187 - 192, The manager's
should_extract_session_memory currently delegates to
self.pipeline.should_extract_session_memory() and can trigger archivist
extractions even when the manager is disabled; modify
should_extract_session_memory to return false when the manager is disabled
(check the manager's enabled flag first) so it short-circuits pipeline calls,
and similarly update session-memory bookkeeping entry points (tick_turn,
record_usage, record_tool_calls, and reduce_before_call) to no-op when the
manager is disabled to prevent background archivist runs and bookkeeping side
effects.

Comment on lines 141 to +170
pub fn record_usage(&mut self, usage: &UsageInfo) {
self.guard.update_usage(usage);
self.session_memory
.record_usage(usage.input_tokens + usage.output_tokens);
let total = usage.input_tokens + usage.output_tokens;
if let Ok(mut sm) = self.session_memory.lock() {
sm.record_usage(total);
}
}

/// Bump the session-memory turn counter. Called once per user turn.
pub fn tick_turn(&mut self) {
self.session_memory.tick_turn();
if let Ok(mut sm) = self.session_memory.lock() {
sm.tick_turn();
}
}

/// Accumulate a turn's tool-call count into the session-memory
/// state. Called once per user turn after tool dispatch settles.
pub fn record_tool_calls(&mut self, n: usize) {
self.session_memory.record_tool_calls(n);
if let Ok(mut sm) = self.session_memory.lock() {
sm.record_tool_calls(n);
}
}

/// Should the caller spawn a background session-memory extraction
/// this turn?
pub fn should_extract_session_memory(&self) -> bool {
self.session_memory
.should_extract(&self.config.session_memory)
.lock()
.map(|sm| sm.should_extract(&self.config.session_memory))
.unwrap_or(false)
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the pipeline.rs file
find . -name "pipeline.rs" -path "*/openhuman/context/*" | head -5

Repository: tinyhumansai/openhuman

Length of output: 101


🏁 Script executed:

# Get the line count to understand file size
wc -l src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 103


🏁 Script executed:

# Examine lines 141-180 to see the actual code
sed -n '141,180p' src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 1467


🏁 Script executed:

# Also check the struct definition and imports to understand the Mutex setup
head -50 src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 2848


🏁 Script executed:

# Check session_memory.rs to understand the structure
head -100 src/openhuman/context/session_memory.rs | tail -50

Repository: tinyhumansai/openhuman

Length of output: 1872


🏁 Script executed:

# Check lines 173-180
sed -n '173,180p' src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 386


🏁 Script executed:

# Find all session_memory.lock() call sites in pipeline.rs
rg -n "session_memory\.lock\(\)" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 248


🏁 Script executed:

# Check if there's any task spawning or background work mentioned
rg -n "spawn\|tokio\|thread" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Let's also check the broader context around session_memory to see if background tasks are spawned
grep -n "extract_session_memory\|background\|spawn" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 566


🏁 Script executed:

# Check the complete should_extract_session_memory and session_memory_snapshot methods
sed -n '166,185p' src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 867


🏁 Script executed:

# Look for the turn.rs file mentioned in comments to see background task spawning
find . -name "turn.rs" -path "*/openhuman/*" | head -3

Repository: tinyhumansai/openhuman

Length of output: 111


🏁 Script executed:

# Check if there are error handling or logging patterns elsewhere in the codebase for Mutex
rg -A 3 "Mutex.*lock\(\)" src/openhuman/context/pipeline.rs | head -30

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Check if there are other Mutex usages in the codebase with poison handling
rg -B 2 -A 2 "unwrap_or_else.*into_inner\|into_inner.*Mutex" src/

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Check the turn.rs file for spawn_session_memory_extraction and background task handling
grep -n "spawn_session_memory_extraction\|session_memory" src/openhuman/agent/harness/session/turn.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 1215


🏁 Script executed:

# Get more context around those lines
grep -B 5 -A 10 "spawn_session_memory_extraction" src/openhuman/agent/harness/session/turn.rs

Repository: tinyhumansai/openhuman

Length of output: 2396


🏁 Script executed:

# Check what types of operations might panic or cause Mutex poison in the spawned task
rg -B 3 -A 8 "fn spawn_session_memory_extraction" src/openhuman/agent/harness/session/turn.rs

Repository: tinyhumansai/openhuman

Length of output: 745


🏁 Script executed:

# Get the full spawn_session_memory_extraction function to see what the background task does
sed -n '787,860p' src/openhuman/agent/harness/session/turn.rs

Repository: tinyhumansai/openhuman

Length of output: 3539


🏁 Script executed:

# Check if the background task calls mark_session_memory_finished which might access the Mutex
grep -n "mark_session_memory_finished\|mark_session_memory" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Get the implementation of mark_session_memory_finished to see if it accesses session_memory
rg -B 2 -A 10 "fn mark_session_memory_finished" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Search for all methods that access session_memory in pipeline.rs
rg -n "fn.*session_memory|fn mark_" src/openhuman/context/pipeline.rs

Repository: tinyhumansai/openhuman

Length of output: 384


🏁 Script executed:

# Check what mark_extraction_complete and mark_extraction_failed do
rg -B 2 -A 5 "mark_extraction_complete\|mark_extraction_failed" src/openhuman/context/session_memory.rs

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Verify that Mutex poisoning can actually occur - check if there are any panics or unwrap() in the Mutex guard usage
rg -n "panic\|unwrap\|expect" src/openhuman/context/session_memory.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Search for mark_extraction_complete in session_memory.rs
grep -n "mark_extraction_complete\|mark_extraction_failed\|mark_session_memory" src/openhuman/context/session_memory.rs

Repository: tinyhumansai/openhuman

Length of output: 319


🏁 Script executed:

# Look at the entire session_memory.rs file structure
grep -n "fn " src/openhuman/context/session_memory.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 970


🏁 Script executed:

# Check if there's any possibility of panic inside the SessionMemoryState methods
sed -n '1,200p' src/openhuman/context/session_memory.rs | grep -n "panic\|unwrap\|expect"

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Check the actual implementation of mark_extraction_complete
rg -B 1 -A 8 "pub fn mark_extraction" src/openhuman/context/session_memory.rs

Repository: tinyhumansai/openhuman

Length of output: 1116


🏁 Script executed:

# Check if run_subagent or any function called in the background task could panic
rg -n "panic\|unwrap\|expect" src/openhuman/agent/harness/session/turn.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Look at what methods are called in the methods that access session_memory
sed -n '93,160p' src/openhuman/context/session_memory.rs

Repository: tinyhumansai/openhuman

Length of output: 2989


🏁 Script executed:

# Check if there are any tests that verify behavior after Mutex poisoning
rg -n "poisoned\|PoisonError" src/

Repository: tinyhumansai/openhuman

Length of output: 48


🏁 Script executed:

# Verify the Rust Mutex poison semantics and recovery pattern
web_search
Search for: "Rust std::sync::Mutex PoisonError recovery into_inner best practice"

Repository: tinyhumansai/openhuman

Length of output: 161


🌐 Web query:

Rust std::sync::Mutex PoisonError recovery into_inner best practice

💡 Result:

The best practice for recovering from a std::sync::Mutex PoisonError using into_inner is to explicitly handle the Result from lock and call into_inner on the PoisonError to obtain the MutexGuard, allowing access to the protected data despite potential corruption from a prior panic. This approach acknowledges that poisoning is advisory—data may be in an inconsistent state, but memory safety is preserved. Official docs recommend this pattern for recovery, while noting that most code simply uses unwrap to propagate panics. Example from Rust std docs: let mut guard = match mutex.lock { Ok(guard) => guard, Err(poisoned) => poisoned.into_inner, }; *guard += 1; Convenience patterns include mutex.lock.unwrap_or_else(|e| e.into_inner). For production code, perform recovery logic (e.g., reset state, log) before using the guard, as blindly ignoring poison risks logical errors. Use Mutex::clear_poison (unstable) only after recovery if needed. Unsafe code cannot rely on poisoning for soundness.

Citations:


🏁 Script executed:

# Check if any background tasks in turn.rs could panic while holding the lock
rg -B 5 -A 5 "harness::run_subagent" src/openhuman/agent/harness/session/turn.rs

Repository: tinyhumansai/openhuman

Length of output: 547


🏁 Script executed:

# Let's also see if there are any error cases in the session_memory operations
sed -n '93,160p' src/openhuman/context/session_memory.rs

Repository: tinyhumansai/openhuman

Length of output: 2989


Recover poisoned session-memory locks instead of silently dropping updates.

std::sync::Mutex::lock() returns Err(PoisonError) after any panic before or while the guard is held. These branches currently ignore poisoning with if let Ok(...) or .unwrap_or(...), causing session-memory state updates to silently drop and extraction checks to return false permanently if poisoning ever occurs. Use unwrap_or_else(|e| e.into_inner()) to recover the guard despite poisoning, optionally logging a warning.

The same pattern appears in the background task spawned from turn.rs (lines 838, 845), which should use the same recovery strategy.

♻️ Minimal recovery pattern
-        if let Ok(mut sm) = self.session_memory.lock() {
-            sm.record_usage(total);
-        }
+        let mut sm = self.session_memory.lock().unwrap_or_else(|e| {
+            tracing::warn!("[context_pipeline] session-memory lock poisoned; recovering");
+            e.into_inner()
+        });
+        sm.record_usage(total);

Apply to all five call sites: record_usage (144), tick_turn (151), record_tool_calls (159), should_extract_session_memory (166–170), and session_memory_snapshot (175–180).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/context/pipeline.rs` around lines 141 - 170, The session-memory
Mutex guard is currently ignored on poisoning (using if let Ok or
unwrap_or(false)), causing updates and checks in record_usage, tick_turn,
record_tool_calls, should_extract_session_memory, and session_memory_snapshot to
be silently dropped; change each lock() call to recover the poisoned guard with
.lock().unwrap_or_else(|e| e.into_inner()) (and optionally emit a warning via
the logger) so the guard is accessed even after a panic; apply the same recovery
pattern in the background task in turn.rs where session_memory is locked.

Comment on lines +549 to +561
// 3c. Optional skills catalogue. Off by default because sub-agents
// usually skip skills entirely. Kept here so a custom
// definition can opt in without falling back to the general
// builder. The renderer intentionally takes no `skills` slice
// — the caller would have to extend this helper before
// enabling this flag for real, which keeps the common (narrow)
// path free of extra arguments.
if options.include_skills_catalog {
out.push_str("## Available Skills\n\n");
out.push_str(
"Skills are loaded on demand. Use `read` on the skill path to get full instructions.\n\n",
);
}
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.

⚠️ Potential issue | 🟠 Major

include_skills_catalog does not actually render a skills catalog.

This branch only prints a generic placeholder, but run_typed_mode() now threads definition.omit_skills_catalog straight into this renderer. Any definition that opts into skills will not receive the actual skill names/paths that SkillsSection exposes, so the shared renderer no longer matches the advertised flag semantics. Either pass &[Skill] through and reuse the real skills rendering, or force this option off until it is fully supported.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/context/prompt.rs` around lines 549 - 561, The branch
controlled by options.include_skills_catalog only emits a placeholder and
doesn't render actual skills, so thread the real skills data into this renderer
or disable the flag: update the caller (run_typed_mode) to pass a &[Skill] (or
Option<&[Skill]>) down into this module and change the renderer to, when
include_skills_catalog is true and a skills slice is provided, call the same
rendering logic used by SkillsSection to emit each skill name/path and
description instead of the generic text; alternatively, if you cannot plumb
skills now, make include_skills_catalog false/unavailable by removing exposure
from run_typed_mode/definition.omit_skills_catalog so callers can't opt in until
full support is implemented.

@senamakel senamakel merged commit 986ff7a into tinyhumansai:main Apr 11, 2026
7 of 9 checks passed
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
* refactor(context): implement layered context pipeline with memory management

- Introduced a new context pipeline orchestrator that manages context reduction stages before each provider call, including tool-result budgeting, microcompaction, and session memory management.
- Added `ContextGuard` to monitor context utilization and trigger auto-compaction when thresholds are exceeded.
- Implemented `microcompact` to replace older tool result payloads with placeholders, preserving API invariants while managing memory effectively.
- Created `SessionMemory` to maintain persistent notes across sessions, updated by a background process to avoid impacting performance during user interactions.
- Enhanced overall context management to improve memory efficiency and ensure smoother operation of the agent's functionality.

* chore(context): add context module and refactor imports

- Introduced a new `context` module to centralize context management for agent sessions, enhancing organization and maintainability.
- Updated various files to replace references to the deprecated `context_pipeline` with the new `context` module, ensuring consistency across the codebase.
- Bumped the version of the `openhuman` package to 0.52.4 in `Cargo.lock` to reflect these changes.

* refactor(imports): update prompt module paths to new context structure

- Refactored import statements across multiple files to replace references to the deprecated `agent::prompt` module with the new `context::prompt` module.
- This change enhances code organization and aligns with the recent restructuring of the context management system.

* feat(context): introduce comprehensive context management configuration

- Added a new `ContextConfig` struct to manage global context settings, including budget thresholds, summarization triggers, and session-memory extraction parameters.
- Implemented environment variable overrides for context management settings, allowing dynamic configuration.
- Refactored the configuration loading process to accommodate the new context management structure, ensuring backward compatibility with existing configurations.
- Introduced a `Summarizer` trait and default implementation for handling conversation summarization, enhancing the agent's ability to manage context effectively.
- Updated related modules to integrate the new context management features, improving overall organization and maintainability of the codebase.

* feat(context): implement ContextManager for enhanced session context handling

- Introduced a new `ContextManager` struct to manage session context, including prompt assembly, context reduction, and summarization dispatch.
- Added methods to `ContextGuard` for tracking input and output token counts, as well as the context window size.
- Updated the `mod.rs` files to include the new `manager` module and its exports, improving organization and maintainability of the context management system.
- Enhanced overall context management capabilities, allowing for more efficient handling of conversation context during agent interactions.

* refactor(context): centralize system prompt assembly for channel runtimes

- Moved the system prompt construction logic for channel runtimes into a new `channels_prompt` module within the `context` directory, ensuring all prompt-building code is organized in one location.
- Updated various files to replace deprecated references to the old prompt module, enhancing code clarity and maintainability.
- Introduced a new `build_system_prompt` function tailored for channel-specific requirements, including tool descriptions and channel-specific preambles, while maintaining byte-stability for cache efficiency.
- Refactored related imports and tests to align with the new structure, improving overall organization of the context management system.

* style(context): apply rustfmt to context module and integration sites

Clippy caught a never-looping `while` in `snap_split_forward` that was
really an `if` in disguise — collapse it. Everything else is pure
`cargo fmt` cleanup on files touched during the context/ refactor.

Full `cargo test --lib` sweep: 2253 tests passing.

* delete(config): remove obsolete user configuration file

- Deleted the `config.toml` file for user `69ccc8e95692bb0ddd56c10f`, which was no longer needed, streamlining the user configuration management.

* refactor(SOUL.md): remove emoji usage guidelines to streamline prompt instructions

- Deleted the section on emoji usage to simplify the prompt guidelines, focusing on clarity and direct communication with users. This change aims to enhance the overall effectiveness of the prompts by reducing unnecessary complexity.

* refactor(config): enhance environment variable handling for context compaction settings

- Improved the parsing and validation of environment variables for `OPENHUMAN_CONTEXT_COMPACTION_TRIGGER_PCT` and `OPENHUMAN_CONTEXT_HARD_LIMIT_PCT`, ensuring that the compaction trigger percentage is strictly less than the hard limit.
- Added warnings for invalid percentage values and retained existing configuration values on failure.
- Updated the context management logic to handle migration from deprecated fields, ensuring backward compatibility and clearer user guidance.

* refactor(context): update tool result budget handling in Agent

- Changed the source of the tool result budget to be derived from the ContextManager, ensuring it reflects the resolved `context.tool_result_budget_bytes` with any environment or configuration overrides.
- This update removes reliance on the deprecated `agent.tool_result_budget_bytes` field, centralizing budget management for improved consistency across the context management system.

* refactor(prompt): enhance subagent rendering options and system prompt construction

- Introduced `SubagentRenderOptions` to manage per-definition rendering flags, allowing for more flexible control over the inclusion of identity, safety preamble, and skills catalog in the system prompt.
- Updated `render_subagent_system_prompt` to utilize these options, ensuring that the rendering behavior aligns with the specified flags.
- Modified `build_system_prompt` to accept an optional `channel_name`, improving clarity in channel capabilities messaging and removing hardcoded references to "Discord".
- Adjusted tests to reflect changes in prompt construction, ensuring comprehensive coverage of the new rendering logic.

* refactor(context): improve session memory handling in ContextManager

- Updated session memory methods in `ContextManager` to use a locked reference for thread safety during extraction state changes.
- Introduced a new method `session_memory_handle` to facilitate background task management of session memory extraction.
- Adjusted the context statistics to utilize a snapshot of session memory, enhancing data integrity and consistency.

* refactor(turn): improve extraction state management in Agent

- Updated the extraction state handling to transition to "in-progress" instead of "complete" immediately, allowing for better control during background extraction tasks.
- Introduced a shared handle for session memory to manage extraction completion and failure states, ensuring deltas are preserved for retries on failure.
- Enhanced logging to provide clearer insights into the extraction process outcomes.
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.

1 participant