Conversation
There was a problem hiding this comment.
Pull request overview
Implements issue #92 by introducing “suite” groupings for pre-built scenarios in the web editor, backed by a new server endpoint that exposes suite→scenario mappings.
Changes:
- Add
/api/suitesendpoint to return a suite-to-scenario mapping from server configuration. - Update ScenarioList UI to display scenarios grouped under collapsible suite headers (with an “Other Scenarios” bucket).
- Add a root-level
package-lock.json.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| web-editor/src/components/ScenarioEditor/ScenarioList.tsx | Fetch suites and render scenarios grouped into collapsible suite sections. |
| src/openutm_verification/server/router.py | Add /api/suites route that exposes configured suites and their scenario names. |
| package-lock.json | Introduces a new root lockfile (currently empty) outside the existing web-editor Node project. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Promise.all([ | ||
| fetch('/api/scenarios').then(res => res.json()), | ||
| fetch('/api/suites').then(res => res.json()).catch(() => ({})), | ||
| ]).then(([scenarioList, suiteMap]: [string[], SuiteMap]) => { | ||
| setScenarios(scenarioList.sort()); | ||
| setSuites(suiteMap); | ||
| }).catch(err => console.error('Failed to load scenarios:', err)); |
There was a problem hiding this comment.
The /api/scenarios and /api/suites fetches assume a successful (2xx) response and the expected JSON shape. If either endpoint returns a non-OK status with an error payload (e.g., FastAPI {detail: ...}), scenarioList.sort() or suiteNames.flatMap(s => suites[s]) can throw at runtime. Consider checking res.ok before parsing, and validating/coercing the parsed JSON into string[] / Record<string, string[]>, falling back to [] / {} on mismatch.
| Promise.all([ | |
| fetch('/api/scenarios').then(res => res.json()), | |
| fetch('/api/suites').then(res => res.json()).catch(() => ({})), | |
| ]).then(([scenarioList, suiteMap]: [string[], SuiteMap]) => { | |
| setScenarios(scenarioList.sort()); | |
| setSuites(suiteMap); | |
| }).catch(err => console.error('Failed to load scenarios:', err)); | |
| const loadData = async () => { | |
| try { | |
| const [scenariosRes, suitesRes] = await Promise.all([ | |
| fetch('/api/scenarios'), | |
| fetch('/api/suites'), | |
| ]); | |
| let rawScenarios: unknown = []; | |
| let rawSuites: unknown = {}; | |
| if (scenariosRes.ok) { | |
| rawScenarios = await scenariosRes.json(); | |
| } | |
| if (suitesRes.ok) { | |
| rawSuites = await suitesRes.json(); | |
| } | |
| // Coerce scenarios into string[] | |
| const scenarioList: string[] = Array.isArray(rawScenarios) | |
| ? rawScenarios.filter((s): s is string => typeof s === 'string') | |
| : []; | |
| // Coerce suites into Record<string, string[]> | |
| const suiteMap: SuiteMap = {}; | |
| if (rawSuites && typeof rawSuites === 'object' && !Array.isArray(rawSuites)) { | |
| for (const [key, value] of Object.entries(rawSuites as Record<string, unknown>)) { | |
| if (Array.isArray(value)) { | |
| suiteMap[key] = value.filter((v): v is string => typeof v === 'string'); | |
| } | |
| } | |
| } | |
| setScenarios(scenarioList.slice().sort()); | |
| setSuites(suiteMap); | |
| } catch (err) { | |
| console.error('Failed to load scenarios:', err); | |
| setScenarios([]); | |
| setSuites({}); | |
| } | |
| }; | |
| void loadData(); |
| it('renders success status icon', () => { | ||
| render(<CustomNode {...defaultProps} />, { wrapper }); | ||
| // Check for CheckCircle icon (or button containing it) | ||
| const button = screen.getByTitle('Success'); | ||
| expect(button).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('calls onShowResult when status icon is clicked', () => { | ||
| render(<CustomNode {...defaultProps} />, { wrapper }); | ||
| const button = screen.getByTitle('Success'); | ||
| fireEvent.click(button); | ||
| expect(mockData.onShowResult).toHaveBeenCalledWith(mockData.result); | ||
| const { container } = render(<CustomNode {...defaultProps} />, { wrapper }); | ||
| // CheckCircle renders as an SVG with class containing 'circle-check-big' | ||
| const svg = container.querySelector('.lucide-circle-check-big'); | ||
| expect(svg).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders failure status', () => { | ||
| it('renders failure status icon', () => { | ||
| const failureProps = { | ||
| ...defaultProps, | ||
| data: { ...mockData, status: 'failure' as const } | ||
| }; | ||
| render(<CustomNode {...failureProps} />, { wrapper }); | ||
| const button = screen.getByTitle('Failure'); | ||
| expect(button).toBeInTheDocument(); | ||
| // We could check for color or specific icon if we could query by icon | ||
| const { container } = render(<CustomNode {...failureProps} />, { wrapper }); | ||
| // XCircle renders as an SVG with class containing 'circle-x' | ||
| const svg = container.querySelector('.lucide-circle-x'); | ||
| expect(svg).toBeInTheDocument(); |
There was a problem hiding this comment.
These assertions rely on Lucide’s internal generated CSS class names (e.g. .lucide-circle-check-big, .lucide-circle-x). Those class names are not a stable public API and can change across Lucide releases, causing brittle tests. Prefer querying via accessible attributes (e.g., add an aria-label/title or data-testid on the status indicator in CustomNode) and use Testing Library queries instead of querySelector on framework-generated classes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
Implements #92