Work on image inspector - #124
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The manifest inspection relies on discarding Manifest.Copy's return value and working only through in-place backing-array mutation, which is fragile and should assign the result back, alongside minor naming/error-message consistency fixes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors the CLI's image-inspection logic into a new pluggable pkg/inspector package and adds a new --inspect-html capability that uses the publication's guided-navigation documents (generated from the (X)HTML with textref locators) to attach image references (source location, description, and role) onto image links in the manifest, working toward issue #88. It replaces the single helpers.ImageInspector with a composable Inspector interface aggregated by a Run orchestrator that implements go-toolkit's ManifestTransformer. It also bumps go-toolkit and many transitive dependencies.
Changes:
- Introduce
pkg/inspectorwith anInspectorinterface and aRunaggregator, plusImage(size/hash) andImageUsage(HTML-derived references) inspectors; remove the oldhelpers/inspector.go. - Wire a new
--inspect-htmlflag in themanifestcommand that configures the guided-navigation service and runs the inspectors over the manifest. - Update
go.mod/go.sum(notablygo-toolkitto the298d30dpseudo-version) and transitive dependency versions.
File summaries
| File | Description |
|---|---|
| pkg/inspector/inspector.go | New Inspector interface and Run orchestrator implementing ManifestTransformer. |
| pkg/inspector/image_inspector.go | New Image inspector delegating bitmap analysis to analyzer.InspectImage. |
| pkg/inspector/image_usage_inspector.go | New ImageUsage inspector building image references from guided-navigation objects. |
| pkg/helpers/inspector.go | Removed; superseded by the pkg/inspector package. |
| internal/cli/manifest.go | Adds --inspect-html, sets up the guided-nav service, and runs inspectors via Manifest.Copy. |
| go.mod / go.sum | Dependency version bumps, including go-toolkit. |
Review details
- Files reviewed: 6/7 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| if len(inspectors) > 0 { | ||
| inspection := inspector.CreateInspection(inspectors) | ||
| publication.Manifest.Copy(inspection) |
| inspector := &inspector.Image{ | ||
| Algorithms: hashAlgorithms, | ||
| Filesystem: fetcher.ToFS(context.TODO(), pub.Fetcher), | ||
| Filesystem: fetcher.ToFS(context.TODO(), publication.Fetcher), | ||
| } | ||
| inspectors = append(inspectors, inspector) |
| for i := range n.inspectors { | ||
| newHREF, err := n.inspectors[i].InspectHREF(href) | ||
| if err != nil { | ||
| n.err = errors.Wrap(err, "failed inspecting href "+href.String()+" with inspector for "+n.inspectors[i].Name()) |
For #88