-
Notifications
You must be signed in to change notification settings - Fork 19
test: command hints, batch tool, finops formatting — 3 untested modules #427
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { describe, test, expect } from "bun:test" | ||
| import { Command } from "../../src/command/index" | ||
|
|
||
| describe("Command.hints: template placeholder extraction", () => { | ||
| test("extracts numbered placeholders in order", () => { | ||
| expect(Command.hints("Do $1 then $2")).toEqual(["$1", "$2"]) | ||
| }) | ||
|
|
||
| test("deduplicates repeated placeholders", () => { | ||
| expect(Command.hints("Use $1 and $1 again")).toEqual(["$1"]) | ||
| }) | ||
|
|
||
| test("sorts numbered placeholders numerically by string sort", () => { | ||
| // $10 sorts after $1 and $2 in string order | ||
| expect(Command.hints("$2 then $1 then $10")).toEqual(["$1", "$10", "$2"]) | ||
| }) | ||
|
|
||
| test("extracts $ARGUMENTS", () => { | ||
| expect(Command.hints("Run with $ARGUMENTS")).toEqual(["$ARGUMENTS"]) | ||
| }) | ||
|
|
||
| test("extracts both numbered and $ARGUMENTS", () => { | ||
| expect(Command.hints("$1 and $ARGUMENTS")).toEqual(["$1", "$ARGUMENTS"]) | ||
| }) | ||
|
|
||
| test("returns empty array for template with no placeholders", () => { | ||
| expect(Command.hints("No placeholders here")).toEqual([]) | ||
| }) | ||
|
|
||
| test("returns empty array for empty string", () => { | ||
| expect(Command.hints("")).toEqual([]) | ||
| }) | ||
|
|
||
| test("does not extract $SOMETHING_ELSE as a hint", () => { | ||
| // Only $N and $ARGUMENTS should be extracted | ||
| expect(Command.hints("$FOO $BAR")).toEqual([]) | ||
| }) | ||
|
|
||
| test("handles template with only $ARGUMENTS", () => { | ||
| expect(Command.hints("$ARGUMENTS")).toEqual(["$ARGUMENTS"]) | ||
| }) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| import { describe, test, expect } from "bun:test" | ||
| import { BatchTool } from "../../src/tool/batch" | ||
|
|
||
| // BatchTool is a Tool.Info object; call .init() to get schema + helpers. | ||
| async function getToolInfo() { | ||
| return BatchTool.init() | ||
| } | ||
|
|
||
| describe("BatchTool: schema validation", () => { | ||
| test("rejects empty tool_calls array", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ tool_calls: [] }) | ||
| expect(result.success).toBe(false) | ||
| }) | ||
|
|
||
| test("accepts single tool call", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [{ tool: "read", parameters: { file_path: "/tmp/x" } }], | ||
| }) | ||
| expect(result.success).toBe(true) | ||
| }) | ||
|
|
||
| test("accepts multiple tool calls", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [ | ||
| { tool: "read", parameters: { file_path: "/tmp/a" } }, | ||
| { tool: "grep", parameters: { pattern: "foo" } }, | ||
| ], | ||
| }) | ||
| expect(result.success).toBe(true) | ||
| }) | ||
|
|
||
| test("rejects tool call without tool name", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [{ parameters: { file_path: "/tmp/x" } }], | ||
| }) | ||
| expect(result.success).toBe(false) | ||
| }) | ||
|
|
||
| test("rejects tool call without parameters object", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [{ tool: "read" }], | ||
| }) | ||
| expect(result.success).toBe(false) | ||
| }) | ||
|
|
||
| test("accepts tool call with empty parameters", async () => { | ||
| const tool = await getToolInfo() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [{ tool: "read", parameters: {} }], | ||
| }) | ||
| expect(result.success).toBe(true) | ||
| }) | ||
| }) | ||
|
|
||
| describe("BatchTool: formatValidationError", () => { | ||
| test("formatValidationError is defined", async () => { | ||
| const tool = await getToolInfo() | ||
| expect(tool.formatValidationError).toBeDefined() | ||
| }) | ||
|
|
||
| test("produces readable error message for empty array", async () => { | ||
| const tool = await getToolInfo() | ||
| expect(tool.formatValidationError).toBeDefined() | ||
| const result = tool.parameters.safeParse({ tool_calls: [] }) | ||
| expect(result.success).toBe(false) | ||
| if (!result.success) { | ||
| const msg = tool.formatValidationError!(result.error) | ||
| expect(msg).toContain("Invalid parameters for tool 'batch'") | ||
| expect(msg).toContain("Expected payload format") | ||
| } | ||
| }) | ||
|
|
||
| test("includes field path in type error", async () => { | ||
| const tool = await getToolInfo() | ||
| expect(tool.formatValidationError).toBeDefined() | ||
| const result = tool.parameters.safeParse({ | ||
| tool_calls: [{ tool: 123, parameters: {} }], | ||
| }) | ||
| expect(result.success).toBe(false) | ||
| if (!result.success) { | ||
| const msg = tool.formatValidationError!(result.error) | ||
| expect(msg).toContain("tool_calls") | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| describe("BatchTool: DISALLOWED set enforcement", () => { | ||
| // The DISALLOWED set prevents recursive batch-in-batch calls. | ||
| // This is a critical safety mechanism — if the LLM can batch the batch tool, | ||
| // it creates infinite recursion. | ||
| // We verify the source code's DISALLOWED set by checking the module exports. | ||
| test("batch tool id is 'batch'", () => { | ||
| expect(BatchTool.id).toBe("batch") | ||
| }) | ||
|
|
||
| // The 25-call cap and DISALLOWED enforcement happen inside execute(), | ||
| // which requires a full Session context. We verify the schema allows | ||
| // up to 25+ items at parse time (the cap is enforced at runtime). | ||
| test("schema accepts 25 tool calls (runtime cap is in execute)", async () => { | ||
| const tool = await getToolInfo() | ||
| const calls = Array.from({ length: 25 }, (_, i) => ({ | ||
| tool: `tool_${i}`, | ||
| parameters: {}, | ||
| })) | ||
| const result = tool.parameters.safeParse({ tool_calls: calls }) | ||
| expect(result.success).toBe(true) | ||
| }) | ||
|
|
||
| test("schema accepts 26+ tool calls (runtime slices to 25)", async () => { | ||
| const tool = await getToolInfo() | ||
| const calls = Array.from({ length: 30 }, (_, i) => ({ | ||
| tool: `tool_${i}`, | ||
| parameters: {}, | ||
| })) | ||
| const result = tool.parameters.safeParse({ tool_calls: calls }) | ||
| // Schema allows it — the 25-cap is enforced in execute() | ||
| expect(result.success).toBe(true) | ||
| }) | ||
| }) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix contradictory sorting description in this test.
Line 13/Line 14 describe sorting ambiguously/incorrectly, while Line 15 correctly asserts lexicographic behavior (
"$10"before"$2"). Please align the wording with the assertion.Suggested wording fix
📝 Committable suggestion
🤖 Prompt for AI Agents