Skip to content

Add support for XML-style <cmt>comment</cmt> <comment>comment</comment> formats in refine-plan - #72

Closed
Lyken17 wants to merge 1 commit into
PolyArch:mainfrom
Lyken17:main
Closed

Add support for XML-style <cmt>comment</cmt> <comment>comment</comment> formats in refine-plan#72
Lyken17 wants to merge 1 commit into
PolyArch:mainfrom
Lyken17:main

Conversation

@Lyken17

@Lyken17 Lyken17 commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Extends the refine-plan command to support three comment annotation formats instead of just CMT:/ENDCMT, making plan reviews more intuitive and flexible
for different user preferences.

New Comment Formats

  • Classic format (existing): CMT: comment ENDCMT
  • Short tag format (new): <cmt>comment</cmt>
  • Long tag format (new): <comment>comment</comment>

Key Features

  • ✅ All formats support both inline and multi-line usage
  • ✅ Formats can be mixed within the same file
  • ✅ Proper nesting detection and error reporting
  • ✅ Code block and HTML comment exclusion works for all formats
  • ✅ 100% backward compatible with existing workflows

Examples

Inline annotations:

Text before CMT: classic comment ENDCMT text after.
Text before <cmt>short tag comment</cmt> text after.
Text before <comment>long tag comment</comment> text after.

Multi-line annotations:
CMT:
Multi-line classic comment
ENDCMT

<cmt>
Multi-line short tag comment
</cmt>

<comment>
Multi-line long tag comment
</comment>

Mixed formats in one file:
## Goal Description
Update the API CMT: why not use REST? ENDCMT to support GraphQL.

## Acceptance Criteria
- AC-1: <cmt>Should this be split into multiple ACs?</cmt>
- AC-2: <comment>Need to clarify the error handling requirements</comment>

Test plan

- Verify all three formats are correctly parsed by the validator
- Test inline and multi-line usage for each format
- Confirm mixed formats work in the same file
- Validate error detection for mismatched start/end markers
- Ensure code blocks and HTML comments properly exclude markers
- Run existing test suite to confirm backward compatibility

Implementation Notes

- Enhanced AWK parser with format-specific marker detection
- Added helper functions for marker length calculation and format validation
- Updated all documentation to reflect the new capabilities

@Lyken17 @claude
Add support for multiple comment formats in refine-plan
79714b7 
Extends comment parsing to support three formats:
- Classic: CMT:/ENDCMT (existing)
- Short tag: <cmt></cmt> (new)
- Long tag: <comment></comment> (new)

All formats support inline and multi-line usage and can be mixed within the same file. Updated documentation and error messages to be format-agnostic.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
@Lyken17 Lyken17 changed the title Add support for XML-style comment formats in refine-plan Add support for XML-style <cmt>comment</cmt> <comment>comment</comment> formats in refine-plan Apr 9, 2026
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Apr 9, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79714b73d5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread scripts/validate-refine-plan-io.sh
if (closest_marker == "") {
return ""
} else {
return closest_marker ":" closest_pos

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Encode marker results without : collisions

find_comment_markers serializes matches as marker:position, but the legacy start token itself contains a colon (CMT:), so classic matches become strings like CMT::12. The later split(..., ":") parsing then yields found_marker="CMT" and an empty marker_pos, which prevents recognizing the marker and can leave pos unchanged (pos = marker_pos + 1), causing scan_cmt_blocks to spin indefinitely on files that still use CMT: blocks. This breaks backward compatibility and can hang refine-plan validation on existing annotated plans.

Useful? React with 👍 / 👎.

@Lyken17 Lyken17 Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex address that feedback

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use Codex here, create a Codex account and connect to github.

@Lyken17 Lyken17 Apr 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex address that feedback

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use Codex here, create a Codex account and connect to github.

@Lyken17 Lyken17 closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Lyken17