Skip to content

fix: surface malformed LLM plan output as LLMError - #59

Open
vitali87 wants to merge 4 commits into
mainfrom
fix/llm-parse-errors
Open

fix: surface malformed LLM plan output as LLMError#59
vitali87 wants to merge 4 commits into
mainfrom
fix/llm-parse-errors

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Summary

_parse_groups raised KeyError (missing field), ValueError (bad assignment_type) or pydantic ValidationError (wrong type, e.g. hunk_indices: "0") on malformed tool output. Both consumers only catch LLMError:

  • _call_chunk_with_retry never retried such responses
  • _refine_plan_with_llm was meant to fall back to the pre-refinement groups but let the exception escape, aborting the split

OpenAI calls do not use strict schema mode, so these shapes are realistic. _parse_groups now wraps parsing and raises LLMError(LLM_PARSE_ERROR) with the underlying error type and message.

Test plan

  • Parametrised test for missing key / bad enum / wrong type → LLMError
  • Refinement test: malformed response falls back to the previous groups instead of raising
  • uv run ruff check / ruff format --check clean
  • uv run pytest -q — 448 passed

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 9 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 85fa4611-5c85-4e02-b56d-aa0b38ebb4e4

📥 Commits

Reviewing files that changed from the base of the PR and between 93cf0fb and 497eb62.

📒 Files selected for processing (2)
  • pr_split/planner/client.py
  • tests/test_client.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown

Greptile Summary

Malformed LLM group entries now become recoverable LLM errors. Chunked planning retries a malformed response, and plan refinement preserves the previous valid plan when a malformed response is returned. The focused planner-client regression suite passes. Safe to merge.

Confidence Score: 5/5

No blocking failure remains.

The exercised malformed-response paths recover as intended, and no accepted blocking findings remain.

T-Rex T-Rex Logs

What T-Rex did

  • Ran a mocked-LLM harness against the base revision and the PR revision to exercise missing required fields, invalid assignment enums, and invalid hunk-index types; on the PR revision, each malformed chunk response produced an LLMError, was retried once, and then returned the valid mocked group, while malformed refinement responses retained the prior valid plan; and a focused pytest run showed all 52 planner-client tests passed.
  • Created the Malformed LLM group end-to-end validation source artifact to help reviewers inspect the end-to-end validation harness.
  • Compared pre-PR and post-PR behavior using logs labeled Before PR malformed LLM group behavior and After PR malformed LLM group recovery and refinement fallback to verify the changes in error handling and fallback logic.
  • Located the relevant implementation in pr_split/planner/client.py, showing where the retry and refinement logic lives (lines 206–213) and how it is consumed by the retry block (179–203) and refinement fallback (353–387).

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (2): Last reviewed commit: "fix: surface malformed LLM plan output a..." | Re-trigger Greptile

_parse_groups raised KeyError, ValueError or pydantic ValidationError on
malformed tool output. The chunk retry loop and the refinement fallback
only catch LLMError, so a single bad response aborted the whole split
with a traceback instead of retrying or keeping the previous plan.
@vitali87
vitali87 force-pushed the fix/llm-parse-errors branch from 345e9b9 to 5281dd4 Compare August 30, 2026 19:21
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.

1 participant