fix(site): refresh landing demos and repo stats#432
Open
TheEverests wants to merge 1 commit into
Open
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR refactors TouchAI's demo components to use lightweight Markdown and LaTeX renderers, adds scroll-driven UI state management across all embedded demo interfaces, implements analytics tracking on the homepage, and updates site configuration. It spans 27 files across rendering utilities, demo components, homepage overhauls, and configuration. ChangesDemo rendering and scroll-driven UI refactoring
Homepage analytics and demo progress management
Site configuration and documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/site/public/touchai-intro-en/touchai-components.html (1)
1651-1702:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the autoplay helper after the demo has already started.
typePromptThenSubmit()can still fire from the 900ms fallback after a user-triggered start, which clears the prompt and restarts the sequence mid-animation.Suggested fix
function showAnswer() { + window.clearTimeout(fallbackTimer); hasReceivedScrollProgress = false; setScrollDrivenState(false); clearPromptTyping(); @@ function typePromptThenSubmit() { + if ( + hasStarted || + document.body.classList.contains('is-answering') || + document.body.classList.contains('is-complete') + ) { + return; + } + + window.clearTimeout(fallbackTimer); hasReceivedScrollProgress = false; setScrollDrivenState(false); clearPromptTyping();Also applies to: 1757-1767
🤖 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 `@apps/site/public/touchai-intro-en/touchai-components.html` around lines 1651 - 1702, typePromptThenSubmit() can run after the demo has already started and restart the sequence mid-animation; guard its work by checking the same start state used elsewhere: at the top of typePromptThenSubmit() and immediately before enqueuing/performing each timed step (inside typeNextPromptCharacter and before the submitButton.click callback) verify hasStarted is false (or that document.body does not contain 'is-answering'); if the demo has started, cancel timers (promptTypingTimer) and return early. Reference symbols: typePromptThenSubmit, typeNextPromptCharacter, promptTypingTimer, submitButton.click, hasStarted, showAnswer, submitPrompt.apps/site/src/scripts/component-demo-runtime.ts (1)
595-614:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlush queued messages when the first
messagehandler is attached.Right now pending messages only drain at the end of
applyDocument(). If a demo registers itsmessagelistener fromDOMContentLoaded,load, or any later async path, queued progress/reset messages stay stranded forever.Suggested fix
windowHandlers.set( handler, wrappedMessageHandler as unknown as EventListenerOrEventListenerObject ); messageHandlers.add(wrappedMessageHandler); host.dataset.touchaiMessageHandlerCount = String(messageHandlers.size); + flushPendingMessages(host); return;🤖 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 `@apps/site/src/scripts/component-demo-runtime.ts` around lines 595 - 614, When attaching a new 'message' handler, detect if this is the first handler (i.e., messageHandlers.size === 0 before adding) and, if so, immediately flush the queued/pending messages the same way applyDocument() does so stranded progress/reset messages are delivered; implement by calling the existing queue-drain logic (reuse the function or block used in applyDocument() that processes the pending message queue) right after registering wrappedMessageHandler and updating host.dataset.touchaiMessageHandlerCount so queued messages are not left stranded.apps/site/public/touchai-intro/touchai-components.html (1)
1722-1770:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
hasReceivedScrollProgresswhen switching back to manual playback.The English intro file clears this flag in both manual entry paths, but this localized version does not. After a scrub-driven run, replay/manual start stays on the “external progress received” path.
Suggested fix
function showAnswer() { + hasReceivedScrollProgress = false; setScrollDrivenState(false); clearPromptTyping(); clearPromptInput(); @@ function typePromptThenSubmit() { + hasReceivedScrollProgress = false; setScrollDrivenState(false); clearPromptTyping(); input.value = '';🤖 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 `@apps/site/public/touchai-intro/touchai-components.html` around lines 1722 - 1770, Reset the hasReceivedScrollProgress flag when switching back to manual playback: set hasReceivedScrollProgress = false inside showAnswer() (so any manual showAnswer path clears external-scroll state) and also set hasReceivedScrollProgress = false at the start of typePromptThenSubmit() (immediately after setScrollDrivenState(false)) so the typed/manual entry path likewise clears the flag.
🤖 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 `@apps/site/public/demo-utils/touchai-lite-math.js`:
- Around line 2-6: escapeHtml currently returns raw strings when
window.TouchAILiteRenderer is unavailable, causing unescaped HTML to be injected
by renderMarkdownWithMath; replace the fallback with a proper escaping routine:
keep using renderer.escapeHtml(value) when renderer exists, otherwise implement
a local escape that converts &, <, >, ", ', and ` to their HTML entities; update
the escapeHtml function (and the identical fallback used around lines 229-231)
so renderMarkdownWithMath always receives safely escaped text.
In `@apps/site/public/feature-reminder-en/touchai-components.html`:
- Around line 1476-1482: The resetVisibleBlocks function's selector set is
missing tool card classes so .tool-call-list and .tool-call elements remain
visible after a reset; update the querySelectorAll call inside
resetVisibleBlocks to include '.tool-call-list' and '.tool-call' alongside the
existing selectors so those elements have 'is-visible' removed during resets.
In `@apps/site/public/feature-reminder/touchai-components.html`:
- Around line 1575-1595: The wheel handler forwardWheelToPage currently prevents
default and calls window.scrollBy, which only scrolls the iframe; instead
forward the computed delta (nextDeltaY) to the host so the embedding page can
handle scrolling—use window.parent.postMessage (or the existing postMessage
channel) to send a message containing { type: 'wheel-delta', deltaY: nextDeltaY
} when isScrollDriven && hasReceivedScrollProgress are true, keep the ctrl/meta
early-return logic, and remove or stop relying on window.scrollBy to avoid
trapping wheel input inside the iframe; reference forwardWheelToPage,
isScrollDriven, hasReceivedScrollProgress and the computed nextDeltaY when
implementing.
- Around line 496-500: The rule body.is-scroll-driven.is-complete .chat-panel is
overriding the mobile rule body.is-complete .chat-panel and reintroducing the
fixed 657px height on short viewports; limit this selector to desktop (e.g.,
wrap it in a desktop media query) or add a matching mobile override that
restores the mobile height behavior so short viewports don’t get the fixed
657px: update the CSS where body.is-scroll-driven.is-complete .chat-panel is
defined and either scope it with a desktop-only media query or add a
body.is-scroll-driven.is-complete .chat-panel mobile-specific rule that mirrors
the existing body.is-complete .chat-panel mobile override.
In `@apps/site/public/feature-solver-en/touchai-components.html`:
- Around line 1968-1988: The wheel handler forwardWheelToPage is trapping wheel
input because it calls event.preventDefault() and uses window.scrollBy() (which
only scrolls the iframe); instead forward the intended scroll to the host and
stop preventing the native event. Modify forwardWheelToPage to remove the
event.preventDefault() call that runs before handing the event off and replace
the iframe-only window.scrollBy(0, nextDeltaY) with a parent.postMessage({type:
'forward-wheel', deltaY: nextDeltaY}) (or, if the iframe and host are
same-origin, call parent.scrollBy(0, nextDeltaY)), keeping the existing guards
(isScrollDriven, hasReceivedScrollProgress) intact so only relevant wheel events
are forwarded.
- Line 2: The page declares lang="en" but still contains Chinese copy in
accessibility/fallback text (e.g., the strings near Line 982, Line 1023 and the
prerendered answer block starting at Line 1049), so either translate those
Chinese strings into English or revert lang to "zh" until translation is done;
specifically locate and update the Chinese text in the prerendered answer body,
any no-JS/failed-script fallback content, and ARIA/alt/label attributes (search
for the prerendered answer container and fallback block IDs/classes and the text
nodes around the noted lines) to their English equivalents so screen readers and
fallbacks get correct language context before keeping <html lang="en">.
- Around line 426-430: The selector body.is-scroll-driven.is-complete
.chat-panel is overriding the mobile-specific rule and forcing the fixed 657px
height on small screens; modify the CSS so the scroll-driven completion rule
does not apply the fixed height on narrow viewports—e.g., add a
media-query-specific override for body.is-scroll-driven.is-complete .chat-panel
(or alter that selector) to restore the mobile-safe properties (remove or unset
min-height/max-height/height or set them to auto/none) so the existing
body.is-complete .chat-panel mobile rule remains effective on small screens.
In `@apps/site/public/feature-solver/touchai-components.html`:
- Around line 1968-1988: The current forwardWheelToPage handler prevents default
and calls window.scrollBy, which only scrolls the iframe and swallows the wheel
for the parent; instead, stop preventing default and stop using window.scrollBy
inside the iframe — forward the computed delta to the host page (e.g., via
postMessage) so the parent can perform the scroll or allow the event to bubble
to the parent by removing event.preventDefault() before window.scrollBy; update
forwardWheelToPage to (1) keep the ctrl/meta early preventDefault branch, (2)
compute deltaMultiplier/nextDeltaY as now, and (3) either postMessage({type:
'wheel', deltaY: nextDeltaY}) to parent or simply remove the final
event.preventDefault() + window.scrollBy call so the host receives the wheel
event.
- Around line 426-430: The new selector body.is-scroll-driven.is-complete
.chat-panel is overriding the mobile rule body.is-complete .chat-panel and
forcing a fixed desktop height on short screens; update the CSS to preserve the
mobile override by adding a mobile-specific rule that targets the scroll-driven
completion state (e.g., inside the same small-screen media query add
body.is-scroll-driven.is-complete .chat-panel and set
min-height/max-height/height to the mobile variable or the previous values), or
increase specificity on the existing mobile selector so
body.is-scroll-driven.is-complete .chat-panel does not win.
In `@apps/site/src/content.config.ts`:
- Around line 1-3: The import block is out of order and failing
simple-import-sort; reorder the imports so external package imports are grouped
and sorted before local/alias imports—specifically ensure 'astro:content'
(defineCollection) and '`@astrojs/starlight/`...' imports (docsLoader, docsSchema)
follow the project's import-sort rules (or run the autofixer) so the imports for
defineCollection, docsLoader, and docsSchema are in the correct sorted groups
and order.
In `@apps/site/src/pages/index.astro`:
- Around line 37-52: The fetch blocks for repoApiUrl and repoOpenIssuesApiUrl
currently swallow all errors (empty catch) and ignore non-ok responses; update
the try/catch handlers around fetch(repoApiUrl, { headers }) and
fetch(repoOpenIssuesApiUrl, { headers }) to log diagnostics: on non-ok responses
log the URL and response.status/response.statusText, and in the catch blocks log
the thrown Error (including message/stack) along with which fetch (repoApiUrl or
repoOpenIssuesApiUrl) failed so build-time failures are visible (use your
existing logger or console.error).
- Around line 2685-2738: Add developer-facing privacy documentation and an
in-code comment noting legal/privacy implications when enabling analytics:
update the .env.example to document that PUBLIC_ANALYTICS_ENDPOINT enables
collection of referrer, viewport, and lang which may trigger GDPR/CCPA
disclosure/consent requirements, and add a clear comment immediately above the
sendAnalytics function (and mention analyticsEndpoint and trackPageView)
explaining what fields are collected, that tracking is opt-in via the env var,
and that deployers must ensure appropriate user notices/consent before enabling
it.
- Around line 1814-1817: The ANALYTICS_ENDPOINT environment value is embedded
directly into data-analytics-endpoint (and used by client fetches) without
validating its URL format; update the front-matter where ANALYTICS_ENDPOINT is
read to validate it (e.g., attempt new URL(ANALYTICS_ENDPOINT) or RegExp) and if
invalid set it to null/empty or a safe fallback, then only render the
data-analytics-endpoint attribute when the validated value exists; also update
the client-side code that reads dataset.analyticsEndpoint to check for a truthy,
valid URL before calling fetch to avoid silent failures.
- Around line 2907-2923: fetchRepoOverview currently adds a timestamp query via
withCacheBuster and also sets fetch option cache: 'no-store' — remove the
redundant timestamp approach: stop calling withCacheBuster(REPO_STATS_API_URL)
and withCacheBuster(REPO_OPEN_ISSUES_API_URL) and pass the plain
REPO_STATS_API_URL and REPO_OPEN_ISSUES_API_URL to fetch (leave cache:
'no-store' in the fetch options); update both fetch calls inside
fetchRepoOverview and, if withCacheBuster is unused elsewhere, remove the
withCacheBuster helper to keep the file clean.
---
Outside diff comments:
In `@apps/site/public/touchai-intro-en/touchai-components.html`:
- Around line 1651-1702: typePromptThenSubmit() can run after the demo has
already started and restart the sequence mid-animation; guard its work by
checking the same start state used elsewhere: at the top of
typePromptThenSubmit() and immediately before enqueuing/performing each timed
step (inside typeNextPromptCharacter and before the submitButton.click callback)
verify hasStarted is false (or that document.body does not contain
'is-answering'); if the demo has started, cancel timers (promptTypingTimer) and
return early. Reference symbols: typePromptThenSubmit, typeNextPromptCharacter,
promptTypingTimer, submitButton.click, hasStarted, showAnswer, submitPrompt.
In `@apps/site/public/touchai-intro/touchai-components.html`:
- Around line 1722-1770: Reset the hasReceivedScrollProgress flag when switching
back to manual playback: set hasReceivedScrollProgress = false inside
showAnswer() (so any manual showAnswer path clears external-scroll state) and
also set hasReceivedScrollProgress = false at the start of
typePromptThenSubmit() (immediately after setScrollDrivenState(false)) so the
typed/manual entry path likewise clears the flag.
In `@apps/site/src/scripts/component-demo-runtime.ts`:
- Around line 595-614: When attaching a new 'message' handler, detect if this is
the first handler (i.e., messageHandlers.size === 0 before adding) and, if so,
immediately flush the queued/pending messages the same way applyDocument() does
so stranded progress/reset messages are delivered; implement by calling the
existing queue-drain logic (reuse the function or block used in applyDocument()
that processes the pending message queue) right after registering
wrappedMessageHandler and updating host.dataset.touchaiMessageHandlerCount so
queued messages are not left stranded.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6da7003b-220a-4cf4-a68c-f1e64db562cc
⛔ Files ignored due to path filters (4)
apps/site/public/apple-touch-icon.pngis excluded by!**/*.pngapps/site/public/icon-192.pngis excluded by!**/*.pngapps/site/public/icon-512.pngis excluded by!**/*.pngapps/site/public/seo-card.pngis excluded by!**/*.png
📒 Files selected for processing (20)
apps/site/.env.exampleapps/site/astro.config.mjsapps/site/public/demo-utils/touchai-lite-math.jsapps/site/public/demo-utils/touchai-lite-renderer.jsapps/site/public/feature-reminder-en/touchai-components.htmlapps/site/public/feature-reminder/touchai-components.htmlapps/site/public/feature-solver-en/touchai-components.htmlapps/site/public/feature-solver/touchai-components.htmlapps/site/public/feature-work-organizer-en/touchai-components.htmlapps/site/public/feature-work-organizer/touchai-components.htmlapps/site/public/robots.txtapps/site/public/site.webmanifestapps/site/public/touchai-intro-en/touchai-components.htmlapps/site/public/touchai-intro/touchai-components.htmlapps/site/src/components/ComponentDemo.astroapps/site/src/content.config.tsapps/site/src/content/docs/404.mdapps/site/src/content/docs/getting-started.mdapps/site/src/pages/index.astroapps/site/src/scripts/component-demo-runtime.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (rust)
🧰 Additional context used
🪛 ESLint
apps/site/src/content.config.ts
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
apps/site/public/demo-utils/touchai-lite-renderer.js
[error] 26-29: Replace ⏎····················/\*\*([^*]+)\*\*/g,⏎····················'<strong>$1</strong>'⏎················ with /\*\*([^*]+)\*\*/g,·'<strong>$1</strong>'
(prettier/prettier)
[error] 51-53: Replace ⏎····················.map((item)·=>·
)⏎···················· with .map((item)·=>·)
(prettier/prettier)
apps/site/public/demo-utils/touchai-lite-math.js
[error] 29-29: 'error' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 29-29: Empty block statement.
(no-empty)
[error] 208-210: Replace ⏎············?·'formula-module'⏎··········· with ·?·'formula-module'
(prettier/prettier)
[error] 239-242: Replace ⏎····················new·RegExp(<p·class="paragraph-node">${token}
,·'g'),⏎····················replacement⏎················ with new·RegExp(<p·class="paragraph-node">${token},·'g'),·replacement
(prettier/prettier)
apps/site/src/scripts/component-demo-runtime.ts
[error] 527-528: Delete ⏎·······
(prettier/prettier)
🔇 Additional comments (36)
apps/site/astro.config.mjs (1)
13-13: LGTM!apps/site/src/content.config.ts (1)
5-10: LGTM!apps/site/public/robots.txt (1)
1-4: LGTM!apps/site/public/site.webmanifest (1)
1-29: LGTM!apps/site/src/content/docs/404.md (1)
1-7: LGTM!apps/site/src/content/docs/getting-started.md (1)
1-29: LGTM!apps/site/src/pages/index.astro (16)
117-226: LGTM!
291-301: LGTM!
842-965: LGTM!
1491-1548: LGTM!
1919-1929: LGTM!
2053-2076: LGTM!
2089-2097: LGTM!Also applies to: 2138-2146, 2186-2194, 2232-2240
2323-2328: LGTM!
2481-2565: LGTM!
2567-2606: LGTM!
2815-2992: LGTM!
3210-3213: LGTM!
3257-3291: LGTM!
3379-3474: LGTM!
3476-3544: LGTM!
3554-3588: LGTM!apps/site/.env.example (1)
15-17: LGTM!apps/site/public/feature-work-organizer/touchai-components.html (7)
1469-1489: Same issue as the English version: verify wheel forwarding target.This has the same
window.scrollByvswindow.parent.scrollByquestion flagged in the English variant.
1563-1567: Same indentation issue as the English version.Lines 1565-1566 are under-indented inside the if block.
86-88: LGTM!Also applies to: 155-168, 470-476, 496-515, 760-760
1116-1116: LGTM!Also applies to: 1130-1130, 1148-1148
1165-1189: LGTM!
1297-1324: LGTM!
1229-1229: LGTM!Also applies to: 1357-1357, 1368-1439, 1461-1467, 1501-1528, 1590-1650, 1661-1769
apps/site/public/feature-work-organizer-en/touchai-components.html (6)
86-88: LGTM!Also applies to: 155-168, 470-476, 496-515, 760-760
1118-1118: LGTM!Also applies to: 1132-1132, 1150-1150
1167-1191: LGTM!
1299-1326: LGTM!
1231-1231: LGTM!Also applies to: 1359-1359, 1370-1441, 1463-1469, 1503-1534, 1596-1656, 1667-1774
1471-1491: Wheel forwarding target is correct (window.scrollBy), no iframe involved.The demos are mounted into the page by
apps/site/src/scripts/component-demo-runtime.tsviadoc.body.cloneNode(true)+host.replaceChildren(...)on thetouchai-component-demoelement (not an iframe). Sowindow.scrollBy(0, nextDeltaY)scrolls the same document that GSAP/ScrollTrigger uses, makingwindow.parent.scrollByunnecessary here.> Likely an incorrect or invalid review comment.
Comment on lines
+2
to
+6
| const renderer = window.TouchAILiteRenderer; | ||
|
|
||
| function escapeHtml(value) { | ||
| return renderer ? renderer.escapeHtml(value) : String(value); | ||
| } |
There was a problem hiding this comment.
Restore a real HTML-escaping fallback here.
escapeHtml() returns raw strings whenever TouchAILiteRenderer is unavailable at parse time. That makes the standalone/load-order fallback in renderMarkdownWithMath() inject raw HTML instead of escaped content.
Suggested fix
(function () {
- const renderer = window.TouchAILiteRenderer;
+ const getRenderer = () => window.TouchAILiteRenderer;
function escapeHtml(value) {
- return renderer ? renderer.escapeHtml(value) : String(value);
+ const renderer = getRenderer();
+ if (renderer?.escapeHtml) {
+ return renderer.escapeHtml(value);
+ }
+
+ return String(value)
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+ .replace(/'/g, '&`#39`;');
}
@@
- let html = renderer
- ? renderer.renderMarkdownContent(protectedMarkdown)
+ const renderer = getRenderer();
+ let html = renderer
+ ? renderer.renderMarkdownContent(protectedMarkdown)
: escapeHtml(protectedMarkdown);Also applies to: 229-231
🤖 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 `@apps/site/public/demo-utils/touchai-lite-math.js` around lines 2 - 6,
escapeHtml currently returns raw strings when window.TouchAILiteRenderer is
unavailable, causing unescaped HTML to be injected by renderMarkdownWithMath;
replace the fallback with a proper escaping routine: keep using
renderer.escapeHtml(value) when renderer exists, otherwise implement a local
escape that converts &, <, >, ", ', and ` to their HTML entities; update the
escapeHtml function (and the identical fallback used around lines 229-231) so
renderMarkdownWithMath always receives safely escaped text.
Comment on lines
1476
to
1482
| function resetVisibleBlocks() { | ||
| response | ||
| .querySelectorAll('p, ul, li, .math-block, .response-divider') | ||
| .querySelectorAll('p, h2, ul, li, .math-block, .response-divider') | ||
| .forEach((block) => { | ||
| block.classList.remove('is-visible'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Clear tool-card visibility during resets.
The new reset selector list still omits .tool-call-list and .tool-call. After the first reveal, scrubbing back or replaying leaves those cards visible from the start of the next run.
Suggested fix
function resetVisibleBlocks() {
response
- .querySelectorAll('p, h2, ul, li, .math-block, .response-divider')
+ .querySelectorAll(
+ 'p, h2, ul, li, .tool-call-list, .tool-call, .math-block, .response-divider'
+ )
.forEach((block) => {
block.classList.remove('is-visible');
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function resetVisibleBlocks() { | |
| response | |
| .querySelectorAll('p, ul, li, .math-block, .response-divider') | |
| .querySelectorAll('p, h2, ul, li, .math-block, .response-divider') | |
| .forEach((block) => { | |
| block.classList.remove('is-visible'); | |
| }); | |
| } | |
| function resetVisibleBlocks() { | |
| response | |
| .querySelectorAll( | |
| 'p, h2, ul, li, .tool-call-list, .tool-call, .math-block, .response-divider' | |
| ) | |
| .forEach((block) => { | |
| block.classList.remove('is-visible'); | |
| }); | |
| } |
🤖 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 `@apps/site/public/feature-reminder-en/touchai-components.html` around lines
1476 - 1482, The resetVisibleBlocks function's selector set is missing tool card
classes so .tool-call-list and .tool-call elements remain visible after a reset;
update the querySelectorAll call inside resetVisibleBlocks to include
'.tool-call-list' and '.tool-call' alongside the existing selectors so those
elements have 'is-visible' removed during resets.
Comment on lines
+496
to
+500
| body.is-scroll-driven.is-complete .chat-panel { | ||
| min-height: var(--window-min-height); | ||
| max-height: var(--window-min-height); | ||
| height: var(--window-min-height); | ||
| } |
There was a problem hiding this comment.
Preserve the mobile height override in scroll-driven completion.
body.is-scroll-driven.is-complete .chat-panel is more specific than the mobile body.is-complete .chat-panel rule below, so on short viewports this reintroduces the fixed 657px height and overflows the iframe/card as soon as the scrubber reaches complete. Scope this rule to desktop or add a matching mobile override.
Suggested CSS fix
- body.is-scroll-driven.is-complete .chat-panel {
- min-height: var(--window-min-height);
- max-height: var(--window-min-height);
- height: var(--window-min-height);
- }
+ `@media` (min-width: 841px) {
+ body.is-scroll-driven.is-complete .chat-panel {
+ min-height: var(--window-min-height);
+ max-height: var(--window-min-height);
+ height: var(--window-min-height);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| body.is-scroll-driven.is-complete .chat-panel { | |
| min-height: var(--window-min-height); | |
| max-height: var(--window-min-height); | |
| height: var(--window-min-height); | |
| } | |
| `@media` (min-width: 841px) { | |
| body.is-scroll-driven.is-complete .chat-panel { | |
| min-height: var(--window-min-height); | |
| max-height: var(--window-min-height); | |
| height: var(--window-min-height); | |
| } | |
| } |
🤖 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 `@apps/site/public/feature-reminder/touchai-components.html` around lines 496 -
500, The rule body.is-scroll-driven.is-complete .chat-panel is overriding the
mobile rule body.is-complete .chat-panel and reintroducing the fixed 657px
height on short viewports; limit this selector to desktop (e.g., wrap it in a
desktop media query) or add a matching mobile override that restores the mobile
height behavior so short viewports don’t get the fixed 657px: update the CSS
where body.is-scroll-driven.is-complete .chat-panel is defined and either scope
it with a desktop-only media query or add a body.is-scroll-driven.is-complete
.chat-panel mobile-specific rule that mirrors the existing body.is-complete
.chat-panel mobile override.
Comment on lines
+1575
to
+1595
| function forwardWheelToPage(event) { | ||
| if (event.ctrlKey || event.metaKey) { | ||
| event.preventDefault(); | ||
| return; | ||
| } | ||
|
|
||
| if (!isScrollDriven || !hasReceivedScrollProgress) { | ||
| return; | ||
| } | ||
|
|
||
| const deltaMultiplier = | ||
| event.deltaMode === 1 ? 16 : event.deltaMode === 2 ? window.innerHeight : 1; | ||
| const nextDeltaY = event.deltaY * deltaMultiplier; | ||
|
|
||
| if (Math.abs(nextDeltaY) < 0.5) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| window.scrollBy(0, nextDeltaY); | ||
| } |
There was a problem hiding this comment.
This wheel handler never reaches the host page.
After preventDefault(), window.scrollBy() only scrolls the iframe document. In scroll-driven mode that traps wheel input inside the demo instead of forwarding it to the embedding page. Forward the delta to window.parent or send it over the existing postMessage channel.
🤖 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 `@apps/site/public/feature-reminder/touchai-components.html` around lines 1575
- 1595, The wheel handler forwardWheelToPage currently prevents default and
calls window.scrollBy, which only scrolls the iframe; instead forward the
computed delta (nextDeltaY) to the host so the embedding page can handle
scrolling—use window.parent.postMessage (or the existing postMessage channel) to
send a message containing { type: 'wheel-delta', deltaY: nextDeltaY } when
isScrollDriven && hasReceivedScrollProgress are true, keep the ctrl/meta
early-return logic, and remove or stop relying on window.scrollBy to avoid
trapping wheel input inside the iframe; reference forwardWheelToPage,
isScrollDriven, hasReceivedScrollProgress and the computed nextDeltaY when
implementing.
| @@ -1,5 +1,5 @@ | |||
| <!doctype html> | |||
| <html lang="zh-CN"> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Translate the remaining accessibility and fallback copy before switching this page to lang="en".
The document is now declared English, but this file still exposes Chinese strings to users and assistive tech — for example Line 982, Line 1023, and the prerendered answer body starting at Line 1049. That gives screen readers the wrong language context and leaves the no-JS/failed-script fallback untranslated.
🤖 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 `@apps/site/public/feature-solver-en/touchai-components.html` at line 2, The
page declares lang="en" but still contains Chinese copy in
accessibility/fallback text (e.g., the strings near Line 982, Line 1023 and the
prerendered answer block starting at Line 1049), so either translate those
Chinese strings into English or revert lang to "zh" until translation is done;
specifically locate and update the Chinese text in the prerendered answer body,
any no-JS/failed-script fallback content, and ARIA/alt/label attributes (search
for the prerendered answer container and fallback block IDs/classes and the text
nodes around the noted lines) to their English equivalents so screen readers and
fallbacks get correct language context before keeping <html lang="en">.
Comment on lines
+1
to
+3
| import { defineCollection } from 'astro:content'; | ||
| import { docsLoader } from '@astrojs/starlight/loaders'; | ||
| import { docsSchema } from '@astrojs/starlight/schema'; |
There was a problem hiding this comment.
Fix import order to satisfy simple-import-sort/imports.
Lint is currently failing on the import block; please run autofix or reorder imports per the configured sorter.
🧰 Tools
🪛 ESLint
[error] 1-3: Run autofix to sort these imports!
(simple-import-sort/imports)
🤖 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 `@apps/site/src/content.config.ts` around lines 1 - 3, The import block is out
of order and failing simple-import-sort; reorder the imports so external package
imports are grouped and sorted before local/alias imports—specifically ensure
'astro:content' (defineCollection) and '`@astrojs/starlight/`...' imports
(docsLoader, docsSchema) follow the project's import-sort rules (or run the
autofixer) so the imports for defineCollection, docsLoader, and docsSchema are
in the correct sorted groups and order.
Source: Linters/SAST tools
Comment on lines
+37
to
+52
| try { | ||
| const response = await fetch(repoApiUrl, { headers }); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| stats.stars = data.stargazers_count; | ||
| stats.forks = data.forks_count; | ||
| } | ||
| } catch {} | ||
|
|
||
| try { | ||
| const response = await fetch(repoOpenIssuesApiUrl, { headers }); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| stats.issues = data.total_count; | ||
| } | ||
| } catch {} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add diagnostic logging for build-time stats fetch failures.
The build-time GitHub stats loader silently swallows all fetch errors. If the GitHub API is unreachable during build (rate-limited, network issue, etc.), the stats will be null and the client will display -- until the first successful runtime refresh. Consider logging failures to help diagnose build-time issues.
📊 Suggested diagnostic logging
try {
const response = await fetch(repoApiUrl, { headers });
if (response.ok) {
const data = await response.json();
stats.stars = data.stargazers_count;
stats.forks = data.forks_count;
+ } else {
+ console.warn(`[build] GitHub repo stats fetch failed: ${response.status}`);
}
- } catch {}
+ } catch (error) {
+ console.warn('[build] GitHub repo stats fetch error:', error instanceof Error ? error.message : String(error));
+ }
try {
const response = await fetch(repoOpenIssuesApiUrl, { headers });
if (response.ok) {
const data = await response.json();
stats.issues = data.total_count;
+ } else {
+ console.warn(`[build] GitHub open issues fetch failed: ${response.status}`);
}
- } catch {}
+ } catch (error) {
+ console.warn('[build] GitHub open issues fetch error:', error instanceof Error ? error.message : String(error));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const response = await fetch(repoApiUrl, { headers }); | |
| if (response.ok) { | |
| const data = await response.json(); | |
| stats.stars = data.stargazers_count; | |
| stats.forks = data.forks_count; | |
| } | |
| } catch {} | |
| try { | |
| const response = await fetch(repoOpenIssuesApiUrl, { headers }); | |
| if (response.ok) { | |
| const data = await response.json(); | |
| stats.issues = data.total_count; | |
| } | |
| } catch {} | |
| try { | |
| const response = await fetch(repoApiUrl, { headers }); | |
| if (response.ok) { | |
| const data = await response.json(); | |
| stats.stars = data.stargazers_count; | |
| stats.forks = data.forks_count; | |
| } else { | |
| console.warn(`[build] GitHub repo stats fetch failed: ${response.status}`); | |
| } | |
| } catch (error) { | |
| console.warn('[build] GitHub repo stats fetch error:', error instanceof Error ? error.message : String(error)); | |
| } | |
| try { | |
| const response = await fetch(repoOpenIssuesApiUrl, { headers }); | |
| if (response.ok) { | |
| const data = await response.json(); | |
| stats.issues = data.total_count; | |
| } else { | |
| console.warn(`[build] GitHub open issues fetch failed: ${response.status}`); | |
| } | |
| } catch (error) { | |
| console.warn('[build] GitHub open issues fetch error:', error instanceof Error ? error.message : String(error)); | |
| } |
🤖 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 `@apps/site/src/pages/index.astro` around lines 37 - 52, The fetch blocks for
repoApiUrl and repoOpenIssuesApiUrl currently swallow all errors (empty catch)
and ignore non-ok responses; update the try/catch handlers around
fetch(repoApiUrl, { headers }) and fetch(repoOpenIssuesApiUrl, { headers }) to
log diagnostics: on non-ok responses log the URL and
response.status/response.statusText, and in the catch blocks log the thrown
Error (including message/stack) along with which fetch (repoApiUrl or
repoOpenIssuesApiUrl) failed so build-time failures are visible (use your
existing logger or console.error).
Comment on lines
+1814
to
+1817
| <body | ||
| data-waitlist-endpoint={WAITLIST_ENDPOINT} | ||
| data-analytics-endpoint={ANALYTICS_ENDPOINT} | ||
| > |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Validate analytics endpoint URL format.
The ANALYTICS_ENDPOINT is passed directly to data-analytics-endpoint without validation. If the environment variable contains an invalid URL, the client-side fetch() calls will fail silently. Consider validating the URL format at build time or checking it before use in the client script.
🔍 Suggested validation
In the front-matter, add validation after reading the env var:
const WAITLIST_ENDPOINT = import.meta.env.PUBLIC_WAITLIST_ENDPOINT ?? '';
-const ANALYTICS_ENDPOINT = import.meta.env.PUBLIC_ANALYTICS_ENDPOINT ?? '';
+const ANALYTICS_ENDPOINT = (() => {
+ const raw = import.meta.env.PUBLIC_ANALYTICS_ENDPOINT ?? '';
+ if (!raw) return '';
+ try {
+ new URL(raw);
+ return raw;
+ } catch {
+ console.warn('[build] PUBLIC_ANALYTICS_ENDPOINT is not a valid URL:', raw);
+ return '';
+ }
+})();🤖 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 `@apps/site/src/pages/index.astro` around lines 1814 - 1817, The
ANALYTICS_ENDPOINT environment value is embedded directly into
data-analytics-endpoint (and used by client fetches) without validating its URL
format; update the front-matter where ANALYTICS_ENDPOINT is read to validate it
(e.g., attempt new URL(ANALYTICS_ENDPOINT) or RegExp) and if invalid set it to
null/empty or a safe fallback, then only render the data-analytics-endpoint
attribute when the validated value exists; also update the client-side code that
reads dataset.analyticsEndpoint to check for a truthy, valid URL before calling
fetch to avoid silent failures.
Comment on lines
+2685
to
+2738
| const analyticsEndpoint = document.body.dataset.analyticsEndpoint?.trim(); | ||
| let hasTrackedPageView = false; | ||
| const sendAnalytics = (type, detail = {}) => { | ||
| if (!analyticsEndpoint) return; | ||
| const payload = JSON.stringify({ | ||
| type, | ||
| href: window.location.href, | ||
| path: window.location.pathname, | ||
| title: document.title, | ||
| lang: document.documentElement.lang, | ||
| referrer: document.referrer || '', | ||
| viewport: `${window.innerWidth}x${window.innerHeight}`, | ||
| ts: new Date().toISOString(), | ||
| ...detail, | ||
| }); | ||
|
|
||
| try { | ||
| if (navigator.sendBeacon) { | ||
| const sent = navigator.sendBeacon( | ||
| analyticsEndpoint, | ||
| new Blob([payload], { type: 'application/json' }) | ||
| ); | ||
| if (sent) return; | ||
| } | ||
| } catch (_error) { | ||
| // Fall through to fetch. | ||
| } | ||
|
|
||
| fetch(analyticsEndpoint, { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: payload, | ||
| keepalive: true, | ||
| }).catch(() => {}); | ||
| }; | ||
| const trackPageView = () => { | ||
| if (hasTrackedPageView) return; | ||
| hasTrackedPageView = true; | ||
| sendAnalytics('pageview'); | ||
| }; | ||
|
|
||
| if (document.readyState === 'complete') { | ||
| queueMicrotask(trackPageView); | ||
| } else { | ||
| window.addEventListener('load', trackPageView, { once: true }); | ||
| } | ||
|
|
||
| document.querySelectorAll('[data-analytics-event]').forEach((node) => { | ||
| node.addEventListener('click', () => { | ||
| sendAnalytics(node.dataset.analyticsEvent, { | ||
| label: node.getAttribute('aria-label') || node.textContent?.trim() || '', | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Document privacy implications of analytics tracking.
The analytics implementation collects referrer, viewport, and lang without user consent or privacy notice. While the tracking is disabled by default (requires PUBLIC_ANALYTICS_ENDPOINT to be set), deployers should be aware that enabling this feature may require privacy disclosures under GDPR/CCPA. Consider adding a comment in the code and updating the .env.example documentation to note this requirement.
📋 Suggested documentation
In .env.example:
# Generic analytics collector endpoint that accepts JSON POST payloads.
# Leave empty to disable the built-in pageview and click tracking.
+# Note: Enabling analytics may require privacy disclosures (GDPR/CCPA).
+# The implementation collects: href, path, title, lang, referrer, viewport, timestamp.
PUBLIC_ANALYTICS_ENDPOINT=In the script, add a comment before sendAnalytics:
+ // Analytics collects referrer, viewport, and language.
+ // Deployers enabling this feature should provide appropriate privacy notices.
const sendAnalytics = (type, detail = {}) => {🤖 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 `@apps/site/src/pages/index.astro` around lines 2685 - 2738, Add
developer-facing privacy documentation and an in-code comment noting
legal/privacy implications when enabling analytics: update the .env.example to
document that PUBLIC_ANALYTICS_ENDPOINT enables collection of referrer,
viewport, and lang which may trigger GDPR/CCPA disclosure/consent requirements,
and add a clear comment immediately above the sendAnalytics function (and
mention analyticsEndpoint and trackPageView) explaining what fields are
collected, that tracking is opt-in via the env var, and that deployers must
ensure appropriate user notices/consent before enabling it.
Comment on lines
+2907
to
+2923
| const withCacheBuster = (url) => | ||
| `${url}${url.includes('?') ? '&' : '?'}ts=${Date.now()}`; | ||
| const fetchRepoOverview = async () => { | ||
| const headers = { | ||
| Accept: 'application/vnd.github+json', | ||
| 'X-GitHub-Api-Version': '2022-11-28', | ||
| }; | ||
| const [repoResponse, openIssuesResponse] = await Promise.all([ | ||
| fetch(withCacheBuster(REPO_STATS_API_URL), { | ||
| cache: 'no-store', | ||
| headers, | ||
| }), | ||
| fetch(withCacheBuster(REPO_OPEN_ISSUES_API_URL), { | ||
| cache: 'no-store', | ||
| headers, | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Remove redundant cache busting.
The fetchRepoOverview function uses both cache: 'no-store' and a timestamp query parameter via withCacheBuster. The cache: 'no-store' directive already prevents the browser from caching the response, making the timestamp query parameter redundant. Consider removing one for clarity.
♻️ Suggested simplification
const fetchRepoOverview = async () => {
const headers = {
Accept: 'application/vnd.github+json',
'X-GitHub-Api-Version': '2022-11-28',
};
const [repoResponse, openIssuesResponse] = await Promise.all([
- fetch(withCacheBuster(REPO_STATS_API_URL), {
+ fetch(REPO_STATS_API_URL, {
cache: 'no-store',
headers,
}),
- fetch(withCacheBuster(REPO_OPEN_ISSUES_API_URL), {
+ fetch(REPO_OPEN_ISSUES_API_URL, {
cache: 'no-store',
headers,
}),
]);Alternatively, if you prefer the timestamp approach (which works even if cache headers are not honored), remove cache: 'no-store' instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const withCacheBuster = (url) => | |
| `${url}${url.includes('?') ? '&' : '?'}ts=${Date.now()}`; | |
| const fetchRepoOverview = async () => { | |
| const headers = { | |
| Accept: 'application/vnd.github+json', | |
| 'X-GitHub-Api-Version': '2022-11-28', | |
| }; | |
| const [repoResponse, openIssuesResponse] = await Promise.all([ | |
| fetch(withCacheBuster(REPO_STATS_API_URL), { | |
| cache: 'no-store', | |
| headers, | |
| }), | |
| fetch(withCacheBuster(REPO_OPEN_ISSUES_API_URL), { | |
| cache: 'no-store', | |
| headers, | |
| }), | |
| ]); | |
| const withCacheBuster = (url) => | |
| `${url}${url.includes('?') ? '&' : '?'}ts=${Date.now()}`; | |
| const fetchRepoOverview = async () => { | |
| const headers = { | |
| Accept: 'application/vnd.github+json', | |
| 'X-GitHub-Api-Version': '2022-11-28', | |
| }; | |
| const [repoResponse, openIssuesResponse] = await Promise.all([ | |
| fetch(REPO_STATS_API_URL, { | |
| cache: 'no-store', | |
| headers, | |
| }), | |
| fetch(REPO_OPEN_ISSUES_API_URL, { | |
| cache: 'no-store', | |
| headers, | |
| }), | |
| ]); |
🤖 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 `@apps/site/src/pages/index.astro` around lines 2907 - 2923, fetchRepoOverview
currently adds a timestamp query via withCacheBuster and also sets fetch option
cache: 'no-store' — remove the redundant timestamp approach: stop calling
withCacheBuster(REPO_STATS_API_URL) and
withCacheBuster(REPO_OPEN_ISSUES_API_URL) and pass the plain REPO_STATS_API_URL
and REPO_OPEN_ISSUES_API_URL to fetch (leave cache: 'no-store' in the fetch
options); update both fetch calls inside fetchRepoOverview and, if
withCacheBuster is unused elsewhere, remove the withCacheBuster helper to keep
the file clean.
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
Open issuescounts only open issues, not pull requests. The page now shows42 Stars / 24 Forks / 72 Open issues.Related issue or RFC
This is a follow-up to the previously merged site refresh and responsive/demo polish PRs.
AI assistance disclosure
apps/site.Testing evidence
Additional local browser verification:
pnpm test:prwas not completed locally. The repository pre-commit path startedpnpm run check:rust, but Rust setup failed while downloading the bundled RTK asset from GitHub:This PR only changes
apps/site; fullpnpm test:prshould be covered by CI before merge.Did you follow TDD? No. This is site UI/runtime polish verified through build output and browser checks; no new automated test was added because the behavior is static site rendering and GitHub API display logic.
Risk notes
AgentService, runtime, MCP, or schema impact: noneScreenshots or recordings
Local browser verification was performed at:
Observed stats:
Checklist
[WIP]or similar title prefixes.AgentService, runtime, MCP, or schema boundaries, there is an accepted RFC. N/A.pnpm test:prfor this code PR, or this is a docs-only change. Local run was blocked by GitHub RTK asset download timeout; relying on CI before merge.pnpm test:coverage:rustor relied on CI coverage evidence. N/A.pnpm test:e2elocally or documented why CI is the first valid proof. N/A.