Skip to content

Improve trsh attempts reporting#871

Open
Lilachn91 wants to merge 1 commit intomainfrom
trsh_counter
Open

Improve trsh attempts reporting#871
Lilachn91 wants to merge 1 commit intomainfrom
trsh_counter

Conversation

@Lilachn91
Copy link
Copy Markdown
Contributor

  • Added explicit reporting of ESS troubleshooting attempt count.
  • Cleaned troubleshooting-method reporting
  • Replaced hardcoded “geometry optimization” wording in failure logs with dynamic job type.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves troubleshooting (ESS) failure reporting by making attempt counts explicit, cleaning up the reported troubleshooting methods, and ensuring failure logs reference the actual job type instead of a hardcoded phrase.

Changes:

  • Log the ESS troubleshooting attempt number when entering Scheduler.troubleshoot_ess().
  • When troubleshooting is exhausted, report the number of attempts and omit internal trsh_attempt markers from the method list in trsh_ess_job().
  • Add a unit test to verify attempt counting + cleaned method reporting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
arc/scheduler.py Adds warning-level logging for ESS troubleshooting attempt number and refines the initial failure warning message.
arc/job/trsh.py Updates “couldn’t troubleshoot” reporting to count attempts and filter trsh_attempt markers; uses dynamic job_type in messages.
arc/job/trsh_test.py Adds coverage validating attempt counting and that trsh_attempt is not reported as a troubleshooting method.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread arc/job/trsh.py
Comment on lines +1143 to +1148

# Build the message with the count and filtered methods
if trsh_attempt_count > 0 and filtered_methods:
message = f'Tried troubleshooting {trsh_attempt_count} times, with the following methods: {filtered_methods}'
elif trsh_attempt_count > 0:
message = f'Tried troubleshooting {trsh_attempt_count} times'
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attempt-count message always uses "times" (e.g., "Tried troubleshooting 1 times"), which reads incorrectly for a single attempt. Consider pluralizing based on the count ("1 time" vs "N times") so the error/output logs are clearer and more polished.

Suggested change
# Build the message with the count and filtered methods
if trsh_attempt_count > 0 and filtered_methods:
message = f'Tried troubleshooting {trsh_attempt_count} times, with the following methods: {filtered_methods}'
elif trsh_attempt_count > 0:
message = f'Tried troubleshooting {trsh_attempt_count} times'
attempt_label = 'time' if trsh_attempt_count == 1 else 'times'
# Build the message with the count and filtered methods
if trsh_attempt_count > 0 and filtered_methods:
message = f'Tried troubleshooting {trsh_attempt_count} {attempt_label}, with the following methods: {filtered_methods}'
elif trsh_attempt_count > 0:
message = f'Tried troubleshooting {trsh_attempt_count} {attempt_label}'

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.40%. Comparing base (70dab25) to head (84184a6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #871   +/-   ##
=======================================
  Coverage   60.39%   60.40%           
=======================================
  Files         102      102           
  Lines       31102    31109    +7     
  Branches     8104     8105    +1     
=======================================
+ Hits        18784    18790    +6     
- Misses       9974     9975    +1     
  Partials     2344     2344           
Flag Coverage Δ
functionaltests 60.40% <ø> (+<0.01%) ⬆️
unittests 60.40% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants