Skip to content

feat: add static script gallery with GitHub Pages deployment#52

Merged
pfeerick merged 15 commits into
mainfrom
pfeerick/gallery-site
May 12, 2026
Merged

feat: add static script gallery with GitHub Pages deployment#52
pfeerick merged 15 commits into
mainfrom
pfeerick/gallery-site

Conversation

@pfeerick
Copy link
Copy Markdown
Member

@pfeerick pfeerick commented May 12, 2026

Summary

  • Adds tools/generate_site.py — a pure-stdlib Python generator that reads scripts.json and produces a self-contained site/index.html using Tailwind CSS CDN and Alpine.js
  • Adds .github/workflows/gh-pages.yml — builds the site with uv run and deploys to GitHub Pages on push to main; PR builds upload a preview artifact instead

Features

  • Card grid with image carousels (auto-advance, prev/next arrows, dot indicators)
  • Lightbox for full-screen image viewing with keyboard navigation
  • Search (name + description), category tab filter, and multi-select tag chips (AND logic)
  • Dark/light mode toggle persisted in localStorage

Setup required

Enable GitHub Pages in repo Settings → Pages → Source: GitHub Actions

Test plan

  • Open site/index.html locally after running uv run tools/generate_site.py
  • Verify all 47 scripts appear, category tabs and tag chips filter correctly
  • Check carousel prev/next and lightbox work
  • Dark/light mode toggle persists on reload
  • Confirm GitHub Actions build passes and Pages URL is live after merge

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Script Gallery site with search, category & tag filtering, dark-mode persistence, per-script image carousels, and keyboard-navigable lightbox.
    • CLI to generate the static gallery from script data and assets.
  • Chores

    • Automated deployment workflow added to publish the gallery to GitHub Pages.
    • Local site output directory ignored and image references updated for one script entry.

Review Change Stack

Generates a single index.html from scripts.json using Tailwind CSS CDN
and Alpine.js for search, category tabs, tag filtering, image carousels,
lightbox, and dark/light mode. GitHub Actions workflow deploys to Pages
on push to main.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A CLI reads scripts.json, copies repo-local ASSETS images into site/images/, rewrites image paths, extracts categories/tags, and outputs a Tailwind+Alpine index.html. A GitHub Actions workflow builds the site on relevant changes and deploys it to GitHub Pages; site/ is gitignored.

Changes

Script Gallery Site Generation and Deployment

Layer / File(s) Summary
Site generator implementation
tools/generate_site.py, scripts.json
load_scripts() validates scripts.json. process_images() copies local ASSETS images into site/images/, preserves http/https URLs, rejects out-of-repo or missing files with stderr warnings, and rewrites each script's images entries to output-relative paths. get_categories() and get_all_tags() derive sorted unique metadata. generate_html() embeds scripts/categories/tags JSON into a Tailwind + Alpine.js template with dark-mode persistence, search, category tabs, AND tag filtering, per-script carousels, and a lightbox modal with keyboard navigation. main() provides CLI args --scripts-json, --assets-dir, --output-dir and writes index.html. The scripts.json entry for "EdgeTX Goodies" now references three local screenshots (screenshot-6.png to screenshot-8.png).
Build and deployment workflow
.github/workflows/gh-pages.yml, .gitignore
Adds a GitHub Actions workflow that triggers on push to main, pull requests, and manual dispatch, filtered to ASSETS/**, scripts.json, tools/generate_site.py, and the workflow file. The build job checks out code, sets up uv, runs tools/generate_site.py, and uploads either a Pages artifact (for push/dispatch) or a PR preview artifact. The deploy job runs after build for push/dispatch events and deploys using actions/deploy-pages. .gitignore now ignores the generated site/ directory.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • EdgeTX/lua-scripts#51: Populates scripts.json and manages ASSETS image files that the new site generator processes for the gallery.

Poem

🐰 I hop through folders, copy each frame bright,

I tuck screenshots in rows and set the theme to night,
Tags and tabs and carousels softly play,
Pages carry our gallery out to display,
A small rabbit cheers the site’s first light.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add static script gallery with GitHub Pages deployment' accurately summarizes the main changes: a new static gallery generator tool and GitHub Pages CI/CD workflow.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pfeerick/gallery-site

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
tools/generate_site.py (1)

83-87: ⚡ Quick win

Narrow the exception type in image copy handling.

Catching Exception here can hide unrelated programming errors. Catch copy/path exceptions instead.

Suggested fix
-            except Exception as e:
+            except (OSError, shutil.Error) as e:
🤖 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 `@tools/generate_site.py` around lines 83 - 87, Replace the broad "except
Exception as e" around the image copy logic with a narrow exception catch for
filesystem/copy errors (e.g., FileNotFoundError, PermissionError, OSError and
shutil.Error) so only copy/path related failures are handled; keep the existing
warning message using image_path and script.get('name', 'unknown') and re-use
the caught exception variable to include error details in the log.
🤖 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 @.github/workflows/gh-pages.yml:
- Around line 32-33: Replace mutable action tags with their immutable commit
SHAs: update the uses entries for actions/checkout (currently uses:
actions/checkout@v6) and astral-sh/setup-uv (uses: astral-sh/[email protected]) to
reference the corresponding full commit SHA; do the same for the other
occurrences called out in the review (the lines referenced at 38, 43, 57).
Locate the strings "actions/checkout@" and "astral-sh/setup-uv@" in the workflow
file and swap the version tags for the verified commit SHAs from the actions’
repositories/releases so the workflow pins to exact commits.

In `@tools/generate_site.py`:
- Around line 114-116: The JSON string variables (scripts_json, categories_json,
all_tags_json) are injected into page <script> blocks and must be escaped to
prevent a metadata value like "</script>" from breaking out and causing XSS;
update the generation code that produces scripts_json, categories_json and
all_tags_json (and the other similar spots) to sanitize the dumped JSON by
escaping the sequence "</script>" (e.g. replace it with "<\/script>" or
otherwise escape closing script tags) before writing into the template so the
embedded JSON cannot prematurely terminate the script element.
- Around line 289-290: The anchor tag that opens external links in a new tab
(the <a> element with target="_blank" using script.infourl and displaying
script.name) should include rel="noopener noreferrer" to prevent
reverse-tabnabbing; update the <a :href="script.infourl" target="_blank" ...>
element to add rel="noopener noreferrer" alongside the existing attributes.
- Around line 69-77: The current image copying logic only preserves the
immediate parent folder (relative_path.parent.name), which collapses nested
directories and can cause filename collisions; change it to preserve the full
relative directory structure by constructing dest_path as images_output /
relative_path (or images_output / relative_path.parent / relative_path.name),
ensure you call dest_path.parent.mkdir(parents=True, exist_ok=True) to create
the full nested directories before writing, and use that dest_path when
saving/copying the image (update references to relative_path, dest_path, and
dest_subdir accordingly).
- Around line 58-65: The code currently builds source_path = assets_dir.parent /
image_path using untrusted values from scripts.json, allowing '../' or absolute
paths to escape the assets directory; fix by validating and constraining
image_path before copying: reject absolute paths and any path that resolves
outside the intended assets root by computing candidate = (assets_dir.parent /
image_path).resolve() and comparing it with assets_dir.parent.resolve() (use
.resolve() on both) to ensure candidate.startswith(assets_dir.parent.resolve()),
log a warning and continue if the check fails, and only proceed with the
existing exists() check and copy when the candidate is inside the allowed
directory.
- Around line 341-343: The keyboard handlers on the modal are bound at element
level so they only fire when the modal has focus; change the three bindings to
use the window modifier so they listen globally (use the `.window` modifier on
the `@keydown.escape`, `@keydown.left`, and `@keydown.right` handlers) so close(),
prev(), and next() work immediately without focusing the modal.

---

Nitpick comments:
In `@tools/generate_site.py`:
- Around line 83-87: Replace the broad "except Exception as e" around the image
copy logic with a narrow exception catch for filesystem/copy errors (e.g.,
FileNotFoundError, PermissionError, OSError and shutil.Error) so only copy/path
related failures are handled; keep the existing warning message using image_path
and script.get('name', 'unknown') and re-use the caught exception variable to
include error details in the log.
🪄 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 Plus

Run ID: 0e7beabf-a9d7-41c0-8ddd-841a80a0bb7a

📥 Commits

Reviewing files that changed from the base of the PR and between b7b3031 and f4ac92f.

📒 Files selected for processing (3)
  • .github/workflows/gh-pages.yml
  • .gitignore
  • tools/generate_site.py

Comment thread .github/workflows/gh-pages.yml
Comment thread tools/generate_site.py Outdated
Comment thread tools/generate_site.py
Comment thread tools/generate_site.py Outdated
Comment thread tools/generate_site.py Outdated
Comment thread tools/generate_site.py Outdated
pfeerick and others added 4 commits May 12, 2026 20:34
- Wrap carousel and lightbox prev/next buttons in <div class="contents">
  so Alpine x-if renders all siblings, not just the first (caused missing
  forward arrow)
- Add tailwind.config darkMode: 'class' so the toggle actually works
  instead of deferring to OS preference
- Replace broken partial sun SVG with Heroicons v2 outline sun and moon,
  matching the stroke style used throughout the rest of the UI

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…eTX Goodies

Downloads 4 widget screenshots from upstream repo and references them
locally, removing the 404 GitHub raw URL.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (4)
tools/generate_site.py (4)

352-354: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Bind keyboard shortcuts to window level for reliable lightbox navigation.

Element-level @keydown.* listeners only fire when the modal div has focus. Users expect arrow keys and Escape to work immediately without clicking the modal first. Use the .window modifier.

Proposed fix
-        `@keydown.escape`="close()"
-        `@keydown.left`="prev()"
-        `@keydown.right`="next()"
+        `@keydown.window.escape`="close()"
+        `@keydown.window.left`="prev()"
+        `@keydown.window.right`="next()"
🤖 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 `@tools/generate_site.py` around lines 352 - 354, The keyboard handlers on the
modal are currently bound at the element level and won't fire unless the modal
has focus; update the Vue keydown directives to use the .window modifier so
Escape, left and right arrow keys work globally (replace
`@keydown.escape`="close()", `@keydown.left`="prev()", `@keydown.right`="next()" with
their .window variants). Locate the modal template where close(), prev(), and
next() are attached and change the directives to `@keydown.window.escape`,
`@keydown.window.left`, and `@keydown.window.right` respectively so the handlers
fire at the window level.

125-127: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Escape embedded JSON to prevent script injection.

If script metadata contains </script>, the page script will be prematurely terminated, creating an injection/XSS risk. Replace < with \u003c in the JSON output.

Proposed fix
+def _safe_json(obj) -> str:
+    """Escape < to prevent </script> injection."""
+    return json.dumps(obj, ensure_ascii=False).replace("<", "\\u003c")
+

 def generate_html(scripts: list, categories: list, all_tags: list) -> str:
     """Generate the complete HTML file."""

     # Convert data to JSON strings
-    scripts_json = json.dumps(scripts, indent=2)
-    categories_json = json.dumps(categories)
-    all_tags_json = json.dumps(all_tags)
+    scripts_json = _safe_json(scripts)
+    categories_json = _safe_json(categories)
+    all_tags_json = _safe_json(all_tags)
🤖 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 `@tools/generate_site.py` around lines 125 - 127, The JSON strings produced for
embedding (variables scripts_json, categories_json, and all_tags_json in
tools/generate_site.py) must escape '<' to avoid prematurely terminating a
<script> tag; after calling json.dumps(...) for each of these (scripts_json,
categories_json, all_tags_json) replace all '<' characters with the Unicode
escape sequence '\u003c' (e.g., scripts_json = json.dumps(scripts,
...).replace('<', '\\u003c')) so any embedded "</script>" becomes safe before
writing them into the page.

80-87: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve full relative image path to avoid collisions.

The current logic only preserves relative_path.parent.name (immediate parent folder). Nested paths like ASSETS/foo/bar/img.png and ASSETS/baz/bar/img.png would both map to images/bar/img.png, causing overwrites.

Proposed fix
-            relative_path = Path(image_path)
-            dest_path = images_output / relative_path.name
-
-            # If the relative path has a subdirectory, preserve it
-            if relative_path.parent != Path('.'):
-                dest_subdir = images_output / relative_path.parent.name
-                dest_subdir.mkdir(parents=True, exist_ok=True)
-                dest_path = dest_subdir / relative_path.name
+            relative_path = Path(image_path)
+            dest_path = images_output / relative_path
+            dest_path.parent.mkdir(parents=True, exist_ok=True)
🤖 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 `@tools/generate_site.py` around lines 80 - 87, The code currently only
preserves the immediate parent folder when computing dest_subdir, causing
collisions for nested paths; update the logic in generate_site.py where
relative_path, dest_path, images_output and dest_subdir are used so that you
preserve the entire relative_path.parent (not just .name): if
relative_path.parent != Path('.') set dest_subdir = images_output /
relative_path.parent, mkdir(parents=True, exist_ok=True) and then set dest_path
= dest_subdir / relative_path.name so the full nested directories (e.g.,
ASSETS/foo/bar) are mirrored under images_output and avoid filename overwrites.

299-305: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add rel="noopener noreferrer" to external links.

The anchor with target="_blank" is missing the rel attribute, leaving a reverse-tabnabbing window channel open.

Proposed fix
-                                <a :href="script.infourl" target="_blank" class="text-red-600 dark:text-red-400 hover:underline flex items-center gap-1">
+                                <a :href="script.infourl" target="_blank" rel="noopener noreferrer" class="text-red-600 dark:text-red-400 hover:underline flex items-center gap-1">
🤖 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 `@tools/generate_site.py` around lines 299 - 305, The external anchor inside
the template conditioned on script.infourl uses target="_blank" but lacks
rel="noopener noreferrer", which enables reverse-tabnabbing; update the anchor
element (the <a> tag that binds :href="script.infourl" and includes
class="text-red-600...") to include rel="noopener noreferrer" so external links
opened in a new tab are safe.
🧹 Nitpick comments (2)
tools/generate_site.py (2)

496-510: 💤 Low value

Consider clearing interval on component destroy.

When cards are filtered out, the setInterval may continue running after the component is removed from the DOM. Alpine.js provides a destroy() lifecycle hook for cleanup.

Proposed fix
                 resumeAuto() {
                     if (this.script.images.length > 1) {
                         this.startAuto();
                     }
                 },

+                destroy() {
+                    clearInterval(this.timer);
+                },
+
                 prev() {
🤖 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 `@tools/generate_site.py` around lines 496 - 510, The interval created in
startAuto() (stored on this.timer) isn't cleared when the component is removed,
so add a destroy() lifecycle hook to call clearInterval(this.timer) (and
optionally set this.timer = null) to ensure the timer is cleaned up; update the
Alpine component that defines startAuto, pauseAuto, resumeAuto to include
destroy() { clearInterval(this.timer) } so the interval stops when the component
is destroyed.

94-98: 💤 Low value

Catch specific exceptions instead of bare Exception.

The broad except Exception can mask unexpected errors. Consider catching OSError or shutil.SameFileError specifically.

Proposed fix
-            except Exception as e:
+            except OSError as e:
🤖 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 `@tools/generate_site.py` around lines 94 - 98, Replace the broad "except
Exception as e" around the image copy with targeted exception handling: catch
shutil.SameFileError and OSError specifically (e.g., "except
(shutil.SameFileError, OSError) as e") so filesystem-related issues are handled
and other unexpected exceptions still surface; ensure shutil is imported and
keep the existing process of printing the warning using image_path and
script.get('name', 'unknown'), and re-raise or let other exceptions propagate
rather than silently swallowing them.
🤖 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.

Duplicate comments:
In `@tools/generate_site.py`:
- Around line 352-354: The keyboard handlers on the modal are currently bound at
the element level and won't fire unless the modal has focus; update the Vue
keydown directives to use the .window modifier so Escape, left and right arrow
keys work globally (replace `@keydown.escape`="close()", `@keydown.left`="prev()",
`@keydown.right`="next()" with their .window variants). Locate the modal template
where close(), prev(), and next() are attached and change the directives to
`@keydown.window.escape`, `@keydown.window.left`, and `@keydown.window.right`
respectively so the handlers fire at the window level.
- Around line 125-127: The JSON strings produced for embedding (variables
scripts_json, categories_json, and all_tags_json in tools/generate_site.py) must
escape '<' to avoid prematurely terminating a <script> tag; after calling
json.dumps(...) for each of these (scripts_json, categories_json, all_tags_json)
replace all '<' characters with the Unicode escape sequence '\u003c' (e.g.,
scripts_json = json.dumps(scripts, ...).replace('<', '\\u003c')) so any embedded
"</script>" becomes safe before writing them into the page.
- Around line 80-87: The code currently only preserves the immediate parent
folder when computing dest_subdir, causing collisions for nested paths; update
the logic in generate_site.py where relative_path, dest_path, images_output and
dest_subdir are used so that you preserve the entire relative_path.parent (not
just .name): if relative_path.parent != Path('.') set dest_subdir =
images_output / relative_path.parent, mkdir(parents=True, exist_ok=True) and
then set dest_path = dest_subdir / relative_path.name so the full nested
directories (e.g., ASSETS/foo/bar) are mirrored under images_output and avoid
filename overwrites.
- Around line 299-305: The external anchor inside the template conditioned on
script.infourl uses target="_blank" but lacks rel="noopener noreferrer", which
enables reverse-tabnabbing; update the anchor element (the <a> tag that binds
:href="script.infourl" and includes class="text-red-600...") to include
rel="noopener noreferrer" so external links opened in a new tab are safe.

---

Nitpick comments:
In `@tools/generate_site.py`:
- Around line 496-510: The interval created in startAuto() (stored on
this.timer) isn't cleared when the component is removed, so add a destroy()
lifecycle hook to call clearInterval(this.timer) (and optionally set this.timer
= null) to ensure the timer is cleaned up; update the Alpine component that
defines startAuto, pauseAuto, resumeAuto to include destroy() {
clearInterval(this.timer) } so the interval stops when the component is
destroyed.
- Around line 94-98: Replace the broad "except Exception as e" around the image
copy with targeted exception handling: catch shutil.SameFileError and OSError
specifically (e.g., "except (shutil.SameFileError, OSError) as e") so
filesystem-related issues are handled and other unexpected exceptions still
surface; ensure shutil is imported and keep the existing process of printing the
warning using image_path and script.get('name', 'unknown'), and re-raise or let
other exceptions propagate rather than silently swallowing them.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f542a11-6bfc-4c7b-a63c-d57195f43719

📥 Commits

Reviewing files that changed from the base of the PR and between f4ac92f and 8afd480.

⛔ Files ignored due to path filters (4)
  • ASSETS/edgetx-goodies/screenshot-5.png is excluded by !**/*.png
  • ASSETS/edgetx-goodies/screenshot-6.png is excluded by !**/*.png
  • ASSETS/edgetx-goodies/screenshot-7.png is excluded by !**/*.png
  • ASSETS/edgetx-goodies/screenshot-8.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • scripts.json
  • tools/generate_site.py

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
tools/generate_site.py (3)

89-93: 💤 Low value

Consider catching specific exceptions instead of bare Exception.

The broad except Exception works but masks the actual failure mode. Narrowing to OSError (which covers IOError, PermissionError, etc.) would be more precise while still handling all realistic file-copy failures.

♻️ Suggested refinement
-            except Exception as e:
+            except OSError as e:
🤖 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 `@tools/generate_site.py` around lines 89 - 93, The current broad except
Exception in the image-copying block masks real failure types; update the except
in the try/except surrounding the copy operation (the block referencing
image_path and script.get('name', 'unknown')) to catch OSError (e.g., except
OSError as e) so file I/O errors like IOError/PermissionError are handled
precisely while still logging the same warning to sys.stderr.

120-121: ⚡ Quick win

Consider broader HTML character escaping in JSON.

The current escaping handles </script> injection, but escaping all < characters would be more robust against edge cases (e.g., <!-- sequences or other tag-like content in metadata).

♻️ More robust escaping
     def _safe_json(obj) -> str:
-        return json.dumps(obj, ensure_ascii=False).replace('</script>', '<\\/script>')
+        return json.dumps(obj, ensure_ascii=False).replace('<', '\\u003c')

This escapes all < characters to their Unicode escape sequence, which is valid JSON and prevents any HTML tag injection without affecting the parsed values.

🤖 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 `@tools/generate_site.py` around lines 120 - 121, The _safe_json function
currently only replaces the string '</script>' which is insufficient; update
_safe_json to JSON-dump as before and then replace all '<' characters with the
Unicode escape '\\u003c' (i.e., use result.replace('<', '\\u003c')) so any
tag-like content (including comments like <!--) is neutralized while preserving
valid JSON parsing; keep ensure_ascii=False and the same function name
_safe_json.

482-524: ⚡ Quick win

Potential timer leak when cards are filtered out.

When a card component is removed from the DOM (e.g., filtered out), the setInterval timer created in init() is not cleared. This can cause orphaned timers accumulating over repeated filter changes.

♻️ Add cleanup via Alpine's destroy lifecycle
 function card(script, si) {
     return {
         script,
         currentIndex: 0,
         timer: null,

         init() {
             if (script.images.length > 1) {
                 this.startAuto();
             }
         },

+        destroy() {
+            this.pauseAuto();
+        },
+
         startAuto() {

Alpine.js calls destroy() when the component's element is removed from the DOM, ensuring the interval is cleared.

🤖 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 `@tools/generate_site.py` around lines 482 - 524, The card component starts
intervals in startAuto() but never clears them when removed; add a destroy()
method on the object returned by function card(...) that calls pauseAuto() to
clear the interval, make pauseAuto() defensively check for this.timer and set
this.timer = null after clearInterval, update startAuto() to clear any existing
timer before creating a new one, and make resumeAuto() only start a new timer
when script.images.length > 1 and this.timer is null so no orphaned intervals
remain when the component is destroyed or filtered out.
🤖 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.

Nitpick comments:
In `@tools/generate_site.py`:
- Around line 89-93: The current broad except Exception in the image-copying
block masks real failure types; update the except in the try/except surrounding
the copy operation (the block referencing image_path and script.get('name',
'unknown')) to catch OSError (e.g., except OSError as e) so file I/O errors like
IOError/PermissionError are handled precisely while still logging the same
warning to sys.stderr.
- Around line 120-121: The _safe_json function currently only replaces the
string '</script>' which is insufficient; update _safe_json to JSON-dump as
before and then replace all '<' characters with the Unicode escape '\\u003c'
(i.e., use result.replace('<', '\\u003c')) so any tag-like content (including
comments like <!--) is neutralized while preserving valid JSON parsing; keep
ensure_ascii=False and the same function name _safe_json.
- Around line 482-524: The card component starts intervals in startAuto() but
never clears them when removed; add a destroy() method on the object returned by
function card(...) that calls pauseAuto() to clear the interval, make
pauseAuto() defensively check for this.timer and set this.timer = null after
clearInterval, update startAuto() to clear any existing timer before creating a
new one, and make resumeAuto() only start a new timer when script.images.length
> 1 and this.timer is null so no orphaned intervals remain when the component is
destroyed or filtered out.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a7b39112-7e87-4fe3-86bd-be982f30be15

📥 Commits

Reviewing files that changed from the base of the PR and between 8afd480 and 47ebc1d.

📒 Files selected for processing (1)
  • tools/generate_site.py

pfeerick added 3 commits May 12, 2026 21:06
… warnings

The broad except Exception works but masks the actual failure mode. Narrowing to OSError (which covers IOError, PermissionError, etc.) would be more precise while still handling all realistic file-copy failures.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@tools/generate_site.py`:
- Line 347: The duplicate `@window`:filter-tag binding causes toggleTag to be
invoked twice; remove the extra global event binding so the handler from
gallery.init() is the sole listener. Locate the template or script element that
contains "@window:filter-tag='toggleTag($event.detail.tag)'" and delete that
attribute (and any identical binding occurrences around the other noted block),
leaving gallery.init()'s registration of the filter-tag event as the only
handler for toggling tags.
- Around line 318-325: The template iterates with x-for="tag in script.tags"
assuming script.tags is always an array, which breaks when tags is
null/undefined; update the Alpine template to guard the loop (for example change
the loop to x-for="tag in script.tags || []" or wrap the tag block in a template
with x-if="script.tags && script.tags.length") so the card render won’t fail for
scripts missing tags; this aligns with the optional handling in get_all_tags()
and existing filter logic.
🪄 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 Plus

Run ID: 611c892c-41f7-4502-b256-5c15687b0a90

📥 Commits

Reviewing files that changed from the base of the PR and between 47ebc1d and 43400c0.

📒 Files selected for processing (1)
  • tools/generate_site.py

Comment thread tools/generate_site.py
Comment thread tools/generate_site.py Outdated
@pfeerick pfeerick merged commit 239bdea into main May 12, 2026
3 checks passed
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.

1 participant