Show each loop step's round, and refuse answers for a round gone by
Execution views now carry the round each step is on and the steps of every earlier round, which is what a page needs to show a loop's history. A node in a loop asks the same question once per round. The interaction and evaluation endpoints take an optional iteration: an answer for a round the loop has already left is refused with 409 instead of being taken as the answer to a draft its author never saw. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
b54c4ef35d
commit
d4429a5ff7
|
|
@ -421,9 +421,11 @@ public class ExecutionsController {
|
|||
@Operation(summary = "Provides human interaction output", description = "Provides the value for a waiting human interaction step.")
|
||||
public ExecutionView provideInteractionValue(@PathVariable String executionId, @PathVariable String nodeId,
|
||||
@PathVariable String fieldName, @RequestBody String value,
|
||||
@RequestParam(required = false) Integer iteration,
|
||||
@AuthenticationPrincipal LoginEntity userDetails) {
|
||||
return ExecutionView.fromExecution(
|
||||
executionService.setInteractionValue(visibleExecution(executionId, userDetails).getId(), nodeId, fieldName, value));
|
||||
String id = visibleExecution(executionId, userDetails).getId();
|
||||
requireRound(id, nodeId, iteration);
|
||||
return ExecutionView.fromExecution(executionService.setInteractionValue(id, nodeId, fieldName, value));
|
||||
}
|
||||
|
||||
@PutMapping(path = "{executionId}/node/{nodeId}/evaluation", consumes = "application/json")
|
||||
|
|
@ -433,7 +435,9 @@ public class ExecutionsController {
|
|||
+ "scale the criterion declares, in which case nothing is recorded.")
|
||||
public ExecutionView submitEvaluation(@PathVariable String executionId, @PathVariable String nodeId,
|
||||
@RequestBody @jakarta.validation.Valid HumanEvaluationRequest request,
|
||||
@RequestParam(required = false) Integer iteration,
|
||||
@AuthenticationPrincipal LoginEntity userDetails) {
|
||||
requireRound(visibleExecution(executionId, userDetails).getId(), nodeId, iteration);
|
||||
Map<String, Object> interaction = new LinkedHashMap<>(request.verdict());
|
||||
if (request.notes() != null) {
|
||||
interaction.put(HumanEvaluationBlockFactory.NOTES_FIELD, request.notes());
|
||||
|
|
@ -454,7 +458,9 @@ public class ExecutionsController {
|
|||
description = "Adds files - screenshots, recordings, exports - to a waiting HumanEvaluation step. They "
|
||||
+ "accumulate across calls and leave on the node's evidence output once the judgement is submitted.")
|
||||
public ExecutionView attachEvaluationEvidence(@PathVariable String executionId, @PathVariable String nodeId,
|
||||
@RequestParam List<MultipartFile> files, @AuthenticationPrincipal LoginEntity userDetails) {
|
||||
@RequestParam List<MultipartFile> files, @RequestParam(required = false) Integer iteration,
|
||||
@AuthenticationPrincipal LoginEntity userDetails) {
|
||||
requireRound(visibleExecution(executionId, userDetails).getId(), nodeId, iteration);
|
||||
List<File> stored = new ArrayList<>();
|
||||
try {
|
||||
for (MultipartFile file : files) {
|
||||
|
|
@ -477,12 +483,30 @@ public class ExecutionsController {
|
|||
+ "node configured to keep it hidden. The execution records the reveal, so a judgement made after "
|
||||
+ "it can be told from one made blind - which is the whole point of hiding it.")
|
||||
public ExecutionView revealEvaluationReference(@PathVariable String executionId, @PathVariable String nodeId,
|
||||
@RequestParam(required = false) Integer iteration,
|
||||
@AuthenticationPrincipal LoginEntity userDetails) {
|
||||
return ExecutionView.fromExecution(executionService.setInteractionValues(
|
||||
visibleExecution(executionId, userDetails).getId(), nodeId,
|
||||
String id = visibleExecution(executionId, userDetails).getId();
|
||||
requireRound(id, nodeId, iteration);
|
||||
return ExecutionView.fromExecution(executionService.setInteractionValues(id, nodeId,
|
||||
Map.of(HumanEvaluationExecutor.REFERENCE_REVEALED_FIELD, Boolean.TRUE)));
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuses an answer meant for another round of a loop. A node in a loop asks the same question
|
||||
* once per round, and an answer given in one tab while another has already moved the loop on
|
||||
* would otherwise be taken as the answer to a draft its author never saw.
|
||||
*/
|
||||
private void requireRound(String executionId, String nodeId, Integer iteration) {
|
||||
if (iteration == null) {
|
||||
return;
|
||||
}
|
||||
var step = executionService.getExecution(executionId).getContext().getSteps().get(nodeId);
|
||||
if (step != null && step.getIteration() != iteration) {
|
||||
throw new ResponseStatusException(HttpStatus.CONFLICT, "This answer is for round " + iteration
|
||||
+ " of the loop, but the node is now on round " + step.getIteration() + ". Reload to see what it asks now");
|
||||
}
|
||||
}
|
||||
|
||||
@PutMapping(path = "{executionId}/authorizations")
|
||||
@Operation(summary = "Provides execution authorization",
|
||||
description = "Stores the selected saved credential reference for a provider authorization required by an "
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
package it.cnr.isti.workflow.manager.executions.api;
|
||||
|
||||
import it.cnr.isti.workflow.manager.executions.persistence.ExecutionStepSnapshot;
|
||||
import java.util.Map;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
|
|
@ -36,6 +37,8 @@ public class ExecutionContextView {
|
|||
private java.util.List<String> warnings;
|
||||
private java.util.List<ExecutionOutcome> outcomes;
|
||||
private Map<String, ExecutionStepView> steps;
|
||||
/** Loop steps as each earlier round left them, oldest first; the steps above are the current round. */
|
||||
private java.util.List<ExecutionStepSnapshot> stepHistory;
|
||||
private ExecutionStatus status;
|
||||
private java.util.List<String> waitingSteps;
|
||||
|
||||
|
|
@ -62,6 +65,7 @@ public class ExecutionContextView {
|
|||
ExecutionStepView::fromStep,
|
||||
(left, right) -> right,
|
||||
java.util.LinkedHashMap::new)))
|
||||
.stepHistory(context.getStepHistory())
|
||||
.status(context.getStatus())
|
||||
.waitingSteps(context.getWaitingSteps())
|
||||
.build();
|
||||
|
|
|
|||
|
|
@ -30,6 +30,8 @@ public class ExecutionStepView {
|
|||
private List<Input> inputs;
|
||||
private List<Output> outputs;
|
||||
private StepStatus status;
|
||||
/** Which round of its loop the step is on; 1 for a step in no loop. */
|
||||
private int iteration;
|
||||
private StepSkipReason skipReason;
|
||||
private boolean simulated;
|
||||
private String activeInnerExecutionId;
|
||||
|
|
@ -44,6 +46,7 @@ public class ExecutionStepView {
|
|||
.inputs(step.getInputs())
|
||||
.outputs(step.getOutputs())
|
||||
.status(step.getStatus())
|
||||
.iteration(step.getIteration())
|
||||
.skipReason(step.getSkipReason())
|
||||
.simulated(step.isSimulated())
|
||||
.activeInnerExecutionId(step.getContainerContinuation() == null
|
||||
|
|
|
|||
|
|
@ -312,7 +312,7 @@ public class ExecutionControllerTest {
|
|||
ExecutionView execution = waitingEvaluation(reviewBlock);
|
||||
|
||||
execution = executionsController.submitEvaluation(execution.getId(), reviewBlock.getId(),
|
||||
new HumanEvaluationRequest(Map.of("works", "fail", "clarity", "2"), "Checkout breaks"), testUser());
|
||||
new HumanEvaluationRequest(Map.of("works", "fail", "clarity", "2"), "Checkout breaks"), null, testUser());
|
||||
|
||||
assertEquals(Map.of("works", "fail", "clarity", "2"), resultOf(execution, reviewBlock.getId(), "verdict"));
|
||||
assertEquals("fail", resultOf(execution, reviewBlock.getId(), "works"));
|
||||
|
|
@ -331,10 +331,10 @@ public class ExecutionControllerTest {
|
|||
.build());
|
||||
|
||||
ExecutionView execution = waitingEvaluation(reviewBlock);
|
||||
executionsController.revealEvaluationReference(execution.getId(), reviewBlock.getId(), testUser());
|
||||
executionsController.revealEvaluationReference(execution.getId(), reviewBlock.getId(), null, testUser());
|
||||
|
||||
execution = executionsController.submitEvaluation(execution.getId(), reviewBlock.getId(),
|
||||
new HumanEvaluationRequest(Map.of("works", "pass"), ""), testUser());
|
||||
new HumanEvaluationRequest(Map.of("works", "pass"), ""), null, testUser());
|
||||
|
||||
assertEquals(false, resultOf(execution, reviewBlock.getId(), "blind"));
|
||||
}
|
||||
|
|
@ -353,7 +353,7 @@ public class ExecutionControllerTest {
|
|||
ResponseStatusException failure =
|
||||
assertThrows(ResponseStatusException.class,
|
||||
() -> executionsController.submitEvaluation(execution.getId(), reviewBlock.getId(),
|
||||
new HumanEvaluationRequest(Map.of("works", "almost"), ""), testUser()));
|
||||
new HumanEvaluationRequest(Map.of("works", "almost"), ""), null, testUser()));
|
||||
|
||||
assertEquals(HttpStatus.BAD_REQUEST, failure.getStatusCode());
|
||||
assertTrue(failure.getReason().contains("works"));
|
||||
|
|
@ -436,6 +436,7 @@ public class ExecutionControllerTest {
|
|||
reviewBlock.getId(),
|
||||
"output",
|
||||
"Approved",
|
||||
null,
|
||||
testUser());
|
||||
|
||||
waitForExecutionStatus(executionObject, ExecutionStatus.SUCCESS);
|
||||
|
|
@ -808,7 +809,7 @@ public class ExecutionControllerTest {
|
|||
waitForExecutionStatus(executionObject, ExecutionStatus.WAITING);
|
||||
|
||||
executionsController.provideInteractionValue(executionObject.getId(), reviewBlock.getId(), "output", "Approved",
|
||||
testUser());
|
||||
null, testUser());
|
||||
|
||||
waitForExecutionStatus(executionObject, ExecutionStatus.SUCCESS);
|
||||
}
|
||||
|
|
@ -1015,7 +1016,7 @@ public class ExecutionControllerTest {
|
|||
org.junit.jupiter.api.Assertions.assertEquals(ExecutionStatus.WAITING, resumed.getContext().getStatus());
|
||||
|
||||
resumed = executionsController.provideInteractionValue(executionId, reviewBlock.getId(), "output", "Approved",
|
||||
testUser());
|
||||
null, testUser());
|
||||
waitForExecutionStatus(resumed, ExecutionStatus.SUCCESS);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ package it.cnr.isti.workflow.manager.executions;
|
|||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
import java.time.Duration;
|
||||
|
|
@ -17,8 +18,12 @@ import org.springframework.beans.factory.annotation.Autowired;
|
|||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.boot.test.context.TestConfiguration;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.test.context.TestPropertySource;
|
||||
import org.springframework.web.server.ResponseStatusException;
|
||||
|
||||
import it.cnr.isti.workflow.manager.auth.repo.LoginEntity;
|
||||
import it.cnr.isti.workflow.manager.controllers.ExecutionsController;
|
||||
import it.cnr.isti.workflow.manager.blocks.Block;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.ConditionalBlockConfiguration;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.EndBlockConfiguration;
|
||||
|
|
@ -88,9 +93,14 @@ class LoopExecutionTest {
|
|||
}
|
||||
}
|
||||
|
||||
private static final LoginEntity USER = new LoginEntity("testuser", "testpassword");
|
||||
|
||||
@Autowired
|
||||
ExecutionsService executionsService;
|
||||
|
||||
@Autowired
|
||||
ExecutionsController executionsController;
|
||||
|
||||
@Autowired
|
||||
LLMBlockFactory llmBlockFactory;
|
||||
|
||||
|
|
@ -215,7 +225,7 @@ class LoopExecutionTest {
|
|||
.connection(connection(review, "approve", approved, EndBlockFactory.INPUT_NAME))
|
||||
.build();
|
||||
|
||||
ExecutionObject execution = executionsService.createExecution("Reviewed until approved", flow);
|
||||
ExecutionObject execution = executionsService.createExecution("Reviewed until approved", flow, USER.getUsername());
|
||||
executionsService.prepareInput(execution.getId(), draft.getId(), "topic", "idea");
|
||||
executionsService.startExecution(execution.getId());
|
||||
|
||||
|
|
@ -226,6 +236,16 @@ class LoopExecutionTest {
|
|||
}
|
||||
awaitStepWaiting(execution.getId(), review.getId(), 3);
|
||||
|
||||
// An answer sent from a page still showing round 2 must not land on round 3's draft.
|
||||
String executionId = execution.getId();
|
||||
ResponseStatusException stale = assertThrows(ResponseStatusException.class,
|
||||
() -> executionsController.provideInteractionValue(executionId, review.getId(),
|
||||
HumanDecisionBlockFactory.CHOICE_FIELD, "approve", 2, USER));
|
||||
assertEquals(HttpStatus.CONFLICT, stale.getStatusCode());
|
||||
assertEquals(StepStatus.WAITING_FOR_INTERACTION,
|
||||
executionsService.getExecution(executionId).getContext().getSteps().get(review.getId()).getStatus(),
|
||||
"the refused answer left the question open");
|
||||
|
||||
// Read back from storage in the middle of the loop: the round it is on, and the rounds
|
||||
// before, have to survive the trip.
|
||||
executionsService.clearInMemoryExecutions();
|
||||
|
|
|
|||
Loading…
Reference in New Issue