diff --git a/backend/app/features/simulation/api.py b/backend/app/features/simulation/api.py index a9ce6c39..65154056 100644 --- a/backend/app/features/simulation/api.py +++ b/backend/app/features/simulation/api.py @@ -16,6 +16,7 @@ SimulationOut, SimulationSummaryCapabilitiesOut, SimulationSummaryOut, + SimulationUpdate, ) from app.features.user.manager import current_active_user from app.features.user.models import User @@ -24,6 +25,15 @@ case_router = APIRouter(prefix="/cases", tags=["Cases"]) +def _simulation_detail_query(db: Session): + return db.query(Simulation).options( + joinedload(Simulation.case), + joinedload(Simulation.machine), + selectinload(Simulation.artifacts), + selectinload(Simulation.links), + ) + + @case_router.get( "", response_model=list[CaseOut], @@ -256,15 +266,7 @@ def create_simulation( # Re-query with relationships loaded sim_loaded = ( - db.query(Simulation) - .options( - joinedload(Simulation.case), - joinedload(Simulation.machine), - selectinload(Simulation.artifacts), - selectinload(Simulation.links), - ) - .filter(Simulation.id == sim.id) - .one_or_none() + _simulation_detail_query(db).filter(Simulation.id == sim.id).one_or_none() ) if sim_loaded is None: @@ -336,6 +338,56 @@ def list_simulations( return [_simulation_to_out(s) for s in sims] +@simulation_router.patch( + "/{sim_id}", + response_model=SimulationOut, + responses={ + 200: {"description": "Simulation updated successfully."}, + 401: {"description": "Unauthorized."}, + 404: {"description": "Simulation not found."}, + 422: {"description": "Validation error."}, + 500: {"description": "Internal server error."}, + }, +) +def update_simulation( + sim_id: UUID, + payload: SimulationUpdate, + db: Session = Depends(get_database_session), + user: User = Depends(current_active_user), +) -> SimulationOut: + """Partially update allowed user-managed simulation fields.""" + sim = db.query(Simulation).filter(Simulation.id == sim_id).one_or_none() + + if sim is None: + raise HTTPException(status_code=404, detail="Simulation not found") + + now = datetime.now(timezone.utc) + updates = payload.model_dump(by_alias=False, exclude_unset=True) + + for field, value in updates.items(): + setattr(sim, field, value) + + sim.last_updated_by = user.id + sim.updated_at = now + + with transaction(db): + db.add(sim) + db.flush() + + db.expire_all() + sim_loaded = ( + _simulation_detail_query(db).filter(Simulation.id == sim_id).one_or_none() + ) + + if sim_loaded is None: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to load updated simulation.", + ) + + return _simulation_to_out(sim_loaded) + + @simulation_router.get( "/{sim_id}", response_model=SimulationOut, @@ -367,17 +419,7 @@ def get_simulation(sim_id: UUID, db: Session = Depends(get_database_session)): HTTPException If the simulation with the given ID is not found, raises a 404 HTTP exception. """ - sim = ( - db.query(Simulation) - .options( - joinedload(Simulation.case), - joinedload(Simulation.machine), - selectinload(Simulation.artifacts), - selectinload(Simulation.links), - ) - .filter(Simulation.id == sim_id) - .one_or_none() - ) + sim = _simulation_detail_query(db).filter(Simulation.id == sim_id).one_or_none() if not sim: raise HTTPException(status_code=404, detail="Simulation not found") diff --git a/backend/app/features/simulation/schemas.py b/backend/app/features/simulation/schemas.py index 3d1b8fac..e0b845b5 100644 --- a/backend/app/features/simulation/schemas.py +++ b/backend/app/features/simulation/schemas.py @@ -3,7 +3,7 @@ from typing import Annotated, Any from uuid import UUID -from pydantic import Field, HttpUrl, computed_field +from pydantic import ConfigDict, Field, HttpUrl, computed_field, field_validator from app.common.schemas.base import CamelInBaseModel, CamelOutBaseModel from app.features.machine.schemas import MachineOut @@ -274,6 +274,64 @@ class SimulationCreate(CamelInBaseModel): ] +class SimulationUpdate(CamelInBaseModel): + """Schema for narrow v1 simulation metadata updates.""" + + model_config = ConfigDict(extra="forbid") + + @field_validator("simulation_type", "status", mode="before") + @classmethod + def reject_null_enum_updates(cls, value: Any) -> Any: + if value is None: + msg = "Field may be omitted for PATCH requests, but cannot be null." + raise ValueError(msg) + return value + + simulation_type: Annotated[ + SimulationType | None, Field(None, description="Type of the simulation") + ] + status: Annotated[ + SimulationStatus | None, + Field(None, description="Current status of the simulation"), + ] + description: Annotated[ + str | None, Field(None, description="Optional description of the simulation") + ] + campaign: Annotated[ + str | None, + Field( + None, description="Campaign or run grouping (e.g. historical, amip, tuning)" + ), + ] + experiment_type: Annotated[ + ExperimentType | str | None, + Field( + None, + description=( + "High-level experiment category (e.g. historical, amip, piControl). " + "Often aligned with CMIP experiment identifiers." + ), + ), + ] + hpc_username: Annotated[ + str | None, + Field( + None, + description="HPC username for provenance (trusted, informational only)", + ), + ] + key_features: Annotated[ + str | None, Field(None, description="Optional key features of the simulation") + ] + known_issues: Annotated[ + str | None, Field(None, description="Optional known issues with the simulation") + ] + notes_markdown: Annotated[ + str | None, + Field(None, description="Optional additional notes in markdown format"), + ] + + class SimulationSummaryOut(CamelOutBaseModel): """Lightweight schema for simulation summaries nested inside CaseOut. diff --git a/backend/tests/features/simulation/test_api.py b/backend/tests/features/simulation/test_api.py index 9f730eb9..9320899e 100644 --- a/backend/tests/features/simulation/test_api.py +++ b/backend/tests/features/simulation/test_api.py @@ -1,4 +1,5 @@ from contextlib import nullcontext +from datetime import datetime, timedelta, timezone from unittest.mock import MagicMock, patch from uuid import uuid4 @@ -11,9 +12,10 @@ from app.features.ingestion.enums import IngestionSourceType, IngestionStatus from app.features.ingestion.models import Ingestion from app.features.machine.models import Machine -from app.features.simulation.api import create_simulation +from app.features.simulation.api import create_simulation, update_simulation +from app.features.simulation.enums import SimulationStatus, SimulationType from app.features.simulation.models import Case, Simulation -from app.features.simulation.schemas import SimulationCreate +from app.features.simulation.schemas import SimulationCreate, SimulationUpdate from app.features.user.manager import current_active_user from app.features.user.models import User, UserRole from app.main import app @@ -48,6 +50,75 @@ def _create_case(db: Session, name: str = "test_case") -> Case: return case +def _create_ingestion( + db: Session, + machine_id, + user_id, + source_reference: str = "test_simulation_ingestion", +) -> Ingestion: + ingestion = Ingestion( + source_type=IngestionSourceType.BROWSER_UPLOAD, + source_reference=source_reference, + machine_id=machine_id, + triggered_by=user_id, + status=IngestionStatus.SUCCESS, + created_count=1, + duplicate_count=0, + error_count=0, + ) + db.add(ingestion) + db.flush() + + return ingestion + + +def _create_simulation_record( + db: Session, + *, + case: Case, + machine_id, + ingestion_id, + created_by, + last_updated_by, + execution_id: str = "patch-test-exec-1", + description: str | None = "Original description", + updated_at: datetime | None = None, +) -> Simulation: + sim = Simulation( + case_id=case.id, + execution_id=execution_id, + description=description, + compset="AQUAPLANET", + compset_alias="QPC4", + grid_name="f19_f19", + grid_resolution="1.9x2.5", + initialization_type="startup", + simulation_type="experimental", + status="created", + campaign="campaign-original", + experiment_type="historical", + machine_id=machine_id, + simulation_start_date="2023-01-01T00:00:00Z", + compiler="gcc", + key_features="Original features", + known_issues="Original issues", + notes_markdown="Original notes", + git_repository_url="https://example.com/original", + git_branch="main", + git_tag="v1.0", + git_commit_hash="abc123", + hpc_username="old-user", + created_by=created_by, + last_updated_by=last_updated_by, + ingestion_id=ingestion_id, + updated_at=updated_at or datetime.now(timezone.utc) - timedelta(days=1), + ) + db.add(sim) + db.flush() + + return sim + + class TestListCases: def test_endpoint_returns_empty_list(self, client): res = client.get(f"{API_BASE}/cases") @@ -794,6 +865,237 @@ def test_endpoint_raises_404_if_simulation_not_found(self, client): assert res.json() == {"detail": "Simulation not found"} +class TestUpdateSimulation: + def test_endpoint_updates_sparse_metadata_and_audit_fields( + self, client, db: Session, normal_user_sync, admin_user_sync + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch", + ) + original_updated_at = datetime.now(timezone.utc) - timedelta(days=2) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=admin_user_sync["id"], + updated_at=original_updated_at, + ) + db.commit() + + payload = { + "simulationType": "production", + "status": "completed", + "description": "Updated description", + "campaign": "campaign-updated", + "notesMarkdown": "Updated notes", + } + + res = client.patch(f"{API_BASE}/simulations/{sim.id}", json=payload) + + assert res.status_code == 200 + data = res.json() + assert data["simulationType"] == payload["simulationType"] + assert data["status"] == payload["status"] + assert data["description"] == payload["description"] + assert data["campaign"] == payload["campaign"] + assert data["notesMarkdown"] == payload["notesMarkdown"] + assert data["compiler"] == "gcc" + assert data["gitRepositoryUrl"] == "https://example.com/original" + assert data["gitTag"] == "v1.0" + assert data["hpcUsername"] == "old-user" + assert data["lastUpdatedBy"] == str(normal_user_sync["id"]) + assert data["lastUpdatedByUser"]["email"] == normal_user_sync["email"] + assert data["updatedAt"] != original_updated_at.isoformat() + + db.expire_all() + updated_sim = db.query(Simulation).filter(Simulation.id == sim.id).one() + assert updated_sim.simulation_type == payload["simulationType"] + assert updated_sim.status == payload["status"] + assert updated_sim.description == payload["description"] + assert updated_sim.campaign == payload["campaign"] + assert updated_sim.notes_markdown == payload["notesMarkdown"] + assert updated_sim.compiler == "gcc" + assert updated_sim.git_repository_url == "https://example.com/original" + assert updated_sim.git_tag == "v1.0" + assert updated_sim.last_updated_by == normal_user_sync["id"] + assert updated_sim.updated_at > original_updated_at + + def test_endpoint_returns_401_without_authentication( + self, client, db: Session, normal_user_sync + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch_unauth") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch_unauth", + ) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=normal_user_sync["id"], + execution_id="patch-test-unauth", + ) + db.commit() + + app.dependency_overrides.pop(current_active_user, None) + + res = client.patch( + f"{API_BASE}/simulations/{sim.id}", + json={"description": "Should fail"}, + ) + + assert res.status_code == 401 + + def test_endpoint_returns_404_when_simulation_not_found(self, client): + res = client.patch( + f"{API_BASE}/simulations/{uuid4()}", + json={"description": "Missing simulation"}, + ) + + assert res.status_code == 404 + assert res.json() == {"detail": "Simulation not found"} + + @pytest.mark.parametrize( + "payload", + [ + {"compiler": "intel"}, + {"gitRepositoryUrl": "https://example.com/updated"}, + {"gitBranch": "feature/test"}, + {"gitTag": "v2.0"}, + {"gitCommitHash": "deadbeef"}, + ], + ) + def test_endpoint_rejects_out_of_scope_fields( + self, client, db: Session, normal_user_sync, payload + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch_scope") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch_scope", + ) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=normal_user_sync["id"], + execution_id="patch-test-scope", + ) + db.commit() + + res = client.patch(f"{API_BASE}/simulations/{sim.id}", json=payload) + + assert res.status_code == 422 + + db.expire_all() + unchanged_sim = db.query(Simulation).filter(Simulation.id == sim.id).one() + assert unchanged_sim.description == "Original description" + assert unchanged_sim.compiler == "gcc" + assert unchanged_sim.case_id == case.id + + @pytest.mark.parametrize("payload", [{"status": None}, {"simulationType": None}]) + def test_endpoint_rejects_explicit_null_for_enum_fields( + self, client, db: Session, normal_user_sync, payload + ): + machine = db.query(Machine).first() + assert machine is not None + + case = _create_case(db, "test_case_patch_null_enum") + ingestion = _create_ingestion( + db, + machine.id, + normal_user_sync["id"], + source_reference="test_simulation_patch_null_enum", + ) + sim = _create_simulation_record( + db, + case=case, + machine_id=machine.id, + ingestion_id=ingestion.id, + created_by=normal_user_sync["id"], + last_updated_by=normal_user_sync["id"], + execution_id="patch-test-null-enum", + ) + db.commit() + + original_updated_at = sim.updated_at + + res = client.patch(f"{API_BASE}/simulations/{sim.id}", json=payload) + + assert res.status_code == 422 + + db.expire_all() + unchanged_sim = db.query(Simulation).filter(Simulation.id == sim.id).one() + assert unchanged_sim.status == SimulationStatus.CREATED + assert unchanged_sim.simulation_type == SimulationType.EXPERIMENTAL + assert unchanged_sim.updated_at == original_updated_at + + def test_update_simulation_raises_500_when_reload_fails(self) -> None: + sim_id = uuid4() + user_id = uuid4() + + payload = SimulationUpdate.model_validate({"description": "Updated"}) + + user = User( + id=user_id, + email="reload-fail@example.com", + is_active=True, + is_verified=True, + role=UserRole.USER, + ) + + sim = MagicMock() + sim_query = MagicMock() + sim_query.filter.return_value.one_or_none.return_value = sim + + detail_query = MagicMock() + detail_query.filter.return_value.one_or_none.return_value = None + + db = MagicMock(spec=Session) + db.query.return_value = sim_query + + with patch( + "app.features.simulation.api._simulation_detail_query", + return_value=detail_query, + ): + with patch( + "app.features.simulation.api.transaction", + return_value=nullcontext(), + ): + with pytest.raises(HTTPException) as exc_info: + update_simulation( + sim_id=sim_id, + payload=payload, + db=db, + user=user, + ) + + assert exc_info.value.status_code == 500 + assert exc_info.value.detail == "Failed to load updated simulation." + + class TestSimulationBrowserIncludesCaseMetadata: def test_simulation_list_includes_case_name_and_id( self, client, db: Session, normal_user_sync, admin_user_sync diff --git a/backend/tests/features/simulation/test_schemas.py b/backend/tests/features/simulation/test_schemas.py index d22ed7ab..4b19b393 100644 --- a/backend/tests/features/simulation/test_schemas.py +++ b/backend/tests/features/simulation/test_schemas.py @@ -1,7 +1,8 @@ from datetime import datetime from uuid import uuid4 -from pydantic import HttpUrl +import pytest +from pydantic import HttpUrl, ValidationError from app.common.schemas.utils import to_snake_case from app.features.machine.schemas import MachineOut @@ -15,6 +16,7 @@ SimulationOut, SimulationSummaryCapabilitiesOut, SimulationSummaryOut, + SimulationUpdate, ) from app.features.user.schemas import UserPreview @@ -103,6 +105,53 @@ def test_valid_simulation_create_optional_fields(self): assert getattr(simulation_create, snake_case_key) == value +class TestSimulationUpdateSchema: + def test_accepts_allowed_optional_fields(self): + payload = { + "simulationType": "production", + "status": "completed", + "description": "Updated description", + "campaign": "campaign-1", + "experimentType": "historical", + "hpcUsername": "sim-user", + "keyFeatures": "feature a\nfeature b", + "knownIssues": "issue a", + "notesMarkdown": "## Notes", + } + + update = SimulationUpdate(**payload) + + for key, value in payload.items(): + assert getattr(update, to_snake_case(key)) == value + + @pytest.mark.parametrize( + ("field_name", "value"), + [ + ("compiler", "intel"), + ("gitRepositoryUrl", "https://example.com/repo"), + ("gitBranch", "main"), + ("gitTag", "v1.2.3"), + ("gitCommitHash", "abc123"), + ], + ) + def test_rejects_non_editable_fields(self, field_name: str, value: str): + with pytest.raises(ValidationError): + SimulationUpdate(**{field_name: value}) + + def test_rejects_invalid_predefined_value(self): + with pytest.raises(ValidationError): + SimulationUpdate(status="done") + + @pytest.mark.parametrize("field_name", ["status", "simulationType"]) + def test_rejects_explicit_null_for_non_nullable_enums(self, field_name: str): + with pytest.raises(ValidationError): + SimulationUpdate(**{field_name: None}) + + def test_rejects_out_of_scope_field(self): + with pytest.raises(ValidationError): + SimulationUpdate(caseName="new-case") + + class TestSimulationOutSchema: def test_valid_simulation_out_required_fields(self): # Arrange: Define the required fields diff --git a/frontend/src/components/shared/SimulationStatusBadge.tsx b/frontend/src/components/shared/SimulationStatusBadge.tsx index 52cc5503..aa219040 100644 --- a/frontend/src/components/shared/SimulationStatusBadge.tsx +++ b/frontend/src/components/shared/SimulationStatusBadge.tsx @@ -1,25 +1,80 @@ import { BadgeCheck, CircleDashed, Rocket, X } from 'lucide-react'; interface SimulationStatusBadgeProps { - status: 'complete' | 'running' | 'failed' | 'not-started' | string; + status: + | 'unknown' + | 'created' + | 'queued' + | 'running' + | 'failed' + | 'completed' + | 'complete' + | 'not-started' + | string; } -export const SimulationStatusBadge = ({ status }: SimulationStatusBadgeProps) => ( - - {status === 'complete' && } - {status === 'running' && } - {status === 'failed' && } - {status === 'not-started' && } - {status === 'not-started' ? 'Not Started' : status.charAt(0).toUpperCase() + status.slice(1)} - -); +const STATUS_STYLES: Record< + string, + { + className: string; + label: string; + Icon?: typeof BadgeCheck; + } +> = { + completed: { + className: 'bg-green-50 text-green-900 border border-green-300', + label: 'Completed', + Icon: BadgeCheck, + }, + complete: { + className: 'bg-green-50 text-green-900 border border-green-300', + label: 'Complete', + Icon: BadgeCheck, + }, + running: { + className: 'bg-yellow-50 text-yellow-900 border border-yellow-300', + label: 'Running', + Icon: Rocket, + }, + failed: { + className: 'bg-red-100 text-red-800 border border-red-200', + label: 'Failed', + Icon: X, + }, + queued: { + className: 'bg-blue-50 text-blue-900 border border-blue-200', + label: 'Queued', + }, + created: { + className: 'bg-slate-100 text-slate-800 border border-slate-200', + label: 'Created', + }, + unknown: { + className: 'bg-gray-200 text-gray-600 border border-gray-300', + label: 'Unknown', + Icon: CircleDashed, + }, + 'not-started': { + className: 'bg-gray-200 text-gray-600 border border-gray-300', + label: 'Not Started', + Icon: CircleDashed, + }, +}; + +export const SimulationStatusBadge = ({ status }: SimulationStatusBadgeProps) => { + const normalizedStatus = status.toLowerCase(); + const style = STATUS_STYLES[normalizedStatus] ?? { + className: 'bg-gray-200 text-gray-600 border border-gray-300', + label: status.charAt(0).toUpperCase() + status.slice(1), + }; + const Icon = style.Icon; + + return ( + + {Icon ? : null} + {style.label} + + ); +}; diff --git a/frontend/src/features/simulations/SimulationDetailsPage.tsx b/frontend/src/features/simulations/SimulationDetailsPage.tsx index 59306ee7..1f2d7806 100644 --- a/frontend/src/features/simulations/SimulationDetailsPage.tsx +++ b/frontend/src/features/simulations/SimulationDetailsPage.tsx @@ -1,17 +1,63 @@ +import axios from 'axios'; import { useEffect, useState } from 'react'; import { useLocation, useParams } from 'react-router-dom'; import { useAuth } from '@/auth/hooks/useAuth'; -import { resolvePaceExecution } from '@/features/simulations/api/api'; +import { resolvePaceExecution, updateSimulation } from '@/features/simulations/api/api'; import { SimulationDetailsView } from '@/features/simulations/components/SimulationDetailsView'; import { useSimulation } from '@/features/simulations/hooks/useSimulation'; import { useSimulationSummary } from '@/features/simulations/hooks/useSimulationSummary'; +import { toast } from '@/hooks/use-toast'; +import type { SimulationOut, SimulationUpdate } from '@/types'; -export const SimulationDetailsPage = () => { +const MAX_COMPARE_SELECTION = 5; + +interface SimulationDetailsPageProps { + selectedSimulationIds: string[]; + setSelectedSimulationIds: (ids: string[]) => void; +} + +const getUpdateErrorMessage = (error: unknown): string => { + if (axios.isAxiosError(error)) { + const detail = error.response?.data?.detail; + + if (typeof detail === 'string') { + return detail; + } + + if (Array.isArray(detail)) { + const messages = detail + .map((item) => { + const field = Array.isArray(item?.loc) ? item.loc[item.loc.length - 1] : null; + const message = typeof item?.msg === 'string' ? item.msg : null; + + if (!message) return null; + if (typeof field !== 'string') return message; + + return `${field}: ${message}`; + }) + .filter((message): message is string => Boolean(message)); + + if (messages.length > 0) { + return messages.join(' '); + } + } + } + + return error instanceof Error ? error.message : 'Failed to update simulation.'; +}; + +export const SimulationDetailsPage = ({ + selectedSimulationIds, + setSelectedSimulationIds, +}: SimulationDetailsPageProps) => { const { id } = useParams<{ id: string }>(); const location = useLocation(); - const { data: simulation, loading, error } = useSimulation(id ?? ''); + const { data: fetchedSimulation, loading, error } = useSimulation(id ?? ''); const { isAuthenticated, loading: authLoading, loginWithGithub } = useAuth(); + const [simulation, setSimulation] = useState(null); + const [isSaving, setIsSaving] = useState(false); + const [saveError, setSaveError] = useState(null); const [paceExperimentId, setPaceExperimentId] = useState(null); const [isResolvingPace, setIsResolvingPace] = useState(false); const [paceResolutionAttempted, setPaceResolutionAttempted] = useState(false); @@ -26,6 +72,7 @@ export const SimulationDetailsPage = () => { : normalizedBackHref.startsWith('/simulations') ? 'Back to Simulations' : 'Back to Runs'; + const canEdit = !authLoading && isAuthenticated; const currentSimulation = simulation?.id === id ? simulation : null; const executionId = currentSimulation?.executionId?.trim() ?? ''; const llmSummaryAvailable = currentSimulation?.summaryCapabilities?.llmAvailable ?? false; @@ -36,6 +83,18 @@ export const SimulationDetailsPage = () => { autoGenerate: shouldAutoGenerateSummary, }); + useEffect(() => { + if (fetchedSimulation?.id === id) { + setSimulation(fetchedSimulation); + setSaveError(null); + return; + } + + if (!loading && !error) { + setSimulation(null); + } + }, [error, fetchedSimulation, id, loading]); + useEffect(() => { if (!executionId) { setPaceExperimentId(null); @@ -72,6 +131,50 @@ export const SimulationDetailsPage = () => { }; }, [executionId]); + const handleSave = async (payload: SimulationUpdate): Promise => { + if (!id) return false; + + setIsSaving(true); + setSaveError(null); + + try { + const updatedSimulation = await updateSimulation(id, payload); + setSimulation(updatedSimulation); + return true; + } catch (saveErr) { + setSaveError(getUpdateErrorMessage(saveErr)); + return false; + } finally { + setIsSaving(false); + } + }; + + const handleToggleCompare = () => { + if (!currentSimulation) return; + + if (selectedSimulationIds.includes(currentSimulation.id)) { + setSelectedSimulationIds( + selectedSimulationIds.filter((simulationId) => simulationId !== currentSimulation.id), + ); + return; + } + + if (selectedSimulationIds.length >= MAX_COMPARE_SELECTION) { + toast({ + title: 'Compare list full', + description: `You can compare up to ${MAX_COMPARE_SELECTION} runs at a time.`, + variant: 'destructive', + }); + return; + } + + setSelectedSimulationIds([...selectedSimulationIds, currentSimulation.id]); + toast({ + title: 'Added to compare', + description: `${currentSimulation.executionId} is now in your compare list.`, + }); + }; + if (!id) { return (
@@ -119,6 +222,9 @@ export const SimulationDetailsPage = () => { return ( { summaryLastDurationMs={summary.lastDurationMs} onGenerateSummary={summary.generate} canGenerateSummary + onToggleCompare={handleToggleCompare} + canEdit={canEdit} + isSaving={isSaving} + saveError={saveError} + onSave={handleSave} + onClearSaveError={() => setSaveError(null)} llmSummaryAvailable={llmSummaryAvailable} autoGenerateDeterministicOnLoad={shouldAutoGenerateSummary} showLoginForAiSummary={showLoginForAiSummary} diff --git a/frontend/src/features/simulations/SimulationsPage.tsx b/frontend/src/features/simulations/SimulationsPage.tsx index bcccb5de..8985d149 100644 --- a/frontend/src/features/simulations/SimulationsPage.tsx +++ b/frontend/src/features/simulations/SimulationsPage.tsx @@ -52,7 +52,11 @@ interface SimulationsPageProps { const statusColors: Record = { running: 'bg-blue-100 text-blue-800', complete: 'bg-green-100 text-green-800', + completed: 'bg-green-100 text-green-800', failed: 'bg-red-100 text-red-800', + created: 'bg-slate-100 text-slate-800', + queued: 'bg-indigo-100 text-indigo-800', + unknown: 'bg-gray-100 text-gray-800', 'not-started': 'bg-gray-100 text-gray-800', }; @@ -60,6 +64,8 @@ const typeColors: Record = { production: 'border-green-600 text-green-700', master: 'border-blue-600 text-blue-700', experimental: 'border-amber-600 text-amber-700', + test: 'border-purple-600 text-purple-700', + unknown: 'border-slate-400 text-slate-600', }; /** diff --git a/frontend/src/features/simulations/api/api.ts b/frontend/src/features/simulations/api/api.ts index f00da3de..4c0983fd 100644 --- a/frontend/src/features/simulations/api/api.ts +++ b/frontend/src/features/simulations/api/api.ts @@ -4,6 +4,7 @@ import type { SimulationCreate, SimulationOut, SimulationSummaryResponseOut, + SimulationUpdate, } from '@/types'; export const SIMULATIONS_URL = '/simulations'; @@ -38,6 +39,15 @@ export const getSimulationById = async (id: string): Promise => { return res.data; }; +export const updateSimulation = async ( + id: string, + data: SimulationUpdate, +): Promise => { + const res = await api.patch(`${SIMULATIONS_URL}/${id}`, data); + + return res.data; +}; + export const generateSimulationSummary = async ( id: string, ): Promise => { diff --git a/frontend/src/features/simulations/components/SimulationDetailsView.tsx b/frontend/src/features/simulations/components/SimulationDetailsView.tsx index 754a254b..8686541b 100644 --- a/frontend/src/features/simulations/components/SimulationDetailsView.tsx +++ b/frontend/src/features/simulations/components/SimulationDetailsView.tsx @@ -1,5 +1,5 @@ import { ArrowLeft, ChevronDown, CircleHelp } from 'lucide-react'; -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { Link } from 'react-router-dom'; import { SimulationStatusBadge } from '@/components/shared/SimulationStatusBadge'; @@ -9,6 +9,13 @@ import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from '@/components/ui/collapsible'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select'; import { Separator } from '@/components/ui/separator'; import { Spinner } from '@/components/ui/spinner'; import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs'; @@ -21,20 +28,36 @@ import { } from '@/features/simulations/components/SimulationSummaryPanel'; import { SimulationTypeBadge } from '@/features/simulations/components/SimulationTypeBadge'; import { cn } from '@/lib/utils'; -import type { SimulationOut, SimulationSummaryResponseOut } from '@/types'; +import type { + SimulationEditableField, + SimulationOut, + SimulationStatusValue, + SimulationSummaryResponseOut, + SimulationTypeValue, + SimulationUpdate, +} from '@/types'; +import { SIMULATION_EDITABLE_FIELDS } from '@/types'; import { getArtifactsByKind } from '@/types/artifact'; import { formatDate, getSimulationDuration } from '@/utils/utils'; // -------------------- Types -------------------- interface SimulationDetailsViewProps { simulation: SimulationOut; + isCompareSelected?: boolean; + compareSelectionCount?: number; + maxCompareSelection?: number; canEdit?: boolean; + isSaving?: boolean; + saveError?: string | null; backHref?: string; backLabel?: string; paceLink?: { href: string; label: string; } | null; + onToggleCompare?: () => void; + onSave?: (payload: SimulationUpdate) => Promise | boolean; + onClearSaveError?: () => void; isResolvingPace?: boolean; showPaceFallbackInfo?: boolean; summary?: SimulationSummaryResponseOut | null; @@ -56,12 +79,29 @@ interface SimulationDetailsViewProps { const FieldRow = ({ label, children }: { label: string; children: React.ReactNode }) => (
-
{children}
+
{children}
); const ReadonlyInput = ({ value, className }: { value?: string | null; className?: string }) => ( - +
+ {value || '—'} +
+); + +const ReadonlyText = ({ value, className }: { value?: string | null; className?: string }) => ( +

+ {value || '—'} +

); const UserDisplay = ({ @@ -87,13 +127,97 @@ const ReadonlyTextBlock = ({ value, className }: { value?: string | null; classN
); +const EDITABLE_FIELDS = SIMULATION_EDITABLE_FIELDS; + +type EditableField = SimulationEditableField; +type EditableFormState = Record; + +const SINGLE_LINE_FIELDS: ReadonlySet = new Set([ + 'simulationType', + 'status', + 'campaign', + 'experimentType', + 'hpcUsername', +]); + +const SIMULATION_TYPE_OPTIONS: ReadonlyArray<{ + value: SimulationTypeValue; + label: string; +}> = [ + { value: 'unknown', label: 'Not set' }, + { value: 'production', label: 'Production' }, + { value: 'experimental', label: 'Experimental' }, + { value: 'test', label: 'Test' }, +]; + +const STATUS_OPTIONS: ReadonlyArray<{ + value: SimulationStatusValue; + label: string; +}> = [ + { value: 'unknown', label: 'Unknown' }, + { value: 'created', label: 'Created' }, + { value: 'queued', label: 'Queued' }, + { value: 'running', label: 'Running' }, + { value: 'failed', label: 'Failed' }, + { value: 'completed', label: 'Completed' }, +]; + +const toEditableFormState = (simulation: SimulationOut): EditableFormState => ({ + simulationType: simulation.simulationType, + status: simulation.status, + description: simulation.description ?? '', + campaign: simulation.campaign ?? '', + experimentType: simulation.experimentType ?? '', + hpcUsername: simulation.hpcUsername ?? '', + keyFeatures: simulation.keyFeatures ?? '', + knownIssues: simulation.knownIssues ?? '', + notesMarkdown: simulation.notesMarkdown ?? '', +}); + +const normalizeEditableValue = (field: EditableField, value: string): string | null => { + const trimmed = value.trim(); + + if (!trimmed) { + return null; + } + + return SINGLE_LINE_FIELDS.has(field) ? trimmed : value; +}; + +const buildUpdatePayload = ( + simulation: SimulationOut, + formState: EditableFormState, +): SimulationUpdate => { + const payload: SimulationUpdate = {}; + const mutablePayload = payload as Record; + + for (const field of EDITABLE_FIELDS) { + const nextValue = normalizeEditableValue(field, formState[field]); + const currentValue = simulation[field] ?? null; + + if (nextValue !== currentValue) { + mutablePayload[field] = nextValue; + } + } + + return payload; +}; + // -------------------- View Component -------------------- export const SimulationDetailsView = ({ simulation, + isCompareSelected = false, + compareSelectionCount: compareSelectionCountProp = 0, + maxCompareSelection: maxCompareSelectionProp = 5, canEdit = false, + isSaving = false, + saveError = null, backHref = '/browse', backLabel = 'Back to Runs', paceLink = null, + onToggleCompare, + onSave, + onClearSaveError, isResolvingPace = false, showPaceFallbackInfo = false, summary = null, @@ -111,8 +235,11 @@ export const SimulationDetailsView = ({ onLoginForSummary, }: SimulationDetailsViewProps) => { const [activeTab, setActiveTab] = useState('summary'); + const [isEditing, setIsEditing] = useState(false); const [isAdvancedMetadataOpen, setIsAdvancedMetadataOpen] = useState(false); - const [notes, setNotes] = useState(simulation.notesMarkdown || ''); + const [formState, setFormState] = useState(() => + toEditableFormState(simulation), + ); const performanceLinks = simulation.groupedLinks.performance ?? []; const outputArtifacts = getArtifactsByKind( simulation.artifacts, @@ -160,6 +287,60 @@ export const SimulationDetailsView = ({ }, ]); + useEffect(() => { + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + }, [simulation]); + + useEffect(() => { + if (!canEdit) { + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + } + }, [canEdit, simulation]); + + const updateField = (field: EditableField, value: string) => { + onClearSaveError?.(); + setFormState((current) => ({ + ...current, + [field]: value, + })); + }; + + const handleCancelEdit = () => { + onClearSaveError?.(); + setFormState(toEditableFormState(simulation)); + setIsEditing(false); + }; + + const handleSave = async () => { + if (!onSave) return; + + const payload = buildUpdatePayload(simulation, formState); + + if (Object.keys(payload).length === 0) { + onClearSaveError?.(); + setIsEditing(false); + return; + } + + const saved = await onSave(payload); + + if (saved) { + setIsEditing(false); + } + }; + + const hasUnsavedChanges = Object.keys(buildUpdatePayload(simulation, formState)).length > 0; + const compareSelectionCount = compareSelectionCountProp ?? 0; + const maxCompareSelection = maxCompareSelectionProp ?? 5; + const isCompareActionDisabled = + !isCompareSelected && compareSelectionCount >= maxCompareSelection; + const compareActionLabel = isCompareSelected ? 'Remove from Comparison' : 'Add to Comparison'; + const compareActionTooltip = isCompareActionDisabled + ? `Compare list is full. Remove one of the ${maxCompareSelection} selected runs first.` + : undefined; + const addComment = () => { if (!newComment.trim()) return; setComments((prev) => [ @@ -203,12 +384,56 @@ export const SimulationDetailsView = ({
- - + {compareActionTooltip ? ( + + + + + + + + {compareActionTooltip} + + + ) : ( + + )} + {!isEditing && + (canEdit ? ( + + ) : ( + + + + + + + + Login required + + + ))} + {canEdit && isEditing && ( + <> + + + + )}
+ {isEditing && saveError &&
{saveError}
} {/* Tabs */} @@ -254,7 +479,10 @@ export const SimulationDetailsView = ({ )} - + @@ -277,12 +505,20 @@ export const SimulationDetailsView = ({ - {simulation.description && ( + {(isEditing || simulation.description) && (
- + {isEditing ? ( +