[BUG]: Incorrect Indexing in handle_plural_values() Causes Inconsistent Whitespace Formatting #96
Description
🐛 Bug Report
About: Create a report to help us improve FireForm.
Title: [BUG]: Off-by-one error in handle_plural_values()
Labels: bug
⚡️ Describe the Bug
The handle_plural_values() method in src/llm.py contains an off-by-one indexing issue when removing whitespace from plural values.
The loop modifies values[i + 1] instead of values[i], which causes the first element in the list to be skipped during whitespace cleanup. This results in inconsistent formatting when plural values are returned by the LLM.
👣 Steps to Reproduce
-
Call
handle_plural_values()with input:"value1 ; value2 ; value3" -
Inspect the returned list.
-
Observe that the first element may retain trailing whitespace.
📉 Expected Behavior
All elements obtained after splitting the string by ";" should have both leading and trailing whitespace removed consistently.
Example expected output:
["value1", "value2", "value3"]
🖥️ Environment Information
| Field | Details |
|---|---|
| OS | Any |
| Docker/Compose Version | N/A |
| Ollama Model Used | Mistral |
📸 Screenshots / Logs
Problematic implementation:
values = plural_value.split(";")
for i in range(len(values)):
current = i + 1
if current < len(values):
clean_value = values[current].lstrip()
values[current] = clean_value
Issues:
- Skips index
0 - Requires boundary checking due to
i + 1 - Uses
lstrip()instead ofstrip() - Introduces unnecessary indexing complexity
🕵️ Possible Fix
Replace the loop with a safer and cleaner implementation:
values = [v.strip() for v in plural_value.split(";")]
This fix:
- Cleans all elements
- Removes both leading and trailing whitespace
- Eliminates off-by-one indexing
- Simplifies the logic