Add daily grader audit workflow

[Copilot]

Adds a scheduled grader audit that finds workflow runs with defined or imported graders and publishes a downloadable 24-hour report. Also adds grader-aware log filtering so audits can target runs with grader output.

• CLI filtering

  • Adds gh aw logs --graders to filter downloaded workflow runs to those containing grader results.
  • Ensures grader-related artifacts are included when artifact filtering is active.

• MCP support

  • Adds a graders parameter to the privileged logs MCP tool.
  • Forwards MCP grader filtering to the CLI.

• Daily audit workflow

  • Adds .github/workflows/daily-grader-audit.md.
  • Generates a Markdown audit report with grader results summarized in tables.
  • Uploads the report artifact with 1-day retention.

• Coverage

  • Adds focused tests for CLI flag registration, artifact expansion, run filtering, and MCP argument forwarding.

Example:

gh aw logs --graders --count 100 --artifacts graders

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

[github-actions]

✅ Ponytail Reviewer completed successfully!

Generated by Ponytail Reviewer for #56359

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

Comment Memory

reviewed_at: 2026-08-27T00:00:00Z
review_event: REQUEST_CHANGES
top_themes:
  - graders filter false negatives
  - daily audit report accuracy depends on grader detection
files_reviewed:
  - .github/workflows/daily-grader-audit.md
  - pkg/cli/logs_artifact_set.go
  - pkg/cli/logs_command.go
  - pkg/cli/logs_orchestrator_download.go
  - pkg/cli/logs_orchestrator_filters.go
  - pkg/cli/logs_orchestrator_types.go
  - pkg/cli/mcp_tools_privileged.go
comment_count: 1

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 25 AIC · ⌖ 8.27 AIC · ⊞ 7K · ◷
_{Comment /review to run again}

[github-actions]

Request changes

--graders introduces a correctness regression in run filtering: it can silently exclude runs that actually have grader output, which makes both the CLI filter and the new daily audit workflow produce incomplete results.

Blocking theme

• grader-run detection is less robust than the existing evals path and currently has a false-negative failure mode
• the new scheduled audit depends on that filter, so the bug directly affects report accuracy

🔎 Code quality review by PR Code Quality Reviewer · pi · gpt54 · 25 AIC · ⌖ 8.27 AIC · ⊞ 7K
_{Comment /review to run again}

[github-actions]

The new --graders filter can silently drop valid runs because it only checks for grader_results.json under the downloaded artifacts and never attempts the same branch fallback that --evals uses, so any run where the usage artifact omitted grader results will be misreported as “no grader results” even though the workflow did produce them.

💡 Why this blocks merge

skipByGradersFilter calls runHasGraders(result.LogsPath) and immediately filters the run out when that file is absent. Unlike the evals path just above it, there is no recovery path analogous to ensureEvalsResultsFromBranch(...).

That means the correctness of gh aw logs --graders now depends on every historical run having grader artifacts mirrored into the downloaded bundle. If that mirroring regresses, if an older run predates it, or if only the fallback artifact contains the data, this filter returns false negatives and the new daily audit workflow will undercount or completely miss grader runs.

Please either add a grader-specific fallback with the same robustness as evals, or explicitly prove at the download layer that grader results are always materialized before filtering.

[github-actions]

Review: Daily Grader Audit Workflow

Modes applied: audit + critique (new feature)

The PR is well-structured. The workflow prompt is clear, Go changes follow existing patterns (applyEvalsArtifact → applyGradersArtifact), and test coverage is adequate. No blocking issues found.

Non-blocking observations

1. Missing test branches for applyGradersArtifact
The test suite doesn't cover:

• gradersOnly=false with a non-empty list → should return unchanged.
• gradersOnly=true with "all" already present → should not append "graders".

These mirror gaps that exist in applyEvalsArtifact tests too, so this is consistent with the existing codebase — but they are the exact branches guarding against duplicate injection.

2. graders: {} in frontmatter
The empty graders: {} map is presumably intentional (the audit workflow itself runs no deterministic graders). Confirm this is the correct idiom for "no graders" vs. simply omitting the key.

3. Phase 4 copy step fragility
The prompt instructs the agent to cp the report file to the safeoutputs staging directory before calling upload_artifact. If the copy fails silently, the upload will proceed with a missing or empty file and the eval downloadable_report_uploaded will fail. Consider adding an explicit file-existence check in the prompt, or referencing the source path directly if the tool supports it.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet46 · 26.5 AIC · ⌖ 12 AIC · ⊞ 6.2K

[Copilot]

Pull request overview

Adds grader-aware log filtering and a scheduled workflow that publishes daily grader health reports.

Changes:

• Adds --graders filtering across CLI and MCP paths.
• Adds grader artifact selection and focused tests.
• Adds the daily grader audit workflow and generated lock file.

Show a summary per file

File	Description
pkg/cli/mcp_tools_privileged.go	Exposes and forwards MCP grader filtering.
pkg/cli/mcp_tools_privileged_test.go	Tests MCP argument forwarding.
pkg/cli/logs_orchestrator_types.go	Adds grader filter options.
pkg/cli/logs_orchestrator_filters.go	Implements grader run filtering and refactors filters.
pkg/cli/logs_orchestrator_filters_test.go	Tests grader-result filtering.
pkg/cli/logs_orchestrator_download.go	Wires filters and refactors collection state.
pkg/cli/logs_filtering_test.go	Tests CLI flag registration.
pkg/cli/logs_command.go	Registers and loads --graders.
pkg/cli/logs_artifact_set.go	Adds the graders artifact set.
pkg/cli/logs_artifact_set_test.go	Tests grader artifact expansion.
.github/workflows/daily-grader-audit.md	Defines the scheduled audit and report.
.github/workflows/daily-grader-audit.lock.yml	Contains the compiled workflow.

Review details

• Files reviewed: 11/12 changed files
• Comments generated: 2
• Review effort level: Balanced

[github-actions]

🧪 Test Quality Sentinel Review

Score: 78/100 ⚠️ Acceptable

Summary

This PR adds 59 new test scenarios across 4 test files for the daily grader audit workflow feature. Tests demonstrate strong behavioral coverage with table-driven testing patterns and comprehensive edge-case handling.

Test Files

File	Tests	Key Coverage
logs_artifact_set_test.go	16	Artifact filter resolution, validation (59 scenarios)
logs_filtering_test.go	7	Command flags, run ID filtering, log discovery
logs_orchestrator_filters_test.go	9	Engine/runtime/firewall/grader filtering
mcp_tools_privileged_test.go	27	MCP tool argument forwarding, error handling

✅ Strengths

• Table-driven excellence: 59 test scenarios with descriptive assertions in logs_artifact_set_test.go
• Edge-case coverage: Multi-location log discovery (6+ engine variants, recursive search, multiple naming conventions)
• No violations: All build tags present, no forbidden mock libraries, assertions have context
• Graders feature fully tested: CLI flag, artifact filter, MCP forwarding, run filtering, artifact expansion

⚠️ Observations (Non-blocking)

1. Test inflation (justified):

   • logs_filtering_test.go: 4.2:1 ratio (21 test lines vs 5 prod lines) — justified by multi-scenario log discovery complexity
   • mcp_tools_privileged_test.go: 3.3:1 ratio (23 test lines vs 7 prod lines) — small prod addition requires comprehensive protocol testing

2. Implementation test ratio: 36% (threshold: ≤30%) — slightly elevated but acceptable given artifact discovery complexity

3. Assertion style: logs_filtering_test.go uses t.Errorf/t.Fatalf; consider testify consistency (not blocking)

Scoring

Design tests (64%):     25.6/40
Error coverage (36%):   10.8/30
No duplicates:          20/20
Inflation penalty:      -10/10
────────────────────────
Total: 78/100

✅ Recommend: APPROVE — Strong test coverage with justified design choices.

🧪 Test quality analysis by Test Quality Sentinel · copilot · haiku45 · 34.1 AIC · ⌖ 6 AIC · ⊞ 8.3K · ◷
_{Comment /review to run again}

[github-actions]

✅ Test Quality Sentinel: 78/100. 64% design tests, 36% implementation tests (threshold: 30%). All tests verify behavioral contracts with comprehensive edge-case coverage. No violations detected. Recommend merge.

[github-actions]

pkg/cli/logs_orchestrator_filters.go:L62: yagni: one helper per filter plus a generic skip logger. Inline the existing checks in applyRunFilters; the extra indirection doesn't buy anything yet.

net: -18 lines possible.

Generated by ✂️ Ponytail Reviewer for #56359 · codex · mai10 · 6.33 AIC · ⌖ 2.14 AIC · ⊞ 17K
_{Comment /ponytail to run again}

[github-actions]

pkg/cli/logs_orchestrator_filters.go:L62: yagni: one helper per filter plus a generic skip logger. Inline the existing checks in applyRunFilters; the extra indirection doesn't buy anything yet.

[github-actions]

Skills-Based Review 🧠

Applied /codebase-design and /diagnosing-bugs — requesting changes on three issues: one dead-code function, one misleading flag description/behaviour gap, and two correctness concerns introduced in the orchestrator refactor.

📋 Key Themes & Highlights

Key Themes

• Dead code: isGradersArtifactRequested is defined but never called in production paths. Either wire it up (as isEvalsArtifactRequested is) or remove it.
• Behaviour/docs mismatch: applyGradersArtifact is only invoked when --artifacts is already set, so --graders alone does not auto-include grader artifacts despite the flag usage text saying it does.
• Iteration accounting: Rate-limit errors increment state.iteration before continue, silently burning toward MaxIterations with no fetch.
• Partial-result contract change: handleLogsBatchError now returns partial state.processedRuns for generic errors where the old code returned nil — the new contract should be documented or reverted to match the old behaviour.

Positive Highlights

• ✅ Excellent decomposition of applyRunFilters into focused skipBy* helpers — the filter chain via || short-circuiting is clean and readable.
• ✅ logsCollectionState struct cleanly captures loop state; the extraction of fetchAndProcessLogsBatch makes the batch loop much easier to test.
• ✅ Good test coverage for the new --graders flag registration, artifact expansion, and MCP forwarding.
• ✅ logAndMaybeExplainSkip removes duplication across all filter helpers — good DRY refactor.
• ✅ The daily audit workflow is well-scoped: it targets a clear time window, specifies exact output structure, and correctly uses upload_artifact safe-output with 1-day retention.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet46 · 97.3 AIC · ⌖ 10.6 AIC · ⊞ 7.6K
_{Comment /matt to run again}

Comments that could not be inline-anchored

pkg/cli/logs_artifact_set.go:389

[/codebase-design] isGradersArtifactRequested is defined but never called in production code — it is dead code as introduced.

isEvalsArtifactRequested is called in audit_run_pipeline.go (line ~81) and logs_run_processor.go (line ~78) to gate fallback download behaviour. If graders need the same integration, those call sites need to be wired up; if not, the function should be removed to keep the codebase navigable.

<details>
<summary>💡 Options</summary>

Option A — wire it up …

pkg/cli/logs_command.go:283

[/codebase-design] applyGradersArtifact is only called when len(options.ArtifactSets) > 0, so gh aw logs --graders without --artifacts will not automatically include grader artifacts — but the flag's usage text claims it does.

The same pattern exists for --evals, so this may be intentional, but the documentation is misleading and the PR description's example (gh aw logs --graders --artifacts graders) suggests users are expected to pass --artifacts graders manually when us…

pkg/cli/logs_orchestrator_download.go:218

[/diagnosing-bugs] Rate-limit error path increments state.iteration before continue, burning an iteration slot even though no batch fetch occurred.

When waitForLogsRateLimit returns a non-deadline/non-canceled error, the code does state.iteration++; continue. On the next loop body waitForLogsRateLimit is called again with the bumped iteration index, but the HTTP fetch is still skipped. If this error recurs repeatedly, MaxIterations can be exhausted with zero successful fetches …

pkg/cli/logs_orchestrator_download.go:293

[/diagnosing-bugs] handleLogsBatchError returns (false, err) for generic (non-context) errors, which causes collectProcessedWorkflowRuns to return partial state.processedRuns to the caller. The old code returned nil for these errors.

Returning partial results on a fetch error can mislead the caller into thinking the scan was complete up to that point, when in fact it was interrupted mid-run. If partial data is acceptable, the function and its caller should document this explicitl…