What happened
PR #60 (#60) added tilde fence support to parse_update_instructions() by extending the fence detection condition from stripped.startswith("```") to stripped.startswith("```") or stripped.startswith("~~~"). The review agent (run 31943342219, https://github.com/redhat-community-ai-tools/code-to-docs/actions/runs/31943342219) identified that the implementation uses a simple boolean toggle (in_code_fence = not in_code_fence) that does not track which delimiter opened the fence. This means a block opened with backticks can be closed by tildes and vice versa, violating CommonMark spec section 4.5. The finding was classified as [low] edge-case and the PR was approved and merged.
What could go better
The code agent implemented the fix exactly as specified in issue #59, which proposed the boolean-based condition extension. Neither the issue author (retro agent on PR #58) nor the code agent recognized that adding a second delimiter type to a boolean toggle creates a cross-delimiter ambiguity. The review agent caught this, but the finding was not actionable before merge since it was correctly classified as low severity. This is a case where the retro agent's fix specification was incomplete — it proposed a syntactic pattern extension without considering the semantic implications for the toggle logic. Confidence: high that the bug exists and is technically real; moderate-to-low that it will manifest in practice (requires a user to open a fence with one delimiter type and close with another within a PR comment).
Proposed change
In src/comments.py, replace the boolean in_code_fence toggle with a delimiter-tracking variable. Instead of in_code_fence = not in_code_fence, store which character opened the fence (e.g., fence_char = None) and only close the fence when the same delimiter is encountered. Specifically: when a line starts with ```, set fence_char = '\'; when it starts with ~~~, set fence_char = '~'. To close, check stripped.startswith(fence_char * 3)and resetfence_char = None. Add a test case where a backtick-opened fence contains a ~~~line followed by afilename.ext: instruction` pattern — the instruction should remain classified as inside the fence (global), not extracted as a per-file instruction.
Validation criteria
A new test case test_cross_delimiter_fence_not_closed passes: given input with a backtick-opened fence containing ~~~ and a subsequent file.rst: instruction line, the instruction is treated as global (inside the fence) rather than extracted as per-file. Existing tests test_tilde_code_fence_preserves_file_pattern and test_tilde_code_fence_with_language_specifier continue to pass. uv run pytest tests/test_comment_parsing.py -x -q shows all green.
Generated by retro agent from #60
What happened
PR #60 (#60) added tilde fence support to
parse_update_instructions()by extending the fence detection condition fromstripped.startswith("```")tostripped.startswith("```") or stripped.startswith("~~~"). The review agent (run 31943342219, https://github.com/redhat-community-ai-tools/code-to-docs/actions/runs/31943342219) identified that the implementation uses a simple boolean toggle (in_code_fence = not in_code_fence) that does not track which delimiter opened the fence. This means a block opened with backticks can be closed by tildes and vice versa, violating CommonMark spec section 4.5. The finding was classified as [low] edge-case and the PR was approved and merged.What could go better
The code agent implemented the fix exactly as specified in issue #59, which proposed the boolean-based condition extension. Neither the issue author (retro agent on PR #58) nor the code agent recognized that adding a second delimiter type to a boolean toggle creates a cross-delimiter ambiguity. The review agent caught this, but the finding was not actionable before merge since it was correctly classified as low severity. This is a case where the retro agent's fix specification was incomplete — it proposed a syntactic pattern extension without considering the semantic implications for the toggle logic. Confidence: high that the bug exists and is technically real; moderate-to-low that it will manifest in practice (requires a user to open a fence with one delimiter type and close with another within a PR comment).
Proposed change
In
src/comments.py, replace the booleanin_code_fencetoggle with a delimiter-tracking variable. Instead ofin_code_fence = not in_code_fence, store which character opened the fence (e.g.,fence_char = None) and only close the fence when the same delimiter is encountered. Specifically: when a line starts with```, setfence_char = '\'; when it starts with~~~, setfence_char = '~'. To close, checkstripped.startswith(fence_char * 3)and resetfence_char = None. Add a test case where a backtick-opened fence contains a~~~line followed by afilename.ext: instruction` pattern — the instruction should remain classified as inside the fence (global), not extracted as a per-file instruction.Validation criteria
A new test case
test_cross_delimiter_fence_not_closedpasses: given input with a backtick-opened fence containing~~~and a subsequentfile.rst: instructionline, the instruction is treated as global (inside the fence) rather than extracted as per-file. Existing teststest_tilde_code_fence_preserves_file_patternandtest_tilde_code_fence_with_language_specifiercontinue to pass.uv run pytest tests/test_comment_parsing.py -x -qshows all green.Generated by retro agent from #60