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(wrappers): add decoration wrapper #63

Conversation

Dam-Buty
Copy link
Contributor

@Dam-Buty Dam-Buty commented Aug 30, 2024

To test this PR :

  • check it out locally
  • run the Literal Api on this branch : damien/eng-1724-integrations-change-how-we-handle-metadatatags
  • unskip all sections marked with "Skip for the CI"
  • run the tests with a .env containing an OPENAI_API_KEY.

Changes

  • API : add possibility to attach a generation to an existing step (unused in the SDK but exposed to users)
await literalClient.api.createGeneration(generation, stepId)
  • OpenAI :
    • fix option to add metadata/tags at the call level
    • add option to specify a Step ID at the call level
    • fix an issue when the same OpenAI client is instrumented more than once (mostly useful for the tests)
const openai_ = new OpenAI();

const openai = client.instrumentation.openai({
  // the initial client needs to be passed when instrumenting
  client: openai_
});

await openai.chat.completions.create(
  {
    model: 'gpt-3.5-turbo',
    messages: [
      { role: 'system', content: 'You are a helpful assistant.' },
      { role: 'user', content: 'What is the capital of Canada?' }
    ]
  },
  {
    literalaiTags: ['tag3', 'tag4'],
    literalaiMetadata: { otherKey: 'otherValue' },
    // You can also specify a Step ID at the call level
    // When this generation is logged, it will be created with the given Step ID
    literalaiStepId
  }
);
  • Langchain
    • add an option to specify a Step ID in a call's metadata
    • add tags/metadata from LangChain to Literal AI generations
const literalaiStepId = uuidv4();

await model.invoke('Hello, how are you?', {
  callbacks: [cb],
  metadata: {
    key: 'value',
    // use literalaiStepId in the metadata to specify a Step ID
    literalaiStepId,
  },
  tags: ['tag1', 'tag2'],
});
  • Vercel AI SDK : add option to add tags, metadata and a step ID at the call level
const { text } = await generateText({
  model: openai('gpt-3.5-turbo'),
  prompt: question,
  literalaiStepId,
  literalaiTags: ['tag1', 'tag2'],
  literalaiMetadata: { otherKey: 'otherValue' }
});
  • Decorate wrapper : this wrapper allows you to store tags, metadata and a step ID inside the context
    • Threads, steps and generations logged inside the context will inherit the tags & metadata
    • The first generation/step created inside the context will inherit the step ID
await client.decorate({ metadata, tags, stepId }).wrap(async () => {
  // This generation will be logged with the provided stepId, metadata and tags
  const completion = await openai.chat.completions.create({
    model: 'gpt-4',
    messages: [{ role: 'user', content: 'Say hello !' }]
  });

  // Because step IDs are unique, this call will revert to the default behaviour of
  // assigning a random UUID to the generation when it is logged.
  const completion = await openai.chat.completions.create({
    model: 'gpt-4',
    messages: [{ role: 'user', content: 'Say hello !' }]
  });
})

openai.ts Outdated
@@ -0,0 +1,141 @@
import 'dotenv/config';
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a test file?

Copy link
Contributor

Choose a reason for hiding this comment

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

Done
Moved to an example folder

package.json Outdated
@@ -52,6 +52,7 @@
"zod-to-json-schema": "^3.23.0"
},
"dependencies": {
"@langchain/openai": "^0.2.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be a peer dependency

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

prompt.ts Outdated
@@ -0,0 +1,13 @@
import 'dotenv/config';
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a test file

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

}
} catch (e) {
console.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

one log should suffice

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@@ -119,6 +123,9 @@ describe('End to end tests for the SDK', function () {

await client.api.deleteThread(thread.id);

// We have to await 5 seconds for the thread to disappear from the cache
Copy link
Contributor

Choose a reason for hiding this comment

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

related to the cache PR but I think we should get rid of it

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@clementsirieix clementsirieix force-pushed the damien/eng-1724-integrations-change-how-we-handle-metadatatags branch from acf33a4 to 773dba5 Compare September 12, 2024 13:08
@clementsirieix clementsirieix force-pushed the damien/eng-1724-integrations-change-how-we-handle-metadatatags branch from 3a05e37 to 05bb5e8 Compare September 12, 2024 13:25
@clementsirieix clementsirieix merged commit fa9fc2e into main Sep 12, 2024
2 checks passed
@clementsirieix clementsirieix deleted the damien/eng-1724-integrations-change-how-we-handle-metadatatags branch September 12, 2024 14:05
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