Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Assistant code to RC branch #322

Merged
merged 57 commits into from
Jan 22, 2025

Conversation

aulorbe
Copy link
Contributor

@aulorbe aulorbe commented Jan 8, 2025

Description

This PR adds the code necessary to use Assistant via our Node SDK.

There are many moving parts here, so I will only call out the most impt:

  • I have divided the Assistant code into control plane ops and data plane ops, as we do with the core DB code. The one exception to this is the Evaluation code, which is spec-d in the OAS under the data plane, but architecturally makes more sense under the control plane, due to it being independent of any particular Assistant.
  • There are unit tests and integration tests for every endpoint. The control plane integration tests spin up Assistants and delete them inplace, while the data plane integration tests use the setup and teardown files to call/update a centralized Assistant in order to use our resources wisely (just like the core DB does with its data plane int. tests)
  • The data plane Assistant ops make use of a Singleton (assistantHostSingleton) much like the core DB code does with indexHostSingleton
  • There is a new function called getHostUrl in the above-mentioned Singleton. This determines which host (between the eu URL and the us URL) to route the data plane calls to. If unable to determine this host, the Singleton defaults it to the us URL.

A few things to call out for the future:

  • The Evaluation service is likely to become its own API (as heard through the grapevine) since it's quite orthogonal to the current conception of an Assistant as we have it today
  • Currently, chat and chatCompletion are essentially the same code with different interface names. The feature team tells me this is because chat is very likely to diverge in the near-time from chatCompletion with the addition of Pinecone-specific features
  • Currently, the latency between when a file is uploaded to an Assistant vs when the Assistant is queryable is very high (we have to sleep 30 secs in our integration tests). It would be ideal to cut this down somehow, although that work will probably lie with the feature team
    • The Describe file happy path int. test in particular is flakey ATM because it takes a variable amount of time for an uploaded file to be available for describing. The test waits 30s, and I'm wary of increasing that limit since I don't want to bloat our whole pipeline just waiting for this 1 test.

Misc.:
This PR includes removing support for Typescript versions <4.5, since we were running into errors with those versions and aim to maintain a 2-ish year sliding window for which versions we support. Everything pre-v4.5 was published in ~2021, so is quite old.

Basic usage

Control plane stuffs

import {Pinecone} from '@pinecone-database/pinecone';
const pc = new Pinecone();
await pc.assistant.<something>

Data plane stuffs

import {Pinecone} from '@pinecone-database/pinecone';
const pc = new Pinecone();
const assistantName = 'test';
const assistant = pc.assistant.Assistant(assistantName);
await assistant.<something>

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

CI passes.


… to Assistant feat. branch (#315)

For an unknown-as-of-right-now reason, the Typescript `fetch` generator
has been failing non-deterministically to generate code. We successfully
got it to generate on a new branch, so this PR simply merges the
generated code to the existing Assistant feature branch.

I will hunt around online to see if I can find any open Github issues
that might reveal what's going on w/the `fetch` generator...

[Note the only diff btwn these 2 branches is the generated code,
isolating the generation issues to the generator itself, not to files in
our client repo.]
@aulorbe aulorbe changed the title Add assistant Add Assistant code to RC branch Jan 9, 2025
Comment on lines +152 to +155
'~5.3.0',
'~5.4.0',
'~5.5.0',
'~5.6.0',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this Slack thread for more color on this change

@aulorbe aulorbe marked this pull request as ready for review January 9, 2025 22:17
@aulorbe aulorbe requested a review from austin-denoble January 9, 2025 22:17
Comment on lines +12 to +13
assistantName: ${{ steps.step3.outputs.ASSISTANT_NAME }}
testFile: ${{ steps.step3.outputs.TEST_FILE }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reviewers: You can ignore these diffs if you like. It's just me allowing CI to spin up a global Assistant and a file to play with.

@austin-denoble austin-denoble merged commit 700bdf6 into release-candidate/2025-01 Jan 22, 2025
29 of 30 checks passed
@austin-denoble austin-denoble deleted the add-assistant branch January 22, 2025 21:19
austin-denoble added a commit that referenced this pull request Jan 23, 2025
## Description 

This PR adds the code necessary to use
[Assistant](https://docs.pinecone.io/guides/assistant/understanding-assistant)
via our Node SDK.

There are many moving parts here, so I will only call out the most impt:
- I have divided the Assistant code into control plane ops and data
plane ops, as we do with the core DB code. The one exception to this is
the [Evaluation
code](https://docs.pinecone.io/guides/assistant/understanding-evaluation),
which is spec-d in the OAS under the data plane, but architecturally
makes more sense under the control plane, due to it being independent of
any particular Assistant.
- There are unit tests and integration tests for every endpoint. The
control plane integration tests spin up Assistants and delete them
inplace, while the data plane integration tests use the `setup` and
`teardown` files to call/update a centralized Assistant in order to use
our resources wisely (just like the core DB does with its data plane
int. tests)
- The data plane Assistant ops make use of a Singleton
(`assistantHostSingleton`) much like the core DB code does with
`indexHostSingleton`
- There is a new function called `getHostUrl` in the above-mentioned
Singleton. This determines which host (between the `eu` URL and the `us`
URL) to route the data plane calls to. If unable to determine this host,
the Singleton defaults it to the `us` URL.

A few things to call out for the future: 
- The Evaluation service is likely to become its own API (as heard
through the grapevine) since it's quite orthogonal to the current
conception of an Assistant as we have it today
- Currently, `chat` and `chatCompletion` are essentially the same code
with different interface names. The feature team tells me this is
because `chat` is very likely to diverge in the near-time from
`chatCompletion` with the addition of Pinecone-specific features
- Currently, the latency between when a file is uploaded to an Assistant
vs when the Assistant is queryable is very high (we have to sleep 30
secs in our integration tests). It would be ideal to cut this down
somehow, although that work will probably lie with the feature team
- The [Describe file happy
path](https://github.com/pinecone-io/pinecone-ts-client/actions/runs/12699145653/job/35399192319)
int. test in particular is flakey ATM because it takes a variable amount
of time for an uploaded file to be available for describing. The test
waits 30s, and I'm wary of increasing that limit since I don't want to
bloat our whole pipeline just waiting for this 1 test.

Misc.:
This PR includes removing support for Typescript versions <`4.5`, since
we were running into errors with those versions and aim to maintain a
2-ish year sliding window for which versions we support. Everything
pre-`v4.5` was published in ~2021, so is quite old.

## Basic usage
### Control plane stuffs
```typescript
import {Pinecone} from '@pinecone-database/pinecone';
const pc = new Pinecone();
await pc.assistant.<something>
```
### Data plane stuffs
```typescript
import {Pinecone} from '@pinecone-database/pinecone';
const pc = new Pinecone();
const assistantName = 'test';
const assistant = pc.assistant.Assistant(assistantName);
await assistant.<something>
```


## Type of Change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Infrastructure change (CI configs, etc)
- [x] Non-code change (docs, etc)
- [ ] None of the above: (explain here)

## Test Plan

CI passes.

---
- To see the specific tasks where the Asana app for GitHub is being
used, see below:
  - https://app.asana.com/0/0/1208894748669518

---------

Co-authored-by: Austin DeNoble <[email protected]>
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.

3 participants