mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 12:13:20 +00:00
Show the arguments a tool call sends, fixed values included
The approval card and a finished call showed what the model asked for, even where a fixed value replaced it. Calls now carry sent_arguments for the chat, leaving headers out since they may hold a fixed secret. The model's own arguments are kept as they were, because they are what later turns replay to it and it never sees fixed values.
This commit is contained in:
1 parent
52217d43be
commit
dba2de590b
9 files changed
+147
-7
No files matched your search
@@ -14,7 +14,7 @@ from docsgpt.agents.default_tools import (
|
||||
synthesized_default_tools,
|
||||
)
|
||||
from docsgpt import tracing
|
||||
from docsgpt.agents.tool_pins import iter_parameters, llm_fills, resolve_arguments
|
||||
from docsgpt.agents.tool_pins import iter_parameters, llm_fills, resolve_arguments, sent_arguments, shown_arguments
|
||||
from docsgpt.agents.tools.tool_action_parser import ToolActionParser
|
||||
from docsgpt.agents.tools.tool_manager import ToolManager
|
||||
from docsgpt.guardrails.types import Stage as GuardrailStage, resolve_tool_result
|
||||
@@ -1102,6 +1102,12 @@ class ToolExecutor:
|
||||
"pause_type": "awaiting_approval",
|
||||
"thought_signature": getattr(call, "thought_signature", None),
|
||||
}
|
||||
# The card shows what will be sent: fixed values replace what the
|
||||
# model asked for. ``arguments`` stays as the model sent it, since
|
||||
# resuming replays it to the model, which never sees fixed values.
|
||||
sent = sent_arguments(action_data, arguments, self._connection_parameters(tool_data))
|
||||
if action_data and sent != arguments:
|
||||
payload["sent_arguments"] = sent
|
||||
# Surface the device id so the approval UI can offer a
|
||||
# "don't ask again" sticky-pattern action for remote devices.
|
||||
if tool_data.get("name") == "remote_device":
|
||||
@@ -1482,6 +1488,11 @@ class ToolExecutor:
|
||||
sections = resolve_arguments(action_data, call_args, self._connection_parameters(tool_data))
|
||||
query_params, headers = sections["query_params"], sections["headers"]
|
||||
body, parameters = sections["body"], sections["parameters"]
|
||||
# The chat shows what was sent; ``arguments`` keeps what the model
|
||||
# asked for, which is what a later turn replays to it.
|
||||
sent = shown_arguments(sections)
|
||||
if sent != (call_args if isinstance(call_args, dict) else {}):
|
||||
tool_call_data["sent_arguments"] = sent
|
||||
|
||||
# Load tool (with caching)
|
||||
from docsgpt.connectors.service import ConnectionUnavailable
|
||||
|
||||
@@ -104,6 +104,32 @@ def resolve_arguments(
|
||||
return resolved
|
||||
|
||||
|
||||
def shown_arguments(sections: Mapping[str, Mapping[str, Any]]) -> dict:
|
||||
"""One flat ``{parameter: value}`` view of resolved sections, for the chat.
|
||||
|
||||
Headers are left out: they may carry a pinned secret (an API key the
|
||||
owner fixed), and the chat is shown to whoever runs the agent.
|
||||
"""
|
||||
shown: dict = {}
|
||||
for section in PARAM_SECTIONS:
|
||||
if section != "headers":
|
||||
shown.update(sections.get(section) or {})
|
||||
return shown
|
||||
|
||||
|
||||
def sent_arguments(
|
||||
action: Mapping,
|
||||
llm_arguments: Mapping[str, Any],
|
||||
connection_pins: Optional[Mapping[str, Any]] = None,
|
||||
) -> dict:
|
||||
"""What a call sends, flattened for display (see :func:`shown_arguments`).
|
||||
|
||||
The approval card and the finished call show this rather than what the
|
||||
model asked for, which fixed values may have replaced.
|
||||
"""
|
||||
return shown_arguments(resolve_arguments(action, llm_arguments, connection_pins))
|
||||
|
||||
|
||||
def coerce_value(details: Mapping, value: Any) -> Any:
|
||||
"""Check a value for a pin against the parameter's declared type.
|
||||
|
||||
|
||||
@@ -1240,6 +1240,9 @@ class LLMHandler(ABC):
|
||||
# can wire the sticky "don't ask again" button.
|
||||
if pause_info.get("device_id"):
|
||||
pause_data["device_id"] = pause_info["device_id"]
|
||||
# What will be sent once fixed values replace the model's.
|
||||
if pause_info.get("sent_arguments") is not None:
|
||||
pause_data["sent_arguments"] = pause_info["sent_arguments"]
|
||||
# A connection-backed tool whose account needs signing in: the
|
||||
# approval card becomes a Connect card.
|
||||
if pause_info.get("connection_required"):
|
||||
|
||||
@@ -17,7 +17,7 @@ import { AnswerSegment, getAnswerSegments } from './answerSegments';
|
||||
import MarkdownAnswer from './MarkdownAnswer';
|
||||
import { type SandboxArtifact } from './sandboxLinks';
|
||||
import StreamingStatusLine from './StreamingStatusLine';
|
||||
import { ToolCallsType } from './types';
|
||||
import { shownArguments, ToolCallsType } from './types';
|
||||
import { isWikiWriteCall } from './wikiToolCall';
|
||||
import { cn } from '@/lib/utils';
|
||||
|
||||
@@ -310,10 +310,10 @@ function InlineToolCallChip({
|
||||
<div className="animate-in fade-in mt-2 mr-5 ml-6 flex flex-col gap-2 duration-160 ease-out motion-reduce:animate-none">
|
||||
<ToolCallPanel
|
||||
title={t('conversation.inlineSteps.arguments')}
|
||||
copyText={JSON.stringify(toolCall.arguments ?? {}, null, 2)}
|
||||
copyText={JSON.stringify(shownArguments(toolCall), null, 2)}
|
||||
>
|
||||
<p className="max-h-80 overflow-y-auto font-mono text-xs whitespace-pre-wrap">
|
||||
{JSON.stringify(toolCall.arguments ?? {}, null, 2)}
|
||||
{JSON.stringify(shownArguments(toolCall), null, 2)}
|
||||
</p>
|
||||
</ToolCallPanel>
|
||||
<ToolCallPanel
|
||||
|
||||
@@ -77,6 +77,30 @@ describe('ConversationBubble', () => {
|
||||
(b) => b.textContent?.trim() === text,
|
||||
) as HTMLButtonElement;
|
||||
|
||||
it('shows the fixed values a call will send, not what the model asked', async () => {
|
||||
await render(
|
||||
<ConversationBubble
|
||||
type="ANSWER"
|
||||
toolCalls={[
|
||||
{
|
||||
...pendingCall,
|
||||
arguments: { text: 'hi', chat_id: '666' },
|
||||
sent_arguments: { text: 'hi', chat_id: '111' },
|
||||
},
|
||||
]}
|
||||
onToolAction={() => {}}
|
||||
/>,
|
||||
);
|
||||
expect(container.textContent).toContain('"chat_id":"111"');
|
||||
expect(container.textContent).not.toContain('666');
|
||||
const details = container.querySelector(
|
||||
`button[aria-label="${tr('conversation.toolApproval.details')}"]`,
|
||||
) as HTMLButtonElement;
|
||||
await act(async () => details.click());
|
||||
expect(container.querySelector('pre')?.textContent).toContain('"111"');
|
||||
expect(container.textContent).not.toContain('666');
|
||||
});
|
||||
|
||||
it('renders the approval bar on Button variants and disables Approve while a deny reason is typed', async () => {
|
||||
await render(
|
||||
<ConversationBubble
|
||||
|
||||
@@ -54,7 +54,7 @@ import { AnswerSegment } from './answerSegments';
|
||||
import { deriveArtifactChips } from './artifactChips';
|
||||
import { FEEDBACK, MESSAGE_TYPE, ResearchState } from './conversationModels';
|
||||
import ResearchProgress from './ResearchProgress';
|
||||
import { ToolCallsType } from './types';
|
||||
import { shownArguments, ToolCallsType } from './types';
|
||||
import { wikiWriteActionKey, wikiWritePath } from './wikiToolCall';
|
||||
|
||||
const DisableSourceFE = envVar('VITE_DISABLE_SOURCE_FE') === 'true';
|
||||
@@ -790,7 +790,7 @@ function ToolCallApprovalBar({
|
||||
0,
|
||||
toolCall.action_name.lastIndexOf('_'),
|
||||
);
|
||||
const argPreview = JSON.stringify(toolCall.arguments);
|
||||
const argPreview = JSON.stringify(shownArguments(toolCall));
|
||||
const truncated =
|
||||
argPreview.length > 60 ? argPreview.slice(0, 57) + '...' : argPreview;
|
||||
|
||||
@@ -899,7 +899,7 @@ function ToolCallApprovalBar({
|
||||
<Card variant="subtle" padding="sm" className="mb-2">
|
||||
<div className="scrollbar-overlay max-h-40 overflow-y-auto">
|
||||
<pre className="font-mono text-xs wrap-break-word whitespace-pre-wrap">
|
||||
{JSON.stringify(toolCall.arguments, null, 2)}
|
||||
{JSON.stringify(shownArguments(toolCall), null, 2)}
|
||||
</pre>
|
||||
</div>
|
||||
</Card>
|
||||
|
||||
@@ -3,6 +3,9 @@ export type ToolCallsType = {
|
||||
action_name: string;
|
||||
call_id: string;
|
||||
arguments: Record<string, any>;
|
||||
// What the call sends once fixed values replace what the model asked for;
|
||||
// absent when they are the same. The chat shows this.
|
||||
sent_arguments?: Record<string, any>;
|
||||
result?: Record<string, any>;
|
||||
error?: string;
|
||||
status?:
|
||||
@@ -39,3 +42,7 @@ export type ToolCallsType = {
|
||||
connector_name?: string | null;
|
||||
access?: 'read' | 'write' | null;
|
||||
};
|
||||
|
||||
/** The arguments to show for a call: what it sends, not what the model asked. */
|
||||
export const shownArguments = (toolCall: ToolCallsType): Record<string, any> =>
|
||||
toolCall.sent_arguments ?? toolCall.arguments ?? {};
|
||||
@@ -238,3 +238,48 @@ class TestPinHelpers:
|
||||
carried = carry_pins(old, fresh)
|
||||
assert carried["parameters"]["properties"]["team"]["value"] == "ENG"
|
||||
assert carried["parameters"]["properties"]["team"]["filled_by_llm"] is False
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestArgumentsShownForACall:
|
||||
"""What the chat shows for a call is what is sent, not what the model asked."""
|
||||
|
||||
def _pause(self, executor, action, call_args, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"docsgpt.agents.tool_executor.ToolActionParser",
|
||||
lambda _cls, **kw: Mock(parse_args=Mock(return_value=("t1", "telegram_send_message", call_args))),
|
||||
)
|
||||
call = Mock()
|
||||
call.name = "telegram_send_message"
|
||||
call.id = "c1"
|
||||
return executor.check_pause(_tools(action), call, "MockLLM")
|
||||
|
||||
def test_approval_card_shows_the_fixed_value(self, monkeypatch):
|
||||
action = {**_action(text=_llm(), chat_id=_pinned("111")), "require_approval": True}
|
||||
pending = self._pause(ToolExecutor(user="u"), action, {"text": "hi", "chat_id": "666"}, monkeypatch)
|
||||
assert pending["pause_type"] == "awaiting_approval"
|
||||
assert pending["sent_arguments"] == {"text": "hi", "chat_id": "111"}
|
||||
# What the model asked stays as it was: resuming replays it to the model.
|
||||
assert pending["arguments"] == {"text": "hi", "chat_id": "666"}
|
||||
|
||||
def test_no_separate_arguments_when_nothing_changes(self, monkeypatch):
|
||||
action = {**_action(text=_llm()), "require_approval": True}
|
||||
pending = self._pause(ToolExecutor(user="u"), action, {"text": "hi"}, monkeypatch)
|
||||
assert "sent_arguments" not in pending
|
||||
|
||||
def test_a_finished_call_records_what_was_sent(self, mock_tool_manager, monkeypatch):
|
||||
executor = ToolExecutor(user="u")
|
||||
action = _action(text=_llm(), chat_id=_pinned("111"))
|
||||
_run(executor, _tools(action), {"text": "hi", "chat_id": "666"}, monkeypatch)
|
||||
recorded = executor.tool_calls[-1]
|
||||
assert recorded["sent_arguments"] == {"text": "hi", "chat_id": "111"}
|
||||
assert recorded["arguments"] == {"text": "hi", "chat_id": "666"}
|
||||
|
||||
def test_headers_are_never_shown(self):
|
||||
from docsgpt.agents.tool_pins import sent_arguments
|
||||
|
||||
action = {
|
||||
"headers": {"properties": {"Authorization": _pinned("Bearer secret")}},
|
||||
"query_params": {"properties": {"id": _llm()}},
|
||||
}
|
||||
assert sent_arguments(action, {"id": "1"}) == {"id": "1"}
|
||||
@@ -215,6 +215,30 @@ class TestHandlerApprovalPause:
|
||||
]
|
||||
assert len(approval_events) == 1
|
||||
|
||||
def test_approval_event_carries_what_will_be_sent(self):
|
||||
handler = ConcreteHandler()
|
||||
agent = self._make_agent({
|
||||
"call_id": "c1",
|
||||
"name": "send_msg_0",
|
||||
"tool_name": "telegram",
|
||||
"tool_id": "0",
|
||||
"action_name": "send_msg",
|
||||
"arguments": {"text": "hello", "chat_id": "666"},
|
||||
"sent_arguments": {"text": "hello", "chat_id": "111"},
|
||||
"pause_type": "awaiting_approval",
|
||||
"thought_signature": None,
|
||||
})
|
||||
call = ToolCall(id="c1", name="send_msg_0", arguments='{"text": "hello"}')
|
||||
gen = handler.handle_tool_calls(agent, [call], {"0": {"name": "telegram"}}, [])
|
||||
events = []
|
||||
try:
|
||||
while True:
|
||||
events.append(next(gen))
|
||||
except StopIteration:
|
||||
pass
|
||||
(event,) = [e for e in events if e.get("data", {}).get("status") == "awaiting_approval"]
|
||||
assert event["data"]["sent_arguments"] == {"text": "hello", "chat_id": "111"}
|
||||
|
||||
def test_mixed_normal_and_approval(self):
|
||||
"""First tool runs normally, second needs approval."""
|
||||
handler = ConcreteHandler()
|
||||
|
||||
Reference in new issue
Block a user