-
Notifications
You must be signed in to change notification settings - Fork 0
fix: render images referenced from the local filesystem #165
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
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
2038ab1
feat(shared): add portable annotation assets
blimmer d80053a
feat(server): serve embedded annotation assets
blimmer 6069173
feat(annotation): render payload image assets
blimmer 2460134
fix(cli): inline local markdown images as assets
blimmer 375cdf6
refactor(cli): model asset extraction rejections as a Result type
blimmer 69ddf24
Merge branch 'main' into fix/plan-image-inlining-148
blimmer 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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 |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { asset } from '@contextbridge/shared/testFactories'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { createRef } from 'react'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { AnnotatedMarkdown } from './AnnotatedMarkdown.tsx'; | ||
|
|
||
| describe('AnnotatedMarkdown image rendering', () => { | ||
| const fixtureAsset = asset.build({ | ||
| id: 'abc123', | ||
| originalPath: '/tmp/fixture.png', | ||
| }); | ||
|
|
||
| it('rewrites local-path img srcs to /assets/<id> when a matching asset is provided', () => { | ||
| render( | ||
| <AnnotatedMarkdown | ||
| content="" | ||
| assets={[fixtureAsset]} | ||
| containerRef={createRef<HTMLDivElement>()} | ||
| />, | ||
| ); | ||
| const img = screen.getByAltText('fixture'); | ||
| expect(img.getAttribute('src')).toBe('/assets/abc123'); | ||
| }); | ||
|
|
||
| it('leaves remote img srcs unchanged', () => { | ||
| render( | ||
| <AnnotatedMarkdown | ||
| content="" | ||
| assets={[fixtureAsset]} | ||
| containerRef={createRef<HTMLDivElement>()} | ||
| />, | ||
| ); | ||
| const img = screen.getByAltText('cat'); | ||
| expect(img.getAttribute('src')).toBe('https://example.com/cat.png'); | ||
| }); | ||
|
|
||
| it('leaves local-path img srcs unchanged when assets prop is omitted', () => { | ||
| render(<AnnotatedMarkdown content="" containerRef={createRef<HTMLDivElement>()} />); | ||
| const img = screen.getByAltText('diagram'); | ||
| expect(img.getAttribute('src')).toBe('/tmp/missing.png'); | ||
| }); | ||
| }); |
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
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 |
|---|---|---|
| @@ -0,0 +1,183 @@ | ||
| import { mkdtempSync, rmSync, truncateSync, writeFileSync } from 'node:fs'; | ||
| import { tmpdir } from 'node:os'; | ||
| import { join } from 'node:path'; | ||
| import { pathToFileURL } from 'node:url'; | ||
| import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; | ||
| import { type TestContext, createStubContext, readLogs } from '#src/testHelpers/index.ts'; | ||
| import { MAX_ASSET_BYTES, MAX_TOTAL_ASSET_BYTES, extractAssets } from './extractAssets.ts'; | ||
|
|
||
| describe('extractAssets', () => { | ||
| let stub: TestContext; | ||
| let tmp: string; | ||
|
|
||
| beforeEach(() => { | ||
| stub = createStubContext(); | ||
| tmp = mkdtempSync(join(tmpdir(), 'cb-extract-assets-')); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| rmSync(tmp, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| it('returns an asset for a single absolute-path image reference that exists on disk', async () => { | ||
| const imgPath = join(tmp, 'a.png'); | ||
| writeFileSync(imgPath, Buffer.from([0x89, 0x50, 0x4e, 0x47])); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `# plan\n\n\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(1); | ||
| expect(assets[0]).toMatchObject({ | ||
| originalPath: imgPath, | ||
| mimeType: 'image/png', | ||
| }); | ||
| expect(assets[0]?.id).toMatch(/^[0-9a-f]{12}$/); | ||
| expect(assets[0]?.dataBase64).toBe('iVBORw=='); | ||
| }); | ||
|
|
||
| it('returns an asset for a file URL image reference', async () => { | ||
| const imgPath = join(tmp, 'file-url.png'); | ||
| writeFileSync(imgPath, Buffer.from([0x03, 0x04])); | ||
| const imgUrl = pathToFileURL(imgPath).toString(); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(1); | ||
| expect(assets[0]).toMatchObject({ | ||
| originalPath: imgUrl, | ||
| mimeType: 'image/png', | ||
| dataBase64: 'AwQ=', | ||
| }); | ||
| }); | ||
|
|
||
| it('dedupes two references with the same as-written path string', async () => { | ||
| const imgPath = join(tmp, 'b.png'); | ||
| writeFileSync(imgPath, Buffer.from([0x01, 0x02])); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n\n\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('skips non-local URLs silently', async () => { | ||
| const assets = await extractAssets(stub.context, { | ||
| content: '\n\n', | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('skips paths whose extension is not in the allowlist (including SVG)', async () => { | ||
| const svg = join(tmp, 'c.svg'); | ||
| writeFileSync(svg, '<svg/>'); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('resolves relative paths against sourcePath when sourcePath is set', async () => { | ||
| writeFileSync(join(tmp, 'side.png'), Buffer.from([0xff])); | ||
| const sourcePath = join(tmp, 'plan.md'); | ||
| writeFileSync(sourcePath, ''); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: '\n', | ||
| sourcePath, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(1); | ||
| expect(assets[0]?.originalPath).toBe('./side.png'); | ||
| }); | ||
|
|
||
| it('resolves relative paths against process.cwd() when sourcePath is unset (stdin case)', async () => { | ||
| const originalCwd = process.cwd(); | ||
| writeFileSync(join(tmp, 'side.png'), Buffer.from([0xfe])); | ||
| process.chdir(tmp); | ||
| try { | ||
| const assets = await extractAssets(stub.context, { | ||
| content: '\n', | ||
| }); | ||
| expect(assets).toHaveLength(1); | ||
| expect(assets[0]?.originalPath).toBe('./side.png'); | ||
| } finally { | ||
| process.chdir(originalCwd); | ||
| } | ||
| }); | ||
|
|
||
| it('logs error and skips when a referenced image cannot be read', async () => { | ||
| const missing = join(tmp, 'missing.png'); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| const logs = readLogs(stub.logs); | ||
| expect(logs.some((log) => log.level === 50 && log.msg === 'failed to read referenced image')).toBe(true); | ||
| }); | ||
|
|
||
| it('logs warning and skips empty image files', async () => { | ||
| const empty = join(tmp, 'empty.png'); | ||
| writeFileSync(empty, Buffer.from([])); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| const logs = readLogs(stub.logs); | ||
| expect(logs.some((log) => log.level === 40 && log.msg === 'referenced image is empty')).toBe(true); | ||
| }); | ||
|
|
||
| it('logs warning and skips images over the per-asset size limit', async () => { | ||
| const oversized = join(tmp, 'oversized.png'); | ||
| writeFileSync(oversized, ''); | ||
| truncateSync(oversized, MAX_ASSET_BYTES + 1); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| const logs = readLogs(stub.logs); | ||
| expect(logs.some((log) => log.level === 40 && log.msg === 'referenced image exceeds per-asset size limit')).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
|
|
||
| it('logs warning and skips images that would exceed the total size limit', async () => { | ||
| const first = join(tmp, 'first.png'); | ||
| const second = join(tmp, 'second.png'); | ||
| const overflow = join(tmp, 'overflow.png'); | ||
| writeFileSync(first, ''); | ||
| writeFileSync(second, ''); | ||
| writeFileSync(overflow, ''); | ||
| truncateSync(first, MAX_ASSET_BYTES); | ||
| truncateSync(second, MAX_ASSET_BYTES); | ||
| truncateSync(overflow, MAX_TOTAL_ASSET_BYTES - 2 * MAX_ASSET_BYTES + 1); | ||
|
|
||
| const assets = await extractAssets(stub.context, { | ||
| content: `\n\n\n`, | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(2); | ||
| const logs = readLogs(stub.logs); | ||
| expect(logs.some((log) => log.level === 40 && log.msg === 'referenced images exceed total size limit')).toBe(true); | ||
| }); | ||
|
|
||
| it('does not extract images inside fenced code blocks', async () => { | ||
| const assets = await extractAssets(stub.context, { | ||
| content: '```\n\n```\n', | ||
| }); | ||
|
|
||
| expect(assets).toHaveLength(0); | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
Snags images defined in our assets metadata and serves them from the purpose-built CLI endpoint.