Skip to content

fix: guard cosine_similarity against empty pagination page - #1058

Open
IshanMaheshwari-777 wants to merge 1 commit into
OWASP:mainfrom
IshanMaheshwari-777:guard-empty-page-cosine-similarity
Open

fix: guard cosine_similarity against empty pagination page#1058
IshanMaheshwari-777 wants to merge 1 commit into
OWASP:mainfrom
IshanMaheshwari-777:guard-empty-page-cosine-similarity

Conversation

@IshanMaheshwari-777

@IshanMaheshwari-777 IshanMaheshwari-777 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1048 — guards the paginated embedding similarity fallback against a page that comes back with zero embeddings.

Problem

If a page's stored vectors all fail to parse (e.g. malformed/corrupted embedding_vec data), get_embeddings_by_doc_type_paginated returns an empty dict for that page. Both _paginated similarity methods still handed that empty page straight to cosine_similarity, which raises:
"ValueError: Found array with 0 feature(s) (shape=(1, 0)) while a minimum of 1 is required by check_pairwise_arrays."

This crashes the whole lookup (chatbot query or importer run) instead of just skipping the bad page and continuing to the next one.

Changes

  • get_id_of_most_similar_cre_paginated and get_id_of_most_similar_node_paginated: skip the embedding-load/similarity-comparison step entirely when the current page's embeddings dict is empty (if embeddings:), while still advancing to the next page as before.
  • Added regression tests (test_node_paginated_skips_empty_final_page, test_cre_paginated_skips_empty_final_page) covering a final page that comes back empty while the real match sits on an earlier page.

Testing

  • pytest application/tests/prompt_client_pgvector_similarity_test.py -v — 10 passed
  • Reverted the guard and reran the new tests to confirm they reproduce the exact ValueError above; restored the guard and confirmed both pass
  • mypy --strict on the modified test file — no errors
  • pytest application/tests/pci_dss_parser_test.py (real caller of the fixed CRE method) — 8 passed
  • Full suite (pytest application/tests/) — 978 passed, 2 pre-existing unrelated failures (unchanged from before this branch, confirmed by rerunning them with this branch's changes stashed out)

CC: @northdpole

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Team

Run ID: e25b4c99-99aa-4bc7-9494-25cfb51393b4

📥 Commits

Reviewing files that changed from the base of the PR and between 5a3c384 and fc65637.

📒 Files selected for processing (2)
  • application/prompt_client/prompt_client.py
  • application/tests/prompt_client_pgvector_similarity_test.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Summary by CodeRabbit

  • Bug Fixes

    • Paginated similarity searches now safely skip empty embedding pages instead of failing during similarity calculations.
    • Searches continue returning valid matches from earlier pages when a later page contains invalid or unavailable embedding data.
  • Tests

    • Added coverage for node and CRE searches with empty final pages.

Walkthrough

Changes

The paginated CRE and node similarity searches now skip empty embedding pages. New tests cover empty final pages while preserving matches from earlier pages.

Paginated similarity search

Layer / File(s) Summary
Empty-page guards
application/prompt_client/prompt_client.py
Both paginated methods load embeddings and compute cosine similarity only when page embeddings are truthy.
Empty-final-page regression tests
application/tests/prompt_client_pgvector_similarity_test.py
Node and CRE tests verify that an earlier-page match is returned when the final page is empty.

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

Merge Risk: ⚪ Minimal · up to fc656

Paginated similarity lookups now skip empty embedding pages rather than failing, while preserving matches from earlier pages. Both CRE and node paths have regression coverage, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. 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.
Title check ✅ Passed The title clearly and concisely describes the main change: guarding cosine similarity when a paginated page has no embeddings.
Description check ✅ Passed The description directly explains the empty-page failure, the implemented guard, the regression tests, and the test results. It is fully related to the changeset.
  • Fix all pre-merge checks with AI
✨ 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant