mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
fix(config): honor system_prompt.prefix from config.toml (#482)
A [system_prompt] prefix set in config.toml was silently ignored: (1) load_config()'s section allowlist dropped the [system_prompt], [memory_files], [compression], and [skills] blocks entirely; (2) SystemPromptConfig had no prefix field; (3) the builder never prepended one. Add the four blocks to the allowlist, add prefix: str = "" to SystemPromptConfig, and prepend a "prefix" PromptSection at the front of SystemPromptBuilder's frozen sections so it leads build() output and is exposed via sections() (#457). Empty prefix emits no section — existing configs are byte-for-byte unchanged. Rebuilt on top of #457 (which refactored the builder to PromptSection objects); the original #452 by @SoulSniper-V2 patched the pre-#457 method. Credit to @SoulSniper-V2. Adds the regression tests the original PR lacked: config parse + prefix-prepended + empty-prefix-unchanged. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
53fb42104e
commit
b3cfa398b4
@@ -1434,6 +1434,7 @@ class MemoryFilesConfig:
|
||||
class SystemPromptConfig:
|
||||
"""Limits and strategy for system-prompt assembly."""
|
||||
|
||||
prefix: str = ""
|
||||
soul_max_chars: int = 4000
|
||||
memory_max_chars: int = 2500
|
||||
user_max_chars: int = 1500
|
||||
@@ -1810,6 +1811,10 @@ def load_config(path: Optional[Path] = None) -> JarvisConfig:
|
||||
"agent_manager",
|
||||
"digest",
|
||||
"proactive",
|
||||
"memory_files",
|
||||
"system_prompt",
|
||||
"compression",
|
||||
"skills",
|
||||
)
|
||||
for section_name in top_sections:
|
||||
if section_name in data:
|
||||
|
||||
@@ -125,14 +125,26 @@ class SystemPromptBuilder:
|
||||
|
||||
def _build_frozen_sections(self) -> list[PromptSection]:
|
||||
sections: list[PromptSection] = []
|
||||
sections.append(
|
||||
PromptSection(
|
||||
name="agent_template",
|
||||
content=self._agent_template,
|
||||
source="agent_template",
|
||||
cache_segment="frozen_prefix",
|
||||
# Config-driven persona prefix from [system_prompt] prefix (#401),
|
||||
# prepended ahead of the agent template so it leads the frozen prefix.
|
||||
if self._sp_config.prefix:
|
||||
sections.append(
|
||||
PromptSection(
|
||||
name="prefix",
|
||||
content=self._sp_config.prefix,
|
||||
source="system_prompt.prefix",
|
||||
cache_segment="frozen_prefix",
|
||||
)
|
||||
)
|
||||
if self._agent_template:
|
||||
sections.append(
|
||||
PromptSection(
|
||||
name="agent_template",
|
||||
content=self._agent_template,
|
||||
source="agent_template",
|
||||
cache_segment="frozen_prefix",
|
||||
)
|
||||
)
|
||||
)
|
||||
sections.extend(self._persona_prompt_sections())
|
||||
# XML skill catalog (preferred over legacy markdown list)
|
||||
if self._skill_catalog_xml:
|
||||
|
||||
@@ -106,6 +106,25 @@ class TestTomlLoading:
|
||||
assert cfg.engine.lemonade.host == "http://custom-lemonade:19000"
|
||||
assert cfg.engine.lemonade_host == "http://custom-lemonade:19000"
|
||||
|
||||
def test_system_prompt_block_parsed(self, tmp_path: Path) -> None:
|
||||
"""Regression for #401: the [system_prompt] block (and its prefix)
|
||||
must reach the runtime config, not be dropped by load_config()."""
|
||||
toml_file = tmp_path / "config.toml"
|
||||
toml_file.write_text(
|
||||
'[system_prompt]\nprefix = "You are Jarvis."\nsoul_max_chars = 999\n'
|
||||
)
|
||||
cfg = load_config(toml_file)
|
||||
assert cfg.system_prompt.prefix == "You are Jarvis."
|
||||
assert cfg.system_prompt.soul_max_chars == 999
|
||||
|
||||
def test_config_without_system_prompt_block_defaults(self, tmp_path: Path) -> None:
|
||||
"""Backward compatibility: a config lacking [system_prompt] keeps
|
||||
the empty-prefix default (no behavior change for existing configs)."""
|
||||
toml_file = tmp_path / "config.toml"
|
||||
toml_file.write_text('[engine]\ndefault = "ollama"\n')
|
||||
cfg = load_config(toml_file)
|
||||
assert cfg.system_prompt.prefix == ""
|
||||
|
||||
|
||||
class TestGenerateToml:
|
||||
def test_contains_engine_section(self) -> None:
|
||||
|
||||
@@ -37,6 +37,53 @@ def test_build_frozen_prefix(memory_dir: Path):
|
||||
assert "Alice" in prompt
|
||||
|
||||
|
||||
def test_config_prefix_prepended(memory_dir: Path):
|
||||
"""Regression for #401: a configured system_prompt.prefix leads the
|
||||
assembled prompt, ahead of the agent template, and is exposed as a
|
||||
'prefix' section."""
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
builder = SystemPromptBuilder(
|
||||
agent_template="You are Jarvis.",
|
||||
memory_files_config=MemoryFilesConfig(
|
||||
soul_path=str(memory_dir / "SOUL.md"),
|
||||
memory_path=str(memory_dir / "MEMORY.md"),
|
||||
user_path=str(memory_dir / "USER.md"),
|
||||
),
|
||||
system_prompt_config=SystemPromptConfig(prefix="ALWAYS ANSWER AS JARVIS."),
|
||||
)
|
||||
prompt = builder.build()
|
||||
assert prompt.startswith("ALWAYS ANSWER AS JARVIS.")
|
||||
assert "You are Jarvis." in prompt
|
||||
# Prefix is visible in the inspection API too (#457), as a frozen section.
|
||||
section_names = [s.name for s in builder.sections()]
|
||||
assert section_names[0] == "prefix"
|
||||
assert builder.sections()[0].cache_segment == "frozen_prefix"
|
||||
|
||||
|
||||
def test_empty_prefix_leaves_prompt_unchanged(memory_dir: Path):
|
||||
"""Backward compatibility: the default empty prefix adds no section and
|
||||
leaves build() output identical to having no prefix configured."""
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
def _make(prefix: str) -> SystemPromptBuilder:
|
||||
return SystemPromptBuilder(
|
||||
agent_template="You are Jarvis.",
|
||||
memory_files_config=MemoryFilesConfig(
|
||||
soul_path=str(memory_dir / "SOUL.md"),
|
||||
memory_path=str(memory_dir / "MEMORY.md"),
|
||||
user_path=str(memory_dir / "USER.md"),
|
||||
),
|
||||
system_prompt_config=SystemPromptConfig(prefix=prefix),
|
||||
)
|
||||
|
||||
assert _make("").build() == _make("").build()
|
||||
# No 'prefix' section is emitted when prefix is empty.
|
||||
assert "prefix" not in [s.name for s in _make("").sections()]
|
||||
# And the first section is the agent template, as before.
|
||||
assert _make("").sections()[0].name == "agent_template"
|
||||
|
||||
|
||||
def test_frozen_prefix_stability(memory_dir: Path):
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
|
||||
Reference in New Issue
Block a user