feat: serve tight favicon variants for API server and docs site#1879
Merged
ZaynJarvis merged 2 commits intovolcengine:mainfrom May 7, 2026
Merged
feat: serve tight favicon variants for API server and docs site#1879ZaynJarvis merged 2 commits intovolcengine:mainfrom
ZaynJarvis merged 2 commits intovolcengine:mainfrom
Conversation
Browsers and MCP clients (claude.ai, Claude Desktop) auto-fetch /favicon.ico and /apple-touch-icon.png to display a server icon. The 1933 server previously returned JSON 404s for these paths, leaving connectors with a generic placeholder. Add small route handlers that serve OV-branded icons from the existing console/static directory (already shipped as package data). Also wire the console index.html to the new icons.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
The docs site previously pointed `<link rel="icon">` at the same 1000x1000 ov-logo.png used by the in-page nav, so the tab favicon rendered visibly small inside heavy whitespace. Reuse the trim+square favicon variants generated for the API server (favicon.ico, favicon-32.png, apple-touch-icon.png) under docs/images/ and wire them into the VitePress head config. The in-page nav logo still uses the original PNG (whitespace looks fine in that context).
t0saki
changed the title
There was a problem hiding this comment.
Pull request overview
Adds OV-branded favicon / touch-icon handling so browsers and MCP clients don’t see JSON 404s (and can display an icon) when probing common root icon paths.
Changes:
- Add root-level routes on the 1933 API server for
/favicon.ico,/favicon.png, and/apple-touch-icon.pngbacked by files inopenviking/console/static/. - Add cache headers (
Cache-Control: public, max-age=86400) for those icon responses. - Update the console
index.htmlto reference the packaged icon assets.
Reviewed changes
Copilot reviewed 3 out of 9 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| openviking/server/app.py | Registers three root icon routes returning FileResponse with caching headers. |
| openviking/console/static/index.html | Adds <link rel="icon"> and <link rel="apple-touch-icon"> tags referencing console icon assets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _favicon_headers = {"Cache-Control": "public, max-age=86400"} | ||
| _favicon_files = { | ||
| "/favicon.ico": ("favicon.ico", "image/x-icon"), | ||
| "/favicon.png": ("favicon-32.png", "image/png"), |
Comment on lines
+428
to
+433
| def _make_favicon_handler(filename: str, media_type: str): | ||
| path = _static_dir / filename | ||
|
|
||
| async def _handler(): | ||
| return FileResponse(path, media_type=media_type, headers=_favicon_headers) | ||
|
|
Comment on lines
+436
to
+439
| for _route, (_fname, _mime) in _favicon_files.items(): | ||
| app.add_api_route( | ||
| _route, _make_favicon_handler(_fname, _mime), include_in_schema=False | ||
| ) |
ZaynJarvis
approved these changes
May 7, 2026
Collaborator
ZaynJarvis
left a comment
ZaynJarvis
left a comment
There was a problem hiding this comment.
ok.. if it works it works; 生态接入的 feat
ZaynJarvis
pushed a commit
that referenced
this pull request
May 7, 2026
#1881) The favicons added in #1879 were generated with `-background white -extent` to pad the trimmed logo to a square canvas. The source ov-logo.png is a transparent sRGBA PNG, so this baked a solid white square into the alpha channel. On dark-mode browser tabs the icon showed up as a white block. Regenerate the variants with `-background none` so the padded canvas stays transparent. The visible glyph is unchanged; only the previously white padding is now alpha=0.
17 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Browsers and MCP clients (claude.ai, Claude Desktop) auto-fetch
/favicon.icoand/apple-touch-icon.pngto display a server icon. The 1933 API server previously returned JSON 404s for these paths, and the VitePress docs site pointed its<link rel=\"icon\">at the 1000×1000ov-logo.pngused by the in-page nav — that PNG has heavy whitespace, so the actual sail glyph rendered visibly small inside the browser tab.This PR adds tight, properly-sized favicon variants (trimmed of whitespace, padded to a square canvas) and wires them into both the API server and the docs site.
Related Issue
N/A
Type of Change
Changes Made
API server (port 1933)
/favicon.ico,/favicon.png,/apple-touch-icon.pngroutes inopenviking/server/app.py, each withCache-Control: public, max-age=86400.openviking/console/static/(already declared inpyproject.toml[tool.setuptools.package-data], so wheels pick them up without an extra entry):favicon.ico— multi-size (16/32/48), ~15 KBfavicon-32.png— 32×32, ~1.7 KBapple-touch-icon.png— 180×180, ~14 KB (used by claude.ai / iOS)openviking/console/static/index.html<link rel=\"icon\">/apple-touch-iconto the new files.Docs site (VitePress)
docs/images/(VitePress'svite.publicDir, served at site root).docs/.vitepress/config.tshead:to reference the tight variants instead of the paddedov-logo.png. The in-page navthemeConfig.logokeeps using the original PNG — the whitespace looks balanced there.Source
All variants were generated from
docs/images/ov-logo.pngviamagick -trim +repage -gravity center -extentto crop whitespace and square the canvas before resizing.Testing
Local verification:
python3 -c \"import ast; ast.parse(open('openviking/server/app.py').read())\"— syntax OK.curl -I http://localhost:1933/favicon.icoreturns200withcontent-type: image/x-icon.curl -I http://localhost:1933/apple-touch-icon.pngreturns200withcontent-type: image/png./console) shows the OV logo at expected size.npm run docs:devindocs/resolves the new icons via${base}favicon.ico, etc.Checklist
Screenshots (if applicable)
N/A — see
docs/images/ov-logo.pngfor the source logo. Generated assets are committed underopenviking/console/static/anddocs/images/.Additional Notes
FileResponsefrom FastAPI.docs/.Cache-Control: public, max-age=86400keeps repeat favicon fetches off the API server.