feat: restructure frontend for zero-config github pages deployment#156
Open
HanshalBobate wants to merge 4 commits into
Open
feat: restructure frontend for zero-config github pages deployment#156HanshalBobate wants to merge 4 commits into
HanshalBobate wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a static archive frontend under ChangesArchive site build and frontend
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Push as Push to main
participant Workflow as update-archive-index.yml
participant Script as build-site-index.mjs
participant Repo as Repository
Push->>Workflow: trigger on matched file changes
Workflow->>Script: run node scripts/build-site-index.mjs
Script->>Repo: walk files, build records
Script->>Repo: write docs/data/index.json
Workflow->>Repo: commit and push if index changed
sequenceDiagram
participant User
participant Router as router.js
participant Main as main.js
participant Data as data.js
participant Markdown as markdown.js
User->>Router: navigate to file route
Router->>Main: onRouteChange callback
Main->>Main: render() rebuilds shell
Main->>Data: fetchFileContent(record)
Data-->>Main: file text (cached or fetched)
Main->>Markdown: renderMarkdown(source) if markdown
Markdown-->>Main: escaped HTML
Main->>User: renderReaderBody with content
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
.github/workflows/update-archive-index.yml (2)
32-37: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo protection against concurrent runs or a stale branch on push.
If two qualifying pushes to
mainland close together, two workflow runs can both check out the same base commit, and the secondgit pushwill fail (non-fast-forward) since there's no rebase/retry, or two commits could race. Consider adding aconcurrencygroup to serialize runs and a rebase/retry before pushing.🔧 Suggested fix
+concurrency: + group: update-archive-index-${{ github.ref }} + cancel-in-progress: false + jobs: update-index: runs-on: ubuntu-latest permissions: contents: write steps: - name: Checkout Repository uses: actions/checkout@v4- name: Commit and push if changed run: | git config --global user.name "github-actions[bot]" git config --global user.email "github-actions[bot]`@users.noreply.github.com`" git add docs/data/index.json - git diff --quiet && git diff --staged --quiet || (git commit -m "chore: update archive index [skip ci]" && git push) + git diff --quiet && git diff --staged --quiet || (git commit -m "chore: update archive index [skip ci]" && git pull --rebase && git push)🤖 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 @.github/workflows/update-archive-index.yml around lines 32 - 37, The workflow’s commit-and-push step can race when multiple runs update the same branch at once, causing non-fast-forward push failures or conflicting commits. Add a concurrency group to the update-archive-index workflow so only one run executes at a time for this job, and update the commit/push logic to rebase or retry on push failure before pushing from the commit-and-push step. Use the existing workflow job and the “Commit and push if changed” step as the place to apply the serialization and stale-branch handling.
21-22: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueStatic analysis flags missing
persist-credentials: false, but it's likely intentional here.The zizmor
artipackedwarning applies generically, but this workflow needs the checkout-persisted token to performgit pushin the later "Commit and push if changed" step (Lines 32-37). Disablingpersist-credentialswould break that step. The job already scopes the token viapermissions: contents: write(Lines 18-19), which mitigates most of the residual risk. No change needed unless you want to switch to an explicit short-lived PAT for tighter control.🤖 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 @.github/workflows/update-archive-index.yml around lines 21 - 22, The warning on the Checkout Repository step is intentional because the later “Commit and push if changed” logic relies on the persisted token to run git push. Keep actions/checkout@v4 as-is in this workflow and do not add persist-credentials: false; the existing permissions: contents: write scope already covers the needed access. If you want stricter control, switch the push flow to an explicit short-lived PAT instead.Source: Linters/SAST tools
scripts/build-site-index.mjs (1)
135-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale comment contradicts the actual code.
The comment says rawUrl is "no longer include[d] ... directly in the record, because the frontend derives it dynamically to ensure fork-compatibility", yet the returned object literal below unconditionally includes both
sourceUrlandrawUrl(Lines 150-151). This is confusing for future maintainers and misrepresents the actual contract consumed downstream (docs/assets/main.jsusesrecord.sourceUrl/record.rawUrldirectly for its "Open on GitHub" and "View raw" links).✏️ Suggested comment fix
- // We no longer include rawUrl directly in the record, because the frontend - // derives it dynamically to ensure fork-compatibility. We include it here - // for legacy or API access if needed, but the frontend will compute its own. + // sourceUrl/rawUrl are computed at generation time using GITHUB_OWNER/GITHUB_REPO + // (env-provided in CI). The frontend also derives its own raw-content URL at + // runtime for fork compatibility (see docs/assets/data.js#fetchFileContent), + // but these fields are still used directly for the "Open on GitHub"/"View raw" links.🤖 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/build-site-index.mjs` around lines 135 - 153, The comment above the returned record is stale and contradicts the actual object shape, since this path still always includes both sourceUrl and rawUrl. Update or remove the misleading note near the record-building logic in the site index generator so it matches the current contract used by downstream consumers, and make sure any mention of frontend-derived URLs is consistent with how docs/assets/main.js reads record.sourceUrl and record.rawUrl.docs/assets/main.js (2)
318-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReimplements
findFileinstead of using the exported helper.
state.index.files.find((f) => f.path === path)appears three times in this file, duplicating thefindFile(files, path)helper already exported from docs/assets/data.js (Line 90-92), which is currently unused. Consolidating onto the shared helper reduces duplication and keeps a single source of truth for this lookup.Also applies to: 403-404, 546-546
🤖 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 `@docs/assets/main.js` around lines 318 - 321, The file lookup is duplicated in this module instead of using the shared helper. Replace the repeated `state.index.files.find((f) => f.path === path)` lookups in `main.js` with the exported `findFile(files, path)` helper from `docs/assets/data.js`, and update the related call sites in the file so all path matching goes through that single helper.
583-585: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates
router.js's exportednavigatefunction.
navigateToreimplementsnavigatefrom docs/assets/router.js (Line 37-39) verbatim, butnavigateisn't imported here. Prefer importing and reusing it instead of maintaining two copies of the same hash-navigation logic.🤖 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 `@docs/assets/main.js` around lines 583 - 585, The hash-navigation helper in navigateTo duplicates the logic already exported by navigate in router.js, so replace the local implementation with a reuse of that shared function. Update main.js to import navigate from docs/assets/router.js and use it at the current navigateTo call sites, keeping the behavior centralized instead of maintaining two copies of the same hash update logic.docs/index.html (1)
13-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
markeddependency is unpinned in the import map.
https://cdn.jsdelivr.net/npm/marked/lib/marked.esm.jsresolves to whatever the latest publishedmarkedrelease is at request time. Since the app relies on specific renderer-override behavior in docs/assets/markdown.js (Line 18-29), an unannounced major-version bump from the CDN could silently break rendering (or change escaping/sanitization semantics) in production with no corresponding code change or CI signal.♻️ Proposed fix: pin an explicit version
"imports": { - "marked": "https://cdn.jsdelivr.net/npm/marked/lib/marked.esm.js" + "marked": "https://cdn.jsdelivr.net/npm/marked@<pinned-version>/lib/marked.esm.js" }🤖 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 `@docs/index.html` around lines 13 - 19, The import map in the docs entrypoint uses an unpinned marked CDN URL, so lock it to a specific marked version to avoid unexpected renderer behavior changes. Update the import map entry for marked in docs/index.html to reference a versioned CDN path, and keep the docs/assets/markdown.js overrides aligned with that pinned release so rendering stays stable across deployments.docs/assets/data.js (1)
12-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant branch + hidden module-state coupling in
fetchFileContent.Two issues here:
- The
localhost/127.0.0.1branch (line 19-20) builds the exact same URL as theelsebranch's default path (lines 22-23, 28) when the hostname isn't.github.io. The special case is unnecessary duplication.fetchFileContentsilently depends on the module-levelcachedIndexpopulated byloadIndex()having already run. If this function were ever called beforeloadIndex()(e.g. future refactor, different call order),cachedIndexwould benulland this throws a rawTypeErroroncachedIndex.repo.♻️ Proposed simplification
export async function fetchFileContent(record) { const cached = contentCache.get(record.path); if (cached !== undefined) return cached; - + const encodedPath = record.path.split("/").map(encodeURIComponent).join("/"); - let rawUrl; - - if (window.location.hostname === "localhost" || window.location.hostname === "127.0.0.1") { - rawUrl = `https://raw.githubusercontent.com/${cachedIndex.repo.owner}/${cachedIndex.repo.name}/${cachedIndex.repo.branch}/${encodedPath}`; - } else { - let owner = cachedIndex.repo.owner; - let repo = cachedIndex.repo.name; - if (window.location.hostname.endsWith(".github.io")) { - owner = window.location.hostname.split(".")[0]; - repo = window.location.pathname.split("/")[1] || repo; - } - rawUrl = `https://raw.githubusercontent.com/${owner}/${repo}/${cachedIndex.repo.branch}/${encodedPath}`; - } + const index = await loadIndex(); + let owner = index.repo.owner; + let repo = index.repo.name; + if (window.location.hostname.endsWith(".github.io")) { + owner = window.location.hostname.split(".")[0]; + repo = window.location.pathname.split("/")[1] || repo; + } + const rawUrl = `https://raw.githubusercontent.com/${owner}/${repo}/${index.repo.branch}/${encodedPath}`;🤖 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 `@docs/assets/data.js` around lines 12 - 36, Simplify fetchFileContent by removing the redundant localhost/127.0.0.1 branch and centralizing rawUrl construction in one path, with only the .github.io owner/repo override logic in the special case. Also guard the module-level cachedIndex dependency before using cachedIndex.repo in fetchFileContent, so callers get a clear error or the index is loaded first instead of a raw TypeError. Keep the fix localized to fetchFileContent and the cachedIndex/loadIndex flow in docs/assets/data.js.
🤖 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 `@docs/assets/data.js`:
- Line 6: The fetch in the data loading path uses a caching mode that can keep
serving an outdated archive index indefinitely. Update the `fetch` call in
`data.js` that loads `data/index.json` to use a non-stale approach (for example,
revalidate or avoid forced caching) so regenerated index data from the archive
update workflow is actually picked up. Keep the fix localized to the
`fetch("data/index.json", ...)` call and preserve the existing data-loading
flow.
In `@docs/assets/main.js`:
- Around line 532-540: The copy action in the clipboard handler can reject
without being handled, causing an unhandled promise rejection and no user
feedback. Update the copy flow in the click handler that uses
navigator.clipboard.writeText and showToast so failures are caught and reported,
using a .catch or async/await with try/catch around the copy operation. Keep the
success toast on success and add a failure path that informs the user when the
copy does not complete.
- Around line 12-34: The init() flow in main.js leaves the UI stuck on the
loading placeholder when loadIndex() rejects, so add explicit error handling
around the await loadIndex() call and the init() startup path. Update init() to
catch failures from loadIndex(), surface a user-facing error state/message
instead of rendering the normal tree, and avoid continuing into buildTree/render
when the index cannot be loaded; also make sure the top-level init() invocation
uses a catch path so the rejection is handled cleanly.
In `@docs/assets/markdown.js`:
- Around line 18-33: The Markdown renderer in renderMarkdown should sanitize
link URLs, not just escape HTML in html(). Update the Marked configuration in
markdown.js so link tokens are validated against an allowlist of safe protocols
(or run the final rendered output through a sanitizer like DOMPurify) before it
is returned for innerHTML usage in main.js. Make sure the fix covers javascript:
and other unsafe URL schemes while preserving normal http/https and relative
links.
In `@docs/assets/router.js`:
- Around line 1-22: parseHash currently calls decodeSegments and
decodeURIComponent directly, so malformed percent-encoding can throw and stop
render(). Update parseHash (and, if helpful, decodeSegments) to catch
URIError/decoding failures for the "b", "f", and "s" cases and return the home
view as a safe fallback. Keep the behavior centered on parseHash so any bad hash
input is handled before it reaches the rest of the router flow.
In `@docs/DEPLOYMENT.md`:
- Line 16: The GitHub Pages URL example in the deployment guide is hardcoded to
a specific repository name, so replace that path segment with a repo-agnostic
placeholder in the docs content. Update the example near the GitHub Pages URL
mention to use a generic repository placeholder or explicitly note the
repository-specific segment, keeping the guidance applicable to forks and future
renames.
---
Nitpick comments:
In @.github/workflows/update-archive-index.yml:
- Around line 32-37: The workflow’s commit-and-push step can race when multiple
runs update the same branch at once, causing non-fast-forward push failures or
conflicting commits. Add a concurrency group to the update-archive-index
workflow so only one run executes at a time for this job, and update the
commit/push logic to rebase or retry on push failure before pushing from the
commit-and-push step. Use the existing workflow job and the “Commit and push if
changed” step as the place to apply the serialization and stale-branch handling.
- Around line 21-22: The warning on the Checkout Repository step is intentional
because the later “Commit and push if changed” logic relies on the persisted
token to run git push. Keep actions/checkout@v4 as-is in this workflow and do
not add persist-credentials: false; the existing permissions: contents: write
scope already covers the needed access. If you want stricter control, switch the
push flow to an explicit short-lived PAT instead.
In `@docs/assets/data.js`:
- Around line 12-36: Simplify fetchFileContent by removing the redundant
localhost/127.0.0.1 branch and centralizing rawUrl construction in one path,
with only the .github.io owner/repo override logic in the special case. Also
guard the module-level cachedIndex dependency before using cachedIndex.repo in
fetchFileContent, so callers get a clear error or the index is loaded first
instead of a raw TypeError. Keep the fix localized to fetchFileContent and the
cachedIndex/loadIndex flow in docs/assets/data.js.
In `@docs/assets/main.js`:
- Around line 318-321: The file lookup is duplicated in this module instead of
using the shared helper. Replace the repeated `state.index.files.find((f) =>
f.path === path)` lookups in `main.js` with the exported `findFile(files, path)`
helper from `docs/assets/data.js`, and update the related call sites in the file
so all path matching goes through that single helper.
- Around line 583-585: The hash-navigation helper in navigateTo duplicates the
logic already exported by navigate in router.js, so replace the local
implementation with a reuse of that shared function. Update main.js to import
navigate from docs/assets/router.js and use it at the current navigateTo call
sites, keeping the behavior centralized instead of maintaining two copies of the
same hash update logic.
In `@docs/index.html`:
- Around line 13-19: The import map in the docs entrypoint uses an unpinned
marked CDN URL, so lock it to a specific marked version to avoid unexpected
renderer behavior changes. Update the import map entry for marked in
docs/index.html to reference a versioned CDN path, and keep the
docs/assets/markdown.js overrides aligned with that pinned release so rendering
stays stable across deployments.
In `@scripts/build-site-index.mjs`:
- Around line 135-153: The comment above the returned record is stale and
contradicts the actual object shape, since this path still always includes both
sourceUrl and rawUrl. Update or remove the misleading note near the
record-building logic in the site index generator so it matches the current
contract used by downstream consumers, and make sure any mention of
frontend-derived URLs is consistent with how docs/assets/main.js reads
record.sourceUrl and record.rawUrl.
🪄 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: 8f765b2b-af85-40ef-bddd-c5051139823d
📒 Files selected for processing (14)
.github/workflows/update-archive-index.ymldocs/.nojekylldocs/DEPLOYMENT.mddocs/README.mddocs/assets/data.jsdocs/assets/main.jsdocs/assets/markdown.jsdocs/assets/palette.jsdocs/assets/router.jsdocs/assets/search.jsdocs/assets/style.cssdocs/data/index.jsondocs/index.htmlscripts/build-site-index.mjs
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 @.github/workflows/update-archive-index.yml:
- Around line 41-45: The shell step in the update-archive-index workflow is
interpolating github.ref directly into git pull --rebase, which can allow
template injection. Move the ref value into the step’s env block and reference
it as a shell variable inside the script, updating the git pull command in the
commit/pull/push block to use that variable instead of direct expression
expansion.
🪄 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: d827485e-399a-48c7-af59-5060311f74de
📒 Files selected for processing (8)
.github/workflows/update-archive-index.ymldocs/DEPLOYMENT.mddocs/assets/data.jsdocs/assets/main.jsdocs/assets/markdown.jsdocs/assets/router.jsdocs/index.htmlscripts/build-site-index.mjs
✅ Files skipped from review due to trivial changes (1)
- docs/DEPLOYMENT.md
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/index.html
- docs/assets/router.js
- docs/assets/markdown.js
- docs/assets/data.js
- scripts/build-site-index.mjs
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
1 participant
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.
This PR restructures the frontend architecture to enable a seamless, zero-configuration deployment to GitHub Pages. It completely eliminates the need for Node.js, npm builds, and local development toolchains.
The frontend is now a completely static application housed entirely within the docs/ directory, relying natively on Vanilla HTML, CSS, and ES Modules.
Key Changes
Zero-Build Architecture: Migrated the Vite/TypeScript frontend to standard Vanilla ES modules inside docs/assets/. The codebase can now be edited and instantly run in any browser using a basic local web server—no compilation required.
docs/ Pages Deployment: All frontend logic and assets have been moved to the docs/ folder. This allows the repository maintainer to simply go to Settings -> Pages -> Deploy from branch -> main -> /docs to turn on the website.
Fork-Resilient Data Loading: The UI now dynamically determines its repository coordinates (owner and repo) at runtime by reading window.location.hostname and window.location.pathname. This ensures that raw.githubusercontent.com URLs automatically resolve correctly, even if the PR is merged into a different fork (e.g., asgeirtj/system_prompts_leaks).
Automated Indexing (update-archive-index.yml): Added a lightweight GitHub Action. Whenever a new .md file is pushed to main, this action runs the generic index generator (scripts/build-site-index.mjs), commits the updated docs/data/index.json file back to the repository, and seamlessly triggers a GitHub Pages deployment so the site is always perfectly in sync with the repository.
Documentation & Config: Added a .nojekyll file to bypass Jekyll processing, and included README.md and DEPLOYMENT.md inside docs/ outlining how the frontend operates and how to enable it.
Why this matters
The repository can remain focused solely on being a system prompt archive. Maintainers do not have to worry about running frontend build scripts or resolving NPM dependencies, but we retain all the dynamic discovery capabilities of the original frontend implementation.
Summary by CodeRabbit