Inject document content when using @-mentions#74
Conversation
|
Awesome, thanks for the PR! Will review and leave my comments. |
| ], | ||
| ) | ||
|
|
||
| async def fetch_document_for_context(self, document_id: str, context: ToolContext) -> MentionedDocumentContext | None: |
There was a problem hiding this comment.
So this func is basically the same as execute above, except for how we handle binary files. Maybe a cleaner approach would be to add a skip_binary_files: bool argument to execute and invoke that with this bool set to True instead? That way we probably won't even need the MentionedDocumentContext dataclass anymore, I think.
prvnsmpth
left a comment
prvnsmpth
left a comment
There was a problem hiding this comment.
Overall looks great! The only major detail we should reconsider is the schema change, the rest LGTM. Thanks again!
| # Inject mentioned document content into the last user message | ||
| last_msg = chat_messages[-1] | ||
| if last_msg.mentioned_document_ids: | ||
| doc_handler = DocumentToolHandler( |
There was a problem hiding this comment.
I like that we're using the document tool handler here, good choice
| if mentioned_doc_context: | ||
| context_blocks = [] | ||
| for doc in mentioned_doc_context: | ||
| context_blocks.append(f'<document doc_id="{doc.doc_id}" doc_title="{doc.title}">\n{doc.content}\n</document>') |
There was a problem hiding this comment.
Probably won't make any difference, but we don't use <document> tags when injecting the results of the read_document tool, so maybe we should do the same here?
And we have yet another way to inject documents into context when using the search_documents tool. We don't need to consolidate it in this PR, but maybe we can add a TODO here to deal with it later.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE chat_messages ADD COLUMN mentioned_document_ids JSONB NOT NULL DEFAULT '[]'; No newline at end of file | |||
There was a problem hiding this comment.
I trhink we can achieve this without making a schema change... I have a somewhat weird idea that we can consider:
- when the user mentions a doc, we inject a synthetic
read_documenttool call into the chat conversation on behalf of the LLM (role = assistant) - when the chat is executed, the main chat loop in
chat.pywill pick this up and handle the tool call automatically, no extra handling required.
No new column required, the mentioned doc IDs will be stored in the chat message content as implicit tool calls.
There's a couple issues with this though: (1) we'll have to generate our own tool call IDs and the LLM provider APIs might not play nicely with fake tool calls (2) The chat creation API (in web/src/routes/api/chat) will need to gain some knowledge of the read_document tool, which is exclusively an ai service concern.
But if we can work around this somehow, I think we'll end up with a much cleaner impl. WDYT?
17774fc to
a4755d2
Compare
- add document mention chips and persist structured mention references - expand mentions through read_document with permission and sandbox handling - enforce chat and upload ownership and add integration coverage
- restrict mention candidates to supported document content types - enforce current user, domain, group, and source permissions per request - add coverage for content types, ACL changes, and result batching
- retain expanded document text across non-Anthropic providers - stage scanned PDFs when indexed text extraction failed - prevent models from searching again for resolved mentions
- include source and content types in typeahead results - reuse the existing document icon resolver for mention suggestions - retain the generic file icon as a fallback
9bbd341 to
f4ac4d4
Compare
2 participants
Summary
Adds document
@-mentions to chat. Users can search indexed documents from the composer, attach one or more references, and send them alongside text and uploads. Mentioned document content is fetched and injected into the LLM request when the message is processed.What changed
Mention composer and rendering
Search and typeahead
Persistence and LLM context
chat_messages.messageJSON; no document content is duplicated in chat persistence.Permissions
Mention typeahead, expansion, and explicit document reads use aligned access rules:
Group/identity lookup failures fail closed. Permission bypass remains limited to contexts that explicitly set
skip_permission_check.Validation
cargo check -p omni-searcher -p shared: passedThe full repository Svelte check still reports pre-existing upstream diagnostics unrelated to this PR.
Manual testing