Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/chronicle/src/cli/commands/static-generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { loadApiSpec, resolveDocument, type ApiSpec } from '@/lib/openapi';
import { buildApiRoutes, getSpecSlug } from '@/lib/api-routes';
import { buildLlmsTxt, type LlmsPage } from '@/lib/llms';
import { DEFAULT_WIDTH, DEFAULT_QUALITY, isLocalImage, isSvg, splitVersion } from '@/lib/image-utils';
import { isAnimatedImage } from '@/lib/image-animation';
import { getAssetVersion } from '@/lib/asset-version';
import type { VersionContext } from '@/lib/version-source';
import type { Frontmatter, PageNavLink } from '@/types';
Expand Down Expand Up @@ -717,7 +718,11 @@ async function optimizeImages(
try {
await fs.mkdir(path.dirname(destPath), { recursive: true });
const source = await fs.readFile(srcPath);
const optimizedBuf = await sharp(source)
// Animated sources must keep every frame — the <picture> element in
// MDXImage always prefers this .webp, so a flattened one would render
// an animated GIF as a still image
const animated = await isAnimatedImage(srcPath);
const optimizedBuf = await sharp(source, { animated })
Comment on lines +721 to +725

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not overwrite the optimized WebP output.

When relativePath already ends in .webp, webpRelative at Line 715 equals relativePath. destPath and origDest then point to the same file. Line 729 writes the optimized buffer, but Line 734 copies the original source over it. This bypasses resizing and quality settings for animated WebP inputs.

Skip the fallback copy when origDest === destPath.

Proposed fix
         const origDest = path.join(outputDir, '_content', relativePath);
-        await fs.mkdir(path.dirname(origDest), { recursive: true });
-        await fs.copyFile(srcPath, origDest);
+        if (origDest !== destPath) {
+          await fs.mkdir(path.dirname(origDest), { recursive: true });
+          await fs.copyFile(srcPath, origDest);
+        }
🤖 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 `@packages/chronicle/src/cli/commands/static-generate.ts` around lines 721 -
725, Update the output handling around the optimized buffer write and fallback
copy so the original source is copied only when origDest and destPath differ.
Preserve the optimized WebP output when relativePath already ends in .webp,
while retaining the fallback copy behavior for distinct destinations.

.resize({ width: DEFAULT_WIDTH, withoutEnlargement: true })
.webp({ quality: DEFAULT_QUALITY })
.toBuffer();
Expand Down
49 changes: 49 additions & 0 deletions packages/chronicle/src/lib/image-animation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
import fs from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path';
import sharp from 'sharp';
import { isAnimatedImage } from './image-animation';

let dir: string;
let animatedGif: string;
let stillGif: string;
let stillPng: string;

async function solidPng(color: string): Promise<Buffer> {
return sharp({ create: { width: 8, height: 8, channels: 3, background: color } }).png().toBuffer();
}

beforeAll(async () => {
dir = await fs.mkdtemp(path.join(os.tmpdir(), 'chronicle-anim-'));
animatedGif = path.join(dir, 'tour.gif');
stillGif = path.join(dir, 'still.gif');
stillPng = path.join(dir, 'photo.png');

const frames = await Promise.all([solidPng('#f00'), solidPng('#00f'), solidPng('#0f0')]);
await fs.writeFile(animatedGif, await sharp(frames, { join: { animated: true } }).gif().toBuffer());
await fs.writeFile(stillGif, await sharp(frames[0]).gif().toBuffer());
await fs.writeFile(stillPng, frames[0]);
});

afterAll(async () => {
await fs.rm(dir, { recursive: true, force: true });
});

describe('isAnimatedImage', () => {
test('returns true for a multi-frame gif', async () => {
expect(await isAnimatedImage(animatedGif)).toBe(true);
});

test('returns false for a single-frame gif', async () => {
expect(await isAnimatedImage(stillGif)).toBe(false);
});

test('returns false for formats that cannot animate', async () => {
expect(await isAnimatedImage(stillPng)).toBe(false);
});

test('returns false for a missing file', async () => {
expect(await isAnimatedImage(path.join(dir, 'nope.gif'))).toBe(false);
});
});
20 changes: 20 additions & 0 deletions packages/chronicle/src/lib/image-animation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import sharp from 'sharp';
import { isAnimatable } from './image-utils';

// Server-only: keeps the `sharp` import out of image-utils.ts, which is shared
// with the client bundle.

/**
* True when the file holds more than one frame. sharp decodes only the first
* frame unless constructed with `{ animated: true }`, so callers need this to
* avoid flattening animated GIF/WebP sources.
*/
export async function isAnimatedImage(filePath: string): Promise<boolean> {
if (!isAnimatable(filePath)) return false;
try {
const { pages } = await sharp(filePath).metadata();
return (pages ?? 1) > 1;
} catch {
return false;
}
}
22 changes: 22 additions & 0 deletions packages/chronicle/src/lib/image-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test';
import {
isLocalImage,
isSvg,
isAnimatable,
buildOptimizedUrl,
webpUrl,
splitVersion,
Expand Down Expand Up @@ -38,6 +39,27 @@ describe('isSvg', () => {
});
});

describe('isAnimatable', () => {
test('returns true for .gif and .webp', () => {
expect(isAnimatable('/_content/tour.gif')).toBe(true);
expect(isAnimatable('/_content/tour.webp')).toBe(true);
});

test('ignores case and query strings', () => {
expect(isAnimatable('/_content/TOUR.GIF?v=abc123')).toBe(true);
});

test('returns false for still-only formats', () => {
expect(isAnimatable('/_content/photo.png')).toBe(false);
expect(isAnimatable('/_content/photo.jpg')).toBe(false);
expect(isAnimatable('/_content/logo.svg')).toBe(false);
});

test('works on filesystem paths', () => {
expect(isAnimatable('/abs/path/.content/docs/tour.gif')).toBe(true);
});
});

describe('buildOptimizedUrl', () => {
test('builds URL with width and default quality', () => {
const url = buildOptimizedUrl('/_content/img.png', 640);
Expand Down
7 changes: 7 additions & 0 deletions packages/chronicle/src/lib/image-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ export function isSvg(url: string): boolean {
return url.split('?')[0].endsWith('.svg');
}

// Only these formats can hold multiple frames — everything else skips the
// metadata probe in isAnimatedImage()
export function isAnimatable(url: string): boolean {
const base = url.split('?')[0].toLowerCase();
return base.endsWith('.gif') || base.endsWith('.webp');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this gate covers .gif and .webp only. Animated PNG (APNG) and animated AVIF also carry multiple pages, so an APNG dropped into content would still get flattened. Adding .png here would make every PNG pay the metadata probe, so leaving it out is a fair tradeoff. A one-line comment noting APNG is intentionally out of scope would stop the next person from treating it as a bug.

}

export function buildOptimizedUrl(url: string, width: number, quality = DEFAULT_QUALITY, version?: string): string {
const base = `/api/image?url=${encodeURIComponent(url)}&w=${width}&q=${quality}`;
return version ? `${base}&v=${version}` : base;
Expand Down
72 changes: 70 additions & 2 deletions packages/chronicle/src/server/api/image.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { describe, expect, test } from 'bun:test';
import { negotiateFormat, cacheKey, MIME } from './image';
import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
import fs from 'node:fs/promises';
import os from 'node:os';
import path from 'node:path';
import sharp from 'sharp';
import { negotiateFormat, cacheKey, MIME, optimizeImage } from './image';

describe('negotiateFormat', () => {
test('returns avif when Accept includes image/avif', () => {
Expand All @@ -21,6 +25,22 @@ describe('negotiateFormat', () => {
test('prefers avif over webp when both present', () => {
expect(negotiateFormat('image/webp,image/avif')).toBe('avif');
});

test('downgrades avif to webp for animated sources', () => {
expect(negotiateFormat('image/avif,image/webp,*/*', true)).toBe('webp');
});

test('returns webp for animated sources when only avif is advertised', () => {
expect(negotiateFormat('image/avif,*/*', true)).toBe('webp');
});

test('returns webp for animated sources when webp is advertised', () => {
expect(negotiateFormat('image/webp,image/png,*/*', true)).toBe('webp');
});

test('returns original for animated sources with neither format', () => {
expect(negotiateFormat('image/png,*/*', true)).toBe('original');
});
});

describe('cacheKey', () => {
Expand Down Expand Up @@ -66,13 +86,61 @@ describe('cacheKey', () => {
expect(a).toBe(b);
});

test('returns different keys for animated and still variants', () => {
const still = cacheKey('/_content/tour.gif', 1024, 75, 'webp', 'aaaa111111');
const animated = cacheKey('/_content/tour.gif', 1024, 75, 'webp', 'aaaa111111', true);
expect(animated).not.toBe(still);
});

test('leaves still-image keys unchanged when animated defaults to false', () => {
const implicit = cacheKey('/_content/img.png', 1024, 75, 'webp', 'aaaa111111');
const explicit = cacheKey('/_content/img.png', 1024, 75, 'webp', 'aaaa111111', false);
expect(explicit).toBe(implicit);
});

test('key ends with format extension', () => {
expect(cacheKey('/_content/img.png', 640, 75, 'webp')).toMatch(/\.webp$/);
expect(cacheKey('/_content/img.png', 640, 75, 'avif')).toMatch(/\.avif$/);
expect(cacheKey('/_content/img.png', 640, 75, 'original')).toMatch(/\.original$/);
});
});

describe('optimizeImage with animated sources', () => {
let dir: string;
let gif: string;

beforeAll(async () => {
dir = await fs.mkdtemp(path.join(os.tmpdir(), 'chronicle-image-'));
gif = path.join(dir, 'tour.gif');
const frames = await Promise.all(
['#f00', '#00f', '#0f0'].map(background =>
sharp({ create: { width: 8, height: 8, channels: 3, background } }).png().toBuffer(),
),
);
await fs.writeFile(gif, await sharp(frames, { join: { animated: true } }).gif().toBuffer());
});

afterAll(async () => {
await fs.rm(dir, { recursive: true, force: true });
});

async function pages(buf: Buffer): Promise<number> {
return (await sharp(buf).metadata()).pages ?? 1;
}

test('keeps every frame on webp output', async () => {
expect(await pages(await optimizeImage(gif, 320, 75, 'webp', true))).toBe(3);
});

test('keeps every frame on original-format output', async () => {
expect(await pages(await optimizeImage(gif, 320, 75, 'original', true))).toBe(3);
});

test('flattens to one frame when animated is not set', async () => {
expect(await pages(await optimizeImage(gif, 320, 75, 'webp'))).toBe(1);
});
});

describe('MIME', () => {
test('maps common image extensions', () => {
expect(MIME['.png']).toBe('image/png');
Expand Down
47 changes: 31 additions & 16 deletions packages/chronicle/src/server/api/image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,22 @@ import { safePath } from '@/server/utils/safe-path'
import { assetCacheControl, etagFor, isNotModified, REVALIDATE_CACHE } from '@/server/utils/asset-cache'
import { getAssetVersion } from '@/lib/asset-version'
import { ALLOWED_WIDTHS, ALLOWED_QUALITIES, DEFAULT_WIDTH, DEFAULT_QUALITY, isLocalImage, isSvg, splitVersion } from '@/lib/image-utils'
import { isAnimatedImage } from '@/lib/image-animation'

export const STORAGE_KEY = 'image-cache'

const inflight = new Map<string, Promise<Buffer>>()

export type OutputFormat = 'avif' | 'webp' | 'original'

export function negotiateFormat(accept: string | null): OutputFormat {
if (accept?.includes('image/avif')) return 'avif'
if (accept?.includes('image/webp')) return 'webp'
export function negotiateFormat(accept: string | null, animated = false): OutputFormat {
const wantsAvif = accept?.includes('image/avif') ?? false
const wantsWebp = accept?.includes('image/webp') ?? false
// sharp flattens animated sources to a single frame on AVIF output, so
// animated images fall back to WebP — every AVIF-capable browser decodes
// animated WebP
if (wantsAvif && !animated) return 'avif'
if (wantsWebp || (animated && wantsAvif)) return 'webp'
return 'original'
}

Expand All @@ -30,8 +36,11 @@ export const MIME: Record<string, string> = {
'.webp': 'image/webp',
}

export function cacheKey(url: string, w: number, q: number, format: OutputFormat, version?: string | number): string {
const hash = crypto.createHash('sha256').update(`${url}:${w}:${q}:${format}:${version ?? 0}`).digest('hex').slice(0, 16)
export function cacheKey(url: string, w: number, q: number, format: OutputFormat, version?: string | number, animated = false): string {
// The animated marker is only appended when set, so keys for still images
// stay stable across the animation fix and the on-disk cache survives
const suffix = animated ? ':animated' : ''
const hash = crypto.createHash('sha256').update(`${url}:${w}:${q}:${format}:${version ?? 0}${suffix}`).digest('hex').slice(0, 16)
return `${hash}.${format}`
}

Expand All @@ -48,9 +57,10 @@ export async function optimizeImage(
w: number,
q: number,
format: OutputFormat,
animated = false,
): Promise<Buffer> {
const source = await fs.readFile(filePath);
const pipeline = sharp(source).resize({ width: w, withoutEnlargement: true });
const pipeline = sharp(source, { animated }).resize({ width: w, withoutEnlargement: true });
if (format === 'avif') return pipeline.avif({ quality: q }).toBuffer();
if (format === 'webp') return pipeline.webp({ quality: q }).toBuffer();
return pipeline.toBuffer();
Expand Down Expand Up @@ -91,24 +101,28 @@ export default defineHandler(async event => {
throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' })
}

const stat = await fs.stat(filePath).catch(() => null)
if (!stat) {
throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' })
}

const animated = await isAnimatedImage(filePath)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: isAnimatedImage now runs on every request before the cache lookup. It is a header-only sharp().metadata() read, and isAnimatable short-circuits png/jpg without touching sharp, so the extra cost lands only on .gif/.webp. Fine at docs traffic, but note that cached hits for those two formats now pay one extra file open plus header parse each time. No change needed, just flagging the added work on the hot path.

const accept = event.headers.get('accept')
const format = negotiateFormat(accept)
const format = negotiateFormat(accept, animated)
const ext = path.extname(filePath).toLowerCase()
const originalMime = MIME[ext] ?? 'application/octet-stream'
const contentType = format === 'original' ? originalMime : `image/${format}`

const stat = await fs.stat(filePath).catch(() => null)
if (!stat) {
throw new HTTPError({ status: StatusCodes.NOT_FOUND, message: 'Not Found' })
}
const currentVersion = await getAssetVersion(filePath)
const key = cacheKey(url, w, q, format, currentVersion ?? stat.mtimeMs)
const key = cacheKey(url, w, q, format, currentVersion ?? stat.mtimeMs, animated)

const requestedVersion = event.url.searchParams.get('v')
const cacheControl = import.meta.dev
? REVALIDATE_CACHE
: assetCacheControl(requestedVersion, currentVersion)
const etag = etagFor(currentVersion ?? String(stat.mtimeMs), String(w), String(q), format)
// `animated` is part of the ETag so browsers holding a pre-fix single-frame
// response revalidate instead of getting a 304
const etag = etagFor(currentVersion ?? String(stat.mtimeMs), String(w), String(q), format, ...(animated ? ['animated'] : []))
Comment on lines 119 to +125

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

ast-grep outline packages/chronicle/src/server/utils/asset-cache.ts --items all
sed -n '1,100p' packages/chronicle/src/server/utils/asset-cache.ts

ast-grep outline packages/chronicle/src/lib/image-utils.ts --items all
rg -n -C 4 'buildOptimizedUrl|assetCacheControl|IMMUTABLE_CACHE' packages/chronicle/src

Repository: raystack/chronicle

Length of output: 21607


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '--- packages/chronicle/src/server/api/image.ts ---\n'
wc -l packages/chronicle/src/server/api/image.ts
sed -n '1,180p' packages/chronicle/src/server/api/image.ts

printf '\n--- getAssetVersion usages ---\n'
rg -n -C 3 'getAssetVersion|version' packages/chronicle/src/lib packages/chronicle/src/server | sed -n '1,220p'

printf '\n--- package/tooling version context ---\n'
sed -n '1,120p' package.json
fd -e toml -e json -e lock -e lockfile -e yarn.lock -e pnpm-lock.yaml . | sed -n '1,80p'

Repository: raystack/chronicle

Length of output: 247


🏁 Script executed:

#!/bin/bash
set -euo pipefail

cat <<'EOF'
--- packages/chronicle/src/server/api/image.ts ---
EOF
wc -l packages/chronicle/src/server/api/image.ts
sed -n '1,180p' packages/chronicle/src/server/api/image.ts

printf '%s\n' ''
printf '%s\n' '--- getAssetVersion usages ---'
rg -n -C 3 'getAssetVersion|version' packages/chronicle/src/lib packages/chronicle/src/server | sed -n '1,220p'

printf '%s\n' ''
printf '%s\n' '--- package/tooling version context ---'
sed -n '1,120p' package.json
fd -e toml -e json -e lock -e lockfile -e yarn.lock -e pnpm-lock.yaml . | sed -n '1,80p'

Repository: raystack/chronicle

Length of output: 25063


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- asset-version source/usages ---'
fd -a 'asset-version' packages/chronicle/src
sed -n '1,180p' packages/chronicle/src/lib/asset-version.ts

printf '%s\n' ''
printf '%s\n' '--- version/currentVersion search ---'
rg -n -C 4 'currentVersion|RECOMMENDED_IMAGE_PIPELINE_VERSION|IMAGE.*VERSION|pipeline|version' packages/chronicle/src packages/chronicle/package.json | sed -n '1,260p'

printf '%s\n' ''
printf '%s\n' '--- precise image handler cache paths ---'
sed -n '88,132p' packages/chronicle/src/server/api/image.ts

printf '%s\n' ''
printf '%s\n' '--- image URL call sites and version passing ---'
python3 - <<'PY'
from pathlib import Path
for p in Path('packages/chronicle/src').rglob('*'):
    if p.is_file() and p.name.endswith(('.ts', '.tsx')):
        text = p.read_text(errors='ignore')
        if 'buildOptimizedUrl' in text:
            for i,l in enumerate(text.splitlines(), 1):
                if 'buildOptimizedUrl' in l:
                    print(f'{p}:{i}:{l.strip()}')
PY

Repository: raystack/chronicle

Length of output: 27532


Invalidate image pipeline revisions in the URL.

buildOptimizedUrl currently omits ?v, so browsers can cache a pre-fix flattened image as public, max-age=31536000, immutable and never hit the handler to see the new ETag. Include an image-pipeline revision in generated /api/image URLs, and test a pre-fix cache hit does not reuse the old response.

🤖 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 `@packages/chronicle/src/server/api/image.ts` around lines 119 - 125, Update
buildOptimizedUrl to append the image-pipeline revision as the v query parameter
on generated /api/image URLs, using the existing revision source rather than
changing handler caching. Add or update coverage to verify a cached pre-fix
flattened image is bypassed when the revision changes and the new response is
returned.

const headers = {
'Content-Type': contentType,
'Cache-Control': cacheControl,
Expand All @@ -132,7 +146,7 @@ export default defineHandler(async event => {
}

const work = (async () => {
const optimized = await optimizeImage(filePath, w, q, format)
const optimized = await optimizeImage(filePath, w, q, format, animated)
await storage.setItemRaw(key, optimized)
return optimized
})()
Expand Down Expand Up @@ -183,12 +197,13 @@ export async function warmupImageCache() {
const stat = await fs.stat(filePath).catch(() => null);
if (!stat) continue;

const key = cacheKey(base, w, q, format, (await getAssetVersion(filePath)) ?? stat.mtimeMs);
const animated = await isAnimatedImage(filePath);
const key = cacheKey(base, w, q, format, (await getAssetVersion(filePath)) ?? stat.mtimeMs, animated);
const cached = await storage.getItemRaw(key);
if (cached) continue;

try {
const optimized = await optimizeImage(filePath, w, q, format);
const optimized = await optimizeImage(filePath, w, q, format, animated);
await storage.setItemRaw(key, optimized);
warmed++;
} catch { /* skip unprocessable */ }
Expand Down
Loading