Skip to content

fix: strip <answer> XML tags before do/finish markers in _parse_response#414

Open
nankingjing wants to merge 2 commits into
zai-org:mainfrom
nankingjing:fix-parse-response-xml-order
Open

fix: strip <answer> XML tags before do/finish markers in _parse_response#414
nankingjing wants to merge 2 commits into
zai-org:mainfrom
nankingjing:fix-parse-response-xml-order

Conversation

@nankingjing

Copy link
Copy Markdown

Summary

Fixes an ordering bug in ModelClient._parse_response (phone_agent/model/client.py) that corrupts every action when the model answers in the tagged format requested by the system prompt.

The bug

The system prompt (phone_agent/config/prompts_zh.py / prompts_en.py) instructs the model to output:

<think>{think}</think>
<answer>{action}</answer>

where {action} is a do(...) or finish(...) call. _parse_response checked its rules in this order:

  1. finish(message= marker
  2. do(action= marker
  3. <answer> XML tags (documented as the fallback)

Because a tagged answer always contains a do(action= or finish(message= call inside the <answer> block, Rule 1/2 match before the tags are stripped. For input:

<think>I should tap the search box</think><answer>do(action="Tap", element=[500, 300])</answer>

the old code returned:

  • thinking = '<think>I should tap the search box</think><answer>' (tags leaked in)
  • action = 'do(action="Tap", element=[500, 300])</answer>' (trailing </answer>)

The action string is not valid Python, so the downstream parse_action (phone_agent/actions/handler.py) raises ValueError. In PhoneAgent._execute_step that exception is swallowed and turned into finish(message=<raw string>), so the agent ends the task immediately instead of tapping. As a consequence, Rule 3 (the XML-tag path) was effectively dead code — it could never be reached for a real action.

The fix

Check for the <answer> tag first. When present, strip <think>/</think>/<answer>/</answer> and return the clean thinking/action pair. Tag-less responses (the AutoGLM-Phone-9B happy path) are unchanged: they fall through to the unchanged finish(message=/do(action= marker rules.

This is a pure reordering — no tag-less behaviour changes.

Tests

Adds tests/test_parse_response_order.py:

  • Tagged do/finish responses now yield a clean action with no </answer> remnant (regression cases).
  • Tag-less do, finish, finish-precedes-do, and plain-text fallback still behave exactly as before.

Verification

Not executed in CI here. Verified by reading the code path end-to-end and by simulating the reordered _parse_response against all test inputs (all produce the asserted (thinking, action) tuples). Both edited files pass ast.parse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant