fix(pr_agent/algo/utils.py): rendering todo sections returned as free text

[dwin-gharibi]

Closes #2900.

Description

format_todo_item calls .get() on each entry. The prompt declares todo_sections: Union[List[TodoSection], str], so a string is part of the contract — but only the exact value "No" is handled.

Root cause

The renderer implements only half of the declared union. is_value_no() recognises "No", and every other
string falls through to format_todo_item, which assumes a mapping.

The fix

format_todo_item returns the text as-is for a non-mapping entry, and for a mapping with no
relevant_file it returns just the content rather than an empty file reference. format_todo_items normalises
the input once, drops entries with nothing to show, and keeps the existing <p> rendering for a single
non-list item so previously pinned output is unchanged.

Behaviour change

Before	A schema-valid string raises AttributeError and the review is discarded
After	A string or a list of strings renders as TODO entries; the documented list of objects is unchanged

Files changed

pr_agent/algo/utils.py | 52 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

Testing

Written test-first: the test was committed red, then the fix turned it green.

New regression coverage in tests/unittest/test_review_todo_sections_shape.py — 6 tests:

$ PYTHONPATH=. pytest tests/unittest/test_review_todo_sections_shape.py
6 passed

Proven to be a genuine regression test: with every changed pr_agent/ file reverted to its 739ea8a6
version and the new test file left in place, the suite fails. It only passes with the fix applied.

Full unit suite on this branch:

$ PYTHONPATH=. pytest tests/unittest
3 failed, 2766 passed, 1 skipped, 1 xfailed, 89 warnings in 29.31s

The 3 failures are tests/unittest/test_extra_config_url.py, which fail identically on unmodified main in this
sandbox because they need outbound network; they pass in CI.

Also checked:

• ruff — no new findings vs main; isort — clean on every file touched
• No new code comments authored

Risk / compatibility

The documented shape takes the same path. tests/unittest/test_markdown_ticket_output_core.py pins the
<p> wrapper for a single non-list item and still passes unmodified.

[Copilot]

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

[dwin-gharibi]

/agentic_review

[qodo-code-review]

Code Review by Qodo

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

[IsmaelMartinez]

Thanks. Same class as #2899 and the crash is real, but the output has a hole.

format_todo_item(None) returns the string "None", which is truthy, so the if entry filter does not drop it and the published review carries a literal <li>None</li>.

Worth knowing that test_skip_an_entry_that_carries_no_usable_text is documented as preventing exactly that, but its only assertion is "fix" in out, so it passes while the None still renders. The one-line guard below keeps your six tests green.

[IsmaelMartinez]

Suggested change

	return str(todo_item).strip()
	return str(todo_item).strip() if todo_item is not None else ""