Add nearest neighbors support #1248

mvanschellebeeck · 2025-02-27T14:35:15Z

Add nearestNeighbor query support.

See runNearestNeighborsTest.ts for example usage

nihalbhatnagar · 2025-02-28T15:19:19Z

packages/api/src/OsdkBase.ts


  readonly $title: string | undefined;
+
+  readonly $score?: number | undefined;


I don't think we should have this belong in OSDK base, since I imagine this will be undefined 90% of the time. What if instead we passed through a generic through the ObjectSet type that determined if we were or had made a nearest neighbors query, and conditionally added it to the Osdk.Instance type?

Or actually looking further, it seems we only want score to be returned when ordering by relevance? Which makes it easier I think, we don't need to pass through a generic we can just add it to the type from a fetchPage result

I'm having trouble implementing the types here - would it be the same approach taken with $rid in https://github.com/palantir/osdk-ts/blob/main/packages/api/src/OsdkObjectFrom.ts#L224-L228?

nihalbhatnagar · 2025-02-28T15:23:30Z

packages/api/src/object/FetchPageArgs.ts

+    | {
+      [K in L]?: "asc" | "desc";
+    }
+    | "relevance";


If we order by relevance but we haven't called nearestNeighbors yet, do we throw?

Also, do we imagine that there will ever be any extra modifiers for relevance ordering? Trying to think if this should be a string or object option

If we order by relevance but we haven't called nearestNeighbors yet, do we throw?

Yep, we throw InvalidOrderType: 8ac3d47

Also, do we imagine that there will ever be any extra modifiers for relevance ordering? Trying to think if this should be a string or object option

This is how its modeled in the internal APIs so any added modifiers there would be a breaking change

nihalbhatnagar · 2025-02-28T15:25:50Z

packages/api/src/objectSet/ObjectSet.ts

+            value, all objects will be returned. This value is limited to 1 &le; numNeighbors &ge; 500.
+   * @param property - The property key with a defined embedding model to search over.
+   *
+   * @returns An object set containing the `numNeighbors` nearest neighbors. To return the objects ordered by relevance and each


If I nest two nearest neighbors object sets, which one takes precedence? Or do we throw

It operates the same way as nested unions/intersects in that it executes from inner to outer and returns the outer result. Added an example in 2538015

nihalbhatnagar · 2025-02-28T15:28:24Z

packages/api/src/objectSet/ObjectSet.ts

+   * Finds the nearest neighbors for a given text or vector within the object set.
+   *
+   * @param query - Queries support either a vector matching the embedding model defined on the property, or text that is
+        automatically embedded.


Getting it automatically embedded is cool

ssanjay1 · 2025-02-28T18:37:04Z

.changeset/rotten-scissors-wonder.md

@@ -0,0 +1,9 @@
+---
+"@osdk/foundry-sdk-generator": minor


Can we make these all patch

ssanjay1 · 2025-02-28T18:41:48Z

packages/api/src/ontology/ObjectOrInterface.ts

  & string;
+
+export type VectorType = Extract<BaseWirePropertyTypes, "vector">;
+export type VectorPropertyKeys<O extends ObjectOrInterfaceDefinition> = keyof {


Can you use our FilteredPropertyKeys type?

ssanjay1 · 2025-02-28T18:44:32Z

packages/api/src/object/FetchPageArgs.ts

+    | {
+      [K in L]?: "asc" | "desc";
+    }
+    | "relevance";


Also, do we imagine that there will ever be any extra modifiers for relevance ordering? Trying to think if this should be a string or object option

nihalbhatnagar · 2025-03-06T15:51:00Z

packages/client/src/objectSet/ObjectSet.test.ts

        },
      });

+      // Ignores "Type instantiation is excessively deep and possibly infinite."


I think you may need to remove the branded which may fix this error? If it fails without it as well thats fine

nihalbhatnagar · 2025-03-06T15:52:46Z

packages/client/src/objectSet/ObjectSet.test.ts

+      "skillSetEmbedding",
+    );
+
+    const { data: unOrdered } = await nearestNeighbors.fetchPage();


Do you mind doing a sanity check here and trying to access .$score on one of the resulting objects (and adding //ts-expect-error) or actually for most of these cases

I don't think we have type tests for OsdkObjectFrom but if we do that might be a better place to put it

ssanjay1 · 2025-03-06T15:48:57Z

packages/api/src/object/FetchPageArgs.ts

  $includeRid?: R;
 }

+export type OrderByType<


nit: can we rename this OrderByOptions?

ssanjay1 · 2025-03-06T15:51:24Z

packages/client/src/objectSet/ObjectSet.test.ts

+    expect(employees).toHaveLength(numNeighbors);
+    // Check that no score is returned when not ordered by relevance
+    // @ts-ignore
+    employees.forEach(e => expect(e.$score).toBeUndefined());


What did you have to ts-ignore here?

Oh is it because $score isn't even a property on the object? If so, could we actually modify by adding some type tests to make sure that $score is not an available option?

@ts-expect-error is useful when you want something to error

Ah ignore my comments about adding this specific test above

update in 0b6b35a

ssanjay1 · 2025-03-06T16:00:10Z

packages/client/src/objectSet/ObjectSet.test.ts

+      $orderBy: "relevance",
+    });
+    expectTypeOf<typeof orderedByRelevance>().toMatchTypeOf<
+      Osdk.Instance<Employee, never, PropertyKeys<Employee>, {}, "relevance">[]


Hmm, I'm not a big fan of relevance being in the type when in reality it's $score that pops up. I think what we want to do is something similar to ExtractOptions so instead of passing in Z directly to Osdk.Instance you pass in ExtractOrderByOptions<Z> which will give you either never or$score that way if someone looks at their return type, its clear what they're getting back on their object.

Like Osdk.Instance<quickAndDirty, "$rid", "name" | "foo"> makes it clear to me I'm getting name, foo, and $rid back, so would be nice to keep it consistent and also throw in $score rather than relevance

yep agreed - fixed in
0b6b35a

nihalbhatnagar · 2025-03-13T13:57:57Z

packages/api/src/object/FetchPageArgs.ts

+  | "relevance";
+
 export interface OrderByArg<
  Q extends ObjectOrInterfaceDefinition,


Noting for myself that this isn't exported, so this arg reorder is fine

nihalbhatnagar · 2025-03-13T14:05:00Z

packages/api/src/objectSet/ObjectSet.ts

+  readonly nearestNeighbors: (
+    query: string | number[],
+    numNeighbors: number,
+    property: VectorPropertyKeys<Q>,


Can we just use FilteredPropertyKeys here instead?

nice, f6eb11b

ssanjay1 · 2025-03-17T19:52:46Z

packages/client/src/objectSet/ObjectSet.test.ts

      });

-      expectTypeOf(objectSet).branded.toEqualTypeOf<
+      expectTypeOf(objectSet).toEqualTypeOf<


Why'd we remove branded here?

ah I was seeing Type instantiation is excessively deep and possibly infinite. Appears to be a failure mode for this branded check according to the docs:

branded: * // ❌ The following line doesn't compile, even though the types are arguably the same. * expectTypeOf<{ a: 1 } & { b: 2 }>().toEqualTypeOf<{ a: 1; b: 2 }>() * ``` * This helper works around this problem by using * a more permissive but less performant check. * * __Note__: This comes at a performance cost, and can cause the compiler * to 'give up' if used with excessively deep types, so use sparingly.

mvanschellebeeck marked this pull request as ready for review February 27, 2025 17:14

nihalbhatnagar reviewed Feb 28, 2025

View reviewed changes

ssanjay1 reviewed Feb 28, 2025

View reviewed changes

mvanschellebeeck force-pushed the mv/knn-2 branch from 3943c37 to 084b04d Compare March 6, 2025 15:09

nihalbhatnagar reviewed Mar 6, 2025

View reviewed changes

ssanjay1 reviewed Mar 6, 2025

View reviewed changes

mvanschellebeeck added 14 commits March 12, 2025 17:01

Add nearestNeighbor support

ea45276

Add tests

600fc19

Fix metadata test

babb562

Fix snapshots

a7bda62

changeset

084fdc1

Use FilterPropertyKeys

5f22cac

Nested query example

dd4a28f

pnpm check

07b9bd3

Invalid order type

7af2e05

Move $score field out of Osdk base and conditionally expose it

5e48fcd

pnpm check

29a0645

Fix tests

b99bb93

Fix metadata snapshot

5c5e674

Extract options + ts-expect-error

7944bcb

mvanschellebeeck force-pushed the mv/knn-2 branch from 0b6b35a to 7944bcb Compare March 13, 2025 00:00

remove generated next files

21378b0

nihalbhatnagar reviewed Mar 13, 2025

View reviewed changes

use FilteredPropertyKeys

f6eb11b

mvanschellebeeck added 3 commits March 17, 2025 10:06

Wrap in WithOrderByRelevance

142b87d

Resolve conflicts

7fff07a

Fix tests + change WithOrderRelevance type

483da4a

mvanschellebeeck force-pushed the mv/knn-2 branch from 37428ee to 483da4a Compare March 17, 2025 15:38

mvanschellebeeck added 2 commits March 17, 2025 11:57

oops

2864ee5

Remove tgz

d68dcda

ssanjay1 reviewed Mar 17, 2025

View reviewed changes

mvanschellebeeck closed this Mar 31, 2025


		readonly $title: string \| undefined;

		readonly $score?: number \| undefined;

Add nearest neighbors support #1248

Add nearest neighbors support #1248

Uh oh!

Conversation

mvanschellebeeck commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nihalbhatnagar Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvanschellebeeck Mar 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nihalbhatnagar Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mvanschellebeeck commented Feb 27, 2025 •

edited

Loading

nihalbhatnagar Feb 28, 2025 •

edited

Loading

mvanschellebeeck Mar 3, 2025 •

edited

Loading

nihalbhatnagar Feb 28, 2025 •

edited

Loading