mirror of
https://github.com/rowboatlabs/rowboat.git
synced 2026-07-24 21:41:08 +02:00
feat(x): drop limit wrap-up special-casing, raise default limit to 50
Per review: no special handling in the agent loop when the model-call limit is hit — the turn fails as before. Instead the default limit rises from 20 to 50 (global, and chat inherits it) so the limit rarely triggers in practice; users can tune it in Settings > Advanced. The friendly limit-failure message in the chat error bubble stays, since it points users at the setting.
This commit is contained in:
Gagan
parent
180ae10e2b
commit
7cb9c478ac
9 changed files with 20 additions and 139 deletions
|
|
@ -29,6 +29,7 @@ import { AccountSettings } from "@/components/settings/account-settings"
|
|||
import { ConnectedAccountsSettings } from "@/components/settings/connected-accounts-settings"
|
||||
import { MobileChannelsSettings } from "@/components/settings/mobile-channels-settings"
|
||||
import type { ApprovalPolicy } from "@x/shared/src/code-mode.js"
|
||||
import { DEFAULT_TURN_LIMITS_SETTINGS } from "@x/shared/src/turn-limits.js"
|
||||
import type { ipc as ipcShared } from "@x/shared"
|
||||
import { startProvisioning, useProvisioning, enabledOptimistic, type AgentStatus, type CodeModeAgentStatus } from "@/lib/code-mode-provisioning"
|
||||
import { useProviderModels } from "@/hooks/use-provider-models"
|
||||
|
|
@ -2807,7 +2808,7 @@ function AdvancedSettings({ dialogOpen }: { dialogOpen: boolean }) {
|
|||
</div>
|
||||
<LimitStepper
|
||||
value={globalLimit}
|
||||
fallback={20}
|
||||
fallback={DEFAULT_TURN_LIMITS_SETTINGS.maxModelCalls}
|
||||
onInput={setGlobalLimit}
|
||||
onCommit={(next) => {
|
||||
setGlobalLimit(next)
|
||||
|
|
@ -2841,7 +2842,7 @@ function AdvancedSettings({ dialogOpen }: { dialogOpen: boolean }) {
|
|||
)}
|
||||
<LimitStepper
|
||||
value={chatLimit}
|
||||
fallback={parseLimit(globalLimit) ?? 20}
|
||||
fallback={parseLimit(globalLimit) ?? DEFAULT_TURN_LIMITS_SETTINGS.maxModelCalls}
|
||||
placeholder="Same as above"
|
||||
onInput={setChatLimit}
|
||||
onCommit={(next) => {
|
||||
|
|
|
|||
|
|
@ -516,16 +516,9 @@ Rules:
|
|||
turn-limit settings (`config/turn_limits.json`) via the injected
|
||||
`ITurnLimitsResolver`: turns with `humanAvailable: true` use the chat
|
||||
override when set, otherwise the global limit; headless turns always use
|
||||
the global limit. Without a resolver (tests) it defaults to `20`. Spawned
|
||||
the global limit. Without a resolver (tests) it defaults to `50`. Spawned
|
||||
sub-agents default to the global limit, which also caps the budget a
|
||||
parent may grant them. Settings changes affect only newly created turns.
|
||||
- Budget exhaustion (§20): interactive turns (`humanAvailable: true`) spend
|
||||
their final budgeted call wrapping up — the request records `wrapUp: true`,
|
||||
and the composer strips tools and appends the shared `wrapUpNotice`, so
|
||||
the turn completes with a real answer that admits what's unfinished.
|
||||
Headless turns (and a wrap-up call that still emits tool calls) keep the
|
||||
hard `turn_failed` with `model-call-limit` — automation needs the
|
||||
machine-readable outcome.
|
||||
- Persisted values are fully resolved and immutable.
|
||||
|
||||
The capability is named `humanAvailable`, not `headless`. `headless` describes
|
||||
|
|
|
|||
|
|
@ -43,9 +43,9 @@ async function writeSettings(content: string): Promise<void> {
|
|||
}
|
||||
|
||||
describe("loadTurnLimitsSettings", () => {
|
||||
it("defaults to the built-in limit (20) when no file exists", async () => {
|
||||
it("defaults to the built-in limit (50) when no file exists", async () => {
|
||||
const { loadTurnLimitsSettings } = await loadTurnLimits();
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 20 });
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 50 });
|
||||
});
|
||||
|
||||
it("reads persisted settings", async () => {
|
||||
|
|
@ -61,7 +61,7 @@ describe("loadTurnLimitsSettings", () => {
|
|||
await writeSettings(JSON.stringify({ chatMaxModelCalls: 5 }));
|
||||
const { loadTurnLimitsSettings } = await loadTurnLimits();
|
||||
expect(loadTurnLimitsSettings()).toEqual({
|
||||
maxModelCalls: 20,
|
||||
maxModelCalls: 50,
|
||||
chatMaxModelCalls: 5,
|
||||
});
|
||||
});
|
||||
|
|
@ -69,13 +69,13 @@ describe("loadTurnLimitsSettings", () => {
|
|||
it("falls back to defaults on a corrupt file", async () => {
|
||||
await writeSettings("{not json");
|
||||
const { loadTurnLimitsSettings } = await loadTurnLimits();
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 20 });
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 50 });
|
||||
});
|
||||
|
||||
it("falls back to defaults on out-of-range values", async () => {
|
||||
await writeSettings(JSON.stringify({ maxModelCalls: 5000 }));
|
||||
const { loadTurnLimitsSettings } = await loadTurnLimits();
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 20 });
|
||||
expect(loadTurnLimitsSettings()).toEqual({ maxModelCalls: 50 });
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -118,9 +118,9 @@ describe("resolveMaxModelCalls", () => {
|
|||
expect(resolveMaxModelCalls({ humanAvailable: true })).toBe(60);
|
||||
});
|
||||
|
||||
it("resolves 20 everywhere with no settings file", async () => {
|
||||
it("resolves 50 everywhere with no settings file", async () => {
|
||||
const { resolveMaxModelCalls } = await loadTurnLimits();
|
||||
expect(resolveMaxModelCalls({ humanAvailable: true })).toBe(20);
|
||||
expect(resolveMaxModelCalls({ humanAvailable: false })).toBe(20);
|
||||
expect(resolveMaxModelCalls({ humanAvailable: true })).toBe(50);
|
||||
expect(resolveMaxModelCalls({ humanAvailable: false })).toBe(50);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -11,8 +11,8 @@ const TURN_LIMITS_CONFIG_PATH = path.join(WorkDir, 'config', 'turn_limits.json')
|
|||
|
||||
/**
|
||||
* Load the model-call limit settings, falling back to the defaults (global
|
||||
* limit 20, no chat override) when the file is absent or malformed — so
|
||||
* existing installations keep today's behavior until the user changes it.
|
||||
* limit DEFAULT_MAX_MODEL_CALLS, no chat override) when the file is absent
|
||||
* or malformed.
|
||||
*/
|
||||
export function loadTurnLimitsSettings(): TurnLimitsSettings {
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ describe("runSpawnedAgent", () => {
|
|||
model: { provider: "parent-p", model: "parent-m" },
|
||||
},
|
||||
});
|
||||
expect(started[0].maxModelCalls).toBe(20);
|
||||
expect(started[0].maxModelCalls).toBe(50);
|
||||
expect(started[0].signal).toBe(signal);
|
||||
expect(progress).toEqual([
|
||||
{ childTurnId: "child-1", agentName: "researcher", task: "find things" },
|
||||
|
|
@ -194,11 +194,11 @@ describe("runSpawnedAgent", () => {
|
|||
it("clamps a model-call budget above the global limit to it", async () => {
|
||||
const { services, started } = fakeServices({});
|
||||
const result = await runSpawnedAgent(
|
||||
{ task: "t", instructions: "x", max_model_calls: 50 },
|
||||
{ task: "t", instructions: "x", max_model_calls: 80 },
|
||||
{ parentTurnId: "parent-1", signal, services },
|
||||
);
|
||||
expect(result.isError).toBe(false);
|
||||
expect(started[0].maxModelCalls).toBe(20);
|
||||
expect(started[0].maxModelCalls).toBe(50);
|
||||
});
|
||||
|
||||
it("uses the configured global limit as the sub-agent default and cap", async () => {
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ import {
|
|||
type TurnState,
|
||||
effectiveTools,
|
||||
requestMessagesFor,
|
||||
wrapUpNotice,
|
||||
} from "@x/shared/dist/turns.js";
|
||||
import type { IContextResolver } from "./context-resolver.js";
|
||||
|
||||
|
|
@ -47,21 +46,12 @@ export function composeModelRequest(
|
|||
for (let index = 0; index <= modelCallIndex; index++) {
|
||||
structural.push(...requestMessagesFor(state, index));
|
||||
}
|
||||
// A wrap-up call (final budgeted call of an interactive turn) sends no
|
||||
// tools and appends the budget notice, so the model must answer in text.
|
||||
// Both derive from the durable request flag + config, preserving exact
|
||||
// wire reproducibility.
|
||||
if (call.request.wrapUp) {
|
||||
structural.push(wrapUpNotice(state.definition.config.maxModelCalls));
|
||||
}
|
||||
return {
|
||||
systemPrompt: agent.systemPrompt,
|
||||
messages: encode(structural),
|
||||
// The snapshot's base tools plus any durable mid-turn extensions
|
||||
// recorded before this call (tools_extended events).
|
||||
tools: call.request.wrapUp
|
||||
? []
|
||||
: effectiveTools(state, modelCallIndex, agent.tools),
|
||||
tools: effectiveTools(state, modelCallIndex, agent.tools),
|
||||
parameters: call.request.parameters,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1789,81 +1789,6 @@ describe("model-call limit (26.9)", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Graceful budget wrap-up (issue #768)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("model-call limit wrap-up", () => {
|
||||
function persistedRequests(log: TEvent[]) {
|
||||
return log.filter(
|
||||
(e): e is Extract<TEvent, { type: "model_call_requested" }> =>
|
||||
e.type === "model_call_requested",
|
||||
);
|
||||
}
|
||||
|
||||
it("spends the final budgeted call of a chat turn wrapping up without tools", async () => {
|
||||
const { runtime, repo, models } = makeRuntime({
|
||||
models: [
|
||||
respond(completedResp(assistantCalls(toolCallPart("S", "echo")))),
|
||||
respond(completedResp(assistantText("partial summary"))),
|
||||
],
|
||||
});
|
||||
const turnId = await newTurn(runtime, {
|
||||
config: { humanAvailable: true, maxModelCalls: 2 },
|
||||
});
|
||||
const { outcome } = await advanceAndSettle(runtime, turnId);
|
||||
expect(outcome?.status).toBe("completed");
|
||||
|
||||
// The wrap-up rides the durable request…
|
||||
const requests = persistedRequests(await persisted(repo, turnId));
|
||||
expect(requests[0].request.wrapUp).toBeUndefined();
|
||||
expect(requests[1].request.wrapUp).toBe(true);
|
||||
|
||||
// …and the wire request had no tools plus the budget notice.
|
||||
expect(models.requests[0].tools.length).toBeGreaterThan(0);
|
||||
expect(models.requests[1].tools).toEqual([]);
|
||||
const messages = models.requests[1].messages;
|
||||
const last = messages[messages.length - 1] as { role: string; content: string };
|
||||
expect(last.role).toBe("user");
|
||||
expect(last.content).toMatch(/model-call limit/);
|
||||
});
|
||||
|
||||
it("a 1-call chat budget answers directly as a wrap-up call", async () => {
|
||||
const { runtime, models } = makeRuntime({
|
||||
models: [respond(completedResp(assistantText("hi")))],
|
||||
});
|
||||
const turnId = await newTurn(runtime, {
|
||||
config: { humanAvailable: true, maxModelCalls: 1 },
|
||||
});
|
||||
const { outcome } = await advanceAndSettle(runtime, turnId);
|
||||
expect(outcome?.status).toBe("completed");
|
||||
expect(models.requests[0].tools).toEqual([]);
|
||||
});
|
||||
|
||||
it("headless turns keep the hard model-call-limit failure with tools intact", async () => {
|
||||
const { runtime, models } = makeRuntime({
|
||||
models: [
|
||||
respond(completedResp(assistantCalls(toolCallPart("S", "echo")))),
|
||||
respond(completedResp(assistantCalls(toolCallPart("S2", "echo")))),
|
||||
],
|
||||
});
|
||||
const turnId = await newTurn(runtime, {
|
||||
config: {
|
||||
humanAvailable: false,
|
||||
autoPermission: true,
|
||||
maxModelCalls: 2,
|
||||
},
|
||||
});
|
||||
const { outcome } = await advanceAndSettle(runtime, turnId);
|
||||
expect(outcome).toMatchObject({
|
||||
status: "failed",
|
||||
code: MODEL_CALL_LIMIT_ERROR_CODE,
|
||||
});
|
||||
// No wrap-up in headless: the final call was composed with tools.
|
||||
expect(models.requests[1].tools.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Tool progress
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -3190,6 +3115,6 @@ describe("model-call limit resolution", () => {
|
|||
const { runtime, repo } = makeRuntime();
|
||||
expect(
|
||||
await createdMaxModelCalls(runtime, repo, { humanAvailable: true }),
|
||||
).toBe(20);
|
||||
).toBe(50);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1190,20 +1190,11 @@ class TurnAdvance {
|
|||
// what it ran with, and the model bridge maps the canonical value to
|
||||
// provider-specific options at invoke time.
|
||||
const reasoningEffort = this.definition.config.reasoningEffort;
|
||||
// Interactive turns spend their final budgeted call wrapping up: the
|
||||
// composer strips tools and appends the budget notice, so the user
|
||||
// gets a real answer instead of a model-call-limit failure. Headless
|
||||
// turns keep the hard failure — automation needs the machine-readable
|
||||
// outcome, and sub-agents already surface partial results.
|
||||
const wrapUp =
|
||||
this.definition.config.humanAvailable &&
|
||||
index === this.definition.config.maxModelCalls - 1;
|
||||
const request: z.infer<typeof ModelRequest> = {
|
||||
...(isRef && index === 0 ? { contextRef: context } : {}),
|
||||
messages: refs,
|
||||
parameters:
|
||||
reasoningEffort === undefined ? {} : { reasoningEffort },
|
||||
...(wrapUp ? { wrapUp: true as const } : {}),
|
||||
};
|
||||
await this.append({
|
||||
type: "model_call_requested",
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import { ReasoningEffort } from "./models.js";
|
|||
|
||||
export type JsonValue = z.infer<ReturnType<typeof z.json>>;
|
||||
|
||||
export const DEFAULT_MAX_MODEL_CALLS = 20;
|
||||
export const DEFAULT_MAX_MODEL_CALLS = 50;
|
||||
export const MODEL_CALL_LIMIT_ERROR_CODE = "model-call-limit";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -233,27 +233,8 @@ export const ModelRequest = z.object({
|
|||
contextRef: TurnContextRef.optional(),
|
||||
messages: z.array(ModelRequestMessageRef),
|
||||
parameters: z.record(z.string(), z.json()),
|
||||
// Final budgeted call of an interactive turn: the composer strips tools
|
||||
// and appends wrapUpNotice() so the model ends with a real answer
|
||||
// instead of the turn failing with model-call-limit (issue #768).
|
||||
wrapUp: z.literal(true).optional(),
|
||||
});
|
||||
|
||||
// The message appended to a wrap-up call's request. A pure function of the
|
||||
// turn's durable config, so replaying the log reproduces the wire bytes.
|
||||
export function wrapUpNotice(
|
||||
maxModelCalls: number,
|
||||
): z.infer<typeof ConversationMessage> {
|
||||
return {
|
||||
role: "user",
|
||||
content:
|
||||
`[system notice] You are on the final model call this turn's budget allows (model-call limit: ${maxModelCalls}); tools are no longer available. ` +
|
||||
"Now answer the user's request as fully and substantively as you can from the tool results and information already gathered above — " +
|
||||
"give them the actual content, not a status report. Then briefly note which parts you didn't get to, " +
|
||||
"and mention the turn was cut short by the model-call limit, which they can raise in Settings → Advanced.",
|
||||
};
|
||||
}
|
||||
|
||||
export const ModelCallRequested = z.object({
|
||||
type: z.literal("model_call_requested"),
|
||||
turnId: z.string(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue