Content transformers: restore PDF/XLSX + describe images on save (get never returns bytes)#396
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes agent/MCP get behavior for content-collection files by restoring missing PDF/XLSX text transformations and preventing raw binary (especially images) from leaking into tool output, while also adding an image-captioning hook into the indexing pipeline via a headless/default chat client provider.
Changes:
- Restores
.pdfand.xlsx/.xlscontent transformers and registers them in the sharedIContentTransformerDI seam. - Guards image reads in
FileContentProvider(placeholder instead of decoded bytes) and routesgeton image paths to the image’sDocumentnode when available. - Adds optional image captioning to the indexing pipeline via
IImageDescriber+ChatClientImageDescriber, and introducesDefaultChatClientProvider+IChatClientFactory.CreateChatClient(model)to avoid shared mutable factory state.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.ContentCollections.Test/PdfXlsxConversionTest.cs | Covers direct transformer output and end-to-end IFileContentProvider/GetDataRequest conversion for PDF/XLSX. |
| test/MeshWeaver.ContentCollections.Test/ImageContentGuardTest.cs | Verifies image reads return a placeholder (not decoded bytes). |
| test/MeshWeaver.ContentCollections.Indexing.Test/ImageDescriptionIndexingTest.cs | Verifies image description flows into embeddings and Document summary when a describer is wired. |
| test/MeshWeaver.ContentCollections.Indexing.Test/FakeImageDescriber.cs | Deterministic describer for indexing tests and call-count assertions. |
| test/MeshWeaver.ContentCollections.Indexing.Graph.Test/ChatClientImageDescriberTest.cs | Verifies multimodal prompt shape and failure/no-client degradation for the chat-based describer. |
| test/MeshWeaver.AI.Test/ContentImagePathParsingTest.cs | Unit coverage for routing image get paths to Document nodes (and excluding recursion). |
| src/MeshWeaver.Documentation/Data/DataMesh/UnifiedPath.md | Updates UnifiedPath docs for new transformer behaviors and binary guidance. |
| src/MeshWeaver.ContentCollections/PdfPigContentTransformer.cs | Adds PDF-to-text transformer via PdfPig. |
| src/MeshWeaver.ContentCollections/MeshWeaver.ContentCollections.csproj | Adds ClosedXML + PdfPig package references. |
| src/MeshWeaver.ContentCollections/FileContentProvider.cs | Adds image placeholder guard and transformer-based conversion path. |
| src/MeshWeaver.ContentCollections/ContentCollectionsExtensions.cs | Registers PDF/XLSX transformers in AddContentService. |
| src/MeshWeaver.ContentCollections/ClosedXmlContentTransformer.cs | Adds XLS/XLSX-to-markdown transformer via ClosedXML. |
| src/MeshWeaver.ContentCollections.Indexing/IImageDescriber.cs | Introduces indexing-core abstraction for image description. |
| src/MeshWeaver.ContentCollections.Indexing/ContentIndexingService.cs | Adds describer-driven image indexing + summary override path. |
| src/MeshWeaver.ContentCollections.Indexing.Graph/ContentIndexingPipelineExtensions.cs | Wires optional image describer into DI and ContentIndexingService. |
| src/MeshWeaver.ContentCollections.Indexing.Graph/ChatClientImageDescriber.cs | Implements IImageDescriber using a multimodal IChatClient via IoPool. |
| src/MeshWeaver.AI/MeshWeaver.AI.csproj | Removes now-unneeded PDF/XLSX libs from AI project. |
| src/MeshWeaver.AI/MeshOperations.cs | Routes get on content images to Document nodes with a short timeout fallback. |
| src/MeshWeaver.AI/LanguageModelNodeType.cs | Registers DefaultChatClientProvider in the language model type DI wiring. |
| src/MeshWeaver.AI/IChatClientFactory.cs | Adds a stateless CreateChatClient(model) API (default throws). |
| src/MeshWeaver.AI/DefaultChatClientProvider.cs | New “headless default chat client” resolver for background jobs. |
| src/MeshWeaver.AI/ChatClientAgentFactory.cs | Implements IChatClientFactory.CreateChatClient(model) and adds CreateChatClientForModel. |
| src/MeshWeaver.AI.OpenAI/OpenAIChatClientAgentFactory.cs | Implements stateless bare-client construction and adjusts logging message shape. |
| src/MeshWeaver.AI.OpenAI/AzureOpenAIChatClientAgentFactory.cs | Implements stateless bare-client construction and adjusts logging message shape. |
| src/MeshWeaver.AI.AzureFoundry/AzureFoundryChatClientAgentFactory.cs | Implements stateless bare-client construction; updates error/log wording to “model”. |
| src/MeshWeaver.AI.AzureFoundry/AzureClaudeChatClientAgentFactory.cs | Implements stateless bare-client construction; updates error/log wording to “model”. |
| memex/Memex.Portal.Shared/MemexConfiguration.cs | Enables image description in the Memex indexing pipeline using the default chat client. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+50
to
+56
| var candidates = factories.Where(f => !f.IsPersistent).OrderBy(f => f.Order).ToList(); | ||
| var factory = candidates.FirstOrDefault(f => f.Supports(modelId)) ?? candidates.FirstOrDefault(); | ||
| if (factory is null) | ||
| { | ||
| logger.LogDebug("No non-persistent chat-client factory available for default model {ModelId}.", modelId); | ||
| return null; | ||
| } |
Comment on lines
+31
to
+35
| [".gif"] = "image/gif", | ||
| [".webp"] = "image/webp", | ||
| [".bmp"] = "image/bmp", | ||
| [".tiff"] = "image/tiff" | ||
| }; |
Comment on lines
+313
to
+316
| private static readonly ImmutableHashSet<string> ImageExtensions = ImmutableHashSet.Create( | ||
| StringComparer.OrdinalIgnoreCase, | ||
| ".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff"); | ||
|
|
Comment on lines
+26
to
+29
| private static readonly HashSet<string> Extensions = new(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| ".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff" | ||
| }; |
Comment on lines
+38
to
+40
| [".bmp"] = "image/bmp", | ||
| [".tiff"] = "image/tiff" | ||
| }; |
Comment on lines
+73
to
+79
| var client = chatClientFactory(); | ||
| if (client is null) | ||
| { | ||
| logger.LogInformation( | ||
| "No vision chat client available; image '{FileName}' indexed without a description.", fileName); | ||
| return string.Empty; | ||
| } |
Comment on lines
+218
to
+220
| var ext = System.IO.Path.GetExtension(fileName).ToLowerInvariant(); | ||
| if (_imageDescriber is not null && _imageDescriber.SupportedExtensions.Contains(ext)) | ||
| return _imageDescriber.Describe(bytes, fileName) |
| | `.xlsx`, `.xls` | Converted to **markdown tables**, one per worksheet (ClosedXML transformer) | | ||
| | `.md`, `.csv`, `.txt`, source files | Returned as text verbatim | | ||
| | `.xlsx`, `.pdf`, images | **No transformer yet** — bytes are decoded as text and arrive corrupted (binary is not JSON-safe) | | ||
| | images and other binaries | **No transformer** — bytes are decoded as text and arrive corrupted (binary is not JSON-safe); do not `get` these into an agent context | |
Test Results (shard 5)827 tests 826 ✅ 5m 49s ⏱️ Results for commit 83f10d8. ♻️ This comment has been updated with latest results. |
Test Results (shard 4) 9 files ± 0 9 suites ±0 2m 50s ⏱️ -22s Results for commit 83f10d8. ± Comparison against base commit c70dc2d. This pull request removes 31 and adds 7 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Test Results 58 files 58 suites 22m 33s ⏱️ Results for commit 83f10d8. ♻️ This comment has been updated with latest results. |
rbuergi
force-pushed
the
feat/content-transformers-image-description
branch
from
41ebfba to
2bb74a9
Compare
Restore the .pdf (PdfPig) and .xlsx/.xls (ClosedXML) IContentTransformers that were dropped when the old agent ContentPlugin reader was consolidated onto the content-collection transformer seam — only .docx (DocSharp) survived that move, so a PDF or spreadsheet read through `get` returned raw binary bytes decoded as text. Both register once at the shared IContentTransformer seam, so the agent `Get` tool and the MCP `get` tool return identical extracted text (PDF → page text, XLSX → one markdown table per worksheet). Also removes the now-dead ClosedXML/OpenXml/PdfPig package references from MeshWeaver.AI (leftovers from the deleted reader). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…never bytes Images carry no extractable text, so a `get` on an image returned raw bytes — flooding an agent context and failing the next model request with 400 (issue #379). Now: - On save, an image is captioned by the mesh's default multimodal model via a new IImageDescriber leaf; the description flows through the existing indexing pipeline (embedded for vector search + written as the file's Document summary). One model call per image; degrades to no-text when no vision model resolves. Wraps the description via ExtractedDocument.PlainText so it rides the positioned-chunk pipeline unchanged. - `get` on an image returns the file's Document node (name + AI description); FileContentProvider guards every image read with a short placeholder so raw bytes never reach a model context, regardless of entry point. - Adds a stateless IChatClientFactory.CreateChatClient(model) seam + DefaultChatClientProvider (resolves only a factory that Supports the model) so a background job resolves a bare chat client with no agent and no shared-factory-state mutation (also removes a latent race on the singleton factory's CurrentModelName). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rbuergi
force-pushed
the
feat/content-transformers-image-description
branch
from
2bb74a9 to
83f10d8
Compare
This was referenced Jul 10, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
A
geton a content-collection file that isn't.docxreturned raw binary decoded as text: PDFs/spreadsheets came back as%PDF…FlateDecode…, and images flooded the agent context and failed the next model request with400(issue #379). Only.docxhad survived the move to the content-collectionIContentTransformerseam.This restores the missing readers and adds machine-readable image content, all at the single shared seam both the agent
Gettool and the MCPgettool flow through — so they return identical results.Commit 1 — restore PDF + XLSX transformers
PdfPigContentTransformer(.pdf→ page text) andClosedXmlContentTransformer(.xlsx/.xls→ one markdown table per worksheet), registered once inAddContentService.ClosedXML/OpenXml/PdfPigrefs fromMeshWeaver.AI(leftovers from the old reader).Commit 2 — describe images on save;
getreturns the descriptionIImageDescriberleaf. The description flows through the existing indexing pipeline: embedded for vector search and written as the file'sDocumentsummary. One model call per image; wrapped viaExtractedDocument.PlainTextso it rides the positioned-chunk pipeline unchanged. Degrades to no-text when no vision model resolves.geton an image returns the file'sDocumentnode (name + AI description).FileContentProvideralso guards every image read with a short placeholder, so raw bytes never reach a model context regardless of entry point (the robust No agent-usable path to a PDF's full text: Get returns raw binary (documented "no transformer yet") and kills the round with 400 — agents fall back to chunk extraction the guidelines forbid #379 fix).IChatClientFactory.CreateChatClient(model)seam +DefaultChatClientProviderso a background job can resolve a bare chat client with no agent and no shared-factory-state mutation (also removes a latent race on the singleton factory'sCurrentModelName).Tests
New:
PdfXlsxConversionTest,ImageContentGuardTest,ImageDescriptionIndexingTest(+FakeImageDescriber),ChatClientImageDescriberTest,ContentImagePathParsingTest. Existing indexing/summarizer tests unchanged and green. Full solutionRelease -warnaserror(CI flags) build is green.🤖 Generated with Claude Code