feat: CEL expression docs + docs search (closes #1, #5)#6
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…achability Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the thin CEL overview with comprehensive documentation covering available variables, common expression patterns, template vs bare syntax, type safety, bracket notation, and limitations. Update links in data-passing and workflow-reference docs to point to cel.dev and the new expressions guide instead of the go-cel GitHub repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Pagefind indexes all 28 doc pages at build time - Search modal with Cmd+K / Ctrl+K keyboard shortcut - Lazy-loads Pagefind JS on first search (no upfront cost) - 200ms debounced input, up to 8 results with title + excerpt - Search trigger in desktop nav (docs pages) and sidebar top - Landing page excluded from index (data-pagefind-ignore) - Styled with Terminal Green design tokens Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a comprehensive in-site CEL expressions reference and introduces docs search powered by Pagefind to improve discoverability across the Astro-based docs site.
Changes:
- Rewrites
/docs/concepts/expressionswith a fuller CEL reference and updates internal docs links to point to it (and cel.dev). - Adds Pagefind-powered docs search UI (modal + Cmd/Ctrl+K) and integrates search triggers into nav/sidebar.
- Updates build pipeline to run Pagefind indexing and adds supporting styling/layout adjustments.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| site/src/styles/global.css | Adds Pagefind highlight styling; attempts mobile/table tweaks for prose content. |
| site/src/pages/index.astro | Marks landing page content to be ignored by Pagefind (partial). |
| site/src/layouts/Docs.astro | Marks docs article body for Pagefind indexing and adjusts layout spacing. |
| site/src/content/docs/workflow-reference/index.md | Updates CEL references/links to cel.dev + new Expressions guide. |
| site/src/content/docs/getting-started/data-passing.md | Points CEL syntax section to the new Expressions guide. |
| site/src/content/docs/concepts/expressions.md | Major rewrite: namespaces, examples, template vs bare syntax, limitations. |
| site/src/components/Nav.astro | Adds desktop search trigger on “docs” pages; replaces mobile hamburger with bottom nav + docs menu button. |
| site/src/components/DocsSidebar.astro | Adds search in sidebar; adjusts mobile drawer sizing and script behavior. |
| site/src/components/DocsSearch.astro | New Pagefind search trigger + modal + lazy-loaded search implementation. |
| site/package.json | Adds Pagefind dependency and indexing step to build script. |
| site/package-lock.json | Locks Pagefind + platform binaries. |
Files not reviewed (1)
- site/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+7
to
+31
| <div class="hidden md:block"> | ||
| <DocsSearch /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
isDocs is derived as !isHome, so non-docs pages like /404 are treated as docs: the desktop search input shows up and the mobile “Menu” button renders but cannot work (no docs sidebar on that page). Determine isDocs from the current path (e.g., Astro.url.pathname.startsWith('/docs')) or pass an explicit isDocs prop from layouts/pages that need it.
Comment on lines
86
to
+105
| <script is:inline> | ||
| function initNav() { | ||
| const btn = document.getElementById('mobile-menu-btn'); | ||
| const menu = document.getElementById('mobile-menu'); | ||
| if (!btn || !menu) return; | ||
| const btn = document.getElementById('bottom-nav-docs-btn'); | ||
| const docsSidebar = document.getElementById('docs-sidebar-mobile'); | ||
| const overlay = document.getElementById('sidebar-overlay'); | ||
| if (!btn || !docsSidebar || !overlay) return; | ||
|
|
||
| // Remove old listeners by cloning | ||
| const newBtn = btn.cloneNode(true); | ||
| btn.parentNode?.replaceChild(newBtn, btn); | ||
| newBtn.addEventListener('click', () => menu.classList.toggle('hidden')); | ||
|
|
||
| newBtn.addEventListener('click', () => { | ||
| const isOpen = !docsSidebar.classList.contains('-translate-x-full'); | ||
| if (isOpen) { | ||
| docsSidebar.classList.add('-translate-x-full'); | ||
| overlay.classList.add('hidden'); | ||
| } else { | ||
| docsSidebar.classList.remove('-translate-x-full'); | ||
| overlay.classList.remove('hidden'); | ||
| } |
There was a problem hiding this comment.
initNav() captures overlay once, but DocsSidebar later replaces #sidebar-overlay via cloneNode in its own initializer. That leaves this click handler toggling classes on a detached overlay node, so the overlay may never appear. Avoid replacing the overlay element in DocsSidebar, or re-query #sidebar-overlay inside the click handler / re-run init after DocsSidebar’s setup so both components act on the same element.
Search component:
- Split into DocsSearch (trigger only) + SearchModal (single shared modal)
- SearchModal rendered once in Base.astro, no duplicate IDs
- Global Cmd+K handler registered once with guard flag
- Excerpt sanitized via DOMParser (only <mark> tags allowed)
- Dialog a11y: role="dialog", aria-modal="true", aria-label
Navigation:
- isDocs derived from Astro.url.pathname.startsWith('/docs')
- Nav/Sidebar overlay: use onclick assignment, no cloneNode (fixes detached ref)
Content:
- data-pagefind-ignore wraps entire landing page (Nav + Footer excluded too)
- Removed unused .table-wrapper CSS, applied scroll directly to .prose table
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- site/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ### Step outputs | ||
|
|
||
| Each connector populates output fields. For the HTTP connector, common fields are `status`, `headers`, `body`, and `json` (the parsed JSON body). Access them with dot or bracket notation: |
There was a problem hiding this comment.
The Expressions guide uses steps.<name>.output.status as the HTTP status field, but other docs in this repo still reference output.status_code (e.g. server-guide/index.md). To avoid conflicting guidance, please standardize on the canonical output field name (or explicitly document both if both are supported).
Suggested change
| Each connector populates output fields. For the HTTP connector, common fields are `status`, `headers`, `body`, and `json` (the parsed JSON body). Access them with dot or bracket notation: | |
| Each connector populates output fields. For the HTTP connector, the canonical status field is `status` (for the numeric HTTP status code). Older examples may refer to `status_code`, which is an alias of `status`. Common HTTP fields are `status`, `headers`, `body`, and `json` (the parsed JSON body). Access them with dot or bracket notation: |
Comment on lines
+126
to
+162
| loadPagefind().then(function(pf) { | ||
| if (!pf) { | ||
| setResultsText('Search unavailable (build required)'); | ||
| return; | ||
| } | ||
|
|
||
| pf.search(query).then(function(search) { | ||
| if (search.results.length === 0) { | ||
| setResultsText('No results for "' + query + '"'); | ||
| return; | ||
| } | ||
|
|
||
| Promise.all(search.results.slice(0, 8).map(function(r) { return r.data(); })) | ||
| .then(function(items) { | ||
| while (results.firstChild) results.removeChild(results.firstChild); | ||
| items.forEach(function(item) { | ||
| var link = document.createElement('a'); | ||
| link.href = item.url; | ||
| link.className = 'block px-3 py-2 hover:bg-surface-container-high transition-colors group'; | ||
|
|
||
| var title = document.createElement('div'); | ||
| title.className = 'font-medium text-on-surface text-sm group-hover:text-primary transition-colors'; | ||
| title.textContent = (item.meta && item.meta.title) || item.url; | ||
| link.appendChild(title); | ||
|
|
||
| if (item.excerpt) { | ||
| var excerptEl = sanitizeExcerpt(item.excerpt); | ||
| excerptEl.className = 'text-xs text-on-surface-variant mt-0.5 line-clamp-2'; | ||
| link.appendChild(excerptEl); | ||
| } | ||
|
|
||
| link.addEventListener('click', closeSearch); | ||
| results.appendChild(link); | ||
| }); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Search results rendering can race: multiple pf.search(query) calls may be in-flight, and a slower response from an older query can overwrite the results for a newer query. Track a monotonically increasing request id (or compare against the latest input value) before updating the DOM so only the latest query updates the results list.
Comment on lines
+46
to
68
| <!-- Mobile bottom navigation bar --> | ||
| <nav id="mobile-bottom-nav" class="fixed bottom-0 left-0 right-0 z-50 md:hidden border-t border-outline-variant bg-surface/95 backdrop-blur-md"> | ||
| <div class="flex items-center justify-around h-14 px-2"> | ||
| <a href="/" class:list={["bottom-nav-item flex flex-col items-center gap-0.5 px-3 py-1 text-[11px] transition-colors", isHome ? "text-primary" : "text-on-surface-variant hover:text-on-surface"]}> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" stroke-width="1.5" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" d="M2.25 12l8.954-8.955a1.126 1.126 0 011.591 0L21.75 12M4.5 9.75v10.125c0 .621.504 1.125 1.125 1.125H9.75v-4.875c0-.621.504-1.125 1.125-1.125h2.25c.621 0 1.125.504 1.125 1.125V21h4.125c.621 0 1.125-.504 1.125-1.125V9.75M8.25 21h8.25"/></svg> | ||
| <span>Home</span> | ||
| </a> | ||
| {isDocs && ( | ||
| <button id="bottom-nav-docs-btn" class="bottom-nav-item flex flex-col items-center gap-0.5 px-3 py-1 text-[11px] text-on-surface-variant hover:text-on-surface transition-colors" aria-label="Documentation menu"> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" stroke-width="1.5" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" d="M3.75 6.75h16.5M3.75 12h16.5m-16.5 5.25h16.5"/></svg> | ||
| <span>Menu</span> | ||
| </button> | ||
| )} | ||
| <a href="/docs/getting-started" class:list={["bottom-nav-item flex flex-col items-center gap-0.5 px-3 py-1 text-[11px] transition-colors", isDocs ? "text-primary" : "text-on-surface-variant hover:text-on-surface"]}> | ||
| <svg class="w-5 h-5" fill="none" stroke="currentColor" stroke-width="1.5" viewBox="0 0 24 24"><path stroke-linecap="round" stroke-linejoin="round" d="M12 6.042A8.967 8.967 0 006 3.75c-1.052 0-2.062.18-3 .512v14.25A8.987 8.987 0 016 18c2.305 0 4.408.867 6 2.292m0-14.25a8.966 8.966 0 016-2.292c1.052 0 2.062.18 3 .512v14.25A8.987 8.987 0 0018 18a8.967 8.967 0 00-6 2.292m0-14.25v14.25"/></svg> | ||
| <span>Docs</span> | ||
| </a> | ||
| <a href="https://github.com/dvflw/mantle" class="bottom-nav-item flex flex-col items-center gap-0.5 px-3 py-1 text-[11px] text-on-surface-variant hover:text-on-surface transition-colors" aria-label="GitHub"> | ||
| <svg class="w-5 h-5" fill="currentColor" viewBox="0 0 24 24"><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> | ||
| <span>GitHub</span> | ||
| </a> | ||
| </div> | ||
| </nav> |
There was a problem hiding this comment.
The mobile bottom nav is fixed and will overlay page content unless every page/layout adds bottom padding. This PR adds padding for the home page and Docs layout, but other pages (e.g. 404) still use min-h-screen without a bottom offset and can be obscured on small screens. Consider moving the bottom padding into a shared layout (e.g. Base) or conditionally applying it whenever #mobile-bottom-nav is present.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Pagefind-based site search (build step and assets), a DocsSearch trigger and lazy-loading SearchModal with keyboard shortcuts and focus trapping, integrates the modal into site layouts and nav, redesigns mobile navigation (bottom bar + sidebar tweaks), updates styles and expands CEL expressions docs. Changes
Sequence DiagramsequenceDiagram
participant User
participant DocsSearch
participant SearchModal
participant Browser
participant Pagefind
User->>DocsSearch: Click trigger or press Ctrl/Meta+K
DocsSearch->>SearchModal: Open modal
SearchModal->>SearchModal: Focus input & trap Tab
User->>SearchModal: Type query
SearchModal->>SearchModal: Debounce (200ms)
SearchModal->>Browser: Ensure Pagefind loaded
alt Pagefind not loaded
Browser->>Pagefind: Import /pagefind/pagefind.js
Browser->>Pagefind: Initialize pf
end
SearchModal->>Pagefind: pf.search(query)
Pagefind-->>SearchModal: Return results
SearchModal->>SearchModal: Sanitize excerpts, limit to 8, render links
User->>SearchModal: Click result or press Escape
SearchModal->>SearchModal: Close modal, restore focus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
1. CEL link: separate external cel.dev link from internal expressions guide 2. status_code → status in server-guide (matches http.go output) 3. sanitizeExcerpt: recursive DOM walk preserves nested <mark> tags 4. Search race condition: requestId counter discards stale responses 5. Bottom padding: moved to global CSS @media (max-width: 1023px) body so all pages (including 404) clear the mobile bottom nav Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@site/src/components/DocsSearch.astro`:
- Line 15: DocsSearch.astro currently renders a macOS-only shortcut hint ("⌘K")
in the <kbd> element; change this to be cross-platform by either (A) rendering
both "⌘K / Ctrl+K" and conditionally hiding one via a small client-side platform
detection script that toggles a CSS class on the <kbd> (use navigator.platform
or navigator.userAgent to detect Mac vs Windows/Linux), or (B) render a neutral
generic hint (e.g., "Search (K)") if you prefer no platform detection; update
the <kbd> element in DocsSearch.astro and add the minimal client script and CSS
class toggling logic to switch between showing "⌘K" and "Ctrl+K" so
Windows/Linux users see Ctrl+K and macOS users see ⌘K.
In `@site/src/components/DocsSidebar.astro`:
- Around line 178-181: The external GitHub anchor in DocsSidebar.astro (the <a
href="https://github.com/dvflw/mantle" ...> element) should open in a new tab
and prevent access to window.opener; update that anchor to include
target="_blank" and rel="noopener" (or rel="noopener noreferrer") so external
navigation opens in a new tab and is secure.
In `@site/src/components/Nav.astro`:
- Around line 47-57: Add an accessible label to the new bottom nav and expose
the sidebar state by giving the <nav id="mobile-bottom-nav"> an aria-label
(e.g., "Mobile bottom navigation") and updating the docs menu button (<button
id="bottom-nav-docs-btn">) to include aria-controls="docs-sidebar-mobile" and
aria-expanded bound to the sidebar open state (the boolean you use to track the
mobile drawer, e.g., docsSidebarOpen). Also ensure the toggle handler (e.g.,
toggleDocsSidebar / the on:click that opens/closes the sidebar) updates that
same boolean so aria-expanded stays in sync with the actual state of
`#docs-sidebar-mobile`.
In `@site/src/components/SearchModal.astro`:
- Around line 5-25: The modal with id "search-modal" is marked aria-modal="true"
but does not trap focus or restore focus on close; update the open/close logic
to save document.activeElement before opening, set focus to the primary control
(the input with data-search-input) when the modal is shown, and on close restore
focus to the saved trigger; implement a keyboard focus trap by capturing
Tab/Shift+Tab inside the modal (use the container element with id "search-modal"
or the inner panel element) to cycle through tabbable elements (including
data-search-input and controls in data-search-results) and prevent Tab from
moving focus outside, and ensure Escape and backdrop click
(data-search-backdrop) close the modal and trigger the focus restoration.
- Around line 56-69: The problem is that searchRequestId only advances for
non-empty queries so stale in-flight responses can still match after the modal
is cleared/closed or reopened; in openSearch() and closeSearch() (and the
related handlers around searchRequestId at 121-145) explicitly advance or reset
searchRequestId when you clear/reset the modal state so any pending requests
become obsolete — e.g., in openSearch() after input.value = '' (or in
closeSearch()) increment or assign a new value to searchRequestId and update the
UI via setResultsText('Type to search...') so old responses no longer repopulate
results; apply the same id-bump in the other search-reset code paths referenced
(the block dealing with searchRequestId at lines ~121-145).
In `@site/src/content/docs/concepts/expressions.md`:
- Around line 55-58: The example YAML uses an invalid input type value
("integer") for the count field; update the count entry by replacing the type
value "integer" with the supported "number" (i.e., change the count field's type
key from integer to number) so the example matches the documented input type
set.
In `@site/src/content/docs/server-guide/index.md`:
- Line 74: The template uses invalid dot notation for the hyphenated step name
steps.check-api.output.status; change it to bracket notation
steps["check-api"].output.status so the CEL expression is valid — locate the
string "API health check failed with status {{ steps.check-api.output.status }}"
and replace the dotted access with the bracketed form
steps["check-api"].output.status.
In `@site/src/styles/global.css`:
- Around line 405-407: Update the CSS rule for the ".prose code" selector to
replace the deprecated "word-break: break-word" declaration with "overflow-wrap:
break-word" so the code elements wrap correctly using the supported property;
locate the ".prose code" rule and remove or replace the "word-break: break-word"
line with "overflow-wrap: break-word".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4f40f281-762b-44b8-b9a3-cb88b8ccd6d9
⛔ Files ignored due to path filters (1)
site/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
site/package.jsonsite/src/components/DocsSearch.astrosite/src/components/DocsSidebar.astrosite/src/components/Nav.astrosite/src/components/SearchModal.astrosite/src/content/docs/concepts/expressions.mdsite/src/content/docs/getting-started/data-passing.mdsite/src/content/docs/server-guide/index.mdsite/src/content/docs/workflow-reference/index.mdsite/src/layouts/Base.astrosite/src/layouts/Docs.astrosite/src/pages/404.astrosite/src/pages/index.astrosite/src/styles/global.css
Comment on lines
+405
to
+407
| .prose code { | ||
| word-break: break-word; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is word-break: break-word deprecated in CSS?
💡 Result:
Yes—the value break-word for the word-break property is deprecated (kept only for legacy compatibility). The word-break property itself is not deprecated. [1][2]
What to use instead:
- Prefer
overflow-wrap: anywhere(closest modern equivalent toword-break: break-word). [1] - Or
overflow-wrap: break-wordif you only want to break long words when necessary to prevent overflow (more conservative thananywhere). [2]
Sources:
[1] CSS Text Module Level 4 (W3C)
[2] MDN: word-break
Replace deprecated word-break: break-word with overflow-wrap: break-word.
The break-word value for word-break is deprecated per CSS specifications. Use overflow-wrap: break-word instead for the same effect.
🔧 Proposed fix
.prose code {
- word-break: break-word;
+ overflow-wrap: break-word;
}📝 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
| .prose code { | |
| word-break: break-word; | |
| } | |
| .prose code { | |
| overflow-wrap: break-word; | |
| } |
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 406-406: Unexpected deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@site/src/styles/global.css` around lines 405 - 407, Update the CSS rule for
the ".prose code" selector to replace the deprecated "word-break: break-word"
declaration with "overflow-wrap: break-word" so the code elements wrap correctly
using the supported property; locate the ".prose code" rule and remove or
replace the "word-break: break-word" line with "overflow-wrap: break-word".
- expressions.md: type: integer → type: number (matches valid input types) - global.css: word-break: break-word → overflow-wrap: anywhere (deprecated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
site/src/styles/global.css (1)
336-344: 🧹 Nitpick | 🔵 TrivialConsider wrapping the table instead of changing its display.
Using
display: blockon a<table>element breaks the native table layout algorithm, which can cause columns to not distribute widths properly. A more robust pattern is to wrap the table in a scrollable container:.prose .table-wrapper { display: block; overflow-x: auto; -webkit-overflow-scrolling: touch; }This requires adding a wrapper
<div class="table-wrapper">around tables in the Markdown renderer, but preserves proper table semantics.If adding wrappers isn't feasible, the current approach works but may produce unexpected column sizing on complex tables.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/src/styles/global.css` around lines 336 - 344, Replace the current .prose table approach with a scrollable wrapper: remove display:block from the .prose table rule (keep border-collapse, width, font-size, margins) and add a new .prose .table-wrapper class that provides display:block, overflow-x:auto and -webkit-overflow-scrolling:touch; then update the Markdown renderer/template that outputs tables to wrap table elements in <div class="table-wrapper"> so native table layout and column sizing are preserved (if wrapping tables is not possible, keep the fallback but note the risk).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@site/src/styles/global.css`:
- Around line 336-344: Replace the current .prose table approach with a
scrollable wrapper: remove display:block from the .prose table rule (keep
border-collapse, width, font-size, margins) and add a new .prose .table-wrapper
class that provides display:block, overflow-x:auto and
-webkit-overflow-scrolling:touch; then update the Markdown renderer/template
that outputs tables to wrap table elements in <div class="table-wrapper"> so
native table layout and column sizing are preserved (if wrapping tables is not
possible, keep the fallback but note the risk).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51ee3847-3b7c-44f7-9d1e-d553eb9173e7
📒 Files selected for processing (2)
site/src/content/docs/concepts/expressions.mdsite/src/styles/global.css
- Focus trap: Tab cycles within modal when open (first/last wrapping) - Restore focus: previously focused trigger gets focus back on close - Invalidate requests: searchRequestId incremented on open AND close, preventing stale responses from populating after modal reset Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
site/src/components/SearchModal.astro (2)
155-159:⚠️ Potential issue | 🟡 MinorIncrement
searchRequestIdwhen query is empty to invalidate pending requests.If the user types a query and then quickly clears the input, the in-flight search response can still pass the stale check (since
searchRequestIdwasn't incremented) and repopulate results under an empty input.🐛 Proposed fix
var query = input.value.trim(); if (!query) { + searchRequestId++; setResultsText('Type to search...'); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/src/components/SearchModal.astro` around lines 155 - 159, When handling the cleared input in SearchModal.astro, increment the shared searchRequestId to invalidate any in-flight searches before returning; specifically, inside the branch that checks if (!query) update searchRequestId++ (or call the same increment logic used when issuing searches), then call setResultsText('Type to search...') and return so stale fetch responses won't repopulate results. Ensure you reference the same searchRequestId variable used by the async search response check.
71-79: 🧹 Nitpick | 🔵 TrivialAdd
clearTimeout(debounceTimer)incloseSearchto cancel pending searches.The focus restoration and
searchRequestId++are correctly implemented, addressing the previous review. However, without clearing the debounce timer, a pending search callback can still fire after the modal closes, triggering an unnecessary network request and Pagefind operation.♻️ Proposed fix
function closeSearch() { var modal = getModal(); if (modal) modal.classList.add('hidden'); searchRequestId++; // invalidate any pending requests + clearTimeout(debounceTimer); if (previousFocus && previousFocus.focus) { previousFocus.focus(); previousFocus = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/src/components/SearchModal.astro` around lines 71 - 79, The closeSearch function should cancel any pending debounced search to prevent callbacks firing after the modal is closed: inside closeSearch (which already calls getModal(), toggles hidden, increments searchRequestId and restores previousFocus), add a clearTimeout call for the debounceTimer and reset debounceTimer (so any scheduled search callback is cancelled) before invalidating requests; reference the closeSearch function and the debounceTimer variable when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@site/src/components/SearchModal.astro`:
- Around line 155-159: When handling the cleared input in SearchModal.astro,
increment the shared searchRequestId to invalidate any in-flight searches before
returning; specifically, inside the branch that checks if (!query) update
searchRequestId++ (or call the same increment logic used when issuing searches),
then call setResultsText('Type to search...') and return so stale fetch
responses won't repopulate results. Ensure you reference the same
searchRequestId variable used by the async search response check.
- Around line 71-79: The closeSearch function should cancel any pending
debounced search to prevent callbacks firing after the modal is closed: inside
closeSearch (which already calls getModal(), toggles hidden, increments
searchRequestId and restores previousFocus), add a clearTimeout call for the
debounceTimer and reset debounceTimer (so any scheduled search callback is
cancelled) before invalidating requests; reference the closeSearch function and
the debounceTimer variable when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 016ef4ff-7e61-4b9b-a22c-850387715c24
📒 Files selected for processing (1)
site/src/components/SearchModal.astro
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Verified each finding against current code — 4 needed fixes, 4 already done: Fixed: 1. DocsSearch: cross-platform kbd (⌘K on Mac, Ctrl+K on Windows/Linux) 2. DocsSidebar: GitHub link target="_blank" rel="noopener" 3. Nav: aria-label on bottom nav, aria-controls + aria-expanded on menu button, synced with toggle handler 4. server-guide: bracket notation for steps['check-api'] on line 74 Already fixed (verified): - SearchModal focus trap (previousFocus, Tab cycling) ✓ - SearchModal requestId invalidation on open/close ✓ - expressions.md type: number (not integer) ✓ - CSS overflow-wrap: anywhere (not word-break: break-word) ✓ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
site/src/components/Nav.astro (1)
86-104: 🧹 Nitpick | 🔵 TrivialConsider syncing
aria-expandedon initialization.The toggle handler correctly updates
aria-expandedon user interaction, but there's no sync on init. If the sidebar's initial state ever differs from closed (e.g., persisted state), the attribute would be stale until the first click.♻️ Optional: Add init sync
var btn = document.getElementById('bottom-nav-docs-btn'); var docsSidebar = document.getElementById('docs-sidebar-mobile'); if (!btn || !docsSidebar) return; + btn.setAttribute('aria-expanded', String(!docsSidebar.classList.contains('-translate-x-full'))); btn.onclick = function() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@site/src/components/Nav.astro` around lines 86 - 104, Sync aria-expanded (and overlay visibility) on init inside initNav: after locating btn, docsSidebar, and overlay, compute isOpen = !docsSidebar.classList.contains('-translate-x-full') and set btn.setAttribute('aria-expanded', isOpen ? 'true' : 'false'); also update overlay (add/remove 'hidden' and set aria-hidden to 'true'/'false') to match the initial isOpen state so the DOM reflects the sidebar's initial visibility before any clicks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@site/src/components/Nav.astro`:
- Around line 86-104: Sync aria-expanded (and overlay visibility) on init inside
initNav: after locating btn, docsSidebar, and overlay, compute isOpen =
!docsSidebar.classList.contains('-translate-x-full') and set
btn.setAttribute('aria-expanded', isOpen ? 'true' : 'false'); also update
overlay (add/remove 'hidden' and set aria-hidden to 'true'/'false') to match the
initial isOpen state so the DOM reflects the sidebar's initial visibility before
any clicks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98e0ef3c-f8b2-4f29-8473-aaf77ce9b2ca
📒 Files selected for processing (3)
site/src/components/DocsSearch.astrosite/src/components/Nav.astrosite/src/content/docs/server-guide/index.md
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
michaelmcnees
added a commit
that referenced
this pull request
Mar 24, 2026
Keep both budget ResetDay validation and sslmode=prefer warning in config.go. Take main's version for all site component files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 24, 2026
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
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
Changes
CEL Expressions (closes #1)
concepts/expressions.mdwith full reference: template vs bare syntax, all variable namespaces, common expression patterns, type safety, limitationsDocs Search (closes #5)
Cmd+K/Ctrl+Kkeyboard shortcutTest plan
npm run buildpasses (Astro build + Pagefind indexing)Summary by CodeRabbit
New Features
Improvements