Redesign add romanian - #28
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (19)
WalkthroughThis PR adds app-wide i18n and locale resolution, localizes route metadata and UI, rewires root and routes (root → /feed, /bookmarks → /salvate, /dashboard → auth entry), introduces authenticated pages (/activitate, /salvate, /profil), updates navigation chrome (header/footer/mobile tab bar), changes sign-in/up redirect defaults to /activitate, and implements backend embedding-dimension and vector-search budgeting + clustering job-state changes. ChangesNavigation and Dashboard Restructuring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/bookmark-button.tsx (1)
46-55: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider simplifying the type guard using optional chaining.
The defensive type checking is appropriate given the alpha version of
@convex-dev/react-query, but the current implementation is quite verbose. Consider using optional chaining for improved readability:♻️ Cleaner alternative using optional chaining
- onSuccess: (data) => { - if ( - typeof data === "object" && - data !== null && - "bookmarked" in data && - data.bookmarked === true - ) { + onSuccess: (data) => { + if (data?.bookmarked === true) { toast.success("Bookmarked"); } else { toast("Bookmark removed"); } },🤖 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/src/components/bookmark-button.tsx` around lines 46 - 55, Replace the verbose defensive type guard with optional chaining to simplify the check: instead of the multiple checks around the local variable data, use a single condition like data?.bookmarked === true to decide between toast.success("Bookmarked") and toast("Bookmark removed"); update the conditional in the BookmarkButton component (the block referencing data and toast) so behavior remains identical but the guard is concise and readable.
🤖 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/src/components/profile/AuthenticatedProfile.tsx`:
- Around line 34-42: The handleSignOut function currently awaits
authClient.signOut but has no error handling; wrap the call in a try/catch (or
add .catch) around authClient.signOut inside handleSignOut, keep the existing
fetchOptions.onSuccess behavior, and in the catch block log the error (e.g.,
console.error) and surface user feedback (e.g., set an error state or call a
toast/alert) so failures (network/auth errors) don’t silently fail and the page
is not reloaded on error. Reference: handleSignOut and authClient.signOut.
In `@apps/web/src/routes/dashboard.tsx`:
- Around line 63-71: The effect showing the verification toast immediately calls
replaceDashboardSearch("signin") which currently does a full
window.location.replace and tears down the Sonner toaster before the toast
renders; change the flow in the useEffect handling isVerified (and using
hasShownVerifiedToastRef) to first show the toast via toast.message and then
update the URL/search using a client-side search updater (not full reload) that
explicitly removes the verified query param (instead of implicitly dropping it),
or defer/await navigation until after the toast has had a chance to render;
update replaceDashboardSearch (or its consumer) to perform a search-only update
(e.g., using navigate or updateSearchParams) that strips verified rather than
calling window.location.replace so the toast remains visible.
- Around line 39-61: The current useEffect and replaceDashboardSearch functions
use window.location.replace which forces full reloads; change them to use
TanStack Router's navigation via useNavigate and the router search updater: in
the component, call const navigate = useNavigate() and replace the
isAuthenticated redirect in useEffect to navigate({ to: "/activitate" }, {
replace: true }) (or use the router's push with replace option), and rewrite
replaceDashboardSearch to build a URLSearchParams but call navigate({ to:
"/dashboard", search: (prev) => { const s = new URLSearchParams(prev);
s.set("mode", mode); if (search.redirect) s.set("redirect", search.redirect); if
(verified !== undefined) s.set("verified", String(verified)); return
s.toString(); } }, { replace: true }) so navigation stays client-side and the
verification toast can render.
In `@apps/web/src/routes/reset-password.tsx`:
- Line 73: Replace the hard navigation call
window.location.assign("/dashboard?mode=signin") with TanStack Router's
useNavigate to preserve SPA routing: import and call useNavigate() (e.g., const
navigate = useNavigate()) in the component and replace the assign call with
navigate({ to: "/dashboard", search: { mode: "signin" } }) or navigate({ to:
"/dashboard?mode=signin" }) to match the existing navigate usage in this file
(see the other navigate call used around line 111); ensure the useNavigate
import is added where hooks are declared.
---
Outside diff comments:
In `@apps/web/src/components/bookmark-button.tsx`:
- Around line 46-55: Replace the verbose defensive type guard with optional
chaining to simplify the check: instead of the multiple checks around the local
variable data, use a single condition like data?.bookmarked === true to decide
between toast.success("Bookmarked") and toast("Bookmark removed"); update the
conditional in the BookmarkButton component (the block referencing data and
toast) so behavior remains identical but the guard is concise and readable.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c9d3cb77-62a2-4059-a0e1-59dbea6c31b2
⛔ Files ignored due to path filters (1)
apps/web/src/routeTree.gen.tsis excluded by!**/routeTree.gen.ts
📒 Files selected for processing (26)
apps/web/src/components/SignInPrompt.tsxapps/web/src/components/auth-prompt-banner.tsxapps/web/src/components/bookmark-button.tsxapps/web/src/components/header.tsxapps/web/src/components/layout/Footer.tsxapps/web/src/components/layout/MobileTabBar.tsxapps/web/src/components/profile/AnonymousProfile.tsxapps/web/src/components/profile/AuthenticatedProfile.tsxapps/web/src/components/sign-in-form.tsxapps/web/src/components/sign-up-form.tsxapps/web/src/lib/auth-redirect.tsapps/web/src/routes/__root.tsxapps/web/src/routes/activitate.tsxapps/web/src/routes/bookmarks.tsxapps/web/src/routes/contact.tsxapps/web/src/routes/cum-functioneaza.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/despre.tsxapps/web/src/routes/index.tsxapps/web/src/routes/parteneri.tsxapps/web/src/routes/politica-confidentialitate.tsxapps/web/src/routes/profil.tsxapps/web/src/routes/reset-password.tsxapps/web/src/routes/salvate.tsxapps/web/src/routes/sursele-noastre.tsxapps/web/src/routes/termeni.tsx
Owner
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/web/src/components/user-menu.tsx (1)
21-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd error handling for sign-out failures.
The
handleSignOutfunction has no error handling. IfauthClient.signOut()fails due to network issues or server errors, the user receives no feedback and may incorrectly believe they are signed out.🛡️ Suggested fix with error handling
const handleSignOut = async () => { - await authClient.signOut({ - fetchOptions: { - onSuccess: () => { - location.reload(); - }, - }, - }); + try { + await authClient.signOut({ + fetchOptions: { + onSuccess: () => { + location.reload(); + }, + onError: (ctx) => { + console.error("Sign out failed:", ctx.error); + // TODO: Show user-facing error message (toast/alert) + alert(t("auth.signOutError") || "Failed to sign out. Please try again."); + }, + }, + }); + } catch (error) { + console.error("Sign out error:", error); + // Fallback error handling + alert(t("auth.signOutError") || "Failed to sign out. Please try again."); + } };🤖 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/src/components/user-menu.tsx` around lines 21 - 29, The handleSignOut function lacks error handling; wrap the await authClient.signOut(...) call in a try/catch inside handleSignOut, keep the existing fetchOptions.onSuccess to call location.reload() on success, and in the catch block surface a user-facing error (e.g., dispatch a toast/snackbar or set an error state) and log the error (console.error or existing logger) so network/server failures give feedback and can be diagnosed; reference handleSignOut and authClient.signOut when making this change.apps/web/src/components/ui/page-loading-state.tsx (1)
7-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefault prop values prevent i18n fallbacks from executing.
The default parameter values on lines 7-8 (
"Loading your page"and"Pulling in the latest view...") are hardcoded English strings that will always be used when the props are omitted. The nullish coalescing operator (??) on lines 16-17 will never trigger the translated fallbacks becausetitleanddescriptionwill never beundefined—they'll be the default English strings instead.This defeats the purpose of the i18n implementation.
🌐 Proposed fix to enable i18n fallbacks
export function PageLoadingState({ - title = "Loading your page", - description = "Pulling in the latest view and getting things ready.", + title, + description, cardCount = 2, }: {Also applies to: 16-17
🤖 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/src/components/ui/page-loading-state.tsx` around lines 7 - 8, The hardcoded English defaults for the component props title and description prevent the i18n fallbacks from ever running; remove those default parameter values (or set them to undefined) in the PageLoadingState component signature so that the nullish coalescing used later (the title ?? ... and description ?? ... expressions) can trigger the translated fallbacks; update the component’s props (title, description) to be optional and rely on the existing ?? translations instead of supplying English literals as defaults.apps/web/src/components/share-event-button.tsx (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead
summaryprop and duplicatedtitle/textpayload innavigator.share.Two related issues:
- The
summaryprop is still exported in the component's public type and is still computed and passed by callers (e.g.,apps/web/src/routes/event.$slug.tsxlines 289–291), but the implementation now discards it (_summary). That's an API/UX regression: the share sheet no longer carries the perspective summary at all. If discarding was intentional, remove the prop so callers stop wasting work; otherwise restore it as the sharetext.navigator.share({ title, text: title, ... })sends the same string twice. Some OS share targets rendertitleandtextas separate fields and will show duplicate content.🛠️ Suggested fix (restore summary in share text and drop the dead destructure)
export default function ShareEventButton({ eventId, interactionContext, slug, title, - summary: _summary, + summary, size = "default", className, }: ShareEventButtonProps) { @@ try { await navigator.share({ title, - text: title, + text: summary ?? title, url: shareUrl, });If discarding
summaryis intentional, remove it fromShareEventButtonPropsand stop passing it fromevent.$slug.tsx.Also applies to: 31-31, 63-66
🤖 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/src/components/share-event-button.tsx` at line 21, The component currently destructures and ignores the summary prop (exported on ShareEventButtonProps) and calls navigator.share with duplicate values (text: title), so either restore summary into the shared text or remove the prop entirely; update the ShareEventButton component by removing the dead "_summary" destructure and use the actual summary prop when building the share payload (navigator.share({ title, text: summary ?? title, url })) so title and text are not identical, and if you intend to drop summary instead, remove it from ShareEventButtonProps and from callers like the code in event.$slug.tsx to stop computing/passing it.apps/web/src/routes/unsubscribe.tsx (1)
53-58: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
useEffectis missingunsubscribe(the mutation object) from its dependency array.The closure captures
unsubscribe.reset. While the mutation object reference is typically stable from@tanstack/react-query, exhaustive-deps lint will flag this. Either addunsubscribeto deps or destructure and depend on the stableresetcallback.🤖 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/src/routes/unsubscribe.tsx` around lines 53 - 58, The useEffect closure references unsubscribe.reset but does not include unsubscribe in the dependency array, which can trigger exhaustive-deps lint; update the effect to either add unsubscribe to the deps or destructure the stable callback (e.g., const { reset } = unsubscribe) and depend on that reset instead—modify the effect that uses useEffect, unsubscribe.reset, setLastEmail, email, and lastEmail so the dependency array includes the chosen stable reference (unsubscribe or reset) to satisfy the linter and ensure correct behavior.apps/web/src/routes/reset-password.tsx (1)
152-156:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winField validation errors lack a live-region announcement.
The per-field error
<p>blocks at lines 152–156 (and 182–186) render synchronously when validation fails on submit but are not wrapped withrole="alert"oraria-live. Screen reader users won't be notified of the new error. Consider rendering the error container withrole="alert"(or wrapping the field group witharia-live="polite").As per coding guidelines: "Forms must have
aria-labelon inputs and ARIA live regions (aria-live,role) on status messages".🤖 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/src/routes/reset-password.tsx` around lines 152 - 156, The per-field error messages rendered from field.state.meta.errors are not announced to screen readers; update the error markup (the <p> elements mapped from field.state.meta.errors) to be an ARIA live region — e.g., add role="alert" (or wrap the field error group with aria-live="polite") so assistive tech announces validation failures, and also ensure the corresponding input elements in the same field group include an accessible label (aria-label or linked <label>) to satisfy the form guidelines; locate the error renderings around the field.state.meta.errors map and the associated input components to make these changes.apps/web/src/components/sign-in-form.tsx (1)
16-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace hardcoded localhost fallback with app origin from
SITE.url.
getPasswordResetRedirectURL()usesredirectFallbackOrigin = "http://localhost:3001"whenwindowis undefined (server-side rendering). If password-reset emails are generated server-side, this localhost URL will be embedded in production emails. Instead of introducing a new env var, importSITEfrom@/lib/seoand useSITE.urlfor consistency with the codebase pattern:-function getPasswordResetRedirectURL() { - if (typeof window === "undefined") { - return `${redirectFallbackOrigin}/reset-password`; - } - return `${window.location.origin}/reset-password`; -} - -const redirectFallbackOrigin = "http://localhost:3001"; +import { SITE } from "@/lib/seo"; + +function getPasswordResetRedirectURL() { + if (typeof window === "undefined") { + return `${SITE.url}/reset-password`; + } + return `${window.location.origin}/reset-password`; +}🤖 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/src/components/sign-in-form.tsx` around lines 16 - 23, Replace the hardcoded redirectFallbackOrigin used by getPasswordResetRedirectURL with the app origin from SITE.url: import SITE from "@/lib/seo" (or destructure SITE.url) and when typeof window === "undefined" return `${SITE.url}/reset-password` instead of using redirectFallbackOrigin; remove or replace the redirectFallbackOrigin constant so server-side rendered password-reset links use SITE.url consistently with the codebase.
🤖 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/src/components/feed/event-claim-comparison.tsx`:
- Around line 179-189: The JSX repeatedly branches on the same status values to
call t(...) causing duplication and potential label drift; refactor by
extracting a single source of truth (e.g., a mapping object or helper function
like getStatusLabel(status) inside the EventClaimComparison component or its
module) that returns the correct translation key (using the existing t function)
and replace all duplicated conditional chains (the status → t(...) logic used
for badge, title, and body — and the similar block around lines 458-481) with
calls to that helper so all places share one mapping.
- Around line 314-318: The loading status container in the EventClaimComparison
component (the div with className "flex items-center gap-3") lacks live-region
semantics; update that container to include role="status" and aria-live="polite"
(optionally aria-atomic="true") so screen readers announce the
"{t('claim.loading')}" message reliably; locate the div in
event-claim-comparison.tsx and add those attributes to the element that wraps
the spinner and loading text.
In `@apps/web/src/components/layout/MobileTabBar.tsx`:
- Around line 36-38: The current matchesPath(pathname: string, to: string) uses
pathname.startsWith(`${to}/`) which can false-positive match routes like
'/feed-archive'; update matchesPath to support an exact-only mode: add an
optional parameter (e.g., allowPrefix: boolean = false) or accept a set of
leaf-only routes and only apply the startsWith check when allowPrefix is true or
when to === '/feed'; for leaf tab targets like '/salvate', '/activitate',
'/profil' always require exact equality (pathname === to) and reserve the prefix
match for '/feed' (or other explicitly allowed parent routes) so tabs only
activate for true children of those parents.
In `@apps/web/src/components/profile/AuthenticatedProfile.tsx`:
- Around line 170-172: The "Delete account" button currently renders <Button
asChild ...><Link to="/contact">{t("profile.deleteAccount")}</Link></Button> but
routes to /contact; either change the label to match the contact flow or
implement a real deletion flow. Option A: rename the translation key used
(replace t("profile.deleteAccount") with t("profile.requestDeletion") and add a
matching i18n string) and update the Link target to a dedicated
"/request-deletion" or keep "/contact" if desired. Option B: implement a proper
self-serve deletion by replacing the Link with a button that calls a new handler
(e.g., onDeleteAccount or showDeleteConfirmation) which opens a confirmation
modal (confirmDeletionModal), then calls a backend API (e.g., deleteAccountApi
or accounts.deleteAccount) to perform deletion, logs the action to audit
(audit.log or createAuditEntry), handles errors via process/error toast, and
redirects or signs out on success; ensure to include CSRF/auth headers and a
clear UI confirmation step.
- Around line 66-69: The avatar fallback currently uses the literal "B" in
AuthenticatedProfile (see the initials || "B" expression) which leaks a brand
placeholder; replace that with a deterministic fallback using the first
character of the user's email (e.g., derive from user.email[0].toUpperCase()) or
render a neutral icon when email is missing/invalid. Update the JSX where
getInitials(displayName) result (initials) is used so it falls back to the
computed email-first-letter or the neutral icon, and ensure you reference
displayName/user.email and the initials variable in the change.
- Line 54: In the AuthenticatedProfile component replace the deprecated Tailwind
class name: update the div whose className contains "bg-gradient-to-b
from-background via-background to-muted/20" to use "bg-linear-to-b" instead of
"bg-gradient-to-b" so it matches the Tailwind v4.1 naming (keep the other
classes unchanged).
In `@apps/web/src/components/sign-in-form.tsx`:
- Around line 70-76: The toast currently shows raw server text from onError
(error.error?.message ?? error.error?.statusText ?? t("auth.signInError")),
which can leak English-only messages to other locales; update the onError
handler in sign-in-form (the onError callback that calls toast.error) to map
known server error identifiers/messages to translation keys via t(...) (e.g.,
match error.error?.code or normalized error.error?.message to keys like
"auth.invalidCredentials") and only fall back to the raw server text as a last
resort or use a generic t("auth.signInError") fallback, ensuring toast.error
always prefers localized strings.
- Around line 245-252: The spinner branch currently builds the label by
concatenating t("auth.signIn") + "..." which is brittle for i18n; add a new
translation key (e.g., "auth.signingIn") and update the component to use
t("auth.signingIn") when state.isSubmitting is true (the branch that renders
Loader2 and the label), leaving resolvedSubmitLabel for the non-submitting
branch so the UI uses a proper localized "Signing in…" string rather than
appending "..." to t("auth.signIn").
In `@apps/web/src/components/sign-up-form.tsx`:
- Line 200: The label currently builds the in-flight text by appending a literal
ellipsis to the translated create-account string
(`{t("auth.createAccount")}...`) which is not natural across locales; add a new
translation key `auth.creatingAccount` (and update locale files) and replace the
concatenation in the SignUpForm component (where `{t("auth.createAccount")}...`
is used) with `t("auth.creatingAccount")` so each locale can supply a proper
submitting phrase; ensure any related UI/state that toggles submitting uses the
new key consistently.
In `@apps/web/src/lib/i18n/LocaleContext.tsx`:
- Line 2: useT duplicates the string resolution logic from strings.ts causing
potential lookup/fallback drift; replace its internal lookup with the shared
getString helper so all resolution and fallback behavior is centralized. Locate
the useT implementation in LocaleContext.tsx and remove the manual
STRINGS[locale][key] checks, calling getString(locale, key) (or the exported
helper name from strings.ts) instead; ensure types still use Locale and
StringKey and update the other duplicated resolution spots (noted around lines
25-27) to also delegate to getString so fallback behavior remains consistent
across the app.
In `@apps/web/src/lib/i18n/resolveLocale.ts`:
- Around line 31-39: The current Accept-Language parsing in resolveLocale (the
codes mapping and loop that checks SUPPORTED and returns a Locale) ignores q=
weights and uses header order; update the logic to parse each header part's
quality value (parse the ;q=... token, defaulting to 1), map to the primary
language subtag as you already do, build an array of {code, weight}, sort that
array by weight descending, then iterate the sorted list to return the first
entry where SUPPORTED.includes(code as Locale). Ensure you still filter out
falsy codes and preserve casting to Locale when returning.
In `@apps/web/src/routes/__root.tsx`:
- Around line 134-145: The empty catch around the call to
ctx.context.convexQueryClient.serverHttpClient?.query(api.user.getCurrentUser,
{}) swallows real errors—capture the error (e.g. catch (err)) and log it at
debug/warn level before falling back to userPreference = null so ops can see
Convex/auth/schema failures; for example, replace the empty catch with a catch
that logs a clear message and the error (e.g. console.warn or your app logger)
and then sets userPreference = null, referencing the serverHttpClient.query and
api.user.getCurrentUser call.
- Around line 109-129: The client-branch in beforeLoad currently always calls
getServerLocale(...) (with userPreference: null) causing a server round-trip on
every navigation; change it to reuse the SSR-resolved locale when unchanged by
comparing getExistingAuthContext(ctx.matches).locale to the current client
cookie or ?lang= param and only call getServerLocale/resolveLocale when the
bv_locale cookie or query lang differs (or when the signed-in user's
preferredLanguage changed), and ensure you read the actual userPreference for
signed-in users instead of hardcoding null; also wire the LanguagePicker to
explicitly invalidate or update the cached client-side locale/store when it
flips the cookie so navigation doesn't trigger unnecessary server hits.
In `@apps/web/src/routes/activitate.tsx`:
- Around line 127-136: The effect using useEffect reads fields from
currentSettings but lists their optional-chained fields in the dependency array;
replace those dependencies with a single currentSettings (or stable destructured
values) so the effect reliably runs when the settings object changes. Update the
dependency array for the useEffect that calls setMinScoreInput,
setConfidenceRatioInput, and setMaxTopicsInput to depend on currentSettings (or
on explicitly memoized/destructured stable values) instead of
currentSettings?.minScore/currentSettings?.confidenceRatio/currentSettings?.maxTopics.
- Around line 712-716: The save-config feedback in activitate.tsx (the JSX that
renders {configMessage}) isn't exposed to assistive tech; update the element
that renders configMessage (used after handleSaveConfig) to be an ARIA live
region by adding attributes such as role="status" and aria-live="polite" (or
aria-live="assertive" for errors) so screen readers announce success/error;
ensure the same wrapper element remains visually unchanged and only augments
accessibility attributes.
- Around line 740-751: The map callbacks for topics use `(t) => t.displayName`
which shadows the `t` returned by `useT()` in the rendering scope; update the
two calls that pass topics (the event.attachedTopics.map and
event.inferredTopics.map used when rendering TopicChipList) to use a different
parameter name (e.g., `topic` or `topicItem`) so they no longer shadow `useT()`
(which is referenced as `t` in AuthorizedDashboard/TopicChipList scope).
- Around line 428-433: Replace the hardcoded "Feed" text inside the Link between
Button and ChevronRight with a translated string; locate the JSX snippet using
Button, Link and ChevronRight in activitate.tsx and change the literal to a call
to the i18n translator (e.g., t("activity.feedLink") or reuse an existing nav
key like t("nav.feed")), ensuring the translator import/context is used the same
way as adjacent labels.
- Around line 30-40: formatReadDuration and formatScrollDepth return hardcoded
English suffixes; change them to use translations (either accept a t parameter
or call useT inside) and return t(...) with placeholders instead of literal "s",
"min", or "depth". Update all callers to pass the translation function (t) from
useT() when invoking formatReadDuration and formatScrollDepth (the route
component already has access to t), and add translation keys like
"read.duration.seconds", "read.duration.minutes" and "scroll.depth" with
appropriate placeholders so the functions produce localized strings.
In `@apps/web/src/routes/event`.$slug.tsx:
- Around line 345-360: The current inline ternary pluralization in
event.$slug.tsx (the articles and sourceCount spans) only handles two plural
forms; replace it with a small helper that uses Intl.PluralRules to pick a
plural category and look up a locale-aware key (e.g., for base keys
"event.articles" and "event.sourceCount" resolve keys like "event.articles.one",
"event.articles.few", "event.articles.many" etc.), then call t(resolvedKey, {
count }) — implement a helper function (e.g., getPluralKey(count, baseKey) or
selectPluralKey) and use it for both articles and sourceCount so additional
plural categories per-locale are handled automatically.
In `@apps/web/src/routes/feed.tsx`:
- Around line 397-425: The IntersectionObserver effect is recreating whenever
the unstable loadMore function identity changes; to fix it create a stable ref
for it (e.g., const loadMoreRef = useRef(loadMore)), update that ref whenever
loadMore changes (useEffect(() => { loadMoreRef.current = loadMore; },
[loadMore])) and inside the observer callback call loadMoreRef.current(pageSize)
instead of loadMore; then remove loadMore from the dependency array of the
observer effect (keep canLoadMore and pageSize) so observer churn stops while
still respecting isLoadingMoreRef and loadMoreTriggerRef.
In `@apps/web/src/routes/profil.tsx`:
- Around line 11-23: Extract the duplicated locale resolution into a helper
named getLocaleFromMatches(matches) (placed alongside Locale/LocaleContext in
the i18n module) and replace the 7-line inline resolver inside each route head
(e.g., head in profil.tsx and other routes) with a call to that helper; ensure
getLocaleFromMatches accepts the matches array, checks matches[0]?.context for a
locale property of type Locale ("ro"|"en") and returns that locale or the
default "en", then update all routes (profil.tsx, __root.tsx, event.$slug.tsx,
feed.tsx, dashboard.tsx, reset-password.tsx, source.$sourceId.tsx,
unsubscribe.tsx) to import and use getLocaleFromMatches instead of duplicating
the logic.
In `@apps/web/src/routes/reset-password.tsx`:
- Around line 20-32: Extract the duplicated locale-resolution logic used inside
head callbacks into a shared helper (e.g., create getLocaleFromMatches(matches)
in your i18n library like "@/lib/i18n/..."); update each head function (the
head: ({ matches }) => { ... } blocks in reset-password.tsx and the other pages:
salvate.tsx, unsubscribe.tsx, source.$sourceId.tsx, activitate.tsx,
dashboard.tsx, feed.tsx) to call getLocaleFromMatches(matches) and then pass its
return value to getString(locale, "reset.metaTitle") (or the page's meta key);
ensure getLocaleFromMatches encapsulates the current checks (matches[0]?.context
is object, "locale" in context, allowed locale values ["ro","en"], default "en")
so future locale changes are centralized.
- Around line 41-49: resetPasswordSchema is recreated on every render causing
unnecessary allocations and stale validator messages when the locale changes;
wrap the Zod schema creation in useMemo so its reference is stable and updates
only when the translation function t changes. Import useMemo from React and
replace the current const resetPasswordSchema = ... with const
resetPasswordSchema = useMemo(() => z.object({...}).refine(...), [t]); this
keeps useForm's validator reference stable while allowing error messages to
update when t changes.
In `@apps/web/src/routes/salvate.tsx`:
- Around line 56-71: The inline SVG elements used as decorative icons (the <svg>
with className "size-8" inside the rounded div and the other decorative SVG
elsewhere in this component) are not marked as decorative for assistive tech;
add aria-hidden="true" and focusable="false" to both SVG tags so screen readers
ignore them and they aren’t keyboard-focusable. Locate the SVG elements
(className "size-8") in the component JSX and add those attributes to each
decorative SVG.
- Around line 17-20: The function safePositiveInt currently allows 0
(Math.max(0,...)) which conflicts with the expected behaviour in /feed and the
semantics implied by the name; change safePositiveInt to enforce a minimum of 1
(use Math.max(1, Math.floor(n))) so callers like EventCard and
runtimeConfig.eventCardMaxSources get at least one source; keep the existing
fallback behavior for non-finite inputs and update any tests or callers if they
depend on 0 semantics.
In `@apps/web/src/routes/source`.$sourceId.tsx:
- Around line 54-63: The code repeats loaderData?.source.name checks; compute a
single sourceName variable from loaderData?.source?.name once (e.g., const
sourceName = loaderData?.source?.name) and then use that same symbol in title
and description: set title to sourceName ? `${sourceName} — ${SITE.name}` :
getString(locale, "source.metaTitle"), and for description use getString(locale,
"source.metaDescriptionLoaded").replace("{name}", sourceName ??
getString(locale, "source.metaTitle")) (or fall back to getString(locale,
"source.metaDescription") when loaderData is missing) so title, description, and
the name replacement all use the same sourceName value consistently.
- Around line 219-222: The translations reuse the `{count}` placeholder for
non-count numeric values (reliabilityScore, rollingBiasMean, rollingBiasStddev);
update the translation keys (e.g., change
source.reliability/source.mean/source.stddev to use `{score}` or `{value}`
instead of `{count}`) and then update the JSX replace calls to use the new
placeholder names (replace("{score}", String(source.reliabilityScore)) or
replace("{value}", String(...))) for the elements currently calling
t("source.reliability") and the rolling bias mean/stddev code paths (the
.replace(...) calls around the reliabilityScore, rollingBiasMean,
rollingBiasStddev usages), ensuring all occurrences (including the block around
lines 317–332) are updated consistently.
In `@apps/web/src/routes/unsubscribe.tsx`:
- Around line 137-142: The fallback `email ? email : ""` is redundant because
the early return when `!email` guarantees `email` is truthy; update the success
message generation to call t("unsubscribe.successBody").replace("{email}",
email) directly, removing the ternary and dead-code fallback so the component
uses the existing `email` variable without unnecessary checks.
---
Outside diff comments:
In `@apps/web/src/components/share-event-button.tsx`:
- Line 21: The component currently destructures and ignores the summary prop
(exported on ShareEventButtonProps) and calls navigator.share with duplicate
values (text: title), so either restore summary into the shared text or remove
the prop entirely; update the ShareEventButton component by removing the dead
"_summary" destructure and use the actual summary prop when building the share
payload (navigator.share({ title, text: summary ?? title, url })) so title and
text are not identical, and if you intend to drop summary instead, remove it
from ShareEventButtonProps and from callers like the code in event.$slug.tsx to
stop computing/passing it.
In `@apps/web/src/components/sign-in-form.tsx`:
- Around line 16-23: Replace the hardcoded redirectFallbackOrigin used by
getPasswordResetRedirectURL with the app origin from SITE.url: import SITE from
"@/lib/seo" (or destructure SITE.url) and when typeof window === "undefined"
return `${SITE.url}/reset-password` instead of using redirectFallbackOrigin;
remove or replace the redirectFallbackOrigin constant so server-side rendered
password-reset links use SITE.url consistently with the codebase.
In `@apps/web/src/components/ui/page-loading-state.tsx`:
- Around line 7-8: The hardcoded English defaults for the component props title
and description prevent the i18n fallbacks from ever running; remove those
default parameter values (or set them to undefined) in the PageLoadingState
component signature so that the nullish coalescing used later (the title ?? ...
and description ?? ... expressions) can trigger the translated fallbacks; update
the component’s props (title, description) to be optional and rely on the
existing ?? translations instead of supplying English literals as defaults.
In `@apps/web/src/components/user-menu.tsx`:
- Around line 21-29: The handleSignOut function lacks error handling; wrap the
await authClient.signOut(...) call in a try/catch inside handleSignOut, keep the
existing fetchOptions.onSuccess to call location.reload() on success, and in the
catch block surface a user-facing error (e.g., dispatch a toast/snackbar or set
an error state) and log the error (console.error or existing logger) so
network/server failures give feedback and can be diagnosed; reference
handleSignOut and authClient.signOut when making this change.
In `@apps/web/src/routes/reset-password.tsx`:
- Around line 152-156: The per-field error messages rendered from
field.state.meta.errors are not announced to screen readers; update the error
markup (the <p> elements mapped from field.state.meta.errors) to be an ARIA live
region — e.g., add role="alert" (or wrap the field error group with
aria-live="polite") so assistive tech announces validation failures, and also
ensure the corresponding input elements in the same field group include an
accessible label (aria-label or linked <label>) to satisfy the form guidelines;
locate the error renderings around the field.state.meta.errors map and the
associated input components to make these changes.
In `@apps/web/src/routes/unsubscribe.tsx`:
- Around line 53-58: The useEffect closure references unsubscribe.reset but does
not include unsubscribe in the dependency array, which can trigger
exhaustive-deps lint; update the effect to either add unsubscribe to the deps or
destructure the stable callback (e.g., const { reset } = unsubscribe) and depend
on that reset instead—modify the effect that uses useEffect, unsubscribe.reset,
setLastEmail, email, and lastEmail so the dependency array includes the chosen
stable reference (unsubscribe or reset) to satisfy the linter and ensure correct
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c82940c2-1302-42a4-bd7d-bc5acf885257
📒 Files selected for processing (49)
apps/web/src/components/LanguagePicker.tsxapps/web/src/components/SignInPrompt.tsxapps/web/src/components/auth-prompt-banner.tsxapps/web/src/components/auth-social.tsxapps/web/src/components/bias-balance-meter.tsxapps/web/src/components/bias-indicator.tsxapps/web/src/components/bookmark-button.tsxapps/web/src/components/feed/articles-list.tsxapps/web/src/components/feed/event-card.tsxapps/web/src/components/feed/event-claim-comparison.tsxapps/web/src/components/feed/event.tsxapps/web/src/components/feed/source-coverage-summary.tsxapps/web/src/components/header.tsxapps/web/src/components/layout/Footer.tsxapps/web/src/components/layout/MobileTabBar.tsxapps/web/src/components/profile/AnonymousProfile.tsxapps/web/src/components/profile/AuthenticatedProfile.tsxapps/web/src/components/share-event-button.tsxapps/web/src/components/sign-in-form.tsxapps/web/src/components/sign-up-form.tsxapps/web/src/components/streak-activity-calendar.tsxapps/web/src/components/ui/page-loading-state.tsxapps/web/src/components/user-menu.tsxapps/web/src/lib/dates.tsapps/web/src/lib/i18n/LocaleContext.tsxapps/web/src/lib/i18n/getServerLocale.tsapps/web/src/lib/i18n/resolveLocale.tsapps/web/src/lib/i18n/strings.tsapps/web/src/lib/seo.tsapps/web/src/router.tsxapps/web/src/routes/__root.tsxapps/web/src/routes/activitate.tsxapps/web/src/routes/contact.tsxapps/web/src/routes/cum-functioneaza.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/despre.tsxapps/web/src/routes/event.$slug.tsxapps/web/src/routes/feed.tsxapps/web/src/routes/parteneri.tsxapps/web/src/routes/politica-confidentialitate.tsxapps/web/src/routes/profil.tsxapps/web/src/routes/reset-password.tsxapps/web/src/routes/salvate.tsxapps/web/src/routes/source.$sourceId.tsxapps/web/src/routes/sursele-noastre.tsxapps/web/src/routes/termeni.tsxapps/web/src/routes/unsubscribe.tsxpackages/backend/convex/schema.tspackages/backend/convex/user.ts
| }; | ||
|
|
||
| return ( | ||
| <div className="bg-gradient-to-b from-background via-background to-muted/20"> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Which Tailwind v4 release fully removed the legacy bg-gradient-to-aliases in favor ofbg-linear-to-?
💡 Result:
Tailwind CSS v4.1 is the release that fully removed the legacy bg-gradient-to-* aliases in favor of bg-linear-to-*. In Tailwind CSS v4.0, Tailwind introduced the rename from bg-gradient-* (v3) to bg-linear-* (v4) and provided upgrade/template migrations to rewrite class names accordingly [1][2]. The Tailwind v4.1 release includes the period when these legacy aliases are no longer available, i.e., bg-gradient-to-* is removed and only bg-linear-to-* remains [3].
Citations:
- 1: Template migrations: Migrate
bg-gradient-*utilities tobg-linear-*tailwindlabs/tailwindcss#14537 - 2: https://tailwindcss.com/blog/tailwindcss-v4
- 3: https://www.digitalapplied.com/blog/tailwind-css-v4-migration-new-features-guide
🏁 Script executed:
# Verify AuthenticatedProfile.tsx line 54 contains bg-gradient-to-b
cat -n apps/web/src/components/profile/AuthenticatedProfile.tsx | sed -n '50,60p'Repository: flvvius/news
Length of output: 580
🏁 Script executed:
# Check event.$slug.tsx for new-style gradient utilities (lines 243 and 263 as mentioned)
cat -n apps/web/src/routes/event.\$slug.tsx | sed -n '240,270p'Repository: flvvius/news
Length of output: 1654
🏁 Script executed:
# Search for all instances of deprecated bg-gradient-to-* in the codebase
rg 'bg-gradient-to-' --type tsx --type jsxRepository: flvvius/news
Length of output: 84
🏁 Script executed:
# Check Tailwind version in package.json
cat package.json | grep -A 2 -B 2 tailwindcssRepository: flvvius/news
Length of output: 38
🏁 Script executed:
# Try searching for deprecated bg-gradient utilities with simpler rg command
rg 'bg-gradient-to-' --glob '*.tsx' --glob '*.jsx'Repository: flvvius/news
Length of output: 1016
🏁 Script executed:
# Check package.json differently
head -50 package.json | grep -i tailwindRepository: flvvius/news
Length of output: 38
🏁 Script executed:
# Search for Tailwind version in various config/package files
find . -maxdepth 3 -type f \( -name "package.json" -o -name "package-lock.json" -o -name "tailwind.config.*" \) -exec grep -l "tailwindcss\|version" {} \; | head -5Repository: flvvius/news
Length of output: 170
🏁 Script executed:
# Check the actual tailwind version specified
rg '"tailwindcss"' --type json -A 1 -B 1Repository: flvvius/news
Length of output: 383
Replace bg-gradient-to-b with bg-linear-to-b — Tailwind v4.1 removed legacy gradient class names.
Tailwind v4.0 renamed bg-gradient-to-* utilities to bg-linear-to-*, and v4.1 fully removed the legacy aliases. This project uses Tailwind v4.1.3, where bg-gradient-to-b does not exist and produces no background gradient. Other files in the codebase already use the correct naming (e.g., bg-linear-to-b in event.$slug.tsx), so this needs to match.
🛠️ Fix
- <div className="bg-gradient-to-b from-background via-background to-muted/20">
+ <div className="bg-linear-to-b from-background via-background to-muted/20">📝 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
| <div className="bg-gradient-to-b from-background via-background to-muted/20"> | |
| <div className="bg-linear-to-b from-background via-background to-muted/20"> |
🤖 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/src/components/profile/AuthenticatedProfile.tsx` at line 54, In the
AuthenticatedProfile component replace the deprecated Tailwind class name:
update the div whose className contains "bg-gradient-to-b from-background
via-background to-muted/20" to use "bg-linear-to-b" instead of
"bg-gradient-to-b" so it matches the Tailwind v4.1 naming (keep the other
classes unchanged).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/src/components/ui/page-loading-state.tsx (1)
19-47: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider adding ARIA live region for loading announcements.
The loading state displays dynamic content but lacks live-region semantics. Screen readers may not announce the loading status reliably.
♿ Proposed enhancement
<div className="flex flex-col gap-6"> - <Card className="overflow-hidden border-border/70 bg-card/80 shadow-sm"> + <Card + className="overflow-hidden border-border/70 bg-card/80 shadow-sm" + role="status" + aria-live="polite" + >As per coding guidelines,
apps/web/**: - Focus on web performance and accessibility.🤖 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/src/components/ui/page-loading-state.tsx` around lines 19 - 47, Add an ARIA live region to announce the loading state by wrapping the dynamic loading text (e.g., the elements that render resolvedTitle/resolvedDescription or the Loader2/ CardHeader area) in a container with role="status" aria-live="polite" aria-atomic="true" (or add a visually-hidden <div> with those attributes that outputs a concise message like "Loading {resolvedTitle}" or the resolvedDescription), and consider setting aria-busy="true" on the outer container while loading; update the JSX around CardHeader/CardContent to include this live region so screen readers reliably receive loading announcements.apps/web/src/components/sign-up-form.tsx (1)
119-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd localized
aria-labels to the sign-up inputs.The visible labels are good, but this repo’s form rule also requires an
aria-labelon each input. That currently affects thename,passwordfields here.As per coding guidelines, "Forms must have
aria-labelon inputs and ARIA live regions (aria-live,role) on status messages".🤖 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/src/components/sign-up-form.tsx` around lines 119 - 178, The name, email and password Input components are missing aria-labels required by our form rules; add an aria-label prop to each Input inside the form.Field for "name", "email", and "password" (use the existing i18n keys so the label is localized, e.g. t("auth.name"), t("auth.email"), t("auth.password") or the appropriate translation keys used for visible labels/placeholders), keeping other props (like disabled={emailLocked}) unchanged so the accessible label matches the visible label.apps/web/src/components/sign-in-form.tsx (1)
136-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd localized
aria-labels to the sign-in inputs.Both form fields still miss explicit
aria-labelprops. The visible labels help, but this repository requiresaria-labelon inputs too.As per coding guidelines, "Forms must have
aria-labelon inputs and ARIA live regions (aria-live,role) on status messages".Also applies to: 163-245
🤖 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/src/components/sign-in-form.tsx` around lines 136 - 145, The Input elements (e.g., the email Input rendered with id/name from field.name and value from field.state.value, and the corresponding password Input later) are missing explicit aria-label attributes; update the Input components (the ones using field.handleChange/field.handleBlur and Label) to include aria-label props using the localized strings (e.g., t("auth.email") for the email field and t("auth.password") for the password field) so each input has an accessible, localized aria-label; apply the same change to the other form inputs in the sign-in form (the block around lines 163-245) to ensure all inputs include aria-labels per the accessibility guideline.apps/web/src/routes/feed.tsx (1)
181-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
aria-labelto the icon-only close button.The sr-only text helps, but this still misses the explicit
aria-labelrequired by the repo’s accessibility rule for icon-only buttons.♿ Suggested fix
<DrawerClose asChild> <Button variant="ghost" size="icon" className="size-8 rounded-full" + aria-label={t("feed.close")} > <XIcon className="size-4" /> <span className="sr-only">{t("feed.close")}</span> </Button> </DrawerClose>As per coding guidelines, "Buttons must have visible text or
aria-labelfor accessibility".🤖 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/src/routes/feed.tsx` around lines 181 - 188, The icon-only close Button in feed.tsx (the Button wrapping XIcon) lacks an explicit aria-label; add an aria-label prop to that Button (e.g., aria-label={t("feed.close")}) so it satisfies the repo accessibility rule for icon-only buttons while keeping the existing XIcon and sr-only span intact; update the Button JSX (the Button component that currently has variant="ghost" size="icon" className="size-8 rounded-full") to include the aria-label prop.
♻️ Duplicate comments (1)
apps/web/src/lib/i18n/resolveLocale.ts (1)
31-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip
q=0languages duringAccept-Languageresolution.
q=0explicitly means “not acceptable”, but the current filter/sort path still leaves those entries inweightedCodes. A header likero;q=0,en;q=0will currently resolve to a supported locale instead of falling through to the default.Suggested fix
const weightedCodes = header .split(",") .map((part) => { const [languagePart, ...params] = part.split(";"); const code = languagePart?.trim().toLowerCase().split("-")[0]; @@ return { code, weight: Number.isFinite(weight) ? weight : 1, }; }) - .filter((entry): entry is { code: string; weight: number } => Boolean(entry.code)) + .filter( + (entry): entry is { code: string; weight: number } => + Boolean(entry.code) && entry.weight > 0, + ) .sort((a, b) => b.weight - a.weight);🤖 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/src/lib/i18n/resolveLocale.ts` around lines 31 - 50, The Accept-Language parsing builds weightedCodes but doesn’t remove entries with q=0 (explicitly unacceptable), so resolveLocale may consider them; update the weightedCodes construction to filter out entries where weight === 0 (e.g., after computing weight in the .map or via an additional .filter) so that the subsequent loop over weightedCodes (the for...of that checks SUPPORTED) will skip q=0 languages and fall back to the default.
🤖 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/src/components/layout/MobileTabBar.tsx`:
- Around line 65-68: The bottom padding inline style currently overrides the
Tailwind "pb-4" causing the bar to sit flush on devices without safe-area
insets; update the MobileTabBar component's style prop (the element using
style={{ paddingBottom: "env(safe-area-inset-bottom)" }}) to preserve the base
gap by folding the fixed spacing into the inline value—use a CSS calc that adds
the fallback 1rem (or equivalent of pb-4) to env(safe-area-inset-bottom) with a
safe fallback for env when missing so the effective padding keeps the pb-4 gap
plus any safe-area inset.
In `@apps/web/src/routes/__root.tsx`:
- Around line 123-125: The call to decodeURIComponent(cookieLocale) in the
beforeLoad flow can throw a URIError for malformed bv_locale values; wrap the
decoding in a safe guard (e.g., try/catch) so that if
decodeURIComponent(cookieLocale) throws you catch the error and set
decodedCookieLocale to null (or a safe default) instead of allowing the
exception to bubble and break client navigation; update the logic around
decodedCookieLocale (and any callers) to use this safely-decoded value.
In `@apps/web/src/routes/activitate.tsx`:
- Around line 129-146: The admin config editor is currently gated on
topicDiagnostics (currentSettings = topicDiagnostics?.[0]?.settings) so admins
cannot edit settings in fresh environments; add a dedicated config read query
(e.g., useQuery(api.config.get) or similar) and use its result as the source of
truth instead of topicDiagnostics, update the useEffect that seeds
minScoreInput/confidenceRatioInput/maxTopicsInput to read from that config query
(or fall back to sensible defaults) and remove the UI render gating that checks
topicDiagnostics.length > 0 so the admin section always renders; keep using
setConfig (useConvexMutationHook(api.config.set)) for saves.
- Around line 660-698: The three admin config Input components (ids
topic-inference-min-score, topic-inference-confidence-ratio,
topic-inference-max-topics) are missing aria-labels; add localized aria-label
props to each Input using the same i18n keys as their visible labels (e.g.
t("activity.admin.minScore") for the min score input,
t("activity.admin.confidence") for the confidence ratio input, and
t("activity.admin.maxTopics") for the max topics input) so the inputs remain
functionally identical (keep value, onChange, disabled) but now include
accessible aria-label attributes.
- Around line 200-220: The current UI calls setConfig three times in parallel
(setConfig for keys topic_inference_min_score, topic_inference_confidence_ratio,
topic_inference_max_topics) which can leave configs partially applied if one
write fails; implement a single Convex mutation named setTopicInferenceSettings
that accepts the three values and updates all three keys inside one server-side
transaction, then replace the Promise.all([...setConfig(...)]) call with a
single call to the new setTopicInferenceSettings mutation so the three settings
are applied atomically.
In `@apps/web/src/routes/dashboard.tsx`:
- Around line 53-57: The useEffect that redirects authenticated users always
sends them to "/activitate" and ignores any validated redirect from the
route/search params; update the effect (the useEffect that checks currentUser
and calls navigate) to read the validated redirect (from the route loader,
search params, or your existing helper — e.g., a validatedRedirect variable
provided by the route or a getValidatedRedirect() helper) and call navigate({
to: validatedRedirect || "/activitate", replace: true }) instead of hardcoding
"/activitate", keeping the same dependency array [currentUser, navigate].
In `@apps/web/src/routes/feed.tsx`:
- Line 89: The CommandInput in feed.tsx currently only uses a placeholder so
screen readers lose the accessible name once a value is entered; update the
<CommandInput> instance to include an explicit aria-label (e.g.,
aria-label={t("feed.topic.search")}) or a prop that sets the accessible name so
the input keeps a reliable label when populated, and verify any related status
messages use appropriate ARIA live regions (aria-live/role) per the
form-labeling guideline.
In `@packages/backend/convex/clustering.ts`:
- Around line 5316-5331: The fallback path is forcing the local attach to
downgrade to heuristics by calling tryBatchLocalAttach(..., true), which ignores
embedding similarity; change those calls to preserve local embedding matching
(remove or set the forceFallback boolean to false) so tryBatchLocalAttach uses
embeddings and only falls back to findHeuristicCandidate when necessary; update
the calls that run after hydrating candidateCache from
getRecentClusterCandidates (and the other occurrences where tryBatchLocalAttach
is invoked in this file) to call tryBatchLocalAttach without the
forced-heuristic flag, ensuring hydrateClusterCandidate and candidateCache
entries are still used for embedding-based matching.
- Around line 2396-2435: getChangedClusterCandidates currently filters by
updatedAt but paginates by lastArticleAt, which is not a stable incremental
cursor; change the query to use a stable composite cursor/index on (updatedAt,
_creationTime) instead of lastArticleAt. Specifically, update the eventCandidacy
queries in getChangedClusterCandidates to use an index that orders by updatedAt
then _creationTime (or add one if missing), apply the since cursor as a
composite (updatedAt, _creationTime) filter and sort by updatedAt desc then
_creationTime desc, and use that same composite tie-breaker when
slicing/advancing lastProcessedAt so no rows are skipped; remove the mixing of
lastArticleAt-based pagination and the limit*3 heuristic. Ensure filtering for
singletonOnly and status remains the same but rely on the composite cursor for
stable pagination.
- Around line 276-306: The current check-then-record race must be replaced with
an atomic reserve/consume pattern: create and call a Convex mutation (e.g.,
internal.vectorSearchBudget.reserveUsage) from the search path (before executing
a batch of vector searches or per-search) that atomically checks budget and
reserves the QGB/slots, and then decrement/commit that reservation via another
mutation (e.g., internal.vectorSearchBudget.consumeReservation) or let it expire
if not used; update the callers that currently call checkBudget and only later
call flushJobMetrics (referencing flushJobMetrics, checkBudget, and the
vectorSearchBudget mutations) to use reserveUsage before running searches and to
consume or release the reservation in finally/exception handlers so mid-run
exceptions cannot drop already-reserved usage; apply the same pattern to the
other spots noted around lines 309-313.
In `@packages/backend/convex/schema.ts`:
- Around line 123-133: The change to the vector index dimensions for
eventEmbeddings.by_embedding from 1536 to 512 requires a migration/backfill:
either re-embed all existing eventEmbeddings to 512-d vectors or delete them
before deploying. Update EMBEDDING_VERSION in enrichmentNode.ts to trigger
reprocessing (or write a deletion job), and implement an idempotent backfill job
using the migrations.ts pattern to iterate existing eventEmbeddings,
compute/store new 512-d embeddings (or remove rows) and mark progress so the job
can be resumed; ensure the migration runs and completes prior to shipping the
schema change to avoid mismatched vector lengths causing search failures.
In `@packages/backend/convex/seeds.ts`:
- Around line 147-169: The seeded rows call new Date().toISOString().slice(0,10)
multiple times which can produce inconsistent day buckets across fields or rows;
compute a single day string once (e.g., const day = new
Date().toISOString().slice(0,10)) before creating the two event embedding
inserts and reuse that variable for updatedDayBucket, mergeSearchBucket and
singletonSearchBucket in both ctx.db.insert calls (located where dummyEmbedding
and the two ctx.db.insert("eventEmbeddings", { ... }) calls are defined) so each
row and all its bucket fields share the same logical timestamp.
In `@packages/backend/convex/vectorSearchBudget.ts`:
- Around line 221-274: recordUsage currently always increments daily totals via
adjustDailyUsage and inserts a vectorSearchRuns row, so retries double-count;
make it idempotent by runId: first query for an existing run by args.runId (use
ctx.db.get / ctx.db.query on "vectorSearchRuns" keyed by runId or a unique
index), and if found return {recorded: true} without calling adjustDailyUsage or
inserting; otherwise proceed to call adjustDailyUsage and
ctx.db.insert("vectorSearchRuns") as now. Ensure the existence check happens
before calling adjustDailyUsage and that you use the same runId field used in
ctx.db.insert to prevent races (or perform both operations in a single DB
transaction if supported).
---
Outside diff comments:
In `@apps/web/src/components/sign-in-form.tsx`:
- Around line 136-145: The Input elements (e.g., the email Input rendered with
id/name from field.name and value from field.state.value, and the corresponding
password Input later) are missing explicit aria-label attributes; update the
Input components (the ones using field.handleChange/field.handleBlur and Label)
to include aria-label props using the localized strings (e.g., t("auth.email")
for the email field and t("auth.password") for the password field) so each input
has an accessible, localized aria-label; apply the same change to the other form
inputs in the sign-in form (the block around lines 163-245) to ensure all inputs
include aria-labels per the accessibility guideline.
In `@apps/web/src/components/sign-up-form.tsx`:
- Around line 119-178: The name, email and password Input components are missing
aria-labels required by our form rules; add an aria-label prop to each Input
inside the form.Field for "name", "email", and "password" (use the existing i18n
keys so the label is localized, e.g. t("auth.name"), t("auth.email"),
t("auth.password") or the appropriate translation keys used for visible
labels/placeholders), keeping other props (like disabled={emailLocked})
unchanged so the accessible label matches the visible label.
In `@apps/web/src/components/ui/page-loading-state.tsx`:
- Around line 19-47: Add an ARIA live region to announce the loading state by
wrapping the dynamic loading text (e.g., the elements that render
resolvedTitle/resolvedDescription or the Loader2/ CardHeader area) in a
container with role="status" aria-live="polite" aria-atomic="true" (or add a
visually-hidden <div> with those attributes that outputs a concise message like
"Loading {resolvedTitle}" or the resolvedDescription), and consider setting
aria-busy="true" on the outer container while loading; update the JSX around
CardHeader/CardContent to include this live region so screen readers reliably
receive loading announcements.
In `@apps/web/src/routes/feed.tsx`:
- Around line 181-188: The icon-only close Button in feed.tsx (the Button
wrapping XIcon) lacks an explicit aria-label; add an aria-label prop to that
Button (e.g., aria-label={t("feed.close")}) so it satisfies the repo
accessibility rule for icon-only buttons while keeping the existing XIcon and
sr-only span intact; update the Button JSX (the Button component that currently
has variant="ghost" size="icon" className="size-8 rounded-full") to include the
aria-label prop.
---
Duplicate comments:
In `@apps/web/src/lib/i18n/resolveLocale.ts`:
- Around line 31-50: The Accept-Language parsing builds weightedCodes but
doesn’t remove entries with q=0 (explicitly unacceptable), so resolveLocale may
consider them; update the weightedCodes construction to filter out entries where
weight === 0 (e.g., after computing weight in the .map or via an additional
.filter) so that the subsequent loop over weightedCodes (the for...of that
checks SUPPORTED) will skip q=0 languages and fall back to the default.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 61b2359d-21c3-4c07-8f08-87ae03805e27
⛔ Files ignored due to path filters (1)
packages/backend/convex/_generated/api.d.tsis excluded by!**/_generated/**,!**/_generated/**
📒 Files selected for processing (32)
apps/web/src/components/LanguagePicker.tsxapps/web/src/components/feed/event-claim-comparison.tsxapps/web/src/components/header.tsxapps/web/src/components/layout/MobileTabBar.tsxapps/web/src/components/profile/AnonymousProfile.tsxapps/web/src/components/profile/AuthenticatedProfile.tsxapps/web/src/components/share-event-button.tsxapps/web/src/components/sign-in-form.tsxapps/web/src/components/sign-up-form.tsxapps/web/src/components/ui/page-loading-state.tsxapps/web/src/components/user-menu.tsxapps/web/src/hooks/use-scroll-visibility.tsapps/web/src/lib/i18n/LocaleContext.tsxapps/web/src/lib/i18n/getLocaleFromMatches.tsapps/web/src/lib/i18n/resolveLocale.tsapps/web/src/lib/i18n/strings.tsapps/web/src/routes/__root.tsxapps/web/src/routes/activitate.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/event.$slug.tsxapps/web/src/routes/feed.tsxapps/web/src/routes/profil.tsxapps/web/src/routes/reset-password.tsxapps/web/src/routes/salvate.tsxapps/web/src/routes/source.$sourceId.tsxapps/web/src/routes/unsubscribe.tsxpackages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/ingestion.tspackages/backend/convex/schema.tspackages/backend/convex/seeds.tspackages/backend/convex/vectorSearchBudget.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (1)
packages/backend/convex/clustering.ts (1)
2447-2492:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCursor still advances past unprocessed rows when more than
limitmatch.The query now orders by
(updatedAt, _creationTime)(good — that addresses the previous tie-breaker concern), but it still doestake(limit)on a DESC-ordered set and thenadvanceChangedCandidateCursor(called at lines 4839-4843 and 5210-5214) moveslastProcessedAtto the maximumupdatedAtof the returned rows. If more thanlimitcandidacies satisfyupdatedAt > sinceTs, the rows in(sinceTs, min(returned.updatedAt))are silently dropped: they weren't returned, but the next run'ssinceTsis nowmax(returned.updatedAt), so they will never be revisited. This reintroduces the same "callers will advance lastProcessedAt past them" failure mode flagged previously. Either order ASC and advance to the highest processed row, or only advance the cursor when the page is fully drained (e.g.rows.length < limit).As per coding guidelines, "Review database schema changes carefully" and "Verify proper error handling".
🤖 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 `@packages/backend/convex/clustering.ts` around lines 2447 - 2492, getChangedClusterCandidates currently queries two DESC-ordered sets with .take(limit) and returns the top N, but advancing the cursor (via advanceChangedCandidateCursor) to the max returned updatedAt will skip any unreturned rows with updatedAt between sinceTs and that max. Fix by either querying in ASC order so you can safely advance the cursor to the highest processed row without skipping (change ordering to asc on updatedAt/_creationTime and paginate forward), or keep DESC but only advance lastProcessedAt when the page is fully drained (i.e., returnedRows.length < limit) so unprocessed rows aren’t skipped; update getChangedClusterCandidates and the cursor-advance callers (advanceChangedCandidateCursor) accordingly to implement one of these strategies consistently.
🤖 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/src/components/layout/MobileTabBar.tsx`:
- Around line 27-31: The active-state boolean (isActive) is being reused to
decide whether to prevent navigation in handleTabClick, causing taps on the Feed
tab from /event/* to merely scroll instead of navigate back to /feed; update the
logic to compute two booleans: keep the existing isActive predicate (pathname
=== "/feed" || pathname.startsWith("/feed/") || pathname.startsWith("/event/"))
for visual highlighting, but introduce an isSameDestination check (e.g.,
pathname === to) and only call preventDefault/avoid navigation when
isSameDestination is true; update handleTabClick to use isSameDestination
instead of isActive so tapping from an event page navigates to /feed while still
showing the active visual state on /event/*.
In `@apps/web/src/components/sign-in-form.tsx`:
- Around line 24-50: getLocalizedSignInError is brittle because it relies on
exact English message strings; change it to normalize and check only the error
code against a small allowlist (e.g. "invalid_credentials", "invalid_password",
"invalid_login", case-insensitive, normalize underscores/hyphens) and return
t("auth.invalidCredentials") when the normalized code matches any allowlisted
value; keep the message/statusText as a best-effort fallback (return
error.error?.message ?? error.error?.statusText ?? t("auth.signInError")), and
ensure you trim/lowercase the code before comparing so variants like
"INVALID_CREDENTIALS" still match.
- Around line 17-22: The getPasswordResetRedirectURL function currently
concatenates paths with SITE.url which can break if SITE.url gains a trailing
slash; replace the string concatenation in getPasswordResetRedirectURL by
constructing the redirect using the URL API (e.g. new URL("/reset-password",
SITE.url).toString()) and do the same when using window.location.origin (or new
URL("/reset-password", window.location.origin).toString()) to ensure robust URL
joining; also scan the codebase for other occurrences of `${SITE.url}/...` and
switch them to new URL(...) or add explicit assertions/validation for SITE.url's
shape in the seo module.
In `@apps/web/src/components/sign-up-form.tsx`:
- Around line 78-81: The onError handler in the SignUpForm component currently
shows raw server text (error.error.message) which is English; replace that by
mapping server error codes/messages to i18n keys and passing the translated
string to toast.error. Reuse the existing getLocalizedSignInError logic (or
create getLocalizedSignUpError) to map common sign-up cases like "user already
exists" / "email already in use" to a translation key (e.g.,
t("auth.emailInUse")), then call that mapping from the onError in sign-up
(instead of error.error.message) and fall back to t("auth.unexpectedError") if
unmapped.
- Around line 85-91: The validation schema passed into the validators/onSubmit
object is created once and closes over t(...) so it doesn't update when locale
changes; wrap the z.object schema (the object with name/email/password using
t("auth.*")) in a useMemo keyed on the current locale (or i18n.language) and
pass that memoized schema into validators.onSubmit so validation messages update
immediately when the language changes.
In `@apps/web/src/components/ui/page-loading-state.tsx`:
- Around line 15-17: The default i18n keys are inconsistent: resolvedTitle falls
back to "feed.loading" while resolvedDescription uses "activity.loading.body";
update the defaults used in PageLoadingState so they come from a shared
namespace (for example use t("common.loading.title") for resolvedTitle and
t("common.loading.body") for resolvedDescription) so both defaults are symmetric
and callers can still override via the title and description props; adjust the
references to resolvedTitle and resolvedDescription in the component
accordingly.
In `@apps/web/src/routes/activitate.tsx`:
- Around line 168-191: currentSettings is rebuilt each render which is
misleading; either memoize it with useMemo or extract the three primitive values
directly so the useEffect deps are clearly stable. Replace the inline object
creation with a useMemo(() => ({ minScore: getNumericConfigValue(...),
confidenceRatio: ..., maxTopics: ... }), [minScoreConfig, confidenceRatioConfig,
maxTopicsConfig]) or assign const minScore = getNumericConfigValue(...), const
confidenceRatio = ..., const maxTopics = ... and then update the useEffect to
depend on those primitives and call setMinScoreInput, setConfidenceRatioInput,
and setMaxTopicsInput accordingly (referencing currentSettings, useEffect,
setMinScoreInput, setConfidenceRatioInput, setMaxTopicsInput,
getNumericConfigValue, TOPIC_INFERENCE_DEFAULTS).
In `@apps/web/src/routes/feed.tsx`:
- Around line 653-660: The status paragraph for indexed search (the JSX block
guarded by isSearching that renders t("feed.search.indexed").replace("{query}",
debouncedSearch)) must be an ARIA live region so screen readers are notified;
update that <p> to include role="status" aria-live="polite" and
aria-atomic="true" (preserving the existing className and text interpolation) so
the change is announced politely when isSearching becomes true.
In `@packages/backend/convex/clustering.ts`:
- Around line 276-364: The five helpers flushJobMetrics,
getVectorSearchBudgetState, reserveVectorSearch, consumeVectorSearchReservation,
and releaseVectorSearchReservation currently use ctx: any (and the last two use
reservationId: any); import ActionCtx from "./_generated/server" and replace
ctx: any with ctx: ActionCtx in all five function signatures, and change
reservationId: any to reservationId: Id<"vectorSearchReservations"> (import Id
from the same generated module if needed) for consumeVectorSearchReservation and
releaseVectorSearchReservation so Convex action context and reservation id types
are enforced.
In `@packages/backend/convex/config.ts`:
- Around line 868-874: The current forcedDefaultKeys array contains the
runtime-tunable keys (vector_search_daily_budget_qgb,
vector_search_budget_enabled, vector_search_fallback_mode_enabled,
clustering_vector_search_limit, merge_vector_search_limit,
recluster_vector_search_limit) which causes seedDefaults to overwrite admin-set
values; remove those vector_search_* keys (and the related
clustering/merge/recluster limits if they should be admin-tunable) from
forcedDefaultKeys so seedDefaults only inserts missing defaults and does not
patch existing rows, or conversely if you intend them to be immutable update
their descriptions and keep them in forcedDefaultKeys; edit the
forcedDefaultKeys array and validate seedDefaults (function seedDefaults)
behavior to ensure it does not call the patch/update path for keys meant to be
overridable.
- Around line 351-367: The server-side validation for args.minScore,
args.confidenceRatio, and args.maxTopics uses hardcoded ranges and must be
unified with the client to avoid drift; extract the bounds into a single shared
source (e.g., a new exported constants object like TOPIC_INFERENCE_BOUNDS or a
getTopicInferenceBounds query) and replace the literal checks in this validation
(the conditions that throw ConvexError for args.minScore, args.confidenceRatio,
args.maxTopics) with references to those shared bounds, then update the client
code in activitate.tsx to read the same shared constants/query instead of
duplicating the min/max literals so both layers use the identical values.
In `@packages/backend/convex/migrations.ts`:
- Around line 558-568: The rescheduling uses ctx.scheduler.runAfter(0, ...)
which queues immediate back-to-back runs and can overload partitions; change the
reschedule to use a small non-zero delay (e.g., 250–1000 ms) instead of 0 and
add an optional maxPages/remainingPages cap passed through the auto-continuation
payload so a single job cannot loop indefinitely; update the continuation logic
around scheduledContinuation/nextCursor to pass { cursor: nextCursor, pageSize:
safePageSize, autoContinue: true, remainingPages: <defaultOrArg> } and use
remainingPages-- on each reschedule, and apply the same change to the
queueEventShareAssetsBackfill rescheduling path and any callers of
api.migrations.deleteInvalidEventEmbeddingsFor512dVectorIndex to ensure both the
delay and page cap are respected.
- Around line 525-579: The migration deletes eventEmbeddings rows with wrong
dimensions but never requests re-embedding, leaving events invisible to vector
search; update deleteInvalidEventEmbeddingsFor512dVectorIndex to also queue each
affected event for re-embedding (or schedule a backfill job) immediately after
deletion: for every row you delete (in the loop where you inspect row.embedding
and call ctx.db.delete(row._id)), record the corresponding event id and either
call/schedule the existing re-embedding entrypoint (e.g., invoke the
ingestion/updateEventEmbedding flow or a new api.migrations.reembedEvent job)
via ctx.scheduler.runAfter or push it to your re-embedding queue so missing
embeddings are regenerated; ensure the scheduled task name and the parameter
(event id) match the handler you implement so deleted embeddings are recovered
automatically.
In `@packages/backend/convex/schema.ts`:
- Around line 710-727: The vectorSearchRuns table lacks a retention/TTL sweep
and will grow indefinitely; add a retention strategy by (1) adding either an
expiresAt numeric column or relying on the existing createdAt plus a new index
that supports age-based pagination (e.g., add index "by_createdAt" on
["createdAt"] or keep "by_job_createdAt"), and (2) implement a scheduled cleanup
mutation (e.g., sweepVectorSearchRuns or cleanupVectorSearchRuns) that paginates
using the chosen index (use query(...).index("by_job_createdAt") or the new
"by_createdAt") and deletes rows with createdAt < now - RETENTION_DAYS*86400;
make retention configurable (RETENTION_DAYS/default 30) and schedule the
mutation via your existing cron/scheduler wiring so old rows are deleted in
pages.
In `@packages/backend/convex/vectorSearchBudget.ts`:
- Around line 469-480: The handler currently computes pageSize from a
potentially fractional v.number() so take(pageSize) can receive a non-integer;
update the normalization to floor the value after clamping so pagination always
uses an integer. Specifically, after computing Math.max(1, Math.min(limit ?? 50,
200)) for pageSize in the async handler (the function handling args: { limit }),
apply Math.floor (or equivalent) before passing pageSize into
ctx.db.query(...).take(pageSize) so take always receives a whole number.
- Around line 174-183: The cleanup in releaseExpiredReservations stops after a
single .take(100) page; change releaseExpiredReservations to page through
results until the query returns zero rows, releasing each batch before
continuing so all expired "vectorSearchReservations" with status "reserved" and
expiresAt <= now are processed; use the existing
ctx.db.query("vectorSearchReservations").withIndex("by_status_expiresAt", ...)
call inside a loop (or use cursor/offset paging) to fetch successive pages,
apply the same release logic to each batch, and adjust totals.qgbRead for every
released reservation so the budget calculation (used by reserveUsage) reflects
all freed quota before returning.
- Around line 90-153: The read-then-insert race can create duplicate
vectorSearchDailyTotal or vectorSearchDaily rows under concurrency; change
ensureDailyTotal and the insert branch of adjustDailyUsage to perform an
insert-with-retry: attempt the insert, but catch a duplicate/unique-constraint
error and then re-query the indexed unique lookup (using
query("vectorSearchDailyTotal").withIndex("by_date", ...) and
query("vectorSearchDaily").withIndex("by_date_shard", ...)) to return the
existing document instead of assuming insert succeeded; apply the same pattern
for both ensureDailyTotal and the else branch in adjustDailyUsage so creation is
effectively serialized and duplicates are resolved by fetching the authoritative
row.
---
Duplicate comments:
In `@packages/backend/convex/clustering.ts`:
- Around line 2447-2492: getChangedClusterCandidates currently queries two
DESC-ordered sets with .take(limit) and returns the top N, but advancing the
cursor (via advanceChangedCandidateCursor) to the max returned updatedAt will
skip any unreturned rows with updatedAt between sinceTs and that max. Fix by
either querying in ASC order so you can safely advance the cursor to the highest
processed row without skipping (change ordering to asc on
updatedAt/_creationTime and paginate forward), or keep DESC but only advance
lastProcessedAt when the page is fully drained (i.e., returnedRows.length <
limit) so unprocessed rows aren’t skipped; update getChangedClusterCandidates
and the cursor-advance callers (advanceChangedCandidateCursor) accordingly to
implement one of these strategies consistently.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b9139c98-2bd8-417a-88da-71b7416db17a
📒 Files selected for processing (15)
apps/web/src/components/layout/MobileTabBar.tsxapps/web/src/components/sign-in-form.tsxapps/web/src/components/sign-up-form.tsxapps/web/src/components/ui/page-loading-state.tsxapps/web/src/lib/i18n/resolveLocale.tsapps/web/src/routes/__root.tsxapps/web/src/routes/activitate.tsxapps/web/src/routes/dashboard.tsxapps/web/src/routes/feed.tsxpackages/backend/convex/clustering.tspackages/backend/convex/config.tspackages/backend/convex/migrations.tspackages/backend/convex/schema.tspackages/backend/convex/seeds.tspackages/backend/convex/vectorSearchBudget.ts
Comment on lines
+710
to
+727
| vectorSearchRuns: defineTable({ | ||
| jobName: v.string(), | ||
| runId: v.string(), | ||
| date: v.string(), | ||
| qgbRead: v.number(), | ||
| vectorSearches: v.number(), | ||
| vectorMatchesReturned: v.number(), | ||
| vectorMatchesHydrated: v.number(), | ||
| vectorMatchesDiscardedPostFetch: v.number(), | ||
| usedFallbackMode: v.boolean(), | ||
| budgetAllowed: v.boolean(), | ||
| elapsedMs: v.number(), | ||
| metricsJson: v.string(), | ||
| createdAt: v.number(), | ||
| }) | ||
| .index("by_run_id", ["runId"]) | ||
| .index("by_date", ["date"]) | ||
| .index("by_job_createdAt", ["jobName", "createdAt"]), |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
vectorSearchRuns has no retention strategy and will grow unbounded.
Every clustering/merge/recluster run writes a row here with a non-trivial JSON metrics blob. The only indexes are by_run_id, by_date, and by_job_createdAt, none of which support efficient deletion by age, and there's no scheduled trimming or TTL mutation in this PR. Over weeks of operation this table will accumulate many rows and inflate read/storage cost for the same admin dashboards that consume it. Consider adding a scheduled cleanup mutation that paginates by by_job_createdAt (or by_date) and deletes rows older than a configurable retention window (e.g., 30 days), mirroring how aiBudgetReservations is swept via by_expiresAt.
🤖 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 `@packages/backend/convex/schema.ts` around lines 710 - 727, The
vectorSearchRuns table lacks a retention/TTL sweep and will grow indefinitely;
add a retention strategy by (1) adding either an expiresAt numeric column or
relying on the existing createdAt plus a new index that supports age-based
pagination (e.g., add index "by_createdAt" on ["createdAt"] or keep
"by_job_createdAt"), and (2) implement a scheduled cleanup mutation (e.g.,
sweepVectorSearchRuns or cleanupVectorSearchRuns) that paginates using the
chosen index (use query(...).index("by_job_createdAt") or the new
"by_createdAt") and deletes rows with createdAt < now - RETENTION_DAYS*86400;
make retention configurable (RETENTION_DAYS/default 30) and schedule the
mutation via your existing cron/scheduler wiring so old rows are deleted in
pages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Navigation & Layout
Localization
UX Improvements