Add system identity and default skills to RAG context

[themartto]

Summary by CodeRabbit

• New Features

  • Dedicated agent system identity file and openheim init now generates a system identity template
  • New default_skills setting to auto-load skills each session

• Behavior Changes

  • System identity is required and always loaded first; default and session skills are merged with order preserved and duplicates removed

• Documentation

  • Quickstart, guides, and CLI examples updated to the new identity+skills flow and openheim subcommands (run, serve, acp, init)

[themartto]

@coderabbitai review

[coderabbitai]

Actionable comments posted: 6

🤖 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 `@README.md`:
- Line 50: Update the README entry titled "System identity" to clarify that the
system.md is required when preparing a session rather than strictly at startup:
change the phrase "Required at startup (created by `openheim init`)." to
something like "Required when preparing a session (created by `openheim init`)."
Also adjust the identical phrasing at the other occurrence around line 182 to
match, and mention `prepare()`/session setup to align the docs with
implementation.
- Around line 83-89: Replace the user-facing example that runs "cargo run --
init" with the installed-binary invocation "openheim init" in the Quickstart
section of README (update the block containing "cargo run -- init" and the
surrounding explanatory text); leave an optional note for developers that "cargo
run -- init" can be used from a source checkout if you want to keep both
examples, but ensure the primary step for most users after "cargo install
openheim" is "openheim init".

In `@src/client.rs`:
- Line 381: The code currently hardcodes default_skills: vec![] in
build_programmatic, preventing callers using OpenheimClient::builder() from
configuring default skills; add a default_skills field to OpenheimBuilder and a
setter method (e.g., OpenheimBuilder::default_skills(&mut self, skills:
Vec<SkillType>)) and thread that field into build_programmatic so it uses the
builder-provided default_skills instead of vec![], updating the build path in
build_programmatic to read the builder.default_skills value when constructing
the programmatic client.

In `@src/config/mod.rs`:
- Around line 47-52: The error message returned in the config_path.exists()
branch incorrectly claims system.md "has been created" even when no write
occurred; update the logic in src/config/mod.rs (the branch that returns
Err(Error::config(...)) and references config_path.display() and
system_path.display()) to either track whether the earlier write actually
happened (e.g., set a boolean when the write near the earlier write call
succeeds) and use that to choose "has been created" vs "is available at", or
simply change the phrasing to "system.md is available at {}" so it never falsely
claims creation; make the change to the Error::config(...) message only.

In `@src/rag/mod.rs`:
- Around line 88-95: The helper merge_skills currently seeds merged from
defaults.to_vec(), so duplicates already present in defaults are preserved;
change merge_skills to build merged from an empty Vec<String> and iterate over
defaults then session, pushing each entry only if merged.contains(...) is false
(i.e., dedupe both inputs via the same path), ensuring duplicates in
default_skills are removed as well as duplicates across session.

In `@src/rag/prompt.rs`:
- Around line 55-59: The current extraction of system_identity trims the value
(via .map(str::trim)) which removes leading/trailing whitespace from the actual
prompt; change the logic so you only use a trimmed view to test blankness but
keep the original text if present. Concretely, obtain the original &str from
self.system_identity (e.g. let orig = self.system_identity.as_deref()), then set
identity = orig.filter(|s| !s.trim().is_empty()) so the prompt uses the original
string but emptiness is determined on s.trim().

🪄 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 Plus

Run ID: 54c1fbfa-8320-4c06-8d4f-410054fe5666

📥 Commits

Reviewing files that changed from the base of the PR and between 326a7a2 and e1877e8.

📒 Files selected for processing (18)

• README.md
• docs/architecture.md
• docs/configuration.md
• docs/skills.md
• src/client.rs
• src/config/client.rs
• src/config/config.toml.default
• src/config/mod.rs
• src/config/resolve.rs
• src/config/types.rs
• src/mcp/client.rs
• src/rag/mod.rs
• src/rag/prompt.rs
• src/rag/system.rs
• src/transport/run.rs
• src/transport/stdio.rs
• src/transport/ws.rs
• src/tui/app.rs

[themartto]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 454d2568-9dd6-48fb-ad5c-b0ba3394f684

📥 Commits

Reviewing files that changed from the base of the PR and between 326a7a2 and 56a83f5.

📒 Files selected for processing (18)

• README.md
• docs/architecture.md
• docs/configuration.md
• docs/skills.md
• src/client.rs
• src/config/client.rs
• src/config/config.toml.default
• src/config/mod.rs
• src/config/resolve.rs
• src/config/types.rs
• src/mcp/client.rs
• src/rag/mod.rs
• src/rag/prompt.rs
• src/rag/system.rs
• src/transport/run.rs
• src/transport/stdio.rs
• src/transport/ws.rs
• src/tui/app.rs

---

📝 Walkthrough

Walkthrough

Separates agent system identity into required ~/.openheim/system.md, adds AppConfig.default_skills, implements SystemLoader, refactors PromptBuilder to assemble identity then skills, updates RagContext to merge defaults with session skills and inject system.md first, wires defaults through builders/transports, and updates docs and tests.

Changes

System Identity and Default Skills Feature

Layer / File(s)	Summary
Configuration schema & initialization src/config/types.rs, src/config/mod.rs, src/config/config.toml.default, src/config/client.rs, src/config/resolve.rs	Adds AppConfig.default_skills: Vec<String> with #[serde(default)]; init_config creates ~/.openheim/system.md from DEFAULT_SYSTEM_MD; default config example documents default_skills; test helpers updated.
SystemLoader implementation src/rag/system.rs, src/rag/mod.rs	New SystemLoader resolves ~/.openheim/system.md; load() reads it or returns a config error instructing openheim init.
PromptBuilder refactor src/rag/prompt.rs	Stores optional system_identity and named (name, content) skills; adds set_system(); build() prepends a structured Role::System message (fixed preamble, identity block, then skill sections with ### <name> and --- separators) only when identity or skills exist; tests updated.
RagContext orchestration & skill merging src/rag/mod.rs	Exports system module; RagContext::new(default_skills) stores defaults and SystemLoader; prepare() merges defaults+call skills via merge_skills(), passes merged list into history.resolve_conversation(), always loads system.md and injects it first via builder.set_system(), then loads skill contents from conversation metadata; unit tests added for merge behavior.
OpenheimBuilder default_skills wiring src/client.rs	Adds OpenheimBuilder.default_skills field and setter, forwards defaults into build_programmatic and populates AppConfig for file-based paths so RagContext receives configured defaults.
Transport & runtime wiring to defaults src/transport/run.rs, src/transport/stdio.rs, src/transport/ws.rs, src/tui/app.rs	All runtime entry points construct RagContext with app_config.default_skills (or explicit vec![] in TUI); run_headless now aborts spawned ACP server handle and returns Ok(()).
MCP child stderr handling src/mcp/client.rs	Stdio-based MCP connection uses TokioChildProcess::builder(cmd) and routes child stderr to Stdio::null() before spawning.
Documentation: identity, skills, and CLI README.md, docs/architecture.md, docs/configuration.md, docs/skills.md	README and docs updated to describe system identity vs. skills, required ~/.openheim/system.md, default_skills config semantics and merge order, updated openheim init/CLI examples, and updated module/project docs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

• weirdstuff-dev/openheim#19: Related change touching RAG prompt construction and per-session skills injection.

Poem

🐇 I put my name first, a steady system tone,
Then skills like carrots line up, each in its own zone,
Defaults hop ahead, duplicates fall away,
The prompt now wears a hat and is ready for play,
Hooray — the rabbit trots, the session greets the day!

\n\n

\n

🚥 Pre-merge checks | ✅ 4 | ❌ 1

\n\n### ❌ Failed checks (1 inconclusive)\n\n| Check name | Status | Explanation | Resolution |\n| :---------: | :------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |\n| Title check | ❓ Inconclusive | The title \"Better rag\" is vague and generic, using non-descriptive language that does not convey meaningful information about the changeset's primary focus. | Use a more descriptive title that clearly indicates the main change, such as \"Add system identity and default skills to RAG context\" or \"Implement system.md loading and default skills management.\" |\n\n

\n

✅ Passed checks (4 passed)

\n\n| Check name | Status | Explanation |\n| :------------------------: | :------- | :----------------------------------------------------------------------------------- |\n| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |\n| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |\n| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |\n| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |\n\n

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

\n\n

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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