mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
[Resolver]: Rename success_explanation to result_explanation for better clarity (#5724)
Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>
This commit is contained in:
parent
5ea096e95b
commit
252c70984c
12
.github/workflows/openhands-resolver.yml
vendored
12
.github/workflows/openhands-resolver.yml
vendored
@ -321,7 +321,7 @@ jobs:
|
||||
|
||||
let prNumber = '';
|
||||
let branchName = '';
|
||||
let successExplanation = '';
|
||||
let resultExplanation = '';
|
||||
|
||||
try {
|
||||
if (success) {
|
||||
@ -336,16 +336,16 @@ jobs:
|
||||
|
||||
try {
|
||||
if (!success){
|
||||
// Read success_explanation from JSON file for failed resolution
|
||||
// Read result_explanation from JSON file for failed resolution
|
||||
const outputFilePath = path.resolve('/tmp/output/output.jsonl');
|
||||
if (fs.existsSync(outputFilePath)) {
|
||||
const outputContent = fs.readFileSync(outputFilePath, 'utf8');
|
||||
const jsonLines = outputContent.split('\n').filter(line => line.trim() !== '');
|
||||
|
||||
if (jsonLines.length > 0) {
|
||||
// First entry in JSON lines has the key 'success_explanation'
|
||||
// First entry in JSON lines has the key 'result_explanation'
|
||||
const firstEntry = JSON.parse(jsonLines[0]);
|
||||
successExplanation = firstEntry.success_explanation || '';
|
||||
resultExplanation = firstEntry.result_explanation || '';
|
||||
}
|
||||
}
|
||||
} catch (error){
|
||||
@ -364,8 +364,8 @@ jobs:
|
||||
} else if (!success && branchName) {
|
||||
let commentBody = `An attempt was made to automatically fix this issue, but it was unsuccessful. A branch named '${branchName}' has been created with the attempted changes. You can view the branch [here](https://github.com/${context.repo.owner}/${context.repo.repo}/tree/${branchName}). Manual intervention may be required.`;
|
||||
|
||||
if (successExplanation) {
|
||||
commentBody += `\n\nAdditional details about the failure:\n${successExplanation}`;
|
||||
if (resultExplanation) {
|
||||
commentBody += `\n\nAdditional details about the failure:\n${resultExplanation}`;
|
||||
}
|
||||
|
||||
github.rest.issues.createComment({
|
||||
|
||||
@ -242,25 +242,25 @@ async def process_issue(
|
||||
metrics = None
|
||||
success = False
|
||||
comment_success = None
|
||||
success_explanation = 'Agent failed to run'
|
||||
result_explanation = 'Agent failed to run'
|
||||
last_error = 'Agent failed to run or crashed'
|
||||
else:
|
||||
histories = [dataclasses.asdict(event) for event in state.history]
|
||||
metrics = state.metrics.get() if state.metrics else None
|
||||
# determine success based on the history and the issue description
|
||||
success, comment_success, success_explanation = issue_handler.guess_success(
|
||||
success, comment_success, result_explanation = issue_handler.guess_success(
|
||||
issue, state.history
|
||||
)
|
||||
|
||||
if issue_handler.issue_type == 'pr' and comment_success:
|
||||
success_log = 'I have updated the PR and resolved some of the issues that were cited in the pull request review. Specifically, I identified the following revision requests, and all the ones that I think I successfully resolved are checked off. All the unchecked ones I was not able to resolve, so manual intervention may be required:\n'
|
||||
try:
|
||||
explanations = json.loads(success_explanation)
|
||||
explanations = json.loads(result_explanation)
|
||||
except json.JSONDecodeError:
|
||||
logger.error(
|
||||
f'Failed to parse success_explanation as JSON: {success_explanation}'
|
||||
f'Failed to parse result_explanation as JSON: {result_explanation}'
|
||||
)
|
||||
explanations = [str(success_explanation)] # Use raw string as fallback
|
||||
explanations = [str(result_explanation)] # Use raw string as fallback
|
||||
|
||||
for success_indicator, explanation in zip(comment_success, explanations):
|
||||
status = (
|
||||
@ -284,7 +284,7 @@ async def process_issue(
|
||||
metrics=metrics,
|
||||
success=success,
|
||||
comment_success=comment_success,
|
||||
success_explanation=success_explanation,
|
||||
result_explanation=result_explanation,
|
||||
error=last_error,
|
||||
)
|
||||
return output
|
||||
|
||||
@ -16,5 +16,5 @@ class ResolverOutput(BaseModel):
|
||||
metrics: dict[str, Any] | None
|
||||
success: bool
|
||||
comment_success: list[bool] | None
|
||||
success_explanation: str
|
||||
result_explanation: str
|
||||
error: str | None
|
||||
|
||||
@ -581,7 +581,7 @@ def process_single_issue(
|
||||
github_token=github_token,
|
||||
github_username=github_username,
|
||||
patch_dir=patched_repo_dir,
|
||||
additional_message=resolver_output.success_explanation,
|
||||
additional_message=resolver_output.result_explanation,
|
||||
llm_config=llm_config,
|
||||
)
|
||||
else:
|
||||
@ -592,7 +592,7 @@ def process_single_issue(
|
||||
patch_dir=patched_repo_dir,
|
||||
pr_type=pr_type,
|
||||
fork_owner=fork_owner,
|
||||
additional_message=resolver_output.success_explanation,
|
||||
additional_message=resolver_output.result_explanation,
|
||||
target_branch=target_branch,
|
||||
reviewer=reviewer,
|
||||
pr_title=pr_title,
|
||||
|
||||
File diff suppressed because one or more lines are too long
@ -463,7 +463,7 @@ async def test_process_issue(mock_output_dir, mock_prompt_template):
|
||||
assert result.base_commit == base_commit
|
||||
assert result.git_patch == 'test patch'
|
||||
assert result.success == test_case['expected_success']
|
||||
assert result.success_explanation == test_case['expected_explanation']
|
||||
assert result.result_explanation == test_case['expected_explanation']
|
||||
assert result.error == test_case['expected_error']
|
||||
|
||||
# Assert that the mocked functions were called
|
||||
|
||||
@ -721,7 +721,7 @@ def test_process_single_pr_update(
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
success_explanation='[Test success 1]',
|
||||
result_explanation='[Test success 1]',
|
||||
error=None,
|
||||
)
|
||||
|
||||
@ -792,7 +792,7 @@ def test_process_single_issue(
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
success_explanation='Test success 1',
|
||||
result_explanation='Test success 1',
|
||||
error=None,
|
||||
)
|
||||
|
||||
@ -830,7 +830,7 @@ def test_process_single_issue(
|
||||
patch_dir=f'{mock_output_dir}/patches/issue_1',
|
||||
pr_type=pr_type,
|
||||
fork_owner=None,
|
||||
additional_message=resolver_output.success_explanation,
|
||||
additional_message=resolver_output.result_explanation,
|
||||
target_branch=None,
|
||||
reviewer=None,
|
||||
pr_title=None,
|
||||
@ -870,7 +870,7 @@ def test_process_single_issue_unsuccessful(
|
||||
metrics={},
|
||||
success=False,
|
||||
comment_success=None,
|
||||
success_explanation='',
|
||||
result_explanation='',
|
||||
error='Test error',
|
||||
)
|
||||
|
||||
@ -916,7 +916,7 @@ def test_process_all_successful_issues(
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
success_explanation='Test success 1',
|
||||
result_explanation='Test success 1',
|
||||
error=None,
|
||||
)
|
||||
|
||||
@ -936,7 +936,7 @@ def test_process_all_successful_issues(
|
||||
metrics={},
|
||||
success=False,
|
||||
comment_success=None,
|
||||
success_explanation='',
|
||||
result_explanation='',
|
||||
error='Test error 2',
|
||||
)
|
||||
|
||||
@ -956,7 +956,7 @@ def test_process_all_successful_issues(
|
||||
metrics={},
|
||||
success=True,
|
||||
comment_success=None,
|
||||
success_explanation='Test success 3',
|
||||
result_explanation='Test success 3',
|
||||
error=None,
|
||||
)
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user