Document high-level ingestion methods#261
Conversation
|
@bmerkle need your Approval. :-) |
There was a problem hiding this comment.
Pull request overview
Updates the high-level API documentation for ConversationBase ingestion methods so users can understand the recommended message ingestion entrypoints.
Changes:
- Documented
ConversationBase.add_messages_with_indexingmethod signature and behavior. - Documented
ConversationBase.add_messages_streamingmethod signature and intended use case. - Minor wording/formatting tweaks in
docs/high-level-api.md.
| timestamp: str | None = None, # ISO timestamp in UTC with 'z' suffix | ||
| metadata: ConversationMessageMeta, # See below | ||
| metadata: ConversationMessageMeta, # See below |
There was a problem hiding this comment.
Docs say the timestamp uses a UTC 'z' suffix (lowercase), but the implementation uses an explicit 'Z' suffix (uppercase) for UTC (see format_timestamp_utc and ConversationMessage.timestamp docs). This example format should be updated to avoid generating/accepting the wrong timestamp format.
| `typeagent.create_conversation` described below. | ||
|
|
||
| It has one public method: | ||
| It has these public methods: |
There was a problem hiding this comment.
The doc now says ConversationBase “has these public methods”, but ConversationBase also exposes other non-underscored methods (e.g., create and add_metadata_to_index). Either document these as well or adjust the wording to avoid implying the list is exhaustive.
| It has these public methods: | |
| Some of its public methods are: |
| ``` | ||
|
|
||
| Adds messages and updates all indexes in a single transaction. | ||
| For SQLite storage this is all-or-nothing. |
There was a problem hiding this comment.
add_messages_with_indexing is documented as “all-or-nothing” for SQLite, but the implementation explicitly notes that in-memory storage has no rollback support and partial changes may remain on error. Consider adding a brief note about the in-memory behavior here so callers don’t assume transactional semantics when dbname=None.
| For SQLite storage this is all-or-nothing. | |
| For SQLite storage this is all-or-nothing. When using in-memory | |
| storage (`dbname=None`), rollback is not supported, so partial | |
| changes may remain if an error occurs. |
| ``` | ||
|
|
||
| Adds messages from an async stream, committing each batch separately. | ||
| Useful for very large ingestions where one large transaction is impractical. |
There was a problem hiding this comment.
add_messages_streaming also performs source-id based skipping (already-ingested messages are silently skipped when source_id is set). This affects messages_added vs. the number of streamed messages; consider documenting this behavior since it’s an important semantic difference from naive “stream + commit” ingestion.
| Useful for very large ingestions where one large transaction is impractical. | |
| Useful for very large ingestions where one large transaction is impractical. | |
| When source-id based deduplication is enabled, messages that were already | |
| ingested may be silently skipped during streaming. As a result, | |
| `AddMessagesResult.messages_added` may be smaller than the total number of | |
| messages yielded by the async iterable. |
I wanted to add docs for
add_messages_streamingand found thatadd_messages_with_indexingalso wasn't documented. Added both.