MAINT: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks#1355
MAINT: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks#1355
Conversation
…hecks in scenarios Co-authored-by: ValbuenaVC <50061128+ValbuenaVC@users.noreply.github.com>
Co-authored-by: ValbuenaVC <50061128+ValbuenaVC@users.noreply.github.com>
Co-authored-by: ValbuenaVC <50061128+ValbuenaVC@users.noreply.github.com>
|
|
||
| version: int = 1 | ||
| VERSION: int = 1 | ||
| version: int = 1 # Alias for backward compatibility |
There was a problem hiding this comment.
@copilot Can you check if there's a way to use VERSION as the reference for the backwards-compatible alias? I don't think something like VERSION: int = 1; version = VERSION would work as expected, but give it (or something similar) a try.
There was a problem hiding this comment.
Done! Changed to version: int = VERSION which works correctly in Python. All tests passing. (4290950)
| raise ValueError( | ||
| "Scenario not properly initialized. Call await scenario.initialize_async() before running." | ||
| ) |
There was a problem hiding this comment.
@copilot Make sure that for each scenario you add the ValueError each scenario raises to the function's docstring(s) to enforce ruff compliance.
There was a problem hiding this comment.
Added ValueError documentation to all 6 affected function docstrings across the scenario files for ruff compliance. (4290950)
…trings Co-authored-by: ValbuenaVC <50061128+ValbuenaVC@users.noreply.github.com>
|
|
||
| version: int = 1 | ||
| VERSION: int = 1 | ||
| version: int = VERSION # Alias for backward compatibility |
There was a problem hiding this comment.
I think we should either print_deprecation_message when lowercase version is used, or we can likely just migrate. I think just migrating is fine since it's likely not used anywhere
If we print_deprecation_message, we should also check the unit test warnings to ensure everything is using the new VERSION
There was a problem hiding this comment.
I would probably just get rid of version
There was a problem hiding this comment.
I agree since it's really really new.
Scenarios were inconsistent: some used
version, othersVERSION. Additionally, lifecycle validation usedassert self._* is not None, which gets stripped in optimized mode (python -O), breaking production validation.Changes
VERSION(uppercase) as primary constant, with lowercaseversionalias that referencesVERSIONdirectly for backward compatibilityassert self._objective_target is not None→if self._objective_target is None: raise ValueError(...)is Nonecomparison to avoid false positives from empty collectionsExample
Affects 7 scenario classes across airt, garak, and foundry packages.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.