docs: ROADMAP from issues + gh CLI workflow; parsing diff-prefix repair (#28)#43
Conversation
… repair (#28) - docs/ROADMAP.md: Backlog from open issues (high/medium/low), gh issue list/edit workflow, labels (priority: *, area: *). Shipped section notes #20 secrets done, #28 parsing fallbacks in progress, #23 verification in place. - parsing: In repair_json_candidates(), add candidate that strips leading '+' from each line when LLM echoes JSON with diff-style prefixes (issue #28). Test: parse_json_with_diff_prefix_artifact. - CLAUDE.md: Link to docs/ and ROADMAP.md. - README Contributing: Link to ROADMAP and gh issue list for triage. - GitHub: Added labels priority: high|medium|low, area: * to open issues via gh. Made-with: Cursor
PR SummaryMedium Risk Overview Adds Written by Cursor Bugbot for commit ae7bd57. This will update automatically on new commits. Configure here. |
| let without_diff_prefix: String = trimmed | ||
| .lines() | ||
| .map(|line| line.strip_prefix('+').map(str::trim).unwrap_or(line)) | ||
| .collect::<Vec<_>>() | ||
| .join("\n"); | ||
| let without_diff_prefix = without_diff_prefix.trim(); | ||
| if without_diff_prefix != trimmed | ||
| && (without_diff_prefix.starts_with('[') || without_diff_prefix.starts_with('{')) | ||
| { | ||
| candidates.push(without_diff_prefix.to_string()); | ||
| } |
There was a problem hiding this comment.
Bug: The logic in repair_json_candidates to strip + prefixes from JSON is unreachable because upstream functions filter out or bypass the +-prefixed content.
Severity: MEDIUM
Suggested Fix
The fix should be applied in extract_json_from_code_block. The condition that checks if the content starts with [ or { should be modified to also account for content where lines might have + prefixes before the JSON structure begins.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/parsing/llm_response.rs#L358-L369
Potential issue: The new code in `repair_json_candidates` intended to strip diff-style
`+` prefixes from JSON content is unreachable. The upstream function
`extract_json_from_code_block` filters out any content that does not start with `[` or
`{`, so `+`-prefixed strings are never passed down. Fallback functions like
`find_json_array` extract valid JSON from within the prefixed lines, bypassing the new
repair logic entirely. As a result, the feature does not work as intended, and the
associated test passes due to a fallback mechanism, not because the new code is
functioning correctly.
Did we get this right? 👍 / 👎 to inform future reviews.
| && (without_diff_prefix.starts_with('[') || without_diff_prefix.starts_with('{')) | ||
| { | ||
| candidates.push(without_diff_prefix.to_string()); | ||
| } |
There was a problem hiding this comment.
Diff-prefix repair unreachable for multi-line JSON input
Medium Severity
The diff-prefix stripping in repair_json_candidates is unreachable for its intended multi-line use case. extract_json_from_code_block rejects code block content starting with + (the starts_with('[') || starts_with('{') guard at line 288), and find_balanced_json can't return content containing embedded + characters because serde validation fails. The repair lives downstream of extraction, so content that needs repairing never reaches it. The test parse_json_with_diff_prefix_artifact passes only because find_json_array locates the valid […] substring directly, never exercising the new repair code.
Additional Locations (2)
| && (without_diff_prefix.starts_with('[') || without_diff_prefix.starts_with('{')) | ||
| { | ||
| candidates.push(without_diff_prefix.to_string()); | ||
| } |
There was a problem hiding this comment.
Repair candidates not composed, combined fixes missed
Low Severity
The diff-prefix stripping candidate is derived from trimmed (original input), and the trailing-comma candidate is also derived from trimmed. Neither repair is applied to the other's output, so if the extracted JSON needs both fixes (e.g., +-prefixed lines and trailing commas), no single candidate in the returned Vec will be valid JSON and parsing silently fails. The diff-prefix candidate also needs trailing-comma removal applied to it.


Summary
gh issue list --label "priority: high",gh issue edit N --add-label "..."). Shipped section notes Built-in secrets detection scanner #20 (secrets done), Robust LLM output parsing with fallback strategies #28 (parsing in progress), Verification pass to catch hallucinations #23 (verification in place).repair_json_candidates(), add a repair candidate that strips leading+from each line when the LLM echoes JSON with diff-style line prefixes. New test:parse_json_with_diff_prefix_artifact.gh issue listfor triage.priority: high|medium|lowandarea: review-pipeline|plugins|platform; applied to open issues viagh issue edit.Test plan
cargo test parsing(69 tests includingparse_json_with_diff_prefix_artifact).Made with Cursor