From d6219f1fd252e32fa3088ca63ba36a10deba6572 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 06:49:13 -0700 Subject: [PATCH 01/16] wip: preserve experiment membership handoff Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d6d0638-e765-4142-b984-9fa2b494bb7f --- apps/api/src/experiment-membership.ts | 468 ++++++++++++++++++ .../src/experiments/experiment-store.ts | 157 +++++- packages/shared/src/experiments/schemas.ts | 5 +- packages/shared/src/experiments/types.ts | 5 +- packages/shared/src/schemas/request.ts | 2 + packages/shared/src/types/types.ts | 4 + 6 files changed, 636 insertions(+), 5 deletions(-) create mode 100644 apps/api/src/experiment-membership.ts diff --git a/apps/api/src/experiment-membership.ts b/apps/api/src/experiment-membership.ts new file mode 100644 index 00000000..27234bfd --- /dev/null +++ b/apps/api/src/experiment-membership.ts @@ -0,0 +1,468 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Collection, Document } from "mongodb"; +import { + ExperimentStore, + FixedPayloadStore, + fingerprintExperimentPayload, + type ExperimentDocument, + type ExperimentExecutionHistoryDocument, + type ExperimentMetadata, + type ExperimentRequestState, + type RequestDocument, +} from "shared"; + +export class ExperimentMembershipError extends Error { + constructor( + message: string, + readonly status: 400 | 404 | 409, + readonly code: string, + readonly conflicts?: Array<{ + requestId: string; + experimentId?: string; + }>, + ) { + super(message); + } +} + +export interface ExperimentMembershipCollections { + requests: Collection; + experiments: Collection; + executions: Collection; + chunks: Collection; +} + +export interface ReplaceMembershipResult { + experiment: ExperimentDocument; + attachedRequestIds: string[]; + detachedRequestIds: string[]; +} + +function uniqueCanonicalRequestIds(requestIds: readonly string[]): string[] { + const canonical = [...requestIds].sort(); + const duplicate = canonical.find( + (requestId, index) => canonical[index - 1] === requestId, + ); + if (duplicate) { + throw new ExperimentMembershipError( + `Request ${duplicate} was selected more than once`, + 400, + "duplicate-request-id", + [{ requestId: duplicate }], + ); + } + return canonical; +} + +export class ExperimentMembershipCoordinator { + private readonly store: ExperimentStore; + private readonly payloads: FixedPayloadStore; + + constructor(private readonly collections: ExperimentMembershipCollections) { + this.store = new ExperimentStore({ + experiments: collections.experiments, + executions: collections.executions, + chunks: collections.chunks, + }); + this.payloads = new FixedPayloadStore(collections.chunks); + } + + async createFromExisting(input: { + projectId: string; + experimentId: string; + experimentRevisionId: string; + experimentExecutionId: string; + metadata: ExperimentMetadata; + requestIds: readonly string[]; + now?: Date; + }): Promise { + const requestIds = uniqueCanonicalRequestIds(input.requestIds); + const documents = await this.loadRequestDocuments( + input.projectId, + requestIds, + ); + const now = input.now ?? new Date(); + const requests = requestIds.map((requestId) => { + const document = documents.get(requestId)!; + return { + requestId, + runId: document.run!._id, + dependencies: [], + blockers: [], + } satisfies ExperimentRequestState; + }); + for (const [index, request] of requests.entries()) { + const result = await this.collections.requests.updateOne( + { + _id: request.requestId, + projectId: input.projectId, + "run._id": request.runId, + deletedAt: { $exists: false }, + experimentId: { $exists: false }, + } as Document, + { + $set: { + experimentId: input.experimentId, + experimentExpectedRequestCount: requests.length, + experimentRequestIndex: index, + experimentInitialRunId: request.runId, + experimentMembershipOperationId: + input.experimentRevisionId, + experimentMembershipRevisionId: + input.experimentRevisionId, + }, + } as Document, + ); + if (result.matchedCount === 0) { + const conflict = await this.collections.requests.findOne( + { _id: request.requestId } as Document, + ); + throw new ExperimentMembershipError( + `Request ${request.requestId} could not be claimed`, + 409, + "request-membership-conflict", + [{ + requestId: request.requestId, + ...(conflict?.experimentId + ? { experimentId: conflict.experimentId } + : {}), + }], + ); + } + } + const pending = await this.store.createPending({ + experimentId: input.experimentId, + projectId: input.projectId, + metadata: input.metadata, + revision: { + experimentRevisionId: input.experimentRevisionId, + revisionNumber: 1, + memberCount: requests.length, + membershipFingerprint: + fingerprintExperimentPayload( + requests.map(({ requestId, runId }) => ({ + requestId, + runId, + })), + ), + definitionFingerprint: fingerprintExperimentPayload({ + metadata: input.metadata, + requestIds, + }), + createdAt: now, + }, + experimentExecutionId: input.experimentExecutionId, + requests, + now, + }); + const published = await this.store.finalizeCreation({ + projectId: input.projectId, + experimentId: input.experimentId, + expectedExperimentExecutionId: + input.experimentExecutionId, + expectedStateVersion: pending.stateVersion, + now, + }); + if (!published) { + throw new ExperimentMembershipError( + "Experiment creation could not be finalized", + 409, + "membership-publication-contention", + ); + } + await this.collections.requests.updateMany( + { + _id: { $in: requestIds }, + projectId: input.projectId, + experimentId: input.experimentId, + experimentMembershipOperationId: + input.experimentRevisionId, + } as Document, + { + $set: { experimentFinalizedAt: now }, + $unset: { experimentMembershipOperationId: "" }, + } as Document, + ); + return { + experiment: published, + attachedRequestIds: requestIds, + detachedRequestIds: [], + }; + } + + async replace(input: { + projectId: string; + experimentId: string; + expectedExperimentRevisionId: string; + requestIds: readonly string[]; + }): Promise { + const requestIds = uniqueCanonicalRequestIds(input.requestIds); + const current = await this.store.get(input.projectId, input.experimentId); + if (!current) { + throw new ExperimentMembershipError( + `Experiment ${input.experimentId} was not found`, + 404, + "experiment-not-found", + ); + } + if ( + current.revision.experimentRevisionId !== + input.expectedExperimentRevisionId + ) { + throw new ExperimentMembershipError( + `Experiment revision is stale; current revision is ${current.revision.experimentRevisionId}`, + 409, + "stale-experiment-revision", + ); + } + + const currentRequests = + (await this.store.readExecutionRequests({ + projectId: input.projectId, + experimentId: input.experimentId, + experimentExecutionId: + current.execution.experimentExecutionId, + })) ?? []; + const currentById = new Map( + currentRequests.map((request) => [request.requestId, request]), + ); + const documents = await this.loadRequestDocuments( + input.projectId, + requestIds, + input.experimentId, + ); + const nextRequests = requestIds.map((requestId) => { + const document = documents.get(requestId)!; + const previous = currentById.get(requestId); + if (previous?.runId === document.run!._id) return previous; + return { + requestId, + runId: document.run!._id, + dependencies: [], + blockers: [], + } satisfies ExperimentRequestState; + }); + const nextFingerprint = fingerprintExperimentPayload({ + experimentId: input.experimentId, + expectedExperimentRevisionId: input.expectedExperimentRevisionId, + requestIds, + runIds: nextRequests.map(({ requestId, runId }) => ({ + requestId, + runId, + })), + }); + const nextExperimentRevisionId = `er_v1_${nextFingerprint}`; + const nextExperimentExecutionId = `ee_v1_${nextFingerprint}`; + const claimed = await this.store.replaceMembership({ + projectId: input.projectId, + experimentId: input.experimentId, + expectedExperimentRevisionId: + input.expectedExperimentRevisionId, + expectedRevisionNumber: current.revision.revisionNumber, + expectedExperimentExecutionId: + current.execution.experimentExecutionId, + expectedExecutionVersion: current.execution.version, + nextExperimentRevisionId, + nextExperimentExecutionId, + requests: nextRequests, + deferPublication: true, + }); + if (!claimed) { + const latest = await this.store.get( + input.projectId, + input.experimentId, + ); + throw new ExperimentMembershipError( + `Experiment revision changed during membership replacement`, + 409, + "membership-contention", + latest + ? [{ + requestId: input.experimentId, + experimentId: latest.revision.experimentRevisionId, + }] + : undefined, + ); + } + return this.repair(input.projectId, input.experimentId); + } + + async repair( + projectId: string, + experimentId: string, + ): Promise { + const experiment = await this.store.get(projectId, experimentId); + const claim = experiment?.execution.rollover; + if (!experiment || claim?.reason !== "membership" || !claim.nextRevision) { + if (!experiment) { + throw new ExperimentMembershipError( + `Experiment ${experimentId} was not found`, + 404, + "experiment-not-found", + ); + } + return { + experiment, + attachedRequestIds: [], + detachedRequestIds: [], + }; + } + const nextRequests = await this.payloads.readRequests( + { + projectId, + experimentId, + ownerId: claim.nextExecution.experimentExecutionId, + generation: claim.nextExecution.attemptNumber, + }, + claim.nextExecution.requests, + ); + const currentRequests = + (await this.store.readExecutionRequests({ + projectId, + experimentId, + experimentExecutionId: + experiment.execution.experimentExecutionId, + })) ?? []; + const currentIds = new Set( + currentRequests.map(({ requestId }) => requestId), + ); + const nextIds = new Set(nextRequests.map(({ requestId }) => requestId)); + const attachedRequestIds = [...nextIds].filter( + (requestId) => !currentIds.has(requestId), + ); + const detachedRequestIds = [...currentIds].filter( + (requestId) => !nextIds.has(requestId), + ); + const operationId = claim.nextRevision.experimentRevisionId; + + for (const request of nextRequests) { + const result = await this.collections.requests.updateOne( + { + _id: request.requestId, + projectId, + "run._id": request.runId, + deletedAt: { $exists: false }, + $or: [ + { experimentId: { $exists: false } }, + { experimentId }, + ], + } as Document, + { + $set: { + experimentId, + experimentMembershipOperationId: operationId, + experimentMembershipRevisionId: operationId, + }, + } as Document, + ); + if (result.matchedCount === 0) { + const conflict = await this.collections.requests.findOne( + { _id: request.requestId } as Document, + ); + throw new ExperimentMembershipError( + `Request ${request.requestId} could not be claimed`, + 409, + "request-membership-conflict", + [{ + requestId: request.requestId, + ...(conflict?.experimentId + ? { experimentId: conflict.experimentId } + : {}), + }], + ); + } + } + + const published = await this.store.repairRollover( + projectId, + experimentId, + ); + if ( + !published || + published.revision.experimentRevisionId !== operationId + ) { + throw new ExperimentMembershipError( + `Experiment membership revision ${operationId} could not be published`, + 409, + "membership-publication-contention", + ); + } + if (detachedRequestIds.length > 0) { + await this.collections.requests.updateMany( + { + _id: { $in: detachedRequestIds }, + projectId, + experimentId, + } as Document, + { + $unset: { + experimentId: "", + experimentMembershipOperationId: "", + experimentMembershipRevisionId: "", + experimentFinalizedAt: "", + }, + } as Document, + ); + } + if (nextRequests.length > 0) { + await this.collections.requests.updateMany( + { + _id: { $in: nextRequests.map(({ requestId }) => requestId) }, + projectId, + experimentId, + experimentMembershipOperationId: operationId, + } as Document, + { + $set: { + experimentFinalizedAt: new Date(), + experimentMembershipRevisionId: operationId, + }, + $unset: { experimentMembershipOperationId: "" }, + } as Document, + ); + } + return { experiment: published, attachedRequestIds, detachedRequestIds }; + } + + private async loadRequestDocuments( + projectId: string, + requestIds: readonly string[], + allowedExperimentId?: string, + ): Promise> { + const documents = await this.collections.requests + .find({ + _id: { $in: [...requestIds] }, + deletedAt: { $exists: false }, + } as Document) + .toArray(); + const byId = new Map(documents.map((document) => [document._id, document])); + const conflicts: Array<{ requestId: string; experimentId?: string }> = []; + for (const requestId of requestIds) { + const document = byId.get(requestId); + if (!document || document.projectId !== projectId || !document.run) { + conflicts.push({ requestId }); + continue; + } + if ( + document.experimentId && + document.experimentId !== allowedExperimentId + ) { + conflicts.push({ + requestId, + experimentId: document.experimentId, + }); + } + } + if (conflicts.length > 0) { + throw new ExperimentMembershipError( + "One or more requests are missing, cross-project, archived-only, or already assigned", + 409, + "request-selection-conflict", + conflicts, + ); + } + return byId; + } +} diff --git a/packages/shared/src/experiments/experiment-store.ts b/packages/shared/src/experiments/experiment-store.ts index d209f6dc..c3e9af96 100644 --- a/packages/shared/src/experiments/experiment-store.ts +++ b/packages/shared/src/experiments/experiment-store.ts @@ -59,6 +59,20 @@ export interface RolloverExperimentInput { now?: Date; } +export interface ReplaceExperimentMembershipInput { + projectId: string; + experimentId: string; + expectedExperimentRevisionId: string; + expectedRevisionNumber: number; + expectedExperimentExecutionId: string; + expectedExecutionVersion: number; + nextExperimentRevisionId: string; + nextExperimentExecutionId: string; + requests: readonly ExperimentRequestState[]; + deferPublication?: boolean; + now?: Date; +} + function parseExperiment(value: unknown): ExperimentDocument { return ExperimentDocumentSchema.parse(value) as ExperimentDocument; } @@ -790,6 +804,7 @@ export class ExperimentStore { ) { return null; } + const requests = await this.payloads.readRequests( this.executionIdentity(experiment), experiment.execution.requests, @@ -797,6 +812,118 @@ export class ExperimentStore { return this.rollover({ ...input, requests, reason: "retry" }); } + async replaceMembership( + input: ReplaceExperimentMembershipInput, + ): Promise { + const experiment = await this.getRaw(input.projectId, input.experimentId); + if (!experiment) return null; + const canonicalRequests = input.requests + .map((request) => ExperimentRequestStateSchema.parse(request)) + .sort((left, right) => + left.requestId.localeCompare(right.requestId), + ) as ExperimentRequestState[]; + const membershipFingerprint = + fingerprintExperimentRequestVector(canonicalRequests); + const nextRevision = ExperimentRevisionSchema.parse({ + experimentRevisionId: input.nextExperimentRevisionId, + revisionNumber: input.expectedRevisionNumber + 1, + memberCount: canonicalRequests.length, + membershipFingerprint, + definitionFingerprint: fingerprintExperimentPayload({ + metadata: experiment.metadata, + requestIds: canonicalRequests.map(({ requestId }) => requestId), + }), + createdAt: input.now ?? new Date(), + }) as ExperimentRevision; + + const existingClaim = experiment.execution.rollover; + if (existingClaim?.reason === "membership") { + if ( + existingClaim.nextRevision?.experimentRevisionId !== + nextRevision.experimentRevisionId || + existingClaim.nextRevision.membershipFingerprint !== + nextRevision.membershipFingerprint || + existingClaim.nextExecution.experimentExecutionId !== + input.nextExperimentExecutionId + ) { + return null; + } + return input.deferPublication + ? experiment + : this.repairRollover(input.projectId, input.experimentId); + } + if ( + experiment.revision.experimentRevisionId !== + input.expectedExperimentRevisionId || + experiment.revision.revisionNumber !== input.expectedRevisionNumber || + experiment.execution.experimentExecutionId !== + input.expectedExperimentExecutionId || + experiment.execution.version !== input.expectedExecutionVersion + ) { + return null; + } + + const now = input.now ?? new Date(); + const payload = await this.payloads.prepareRequests({ + identity: { + projectId: input.projectId, + experimentId: input.experimentId, + ownerId: input.nextExperimentExecutionId, + generation: 1, + }, + entries: canonicalRequests, + inlineDocument: (candidate) => ({ + ...experiment, + revision: nextRevision, + execution: this.membershipExecution( + input.nextExperimentExecutionId, + candidate, + canonicalRequests, + now, + ), + }), + now, + }); + const nextExecution = this.membershipExecution( + input.nextExperimentExecutionId, + payload, + canonicalRequests, + now, + ); + const claimed = await this.collections.experiments.findOneAndUpdate( + { + _id: input.experimentId, + projectId: input.projectId, + "revision.experimentRevisionId": + input.expectedExperimentRevisionId, + "revision.revisionNumber": input.expectedRevisionNumber, + "execution.experimentExecutionId": + input.expectedExperimentExecutionId, + "execution.version": input.expectedExecutionVersion, + "execution.rollover": { $exists: false }, + } as Document, + { + $set: { + "execution.frozenAt": now, + "execution.rollover": { + nextExecution, + nextRevision, + reason: "membership", + claimedAt: now, + }, + "execution.updatedAt": now, + updatedAt: now, + }, + $inc: { "execution.version": 1, stateVersion: 1 }, + } as Document, + { returnDocument: "after" }, + ); + if (!claimed) return null; + return input.deferPublication + ? parseExperiment(claimed) + : this.repairRollover(input.projectId, input.experimentId); + } + async discardStale(input: { projectId: string; experimentId: string; @@ -838,6 +965,11 @@ export class ExperimentStore { const rollover = experiment?.execution.rollover; if (!experiment || !rollover) return experiment; const next = parseExecution(rollover.nextExecution); + const nextRevision = rollover.nextRevision + ? (ExperimentRevisionSchema.parse( + rollover.nextRevision, + ) as ExperimentRevision) + : undefined; await this.payloads.verifyCompleteSet( { projectId, @@ -859,7 +991,11 @@ export class ExperimentStore { next.experimentExecutionId, } as Document, { - $set: { execution: next, updatedAt: now }, + $set: { + execution: next, + ...(nextRevision ? { revision: nextRevision } : {}), + updatedAt: now, + }, $inc: { stateVersion: 1 }, } as Document, { returnDocument: "after" }, @@ -908,6 +1044,25 @@ export class ExperimentStore { }; } + private membershipExecution( + experimentExecutionId: string, + requests: FixedPayload, + entries: ExperimentRequestState[], + now: Date, + ): ExperimentExecution { + return { + experimentExecutionId, + attemptNumber: 1, + version: 1, + requests, + vectorFingerprint: fingerprintExperimentRequestVector(entries), + handlerSummary: {}, + blocking: [], + createdAt: now, + updatedAt: now, + }; + } + private async archive( experiment: ExperimentDocument, archivedAt: Date, diff --git a/packages/shared/src/experiments/schemas.ts b/packages/shared/src/experiments/schemas.ts index 0512e6f0..1ea7c50b 100644 --- a/packages/shared/src/experiments/schemas.ts +++ b/packages/shared/src/experiments/schemas.ts @@ -139,7 +139,7 @@ const ExperimentExecutionBaseSchema = z discardReason: z.string().max(2_048).optional(), supersededAt: z.coerce.date().optional(), supersededByExperimentExecutionId: IdSchema.optional(), - supersededReason: z.enum(["rollover", "retry"]).optional(), + supersededReason: z.enum(["rollover", "retry", "membership"]).optional(), requestStateUpdate: z .object({ updateId: IdSchema, @@ -157,7 +157,8 @@ export const ExperimentExecutionSchema: z.ZodType = ExperimentExecutionBaseSchem rollover: z .object({ nextExecution: ExperimentExecutionBaseSchema, - reason: z.enum(["rollover", "retry"]), + nextRevision: ExperimentRevisionSchema.optional(), + reason: z.enum(["rollover", "retry", "membership"]), claimedAt: z.coerce.date(), }) .strict() diff --git a/packages/shared/src/experiments/types.ts b/packages/shared/src/experiments/types.ts index 8f780525..3c125186 100644 --- a/packages/shared/src/experiments/types.ts +++ b/packages/shared/src/experiments/types.ts @@ -99,7 +99,8 @@ export interface ExperimentHandlerSummary { export interface ExperimentRolloverClaim { nextExecution: ExperimentExecution; - reason: "rollover" | "retry"; + nextRevision?: ExperimentRevision; + reason: "rollover" | "retry" | "membership"; claimedAt: Date; } @@ -134,7 +135,7 @@ export interface ExperimentExecution { discardReason?: string; supersededAt?: Date; supersededByExperimentExecutionId?: string; - supersededReason?: "rollover" | "retry"; + supersededReason?: "rollover" | "retry" | "membership"; requestStateUpdate?: ExperimentRequestStateUpdateClaim; rollover?: ExperimentRolloverClaim; } diff --git a/packages/shared/src/schemas/request.ts b/packages/shared/src/schemas/request.ts index bf4c7574..f721630a 100644 --- a/packages/shared/src/schemas/request.ts +++ b/packages/shared/src/schemas/request.ts @@ -193,6 +193,8 @@ export const RequestResponseSchema = z experimentRequestIndex: z.number().int().min(0).optional(), experimentInitialRunId: z.string().optional(), experimentFinalizedAt: z.coerce.date().optional(), + experimentMembershipOperationId: z.string().optional(), + experimentMembershipRevisionId: z.string().optional(), priority: z.number().int().default(0), gates: z.array(GateConfigSchema).optional(), observations: z.array(z.string()).optional(), diff --git a/packages/shared/src/types/types.ts b/packages/shared/src/types/types.ts index 28f5b569..88028af2 100644 --- a/packages/shared/src/types/types.ts +++ b/packages/shared/src/types/types.ts @@ -398,6 +398,10 @@ export interface RequestDocument { /** Set after the parent ExperimentDocument has been idempotently finalized. * Missing on interrupted native writes so the scheduler can recover them. */ experimentFinalizedAt?: Date; + /** Recoverable membership mutation that owns this denormalized assignment. */ + experimentMembershipOperationId?: string; + /** Experiment revision that most recently claimed this request. */ + experimentMembershipRevisionId?: string; /** Scheduling priority. Higher = processed first. Default: 0. */ priority: number; /** From fe5695d0b09748e9dc16b892eb92167811b80e09 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:12:17 -0700 Subject: [PATCH 02/16] fix: repair membership before publication Ensure interrupted membership rollovers claim request ownership before publishing the next experiment revision, clean up failed unpublished claims, and include membership repair in coordinator polling.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/experiment-coordinator.ts | 36 ++++- apps/api/src/experiment-dispatcher.ts | 1 + apps/api/src/experiment-membership.ts | 187 +++++++++++++++---------- 3 files changed, 150 insertions(+), 74 deletions(-) diff --git a/apps/api/src/experiment-coordinator.ts b/apps/api/src/experiment-coordinator.ts index 50dd0685..dc3cfa12 100644 --- a/apps/api/src/experiment-coordinator.ts +++ b/apps/api/src/experiment-coordinator.ts @@ -19,6 +19,7 @@ import { type RequestDocument, type ServiceHandlerDocument, } from "shared"; +import { ExperimentMembershipCoordinator } from "./experiment-membership.js"; export interface ExperimentCoordinatorCollections { requests: Collection; @@ -129,6 +130,7 @@ function sameProjection( export class ExperimentCoordinator { private readonly store: ExperimentStore; private readonly payloads: FixedPayloadStore; + private readonly membership: ExperimentMembershipCoordinator; constructor( private readonly collections: ExperimentCoordinatorCollections, @@ -140,6 +142,7 @@ export class ExperimentCoordinator { chunks: collections.chunks, }); this.payloads = new FixedPayloadStore(collections.chunks); + this.membership = new ExperimentMembershipCoordinator(collections); } async reconcile( @@ -151,9 +154,18 @@ export class ExperimentCoordinator { let experiment = await this.store.get(projectId, experimentId); if (!experiment) return null; if (experiment.execution.rollover) { - const repaired = await this.store.repairRollover(projectId, experimentId); - if (!repaired) return null; - experiment = repaired; + if (experiment.execution.rollover.reason === "membership") { + experiment = ( + await this.membership.repair(projectId, experimentId) + ).experiment; + } else { + const repaired = await this.store.repairRollover( + projectId, + experimentId, + ); + if (!repaired) return null; + experiment = repaired; + } rolled = true; } @@ -351,6 +363,24 @@ export class ExperimentCoordinator { return recovered; } + async recoverInterruptedMemberships(limit = 30): Promise { + const candidates = await this.collections.experiments + .find({ + finalizedAt: { $exists: true }, + "execution.rollover.reason": "membership", + } as Document) + .sort({ _id: 1 }) + .limit(limit) + .project({ _id: 1, projectId: 1 }) + .toArray(); + let recovered = 0; + for (const candidate of candidates) { + await this.membership.repair(candidate.projectId, candidate._id); + recovered += 1; + } + return recovered; + } + private async readRequests( experiment: ExperimentDocument, ): Promise { diff --git a/apps/api/src/experiment-dispatcher.ts b/apps/api/src/experiment-dispatcher.ts index 4e0df809..51e9181c 100644 --- a/apps/api/src/experiment-dispatcher.ts +++ b/apps/api/src/experiment-dispatcher.ts @@ -122,6 +122,7 @@ export class ExperimentDispatcher { this.dispatching = true; try { await this.coordinator.recoverInterruptedCreations(this.batchSize); + await this.coordinator.recoverInterruptedMemberships(this.batchSize); const candidates = await this.experiments .find( this.scanAfterExperimentId diff --git a/apps/api/src/experiment-membership.ts b/apps/api/src/experiment-membership.ts index 27234bfd..5941e759 100644 --- a/apps/api/src/experiment-membership.ts +++ b/apps/api/src/experiment-membership.ts @@ -93,44 +93,53 @@ export class ExperimentMembershipCoordinator { blockers: [], } satisfies ExperimentRequestState; }); - for (const [index, request] of requests.entries()) { - const result = await this.collections.requests.updateOne( - { - _id: request.requestId, - projectId: input.projectId, - "run._id": request.runId, - deletedAt: { $exists: false }, - experimentId: { $exists: false }, - } as Document, - { - $set: { - experimentId: input.experimentId, - experimentExpectedRequestCount: requests.length, - experimentRequestIndex: index, - experimentInitialRunId: request.runId, - experimentMembershipOperationId: - input.experimentRevisionId, - experimentMembershipRevisionId: - input.experimentRevisionId, - }, - } as Document, - ); - if (result.matchedCount === 0) { - const conflict = await this.collections.requests.findOne( - { _id: request.requestId } as Document, - ); - throw new ExperimentMembershipError( - `Request ${request.requestId} could not be claimed`, - 409, - "request-membership-conflict", - [{ - requestId: request.requestId, - ...(conflict?.experimentId - ? { experimentId: conflict.experimentId } - : {}), - }], + try { + for (const [index, request] of requests.entries()) { + const result = await this.collections.requests.updateOne( + { + _id: request.requestId, + projectId: input.projectId, + "run._id": request.runId, + deletedAt: { $exists: false }, + experimentId: { $exists: false }, + } as Document, + { + $set: { + experimentId: input.experimentId, + experimentExpectedRequestCount: requests.length, + experimentRequestIndex: index, + experimentInitialRunId: request.runId, + experimentMembershipOperationId: + input.experimentRevisionId, + experimentMembershipRevisionId: + input.experimentRevisionId, + }, + } as Document, ); + if (result.matchedCount === 0) { + const conflict = await this.collections.requests.findOne( + { _id: request.requestId } as Document, + ); + throw new ExperimentMembershipError( + `Request ${request.requestId} could not be claimed`, + 409, + "request-membership-conflict", + [{ + requestId: request.requestId, + ...(conflict?.experimentId + ? { experimentId: conflict.experimentId } + : {}), + }], + ); + } } + } catch (error) { + await this.releaseUnpublishedClaims({ + projectId: input.projectId, + experimentId: input.experimentId, + operationId: input.experimentRevisionId, + }); + throw error; } const pending = await this.store.createPending({ experimentId: input.experimentId, @@ -337,42 +346,54 @@ export class ExperimentMembershipCoordinator { ); const operationId = claim.nextRevision.experimentRevisionId; - for (const request of nextRequests) { - const result = await this.collections.requests.updateOne( - { - _id: request.requestId, - projectId, - "run._id": request.runId, - deletedAt: { $exists: false }, - $or: [ - { experimentId: { $exists: false } }, - { experimentId }, - ], - } as Document, - { - $set: { - experimentId, - experimentMembershipOperationId: operationId, - experimentMembershipRevisionId: operationId, - }, - } as Document, - ); - if (result.matchedCount === 0) { - const conflict = await this.collections.requests.findOne( - { _id: request.requestId } as Document, - ); - throw new ExperimentMembershipError( - `Request ${request.requestId} could not be claimed`, - 409, - "request-membership-conflict", - [{ - requestId: request.requestId, - ...(conflict?.experimentId - ? { experimentId: conflict.experimentId } - : {}), - }], + try { + for (const request of nextRequests) { + if (currentIds.has(request.requestId)) continue; + const result = await this.collections.requests.updateOne( + { + _id: request.requestId, + projectId, + "run._id": request.runId, + deletedAt: { $exists: false }, + $or: [ + { experimentId: { $exists: false } }, + { + experimentId, + experimentMembershipOperationId: operationId, + }, + ], + } as Document, + { + $set: { + experimentId, + experimentMembershipOperationId: operationId, + }, + } as Document, ); + if (result.matchedCount === 0) { + const conflict = await this.collections.requests.findOne( + { _id: request.requestId } as Document, + ); + throw new ExperimentMembershipError( + `Request ${request.requestId} could not be claimed`, + 409, + "request-membership-conflict", + [{ + requestId: request.requestId, + ...(conflict?.experimentId + ? { experimentId: conflict.experimentId } + : {}), + }], + ); + } } + } catch (error) { + await this.releaseUnpublishedClaims({ + projectId, + experimentId, + operationId, + }); + throw error; } const published = await this.store.repairRollover( @@ -412,7 +433,6 @@ export class ExperimentMembershipCoordinator { _id: { $in: nextRequests.map(({ requestId }) => requestId) }, projectId, experimentId, - experimentMembershipOperationId: operationId, } as Document, { $set: { @@ -426,6 +446,31 @@ export class ExperimentMembershipCoordinator { return { experiment: published, attachedRequestIds, detachedRequestIds }; } + private async releaseUnpublishedClaims(input: { + projectId: string; + experimentId: string; + operationId: string; + }): Promise { + await this.collections.requests.updateMany( + { + projectId: input.projectId, + experimentId: input.experimentId, + experimentMembershipOperationId: input.operationId, + } as Document, + { + $unset: { + experimentId: "", + experimentMembershipOperationId: "", + experimentMembershipRevisionId: "", + experimentExpectedRequestCount: "", + experimentRequestIndex: "", + experimentInitialRunId: "", + experimentFinalizedAt: "", + }, + } as Document, + ); + } + private async loadRequestDocuments( projectId: string, requestIds: readonly string[], From e738ed628d7a3b780f64c7d8a473005ebd2ee545 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:23:48 -0700 Subject: [PATCH 03/16] feat: expose experiment membership revisions Create Experiments from existing standalone requests and add or remove multiple members through a single complete-set revision replacement with exact conflict reporting.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/experiment-membership.test.ts | 186 +++++++++++++++++++++ apps/api/src/experiment-membership.ts | 93 +++++++++++ apps/api/src/routes/experiments.test.ts | 1 + apps/api/src/routes/experiments.ts | 158 ++++++++++++++++- 4 files changed, 433 insertions(+), 5 deletions(-) create mode 100644 apps/api/src/experiment-membership.test.ts diff --git a/apps/api/src/experiment-membership.test.ts b/apps/api/src/experiment-membership.test.ts new file mode 100644 index 00000000..3f2efcc0 --- /dev/null +++ b/apps/api/src/experiment-membership.test.ts @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Collection } from "mongodb"; +import { + EXPERIMENT_STORAGE_PROFILE_VERSION, + ExperimentStore, + fingerprintExperimentPayload, + type ExperimentDocument, + type ExperimentExecutionHistoryDocument, + type ExperimentRequestState, + type RequestDocument, +} from "shared"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + ExperimentMembershipCoordinator, + ExperimentMembershipError, +} from "./experiment-membership.js"; + +const now = new Date("2026-10-16T00:00:00.000Z"); +const request = ( + requestId: string, + runId: string, +): ExperimentRequestState => ({ + requestId, + runId, + dependencies: [], + blockers: [], +}); + +function experiment(requests: ExperimentRequestState[]): ExperimentDocument { + const fingerprint = fingerprintExperimentPayload(requests); + return { + _id: "experiment-1", + experimentId: "experiment-1", + projectId: "project-1", + metadata: { name: "Experiment" }, + storageProfileVersion: EXPERIMENT_STORAGE_PROFILE_VERSION, + stateVersion: 1, + revision: { + experimentRevisionId: "revision-1", + revisionNumber: 1, + memberCount: requests.length, + membershipFingerprint: fingerprint, + definitionFingerprint: "definition-1", + createdAt: now, + }, + execution: { + experimentExecutionId: "execution-1", + attemptNumber: 1, + version: 1, + requests: { + storage: "inline", + entryCount: requests.length, + payloadFingerprint: fingerprint, + payloadVersion: 1, + entries: requests, + }, + vectorFingerprint: fingerprint, + handlerSummary: {}, + blocking: [], + createdAt: now, + updatedAt: now, + }, + createdAt: now, + updatedAt: now, + finalizedAt: now, + }; +} + +function coordinator(): ExperimentMembershipCoordinator { + return new ExperimentMembershipCoordinator({ + requests: {} as Collection, + experiments: {} as Collection, + executions: {} as Collection, + chunks: {} as Collection, + }); +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("ExperimentMembershipCoordinator complete-set operations", () => { + it("adds several requests through one complete-set replacement", async () => { + const current = experiment([request("request-1", "run-1")]); + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue(current); + vi.spyOn( + ExperimentStore.prototype, + "readExecutionRequests", + ).mockResolvedValue([request("request-1", "run-1")]); + const membership = coordinator(); + const replace = vi.spyOn(membership, "replace").mockResolvedValue({ + experiment: current, + attachedRequestIds: ["request-2", "request-3"], + detachedRequestIds: [], + }); + + await membership.add({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-3", "request-2"], + }); + + expect(replace).toHaveBeenCalledOnce(); + expect(replace).toHaveBeenCalledWith({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1", "request-2", "request-3"], + }); + }); + + it("removes the final member through one visible empty revision", async () => { + const current = experiment([request("request-1", "run-1")]); + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue(current); + vi.spyOn( + ExperimentStore.prototype, + "readExecutionRequests", + ).mockResolvedValue([request("request-1", "run-1")]); + const membership = coordinator(); + const replace = vi.spyOn(membership, "replace").mockResolvedValue({ + experiment: current, + attachedRequestIds: [], + detachedRequestIds: ["request-1"], + }); + + await membership.remove({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + }); + + expect(replace).toHaveBeenCalledWith({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: [], + }); + }); + + it("reports exact request and owner conflicts for duplicate attachment", async () => { + const current = experiment([request("request-1", "run-1")]); + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue(current); + vi.spyOn( + ExperimentStore.prototype, + "readExecutionRequests", + ).mockResolvedValue([request("request-1", "run-1")]); + + await expect( + coordinator().add({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + }), + ).rejects.toMatchObject>({ + status: 409, + code: "request-selection-conflict", + conflicts: [{ + requestId: "request-1", + experimentId: "experiment-1", + }], + }); + }); + + it("rejects a stale expected revision before planning membership", async () => { + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue( + experiment([request("request-1", "run-1")]), + ); + + await expect( + coordinator().remove({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "stale-revision", + requestIds: ["request-1"], + }), + ).rejects.toMatchObject>({ + status: 409, + code: "stale-experiment-revision", + }); + }); +}); diff --git a/apps/api/src/experiment-membership.ts b/apps/api/src/experiment-membership.ts index 5941e759..66c10f44 100644 --- a/apps/api/src/experiment-membership.ts +++ b/apps/api/src/experiment-membership.ts @@ -298,6 +298,64 @@ export class ExperimentMembershipCoordinator { return this.repair(input.projectId, input.experimentId); } + async add(input: { + projectId: string; + experimentId: string; + expectedExperimentRevisionId: string; + requestIds: readonly string[]; + }): Promise { + const selected = uniqueCanonicalRequestIds(input.requestIds); + const current = await this.currentMembership(input); + const currentIds = new Set(current.requestIds); + const conflicts = selected + .filter((requestId) => currentIds.has(requestId)) + .map((requestId) => ({ + requestId, + experimentId: input.experimentId, + })); + if (conflicts.length > 0) { + throw new ExperimentMembershipError( + "One or more requests are already members of the target Experiment", + 409, + "request-selection-conflict", + conflicts, + ); + } + return this.replace({ + ...input, + requestIds: [...current.requestIds, ...selected], + }); + } + + async remove(input: { + projectId: string; + experimentId: string; + expectedExperimentRevisionId: string; + requestIds: readonly string[]; + }): Promise { + const selected = uniqueCanonicalRequestIds(input.requestIds); + const current = await this.currentMembership(input); + const selectedIds = new Set(selected); + const currentIds = new Set(current.requestIds); + const conflicts = selected + .filter((requestId) => !currentIds.has(requestId)) + .map((requestId) => ({ requestId })); + if (conflicts.length > 0) { + throw new ExperimentMembershipError( + "One or more requests are not current members of the target Experiment", + 409, + "request-selection-conflict", + conflicts, + ); + } + return this.replace({ + ...input, + requestIds: current.requestIds.filter( + (requestId) => !selectedIds.has(requestId), + ), + }); + } + async repair( projectId: string, experimentId: string, @@ -510,4 +568,39 @@ export class ExperimentMembershipCoordinator { } return byId; } + + private async currentMembership(input: { + projectId: string; + experimentId: string; + expectedExperimentRevisionId: string; + }): Promise<{ requestIds: string[] }> { + const current = await this.store.get(input.projectId, input.experimentId); + if (!current) { + throw new ExperimentMembershipError( + `Experiment ${input.experimentId} was not found`, + 404, + "experiment-not-found", + ); + } + if ( + current.revision.experimentRevisionId !== + input.expectedExperimentRevisionId + ) { + throw new ExperimentMembershipError( + `Experiment revision is stale; current revision is ${current.revision.experimentRevisionId}`, + 409, + "stale-experiment-revision", + ); + } + const requests = + (await this.store.readExecutionRequests({ + projectId: input.projectId, + experimentId: input.experimentId, + experimentExecutionId: + current.execution.experimentExecutionId, + })) ?? []; + return { + requestIds: requests.map(({ requestId }) => requestId), + }; + } } diff --git a/apps/api/src/routes/experiments.test.ts b/apps/api/src/routes/experiments.test.ts index 7579f648..fd854dc1 100644 --- a/apps/api/src/routes/experiments.test.ts +++ b/apps/api/src/routes/experiments.test.ts @@ -257,6 +257,7 @@ describe("Experiment API contract", () => { "/api/v1/experiments/{id}/executions/{experimentExecutionId}/logs", "/api/v1/experiments/{id}/executions/{experimentExecutionId}/reports", "/api/v1/experiments/{id}/executions/{experimentExecutionId}/reports/{reportId}/structured", + "/api/v1/experiments/{id}/requests", "/api/v1/experiments/{id}/retry", ]); expect(document.paths).not.toHaveProperty("/api/v1/submissions"); diff --git a/apps/api/src/routes/experiments.ts b/apps/api/src/routes/experiments.ts index 88b29539..e53627d4 100644 --- a/apps/api/src/routes/experiments.ts +++ b/apps/api/src/routes/experiments.ts @@ -25,6 +25,11 @@ import { } from "../utils/project-scope.js"; import { subscribeClient, unsubscribeClient } from "../utils/sse.js"; import type { SSEClient } from "../utils/sse.js"; +import { + ExperimentMembershipCoordinator, + ExperimentMembershipError, + type ReplaceMembershipResult, +} from "../experiment-membership.js"; const CursorSchema = z.string().min(1).optional(); const PageQuerySchema = ProjectIdQuerySchema.extend({ @@ -46,23 +51,40 @@ const ExperimentRequestInputSchema = CreateRequestInputSchema.omit({ const CreateExperimentInputSchema = z .object({ - mode: z.enum(["compose", "empty"]), + mode: z.enum(["compose", "empty", "existing"]), metadata: ExperimentMetadataInputSchema, requests: z.array(ExperimentRequestInputSchema).max(250_000).default([]), + requestIds: z.array(z.string().min(1)).max(250_000).default([]), }) .superRefine((value, ctx) => { - if (value.mode === "empty" && value.requests.length !== 0) { + if ( + value.mode === "empty" && + (value.requests.length !== 0 || value.requestIds.length !== 0) + ) { ctx.addIssue({ code: "custom", path: ["requests"], - message: "empty mode requires an empty requests array", + message: "empty mode requires no requests or requestIds", }); } - if (value.mode === "compose" && value.requests.length === 0) { + if ( + value.mode === "compose" && + (value.requests.length === 0 || value.requestIds.length !== 0) + ) { ctx.addIssue({ code: "custom", path: ["requests"], - message: "compose mode requires at least one request", + message: "compose mode requires new requests and no requestIds", + }); + } + if ( + value.mode === "existing" && + (value.requestIds.length === 0 || value.requests.length !== 0) + ) { + ctx.addIssue({ + code: "custom", + path: ["requestIds"], + message: "existing mode requires requestIds and no new requests", }); } }) @@ -196,6 +218,22 @@ const RetryExperimentResponseSchema = z }) .openapi("RetryExperimentResponse"); +const MembershipInputSchema = z + .object({ + expectedExperimentRevisionId: z.string().min(1), + requestIds: z.array(z.string().min(1)).min(1).max(250_000), + }) + .strict() + .openapi("ExperimentMembershipInput"); + +const MembershipResponseSchema = z + .object({ + experiment: ExperimentResponseSchema, + attachedRequestIds: z.array(z.string()), + detachedRequestIds: z.array(z.string()), + }) + .openapi("ExperimentMembershipResponse"); + type Readiness = z.infer; function store(ctx: RouteContext): ExperimentStore { @@ -206,6 +244,36 @@ function store(ctx: RouteContext): ExperimentStore { }); } +function membership(ctx: RouteContext): ExperimentMembershipCoordinator { + return new ExperimentMembershipCoordinator({ + requests: ctx.requestCollection, + experiments: ctx.experimentCollection, + executions: ctx.experimentExecutionsCollection, + chunks: ctx.experimentChunksCollection, + }); +} + +function membershipResponse(result: ReplaceMembershipResult) { + return { + experiment: experimentResponse(result.experiment), + attachedRequestIds: result.attachedRequestIds, + detachedRequestIds: result.detachedRequestIds, + }; +} + +function sendMembershipError( + res: Response, + error: unknown, +): boolean { + if (!(error instanceof ExperimentMembershipError)) return false; + res.status(error.status).json({ + error: error.message, + code: error.code, + ...(error.conflicts ? { conflicts: error.conflicts } : {}), + }); + return true; +} + function readiness(execution: ExperimentExecution): Readiness { if (execution.requests.entryCount === 0) return "empty"; if (execution.discardedAt) return "failed"; @@ -533,6 +601,8 @@ export function registerExperimentsRoutes(ctx: RouteContext): void { body: CreateExperimentInputSchema, response: CreateExperimentResponseSchema, errorResponses: { + 400: { description: "Invalid request selection" }, + 404: { description: "Request not found" }, 409: { description: "Experiment identity conflict" }, }, handler: async (req, res) => { @@ -541,6 +611,26 @@ export function registerExperimentsRoutes(ctx: RouteContext): void { const experimentExecutionId = uuidv4(); const experimentRevisionId = uuidv4(); const now = new Date(); + if (req.body.mode === "existing") { + try { + const result = await membership(ctx).createFromExisting({ + projectId, + experimentId, + experimentExecutionId, + experimentRevisionId, + metadata: req.body.metadata, + requestIds: req.body.requestIds, + now, + }); + res.status(201).json({ + experiment: experimentResponse(result.experiment), + requestIds: result.attachedRequestIds, + }); + } catch (error) { + if (!sendMembershipError(res, error)) throw error; + } + return; + } const identities = req.body.requests.map(() => ({ requestId: uuidv4(), runId: uuidv4(), @@ -630,6 +720,64 @@ export function registerExperimentsRoutes(ctx: RouteContext): void { }, }); + apiRoute(ctx.app, ctx.registry, { + method: "post", + path: "/api/v1/experiments/:id/requests", + tags: ["Experiments"], + summary: "Move standalone requests into an Experiment", + params: z.object({ id: z.string().min(1) }), + query: ProjectIdQuerySchema, + body: MembershipInputSchema, + response: MembershipResponseSchema, + errorResponses: { + 404: { description: "Experiment or request not found" }, + 409: { description: "Stale revision or membership conflict" }, + }, + handler: async (req, res) => { + try { + const result = await membership(ctx).add({ + projectId: getQueryProjectId(req), + experimentId: req.params.id, + expectedExperimentRevisionId: + req.body.expectedExperimentRevisionId, + requestIds: req.body.requestIds, + }); + res.status(201).json(membershipResponse(result)); + } catch (error) { + if (!sendMembershipError(res, error)) throw error; + } + }, + }); + + apiRoute(ctx.app, ctx.registry, { + method: "delete", + path: "/api/v1/experiments/:id/requests", + tags: ["Experiments"], + summary: "Remove requests from an Experiment", + params: z.object({ id: z.string().min(1) }), + query: ProjectIdQuerySchema, + body: MembershipInputSchema, + response: MembershipResponseSchema, + errorResponses: { + 404: { description: "Experiment or request not found" }, + 409: { description: "Stale revision or membership conflict" }, + }, + handler: async (req, res) => { + try { + const result = await membership(ctx).remove({ + projectId: getQueryProjectId(req), + experimentId: req.params.id, + expectedExperimentRevisionId: + req.body.expectedExperimentRevisionId, + requestIds: req.body.requestIds, + }); + res.status(201).json(membershipResponse(result)); + } catch (error) { + if (!sendMembershipError(res, error)) throw error; + } + }, + }); + apiRoute(ctx.app, ctx.registry, { method: "get", path: "/api/v1/experiments", From 516725b7a6a291e4fe040cdb64d11ede73135289 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:33:52 -0700 Subject: [PATCH 04/16] feat: coordinate experiment member deletion Publish one membership revision before deleting selected members, surface the resulting revision identity, and reject bulk deletion across multiple Experiments to avoid partial success.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/endpoints.test.ts | 4 + apps/api/src/routes/requests/index.ts | 188 ++++++++++++++++++++++++-- 2 files changed, 177 insertions(+), 15 deletions(-) diff --git a/apps/api/src/endpoints.test.ts b/apps/api/src/endpoints.test.ts index f3c93c4f..f92d9c5a 100644 --- a/apps/api/src/endpoints.test.ts +++ b/apps/api/src/endpoints.test.ts @@ -1137,6 +1137,10 @@ describe("API Endpoints", () => { describe("DELETE /api/v1/requests/:id", () => { it("returns 200 when request is soft-deleted", async () => { + (mocks.collection.findOne as any).mockResolvedValue({ + _id: "r1", + projectId: TEST_PROJECT_ID, + }); (mocks.collection.updateOne as any).mockResolvedValue({ matchedCount: 1, modifiedCount: 1 }); const res = await request(testServer()).delete("/api/v1/requests/r1"); diff --git a/apps/api/src/routes/requests/index.ts b/apps/api/src/routes/requests/index.ts index 0b5d2df8..0d5fb666 100644 --- a/apps/api/src/routes/requests/index.ts +++ b/apps/api/src/routes/requests/index.ts @@ -37,6 +37,7 @@ import { isCriterionCompatibleWithGate, orderGates, computeTaskPromptId, + ExperimentStore, LEGACY_REQUEST_COMPOSITION_DEPRECATION_ID, } from "shared"; import { trackMetric } from "telemetry"; @@ -85,6 +86,10 @@ import { requestedAgentCapabilities, validateAgentTarget, } from "../../utils/agent-helpers.js"; +import { + ExperimentMembershipCoordinator, + ExperimentMembershipError, +} from "../../experiment-membership.js"; type RequestCollection = RouteContext["requestCollection"]; type RunFacetsResponse = z.infer; @@ -218,6 +223,83 @@ export function planResubmitResources( export function registerRequestsRoutes(ctx: RouteContext): void { +const experimentStore = new ExperimentStore({ + experiments: ctx.experimentCollection, + executions: ctx.experimentExecutionsCollection, + chunks: ctx.experimentChunksCollection, +}); +const experimentMembership = new ExperimentMembershipCoordinator({ + requests: ctx.requestCollection, + experiments: ctx.experimentCollection, + executions: ctx.experimentExecutionsCollection, + chunks: ctx.experimentChunksCollection, +}); + +const coordinatedDeletionRevision = async ( + documents: RequestDocument[], +): Promise< + | { + experimentId: string; + experimentRevisionId: string; + revisionNumber: number; + } + | undefined +> => { + const assigned = documents.filter( + (document): document is RequestDocument & { experimentId: string } => + typeof document.experimentId === "string", + ); + if (assigned.length === 0) return undefined; + const experimentIds = [...new Set(assigned.map(({ experimentId }) => experimentId))]; + if (experimentIds.length !== 1) { + throw new ExperimentMembershipError( + "Bulk deletion cannot coordinate members from multiple Experiments; delete one Experiment selection at a time", + 409, + "multiple-experiment-delete-conflict", + assigned.map(({ _id, experimentId }) => ({ + requestId: _id, + experimentId, + })), + ); + } + const experimentId = experimentIds[0]!; + const projectIds = [...new Set(assigned.map(({ projectId }) => projectId))]; + if (projectIds.length !== 1) { + throw new ExperimentMembershipError( + "Experiment members must belong to one project", + 409, + "cross-project-membership-conflict", + assigned.map(({ _id, experimentId: owner }) => ({ + requestId: _id, + experimentId: owner, + })), + ); + } + const projectId = projectIds[0]!; + const experiment = await experimentStore.get(projectId, experimentId); + if (!experiment) { + throw new ExperimentMembershipError( + `Owning Experiment ${experimentId} was not found`, + 409, + "experiment-membership-owner-missing", + assigned.map(({ _id }) => ({ requestId: _id, experimentId })), + ); + } + const result = await experimentMembership.remove({ + projectId, + experimentId, + expectedExperimentRevisionId: + experiment.revision.experimentRevisionId, + requestIds: assigned.map(({ _id }) => _id), + }); + return { + experimentId, + experimentRevisionId: + result.experiment.revision.experimentRevisionId, + revisionNumber: result.experiment.revision.revisionNumber, + }; +}; + const upload = multer({ dest: tmpdir() }); const validatePersistedRequestTarget = (request: RequestDocument) => @@ -2287,7 +2369,18 @@ apiRoute(ctx.app, ctx.registry, { tags: ["Requests"], summary: "Bulk soft-delete requests", body: z.object({ ids: z.array(z.string()) }), - response: z.object({ deleted: z.number() }), + response: z.object({ + deleted: z.number(), + notFound: z.array(z.string()), + membershipRevision: z.object({ + experimentId: z.string(), + experimentRevisionId: z.string(), + revisionNumber: z.number().int(), + }).optional(), + }), + errorResponses: { + 409: { description: "Deletion spans Experiments or membership changed" }, + }, handler: async (req, res) => { const { ids } = req.body as { ids?: string[] }; @@ -2297,24 +2390,48 @@ apiRoute(ctx.app, ctx.registry, { } // Find which IDs exist and are not already deleted + const uniqueIds = [...new Set(ids)]; + if (uniqueIds.length !== ids.length) { + res.status(400).json({ error: "Request IDs must be unique" }); + return; + } const existingDocs = await ctx.requestCollection.find( - { _id: { $in: ids }, deletedAt: { $exists: false } }, - { projection: { _id: 1 } } + { _id: { $in: uniqueIds }, deletedAt: { $exists: false } }, ).toArray(); const existingIds = new Set(existingDocs.map(d => d._id)); + let membershipRevision; + try { + membershipRevision = await coordinatedDeletionRevision(existingDocs); + } catch (error) { + if (error instanceof ExperimentMembershipError) { + res.status(error.status).json({ + error: error.message, + code: error.code, + ...(error.conflicts ? { conflicts: error.conflicts } : {}), + }); + return; + } + throw error; + } + // Soft-delete all matching documents const result = await ctx.requestCollection.updateMany( - { _id: { $in: ids }, deletedAt: { $exists: false } }, + { + _id: { $in: uniqueIds }, + deletedAt: { $exists: false }, + experimentId: { $exists: false }, + }, { $set: { deletedAt: new Date() } } ); // Determine which IDs were not found or already deleted - const notFound = ids.filter(id => !existingIds.has(id)); + const notFound = uniqueIds.filter(id => !existingIds.has(id)); res.json({ deleted: result.modifiedCount, notFound, + ...(membershipRevision ? { membershipRevision } : {}), }); }, }); @@ -2326,27 +2443,68 @@ apiRoute(ctx.app, ctx.registry, { tags: ["Requests"], summary: "Soft-delete request", params: z.object({ id: z.string() }), - response: z.object({ message: z.string() }), - errorResponses: { 404: { description: "Not found" } }, + response: z.object({ + id: z.string(), + deleted: z.literal(true), + membershipRevision: z.object({ + experimentId: z.string(), + experimentRevisionId: z.string(), + revisionNumber: z.number().int(), + }).optional(), + }), + errorResponses: { + 404: { description: "Not found" }, + 409: { description: "Membership changed" }, + 410: { description: "Already deleted" }, + }, handler: async (req, res) => { const { id } = req.params; + const document = await ctx.requestCollection.findOne({ _id: id }); + if (!document) { + res.status(404).json({ error: "Request not found" }); + return; + } + if (document.deletedAt) { + res.status(410).json({ error: "Request already deleted" }); + return; + } + let membershipRevision; + try { + membershipRevision = await coordinatedDeletionRevision([document]); + } catch (error) { + if (error instanceof ExperimentMembershipError) { + res.status(error.status).json({ + error: error.message, + code: error.code, + ...(error.conflicts ? { conflicts: error.conflicts } : {}), + }); + return; + } + throw error; + } const result = await ctx.requestCollection.updateOne( - { _id: id, deletedAt: { $exists: false } }, + { + _id: id, + deletedAt: { $exists: false }, + experimentId: { $exists: false }, + }, { $set: { deletedAt: new Date() } } ); if (result.matchedCount === 0) { - const exists = await ctx.requestCollection.findOne({ _id: id }); - if (!exists) { - res.status(404).json({ error: "Request not found" }); - } else { - res.status(410).json({ error: "Request already deleted" }); - } + res.status(409).json({ + error: "Request membership changed before deletion", + code: "request-membership-contention", + }); return; } - res.json({ id, deleted: true }); + res.json({ + id, + deleted: true, + ...(membershipRevision ? { membershipRevision } : {}), + }); }, }); From 28a67858b5ccc8117319d2042e262ca7b4cf7a03 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:40:33 -0700 Subject: [PATCH 05/16] feat: add experiment membership CLI Support creating Experiments from existing requests and moving or removing multiple requests with expected revision CAS and resulting revision output.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/cli/src/commands/experiment.test.ts | 93 +++++++++++++++++ apps/cli/src/commands/experiment.ts | 124 ++++++++++++++++++++++- 2 files changed, 214 insertions(+), 3 deletions(-) diff --git a/apps/cli/src/commands/experiment.test.ts b/apps/cli/src/commands/experiment.test.ts index 0df68306..b6ae5f51 100644 --- a/apps/cli/src/commands/experiment.test.ts +++ b/apps/cli/src/commands/experiment.test.ts @@ -135,6 +135,7 @@ describe("experiment commands", () => { mode: "empty", metadata: { name: "Empty" }, requests: [], + requestIds: [], }), }); expect(requests[0]?.url).toBe( @@ -142,6 +143,98 @@ describe("experiment commands", () => { ); }); + it("creates an experiment from existing standalone requests", async () => { + const { requests } = mockFetchWith({ + experiment: { + ...experiment, + revision: { ...experiment.revision, memberCount: 2 }, + }, + requestIds: ["request-1", "request-2"], + }); + vi.spyOn(console, "log").mockImplementation(() => {}); + + await makeProgram().parseAsync( + [ + "experiment", + "create", + "--name", + "Existing", + "--request-id", + "request-1", + "--request-id", + "request-2", + "--project", + "project-1", + "--url", + "http://localhost:3100", + ], + { from: "user" }, + ); + + expect(requests[0]).toMatchObject({ + method: "POST", + body: JSON.stringify({ + mode: "existing", + metadata: { name: "Existing" }, + requests: [], + requestIds: ["request-1", "request-2"], + }), + }); + }); + + it.each([ + ["add", "POST"], + ["remove", "DELETE"], + ])( + "%s sends one complete membership revision request", + async (command, method) => { + const { requests } = mockFetchWith({ + experiment: { + ...experiment, + revision: { + ...experiment.revision, + experimentRevisionId: "revision-2", + revisionNumber: 2, + }, + }, + attachedRequestIds: command === "add" ? ["request-1", "request-2"] : [], + detachedRequestIds: + command === "remove" ? ["request-1", "request-2"] : [], + }); + vi.spyOn(console, "log").mockImplementation(() => {}); + + await makeProgram().parseAsync( + [ + "experiment", + command, + "experiment-1", + "--revision", + "revision-1", + "--request-id", + "request-1", + "--request-id", + "request-2", + "--project", + "project-1", + "--url", + "http://localhost:3100", + ], + { from: "user" }, + ); + + expect(requests[0]).toMatchObject({ + method, + body: JSON.stringify({ + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1", "request-2"], + }), + }); + expect(requests[0]?.url).toBe( + "http://localhost:3100/api/v1/experiments/experiment-1/requests?projectId=project-1", + ); + }, + ); + it("gets an exact project-scoped execution", async () => { const { requests } = mockFetchWith(experiment.execution); vi.spyOn(console, "log").mockImplementation(() => {}); diff --git a/apps/cli/src/commands/experiment.ts b/apps/cli/src/commands/experiment.ts index 84da5847..448f982d 100644 --- a/apps/cli/src/commands/experiment.ts +++ b/apps/cli/src/commands/experiment.ts @@ -578,6 +578,12 @@ export function registerExperimentCommands(program: Command): void { .option("--description ", "Experiment description") .option("--tags ", "Experiment tags") .option("--empty", "Create an explicit empty experiment") + .option( + "--request-id ", + "Create from an existing standalone request (repeatable)", + collectRepeatable, + [], + ) .option( "--preview", "Print the composition and request body without creating", @@ -611,13 +617,46 @@ export function registerExperimentCommands(program: Command): void { ); } } - const built = options.empty ? undefined : buildRequest(options); + const requestIds = Array.isArray(options.requestId) + ? options.requestId as string[] + : []; + if (options.empty && requestIds.length > 0) { + throw new Error("--empty cannot be combined with --request-id"); + } + if (requestIds.length > 0) { + const conflicting = [ + "scenario", + "message", + "worker", + "criteria", + "profile", + "profileVariationsFile", + "count", + ].filter((key) => options[key] !== undefined); + if (conflicting.length > 0) { + throw new Error( + `--request-id cannot be combined with run configuration options: ${conflicting.join(", ")}`, + ); + } + } + const built = + options.empty || requestIds.length > 0 + ? undefined + : buildRequest(options); const body = options.empty - ? { mode: "empty" as const, metadata, requests: [] } - : { + ? { mode: "empty" as const, metadata, requests: [], requestIds: [] } + : requestIds.length > 0 + ? { + mode: "existing" as const, + metadata, + requests: [], + requestIds, + } + : { mode: "compose" as const, metadata, requests: [built!.request], + requestIds: [], }; const expandedRunCount = built ? built.profileCount * built.occurrences @@ -692,6 +731,85 @@ export function registerExperimentCommands(program: Command): void { } }); + const membershipAction = ( + command: Command, + method: "POST" | "DELETE", + verb: "added" | "removed", + ) => { + withProjectOption( + command + .argument("", "Experiment ID") + .requiredOption( + "--revision ", + "Expected current Experiment revision ID", + ) + .requiredOption( + "--request-id ", + "Request ID (repeatable)", + collectRepeatable, + [], + ) + .option("-u, --url ", "API base URL", getDefaultApiUrl()), + ).action(async (id, options) => { + try { + const projectId = requireProjectId(options.project); + const requestIds = options.requestId as string[]; + const response = await apiFetch( + options.url, + `/experiments/${encodeURIComponent(id)}/requests`, + { + method, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + expectedExperimentRevisionId: options.revision, + requestIds, + }), + projectId, + }, + ); + if (!response.ok) await throwApiError(response); + const result = asRecord( + await json(response), + `experiment membership ${verb}`, + ); + const updated = parseExperiment(result.experiment); + console.log( + successText( + `${requestIds.length} request${requestIds.length === 1 ? "" : "s"} ${verb}`, + ), + ); + console.log( + `${label("Revision:")} ${value(updated.revision.experimentRevisionId)} (${value(`r${updated.revision.revisionNumber}`)})`, + ); + console.log( + `${label("Execution:")} ${value(updated.execution.experimentExecutionId)}`, + ); + } catch (error) { + console.error( + errorText("Error:"), + error instanceof Error ? error.message : error, + ); + process.exitCode = 1; + } + }); + }; + + membershipAction( + experiment + .command("add") + .alias("move") + .description("Move standalone requests into an Experiment"), + "POST", + "added", + ); + membershipAction( + experiment + .command("remove") + .description("Remove requests from an Experiment to standalone status"), + "DELETE", + "removed", + ); + withProjectOption( withOutputOption( experiment From b03899b5af49985fe8390f1bc5cf83253aa1706d Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:51:46 -0700 Subject: [PATCH 06/16] test: cover membership storage transitions Verify inline-to-inline, inline-to-chunked with immutable archive, chunked growth, and chunked-to-inline shrink while preserving exact vectors and fingerprints.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- .../src/experiments/experiment-store.test.ts | 228 ++++++++++++++++++ 1 file changed, 228 insertions(+) diff --git a/packages/shared/src/experiments/experiment-store.test.ts b/packages/shared/src/experiments/experiment-store.test.ts index 4561cfd0..aabe618b 100644 --- a/packages/shared/src/experiments/experiment-store.test.ts +++ b/packages/shared/src/experiments/experiment-store.test.ts @@ -217,7 +217,235 @@ function chunkCollection() { }; } +function historyCollection() { + const documents = new Map(); + return { + documents, + async updateOne( + filter: Document, + update: { $setOnInsert?: Document }, + ) { + const id = String(filter._id); + if (!documents.has(id) && update.$setOnInsert) { + documents.set(id, structuredClone(update.$setOnInsert)); + } + return { acknowledged: true, matchedCount: 1, modifiedCount: 1 }; + }, + async findOne(filter: Document) { + return structuredClone(documents.get(String(filter._id)) ?? null); + }, + }; +} + +function requestVector(count: number): ExperimentRequestState[] { + return Array.from({ length: count }, (_, index) => ({ + requestId: `request-${index.toString().padStart(5, "0")}`, + runId: `run-${index.toString().padStart(5, "0")}`, + dependencies: [ + { + handlerId: "pp-observations", + state: index % 2 === 0 ? "ready" as const : "waiting" as const, + ...(index % 2 === 0 ? { version: 3 } : {}), + }, + ], + blockers: + index % 2 === 0 + ? [] + : [{ + requestId: `request-${index.toString().padStart(5, "0")}`, + runId: `run-${index.toString().padStart(5, "0")}`, + handlerId: "pp-observations", + reason: "dependency-not-ready" as const, + }], + })); +} + describe("ExperimentStore CAS contracts", () => { + it("publishes inline membership revisions without changing representation", async () => { + const experiments = atomicCollection(experiment()); + const histories = historyCollection(); + const store = new ExperimentStore({ + experiments: + experiments as unknown as Collection, + executions: + histories as unknown as Collection, + chunks: chunkCollection() as unknown as Collection, + }); + const nextRequests = requestVector(2); + + const claimed = await store.replaceMembership({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + expectedRevisionNumber: 1, + expectedExperimentExecutionId: "execution-1", + expectedExecutionVersion: 1, + nextExperimentRevisionId: "revision-2", + nextExperimentExecutionId: "execution-2", + requests: nextRequests, + deferPublication: true, + }); + + expect(claimed?.execution.requests.storage).toBe("inline"); + expect( + claimed?.execution.rollover?.nextExecution.requests.storage, + ).toBe("inline"); + const published = await store.repairRollover( + "project-1", + "experiment-1", + ); + expect(published).toMatchObject({ + revision: { + experimentRevisionId: "revision-2", + revisionNumber: 2, + memberCount: 2, + }, + execution: { + experimentExecutionId: "execution-2", + attemptNumber: 1, + requests: { + storage: "inline", + entryCount: 2, + }, + }, + }); + expect( + await store.readExecutionRequests({ + projectId: "project-1", + experimentId: "experiment-1", + experimentExecutionId: "execution-2", + }), + ).toEqual(nextRequests); + }); + + it("archives an immutable inline revision while publishing chunked membership", async () => { + const experiments = atomicCollection(experiment()); + const chunks = chunkCollection(); + const histories = historyCollection(); + const store = new ExperimentStore({ + experiments: + experiments as unknown as Collection, + executions: + histories as unknown as Collection, + chunks: chunks as unknown as Collection, + }); + const nextRequests = requestVector(4_000); + + const claimed = await store.replaceMembership({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + expectedRevisionNumber: 1, + expectedExperimentExecutionId: "execution-1", + expectedExecutionVersion: 1, + nextExperimentRevisionId: "revision-2", + nextExperimentExecutionId: "execution-2", + requests: nextRequests, + deferPublication: true, + }); + + expect( + claimed?.execution.rollover?.nextExecution.requests.storage, + ).toBe("chunked"); + const published = await store.repairRollover( + "project-1", + "experiment-1", + ); + expect(published?.execution.requests.storage).toBe("chunked"); + expect(published?.revision.membershipFingerprint).toBe( + fingerprintExperimentRequestVector(nextRequests), + ); + expect( + await store.readExecutionRequests({ + projectId: "project-1", + experimentId: "experiment-1", + experimentExecutionId: "execution-2", + }), + ).toEqual(nextRequests); + expect(histories.documents.get("execution-1")).toMatchObject({ + revision: { experimentRevisionId: "revision-1" }, + requests: { storage: "inline", entryCount: 1 }, + }); + }); + + it("plans chunked-to-chunked growth and chunked-to-inline shrink independently", async () => { + const chunks = chunkCollection(); + const payloadStore = new FixedPayloadStore( + chunks as unknown as Collection, + ); + const currentRequests = requestVector(4_000); + const root = experiment(); + root.revision.memberCount = currentRequests.length; + root.revision.membershipFingerprint = + fingerprintExperimentRequestVector(currentRequests); + root.execution.requests = await payloadStore.prepareRequests({ + identity: { + projectId: "project-1", + experimentId: "experiment-1", + ownerId: "execution-1", + generation: 1, + }, + entries: currentRequests, + inlineDocument: (requests) => ({ + ...root, + execution: { ...root.execution, requests }, + }), + }); + root.execution.vectorFingerprint = + fingerprintExperimentRequestVector(currentRequests); + expect(root.execution.requests.storage).toBe("chunked"); + + const growExperiments = atomicCollection(root); + const grow = new ExperimentStore({ + experiments: + growExperiments as unknown as Collection, + executions: + historyCollection() as unknown as Collection, + chunks: chunks as unknown as Collection, + }); + const grownRequests = requestVector(4_200); + const grown = await grow.replaceMembership({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + expectedRevisionNumber: 1, + expectedExperimentExecutionId: "execution-1", + expectedExecutionVersion: 1, + nextExperimentRevisionId: "revision-grown", + nextExperimentExecutionId: "execution-grown", + requests: grownRequests, + deferPublication: true, + }); + expect( + grown?.execution.rollover?.nextExecution.requests.storage, + ).toBe("chunked"); + + const shrinkExperiments = atomicCollection(root); + const shrink = new ExperimentStore({ + experiments: + shrinkExperiments as unknown as Collection, + executions: + historyCollection() as unknown as Collection, + chunks: chunks as unknown as Collection, + }); + const shrunkRequests = requestVector(1); + const shrunk = await shrink.replaceMembership({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + expectedRevisionNumber: 1, + expectedExperimentExecutionId: "execution-1", + expectedExecutionVersion: 1, + nextExperimentRevisionId: "revision-shrunk", + nextExperimentExecutionId: "execution-shrunk", + requests: shrunkRequests, + deferPublication: true, + }); + expect( + shrunk?.execution.rollover?.nextExecution.requests.storage, + ).toBe("inline"); + }); + it("allows exactly one winner in a writer-freezer race", async () => { const experiments = atomicCollection(experiment()); const store = new ExperimentStore({ From 323a4bf77f82fd21c383b85a4f9f31598a2eff9a Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 07:59:47 -0700 Subject: [PATCH 07/16] fix: make membership replay idempotent Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/experiment-membership.test.ts | 31 +++++++++++++++++++++ apps/api/src/experiment-membership.ts | 32 ++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/apps/api/src/experiment-membership.test.ts b/apps/api/src/experiment-membership.test.ts index 3f2efcc0..a52f32e1 100644 --- a/apps/api/src/experiment-membership.test.ts +++ b/apps/api/src/experiment-membership.test.ts @@ -183,4 +183,35 @@ describe("ExperimentMembershipCoordinator complete-set operations", () => { code: "stale-experiment-revision", }); }); + + it("returns the already-published deterministic revision on idempotent replay", async () => { + const requests = [request("request-1", "run-1")]; + const replayFingerprint = fingerprintExperimentPayload({ + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + runIds: [{ requestId: "request-1", runId: "run-1" }], + }); + const published = experiment(requests); + published.revision.experimentRevisionId = `er_v1_${replayFingerprint}`; + published.revision.revisionNumber = 2; + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue(published); + vi.spyOn( + ExperimentStore.prototype, + "readExecutionRequests", + ).mockResolvedValue(requests); + + await expect( + coordinator().replace({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + }), + ).resolves.toEqual({ + experiment: published, + attachedRequestIds: [], + detachedRequestIds: [], + }); + }); }); diff --git a/apps/api/src/experiment-membership.ts b/apps/api/src/experiment-membership.ts index 66c10f44..659150de 100644 --- a/apps/api/src/experiment-membership.ts +++ b/apps/api/src/experiment-membership.ts @@ -220,6 +220,38 @@ export class ExperimentMembershipCoordinator { current.revision.experimentRevisionId !== input.expectedExperimentRevisionId ) { + const publishedRequests = + (await this.store.readExecutionRequests({ + projectId: input.projectId, + experimentId: input.experimentId, + experimentExecutionId: + current.execution.experimentExecutionId, + })) ?? []; + const publishedIds = publishedRequests + .map(({ requestId }) => requestId) + .sort(); + const replayFingerprint = fingerprintExperimentPayload({ + experimentId: input.experimentId, + expectedExperimentRevisionId: input.expectedExperimentRevisionId, + requestIds, + runIds: publishedRequests + .map(({ requestId, runId }) => ({ requestId, runId })) + .sort((left, right) => left.requestId.localeCompare(right.requestId)), + }); + if ( + publishedIds.length === requestIds.length && + publishedIds.every( + (requestId, index) => requestId === requestIds[index], + ) && + current.revision.experimentRevisionId === + `er_v1_${replayFingerprint}` + ) { + return { + experiment: current, + attachedRequestIds: [], + detachedRequestIds: [], + }; + } throw new ExperimentMembershipError( `Experiment revision is stale; current revision is ${current.revision.experimentRevisionId}`, 409, From b2acfc4ef1c9588d1dcd457d8b548ea5384787ae Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 08:21:47 -0700 Subject: [PATCH 08/16] fix: replan interrupted membership claims Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/experiment-membership.test.ts | 81 +++++++++++++++++++ apps/api/src/experiment-membership.ts | 36 ++++++++- .../src/experiments/experiment-store.test.ts | 56 +++++++++++++ .../src/experiments/experiment-store.ts | 61 +++++++++++++- 4 files changed, 231 insertions(+), 3 deletions(-) diff --git a/apps/api/src/experiment-membership.test.ts b/apps/api/src/experiment-membership.test.ts index a52f32e1..bc5b81b3 100644 --- a/apps/api/src/experiment-membership.test.ts +++ b/apps/api/src/experiment-membership.test.ts @@ -5,6 +5,7 @@ import type { Collection } from "mongodb"; import { EXPERIMENT_STORAGE_PROFILE_VERSION, ExperimentStore, + FixedPayloadStore, fingerprintExperimentPayload, type ExperimentDocument, type ExperimentExecutionHistoryDocument, @@ -214,4 +215,84 @@ describe("ExperimentMembershipCoordinator complete-set operations", () => { detachedRequestIds: [], }); }); + + it("aborts and replans once when a selected request retries before claim", async () => { + const current = experiment([request("request-1", "run-1")]); + current.execution.frozenAt = now; + current.execution.rollover = { + reason: "membership", + claimedAt: now, + nextRevision: { + ...current.revision, + experimentRevisionId: "revision-2", + revisionNumber: 2, + memberCount: 2, + }, + nextExecution: { + ...current.execution, + experimentExecutionId: "execution-2", + requests: { + storage: "inline", + entryCount: 2, + payloadFingerprint: "next-payload", + payloadVersion: 1, + entries: [ + request("request-1", "run-1"), + request("request-2", "run-old"), + ], + }, + }, + }; + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue(current); + vi.spyOn( + ExperimentStore.prototype, + "readExecutionRequests", + ).mockResolvedValue([request("request-1", "run-1")]); + vi.spyOn( + FixedPayloadStore.prototype, + "readRequests", + ).mockResolvedValue([ + request("request-1", "run-1"), + request("request-2", "run-old"), + ]); + vi.spyOn( + ExperimentStore.prototype, + "abortMembershipReplacement", + ).mockResolvedValue(experiment([request("request-1", "run-1")])); + const updateMany = vi.fn().mockResolvedValue({ matchedCount: 1 }); + const membership = new ExperimentMembershipCoordinator({ + requests: { + updateOne: vi.fn().mockResolvedValue({ matchedCount: 0 }), + updateMany, + findOne: vi.fn().mockResolvedValue({ + _id: "request-2", + projectId: "project-1", + run: { _id: "run-new" }, + }), + } as unknown as Collection, + experiments: {} as Collection, + executions: {} as Collection, + chunks: {} as Collection, + }); + const result = { + experiment: current, + attachedRequestIds: ["request-2"], + detachedRequestIds: [], + }; + const replace = vi.spyOn(membership, "replace").mockResolvedValue(result); + + await expect( + membership.repair("project-1", "experiment-1"), + ).resolves.toBe(result); + expect(replace).toHaveBeenCalledWith( + { + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1", "request-2"], + }, + false, + ); + expect(updateMany).toHaveBeenCalledOnce(); + }); }); diff --git a/apps/api/src/experiment-membership.ts b/apps/api/src/experiment-membership.ts index 659150de..cb7646b1 100644 --- a/apps/api/src/experiment-membership.ts +++ b/apps/api/src/experiment-membership.ts @@ -206,7 +206,7 @@ export class ExperimentMembershipCoordinator { experimentId: string; expectedExperimentRevisionId: string; requestIds: readonly string[]; - }): Promise { + }, allowReplan = true): Promise { const requestIds = uniqueCanonicalRequestIds(input.requestIds); const current = await this.store.get(input.projectId, input.experimentId); if (!current) { @@ -327,7 +327,11 @@ export class ExperimentMembershipCoordinator { : undefined, ); } - return this.repair(input.projectId, input.experimentId); + return this.repair( + input.projectId, + input.experimentId, + allowReplan, + ); } async add(input: { @@ -391,6 +395,7 @@ export class ExperimentMembershipCoordinator { async repair( projectId: string, experimentId: string, + allowReplan = true, ): Promise { const experiment = await this.store.get(projectId, experimentId); const claim = experiment?.execution.rollover; @@ -483,6 +488,33 @@ export class ExperimentMembershipCoordinator { experimentId, operationId, }); + const aborted = await this.store.abortMembershipReplacement({ + projectId, + experimentId, + expectedExperimentExecutionId: + experiment.execution.experimentExecutionId, + nextExperimentExecutionId: + claim.nextExecution.experimentExecutionId, + nextExperimentRevisionId: operationId, + }); + if ( + allowReplan && + aborted && + error instanceof ExperimentMembershipError && + error.code === "request-membership-conflict" && + error.conflicts?.every((conflict) => !conflict.experimentId) + ) { + return this.replace( + { + projectId, + experimentId, + expectedExperimentRevisionId: + aborted.revision.experimentRevisionId, + requestIds: [...nextIds], + }, + false, + ); + } throw error; } diff --git a/packages/shared/src/experiments/experiment-store.test.ts b/packages/shared/src/experiments/experiment-store.test.ts index aabe618b..e30761c0 100644 --- a/packages/shared/src/experiments/experiment-store.test.ts +++ b/packages/shared/src/experiments/experiment-store.test.ts @@ -182,6 +182,7 @@ function experiment(): ExperimentDocument { function chunkCollection() { const documents = new Map(); return { + documents, async updateOne( filter: Document, update: { $setOnInsert?: Document; $set?: Document }, @@ -214,6 +215,13 @@ function chunkCollection() { async findOne(filter: Document) { return documents.get(String(filter._id)) ?? null; }, + async deleteMany(filter: { _id: { $in: string[] } }) { + let deletedCount = 0; + for (const id of filter._id.$in) { + if (documents.delete(id)) deletedCount += 1; + } + return { deletedCount }; + }, }; } @@ -446,6 +454,54 @@ describe("ExperimentStore CAS contracts", () => { ).toBe("inline"); }); + it("atomically aborts an unpublished membership claim and discards its chunks", async () => { + const experiments = atomicCollection(experiment()); + const chunks = chunkCollection(); + const store = new ExperimentStore({ + experiments: + experiments as unknown as Collection, + executions: + historyCollection() as unknown as Collection, + chunks: chunks as unknown as Collection, + }); + const nextRequests = requestVector(4_000); + + const claimed = await store.replaceMembership({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + expectedRevisionNumber: 1, + expectedExperimentExecutionId: "execution-1", + expectedExecutionVersion: 1, + nextExperimentRevisionId: "revision-2", + nextExperimentExecutionId: "execution-2", + requests: nextRequests, + deferPublication: true, + }); + const nextPayload = claimed?.execution.rollover?.nextExecution.requests; + expect(nextPayload?.storage).toBe("chunked"); + expect(chunks.documents.size).toBeGreaterThan(0); + + const aborted = await store.abortMembershipReplacement({ + projectId: "project-1", + experimentId: "experiment-1", + expectedExperimentExecutionId: "execution-1", + nextExperimentExecutionId: "execution-2", + nextExperimentRevisionId: "revision-2", + }); + + expect(aborted).toMatchObject({ + revision: { experimentRevisionId: "revision-1" }, + execution: { + experimentExecutionId: "execution-1", + version: 3, + }, + }); + expect(aborted?.execution.rollover).toBeUndefined(); + expect(aborted?.execution.frozenAt).toBeUndefined(); + expect(chunks.documents.size).toBe(0); + }); + it("allows exactly one winner in a writer-freezer race", async () => { const experiments = atomicCollection(experiment()); const store = new ExperimentStore({ diff --git a/packages/shared/src/experiments/experiment-store.ts b/packages/shared/src/experiments/experiment-store.ts index c3e9af96..619203fa 100644 --- a/packages/shared/src/experiments/experiment-store.ts +++ b/packages/shared/src/experiments/experiment-store.ts @@ -918,12 +918,71 @@ export class ExperimentStore { } as Document, { returnDocument: "after" }, ); - if (!claimed) return null; + if (!claimed) { + await this.payloads.discardUnpublished(payload); + return null; + } return input.deferPublication ? parseExperiment(claimed) : this.repairRollover(input.projectId, input.experimentId); } + async abortMembershipReplacement(input: { + projectId: string; + experimentId: string; + expectedExperimentExecutionId: string; + nextExperimentExecutionId: string; + nextExperimentRevisionId: string; + now?: Date; + }): Promise { + const experiment = await this.getRaw(input.projectId, input.experimentId); + const rollover = experiment?.execution.rollover; + if ( + !experiment || + rollover?.reason !== "membership" || + rollover.nextRevision?.experimentRevisionId !== + input.nextExperimentRevisionId || + rollover.nextExecution.experimentExecutionId !== + input.nextExperimentExecutionId || + experiment.execution.experimentExecutionId !== + input.expectedExperimentExecutionId + ) { + return null; + } + + const now = input.now ?? new Date(); + const updated = await this.collections.experiments.findOneAndUpdate( + { + _id: input.experimentId, + projectId: input.projectId, + "execution.experimentExecutionId": + input.expectedExperimentExecutionId, + "execution.rollover.reason": "membership", + "execution.rollover.nextRevision.experimentRevisionId": + input.nextExperimentRevisionId, + "execution.rollover.nextExecution.experimentExecutionId": + input.nextExperimentExecutionId, + } as Document, + { + $set: { + "execution.updatedAt": now, + updatedAt: now, + }, + $unset: { + "execution.frozenAt": "", + "execution.rollover": "", + }, + $inc: { "execution.version": 1, stateVersion: 1 }, + } as Document, + { returnDocument: "after" }, + ); + if (!updated) return null; + await this.payloads.discardUnpublished( + rollover.nextExecution.requests, + ); + return parseExperiment(updated); + } + async discardStale(input: { projectId: string; experimentId: string; From 348d6f26c1e4bc9c8ecbdeccd2a431737af237b2 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 08:37:48 -0700 Subject: [PATCH 09/16] test: verify coordinated member deletion Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/api/src/endpoints.test.ts | 151 +++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/apps/api/src/endpoints.test.ts b/apps/api/src/endpoints.test.ts index f92d9c5a..f28c9ba9 100644 --- a/apps/api/src/endpoints.test.ts +++ b/apps/api/src/endpoints.test.ts @@ -7,6 +7,8 @@ import { app, _injectTestDependencies } from "./index.js"; import { useTestServer } from "./test-server.js"; import { _resetRunFacetsCacheForTests } from "./routes/requests/index.js"; import { createAllMockDependencies, createMockCollection } from "./test-helpers.js"; +import { ExperimentStore } from "shared"; +import { ExperimentMembershipCoordinator } from "./experiment-membership.js"; const TEST_PROJECT_ID = "test-project"; @@ -1163,6 +1165,155 @@ describe("API Endpoints", () => { const res = await request(testServer()).delete("/api/v1/requests/r1"); expect(res.status).toBe(410); }); + + it("publishes one membership revision before deleting a member", async () => { + (mocks.collection.findOne as any).mockResolvedValue({ + _id: "r1", + projectId: TEST_PROJECT_ID, + experimentId: "experiment-1", + }); + const get = vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue({ + revision: { + experimentRevisionId: "revision-1", + }, + } as any); + const remove = vi.spyOn( + ExperimentMembershipCoordinator.prototype, + "remove", + ).mockResolvedValue({ + experiment: { + revision: { + experimentRevisionId: "revision-2", + revisionNumber: 2, + }, + }, + attachedRequestIds: [], + detachedRequestIds: ["r1"], + } as any); + (mocks.collection.updateOne as any).mockResolvedValue({ + matchedCount: 1, + modifiedCount: 1, + }); + + const res = await request(testServer()).delete("/api/v1/requests/r1"); + + expect(res.status).toBe(200); + expect(res.body.membershipRevision).toEqual({ + experimentId: "experiment-1", + experimentRevisionId: "revision-2", + revisionNumber: 2, + }); + expect(get).toHaveBeenCalledWith(TEST_PROJECT_ID, "experiment-1"); + expect(remove).toHaveBeenCalledWith({ + projectId: TEST_PROJECT_ID, + experimentId: "experiment-1", + expectedExperimentRevisionId: "revision-1", + requestIds: ["r1"], + }); + expect(remove.mock.invocationCallOrder[0]).toBeLessThan( + (mocks.collection.updateOne as any).mock.invocationCallOrder[0], + ); + expect(mocks.collection.updateOne).toHaveBeenCalledWith( + expect.objectContaining({ + _id: "r1", + experimentId: { $exists: false }, + }), + expect.anything(), + ); + }); + }); + + describe("DELETE /api/v1/requests/bulk", () => { + it("publishes one revision for several members before deleting them", async () => { + const documents = [ + { + _id: "r1", + projectId: TEST_PROJECT_ID, + experimentId: "experiment-1", + }, + { + _id: "r2", + projectId: TEST_PROJECT_ID, + experimentId: "experiment-1", + }, + ]; + (mocks.collection.find as any).mockReturnValue({ + toArray: vi.fn().mockResolvedValue(documents), + }); + vi.spyOn(ExperimentStore.prototype, "get").mockResolvedValue({ + revision: { experimentRevisionId: "revision-1" }, + } as any); + const remove = vi.spyOn( + ExperimentMembershipCoordinator.prototype, + "remove", + ).mockResolvedValue({ + experiment: { + revision: { + experimentRevisionId: "revision-2", + revisionNumber: 2, + }, + }, + attachedRequestIds: [], + detachedRequestIds: ["r1", "r2"], + } as any); + (mocks.collection.updateMany as any).mockResolvedValue({ + matchedCount: 2, + modifiedCount: 2, + }); + + const res = await request(testServer()) + .delete("/api/v1/requests/bulk") + .send({ ids: ["r1", "r2"] }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + deleted: 2, + notFound: [], + membershipRevision: { + experimentId: "experiment-1", + experimentRevisionId: "revision-2", + revisionNumber: 2, + }, + }); + expect(remove).toHaveBeenCalledOnce(); + expect(remove).toHaveBeenCalledWith( + expect.objectContaining({ requestIds: ["r1", "r2"] }), + ); + expect(remove.mock.invocationCallOrder[0]).toBeLessThan( + (mocks.collection.updateMany as any).mock.invocationCallOrder[0], + ); + }); + + it("rejects a selection spanning several Experiments without deletion", async () => { + (mocks.collection.find as any).mockReturnValue({ + toArray: vi.fn().mockResolvedValue([ + { + _id: "r1", + projectId: TEST_PROJECT_ID, + experimentId: "experiment-1", + }, + { + _id: "r2", + projectId: TEST_PROJECT_ID, + experimentId: "experiment-2", + }, + ]), + }); + + const res = await request(testServer()) + .delete("/api/v1/requests/bulk") + .send({ ids: ["r1", "r2"] }); + + expect(res.status).toBe(409); + expect(res.body).toMatchObject({ + code: "multiple-experiment-delete-conflict", + conflicts: [ + { requestId: "r1", experimentId: "experiment-1" }, + { requestId: "r2", experimentId: "experiment-2" }, + ], + }); + expect(mocks.collection.updateMany).not.toHaveBeenCalled(); + }); }); // =================================================================== From 7b1c5aec6e70a60bd02baa5aa38fab8f093723a9 Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 08:49:48 -0700 Subject: [PATCH 10/16] docs: describe membership revision workflows Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- apps/cli/README.md | 12 +++++ docs/architecture/app-design.md | 15 ++++-- docs/architecture/experiments.md | 83 ++++++++++++++++++++++++-------- 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/apps/cli/README.md b/apps/cli/README.md index b40bc2b5..ead7c262 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -83,6 +83,18 @@ scope run get -i # Stream logs scope run logs -i +# Create an Experiment from existing standalone runs +scope experiment create --name "Comparison" \ + --request-id --request-id + +# Move standalone runs into one new Experiment revision +scope experiment add --revision \ + --request-id --request-id + +# Remove members to standalone status in one new revision +scope experiment remove --revision \ + --request-id + # List criteria scope criteria list diff --git a/docs/architecture/app-design.md b/docs/architecture/app-design.md index bb3c6a8e..f98d521d 100644 --- a/docs/architecture/app-design.md +++ b/docs/architecture/app-design.md @@ -472,15 +472,20 @@ Profile fan-out mode is also supported for comparative runs: ### Experiment identity and processing executions -An [Experiment](experiments.md) is created only through the deliberate -Experiment creation surface. It owns a stable project-scoped identity, revision, -and exact request/run vector. The current execution is embedded in the bounded +An [Experiment](experiments.md) is created only through a deliberate Experiment +creation surface, including bulk creation from existing standalone requests. It +owns a stable project-scoped identity, immutable revision history, and an exact +request/run vector per revision. Complete-set add/move/remove operations publish +one new revision and execution. The current execution is embedded in the bounded root, immutable history is archived separately, and deterministic fixed buckets keep large vectors below Cosmos DB's universal 2 MB limit. Standalone request creation, profile fan-out, and bulk resubmission remain -first-class request workflows and never infer an Experiment. Only legacy -composition inputs that overlap Experiment creation are deprecated. +first-class request workflows and never infer an Experiment. Request documents +denormalize the current `experimentId` for lookup, but Experiment history is +authoritative. Direct deletion of a member publishes a removal revision and +clears that field before soft deletion. Only legacy composition inputs that +overlap Experiment creation are deprecated. ## Runs List Query API diff --git a/docs/architecture/experiments.md b/docs/architecture/experiments.md index eae65a0a..fd0fe028 100644 --- a/docs/architecture/experiments.md +++ b/docs/architecture/experiments.md @@ -1,16 +1,17 @@ # Experiments Architecture -> **Status:** PR A foundation plus the PR B built-in readout, current as of -> October 2026. +> **Status:** Foundation, built-in readout, and existing-run membership +> revisions, current as of October 2026. Experiments are project-scoped, named benchmark compositions. An Experiment stores durable metadata, one immutable revision, and one current processing execution. Previous executions are immutable history records. -PR A provides the canonical storage model, project-scoped public API contract, -CLI commands, Portal creation/list/detail flows, and run correlation. PR B adds -one built-in `pp-readout` Experiment handler and generic Reports presentation. -Neither contribution adds mutable membership or cascading lifecycle behavior. +The foundation provides the canonical storage model, project-scoped public API +contract, CLI commands, Portal creation/list/detail flows, and run correlation. +The built-in `pp-readout` Experiment handler adds generic Reports presentation. +Existing-run membership workflows publish new immutable revisions while +preserving Experiment history as the authority. ## Model @@ -72,8 +73,9 @@ An **Experiment execution** is one processing attempt over that revision: - freeze, discard, and supersession state. Retry creates a new execution attempt over the same stored request vector. It -does not create a new revision or retry member runs. Revision membership -mutation is deferred to PR C. +does not create a new revision or retry member runs. A membership replacement +instead creates exactly one new revision and one new execution from the +complete next request/run vector. ### Exact request/run vector @@ -138,6 +140,9 @@ an existing deterministic ID whose identity or contents differ. ## Publication and concurrency Experiment creation and execution rollover use a prepare-then-publish protocol. +Membership replacement uses the same representation-independent protocol: +preplan one complete `FixedPayload`, claim request ownership, archive the +current execution, and atomically install the next revision and execution. ```mermaid sequenceDiagram @@ -194,6 +199,20 @@ execution. A caller can resume this sequence after interruption without inventing state. Prepared chunks that were never published can be removed with `discardUnpublished()`. +Membership repair runs before generic rollover repair because denormalized +request ownership must be claimed before publication. Claims use the exact +project, request, and current run IDs. A run retry between planning and claim +aborts the unpublished successor, discards its unpublished chunks, and replans +once from current run identities. Other ownership conflicts abort without +partial publication and report the exact request IDs and current owners. +Replaying a completed mutation returns its deterministic successor revision. + +After publication, removed requests have `experimentId` cleared and retained +requests carry the published membership revision. The Experiment revision and +archived exact vectors remain authoritative; `request.experimentId` is only a +denormalized lookup field. An empty membership revision remains visible and +non-dispatchable, and a later add can resume it with another revision. + ## Collection and index ownership | Collection | Owner and contents | Indexes created by migration 034 | @@ -275,7 +294,7 @@ All routes require `?projectId=`: | Method | Route | Behavior | | --- | --- | --- | -| `POST` | `/api/v1/experiments` | Create from `{ mode: "compose", metadata, requests }` or `{ mode: "empty", metadata, requests: [] }` | +| `POST` | `/api/v1/experiments` | Create from composed definitions, explicitly empty membership, or `{ mode: "existing", metadata, requestIds }` | | `GET` | `/api/v1/experiments` | Cursor-page finalized Experiments | | `GET` | `/api/v1/experiments/:id` | Read the public Experiment and current execution | | `GET` | `/api/v1/experiments/:id/executions` | List current and archived execution summaries | @@ -284,10 +303,22 @@ All routes require `?projectId=`: | `GET` | `/api/v1/experiments/:id/executions/:experimentExecutionId/reports/:reportId/structured` | Download that exact report's structured JSON artifact | | `GET` | `/api/v1/experiments/:id/executions/:experimentExecutionId/logs` | Stream attempt-scoped handler logs as SSE | | `POST` | `/api/v1/experiments/:id/retry` | Start a new processing attempt for the current exact vector | - -Creation metadata requires `name` and accepts optional `description` and -`tags`. Compose mode requires one or more complete request definitions and -empty mode requires an empty request array. +| `POST` | `/api/v1/experiments/:id/requests` | Add or move standalone requests in one complete-set revision | +| `DELETE` | `/api/v1/experiments/:id/requests` | Remove members to standalone status in one complete-set revision | + +Creation metadata requires `name` and accepts an optional `description`. +Compose mode requires one or more complete request definitions, existing mode +requires one or more current standalone request IDs, and empty mode requires an +empty request array. + +Membership mutation requires the expected current revision ID. Every selected +request must exist in the same project with a current run. Duplicate IDs, +cross-project or missing requests, already-assigned requests, and +Experiment-to-Experiment reassignment fail the complete operation. Direct +member deletion first publishes one removal revision, clears `experimentId`, +and only then soft-deletes the request. A bulk delete can coordinate standalone +requests plus members from one Experiment; selections spanning several +Experiments fail with exact owner conflicts. The public view exposes identity, project, metadata, revision, current execution, readiness, blockers, handler progress, supersession, report, @@ -306,6 +337,10 @@ The CLI provides: ```text scope experiment create +scope experiment create --name --request-id [--request-id ...] +scope experiment add --revision --request-id [--request-id ...] +scope experiment move --revision --request-id [--request-id ...] +scope experiment remove --revision --request-id [--request-id ...] scope experiment list scope experiment get scope experiment executions [experiment-execution-id] @@ -315,11 +350,20 @@ scope experiment retry --execution ``` Creation reuses run configuration flags, adds Experiment metadata, supports -composition preview, and supports explicit empty creation. Every command -requires project scope. +composition preview, explicit empty creation, and creation from existing +standalone requests. `add`/`move` and `remove` accept repeatable request IDs and +print the resulting revision and execution identities. Every command requires +project scope. The Portal provides `/experiments`, `/experiments/new`, and -`/experiments/:id`. The shared creation form retains the established run +`/experiments/:id`. The Runs list can bulk-create an Experiment from eligible +standalone selections or move them into an existing Experiment. Experiment +Detail can remove selected current members back to standalone status. Conflicts +keep the selection visible, identify ineligible requests and owners, and avoid +partial navigation. Coordinated deletion warns about membership changes and +shows the resulting revision. + +The shared creation form retains the established run configuration and expansion count, adds Experiment metadata and an empty option, and navigates to Experiment Detail after creation. Detail shows current state, linked runs, blockers, exact execution history, retry, and the selected @@ -357,16 +401,14 @@ Experiment creation. ## Deferred capabilities -The following are explicitly outside PR A: +The following remain outside the current implementation: | Follow-up | Deferred capability | | --- | --- | -| PR C | Membership mutation and new revision creation after initial publication | | PR D | Report-template attachment or template-driven report orchestration | | PR E | Cascading lifecycle operations across an Experiment and its member requests/runs | -GEPA restacking is also excluded. PR A must not imply these capabilities -through storage methods, client actions, or API contracts. +GEPA restacking is also excluded. ## Key files @@ -380,6 +422,7 @@ through storage methods, client actions, or API contracts. | `packages/shared/src/experiments/experiment-store.ts` | Creation, CAS processing, freeze, archive, rollover, retry, and repair | | `packages/db-migrations/src/migrations/034-create-experiment-storage-indexes.ts` | Collection index ownership | | `apps/api/src/experiment-coordinator.ts` | API-owned workflow policy and repair | +| `apps/api/src/experiment-membership.ts` | Complete-set membership validation, request ownership, publication, and repair | | `apps/api/src/experiment-dispatcher.ts` | Durable polling and queue delivery | | `apps/workers/readout/` | Exact-execution readout synthesis, append-only report storage, and final CAS publication | | `apps/cli/src/commands/experiment.ts` | Project-scoped CLI surface | From 1bc07f03882546b540a32af6ad3e3045d4028e7a Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 09:12:31 -0700 Subject: [PATCH 11/16] feat: manage experiment members in portal Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- .../src/lib/api-experiment-membership.test.ts | 101 ++++++++ apps/portal/src/lib/api.ts | 78 +++++- .../src/lib/experiment-membership.test.ts | 37 +++ apps/portal/src/lib/experiment-membership.ts | 35 +++ apps/portal/src/lib/experiments.ts | 17 ++ .../src/pages/ExperimentDetail.test.tsx | 49 ++++ apps/portal/src/pages/ExperimentDetail.tsx | 161 +++++++++++-- apps/portal/src/pages/RunsList.tsx | 223 +++++++++++++++++- 8 files changed, 671 insertions(+), 30 deletions(-) create mode 100644 apps/portal/src/lib/api-experiment-membership.test.ts create mode 100644 apps/portal/src/lib/experiment-membership.test.ts create mode 100644 apps/portal/src/lib/experiment-membership.ts diff --git a/apps/portal/src/lib/api-experiment-membership.test.ts b/apps/portal/src/lib/api-experiment-membership.test.ts new file mode 100644 index 00000000..4f8e4b56 --- /dev/null +++ b/apps/portal/src/lib/api-experiment-membership.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { api } from "./api"; +import { setSelectedProjectIdHolder } from "./project-scope"; + +const apiClientMock = vi.hoisted(() => vi.fn()); + +vi.mock("./api-client", () => ({ + apiClient: apiClientMock, +})); + +const experiment = { + experimentId: "experiment-1", + projectId: "project-1", + metadata: { name: "Existing runs" }, + revision: { + experimentRevisionId: "revision-2", + revisionNumber: 2, + memberCount: 2, + }, + execution: { + experimentExecutionId: "execution-2", + attemptNumber: 1, + requests: [], + handlerProgress: {}, + blocking: [], + createdAt: "2026-10-16T00:00:00.000Z", + }, + blocking: [], + createdRequestIds: [], + createdAt: "2026-10-16T00:00:00.000Z", +}; + +describe("Experiment membership API", () => { + beforeEach(() => { + apiClientMock.mockReset(); + setSelectedProjectIdHolder("project-1"); + }); + + afterEach(() => { + setSelectedProjectIdHolder(undefined); + }); + + it("creates an Experiment from existing request ids", async () => { + apiClientMock.mockResolvedValue( + new Response(JSON.stringify({ experiment, requestIds: ["request-1", "request-2"] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + + const result = await api.createExperimentFromRuns({ + metadata: { name: "Existing runs" }, + requestIds: ["request-1", "request-2"], + }); + + expect(result.experiment.revision.revisionNumber).toBe(2); + expect(apiClientMock).toHaveBeenCalledWith( + "/api/v1/experiments?projectId=project-1", + expect.objectContaining({ method: "POST" }), + ); + const init = apiClientMock.mock.calls[0][1] as RequestInit; + expect(JSON.parse(String(init.body))).toEqual({ + mode: "existing", + metadata: { name: "Existing runs" }, + requests: [], + requestIds: ["request-1", "request-2"], + }); + }); + + it("sends one revision-guarded removal request and parses its result", async () => { + apiClientMock.mockResolvedValue( + new Response( + JSON.stringify({ + experiment, + attachedRequestIds: [], + detachedRequestIds: ["request-1"], + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ), + ); + + const result = await api.updateExperimentRequests("DELETE", "experiment-1", { + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + }); + + expect(result.detachedRequestIds).toEqual(["request-1"]); + expect(apiClientMock).toHaveBeenCalledWith( + "/api/v1/experiments/experiment-1/requests?projectId=project-1", + expect.objectContaining({ method: "DELETE" }), + ); + const init = apiClientMock.mock.calls[0][1] as RequestInit; + expect(JSON.parse(String(init.body))).toEqual({ + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-1"], + }); + }); +}); diff --git a/apps/portal/src/lib/api.ts b/apps/portal/src/lib/api.ts index 2dcaf9b1..bd94f019 100644 --- a/apps/portal/src/lib/api.ts +++ b/apps/portal/src/lib/api.ts @@ -14,10 +14,13 @@ import { parseExperimentExecution, parseExperimentExecutionPage, parseExperimentPage, + type ExistingRunExperimentCreateView, type ExperimentExecutionPageView, type ExperimentExecutionView, + type ExperimentMembershipMutationView, type ExperimentMetadataView, type ExperimentPageView, + type ExperimentRevisionIdentityView, type ExperimentView, } from "./experiments"; @@ -301,6 +304,69 @@ export const api = { return parseExperiment(value.experiment); }, + createExperimentFromRuns: async (body: { + metadata: ExperimentMetadataView; + requestIds: string[]; + }): Promise => { + const value = await request( + "/experiments", + { + method: "POST", + body: JSON.stringify({ + mode: "existing", + metadata: body.metadata, + requests: [], + requestIds: body.requestIds, + }), + }, + { scoped: true }, + ); + if ( + !value || + typeof value !== "object" || + !("experiment" in value) || + !("requestIds" in value) || + !Array.isArray(value.requestIds) || + !value.requestIds.every((requestId) => typeof requestId === "string") + ) { + throw new Error("Invalid existing-run experiment response from Scope"); + } + return { + experiment: parseExperiment(value.experiment), + requestIds: value.requestIds, + }; + }, + + updateExperimentRequests: async ( + method: "POST" | "DELETE", + id: string, + body: { expectedExperimentRevisionId: string; requestIds: string[] }, + ): Promise => { + const value = await request( + `/experiments/${encodeURIComponent(id)}/requests`, + { method, body: JSON.stringify(body) }, + { scoped: true }, + ); + if ( + !value || + typeof value !== "object" || + !("experiment" in value) || + !("attachedRequestIds" in value) || + !Array.isArray(value.attachedRequestIds) || + !value.attachedRequestIds.every((requestId) => typeof requestId === "string") || + !("detachedRequestIds" in value) || + !Array.isArray(value.detachedRequestIds) || + !value.detachedRequestIds.every((requestId) => typeof requestId === "string") + ) { + throw new Error("Invalid experiment membership response from Scope"); + } + return { + experiment: parseExperiment(value.experiment), + attachedRequestIds: value.attachedRequestIds, + detachedRequestIds: value.detachedRequestIds, + }; + }, + listExperiments: async (opts?: { cursor?: string; limit?: number; @@ -411,7 +477,11 @@ export const api = { }, /** Soft-delete a run */ - deleteRun: (id: string): Promise<{ id: string; deleted: boolean }> => { + deleteRun: (id: string): Promise<{ + id: string; + deleted: boolean; + membershipRevision?: ExperimentRevisionIdentityView; + }> => { return request(`/requests/${id}`, { method: "DELETE" }); }, @@ -484,7 +554,11 @@ export const api = { }, /** Bulk soft-delete multiple runs */ - bulkDeleteRuns: (ids: string[]): Promise<{ deleted: number; notFound: string[] }> => { + bulkDeleteRuns: (ids: string[]): Promise<{ + deleted: number; + notFound: string[]; + membershipRevision?: ExperimentRevisionIdentityView; + }> => { return request(`/requests/bulk`, { method: "DELETE", body: JSON.stringify({ ids }), diff --git a/apps/portal/src/lib/experiment-membership.test.ts b/apps/portal/src/lib/experiment-membership.test.ts new file mode 100644 index 00000000..e0ae1934 --- /dev/null +++ b/apps/portal/src/lib/experiment-membership.test.ts @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, expect, it } from "vitest"; +import type { Run } from "@/types"; +import { getExistingRunMembershipEligibility } from "./experiment-membership"; + +const run = (id: string, experimentId?: string): Run => ({ + _id: id, + id, + workerType: "copilot", + createdAt: "2026-10-16T00:00:00.000Z", + experimentId, +}); + +describe("existing-run Experiment eligibility", () => { + it("allows a fully loaded standalone selection", () => { + expect( + getExistingRunMembershipEligibility( + new Set(["request-1", "request-2"]), + [run("request-1"), run("request-2")], + ), + ).toEqual({ eligible: true, assignedCount: 0 }); + }); + + it("disables and explains selections containing an Experiment member", () => { + const result = getExistingRunMembershipEligibility( + new Set(["request-1", "request-2"]), + [run("request-1"), run("request-2", "experiment-1")], + ); + + expect(result.eligible).toBe(false); + expect(result.assignedCount).toBe(1); + expect(result.reason).toContain("already assigned"); + }); +}); + diff --git a/apps/portal/src/lib/experiment-membership.ts b/apps/portal/src/lib/experiment-membership.ts new file mode 100644 index 00000000..9de4ba9b --- /dev/null +++ b/apps/portal/src/lib/experiment-membership.ts @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Run } from "@/types"; + +export interface ExistingRunMembershipEligibility { + eligible: boolean; + assignedCount: number; + reason?: string; +} + +export function getExistingRunMembershipEligibility( + selectedIds: ReadonlySet, + loadedSelectedRuns: Run[], +): ExistingRunMembershipEligibility { + if (selectedIds.size === 0) { + return { eligible: false, assignedCount: 0, reason: "Select at least one run." }; + } + if (loadedSelectedRuns.length !== selectedIds.size) { + return { + eligible: false, + assignedCount: 0, + reason: "Return to the page where every selected run is visible to verify eligibility.", + }; + } + const assignedCount = loadedSelectedRuns.filter((run) => Boolean(run.experimentId)).length; + if (assignedCount > 0) { + return { + eligible: false, + assignedCount, + reason: `${assignedCount} selected run${assignedCount === 1 ? " is" : "s are"} already assigned to an Experiment.`, + }; + } + return { eligible: true, assignedCount: 0 }; +} diff --git a/apps/portal/src/lib/experiments.ts b/apps/portal/src/lib/experiments.ts index 9a7ea4fe..7adad522 100644 --- a/apps/portal/src/lib/experiments.ts +++ b/apps/portal/src/lib/experiments.ts @@ -20,6 +20,23 @@ export interface ExperimentRevisionView { createdAt?: string; } +export interface ExperimentRevisionIdentityView { + experimentId: string; + experimentRevisionId: string; + revisionNumber: number; +} + +export interface ExperimentMembershipMutationView { + experiment: ExperimentView; + attachedRequestIds: string[]; + detachedRequestIds: string[]; +} + +export interface ExistingRunExperimentCreateView { + experiment: ExperimentView; + requestIds: string[]; +} + export interface ExperimentBlockerView { handlerId: string; reason: string; diff --git a/apps/portal/src/pages/ExperimentDetail.test.tsx b/apps/portal/src/pages/ExperimentDetail.test.tsx index aae87d1b..d7c039fc 100644 --- a/apps/portal/src/pages/ExperimentDetail.test.tsx +++ b/apps/portal/src/pages/ExperimentDetail.test.tsx @@ -15,6 +15,7 @@ const apiMocks = vi.hoisted(() => ({ getExperimentExecution: vi.fn(), listExperimentReports: vi.fn(), retryExperiment: vi.fn(), + updateExperimentRequests: vi.fn(), experimentReportStructuredUrl: vi.fn( () => "/api/v1/experiments/experiment-1/executions/execution-1/reports/report-1/structured?projectId=project-1", ), @@ -169,4 +170,52 @@ describe("ExperimentDetail reports", () => { await screen.findByText("No built-in report is available for this execution."), ).toBeDefined(); }); + + it("removes selected current members in one revision-guarded request", async () => { + apiMocks.getExperiment.mockResolvedValue(experiment); + apiMocks.listExperimentExecutions.mockResolvedValue({ + items: [currentExecution], + }); + apiMocks.getExperimentExecution.mockResolvedValue(currentExecution); + apiMocks.listExperimentReports.mockResolvedValue([]); + const nextExperiment: ExperimentView = { + ...experiment, + revision: { + experimentRevisionId: "revision-2", + revisionNumber: 2, + memberCount: 0, + }, + execution: { + ...currentExecution, + experimentExecutionId: "execution-3", + requestCount: 0, + requests: [], + }, + }; + apiMocks.updateExperimentRequests.mockResolvedValue({ + experiment: nextExperiment, + attachedRequestIds: [], + detachedRequestIds: ["request-2"], + }); + + renderPage("/experiments/experiment-1"); + + fireEvent.click(await screen.findByRole("checkbox", { name: "Select request-2" })); + fireEvent.click(screen.getByRole("button", { name: /Remove from experiment/ })); + fireEvent.click(screen.getByRole("button", { name: /Remove runs/ })); + + await waitFor(() => { + expect(apiMocks.updateExperimentRequests).toHaveBeenCalledTimes(1); + }); + expect(apiMocks.updateExperimentRequests).toHaveBeenCalledWith( + "DELETE", + "experiment-1", + { + expectedExperimentRevisionId: "revision-1", + requestIds: ["request-2"], + }, + ); + expect(await screen.findByText("This experiment is empty.")).toBeDefined(); + expect(screen.getByText(/Viewing revision 2/)).toBeDefined(); + }); }); diff --git a/apps/portal/src/pages/ExperimentDetail.tsx b/apps/portal/src/pages/ExperimentDetail.tsx index fbb1bdc2..6aa3aec7 100644 --- a/apps/portal/src/pages/ExperimentDetail.tsx +++ b/apps/portal/src/pages/ExperimentDetail.tsx @@ -1,8 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { ArrowLeft, FileText, RotateCcw } from "lucide-react"; +import { ArrowLeft, FileText, RotateCcw, Unlink } from "lucide-react"; import { Link, useParams, useSearchParams } from "react-router-dom"; import { toast } from "sonner"; import { ExperimentReportCard } from "@/components/ExperimentReportCard"; @@ -10,7 +11,12 @@ import { ExperimentStatusCard } from "@/components/ExperimentStatusCard"; import { ShortId } from "@/components/ShortId"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; +import { Checkbox } from "@/components/ui/checkbox"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { + AlertDialog, AlertDialogAction, AlertDialogCancel, AlertDialogContent, + AlertDialogDescription, AlertDialogFooter, AlertDialogHeader, AlertDialogTitle, +} from "@/components/ui/alert-dialog"; import { ApiError, api } from "@/lib/api"; import type { ExperimentExecutionView } from "@/lib/experiments"; import { formatDate } from "@/lib/utils"; @@ -64,6 +70,8 @@ export function ExperimentDetail() { const { id } = useParams<{ id: string }>(); const queryClient = useQueryClient(); const [searchParams, setSearchParams] = useSearchParams(); + const [selectedRequestIds, setSelectedRequestIds] = useState>(new Set()); + const [removeDialogOpen, setRemoveDialogOpen] = useState(false); const experimentQuery = useQuery({ queryKey: ["experiment", id], queryFn: () => api.getExperiment(id!), @@ -122,6 +130,38 @@ export function ExperimentDetail() { toast.error(error instanceof Error ? error.message : "Retry failed"); }, }); + const removeMembers = useMutation({ + mutationFn: () => + api.updateExperimentRequests("DELETE", experimentQuery.data!.experimentId, { + expectedExperimentRevisionId: + experimentQuery.data!.revision.experimentRevisionId, + requestIds: [...selectedRequestIds], + }), + onSuccess: ({ experiment: nextExperiment, detachedRequestIds }) => { + toast.success( + `Removed ${detachedRequestIds.length} run${detachedRequestIds.length === 1 ? "" : "s"} · revision ${nextExperiment.revision.revisionNumber}`, + ); + setSelectedRequestIds(new Set()); + setRemoveDialogOpen(false); + queryClient.setQueryData(["experiment", id], nextExperiment); + void queryClient.invalidateQueries({ queryKey: ["experiment-executions", id] }); + void queryClient.invalidateQueries({ queryKey: ["experiments"] }); + void queryClient.invalidateQueries({ queryKey: ["runs"] }); + setSearchParams( + (current) => { + const next = new URLSearchParams(current); + next.set("revision", nextExperiment.revision.experimentRevisionId); + next.set("execution", nextExperiment.execution.experimentExecutionId); + next.delete("report"); + return next; + }, + { replace: false }, + ); + }, + onError: (error) => { + toast.error(error instanceof Error ? error.message : "Runs could not be removed"); + }, + }); if (experimentQuery.isLoading) { return ( @@ -236,6 +276,16 @@ export function ExperimentDetail() { + {searchParams.get("revision") && ( +

+ Viewing revision {experiment.revision.revisionNumber}{" "} + +

+ )} + - - Current runs + +
+ Current runs +

+ Revision {experiment.revision.revisionNumber} · {experiment.revision.memberCount} members +

+
+ - {!selectedExecution || selectedExecution.requests.length === 0 ? ( + {experiment.execution.requests.length === 0 ? (

{experiment.execution.requestCount === 0 ? "This experiment is empty." : `${experiment.execution.requestCount} linked runs are stored outside the inline response.`}

) : ( - selectedExecution.requests.map((request) => ( - ( +
-
-
- - - {request.task ?? request.requestId} - + + setSelectedRequestIds((current) => { + const next = new Set(current); + if (checked) next.add(request.requestId); + else next.delete(request.requestId); + return next; + }) + } + /> + +
+
+ + + {request.task ?? request.requestId} + +
+
+ {request.requestId} · {request.runId} +
-
- {request.requestId} · {request.runId} +
+ {request.status && ( + {request.status} + )} + {request.outcome && ( + {request.outcome} + )}
-
-
- {request.status && ( - {request.status} - )} - {request.outcome && ( - {request.outcome} - )} -
- + +
)) )} + + + + + Remove {selectedRequestIds.size} run + {selectedRequestIds.size === 1 ? "" : "s"} from this Experiment? + + + One membership request will publish a new immutable revision based on + revision {experiment.revision.revisionNumber}. The runs themselves will + not be deleted. + + + + Cancel + { + event.preventDefault(); + removeMembers.mutate(); + }} + disabled={removeMembers.isPending || selectedRequestIds.size === 0} + > + {removeMembers.isPending ? "Removing…" : "Remove runs"} + + + + + Execution history diff --git a/apps/portal/src/pages/RunsList.tsx b/apps/portal/src/pages/RunsList.tsx index 31007a29..a1674bee 100644 --- a/apps/portal/src/pages/RunsList.tsx +++ b/apps/portal/src/pages/RunsList.tsx @@ -4,7 +4,7 @@ import { useMemo, useState, useEffect, useCallback, Fragment, type Key, type ReactNode } from "react"; import { useQuery, useMutation, useQueryClient, keepPreviousData } from "@tanstack/react-query"; import { Link, useNavigate, useOutlet, useParams, useSearchParams } from "react-router-dom"; -import { Trash2, Repeat, RotateCcw, Pause, Play, ChevronDown, ChevronRight, Apple, AppWindow, ArrowUpDown, FileText, Download, Lock, Eye } from "lucide-react"; +import { Trash2, Repeat, RotateCcw, Pause, Play, ChevronDown, ChevronRight, Apple, AppWindow, ArrowUpDown, FileText, Download, Lock, Eye, FlaskConical, MoveRight } from "lucide-react"; import { FaLinux } from "react-icons/fa"; import { toast } from "sonner"; import { api } from "@/lib/api"; @@ -14,6 +14,9 @@ import { AlertDialog, AlertDialogAction, AlertDialogCancel, AlertDialogContent, AlertDialogDescription, AlertDialogFooter, AlertDialogHeader, AlertDialogTitle, AlertDialogTrigger, } from "@/components/ui/alert-dialog"; +import { + Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle, +} from "@/components/ui/dialog"; import { DropdownMenu, DropdownMenuContent, @@ -52,6 +55,7 @@ import { useDebounce } from "@/hooks/useDebounce"; import { useStrictAgentCapabilities } from "@/hooks/useStrictAgentCapabilities"; import { useModelCapabilities, ModelSelectItems } from "@/components/ReasoningEffortSelect"; import { formatDate, formatId, formatDuration, truncate, cn } from "@/lib/utils"; +import { getExistingRunMembershipEligibility } from "@/lib/experiment-membership"; import { isAgentAvailable, isAgentVersionAvailable, STATUS_LIST, OUTCOME_LIST } from "@/types"; import type { Run, RunStatus, RunOutcome, IterationOp, BulkResubmitOverrides, RunSortField, RunFacetBucket } from "@/types"; import { buildRunWorkerFilterOptions } from "./run-worker-filter-options"; @@ -524,6 +528,10 @@ export function RunsList() { const [resubmitDialogOpen, setResubmitDialogOpen] = useState(false); const [resubmitCount, setResubmitCount] = useState(1); const [resubmitOverrides, setResubmitOverrides] = useState({}); + const [createExperimentDialogOpen, setCreateExperimentDialogOpen] = useState(false); + const [moveExperimentDialogOpen, setMoveExperimentDialogOpen] = useState(false); + const [experimentName, setExperimentName] = useState(""); + const [targetExperimentId, setTargetExperimentId] = useState(""); // Reset stack whenever filters or page size change. const filtersKey = useMemo( @@ -703,6 +711,12 @@ export function RunsList() { queryFn: () => api.listProfiles(), staleTime: 60_000, }); + const experimentsQuery = useQuery({ + queryKey: ["experiments", "runs-membership"], + queryFn: () => api.listExperiments({ limit: 100 }), + enabled: moveExperimentDialogOpen, + staleTime: 30_000, + }); // Registry data also supplies worker labels for filters and historical values. const { data: agentsData = [] } = useQuery({ queryKey: ["agents", "include-deleted"], @@ -901,9 +915,17 @@ export function RunsList() { const bulkDeleteMutation = useMutation({ mutationFn: (ids: string[]) => api.bulkDeleteRuns(ids), onSuccess: (res) => { - toast.success(`Deleted ${res.deleted} run${res.deleted !== 1 ? "s" : ""}`); + const revision = res.membershipRevision; + toast.success( + `Deleted ${res.deleted} run${res.deleted !== 1 ? "s" : ""}` + + (revision ? ` · Experiment revision ${revision.revisionNumber} (${formatId(revision.experimentRevisionId)})` : ""), + ); setSelectedIds(new Set()); invalidateRuns(); + if (revision) { + void queryClient.invalidateQueries({ queryKey: ["experiment", revision.experimentId] }); + void queryClient.invalidateQueries({ queryKey: ["experiments"] }); + } }, onError: (err: Error) => toast.error(`Failed to delete: ${err.message}`), }); @@ -1041,6 +1063,63 @@ export function RunsList() { () => allRuns.filter((r) => selectedIds.has(r._id)), [allRuns, selectedIds], ); + const experimentMembershipEligibility = useMemo( + () => getExistingRunMembershipEligibility(selectedIds, selectedRunsList), + [selectedIds, selectedRunsList], + ); + const selectedExperimentMemberCount = useMemo( + () => selectedRunsList.filter((run) => Boolean(run.experimentId)).length, + [selectedRunsList], + ); + + const createExperimentMutation = useMutation({ + mutationFn: () => + api.createExperimentFromRuns({ + metadata: { name: experimentName.trim() }, + requestIds: [...selectedIds], + }), + onSuccess: ({ experiment }) => { + toast.success( + `Created ${experiment.metadata.name} · revision ${experiment.revision.revisionNumber}`, + ); + setSelectedIds(new Set()); + setExperimentName(""); + setCreateExperimentDialogOpen(false); + void queryClient.invalidateQueries({ queryKey: ["runs"] }); + void queryClient.invalidateQueries({ queryKey: ["experiments"] }); + navigate( + `/experiments/${encodeURIComponent(experiment.experimentId)}?revision=${encodeURIComponent(experiment.revision.experimentRevisionId)}&execution=${encodeURIComponent(experiment.execution.experimentExecutionId)}`, + ); + }, + onError: (error: Error) => toast.error(`Failed to create Experiment: ${error.message}`), + }); + + const moveToExperimentMutation = useMutation({ + mutationFn: () => { + const target = experimentsQuery.data?.items.find( + (experiment) => experiment.experimentId === targetExperimentId, + ); + if (!target) throw new Error("Choose an Experiment"); + return api.updateExperimentRequests("POST", target.experimentId, { + expectedExperimentRevisionId: target.revision.experimentRevisionId, + requestIds: [...selectedIds], + }); + }, + onSuccess: ({ experiment, attachedRequestIds }) => { + toast.success( + `Moved ${attachedRequestIds.length} run${attachedRequestIds.length === 1 ? "" : "s"} · revision ${experiment.revision.revisionNumber}`, + ); + setSelectedIds(new Set()); + setTargetExperimentId(""); + setMoveExperimentDialogOpen(false); + void queryClient.invalidateQueries({ queryKey: ["runs"] }); + void queryClient.invalidateQueries({ queryKey: ["experiments"] }); + navigate( + `/experiments/${encodeURIComponent(experiment.experimentId)}?revision=${encodeURIComponent(experiment.revision.experimentRevisionId)}&execution=${encodeURIComponent(experiment.execution.experimentExecutionId)}`, + ); + }, + onError: (error: Error) => toast.error(`Failed to move runs: ${error.message}`), + }); // Capability counts so action buttons can disable cleanly. const selectionCaps = useMemo(() => { @@ -1211,6 +1290,8 @@ export function RunsList() { const isBusy = bulkDeleteMutation.isPending || + createExperimentMutation.isPending || + moveToExperimentMutation.isPending || bulkRetryMutation.isPending || bulkResubmitMutation.isPending || bulkPauseMutation.isPending || @@ -2137,6 +2218,33 @@ export function RunsList() { Runs + + + {!experimentMembershipEligibility.eligible && + experimentMembershipEligibility.reason && + selectedIds.size > 0 && ( + + {experimentMembershipEligibility.reason} + + )} + + + + + + { + setMoveExperimentDialogOpen(open); + if (!open) setTargetExperimentId(""); + }} + > + + + Move to experiment + + Add the {selectedIds.size} selected standalone run + {selectedIds.size === 1 ? "" : "s"} to the target's current revision. + + +
+ + + {experimentsQuery.isError && ( +

+ {experimentsQuery.error instanceof Error + ? experimentsQuery.error.message + : "Experiments could not be loaded."} +

+ )} + {experimentsQuery.data?.items.length === 0 && ( +

+ No target Experiments are available. +

+ )} +
+ + + + + + + @@ -3250,6 +3461,14 @@ export function RunsList() { This permanently removes the selected runs along with their logs, turns, and report associations. This action cannot be undone. + {selectedExperimentMemberCount > 0 && ( + + {selectedExperimentMemberCount} selected run + {selectedExperimentMemberCount === 1 ? " is" : "s are"} an Experiment + member. Deleting {selectedExperimentMemberCount === 1 ? "it" : "them"} will + publish a new Experiment revision without those runs. + + )} From c887ad002b66d012240d85341c2e87f759c00cce Mon Sep 17 00:00:00 2001 From: Cedric Vidal Date: Fri, 16 Oct 2026 09:28:09 -0700 Subject: [PATCH 12/16] test: catalog experiment membership actions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d995fd8-10c6-477c-b1d4-27ba9b2d762a --- .../ExperimentMembershipActions.stories.tsx | 56 ++++++++++++++++++ .../ExperimentMembershipActions.tsx | 59 +++++++++++++++++++ apps/portal/src/pages/RunsList.tsx | 37 +++--------- 3 files changed, 124 insertions(+), 28 deletions(-) create mode 100644 apps/portal/src/components/ExperimentMembershipActions.stories.tsx create mode 100644 apps/portal/src/components/ExperimentMembershipActions.tsx diff --git a/apps/portal/src/components/ExperimentMembershipActions.stories.tsx b/apps/portal/src/components/ExperimentMembershipActions.stories.tsx new file mode 100644 index 00000000..89ccc896 --- /dev/null +++ b/apps/portal/src/components/ExperimentMembershipActions.stories.tsx @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, fn, userEvent } from "storybook/test"; +import { ExperimentMembershipActions } from "./ExperimentMembershipActions"; + +const meta = { + title: "Experiments/Membership Actions", + component: ExperimentMembershipActions, + args: { + selectedCount: 2, + busy: false, + onCreate: fn(), + onMove: fn(), + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const EligibleStandaloneRuns: Story = { + args: { + eligibility: { eligible: true, assignedCount: 0 }, + }, + play: async ({ canvas, args }) => { + await userEvent.click( + canvas.getByRole("button", { name: "Create experiment" }), + ); + await expect(args.onCreate).toHaveBeenCalledOnce(); + await expect( + canvas.getByRole("button", { name: "Move to experiment" }), + ).toBeEnabled(); + }, +}; + +export const AssignedRunConflict: Story = { + args: { + eligibility: { + eligible: false, + assignedCount: 1, + reason: "1 selected run is already assigned to an Experiment.", + }, + }, + play: async ({ canvas }) => { + await expect( + canvas.getByText("1 selected run is already assigned to an Experiment."), + ).toBeVisible(); + await expect( + canvas.getByRole("button", { name: "Create experiment" }), + ).toBeDisabled(); + await expect( + canvas.getByRole("button", { name: "Move to experiment" }), + ).toBeDisabled(); + }, +}; diff --git a/apps/portal/src/components/ExperimentMembershipActions.tsx b/apps/portal/src/components/ExperimentMembershipActions.tsx new file mode 100644 index 00000000..b852352b --- /dev/null +++ b/apps/portal/src/components/ExperimentMembershipActions.tsx @@ -0,0 +1,59 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { FlaskConical, MoveRight } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import type { ExistingRunMembershipEligibility } from "@/lib/experiment-membership"; + +export interface ExperimentMembershipActionsProps { + eligibility: ExistingRunMembershipEligibility; + selectedCount: number; + busy?: boolean; + onCreate: () => void; + onMove: () => void; +} + +export function ExperimentMembershipActions({ + eligibility, + selectedCount, + busy = false, + onCreate, + onMove, +}: ExperimentMembershipActionsProps) { + const disabled = busy || !eligibility.eligible; + return ( + <> + + + {!eligibility.eligible && eligibility.reason && selectedCount > 0 && ( + + {eligibility.reason} + + )} + + ); +} diff --git a/apps/portal/src/pages/RunsList.tsx b/apps/portal/src/pages/RunsList.tsx index a1674bee..e4422b46 100644 --- a/apps/portal/src/pages/RunsList.tsx +++ b/apps/portal/src/pages/RunsList.tsx @@ -4,7 +4,7 @@ import { useMemo, useState, useEffect, useCallback, Fragment, type Key, type ReactNode } from "react"; import { useQuery, useMutation, useQueryClient, keepPreviousData } from "@tanstack/react-query"; import { Link, useNavigate, useOutlet, useParams, useSearchParams } from "react-router-dom"; -import { Trash2, Repeat, RotateCcw, Pause, Play, ChevronDown, ChevronRight, Apple, AppWindow, ArrowUpDown, FileText, Download, Lock, Eye, FlaskConical, MoveRight } from "lucide-react"; +import { Trash2, Repeat, RotateCcw, Pause, Play, ChevronDown, ChevronRight, Apple, AppWindow, ArrowUpDown, FileText, Download, Lock, Eye } from "lucide-react"; import { FaLinux } from "react-icons/fa"; import { toast } from "sonner"; import { api } from "@/lib/api"; @@ -56,6 +56,7 @@ import { useStrictAgentCapabilities } from "@/hooks/useStrictAgentCapabilities"; import { useModelCapabilities, ModelSelectItems } from "@/components/ReasoningEffortSelect"; import { formatDate, formatId, formatDuration, truncate, cn } from "@/lib/utils"; import { getExistingRunMembershipEligibility } from "@/lib/experiment-membership"; +import { ExperimentMembershipActions } from "@/components/ExperimentMembershipActions"; import { isAgentAvailable, isAgentVersionAvailable, STATUS_LIST, OUTCOME_LIST } from "@/types"; import type { Run, RunStatus, RunOutcome, IterationOp, BulkResubmitOverrides, RunSortField, RunFacetBucket } from "@/types"; import { buildRunWorkerFilterOptions } from "./run-worker-filter-options"; @@ -2218,33 +2219,13 @@ export function RunsList() { Runs - - - {!experimentMembershipEligibility.eligible && - experimentMembershipEligibility.reason && - selectedIds.size > 0 && ( - - {experimentMembershipEligibility.reason} - - )} + setCreateExperimentDialogOpen(true)} + onMove={() => setMoveExperimentDialogOpen(true)} + />