Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 65 additions & 75 deletions .github/instructions/style-guide.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,29 +95,65 @@ def process(self, data: str) -> str:
...
```

## Documentation Standards
## Imports

### Placement and Organization

Top of file, grouped: stdlib → third-party → local.

### Deferred Imports for Performance

Imports may be placed inside functions/methods when they pull in expensive
third-party packages (`transformers`, `azure.storage.blob`, `alembic`, `openai`,
`scipy`, `pandas`, `av`). Two cases:

1. **CLI entry points** — defer heavy imports to after arg parsing so `--help` is instant.
2. **Internal modules** — when a method is the only consumer of a heavy package.

```python
def main() -> int:
parsed_args = parse_args()
from pyrit.cli import frontend_core # deferred: heavy
...

async def _create_container_client_async(self):
from azure.storage.blob.aio import ContainerClient # deferred: heavy
...
```

Guard tests in `tests/unit/cli/test_import_guards.py` enforce that key import
paths stay fast.

### Lazy `__init__.py` Exports (PEP 562)

Public API packages (`pyrit.prompt_target`, `pyrit.prompt_converter`, `pyrit.score`)
use `__getattr__`-based lazy loading so heavy symbols can be imported from the
package without paying the cost at package load time. See
`pyrit/prompt_target/__init__.py` for the canonical example. Rules:

### Import Placement
- **MANDATORY**: All import statements MUST be at the top of the file
- Do NOT use inline/local imports inside functions or methods
- The only exception is breaking circular import dependencies, which should be rare and documented
- Lazy names must remain in `__all__` and have a `TYPE_CHECKING` import for IDE support.
- Internal utility packages (e.g., `pyrit.common`) simply omit heavy submodules
from `__init__.py` — consumers import directly from the specific file.

### Import Paths

Import from the package root when the symbol is exported from `__init__.py`:

```python
# CORRECT — imports at the top of the file
from contextlib import closing
from sqlalchemy.exc import SQLAlchemyError

def update_entry(self, entry: Base) -> None:
with closing(self.get_session()) as session:
...

# INCORRECT — inline import inside a function
def update_entry(self, entry: Base) -> None:
from contextlib import closing # ← WRONG, must be at top of file
with closing(self.get_session()) as session:
...
from pyrit.prompt_target import PromptChatTarget # CORRECT
from pyrit.prompt_target.common.prompt_chat_target import PromptChatTarget # WRONG
```

Heavy submodules not re-exported from `__init__.py` are imported directly:

```python
from pyrit.common.net_utility import get_httpx_client
```

Within the same package, import from the specific file to avoid circular imports.

## Documentation Standards

### Docstring Format
- Use Google-style docstrings
- Include type information in parameter descriptions
Expand Down Expand Up @@ -211,63 +247,6 @@ async def execute_attack_async(self, *, context: AttackContext) -> AttackResult:
5. Private methods (internal implementation)
6. Static methods and class methods at the end

### Import Organization
```python
# Standard library imports
import asyncio
import json
import logging
from dataclasses import dataclass
from enum import Enum
from pathlib import Path
from typing import Any

# Third-party imports
import numpy as np
from tqdm import tqdm

# Local application imports
from pyrit.attacks.base import AttackStrategy
from pyrit.models import AttackResult
from pyrit.prompt_target import PromptTarget
```

Unless necessary, always import at the top of the file. Don't import inside a function or method.


### Import paths

Often, pyrit has specific files that can be imported. However IF you are importing from a different module than your namespace,
import from the root pyrit module if it's exposed from init.

In the same module, importing from the specific path is usually necessary to prevent circular imports.

- Always check __init__.py exports first - Before using a specific file path, verify if the class/function is exposed at a higher level
- Group related imports - Put all imports from the same root module together
- Use multi-line formatting for readability - When importing 3+ items from the same module, use parentheses


```python
# Correct
from pyrit.prompt_target import PromptChatTarget, OpenAIChatTarget

# Correct
from pyrit.score import (
AzureContentFilterScorer,
FloatScaleThresholdScorer,
SelfAskRefusalScorer,
TrueFalseCompositeScorer,
TrueFalseInverterScorer,
TrueFalseScoreAggregator,
TrueFalseScorer,
)

# Incorrect (if importing from a non-target module)
from pyrit.prompt_target.common.prompt_chat_target import PromptChatTarget
from pyrit.prompt_target.openai.openai_chat_target import OpenAIChatTarget

```

## Error Handling

### Specific Exceptions
Expand Down Expand Up @@ -438,6 +417,16 @@ def process_large_dataset(self, *, file_path: Path) -> list[Result]:
return [self._process_line(line) for line in lines]
```

### Lazy Imports for Startup Performance
- When adding a new module that imports heavy third-party packages (e.g., `transformers`,
`scipy`, `PIL`, `datasets`, `av`), consider whether it is re-exported from a package
`__init__.py` that is on the CLI startup path
- If so, add it to the `_LAZY_IMPORTS` dict in that `__init__.py` instead of as an
eager top-level import (see the Import Placement section for the pattern)
- This is especially important for `pyrit/common/__init__.py`, `pyrit/prompt_target/__init__.py`,
`pyrit/prompt_converter/__init__.py`, and `pyrit/score/__init__.py` which are all on the
import path for CLI startup

## Final Checklist

Before committing code, ensure:
Expand All @@ -449,6 +438,7 @@ Before committing code, ensure:
- [ ] Functions are focused and under 20 lines
- [ ] Error messages are helpful and specific
- [ ] Code follows the import organization pattern
- [ ] New modules with heavy deps follow `__init__.py` startup guidance
- [ ] No hard-coded dependencies
- [ ] Complex logic is extracted to helper methods

Expand Down
47 changes: 28 additions & 19 deletions pyrit/cli/pyrit_scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@
from pathlib import Path
from typing import Optional

from pyrit.cli import frontend_core
from pyrit.cli._cli_args import (
ARG_HELP,
_parse_initializer_arg,
non_negative_int,
positive_int,
validate_log_level_argparse,
)


def parse_args(args: Optional[list[str]] = None) -> Namespace:
Expand Down Expand Up @@ -53,12 +59,12 @@ def parse_args(args: Optional[list[str]] = None) -> Namespace:
parser.add_argument(
"--config-file",
type=Path,
help=frontend_core.ARG_HELP["config_file"],
help=ARG_HELP["config_file"],
)

parser.add_argument(
"--log-level",
type=frontend_core.validate_log_level_argparse,
type=validate_log_level_argparse,
default=logging.WARNING,
help="Logging level (DEBUG, INFO, WARNING, ERROR, CRITICAL) (default: WARNING)",
)
Expand Down Expand Up @@ -91,16 +97,16 @@ def parse_args(args: Optional[list[str]] = None) -> Namespace:

parser.add_argument(
"--initializers",
type=frontend_core._parse_initializer_arg,
type=_parse_initializer_arg,
nargs="+",
help=frontend_core.ARG_HELP["initializers"],
help=ARG_HELP["initializers"],
)

parser.add_argument(
"--initialization-scripts",
type=str,
nargs="+",
help=frontend_core.ARG_HELP["initialization_scripts"],
help=ARG_HELP["initialization_scripts"],
)

parser.add_argument(
Expand All @@ -109,44 +115,44 @@ def parse_args(args: Optional[list[str]] = None) -> Namespace:
type=str,
nargs="+",
dest="scenario_strategies",
help=frontend_core.ARG_HELP["scenario_strategies"],
help=ARG_HELP["scenario_strategies"],
)

parser.add_argument(
"--max-concurrency",
type=frontend_core.positive_int,
help=frontend_core.ARG_HELP["max_concurrency"],
type=positive_int,
help=ARG_HELP["max_concurrency"],
)

parser.add_argument(
"--max-retries",
type=frontend_core.non_negative_int,
help=frontend_core.ARG_HELP["max_retries"],
type=non_negative_int,
help=ARG_HELP["max_retries"],
)

parser.add_argument(
"--memory-labels",
type=str,
help=frontend_core.ARG_HELP["memory_labels"],
help=ARG_HELP["memory_labels"],
)

parser.add_argument(
"--dataset-names",
type=str,
nargs="+",
help=frontend_core.ARG_HELP["dataset_names"],
help=ARG_HELP["dataset_names"],
)

parser.add_argument(
"--max-dataset-size",
type=frontend_core.positive_int,
help=frontend_core.ARG_HELP["max_dataset_size"],
type=positive_int,
help=ARG_HELP["max_dataset_size"],
)

parser.add_argument(
"--target",
type=str,
help=frontend_core.ARG_HELP["target"],
help=ARG_HELP["target"],
)

return parser.parse_args(args)
Expand All @@ -159,14 +165,17 @@ def main(args: Optional[list[str]] = None) -> int:
Returns:
int: Exit code (0 for success, 1 for error).
"""
print("Starting PyRIT...")
sys.stdout.flush()

try:
parsed_args = parse_args(args)
except SystemExit as e:
return e.code if isinstance(e.code, int) else 1

print("Starting PyRIT...")
sys.stdout.flush()

# Defer the heavy import until after arg parsing so --help is instant.
from pyrit.cli import frontend_core

# Handle list commands (don't need full context)
if parsed_args.list_scenarios:
# Simple context just for listing
Expand Down
33 changes: 11 additions & 22 deletions pyrit/common/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT license.

"""Common utilities and helpers for PyRIT."""
"""
Common utilities and helpers for PyRIT.

Heavy submodules (data_url_converter, display_response, download_hf_model,
net_utility) are intentionally NOT re-exported here to keep ``import pyrit``
fast. Import them directly, e.g.::

from pyrit.common.net_utility import get_httpx_client
"""

from pyrit.common.apply_defaults import (
REQUIRED_VALUE,
Expand All @@ -12,18 +20,8 @@
reset_default_values,
set_default_value,
)
from pyrit.common.data_url_converter import convert_local_image_to_data_url
from pyrit.common.default_values import get_non_required_value, get_required_value
from pyrit.common.deprecation import print_deprecation_message
from pyrit.common.display_response import display_image_response
from pyrit.common.download_hf_model import (
download_chunk,
download_file,
download_files,
download_specific_files,
get_available_files,
)
from pyrit.common.net_utility import get_httpx_client, make_request_and_raise_if_error_async
from pyrit.common.notebook_utils import is_in_ipython_session
from pyrit.common.singleton import Singleton
from pyrit.common.utils import (
Expand All @@ -41,28 +39,19 @@
"apply_defaults_to_method",
"combine_dict",
"combine_list",
"convert_local_image_to_data_url",
"DefaultValueScope",
"display_image_response",
"download_chunk",
"download_file",
"download_files",
"download_specific_files",
"get_available_files",
"get_global_default_values",
"get_httpx_client",
"get_kwarg_param",
"get_non_required_value",
"get_random_indices",
"get_required_value",
"verify_and_resolve_path",
"is_in_ipython_session",
"make_request_and_raise_if_error_async",
"print_deprecation_message",
"REQUIRED_VALUE",
"reset_default_values",
"set_default_value",
"Singleton",
"verify_and_resolve_path",
"warn_if_set",
"YamlLoadable",
"print_deprecation_message",
]
Loading
Loading