chore: run ruff format across nodes/src/nodes/ for consistent baseline#485
Open
josephmccann wants to merge 1 commit intorocketride-org:developfrom
Open
chore: run ruff format across nodes/src/nodes/ for consistent baseline#485josephmccann wants to merge 1 commit intorocketride-org:developfrom
josephmccann wants to merge 1 commit intorocketride-org:developfrom
Conversation
Fixes rocketride-org#432 Run `ruff format` across nodes/src/nodes/ Python files to establish a consistent formatting baseline. Changes: - 45 files reformatted with ruff - No logic changes — formatting only This is the one-time pass referenced in the issue before CI enforcement is added.
josephmccann
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
Contributor
📝 WalkthroughWalkthroughApplies Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nodes/src/nodes/accessibility_describe/accessibility_vision.py`:
- Line 147: Change the returned string to use single-quoted string literals per
the nodes/**/*.py guideline; update the return in accessibility_vision.py (the
line that builds the message using self._model) to use single quotes for the
outer string and escape the inner single quotes around the model placeholder so
the message remains identical (i.e., keep the same text including the quotes
around the model but switch outer quotes to single and escape the inner ones).
- Line 70: Replace the double-quoted string value for the 'clock' key in the
accessibility_vision dictionary with a single-quoted string and escape the
internal single quote (i.e., change the value for the 'clock' key in
nodes/src/nodes/accessibility_describe/accessibility_vision.py), and also update
the project's tooling by adding quote-style = 'single' to the [tool.ruff]
section of pyproject.toml so Ruff enforces single-quote strings across
nodes/**/*.py.
In `@nodes/src/nodes/anonymize/glinerRecognizer.py`:
- Line 237: Replace the nested generator expression used to build
existing_matches with a direct list comprehension to avoid Ruff C400 and extra
overhead: change the current expression that uses list((m['offset'],
m['length']) for m in ((m.get('location', {}).get('inChars') or m) for m in
text_matches)) to a comprehension like [(m['offset'], m['length']) for tm in
text_matches for m in (tm.get('location', {}).get('inChars') or tm)] so the same
elements are produced without the intermediate generator; update the assignment
to existing_matches accordingly.
In `@nodes/src/nodes/anonymize/IInstance.py`:
- Line 32: DEFAULT_PII_LABELS is defined as a mutable class-level list on
IInstance which can be accidentally mutated across instances; change it to an
immutable tuple (e.g., DEFAULT_PII_LABELS = ('person', 'name', ...)) to prevent
shared-state bugs and update any uses expecting a list (convert to list locally
if needed) and re-run Ruff formatting/linting for the nodes package.
In `@nodes/src/nodes/llamaparse/IInstance.py`:
- Line 322: Replace the use of str(e) in the f-string that constructs error_text
with the f-string conversion flag e!s to satisfy Ruff RUF010 and PEP style;
locate the error_text assignment in IInstance.py (the f-string that begins '#
LlamaParse Processing Error') and change the embedded expression from {str(e)}
to {e!s} so the exception is formatted using the conversion flag.
In `@nodes/src/nodes/llm_ibm_watson/ibm_watson.py`:
- Line 71: The long inline error message in the raise Expression triggers Ruff
TRY003; change the code to build the message string before raising: construct a
variable (e.g., msg = f'Unknown IBM Cloud location: {location!r}. Valid
locations: {", ".join(sorted(_VALID_LOCATIONS))}') and then use raise
ValueError(msg) so the message construction is not inside the raise; reference
the existing variables/location and _VALID_LOCATIONS used in the current check.
In `@nodes/src/nodes/ner/IInstance.py`:
- Around line 96-98: The code uses an unnecessary intermediate list when
deduplicating and sorting entity words; replace the expression constructing
unique_words in the loop over entities_by_type with sorted(set(words)) to avoid
the extra list allocation (update the block handling entities_by_type and the
assignment to enriched_doc.metadata[f'entities_{entity_type.lower()}']
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8dfd132-950a-42f5-ac04-1cbff688631d
📒 Files selected for processing (45)
nodes/src/nodes/accessibility_describe/accessibility_vision.pynodes/src/nodes/agent_crewai/crewai.pynodes/src/nodes/agent_langchain/IInstance.pynodes/src/nodes/agent_langchain/langchain.pynodes/src/nodes/agent_rocketride/IGlobal.pynodes/src/nodes/agent_rocketride/executor.pynodes/src/nodes/agent_rocketride/rocketride_agent.pynodes/src/nodes/anonymize/IInstance.pynodes/src/nodes/anonymize/anonymize.pynodes/src/nodes/anonymize/glinerRecognizer.pynodes/src/nodes/audio_transcribe/IGlobal.pynodes/src/nodes/autopipe/IGlobal.pynodes/src/nodes/chart_chartjs/IInstance.pynodes/src/nodes/chart_chartjs/chartjs_driver.pynodes/src/nodes/db_mysql/IGlobal.pynodes/src/nodes/db_postgres/IGlobal.pynodes/src/nodes/index_search/IEndpoint.pynodes/src/nodes/index_search/IGlobal.pynodes/src/nodes/index_search/IInstance.pynodes/src/nodes/index_search/__init__.pynodes/src/nodes/index_search/constants.pynodes/src/nodes/index_search/elasticsearch_store.pynodes/src/nodes/index_search/opensearch_client.pynodes/src/nodes/library/msgraph/application_permissions.pynodes/src/nodes/library/permissions/utils.pynodes/src/nodes/llamaparse/IInstance.pynodes/src/nodes/llm_ibm_watson/ibm_watson.pynodes/src/nodes/llm_openai/IGlobal.pynodes/src/nodes/mcp_client/IGlobal.pynodes/src/nodes/mcp_client/IInstance.pynodes/src/nodes/mcp_client/__init__.pynodes/src/nodes/mcp_client/mcp_driver.pynodes/src/nodes/mcp_client/mcp_sse_client.pynodes/src/nodes/mcp_client/mcp_stdio_client.pynodes/src/nodes/mcp_client/mcp_streamable_http_client.pynodes/src/nodes/memory_internal/memory.pynodes/src/nodes/ner/IGlobal.pynodes/src/nodes/ner/IInstance.pynodes/src/nodes/ocr/IGlobal.pynodes/src/nodes/ocr/IInstance.pynodes/src/nodes/tool_firecrawl/firecrawl_driver.pynodes/src/nodes/tool_http_request/IGlobal.pynodes/src/nodes/tool_http_request/http_client.pynodes/src/nodes/tool_http_request/http_driver.pynodes/src/nodes/vectordb_postgres/vectordb_postgres.py
💤 Files with no reviewable changes (7)
- nodes/src/nodes/mcp_client/IInstance.py
- nodes/src/nodes/agent_langchain/IInstance.py
- nodes/src/nodes/mcp_client/mcp_streamable_http_client.py
- nodes/src/nodes/mcp_client/mcp_stdio_client.py
- nodes/src/nodes/mcp_client/mcp_driver.py
- nodes/src/nodes/mcp_client/init.py
- nodes/src/nodes/autopipe/IGlobal.py
| # Spatial format prompt modifiers | ||
| SPATIAL_PROMPTS = { | ||
| 'clock': '\n\nUse clock positions for spatial references (12 o\'clock = straight ahead).', | ||
| 'clock': "\n\nUse clock positions for spatial references (12 o'clock = straight ahead).", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if pyproject.toml contains ruff quote-style configuration
rg -A5 '\[tool\.ruff\]' pyproject.tomlRepository: rocketride-org/rocketride-server
Length of output: 210
String literal uses double quotes instead of required single quotes.
Line 70 should use single quotes per the coding guideline for nodes/**/*.py. While the string contains a single quote character (o'clock), it should use single quotes with escaping rather than double quotes:
'clock': 'Use clock positions for spatial references (12 o\'clock = straight ahead).',Additionally, the ruff configuration in pyproject.toml is missing quote-style = 'single', which would enforce this convention automatically. Add this to the [tool.ruff] section to align with project standards.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/accessibility_describe/accessibility_vision.py` at line 70,
Replace the double-quoted string value for the 'clock' key in the
accessibility_vision dictionary with a single-quoted string and escape the
internal single quote (i.e., change the value for the 'clock' key in
nodes/src/nodes/accessibility_describe/accessibility_vision.py), and also update
the project's tooling by adding quote-style = 'single' to the [tool.ruff]
section of pyproject.toml so Ruff enforces single-quote strings across
nodes/**/*.py.
| return 'Invalid input. Please check your image format and prompt.' | ||
| if any(phrase in error_lower for phrase in ['model not found', 'unavailable', 'not supported']): | ||
| return f'Model \'{self._model}\' is currently unavailable. Please try a different model.' | ||
| return f"Model '{self._model}' is currently unavailable. Please try a different model." |
Contributor
There was a problem hiding this comment.
Quote style violates coding guideline.
This f-string was reformatted to use double quotes, but the coding guideline requires single quotes for regular string literals. While the string contains single quotes (making double quotes convenient to avoid escaping), the project standard requires consistent use of single quotes.
As per coding guidelines: nodes/**/*.py must use single quotes for regular string literals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/accessibility_describe/accessibility_vision.py` at line 147,
Change the returned string to use single-quoted string literals per the
nodes/**/*.py guideline; update the return in accessibility_vision.py (the line
that builds the message using self._model) to use single quotes for the outer
string and escape the inner single quotes around the model placeholder so the
message remains identical (i.e., keep the same text including the quotes around
the model but switch outer quotes to single and escape the inner ones).
| (m['offset'], m['length']) | ||
| for m in ((m.get('location', {}).get('inChars') or m) for m in text_matches) | ||
| ) | ||
| existing_matches = list((m['offset'], m['length']) for m in ((m.get('location', {}).get('inChars') or m) for m in text_matches)) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Replace the unnecessary generator with a direct list comprehension.
Line 237 materializes list((...) for ...), which triggers Ruff C400 and adds avoidable overhead.
Proposed fix
- existing_matches = list((m['offset'], m['length']) for m in ((m.get('location', {}).get('inChars') or m) for m in text_matches))
+ existing_matches = [
+ (m['offset'], m['length'])
+ for m in ((m.get('location', {}).get('inChars') or m) for m in text_matches)
+ ]🧰 Tools
🪛 Ruff (0.15.7)
[warning] 237-237: Unnecessary generator (rewrite as a list comprehension)
Rewrite as a list comprehension
(C400)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/anonymize/glinerRecognizer.py` at line 237, Replace the
nested generator expression used to build existing_matches with a direct list
comprehension to avoid Ruff C400 and extra overhead: change the current
expression that uses list((m['offset'], m['length']) for m in
((m.get('location', {}).get('inChars') or m) for m in text_matches)) to a
comprehension like [(m['offset'], m['length']) for tm in text_matches for m in
(tm.get('location', {}).get('inChars') or tm)] so the same elements are produced
without the intermediate generator; update the assignment to existing_matches
accordingly.
| 'bank account', 'passport number', 'driver license' | ||
| ] | ||
|
|
||
| DEFAULT_PII_LABELS = ['person', 'name', 'email', 'phone number', 'address', 'social security number', 'credit card number', 'date of birth', 'organization', 'company', 'location', 'ip address', 'bank account', 'passport number', 'driver license'] |
Contributor
There was a problem hiding this comment.
Avoid mutable class-level defaults for DEFAULT_PII_LABELS.
Line 32 uses a class-level list, which is shared and mutable across all instances (RUF012). Use an immutable tuple to prevent accidental mutation.
Proposed fix
- DEFAULT_PII_LABELS = ['person', 'name', 'email', 'phone number', 'address', 'social security number', 'credit card number', 'date of birth', 'organization', 'company', 'location', 'ip address', 'bank account', 'passport number', 'driver license']
+ DEFAULT_PII_LABELS = (
+ 'person', 'name', 'email', 'phone number', 'address', 'social security number',
+ 'credit card number', 'date of birth', 'organization', 'company', 'location',
+ 'ip address', 'bank account', 'passport number', 'driver license',
+ )📝 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.
Suggested change
| DEFAULT_PII_LABELS = ['person', 'name', 'email', 'phone number', 'address', 'social security number', 'credit card number', 'date of birth', 'organization', 'company', 'location', 'ip address', 'bank account', 'passport number', 'driver license'] | |
| DEFAULT_PII_LABELS = ( | |
| 'person', 'name', 'email', 'phone number', 'address', 'social security number', | |
| 'credit card number', 'date of birth', 'organization', 'company', 'location', | |
| 'ip address', 'bank account', 'passport number', 'driver license', | |
| ) |
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 32-32: Mutable default value for class attribute
(RUF012)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/anonymize/IInstance.py` at line 32, DEFAULT_PII_LABELS is
defined as a mutable class-level list on IInstance which can be accidentally
mutated across instances; change it to an immutable tuple (e.g.,
DEFAULT_PII_LABELS = ('person', 'name', ...)) to prevent shared-state bugs and
update any uses expecting a list (convert to list locally if needed) and re-run
Ruff formatting/linting for the nodes package.
| f'**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n' | ||
| f'*The document could not be processed due to an error in the LlamaParse service.*' | ||
| ) | ||
| error_text = f'# LlamaParse Processing Error\n\n**Error Type:** {type(e).__name__}\n**Error Message:** {str(e)}\n\n**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n*The document could not be processed due to an error in the LlamaParse service.*' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "IInstance.py" -path "*/llamaparse/*"Repository: rocketride-org/rocketride-server
Length of output: 117
🏁 Script executed:
cat -n nodes/src/nodes/llamaparse/IInstance.py | sed -n '315,330p'Repository: rocketride-org/rocketride-server
Length of output: 1054
🏁 Script executed:
# Check if the file uses the recommended Python version and ruff
head -20 nodes/src/nodes/llamaparse/IInstance.pyRepository: rocketride-org/rocketride-server
Length of output: 1255
Use f-string conversion flag instead of str() in f-string.
Line 322 should use {e!s} instead of {str(e)} to satisfy Ruff RUF010 and follow PEP style guidelines.
Suggested fix
- error_text = f'# LlamaParse Processing Error\n\n**Error Type:** {type(e).__name__}\n**Error Message:** {str(e)}\n\n**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n*The document could not be processed due to an error in the LlamaParse service.*'
+ error_text = f'# LlamaParse Processing Error\n\n**Error Type:** {type(e).__name__}\n**Error Message:** {e!s}\n\n**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n*The document could not be processed due to an error in the LlamaParse service.*'📝 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.
Suggested change
| error_text = f'# LlamaParse Processing Error\n\n**Error Type:** {type(e).__name__}\n**Error Message:** {str(e)}\n\n**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n*The document could not be processed due to an error in the LlamaParse service.*' | |
| error_text = f'# LlamaParse Processing Error\n\n**Error Type:** {type(e).__name__}\n**Error Message:** {e!s}\n\n**File Type:** {mime_type}\n**File Size:** {len(document_data)} bytes\n\n*The document could not be processed due to an error in the LlamaParse service.*' |
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 322-322: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/llamaparse/IInstance.py` at line 322, Replace the use of
str(e) in the f-string that constructs error_text with the f-string conversion
flag e!s to satisfy Ruff RUF010 and PEP style; locate the error_text assignment
in IInstance.py (the f-string that begins '# LlamaParse Processing Error') and
change the embedded expression from {str(e)} to {e!s} so the exception is
formatted using the conversion flag.
| f'Unknown IBM Cloud location: {location!r}. ' | ||
| f'Valid locations: {", ".join(sorted(_VALID_LOCATIONS))}' | ||
| ) | ||
| raise ValueError(f'Unknown IBM Cloud location: {location!r}. Valid locations: {", ".join(sorted(_VALID_LOCATIONS))}') |
Contributor
There was a problem hiding this comment.
Ruff TRY003 warning still present on raised long inline message.
This line still triggers TRY003; please move message construction out of the raise expression to keep lint clean before CI enforcement.
♻️ Proposed fix
if location not in _VALID_LOCATIONS:
- raise ValueError(f'Unknown IBM Cloud location: {location!r}. Valid locations: {", ".join(sorted(_VALID_LOCATIONS))}')
+ valid_locations = ', '.join(sorted(_VALID_LOCATIONS))
+ msg = f'Unknown IBM Cloud location: {location!r}. Valid locations: {valid_locations}'
+ raise ValueError(msg)📝 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.
Suggested change
| raise ValueError(f'Unknown IBM Cloud location: {location!r}. Valid locations: {", ".join(sorted(_VALID_LOCATIONS))}') | |
| if location not in _VALID_LOCATIONS: | |
| valid_locations = ', '.join(sorted(_VALID_LOCATIONS)) | |
| msg = f'Unknown IBM Cloud location: {location!r}. Valid locations: {valid_locations}' | |
| raise ValueError(msg) |
🧰 Tools
🪛 Ruff (0.15.7)
[warning] 71-71: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/llm_ibm_watson/ibm_watson.py` at line 71, The long inline
error message in the raise Expression triggers Ruff TRY003; change the code to
build the message string before raising: construct a variable (e.g., msg =
f'Unknown IBM Cloud location: {location!r}. Valid locations: {",
".join(sorted(_VALID_LOCATIONS))}') and then use raise ValueError(msg) so the
message construction is not inside the raise; reference the existing
variables/location and _VALID_LOCATIONS used in the current check.
Comment on lines
96
to
98
| for entity_type, words in entities_by_type.items(): | ||
| unique_words = sorted(list(set(words))) | ||
| enriched_doc.metadata[f'entities_{entity_type.lower()}'] = unique_words |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use sorted(set(words)) directly.
sorted(list(set(words))) has an unnecessary intermediate list() allocation.
Suggested cleanup
- unique_words = sorted(list(set(words)))
+ unique_words = sorted(set(words))🧰 Tools
🪛 Ruff (0.15.7)
[warning] 97-97: Unnecessary list() call within sorted()
Remove the inner list() call
(C414)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nodes/src/nodes/ner/IInstance.py` around lines 96 - 98, The code uses an
unnecessary intermediate list when deduplicating and sorting entity words;
replace the expression constructing unique_words in the loop over
entities_by_type with sorted(set(words)) to avoid the extra list allocation
(update the block handling entities_by_type and the assignment to
enriched_doc.metadata[f'entities_{entity_type.lower()}'] accordingly).
4 tasks
Contributor
Senior Review: chore — Run ruff format across nodesWhat works well
Blockers (must fix before merge)
Should fix
RecommendationThe most practical path forward: rebase onto latest |
Contributor
🚀 Merge RequestGood formatting baseline. Before merge:
Suggest rebasing on a fresh branch and merging during a quiet window. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #432
Run
ruff formatacrossnodes/src/nodes/Python files to establish a consistent formatting baseline.Changes:
This is the one-time pass referenced in the issue before CI enforcement is added.
Summary by CodeRabbit