AI-207: Add Smart Suggestion Engine for MCP Tools - #297
Conversation
|
Hi , could you attach a video demo or ss as proof of working |
|
Also update your tools in the readme and add them in registry.py |
|
Hi @gyanLM10, thank you for the feedback! I have implemented the Smart Suggestion Engine and tested it locally. screenshot 1:https://drive.google.com/file/d/1s8-hlziwlDSdFS8JR6WyMM7gRrB7Y5j0/view?usp=drive_link The suggestion engine generates context-aware next-step recommendations I will now proceed to:
Please let me know if any further improvements are needed. |
|
Hi @gyanLM10, Thanks for the feedback! I will:
Also, I would appreciate your feedback on extending this suggestion engine to savings and client domains. Thanks! |
|
Since this repository is intended as an MCP server for AI agents, it would be better to avoid including the Suggestion Engine here. The MCP layer should focus on exposing data and tool schemas, while the decision-making can be handled by the LLM. Adding hard-coded suggestions in tool responses may also increase context size unnecessarily. It might be cleaner to keep this layer focused on data and tool execution and move the suggestion logic elsewhere. What do you think @IOhacker ? |
|
Thank you for the detailed feedback — that makes a lot of sense. I agree that the MCP layer should remain focused on exposing tools and data, keeping it lightweight and clean. I understand that adding hard-coded suggestions may increase context size and is not aligned with the core responsibility of MCP. I will adjust my approach accordingly and explore shifting the suggestion logic to the LLM layer or a higher-level agent layer, where decision-making fits more naturally. Thanks again for the guidance — I’ll update the implementation and keep the architecture aligned with MCP principles. |
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
@KAishwarya2429 add this ticket in the PR title https://mifosforge.jira.com/browse/AI-207
KAishwarya2429
changed the title
|
Hi @IOhacker, Updated the PR title to include the Jira ticket (AI-207). Thanks! |
|
Hi, I went through this PR — really like the approach of generating context-aware suggestions after tool execution. I recently worked on a query-based suggestion layer (AI-207), which focuses on suggesting relevant MCP tools before execution based on user intent. It seems both approaches operate at different stages:
I think these could complement each other nicely as a two-stage flow. Happy to contribute here — I can help with:
Let me know what would be most useful. |
|
Hi, I explored this PR and added a small set of improvements focused on reliability and safety. This includes:
All changes are minimal and additive, without modifying the core approach. Happy to adjust or refine based on feedback. |
|
Thanks again for the detailed suggestions — I really like the idea of combining query-based and response-based flows. At the moment, I’m aligning the PR with mentor feedback by keeping the MCP layer focused on data and moving suggestion logic to an external/agent layer. Once that is updated, I’d be happy to collaborate further on improvements like test coverage and structured outputs. Really appreciate your contributions! |
…l as per review feedback
|
Hi @IOhacker and reviewers, Thank you for the valuable feedback. I have updated the PR based on the suggestions: ✅ Removed suggestion logic from MCP tool responses to keep the MCP layer clean and focused on data execution This ensures proper separation of concerns between the MCP server (execution layer) and AI decision-making. Please let me know if any further refinements are needed. Thanks again! |
|
Hi @IOhacker, Thanks for the review and suggestions. I’ve addressed the requested changes:
Please let me know if anything else needs improvement. Thanks! |
|
Please review the earlier comments |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA suggestion engine feature has been introduced across the codebase. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP Server
participant Suggestion Engine
participant Registry
Client->>MCP Server: get_loan(loanId)
MCP Server->>MCP Server: Retrieve loan details
MCP Server->>Suggestion Engine: generate_suggestions("get_loan_details", response)
Suggestion Engine->>Suggestion Engine: Evaluate loan status
Suggestion Engine-->>MCP Server: Return suggestions list
MCP Server-->>Client: Return {data: loan_details, suggestions: [...]}
Client->>MCP Server: get_overdue_loans_for_client(clientId)
MCP Server->>MCP Server: Fetch overdue loans
MCP Server->>Suggestion Engine: generate_suggestions("get_overdue_loans", loans)
Suggestion Engine->>Suggestion Engine: Iterate loans, extract IDs
Suggestion Engine-->>MCP Server: Return suggestions list
MCP Server-->>Client: Return enhanced response with suggestions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/README.md (1)
234-235:⚠️ Potential issue | 🟡 MinorAdd
suggestion_engine.pyto the project structure.The README introduces
core/suggestion_engine.py, but the tree still shows onlyapi_server.pyundercore.Proposed fix
└── core/ - └── api_server.py # Optional HTTP/SSE transport layer + ├── api_server.py # Optional HTTP/SSE transport layer + └── suggestion_engine.py # Experimental client/LLM suggestion helperAlso applies to: 383-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/README.md` around lines 234 - 235, The README's project tree under core is missing the new file referenced in the text; add "suggestion_engine.py" to the core tree entry so it lists both "api_server.py" and "suggestion_engine.py" (update the block showing "└── core/" to include a "└── suggestion_engine.py # suggestion engine" line), and make the same insertion for the second occurrence mentioned (around the other tree at lines referenced in the comment) so the README's file list matches the described content.
🧹 Nitpick comments (3)
python/core/suggestion_engine.py (2)
1-63: Move suggestion rules out of the core transport package.This module contains business/UX decision logic, not MCP protocol, stream parsing, or connection lifecycle code. Move it to an experimental client/helper package outside
python/core, or keep it entirely in the client/LLM layer.As per coding guidelines, “Code in this folder must handle the MCP protocol, stream parsing, and connection lifecycles.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/core/suggestion_engine.py` around lines 1 - 63, The generate_suggestions function in python/core/suggestion_engine.py contains business/UX logic that doesn't belong in the core transport package; move the entire module (including generate_suggestions and its helper logic) out of python/core into an experimental client/helper package or the client/LLM layer where UX/business rules live, update any import sites to point to the new module location, and ensure tests and package exports are adjusted accordingly so python/core only contains MCP protocol, stream parsing, and connection lifecycle code.
23-34: Cap and deduplicate overdue-loan suggestions.This generates three suggestions per overdue loan with no upper bound. Large overdue lists can bloat downstream LLM context; track seen loan IDs and apply a small max.
Proposed fix
+MAX_OVERDUE_SUGGESTION_LOANS = 5 + def generate_suggestions(intent: str, data: Any) -> List[str]: @@ - for loan in loans: + seen_loan_ids = set() + for loan in loans: if not isinstance(loan, dict): continue loan_id = loan.get("loanId") or loan.get("id") - if not loan_id: + if not loan_id or loan_id in seen_loan_ids: continue + + if len(seen_loan_ids) >= MAX_OVERDUE_SUGGESTION_LOANS: + break + + seen_loan_ids.add(loan_id) suggestions.append(f"Apply a late fee to loan {loan_id}") suggestions.append(f"View repayment schedule for loan {loan_id}") suggestions.append(f"Send a repayment reminder for loan {loan_id}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/core/suggestion_engine.py` around lines 23 - 34, The loop that builds suggestions from loans should deduplicate by loan ID and stop once a small cap is reached to avoid unbounded output; add a seen set (e.g., seen_loan_ids) and a MAX_SUGGESTIONS constant, skip any loan whose loan_id is already in seen_loan_ids, add loan_id to seen_loan_ids when you append its three suggestions, and break out (or stop adding) when len(suggestions) >= MAX_SUGGESTIONS; refer to the variables/collections loans, suggestions, loan_id and update that loop to enforce the dedupe+cap logic.python/tools/registry.py (1)
58-75: Remove suggestion-engine coupling from the registry.
DomainRegistryshould route domains/tools. Importingcore.suggestion_engineand exposingget_suggestions()keeps suggestion decision logic in the MCP registry instead of the client/LLM layer.Proposed fix
-# ✅ AI Suggestion Engine (New Contribution) -from core.suggestion_engine import generate_suggestions - @@ - NOTE: - ----- - The Suggestion Engine is NOT registered as a tool because: - - It does not call Fineract APIs directly - - It enhances responses AFTER tool execution - - It is used as a helper layer inside MCP server - - This maintains clean separation of concerns: - - Tools → Data operations - - Suggestion Engine → UX enhancement layer @@ - # ✅ NEW HELPER METHOD - def get_suggestions(self, intent: str, data): - """ - Generate intelligent suggestions based on tool output. - - This enhances MCP responses by guiding users with next actions. - """ - return generate_suggestions(intent, data) -As per coding guidelines, “Ensure
mcp_server.py,mcp_adapter.py, andregistry.pyproperly decouple the transport protocol from the business schemas.”Also applies to: 184-191
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/tools/registry.py` around lines 58 - 75, Remove the direct coupling to the suggestion engine from DomainRegistry: delete the import of core.suggestion_engine and any uses/exposure of generate_suggestions or get_suggestions inside the DomainRegistry class so the registry only maps domains to tool identifiers; instead add a simple hook/signature (e.g., a placeholder method or config entry) that allows mcp_server.py or mcp_adapter.py to call the suggestion engine externally, and move any suggestion decision logic out of registry.py (also remove similar references around the other occurrences noted ~184-191) so transport/business schema responsibilities remain decoupled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/core/suggestion_engine.py`:
- Around line 1-6: The function generate_suggestions is missing mypy-compatible
annotations and references List without importing it; update the signature to
include concrete types (e.g., intent: str, data: Dict[str, Any]) and a return
type of List[str], and add the necessary typing imports (from typing import
List, Dict, Any) at the top of the file so the variable suggestions: List[str]
and the function signature are fully typed for mypy.
In `@python/mcp_server.py`:
- Around line 318-325: The current get_loan handler calls an undefined
generate_suggestions and wraps the loan payload into {"data", "suggestions"},
changing the expected MCP schema and causing a NameError; revert get_loan to
return the original loan response object directly (remove the
generate_suggestions call and the {"data","suggestions"} wrapper) so the
top-level loan fields remain unchanged, and do not import or reference
generate_suggestions in mcp_server.py; also ensure corresponding
adapters/registry code (e.g., mcp_adapter and registry integration points)
remain decoupled from any transport-level "suggestions" envelope so business
schemas stay unchanged.
- Around line 430-445: The function get_overdue_loans_for_client should not
early-return before building suggestions: remove the premature return of
get_overdue_loans.func(clientId), instead call and assign its output to a
clearly typed variable (e.g., result = get_overdue_loans.func(clientId)), then
call suggestions = generate_suggestions("get_overdue_loans", result) and return
a merged dict/object that includes the original payload and "suggestions"; also
update the function signature return type from -> list to a mypy-compatible type
matching the overdue-loans payload (e.g., Dict[str, Any] or a defined TypedDict/
dataclass) and add appropriate type hints for result and suggestions.
In `@python/tools/registry.py`:
- Around line 156-157: The current keyword check in registry.py only looks for
["journal", "ledger", "accounting"] so queries like "list GL accounts" or "chart
of accounts" don't match; update the condition that inspects query (the code
using variable query and calling active_domains.add("accounting")) to include
additional accounting keywords such as "gl", "gl account", "gl accounts",
"general ledger", "chart of accounts", and "chart of account" (keeping the check
case-insensitive) so those queries are routed to the accounting domain.
---
Outside diff comments:
In `@python/README.md`:
- Around line 234-235: The README's project tree under core is missing the new
file referenced in the text; add "suggestion_engine.py" to the core tree entry
so it lists both "api_server.py" and "suggestion_engine.py" (update the block
showing "└── core/" to include a "└── suggestion_engine.py # suggestion
engine" line), and make the same insertion for the second occurrence mentioned
(around the other tree at lines referenced in the comment) so the README's file
list matches the described content.
---
Nitpick comments:
In `@python/core/suggestion_engine.py`:
- Around line 1-63: The generate_suggestions function in
python/core/suggestion_engine.py contains business/UX logic that doesn't belong
in the core transport package; move the entire module (including
generate_suggestions and its helper logic) out of python/core into an
experimental client/helper package or the client/LLM layer where UX/business
rules live, update any import sites to point to the new module location, and
ensure tests and package exports are adjusted accordingly so python/core only
contains MCP protocol, stream parsing, and connection lifecycle code.
- Around line 23-34: The loop that builds suggestions from loans should
deduplicate by loan ID and stop once a small cap is reached to avoid unbounded
output; add a seen set (e.g., seen_loan_ids) and a MAX_SUGGESTIONS constant,
skip any loan whose loan_id is already in seen_loan_ids, add loan_id to
seen_loan_ids when you append its three suggestions, and break out (or stop
adding) when len(suggestions) >= MAX_SUGGESTIONS; refer to the
variables/collections loans, suggestions, loan_id and update that loop to
enforce the dedupe+cap logic.
In `@python/tools/registry.py`:
- Around line 58-75: Remove the direct coupling to the suggestion engine from
DomainRegistry: delete the import of core.suggestion_engine and any
uses/exposure of generate_suggestions or get_suggestions inside the
DomainRegistry class so the registry only maps domains to tool identifiers;
instead add a simple hook/signature (e.g., a placeholder method or config entry)
that allows mcp_server.py or mcp_adapter.py to call the suggestion engine
externally, and move any suggestion decision logic out of registry.py (also
remove similar references around the other occurrences noted ~184-191) so
transport/business schema responsibilities remain decoupled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 48afe7cc-9537-4770-b1dc-8e6368e98444
📒 Files selected for processing (4)
python/README.mdpython/core/suggestion_engine.pypython/mcp_server.pypython/tools/registry.py
| def generate_suggestions(intent, data): | ||
| """ | ||
| Generate context-aware suggestions based on MCP tool responses. | ||
| """ | ||
|
|
||
| suggestions: List[str] = [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add mypy-compatible type annotations.
generate_suggestions has untyped parameters/return, and List is referenced without an import. Add explicit types and import the typing symbols used.
Proposed fix
+from typing import Any, List
+
-def generate_suggestions(intent, data):
+def generate_suggestions(intent: str, data: Any) -> List[str]:
"""
Generate context-aware suggestions based on MCP tool responses.
"""
suggestions: List[str] = []As per coding guidelines, “Type Safety: Flag any new function signatures, complex variable assignments, or class attributes that are missing mypy-compatible type hints.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/core/suggestion_engine.py` around lines 1 - 6, The function
generate_suggestions is missing mypy-compatible annotations and references List
without importing it; update the signature to include concrete types (e.g.,
intent: str, data: Dict[str, Any]) and a return type of List[str], and add the
necessary typing imports (from typing import List, Dict, Any) at the top of the
file so the variable suggestions: List[str] and the function signature are fully
typed for mypy.
| # 🔹 Step 2: Generate suggestions | ||
| suggestions = generate_suggestions("get_loan_details", response) | ||
|
|
||
| # 🔹 Step 3: Return enhanced response | ||
| return { | ||
| "data": response, | ||
| "suggestions": suggestions | ||
| } |
There was a problem hiding this comment.
Do not wrap loan responses with server-side suggestions.
generate_suggestions is not imported in this file, so get_loan raises NameError. This also changes the tool contract from top-level loan fields to {data, suggestions} despite the documented MCP-only execution scope.
Proposed fix
- # 🔹 Step 2: Generate suggestions
- suggestions = generate_suggestions("get_loan_details", response)
-
- # 🔹 Step 3: Return enhanced response
- return {
- "data": response,
- "suggestions": suggestions
- }
+ return responseAs per coding guidelines, “Ensure mcp_server.py, mcp_adapter.py, and registry.py properly decouple the transport protocol from the business schemas.”
📝 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.
| # 🔹 Step 2: Generate suggestions | |
| suggestions = generate_suggestions("get_loan_details", response) | |
| # 🔹 Step 3: Return enhanced response | |
| return { | |
| "data": response, | |
| "suggestions": suggestions | |
| } | |
| return response |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/mcp_server.py` around lines 318 - 325, The current get_loan handler
calls an undefined generate_suggestions and wraps the loan payload into {"data",
"suggestions"}, changing the expected MCP schema and causing a NameError; revert
get_loan to return the original loan response object directly (remove the
generate_suggestions call and the {"data","suggestions"} wrapper) so the
top-level loan fields remain unchanged, and do not import or reference
generate_suggestions in mcp_server.py; also ensure corresponding
adapters/registry code (e.g., mcp_adapter and registry integration points)
remain decoupled from any transport-level "suggestions" envelope so business
schemas stay unchanged.
| if any(w in query for w in ["journal", "ledger", "accounting"]): | ||
| active_domains.add("accounting") |
There was a problem hiding this comment.
Route GL account queries to accounting tools.
After narrowing the keywords, queries like “list GL accounts” or “chart of accounts” do not match accounting and fall back to clients.
Proposed fix
- if any(w in query for w in ["journal", "ledger", "accounting"]):
+ if any(w in query for w in ["journal", "ledger", "accounting", "gl account", "general ledger", "chart of accounts"]):
active_domains.add("accounting")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/tools/registry.py` around lines 156 - 157, The current keyword check
in registry.py only looks for ["journal", "ledger", "accounting"] so queries
like "list GL accounts" or "chart of accounts" don't match; update the condition
that inspects query (the code using variable query and calling
active_domains.add("accounting")) to include additional accounting keywords such
as "gl", "gl account", "gl accounts", "general ledger", "chart of accounts", and
"chart of account" (keeping the check case-insensitive) so those queries are
routed to the accounting domain.
5 participants
This PR introduces a Smart Suggestion Engine that enhances MCP tools
by providing context-aware next-step recommendations based on response data.
Key Features:
suggestion_engine.pyfor reusable suggestion logicExample:
After fetching overdue loans, users receive suggestions such as:
Impact:
Summary by CodeRabbit
New Features
Documentation