Skip to content

DOC: Replace reST cross-reference roles in docstrings with plain backticks#1782

Merged
romanlutz merged 2 commits into
microsoft:mainfrom
romanlutz:romanlutz/audit-rest-roles
May 26, 2026
Merged

DOC: Replace reST cross-reference roles in docstrings with plain backticks#1782
romanlutz merged 2 commits into
microsoft:mainfrom
romanlutz:romanlutz/audit-rest-roles

Conversation

@romanlutz
Copy link
Copy Markdown
Contributor

Why

PyRIT's docs build uses MyST (Markdown-flavoured), not reStructuredText.
reST cross-reference roles like :class:SeedPrompt, :func: oo, :meth:�ar etc. render as raw text under MyST and are inconsistent with the prevailing convention — plain double-backtick code spans inside Google-style docstrings (see e.g. pyrit/datasets/seed_datasets/remote/visual_leak_bench_dataset.py and pyrit/datasets/seed_datasets/remote/harmbench_multimodal_dataset.py).

A reviewer flagged this on pyrit/datasets/seed_datasets/remote/mossbench_dataset.py and it was fixed there in a follow-up commit. This PR audits and fixes the rest of the repo, and adds a style-guide rule to prevent recurrence.

What changed

Audit + fix (Part 1)

Searched pyrit/, tests/, doc/ for reST role syntax in docstrings/comments. Found 55 occurrences across 17 .py files (zero hits in .md/.ipynb):

Role Count
:class: 25
:func: 11
:meth: 10
:data: 7
:py:func: 2

Every occurrence was rewritten from :role:Name to Name. Pure docstring/comment text changes — no logic or signature changes.

Style rule (Part 2)

Added a "Code references in docstrings" subsection to .github/instructions/style-guide.instructions.md under Documentation Standards → Docstring Format. Its existing applyTo: '**/*.py' front-matter ensures Copilot picks it up for every Python file. The rule:

  • Forbids reST cross-reference roles in docstrings/comments.
  • Mandates plain double-backtick code spans (matches existing convention).
  • Notes the MyST {class}Name alternative only as a rare fallback — the default in PyRIT is plain backticks, not cross-references.

Also added a checklist item to the "Final Checklist" near the end of the file.

Verification

  • uv run ruff check — passes
  • uv run ruff format --check — all 17 files already formatted
  • uv run pre-commit run --files <changed> — all hooks pass (trailing whitespace, EOF, ruff, ty, etc.)
  • uv run pytest on changed-module test files — 146/146 passed
  • Re-ran the grep — 0 remaining reST role occurrences anywhere under pyrit/, tests/, or doc/.

romanlutz and others added 2 commits May 22, 2026 15:42
@hannahwestra25 hannahwestra25 self-assigned this May 26, 2026
@romanlutz romanlutz added this pull request to the merge queue May 26, 2026
Merged via the queue into microsoft:main with commit e53ada3 May 26, 2026
48 checks passed
@romanlutz romanlutz deleted the romanlutz/audit-rest-roles branch May 26, 2026 21:24
ValbuenaVC pushed a commit to ValbuenaVC/PyRIT that referenced this pull request May 28, 2026
…ex copy

Combines three small post-review cleanups in one commit:

* Migrate :class: / :func: / :meth: reST roles to plain double-backtick
  code spans across files touched by this PR (adversarial.py, the two
  test modules, and the captured scenario-description output in the
  benchmark scanner notebook). Matches Rich's microsoft#1782 style-guide rule
  and the new docstring-style entry in .github/instructions/.

* Rewrite the VERSION inline doc to describe the actual bump trigger
  (param + atomic_attack_name format change) and cache-scoping
  semantics (v1 results stay queryable but don't suppress v2 runs).

* Simplify the objective_scorer Args block to reflect c7e7b93
  (guard removed; annotation narrowed to TrueFalseScorer | None) and
  delete the now-lying Raises: TypeError section.

* Inline _select_adversarial_specs at its single call site as a 2-line
  dict lookup and delete the method. The defensive double-filter
  (non-adversarial + drift warnings) is dead by construction since
  BenchmarkStrategy is built from adversarial-capable SCENARIO_TECHNIQUES
  entries only. Drop the corresponding TestSelectAdversarialSpecs class.

* Delete TestAdversarialBenchmarkScorerFlexibility - its sole remaining
  test was a duplicate of test_construct_with_explicit_objective_scorer
  after the c7e7b93 guard removal.

43 unit tests pass (was 46 - -3 for the deletions). No source behavior
change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants