Skip to content

Commit fe5d7cb

Browse files
hbrodinhbrodin
committed
Allows the user to accept/reject the generated patch
1 parent 54de06e commit fe5d7cb

4 files changed

Lines changed: 258 additions & 7 deletions

File tree

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

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
from pydantic import BaseModel
1919

2020
from buttercup.common.telemetry import crs_instance_id
21+
from buttercup.orchestrator.competition_api_client.models.types_patch_submission_response import (
22+
TypesPatchSubmissionResponse,
23+
)
24+
from buttercup.orchestrator.competition_api_client.models.types_submission_status import TypesSubmissionStatus
2125
from buttercup.orchestrator.ui.competition_api.models.types import (
2226
BundleSubmission,
2327
BundleSubmissionResponse,
@@ -374,6 +378,7 @@ def patch_to_patch_info(patch: Patch) -> dict[str, Any]:
374378
"patch_id": getattr(patch, "patch_id", "unknown"),
375379
"timestamp": getattr(patch, "created_at", datetime.now()),
376380
"patch": base64.b64encode(getattr(patch, "patch", "").encode("utf-8", errors="ignore")),
381+
"status": getattr(patch, "status", "accepted"),
377382
}
378383
except Exception as e:
379384
logger.error(f"Error converting patch to info: {e}")
@@ -1044,7 +1049,7 @@ def post_v1_task_task_id_patch_(
10441049

10451050
@app.get(
10461051
"/v1/task/{task_id}/patch/{patch_id}/",
1047-
response_model=PatchSubmissionResponse,
1052+
response_model=TypesPatchSubmissionResponse,
10481053
responses={
10491054
"400": {"model": Error},
10501055
"401": {"model": Error},
@@ -1060,17 +1065,94 @@ def get_v1_task_task_id_patch_patch_id_(
10601065
) -> PatchSubmissionResponse | Error:
10611066
"""Patch Status"""
10621067
logger.info(f"Patch status check - Task: {task_id}, Patch ID: {patch_id}")
1068+
1069+
with database_manager.get_patch(patch_id, task_id) as patch_obj:
1070+
if patch_obj is None:
1071+
return Error(message=f"Patch {patch_id} not found")
1072+
1073+
# Access patch attributes within the session context
1074+
patch_status = patch_obj.status
1075+
1076+
# Map the database status to TypesSubmissionStatus enum
1077+
status_mapping = {
1078+
"accepted": TypesSubmissionStatus.SubmissionStatusAccepted,
1079+
"passed": TypesSubmissionStatus.SubmissionStatusPassed,
1080+
"failed": TypesSubmissionStatus.SubmissionStatusFailed,
1081+
}
1082+
1083+
status = status_mapping.get(patch_status, TypesSubmissionStatus.SubmissionStatusAccepted)
1084+
1085+
return TypesPatchSubmissionResponse(
1086+
patch_id=patch_id,
1087+
status=status,
1088+
functionality_tests_passing=patch_status == "passed",
1089+
)
1090+
1091+
1092+
@app.post(
1093+
"/v1/task/{task_id}/patch/{patch_id}/approve",
1094+
response_model=TypesPatchSubmissionResponse,
1095+
responses={
1096+
"400": {"model": Error},
1097+
"401": {"model": Error},
1098+
"404": {"model": Error},
1099+
"500": {"model": Error},
1100+
},
1101+
tags=["patch"],
1102+
)
1103+
def approve_patch(
1104+
task_id: str,
1105+
patch_id: str,
1106+
database_manager: DatabaseManager = Depends(get_database_manager),
1107+
) -> PatchSubmissionResponse | Error:
1108+
"""Approve a patch"""
1109+
logger.info(f"Patch approval - Task: {task_id}, Patch ID: {patch_id}")
1110+
10631111
with database_manager.get_patch(patch_id, task_id) as patch:
10641112
if patch is None:
10651113
return Error(message=f"Patch {patch_id} not found")
10661114

1067-
return PatchSubmissionResponse(
1115+
database_manager.update_patch_status(patch_id=patch_id, status="passed")
1116+
1117+
return TypesPatchSubmissionResponse(
10681118
patch_id=patch_id,
1069-
status=SubmissionStatus.SubmissionStatusPassed,
1119+
status=TypesSubmissionStatus.SubmissionStatusPassed,
10701120
functionality_tests_passing=True,
10711121
)
10721122

10731123

1124+
@app.post(
1125+
"/v1/task/{task_id}/patch/{patch_id}/reject",
1126+
response_model=TypesPatchSubmissionResponse,
1127+
responses={
1128+
"400": {"model": Error},
1129+
"401": {"model": Error},
1130+
"404": {"model": Error},
1131+
"500": {"model": Error},
1132+
},
1133+
tags=["patch"],
1134+
)
1135+
def reject_patch(
1136+
task_id: str,
1137+
patch_id: str,
1138+
database_manager: DatabaseManager = Depends(get_database_manager),
1139+
) -> PatchSubmissionResponse | Error:
1140+
"""Reject a patch"""
1141+
logger.info(f"Patch rejection - Task: {task_id}, Patch ID: {patch_id}")
1142+
1143+
with database_manager.get_patch(patch_id, task_id) as patch:
1144+
if patch is None:
1145+
return Error(message=f"Patch {patch_id} not found")
1146+
1147+
database_manager.update_patch_status(patch_id=patch_id, status="failed")
1148+
1149+
return TypesPatchSubmissionResponse(
1150+
patch_id=patch_id,
1151+
status=TypesSubmissionStatus.SubmissionStatusFailed,
1152+
functionality_tests_passing=False,
1153+
)
1154+
1155+
10741156
@app.post(
10751157
"/v1/task/{task_id}/pov/",
10761158
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/script.js‎

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -404,12 +404,23 @@ function renderPatches() {
404404
<div class="artifact-meta">
405405
<span>ID: ${item.patch.patch_id}</span>
406406
<span>Size: ${(item.patch.patch || '').length} chars</span>
407+
<span class="status-badge status-${item.patch.status}">${item.patch.status}</span>
407408
</div>
408409
<div class="artifact-timestamp">${formatTimestamp(item.patch.timestamp)}</div>
409410
</div>
410-
<button class="download-button" onclick="event.stopPropagation(); downloadArtifact('patch', '${item.task_id}', '${item.patch.patch_id}')">
411-
Download
412-
</button>
411+
<div class="artifact-actions">
412+
${item.patch.status === 'accepted' ? `
413+
<button class="approve-button" onclick="event.stopPropagation(); approvePatch('${item.task_id}', '${item.patch.patch_id}')">
414+
Approve
415+
</button>
416+
<button class="reject-button" onclick="event.stopPropagation(); rejectPatch('${item.task_id}', '${item.patch.patch_id}')">
417+
Reject
418+
</button>
419+
` : ''}
420+
<button class="download-button" onclick="event.stopPropagation(); downloadArtifact('patch', '${item.task_id}', '${item.patch.patch_id}')">
421+
Download
422+
</button>
423+
</div>
413424
</div>
414425
`).join('');
415426
}
@@ -767,6 +778,54 @@ function renderArtifact(artifact, type) {
767778
`;
768779
}
769780

781+
// Approve patch
782+
async function approvePatch(taskId, patchId) {
783+
try {
784+
const response = await fetch(`${API_BASE}/v1/task/${taskId}/patch/${patchId}/approve`, {
785+
method: 'POST',
786+
headers: {
787+
'Content-Type': 'application/json',
788+
},
789+
});
790+
791+
if (response.ok) {
792+
showNotification('Patch approved successfully', 'success');
793+
// Refresh the dashboard to show updated status
794+
loadDashboard();
795+
} else {
796+
const errorData = await response.json();
797+
showNotification(`Failed to approve patch: ${errorData.message || 'Unknown error'}`, 'error');
798+
}
799+
} catch (error) {
800+
console.error('Approve patch error:', error);
801+
showNotification('Error approving patch', 'error');
802+
}
803+
}
804+
805+
// Reject patch
806+
async function rejectPatch(taskId, patchId) {
807+
try {
808+
const response = await fetch(`${API_BASE}/v1/task/${taskId}/patch/${patchId}/reject`, {
809+
method: 'POST',
810+
headers: {
811+
'Content-Type': 'application/json',
812+
},
813+
});
814+
815+
if (response.ok) {
816+
showNotification('Patch rejected successfully', 'success');
817+
// Refresh the dashboard to show updated status
818+
loadDashboard();
819+
} else {
820+
const errorData = await response.json();
821+
showNotification(`Failed to reject patch: ${errorData.message || 'Unknown error'}`, 'error');
822+
}
823+
} catch (error) {
824+
console.error('Reject patch error:', error);
825+
showNotification('Error rejecting patch', 'error');
826+
}
827+
}
828+
770829
// Download artifact
771830
async function downloadArtifact(type, taskId, artifactId) {
772831
try {
@@ -890,6 +949,10 @@ function renderArtifactDetail(detailData, type) {
890949
? patchContent.substring(0, 300) + '...'
891950
: patchContent;
892951
specificContent = `
952+
<div class="detail-label">Status:</div>
953+
<div class="detail-value">
954+
<span class="status-badge status-${artifact.status || 'accepted'}">${artifact.status || 'accepted'}</span>
955+
</div>
893956
<div class="detail-label">Patch Size:</div>
894957
<div class="detail-value">${originalSize} characters (${patchContent.length} decoded)</div>
895958
<div class="detail-label">Patch Content:</div>
@@ -920,10 +983,18 @@ function renderArtifactDetail(detailData, type) {
920983
${specificContent}
921984
</div>
922985
${detailData.task_id ? `
923-
<div style="margin-top: 1.5rem;">
986+
<div style="margin-top: 1.5rem; display: flex; gap: 1rem; flex-wrap: wrap;">
924987
<button class="btn btn-primary" onclick="downloadArtifact('${type}', '${detailData.task_id}', '${artifactId}')">
925988
Download ${type.toUpperCase()}
926989
</button>
990+
${type === 'patch' && (artifact.status || 'accepted') === 'accepted' ? `
991+
<button class="btn btn-success" onclick="approvePatch('${detailData.task_id}', '${artifactId}')">
992+
Approve Patch
993+
</button>
994+
<button class="btn btn-danger" onclick="rejectPatch('${detailData.task_id}', '${artifactId}')">
995+
Reject Patch
996+
</button>
997+
` : ''}
927998
</div>
928999
` : ''}
9291000
</div>

‎orchestrator/src/buttercup/orchestrator/ui/static/styles.css‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,15 @@ body {
440440
background-color: #218838;
441441
}
442442

443+
.btn-danger {
444+
background-color: #dc3545;
445+
color: white;
446+
}
447+
448+
.btn-danger:hover {
449+
background-color: #c82333;
450+
}
451+
443452
/* Modal styles */
444453
.modal {
445454
display: none;
@@ -713,6 +722,65 @@ form {
713722
background: #5a6bd8;
714723
}
715724

725+
.artifact-actions {
726+
display: flex;
727+
gap: 0.5rem;
728+
align-items: center;
729+
}
730+
731+
.approve-button {
732+
padding: 0.5rem 1rem;
733+
background: #10b981;
734+
color: white;
735+
border: none;
736+
border-radius: 4px;
737+
cursor: pointer;
738+
font-size: 0.9rem;
739+
transition: background 0.2s ease;
740+
}
741+
742+
.approve-button:hover {
743+
background: #059669;
744+
}
745+
746+
.reject-button {
747+
padding: 0.5rem 1rem;
748+
background: #ef4444;
749+
color: white;
750+
border: none;
751+
border-radius: 4px;
752+
cursor: pointer;
753+
font-size: 0.9rem;
754+
transition: background 0.2s ease;
755+
}
756+
757+
.reject-button:hover {
758+
background: #dc2626;
759+
}
760+
761+
.status-badge {
762+
padding: 0.25rem 0.5rem;
763+
border-radius: 4px;
764+
font-size: 0.8rem;
765+
font-weight: 500;
766+
text-transform: uppercase;
767+
}
768+
769+
.status-badge.status-accepted {
770+
background: #fef3c7;
771+
color: #92400e;
772+
}
773+
774+
.status-badge.status-passed {
775+
background: #d1fae5;
776+
color: #065f46;
777+
}
778+
779+
.status-badge.status-failed {
780+
background: #fee2e2;
781+
color: #991b1b;
782+
}
783+
716784
/* Responsive design */
717785
@media (max-width: 768px) {
718786
.nav-container {

0 commit comments

Comments
 (0)