-
Notifications
You must be signed in to change notification settings - Fork 46
[AIT-106] - Feature documentation accepting user input #3071
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
base: AIT-129-AIT-Docs-release-branch
Are you sure you want to change the base?
[AIT-106] - Feature documentation accepting user input #3071
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
aebe2c1 to
ea0ac8d
Compare
mschristensen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really great so far, thanks Greg. Left some suggestions and comments.
| 3. The agent receives the message, processes it, and generates a response. | ||
| 4. The agent publishes the response back to the channel, correlating it to the original input. | ||
|
|
||
| This decoupled approach means agents don't need persistent connections to individual users. Instead, they subscribe to channels and respond to messages as they arrive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This decoupled approach means agents don't need persistent connections to individual users. Instead, they subscribe to channels and respond to messages as they arrive. | |
| This decoupled approach means agents don't need to manage persistent connections to individual users. Instead, they subscribe to channels and respond to messages as they arrive. | |
| <Aside data-type="further-reading"> | |
| Learn more about channel-based communication in [channel-oriented sessions](/docs/ai-transport/features/sessions-identity#connection-oriented-vs-channel-oriented-sessions). | |
| </Aside> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| This decoupled approach means agents don't need persistent connections to individual users. Instead, they subscribe to channels and respond to messages as they arrive. | ||
|
|
||
| ## Subscribe to user input <a id="subscribe"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should appear after we've illustrated how the user input message was published.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| </Code> | ||
|
|
||
| <Aside data-type="note"> | ||
| The agent can use the `message.clientId` to identify which user sent the prompt. This is a verified identity when using [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The agent can use the `message.clientId` to identify which user sent the prompt. This is a verified identity when using [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents). | |
| The agent can use the `message.clientId` to identify which user sent the prompt. This is a verified identity when using [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| ## Identify the user <a id="identify-user"/> | ||
|
|
||
| Users must be [identified clients](/docs/auth/identified-clients) to send input to agents. This ensures the agent can trust the identity of message senders and prevents users from impersonating others. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Users must be [identified clients](/docs/auth/identified-clients) to send input to agents. This ensures the agent can trust the identity of message senders and prevents users from impersonating others. | |
| Use [identified clients](/docs/auth/identified-clients) to establish a verified identity for each user client that sends input to agents. This ensures the agent can trust the identity of message senders and prevents users from impersonating others. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The agent can use the `message.clientId` to identify which user sent the prompt. This is a verified identity when using [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents). | ||
| </Aside> | ||
|
|
||
| ## Identify the user <a id="identify-user"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the simplest requirement is that the sender has a verified role: /docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-claims i.e. so that the agent can determine the message is from a "user" rather than e.g. another agent sharing the channel. If the agent needs to wants to know the identity of a specific user that sent a message, then verified clients should be used.
Do you think we can describe both patterns in these docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rewritten this section to use both patterns and explain why you would use each: a28b828
|
|
||
| // Track sent prompts | ||
| async function sendPrompt(prompt) { | ||
| const result = await channel.publish('user-input', { prompt }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here, can just be await channel.publish('user-input', prompt);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we not want the result if we're using the newer version of Ably-js?
Such as:
async function sendPrompt(prompt) {
const result = await channel.publish('user-input', prompt);
pendingPrompts.set(result.serial, { prompt });
return result.serial;
}
| // Track sent prompts | ||
| async function sendPrompt(prompt) { | ||
| const result = await channel.publish('user-input', { prompt }); | ||
| pendingPrompts.set(result.id, { prompt, sentAt: Date.now() }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid adding unrelated functionality to code examples, so lets just store the prompt and exclude the sentAt
| </Code> | ||
|
|
||
| <Aside data-type="note"> | ||
| When appending tokens, include the `extras` with all headers to preserve them on the message. If you omit `extras` from an append operation, any existing headers will be removed. See [token streaming](/docs/ai-transport/features/token-streaming/message-per-response) for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| When appending tokens, include the `extras` with all headers to preserve them on the message. If you omit `extras` from an append operation, any existing headers will be removed. See [token streaming](/docs/ai-transport/features/token-streaming/message-per-response) for more details. | |
| When appending tokens, include the `extras` with all headers to preserve them on the message. If you omit `extras` from an append operation, any existing headers will be removed. See token streaming with the [message per response](/docs/ai-transport/features/token-streaming/message-per-response) pattern for more details. |
| activeRequests.set(inputMessageId, { | ||
| userId, | ||
| prompt: message.data.prompt, | ||
| startedAt: Date.now() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, this is superfluous to requirements
src/data/nav/aitransport.ts
Outdated
| name: 'Messaging', | ||
| pages: [ | ||
| { | ||
| name: 'User input', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On reflection, can we call this Accepting user input, which aligns with the style used in other sections?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've also renamed the file so it follows the same naming for the url.
16f6594 to
95627c1
Compare
|
Thank you @mschristensen I've updated most. There's a couple comments on others though. |
mschristensen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, a couple of last comments but approving ahead of that
| ### Establish user identity <a id="user-identity"/> | ||
| Use [identified clients](/docs/auth/identified-clients) when the agent needs to know which specific user sent a message. This enables personalized responses, per-user rate limiting, or access control based on user identity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For parity with the section above, I think this should link to the docs on the sessions & identity section: /docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity
And then the link at the end of this section can link to /docs/auth/identified-clients
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function getJWTClaims(userId) { | ||
| return { | ||
| "x-ably-clientId": userId, | ||
| "ably.channel.*": "user", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't include the user claim here, as that is not what is being documented, and is covered in the section above. Requires a few small tweaks to the following content.
(In general I think it works well when each section is orthogonal, so that the reader doesn't have to rely on context in one section that was introduced in some prior section)
| const channel = ably.channels.get('conversation:{{RANDOM_CHANNEL_NAME}}'); | ||
|
|
||
| const result = await channel.publish('user-input', 'What is the weather like today?'); | ||
| console.log(`Published message with serial: ${result.serial}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be:
| console.log(`Published message with serial: ${result.serial}`); | |
| console.log(`Published message with serial: ${result.serials[0]}`); |
| The agent can use the `message.clientId` to identify which user sent the prompt. This is a verified identity when using [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity). | ||
| </Aside> | ||
| ## Correlate responses to input <a id="correlate"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be "Publish agent responses"
mschristensen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing - I know I suggested using the serial. However I noticed that most other docs use a user-generated UUID for correlation (e.g. in the HITL docs). I think we should use that approach here too, instead of the serial:
- Removes dependency on a specific SDK version
- Handles the case when the ACK arrives after the response (unlikely, but I think possible)
- Avoids dependency on Ably-assigned identifiers. I know we use them in appends for the message-per-response model, but we plan to add support for a client-specified identifier in this case too.
Document how users send prompts to AI agents over Ably channels, including identified clients, message correlation, and handling concurrent prompts.
faaa6a7 to
f346f14
Compare
I've updated the page to use a promptId which is user-generated UUID. All changes for this can be found in: 609ad8e |
mschristensen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, last couple of comments mostly to align with the improvements we ended up making on the HITL docs
| ``` | ||
| </Code> | ||
| ### Establish user identity <a id="user-identity"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For alignment with the preceding heading, I think this should be:
| ### Establish user identity <a id="user-identity"/> | |
| ### Verify user identity <a id="verify-identity"/> |
| ### Establish user identity <a id="user-identity"/> | ||
| Use [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity) when the agent needs to know which specific user sent a message. This enables personalized responses, per-user rate limiting, or access control based on user identity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should mention rate limiting, to avoid potential confusion with Ably rate limits, and avoid referencing access control, which is not specifically a concern wrt. user input:
| Use [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity) when the agent needs to know which specific user sent a message. This enables personalized responses, per-user rate limiting, or access control based on user identity. | |
| Use [identified clients](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-identity) when the agent needs to know which specific user sent a message. This enables personalized responses or custom behaviour based on user identity. |
|
|
||
| Use [user claims](/docs/ai-transport/features/sessions-identity/identifying-users-and-agents#user-claims) to verify that a message comes from a user rather than another agent sharing the channel. This is the simplest approach when the agent only needs to distinguish message sources. | ||
|
|
||
| Your auth server embeds a role claim in the user's token: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we align the role/identity stuff with the approach we used in HITL?
I.e. an Aside with links to the relevant sections, code snippet with just the claim object definitions, and leaving out the auth endpoint definition which is covered elsewhere? Can probably align some of the copy here too
| // The auth endpoint returns a token with the user's clientId embedded | ||
| }); | ||
|
|
||
| const channel = ably.channels.get('conversation:{{RANDOM_CHANNEL_NAME}}'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for the conversation: prefix? We excluded in HITL and can do so here (and any claim can be specified on ably.channels.*)
Description
Add documentation for user input messaging in AI Transport, covering how users send prompts to AI agents over Ably channels.
Topics covered:
clientId