Conversation
There was a problem hiding this comment.
Pull request overview
Implements an estimated time-to-completion (ETA) display for running simulations by deriving progress/remaining work from task primaries data, and updates lint ignore patterns.
Changes:
- Replace percent-only progress calculation with a metrics calculation that also returns remaining primaries and an ETA.
- Update the simulation progress UI to show “left”, “ETA”, and “Elapsed” alongside the progress bar.
- Expand
.eslintignoreto ignore common build output directories andnode_modules.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/WrapperApp/components/Simulation/SimulationCard/SimulationCardContent.tsx | Adds metrics calculation (progress/remaining/ETA) and displays ETA + remaining in the progress header UI. |
| .eslintignore | Adds ignore patterns for node_modules, build, dist, and clarifies comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // ETA calculation: (Remaining Work) * (Time Spent / Work Done) | ||
| let eta = null; | ||
|
|
||
| if (simulatedSum > 0 && remaining > 0 && duration > 0) { | ||
| const msPerPrimary = duration / simulatedSum; | ||
| eta = remaining * msPerPrimary; | ||
| } |
There was a problem hiding this comment.
ETA is computed using duration derived from job startTime, which can include time spent in PENDING/initialization before any primaries were actually simulated. This will systematically overestimate ms/primary early on and can produce misleading ETAs. Consider basing the rate on time since the first non-zero simulatedPrimaries (e.g., capture a firstSimulatedAtDuration when simulatedSum first becomes >0 and use duration - firstSimulatedAtDuration), or use task startTime for RUNNING tasks when available.
| const [metrics, setMetrics] = useState({ | ||
| progress: 0, | ||
| eta: null as number | null, | ||
| remaining: 0 | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (simulationStatus.jobTasksStatus) { | ||
| const jobTasksStatus = simulationStatus.jobTasksStatus; | ||
| setSimulationProgressPercent(calculateSimulationProgress(jobTasksStatus)); | ||
| const results = calculateSimulationMetrics(simulationStatus.jobTasksStatus, duration); | ||
| setMetrics(results); | ||
| } | ||
| }, [simulationStatus, setSimulationProgressPercent]); | ||
| }, [simulationStatus, duration]); |
There was a problem hiding this comment.
metrics is fully derived from props (simulationStatus.jobTasksStatus and duration) but is stored in local state and updated via an effect. This adds an extra render on every duration/status update and risks derived-state drift. Consider computing it via useMemo (or inline) instead of useState/useEffect.
Fixes #2295