Skip to content

MAINT: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks#1355

Open
Copilot wants to merge 5 commits intomainfrom
copilot/fix-scenario-version-capitalization
Open

MAINT: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks#1355
Copilot wants to merge 5 commits intomainfrom
copilot/fix-scenario-version-capitalization

Conversation

Copy link
Contributor

Copilot AI commented Feb 5, 2026

Scenarios were inconsistent: some used version, others VERSION. Additionally, lifecycle validation used assert self._* is not None, which gets stripped in optimized mode (python -O), breaking production validation.

Changes

  • Standardize version naming: All scenarios now use VERSION (uppercase) as primary constant, with lowercase version alias that references VERSION directly for backward compatibility
  • Replace asserts with explicit checks: Changed assert self._objective_target is not Noneif self._objective_target is None: raise ValueError(...)
    • Uses explicit is None comparison to avoid false positives from empty collections
    • Provides clear error messages that persist in production
    • Added ValueError documentation to function docstrings for ruff compliance

Example

# Before
class Cyber(Scenario):
    version: int = 1
    
    def _create_atomic_attack(self, strategy: str) -> AtomicAttack:
        assert self._objective_target is not None
        # ...

# After  
class Cyber(Scenario):
    VERSION: int = 1
    version: int = VERSION  # Alias for backward compatibility
    
    def _create_atomic_attack(self, strategy: str) -> AtomicAttack:
        """
        ...
        
        Raises:
            ValueError: If scenario is not properly initialized or an unknown strategy is passed.
        """
        if self._objective_target is None:
            raise ValueError(
                "Scenario not properly initialized. Call await scenario.initialize_async() before running."
            )
        # ...

Affects 7 scenario classes across airt, garak, and foundry packages.

Original prompt

This section details on the original issue you should resolve

<issue_title>FEAT Scenario Consistency</issue_title>
<issue_description>Raising issue based on concerns from #1329:

  • version should be capitalized to VERSION for all scenarios.
  • Scenarios often use aassert self._* is not None pattern to ensure lifecycle compliance. This may cause problems. Originally referenced here: (FEAT: Jailbreak Scenario #1329 (comment))</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits February 5, 2026 22:15
…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>
Copilot AI changed the title [WIP] Fix scenario consistency for version capitalization Standardize scenario VERSION constant and replace asserts with proper lifecycle checks Feb 5, 2026
Copilot AI requested a review from ValbuenaVC February 5, 2026 22:30

version: int = 1
VERSION: int = 1
version: int = 1 # Alias for backward compatibility
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Changed to version: int = VERSION which works correctly in Python. All tests passing. (4290950)

Comment on lines +246 to +248
raise ValueError(
"Scenario not properly initialized. Call await scenario.initialize_async() before running."
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Make sure that for each scenario you add the ValueError each scenario raises to the function's docstring(s) to enforce ruff compliance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Copilot AI requested a review from ValbuenaVC February 5, 2026 23:01
@ValbuenaVC ValbuenaVC marked this pull request as ready for review February 5, 2026 23:06
@ValbuenaVC ValbuenaVC changed the title Standardize scenario VERSION constant and replace asserts with proper lifecycle checks FIX: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks Feb 5, 2026
@ValbuenaVC ValbuenaVC changed the title FIX: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks MAINT: Standardize scenario VERSION constant and replace asserts with proper lifecycle checks Feb 5, 2026

version: int = 1
VERSION: int = 1
version: int = VERSION # Alias for backward compatibility
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably just get rid of version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEAT Scenario Consistency

3 participants