Skip to content

fix: list/set sampling optimization bug in _recursive_datetime_check#118

Open
Jah-yee wants to merge 2 commits intoEverMind-AI:mainfrom
Jah-yee:fix/list-datetime-conversion-bug
Open

fix: list/set sampling optimization bug in _recursive_datetime_check#118
Jah-yee wants to merge 2 commits intoEverMind-AI:mainfrom
Jah-yee:fix/list-datetime-conversion-bug

Conversation

@Jah-yee
Copy link

@Jah-yee Jah-yee commented Mar 6, 2026

Good day,

I found and fixed a bug in the _recursive_datetime_check method in DocumentBase.

Bug Description

The original code has a list/set sampling optimization that only checks the first element to decide whether to process the entire collection:

# Original buggy code
if first_checked is first_item:
    return obj

Problem: When list elements are BaseModel objects, _recursive_datetime_check modifies them in-place (via __dict__), not returning a new object. This causes first_checked is first_item to always be True, making the code incorrectly assume no conversion is needed and skipping all elements after the first one.

This was confirmed by the test file tests/test_group_profile_datetime_check.py (see test test_list_datetime_conversion):

  • topics[0]: Converted ✅
  • topics[1], topics[2]: Not converted ❌

Fix

  1. Track converted fields: In BaseModel processing, track which datetime fields were actually converted
  2. Add conversion marker: Attach _datetime_converted_fields marker to detect actual changes
  3. Check marker for BaseModel: In list/set sampling logic, check the marker instead of object reference

Testing

  • The existing test test_list_datetime_confirm_conversion verifies this fix
  • All datetime fields in list elements should now be properly converted

感谢你们的奉献,希望能提供帮助。如果我解决得有问题或有待商妥的地方,请在下面留言,我会来处理。

Warmly,
Spark Lab Scout

Spark Lab Scout added 2 commits March 6, 2026 20:47
The original code had a bug in the list/set sampling optimization:
- When list elements are BaseModel objects, _recursive_datetime_check
  modifies them in-place (via __dict__), not returning a new object
- This caused 'first_checked is first_item' to always be True
- The code incorrectly assumed no conversion was needed, skipping
  all elements after the first one

Fix:
- Track converted datetime fields in BaseModel processing
- Add '_datetime_converted_fields' marker to detect actual changes
- Check the marker for BaseModel items in list/set sampling logic
- This ensures all elements in a list are properly processed
- Fix get_keyword_search_results to iterate over all supported memory types
- Fix get_vector_search_results to iterate over all supported memory types
- Add MILVUS_REPO_MAP for consistent memory type mapping
- Skip unsupported types (e.g., profile) with warning logs instead of failing
- Handle individual search failures gracefully to continue with other types

Fixes issue: Search API only uses memory_types[0], silently ignoring all other types
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