fix(log-watcher): remove branch filter; fold high-cost alert into primary report#19
Conversation
|
Finding
No other correctness issues stood out in this diff. The Step 8 change is directionally good because it avoids creating a second issue for high-cost failures. |
|
Fair point - both concerns are valid and the real answer is that this should be user-configurable. Updated in 03ccd95: the |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Review Summary
This documentation change correctly addresses both issues from #18:
✅ Branch filter removal - Clear comment with trade-off explanation
✅ High-cost alert consolidation - Properly folded into Step 7's report to maintain "one report per run"
Found one minor clarity issue with variable naming consistency between Step 4 and Step 8. Otherwise the changes are clear, accurate, and achieve the stated goals.
Generated by Agent: Code Review for issue #19
|
Findings:
Overall the rest of the flow looks solid, but I couldn’t run the test suite in this checkout because dependencies are not installed. |
Addresses two issues flagged in #18:
High - branch filter removed
The
branches: [main]filter on theworkflow_runtrigger caused the watcher to silently skip all PR-branch agent runs, which is the common case for gh-aw. Removing the filter means the workflow fires for any branch, matching the behavior of the Cost Tracker and the rest of the agentics sample pack.Medium - double-issue removed
Step 8 previously created a second issue for high-cost failures even when Step 7 had already opened a primary diagnosis issue (the no-PR path). This violated the "One report per run" guideline and would spam notifications. The high-cost callout is now folded into the body of the report that Step 7 already posts, keeping exactly one comment or issue per triggering run.
Closes the two bot findings on #18.