Skip to content

fix(agents): guard on chat.messages not chat.last to avoid IndexError in _run_tools#2609

Merged
kevinmessiaen merged 2 commits into
Giskard-AI:mainfrom
chuenchen309:fix/chat-last-indexerror-on-empty-messages
Jul 16, 2026
Merged

fix(agents): guard on chat.messages not chat.last to avoid IndexError in _run_tools#2609
kevinmessiaen merged 2 commits into
Giskard-AI:mainfrom
chuenchen309:fix/chat-last-indexerror-on-empty-messages

Conversation

@chuenchen309

Copy link
Copy Markdown
Contributor

What

ChatWorkflow(...).run() with no messages added crashes with IndexError instead of the intended no-op.

The "no tool calls" guard in _run_tools (libs/giskard-agents/src/giskard/agents/workflow.py) starts with:

if not chat.last or ...:
    return

But Chat.last is self.messages[-1], which raises IndexError: list index out of range on an empty messages list rather than returning something falsy. Since messages defaults to [], a workflow that runs before any .chat() call hits this — the IndexError propagates and run() (default RAISE error policy) wraps it in WorkflowError("Step processing failed") instead of the intended early return.

Fix

Guard on chat.messages (the list) instead of chat.last (which indexes into it):

if not chat.messages or ...:
    return

One-line change in workflow.py.

Testing

Added test_run_with_no_messages_does_not_raise_indexerror to libs/giskard-agents/tests/test_workflow_steps.py (mocked BaseGenerator, no real API calls). TDD red confirmed against the unfixed guard (raises WorkflowError wrapping the IndexError), green after the fix. libs/giskard-agents/tests/test_workflow_steps.py (non-functional): 4 passed. ruff check + ruff format --check clean.

xref: no open PR or issue touches workflow.py, _run_tools, Chat.last, or ChatWorkflow.


Disclosure: this PR was drafted with AI assistance (Claude); I reviewed, tested, and take responsibility for the change.

`_run_tools`'s no-op guard checked `not chat.last`, but `Chat.last` indexes
`messages[-1]`, which raises IndexError on an empty list instead of behaving
falsy. A ChatWorkflow with no messages added (e.g. `ChatWorkflow(...).run()`)
hit this IndexError inside the guard's own condition instead of the intended
clean no-op. Check `not chat.messages` directly instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request fixes a potential IndexError in the _run_tools method of ChatWorkflow by checking if chat.messages is empty instead of checking chat.last (which accesses messages[-1]). It also adds a corresponding unit test to verify this behavior and prevent regressions. There are no review comments to address, and I have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@kevinmessiaen kevinmessiaen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@kevinmessiaen
kevinmessiaen enabled auto-merge (squash) July 16, 2026 04:30
@kevinmessiaen
kevinmessiaen merged commit 175670e into Giskard-AI:main Jul 16, 2026
29 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants