Visitar URL original
feat(cli): add calls-tool and no-tool-call assertions (#412) by bhushan4work · Pull Request #446 · codercops/chatcops · GitHub
Skip to content

feat(cli): add calls-tool and no-tool-call assertions (#412) - #446

Merged
anurag629 merged 3 commits into
codercops:devfrom
bhushan4work:fix#412
Oct 29, 2026
Merged

anurag629 merged 3 commits into
codercops:devfrom
bhushan4work:fix#412

Conversation

@bhushan4work

Copy link
Copy Markdown
Contributor

What does this change?

Adds calls-tool and no-tool-call assertions to verify tool names, inputs, partial nested argument matches, and unexpected tool usage. Includes unit and end-to-end tests, documentation, and a minor changeset.

Closes #412

How I tested this

  • pnpm test packages/cli/tests/test-command.test.ts passed (30/30 tests).
  • pnpm test packages/cli passed (80/80 tests).
  • pnpm test:coverage passed (1,789 tests, all coverage thresholds met).
  • pnpm typecheck passed.
  • pnpm lint passed.
  • pnpm docs:check passed (141 TypeScript blocks checked).

AI assistance

none

Checklist

  • This PR targets dev
  • I was assigned the linked issue
  • pnpm -r build, pnpm test and pnpm -r typecheck pass locally
  • Tests added or updated where it makes sense
  • For a bug fix: the new test fails without my change
  • Changeset added (pnpm changeset) if this changes core, widget or server
  • pnpm api run and api/ committed if an exported type or option changed
  • Docs updated if behavior or options changed

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @bhushan4work, this does what #412 asks for. Matching is right, bad values throw so not- can't turn them into a pass, and the failure reasons are useful. With current dev merged in, build, typecheck, test:coverage, lint (same as dev), api:check, publint and docs:check pass. Three small things before I merge:

  1. packages/cli/src/test/assertions/tools.ts:7-23: exactMatch is never fully run by the tests. Coverage shows lines 10-11 and 14-19 aren't hit, because the only array case (test-command.test.ts:558) holds strings. That's the code where partial and exact matching differ, so please add a case with an object inside an array, for example args: { items: [{ sku: 1 }] } against an input of { items: [{ sku: 1, qty: 2 }] }, and expect it to fail.

  2. While you're there, exactMatch can be isDeepStrictEqual from node:util (the CLI only runs on Node, and for JSON values the result is the same). The array branch in matchesPartial at tools.ts:27-30 repeats the one in exactMatch, so it can become if (Array.isArray(expected)) return isDeepStrictEqual(expected, actual);. That removes about 15 lines, the uncovered ones with them.

  3. tools.ts:102: the reason reads the turn called tools (it called lookup_order({"id":"A-1"})), which says "called" twice. Something like the turn called ${calls.map(formatCall).join(', ')} reads better. Update the expected strings at test-command.test.ts:633 and the not- one to match.

A minor one, take it or leave it: turn.toolCalls ?? [] at tools.ts:80 and :97 isn't needed, since toolCalls is always set on TurnResult.

Heads up: #430 edits the import lines next to yours in assertions/index.ts, so whichever of the two lands second will need a quick rebase.

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @bhushan4work, all of it is in: isDeepStrictEqual for arrays, the object-in-array cases both ways, the reworded reason, and the ?? [] gone. tools.ts is at 98% statements and 96% branches now. With current dev merged in, build, typecheck, test:coverage, lint (same as dev), api:check and docs:check pass. Merging once CI is green, and then #430 will need the small rebase in assertions/index.ts.

@anurag629
anurag629 merged commit 18a77bd into codercops:dev Oct 29, 2026
5 checks passed
@anurag629 anurag629 added the hacktoberfest-accepted PR counts for Hacktoberfest label Oct 29, 2026
@anurag629

Copy link
Copy Markdown
Member

Merged, thanks @bhushan4work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest-accepted PR counts for Hacktoberfest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chatcops test: calls-tool and no-tool-call assertions

2 participants