Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions apps/webapp/app/components/logs/LogsVersionFilter.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import * as Ariakit from "@ariakit/react";
import { SelectTrigger } from "~/components/primitives/Select";
import { useSearchParams } from "~/hooks/useSearchParam";
import { appliedSummary, FilterMenuProvider } from "~/components/runs/v3/SharedFilters";
import { filterIcon, VersionsDropdown } from "~/components/runs/v3/RunFilters";
import { AppliedFilter } from "~/components/primitives/AppliedFilter";

const shortcut = { key: "v" };

export function LogsVersionFilter() {
const { values, del } = useSearchParams();
const selectedVersions = values("versions");

if (selectedVersions.length === 0 || selectedVersions.every((v) => v === "")) {
return (
<FilterMenuProvider>
{(search, setSearch) => (
<VersionsDropdown
trigger={
<SelectTrigger
icon={filterIcon("versions")}
variant="secondary/small"
shortcut={shortcut}
tooltipTitle="Filter by version"
>
<span className="ml-0.5">Versions</span>
</SelectTrigger>
}
searchValue={search}
clearSearchValue={() => setSearch("")}
/>
)}
</FilterMenuProvider>
);
}

return (
<FilterMenuProvider>
{(search, setSearch) => (
<VersionsDropdown
trigger={
<Ariakit.Select render={<div className="group cursor-pointer focus-custom" />}>
<AppliedFilter
label="Versions"
icon={filterIcon("versions")}
value={appliedSummary(selectedVersions)}
onRemove={() => del(["versions", "cursor", "direction"])}
variant="secondary/small"
/>
</Ariakit.Select>
}
searchValue={search}
clearSearchValue={() => setSearch("")}
/>
)}
</FilterMenuProvider>
);
}
7 changes: 7 additions & 0 deletions apps/webapp/app/components/primitives/charts/ChartRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export type ChartRootProps = {
onViewAllLegendItems?: () => void;
/** When true, constrains legend to max 50% height with scrolling */
legendScrollable?: boolean;
/** Additional className for the legend */
legendClassName?: string;
/** When true, chart fills its parent container height and distributes space between chart and legend */
fillContainer?: boolean;
/** Content rendered between the chart and the legend */
Expand Down Expand Up @@ -87,6 +89,7 @@ export function ChartRoot({
legendValueFormatter,
onViewAllLegendItems,
legendScrollable = false,
legendClassName,
fillContainer = false,
beforeLegend,
children,
Expand Down Expand Up @@ -114,6 +117,7 @@ export function ChartRoot({
legendValueFormatter={legendValueFormatter}
onViewAllLegendItems={onViewAllLegendItems}
legendScrollable={legendScrollable}
legendClassName={legendClassName}
fillContainer={fillContainer}
beforeLegend={beforeLegend}
>
Expand All @@ -133,6 +137,7 @@ type ChartRootInnerProps = {
legendValueFormatter?: (value: number) => string;
onViewAllLegendItems?: () => void;
legendScrollable?: boolean;
legendClassName?: string;
fillContainer?: boolean;
beforeLegend?: React.ReactNode;
children: React.ComponentProps<typeof RechartsPrimitive.ResponsiveContainer>["children"];
Expand All @@ -148,6 +153,7 @@ function ChartRootInner({
legendValueFormatter,
onViewAllLegendItems,
legendScrollable = false,
legendClassName,
fillContainer = false,
beforeLegend,
children,
Expand Down Expand Up @@ -193,6 +199,7 @@ function ChartRootInner({
valueFormatter={legendValueFormatter}
onViewAllLegendItems={onViewAllLegendItems}
scrollable={legendScrollable}
className={legendClassName}
/>
)}
</div>
Expand Down
2 changes: 1 addition & 1 deletion apps/webapp/app/components/runs/v3/RunFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1216,7 +1216,7 @@ function AppliedMachinesFilter() {
);
}

function VersionsDropdown({
export function VersionsDropdown({
trigger,
clearSearchValue,
searchValue,
Expand Down
52 changes: 38 additions & 14 deletions apps/webapp/app/presenters/v3/ErrorGroupPresenter.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export type ErrorGroupOptions = {
userId?: string;
projectId: string;
fingerprint: string;
versions?: string[];
runsPageSize?: number;
period?: string;
from?: number;
Expand All @@ -39,6 +40,7 @@ export const ErrorGroupOptionsSchema = z.object({
userId: z.string().optional(),
projectId: z.string(),
fingerprint: z.string(),
versions: z.array(z.string()).optional(),
runsPageSize: z.number().int().positive().max(1000).optional(),
period: z.string().optional(),
from: z.number().int().nonnegative().optional(),
Expand Down Expand Up @@ -72,6 +74,7 @@ export type ErrorGroupSummary = {

export type ErrorGroupOccurrences = Awaited<ReturnType<ErrorGroupPresenter["getOccurrences"]>>;
export type ErrorGroupActivity = ErrorGroupOccurrences["data"];
export type ErrorGroupActivityVersions = ErrorGroupOccurrences["versions"];

export class ErrorGroupPresenter extends BasePresenter {
constructor(
Expand All @@ -89,6 +92,7 @@ export class ErrorGroupPresenter extends BasePresenter {
userId,
projectId,
fingerprint,
versions,
runsPageSize = DEFAULT_RUNS_PAGE_SIZE,
period,
from,
Expand Down Expand Up @@ -117,6 +121,7 @@ export class ErrorGroupPresenter extends BasePresenter {
userId,
projectId,
fingerprint,
versions,
pageSize: runsPageSize,
from: time.from.getTime(),
to: time.to.getTime(),
Expand All @@ -140,23 +145,26 @@ export class ErrorGroupPresenter extends BasePresenter {
}

/**
* Returns bucketed occurrence counts for a single fingerprint over a time range.
* Granularity is determined automatically from the range span.
* Returns bucketed occurrence counts for a single fingerprint over a time range,
* grouped by task_version for stacked charts.
*/
public async getOccurrences(
organizationId: string,
projectId: string,
environmentId: string,
fingerprint: string,
from: Date,
to: Date
to: Date,
versions?: string[]
): Promise<{
data: Array<{ date: Date; count: number }>;
data: Array<Record<string, number | Date>>;
versions: string[];
}> {
const granularityMs = errorGroupGranularity.getTimeGranularityMs(from, to);
const intervalExpr = msToClickHouseInterval(granularityMs);

const queryBuilder = this.logsClickhouse.errors.createOccurrencesQueryBuilder(intervalExpr);
const queryBuilder =
this.logsClickhouse.errors.createOccurrencesByVersionQueryBuilder(intervalExpr);

queryBuilder.where("organization_id = {organizationId: String}", { organizationId });
queryBuilder.where("project_id = {projectId: String}", { projectId });
Expand All @@ -169,7 +177,11 @@ export class ErrorGroupPresenter extends BasePresenter {
toTimeMs: to.getTime(),
});

queryBuilder.groupBy("error_fingerprint, bucket_epoch");
if (versions && versions.length > 0) {
queryBuilder.where("task_version IN {versions: Array(String)}", { versions });
}

queryBuilder.groupBy("error_fingerprint, task_version, bucket_epoch");
queryBuilder.orderBy("bucket_epoch ASC");

const [queryError, records] = await queryBuilder.execute();
Expand All @@ -186,17 +198,27 @@ export class ErrorGroupPresenter extends BasePresenter {
buckets.push(epoch);
}

const byBucket = new Map<number, number>();
// Collect distinct versions and index results by (epoch, version)
const versionSet = new Set<string>();
const byBucketVersion = new Map<string, number>();
for (const row of records ?? []) {
byBucket.set(row.bucket_epoch, (byBucket.get(row.bucket_epoch) ?? 0) + row.count);
const version = row.task_version || "unknown";
versionSet.add(version);
const key = `${row.bucket_epoch}:${version}`;
byBucketVersion.set(key, (byBucketVersion.get(key) ?? 0) + row.count);
}

return {
data: buckets.map((epoch) => ({
date: new Date(epoch * 1000),
count: byBucket.get(epoch) ?? 0,
})),
};
const sortedVersions = sortVersionsDescending([...versionSet]);

const data = buckets.map((epoch) => {
const point: Record<string, number | Date> = { date: new Date(epoch * 1000) };
for (const version of sortedVersions) {
point[version] = byBucketVersion.get(`${epoch}:${version}`) ?? 0;
}
return point;
});
Comment on lines +213 to +219
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Version string used as Record key can collide with reserved date key, breaking the chart

In getOccurrences, version strings are used as dynamic keys in the same Record<string, number | Date> that already has a date key (apps/webapp/app/presenters/v3/ErrorGroupPresenter.server.ts:214-217). If a task_version value is literally "date" (or empty-string mapped to "unknown" which is safe, but "date" is not guarded), the version count (a number) overwrites the date field (a Date). Downstream in ActivityChart (apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx:428), d.date instanceof Date would be false and new Date(smallNumber).getTime() would produce a nonsensical timestamp (epoch 1970), completely breaking the chart's x-axis. The same collision can occur with "__timestamp" added at route.tsx:427.

Prompt for agents
In apps/webapp/app/presenters/v3/ErrorGroupPresenter.server.ts, the getOccurrences method (lines 213-219) uses version strings as keys in a Record that also has a "date" key. To prevent collisions, either:

1. Prefix version keys with a safe namespace (e.g., "v:" prefix) and update the chart consumer to strip the prefix when displaying labels, OR
2. Restructure the data to use a separate nested object for version counts, e.g.:
   { date: Date, versions: Record<string, number> }
   and update the ChartRoot consumer in the fingerprint route (route.tsx lines 475-496) to match.

Also in apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx line 427, the __timestamp key added via spread could similarly collide if a version were named "__timestamp". Guard against both reserved keys ("date" and "__timestamp").
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


return { data, versions: sortedVersions };
}

private async getSummary(
Expand Down Expand Up @@ -275,6 +297,7 @@ export class ErrorGroupPresenter extends BasePresenter {
userId?: string;
projectId: string;
fingerprint: string;
versions?: string[];
pageSize: number;
from?: number;
to?: number;
Expand All @@ -289,6 +312,7 @@ export class ErrorGroupPresenter extends BasePresenter {
projectId: options.projectId,
rootOnly: false,
errorId: ErrorId.toFriendlyId(options.fingerprint),
versions: options.versions,
pageSize: options.pageSize,
from: options.from,
to: options.to,
Expand Down
9 changes: 9 additions & 0 deletions apps/webapp/app/presenters/v3/ErrorsListPresenter.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export type ErrorsListOptions = {
projectId: string;
// filters
tasks?: string[];
versions?: string[];
period?: string;
from?: number;
to?: number;
Expand All @@ -39,6 +40,7 @@ export const ErrorsListOptionsSchema = z.object({
userId: z.string().optional(),
projectId: z.string(),
tasks: z.array(z.string()).optional(),
versions: z.array(z.string()).optional(),
period: z.string().optional(),
from: z.number().int().nonnegative().optional(),
to: z.number().int().nonnegative().optional(),
Expand Down Expand Up @@ -123,6 +125,7 @@ export class ErrorsListPresenter extends BasePresenter {
userId,
projectId,
tasks,
versions,
period,
search,
from,
Expand Down Expand Up @@ -156,6 +159,7 @@ export class ErrorsListPresenter extends BasePresenter {

const hasFilters =
(tasks !== undefined && tasks.length > 0) ||
(versions !== undefined && versions.length > 0) ||
(search !== undefined && search !== "") ||
!time.isDefault;

Expand Down Expand Up @@ -189,6 +193,10 @@ export class ErrorsListPresenter extends BasePresenter {
queryBuilder.where("task_identifier IN {tasks: Array(String)}", { tasks });
}

if (versions && versions.length > 0) {
queryBuilder.where("task_version IN {versions: Array(String)}", { versions });
}

queryBuilder.groupBy("error_fingerprint, task_identifier");

// Text search via HAVING (operates on aggregated values)
Expand Down Expand Up @@ -282,6 +290,7 @@ export class ErrorsListPresenter extends BasePresenter {
},
filters: {
tasks,
versions,
search,
period: time,
from: effectiveFrom,
Expand Down
Loading
Loading