-
Notifications
You must be signed in to change notification settings - Fork 5k
Sanitize image markdown in sources #2765
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
Merged
+57
−6
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,6 +6,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from azure.search.documents.models import VectorizedQuery | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from openai.types.chat import ChatCompletion | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from approaches.approach import Document | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from approaches.chatreadretrieveread import ChatReadRetrieveReadApproach | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from approaches.promptmanager import PromptyManager | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from prepdocslib.embeddings import ImageEmbeddings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -255,3 +256,52 @@ async def test_compute_multimodal_embedding_no_client(): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Test that calling compute_multimodal_embedding raises a ValueError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
with pytest.raises(ValueError, match="Approach is missing an image embeddings client for multimodal queries"): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
await chat_approach.compute_multimodal_embedding("What's in this image?") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@pytest.mark.asyncio | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async def test_chat_prompt_render_with_image_directive(chat_approach): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"""Verify DocFX style :::image directive is sanitized (replaced with [image]) during prompt rendering.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
image_directive = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"activator-introduction.md#page=1: Intro text before image. " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
':::image type="content" source="./media/activator-introduction/activator.png" ' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
'alt-text="Diagram that shows the architecture of Fabric Activator."::: More text after image.' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async def build_sources(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return await chat_approach.get_sources_content( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Document( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id="doc1", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
content=image_directive.split(": ", 1)[1], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sourcepage="activator-introduction.md#page=1", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+264
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] The test string contains a magic value that mixes citation format with content. Consider extracting the citation prefix and content into separate variables to make the test structure clearer and more maintainable.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sourcefile="activator-introduction.md", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
use_semantic_captions=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
include_text_sources=True, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
download_image_sources=False, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
user_oid=None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
data_points = await build_sources() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
messages = chat_approach.prompt_manager.render_prompt( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
chat_approach.answer_prompt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"include_follow_up_questions": False, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"past_messages": [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"user_query": "What is Fabric Activator?", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"text_sources": data_points.text, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"image_sources": data_points.images, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"citations": data_points.citations, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assert messages | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Find the user message containing Sources and verify placeholder | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
combined = "\n".join([m["content"] for m in messages if m["role"] == "user"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Expect triple colons escaped | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assert ":::image" in combined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assert "activator-introduction/activator.png" in combined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assert "Diagram that shows the architecture of Fabric Activator." in combined | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Original unescaped sequence should be gone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
assert ":::image" not in combined |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
[nitpick] The function modifies and reassigns the same variable 's' multiple times. Consider using a more functional approach by chaining the operations or using intermediate variables for better readability.
Copilot uses AI. Check for mistakes.