Skip to content

Commit d28e461

Browse files
hbrodinhbrodin
authored
Allows the user to accept/reject the generated patch (#330)
* Allows the user to accept/reject the generated patch * Use correct types * Further style enhancements
1 parent 891d7d1 commit d28e461

File tree

5 files changed

+346
-5
lines changed

5 files changed

+346
-5
lines changed

‎orchestrator/src/buttercup/orchestrator/ui/competition_api/main.py‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ def patch_to_patch_info(patch: Patch) -> dict[str, Any]:
374374
"patch_id": getattr(patch, "patch_id", "unknown"),
375375
"timestamp": getattr(patch, "created_at", datetime.now()),
376376
"patch": base64.b64encode(getattr(patch, "patch", "").encode("utf-8", errors="ignore")),
377+
"status": getattr(patch, "status", "accepted"),
377378
}
378379
except Exception as e:
379380
logger.error(f"Error converting patch to info: {e}")
@@ -1060,17 +1061,94 @@ def get_v1_task_task_id_patch_patch_id_(
10601061
) -> PatchSubmissionResponse | Error:
10611062
"""Patch Status"""
10621063
logger.info(f"Patch status check - Task: {task_id}, Patch ID: {patch_id}")
1064+
1065+
with database_manager.get_patch(patch_id, task_id) as patch_obj:
1066+
if patch_obj is None:
1067+
return Error(message=f"Patch {patch_id} not found")
1068+
1069+
# Access patch attributes within the session context
1070+
patch_status = patch_obj.status
1071+
1072+
# Map the database status to SubmissionStatus enum
1073+
status_mapping = {
1074+
"accepted": SubmissionStatus.SubmissionStatusAccepted,
1075+
"passed": SubmissionStatus.SubmissionStatusPassed,
1076+
"failed": SubmissionStatus.SubmissionStatusFailed,
1077+
}
1078+
1079+
status = status_mapping.get(patch_status, SubmissionStatus.SubmissionStatusAccepted)
1080+
1081+
return PatchSubmissionResponse(
1082+
patch_id=patch_id,
1083+
status=status,
1084+
functionality_tests_passing=patch_status == "passed",
1085+
)
1086+
1087+
1088+
@app.post(
1089+
"/v1/task/{task_id}/patch/{patch_id}/approve",
1090+
response_model=PatchSubmissionResponse,
1091+
responses={
1092+
"400": {"model": Error},
1093+
"401": {"model": Error},
1094+
"404": {"model": Error},
1095+
"500": {"model": Error},
1096+
},
1097+
tags=["patch"],
1098+
)
1099+
def approve_patch(
1100+
task_id: str,
1101+
patch_id: str,
1102+
database_manager: DatabaseManager = Depends(get_database_manager),
1103+
) -> PatchSubmissionResponse | Error:
1104+
"""Approve a patch"""
1105+
logger.info(f"Patch approval - Task: {task_id}, Patch ID: {patch_id}")
1106+
10631107
with database_manager.get_patch(patch_id, task_id) as patch:
10641108
if patch is None:
10651109
return Error(message=f"Patch {patch_id} not found")
10661110

1111+
database_manager.update_patch_status(patch_id=patch_id, status="passed")
1112+
10671113
return PatchSubmissionResponse(
10681114
patch_id=patch_id,
10691115
status=SubmissionStatus.SubmissionStatusPassed,
10701116
functionality_tests_passing=True,
10711117
)
10721118

10731119

1120+
@app.post(
1121+
"/v1/task/{task_id}/patch/{patch_id}/reject",
1122+
response_model=PatchSubmissionResponse,
1123+
responses={
1124+
"400": {"model": Error},
1125+
"401": {"model": Error},
1126+
"404": {"model": Error},
1127+
"500": {"model": Error},
1128+
},
1129+
tags=["patch"],
1130+
)
1131+
def reject_patch(
1132+
task_id: str,
1133+
patch_id: str,
1134+
database_manager: DatabaseManager = Depends(get_database_manager),
1135+
) -> PatchSubmissionResponse | Error:
1136+
"""Reject a patch"""
1137+
logger.info(f"Patch rejection - Task: {task_id}, Patch ID: {patch_id}")
1138+
1139+
with database_manager.get_patch(patch_id, task_id) as patch:
1140+
if patch is None:
1141+
return Error(message=f"Patch {patch_id} not found")
1142+
1143+
database_manager.update_patch_status(patch_id=patch_id, status="failed")
1144+
1145+
return PatchSubmissionResponse(
1146+
patch_id=patch_id,
1147+
status=SubmissionStatus.SubmissionStatusFailed,
1148+
functionality_tests_passing=False,
1149+
)
1150+
1151+
10741152
@app.post(
10751153
"/v1/task/{task_id}/pov/",
10761154
response_model=POVSubmissionResponse,

‎orchestrator/src/buttercup/orchestrator/ui/database.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ class Patch(Base):
8181
patch_id: Mapped[str] = mapped_column(String, primary_key=True, default=lambda: str(uuid.uuid4()))
8282
task_id: Mapped[str] = mapped_column(String, ForeignKey("tasks.task_id"), nullable=False)
8383
patch: Mapped[str] = mapped_column(Text)
84+
status: Mapped[str] = mapped_column(String, default="accepted") # accepted, passed, failed
8485
created_at: Mapped[datetime] = mapped_column(DateTime, default=func.now())
8586

8687
task: Mapped[Task] = relationship("Task", back_populates="patches")
@@ -118,6 +119,24 @@ def _create_tables(self) -> None:
118119
Base.metadata.create_all(bind=self.engine)
119120
logger.info("Database tables created/verified")
120121

122+
# Migrate existing patches to have status field
123+
self._migrate_patch_status()
124+
125+
def _migrate_patch_status(self) -> None:
126+
"""Migrate existing patches to have a status field."""
127+
try:
128+
with self.get_session() as session:
129+
# Check if there are any patches without status
130+
patches_without_status = session.query(Patch).filter(Patch.status.is_(None)).all()
131+
if patches_without_status:
132+
logger.info(f"Migrating {len(patches_without_status)} patches to have status field")
133+
for patch in patches_without_status:
134+
patch.status = "accepted"
135+
session.commit()
136+
logger.info("Patch status migration completed")
137+
except Exception as e:
138+
logger.warning(f"Patch status migration failed (this is normal for new databases): {e}")
139+
121140
def get_session(self) -> Session:
122141
"""Get a database session."""
123142
return self.SessionLocal()
@@ -347,6 +366,17 @@ def create_patch(self, *, task_id: str, patch: str) -> Patch:
347366
logger.info(f"Created patch: {patch_obj.patch_id}")
348367
return patch_obj
349368

369+
def update_patch_status(self, *, patch_id: str, status: str) -> None:
370+
"""Update the status of a patch."""
371+
with self.get_session() as session:
372+
patch = session.query(Patch).filter(Patch.patch_id == patch_id).first()
373+
if patch:
374+
patch.status = status
375+
session.commit()
376+
logger.info(f"Updated patch {patch_id} status to: {status}")
377+
else:
378+
logger.error(f"Patch {patch_id} not found for status update")
379+
350380
# Bundle operations
351381
def create_bundle(
352382
self,

‎orchestrator/src/buttercup/orchestrator/ui/static/index.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ <h2>Submit New Task</h2>
150150
placeholder="1800">
151151
</div>
152152
<div class="form-buttons">
153-
<button type="button" id="fill-example-btn" class="btn btn-success">Fill Example-libpng Values</button>
153+
<button type="button" id="fill-example-btn" class="btn btn-success">Fill example-libpng Values</button>
154154
<button type="button" id="cancel-btn" class="btn btn-secondary">Cancel</button>
155155
<button type="submit" class="btn btn-primary">Submit Task</button>
156156
</div>

‎orchestrator/src/buttercup/orchestrator/ui/static/script.js‎

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -418,12 +418,23 @@ function renderPatches() {
418418
<div class="artifact-meta">
419419
<span>ID: ${item.patch.patch_id}</span>
420420
<span>Size: ${(item.patch.patch || '').length} chars</span>
421+
<span class="status-badge status-${item.patch.status}">${item.patch.status}</span>
421422
</div>
422423
<div class="artifact-timestamp">${formatTimestamp(item.patch.timestamp)}</div>
423424
</div>
424-
<button class="download-button" onclick="event.stopPropagation(); downloadArtifact('patch', '${item.task_id}', '${item.patch.patch_id}')">
425-
Download
426-
</button>
425+
<div class="artifact-actions">
426+
${item.patch.status === 'accepted' ? `
427+
<button class="approve-button" onclick="event.stopPropagation(); approvePatch('${item.task_id}', '${item.patch.patch_id}')">
428+
Approve
429+
</button>
430+
<button class="reject-button" onclick="event.stopPropagation(); rejectPatch('${item.task_id}', '${item.patch.patch_id}')">
431+
Reject
432+
</button>
433+
` : ''}
434+
<button class="download-button" onclick="event.stopPropagation(); downloadArtifact('patch', '${item.task_id}', '${item.patch.patch_id}')">
435+
Download
436+
</button>
437+
</div>
427438
</div>
428439
`).join('');
429440
}
@@ -762,6 +773,54 @@ function renderArtifact(artifact, type) {
762773
`;
763774
}
764775

776+
// Approve patch
777+
async function approvePatch(taskId, patchId) {
778+
try {
779+
const response = await fetch(`${API_BASE}/v1/task/${taskId}/patch/${patchId}/approve`, {
780+
method: 'POST',
781+
headers: {
782+
'Content-Type': 'application/json',
783+
},
784+
});
785+
786+
if (response.ok) {
787+
showNotification('Patch approved successfully', 'success');
788+
// Refresh the dashboard to show updated status
789+
loadDashboard();
790+
} else {
791+
const errorData = await response.json();
792+
showNotification(`Failed to approve patch: ${errorData.message || 'Unknown error'}`, 'error');
793+
}
794+
} catch (error) {
795+
console.error('Approve patch error:', error);
796+
showNotification('Error approving patch', 'error');
797+
}
798+
}
799+
800+
// Reject patch
801+
async function rejectPatch(taskId, patchId) {
802+
try {
803+
const response = await fetch(`${API_BASE}/v1/task/${taskId}/patch/${patchId}/reject`, {
804+
method: 'POST',
805+
headers: {
806+
'Content-Type': 'application/json',
807+
},
808+
});
809+
810+
if (response.ok) {
811+
showNotification('Patch rejected successfully', 'success');
812+
// Refresh the dashboard to show updated status
813+
loadDashboard();
814+
} else {
815+
const errorData = await response.json();
816+
showNotification(`Failed to reject patch: ${errorData.message || 'Unknown error'}`, 'error');
817+
}
818+
} catch (error) {
819+
console.error('Reject patch error:', error);
820+
showNotification('Error rejecting patch', 'error');
821+
}
822+
}
823+
765824
// Download artifact
766825
async function downloadArtifact(type, taskId, artifactId) {
767826
try {
@@ -885,6 +944,10 @@ function renderArtifactDetail(detailData, type) {
885944
? patchContent.substring(0, 300) + '...'
886945
: patchContent;
887946
specificContent = `
947+
<div class="detail-label">Status:</div>
948+
<div class="detail-value">
949+
<span class="status-badge status-${artifact.status || 'accepted'}">${artifact.status || 'accepted'}</span>
950+
</div>
888951
<div class="detail-label">Patch Size:</div>
889952
<div class="detail-value">${originalSize} characters (${patchContent.length} decoded)</div>
890953
<div class="detail-label">Patch Content:</div>
@@ -915,10 +978,18 @@ function renderArtifactDetail(detailData, type) {
915978
${specificContent}
916979
</div>
917980
${detailData.task_id ? `
918-
<div style="margin-top: 1.5rem;">
981+
<div style="margin-top: 1.5rem; display: flex; gap: 1rem; flex-wrap: wrap;">
919982
<button class="btn btn-primary" onclick="downloadArtifact('${type}', '${detailData.task_id}', '${artifactId}')">
920983
Download ${type.toUpperCase()}
921984
</button>
985+
${type === 'patch' && (artifact.status || 'accepted') === 'accepted' ? `
986+
<button class="btn btn-success" onclick="approvePatch('${detailData.task_id}', '${artifactId}')">
987+
Approve Patch
988+
</button>
989+
<button class="btn btn-danger" onclick="rejectPatch('${detailData.task_id}', '${artifactId}')">
990+
Reject Patch
991+
</button>
992+
` : ''}
922993
</div>
923994
` : ''}
924995
</div>

0 commit comments

Comments
 (0)