FN-7212: apply reviewer model overrides
Route reviewer-family sessions through the shared validator model resolver so task overrides are honored consistently. - Resolve reviewer, workflow review-step, and spec-review models from one effective settings snapshot. - Forward task reviewer model overrides from triage into spec review calls. - Preserve review text capture for session implementations without subscribe support. - Add coverage for override precedence, test-mode forcing, and optional review lanes. - Add a patch changeset for the published Fusion package. Files changed: .changeset/fn-7212-reviewer-model-overrides.md | 7 ++ .../core/src/__tests__/model-resolution.test.ts | 117 +++++++++++++++++++ .../src/__tests__/executor-review-verdicts.test.ts | 53 ++++++++- packages/engine/src/__tests__/reviewer.test.ts | 130 +++++++++++++++++++-- packages/engine/src/__tests__/triage.test.ts | 14 ++- packages/engine/src/reviewer.ts | 91 +++++++++------ packages/engine/src/triage.ts | 3 + 7 files changed, 369 insertions(+), 46 deletions(-) Fusion-Task-Id: FN-7212 Fusion-Task-Lineage: 34820552-e99c-4da0-97ef-fb1812cb917e Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7212-reviewer-model-overrides.md
Normal file
7
.changeset/fn-7212-reviewer-model-overrides.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Apply task reviewer model overrides consistently to reviewer and code-review lanes.
|
||||
category: fix
|
||||
dev: Reviewer sessions now resolve primary models through the validator-lane resolver, including test-mode forcing.
|
||||
@@ -269,6 +269,123 @@ describe("model-resolution", () => {
|
||||
).toEqual({ provider: "google", modelId: "gemini-2.5-pro" });
|
||||
});
|
||||
|
||||
it("resolves task validator models through the full reviewer hierarchy", () => {
|
||||
const task = {
|
||||
validatorModelProvider: "task-reviewer-provider",
|
||||
validatorModelId: "task-reviewer-model",
|
||||
};
|
||||
const settings = {
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorModelId: "project-reviewer-model",
|
||||
validatorGlobalProvider: "global-reviewer-provider",
|
||||
validatorGlobalModelId: "global-reviewer-model",
|
||||
defaultProviderOverride: "project-default-provider",
|
||||
defaultModelIdOverride: "project-default-model",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
};
|
||||
|
||||
expect(resolveTaskValidatorModel(task, settings)).toEqual({
|
||||
provider: "task-reviewer-provider",
|
||||
modelId: "task-reviewer-model",
|
||||
});
|
||||
expect(resolveTaskValidatorModel({}, settings)).toEqual({
|
||||
provider: "project-reviewer-provider",
|
||||
modelId: "project-reviewer-model",
|
||||
});
|
||||
expect(resolveTaskValidatorModel({}, {
|
||||
...settings,
|
||||
validatorProvider: undefined,
|
||||
validatorModelId: undefined,
|
||||
})).toEqual({
|
||||
provider: "global-reviewer-provider",
|
||||
modelId: "global-reviewer-model",
|
||||
});
|
||||
expect(resolveTaskValidatorModel({}, {
|
||||
...settings,
|
||||
validatorProvider: undefined,
|
||||
validatorModelId: undefined,
|
||||
validatorGlobalProvider: undefined,
|
||||
validatorGlobalModelId: undefined,
|
||||
})).toEqual({
|
||||
provider: "project-default-provider",
|
||||
modelId: "project-default-model",
|
||||
});
|
||||
expect(resolveTaskValidatorModel({}, {
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
})).toEqual({
|
||||
provider: "global-default-provider",
|
||||
modelId: "global-default-model",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not mix partial reviewer pairs across task, lane, and default tiers", () => {
|
||||
expect(resolveTaskValidatorModel(
|
||||
{ validatorModelProvider: "task-provider-only" },
|
||||
{
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorModelId: "project-reviewer-model",
|
||||
},
|
||||
)).toEqual({ provider: "project-reviewer-provider", modelId: "project-reviewer-model" });
|
||||
|
||||
expect(resolveTaskValidatorModel(
|
||||
{ validatorModelId: "task-model-only" },
|
||||
{
|
||||
validatorProvider: "project-provider-only",
|
||||
validatorGlobalProvider: "global-reviewer-provider",
|
||||
validatorGlobalModelId: "global-reviewer-model",
|
||||
},
|
||||
)).toEqual({ provider: "global-reviewer-provider", modelId: "global-reviewer-model" });
|
||||
|
||||
expect(resolveTaskValidatorModel(
|
||||
{},
|
||||
{
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorGlobalModelId: "global-model-only",
|
||||
defaultProviderOverride: "project-default-provider",
|
||||
defaultModelIdOverride: "project-default-model",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
},
|
||||
)).toEqual({ provider: "project-default-provider", modelId: "project-default-model" });
|
||||
|
||||
expect(resolveTaskValidatorModel(
|
||||
{},
|
||||
{
|
||||
defaultProviderOverride: "project-default-provider",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
},
|
||||
)).toEqual({ provider: "global-default-provider", modelId: "global-default-model" });
|
||||
});
|
||||
|
||||
it("forces task reviewer overrides to mock/scripted in test mode and mock default mode", () => {
|
||||
const task = {
|
||||
validatorModelProvider: "task-reviewer-provider",
|
||||
validatorModelId: "task-reviewer-model",
|
||||
};
|
||||
const populatedSettings = {
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorModelId: "project-reviewer-model",
|
||||
validatorGlobalProvider: "global-reviewer-provider",
|
||||
validatorGlobalModelId: "global-reviewer-model",
|
||||
defaultProviderOverride: "project-default-provider",
|
||||
defaultModelIdOverride: "project-default-model",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
};
|
||||
|
||||
expect(resolveTaskValidatorModel(task, {
|
||||
...populatedSettings,
|
||||
testMode: true,
|
||||
})).toEqual(TEST_MODE_RESOLVED);
|
||||
expect(resolveTaskValidatorModel(task, {
|
||||
...populatedSettings,
|
||||
defaultProvider: "mock",
|
||||
})).toEqual(TEST_MODE_RESOLVED);
|
||||
});
|
||||
|
||||
it("forces every lane to mock when testMode is true", () => {
|
||||
const settings = {
|
||||
testMode: true,
|
||||
|
||||
@@ -252,12 +252,18 @@ describe("TaskExecutor enginePaused soft pause (no agent termination)", () => {
|
||||
* Helper: executes a task and captures the custom tools passed to createFnAgent.
|
||||
* Returns a map of tool name → tool execute function for direct testing.
|
||||
*/
|
||||
async function captureTools(settingsOverride?: Record<string, unknown>): Promise<Record<string, (id: string, params: any) => Promise<any>>> {
|
||||
const { tools } = await captureToolsWithStore(settingsOverride);
|
||||
async function captureTools(
|
||||
settingsOverride?: Record<string, unknown>,
|
||||
taskOverride?: Record<string, unknown>,
|
||||
): Promise<Record<string, (id: string, params: any) => Promise<any>>> {
|
||||
const { tools } = await captureToolsWithStore(settingsOverride, taskOverride);
|
||||
return tools;
|
||||
}
|
||||
|
||||
async function captureToolsWithStore(settingsOverride?: Record<string, unknown>): Promise<{
|
||||
async function captureToolsWithStore(
|
||||
settingsOverride?: Record<string, unknown>,
|
||||
taskOverride?: Record<string, unknown>,
|
||||
): Promise<{
|
||||
tools: Record<string, (id: string, params: any) => Promise<any>>;
|
||||
store: ReturnType<typeof createMockStore>;
|
||||
}> {
|
||||
@@ -285,6 +291,7 @@ async function captureToolsWithStore(settingsOverride?: Record<string, unknown>)
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...taskOverride,
|
||||
}));
|
||||
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
|
||||
const current = stepStates[stepIndex];
|
||||
@@ -339,6 +346,46 @@ describe("Code review verdict tracking", () => {
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
});
|
||||
|
||||
it("passes task reviewer overrides to fn_review_step instead of executor overrides", async () => {
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "Looks good",
|
||||
summary: "Approved",
|
||||
});
|
||||
|
||||
const tools = await captureTools(
|
||||
{
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorModelId: "project-reviewer-model",
|
||||
},
|
||||
{
|
||||
modelProvider: "task-executor-provider",
|
||||
modelId: "task-executor-model",
|
||||
validatorModelProvider: "task-reviewer-provider",
|
||||
validatorModelId: "task-reviewer-model",
|
||||
},
|
||||
);
|
||||
|
||||
await tools.fn_review_step("call-reviewer-model", {
|
||||
step: 1,
|
||||
type: "code",
|
||||
step_name: "Implement",
|
||||
baseline: "abc123",
|
||||
});
|
||||
|
||||
const reviewOptions = mockedReviewStep.mock.calls[0]?.[7];
|
||||
expect(reviewOptions).toMatchObject({
|
||||
taskValidatorProvider: "task-reviewer-provider",
|
||||
taskValidatorModelId: "task-reviewer-model",
|
||||
projectValidatorProvider: "project-reviewer-provider",
|
||||
projectValidatorModelId: "project-reviewer-model",
|
||||
});
|
||||
expect(reviewOptions.taskValidatorProvider).not.toBe("task-executor-provider");
|
||||
expect(reviewOptions.taskValidatorModelId).not.toBe("task-executor-model");
|
||||
});
|
||||
|
||||
it("code review REVISE sets tracking state", async () => {
|
||||
mockedReviewStep.mockResolvedValue({
|
||||
verdict: "REVISE",
|
||||
|
||||
@@ -4,10 +4,14 @@ vi.mock("../pi.js", () => ({
|
||||
createFnAgent: vi.fn(),
|
||||
describeModel: vi.fn().mockReturnValue("mock-provider/mock-model"),
|
||||
promptWithFallback: vi.fn(async (session, prompt, options) => {
|
||||
if (options === undefined) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
if (typeof session.prompt === "function") {
|
||||
if (options === undefined) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
}
|
||||
} else if (typeof session.promptWithFallback === "function") {
|
||||
await session.promptWithFallback(prompt, options);
|
||||
}
|
||||
}),
|
||||
}));
|
||||
@@ -41,10 +45,14 @@ function createMockSession(reviewText: string) {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedPromptWithFallback.mockImplementation(async (session, prompt, options) => {
|
||||
if (options == null) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
if (typeof session.prompt === "function") {
|
||||
if (options == null) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
}
|
||||
} else if (typeof session.promptWithFallback === "function") {
|
||||
await session.promptWithFallback(prompt, options);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -91,6 +99,112 @@ describe("reviewStep — model settings threading", () => {
|
||||
expect(opts.defaultModelId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses task reviewer overrides before conflicting validator and default settings", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nReviewer override honored."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "code", "# prompt",
|
||||
"abc123",
|
||||
{
|
||||
taskValidatorProvider: "task-reviewer-provider",
|
||||
taskValidatorModelId: "task-reviewer-model",
|
||||
projectValidatorProvider: "project-reviewer-provider",
|
||||
projectValidatorModelId: "project-reviewer-model",
|
||||
globalValidatorProvider: "global-reviewer-provider",
|
||||
globalValidatorModelId: "global-reviewer-model",
|
||||
projectDefaultOverrideProvider: "project-default-provider",
|
||||
projectDefaultOverrideModelId: "project-default-model",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
},
|
||||
);
|
||||
|
||||
const opts = mockedCreateFnAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBe("task-reviewer-provider");
|
||||
expect(opts.defaultModelId).toBe("task-reviewer-model");
|
||||
});
|
||||
|
||||
it("falls through reviewer settings without mixing partial pairs", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nReviewer fallback honored."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{
|
||||
taskValidatorProvider: "task-provider-only",
|
||||
projectValidatorProvider: "project-provider-only",
|
||||
globalValidatorModelId: "global-model-only",
|
||||
projectDefaultOverrideProvider: "project-default-provider",
|
||||
projectDefaultOverrideModelId: "project-default-model",
|
||||
defaultProvider: "global-default-provider",
|
||||
defaultModelId: "global-default-model",
|
||||
},
|
||||
);
|
||||
|
||||
const opts = mockedCreateFnAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBe("project-default-provider");
|
||||
expect(opts.defaultModelId).toBe("project-default-model");
|
||||
});
|
||||
|
||||
it("forces reviewer sessions to mock/scripted in test mode", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nTest mode honored."),
|
||||
);
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{
|
||||
taskValidatorProvider: "task-reviewer-provider",
|
||||
taskValidatorModelId: "task-reviewer-model",
|
||||
projectValidatorProvider: "project-reviewer-provider",
|
||||
projectValidatorModelId: "project-reviewer-model",
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
settings: { testMode: true } as any,
|
||||
},
|
||||
);
|
||||
|
||||
expect(mockedCreateFnAgent).not.toHaveBeenCalled();
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
});
|
||||
|
||||
it("uses live store settings for reviewer test-mode forcing when settings snapshot is omitted", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nShould not spawn pi."),
|
||||
);
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
testMode: true,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
validatorProvider: "project-reviewer-provider",
|
||||
validatorModelId: "project-reviewer-model",
|
||||
}),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
appendAgentLog: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "code", "# prompt",
|
||||
"abc123",
|
||||
{
|
||||
store: store as any,
|
||||
taskId: "FN-100",
|
||||
taskValidatorProvider: "task-reviewer-provider",
|
||||
taskValidatorModelId: "task-reviewer-model",
|
||||
},
|
||||
);
|
||||
|
||||
expect(store.getSettings).toHaveBeenCalled();
|
||||
expect(mockedCreateFnAgent).not.toHaveBeenCalled();
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
});
|
||||
|
||||
it("extracts APPROVE verdict correctly", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
|
||||
@@ -1022,7 +1022,15 @@ describe("fast-mode triage", () => {
|
||||
mockReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" });
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...mockTaskDetail, id: taskId, comments: [] }),
|
||||
getTask: vi.fn().mockResolvedValue({
|
||||
...mockTaskDetail,
|
||||
id: taskId,
|
||||
comments: [],
|
||||
modelProvider: "task-executor-provider",
|
||||
modelId: "task-executor-model",
|
||||
validatorModelProvider: "task-reviewer-provider",
|
||||
validatorModelId: "task-reviewer-model",
|
||||
}),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
@@ -1055,12 +1063,16 @@ describe("fast-mode triage", () => {
|
||||
|
||||
const reviewOptions = mockReviewStep.mock.calls[0]?.[7];
|
||||
expect(reviewOptions).toMatchObject({
|
||||
taskValidatorProvider: "task-reviewer-provider",
|
||||
taskValidatorModelId: "task-reviewer-model",
|
||||
projectDefaultOverrideProvider: "openai-codex",
|
||||
projectDefaultOverrideModelId: "gpt-5.5",
|
||||
fallbackProvider: "openai-codex",
|
||||
fallbackModelId: "gpt-5.5",
|
||||
agentPrompts: { roleAssignments: { reviewer: "custom-reviewer" } },
|
||||
});
|
||||
expect(reviewOptions.taskValidatorProvider).not.toBe("task-executor-provider");
|
||||
expect(reviewOptions.taskValidatorModelId).not.toBe("task-executor-model");
|
||||
expect(reviewOptions.settings).toMatchObject({
|
||||
memoryEnabled: false,
|
||||
agentPrompts: { roleAssignments: { reviewer: "custom-reviewer" } },
|
||||
|
||||
@@ -21,7 +21,7 @@ import { recordRetry } from "./retry-burned-logger.js";
|
||||
import { mergeEffectiveSettings } from "./effective-settings.js";
|
||||
import { describeModel, promptWithFallback } from "./pi.js";
|
||||
import { isContextLimitError } from "./context-limit-detector.js";
|
||||
import { createResolvedAgentSession, extractRuntimeHint } from "./agent-session-helpers.js";
|
||||
import { createResolvedAgentSession, extractRuntimeHint, resolveValidatorSessionModel } from "./agent-session-helpers.js";
|
||||
import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
import { reviewerLog } from "./logger.js";
|
||||
@@ -168,6 +168,7 @@ export async function reviewStep(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
|
||||
);
|
||||
|
||||
const effectiveSettings = liveSettings ?? options.settings;
|
||||
const agentLogger = options.store && options.taskId
|
||||
? new AgentLogger({
|
||||
store: options.store,
|
||||
@@ -176,30 +177,37 @@ export async function reviewStep(
|
||||
onAgentText: options.onText
|
||||
? (_id, delta) => options.onText!(delta)
|
||||
: undefined,
|
||||
persistAgentToolOutput: liveSettings?.persistAgentToolOutput,
|
||||
persistAgentToolOutput: effectiveSettings?.persistAgentToolOutput,
|
||||
// Reviewer sessions are task-scoped ephemeral workers.
|
||||
persistAgentThinkingLog: resolvePersistAgentThinkingLog(liveSettings, { ephemeral: true }),
|
||||
persistAgentThinkingLog: resolvePersistAgentThinkingLog(effectiveSettings, { ephemeral: true }),
|
||||
})
|
||||
: null;
|
||||
|
||||
const validatorProvider = options.taskValidatorProvider && options.taskValidatorModelId
|
||||
? options.taskValidatorProvider
|
||||
: (options.projectValidatorProvider && options.projectValidatorModelId
|
||||
? options.projectValidatorProvider
|
||||
: (options.globalValidatorProvider && options.globalValidatorModelId
|
||||
? options.globalValidatorProvider
|
||||
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
|
||||
? options.projectDefaultOverrideProvider
|
||||
: options.defaultProvider)));
|
||||
const validatorModelId = options.taskValidatorProvider && options.taskValidatorModelId
|
||||
? options.taskValidatorModelId
|
||||
: (options.projectValidatorProvider && options.projectValidatorModelId
|
||||
? options.projectValidatorModelId
|
||||
: (options.globalValidatorProvider && options.globalValidatorModelId
|
||||
? options.globalValidatorModelId
|
||||
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
|
||||
? options.projectDefaultOverrideModelId
|
||||
: options.defaultModelId)));
|
||||
/*
|
||||
FNXC:ModelResolution 2026-06-28-17:00:
|
||||
Reviewer, spec-review, and workflow review-step sessions are validator-lane sessions. Resolve their primary model through the shared session helper so task reviewer overrides, project/global validator lanes, project/global defaults, and test-mode mock forcing stay identical to core model resolution instead of drifting in a reviewer-local precedence chain.
|
||||
|
||||
FNXC:ModelResolution 2026-06-28-17:48:
|
||||
Re-read store settings are the authoritative reviewer snapshot because optional review steps may omit `options.settings` or hold stale settings while test mode/default-provider mock has changed. Explicit per-call overrides still win, but every unspecified tier must come from the same live settings object passed into session creation.
|
||||
*/
|
||||
const reviewerModelSettings: Partial<Settings> = {
|
||||
...(effectiveSettings ?? {}),
|
||||
defaultProvider: options.defaultProvider ?? effectiveSettings?.defaultProvider,
|
||||
defaultModelId: options.defaultModelId ?? effectiveSettings?.defaultModelId,
|
||||
validatorProvider: options.projectValidatorProvider ?? effectiveSettings?.validatorProvider,
|
||||
validatorModelId: options.projectValidatorModelId ?? effectiveSettings?.validatorModelId,
|
||||
validatorGlobalProvider: options.globalValidatorProvider ?? effectiveSettings?.validatorGlobalProvider,
|
||||
validatorGlobalModelId: options.globalValidatorModelId ?? effectiveSettings?.validatorGlobalModelId,
|
||||
defaultProviderOverride: options.projectDefaultOverrideProvider ?? effectiveSettings?.defaultProviderOverride,
|
||||
defaultModelIdOverride: options.projectDefaultOverrideModelId ?? effectiveSettings?.defaultModelIdOverride,
|
||||
};
|
||||
const reviewerModel = resolveValidatorSessionModel(
|
||||
options.taskValidatorProvider,
|
||||
options.taskValidatorModelId,
|
||||
reviewerModelSettings,
|
||||
);
|
||||
const validatorProvider = reviewerModel.provider;
|
||||
const validatorModelId = reviewerModel.modelId;
|
||||
|
||||
const validatorFallbackProvider = options.projectValidatorFallbackProvider && options.projectValidatorFallbackModelId
|
||||
? options.projectValidatorFallbackProvider
|
||||
@@ -214,7 +222,7 @@ export async function reviewStep(
|
||||
const agents = await options.agentStore.listAgents({ role: "reviewer" });
|
||||
for (const agent of agents) {
|
||||
if (agent.instructionsText || agent.instructionsPath) {
|
||||
const memoryMode = resolveAgentMemoryInclusionMode({ agent, globalSettings: options.settings }).mode;
|
||||
const memoryMode = resolveAgentMemoryInclusionMode({ agent, globalSettings: effectiveSettings }).mode;
|
||||
reviewerInstructions = await resolveAgentInstructions(agent, options.rootDir, undefined, memoryMode);
|
||||
break;
|
||||
}
|
||||
@@ -232,8 +240,8 @@ export async function reviewStep(
|
||||
// FN-6235: built-in reviewer policy is sourced from the resolved workflow IR review node;
|
||||
// explicit reviewer role overrides still win, and the built-in default keeps this fail-soft.
|
||||
const reviewerBasePrompt = userReviewerPrompt || workflowReviewerPrompt || resolveAgentPrompt("reviewer");
|
||||
const memorySection = options.rootDir && options.settings?.memoryEnabled !== false
|
||||
? buildReviewerMemoryInstructions(options.rootDir, options.settings)
|
||||
const memorySection = options.rootDir && effectiveSettings?.memoryEnabled !== false
|
||||
? buildReviewerMemoryInstructions(options.rootDir, effectiveSettings)
|
||||
: "";
|
||||
|
||||
const reviewerPluginContributions = buildPluginPromptSection(
|
||||
@@ -276,16 +284,16 @@ export async function reviewStep(
|
||||
&& typeof (agentStore as { getAgent?: unknown }).getAgent === "function"
|
||||
? await agentStore.getAgent(assignedAgentId).catch(() => null)
|
||||
: null;
|
||||
const memoryTools = options.rootDir && options.settings?.memoryEnabled !== false
|
||||
const memoryTools = options.rootDir && effectiveSettings?.memoryEnabled !== false
|
||||
? [
|
||||
createMemorySearchTool(options.rootDir, options.settings, memoryAgent ? {
|
||||
createMemorySearchTool(options.rootDir, effectiveSettings, memoryAgent ? {
|
||||
agentMemory: {
|
||||
agentId: memoryAgent.id,
|
||||
agentName: memoryAgent.name,
|
||||
memory: memoryAgent.memory,
|
||||
},
|
||||
} : undefined),
|
||||
createMemoryGetTool(options.rootDir, options.settings, memoryAgent ? {
|
||||
createMemoryGetTool(options.rootDir, effectiveSettings, memoryAgent ? {
|
||||
agentMemory: {
|
||||
agentId: memoryAgent.id,
|
||||
agentName: memoryAgent.name,
|
||||
@@ -331,6 +339,17 @@ export async function reviewStep(
|
||||
const createReviewerSession = async (
|
||||
overrides?: { forceProvider?: string; forceModelId?: string },
|
||||
): Promise<import("@earendil-works/pi-coding-agent").AgentSession> => {
|
||||
let streamReviewTextFromOnText = false;
|
||||
const handleReviewerText = (delta: string) => {
|
||||
if (streamReviewTextFromOnText) {
|
||||
reviewText += delta;
|
||||
}
|
||||
if (agentLogger) {
|
||||
agentLogger.onText(delta);
|
||||
} else {
|
||||
options.onText?.(delta);
|
||||
}
|
||||
};
|
||||
const runAuditor = options.store
|
||||
? createRunAuditor(options.store, {
|
||||
runId: generateSyntheticRunId("reviewer", options.taskId ?? "review"),
|
||||
@@ -349,7 +368,7 @@ export async function reviewStep(
|
||||
systemPromptLayers: layers,
|
||||
tools: "readonly",
|
||||
customTools: [createWebFetchTool(), ...(memoryTools ?? [])],
|
||||
onText: agentLogger ? agentLogger.onText : (delta) => options.onText?.(delta),
|
||||
onText: handleReviewerText,
|
||||
onThinking: agentLogger?.onThinking,
|
||||
onToolStart: agentLogger?.onToolStart,
|
||||
onToolEnd: agentLogger?.onToolEnd,
|
||||
@@ -359,7 +378,7 @@ export async function reviewStep(
|
||||
fallbackModelId: validatorFallbackModelId,
|
||||
defaultThinkingLevel: options.defaultThinkingLevel,
|
||||
runAuditor,
|
||||
settings: options.settings,
|
||||
settings: effectiveSettings,
|
||||
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||
taskId: options.taskId,
|
||||
taskTitle: options.taskTitle,
|
||||
@@ -397,11 +416,15 @@ export async function reviewStep(
|
||||
|
||||
activeSessions.add(session);
|
||||
options.onSessionCreated?.(session);
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
||||
reviewText += event.assistantMessageEvent.delta;
|
||||
}
|
||||
});
|
||||
if (typeof session.subscribe === "function") {
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
||||
reviewText += event.assistantMessageEvent.delta;
|
||||
}
|
||||
});
|
||||
} else {
|
||||
streamReviewTextFromOnText = true;
|
||||
}
|
||||
|
||||
return session;
|
||||
};
|
||||
|
||||
@@ -2085,6 +2085,9 @@ export class TriageProcessor {
|
||||
// Execution defaults as final fallback
|
||||
defaultProvider: currentSettings.defaultProvider,
|
||||
defaultModelId: currentSettings.defaultModelId,
|
||||
// FNXC:ModelResolution 2026-06-28-17:10: Spec review is a reviewer/validator lane, so triage must forward the task reviewer override to reviewStep instead of letting project/global settings mask a per-task validator model.
|
||||
taskValidatorProvider: currentDetail.validatorModelProvider,
|
||||
taskValidatorModelId: currentDetail.validatorModelId,
|
||||
// Project-level validator override
|
||||
projectValidatorProvider: currentSettings.validatorProvider,
|
||||
projectValidatorModelId: currentSettings.validatorModelId,
|
||||
|
||||
Reference in New Issue
Block a user