fix(parser): rendered images are missing alt, width, height and loading attributes (#97)

[harrydayexe]

Closes #97

Note

This pull request was written by rain running Claude Code. It has not been reviewed by a human. CI and an automated review pass run before it is handed over.

Issue

#97: fix(parser): rendered images are missing alt, width, height and loading attributes

What the agent reports

Done — branch rain/issue-97 is pushed with four commits.

What changed

pkg/parser (the fix)

• images.go: imageTransformer now sets loading="lazy" and decoding="async" on every image, and width/height on images it rewrote to an assets URL and could measure. assetURL became resolveAsset, which returns both the served URL and the matching path inside the assets FS (/images/screenshots/a.png → screenshots/a.png), derived from the same normalisation, so the existing .. containment guarantee still governs what can be read.
• replaceImageEmbeds (option 1 from the issue): wikilink image embeds are swapped for *ast.Image nodes during AST transformation, so both syntaxes share one rendering path. alt="" for a bare ![[foo.png]] falls out of the wikilink renderer's own "label == target means no alt" rule. wikilinkResolver lost its now-dead image branch and its unused blogRoot.
• dimensions.go (new): imageMeasurer reads headers via image.DecodeConfig (png/jpeg/gif registered), caches per path including failures, and is mutex-guarded since Parser is documented concurrency-safe. Every failure path is a debug log and no dimensions — never a parse error.
• New parser.WithAssetsDir option + Config.AssetsDir, matching how WithBlogRoot/WithLogger are already wired into parser.Config; Parser embeds config.AssetsDir.

Plumbing: Generator embeds config.AssetsDir, applies WithAssetsDirFunc, and forwards it to the parser config in Generate. pkg/server.New forwards its resolved AssetsDir into the generator options when set. The CLI needed no change — generate already passed config.WithAssetsDir(...).AsGeneratorOption() and serve passes it as a server option.

Docs: README images section (full rendered tag, dimensions sourced from the file, unmeasurable files omit them, bare-embed alt caveat deleted), --assets-dir flag row, pkg/parser/doc.go, pkg/config/doc.go, pkg/config/baseOption.go, pkg/server/doc.go. The .prose img rule in head.tmpl is untouched.

Tests: parser table tests over both syntaxes with real encoded PNG/JPEG headers (measurable, bare embed, labelled embed, subdirectory, missing, corrupt, SVG, absolute/protocol-relative/data//-rooted, .., no assets FS), a cache-hit-count test, a concurrency test, plus plumbing tests in pkg/generator, pkg/server, internal/generator (writes real output HTML) and internal/server.

Verified with

gofmt -l ., go vet ./..., go test ./..., go test -race ./..., go mod tidy (no dependency change), and a real run: go run ./cmd/goblog generate docs/example-posts /tmp/out97 renders <img src="/images/github.png" alt="The GoBlog README on GitHub" width="943" height="581" loading="lazy" decoding="async" />.

For the reviewer

1. Two deliberate decisions worth confirming. I did not add golang.org/x/image/webp — the issue offered that choice, so WebP joins SVG and AVIF as undimensioned (documented in README and godoc). And because wikilink embeds are now real ast.Image nodes, they render in the XHTML self-closing form … /> rather than the …> shown in the issue's expected-output snippet; the attribute set matches the spec exactly, only the closing token differs from the pasted-from-current-behaviour example.
2. replaceImageEmbeds node surgery in pkg/parser/images.go — I collect the embeds in a full walk, then replace, and copy the label as a fresh ast.NewTextSegment rather than re-parenting the original child, to avoid touching sibling pointers mid-walk.
3. Integration tests did not run — Docker is not available in this environment. integration/container_test.go's TestServe_Images was extended (real 640×480 PNG, asserts both <img> tags including alt="" for the bare embed); it compiles and vets, but has not executed. The generate half of that request is covered by TestRunGenerate_ImageDimensions in internal/generator instead, since the container's ENTRYPOINT is goblog serve and generate isn't reachable through it.
4. Cache lifetime is per-Parser, so with serve a resized image needs a reload for width/height to catch up (each Generate builds a fresh parser). Noted in pkg/server/doc.go and the README.

Changelog (#100)

✨ New Features

• (parser) emit alt, width, height and loading on rendered images

🐛 Bug Fixes

• (parser) keep Parser.AsOption promoted from the embedded logger

📚 Documentation

• document image attributes and the parser's assets directory

🧪 Tests

• cover image dimensions across parser, generator, server and CLI
• (parser) cover concurrent use of the measurement cache

🧹 Chores

• merge main into rain/issue-97

[harrydayexe]

Automated review

I read the full diff, the surrounding parser/server/generator code, the goldmark and wikilink library sources it depends on, and ran go vet, the full suite, and -race on the touched packages. The core feature is correct: resolveAsset keeps the .. containment guarantee and derives the FS path from the same normalisation as the URL (not by trimming the URL); the embed→*ast.Image rewrite is faithful to the old wikilink renderer's alt rule, so alt="" falls out naturally; measurement degrades silently in every failure mode; the cache is mutex-guarded and is rebuilt on every Generate, so serve --watch re-measures as the docs claim; and routing embeds through goldmark's image renderer actually tightens javascript:-style destinations via IsDangerousURL. Tests cover the interesting failure cases, not just the happy path.

One problem:

1. pkg/parser/parser.go:35 — adding goblogconfig.AssetsDir to the Parser struct silently breaks Parser's promoted AsOption() method. config.Logger and config.AssetsDir both declare AsOption() BaseOption (pkg/config/baseOption.go:65 and :138), so with both embedded at depth 1 the selector is ambiguous and the method is no longer promoted. Before this PR Parser embedded only goblogconfig.Logger, so p.AsOption() compiled; I verified the break by building an external module against this worktree:

   ./main.go:11:16: ambiguous selector p.AsOption
   

   This is a source-breaking change to an exported API of a v2 library, made incidentally rather than as part of issue fix(parser): rendered images are missing alt, width, height and loading attributes #97, and it also leaks Parser.FS and Parser.Enabled() into the public surface. (Generator and Server are unaffected — they already embed several AsOption()-bearing types, so theirs was already ambiguous.)

   The embed buys nothing here: p.AssetsDir.FS is written at pkg/parser/parser.go:73 and read once at :101, and the value is already carried by the new Config.AssetsDir field. parser does not use the config.BaseOption application chain that CLAUDE.md's config pattern describes — it has its own parser.Option func(*Config) and stores BlogRoot as a plain Config field — so dropping the embed is consistent with the rest of the package.

   Fix: delete goblogconfig.AssetsDir from the Parser struct (pkg/parser/parser.go:35) and the assignment at :73, and construct the measurer straight from the config:

   measurer: &imageMeasurer{fsys: config.AssetsDir, logger: p.Logger.Logger},

Agent response

Pushed to rain/issue-97 (commit fd64e83), PR #100 updated.

Review points

1. pkg/parser/parser.go:35 — embedding goblogconfig.AssetsDir breaks the promoted AsOption() — Fixed.

I confirmed the break independently before changing anything: built an external module against this worktree calling p.AsOption() and got ./main.go:11:16: ambiguous selector p.AsOption. After the fix, that same external build compiles.

The fix, as suggested:

• Removed goblogconfig.AssetsDir from the Parser struct.
• Removed the p.AssetsDir.FS = config.AssetsDir assignment.
• Build the measurer straight from the config: measurer: &imageMeasurer{fsys: config.AssetsDir, logger: p.Logger.Logger}.

Beyond the minimal change, I added a regression test (TestParser_AsOption in pkg/parser/parser_test.go) that calls p.AsOption() and asserts the returned BaseOption forwards the injected logger. The compile step alone catches the ambiguity — this failure mode is invisible to the package's own code, so a test that exercises the selector is what keeps a future embed from silently re-breaking it. Its comment says why.

No documentation needed updating: config.AssetsDir's godoc describes it as embedded in "server and outputter configuration structs" (still accurate), and the parser docs all reference parser.WithAssetsDir / Config.AssetsDir, neither of which changed. The exported surface is otherwise identical — parser.WithAssetsDir and Config.AssetsDir are untouched, so the issue #97 feature is unaffected.

Verification

• go build ./..., go vet ./..., gofmt -l . (clean), go mod tidy (no diff)
• go test ./... — all packages pass
• go test -race ./... — all packages pass
• External-consumer build calling p.AsOption() — now compiles (previously failed)

---

Posted by rain. Both halves were written by Claude Code; treat them as a starting point for human review, not a substitute for it.