From b3cfa398b4ff169372bcca922d8cb3beeeb72522 Mon Sep 17 00:00:00 2001 From: Jon Saad-Falcon <41205309+jonsaadfalcon@users.noreply.github.com> Date: Tue, 2 Jun 2026 20:14:42 -0700 Subject: [PATCH] fix(config): honor system_prompt.prefix from config.toml (#482) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/openjarvis/core/config.py | 5 ++++ src/openjarvis/prompt/builder.py | 26 +++++++++++++----- tests/core/test_config.py | 19 +++++++++++++ tests/prompt/test_builder.py | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) diff --git a/src/openjarvis/core/config.py b/src/openjarvis/core/config.py index 61c76fe3..07effe4c 100644 --- a/src/openjarvis/core/config.py +++ b/src/openjarvis/core/config.py @@ -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: diff --git a/src/openjarvis/prompt/builder.py b/src/openjarvis/prompt/builder.py index eaa4723a..e812a528 100644 --- a/src/openjarvis/prompt/builder.py +++ b/src/openjarvis/prompt/builder.py @@ -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: diff --git a/tests/core/test_config.py b/tests/core/test_config.py index 337f671f..2d4ba8ff 100644 --- a/tests/core/test_config.py +++ b/tests/core/test_config.py @@ -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: diff --git a/tests/prompt/test_builder.py b/tests/prompt/test_builder.py index 4170b987..7f8dd7d0 100644 --- a/tests/prompt/test_builder.py +++ b/tests/prompt/test_builder.py @@ -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