feat: payment-aware RAG routing with SKALE integration#496
feat: payment-aware RAG routing with SKALE integration#496Subhajitdas99 wants to merge 22 commits intoGetBindu:mainfrom
Conversation
…LE docs clarification)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new RAG router agent example (intent classification, source selection free/premium, file-backed retrieval, SKALE facilitator reachability checks, optional OpenRouter LLM synthesis) and replaces the prior X402 payment guide in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Handler as Agent Handler
participant Router as Intent Router
participant SourceRouter as Source Router
participant Facilitator as SKALE Facilitator
participant Retriever as Doc Retriever
participant DomainAgent as Domain Agent
participant LLM as OpenRouter LLM
User->>Handler: handler(messages)
Handler->>Router: classify_intent(query)
Router-->>Handler: intent
Handler->>SourceRouter: select_source(intent, query)
SourceRouter-->>Handler: source {path,type}
alt premium source selected
Handler->>Facilitator: call_skale_facilitator()
Facilitator-->>Handler: {status,code}
alt payment failed
Handler-->>User: error response
end
end
Handler->>Retriever: retrieve_docs(db_path, query)
Retriever-->>Handler: top-k documents
alt documents found
Handler->>Router: route_agent(intent)
Router-->>Handler: domain_agent
Handler->>DomainAgent: domain_agent(query, context)
DomainAgent-->>Handler: domain response
alt LLM available
Handler->>LLM: agent.run(query, context)
LLM-->>Handler: refined response
end
Handler-->>User: JSON response
else no documents
Handler-->>User: error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
examples/rag_router_agent/test_local.py (1)
1-23: Rename to avoid pytest auto-collection.Files matching
test_*.pyare auto-discovered by pytest. This script is a manual smoke-test runner (depends onagentbeing importable from CWD and performs network/LLM calls viahandler), not a unit test. If CI ever runspytest examples/, this file will be collected, and although it defines notest_*functions (so nothing executes), having a non-test script under thetest_prefix is misleading. Consider renaming to e.g.run_local.pyorsmoke_local.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/test_local.py` around lines 1 - 23, This file is named with the pytest-discovered prefix `test_` but is a manual smoke-test runner; rename the file (e.g., to run_local.py or smoke_local.py) so pytest won't auto-collect it, keep the existing run_test() function and the if __name__ == "__main__": run_test() guard unchanged, and ensure any README or developer doc that references this script updates the new filename; the script should continue to import handler from agent and invoke handler([{"role":"user","content": q}]) as it does now.examples/rag_router_agent/skale_payment.py (1)
34-37: Narrow the exception and avoid leaking raw error strings.
except Exceptionwill also swallow things likeKeyboardInterruptsubclasses from user code and masks programming errors. Catchrequests.RequestException(the base for network/timeout/SSL errors) instead. Returningstr(e)verbatim can also surface internal URLs or stack details into API responses; consider logging the exception and returning a generic message.♻️ Proposed fix
- except Exception as e: - return { - "status": "error", - "message": str(e) - } + except requests.RequestException as e: + return { + "status": "error", + "message": f"facilitator request failed: {type(e).__name__}", + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/skale_payment.py` around lines 34 - 37, Narrow the broad except and avoid returning raw error strings: replace the current except Exception block with an except requests.RequestException (import requests if not present), log the caught exception (including e) to your logger, and return a generic error payload keeping the same keys ("status": "error", "message": "<generic user-facing message>") so internal URLs/stack traces are not leaked; ensure other exceptions propagate normally (so KeyboardInterrupt and programming errors aren't swallowed).examples/rag_router_agent/retriever.py (2)
5-10: Narrowexceptmay let other IO errors surface unfiltered.Only
FileNotFoundErroris handled;PermissionError,IsADirectoryError, andUnicodeDecodeError(possible if a DB file isn't valid UTF-8) will bubble out ofretrieve_docsand crash the handler inagent.py. Consider widening toOSError(and optionallyUnicodeDecodeError) and returning[]with a log line, consistent with the graceful-degradation pattern used elsewhere in the agent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/retriever.py` around lines 5 - 10, In retrieve_docs, broaden the exception handling so other IO errors don't crash the agent: catch OSError (which covers FileNotFoundError, PermissionError, IsADirectoryError, etc.) and also catch UnicodeDecodeError when opening/reading db_path, log a warning via the module's logger (or processLogger used elsewhere) with the error details, and return an empty list []; update the except block around the open/read in retriever.py accordingly so agent.py continues graceful degradation.
13-29: Token-intersection scoring over-weights stopwords.
query_words & doc_wordstreats every shared token equally, so high-frequency words likewhat,is,the,newdominate ranking and can surface unrelated lines. Consider dropping a basic stopword set (or switching to TF-style weighting) so domain tokens likegst,reconciliationdrive the score. Not a blocker for the demo, but premium vs free ranking will be noisy otherwise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/retriever.py` around lines 13 - 29, The current token-intersection scoring (query_words & doc_words computed with re.findall) over-weights common stopwords; update the scoring in retriever.py by filtering out a small STOPWORDS set before forming query_words and doc_words (i.e., define STOPWORDS and replace query_words = set(re.findall(...)) and doc_words = set(re.findall(...)) with filtered sets) so score = len(query_words & doc_words) reflects domain tokens; alternatively, switch scoring to simple TF-style weighting by counting token frequencies from re.findall and computing sum(min(tf_query[token], tf_doc[token]) for token in shared_tokens) before appending to scored and sorting by score.examples/rag_router_agent/agent.py (2)
77-77:payment_resultcan only be a dict here, but guard againstNonefor safety.If
call_skale_facilitatorever returnsNone(e.g., future refactor),payment_result.get(...)raisesAttributeError. Tiny defensive tweak:- if payment_result.get("status") not in ["success", "reachable"]: + if not payment_result or payment_result.get("status") not in ("success", "reachable"):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/agent.py` at line 77, The current check uses payment_result.get(...) which will raise if payment_result is None; update the guard around the call handling (where call_skale_facilitator is used and payment_result is assigned) to defensively ensure payment_result is not None (e.g., check if payment_result is truthy or is a dict) before calling .get, then treat a None result as a failure path (same behavior as non-success/non-reachable statuses). Reference payment_result and the call_skale_facilitator call site when adding the None-safe conditional.
116-121: Swallowed agent-execution errors lose the stack trace.
except Exception+print(...)hides the root cause (ruff BLE001). For an example this is fine, but in practice you'll wantlogging.exception(...)so failures in a domain agent are diagnosable. Same applies to the LLM try/except at lines 144–146. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/agent.py` around lines 116 - 121, The except block around agent_fn(query, context) currently prints the error and swallows the stack trace; replace print(...) with logging.exception(...) to log the full stack and error context while still setting agent_response fallback, and do the same for the LLM call's try/except (the block handling the LLM response) so both agent_fn and the LLM exceptions use logging.exception(...) instead of print/quiet catches; reference the agent_fn, agent_response, query, context symbols and the LLM call site (the function/variable used to invoke the model) when making the changes.examples/rag_router_agent/router.py (1)
8-10:sys.pathmanipulation + absoluteagents.*imports are fragile.The
sys.path.insert(0, BASE_DIR)dance plus the post-definitionfrom agents.finance_agent import finance_agentpattern will break if this module is imported from a different CWD or via a different package alias (e.g., when someone installs the example as a package, or runs tests from the repo root). Sinceagent.pyalready handles both relative and script-mode imports with atry/except ImportError, applying the same pattern here would remove the need to mutatesys.pathglobally.♻️ Suggested pattern
-import os -import re -import sys - -# 📁 Resolve base directory safely -BASE_DIR = os.path.dirname(os.path.abspath(__file__)) - -# (Optional but safer import handling) -if BASE_DIR not in sys.path: - sys.path.insert(0, BASE_DIR) +import os +import re + +BASE_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -from agents.finance_agent import finance_agent -from agents.legal_agent import legal_agent -from agents.tech_agent import tech_agent +try: + from .agents.finance_agent import finance_agent + from .agents.legal_agent import legal_agent + from .agents.tech_agent import tech_agent +except ImportError: + from agents.finance_agent import finance_agent + from agents.legal_agent import legal_agent + from agents.tech_agent import tech_agentAlso applies to: 34-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/router.py` around lines 8 - 10, Remove the fragile global sys.path manipulation (the BASE_DIR check and sys.path.insert call) and replace the absolute imports like from agents.finance_agent import finance_agent with a try/except ImportError pattern (as used in agent.py) that first attempts the absolute import and falls back to a relative import (or vice versa) so the module works when run as a script or imported as a package; update the same pattern for the other absolute imports around the block identified (the imports at/near lines 34-37) and ensure you reference BASE_DIR only if still needed locally, not to mutate sys.path globally.examples/rag_router_agent/source_router.py (1)
57-58: Dead fallback branch.
sourcesis already guaranteed non-empty by the early return at line 38–39, sosources[0] if sources else Noneis equivalent toreturn sources[0]. Minor cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/source_router.py` around lines 57 - 58, The final fallback uses a redundant conditional: replace the expression "return sources[0] if sources else None" with a direct "return sources[0]" because the preceding logic already guarantees that the list variable "sources" is non-empty; update the return in the function that determines the selected source so it unconditionally returns sources[0].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/rag_router_agent/agent.py`:
- Around line 1-6: Remove the stray top-of-file __main__ block that imports and
calls bindufy(config, handler) before config and handler are defined: delete the
lines importing bindufy and the immediate bindufy(config, handler) call at the
top of the file; instead ensure bindufy is imported only alongside the
existing/final __main__ block that correctly runs the script (so the bindufy
import/usage occurs after config and handler are defined and the final __main__
executes). This prevents the NameError for config/handler and keeps the existing
correct __main__ flow intact.
- Around line 22-36: The module currently constructs Agent(...) at import time
causing OpenAIChat(api_key=None) to raise when OPENROUTER_API_KEY is unset; fix
it by guarding instantiation: check api_key and only create the OpenAIChat and
Agent objects when api_key is truthy (e.g., if api_key: agent = Agent(... using
OpenAIChat(...))) otherwise set agent = None or provide a fallback factory
function get_agent() that constructs Agent lazily; update any code that assumes
a global Agent instance to handle agent being None or call the factory so
imports no longer raise ModelAuthenticationError (refer to the Agent and
OpenAIChat symbols when making the change).
In `@examples/rag_router_agent/README.md`:
- Around line 48-55: The README example shows a relative db_used value but at
runtime agent.py builds db_used with os.path.join(BASE_DIR, "db", ...) producing
an absolute path; update the runtime output instead of the docs by normalizing
db_used in the response path generator (the _base_response function) to a stable
relative form (e.g., use os.path.relpath(db_used, BASE_DIR) or strip BASE_DIR)
before adding it to the response, referencing the db_used field, _base_response,
BASE_DIR and the code path that uses os.path.join to construct the path.
In `@examples/rag_router_agent/skale_payment.py`:
- Line 13: The code currently calls requests.get(FACILITATOR_URL, timeout=5,
verify=False) which disables TLS verification; revert to secure defaults by
removing verify=False so requests.get(FACILITATOR_URL, timeout=5) uses
certificate validation, and if you must allow self-signed certs make this opt-in
via a clearly named env var (e.g., FACILITATOR_SKIP_TLS_VERIFY) that, when set,
passes verify=False; also avoid globally suppressing InsecureRequestWarning and
instead warn when the opt-in is used so the insecure mode is explicit (locate
this change around the requests.get call and any import-level
InsecureRequestWarning suppression).
In `@examples/rag_router_agent/source_router.py`:
- Around line 43-50: The premium keyword check in source_router.py uses
substring matching (premium_keywords and any(word in query_lower ...)) which
causes false positives (e.g., "pro" matching "problem"); update it to tokenize
the query the same way router.classify_intent does (use re.findall(r"\w+",
query_lower) after adding import re) and test membership against the token set
(e.g., tokens = set(re.findall(...)) and then any(keyword in tokens for keyword
in premium_keywords)) before iterating sources to find a premium source.
---
Nitpick comments:
In `@examples/rag_router_agent/agent.py`:
- Line 77: The current check uses payment_result.get(...) which will raise if
payment_result is None; update the guard around the call handling (where
call_skale_facilitator is used and payment_result is assigned) to defensively
ensure payment_result is not None (e.g., check if payment_result is truthy or is
a dict) before calling .get, then treat a None result as a failure path (same
behavior as non-success/non-reachable statuses). Reference payment_result and
the call_skale_facilitator call site when adding the None-safe conditional.
- Around line 116-121: The except block around agent_fn(query, context)
currently prints the error and swallows the stack trace; replace print(...) with
logging.exception(...) to log the full stack and error context while still
setting agent_response fallback, and do the same for the LLM call's try/except
(the block handling the LLM response) so both agent_fn and the LLM exceptions
use logging.exception(...) instead of print/quiet catches; reference the
agent_fn, agent_response, query, context symbols and the LLM call site (the
function/variable used to invoke the model) when making the changes.
In `@examples/rag_router_agent/retriever.py`:
- Around line 5-10: In retrieve_docs, broaden the exception handling so other IO
errors don't crash the agent: catch OSError (which covers FileNotFoundError,
PermissionError, IsADirectoryError, etc.) and also catch UnicodeDecodeError when
opening/reading db_path, log a warning via the module's logger (or processLogger
used elsewhere) with the error details, and return an empty list []; update the
except block around the open/read in retriever.py accordingly so agent.py
continues graceful degradation.
- Around line 13-29: The current token-intersection scoring (query_words &
doc_words computed with re.findall) over-weights common stopwords; update the
scoring in retriever.py by filtering out a small STOPWORDS set before forming
query_words and doc_words (i.e., define STOPWORDS and replace query_words =
set(re.findall(...)) and doc_words = set(re.findall(...)) with filtered sets) so
score = len(query_words & doc_words) reflects domain tokens; alternatively,
switch scoring to simple TF-style weighting by counting token frequencies from
re.findall and computing sum(min(tf_query[token], tf_doc[token]) for token in
shared_tokens) before appending to scored and sorting by score.
In `@examples/rag_router_agent/router.py`:
- Around line 8-10: Remove the fragile global sys.path manipulation (the
BASE_DIR check and sys.path.insert call) and replace the absolute imports like
from agents.finance_agent import finance_agent with a try/except ImportError
pattern (as used in agent.py) that first attempts the absolute import and falls
back to a relative import (or vice versa) so the module works when run as a
script or imported as a package; update the same pattern for the other absolute
imports around the block identified (the imports at/near lines 34-37) and ensure
you reference BASE_DIR only if still needed locally, not to mutate sys.path
globally.
In `@examples/rag_router_agent/skale_payment.py`:
- Around line 34-37: Narrow the broad except and avoid returning raw error
strings: replace the current except Exception block with an except
requests.RequestException (import requests if not present), log the caught
exception (including e) to your logger, and return a generic error payload
keeping the same keys ("status": "error", "message": "<generic user-facing
message>") so internal URLs/stack traces are not leaked; ensure other exceptions
propagate normally (so KeyboardInterrupt and programming errors aren't
swallowed).
In `@examples/rag_router_agent/source_router.py`:
- Around line 57-58: The final fallback uses a redundant conditional: replace
the expression "return sources[0] if sources else None" with a direct "return
sources[0]" because the preceding logic already guarantees that the list
variable "sources" is non-empty; update the return in the function that
determines the selected source so it unconditionally returns sources[0].
In `@examples/rag_router_agent/test_local.py`:
- Around line 1-23: This file is named with the pytest-discovered prefix `test_`
but is a manual smoke-test runner; rename the file (e.g., to run_local.py or
smoke_local.py) so pytest won't auto-collect it, keep the existing run_test()
function and the if __name__ == "__main__": run_test() guard unchanged, and
ensure any README or developer doc that references this script updates the new
filename; the script should continue to import handler from agent and invoke
handler([{"role":"user","content": q}]) as it does now.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bff48ac-2da2-4722-851f-c1518d60fd30
📒 Files selected for processing (15)
docs/PAYMENT.mdexamples/rag_router_agent/README.mdexamples/rag_router_agent/agent.pyexamples/rag_router_agent/agents/finance_agent.pyexamples/rag_router_agent/agents/legal_agent.pyexamples/rag_router_agent/agents/tech_agent.pyexamples/rag_router_agent/db/finance.txtexamples/rag_router_agent/db/finance_premium.txtexamples/rag_router_agent/db/legal.txtexamples/rag_router_agent/db/tech.txtexamples/rag_router_agent/retriever.pyexamples/rag_router_agent/router.pyexamples/rag_router_agent/skale_payment.pyexamples/rag_router_agent/source_router.pyexamples/rag_router_agent/test_local.py
| ```json | ||
| { | ||
| "answer": "GST is a tax applied on goods and services...", | ||
| "intent": "finance", | ||
| "agent_used": "finance", | ||
| "db_used": "db/finance.txt" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Documented db_used value won't match runtime output.
agent.py populates db_used via os.path.join(BASE_DIR, "db", "finance.txt"), so the actual response contains an absolute path (e.g., /.../examples/rag_router_agent/db/finance.txt), not the relative db/finance.txt shown here. Either document the absolute form, or post-process the path in _base_response to expose a stable relative value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/rag_router_agent/README.md` around lines 48 - 55, The README example
shows a relative db_used value but at runtime agent.py builds db_used with
os.path.join(BASE_DIR, "db", ...) producing an absolute path; update the runtime
output instead of the docs by normalizing db_used in the response path generator
(the _base_response function) to a stable relative form (e.g., use
os.path.relpath(db_used, BASE_DIR) or strip BASE_DIR) before adding it to the
response, referencing the db_used field, _base_response, BASE_DIR and the code
path that uses os.path.join to construct the path.
…uting, test cleanup)
|
Pushed updates addressing review feedback:
Also verified behavior locally:
Would appreciate another review! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/rag_router_agent/source_router.py (1)
6-11:⚠️ Potential issue | 🟠 MajorAdd source entries for every routed intent.
classify_intent()can returnlegalandtech, butSOURCESonly containsfinance, so legal/tech queries always return"No source found"before retrieval or domain-agent execution.🛠️ Suggested shape
SOURCES = { "finance": [ {"path": os.path.join(BASE_DIR, "db", "finance.txt"), "type": "free"}, {"path": os.path.join(BASE_DIR, "db", "finance_premium.txt"), "type": "premium"}, - ] + ], + "legal": [ + {"path": os.path.join(BASE_DIR, "db", "<legal-db-file>.txt"), "type": "free"}, + ], + "tech": [ + {"path": os.path.join(BASE_DIR, "db", "<tech-db-file>.txt"), "type": "free"}, + ], }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/source_router.py` around lines 6 - 11, SOURCES currently only defines "finance" but classify_intent() can return "legal" and "tech", causing early "No source found" responses; update the SOURCES dict to include matching entries for "legal" and "tech" with appropriate path and type metadata (e.g. add keys "legal" and "tech" with lists of source objects similar to finance) so that the lookup in the routing logic finds sources for all intents returned by classify_intent().
🧹 Nitpick comments (1)
examples/rag_router_agent/run_local.py (1)
2-19: Avoid swallowing smoke-test failures.The broad catches hide the traceback and trigger Ruff BLE001. Narrow the import error and let handler failures fail fast, or catch specific expected errors only.
♻️ Proposed cleanup
def run_test(): try: from agent import handler # direct import (works when run as script) - except Exception as e: + except ImportError as e: print(f"[ERROR] Import failed: {e}") return @@ for q in queries: print(f"\nQuery: {q}") - try: - response = handler([{"role": "user", "content": q}]) - print(response) - except Exception as e: - print(f"[ERROR] {e}") + response = handler([{"role": "user", "content": q}]) + print(response)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/run_local.py` around lines 2 - 19, The current broad excepts around "from agent import handler" and the handler invocation swallow errors and hide tracebacks; change the import block to catch only ImportError (or ModuleNotFoundError) and include the full traceback or re-raise to surface import failures, and remove the blanket "except Exception" around calling handler — either let handler(...) failures propagate (fail fast) or catch only specific expected exceptions and log the traceback before re-raising; reference the import of handler and the handler([...]) call when making these edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/rag_router_agent/agent.py`:
- Around line 35-40: The handler currently assumes messages[-1] is a dict and
that messages[-1]["content"] is a string before calling .strip(), which can
raise if the item is not a dict or content is None/non-string; update the
validation in handler to check that messages is non-empty, messages[-1] is an
instance of dict, that "content" exists in that dict, and that
messages[-1]["content"] is a str (or convert safely) before calling .strip();
return _error("Invalid input") when these checks fail so _error is reached
instead of raising; reference the handler function and the use of
messages[-1]["content"].strip() to locate the change.
---
Duplicate comments:
In `@examples/rag_router_agent/source_router.py`:
- Around line 6-11: SOURCES currently only defines "finance" but
classify_intent() can return "legal" and "tech", causing early "No source found"
responses; update the SOURCES dict to include matching entries for "legal" and
"tech" with appropriate path and type metadata (e.g. add keys "legal" and "tech"
with lists of source objects similar to finance) so that the lookup in the
routing logic finds sources for all intents returned by classify_intent().
---
Nitpick comments:
In `@examples/rag_router_agent/run_local.py`:
- Around line 2-19: The current broad excepts around "from agent import handler"
and the handler invocation swallow errors and hide tracebacks; change the import
block to catch only ImportError (or ModuleNotFoundError) and include the full
traceback or re-raise to surface import failures, and remove the blanket "except
Exception" around calling handler — either let handler(...) failures propagate
(fail fast) or catch only specific expected exceptions and log the traceback
before re-raising; reference the import of handler and the handler([...]) call
when making these edits.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: be366d64-81d1-4b88-b585-165724a99e7c
📒 Files selected for processing (5)
examples/rag_router_agent/agent.pyexamples/rag_router_agent/retriever.pyexamples/rag_router_agent/run_local.pyexamples/rag_router_agent/skale_payment.pyexamples/rag_router_agent/source_router.py
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/rag_router_agent/retriever.py
…l calls, retrieval fixes)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/rag_router_agent/agent.py (1)
36-40:⚠️ Potential issue | 🟡 MinorValidate the last message shape before calling
.strip().
messages[-1]can be non-dict, or"content"can beNone/non-string, causing a crash before the intended"Invalid input"response.Proposed guard
def handler(messages): - if not messages or "content" not in messages[-1]: + if not messages or not isinstance(messages[-1], dict): return _error("Invalid input") - query = messages[-1]["content"].strip() + content = messages[-1].get("content") + if not isinstance(content, str) or not content.strip(): + return _error("Invalid input") + + query = content.strip()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/agent.py` around lines 36 - 40, Validate the shape and types of the last message in handler before calling .strip(): ensure messages[-1] is a dict-like object and that messages[-1].get("content") is a non-null string (e.g., isinstance(content, str)) and handle falsy/empty content by returning _error("Invalid input"); update the early check in handler to perform these type checks instead of only checking key presence so attempts to call .strip() cannot raise on non-dict or non-string values.
🧹 Nitpick comments (1)
examples/rag_router_agent/retriever.py (1)
11-12: Normalize document lines before returning them.
readlines()preserves trailing newlines, sodocs_usedcan contain raw\nandagent.py’s"\n".join(docs)can double-space the LLM context.Proposed cleanup
try: with open(db_path, "r", encoding="utf-8") as f: - docs = f.readlines() + docs = [line.rstrip("\n") for line in f]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/retriever.py` around lines 11 - 12, The docs are read with readlines() which preserves trailing newlines, causing docs_used to include raw "\n" and double-spacing when agent.py does "\n".join(docs); update the reader in retriever.py to normalize lines when populating docs (e.g., within the block that opens db_path and sets docs) by stripping trailing newlines or using splitlines() and also filter out empty lines so docs and docs_used contain clean entries for downstream joining.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/rag_router_agent/skale_payment.py`:
- Around line 39-44: The except requests.RequestException branch currently
returns {"status": "error", ...} which is not in the agent allowlist and blocks
premium queries; change the returned status to an allowed non-blocking fallback
(e.g., "skipped") and update the message to reflect the facilitator was
unreachable; keep the logger.warning call and the exception catch
(requests.RequestException) but return {"status": "skipped", "message":
"facilitator unreachable, skipping"} instead so agent.py accepts the fallback.
---
Duplicate comments:
In `@examples/rag_router_agent/agent.py`:
- Around line 36-40: Validate the shape and types of the last message in handler
before calling .strip(): ensure messages[-1] is a dict-like object and that
messages[-1].get("content") is a non-null string (e.g., isinstance(content,
str)) and handle falsy/empty content by returning _error("Invalid input");
update the early check in handler to perform these type checks instead of only
checking key presence so attempts to call .strip() cannot raise on non-dict or
non-string values.
---
Nitpick comments:
In `@examples/rag_router_agent/retriever.py`:
- Around line 11-12: The docs are read with readlines() which preserves trailing
newlines, causing docs_used to include raw "\n" and double-spacing when agent.py
does "\n".join(docs); update the reader in retriever.py to normalize lines when
populating docs (e.g., within the block that opens db_path and sets docs) by
stripping trailing newlines or using splitlines() and also filter out empty
lines so docs and docs_used contain clean entries for downstream joining.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e72cbd77-e2e4-4659-be5a-aa13dcbffba7
📒 Files selected for processing (3)
examples/rag_router_agent/agent.pyexamples/rag_router_agent/retriever.pyexamples/rag_router_agent/skale_payment.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/rag_router_agent/skale_payment.py`:
- Line 37: The current handler returns {"status":"unexpected", "code":
response.status_code} for all non-OK responses, which breaks the agent's
expected non-blocking fallback; update the logic around the response handling
(the branch that currently returns the "unexpected" dict) to treat transient
HTTP errors as non-blocking by returning {"status":"reachable", "code":
response.status_code, "reason": "<brief note>"} when response.status_code is 429
or >=500 (use response.ok to detect success first), and only return "unexpected"
for other permanent client errors (4xx excluding 429); ensure the change is
applied where the response dict is constructed so agent.py's accepted statuses
("success", "reachable", "skipped") are preserved.
- Line 37: The current fallback for non-exception HTTP error responses returns
{"status": "unexpected", "code": response.status_code}, which the caller in
agent.py only treats as blocking; change that return to use the non-blocking
fallback status "reachable" (e.g., return {"status": "reachable", "code":
response.status_code}) so HTTP 429/5xx responses from requests.get() are handled
the same way as exceptions and do not block premium queries; update the return
in the function that builds this response accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7c2ab0a-0e88-4d79-a8da-4e9e6f475228
📒 Files selected for processing (1)
examples/rag_router_agent/skale_payment.py
|
Thanks for the feedback! Updated facilitator failure handling to align with non-blocking design:
Also verified end-to-end behavior:
This should resolve the remaining CI issue. Would appreciate another pass! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/rag_router_agent/skale_payment.py (1)
12-12: Optional: log a warning when TLS verification is disabled.When
FACILITATOR_SKIP_TLS_VERIFY=true, the facilitator call silently runs without certificate validation. Emitting a one-time warning makes the insecure mode explicit to operators and easier to catch in logs.♻️ Proposed refinement
SKIP_TLS = os.getenv("FACILITATOR_SKIP_TLS_VERIFY", "false").lower() == "true" +if SKIP_TLS: + logger.warning( + "FACILITATOR_SKIP_TLS_VERIFY is enabled; TLS certificate validation is disabled for %s", + FACILITATOR_URL, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/rag_router_agent/skale_payment.py` at line 12, When the module-level flag SKIP_TLS is true, emit a one-time warning to make the insecure TLS setting explicit: detect SKIP_TLS in the module init (near SKIP_TLS = os.getenv(...)) and call the application logger (e.g., logging.getLogger(__name__).warning(...)) or warnings.warn(...) with a clear message like "FACILITATOR_SKIP_TLS_VERIFY=true — TLS certificate verification is disabled (insecure)". Ensure this runs only once at import (module-level) so operators see the warning in logs without spamming.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/rag_router_agent/skale_payment.py`:
- Line 12: When the module-level flag SKIP_TLS is true, emit a one-time warning
to make the insecure TLS setting explicit: detect SKIP_TLS in the module init
(near SKIP_TLS = os.getenv(...)) and call the application logger (e.g.,
logging.getLogger(__name__).warning(...)) or warnings.warn(...) with a clear
message like "FACILITATOR_SKIP_TLS_VERIFY=true — TLS certificate verification is
disabled (insecure)". Ensure this runs only once at import (module-level) so
operators see the warning in logs without spamming.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a4c284db-63c0-4aef-868e-772e915e2168
📒 Files selected for processing (1)
examples/rag_router_agent/skale_payment.py
##Summary
Problem: Current RAG agent retrieves data without considering cost or payment requirements, limiting alignment with x402-based agent economics.
Why it matters: Bindu’s roadmap emphasizes cost-aware agent interactions; without payment-aware routing, agents cannot differentiate between free and premium data sources.
What changed:
Added source-level metadata (free vs premium)
Introduced payment-aware routing logic via select_source
Triggered SKALE facilitator only for premium data access
Added payment_reason field for transparency
Fixed path resolution for consistent retrieval across environments
What did NOT change (scope boundary):
No changes to core Bindu framework
No modifications to x402 SDK or facilitator behavior
Existing agent logic and APIs remain intact
Change Type (select all that apply)
Scope (select all touched areas)
Linked Issue/PR
User-Visible / Behavior Changes
Agent now distinguishes between free and premium data sources
Premium queries trigger a payment check via SKALE facilitator
Responses include a new field:
"payment_reason": "free_access" | "premium_data_access"
Security Impact (required)
#Explaination:
A facilitator call is triggered for premium routes (existing endpoint reused)
Risk: external dependency failure (e.g., SSL or auth)
Mitigation: graceful fallback with "status": "reachable" and non-blocking flow
Verification
Environment
Environment
OS: Windows 10
Python version: 3.12
Storage backend: Local text files
Scheduler backend: None
Steps to Test
1.Run:
python examples/rag_router_agent/test_local.py
2.Test free query:
What is GST?
3.Test premium query:
latest GST updates
Expected Behavior
Free query → uses free DB, no payment
Premium query → uses premium DB, triggers payment
Valid responses returned in both cases
Actual Behavior
Matches expected behavior
Payment triggered only for premium queries
Retrieval works with correct documents
Evidence (attach at least one)
Human Verification (required)
#Verified scenarios:
Free vs premium routing works correctly
Payment triggered only for premium queries
Retrieval returns correct documents
#Edge cases checked:
Missing docs → handled gracefully
No API key → fallback works
Facilitator errors → handled without crash
#What you did NOT verify::
Real authenticated SKALE payment flow (only reachability tested)
Compatibility / Migration
Failure Recovery (if this breaks)
Disable by reverting to previous routing (route_db)
Files to restore:
agent.py
remove source_router.py
Known symptoms:
Incorrect DB selection
Payment triggered unexpectedly
Risks and Mitigations
List only real risks for this PR. If none, write
None.Checklist
uv run pytest)uv run pre-commit run --all-files)Summary by CodeRabbit
Documentation
New Features