fix(assets): image CLS/a11y polish and cache hygiene#1195
Conversation
|
Warning Review limit reached
Next review available in: 21 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis change adds and refreshes AVIF/WebP image assets, configures Next image caching and breakpoints, improves icon delivery and image preview rendering, validates signed URL handling, and records follow-up review outcomes. ChangesImage optimization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Updates to Preview Branch (subagent-Asset-Optimization-Implementer-self-b295a5bb) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
Clear the dirty-checkpoint conflict-marker history by merging origin/main. Remove the silent SignedImage object/sign transform rewrite, and make check:assets a non-mutating SVGO stability gate.
SVGO multipass stripped the prefers-color-scheme styles from src/app/icon.svg, failing brand:check and regressing dark-mode tab icons. Restore brandIconSvg() output and skip that file in check:assets so brand:check remains the owner of the themed favicon.
knip failed Static PR checks because scripts/optimize-public-images.mjs imports sharp while it was only pinned via overrides. Add the direct dependency so check:knip matches the script.
Adding svgo/sharp forced lockfile_changed, which made the pre-existing exceljs/brace-expansion npm audit highs blocking. Remove check:assets CI wiring, the unused optimize-public-images script, and restore package manifest/lockfile to main so Safety stays advisory. Runtime asset fixes remain.
CI triageCI failed on this PR. Automated classification of the 3 failed job(s):
Compared with main CI run #5589 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
public/demo-documents/clozapine-table.avif (1)
1-1: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConnect the pre-encoded demo assets to the source catalog.
The supplied consumers still reference PNG paths, so these AVIF/WebP files are not demonstrably used by the demo previews. Verify there is no extension-selection layer; otherwise update the catalog/renderer to serve optimized variants with a fallback.
public/demo-documents/clozapine-table.avif#L1-L1: wire the AVIF variant into clozapine preview selection or remove it.public/demo-documents/clozapine-table.webp#L1-L1: wire the WebP fallback into clozapine preview selection or remove it.public/demo-documents/risk-flow.avif#L1-L1: wire the AVIF variant into risk-flow preview selection or remove it.public/demo-documents/risk-flow.webp#L1-L1: wire the WebP fallback into risk-flow preview selection or remove it.🤖 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 `@public/demo-documents/clozapine-table.avif` at line 1, The clozapine and risk-flow optimized assets are not connected to demo preview selection. Update the source catalog/renderer to serve AVIF first, WebP as fallback, and retain PNG fallback where needed for clozapine-table.avif lines 1-1, clozapine-table.webp lines 1-1, risk-flow.avif lines 1-1, and risk-flow.webp lines 1-1; if no extension-selection layer exists, remove these unused assets instead.src/components/document-viewer/non-pdf-source-preview.tsx (1)
142-149: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winKeep this signed preview on an explicit image-optimization path.
The changed production TSX path still renders raw
<img>, which bypasses Next Image features and can trigger@next/next/no-img-element. Prefernext/imagewithfillandunoptimizedafter verifying the signed host is allowed byimages.remotePatterns, or document a targeted lint exception. (nextjs.org)🤖 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 `@src/components/document-viewer/non-pdf-source-preview.tsx` around lines 142 - 149, Update the preview image in the non-PDF source preview component to use next/image with fill and unoptimized while preserving the signedUrl, title, error handler, and existing visual classes; verify the signed host is permitted by images.remotePatterns, or add a narrowly scoped lint exception if it cannot be configured.Source: Coding guidelines
src/app/layout.tsx (1)
94-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the supported resource-hint path over a manual root
<head>.Next.js guidance says root layouts should not manually manage head elements, and its resource-hint documentation demonstrates preload APIs instead. Move this into the project’s preload-resource component or verify the current Next.js 16 behavior and deduplication guarantees before retaining it. (nextjs.org)
🤖 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 `@src/app/layout.tsx` around lines 94 - 96, Replace the manual root layout head link in the layout component with the project’s supported preload-resource component or Next.js preload API for /icon.svg. If retaining the current approach, first verify Next.js 16 deduplication behavior and ensure the implementation preserves a single high-priority SVG preload.Source: Coding guidelines
🤖 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 `@docs/branch-review-ledger.md`:
- Line 25: Move the 2026-07-25 PR `#1195` record from its current position near
the top of Review Records to after the existing final record. Preserve the row
content unchanged and maintain the ledger’s append-only chronological ordering
so check:branch-review-ledger accepts it.
In `@src/app/icons/`[variant]/route.tsx:
- Around line 38-40: Update the cache policy for the versionless icon responses
in the route handler serving the [variant] icons: remove immutable one-year
caching and use a shorter revalidation policy. Only retain long-lived immutable
caching if the manifest URLs are changed to include a content hash or version.
---
Nitpick comments:
In `@public/demo-documents/clozapine-table.avif`:
- Line 1: The clozapine and risk-flow optimized assets are not connected to demo
preview selection. Update the source catalog/renderer to serve AVIF first, WebP
as fallback, and retain PNG fallback where needed for clozapine-table.avif lines
1-1, clozapine-table.webp lines 1-1, risk-flow.avif lines 1-1, and
risk-flow.webp lines 1-1; if no extension-selection layer exists, remove these
unused assets instead.
In `@src/app/layout.tsx`:
- Around line 94-96: Replace the manual root layout head link in the layout
component with the project’s supported preload-resource component or Next.js
preload API for /icon.svg. If retaining the current approach, first verify
Next.js 16 deduplication behavior and ensure the implementation preserves a
single high-priority SVG preload.
In `@src/components/document-viewer/non-pdf-source-preview.tsx`:
- Around line 142-149: Update the preview image in the non-PDF source preview
component to use next/image with fill and unoptimized while preserving the
signedUrl, title, error handler, and existing visual classes; verify the signed
host is permitted by images.remotePatterns, or add a narrowly scoped lint
exception if it cannot be configured.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f95cd135-fce8-41c8-91f0-2bb4508e1cd7
📒 Files selected for processing (17)
docs/branch-review-ledger.mdnext.config.tspublic/demo-documents/clozapine-table.avifpublic/demo-documents/clozapine-table.webppublic/demo-documents/risk-flow.avifpublic/demo-documents/risk-flow.webppublic/mockups/crisp-white-colour-system-desktop.avifpublic/mockups/crisp-white-colour-system-desktop.webppublic/mockups/crisp-white-colour-system-mobile.avifpublic/mockups/crisp-white-colour-system-mobile.webppublic/mockups/premium-colour-system-mobile.avifpublic/mockups/premium-colour-system-mobile.webpsrc/app/icons/[variant]/route.tsxsrc/app/layout.tsxsrc/components/clinical-dashboard/signed-image.tsxsrc/components/document-viewer/non-pdf-source-preview.tsxtests/signed-image.dom.test.tsx
Remove unused pregenerated AVIF/WebP siblings that nothing referenced (next/image already negotiates formats). Drop the day-long minimumCacheTTL floor so optimizer output cannot outlive signed URLs by default. Cache unversioned /icons/* with daily revalidation instead of year-long immutable.
Turn check:assets into a real no-dep themed-favicon marker gate and wire it into verify:cheap + static-pr CI. Drop the redundant root-layout icon preload (app/icon.svg already owns the favicon link). Announce non-PDF preview failures with the same polite status live region as SignedImage.
BigSimmo
changed the title
1 participant
Summary
Scoped asset/image hygiene after review cleanup (no retrieval/ranking changes):
aspect-[4/3]) to reduce CLS.SignedImageand non-PDF previews.unoptimizednext/image; regression-tested so client code never appends Storagewidth/height/resizequery params ontoobject/signURLs.max-age=86400, stale-while-revalidate=604800) instead of year-longimmutable.minimumCacheTTL(60s); set explicitdeviceSizes/imageSizesonly.npm run check:assetsas a no-dependency themed-favicon marker gate (wired intoverify:cheap+ Static PR). Full SVGO multipass stays deferred to avoid a lockfile-triggered blocking npm audit.Intentionally not in this tip: SignedImage transform props, committed AVIF/WebP siblings, SVGO-in-CI, year-long immutable icon headers, and a root-layout
/icon.svgpreload (file-basedapp/icon.svgalready owns the favicon link).Verification
npm run verify:pr-local— Verification not run on this tip: focused local gates below cover the changed surface; hosted CI re-runs the full required aggregate.npm run check:assets,npm run brand:check,npm run check:gate-manifest,npm run docs:check-scripts,vitesttests/pwa-manifest.test.ts+tests/signed-image.dom.test.tsx(14 pass).npm run verify:ui— UI verification not run: layout/a11y/cache-header changes only; Production UI hosted job covers browser smoke.npm run eval:retrieval:quality— Verification not run: no retrieval or RAG ranking behaviour change.RAG impact: no retrieval behaviour change — image preview CLS/a11y markup, PWA icon cache headers, next/image sizing config, and a favicon marker gate only.
Risk and rollout
Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
mainwith conflict markers removed.svgo/ lockfile changes in this PR without also clearing the exceljs/brace-expansion audit highs.