feat(learning): explicit user-preference tool + always-on pinned-facet prompt injection#2150
Merged
graycyrus merged 3 commits intoMay 19, 2026
Merged
Conversation
Contributor
|
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 (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughAdds an explicit-preferences mode: config and agent wiring to enable pinned user preferences independent of learning, conditional fetch logic to return only pinned preferences when configured, a RememberPreferenceTool to persist pinned prefs, tests, and registry integration. ChangesExplicit Preferences Feature
Sequence DiagramsequenceDiagram
participant User
participant RememberPreferenceTool
participant SecurityPolicy
participant Memory
User->>RememberPreferenceTool: execute(class, key, value)
RememberPreferenceTool->>SecurityPolicy: enforce_tool_operation(Act, remember_preference)
alt Security denied
SecurityPolicy-->>RememberPreferenceTool: error
RememberPreferenceTool-->>User: ToolResult::error
else Security allowed
SecurityPolicy-->>RememberPreferenceTool: ok
RememberPreferenceTool->>RememberPreferenceTool: validate class/key/value
RememberPreferenceTool->>Memory: store(namespace="user_profile", key=pinned/{class}/{key}, content="[pinned] ...", MemoryCategory::Core)
Memory-->>RememberPreferenceTool: stored
RememberPreferenceTool-->>User: ToolResult::success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/openhuman/config/schema/load.rs (1)
1282-1289: ⚡ Quick winUse
parse_env_boolfor this new env flag to avoid silent misconfigurations.Line 1282 currently reimplements bool parsing and drops invalid values silently. Reusing the shared parser keeps behavior consistent and emits the warning that this file already relies on for other env flags.
Suggested patch
if let Some(flag) = env.get("OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED") { - let normalized = flag.trim().to_ascii_lowercase(); - match normalized.as_str() { - "1" | "true" | "yes" | "on" => self.learning.explicit_preferences_enabled = true, - "0" | "false" | "no" | "off" => self.learning.explicit_preferences_enabled = false, - _ => {} - } + if let Some(enabled) = parse_env_bool( + "OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED", + &flag, + ) { + self.learning.explicit_preferences_enabled = enabled; + } }🤖 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/config/schema/load.rs` around lines 1282 - 1289, The block manually parsing OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED should use the shared parse_env_bool helper instead of reimplementing parsing and swallowing invalid values; call parse_env_bool(env, "OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED") (or the existing helper signature used elsewhere) and, if it returns Some(bool), assign it to self.learning.explicit_preferences_enabled so the same warning/logging behavior and consistent parsing are used across the file.src/openhuman/tools/impl/agent/remember_preference.rs (1)
305-597: ⚡ Quick winMove the test module into a sibling
remember_preference_test.rsfile.This module is already large, and the repo guidance prefers sibling Rust test files over big inline
mod testsblocks for new functionality.As per coding guidelines, "When extracting Rust tests out of an implementation file, prefer a sibling `*_test.rs` file wired in with `#[cfg(test)] #[path = \"..._test.rs\"] mod tests;`."♻️ Proposed wiring change
-#[cfg(test)] -mod tests { - // moved to remember_preference_test.rs -} +#[cfg(test)] +#[path = "remember_preference_test.rs"] +mod tests;🤖 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/tools/impl/agent/remember_preference.rs` around lines 305 - 597, The tests in the inline mod tests block should be moved to a sibling remember_preference_test.rs file and the implementation file should be wired to it; extract the entire #[cfg(test)] mod tests { ... } block into that new file, preserve all test functions (e.g., facet_class_parse_case_insensitive, pinned_key_format, stores_preference_in_user_profile_namespace, idempotent_overwrite_does_not_create_duplicate, stores_all_six_classes, blocked_in_readonly_mode) and imports (NoopEmbedding, UnifiedMemory, SecurityPolicy, TempDir, json), then in the original remember_preference.rs add at top (or near other module-level test wiring) the line: #[cfg(test)] #[path = "remember_preference_test.rs"] mod tests; so tests still reference symbols like RememberPreferenceTool, FacetClass, pinned_key, PINNED_PREFERENCES_NAMESPACE and compile against the same crate items.
🤖 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/agent/harness/session/turn.rs`:
- Around line 1273-1277: The namespace filter currently allows rows with missing
namespaces because it uses as_deref().map_or(true, |ns| ns == "user_profile");
change the predicate to require an explicit "user_profile" namespace (e.g., use
map_or(false, |ns| ns == "user_profile") or as_deref().map(|ns| ns ==
"user_profile").unwrap_or(false)) so that only entries where e.namespace ==
"user_profile" are kept; update the closure around e.namespace/as_deref() in the
filter to enforce this stricter check.
In `@src/openhuman/tools/impl/agent/remember_preference.rs`:
- Around line 175-180: The debug logs in remember_preference are currently
emitting potentially sensitive preference values (args.get("value")) verbatim;
update the tracing::debug calls inside the remember_preference tool (and the
similar debug block around the later occurrence referenced) to redact or omit
the raw value — e.g., replace the raw value with a redacted marker or log only
its presence/length (such as value.is_some() and value.as_ref().map(|v|
v.len())), keep class and key as before, and ensure any other debug that reads
args.get("value") follows the same pattern so no full PII or profile data is
written to logs.
- Around line 158-160: The validator for preference keys in
remember_preference.rs is too permissive (it currently allows uppercase letters
and '-' contrary to the schema description); update the runtime validation used
in the functions named validate_key and any related validation helpers (e.g.,
validate_keys_in_map / validate_runtime_key) to enforce the snake_case contract
by using a stricter pattern such as ^[a-z0-9_]+$ (only lowercase letters,
digits, and underscores), and adjust the error messages to explicitly state the
allowed characters so stored keys match the advertised schema.
- Around line 243-256: The extracted value currently uses value.trim() but still
allows newlines which can break the line-oriented pinned record; after getting
the string from args.get("value") (the block around args.get(...).and_then(|v|
v.as_str())), normalize it to a single-line string by replacing any \r or \n
(and collapsing consecutive whitespace) into single spaces (e.g.,
split_whitespace().join(" ")), then re-check for empty and return
ToolResult::error("value cannot be empty") if it is empty; use that sanitized
single-line string when building pinned_content(class, key, value) and
pinned_key(class, key).
---
Nitpick comments:
In `@src/openhuman/config/schema/load.rs`:
- Around line 1282-1289: The block manually parsing
OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED should use the shared
parse_env_bool helper instead of reimplementing parsing and swallowing invalid
values; call parse_env_bool(env,
"OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED") (or the existing helper
signature used elsewhere) and, if it returns Some(bool), assign it to
self.learning.explicit_preferences_enabled so the same warning/logging behavior
and consistent parsing are used across the file.
In `@src/openhuman/tools/impl/agent/remember_preference.rs`:
- Around line 305-597: The tests in the inline mod tests block should be moved
to a sibling remember_preference_test.rs file and the implementation file should
be wired to it; extract the entire #[cfg(test)] mod tests { ... } block into
that new file, preserve all test functions (e.g.,
facet_class_parse_case_insensitive, pinned_key_format,
stores_preference_in_user_profile_namespace,
idempotent_overwrite_does_not_create_duplicate, stores_all_six_classes,
blocked_in_readonly_mode) and imports (NoopEmbedding, UnifiedMemory,
SecurityPolicy, TempDir, json), then in the original remember_preference.rs add
at top (or near other module-level test wiring) the line: #[cfg(test)] #[path =
"remember_preference_test.rs"] mod tests; so tests still reference symbols like
RememberPreferenceTool, FacetClass, pinned_key, PINNED_PREFERENCES_NAMESPACE and
compile against the same crate items.
🪄 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: 7f2526f7-d616-4e04-8d36-928907f3d2d6
📒 Files selected for processing (9)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/turn_tests.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/config/schema/learning.rssrc/openhuman/config/schema/load.rssrc/openhuman/tools/impl/agent/mod.rssrc/openhuman/tools/impl/agent/remember_preference.rssrc/openhuman/tools/ops.rs
Contributor
Author
Test-wiring investigation resultsThis comment reports the findings from investigating the two alleged test-wiring defects on Defect 1:
|
12 tasks
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
May 19, 2026
…act + single-line preference value (tinyhumansai#2150) - turn.rs: namespace filter now uses map_or(false, …) so entries with a missing namespace are excluded; only entries where namespace == "user_profile" pass through, preventing non-user_profile rows leaking into the profile context. - remember_preference.rs: tightened key validator to accept only [a-z0-9_] (lowercase snake_case); uppercase letters and hyphens are now rejected with a clear error message consistent with the existing validation style. - remember_preference.rs: both debug log statements that previously emitted the raw preference value (PII risk) now log only value_present/value_len; the key label is retained as it is non-sensitive. - remember_preference.rs: value is normalized to a single line before being passed to pinned_content(); embedded CR/LF are replaced with a space and the result is trimmed, preventing newline-injection into the line-oriented [pinned] key: value storage format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
graycyrus
reviewed
May 19, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Walkthrough
Clean, well-structured feature addition. Adds a deterministic remember_preference tool for pinning explicit user preferences, plus an always-on injection path via explicit_preferences_enabled (default true). The three-path fetch_learned_context dispatch is a good design — explicit preferences surface even when the probabilistic learning subsystem is off. 19 tests cover validation, happy path, idempotency, security gate, and all three fetch paths. CodeRabbit's 4 actionable findings were all addressed in the fix commit.
Change Summary
| File | Change type | Description |
|---|---|---|
tools/impl/agent/remember_preference.rs |
New | RememberPreferenceTool — parses (class, key, value), validates, upserts to user_profile namespace as pinned facet. 16 tests. |
agent/harness/session/turn.rs |
Modified | fetch_learned_context → 3-path dispatch: both-off (empty), explicit-only (pinned prefs), full learning |
agent/harness/session/turn_tests.rs |
Modified | 3 integration tests for the 3 fetch paths, using real UnifiedMemory |
agent/harness/session/builder.rs |
Modified | Wires explicit_preferences_enabled through builder + registers UserProfileSection when explicit-only |
agent/harness/session/types.rs |
Modified | Adds explicit_preferences_enabled field to Agent and AgentBuilder |
config/schema/learning.rs |
Modified | New explicit_preferences_enabled config field with serde default + doc |
config/schema/load.rs |
Modified | Env var override OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED |
tools/impl/agent/mod.rs |
Modified | Module declaration + re-export |
tools/ops.rs |
Modified | Registers RememberPreferenceTool in all_tools_with_runtime |
Per-file analysis
remember_preference.rs — Well-written. FacetClass enum with parse/round-trip, clean validation flow, proper security gate, good error messages. The PII redaction and single-line normalization fixes from the second commit are solid.
turn.rs — Three-path dispatch is clear and well-commented. Two minor observations below.
builder.rs — Correctly gates UserProfileSection registration on explicit_preferences_enabled && !learning.enabled to avoid double-registration when full learning is active.
load.rs — Env var parsing works but deviates from the file's pattern (see CodeRabbit nitpick #5 — parse_env_bool used everywhere else).
Overall: solid work. Only minor nits below — no blockers.
…t prompt injection Add `remember_preference` agent tool so the LLM can persist explicit user preferences (style, identity, tooling, veto, goal, channel) to the `user_profile` memory namespace as keyed pinned entries, independent of the full inference-based learning subsystem. Add `learning.explicit_preferences_enabled` config flag (default `true`, env override `OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLED`) that injects a `UserProfileSection` into the system prompt and fetches pinned preferences on every turn even when `learning.enabled=false`. The `fetch_learned_context` narrow path uses `memory.get()` semantics (via the `list` + key-prefix filter) for deduplication checks, and `memory.get()` directly for content assertions in tests, avoiding the `list()` → `title` column mismatch in the unified memory backend. 16 unit tests cover arg validation, class parsing, upsert idempotency, and all six facet classes. Three turn tests cover the three flag combinations (both off, explicit-only, both on).
…act + single-line preference value (tinyhumansai#2150) - turn.rs: namespace filter now uses map_or(false, …) so entries with a missing namespace are excluded; only entries where namespace == "user_profile" pass through, preventing non-user_profile rows leaking into the profile context. - remember_preference.rs: tightened key validator to accept only [a-z0-9_] (lowercase snake_case); uppercase letters and hyphens are now rejected with a clear error message consistent with the existing validation style. - remember_preference.rs: both debug log statements that previously emitted the raw preference value (PII risk) now log only value_present/value_len; the key label is retained as it is non-sensitive. - remember_preference.rs: value is normalized to a single line before being passed to pinned_content(); embedded CR/LF are replaced with a space and the result is trimmed, preventing newline-injection into the line-oriented [pinned] key: value storage format. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d-pref truncation (tinyhumansai#2150) - turn.rs: Rewrote the misleading comment on the `.filter()` block. The previous comment claimed "list without namespace filter would return everything", but `.list(Some("user_profile"), ...)` already scopes at the store layer. The new comment makes clear that `.list()` provides the real scoping and the `.filter()` is a belt-and-suspenders defensive guard against any future store-layer change. - turn.rs: Added `tracing::warn!` (with structured `total` / `dropped` fields) when `profile_entries.len() > 50` so silent truncation by `.take(50)` is visible in debug logs. The warn fires before the iterator chain and logs only the count — no preference values or PII are emitted, consistent with the redaction fix from the prior commit. Addresses graycyrus review comments #3265703537 and #3265703543. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sanil-23
force-pushed
the
feat/explicit-user-preferences
branch
from
a4f8d93 to
3769594
Compare
graycyrus
approved these changes
May 19, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Looks good, nice work!
Merged
7 tasks
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
…t prompt injection (tinyhumansai#2150) Co-authored-by: Gray Cyrus <cyrus@tinyhumans.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
remember_preferenceagent tool so the model can persist user-stated preferences (e.g. "always be terse", "I use pnpm") as deterministic, pinned profile facets.learning.explicit_preferences_enabledflag (defaulttrue), independent of the inference-basedlearning.enabledmaster switch (defaultfalse).fetch_learned_context(both-off / explicit-only / full-learning) so explicit preferences surface even when the probabilistic learning subsystem is disabled.Problem
CueFamily::Explicit(a user directly stating a preference) as the highest-trust signal, but there was no deterministic path to capture it — explicit preferences were only picked up probabilistically by the LLM summariser / heuristic detectors, all gated behindlearning.enabled(default off).Solution
RememberPreferenceToolmaps a user-stated preference to a(FacetClass, key, value)and upserts it as a pinned facet through the existing facet store — deterministic, idempotent (re-setting the same(class,key)overwrites; no duplicates), with no dependence on the inference engine.learning.explicit_preferences_enabled(defaulttrue) +OPENHUMAN_LEARNING_EXPLICIT_PREFERENCES_ENABLEDenv override gates an always-onUserProfileSectioninjection path, separate fromlearning.enabled.Submission Checklist
remember_preferencetests (arg/class validation, idempotent overwrite / no-duplicate edge) + 3fetch_learned_contextpath tests.diff-cover/coverage tooling not runnable in this ephemeral worktree (nonode_modules); new code is unit-tested and CIcoverage.ymlis the authoritative gate.remember_preferencetool row).## Related—N/A: no existing matrix rows touched; none to list.Closes #NNN— no tracked issue; feature originated from a design working session.Impact
Related
remember_preference; broader STM / memory-pipeline work (separate WIP).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/explicit-user-preferences858f5bc6fd3ddef5604d183766019be476679ed3Validation Run
pnpm --filter openhuman-app format:check— no frontend files changed.pnpm typecheck— no frontend/TS changed.cargo test --lib -- remember_preference fetch_learned_context: 19 passed, 0 failed (7624 filtered out) — independent re-run on PR head858f5bc6. (The literal two-positional formcargo test remember_preference fetch_learned_contextis invalid cargo CLI syntax; both filters must be passed after--.)cargo check --manifest-path Cargo.toml→ exit 0 (compiles on currentmainafter rebase).src-tauriunchanged.Validation Blocked
command:git push(pre-push hook runspnpm --filter openhuman-app format)error:prettier: command not found— ephemeral worktree has nonode_modulesimpact:Frontend format hook cannot run in this worktree; change is Rust-only (0 frontend files). Pushed with--no-verifyper repo policy for unrelated hook failures. CI runs full validation.Behavior Changes
learning.enabled.remember_preferenceis invoked (no pre-existing pinned facets).Parity Contract
learning.enabledsemantics unchanged; new path is additive and inert-by-default until the tool is used.fetch_learned_contextpreserves prior behaviour for both-flags-off and full-learning-on.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Chores
Tests