Skip to content

fix(store): honor _createCollection() return value before indexing#1559

Open
SabariIyyappan wants to merge 1 commit into
rocketride-org:developfrom
SabariIyyappan:fix/RR-1495-createcollection-honor-result
Open

fix(store): honor _createCollection() return value before indexing#1559
SabariIyyappan wants to merge 1 commit into
rocketride-org:developfrom
SabariIyyappan:fix/RR-1495-createcollection-honor-result

Conversation

@SabariIyyappan

@SabariIyyappan SabariIyyappan commented Jul 13, 2026

Copy link
Copy Markdown

DocumentStoreBase.createCollection() discarded the bool returned by the
provider-specific _createCollection(), so a driver that reported failure by
returning False was ignored: the base proceeded to addChunks() against a
collection that was never created, crashing cryptically (e.g. Chroma's
"'NoneType' object has no attribute 'delete'") far from the real cause.

Summary

  • Base fix: DocumentStoreBase.createCollection() now captures _createCollection()'s return value and raises a clear error when it is exactly False, before calling addChunks(). is False is deliberate — drivers that return None on success (weaviate, vectordb_postgres) are unaffected, and drivers that raise on failure (qdrant, pinecone) propagate their own error before the check is reached.
  • Milvus prerequisite: milvus._createCollection had inverted semantics (returned False on success, True on failure), which honoring the return value would otherwise break. Corrected to the documented contract: return True on success and let real failures raise (removing a try/except that swallowed the real error).
  • Test: added packages/ai/tests/ai/common/test_store.py covering the raise-on-False, success, and exception-propagation paths.

Scope is intentionally limited to #1495. Atlas's always-True swallow and weaviate/postgres's None-return are separate defects that do not interact with this fix (the is False check leaves them working) and are left for follow-up issues.

Type

fix

Testing

  • Tests added or updated
  • Tested locally
  • ./builder test passes

./builder ai:test passes fully (including the new test_store.py). The nodes suite passes (1508 passed) including the Milvus tests; the full ./builder test has one unrelated failure — preprocessor_langchain fails to download PyTorch due to insufficient local disk space in my local (env issue, not this change).

Checklist

  • Commit messages follow conventional commits
  • No secrets or credentials included
  • Wiki updated (if applicable)
  • Breaking changes documented (if applicable)

Linked Issue

Fixes #1495

Summary by CodeRabbit

  • Bug Fixes

    • Improved vector collection creation error handling.
    • Collection setup failures are now surfaced instead of being treated as successful.
    • Prevented document indexing from continuing when collection creation fails.
    • Preserved underlying driver errors to provide clearer failure feedback.
  • Tests

    • Added coverage for successful creation, explicit failures, and propagated errors.

DocumentStoreBase.createCollection() discarded the bool returned by the
provider-specific _createCollection(), so a driver that reported failure by
returning False was ignored: the base proceeded to addChunks() against a
collection that was never created, crashing cryptically (e.g. Chroma's
"'NoneType' object has no attribute 'delete'") far from the real cause.

Capture the return value and raise a clear error when it is exactly False,
before indexing. `is False` is deliberate so drivers that return None on
success (weaviate, vectordb_postgres) are unaffected; drivers that raise on
failure (qdrant, pinecone) propagate their own error before the check.

Milvus's _createCollection had inverted semantics (returned False on success,
True on failure), which honoring the return value would otherwise break; it is
corrected to the documented contract (return True on success, let real
failures raise) in the same change.

Fixes rocketride-org#1495
@github-actions github-actions Bot added module:nodes Python pipeline nodes module:ai AI/ML modules labels Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor
🤖 Internal: Discord sync marker

Auto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Failed to post review comments.

GitHub was unavailable or timed out while CodeRabbit was posting the review. Please request a new review later if the pull request still needs one. This happened while posting 1 inline comment. Use @coderabbitai full review to retry the review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c981697d-0d00-4dd2-b753-1fdd08624fd2

📥 Commits

Reviewing files that changed from the base of the PR and between 37cd055 and 9e76062.

📒 Files selected for processing (3)
  • nodes/src/nodes/milvus/milvus.py
  • packages/ai/src/ai/common/store.py
  • packages/ai/tests/ai/common/test_store.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

Verify code follows documented patterns in ROCKETRIDE_python_API.md before submitting Python Rocket Ride client code

Files:

  • nodes/src/nodes/milvus/milvus.py
  • packages/ai/src/ai/common/store.py
  • packages/ai/tests/ai/common/test_store.py
nodes/**/*.py

⚙️ CodeRabbit configuration file

nodes/**/*.py: Python pipeline nodes: use single quotes, ruff for linting/formatting,
PEP 257 docstrings, target Python 3.10+.

Files:

  • nodes/src/nodes/milvus/milvus.py
🧠 Learnings (26)
📚 Learning: 2026-03-24T21:28:07.433Z
Learnt from: asclearuc
Repo: rocketride-org/rocketride-server PR: 417
File: nodes/src/nodes/llm_vision_ollama/__init__.py:28-32
Timestamp: 2026-03-24T21:28:07.433Z
Learning: For Python pipeline node files under `nodes/**/*.py`, follow the repo’s string/docstring quoting conventions: use single quotes for regular string literals, and require PEP 257-style docstrings to use triple double quotes (`"""..."""`). Do not recommend changing docstring quotes from `"""..."""` to `'''...'''`.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-03-25T16:11:50.206Z
Learnt from: KaushikSiva
Repo: rocketride-org/rocketride-server PR: 386
File: nodes/src/nodes/search_exa/IInstance.py:31-32
Timestamp: 2026-03-25T16:11:50.206Z
Learning: In nodes/**/*.py, treat `self.instance.writeAnswers(...)` as accepting a single `Answer` object (not `List[Answer]`). In reviews, do not flag calls where a single `Answer` is passed to `writeAnswers` as type mismatches. If the abstract base class (e.g., `filters.py`) still declares `List[Answer]`, consider updating that type annotation to match the actual runtime/override usage, since concrete implementations (e.g., `response/IInstance.py`, `extract_data/IInstance.py`, `dictionary/IInstance.py`, `db_instance_base.py`) and all call sites pass a single `Answer`.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-03-27T23:31:51.042Z
Learnt from: asclearuc
Repo: rocketride-org/rocketride-server PR: 456
File: nodes/src/nodes/db_neo4j/IInstance.py:98-100
Timestamp: 2026-03-27T23:31:51.042Z
Learning: In this project’s Python lifecycle stubs under `nodes/` (e.g., empty lifecycle methods like `endInstance`), it may be intentional to include an explicit `pass` statement even if the method already contains a docstring. When reviewing, do not flag this as redundant (Ruff `PIE790`) for these empty stub lifecycle methods when the `pass` is used to clearly mark the body as intentionally empty.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-03-30T21:57:11.296Z
Learnt from: chinesepowered
Repo: rocketride-org/rocketride-server PR: 518
File: nodes/src/nodes/llm_openai_api/openai_client.py:25-32
Timestamp: 2026-03-30T21:57:11.296Z
Learning: In Python LLM node implementations under `nodes/**/*.py`, it’s an established pattern that `ChatBase.__init__` may not store all configuration on the instance. Subclasses may intentionally call `Config.getNodeConfig(provider, connConfig)` again inside their own `__init__` to read provider-specific fields (e.g., `apikey`, `base_url`). During code review, do not flag this as a redundant duplicate call when it’s used to obtain provider-specific configuration for the subclass.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-03-30T22:18:48.072Z
Learnt from: chinesepowered
Repo: rocketride-org/rocketride-server PR: 518
File: nodes/src/nodes/llm_openai_api/IGlobal.py:11-81
Timestamp: 2026-03-30T22:18:48.072Z
Learning: In this repository’s Python LLM node implementation files under `nodes/**/*.py`, follow the established `validateConfig` style: use the existing “monolithic probe + error-formatting” approach for provider-first exceptions to keep behavior consistent. When linting flags require suppression, prefer the smallest/most targeted Ruff `# noqa` usage (e.g., `PLR0912`/`PLR0915`) and add specific justification comments for any targeted `BLE001`/similar codes rather than refactoring into multiple helper functions solely to satisfy linting. Only refactor the exception/validation pattern if you can update it consistently across all similar nodes.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-02T13:09:51.916Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 583
File: nodes/src/nodes/audio_tts/IGlobal.py:236-264
Timestamp: 2026-04-02T13:09:51.916Z
Learning: In RocketRide, filter node code (e.g., implementations under `nodes/**/*.py`) should assume `IGlobal.synthesize()` and filter node methods are executed sequentially per pipeline instance: one document is processed through the filter at a time. Concurrent access to the same `IGlobal` instance is not an expected runtime scenario. Therefore, do not raise review findings about missing synchronization/locks (e.g., missing mutex/atomic protections) for shared `IGlobal` state in filter node implementations unless there is explicit evidence of concurrent access beyond the framework’s pipeline execution model.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-03T04:00:08.250Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 599
File: nodes/src/nodes/tool_firecrawl/IInstance.py:103-103
Timestamp: 2026-04-03T04:00:08.250Z
Learning: When reviewing Python files under `nodes/**/*.py`, do not flag Ruff Q000 (single-quote style) violations for double-quoted string literals if the content contains a single quote/apostrophe (e.g., `"Map a website's structure..."`). In such cases, double quotes are intentional to avoid backslash escapes and are considered correct under Ruff’s Q000 behavior.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-03T05:20:16.219Z
Learnt from: nihalnihalani
Repo: rocketride-org/rocketride-server PR: 525
File: nodes/src/nodes/router_llm/IInstance.py:0-0
Timestamp: 2026-04-03T05:20:16.219Z
Learning: When reviewing Python code under the `nodes/` package, treat the `IInstanceBase.open()` lifecycle method’s parameter named `obj: Entry` as the correct engine Python-bridge interface convention. Do not flag `obj` as a non-standard parameter name. This convention exists specifically because the earlier parameter name `object` (which shadowed the Python builtin) was replaced with `obj`.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-03T18:31:22.345Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 599
File: nodes/src/nodes/memory_internal/IInstance.py:33-35
Timestamp: 2026-04-03T18:31:22.345Z
Learning: In RocketRide pipeline engine nodes (nodes/**/*.py), rely on the lifecycle invariant: `beginInstance()` is guaranteed to complete successfully before `open()` runs, and if `beginInstance()` throws, the engine aborts and `open()` is never called. Therefore, during code review, do not flag instance fields that are initialized in `beginInstance()` (e.g., `self._store`) as potentially `None`/uninitialized when they are referenced in `open()`. This is safe because the framework lifecycle enforces it.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-03T18:31:20.227Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 599
File: nodes/src/nodes/llm_base/IInstance.py:36-50
Timestamp: 2026-04-03T18:31:20.227Z
Learning: In Python pipeline node files under `nodes/**/*.py` (including `nodes/src/nodes/llm_base/IInstance.py`), do not flag missing PEP 257 docstrings for trivial one-liner methods decorated with `invoke_function` (or similar decorator-based invocation). Skip the docstring requirement when the method’s name and return value/behavior make the contract self-evident (e.g., `getContextLength`, `getOutputLength`, `getTokenCounter`, `ask`), because the decorator plus the method name already communicate intent clearly.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-03T18:31:34.956Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 599
File: nodes/src/nodes/tool_firecrawl/IInstance.py:67-69
Timestamp: 2026-04-03T18:31:34.956Z
Learning: For Python tools implemented with rocketlib’s `tool_function` decorator, the framework validates the declared `input_schema` at `tool.invoke` before entering the tool method body. Therefore, do not require (or flag) redundant runtime type checks inside the tool (e.g., `isinstance(url, str)` when the schema declares `url` as a string). Instead, only add lightweight value guards for schema-compatible edge cases (e.g., `if not url:` to handle the empty-string case) and avoid duplicating schema-based type validation logic.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-08T07:11:59.835Z
Learnt from: charliegillet
Repo: rocketride-org/rocketride-server PR: 509
File: nodes/src/nodes/tool_exa_search/exa_driver.py:146-198
Timestamp: 2026-04-08T07:11:59.835Z
Learning: In RocketRide tool driver implementations under nodes/**/*.py that use ToolsBase, ensure _tool_validate normalizes inputs exactly the same way as _tool_invoke (via a shared _normalize_tool_input helper). This must allow inputs consistently when ToolsBase.handle_invoke triggers validation directly—specifically, JSON strings, Pydantic-like models, and wrapped payloads like {"input": ...} should be accepted and normalized prior to validation.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-23T09:43:35.158Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 662
File: nodes/src/nodes/audio_tts/IInstance.py:41-43
Timestamp: 2026-04-23T09:43:35.158Z
Learning: In rocketride-server pipeline node code under nodes/**/*.py, treat writeAudio(AVI_ACTION.BEGIN, mimeType) and writeAudio(AVI_ACTION.END, mimeType) as an intentional established pattern: they should be called with exactly two arguments (no buffer parameter). Only AVI_ACTION.WRITE calls should include the buffer. Do not raise a review issue for a missing buffer argument on BEGIN/END writeAudio calls, as this matches the existing codebase behavior.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-05-18T11:51:47.483Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 889
File: nodes/src/nodes/llm_vision_mistral/mistral_vision.py:250-252
Timestamp: 2026-05-18T11:51:47.483Z
Learning: In OpenAI-compatible provider implementations (including nodes/src/nodes/llm_vision_mistral/mistral_vision.py and other provider files under nodes/), do not flag `chat_response.choices[0]` as missing a guard after a successful SDK call (e.g., after a 2xx response). These SDKs populate `choices` on success, and any unexpected `IndexError` would be caught by the surrounding `except Exception` retry/error-handling chain (which passes through `_shouldRetry` and formats via `_format_user_error`). Only raise an issue if the code reaches `choices[0]` without having established the successful-response/error-handling assumptions that the existing retry wrapper relies on.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-05-20T14:02:50.466Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 690
File: nodes/src/nodes/flow_base/IInstance.py:122-129
Timestamp: 2026-05-20T14:02:50.466Z
Learning: In rocketride-server Python pipeline node implementations under `nodes/**/*.py`, the AVI streaming inbound handler methods `writeImage`, `writeAudio`, and `writeVideo` are invoked by the C++ engine trampoline (e.g., `python-instance.avi.cpp`) and must use the required 3-parameter signature: `def writeImage(self, action: int, mimeType: str, buffer: bytes) -> None` (and analogous signatures for the other methods). For `BEGIN` and `END` actions, `buffer` is still passed and is an empty bytes value (`b''`) rather than being omitted. When reviewing similar inbound handlers in sibling node modules (e.g., `ocr`, `thumbnail`, `llm_vision_*`, `embedding_image`, `accessibility_describe`, `image_cleanup`, `flow_base`), do not flag the required `buffer` parameter as inconsistent with outbound call sites like `pipe.writeImage(AVI_ACTION.BEGIN, mime)`, since those are different outbound write APIs and are not the same as inbound handler call signatures.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-06-13T07:37:27.809Z
Learnt from: asclearuc
Repo: rocketride-org/rocketride-server PR: 1267
File: nodes/src/nodes/landing_ai/parse/IInstance.py:42-42
Timestamp: 2026-06-13T07:37:27.809Z
Learning: When reviewing Python code in nodes/**/*.py, do not flag double-quoted string literals that occur inside f-string expression slots (the `{...}` portion). For example, `f'... {hasattr(obj, "fileName")}...'` is acceptable: using double quotes inside the expression is idiomatic and does not conflict with any single-quote convention that only applies to the outer f-string/overall string delimiter.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-02T20:29:16.277Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 599
File: nodes/src/nodes/db_neo4j/IInstance.py:158-163
Timestamp: 2026-04-02T20:29:16.277Z
Learning: When reviewing Python code in this repository, do not treat Ruff rule RET505 ("Unnecessary elif after return statement") as a review finding. Maintainers consider this nitpick unnecessary, and it is acceptable to keep elif/else branching even immediately after a return.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
  • packages/ai/src/ai/common/store.py
  • packages/ai/tests/ai/common/test_store.py
📚 Learning: 2026-06-05T22:02:43.464Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 1138
File: packages/ai/src/ai/web/server.py:266-275
Timestamp: 2026-06-05T22:02:43.464Z
Learning: Use `debug()` imported from `rocketlib` (e.g., `from rocketlib import debug`) as the standard logging utility for diagnostic output in this codebase (including warnings and misconfiguration notices). Do not recommend replacing existing `debug()` calls with Python’s built-in `logging` (e.g., `logger.warning()` / `logging.warning()`) since maintainers intentionally rely on `rocketlib.debug` for these messages.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
  • packages/ai/src/ai/common/store.py
  • packages/ai/tests/ai/common/test_store.py
📚 Learning: 2026-04-09T17:54:10.282Z
Learnt from: ryan-t-christensen
Repo: rocketride-org/rocketride-server PR: 639
File: nodes/src/nodes/milvus/README.md:0-0
Timestamp: 2026-04-09T17:54:10.282Z
Learning: In `milvus.py`, ensure the `retrieval_score_threshold` configuration value is actually enforced when filtering semantic search results. Specifically, in `searchSemantic`, do not return hits solely based on retrieval—filter out (or otherwise exclude) results whose similarity/score is below `retrieval_score_threshold` (default `0.5`) so the config meaningfully controls the returned semantic hits.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-04-15T00:13:11.455Z
Learnt from: ryan-t-christensen
Repo: rocketride-org/rocketride-server PR: 673
File: nodes/src/nodes/llm_vision_anthropic/IInstance.py:91-92
Timestamp: 2026-04-15T00:13:11.455Z
Learning: In the rocketride-server pipeline, the Image document producers (frame_grabber, thumbnail, embedding_image) normalize produced image outputs to PNG via ImageProcessor. Therefore, when building image data URIs from Doc.page_content in any nodes Python code, it is correct and intentional to hardcode the MIME type to `image/png`. Do not raise/flag a potential MIME type mismatch for this specific PNG-normalized flow.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-05-18T11:51:47.483Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 889
File: nodes/src/nodes/llm_vision_mistral/mistral_vision.py:250-252
Timestamp: 2026-05-18T11:51:47.483Z
Learning: When reviewing these OpenAI-compatible LLM vision/provider node implementations, do not flag direct access to `chat_response.choices[0]` after a successful SDK call as missing an empty-list guard. The relevant SDKs are expected to populate `choices` on 2xx responses, while 4xx/5xx/rate limit/auth failures raise exceptions before reaching `choices[0]`. If `choices` were unexpectedly empty, any resulting `IndexError` should be handled by the surrounding `except Exception` retry flow, which routes through `_shouldRetry` (returning False) and then `_format_user_error`, allowing the error to propagate cleanly to the pipeline. If cross-provider hardening is later required, implement it in a dedicated PR using a shared helper applied uniformly across provider nodes rather than adding ad-hoc guards per file.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-05-20T14:18:46.136Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 690
File: nodes/src/nodes/flow_llm/IInstance.py:72-75
Timestamp: 2026-05-20T14:18:46.136Z
Learning: When calling LLMs from Python nodes in this repo, do not expect or add per-call timeout parameters to `IInvokeLLM.Ask` or `instance.invoke(...)` (these APIs carry only fields like `lane`, `op`, `question`, and do not provide a `timeout` argument). Instead, rely on the wired LLM provider node to enforce request timeouts. If Python-side timeout behavior is required for an LLM call, implement it by running `instance.invoke(...)` in a separate worker (e.g., `threading.Thread` with `join(timeout=...)`) and treating a non-finished join as a timeout, rather than trying to pass a timeout via the LLM API.

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
📚 Learning: 2026-06-15T16:17:16.443Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 1278
File: nodes/test/test_vectordb_tool_mixin.py:380-380
Timestamp: 2026-06-15T16:17:16.443Z
Learning: In rocketride-server, for any pipeline node class methods decorated with `tool_function` under `nodes/**/*.py` and `packages/ai/**/*.py`, require that the tool method name is a bare method name (e.g., `search`, `upsert`, `delete`, `objdir`, `get`, `stats`) rather than a self-namespaced string. The agent framework will automatically namespace tools with a node-ID prefix at tool-discovery time (e.g., `<nodeId>.search`), so reviewers should not flag these bare names or suggest naming methods like `<serverName>.search` (or otherwise adding prefixes inside the node class itself).

Applied to files:

  • nodes/src/nodes/milvus/milvus.py
  • packages/ai/src/ai/common/store.py
  • packages/ai/tests/ai/common/test_store.py
📚 Learning: 2026-04-02T20:34:40.920Z
Learnt from: Rod-Christensen
Repo: rocketride-org/rocketride-server PR: 600
File: packages/ai/src/ai/account/file_store.py:273-286
Timestamp: 2026-04-02T20:34:40.920Z
Learning: In Python asyncio code, atomicity/interleaving concerns should be suppressed when two sequential statements in the same coroutine have no `await` between them. For example, a check immediately followed by an action (with no `await` in between) should be treated as effectively atomic with respect to other coroutines, since no other task can run until an `await` point is reached. Do not raise interleaving/TOCTOU concerns for such contiguous, await-free sequences.

Applied to files:

  • packages/ai/src/ai/common/store.py
📚 Learning: 2026-05-18T11:43:08.452Z
Learnt from: dsapandora
Repo: rocketride-org/rocketride-server PR: 889
File: packages/ai/src/ai/common/llm_native_stream.py:234-242
Timestamp: 2026-05-18T11:43:08.452Z
Learning: When reviewing the AI streaming/retry logic in packages/ai/src/ai/common (notably ChatBase.chat_string and the native chat stream dispatch/handlers), ensure the “no duplicate/fallback request after partial emission” guard is centralized in ChatBase.chat_string: it should wrap on_chunk with an emitted-state closure (e.g., emitted={'any': False}) before calling dispatch_native_chat_stream, and after the dispatch returns, if native_text is None while emitted['any'] is True, it should call on_finish('error') and return '' without issuing a second request. Native handler implementations should return None unconditionally on failure without tracking emission state internally (the caller is responsible for the emitted-but-failed case). Also, only gate/decide fallback behavior based on on_chunk (user-visible text). Do not gate on_reasoning_chunk, since reasoning content renders separately and a non-streaming retry can overwrite it safely.

Applied to files:

  • packages/ai/src/ai/common/store.py
📚 Learning: 2026-06-01T21:15:35.971Z
Learnt from: stepmikhaylov
Repo: rocketride-org/rocketride-server PR: 1044
File: packages/ai/tests/ai/account/test_store.py:425-429
Timestamp: 2026-06-01T21:15:35.971Z
Learning: When reviewing test fixtures that clean up S3/Azure test storage (e.g., deleting objects/blobs by a prefix like `Prefix='tmp_'`), don’t treat prefix-based cleanup as risky to other concurrent CI runs. In `rocketride-org/rocketride-server`, the CI workflow `.github/workflows/_build.yaml` provisions isolated MinIO (S3 tests) and Azurite (Azure Blob tests) instances per CI run on Ubuntu, so each run has separate storage. Therefore, broad prefix cleanup within the test fixtures should not delete data from other concurrent CI runs (and tests within a run are sequential).

Applied to files:

  • packages/ai/tests/ai/common/test_store.py
📝 Walkthrough

Walkthrough

DocumentStoreBase.createCollection now aborts when collection creation returns False, while driver exceptions propagate. Milvus no longer swallows creation errors, and regression tests cover failure, success, and exception paths.

Changes

Collection creation failure handling

Layer / File(s) Summary
Base creation failure contract
packages/ai/src/ai/common/store.py
Documents driver failure signaling and raises before indexing when _createCollection returns exactly False.
Driver propagation and validation
nodes/src/nodes/milvus/milvus.py, packages/ai/tests/ai/common/test_store.py
Milvus propagates collection creation exceptions, with tests covering false returns, successful creation, and propagated exceptions.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

Suggested reviewers: jmaionchi, rod-christensen, stepmikhaylov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: honoring _createCollection() results before indexing.
Linked Issues check ✅ Passed The base-store fix and Milvus adjustment directly address #1495 by stopping indexing on creation failure.
Out of Scope Changes check ✅ Passed The Milvus update and added tests are in scope as supporting fixes for the shared createCollection bug.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ai AI/ML modules module:nodes Python pipeline nodes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Base store createCollection() ignores _createCollection() failure (silent/cryptic crash across all vector stores)

1 participant