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

feat: OpenAIGeneration model for embedder #9474

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

jordanh
Copy link
Contributor

@jordanh jordanh commented Feb 29, 2024

Description

Implements #9436

Demo

...
embedder:     ...summarizing retrospectiveDiscussionTopic:nzyvX6Nbzy:nzyw41YGfa for Embeddings_ember_1
embedder: completed retrospectiveDiscussionTopic:nzyvX6Nbzy:nzyw41YGfa -> Embeddings_ember_1
...

Original fullText (names redacted):

A topic "Culture & Departures" was discussed during the meeting "Retro
#108" that followed the "Hero’s Journey 👑 + Kudos" template.

Participants were prompted with, "Cavern: What challenges lie ahead?".

- Anonymous wrote, "(long, sorry!) I hope the majority of the company
  does not think that X was treated unfairly. I also hope they not
  think that the Sales team is a bunch of "sharks" who aren't willing to
  accommodate people with different personality types. From my
  perspective X was given nothing but support and help. A LOT of
  help. Despite that, X was consistently the lowest performer on the
  team by a large margin.

I was very frustrated by X's going away message, which to me came across
as selfish, immature, and unnecessarily inflammatory. No company is
perfect, we can all improve. I like X a lot as a person. But I don't
think X was treated poorly during his time here and it bothers me
that X left how he did, making all those accusations. - Y "

- Anonymous wrote, "I feel that the recent departures somehow impact the
  trust of many members towards how things are handled, and we need now
  more than ever to regroup, be very transparent and reflect on how we
  can do better as a team "
- Anonymous wrote, "I didn't like X's insinuations that we are kinda
  sorta racist. I think he would have been the first person to tell you
  that Z (former sales member) was bad at their job.

We can be better at DE&I, but I also see that our company has a wide
variety of people from all kinds of backgrounds working together and
thriving. Let's not lose sight of that. "

- Anonymous wrote, "I feel like we have had some weird days, maybe even
  with a bit of tension. Watching P's new TT video made me reflect
  on some silent meetings we have sat on during the past weeks. I don't
  think this is a very big issue, but I wonder if we are keeping some
  tensions unsaid."
- Anonymous wrote, "X's message hit me hard, it made me wonder how
  much his perspective or general ... concern? is shared"

OpenAI GPT 3.5 Turbo:

During the meeting "Retro #108", the topic of "Culture & Departures" was discussed following the "Hero’s Journey 👑 + Kudos" template. Participants were prompted with the question, "Cavern: What challenges lie ahead?". Various anonymous comments were made regarding recent departures impacting trust within the team, reflections on diversity, tensions within the company, and differing perspectives on the departure of a former team member. The discussion highlighted the importance of transparency, reflection, and unity in moving forward as a team.

Compare with Zephyr:

 During the "Retro #108" meeting, using the "Hero’s Journey 👑 + Kudos" template, the topic of culture and departures was discussed. Participants were prompted to contemplate the challenges that lie ahead in the cavernous space. An anonymous individual expressed their hope that the majority of the company did not view X's departure as unfair or interpret the Sales team's actions as unaccommodating towards those with different personality types. The writer described X as receiving a significant amount of assistance and support, yet continuously being the lowest-performing member of the team by a substantial margin. They expressed frustration with X's dismissal message, which they felt came across as selfish, immature, and inflammatory. Another anonymous individual voiced concerns regarding the impact of recent departures on members' trust in the handling of issues. They suggested that the company should be transparent and reflect on how to improve as a team. A third anonymous writer disapproved of X insinuations about potential racism within the company, highlighting Z's poor job performance as evidence. The individual expressed concern regarding the company's efforts towards DE&I but acknowledged that it employs a diverse group of individuals who thrive together. Another anonymous writer reflected on some tense meetings and wondered if there were unsaid tensions present. A final anonymous writer was moved by X's farewell message and questioned how widely held perspectives or concerns might be.

Testing scenarios

If you have a set of embeddings generated using a local LLM for summarization, I recommend running a query such as...

UPDATE "EmbeddingsMetadata" SET "models" = '{}'
WHERE id IN (
    SELECT em.id
    FROM "EmbeddingsMetadata" em
    JOIN "Embeddings_ember_1" e ON em.id = e."embeddingsMetadataId"
    WHERE NOT "embedText" IS NULL
)

...in order to cause the embedder to re-embed rows that required summarization.

Make sure to enable this summarizer in .env by setting:

AI_EMBEDDING_MODELS='[{"model": "text-embeddings-inference:llmrails/ember-v1", "url": "http://localhost:3040/"}]'
AI_GENERATION_MODELS='[{"model": "openai:gpt-3.5-turbo-0125"}]'
AI_EMBEDDER_ENABLED='true'
OPEN_AI_API_KEY='sk-xxxxx.....'
OPEN_AI_ORG_ID='org-yyyyy....'
  • Run embedder
    • Observe summaries being generated, embeddings added

Final checklist

  • I checked the code review guidelines
  • I have performed a self-review of my code, the same way I'd do it for any other team member
  • I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
  • Whenever I took a non-obvious choice I added a comment explaining why I did it this way
  • I added the label Skip Maintainer Review Indicating the PR only requires reviewer review and can be merged right after it's approved if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
  • PR title is human readable and could be used in changelog

@@ -11,7 +11,6 @@ export interface GenerationModelConfig extends ModelConfig {}

export abstract class AbstractModel {
public readonly url?: string
public modelInstance: any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was unused. Good cruft to remove

@@ -57,7 +56,6 @@ export interface GenerationOptions {
temperature?: number
topK?: number
topP?: number
truncate?: boolean
Copy link
Contributor Author

@jordanh jordanh Feb 29, 2024

Choose a reason for hiding this comment

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

This is not supported by OpenAI, I believe it truncates by default. I changed the TextGenerationInference implementation to always truncate and removed this option.

throw new Error(eMsg)
}
const {maxNewTokens: max_tokens = 512, seed, stop, temperature = 0.8, topP: top_p} = options
const prompt = `Create a brief, one-paragraph summary of the following: ${content}`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Observing the differences between Zephyr and gpt-3.5-turbo, I have a feeling we may need to play with this prompt a bit to maximize which features are included in the summary. There are choices we may want to consider:

  • Preserving names vs. using pronouns if they are known
  • Attempting to summarize each thread into a chunk of text, rather than summarizing the entire block

I'm not sure yet, though, so I want to forge ahead and try and do some reporting before iterating on this pipeline.

console.log(`embedder: ...summarizing ${itemKey} for ${modelTable}`)
embedText = await generator.summarize(fullText, summarizeOptions)
embedText = await generator.summarize(fullText, {maxNewTokens: maxInputTokens})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a bug here! maxInputTokens should have been maxNewTokens

@@ -52,7 +52,7 @@ export async function selectMetaToQueue(
.where(({eb, not, or, and, exists, selectFrom}) =>
and([
or([
not(eb('em.models', '<@', sql`ARRAY[${sql.ref('model')}]::varchar[]` as any) as any),
not(eb('em.models', '@>', sql`ARRAY[${sql.ref('model')}]::varchar[]` as any) as any),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a bug here! We want to check if em.models is missing any configured models. Before this change, we were checking if any configured models were missing any elements in em.models. The only reason this produced queued jobs before was because em.models defaults to NULL.

The <@ is the "is contained by" operator. It checks if the left-hand operand is contained within the right-hand operand. It returns true if the set on the right includes all elements or the specific structure on the left.

ARRAY[2,3] <@ ARRAY[1,2,3] returns true
ARRAY[2] <@ ARRAY[1,2,3] returns true (not what we want!)

The @> operator is the "contains" operator. It checks if the left-hand operand contains the right-hand operand. It returns true if the set on the left includes all elements or the specific structure on the right.

ARRAY[1,2,3] @> ARRAY[2,3] returns true
ARRAY[1,2] @> ARRAY[2,3] returns false (what we want)

The better fix is to just stop using the models array...but we've already got that on our list. This just makes the current state work as designed.

@jordanh jordanh marked this pull request as ready for review February 29, 2024 06:18
@jordanh
Copy link
Contributor Author

jordanh commented Feb 29, 2024

@mattkrick this is ready for review. Wanted to knock this one out. Note: there are a few bug fixes in here, too.

@mattkrick
Copy link
Member

didn't run a test but code looks good! i figure we'll do plenty of testing before we turn the thing on

@mattkrick mattkrick merged commit 807e347 into master Feb 29, 2024
7 checks passed
@mattkrick mattkrick deleted the feat/embedder-openai-generator branch February 29, 2024 19:44
@github-actions github-actions bot mentioned this pull request Mar 1, 2024
24 tasks
mattkrick added a commit that referenced this pull request Mar 1, 2024
* chore(env vars): Stripe vars moved to the Integrations section (#9427)

* chore: fix misleading `isLead` field name on `Team` (#9413)

* chore: fix misleading `isLead` field name on `Team`

The field indicates whether the viewer is the lead, but when used in a
query for a different user, the result could be read wrong.

* Fix Team.isLead dependencies

* feat: remove team template limit (#9424)

* update error message and increase template limit

* remove max team template limits

* remove canClone prop from CloneTemplate

* remove unused threshold

* remove unused threshold

* feat: Add Google calendar meeting series for recurrence (#9380)

* feat: Add recurrence to GCal events

* Fun with timezones

* fix: Increase the number of projects fetched per request from Atlassian (#9435)

We ran into timeouts in `getAllProjects`, presumably because we're doing
too many roundtrips. As a quick fix, increse the number of projects
fetched per request from 50 to 500.

* chore(deps): bump ip from 1.1.8 to 1.1.9 (#9442)

Bumps [ip](https://github.com/indutny/node-ip) from 1.1.8 to 1.1.9.
- [Commits](indutny/node-ip@v1.1.8...v1.1.9)

---
updated-dependencies:
- dependency-name: ip
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(release): release v7.17.0 (#9428)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* feat(standalone-deployment): Standalone host deployment improved and documented (#9445)

* Docker compose stack improved

* Remove unused containers from docker-compse and add useful comment on .env.example about PGSSLMODE

* Docker compose profiles added. Documentation extended on how to use the profiles to manage the stack.

* README fixed as docker compose up and down commands were not working

* Typo fixed and docker-compose command replaced by docker compose

* feat: support env-defined saml issuer for PPMIs (#9455)

* feat: support env-defined saml issuer for PPMIs

Signed-off-by: Matt Krick <[email protected]>

* feat: support single SAML for entire tenant

Signed-off-by: Matt Krick <[email protected]>

---------

Signed-off-by: Matt Krick <[email protected]>

* chore: Associate logs with traces (#9444)

* chore: Associate logs with traces

Add trace information to log output for server side log statements. This
does not include logging from code exclusively used for debugging,
deploying or development.

* Actually add the logger

* Fix DD_LOGS_INJECTION check

* chore(release): release v7.18.0 (#9450)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* chore: no force-push to prod (#9401)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.18.1 (#9459)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* feat: embedder service (#9417)

* feat: add embedder service

---------

Signed-off-by: Matt Krick <[email protected]>
Co-authored-by: Matt Krick <[email protected]>

* merge production to avoid force push (#9461)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.19.0 (#9460)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: checkout prod before merging it (#9463)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.19.1 (#9464)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: mrege origin/production strategy (#9465)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.19.2 (#9466)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: force push 5 (#9467)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.19.3 (#9468)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: limit invites from spammers (#9416)

* fix: limit invites from spammers

* update where we check pending emails

* check total plus pending invites

* use invitees instead of pending

* fix: Fetch Jira projects in parallel (#9456)

Previously we tried to fetch more projects per page, but Jira only ever
returns 50 max. Instead once we know how many projects there are after
fetching the first page, we fetch all remaining pages in parallel.

* fix: replace lone surrogates in draft-js content (#9415)

* fix: replace lone surrogates in draft-js content

Signed-off-by: Matt Krick <[email protected]>

* fix typo

Signed-off-by: Matt Krick <[email protected]>

* fix: eslint errors

Signed-off-by: Matt Krick <[email protected]>

---------

Signed-off-by: Matt Krick <[email protected]>

* chore(deps): bump es5-ext from 0.10.62 to 0.10.64 (#9457)

Bumps [es5-ext](https://github.com/medikoo/es5-ext) from 0.10.62 to 0.10.64.
- [Release notes](https://github.com/medikoo/es5-ext/releases)
- [Changelog](https://github.com/medikoo/es5-ext/blob/main/CHANGELOG.md)
- [Commits](medikoo/es5-ext@v0.10.62...v0.10.64)

---
updated-dependencies:
- dependency-name: es5-ext
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: packages/server/package.json to reduce vulnerabilities (#9434)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-UNDICI-6252336

Co-authored-by: snyk-bot <[email protected]>

* fix: packages/server/package.json to reduce vulnerabilities (#9392)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-NODEMAILER-6219989

Co-authored-by: snyk-bot <[email protected]>

* fix: packages/server/package.json to reduce vulnerabilities (#9298)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-FOLLOWREDIRECTS-6141137

Co-authored-by: snyk-bot <[email protected]>

* chore(deps): bump follow-redirects from 1.14.8 to 1.15.4 (#9312)

Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.8 to 1.15.4.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.14.8...v1.15.4)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Matt Krick <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Matt Krick <[email protected]>

* chore: add upload to GCS step in ironbank (#9471)

* add upload to GCS step in ironbank

* update workflow name

* chore(release): release v7.19.4 (#9470)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: Fix seasonal templates for leap years (#9476)

* fix: Fix seasonal templates for leap years

It would produce invalid dates on February 29th.

* Master was not clean

* chore(release): release v7.19.5 (#9477)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix: After parameter for meetingCount was ignored (#9479)

* chore(docker-build): simplify the docker build process and reduce docker image size (#9447)

* Dockerfile basic created. Improvements added to reduce build time and size (down from 795MB to 445MB, depending on systemtap). Readme reduced, removing the old process used to build the image.

* basic-env file using a RethinkDB database name that is clearly dedicated to the building proces.

* Readme improved to run all three components

* Unused dockerfiles removed. Docker entrypoint renamed. Docker Readme adapted

* Legacy build kept in both dockerfile and env file. Readme adapted to use the new basic image. Build GH workflow adapted to use the new basic.dockerfile.

* chore(release): release v7.19.6 (#9480)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* fix(docker-build): home folder is /home/node now (#9482)

* chore(release): release v7.19.7 (#9483)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

* feat: OpenAIGeneration model for embedder (#9474)

* fix: support single-tenant saml record (#9486)

Signed-off-by: Matt Krick <[email protected]>

* chore(release): release v7.20.0 (#9485)

Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Matt Krick <[email protected]>
Co-authored-by: Rafa <[email protected]>
Co-authored-by: Georg Bremer <[email protected]>
Co-authored-by: Nick O'Ferrall <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: parabol-release-bot[bot] <150284312+parabol-release-bot[bot]@users.noreply.github.com>
Co-authored-by: Matt Krick <[email protected]>
Co-authored-by: Jordan Husney <[email protected]>
Co-authored-by: adaniels-parabol <[email protected]>
Co-authored-by: snyk-bot <[email protected]>
Co-authored-by: Dale Bumblis <[email protected]>
Co-authored-by: github-actions <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants