Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new apps/website Vite-powered marketing site to the monorepo and wires it into root workspace scripts, plus updates repository clone URLs in READMEs.
Changes:
- Introduces a new
apps/websiteworkspace (HTML + TS components + CSS design system assets + favicon). - Adds root npm scripts to run/build the website workspace and updates lockfile for the new workspace dependencies.
- Updates clone URLs in
README.mdandv1_agent/README.md.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| v1_agent/README.md | Updates clone URL to the correct GitHub repo. |
| package.json | Adds workspace scripts for running/building the website app. |
| package-lock.json | Adds the apps/website workspace dependency tree and lock entries. |
| apps/website/tsconfig.json | Adds website TypeScript configuration for bundler-mode builds. |
| apps/website/src/styles/variables.css | Adds design tokens and shared base styles. |
| apps/website/src/styles/use-cases.css | Adds styling for the “Use Cases” section. |
| apps/website/src/styles/reset.css | Adds a CSS reset for the website. |
| apps/website/src/styles/quickstart.css | Adds styling for the “Quick Start” section. |
| apps/website/src/styles/problem-solution.css | Adds styling for the problem/how-it-works split layout. |
| apps/website/src/styles/navbar.css | Adds styling for the fixed navbar + mobile overlay behavior. |
| apps/website/src/styles/live-preview.css | Adds styling for the live preview tabs/code blocks. |
| apps/website/src/styles/how-it-works.css | Adds styling for the BYOK section. |
| apps/website/src/styles/hero.css | Adds styling for the hero section and canvas wrapper. |
| apps/website/src/styles/footer.css | Adds styling for CTA + footer content. |
| apps/website/src/styles/architecture.css | Adds styling for the architecture cards section. |
| apps/website/src/main.ts | Website entrypoint that initializes UI behaviors. |
| apps/website/src/components/preview.ts | Adds preview tabs and copy-to-clipboard interactions. |
| apps/website/src/components/observer.ts | Adds IntersectionObserver-based scroll animations with reduced-motion support. |
| apps/website/src/components/navbar.ts | Adds navbar scroll behavior, mobile overlay toggle, and smooth scrolling. |
| apps/website/src/components/data-flow.ts | Adds canvas-based “data flow” hero animation. |
| apps/website/public/favicon.svg | Adds website favicon. |
| apps/website/package.json | Adds website workspace config and Vite/TypeScript devDependencies. |
| apps/website/index.html | Adds full landing page markup and inline code samples. |
| apps/website/.gitignore | Adds website-specific ignores (dist, node_modules, editor files). |
| README.md | Updates clone URL to the correct GitHub repo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Close mobile menu on link click | ||
| links?.querySelectorAll("a").forEach((link) => { | ||
| link.addEventListener("click", () => { | ||
| links.classList.remove("open"); | ||
| toggle?.classList.remove("active"); | ||
| document.body.style.overflow = ""; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
links?.querySelectorAll("a").forEach(...) will still throw if links is null because the optional chain only guards the querySelectorAll call; .forEach is invoked on the result. Use links?.querySelectorAll("a")?.forEach(...) (or guard if (!links) return) before iterating.
| <pre><span class="t-kw">import</span> { MarrowClient } <span class="t-kw">from</span> <span class="t-str">"@marrow/client"</span>; | ||
|
|
||
| <span class="t-comment">// Bring your own key — any major LLM</span> | ||
| <span class="t-kw">const</span> marrow = <span class="t-kw">new</span> <span class="t-type">MarrowClient</span>({ | ||
| <span class="t-prop">model</span>: <span class="t-str">"gpt-4o"</span>, | ||
| <span class="t-prop">apiKey</span>: process.env.<span class="t-prop">AI_API_KEY</span>, | ||
| <span class="t-prop">registryUrl</span>: process.env.<span class="t-prop">REGISTRY_URL</span>, | ||
| }); | ||
|
|
||
| <span class="t-comment">// Get a map — cache hit or JIT generation</span> | ||
| <span class="t-kw">const</span> map = <span class="t-kw">await</span> marrow.<span class="t-method">getMap</span>( | ||
| <span class="t-str">"linkedin.com/jobs"</span> | ||
| ); | ||
|
|
||
| <span class="t-comment">// Use semantic selectors</span> | ||
| <span class="t-kw">const</span> btn = map.elements.<span class="t-prop">apply_button</span>; | ||
| <span class="t-kw">await</span> page.<span class="t-method">click</span>(btn.strategies[<span class="t-num">0</span>].value);</pre> |
There was a problem hiding this comment.
The Quick Start code sample doesn’t match the actual MarrowClient implementation in this repo (apps/client/src/index.ts constructor takes { geminiKey, registryUrl }, and map.elements is an array, not an object with apply_button). If this snippet is meant to be runnable, update it to reflect the current API shape; otherwise, label it clearly as pseudocode to avoid confusing users.
| toggle?.addEventListener("click", () => { | ||
| const isOpen = links?.classList.toggle("open"); | ||
| toggle.classList.toggle("active", isOpen); | ||
| document.body.style.overflow = isOpen ? "hidden" : ""; | ||
| }); | ||
|
|
||
| // Close mobile menu on link click | ||
| links?.querySelectorAll("a").forEach((link) => { | ||
| link.addEventListener("click", () => { | ||
| links.classList.remove("open"); | ||
| toggle?.classList.remove("active"); | ||
| document.body.style.overflow = ""; | ||
| }); | ||
| }); |
There was a problem hiding this comment.
toggle and links are typed as HTMLElement | null, but inside the click handler you dereference toggle.classList and rely on the boolean result of links?.classList.toggle(...). In strict TS this typically triggers "possibly null" errors, and if links is missing isOpen becomes undefined which makes toggle.classList.toggle('active', isOpen) behave unexpectedly. Consider early-returning unless both elements exist (or capturing non-null locals) and ensuring isOpen is always a boolean.
| toggle?.addEventListener("click", () => { | |
| const isOpen = links?.classList.toggle("open"); | |
| toggle.classList.toggle("active", isOpen); | |
| document.body.style.overflow = isOpen ? "hidden" : ""; | |
| }); | |
| // Close mobile menu on link click | |
| links?.querySelectorAll("a").forEach((link) => { | |
| link.addEventListener("click", () => { | |
| links.classList.remove("open"); | |
| toggle?.classList.remove("active"); | |
| document.body.style.overflow = ""; | |
| }); | |
| }); | |
| if (toggle && links) { | |
| toggle.addEventListener("click", () => { | |
| const isOpen = links.classList.toggle("open"); | |
| toggle.classList.toggle("active", isOpen); | |
| document.body.style.overflow = isOpen ? "hidden" : ""; | |
| }); | |
| // Close mobile menu on link click | |
| links.querySelectorAll("a").forEach((link) => { | |
| link.addEventListener("click", () => { | |
| links.classList.remove("open"); | |
| toggle.classList.remove("active"); | |
| document.body.style.overflow = ""; | |
| }); | |
| }); | |
| } |
| const text = activePanel.textContent || ""; | ||
| navigator.clipboard.writeText(text).then(() => { | ||
| copyBtn.textContent = "Copied!"; | ||
| copyBtn.classList.add("copied"); | ||
| setTimeout(() => { | ||
| copyBtn.textContent = "Copy"; | ||
| copyBtn.classList.remove("copied"); | ||
| }, 2000); | ||
| }); | ||
| }); | ||
|
|
||
| // Quick start copy button | ||
| document.querySelectorAll(".qs-copy-btn").forEach((btn) => { | ||
| btn.addEventListener("click", () => { | ||
| const codeBlock = | ||
| btn.closest(".qs-code-window")?.querySelector("pre") ?? | ||
| btn.parentElement?.querySelector("code"); | ||
| if (!codeBlock) return; | ||
|
|
||
| const text = codeBlock.textContent || ""; | ||
| navigator.clipboard.writeText(text).then(() => { | ||
| (btn as HTMLElement).textContent = "Copied!"; | ||
| btn.classList.add("copied"); | ||
| setTimeout(() => { | ||
| (btn as HTMLElement).textContent = "Copy"; | ||
| btn.classList.remove("copied"); | ||
| }, 2000); | ||
| }); |
There was a problem hiding this comment.
Both clipboard write flows call navigator.clipboard.writeText(...).then(...) without handling rejections. In non-secure contexts, unsupported browsers, or when permission is denied, this results in an unhandled promise rejection and no UX feedback. Add a .catch(...) path (and consider a fallback copy method) so failures are handled gracefully.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| <div class="nav-center" id="nav-links"> | ||
| <a href="#features">Features</a> | ||
| <a href="/docs" class="nav-docs-link">Docs</a> | ||
| <a href="#quickstart">Quick Start</a> | ||
| <a href="#architecture">Architecture</a> | ||
| <a href="#use-cases">Use Cases</a> | ||
| </div> | ||
|
|
||
| <div class="nav-actions"> | ||
| <a href="https://github.com/tomiwa-a/marrow" target="_blank" rel="noopener" class="nav-github" aria-label="GitHub"> | ||
| <svg viewBox="0 0 24 24" fill="currentColor"><path d="M12 0C5.37 0 0 5.37 0 12c0 5.31 3.435 9.795 8.205 11.385.6.105.825-.255.825-.57 0-.285-.015-1.23-.015-2.235-3.015.555-3.795-.735-4.035-1.41-.135-.345-.72-1.41-1.23-1.695-.42-.225-1.02-.78-.015-.795.945-.015 1.62.87 1.845 1.23 1.08 1.815 2.805 1.305 3.495.99.105-.78.42-1.305.765-1.605-2.67-.3-5.46-1.335-5.46-5.925 0-1.305.465-2.385 1.23-3.225-.12-.3-.54-1.53.12-3.18 0 0 1.005-.315 3.3 1.23.96-.27 1.98-.405 3-.405s2.04.135 3 .405c2.295-1.56 3.3-1.23 3.3-1.23.66 1.65.24 2.88.12 3.18.765.84 1.23 1.905 1.23 3.225 0 4.605-2.805 5.625-5.475 5.925.435.375.81 1.095.81 2.22 0 1.605-.015 2.895-.015 3.3 0 .315.225.69.825.57A12.02 12.02 0 0024 12c0-6.63-5.37-12-12-12z"/></svg> | ||
| </a> | ||
| <a href="#quickstart" class="nav-cta">Get Started</a> | ||
| <button class="nav-toggle" id="nav-toggle" aria-label="Toggle menu"> | ||
| <span></span><span></span><span></span> | ||
| </button> |
There was a problem hiding this comment.
The mobile menu toggle button only has aria-label. For accessibility, it should also expose state (e.g., aria-expanded) and association (e.g., aria-controls="nav-links"), and initNavbar() should update aria-expanded when toggling. Without this, screen readers can’t tell whether the menu is open.
| <pre class="preview-code" data-panel="sdk"><span class="t-kw">import</span> { MarrowClient } <span class="t-kw">from</span> <span class="t-str">"@marrow/client"</span>; | ||
|
|
||
| <span class="t-comment">// Works with any major LLM</span> | ||
| <span class="t-kw">const</span> marrow = <span class="t-kw">new</span> <span class="t-type">MarrowClient</span>({ | ||
| <span class="t-prop">model</span>: <span class="t-str">"claude-sonnet-4-20250514"</span>, | ||
| <span class="t-prop">apiKey</span>: process.env.<span class="t-prop">AI_API_KEY</span>, | ||
| <span class="t-prop">registryUrl</span>: process.env.<span class="t-prop">REGISTRY_URL</span>, | ||
| }); | ||
|
|
||
| <span class="t-comment">// Get a map — cache hit or JIT generation</span> | ||
| <span class="t-kw">const</span> map = <span class="t-kw">await</span> marrow.<span class="t-method">getMap</span>(<span class="t-str">"linkedin.com/jobs"</span>); | ||
|
|
||
| <span class="t-comment">// Use semantic selectors — survives CSS refactors</span> | ||
| <span class="t-kw">const</span> btn = map.elements.<span class="t-prop">apply_button</span>; | ||
| <span class="t-kw">await</span> page.<span class="t-method">click</span>(btn.strategies[<span class="t-num">0</span>].value);</pre> |
There was a problem hiding this comment.
The “SDK Usage” snippet repeats the same API mismatches as Quick Start (constructor options + map.elements.apply_button vs an elements array). If the website is meant to document the repo’s API, these examples should be brought in sync with the current client/schema to avoid sending users down a broken path.
| "devDependencies": { | ||
| "typescript": "~5.9.3", | ||
| "vite": "npm:[email protected]" | ||
| }, | ||
| "overrides": { | ||
| "vite": "npm:[email protected]" | ||
| } |
There was a problem hiding this comment.
vite is pinned to npm:[email protected], which (per the lockfile) declares a Node engine of ^20.19.0 || >=22.12.0. Since the repo doesn’t currently document/enforce a Node version, this can break installs for contributors on Node 18/20.0-20.18. Consider either switching to a standard Vite version compatible with your supported Node range, or adding an engines.node policy + documentation at the repo root.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| /* ─── Responsive ─── */ | ||
| @media (max-width: 1024px) { | ||
| .hero { | ||
| padding-top: calc(var(--nav-height) + var(--space-16)); | ||
| } | ||
| } | ||
| @media (max-width: 1024px) { | ||
| .hero > .container { | ||
| grid-template-columns: 1fr; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .hero-content { | ||
| max-width: 600px; | ||
| margin-inline: auto; | ||
| } | ||
|
|
||
| .hero-subtitle { | ||
| max-width: 500px; | ||
| margin-inline: auto; | ||
| } | ||
|
|
||
| .hero-ctas { | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .hero-stats { | ||
| justify-content: center; | ||
| } | ||
|
|
||
| .hero-visual { | ||
| max-width: 560px; | ||
| margin-inline: auto; | ||
| } | ||
| } |
There was a problem hiding this comment.
There are two separate @media (max-width: 1024px) blocks back-to-back. This is functionally fine, but combining them reduces duplication and makes responsive behavior easier to maintain.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <[email protected]>
….css Co-authored-by: tomiwa-a <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: tomiwa-a <[email protected]>
Combine duplicate media query blocks in hero.css
Co-authored-by: tomiwa-a <[email protected]>
Add error handling and fallback for clipboard operations
No description provided.