Open
Conversation
|
@lukyrys is attempting to deploy a commit to the goldflag's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughIntroduces first-class app sites (site.type "web" | "app"): captures device_model and app_version end‑to‑end, adds site icon storage and CRUD endpoints, parses SDK User-Agent, makes filters and UI app-aware, adds Flutter SDK integration UI, client icon resizing and package-name validation, plus many localization entries. Changes
Sequence Diagram(s)sequenceDiagram
participant SDK as Client SDK
participant Tracker as Tracker Service
participant ClickHouse as ClickHouse
participant API as Server API
participant Postgres as Postgres
participant UI as Web UI
SDK->>Tracker: POST /track (payload with app_version, device_model, ua)
Note right of Tracker: sdkUA = parseSDKUserAgent(pv.ua)
alt sdkUA parsed
Tracker->>ClickHouse: INSERT event (app_version, device_model, sdkUA.browser, sdkUA.os)
else fallback
Tracker->>ClickHouse: INSERT event (app_version, device_model, pv.ua.*)
end
UI->>API: GET /sites/:siteId/icon
API->>Postgres: SELECT sites.icon WHERE id=siteId
Postgres-->>API: icon (bytea) or null
API-->>UI: image/png or 404
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
lukyrys
changed the title
![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: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
client/src/app/[site]/users/components/UsersTable.tsx (1)
18-18:⚠️ Potential issue | 🔴 CriticalInvalid import:
useExtracteddoes not exist innext-intl.The
next-intlpackage does not exportuseExtracted. The correct hook isuseTranslations. This will cause a build/runtime error.🐛 Proposed fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";And update the usage on line 79:
- const t = useExtracted(); + const t = useTranslations();As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n".
Also applies to: 79-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/components/UsersTable.tsx at line 18, The import of useExtracted from next-intl is invalid; update the import in UsersTable.tsx to import useTranslations instead and replace any call sites of useExtracted (e.g., the hook invocation used around the UsersTable component at the spot referenced on line 79) with useTranslations(), ensuring the variable name and subsequent calls (like t(...) or similar translation lookups) match the useTranslations API.client/src/app/[site]/main/components/MainSection/MainSection.tsx (1)
4-4:⚠️ Potential issue | 🟠 MajorUse
useTranslations()instead ofuseExtracted()for client/src scope.The
useExtracted()hook is an experimental next-intl feature for the extraction/docs scope (as shown in learnings), but this file is inclient/src, which must use the standarduseTranslations()hook per coding guidelines and learnings.Change the import and usage:
Fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/main/components/MainSection/MainSection.tsx at line 4, Replace the experimental useExtracted hook import and usage with the standard useTranslations hook: change the import line to import { useTranslations } from "next-intl" (replacing useExtracted), and update any calls of useExtracted(...) in MainSection (e.g., variable assignments or destructuring) to useTranslations(...) with the same namespace/key argument(s); ensure any returned value names (like t or translations) remain consistent and adjust call sites accordingly.client/src/app/[site]/main/components/sections/Events.tsx (1)
2-2:⚠️ Potential issue | 🟡 MinorReset tab when app sites disable the Outbound tab.
On Line 68/Line 91,
isAppcan hide the"outbound"tab/content whiletabstill holds"outbound", leaving an empty panel state.💡 Suggested fix
-import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [tab, setTab] = useState<Tab>("events"); const [expandedOutbound, setExpandedOutbound] = useState(false); const t = useExtracted(); const { data: siteMetadata } = useGetSite(); const isApp = siteMetadata?.type === "app"; + + useEffect(() => { + if (isApp && tab === "outbound") { + setTab("events"); + } + }, [isApp, tab]);Also applies to: 65-66, 68-69, 79-79, 91-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/main/components/sections/Events.tsx at line 2, The component Events has state variable tab and conditional rendering that hides the "outbound" tab when isApp is true/false, but it does not reset tab if it currently equals "outbound", leaving an empty panel; inside the Events component add a small effect or conditional update that watches isApp and tab and when isApp causes the outbound tab to be hidden (e.g., isApp === true or false per your logic) and tab === "outbound", setTab to a valid fallback (for example "inbound" or the first visible tab); reference the Events component, the tab state setter setTab, and the isApp value to implement this guard so the UI never retains "outbound" while that tab is hidden.
🟡 Minor comments (19)
client/messages/cs.json-1092-1092 (1)
1092-1092:⚠️ Potential issue | 🟡 MinorInconsistent formality: Uses formal "vy" form while rest of file uses informal "ty" form.
This line uses formal Czech ("Nainstalujte...vaší aplikace") while the rest of the file consistently uses informal addressing ("ty" form). For example:
- Line 41: "Vlož tento kód..." (informal)
- Line 906: "Začni 7denní..." (informal)
For consistency, consider changing to informal form:
- "Nainstaluj Rybbit Flutter SDK pro sledování událostí z tvé aplikace."
The same inconsistency appears at line 924 ("Začněte...nebudete účtováni").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/cs.json` at line 1092, Update the Czech translation for key "z0p82B" to use the informal "ty" form to match the rest of the file (e.g., change "Nainstalujte Rybbit Flutter SDK pro sledování událostí z vaší aplikace." to an informal variant like "Nainstaluj Rybbit Flutter SDK pro sledování událostí z tvé aplikace.") and similarly review the translation referenced around line 924 ("Začněte...nebudete účtováni") to convert any remaining formal "vy"/"vaší" forms to the informal "ty"/"tvé" forms for consistency.client/messages/zh.json-1157-1160 (1)
1157-1160:⚠️ Potential issue | 🟡 MinorMissing translations for icon preview and upload status.
Keys
TJo5E6andJEsxDware empty in zh.json but have values in en.json ("Preview" and "Uploading..." respectively). Provide Chinese translations for these UI elements:
TJo5E6: "Preview"JEsxDw: "Uploading..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/zh.json` around lines 1157 - 1160, Add Chinese translations for the missing keys in client/messages/zh.json: set "TJo5E6" to "预览" for the "Preview" label and set "JEsxDw" to "正在上传…" for the "Uploading..." status; update the existing JSON entries for the keys TJo5E6 and JEsxDw so they match the English semantics and preserve JSON formatting.client/messages/de.json-636-637 (1)
636-637:⚠️ Potential issue | 🟡 MinorApp creation copy is inconsistent in adjacent strings.
Line 636 says “App hinzufügen”, but Line 637 still says “neue Website”. In app mode this reads inconsistently.
💡 Suggested fix
- "m5aC6r": "Verfolgen Sie Analysen für eine neue Website in Ihrer Organisation", + "m5aC6r": "Verfolgen Sie Analysen für eine neue App in Ihrer Organisation",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/de.json` around lines 636 - 637, The two adjacent i18n entries are inconsistent: key "VxUjLr" uses "App hinzufügen" while key "m5aC6r" still says "neue Website"; update the copy so both refer to the same concept (App or Website). Locate the keys "VxUjLr" and "m5aC6r" and change the value for "m5aC6r" to a phrasing consistent with app mode (e.g., replace "neue Website" with "neue App") or alternatively change "VxUjLr" to mention "Website" if that is the intended domain, ensuring both strings use the same noun and tone.client/messages/de.json-36-39 (1)
36-39:⚠️ Potential issue | 🟡 MinorKeep pronoun/register consistent across German locale strings.
These new strings mix informal phrasing (
Füge,Sieh dir,deiner) with the otherwise formal tone (Sie/Ihr) used across this locale file.💡 Suggested fix
- "BiyUAo": "Füge das Rybbit SDK zu deiner App hinzu:", - "Z8TXx9": "Sieh dir die", + "BiyUAo": "Fügen Sie das Rybbit SDK zu Ihrer App hinzu:", + "Z8TXx9": "Sehen Sie sich die", - "z0p82B": "Installiere das Rybbit Flutter SDK, um Ereignisse aus deiner App zu verfolgen.", + "z0p82B": "Installieren Sie das Rybbit Flutter SDK, um Events aus Ihrer App zu verfolgen.", - "c1bAPv": "Erfasse Fehler und Ausnahmen aus deiner App", + "c1bAPv": "Erfasst Fehler und Ausnahmen aus Ihrer App",Also applies to: 1092-1092, 1136-1136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/de.json` around lines 36 - 39, The German strings use informal phrasing; update the entries with keys "BiyUAo", "Z8TXx9", "5qOEz+", and "3jM2wN" to the formal register (Sie/Ihre) to match the rest of the locale file (e.g., change verbs and pronouns from informal to formal). Also check and correct the same register for the other affected keys mentioned in the review so all German strings consistently use the formal tone across the file.client/messages/pl.json-1157-1160 (1)
1157-1160:⚠️ Potential issue | 🟡 MinorAdd missing Polish translations for "Preview" and "Uploading..." keys.
Two translation keys have empty string values while their English counterparts contain visible UI text:
- Line 1157:
"TJo5E6": ""should translate"Preview"- Line 1160:
"JEsxDw": ""should translate"Uploading..."Provide Polish translations for these keys to complete the icon upload/delete feature localization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/pl.json` around lines 1157 - 1160, Fill the two empty Polish translation keys by replacing the empty strings for "TJo5E6" and "JEsxDw" with Polish text: set "TJo5E6" to the Polish word for "Preview" (e.g., "Podgląd") and set "JEsxDw" to the Polish phrase for "Uploading..." (e.g., "Przesyłanie...") so the icon upload/delete UI shows localized text; update the values for the keys TJo5E6 and JEsxDw in client/messages/pl.json accordingly.client/messages/ko.json-1136-1136 (1)
1136-1136:⚠️ Potential issue | 🟡 Minor설정 설명 문체를 주변 키와 맞춰 주세요.
Line 1136의 “앱의 오류 및 예외 캡처”는 명사형이라, 인접 설명문(“…추적합니다/…캡처합니다”)과 톤이 다릅니다.
✏️ Suggested copy fix
- "c1bAPv": "앱의 오류 및 예외 캡처", + "c1bAPv": "앱의 오류 및 예외를 캡처합니다",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/ko.json` at line 1136, Update the translation value for the key "c1bAPv" in client/messages/ko.json from the noun phrase to a verb-style description to match surrounding sentences—replace "앱의 오류 및 예외 캡처" with a verb-form like "앱의 오류 및 예외를 캡처합니다" (or alternatively "앱의 오류 및 예외를 추적합니다") so the tone matches adjacent explanatory lines.client/messages/ko.json-636-637 (1)
636-637:⚠️ Potential issue | 🟡 Minor앱 생성 문맥인데 설명 문구가 웹사이트 전용으로 보입니다.
Line 636은 “앱 추가”인데, Line 637은 “새 웹사이트”로 한정되어 있어 문맥이 어긋납니다. 앱/웹 공통 용어(예: “사이트”)로 맞추는 편이 정확합니다.
✏️ Suggested copy fix
- "m5aC6r": "조직에서 새 웹사이트의 분석을 추적합니다", + "m5aC6r": "조직에서 새 사이트(웹/앱)의 분석을 추적합니다",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/ko.json` around lines 636 - 637, The description for key "m5aC6r" is web-specific while key "VxUjLr" is "앱 추가"; update the "m5aC6r" value to use a neutral term to match the app context (for example change "조직에서 새 웹사이트의 분석을 추적합니다" to "조직에서 새 사이트의 분석을 추적합니다" or "조직에서 새 앱/사이트의 분석을 추적합니다") so both "VxUjLr" and "m5aC6r" are consistent.client/messages/ja.json-1157-1157 (1)
1157-1157:⚠️ Potential issue | 🟡 MinorRemove unused translation keys.
The translation keys
TJo5E6(line 1157) andJEsxDw(line 1160) are not referenced anywhere in the codebase. These empty entries should be removed fromclient/messages/ja.jsonunless they are placeholders for upcoming features. If they are intentional placeholders, add a comment to clarify their purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/ja.json` at line 1157, Remove the unused translation keys TJo5E6 and JEsxDw from the ja.json translations (they are empty and not referenced anywhere) by deleting those entries; if they are intended placeholders for upcoming features, instead retain them but add a clear inline comment explaining their purpose and planned use so future reviewers understand they are intentional.client/src/app/[site]/components/Sidebar/Sidebar.tsx-39-39 (1)
39-39:⚠️ Potential issue | 🟡 MinorPrevent web-only sidebar items from flashing on app sites during site metadata load.
Because
isAppisfalsewhilesiteis unresolved, Line 97 and Line 120 can briefly render web-only entries (Performance,Replay) for app sites. Same transient mismatch affects the header label at Line 76.💡 Suggested fix
- const isApp = site?.type === "app"; + const siteType = site?.type; + const isApp = siteType === "app"; + const isWeb = siteType === "web"; - <SidebarComponents.SectionHeader>{isApp ? t("Analytics") : t("Web Analytics")}</SidebarComponents.SectionHeader> + <SidebarComponents.SectionHeader> + {isApp ? t("Analytics") : isWeb ? t("Web Analytics") : t("Analytics")} + </SidebarComponents.SectionHeader> - {IS_CLOUD && !isApp && ( + {IS_CLOUD && isWeb && ( <SidebarComponents.Item label={t("Performance")} active={isActiveTab("performance")} href={getTabPath("performance")} icon={<Gauge className="w-4 h-4" />} /> )} - {!isApp && ( + {isWeb && ( <div className="hidden md:block"> {!subscription?.planName?.startsWith("appsumo") && !isSubscriptionLoading && ( <SidebarComponents.Item label={t("Replay")} active={isActiveTab("replay")} href={getTabPath("replay")} icon={<Video className="w-4 h-4" />} /> )} </div> )}Also applies to: 76-76, 97-97, 120-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Sidebar/Sidebar.tsx at line 39, The transient flash happens because isApp is false while site is still undefined; change the logic to treat isApp as unknown until site is loaded and only render site-type-dependent UI after site is non-null. Concretely, compute isApp with a guarded check like "const isApp = site != null ? site.type === 'app' : undefined" (or use "site != null && site.type === 'app'") and update the Sidebar render branches (the header label and the web-only items 'Performance' and 'Replay' in the Sidebar component) to only render when site != null (or isApp === true/false explicitly), preventing rendering when site is unresolved.client/src/app/[site]/components/Sidebar/SiteSelector.tsx-107-111 (1)
107-111:⚠️ Potential issue | 🟡 MinorDuplicate Smartphone icon rendering.
The
Faviconcomponent already renders aSmartphoneicon whensiteType !== "web"(as shown inFavicon.tsxlines 27-35). Adding anotherSmartphoneicon in lines 109-111 creates duplicate smartphone indicators for app sites in the selector list.🐛 Suggested fix
Remove the redundant Smartphone icon since Favicon handles this:
<Favicon domain={site.domain} className="w-4 h-4 shrink-0" siteType={site.type} siteId={site.siteId} /> <div className="text-sm text-neutral-900 dark:text-white truncate">{site.domain}</div> - {site.type && site.type !== "web" && ( - <Smartphone className="h-3 w-3 text-neutral-400 shrink-0" /> - )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Sidebar/SiteSelector.tsx around lines 107 - 111, Remove the redundant Smartphone icon rendered in the SiteSelector component: the Favicon component already shows the Smartphone when site.type !== "web", so delete the conditional JSX that renders <Smartphone ... /> in SiteSelector.tsx (the block that checks site.type && site.type !== "web") to avoid duplicate icons; leave the Favicon usage intact.client/src/components/Sessions/SessionCard.tsx-39-42 (1)
39-42:⚠️ Potential issue | 🟡 MinorBind site metadata query to the current route param.
useGetSite()at Line 41 currently relies on store fallback. Passing the routesiteexplicitly avoids app/web mode mismatches during navigation or delayed store sync.💡 Proposed fix
- const { data: siteMetadata } = useGetSite(); + const { data: siteMetadata } = useGetSite(typeof site === "string" ? site : undefined);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Sessions/SessionCard.tsx` around lines 39 - 42, The site metadata query should be bound to the current route param: instead of calling useGetSite() with store fallback, pass the route value from useParams (the site variable) into useGetSite so it always fetches metadata for the current route; update references that use siteMetadata (e.g., isApp = siteMetadata?.type === "app") to rely on the result returned from useGetSite(site). Ensure the callsite is changed from useGetSite() to useGetSite(site) (or the appropriate single-arg signature your hook expects) so navigation/delayed-store sync won't produce app/web mode mismatches.client/src/app/[site]/components/Header/NoData.tsx-78-79 (1)
78-79:⚠️ Potential issue | 🟡 MinorUse explicit app check to avoid misclassification.
At Line 79,
siteType !== "web"treats unknown values as app. PrefersiteType === "app"so invalid/unexpected values stay on the safer web fallback path.💡 Proposed fix
- const siteType = siteMetadata?.type || "web"; - const isApp = siteType !== "web"; + const siteType = siteMetadata?.type ?? "web"; + const isApp = siteType === "app";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Header/NoData.tsx around lines 78 - 79, The variable isApp currently treats any non-"web" value as an app which can misclassify unknown site types; update the check in NoData.tsx so isApp is set using an explicit equality (siteType === "app") instead of inequality, keeping siteType computed as siteMetadata?.type || "web" and ensuring other code paths that rely on isApp (e.g., conditional rendering in the NoData component) continue to operate unchanged.server/src/api/sites/updateSiteConfig.ts-84-84 (1)
84-84:⚠️ Potential issue | 🟡 MinorRemove the
as anytype assertion; thetypefield is properly typed and can be accessed directly.The
sitesschema includestype: text("type").default("web").notNull(), and the Drizzle query returns the full row with proper typing. This is evident from other handlers likegetSite.tswhich accesssite.typewithout assertion. The cast bypasses TypeScript's type checking unnecessarily.Change line 84 from:
const siteType = (site as any).type || "web";to:
const siteType = site.type;The fallback is also unnecessary since the field has a database default of
"web"and is NOT NULL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/sites/updateSiteConfig.ts` at line 84, Remove the unnecessary type assertion and fallback: replace the use of (site as any).type with the properly typed site.type in updateSiteConfig.ts (the const siteType variable), since the sites schema guarantees a non-null default "web" and other handlers already access site.type directly; simply assign siteType = site.type and remove the "as any" cast and the || "web" fallback.client/src/app/[site]/components/SubHeader/Filters/utils.ts-10-13 (1)
10-13:⚠️ Potential issue | 🟡 MinorLocalize the new filter labels instead of hardcoding English.
Lines 10–13 add new user-facing strings that won’t go through translations. Please route these labels through your i18n key flow, same as other localized UI text.
As per coding guidelines "client/src/**/*.{ts,tsx}: Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/SubHeader/Filters/utils.ts around lines 10 - 13, The two hardcoded labels returned in Filters/utils.ts for the "device_model" and "app_version" cases must be localized: change the implementation to return translation keys (e.g., "filters.device_model" and "filters.app_version") or accept a translations function, and ensure the calling component uses next-intl's useTranslations() to resolve those keys (or import/use useTranslations() inside the helper) instead of hardcoded English; also add the corresponding entries to your messages/en.json (and other locale files) under the filters namespace so translations are available.client/src/components/SiteSettings/SiteSettings.tsx-43-44 (1)
43-44:⚠️ Potential issue | 🟡 MinorAvoid type assertions; use explicit type narrowing instead.
The code uses an unsafe
as "app"assertion (line 79) which bypasses TypeScript's strict typing requirements. SincesiteTypeis derived as"web" | "app"and the ternary condition already narrows to the "app" case in the else branch, the assertion is unnecessary.Explicitly type
siteTypeand use conditional narrowing to satisfy the type system without assertions:♻️ Suggested refactor
- const siteType = siteMetadata.type || "web"; + const siteType: "web" | "app" = siteMetadata.type === "app" ? "app" : "web";Then at line 79:
- siteType={siteType as "app"} + siteType="app"Per coding guidelines: "Use TypeScript with strict typing throughout both client and server."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteSettings.tsx` around lines 43 - 44, The code is using an unsafe type assertion (as "app") for siteType; instead explicitly type or infer siteType as "web" | "app" and remove the assertion. Update the SiteSettings component so the declaration of siteType (derived from siteMetadata.type || "web") is typed or inferred to the union "web" | "app", then rely on normal conditional narrowing in the branch where you previously used the assertion (the else/“app” branch) rather than forcing the type with as "app"; this keeps TypeScript strict and satisfies the compiler without assertions.client/src/app/components/AddSite.tsx-82-88 (1)
82-88:⚠️ Potential issue | 🟡 MinorPrevent stale app icon state from affecting web site creation.
If a user selects an app icon, then switches to Web,
iconBase64persists and Line 82 can still upload that icon for a web site. Clear icon state when switching to Web and guard upload bysiteType === "app".🔧 Suggested fix
- if (iconBase64) { + if (siteType === "app" && iconBase64) { try { await uploadSiteIcon(site.siteId, iconBase64); } catch { // Non-critical — site was created, icon upload failed silently } } ... - onClick={() => { setSiteType(value); setDomain(""); setError(""); }} + onClick={() => { + setSiteType(value); + setDomain(""); + setError(""); + if (value === "web") { + setIconPreview(null); + setIconBase64(null); + } + }}Also applies to: 204-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/components/AddSite.tsx` around lines 82 - 88, The code uploads iconBase64 regardless of site type; fix by clearing the icon state when the user switches to Web and by guarding uploads so uploadSiteIcon(site.siteId, iconBase64) only runs when siteType === "app". Locate the AddSite component state handlers that set iconBase64 and the UI switch between app/web, reset/clear the iconBase64 state on switching to Web, and wrap the existing uploadSiteIcon call with a check for siteType === "app" (also apply the same guard where similar upload logic exists around the other occurrence noted).client/src/app/components/AddSite.tsx-220-220 (1)
220-220:⚠️ Potential issue | 🟡 MinorLocalize new placeholder/alt strings.
Line 220 placeholder and Line 228 alt text are hardcoded English strings; these should go through
t(...)like the rest of the dialog copy.As per coding guidelines: `client/src/**/*.{ts,tsx}: Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)`.🌐 Suggested fix
- placeholder={siteType === "web" ? "example.com or sub.example.com" : "com.example.app"} + placeholder={ + siteType === "web" + ? t("example.com or sub.example.com") + : t("com.example.app") + } ... - <img src={iconPreview} alt="App icon preview" className="w-10 h-10 rounded" /> + <img src={iconPreview} alt={t("App icon preview")} className="w-10 h-10 rounded" />Also applies to: 228-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/components/AddSite.tsx` at line 220, In AddSite.tsx the placeholder ("example.com or sub.example.com" / "com.example.app") and the alt text at the same dialog are hardcoded; update the AddSite component to use next-intl's useTranslations() hook (e.g., const t = useTranslations('AddSite')) and replace those hardcoded strings with t('placeholder.web') / t('placeholder.mobile') and t('alt.input') (or similar keys) so they go through i18n, then add corresponding keys and translations to messages/en.json (and other locales).client/src/components/SiteSettings/SiteConfiguration.tsx-403-403 (1)
403-403:⚠️ Potential issue | 🟡 MinorLocalize newly added alt/placeholder text.
Line 403 (
alt="App icon") and Line 487 placeholder values are hardcoded. Route these through translations for non-English locales.As per coding guidelines: `client/src/**/*.{ts,tsx}: Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)`.🌐 Suggested fix
- alt="App icon" + alt={t("App icon")} ... - placeholder={siteType === "web" ? "example.com" : "com.example.app"} + placeholder={siteType === "web" ? t("example.com") : t("com.example.app")}Also applies to: 487-487
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` at line 403, The hardcoded strings for the image alt ("App icon") and input placeholder (line ~487) in the SiteConfiguration component should be routed through next-intl: import and call useTranslations() at the top of the SiteConfiguration component, add suitable keys (e.g., "site.appIconAlt" and "site.namePlaceholder") to the messages/*.json files, and replace the literal alt and placeholder values with t('site.appIconAlt') and t('site.namePlaceholder') respectively; update messages/en.json (and other locales) with the new keys and translations.client/src/components/SiteSettings/SiteConfiguration.tsx-423-424 (1)
423-424:⚠️ Potential issue | 🟡 MinorBlock concurrent icon upload/delete actions.
Upload and remove actions can run concurrently because each button only checks its own loading flag. Disable both while either mutation is in flight.
🔧 Suggested fix
<Button variant="outline" size="sm" - disabled={disabled || isUploadingIcon} + disabled={disabled || isUploadingIcon || isDeletingIcon} onClick={() => { const input = document.createElement("input"); ... <Button variant="ghost" size="sm" className="text-muted-foreground" - disabled={disabled || isDeletingIcon} + disabled={disabled || isDeletingIcon || isUploadingIcon} onClick={async () => {Also applies to: 453-465
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 423 - 424, The upload and delete icon buttons only check their own loading flags (e.g., isUploadingIcon and isDeletingIcon) so they can run concurrently; create a combined flag (e.g., isIconMutating = isUploadingIcon || isDeletingIcon) and use that in the disabled prop and any conditional UI for both the upload button (where disabled currently reads disabled || isUploadingIcon) and the delete/remove button (the block around lines 453-465) so that either mutation disables both buttons and prevents concurrent actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/de.json`:
- Around line 1157-1160: Two localization entries are empty which will cause
missing UI text: the keys "TJo5E6" and "JEsxDw" in client/messages/de.json;
update these keys with appropriate German translations (or remove the unused
keys) so they are not empty, ensuring any UI that references TJo5E6 or JEsxDw
displays proper text; locate where these keys are used in the codebase
(component/button/label referencing TJo5E6/JEsxDw) to confirm the correct
phrasing and consistency.
In `@client/messages/en.json`:
- Around line 36-40: The Czech locale is missing two translation keys present in
English: JEsxDw and TJo5E6; fix by syncing the locales—either run "npm run
extract" to auto-add missing keys to cs.json from en.json, or manually add
JEsxDw and TJo5E6 entries to the Czech locale file using the corresponding
values from the en.json source (ensure key names exactly match).
In `@client/messages/fr.json`:
- Around line 1156-1164: The two French localization keys are empty and must be
filled: set "TJo5E6" to "Aperçu" (Preview) and set "JEsxDw" to
"Téléversement..." (Uploading...) so the icon upload UI shows proper French
text; update the entries for TJo5E6 and JEsxDw in client/messages/fr.json
accordingly, matching the surrounding style (use escaped characters if needed).
In `@client/messages/it.json`:
- Around line 1157-1160: Two translation keys in client/messages/it.json are
blank (TJo5E6 and JEsxDw), which will render empty UI text; update those keys
with the correct Italian translations for the corresponding messages (match tone
and context of nearby keys like LUJTwh and nJNzqM), ensuring both strings are
non-empty and validate JSON remains well-formed after editing; if unsure of
exact copy, add a clear placeholder translation (e.g., a short Italian phrase)
so the UI is not blank and flag for proper localization review.
In `@client/messages/ko.json`:
- Around line 1157-1160: Add the missing Korean translations for the two empty
keys in client/messages/ko.json: set TJo5E6 to the Korean for "Preview" (미리보기)
and set JEsxDw to the Korean for "Uploading..." (업로드 중...) so UI labels/buttons
are not blank; update the values for keys "TJo5E6" and "JEsxDw" accordingly.
In `@client/src/components/SiteSettings/SDKIntegration.tsx`:
- Around line 24-26: The Dart snippet currently uses the wrong SDK API (await
Rybbit.init(...)); update it to use RybbitFlutter.instance.initialize and wrap
params in a RybbitConfig constructor, replacing the host param with apiKey and
keeping siteId; i.e., call
RybbitFlutter.instance.initialize(RybbitConfig(apiKey: '...', siteId: '...')) so
the displayed snippet matches the actual Rybbit Flutter SDK usage.
In `@server/src/api/sites/siteIcon.ts`:
- Around line 18-23: Validate and reject invalid or nonexistent site IDs before
returning success: parse and check request.params.siteId (e.g., use
Number(siteId) or parseInt and if isNaN return a 400), then after calling
db.query.sites.findFirst (the query using eq(sites.siteId, Number(siteId)) and
columns: { icon: true }) verify the returned site row is not null and return a
404 if missing; apply the same validation/exists check to the upload and delete
handlers so they don't return 200 for invalid or nonexistent site IDs.
---
Outside diff comments:
In `@client/src/app/`[site]/main/components/MainSection/MainSection.tsx:
- Line 4: Replace the experimental useExtracted hook import and usage with the
standard useTranslations hook: change the import line to import {
useTranslations } from "next-intl" (replacing useExtracted), and update any
calls of useExtracted(...) in MainSection (e.g., variable assignments or
destructuring) to useTranslations(...) with the same namespace/key argument(s);
ensure any returned value names (like t or translations) remain consistent and
adjust call sites accordingly.
In `@client/src/app/`[site]/main/components/sections/Events.tsx:
- Line 2: The component Events has state variable tab and conditional rendering
that hides the "outbound" tab when isApp is true/false, but it does not reset
tab if it currently equals "outbound", leaving an empty panel; inside the Events
component add a small effect or conditional update that watches isApp and tab
and when isApp causes the outbound tab to be hidden (e.g., isApp === true or
false per your logic) and tab === "outbound", setTab to a valid fallback (for
example "inbound" or the first visible tab); reference the Events component, the
tab state setter setTab, and the isApp value to implement this guard so the UI
never retains "outbound" while that tab is hidden.
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Line 18: The import of useExtracted from next-intl is invalid; update the
import in UsersTable.tsx to import useTranslations instead and replace any call
sites of useExtracted (e.g., the hook invocation used around the UsersTable
component at the spot referenced on line 79) with useTranslations(), ensuring
the variable name and subsequent calls (like t(...) or similar translation
lookups) match the useTranslations API.
---
Minor comments:
In `@client/messages/cs.json`:
- Line 1092: Update the Czech translation for key "z0p82B" to use the informal
"ty" form to match the rest of the file (e.g., change "Nainstalujte Rybbit
Flutter SDK pro sledování událostí z vaší aplikace." to an informal variant like
"Nainstaluj Rybbit Flutter SDK pro sledování událostí z tvé aplikace.") and
similarly review the translation referenced around line 924 ("Začněte...nebudete
účtováni") to convert any remaining formal "vy"/"vaší" forms to the informal
"ty"/"tvé" forms for consistency.
In `@client/messages/de.json`:
- Around line 636-637: The two adjacent i18n entries are inconsistent: key
"VxUjLr" uses "App hinzufügen" while key "m5aC6r" still says "neue Website";
update the copy so both refer to the same concept (App or Website). Locate the
keys "VxUjLr" and "m5aC6r" and change the value for "m5aC6r" to a phrasing
consistent with app mode (e.g., replace "neue Website" with "neue App") or
alternatively change "VxUjLr" to mention "Website" if that is the intended
domain, ensuring both strings use the same noun and tone.
- Around line 36-39: The German strings use informal phrasing; update the
entries with keys "BiyUAo", "Z8TXx9", "5qOEz+", and "3jM2wN" to the formal
register (Sie/Ihre) to match the rest of the locale file (e.g., change verbs and
pronouns from informal to formal). Also check and correct the same register for
the other affected keys mentioned in the review so all German strings
consistently use the formal tone across the file.
In `@client/messages/ja.json`:
- Line 1157: Remove the unused translation keys TJo5E6 and JEsxDw from the
ja.json translations (they are empty and not referenced anywhere) by deleting
those entries; if they are intended placeholders for upcoming features, instead
retain them but add a clear inline comment explaining their purpose and planned
use so future reviewers understand they are intentional.
In `@client/messages/ko.json`:
- Line 1136: Update the translation value for the key "c1bAPv" in
client/messages/ko.json from the noun phrase to a verb-style description to
match surrounding sentences—replace "앱의 오류 및 예외 캡처" with a verb-form like "앱의 오류
및 예외를 캡처합니다" (or alternatively "앱의 오류 및 예외를 추적합니다") so the tone matches adjacent
explanatory lines.
- Around line 636-637: The description for key "m5aC6r" is web-specific while
key "VxUjLr" is "앱 추가"; update the "m5aC6r" value to use a neutral term to match
the app context (for example change "조직에서 새 웹사이트의 분석을 추적합니다" to "조직에서 새 사이트의 분석을
추적합니다" or "조직에서 새 앱/사이트의 분석을 추적합니다") so both "VxUjLr" and "m5aC6r" are
consistent.
In `@client/messages/pl.json`:
- Around line 1157-1160: Fill the two empty Polish translation keys by replacing
the empty strings for "TJo5E6" and "JEsxDw" with Polish text: set "TJo5E6" to
the Polish word for "Preview" (e.g., "Podgląd") and set "JEsxDw" to the Polish
phrase for "Uploading..." (e.g., "Przesyłanie...") so the icon upload/delete UI
shows localized text; update the values for the keys TJo5E6 and JEsxDw in
client/messages/pl.json accordingly.
In `@client/messages/zh.json`:
- Around line 1157-1160: Add Chinese translations for the missing keys in
client/messages/zh.json: set "TJo5E6" to "预览" for the "Preview" label and set
"JEsxDw" to "正在上传…" for the "Uploading..." status; update the existing JSON
entries for the keys TJo5E6 and JEsxDw so they match the English semantics and
preserve JSON formatting.
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Around line 78-79: The variable isApp currently treats any non-"web" value as
an app which can misclassify unknown site types; update the check in NoData.tsx
so isApp is set using an explicit equality (siteType === "app") instead of
inequality, keeping siteType computed as siteMetadata?.type || "web" and
ensuring other code paths that rely on isApp (e.g., conditional rendering in the
NoData component) continue to operate unchanged.
In `@client/src/app/`[site]/components/Sidebar/Sidebar.tsx:
- Line 39: The transient flash happens because isApp is false while site is
still undefined; change the logic to treat isApp as unknown until site is loaded
and only render site-type-dependent UI after site is non-null. Concretely,
compute isApp with a guarded check like "const isApp = site != null ? site.type
=== 'app' : undefined" (or use "site != null && site.type === 'app'") and update
the Sidebar render branches (the header label and the web-only items
'Performance' and 'Replay' in the Sidebar component) to only render when site !=
null (or isApp === true/false explicitly), preventing rendering when site is
unresolved.
In `@client/src/app/`[site]/components/Sidebar/SiteSelector.tsx:
- Around line 107-111: Remove the redundant Smartphone icon rendered in the
SiteSelector component: the Favicon component already shows the Smartphone when
site.type !== "web", so delete the conditional JSX that renders <Smartphone ...
/> in SiteSelector.tsx (the block that checks site.type && site.type !== "web")
to avoid duplicate icons; leave the Favicon usage intact.
In `@client/src/app/`[site]/components/SubHeader/Filters/utils.ts:
- Around line 10-13: The two hardcoded labels returned in Filters/utils.ts for
the "device_model" and "app_version" cases must be localized: change the
implementation to return translation keys (e.g., "filters.device_model" and
"filters.app_version") or accept a translations function, and ensure the calling
component uses next-intl's useTranslations() to resolve those keys (or
import/use useTranslations() inside the helper) instead of hardcoded English;
also add the corresponding entries to your messages/en.json (and other locale
files) under the filters namespace so translations are available.
In `@client/src/app/components/AddSite.tsx`:
- Around line 82-88: The code uploads iconBase64 regardless of site type; fix by
clearing the icon state when the user switches to Web and by guarding uploads so
uploadSiteIcon(site.siteId, iconBase64) only runs when siteType === "app".
Locate the AddSite component state handlers that set iconBase64 and the UI
switch between app/web, reset/clear the iconBase64 state on switching to Web,
and wrap the existing uploadSiteIcon call with a check for siteType === "app"
(also apply the same guard where similar upload logic exists around the other
occurrence noted).
- Line 220: In AddSite.tsx the placeholder ("example.com or sub.example.com" /
"com.example.app") and the alt text at the same dialog are hardcoded; update the
AddSite component to use next-intl's useTranslations() hook (e.g., const t =
useTranslations('AddSite')) and replace those hardcoded strings with
t('placeholder.web') / t('placeholder.mobile') and t('alt.input') (or similar
keys) so they go through i18n, then add corresponding keys and translations to
messages/en.json (and other locales).
In `@client/src/components/Sessions/SessionCard.tsx`:
- Around line 39-42: The site metadata query should be bound to the current
route param: instead of calling useGetSite() with store fallback, pass the route
value from useParams (the site variable) into useGetSite so it always fetches
metadata for the current route; update references that use siteMetadata (e.g.,
isApp = siteMetadata?.type === "app") to rely on the result returned from
useGetSite(site). Ensure the callsite is changed from useGetSite() to
useGetSite(site) (or the appropriate single-arg signature your hook expects) so
navigation/delayed-store sync won't produce app/web mode mismatches.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Line 403: The hardcoded strings for the image alt ("App icon") and input
placeholder (line ~487) in the SiteConfiguration component should be routed
through next-intl: import and call useTranslations() at the top of the
SiteConfiguration component, add suitable keys (e.g., "site.appIconAlt" and
"site.namePlaceholder") to the messages/*.json files, and replace the literal
alt and placeholder values with t('site.appIconAlt') and
t('site.namePlaceholder') respectively; update messages/en.json (and other
locales) with the new keys and translations.
- Around line 423-424: The upload and delete icon buttons only check their own
loading flags (e.g., isUploadingIcon and isDeletingIcon) so they can run
concurrently; create a combined flag (e.g., isIconMutating = isUploadingIcon ||
isDeletingIcon) and use that in the disabled prop and any conditional UI for
both the upload button (where disabled currently reads disabled ||
isUploadingIcon) and the delete/remove button (the block around lines 453-465)
so that either mutation disables both buttons and prevents concurrent actions.
In `@client/src/components/SiteSettings/SiteSettings.tsx`:
- Around line 43-44: The code is using an unsafe type assertion (as "app") for
siteType; instead explicitly type or infer siteType as "web" | "app" and remove
the assertion. Update the SiteSettings component so the declaration of siteType
(derived from siteMetadata.type || "web") is typed or inferred to the union
"web" | "app", then rely on normal conditional narrowing in the branch where you
previously used the assertion (the else/“app” branch) rather than forcing the
type with as "app"; this keeps TypeScript strict and satisfies the compiler
without assertions.
In `@server/src/api/sites/updateSiteConfig.ts`:
- Line 84: Remove the unnecessary type assertion and fallback: replace the use
of (site as any).type with the properly typed site.type in updateSiteConfig.ts
(the const siteType variable), since the sites schema guarantees a non-null
default "web" and other handlers already access site.type directly; simply
assign siteType = site.type and remove the "as any" cast and the || "web"
fallback.
---
Nitpick comments:
In `@client/messages/en.json`:
- Line 631: The messages for package-name validation are inconsistent between
keys (e.g. "39ieyq" at line 631 and the corresponding key at line 1096); update
both entries to the same unified copy (pick one canonical string such as
"Invalid package name format. Use reverse-domain notation like com.example.app")
or explicitly separate semantics by creating two distinct keys (one for inline
hint, one for toast) and adjust callers accordingly; locate and update the
message key "39ieyq" and the other package-name validation key so they match the
chosen wording and ensure tests/localization references use the new unified
key(s).
In `@client/messages/it.json`:
- Around line 636-643: Replace the Italian copy for the app creation description
keyed by "m5aC6r" so it uses neutral "sito" wording instead of "nuovo sito web"
(e.g., change "Traccia le analisi per un nuovo sito web nella tua
organizzazione" to "Traccia le analisi per un nuovo sito nella tua
organizzazione") to keep app-site onboarding language consistent; update the
string value for key "m5aC6r" in client/messages/it.json accordingly.
In `@client/messages/ja.json`:
- Around line 1017-1018: Update the Japanese translation entries that use
half-width colons to full-width colons for consistency: change the values for
keys "tH05Pi" ("デバイスモデル:") and "ShHw0c" ("アプリバージョン:") (and the entry around 1027
mentioned in the review) so each string uses the full-width colon "：" to match
neighboring strings like "ブラウザ：" and "OS：". Ensure only the punctuation is
changed and keep the rest of each string intact.
In `@client/messages/pt.json`:
- Around line 1017-1018: The translation uses inconsistent terminology ("app" vs
"aplicativo"); update the Portuguese string for key "ShHw0c" from "Versão do
app:" to "Versão do aplicativo:" (or vice‑versa if you choose the informal form)
so it matches the other occurrence and keep terminology consistent across the
file (search for other keys like "ShHw0c" and "tH05Pi" to verify and update all
matches).
In `@client/src/api/admin/endpoints/sites.ts`:
- Around line 162-173: Rename the endpoint helper uploadSiteIcon to follow
conventions by changing its name to updateSiteIcon and update any
references/exports accordingly; keep the function body, signature (siteId:
number, icon: string) and the authedFetch call intact (the DELETE function
deleteSiteIcon remains unchanged), and update all call sites that import or
invoke uploadSiteIcon to use updateSiteIcon so the PUT-based update endpoint
name matches the project's fetch/create/update/delete naming guideline.
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx:
- Line 112: In the EventDetailsSheet component update the ambiguous labels:
change the conditional label span (the JSX using isApp ? t("Screen") : t("Path")
in the span with className "text-neutral-500 dark:text-neutral-400") to use a
clearer key like t("Screen Route") or t("Screen Name") for the app path case,
and update the other occurrence that currently uses t("Screen") for display size
(around the screen dimensions field) to t("Screen Dimensions"); ensure you
add/adjust the corresponding translation keys used by the t(...) calls so both
labels are distinct and localized.
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Around line 98-131: EventRow hides Browser and DeviceIcon for app sites,
leaving less context for app events; update the JSX in EventRow.tsx to show
app-specific metadata when isApp is true by rendering device_model and/or
app_version inside the existing Tooltip/TooltipContent pattern (reuse Tooltip,
TooltipTrigger, TooltipContent components and OperatingSystem/Browser layout) —
e.g., when isApp render a Tooltip that shows event.device_model || t("Unknown
device model") and another Tooltip (or augment an existing slot) to show
event.app_version || t("Unknown app version"); keep existing fallbacks and
translation calls and only change the conditional branches that currently skip
Browser and DeviceIcon.
In `@client/src/app/`[site]/events/page.tsx:
- Around line 17-25: siteMetadata may be undefined while useGetSite is loading,
causing isApp to default to false and getEventFilters(false) to render then
flip; update the component to wait for the site metadata loading state from
useGetSite (e.g., useGetSite's isLoading or similar) before calling
getEventFilters and rendering SubHeader inside DisabledOverlay, or compute
filters based on a guarded isApp (e.g., only compute
getEventFilters(siteMetadata?.type === "app") after siteMetadata is defined) so
the SubHeader (and getEventFilters) doesn't flicker when siteMetadata resolves;
reference useGetSite, siteMetadata, isApp, getEventFilters, SubHeader, and
DisabledOverlay when making the change.
In `@client/src/app/`[site]/goals/page.tsx:
- Around line 3-18: Imports are not grouped/sorted per guidelines; move external
packages (e.g., Target from "lucide-react", Link from "next/link", useExtracted
from "next-intl", useMemo/useState from "react") to the top as one block and
sort them alphabetically, then place all internal imports (e.g., Input from
"@/components/ui/input", useGetGoals, useGetSite, useStore, getGoalsPageFilters,
useSetPageTitle, SubHeader, CreateGoalButton, GoalsList, DisabledOverlay,
NothingFound, Pagination) in a separate block sorted alphabetically; update the
import order in page.tsx accordingly and run your formatter/linter to ensure
correct spacing and order.
In `@client/src/app/`[site]/main/components/sections/Weekdays.tsx:
- Around line 129-140: The metric label mapping is duplicated; instead of
re-implementing the isApp branching when building the select options, reuse the
existing getMetricDisplayName(metric: StatType) helper to produce labels
consistently; update the select rendering code (the component that currently
repeats the "pageviews"/"screens" and "pages_per_session"/"screens per session"
branching) to call getMetricDisplayName for each StatType option so tooltip and
select remain in sync.
In `@client/src/app/`[site]/pages/components/PageListItem.tsx:
- Around line 85-87: The call to usePageMetadata currently passes an empty
string when isApp is true; instead pass null to make intent clearer and match
the hook signature (string | null). Update the invocation in PageListItem where
usePageMetadata(isApp ? "" : pageUrl) is used to use usePageMetadata(isApp ?
null : pageUrl) so the hook's enabled: !!pageUrl guard behaves clearly when
input is absent.
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Around line 80-81: UsersTable currently derives isApp from siteMetadata (const
isApp = siteMetadata?.type === "app") which treats undefined as false and causes
a flash of web-mode columns while metadata loads; update UsersTable to read the
loading state from useGetSite (e.g., isLoading or isSiteLoading) and avoid
rendering mode-specific columns until loading finishes—either render a column
skeleton/placeholder or a neutral column set while isSiteLoading is true, then
switch to app-mode when isApp is true; update any conditional rendering logic
that uses isApp to check isSiteLoading first so columns only change after
siteMetadata is resolved.
In `@client/src/components/Sessions/SessionCard.tsx`:
- Line 5: The import statement in SessionCard.tsx currently mixes imports;
reorder imports so external packages come first and internal imports after,
sorting names alphabetically within each group — for example ensure the
lucide-react symbols (ArrowRight, ChevronDown, ChevronRight, Smartphone, Video)
are alphabetized and placed with other external imports, and move any
local/module imports to the internal group (also alphabetized); apply the same
grouping/sorting rule to the other affected import on the file (line referenced
28-28) to comply with the project import organization guideline.
- Around line 168-193: Create a small shared render helper/component (e.g.,
SessionBadges or renderDeviceOrChannel) that takes props { isApp, session,
handleFilterClick } and contains the logic that currently appears inline: when
isApp is true render the device/app Badges (use session.device_model,
session.app_version, Badge, Smartphone and onClick calling handleFilterClick
with keys "device_model" and "app_version"), otherwise render the Channel
component with channel={session.channel}, referrer={session.referrer} and
onClick calling handleFilterClick("channel", session.channel); replace both
duplicated inline blocks with this new helper, export/define appropriate props
types, and memoize the component if needed to avoid unnecessary re-renders.
In `@client/src/components/SiteSettings/SDKIntegration.tsx`:
- Around line 4-12: The SDKIntegration component declares props siteType and
domain but never uses them; update the component signature and any related types
to remove siteType and domain (leaving only siteId) or alternatively use them in
the render logic (for example include domain in displayed text or branch
behavior by siteType); edit the SDKIntegration function parameter destructuring
and its type annotation to match whichever approach you choose (references:
SDKIntegration, siteType, domain, siteId).
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Around line 124-139: For app sites (when siteType !== "web") trim the user
input once before validating and updating: call .trim() on newDomain into a
local variable (e.g., trimmedDomain), use that for isValidPackageName validation
and for the value passed to updateSiteConfig; for web sites continue to
validate/normalize as before (use normalizeDomain only after trimming if you
choose to trim web input too). Update references in this block (siteType,
isValidPackageName, normalizeDomain, updateSiteConfig, setIsChangingDomain) so
validation and the update use the trimmed value to avoid failing on
leading/trailing whitespace.
In `@client/src/lib/filterGroups.ts`:
- Around line 48-50: getEventFilters currently returns
[...getBaseFilters(isApp), "pathname", "page_title", "event_name", "entry_page",
"exit_page"], but "page_title" is already provided by COMMON_FILTERS via
getBaseFilters; remove the redundant "page_title" string from the returned array
in getEventFilters so the list becomes [...getBaseFilters(isApp), "pathname",
"event_name", "entry_page", "exit_page"] (keep getBaseFilters, COMMON_FILTERS,
and the other filter names unchanged).
In `@client/src/lib/imageUtils.ts`:
- Line 26: Replace the direct stretch call to ctx.drawImage(img, 0, 0, size,
size) with an aspect-ratio-preserving center-crop: compute the source crop
rectangle (sx, sy, sWidth, sHeight) based on img.width and img.height so the
larger dimension is cropped to match the square target, then call
ctx.drawImage(img, sx, sy, sWidth, sHeight, 0, 0, size, size). Update the
function that performs this draw (the code containing ctx.drawImage and
variables img and size) to calculate those crop coordinates before drawing so
non-square images are not stretched.
In `@server/src/api/sites/siteIcon.ts`:
- Around line 58-61: The PNG validation in siteIcon.ts uses only the first 4
bytes; update the pngMagic constant and length check to use the full 8-byte PNG
signature (0x89,0x50,0x4E,0x47,0x0D,0x0A,0x1A,0x0A) and change the buffer length
guard and subarray comparison to 8 bytes so the validation in the icon upload
handler correctly rejects malformed PNGs.
In `@server/src/db/postgres/schema.ts`:
- Line 73: Add a DB-level check constraint to enforce that the "type" column
only contains 'web' or 'app': inside the table's constraint array (next to other
constraints in the schema where the "type" column is declared) add a check
constraint named sites_type_check that uses sql`type IN ('web','app')`;
alternatively replace the text("type") column with a pgEnum similar to
importPlatformEnum and use that enum type to restrict values. Ensure you add the
constraint/enum alongside the existing column definition so the migration
enforces allowed values.
In `@server/src/services/tracker/utils.ts`:
- Line 135: Remove the "as any" cast on the returned object and make the return
value conform to the TotalTrackingPayload type: locate the return in
server/src/services/tracker/utils.ts (the object that currently has app_version
and device_model) and remove "as any", then either adjust the object fields
(ensure app_version and device_model types match TotalTrackingPayload) or
update/transform values to the expected types so the TypeScript compiler accepts
the return as TotalTrackingPayload without using any-casts.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (77)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/sites.tsclient/src/api/analytics/endpoints/events.tsclient/src/api/analytics/endpoints/sessions.tsclient/src/api/analytics/endpoints/users.tsclient/src/app/[site]/components/Header/NoData.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/components/Sidebar/SiteSelector.tsxclient/src/app/[site]/components/SubHeader/Export/exportCsv.tsclient/src/app/[site]/components/SubHeader/Filters/FilterComponent.tsxclient/src/app/[site]/components/SubHeader/Filters/Filters.tsxclient/src/app/[site]/components/SubHeader/Filters/const.tsxclient/src/app/[site]/components/SubHeader/Filters/utils.tsclient/src/app/[site]/errors/components/EnableErrorTracking.tsxclient/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsxclient/src/app/[site]/events/components/EventLog/EventRow.tsxclient/src/app/[site]/events/components/EventLog/eventLogUtils.tsxclient/src/app/[site]/events/components/EventTypesChart.tsxclient/src/app/[site]/events/page.tsxclient/src/app/[site]/funnels/page.tsxclient/src/app/[site]/goals/page.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/main/components/MainSection/MainSection.tsxclient/src/app/[site]/main/components/MainSection/Overview.tsxclient/src/app/[site]/main/components/sections/Devices.tsxclient/src/app/[site]/main/components/sections/Events.tsxclient/src/app/[site]/main/components/sections/Pages.tsxclient/src/app/[site]/main/components/sections/Weekdays.tsxclient/src/app/[site]/main/page.tsxclient/src/app/[site]/pages/components/PageListItem.tsxclient/src/app/[site]/sessions/page.tsxclient/src/app/[site]/user/[userId]/components/UserSidebar.tsxclient/src/app/[site]/users/components/UsersTable.tsxclient/src/app/[site]/users/page.tsxclient/src/app/components/AddSite.tsxclient/src/app/page.tsxclient/src/components/EventTypeFilter.tsxclient/src/components/Favicon.tsxclient/src/components/Sessions/SessionCard.tsxclient/src/components/Sessions/SessionDetails/SessionInfoTab.tsxclient/src/components/SiteCard.tsxclient/src/components/SiteSettings/SDKIntegration.tsxclient/src/components/SiteSettings/SiteConfiguration.tsxclient/src/components/SiteSettings/SiteSettings.tsxclient/src/lib/filterGroups.tsclient/src/lib/imageUtils.tsclient/src/lib/utils.tsserver/src/api/analytics/events/getEvents.tsserver/src/api/analytics/sessions/getSession.tsserver/src/api/analytics/sessions/getSessions.tsserver/src/api/analytics/users/getUserInfo.tsserver/src/api/analytics/users/getUsers.tsserver/src/api/analytics/utils/query-validation.tsserver/src/api/sites/addSite.tsserver/src/api/sites/getSite.tsserver/src/api/sites/index.tsserver/src/api/sites/siteIcon.tsserver/src/api/sites/updateSiteConfig.tsserver/src/db/clickhouse/clickhouse.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/services/tracker/pageviewQueue.tsserver/src/services/tracker/trackEvent.tsserver/src/services/tracker/utils.tsserver/src/utils.tsshared/src/filters.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/fr.json`:
- Line 1160: The French translation for the icon upload flow uses the wrong
word; find the JSON entry with key "JEsxDw" and change its value from
"Téléchargement..." (downloading) to the correct uploading term
"Téléversement..." so the string matches the upload context used elsewhere
(e.g., Spanish "Subiendo..." and Japanese "アップロード中...").
- Line 36: The value for the translation key "BiyUAo" is inconsistent (currently
"Mobile"); update the French translation for "BiyUAo" to match the SDK
installation instruction style used in other locales (e.g., replace with a
sentence like "Ajoutez le SDK Rybbit à votre application :" or similar French
phrasing) so the UI text is consistent across languages.
In `@client/messages/pt.json`:
- Around line 512-513: The "fsUN9Y" translation currently duplicates the
page-view label ("Mín. visualizações") and should explicitly indicate "screen
views"; update the value for the fsUN9Y key (not Z2Mi3c) to a distinct
Portuguese string like "Mín. visualizações de tela" (or similar phrasing you
prefer) so the UI differentiates the screen-views filter from the page-views
filter.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonserver/src/api/sites/siteIcon.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/api/sites/siteIcon.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/src/app/[site]/components/Header/NoData.tsx (1)
103-117: Deduplicate the Flutter SDK URL to prevent future link drift.The same URL is repeated at Lines 103 and 116. Extract one constant and reuse it.
Suggested change
+const FLUTTER_SDK_URL = "https://github.com/nks-hub/rybbit-flutter-sdk"; ... - <a href="https://github.com/nks-hub/rybbit-flutter-sdk" target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:underline"> + <a href={FLUTTER_SDK_URL} target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:underline"> ... - href="https://github.com/nks-hub/rybbit-flutter-sdk" + href={FLUTTER_SDK_URL}As per coding guidelines,
client/**/*.{ts,tsx}should use UPPER_SNAKE_CASE for constants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Header/NoData.tsx around lines 103 - 117, Create a single UPPER_SNAKE_CASE constant (e.g., FLUTTER_SDK_URL) in the NoData.tsx component and replace the repeated hardcoded URL occurrences (the anchor href and the Card href) with that constant; update the anchor and the Card props in the NoData component to reference FLUTTER_SDK_URL so the link is defined once and reused to prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Around line 91-95: The current JSX always renders the domain-centric message
using siteMetadata?.domain; update the rendering in the NoData component so the
font-medium div shows app-specific copy when isApp is true (e.g., use t("Waiting
for analytics from your app...") or similar) and only uses t("Waiting for
analytics from {domain}...", { domain: siteMetadata?.domain ?? "" }) when isApp
is false; modify the conditional around the div with className "font-medium" in
NoData.tsx to branch on isApp instead of always interpolating
siteMetadata?.domain.
- Around line 78-79: Replace the defensive fallback and negative check pattern:
compute siteType from siteMetadata using the nullish coalescing operator
(siteMetadata?.type ?? "web") and determine app membership with an explicit
positive comparison (isApp = siteType === "app") instead of siteType !== "web";
update the variables siteType and isApp accordingly to match the codebase
convention used elsewhere.
---
Nitpick comments:
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Around line 103-117: Create a single UPPER_SNAKE_CASE constant (e.g.,
FLUTTER_SDK_URL) in the NoData.tsx component and replace the repeated hardcoded
URL occurrences (the anchor href and the Card href) with that constant; update
the anchor and the Card props in the NoData component to reference
FLUTTER_SDK_URL so the link is defined once and reused to prevent drift.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/app/[site]/components/Header/NoData.tsxclient/src/components/SiteSettings/SDKIntegration.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/components/SiteSettings/SDKIntegration.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
client/src/app/[site]/components/Header/NoData.tsx (1)
109-109: Remove emptyclassNameattributes.Lines 109 and 142 have
className=""which is unnecessary.✨ Suggested cleanup
- <div className=""> + <div>Also applies to: 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Header/NoData.tsx at line 109, In the NoData React component remove the redundant empty className attributes on the container divs: locate the JSX in the NoData component (the top-level <div className=""> and the other <div className="">) and delete the className="" props so the elements render without empty attributes; verify there are no other empty className usages in the same component.client/messages/fr.json (1)
1136-1136: Consider terminology consistency: “app” vs “application”.Line 1136 uses “app” while nearby strings mostly use “application”. Unifying the term would make the French copy more consistent.
💬 Optional consistency tweak
- "c1bAPv": "Capturer les erreurs et exceptions de votre app", + "c1bAPv": "Capturer les erreurs et exceptions de votre application",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/fr.json` at line 1136, Update the French string value for the key "c1bAPv" in client/messages/fr.json to use "application" instead of "app" for terminology consistency with nearby strings; locate the entry with key "c1bAPv" and replace "Capturer les erreurs et exceptions de votre app" with the equivalent using "application" (e.g., "Capturer les erreurs et exceptions de votre application").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/fr.json`:
- Line 631: Update the French message for key "39ieyq": replace the wording
"Doit être comme com.example.app" with the more natural UI copy "Doit être au
format com.example.app" so the hint reads "Format de nom de paquet invalide.
Doit être au format com.example.app"; ensure you only change the string value
for "39ieyq".
In `@client/messages/pt.json`:
- Around line 1017-1018: The Portuguese translation uses inconsistent
terminology for "app" — update the string value for key "ShHw0c" (currently
"Versão do app:") to match the other file entry that uses "Versão do aplicativo"
so both labels use the same term; locate key "ShHw0c" in client/messages/pt.json
and replace "app" with "aplicativo" to ensure consistency with the existing
"Versão do aplicativo" translation.
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Around line 124-128: The template literal passed to the CodeSnippet in
NoData.tsx uses globalThis.location.origin without optional chaining which can
crash during SSR; update that string to use globalThis.location?.origin (e.g.,
change `${globalThis.location.origin}/api/script.js` to
`${globalThis.location?.origin}/api/script.js`) so it matches the safe usage
already present in the other branch and preserves siteMetadata usage
(`siteMetadata?.id ?? siteMetadata?.siteId`) while avoiding runtime errors on
the server.
---
Nitpick comments:
In `@client/messages/fr.json`:
- Line 1136: Update the French string value for the key "c1bAPv" in
client/messages/fr.json to use "application" instead of "app" for terminology
consistency with nearby strings; locate the entry with key "c1bAPv" and replace
"Capturer les erreurs et exceptions de votre app" with the equivalent using
"application" (e.g., "Capturer les erreurs et exceptions de votre application").
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Line 109: In the NoData React component remove the redundant empty className
attributes on the container divs: locate the JSX in the NoData component (the
top-level <div className=""> and the other <div className="">) and delete the
className="" props so the elements render without empty attributes; verify there
are no other empty className usages in the same component.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/messages/fr.jsonclient/messages/pt.jsonclient/src/app/[site]/components/Header/NoData.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
client/messages/fr.json (1)
1136-1140: Minor terminology inconsistency: "app" vs "application".Line 1136 uses "votre app" while other translations in this file consistently use "application" (e.g., line 115 "Version de l'application", line 182 "votre application"). Consider aligning for consistency.
✏️ Suggested fix
- "c1bAPv": "Capturer les erreurs et exceptions de votre app", + "c1bAPv": "Capturer les erreurs et exceptions de votre application",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/fr.json` around lines 1136 - 1140, Replace the inconsistent shorthand "votre app" with "votre application" for the translation key "c1bAPv" in client/messages/fr.json so the phrasing matches other entries (e.g., "Version de l'application") and maintains consistent terminology across the file.client/messages/pt.json (1)
1136-1140: Minor terminology inconsistency: "app" vs "aplicativo".Line 1136 uses "seu app" while other translations consistently use "aplicativo" (e.g., line 115, 182, 390, 1018, 1092). Consider aligning for consistency.
✏️ Suggested fix
- "c1bAPv": "Capture erros e exceções do seu app", + "c1bAPv": "Capture erros e exceções do seu aplicativo",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/pt.json` around lines 1136 - 1140, Replace the inconsistent term "app" with "aplicativo" in the Portuguese translation for the message key "c1bAPv" so it matches other entries; update the value "Capture erros e exceções do seu app" to use "aplicativo" (e.g., "Capture erros e exceções do seu aplicativo") and keep punctuation/formatting consistent with surrounding strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@client/messages/fr.json`:
- Around line 1136-1140: Replace the inconsistent shorthand "votre app" with
"votre application" for the translation key "c1bAPv" in client/messages/fr.json
so the phrasing matches other entries (e.g., "Version de l'application") and
maintains consistent terminology across the file.
In `@client/messages/pt.json`:
- Around line 1136-1140: Replace the inconsistent term "app" with "aplicativo"
in the Portuguese translation for the message key "c1bAPv" so it matches other
entries; update the value "Capture erros e exceções do seu app" to use
"aplicativo" (e.g., "Capture erros e exceções do seu aplicativo") and keep
punctuation/formatting consistent with surrounding strings.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/messages/fr.jsonclient/messages/pt.jsonclient/src/app/[site]/components/Header/NoData.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/app/[site]/components/Header/NoData.tsx
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
lukyrys
force-pushed
the
feat/mobile-platform-support
branch
from
007430a to
980f196
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/app/[site]/users/components/UsersTable.tsx (1)
18-18:⚠️ Potential issue | 🔴 CriticalReplace
useExtractedwith the correctuseTranslationshook from next-intl.The
useExtractedhook does not exist in next-intl and will cause an import error. UseuseTranslationsinstead, which is the standard hook for accessing translations in next-intl.Required changes
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";Line 79:
- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/components/UsersTable.tsx at line 18, The import and hook usage currently reference the non-existent useExtracted hook; replace the import of useExtracted with useTranslations from next-intl and update any calls to useExtracted(...) to useTranslations(...) (e.g., in the UsersTable component where the translation hook is invoked around line 79) so the component uses the correct next-intl hook for translations.client/src/app/[site]/main/components/sections/Pages.tsx (1)
5-5:⚠️ Potential issue | 🔴 CriticalReplace
useExtractedwithuseTranslations— this is a client file that must use the correct next-intl hook.This file is located in
client/src/**and must useuseTranslations()per coding guidelines, notuseExtracted(which is reserved for docs files).🐛 Proposed fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";And at line 20:
- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/main/components/sections/Pages.tsx at line 5, The file imports and uses the wrong next-intl hook: replace the import of useExtracted with useTranslations and update the hook call inside the Pages component (where useExtracted is invoked around line 20) to call useTranslations instead; ensure the variable name and any subsequent calls (e.g., t(...) or similar) remain compatible with useTranslations so translations still resolve correctly.
🧹 Nitpick comments (13)
client/src/app/[site]/users/components/UsersTable.tsx (1)
248-253: Consider adding filter click functionality toapp_versioncolumn.The
device_modelcolumn (line 178) supports click-to-filter, butapp_versionis display-only. If filtering by app version is a supported use case, consider adding the same interaction pattern for consistency.♻️ Proposed enhancement
columnHelper.accessor("app_version", { header: t("App Version"), - cell: info => <div className="whitespace-nowrap">{info.getValue() || "—"}</div>, + cell: info => { + const version = info.getValue(); + return ( + <div + className="whitespace-nowrap cursor-pointer hover:opacity-70" + onClick={e => handleFilterClick(e, "app_version", version)} + > + {version || "—"} + </div> + ); + }, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/components/UsersTable.tsx around lines 248 - 253, The app_version column is currently display-only; to add click-to-filter parity with the device_model column, change the column definition created by columnHelper.accessor("app_version") in UsersTable.tsx to render the cell as an interactive element that calls the same filter action used by device_model (e.g., the table's setFilter or toggle logic). Specifically, mirror the device_model cell's behavior: wrap info.getValue() in a clickable/button element, prevent default selection behavior if needed, and invoke the existing filter function (the same handler or table.setFilter used by the device_model column) passing the "app_version" column id and the clicked value so clicking an app version applies/removes that filter. Ensure the header and fallback "—" behavior remain unchanged.server/src/api/sites/updateSiteConfig.ts (1)
82-85: Avoid theanyescape hatch onsite.type.
(site as any).typeremoves compile-time checks from the new app/web validation branch. Since this field now exists in the schema, it should be read through a typed path so mismatches fail fast during development instead of silently picking the wrong validator.Typed version
- const siteType = (site as any).type || "web"; + const siteType = site.type ?? "web";As per coding guidelines, "Use TypeScript with strict typing throughout both client and server".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/sites/updateSiteConfig.ts` around lines 82 - 85, The code uses an unsafe cast (site as any).type when deciding validation, which loses type-safety; update the Site typing to include the type property (or import/extend the correct Site interface) and read site.type directly (e.g., const siteType = site.type ?? "web") or use a narrow/type-guard if the shape can vary, then branch on siteType === "web" to choose the validator; ensure any place referencing site.type is updated to the typed property instead of casting to any.client/src/app/[site]/events/components/EventLog/EventRow.tsx (1)
18-18: Hoist the site lookup out of each row.Every
EventRownow mounts its ownuseGetSite()observer for the same site. React Query will dedupe the fetch, but a large event table still pays for N subscriptions and an extra rerender wave when that query resolves. FetchisApponce in the parent list and pass it down.Also applies to: 28-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx at line 18, EventRow currently calls useGetSite() on every row; move that lookup up to the parent event list component (the EventLog/EventList that renders the rows), call useGetSite() once there to compute isApp, then pass isApp into EventRow as a prop (add isApp to EventRow's props and remove the useGetSite import/usages inside EventRow). Update the parent render (where EventRow is mapped) to pass the new isApp prop and adjust types/interfaces accordingly, and remove the redundant lines (the useGetSite import and hooks at lines referenced) from EventRow.client/src/app/[site]/funnels/page.tsx (1)
5-6: Minor: Consider sorting internal imports alphabetically.Same as other files -
api/imports should precedelib/imports per coding guidelines.♻️ Suggested reorder
-import { getFunnelPageFilters } from "@/lib/filterGroups"; -import { useGetSite } from "../../../api/admin/hooks/useSites"; +import { useGetSite } from "../../../api/admin/hooks/useSites"; +import { getFunnelPageFilters } from "@/lib/filterGroups";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/funnels/page.tsx around lines 5 - 6, Reorder the internal imports so module groups follow the project convention: bring the api import(s) before lib imports and alphabetize internal imports; specifically swap and sort the two imports so useGetSite (from "../../../api/admin/hooks/useSites") appears before getFunnelPageFilters (from "@/lib/filterGroups") and ensure other internal imports in this file follow the same alphabetical ordering.client/src/app/[site]/users/page.tsx (1)
7-8: Minor: Consider sorting internal imports alphabetically.Per coding guidelines, internal imports should be sorted alphabetically within their group. The
api/path should come beforelib/.♻️ Suggested reorder
-import { getUserPageFilters } from "../../../lib/filterGroups"; -import { useGetSite } from "../../../api/admin/hooks/useSites"; +import { useGetSite } from "../../../api/admin/hooks/useSites"; +import { getUserPageFilters } from "../../../lib/filterGroups";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/page.tsx around lines 7 - 8, Reorder the internal import group so they are alphabetized: move the import for useGetSite (from the api path) above the import for getUserPageFilters (from the lib path); update the import block containing useGetSite and getUserPageFilters so internal imports are sorted alphabetically while preserving existing relative import styles and semicolons.client/src/app/[site]/pages/components/PageListItem.tsx (1)
33-34: Use consistentisAppderivation pattern across the codebase.The pattern here (
siteType !== "web") differs from the 21+ other locations that use the explicit positive check (siteMetadata?.type === "app"). Additionally, prefer??over||for the default value to properly handle only null/undefined.♻️ Suggested refactor for consistency
- const siteType = siteMetadata?.type || "web"; - const isApp = siteType !== "web"; + const siteType = siteMetadata?.type ?? "web"; + const isApp = siteType === "app";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/pages/components/PageListItem.tsx around lines 33 - 34, Replace the loose fallback and negative check with the codebase’s consistent pattern: use the nullish coalescing operator for the default (siteType = siteMetadata?.type ?? "web") and derive isApp via the explicit positive check (isApp = siteMetadata?.type === "app") rather than siteType !== "web"; update the variables siteType and isApp accordingly in PageListItem so they match other usages.client/src/components/SiteSettings/SDKIntegration.tsx (1)
4-12: Remove unuseddomainprop.The
domainparameter is declared but never used within the component. This is dead code that should be removed to avoid confusion.♻️ Proposed fix
export function SDKIntegration({ siteId, - siteType, - domain, }: { siteId: string; - siteType: "app"; - domain: string; }) {If
siteTypeanddomainare intended for future use, consider adding a TODO comment or removing them until needed to keep the API surface clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SDKIntegration.tsx` around lines 4 - 12, The SDKIntegration component declares an unused prop "domain"; remove "domain" from the props object and its TypeScript type annotation in the SDKIntegration signature (leave siteId and siteType as-is), and update any callers to stop passing a domain prop or adjust their calls accordingly; if domain is intended later, add a TODO comment in SDKIntegration instead of keeping the unused parameter.client/src/app/components/AddSite.tsx (1)
82-88: Consider logging or notifying on icon upload failure.The icon upload failure is silently caught, which could confuse users if the icon doesn't appear after site creation. Consider either:
- Logging the error for debugging
- Showing a non-blocking toast notification that the icon upload failed but the site was created successfully
♻️ Suggested improvement
if (iconBase64) { try { await uploadSiteIcon(site.siteId, iconBase64); - } catch { - // Non-critical — site was created, icon upload failed silently + } catch (iconError) { + // Non-critical — site was created, icon upload failed + console.warn("Icon upload failed:", iconError); + // Optionally: show a toast notification here } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/components/AddSite.tsx` around lines 82 - 88, The catch block swallowing uploadSiteIcon(site.siteId, iconBase64) failures hides errors and confuses users; update the catch to both log the error (e.g., console.error or the app's logger) and display a non-blocking user notification/toast indicating "Icon upload failed but site was created" so the success flow remains unchanged; modify the catch in AddSite.tsx where uploadSiteIcon is awaited and use the app's notification helper (or enqueueSnackbar/showToast) alongside logging the caught error.client/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsx (1)
163-164: Duplicate "Screen" label may cause user confusion.Line 113 conditionally shows
t("Screen")for path/screen name (app mode), and line 163 also usest("Screen")for screen dimensions. Consider using a more specific key liket("Screen Dimensions")ort("Display")for the dimensions row to differentiate.♻️ Suggested fix
<div className="flex items-center justify-between border-b border-neutral-50 dark:border-neutral-850 pb-1.5"> - <span className="text-neutral-500 dark:text-neutral-400">{t("Screen")}</span> + <span className="text-neutral-500 dark:text-neutral-400">{t("Screen Dimensions")}</span> <span>{event.screen_width && event.screen_height ? `${event.screen_width} × ${event.screen_height}` : "-"}</span> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx around lines 163 - 164, In EventDetailsSheet (component EventDetailsSheet.tsx) the same translation key t("Screen") is used twice — once for the path/screen name row and again for the dimensions row (using event.screen_width and event.screen_height) which is confusing; change the dimensions row to use a specific key such as t("Screen Dimensions") or t("Display") instead of t("Screen"), update the i18n locales to include the new key, and ensure any uses of event.screen_width/event.screen_height remain unchanged so the span still renders `${event.screen_width} × ${event.screen_height}` or "-" when absent.client/src/lib/filterGroups.ts (2)
48-50: Duplicate filter entry:page_title.
page_titleis already included inCOMMON_FILTERS(line 30), which is part ofgetBaseFilters. Adding it again ingetEventFilterscreates a duplicate (thoughallFiltersdeduplicates at export time, it's still redundant here).♻️ Suggested fix
export function getEventFilters(isApp: boolean): FilterParameter[] { - return [...getBaseFilters(isApp), "pathname", "page_title", "event_name", "entry_page", "exit_page"]; + return [...getBaseFilters(isApp), "pathname", "event_name", "entry_page", "exit_page"]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/lib/filterGroups.ts` around lines 48 - 50, getEventFilters currently appends "page_title" even though that field is already included via COMMON_FILTERS returned by getBaseFilters, causing a redundant duplicate; remove "page_title" from the array literal in getEventFilters so it only returns [...getBaseFilters(isApp), "pathname", "event_name", "entry_page", "exit_page"] and leave deduplication to allFilters/common logic.
100-111: Commented-out UTM filters in journey page.The commented UTM filters (lines 103-108) are presumably intentionally excluded. Consider adding a brief comment explaining why these are omitted, or remove them if they're not planned for future inclusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/lib/filterGroups.ts` around lines 100 - 111, The commented-out UTM filter entries (utm_source, utm_medium, utm_campaign, utm_term, utm_content) in the array returned alongside "hostname", "referrer", and ...base should either be removed or documented: update the return block in client/src/lib/filterGroups.ts to either delete those commented lines if they are not intended, or add a one-line comment explaining why UTM filters are intentionally excluded (e.g., deprecated, tracked elsewhere, or pending implementation) so future readers know the rationale; reference the specific identifiers utm_source, utm_medium, utm_campaign, utm_term, utm_content and the surrounding entries "hostname", "referrer", and ...base when making the change.client/src/components/SiteSettings/SiteConfiguration.tsx (2)
437-438: Log errors in catch block for debugging.The empty
catchblock discards error details, making debugging difficult when uploads fail.♻️ Suggested improvement
- } catch { + } catch (error) { + console.error("Icon upload failed:", error); toast.error(t("Failed to upload icon"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 437 - 438, The catch block in SiteConfiguration.tsx currently swallows errors during icon upload; change it to capture the exception (e.g., catch (err)) and log the error details before showing the user toast — include a contextual message like "Failed to upload icon" and the error object using console.error or your app logger (referencing the catch block around the icon upload, and the toast.error / t("Failed to upload icon") call) so failures are recorded for debugging.
399-417: Avoid direct DOM manipulation for visibility toggling.Using
nextElementSiblingto toggle visibility is fragile and non-idiomatic in React. If the DOM structure changes, this breaks silently.♻️ Suggested refactor using state
+ const [iconLoadError, setIconLoadError] = useState(false); // In the JSX: <img key={iconVersion} src={`${BACKEND_URL}/sites/${siteMetadata.siteId}/icon?v=${iconVersion}`} alt="App icon" - className="w-12 h-12 rounded-lg border border-border" - onError={(e) => { - (e.target as HTMLImageElement).style.display = "none"; - (e.target as HTMLImageElement).nextElementSibling?.classList.remove("hidden"); - }} - onLoad={(e) => { - (e.target as HTMLImageElement).style.display = ""; - (e.target as HTMLImageElement).nextElementSibling?.classList.add("hidden"); - }} + className={`w-12 h-12 rounded-lg border border-border ${iconLoadError ? "hidden" : ""}`} + onError={() => setIconLoadError(true)} + onLoad={() => setIconLoadError(false)} /> -<div className="hidden w-12 h-12 rounded-lg border border-dashed border-border bg-neutral-100 dark:bg-neutral-800 flex items-center justify-center"> +<div className={`w-12 h-12 rounded-lg border border-dashed border-border bg-neutral-100 dark:bg-neutral-800 flex items-center justify-center ${iconLoadError ? "" : "hidden"}`}> <Smartphone className="w-5 h-5 text-muted-foreground" /> </div>Also reset
iconLoadErrortofalsewheniconVersionchanges to ensure fresh load attempts show the image.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 399 - 417, The image visibility toggling currently manipulates the DOM via nextElementSibling in the img onError/onLoad handlers; replace this with a React state flag (e.g., iconLoadError) and conditional rendering or className application for the <img> and the fallback <div>, update the img handlers to setIconLoadError(true) on error and setIconLoadError(false) on load, and add a useEffect that resets setIconLoadError(false) whenever iconVersion changes so each new version tries to load the image afresh; locate the handlers on the <img> element and the sibling fallback <div> to implement the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/zh.json`:
- Around line 1017-1018: Replace the ASCII colon with a full-width Chinese colon
for the specified translation keys so punctuation matches neighboring entries:
update the values for keys "tH05Pi" and "ShHw0c" (and the other similar key
noted) in the zh.json translations to use "：" instead of ":" ensuring consistent
full-width punctuation in the details panel.
- Around line 182-183: The locale mixes two terms — change the value for key
"4vQ9fL" from using "错误跟踪" to use "错误追踪" so it matches the existing wording used
by key "BffFao" (i.e., update "错误跟踪捕获应用程序中的错误和异常。" to "错误追踪捕获应用程序中的错误和异常。").
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx:
- Around line 31-33: The component EventDetailsSheet receives a site prop but
calls useGetSite() with no arguments causing it to use store state; update the
call to pass the prop (useGetSite(site)) so the hook fetches metadata for the
provided site ID and then compute isApp from siteMetadata?.type === "app" as
before; locate the useGetSite invocation in EventDetailsSheet.tsx and replace
the no-arg call with useGetSite(site).
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Around line 28-29: The path column currently always renders an external anchor
built with `https://${event.hostname}${pagePath}` even when `isApp` (derived
from `useGetSite()` and `siteMetadata?.type === "app"`) is true; change the
render in the `EventRow` component so the anchor is only used when `!isApp` and
for app events render plain text (or an app-specific deep-link field) instead of
the external URL — locate the cells that use `event.hostname` and `pagePath`
(including the occurrences that build `https://${event.hostname}${pagePath}`)
and wrap the anchor rendering with a `!isApp` condition, falling back to
text/deep-link for app mode.
In `@client/src/app/`[site]/main/components/sections/Devices.tsx:
- Line 3: Remove the unused icon imports from the lucide-react import: drop
Monitor, Smartphone, and Tablet from the import list in the Devices component so
only the used symbols (Expand, TabletSmartphone, Tag) remain; update the import
statement that currently imports Expand, Monitor, Smartphone, Tablet,
TabletSmartphone, Tag to only import the actually used icons.
In `@client/src/app/components/AddSite.tsx`:
- Line 4: The file imports an unused symbol DateTime from luxon in AddSite.tsx;
remove the unused import (the `DateTime` identifier) from the import statement
so the module no longer imports luxon DateTime that isn't referenced in the
file.
In `@client/src/components/Favicon.tsx`:
- Around line 20-27: The app-icon URL needs a cache-busting query key so clients
don't display a stale 24-hour-cached image; update the src in the Favicon
component (the <img> rendering block that uses BACKEND_URL, siteId and domain)
to append a version query param like ?v=${iconVersion ?? siteUpdatedAt ??
Date.now()} (preferably use a stable value such as an iconVersion or
site.updatedAt property passed into Favicon; fall back to Date.now() only if no
stable value exists) so the URL changes after upload/delete and forces the
browser/cache to refresh.
In `@client/src/components/Sessions/SessionCard.tsx`:
- Line 7: Replace the experimental useExtracted import with the stable
useTranslations hook: change the import in SessionCard.tsx from useExtracted to
useTranslations, then update any calls of useExtracted(...) to
useTranslations(...) and adjust how translation keys are accessed (e.g., the
returned t function usage) so the component uses the client-side next-intl API
correctly.
In `@client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx`:
- Around line 45-46: The isApp boolean is computed from siteMetadata which is
undefined on first render causing a flash of web layout; in SessionInfoTab
(where useGetSite and const isApp = siteMetadata?.type === "app" are used)
either accept a site.type prop from the parent and use that synchronously or
check the query state (e.g., only compute/render app-vs-web branches when the
site query is settled/loaded) before rendering the layout-specific rows; update
all uses (the initial isApp, and other branches around the same component) to
guard on the query being settled or switch to the passed down site.type to
prevent the transient incorrect UI.
- Around line 183-190: The App Version display in SessionInfoTab.tsx currently
prefixes sessionDetails?.app_version with a hardcoded "v", which can produce
duplicate or awkward values; update the render in the SessionInfoTab component
to output sessionDetails.app_version verbatim (remove the leading "v"
concatenation) so the stored SDK string is shown exactly as-is, and ensure you
handle undefined/null the same way as other fields (i.e., only render when
sessionDetails?.app_version is truthy).
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Line 4: The import and hook call useExtracted from next-intl are incorrect;
replace the import with useTranslations from "next-intl" and update the
SiteConfiguration component to call useTranslations() instead of useExtracted(),
then adjust any subsequent usage of the returned value (e.g., replace
extract(...) or the previous API with the t(...) translation function) so all
translations in SiteConfiguration use the useTranslations hook correctly.
In `@client/src/lib/imageUtils.ts`:
- Around line 16-31: The current drawImage call (drawImage(img, 0, 0, size,
size)) forcibly stretches images to a square and distorts non-square uploads;
instead compute a centered square source crop from img.width/img.height (or
img.naturalWidth/img.naturalHeight), derive sx, sy, sWidth, sHeight to select
the largest centered square region, then call the 9-argument ctx.drawImage(img,
sx, sy, sWidth, sHeight, 0, 0, size, size) so the canvas (canvas, ctx, size)
preserves aspect ratio without distortion before calling canvas.toDataURL and
extracting base64.
In `@server/src/api/sites/siteIcon.ts`:
- Around line 52-56: The handler currently destructures icon from request.body
which will throw if request.body is null/undefined; update the siteIcon handler
to first guard request.body (e.g., if request.body == null return
reply.status(400).send({ error: "icon field is required" })) or assign a safe
body variable before destructuring, then validate the icon variable as before,
referencing the existing request.body and icon variables in this file
(siteIcon.ts).
In `@server/src/db/postgres/schema.ts`:
- Line 73: The sites.type column is currently defined as
text("type").default("web").notNull() which permits any string; change it to a
database-enforced enum/check so only "web" or "app" can be stored. Replace the
text(...) definition for the type column with a Postgres enum (e.g. create a
pgEnum like pgEnum("site_type", ["web","app"]) and use that enum for the column:
use the enum column constructor instead of text("type") and keep
.default("web").notNull(), or alternatively add a CHECK constraint restricting
values to ('web','app'); update the sites table column definition (the 'type'
column in the sites table) accordingly.
In `@server/src/utils.ts`:
- Around line 129-157: The parseSDKUserAgent function currently validates a
trailing deviceModel token but doesn't capture or return it; update the function
signature to include deviceModel in the returned object and modify both regexes
in parseSDKUserAgent to capture the deviceModel token (e.g., add an extra
capture group where the final "\S+\/\S+$" is validated), then return deviceModel
(trimmed) in both the match3 and match2 branches so callers receive device_model
metadata.
---
Outside diff comments:
In `@client/src/app/`[site]/main/components/sections/Pages.tsx:
- Line 5: The file imports and uses the wrong next-intl hook: replace the import
of useExtracted with useTranslations and update the hook call inside the Pages
component (where useExtracted is invoked around line 20) to call useTranslations
instead; ensure the variable name and any subsequent calls (e.g., t(...) or
similar) remain compatible with useTranslations so translations still resolve
correctly.
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Line 18: The import and hook usage currently reference the non-existent
useExtracted hook; replace the import of useExtracted with useTranslations from
next-intl and update any calls to useExtracted(...) to useTranslations(...)
(e.g., in the UsersTable component where the translation hook is invoked around
line 79) so the component uses the correct next-intl hook for translations.
---
Nitpick comments:
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx:
- Around line 163-164: In EventDetailsSheet (component EventDetailsSheet.tsx)
the same translation key t("Screen") is used twice — once for the path/screen
name row and again for the dimensions row (using event.screen_width and
event.screen_height) which is confusing; change the dimensions row to use a
specific key such as t("Screen Dimensions") or t("Display") instead of
t("Screen"), update the i18n locales to include the new key, and ensure any uses
of event.screen_width/event.screen_height remain unchanged so the span still
renders `${event.screen_width} × ${event.screen_height}` or "-" when absent.
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Line 18: EventRow currently calls useGetSite() on every row; move that lookup
up to the parent event list component (the EventLog/EventList that renders the
rows), call useGetSite() once there to compute isApp, then pass isApp into
EventRow as a prop (add isApp to EventRow's props and remove the useGetSite
import/usages inside EventRow). Update the parent render (where EventRow is
mapped) to pass the new isApp prop and adjust types/interfaces accordingly, and
remove the redundant lines (the useGetSite import and hooks at lines referenced)
from EventRow.
In `@client/src/app/`[site]/funnels/page.tsx:
- Around line 5-6: Reorder the internal imports so module groups follow the
project convention: bring the api import(s) before lib imports and alphabetize
internal imports; specifically swap and sort the two imports so useGetSite (from
"../../../api/admin/hooks/useSites") appears before getFunnelPageFilters (from
"@/lib/filterGroups") and ensure other internal imports in this file follow the
same alphabetical ordering.
In `@client/src/app/`[site]/pages/components/PageListItem.tsx:
- Around line 33-34: Replace the loose fallback and negative check with the
codebase’s consistent pattern: use the nullish coalescing operator for the
default (siteType = siteMetadata?.type ?? "web") and derive isApp via the
explicit positive check (isApp = siteMetadata?.type === "app") rather than
siteType !== "web"; update the variables siteType and isApp accordingly in
PageListItem so they match other usages.
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Around line 248-253: The app_version column is currently display-only; to add
click-to-filter parity with the device_model column, change the column
definition created by columnHelper.accessor("app_version") in UsersTable.tsx to
render the cell as an interactive element that calls the same filter action used
by device_model (e.g., the table's setFilter or toggle logic). Specifically,
mirror the device_model cell's behavior: wrap info.getValue() in a
clickable/button element, prevent default selection behavior if needed, and
invoke the existing filter function (the same handler or table.setFilter used by
the device_model column) passing the "app_version" column id and the clicked
value so clicking an app version applies/removes that filter. Ensure the header
and fallback "—" behavior remain unchanged.
In `@client/src/app/`[site]/users/page.tsx:
- Around line 7-8: Reorder the internal import group so they are alphabetized:
move the import for useGetSite (from the api path) above the import for
getUserPageFilters (from the lib path); update the import block containing
useGetSite and getUserPageFilters so internal imports are sorted alphabetically
while preserving existing relative import styles and semicolons.
In `@client/src/app/components/AddSite.tsx`:
- Around line 82-88: The catch block swallowing uploadSiteIcon(site.siteId,
iconBase64) failures hides errors and confuses users; update the catch to both
log the error (e.g., console.error or the app's logger) and display a
non-blocking user notification/toast indicating "Icon upload failed but site was
created" so the success flow remains unchanged; modify the catch in AddSite.tsx
where uploadSiteIcon is awaited and use the app's notification helper (or
enqueueSnackbar/showToast) alongside logging the caught error.
In `@client/src/components/SiteSettings/SDKIntegration.tsx`:
- Around line 4-12: The SDKIntegration component declares an unused prop
"domain"; remove "domain" from the props object and its TypeScript type
annotation in the SDKIntegration signature (leave siteId and siteType as-is),
and update any callers to stop passing a domain prop or adjust their calls
accordingly; if domain is intended later, add a TODO comment in SDKIntegration
instead of keeping the unused parameter.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Around line 437-438: The catch block in SiteConfiguration.tsx currently
swallows errors during icon upload; change it to capture the exception (e.g.,
catch (err)) and log the error details before showing the user toast — include a
contextual message like "Failed to upload icon" and the error object using
console.error or your app logger (referencing the catch block around the icon
upload, and the toast.error / t("Failed to upload icon") call) so failures are
recorded for debugging.
- Around line 399-417: The image visibility toggling currently manipulates the
DOM via nextElementSibling in the img onError/onLoad handlers; replace this with
a React state flag (e.g., iconLoadError) and conditional rendering or className
application for the <img> and the fallback <div>, update the img handlers to
setIconLoadError(true) on error and setIconLoadError(false) on load, and add a
useEffect that resets setIconLoadError(false) whenever iconVersion changes so
each new version tries to load the image afresh; locate the handlers on the
<img> element and the sibling fallback <div> to implement the change.
In `@client/src/lib/filterGroups.ts`:
- Around line 48-50: getEventFilters currently appends "page_title" even though
that field is already included via COMMON_FILTERS returned by getBaseFilters,
causing a redundant duplicate; remove "page_title" from the array literal in
getEventFilters so it only returns [...getBaseFilters(isApp), "pathname",
"event_name", "entry_page", "exit_page"] and leave deduplication to
allFilters/common logic.
- Around line 100-111: The commented-out UTM filter entries (utm_source,
utm_medium, utm_campaign, utm_term, utm_content) in the array returned alongside
"hostname", "referrer", and ...base should either be removed or documented:
update the return block in client/src/lib/filterGroups.ts to either delete those
commented lines if they are not intended, or add a one-line comment explaining
why UTM filters are intentionally excluded (e.g., deprecated, tracked elsewhere,
or pending implementation) so future readers know the rationale; reference the
specific identifiers utm_source, utm_medium, utm_campaign, utm_term, utm_content
and the surrounding entries "hostname", "referrer", and ...base when making the
change.
In `@server/src/api/sites/updateSiteConfig.ts`:
- Around line 82-85: The code uses an unsafe cast (site as any).type when
deciding validation, which loses type-safety; update the Site typing to include
the type property (or import/extend the correct Site interface) and read
site.type directly (e.g., const siteType = site.type ?? "web") or use a
narrow/type-guard if the shape can vary, then branch on siteType === "web" to
choose the validator; ensure any place referencing site.type is updated to the
typed property instead of casting to any.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd2c99b8-59ef-4ee9-95f3-b92cf149921c
📒 Files selected for processing (77)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/sites.tsclient/src/api/analytics/endpoints/events.tsclient/src/api/analytics/endpoints/sessions.tsclient/src/api/analytics/endpoints/users.tsclient/src/app/[site]/components/Header/NoData.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/components/Sidebar/SiteSelector.tsxclient/src/app/[site]/components/SubHeader/Export/exportCsv.tsclient/src/app/[site]/components/SubHeader/Filters/FilterComponent.tsxclient/src/app/[site]/components/SubHeader/Filters/Filters.tsxclient/src/app/[site]/components/SubHeader/Filters/const.tsxclient/src/app/[site]/components/SubHeader/Filters/utils.tsclient/src/app/[site]/errors/components/EnableErrorTracking.tsxclient/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsxclient/src/app/[site]/events/components/EventLog/EventRow.tsxclient/src/app/[site]/events/components/EventLog/eventLogUtils.tsxclient/src/app/[site]/events/components/EventTypesChart.tsxclient/src/app/[site]/events/page.tsxclient/src/app/[site]/funnels/page.tsxclient/src/app/[site]/goals/page.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/main/components/MainSection/MainSection.tsxclient/src/app/[site]/main/components/MainSection/Overview.tsxclient/src/app/[site]/main/components/sections/Devices.tsxclient/src/app/[site]/main/components/sections/Events.tsxclient/src/app/[site]/main/components/sections/Pages.tsxclient/src/app/[site]/main/components/sections/Weekdays.tsxclient/src/app/[site]/main/page.tsxclient/src/app/[site]/pages/components/PageListItem.tsxclient/src/app/[site]/sessions/page.tsxclient/src/app/[site]/user/[userId]/components/UserSidebar.tsxclient/src/app/[site]/users/components/UsersTable.tsxclient/src/app/[site]/users/page.tsxclient/src/app/components/AddSite.tsxclient/src/app/page.tsxclient/src/components/EventTypeFilter.tsxclient/src/components/Favicon.tsxclient/src/components/Sessions/SessionCard.tsxclient/src/components/Sessions/SessionDetails/SessionInfoTab.tsxclient/src/components/SiteCard.tsxclient/src/components/SiteSettings/SDKIntegration.tsxclient/src/components/SiteSettings/SiteConfiguration.tsxclient/src/components/SiteSettings/SiteSettings.tsxclient/src/lib/filterGroups.tsclient/src/lib/imageUtils.tsclient/src/lib/utils.tsserver/src/api/analytics/events/getEvents.tsserver/src/api/analytics/sessions/getSession.tsserver/src/api/analytics/sessions/getSessions.tsserver/src/api/analytics/users/getUserInfo.tsserver/src/api/analytics/users/getUsers.tsserver/src/api/analytics/utils/query-validation.tsserver/src/api/sites/addSite.tsserver/src/api/sites/getSite.tsserver/src/api/sites/index.tsserver/src/api/sites/siteIcon.tsserver/src/api/sites/updateSiteConfig.tsserver/src/db/clickhouse/clickhouse.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/services/tracker/pageviewQueue.tsserver/src/services/tracker/trackEvent.tsserver/src/services/tracker/utils.tsserver/src/utils.tsshared/src/filters.ts
🚧 Files skipped from review as they are similar to previous changes (36)
- client/src/app/[site]/components/SubHeader/Filters/const.tsx
- client/src/app/[site]/user/[userId]/components/UserSidebar.tsx
- client/src/app/[site]/components/SubHeader/Filters/Filters.tsx
- client/src/api/analytics/endpoints/users.ts
- server/src/api/analytics/utils/query-validation.ts
- client/src/app/[site]/main/components/MainSection/MainSection.tsx
- client/src/components/SiteSettings/SiteSettings.tsx
- server/src/index.ts
- client/src/app/[site]/main/components/sections/Events.tsx
- server/src/api/analytics/sessions/getSession.ts
- client/src/app/[site]/events/page.tsx
- client/src/app/[site]/main/components/MainSection/Overview.tsx
- client/src/app/[site]/components/SubHeader/Filters/utils.ts
- client/src/app/[site]/events/components/EventTypesChart.tsx
- client/src/app/[site]/main/components/sections/Weekdays.tsx
- client/src/app/[site]/components/SubHeader/Export/exportCsv.ts
- client/src/app/[site]/components/Sidebar/SiteSelector.tsx
- server/src/services/tracker/pageviewQueue.ts
- server/src/api/analytics/users/getUsers.ts
- client/src/app/[site]/goals/page.tsx
- server/src/api/analytics/sessions/getSessions.ts
- client/src/api/admin/endpoints/index.ts
- client/src/app/[site]/components/SubHeader/Filters/FilterComponent.tsx
- server/src/api/sites/addSite.ts
- client/src/lib/utils.ts
- shared/src/filters.ts
- client/src/components/SiteCard.tsx
- client/src/app/[site]/sessions/page.tsx
- server/src/services/tracker/trackEvent.ts
- server/src/services/tracker/utils.ts
- client/src/app/page.tsx
- server/src/api/analytics/users/getUserInfo.ts
- client/src/app/[site]/errors/components/EnableErrorTracking.tsx
- client/src/components/EventTypeFilter.tsx
- server/src/api/sites/index.ts
- client/messages/es.json
Comment on lines
31
to
+33
| const t = useExtracted(); | ||
| const { data: siteMetadata } = useGetSite(); | ||
| const isApp = siteMetadata?.type === "app"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check useGetSite hook implementation to verify it uses route params
ast-grep --pattern $'export function useGetSite($$$) {
$$$
}'Repository: rybbit-io/rybbit
Length of output: 1292
🏁 Script executed:
# Check EventDetailsSheet component and its usage
cat -n client/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsx | head -50Repository: rybbit-io/rybbit
Length of output: 2600
🏁 Script executed:
# Find where the store's site value is being set
rg "site:" -A 3 -B 3 | grep -E "(useStore|store.*site)" | head -20Repository: rybbit-io/rybbit
Length of output: 661
🏁 Script executed:
# Check if there's a layout file in the [site] directory that initializes store
fd -t f "layout" client/src/app/[site]/ --exec cat -n {} \;Repository: rybbit-io/rybbit
Length of output: 2697
Pass the site prop to useGetSite()
The component receives site as a prop but calls useGetSite() without arguments, causing it to fall back on store state. Use useGetSite(site) instead to explicitly pass the site ID from the prop.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx
around lines 31 - 33, The component EventDetailsSheet receives a site prop but
calls useGetSite() with no arguments causing it to use store state; update the
call to pass the prop (useGetSite(site)) so the hook fetches metadata for the
provided site ID and then compute isApp from siteMetadata?.type === "app" as
before; locate the useGetSite invocation in EventDetailsSheet.tsx and replace
the no-arg call with useGetSite(site).
| "use client"; | ||
|
|
||
| import { Expand, Monitor, Smartphone, Tablet } from "lucide-react"; | ||
| import { Expand, Monitor, Smartphone, Tablet, TabletSmartphone, Tag } from "lucide-react"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Monitor, Smartphone, Tablet are used in this file
rg -n "Monitor|Smartphone|Tablet" "client/src/app/\[site\]/main/components/sections/Devices.tsx" | grep -v "TabletSmartphone" | grep -v "import"Repository: rybbit-io/rybbit
Length of output: 160
🏁 Script executed:
fd "Devices.tsx" client/srcRepository: rybbit-io/rybbit
Length of output: 118
🏁 Script executed:
cat -n "client/src/app/[site]/main/components/sections/Devices.tsx" | head -100Repository: rybbit-io/rybbit
Length of output: 4840
🏁 Script executed:
cat -n "client/src/app/[site]/main/components/sections/Devices.tsx" | tail -n +100Repository: rybbit-io/rybbit
Length of output: 2776
Remove unused imports from lucide-react.
Monitor, Smartphone, and Tablet are imported on line 3 but are never used in this file. Remove them to keep imports clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/`[site]/main/components/sections/Devices.tsx at line 3, Remove
the unused icon imports from the lucide-react import: drop Monitor, Smartphone,
and Tablet from the import list in the Devices component so only the used
symbols (Expand, TabletSmartphone, Tag) remain; update the import statement that
currently imports Expand, Monitor, Smartphone, Tablet, TabletSmartphone, Tag to
only import the actually used icons.
|
|
||
| import { AlertTriangle } from "lucide-react"; | ||
| import { AlertTriangle, Smartphone, Trash2, Upload } from "lucide-react"; | ||
| import { useExtracted } from "next-intl"; |
Contributor
There was a problem hiding this comment.
Incorrect next-intl hook name.
useExtracted is not a valid hook from next-intl. The correct hook is useTranslations().
🐛 Proposed fix
-import { useExtracted } from "next-intl";
+import { useTranslations } from "next-intl";And update line 56:
- const t = useExtracted();
+ const t = useTranslations();As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx` at line 4, The
import and hook call useExtracted from next-intl are incorrect; replace the
import with useTranslations from "next-intl" and update the SiteConfiguration
component to call useTranslations() instead of useExtracted(), then adjust any
subsequent usage of the returned value (e.g., replace extract(...) or the
previous API with the t(...) translation function) so all translations in
SiteConfiguration use the useTranslations hook correctly.
Comment on lines
+16
to
+31
| const canvas = document.createElement("canvas"); | ||
| canvas.width = size; | ||
| canvas.height = size; | ||
|
|
||
| const ctx = canvas.getContext("2d"); | ||
| if (!ctx) { | ||
| reject(new Error("Could not get canvas context")); | ||
| return; | ||
| } | ||
|
|
||
| ctx.drawImage(img, 0, 0, size, size); | ||
|
|
||
| const dataUrl = canvas.toDataURL("image/png"); | ||
| // Strip the "data:image/png;base64," prefix | ||
| const base64 = dataUrl.split(",")[1]; | ||
| resolve(base64); |
Contributor
There was a problem hiding this comment.
Preserve aspect ratio when generating the icon.
drawImage(img, 0, 0, size, size) squashes any non-square upload. Since this flow accepts arbitrary image formats, rectangular images will end up as visibly distorted icons.
Possible fix
const ctx = canvas.getContext("2d");
if (!ctx) {
reject(new Error("Could not get canvas context"));
return;
}
- ctx.drawImage(img, 0, 0, size, size);
+ const scale = Math.min(size / img.width, size / img.height);
+ const width = img.width * scale;
+ const height = img.height * scale;
+ const x = (size - width) / 2;
+ const y = (size - height) / 2;
+
+ ctx.clearRect(0, 0, size, size);
+ ctx.drawImage(img, x, y, width, height);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/lib/imageUtils.ts` around lines 16 - 31, The current drawImage
call (drawImage(img, 0, 0, size, size)) forcibly stretches images to a square
and distorts non-square uploads; instead compute a centered square source crop
from img.width/img.height (or img.naturalWidth/img.naturalHeight), derive sx,
sy, sWidth, sHeight to select the largest centered square region, then call the
9-argument ctx.drawImage(img, sx, sy, sWidth, sHeight, 0, 0, size, size) so the
canvas (canvas, ctx, size) preserves aspect ratio without distortion before
calling canvas.toDataURL and extracting base64.
| siteId: serial("site_id").primaryKey().notNull(), | ||
| name: text("name").notNull(), | ||
| domain: text("domain").notNull(), | ||
| type: text("type").default("web").notNull(), // 'web', 'app' |
Contributor
There was a problem hiding this comment.
Constrain sites.type at the database layer.
This field now drives validation and UI branching as if it were only 'web' | 'app', but text().default("web") still allows any other value to be persisted. One bad row will send update flows down the wrong validator and break client assumptions.
Suggested schema hardening
+export const siteTypeEnum = pgEnum("site_type", ["web", "app"]);
+
export const sites = pgTable("sites", {
id: text("id").$defaultFn(() => sql`encode(gen_random_bytes(6), 'hex')`),
// deprecated - keeping as primary key for backwards compatibility
siteId: serial("site_id").primaryKey().notNull(),
name: text("name").notNull(),
domain: text("domain").notNull(),
- type: text("type").default("web").notNull(), // 'web', 'app'
+ type: siteTypeEnum("type").default("web").notNull(),📝 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
| type: text("type").default("web").notNull(), // 'web', 'app' | |
| export const siteTypeEnum = pgEnum("site_type", ["web", "app"]); | |
| export const sites = pgTable("sites", { | |
| id: text("id").$defaultFn(() => sql`encode(gen_random_bytes(6), 'hex')`), | |
| // deprecated - keeping as primary key for backwards compatibility | |
| siteId: serial("site_id").primaryKey().notNull(), | |
| name: text("name").notNull(), | |
| domain: text("domain").notNull(), | |
| type: siteTypeEnum("type").default("web").notNull(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/db/postgres/schema.ts` at line 73, The sites.type column is
currently defined as text("type").default("web").notNull() which permits any
string; change it to a database-enforced enum/check so only "web" or "app" can
be stored. Replace the text(...) definition for the type column with a Postgres
enum (e.g. create a pgEnum like pgEnum("site_type", ["web","app"]) and use that
enum for the column: use the enum column constructor instead of text("type") and
keep .default("web").notNull(), or alternatively add a CHECK constraint
restricting values to ('web','app'); update the sites table column definition
(the 'type' column in the sites table) accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/app/[site]/events/components/EventLog/EventRow.tsx (1)
3-3:⚠️ Potential issue | 🔴 CriticalReplace
useExtractedwithuseTranslationsfromnext-intl.The
useExtractedhook does not exist in thenext-intllibrary. The import and usage on lines 3 and 27 will cause a runtime error. UseuseTranslationsinstead, which is the standard i18n hook for client components.Fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx at line 3, Replace the invalid next-intl hook: change the import of useExtracted to useTranslations and update all call sites in the EventRow component (e.g., where useExtracted(...) is invoked) to call useTranslations(...) instead; ensure the variable name and usage (e.g., const t = useTranslations('Namespace') and then t('key')) match next-intl's API so translations resolve without runtime errors in EventRow.tsx.
♻️ Duplicate comments (1)
client/messages/zh.json (1)
1027-1027:⚠️ Potential issue | 🟡 MinorReplace the ASCII colon in
入口屏幕:.Line 1027 still uses
:while the surrounding zh-CN detail labels use：, so this panel will render with inconsistent punctuation.✏️ Suggested fix
- "RvxWt3": "入口屏幕:", + "RvxWt3": "入口屏幕：",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/zh.json` at line 1027, The JSON entry with key "RvxWt3" uses an ASCII colon ("入口屏幕:") which is inconsistent with other zh‑CN labels; update the value for "RvxWt3" to use the full‑width Chinese colon ("入口屏幕：") so punctuation matches the surrounding translations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/zh.json`:
- Around line 638-643: The zh-CN copy uses page-specific wording; replace the
site-specific strings to be site-neutral by updating the message keys: change
the value for "B6CB37" from "网页" to "站点" (or another neutral term like "网站/站点"),
change "CLGask" ("应用图标") to a neutral label such as "站点图标" (or "站点/应用图标"), and
ensure the tooltip "o9wmBB" reads neutrally (e.g., "可选。将调整为 128×128 的 PNG。");
make the same replacements for the same keys at the other occurrence noted
(around 1156).
---
Outside diff comments:
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Line 3: Replace the invalid next-intl hook: change the import of useExtracted
to useTranslations and update all call sites in the EventRow component (e.g.,
where useExtracted(...) is invoked) to call useTranslations(...) instead; ensure
the variable name and usage (e.g., const t = useTranslations('Namespace') and
then t('key')) match next-intl's API so translations resolve without runtime
errors in EventRow.tsx.
---
Duplicate comments:
In `@client/messages/zh.json`:
- Line 1027: The JSON entry with key "RvxWt3" uses an ASCII colon ("入口屏幕:")
which is inconsistent with other zh‑CN labels; update the value for "RvxWt3" to
use the full‑width Chinese colon ("入口屏幕：") so punctuation matches the
surrounding translations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad9921dd-88c6-4d60-9d89-c461c2714daa
📒 Files selected for processing (3)
client/messages/zh.jsonclient/src/app/[site]/events/components/EventLog/EventRow.tsxclient/src/components/Favicon.tsx
Comment on lines
+638
to
+643
| "Mmuj1R": "平台", | ||
| "B6CB37": "网页", | ||
| "2rUVsU": "应用", | ||
| "uHP8ou": "包名", | ||
| "CLGask": "应用图标", | ||
| "o9wmBB": "可选。将调整为 128x128 PNG。", |
Contributor
There was a problem hiding this comment.
Keep the new platform copy site-neutral.
"网页" reads as a single page, not the existing site-level concept, and "网站图标" is misleading now that the same icon flow also applies to app sites. Using neutral site wording will make the zh-CN UI read correctly in both variants.
✏️ Suggested wording
- "B6CB37": "网页",
+ "B6CB37": "网站",
...
- "jxsBGE": "在侧边栏、站点选择器和站点卡片中显示为网站图标。支持任意图片格式 — 自动调整为 128×128 PNG。最大 50 KB。",
+ "jxsBGE": "在侧边栏、站点选择器和站点卡片中显示为站点图标。支持任意图片格式 — 自动调整为 128×128 PNG。最大 50 KB。",Also applies to: 1156-1156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/zh.json` around lines 638 - 643, The zh-CN copy uses
page-specific wording; replace the site-specific strings to be site-neutral by
updating the message keys: change the value for "B6CB37" from "网页" to "站点" (or
another neutral term like "网站/站点"), change "CLGask" ("应用图标") to a neutral label
such as "站点图标" (or "站点/应用图标"), and ensure the tooltip "o9wmBB" reads neutrally
(e.g., "可选。将调整为 128×128 的 PNG。"); make the same replacements for the same keys
at the other occurrence noted (around 1156).
lukyrys
force-pushed
the
feat/mobile-platform-support
branch
from
651421d to
5339470
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx (1)
3-3:⚠️ Potential issue | 🔴 CriticalInvalid import:
useExtractedis not exported bynext-intl.
useExtracteddoes not exist innext-intl. This will fail at build time or runtime. The standard translation hook isuseTranslations.Proposed fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";And update line 44:
- const t = useExtracted(); + const t = useTranslations();As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx` at line 3, The import of useExtracted from next-intl is invalid; replace it with the correct hook useTranslations and update any usage in the SessionInfoTab component (referenced symbol: SessionInfoTab and the import line that currently imports useExtracted) — change the import to useTranslations and update the call/site where useExtracted is used (around the usage on line ~44) to call useTranslations(...) with the appropriate namespace/key and adjust any returned value handling to match useTranslations' API.client/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsx (1)
3-3:⚠️ Potential issue | 🔴 CriticalInvalid import:
useExtractedis not exported bynext-intl.The
next-intllibrary does not exportuseExtracted, which will cause a runtime error. Per the coding guidelines, useuseTranslations()for i18n in client code.Proposed fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";And update the usage:
- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx at line 3, The import useExtracted from next-intl is invalid; replace it with useTranslations and update EventDetailsSheet to call useTranslations() wherever useExtracted was used (e.g., in the EventDetailsSheet component and any helper functions inside that file), adjusting any API differences (pass namespace/key strings to useTranslations and replace .extract or similar usages with the returned t(...) calls) so the component uses the next-intl client hook correctly.
♻️ Duplicate comments (1)
client/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsx (1)
31-33:⚠️ Potential issue | 🟠 MajorPass the
siteprop touseGetSite().The component receives
siteas a prop (line 30) but callsuseGetSite()without arguments, causing it to rely on store state instead of the explicit prop. This was flagged in a previous review.Proposed fix
- const { data: siteMetadata } = useGetSite(); + const { data: siteMetadata } = useGetSite(site);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx around lines 31 - 33, EventDetailsSheet is calling useGetSite() without the component's site prop, so it uses store state instead of the explicit prop; update the call to pass the received site prop into useGetSite (e.g., call useGetSite(site)) and keep the existing destructuring const { data: siteMetadata } = useGetSite(site) so the hook resolves based on the provided site prop rather than global state.
🧹 Nitpick comments (6)
client/src/app/[site]/users/components/UsersTable.tsx (2)
80-81: Potential column layout shift during loading.When
siteMetadatais initiallyundefined,isAppwill befalse, causing web-specific columns to render. Once the site data loads, if it's an app site, columns will suddenly change. This could cause a jarring layout shift for users.Consider either:
- Deferring table render until site metadata is loaded
- Using a loading skeleton for the entire table when
siteMetadatais undefinedOption 1: Defer column definition until metadata is ready
+ const { data: siteMetadata, isLoading: isSiteLoading } = useGetSite(); - const { data: siteMetadata } = useGetSite(); const isApp = siteMetadata?.type === "app";Then in the return statement, handle
isSiteLoadingstate appropriately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/components/UsersTable.tsx around lines 80 - 81, siteMetadata from useGetSite() is undefined during initial render so isApp becomes false and web-specific columns render then shift when data arrives; update UsersTable to avoid this by checking the loading state before defining/rendering columns: use the hook's loading flag (e.g., isSiteLoading or similar from useGetSite) and either (a) defer building the column definition that references isApp until siteMetadata is defined, or (b) return a table skeleton placeholder while siteMetadata is undefined; ensure references to useGetSite, siteMetadata, and isApp are replaced with the loading-aware logic so columns remain stable once rendered.
248-253: Consider adding filter click handler toapp_versionfor consistency.The
device_modelcolumn (line 178) has a filter click handler, butapp_versiondoes not. According to the PR summary,app_versionis exposed as a filter dimension. For consistency and user expectation, consider making it filterable.Proposed fix to add filter interactivity
columnHelper.accessor("app_version", { header: t("App Version"), - cell: info => <div className="whitespace-nowrap">{info.getValue() || "—"}</div>, + cell: info => { + const version = info.getValue(); + return ( + <div + className="flex items-center gap-2 whitespace-nowrap cursor-pointer hover:opacity-70" + onClick={e => handleFilterClick(e, "app_version", version)} + > + {version || "—"} + </div> + ); + }, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/users/components/UsersTable.tsx around lines 248 - 253, The app_version column currently renders a plain cell; add the same filter-click interactivity used by the device_model column so users can click an app version to filter. Update the columnHelper.accessor("app_version", { header: t("App Version"), cell: info => ... }) to wrap the value in the same clickable div/handler used by the device_model accessor (i.e., attach the onClick that calls the existing filter toggle function such as onFilterClick/toggleFilter with the column id "app_version" and the cell value), preserving the whitespace-nowrap fallback ("—") behavior and accessibility attributes used in device_model.client/src/components/SiteSettings/SiteConfiguration.tsx (1)
124-139: Normalize app package input consistently with AddSite flow.App updates currently keep raw
newDomain; trimming here avoids accidental whitespace and keeps behavior aligned across create/edit flows.Suggested fix
- if (siteType === "web") { - if (!isValidDomain(newDomain)) { + const candidate = siteType === "web" ? newDomain : newDomain.trim(); + + if (siteType === "web") { + if (!isValidDomain(candidate)) { toast.error(t("Invalid domain format")); return; } } else { - if (!isValidPackageName(newDomain)) { + if (!isValidPackageName(candidate)) { toast.error(t("Invalid package name format")); return; } } try { setIsChangingDomain(true); - const normalizedDomain = siteType === "web" ? normalizeDomain(newDomain) : newDomain; + const normalizedDomain = siteType === "web" ? normalizeDomain(candidate) : candidate;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 124 - 139, The app-package branch currently validates and saves raw newDomain, which can include stray whitespace; trim newDomain before validating and before building normalizedDomain so behavior matches the AddSite flow. Concretely, compute a trimmed value (e.g., const trimmed = newDomain.trim()) and use trimmed when calling isValidDomain/isValidPackageName and when creating normalizedDomain (siteType === "web" ? normalizeDomain(trimmed) : trimmed) before passing to updateSiteConfig; keep the setIsChangingDomain/try block the same.client/src/components/SiteSettings/SiteSettings.tsx (1)
43-44: Avoidas "app"assertion—narrowsiteTypeexplicitly.The cast at Line 79 weakens type safety and can hide unexpected future values from
siteMetadata.type.♻️ Suggested refactor
- const siteType = siteMetadata.type || "web"; + const siteType: "web" | "app" = siteMetadata.type === "app" ? "app" : "web"; @@ - siteType={siteType as "app"} + siteType="app"As per coding guidelines, "Use TypeScript with strict typing throughout both client and server".
Also applies to: 79-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteSettings.tsx` around lines 43 - 44, The current cast using as "app" weakens type safety for siteType; instead tighten the type by deriving siteType from siteMetadata.type with an explicit union guard (e.g., ensure siteType is either "web" or "app") inside the SiteSettings component so unexpected strings are mapped to a safe default; update the siteType assignment (the variable named siteType that reads siteMetadata.type) to perform an explicit conditional/type-check rather than a type assertion.server/src/utils.ts (1)
137-138: Update regex patterns to handle OS names with spaces.The patterns at both locations use
\w+to capture the OS name, which only matches word characters [a-zA-Z0-9_]. The codebase defines valid OS names with spaces in thedesktopOSandmobileOSsets (e.g., "Windows Phone", "Firefox OS", "elementary OS"), but the regex cannot match them. If the SDK sends such values, parsing fails. Change(\w+)to([^;]+)or similar to allow spaces in OS names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/utils.ts` around lines 137 - 138, The regex used to parse the user agent (the call assigning match3 via userAgent.match in server/src/utils.ts) uses (\w+) to capture the OS name which fails for multi-word OS names; update that pattern to use a broader character class such as ([^;]+) (or similar) instead of (\w+) so OS names with spaces like "Windows Phone" or "elementary OS" are matched; make the same change for the other analogous userAgent regex in this file so both matches accept spaces in OS names.client/src/lib/filterGroups.ts (1)
48-50: Duplicatepage_titlein getEventFilters.
page_titleis already included inCOMMON_FILTERS(line 30), which is part ofgetBaseFilters. Adding it again here creates a duplicate in the returned array. While the static exports useallFiltersfor deduplication, direct callers ofgetEventFilters(isApp)will receive duplicates.Remove the duplicate
export function getEventFilters(isApp: boolean): FilterParameter[] { - return [...getBaseFilters(isApp), "pathname", "page_title", "event_name", "entry_page", "exit_page"]; + return [...getBaseFilters(isApp), "pathname", "event_name", "entry_page", "exit_page"]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/lib/filterGroups.ts` around lines 48 - 50, getEventFilters returns [...getBaseFilters(isApp), "pathname", "page_title", "event_name", "entry_page", "exit_page"] which duplicates "page_title" because COMMON_FILTERS (used by getBaseFilters) already includes it; remove the duplicate by deleting "page_title" from the explicit list in getEventFilters so the returned array relies on getBaseFilters/isApp for that filter and avoids duplicates (refer to getEventFilters, getBaseFilters, and COMMON_FILTERS when locating the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/cs.json`:
- Line 1188: The JSON file contains a duplicate translation key "ja2Zok" which
shadows the earlier definition; remove the later occurrence (the entry with
"ja2Zok": "Upgradovat na Pro") so only the original "ja2Zok" key remains, and
run a quick JSON lint to ensure no trailing commas or structural issues after
deletion; if the later string was the intended value, instead update the
original "ja2Zok" entry to this text rather than adding a duplicate.
In `@client/messages/it.json`:
- Around line 512-513: The label for key "fsUN9Y" is ambiguous with page views;
update the string value for fsUN9Y to explicitly reference screens (e.g., change
"Min. visualizzazioni" to "Min. visualizzazioni schermo" or "Visualizzazioni
schermo minime") so it cannot be confused with the "Visualizzazioni di pagina
minime" entry under key "Z2Mi3c"; keep the same key and only modify the
translated text.
In `@client/messages/zh.json`:
- Line 1029: The JSON entry with key "RvxWt3" uses an ASCII colon after the
label ("入口屏幕:") causing punctuation inconsistency; update the value for "RvxWt3"
to use a Chinese full-width colon ("入口屏幕：") so it matches neighboring detail
labels and preserves consistent punctuation in client/messages/zh.json.
In `@client/src/app/`[site]/components/Sidebar/Sidebar.tsx:
- Line 39: isApp is computed as false while useGetSite is still loading, causing
web-only tabs to flash before site type resolves; change the isApp computation
to be tri-state (true/false/undefined) by using the site value directly (e.g.,
const isApp = site ? site.type === "app" : undefined) and update all render
conditions in Sidebar.tsx to check explicitly for isApp === true or isApp ===
false so that when isApp is undefined the component avoids rendering app-only or
web-only labels/tabs until the site data has resolved (useGetSite).
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Around line 269-271: The code in UsersTable.tsx uses semantic keys
t("Screenviews") and t("Pageviews") which don't exist in the translation map;
update the call inside the columnHelper.accessor("pageviews", ...) header (the
SortHeader usage) to use the correct hashed translation keys (e.g., replace
t("Screenviews") with t("Pbtcej") and t("Pageviews") with t("0UAnct")) or
alternatively regenerate/transpile the translation resources so semantic keys
exist, ensuring the change is applied where isApp is checked and inside the
SortHeader component invocation.
In `@client/src/app/components/AddSite.tsx`:
- Line 220: The placeholder and image alt text in AddSite.tsx are hardcoded; use
next-intl's useTranslations hook to pull localized strings instead. Import and
call useTranslations (e.g., const t = useTranslations('AddSite')) at the top of
the AddSite component, replace the inline placeholder (currently using siteType
? "example.com..." : "com.example.app") with t(...) keys (e.g.,
t('placeholder.web') / t('placeholder.app') or a single key that accepts
siteType), and replace the hardcoded img alt text with t('imageAlt') (or a key
per type). Add corresponding keys to messages/*.json (en.json, de.json, etc.)
under the AddSite namespace and use those translation keys in the component.
- Around line 82-87: The current catch block in the AddSite component swallows
errors from uploadSiteIcon(site.siteId, iconBase64); change the catch to capture
the error (catch (err)) and surface it instead of failing silently: log the
error (e.g., console.error) and/or set a component state or call your existing
notification/toast helper to inform the user that the icon upload failed (e.g.,
setIconUploadError or showToast("Icon upload failed", err.message)); keep site
creation successful but ensure uploadSiteIcon's failure is visible to the user
so they can retry.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Line 403: In SiteConfiguration.tsx, replace hardcoded UI strings (e.g.,
alt="App icon" and any raw placeholder values found around the image/inputs)
with localized strings using next-intl's useTranslations() hook inside the
SiteConfiguration component (call useTranslations('SiteSettings') or similar),
then swap those attributes to use t('appIconAlt') and t('somePlaceholderKey')
respectively; finally add corresponding keys (appIconAlt, somePlaceholderKey,
etc.) to your messages/*.json files (en.json, de.json) so translations are
available.
---
Outside diff comments:
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx:
- Line 3: The import useExtracted from next-intl is invalid; replace it with
useTranslations and update EventDetailsSheet to call useTranslations() wherever
useExtracted was used (e.g., in the EventDetailsSheet component and any helper
functions inside that file), adjusting any API differences (pass namespace/key
strings to useTranslations and replace .extract or similar usages with the
returned t(...) calls) so the component uses the next-intl client hook
correctly.
In `@client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx`:
- Line 3: The import of useExtracted from next-intl is invalid; replace it with
the correct hook useTranslations and update any usage in the SessionInfoTab
component (referenced symbol: SessionInfoTab and the import line that currently
imports useExtracted) — change the import to useTranslations and update the
call/site where useExtracted is used (around the usage on line ~44) to call
useTranslations(...) with the appropriate namespace/key and adjust any returned
value handling to match useTranslations' API.
---
Duplicate comments:
In `@client/src/app/`[site]/events/components/EventLog/EventDetailsSheet.tsx:
- Around line 31-33: EventDetailsSheet is calling useGetSite() without the
component's site prop, so it uses store state instead of the explicit prop;
update the call to pass the received site prop into useGetSite (e.g., call
useGetSite(site)) and keep the existing destructuring const { data: siteMetadata
} = useGetSite(site) so the hook resolves based on the provided site prop rather
than global state.
---
Nitpick comments:
In `@client/src/app/`[site]/users/components/UsersTable.tsx:
- Around line 80-81: siteMetadata from useGetSite() is undefined during initial
render so isApp becomes false and web-specific columns render then shift when
data arrives; update UsersTable to avoid this by checking the loading state
before defining/rendering columns: use the hook's loading flag (e.g.,
isSiteLoading or similar from useGetSite) and either (a) defer building the
column definition that references isApp until siteMetadata is defined, or (b)
return a table skeleton placeholder while siteMetadata is undefined; ensure
references to useGetSite, siteMetadata, and isApp are replaced with the
loading-aware logic so columns remain stable once rendered.
- Around line 248-253: The app_version column currently renders a plain cell;
add the same filter-click interactivity used by the device_model column so users
can click an app version to filter. Update the
columnHelper.accessor("app_version", { header: t("App Version"), cell: info =>
... }) to wrap the value in the same clickable div/handler used by the
device_model accessor (i.e., attach the onClick that calls the existing filter
toggle function such as onFilterClick/toggleFilter with the column id
"app_version" and the cell value), preserving the whitespace-nowrap fallback
("—") behavior and accessibility attributes used in device_model.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Around line 124-139: The app-package branch currently validates and saves raw
newDomain, which can include stray whitespace; trim newDomain before validating
and before building normalizedDomain so behavior matches the AddSite flow.
Concretely, compute a trimmed value (e.g., const trimmed = newDomain.trim()) and
use trimmed when calling isValidDomain/isValidPackageName and when creating
normalizedDomain (siteType === "web" ? normalizeDomain(trimmed) : trimmed)
before passing to updateSiteConfig; keep the setIsChangingDomain/try block the
same.
In `@client/src/components/SiteSettings/SiteSettings.tsx`:
- Around line 43-44: The current cast using as "app" weakens type safety for
siteType; instead tighten the type by deriving siteType from siteMetadata.type
with an explicit union guard (e.g., ensure siteType is either "web" or "app")
inside the SiteSettings component so unexpected strings are mapped to a safe
default; update the siteType assignment (the variable named siteType that reads
siteMetadata.type) to perform an explicit conditional/type-check rather than a
type assertion.
In `@client/src/lib/filterGroups.ts`:
- Around line 48-50: getEventFilters returns [...getBaseFilters(isApp),
"pathname", "page_title", "event_name", "entry_page", "exit_page"] which
duplicates "page_title" because COMMON_FILTERS (used by getBaseFilters) already
includes it; remove the duplicate by deleting "page_title" from the explicit
list in getEventFilters so the returned array relies on getBaseFilters/isApp for
that filter and avoids duplicates (refer to getEventFilters, getBaseFilters, and
COMMON_FILTERS when locating the change).
In `@server/src/utils.ts`:
- Around line 137-138: The regex used to parse the user agent (the call
assigning match3 via userAgent.match in server/src/utils.ts) uses (\w+) to
capture the OS name which fails for multi-word OS names; update that pattern to
use a broader character class such as ([^;]+) (or similar) instead of (\w+) so
OS names with spaces like "Windows Phone" or "elementary OS" are matched; make
the same change for the other analogous userAgent regex in this file so both
matches accept spaces in OS names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cfe24da-601f-4647-93e4-aa22f7ac2206
📒 Files selected for processing (77)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/sites.tsclient/src/api/analytics/endpoints/events.tsclient/src/api/analytics/endpoints/sessions.tsclient/src/api/analytics/endpoints/users.tsclient/src/app/[site]/components/Header/NoData.tsxclient/src/app/[site]/components/Sidebar/Sidebar.tsxclient/src/app/[site]/components/Sidebar/SiteSelector.tsxclient/src/app/[site]/components/SubHeader/Export/exportCsv.tsclient/src/app/[site]/components/SubHeader/Filters/FilterComponent.tsxclient/src/app/[site]/components/SubHeader/Filters/Filters.tsxclient/src/app/[site]/components/SubHeader/Filters/const.tsxclient/src/app/[site]/components/SubHeader/Filters/utils.tsclient/src/app/[site]/errors/components/EnableErrorTracking.tsxclient/src/app/[site]/events/components/EventLog/EventDetailsSheet.tsxclient/src/app/[site]/events/components/EventLog/EventRow.tsxclient/src/app/[site]/events/components/EventLog/eventLogUtils.tsxclient/src/app/[site]/events/components/EventTypesChart.tsxclient/src/app/[site]/events/page.tsxclient/src/app/[site]/funnels/page.tsxclient/src/app/[site]/goals/page.tsxclient/src/app/[site]/journeys/page.tsxclient/src/app/[site]/main/components/MainSection/MainSection.tsxclient/src/app/[site]/main/components/MainSection/Overview.tsxclient/src/app/[site]/main/components/sections/Devices.tsxclient/src/app/[site]/main/components/sections/Events.tsxclient/src/app/[site]/main/components/sections/Pages.tsxclient/src/app/[site]/main/components/sections/Weekdays.tsxclient/src/app/[site]/main/page.tsxclient/src/app/[site]/pages/components/PageListItem.tsxclient/src/app/[site]/sessions/page.tsxclient/src/app/[site]/user/[userId]/components/UserSidebar.tsxclient/src/app/[site]/users/components/UsersTable.tsxclient/src/app/[site]/users/page.tsxclient/src/app/components/AddSite.tsxclient/src/app/page.tsxclient/src/components/EventTypeFilter.tsxclient/src/components/Favicon.tsxclient/src/components/Sessions/SessionCard.tsxclient/src/components/Sessions/SessionDetails/SessionInfoTab.tsxclient/src/components/SiteCard.tsxclient/src/components/SiteSettings/SDKIntegration.tsxclient/src/components/SiteSettings/SiteConfiguration.tsxclient/src/components/SiteSettings/SiteSettings.tsxclient/src/lib/filterGroups.tsclient/src/lib/imageUtils.tsclient/src/lib/utils.tsserver/src/api/analytics/events/getEvents.tsserver/src/api/analytics/sessions/getSession.tsserver/src/api/analytics/sessions/getSessions.tsserver/src/api/analytics/users/getUserInfo.tsserver/src/api/analytics/users/getUsers.tsserver/src/api/analytics/utils/query-validation.tsserver/src/api/sites/addSite.tsserver/src/api/sites/getSite.tsserver/src/api/sites/index.tsserver/src/api/sites/siteIcon.tsserver/src/api/sites/updateSiteConfig.tsserver/src/db/clickhouse/clickhouse.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/services/tracker/pageviewQueue.tsserver/src/services/tracker/trackEvent.tsserver/src/services/tracker/utils.tsserver/src/utils.tsshared/src/filters.ts
🚧 Files skipped from review as they are similar to previous changes (38)
- client/src/api/analytics/endpoints/sessions.ts
- client/src/app/[site]/components/SubHeader/Filters/utils.ts
- client/src/app/[site]/components/SubHeader/Export/exportCsv.ts
- server/src/api/sites/index.ts
- client/src/lib/imageUtils.ts
- server/src/index.ts
- client/src/app/[site]/components/Header/NoData.tsx
- client/src/api/analytics/endpoints/users.ts
- client/src/app/[site]/main/components/sections/Events.tsx
- client/src/app/[site]/errors/components/EnableErrorTracking.tsx
- client/src/app/[site]/journeys/page.tsx
- server/src/api/sites/getSite.ts
- client/src/app/[site]/users/page.tsx
- client/src/app/[site]/components/Sidebar/SiteSelector.tsx
- client/src/api/admin/endpoints/sites.ts
- server/src/db/clickhouse/clickhouse.ts
- client/src/app/[site]/events/page.tsx
- server/src/api/analytics/sessions/getSession.ts
- server/src/api/analytics/users/getUserInfo.ts
- client/src/app/[site]/components/SubHeader/Filters/const.tsx
- server/src/api/sites/siteIcon.ts
- client/src/app/[site]/user/[userId]/components/UserSidebar.tsx
- client/src/app/[site]/goals/page.tsx
- server/src/api/analytics/users/getUsers.ts
- server/src/db/postgres/schema.ts
- client/src/components/EventTypeFilter.tsx
- client/src/components/Sessions/SessionCard.tsx
- client/src/lib/utils.ts
- client/src/components/SiteSettings/SDKIntegration.tsx
- client/src/api/admin/endpoints/index.ts
- client/src/app/[site]/main/components/MainSection/MainSection.tsx
- client/src/app/[site]/events/components/EventLog/eventLogUtils.tsx
- client/src/app/[site]/main/page.tsx
- server/src/api/sites/updateSiteConfig.ts
- client/src/app/page.tsx
- client/src/app/[site]/main/components/MainSection/Overview.tsx
- client/messages/en.json
- client/src/app/[site]/components/SubHeader/Filters/FilterComponent.tsx
client/messages/cs.json
Outdated
| "S7U3Nx": "Blížíš se k měsíčnímu limitu událostí. Zvaž upgrade na placený plán pro vyšší limity.", | ||
| "wbsq7O": "Využití", | ||
| "rd5F5r": "", | ||
| "ja2Zok": "Upgradovat na Pro", |
Contributor
There was a problem hiding this comment.
Remove the duplicate translation key ja2Zok.
This key is already defined earlier; re-declaring it later shadows the first definition and is easy to break during future edits.
Suggested fix
- "ja2Zok": "Upgradovat na Pro",📝 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
| "ja2Zok": "Upgradovat na Pro", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/cs.json` at line 1188, The JSON file contains a duplicate
translation key "ja2Zok" which shadows the earlier definition; remove the later
occurrence (the entry with "ja2Zok": "Upgradovat na Pro") so only the original
"ja2Zok" key remains, and run a quick JSON lint to ensure no trailing commas or
structural issues after deletion; if the later string was the intended value,
instead update the original "ja2Zok" entry to this text rather than adding a
duplicate.
| const embed = useEmbedablePage(); | ||
|
|
||
| const { data: site } = useGetSite(Number(pathname.split("/")[1])); | ||
| const isApp = site?.type === "app"; |
Contributor
There was a problem hiding this comment.
Prevent app/web nav flicker before site type resolves.
At Line 39, isApp is false until useGetSite returns, so app sites can briefly render web-only labels/tabs (e.g., “Web Analytics”, “Performance”, “Replay”) and then switch.
💡 Suggested patch
- const { data: site } = useGetSite(Number(pathname.split("/")[1]));
+ const { data: site, isLoading: isSiteLoading } = useGetSite(Number(pathname.split("/")[1]));
const isApp = site?.type === "app";
@@
- <SidebarComponents.SectionHeader>{isApp ? t("Analytics") : t("Web Analytics")}</SidebarComponents.SectionHeader>
+ <SidebarComponents.SectionHeader>
+ {isSiteLoading ? t("Analytics") : isApp ? t("Analytics") : t("Web Analytics")}
+ </SidebarComponents.SectionHeader>
@@
- {IS_CLOUD && !isApp && (
+ {IS_CLOUD && !isSiteLoading && !isApp && (
@@
- {!isApp && (
+ {!isSiteLoading && !isApp && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/`[site]/components/Sidebar/Sidebar.tsx at line 39, isApp is
computed as false while useGetSite is still loading, causing web-only tabs to
flash before site type resolves; change the isApp computation to be tri-state
(true/false/undefined) by using the site value directly (e.g., const isApp =
site ? site.type === "app" : undefined) and update all render conditions in
Sidebar.tsx to check explicitly for isApp === true or isApp === false so that
when isApp is undefined the component avoids rendering app-only or web-only
labels/tabs until the site data has resolved (useGetSite).
Comment on lines
269
to
271
| columnHelper.accessor("pageviews", { | ||
| header: ({ column }) => <SortHeader column={column}>{t("Pageviews")}</SortHeader>, | ||
| header: ({ column }) => <SortHeader column={column}>{isApp ? t("Screenviews") : t("Pageviews")}</SortHeader>, | ||
| cell: info => <div className="whitespace-nowrap">{info.getValue().toLocaleString()}</div>, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify translation keys exist for Screenviews and Pageviews
echo "=== Checking for Screenviews translation key ==="
rg -l "Screenviews" --glob '*.json' | head -5
echo "=== Checking for Pageviews translation key ==="
rg -l "Pageviews" --glob '*.json' | head -5
echo "=== Sample content from en.json ==="
fd -t f 'en.json' --exec sh -c 'cat "$1" | jq "keys | map(select(test(\"view\"; \"i\")))"' _ {}Repository: rybbit-io/rybbit
Length of output: 245
🏁 Script executed:
#!/bin/bash
# Get the actual structure of en.json to see how translation keys are organized
echo "=== Size and structure of en.json ==="
wc -l client/messages/en.json
echo "=== Searching for Screenviews and Pageviews in context ==="
rg "Screenviews|Pageviews" client/messages/en.json -A 1 -B 1
echo "=== List all translation files ==="
fd -t f '\.(json)$' client/messages/
echo "=== Check first 50 lines of en.json to understand structure ==="
head -50 client/messages/en.jsonRepository: rybbit-io/rybbit
Length of output: 2709
🏁 Script executed:
#!/bin/bash
# Examine the UsersTable.tsx file to see how t() is used and imported
echo "=== First 100 lines of UsersTable.tsx ==="
head -100 client/src/app/\[site\]/users/components/UsersTable.tsx
echo ""
echo "=== Imports and t() function initialization ==="
head -30 client/src/app/\[site\]/users/components/UsersTable.tsx | grep -E "^import|^export|useTranslations|const t"
echo ""
echo "=== Lines around 269-271 with more context (250-290) ==="
sed -n '250,290p' client/src/app/\[site\]/users/components/UsersTable.tsxRepository: rybbit-io/rybbit
Length of output: 6865
🏁 Script executed:
#!/bin/bash
# Check if semantic string keys exist as keys in en.json (not just values)
echo "=== Check for 'Screenviews' as a key in en.json ==="
jq 'keys[] | select(. == "Screenviews")' client/messages/en.json
echo "=== Check for 'Pageviews' as a key in en.json ==="
jq 'keys[] | select(. == "Pageviews")' client/messages/en.json
echo "=== Check for 'Device' as a key in en.json ==="
jq 'keys[] | select(. == "Device")' client/messages/en.json
echo "=== Check for 'Events' as a key in en.json ==="
jq 'keys[] | select(. == "Events")' client/messages/en.json
echo "=== Search for all keys containing lowercase 'screen' or 'page' ==="
jq 'keys[] | select(test("screen|page"; "i"))' client/messages/en.jsonRepository: rybbit-io/rybbit
Length of output: 323
🏁 Script executed:
#!/bin/bash
# Look for translation extraction or build configuration
echo "=== Check for translation extraction config files ==="
fd -t f 'next-intl|translation|i18n' -H
echo "=== Check package.json for build/extract scripts ==="
jq '.scripts | keys[]' package.json 2>/dev/null | grep -i "extract\|i18n\|intl"
echo "=== Look for messages.config file ==="
fd -t f 'messages' -e config -e ts -e js | head -10
echo "=== Check if there are other JSON translation files with semantic keys ==="
fd -t f '\.json$' client/ | xargs grep -l '"Screenviews"\|"Pageviews"' 2>/dev/null | head -10Repository: rybbit-io/rybbit
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Search for the exact line with Screenviews and Pageviews as keys
echo "=== Lines containing Screenviews and Pageviews with context ==="
grep -n '"Screenviews"\|"Pageviews"' client/messages/en.json -B 1 -A 1
echo ""
echo "=== Check the i18n configuration ==="
cat docs/src/lib/i18n.ts | head -50Repository: rybbit-io/rybbit
Length of output: 607
Fix missing translation keys — "Screenviews" and "Pageviews" are not defined.
The translation files use hashed keys (e.g., "Pbtcej": "Screenviews", "0UAnct": "Pageviews"), but the code calls t("Screenviews") and t("Pageviews") with semantic string keys that do not exist in the translation files. This will cause runtime translation failures. Either update the code to use the correct hashed keys or regenerate the translation files with semantic keys that match the code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/`[site]/users/components/UsersTable.tsx around lines 269 -
271, The code in UsersTable.tsx uses semantic keys t("Screenviews") and
t("Pageviews") which don't exist in the translation map; update the call inside
the columnHelper.accessor("pageviews", ...) header (the SortHeader usage) to use
the correct hashed translation keys (e.g., replace t("Screenviews") with
t("Pbtcej") and t("Pageviews") with t("0UAnct")) or alternatively
regenerate/transpile the translation resources so semantic keys exist, ensuring
the change is applied where isApp is checked and inside the SortHeader component
invocation.
Comment on lines
+82
to
+87
| if (iconBase64) { | ||
| try { | ||
| await uploadSiteIcon(site.siteId, iconBase64); | ||
| } catch { | ||
| // Non-critical — site was created, icon upload failed silently | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t silently swallow icon upload failures.
If site creation succeeds but icon upload fails, users get no signal and assume the icon was saved.
Suggested fix
if (iconBase64) {
try {
await uploadSiteIcon(site.siteId, iconBase64);
- } catch {
- // Non-critical — site was created, icon upload failed silently
+ } catch (iconError) {
+ console.warn("Site created but icon upload failed", iconError);
+ // optionally surface a non-blocking toast here
}
}📝 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
| if (iconBase64) { | |
| try { | |
| await uploadSiteIcon(site.siteId, iconBase64); | |
| } catch { | |
| // Non-critical — site was created, icon upload failed silently | |
| } | |
| if (iconBase64) { | |
| try { | |
| await uploadSiteIcon(site.siteId, iconBase64); | |
| } catch (iconError) { | |
| console.warn("Site created but icon upload failed", iconError); | |
| // optionally surface a non-blocking toast here | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/app/components/AddSite.tsx` around lines 82 - 87, The current
catch block in the AddSite component swallows errors from
uploadSiteIcon(site.siteId, iconBase64); change the catch to capture the error
(catch (err)) and surface it instead of failing silently: log the error (e.g.,
console.error) and/or set a component state or call your existing
notification/toast helper to inform the user that the icon upload failed (e.g.,
setIconUploadError or showToast("Icon upload failed", err.message)); keep site
creation successful but ensure uploadSiteIcon's failure is visible to the user
so they can retry.
| <img | ||
| key={iconVersion} | ||
| src={`${BACKEND_URL}/sites/${siteMetadata.siteId}/icon?v=${iconVersion}`} | ||
| alt="App icon" |
Contributor
There was a problem hiding this comment.
Localize newly introduced UI strings in settings.
alt="App icon" and the raw placeholder values are hardcoded, which breaks locale consistency.
As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
Also applies to: 487-487
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx` at line 403, In
SiteConfiguration.tsx, replace hardcoded UI strings (e.g., alt="App icon" and
any raw placeholder values found around the image/inputs) with localized strings
using next-intl's useTranslations() hook inside the SiteConfiguration component
(call useTranslations('SiteSettings') or similar), then swap those attributes to
use t('appIconAlt') and t('somePlaceholderKey') respectively; finally add
corresponding keys (appIconAlt, somePlaceholderKey, etc.) to your
messages/*.json files (en.json, de.json) so translations are available.
5339470 to
3c27892
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
client/messages/cs.json (2)
1180-1183:⚠️ Potential issue | 🟡 MinorEmpty translation strings for invoice/billing section.
These keys have empty values but should be translated. Based on the Portuguese translations, these are billing-related labels.
🔧 Suggested translations
- "6dqYnF": "", - "P7PLVj": "", - "/0TOL5": "", - "FgydNe": "", + "6dqYnF": "Faktury", + "P7PLVj": "Datum", + "/0TOL5": "Částka", + "FgydNe": "Zobrazit",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/cs.json` around lines 1180 - 1183, The four empty translation entries "6dqYnF", "P7PLVj", "/0TOL5", and "FgydNe" in client/messages/cs.json are missing Czech invoice/billing labels; update each key with the correct Czech translations (mirroring the meaning from the Portuguese set) so billing UI shows localized text—locate the entries by the exact keys "6dqYnF", "P7PLVj", "/0TOL5", and "FgydNe" and replace the empty string values with the appropriate Czech phrases for those billing/invoice labels.
594-595:⚠️ Potential issue | 🟡 MinorEmpty translation strings for On/Off toggles.
These keys have empty values but should be translated. In Portuguese they are "Ligado"/"Desligado" (On/Off).
🔧 Suggested translations
- "Zh+5A6": "", - "OvzONl": "", + "Zh+5A6": "Zapnuto", + "OvzONl": "Vypnuto",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/cs.json` around lines 594 - 595, The JSON translation file has empty values for keys "Zh+5A6" and "OvzONl" (these appear to be the On/Off toggle strings); update client/messages/cs.json to provide the Portuguese translations by setting "Zh+5A6" to "Ligado" and "OvzONl" to "Desligado" so the toggles display correctly.client/src/app/[site]/components/Header/NoData.tsx (1)
37-37:⚠️ Potential issue | 🔴 CriticalReplace
useExtractedwithuseTranslationsfromnext-intl.
useExtractedis not a valid export fromnext-intland will cause a runtime import error. UseuseTranslations()as per the coding guidelines.Fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";And at line 73:
- const t = useExtracted(); + const t = useTranslations();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Header/NoData.tsx at line 37, The file's import and usage of useExtracted in the NoData component is invalid; replace the import "useExtracted" with "useTranslations" from "next-intl" and update any call sites (e.g., the hook invocation around line 73 inside the NoData component) to use useTranslations(...) accordingly (assign to a translator variable and use that instead of the non-existent useExtracted). Ensure the hook is imported as useTranslations and that all subsequent calls/reference names match the new hook.client/src/app/[site]/events/components/EventLog/EventRow.tsx (1)
3-3:⚠️ Potential issue | 🔴 CriticalReplace
useExtractedwith standarduseTranslationshook from next-intl.
useExtractedis not exported from thenext-intlpackage. The standard hook for client-side translations in this location isuseTranslations(). This will cause a runtime import error.Proposed fix
-import { useExtracted } from "next-intl"; +import { useTranslations } from "next-intl";export function EventRow({ event, site, onClick }: EventRowProps) { - const t = useExtracted(); + const t = useTranslations();Per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n" for
client/src/**/*.{ts,tsx}files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx at line 3, The import of useExtracted from next-intl in EventRow.tsx is invalid; replace the import and hook usage with next-intl's standard useTranslations hook: change the import statement to import useTranslations and update any call sites (e.g., where useExtracted() is invoked inside the EventRow component) to use useTranslations() instead so translations resolve at runtime; ensure any variables/keys passed into the hook remain the same (for example, replace const t = useExtracted() with const t = useTranslations('...') as used elsewhere).
♻️ Duplicate comments (7)
client/messages/zh.json (1)
641-645:⚠️ Potential issue | 🟡 Minor图标文案仍建议保持“站点”中性表述，并统一尺寸符号。
Line 644 的“应用图标”与 Line 1158 的“网站图标”在 Web/App 双平台下都偏单侧；此外 Line 645 仍为
128x128（ASCIIx），与文件内128×128不一致。✏️ 建议修改
- "CLGask": "应用图标", + "CLGask": "站点图标", - "o9wmBB": "可选。将调整为 128x128 PNG。", + "o9wmBB": "可选。将调整为 128×128 PNG。", - "jxsBGE": "在侧边栏、站点选择器和站点卡片中显示为网站图标。支持任意图片格式 — 自动调整为 128×128 PNG。最大 50 KB。", + "jxsBGE": "在侧边栏、站点选择器和站点卡片中显示为站点图标。支持任意图片格式 — 自动调整为 128×128 PNG。最大 50 KB。",Also applies to: 1158-1158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/zh.json` around lines 641 - 645, Update the localized strings to use a neutral "站点" term and a consistent multiplication sign: change the value for key "CLGask" from "应用图标" to "站点图标" (or "网站图标" if project standard uses 网站) and update the value for key "o9wmBB" from "可选。将调整为 128x128 PNG。" to "可选。将调整为 128×128 PNG。"; also make the same change for the corresponding "网站图标" entry referenced at the other location (the duplicate around line 1158) so both keys use the same neutral label and the unicode "×" in the size.client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx (1)
45-46:⚠️ Potential issue | 🟡 MinorPrevent transient web-layout rendering on app sites before site metadata resolves.
On initial render,
siteMetadatais unresolved, soisAppisfalse(Line 46). That briefly shows web-only rows (Device,Browser,Channel,Referrer) for app sites before flipping.Possible local mitigation
- const { data: siteMetadata } = useGetSite(); + const { data: siteMetadata, isLoading: isSiteLoading } = useGetSite(); const isApp = siteMetadata?.type === "app";- {!isApp && ( + {!isSiteLoading && !isApp && (- {isApp ? ( + {isSiteLoading ? null : isApp ? (- {!isApp && ( + {!isSiteLoading && !isApp && (Also applies to: 159-171, 194-213, 262-285
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx` around lines 45 - 46, The transient flip happens because isApp is computed as false while siteMetadata is unresolved; change the derived flag to be tri-state and only treat the page as web when siteMetadata is loaded: replace const isApp = siteMetadata?.type === "app" with something like const isApp = siteMetadata ? siteMetadata.type === "app" : undefined (or null) and update the conditional rendering for the web-only rows (the blocks showing Device, Browser, Channel, Referrer) to render them only when isApp === false (i.e., siteMetadata is loaded and type !== "app"); apply the same pattern wherever isApp is used (all spots referencing useGetSite/siteMetadata and isApp).client/src/components/Favicon.tsx (1)
11-27:⚠️ Potential issue | 🟠 MajorCache busting is still optional, so stale app icons can still persist.
At Line 26, the query key only appears when
iconVersionis provided. Current usages inclient/src/app/[site]/components/Sidebar/SiteSelector.tsx(Line 107, Line 161) andclient/src/components/SiteCard.tsx(Line 108) omit this prop, so icon URLs often remain static after upload/delete.Suggested fix
export function Favicon({ domain, className, siteType, siteId, iconVersion, + siteUpdatedAt, }: { domain: string; className?: string; siteType?: "web" | "app"; siteId?: number; iconVersion?: number; + siteUpdatedAt?: string; }) { @@ - if (siteId && !imageError) { + if (siteId !== undefined && !imageError) { + const iconCacheKey = iconVersion ?? siteUpdatedAt; return ( <img - src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion ? `?v=${iconVersion}` : ""}`} + src={`${BACKEND_URL}/sites/${siteId}/icon${iconCacheKey ? `?v=${encodeURIComponent(String(iconCacheKey))}` : ""}`} className={cn("rounded", className ?? "w-4 h-4")} alt={`Icon for ${domain}`} onError={() => setImageError(true)} /> );Then pass
site.updatedAt(or a dedicatediconVersion) from callers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Favicon.tsx` around lines 11 - 27, The Favicon component currently appends the cache-busting query only when the optional prop iconVersion is provided, so uploaded/deleted icons stay stale; update the Favicon usage and/or signature to always pass a stable cache-bust value (e.g., pass site.updatedAt or a dedicated iconVersion from callers) and ensure the img src built in Favicon (using BACKEND_URL and sites/{siteId}/icon) always includes `?v=` with that value (or a timestamp) even when zero/empty, then update callers (SiteSelector and SiteCard) to forward site.updatedAt (or computed iconVersion) into the Favicon component so the URL changes when the icon is updated.client/src/app/components/AddSite.tsx (2)
183-187:⚠️ Potential issue | 🟡 MinorFinish the localized app copy path.
After switching to App, Line 187 still describes a "website", and the placeholder / preview
alttext are still hardcoded English. The new branch should use localized app-specific strings end-to-end.As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
Also applies to: 220-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/components/AddSite.tsx` around lines 183 - 187, The displayed copy still uses hardcoded English and web-specific copy when siteType === "app"; update AddSite to use next-intl's useTranslations hook (e.g., const t = useTranslations('AddSite')) and replace the DialogTitle/DialogDescription strings and any placeholder/alt text in the preview components with localized keys (e.g., t('title.web'), t('title.app'), t('description.web'), t('description.app'), t('previewAlt.app'), etc.), and apply the same changes for the preview/placeholder code referenced around the preview block (the JSX that renders the preview/alt text and inputs between the DialogTitle/DialogDescription area and lines ~220-228) so all app-specific text uses t(...) instead of hardcoded English.
82-88:⚠️ Potential issue | 🟡 MinorOnly upload icons for app sites, and make failures visible.
If someone selects an icon in App mode, switches back to Web, and submits, the stale
iconBase64still goes through this path. The log-onlycatchalso hides upload failures even though the site was created successfully.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/components/AddSite.tsx` around lines 82 - 88, Only upload the icon when the site is actually an app and surface upload failures instead of silently logging them: before calling uploadSiteIcon(iconBase64, site.siteId) guard on the current app-mode flag (e.g., check site.type === 'app' or the local mode state that indicates "App" mode) so stale iconBase64 from Web mode is ignored, and in the catch for uploadSiteIcon replace console.warn with propagating or storing the error (rethrow, return an error from the submit handler, or set a visible error state / toast) so the UI shows the upload failure to the user. Ensure you reference iconBase64, uploadSiteIcon, and site.siteId when making these changes.client/src/lib/imageUtils.ts (1)
16-31:⚠️ Potential issue | 🟡 MinorAvoid stretching rectangular uploads into the icon canvas.
ctx.drawImage(img, 0, 0, size, size)squashes any non-square source, so wide/tall uploads end up visibly distorted.Possible fix
- ctx.drawImage(img, 0, 0, size, size); + const sourceSize = Math.min(img.width, img.height); + const sx = (img.width - sourceSize) / 2; + const sy = (img.height - sourceSize) / 2; + + ctx.drawImage(img, sx, sy, sourceSize, sourceSize, 0, 0, size, size);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/lib/imageUtils.ts` around lines 16 - 31, The current drawImage call squashes non-square images; update the image resizing in the routine that creates canvas/ctx/img/size so it preserves aspect ratio by center-cropping to a square before scaling: read img.width and img.height, compute a square source region (sx, sy, sSize) that crops the longer dimension to the shorter (centered), then call ctx.drawImage(img, sx, sy, sSize, sSize, 0, 0, size, size) instead of ctx.drawImage(img, 0, 0, size, size); keep the existing canvas.toDataURL and base64 extraction logic.client/src/components/SiteSettings/SiteConfiguration.tsx (1)
118-145:⚠️ Potential issue | 🟡 MinorUse localized package-name copy throughout the app branch.
The validation logic now distinguishes package names, but Line 120, Line 140, and Line 145 still talk about domains, and the input placeholder is hardcoded English. App sites should see package-name wording here too.
As per coding guidelines: "Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)".
Also applies to: 476-487
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 118 - 145, The text still uses "domain" and hardcoded English in the package-name branch and input placeholder; update handleDomainChange and the related input placeholder to use next-intl translations via useTranslations() and switch wording based on siteType so package-name copy appears when siteType !== "web". Specifically, replace hardcoded strings and toast messages around newDomain validation (isValidPackageName branch), success/failure messages (toast.success/toast.error), and the input placeholder to call t("siteSettings.packageNamePlaceholder") / t("siteSettings.domainPlaceholder") as appropriate; ensure normalizeDomain is only applied for siteType === "web" and keep updateSiteConfig(siteMetadata.siteId, { domain: normalizedDomain || newDomain }) behavior while using localized message keys. Also apply the same changes to the other occurrence handling the same logic (the block around the other occurrence referenced).
🧹 Nitpick comments (11)
client/messages/zh.json (1)
1140-1142: 建议统一“追踪/跟踪”用词。Line 1140-1142 使用“跟踪”，而同区域多数开关文案使用“追踪”。建议统一成一个术语，减少 UI 语感跳变。
✏️ 可选改法
- "wc+2Lv": "跟踪用户点击外部链接的情况", - "mFg4Ag": "出站跟踪已启用", - "sbjdWu": "出站跟踪已禁用", + "wc+2Lv": "追踪用户点击外部链接的情况", + "mFg4Ag": "出站追踪已启用", + "sbjdWu": "出站追踪已禁用",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/zh.json` around lines 1140 - 1142, The keys "wc+2Lv", "mFg4Ag", and "sbjdWu" use the term “跟踪” which is inconsistent with the surrounding UI copy that uses “追踪”; update the values for these keys to use the unified term “追踪” (e.g., change "跟踪用户点击外部链接的情况" → use "追踪...") so the translation is consistent across the file, and run a quick grep for these keys to ensure no other instances remain.client/messages/es.json (3)
69-69: Use plural form for analytics label to match existing UI wording.This label is singular while nearby navigation terminology is pluralized.
Suggested tweak
- "GZJpDf": "Analítica", + "GZJpDf": "Analíticas",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/es.json` at line 69, The Spanish translation key "GZJpDf" currently uses the singular label "Analítica"; update the value for "GZJpDf" in client/messages/es.json to the plural form "Analíticas" so it matches the surrounding UI terminology and pluralized navigation labels.
633-633: Align package-name validation messages for consistent guidance.One message includes the expected format example and the other does not.
Suggested tweak
- "ynio5d": "Formato de nombre de paquete no válido", + "ynio5d": "Formato de nombre de paquete no válido. Debe ser como com.example.app",Also applies to: 1098-1098
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/es.json` at line 633, Update the package-name validation messages so both Spanish entries consistently include the expected format example; change the message with key "39ieyq" and the other package-name key referenced around the second occurrence so they both read something like "Formato de nombre de paquete no válido. Debe ser como com.example.app" (ensure exact phrasing matches project style), keeping the same message ID keys ("39ieyq" and the other package-name key) so callers are unaffected.
182-182: Standardize terminology (“rastreo” and “aplicación”) across similar settings.These strings currently mix style with nearby entries, which can feel inconsistent in settings UI copy.
Suggested tweak
- "4vQ9fL": "El seguimiento de errores captura errores y excepciones de tu aplicación.", + "4vQ9fL": "El rastreo de errores captura errores y excepciones de tu aplicación.", - "c1bAPv": "Captura errores y excepciones de tu app", + "c1bAPv": "Captura errores y excepciones de tu aplicación",Also applies to: 1138-1138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/es.json` at line 182, Update the Spanish copy for key "4vQ9fL" to use the standardized terminology—replace "El seguimiento de errores captura errores y excepciones de tu aplicación." with a phrasing that uses "rastreo" and consistent ordering, e.g. "El rastreo de errores captura las excepciones y errores de tu aplicación."; apply the same wording to the other identical entry mentioned in the review so both occurrences use "rastreo" and the same phrasing.client/messages/fr.json (2)
634-634: Same unicode encoding inconsistency applies here.These lines also use
\u00f4for "ô" while identical characters appear as direct UTF-8 elsewhere in the file (e.g., line 101 "Chemin", line 288 "qpwIG+").✏️ Suggested normalization
- "/0J7Xq": "Impossible de traiter l'image de l'ic\u00f4ne", + "/0J7Xq": "Impossible de traiter l'image de l'icône",- "CLGask": "Ic\u00f4ne de l'application", + "CLGask": "Icône de l'application",Also applies to: 644-644
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/fr.json` at line 634, Keys like "/0J7Xq" (and the similar entry at line 644) contain the character encoded as "\u00f4" instead of the UTF-8 "ô", causing inconsistent encoding; update those JSON values to use the direct UTF-8 character (replace "\u00f4" with "ô") so the file uses a consistent UTF-8 representation like other entries (e.g., "Chemin"), and save the file with UTF-8 encoding to avoid reintroducing escaped sequences.
1158-1166: Unicode escape sequences are inconsistent with rest of file.These lines use escaped unicode (
\u00f4,\u00e9,\u00d7) while the rest of the file uses direct UTF-8 characters for accented letters. For example, "é" appears directly in hundreds of other entries but as\u00e9here.Both forms are valid JSON, but mixing encoding styles reduces readability and maintainability. Consider normalizing to direct UTF-8 characters for consistency.
✏️ Suggested normalization
- "jxsBGE": "Affich\u00e9 comme favicon dans la barre lat\u00e9rale, le s\u00e9lecteur de sites et les cartes de sites. Accepte tout format d'image \u2014 automatiquement redimensionn\u00e9 en 128\u00d7128 PNG. Max 50 Ko.", + "jxsBGE": "Affiché comme favicon dans la barre latérale, le sélecteur de sites et les cartes de sites. Accepte tout format d'image — automatiquement redimensionné en 128×128 PNG. Max 50 Ko.", "TJo5E6": "Aperçu", - "LUJTwh": "Ic\u00f4ne t\u00e9l\u00e9vers\u00e9e", - "nJNzqM": "Impossible de t\u00e9l\u00e9verser l'ic\u00f4ne", + "LUJTwh": "Icône téléversée", + "nJNzqM": "Impossible de téléverser l'icône", "JEsxDw": "Téléversement...", - "5Zmqv6": "T\u00e9l\u00e9verser l'ic\u00f4ne", - "S9jCu8": "Ic\u00f4ne supprim\u00e9e", - "smsDJm": "Impossible de supprimer l'ic\u00f4ne", - "MVEvjb": "Supprimer l'ic\u00f4ne", + "5Zmqv6": "Téléverser l'icône", + "S9jCu8": "Icône supprimée", + "smsDJm": "Impossible de supprimer l'icône", + "MVEvjb": "Supprimer l'icône",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/fr.json` around lines 1158 - 1166, Normalize the unicode escapes to direct UTF-8 characters for the listed message keys to match the rest of the file: replace sequences like \u00e9, \u00f4, \u00d7 in the values for "jxsBGE", "TJo5E6", "LUJTwh", "nJNzqM", "JEsxDw", "5Zmqv6", "S9jCu8", "smsDJm", and "MVEvjb" with their actual accented characters (é, ô, ×, etc.), ensuring the JSON file remains valid UTF-8 and preserving the exact text content and punctuation.client/messages/ja.json (1)
645-645: 画像サイズ表記（x/×）は揃えると読みやすいです。Line [645] の
128x128と Line [1158] の128×128が混在しています。✏️ Suggested fix
- "o9wmBB": "任意。128x128 PNG にリサイズされます。", + "o9wmBB": "任意。128×128 PNG にリサイズされます。",Also applies to: 1158-1158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/messages/ja.json` at line 645, Standardize the multiplication sign in the image size strings: change the value for key "o9wmBB" which currently uses "128x128" to use the same character as the other entry ("128×128"), and scan for any other occurrences to make all image-size mentions consistent (use either "x" or "×" project-wide—prefer the fullwidth multiplication sign "×" used elsewhere).client/src/app/[site]/components/Header/NoData.tsx (1)
102-107: Consider using a single translation key with interpolation for better i18n support.Splitting sentences across multiple
t()calls is an anti-pattern that can break in languages with different word orders (e.g., RTL languages, German, Japanese). The same pattern exists in the web branch (lines 129-138).♻️ Suggested approach
Use a single translation key with a placeholder for the link, or use the
richtext feature from next-intl:- <span className="text-xs text-muted-foreground"> - {t("See the")}{" "} - <a href="https://github.com/nks-hub/rybbit-flutter-sdk" target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:underline"> - {t("Flutter SDK documentation")} - </a>{" "} - {t("for installation and usage instructions.")} - </span> + <span className="text-xs text-muted-foreground"> + {t.rich("flutterSdkDocsMessage", { + link: (chunks) => ( + <a href="https://github.com/nks-hub/rybbit-flutter-sdk" target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:underline"> + {chunks} + </a> + ), + })} + </span>With a translation key like:
"flutterSdkDocsMessage": "See the <link>Flutter SDK documentation</link> for installation and usage instructions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/components/Header/NoData.tsx around lines 102 - 107, In NoData.tsx replace the split t() calls that build the sentence around the anchor with a single translation key using interpolation or next-intl's rich text support (e.g., a key like "flutterSdkDocsMessage" containing a <link> placeholder), then render that key with the anchor element injected so the entire sentence is translated as one unit; update the same pattern in the web branch instance and ensure you use the existing t function from the component to look up the new key and supply the link element as the interpolation/rich value.client/src/components/Favicon.tsx (1)
28-28: Localize the new alt text string.Line 28 adds a new user-facing string (
Icon for ...) but it is hardcoded. Please wire it throughuseTranslations()and messages files.As per coding guidelines, "Use next-intl's 'useTranslations()' hook for i18n; maintain translation files in messages/ directory (en.json, de.json, etc.)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/Favicon.tsx` at line 28, The alt text in the Favicon component is hardcoded; replace it with a localized string using next-intl's useTranslations() hook (import and call useTranslations in Favicon.tsx), change the alt prop to call the translator (e.g., t('favicon.alt', { domain })) and add the corresponding key ("favicon.alt": "Icon for {domain}") to the messages files (en.json, de.json, etc.); ensure you pass the domain variable as the interpolation parameter when calling the translator.client/src/app/[site]/events/components/EventLog/EventRow.tsx (1)
99-132: App-specific icons hidden, but no replacement shown.Hiding the Browser and DeviceIcon tooltips for app sites is appropriate since these are web-specific. However, the PR objectives mention that app sites track
device_modelandapp_version, yet this component doesn't display either for app events.Consider whether app-specific metadata (e.g., device model badge or app version) should appear here, or confirm this is intentionally displayed only in the details sheet or other views.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx around lines 99 - 132, EventRow currently hides Browser and DeviceIcon for app sites (isApp) but doesn't show app-specific metadata; update EventRow to render app-specific UI when isApp is true by replacing or augmenting those hidden blocks with components or elements showing event.device_model and event.app_version (e.g., a DeviceModelBadge and AppVersionBadge or simple TooltipContent) adjacent to OperatingSystem; ensure you conditionally render based on isApp, keep existing non-app behavior for Browser and DeviceIcon, and use the same Tooltip/TooltipTrigger/TooltipContent pattern so the new fields mirror existing accessibility/hover behavior.client/src/components/SiteSettings/SDKIntegration.tsx (1)
6-7: Unuseddomainprop.The
domainparameter is declared in the props but never referenced in the component body. Either remove it or use it (e.g., for display or as part of the SDK configuration example).♻️ Option 1: Remove unused prop
export function SDKIntegration({ siteId, siteType, - domain, }: { siteId: string; siteType: "app"; - domain: string; }) {Also applies to: 11-11
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/SiteSettings/SDKIntegration.tsx` around lines 6 - 7, The SDKIntegration component declares a domain prop that is never used; either remove domain from the component props or incorporate it into the UI/config example. Fix by editing the SDKIntegration function/component: if you don't need domain, delete domain from the props destructuring (and any related TypeScript prop type/interface); otherwise, reference domain where appropriate (for example in the displayed SDK config string or a label) so the prop is consumed. Ensure tests/types compile after the change and update any prop usages elsewhere that pass domain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/messages/de.json`:
- Around line 36-39: The German locale uses informal "du/deiner" in some
entries; update the strings to formal "Sie/Ihr/Ihnen" to match the rest of the
locale: replace the value for key "BiyUAo" with a formal variant (e.g., "Fügen
Sie das Rybbit SDK zu Ihrer App hinzu:") and similarly update the other affected
keys (the entries at the locations referenced in the review) so phrases like
"du/deiner" become "Sie/Ihre/Ihnen" and adjust surrounding grammar (e.g., "Sieh
dir die" → "Sehen Sie die" or "Sehen Sie sich die ... an") so the wording
remains natural and formal throughout.
In `@client/messages/es.json`:
- Line 288: The translation value for key "qpwIG+" in client/messages/es.json
ends with a trailing colon which can appear as dangling punctuation when shown
standalone; either remove the colon ("Error al cargar embudo") or change usages
that rely on concatenation to explicitly format with the additional text so the
colon is only present when followed by appended content. Update the "qpwIG+"
value accordingly and audit components that read this key to ensure they still
render correctly.
In `@client/messages/ja.json`:
- Line 406: Replace the mismatched label "入口画面" (key "E3X5ej") so all
occurrences use the same term and full-width colon: change "入口画面" to "エントリ画面："
and also update any entries currently using the half-width colon "エントリ画面:" (and
the other two entries mentioned) to use the full-width colon "：" so every label
consistently reads "エントリ画面：".
- Line 512: The translation for key "fsUN9Y" currently reads "最小スクリーンビュー" but is
missing the counter "数"; update the value for "fsUN9Y" to "最小スクリーンビュー数" so it
matches the adjacent phrasing like "最小ページビュー数" and keeps consistent granularity
in the UI copy.
In `@client/messages/pl.json`:
- Around line 37-39: The Polish translation strings are split awkwardly across
keys ("Z8TXx9", "5qOEz+", "3jM2wN") so the assembled sentence is unnatural;
update the value for key "3jM2wN" (or merge the keys) to include a connector
such as "aby" so the composed sentence reads like "Zobacz dokumentację Flutter
SDK, aby uzyskać instrukcje instalacji i użytkowania." Ensure the change
preserves punctuation and spacing so concatenation with "Z8TXx9" and "5qOEz+"
produces a single natural sentence.
In `@client/src/components/Favicon.tsx`:
- Around line 23-27: The conditional and template use truthiness which drops
valid numeric 0 values; update the Favicon component to check siteId and
iconVersion with explicit nullish checks (e.g., siteId !== null && siteId !==
undefined and iconVersion !== null && iconVersion !== undefined) while keeping
the existing imageError boolean check and className handling so zero site IDs or
version 0 still load the icon/version query string.
In `@client/src/components/SiteSettings/SDKIntegration.tsx`:
- Line 2: Replace the invalid next-intl hook useExtracted with useTranslations
in the SDKIntegration component: update the import to import { useTranslations }
from "next-intl" and change any calls of useExtracted(...) inside the
SDKIntegration component to useTranslations(...) (and then call the returned
translation function as t(key) where it previously used the extracted values).
Ensure all references to useExtracted are removed and the component uses the
useTranslations hook symbol instead.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Around line 420-465: The upload and delete handlers can run concurrently
because each button only checks its own flag (isUploadingIcon / isDeletingIcon);
change the logic so icon mutations are serialized by using a single shared flag
(e.g., isProcessingIcon) or by disabling each button when either flag is true
(disabled || isUploadingIcon || isDeletingIcon) and by guarding the handlers to
return early if a processing flag is set; update references in the component
(Button disabled props, the upload handler that uses uploadSiteIcon and
setIsUploadingIcon, and the delete handler that uses deleteSiteIcon and
setIsDeletingIcon) so both handlers set and clear the shared processing state
(or both flags) and bail out if another operation is in progress to prevent
races.
---
Outside diff comments:
In `@client/messages/cs.json`:
- Around line 1180-1183: The four empty translation entries "6dqYnF", "P7PLVj",
"/0TOL5", and "FgydNe" in client/messages/cs.json are missing Czech
invoice/billing labels; update each key with the correct Czech translations
(mirroring the meaning from the Portuguese set) so billing UI shows localized
text—locate the entries by the exact keys "6dqYnF", "P7PLVj", "/0TOL5", and
"FgydNe" and replace the empty string values with the appropriate Czech phrases
for those billing/invoice labels.
- Around line 594-595: The JSON translation file has empty values for keys
"Zh+5A6" and "OvzONl" (these appear to be the On/Off toggle strings); update
client/messages/cs.json to provide the Portuguese translations by setting
"Zh+5A6" to "Ligado" and "OvzONl" to "Desligado" so the toggles display
correctly.
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Line 37: The file's import and usage of useExtracted in the NoData component
is invalid; replace the import "useExtracted" with "useTranslations" from
"next-intl" and update any call sites (e.g., the hook invocation around line 73
inside the NoData component) to use useTranslations(...) accordingly (assign to
a translator variable and use that instead of the non-existent useExtracted).
Ensure the hook is imported as useTranslations and that all subsequent
calls/reference names match the new hook.
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Line 3: The import of useExtracted from next-intl in EventRow.tsx is invalid;
replace the import and hook usage with next-intl's standard useTranslations
hook: change the import statement to import useTranslations and update any call
sites (e.g., where useExtracted() is invoked inside the EventRow component) to
use useTranslations() instead so translations resolve at runtime; ensure any
variables/keys passed into the hook remain the same (for example, replace const
t = useExtracted() with const t = useTranslations('...') as used elsewhere).
---
Duplicate comments:
In `@client/messages/zh.json`:
- Around line 641-645: Update the localized strings to use a neutral "站点" term
and a consistent multiplication sign: change the value for key "CLGask" from
"应用图标" to "站点图标" (or "网站图标" if project standard uses 网站) and update the value
for key "o9wmBB" from "可选。将调整为 128x128 PNG。" to "可选。将调整为 128×128 PNG。"; also
make the same change for the corresponding "网站图标" entry referenced at the other
location (the duplicate around line 1158) so both keys use the same neutral
label and the unicode "×" in the size.
In `@client/src/app/components/AddSite.tsx`:
- Around line 183-187: The displayed copy still uses hardcoded English and
web-specific copy when siteType === "app"; update AddSite to use next-intl's
useTranslations hook (e.g., const t = useTranslations('AddSite')) and replace
the DialogTitle/DialogDescription strings and any placeholder/alt text in the
preview components with localized keys (e.g., t('title.web'), t('title.app'),
t('description.web'), t('description.app'), t('previewAlt.app'), etc.), and
apply the same changes for the preview/placeholder code referenced around the
preview block (the JSX that renders the preview/alt text and inputs between the
DialogTitle/DialogDescription area and lines ~220-228) so all app-specific text
uses t(...) instead of hardcoded English.
- Around line 82-88: Only upload the icon when the site is actually an app and
surface upload failures instead of silently logging them: before calling
uploadSiteIcon(iconBase64, site.siteId) guard on the current app-mode flag
(e.g., check site.type === 'app' or the local mode state that indicates "App"
mode) so stale iconBase64 from Web mode is ignored, and in the catch for
uploadSiteIcon replace console.warn with propagating or storing the error
(rethrow, return an error from the submit handler, or set a visible error state
/ toast) so the UI shows the upload failure to the user. Ensure you reference
iconBase64, uploadSiteIcon, and site.siteId when making these changes.
In `@client/src/components/Favicon.tsx`:
- Around line 11-27: The Favicon component currently appends the cache-busting
query only when the optional prop iconVersion is provided, so uploaded/deleted
icons stay stale; update the Favicon usage and/or signature to always pass a
stable cache-bust value (e.g., pass site.updatedAt or a dedicated iconVersion
from callers) and ensure the img src built in Favicon (using BACKEND_URL and
sites/{siteId}/icon) always includes `?v=` with that value (or a timestamp) even
when zero/empty, then update callers (SiteSelector and SiteCard) to forward
site.updatedAt (or computed iconVersion) into the Favicon component so the URL
changes when the icon is updated.
In `@client/src/components/Sessions/SessionDetails/SessionInfoTab.tsx`:
- Around line 45-46: The transient flip happens because isApp is computed as
false while siteMetadata is unresolved; change the derived flag to be tri-state
and only treat the page as web when siteMetadata is loaded: replace const isApp
= siteMetadata?.type === "app" with something like const isApp = siteMetadata ?
siteMetadata.type === "app" : undefined (or null) and update the conditional
rendering for the web-only rows (the blocks showing Device, Browser, Channel,
Referrer) to render them only when isApp === false (i.e., siteMetadata is loaded
and type !== "app"); apply the same pattern wherever isApp is used (all spots
referencing useGetSite/siteMetadata and isApp).
In `@client/src/components/SiteSettings/SiteConfiguration.tsx`:
- Around line 118-145: The text still uses "domain" and hardcoded English in the
package-name branch and input placeholder; update handleDomainChange and the
related input placeholder to use next-intl translations via useTranslations()
and switch wording based on siteType so package-name copy appears when siteType
!== "web". Specifically, replace hardcoded strings and toast messages around
newDomain validation (isValidPackageName branch), success/failure messages
(toast.success/toast.error), and the input placeholder to call
t("siteSettings.packageNamePlaceholder") / t("siteSettings.domainPlaceholder")
as appropriate; ensure normalizeDomain is only applied for siteType === "web"
and keep updateSiteConfig(siteMetadata.siteId, { domain: normalizedDomain ||
newDomain }) behavior while using localized message keys. Also apply the same
changes to the other occurrence handling the same logic (the block around the
other occurrence referenced).
In `@client/src/lib/imageUtils.ts`:
- Around line 16-31: The current drawImage call squashes non-square images;
update the image resizing in the routine that creates canvas/ctx/img/size so it
preserves aspect ratio by center-cropping to a square before scaling: read
img.width and img.height, compute a square source region (sx, sy, sSize) that
crops the longer dimension to the shorter (centered), then call
ctx.drawImage(img, sx, sy, sSize, sSize, 0, 0, size, size) instead of
ctx.drawImage(img, 0, 0, size, size); keep the existing canvas.toDataURL and
base64 extraction logic.
---
Nitpick comments:
In `@client/messages/es.json`:
- Line 69: The Spanish translation key "GZJpDf" currently uses the singular
label "Analítica"; update the value for "GZJpDf" in client/messages/es.json to
the plural form "Analíticas" so it matches the surrounding UI terminology and
pluralized navigation labels.
- Line 633: Update the package-name validation messages so both Spanish entries
consistently include the expected format example; change the message with key
"39ieyq" and the other package-name key referenced around the second occurrence
so they both read something like "Formato de nombre de paquete no válido. Debe
ser como com.example.app" (ensure exact phrasing matches project style), keeping
the same message ID keys ("39ieyq" and the other package-name key) so callers
are unaffected.
- Line 182: Update the Spanish copy for key "4vQ9fL" to use the standardized
terminology—replace "El seguimiento de errores captura errores y excepciones de
tu aplicación." with a phrasing that uses "rastreo" and consistent ordering,
e.g. "El rastreo de errores captura las excepciones y errores de tu
aplicación."; apply the same wording to the other identical entry mentioned in
the review so both occurrences use "rastreo" and the same phrasing.
In `@client/messages/fr.json`:
- Line 634: Keys like "/0J7Xq" (and the similar entry at line 644) contain the
character encoded as "\u00f4" instead of the UTF-8 "ô", causing inconsistent
encoding; update those JSON values to use the direct UTF-8 character (replace
"\u00f4" with "ô") so the file uses a consistent UTF-8 representation like other
entries (e.g., "Chemin"), and save the file with UTF-8 encoding to avoid
reintroducing escaped sequences.
- Around line 1158-1166: Normalize the unicode escapes to direct UTF-8
characters for the listed message keys to match the rest of the file: replace
sequences like \u00e9, \u00f4, \u00d7 in the values for "jxsBGE", "TJo5E6",
"LUJTwh", "nJNzqM", "JEsxDw", "5Zmqv6", "S9jCu8", "smsDJm", and "MVEvjb" with
their actual accented characters (é, ô, ×, etc.), ensuring the JSON file remains
valid UTF-8 and preserving the exact text content and punctuation.
In `@client/messages/ja.json`:
- Line 645: Standardize the multiplication sign in the image size strings:
change the value for key "o9wmBB" which currently uses "128x128" to use the same
character as the other entry ("128×128"), and scan for any other occurrences to
make all image-size mentions consistent (use either "x" or "×"
project-wide—prefer the fullwidth multiplication sign "×" used elsewhere).
In `@client/messages/zh.json`:
- Around line 1140-1142: The keys "wc+2Lv", "mFg4Ag", and "sbjdWu" use the term
“跟踪” which is inconsistent with the surrounding UI copy that uses “追踪”; update
the values for these keys to use the unified term “追踪” (e.g., change
"跟踪用户点击外部链接的情况" → use "追踪...") so the translation is consistent across the file,
and run a quick grep for these keys to ensure no other instances remain.
In `@client/src/app/`[site]/components/Header/NoData.tsx:
- Around line 102-107: In NoData.tsx replace the split t() calls that build the
sentence around the anchor with a single translation key using interpolation or
next-intl's rich text support (e.g., a key like "flutterSdkDocsMessage"
containing a <link> placeholder), then render that key with the anchor element
injected so the entire sentence is translated as one unit; update the same
pattern in the web branch instance and ensure you use the existing t function
from the component to look up the new key and supply the link element as the
interpolation/rich value.
In `@client/src/app/`[site]/events/components/EventLog/EventRow.tsx:
- Around line 99-132: EventRow currently hides Browser and DeviceIcon for app
sites (isApp) but doesn't show app-specific metadata; update EventRow to render
app-specific UI when isApp is true by replacing or augmenting those hidden
blocks with components or elements showing event.device_model and
event.app_version (e.g., a DeviceModelBadge and AppVersionBadge or simple
TooltipContent) adjacent to OperatingSystem; ensure you conditionally render
based on isApp, keep existing non-app behavior for Browser and DeviceIcon, and
use the same Tooltip/TooltipTrigger/TooltipContent pattern so the new fields
mirror existing accessibility/hover behavior.
In `@client/src/components/Favicon.tsx`:
- Line 28: The alt text in the Favicon component is hardcoded; replace it with a
localized string using next-intl's useTranslations() hook (import and call
useTranslations in Favicon.tsx), change the alt prop to call the translator
(e.g., t('favicon.alt', { domain })) and add the corresponding key
("favicon.alt": "Icon for {domain}") to the messages files (en.json, de.json,
etc.); ensure you pass the domain variable as the interpolation parameter when
calling the translator.
In `@client/src/components/SiteSettings/SDKIntegration.tsx`:
- Around line 6-7: The SDKIntegration component declares a domain prop that is
never used; either remove domain from the component props or incorporate it into
the UI/config example. Fix by editing the SDKIntegration function/component: if
you don't need domain, delete domain from the props destructuring (and any
related TypeScript prop type/interface); otherwise, reference domain where
appropriate (for example in the displayed SDK config string or a label) so the
prop is consumed. Ensure tests/types compile after the change and update any
prop usages elsewhere that pass domain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d85c9cc6-e49b-474f-9c9d-827a5962f04c
📒 Files selected for processing (29)
client/messages/cs.jsonclient/messages/de.jsonclient/messages/en.jsonclient/messages/es.jsonclient/messages/fr.jsonclient/messages/it.jsonclient/messages/ja.jsonclient/messages/ko.jsonclient/messages/pl.jsonclient/messages/pt.jsonclient/messages/zh.jsonclient/src/api/admin/endpoints/index.tsclient/src/api/admin/endpoints/sites.tsclient/src/app/[site]/components/Header/NoData.tsxclient/src/app/[site]/components/Sidebar/SiteSelector.tsxclient/src/app/[site]/events/components/EventLog/EventRow.tsxclient/src/app/components/AddSite.tsxclient/src/components/Favicon.tsxclient/src/components/Sessions/SessionDetails/SessionInfoTab.tsxclient/src/components/SiteCard.tsxclient/src/components/SiteSettings/SDKIntegration.tsxclient/src/components/SiteSettings/SiteConfiguration.tsxclient/src/lib/imageUtils.tsserver/src/api/sites/index.tsserver/src/api/sites/siteIcon.tsserver/src/db/postgres/schema.tsserver/src/index.tsserver/src/services/tracker/pageviewQueue.tsserver/src/utils.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- server/src/index.ts
- server/src/services/tracker/pageviewQueue.ts
- server/src/utils.ts
- client/messages/it.json
- client/messages/en.json
- client/src/api/admin/endpoints/sites.ts
- client/src/app/[site]/components/Sidebar/SiteSelector.tsx
- server/src/api/sites/siteIcon.ts
- client/src/components/SiteCard.tsx
Comment on lines
+36
to
+39
| "BiyUAo": "Füge das Rybbit SDK zu deiner App hinzu:", | ||
| "Z8TXx9": "Sieh dir die", | ||
| "5qOEz+": "Flutter SDK Dokumentation", | ||
| "3jM2wN": "für Installations- und Nutzungsanweisungen an.", |
Contributor
There was a problem hiding this comment.
Keep German tone consistent (formal “Sie”) in the new app/SDK copy.
Line 36, Line 37, Line 1094, and Line 1138 use informal phrasing (du/deiner), while this locale is otherwise mostly formal. This will feel inconsistent in UI.
✏️ Suggested wording alignment
- "BiyUAo": "Füge das Rybbit SDK zu deiner App hinzu:",
- "Z8TXx9": "Sieh dir die",
+ "BiyUAo": "Fügen Sie das Rybbit SDK zu Ihrer App hinzu:",
+ "Z8TXx9": "Sehen Sie sich die",
@@
- "z0p82B": "Installiere das Rybbit Flutter SDK, um Ereignisse aus deiner App zu verfolgen.",
+ "z0p82B": "Installieren Sie das Rybbit Flutter SDK, um Ereignisse aus Ihrer App zu verfolgen.",
@@
- "c1bAPv": "Erfasse Fehler und Ausnahmen aus deiner App",
+ "c1bAPv": "Erfassen Sie Fehler und Ausnahmen aus Ihrer App",Also applies to: 1094-1094, 1138-1138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/de.json` around lines 36 - 39, The German locale uses
informal "du/deiner" in some entries; update the strings to formal
"Sie/Ihr/Ihnen" to match the rest of the locale: replace the value for key
"BiyUAo" with a formal variant (e.g., "Fügen Sie das Rybbit SDK zu Ihrer App
hinzu:") and similarly update the other affected keys (the entries at the
locations referenced in the review) so phrases like "du/deiner" become
"Sie/Ihre/Ihnen" and adjust surrounding grammar (e.g., "Sieh dir die" → "Sehen
Sie die" or "Sehen Sie sich die ... an") so the wording remains natural and
formal throughout.
| "znwd1N": "Eliminar embudo", | ||
| "qpwIG+": "Error al cargar embudo:", | ||
| "qDwvZ4": "Error desconocido", | ||
| "qpwIG+": "Error al cargar embudo:", |
Contributor
There was a problem hiding this comment.
Avoid trailing colon unless a message is always concatenated.
This can render as a dangling punctuation mark if shown standalone.
Suggested tweak
- "qpwIG+": "Error al cargar embudo:",
+ "qpwIG+": "Error al cargar embudo",📝 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
| "qpwIG+": "Error al cargar embudo:", | |
| "qpwIG+": "Error al cargar embudo", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/es.json` at line 288, The translation value for key "qpwIG+"
in client/messages/es.json ends with a trailing colon which can appear as
dangling punctuation when shown standalone; either remove the colon ("Error al
cargar embudo") or change usages that rely on concatenation to explicitly format
with the additional text so the colon is only present when followed by appended
content. Update the "qpwIG+" value accordingly and audit components that read
this key to ensure they still render correctly.
| "pEc/5k": "クリック率", | ||
| "wU7PzI": "最終クリック", | ||
| "4KEfRY": "タイトル", | ||
| "E3X5ej": "入口画面", |
Contributor
There was a problem hiding this comment.
用語とコロン表記を統一してください。
Line [406] が「入口画面」、Line [1029] が「エントリ画面:」で揺れています。加えて Line [1019], Line [1020], Line [1029] は半角コロン : になっており、同ファイル内のラベル表記（：）と不一致です。
✏️ Suggested fix
- "tH05Pi": "デバイスモデル:",
- "ShHw0c": "アプリバージョン:",
- "RvxWt3": "エントリ画面:",
+ "tH05Pi": "デバイスモデル：",
+ "ShHw0c": "アプリバージョン：",
+ "RvxWt3": "入口画面：",Also applies to: 1019-1020, 1029-1029
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/ja.json` at line 406, Replace the mismatched label "入口画面"
(key "E3X5ej") so all occurrences use the same term and full-width colon: change
"入口画面" to "エントリ画面：" and also update any entries currently using the half-width
colon "エントリ画面:" (and the other two entries mentioned) to use the full-width
colon "：" so every label consistently reads "エントリ画面：".
| "56nzII": "{value}週目", | ||
| "xnky+a": "識別済みのみ", | ||
| "O/IeYM": "ユーザーの識別方法を学ぶ", | ||
| "fsUN9Y": "最小スクリーンビュー", |
Contributor
There was a problem hiding this comment.
文言に助数詞が抜けています。
Line [512] の「最小スクリーンビュー」は、隣接する「最小ページビュー数」と粒度を揃えるなら「数」を付ける方が自然です。
✏️ Suggested fix
- "fsUN9Y": "最小スクリーンビュー",
+ "fsUN9Y": "最小スクリーンビュー数",📝 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
| "fsUN9Y": "最小スクリーンビュー", | |
| "fsUN9Y": "最小スクリーンビュー数", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/ja.json` at line 512, The translation for key "fsUN9Y"
currently reads "最小スクリーンビュー" but is missing the counter "数"; update the value
for "fsUN9Y" to "最小スクリーンビュー数" so it matches the adjacent phrasing like
"最小ページビュー数" and keeps consistent granularity in the UI copy.
Comment on lines
+37
to
+39
| "Z8TXx9": "Zobacz", | ||
| "5qOEz+": "dokumentację Flutter SDK", | ||
| "3jM2wN": "instrukcje instalacji i użytkowania.", |
Contributor
There was a problem hiding this comment.
Polish sentence split is awkward without a connector
At Line 39, the phrase should include a connector so the composed sentence reads naturally (e.g., “Zobacz … aby uzyskać instrukcje …”).
💬 Suggested text fix
- "3jM2wN": "instrukcje instalacji i użytkowania.",
+ "3jM2wN": "aby uzyskać instrukcje instalacji i użytkowania.",📝 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
| "Z8TXx9": "Zobacz", | |
| "5qOEz+": "dokumentację Flutter SDK", | |
| "3jM2wN": "instrukcje instalacji i użytkowania.", | |
| "Z8TXx9": "Zobacz", | |
| "5qOEz+": "dokumentację Flutter SDK", | |
| "3jM2wN": "aby uzyskać instrukcje instalacji i użytkowania.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/messages/pl.json` around lines 37 - 39, The Polish translation strings
are split awkwardly across keys ("Z8TXx9", "5qOEz+", "3jM2wN") so the assembled
sentence is unnatural; update the value for key "3jM2wN" (or merge the keys) to
include a connector such as "aby" so the composed sentence reads like "Zobacz
dokumentację Flutter SDK, aby uzyskać instrukcje instalacji i użytkowania."
Ensure the change preserves punctuation and spacing so concatenation with
"Z8TXx9" and "5qOEz+" produces a single natural sentence.
Comment on lines
+23
to
+27
| if (siteId && !imageError) { | ||
| return ( | ||
| <img | ||
| src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion ? `?v=${iconVersion}` : ""}`} | ||
| className={cn("rounded", className ?? "w-4 h-4")} |
Contributor
There was a problem hiding this comment.
Use explicit nullish checks for numeric props.
Line 23 (if (siteId && !imageError)) and Line 26 (iconVersion ? ...) rely on truthiness. This drops valid 0 values and can silently disable icon loading or versioning.
Suggested fix
- if (siteId && !imageError) {
+ if (siteId !== undefined && !imageError) {
@@
- src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion ? `?v=${iconVersion}` : ""}`}
+ src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion !== undefined ? `?v=${iconVersion}` : ""}`}📝 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
| if (siteId && !imageError) { | |
| return ( | |
| <img | |
| src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion ? `?v=${iconVersion}` : ""}`} | |
| className={cn("rounded", className ?? "w-4 h-4")} | |
| if (siteId !== undefined && !imageError) { | |
| return ( | |
| <img | |
| src={`${BACKEND_URL}/sites/${siteId}/icon${iconVersion !== undefined ? `?v=${iconVersion}` : ""}`} | |
| className={cn("rounded", className ?? "w-4 h-4")} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/Favicon.tsx` around lines 23 - 27, The conditional and
template use truthiness which drops valid numeric 0 values; update the Favicon
component to check siteId and iconVersion with explicit nullish checks (e.g.,
siteId !== null && siteId !== undefined and iconVersion !== null && iconVersion
!== undefined) while keeping the existing imageError boolean check and className
handling so zero site IDs or version 0 still load the icon/version query string.
| @@ -0,0 +1,39 @@ | |||
| "use client"; | |||
| import { useExtracted } from "next-intl"; | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
next-intl useTranslations hook API version 4
💡 Result:
useTranslations (next-intl v4) API
- Import:
import {useTranslations} from 'next-intl';[1] - Signature:
useTranslations(namespace?: string)→ returns a translation functiont[1]- With a namespace:
const t = useTranslations('About');thent('title')[1] - Without a namespace (access everything via dotted keys):
const t = useTranslations();thent('About.title')[1]
- With a namespace:
- Formatting / rich text: use
t.rich(key, values)to map tags in messages to React components [1]
Server Components note (v4)
- In non-async Server Components (and shared components), you can use
useTranslationsdirectly. [2] - In async Server Components (can’t use hooks), use the awaitable API:
import {getTranslations} from 'next-intl/server';thenconst t = await getTranslations('ProfilePage');[2]
(Next-intl is currently in the v4 line; e.g., the repo shows v4.8.3 released Feb 16, 2026.) [3]
Sources: [1] https://next-intl.dev/docs/usage/translations · [2] https://next-intl.dev/docs/environments/server-client-components · [3] https://github.com/amannn/next-intl
🏁 Script executed:
cat -n client/src/components/SiteSettings/SDKIntegration.tsx | head -20Repository: rybbit-io/rybbit
Length of output: 705
🏁 Script executed:
cat -n client/src/components/SiteSettings/SDKIntegration.tsxRepository: rybbit-io/rybbit
Length of output: 1487
Replace useExtracted with useTranslations from next-intl.
The useExtracted hook is not a valid export from next-intl. Use useTranslations instead, as required for client components in client/src/**.
Proposed fix
"use client";
-import { useExtracted } from "next-intl";
+import { useTranslations } from "next-intl";
export function SDKIntegration({
siteId,
siteType,
domain,
}: {
siteId: string;
siteType: "app";
domain: string;
}) {
- const t = useExtracted();
+ const t = useTranslations();📝 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
| import { useExtracted } from "next-intl"; | |
| "use client"; | |
| import { useTranslations } from "next-intl"; | |
| export function SDKIntegration({ | |
| siteId, | |
| siteType, | |
| domain, | |
| }: { | |
| siteId: string; | |
| siteType: "app"; | |
| domain: string; | |
| }) { | |
| const t = useTranslations(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/SiteSettings/SDKIntegration.tsx` at line 2, Replace the
invalid next-intl hook useExtracted with useTranslations in the SDKIntegration
component: update the import to import { useTranslations } from "next-intl" and
change any calls of useExtracted(...) inside the SDKIntegration component to
useTranslations(...) (and then call the returned translation function as t(key)
where it previously used the extracted values). Ensure all references to
useExtracted are removed and the component uses the useTranslations hook symbol
instead.
Comment on lines
+420
to
+465
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| disabled={disabled || isUploadingIcon} | ||
| onClick={() => { | ||
| const input = document.createElement("input"); | ||
| input.type = "file"; | ||
| input.accept = "image/*"; | ||
| input.onchange = async (e) => { | ||
| const file = (e.target as HTMLInputElement).files?.[0]; | ||
| if (!file) return; | ||
| setIsUploadingIcon(true); | ||
| try { | ||
| const base64 = await resizeImageToIcon(file); | ||
| await uploadSiteIcon(siteMetadata.siteId, base64); | ||
| setIconVersion((v) => v + 1); | ||
| toast.success(t("Icon uploaded")); | ||
| } catch { | ||
| toast.error(t("Failed to upload icon")); | ||
| } finally { | ||
| setIsUploadingIcon(false); | ||
| } | ||
| }; | ||
| input.click(); | ||
| }} | ||
| > | ||
| <Upload className="h-4 w-4" /> | ||
| {isUploadingIcon ? t("Uploading...") : t("Upload Icon")} | ||
| </Button> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="text-muted-foreground" | ||
| disabled={disabled || isDeletingIcon} | ||
| onClick={async () => { | ||
| setIsDeletingIcon(true); | ||
| try { | ||
| await deleteSiteIcon(siteMetadata.siteId); | ||
| setIconVersion((v) => v + 1); | ||
| toast.success(t("Icon removed")); | ||
| } catch { | ||
| toast.error(t("Failed to remove icon")); | ||
| } finally { | ||
| setIsDeletingIcon(false); | ||
| } | ||
| }} |
Contributor
There was a problem hiding this comment.
Serialize icon upload and delete mutations.
The upload button stays enabled during delete, and the delete button stays enabled during upload. Two writes to the same icon resource can race, so the final state depends on request order.
Possible fix
+ const isMutatingIcon = isUploadingIcon || isDeletingIcon;
...
- disabled={disabled || isUploadingIcon}
+ disabled={disabled || isMutatingIcon}
...
- disabled={disabled || isDeletingIcon}
+ disabled={disabled || isMutatingIcon}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/SiteSettings/SiteConfiguration.tsx` around lines 420 -
465, The upload and delete handlers can run concurrently because each button
only checks its own flag (isUploadingIcon / isDeletingIcon); change the logic so
icon mutations are serialized by using a single shared flag (e.g.,
isProcessingIcon) or by disabling each button when either flag is true (disabled
|| isUploadingIcon || isDeletingIcon) and by guarding the handlers to return
early if a processing flag is set; update references in the component (Button
disabled props, the upload handler that uses uploadSiteIcon and
setIsUploadingIcon, and the delete handler that uses deleteSiteIcon and
setIsDeletingIcon) so both handlers set and clear the shared processing state
(or both flags) and bail out if another operation is in progress to prevent
races.
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
Adds first-class support for app sites (mobile, desktop, hybrid) alongside existing web sites. An app site tracks app-specific data (app version, device model, screen views) and the dashboard adapts accordingly — hiding irrelevant web-only features and showing app-specific breakdowns.
For a detailed description of the motivation, architecture, and all changes, see #922.
What's new
Server
Schema:
sites.type(web/app),sites.icon(bytea), ClickHouseapp_version+device_modelcolumnsUser-Agent parsing: RFC 7231 parser extracts app metadata from SDK clients
Icon API:
GET /sites/:id/icon(public, PNG, cached 24h),PUT(admin, base64, 50KB max),DELETEAnalytics endpoints:
app_versionanddevice_modelexposed in events, sessions, usersFilters:
app_versionanddevice_modelaccepted as filter dimensionsClient
AddSite: Web/App platform toggle, package name validation, optional icon upload with canvas resize
Dashboard: platform-aware layout — Screens/Devices/OS/App Version tabs for app sites
Sessions/Events/Users: app version badges, device model columns, app-specific details
Settings: App Icon upload/delete/preview, web-only toggles hidden, SDK Integration tab
Favicon: custom icon from API for app sites, Smartphone fallback
Filters:
getFilterGroups(siteType)across all pagesTranslations
All new strings in 11 languages (en, cs, de, es, fr, it, ja, ko, pl, pt, zh).
Companion SDK
nks-hub/rybbit-flutter-sdk — Flutter/Dart SDK with offline queue, lifecycle tracking, error capture, GA4 events, auto icon upload. 60 tests, multi-platform. GPL v3.
We're happy to transfer this repo to
rybbit-ioor have it forked for official maintenance.Summary by CodeRabbit
New Features
Improvements
Bug Fixes