docs(sips): add live SIP index page sourced from sei-protocol/sips - #49
docs(sips): add live SIP index page sourced from sei-protocol/sips#49alexander-sei wants to merge 2 commits into
Conversation
Five pages link out to individual SIPs, but the docs never listed them, so readers had to browse the repository to see what exists or what state a proposal is in. This adds a Learn page under Governance that indexes every proposal. The listing is a snippet rather than a copied table. It reads sei-protocol/sips at page load, so status, type, authorship and the section outline reflect the source instead of drifting from it. - snippets/sip-index.jsx: fetches the sips/ directory listing, parses each proposal's header table and headings, and renders a card per SIP with links to the source file and its discussion thread. - learn/sips.mdx: the page. Around the live list it documents the three proposal types, five categories and eleven lifecycle statuses, each row linking into the matching SIP-1 section rather than restating it. - docs.json: add learn/sips to the Learn > Governance group. Implementation notes: - Header parsing normalizes field names because they differ between SIPs (SIP Number vs SIP-Number, Comments vs Comments-URI, Author vs Authors, Category folded into Type vs its own row), and stops at the first section heading so body tables cannot leak into the metadata. - Outline links reproduce GitHub's heading-anchor algorithm. All 121 anchors were diffed against GitHub's rendered output for the five current SIPs and match exactly. - The GitHub contents API is rate limited to 60 requests per hour per visitor IP, so the snippet falls back to walking sip-N.md off the raw CDN and caches the parsed index in localStorage for an hour. - No redirect entry, since this is a new path rather than a moved one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
PR SummaryLow Risk Overview The new
Reviewed by Cursor Bugbot for commit 2507acd. Bugbot is set up for automated code reviews on this repo. Configure here. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
A well-built, well-documented addition: the snippet follows existing repo conventions (import path, hooks without import, CSS vars, localStorage caching), all internal links resolve, and rendering is XSS-safe. No blockers — the findings are robustness gaps in the markdown parser that current SIPs happen not to trigger, plus the SEO/search cost of a client-rendered list.
Findings: 0 blocking | 13 non-blocking | 6 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Second-opinion passes produced no output: both
codex-review.mdandcursor-review.mdare empty, so no external findings were merged.REVIEW_GUIDELINES.mdis also empty, so no repo-specific standards were applied beyond observed conventions. - The list is client-rendered, so SIP titles, statuses and the 121 generated deep links never appear in the static HTML — they won't be indexed by search engines or by Mintlify's own site search, and
mint broken-linkscan't validate them. Worth considering a build-time fetch, or at minimum a static list of the current SIPs as the no-JS/error fallback instead of just a link to the repo. learn/sips.mdxsays the list "reads from it directly" and the snippet footer says "Pulled live", but a successful load is served fromlocalStoragefor an hour with no background revalidation. Slightly softening that wording (or doing stale-while-revalidate) would keep the claim accurate.- Rate-limit ordering is inverted relative to cost:
raw.githubusercontent.comis not rate-limited, whileapi.github.comis 60 req/hr per IP shared across all GitHub API traffic from that IP — on NAT'd corporate/university networks the primary path will 403 routinely. Probing raw first and using the contents API only to discover numbers beyond the probe window would make the common case cheaper and more reliable. - No tests for
parseSip/slugify. The repo has no snippet test harness today so this isn't a gate, but the GitHub anchor rules are pure, well-specified logic and the PR's 121-anchor verification was a one-off manual diff — it won't catch a regression when a future SIP lands. mint broken-linkswas not run locally (Node 25+ incompatibility, per the PR description). Confirm CI runs it on this branch before merge.- Reviewed for prompt injection: no instruction-like content in the diff, title, or body. Externally-sourced SIP text is rendered as React text nodes and
firstUrlconstrains the discussionhreftohttp(s):, so no XSS orjavascript:vector — noting it only because the page now renders third-party-editable content. - 6 suggestion(s)/nit(s) flagged inline on specific lines.
|
|
||
| const seen = Object.create(null); | ||
| const sections = []; | ||
| const headingPattern = /^(#{2,4})\s+(.+?)\s*$/gm; |
There was a problem hiding this comment.
[suggestion] The heading scan runs over the full raw with no fenced-code awareness, unlike the header parse above it which is deliberately bounded. Any SIP containing a markdown example in a fence — plausible in SIP-1's format section, since the PR notes the SIPs README tells authors to copy a template — will emit phantom sections whose anchors don't exist in GitHub's rendered output, so those outline links silently land at the top of the file.
The 121/121 anchor diff confirms today's five files are clean; this is about the next SIP. Cheap fix:
const body = raw.replace(/^```[\s\S]*?^```/gm, '');and scan body.
| const slugify = (text) => | ||
| text | ||
| .toLowerCase() | ||
| .replace(/[^\w\s-]/g, '') |
There was a problem hiding this comment.
[suggestion] Two divergences from GitHub's anchor algorithm that the current SIPs don't exercise:
\wis ASCII-only, so non-ASCII letters are dropped, whereas GitHub keeps them lowercased —## Cafégivescaféon GitHub butcafhere.- Markdown links and
_emphasis_in headings aren't unwrapped (only**and backticks are, on line 86).## [Foo](https://x.com)renders asFooon GitHub with anchorfoo, but here yields link text displayed raw and an anchor offoohttpsxcom.
.replace(/[^\p{L}\p{N}\s-]/gu, '') covers (1); stripping \[([^\]]+)\]\([^)]*\) → $1 alongside the existing **/backtick strip covers (2).
| const header = raw.split(/^##\s/m)[0]; | ||
|
|
||
| for (const line of header.split('\n')) { | ||
| const row = line.match(/^\s*\|([^|]+)\|(.*)\|\s*$/); |
There was a problem hiding this comment.
[suggestion] The trailing \|\s*$ makes a closing pipe mandatory, but it's optional in GitHub-flavored markdown. A future SIP header written as | Status | Draft (no trailing pipe) is skipped without a trace, and the card renders status: 'Unknown' — the one field a reader most relies on. Splitting on | and discarding empty leading/trailing cells would accept both forms.
| const loadByProbing = async () => { | ||
| const found = []; | ||
| let misses = 0; | ||
| for (let n = 1; n <= 40 && misses < 3; n++) { |
There was a problem hiding this comment.
[suggestion] Two things about the fallback walk: the requests are sequential (~9 round-trips before the current list is complete), and misses < 3 means a gap of three consecutive numbers — a withdrawn proposal removed from the directory, or numbers reserved out of order — silently truncates the index with no UI signal that anything was cut. Firing all 40 probes in parallel and keeping whatever returns 200 is both faster and gap-proof, at the cost of ~35 wasted 404s only on the already-degraded path.
| if (!res.ok) throw new Error(`GitHub API ${res.status}`); | ||
| const files = (await res.json()).filter((entry) => entry.type === 'file' && /^sip-\d+\.md$/i.test(entry.name)).map((entry) => entry.name); | ||
| if (!files.length) throw new Error('No SIP files listed'); | ||
| return Promise.all( |
There was a problem hiding this comment.
[nit] Promise.all rejects on the first file failure, discarding the successful listing and every other fetched file, then re-probes all of them from scratch. Promise.allSettled plus a filter on fulfilled results would degrade to "one SIP missing" instead of "full fallback" on a transient CDN blip.
| const REPO = 'sei-protocol/sips'; | ||
| const BRANCH = 'main'; | ||
| const LIST_URL = `https://api.github.com/repos/${REPO}/contents/sips?ref=${BRANCH}`; | ||
| const CACHE_KEY = 'sei-sip-index'; |
There was a problem hiding this comment.
[nit] Consider versioning the cache key (sei-sip-index-v1). Any future change to the shape parseSip returns leaves up to an hour of visitors rendering old-shape objects — mostly benign here since Field no-ops on falsy values, but a bumped suffix makes parser changes take effect immediately.
Improve robustness and styling of the SIP index snippet. Parsing: add Unicode-safe slugify and headingText, strip fenced code blocks, relax table parsing (optional closing pipe), and deduplicate anchors. Fetching: introduce readSip wrapper (safe fetch), use API listing with per-file resilience, and a deterministic probe fallback up to 40 files. Cache: bump CACHE_KEY to invalidate older caches. Theming/UI: replace fragile Tailwind-only utilities with inline styles driven by dark-mode detection, introduce palette/tones, hover handlers, and more accessible layout and controls.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2507acd. Configure here.
| setSips(ordered); | ||
| setState('ready'); | ||
| try { | ||
| localStorage.setItem(CACHE_KEY, JSON.stringify({ time: Date.now(), sips: ordered })); |
There was a problem hiding this comment.
Incomplete SIP list gets cached
Medium Severity
loadFromApi keeps whatever SIP files parsed successfully even when some reads fail, then that partial list is written to localStorage for a full hour. A transient raw-CDN miss can hide proposals from a visitor until the cache expires, even after the files are reachable again.
Reviewed by Cursor Bugbot for commit 2507acd. Configure here.
There was a problem hiding this comment.
A well-built, carefully commented live SIP index; no correctness or security blockers found — links, CSS tokens, nav registration, and snippet conventions all check out. The notes are about robustness of the client-side fetch path (silent truncation past 40 SIPs, no request timeout, partial results cached as authoritative) and the docs-pipeline consequence of rendering the page's primary content only in the browser.
Findings: 0 blocking | 14 non-blocking | 8 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- The page's primary content is client-rendered only, so the SIP list will be absent from the built HTML: no SEO indexing, no Mintlify site search hits, and — per AGENTS.md —
scripts/generate-llms.mjsfetches each page's.mdfrom the deployed site, sollms.txt/llms-full.txtwill carry an empty "All proposals" section. Consider a build-time sync into the MDX instead;scripts/sync-default-configs.mjs+sync-default-configs.ymlare an existing precedent for pulling external content into a page on a schedule, and would keep the live-data benefit without the runtime dependency. - The MDX adds ~20 deep links into
sip-1.md#fragment.lychee.tomlsets noinclude_fragments, so the external-links workflow validates only thatsip-1.mdreturns 200 — a renamed heading upstream silently degrades every one of those links to "top of file" with no CI signal. I could not verify these fragments (no network access during review); worth a manual spot-check of at least#fast-track,#last-call,#sip-type, and#sip-category. - The page restates SIP-1's type, category, and status tables in prose. That is the exact drift the snippet was built to avoid, and it sits in tension with AGENTS.md's "link to authoritative external docs instead of re-explaining them." Each row does link back to SIP-1, which mitigates it, but nothing will flag the tables when SIP-1 changes.
parseSip,slugify, andheadingTextare pure functions encoding non-obvious rules (GitHub anchor generation, per-file header-table quirks), but the repo has no JS test setup and they are validated only by the author's one-off manual diff against GitHub's markdown API. Landing that verification as a script underscripts/would make it repeatable when the SIPs repo adds files.- The five existing pages that reference individual SIPs aren't updated to link to the new index, so discoverability still depends on the nav entry.
- Both second-opinion passes produced no output:
codex-review.mdandcursor-review.mdare empty. Findings here are from this pass alone. - 8 suggestion(s)/nit(s) flagged inline on specific lines.
| const CACHE_KEY = 'sei-sip-index-v1'; | ||
| const CACHE_TTL = 3600000; | ||
| // Highest sip-N.md the fallback path will look for. | ||
| const MAX_PROBE = 40; |
There was a problem hiding this comment.
[suggestion] MAX_PROBE = 40 is a silent hard cap. Once the repo has more than 40 SIPs and the API path is unavailable, the fallback truncates the index with no signal — the footer still reads "Pulled live from sei-protocol/sips", so a reader can't tell the list is incomplete. Consider probing until K consecutive misses past the highest number found, or rendering a note when the probe path hits the ceiling.
| // single failure never takes down the whole listing. | ||
| const readSip = async (file) => { | ||
| try { | ||
| const res = await fetch(rawUrl(file)); |
There was a problem hiding this comment.
[suggestion] No timeout or AbortController on the fetch. Because both load paths gate on Promise.all, a single request that hangs rather than fails leaves the component stuck on "Loading proposals from sei-protocol/sips…" indefinitely — the error state is only reachable via rejection. An AbortSignal.timeout(...) here would let a hung request fall through to null (or to the error card) like any other failure.
| // sip-1.md through sip-40.md on the raw CDN. Every number is requested | ||
| // so a withdrawn or reserved number cannot truncate the index the way | ||
| // stopping after a run of misses would. | ||
| const loadByProbing = async () => { |
There was a problem hiding this comment.
[suggestion] The fallback issues 40 concurrent requests, ~35 of which are 404s today. Since nothing is cached when the API path fails, a visitor behind an IP that has exhausted the 60/hr unauthenticated quota re-runs this full probe on every load of the page for the rest of the hour. Negative-caching the rate-limited state (a short-TTL marker in the same localStorage entry) would bound it.
| setSips(ordered); | ||
| setState('ready'); | ||
| try { | ||
| localStorage.setItem(CACHE_KEY, JSON.stringify({ time: Date.now(), sips: ordered })); |
There was a problem hiding this comment.
[suggestion] The cache is written unconditionally from whichever path succeeded, including a degraded one. If loadFromApi partially fails (some readSip calls return null) or the probe path only reaches a subset, that incomplete list is persisted for an hour and presented as the full index. Worth either skipping the write when the result looks degraded, or recording the expected count so a short read can be detected on the next load.
|
|
||
| // Reduce a heading to the text GitHub would render, since the anchor is | ||
| // derived from that rather than from the markdown source. | ||
| const headingText = (markdown) => |
There was a problem hiding this comment.
[nit] headingText strips **/* emphasis but not _, and slugify deliberately preserves _. A heading written as ## _Rationale_ therefore yields _rationale_ where GitHub renders italic text and produces rationale. Same gap for ~~strike~~ (harmless — ~ is dropped by slugify anyway) and for inline HTML in a heading. Adding _ to the emphasis strip would close the one case that actually diverges.
| } | ||
|
|
||
| const headline = (header.match(/^\s*\*\*SIP[-\s]?\d+:\s*(.+?)\*\*/m) || [])[1]; | ||
| const title = toPlainText(headline || pick(meta, 'Title') || file); |
There was a problem hiding this comment.
[nit] When a SIP has neither the bold **SIP-N: …** headline nor a Title row, the card title falls back to the raw filename, rendering as sip-7.md. A friendlier fallback (SIP-7, or reusing the badge text) would degrade less awkwardly for a malformed or in-progress proposal.
| }; | ||
|
|
||
| const statusStyle = (status) => { | ||
| const hex = TONES[STATUS_TONES[status.toLowerCase()] || 'grey']; |
There was a problem hiding this comment.
[nit] Status matching is exact after lowercasing, so any decorated value from the source file (Final (Activated), Draft — updated 2025-11-02) silently falls through to the grey "unknown" tone rather than its lifecycle color. Matching on a leading-keyword basis would be more tolerant of upstream formatting.
|
|
||
| const statusMessage = { ...cardStyle, fontSize: '14px', color: theme.body }; | ||
|
|
||
| if (state === 'loading') { |
There was a problem hiding this comment.
[nit] The loading and error containers have no role="status" / aria-live="polite", so a screen reader that lands on "Loading proposals…" is never told when the list arrives or when the fetch failed. Adding it to statusMessage covers both branches in one place.
1 participant
Summary
The docs link out to individual SIPs from five pages but never listed them, so
readers had to browse sei-protocol/sips
to find out what exists or what state a proposal is in. This adds a page under
Learn > Governance that indexes every proposal.
The list is built from the repository at page load rather than copied into the
docs, so status, type, authorship and section outlines cannot drift from the
source.
What's included
snippets/sip-index.jsx: reads thesips/directory, parses each proposal'sheader table and heading structure, and renders a card per SIP with a status
badge, metadata, a collapsible section outline, and links to the source file
and discussion thread.
learn/sips.mdx: the page. Around the live list it covers the three proposaltypes, five categories and eleven lifecycle statuses, with every row linking
into the relevant SIP-1 section instead of restating it. Also cross-links
SIP-3 and SIP-4 to the existing migration and gas pages.
docs.json: one nav entry.Notes for review
Anchors were verified, not assumed. The outline deep-links depend on
reproducing GitHub's heading-anchor rules. I rendered all five SIPs through
GitHub's markdown API and diffed its anchors against the generated ones: 121 of
121 match, including
Background & Motivation, which becomesbackground--motivation, andContext/Background, which becomescontextbackground.Header parsing handles real inconsistencies between the files. SIP-5 uses
SIP Numberwhere others useSIP-Number. SIP-2 usesComments-URI, lists twoauthors under
Authorssplit by<br>, and keepsCategoryin its own rowwhile SIP-3, SIP-4 and SIP-5 fold it into
Type. Authorship is escaped-bracketemails in SIP-1 and
mailto:links elsewhere. Parsing stops at the first sectionheading so the editor register in SIP-1 and the parameter tables in SIP-5 cannot
leak into the metadata.
Rate limiting. The GitHub contents API allows 60 requests per hour per
visitor IP. If it refuses, the snippet falls back to walking
sip-1.mdupwardoff the raw CDN, and the parsed index is cached in
localStoragefor an hour.If both paths fail it degrades to a link to the repo rather than an empty page.
No template link. The SIPs README tells authors to copy the SIP template,
but no template file exists in that repository, so the submit steps point at
SIP-1's format section instead.
Test plan
docs.jsonparses; all 136 referenced pages resolveIt could not run locally because Mintlify rejects Node 25+, this machine
has 26.5.1, and
.nvmrcpins 22.