-
Notifications
You must be signed in to change notification settings - Fork 178
Feature/chatbot integration #215
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.
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
base: main
Are you sure you want to change the base?
Feature/chatbot integration #215
Conversation
📝 WalkthroughWalkthroughAdds a new Explorer component and route, updates Navbar navigation, refactors process-creation flow and CreateProcessPage state/validation, adds Terms & Conditions in AddCandidateModal, replaces the ChatBot remote API with a local intent matcher and exposes a Flask Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (5)
chatbot/intents.json (1)
1-1: Minor: Accidental leading whitespace on line 1.There's extraneous whitespace before the opening
{on line 1. This is valid JSON but inconsistent formatting that may have been introduced accidentally.Proposed fix
- { +{client/app/components/ChatBot/ChatBot.tsx (2)
57-74: Pattern matching may cause false positives with substring matches.Using
includes(pattern)can match unintended substrings. For example, the pattern"hi"would match"this"or"within", potentially returning a greeting when the user didn't intend one.Proposed fix using word boundaries
const getBotReply = (message: string): string => { const lowerMsg = message.toLowerCase(); for (const intent of intents) { for (const pattern of intent.patterns) { - if (lowerMsg.includes(pattern)) { + const regex = new RegExp(`\\b${pattern}\\b`, "i"); + if (regex.test(lowerMsg)) { return intent.responses[ Math.floor(Math.random() * intent.responses.length) ]; } } }
179-184: Minor: Add aria-label for accessibility.The input field lacks an accessible label. Screen readers won't be able to identify the input's purpose.
Proposed fix
<input value={inputMessage} onChange={(e) => setInputMessage(e.target.value)} placeholder="Type your message..." + aria-label="Chat message input" className="flex-1 px-3 py-2 rounded-full text-sm focus:outline-none" />anonymousVoting/clientAnonymousVoting/src/App.js (1)
21-45: Consider removing commented-out conditional routing code.Lines 21-32 and 43 contain commented-out routing logic that creates noise. If the
hasRegisteredconditional routing is no longer needed, consider removing the dead code entirely.anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (1)
30-30: Remove commented-out code.Dead code should be deleted rather than commented out. Version control preserves the history if you need to reference it later.
🔎 Proposed fix
- // const refValue = useRef(false); -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
anonymousVoting/clientAnonymousVoting/public/assets/explorer.pngis excluded by!**/*.pnganonymousVoting/clientAnonymousVoting/public/assets/image.pngis excluded by!**/*.pngclient/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
anonymousVoting/clientAnonymousVoting/src/App.jsanonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.jsanonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.jsanonymousVoting/clientAnonymousVoting/src/components/Explorer.jsanonymousVoting/clientAnonymousVoting/src/components/Navbar.jsanonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.jschatbot/app.pychatbot/intents.jsonclient/app/components/ChatBot/ChatBot.tsx
🧰 Additional context used
🪛 Ruff (0.14.10)
chatbot/app.py
90-90: Possible binding to all interfaces
(S104)
90-90: Use of debug=True in Flask app detected
(S201)
🔇 Additional comments (10)
client/app/components/ChatBot/ChatBot.tsx (2)
21-54: Discrepancy: PR objectives mention API integration, but implementation is rule-based.The linked issue #148 specifies using "a generative language API to generate real-time responses" with "API integration with proper environment configuration." This implementation uses hardcoded local intents instead, which diverges from the requirements.
Additionally, these intents duplicate content from
chatbot/intents.jsonbut with a reduced subset. Consider either:
- Implementing API integration as per requirements, or
- Importing/sharing the intents from a single source to avoid drift.
77-114: LGTM on component state management and submission flow.The state management is clean, the typing indicator provides good UX feedback, and the 800ms delay simulates realistic bot response time.
anonymousVoting/clientAnonymousVoting/src/App.js (1)
2-2: LGTM on Explorer routing.The Explorer component import and route configuration are correctly implemented. The route placement before the root redirect ensures proper matching.
Also applies to: 35-35
anonymousVoting/clientAnonymousVoting/src/components/Navbar.js (2)
16-20: Settings icon is non-interactive—intentional?The settings icon is no longer wrapped in a
<Link>, making it non-clickable. If this is intentional (placeholder for future functionality), consider adding atitleoraria-labelto indicate it's disabled, or style it to appear inactive.
22-28: The explorer icon asset is present atanonymousVoting/clientAnonymousVoting/public/assets/explorer.png, so the image reference in the code will resolve correctly.anonymousVoting/clientAnonymousVoting/src/components/Explorer.js (1)
1-10: LGTM as a placeholder component.Simple and effective placeholder for the Explorer feature. The component correctly follows React conventions and exports as default for the route configuration.
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js (1)
17-18: LGTM on Terms & Conditions implementation.The T&C acceptance flow is well-implemented: state properly initialized, reset on close/success, enforced in submit handler, and button disabled until accepted.
Also applies to: 121-145
anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js (3)
8-9: LGTM: Clean import refactoring.The removal of unused test-related imports improves code clarity.
114-131: LGTM: Excellent pending state handling.The button properly disables during transaction submission and provides clear visual feedback with the spinner and "Creating..." label.
40-60: Function parameter mismatch: arguments do not align withdeployVotingProcesssignature.The
deployVotingProcessfunction atcontracts.js:12expects exactly 4 parameters:(name, description, startDate, endDate). However, the call atCreateProcessPage.js:44-50passes 5 arguments where the 3rd, 4th, and 5th parameters areproposalArray,1000000, and1000000.This causes:
proposalArray(bytes array) to be passed asstartDate(should be a timestamp)- First
1000000to be passed asendDate(should be a timestamp)- Second
1000000to be ignored as an extra argumentThe voting process will be created with incorrect start/end dates. Pass the correct date values (timestamps) and verify that
proposalArrayis handled appropriately by the contract method.Likely an incorrect or invalid review comment.
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.js
Outdated
Show resolved
Hide resolved
| const isFormValid = () => { | ||
| if(proposals < 2) | ||
| return false; | ||
| return true; | ||
| } | ||
| return proposals.split(',').length >= 2; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation doesn't reject empty proposals after trimming.
The validation only counts commas but doesn't verify that trimmed proposals are non-empty. Input like "proposal1, , , proposal2" or " , , " would pass validation but result in empty proposal strings.
Filter and validate trimmed proposals.
🔎 Proposed fix
const isFormValid = () => {
- return proposals.split(',').length >= 2;
+ const trimmedProposals = proposals.split(',').map(p => p.trim()).filter(p => p.length > 0);
+ return trimmedProposals.length >= 2;
};📝 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.
| const isFormValid = () => { | |
| if(proposals < 2) | |
| return false; | |
| return true; | |
| } | |
| return proposals.split(',').length >= 2; | |
| }; | |
| const isFormValid = () => { | |
| const trimmedProposals = proposals.split(',').map(p => p.trim()).filter(p => p.length > 0); | |
| return trimmedProposals.length >= 2; | |
| }; |
🤖 Prompt for AI Agents
In anonymousVoting/clientAnonymousVoting/src/components/CreateProcessPage.js
around lines 64 to 66, the isFormValid function currently counts commas and can
treat empty or whitespace-only entries as valid; change the validation to split
proposals on commas, map over parts and trim each one, filter out any empty
strings, and then require that the resulting array has length >= 2 so only
non-empty trimmed proposals count as valid.
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
Show resolved
Hide resolved
anonymousVoting/clientAnonymousVoting/src/components/modals/AddCandidateModal.js
Outdated
Show resolved
Hide resolved
chatbot/app.py
Outdated
| if __name__ == '__main__': | ||
| app.run(host="0.0.0.0", port=5000,debug=True) | ||
| app.run(host="0.0.0.0", port=5000,debug=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Avoid debug=True and binding to all interfaces in production.
Running Flask with debug=True enables the interactive debugger, which can execute arbitrary code. Combined with host="0.0.0.0", this exposes the debugger to the network.
Proposed fix using environment variables
+import os
+
if __name__ == '__main__':
- app.run(host="0.0.0.0", port=5000,debug=True)
+ debug_mode = os.environ.get("FLASK_DEBUG", "false").lower() == "true"
+ host = os.environ.get("FLASK_HOST", "127.0.0.1")
+ port = int(os.environ.get("FLASK_PORT", 5000))
+ app.run(host=host, port=port, debug=debug_mode)🧰 Tools
🪛 ast-grep (0.40.3)
[warning] 89-89: Detected Flask app with debug=True. Do not deploy to production with this flag enabled as it will leak sensitive information. Instead, consider using Flask configuration variables or setting 'debug' using system environment variables.
Context: app.run(host="0.0.0.0", port=5000,debug=True)
Note: [CWE-489] Active Debug Code. [REFERENCES]
- https://labs.detectify.com/2015/10/02/how-patreon-got-hacked-publicly-exposed-werkzeug-debugger/
(debug-enabled-python)
[warning] 89-89: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(host="0.0.0.0", port=5000,debug=True)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control
(avoid_app_run_with_bad_host-python)
🪛 Ruff (0.14.10)
90-90: Possible binding to all interfaces
(S104)
90-90: Use of debug=True in Flask app detected
(S201)
🤖 Prompt for AI Agents
In chatbot/app.py around lines 89-90, running Flask with debug=True and
host="0.0.0.0" is unsafe for production; change the startup to read an
environment/config flag (e.g., FLASK_ENV or APP_ENV) and only set debug=True and
open bind when that flag equals "development"; otherwise set debug=False and
bind to 127.0.0.1 (or better, do not call app.run at all in production and run
under a WSGI server like gunicorn/uwsgi). Ensure the environment-controlled
logic avoids exposing the interactive debugger and restricting network exposure
in non-development environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.js (1)
20-20: Consider removing commented-out code.This commented-out state declaration appears to be unused dead code.
🔎 Suggested cleanup
- // const [correctNetwork2, setCorrectNetwork2] = useState(true); -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.js
🔇 Additional comments (1)
anonymousVoting/clientAnonymousVoting/src/components/ConnectWallet.js (1)
7-7: Previous critical issue resolved.The import typo (
seColectCorrectNetwork→selectCorrectNetwork) flagged in the previous review has been corrected. The import now matches the usage on line 14.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
chatbot/app.py
🧰 Additional context used
🧬 Code graph analysis (1)
chatbot/app.py (1)
chatbot/train.py (1)
bag_of_words(19-21)
🪛 ast-grep (0.40.3)
chatbot/app.py
[warning] 89-89: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(host="0.0.0.0", port=5000, debug=debug_mode)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control
(avoid_app_run_with_bad_host-python)
🪛 Ruff (0.14.10)
chatbot/app.py
90-90: Possible binding to all interfaces
(S104)
🔇 Additional comments (4)
chatbot/app.py (4)
5-5: LGTM: Import added for environment variable access.The
osimport is correctly used at line 89 to read the Flask debug mode from an environment variable.
44-44: LGTM: Security improvement withweights_only=True.Setting
weights_only=Trueprevents arbitrary code execution when loading model files, which is a security best practice when deserializing PyTorch data.
55-55: LGTM: Essential call to set model in evaluation mode.Calling
model.eval()disables training-specific behaviors like dropout and ensures correct inference. This is a necessary addition for the chatbot endpoint.
66-66: LGTM: Consistent variable usage.Using the top-level
all_wordsandtagsvariables (extracted at lines 49-50) is consistent and improves code readability compared to nested dictionary access.Also applies to: 70-70
| debug_mode = os.getenv("FLASK_DEBUG", "false").lower() == "true" | ||
| app.run(host="0.0.0.0", port=5000, debug=debug_mode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete security fix: Host should also be configurable.
Good progress making debug mode environment-driven, but the host remains hardcoded to "0.0.0.0", which binds to all network interfaces and can expose the application publicly. The past review recommended making the host configurable as well.
🔎 Complete the security fix by making host configurable
if __name__ == '__main__':
debug_mode = os.getenv("FLASK_DEBUG", "false").lower() == "true"
- app.run(host="0.0.0.0", port=5000, debug=debug_mode)
+ host = os.getenv("FLASK_HOST", "127.0.0.1")
+ port = int(os.getenv("FLASK_PORT", "5000"))
+ app.run(host=host, port=port, debug=debug_mode)This defaults to 127.0.0.1 (localhost), restricting access to the local machine, and allows production deployments to override via environment variables when needed.
Based on past review and static analysis tools.
🧰 Tools
🪛 ast-grep (0.40.3)
[warning] 89-89: Running flask app with host 0.0.0.0 could expose the server publicly.
Context: app.run(host="0.0.0.0", port=5000, debug=debug_mode)
Note: [CWE-668]: Exposure of Resource to Wrong Sphere [OWASP A01:2021]: Broken Access Control [REFERENCES]
https://owasp.org/Top10/A01_2021-Broken_Access_Control
(avoid_app_run_with_bad_host-python)
🪛 Ruff (0.14.10)
90-90: Possible binding to all interfaces
(S104)
🤖 Prompt for AI Agents
In chatbot/app.py around lines 89 to 90, the host is hardcoded to "0.0.0.0"
which exposes the app to all network interfaces; change to read a HOST env var
with a safe default of "127.0.0.1" (e.g., host = os.getenv("FLASK_HOST",
"127.0.0.1")), then pass that host variable to app.run alongside the existing
debug_mode so deployments can override the host while defaulting to localhost.
Description
This PR adds a frontend chatbot component to the Agora client to improve user interaction and onboarding. The chatbot provides instant, intent-based responses for common voting and blockchain-related queries, enhancing usability without introducing backend dependencies.
The component is implemented as a reusable client-side module with smooth animations, typing indicators, and a minimal UI footprint.
Fixes #148
Type of change
Updated UI/UX
Added new feature
Improved the business logic of code
Other
Checklist
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Summary by CodeRabbit
New Features
Bug Fixes / UX
Style / Cleanup
✏️ Tip: You can customize this high-level summary in your review settings.