feat(docs): add 3D hero visual — cinematic particle network#211
feat(docs): add 3D hero visual — cinematic particle network#211
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a custom 3D hero to the VitePress docs: a Vue Changes
Sequence DiagramsequenceDiagram
participant Browser
participant VitePress as VitePress Route
participant Layout
participant Hero3D
participant useThreeScene as useThreeScene<br/>(Composable)
participant ThreeJS as Three.js Scene<br/>(Renderer/Composer)
Browser->>VitePress: Navigate to home (/ or /tywrap/)
VitePress->>Layout: Render layout
Layout-->>Layout: Check route is home?
alt Home Route
Layout->>Hero3D: Mount component (layout-top)
Hero3D->>useThreeScene: init with canvas,width,height
useThreeScene->>ThreeJS: Create scene, camera, renderer, composer, objects
Hero3D->>useThreeScene: threeScene.start()
useThreeScene->>ThreeJS: Start RAF loop (update uniforms, animate, render)
Browser->>Hero3D: Scroll / Resize / Mouse events
Hero3D->>useThreeScene: onScroll / resize / mouse -> update targets
useThreeScene->>ThreeJS: Apply scroll/mouse to uniforms/positions
Browser->>Hero3D: Navigate away
Hero3D->>useThreeScene: dispose()
useThreeScene->>ThreeJS: Cancel RAF, dispose resources
else Non-Home Route
Layout->>Layout: Render default layout only
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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)
📝 Coding Plan
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
added
documentation
Add an interactive 3D hero section to the VitePress docs site, featuring a Hermes 4-inspired cinematic particle network with: - 15K noise-driven particles in amber/sapphire/green tywrap palette - Neural edge tubes with pulsing opacity for data-flow aesthetic - Bloom + vignette post-processing for high-end visual feel - Mouse parallax and scroll-reactive camera movement - Asymmetric left-aligned layout with shimmer gradient headline - Force-dark theme, transparent backgrounds for canvas integration Ported from bbopen/tywrap-hero-visual (React Three Fiber) and reimagined as a raw Three.js + Vue 3 composable. Includes design docs and codecert patch equivalence verification. New deps: three, postprocessing, @types/three
bbopen
force-pushed
the
feat/hero-visual
branch
from
432a3b5 to
d120e44
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 432a3b5d0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "postprocessing": "^6.38.3", | ||
| "three": "^0.183.2", |
There was a problem hiding this comment.
three and postprocessing are only imported from docs/.vitepress/theme/composables/useThreeScene.ts, while this package publishes only dist, runtime, src, and README.md. Keeping those browser-only docs packages in dependencies means every npm install tywrap pulls them into production projects even though the published artifact cannot reference them, which is an avoidable install-size and audit-surface regression for all consumers.
Useful? React with 👍 / 👎.
| coreGroup.position.x = 8 + mouse.x * 2.0 | ||
| coreGroup.position.y = mouse.y * 2.0 - (scrollState.current * 0.01) |
There was a problem hiding this comment.
onMouseMove writes into targetMouse, but the render loop still reads mouse.x and mouse.y here and never copies or lerps from the target. In practice that means the new hero never reacts to cursor movement, so the advertised mouse-parallax effect is disabled for every home-page visitor.
Useful? React with 👍 / 👎.
| function dispose() { | ||
| if (rafId !== null) cancelAnimationFrame(rafId) | ||
| window.removeEventListener('mousemove', onMouseMove) | ||
| composer.dispose() | ||
| renderer.dispose() | ||
| geometry.dispose() | ||
| material.dispose() |
There was a problem hiding this comment.
dispose() only frees the particle geometry/material and renderer. The instanced node SphereGeometry/material and the 40 TubeGeometry meshes added to linesGroup are never disposed, so each home → docs page → home navigation in the SPA leaks GPU buffers; after a few visits or hot reloads this will steadily increase VRAM usage and can start degrading or losing the WebGL context on weaker devices.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/.vitepress/theme/components/Hero3D.vue`:
- Around line 16-24: The onScroll function uses a non-null assertion threeScene!
even though a guard exists; instead capture the reference to threeScene before
scheduling the frame: inside onScroll, assign const scene = threeScene and use
scene and scene.onScroll in the requestAnimationFrame callback (and keep the
existing scrollTicking logic), so you avoid the non-null assertion while
preserving the guard and behavior of onScroll and scrollTicking.
In `@docs/.vitepress/theme/composables/useThreeScene.ts`:
- Around line 365-372: In the dispose() function add disposal for the remaining
resources mentioned in the review: call nodeGeometry.dispose() and
nodeMaterial.dispose(), and iterate any arrays/collections of node meshes or
extra geometries/materials created elsewhere (e.g., the nodes array or other
createdBufferGeometries/materials) to call .dispose() on each, ensuring all
dynamically created geometries and materials are released in addition to the
existing composer.dispose(), renderer.dispose(), geometry.dispose(), and
material.dispose() calls.
- Line 295: The InstancedMesh named nodes is being added to scene
(scene.add(nodes)) while particles and lines are added to coreGroup, so nodes
won't inherit coreGroup's rotation/parallax transforms; either move nodes into
coreGroup by replacing scene.add(nodes) with coreGroup.add(nodes) so it
orbits/moves with the core, or if the static behavior is intentional, leave
scene.add(nodes) but add a clarifying comment next to the scene.add(nodes) call
explaining why nodes are excluded from coreGroup transforms (reference symbols:
nodes, scene.add, coreGroup and the coreGroup rotation/parallax transform
usage).
- Around line 304-326: The loop creates THREE.TubeGeometry and
THREE.MeshBasicMaterial instances that are never disposed; update the code to
track each tube geometry (e.g., push the created tubeGeo into a new array like
lineGeometries or store on the mesh) alongside the existing lineMaterials and
meshes in linesGroup, and then in the existing dispose() function iterate over
those stored meshes/geometries/materials to call geometry.dispose() and
material.dispose(), remove meshes from linesGroup, and clear the arrays
(lineGeometries, lineMaterials, and any mesh references) to release WebGL
resources; reference the TubeGeometry, MeshBasicMaterial, linesGroup,
lineMaterials and dispose() symbols when making the change.
- Around line 156-165: The mouse parallax is broken because the mouse Vector2 is
never updated from targetMouse—add smoothing at the start of the animate() loop
to lerp mouse toward targetMouse (e.g., mouse.lerp(targetMouse, smoothingFactor)
or manual interpolation) so the values used later for parallax (where mouse is
read) reflect the latest pointer with smoothing; update any related use of
scrollState as needed, and ensure the onMouseMove listener remains and animate()
runs each frame.
In `@docs/.vitepress/theme/custom.css`:
- Around line 9-21: The stylesheet overuses !important on selectors .VPHome,
.VPHome .VPFeatures, and .VPHome .VPHomeContent which can hinder future
overrides; add a clear comment above these rules explaining why the VitePress
defaults require !important (e.g., specificity of upstream styles or inline
styles from VitePress) and, if feasible, replace !important by increasing
selector specificity (e.g., more specific class chains or attribute selectors)
so the overrides are intentional and maintainable; ensure the comment references
.VPHome, .VPHomeContent and .VPFeatures and describes the exact specificity
issue and where to revisit when VitePress changes.
In `@docs/.vitepress/theme/index.ts`:
- Around line 10-15: The current Layout component uses route.path strict
equality to detect home (const isHome = route.path === '/' || route.path ===
'/tywrap/'), which is brittle when cleanUrls toggles trailing slashes; update
the check in Layout to normalize or robustly match the route (e.g., trim
trailing slash or use startsWith or the route.name) so '/' and '/tywrap' (with
or without trailing slash) are treated as home—modify the isHome calculation
inside Layout (referencing Layout, useRoute, isHome, and route.path) to perform
normalization or a safer match.
In `@docs/check_errors.cjs`:
- Around line 3-19: Add failure tracking and timeout/error handling so the
script fails CI when page errors occur: introduce a boolean flag (e.g.,
hasErrors) captured by the existing page.on('console', ...) and
page.on('pageerror', ...) handlers to set true when encountering
error/warning/pageerror; call page.goto with a timeout option and wrap it in
try/catch so navigation failures are logged and set hasErrors; ensure
browser.close runs in a finally block; after closing, call
process.exit(hasErrors ? 1 : 0) to return non-zero on any detected problems.
In `@docs/download_hermes.py`:
- Around line 1-36: The file docs/download_hermes.py appears unrelated and
downloads external JS to /tmp (see top-level symbols: url, script_urls,
combined_js and the output path "/tmp/hermes4_js/all_scripts.js"); either remove
this file from the PR or move it out of docs/ into a dedicated dev-scripts or
tools directory, add a short README entry describing its purpose and usage if
you intend to keep it, and update the commit to exclude it from the feature
branch if it was added accidentally (or add it to .gitignore/CI rules if it must
remain local).
- Around line 6-9: Validate the URL scheme before calling
urllib.request.Request/urllib.request.urlopen by parsing the url variable (e.g.,
with urllib.parse.urlparse) and ensuring parsed.scheme is "https" (or whitelist
"http" and "https") and reject or raise/log an error for any other scheme;
update the code around the url, urllib.request.Request, and
urllib.request.urlopen calls to perform this check and abort/raise if the scheme
is not allowed.
- Line 4: Remove the unused module import by deleting the top-level "import
json" statement in the file; ensure no other code references the json module
(search for any uses like json.loads/json.dumps) and run linters or tests to
confirm no breakage.
In `@docs/plans/2026-03-19-hero-visual-port-design.md`:
- Around line 31-51: Update the fenced code block in the hero visual plan so it
includes a language identifier (e.g., change the opening ``` to ```text or
```plaintext) to satisfy linters and improve screen-reader accessibility; locate
the scene graph block (the multi-line diagram starting with "Canvas (alpha, dpr
[1,2], antialias off)" and replace the fence marker accordingly so the rest of
the content (PerspectiveCamera, AmbientLight, Torus, GlassShield, LightStream,
Sparkles, Environment, EffectComposer, Bloom) remains unchanged.
In `@package.json`:
- Around line 120-121: The package.json currently lists "three" and
"postprocessing" as runtime dependencies; remove those two entries from the
top-level "dependencies" section and add them with the same semver versions into
"devDependencies" instead so they are only installed for development (used by
the VitePress docs in docs/.vitepress/theme/); ensure you update any lockfile
(e.g., package-lock.json or yarn.lock) by reinstalling so the dependency tree
reflects the move.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2dc6a90c-f567-40c6-bf1e-b6b1f0bf56c3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
docs/.vitepress/config.tsdocs/.vitepress/theme/components/Hero3D.vuedocs/.vitepress/theme/composables/useThreeScene.tsdocs/.vitepress/theme/custom.cssdocs/.vitepress/theme/index.tsdocs/check_errors.cjsdocs/download_hermes.pydocs/index.mddocs/plans/2026-03-19-hero-visual-port-codecert.mddocs/plans/2026-03-19-hero-visual-port-design.mddocs/plans/2026-03-19-hero-visual-port.mddocs/public/llms-full.txtpackage.json
💤 Files with no reviewable changes (2)
- docs/index.md
- docs/public/llms-full.txt
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.21.0)
docs/plans/2026-03-19-hero-visual-port-design.md
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.15.6)
docs/download_hermes.py
[error] 8-8: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 9-9: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 14-14: Probable insecure usage of temporary file or directory: "/tmp/hermes4_js"
(S108)
[error] 25-25: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 26-26: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 28-28: Do not catch blind exception: Exception
(BLE001)
[error] 31-31: Probable insecure usage of temporary file or directory: "/tmp/hermes4_js/all_scripts.js"
(S108)
[warning] 35-35: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (15)
package.json (1)
122-123: Tree-sitter version downgrade appears intentional.The downgrade from
^0.25.0to^0.21.0aligns with the comment insrc/core/analyzer.tswhich notes typings differences. Ensure this version is tested across all platforms, as tree-sitter has native bindings that may behave differently across Node versions.docs/.vitepress/config.ts (1)
7-7: LGTM — forces dark theme as intended.This aligns with the 3D hero visual design. Note that this removes the user's ability to switch to light mode site-wide, which may affect accessibility for users with light-mode preferences or certain visual conditions.
docs/.vitepress/theme/custom.css (1)
24-30: LGTM — asymmetric layout for desktop.The 60% max-width constraint and left alignment create the intended asymmetric layout that keeps content readable over the 3D background.
docs/plans/2026-03-19-hero-visual-port-design.md (1)
1-77: Design document is comprehensive and well-structured.Good documentation of the porting decisions, scene graph, and verification approach. The codecert verification plan will help ensure parity with the React source.
docs/.vitepress/theme/components/Hero3D.vue (3)
26-41: LGTM — proper lifecycle management.The scene is initialized on mount, listeners are attached with proper passive flag for scroll, and cleanup is handled correctly in
onBeforeUnmount.
77-83: Anchor elements styled as buttons lack explicitroleattributes.For better accessibility, consider adding
role="button"to the styled anchor elements, or leave as-is since they are legitimate navigation links (not actions).Actually, these are navigation links to other pages, so using
<a>is semantically correct. No change needed.
154-163: Good implementation of gradient text with shimmer animation.The background-clip technique with animation creates an engaging effect. The
-webkit-text-strokeprovides subtle definition. Performance should be monitored on lower-end devices as animated gradients can be CPU-intensive.docs/plans/2026-03-19-hero-visual-port.md (1)
1-10: Plan describes a different implementation than what was delivered.The plan outlines a 1:1 port with torus rings, glass shield (icosahedron), light streams (tube curves), and sparkles. However, the actual implementation in
useThreeScene.tsis a fundamentally different design: a particle network with 15,000 noise-driven particles, instanced node spheres, and neural edge tubes.This discrepancy means:
- The plan is now stale documentation that doesn't reflect the shipped code
- The codecert verification document (which claims "EQUIVALENT") references line numbers and parameters that don't exist in the actual implementation
Consider either updating this plan to match the new particle-network design, or archiving it with a note that the approach was changed.
[raise_minor_issue, inconsistent_summary]
docs/plans/2026-03-19-hero-visual-port-codecert.md (1)
1-11: Certificate references non-existent code — verification is invalid.This certificate claims "EQUIVALENT" and references specific line numbers in
useThreeScene.ts(e.g., Lines 47-48 for camera, Line 124 for TorusGeometry, Lines 161-180 for GlassShield). However, the actual implementation:
- Has no
TorusGeometry(torus rings)- Has no
IcosahedronGeometry(glass shield)- Has no
createLightStreamfactory- Uses camera at
z=25(Line 154), notz=12- Uses bloom intensity
4.0(Line 189), not1.0- Is a particle-network design, not the torus/shield design described here
Either the implementation was rewritten after this certificate was created, or this certificate was generated for a different codebase. This document should be removed or regenerated against the actual shipped code.
[raise_major_issue, inconsistent_summary]
docs/.vitepress/theme/composables/useThreeScene.ts (6)
18-32: LGTM!The interface definitions are well-structured with clear typing for both input options and return values.
104-129: LGTM!The vertex shader correctly implements noise-based particle animation with scroll parallax. The point size calculation accounts for perspective, and the alpha twinkling effect uses noise for organic variation.
167-196: LGTM!The renderer configuration with ACESFilmicToneMapping and the postprocessing pipeline with bloom and vignette are well-configured for the intended cinematic aesthetic.
203-264: LGTM!The particle system initialization correctly builds the BufferGeometry with custom attributes for shader consumption. The spherical distribution with elliptical stretching creates the intended visual effect.
331-358: LGTM!The animation loop correctly updates uniforms, applies time-based rotations, and renders via the composer. The line opacity pulsing creates a pleasing visual rhythm.
374-379: LGTM!The resize handler correctly updates camera aspect ratio, projection matrix, and both renderer and composer sizes.
| function onScroll() { | ||
| if (!scrollTicking && threeScene && threeScene.onScroll) { | ||
| window.requestAnimationFrame(() => { | ||
| threeScene!.onScroll(window.scrollY) | ||
| scrollTicking = false | ||
| }) | ||
| scrollTicking = true | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Non-null assertion is safe but could be cleaner.
The threeScene! assertion on line 19 is safe because of the guard on line 17, but the pattern could be cleaner by capturing the reference:
Capture reference to avoid assertion
function onScroll() {
- if (!scrollTicking && threeScene && threeScene.onScroll) {
+ const scene = threeScene
+ if (!scrollTicking && scene?.onScroll) {
window.requestAnimationFrame(() => {
- threeScene!.onScroll(window.scrollY)
+ scene.onScroll(window.scrollY)
scrollTicking = false
})
scrollTicking = true
}
}📝 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.
| function onScroll() { | |
| if (!scrollTicking && threeScene && threeScene.onScroll) { | |
| window.requestAnimationFrame(() => { | |
| threeScene!.onScroll(window.scrollY) | |
| scrollTicking = false | |
| }) | |
| scrollTicking = true | |
| } | |
| } | |
| function onScroll() { | |
| const scene = threeScene | |
| if (!scrollTicking && scene?.onScroll) { | |
| window.requestAnimationFrame(() => { | |
| scene.onScroll(window.scrollY) | |
| scrollTicking = false | |
| }) | |
| scrollTicking = true | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/.vitepress/theme/components/Hero3D.vue` around lines 16 - 24, The
onScroll function uses a non-null assertion threeScene! even though a guard
exists; instead capture the reference to threeScene before scheduling the frame:
inside onScroll, assign const scene = threeScene and use scene and
scene.onScroll in the requestAnimationFrame callback (and keep the existing
scrollTicking logic), so you avoid the non-null assertion while preserving the
guard and behavior of onScroll and scrollTicking.
| const mouse = new THREE.Vector2() | ||
| const targetMouse = new THREE.Vector2() | ||
| const scrollState = { target: 0, current: 0 } | ||
|
|
||
| function onMouseMove(event: MouseEvent) { | ||
| targetMouse.x = (event.clientX / window.innerWidth) * 2 - 1 | ||
| targetMouse.y = -(event.clientY / window.innerHeight) * 2 + 1 | ||
| } | ||
| window.addEventListener('mousemove', onMouseMove) | ||
| function onScroll(scrollY: number) { scrollState.target = scrollY } |
There was a problem hiding this comment.
Mouse parallax is broken — mouse is never updated from targetMouse.
The mouse vector is used for parallax at Lines 349-350, but it's never updated. The targetMouse receives mouse positions via the event listener, but the smoothing/assignment to mouse is missing from the animation loop.
🐛 Proposed fix — add lerp in animate()
Add the smoothing logic at the beginning of the animate function (around Line 335):
function animate() {
rafId = requestAnimationFrame(animate)
const elapsed = clock.getElapsedTime()
scrollState.current += (scrollState.target - scrollState.current) * 0.05
+
+ // Smooth mouse parallax
+ mouse.x += (targetMouse.x - mouse.x) * 0.05
+ mouse.y += (targetMouse.y - mouse.y) * 0.05
// Update uniforms
material.uniforms.uTime.value = elapsed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/.vitepress/theme/composables/useThreeScene.ts` around lines 156 - 165,
The mouse parallax is broken because the mouse Vector2 is never updated from
targetMouse—add smoothing at the start of the animate() loop to lerp mouse
toward targetMouse (e.g., mouse.lerp(targetMouse, smoothingFactor) or manual
interpolation) so the values used later for parallax (where mouse is read)
reflect the latest pointer with smoothing; update any related use of scrollState
as needed, and ensure the onMouseMove listener remains and animate() runs each
frame.
| } | ||
| nodes.instanceMatrix.needsUpdate = true | ||
| if (nodes.instanceColor) nodes.instanceColor.needsUpdate = true | ||
| scene.add(nodes) |
There was a problem hiding this comment.
Nodes added to scene instead of coreGroup — intentional?
The nodes InstancedMesh is added to scene (Line 295) while particles and lines are added to coreGroup. This means nodes won't follow the coreGroup's rotation and parallax transformations applied at Lines 344-350.
If nodes should orbit/move with the particle core, add them to coreGroup instead:
- scene.add(nodes)
+ coreGroup.add(nodes)If the static behavior is intentional for visual contrast, consider adding a comment to clarify.
📝 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.
| scene.add(nodes) | |
| coreGroup.add(nodes) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/.vitepress/theme/composables/useThreeScene.ts` at line 295, The
InstancedMesh named nodes is being added to scene (scene.add(nodes)) while
particles and lines are added to coreGroup, so nodes won't inherit coreGroup's
rotation/parallax transforms; either move nodes into coreGroup by replacing
scene.add(nodes) with coreGroup.add(nodes) so it orbits/moves with the core, or
if the static behavior is intentional, leave scene.add(nodes) but add a
clarifying comment next to the scene.add(nodes) call explaining why nodes are
excluded from coreGroup transforms (reference symbols: nodes, scene.add,
coreGroup and the coreGroup rotation/parallax transform usage).
| for(let i=0; i<lineCount; i++) { | ||
| const points = [] | ||
| // Random path from center extending outward | ||
| let currentPt = new THREE.Vector3((Math.random()-0.5)*2, (Math.random()-0.5)*2, (Math.random()-0.5)*2) | ||
| for(let step=0; step<15; step++) { | ||
| points.push(currentPt.clone()) | ||
| // Wander outward | ||
| const wander = new THREE.Vector3((Math.random()-0.5)*3, (Math.random()-0.5)*3, (Math.random()-0.5)*3) | ||
| currentPt.add(wander).add(currentPt.clone().normalize().multiplyScalar(1.5)) | ||
| } | ||
| const curve = new THREE.CatmullRomCurve3(points) | ||
| const tubeGeo = new THREE.TubeGeometry(curve, 64, 0.05, 8, false) | ||
|
|
||
| const mat = new THREE.MeshBasicMaterial({ | ||
| color: i % 2 === 0 ? 0x3b82f6 : 0x10b981, | ||
| transparent: true, | ||
| opacity: 0.6, | ||
| blending: THREE.AdditiveBlending, | ||
| depthWrite: false, | ||
| }) | ||
| lineMaterials.push(mat) | ||
| linesGroup.add(new THREE.Mesh(tubeGeo, mat)) | ||
| } |
There was a problem hiding this comment.
Memory leak — tube geometries and materials not disposed.
The tube geometries and materials created in this loop are not cleaned up in the dispose() function. Each TubeGeometry and MeshBasicMaterial should be disposed to prevent WebGL resource leaks.
🛠️ Proposed fix — track and dispose tube resources
Store geometries for disposal:
const lineMaterials: THREE.MeshBasicMaterial[] = []
+ const lineGeometries: THREE.TubeGeometry[] = []
for(let i=0; i<lineCount; i++) {
// ... path generation ...
const tubeGeo = new THREE.TubeGeometry(curve, 64, 0.05, 8, false)
+ lineGeometries.push(tubeGeo)
const mat = new THREE.MeshBasicMaterial({
// ...
})
lineMaterials.push(mat)
linesGroup.add(new THREE.Mesh(tubeGeo, mat))
}Then update dispose():
function dispose() {
if (rafId !== null) cancelAnimationFrame(rafId)
window.removeEventListener('mousemove', onMouseMove)
composer.dispose()
renderer.dispose()
geometry.dispose()
material.dispose()
+ lineGeometries.forEach(g => g.dispose())
+ lineMaterials.forEach(m => m.dispose())
+ nodeGeometry.dispose()
+ nodeMaterial.dispose()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/.vitepress/theme/composables/useThreeScene.ts` around lines 304 - 326,
The loop creates THREE.TubeGeometry and THREE.MeshBasicMaterial instances that
are never disposed; update the code to track each tube geometry (e.g., push the
created tubeGeo into a new array like lineGeometries or store on the mesh)
alongside the existing lineMaterials and meshes in linesGroup, and then in the
existing dispose() function iterate over those stored
meshes/geometries/materials to call geometry.dispose() and material.dispose(),
remove meshes from linesGroup, and clear the arrays (lineGeometries,
lineMaterials, and any mesh references) to release WebGL resources; reference
the TubeGeometry, MeshBasicMaterial, linesGroup, lineMaterials and dispose()
symbols when making the change.
| function dispose() { | ||
| if (rafId !== null) cancelAnimationFrame(rafId) | ||
| window.removeEventListener('mousemove', onMouseMove) | ||
| composer.dispose() | ||
| renderer.dispose() | ||
| geometry.dispose() | ||
| material.dispose() | ||
| } |
There was a problem hiding this comment.
Incomplete resource disposal — additional geometries and materials leaked.
Beyond the tube resources mentioned earlier, nodeGeometry and nodeMaterial (Lines 267-274) are also not disposed. See the fix proposed in the previous comment for a complete solution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/.vitepress/theme/composables/useThreeScene.ts` around lines 365 - 372,
In the dispose() function add disposal for the remaining resources mentioned in
the review: call nodeGeometry.dispose() and nodeMaterial.dispose(), and iterate
any arrays/collections of node meshes or extra geometries/materials created
elsewhere (e.g., the nodes array or other createdBufferGeometries/materials) to
call .dispose() on each, ensuring all dynamically created geometries and
materials are released in addition to the existing composer.dispose(),
renderer.dispose(), geometry.dispose(), and material.dispose() calls.
| import urllib.request | ||
| import re | ||
| import os | ||
| import json | ||
|
|
||
| url = "https://hermes4.nousresearch.com/" | ||
| try: | ||
| req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'}) | ||
| html = urllib.request.urlopen(req).read().decode('utf-8') | ||
|
|
||
| script_urls = re.findall(r'<script[^>]+src="([^"]+)"', html) | ||
| print(f"Found {len(script_urls)} scripts.") | ||
|
|
||
| os.makedirs("/tmp/hermes4_js", exist_ok=True) | ||
|
|
||
| combined_js = "" | ||
| for src in script_urls: | ||
| if src.startswith('/'): | ||
| full_url = "https://hermes4.nousresearch.com" + src | ||
| else: | ||
| full_url = src | ||
|
|
||
| print(f"Downloading {full_url}") | ||
| try: | ||
| req = urllib.request.Request(full_url, headers={'User-Agent': 'Mozilla/5.0'}) | ||
| js = urllib.request.urlopen(req).read().decode('utf-8') | ||
| combined_js += f"\n\n/* --- {src} --- */\n\n" + js | ||
| except Exception as e: | ||
| print(f"Failed to download {full_url}: {e}") | ||
|
|
||
| with open("/tmp/hermes4_js/all_scripts.js", "w") as f: | ||
| f.write(combined_js) | ||
| print("Saved to /tmp/hermes4_js/all_scripts.js") | ||
|
|
||
| except Exception as e: | ||
| print(f"Error: {e}") |
There was a problem hiding this comment.
This script appears unrelated to the PR objective.
This file downloads JavaScript from an external site (hermes4.nousresearch.com) and concatenates it to /tmp. It has no references from other PR files and doesn't align with the stated goal of adding a 3D hero visual.
Was this file included accidentally? If it's a development utility, consider:
- Moving it outside the
docs/directory - Adding documentation explaining its purpose
- Removing it from this PR if unrelated
🧰 Tools
🪛 Ruff (0.15.6)
[error] 8-8: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 9-9: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 14-14: Probable insecure usage of temporary file or directory: "/tmp/hermes4_js"
(S108)
[error] 25-25: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 26-26: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 28-28: Do not catch blind exception: Exception
(BLE001)
[error] 31-31: Probable insecure usage of temporary file or directory: "/tmp/hermes4_js/all_scripts.js"
(S108)
[warning] 35-35: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/download_hermes.py` around lines 1 - 36, The file
docs/download_hermes.py appears unrelated and downloads external JS to /tmp (see
top-level symbols: url, script_urls, combined_js and the output path
"/tmp/hermes4_js/all_scripts.js"); either remove this file from the PR or move
it out of docs/ into a dedicated dev-scripts or tools directory, add a short
README entry describing its purpose and usage if you intend to keep it, and
update the commit to exclude it from the feature branch if it was added
accidentally (or add it to .gitignore/CI rules if it must remain local).
| import urllib.request | ||
| import re | ||
| import os | ||
| import json |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import.
The json module is imported but never used.
import urllib.request
import re
import os
-import json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/download_hermes.py` at line 4, Remove the unused module import by
deleting the top-level "import json" statement in the file; ensure no other code
references the json module (search for any uses like json.loads/json.dumps) and
run linters or tests to confirm no breakage.
| url = "https://hermes4.nousresearch.com/" | ||
| try: | ||
| req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'}) | ||
| html = urllib.request.urlopen(req).read().decode('utf-8') |
There was a problem hiding this comment.
URL scheme is not validated before opening.
The script opens URLs without validating the scheme, which could allow file:// or other unexpected protocols if the URL variable is modified. While the URL is currently hardcoded to HTTPS, adding validation is a defensive measure.
Add scheme validation
url = "https://hermes4.nousresearch.com/"
+if not url.startswith(('https://', 'http://')):
+ raise ValueError(f"Invalid URL scheme: {url}")
try:
req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'})📝 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.
| url = "https://hermes4.nousresearch.com/" | |
| try: | |
| req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'}) | |
| html = urllib.request.urlopen(req).read().decode('utf-8') | |
| url = "https://hermes4.nousresearch.com/" | |
| if not url.startswith(('https://', 'http://')): | |
| raise ValueError(f"Invalid URL scheme: {url}") | |
| try: | |
| req = urllib.request.Request(url, headers={'User-Agent': 'Mozilla/5.0'}) | |
| html = urllib.request.urlopen(req).read().decode('utf-8') |
🧰 Tools
🪛 Ruff (0.15.6)
[error] 8-8: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[error] 9-9: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/download_hermes.py` around lines 6 - 9, Validate the URL scheme before
calling urllib.request.Request/urllib.request.urlopen by parsing the url
variable (e.g., with urllib.parse.urlparse) and ensuring parsed.scheme is
"https" (or whitelist "http" and "https") and reject or raise/log an error for
any other scheme; update the code around the url, urllib.request.Request, and
urllib.request.urlopen calls to perform this check and abort/raise if the scheme
is not allowed.
| ``` | ||
| Canvas (alpha, dpr [1,2], antialias off) | ||
| ├── PerspectiveCamera (pos [0,0,12], fov 45) | ||
| ├── AmbientLight (0.2) | ||
| ├── PointLight (pos [10,10,10], intensity 1) | ||
| ├── PointLight (pos [-10,-10,-10], #3b82f6, intensity 2) | ||
| ├── Float group (speed 2, rotationIntensity 0.5, floatIntensity 0.5) | ||
| │ ├── Core group (rotation x*0.4, y*0.3) | ||
| │ │ ├── Torus — Python ring (#f59e0b, emissive 2, rot [π/2,0,0], r=1.2, tube=0.08) | ||
| │ │ ├── Torus — TypeScript ring (#3b82f6, emissive 2, rot [0,π/2,0], r=1.2, tube=0.08) | ||
| │ │ └── Sphere — inner glow (#fff, emissive 1, r=0.7) | ||
| │ ├── GlassShield — Icosahedron (r=2.4, detail 1, transmission, ior 1.5, clearcoat) | ||
| │ │ └── Edges (threshold 15, #3b82f6) | ||
| │ ├── LightStream — amber (#f59e0b, speed 0.8, offset 0, r=3.5, h=8, intensity 5) | ||
| │ └── LightStream — blue (#3b82f6, speed -1, offset π, r=4, h=8, intensity 5) | ||
| ├── Sparkles (#3b82f6, count 200, scale 15, size 2, speed 0.2, opacity 0.5) | ||
| ├── Sparkles (#f59e0b, count 100, scale 15, size 3, speed 0.4, opacity 0.3) | ||
| ├── Environment (preset "city") | ||
| └── EffectComposer | ||
| └── Bloom (luminanceThreshold 1, mipmapBlur, intensity 1.0) | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add language identifier to fenced code block.
The scene graph diagram lacks a language identifier. Use text or plaintext to satisfy linters and improve accessibility for screen readers.
-```
+```text
Canvas (alpha, dpr [1,2], antialias off)
├── PerspectiveCamera (pos [0,0,12], fov 45)🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/2026-03-19-hero-visual-port-design.md` around lines 31 - 51,
Update the fenced code block in the hero visual plan so it includes a language
identifier (e.g., change the opening ``` to ```text or ```plaintext) to satisfy
linters and improve screen-reader accessibility; locate the scene graph block
(the multi-line diagram starting with "Canvas (alpha, dpr [1,2], antialias off)"
and replace the fence marker accordingly so the rest of the content
(PerspectiveCamera, AmbientLight, Torus, GlassShield, LightStream, Sparkles,
Environment, EffectComposer, Bloom) remains unchanged.
| "postprocessing": "^6.38.3", | ||
| "three": "^0.183.2", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving three and postprocessing to devDependencies.
These packages are only used in the VitePress documentation site (docs/.vitepress/theme/), not in the main library runtime. Adding them as runtime dependencies increases the install footprint for all consumers of the package.
Move to devDependencies
"dependencies": {
"@babel/parser": "^7.25.6",
"@babel/types": "^7.25.6",
- "postprocessing": "^6.38.3",
- "three": "^0.183.2",
"tree-sitter": "^0.21.0",Add to devDependencies section:
"devDependencies": {
"@types/bun": "^1.2.19",
"@types/node": "^22.5.4",
"@types/three": "^0.183.1",
+ "postprocessing": "^6.38.3",
+ "three": "^0.183.2",📝 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.
| "postprocessing": "^6.38.3", | |
| "three": "^0.183.2", | |
| "@babel/parser": "^7.25.6", | |
| "@babel/types": "^7.25.6", | |
| "tree-sitter": "^0.21.0", |
| "postprocessing": "^6.38.3", | |
| "three": "^0.183.2", | |
| "@types/bun": "^1.2.19", | |
| "@types/node": "^22.5.4", | |
| "@types/three": "^0.183.1", | |
| "postprocessing": "^6.38.3", | |
| "three": "^0.183.2", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 120 - 121, The package.json currently lists
"three" and "postprocessing" as runtime dependencies; remove those two entries
from the top-level "dependencies" section and add them with the same semver
versions into "devDependencies" instead so they are only installed for
development (used by the VitePress docs in docs/.vitepress/theme/); ensure you
update any lockfile (e.g., package-lock.json or yarn.lock) by reinstalling so
the dependency tree reflects the move.
|
Closing to recreate with review fixes applied (removed unrelated scripts, fixed mouse parallax, resource disposal, dep placement). |
2 participants
Summary
bbopen/tywrap-hero-visual(React Three Fiber), reimagined as raw Three.js + Vue 3New dependencies
three(Three.js core)postprocessing(Bloom, Vignette effects)@types/three(dev)Files
docs/.vitepress/theme/composables/useThreeScene.tsdocs/.vitepress/theme/components/Hero3D.vuedocs/.vitepress/theme/custom.cssdocs/.vitepress/theme/index.tslayout-topslotdocs/.vitepress/config.tsappearance: 'force-dark'docs/index.mdTest plan
npm run docs:buildpasses