fix: replace list.remove() during iteration with comprehension filtering

[lil-aditya]

Both deleteScenarioCaseRuns and deleteCaseRun used the anti-pattern of calling list.remove(item) inside a for-loop over the same list. When an element is removed, the iterator skips the next element because the internal index advances past the shifted items.

Impact:

• deleteScenarioCaseRuns: if a scenario ID appeared in consecutive positions within a case run's Scenarios list, the second occurrence was silently retained, leaving orphaned scenario data in resData.json that could not be deleted through the UI.
• deleteCaseRun: same pattern, though less likely to trigger in practice since case names are typically unique.

Fix: replace the mutation-during-iteration pattern with list-comprehension filtering, which builds a new list and is both safe and idiomatic Python.

Includes 6 new unit tests covering:

• Single scenario removal
• Consecutive duplicate removal (the regression test)
• Cross-case-run removal
• No-match noop
• resultsOnly flag behavior

Linked Issue

• Closes [Bug] deleteScenarioCaseRuns and deleteCaseRun silently skip entries due to list mutation during iteration #442

Existing related work reviewed

• Issues/PRs reviewed: fix: remove obsolete CSV generator backup methods (~1550 lines dead code) #333, fix(backupCase): replace download thread race with after_this_request #229 #341, Fix deleteCase error handling and session cleanup #346, Draft: PoC - Canonical CLEWS Result Extraction for OG-Core Integration #349, fix: add session validation to ViewDataRoute endpoints #350, fix: use enumerate so first tech/comm/emi/stg gets first=True #352, Fix incorrect exception handling in FileClass write methods #354, feat: add model duplicate/clone functionality #355
• None found addressing this specific list-mutation-during-iteration bug after search

Overlap assessment

• Classification: Bug fix
• Overlapping items: None — no existing PR touches deleteScenarioCaseRuns or deleteCaseRun logic
• Why this is not duplicate/superseded: Existing PRs address exception handling (Fix incorrect exception handling in FileClass write methods #354), path traversal (Fix deleteCase error handling and session cleanup #346), session validation (fix: add session validation to ViewDataRoute endpoints #350), and CSV cleanup (fix: remove obsolete CSV generator backup methods (~1550 lines dead code) #333) — none fix the list-mutation pattern

Why this PR should proceed

This is a silent data-corruption bug that leaves orphaned scenario entries in resData.json, which users cannot remove through the UI.

Summary

• What changed: Replaced list.remove() inside for loops with list-comprehension filtering in deleteScenarioCaseRuns and deleteCaseRun
• Why: The old code skips elements when the list shrinks mid-iteration — a well-known Python anti-pattern

Validation

• Tests added/updated (or not applicable)
• Validation steps documented
• Evidence attached (logs/screenshots/output as relevant)

All 55 tests pass (6 new + 49 existing):

Scope check

• No unrelated refactors
• Implemented from a feature branch
• Change is deliverable without upstream 'OSeMOSYS/MuIO' dependency

[Copilot]

Pull request overview

Fixes a silent Python list-mutation bug in case/scenario deletion logic so that all matching entries are removed reliably (including consecutive duplicates), and adds regression/unit tests to prevent reintroduction.

Changes:

• Replace list.remove()-during-iteration in deleteScenarioCaseRuns with comprehension-based filtering.
• Replace list.remove()-during-iteration in deleteCaseRun with comprehension-based filtering.
• Add unit tests covering single removal, consecutive-duplicate regression, cross-case-run removal, noop, and resultsOnly behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
API/Classes/Case/DataFileClass.py	Replaces in-loop list mutation with safe list filtering in scenario and case-run deletion paths.
tests/test_list_mutation_fix.py	Adds targeted tests validating the deletion behavior and the regression scenario.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This test file manually inserts API/ into sys.path, but the repo already sets pythonpath = ["API"] in pyproject.toml and tests/conftest.py explicitly documents that imports should work without sys.path hacks. Keeping this can cause duplicate-import edge cases and makes the test suite less consistent; please remove the sys.path modification and rely on the configured pytest pythonpath instead.

Suggested change

	import sys
	from pathlib import Path
	from unittest.mock import patch, MagicMock
	import pytest
	# Ensure the API directory is on sys.path so we can import the classes
	API_DIR = str(Path(__file__).resolve().parent.parent / "API")
	if API_DIR not in sys.path:
	sys.path.insert(0, API_DIR)
	from pathlib import Path
	from unittest.mock import patch, MagicMock
	import pytest

[Copilot]

MagicMock is imported here but never used in this test module. Please drop the unused import to keep the test code minimal and avoid confusion about intended mocking.

Suggested change

	from unittest.mock import patch, MagicMock
	from unittest.mock import patch