Implement auto-fix for markdown flavor validation rule - #169
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
==========================================
+ Coverage 89.60% 89.65% +0.05%
==========================================
Files 112 113 +1
Lines 12062 12163 +101
==========================================
+ Hits 10808 10905 +97
- Misses 796 798 +2
- Partials 458 460 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements auto-fix support for MDS034 (markdown flavor validation) so mdsmith fix can rewrite unsupported Markdown constructs into flavor-compatible syntax.
Changes:
- Added a byte-range edit framework to remove/transform six unsupported feature syntaxes (heading IDs, strikethrough, task lists, superscript, subscript, bare URLs).
- Refactored MDS034’s
Fix()flow to run GitHub Alerts marker stripping first, then apply byte-range fixes depending on configured flavor support. - Added unit tests and golden “fixed” fixtures; marked plan 86 complete in planning docs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plan/86_markdown-flavor-validation.md | Marks the plan’s auto-fix acceptance criteria as complete and documents what is fixed. |
| internal/rules/markdownflavor/rule.go | Refactors Fix() into GitHub Alerts stripping + delegation to byte-range fix pipeline. |
| internal/rules/markdownflavor/fix.go | New byte-range edit collection + application logic for six fixable features. |
| internal/rules/markdownflavor/fix_test.go | New focused unit tests for each fix plus a multi-edit composition test. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-task-list.md | Golden output for task list fix under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-superscript.md | Golden output for superscript fix under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-subscript.md | Golden output for subscript fix under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-strikethrough.md | Golden output for strikethrough fix under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-heading-id.md | Golden output for heading ID fix under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-bare-url.md | Golden output for bare URL autolink wrapping under CommonMark. |
| PLAN.md | Updates plan 86 status to ✅ in the top-level plan index. |
Resolves four review comments on PR #169: - delimiterPairEdits was off-by-one for wrappers with nested inline markup (`~~*bold*~~` would have produced `~bold~`); rewrite to accept only single-Text-child wrappers and decline gracefully when emphasis, links, or breaks appear inside, leaving the diagnostic for manual resolution. - Fix() no longer returns early after stripping GitHub Alert markers. After alerts are removed it re-parses the rewritten source and runs the byte-range pipeline on the new AST so heading IDs / strike- through / bare URLs in alert-bearing docs get fixed in one call. - applyEdits now builds the output in a single ascending-order pass with a pre-sized buffer (was O(n*m) due to per-edit slice copies). - Add tests covering: alerts + bare URL composition, nested-inline rejection (single-child and multi-sibling), FlavorAny no-op, and the supports-branch returns in dualNodeEdits for super/sub. Also drops dead defensive code (Extra type assertion, marker bounds checks, taskCheckBox guards, applyEdits overlap check) per repo guidance to trust framework guarantees, lifting fix.go to 100% statement coverage and addressing the codecov patch gate.
Resolves four review comments on PR #169: - delimiterPairEdits was off-by-one for wrappers with nested inline markup (`~~*bold*~~` would have produced `~bold~`); rewrite to accept only single-Text-child wrappers and decline gracefully when emphasis, links, or breaks appear inside, leaving the diagnostic for manual resolution. - Fix() no longer returns early after stripping GitHub Alert markers. After alerts are removed it re-parses the rewritten source and runs the byte-range pipeline on the new AST so heading IDs / strike- through / bare URLs in alert-bearing docs get fixed in one call. - applyEdits now builds the output in a single ascending-order pass with a pre-sized buffer (was O(n*m) due to per-edit slice copies). - Add tests covering: alerts + bare URL composition, nested-inline rejection (single-child and multi-sibling), FlavorAny no-op, and the supports-branch returns in dualNodeEdits for super/sub. Also drops dead defensive code (Extra type assertion, marker bounds checks, taskCheckBox guards, applyEdits overlap check) per repo guidance to trust framework guarantees, lifting fix.go to 100% statement coverage and addressing the codecov patch gate.
76e22e2 to
8f916c7
Compare
There was a problem hiding this comment.
Pull request overview
Adds auto-fix support to MDS034 (markdown flavor validation) so mdsmith fix can rewrite unsupported Markdown constructs to a flavor-compatible form.
Changes:
- Introduces a byte-range edit pipeline to fix six unsupported feature types (heading IDs, strikethrough, task lists, super/subscript, bare-URL autolinks).
- Refactors MDS034
Fix()to compose GitHub Alerts marker stripping with the new byte-range fixes, including re-parsing when the source changes. - Adds unit tests plus fixed-output fixtures and marks plan 86 complete.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plan/86_markdown-flavor-validation.md | Marks the MDS034 plan slice complete and documents the set of auto-fixes. |
| internal/rules/markdownflavor/rule.go | Composes alert stripping + byte-range fixing; re-parses after alert edits to keep offsets correct. |
| internal/rules/markdownflavor/fix.go | New edit collection + application pipeline for fixable flavor features. |
| internal/rules/markdownflavor/fix_test.go | Adds focused and composition tests for each fix behavior. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-task-list.md | Golden fixed output for task list removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-superscript.md | Golden fixed output for superscript removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-subscript.md | Golden fixed output for subscript removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-strikethrough.md | Golden fixed output for strikethrough removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-heading-id.md | Golden fixed output for heading-id attribute stripping under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-bare-url.md | Golden fixed output for bare-URL wrapping under CommonMark. |
| PLAN.md | Updates plan index to reflect plan 86 completion. |
Address PR #169 review nit: TestRuleFixMultipleFeaturesOnOneLine's doc comment described "reverse-order edit application", which was true of the prior implementation. applyEdits now sorts edits in ascending start order and builds the output in a single forward pass, so update the comment to match.
There was a problem hiding this comment.
Pull request overview
Implements auto-fix support for MDS034 (markdown flavor validation), allowing mdsmith fix to rewrite several unsupported Markdown constructs based on the configured flavor.
Changes:
- Added a byte-range edit pipeline to remove/transform six fixable feature types (e.g., heading IDs, strikethrough, task lists, super/subscript, bare URLs).
- Refactored
Rule.Fix()to compose GitHub Alerts marker stripping with the new byte-range feature fixes (including re-parsing when alerts are stripped). - Added unit tests and rule fixtures (golden “fixed/” outputs) for CommonMark auto-fix behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plan/86_markdown-flavor-validation.md | Marks the plan as complete and records the implemented fix scope. |
| internal/rules/markdownflavor/rule.go | Composes alert stripping with byte-range fixes, re-parsing as needed. |
| internal/rules/markdownflavor/fix.go | Implements edit collection + single-pass application for fixable features. |
| internal/rules/markdownflavor/fix_test.go | Adds targeted unit coverage for each fix plus composition cases. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-task-list.md | Golden fixed output for task-list marker removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-superscript.md | Golden fixed output for superscript marker removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-subscript.md | Golden fixed output for subscript marker removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-strikethrough.md | Golden fixed output for strikethrough marker removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-heading-id.md | Golden fixed output for heading ID attribute removal under CommonMark. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-bare-url.md | Golden fixed output for bare URL wrapping under CommonMark. |
| PLAN.md | Updates the plan index to reflect completion of plan 86. |
jeduden
added
queue
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
added
queue
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
jeduden
added
queue:active
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
removed
the
queue:active
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
jeduden
added
queue
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
added
queue:active
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
added
queue
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
jeduden
added
queue:active
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
removed
the
queue:active
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
jeduden
added
queue
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
jeduden
removed
the
queue:active
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
jeduden
added
queue
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
|
⏳ Merge Queue — requeued The merge queue hit an error while processing this PR:
Next: No action needed — the queue will retry on the next tick. |
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
Adds byte-range edit pipeline to markdown-flavor (MDS034) Fix so heading IDs, strikethrough, task lists, superscript, subscript, and bare-URL autolinks are auto-fixed when the configured flavor does not support them. Fix() now runs the existing GitHub Alert handler first (line-based, handles lazy continuation), then dispatches to the new fixByteRangeFeatures pass which collects edits from the dual-parser AST and the bare-URL detector and applies them in reverse start order. Six fixed/ fixtures and 14 unit tests cover each feature plus a combined case (strikethrough + heading ID + bare URL on adjacent lines) that exercises reverse-order edit composition.
Resolves four review comments on PR #169: - delimiterPairEdits was off-by-one for wrappers with nested inline markup (`~~*bold*~~` would have produced `~bold~`); rewrite to accept only single-Text-child wrappers and decline gracefully when emphasis, links, or breaks appear inside, leaving the diagnostic for manual resolution. - Fix() no longer returns early after stripping GitHub Alert markers. After alerts are removed it re-parses the rewritten source and runs the byte-range pipeline on the new AST so heading IDs / strike- through / bare URLs in alert-bearing docs get fixed in one call. - applyEdits now builds the output in a single ascending-order pass with a pre-sized buffer (was O(n*m) due to per-edit slice copies). - Add tests covering: alerts + bare URL composition, nested-inline rejection (single-child and multi-sibling), FlavorAny no-op, and the supports-branch returns in dualNodeEdits for super/sub. Also drops dead defensive code (Extra type assertion, marker bounds checks, taskCheckBox guards, applyEdits overlap check) per repo guidance to trust framework guarantees, lifting fix.go to 100% statement coverage and addressing the codecov patch gate.
Address PR #169 review nit: TestRuleFixMultipleFeaturesOnOneLine's doc comment described "reverse-order edit application", which was true of the prior implementation. applyEdits now sorts edits in ascending start order and builds the output in a single forward pass, so update the comment to match.
There was a problem hiding this comment.
Pull request overview
Implements the remaining auto-fix pipeline for rule MDS034 (markdown-flavor) so mdsmith fix can rewrite certain unsupported Markdown constructs into flavor-compatible syntax (instead of only emitting diagnostics), while preserving correct AST byte offsets after alert-marker stripping.
Changes:
- Added a byte-range edit pipeline (
fix.go) to auto-fix six unsupported feature types (heading IDs, strikethrough, task lists, superscript, subscript, bare-URL autolinks). - Refactored
Rule.Fix()to compose GitHub Alerts marker stripping with the byte-range fix pass, including a re-parse step when the alert rewrite changes bytes. - Added unit tests and folder-based golden
fixed/fixtures validating fix output.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| plan/86_markdown-flavor-validation.md | Marks the plan complete and records the now-implemented fix surface area. |
| internal/rules/markdownflavor/rule.go | Updates Fix() to apply alert stripping + reparse + byte-range feature fixes in one call. |
| internal/rules/markdownflavor/fix.go | Introduces edit collection + applyEdits infrastructure and concrete fix implementations for six features. |
| internal/rules/markdownflavor/fix_test.go | Adds focused unit tests for each fixer and composition/edge-case behavior. |
| internal/rules/MDS034-markdown-flavor/fixed/commonmark-*.md | Adds golden outputs for Fix() under CommonMark flavor for each newly fixable feature. |
| PLAN.md | Updates the plan index row for plan 86 to ✅. |
|
🟢 Merge Queue — picked up This PR is in the queue and will be batched with other Next: No action needed — you'll get another comment when CI starts on the batch. View merge queue run. |
|
🔵 Merge Queue — CI running Merged into batch branch Next: No action needed — you'll be notified when CI completes. |
|
✅ Merge Queue — merged This PR landed on Next: Done — nothing more to do here. |
Summary
Implements the auto-fix functionality for the markdown flavor validation rule (MDS034), enabling
mdsmith fixto automatically correct unsupported markdown features based on the configured flavor.Key Changes
Added
fix.go: New module implementing byte-range edit infrastructure for fixing six unsupported markdown features:{#id}attribute blocks)~~delimiters)[x]checkboxes while preserving bullets)^delimiters)~delimiters)<url>)Updated
rule.go: RefactoredFix()method to:fixByteRangeFeatures()for the six byte-range features in the same callAdded comprehensive test coverage (
fix_test.go):Fix()call~~*bold*~~)Added golden test fixtures: Six markdown files demonstrating the expected output after fixing each feature type for CommonMark flavor
Implementation Details
applyEditssorts edits by ascending start offset and builds the output in a single forward pass: it appends each unchanged span from the source, then the edit's replacement bytes, advancing a cursor across non-overlapping edits. The output buffer is pre-sized from the cumulative size delta so the build runs in O(len(src) + total edit work) without per-edit reallocation.The dual-parser technique detects features like heading IDs and strikethrough by parsing with an extended markdown parser, while bare URLs are detected on the standard CommonMark AST to correctly skip URLs inside code spans and fences.
delimiterPairEditsonly rewrites wrappers with a single Text child; nested inline markup (emphasis, links, code spans) declines the fix and leaves the diagnostic for the user, since locating the wrapper marker through nested children would require recursively reconstructing each child's own marker span.The plan status for MDS034 is now marked complete (✅).
https://claude.ai/code/session_01HyTXePzdUP6WLrwX571gk8