feat(memory): expose compiled flavour profiles via a read-only memory_flavour tool - #5175
Conversation
…sona profiles Persona ingestion distills a person's coding-agent history into seven PersonaFacet flavoured trees (communication, coding_style, stack, workflow, environment, directives, anti_preferences), each compiled into a small prompt-ready markdown profile via tinycortex's compile_flavoured_root. Until now nothing surfaced those compiled profiles to the agent loop — the ingested data sat unread. memory_flavour lets an agent pull one facet's profile on demand. It is genuinely read-only: both permission_level() and permission_level_with_args() are overridden to unconditionally return ReadOnly (not left to the trait default), and an unbuilt/empty profile returns a clear "no profile built yet" ToolResult::success message rather than an error or a fabricated profile. Closes tinyhumansai#5172
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds the read-only ChangesMemory flavour retrieval
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant ToolRegistry
participant MemoryFlavourTool
participant TreeStore
Agent->>ToolRegistry: Request memory_flavour
ToolRegistry->>MemoryFlavourTool: Execute flavour argument
MemoryFlavourTool->>MemoryFlavourTool: Read compiled profile
MemoryFlavourTool->>TreeStore: Lookup and compile when needed
TreeStore-->>MemoryFlavourTool: Return tree or no tree
MemoryFlavourTool-->>Agent: Return profile or no-profile response
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
Collapse the multi-line anyhow!() in parse_loose error handling onto one line per rustfmt. The other Rust Quality diffs on this branch (caps.rs, provider.rs) are pre-existing on upstream/main (introduced by tinyhumansai#5151 et al.) — main head e780277 already fails Rust Quality — and are not touched by this PR.
…-tool # Conflicts: # tests/json_rpc_e2e.rs
coderabbitai
Bot
added
feature
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47fb6f5017
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // (communication/coding_style/stack/workflow/environment/directives/ | ||
| // anti_preferences) that persona ingestion builds but nothing | ||
| // previously surfaced to the agent loop. | ||
| Box::new(MemoryFlavourTool::new(config.clone())), |
There was a problem hiding this comment.
Registering the tool here makes it part of the global registry, but normal chat sessions are built against the orchestrator definition and memory work is delegated to agent_memory/profile_memory_agent, all of which use ToolScope::Named allowlists. I checked those TOML allowlists and none include memory_flavour, so in the actual CLI/Tauri chat paths the harness filters this tool out and the model never sees or calls it; only wildcard/custom sessions would. Please add the tool to the appropriate named allowlist(s) and prompt guidance, or this feature remains unreachable for the intended agent loop.
Useful? React with 👍 / 👎.
| Ok(ToolResult::success(format!( | ||
| "Profile for {heading} could not be compiled: {err}" | ||
| ))) |
There was a problem hiding this comment.
When a flavoured tree exists but compilation fails, for example because the tree state is corrupt or the compiled artifact path cannot be written, this branch returns ToolResult::success(...), so is_error stays false and the harness/telemetry records a failed profile read as a successful tool call; the lookup-error branch below does the same. Return ToolResult::error or propagate for real store/compile failures, reserving success for the expected “no profile built yet” case.
Useful? React with 👍 / 👎.
|
| Filename | Overview |
|---|---|
| src/openhuman/memory/tools/flavour.rs | New MemoryFlavourTool: fast-path disk read + slow-path tree-store lookup; both paths correctly strip YAML front matter via body_after_front_matter before returning; debug tracing on every branch; comprehensive unit tests. All three issues from the prior review cycle are resolved. |
| src/openhuman/memory/tools.rs | Adds mod flavour and re-exports MemoryFlavourTool alongside the existing memory tools; no functional changes to other tools. |
| src/openhuman/tools/ops.rs | Registers MemoryFlavourTool in all_tools_with_runtime between MemoryDoctorTool and MemoryQueryTool; comment ties registration back to issue #5172. |
| tests/json_rpc_e2e.rs | Adds memory_flavour_agent_tool_e2e_5172: verifies tool name, no-profile-yet success path, and unknown-slug rejection — covers registration reachability end-to-end. |
| src/openhuman/agent_memory/agent/agent.toml | Adds memory_flavour to the agent_memory agent's named tool allowlist with an explanatory comment. |
| src/openhuman/agent_registry/agents/context_scout/agent.toml | Adds memory_flavour to the context_scout agent's named tool allowlist; read-only and safe for the scout's usage pattern. |
| src/openhuman/agent_registry/agents/profile_memory_agent/agent.toml | Adds memory_flavour to the profile_memory_agent's named tool allowlist alongside other read tools. |
Sequence Diagram
sequenceDiagram
participant Agent
participant MemoryFlavourTool
participant FS as Filesystem
participant TreeStore as TreeStore (SQLite)
Agent->>MemoryFlavourTool: "execute({flavour: "coding_style"})"
MemoryFlavourTool->>MemoryFlavourTool: PersonaFacet::parse_loose()
alt Invalid / empty slug
MemoryFlavourTool-->>Agent: Err(anyhow)
end
MemoryFlavourTool->>FS: is_file(flavoured_root_abs_path)
alt Fast path - artifact exists on disk
FS-->>MemoryFlavourTool: read_to_string Ok(content)
MemoryFlavourTool->>MemoryFlavourTool: body_after_front_matter(content)
alt body non-empty
MemoryFlavourTool-->>Agent: ToolResult::success(body)
end
end
MemoryFlavourTool->>TreeStore: get_tree_by_scope(Flavoured, scope)
alt No tree found
TreeStore-->>MemoryFlavourTool: Ok(None)
MemoryFlavourTool-->>Agent: ToolResult::success(No profile built yet)
else Lookup error
TreeStore-->>MemoryFlavourTool: Err(e)
MemoryFlavourTool-->>Agent: ToolResult::error(Failed to look up)
else Tree found
TreeStore-->>MemoryFlavourTool: Ok(Some(tree))
MemoryFlavourTool->>FS: compile_flavoured_root(mc, tree.id)
alt Compile succeeds, body non-empty
FS-->>MemoryFlavourTool: Ok(markdown)
MemoryFlavourTool->>MemoryFlavourTool: body_after_front_matter(markdown)
MemoryFlavourTool-->>Agent: ToolResult::success(body)
else Compile succeeds, body empty
FS-->>MemoryFlavourTool: Ok(markdown)
MemoryFlavourTool-->>Agent: ToolResult::success(No profile built yet)
else Compile error
FS-->>MemoryFlavourTool: Err(e)
MemoryFlavourTool-->>Agent: ToolResult::error(Failed to compile)
end
end
Reviews (2): Last reviewed commit: "fix(memory): address review — strip fron..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/openhuman/memory/tools/flavour.rs (1)
171-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the actual successful-profile path.
Every test here covers validation, permissions, or the "no profile built yet" fallback. None verifies the primary feature: reading a profile that actually has content (either via the fast-path file read at
flavoured_root_abs_path, or viacompile_flavoured_rootreturning a non-empty body). Given the PR's stated goal — "Return profiles for facets such ascommunicationandcoding_style" — this success path is untested.A test could write a fake compiled artifact (front matter + body) directly to the path returned by
flavoured_root_abs_path(&mc, &scope)and assertexecutereturns that body via the fast path, exercisingbody_after_front_matterend-to-end too.🤖 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/tools/flavour.rs` around lines 171 - 262, Add a success-path test alongside the existing MemoryFlavourTool tests that creates a compiled profile artifact at the path produced by flavoured_root_abs_path for a valid flavour and scope, including front matter and a non-empty body. Execute the tool and assert it succeeds, is not an error, and returns the profile body, thereby exercising the fast-path file read and body_after_front_matter behavior.tests/json_rpc_e2e.rs (1)
14841-14887: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest bypasses JSON-RPC dispatch despite living in the JSON-RPC E2E suite.
memory_flavour_agent_tool_e2e_5172constructsMemoryFlavourTooldirectly and callsexecute(), which is unit-level coverage, not an actual JSON-RPC round trip. It doesn't confirm the tool is reachable/permission-enforced through the realtool_callJSON-RPC dispatch path referenced in the PR objectives.Consider routing this test through the same JSON-RPC dispatch helper other tests in this file use for tool-call verification, or moving it to a plain unit test file if a dispatch-level test already exists elsewhere for
memory_flavour.
As per coding guidelines, "Follow the workflow: specify, prove in Rust, prove over JSON-RPC, surface in the UI, and test at unit and E2E levels."🤖 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/json_rpc_e2e.rs` around lines 14841 - 14887, Update memory_flavour_agent_tool_e2e_5172 to invoke memory_flavour through the existing JSON-RPC tool_call dispatch helper used by nearby tests instead of constructing MemoryFlavourTool and calling execute() directly. Preserve assertions for successful no-profile behavior and unknown-flavour rejection, while ensuring the round trip exercises registration and permission enforcement; if equivalent dispatch coverage already exists, move these direct-execution assertions to an appropriate unit test instead.Source: Coding guidelines
🤖 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/tools/flavour.rs`:
- Around line 130-167: Update the outer Err(err) arm handling get_tree_by_scope
in the flavoured-profile lookup flow to report a tree lookup failure rather than
reusing the compile-failure message. Keep the existing compile-specific message
in the compile_flavoured_root error arm unchanged, and preserve the current
error details and successful result behavior.
---
Nitpick comments:
In `@src/openhuman/memory/tools/flavour.rs`:
- Around line 171-262: Add a success-path test alongside the existing
MemoryFlavourTool tests that creates a compiled profile artifact at the path
produced by flavoured_root_abs_path for a valid flavour and scope, including
front matter and a non-empty body. Execute the tool and assert it succeeds, is
not an error, and returns the profile body, thereby exercising the fast-path
file read and body_after_front_matter behavior.
In `@tests/json_rpc_e2e.rs`:
- Around line 14841-14887: Update memory_flavour_agent_tool_e2e_5172 to invoke
memory_flavour through the existing JSON-RPC tool_call dispatch helper used by
nearby tests instead of constructing MemoryFlavourTool and calling execute()
directly. Preserve assertions for successful no-profile behavior and
unknown-flavour rejection, while ensuring the round trip exercises registration
and permission enforcement; if equivalent dispatch coverage already exists, move
these direct-execution assertions to an appropriate unit test instead.
🪄 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: 0f26aeee-4cde-43d8-a7b4-16da7e515a51
📒 Files selected for processing (4)
src/openhuman/memory/tools.rssrc/openhuman/memory/tools/flavour.rssrc/openhuman/tools/ops.rstests/json_rpc_e2e.rs
| match get_tree_by_scope(&mc, TreeKind::Flavoured, &scope) { | ||
| Ok(None) => Ok(ToolResult::success(format!( | ||
| "No profile built yet for {heading}. Run persona ingestion first, then try \ | ||
| again." | ||
| ))), | ||
| Ok(Some(tree)) => match compile_flavoured_root(&mc, &tree.id) { | ||
| Ok(markdown) => { | ||
| if body_after_front_matter(&markdown).trim().is_empty() { | ||
| Ok(ToolResult::success(format!( | ||
| "No profile built yet for {heading}. Run persona ingestion first, \ | ||
| then try again." | ||
| ))) | ||
| } else { | ||
| Ok(ToolResult::success(markdown)) | ||
| } | ||
| } | ||
| Err(err) => { | ||
| tracing::warn!( | ||
| %err, | ||
| flavour = flavour_raw, | ||
| "[memory_flavour] failed to compile flavoured profile" | ||
| ); | ||
| Ok(ToolResult::success(format!( | ||
| "Profile for {heading} could not be compiled: {err}" | ||
| ))) | ||
| } | ||
| }, | ||
| Err(err) => { | ||
| tracing::warn!( | ||
| %err, | ||
| flavour = flavour_raw, | ||
| "[memory_flavour] failed to look up flavoured tree" | ||
| ); | ||
| Ok(ToolResult::success(format!( | ||
| "Profile for {heading} could not be compiled: {err}" | ||
| ))) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Lookup failure is mislabeled as a compile failure.
The Err(err) arm at Line 157 (from get_tree_by_scope, a tree-lookup failure) reuses the same "Profile for {heading} could not be compiled: {err}" message as the genuine compile failure at Line 146. An agent/user reading this will think compilation broke when the tree lookup itself failed, which muddies troubleshooting.
🐛 Proposed fix to distinguish lookup vs compile failures
Err(err) => {
tracing::warn!(
%err,
flavour = flavour_raw,
"[memory_flavour] failed to look up flavoured tree"
);
Ok(ToolResult::success(format!(
- "Profile for {heading} could not be compiled: {err}"
+ "Profile for {heading} could not be looked up: {err}"
)))
}📝 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.
| match get_tree_by_scope(&mc, TreeKind::Flavoured, &scope) { | |
| Ok(None) => Ok(ToolResult::success(format!( | |
| "No profile built yet for {heading}. Run persona ingestion first, then try \ | |
| again." | |
| ))), | |
| Ok(Some(tree)) => match compile_flavoured_root(&mc, &tree.id) { | |
| Ok(markdown) => { | |
| if body_after_front_matter(&markdown).trim().is_empty() { | |
| Ok(ToolResult::success(format!( | |
| "No profile built yet for {heading}. Run persona ingestion first, \ | |
| then try again." | |
| ))) | |
| } else { | |
| Ok(ToolResult::success(markdown)) | |
| } | |
| } | |
| Err(err) => { | |
| tracing::warn!( | |
| %err, | |
| flavour = flavour_raw, | |
| "[memory_flavour] failed to compile flavoured profile" | |
| ); | |
| Ok(ToolResult::success(format!( | |
| "Profile for {heading} could not be compiled: {err}" | |
| ))) | |
| } | |
| }, | |
| Err(err) => { | |
| tracing::warn!( | |
| %err, | |
| flavour = flavour_raw, | |
| "[memory_flavour] failed to look up flavoured tree" | |
| ); | |
| Ok(ToolResult::success(format!( | |
| "Profile for {heading} could not be compiled: {err}" | |
| ))) | |
| } | |
| } | |
| match get_tree_by_scope(&mc, TreeKind::Flavoured, &scope) { | |
| Ok(None) => Ok(ToolResult::success(format!( | |
| "No profile built yet for {heading}. Run persona ingestion first, then try \ | |
| again." | |
| ))), | |
| Ok(Some(tree)) => match compile_flavoured_root(&mc, &tree.id) { | |
| Ok(markdown) => { | |
| if body_after_front_matter(&markdown).trim().is_empty() { | |
| Ok(ToolResult::success(format!( | |
| "No profile built yet for {heading}. Run persona ingestion first, \ | |
| then try again." | |
| ))) | |
| } else { | |
| Ok(ToolResult::success(markdown)) | |
| } | |
| } | |
| Err(err) => { | |
| tracing::warn!( | |
| %err, | |
| flavour = flavour_raw, | |
| "[memory_flavour] failed to compile flavoured profile" | |
| ); | |
| Ok(ToolResult::success(format!( | |
| "Profile for {heading} could not be compiled: {err}" | |
| ))) | |
| } | |
| }, | |
| Err(err) => { | |
| tracing::warn!( | |
| %err, | |
| flavour = flavour_raw, | |
| "[memory_flavour] failed to look up flavoured tree" | |
| ); | |
| Ok(ToolResult::success(format!( | |
| "Profile for {heading} could not be looked up: {err}" | |
| ))) | |
| } | |
| } |
🤖 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/tools/flavour.rs` around lines 130 - 167, Update the
outer Err(err) arm handling get_tree_by_scope in the flavoured-profile lookup
flow to report a tree lookup failure rather than reusing the compile-failure
message. Keep the existing compile-specific message in the
compile_flavoured_root error arm unchanged, and preserve the current error
details and successful result behavior.
…res, reachability, tracing Three bots flagged real issues on the memory_flavour tool (PR tinyhumansai#5175): - Greptile P1: both read paths (fast disk-cache hit and the slow compile-then-read path) returned the raw compiled artifact, including its `---\n...\n---\n` YAML front matter, despite the tool description promising markdown prose. Both paths now strip to the body via body_after_front_matter() before returning. - Greptile P2: body_after_front_matter's malformed-front-matter fallback (opener present, no closer) returned the full original content, leaking the `---` opener. Now falls back to `rest` (everything after the opener). - Codex P2 / CodeRabbit: genuine failures (tree lookup error, compile error) were wrapped in ToolResult::success(...), so is_error stayed false and telemetry recorded failed reads as successes. Now returns ToolResult::error(...) with distinct "look up" vs "compile" wording; the expected "no profile built yet" case is unchanged (still success). - Greptile P2: added tracing::debug! at entry, fast-path hit, fast->slow fallthrough, tree-lookup outcome (none vs found+compiling), and compiled body return, per CLAUDE.md's verbose-diagnostics rule. Logs slugs/lengths only, never profile content. - Codex P2 (reachability): the tool was registered globally but excluded from every agent's `[tools] named` allowlist, so ToolScope::Named filtering hid it from the harness entirely. Added "memory_flavour" (with a prompt note) to agent_memory, profile_memory_agent, and context_scout — the three read-oriented memory/profile agents where it belongs.
coderabbitai
Bot
added
agent
1 participant
Summary
memory_flavour, a new read-only agent tool that reads the compiled persona flavour profile for one of the sevenPersonaFacetlenses (communication, coding_style, stack, workflow, environment, directives, anti_preferences).tinycortexflavoured-tree profiles (compiled viacompile_flavoured_root) into the agent tool surface for the first time — persona ingestion built these profiles but nothing surfaced them to the agent loop.permission_level()andpermission_level_with_args()are overridden to unconditionally returnReadOnly, avoiding the args-aware permission gap thememory_treetool previously had.ToolResult::success("No profile built yet for <facet>. Run persona ingestion first, then try again.")— never an error, never a fabricated profile.all_tools_with_runtimealongside the othermemory_*tools; classified underDomainGroup::Memoryautomatically via the existingmemory_name-prefix routing (no gating changes needed).Problem
src/openhuman/tinycortex/persona.rs) already distills a user's coding-agent history into seven flavoured trees and compiles each into a small prompt-ready markdown profile, but no agent tool ever read that compiled output back. The data was computed and staged to disk, then never used (feat(memory): expose compiled flavour profiles via a read-only memory_flavour tool #5172).Solution
MemoryFlavourTool(src/openhuman/memory/tools/flavour.rs), mirroring the existingmemory_doctor/memory_recalltool shape (Arc<Config>, mapped viamemory_config_from).execute()validates theflavourarg viaPersonaFacet::parse_loose(accepts aliases likecomms,coding,env,rules,dislikes), then:get_tree_by_scope(..., TreeKind::Flavoured, ...)and (re)compiles its root viacompile_flavoured_root; a missing tree or an empty compiled body both resolve to the same clear "no profile built yet" success message rather than an error.compile_flavoured_root/lookupErris logged (tracing::warn!) and surfaced as a non-error "could not be compiled" message, never a fabricated profile.Submission Checklist
flavour.rs(name/schema, both permission overrides, missing/empty/unknown flavour errors, valid-flavour-no-tree-yet success, 5 facet aliases) plus a newtests/json_rpc_e2e.rs::memory_flavour_agent_tool_e2e_5172covering registration reachability, the no-profile-yet success path, and the unknown-flavour error path.cargo llvm-cov/pnpm test:rustlocally in this environment (no-build constraint on this task) — the new file is fully covered by the added unit + e2e tests; CI'srust-core-coveragelane will validate the ≥80% diff-coverage gate.memory_doctor/memory_recall) has a row in the coverage matrix; this follows the same convention.tinycortexengine, no new crates.Closes #5172below.Impact
src/openhuman/memory/tools/flavour.rs,src/openhuman/memory/tools.rs,src/openhuman/tools/ops.rs); no frontend or Tauri shell changes. No new dependencies, no feature-gate changes.Related
Summary by CodeRabbit
memory_flavourtool to retrieve compiled persona “flavour” profiles (including accepted aliases).memory_flavourinto the default tool registry and enabled it for relevant memory/agent flows via configuration and retrieval prompts.