feat(chroma): expose tunable top_k retrieval limit

[krishhgg]

Summary

• Add an optional Top K (top_k) retrieval limit to the Chroma node — there was previously no knob to widen the candidate pool (the data lane always fetched the DocFilter default of 25), which is the single biggest lever for recall on precise, number-dense queries (field report I-4/I-8).
• top_k applies to similarity search only (searchSemantic/searchKeyword); get() deliberately keeps the caller's limit because it also serves full-table/full-document rehydration, which must never be truncated. Unset top_k ⇒ prior behavior exactly.
• Document reranking: rerank_cohere wiring section, a "Retrieval tuning" section in the Chroma README, and a runnable examples/rag-rerank-pipeline.pipe wiring chroma (top_k: 20) → rerank_cohere (top_n: 5) → llm — retrieve generously, rerank down.
• Fix the Chroma contract-test score assertion: the test framework implements only greaterThan (nodes/test/framework/expectations.py), so the previous greaterThanOrEqual: 0.5 assertion was silently ignored; it's now greaterThan: 0.49 and actually runs.

⚠️ Descope from the previously-approved revision

The earlier revision of this PR also exposed the score threshold and removed the hardcoded 0.20 floor in _convertToDocs. That part was removed on purpose: Chroma returns distances (lower = more similar; cosine distance = 1 − cosine similarity, range [0, 2]), but _convertToDocs maps larger distances to larger scores ((distance + 1) / 2 for cosine, and a sigmoid increasing in distance otherwise). A user-facing "minimum score" threshold on top of that is semantically reversed — it would preferentially drop the most similar documents. Proper score normalization needs its own design per metric (cosine / L2 / inner product) and will be proposed separately. The floor and score handling in this diff are byte-identical to develop.

How

• Store.top_k config field (validated by _coerceTopK: positive whole numbers only, bool rejected, unset/blank ⇒ None).
• _effectiveLimit(docFilter): configured top_k wins, else the caller's docFilter.limit.
• Schema: vector.top_k added to services.common.vector.json and to Chroma's local/cloud profiles. vector.score is not exposed.

Type

feat

Testing

• Tests added or updated — nodes/test/chroma/test_retrieval_tuning.py (10 cases: _coerceTopK validation, _effectiveLimit precedence/fallback, top_k override in semantic + keyword search, get() ignores top_k, and a regression test that the 0.20 floor still applies)
• Tested locally — full nodes/test/chroma/ suite 39 passed (includes fix(nodes): friendlier chroma port + splitter-profile config (RR-1416) #1496's test_coerce_port.py; branch is rebuilt on current develop); ruff check + ruff format --check clean
• ./builder test passes — runs in CI (node-source edits force a full engine recompile locally)

Checklist

• Commit messages follow conventional commits
• No secrets or credentials included (example uses ${ROCKETRIDE_*} placeholders)
• Wiki updated (if applicable) — node READMEs updated in-repo
• Breaking changes documented (if applicable) — none; unset top_k ⇒ prior behavior

Linked Issue

Part of #1411 (score-threshold + hybrid retrieval remain open; score normalization to be designed separately)

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR adds configurable top_k retrieval for Chroma similarity searches, exposes and documents the setting, adds regression tests, and includes a runnable RAG and Cohere reranking pipeline example.

Changes

Chroma top_k retrieval tuning

Layer / File(s)	Summary
Store top_k and retrieval limit resolution nodes/src/nodes/chroma/chroma.py	Adds validated top_k configuration, applies it to semantic and keyword candidate retrieval, and preserves DocFilter.limit for direct fetches.
Retrieval configuration and rerank wiring nodes/src/nodes/core/services.common.vector.json, nodes/src/nodes/chroma/services.json, nodes/src/nodes/chroma/README.md, nodes/src/nodes/rerank_cohere/README.md	Defines and exposes vector.top_k, updates the similarity assertion, and documents retrieval tuning and reranker placement.
Retrieval tuning tests nodes/test/chroma/test_retrieval_tuning.py	Tests score handling, limit resolution, search overrides, direct fetch behavior, and top_k validation using scoped dependency stubs.
Example RAG and rerank pipeline examples/rag-rerank-pipeline.pipe	Adds ingestion, embedding, Chroma retrieval, Cohere reranking, prompting, OpenAI generation, and response formatting.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Chat
  participant Chroma
  participant CohereRerank
  participant Prompt
  participant LLMOpenAI

  Chat->>Chroma: question embedding and top_k retrieval
  Chroma-->>CohereRerank: candidate documents
  CohereRerank-->>Prompt: reranked documents
  Prompt->>LLMOpenAI: question and context prompt
  LLMOpenAI-->>Chat: formatted answers

Loading

Suggested labels: module:ai

Suggested reviewers: jmaionchi, stepmikhaylov, rod-christensen, joshuadarron

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: exposing a configurable top_k retrieval limit for Chroma.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch kgarg2468/fix-1411-chroma-retrieval

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nodes/src/nodes/chroma/services.json`:
- Line 119: The score assertion in the Chroma service test is too loose and
could hide regressions in score computation. Tighten the expectation in the
services.json test case that checks the `[0].score` result so it asserts the
expected near-0.5 value with minimal floating-point tolerance, rather than using
a broader lower bound. Keep the change localized to the score check in the
Chroma query scenario so the test continues to validate identical-embedding
cosine scoring accurately.

In `@nodes/src/nodes/core/services.common.vector.json`:
- Around line 65-70: The `vector.top_k` schema currently uses a generic
`number`, which allows fractional values that later fail in `chroma.py`’s
`_coerceTopK` runtime coercion. Update the `vector.top_k` definition in
`services.common.vector.json` to accept only integers (while keeping the
existing minimum and description) so validation matches the `_coerceTopK`
contract and prevents invalid fractional inputs from reaching `chroma.py`.

In `@nodes/src/nodes/rerank_cohere/README.md`:
- Around line 53-55: The fenced diagram in the rerank_cohere README is missing a
language tag, which triggers markdownlint MD040. Update the diagram fence to use
a text language identifier for the block so the markdown linter recognizes it,
keeping the existing diagram content unchanged.

In `@nodes/test/chroma/test_retrieval_tuning.py`:
- Around line 229-257: Add a regression test for the store.get path to verify a
configured top_k does not change its limit or behavior. Use the existing
_make_store, MagicMock collectionObj, and doesCollectionExist setup alongside
the get method on the store, and assert that get continues to use the docfilter
limit or default behavior rather than being overridden by top_k, unlike
searchSemantic and searchKeyword.

🪄 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: 60ce6aa5-4c76-4313-a068-82311003d32d

📥 Commits

Reviewing files that changed from the base of the PR and between b4a461b and 1f128e1.

📒 Files selected for processing (7)

• examples/rag-rerank-pipeline.pipe
• nodes/src/nodes/chroma/README.md
• nodes/src/nodes/chroma/chroma.py
• nodes/src/nodes/chroma/services.json
• nodes/src/nodes/core/services.common.vector.json
• nodes/src/nodes/rerank_cohere/README.md
• nodes/test/chroma/test_retrieval_tuning.py

[coderabbitai]

♻️ Duplicate comments (2)

nodes/src/nodes/core/services.common.vector.json (1)

65-70: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Constrain vector.top_k to integers.

"type": "number" still permits fractional input (e.g. 2.5), but _coerceTopK in chroma.py raises ValueError for non-integer floats, surfacing as an unhandled error during Store.__init__. This was previously flagged as addressed, yet the type is still number.

🔧 Proposed fix

 		"vector.top_k": {
-			"type": "number",
+			"type": "integer",
 			"title": "Top K",
 			"minimum": 1,
 			"description": "Maximum number of candidate documents to retrieve before score filtering. Leave unset to use the pipeline default (25). Raise it to improve recall for precise, number-dense queries."
 		},

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nodes/src/nodes/core/services.common.vector.json` around lines 65 - 70, The
`vector.top_k` schema in `services.common.vector.json` still allows fractional
values because it is declared as `number`, which can reach `_coerceTopK` in
`chroma.py` and fail during `Store.__init__`. Update the `vector.top_k` property
to accept only integers (for example by changing the schema type used for this
setting) so the UI/config validation prevents non-integer input before it
reaches `Store.__init__` and `_coerceTopK`.

nodes/src/nodes/chroma/services.json (1)

119-119: 🎯 Functional Correctness | 🟡 Minor | 💤 Low value

Loosened score assertion may mask a scoring regression.

Identical doc/query embeddings yield an exact cosine score of ~0.5, so > 0.49 is wider than floating-point noise requires and could hide a real shift in score computation. Prefer asserting the value near 0.5 with a minimal tolerance.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nodes/src/nodes/chroma/services.json` at line 119, The score assertion in the
Chroma service test is too loose and could hide regressions in score
computation. Update the check around the "[0].score" assertion to verify the
score is near 0.5 with a small tolerance instead of using a broad greaterThan
threshold, so the test still passes for floating-point noise but fails on real
scoring shifts.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@nodes/src/nodes/chroma/services.json`:
- Line 119: The score assertion in the Chroma service test is too loose and
could hide regressions in score computation. Update the check around the
"[0].score" assertion to verify the score is near 0.5 with a small tolerance
instead of using a broad greaterThan threshold, so the test still passes for
floating-point noise but fails on real scoring shifts.

In `@nodes/src/nodes/core/services.common.vector.json`:
- Around line 65-70: The `vector.top_k` schema in `services.common.vector.json`
still allows fractional values because it is declared as `number`, which can
reach `_coerceTopK` in `chroma.py` and fail during `Store.__init__`. Update the
`vector.top_k` property to accept only integers (for example by changing the
schema type used for this setting) so the UI/config validation prevents
non-integer input before it reaches `Store.__init__` and `_coerceTopK`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 354d3a3a-298a-4a92-989c-2d2eb69a9f50

📥 Commits

Reviewing files that changed from the base of the PR and between c32f7f3 and 4390b7a.

📒 Files selected for processing (7)

• examples/rag-rerank-pipeline.pipe
• nodes/src/nodes/chroma/README.md
• nodes/src/nodes/chroma/chroma.py
• nodes/src/nodes/chroma/services.json
• nodes/src/nodes/core/services.common.vector.json
• nodes/src/nodes/rerank_cohere/README.md
• nodes/test/chroma/test_retrieval_tuning.py

[joshuadarron]

Review

Verdict: Approve. The two knobs land where the issue says they were missing, the floor removal is verified safe (arguably a fix in itself), and the change is carefully scoped to the similarity-search paths only.

Verified against the code (not just the diff)

• The 0.20 floor was near-dead code — and harmful where it wasn't. With real Chroma outputs, the cosine branch maps distances [0,2] to scores [0.5,1.5] and the sigmoid branch maps d ≥ 0 (l2) to [0.5,1) — neither can produce a score below 0.20, so for cosine/l2 the floor never fired. The one case that can go below 0.5 is inner-product (negative distances), where the sigmoid gives the best matches the lowest scores — meaning the old floor was silently dropping the strongest IP matches. Removing it is a fix there, not just a cleanup. (The inverted IP/cosine score orientation itself is pre-existing and untouched — worth a separate look someday, but correctly out of scope here.)
• Data lane stays gated: DocumentStoreBase._addDoc (store.py:288) drops doc.score < self.threshold_search, and threshold_search now actually reads the newly-exposed score field. The claim that Chroma's config.get('score', 0.5) was frozen checks out — vector.score was absent from both chroma profile property lists.
• get() exclusion is right: it serves full-table/document rehydration; truncating it with top_k would corrupt whole-object fetches. The comment documents this at the site.
• Assertion-op fix verified: nodes/test/framework/expectations.py implements greaterThan/lessThan and silently ignores unknown keys — so the old greaterThanOrEqual really was a no-op assertion, and greaterThan: 0.49 restores it.
• _coerceTopK rejects bools (int subclass), non-integral floats, and < 1 — validation at the config boundary, with tests for each shape.

Non-blocking observations

• The README sentence "…and the search tool's own score_threshold on the tool path" slightly oversells: _vectordb_score_threshold() reads IGlobal.score_threshold, which Chroma's IGlobal never sets and services.json doesn't expose — so the tool-path gate defaults to 0.0 (off). Harmless in practice given the score ranges above, but if you want the tool path genuinely tunable, exposing that knob would be a small follow-up.
• top_k also overrides the limit on searchKeyword, which supports offset paging — a paginating caller would get top_k-sized pages. Documented behavior, just noting the interaction.
• _effectiveLimit returns docFilter.limit untouched when top_k is unset, so existing pipelines are bit-for-bit unchanged. Confirmed.

CI: builds, ruff, gitleaks, CodeRabbit all green.

[github-actions]

🤖 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.

[krishhgg]

@joshuadarron heads-up: this PR changed materially after your approval, so it needs a fresh look — sorry for the churn, but we caught a real problem before it shipped.

What changed: rebuilt on current develop (resolves the conflict with #1496) and descoped to top_k only. The tunable score threshold + 0.20-floor removal are gone.

Why: Chroma returns distances (lower = more similar; cosine distance = 1 − similarity, range [0, 2]), but _convertToDocs maps larger distances to larger scores ((distance + 1) / 2 for cosine; a distance-increasing sigmoid otherwise). A user-facing minimum-score threshold on top of that is semantically reversed — it would preferentially drop the most similar documents. (The earlier revision's own test inadvertently proved this: it used a cosine distance of −0.9, which can't occur.) Score normalization needs a per-metric design (cosine / L2 / inner product) and will come as its own proposal.

What stayed: the top_k knob (similarity search only — new test proves get() ignores it, protecting full-document rehydration), the rerank docs + example pipeline (example no longer sets score), and the contract-test fix (greaterThanOrEqual is silently ignored by the test framework — only greaterThan is implemented — so the score assertion now actually runs).

Locally: 39/39 chroma tests green on the rebuilt branch, ruff clean. The PR description has the full details.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

nodes/src/nodes/rerank_cohere/README.md (1)

62-64: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low value

Add a language tag to the fenced diagram.

Markdownlint flags this block (MD040). text is enough here.

♻️ Proposed fix

-```
+```text
 … → embedding → store ──questions──▶ rerank_cohere ──documents──▶ prompt → llm → response

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @nodes/src/nodes/rerank_cohere/README.md around lines 62 - 64, Add the text
language tag to the fenced diagram in the README while preserving the diagram
content unchanged.

</details>

<!-- cr-comment:v1:074bcad3ca6579eca5b81fca -->

_Source: Linters/SAST tools_

</blockquote></details>
<details>
<summary>nodes/src/nodes/core/services.common.vector.json (1)</summary><blockquote>

`65-70`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_

**Consider constraining `vector.top_k` to integers.**

The schema type is `number`, allowing fractional input (e.g. `2.5`), but `nodes/src/nodes/chroma/chroma.py`'s `_coerceTopK` (per upstream contract snippet) raises `ValueError` for non-integer floats. A value that passes schema validation could still fail at runtime with an unhandled coercion error.





<details>
<summary>🔧 Proposed fix</summary>

```diff
 		"vector.top_k": {
-			"type": "number",
+			"type": "integer",
 			"title": "Top K",
 			"minimum": 1,
 			"description": "Maximum number of candidate documents to retrieve before score filtering. Leave unset to use the pipeline default (25). Raise it to improve recall for precise, number-dense queries."
 		},

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@nodes/src/nodes/core/services.common.vector.json` around lines 65 - 70,
Update the vector.top_k schema definition to reject fractional values by
requiring integer numbers while preserving its existing minimum and default
behavior. Use the top_k property in the vector service schema; do not change the
runtime coercion logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nodes/src/nodes/chroma/chroma.py`:
- Line 74: Remove the unused self.threshold_search assignment from the Chroma
node initialization, leaving the surrounding configuration handling unchanged.

---

Duplicate comments:
In `@nodes/src/nodes/core/services.common.vector.json`:
- Around line 65-70: Update the vector.top_k schema definition to reject
fractional values by requiring integer numbers while preserving its existing
minimum and default behavior. Use the top_k property in the vector service
schema; do not change the runtime coercion logic.

In `@nodes/src/nodes/rerank_cohere/README.md`:
- Around line 62-64: Add the `text` language tag to the fenced diagram in the
README while preserving the diagram content unchanged.

🪄 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: 7e2f0396-6e4f-4c6a-973f-99443a783124

📥 Commits

Reviewing files that changed from the base of the PR and between 4390b7a and b30b3f6.

📒 Files selected for processing (7)

• examples/rag-rerank-pipeline.pipe
• nodes/src/nodes/chroma/README.md
• nodes/src/nodes/chroma/chroma.py
• nodes/src/nodes/chroma/services.json
• nodes/src/nodes/core/services.common.vector.json
• nodes/src/nodes/rerank_cohere/README.md
• nodes/test/chroma/test_retrieval_tuning.py

[krishhgg]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[asclearuc]

Thanks for this, @kgarg2468 - really clean, well-scoped work. Approving. The top_k knob lands exactly where retrieval was un-tunable, the search-only / get()-excluded boundary is the right call and is documented at the call site, and unset top_k keeps existing pipelines bit-for-bit unchanged; the 10-case test file and green CI back it up.

I left two optional inline nits (reusing/justifying the shared config_int parser, and an optional upper bound on top_k). Two body notes: I agree with CodeRabbit's already-discussed Minor point that the loosened score assertion in services.json:119 could tighten from > 0.49 to > 0.499 (purely optional - your "framework no-op" explanation is correct). And one quick check before merge: please confirm builder docs:build is clean now that vector.top_k is in the profile property lists, since the generated PARAMS table in store_chroma/README.md does not list it (though vector.collection/vector.apikey are also absent, so this may just be expected generator behavior). Nothing here blocks.

[joshuadarron]

Fix the merge conflicts.

Dismissing as stale: the merge conflicts this review asked for were resolved by merging develop into the branch (77b0172), and GitHub now reports the PR as mergeable.

[nihalnihalani]

Both approvals were dismissed by the develop merge in 77b0172, which is why this is sitting at blocked with no red checks - it needs a fresh approving review rather than any code change.

On @asclearuc's remaining pre-merge question (whether builder docs:build is clean now that vector.top_k is in the profile property lists): it is, and no regeneration is needed. gen-node-tables.mjs builds the schema block from readdirSync(dir).filter(f => /^services.*\.json$/.test(f)) over the node's own directory only, so a field declared in nodes/src/nodes/core/services.common.vector.json cannot appear in store_chroma's generated table. That is the same reason vector.collection and vector.apikey are already missing from it on develop - his own hunch, confirmed.

One thing to watch on the way in: #1768 rewrites the same two lines of services.json (chroma.cloud.properties and chroma.local.properties) to add tenant/database. Whichever of the two merges second will have to combine those arrays by hand.

[krishhgg]

Pushed e7c8928 (merge) and 0fc020b1 (review fixes). Rebuilt on current develop, conflicts resolved, all open review points addressed.

Merge conflict

Exactly the collision @nihalnihalani flagged: #1768 added chroma.tenant / chroma.database / chroma.ssl to chroma.cloud.properties while this branch added vector.top_k. Both arrays kept, vector.top_k after vector.collection to mirror chroma.local.properties:

"properties": ["vector.cloud.host", "vector.cloud.port", "vector.apikey", "chroma.tenant", "chroma.database", "vector.collection", "vector.top_k", "chroma.serverName", "chroma.ssl"]

That was the only conflict; chroma.py and README.md auto-merged. To stop this recurring silently, the new test file asserts both profiles still list vector.top_k — a bad conflict resolution now fails a test instead of just dropping the knob from the editor.

Review fixes

@asclearuc — upper bound. Capped: Store.MAX_TOP_K = 1000, enforced in _coerceTopK, mirrored as "maximum": 1000 on the schema. 1000 rather than the tool's 100 because the knob exists to widen the pool before a reranker — 40x the data-lane default, 10x the tool cap. Rejected rather than clamped, so top_k: 10000000 errors at startup instead of quietly becoming something else.

@asclearuc — config_int DRY. Kept the local helper, documented why: config_int never returns None (so "unset" could not fall back to docFilter.limit), treats <= 0 as unspecified, and clamps instead of raising. The docstring now ends with an explicit "do not simplify this to config_int".

@asclearuc + CodeRabbit — score assertion. Now two-sided, greaterThan: 0.499 + lessThan: 0.501. Both matchers fire from one property entry; verified against the real validator (0.5 and 0.5±1e-7 pass; 0.49, 0.499, 0.501, 0.9, 1.0 all fail). The old one-sided bound tolerated drift down to 0.491 and any drift upward.

@asclearuc — builder docs:build. Confirmed clean, no regeneration needed, and @nihalnihalani's reasoning holds. Two independent gates in nodes/scripts/gen-node-tables.mjs: it reads only readdirSync(dir).filter(f => /^services.*\.json$/.test(f)) over the node's own directory, so a field declared in core/services.common.vector.json cannot reach store_chroma's generated table (same reason vector.collection / vector.apikey are absent on develop) — and main() returns early on any branch outside main/stage/develop. The top_k row in the README is in the hand-written Fields table, outside the ROCKETRIDE:GENERATED:PARAMS region (lines 110–135).

Testing

Ran against a local env emulating the engine runtime (real rocketlib / ai.common / chromadb-client / numpy, native engLib stubbed):

• nodes/test/store_chroma/ — 91 passed, up from 39. The pre-existing 82 pass unchanged on the merge result, confirming the merge itself is clean.
• All seven vector-store nodes — 165 passed, 1 skipped (pre-existing todo: skip in milvus). The vector.top_k schema edit is in a shared file, so astra / atlas / milvus / pinecone / qdrant / weaviate were run too; vector.top_k is referenced only by store_chroma's two profiles.
• ruff check nodes/ and ruff format --check clean.
• Score assertion driven through ExpectationValidator directly (table above).
• examples/rag-rerank-pipeline.pipe re-validated against current develop: every provider resolves and every profile still exists (preprocessor_langchain.default, embedding_transformer.miniLM, chroma.local, rerank_cohere.rerank-english-v3.0, llm_openai.openai-4o), and the lane wiring is legal — chroma questions → questions, rerank questions → documents. Its top_k: 20 is well inside the new bound.

New tests

nodes/test/store_chroma/test_services_top_k.py (4 cases, pure JSON reads, no engine build) pins the schema to the runtime contract: vector.top_k stays integer, its minimum/maximum match Store.MAX_TOP_K (parsed from chroma.py via ast, so the two files cannot drift apart), and both Chroma profiles list it.

Plus in test_retrieval_tuning.py: test_coerce_top_k_rejects_above_max, test_coerce_top_k_rejects_rather_than_clamps, and MAX_TOP_K added to the valid-value parametrize.

Not blocking, but worth knowing

Both approvals were dismissed by the develop merge, and this push adds another commit — so it needs a fresh look regardless. @joshuadarron @asclearuc when you have a moment.

[asclearuc]

Thanks @kgarg2468 — the rebuilt branch answers everything from the last round. MAX_TOP_K is mirrored in the schema and pinned by a test, the config_int decision is written down in the docstring, the score assertion is two-sided instead of a silently-ignored no-op, and CodeRabbit's Major finding (a regression test for get()) is covered by test_get_ignores_top_k_and_uses_docfilter_limit. Both of my earlier nits are resolved.

Request changes

Three non-blocking comments are inline. One more finding cannot be an inline comment, because it points at an unchanged line (and CodeRabbit already has a thread on the changed line next to it), so it is here:

---

MUST fix — the new score assertion pins what looks like an inverted relevance scale.

nodes/src/nodes/store_chroma/services.json:119 now asserts that a document whose embedding is identical to the query scores 0.5. That is a perfect match. On a 0..1 relevance scale it should score 1.0.

The cause is chroma.py:419, unchanged by this PR:

if self.similarity == 'cosine':
    score = (results['distances'][i] + 1) / 2

Chroma returns a distance for hnsw:space: cosine — 1 - cosine_similarity, range 0..2. Qdrant's point.score is a similarity in -1..1, and nodes/src/nodes/store_qdrant/qdrant.py:307 uses the same (x + 1) / 2 line, correctly, there. Applied to a distance it reverses the order:

vectors	cosine distance	score today	score expected
identical	0	0.5	1.0
orthogonal	1	1.0	0.5
opposite	2	1.5	0.0

DocumentStoreBase._addDoc (packages/ai/src/ai/common/store/document_store.py:295) then drops every doc below threshold_search, so raising the Retrieval Score knob on a Chroma node discards the best matches and keeps the worst. The l2 branch on the next line has the same shape (d=0 gives 0.5, large d approaches 1.0), which is also why the score < 0.20 noise floor can never fire on the cosine path.

Fixing the formula is outside the scope of a top_k PR, and I am not asking you to widen it. But the new two-sided band makes 0.5 the expected value for a perfect match, so a later fix will now fail this test. Please either fix it here, or open an issue and add a one-line comment next to the assertion saying it pins current behaviour and linking the issue.

---

One verification command that settles the scale, if you want it before filing anything:

python -c "import chromadb; c=chromadb.Client().create_collection('t', metadata={'hnsw:space':'cosine'}); c.add(ids=['a'], embeddings=[[1.0,0.0]]); print(c.query(query_embeddings=[[1.0,0.0]], n_results=1)['distances'])"

[krishhgg]

Pushed b0356ab5. All four points addressed, plus a merge blocker your review led me to that nobody had spotted.

The MUST-fix: you're right, and I verified it empirically

I ran it against chromadb==1.5.9 (our pinned client) rather than reasoning from docs. Query [1, 0], hnsw:space: cosine:

vector	distance	current score	correct
identical	0.000000	0.5000	1.0000
45°	0.292893	0.6464	0.8536
orthogonal	1.000000	1.0000	0.5000
opposite	2.000000	1.5000	0.0000

Your table was exactly right. All three metrics are affected, not just cosine — l2 returns squared distance and sigmoid(d/100) is nearly flat (0.500 → 0.601 across a distance of 41), and ip returns 1 − inner_product, which goes negative for strong matches, so the best IP match scores 0.490 and gets dropped by the default threshold.

Taking your pin-and-file option, and here's the argument for it beyond scope discipline. similarity isn't exposed in services.json, so it's always cosine; vector.score isn't exposed either, so threshold_search is always the default 0.5. Current cosine scores are always >= 0.5, which means _addDoc's gate never drops anything today and the 0.20 floor never fires. Correct the formula to 1 - d/2 and that dormant gate becomes live:

threshold	accepted today	accepted after fix
0.5 (default)	everything	cos sim >= 0

So the "obvious" one-line fix silently cuts recall — inside a PR whose entire purpose is improving recall. It has to land with a decision on the default threshold and the floor.

Filed as #2236 with the reproduction, the full measured table, the migration analysis, and a note that store_weaviate has the same directional defect and store_milvus shares one non-cosine branch across metrics that run in opposite directions. The assertion now carries a comment marking it a compatibility pin and naming #2236 as the thing that must update it. _convertToDocs is byte-identical to develop — I checked with git diff.

One deliberate deviation: I put the comment above "property": [ rather than inside the array. Mine would have been the only comment inside an array in any node services.json, and the C++ loader parses these at startup — not worth the risk for a formatting preference. Verified both json5 and the test helper parse it to the identical tree.

Merge blocker found while acting on your nit — top_k was silently required

Chasing your "type: integer" point, I hit this: vector.top_k has no "optional": true, and engLib/store/services/services.cpp:480 reads

// Optional not specified, it is required

then appends it to required at :1293. top_k has no default, so it compiled to required-with-nothing-to-satisfy-it — the field is meant to be optional in every sense. Fixed, with test_top_k_is_optional as a regression guard citing that rule.

Related, not fixed here: vector.host, vector.port, vector.cloud.*, vector.local.* in the shared catalogue have the same shape. Chroma escapes it because store_chroma/services.json overrides both port fields with a default, but a store inheriting the bare entries would not. Worth its own audit — say the word and I'll file it.

The example was backwards

DocFilter.limit defaults to 25 (client-python/schema/doc_filter.py:96). The README said "raise it, for example to 20" and the new example used 20 — a 20% reduction. Example now uses 50, and the prose says top_k overrides in both directions, since a top_k of 3 also narrows a caller asking for 100. Thanks for the "avoid saying it always widens" framing; that's what surfaced it.

Other two

• Shared catalogue — keeping it Chroma-only per your second option. Hoisting the helper alone wouldn't deliver the feature; each driver's query methods would still need to call it. Description now leads with "Chroma only:".
• Test loader DRY — deferring, as you flagged it optional. The four loaders differ in dependency stubs and module lifetime, so collapsing them is a test-isolation change I'd rather not pair with a config-surface change given the sys.modules history in Test pollution: node tests leak core-module stubs (rocketlib/pydantic) into sys.modules #1640/feat(nodes): add tool_pipedrive CRM tool node #1687.

Verification

• nodes/test/store_chroma/ — 114 passed (91 before).
• All seven vector stores — 188 passed, 1 pre-existing skip (165 before). Ran these because the schema edit is in the shared catalogue.
• ruff check nodes/ + format clean.
• _coerceTopK exercised directly across 25 inputs (see the inline reply for the table).
• Example pipeline re-validated against current develop: every provider and profile resolves, lane wiring legal, top_k: 50 confirmed above the 25 default.

[krishhgg]

CI is green on `b0356ab5`: 17 passing, 4 skipped, only `discord-notify` (a notification job, not a gate) outstanding.

One note on the red Build / Windows Server 2022 in the run history, so it does not cost anyone a second look. It failed on two tests that this PR does not touch:

FAILED test_node_cases[store_pinecone:services:serverless-dense]
FAILED test_node_cases[store_postgres:services:local]
  RuntimeError: No subprocess events received for 300 seconds.
  Task stuck in state 2 (INITIALIZING)

A startup timeout, not an assertion. Both passed on macOS and Ubuntu at the identical commit, and store_chroma:services:local passed on Windows in the same run. The diff also cannot reach those nodes: vector.top_k is referenced by exactly two files repo-wide (services.common.vector.json and store_chroma/services.json), and grep -c top_k on the pinecone and postgres services.json returns 0 for both. I re-ran the job and it passed — Build / Windows Server 2022 pass 21m9s.

@asclearuc this is ready for another look whenever you have time. The scoring-inversion pin is the one judgement call I would most like you to sanity-check — reasoning is in the thread above, and the fix is tracked in #2236.

[krishhgg]

Pushed dc1c27b2 (merge of current develop, 39 commits) and adadf97f (docs fixes). The branch was conflicting with develop again after #2139 landed; it is MERGEABLE now.

What #2139 changed for this PR

The documentation revamp added scripts/validate-node-readme.py and a docs-schemas.yml gate that now runs on this PR. Two things in the previous head would have failed it: ## Retrieval tuning in the Chroma README and ## Wiring in the reranker README are non-canonical ## headings, and the hand-written Fields table is forbidden (the generated ## Schema block owns field rows). So the top_k docs are re-homed rather than re-merged:

• store_chroma takes develop's layout, with a ### Top K subsection under ## Configuration.
• rerank_cohere: the wiring guidance moves under ## Notes. It also had a real error: it called top_k: 20 a "higher" pool, but DocFilter.limit defaults to 25 on the data lane, so following it narrowed retrieval by 20%. It now recommends 50, says explicitly that only Chroma exposes Top K, and spells out that 20 is below the default.
• core: vector.top_k is added to the hand-written common-vector table and the generated block is regenerated with nodes:docs-generate. One thing to be aware of: regeneration also emitted the gcp and microsoft tables, because those catalogues were added after the block was last generated. That is the tool's output, not a hand edit, and hand-trimming a generated block is what the co-located docs rule forbids, so I kept it and am flagging it here.

Untouched: chroma.py, services.json, the compatibility pin and its #2236 link.

Verified locally

validate-node-readme.py --all      every node PASS (0 failed, 0 warnings)
docs:test / docs:build / docs:check all pass
nodes/test/store_chroma             114 passed
nodes/test/store_*                  188 passed, 1 pre-existing skip

[asclearuc]

Thanks @kgarg2468 — adadf97f is a clean landing. The retrieval code is byte-for-byte what I approved last round; this push only merges develop and re-homes the documentation to the README schema that #2139 introduced. The reranker guidance now recommends 50 instead of 20, which is the right side of the caller's 25 default, and I checked that the gcp and microsoft tables you picked up sit inside the ROCKETRIDE:GENERATED:PARAMS block, so they are regenerated output and not a hand edit. Merge base is one day old and every check is green.

Approve

One non-blocking should fix is inline, on a single sentence in the Top K section. Two CodeRabbit threads are still open on this head and both are false positives — please decline them rather than act on them:

• #discussion_r3539638838 (Major) asks for a regression test proving get() ignores top_k. It exists: test_get_ignores_top_k_and_uses_docfilter_limit at nodes/test/store_chroma/test_retrieval_tuning.py:252.
• #discussion_r3611550904 (Minor) asks you to delete self.threshold_search at nodes/src/nodes/store_chroma/chroma.py:115 as dead code. It is read by _addDoc inside DocumentStoreBase._queryDocuments at packages/ai/src/ai/common/store/document_store.py:295, so removing it would disable the Retrieval Score knob. It is also an unchanged line in this PR.