Repository navigation
chore(nodes): full node test improvements and mock cleanup - #687
Merged
Merged
Conversation
- Add two-group test config for `llm_openai`: mock group (`test1`) runs with placeholder keys, real group (`test2`) requires `ROCKETRIDE_OPENAI_KEY` and uses `avoidMocks` to disable mock loading in the node subprocess - Add `avoidMocks` support: `discovery.py` parses it, `pipeline.py` propagates it, `task_engine.py` strips `ROCKETRIDE_MOCK` from subprocess env when set - Unify env var naming to `ROCKETRIDE_<PROVIDER>_<WHAT>` convention across all docs, examples, tests, and node code (e.g. `ROCKETRIDE_OPENAI_KEY`, `ROCKETRIDE_ANTHROPIC_KEY`, `ROCKETRIDE_GEMINI_KEY`, `ROCKETRIDE_OLLAMA_HOST`) - Add credential injection via env var name parsing (`_parse_credential_env_var`) so `requires` order doesn't matter - Add `google.genai` mock for `llm_gemini` and `accessibility_describe` nodes - Add `--pytest-pattern=` CLI flag for multi-word `-k` expressions - Format `llm_openai_api/services.json` with CLAUDE.md comment blocks - Set `ROCKETRIDE_MOCK` in pytest env for mock credential injection
`runner.py` was sending `lane/image` as the MIME type for file-based inputs (image, audio, video, documents). Nodes like `accessibility_describe` that use `mimeType` to build data URLs received an invalid type, causing silent failures. Detect the actual MIME type from the file extension via `mimetypes.guess_type()`.
…ollama` real test
- Remove `transformers`/`tokenizers` and transitive deps from LLM node `requirements.txt` files
- Re-enable previously skipped nodes in `conftest.py` `skip_nodes`
- Add mock credentials for newly enabled nodes in `_LLM_MOCK_CREDENTIALS`
- Add two-group test for `llm_ollama` with `avoidMocks` and `requires`
- Simplify `_parse_credential_env_var()` to use `split('_', 2)`
asclearuc
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
April 21, 2026 19:55
github-actions
Bot
added
docs
Contributor
|
No description provided. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRenamed numerous LLM-related environment variables, pruned transformer deps from many node requirements, expanded/normalized node service and test metadata, added Google GenAI test mocks, introduced avoidMocks propagation through test pipeline → task → subprocess, and reduced heavy test stubbing across many tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Discovery as Test Discovery
participant Pipeline as PipelineBuilder
participant Runner as Test Runner
participant Task as Task Engine
participant Node as Node subprocess
Discovery->>Pipeline: request node config (service group, profile)
Pipeline->>Pipeline: read service.requires env vars via _parse_credential_env_var
alt env creds present
Pipeline->>Pipeline: inject real credentials into node config
else if ROCKETRIDE_MOCK && !avoidMocks
Pipeline->>Pipeline: inject mock placeholders
end
Pipeline->>Runner: return pipeline config (includes avoidMocks)
Runner->>Task: start_task(pipeline)
Task->>Task: if pipeline.avoidMocks == true: pop('ROCKETRIDE_MOCK', None)
Task->>Node: spawn subprocess with filtered env
Node->>Node: load real libs or mocks based on env
Node->>Runner: run test case and return result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 docstrings
🧪 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/nodes/llm-openai-usage.md (1)
75-75:⚠️ Potential issue | 🟡 MinorInconsistent env var name in parallel-calls example.
Lines 15, 59, and 107 were updated to
ROCKETRIDE_OPENAI_KEY, but line 75 still references"apikey_env": "OPENAI_KEY". For consistency with the rest of the doc (and the repo-wide rename), update this example too.Proposed fix
- {"id": "model_a", "type": "llm_openai", "config": {"model": "gpt-4o", "apikey_env": "OPENAI_KEY", "system": "..."}}, + {"id": "model_a", "type": "llm_openai", "config": {"model": "gpt-4o", "apikey_env": "ROCKETRIDE_OPENAI_KEY", "system": "..."}},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/nodes/llm-openai-usage.md` at line 75, Update the example config for the llm_openai node (the JSON object with "id": "model_a" and "type": "llm_openai") so that the "config" key uses the repo-wide env var name "ROCKETRIDE_OPENAI_KEY" instead of "OPENAI_KEY" by replacing the value of "apikey_env": "OPENAI_KEY" with "apikey_env": "ROCKETRIDE_OPENAI_KEY" to match the other examples in the document.nodes/test/framework/runner.py (1)
181-193:⚠️ Potential issue | 🟠 MajorKeep
documentson the explicit lane route.Line 189 includes
documents, butDataConn._determine_lane()only MIME-routestext,image,video, andaudio;application/pdf/similar document MIME types fall through toraw. That changes document tests from the directlane/documentspath to raw handling.🐛 Proposed fix
- FILE_LANES = {'image', 'audio', 'video', 'documents'} - if pipeline_input_lane in FILE_LANES and isinstance(case.input_data, str): + MIME_ROUTED_FILE_LANES = {'image', 'audio', 'video'} + if pipeline_input_lane in MIME_ROUTED_FILE_LANES and isinstance(case.input_data, str): import mimetypes guessed, _ = mimetypes.guess_type(case.input_data) if guessed: mime_type = guessed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodes/test/framework/runner.py` around lines 181 - 193, The tests add "documents" to FILE_LANES and set mime_type from the file extension, but DataConn._determine_lane still only maps MIME types for 'text', 'image', 'video', and 'audio', causing document MIME types (e.g., application/pdf) to fall through to 'raw'; update DataConn._determine_lane to recognize common document MIME types (application/pdf, application/msword, application/vnd.openxmlformats-officedocument.* and similar) and return the 'documents' lane so that the pipeline_input_lane='documents' path is preserved; refer to the mime_type variable and FILE_LANES in runner.py and the DataConn._determine_lane method when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.template:
- Around line 11-14: Update the ROCKETRIDE_OLLAMA_HOST placeholder to indicate
it expects a base URL (host) not an API key: replace the "[your API key]" text
for ROCKETRIDE_OLLAMA_HOST with a host URL placeholder (e.g., a scheme + host
like "https://your-ollama-host" or "http://localhost:11434") and ensure the
comment clarifies it is a base URL; locate the ROCKETRIDE_OLLAMA_HOST line in
.env.template to make this change.
In `@nodes/src/nodes/llm_ollama/services.json`:
- Around line 472-488: Update the real-invocation test group: rename the case
"LLM returns mock response" to something like "LLM returns real response" (this
group has avoidMocks: true and profiles: ["qwen2_5-0_5b"]) and change the
requires array to only include "ROCKETRIDE_OLLAMA_SERVERBASE" (remove
ROCKETRIDE_OLLAMA_HOST and the model env var) because the ollama driver reads
serverbase from the env mapping and the model is hardcoded in the
profile/driver.
In `@nodes/test/framework/pipeline.py`:
- Around line 130-149: The code currently skips mock credentials whenever
self.config.requires is set; change the conditional so ROCKETRIDE_MOCK
short-circuits regardless of requires: check os.environ.get('ROCKETRIDE_MOCK')
and provider in _LLM_MOCK_CREDENTIALS and profile first and use
_LLM_MOCK_CREDENTIALS[provider] when present, otherwise fall back to the
existing branch that reads self.config.requires/env vars and builds creds;
ensure the rest of the logic (merging into config[profile]) remains unchanged so
mocked services receive the mock creds even if requires is declared.
- Around line 42-63: _parse_credential_env_var currently uses env_var.split('_',
2) which misparses provider names that contain underscores (e.g.,
ROCKETRIDE_GMI_CLOUD_KEY), causing the attribute to be wrong; change the parsing
to remove the "ROCKETRIDE_" prefix first and then split the remainder on the
first underscore only (e.g., remainder = env_var[len("ROCKETRIDE_"):];
provider_attr = remainder.split('_', 1)) so attr = provider_attr[1]; then map
attr via _ENV_ATTR_MAP or lower-case it as before. This preserves multiword
provider names and ensures the attribute resolution in _parse_credential_env_var
and lookups against _ENV_ATTR_MAP are correct.
---
Outside diff comments:
In `@docs/nodes/llm-openai-usage.md`:
- Line 75: Update the example config for the llm_openai node (the JSON object
with "id": "model_a" and "type": "llm_openai") so that the "config" key uses the
repo-wide env var name "ROCKETRIDE_OPENAI_KEY" instead of "OPENAI_KEY" by
replacing the value of "apikey_env": "OPENAI_KEY" with "apikey_env":
"ROCKETRIDE_OPENAI_KEY" to match the other examples in the document.
In `@nodes/test/framework/runner.py`:
- Around line 181-193: The tests add "documents" to FILE_LANES and set mime_type
from the file extension, but DataConn._determine_lane still only maps MIME types
for 'text', 'image', 'video', and 'audio', causing document MIME types (e.g.,
application/pdf) to fall through to 'raw'; update DataConn._determine_lane to
recognize common document MIME types (application/pdf, application/msword,
application/vnd.openxmlformats-officedocument.* and similar) and return the
'documents' lane so that the pipeline_input_lane='documents' path is preserved;
refer to the mime_type variable and FILE_LANES in runner.py and the
DataConn._determine_lane method when making the change.
🪄 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: bfcb91c9-3cc7-444b-a0ae-512a17996535
📒 Files selected for processing (38)
.env.templatedocs/nodes/llm-openai-usage.mdnodes/scripts/tasks.jsnodes/src/nodes/llm_anthropic/requirements.txtnodes/src/nodes/llm_bedrock/requirements.txtnodes/src/nodes/llm_deepseek/requirements.txtnodes/src/nodes/llm_gemini/services.jsonnodes/src/nodes/llm_ollama/requirements.txtnodes/src/nodes/llm_ollama/services.jsonnodes/src/nodes/llm_openai/requirements.txtnodes/src/nodes/llm_openai/services.jsonnodes/src/nodes/llm_openai_api/requirements.txtnodes/src/nodes/llm_openai_api/services.jsonnodes/src/nodes/llm_perplexity/requirements.txtnodes/src/nodes/llm_qwen/requirements.txtnodes/src/nodes/llm_vertex/requirements.txtnodes/src/nodes/llm_vision_ollama/requirements.txtnodes/src/nodes/llm_xai/requirements.txtnodes/src/nodes/search_exa/IGlobal.pynodes/src/nodes/search_exa/exa_search.pynodes/src/nodes/tool_exa_search/IInstance.pynodes/src/nodes/tool_exa_search/services.jsonnodes/test/conftest.pynodes/test/framework/discovery.pynodes/test/framework/pipeline.pynodes/test/framework/runner.pynodes/test/milvus/test_filter_injection.pynodes/test/mocks/google/__init__.pynodes/test/mocks/google/genai/__init__.pynodes/test/mocks/google/genai/types.pypackages/ai/src/ai/modules/task/task_engine.pypackages/client-python/.env.examplepackages/client-python/tests/RocketRideClient_test.pypackages/client-python/tests/chat_pipeline.pypackages/client-typescript/tests/RocketRideClient.test.tspackages/client-typescript/tests/chat.pipeline.tspipelines/exa-search-working.pipescripts/build.js
💤 Files with no reviewable changes (11)
- nodes/src/nodes/llm_openai_api/requirements.txt
- nodes/src/nodes/llm_qwen/requirements.txt
- nodes/src/nodes/llm_vision_ollama/requirements.txt
- nodes/src/nodes/tool_exa_search/IInstance.py
- nodes/src/nodes/llm_vertex/requirements.txt
- nodes/src/nodes/llm_deepseek/requirements.txt
- nodes/src/nodes/llm_perplexity/requirements.txt
- nodes/src/nodes/llm_openai/requirements.txt
- nodes/src/nodes/llm_bedrock/requirements.txt
- nodes/src/nodes/llm_xai/requirements.txt
- nodes/src/nodes/llm_ollama/requirements.txt
asclearuc
marked this pull request as draft
April 21, 2026 20:22
…reinstall` flag - Move `pytest`, `pytest-asyncio` from inline pip install to `nodes/test/requirements.txt` - Add `pytest-timeout` (required by `pyproject.toml` `timeout = 120` config) - `server:setup-pip` now installs test deps from requirements file - Add `--pytest-preinstall="dep1,dep2>=3"` CLI flag for pre-installing heavy packages (e.g. `faster-whisper`) before test runs
- Remove `depends`, `engLib`, `rocketlib`, `ai.*` mocks from 8 test files — use real modules instead of `sys.modules` stubs - Patch `doesCollectionExist`/`_checkCollectionExists` at instance level where the real `DocumentStoreBase` needs infrastructure - Increase default `pytest-timeout` from 120s to 600s to accommodate nodes that install heavy dependencies on first run
asclearuc
changed the title
asclearuc
changed the title
- `llm_ollama/services.json`: small fix - `_parse_credential_env_var`: validate the env var's provider segment against the known provider (full `LLM_OPENAI` or short `OPENAI` form) so typos and cross-provider pollution (e.g. `ROCKETRIDE_ANTHROPIC_KEY` in an OpenAI `requires` list) are rejected instead of silently mapping to `apikey`. Also handles multi-word providers like `llm_gmi_cloud` - `_get_node_config`: let `ROCKETRIDE_MOCK` + `requires` compose independently — mock placeholders seed the config when mocks are enabled, then `requires` env vars overlay on top. Previously, any group with `requires` skipped `_LLM_MOCK_CREDENTIALS` entirely, leaving mock-capable nodes uncredentialed
asclearuc
marked this pull request as ready for review
April 22, 2026 11:43
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/test/test_contracts.py (1)
461-522: 🛠️ Refactor suggestion | 🟠 MajorRemove dead
sys.modulestracking scaffolding left over from stub removal.
original_modulesandmodules_to_mockare built but never read, and there's no teardown that restores them — consistent with the PR's goal of dropping internal-module stubs fromsys.modules. Drop the unused state to avoid misleading readers into thinking stubs are still installed.♻️ Proposed cleanup
`@pytest.fixture`(autouse=True) def mock_engine_libs(): - """Mock engLib, rocketlib, and ai modules before tests.""" - # Track original modules - original_modules = {} - modules_to_mock = ['engLib', 'rocketlib', 'ai', 'ai.common', 'ai.common.schema', 'depends', 'util'] - - for mod_name in modules_to_mock: - original_modules[mod_name] = sys.modules.get(mod_name) - - # Mock engLib + """Yield mock engLib/rocketlib/ai/depends/util objects for tests that opt in.""" + # Mock engLib mock_englib = MagicMock()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodes/test/test_contracts.py` around lines 461 - 522, The fixture mock_engine_libs contains unused tracking variables original_modules and modules_to_mock that were left over from stub-install scaffolding; remove those two variables and their related comments so the fixture only builds and yields the mock objects (keep mock_engine_libs, mock_englib, mock_rocketlib, mock_ai, mock_depends, mock_util and the returned dict intact), and ensure there is no leftover teardown or reference to original_modules/modules_to_mock elsewhere in the test file.
🤖 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/test/conftest.py`:
- Around line 60-68: The __init__ docstring incorrectly documents parameters
that don't exist; update the __init__ method's docstring to a concise PEP 257
description stating that the test configuration is initialized from environment
variables (no parameters), mention any environment variables read (if
applicable), and remove the bogus Args section; ensure it is a one-line summary
followed by an optional short description per PEP 257 and applies to the
__init__ method in this file.
In `@packages/server/scripts/tasks.js`:
- Around line 826-832: The code that builds the deps list from preinstall in the
preinstall branch (variable preinstall and array deps used with execCommand and
enginePath/DIST_DIR) uses preinstall.split(',').filter(Boolean) which leaves
surrounding whitespace on tokens (e.g., " dep2"), so trim each token before
filtering; change the construction of deps to split on ',' then map each token
with .trim() and then filter(Boolean) so task.output and pip installs receive
clean package strings.
In `@scripts/build.js`:
- Around line 80-83: The parser currently overwrites options.pytestPattern and
options.pytestPreinstall when their flags repeat (see
arg.startsWith('--pytest-pattern=') and arg.startsWith('--pytest-preinstall='));
keep pytestPattern as a single value but change pytestPreinstall to accumulate
values like --pytest= does: initialize options.pytestPreinstall as an array and
push each parsed value instead of assigning, and update any help/usage text to
document that --pytest-preinstall can be repeated (or alternatively, explicitly
document it as single-value if you prefer not to change behavior).
---
Outside diff comments:
In `@nodes/test/test_contracts.py`:
- Around line 461-522: The fixture mock_engine_libs contains unused tracking
variables original_modules and modules_to_mock that were left over from
stub-install scaffolding; remove those two variables and their related comments
so the fixture only builds and yields the mock objects (keep mock_engine_libs,
mock_englib, mock_rocketlib, mock_ai, mock_depends, mock_util and the returned
dict intact), and ensure there is no leftover teardown or reference to
original_modules/modules_to_mock elsewhere in the test file.
🪄 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: a5f6260f-2d1e-4d15-9e79-eb6fed4b6d5d
📒 Files selected for processing (16)
.env.templatenodes/src/nodes/llm_ollama/services.jsonnodes/test/atlas/test_isdeleted_filter.pynodes/test/chroma/test_convert_filter_explicit_none.pynodes/test/conftest.pynodes/test/framework/pipeline.pynodes/test/llm_ibm_watson/test_location_injection.pynodes/test/milvus/test_filter_injection.pynodes/test/pinecone/test_record_updates.pynodes/test/qdrant/test_mark_deleted.pynodes/test/requirements.txtnodes/test/test_contracts.pynodes/test/weaviate/test_record_updates.pypackages/server/scripts/tasks.jspyproject.tomlscripts/build.js
💤 Files with no reviewable changes (2)
- nodes/test/chroma/test_convert_filter_explicit_none.py
- nodes/test/pinecone/test_record_updates.py
- `llm_ollama/services.json`: rename `ROCKETRIDE_OLLAMA_HOST` to `ROCKETRIDE_OLLAMA_SERVERBASE` so `_parse_credential_env_var` maps it to the `serverbase` config field the driver actually reads - `nodes/test/conftest.py`: replace `TestConfig.__init__` docstring with an accurate PEP 257 summary (previous `Args` block listed parameters that don't exist — copy/paste from an LLM node) - `packages/server/scripts/tasks.js`: trim whitespace from `--pytest-preinstall` deps so `"dep1, dep2"` doesn't send `" dep2"` to `pip install`
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/test/conftest.py (1)
224-239:⚠️ Potential issue | 🟡 MinorUpdate the skip-node comment and add tracking for the new skip.
The comment still lists
llm_anthropic/llm_ollamaas skipped, but they are no longer inskip_nodes. Also, the newly addedaudio_transcribeskip has a reason but no follow-up/tracking reference, which conflicts with the PR objective to avoid unchecked skip additions.Proposed cleanup
- # Groups: ML/heavy (anonymize, ocr, ner, embedding_image); image/video (image_cleanup, frame_grabber); LLM/local (llm_anthropic, llm_ollama). + # Groups: ML/heavy (anonymize, ocr, ner, embedding_image, audio_transcribe); image/video (image_cleanup, frame_grabber). skip_nodes = { 'anonymize', 'ocr', 'ner', 'embedding_image', 'image_cleanup', 'frame_grabber', - 'audio_transcribe', # it downloads faster-whisper model (1.5GB) + 'audio_transcribe', # downloads faster-whisper model (~1.5GB); TODO(#<issue>): move to explicit heavy-test lane🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodes/test/conftest.py` around lines 224 - 239, The comment above the skip_nodes set is out of sync: remove mention of llm_anthropic and llm_ollama (they are no longer in skip_nodes) and update the “Groups” list to reflect the current skipped items; also add a tracking reference (issue/PR ID or TODO with owner) next to the newly added 'audio_transcribe' entry in skip_nodes so every skip has an associated follow-up, and ensure the comment mentions that 'audio_transcribe' is skipped due to the faster-whisper model download with the tracking reference included.
♻️ Duplicate comments (1)
nodes/src/nodes/llm_ollama/services.json (1)
473-473:⚠️ Potential issue | 🟡 Minor
ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5Bhas no effect on the driver — acts as a gate only.The
SERVERBASErename correctly maps to the driver'sserverbasefield via_parse_credential_env_var's lowercase fallback. However,ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5Bparses to fieldmodel_qwen2_5_0_5b, which the ollama driver never reads — the actual model (qwen2.5:0.5b) is hardcoded in theqwen2_5-0_5bprofile (line 161). So this env var functions purely as a test gate; its value is silently ignored.If the intent is just to signal "real Ollama is available with this model pulled," consider renaming to something flag-like (e.g.,
ROCKETRIDE_OLLAMA_AVAILABLE) or dropping it, to avoid implying the model is overridable from env. If the intent is to override the model dynamically, the driver/profile plumbing would need to change as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodes/src/nodes/llm_ollama/services.json` at line 473, The env var ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5B currently only gates availability because _parse_credential_env_var maps it to model_qwen2_5_0_5b which the ollama driver/profile (qwen2_5-0_5b) never reads; either rename/remove it to avoid implying an override (e.g., ROCKETRIDE_OLLAMA_AVAILABLE) or modify the driver/profile to honor the parsed model field: update the ollama driver code that builds the request (and/or the qwen2_5-0_5b profile) to read model_qwen2_5_0_5b (fallback logic in _parse_credential_env_var) and use that value instead of the hardcoded qwen2.5:0.5b; reference symbols to change: ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5B, _parse_credential_env_var, model_qwen2_5_0_5b, the ollama driver code that selects model and the qwen2_5-0_5b profile.
🤖 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/test/conftest.py`:
- Line 60: Add the missing explicit return type annotation to the constructor
and update the Optional typing to Python 3.10 union syntax: change the __init__
signature in the class (the def __init__(self)) to include an explicit "-> None"
return annotation, and replace any occurrences of Optional[str] (or similar
Optional[...] usages) in the file (e.g., the parameter/attribute at the location
referenced around line 181) with the modern union form "str | None" to satisfy
Ruff's ANN204 and RUF013 checks.
- Around line 113-114: The client fixture currently declares an unused parameter
server_available; replace this dependency with a decorator to avoid ARG001. Add
`@pytest.mark.usefixtures`('server_available') above the existing
`@pytest_asyncio.fixture` and remove server_available from the async def
client(...) signature so the fixture remains async def client(...): while still
depending on server_available via the decorator.
In `@packages/server/scripts/tasks.js`:
- Around line 810-824: The current bootstrap checks the boolean state key
'server.pipInstalled' (via getState/setState) so environments with that flag
already true will skip re-installing nodes/test/requirements.txt; change the
logic to detect changes in the requirements file instead of a static boolean:
compute a content hash (e.g., SHA256) of nodes/test/requirements.txt and store
it with setState (new key like 'server.pipInstalledHash' or bump the key to
'server.pipInstalledV2'), then compare the stored hash to the current file hash
before deciding to skip; if the hash differs or no stored value exists, run the
existing execCommand sequence (the pip bootstrap and the install -r
path.join(PROJECT_ROOT,'nodes','test','requirements.txt')) and update the stored
hash with setState so future runs skip only when the requirements file is
unchanged.
---
Outside diff comments:
In `@nodes/test/conftest.py`:
- Around line 224-239: The comment above the skip_nodes set is out of sync:
remove mention of llm_anthropic and llm_ollama (they are no longer in
skip_nodes) and update the “Groups” list to reflect the current skipped items;
also add a tracking reference (issue/PR ID or TODO with owner) next to the newly
added 'audio_transcribe' entry in skip_nodes so every skip has an associated
follow-up, and ensure the comment mentions that 'audio_transcribe' is skipped
due to the faster-whisper model download with the tracking reference included.
---
Duplicate comments:
In `@nodes/src/nodes/llm_ollama/services.json`:
- Line 473: The env var ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5B currently only
gates availability because _parse_credential_env_var maps it to
model_qwen2_5_0_5b which the ollama driver/profile (qwen2_5-0_5b) never reads;
either rename/remove it to avoid implying an override (e.g.,
ROCKETRIDE_OLLAMA_AVAILABLE) or modify the driver/profile to honor the parsed
model field: update the ollama driver code that builds the request (and/or the
qwen2_5-0_5b profile) to read model_qwen2_5_0_5b (fallback logic in
_parse_credential_env_var) and use that value instead of the hardcoded
qwen2.5:0.5b; reference symbols to change: ROCKETRIDE_OLLAMA_MODEL_QWEN2_5_0_5B,
_parse_credential_env_var, model_qwen2_5_0_5b, the ollama driver code that
selects model and the qwen2_5-0_5b profile.
🪄 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: 8b9874c7-202c-4ba0-9a8a-f62c8698f3ba
📒 Files selected for processing (3)
nodes/src/nodes/llm_ollama/services.jsonnodes/test/conftest.pypackages/server/scripts/tasks.js
stepmikhaylov
approved these changes
Apr 22, 2026
This was referenced May 11, 2026
This was referenced May 21, 2026
This was referenced Jun 2, 2026
7 of 8 tasks
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.
Summary
sys.modulesstubs for internal modules (depends,engLib,rocketlib,ai.*) across 8 test files — use real modules, patchdoesCollectionExist/_checkCollectionExistsat instance level where needednodes/test/requirements.txt, addpytest-timeout, add--pytest-preinstallCLI flag, bump pytest timeout to 600sllm_openai(mock + real groups) withavoidMockssupport that stripsROCKETRIDE_MOCKfrom subprocess env; addllm_ollamareal test withrequiresdirectiveROCKETRIDE_<PROVIDER>_<WHAT>across docs, examples, tests, and node code; add credential injection via_parse_credential_env_varrunner.pyto detect real MIME types from file extensions viamimetypes.guess_type()instead of sending invalidlane/imagetypestransformers/tokenizersdeps from LLM noderequirements.txtfiles (llm_ollama,llm_bedrock,llm_deepseek,llm_perplexity,llm_xai,llm_qwen,llm_vertex)conftest.py, addaudio_transcribetoskip_nodes(heavyfaster-whisperinstall), add test config fortool_exa_searchgoogle.genaimock forllm_geminiandaccessibility_describenodes; fix ollama host URL variable renameType
chore
Testing
./builder testpassesChecklist
Linked Issue
Fixes #663
Summary by CodeRabbit
New Features
Tests
Bug Fixes
Documentation
Chores