Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughTag-based filtering is now available for DAG runs across the stack. The API, backend service layer, and frontend UI all support filtering DAG runs by multiple DAG tags using AND logic (results must match all specified tags). The feature includes a dropdown UI for tag selection, URL persistence, and real-time data synchronization. Changes
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant API
participant Service
participant Core
User->>Frontend: Select tags from dropdown
Frontend->>Frontend: Parse & store selected tags
Frontend->>API: GET /dag-runs?tags=tag1,tag2
API->>Service: ListDAGRuns(ctx, opts.WithTags(...))
Service->>Core: Fetch all DAGs
Core-->>Service: DAG list
Service->>Service: Filter DAGs matching ALL tags (case-insensitive)
Service->>Core: Fetch DAG run statuses
Core-->>Service: DAG run statuses
Service->>Service: Filter runs to only matched DAG names
Service-->>API: Filtered DAG run results
API-->>Frontend: DAG runs response
Frontend->>Frontend: Update UI with filtered results
Frontend->>User: Display filtered DAG runs & selected tags
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @internal/service/frontend/api/v2/dagruns.go:
- Around line 336-390: listDAGRuns currently calls a paginated dagStore.List via
a default 50-item paginator and extracts tags by applying options to a temp
struct which can drop other fields; fix by (1) fetching all DAGs when tagsFilter
is used: call a.dagStore.List in a loop (or use a paginator that returns all
pages) and accumulate result.Items into a single slice before applying
hasAllTags so you don’t miss DAGs beyond the first page (use the same
exec.ListDAGsOptions but iterate pages until no more results), and (2) preserve
other ListDAGRunStatusesOption effects while removing only Tags for the store
query: for each opt used to detect tags in listDAGRuns, apply it to a copy of
exec.ListDAGRunStatusesOptions to read temp.Tags, collect tags into tagsFilter,
but when building filteredOpts append a wrapper option that applies the original
opt then clears Tags (so other fields remain) before calling
a.dagRunStore.ListStatuses.
In @ui/src/pages/dag-runs/index.tsx:
- Around line 266-272: When parsing the URL tags into urlFilters.tags, trim each
split token before adding it: take the result of params.get('tags') (tagsParam),
split on ',', map each token through .trim(), then filter out empty strings and
assign that array to urlFilters.tags; update the block that references params,
tagsParam, and urlFilters.tags so the filtering uses the trimmed value rather
than only checking t.trim() without storing the trimmed token.
🧹 Nitpick comments (6)
internal/service/frontend/api/v2/dagruns.go (2)
280-293: Consider normalizing/deduping incomingtagsCurrent parsing is fine, but you may want to
strings.ToLower+ dedupe inListDAGRunsso the rest of the pipeline does less work and treatsfoo,Fooas one tag.
392-404: HardenhasAllTagsfor whitespace + reduce allocationsConsider
strings.TrimSpaceondagTagstoo (defensive against messy stored tags), and pre-size the map:make(map[string]struct{}, len(dagTags)).api/v2/api.gen.go (1)
1459-1464: Tags filter looks correctly plumbed; consider modeling as an array in OpenAPI if you want repeated query params.At Line 1462-1463,
Tags *stringmatches the “comma-separated” contract and keeps parsing responsibility in the handler/service layer. If clients might send?tags=a&tags=b, consider changing the OpenAPI schema to anarray[string](with the desiredexplodebehavior) so generated bindings/types guide clients correctly.ui/src/pages/dag-runs/index.tsx (3)
808-808: Use compact button height per coding guidelines.The button uses
h-9but the coding guidelines specifyh-7 or h-8for buttons in form elements.Suggested fix
- <Button variant="outline" className="h-9 px-3"> + <Button variant="outline" className="h-8 px-3">
846-855: Consider using a regular menu item for the "Clear all" action.
DropdownMenuCheckboxItemis semantically intended for toggleable items with checked state. For an action like "Clear all", a regularDropdownMenuItemwould be more appropriate and wouldn't reserve space for a checkbox indicator.This is a minor refinement - the current implementation is functional.
864-872: Consider adding keyboard accessibility to dismissible tag badges.The tag badges are clickable for removal but lack keyboard event handlers. For full accessibility, add
tabIndex={0}andonKeyDownto handle Enter/Space keys.Suggested enhancement
<Badge key={tag} variant="secondary" className="text-xs cursor-pointer hover:bg-destructive/20" onClick={() => handleTagToggle(tag)} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleTagToggle(tag); + } + }} + role="button" + aria-label={`Remove tag ${tag}`} >Based on coding guidelines: "Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels."
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/v2/api.gen.goapi/v2/api.yamlinternal/core/exec/dagrun.gointernal/service/frontend/api/v2/dagruns.goui/src/api/v2/schema.tsui/src/pages/dag-runs/index.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/core/exec/dagrun.gointernal/service/frontend/api/v2/dagruns.goapi/v2/api.gen.go
ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{ts,tsx,jsx,js}: Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Support both light and dark modes for all UI components using Tailwind CSS class pairs likedark:bg-slate-700
NEVER use full-page loading overlays or LoadingIndicator components that hide content - show stale data while fetching updates instead
Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text withwhitespace-normal break-words
Use consistent metadata styling withbg-slate-200 dark:bg-slate-700backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, account for fixed elements when setting heights
Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels
Files:
ui/src/api/v2/schema.tsui/src/pages/dag-runs/index.tsx
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/api/v2/schema.tsui/src/pages/dag-runs/index.tsx
{api/v1,api/v2}/**/*.{proto,yaml,yml,json}
📄 CodeRabbit inference engine (AGENTS.md)
API definitions live in
api/v1andapi/v2; generated server stubs land ininternal/service, while matching TypeScript clients flow intoui/src/api
Files:
api/v2/api.yaml
🧠 Learnings (2)
📚 Learning: 2025-12-04T10:34:01.045Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: ui/CLAUDE.md:0-0
Timestamp: 2025-12-04T10:34:01.045Z
Learning: Applies to ui/**/*.{ts,tsx,jsx,js} : Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Applied to files:
ui/src/pages/dag-runs/index.tsx
📚 Learning: 2025-12-04T10:34:17.062Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.062Z
Learning: Applies to ui/**/*.{ts,tsx} : UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (`JobList.tsx`) and hooks with `use*` (`useJobs.ts`)
Applied to files:
ui/src/pages/dag-runs/index.tsx
🧬 Code graph analysis (2)
api/v2/api.gen.go (1)
api/v1/api.gen.go (1)
InvalidParamFormatError(1166-1169)
ui/src/pages/dag-runs/index.tsx (4)
ui/src/hooks/api.ts (1)
useQuery(28-28)ui/src/components/ui/dropdown-menu.tsx (6)
DropdownMenu(182-182)DropdownMenuTrigger(183-183)DropdownMenuContent(184-184)DropdownMenuLabel(188-188)DropdownMenuSeparator(189-189)DropdownMenuCheckboxItem(186-186)ui/src/components/ui/button.tsx (1)
Button(57-57)ui/src/components/ui/badge.tsx (1)
Badge(46-46)
⏰ 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). (2)
- GitHub Check: Build
- GitHub Check: Test on ubuntu-latest
🔇 Additional comments (13)
internal/core/exec/dagrun.go (2)
63-63: LGTM!The new
Tagsfilter field follows the established pattern of other filter fields in the struct, and the inline comment clearly documents the AND logic semantics.
111-116: LGTM!The
WithTagsfunctional option follows the idiomatic pattern established by other options in this file (WithStatuses,WithName, etc.).api/v2/api.yaml (1)
1476-1481: LGTM!The
tagsquery parameter is well-documented with clear semantics (comma-separated, AND logic). The implementation aligns with the backendWithTagsfilter and follows the existing parameter patterns in the API spec.ui/src/api/v2/schema.ts (1)
3876-3878: Keeptagsquery param generation as the single source of truthSince this file is auto-generated, ensure the
tags?: stringaddition is coming from the OpenAPI spec (and regeneration is enforced), rather than being hand-edited, to avoid drift.api/v2/api.gen.go (2)
3227-3234: Query binding fortagsis consistent with other optional params.At Line 3229-3233, binding into
params.TagswithInvalidParamFormatErroron failure matches the existing pattern forstatus/fromDate/.../name.
11795-12065: swaggerSpec regen looks fine (expected with OpenAPI changes).The large
swaggerSpecblob update is consistent with adding a new query parameter in the OpenAPI document; nothing to flag here beyond ensuring it was regenerated via the normal pipeline/tooling.ui/src/pages/dag-runs/index.tsx (7)
2-16: LGTM!The imports are appropriate for the new tag filtering feature - Badge for count display, DropdownMenu components for the multi-select UI, and lucide icons for visual elements.
50-55: LGTM!The
areTagsEqualhelper correctly compares tag arrays regardless of order using sorting and element-wise comparison.
159-175: LGTM!The dual state pattern for
selectedTagsandapiTagsfollows the established convention in this file (matchingsearchText/apiSearchText, etc.), maintaining separation between UI state and API query state.
367-382: LGTM!The tags fetch correctly uses conservative revalidation settings since available tags change infrequently. The fallback to an empty array handles the loading state gracefully.
467-482: LGTM!The tag handlers correctly implement immediate filtering (like status changes) by updating both UI and API state simultaneously and triggering a refetch. This provides a responsive user experience.
394-394: LGTM!The tags are correctly formatted as a comma-separated string for the API, matching the backend's expected format per the OpenAPI spec changes.
431-439: LGTM!The
handleSearchfunction correctly includes tag state synchronization, following the same pattern as other filters.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1576 +/- ##
==========================================
- Coverage 64.86% 64.84% -0.02%
==========================================
Files 255 255
Lines 28427 28430 +3
==========================================
- Hits 18438 18436 -2
- Misses 8342 8348 +6
+ Partials 1647 1646 -1
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
1 participant
Resolves #1494
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.