Skip to content

Use Alchemistic logo as favicon; fix and finish icon-generation pipeline - #42

Merged
aquarion merged 4 commits into
mainfrom
claude/alchemistic-favicon-t0pwbb
Jul 30, 2026
Merged

Use Alchemistic logo as favicon; fix and finish icon-generation pipeline#42
aquarion merged 4 commits into
mainfrom
claude/alchemistic-favicon-t0pwbb

Conversation

@aquarion

Copy link
Copy Markdown
Member

Summary

The Alchemistic logo (yellow rounded square, black diagonal-stripe glyph) is now used for the site's favicon and Apple touch icon, generated from resources/branding/alchemistic.svg via a Vite-time icon-generation pipeline. The Istic logo in the sidebar (resources/views/components/app-logo-icon.blade.php) is untouched.

The icon-generation pipeline itself (bin/icons/*) was added in a prior commit on this branch but had several bugs preventing it from actually working; this PR fixes those and wires it up:

  • Added the missing vitest devDependency and npm test script (test files imported from vitest but it was never installed, and there was no way to run them).
  • Fixed generateFlatIcon (used by apps without a bloom.icon Apple Icon Composer bundle, like this one) — it was outputting a blank solid-color square instead of the brand glyph.
  • Fixed glyph scaling for that fallback: it now renders at the source SVG's native 1200 viewBox and lets sharp downscale, instead of embedding the same path coordinates inside a mismatched smaller viewBox (which cropped and off-centered the glyph).
  • Reconciled generate-web-icons.js's output filenames with its own test (logo-standard.*/logo-on-white.png) and updated docs/branding.md to match, plus corrected a couple of other doc inaccuracies (glyph color, regeneration instructions referencing a script that doesn't exist).
  • Rewrote generate-apple-touch-icon.test.js, which previously mutated a real resources/branding/bloom.icon/icon.json file that doesn't exist in this repo (so it always failed); it now covers the flat-fallback path this app actually uses.
  • Added vitest.config.js so npm test doesn't pick up vite.config.js's plugins and silently regenerate real icon files as a side effect of running tests.
  • Deleted orphaned duplicate icon files (application-*, bloom-*) left over from earlier renames — some had been committed with the dev-only debug background color (#CC0000) instead of the real brand color.
  • Gitignored resources/icons/* since it's fully regenerated per APP_ENV by the Vite plugin on every dev/build run — keeping it out of git is what prevents stale/wrong-environment-colored files from being committed again.
  • Wired resources/views/partials/head.blade.php to the Vite-managed icons via Vite::asset(), and removed the now-superseded static files in public/.

Test plan

  • npm test — all 17 tests pass
  • npm run build — icon-generation plugin runs successfully (build itself fails later on an unrelated missing vendor/livewire/flux/dist/flux.css, because Composer dependencies aren't installed in this sandbox — pre-existing environment limitation, unrelated to this change)
  • Visually inspected generated favicon.svg, favicon-96x96.png, apple-touch-icon.png, and logo-on-white.png — glyph renders centered and correctly proportioned
  • Verify in a real Sail environment that the favicon/apple-touch-icon show up correctly in a browser tab and on iOS home screen

🤖 Generated with Claude Code

https://claude.ai/code/session_01StCfH1mTviG3UqQoL1bUW5


Generated by Claude Code

aquarion and others added 2 commits July 30, 2026 20:46
- Add missing vitest devDependency and npm test script
- Fix generateFlatIcon to render the brand glyph on the background
  color instead of a blank square (apps without a bloom.icon
  Apple Icon Composer bundle, like this one, use this path)
- Fix glyph scaling: render at the source's native 1200 viewBox and
  let sharp downscale, instead of reusing glyph path coordinates
  inside a mismatched viewBox (was cropping/offsetting the glyph)
- Reconcile generate-web-icons.js output filenames with its test
  (logo-standard/logo-on-white) and update docs/branding.md to match
- Rewrite generate-apple-touch-icon.test.js to cover the flat-fallback
  path this app actually exercises, instead of mutating a real
  bloom.icon/icon.json file that doesn't exist in this repo
- Add vitest.config.js so `npm test` doesn't pick up vite.config.js's
  plugins and silently regenerate real icon files as a side effect
- Delete orphaned duplicate icon files (application-*, bloom-*) left
  over from earlier renames, including copies baked with the dev
  debug color (#CC0000) that had been committed by mistake
- Gitignore resources/icons/* since it's fully regenerated per
  APP_ENV by the Vite plugin on every dev/build run
- Wire resources/views/partials/head.blade.php to the Vite-managed
  icons via Vite::asset() and remove the now-superseded static files
  in public/ (sidebar logo in app-logo-icon.blade.php is untouched)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StCfH1mTviG3UqQoL1bUW5
@aquarion

Copy link
Copy Markdown
Member Author

PR Review Summary (multi-agent: code, tests, silent-failures, comments)

Critical Issues (1)

  • bin/icons/generate-apple-touch-icon.js:65-78, 191-199glyphLayer() silently returns null if a referenced glyph asset is missing/unreadable, or its SVG doesn't match the naive <path d="..."> regex. The caller (generateFromIconFile) just skips it — no throw, no warning. A bad icon.json reference could produce a broken icon (background + effects, no logo) that ships with a clean exit code, regenerated on every build via vite-plugin.js. This path has zero test coverage today (no bloom.icon bundle in-repo yet). Recommendation: throw instead of returning null in both branches — a failed build beats a silently glyph-less icon in production.

Important Issues (3)

  • docker/cloudflared/entrypoint.sh:31 — health check curls http://application:${APP_PORT}, but APP_PORT is the host-side forward port; inside the container network the app only listens on 80 (as config.yml:6 correctly hardcodes). Works today only because .env happens to set APP_PORT=80. If a dev changes APP_PORT to avoid a host port clash, the tunnel container will falsely report "application did not become ready" after exhausting all 60 retries. Fix: hardcode 80 for the internal health check like config.yml does.
  • bin/icons/generate-apple-touch-icon.js — the entire Icon Composer rendering path (generateFromIconFile, backgroundLayer, glyphLayer, edgeGlowLayer, cornerSpecularLayer) has zero test coverage, since no bloom.icon bundle exists in this repo yet. Worth a minimal test fixture to exercise this code.
  • bin/icons/generate-apple-touch-icon.js:74-78 — comment says translucency=0.5 yields "~0.70 opacity," but the formula 0.4 + translucency * 0.55 actually gives 0.675 — a real ~3.7% drift, invisible because of the untested-code gap above.

Suggestions

  • bin/icons/vite-plugin.js has no test file — the APP_ENV ?? config.mode precedence and syncJson: !isServe behavior are untested.
  • resources/branding/alchemistic.svg:3 — the new <g transform="translate(600 600) scale(1.0588235294) translate(-600 -600)"> wrapper (added to reduce icon padding) has no comment explaining the magic scale factor (18/17, targeting ~5% margin on the tall axis).
  • New vitest suite (bin/icons/*.test.js) isn't run in CI — .github/workflows/tests.yml only runs Pest.
  • docker/development/entrypoint.sh:39-49 unconditionally runs config:cache/route:cache/view:cache on every dev-container start — a dev footgun if .env edits are expected to take effect without a rebuild.
  • Minor: docker/development/entrypoint.sh:8 id -u www-data failure isn't guarded under set -e; .env sourcing is fragile against unquoted special characters (pre-existing pattern); supervisord.pid is untracked and probably belongs in .gitignore.

Strengths

  • Icon-pipeline JS tests are genuinely behavioral (real sharp output, real ICO byte layout, real pixel colors), not mocked.
  • Error-handling elsewhere in the icon pipeline consistently favors throwing over silent defaults — generate-apple-touch-icon.js's glyphLayer is the one outlier.
  • ngrok→Cloudflare migration is clean: no orphaned scripts, .gitignore updated coherently, no secrets committed.
  • Prior review feedback on this branch (usermod error handling, retry-timeout handling, entrypoint logging gaps) has all been addressed.
  • Blade head correctly uses config('app.name', …) rather than env().

🤖 Generated with multi-agent review (code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer)

- glyphLayer() now throws instead of silently returning null when a
  bloom.icon Apple Icon Composer asset is missing or doesn't match
  the expected <path d="..."> shape, so a bad icon.json reference
  fails the build instead of silently shipping a logo-less icon
- Fix a comment overstating translucency=0.5's resulting opacity
  (0.4 + 0.55*0.5 = 0.675, not ~0.70)
- Add `npm test` to ci.yml so the vitest suite in bin/icons/ actually
  runs in CI instead of only being runnable locally

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01StCfH1mTviG3UqQoL1bUW5
@aquarion

Copy link
Copy Markdown
Member Author

Thanks — pushed fixes for the parts of this in scope for this PR (f70921e):

  • Critical: glyphLayer() silent null — fixed. Both branches (missing asset, unmatched <path d="..."> regex) now throw instead of silently dropping the layer, so a bad icon.json reference fails the build loudly rather than shipping a logo-less Apple touch icon. Also dropped the now-dead if (composite) guard in the caller.
  • Translucency comment drift (0.675 vs claimed ~0.70) — fixed.
  • Vitest suite not run in CI — fixed, added npm test to ci.yml's test job.

Left for later, not urgent:

  • Icon Composer path (generateFromIconFile and friends) has no test coverage — agreed, but this repo has no bloom.icon bundle at all yet, so there's nothing real to build a fixture against without inventing one. Worth doing once an app actually uses that path; will track separately rather than block this PR on it.
  • vite-plugin.js untested — noted, same "not urgent" bucket.

Not acting on, out of scope for this PR:

  • docker/cloudflared/entrypoint.sh and docker/development/entrypoint.sh items — neither file is touched by this PR's diff (they came in via the merge from Fix dev container HTTP server and replace ngrok with Cloudflare named tunnels #40, already merged to main). Happy to open a follow-up if these still stand, but won't fix files this PR doesn't own.
  • resources/branding/alchemistic.svg:3 — the review describes a <g transform="translate(600 600) scale(1.0588235294)..."> wrapper that doesn't exist in the current file (it's just two <path> elements, no <g>). Looks like a mismatch on the reviewer's end; nothing to change here.

Generated by Claude Code

@aquarion
aquarion marked this pull request as ready for review July 30, 2026 23:37
@aquarion
aquarion merged commit f433407 into main Jul 30, 2026
7 checks passed
@aquarion
aquarion deleted the claude/alchemistic-favicon-t0pwbb branch July 30, 2026 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants