Skip to content

fix: correctly detect closing code fence in review comment extraction#213

Merged
matt2e merged 2 commits intomainfrom
code-review-not-working
Feb 19, 2026
Merged

fix: correctly detect closing code fence in review comment extraction#213
matt2e merged 2 commits intomainfrom
code-review-not-working

Conversation

@matt2e
Copy link
Contributor

@matt2e matt2e commented Feb 19, 2026

Fix the extract_review_comments parser to correctly identify closing `````` fences for review-comments code blocks.

Previously, the parser matched any occurrence of as a closing fence, which caused false matches on embedded code fences likerust` within JSON string values. This broke review comment extraction when LLM responses contained nested code blocks.

Changes:

  • Introduce find_closing_fence() helper that requires the `````` to appear at the start of a line and be followed only by whitespace/EOF (not an info-string like rust or `sql`)
  • Replace the naive marker_end substring search with the new helper
  • Add comprehensive tests covering: simple closing fences, fences not at line start, fences with trailing info-strings, embedded fences in JSON, multiple review blocks, empty paths/content filtering, and note content/title extraction

The extract_review_comments parser was using a naive find("```") to
locate the closing fence of ```review-comments blocks. When the JSON
content contained embedded markdown code blocks (e.g. ```rust ...
```), the parser matched the first backtick-triple inside a JSON
string value, truncating the JSON and causing serde deserialization to
fail silently. This meant no review comments were ever extracted.

Replace the naive search with a find_closing_fence() helper that
requires the ``` to appear at column 0 (start of line) and be
followed only by EOF, a newline, or whitespace — distinguishing it
from opening fences with info-strings like ```rust and from
backticks embedded mid-line within JSON string values.

Add 15 unit tests covering find_closing_fence, extract_review_comments,
extract_note_content, and extract_note_title.
Replace the verbose match expression on remaining.find(fence) with the
? operator in find_closing_fence(). This resolves the clippy
question_mark lint error that was causing CI to fail with -D warnings.
@matt2e matt2e merged commit 1027189 into main Feb 19, 2026
3 checks passed
@matt2e matt2e deleted the code-review-not-working branch February 19, 2026 19:26
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

Comments