feat(validation): attribute subflow errors to the container and the inner element
Errors produced inside a container's subflow were remapped to the
container (entity=container, id=containerId, field=specificConfiguration.subFlow)
but the inner element's own id was overwritten and lost, so a client
could tell WHICH container had a problem but not WHICH inner node/
connection. And a LoopContainer's guardSubFlow errors only surfaced as
a single CONTAINER_SUBFLOW_INVALID whose message was a nested JSON blob.
- remapSubFlowErrors now preserves the offending inner element's id in
relatedNodeIds (deduped, forward of any it already carried), so the UI
can highlight the specific inner node/connection when the container is
opened - not just the container.
- The guard subflow is now recursed and exploded the same way as the
body (field specificConfiguration.guardSubFlow), so guard errors come
through as individual, typed, node-pointed errors instead of a nested
JSON blob. No-op for a valid (backend-generated) guard, so no
regression for normal flows.
Test: a container whose subflow has a dangling inner connection saves as
a draft, and GET /flows/{id}/validation reports the error with
entity=container, the container id, field=specificConfiguration.subFlow,
and the inner connection id in relatedNodeIds. 446/446 (excl. the known
loop-timing flake).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d5d4118cba
commit
f297a057de
|
|
@ -157,15 +157,21 @@ public class FlowExecutionValidator {
|
|||
|
||||
Set<String> subFlowExternalSessions = externalSessionsAvailableTo(container, flowData,
|
||||
externalSessions);
|
||||
errors.addAll(collectErrors(containerConfiguration.getSubFlow(), subFlowExternalSessions).stream()
|
||||
.map(error -> new ValidationError(
|
||||
error.code(),
|
||||
"container",
|
||||
container.getId(),
|
||||
"specificConfiguration.subFlow",
|
||||
error.message(),
|
||||
error.relatedNodeIds()))
|
||||
.toList());
|
||||
errors.addAll(remapSubFlowErrors(
|
||||
collectErrors(containerConfiguration.getSubFlow(), subFlowExternalSessions),
|
||||
container.getId(),
|
||||
"specificConfiguration.subFlow"));
|
||||
if (containerConfiguration instanceof LoopContainerConfiguration loopConfiguration
|
||||
&& loopConfiguration.getGuardSubFlow() != null
|
||||
&& !loopConfiguration.getGuardSubFlow().getNodes().isEmpty()) {
|
||||
// Explode the guard subflow's own errors too (attributed to this container,
|
||||
// field guardSubFlow) so the UI can show them per-node like the body's,
|
||||
// instead of only as a nested JSON blob inside a CONTAINER_SUBFLOW_INVALID.
|
||||
errors.addAll(remapSubFlowErrors(
|
||||
collectErrors(loopConfiguration.getGuardSubFlow(), subFlowExternalSessions),
|
||||
container.getId(),
|
||||
"specificConfiguration.guardSubFlow"));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -179,6 +185,39 @@ public class FlowExecutionValidator {
|
|||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-tags every error produced inside a container's subflow so a client can attribute it to the
|
||||
* enclosing container: entity {@code container}, the container's id, and {@code field} set to the
|
||||
* subflow it came from ({@code specificConfiguration.subFlow} or {@code ...guardSubFlow}). The
|
||||
* inner element's own id (the offending block/connection) is preserved into {@code relatedNodeIds}
|
||||
* - it would otherwise be lost when the id is overwritten by the container id - so the UI can
|
||||
* still highlight the specific inner node/connection, not just the container.
|
||||
*/
|
||||
private List<ValidationError> remapSubFlowErrors(List<ValidationError> innerErrors, String containerId,
|
||||
String field) {
|
||||
return innerErrors.stream()
|
||||
.map(error -> new ValidationError(
|
||||
error.code(),
|
||||
"container",
|
||||
containerId,
|
||||
field,
|
||||
error.message(),
|
||||
withInnerElementId(error.id(), error.relatedNodeIds())))
|
||||
.toList();
|
||||
}
|
||||
|
||||
private List<String> withInnerElementId(String innerId, List<String> existing) {
|
||||
if (innerId == null || innerId.isBlank()) {
|
||||
return existing;
|
||||
}
|
||||
List<String> merged = new ArrayList<>();
|
||||
merged.add(innerId);
|
||||
if (existing != null) {
|
||||
existing.stream().filter(id -> !merged.contains(id)).forEach(merged::add);
|
||||
}
|
||||
return List.copyOf(merged);
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared-MCP-session names available to a container's subflow: those inherited from the
|
||||
* enclosing scope, plus those produced by a top-level block in {@code flowData} that runs
|
||||
|
|
|
|||
|
|
@ -864,6 +864,54 @@ public class FlowControllerTest {
|
|||
() -> "expected CONNECTION_TARGET_INPUT_NOT_FOUND among execution errors: " + validation);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void containerSubFlowValidationErrorIsAttributedToContainerAndInnerElement() {
|
||||
// An error inside a container's subflow must be attributable by a client to (a) the specific
|
||||
// container and (b) the offending inner element - so the editor can surface it when that
|
||||
// container is opened and still highlight the inner node/connection, not just "the flow is
|
||||
// invalid". The inner element's id is preserved in relatedNodeIds even though the error's own
|
||||
// id is re-tagged to the container.
|
||||
LLMDescriptor llmDescriptor = LLMDescriptor.builder().provider("testProvider").model("testModel").build();
|
||||
Block<LLMBlockType> inner1 = blocksController.create(LLMBlockConfiguration.builder()
|
||||
.name("Analyze").llmDescriptor(llmDescriptor).prompt("Analyze ${{candidate}}").build());
|
||||
Block<LLMBlockType> inner2 = blocksController.create(LLMBlockConfiguration.builder()
|
||||
.name("Score").llmDescriptor(llmDescriptor).prompt("Score ${{profile}}").build());
|
||||
// inner2's real input is "profile"; this connection targets a non-existent one.
|
||||
Connection danglingInner = Connection.builder()
|
||||
.sourceId(inner1.getId()).sourceName("response")
|
||||
.targetId(inner2.getId()).targetName("does_not_exist")
|
||||
.build();
|
||||
Container<IteratorContainerType> container = containersController.create(
|
||||
IteratorContainerConfiguration.builder()
|
||||
.name("Review")
|
||||
.subFlow(FlowData.builder().block(inner1).block(inner2).connection(danglingInner).build())
|
||||
.build());
|
||||
|
||||
FlowCreateRequest request = new FlowCreateRequest(
|
||||
"Container with bad subflow",
|
||||
"The subflow has a dangling inner connection",
|
||||
FlowData.builder().container(container).build());
|
||||
|
||||
FlowView created = flowController.createFlow(request, new LoginEntity("testuser", "testpassword")).getBody();
|
||||
assertNotNull(created);
|
||||
assertEquals(FlowViewStatus.DRAFT, created.status());
|
||||
|
||||
List<ValidationError> validation = flowController
|
||||
.getFlowValidation(created.id(), new LoginEntity("testuser", "testpassword")).getBody();
|
||||
assertNotNull(validation);
|
||||
ValidationError subFlowError = validation.stream()
|
||||
.filter(e -> e.code() == ValidationErrorCode.CONNECTION_TARGET_INPUT_NOT_FOUND)
|
||||
.findFirst()
|
||||
.orElseThrow(() -> new AssertionError("expected an exploded inner connection error: " + validation));
|
||||
// (a) attributed to the specific container and its body subflow...
|
||||
assertEquals("container", subFlowError.entity());
|
||||
assertEquals(container.getId(), subFlowError.id());
|
||||
assertEquals("specificConfiguration.subFlow", subFlowError.field());
|
||||
// (b) ...while still pinpointing the offending inner connection.
|
||||
assertTrue(subFlowError.relatedNodeIds().contains(danglingInner.getId()),
|
||||
() -> "expected inner connection id " + danglingInner.getId() + " in " + subFlowError.relatedNodeIds());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void createEmptyFlowReturnsDraftStatus() {
|
||||
FlowCreateRequest request = new FlowCreateRequest(
|
||||
|
|
|
|||
Loading…
Reference in New Issue