mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-11-13 07:40:59 +00:00
fix(assistant): send a blind write back with its schema, pair unrun calls with their answers, and give the wordless-answer recovery its own nudge
A write from a group the turn had not loaded was held as a change card with schema-less arguments and failed on apply (#2967); it now goes back to the model with the group loaded. Calls answered as not run are now listed on the assistant turn too, so a thread replayed on another provider has no orphan tool results. The blind-call hint only goes with the blind call.
This commit is contained in:
@@ -264,8 +264,14 @@ interface Turn {
|
||||
history: AiChatMessage[];
|
||||
usage: { inputTokens: number; cachedInputTokens: number; outputTokens: number };
|
||||
knownPlaceIds: Set<string>;
|
||||
/** Whether the turn used its one push (an empty, cut-off, announcing or wordless answer). */
|
||||
/** Whether the turn used its one push (an empty, cut-off or announcing answer). */
|
||||
nudged: boolean;
|
||||
/**
|
||||
* Whether the turn already asked for the answer after a step that only
|
||||
* offered follow-ups. Apart from the push, so a turn that was pushed once
|
||||
* still gets its answer written (#2789); each is asked at most once.
|
||||
*/
|
||||
askedForAnswer: boolean;
|
||||
/** The chips of follow-ups offered before any answer, shown under the answer once it is written. */
|
||||
followUps: AssistantFollowUps | null;
|
||||
}
|
||||
@@ -528,7 +534,7 @@ function answeredAsk(call: AiToolCall): AssistantMessagePart {
|
||||
*/
|
||||
async function askForTheAnswer(turn: Turn, rowId: number, step: StepRecord & { asks: AiToolCall[] }): Promise<null> {
|
||||
const { asks, providerState } = step;
|
||||
turn.nudged = true;
|
||||
turn.askedForAnswer = true;
|
||||
turn.followUps = followUpsOf(asks[asks.length - 1].args);
|
||||
const parts = [...step.parts, ...asks.map((call) => ({ ...answeredAsk(call), result: WRITE_THE_ANSWER }))];
|
||||
await turn.deps.store.saveStep(rowId, { parts, providerState }, 'complete');
|
||||
@@ -560,12 +566,15 @@ async function runStep(turn: Turn, step: number): Promise<AgentOutcome | null> {
|
||||
...searched.tail,
|
||||
];
|
||||
const empty = received.length === 0 && !text.trim();
|
||||
if (empty && asks.length > 0 && !turn.nudged) return askForTheAnswer(turn, rowId, { parts, providerState, asks });
|
||||
if (empty && asks.length > 0 && !turn.askedForAnswer)
|
||||
return askForTheAnswer(turn, rowId, { parts, providerState, asks });
|
||||
// An answer that offered follow-ups is finished, however it ends.
|
||||
const settled = asks.length > 0 && !empty;
|
||||
const announced = !settled && received.length === 0 && step === 0 && ANNOUNCED.test(text.trimEnd());
|
||||
const push = empty || (!settled && received.length === 0 && cutOff) ? GO_ON : announced ? MAKE_THE_CALLS : null;
|
||||
if (push && !turn.nudged) {
|
||||
// Follow-ups with no answer a second time end the turn: the model was already asked for the answer.
|
||||
const wordlessAgain = empty && asks.length > 0;
|
||||
if (push && !turn.nudged && !wordlessAgain) {
|
||||
turn.nudged = true;
|
||||
await deps.store.saveStep(rowId, { parts, providerState }, 'complete');
|
||||
w.finishStep();
|
||||
@@ -663,6 +672,7 @@ export async function runAgentLoop(
|
||||
usage: { inputTokens: 0, cachedInputTokens: 0, outputTokens: 0 },
|
||||
knownPlaceIds,
|
||||
nudged: false,
|
||||
askedForAnswer: false,
|
||||
followUps: null,
|
||||
};
|
||||
return stepsFrom(turn, 0);
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { AssistantToolCallRow } from '../../db/repositories/AssistantToolCa
|
||||
import { parseDbTimestamp, utcSuffix } from '../../db/types';
|
||||
import { decodeJson } from '../../utils/json-column';
|
||||
import type { AiChatMessage, AiProviderState, AiToolCall } from '../ai-connections/ai-chat-client.interface';
|
||||
import { asRecord, asText, parseToolArgs } from '../ai-connections/providers/provider-http';
|
||||
import { tourFileOutcome, tourFileReadNote, tourStopOutcome } from './assistant-history-tours';
|
||||
import { fitToSize } from './assistant-output';
|
||||
import {
|
||||
@@ -48,17 +49,23 @@ const NOT_RUN = 'This call did not run: the answer was stopped before it.';
|
||||
* The calls a provider's own record of a step holds, which it replays as they
|
||||
* came and wants answered: Anthropic's tool uses, the Responses API's function
|
||||
* calls. A step stopped while its calls ran (the user left the page) keeps the
|
||||
* whole record but answers only the calls that ran.
|
||||
* whole record but answers only the calls that ran. The arguments come along,
|
||||
* so an adapter that rebuilds the step from its calls (another provider than
|
||||
* the one that recorded it) still finds every call an answer names.
|
||||
*/
|
||||
function recordedCalls(state: AiProviderState | null): Array<{ id: string; name: string }> {
|
||||
const entries =
|
||||
function recordedCalls(state: AiProviderState | null): AiToolCall[] {
|
||||
const calls =
|
||||
state?.adapter === 'anthropic-messages'
|
||||
? state.blocks.filter((block) => block.type === 'tool_use').map((block) => [block.id, block.name])
|
||||
? state.blocks
|
||||
.filter((block) => block.type === 'tool_use')
|
||||
.map((block) => ({ id: block.id, name: block.name, args: asRecord(block.input) }))
|
||||
: state?.adapter === 'openai-responses'
|
||||
? state.items.filter((item) => item.type === 'function_call').map((item) => [item.call_id, item.name])
|
||||
? state.items
|
||||
.filter((item) => item.type === 'function_call')
|
||||
.map((item) => ({ id: item.call_id, name: item.name, args: parseToolArgs(asText(item.arguments)) }))
|
||||
: [];
|
||||
return entries.flatMap(([id, name]) =>
|
||||
typeof id === 'string' ? [{ id, name: typeof name === 'string' ? name : '' }] : [],
|
||||
return calls.flatMap(({ id, name, args }) =>
|
||||
typeof id === 'string' ? [{ id, name: typeof name === 'string' ? name : '', args }] : [],
|
||||
);
|
||||
}
|
||||
|
||||
@@ -314,7 +321,16 @@ export function toChatHistory(
|
||||
const text = textOf(parts);
|
||||
if (!text && toolCalls.length === 0) return;
|
||||
const providerState = decodeJson(ASSISTANT_PROVIDER_STATE, row.provider_state, `message ${row.id}`);
|
||||
history.push({ role: 'assistant', text, toolCalls, providerState });
|
||||
// The recorded calls that never ran go into the step's calls as well: an adapter that does not
|
||||
// replay this record rebuilds the step from them, and would otherwise find answers to calls it never made.
|
||||
const answered = new Set(toolCalls.map((call) => call.id));
|
||||
const unrun: AiToolCall[] = [];
|
||||
for (const recorded of recordedCalls(providerState)) {
|
||||
if (answered.has(recorded.id)) continue;
|
||||
answered.add(recorded.id);
|
||||
unrun.push(recorded);
|
||||
}
|
||||
history.push({ role: 'assistant', text, toolCalls: [...toolCalls, ...unrun], providerState });
|
||||
calls.forEach((part, index) => {
|
||||
history.push({
|
||||
role: 'tool',
|
||||
@@ -324,11 +340,8 @@ export function toChatHistory(
|
||||
...(part.isError ? { isError: true } : {}),
|
||||
});
|
||||
});
|
||||
const answered = new Set(toolCalls.map((call) => call.id));
|
||||
for (const recorded of recordedCalls(providerState)) {
|
||||
if (answered.has(recorded.id)) continue;
|
||||
answered.add(recorded.id);
|
||||
history.push({ role: 'tool', toolCallId: recorded.id, name: recorded.name, output: NOT_RUN, isError: true });
|
||||
for (const call of unrun) {
|
||||
history.push({ role: 'tool', toolCallId: call.id, name: call.name, output: NOT_RUN, isError: true });
|
||||
}
|
||||
});
|
||||
return history;
|
||||
|
||||
@@ -266,9 +266,12 @@ export class AssistantToolSession {
|
||||
/** The groups this turn loaded; the core is always there. */
|
||||
private readonly loaded = new Set<string>();
|
||||
|
||||
/** Tools the model called before their group was loaded in this turn, by name, with that group. */
|
||||
/** Reads the model called before their group was loaded in this turn, by name, with that group; told once. */
|
||||
private readonly calledBlind = new Map<string, string>();
|
||||
|
||||
/** Groups a blind call loaded whose schemas no step has been offered yet. */
|
||||
private readonly unoffered = new Set<string>();
|
||||
|
||||
/** Whether the session offers its tools by group: a trip's turn and a home turn do. */
|
||||
private get grouped(): boolean {
|
||||
return this.mode === 'turn' || this.mode === 'home';
|
||||
@@ -280,6 +283,8 @@ export class AssistantToolSession {
|
||||
* journey's session offers all it holds and no more.
|
||||
*/
|
||||
specs(): AiToolSpec[] {
|
||||
// The step these specs go to sees every loaded group's schema.
|
||||
this.unoffered.clear();
|
||||
const listed = [...this.tools.values()]
|
||||
.filter((tool) => tool.offered)
|
||||
.filter((tool) => !this.grouped || tool.hint.group === null || this.loaded.has(tool.hint.group))
|
||||
@@ -338,9 +343,25 @@ export class AssistantToolSession {
|
||||
// The model called a tool it was not offered in this turn (it knows the
|
||||
// name from an earlier turn), so it wrote the arguments without the
|
||||
// schema (#2967). Its group is loaded here, so the next step offers the
|
||||
// schema, and a refusal of these arguments says so (see call()).
|
||||
// schema.
|
||||
this.loaded.add(unloaded);
|
||||
this.calledBlind.set(name, unloaded);
|
||||
this.unoffered.add(unloaded);
|
||||
}
|
||||
const group = this.tools.get(name)?.hint.group;
|
||||
if (group && this.unoffered.has(group)) {
|
||||
// A change is never run in the turn but held as a card, and the turn
|
||||
// ends there: its arguments would only fail once the user applies it,
|
||||
// with the model long gone. It is sent back for the next step instead.
|
||||
if (this.tools.get(name)?.kind === 'write') {
|
||||
return {
|
||||
error: `${name} belongs to the "${group}" tools, which were not loaded in this turn, so it did not run. They are loaded now: send the call again with the arguments exactly as its schema describes them.`,
|
||||
};
|
||||
}
|
||||
// A read runs at once; a refusal of its arguments says why (see call()).
|
||||
this.calledBlind.set(name, group);
|
||||
} else {
|
||||
// A blind read refused before it ran (another trip, an unknown place) must not hint a later call.
|
||||
this.calledBlind.delete(name);
|
||||
}
|
||||
const given = pinNamesOf(this.pinArg).filter((key) => args[key] !== undefined);
|
||||
if (given.some((key) => Number(args[key]) !== this.pinnedId)) {
|
||||
@@ -361,7 +382,10 @@ export class AssistantToolSession {
|
||||
const final = name === TRANSIT_CREATE_TOOL ? await this.withFoundItinerary(args) : args;
|
||||
const run = await this.invoke(name, final);
|
||||
this.audit(name);
|
||||
// Only the blind call itself is told about the schema: a later failure of the
|
||||
// same tool, written with the schema in hand, has another reason.
|
||||
const blind = this.calledBlind.get(name);
|
||||
this.calledBlind.delete(name);
|
||||
if (!run.ok && blind) {
|
||||
const hint = `${name} was called before the "${blind}" tools were loaded in this turn, so its arguments were written without its schema. They are loaded now and the schema is in your tools: send the call again with the arguments exactly as it describes them.`;
|
||||
return {
|
||||
|
||||
@@ -859,6 +859,49 @@ describe('runAgentLoop', () => {
|
||||
expect(outcome).toMatchObject({ end: 'failed', code: 'empty_answer' });
|
||||
expect(t.requests).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('ASTAGT-045: follow-ups with no answer after an announcing first answer are still asked for the answer', async () => {
|
||||
const t = setup([
|
||||
[{ type: 'text-delta', delta: 'Let me check your places:' }],
|
||||
[{ type: 'tool-call', call: { id: 'l1', name: 'list_places', args: {} } }],
|
||||
[ask('toolu_8', ['Add Ginkaku-ji']), { type: 'finish', reason: 'tool-calls' }],
|
||||
[{ type: 'text-delta', delta: 'Kinkaku-ji is the only place so far.' }],
|
||||
]);
|
||||
const outcome = await runAgentLoop(t.deps, t.input, t.writer, new AbortController().signal);
|
||||
expect(outcome.end).toBe('stop');
|
||||
expect(t.requests).toHaveLength(4);
|
||||
expect(t.requests[1].messages.at(-1)).toMatchObject({ role: 'user', text: expect.stringContaining('now') });
|
||||
expect(t.requests[3].messages.at(-1)).toMatchObject({
|
||||
role: 'tool',
|
||||
toolCallId: 'toolu_8',
|
||||
output: expect.stringContaining('Not shown yet'),
|
||||
});
|
||||
expect(t.chunks).toContainEqual({ type: 'data-trek-follow-ups', data: { questions: ['Add Ginkaku-ji'] } });
|
||||
expect(t.memory.saved.map((saved) => saved.status)).not.toContain('error');
|
||||
});
|
||||
});
|
||||
|
||||
it('ASTAGT-046: a change its session sends back (called before its tools were loaded) is a tool error, never held (#2967)', async () => {
|
||||
const t = setup([
|
||||
[{ type: 'tool-call', call: { id: 'w1', name: 'create_place', args: { name: 'Blind' } } }],
|
||||
[{ type: 'tool-call', call: { id: 'w2', name: 'create_place', args: { name: 'With schema' } } }],
|
||||
]);
|
||||
t.tools.pinArgs.mockImplementationOnce(() => ({ error: 'create_place did not run: send it again' }));
|
||||
const outcome = await runAgentLoop(t.deps, t.input, t.writer, new AbortController().signal);
|
||||
expect(outcome.end).toBe('awaiting');
|
||||
expect(t.memory.saved[0].step.parts).toContainEqual({
|
||||
type: 'tool-call',
|
||||
toolCallId: 'w1',
|
||||
toolName: 'create_place',
|
||||
args: { name: 'Blind' },
|
||||
result: 'create_place did not run: send it again',
|
||||
isError: true,
|
||||
});
|
||||
const sentBack = t.requests[1].messages.find((m) => m.role === 'tool' && m.toolCallId === 'w1');
|
||||
expect(sentBack).toMatchObject({ isError: true, output: expect.stringContaining('did not run') });
|
||||
expect(t.deps.describe).toHaveBeenCalledTimes(1);
|
||||
expect(t.memory.held).toHaveLength(1);
|
||||
expect(t.memory.held[0].held.map((call) => call.args)).toEqual([{ name: 'With schema' }]);
|
||||
});
|
||||
|
||||
it('ASTAGT-038: followUpsOf trims, drops duplicates and blanks, cuts each to a chip and keeps three', () => {
|
||||
|
||||
@@ -651,6 +651,61 @@ describe('toChatHistory: calls the model made of the server tools (#2845)', () =
|
||||
expect(outputs).toEqual(['call_1', 'call_2']);
|
||||
});
|
||||
|
||||
it('ASTHIS-021: a stopped step replayed on another provider names every call it answers', () => {
|
||||
const anthropic = stoppedMidStep({
|
||||
adapter: 'anthropic-messages',
|
||||
blocks: [
|
||||
{ type: 'text', text: 'Let me look.' },
|
||||
{ type: 'tool_use', id: 'call_1', name: 'list_places', input: {} },
|
||||
{ type: 'tool_use', id: 'call_2', name: 'get_weather', input: { day: 3 } },
|
||||
],
|
||||
});
|
||||
expect(anthropic.find((message) => message.role === 'assistant')).toMatchObject({
|
||||
toolCalls: [
|
||||
{ id: 'call_1', name: 'list_places', args: {} },
|
||||
{ id: 'call_2', name: 'get_weather', args: { day: 3 } },
|
||||
],
|
||||
});
|
||||
|
||||
// Chat Completions: every tool message follows an assistant message whose tool_calls name it.
|
||||
const chat = toChatMessages({ system: '', messages: anthropic, tools: [] });
|
||||
chat.forEach((message, index) => {
|
||||
if (message.role !== 'tool') return;
|
||||
const before = chat.slice(0, index).reverse().find((earlier) => earlier.role === 'assistant');
|
||||
const named = ((before?.tool_calls ?? []) as Array<{ id: string }>).map((call) => call.id);
|
||||
expect(named).toContain(message.tool_call_id);
|
||||
});
|
||||
|
||||
// Responses: every function_call_output has its function_call before it.
|
||||
const items = toResponsesInput({ system: '', messages: anthropic, tools: [] });
|
||||
items.forEach((item, index) => {
|
||||
if (item.type !== 'function_call_output') return;
|
||||
const made = items
|
||||
.slice(0, index)
|
||||
.some((earlier) => earlier.type === 'function_call' && earlier.call_id === item.call_id);
|
||||
expect(made).toBe(true);
|
||||
});
|
||||
|
||||
// Anthropic: a step the Responses API recorded is rebuilt with every tool_use its results answer.
|
||||
const responses = stoppedMidStep({
|
||||
adapter: 'openai-responses',
|
||||
items: [
|
||||
{ type: 'function_call', call_id: 'call_1', name: 'list_places', arguments: '{}' },
|
||||
{ type: 'function_call', call_id: 'call_2', name: 'get_weather', arguments: 'not json' },
|
||||
],
|
||||
});
|
||||
const messages = toAnthropicMessages(responses);
|
||||
const uses = messages[1].content.filter((block) => block.type === 'tool_use');
|
||||
expect(uses.map((block) => [block.id, block.input])).toEqual([
|
||||
['call_1', {}],
|
||||
['call_2', {}],
|
||||
]);
|
||||
const results = messages[2].content
|
||||
.filter((block) => block.type === 'tool_result')
|
||||
.map((block) => block.tool_use_id);
|
||||
expect(results).toEqual(['call_1', 'call_2']);
|
||||
});
|
||||
|
||||
it('ASTHIS-017: a step that only kept a note is the model call it was, answered with what was kept', () => {
|
||||
const history = toChatHistory(
|
||||
[
|
||||
|
||||
@@ -425,6 +425,63 @@ describe('an assistant tool session', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('ASTTOOL-025: a change called before its group was loaded is sent back until a step was offered its schema (#2967)', async () => {
|
||||
const { userId, tripId } = await ownerWithTrips();
|
||||
const session = await tools.open(userId, tripId);
|
||||
try {
|
||||
expect(session.specs().map((spec) => spec.name)).not.toContain('insert_tour_stop');
|
||||
// Held as a card, the blind arguments would only fail once the user applies it.
|
||||
expect(session.pinArgs('insert_tour_stop', { placeId: '530' })).toEqual({
|
||||
error: expect.stringContaining('"tours" tools, which were not loaded in this turn, so it did not run'),
|
||||
});
|
||||
// A second change of the same step was written without the schema too.
|
||||
expect(session.pinArgs('create_city_walk', { stations: '[1, 2]' })).toEqual({
|
||||
error: expect.stringContaining('did not run'),
|
||||
});
|
||||
// The next step is offered the schema, and the call goes through.
|
||||
expect(session.specs().map((spec) => spec.name)).toContain('insert_tour_stop');
|
||||
expect(session.pinArgs('insert_tour_stop', { placeId: 530 })).toHaveProperty('args');
|
||||
} finally {
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('ASTTOOL-026: only the blind call itself is told about the schema; a later failure of the same tool is not', async () => {
|
||||
const { userId, tripId } = await ownerWithTrips();
|
||||
const session = await tools.open(userId, tripId);
|
||||
try {
|
||||
session.specs();
|
||||
expect(session.pinArgs('get_tour_plan', { placeId: '530' })).toEqual({ args: { placeId: '530' } });
|
||||
const blind = await session.call('get_tour_plan', { placeId: '530' });
|
||||
expect(String(blind.value)).toContain('"tours" tools were loaded in this turn');
|
||||
// Sent again after the next step's specs: it fails on its own, without the hint.
|
||||
session.specs();
|
||||
expect(session.pinArgs('get_tour_plan', { placeId: 999_999 })).toEqual({ args: { placeId: 999_999 } });
|
||||
const again = await session.call('get_tour_plan', { placeId: 999_999 });
|
||||
expect(again.ok).toBe(false);
|
||||
expect(String(again.value)).not.toContain('were loaded in this turn');
|
||||
} finally {
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('ASTTOOL-027: a blind read refused before it ran leaves no hint for a later call of the same tool', async () => {
|
||||
const { userId, tripId } = await ownerWithTrips();
|
||||
const session = await tools.open(userId, tripId);
|
||||
try {
|
||||
session.specs();
|
||||
// Called blind, then refused by the loop before it ran: call() never sees it.
|
||||
expect(session.pinArgs('get_tour_plan', { placeId: '530' })).toHaveProperty('args');
|
||||
session.specs();
|
||||
expect(session.pinArgs('get_tour_plan', { placeId: 999_999 })).toEqual({ args: { placeId: 999_999 } });
|
||||
const run = await session.call('get_tour_plan', { placeId: 999_999 });
|
||||
expect(run.ok).toBe(false);
|
||||
expect(String(run.value)).not.toContain('were loaded in this turn');
|
||||
} finally {
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('ASTTOOL-014: an undo session holds its target and nothing else', async () => {
|
||||
const { userId, tripId } = await ownerWithTrips();
|
||||
const session = await tools.open(userId, tripId, new Set(['delete_place']));
|
||||
|
||||
Reference in New Issue
Block a user