Repository navigation
feat: SEO & GEO insights - #504
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new SEO dashboard powered by Google Search Console: backend GSC service and controller endpoints (dashboard/details with filters, pagination, branded classification), persisted project Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as AnalyticsController
participant DB as ProjectStore
participant GSC as GSCService
participant Google as GSC API
Client->>Controller: GET /analytics/gsc-dashboard (pid, period, timeBucket, filters)
Controller->>DB: checkProjectAccess(pid, uid, x-password) / checkBillingAccess(pid)
Controller->>GSC: getDashboard(pid, groupFrom, groupTo, timeBucket, filters)
par parallel fetches
GSC->>Google: searchanalytics.query (summary / dateSeries / topPages / topQueries / countries / devices)
GSC->>GSC: compute branded traffic (iterative queries + keyword matching)
end
Google-->>GSC: rows/metrics
GSC-->>Controller: aggregated dashboard payload
Controller-->>Client: GSCDashboardResponse
sequenceDiagram
participant ClientReq
participant Parser as parseFilters()
participant Builder as getDimensionFilterGroups()
participant Query as searchanalytics.query
ClientReq->>Parser: filtersStr (JSON)
Parser->>Parser: JSON.parse & map columns/operators (pg→page, keywords→query, cc→country, dv→device)
Parser->>Builder: produce dimensionFilterGroups + extraFilters (e.g., page)
Builder->>Query: attach filters to GSC query
Query-->>ClientReq: filtered results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Blaumaus
marked this pull request as ready for review
April 7, 2026 17:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/lib/constants/index.ts (1)
499-517:⚠️ Potential issue | 🔴 CriticalRuntime/type mismatch causes undefined keys in selfhosted deployments.
The cast at lines 515–517 forces the conditional
PROJECT_TABSto thePRODUCTION_PROJECT_TABStype, masking thatSELFHOSTED_PROJECT_TABSlacksseo,ai,alerts, andexperiments. This creates critical issues:
- Lines 170, 178–180:
tabNamesobject unconditionally maps production-only keys, but in selfhosted mode at runtime these resolve toundefined.- Lines 482, 546, etc.: Tab comparisons like
tab === PROJECT_TABS.seofail silently in selfhosted becausePROJECT_TABS.seoisundefinedat runtime.Remove the cast and explicitly handle the type difference, or guard all production-only key accesses (e.g.,
PROJECT_TABS.seo && tabNames[PROJECT_TABS.seo]).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/lib/constants/index.ts` around lines 499 - 517, PROJECT_TABS is being force-cast to typeof PRODUCTION_PROJECT_TABS which hides missing keys on SELFHOSTED_PROJECT_TABS; remove that cast and either type PROJECT_TABS as a union of typeof PRODUCTION_PROJECT_TABS | typeof SELFHOSTED_PROJECT_TABS (or a shared keys interface) and update call sites to handle optional keys, or keep the cast removed and add runtime guards/optional checks where production-only keys are used (e.g., replace unconditional uses of PROJECT_TABS.seo / tabNames[PROJECT_TABS.seo] with PROJECT_TABS.seo && tabNames[PROJECT_TABS.seo] or optional chaining) so accesses to PRODUCTION-only keys (seo, ai, alerts, experiments) won’t produce undefined at runtime; locate symbols PRODUCTION_PROJECT_TABS, SELFHOSTED_PROJECT_TABS, PROJECT_TABS and the tabNames usage to apply the change.
🧹 Nitpick comments (3)
web/app/pages/Project/Settings/tabs/General.tsx (1)
11-11: WirebrandKeywordsfield errors instead of hardcodingnull.Right now this input can’t surface validation feedback.
Proposed refactor
interface GeneralProps { form: { name?: string id?: string websiteUrl?: string | null brandKeywords?: string } errors: { name?: string websiteUrl?: string + brandKeywords?: string } @@ <Input name='brandKeywords' label={t('project.settings.brandKeywords')} hint={t('project.settings.brandKeywordsHint')} value={form.brandKeywords || ''} placeholder={t('project.settings.brandKeywordsPlaceholder')} className='mt-4' onChange={handleInput} - error={null} + error={beenSubmitted ? errors.brandKeywords : null} />Also applies to: 64-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/Settings/tabs/General.tsx` at line 11, The Brand Keywords input currently hardcodes its error display to null; update the component(s) that render the brandKeywords field (referencing the brandKeywords prop/name) to surface validation feedback by wiring the form errors instead of null — e.g., read formState.errors.brandKeywords (or errors.brandKeywords?.message) from your react-hook-form context and pass that string into the input/Field error/helperText props; do the same replacement for the other occurrences around lines 64-73 so all brandKeywords inputs show real validation messages.backend/apps/cloud/src/project/gsc.service.ts (2)
345-350: Page filter uses exact URL match.The page filter uses the default
equalsoperator, requiring an exact URL match. If partial matching is intended (e.g., matching all URLs under a path), consider usingcontainsoperator instead.💡 Alternative with contains operator
if (page) { dimensionFilterGroups.push({ groupType: 'and', - filters: [{ dimension: 'page', expression: page }], + filters: [{ dimension: 'page', operator: 'contains', expression: page }], }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/project/gsc.service.ts` around lines 345 - 350, The page filter currently builds a dimension filter that uses the default equals operator (see the dimensionFilterGroups construction and the filters entry where dimension: 'page'), so change that filter to use the contains operator if you intend partial/path matching: update the filters item for page to include operator: 'contains' instead of relying on exact match so URLs under a path will be matched (locate the page branch that pushes into dimensionFilterGroups and add the operator property to the filter object).
307-310: Silent error swallowing hides debugging information.Parse failures return
undefinedsilently, making it difficult to diagnose issues when users provide malformed filter strings. Consider logging the error for observability.♻️ Proposed improvement
} catch (e) { - // Ignore parse errors + console.warn('Failed to parse GSC filters:', e) } return undefined🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/project/gsc.service.ts` around lines 307 - 310, The catch block currently swallowing parse errors ("catch { // Ignore parse errors }") should capture the error and log it before returning undefined: change to "catch (err)" and call the service logger (e.g., this.logger.error(..., err)) or console.error if no instance logger exists, including the malformed input or context so the failure in the parse function in gsc.service.ts is observable; then keep returning undefined as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/analytics/analytics.controller.ts`:
- Around line 916-919: The current logging in the GET /analytics/gsc-dashboard
flow (see the logger.log call in analytics.controller.ts and the getGSCDetails
path) writes raw filter/page/query payloads which may contain PII; update the
logging to avoid raw values by either (1) moving the logger.log call to after
the access/checks have completed so sensitive input isn't logged pre-auth, and
(2) replace the raw variables (filters, pages, queries, finalTimeBucket) in the
log with sanitized summaries — e.g., counts, presence flags, truncated strings,
or a stable hash (sha256) — and log those summaries instead; locate the
logger.log usage and the getGSCDetails handling code to apply the change.
In `@backend/apps/cloud/src/project/entity/project.entity.ts`:
- Around line 183-185: The Swagger docs are wrong because Project.brandKeywords
is declared as string|null but responses send string[]|null; fix by updating the
OpenAPI type instead of leaving `@ApiProperty`() ambiguous: either change the
entity decorator to `@ApiProperty`({ type: [String], nullable: true }) on
brandKeywords OR (preferred) create a clear response DTO (e.g.,
ProjectResponseDto) with brandKeywords: string[] | null and replace usages of
`@ApiResponse`({ type: Project }) in the controller methods that return projects
(the methods in project.controller that currently reference Project in
`@ApiResponse`) to `@ApiResponse`({ type: ProjectResponseDto }); ensure the DTO is
used for documentation only so runtime entities remain unchanged.
In `@backend/apps/cloud/src/project/project.controller.ts`:
- Around line 1820-1827: The code blindly JSON.parses result.brandKeywords and
returns it even if the decoded value isn't a string array; change the handling
in project.controller.ts (the block that reads and assigns result.brandKeywords
from JSON) to parse inside the try, validate that the parsed value is an Array
and every element is a string (e.g., Array.isArray(parsed) && parsed.every(item
=> typeof item === 'string')), and only then assign it as string[]; otherwise
set result.brandKeywords = null. Apply the same shape-checking fix to the other
identical parsing site in this file (the second brandKeywords parse block).
- Around line 1808-1813: The code currently maps projectDTO.brandKeywords
through _trim and serializes them even when entries become empty strings; update
the logic in the project controller where projectDTO.brandKeywords is handled so
you trim each keyword, filter out any resulting empty strings (e.g., k =>
_trim(k) and then keep only non-empty), and only JSON.stringify when the
filtered array has length > 0, otherwise set project.brandKeywords = null;
reference the projectDTO.brandKeywords handling, the project.brandKeywords
assignment, and the _trim call to locate the change.
In `@web/app/pages/Project/Settings/ProjectSettings.tsx`:
- Line 326: The ProjectSettings initialization currently always includes
brandKeywords (brandKeywords: initialProject.brandKeywords?.join(', ') || ''),
causing CE/self-hosted backends to receive unsupported fields; update the
initialization in ProjectSettings.tsx to only set brandKeywords when the
cloud-only settings path is active (use the existing cloud/self-hosted guard
used elsewhere, e.g., isSelfhosted or cloudSettings flag) and ensure the submit
payload omits brandKeywords for self-hosted builds; also update the General tab
(General.tsx) to conditionally render/hide the Brand Keywords input when running
self-hosted by guarding its JSX with the same isSelfhosted/cloud-only check so
the field is neither shown nor sent for CE/self-hosted deployments.
In `@web/app/pages/Project/tabs/SEO/SEOView.tsx`:
- Around line 1093-1149: The trend badge directions are inverted for the
MetricCard components: update the MetricCard with label t('project.seo.clicks'),
t('project.seo.impressions'), and t('project.seo.avgCTR') to use
goodChangeDirection='up' (since higher is better), and update the MetricCard
with label t('project.seo.avgPosition') to use goodChangeDirection='down' (since
a lower average position is better); locate these in SEOView.tsx where
MetricCard is rendered and change the goodChangeDirection prop accordingly.
- Around line 159-162: deriveBrandKeywords currently maps project.brandKeywords
but does not filter out empty tokens, so entries like '' make lower.includes('')
always true; update the deriveBrandKeywords function to first filter out
falsy/whitespace-only strings (e.g., k => k && k.trim().length) then map to
lowercased trimmed values, and ensure it returns an empty array when
project.brandKeywords is missing to avoid undefined downstream.
- Around line 126-155: The aggregation currently sums per-day ctr and position
into buckets and then divides by count in the return mapping, which skews
multi-day CTR/position when daily traffic varies; update the logic in the series
aggregation (the for ... of series loop that builds buckets and the return
mapping that reads from buckets) to accumulate total clicks and impressions and
also accumulate positionWeighted = position * impressions (instead of summing
raw position and ctr), then in the final map compute ctr = totalClicks /
totalImpressions (guarding against zero impressions) and position =
positionWeighted / totalImpressions; keep using getBucketKey and the same
buckets/orderedKeys structure but replace b.ctr and b.position averaging with
these recomputed values.
- Around line 735-755: The tooltip contents function is injecting the raw GSC
query name (variable name) into the returned HTML, creating an XSS risk; update
the contents (the contents: (items: any) => { ... }) to HTML-escape/sanitize the
query string before interpolation (escape the variable name used in the
template), e.g. call an existing escapeHtml/sanitize utility or add one and use
it on name (and any other user-controlled values like impressions if needed) so
the tooltip HTML only contains escaped text rather than raw user input.
In `@web/app/pages/Project/View/Panels.tsx`:
- Around line 207-211: The div wrapper is using fallback via || so an empty
string passed as contentClassName is treated as unset and the default 'relative
flex h-[19.6rem] flex-col overflow-hidden' stays applied, which clips the chart;
update the JSX className expression on that div to use nullish coalescing
(contentClassName ?? 'relative flex h-[19.6rem] flex-col overflow-hidden') so
callers can intentionally pass an empty string to clear the default wrapper
classes (refer to the div with className and the contentClassName prop in
Panels.tsx).
In `@web/app/routes/api.analytics.ts`:
- Around line 124-125: The branch for getGSCDetails is reading page and query
from (body as any) instead of using the typed ProxyRequest.params contract;
update getGSCDetails to pull all endpoint inputs (page, query, etc.) from params
(like getGSCDashboard does) and pass those params into getGSCDetailsServer,
ensuring ProxyRequest.params is the source of truth so callers using
fetch*(projectId, params) are honored and typings remain correct.
In `@web/app/routes/projects`.$id.tsx:
- Around line 482-488: The branch handling PROJECT_TABS.seo is preloading
trafficData by calling getProjectDataServer unnecessarily; remove the
getProjectDataServer(...) promise assignment for the seo tab so trafficData is
not fetched here. Keep the PROJECT_TABS.seo branch free of trafficData preloads
and rely on SEOView/useGSCDashboardProxy to fetch its own dashboard or, if
referral data is later required, fetch the narrower dataset inside SEOView
instead of in this route. Ensure any references to the trafficData variable in
this branch are omitted so no unused network call is started.
---
Outside diff comments:
In `@web/app/lib/constants/index.ts`:
- Around line 499-517: PROJECT_TABS is being force-cast to typeof
PRODUCTION_PROJECT_TABS which hides missing keys on SELFHOSTED_PROJECT_TABS;
remove that cast and either type PROJECT_TABS as a union of typeof
PRODUCTION_PROJECT_TABS | typeof SELFHOSTED_PROJECT_TABS (or a shared keys
interface) and update call sites to handle optional keys, or keep the cast
removed and add runtime guards/optional checks where production-only keys are
used (e.g., replace unconditional uses of PROJECT_TABS.seo /
tabNames[PROJECT_TABS.seo] with PROJECT_TABS.seo && tabNames[PROJECT_TABS.seo]
or optional chaining) so accesses to PRODUCTION-only keys (seo, ai, alerts,
experiments) won’t produce undefined at runtime; locate symbols
PRODUCTION_PROJECT_TABS, SELFHOSTED_PROJECT_TABS, PROJECT_TABS and the tabNames
usage to apply the change.
---
Nitpick comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 345-350: The page filter currently builds a dimension filter that
uses the default equals operator (see the dimensionFilterGroups construction and
the filters entry where dimension: 'page'), so change that filter to use the
contains operator if you intend partial/path matching: update the filters item
for page to include operator: 'contains' instead of relying on exact match so
URLs under a path will be matched (locate the page branch that pushes into
dimensionFilterGroups and add the operator property to the filter object).
- Around line 307-310: The catch block currently swallowing parse errors ("catch
{ // Ignore parse errors }") should capture the error and log it before
returning undefined: change to "catch (err)" and call the service logger (e.g.,
this.logger.error(..., err)) or console.error if no instance logger exists,
including the malformed input or context so the failure in the parse function in
gsc.service.ts is observable; then keep returning undefined as before.
In `@web/app/pages/Project/Settings/tabs/General.tsx`:
- Line 11: The Brand Keywords input currently hardcodes its error display to
null; update the component(s) that render the brandKeywords field (referencing
the brandKeywords prop/name) to surface validation feedback by wiring the form
errors instead of null — e.g., read formState.errors.brandKeywords (or
errors.brandKeywords?.message) from your react-hook-form context and pass that
string into the input/Field error/helperText props; do the same replacement for
the other occurrences around lines 64-73 so all brandKeywords inputs show real
validation messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f74d8605-a097-4215-95ba-f4ae755c19bc
📒 Files selected for processing (33)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/dto/get-keywords.dto.tsbackend/apps/cloud/src/project/dto/project.dto.tsbackend/apps/cloud/src/project/entity/project.entity.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.controller.tsbackend/migrations/mysql/2026_04_05_brand_keywords.sqldocs/content/docs/analytics-dashboard/meta.jsondocs/content/docs/analytics-dashboard/seo.mdxdocs/content/docs/analytics-dashboard/traffic.mdxdocs/content/docs/integrations/google-search-console.mdxdocs/content/docs/introduction.mdxweb/app/api/api.server.tsweb/app/hooks/useAnalyticsProxy.tsweb/app/lib/constants/index.tsweb/app/lib/models/Project.tsweb/app/pages/OpenStartup/index.tsxweb/app/pages/Project/Settings/ProjectSettings.tsxweb/app/pages/Project/Settings/tabs/General.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/View/components/ProjectSidebar.tsxweb/app/pages/Project/View/utils/filters.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/referrers.map.jsonweb/app/routes/api.analytics.tsweb/app/routes/projects.$id.tsxweb/app/routes/projects.settings.$id.tsxweb/app/styles/ProjectViewStyle.cssweb/app/utils/referrers.tsweb/public/locales/en.json
💤 Files with no reviewable changes (1)
- web/app/pages/OpenStartup/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
web/app/pages/Project/tabs/SEO/SEOView.tsx (2)
1088-1088:⚠️ Potential issue | 🟠 MajorThe KPI change directions are still inverted.
Higher clicks, impressions, and CTR are good, while lower average position is good. These props currently mark gains as regressions and regressions as improvements.
Also applies to: 1102-1102, 1116-1116, 1133-1133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/SEOView.tsx` at line 1088, The KPI change direction props are inverted: update the components that render the clicks, impressions, and CTR metrics to use goodChangeDirection='up' (so increases are shown as improvements) and ensure the average position metric uses goodChangeDirection='down' (so decreases are shown as improvements); locate the instances where goodChangeDirection='down' is passed for those metric renderers (the KPI/metric components that reference clicks, impressions, ctr, and averagePosition in SEOView.tsx) and change the prop values accordingly for each duplicated occurrence mentioned.
724-728:⚠️ Potential issue | 🔴 CriticalEscape query names before interpolating them into tooltip HTML.
names[d.index]comes from GSC query text and is inserted into a raw HTML string here. A crafted query can inject markup or script into the billboard tooltip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/SEOView.tsx` around lines 724 - 728, The tooltip HTML is directly interpolating user-controlled names[d.index] (and similarly impressions[d.index]) into a template string in SEOView.tsx which allows HTML/script injection; update the code that builds the tooltip (the template string around the <ul> with ${name}) to escape HTML special characters before interpolation (create or use a sanitizing helper like escapeHtml and apply it to names[d.index] and any other user data used in the string), or alternatively build the tooltip using text nodes/DOM APIs instead of raw innerHTML so values are treated as text, ensuring names and impressions cannot inject markup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 371-377: The current code in gsc.service.ts derives a brand
keyword from the penultimate hostname label (variables url, domain, parts, main,
keywords), which misclassifies multi-part TLDs like example.com.au; update the
fallback so it extracts the first label of the registrable domain instead of
parts[parts.length-2] (use a public suffix-aware library or the Public Suffix
List to get the registrable domain and then take its first label), and if you
cannot reliably compute the registrable domain, skip adding a keyword from the
URL fallback so the later query.includes(keyword) check does not produce overly
broad branded matches.
- Around line 583-586: The hourly bucket key formatting drops the timezone
offset by using dayjs(row.keys[0]).format('YYYY-MM-DD HH:mm:ss'), causing
browser-local parsing shifts; update the isHourly branch that sets date (the
date: row.keys?.[0] ? isHourly ? ... : ...) to preserve the original offset by
either returning the raw row.keys[0] value or formatting it as an
offset-preserving ISO (e.g., an ISO string that includes the timezone offset) so
the hour bucket stays unshifted when parsed in the browser.
In `@web/app/pages/Project/tabs/SEO/SEOView.tsx`:
- Around line 778-795: The chart is assigning incompatible metrics (counts vs
CTR vs position) to the same axis; update the axis-assignment logic around
activeMetrics, columns, axes and needsY2 so only compatible metric families
share an axis: group metrics into families (counts: clicks/impressions, rates:
ctr, position: position), detect which families are active, and then either (A)
only map one compatible family to y2 (e.g., counts) and leave CTR and position
on their own axes (different axis keys like 'y_ctr' and 'y_pos') or (B) if you
want to keep two axes, split into separate charts when more than two families
are active; implement the family-check before pushing label/axes (use
aggregatedSeries, label, colors as before) so CTR and position are never forced
onto the same axis as counts or each other.
---
Duplicate comments:
In `@web/app/pages/Project/tabs/SEO/SEOView.tsx`:
- Line 1088: The KPI change direction props are inverted: update the components
that render the clicks, impressions, and CTR metrics to use
goodChangeDirection='up' (so increases are shown as improvements) and ensure the
average position metric uses goodChangeDirection='down' (so decreases are shown
as improvements); locate the instances where goodChangeDirection='down' is
passed for those metric renderers (the KPI/metric components that reference
clicks, impressions, ctr, and averagePosition in SEOView.tsx) and change the
prop values accordingly for each duplicated occurrence mentioned.
- Around line 724-728: The tooltip HTML is directly interpolating
user-controlled names[d.index] (and similarly impressions[d.index]) into a
template string in SEOView.tsx which allows HTML/script injection; update the
code that builds the tooltip (the template string around the <ul> with ${name})
to escape HTML special characters before interpolation (create or use a
sanitizing helper like escapeHtml and apply it to names[d.index] and any other
user data used in the string), or alternatively build the tooltip using text
nodes/DOM APIs instead of raw innerHTML so values are treated as text, ensuring
names and impressions cannot inject markup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0de22cd6-1a8e-47eb-9f53-0d930e91651a
📒 Files selected for processing (5)
backend/apps/cloud/src/project/gsc.service.tsweb/app/api/api.server.tsweb/app/pages/Project/View/ViewProject.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/routes/projects.$id.tsx
✅ Files skipped from review due to trivial changes (1)
- web/app/pages/Project/View/ViewProject.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/app/routes/projects.$id.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/app/utils/toc.ts (1)
1-8:⚠️ Potential issue | 🔴 CriticalImport statement must be placed before the interface declaration.
The
importstatement on line 8 appears after theTocIteminterface declaration (lines 1-6). In TypeScript/JavaScript, allimportstatements must be at the top of the file before any other code. This would cause a syntax error.Proposed fix to move import to top of file
+import { escapeHtml } from './generic' + interface TocItem { id: string text: string level: number children: TocItem[] } - -import { escapeHtml } from './generic'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/utils/toc.ts` around lines 1 - 8, Move the import so it's at the top of the file before any declarations: place the existing "import { escapeHtml } from './generic'" above the TocItem interface declaration. This ensures module imports are declared prior to the TocItem interface and avoids the syntax error; reference TocItem and the escapeHtml import to locate the lines to reorder.web/app/pages/Project/View/Panels.tsx (1)
2037-2056:⚠️ Potential issue | 🟡 MinorAccessibility issue:
role='presentation'is incorrect for an interactive element.When
onRowClickis provided, this div becomes interactive viaonClick, butrole='presentation'tells assistive technologies to ignore it. Additionally, keyboard users cannot interact with this element since there's notabIndexor keyboard event handlers.Consider:
- Conditionally applying the role based on whether
onRowClickis set- Adding keyboard support when interactive
Suggested fix
)} - onClick={ - onRowClick && entryName - ? () => onRowClick(entryName) - : undefined - } - role='presentation' + onClick={ + onRowClick && entryName + ? () => onRowClick(entryName) + : undefined + } + onKeyDown={ + onRowClick && entryName + ? (e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + onRowClick(entryName) + } + } + : undefined + } + tabIndex={onRowClick && entryName ? 0 : undefined} + role={onRowClick && entryName ? 'button' : 'presentation'} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/View/Panels.tsx` around lines 2037 - 2056, The div currently uses role='presentation' while also becoming interactive via onRowClick (see the div with onClick that references onRowClick and entryName); remove or conditionally set role='presentation' only when onRowClick is absent, and when onRowClick is present make the element keyboard-accessible by adding tabIndex={0} and an onKeyDown handler that invokes onRowClick(entryName) for Enter and Space keys (and preventDefault for Space), and add an appropriate ARIA attribute (e.g., aria-pressed or aria-selected) if isSelected semantics apply; ensure the conditional class logic (cursor-pointer vs cursor-wait) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 883-889: The previous-period window is computed using durationMs =
toDate.diff(fromDate), which treats the GSC YYYY-MM-DD range as exclusive and
shortens the prior window; fix by calculating an inclusive span before
subtracting: compute the inclusive length as days = toDate.diff(fromDate, 'day')
+ 1 (or equivalently make durationMs based on toDate.add(1,
'day').diff(fromDate)), then set prevTo = fromDate.subtract(1,
'day').format(...) and prevFrom = fromDate.subtract(days, 'day').format(...).
Update the logic around fromDate, toDate, durationMs, prevTo, and prevFrom to
use the inclusive-day calculation so the previousSummary window matches GSC's
inclusive range.
In `@backend/apps/cloud/src/project/project.controller.ts`:
- Around line 1808-1815: Update the UpdateProjectDto so that brandKeywords can
be explicitly cleared (null) by altering its validation: change the property
type to allow null (e.g., brandKeywords: string[] | null) and add a validator to
skip array/string checks when null—for example add `@ValidateIf`(o =>
o.brandKeywords !== null) above the existing validators (or use a custom
validator that permits null), keeping `@IsArray/`@IsString checks for non-null
values; this ensures the controller logic handling project.brandKeywords (the
normalization in project.controller using projectDTO.brandKeywords) receives
null when intended instead of failing validation.
---
Outside diff comments:
In `@web/app/pages/Project/View/Panels.tsx`:
- Around line 2037-2056: The div currently uses role='presentation' while also
becoming interactive via onRowClick (see the div with onClick that references
onRowClick and entryName); remove or conditionally set role='presentation' only
when onRowClick is absent, and when onRowClick is present make the element
keyboard-accessible by adding tabIndex={0} and an onKeyDown handler that invokes
onRowClick(entryName) for Enter and Space keys (and preventDefault for Space),
and add an appropriate ARIA attribute (e.g., aria-pressed or aria-selected) if
isSelected semantics apply; ensure the conditional class logic (cursor-pointer
vs cursor-wait) remains unchanged.
In `@web/app/utils/toc.ts`:
- Around line 1-8: Move the import so it's at the top of the file before any
declarations: place the existing "import { escapeHtml } from './generic'" above
the TocItem interface declaration. This ensures module imports are declared
prior to the TocItem interface and avoids the syntax error; reference TocItem
and the escapeHtml import to locate the lines to reorder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2f671d2-d0d5-4c4a-b269-825f2ec8df50
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
backend/apps/cloud/src/project/entity/project.entity.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.controller.tsbackend/package.jsonweb/app/pages/Project/Settings/ProjectSettings.tsxweb/app/pages/Project/Settings/tabs/General.tsxweb/app/pages/Project/View/Panels.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/routes/api.analytics.tsweb/app/utils/generic.tsweb/app/utils/toc.ts
✅ Files skipped from review due to trivial changes (2)
- backend/package.json
- web/app/utils/generic.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- web/app/pages/Project/Settings/tabs/General.tsx
- backend/apps/cloud/src/project/entity/project.entity.ts
- web/app/pages/Project/tabs/SEO/SEOView.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/apps/cloud/src/project/gsc.service.ts (1)
582-585:⚠️ Potential issue | 🟠 MajorPreserve hourly key timezone offset end-to-end.
At Line [584], formatting
row.keys[0]toYYYY-MM-DD HH:mm:ssstrips the offset from the GSC hourly key. That can shift buckets when parsed client-side in local timezone. Return the raw hourly key (or another offset-preserving format) instead.🕒 Suggested fix
return rows.map((row) => ({ date: row.keys?.[0] ? isHourly - ? dayjs.utc(row.keys[0]).format('YYYY-MM-DD HH:mm:ss') + ? row.keys[0] : row.keys[0] : '',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/project/gsc.service.ts` around lines 582 - 585, The current logic that formats row.keys?.[0] when isHourly (in the method handling GSC aggregation rows) strips the timezone offset and can shift hourly buckets client-side; change the isHourly branch to return the raw hourly key (row.keys[0]) or an offset-preserving ISO string instead of formatting to 'YYYY-MM-DD HH:mm:ss' so the original offset is preserved end-to-end—update the code paths around row.keys?.[0] and the isHourly conditional in the GSC service method to return the unformatted key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 277-315: The parseFilters function currently swallows JSON
parse/shape errors and returns undefined, allowing malformed filters to silently
pass; update parseFilters to validate input and throw a BadRequestException
(imported from `@nestjs/common`) when JSON.parse fails or when the parsed value is
not an array or filters have an unexpected shape (missing column or
filter/value), instead of returning undefined — keep the existing mapping logic
(refer to parseFilters, mappedFilters, and the operator/dimension mapping) but
wrap parsing/validation so any parse error or invalid structure results in
throwing new BadRequestException('Invalid filters parameter') so callers receive
a 400 for bad input.
- Around line 398-400: Add explicit bounds validation for Search Console
pagination parameters: ensure the limit/offset passed by getKeywords,
getTopPages, getTopCountries, and getTopDevices are validated/clamped to the
API's allowed range (rowLimit 1–25000) instead of forwarding raw values. Update
the relevant DTO(s) used by these methods (or create a shared GscPaginationDto
similar to GetSessionsDto/GetProfilesDto) and annotate the limit with `@Min`(1)
and `@Max`(25000) (and offset with `@Min`(0) if applicable), or apply server-side
clamping before calling the Search Console client inside the methods to return a
400 for invalid inputs or safely clamp to [1,25000]; locate changes around the
getKeywords/getTopPages/getTopCountries/getTopDevices method definitions and
their request DTOs to implement the validation consistently.
---
Duplicate comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 582-585: The current logic that formats row.keys?.[0] when
isHourly (in the method handling GSC aggregation rows) strips the timezone
offset and can shift hourly buckets client-side; change the isHourly branch to
return the raw hourly key (row.keys[0]) or an offset-preserving ISO string
instead of formatting to 'YYYY-MM-DD HH:mm:ss' so the original offset is
preserved end-to-end—update the code paths around row.keys?.[0] and the isHourly
conditional in the GSC service method to return the unformatted key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35658495-e2d4-425d-9031-8ecc6eb253a5
📒 Files selected for processing (2)
backend/apps/cloud/src/project/dto/project.dto.tsbackend/apps/cloud/src/project/gsc.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/cloud/src/project/dto/project.dto.ts
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/apps/cloud/src/project/gsc.service.ts (1)
353-365:⚠️ Potential issue | 🟡 MinorReturn a 400 for malformed
filtersinstead of silently dropping them.At Line 359-Line 365, invalid JSON/shape currently degrades to
undefined, which can silently remove all filters and return misleading data.💡 Proposed fix
private parseFilters(filtersStr?: string) { if (!filtersStr) return undefined let filters: unknown try { filters = JSON.parse(filtersStr) } catch { - return undefined + throw new BadRequestException('Invalid filters parameter') } - if (!Array.isArray(filters) || filters.length > MAX_FILTERS) { - return undefined - } + if (!Array.isArray(filters) || filters.length > MAX_FILTERS) { + throw new BadRequestException('Invalid filters parameter') + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/apps/cloud/src/project/gsc.service.ts` around lines 353 - 365, The parseFilters function currently returns undefined on invalid JSON/shape which silently drops filters; instead, change parseFilters(filtersStr?: string) to validate and, on parse failure or invalid shape (not an array or length > MAX_FILTERS), throw a BadRequestException (or equivalent HTTP 400 error) with a clear message like "malformed filters" and include the parse error details when available; update callers of parseFilters if needed to let the exception propagate so the controller returns a 400 response rather than silently ignoring filters.
🧹 Nitpick comments (4)
web/app/pages/Project/tabs/SEO/CompactReferralPanel.tsx (2)
55-70: Consider extracting a shared panel shell/header to reduce duplication.The empty and non-empty branches repeat the same container/header structure; extracting a small wrapper would simplify maintenance.
Also applies to: 116-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/CompactReferralPanel.tsx` around lines 55 - 70, The JSX for the panel container and header is duplicated in CompactReferralPanel (both the empty branch around the _isEmpty(data) check and the non-empty branch later); extract a small reusable wrapper component (e.g., PanelShell or CompactPanelShell) that accepts props icon, title and children and renders the outer div with the rounded border, header row (using icon and title) and inner content slot, then replace the duplicated markup in CompactReferralPanel with this new component (update both places that render the container/header so they pass the same icon, title and inner content).
13-18: TightenrowMappertyping to avoidany.Line 17 uses
any, which removes compile-time checks for renderer inputs. Sincedatais typed asEntry[]androwMapperreceives entries from that array at line 91, the parameter should beEntryto preserve type safety. The callback passed in SEOView.tsx (refRowMapper) only destructures thenameproperty, which is available on Entry.Proposed diff
interface CompactReferralPanelProps { title: string data: Entry[] icon: React.ReactNode - rowMapper: (entry: any) => React.ReactNode + rowMapper: (entry: Entry) => React.ReactNode }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/CompactReferralPanel.tsx` around lines 13 - 18, The rowMapper parameter is currently typed as any which weakens type safety; update the CompactReferralPanelProps interface to type rowMapper as (entry: Entry) => React.ReactNode so it accepts the same Entry objects as the data prop, then ensure usages (e.g., the call site in CompactReferralPanel where rowMapper is invoked and the refRowMapper passed from SEOView.tsx) match the new signature and still only access Entry properties like name.web/app/pages/Project/tabs/SEO/seo-utils.ts (1)
26-48: Use a typedTimeBucketunion instead of rawstring.Line 26 and Line 47 accept any string; this makes invalid bucket values easy to pass silently. A union type gives compile-time protection and keeps chart/aggregation behavior aligned.
💡 Proposed fix
+export type TimeBucket = 'hour' | 'day' | 'week' | 'month' | 'quarter' | 'year' + -const getBucketKey = (date: string, bucket: string): string => { +const getBucketKey = (date: string, bucket: TimeBucket): string => { @@ export const aggregateDateSeries = ( series: DateSeriesEntry[], - bucket: string, + bucket: TimeBucket, ): DateSeriesEntry[] => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/seo-utils.ts` around lines 26 - 48, Replace the untyped bucket parameters with a TimeBucket union to prevent invalid values: define a type alias TimeBucket = 'hour' | 'day' | 'week' | 'month' | 'quarter' | 'year' and change the bucket parameter types on getBucketKey and aggregateDateSeries from string to TimeBucket; update any callers (or export TimeBucket) so they compile, and optionally remove or narrow the switch default branch now that bucket values are exhaustive for getBucketKey.web/app/pages/Project/tabs/SEO/seo-chart-options.ts (1)
19-19: Reduceanyusage in chart option builders.Line 19, Line 154, Line 231, and Line 261 use
any, which weakens static checks in a chart-heavy module. Prefer narrow local types (or billboard callback types) for safer refactors.Also applies to: 154-154, 231-231, 261-261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/pages/Project/tabs/SEO/seo-chart-options.ts` at line 19, The use of any for chart column arrays (e.g., the columns variable and the other three spots) weakens type safety; replace any[] with a narrow array type such as Columns from billboard.js (import { Columns } from 'billboard.js') or a local type like Array<Array<string | number | null>> / (string | number | null)[][] that matches your date/metric shapes, then update the declarations where columns and the other three any usages appear (search for the symbol columns and the other variables at the flagged locations) so callers and callbacks get proper static checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 873-878: The current try/catch around await
this.getGSCContext(pid) in gsc.service.ts maps every exception to {
notConnected: true }; change it to inspect the caught error (e) instead of
swallowing it: if e clearly represents a missing/unlinked GSC account (e.g.,
specific error.name, code like 'NO_GSC_CONTEXT', or HTTP 401/403 indicating
unauthorized), then return { notConnected: true }, otherwise rethrow the error
(or propagate it) so transient OAuth/Google API failures surface to
callers/logging; keep the change localized around the getGSCContext call and its
surrounding method.
In `@web/app/pages/Project/tabs/SEO/seo-chart-options.ts`:
- Around line 140-151: The tooltip contents callback in seo-chart-options.ts
accesses items[0] (e.g., when computing headerLabel with
d3.timeFormat(tooltipFmt)(items[0].x)) without checking for an empty items
array; add an early guard at the top of the contents function to return an empty
string (or minimal HTML) if !items || items.length === 0 to avoid runtime
exceptions, and apply the same empty-array check for the other places in this
file where items[0] is used (the second callback around the other access at the
same module) so the billboard.js tooltip handlers never dereference items[0]
when payloads are empty.
In `@web/app/pages/Project/tabs/SEO/seo-utils.ts`:
- Around line 45-94: aggregateDateSeries currently returns buckets in the
insertion (first-seen) order via orderedKeys, which can be wrong if input series
is unsorted; update aggregateDateSeries to sort the bucket keys chronologically
before mapping to results — after building buckets and orderedKeys, derive a
sortedKeys array by converting each key back to a Date (use getBucketKey’s
inverse or Date.parse on the key/ISO string) and sort ascending, then map
sortedKeys to produce the final DateSeriesEntry array so the output is always
chronological (refer to aggregateDateSeries, orderedKeys, buckets and
getBucketKey).
---
Duplicate comments:
In `@backend/apps/cloud/src/project/gsc.service.ts`:
- Around line 353-365: The parseFilters function currently returns undefined on
invalid JSON/shape which silently drops filters; instead, change
parseFilters(filtersStr?: string) to validate and, on parse failure or invalid
shape (not an array or length > MAX_FILTERS), throw a BadRequestException (or
equivalent HTTP 400 error) with a clear message like "malformed filters" and
include the parse error details when available; update callers of parseFilters
if needed to let the exception propagate so the controller returns a 400
response rather than silently ignoring filters.
---
Nitpick comments:
In `@web/app/pages/Project/tabs/SEO/CompactReferralPanel.tsx`:
- Around line 55-70: The JSX for the panel container and header is duplicated in
CompactReferralPanel (both the empty branch around the _isEmpty(data) check and
the non-empty branch later); extract a small reusable wrapper component (e.g.,
PanelShell or CompactPanelShell) that accepts props icon, title and children and
renders the outer div with the rounded border, header row (using icon and title)
and inner content slot, then replace the duplicated markup in
CompactReferralPanel with this new component (update both places that render the
container/header so they pass the same icon, title and inner content).
- Around line 13-18: The rowMapper parameter is currently typed as any which
weakens type safety; update the CompactReferralPanelProps interface to type
rowMapper as (entry: Entry) => React.ReactNode so it accepts the same Entry
objects as the data prop, then ensure usages (e.g., the call site in
CompactReferralPanel where rowMapper is invoked and the refRowMapper passed from
SEOView.tsx) match the new signature and still only access Entry properties like
name.
In `@web/app/pages/Project/tabs/SEO/seo-chart-options.ts`:
- Line 19: The use of any for chart column arrays (e.g., the columns variable
and the other three spots) weakens type safety; replace any[] with a narrow
array type such as Columns from billboard.js (import { Columns } from
'billboard.js') or a local type like Array<Array<string | number | null>> /
(string | number | null)[][] that matches your date/metric shapes, then update
the declarations where columns and the other three any usages appear (search for
the symbol columns and the other variables at the flagged locations) so callers
and callbacks get proper static checks.
In `@web/app/pages/Project/tabs/SEO/seo-utils.ts`:
- Around line 26-48: Replace the untyped bucket parameters with a TimeBucket
union to prevent invalid values: define a type alias TimeBucket = 'hour' | 'day'
| 'week' | 'month' | 'quarter' | 'year' and change the bucket parameter types on
getBucketKey and aggregateDateSeries from string to TimeBucket; update any
callers (or export TimeBucket) so they compile, and optionally remove or narrow
the switch default branch now that bucket values are exhaustive for
getBucketKey.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 197bbf8b-b928-4534-917a-f9393c138e38
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/project/gsc.service.tsbackend/apps/cloud/src/project/project.controller.tsbackend/package.jsonweb/app/api/api.server.tsweb/app/pages/Project/tabs/SEO/CompactReferralPanel.tsxweb/app/pages/Project/tabs/SEO/SEOView.tsxweb/app/pages/Project/tabs/SEO/seo-chart-options.tsweb/app/pages/Project/tabs/SEO/seo-utils.tsweb/app/routes/projects.$id.tsx
✅ Files skipped from review due to trivial changes (2)
- backend/package.json
- backend/apps/cloud/src/project/project.controller.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- web/app/routes/projects.$id.tsx
- backend/apps/cloud/src/analytics/analytics.controller.ts
- web/app/pages/Project/tabs/SEO/SEOView.tsx
Comment on lines
+140
to
+151
| contents: (items, _defaultTitleFormat, _defaultValueFormat, color) => { | ||
| const tooltipFormatMap: Record<string, string> = { | ||
| hour: '%b %d, %Y %H:%M', | ||
| day: '%b %d, %Y', | ||
| week: 'Week of %b %d, %Y', | ||
| month: '%B %Y', | ||
| quarter: '%B %Y', | ||
| year: '%Y', | ||
| } | ||
| const tooltipFmt = tooltipFormatMap[timeBucket] || '%b %d, %Y' | ||
| const headerLabel = d3.timeFormat(tooltipFmt)(items[0].x) | ||
| return `<ul class='bg-gray-50 dark:text-gray-50 dark:bg-slate-900 rounded-md ring-1 ring-black/10 px-2 py-1 text-xs md:text-sm max-h-[250px] md:max-h-[350px] overflow-y-auto shadow-md z-50'> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -300 web/app/pages/Project/tabs/SEO/seo-chart-options.ts | tail -170Repository: Swetrix/swetrix
Length of output: 5317
🏁 Script executed:
rg -n "tooltip.*contents" web/app/pages/Project/tabs/SEO/seo-chart-options.ts -A 3Repository: Swetrix/swetrix
Length of output: 41
🏁 Script executed:
rg -n "items\[0\]" web/app/pages/Project/tabs/SEO/Repository: Swetrix/swetrix
Length of output: 263
🏁 Script executed:
# Check if there are tests or examples showing empty items
rg -n "items.*length\|items.*empty" web/app/ --type ts --type tsxRepository: Swetrix/swetrix
Length of output: 86
🏁 Script executed:
head -20 web/app/pages/Project/tabs/SEO/seo-chart-options.tsRepository: Swetrix/swetrix
Length of output: 717
🏁 Script executed:
# Search for similar guards in other tooltip callbacks in the project
rg -n "tooltip" web/app/ --type ts -B 2 -A 5 | grep -A 5 "items\|contents"Repository: Swetrix/swetrix
Length of output: 13445
Guard tooltip callbacks against empty items array.
Lines 150 and 262 access items[0] without verifying the array is not empty. This pattern is inconsistently applied across the codebase—other tooltip callbacks in GoalsView.tsx, ViewProject.helpers.tsx, ExperimentResults.tsx, and ErrorsView.tsx all guard against this. Add an early return to prevent runtime exceptions when the billboard.js chart invokes these callbacks with an empty payload.
Proposed fix
tooltip: {
contents: (items, _defaultTitleFormat, _defaultValueFormat, color) => {
+ if (!items?.length) return ''
const tooltipFormatMap: Record<string, string> = { tooltip: {
contents: (items: any) => {
+ if (!items?.length) return ''
const d = items[0]📝 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
| contents: (items, _defaultTitleFormat, _defaultValueFormat, color) => { | |
| const tooltipFormatMap: Record<string, string> = { | |
| hour: '%b %d, %Y %H:%M', | |
| day: '%b %d, %Y', | |
| week: 'Week of %b %d, %Y', | |
| month: '%B %Y', | |
| quarter: '%B %Y', | |
| year: '%Y', | |
| } | |
| const tooltipFmt = tooltipFormatMap[timeBucket] || '%b %d, %Y' | |
| const headerLabel = d3.timeFormat(tooltipFmt)(items[0].x) | |
| return `<ul class='bg-gray-50 dark:text-gray-50 dark:bg-slate-900 rounded-md ring-1 ring-black/10 px-2 py-1 text-xs md:text-sm max-h-[250px] md:max-h-[350px] overflow-y-auto shadow-md z-50'> | |
| contents: (items, _defaultTitleFormat, _defaultValueFormat, color) => { | |
| if (!items?.length) return '' | |
| const tooltipFormatMap: Record<string, string> = { | |
| hour: '%b %d, %Y %H:%M', | |
| day: '%b %d, %Y', | |
| week: 'Week of %b %d, %Y', | |
| month: '%B %Y', | |
| quarter: '%B %Y', | |
| year: '%Y', | |
| } | |
| const tooltipFmt = tooltipFormatMap[timeBucket] || '%b %d, %Y' | |
| const headerLabel = d3.timeFormat(tooltipFmt)(items[0].x) | |
| return `<ul class='bg-gray-50 dark:text-gray-50 dark:bg-slate-900 rounded-md ring-1 ring-black/10 px-2 py-1 text-xs md:text-sm max-h-[250px] md:max-h-[350px] overflow-y-auto shadow-md z-50'> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/pages/Project/tabs/SEO/seo-chart-options.ts` around lines 140 - 151,
The tooltip contents callback in seo-chart-options.ts accesses items[0] (e.g.,
when computing headerLabel with d3.timeFormat(tooltipFmt)(items[0].x)) without
checking for an empty items array; add an early guard at the top of the contents
function to return an empty string (or minimal HTML) if !items || items.length
=== 0 to avoid runtime exceptions, and apply the same empty-array check for the
other places in this file where items[0] is used (the second callback around the
other access at the same module) so the billboard.js tooltip handlers never
dereference items[0] when payloads are empty.
Comment on lines
+45
to
+94
| export const aggregateDateSeries = ( | ||
| series: DateSeriesEntry[], | ||
| bucket: string, | ||
| ): DateSeriesEntry[] => { | ||
| if (!series.length || bucket === 'day' || bucket === 'hour') return series | ||
|
|
||
| const buckets = new Map< | ||
| string, | ||
| { | ||
| clicks: number | ||
| impressions: number | ||
| weightedPosition: number | ||
| } | ||
| >() | ||
| const orderedKeys: string[] = [] | ||
|
|
||
| for (const entry of series) { | ||
| const key = getBucketKey(entry.date, bucket) | ||
| const existing = buckets.get(key) | ||
| if (existing) { | ||
| existing.clicks += entry.clicks | ||
| existing.impressions += entry.impressions | ||
| existing.weightedPosition += entry.position * entry.impressions | ||
| } else { | ||
| orderedKeys.push(key) | ||
| buckets.set(key, { | ||
| clicks: entry.clicks, | ||
| impressions: entry.impressions, | ||
| weightedPosition: entry.position * entry.impressions, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| return orderedKeys.map((key) => { | ||
| const b = buckets.get(key)! | ||
| return { | ||
| date: key, | ||
| clicks: b.clicks, | ||
| impressions: b.impressions, | ||
| ctr: | ||
| b.impressions > 0 | ||
| ? Number(((b.clicks / b.impressions) * 100).toFixed(2)) | ||
| : 0, | ||
| position: | ||
| b.impressions > 0 | ||
| ? Number((b.weightedPosition / b.impressions).toFixed(1)) | ||
| : 0, | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Ensure aggregated buckets are returned in chronological order.
Line 59 and Line 78 currently preserve first-seen key order, not actual date order. If upstream series ever arrives unsorted, chart points will be out of sequence.
💡 Proposed fix
- const orderedKeys: string[] = []
@@
- orderedKeys.push(key)
buckets.set(key, {
clicks: entry.clicks,
impressions: entry.impressions,
weightedPosition: entry.position * entry.impressions,
})
@@
- return orderedKeys.map((key) => {
+ const orderedKeys = [...buckets.keys()].sort(
+ (a, b) => dayjs(a).valueOf() - dayjs(b).valueOf(),
+ )
+
+ return orderedKeys.map((key) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/app/pages/Project/tabs/SEO/seo-utils.ts` around lines 45 - 94,
aggregateDateSeries currently returns buckets in the insertion (first-seen)
order via orderedKeys, which can be wrong if input series is unsorted; update
aggregateDateSeries to sort the bucket keys chronologically before mapping to
results — after building buckets and orderedKeys, derive a sortedKeys array by
converting each key back to a Date (use getBucketKey’s inverse or Date.parse on
the key/ISO string) and sort ascending, then map sortedKeys to produce the final
DateSeriesEntry array so the output is always chronological (refer to
aggregateDateSeries, orderedKeys, buckets and getBucketKey).
This was referenced Apr 8, 2026
This was referenced Apr 28, 2026
This was referenced May 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit