-
Notifications
You must be signed in to change notification settings - Fork 0
[#22] fix(operations): improve execution status tracking and service respose display #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…se display - Add store-level polling for async action execution status - Fix ExecutionStatus type to include 'completed' (SOVD standard) - Add TrackedExecution interface with metadata for polling - Fix OperationResponseDisplay to show live status from store for actions - Fix service response display (handle 'parameters' field format) - Remove auto-refresh checkbox, hide ActionStatusPanel when terminal - Fix ConfigurationPanel overflow with scrollable container - Clean up unused imports and debug logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where async operation execution status was not updating correctly (issue #22). The fix introduces store-level polling to automatically track and update execution statuses, adds the 'completed' status to the ExecutionStatus type per SOVD standards, and improves the UI display for both service and action responses.
Changes:
- Added store-level polling mechanism for tracking async action execution status with automatic cleanup
- Added 'completed' status to ExecutionStatus type and normalized status mapping from API responses
- Refactored operation response display to show live status from store and handle service/action response formats differently
- Simplified ActionStatusPanel by removing manual auto-refresh controls and moving it outside CollapsibleContent
- Added scrollable containers to ConfigurationPanel, OperationsPanel, and EntityDetailPanel to prevent UI overflow
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/types.ts | Added 'completed' to ExecutionStatus type for SOVD compliance |
| src/lib/store.ts | Added TrackedExecution interface, polling logic (startExecutionPolling/stopExecutionPolling), changed autoRefreshExecutions default to true, cleanup in disconnect |
| src/lib/sovd-api.ts | Added normalizeExecutionStatus method to map API status values and extract x-medkit metadata, updated getExecution and cancelExecution to use normalization |
| src/components/OperationsPanel.tsx | Moved ActionStatusPanel outside CollapsibleContent to keep polling active, removed unused entityId/entityType from OperationRow props, added scrollable container with sticky headers |
| src/components/OperationResponse.tsx | Added live status subscription from store, service/action response format handling, displays execution results dynamically |
| src/components/ActionStatusPanel.tsx | Simplified props (only executionId needed), removed auto-refresh checkbox, returns null for terminal executions, uses store-managed polling |
| src/components/EntityDetailPanel.tsx | Added scrollable containers to prevent overflow in data topics list |
| src/components/ConfigurationPanel.tsx | Added scrollable container to prevent overflow in parameters list |
Comments suppressed due to low confidence (1)
src/components/OperationsPanel.tsx:117
- The type declaration still includes 'entityId' and 'entityType' parameters (lines 114, 117), but these are not included in the destructured function parameters (lines 108-112). Since these parameters were removed from the function signature, they should also be removed from the type declaration to maintain consistency.
}: {
operation: Operation;
entityId: string;
onInvoke: (opName: string, payload: unknown) => Promise<CreateExecutionResponse | null>;
defaultExpanded?: boolean;
entityType?: SovdResourceEntityType;
- Change unknown status fallback from 'running' to 'pending' with warning log - Make CreateExecutionResponse.id optional for service calls - Add optional parameters field to CreateExecutionResponse type - Filter internal envelope fields (status, kind, error) from service results - Extract getBorderClass helper to improve readability - Fix race condition in parallel execution status updates (batch updates) - Fix race condition in startExecutionPolling (atomic check + set) - Add execution cleanup after 5 minutes to prevent memory growth - Add 3-second delay before hiding terminal state panels for better UX
Pull Request
Summary
Issue
Link the related issue (required):
Type
Testing
How was this tested / how should reviewers verify it?
manually tested with Turtlebot3 demo
Checklist
npm run lint)npm run build)