Repository: deepset-ai/haystack
Pull request: refactor: Allow flexible document id generation
### Related Issues
- fixes #4317 

### Proposed Changes:
Allow users to choose custom meta keys to be used as part of the id generation. This will help to avoid rebuilding the documents in cases where just part of the meta keys make sense for identification, and sometimes even more than the content.
@julian-risch suggestion was adopted.

### How did you test it?
Current tests and one extra test has been added.

### Notes for the reviewer
The max depth has been set to one because some document stores don't allow nested fields. User can normalize to this depth by using composed names, e.g.,  "_cache.url_" - `meta['cache.url']`

Users were able to set id_hash_keys to ['content', 'meta'] or to one of them. Now they will be able to use specific meta fields which will turn possible the use in id_hash_keys of lists like ['meta.url','meta.date','content'], or only one of them them. 

If the user uses a field like 'meta.cache.url', Haystack will look for a key in the first depth level with the name 'cache.url'.

### Checklist
- [x] I have read the [contributors guidelines](https://github.com/deepset-ai/haystack/blob/main/CONTRIBUTING.md) and the [code of conduct](https://github.com/deepset-ai/haystack/blob/main/code_of_conduct.txt)
- [x] I have updated the related issue with new insights and changes
- [x] I added tests that demonstrate the correct behavior of the change
- [x] I've used one of the [conventional commit types](https://www.conventionalcommits.org/en/v1.0.0/) for my PR title: `fix:`, `feat:`, `build:`, `chore:`, `ci:`, `docs:`, `style:`, `refactor:`, `perf:`, `test:`.
- [x] I documented my code
- [x] I ran [pre-commit hooks](https://github.com/deepset-ai/haystack/blob/main/CONTRIBUTING.md#installation) and fixed any issue


Problem statement:
Allow more flexible Document id hashing
**Is your feature request related to a problem? Please describe.**
As a Document user, I found myself in a situation where content and meta fields are too broad or too narrow for id calculations. We currently don't allow a specific set of keys in meta to be used as `id_hash_keys` in the Document constructor, i.e.

```
 def __init__(
        self,
        content: Union[str, pd.DataFrame],
        content_type: ContentTypes = "text",
        id: Optional[str] = None,
        score: Optional[float] = None,
        meta: Optional[Dict[str, Any]] = None,
        embedding: Optional[np.ndarray] = None,
        id_hash_keys: Optional[List[str]] = None,
    ):
```

My primary motivation is not having the flexibility to calculate Document id on a specific subset of meta keys. Documents retrieved from the web (via WebRetriever) will have many meta keys (e.g. URL, search rank, index_date etc.), but I would like to base the Document id on URL and query only rather than all meta keys (I don't care of rank or index_date change for example). 

**Describe the solution you'd like**
A hash function parameter to Document init will allow me to select the particular fields from meta to include in the id calculation and calculate the Document id myself. This function should receive all other parameters to init as its parameters. 

**Describe alternatives you've considered**
Using a big kludge of DocumentBuilder class that calculates Document id on a specific set of meta keys and then assigning the id myself. 

**Additional context**
N/A

Hints from issue discussion:
That's great. Indeed, we use a document builder on top of Haystack because of the lack of flexibility. We store a bunch of metadata and just a small piece of it makes sense for the ID (helping to control when to overwrite, without the need to query before each write and update related documents).

I think this will be a major improvement.

There are other future cases which are related to the agent orientation that we've seen in the last days:

1. Cache Document Store
2. Memory Document Store
3. Conversational turn Store

We've learned from our internal agent implementation that sometimes you can avoid hitting the LLM at each agent's request by storing last conversations and linking them to some metadata. In our case, the normalized user intent (it can be achieved using a smaller and cheaper model, like _ada_). When the intent arrives again, we do a vector search for similar intent/results and use that as input to feed the prompt. Of course there are filters and so on, but the concept of being able to use a custom ID (meta oriented) is highly important.
I support the idea of having a more fine-grained definition of hash keys to generate document ids. Let's discuss what format works best to pass this more fine-grained definition then. Right now, we have `id_hash_keys=["content", "meta"]`. What do you think about sth like `id_hash_keys=["content", "meta.url"]`? 
I would love to offload this responsibility and freedom to users, but if dC can't use those custom id function, then we don't have a choice but to go with your suggestion Julian. 
> I would love to offload this responsibility and freedom to users, but if dC can't use those custom id function, then we don't have a choice but to go with your suggestion Julian.

For dC and Haystack API users, the hash keys need to be defined in the pipeline yaml. That's the only option besides implementing custom nodes. For example, via setting the `id_hash_keys` parameter in the init function of the `PreProcessor`. It's not feasible to pass a custom function there unfortunately.

Commit message: refactor: allow flexible document id generation

Patch to review:
diff --git a/haystack/schema.py b/haystack/schema.py
index 0bda792214..f0acafe2a0 100644
--- a/haystack/schema.py
+++ b/haystack/schema.py
@@ -101,11 +101,15 @@ def __init__(
         allowed_hash_key_attributes = ["content", "content_type", "score", "meta", "embedding"]
 
         if id_hash_keys is not None:
-            if not set(id_hash_keys) <= set(allowed_hash_key_attributes):
+            if not all(
+                True if key in allowed_hash_key_attributes or key.startswith("meta.") else False for key in id_hash_keys
+            ):
                 raise ValueError(
                     f"You passed custom strings {id_hash_keys} to id_hash_keys which is deprecated. Supply instead a "
-                    f"list of Document's attribute names (like {', '.join(allowed_hash_key_attributes)}). "
-                    "See https://github.com/deepset-ai/haystack/pull/1910 for details)"
+                    f"list of Document's attribute names (like {', '.join(allowed_hash_key_attributes)}) or "
+                    f"a key of meta with a maximum depth of 1 (like meta.url). "
+                    "See https://github.com/deepset-ai/haystack/pull/1910 and "
+                    "https://github.com/deepset-ai/haystack/issues/4317 for details"
                 )
         # We store id_hash_keys to be able to clone documents, for example when splitting them during pre-processing
         self.id_hash_keys = id_hash_keys or ["content"]
@@ -133,7 +137,12 @@ def _get_id(self, id_hash_keys: Optional[List[str]] = None):
 
         final_hash_key = ""
         for attr in id_hash_keys:
-            final_hash_key += ":" + str(getattr(self, attr))
+            if attr.startswith("meta."):
+                meta_key = attr.split(".", maxsplit=1)[1]
+                if meta_key in self.meta:
+                    final_hash_key += ":" + str(self.meta[meta_key])
+            else:
+                final_hash_key += ":" + str(getattr(self, attr))
 
         if final_hash_key == "":
             raise ValueError(
diff --git a/test/others/test_schema.py b/test/others/test_schema.py
index a502c0b7f0..eec9f98b64 100644
--- a/test/others/test_schema.py
+++ b/test/others/test_schema.py
@@ -219,6 +219,29 @@ def test_generate_doc_id_using_custom_list():
         _ = Document(content=text1, meta={"name": "doc1"}, id_hash_keys=["content", "non_existing_field"])
 
 
+def test_generate_doc_id_custom_list_meta():
+    text1 = "text1"
+    text2 = "text2"
+
+    doc1_text1 = Document(
+        content=text1, meta={"name": "doc1", "url": "https://deepset.ai"}, id_hash_keys=["content", "meta.url"]
+    )
+    doc2_text1 = Document(
+        content=text1, meta={"name": "doc2", "url": "https://deepset.ai"}, id_hash_keys=["content", "meta.url"]
+    )
+    assert doc1_text1.id == doc2_text1.id
+
+    doc1_text1 = Document(content=text1, meta={"name": "doc1", "url": "https://deepset.ai"}, id_hash_keys=["meta.url"])
+    doc2_text2 = Document(content=text2, meta={"name": "doc2", "url": "https://deepset.ai"}, id_hash_keys=["meta.url"])
+    assert doc1_text1.id == doc2_text2.id
+
+    doc1_text1 = Document(content=text1, meta={"name": "doc1", "url": "https://deepset.ai"}, id_hash_keys=["meta.url"])
+    doc2_text2 = Document(
+        content=text2, meta={"name": "doc2", "url": "https://deepset.ai"}, id_hash_keys=["meta.url", "meta.name"]
+    )
+    assert doc1_text1.id != doc2_text2.id
+
+
 def test_aggregate_labels_with_labels():
     label1_with_filter1 = Label(
         query="question",


Code review comments:
diff_hunk: @@ -101,11 +101,15 @@ def __init__(
         allowed_hash_key_attributes = ["content", "content_type", "score", "meta", "embedding"]
 
         if id_hash_keys is not None:
-            if not set(id_hash_keys) <= set(allowed_hash_key_attributes):
+            if not all(
+                True if key in allowed_hash_key_attributes or key.startswith("meta.") else False for key in id_hash_keys
+            ):
path: haystack/schema.py
text: @user1:
This can be simplified like so, I think it's easier to read.

```suggestion
            if not all(key in allowed_hash_key_attributes or key.startswith("meta.") for key in id_hash_keys):
```