diff --git a/docs/releases/4.2.0.md b/docs/releases/4.2.0.md index 11182cd0..2b26b4ee 100644 --- a/docs/releases/4.2.0.md +++ b/docs/releases/4.2.0.md @@ -84,6 +84,11 @@ ## Fixed +- (#1421) Fixed natural language parser only capturing one date when both scheduled and due dates are specified + - Input like "Task scheduled for tomorrow due next week" now correctly sets both scheduled and due dates + - Previously the parser would return early after finding the first date trigger, ignoring any subsequent triggers + - Thanks to @wealthychef1 for reporting + - (#1384) Fixed title being sanitized even when "Store Task Title in Filename" is disabled - Characters like `?`, `<`, `>`, `:`, etc. are now preserved in task titles when they won't be used in filenames - When `storeTitleInFilename` is false, only minimal sanitization (whitespace normalization, control character removal) is applied diff --git a/src/services/NaturalLanguageParser.ts b/src/services/NaturalLanguageParser.ts index 6adf34a3..e85e8632 100644 --- a/src/services/NaturalLanguageParser.ts +++ b/src/services/NaturalLanguageParser.ts @@ -715,7 +715,8 @@ export class NaturalLanguageParser { }, ]; - // Check for explicit triggers first + // Check for explicit triggers - process all triggers, not just the first one + let foundExplicitTrigger = false; for (const triggerPattern of triggerPatterns) { const match = workingText.match(triggerPattern.regex); if (match) { @@ -727,6 +728,7 @@ export class NaturalLanguageParser { const chronoParsed = this.parseChronoFromPosition(remainingText); if (chronoParsed.success) { + foundExplicitTrigger = true; // Assign to the correct field based on trigger type if (triggerPattern.type === "due") { result.dueDate = chronoParsed.date; @@ -746,11 +748,16 @@ export class NaturalLanguageParser { workingText = workingText.replace(chronoParsed.matchedText, ""); } workingText = this.cleanupWhitespace(workingText); - return workingText; // Early return after finding explicit trigger + // Continue processing to find additional triggers (Issue #1421) } } } + // Return early if we found explicit triggers - no need for implicit parsing + if (foundExplicitTrigger) { + return workingText; + } + // If no explicit triggers found, parse all remaining dates with context const parsedResults = chronoParser.parse(text, new Date(), { forwardDate: true }); if (parsedResults.length === 0) { diff --git a/tests/__mocks__/chrono-node.ts b/tests/__mocks__/chrono-node.ts index 9b809786..827321a7 100644 --- a/tests/__mocks__/chrono-node.ts +++ b/tests/__mocks__/chrono-node.ts @@ -94,8 +94,25 @@ const mockChrono = { new Date(parseInt(match[1]), parseInt(match[2]) - 1, parseInt(match[3])) }, { regex: /(\d{1,2})\/(\d{1,2})\/(\d{4})/i, dateFn: (match: RegExpMatchArray) => new Date(parseInt(match[3]), parseInt(match[1]) - 1, parseInt(match[2])) }, + // Month name patterns (e.g., "Jan 9", "January 15") + { regex: /\b(jan(?:uary)?|feb(?:ruary)?|mar(?:ch)?|apr(?:il)?|may|jun(?:e)?|jul(?:y)?|aug(?:ust)?|sep(?:t(?:ember)?)?|oct(?:ober)?|nov(?:ember)?|dec(?:ember)?)\s+(\d{1,2})\b/i, + dateFn: (match: RegExpMatchArray) => { + const monthNames: { [key: string]: number } = { + jan: 0, january: 0, feb: 1, february: 1, mar: 2, march: 2, apr: 3, april: 3, + may: 4, jun: 5, june: 5, jul: 6, july: 6, aug: 7, august: 7, sep: 8, sept: 8, september: 8, + oct: 9, october: 9, nov: 10, november: 10, dec: 11, december: 11 + }; + const month = monthNames[match[1].toLowerCase()]; + const day = parseInt(match[2]); + const year = new Date().getFullYear(); + return new Date(year, month, day); + } + }, ]; + // Track all matches with their positions to find all date expressions + const allMatches: Array<{ pattern: typeof patterns[0], match: RegExpMatchArray, targetDate: Date }> = []; + for (const pattern of patterns) { const match = text.match(pattern.regex); if (match) { @@ -120,24 +137,29 @@ const mockChrono = { targetDate.setHours(hour, 0, 0, 0); } - // Determine certain components based on pattern - let certainComponents: string[] = ['year', 'month', 'day']; - if (pattern.timeFn || pattern.regex.toString().includes('am|pm')) { - certainComponents.push('hour', 'minute'); - } - - results.push(createMockParsedResult( - match[0], // This now includes the full match like "tomorrow at 3pm" - targetDate, - match.index || 0, - undefined, - certainComponents - )); - - // Return first match only (like real chrono-node) - break; + allMatches.push({ pattern, match, targetDate }); } } + + // Sort matches by position in text (like real chrono-node) + allMatches.sort((a, b) => (a.match.index || 0) - (b.match.index || 0)); + + // Add results in order of appearance + for (const { pattern, match, targetDate } of allMatches) { + // Determine certain components based on pattern + let certainComponents: string[] = ['year', 'month', 'day']; + if (pattern.timeFn || pattern.regex.toString().includes('am|pm')) { + certainComponents.push('hour', 'minute'); + } + + results.push(createMockParsedResult( + match[0], + targetDate, + match.index || 0, + undefined, + certainComponents + )); + } return results; }), diff --git a/tests/unit/issues/issue-1421-dual-dates-nlp.test.ts b/tests/unit/issues/issue-1421-dual-dates-nlp.test.ts index 95b285b6..362d25b3 100644 --- a/tests/unit/issues/issue-1421-dual-dates-nlp.test.ts +++ b/tests/unit/issues/issue-1421-dual-dates-nlp.test.ts @@ -67,31 +67,28 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language }); describe("Dual date extraction", () => { - it.skip("should extract both scheduled and due dates when both are specified", () => { - // This test documents the bug - it should pass once fixed + it("should extract both scheduled and due dates when both are specified", () => { const input = "Task scheduled for tomorrow due next week"; const result = parser.parseInput(input); - // Currently fails: only scheduledDate is set because of early return expect(result.scheduledDate).toBeDefined(); expect(result.dueDate).toBeDefined(); expect(result.title).toBe("Task"); }); - it.skip("should handle due before scheduled in input order", () => { + it("should handle due before scheduled in input order", () => { // Testing the reverse order to ensure both triggers work regardless of order const input = "Task due next week scheduled for tomorrow"; const result = parser.parseInput(input); - // Currently fails: only dueDate is set because "due" trigger is first in array expect(result.scheduledDate).toBeDefined(); expect(result.dueDate).toBeDefined(); expect(result.title).toBe("Task"); }); - it.skip("should extract both dates with times when specified", () => { + it("should extract both dates with times when specified", () => { // Extended case with times const input = "Meeting scheduled for tomorrow at 9am due tomorrow at 5pm"; @@ -104,7 +101,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.dueTime).toBeDefined(); }); - it.skip("should handle same date for both scheduled and due", () => { + it("should handle same date for both scheduled and due", () => { // The exact scenario from the issue report const input = "Task scheduled for Jan 9 due Jan 9"; @@ -116,7 +113,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.scheduledDate).toBe(result.dueDate); }); - it.skip("should properly clean title after extracting both dates", () => { + it("should properly clean title after extracting both dates", () => { // Ensure the title is properly cleaned of both date expressions const input = "Complete report scheduled for tomorrow due next week #work"; @@ -161,7 +158,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language }); describe("Edge cases", () => { - it.skip("should handle multiple date formats in dual date input", () => { + it("should handle multiple date formats in dual date input", () => { // Different date formats for each trigger const input = "Project scheduled for 2025-01-15 due 2025-01-20"; @@ -171,7 +168,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.dueDate).toBe("2025-01-20"); }); - it.skip("should preserve tags and contexts with dual dates", () => { + it("should preserve tags and contexts with dual dates", () => { const input = "Task scheduled for tomorrow due next week #urgent @home +project"; const result = parser.parseInput(input); @@ -183,7 +180,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.projects).toContain("project"); }); - it.skip("should handle alternative trigger words for both dates", () => { + it("should handle alternative trigger words for both dates", () => { // Using alternative trigger words const input = "Task on tomorrow deadline next week"; @@ -220,7 +217,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.scheduledDate).toBeUndefined(); }); - it.skip("should ignore nlpDefaultToScheduled when both explicit triggers present", () => { + it("should ignore nlpDefaultToScheduled when both explicit triggers present", () => { // When explicit triggers are used, the setting should be irrelevant // because we're explicitly specifying both dates const parserDueDefault = new NaturalLanguageParser( @@ -238,46 +235,38 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language }); }); - describe("Trigger processing order (documents bug behavior)", () => { - it.skip("should process both triggers regardless of input order - scheduled first", () => { + describe("Trigger processing order", () => { + it("should process both triggers regardless of input order - scheduled first", () => { // User puts scheduled before due in their input const input = "Task scheduled for tomorrow due next week"; const result = parser.parseInput(input); - // BUG: Currently only dueDate is set because "due" is checked first in loop - // The loop order is ["due", "scheduled"], so "due next week" is found first expect(result.scheduledDate).toBeDefined(); expect(result.dueDate).toBeDefined(); }); - it.skip("should process both triggers regardless of input order - due first", () => { + it("should process both triggers regardless of input order - due first", () => { // User puts due before scheduled in their input const input = "Task due next week scheduled for tomorrow"; const result = parser.parseInput(input); - // BUG: Only dueDate is set because "due" is checked first in loop expect(result.scheduledDate).toBeDefined(); expect(result.dueDate).toBeDefined(); }); - it("documents current buggy behavior: only due is captured", () => { - // This test documents the CURRENT (buggy) behavior - // It should be updated when the fix is implemented + it("confirms fix: both scheduled and due are captured", () => { + // This test confirms the fix for Issue #1421 const input = "Task scheduled for tomorrow due next week"; const result = parser.parseInput(input); - // Currently, only dueDate is set because: - // 1. triggerPatterns loop order is ["due", "scheduled"] - // 2. "due" trigger is found at position in text - // 3. Parser returns early after processing first match + // Both dates should now be captured correctly expect(result.dueDate).toBeDefined(); - // This assertion documents the bug - scheduledDate should be set but isn't - expect(result.scheduledDate).toBeUndefined(); + expect(result.scheduledDate).toBeDefined(); }); }); describe("Alternative trigger words", () => { - it.skip("should handle 'deadline' trigger for due date with explicit scheduled", () => { + it("should handle 'deadline' trigger for due date with explicit scheduled", () => { const input = "Task scheduled for tomorrow deadline next week"; const result = parser.parseInput(input); @@ -285,7 +274,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.dueDate).toBeDefined(); }); - it.skip("should handle 'by' trigger for due date with explicit scheduled", () => { + it("should handle 'by' trigger for due date with explicit scheduled", () => { const input = "Task scheduled for tomorrow by next week"; const result = parser.parseInput(input); @@ -293,7 +282,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.dueDate).toBeDefined(); }); - it.skip("should handle 'start on' trigger for scheduled with explicit due", () => { + it("should handle 'start on' trigger for scheduled with explicit due", () => { const input = "Task start on tomorrow due next week"; const result = parser.parseInput(input); @@ -301,7 +290,7 @@ describe("Issue #1421: Setting both due and scheduled dates via natural language expect(result.dueDate).toBeDefined(); }); - it.skip("should handle 'work on' trigger for scheduled with explicit due", () => { + it("should handle 'work on' trigger for scheduled with explicit due", () => { const input = "Task work on tomorrow due next week"; const result = parser.parseInput(input);