-
Notifications
You must be signed in to change notification settings - Fork 0
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
fix: avoid duplicate references! #122
Conversation
WalkthroughThe changes in the Changes
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (3)
utils/query_engine/prepare_answer_sources.py (3)
Line range hint
13-17
: Update method parameter type to excludeNone
valuesThe method
prepare_answer_sources
currently acceptsnodes: list[NodeWithScore | None]
and then filters outNone
values using a list comprehension. To enhance type safety and simplify the code, consider updating the parameter type tonodes: list[NodeWithScore]
and ensuring thatNone
values are not passed to this method.
42-42
: Adjust the docstring 'Returns' section to follow style conventionsIn the docstring's 'Returns' section, the return type includes the variable name
all_sources : str
. According to standard docstring conventions (e.g., NumPy style), the variable name should be omitted. Please update it to:Returns ------- str A formatted string containing numbered URLs organized by tool name...
71-75
: Consolidate duplicate error handling logicThe error handling for cases where no valid sources are found is duplicated at lines 71-75 and 102-106. This redundancy can be reduced by consolidating the error checking into a single conditional block, which will improve code clarity and maintainability.
Also applies to: 102-106
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/query_engine/prepare_answer_sources.py
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci / test / Test
- GitHub Check: ci / lint / Lint
🔇 Additional comments (1)
utils/query_engine/prepare_answer_sources.py (1)
108-109
:⚠️ Potential issueReplace
removesuffix
for Python version compatibilityThe use of
removesuffix("\n\n")
inreturn all_sources.removesuffix("\n\n")
requires Python 3.9 or later. If your codebase needs to support earlier Python versions, consider replacing it with an alternative method to ensure compatibility.You can use:
return all_sources.rstrip('\n')Or apply this diff:
- return all_sources.removesuffix("\n\n") + return all_sources[:-2] if all_sources.endswith("\n\n") else all_sources
Summary by CodeRabbit