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

update Read me #10

Merged
merged 2 commits into from
Jan 22, 2025
Merged

update Read me #10

merged 2 commits into from
Jan 22, 2025

Conversation

adeelehsan
Copy link
Contributor

No description provided.

@adeelehsan adeelehsan requested a review from ofermend January 19, 2025 08:19
.fernignore Outdated
src/Client.ts
src/core/fetcher/Fetcher.ts
tests/integration
src/wrapper
src/index.ts
src/api/resources/auth/client/Client.ts
src/core/fetcher/getRequestBody.ts
contributing.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it usually with uppercase: CONTRIBUTING.md?

README.md Outdated
citations: {
style: "none",
},
enableFactualConsistencyScore: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have this true in the example?

generation: {
};

const generationParams: GenerationParameters = {
Copy link
Contributor

Choose a reason for hiding this comment

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

IN this example, what would be the default prompt/LLM? perhaps add to the example also that variable to show how it's used and a link to the docs for options to choose from?

README.md Outdated

### Streaming

The SDK supports streaming responses for both query and chat. When using streaming, the response will be a generator that you can iterate.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "... that you can iterate over".

README.md Outdated
}
```

And streaming the chat response:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rephrase as "And stream with chat:"

generation: generationParams
});

const responseItems = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a link in the docs you can refer to that explains the various "types" (chat_info, generation_chunk, etc)? or if not, perhaps explain this a bit.
Also we call this "chunk" in the loop, but it's not really a chunk anymore (chunk to me says part of the text). It's like "item" or "event"?

Copy link
Contributor

@ofermend ofermend left a comment

Choose a reason for hiding this comment

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

I made some comments inline. Approved once addressed.

@adeelehsan adeelehsan merged commit 5713727 into main Jan 22, 2025
2 of 3 checks passed
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.

2 participants