Skip to content

fix(parser): classify delta operation by earliest keyword, hyphen-safe #1331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
seattled23 wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −9
Open

fix(parser): classify delta operation by earliest keyword, hyphen-safe #1331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions src/core/parsers/markdown-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,25 @@ export class MarkdownParser {

let operation: DeltaOperation = 'MODIFIED';
const lowerDesc = description.toLowerCase();

// Use word boundaries to avoid false matches (e.g., "address" matching "add")
// Check RENAMED first since it's more specific than patterns containing "new"
if (/\brename(s|d|ing)?\b/.test(lowerDesc) || /\brenamed\s+(to|from)\b/.test(lowerDesc)) {
operation = 'RENAMED';
} else if (/\badd(s|ed|ing)?\b/.test(lowerDesc) || /\bcreate(s|d|ing)?\b/.test(lowerDesc) || /\bnew\b/.test(lowerDesc)) {
operation = 'ADDED';
} else if (/\bremove(s|d|ing)?\b/.test(lowerDesc) || /\bdelete(s|d|ing)?\b/.test(lowerDesc)) {
operation = 'REMOVED';

// Classify by the FIRST operation keyword that appears, not a fixed
// type priority. Otherwise an incidental later keyword outranks the
// actual verb — e.g. "Remove the add-ons page" matched ADDED because
// ADDED was tested before REMOVED. The (?<![\w-])/(?![\w-]) boundaries
// also exclude hyphen-joined words so "add-ons"/"new-user" no longer
// match "add"/"new".
const opPatterns: Array<{ op: DeltaOperation; re: RegExp }> = [
{ op: 'RENAMED', re: /(?<![\w-])rename(s|d|ing)?(?![\w-])/ },
{ op: 'REMOVED', re: /(?<![\w-])(remove(s|d|ing)?|delete(s|d|ing)?)(?![\w-])/ },
{ op: 'ADDED', re: /(?<![\w-])(add(s|ed|ing)?|create(s|d|ing)?|new)(?![\w-])/ },
Comment on lines +207 to +209

@coderabbitai coderabbitai Bot Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix gerund matching for renaming, removing, and creating.

The current suffix pattern builds renameing, removeing, and createing, so descriptions like “Removing old flow” or “Creating new spec” fall back to MODIFIED.

Proposed fix 
-          { op: 'RENAMED', re: /(?<![\w-])rename(s|d|ing)?(?![\w-])/ },
-          { op: 'REMOVED', re: /(?<![\w-])(remove(s|d|ing)?|delete(s|d|ing)?)(?![\w-])/ },
-          { op: 'ADDED', re: /(?<![\w-])(add(s|ed|ing)?|create(s|d|ing)?|new)(?![\w-])/ },
+          { op: 'RENAMED', re: /(?<![\w-])(?:rename[sd]?|renaming)(?![\w-])/ },
+          { op: 'REMOVED', re: /(?<![\w-])(?:remove[sd]?|removing|delete[sd]?|deleting)(?![\w-])/ },
+          { op: 'ADDED', re: /(?<![\w-])(?:add(?:s|ed|ing)?|create[sd]?|creating|new)(?![\w-])/ },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{ op: 'RENAMED', re: /(?<![\w-])rename(s|d|ing)?(?![\w-])/ },
{ op: 'REMOVED', re: /(?<![\w-])(remove(s|d|ing)?|delete(s|d|ing)?)(?![\w-])/ },
{ op: 'ADDED', re: /(?<![\w-])(add(s|ed|ing)?|create(s|d|ing)?|new)(?![\w-])/ },
{ op: 'RENAMED', re: /(?<![\w-])(?:rename[sd]?|renaming)(?![\w-])/ },
{ op: 'REMOVED', re: /(?<![\w-])(?:remove[sd]?|removing|delete[sd]?|deleting)(?![\w-])/ },
{ op: 'ADDED', re: /(?<![\w-])(?:add(?:s|ed|ing)?|create[sd]?|creating|new)(?![\w-])/ },
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/parsers/markdown-parser.ts` around lines 207 - 209, The regexes in
markdown-parser.ts for the op mapping are building invalid gerund forms, so
`renaming`, `removing`, and `creating` are not matched and fall through to
`MODIFIED`. Update the word-pattern logic in the parser’s operation detection
table so the `RENAMED`, `REMOVED`, and `ADDED` cases match the correct gerund
spellings, and verify the matching behavior in the relevant parser
function/class that scans description text.

];
let bestIndex = Infinity;
for (const { op, re } of opPatterns) {
const match = lowerDesc.match(re);
if (match && match.index !== undefined && match.index < bestIndex) {
bestIndex = match.index;
operation = op;
}
}

deltas.push({
Expand Down
32 changes: 32 additions & 0 deletions test/core/parsers/markdown-parser.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,38 @@ We need to implement user authentication to secure the application and protect u
expect(change.deltas[2].operation).toBe('REMOVED');
});

it('classifies deltas by the first operation keyword, ignoring hyphen-joined words', () => {
// Each of these was previously misclassified: a fixed type-priority let an
// incidental later keyword outrank the real verb, and unbounded matching let
// "add-ons"/"new-user" match "add"/"new".
// - "Remove the add-ons page..." -> REMOVED (not ADDED via "add-ons")
// - "Add a rename button..." -> ADDED (not RENAMED via "rename")
// - "Remove the new-user flow..." -> REMOVED (not ADDED via "new-user")
const content = `# Reclassify Deltas

## Why
We need to confirm delta operations are classified by the leading verb, not by an incidental keyword appearing later in the sentence.

## What Changes
- **settings:** Remove the add-ons page from settings
- **toolbar:** Add a rename button to the toolbar
- **onboarding:** Remove the new-user onboarding flow`;

const parser = new MarkdownParser(content);
const change = parser.parseChange('reclassify-deltas');

expect(change.deltas).toHaveLength(3);

expect(change.deltas[0].spec).toBe('settings');
expect(change.deltas[0].operation).toBe('REMOVED');

expect(change.deltas[1].spec).toBe('toolbar');
expect(change.deltas[1].operation).toBe('ADDED');

expect(change.deltas[2].spec).toBe('onboarding');
expect(change.deltas[2].operation).toBe('REMOVED');
});

it('should throw error for missing why section', () => {
const content = `# Test Change

Expand Down