Feature/publish typescript sdk #250
Conversation
geclos
force-pushed
the
feature/publish_typescript_sdk
branch
9 times, most recently
from
d865917 to
c3c461d
Compare
geclos
force-pushed
the
feature/publish_typescript_sdk
branch
from
c3c461d to
ddfcf19
Compare
| const factory = new Factory() | ||
|
|
||
| const schema = z.object({ | ||
| messages: z.array(messageSchema), | ||
| documentLogUuid: z.string(), | ||
| source: z.nativeEnum(LogSources).optional().default(LogSources.API), |
There was a problem hiding this comment.
public API has to be the simplest possible
| messages, | ||
| source, | ||
| source: LogSources.API, |
There was a problem hiding this comment.
I know this is not technically correct because we use ts sdk in playground but honestly i don't care
| await pipeToStream(stream, result.stream) | ||
| let id = 0 | ||
| for await (const event of streamToGenerator(result.stream)) { | ||
| const data = chainEventPresenter(event) |
There was a problem hiding this comment.
internally nothing has really changed but from the public API perspective I don't want to expose users to concepts such as providerlogs or document logs, and thus I'm introducing event presenters to filter this information out
| throw new BadRequestError( | ||
| `Unknown event type in chainEventPresenter ${JSON.stringify(event)}`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
this probably can be further simplified in the future
|
|
||
| process.on('unhandledRejection', (reason: string) => { | ||
| captureMessage(reason) | ||
| }) |
There was a problem hiding this comment.
important otherwise gateway doesn't recover from any unexpected error (hono is a bit crap honestly)
| params: { documentLogUuid, messages, source: LogSources.Playground }, | ||
| onMessage: (chainEvent) => { | ||
| stream.update(chainEvent) | ||
| const response = sdk.chat(documentLogUuid, messages, { |
There was a problem hiding this comment.
I'm changing the public API to follow the pattern of positional argument for mandatory arguments and a last hash object for any optional argument. I think it's a better dev experience.
There was a problem hiding this comment.
I'm also changing the two main methods from runDocument and addMessages to run and chat. It's easier and faster to write at the cost of sliiiightly less expresiveness.
| }, | ||
| onMessage: (chainEvent) => { | ||
| stream.update(chainEvent) | ||
| const sdk = await createSdk(projectId).then((r) => r.unwrap()) |
There was a problem hiding this comment.
projectid can now be a constructor argument so you don't have to repeat it everywhere
| host: env.GATEWAY_HOSTNAME, | ||
| port: env.GATEWAY_PORT, | ||
| ssl: env.GATEWAY_SSL, | ||
| } |
There was a problem hiding this comment.
shouldn't be needed and in fact i will probably remove this prop from the constructor of the sdk at some point
| @@ -44,32 +44,27 @@ export type ChainStepTextResponse = { | |||
| text: string | |||
| usage: CompletionTokenUsage | |||
| toolCalls: ToolCall[] | |||
| documentLogUuid: string | |||
| providerLog: undefined | |||
| documentLogUuid?: string | |||
There was a problem hiding this comment.
these can always be optional since not all provider calls are made in the context of a documentloguuid
| | { | ||
| type: ChainEventTypes.Error | ||
| error: Error | ||
| } |
There was a problem hiding this comment.
these are mostly the same than ChainEvent but with not internal concepts such as providerLog or documentLog
| onFinished, | ||
| onError, | ||
| }) | ||
| } catch (err) { |
There was a problem hiding this comment.
run and chat methods should not return a promise that can fail as otherwise it makes the dev experience kind of shit. In theory we should tell devs the way to handle any error from the sdk is through the onError callback.
Later on, we will introduce a no stream option that will return a promise and those methods can actually return failable promises
| const conversation: Message[] = [] | ||
| let chainResponse: ChainCallResponse | ||
| onEvent?.({ event: event.event as StreamEventTypes, data: json }) | ||
| } |
There was a problem hiding this comment.
bunch of improvements mostly related to error handling
| } | ||
|
|
||
| if (!uuid || !chainResponse) return |
There was a problem hiding this comment.
it means the streaming failed at some point. Should never be true but better safe than sorry.
No description provided.