feat: favicon setup from logo-icon.svg (Closes #471)#623
feat: favicon setup from logo-icon.svg (Closes #471)#623LaphoqueRC wants to merge 1 commit intoSolFoundry:mainfrom
Conversation
…471) - Copy logo-icon.svg as favicon.svg for modern browsers (SVG-first approach) - Add site.webmanifest with SVG icon reference - Add generation script (scripts/generate-favicons.sh) for PNG/ICO sizes: 16x16, 32x32, 180x180 (apple-touch), 192x192, 512x512, and multi-size .ico - Update index.html head with apple-touch-icon and manifest link tags - Zero binary files committed; PNGs generated on demand via script
📝 WalkthroughWalkthroughThe PR adds Progressive Web App (PWA) configuration and favicon generation infrastructure to the SolFoundry project. Changes include: (1) adding PWA-related Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issuesNo additional issues to link beyond those already associated with this PR. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment Tip Flake8 can be used to improve the quality of Python code reviews.Flake8 is a Python linter that wraps PyFlakes, pycodestyle and Ned Batchelder's McCabe script. To configure Flake8, add a '.flake8' or 'setup.cfg' file to your project root. See Flake8 Documentation for more details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/public/site.webmanifest`:
- Around line 5-11: The manifest currently lists only an SVG favicon in the
"icons" array, which breaks cross-browser/PWA install expectations; update the
site.webmanifest "icons" array to include raster PNG entries for 192x192 and
512x512 (e.g., add objects with "src": "/favicon-192.png", "sizes": "192x192",
"type": "image/png" and "src": "/favicon-512.png", "sizes": "512x512", "type":
"image/png"), optionally include "purpose": "any maskable" for one entry, and
ensure the referenced files are added to your static/assets so the build serves
them (look for the "icons" array in site.webmanifest to apply the changes).
In `@scripts/generate-favicons.sh`:
- Around line 61-88: The script writes a site.webmanifest that lists PNG icons
while also generating raster PNGs (favicon-16x16.png, favicon-32x32.png,
apple-touch-icon.png, android-chrome-192x192.png, android-chrome-512x512.png),
causing the generated manifest to diverge from the committed
frontend/public/site.webmanifest and breaking reproducibility; update
scripts/generate-favicons.sh so the cat > "$OUT/site.webmanifest" block only
references the SVG (favicon.svg) to match the committed manifest and remove or
disable the code that generates raster PNGs (the generation of
favicon-16x16.png, favicon-32x32.png, apple-touch-icon.png,
android-chrome-192x192.png, android-chrome-512x512.png), and then either update
frontend/index.html to reference the SVG (instead of apple-touch-icon.png) or
commit the raster assets to the repo so manifest, index.html, and repository
state remain consistent.
- Around line 51-55: The ICO creation is brittle because convert is given the
SVG directly; instead generate a 48x48 PNG from the verified SVG converter
(rsvg-convert or inkscape) and then invoke convert only with PNG inputs. Update
the block that currently calls convert with "\( "$SRC" -resize 48x48 \)" to
first run the existing SVG converter (use the same check that validated
rsvg-convert or inkscape) to produce "$OUT/favicon-48x48.png", then call convert
"$OUT/favicon-16x16.png" "$OUT/favicon-32x32.png" "$OUT/favicon-48x48.png"
"$OUT/favicon.ico"; also change the echo to reference 16, 32, 48 and ensure any
temporary file is cleaned if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13c2c11c-b2e7-4fdd-bd93-137f5f87210b
⛔ Files ignored due to path filters (1)
frontend/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
frontend/index.htmlfrontend/public/site.webmanifestscripts/generate-favicons.sh
| "icons": [ | ||
| { | ||
| "src": "/favicon.svg", | ||
| "sizes": "any", | ||
| "type": "image/svg+xml" | ||
| } | ||
| ], |
There was a problem hiding this comment.
Manifest icon set is incomplete for the stated favicon/PWA targets.
Line 5-11 defines only an SVG icon. The committed manifest is missing raster icon references (notably 192×192 and 512×512 PNG), so the repository state does not currently satisfy the stated cross-browser/install-icon expectations without an extra generation step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/site.webmanifest` around lines 5 - 11, The manifest currently
lists only an SVG favicon in the "icons" array, which breaks cross-browser/PWA
install expectations; update the site.webmanifest "icons" array to include
raster PNG entries for 192x192 and 512x512 (e.g., add objects with "src":
"/favicon-192.png", "sizes": "192x192", "type": "image/png" and "src":
"/favicon-512.png", "sizes": "512x512", "type": "image/png"), optionally include
"purpose": "any maskable" for one entry, and ensure the referenced files are
added to your static/assets so the build serves them (look for the "icons" array
in site.webmanifest to apply the changes).
| if command -v convert &>/dev/null; then | ||
| convert "$OUT/favicon-16x16.png" "$OUT/favicon-32x32.png" \ | ||
| \( "$SRC" -resize 48x48 \) \ | ||
| "$OUT/favicon.ico" | ||
| echo " ✓ favicon.ico (16, 32, 48)" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Verify ICO generation assumptions in the script:
# - only `convert` is checked
# - whether `magick` fallback is absent
# - SVG is fed directly into ICO conversion path
rg -n 'command -v convert|command -v magick|convert .*favicon\.ico|\$SRC" -resize 48x48' scripts/generate-favicons.shRepository: SolFoundry/solfoundry
Length of output: 143
🏁 Script executed:
cat -n scripts/generate-favicons.sh | head -70Repository: SolFoundry/solfoundry
Length of output: 2485
🌐 Web query:
ImageMagick convert SVG to ICO delegate support requirements
💡 Result:
To convert SVG → ICO with ImageMagick, you need two things:
- An SVG “decode” delegate (rasterizer) available at runtime
ImageMagick generally rasterizes SVG via an external delegate defined indelegates.xml, most commonly:
- librsvg via the
rsvg-convertcommand, or - Inkscape via the
inkscapeCLI (also configured viadelegates.xml).
If neither delegate is present/usable, SVG reading may fail or fall back to a limited renderer depending on your build. [2][3][4]
- ICO encoding support + PNG support (strongly recommended)
ICO creation is typically done by generating multiple raster sizes and writing a single.ico. ImageMagick supports building multi-resolution ICOs via-define icon:auto-resize=.... [1]
For modern Windows-style icons, the 256×256 icon image inside an ICO is commonly stored as PNG, so having PNG support in your ImageMagick build is important. [1]
How to verify quickly
- Check compiled features/delegates:
magick -version(look at the “Delegates” line). - Check format support:
magick -list format | findstr /i "SVG ICO PNG"(Windows) orgrep -E 'SVG|ICO|PNG'(Linux/macOS). - Check/override which SVG delegate is used: inspect
delegates.xmlentries forsvg:decode(Inkscape) and/orrsvg-convert. [2][3]
Typical working command
magick -background none input.svg -define icon:auto-resize=256,128,64,48,32,16 output.ico(requires an SVG delegate to rasterize first). [1][2]
Sources: ImageMagick Examples (ICO auto-resize) [1]; ImageMagick delegate configuration / SVG decode delegate examples (Inkscape) [2][3]; librsvg / rsvg-convert overview [4].
ICO generation path is brittle across ImageMagick environments.
Lines 51-55 pass the SVG directly to convert, but only verify that the convert command exists—not whether it has SVG delegate support (rsvg-convert or inkscape). In environments where ImageMagick is installed without an SVG delegate, the convert command fails and aborts the entire script (due to set -euo pipefail at line 9).
A more robust approach: generate the 48×48 PNG size using the already-verified SVG converter (rsvg-convert or inkscape), then create the ICO from PNG files only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-favicons.sh` around lines 51 - 55, The ICO creation is
brittle because convert is given the SVG directly; instead generate a 48x48 PNG
from the verified SVG converter (rsvg-convert or inkscape) and then invoke
convert only with PNG inputs. Update the block that currently calls convert with
"\( "$SRC" -resize 48x48 \)" to first run the existing SVG converter (use the
same check that validated rsvg-convert or inkscape) to produce
"$OUT/favicon-48x48.png", then call convert "$OUT/favicon-16x16.png"
"$OUT/favicon-32x32.png" "$OUT/favicon-48x48.png" "$OUT/favicon.ico"; also
change the echo to reference 16, 32, 48 and ensure any temporary file is cleaned
if needed.
| cat > "$OUT/site.webmanifest" << 'EOF' | ||
| { | ||
| "name": "SolFoundry", | ||
| "short_name": "SolFoundry", | ||
| "description": "Autonomous AI Software Factory on Solana", | ||
| "icons": [ | ||
| { | ||
| "src": "/android-chrome-192x192.png", | ||
| "sizes": "192x192", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/android-chrome-512x512.png", | ||
| "sizes": "512x512", | ||
| "type": "image/png" | ||
| }, | ||
| { | ||
| "src": "/favicon.svg", | ||
| "sizes": "any", | ||
| "type": "image/svg+xml" | ||
| } | ||
| ], | ||
| "theme_color": "#0a0a0a", | ||
| "background_color": "#0a0a0a", | ||
| "display": "standalone", | ||
| "start_url": "/" | ||
| } | ||
| EOF |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== Check where favicon generation is wired =="
rg -n 'generate-favicons\.sh|apple-touch-icon\.png|android-chrome-192x192\.png|android-chrome-512x512\.png|site\.webmanifest'
echo
echo "== Check whether generated raster favicon assets are present in repo =="
fd -HI '^(favicon-16x16\.png|favicon-32x32\.png|apple-touch-icon\.png|android-chrome-192x192\.png|android-chrome-512x512\.png|favicon\.ico)$'
echo
echo "== Current committed manifest =="
cat frontend/public/site.webmanifestRepository: SolFoundry/solfoundry
Length of output: 1214
Manifest and raster favicon generation breaks build reproducibility.
Lines 61–88 write site.webmanifest with PNG icon references (android-chrome-192x192.png, android-chrome-512x512.png), while the committed manifest in frontend/public/site.webmanifest contains only an SVG icon entry. The script also generates raster PNG assets on lines 43 (favicon-16x16.png, favicon-32x32.png, apple-touch-icon.png, android-chrome-192x192.png, android-chrome-512x512.png), none of which are committed to the repository.
As a result:
- Raster favicon assets are not available in the repo; they must be generated by manually executing the script.
- The generated manifest will differ from the committed manifest.
frontend/index.html(lines 9–10) references/apple-touch-icon.pngand the manifest file, but these won't match the expected PNG icons without running the generation step.- The build cannot produce the correct manifest and assets from repo state alone.
This violates reproducibility: runtime correctness depends on an external, manual generation step rather than being derivable from committed sources.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/generate-favicons.sh` around lines 61 - 88, The script writes a
site.webmanifest that lists PNG icons while also generating raster PNGs
(favicon-16x16.png, favicon-32x32.png, apple-touch-icon.png,
android-chrome-192x192.png, android-chrome-512x512.png), causing the generated
manifest to diverge from the committed frontend/public/site.webmanifest and
breaking reproducibility; update scripts/generate-favicons.sh so the cat >
"$OUT/site.webmanifest" block only references the SVG (favicon.svg) to match the
committed manifest and remove or disable the code that generates raster PNGs
(the generation of favicon-16x16.png, favicon-32x32.png, apple-touch-icon.png,
android-chrome-192x192.png, android-chrome-512x512.png), and then either update
frontend/index.html to reference the SVG (instead of apple-touch-icon.png) or
commit the raster assets to the repo so manifest, index.html, and repository
state remain consistent.
1 participant
Description
SVG-first favicon setup from
logo-icon.svgwith a generation script for PNG/ICO sizes. Zero binary files — all raster icons generated on demand.Closes #471
Payout Wallet
SOL:
HZV6YPdTeJPjPujWjzsFLLKja91K2Ze78XeY8MeFhfK8Type of Change
What Was Built
frontend/public/favicon.svgassets/logo-icon.svg— modern browsers use SVG directly<link rel="icon" type="image/svg+xml">frontend/public/site.webmanifest#0a0a0a)scripts/generate-favicons.shfavicon.ico(16, 32, 48)rsvg-convertorinkscape+imagemagicksite.webmanifestwith PNG icon references after generationfrontend/index.html<head>updates<link rel="apple-touch-icon">for iOS<link rel="manifest">for web manifestFavicon visible in:
Checklist