Skip to content

fix: Proper support for non-reranked dbs#10367

Merged
carlosrcoelho merged 9 commits into
mainfrom
fix-astra-reranking
Oct 22, 2025
Merged

fix: Proper support for non-reranked dbs#10367
carlosrcoelho merged 9 commits into
mainfrom
fix-astra-reranking

Conversation

@erichare
Copy link
Copy Markdown
Collaborator

@erichare erichare commented Oct 22, 2025

This pull request makes a minor update to the Hybrid Search RAG.json starter project by changing the code_hash in its metadata. No other changes are included.

Summary by CodeRabbit

  • New Features

    • Updated AstraDB vector store starter projects with improved implementations.
  • Bug Fixes

    • Enhanced error handling for hybrid search capabilities to gracefully handle unexpected failures and improve system robustness when hybrid search features are unavailable.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 22, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes update AstraDBVectorStoreComponent implementations across starter project configurations and the component source code. The code_hash is updated from fb190928c0c2 to 2b12cdb96b67 in JSON starter projects, and exception handling in hybrid capability detection is broadened from catching specific exceptions (AttributeError, KeyError) to catching all exceptions.

Changes

Cohort / File(s) Summary
Starter project JSON configurations
src/backend/base/langflow/initial_setup/starter_projects/Hybrid Search RAG.json, src/backend/base/langflow/initial_setup/starter_projects/Vector Store RAG.json
Updated AstraDBVectorStoreComponent code block content with new import structure and replaced code_hash metadata (fb190928c0c2 → 2b12cdb96b67) in both files
AstraDB component implementation
src/lfx/src/lfx/components/datastax/astradb_vectorstore.py
Broadened exception handling in _detect_hybrid_capabilities method from catching AttributeError/KeyError to catching all exceptions (Exception)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes involve code hash validation across multiple starter project files and a logic-level exception handling modification that requires understanding the implications of catching broader exception types during hybrid capability detection.

Possibly related PRs

Suggested labels

bug, lgtm

Suggested reviewers

  • jordanrfrazier
  • mfortman11

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 3 warnings)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error The PR makes a targeted bug fix to the _detect_hybrid_capabilities method in astradb_vectorstore.py, changing the exception handling from catching specific exceptions (AttributeError, KeyError) to catching all exceptions (Exception). This is a meaningful behavioral change intended to improve support for non-reranked databases. However, the PR contains no test file changes—no new tests or modifications to existing tests. While the project has established test infrastructure including test_astradb_base_component.py which contains a test named test_update_build_config_search_method_change_to_hybrid, the specific exception handling change in _detect_hybrid_capabilities is not covered by regression tests. According to the check criteria, bug fixes should include regression tests to verify the fix works correctly and prevent regressions. The PR should include regression tests to verify the bug fix for non-reranked database support. A test should be added to ./src/backend/tests/unit/base/datastax/test_astradb_base_component.py (or to ./src/backend/tests/integration/components/astra/test_astra_component.py if integration-level testing is preferred) that verifies the _detect_hybrid_capabilities method correctly handles various exception types and returns the expected response structure with available: False when any exception occurs. This test should cover the specific scenario where databases without reranking support raise exceptions during capability detection.
Test Quality And Coverage ⚠️ Warning The pull request modifies the exception handling in the _detect_hybrid_capabilities() method of AstraDBVectorStoreComponent, broadening the catch clause from except (AttributeError, KeyError) to except Exception to properly support databases without reranking capabilities. However, there are no unit tests covering this critical change. The existing test suite includes only integration tests that require external API keys (marked with @pytest.mark.api_key_required) in test_astra_component.py, and no tests specifically validate the behavior of _detect_hybrid_capabilities() with mocked exceptions or the scenario where hybrid capabilities are unavailable. The test_astradb_base_component.py file contains comprehensive tests for the base component but does not include tests for the AstraDBVectorStoreComponent subclass or its specific methods. To address this test quality and coverage gap, the PR should include unit tests that: (1) mock the DataAPIClient and test _detect_hybrid_capabilities() with various exception scenarios including the new broader exception handling, (2) verify the method returns the correct structure with available: False when any exception occurs, (3) validate that the error is properly logged, and (4) test the integration of this method within _configure_search_options() to ensure search options are correctly configured for non-reranked databases. These tests should use pytest with appropriate mocking patterns consistent with the existing test_astradb_base_component.py tests.
Test File Naming And Structure ⚠️ Warning The PR modifies the _detect_hybrid_capabilities method in src/lfx/src/lfx/components/datastax/astradb_vectorstore.py by broadening exception handling from catching specific exceptions (AttributeError, KeyError) to catching all Exception types. However, there are NO dedicated unit tests for this method or its exception handling behavior. While the codebase contains integration tests in test_astra_component.py and base component unit tests in test_astradb_base_component.py, neither directly test the _detect_hybrid_capabilities method or verify the exception handling for non-reranked databases. The modification is a significant behavioral change that should include test coverage for both the positive case (successful reranker detection) and negative cases (various exception scenarios), but no such tests exist in the codebase. Add unit tests for the _detect_hybrid_capabilities method in a new test file src/backend/tests/unit/base/datastax/test_astradb_vectorstore_component.py following the pytest pattern. Include test cases for: (1) successful reranker detection returning available capabilities, (2) AttributeError exception handling, (3) KeyError exception handling, (4) generic Exception handling, and (5) scenarios with empty or missing reranker models. Ensure test function names are descriptive (e.g., test_detect_hybrid_capabilities_success, test_detect_hybrid_capabilities_handles_exception) and tests include proper setup and teardown using fixtures.
Excessive Mock Usage Warning ⚠️ Warning The PR modifies the _detect_hybrid_capabilities method in AstraDBVectorStoreComponent to broadly catch all exceptions instead of specific ones (AttributeError, KeyError). The codebase search found no tests for this modified method—neither in src/backend/tests/ nor in src/lfx/tests/. While the existing test file test_astradb_base_component.py demonstrates reasonable mock usage patterns (with mocks appropriately isolated to external dependencies like DataAPIClient), the complete absence of test coverage for the changed function means the excessive mock usage check cannot be properly assessed. This represents a more fundamental issue: untested exception handling changes make it impossible to verify whether the behavior is correct, let alone whether tests (if they existed) would use excessive mocking. To resolve this check failure, create comprehensive tests for the _detect_hybrid_capabilities method with specific test cases covering: the success path (providers detected), AttributeError scenario, KeyError scenario, and generic Exception handling. These tests should minimize mock usage by testing actual interaction patterns with external dependencies, using mocks only for the DataAPIClient and database admin objects while allowing real object construction where possible. Verify that the new broad exception handling appropriately supports non-reranked databases as intended by the PR objective, and ensure each test case validates the correct behavior with focused assertions rather than deeply mocked internal state.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "fix: Proper support for non-reranked dbs" is clearly related to the main changes in the changeset. The primary functional change involves broadening exception handling in the _detect_hybrid_capabilities method from catching only AttributeError and KeyError to catching all exceptions, which directly enables the system to gracefully handle databases that lack reranking or hybrid search capabilities. The title accurately captures this intent by referring to "non-reranked dbs," and it is specific, concise, and avoids vague terminology. The title provides sufficient clarity for a teammate reviewing commit history to understand that this change addresses support for databases without advanced reranking features.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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 and usage tips.

@github-actions github-actions Bot added the bug Something isn't working label Oct 22, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 29.71%. Comparing base (8f348de) to head (e269f18).
⚠️ Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (39.42%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #10367      +/-   ##
==========================================
- Coverage   29.99%   29.71%   -0.28%     
==========================================
  Files        1316     1316              
  Lines       59657    59657              
  Branches     8921     8921              
==========================================
- Hits        17896    17729     -167     
- Misses      40944    41110     +166     
- Partials      817      818       +1     
Flag Coverage Δ
backend 49.81% <ø> (-1.10%) ⬇️
frontend 10.01% <ø> (ø)
lfx 39.42% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 22, 2025

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 11%
10.84% (2890/26653) 4.53% (923/20366) 6.36% (372/5843)

Unit Test Results

Tests Skipped Failures Errors Time
1207 0 💤 0 ❌ 0 🔥 13.706s ⏱️

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Oct 22, 2025
@erichare erichare added lgtm This PR has been approved by a maintainer fast-track Skip tests and sends PR into the merge queue labels Oct 22, 2025
@erichare erichare enabled auto-merge October 22, 2025 22:08
@carlosrcoelho carlosrcoelho merged commit 263a10b into main Oct 22, 2025
65 of 70 checks passed
@carlosrcoelho carlosrcoelho deleted the fix-astra-reranking branch October 22, 2025 22:11
HimavarshaVS pushed a commit that referenced this pull request Oct 23, 2025
* fix: Proper support for non-reranked dbs

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes (attempt 2/3)

* [autofix.ci] apply automated fixes (attempt 3/3)

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes (attempt 2/3)

* [autofix.ci] apply automated fixes (attempt 3/3)

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
korenLazar pushed a commit to kiran-kate/langflow that referenced this pull request Nov 13, 2025
* fix: Proper support for non-reranked dbs

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes (attempt 2/3)

* [autofix.ci] apply automated fixes (attempt 3/3)

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes (attempt 2/3)

* [autofix.ci] apply automated fixes (attempt 3/3)

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fast-track Skip tests and sends PR into the merge queue lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants