Skip to content

refactor: rename search test to search and update related components and APIs#976

Merged
iziang merged 3 commits into
mainfrom
support/rename-search-test
Jun 23, 2025
Merged

refactor: rename search test to search and update related components and APIs#976
iziang merged 3 commits into
mainfrom
support/rename-search-test

Conversation

@iziang
Copy link
Copy Markdown
Contributor

@iziang iziang commented Jun 23, 2025

No description provided.

@apecloud-bot apecloud-bot added the size/XL Denotes a PR that changes 500-999 lines. label Jun 23, 2025
@apecloud-bot apecloud-bot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Jun 23, 2025
cursor[bot]

This comment was marked as outdated.

@iziang iziang merged commit f50a84a into main Jun 23, 2025
6 of 7 checks passed
@iziang iziang deleted the support/rename-search-test branch June 23, 2025 03:31
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Function Name Collision Causes Test Failures

A function name collision exists for assert_search_result_item in tests/e2e_test/utils.py. One definition is a nested function within assert_search_result (renamed from assert_search_test_result_item) that validates search item properties (score, content, rank). The other is a newly added module-level function that checks for recall_type existence. This conflict means any external calls to assert_search_result_item will resolve to the module-level function, potentially breaking existing test validation logic that relies on the original nested function's behavior.

tests/e2e_test/utils.py#L57-L90

def assert_search_result_item(search_type, items):
for item in items:
if search_type != "graph_search":
assert isinstance(item["score"], float)
assert isinstance(item["content"], str)
assert isinstance(item["rank"], int)
if "vector_search" in expected:
assert actual["vector_search"] is not None
for key in ["topk", "similarity"]:
assert expected["vector_search"][key] == actual["vector_search"][key]
assert_search_result_item("vector_search", actual["items"])
if "fulltext_search" in expected:
assert actual["fulltext_search"] is not None
for key in ["topk"]:
assert expected["fulltext_search"][key] == actual["fulltext_search"][key]
assert_search_result_item("fulltext_search", actual["items"])
if "graph_search" in expected:
assert actual["graph_search"] is not None
for key in ["topk"]:
assert expected["graph_search"][key] == actual["graph_search"][key]
assert_search_result_item("graph_search", actual["items"])
def assert_search_result_item(search_type, items):
found = False
for item in items:
if item.get("recall_type") == search_type:
found = True
break
assert found, f"No {search_type} item found in search results"

Fix in Cursor


Bug: Migration Script Renames Tables, Fails Index Drops

The migration script introduces two issues:

  1. It includes an unrelated rename of the document_indexes table to document_index.
  2. After renaming searchtesthistory to searchhistory, it attempts to drop indexes by their old names (e.g., ix_searchtesthistory_collection_id) on the new table (searchhistory). This fails because indexes are automatically renamed or become invalid when their parent table is renamed.

aperag/migration/versions/20250623110658-23c0533b6b63.py#L21-L34

def upgrade() -> None:
# Rename table from searchtesthistory to searchhistory
op.rename_table('searchtesthistory', 'searchhistory')
op.rename_table('document_indexes', 'document_index')
# Update any indexes that reference the old table name
op.drop_index('ix_searchtesthistory_collection_id', table_name='searchhistory')
op.drop_index('ix_searchtesthistory_gmt_deleted', table_name='searchhistory')
op.drop_index('ix_searchtesthistory_user', table_name='searchhistory')
# Create new indexes with updated names
op.create_index('ix_searchhistory_collection_id', 'searchhistory', ['collection_id'], unique=False)
op.create_index('ix_searchhistory_gmt_deleted', 'searchhistory', ['gmt_deleted'], unique=False)
op.create_index('ix_searchhistory_user', 'searchhistory', ['user'], unique=False)

Fix in Cursor


Was this report helpful? Give feedback by reacting with 👍 or 👎

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

Labels

size/XXL Denotes a PR that changes 1000+ lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants