-
Notifications
You must be signed in to change notification settings - Fork 431
View toolcall details #1398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
View toolcall details #1398
Changes from all commits
c57c8f6
8f6f0b4
91aad19
54e9feb
67c5506
8932586
cdaa6a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| ALTER TABLE | ||
| chat_messages | ||
| ADD | ||
| COLUMN tool_details TEXT DEFAULT NULL; | ||
|
Comment on lines
+1
to
+4
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainEnsure we persist NULL (not empty string) when there are no tool details. Downstream code appears to JSON-parse this field; an empty string would throw. Please confirm that inserts bind If // Before (risk: "" gets stored)
tool_details: message.tool_details.unwrap_or_default(),
// After (store NULL when absent)
tool_details: message.tool_details.as_deref()Run to verify call sites: 🏁 Script executed: #!/bin/bash
rg -n "tool_details.*unwrap_or_default\(\)|unwrap_or\(\"\"\)" -C2
rg -n "INSERT.*chat_messages" -C3 crates/db-user/src/chat_messages_ops.rsLength of output: 3741 Persist NULL for missing The current implementation in • Location: Suggested diff: - &message.tool_details.unwrap_or_default(),
+ &message.tool_details.as_deref(),This ensures:
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,9 @@ impl UserDatabase { | |
| created_at, | ||
| role, | ||
| content, | ||
| type | ||
| ) VALUES (?, ?, ?, ?, ?, ?) | ||
| type, | ||
| tool_details | ||
| ) VALUES (?, ?, ?, ?, ?, ?, ?) | ||
| RETURNING *", | ||
| vec![ | ||
| message.id, | ||
|
|
@@ -25,6 +26,7 @@ impl UserDatabase { | |
| message.role.to_string(), | ||
| message.content, | ||
| message.r#type.to_string(), | ||
| message.tool_details.unwrap_or_default(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inserting empty string for absent tool_details loses null semantics and can break queries expecting NULL; pass SQL NULL when the Option is None for consistency with schema and existing patterns. (Based on your team's feedback about ensuring Option fields map to SQL NULL instead of empty strings for accurate database semantics.) Prompt for AI agents |
||
| ], | ||
| ) | ||
| .await?; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.