fix: canonical logo, robust groundedness, richer citations, delete#13
Conversation
- Dashboard sidebar uses the canonical VectorlessIcon (blue tile, white V, pink focal dot) instead of the recolored dot. - Playground groundedness: LLM-extracted quotes aren't byte-exact, so strict substring matching gave false 0%. Accept exact OR >=80% word overlap against the cited section. - Playground citations show pages AND section count; answer body renders in readable ink (not muted) with relaxed leading. - Delete document: defer window.confirm() past Radix's menu-close so it isn't suppressed (the reason delete appeared to do nothing).
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR contains four independent small changes: deferring the document-delete confirmation via onSelect/setTimeout, adding a more tolerant word-overlap grounding check in the playground grounding API, swapping the sidebar brand icon to VectorlessIcon, and adjusting citation section-count display and assistant markdown styling in ChatThread. ChangesDocument Delete Interaction
Grounding Check Tolerance
Sidebar Brand Icon
Chat Citation and Markdown Display
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/(dashboard)/dashboard/documents/page.tsxOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) apps/web/app/api/dashboard/playground/grounding/route.tsOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25) apps/web/components/dashboard/AppSidebar.tsxOops! Something went wrong! :( ESLint: 9.39.1 TypeError: Converting circular structure to JSON ... [truncated 455 characters] ... c/dist/eslintrc.cjs:3261:25)
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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/components/dashboard/AppSidebar.tsx (1)
122-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
rounded-lgis redundant given the icon's built-inrx="8"background rect.Since
VectorlessIconalready draws its own rounded background (rect ... rx="8"), applyingrounded-lgto the outer<svg>element has no visible effect beyond the icon's own shape. Not a functional issue, just a leftover from the previous wrapper-based styling.🤖 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/web/components/dashboard/AppSidebar.tsx` at line 122, The `VectorlessIcon` in `AppSidebar` already renders a rounded background via its internal `rect` with `rx="8"`, so the outer `rounded-lg` class on the icon is redundant. Update the `AppSidebar` JSX to remove that extra class from the `VectorlessIcon` usage and keep the styling aligned with the icon’s built-in shape.apps/web/app/api/dashboard/playground/grounding/route.ts (1)
30-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWord-overlap check is substring-based, not word-boundary aware — risks false positives.
hay.includes(w)at Line 37 matcheswas a raw substring anywhere in the normalized source, not as a whole word. Two compounding effects:
- Common short words (len 3-4, e.g. "the", "and", "for") pass the
length > 2filter and will almost always be found in any prose, inflating overlap for otherwise unrelated quotes.includesalso matches inside unrelated longer words (e.g."cat"inside"category"), further loosening the check.For longer quotes padded with filler words, this could push overlap past 80% even when the quote isn't actually grounded — undermining the faithfulness signal the endpoint is meant to provide.
Consider a stopword exclusion list and/or word-boundary matching (e.g. split source into a word set and do exact set membership, or regex
\bword\b) to keep the tolerance targeted at genuine near-matches rather than incidental substring hits.💡 Possible tightening
function isGrounded(quote: string, source: string): boolean { const hay = norm(source); if (!hay) return false; const needle = norm(quote); if (hay.includes(needle)) return true; - const words = needle.split(" ").filter((w) => w.length > 2); + const hayWords = new Set(hay.split(" ")); + const words = needle.split(" ").filter((w) => w.length > 2); if (words.length === 0) return false; - const hit = words.filter((w) => hay.includes(w)).length; + const hit = words.filter((w) => hayWords.has(w)).length; return hit / words.length >= 0.8; }🤖 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/web/app/api/dashboard/playground/grounding/route.ts` around lines 30 - 39, The grounding check in isGrounded is too permissive because it uses raw substring matching for overlap, which can count stopwords and partial word matches as evidence. Tighten the matching logic in isGrounded by excluding common short/stop words and switching to whole-word comparison using a token set or word-boundary matching so only genuine near-matches contribute to the overlap score. Keep the existing normalization flow and update the hit calculation to use exact word matches rather than hay.includes(w).
🤖 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/web/app/`(dashboard)/dashboard/documents/page.tsx:
- Around line 322-328: The delete action in the document menu currently prevents
Radix from auto-closing, so the dropdown can stay open when confirmation is
canceled or the delete fails. Update the onSelect flow in the menu action around
handleDelete so the menu is explicitly closed after the delete attempt settles,
while still deferring long enough for window.confirm() to work. Use the existing
handleDelete and doc.doc_id path to place the close behavior alongside the
delete flow rather than relying on row unmounting from mutate.
In `@apps/web/components/playground/ChatThread.tsx`:
- Around line 63-68: The Markdown styling in ChatThread should use Tailwind v4
suffix important modifiers instead of the old prefix form. Update the className
passed to Markdown so the existing prose-p and prose-li overrides in ChatThread
use the new suffix-style important syntax, matching the reviewer’s examples, and
keep the same text color and leading utilities.
---
Nitpick comments:
In `@apps/web/app/api/dashboard/playground/grounding/route.ts`:
- Around line 30-39: The grounding check in isGrounded is too permissive because
it uses raw substring matching for overlap, which can count stopwords and
partial word matches as evidence. Tighten the matching logic in isGrounded by
excluding common short/stop words and switching to whole-word comparison using a
token set or word-boundary matching so only genuine near-matches contribute to
the overlap score. Keep the existing normalization flow and update the hit
calculation to use exact word matches rather than hay.includes(w).
In `@apps/web/components/dashboard/AppSidebar.tsx`:
- Line 122: The `VectorlessIcon` in `AppSidebar` already renders a rounded
background via its internal `rect` with `rx="8"`, so the outer `rounded-lg`
class on the icon is redundant. Update the `AppSidebar` JSX to remove that extra
class from the `VectorlessIcon` usage and keep the styling aligned with the
icon’s built-in shape.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7e2155e-2199-4d79-8eab-5448c56e03b6
📒 Files selected for processing (4)
apps/web/app/(dashboard)/dashboard/documents/page.tsxapps/web/app/api/dashboard/playground/grounding/route.tsapps/web/components/dashboard/AppSidebar.tsxapps/web/components/playground/ChatThread.tsx
| onSelect={(e) => { | ||
| // Defer past Radix's menu-close/focus-restore | ||
| // so window.confirm() isn't suppressed (the | ||
| // reason delete appeared to do nothing). | ||
| e.preventDefault(); | ||
| setTimeout(() => handleDelete(doc.doc_id), 0); | ||
| }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '90,170p' apps/web/app/'(dashboard)'/dashboard/documents/page.tsx && printf '\n---\n' && sed -n '300,350p' apps/web/app/'(dashboard)'/dashboard/documents/page.tsxRepository: hallelx2/vectorless
Length of output: 5239
Close the dropdown after the delete flow settles. preventDefault() keeps Radix from auto-closing this menu, so canceling confirm() or a failed DELETE leaves it open until the user clicks away. The success path only looks fine because the row unmounts on mutate. apps/web/app/(dashboard)/dashboard/documents/page.tsx:322-328
🤖 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/web/app/`(dashboard)/dashboard/documents/page.tsx around lines 322 -
328, The delete action in the document menu currently prevents Radix from
auto-closing, so the dropdown can stay open when confirmation is canceled or the
delete fails. Update the onSelect flow in the menu action around handleDelete so
the menu is explicitly closed after the delete attempt settles, while still
deferring long enough for window.confirm() to work. Use the existing
handleDelete and doc.doc_id path to place the close behavior alongside the
delete flow rather than relying on row unmounting from mutate.
| {m.answer && ( | ||
| <Markdown | ||
| content={m.answer} | ||
| className="prose-p:!text-foreground/90 prose-li:!text-foreground/90 prose-p:!leading-relaxed" | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Tailwind version references ==\n'
rg -n '"tailwindcss"|tailwindcss' package.json pnpm-lock.yaml yarn.lock package-lock.json apps/web -g '!**/node_modules/**' || true
printf '\n== ChatThread.tsx excerpt ==\n'
sed -n '55,75p' apps/web/components/playground/ChatThread.tsx
printf '\n== Any obvious Tailwind config files ==\n'
fd -a 'tailwind*' . | sed -n '1,120p'Repository: hallelx2/vectorless
Length of output: 10135
🌐 Web query:
Tailwind CSS v4 important modifier syntax suffix utility! variants documentation
💡 Result:
In Tailwind CSS v4, the syntax for applying the!important modifier has changed to improve consistency with native CSS [1]. Important Modifier Syntax In Tailwind CSS v4, the!important modifier is applied by adding a "!" suffix to the end of a utility class (e.g., text-red-500!) [1][2]. This replaces the v3 syntax of prefixing the utility with an exclamation mark (e.g.,!text-red-500) [1][2]. To apply!important to all utility declarations globally, you can add the important flag to your CSS import statement [3]: @import "tailwindcss" important; Variants Variants (such as hover:, focus:, md:, etc.) continue to be applied by prefixing them to the utility class, just as they were in previous versions [4]. You can stack multiple variants by chaining them (e.g., hover:md:text-blue-500) [5]. Summary of Key Changes - Individual Utility Modifier: Changed from prefix (!) to suffix (!) [1][2]. - Global Important: Now configured via the @import statement rather than tailwind.config.js (though the config option remains for backwards compatibility) [3]. - Variants: The prefix syntax for variants (variant:utility) remains unchanged [4].
Citations:
- 1: https://stackoverflow.com/questions/75664539/tailwind-css-class-precedence-is-not-respected
- 2: https://medium.com/villa-plus-engineering/upgrading-to-tailwind-css-v4-a-practical-guide-5faaeb8ef25b
- 3: Add support for
importantin v4 tailwindlabs/tailwindcss#14448 - 4: https://tailwindcss.com/docs/hover-focus-and-other-states
- 5: https://tailwindcss.com/docs/adding-custom-styles
Tailwind v4 important modifiers need suffix syntax here.
prose-p:!text-foreground/90, prose-li:!text-foreground/90, and prose-p:!leading-relaxed use the old prefix form, so these overrides won’t apply in this Tailwind 4 setup. Switch them to prose-p:text-foreground/90! prose-li:text-foreground/90! prose-p:leading-relaxed!.
🤖 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/web/components/playground/ChatThread.tsx` around lines 63 - 68, The
Markdown styling in ChatThread should use Tailwind v4 suffix important modifiers
instead of the old prefix form. Update the className passed to Markdown so the
existing prose-p and prose-li overrides in ChatThread use the new suffix-style
important syntax, matching the reviewer’s examples, and keep the same text color
and leading utilities.
1 participant
confirm()past Radix menu-close so it fires (was silently suppressed).Summary by CodeRabbit
Bug Fixes
New Features
Style