Repository navigation
[BREAKING] FEAT: Make scenario dataset sources and limits explicit - #2956
Open
Richard Lundeen (richlundeen) wants to merge 5 commits into
Open
Richard Lundeen (richlundeen) wants to merge 5 commits into
Richard Lundeen (richlundeen) wants to merge 5 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve latent-injection test coverage with an explicit total limit while preserving default and all-limit behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use explicit dataset sources and max_total=all for the complete latent-injection population check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve upstream request bounds alongside explicit dataset limits. Increment the benchmark version to 7 for independent named-source sampling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Remove sampling_scope, retain empty dataset keys, and preserve Psychosocial sub-harm coverage within the total cap. Update scenario guidance and adjust the merged adaptive run-plan test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment on lines
380
to
382
| if not isinstance(max_dataset_size, _Unset): | ||
| if not isinstance(max_total, _Unset): | ||
| raise ValueError("Use only one of 'max_dataset_size' and 'max_total'.") |
Contributor
There was a problem hiding this comment.
max_dataset_size=10 with one dataset gives 10 on main but 5 here, bc max_per_dataset still defaults to 5. None went from unlimited to 5 too. the warning just says rename, so no one will catch it. can we set max_per_dataset="all" here
Comment on lines
+495
to
+497
| raise DatasetConstraintError( | ||
| f"Psychosocial max_total ({cap}) must cover every selected sub-harm " | ||
| f"({len(self._selected_sub_harms())}); use max_per_dataset=1 for one objective per sub-harm." |
Contributor
There was a problem hiding this comment.
the check is cap < len(sub_harms), so max_per_dataset=1 doesn't change it. and the backend resolver only takes max_dataset_size, so you can't even set it there. maybe say bump max_total or pick fewer sub-harms?
hannahwestra25
approved these changes
Oct 9, 2026
hannahwestra25
left a comment
hannahwestra25
left a comment
Contributor
There was a problem hiding this comment.
ghcp found :
doc/scanner/garak.py:602 uses dataset_names=, which is deprecated now. and the line below still has max_dataset_size. those are pre-existing, but since we're already in this file could we migrate them? otherwise the docs teach the old way.
so double check that all the docs have been updated
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Scenario dataset preparation and limits need clear, consistent rules so overrides do not change fetch policies or remove unrelated caps. This implements the next step in the dataset-generation proposal.
Adds named dataset sources, explicit preparation, and separate source and total limits while keeping dataset reads free of fetching. Uses one limit contract across Python, CLI, API, and GUI: omitted,
None, empty, and"default"use defaults;"all"removes the specified cap. Migrates built-in scenarios and preserves retained source settings during overrides; changingNonefrom unlimited to default is a breaking change.Tests and Documentation
Added regression coverage for preparation, selection, overrides, limit normalization, request persistence, and GUI controls. Updated scenario guidance and paired scenario/scanner notebooks.
Validation commands and results
$env:UV_NO_SYNC = '1'; uv run --no-sync pre-commit run --all-files— passed all hooks, including full Python type checking.uv run --no-sync pytest tests\unit\scenario tests\unit\models\test_scenario_request.py tests\unit\models\test_import_boundary.py tests\unit\backend\test_scenario_configuration_resolver.py tests\unit\backend\test_scenario_service.py tests\unit\backend\test_scenario_run_service.py tests\unit\backend\test_scenario_resume.py tests\unit\cli\test_cli_args.py tests\unit\cli\test_pyrit_scan.py tests\unit\cli\test_pyrit_shell.py tests\unit\cli\test_api_client.py -q -n 4 --tb=short --disable-warnings— 2,252 passed.frontend:npm run type-check -- --pretty false— passed.frontend:npm test -- --runInBand --silent src\components\Scenarios\ScenarioDetail.test.tsx— 65 passed.Jupytext cell comparison confirmed that both changed
.py/.ipynbpairs match. Notebooks were not executed.