Skip to content

refactor(agent): remove skip_soul context loading path - #23062

Open
GritLs wants to merge 1 commit into
NousResearch:mainfrom
GritLs:refactor/remove-skip-soul
Open

refactor(agent): remove skip_soul context loading path#23062
GritLs wants to merge 1 commit into
NousResearch:mainfrom
GritLs:refactor/remove-skip-soul

Conversation

@GritLs

@GritLs GritLs commented May 10, 2026

Copy link
Copy Markdown

Summary

  • Remove the skip_soul parameter from build_context_files_prompt().
  • Keep SOUL.md loading exclusively in load_soul_md() as the identity layer.
  • Update tests and docs so project context assembly no longer suggests a second SOUL.md injection path.

Why

skip_soul was added when SOUL.md moved from the project context block to the primary identity slot in #1922. The main prompt assembly now has load_soul_identity for the real cron use case from #17509, so the helper-level fallback path preserves old mixed context behavior and makes the prompt order harder to understand.

How to test

  • .venv/bin/python -m pytest -o addopts='' tests/agent/test_prompt_builder.py::TestBuildContextFilesPrompt tests/run_agent/test_run_agent.py::TestBuildSystemPrompt tests/cron/test_cron_workdir.py::TestRunJobTerminalCwd -q
  • .venv/bin/python - <<'PY' ... PY smoke test covering load_soul_md() plus build_context_files_prompt()
  • git diff --check

Platforms tested

  • macOS 26.4, CPython 3.14.3 via uv-created .venv

Related

Cross-platform impact

This is prompt assembly cleanup only. It removes duplicate SOUL.md loading from the context helper and does not add platform-specific behavior.

@GritLs
GritLs marked this pull request as ready for review May 10, 2026 07:52
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 10, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused prompt-assembly cleanup. The duplicate-injection guard is still present on current main, but this branch predates the system-prompt extraction.

Problems

  • agent/system_prompt.py:451-453 is now the live call site and still passes skip_soul=_soul_loaded. The PR removes that parameter in agent/prompt_builder.py but changes the former run_agent.py call site instead. Current main moved the implementation in 2d2cd5e904abc11eb6c00e88e2b6d0c8b025a597; applying the signature change without updating agent/system_prompt.py would raise TypeError while building a prompt.
  • The changed English prompt-assembly docs would leave the Chinese rendered counterpart stale: website/i18n/zh-Hans/docusaurus-plugin-content-docs/current/developer-guide/prompt-assembly.md:135,155,170-172,225 still documents skip_soul.

Suggested changes

  • Port the caller and regression coverage to agent/system_prompt.py / tests/agent/test_system_prompt.py, then remove the obsolete helper fallback.
  • Update the matching translated documentation with the English docs.

Automated hermes-sweeper review.

Comment thread run_agent.py
@@ -5754,8 +5754,7 @@ def _build_system_prompt(self, system_message: str = None) -> str:
# dir, so os.getcwd() would pick up the repo's AGENTS.md and

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On current main this call was extracted to agent/system_prompt.py:451-453 by 2d2cd5e904abc11eb6c00e88e2b6d0c8b025a597. Please port this change there as well: that live caller still passes skip_soul=_soul_loaded, which would otherwise become an unexpected keyword after the helper signature is reduced.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants