Conversation
matt-aitken
commented
Mar 5, 2026
matt-aitken
commented
- Added versions filtering on the Errors list and page
- Added errors stacked bars to the graph on the individual error page
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/**/*.{ts,tsx,js}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/webapp/**/*.server.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
🧠 Learnings (12)📚 Learning: 2026-03-02T12:42:56.114ZApplied to files:
📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
📚 Learning: 2026-02-06T19:53:38.843ZApplied to files:
📚 Learning: 2026-02-11T16:37:32.429ZApplied to files:
📚 Learning: 2025-12-08T15:19:56.823ZApplied to files:
📚 Learning: 2026-02-11T16:50:14.167ZApplied to files:
📚 Learning: 2025-07-12T18:00:06.163ZApplied to files:
📚 Learning: 2026-02-03T18:27:40.429ZApplied to files:
📚 Learning: 2026-03-02T12:42:41.110ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2026-01-28T14:15:15.011ZApplied to files:
📚 Learning: 2025-07-12T18:06:04.133ZApplied to files:
🔇 Additional comments (9)
WalkthroughThis PR adds version-based filtering and aggregation capabilities to the error tracking system. It introduces a new LogsVersionFilter React component for version selection in the UI, extends ErrorGroupPresenter and ErrorsListPresenter to accept and propagate version parameters through their APIs, updates ChartRoot to support optional legend styling, exports VersionsDropdown from RunFilters for reusability, and adds ClickHouse query builders (createErrorOccurrencesByVersionQueryBuilder and createOccurrencesByVersionQueryBuilder) to support per-version error aggregation. These changes integrate through route loaders, presenter calls, and database queries to enable filtering errors by task version with multi-version series support in activity charts. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 Errors list sparklines don't reflect the version filter
When a user filters errors by version on the list page (errors._index/route.tsx), the occurrencesPromise at line 120-134 chains from the filtered listPromise but calls presenter.getOccurrences() without passing the versions filter. This means the sparkline activity graphs shown in each error row will display occurrences across ALL versions, even when the list itself is filtered to specific versions. This may be intentional (showing full activity context) but could confuse users who expect the sparklines to match the filtered view.
(Refers to lines 120-134)
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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; | ||
| }); |
There was a problem hiding this comment.
🟡 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").
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
