-
Notifications
You must be signed in to change notification settings - Fork 46
AIT-133 - citations feature documentation #3077
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-133 - citations feature documentation #3077
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
|
This needs a rebase or a different base branch |
|
|
||
| ## Setup <a id="setup"/> | ||
|
|
||
| Message annotations require the "Message annotations, updates, and deletes" [channel rule](/docs/channels#rules) enabled for your channel or [namespace](/docs/channels#namespaces). This rule automatically enables message persistence. |
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 appends
|
|
||
| ### Citation payload <a id="citation-payload"/> | ||
|
|
||
| Each citation is carried in the Ably annotation `data` field and should include: |
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.
Are we proposing this as definitive structure, or is this just a suggestion?
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 is just a suggestion and not a definitive structure. I've updated the wording to reflect that, happy to get your thoughts.
| - `startOffset`: Character position in response where citation applies (optional) | ||
| - `endOffset`: End character position for citation range (optional) | ||
| - `snippet`: Short excerpt from the source (optional) | ||
| - `sourceId`: Stable identifier for deduplication (optional) |
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.
It's unclear what the purpose of this is
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 reframed this to state the intent and why for each field, happy to get your thoughts.
|
|
||
| ## Publishing citations from agents <a id="publishing"/> | ||
|
|
||
| After publishing the response (or during streaming, if citations are available), publish citation annotations referencing the message serial. For incremental streaming using message appends, see [message-per-response](/docs/ai-transport/message-per-response). |
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 can't parse this sentence. And I don't understand the purpose of this paragraph
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.
Agree, I have reframed it now, happy to get your thoughts.
| - **Individual events**: Each citation arrives as a separate Ably event | ||
|
|
||
| <Aside data-type="note"> | ||
| Use summary view by default for efficient rendering and subscribe to raw annotations only when users need detailed citation information. This minimizes overhead while providing full transparency when required. |
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.
Should there be a mention of obtaining annotation details via the REST API instead of realtime subscription?
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.
Good point, thanks for spotting this. I've added the details with a link to the REST API
a1346cc to
64bfda3
Compare
|
@paddybyers - thanks for reviewing this, I’ve addressed all the comments. Could you take another look when you have a moment? |
paddybyers
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.
lgtm, thanks
Co-authored-by: Paddy Byers <paddy.byers@gmail.com>
No description provided.