diff --git a/backend/src/Taskdeck.Application/Services/LlmIntentClassifier.cs b/backend/src/Taskdeck.Application/Services/LlmIntentClassifier.cs index 4ff0e1c83..93ee00187 100644 --- a/backend/src/Taskdeck.Application/Services/LlmIntentClassifier.cs +++ b/backend/src/Taskdeck.Application/Services/LlmIntentClassifier.cs @@ -1,7 +1,76 @@ +using System.Text.RegularExpressions; + namespace Taskdeck.Application.Services; public static class LlmIntentClassifier { + // Timeout to prevent catastrophic backtracking + private static readonly TimeSpan RegexTimeout = TimeSpan.FromMilliseconds(100); + + // Negative context patterns — suppress matches in these contexts + private static readonly Regex NegationPattern = new( + @"\b(don'?t|do not|never|stop|cancel|undo|avoid)\b(\s+\w+){0,6}\s+\b(create|add|make|move|archive|delete|remove|update|edit|rename|generate|build|set up|prepare)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + private static readonly Regex OtherToolPattern = new( + @"\b(in|for|with|using|on)\s+(jira|trello|asana|notion|monday|clickup|linear|github issues|azure devops)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + private static readonly Regex QuestionAboutHowPattern = new( + @"^\s*(how|what|where|when|why|can\s+i|is\s+it)\b.*\?$", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Card creation — verbs followed by optional words then card/task nouns + private static readonly Regex CardCreatePattern = new( + @"\b(create|add|make|generate|build|prepare|set\s+up)\b(\s+\w+){0,5}\s+\b(cards?|tasks?|items?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // "new card/task" with optional words between + private static readonly Regex NewCardPattern = new( + @"\b(new)\b(\s+\w+){0,4}\s+\b(cards?|tasks?|items?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Card move — "move" + optional words + "card/task" + private static readonly Regex CardMovePattern = new( + @"\bmove\b(\s+\w+){0,4}\s+\b(cards?|tasks?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Card archive — "archive/delete/remove" + optional words + "card/task" + private static readonly Regex CardArchivePattern = new( + @"\b(archive|delete|remove)\b(\s+\w+){0,4}\s+\b(cards?|tasks?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Card update — "update/edit/rename" + optional words + "card/task" + private static readonly Regex CardUpdatePattern = new( + @"\b(update|edit|rename|modify|change)\b(\s+\w+){0,4}\s+\b(cards?|tasks?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Board creation — verbs + optional words + "board" + private static readonly Regex BoardCreatePattern = new( + @"\b(create|add|make|generate|build|prepare|set\s+up|new)\b(\s+\w+){0,4}\s+\b(boards?|project\s+boards?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Board rename + private static readonly Regex BoardRenamePattern = new( + @"\b(rename|update|edit)\b(\s+\w+){0,4}\s+\bboards?\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + + // Column reorder / sort + private static readonly Regex ReorderPattern = new( + @"\b(reorder|sort|rearrange|reorganize)\b(\s+\w+){0,4}\s+\b(cards?|columns?|boards?)\b", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexTimeout); + public static (bool IsActionable, string? ActionIntent) Classify(string message) { if (string.IsNullOrWhiteSpace(message)) @@ -9,35 +78,142 @@ public static (bool IsActionable, string? ActionIntent) Classify(string message) var lower = message.ToLowerInvariant(); - // Card creation — explicit commands and natural language - if (lower.Contains("create card") || lower.Contains("add card") - || lower.Contains("create a card") || lower.Contains("add a card") - || lower.Contains("create task") || lower.Contains("add task") - || lower.Contains("create a task") || lower.Contains("add a task") - || lower.Contains("new card") || lower.Contains("new task") - || lower.Contains("make a card") || lower.Contains("make a task") - || lower.Contains("make card") || lower.Contains("make task")) - return (true, "card.create"); + // Check negative context first — suppress if negated or about another tool + if (IsNegativeContext(lower, message)) + return (false, null); - if (lower.Contains("move card")) - return (true, "card.move"); - if (lower.Contains("archive card") || lower.Contains("delete card") - || lower.Contains("remove card")) + // Archive/delete/remove must be checked BEFORE move to fix the + // "remove card" substring bug (remove contains "move") + if (MatchesCardArchive(lower)) return (true, "card.archive"); - if (lower.Contains("update card") || lower.Contains("edit card") - || lower.Contains("rename card")) + + if (MatchesCardMove(lower)) + return (true, "card.move"); + + if (MatchesCardUpdate(lower)) return (true, "card.update"); - if (lower.Contains("create board") || lower.Contains("add board") - || lower.Contains("new board")) + + if (MatchesCardCreate(lower)) + return (true, "card.create"); + + if (MatchesBoardCreate(lower)) return (true, "board.create"); - if (lower.Contains("rename board")) + + if (MatchesBoardRename(lower)) return (true, "board.update"); - if (lower.Contains("reorder cards") || lower.Contains("reorder column") - || lower.Contains("reorder columns") || lower.Contains("reorder board") - || lower.Contains("sort cards") || lower.Contains("sort column") - || lower.Contains("sort columns") || lower.Contains("sort board")) + + if (MatchesReorder(lower)) return (true, "column.reorder"); return (false, null); } + + private static bool IsNegativeContext(string lower, string original) + { + try + { + // Negation: "don't create task yet" + if (NegationPattern.IsMatch(lower)) + return true; + + // Asking about another tool: "how do I create a card in Jira?" + if (OtherToolPattern.IsMatch(lower) && QuestionAboutHowPattern.IsMatch(original.Trim())) + return true; + } + catch (RegexMatchTimeoutException) + { + // On timeout, fall through to normal classification + } + + return false; + } + + private static bool MatchesCardCreate(string lower) + { + try + { + if (CardCreatePattern.IsMatch(lower)) + return true; + if (NewCardPattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) + { + // Fall through — don't match on timeout + } + + return false; + } + + private static bool MatchesCardMove(string lower) + { + try + { + if (CardMovePattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } + + private static bool MatchesCardArchive(string lower) + { + try + { + if (CardArchivePattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } + + private static bool MatchesCardUpdate(string lower) + { + try + { + if (CardUpdatePattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } + + private static bool MatchesBoardCreate(string lower) + { + try + { + if (BoardCreatePattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } + + private static bool MatchesBoardRename(string lower) + { + try + { + if (BoardRenamePattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } + + private static bool MatchesReorder(string lower) + { + try + { + if (ReorderPattern.IsMatch(lower)) + return true; + } + catch (RegexMatchTimeoutException) { } + + return false; + } } diff --git a/backend/tests/Taskdeck.Application.Tests/Services/LlmIntentClassifierTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/LlmIntentClassifierTests.cs index f4577031d..5ef04dfb8 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/LlmIntentClassifierTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/LlmIntentClassifierTests.cs @@ -6,7 +6,7 @@ namespace Taskdeck.Application.Tests.Services; public class LlmIntentClassifierTests { - #region Card Creation — Current Supported Patterns + #region Card Creation — Exact Patterns (Backward Compatibility) [Theory] [InlineData("create card \"test\"")] @@ -33,7 +33,35 @@ public void Classify_CardCreation_ShouldDetect_ExactPatterns(string message) #endregion - #region Card Operations — Current Supported Patterns + #region Card Creation — Natural Language Patterns (New) + + [Theory] + [InlineData("can you create new onboarding tasks for people who aren't technical?")] + [InlineData("create some tasks for the onboarding process")] + [InlineData("could you make me a few cards for the backlog?")] + [InlineData("generate tasks for the release checklist")] + [InlineData("please add these items: meeting notes, code review, deployment")] + [InlineData("I need three new cards for the sprint")] + [InlineData("create tasks for onboarding")] + [InlineData("create three cards for deployment")] + [InlineData("add several tasks to the board")] + [InlineData("create a new onboarding task")] + [InlineData("generate a card for this feature")] + [InlineData("build out some tasks for the release")] + [InlineData("prepare a task for code review")] + [InlineData("set up a few cards for the sprint")] + public void Classify_CardCreation_ShouldDetect_NaturalLanguage(string message) + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); + + isActionable.Should().BeTrue( + because: $"'{message}' expresses card creation intent"); + actionIntent.Should().Be("card.create"); + } + + #endregion + + #region Card Operations — Exact Patterns (Backward Compatibility) [Fact] public void Classify_MoveCard_ShouldDetect() @@ -56,19 +84,17 @@ public void Classify_ArchiveCard_ShouldDetect(string message) } /// - /// Documents a classifier ordering bug: "remove card" contains the substring - /// "move card", so the move-card check fires first and returns card.move - /// instead of card.archive. See #571 for fix. + /// Fixed: "remove card" now correctly classifies as card.archive + /// because archive/delete/remove are checked before move. /// [Fact] - public void Classify_RemoveCard_MatchesMoveCardDueToSubstringOrdering() + public void Classify_RemoveCard_ShouldClassifyAsArchive() { var (isActionable, actionIntent) = LlmIntentClassifier.Classify("remove card abc123"); isActionable.Should().BeTrue(); - // Bug: "remove card" contains "move card" as substring, so card.move fires first - actionIntent.Should().Be("card.move", - because: "substring 'move card' appears inside 'remove card' — classifier ordering bug tracked in #571"); + actionIntent.Should().Be("card.archive", + because: "archive/delete/remove checks now run before move checks"); } [Theory] @@ -85,7 +111,7 @@ public void Classify_UpdateCard_ShouldDetect(string message) #endregion - #region Board Operations — Current Supported Patterns + #region Board Operations — Exact Patterns (Backward Compatibility) [Theory] [InlineData("create board")] @@ -123,21 +149,18 @@ public void Classify_Reorder_ShouldDetect(string message) #endregion - #region Non-Actionable — Should Return False + #region Board Operations — Natural Language (New) [Theory] - [InlineData("what is the weather today?")] - [InlineData("tell me about project management")] - [InlineData("how does taskdeck work?")] - [InlineData("explain the board layout")] - [InlineData("")] - [InlineData("hello")] - public void Classify_NonActionable_ShouldReturnFalse(string message) + [InlineData("set up a project board for Q2 planning")] + [InlineData("build out the sprint board with planning items")] + public void Classify_BoardCreation_ShouldDetect_NaturalLanguage(string message) { var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); - isActionable.Should().BeFalse(); - actionIntent.Should().BeNull(); + isActionable.Should().BeTrue( + because: $"'{message}' expresses board creation intent"); + actionIntent.Should().Be("board.create"); } #endregion @@ -222,102 +245,141 @@ public void Classify_PatternWithNewlines_StillMatches() #region Known Gaps — Natural Language Misses (Documents #570/#571) - /// - /// These tests document the current NLP gap where natural language phrasing - /// that expresses card creation intent is NOT detected by the classifier. - /// When #571 (improved classifier) is implemented, these should be updated - /// to assert IsActionable == true. - /// [Theory] - [InlineData("can you create new onboarding tasks for people who aren't technical?")] - // Note: "I need three new cards for the sprint" is NOT here because - // "new cards" contains "new card" as substring — it DOES match card.create. - // This is accidental correctness from substring matching, not intentional NLP. - [InlineData("set up a project board for Q2 planning")] - [InlineData("please add these items: meeting notes, code review, deployment")] - [InlineData("create some tasks for the onboarding process")] - [InlineData("could you make me a few cards for the backlog?")] - [InlineData("generate tasks for the release checklist")] - [InlineData("build out the sprint board with planning items")] - public void Classify_NaturalLanguage_CurrentlyMisses_CardCreationIntent(string message) + [InlineData("don't create task yet, just explain")] + [InlineData("do not create a card until I approve")] + [InlineData("don't add task please")] + [InlineData("never create tasks automatically")] + [InlineData("cancel the add task operation")] + public void Classify_Negation_ShouldReturnFalse(string message) { - // Documents current behavior: these natural language phrases are NOT detected - // See #570 and #571 for the improvement plan var (isActionable, _) = LlmIntentClassifier.Classify(message); isActionable.Should().BeFalse( - because: $"current classifier uses exact substring matching and misses natural phrasing: '{message}'. " + - "This documents a known gap tracked in #570/#571."); + because: $"'{message}' contains a negation that should suppress the intent"); } /// - /// Documents cases where words are present but not adjacent, - /// causing the substring matcher to miss them. + /// "stop creating cards" is non-actionable, but NOT because negation catches it. + /// The gerund "creating" doesn't match \bcreate\b in either the negation or + /// positive patterns. This documents a known gap: gerund forms are invisible + /// to both negation and classification. /// + [Fact] + public void Classify_GerundForm_IsNonActionable_ButNotDueToNegation() + { + var (isActionable, _) = LlmIntentClassifier.Classify("stop creating cards"); + + isActionable.Should().BeFalse( + because: "gerund 'creating' does not match \\bcreate\\b — neither negation nor positive patterns fire"); + } + + #endregion + + #region Negative Context — Questions About Other Tools + [Theory] - [InlineData("create a new onboarding task")] // "create" + gap + "task" — not "create task" - [InlineData("create three cards for deployment")] // "create" + gap + "cards" — not "create card" - [InlineData("add several tasks to the board")] // "add" + gap + "tasks" — not "add task" - public void Classify_WordGap_CurrentlyMisses(string message) + [InlineData("how do I create a card in Jira?")] + [InlineData("how can I add a task in Trello?")] + [InlineData("where do I create cards in Asana?")] + public void Classify_OtherToolQuestions_ShouldReturnFalse(string message) { - // "create" and "task" are both present but not adjacent - // Current classifier requires exact substrings like "create task" var (isActionable, _) = LlmIntentClassifier.Classify(message); - // Note: some of these may currently match depending on exact wording. - // The point is to document the fragility of substring matching. - // If this test fails because the classifier DID match, that's fine — - // update the test to reflect reality. - if (!isActionable) - { - isActionable.Should().BeFalse( - because: "documents word-gap limitation in substring matching"); - } + isActionable.Should().BeFalse( + because: $"'{message}' is a question about another tool, not a Taskdeck action"); } /// - /// Documents potential false positives where the classifier triggers - /// on keywords in non-actionable context. + /// Commands (not questions) mentioning other tools should still classify, + /// because the user might be saying "create a card like I do in Jira". /// [Theory] - [InlineData("how do I create a card in Jira?")] // Asking about another tool - [InlineData("don't create task yet, just explain")] // Negation - [InlineData("I deleted the create card button by accident")] // Past tense / UI reference - public void Classify_FalsePositives_CurrentBehavior(string message) + [InlineData("create a card similar to what I have in Jira")] + [InlineData("add task, I used to do this in Trello")] + public void Classify_CommandsMentioningOtherTools_ShouldStillMatch(string message) { - // These SHOULD ideally be non-actionable but may trigger due to keyword presence - // Documents the false-positive gap tracked in #571 var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); - // Just document what happens — don't assert a specific expectation - // because these are edge cases that will be addressed in #571 - if (isActionable) - { - // False positive: classifier triggered on keywords in non-actionable context - actionIntent.Should().NotBeNull( - because: "if actionable, intent should be set"); - } + isActionable.Should().BeTrue( + because: $"'{message}' is a command, not a question about another tool"); + actionIntent.Should().Be("card.create"); } #endregion - #region Accidental Matches — Work Due to Substring Overlap + #region Non-Actionable — Should Return False - /// - /// These match by accident because the plural form contains the singular - /// as a substring (e.g., "new cards" contains "new card"). - /// [Theory] - [InlineData("I need three new cards for the sprint")] // "new cards" contains "new card" - [InlineData("create tasks for onboarding")] // "create task" is in "create tasks" - public void Classify_AccidentalSubstringMatches_CurrentlyWork(string message) + [InlineData("what is the weather today?")] + [InlineData("tell me about project management")] + [InlineData("how does taskdeck work?")] + [InlineData("explain the board layout")] + [InlineData("")] + [InlineData("hello")] + [InlineData(" ")] + public void Classify_NonActionable_ShouldReturnFalse(string message) { var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); - isActionable.Should().BeTrue(); + isActionable.Should().BeFalse(); + actionIntent.Should().BeNull(); + } + + #endregion + + #region Edge Cases + + /// + /// Known false positive: "I deleted the create card button by accident" + /// contains the literal substring "create card", so it matches card.create. + /// Fixing this would require POS tagging or context analysis beyond keyword matching. + /// + [Fact] + public void Classify_PastTenseNarrative_IsKnownFalsePositive() + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify( + "I deleted the create card button by accident"); + + isActionable.Should().BeTrue( + because: "substring 'create card' appears literally — known false positive"); actionIntent.Should().Be("card.create"); } + [Fact] + public void Classify_NullInput_ShouldReturnFalse() + { + // null is not valid for the parameter type, but whitespace-only is + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(" \t "); + + isActionable.Should().BeFalse(); + actionIntent.Should().BeNull(); + } + + [Fact] + public void Classify_VeryLongInput_ShouldNotHang() + { + // Ensure regex doesn't catastrophically backtrack on long input + var longMessage = "please " + string.Join(" ", Enumerable.Repeat("very", 200)) + " create a task for me"; + + // Should complete without timeout; may or may not match depending on word count limit + var (isActionable, _) = LlmIntentClassifier.Classify(longMessage); + + // We just verify it completes without throwing + _ = isActionable; + } + + [Fact] + public void Classify_MixedIntents_FirstMatchWins() + { + // "archive" is checked before "move" and "create" + var (isActionable, actionIntent) = LlmIntentClassifier.Classify("archive card and create task"); + + isActionable.Should().BeTrue(); + actionIntent.Should().Be("card.archive", + because: "archive is checked before create in the classification order"); + } + #endregion #region Case Insensitivity @@ -326,6 +388,8 @@ public void Classify_AccidentalSubstringMatches_CurrentlyWork(string message) [InlineData("CREATE CARD \"test\"")] [InlineData("Create Card \"test\"")] [InlineData("cReAtE cArD \"test\"")] + [InlineData("GENERATE TASKS for release")] + [InlineData("Build Some Tasks for sprint")] public void Classify_ShouldBeCaseInsensitive(string message) { var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); @@ -335,4 +399,68 @@ public void Classify_ShouldBeCaseInsensitive(string message) } #endregion + + #region Plural Forms + + [Theory] + [InlineData("create cards for the team")] + [InlineData("add tasks to the board")] + [InlineData("new items for sprint planning")] + [InlineData("make cards for each milestone")] + public void Classify_PluralForms_ShouldMatchCardCreate(string message) + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); + + isActionable.Should().BeTrue(); + actionIntent.Should().Be("card.create"); + } + + [Theory] + [InlineData("delete cards from the archive")] + [InlineData("remove tasks that are done")] + public void Classify_PluralForms_ShouldMatchCardArchive(string message) + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); + + isActionable.Should().BeTrue(); + actionIntent.Should().Be("card.archive"); + } + + #endregion + + #region Broader Verb Coverage + + [Theory] + [InlineData("generate a task for deployment")] + [InlineData("build a card for the feature")] + [InlineData("prepare tasks for the meeting")] + [InlineData("set up items for review")] + public void Classify_BroaderVerbs_ShouldMatchCardCreate(string message) + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); + + isActionable.Should().BeTrue(); + actionIntent.Should().Be("card.create"); + } + + #endregion + + #region Word-Distance Matching + + [Theory] + [InlineData("create a new onboarding task")] + [InlineData("create three cards for deployment")] + [InlineData("add several tasks to the board")] + [InlineData("make me a couple of cards")] + [InlineData("generate some important tasks")] + public void Classify_WordGap_ShouldNowMatch(string message) + { + var (isActionable, actionIntent) = LlmIntentClassifier.Classify(message); + + isActionable.Should().BeTrue( + because: $"'{message}' has verb and noun with words in between"); + actionIntent.Should().Be("card.create"); + } + + #endregion }