docs(brand): add brand foundation page on archon.diy#1745
Conversation
- Mount the canonical Archon brand sheet at `/brand/` in the docs site (Penpot-exported standalone HTML, top-right "Console →" cross-link surgically removed via a re-runnable patch script). - Add a Starlight overview page with a Quick reference (gradient, surface) and an embedded full brand sheet. - Sidebar gains a "🎨 Brand" entry between Roadmap and The Book of Archon. - Fix the dark-mode active sidebar link being unreadable (`color: var(--sl-color-white)`). - Require future UI changes to align with the brand foundation (new "UI and Visual Design" section in root CLAUDE.md). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new brand documentation section to the Archon docs site with UI contributor guidelines requiring brand-aligned design tokens, introduces comprehensive CSS theme tokens and global styling, creates reusable logo and form control components, and builds an interactive brand foundation showcase app with theme switching and sticky section navigation, all loaded via an HTML entrypoint using Babel-standalone JSX compilation. ChangesBrand Documentation and Foundation System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/docs-web/scripts/brand/_patch.ts (1)
12-13: 💤 Low valueWhitespace-sensitive string matching is fragile across re-exports.
HEADER_BLOCK_STARTandHEADER_BLOCK_ENDhard-code exact indentation (' <div ...',' </a>\n </div>'). A Penpot re-export that changes formatter settings, prettier width, or tab/space style will causeindexOfto return-1and the script to throw "already patched?" — which is misleading and will send the next maintainer down the wrong debugging path.Consider either matching with a tolerant regex (leading whitespace + structural anchors) or updating the "already patched?" message to acknowledge both possibilities so the failure mode points at the real cause.
♻️ Minimal message tweak
-if (startIdx < 0) throw new Error('header block start not found — already patched?'); +if (startIdx < 0) { + throw new Error( + 'header block start not found — already patched, or the Penpot export changed; ' + + 'rerun _find-console.ts and update HEADER_BLOCK_START / TARGET_UUID', + ); +}🤖 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 `@packages/docs-web/scripts/brand/_patch.ts` around lines 12 - 13, The hard-coded whitespace in HEADER_BLOCK_START and HEADER_BLOCK_END makes indexOf matching fragile; update the patch logic in this file to use tolerant matching (e.g., test with a regex that allows leading/trailing whitespace and matches the HTML structure) instead of strict string indexOf, or at minimum broaden the failure message that currently says "already patched?" to mention possible whitespace/formatting mismatches; refer to symbols HEADER_BLOCK_START, HEADER_BLOCK_END, and the indexOf-based check in this file and replace it with a regex-driven search (or change the error text) so formatting changes don't cause a misleading -1 match.packages/docs-web/scripts/brand/_dump.ts (1)
4-9: 💤 Low valueAdd a minimal usage check and clarify the magic line index.
When invoked with missing args,
readFileSync(undefined)throws a confusingERR_INVALID_ARG_TYPEinstead of a usage hint, and thei >= 180heuristic is unexplained and duplicated across the three sibling scripts. A short guard plus a one-line comment makes this one-shot helper friendlier without overengineering.♻️ Suggested tweak
-const file = readFileSync(process.argv[2], 'utf8'); -const uuid = process.argv[3]; -const outFile = process.argv[4]; - -const lines = file.split('\n'); -const manifestLine = lines.find((l, i) => i >= 180 && l.startsWith('{"')); +const [, , inPath, uuid, outFile] = process.argv; +if (!inPath || !uuid || !outFile) { + console.error('usage: bun _dump.ts <foundation.html> <uuid> <out>'); + process.exit(1); +} +const file = readFileSync(inPath, 'utf8'); +const lines = file.split('\n'); +// Manifest JSON is appended near the bottom of the Penpot export; skip the head +// HTML/JS so we do not match an earlier `{"` literal in inline scripts. +const manifestLine = lines.find((l, i) => i >= 180 && l.startsWith('{"'));🤖 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 `@packages/docs-web/scripts/brand/_dump.ts` around lines 4 - 9, Add a minimal usage guard and document the magic index: check process.argv[2], [3], [4] at the top and print a short usage message (e.g. expected: input-file uuid out-file) then exit if missing to avoid readFileSync(undefined) errors; add a one-line comment above the manifestLine declaration explaining the heuristic "i >= 180" (why we're looking past header lines) and keep the same lines.find usage (manifestLine) so behavior is unchanged.
🤖 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 `@packages/docs-web/scripts/brand/_patch.ts`:
- Around line 45-55: The post-patch check currently only logs the count from
patched.match(/Console →/g) (variable verify) but still proceeds to re-encode
and write the file; change this to enforce the invariant by throwing an error if
verify.length !== 1 before performing the gzip/Buffer re-encoding and the
manifest update (manifest[TARGET_UUID] = ...) and before
writeFileSync(lines.join('\n')) so the file is never overwritten in an
inconsistent state; include a clear message referencing the expected "Console →
occurrences: 1" and the actual count to aid debugging.
---
Nitpick comments:
In `@packages/docs-web/scripts/brand/_dump.ts`:
- Around line 4-9: Add a minimal usage guard and document the magic index: check
process.argv[2], [3], [4] at the top and print a short usage message (e.g.
expected: input-file uuid out-file) then exit if missing to avoid
readFileSync(undefined) errors; add a one-line comment above the manifestLine
declaration explaining the heuristic "i >= 180" (why we're looking past header
lines) and keep the same lines.find usage (manifestLine) so behavior is
unchanged.
In `@packages/docs-web/scripts/brand/_patch.ts`:
- Around line 12-13: The hard-coded whitespace in HEADER_BLOCK_START and
HEADER_BLOCK_END makes indexOf matching fragile; update the patch logic in this
file to use tolerant matching (e.g., test with a regex that allows
leading/trailing whitespace and matches the HTML structure) instead of strict
string indexOf, or at minimum broaden the failure message that currently says
"already patched?" to mention possible whitespace/formatting mismatches; refer
to symbols HEADER_BLOCK_START, HEADER_BLOCK_END, and the indexOf-based check in
this file and replace it with a regex-driven search (or change the error text)
so formatting changes don't cause a misleading -1 match.
🪄 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: f090bbde-c74e-4b44-896c-7682c02d7eaf
📒 Files selected for processing (9)
CLAUDE.mdpackages/docs-web/astro.config.mjspackages/docs-web/public/brand/foundation.htmlpackages/docs-web/scripts/brand/README.mdpackages/docs-web/scripts/brand/_dump.tspackages/docs-web/scripts/brand/_find-console.tspackages/docs-web/scripts/brand/_patch.tspackages/docs-web/src/content/docs/brand/index.mdpackages/docs-web/src/styles/custom.css
| const verify = patched.match(/Console →/g) ?? []; | ||
| console.log(`Console → occurrences: ${verify.length} (was 2, should be 1 after patch)`); | ||
|
|
||
| const reEncoded = entry.compressed | ||
| ? gzipSync(Buffer.from(patched, 'utf8')).toString('base64') | ||
| : Buffer.from(patched, 'utf8').toString('base64'); | ||
|
|
||
| manifest[TARGET_UUID] = { ...entry, data: reEncoded }; | ||
| lines[manifestLineIdx] = JSON.stringify(manifest); | ||
|
|
||
| writeFileSync(path, lines.join('\n')); |
There was a problem hiding this comment.
Enforce the post-patch invariant — don't just log it.
The README states this script is idempotent and "fails loudly if the source no longer matches", and the upstream indexOf guards (lines 37, 39) do fail fast. The post-condition on line 46, however, only prints the Console → count and then proceeds to overwrite foundation.html regardless. If a future Penpot export renames the pill or moves it outside the targeted block, the file silently gets a partial/wrong patch.
Throw when the count is not the expected 1 so the file is never written in an inconsistent state.
🛡️ Suggested fix
-const verify = patched.match(/Console →/g) ?? [];
-console.log(`Console → occurrences: ${verify.length} (was 2, should be 1 after patch)`);
+const verify = patched.match(/Console →/g) ?? [];
+if (verify.length !== 1) {
+ throw new Error(
+ `expected exactly 1 "Console →" occurrence after patch, found ${verify.length}`,
+ );
+}
+console.log(`Console → occurrences: ${verify.length} (was 2, now 1 — OK)`);As per coding guidelines: "Prefer throwing early with a clear error for unsupported or unsafe states — never silently swallow errors".
🤖 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 `@packages/docs-web/scripts/brand/_patch.ts` around lines 45 - 55, The
post-patch check currently only logs the count from patched.match(/Console →/g)
(variable verify) but still proceeds to re-encode and write the file; change
this to enforce the invariant by throwing an error if verify.length !== 1 before
performing the gzip/Buffer re-encoding and the manifest update
(manifest[TARGET_UUID] = ...) and before writeFileSync(lines.join('\n')) so the
file is never overwritten in an inconsistent state; include a clear message
referencing the expected "Console → occurrences: 1" and the actual count to aid
debugging.
…er scripts The brand sheet now ships as plain Penpot-exported source (Brand.html shell, brand-app.jsx, logo.jsx, tweaks-panel.jsx, standalone-tweaks-toggle.jsx, app.css, archon-logo.png) and is edited like any other code in the repo: open the JSX, change it, refresh the page. - public/brand/foundation.html now loads React + Babel from unpkg (with integrity hashes) and compiles the JSX in the browser. Adds one local override: hide the Penpot Tweaks toggle on the public site. - brand-app.jsx carries our single local delta: the top-right "Console →" cross-link is removed (the sibling Archon Console doc isn't published). - public/brand/README.md documents what each file owns and the local delta. - The 1.5 MB self-extracting bundle and the scripts/brand/ decoder pipeline (_find-console.ts, _dump.ts, _patch.ts) are deleted. Net: the repo loses ~1.5 MB of opaque base64 + 4 maintenance scripts; gains ~85 KB of editable source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review SummaryVerdict: ready-to-merge This PR adds a brand foundation page to the docs site (Archon brand guide with logo, colors, typography) and updates CLAUDE.md to codify brand alignment requirements for future UI work. Clean, well-scoped deliverable with no blocking issues. Minor / nice-to-have
Compliments
Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, comment-quality, docs-impact. |
2 participants
Summary
/consoleexperiment, marketing surfaces) drifts because there's no canonical reference for colors, gradient, logo, or typography.packages/web/design tokens, no changes to any application UI. This PR is docs-only plus a single-section CLAUDE.md rule. Token migration is deliberately left for follow-up.UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
astro.config.mjssidebar/brand/routecontent/docs/brand/index.md/brand/foundation.htmlscripts/brand/_patch.tspublic/brand/foundation.htmlCLAUDE.md(root)custom.css.sidebar-content a[aria-current='page']--sl-color-whiteLabel Snapshot
risk: lowsize: M(mostly the 1.5 MB self-contained brand HTML asset; logic surface is small)docsdocs:brandChange Metadata
docsdocsLinked Issue
No tracking issue — this introduces a new docs section requested in-session.
Validation Evidence (required)
/brand/index.htmland the new sidebar entry renders without errors.bun run testand fullvalidateintentionally skipped — this PR touches docs/CSS/MJS config only, no TypeScript runtime code, no schemas, no tests; husky's lint-staged ran prettier + eslint on the staged files and passed.bun run testandbun run type-checkskipped because no app TypeScript was modified. The helper scripts underscripts/brand/are standalone one-shot maintenance utilities (not in any tsconfig include path); they execute underbundirectly.Security Impact (required)
foundation.htmlis fully self-contained (Penpot bundles all assets as base64 in the manifest).foundation.htmlruns in the docs-site origin via an<iframe>from/brand/→/brand/foundation.html(same-origin). The bundler decodes base64 assets intoURL.createObjectURLblobs; no remote fetches.Compatibility / Migration
Human Verification (required)
bun --filter @archon/docs-web dev) renders/brand/with the iframe, Quick reference, and Open-in-new-tab link.Console →cross-link infoundation.htmlis gone after the patch; the body example variant ("Lockup with product" withsubtitle="Console") and the body CTA ("See the Console") were intentionally left alone.bun --filter @archon/docs-web buildproduces 73 pages (was 72) with no errors._patch.tsafter the patch fails loudly ("header block start not found — already patched?") — idempotent enough to detect re-application against a stale HTML.#0F1115stays legible).main, so the route will go live only after thisdevPR is merged and a release bringsdev→main).Side Effects / Blast Radius (required)
/brand/route, adds one sidebar entry, recomputes Pagefind search index.foundation.htmlincreases the docs site build output. It is loaded only when a visitor opens/brand/(lazy iframe).scripts/brand/README.mddocuments when to re-run the patch (after a fresh Penpot export). The patch fails loudly if its source markers no longer match — no silent drift.Rollback Plan (required)
git revert <merge-commit>ondev. Single-commit PR, no schema or runtime coupling, so rollback is contained to docs.main), or the/brand/route 404-ing.Risks and Mitigations
Risk:
foundation.htmlis a generated asset; future re-exports from Penpot will overwrite our top-right pill removal.scripts/brand/is documented and idempotent; the README spells out exactly when to re-run it. The patch script fails loudly if its expected markers are not found, so silent drift is impossible.Risk: The CLAUDE.md rule is broad ("all UI changes must align with the brand foundation") and could be misread as blocking small refactors.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation