Skip to content

fix: keep the leading system prompt when sanitizing an unanswered history - #373

Open
BrennanTM wants to merge 2 commits into
lanl:mainfrom
BrennanTM:sanitize-leading-system
Open

BrennanTM wants to merge 2 commits into
lanl:mainfrom
BrennanTM:sanitize-leading-system

Conversation

@BrennanTM

Copy link
Copy Markdown
Contributor

Summary

Fixes #372. Since #339, _sanitize_history drops the leading system prompt from any history that has no reply yet, so the first request of every ChatAgent conversation started through format_query (the TUI path for its default agent) and any caller-supplied leading SystemMessage reach the model without it. The "unanswered trailing prompts" step computes the tail from the last AIMessage or ToolMessage; with no reply the tail is the whole list and only the last HumanMessage survives. Present in v0.17.1 and v0.17.2; absent in v0.17.0.

The change

A leading SystemMessage is held out of the tail computation and put back in front: head + body[:last_reply + 1] + kept prompt. Everything #339 intended is preserved: duplicate trailing prompts still collapse to the last one, now also on a first turn when a caller holding its own state retries, and mid-list system messages are still dropped. A history holding only a system prompt is returned as is instead of becoming an empty request. The hunk in base.py is six insertions and three deletions, with no signature change and no change to any caller.

Tests

tests/agents/test_sanitize_history.py, committed red first and flipped by the fix with no test edits. Four fail on main: the first ChatAgent request keeps its system prompt (it was human only), a caller-supplied leading SystemMessage survives the first call, a first-turn duplicate prompt collapses while the prompt stays, and a system-only history is left alone (it became empty). Three guards pass throughout: trailing duplicates after a reply still collapse, mid-list system messages are still dropped, and a history without a system prompt keeps only its last prompt. Running the test commit without the fix commit gives exactly 4 failed / 3 passed. Full suite locally with the dev dependencies installed: 1124 passed, 6 skipped, 1 xfailed (the pinned #295 case).

The sanitizer from lanl#339 removes unanswered trailing prompts by keeping
only the last human message after the last reply. With no reply yet the
whole history is the tail, so the first request of every ChatAgent
conversation, and any caller-supplied leading system message, reaches
the model without its system prompt, and a history holding only a
system prompt becomes an empty request. Red first: four cases fail on
main; three guards pin the cleanups lanl#339 intended.
…tory

_sanitize_history treats everything after the last reply as unanswered
trailing prompts and keeps only the last human message. A history with
no reply yet made the whole list the tail, so the leading system prompt
was dropped from the first request of every conversation (ChatAgent,
and any caller-supplied system message) since lanl#339. The leading system
message is now held out of the tail computation; duplicate trailing
prompts still collapse and mid-list system messages are still dropped.
@BrennanTM

Copy link
Copy Markdown
Contributor Author

CI note: the Windows job's single failure is tests/cli/tui/event_cards/test_plan.py::test_plan_card_renders_drafting_and_collapsed_steps (assert 'truncated' not in ... after the terminal resize), a TUI plan-card rendering test this PR does not touch. The same test failed with the same assertion on windows-latest in both CI runs of the docstring-only #368 (runs 38648069797 and 38791751260), while ubuntu and macos passed each time, and the latest run on main is green on all three. The rest of the Windows run here is 1124 passed, 5 skipped, 1 xfailed, including the seven new tests. I do not have permission to re-run the job. Happy to look at the flake separately if that would help.

This branch has not been deployed

No deployments
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.

Since #339 the chat agent's first request omits its system prompt

1 participant