refactor(assistant): extract post-assembly validation into AssistantFlowValidation
Cluster M from the structural analysis: bean-validation of the assembled FlowCreateRequest plus FlowExecutionValidator's structural/execution checks (dangling connections, unconnected BranchRejoin inputs, global- input mismatches, container subflow rules). - New AssistantFlowValidation holds validate/toFallbackError - validator and flowExecutionValidator now passed as explicit parameters 1341 -> 1081 -> 1046 lines. 3350 -> 1046 total (-2304, ~69%). Behavior-preserving: pure extraction, no logic changes. This closes out the batch of medium/low-risk cluster extractions from FlowAssistantService. What remains in the file is the entry-point/ retry-loop orchestration (generateFlow, draft/refine/fix/explain), assembleFlow/assembleContainer (the central assembler - intentionally left alone, flagged in the original analysis as near-duplicated with subtle behavioral divergences, risky to touch), and the MDC request- scoped logging plumbing (an ownership invariant, left untouched). Verified with `mvn test`: 462 tests, 0 failures, 0 errors. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
756c07fe70
commit
9c7e0233a2
|
|
@ -0,0 +1,52 @@
|
|||
package it.cnr.isti.workflow.manager.assistant;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
|
||||
import it.cnr.isti.workflow.manager.flows.model.FlowCreateRequest;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.FlowExecutionValidator;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.ValidationError;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCodec;
|
||||
import jakarta.validation.ConstraintViolation;
|
||||
import jakarta.validation.Validator;
|
||||
|
||||
final class AssistantFlowValidation {
|
||||
|
||||
private AssistantFlowValidation() {
|
||||
}
|
||||
|
||||
static List<ValidationError> validate(FlowCreateRequest flow, Validator validator,
|
||||
FlowExecutionValidator flowExecutionValidator) {
|
||||
List<ValidationError> errors = new ArrayList<>();
|
||||
|
||||
Set<ConstraintViolation<FlowCreateRequest>> violations = validator.validate(flow);
|
||||
for (ConstraintViolation<FlowCreateRequest> violation : violations) {
|
||||
List<ValidationError> decoded = ValidationErrorCodec.decode(violation.getMessage());
|
||||
if (decoded.isEmpty()) {
|
||||
errors.add(toFallbackError(violation));
|
||||
continue;
|
||||
}
|
||||
for (ValidationError error : decoded) {
|
||||
if (error.message() == null || Objects.equals(error.message(), violation.getMessage())) {
|
||||
errors.add(toFallbackError(violation));
|
||||
} else {
|
||||
errors.add(error);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Bean validation alone misses structural/execution issues (dangling connections,
|
||||
// an unconnected BranchRejoin input, global-input reference mismatches, container
|
||||
// subflow rules, ...) that only surface at actual execution time otherwise.
|
||||
if (flow != null && flow.flow() != null) {
|
||||
errors.addAll(flowExecutionValidator.collectErrors(flow.flow()));
|
||||
}
|
||||
return errors;
|
||||
}
|
||||
|
||||
private static ValidationError toFallbackError(ConstraintViolation<FlowCreateRequest> violation) {
|
||||
return new ValidationError("flow", null, violation.getPropertyPath().toString(), violation.getMessage());
|
||||
}
|
||||
}
|
||||
|
|
@ -61,12 +61,10 @@ import it.cnr.isti.workflow.manager.flows.model.FlowData;
|
|||
import it.cnr.isti.workflow.manager.flows.validation.FlowExecutionValidator;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.ValidationError;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCode;
|
||||
import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCodec;
|
||||
import it.cnr.isti.workflow.manager.ios.IODescriptor;
|
||||
import it.cnr.isti.workflow.manager.ios.IOType;
|
||||
import it.cnr.isti.workflow.manager.llms.providers.LLMProvider;
|
||||
import it.cnr.isti.workflow.manager.vault.UserSecretService;
|
||||
import jakarta.validation.ConstraintViolation;
|
||||
import jakarta.validation.Validator;
|
||||
|
||||
@Service
|
||||
|
|
@ -355,7 +353,7 @@ public class FlowAssistantService {
|
|||
|
||||
public AssistantFlowResponse fix(AssistantFixRequest request, String owner, ProgressListener progressListener) {
|
||||
List<ValidationError> initialErrors = request.validationErrors() == null || request.validationErrors().isEmpty()
|
||||
? validate(request.flow())
|
||||
? AssistantFlowValidation.validate(request.flow(), validator, flowExecutionValidator)
|
||||
: request.validationErrors();
|
||||
boolean directMdc = ensureAssistantRequestMdc(OperationMode.FIX.name());
|
||||
try {
|
||||
|
|
@ -416,7 +414,7 @@ public class FlowAssistantService {
|
|||
assembled = assembleFlow(provider, authorization, assistantModel, generatedFlowProvider, generatedFlowModel,
|
||||
phaseModels, mode, userPrompt, flowContext, errorContext, progressListener);
|
||||
progressListener.onProgress("validating", "Validating the assembled flow");
|
||||
errors = validate(assembled.flow());
|
||||
errors = AssistantFlowValidation.validate(assembled.flow(), validator, flowExecutionValidator);
|
||||
if (errors.isEmpty() || repairs >= allowedRepairs) {
|
||||
break;
|
||||
}
|
||||
|
|
@ -1038,39 +1036,6 @@ public class FlowAssistantService {
|
|||
return SharedMemoryIntentClassifier.isSharedMemoryRequest(flowText.toString());
|
||||
}
|
||||
|
||||
private List<ValidationError> validate(FlowCreateRequest flow) {
|
||||
List<ValidationError> errors = new ArrayList<>();
|
||||
|
||||
Set<ConstraintViolation<FlowCreateRequest>> violations = validator.validate(flow);
|
||||
for (ConstraintViolation<FlowCreateRequest> violation : violations) {
|
||||
List<ValidationError> decoded = ValidationErrorCodec.decode(violation.getMessage());
|
||||
if (decoded.isEmpty()) {
|
||||
errors.add(toFallbackError(violation));
|
||||
continue;
|
||||
}
|
||||
for (ValidationError error : decoded) {
|
||||
if (error.message() == null || Objects.equals(error.message(), violation.getMessage())) {
|
||||
errors.add(toFallbackError(violation));
|
||||
} else {
|
||||
errors.add(error);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Bean validation alone misses structural/execution issues (dangling connections,
|
||||
// an unconnected BranchRejoin input, global-input reference mismatches, container
|
||||
// subflow rules, ...) that only surface at actual execution time otherwise.
|
||||
if (flow != null && flow.flow() != null) {
|
||||
errors.addAll(flowExecutionValidator.collectErrors(flow.flow()));
|
||||
}
|
||||
return errors;
|
||||
}
|
||||
|
||||
private ValidationError toFallbackError(ConstraintViolation<FlowCreateRequest> violation) {
|
||||
return new ValidationError("flow", null, violation.getPropertyPath().toString(), violation.getMessage());
|
||||
}
|
||||
|
||||
|
||||
private void appendRationale(List<String> target, String rationale) {
|
||||
if (rationale != null && !rationale.isBlank()) {
|
||||
target.add(rationale.trim());
|
||||
|
|
|
|||
Loading…
Reference in New Issue