Address various additional site nits discovered in testing#1576
Merged
anthonyiscoding merged 12 commits intomainfrom Apr 29, 2026
Merged
Address various additional site nits discovered in testing#1576anthonyiscoding merged 12 commits intomainfrom
anthonyiscoding merged 12 commits intomainfrom
Conversation
Restore cookie banner with iii_cookie_consent storage matching the prior
React site; load cr-relay signals only after Accept or when consent was
already accepted.
After successful Mailmodo POST, call signals.form({ email }) when consent
allows; add name=email on signup inputs.
Apply the same consent loader pattern on manifesto for consistency.
The CloudFront function now owns redirects and SPA fallback (see infra/terraform/website/cloudfront_functions/redirects.js); .github workflow syncs the static site directly to S3.
…route Replaces the hand-maintained /ai SPA route with two industry-standard plain-text artifacts produced from index.html at build time: - llms.txt — llms.txt-style snapshot (H1 + blockquote + sections) - AGENTS.md — agents.md-style product context for autonomous agents Both files share a canonical "AI overview" (problem framing, three primitives, two audiences, comparisons) and an extracted homepage copy fragment scraped from index.html via node-html-parser. AGENTS.md adds a wire-level appendix (ports, primitives, quickstart) for agent runtimes. Build pipeline: - pnpm --filter iii-website build now runs generate-llms-agents.ts and generate-sitemap.ts (writes /sitemap.xml directly). - sitemap now lists /llms.txt and /AGENTS.md as discoverable URLs. - deploy-website.yml installs deps, builds, and syncs llms.txt/AGENTS.md with no-cache headers (alongside *.html). - /ai route + prerender skip removed; CloudFront function now SPA-falls /ai back to /index.html and passes /AGENTS.md through unchanged. Tests: 4 new node:test cases for the generator shape; redirects test updated for /ai SPA fallback and /AGENTS.md passthrough. Generated llms.txt and AGENTS.md are gitignored going forward; current content is committed as the first generation baseline.
…stored Previously the banner script returned early when localStorage had 'accepted' or 'rejected', leaving the banner element hidden but still in the DOM (so its CSS reservation, ARIA, and event handlers stuck around). Now we remove the node entirely once a prior choice exists. Same fix applied to index.html and manifesto.html.
…ulas
Adds a small SVG complexity chart and MathML formula readout under the
hero pairwise-mesh visualization. Each stage now expresses its real cost
shape, not just a label:
- mesh: O(n(n-1)/2) — steep pairwise curve, dark
- actual: O(n(n-1)/(2·tolerance)) — damped curve at typical service
count, dark
- iii: O(0) — flat on the axis, accent
The chart is desktop-only inside the classic hero-viz (skipped in the
triptych design). Curves cross-fade by data-stage, and the formula is
rendered via inline <math> so the typography reads as math, not code.
Bumps .hero-viz-canvas aspect-ratio 1/1 → 5/4 to make room for the new
chart band, and sets a stable grid-template-rows on the desktop layout.
… overflow Three mobile UX fixes that shipped together because they share layout plumbing (min-width:0, overflow handling, scroll-snap): Console section (#cs-scroll): - Replace the per-panel "is-active fade" with a horizontal scroll-snap carousel that slides like the desktop translate track. - Stage now has a fixed clamped height; track is overflow-x:auto with scroll-snap-type:x mandatory; panels are flex 0 0 100%. - Inner card bodies (fn-list, states-table, traces, logs, config, etc.) scroll vertically within their panel; pre/code blocks pre-wrap. - New csScrollSync flag debounces JS-driven scrolls vs. user-driven scrolls so chip clicks and resize don't fight each other. Hello flow (#hello-mflow): - Wrap track + dots in a new .hello-mflow-body so the swipe handler can page the carousel from outside the inner code scroller. - Swipe handler reads touchstart/end on the body, distinguishes horizontal vs. vertical, and ignores swipes that start inside the track (so native scroll on the track still works). - Drop the obsolete .hello-mflow-slide overflow-x:hidden block; code panels are now overflow:hidden + pre-wrap (no horizontal trap). Footer (.foot): - Cap email/install font-size with clamp(11px,2.85vw,13px) and add min-width:0 + max-width:100% on grid children so long install commands and email inputs no longer push the row off-screen on narrow phones. - Tighten foot-cta padding and row gaps for sub-400px widths.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR introduces AI snapshot generation ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 5/8 reviews remaining, refill in 17 minutes and 22 seconds.Comment |
Contributor
Terraform plan —
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy-website.yml (1)
59-83:⚠️ Potential issue | 🟠 MajorDeploy
sitemap.xmlwith revalidated headers too.
website/scripts/generate-sitemap.tsnow regenerateswebsite/sitemap.xml, but this workflow still uploads it in the immutable pass. That means route/indexability changes can leave crawlers seeing a stale sitemap long after deploy.Suggested fix
- name: Sync static assets (long cache, immutable) run: | aws s3 sync website/ "s3://${{ vars.S3_BUCKET }}/" \ --delete \ --cache-control "public,max-age=31536000,immutable" \ --exclude "*.html" \ + --exclude "sitemap.xml" \ --exclude "llms.txt" \ --exclude "AGENTS.md" \ --exclude "node_modules/*" \ --exclude "package.json" \ --exclude "package-lock.json" \ @@ - name: Sync HTML and AI snapshot (no cache, must revalidate) run: | aws s3 sync website/ "s3://${{ vars.S3_BUCKET }}/" \ --delete \ --cache-control "public,max-age=0,must-revalidate" \ --exclude "*" \ --include "*.html" \ + --include "sitemap.xml" \ --include "llms.txt" \ --include "AGENTS.md"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-website.yml around lines 59 - 83, The sitemap.xml is being uploaded in the long-cache "Sync static assets (long cache, immutable)" step causing stale sitemap delivery; update the workflow so website/sitemap.xml is excluded from the immutable sync and instead included in the "Sync HTML and AI snapshot (no cache, must revalidate)" sync (modify the --exclude/--include lists accordingly), ensuring sitemap.xml uses --cache-control "public,max-age=0,must-revalidate" like "*.html" and is uploaded by the step named "Sync HTML and AI snapshot (no cache, must revalidate)" so crawlers always get the revalidated sitemap.
🧹 Nitpick comments (1)
website/scripts/generate-sitemap.ts (1)
1-6: Resolve the output path relative to the script, notprocess.cwd().This now works only as long as callers execute the script from the
website/package root. A direct invocation from the repo root would writesitemap.xmlto the wrong place. The sibling generator already uses a script-relative root, so this is worth keeping consistent here too.Suggested refactor
import fs from 'node:fs/promises' import path from 'node:path' +import { fileURLToPath } from 'node:url' import { INDEXABLE_ROUTES, SITE_ORIGIN } from './routes' -const OUT_PATH = path.resolve(process.cwd(), 'sitemap.xml') +const WEBSITE_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..') +const OUT_PATH = path.join(WEBSITE_ROOT, 'sitemap.xml')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/scripts/generate-sitemap.ts` around lines 1 - 6, The OUT_PATH uses process.cwd() which makes output dependent on where the script is run; change OUT_PATH to resolve relative to the script file instead. Import fileURLToPath from 'node:url', derive __dirname with fileURLToPath(import.meta.url) and path.dirname, then set OUT_PATH = path.resolve(__dirname, '..', 'sitemap.xml') (replace the current OUT_PATH declaration that uses process.cwd()). Ensure you update any imports needed (fileURLToPath) and keep the symbol name OUT_PATH unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/index.html`:
- Around line 23-55: The GTM bootstrap is being initialized unconditionally
while iiiLoadCommonRoomSignals is gated by STORAGE_KEY ('iii_cookie_consent'),
so update the GTM initialization to respect the same consent flow: either move
the GTM script injection into a function (e.g., window.iiiLoadGTM) and call it
only when localStorage.getItem(STORAGE_KEY) === 'accepted' (matching how
window.iiiLoadCommonRoomSignals is invoked), or wrap the existing GTM bootstrap
so it first checks STORAGE_KEY and aborts if not 'accepted'; ensure you
reference the same STORAGE_KEY/localStorage check and the GTM init location so
declining the banner actually prevents GTM from loading.
- Around line 7977-7984: The mobile resize/orientation path uses lastIdx
(updated only by desktop scroll) when computing syncIdx, causing mobile swipes
to be overridden; change the logic in the resize/orientation/applyMode() flow to
prefer mIdx when on mobile by computing syncIdx = (isMobileMode ? mIdx :
(lastIdx >= 0 ? lastIdx : mIdx)), then call applyPanelChrome(syncIdx) and set
track.scrollLeft = mIdx * w only when needed while preserving csScrollSync
semantics; update references to lastIdx, mIdx, applyPanelChrome, applyMode(),
csScrollSync, and track.scrollLeft so mobile resizes use mIdx and desktop still
uses lastIdx.
- Around line 7392-7419: The touchend handler currently computes nextIdx then
directly uses mTrack.scrollTo(...) and toggles mScrollSyncing, which leaves UI
state (pills/dots/label) stale when the swipe started outside the track; replace
the direct scroll with a call to applyFlowStep(nextIdx) so the flow state and UI
are updated consistently. Locate the touchend callback (symbols:
mBody.addEventListener('touchend', mSwipe0, mSwipeStartedOnTrack, stopFlow(),
FLOW_STEPS, mTrack, mScrollSyncing) and swap the scrollTo + setTimeout block for
a single applyFlowStep(nextIdx) call (remove or avoid duplicating
scroll/mScrollSyncing logic if applyFlowStep already handles it). Ensure mSwipe0
is still cleared and behavior for small/vertical swipes and mSwipeStartedOnTrack
remains unchanged.
In `@website/manifesto.html`:
- Around line 23-57: The page only gates Common Room via
window.iiiLoadCommonRoomSignals/STORAGE_KEY but leaves Google Tag Manager (GTM)
loading unconditionally (both the inline GTM script and the <noscript> iframe),
so "Decline" doesn't stop non-essential analytics; fix it by removing or
deferring the hardcoded GTM include and instead implement a gated loader (e.g.,
create window.iiiLoadGTM or loadGTM function) that checks the same STORAGE_KEY
('iii_cookie_consent') and only injects the GTM script and noscript iframe into
the DOM when consent === 'accepted'; also ensure any existing inline GTM
bootstrap (dataLayer pushes or global functions) are no-ops until loadGTM runs
so GTM cannot execute before consent.
---
Outside diff comments:
In @.github/workflows/deploy-website.yml:
- Around line 59-83: The sitemap.xml is being uploaded in the long-cache "Sync
static assets (long cache, immutable)" step causing stale sitemap delivery;
update the workflow so website/sitemap.xml is excluded from the immutable sync
and instead included in the "Sync HTML and AI snapshot (no cache, must
revalidate)" sync (modify the --exclude/--include lists accordingly), ensuring
sitemap.xml uses --cache-control "public,max-age=0,must-revalidate" like
"*.html" and is uploaded by the step named "Sync HTML and AI snapshot (no cache,
must revalidate)" so crawlers always get the revalidated sitemap.
---
Nitpick comments:
In `@website/scripts/generate-sitemap.ts`:
- Around line 1-6: The OUT_PATH uses process.cwd() which makes output dependent
on where the script is run; change OUT_PATH to resolve relative to the script
file instead. Import fileURLToPath from 'node:url', derive __dirname with
fileURLToPath(import.meta.url) and path.dirname, then set OUT_PATH =
path.resolve(__dirname, '..', 'sitemap.xml') (replace the current OUT_PATH
declaration that uses process.cwd()). Ensure you update any imports needed
(fileURLToPath) and keep the symbol name OUT_PATH unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4bc31bf8-847b-424a-9a69-bd9fbc7cb257
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.github/workflows/deploy-website.ymlinfra/terraform/website/cloudfront_functions/redirects.test.jswebsite/.gitignorewebsite/AGENTS.mdwebsite/index.htmlwebsite/llms.txtwebsite/manifesto.htmlwebsite/package.jsonwebsite/scripts/agents-appendix.mdwebsite/scripts/ai-overview.tswebsite/scripts/generate-llms-agents.test.tswebsite/scripts/generate-llms-agents.tswebsite/scripts/generate-sitemap.tswebsite/scripts/prerender-routes.tswebsite/scripts/routes.tswebsite/sitemap.xmlwebsite/vercel.json
💤 Files with no reviewable changes (3)
- website/scripts/prerender-routes.ts
- website/vercel.json
- website/scripts/routes.ts
…t tests into pnpm test - CloudFront viewer-request now rewrites /manifesto → /manifesto.html (Option A flat HTML), with matching test update. - Tighten AGENTS.md and llms.txt: drop hardcoded port list (point at config.yaml + docs), reframe Quickstart as Install/start, clarify that `iii trigger` is for manual checks (not app integration), simplify licensing line. - Run cloudfront_functions/*.test.js as part of `pnpm --filter iii-website test` so they stop being orphaned.
Contributor
Terraform plan —
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
infra/terraform/website/cloudfront_functions/redirects.js (1)
30-33: HoisthtmlPrettyout ofhandlerto avoid per-request allocation.
htmlPrettyis static and recreated on every invocation. Moving it to module scope is a small but clean optimization for hot-path request handling.Optional refactor
+var HTML_PRETTY = { + '/manifesto': '/manifesto.html', +}; + function handler(event) { @@ - var htmlPretty = { - '/manifesto': '/manifesto.html', - }; - var htmlTarget = htmlPretty[uri]; + var htmlTarget = HTML_PRETTY[uri];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/terraform/website/cloudfront_functions/redirects.js` around lines 30 - 33, Move the static htmlPretty map out of the per-request handler function to module scope so it isn't recreated on every invocation: declare htmlPretty at top-level (module scope) and then inside handler reference htmlPretty[uri] to compute htmlTarget as before; ensure no other references rely on closure state so only htmlPretty is relocated and htmlTarget/uri usage inside handler remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infra/terraform/website/cloudfront_functions/redirects.js`:
- Line 24: Implement a serializeQuerystring helper that iterates over
request.querystring (shape: {param: {value, multiValue?}}) and returns a
properly encoded "?a=1&b=2" string handling multiValue arrays and empty values;
replace the bare redirect in redirects.js (the host === 'www.iii.dev' branch
that calls redirect(`https://iii.dev${uri}`)) to append the serialized
querystring (e.g.,
redirect(`https://iii.dev${uri}${serializeQuerystring(request.querystring)}`));
add a unit test exercising a non-empty querystring (including multiValue and
empty param cases) to prevent regression.
In `@website/scripts/agents-appendix.md`:
- Around line 15-19: Update the "Function" primitive row in the agents appendix
so its "How an agent uses it" cell exactly matches the generated AGENTS.md:
replace the current sentence that starts "Invoke from another worker via your
language SDK..." with "Call via `iii.trigger(name, input)` from anywhere else on
the engine" (ensure the text includes the exact `iii.trigger(name, input)`
phrasing referenced).
---
Nitpick comments:
In `@infra/terraform/website/cloudfront_functions/redirects.js`:
- Around line 30-33: Move the static htmlPretty map out of the per-request
handler function to module scope so it isn't recreated on every invocation:
declare htmlPretty at top-level (module scope) and then inside handler reference
htmlPretty[uri] to compute htmlTarget as before; ensure no other references rely
on closure state so only htmlPretty is relocated and htmlTarget/uri usage inside
handler remains unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3b1b6811-2a62-43f2-94a3-dde2fcc1f34a
📒 Files selected for processing (6)
infra/terraform/website/cloudfront_functions/redirects.jsinfra/terraform/website/cloudfront_functions/redirects.test.jswebsite/AGENTS.mdwebsite/package.jsonwebsite/scripts/agents-appendix.mdwebsite/scripts/generate-llms-agents.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- infra/terraform/website/cloudfront_functions/redirects.test.js
- website/package.json
- website/scripts/generate-llms-agents.ts
The CloudFront Function dropped any inbound query params when redirecting www.iii.dev to iii.dev, breaking inbound links that carried tracking parameters or auth state. Add a serializeQuerystring helper that walks the {value, multiValue?} shape, re-encodes keys/values, and append it to the redirect target. Covered by new tests for multiValue, empty values, and the no-querystring case.
Contributor
Terraform plan —
|
Both files are produced by `pnpm --filter website build` via `scripts/generate-llms-agents.ts` and are already listed in `website/.gitignore`, but historical copies remained tracked and would churn on every developer's build (the generator stamps `Last updated: <today>`). Files stay on disk locally; the build will overwrite them.
Contributor
Terraform plan —
|
Adds two regressions on top of the www→apex querystring fix: - reserved chars (&, =, +, #) in values and spaces in keys must percent-encode so they cannot corrupt the 301 target, - the SPA-fallback path mutates uri but must leave request.querystring identical (CloudFront forwards it as-is); pin the no-op so a future refactor cannot silently drop it.
Contributor
Terraform plan —
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infra/terraform/website/cloudfront_functions/redirects.test.js (1)
124-133: Clarify SPA test intent and avoid identity-coupled assertion.The test name says “no rewrite” but
Line 132expects a rewrite to/index.html(it’s “no redirect,” not “no rewrite”). Also,Line 133uses reference equality forquerystring, which makes the test brittle to harmless refactors that clone objects while preserving values.Proposed test-tightening diff
-test('SPA fallback preserves querystring on the request object (no rewrite)', () => { +test('SPA fallback rewrites uri and preserves querystring values (no redirect)', () => { @@ - assert.equal(result.querystring, qs) + assert.deepEqual(result.querystring, qs) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infra/terraform/website/cloudfront_functions/redirects.test.js` around lines 124 - 133, Rename or rephrase the test description to reflect "no redirect" (not "no rewrite") and update assertions in the SPA fallback test: keep calling buildEvent('/some/route', 'iii.dev', qs) and handler(event), assert that isRedirect(result) is false and result.uri === '/index.html', but replace the identity check assert.equal(result.querystring, qs) with a value equality check (e.g., assert.deepEqual or assert.deepStrictEqual) so the test verifies preserved querystring values rather than object identity; reference symbols: test name, handler, isRedirect, buildEvent, result.querystring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infra/terraform/website/cloudfront_functions/redirects.test.js`:
- Around line 124-133: Rename or rephrase the test description to reflect "no
redirect" (not "no rewrite") and update assertions in the SPA fallback test:
keep calling buildEvent('/some/route', 'iii.dev', qs) and handler(event), assert
that isRedirect(result) is false and result.uri === '/index.html', but replace
the identity check assert.equal(result.querystring, qs) with a value equality
check (e.g., assert.deepEqual or assert.deepStrictEqual) so the test verifies
preserved querystring values rather than object identity; reference symbols:
test name, handler, isRedirect, buildEvent, result.querystring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bd671b73-ea63-4264-9e49-8771480565a4
📒 Files selected for processing (3)
infra/terraform/website/cloudfront_functions/redirects.jsinfra/terraform/website/cloudfront_functions/redirects.test.jswebsite/llms.txt
💤 Files with no reviewable changes (1)
- website/llms.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- infra/terraform/website/cloudfront_functions/redirects.js
4 tasks
ytallo
added a commit
that referenced
this pull request
Apr 29, 2026
Two changes: 1. Strip stale `# Leave false until Phase 4 cutover…` comments from manage_apex_records / manage_www_records — the variable description field already says what they do, and the cutover narrative they preserved is no longer load-bearing. 2. Add a tf-apply pipeline (`.github/workflows/tf-apply.yml`) so changes under `infra/terraform/website/` actually deploy on merge to main. Previously only `tf-plan.yml` ran on PRs and applies were manual, which is how the cleanUrls fix sat unapplied for hours after #1576 merged. - New IAM role `iii-website-prod-github-tf-apply` (AdministratorAccess, trust narrowly scoped to a new `iii-website-prod-tf-apply` env so repo settings can require reviewers without gating routine S3 deploys). - Workflow runs on push to main + workflow_dispatch, uses concurrency `tf-apply-website` to serialize applies, captures output to the job summary. Bootstrap (one-time, manual): AWS_PROFILE=motia-prod terraform apply → grab `github_tf_apply_role_arn` from outputs → set repo secret `AWS_TF_APPLY_ROLE_ARN` → create `iii-website-prod-tf-apply` GitHub environment with required reviewers
4 tasks
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.
Summary by CodeRabbit
New Features
Improvements