Conversation
📚 Mintlify Preview Links📝 Changed (3 total)📄 Pages (2)
⚙️ Other (1)
🤖 Generated automatically when Mintlify deployment succeeds |
🔗 Link Checker Results✅ All links are valid! No broken links were detected. Checked against: https://wb-21fd5541-openai-agent-ts-docs-1934.mintlify.app |
chance-wnb
left a comment
chance-wnb
left a comment
There was a problem hiding this comment.
Thank you so much Anastasia. Overall this looks great! 👍 I thought about some minor details. Can you help me verify a few things please?
| In the following code sample, an OpenAI Agent is created and integrated with Weave for traceability. First, update `your-team-name/your-project-name` in the Weave project initialization to real values. A `get_weather` tool is defined that returns a sample weather report. An agent named `Hello world` is configured with basic instructions and access to the weather tool. The main function runs the agent with a sample input (`What's the weather in Tokyo?`) and outputs the final response. | ||
|
|
||
| ```typescript | ||
| import * as weave from "weave"; |
There was a problem hiding this comment.
in the CommonJS way, since we are not relying on pre-loader, if we move import * as weave from "weave"; after import { Agent, run, tool } from "@openai/agents"; (which means the import of @openai/agents runs first), does it still work I wonder?
If it doesn't, and for some reason users are unable to put import * as weave from "weave"; first. We can suggest them to manually patch in such a situtation.
If it still works, then we are good!
There was a problem hiding this comment.
in cjs, we don't use import, so assuming for cjs you meant
const weave = require("weave");
const { Agent, run, tool } = require("@openai/agents");
changed to
const { Agent, run, tool } = require("@openai/agents");
const weave = require("weave");
no, it didn't work.
So i will call this ordering out specifically, good catch. Tho calling this out might be a little confusing since the import code is different from the example, i'll see what I can do
(just to be sure i did test this for the esm version and it was fine)
There was a problem hiding this comment.
I see. what I said earlier is confusing, let me clarify: regardless of whether it is CJS or ESM, people always write it as:
import * as weave from "weave";
import { Agent, run, tool } from "@openai/agents";
as TypeScript. This looks identical like ESM, but it is TypeScript :) It just happens that TypeScript uses the modern ESM syntax.
Eventually, under CJS config, it will be configured to:
const weave = require("weave");
const { Agent, run, tool } = require("@openai/agents");
…tup, clarified import ordering for CommonJS agent integration
| In the following code sample, an OpenAI Agent is created and integrated with Weave for traceability. First, update `your-team-name/your-project-name` in the Weave project initialization to real values. A `get_weather` tool is defined that returns a sample weather report. An agent named `Hello world` is configured with basic instructions and access to the weather tool. The main function runs the agent with a sample input (`What's the weather in Tokyo?`) and outputs the final response. | ||
|
|
||
| ```typescript | ||
| import * as weave from "weave"; |
There was a problem hiding this comment.
I see. what I said earlier is confusing, let me clarify: regardless of whether it is CJS or ESM, people always write it as:
import * as weave from "weave";
import { Agent, run, tool } from "@openai/agents";
as TypeScript. This looks identical like ESM, but it is TypeScript :) It just happens that TypeScript uses the modern ESM syntax.
Eventually, under CJS config, it will be configured to:
const weave = require("weave");
const { Agent, run, tool } = require("@openai/agents");
|
|
||
| The example code on this page uses ESM syntax. In CommonJS projects, the equivalent import statement is: | ||
| ```typescript lines | ||
| const weave = require("weave"); |
There was a problem hiding this comment.
so here maybe you also want to mention the typescript syntax as well:
import * as weave from "weave";
import { Agent, run, tool } from "@openai/agents";
and with CommonJS configuration, the order of that two import statements(which will be translated to require() in compiled code) matters.
chance-wnb
left a comment
chance-wnb
left a comment
There was a problem hiding this comment.
Thank you so much for the due diligence and attention to every detail! It is much appreciated. Great job!
weave/guides/integrations/js.mdx
Outdated
| - **`target: "es2022"`** *(Recommended)* — Emits modern JavaScript compatible with recent Node.js versions. | ||
| For details on this compiler option, see [TypeScript - Target](https://www.typescriptlang.org/tsconfig/#target). | ||
|
|
||
| - **`verbatimModuleSyntax: false`** — Allows ESM-style `import`/`export` syntax in source files while emitting CommonJS output. |
There was a problem hiding this comment.
nit: in my opinion, this doesn't seem to be a commonly-seen option. with its default value being false, we probably don't need to mention it. So the simpler the user's config is, the better.
Since tsconfig is such a huge topic, our doc is not the go-to place for every details, as long as we bring something basic that works, we already fulfill our limited liability :)
Just my two cents, and it is a nit-picky comment. I will leave it to your decision.
…ter as a FAQ instead.
2 participants
Description
Documentation for TypeScript SDK: OpenAI agents Integration
I have tested this 'getting started' code example but it does require 'await weave.instrumentOpenAIAgents();' to work.
if that's accurate, Proposing we remove these lines under Section "manual instrumentation":
In most cases, automatic instrumentation handles everything. If you need manual control, for example when using dynamic imports or bundlers that bypass module hooks, you can call
instrumentOpenAIAgents()explicitly:Testing
mint dev)mint broken-links)Related issues