Skip to content

FEAT Add execution-layer trial populations and threshold verdicts#121

Open
behnam-o wants to merge 9 commits into
microsoft:mainfrom
behnam-o:execution-trials
Open

FEAT Add execution-layer trial populations and threshold verdicts#121
behnam-o wants to merge 9 commits into
microsoft:mainfrom
behnam-o:execution-trials

Conversation

@behnam-o

Copy link
Copy Markdown
  • Add execute_trials_async() for repeated execution with one aggregate verdict.
  • Add and publicly export PopulationResult.
  • Define threshold semantics for SAFE, UNSAFE, UNDETERMINED, and ERROR results.
  • Reconcile legacy trial aggregation with threshold-based verdicts while retaining @pytest.mark.trial compatibility.
  • Update trial documentation and unit tests.

@behnam-o
behnam-o requested a review from a team July 20, 2026 20:28
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI review requested due to automatic review settings July 22, 2026 20:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a first-class “population” execution path to RAMPART’s execution layer so callers can run a safety test multiple times and assert a single threshold-based verdict, while updating pytest plugin aggregation semantics and associated docs/tests.

Changes:

  • Introduces execute_trials_async() returning a new PopulationResult aggregate with threshold-based verdict semantics.
  • Updates pytest trial-group aggregation/gating semantics (notably allowing UNSAFE outcomes when the SAFE pass-rate meets the threshold, while ERROR should fail the group).
  • Refreshes documentation and unit tests to reflect the new execution- and aggregation-layer behavior.

Reviewed changes

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

Show a summary per file
File Description
tests/unit/pytest_plugin/test_xdist_aggregation.py Updates xdist aggregation expectations from unconditional UNSAFE-fail to threshold-based pass behavior.
tests/unit/pytest_plugin/test_plugin.py Adjusts trial-group aggregation unit tests and adds coverage for excluding no-result clones from the denominator.
tests/unit/probes/test_single_turn.py Adds a regression test ensuring each trial creates an isolated session when using repeated execution.
tests/unit/core/test_result.py Adds PopulationResult unit tests covering threshold logic and status precedence.
tests/unit/core/test_execution.py Adds execution-layer tests for execute_trials_async() and public export checks for PopulationResult.
rampart/pytest_plugin/plugin.py Updates gate-evaluation logging semantics (now focusing on ERROR and threshold-based pass rate).
rampart/pytest_plugin/_session.py Changes trial-group aggregation semantics: pass-rate denominator excludes no_result, and group passing is threshold-based.
rampart/core/result.py Introduces PopulationResult aggregate type and its status/summary behavior.
rampart/core/execution.py Adds execute_trials_async() to run n full lifecycles and return a PopulationResult.
rampart/core/init.py Re-exports PopulationResult from rampart.core.
rampart/init.py Re-exports PopulationResult from the top-level rampart API.
docs/usage/pytest-integration.md Documents execute_trials_async() semantics as the primary repeated-execution approach.
docs/usage/ci-integration.md Updates CI guidance for trial semantics and threshold/error/no-result behavior.
docs/getting-started/quickstart.md Updates the quickstart example to use execute_trials_async() and describes population-level assertion behavior.

Comment thread rampart/pytest_plugin/plugin.py
Comment thread docs/usage/pytest-integration.md Outdated
Comment thread rampart/pytest_plugin/_session.py Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 21:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

rampart/pytest_plugin/_session.py:283

  • record_trial_group() is documented as “ERROR results make the group fail”, but the counting logic can miss ERRORs when a clone recorded multiple results containing both ERROR and UNSAFE (current if has_unsafe: … elif has_error: …). In that case error_count stays 0 and the group can incorrectly pass if the threshold is met. Prioritize ERROR over UNSAFE so any ERROR in a clone is always reflected in group.errors and group.passed.
        executed_count = total - no_result_count
        pass_rate = safe_count / executed_count if executed_count > 0 else 0.0
        passed = (
            error_count == 0
            and executed_count > 0

docs/usage/pytest-integration.md:71

  • pytest-integration.md now documents execute_trials_async, but it no longer mentions @pytest.mark.trial even though other docs (e.g. CI integration) still recommend it and the quickstart links here as the marker reference. Add a short @pytest.mark.trial subsection clarifying that it runs clones as separate pytest items (possibly across xdist workers) and that execute_trials_async is the option when the threshold must control the single pytest verdict.
**Trial semantics:**

- One logical test produces one pytest verdict
- `threshold` sets the minimum pass rate: `threshold=0.8` requires ≥ 80% SAFE
- An `ERROR` trial resolves the population to `ERROR`
- `UNDETERMINED` trials count against the pass rate
- Individual results and the aggregate verdict are available through `PopulationResult`

Comment thread rampart/pytest_plugin/_session.py Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 21:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

rampart/pytest_plugin/_session.py:244

  • The updated "Semantics" bullet list in this docstring has broken indentation/wrapping, and it doesn’t explicitly state the new behavior that a group fails when all clones are no_result (executed==0). Reformatting the bullets will improve readability and keep the docstring aligned with the implementation.
        Semantics:
                - ERROR results make the group fail.
                - threshold is the minimum pass rate (SAFE / executed).
          e.g. 0.8 means at least 80% of runs must be SAFE.
        - Clones with zero results (skipped or crashed before producing

rampart/pytest_plugin/plugin.py:644

  • Gate logs currently report safe counts as safe/total, but pass_rate is computed using the executed denominator (total - no_result). When there are no-result clones, this can produce misleading logs like "1/2 safe (100% pass rate)". Use the executed denominator in the log counts to keep them consistent with the pass-rate semantics.
                group.total,
                group.pass_rate * 100,
                group.threshold * 100,
            )
        elif group.errors > 0:

rampart/pytest_plugin/_session.py:280

  • record_trial_group counts a clone as UNSAFE before checking for ERROR (if has_unsafe: ... elif has_error: ...). If a clone recorded multiple Results and includes both an ERROR and an UNSAFE, it will be classified as UNSAFE (so error_count stays 0) and the group can incorrectly pass even though an ERROR occurred. ERROR should take precedence in the per-clone classification.
            elif has_safe:
                safe_count += 1

        executed_count = total - no_result_count
        pass_rate = safe_count / executed_count if executed_count > 0 else 0.0

Comment thread rampart/core/result.py Outdated
Comment thread rampart/pytest_plugin/_session.py Outdated
Comment thread rampart/pytest_plugin/_session.py Outdated
Comment thread rampart/core/execution.py
Comment thread rampart/core/result.py Outdated
Comment thread rampart/core/execution.py
)
return result

async def execute_trials_async(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think there is a problem we need to address because only the child Results are auto‑recorded, the PopulationResult and its threshold and grouping is discarded when the test body returns. We may want to capture that in the report.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we could do something like this:

import uuid  

    ...
    population_id = uuid.uuid4().hex
    results: list[Result] = []
    for index in range(n):
        result = await self.execute_async(adapter=adapter)
        result.metadata.setdefault(
            "_rampart_population",
            {
                "id": population_id,
                "threshold": threshold,
                "n": n,
                "index": index,
            },
        )
        results.append(result)

    return PopulationResult(results=results, threshold=threshold)

Although we need to think about this a bit more. uuid avoids population conflict but varies run to run..

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@bashirpartovi good callout.

I liked your metadata suggestion and implemented it with a minor tweak: execute_async() now accepts additional result metadata and attaches it before ON_POST_EXECUTE fires. This ensures every event handler, including the built-in result collector, observes the final annotated Result, and the metadata flows through to reporting sinks by default.

let me know what you think

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

also about the uuid - I think it's actuallya good think to use new ones per population, because it only groups results from one execute_trials_async() invocation; stable pytest context such as _pytest_nodeid and _rampart_result_index is persisted separately for cross-run identification and ordering.

Comment thread rampart/core/result.py
Comment thread rampart/core/execution.py Outdated
Comment thread rampart/core/result.py Outdated
Comment thread tests/unit/core/test_result.py
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.

4 participants