feat: new app icon & logo from @Tetonne's suggestion (#39)#43
Conversation
Replace the black-background shield icon with the dark-navy shield+spear squircle suggested by @Tetonne in #39. Applied everywhere: app icon (AppIcon.icns), in-app logo (VPNBypass.png), and the README banner. The source art is a flattened JPEG with a baked-in transparency checkerboard; scripts/generate-icon.py crops the squircle, rebuilds real alpha via a macOS continuous-corner superellipse mask, and emits the full iconset plus the 512px in-app logo.
📝 WalkthroughWalkthroughAdds a standalone icon-generation script that detects a content ROI, creates a squircle-style alpha mask, and exports a 1024×1024 RGBA master plus macOS ChangesRuntime VPN-state & UI
macOS Icon Generation
Sequence Diagram(s)sequenceDiagram
participant AppDelegate
participant HelperManager
participant RouteManager
participant NetworkInterfaces
AppDelegate->>HelperManager: ensureHelperReady()
alt helper not ready
AppDelegate->>RouteManager: detectVPNStateOnly() (async)
RouteManager->>NetworkInterfaces: query SSID / gateway / interfaces
RouteManager-->>AppDelegate: publish isVPNConnected, vpnInterface, vpnType, localGateway, isLoading=false
else helper ready
AppDelegate->>RouteManager: detectAndApplyRoutesAsync()
end
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
scripts/generate-icon.py (6)
3-3: ⚡ Quick winRemove unused
mathimport.The
mathmodule is imported but never used in the script.♻️ Proposed fix
-import os, math +import os🤖 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 `@scripts/generate-icon.py` at line 3, Remove the unused math import from the top of scripts/generate-icon.py by deleting the ", math" portion of the import statement (the line "import os, math") so only the used modules (e.g., os) are imported; ensure no other references to math remain in the file and run a quick linter/flake8 check to confirm.
1-3: ⚡ Quick winDocument script dependencies.
The script requires
numpyandPillow, but there's norequirements.txtor inline documentation. Add a docstring or comment at the top listing dependencies and usage.📝 Example docstring
+#!/usr/bin/env python3 +""" +Generate macOS app icon assets from a source image. + +Dependencies: + - numpy + - Pillow (PIL) + +Usage: + python generate-icon.py <source_image> [output_dir] +""" + import numpy as np🤖 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 `@scripts/generate-icon.py` around lines 1 - 3, Add a module-level docstring at the top of scripts/generate-icon.py that lists the runtime dependencies (numpy, Pillow), shows the pip install command (e.g., pip install numpy pillow), and provides a one-line usage example or invocation pattern for the script; update or create the top-of-file docstring so future readers and automated tooling can see the dependencies and how to run the script.
27-27: ⚡ Quick winDocument squircle parameters.
The superellipse exponent
n=5.0and edge widthedge=0.06are magic numbers. Add comments or extract as constants to clarify their effect on the mask shape.♻️ Example
# macOS continuous-corner squircle (superellipse) mask, full-bleed +SQUIRCLE_EXPONENT = 5.0 # higher = more square, lower = more circular +ANTIALIAS_EDGE = 0.06 # edge softness (0.06 = ~3% of radius) + N=1024 yy,xx=np.mgrid[0:N,0:N].astype(float) xn=(xx-(N-1)/2)/((N-1)/2); yn=(yy-(N-1)/2)/((N-1)/2) -n=5.0 +n=SQUIRCLE_EXPONENT d=np.abs(xn)**n+np.abs(yn)**n # antialiased edge around d==1 -edge=0.06 +edge=ANTIALIAS_EDGE alpha=np.clip((1.0-d)/edge + 0.5, 0, 1)Also applies to: 30-30
🤖 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 `@scripts/generate-icon.py` at line 27, The squircle parameters are magic numbers; extract n and edge into clearly named constants (e.g., SQUIRCLE_EXPONENT = 5.0 and EDGE_WIDTH = 0.06) and add brief comments explaining their effect on the mask shape (higher exponent -> sharper corners, edge controls border thickness/feather). Update occurrences of n and edge in scripts/generate-icon.py (and the matching spots around line 30) to use these constants so the intent is clear and easier to tune.
12-13: 💤 Low valueSplit semicolon-separated statements for readability.
Multiple statements on one line reduce readability and are flagged by linters (E702).
♻️ Example for lines 12-13
-mx=np.maximum(np.maximum(r,g),b); mn=np.minimum(np.minimum(r,g),b) -sat=mx-mn; bright=mx +mx = np.maximum(np.maximum(r,g),b) +mn = np.minimum(np.minimum(r,g),b) +sat = mx-mn +bright = mxAlso applies to: 20-20, 26-26, 40-40
🤖 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 `@scripts/generate-icon.py` around lines 12 - 13, Split the semicolon-separated statements into separate lines to fix lint E702 and improve readability: replace combined assignments like "mx=np.maximum(np.maximum(r,g),b); mn=np.minimum(np.minimum(r,g),b)" with two distinct statements (one for mx, one for mn), and do the same for other occurrences such as the "sat=mx-mn; bright=mx" pair and the similar semicolon-joined lines at the other spots noted; locate these by the variables used (mx, mn, sat, bright, r, g, b) in generate-icon.py and refactor each semicolon-delimited expression into its own line.
19-19: ⚡ Quick winExtract magic margin constant.
The
6pixel margin is unexplained. Extract it as a named constant for clarity.♻️ Example
+CROP_MARGIN = 6 # pixels -half = side/2.0 + 6 # tiny margin +half = side/2.0 + CROP_MARGIN🤖 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 `@scripts/generate-icon.py` at line 19, The hard-coded 6px "tiny margin" in the calculation half = side/2.0 + 6 should be extracted into a clearly named constant (e.g., ICON_MARGIN_PX or DEFAULT_MARGIN_PX) declared near the top of scripts/generate-icon.py; replace the literal in the expression with that constant and add a short comment explaining its purpose so the margin is self-documented and easy to adjust.
14-14: ⚡ Quick winDocument or extract magic numbers for maintainability.
The heuristic
(sat>28) | ((bright<140)&(b>=r))uses unexplained constants (28, 140). These make the code hard to tune or understand. Consider adding comments explaining the thresholds or extracting them as named constants.♻️ Example refactor
+# Heuristic thresholds: detect saturated colors or dark-blue regions +SAT_THRESHOLD = 28 # min saturation delta (max-min RGB) +BRIGHT_THRESHOLD = 140 # max brightness for dark region detection + -core = (sat>28) | ((bright<140)&(b>=r)) +core = (sat > SAT_THRESHOLD) | ((bright < BRIGHT_THRESHOLD) & (b >= r))🤖 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 `@scripts/generate-icon.py` at line 14, Extract the hard-coded numeric thresholds in the core pixel heuristic into named constants (e.g., SAT_THRESHOLD = 28, BRIGHT_THRESHOLD = 140) and replace the inline literals in the expression used to compute core (the line assigning core = (sat>28) | ((bright<140)&(b>=r))). Add a short comment above the constants explaining their meaning (why saturation and brightness thresholds were chosen) so future maintainers can tune them easily, and update any related documentation or tests that rely on these thresholds.
🤖 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 `@scripts/generate-icon.py`:
- Line 9: Add explicit error handling around opening the source image: check
that the SRC path exists (os.path.exists(SRC)) before calling Image.open or wrap
Image.open(SRC) in a try/except FileNotFoundError to print a clear user-facing
message and exit (use sys.exit(1)). Ensure any new code references the same SRC
variable and the Image.open(...) call so the script fails gracefully when the
source image is missing and informs the user how to fix it.
- Around line 5-7: The script currently hardcodes SRC and OUT (SRC =
"/tmp/tetonne1.png", OUT = "/tmp/iconout") making it non-portable; update
generate-icon.py to parse command-line arguments (e.g., using argparse) to
accept --src and --out (provide sensible defaults such as
"assets/AppIcon-source-tetonne.png" for src and a local output folder for out),
replace the hardcoded SRC and OUT variables with the parsed values, and keep the
existing os.makedirs(OUT, exist_ok=True) behavior so the output directory is
created as needed; ensure any references to SRC and OUT inside functions/classes
are updated to use the new variables (search for SRC, OUT, os.makedirs to locate
usages).
- Around line 14-16: The core mask computation can produce empty xs/ys arrays
causing xs.min()/ys.min() to raise ValueError; after computing ys,xs =
np.where(core) (referencing variables core, xs, ys and the bounding calc
x0,x1,y0,y1) add a guard that checks if xs.size==0 or ys.size==0 and fail
gracefully (raise a clear exception, log an error, or return early) with a
helpful message like "no core pixels detected" before attempting xs.min()/max()
or ys.min()/max().
---
Nitpick comments:
In `@scripts/generate-icon.py`:
- Line 3: Remove the unused math import from the top of scripts/generate-icon.py
by deleting the ", math" portion of the import statement (the line "import os,
math") so only the used modules (e.g., os) are imported; ensure no other
references to math remain in the file and run a quick linter/flake8 check to
confirm.
- Around line 1-3: Add a module-level docstring at the top of
scripts/generate-icon.py that lists the runtime dependencies (numpy, Pillow),
shows the pip install command (e.g., pip install numpy pillow), and provides a
one-line usage example or invocation pattern for the script; update or create
the top-of-file docstring so future readers and automated tooling can see the
dependencies and how to run the script.
- Line 27: The squircle parameters are magic numbers; extract n and edge into
clearly named constants (e.g., SQUIRCLE_EXPONENT = 5.0 and EDGE_WIDTH = 0.06)
and add brief comments explaining their effect on the mask shape (higher
exponent -> sharper corners, edge controls border thickness/feather). Update
occurrences of n and edge in scripts/generate-icon.py (and the matching spots
around line 30) to use these constants so the intent is clear and easier to
tune.
- Around line 12-13: Split the semicolon-separated statements into separate
lines to fix lint E702 and improve readability: replace combined assignments
like "mx=np.maximum(np.maximum(r,g),b); mn=np.minimum(np.minimum(r,g),b)" with
two distinct statements (one for mx, one for mn), and do the same for other
occurrences such as the "sat=mx-mn; bright=mx" pair and the similar
semicolon-joined lines at the other spots noted; locate these by the variables
used (mx, mn, sat, bright, r, g, b) in generate-icon.py and refactor each
semicolon-delimited expression into its own line.
- Line 19: The hard-coded 6px "tiny margin" in the calculation half = side/2.0 +
6 should be extracted into a clearly named constant (e.g., ICON_MARGIN_PX or
DEFAULT_MARGIN_PX) declared near the top of scripts/generate-icon.py; replace
the literal in the expression with that constant and add a short comment
explaining its purpose so the margin is self-documented and easy to adjust.
- Line 14: Extract the hard-coded numeric thresholds in the core pixel heuristic
into named constants (e.g., SAT_THRESHOLD = 28, BRIGHT_THRESHOLD = 140) and
replace the inline literals in the expression used to compute core (the line
assigning core = (sat>28) | ((bright<140)&(b>=r))). Add a short comment above
the constants explaining their meaning (why saturation and brightness thresholds
were chosen) so future maintainers can tune them easily, and update any related
documentation or tests that rely on these thresholds.
🪄 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
Run ID: 130ff0ea-c234-4919-9a77-d7c2ec6ef464
⛔ Files ignored due to path filters (4)
assets/AppIcon-source-tetonne.pngis excluded by!**/*.pngassets/VPNBypass-AppIcon.pngis excluded by!**/*.pngassets/VPNBypass.pngis excluded by!**/*.pngdocs/images/banner.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
assets/AppIcon.icnsscripts/generate-icon.py
| SRC = "/tmp/tetonne1.png" | ||
| OUT = "/tmp/iconout" | ||
| os.makedirs(OUT, exist_ok=True) |
There was a problem hiding this comment.
Replace hardcoded /tmp paths with configurable arguments.
The script uses hardcoded /tmp paths, making it non-reproducible and non-portable. For a versioned "reproducible generator," the source and output paths must be configurable. The PR summary mentions this script generates assets from assets/AppIcon-source-tetonne.png, yet the script points to /tmp/tetonne1.png.
🔧 Suggested approach
Add command-line arguments:
+import sys
+
+if len(sys.argv) < 2:
+ print("Usage: python generate-icon.py <source_image> [output_dir]")
+ sys.exit(1)
+
-SRC = "/tmp/tetonne1.png"
-OUT = "/tmp/iconout"
+SRC = sys.argv[1]
+OUT = sys.argv[2] if len(sys.argv) > 2 else "assets"
os.makedirs(OUT, exist_ok=True)🧰 Tools
🪛 Ruff (0.15.14)
[error] 5-5: Probable insecure usage of temporary file or directory: "/tmp/tetonne1.png"
(S108)
[error] 6-6: Probable insecure usage of temporary file or directory: "/tmp/iconout"
(S108)
🤖 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 `@scripts/generate-icon.py` around lines 5 - 7, The script currently hardcodes
SRC and OUT (SRC = "/tmp/tetonne1.png", OUT = "/tmp/iconout") making it
non-portable; update generate-icon.py to parse command-line arguments (e.g.,
using argparse) to accept --src and --out (provide sensible defaults such as
"assets/AppIcon-source-tetonne.png" for src and a local output folder for out),
replace the hardcoded SRC and OUT variables with the parsed values, and keep the
existing os.makedirs(OUT, exist_ok=True) behavior so the output directory is
created as needed; ensure any references to SRC and OUT inside functions/classes
are updated to use the new variables (search for SRC, OUT, os.makedirs to locate
usages).
| OUT = "/tmp/iconout" | ||
| os.makedirs(OUT, exist_ok=True) | ||
|
|
||
| im = Image.open(SRC).convert("RGB") |
There was a problem hiding this comment.
Add error handling for missing source file.
The script will crash with FileNotFoundError if the source image does not exist. Add a check or let the user know the requirement upfront.
🛡️ Proposed fix
+if not os.path.isfile(SRC):
+ print(f"Error: Source image not found: {SRC}")
+ sys.exit(1)
+
im = Image.open(SRC).convert("RGB")🤖 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 `@scripts/generate-icon.py` at line 9, Add explicit error handling around
opening the source image: check that the SRC path exists (os.path.exists(SRC))
before calling Image.open or wrap Image.open(SRC) in a try/except
FileNotFoundError to print a clear user-facing message and exit (use
sys.exit(1)). Ensure any new code references the same SRC variable and the
Image.open(...) call so the script fails gracefully when the source image is
missing and informs the user how to fix it.
| core = (sat>28) | ((bright<140)&(b>=r)) | ||
| ys,xs=np.where(core) | ||
| x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max() |
There was a problem hiding this comment.
Add validation for empty core detection.
If no pixels match the saturation/brightness heuristic (line 14), xs and ys will be empty arrays, causing xs.min() on line 16 to raise ValueError. Add a check to fail gracefully if core detection finds nothing.
🛡️ Proposed fix
core = (sat>28) | ((bright<140)&(b>=r))
ys,xs=np.where(core)
+if len(xs) == 0:
+ print("Error: Could not detect content region in source image.")
+ sys.exit(1)
x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| core = (sat>28) | ((bright<140)&(b>=r)) | |
| ys,xs=np.where(core) | |
| x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max() | |
| core = (sat>28) | ((bright<140)&(b>=r)) | |
| ys,xs=np.where(core) | |
| if len(xs) == 0: | |
| print("Error: Could not detect content region in source image.") | |
| sys.exit(1) | |
| x0,x1,y0,y1 = xs.min(),xs.max(),ys.min(),ys.max() |
🤖 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 `@scripts/generate-icon.py` around lines 14 - 16, The core mask computation can
produce empty xs/ys arrays causing xs.min()/ys.min() to raise ValueError; after
computing ys,xs = np.where(core) (referencing variables core, xs, ys and the
bounding calc x0,x1,y0,y1) add a guard that checks if xs.size==0 or ys.size==0
and fail gracefully (raise a clear exception, log an error, or return early)
with a helpful message like "no core pixels detected" before attempting
xs.min()/max() or ys.min()/max().
- Recolor JPEG checkerboard fringe to navy before superellipse mask so the app icon no longer shows white edges on the left/right. - Remove the logo image from the menu-bar dropdown header, keeping only the branded app name text. - When the privileged helper is not ready, detect VPN state for display only instead of leaving isLoading=true forever, which clears the permanent "Setting Up..." spinner.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/generate-icon.py`:
- Around line 77-83: The base64 encoding step opens the image file without
closing it; change the one-liner that calls open(f"{OUT}/banner_logo.png",
"rb").read() to explicitly open the image in a with statement (e.g., with
open(f"{OUT}/banner_logo.png", "rb") as img:) and use img.read() for
base64.b64encode, then write the decoded string to the existing
banner_logo.b64.txt file inside its with block so both file handles (the image
and the output) are properly closed; update references to OUT and
banner_logo.png accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6cc7012-23dd-48aa-bbe5-4e44a059b54d
⛔ Files ignored due to path filters (3)
assets/VPNBypass-AppIcon.pngis excluded by!**/*.pngassets/VPNBypass.pngis excluded by!**/*.pngdocs/images/banner.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
Sources/VPNBypassCore/MenuBarViews.swiftSources/VPNBypassCore/RouteManager.swiftSources/VPNBypassCore/VPNBypassApp.swiftassets/AppIcon.icnsscripts/generate-icon.py
💤 Files with no reviewable changes (1)
- Sources/VPNBypassCore/MenuBarViews.swift
| # banner logo + base64 | ||
| import base64 | ||
| small = master.resize((160, 160), Image.LANCZOS) | ||
| small.save(f"{OUT}/banner_logo.png") | ||
| with open(f"{OUT}/banner_logo.b64.txt", "w") as f: | ||
| f.write(base64.b64encode(open(f"{OUT}/banner_logo.png", "rb").read()).decode()) | ||
| print("logo + banner assets written") |
There was a problem hiding this comment.
Close file handle properly in base64 encoding.
Line 82 opens a file handle without closing it. While Python's garbage collector will eventually clean it up, this is poor resource management.
🛡️ Proposed fix
-with open(f"{OUT}/banner_logo.b64.txt", "w") as f:
- f.write(base64.b64encode(open(f"{OUT}/banner_logo.png", "rb").read()).decode())
+with open(f"{OUT}/banner_logo.b64.txt", "w") as f:
+ with open(f"{OUT}/banner_logo.png", "rb") as img:
+ f.write(base64.b64encode(img.read()).decode())🤖 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 `@scripts/generate-icon.py` around lines 77 - 83, The base64 encoding step
opens the image file without closing it; change the one-liner that calls
open(f"{OUT}/banner_logo.png", "rb").read() to explicitly open the image in a
with statement (e.g., with open(f"{OUT}/banner_logo.png", "rb") as img:) and use
img.read() for base64.b64encode, then write the decoded string to the existing
banner_logo.b64.txt file inside its with block so both file handles (the image
and the output) are properly closed; update references to OUT and
banner_logo.png accordingly.
Summary
Implements the icon refresh discussed in #39. @Tetonne suggested a new shield+spear icon and offered it for the app; this PR wires that art in everywhere (app icon, in-app logo, README banner) so it can be tested locally.
Chose the dark-navy variant — it matches the app's existing dark theme and the mood of the old black-background icon, just with a proper macOS squircle instead of a bare black square.
What changed
assets/AppIcon.icns— Dock / Finder / Launchpad / notification icon. Full iconset (16→1024, incl. @2x).assets/VPNBypass.png— in-app logo (Settings + menu bar header), 512² transparent.assets/VPNBypass-AppIcon.png— 1024² master source.docs/images/banner.svg— README banner logo slot swapped to the new squircle (now square, 90×90).assets/AppIcon-source-tetonne.png— original art from the issue, kept for provenance.scripts/generate-icon.py— reproducible generator (see note below).Note on the source art
Tetonne's image is a flattened JPEG — the "transparent" checkerboard is baked into the pixels, not real alpha.
scripts/generate-icon.pydetects the squircle, crops it, and rebuilds genuine transparency with a macOS continuous-corner (superellipse) mask, then emits every size. Quality is bounded by the ~700px AI-generated source, but it's clean at all sizes.Test plan
Closes #39.
Thanks @Tetonne for the icon suggestion!
Summary by CodeRabbit