-
Notifications
You must be signed in to change notification settings - Fork 456
feat: add --evals flag to logs and audit commands to filter by evals results #45373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
64c9b81
ac429a8
63ca431
04309d3
53a7bb7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ type AuditOptions struct { | |
| ArtifactSets []string | ||
| ExperimentFilter string | ||
| VariantFilter string | ||
| EvalsOnly bool | ||
| } | ||
|
|
||
| var auditCommandLong = `Audit one or more workflow runs by downloading artifacts and logs, detecting errors, | ||
|
|
@@ -82,6 +83,7 @@ type auditCommandOptions struct { | |
| stdin bool | ||
| experimentFilter string | ||
| variantFilter string | ||
| evalsOnly bool | ||
| } | ||
|
|
||
| // NewAuditCommand creates the audit command | ||
|
|
@@ -109,6 +111,7 @@ func registerAuditCommandFlags(cmd *cobra.Command) { | |
| cmd.Flags().Bool("stdin", false, "Read workflow run IDs or URLs from stdin (one per line) instead of positional arguments") | ||
| cmd.Flags().String("experiment", "", "Filter to runs that include this experiment name") | ||
| cmd.Flags().String("variant", "", "Filter to runs with a specific variant value (requires --experiment)") | ||
| cmd.Flags().Bool("evals", false, "Skip runs that do not contain evals results (evals.jsonl); automatically downloads the evals artifact when --artifacts is narrowed") | ||
| RegisterDirFlagCompletion(cmd, "output") | ||
| } | ||
|
|
||
|
|
@@ -124,6 +127,12 @@ func runAuditCommand(cmd *cobra.Command, args []string) error { | |
| if len(args) == 1 { | ||
| return runAuditSingle(cmd.Context(), args[0], opts) | ||
| } | ||
| if opts.evalsOnly { | ||
| return errors.New(console.FormatErrorWithSuggestions( | ||
| "--evals is not supported in multi-run diff mode", | ||
| []string{"Provide a single run ID with --evals to filter by evals results"}, | ||
| )) | ||
| } | ||
| return runAuditMulti(cmd.Context(), args, opts.repoFlag, opts.outputDir, opts.verbose, opts.jsonOutput, opts.format, opts.artifacts) | ||
| } | ||
|
|
||
|
|
@@ -139,12 +148,21 @@ func getAuditCommandOptions(cmd *cobra.Command) (auditCommandOptions, error) { | |
| opts.stdin, _ = cmd.Flags().GetBool("stdin") | ||
| opts.experimentFilter, _ = cmd.Flags().GetString("experiment") | ||
| opts.variantFilter, _ = cmd.Flags().GetString("variant") | ||
| opts.evalsOnly, _ = cmd.Flags().GetBool("evals") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 04309d3 — |
||
| if opts.variantFilter != "" && opts.experimentFilter == "" { | ||
| return auditCommandOptions{}, errors.New(console.FormatErrorWithSuggestions( | ||
| "--variant requires --experiment to be specified", | ||
| []string{"Add --experiment <name> to filter by experiment name alongside --variant"}, | ||
| )) | ||
| } | ||
| // Auto-include the evals artifact when --evals is specified and the user has | ||
| // narrowed the artifact set (non-empty --artifacts). When --artifacts is empty | ||
| // the default is "all", which already includes every artifact including evals, | ||
| // so we must not append here: doing so would change the default from "all" to | ||
| // "evals-only" and omit the activation/agent artifacts required for a full report. | ||
| if len(opts.artifacts) > 0 { | ||
| opts.artifacts = applyEvalsArtifact(opts.artifacts, opts.evalsOnly) | ||
| } | ||
| return opts, nil | ||
| } | ||
|
|
||
|
|
@@ -199,6 +217,7 @@ func runAuditSingle(ctx context.Context, runIDOrURL string, opts auditCommandOpt | |
| ArtifactSets: opts.artifacts, | ||
| ExperimentFilter: opts.experimentFilter, | ||
| VariantFilter: opts.variantFilter, | ||
| EvalsOnly: opts.evalsOnly, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -302,6 +321,7 @@ type auditRunConfig struct { | |
| artifactFilter []string | ||
| experimentFilter string | ||
| variantFilter string | ||
| evalsOnly bool | ||
| } | ||
|
|
||
| type auditAnalysisResults struct { | ||
|
|
@@ -355,6 +375,9 @@ func AuditWorkflowRun(ctx context.Context, runID int64, opts AuditOptions) error | |
| if shouldSkipAuditRun(cfg.runID, cfg.outputDir, cfg.experimentFilter, cfg.variantFilter) { | ||
| return nil | ||
| } | ||
| if shouldSkipForEvals(cfg) { | ||
| return nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] The evals-skip block (lines 373-377) is copy-pasted verbatim into 💡 SuggestionExtract a small helper used by both: // shouldSkipForEvals returns true and logs when no evals results are present.
func shouldSkipForEvals(cfg auditRunConfig) bool {
if !cfg.evalsOnly {
return false
}
if runHasEvals(cfg.outputDir, cfg.verbose) {
return false
}
auditLog.Printf("Skipping run %d: no evals results found (filtered by --evals)", cfg.runID)
fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(
fmt.Sprintf("Skipping run %d: workflow does not have evals results (filtered by --evals)", cfg.runID)))
return true
}@copilot please address this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 53a7bb7 — extracted |
||
| } | ||
| return renderAuditReport(ctx, processedRun, results.metrics, results.mcpToolUsage, cfg.auditOptions()) | ||
| } | ||
|
|
||
|
|
@@ -376,6 +399,7 @@ func newAuditRunConfig(runID int64, opts AuditOptions) (auditRunConfig, error) { | |
| artifactFilter: ResolveArtifactFilter(opts.ArtifactSets), | ||
| experimentFilter: opts.ExperimentFilter, | ||
| variantFilter: opts.VariantFilter, | ||
| evalsOnly: opts.EvalsOnly, | ||
| }, nil | ||
| } | ||
|
|
||
|
|
@@ -454,6 +478,7 @@ func (cfg auditRunConfig) auditOptions() AuditOptions { | |
| Verbose: cfg.verbose, | ||
| Parse: cfg.parse, | ||
| JSONOutput: cfg.jsonOutput, | ||
| EvalsOnly: cfg.evalsOnly, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -469,6 +494,14 @@ func renderCachedAuditIfAvailable(ctx context.Context, cfg auditRunConfig) (bool | |
| if shouldSkipAuditRun(cfg.runID, cfg.outputDir, cfg.experimentFilter, cfg.variantFilter) { | ||
| return true, nil | ||
| } | ||
| // When --evals is set but the evals artifact is not present locally, the cache | ||
| // was created without evals (e.g., default usage-only download). Bypass the | ||
| // cache so prepareAuditWorkflowRun can fetch the evals artifact; the filter at | ||
| // the post-download check will then correctly decide whether to skip the run. | ||
| if cfg.evalsOnly && !runHasEvals(cfg.outputDir, cfg.verbose) { | ||
| auditLog.Printf("Cache miss for run %d evals: evals artifact not present locally, bypassing cache", cfg.runID) | ||
| return false, nil | ||
| } | ||
| processedRun := processedRunFromSummary(summary, cfg.outputDir) | ||
| return true, renderAuditReport(ctx, processedRun, summary.Metrics, summary.MCPToolUsage, cfg.auditOptions()) | ||
| } | ||
|
|
@@ -931,3 +964,20 @@ func renderAuditCompletion(runOutputDir string, jsonOutput bool) { | |
| fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Audit complete. Logs saved to "+absOutputDir)) | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Tip: use --artifacts to select specific artifact sets (agent, firewall, mcp, activation, detection, etc.)")) | ||
| } | ||
|
|
||
| // shouldSkipForEvals returns true when evals filtering is active but no evals results | ||
| // are found locally after download. It logs the skip decision and, when verbose, prints | ||
| // an info message to stderr. Call this only after artifact download has completed. | ||
| func shouldSkipForEvals(cfg auditRunConfig) bool { | ||
| if !cfg.evalsOnly { | ||
| return false | ||
| } | ||
| if runHasEvals(cfg.outputDir, cfg.verbose) { | ||
| return false | ||
| } | ||
| auditLog.Printf("Skipping run %d: no evals results found (filtered by --evals)", cfg.runID) | ||
| if cfg.verbose { | ||
| fmt.Fprintf(os.Stderr, "%s\n", console.FormatInfoMessage(fmt.Sprintf("Skipping run %d: workflow does not have evals results (filtered by --evals)", cfg.runID))) | ||
| } | ||
| return true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| ` + string(constants.CLIExtensionPrefix) + ` logs --ref main # Filter logs by branch or tag | ||
| ` + string(constants.CLIExtensionPrefix) + ` logs --ref feature-xyz # Filter logs by feature branch | ||
| ` + string(constants.CLIExtensionPrefix) + ` logs --filtered-integrity # Filter logs containing items that were filtered by gateway integrity checks | ||
| ` + string(constants.CLIExtensionPrefix) + ` logs --evals # Filter logs from workflows with evals results | ||
| ` + string(constants.CLIExtensionPrefix) + ` logs --exclude-staged # Exclude staged workflow runs from results | ||
|
|
||
| # Run ID range filtering | ||
|
|
@@ -152,6 +153,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| summaryFile, _ := cmd.Flags().GetString("summary-file") | ||
| safeOutputType, _ := cmd.Flags().GetString("safe-output") | ||
| filteredIntegrity, _ := cmd.Flags().GetBool("filtered-integrity") | ||
| evalsOnly, _ := cmd.Flags().GetBool("evals") | ||
| train, _ := cmd.Flags().GetBool("train") | ||
| format, _ := cmd.Flags().GetString("format") | ||
| reportFile, _ := cmd.Flags().GetString("report-file") | ||
|
|
@@ -170,6 +172,8 @@ Downloaded artifacts include (when using --artifacts all): | |
| return err | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/codebase-design] The 💡 SuggestionExtract a small helper or move the auto-include into func applyEvalsArtifact(artifacts []string, evalsOnly bool) []string {
if evalsOnly && !slices.Contains(artifacts, string(ArtifactSetEvals)) && !slices.Contains(artifacts, string(ArtifactSetAll)) {
return append(artifacts, string(ArtifactSetEvals))
}
return artifacts
}Then call @copilot please address this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 53a7bb7 — added |
||
| } | ||
|
|
||
| artifacts = applyEvalsArtifact(artifacts, evalsOnly) | ||
|
|
||
| return DownloadWorkflowLogsFromStdin(cmd.Context(), StdinLogsOptions{ | ||
| RunURLs: runURLs, | ||
| OutputDir: outputDir, | ||
|
|
@@ -186,6 +190,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| SummaryFile: summaryFile, | ||
| SafeOutputType: safeOutputType, | ||
| FilteredIntegrity: filteredIntegrity, | ||
| EvalsOnly: evalsOnly, | ||
| Train: train, | ||
| Format: format, | ||
| ReportFile: reportFile, | ||
|
|
@@ -273,6 +278,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| summaryFile, _ := cmd.Flags().GetString("summary-file") | ||
| safeOutputType, _ := cmd.Flags().GetString("safe-output") | ||
| filteredIntegrity, _ := cmd.Flags().GetBool("filtered-integrity") | ||
| evalsOnly, _ := cmd.Flags().GetBool("evals") | ||
| train, _ := cmd.Flags().GetBool("train") | ||
| format, _ := cmd.Flags().GetString("format") | ||
| reportFile, _ := cmd.Flags().GetString("report-file") | ||
|
|
@@ -321,6 +327,8 @@ Downloaded artifacts include (when using --artifacts all): | |
|
|
||
| logsCommandLog.Printf("Executing logs download: workflow=%s, count=%d, engine=%s, train=%v, cache_before=%s", workflowName, count, engine, train, cacheBefore) | ||
|
|
||
| artifacts = applyEvalsArtifact(artifacts, evalsOnly) | ||
|
|
||
| return DownloadWorkflowLogs(cmd.Context(), LogsDownloadOptions{ | ||
| WorkflowName: workflowName, | ||
| Count: count, | ||
|
|
@@ -343,6 +351,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| SummaryFile: summaryFile, | ||
| SafeOutputType: safeOutputType, | ||
| FilteredIntegrity: filteredIntegrity, | ||
| EvalsOnly: evalsOnly, | ||
| Train: train, | ||
| Format: format, | ||
| ReportFile: reportFile, | ||
|
|
@@ -368,6 +377,7 @@ Downloaded artifacts include (when using --artifacts all): | |
| logsCmd.Flags().Bool("no-firewall", false, "Filter to only runs without firewall enabled") | ||
| logsCmd.Flags().String("safe-output", "", "Filter to runs containing a specific safe output type (e.g., create-issue, missing-tool, missing-data, noop, report-incomplete)") | ||
| logsCmd.Flags().Bool("filtered-integrity", false, "Filter to runs containing items that were filtered by gateway integrity checks") | ||
| logsCmd.Flags().Bool("evals", false, "Filter to runs containing evals results (evals.jsonl); automatically includes the evals artifact") | ||
| logsCmd.Flags().Bool("parse", false, "Run JavaScript parsers on agent logs and firewall logs, writing Markdown to log.md and firewall.md") | ||
| addJSONFlag(logsCmd) | ||
| logsCmd.Flags().Int("timeout", 0, "Download timeout in minutes (0 = no timeout)") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ type runFilterOpts struct { | |
| noFirewall bool | ||
| safeOutputType string | ||
| filteredIntegrity bool | ||
| evalsOnly bool | ||
| } | ||
|
|
||
| var fetchJobStatusesForProcessedRun = fetchJobStatuses | ||
|
|
@@ -131,6 +132,17 @@ func applyRunFilters(result DownloadResult, opts runFilterOpts, verbose bool) bo | |
| } | ||
| } | ||
|
|
||
| // Apply evals filtering if --evals flag is specified. | ||
| if opts.evalsOnly { | ||
| if !runHasEvals(result.LogsPath, verbose) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 04309d3 — |
||
| logsOrchestratorLog.Printf("Skipping run %d: no evals results found, filtered by --evals", result.Run.DatabaseID) | ||
| if verbose { | ||
| fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skipping run %d: workflow does not have evals results (filtered by --evals)", result.Run.DatabaseID))) | ||
| } | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/grill-with-docs] The flag description says "Include evals results in audit report" but its primary effect is filtering (skipping runs without evals). This is more consistent with the
logs --evalsdescription ("Filter to runs containing evals results"). A misleading description can confuse users expecting the flag to add something to the report rather than exclude runs.@copilot please address this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 53a7bb7 — the audit
--evalsflag description now accurately says"Skip runs that do not contain evals results (evals.jsonl); automatically downloads the evals artifact when --artifacts is narrowed".