fix: updated result to json handling to consider duplicate dataframe ids

[frayle-ons]

resolves #167

✨ Summary

These changes modify how the polars/dataclass dataframes are handled in:

1. the reverse search code of the indexers module
2. the servers module code that converts vectorstore search results into valid JSON response format.

In both these sections of the code we were grouping the dataframes by 'id' column value, to separate different queries and their results from each other. However in cases where the same 'id' value existed for multiple queries this caused some merging of independent results. For example the search results for an input search dataclass object:

test_input_data_a  = VectorStoreSearchInput(
    {
        "id": ["1", "1"], 
        "query": ["vegetable farmer", "software engineer"]
     }
)

would trigger undesired behaviour because during conversion of the results to a JSON object, the code grouped = df.groupby("id") would pack all of the results from vegetable farmer and software engineer into one result.

The changes made here make the package more resilient to several potential cases like this including:

• duplicate values in the id column of the input to the search and reverse_search methods,

test_input_data_b  = VectorStoreSearchInput(
    {"id": ["1", "1"], "query": ["vegetable farmer", "software engineer"]}
)

• duplicate values in query/doc_label column of the input to the search and reverse_search methods

test_input_data_c  = VectorStoreSearchInput(
    {"id": ["1", "2"], "query": ["vegetable farmer", "vegetable farmer"]}
)

---

NOTE:

However, the behaviour of ClassifAI when the user inputs 2 completely indistinguishable search inputs has odd outputs, e.g.:

test_input_data_d  = VectorStoreSearchInput(
    {"id": ["1", "1"], "query": ["vegetable farmer", "vegetable farmer"]}
)

still has some strange behaviours. Since this is effectively a 'duplicate' search entry, I'm not sure how this should be handled; whether to modify the existing ClassifAI code to handle this issue, or if we should introduce some new warning or a specific automatic row deduplication feature. This seems non-trivial because the user may have additional metadata columns that do indicate some difference between the rows that do share the same id and query column values. Might be a good idea to run a deduplication step where non-unique rows are removed where the entire row is non-unique, considering all columns.

---

📜 Changes Introduced

• Modified pydantic_models.py file to catch cases where 'id' columns have non-unique id
• 'modified indexers main.py file reverse search method to catch cases where id columns have non-unique values.

🔍 How to Test

I have created a simple setup script to run on this current branch (it runs with our normal mock DEMO test data available from the repo. Because the servers module and the indexers module are affected, the results should be examined in both the python runtime output, and also testing on a running FastAPI server instance created with the servers module.

from classifai.vectorisers import HuggingFaceVectoriser
from classifai.indexers import VectorStore
from classifai.servers import run_server
from classifai.indexers.dataclasses import VectorStoreReverseSearchInput, VectorStoreSearchInput


#start a vectoriser
my_vectoriser = HuggingFaceVectoriser(model_name="sentence-transformers/all-MiniLM-L6-v2")


#build a vector store
my_vector_store = VectorStore(
    file_name="./DEMO/data/fake_soc_dataset.csv",
    data_type="csv",
    vectoriser=my_vectoriser,
    skip_save=True,
)

#run a live server for the vectorstore
run_server([my_vector_store], endpoint_names=["test_vectorstore"], log_level="info", demo_mode=True)

The final line of that code will trigger the RESTAPI server to run, and the tester may then be able to manually query the endpoints to see how the resulting data is affected by the changes. Possibly running this script on main branch and comparing would be useful too to see how the code previously caused bugs.

Some useful test cases are below for each of the affected VectorStore methods. It is worth testing these through the started RESTAPI but also just as part of Python script that accesses the VectorStore Search and Reverse Search methods. I've written these in Python code but the content can be used to understand good RESTAPI request bodies.

Append these to the end of the setup script, commenting out the run_server line at the end.

search method test inputs:

# a good input that always worked
test_input_data_1  = VectorStoreSearchInput(
    {"id": ["1", "2"], "query": ["golden farmer", "golden software engineer"]}
)
print(my_vector_store.search(test_input_data_1, n_results=3))


############################

# duplicate ids would have caused problems before changes
test_input_data_2  = VectorStoreSearchInput(
    {"id": ["1", "1"], "query": ["golden farmer", "golden software engineer"]}
)
print(my_vector_store.search(test_input_data_2, n_results=3))


############################

# duplicate query texts may have caused problems
test_input_data_3  = VectorStoreSearchInput(
    {"id": ["1", "2"], "query": ["golden farmer", "golden farmer"]}
)
print(my_vector_store.search(test_input_data_3, n_results=3))


############################

# the test cases these changes do not handle and may still cause bugs
test_input_data_4  = VectorStoreSearchInput(
    {"id": ["1", "1"], "query": ["golden farmer", "golden farmer"]}
)
print(my_vector_store.search(test_input_data_4, n_results=3))

test inputs for reverse search:

# good test data that's always worked
test_input_data_5  = VectorStoreReverseSearchInput(
    {"id": ["1", "2"], "doc_label": ["101", "130"]}
)
print(my_vector_store.reverse_search(test_input_data_5, max_n_results=3))


############################

# duplicate ids can cause issues
test_input_data_6  = VectorStoreReverseSearchInput(
    {"id": ["1", "1"], "doc_label": ["101", "130"]}
)
print(my_vector_store.reverse_search(test_input_data_6, max_n_results=3))


############################

# duplicate doc labels may cause issues
test_input_data_7  = VectorStoreReverseSearchInput(
    {"id": ["1", "2"], "doc_label": ["101", "101"]}
)
print(my_vector_store.reverse_search(test_input_data_7, max_n_results=3))


############################

# an example of full row duplication that we don't catch with these changes
test_input_data_8  = VectorStoreReverseSearchInput(
    {"id": ["1", "1"], "doc_label": ["101", "101"]}
)
print(my_vector_store.reverse_search(test_input_data_8, max_n_results=3))

test inputs for the reverse search with partial matching enabled:

# another good example that works normally
test_input_data_9  = VectorStoreReverseSearchInput(
    {"id": ["1", "2"], "doc_label": ["10", "13"]}
)
print(my_vector_store.reverse_search(test_input_data_9, max_n_results=3, partial_match=True))


############################

# duplicate ID values would cause errors without changes
test_input_data_10  = VectorStoreReverseSearchInput(
    {"id": ["1", "1"], "doc_label": ["10", "13"]}
)
print(my_vector_store.reverse_search(test_input_data_10, max_n_results=3, partial_match=True))


############################

# duplicate doc_label values which may have caused errors in the past
test_input_data_11  = VectorStoreReverseSearchInput(
    {"id": ["1", "2"], "doc_label": ["10", "10"]}
)
print(my_vector_store.reverse_search(test_input_data_11, max_n_results=3, partial_match=True))


############################

# full row duplication that may still cause errors
test_input_data_12  = VectorStoreReverseSearchInput(
    {"id": ["1", "1"], "doc_label": ["10", "10"]}
)
print(my_vector_store.reverse_search(test_input_data_12, max_n_results=3, partial_match=True))

finally, and to reiterate, it is worth testing both through the server and the python runtime vectorstore methods, across this branch and main, to best see how the different dataclass inputs are handled in each unique test case.

[lukeroantreeONS]

Thanks for looking into this @frayle-ons

Could I check what the reasoning behind facilitating non-unique IDs is though?
It seems unintuitive to me for that to be something we want to allow, I had assumed the resolution to this would be a uniqueness validation check in the Input Dataclass schemas (see the issue comments)