feat(triage signals): Remove fixability from run_automation and put into _generate_summary #103485
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Mihir-Mavalankar
changed the title
| if fixability_score is None: | ||
| with sentry_sdk.start_span(op="ai_summary.generate_fixability_score"): | ||
| fixability_response = _generate_fixability_score(group) | ||
|
|
||
| if not issue_summary.scores: | ||
| raise ValueError("Issue summary scores is None or empty.") | ||
| if issue_summary.scores.fixability_score is None: | ||
| raise ValueError("Issue summary fixability score is None.") | ||
| if not fixability_response.scores: | ||
| raise ValueError("Issue summary scores is None or empty.") | ||
| if fixability_response.scores.fixability_score is None: | ||
| raise ValueError("Issue summary fixability score is None.") | ||
|
|
||
| group.update(seer_fixability_score=issue_summary.scores.fixability_score) | ||
| group.update(seer_fixability_score=fixability_response.scores.fixability_score) | ||
| fixability_score = fixability_response.scores.fixability_score |
There was a problem hiding this comment.
feels like this could be a helper as it's duplicate code.
and also, what if there's no summary generated? should that be part of the retry?
There was a problem hiding this comment.
That check I will have in the job that triggers the direct call to run_automation.
There was a problem hiding this comment.
oh i mean in that case we could also keep the fixability score check there too instead of putting the retry in here
There was a problem hiding this comment.
So you mean remove it from run_automation and add it in the task at directly calls run_automation? I did think of that and there I think if issue summary doesn't exist I was just gonna call _generate_summary which would do all 3 generate_summary, generate fixability and start_autofix.
There was a problem hiding this comment.
Or even just have a fixability check there.
There was a problem hiding this comment.
I think either you should put a retry for both fixability and issue summary inside run_automation, or you should check the same thing in every place before you call run_automation.
The former might be cleaner long term but up to you
There was a problem hiding this comment.
No I don't think putting fixability and issue summary inside run_automation is a good idea. That function should just be to trigger automation.
Ideally, I don't even want this re-try in run_automation. Even in the new flow, run_automation will never be called without issue_summary running first so we will always have fixability. run_automation will fail only if the call to that fixability endpoint fails which is true even today.
There was a problem hiding this comment.
I'm rethinking the last commit, because we will always check if issue_summary is present before running automation and run issue summary (and hence fixability) if it's not present.
If issue_summary ran -> fixability exists (high degree of certainty). So I don't think we need a re-try in run_automation.
There was a problem hiding this comment.
sure if you do the retry before calling run_automation, that works too
Mihir-Mavalankar
changed the title
jennmueng
left a comment
jennmueng
left a comment
There was a problem hiding this comment.
after reviewing the diagram and looking at the threads this makes sense to me, although not fully in context with the discussions this seems like it wouldn't break the current flow.
Co-authored-by: Rohan Agarwal <[email protected]>
This reverts commit b18d38c.
Mihir-Mavalankar
force-pushed
the
extract-fixability-v2
branch
from
a6803d7 to
99ab73b
Compare
With fixability score of 0.80, the stopping point should be OPEN_PR (not CODE_CHANGES) because 0.80 >= 0.78 (SUPER_HIGH threshold of 0.76 + 0.02 buffer).
Mihir-Mavalankar
added
the
Trigger: Revert
|
PR reverted: 0f2707e |
…nd put into _generate_summary (#103485)" This reverts commit 699b63a. Co-authored-by: Mihir-Mavalankar <[email protected]>
…ged] (#103676) ## PR Details + I had to revert by last fixability [PR](#103485). I am going to address it separately after this PR - made a ticket for that: https://linear.app/getsentry/issue/AIML-1650/address-fixability-issue-in-the-new-automation-flow + Also mentioned in the TODO comment + This PR just creates the new automation flow for triage signals behind a feature flag. + Reference diagram: https://miro.com/app/board/uXjVJqn1-fQ=/?focusWidget=3458764648325594380 + Also make `run_automation` a public method now since we directly use in a task.
PR Details