Implement Robust Fallback for Unknown Queries#70
Implement Robust Fallback for Unknown Queries#70vaibhav45sktech wants to merge 6 commits intodbpedia:masterfrom
Conversation
📝 WalkthroughWalkthroughImplements responsive flex styling for smart-reply UI, augments TextHandler with a fallback suggestion flow that appends DBpedia-focused smart replies (using keyword extraction and Eliza as needed), and updates RiveScript fallback messages; README badge removed. Changes
Sequence DiagramsequenceDiagram
participant User
participant TextHandler
participant NLHandler
participant Eliza
participant DBpedia
participant ResponseBuilder
User->>TextHandler: submitText(text)
TextHandler->>NLHandler: requestReplies(text)
alt NL replies returned
NLHandler-->>TextHandler: replies
else no NL replies or punctuation-triggered fallback
TextHandler->>Eliza: (optional) requestElizaReply(text)
Eliza-->>TextHandler: elizaReply
TextHandler->>TextHandler: extractKeyword(text)
TextHandler->>DBpedia: querySuggestions(keyword)
DBpedia-->>TextHandler: suggestions
end
TextHandler->>ResponseBuilder: buildResponse(replies, suggestions)
ResponseBuilder-->>User: sendResponse(response)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/lib/handlers/TextHandler.java`:
- Around line 119-122: The current assignment to fallbackPrompt in TextHandler
uses helper.getRiveScriptBot().answer(request.getUserId(),
RiveScriptReplyType.FALLBACK_TEXT)[0] which can throw
ArrayIndexOutOfBoundsException if the returned String[] is empty; update the
code to capture the String[] result from helper.getRiveScriptBot().answer(...),
check if the array length is > 0 and use result[0], otherwise assign a safe
hardcoded default fallback (e.g., "Sorry, I don't understand.") to
fallbackPrompt; reference the helper.getRiveScriptBot().answer call,
RiveScriptReplyType.FALLBACK_TEXT, and fallbackPrompt when making this change.
- Around line 81-101: The code sets fallbackTriggered true before calling
NLHandler.answer() which causes appendFallbackSuggestions to run even when
NLHandler returned a valid response; change the logic in TextHandler so that you
do not unconditionally set fallbackTriggered=true before invoking new
NLHandler(request, textMessage, helper).answer(), instead call
NLHandler.answer(), then inspect the returned responseGenerator (or its response
list via responseGenerator.getResponse().size()) and only set fallbackTriggered
(or invoke appendFallbackSuggestions) when that response is empty; update any
references to fallbackTriggered and the FALLBACK_SCENARIO branch to use the
actual result from NLHandler.answer() to decide whether to
appendFallbackSuggestions.
🧹 Nitpick comments (6)
src/main/resources/rivescript/scenario-text.rive (1)
43-47:fallbacktextandnoresultstextnow have identical responses — intentional?Both triggers return the exact same string. This means the user sees identical messaging whether the bot hit its general fallback or specifically found no results, making it harder to distinguish scenarios during debugging and providing no differentiated guidance to the user. If this is intentional, consider at least adding a second
-alternative to each so RiveScript can vary responses.app/src/less/modules/smart-reply.less (2)
15-17:transition: allmay cause unintended animation side-effects.Transitioning every property (including layout-related ones like
padding,border-width) can produce visual jank and hurt performance. Scope it to the properties you actually intend to animate.♻️ Suggested change
- transition: all 120ms ease-in-out; + transition: background 120ms ease-in-out, color 120ms ease-in-out, box-shadow 120ms ease-in-out;
20-27: Focus indicator may not meet WCAG 2.1 requirements.The
:focusstate relies solely on a box-shadow glow with no distinct outline. Users navigating by keyboard may have difficulty identifying which pill is focused, especially on high-contrast or forced-color modes wherebox-shadowis dropped. Consider addingoutlineor using:focus-visiblewith a clear ring.♿ Suggested addition
.smart-reply:hover, .smart-reply:active, .smart-reply:focus { background: `@brand-primary`; color: `@white`; box-shadow: 0 4px 10px fade(`@brand-primary`, 35%); text-decoration: none; } + +.smart-reply:focus-visible { + outline: 2px solid `@brand-primary`; + outline-offset: 2px; +}src/main/java/chatbot/lib/handlers/TextHandler.java (3)
144-167: Stop-word set is re-created on every invocation; use astatic finalfield instead.
extractKeywordallocates a newHashSetand array each time it's called. Since the stop-word list is fixed, promote it to aprivate static final Set<String>class constant. Also, prefer the already-importedjava.utiltypes rather than inline fully-qualified names.♻️ Proposed refactor
Add a class-level constant:
private static final java.util.Set<String> STOP_WORDS = java.util.Set.of( "what", "who", "where", "why", "how", "is", "are", "the", "a", "an", "of", "in", "on", "for", "with", "to", "tell", "me", "about", "please");Then simplify
extractKeyword:private String extractKeyword(String text) { if (text == null || text.trim().length() == 0) { return null; } String cleaned = text.replaceAll("[^A-Za-z0-9 ]", " ").toLowerCase(); String[] tokens = cleaned.split(" +"); - String[] stop = new String[] { "what", "who", "where", "why", "how", "is", "are", "the", "a", "an", "of", "in", - "on", "for", "with", "to", "tell", "me", "about", "please" }; - java.util.Set<String> stopSet = new java.util.HashSet<>(); - for (String s : stop) - stopSet.add(s); String best = null; for (String t : tokens) { if (t.length() < 3) continue; - if (stopSet.contains(t)) + if (STOP_WORDS.contains(t)) continue; if (best == null || t.length() > best.length()) { best = t; } } return best; }
59-60: Nit: redundant== truecomparison.
Utility.isJSONObject(reply)already returns aboolean; the== trueis unnecessary.♻️ Suggested change
- if (Utility.isJSONObject(reply) == true) { + if (Utility.isJSONObject(reply)) {
39-39: PreferisEmpty()oversize() > 0.Minor style nit —
!replacements.isEmpty()is the idiomatic Java way to check for a non-empty collection.♻️ Suggested change
- if (replacements.size() > 0) { + if (!replacements.isEmpty()) {
|
@CodeRabbit please review |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/lib/handlers/TextHandler.java`:
- Around line 114-132: appendFallbackSuggestions currently always adds a “I
didn't quite understand…” prompt which conflicts when the Eliza path already
added a meaningful reply (controlled by fallbackTriggered); modify
appendFallbackSuggestions (or its caller) to check the fallbackTriggered flag
(set when Eliza responds) and either skip adding the fallback prompt when
fallbackTriggered==true or switch to a non-contradictory message (e.g., "Here
are some things I can help with:") before calling
responseGenerator.addTextResponse; locate the logic in appendFallbackSuggestions
and where fallbackTriggered is set/checked to implement the conditional behavior
and adjust the prompt selection accordingly.
🧹 Nitpick comments (3)
src/main/java/chatbot/lib/handlers/TextHandler.java (3)
102-110: Eliza final-fallback (lines 107-110) is now unreachable dead code.
appendFallbackSuggestionsalways adds at least one text response (line 132), so after the block at lines 103-105 runs,responseGenerator.getResponse().size()is guaranteed to be > 0. The only path that skips lines 103-105 is whenfallbackTriggered == false && size > 0— which also means line 108's check fails. No execution path reaches line 109.This dead block may mask future bugs if the fallback flow is modified. Consider removing it or restructuring so the Eliza fallback is an explicit
elsebranch within the suggestion guard.Suggested restructuring
// Add guided fallback suggestions when we could not understand if (fallbackTriggered || responseGenerator.getResponse().size() == 0) { responseGenerator = appendFallbackSuggestions(responseGenerator, textMessage); } - - // Fallback when everything else fails Eliza will answer - if (responseGenerator.getResponse().size() == 0) { - responseGenerator.addTextResponse(new ResponseData(helper.getEliza().processInput(textMessage))); - } return responseGenerator;If you want to keep Eliza as a last-resort safety net (defensive coding), at least add a comment explaining it's intentionally unreachable under current logic.
60-60: Nit: redundant== truecomparison.
Utility.isJSONObject(reply)already returns a boolean.- if (Utility.isJSONObject(reply) == true) { + if (Utility.isJSONObject(reply)) {
153-176: Hoist stop-word set to astatic finalfield; use imports instead of fully-qualified types.The
stopSetis rebuilt on every call with the same contents. Promoting it to a class-level constant avoids repeated allocation and makes the intent clearer. The inlinejava.util.Set/java.util.HashSetreferences should also use the existing import section.Suggested refactor
Add imports at the top of the file:
import java.util.Arrays; import java.util.HashSet; import java.util.Set;Add a static field in the class:
private static final Set<String> STOP_WORDS = new HashSet<>(Arrays.asList( "what", "who", "where", "why", "how", "is", "are", "the", "a", "an", "of", "in", "on", "for", "with", "to", "tell", "me", "about", "please" ));Then simplify
extractKeyword:private String extractKeyword(String text) { if (text == null || text.trim().length() == 0) { return null; } String cleaned = text.replaceAll("[^A-Za-z0-9 ]", " ").toLowerCase(); String[] tokens = cleaned.split(" +"); - String[] stop = new String[] { "what", "who", "where", "why", "how", "is", "are", "the", "a", "an", "of", "in", - "on", "for", "with", "to", "tell", "me", "about", "please" }; - java.util.Set<String> stopSet = new java.util.HashSet<>(); - for (String s : stop) - stopSet.add(s); - String best = null; for (String t : tokens) { if (t.length() < 3) continue; - if (stopSet.contains(t)) + if (STOP_WORDS.contains(t)) continue; if (best == null || t.length() > best.length()) { best = t; } } return best; }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/chatbot/lib/handlers/TextHandler.java`:
- Around line 81-105: Introduce a dedicated boolean elizaResponded (initialize
alongside fallbackTriggered) and set elizaResponded = true only in the Eliza
branch where you call helper.getEliza().processInput(...); keep
fallbackTriggered true for any fallback condition (both Eliza and
NLHandler-empty) but do NOT set elizaResponded when NLHandler returns zero
results; finally call appendFallbackSuggestions(responseGenerator, textMessage,
elizaResponded) so appendFallbackSuggestions can distinguish Eliza vs keyword
fallback; update references in TextHandler (the Eliza branch, the NLHandler
empty-result branch, and the final appendFallbackSuggestions call) accordingly.
🧹 Nitpick comments (2)
src/main/java/chatbot/lib/handlers/TextHandler.java (2)
59-60: Nit: redundant== truecomparison.
Utility.isJSONObject(reply)already returnsboolean.Suggested diff
- if (Utility.isJSONObject(reply) == true) { + if (Utility.isJSONObject(reply)) {
159-182: Stop-word set is rebuilt on every call; prefer a static constant and use imports.
extractKeywordallocates a newHashSetfrom an array literal on each invocation. Since the stop list is fixed, promote it to aprivate static final Set<String>. Also, the inline fully-qualifiedjava.util.Set/java.util.HashSetreferences should be proper imports at the top of the file.♻️ Suggested refactor
Add to imports:
import java.util.Arrays; import java.util.HashSet; import java.util.Set;Add a class-level constant:
private static final Set<String> STOP_WORDS = new HashSet<>(Arrays.asList( "what", "who", "where", "why", "how", "is", "are", "the", "a", "an", "of", "in", "on", "for", "with", "to", "tell", "me", "about", "please" ));Then simplify
extractKeyword:private String extractKeyword(String text) { if (text == null || text.trim().length() == 0) { return null; } String cleaned = text.replaceAll("[^A-Za-z0-9 ]", " ").toLowerCase(); String[] tokens = cleaned.split(" +"); - String[] stop = new String[] { "what", "who", "where", ... }; - java.util.Set<String> stopSet = new java.util.HashSet<>(); - for (String s : stop) - stopSet.add(s); String best = null; for (String t : tokens) { if (t.length() < 3) continue; - if (stopSet.contains(t)) + if (STOP_WORDS.contains(t)) continue; if (best == null || t.length() > best.length()) { best = t; } } return best; }
|
Greetings @RicardoUsbeck @ram-g-athreya , kindly review my pr whenever available , Thankyou. |
1 participant
Limit fallback smart replies to DBpedia topics for faster, more relevant suggestions.
Update smart-reply pill layout for better tap targets and accessibility.
Improve fallback messaging and keyword extraction logic.
No breaking changes.
Resolves #56
README.mdSummary by CodeRabbit
New Features
Improvements
Documentation