root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(FN-4092): complete Step 2 — add reviewer fallback retry

Browse Source 
Fusion-Task-Id: FN-4092
Fusion-Task-Lineage: fcee3a44-5c36-4899-bf9d-835d36abdf2e
This commit is contained in:
Fusion
2026-05-13 02:33:26 -07:00
committed by gsxdsm
parent fcf2c39a9e
commit 53f4de2fcb
3 changed files with 223 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
packages/engine/src/__tests__/plan-review-unavailable-recovery.test.ts
View File

@@ -38,8 +38,10 @@ describe("FN-4068 baseline — plan review UNAVAILABLE", () => {
vi.clearAllMocks();
});
it("returns UNAVAILABLE without recovery log when verdict is not parseable", async () => {
mockedCreateResolvedAgentSession.mockResolvedValue(buildSession("Reviewer output without verdict heading."));
it("retries once then returns terminal UNAVAILABLE when verdict is not parseable", async () => {
mockedCreateResolvedAgentSession
.mockResolvedValueOnce(buildSession("Reviewer output without verdict heading."))
.mockResolvedValueOnce(buildSession("Still no verdict heading."));
const store = {
getSettings: vi.fn().mockResolvedValue({}),
@@ -59,11 +61,11 @@ describe("FN-4068 baseline — plan review UNAVAILABLE", () => {
);
expect(result.verdict).toBe("UNAVAILABLE");
expect(result.review).toContain("without verdict");
expect(mockedCreateResolvedAgentSession).toHaveBeenCalledTimes(1);
expect(store.logEntry).not.toHaveBeenCalledWith(
expect(result.review).toContain("Still no verdict heading");
expect(mockedCreateResolvedAgentSession).toHaveBeenCalledTimes(2);
expect(store.logEntry).toHaveBeenCalledWith(
"FN-4092",
expect.stringContaining("retry with fallback model"),
expect.stringContaining("review retry with fallback model after UNAVAILABLE verdict"),
);
});
});

101
packages/engine/src/__tests__/reviewer.test.ts
View File

@@ -387,7 +387,100 @@ describe("reviewStep — context-limit retry", () => {
};
await expect(runReview()).resolves.toEqual({ verdict: "UNAVAILABLE" });
expect(mockedPromptWithFallback).toHaveBeenCalledTimes(2);
expect(mockedPromptWithFallback).toHaveBeenCalledTimes(4);
});
});
describe("reviewStep — fallback retry for terminal unavailable", () => {
beforeEach(() => {
vi.clearAllMocks();
});
it("retries once on fallback model when first verdict is UNAVAILABLE", async () => {
mockedCreateFnAgent
.mockResolvedValueOnce(createMockSession("No parseable verdict here."))
.mockResolvedValueOnce(createMockSession("### Verdict: APPROVE\n### Summary\nRecovered on fallback."));
const store = {
getSettings: vi.fn().mockResolvedValue({}),
logEntry: vi.fn().mockResolvedValue(undefined),
appendAgentLog: vi.fn().mockResolvedValue(undefined),
};
const result = await reviewStep(
"/tmp/worktree", "FN-4092", 2, "Retry", "plan", "# prompt", undefined,
{
store: store as any,
taskId: "FN-4092",
projectValidatorFallbackProvider: "openai",
projectValidatorFallbackModelId: "gpt-5-mini",
},
);
expect(result.verdict).toBe("APPROVE");
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
expect(store.logEntry).toHaveBeenCalledWith(
"FN-4092",
expect.stringContaining("review retry with fallback model after UNAVAILABLE verdict"),
);
});
it("retries once after non-context reviewer error", async () => {
mockedCreateFnAgent
.mockResolvedValueOnce(createMockSession("### Verdict: APPROVE\n### Summary\nunused"))
.mockResolvedValueOnce(createMockSession("### Verdict: REVISE\n### Summary\nRetry recovered."));
mockedPromptWithFallback
.mockRejectedValueOnce(new Error("transient reviewer failure"))
.mockImplementation(async (session, prompt, options) => {
if (options == null) await session.prompt(prompt);
else await session.prompt(prompt, options);
});
const result = await reviewStep(
"/tmp/worktree", "FN-4092", 2, "Retry", "code", "# prompt", "abc123",
{
projectValidatorFallbackProvider: "openai",
projectValidatorFallbackModelId: "gpt-5-mini",
},
);
expect(result.verdict).toBe("REVISE");
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
});
it("returns terminal UNAVAILABLE when fallback attempt is also UNAVAILABLE", async () => {
mockedCreateFnAgent
.mockResolvedValueOnce(createMockSession("No parseable verdict #1"))
.mockResolvedValueOnce(createMockSession("No parseable verdict #2"));
const result = await reviewStep(
"/tmp/worktree", "FN-4092", 2, "Retry", "spec", "# prompt", undefined,
{
projectValidatorFallbackProvider: "openai",
projectValidatorFallbackModelId: "gpt-5-mini",
},
);
expect(result.verdict).toBe("UNAVAILABLE");
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
});
it("does not retry pause-driven UNAVAILABLE", async () => {
mockedCreateFnAgent.mockResolvedValue(createMockSession("### Verdict: APPROVE\n### Summary\nunused"));
const store = {
getSettings: vi.fn().mockResolvedValue({ globalPause: true }),
logEntry: vi.fn().mockResolvedValue(undefined),
appendAgentLog: vi.fn().mockResolvedValue(undefined),
};
const result = await reviewStep(
"/tmp/worktree", "FN-4092", 2, "Retry", "plan", "# prompt", undefined,
{ store: store as any, taskId: "FN-4092" },
);
expect(result.verdict).toBe("UNAVAILABLE");
expect(mockedCreateFnAgent).not.toHaveBeenCalled();
});
});
@@ -1112,7 +1205,7 @@ describe("reviewStep — subagent lifecycle hooks", () => {
const mockSession = createMockSession("");
mockedCreateFnAgent.mockResolvedValue(mockSession);
const { promptWithFallback } = await import("../pi.js");
vi.mocked(promptWithFallback).mockRejectedValueOnce(new Error("boom"));
vi.mocked(promptWithFallback).mockRejectedValue(new Error("boom"));
const onSessionCreated = vi.fn();
const onSessionEnded = vi.fn();
@@ -1125,7 +1218,7 @@ describe("reviewStep — subagent lifecycle hooks", () => {
),
).rejects.toThrow("boom");
expect(onSessionCreated).toHaveBeenCalledTimes(1);
expect(onSessionEnded).toHaveBeenCalledTimes(1);
expect(onSessionCreated).toHaveBeenCalledTimes(2);
expect(onSessionEnded).toHaveBeenCalledTimes(2);
});
});

177
packages/engine/src/reviewer.ts
View File

@@ -491,7 +491,9 @@ export async function reviewStep(
};
};
const createReviewerSession = async (): Promise<import("@mariozechner/pi-coding-agent").AgentSession> => {
const createReviewerSession = async (
overrides?: { forceProvider?: string; forceModelId?: string },
): Promise<import("@mariozechner/pi-coding-agent").AgentSession> => {
const { session } = await createResolvedAgentSession({
sessionPurpose: "reviewer",
runtimeHint: extractRuntimeHint(memoryAgent?.runtimeConfig),
@@ -505,8 +507,8 @@ export async function reviewStep(
onThinking: agentLogger?.onThinking,
onToolStart: agentLogger?.onToolStart,
onToolEnd: agentLogger?.onToolEnd,
defaultProvider: validatorProvider,
defaultModelId: validatorModelId,
defaultProvider: overrides?.forceProvider ?? validatorProvider,
defaultModelId: overrides?.forceModelId ?? validatorModelId,
fallbackProvider: validatorFallbackProvider,
fallbackModelId: validatorFallbackModelId,
defaultThinkingLevel: options.defaultThinkingLevel,
@@ -562,69 +564,126 @@ export async function reviewStep(
checkSessionError(session);
};
let session: import("@mariozechner/pi-coding-agent").AgentSession;
try {
session = await createReviewerSession();
} catch (err) {
if (err instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(err.reason);
}
throw err;
}
try {
const runAttempt = async (
attemptRequest: string,
sessionOptions?: { forceProvider?: string; forceModelId?: string },
): Promise<{ verdict: ReviewVerdict; summary: string; review: string }> => {
reviewText = "";
let session: import("@mariozechner/pi-coding-agent").AgentSession;
try {
await runReviewPrompt(session, request);
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
if (!isContextLimitError(errorMessage)) {
session = await createReviewerSession(sessionOptions);
} catch (err) {
if (err instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(err.reason);
}
throw err;
}
try {
try {
await runReviewPrompt(session, attemptRequest);
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
if (!isContextLimitError(errorMessage)) {
throw err;
}
const retryLogMessage = reviewType === "code"
? "code review hit context limit — retrying with compacted request"
: `${reviewType} review hit context limit — retrying with compacted request`;
reviewerLog.warn(`${taskId}: ${retryLogMessage}`);
if (options.store && options.taskId) {
await options.store.logEntry(options.taskId, retryLogMessage).catch(() => undefined);
}
reviewText = "";
const reducedRequest = buildReducedReviewRequest(
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
);
try {
await runReviewPrompt(session, reducedRequest);
} catch (retryErr: unknown) {
if (!isReviewerSessionReuseError(retryErr)) {
throw retryErr;
}
endSession(session);
try {
session = await createReviewerSession(sessionOptions);
} catch (recreateErr) {
if (recreateErr instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(recreateErr.reason);
}
throw recreateErr;
}
await runReviewPrompt(session, reducedRequest);
}
}
} finally {
if (agentLogger) {
await agentLogger.flush();
}
for (const activeSession of [...activeSessions]) {
endSession(activeSession);
}
}
const verdict = extractVerdict(reviewText);
const summary = extractSummary(reviewText);
return { verdict, review: reviewText, summary };
};
const fallbackReviewRequest = `${request}\n\nIMPORTANT: Respond with exactly one of: APPROVE | REVISE | RETHINK on a line starting with \"Verdict:\".`;
const logFallbackRetry = async (reason: string, mode: string): Promise<void> => {
const message = `${reviewType} review retry with fallback model after ${reason} (${mode})`;
reviewerLog.warn(`${taskId}: ${message}`);
if (options.store && options.taskId) {
await options.store.logEntry(options.taskId, message).catch(() => undefined);
}
};
const hasConfiguredFallback = Boolean(validatorFallbackProvider && validatorFallbackModelId);
let firstAttempt: { verdict: ReviewVerdict; summary: string; review: string };
try {
firstAttempt = await runAttempt(request);
} catch (err) {
if (hasConfiguredFallback) {
await logFallbackRetry("reviewer error", `${validatorFallbackProvider}/${validatorFallbackModelId}`);
try {
return await runAttempt(request, {
forceProvider: validatorFallbackProvider,
forceModelId: validatorFallbackModelId,
});
} catch {
throw err;
}
const retryLogMessage = reviewType === "code"
? "code review hit context limit — retrying with compacted request"
: `${reviewType} review hit context limit — retrying with compacted request`;
reviewerLog.warn(`${taskId}: ${retryLogMessage}`);
if (options.store && options.taskId) {
await options.store.logEntry(options.taskId, retryLogMessage).catch(() => undefined);
}
reviewText = "";
const reducedRequest = buildReducedReviewRequest(
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
);
try {
await runReviewPrompt(session, reducedRequest);
} catch (retryErr: unknown) {
if (!isReviewerSessionReuseError(retryErr)) {
throw retryErr;
}
endSession(session);
try {
session = await createReviewerSession();
} catch (recreateErr) {
if (recreateErr instanceof ReviewerPauseAbortError) {
return buildPauseUnavailableResult(recreateErr.reason);
}
throw recreateErr;
}
await runReviewPrompt(session, reducedRequest);
}
}
} finally {
if (agentLogger) {
await agentLogger.flush();
}
for (const activeSession of [...activeSessions]) {
endSession(activeSession);
await logFallbackRetry("reviewer error", "same-model strict prompt");
try {
return await runAttempt(fallbackReviewRequest);
} catch {
throw err;
}
}
const verdict = extractVerdict(reviewText);
const summary = extractSummary(reviewText);
return { verdict, review: reviewText, summary };
if (firstAttempt.verdict !== "UNAVAILABLE") {
return firstAttempt;
}
if (hasConfiguredFallback) {
await logFallbackRetry("UNAVAILABLE verdict", `${validatorFallbackProvider}/${validatorFallbackModelId}`);
return runAttempt(request, {
forceProvider: validatorFallbackProvider,
forceModelId: validatorFallbackModelId,
});
}
await logFallbackRetry("UNAVAILABLE verdict", "same-model strict prompt");
return runAttempt(fallbackReviewRequest);
}
function isReviewerSessionReuseError(error: unknown): boolean {