Include tool call ID in enhanced tool results for artifact creation#1974
Include tool call ID in enhanced tool results for artifact creation#1974tim-inkeep merged 27 commits intomainfrom
Conversation
* few fixes for GenericPromptEditor/GenericJsonEditor * Add changeset for GenericPromptEditor/GenericJsonEditor layout fixes Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com> * Update generic-prompt-editor.tsx --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com>
…aces (#1903) * local time for traces, end date when agent is done running * changeset * lint and claude comment * remove unneeded state
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 575bf43 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No documentation updates needed for this PR. This is an internal bugfix that ensures tool call IDs are properly passed during artifact creation—an implementation detail that doesn't affect user-facing APIs or behavior. |
There was a problem hiding this comment.
PR Review Summary
(2) Total Issues | Risk: Low
🟠⚠️ Major (1) 🟠⚠️
🟠 1) Agent.ts:2495-2730 Missing test coverage for enhanceToolResultWithStructureHints
Issue: The enhanceToolResultWithStructureHints method has zero unit test coverage. This PR adds a new toolCallId parameter and _toolCallId field to the method, but there are no tests verifying this behavior works correctly.
Why: Without tests, the following regressions could go undetected:
- The
_toolCallIdfield might not be added when expected (toolCallId provided + artifact components present), causing artifact creation to fail with incorrect tool call IDs - The
_toolCallIdfield might be incorrectly added when it should not be (toolCallId is undefined/null) - Edge cases like null/undefined results or non-object results could break silently
This is important business logic for artifact creation that could cause user-facing errors if tool call IDs are missing or incorrect.
Fix: Add unit tests for enhanceToolResultWithStructureHints. Consider testing through the Agent class or extracting to a testable utility. Suggested test cases:
// 1. should add _toolCallId when toolCallId provided and artifactComponents exist
// Input: result = { data: 'test' }, toolCallId = 'call_123', agent has artifactComponents
// Expected: result contains _toolCallId: 'call_123'
// 2. should not add _toolCallId when toolCallId is undefined
// Input: result = { data: 'test' }, toolCallId = undefined
// Expected: result does NOT contain _toolCallId key
// 3. should not enhance result when no artifactComponents
// Input: result = { data: 'test' }, toolCallId = 'call_123', agent has no artifactComponents
// Expected: result returned unchanged
// 4. should return null/undefined result unchanged
// Input: result = null, toolCallId = 'call_123'
// Expected: returns nullRefs:
- Agent.ts:2495-2730 — enhanceToolResultWithStructureHints method
- Agent.ts:1065-1068 — call site passing toolCallId
🟡 Minor (1) 🟡
🟡 1) Agent.ts:2462 Inconsistent metadata filtering: _toolCallId not stripped alongside _structureHints
Issue: The formatToolResult method filters out _structureHints from tool results before storing in conversation history (line 2462), but does not filter the newly added _toolCallId field. The same filtering pattern exists in ArtifactService.ts:151 and BaseCompressor.ts:609.
Why: This creates inconsistent behavior where _toolCallId persists in some contexts (fresh tool results, stored history) while _structureHints is removed. If _toolCallId is intended only as runtime guidance for the LLM during artifact creation, it may be worth filtering it consistently with _structureHints. If it's intentionally meant to persist, consider documenting this decision.
Fix: Either:
- Add
_toolCallIdto the filter conditions alongside_structureHintsin all three locations, OR - Add a code comment explicitly documenting that
_toolCallIdis intentionally preserved (and why)
Refs:
- Agent.ts:2462 — formatToolResult filtering
- ArtifactService.ts:151 — artifact creation filtering
- BaseCompressor.ts:609 — compression filtering
💡 APPROVE WITH SUGGESTIONS
Summary: The implementation is straightforward and correct — the _toolCallId field is properly added to enhanced tool results with appropriate conditional logic. However, the change would benefit from test coverage to prevent regressions, and there's a minor design question about whether _toolCallId should be filtered consistently with _structureHints in cleanup paths. Neither issue is blocking, but addressing the test coverage would improve confidence in this change.
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
Agent.ts:2717 |
Guidance text uses emoji prefixes (🔧 CRITICAL:) |
Consistent with existing patterns in the same code block; developer preference |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-tests |
2 | 1 | 0 | 0 | 0 | 0 | 0 |
pr-review-llm |
2 | 1 | 0 | 0 | 0 | 0 | 1 |
| Total | 4 | 2 | 0 | 0 | 0 | 0 | 1 |
…1974) * updated test * added changeset * added tests * updated * tupecheck * few fixes for GenericPromptEditor/GenericJsonEditor (#1904) * few fixes for GenericPromptEditor/GenericJsonEditor * Add changeset for GenericPromptEditor/GenericJsonEditor layout fixes Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com> * Update generic-prompt-editor.tsx --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com> * local time for traces, end date only when agent is done running in traces (#1903) * local time for traces, end date when agent is done running * changeset * lint and claude comment * remove unneeded state * updated tests * updated * updated tests * made a single transaction * renamed function * updated * updated tool results to have the tool call id * added changeset * address spicedb comments * reverted bad changes * removed --------- Co-authored-by: Dimitri POSTOLOV <dmytropostolov@gmail.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Dimitri POSTOLOV <undefined@users.noreply.github.com> Co-authored-by: shagun-singh-inkeep <shagun.singh@inkeep.com> Co-authored-by: omar-inkeep <omar@inkeep.com>
Adds
_toolCallIdfield to tool results when artifact components are available. This ensures agents have the correct tool call ID reference when creating artifacts, preventing the need to generate or guess IDs.