fix: correct tag-base search for dag-runs impl by yohamta0 · Pull Request #1586 · dagucloud/dagu

yohamta0 · 2026-01-17T05:39:19Z

Summary by CodeRabbit

New Features
- Added tags support for DAG runs—view and filter DAG runs by tags using AND logic matching.
- Introduced dedicated log download endpoints for DAG runs, steps, and sub-DAG runs.
Bug Fixes
- Improved command parsing validation with clearer error messages for invalid command array formats.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-17T05:39:35Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

This PR adds tag-based filtering for DAG runs throughout the system. It extends DAG run data models with an optional Tags field, implements server-side tag filtering in the store layer, propagates DAG-level tags into run statuses, and removes redundant client-side filtering logic. Additionally, command parsing is enhanced to support single-key maps formatted as "key: value" pairs.

Changes

Cohort / File(s)	Summary
API Generation & Schema `api/v2/api.gen.go`, `api/v2/api.yaml`	Added `Tags *[]string` field to `DAGRunDetails` and `DAGRunSummary` models; updated embedded OpenAPI specification to reflect new tags field for DAG run categorization and filtering.
Core Data Structures `internal/core/exec/runstatus.go`	Added `Tags []string` field to `DAGRunStatus` struct and initialized it from `dag.Tags` in `InitialStatus`.
Command Parsing Enhancement `internal/core/spec/step.go`, `internal/core/spec/step_test.go`	Enhanced `buildMultipleCommands` to accept single-key maps formatted as "key: value"; added validation error for multi-key maps and non-primitive values; test updated to rename "RejectsMapType" → "AcceptsSingleKeyMap" and added "RejectsMultiKeyMap" case.
Data Migration `internal/cmd/migrator.go`	Added `Tags []string` and `Preconditions []*core.Condition` fields to `DAGRunStatus` in migration; conditional population of Tags and Preconditions from DAG.
Storage Layer `internal/persis/filedagrun/store.go`, `internal/persis/filedagrun/store_test.go`	Implemented tag-based filtering in `collectStatusesFromRoots` using AND logic; added test case verifying filtering by single/multiple tags and non-existent tags.
Test Setup `internal/persis/filedagrun/setup_test.go`	Introduced `CreateAttemptWithDAG` helper method; refactored `CreateAttempt` to delegate to new helper.
Service Handler `internal/service/coordinator/handler.go`	Extended `writeInitialStatus` signature with `tags []string` parameter; updated call sites in `createAttemptForTask` and `createSubAttemptForTask` to pass `dag.Tags`.
Frontend API `internal/service/frontend/api/v2/dagruns.go`	Removed client-side tag filtering logic (`extractTagsFilter`, `buildAllowedDAGNames`, `hasAllTags` functions); API now calls store directly without per-DAG filtering; removed unused `math` import.
API Transformer `internal/service/frontend/api/v2/transformer.go`	Added `Tags: &s.Tags` population in `DAGRunSummary` and `DAGRunDetails` construction.
UI Schema `ui/src/api/v2/schema.ts`	Added four new log download endpoints: `/dag-runs/{name}/{dagRunId}/log/download`, `/dag-runs/{name}/{dagRunId}/steps/{stepName}/log/download`, `/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}/log/download`, `/dag-runs/{name}/{dagRunId}/sub-dag-runs/{subDAGRunId}/steps/{stepName}/log/download`; extended `DAGRunSummary` schema with optional `tags` array.
Integration Test `internal/intg/mltcmd_test.go`	Added test case "CommandWithColonInArray" to validate command execution with colon in echoed string.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

feat: enhance stability of distributed execution #1580: Modifies internal/service/coordinator/handler.go and updates writeInitialStatus with similar tag propagation and call-site changes in createAttemptForTask/createSubAttemptForTask.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix: correct tag-base search for dag-runs impl' accurately describes the main focus on tag-based filtering/search functionality for DAG runs, though it contains a minor typo ('tag-base' should be 'tag-based') and is abbreviated ('impl' instead of 'implementation').

_{✏️ 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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@internal/core/spec/step.go`:
- Around line 861-875: Update the error strings passed to
core.NewValidationError in the command-array parsing block so they don't end
with a period (to satisfy ST1005): change the literal in the nested case where
fmt.Errorf("command array elements must be strings. If this contains a colon,
wrap it in quotes. Got nested %T", v2) and the non-nested case
fmt.Errorf("command array elements must be strings. If this contains a colon,
wrap it in quotes.") to remove or replace the trailing period/terminal
punctuation (e.g., drop the final period or use a semicolon) while leaving the
rest of the messages and the core.NewValidationError(fmt.Sprintf("command[%d]",
i), v, ...) calls unchanged.

🧹 Nitpick comments (1)

internal/service/frontend/api/v2/transformer.go (1)
134-151: Consider omitempty behavior for Tags pointers.

Using &s.Tags always yields a non-nil pointer, so omitempty won’t omit the field when tags are absent. If you want the field omitted when there are no tags, gate the pointer on len(s.Tags) > 0.
♻️ Suggested change
 func toDAGRunSummary(s exec.DAGRunStatus) api.DAGRunSummary {
+	var tags *[]string
+	if len(s.Tags) > 0 {
+		tags = &s.Tags
+	}
 	return api.DAGRunSummary{
 		RootDAGRunName:   s.Root.Name,
 		RootDAGRunId:     s.Root.ID,
@@
-		Tags:             &s.Tags,
+		Tags:             tags,
 	}
 }
 
 func toDAGRunDetails(s exec.DAGRunStatus) api.DAGRunDetails {
+	var tags *[]string
+	if len(s.Tags) > 0 {
+		tags = &s.Tags
+	}
 	preconditions := make([]api.Condition, len(s.Preconditions))
@@
-		Tags:             &s.Tags,
+		Tags:             tags,
 	}
 }
Also applies to: 154-184

codecov · 2026-01-17T06:29:34Z

Codecov Report

❌ Patch coverage is 80.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.78%. Comparing base (621444e) to head (4590844).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/core/spec/step.go	66.66%	6 Missing ⚠️
internal/service/coordinator/handler.go	0.00%	4 Missing ⚠️
internal/core/exec/runstatus.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1586      +/-   ##
==========================================
- Coverage   64.78%   64.78%   -0.01%     
==========================================
  Files         259      259              
  Lines       28833    28871      +38     
==========================================
+ Hits        18680    18703      +23     
- Misses       8480     8492      +12     
- Partials     1673     1676       +3

Files with missing lines	Coverage Δ
internal/cmd/migrator.go	`66.02% <100.00%> (+0.66%)`	⬆️
internal/persis/filedagrun/store.go	`64.35% <100.00%> (+1.67%)`	⬆️
internal/core/exec/runstatus.go	`0.00% <0.00%> (ø)`
internal/service/coordinator/handler.go	`53.46% <0.00%> (-0.10%)`	⬇️
internal/core/spec/step.go	`85.34% <66.66%> (-0.45%)`	⬇️

... and 4 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 621444e...4590844. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

yohamta0 added 5 commits January 17, 2026 14:01

fix: validation must pass multicommand

405e9bf

fix tag filtering for dag-runs

1b2c3d3

fix

0de13d4

fix: set tag to status for worker as well

d483c14

add test

1528754

yohamta0 changed the title ~~fix: correct tag-base search for dag-runs implgg~~ fix: correct tag-base search for dag-runs impl Jan 17, 2026

fix bug

51d9cd1

coderabbitai Bot reviewed Jan 17, 2026

View reviewed changes

Comment thread internal/core/spec/step.go

fix ui issue

4590844

yohamta0 merged commit c19a003 into main Jan 17, 2026
5 checks passed

yohamta0 deleted the fix-multcmd-vald branch January 17, 2026 06:25

This was referenced Jan 23, 2026

feat: add support for key=value tags #1604

Merged

refactor: polling to SSE for realtime update #1608

Merged

feat: Trigger visibility #1612

Merged

This was referenced Mar 6, 2026

feat: add file-based index for DAG and DAG run stores #1729

Merged

feat: add tags parameter to start and enqueue APIs #1730

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: correct tag-base search for dag-runs impl#1586

fix: correct tag-base search for dag-runs impl#1586
yohamta0 merged 7 commits into
mainfrom
fix-multcmd-vald

yohamta0 commented Jan 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 17, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jan 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yohamta0 commented Jan 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jan 17, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yohamta0 commented Jan 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 17, 2026 •

edited

Loading