diff --git a/audits/experiments.py b/audits/experiments.py index 3be7bf8e..195f4f4b 100644 --- a/audits/experiments.py +++ b/audits/experiments.py @@ -478,8 +478,54 @@ def spec_to_row(spec: dict) -> str: return json.dumps(row) -def launch_experiment(*, project, user, name: str, runs: list[dict], repeat: dict | None = None) -> Experiment: - """Create the Experiment and its frozen runs, then submit them. +def resolve_into_experiment(project, raw) -> Experiment | None: + """The existing experiment a New Experiment launch should join, or None for a new one. + + ``raw`` is the form's ``into_experiment`` value. Blank means "create a new + experiment"; anything else must name an experiment in this workspace. + """ + raw = (raw or "").strip() + if not raw: + return None + if not raw.isdigit(): + raise DesignError("The selected experiment is not valid.") + experiment = Experiment.objects.filter(pk=int(raw), project=project).first() + if experiment is None: + raise DesignError("The selected experiment no longer exists in this workspace.") + return experiment + + +def prepare_into_experiment(*, experiment: Experiment, user, n_new_runs: int, confirm_public: bool) -> None: + """Check that ``n_new_runs`` may be launched into an existing experiment. + + Same rules as adding runs by hand (add_runs): admin/auditor, and admin plus + confirmation for a public experiment, since the new runs become public. + Call inside the launch transaction: the experiment row is locked so + concurrent launches cannot exceed the run cap. + """ + _require_membership_role(user, experiment, confirm_public=confirm_public) + Experiment.objects.select_for_update().filter(pk=experiment.pk).first() + if experiment.memberships.count() + n_new_runs > MAX_RUNS_PER_EXPERIMENT: + raise DesignError( + f"“{experiment.name}” would exceed {MAX_RUNS_PER_EXPERIMENT} runs; " + "pick fewer runs or start a new experiment." + ) + + +def launch_experiment( + *, + project, + user, + name: str, + runs: list[dict], + repeat: dict | None = None, + into: Experiment | None = None, + confirm_public: bool = False, +) -> Experiment: + """Create the Experiment (or join ``into``) and its frozen runs, then submit them. + + With ``into`` the runs are added to that existing experiment instead of a + new one (``name`` is then ignored); see prepare_into_experiment for the rules. ``runs`` items: {"name", "version", "target", "auditor", "judge_model", "judge", "max_turns", "language", "n_repetitions", "gen_config"}. With ``repeat`` @@ -505,9 +551,17 @@ def launch_experiment(*, project, user, name: str, runs: list[dict], repeat: dic # audit the agent's bare base model instead of the agent. raise DesignError("Repeating an agent experiment is not supported yet; launch it once instead.") with transaction.atomic(): - experiment = Experiment.objects.create( - project=project, name=name.strip()[:250] or "Experiment", created_by=user - ) + if into is not None: + run_now_count = len(runs) if (not repeat or repeat["start"] == "now") else 0 + prepare_into_experiment( + experiment=into, user=user, n_new_runs=run_now_count, confirm_public=confirm_public + ) + experiment = into + experiment.save(update_fields=["updated_at"]) + else: + experiment = Experiment.objects.create( + project=project, name=name.strip()[:250] or "Experiment", created_by=user + ) run_now = not repeat or repeat["start"] == "now" created = [ create_audit_run( diff --git a/infra/tests/test_experiment_attach.py b/infra/tests/test_experiment_attach.py new file mode 100644 index 00000000..3121dc40 --- /dev/null +++ b/infra/tests/test_experiment_attach.py @@ -0,0 +1,90 @@ +"""Experiments list delete, and New Experiment launching into an existing experiment. + +Run: + SIMPLEAUDIT_LOCAL_SQLITE=1 uv run manage.py test infra.tests.test_experiment_attach +""" +from django.test import Client + +from audits.models import AuditRun, Experiment +from infra.tests.factories import MembershipFactory, UserFactory +from infra.tests.test_experiments import _ExperimentBase + + +class ExperimentListDeleteTests(_ExperimentBase): + def test_list_shows_delete_and_deletes_keeping_runs(self): + self.client.post("/experiments/new/", self._design(target_model=[self.t1.id], max_turns="3")) + exp = Experiment.objects.get(project=self.project) + page = self.client.get("/experiments/") + self.assertContains(page, f"/experiments/{exp.id}/delete/") + resp = self.client.post(f"/experiments/{exp.id}/delete/", {"next": "list"}) + self.assertRedirects(resp, "/experiments/", fetch_redirect_response=False) + self.assertFalse(Experiment.objects.filter(pk=exp.pk).exists()) + self.assertEqual(AuditRun.objects.filter(project=self.project).count(), 1) + + def test_viewer_sees_no_delete(self): + exp = Experiment.objects.create(project=self.project, name="E", created_by=self.user) + viewer = UserFactory() + viewer.set_password("pw") + viewer.save() + MembershipFactory(user=viewer, project=self.project, role="viewer") + c = Client() + c.login(username=viewer.username, password="pw") + page = c.get("/experiments/") + self.assertNotContains(page, f"/experiments/{exp.id}/delete/") + + +class LaunchIntoExistingTests(_ExperimentBase): + def setUp(self): + super().setUp() + self.client.post("/experiments/new/", self._design(target_model=[self.t1.id], max_turns="3", name="Base")) + self.exp = Experiment.objects.get(project=self.project) + + def test_design_page_offers_existing_experiments_and_prefills_into(self): + page = self.client.get(f"/experiments/new/?into={self.exp.id}") + self.assertContains(page, 'name="into_experiment"') + self.assertContains(page, "1 run · updated") + self.assertRegex(page.content.decode(), rf'value="{self.exp.id}" data-public=""[^>]*checked') + + def test_detail_page_links_to_new_runs(self): + page = self.client.get(f"/experiments/{self.exp.id}/") + self.assertContains(page, f"/experiments/new/?into={self.exp.id}") + + def test_single_run_joins_existing(self): + resp = self.client.post("/experiments/new/", self._design( + target_model=[self.t2.id], max_turns="3", into_experiment=str(self.exp.id))) + self.assertRedirects(resp, f"/experiments/{self.exp.id}/", fetch_redirect_response=False) + self.assertEqual(Experiment.objects.filter(project=self.project).count(), 1) + self.assertEqual(self.exp.runs.count(), 2) + self.assertEqual(Experiment.objects.get(pk=self.exp.pk).name, "Base") + + def test_multi_run_review_and_launch_join_existing(self): + review = self.client.post("/experiments/new/", self._design(into_experiment=str(self.exp.id))) + self.assertEqual(review.context["into_experiment"], self.exp) + self.assertContains(review, "Adding to experiment") + resp = self.client.post("/experiments/new/", self._review_to_launch_payload(review)) + self.assertRedirects(resp, f"/experiments/{self.exp.id}/", fetch_redirect_response=False) + self.assertEqual(Experiment.objects.filter(project=self.project).count(), 1) + self.assertEqual(self.exp.runs.count(), 5) + + def test_public_experiment_needs_confirmation(self): + self.exp.visibility = Experiment.Visibility.PUBLIC + self.exp.save() + resp = self.client.post("/experiments/new/", self._design( + target_model=[self.t2.id], max_turns="3", into_experiment=str(self.exp.id))) + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "Confirm the change") + self.assertEqual(self.exp.runs.count(), 1) + self.client.post("/experiments/new/", self._design( + target_model=[self.t2.id], max_turns="3", into_experiment=str(self.exp.id), into_confirm_public="1")) + self.assertEqual(self.exp.runs.count(), 2) + + def test_other_workspace_experiment_rejected(self): + from infra.tests.factories import ProjectFactory + + other = Experiment.objects.create(project=ProjectFactory(), name="Other") + resp = self.client.post("/experiments/new/", self._design( + target_model=[self.t2.id], max_turns="3", into_experiment=str(other.id))) + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "no longer exists") + self.assertEqual(other.runs.count(), 0) + self.assertEqual(AuditRun.objects.filter(project=self.project).count(), 1) diff --git a/infra/ui.py b/infra/ui.py index 27c4d19a..be38f8dc 100644 --- a/infra/ui.py +++ b/infra/ui.py @@ -49,6 +49,7 @@ submit_audit_run, ) from infra.chat_feature import chat_enabled +from infra.exceptions import StableAPIError from infra.export_utils import csv_safe from infra.seo import SITE_NAME, _share_context from scenarios.models import ( @@ -86,6 +87,31 @@ def _public_experiment_filter(request) -> dict: return {"visibility": Experiment.Visibility.PUBLIC} +def _into_permission_error(into) -> str: + """Why a launch into an existing experiment was refused (role check failed).""" + if into is not None and into.is_public: + return f"Adding runs to the public experiment “{into.name}” needs the workspace admin role." + return "Adding runs to an experiment needs the workspace admin or auditor role." + + +def _attach_choices(user, project) -> list: + """Experiments a New Experiment launch can join: private ones for admins/auditors, public ones for admins.""" + from accounts.models import ProjectMembership + from audits.models import Experiment + + if not _can_edit_membership(user, project): + return [] + is_admin = user.is_superuser or ProjectMembership.objects.filter( + user=user, project=project, role=ProjectMembership.Role.ADMIN + ).exists() + from django.db.models import Count + + qs = Experiment.objects.filter(project=project).annotate(n_runs=Count("memberships")).order_by("-updated_at") + if not is_admin: + qs = qs.exclude(visibility=Experiment.Visibility.PUBLIC) + return list(qs.only("id", "name", "visibility", "updated_at")[:200]) + + def _can_edit_membership(user, project) -> bool: """Whether ``user`` may add/remove experiment runs in ``project`` (admin/auditor). @@ -1410,6 +1436,7 @@ def get_context_data(self, **kw): "start": "baseline" if source and get.get("repeat") else "now", "first_run_at": "", "clone_from": str(source.id) if source else "", + "into_experiment": get.get("into", "") if is_id(get.get("into", "")) else "", }) # Connections the picker offers: this workspace's own plus any shared # into it (public / admin-shared / explicitly shared). Each carries a @@ -1484,6 +1511,7 @@ def get_context_data(self, **kw): repeat_choices=REPEAT_CHOICES, timezones=_timezone_choices(), source_run=source, + attach_choices=_attach_choices(self.request.user, p), # Same rule as launching a run: admin or auditor (superusers too). can_launch=has_write_role(self.request.user, p), ) @@ -1491,7 +1519,7 @@ def get_context_data(self, **kw): def _redesign(self, post, error=None): rep = {k: post.get(k, "") for k in ("repeat", "timezone", "cron_expression", "start", "first_run_at", "clone_from", - "name")} + "name", "into_experiment", "into_confirm_public")} rep["repeat"] = rep["repeat"] or "once" source = None if is_id(rep["clone_from"]): @@ -1539,21 +1567,36 @@ def _launch_single(self, request, spec, repeat): """ from django.db import transaction - from audits.experiments import spec_to_run + from audits.experiments import ( + prepare_into_experiment, + resolve_into_experiment, + spec_to_run, + ) from audits.models import Experiment from audits.monitors import create_monitor p = request.project run_spec = spec_to_run(spec, name="") custom_name = request.POST.get("name", "").strip()[:250] - run = monitor = None + run = monitor = into = None try: + into = resolve_into_experiment(p, request.POST.get("into_experiment")) with transaction.atomic(): - experiment = Experiment.objects.create( - project=p, - name=(custom_name or f"{spec['target'].display_name} · {spec['scenario_set'].label}")[:250], - created_by=request.user, - ) + if into is not None: + prepare_into_experiment( + experiment=into, + user=request.user, + n_new_runs=1 if (not repeat or repeat["start"] == "now") else 0, + confirm_public=request.POST.get("into_confirm_public") == "1", + ) + experiment = into + experiment.save(update_fields=["updated_at"]) + else: + experiment = Experiment.objects.create( + project=p, + name=(custom_name or f"{spec['target'].display_name} · {spec['scenario_set'].label}")[:250], + created_by=request.user, + ) if not repeat or repeat["start"] == "now": run = create_audit_run( project=p, @@ -1587,10 +1630,14 @@ def _launch_single(self, request, spec, repeat): experiment=experiment, first_point=first_point, ) + except StableAPIError: + return self._redesign(request.POST, _into_permission_error(into)) except Exception as e: # noqa: BLE001 - surface any creation failure to the user return self._redesign(request.POST, str(e)) if run: submit_audit_run(run) + if into is not None and run: + messages.success(request, f"Run added to “{experiment.name}”.") if monitor: messages.success(request, f"Monitor “{monitor.name}” created: next run {monitor.next_run_at:%Y-%m-%d %H:%M} UTC.") return redirect(f"/experiments/{experiment.id}/") @@ -1645,8 +1692,15 @@ def _render_review(self, request, *, rows, experiment_name, design_post, repeat= for row in rows: row["cells"] = [row["values"][k] for k, _ in columns] row["warnings"] = [w for w in row["warnings"] if w not in shared] + into = None + into_raw = dict(design_post).get("into_experiment", "") + if is_id(into_raw): + from audits.models import Experiment + + into = Experiment.objects.filter(pk=int(into_raw), project=request.project).first() return render(request, "experiment_review.html", { "experiment_name": experiment_name, + "into_experiment": into, "columns": columns, "fixed": fixed, "rows": rows, @@ -1847,12 +1901,24 @@ def _launch(self, request): repeat = None errors.append(str(e)) name = post.get("experiment_name", "") + into = None + try: + into = ex.resolve_into_experiment(p, design.get("into_experiment")) + except ex.DesignError as e: + errors.append(str(e)) if not errors: try: - experiment = ex.launch_experiment(project=p, user=request.user, name=name, runs=runs, repeat=repeat) + experiment = ex.launch_experiment( + project=p, user=request.user, name=name, runs=runs, repeat=repeat, into=into, + confirm_public=design.get("into_confirm_public") == "1", + ) + except StableAPIError: + errors.append(_into_permission_error(into)) except Exception as e: # noqa: BLE001 - show the reason on the review screen errors.append(str(e)) else: + if into is not None and not repeat: + messages.success(request, f"{len(runs)} runs added to “{experiment.name}”.") if repeat: messages.success(request, f"Experiment launched; its {len(runs)} run setups now repeat {ex_repeat(repeat)}.") return redirect(f"/experiments/{experiment.id}/") @@ -1932,6 +1998,7 @@ def get_context_data(self, **kw): e.factor_labels = [FACTORS.get(f, f) for f in factors_for_runs(runs)] kw["experiments"] = experiments kw["can_manage_visibility"] = self._is_admin() + kw["can_edit_membership"] = _can_edit_membership(self.request.user, self.request.project) return super().get_context_data(**kw) def _is_admin(self) -> bool: @@ -3823,6 +3890,8 @@ def post(self, request, experiment_id): return HttpResponseForbidden("Workspace admin or auditor access required.") except MembershipError as e: messages.error(request, str(e)) + if request.POST.get("next") == "list": + return redirect("experiments") return redirect("experiment_detail", experiment_id) messages.success(request, f"Experiment “{name}” deleted. Its runs are kept.") return redirect("experiments") diff --git a/templates/experiment_new.html b/templates/experiment_new.html index dfa8dee6..25900c4d 100644 --- a/templates/experiment_new.html +++ b/templates/experiment_new.html @@ -145,6 +145,47 @@
Start a new experiment, or add these runs to an existing one to compare them with its runs.
+ {% with joining=rep.into_experiment %} + + +No experiment matches.
+Pick an experiment above.
+ + {% endwith %} +Adding to experiment
+{{ into_experiment.name }} + {% if into_experiment.is_public %}Public{% endif %} + · {{ into_experiment.memberships.count }} runs now
+ +Same in every run
diff --git a/templates/experiments.html b/templates/experiments.html index f1aa3c53..d9e6b395 100644 --- a/templates/experiments.html +++ b/templates/experiments.html @@ -18,6 +18,7 @@