feat(HAI-102): thread model settings through reviewer and merger
- Add defaultProvider and defaultModelId to ReviewOptions interface - Forward model settings from store to createHaiAgent in reviewer - Forward model settings from store to createHaiAgent in merger - Pass settings from executor to reviewStep call - Add tests for model settings threading in both reviewer and merger
This commit is contained in:
@@ -533,10 +533,15 @@ export class TaskExecutor {
|
||||
await store.logEntry(taskId, `${reviewType} review requested for Step ${step} (${step_name})`);
|
||||
|
||||
try {
|
||||
const settings = await store.getSettings();
|
||||
const result = await reviewStep(
|
||||
worktreePath, taskId, step, step_name,
|
||||
reviewType, promptContent, baseline,
|
||||
{ onText: (delta) => options.onAgentText?.(taskId, delta) },
|
||||
{
|
||||
onText: (delta) => options.onAgentText?.(taskId, delta),
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
},
|
||||
);
|
||||
|
||||
await store.logEntry(
|
||||
|
||||
@@ -292,6 +292,52 @@ describe("aiMergeTask — includeTaskIdInCommit setting", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("aiMergeTask — model settings threading", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
setupHappyPathExecSync();
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
});
|
||||
|
||||
it("passes defaultProvider and defaultModelId from settings to createHaiAgent", async () => {
|
||||
const store = createMockStore(
|
||||
{ id: "HAI-050", worktree: "/tmp/root/.worktrees/HAI-050" },
|
||||
[{ id: "HAI-050", worktree: "/tmp/root/.worktrees/HAI-050", column: "in-review" } as Task],
|
||||
);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
defaultProvider: "openai",
|
||||
defaultModelId: "gpt-4o",
|
||||
});
|
||||
|
||||
await aiMergeTask(store, "/tmp/root", "HAI-050");
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateHaiAgent.mock.calls[0][0] as any;
|
||||
expect(opts.defaultProvider).toBe("openai");
|
||||
expect(opts.defaultModelId).toBe("gpt-4o");
|
||||
});
|
||||
|
||||
it("does not set model fields when settings omit them", async () => {
|
||||
const store = createMockStore(
|
||||
{ id: "HAI-050", worktree: "/tmp/root/.worktrees/HAI-050" },
|
||||
[{ id: "HAI-050", worktree: "/tmp/root/.worktrees/HAI-050", column: "in-review" } as Task],
|
||||
);
|
||||
|
||||
await aiMergeTask(store, "/tmp/root", "HAI-050");
|
||||
|
||||
const opts = mockedCreateHaiAgent.mock.calls[0][0] as any;
|
||||
expect(opts.defaultProvider).toBeUndefined();
|
||||
expect(opts.defaultModelId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("aiMergeTask — agent log persistence", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -231,12 +231,15 @@ export async function aiMergeTask(
|
||||
: undefined,
|
||||
});
|
||||
|
||||
// Forward model settings from store so the merger honours the user's model choice
|
||||
const { session } = await createHaiAgent({
|
||||
cwd: rootDir,
|
||||
systemPrompt: buildMergeSystemPrompt(includeTaskId),
|
||||
tools: "coding",
|
||||
onText: agentLogger.onText,
|
||||
onToolStart: agentLogger.onToolStart,
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
});
|
||||
|
||||
try {
|
||||
|
||||
81
packages/engine/src/reviewer.test.ts
Normal file
81
packages/engine/src/reviewer.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("./pi.js", () => ({
|
||||
createHaiAgent: vi.fn(),
|
||||
}));
|
||||
|
||||
import { reviewStep } from "./reviewer.js";
|
||||
import { createHaiAgent } from "./pi.js";
|
||||
|
||||
const mockedCreateHaiAgent = vi.mocked(createHaiAgent);
|
||||
|
||||
function createMockSession(reviewText: string) {
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
subscribe: vi.fn().mockImplementation((cb: any) => {
|
||||
// Simulate the reviewer producing text
|
||||
cb({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: reviewText },
|
||||
});
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any;
|
||||
}
|
||||
|
||||
describe("reviewStep — model settings threading", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("passes defaultProvider and defaultModelId to createHaiAgent when provided", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "HAI-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
},
|
||||
);
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateHaiAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBe("anthropic");
|
||||
expect(opts.defaultModelId).toBe("claude-sonnet-4-5");
|
||||
});
|
||||
|
||||
it("does not set model fields when ReviewOptions omits them", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nAll good."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "HAI-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{},
|
||||
);
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateHaiAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBeUndefined();
|
||||
expect(opts.defaultModelId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("extracts APPROVE verdict correctly", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
);
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree", "HAI-100", 1, "Test Step", "plan", "# prompt",
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
});
|
||||
});
|
||||
@@ -111,6 +111,10 @@ export interface ReviewResult {
|
||||
|
||||
export interface ReviewOptions {
|
||||
onText?: (delta: string) => void;
|
||||
/** Default model provider (e.g. "anthropic"). When set with `defaultModelId`, overrides the reviewer's model selection. */
|
||||
defaultProvider?: string;
|
||||
/** Default model ID within the provider (e.g. "claude-sonnet-4-5"). When set with `defaultProvider`, overrides the reviewer's model selection. */
|
||||
defaultModelId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -137,6 +141,8 @@ export async function reviewStep(
|
||||
systemPrompt: REVIEWER_SYSTEM_PROMPT,
|
||||
tools: "readonly",
|
||||
onText: (delta) => options.onText?.(delta),
|
||||
defaultProvider: options.defaultProvider,
|
||||
defaultModelId: options.defaultModelId,
|
||||
});
|
||||
|
||||
let reviewText = "";
|
||||
|
||||
Reference in New Issue
Block a user