fix(samples): report failed issue counts and exit non-zero in adk_stale_agent - #6521
fix(samples): report failed issue counts and exit non-zero in adk_stale_agent#6521chelsealong wants to merge 2 commits into
Conversation
…le_agent process_single_issue() swallowed per-issue exceptions and still returned a normal result, so main() counted every issue as processed and the batch always exited 0 even when every issue audit failed. Now failures are tracked separately, main() raises if any issue failed, and the __main__ handler propagates a non-zero exit code while still isolating per-issue failures within a batch. Fixes google#6520
| processed_count += len(chunk) | ||
| logger.info( | ||
| f"--- Finished chunk {current_chunk_num}. Progress:" | ||
| f" {processed_count}/{total_count} ---" |
There was a problem hiding this comment.
I would introduce completed_count because the processed_count value is now controlled by a condition, so the progress log could be misleading
| ) | ||
|
|
||
| if failed_issue_numbers: | ||
| raise RuntimeError( |
There was a problem hiding this comment.
This is not an unexpected runtime failure, some individual issues failing is a normal outcome
…r for expected per-issue failures Addresses review feedback on adk_stale_agent: track attempted vs. successful issues separately so the per-chunk progress log isn't misleading, and stop treating per-issue failures (a normal outcome) as an unexpected runtime error. main() now returns a success bool that sets the process exit code directly.
|
Addressed both comments: introduced a separate |
| f"{len(failed_issue_numbers)} of {total_count} issue(s) failed" | ||
| f" processing: {failed_issue_numbers}" | ||
| ) | ||
| return not failed_issue_numbers |
There was a problem hiding this comment.
Could the author confirm whether this PR, including its review-response commits, was created or updated by an automated AI pipeline? If so, please disclose the tool and confirm that a human has reviewed and tested the resulting changes.
|
Yes — this PR, including the review-response commits (a4384a8, dad579e) addressing the |
Description
In
contributing/samples/adk_team/adk_stale_agent/main.py,process_single_issue()caught any exception from the runner and still returned its normal(duration, api_calls)result.main()then incrementedprocessed_countbylen(chunk)regardless of whether any of those issues actually succeeded, so a run where every issue raised could still logSuccessfully processed N issues.and exit 0.This PR:
process_single_issue()to return asuccessflag alongside its metrics, and sets it toFalsewhen it catches an exception.main()to only incrementprocessed_countfor successes, and to collect failed issue numbers separately, logging them.main()raise aRuntimeErrorif any issues failed, after all chunks have finished (so per-issue isolation within a batch is preserved).__main__block to track an explicit exit code and callsys.exit(exit_code), so the exception raised bymain()actually results in a non-zero process exit status (previously it was caught and logged but the process still exited 0).Fixes #6520
Testing plan
Added
tests/unittests/test_adk_stale_agent_main.py, which imports the sample module (with a fakeGITHUB_TOKEN) and:InMemoryRunner,get_old_open_issue_numbers,get_api_call_count, andreset_api_call_countwith a fake runner whoserun_asyncalways raises (mirroring the issue's reproduction), then assertsmain()raisesRuntimeErrorwhen all issues fail.process_single_issue()returnssuccess=Falsewhen the runner raises.Verified the test fails without the fix: reverted
main.pyto its pre-fix state (git stash/git checkout) and reran the test — it failed withDID NOT RAISE RuntimeError/ValueError: not enough values to unpack (expected 3, got 2), reproducing the exact bug described in the issue (batch logs "Successfully processed 3 issues" and exits 0 despite every issue failing). Restored the fix and reran; both tests pass.Also ran the broader sample test suite (
tests/unittests/test_samples.py) to confirm no regressions: 85 passed.Ran
pre-commit run --files contributing/samples/adk_team/adk_stale_agent/main.py tests/unittests/test_adk_stale_agent_main.py(isort, pyink, addlicense, ADK compliance checks) — all passed.AI-assistance disclosure
This change was prepared with the help of an AI coding assistant (Claude), with the diff and test evidence reviewed before submission.