Repository navigation
fix(gemini): preserve linked text and images in html and pdf exports (#110) - #140
Conversation
…110) - Resolve Gemini API <Image> tags to markdown images via candidate image tag map - Resolve Gemini API youtube_content media placeholders to markdown links - Preserve DOM citation chips, source buttons, and .sources-list in Gemini exports - Add defensive fallback in html.js markdownToHtml for <Image> XML tags - Sanitize Google Search AI client API keys in test fixtures to complete Alert #8
PR Summary by QodoPreserve Gemini links and images in HTML and PDF exports
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 3
WalkthroughGemini API conversion now resolves image tags and video placeholders from the selected candidate and collects citations from that candidate. DOM extraction preserves citation content, links, images and source-list content. The HTML formatter converts supported XML-style Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to
|
| for (const cite of citeGroup) { | ||
| if (!Array.isArray(cite)) continue; | ||
| const meta = cite[2]; | ||
| const url = | ||
| Array.isArray(meta) && |
There was a problem hiding this comment.
🟠 Medium
2. Candidate citations vanish from exports 🐞 Bug ≡ Correctness
extractActivitySources iterates through the elements of each candidate citation tuple and then looks for a URL at cite[2], rather than reading the tuple's URL-bearing field. For citations with the shape present in the API fixture, no candidate source is appended unless it also appears in the separate activity trail.
Qodo FixerNo findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page. |
There was a problem hiding this comment.
Hey - I've found 4 issues
Fixed security issues:
- packages/decant-core/tests/fixtures/google-search-ai-chat.html (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="content/formatters/html.js" line_range="1405-1416" />
<code_context>
});
+ // Fallback: normalize XML-style <Image alt="..." caption="..." src="..."/> tags into markdown images
+ mdText = mdText.replace(/<Image\s+([^>]*?)(?:\/>|>(?:<\/Image>)?)/gi, (match, attrsStr) => {
+ const altMatch = attrsStr.match(/alt="([^"]*)"/i) || attrsStr.match(/alt='([^']*)'/i);
+ const captionMatch =
+ attrsStr.match(/caption="([^"]*)"/i) || attrsStr.match(/caption='([^']*)'/i);
+ const srcMatch = attrsStr.match(/src="([^"]*)"/i) || attrsStr.match(/src='([^']*)'/i);
+ const src = srcMatch ? srcMatch[1] : '';
+ const alt = altMatch ? altMatch[1] : captionMatch ? captionMatch[1] : 'Image';
+ if (src && /^(?:https?:|data:image|blob:|\/|\.\/)/i.test(src)) {
+ return `\n\n\n\n`;
+ }
+ return '';
+ });
+
// 1. Protect fenced code blocks (``` ... ``` or ~~~ ... ~~~)
</code_context>
<issue_to_address>
**Code examples are rewritten**
When a response contains a literal `<Image ...>` example inside inline or fenced code, `markdownToHtml` rewrites matching tags before protecting code, converting them to image syntax or removing them when the `src` is unsupported; exported code no longer matches the original.
Protect inline and fenced code before rewriting image tags, or skip image-tag matches inside code.
Also at `packages/decant-core/ai/gemini.js:516-517`.
</issue_to_address>
### Comment 2
<location path="packages/decant-core/ai/gemini.js" line_range="1050-1064" />
<code_context>
const clone = markdownDiv.cloneNode(true);
+ // Extract sources from .sources-list before removal
+ const domSources = [];
+ clone.querySelectorAll(".sources-list").forEach((sl) => {
+ sl.querySelectorAll("a[href]").forEach((a) => {
+ const href = a.getAttribute("href")?.trim();
+ const title = a.textContent?.trim() || href;
+ if (
+ href &&
+ href.startsWith("http") &&
+ !domSources.some((s) => s.href === href)
+ ) {
+ domSources.push({ href, title });
+ }
+ });
+ });
+
+ // Convert citation chips and source buttons to links or text before generic button removal
+ clone
</code_context>
<issue_to_address>
**Sources appear twice**
When a model response contains linked entries in `.sources-list` inside the selected Markdown container, `parseFromDom` converts the `.sources-list` links to Markdown and then appends the same links under “Sources consulted,” so DOM exports show each source twice.
Remove `.sources-list` nodes from the clone after collecting their links, or exclude them from Markdown conversion.
Also at `packages/decant-core/ai/gemini.js:1128-1133`.
</issue_to_address>
### Comment 3
<location path="packages/decant-core/ai/gemini.js" line_range="559" />
<code_context>
+ const title = videoDetail[0] || "Video";
+ const url = videoDetail[2];
+ if (url && typeof url === "string" && url.startsWith("http")) {
+ resolved = resolved.split(tagUrl).join(`[${title}](${url})`);
+ }
+ }
</code_context>
<issue_to_address>
**Generated links are malformed**
When a video or source title contains `]`, or a URL contains an unescaped `)`, `resolveMediaPlaceholders` interpolates titles and URLs into Markdown link syntax without escaping delimiters, so `inlineParse` fails to recognize the link and the export shows raw Markdown instead of a clickable link.
Escape Markdown delimiters in link titles and URLs before generating the link syntax.
Also at `packages/decant-core/ai/gemini.js:1130`.
</issue_to_address>
### Comment 4
<location path="packages/decant-core/ai/gemini.js" line_range="1051-1063" />
<code_context>
+ // Extract sources from .sources-list before removal
+ const domSources = [];
+ clone.querySelectorAll(".sources-list").forEach((sl) => {
+ sl.querySelectorAll("a[href]").forEach((a) => {
+ const href = a.getAttribute("href")?.trim();
+ const title = a.textContent?.trim() || href;
+ if (
+ href &&
+ href.startsWith("http") &&
+ !domSources.some((s) => s.href === href)
+ ) {
+ domSources.push({ href, title });
+ }
+ });
+ });
+
</code_context>
<issue_to_address>
**DOM source links are missed**
When gemini places populated `.sources-list` links in the response footer outside `.markdown`, `parseFromDom` searches for `.sources-list` only inside a clone of `markdownDiv`, so source links in the response footer are never collected or appended to the exported message.
Collect `.sources-list` links from the full `modelResponse` or another subtree that includes the response footer before converting the markdown content.
</issue_to_address>| const title = videoDetail[0] || "Video"; | ||
| const url = videoDetail[2]; | ||
| if (url && typeof url === "string" && url.startsWith("http")) { | ||
| resolved = resolved.split(tagUrl).join(`[${title}](${url})`); |
There was a problem hiding this comment.
🟡 Medium · Generated links are malformed
When a video or source title contains ], or a URL contains an unescaped ), resolveMediaPlaceholders interpolates titles and URLs into Markdown link syntax without escaping delimiters, so inlineParse fails to recognize the link and the export shows raw Markdown instead of a clickable link.
Escape Markdown delimiters in link titles and URLs before generating the link syntax.
Also at packages/decant-core/ai/gemini.js:1130.
Prompt for AI agents
In `packages/decant-core/ai/gemini.js` at line 559:
**Generated links are malformed**
When a video or source title contains `]`, or a URL contains an unescaped `)`, `resolveMediaPlaceholders` interpolates titles and URLs into Markdown link syntax without escaping delimiters, so `inlineParse` fails to recognize the link and the export shows raw Markdown instead of a clickable link.
Escape Markdown delimiters in link titles and URLs before generating the link syntax.
Also at `packages/decant-core/ai/gemini.js:1130`.There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f6f4d507-426b-4188-b889-a53db2a61a6d
📒 Files selected for processing: 5
content/formatters/html.jspackages/decant-core/ai/gemini.jspackages/decant-core/tests/fixtures/google-search-ai-chat.htmlpackages/decant-core/tests/gemini.test.jstests/html-formatter.test.mjs
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout: 4
- GitHub Check: codeql / analyze
- GitHub Check: Sourcery review
- GitHub Check: Gitar
- GitHub Check: test-and-build
🔇 Additional comments: 5
packages/decant-core/ai/gemini.js: 3
463-468: LGTM!
850-876: LGTM!
1049-1103: LGTM!Also applies to: 1115-1128
packages/decant-core/tests/gemini.test.js: 1
223-273: LGTM!Also applies to: 275-323
tests/html-formatter.test.mjs: 1
490-522: LGTM!
| const altMatch = attrsStr.match(/alt="([^"]*)"/i) || attrsStr.match(/alt='([^']*)'/i); | ||
| const captionMatch = | ||
| attrsStr.match(/caption="([^"]*)"/i) || attrsStr.match(/caption='([^']*)'/i); | ||
| const srcMatch = attrsStr.match(/src="([^"]*)"/i) || attrsStr.match(/src='([^']*)'/i); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the complete src attribute name.
The /src=/ search also matches data-src. For <Image data-src="https://example.com/preview.png" src="https://example.com/full.png"/>, the fallback renders the preview URL instead of src. Match an attribute-name boundary so the fallback selects only src. Based on learnings, attribute extraction must distinguish overlapping attribute names.
Source: Learnings
| const src = srcMatch ? srcMatch[1] : ''; | ||
| const alt = altMatch ? altMatch[1] : captionMatch ? captionMatch[1] : 'Image'; | ||
| if (src && /^(?:https?:|data:image|blob:|\/|\.\/)/i.test(src)) { | ||
| return `\n\n\n\n`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve parentheses in image URLs.
If src is https://example.com/diagram(1).png, the generated Markdown reaches inlineParse, whose image pattern at Line 1270 stops at the first ). The rendered image then receives the truncated URL https://example.com/diagram(1, and .png) appears as text. Encode Markdown-significant URL characters before emitting the image, or pass the image to the renderer without a Markdown round trip. Based on learnings, generated Markdown destinations with parentheses need protection.
Source: Learnings
Confidence Score: 4/5[Medium impact] The changes affect Gemini conversation exports and a shared HTML export path. The bracketed-label export issue must be fixed before merging. Findings
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Keep video replacement out of code samples. · gemini.js:467
packages/decant-core/ai/gemini.js:467
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep video replacement out of code samples.
When an answer quotes a YouTube placeholder inside fenced or inline code,
resolveImageTagsrestores that code before this call.resolveMediaPlaceholdersthen changes the quoted URL to[title](url). Protect code spans across both replacements so the export preserves the sample.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f3b4adf0-67c7-4887-b69e-39ce77699a07
📒 Files selected for processing: 4
content/formatters/html.jspackages/decant-core/ai/gemini.jspackages/decant-core/tests/gemini.test.jstests/html-formatter.test.mjs
🚧 Files skipped from review as they are similar to previous changes: 1
- tests/html-formatter.test.mjs
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.45.3)
packages/decant-core/ai/gemini.js
[warning] 580-580: Detects non-literal values in regular expressions
Context: new RegExp(${escaped}(?![\\w/]), "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
🪛 GitHub Check: CodeQL
packages/decant-core/tests/gemini.test.js
[failure] 485-485: Missing regular expression anchor
When this is used as a regular expression on a URL, it may match anywhere, and arbitrary hosts may come before or after it.
🔇 Additional comments: 1
packages/decant-core/tests/gemini.test.js: 1
325-385: LGTM!Also applies to: 387-440, 442-491
| const srcMatch = attrsStr.match(/src="([^"]*)"/i) || attrsStr.match(/src='([^']*)'/i); | ||
| const src = srcMatch ? srcMatch[1] : ''; | ||
| const rawAlt = altMatch ? altMatch[1] : captionMatch ? captionMatch[1] : 'Image'; | ||
| const alt = rawAlt.replace(/[[\]]/g, '\\$&'); |
There was a problem hiding this comment.
⚠️ Bug: Escaped \[ \] in image alt text gets turned into display math
This commit escapes brackets in alt text, so a label like Paired [V1] becomes ![Paired \[V1\]](url). In markdownToHtml that escaped string reaches step 3 (line 1439), and step 3 converts \[ … \] to display math. Its (?!\() lookahead only looks at the character right after \]. Here that character is the alt's closing ], not (, so the guard doesn't stop the match. The output becomes , which breaks the image into a stray math block and literal text in HTML/PDF exports. This hits both the <Image> fallback in html.js and the Gemini output from resolveImageTags/resolveMediaPlaceholders (gemini.js:545,576), because that markdown later goes through markdownToHtml too. (inlineParse step 1, line 1187, has the same \[…\] pattern.) Possible fixes: drop or replace the brackets instead of backslash-escaping them (e.g. map [/] to (/) or remove them), or move the markdown images out into placeholders before step 3 runs. Add a test that sends an image with a bracketed alt through markdownToHtml and checks the result is an <img>.
Swap brackets for parentheses so the alt text can't be read as math delimiters (apply the same change at gemini.js:545 and 576):
const alt = rawAlt.replace(/\[/g, '(').replace(/\]/g, ')');
Was this helpful? React with 👍 / 👎
| ); | ||
|
|
||
| let replaced = protectedText.replace( | ||
| /<Image\s+([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, |
There was a problem hiding this comment.
🟠 Medium
4. Image labels break at greater-than signs 🐞 Bug ≡ Correctness
resolveImageTags stops its attribute capture at the first > instead of respecting quoted attribute values. When Gemini returns an alt or caption such as alt="a > b", the parser falls back to the default label and leaves the remainder of the tag in the response, while the HTML fallback has the same failure mode.
| : capMatch | ||
| ? capMatch[1] | ||
| : "Image"; | ||
| const alt = rawAlt.replace(/[[\]]/g, "\\$&"); |
There was a problem hiding this comment.
🟠 Medium
5. Image labels gain unintended formatting 🐞 Bug ≡ Correctness
The new image conversion escapes only square brackets before inserting alt or caption text into Markdown, so delimiters such as * remain active. During HTML conversion those delimiters become <em> markup inside the generated alt attribute, changing the label text instead of preserving the Gemini-provided description.
| /<Image\s+([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, | ||
| (match, attrs) => { | ||
| const altMatch = | ||
| attrs.match(/alt="([^"]*)"/i) || attrs.match(/alt='([^']*)'/i); | ||
| const capMatch = | ||
| attrs.match(/caption="([^"]*)"/i) || | ||
| attrs.match(/caption='([^']*)'/i); | ||
| const srcMatch = | ||
| attrs.match(/src="([^"]*)"/i) || attrs.match(/src='([^']*)'/i); | ||
|
|
||
| const src = srcMatch ? srcMatch[1] : ""; | ||
| const rawAlt = altMatch | ||
| ? altMatch[1] | ||
| : capMatch | ||
| ? capMatch[1] | ||
| : "Image"; | ||
| const alt = rawAlt.replace(/[[\]]/g, "\\$&"); | ||
|
|
||
| const resolvedUrl = | ||
| imageMap.get(src) || (src.startsWith("http") ? src : null); | ||
| if (resolvedUrl) { | ||
| return ``; |
There was a problem hiding this comment.
🟠 Medium
6. Released extension misses image fixes 🔗 Cross-repo conflict ☼ Reliability
The new resolveImageTags behavior exists only in the workspace parser, while the standalone decant package still has the old candidate-processing path. When a release build runs before decant-core is synchronized and published, the extension installs the old package and ships without the Gemini image-resolution changes.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
85755358-440f-493c-a35e-e6521a91a7d0
📒 Files selected for processing: 3
content/formatters/html.jspackages/decant-core/ai/gemini.jspackages/decant-core/tests/gemini.test.js
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout: 4
- GitHub Check: Sourcery review
- GitHub Check: codeql / analyze
- GitHub Check: test-and-build
- GitHub Check: Gitar
🔇 Additional comments: 3
packages/decant-core/ai/gemini.js: 1
529-529: LGTM!Also applies to: 540-540, 545-545, 575-576
packages/decant-core/tests/gemini.test.js: 1
385-392: LGTM!Also applies to: 493-496
content/formatters/html.js: 1
1430-1431: LGTM!
|
|
||
| // Fallback: normalize XML-style <Image alt="..." caption="..." src="..."/> tags into markdown images | ||
| // (Runs after code blocks are protected so code samples are not modified) | ||
| mdText = mdText.replace(/<Image\s+([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, (match, attrsStr) => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match Image tags with no attributes.
For <Image/> or <Image></Image>, \s+ prevents the regex from matching. The raw tag then remains in the export instead of being removed when it has no src. Allow zero attributes while keeping a tag-name boundary.
The supplied change details state that tags without an allowed src are removed.
Proposed fix
--- "a/content/formatters/html.js"
+++ "b/content/formatters/html.js"
@@ -1421,7 +1421,7 @@
// Fallback: normalize XML-style <Image alt="..." caption="..." src="..."/> tags into markdown images
// (Runs after code blocks are protected so code samples are not modified)
- mdText = mdText.replace(/<Image\s+([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, (match, attrsStr) => {
+ mdText = mdText.replace(/<Image(?=\s|\/?>)\s*([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, (match, attrsStr) => {
const altMatch = attrsStr.match(/alt="([^"]*)"/i) || attrsStr.match(/alt='([^']*)'/i);
const captionMatch =
attrsStr.match(/caption="([^"]*)"/i) || attrsStr.match(/caption='([^']*)'/i);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mdText = mdText.replace(/<Image\s+([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, (match, attrsStr) => { | |
| mdText = mdText.replace(/<Image(?=\s|\/?>)\s*([^>]*?)(?:\/>|>(?:\s*<\/Image>)?)/gi, (match, attrsStr) => { |
| const srcMatch = attrsStr.match(/src="([^"]*)"/i) || attrsStr.match(/src='([^']*)'/i); | ||
| const src = srcMatch ? srcMatch[1] : ''; | ||
| const rawAlt = altMatch ? altMatch[1] : captionMatch ? captionMatch[1] : 'Image'; | ||
| const alt = rawAlt.replace(/[[\]]/g, '\\$&'); |
There was a problem hiding this comment.
🟥 P1 · Bracketed labels break exports
Escaping brackets here turns image labels into display-math delimiters. For example, <Image alt="Paired [V1]" src="https://example.com/image.png"/> becomes ![Paired \[V1\]](...). The subsequent math replacement inserts blank lines inside the image Markdown, so the HTML export contains raw Markdown and a math block instead of an image. PDF exports use the same formatter.
The new escaping in Gemini’s image and video resolvers also conflicts with math normalization. Through the complete Gemini API conversion path, the media survives but its image description or video title is corrupted instead.
Protect image and link labels from math normalization in both paths before merging, and add parser-to-export regression tests with bracket-containing labels.
Knowledge Base Used: Export formatting pipeline
Artifacts
Executed bracketed-label pipeline test source
- The authored harness executes real revision-specific parser and formatter modules and checks exported media labels against controls, making the regression reproducible.
Pipeline results before bracket escaping
- Ran the 15-check harness at 4df1a9d and captured formatter output plus exported DOM observations; all checks pass.
Pipeline results with bracket escaping at PR head
- Ran the identical harness at 502637d and captured missing media and corrupted labels alongside passing controls; six checks demonstrate the regression.
Existing formatter and Gemini test results at PR head
- Ran both existing scoped test files at head and captured 31 passing tests; the current suite does not catch the demonstrated pipeline regression.
🦖 Ran code and verified through TREX
Code Review
|
| Auto-apply | Compact |
|
|
Important
Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.
Was this helpful? React with 👍 / 👎 | Gitar
Closes #110
Summary of Changes
Gemini API Image & Media Placeholder Resolution:
packages/decant-core/ai/gemini.js, resolved<Image ... src="image_agent_tag_...">XML tags into standard Markdown image tagsusing image metadata from candidate slot 12 (cand[12][1]).http://googleusercontent.com/youtube_content/...placeholder links incand[12][4]to clickable Markdown video links.cand[2]) intoextractActivitySourcesso citations are preserved under**Sources consulted:**.Gemini DOM Link & Source Extraction:
parseFromDom, inspected citation chips (citation-chip,button[aria-label*="View source"]) and extracted link URLs and title text, converting them into Markdown links or bracketed citations.<a>links nested inside buttons to prevent link deletion..sources-listbefore removal and appended them under**Sources consulted:**.image-containerandpicturewrappers so images inside the Gemini markdown container are preserved by Turndown.HTML Formatter Defense:
content/formatters/html.js(markdownToHtml), added fallback normalization converting any unparsed self-closing<Image ... src="...">XML tags into standard Markdown images ifsrcis a valid URL, ensuring HTML and PDF exports render them as<img>.Fixture Secret Sanitization (Alert Output formatting #8):
packages/decant-core/tests/fixtures/google-search-ai-chat.htmlwith masked placeholder tokens.Verification
npm testpassing 100% across all 139 tests indecant-coreand extension test suites.npm run lintpassed (0 errors, 0 warnings).npm run format:checkpassed.npm run buildandnpm run build:firefoxsucceeded.