feat: render embedded asset files inline - #204
Merged
Merged
Conversation
Uploading or importing a file already copied it into assets/, but nothing rendered it. Every asset, images included, landed as a plain [name](assets/…) link, so an imported graph showed clickable text where you expected to see the picture.  is now a real inline token in outl-md, parsed once and consumed by every client, so there's no parallel tokenizer to keep in sync. Desktop and mobile render an inline <img>. A local asset loads its bytes through a new read_asset_data_url command as a size-capped data URL, a remote image loads straight from its URL. The TUI paints a text placeholder since a terminal can't show pixels. The importers now emit the embed form for images, and every non-image file stays a click-to-open chip. Fixes #203 Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds Markdown image tokens, image-aware asset imports, secure local asset data-URL loading, shared desktop/mobile rendering, file-chip fallbacks, and TUI placeholders. ChangesEmbedded asset rendering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MarkdownTokenizer
participant MarkdownInline
participant readAssetDataUrl
participant read_asset_data_url
participant WorkspaceAssets
MarkdownTokenizer->>MarkdownInline: provide Image token
MarkdownInline->>readAssetDataUrl: request local asset
readAssetDataUrl->>read_asset_data_url: invoke Tauri command
read_asset_data_url->>WorkspaceAssets: resolve and read bytes
WorkspaceAssets-->>read_asset_data_url: asset bytes
read_asset_data_url-->>MarkdownInline: data URL
MarkdownInline-->>MarkdownInline: render image or file chip
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements inline rendering for embedded assets using standard Markdown image syntax (), by adding a first-class image token in outl-md and rendering it consistently across TUI + desktop + mobile + importers (Fixes #203).
Changes:
- Add
InlineTok::Image/ ownedInlineToken::Image { alt, href }tooutl-md, with tests and documentation updates. - Render image tokens in GUI clients via shared
<MarkdownInline />(localassets/...via a newread_asset_data_urlcommand; remotehttp(s)directly), and render placeholders in the TUI. - Update asset import/attach paths and importer E2E tests so images emit the embed form (
) while non-images remain plain links.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/markdown-format.md | Documents  as an inline image/asset token and clarifies asset-link behavior. |
| crates/outl-tui/src/view/inline.rs | Adds TUI placeholder rendering for InlineTok::Image plus cursor-aligned raw rendering and tests. |
| crates/outl-tauri-shared/src/commands/asset.rs | Adds read_asset_data_url command to load local assets as size-capped data: URLs. |
| crates/outl-tauri-shared/CLAUDE.md | Updates crate docs to include the new asset inline-render command. |
| crates/outl-tauri-shared/Cargo.toml | Adds base64 dependency for data URL encoding. |
| crates/outl-mobile/src-tauri/src/lib.rs | Exposes read_asset_data_url command to the mobile webview. |
| crates/outl-mobile/src-tauri/src/commands/asset.rs | Adds the read_asset_data_url Tauri command wrapper for mobile. |
| crates/outl-md/tests/tokenize_owned.rs | Adds owned-token wire-form tests including the new image variant. |
| crates/outl-md/tests/inline.rs | Adds borrowed-token tokenizer tests for  parsing behavior. |
| crates/outl-md/src/inline.rs | Introduces InlineTok::Image + owned form support, parsing, and source roundtrip. |
| crates/outl-md/CLAUDE.md | Documents the new image token and matching order guarantees. |
| crates/outl-import/tests/obsidian_e2e.rs | Updates import expectations so imported images become . |
| crates/outl-import/tests/logseq_e2e.rs | Updates import expectations so imported images become . |
| crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx | Renders image tokens as inline <img> (local via data URL) or file chips for non-images. |
| crates/outl-frontend-shared/src/markdown/MarkdownInline.test.tsx | Adds unit tests for the new image/asset rendering behavior (mocking backend call). |
| crates/outl-frontend-shared/src/links/index.ts | Adds shared helpers for classifying image-vs-non-image and extracting labels. |
| crates/outl-frontend-shared/src/api/types.ts | Extends the InlineToken TS union with { kind: "image"; alt; href }. |
| crates/outl-frontend-shared/src/api/commands.ts | Adds the readAssetDataUrl command wrapper. |
| crates/outl-frontend-shared/CLAUDE.md | Documents the shared frontend behavior for  rendering. |
| crates/outl-desktop/src-tauri/src/lib.rs | Exposes read_asset_data_url command to the desktop webview. |
| crates/outl-desktop/src-tauri/src/commands/asset.rs | Adds the read_asset_data_url Tauri command wrapper for desktop. |
| crates/outl-actions/src/asset.rs | Changes imported image assets to emit  instead of [...](...); updates tests. |
| CHANGELOG.md | Adds release notes for embedded asset rendering and related behavior changes. |
| Cargo.lock | Locks the new base64 dependency for outl-tauri-shared. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/outl-frontend-shared/src/markdown/MarkdownInline.test.tsx`:
- Around line 11-16: Wrap the readAssetDataUrlMock creation in vi.hoisted() so
it is initialized before the hoisted vi.mock("../api/commands") factory
executes. Keep the mock’s existing resolved data URL behavior and preserve the
readAssetDataUrlMock reference for later mockClear() calls.
In `@crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx`:
- Around line 252-264: The remote-image rendering branch in MarkdownInline must
not load unreviewed HTTP(S) URLs automatically. Add the existing per-image or
per-page opt-in before rendering the <img> for non-asset links, while preserving
backend loading for isAssetLink paths; if remote loading is intentionally
retained as MVP behavior, document that privacy decision at the relevant
implementation point.
In `@crates/outl-md/src/inline.rs`:
- Around line 749-773: The try_image function currently uses find(']') to locate
the alt-text terminator without accounting for escaped closing brackets emitted
by escape_link_label. Replace the simple bracket search with a scan through the
alt text that identifies an unescaped closing bracket (one not preceded by a
backslash). Update the logic around bracket_close to skip escaped brackets until
finding a truly unescaped terminator, so syntax like
![pic\]final.png](assets/hash.png) parses correctly. Add a regression test case
for this escaped-bracket scenario.
In `@crates/outl-tauri-shared/src/commands/asset.rs`:
- Around line 117-128: Update the asset-reading flow around the metadata check
and std::fs::read to open one file handle and read at most MAX_DATA_URL_BYTES +
1 bytes, rejecting the result when the extra byte is present before Base64
encoding. Preserve the existing oversized-asset error behavior, and add a
regression test covering the bounded reader or a file that changes between size
observation and reading.
In `@crates/outl-tui/src/view/inline.rs`:
- Around line 214-224: Update the image-link rendering logic around
is_image_target to strip query and fragment suffixes before image classification
and filename fallback, while preserving the existing alt-text behavior. In
crates/outl-tui/src/view/inline.rs lines 520-550, add a regression test covering
empty alt text with a remote image URL containing a query string and assert the
expected image placeholder and fallback label.
- Around line 207-225: Extend the cursor link resolution used by decide_gx so
InlineTok::Image entries with asset targets are treated like links and passed to
open_external_url, including images where the alt text and URL are identical.
Preserve existing InlineTok::Link behavior and add a regression test covering gx
on a markdown image.
- Around line 520-550: Extend the image_pretty_shows_glyph_raw_keeps_source test
function to cover additional fallback paths. Add test cases using
render_pretty_block_text for an image reference with empty alt text (e.g.,
) and a remote image URL with a query string (e.g.,
). For each case, assert that
the image glyph appears in the pretty render and that the normalized filename
fallback is correctly handled, matching the pattern of the existing assertions
for .png and .pdf cases.
In `@docs/markdown-format.md`:
- Around line 285-287: Update docs/markdown-format.md lines 285-287 to state the
100 MiB [assets] max_bytes ingestion limit separately from the 25 MiB inline
data-URL rendering limit. Update CHANGELOG.md line 13 to use “25 MiB” instead of
“25 MB” for the inline limit.
- Around line 280-281: Update the image-extension documentation to include tif,
and modify is_image_target to remove query strings before checking the file
extension, matching isImagePath behavior so URLs such as photo.png?size=large
are classified as images consistently across renderers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad2eee74-e1bb-4976-a521-80ae70ecb8de
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
CHANGELOG.mdcrates/outl-actions/src/asset.rscrates/outl-desktop/src-tauri/src/commands/asset.rscrates/outl-desktop/src-tauri/src/lib.rscrates/outl-frontend-shared/CLAUDE.mdcrates/outl-frontend-shared/src/api/commands.tscrates/outl-frontend-shared/src/api/types.tscrates/outl-frontend-shared/src/links/index.tscrates/outl-frontend-shared/src/markdown/MarkdownInline.test.tsxcrates/outl-frontend-shared/src/markdown/MarkdownInline.tsxcrates/outl-import/tests/logseq_e2e.rscrates/outl-import/tests/obsidian_e2e.rscrates/outl-md/CLAUDE.mdcrates/outl-md/src/inline.rscrates/outl-md/tests/inline.rscrates/outl-md/tests/tokenize_owned.rscrates/outl-mobile/src-tauri/src/commands/asset.rscrates/outl-mobile/src-tauri/src/lib.rscrates/outl-tauri-shared/CLAUDE.mdcrates/outl-tauri-shared/Cargo.tomlcrates/outl-tauri-shared/src/commands/asset.rscrates/outl-tui/src/view/inline.rsdocs/markdown-format.md
Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/markdown-format.md (1)
217-217: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDocument only supported remote schemes.
The table says “a remote URL,” but remote image rendering accepts only
httpandhttps.
Change this to “a remote HTTP(S) URL” to match the renderer and Line 288.Based on coding guidelines:
isSafeHttpUrlmust allow onlyhttpandhttps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/markdown-format.md` at line 217, Update the image-link description in the markdown format table to specify “a remote HTTP(S) URL” instead of a generic remote URL, matching the renderer’s supported schemes and the wording near the existing asset-link guidance.Source: Coding guidelines
crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx (1)
298-334: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache
readAssetDataUrlresults to avoid repeated disk reads and re-encoding.Each
imagetoken creates its owncreateResource(() => tok.href, readAssetDataUrl). The same asset referenced twice on a page, or aMarkdownInlineremount (e.g., during list virtualization or navigation back), re-triggers a full IPC round-trip, disk read, and base64 re-encoding of up to 25 MiB on the backend. Add a small shared cache (e.g., a module-levelMap<string, Promise<string>>, with a cap/eviction so it doesn't grow unbounded for large workspaces) keyed bytok.href, and reuse the in-flight or resolved promise across resources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx` around lines 298 - 334, Update the image handling in MarkdownInline to reuse readAssetDataUrl results through a module-level, bounded cache keyed by tok.href. Store and return the same Promise for both in-flight and resolved requests so duplicate image tokens and MarkdownInline remounts avoid repeated IPC, disk reads, and encoding; evict older entries when the configured cap is reached.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/outl-frontend-shared/src/markdown/MarkdownInline.tsx`:
- Around line 298-334: Update the image handling in MarkdownInline to reuse
readAssetDataUrl results through a module-level, bounded cache keyed by
tok.href. Store and return the same Promise for both in-flight and resolved
requests so duplicate image tokens and MarkdownInline remounts avoid repeated
IPC, disk reads, and encoding; evict older entries when the configured cap is
reached.
In `@docs/markdown-format.md`:
- Line 217: Update the image-link description in the markdown format table to
specify “a remote HTTP(S) URL” instead of a generic remote URL, matching the
renderer’s supported schemes and the wording near the existing asset-link
guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef01b6bc-e03f-4b9e-8064-c30d2f4892f1
📒 Files selected for processing (9)
crates/outl-frontend-shared/CLAUDE.mdcrates/outl-frontend-shared/src/links/index.tscrates/outl-frontend-shared/src/markdown/MarkdownInline.test.tsxcrates/outl-frontend-shared/src/markdown/MarkdownInline.tsxcrates/outl-md/tests/inline.rscrates/outl-md/tests/tokenize_owned.rscrates/outl-tauri-shared/CLAUDE.mdcrates/outl-tauri-shared/src/commands/asset.rsdocs/markdown-format.md
Signed-off-by: Avelino <31996+avelino@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/outl-frontend-shared/src/links/index.test.ts`:
- Around line 3-5: Add boundary coverage for isAssetLink in the existing test
suite: import isAssetLink and assert it accepts assets/…, ./assets/…, and
/assets/… paths while rejecting scheme-based URLs, including https://…/assets/….
Ensure the tests enforce that any value containing :// is not classified as a
workspace asset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4cc01613-1930-4a70-9539-b94b62d4da16
📒 Files selected for processing (1)
crates/outl-frontend-shared/src/links/index.test.ts
4 tasks
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.
Uploading or importing a file already copied it into assets/, but nothing rendered it. Every asset, images included, landed as a plain
[name](assets/…)link, so an imported graph showed clickable text where you expected to see the picture.is now a real inline token in outl-md, parsed once and consumed by every client, so there's no parallel tokenizer to keep in sync. Desktop and mobile render an inlineFixes #203