Implemented ElasticSearchStore - All Actions #1

Jose-Perigolo · 2024-06-20T05:43:54Z

To run tests locally, make sure you have a '.env.local' file in the format:

ELASTICSEARCH_NODE=http://localhost:9200
ELASTICSEARCH_INDEX=vector_index

Also, you need Docker installed on your machine. Pull the Elastic Docker image from the hub and run the container. You can do this by running the script:

./test/setup.sh

After that, create the index in your local image:

npm run create-index

And to run the tests, use:

npm run test

BeAllAround · 2024-06-20T09:12:12Z

I suggest you add those notes into, say, /test/setup.sh. A dev will be able to easily just run it and read your notes.

BeAllAround · 2024-06-20T10:01:54Z

I am seeing that most of the files are like this: ElasticsearchStore. Please, replace them with camelCase ElasticSearchStore consistently.

BeAllAround

Review submitted

BeAllAround · 2024-06-21T10:57:54Z

src/ElasticSearchStore.ts

+        });
+      }
+
+      if (msg.directive$ && msg.directive$.vector$) {


You need to separate msg.q from msg. It is not the same. directive$ is supposed to come from the msg.q.

You can do something like this:
const q = msg.q

// use q - no need to reference it every time as msg.q :)
The unit tests happened to pass but you are never actually creating this query since msg.directive$ will always be undefined. Please, write another set of tests validating vector$.k works with returning the correct number of the approximate nearest neighbors. We must use the latest: 8.14.0 of the npm package since the nearest neighbors feature isn't entirely supported by 7.x.x.

Helpful: https://www.elastic.co/blog/introducing-approximate-nearest-neighbor-search-in-elasticsearch-8-0

Indeed, it was a mistake, did not notice because the test created for it was missleading.
Fixed.

BeAllAround · 2024-06-21T11:00:38Z

src/ElasticSearchStore.ts

+
+    save: async function (msg: any, reply: any) {
+      const ent = msg.ent;
+      const index = resolveIndex(ent, options);


Don't use semi-colons :)

Indeed, done!

BeAllAround · 2024-06-21T11:01:59Z

test/create_index.js

@@ -0,0 +1,63 @@
+const path = require('path');
+require('dotenv').config({ path: path.resolve(__dirname, '../.env.local') });


Again, don't use semi-colons

BeAllAround · 2024-06-21T11:04:23Z

src/ElasticSearchStore.ts

+        query.bool.must.push({
+          knn: {
+            field: vectorFieldName,
+            query_vector: msg.vector,


comes from the query: q.vector

Indeed, fixed.

BeAllAround · 2024-06-21T11:04:39Z

src/ElasticSearchStore.ts

+          knn: {
+            field: vectorFieldName,
+            query_vector: msg.vector,
+            k: msg.directive$.vector$.k,


You are missing this logic here:

null == vector$.k ? 11 : vector$.k

There is no need for it anymore.

BeAllAround · 2024-06-21T11:18:55Z

src/ElasticSearchStore.ts

+
+      if (msg.q) {
+        Object.keys(msg.q).forEach(key => {
+          if (key !== 'directive$' && key !== 'vector') {


There are two ways to approach this.
Prefer to use (!key.match(/\$/)) instead of (key !== 'directive$') in this case since there can be other query properties ending in $ such as fields$, limit$, etc.

You can, of course do this.

let q = msg.q let cq = seneca.util.clean(q) // removes all properties ending in '$' // use cq, q

But I will let you decide :)

Also, please use https://en.wikipedia.org/wiki/Yoda_conditions - that stands for Yoda Style in programming.

Using:
let cq = seneca.util.clean(q)

Did not know about this one, thanks. Using it in a new util function for readability:

function buildQuery(cleanedQuery: any) { const boolQuery: any = { must: [], filter: [] } Object.keys(cleanedQuery).forEach((key) => { if ('vector' !== key) { boolQuery.filter.push({ term: { [key]: cleanedQuery[key] } }) } }) return { bool: boolQuery } }

BeAllAround · 2024-06-21T12:20:09Z

package.json

@@ -64,6 +65,6 @@
  ],
  "dependencies": {
    "@aws-sdk/credential-provider-node": "^3.525.0",
-    "@opensearch-project/opensearch": "^2.5.0"
+    "@elastic/elasticsearch": "^7.17.13"


Must use the latest: https://www.npmjs.com/package/@elastic/elasticsearch: 8.14.0

What is @aws-sdk/credential-provider-node needed for?

Using "@elastic/elasticsearch": "^8.14.0"
Removed "@aws-sdk/credential-provider-node": "^3.525.0",

Thanks! Indeed we dont need aws and 8.14.0 is a must for kNN search.

BeAllAround · 2024-06-21T12:21:58Z

test/create_index.js

+              "type": "dense_vector",
+              "dims": 8,
+              "index": true, // Enable k-NN indexing
+              "similarity": "l2_norm" // Specify similarity metric


we use: "similarity": "cosine"

BeAllAround

Reviewed

BeAllAround · 2024-06-26T14:31:56Z

test/create_index.js

+              "type": "dense_vector",
+              "dims": 8,
+              "index": true, // Enable k-NN indexing
+              "similarity": "cosine" // Specify similarity metric


Please, add this in case we need to be more specific about the index_options for podmind

"index_options": { "type": "hnsw", "m": 16, "ef_construction": 512 }

BeAllAround · 2024-06-26T14:40:19Z

src/ElasticSearchStore.ts

+  const { hits } = knnResponse
+  return hits.hits
+    .filter((hit: any) =>  0.5 <= hit._score) // Adjust the threshold based on your similarity measure
+    .map((hit: any) => ({ id: hit._id, ...hit._source }))


Remove the Array.prototype.filter and in the Array.prototype.map, you can both entize and set the custom$.score.

{ ..., custom$: { score: hit._score } }

Also, make sure you unit test this.

Done, please check about these changes to see if its ok.

BeAllAround · 2024-06-26T14:41:23Z

test/ElasticSearch.test.ts

+      test: 'knn-search'
+    });
+
+    expect(list.length).toEqual(0);


actually, it should be 2

Jose-Perigolo added 8 commits June 20, 2024 02:33

Updated required Libs

a4d52e6

Update readme

100d409

Update compiled files

59775a6

Create ElastisearchStore

202731a

deleted old files

67ffec2

Adjusted tests

8267aeb

quick testing files

a002c4b

cleaning

57a52f8

Jose-Perigolo added 6 commits June 20, 2024 22:33

Deleted unused files

2a41606

Updated steps

bf257e9

Update package.json

5f8624b

Updated compiled files

815bdad

Update ElasticSearchStore

e370fdc

Update tests setups

4cc058c

Jose-Perigolo changed the title ~~Initial Commit - Basic Functionalities~~ Implemented ElasticSearchStore - All functionaties working Jun 21, 2024

Remove unnecessary logs

e103d92

Jose-Perigolo changed the title ~~Implemented ElasticSearchStore - All functionaties working~~ Implemented ElasticSearchStore - All Actions Jun 21, 2024

BeAllAround reviewed Jun 21, 2024

View reviewed changes

Jose-Perigolo added 5 commits June 26, 2024 00:36

Update to 8.14.0

fe6058b

Update compiled files

d08bc4d

Update main file

f238bd6

Updated tests

d91ad65

Update comment

84741cd

Jose-Perigolo requested a review from BeAllAround June 26, 2024 03:41

BeAllAround reviewed Jun 26, 2024

View reviewed changes

Jose-Perigolo added 2 commits July 1, 2024 13:08

New compiled files

49ec332

Remove filter, entize

b49709e

Adjusted tests

bee00a9

Jose-Perigolo requested a review from BeAllAround July 1, 2024 16:11

Jose-Perigolo added 3 commits July 1, 2024 13:25

Entize list data

dd254ed

New compiled files

db71317

Add object match tests

820bb3b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented ElasticSearchStore - All Actions #1

Implemented ElasticSearchStore - All Actions #1

Jose-Perigolo commented Jun 20, 2024 •

edited

Loading

BeAllAround commented Jun 20, 2024 •

edited

Loading

BeAllAround commented Jun 20, 2024

BeAllAround left a comment

BeAllAround Jun 21, 2024 •

edited

Loading

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024 •

edited

Loading

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround Jun 21, 2024

Jose-Perigolo Jun 26, 2024

BeAllAround left a comment

BeAllAround Jun 26, 2024

Jose-Perigolo Jul 1, 2024

BeAllAround Jun 26, 2024

Jose-Perigolo Jul 1, 2024

BeAllAround Jun 26, 2024

Jose-Perigolo Jul 1, 2024

		@@ -0,0 +1,63 @@
		const path = require('path');
		require('dotenv').config({ path: path.resolve(__dirname, '../.env.local') });

Implemented ElasticSearchStore - All Actions #1

Are you sure you want to change the base?

Implemented ElasticSearchStore - All Actions #1

Conversation

Jose-Perigolo commented Jun 20, 2024 • edited Loading

BeAllAround commented Jun 20, 2024 • edited Loading

BeAllAround commented Jun 20, 2024

BeAllAround left a comment

Choose a reason for hiding this comment

BeAllAround Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BeAllAround Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BeAllAround left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jose-Perigolo commented Jun 20, 2024 •

edited

Loading

BeAllAround commented Jun 20, 2024 •

edited

Loading

BeAllAround Jun 21, 2024 •

edited

Loading

BeAllAround Jun 21, 2024 •

edited

Loading