mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🙊 refactor: Clarify Ask Question Schema Errors and Retry Guidance (#14279)
* fix(agents): clarify ask question validation errors * fix(agents): narrow question failure detection * fix(agents): persist question validation failures * fix(agents): track question validation failures
This commit is contained in:
parent
b7542871b9
commit
7447fddfb2
19 changed files with 525 additions and 14 deletions
|
|
@ -8,6 +8,15 @@ jest.mock('nanoid', () => ({
|
|||
jest.mock('@librechat/api', () => ({
|
||||
sendEvent: jest.fn(),
|
||||
HOST_FILE_AUTHORING_ARTIFACT_KEY: '__librechat_file_authoring',
|
||||
getToolInputValidationDetails: jest.fn((result, validationError) =>
|
||||
validationError != null
|
||||
? {
|
||||
toolName: result.tool_call.name,
|
||||
reason: 'option_label_too_long',
|
||||
fieldPath: validationError.fieldPath,
|
||||
}
|
||||
: null,
|
||||
),
|
||||
isCodeSessionToolName: jest.fn((name) =>
|
||||
['execute_code', 'bash_tool', 'read_file'].includes(name),
|
||||
),
|
||||
|
|
@ -15,6 +24,7 @@ jest.mock('@librechat/api', () => ({
|
|||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: {
|
||||
debug: jest.fn(),
|
||||
error: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
|
@ -648,6 +658,108 @@ describe('createToolEndCallback', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('tool input validation marker', () => {
|
||||
it('marks the streamed result and persisted content part out of band', async () => {
|
||||
const { GraphEvents, createContentAggregator } = jest.requireActual('@librechat/agents');
|
||||
const { getDefaultHandlers } = require('../callbacks');
|
||||
const { contentParts, aggregateContent, stepMap } = createContentAggregator();
|
||||
const toolInputValidationErrors = new Map([
|
||||
['tool-1', { fieldPath: 'options[0].label', isLengthLimit: true }],
|
||||
]);
|
||||
const handlers = getDefaultHandlers({
|
||||
res: { write: jest.fn() },
|
||||
contentParts,
|
||||
stepMap,
|
||||
aggregateContent,
|
||||
toolInputValidationErrors,
|
||||
toolEndCallback: jest.fn(),
|
||||
collectedUsage: [],
|
||||
});
|
||||
|
||||
aggregateContent({
|
||||
event: GraphEvents.ON_RUN_STEP,
|
||||
data: {
|
||||
id: 'step-1',
|
||||
index: 0,
|
||||
stepDetails: {
|
||||
type: 'tool_calls',
|
||||
tool_calls: [{ id: 'tool-1', name: 'ask_user_question', args: '{}' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const data = {
|
||||
result: {
|
||||
id: 'step-1',
|
||||
tool_call: {
|
||||
id: 'tool-1',
|
||||
name: 'ask_user_question',
|
||||
output:
|
||||
'Error processing tool: Received tool input did not match expected schema ' +
|
||||
'→ at options[0].label',
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
await handlers[GraphEvents.ON_RUN_STEP_COMPLETED].handle(
|
||||
GraphEvents.ON_RUN_STEP_COMPLETED,
|
||||
data,
|
||||
{ run_id: 'run-1', thread_id: 'conversation-1' },
|
||||
);
|
||||
|
||||
expect(data.result.tool_call.inputValidationError).toBe(true);
|
||||
expect(contentParts[0].tool_call.inputValidationError).toBe(true);
|
||||
expect(toolInputValidationErrors.size).toBe(0);
|
||||
});
|
||||
|
||||
it('does not mark successful output that resembles a schema error', async () => {
|
||||
const { GraphEvents, createContentAggregator } = jest.requireActual('@librechat/agents');
|
||||
const { getDefaultHandlers } = require('../callbacks');
|
||||
const { contentParts, aggregateContent, stepMap } = createContentAggregator();
|
||||
const handlers = getDefaultHandlers({
|
||||
res: { write: jest.fn() },
|
||||
contentParts,
|
||||
stepMap,
|
||||
aggregateContent,
|
||||
toolInputValidationErrors: new Map(),
|
||||
toolEndCallback: jest.fn(),
|
||||
collectedUsage: [],
|
||||
});
|
||||
|
||||
aggregateContent({
|
||||
event: GraphEvents.ON_RUN_STEP,
|
||||
data: {
|
||||
id: 'step-1',
|
||||
index: 0,
|
||||
stepDetails: {
|
||||
type: 'tool_calls',
|
||||
tool_calls: [{ id: 'tool-1', name: 'ask_user_question', args: '{}' }],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const data = {
|
||||
result: {
|
||||
id: 'step-1',
|
||||
tool_call: {
|
||||
id: 'tool-1',
|
||||
name: 'ask_user_question',
|
||||
output: 'Received tool input did not match expected schema → at options[0].label',
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
await handlers[GraphEvents.ON_RUN_STEP_COMPLETED].handle(
|
||||
GraphEvents.ON_RUN_STEP_COMPLETED,
|
||||
data,
|
||||
{ run_id: 'run-1', thread_id: 'conversation-1' },
|
||||
);
|
||||
|
||||
expect(data.result.tool_call).not.toHaveProperty('inputValidationError');
|
||||
expect(contentParts[0].tool_call).not.toHaveProperty('inputValidationError');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isStreamWritable', () => {
|
||||
/* Direct parametric coverage of the predicate that gates SSE writes
|
||||
* in both the chat-completions and Open Responses callbacks. The
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ const {
|
|||
HOST_FILE_AUTHORING_ARTIFACT_KEY,
|
||||
isCodeSessionToolName,
|
||||
shouldSignalSandboxStart,
|
||||
getToolInputValidationDetails,
|
||||
} = require('@librechat/api');
|
||||
const { processFileCitations } = require('~/server/services/Files/Citations');
|
||||
const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process');
|
||||
|
|
@ -314,6 +315,10 @@ function feedSubagentAggregator(aggregator, event) {
|
|||
* @param {Object} options - The options object.
|
||||
* @param {ServerResponse} options.res - The server response object.
|
||||
* @param {ContentAggregator} options.aggregateContent - Content aggregator function.
|
||||
* @param {Array<Object>} [options.contentParts] - Aggregated message content parts.
|
||||
* @param {Map<string, Object>} [options.stepMap] - Run steps keyed by step ID.
|
||||
* @param {Map<string, import('@librechat/api').ToolInputValidationError>} [options.toolInputValidationErrors]
|
||||
* Schema-validation errors keyed by tool-call ID at the execution error boundary.
|
||||
* @param {ToolEndCallback} options.toolEndCallback - Callback to use when tool ends.
|
||||
* @param {Array<UsageMetadata>} options.collectedUsage - The list of collected usage metadata.
|
||||
* @param {string | null} [options.streamId] - The stream ID for resumable mode, or null for standard mode.
|
||||
|
|
@ -330,6 +335,9 @@ function feedSubagentAggregator(aggregator, event) {
|
|||
function getDefaultHandlers({
|
||||
res,
|
||||
aggregateContent,
|
||||
contentParts = null,
|
||||
stepMap = null,
|
||||
toolInputValidationErrors = null,
|
||||
toolEndCallback,
|
||||
collectedUsage,
|
||||
collectedThoughtSignatures = null,
|
||||
|
|
@ -441,7 +449,32 @@ function getDefaultHandlers({
|
|||
* @param {GraphRunnableConfig['configurable']} [metadata] The runnable metadata.
|
||||
*/
|
||||
handle: async (event, data, metadata) => {
|
||||
const toolCallId = data?.result?.tool_call?.id;
|
||||
const validationError =
|
||||
typeof toolCallId === 'string' ? toolInputValidationErrors?.get(toolCallId) : null;
|
||||
const validationDetails = getToolInputValidationDetails(data?.result, validationError);
|
||||
if (typeof toolCallId === 'string') {
|
||||
toolInputValidationErrors?.delete(toolCallId);
|
||||
}
|
||||
if (validationDetails != null) {
|
||||
if (data?.result?.tool_call != null) {
|
||||
data.result.tool_call.inputValidationError = true;
|
||||
}
|
||||
logger.debug('[AgentToolValidation] Tool input rejected', {
|
||||
...validationDetails,
|
||||
runId: metadata?.run_id,
|
||||
conversationId: metadata?.thread_id,
|
||||
agentId: metadata?.agent_id,
|
||||
});
|
||||
}
|
||||
aggregateContent({ event, data });
|
||||
if (validationDetails != null) {
|
||||
const runStep = stepMap?.get(data?.result?.id);
|
||||
const toolCall = contentParts?.[runStep?.index]?.tool_call;
|
||||
if (toolCall != null) {
|
||||
toolCall.inputValidationError = true;
|
||||
}
|
||||
}
|
||||
if (data?.result != null) {
|
||||
await emitEvent(res, streamId, { event, data });
|
||||
} else if (checkIfLastAgent(metadata?.last_agent_id, metadata?.langgraph_node)) {
|
||||
|
|
|
|||
|
|
@ -143,6 +143,7 @@ class AgentClient extends BaseClient {
|
|||
subagentAggregatorsByToolCallId,
|
||||
contextUsageSink,
|
||||
usageEmitSink,
|
||||
toolInputValidationErrors,
|
||||
...clientOptions
|
||||
} = options;
|
||||
|
||||
|
|
@ -157,6 +158,11 @@ class AgentClient extends BaseClient {
|
|||
* persisted on `metadata.usage`.
|
||||
* @type {Array<import('librechat-data-provider').TTokenUsageEvent> | undefined} */
|
||||
this.usageEmitSink = usageEmitSink;
|
||||
/** Schema-validation exceptions keyed by tool-call ID. The completion
|
||||
* handler consumes these to distinguish execution failures from tool
|
||||
* output that merely contains similar text.
|
||||
* @type {Map<string, import('@librechat/api').ToolInputValidationError> | undefined} */
|
||||
this.toolInputValidationErrors = toolInputValidationErrors;
|
||||
/** @type {MessageContentComplex[]} */
|
||||
this.contentParts = contentParts;
|
||||
/** @type {Array<UsageMetadata>} */
|
||||
|
|
@ -1759,6 +1765,7 @@ class AgentClient extends BaseClient {
|
|||
// opts into the tool-approval wiring. Non-resumable callers (OpenAI-compat, Responses)
|
||||
// leave this off so an approval-gated tool can't pause where there's no resume path.
|
||||
hitlCapable: true,
|
||||
toolInputValidationErrors: this.toolInputValidationErrors,
|
||||
// Mid-run steering: drain queued user messages at each tool-batch
|
||||
// boundary and inject them into graph state. The offset wrapper
|
||||
// shifts SDK content indices past any spliced steer parts.
|
||||
|
|
@ -2089,6 +2096,7 @@ class AgentClient extends BaseClient {
|
|||
// The resumed run can pause AGAIN (another tool, a follow-up question), and this
|
||||
// controller owns that lifecycle, so it must keep the HITL wiring on the rebuilt run.
|
||||
hitlCapable: true,
|
||||
toolInputValidationErrors: this.toolInputValidationErrors,
|
||||
// Steering stays live across a pause/resume cycle: steers queued while
|
||||
// the resumed segment runs drain at its tool-batch boundaries.
|
||||
steering: this.buildSteerWiring(streamId),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue