[Resolver] (refactor) Generalize the resolver #6125
Conversation
Signed-off-by: José Luis Di Biase <josx@interorganic.com.ar>
…nction refactor(resolver) add pull url and authorize url functions
branch_exists and get_branch_name functions refactored
* get_branch_url * implemente get_branch_url() in branch_exists() * get_branch_url --------- Co-authored-by: Charlie <charlie@camba.coop>
cuococarlos
changed the title
| self.owner = owner | ||
| self.repo = repo | ||
| self.token = token | ||
| def __init__(self, strategy, llm_config: LLMConfig): |
There was a problem hiding this comment.
| def __init__(self, strategy, llm_config: LLMConfig): | |
| def __init__(self, strategy: IssueHandlerInterface, llm_config: LLMConfig): |
It would be nice to add typing to strategy; same for other places where strategy is used
| from openhands.resolver.utils import extract_issue_references | ||
|
|
||
|
|
||
| class IssueHandlerInterface(ABC): |
There was a problem hiding this comment.
Just to confirm, when supporting Gitlab all of these interface methods will be implemented?
I'd assume yes, but want to double check
| @@ -6,7 +6,7 @@ class ReviewThread(BaseModel): | |||
| files: list[str] | |||
|
|
|||
|
|
|||
| class GithubIssue(BaseModel): | |||
| class Issue(BaseModel): | |||
There was a problem hiding this comment.
Could we add another field Source, which would be either an enum Github or Gitlab?
| all_issues = [ | ||
| issue | ||
| for issue in all_issues | ||
| if issue['number'] in issue_numbers and 'pull_request' not in issue | ||
| ] |
There was a problem hiding this comment.
Could we do these checks in the GithubHandler? I'm thinking the following
- The handler pulls all the issues and relevant context, and normalizes the data to follow the structure in
openhands/resolver/issue.py - The service context will return details of the context (the issue itself), and all instruction prompts for the resolver task/guessing success
This is for pull requests but similar idea applies to regular issues
| @@ -0,0 +1,481 @@ | |||
| from abc import ABC, abstractmethod | |||
There was a problem hiding this comment.
NIT:
- Can we create a subfolder at this directory level
- In the subfolder, a file which contains the interface definition
- In the same subfolder, the file
github.pythat implements the interface
When adding Gitlab support, I'd imagine we just need to create another file gitlab.py in this subfolder
| ), | ||
| None, | ||
| # Handle PRs with file-specific review comments | ||
| if issue.review_threads: |
There was a problem hiding this comment.
In Github, we have review_threads, thread_comments, and review_comments
Is this same in Gitlab? Reason I ask, is because I'd imagine the next step from this PR is to create a GitlabHandler which can be plugged right into ServiceContext
If Github and Gitlab don't match on this terminology, we may need to do further abstracting on the IssueHandler level? LMK what you think!
End-user friendly description of the problem this fixes or functionality that this introduces
This PR separates the logic of GitHub from the resolver component. The final objective is for OpenHands to use in various code repositories, not just GitHub.
This PR has two steps to achieve the objective:
A new handler to manage all pull requests and issue logic
Make the resolver's GitHub-related logic more abstract and give GitHub's logic to the GithubIssueHandler class.
Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
Give a summary of what the PR does, explaining any non-trivial design decisions
Here are some important decisions:
Link of any specific issues this addresses
[Resolver]: Gitlab support #5210