Skip to content

fix: extract multi-digit state indices in extract_state_value_from_output#2081

Open
Osamaali313 wants to merge 1 commit into
FoundationAgents:mainfrom
Osamaali313:fix/extract-state-multi-digit
Open

fix: extract multi-digit state indices in extract_state_value_from_output#2081
Osamaali313 wants to merge 1 commit into
FoundationAgents:mainfrom
Osamaali313:fix/extract-state-multi-digit

Conversation

@Osamaali313

Copy link
Copy Markdown

Problem

extract_state_value_from_output is used by Role._think to read the next state index from an LLM's reply (role.py):

pattern = r"(?<!-)[0-9]"
matches = re.findall(pattern, content, re.DOTALL)
matches = list(set(matches))
state = matches[0] if len(matches) > 0 else "-1"

Two issues:

  1. The pattern matches a single digit, so a state index >= 10 is split into separate digits. For content == "10" the result is "1" or "0" — never "10".
  2. list(set(matches)) makes the choice order-dependent on set hashing, which varies with PYTHONHASHSEED across processes, so the returned digit is nondeterministic.

Because the caller only validates int(next_state) in range(-1, len(self.states)), a wrong-but-in-range digit passes silently, so any role with 10+ states can transition to the wrong state (or loop) without error.

Fix

Match the first optionally-negative, multi-digit integer and drop the set() dedup:

pattern = r"-?\d+"
matches = re.findall(pattern, content, re.DOTALL)
state = matches[0] if len(matches) > 0 else "-1"

Single-digit outputs and the "-1" no-op state are unchanged; multi-digit indices are now returned whole and deterministically.

Test

Adds test_extract_state_value_from_output covering "0", " 3\n", "-1", "10", and "The next state is 12.". Without the fix the "10" and "12" cases fail (return a single digit).

…tput

The regex matched a single digit and the matches were passed through set(), so a state index >= 10 (e.g. "10") was split into separate digits and an arbitrary one was returned, with the result order depending on set hashing (nondeterministic across runs). Role._think then read the wrong state and silently transitioned incorrectly, or looped, for any role with 10+ states.

Match the first (optionally negative, multi-digit) integer and drop the set() dedup so the whole number is returned deterministically. Single-digit and "-1" outputs are unchanged. Adds a regression test.
Copilot AI review requested due to automatic review settings June 24, 2026 20:47

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants