Skip to content

fix(agent): inject OPENAI_MODEL_EXECUTION_GUIDANCE for GLM/Qwen/DeepSeek - #53847

Open
tauros1983 wants to merge 1 commit into
NousResearch:mainfrom
tauros1983:fix/openai-exec-guidance-for-glm
Open

fix(agent): inject OPENAI_MODEL_EXECUTION_GUIDANCE for GLM/Qwen/DeepSeek#53847
tauros1983 wants to merge 1 commit into
NousResearch:mainfrom
tauros1983:fix/openai-exec-guidance-for-glm

Conversation

@tauros1983

Copy link
Copy Markdown

Telegram sessions on z-ai/glm-5.2 stalled after one assistant turn: the model replied with a plain-text '[TOOL_CALL]...[/TOOL_CALL]' marker instead of a structured tool_calls JSON block, so the runtime saw tool_calls=None and finished with finish_reason=stop. Two sessions on 2026-06-27 (20260627_144930_7531d0bd, 20260627_145132_ 3c5abc52) reproduced this — 3 messages, 0 tool_calls each, then silence.

Root cause: build_system_prompt_parts only appended OPENAI_MODEL_EXECUTION_GUIDANCE when the model name contained 'gpt', 'codex', or 'grok'. GLM/Qwen/DeepSeek matched TOOL_USE_ENFORCEMENT_MODELS (so they got the lighter TOOL_USE_ENFORCEMENT_GUIDANCE block) but not the execution-discipline block (tool persistence, mandatory_tool_use, anti-fabrication). On a weaker tool_use format-trained model that lighter block alone is not enough to stop the model from emitting its tool intent as natural- language markdown.

The OPENAI_ prefix reflects origin, not exclusivity (see the comment on prompt_builder.OPENAI_MODEL_EXECUTION_GUIDANCE). Expand the gate to the non-Google slice of TOOL_USE_ENFORCEMENT_MODELS — Google keeps its own more specific GOOGLE_MODEL_OPERATIONAL_GUIDANCE block, so we skip gemini/gemma here to avoid a duplicate parallel-tool-call steer.

Tests: 8 regression tests in TestOpenAIExecutionGuidanceInjection cover GLM/DeepSeek/Qwen (positive), GPT/Grok (no regression), Opus/Gemini (no false-positive), and the off-switch. 169/169 pass in test_prompt_builder.py + test_system_prompt.py; full suite minus the unrelated pre-existing test_anthropic_adapter.py MagicMock failure also clean.

Reproduction evidence (from ~/.hermes/state.db on 2026-06-27):

SELECT message_count, tool_call_count, end_reason FROM sessions
WHERE id IN ('20260627_144930_7531d0bd','20260627_145132_3c5abc52');
-> 3|0|| (both)

SELECT substr(content, 1, 200) FROM messages
WHERE session_id='20260627_144930_7531d0bd' AND role='assistant';
-> 'Ik ga die 3 commits lezen. Eerst kijken waar ik sta...
[TOOL_CALL]\nrun_command\ncommand=cd /Users/guidolassally/esmi-dashboard
2>/dev/null && git log --oneline -1 cc3cd6b92 && ...'

finish_reason was 'stop' and tool_calls was NULL on both sessions.

Live verification (after patch):

python -c 'from unittest.mock import patch; from types import
SimpleNamespace; ... build_system_prompt_parts(agent)["stable"]'
for z-ai/glm-5.2 now contains 'Execution discipline',
'<tool_persistence>', and '<mandatory_tool_use>' sections that
previously were absent.

What does this PR do?

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

How to Test

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

Telegram sessions on z-ai/glm-5.2 stalled after one assistant turn:
the model replied with a plain-text '[TOOL_CALL]...[/TOOL_CALL]'
marker instead of a structured tool_calls JSON block, so the runtime
saw tool_calls=None and finished with finish_reason=stop. Two
sessions on 2026-06-27 (20260627_144930_7531d0bd, 20260627_145132_
3c5abc52) reproduced this — 3 messages, 0 tool_calls each, then
silence.

Root cause: build_system_prompt_parts only appended
OPENAI_MODEL_EXECUTION_GUIDANCE when the model name contained
'gpt', 'codex', or 'grok'. GLM/Qwen/DeepSeek matched
TOOL_USE_ENFORCEMENT_MODELS (so they got the lighter
TOOL_USE_ENFORCEMENT_GUIDANCE block) but not the execution-discipline
block (tool persistence, mandatory_tool_use, anti-fabrication). On a
weaker tool_use format-trained model that lighter block alone is not
enough to stop the model from emitting its tool intent as natural-
language markdown.

The OPENAI_ prefix reflects origin, not exclusivity (see the comment
on prompt_builder.OPENAI_MODEL_EXECUTION_GUIDANCE). Expand the gate
to the non-Google slice of TOOL_USE_ENFORCEMENT_MODELS — Google keeps
its own more specific GOOGLE_MODEL_OPERATIONAL_GUIDANCE block, so we
skip gemini/gemma here to avoid a duplicate parallel-tool-call steer.

Tests: 8 regression tests in TestOpenAIExecutionGuidanceInjection cover
GLM/DeepSeek/Qwen (positive), GPT/Grok (no regression), Opus/Gemini
(no false-positive), and the off-switch. 169/169 pass in
test_prompt_builder.py + test_system_prompt.py; full suite minus the
unrelated pre-existing test_anthropic_adapter.py MagicMock failure
also clean.

Reproduction evidence (from ~/.hermes/state.db on 2026-06-27):

  SELECT message_count, tool_call_count, end_reason FROM sessions
  WHERE id IN ('20260627_144930_7531d0bd','20260627_145132_3c5abc52');
  -> 3|0||  (both)

  SELECT substr(content, 1, 200) FROM messages
  WHERE session_id='20260627_144930_7531d0bd' AND role='assistant';
  -> 'Ik ga die 3 commits lezen. Eerst kijken waar ik sta...
      [TOOL_CALL]\nrun_command\ncommand=cd /Users/guidolassally/esmi-dashboard
      2>/dev/null && git log --oneline -1 cc3cd6b92 && ...'

  finish_reason was 'stop' and tool_calls was NULL on both sessions.

Live verification (after patch):

  python -c 'from unittest.mock import patch; from types import
  SimpleNamespace; ... build_system_prompt_parts(agent)["stable"]'
  for z-ai/glm-5.2 now contains 'Execution discipline',
  '<tool_persistence>', and '<mandatory_tool_use>' sections that
  previously were absent.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/zai ZAI provider provider/qwen Qwen / Alibaba Cloud (OAuth) provider/deepseek DeepSeek API P2 Medium — degraded but workaround exists labels Jun 27, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #35087 solves the same GLM/Qwen/DeepSeek tool-call stall via a different mechanism (new OPENAI_EXECUTION_DISCIPLINE_MODELS tuple + an agent.execution_discipline config knob). This PR instead reuses the existing TOOL_USE_ENFORCEMENT_MODELS set (minus gemini/gemma). Competing approaches for the same fix — flagging the cluster so a maintainer can pick. (#28195, which touched the enforcement tuple, is closed.)

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

OpenAI execution guidance injection for GLM/Qwen/DeepSeek (2 files). Important fix:

  • Expands the execution guidance injection to cover all TOOL_USE_ENFORCEMENT_MODELS families (not just gpt/codex/grok)
  • Uses TOOL_USE_ENFORCEMENT_MODELS list directly (maintainable, not hardcoded)
  • Google models still get the more specific GOOGLE_MODEL_OPERATIONAL_GUIDANCE (correct)
  • Comprehensive test coverage: 6 new tests covering GLM, DeepSeek, Qwen, and edge cases

Good fix for a real stall issue. The test suite covers the exact regression scenario.

@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 isolating a real prompt-tier mismatch. Current main still applies execution discipline only to gpt, codex, and grok at agent/system_prompt.py:289, while the automatic enforcement family tuple includes glm, qwen, and deepseek at agent/prompt_builder.py:302.

Problems

  • The new test helper forces tool_use_enforcement=True at tests/agent/test_system_prompt.py:120. That bypasses the default auto selection path, so the tests do not directly verify the TOOL_USE_ENFORCEMENT_MODELS relationship the implementation reuses.

Suggested changes

  • Add an auto-mode GLM regression test, plus at least one Qwen or DeepSeek case, asserting both enforcement and execution-discipline guidance are injected. Keep the explicit-off regression test.

This is an automated hermes-sweeper review.

block (tool persistence, anti-fabrication, mandatory_tool_use).
"""

def _prompt(self, model, *, valid_tool_names=("terminal", "read_file"),

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.

This default forces the outer enforcement branch, so the new cases do not exercise the production auto route through TOOL_USE_ENFORCEMENT_MODELS. Please add at least one GLM/Qwen/DeepSeek case using tool_use_enforcement="auto" and assert both guidance blocks.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the same one-turn GLM/Qwen/DeepSeek stall through complementary prompt-injection fixes: #53847 extends execution discipline to the affected model families, while #53857 makes baseline tool-use enforcement independent of whether tools are currently loaded.

Related pull requests

  • #53847 related — (+85/-4) — primary, needs changes: The diff directly fixes the model-family mismatch by deriving the non-Google execution-guidance families from TOOL_USE_ENFORCEMENT_MODELS, but its tests force enforcement on and therefore miss the automatic-selection path identified by the keep_open contributor review; add auto-mode GLM and Qwen or DeepSeek coverage before merge.
  • #53857 related — (+149/-27) — complementary but supersedable: The diff fixes the separate no-tools gate that suppresses baseline enforcement in gateway sessions, but it also injects tool-specific OPENAI_MODEL_EXECUTION_GUIDANCE into no-tool GPT/Codex/Grok sessions and leaves the existing integration-test contract unupdated. Despite the keep_open review on #53857, its useful change is the narrower movement of TOOL_USE_ENFORCEMENT_GUIDANCE outside the tools gate, which can be consolidated into #53847 while retaining operational-guidance gating and updating the integration test.

Suggested consolidation

Merge #53847 after adding auto-mode regression coverage and incorporating the narrow no-tools baseline-enforcement fix from #53857, with Google and OpenAI operational guidance remaining tool-gated and the existing no-tools integration test updated. Then close #53857 as superseded by the consolidated #53847 rather than as a duplicate, because the two current diffs fix distinct contributing gates.

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 16 kB of PR diffs, 12 kB of issue/PR text, 4 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 P2 Medium — degraded but workaround exists provider/deepseek DeepSeek API provider/qwen Qwen / Alibaba Cloud (OAuth) provider/zai ZAI provider sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants