Skip to content

Add .NET low-level tool-definition E2E test [6/6]#1728

Merged
edburns merged 2 commits into
mainfrom
edburns/1682-06-dotnet-new-test
Jun 18, 2026
Merged

Add .NET low-level tool-definition E2E test [6/6]#1728
edburns merged 2 commits into
mainfrom
edburns/1682-06-dotnet-new-test

Conversation

@edburns

@edburns edburns commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds .NET low-level tool-definition E2E coverage and includes the session-lifecycle replay prompt alignment fix.

This PR is related to issue #1682 but does not fix #1682.

What changed

  • Adds Low_Level_Tool_Definition coverage in:
    • dotnet/test/E2E/ToolsE2ETests.cs
  • Includes session-lifecycle prompt alignment fix:
    • dotnet/test/E2E/SessionLifecycleE2ETests.cs
  • Reuses the shared low-level snapshot introduced by PR Add Java low-level tool definition E2E test and skill [1/6] #1721:
    • test/snapshots/tools/low_level_tool_definition.yaml
  • Aligns low-level tool definitions with snapshot-exercised paths only and validates keyword argument handling in the search tool.

Dependency / sequencing

Related

Related to issue #1682 but does not fix #1682.

Align low_level_tool_definition coverage with PR #1721 snapshot behavior by only defining tools exercised by the shared snapshot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@edburns edburns requested a review from a team as a code owner June 18, 2026 19:57
Copilot AI review requested due to automatic review settings June 18, 2026 19:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Not ready to approve

The updated session-lifecycle prompt no longer matches the existing replay snapshot, which will cause the E2E test to fail unless the snapshot is updated in the same PR.

Pull request overview

Adds .NET E2E coverage for low-level tool definition APIs using the shared replay snapshot, and tweaks a session-lifecycle E2E prompt string.

Changes:

  • Add a new .NET E2E test Low_Level_Tool_Definition that defines custom tools via AIFunctionFactory and asserts tool argument handling + resulting assistant content.
  • Change the second session’s persistence “activity” prompt in the session-lifecycle E2E test (but this currently risks desync with the existing replay snapshot).
File summaries
File Description
dotnet/test/E2E/ToolsE2ETests.cs Adds a new low-level tool-definition E2E test that exercises multiple custom tools and validates keyword argument plumbing.
dotnet/test/E2E/SessionLifecycleE2ETests.cs Updates the second session prompt used to generate persistence activity for the replay-based lifecycle test.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment thread dotnet/test/E2E/SessionLifecycleE2ETests.cs Outdated
Restore the second lifecycle prompt to 'Say world' to match the existing session_lifecycle snapshot and avoid replay cache misses in CI.

Related to issue #1682 but does not fix #1682.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

Overview

This PR is the last ([6/6]) in a coordinated series that adds low_level_tool_definition E2E coverage to all six SDK implementations. With the sibling PRs #1724 (Go), #1725 (Node.js), #1726 (Python), and #1727 (Rust) also open, the full SDK matrix will have equivalent coverage — good planning.

The .NET implementation is consistent with the reference Java test (LowLevelToolDefinitionIT.java) in the key respects:

  • Reuses the shared test/snapshots/tools/low_level_tool_definition.yaml snapshot
  • Exercises the same two tools (set_current_phase / search_items) that the snapshot covers
  • Uses the idiomatic .NET raw-arg access pattern (AIFunctionArguments) analogous to Java's invocation.getArguments()
  • Asserts currentPhase, content keywords, and AIFunctionArguments keyword value — matching Java's assertions

Minor Observations

1. Scope difference vs Java (intentional, per PR description)

Java's LowLevelToolDefinitionIT registers a third tool — ToolDefinition.createOverride("grep", ...) — to test the built-in tool override path via the low-level API. The .NET test intentionally omits this ("aligned with snapshot-exercised paths only"), which is reasonable since grep is never invoked in the snapshot and the override scenario is already covered by ToolsE2ETests.Overrides_Built_In_Tool_With_Custom_Tool. No action needed, but worth noting for future alignment.

2. PR description mentions SessionLifecycleE2ETests.cs — not present in the diff

The PR body says:

Includes session-lifecycle prompt alignment fix:
dotnet/test/E2E/SessionLifecycleE2ETests.cs

However, only dotnet/test/E2E/ToolsE2ETests.cs appears in the file-changed list. If the session-lifecycle fix was intentionally dropped or moved to a separate PR, it may be worth updating the description to avoid confusion for reviewers and future readers.

Summary

The change is well-scoped, follows established patterns, and the broader series ensures all SDKs will have equivalent low-level tool definition coverage. No cross-SDK API consistency issues found.

Generated by SDK Consistency Review Agent for issue #1728 · sonnet46 2.3M ·

@edburns edburns enabled auto-merge June 18, 2026 20:39
@edburns edburns disabled auto-merge June 18, 2026 20:39
@edburns edburns merged commit c795282 into main Jun 18, 2026
18 checks passed
@edburns edburns deleted the edburns/1682-06-dotnet-new-test branch June 18, 2026 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Java: Ergonomics: Defining tools

2 participants