feat(goal): deterministic completion + remaining-criteria followup + auto-followup gates

This commit is contained in:
matevip 2026-06-03 21:19:26 +08:00
parent 0fc8579a3e
commit c5ccce8ebc
8 changed files with 135 additions and 37 deletions

View File

@ -185,7 +185,9 @@ public class GoalEvaluationNode implements NodeAction {
// failure on completion) does not propagate into the chat graph
// and abort the streamed answer the user already sees.
try {
if (result.completed() || result.score() >= 0.95) {
// Completion is the deterministic "all criteria passed" signal the
// evaluator already folded into result.completed() no score gate.
if (result.completed()) {
GoalEntity completed = goalService.markCompleted(refreshed.getId(), result);
return MateClawStateAccessor.output()
.goalEvaluationResult(result.toMap())

View File

@ -7,10 +7,9 @@ import org.springframework.stereotype.Component;
/**
* Configuration knobs for the persistent-goal subsystem.
*
* <p>{@link #enabled} is the master gate: when {@code false} (PR1-4 default)
* the StateGraph wiring stays inactive and {@code findActiveByConversation}
* still works for tests, but no graph node touches the table. PR5 flips it
* to {@code true}.
* <p>{@link #enabled} is the master gate: when {@code false} the StateGraph
* wiring stays inactive (no graph node touches the table), while
* {@code findActiveByConversation} still works for tests.
*/
@Data
@Component
@ -20,12 +19,26 @@ public class GoalProperties {
/**
* Master switch when off, the graph never invokes GoalEvaluationNode
* (the conditional edge sees no active goal, so the node is unreachable).
* Defaults to true now that the full PR1-5 chain is in place; operators
* who want to disable goal evaluation can override via
* Operators who want to disable goal evaluation entirely can override via
* {@code mateclaw.goal.enabled=false} in application.yml.
*/
private boolean enabled = true;
/**
* Create-time default for a goal's {@code autoFollowupEnabled} when the
* caller leaves it unspecified (null). Explicit true/false in the request
* is never overridden by this.
*/
private boolean defaultAutoFollowup = true;
/**
* Runtime hard gate for auto-followup. When false, no goal injects a
* follow-up regardless of its per-goal {@code autoFollowupEnabled} flag
* the operator's kill switch for the self-continuation loop that takes
* effect immediately, even for goals created with the flag on.
*/
private boolean allowAutoFollowup = true;
/** Default turn budget when the user doesn't override. */
private int defaultTurnBudget = 20;

View File

@ -1,38 +1,53 @@
package vip.mate.goal.service;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Service;
import vip.mate.goal.config.GoalProperties;
import vip.mate.goal.model.GoalCriteriaCodec;
import vip.mate.goal.model.GoalCriterion;
import vip.mate.goal.model.GoalEntity;
import vip.mate.goal.model.GoalEvaluationResult;
import java.time.Duration;
import java.time.LocalDateTime;
import java.util.List;
import java.util.Optional;
/**
* Decides whether to inject a follow-up user prompt for the next graph
* pass. PR2 wires the plumbing; the actual "yes, continue" path defaults
* to off until PR5 flips {@code mateclaw.goal.enabled=true} and operators
* opt their goals in via {@code auto_followup_enabled}.
* Decides whether to inject a follow-up user prompt for the next graph pass,
* driving the autonomous "continue until the checklist is complete" loop.
*/
@Slf4j
@Service
public class GoalFollowupService {
private final GoalProperties properties;
private final ObjectMapper objectMapper;
public GoalFollowupService(GoalProperties properties, ObjectMapper objectMapper) {
this.properties = properties;
this.objectMapper = objectMapper;
}
/**
* Build the follow-up prompt to inject, or empty when no follow-up
* should fire this turn. Conditions follow RFC 48 §3.10:
* Build the follow-up prompt to inject, or empty when no follow-up should
* fire this turn. Gating order:
* <ol>
* <li>{@code autoFollowupEnabled} is true.</li>
* <li>{@code allow-auto-followup} runtime hard gate (operator kill
* switch; overrides per-goal flag).</li>
* <li>Per-goal {@code autoFollowupEnabled}.</li>
* <li>Evaluator decision is "continue" with score &lt; 0.95.</li>
* <li>Cooldown since the last follow-up has elapsed.</li>
* <li>turn_budget has at least one slot left after this turn.</li>
* <li>(agent + eval) LLM calls below 90 % of llm_call_budget.</li>
* <li>(agent + eval) LLM calls below 90% of llm_call_budget.</li>
* </ol>
*/
public Optional<String> maybeBuildFollowup(GoalEntity goal,
GoalEvaluationResult result) {
GoalEvaluationResult result) {
if (goal == null || result == null) return Optional.empty();
// Runtime hard gate first overrides any per-goal flag.
if (!properties.isAllowAutoFollowup()) return Optional.empty();
if (!Boolean.TRUE.equals(goal.getAutoFollowupEnabled())) return Optional.empty();
if (!GoalEvaluationResult.DECISION_CONTINUE.equals(result.decision())) {
return Optional.empty();
@ -51,17 +66,39 @@ public class GoalFollowupService {
int turnsUsed = goal.getTurnsUsed() != null ? goal.getTurnsUsed() : 0;
int turnBudget = goal.getTurnBudget() != null ? goal.getTurnBudget() : Integer.MAX_VALUE;
// Leave at least one turn slot for the real user refuse to burn
// the final slot on an auto-followup that the user can't watch.
// Leave at least one turn slot for the real user refuse to burn the
// final slot on an auto-followup the user can't watch.
if (turnsUsed >= turnBudget - 1) return Optional.empty();
int callBudget = goal.getLlmCallBudget() != null ? goal.getLlmCallBudget() : Integer.MAX_VALUE;
if (goal.totalLlmCallsUsed() >= (int) (callBudget * 0.9)) return Optional.empty();
return Optional.of(buildPrompt(goal, result));
}
/**
* Prefer a concrete remaining-criteria list when the goal has a checklist;
* fall back to the free-text gap otherwise. Both end with the same "take
* the next concrete step" instruction.
*/
private String buildPrompt(GoalEntity goal, GoalEvaluationResult result) {
List<GoalCriterion> all = GoalCriteriaCodec.parse(goal.getCriteria(), objectMapper);
List<GoalCriterion> remaining = GoalCriteriaCodec.remaining(all);
if (!remaining.isEmpty()) {
int total = all.size();
int passed = total - remaining.size();
StringBuilder sb = new StringBuilder();
sb.append("Continue working toward the goal. ")
.append(passed).append('/').append(total).append(" criteria passed. Remaining:\n");
for (GoalCriterion c : remaining) {
sb.append(" - ").append(c.text()).append('\n');
}
sb.append("Take the next concrete step on the remaining criteria.");
return sb.toString();
}
String gap = result.gap();
if (gap == null || gap.isBlank()) gap = "the goal is not yet complete.";
String prompt = "Continue working on the goal. Still missing: " + gap
return "Continue working on the goal. Still missing: " + gap
+ "\nTake the next concrete step.";
return Optional.of(prompt);
}
}

View File

@ -115,7 +115,11 @@ public class GoalServiceImpl implements GoalService {
? req.getLlmCallBudget() : properties.getDefaultLlmCallBudget());
entity.setAgentLlmCallsUsed(0);
entity.setEvalLlmCallsUsed(0);
entity.setAutoFollowupEnabled(Boolean.TRUE.equals(req.getAutoFollowupEnabled()));
// Three-state default: explicit true/false is honored; null falls
// back to the configured create-time default.
entity.setAutoFollowupEnabled(req.getAutoFollowupEnabled() != null
? req.getAutoFollowupEnabled()
: properties.isDefaultAutoFollowup());
entity.setFollowupCooldownSeconds(req.getFollowupCooldownSeconds() != null
? req.getFollowupCooldownSeconds() : properties.getAutoFollowupCooldownSeconds());
// Normalize any caller-supplied checklist: assign C1..Cn, force
@ -467,19 +471,32 @@ public class GoalServiceImpl implements GoalService {
throw new MateClawException("err.goal.criterion_empty", 400, "Criterion must not be empty");
}
String trimmed = criterion.trim();
// Merge against the freshly refetched criteria so a concurrent
// addCriterion never silently overwrites a sibling's append.
// Double-write: append a structured criterion (authoritative) and
// mirror the text into exit_criteria for backward compatibility /
// human readability. New id is the current max ordinal + 1. Merge
// against the freshly refetched row so concurrent appends don't clobber.
GoalEntity g = retryOptimistic(id, "appendCriterion", fresh -> {
ensureNotTerminal(fresh, "appendCriterion");
List<GoalCriterion> list = GoalCriteriaCodec.parse(fresh.getCriteria(), objectMapper);
list.add(new GoalCriterion("C" + (list.size() + 1), trimmed, false, ""));
String criteriaJson = GoalCriteriaCodec.serialize(GoalCriteriaCodec.reindex(list), objectMapper);
String existing = fresh.getExitCriteria() != null ? fresh.getExitCriteria() : "";
String merged = existing.isEmpty() ? trimmed : existing + "\n+ " + trimmed;
String mergedText = existing.isEmpty() ? trimmed : existing + "\n+ " + trimmed;
LambdaUpdateWrapper<GoalEntity> w = baseLockedUpdate(fresh)
.set(GoalEntity::getExitCriteria, merged);
.set(GoalEntity::getCriteria, criteriaJson)
.set(GoalEntity::getExitCriteria, mergedText);
bumpVersionAndTime(w);
return w;
});
List<GoalCriterion> full = GoalCriteriaCodec.parse(g.getCriteria(), objectMapper);
String criterionId = full.isEmpty() ? "" : full.get(full.size() - 1).id();
writeEvent(id, GoalEventType.CRITERION_ADDED, null, Map.of(
"criterion", trimmed,
"criterionId", criterionId,
"criteria", full,
"by", username));
return g;
}

View File

@ -58,7 +58,8 @@ public class GoalManagementTool {
required = false) String exitCriteria,
@ToolParam(description = "Max evaluation turns before exhaustion. Default 20.",
required = false) Integer turnBudget,
@ToolParam(description = "If true, the agent may auto-followup when progress is incomplete. Default false.",
@ToolParam(description = "If true, the agent may auto-followup when progress is incomplete. "
+ "Omit to use the system default.",
required = false) Boolean autoFollowup,
@ToolParam(description = "Optional initial checklist: a list of short, individually verifiable "
+ "acceptance criteria. Omit to let the system derive the checklist on first evaluation.",
@ -168,7 +169,8 @@ public class GoalManagementTool {
if (streamTracker != null && completed.getConversationId() != null) {
streamTracker.broadcastObject(completed.getConversationId(), "goal_completed", Map.of(
"goalId", String.valueOf(completed.getId()),
"score", synthetic.score()));
"score", synthetic.score(),
"goal", goalService.toResponse(completed)));
}
return successJson(Map.of(
"goalId", String.valueOf(completed.getId()),

View File

@ -10,6 +10,7 @@ import vip.mate.common.result.R;
import vip.mate.exception.MateClawException;
import vip.mate.goal.model.GoalCreateRequest;
import vip.mate.goal.model.GoalEntity;
import vip.mate.goal.model.GoalResponse;
import vip.mate.goal.model.GoalStatus;
import vip.mate.goal.model.GoalUpdateRequest;
import vip.mate.goal.service.GoalService;
@ -64,6 +65,13 @@ class GoalControllerTest {
return g;
}
private GoalResponse resp(Long id, GoalStatus status) {
GoalResponse r = new GoalResponse();
r.setId(id);
r.setStatus(status);
return r;
}
private GoalCreateRequest req(String convId) {
GoalCreateRequest r = new GoalCreateRequest();
r.setConversationId(convId);
@ -90,7 +98,8 @@ class GoalControllerTest {
when(conversationService.findByConversationId("conv-1")).thenReturn(conv("conv-1", 10L, 1L));
when(goalService.create(any(), eq("alice")))
.thenReturn(goal(1L, "conv-1", GoalStatus.ACTIVE));
R<GoalEntity> result = controller.create(req("conv-1"), auth);
when(goalService.toResponse(any())).thenReturn(resp(1L, GoalStatus.ACTIVE));
R<GoalResponse> result = controller.create(req("conv-1"), auth);
assertNotNull(result.getData());
assertEquals(1L, result.getData().getId());
}
@ -177,8 +186,9 @@ class GoalControllerTest {
when(goalService.getById(1L)).thenReturn(g);
when(conversationService.isConversationOwner("conv-1", "alice")).thenReturn(true);
when(goalService.pause(1L, "alice")).thenReturn(goal(1L, "conv-1", GoalStatus.PAUSED));
when(goalService.toResponse(any())).thenReturn(resp(1L, GoalStatus.PAUSED));
R<GoalEntity> result = controller.pause(1L, auth);
R<GoalResponse> result = controller.pause(1L, auth);
assertEquals(GoalStatus.PAUSED, result.getData().getStatus());
}

View File

@ -1,6 +1,8 @@
package vip.mate.goal.service;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;
import vip.mate.goal.config.GoalProperties;
import vip.mate.goal.model.GoalEntity;
import vip.mate.goal.model.GoalEvaluationResult;
import vip.mate.goal.model.GoalStatus;
@ -8,16 +10,16 @@ import vip.mate.goal.model.GoalStatus;
import java.time.LocalDateTime;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* Covers the five follow-up gating conditions from RFC 48 §3.10. Every
* negative case must independently block the follow-up.
* Covers the follow-up gating conditions. Every negative case must
* independently block the follow-up.
*/
class GoalFollowupServiceTest {
private final GoalFollowupService svc = new GoalFollowupService();
private final GoalProperties properties = new GoalProperties();
private final GoalFollowupService svc = new GoalFollowupService(properties, new ObjectMapper());
private GoalEntity goal(boolean autoEnabled) {
GoalEntity g = new GoalEntity();
@ -50,6 +52,20 @@ class GoalFollowupServiceTest {
assertTrue(out.isEmpty());
}
@Test
void allowAutoFollowupGate_overridesPerGoalFlag() {
properties.setAllowAutoFollowup(false);
try {
// per-goal flag on + budget healthy, yet the runtime hard gate wins.
Optional<String> out = svc.maybeBuildFollowup(
goal(true),
res(0.6, GoalEvaluationResult.DECISION_CONTINUE));
assertTrue(out.isEmpty());
} finally {
properties.setAllowAutoFollowup(true);
}
}
@Test
void completedDecision_returnsEmpty() {
Optional<String> out = svc.maybeBuildFollowup(

View File

@ -75,7 +75,7 @@ class GoalManagementToolTest {
@Test
void setGoal_disabledFlag_returnsError() {
properties.setEnabled(false);
String result = tool.setGoal("title", null, null, null, null,
String result = tool.setGoal("title", null, null, null, null, null,
ctxWith("conv-1", 10L, "alice"));
assertTrue(result.contains("disabled"));
verify(goalService, never()).create(any(), anyString());
@ -83,7 +83,7 @@ class GoalManagementToolTest {
@Test
void setGoal_blankTitle_returnsError() {
String result = tool.setGoal(" ", null, null, null, null,
String result = tool.setGoal(" ", null, null, null, null, null,
ctxWith("conv-1", 10L, "alice"));
assertTrue(result.contains("title is required"));
}
@ -95,7 +95,7 @@ class GoalManagementToolTest {
String result = tool.setGoal("ship the blog",
"deploy to fly.io",
"tests pass + deployed",
15, true,
15, true, null,
ctxWith("conv-1", 10L, "alice"));
assertTrue(result.contains("\"goalId\":\"123\""));
assertTrue(result.contains("\"status\":\"active\""));
@ -103,7 +103,7 @@ class GoalManagementToolTest {
@Test
void setGoal_missingConversationContext_returnsError() {
String result = tool.setGoal("title", null, null, null, null, null);
String result = tool.setGoal("title", null, null, null, null, null, null);
assertTrue(result.contains("requires a bound conversation"));
}
@ -151,6 +151,7 @@ class GoalManagementToolTest {
GoalEntity completed = goal(GoalStatus.COMPLETED);
when(goalService.markCompleted(eq(123L), any(GoalEvaluationResult.class)))
.thenReturn(completed);
when(goalService.toResponse(any())).thenReturn(new vip.mate.goal.model.GoalResponse());
String result = tool.completeGoal(ctxWith("conv-1", 10L, "alice"));
assertTrue(result.contains("\"status\":\"completed\""));
}