[issues/569] Add CMD_BIND_TO_CUSTOM_AI_BY_ID for programmatic custom AI binding#597
Open
couimet wants to merge 3 commits into
Open
[issues/569] Add CMD_BIND_TO_CUSTOM_AI_BY_ID for programmatic custom AI binding#597couimet wants to merge 3 commits into
CMD_BIND_TO_CUSTOM_AI_BY_ID for programmatic custom AI binding#597couimet wants to merge 3 commits into
Conversation
…m AI binding
## Summary
Adds `rangelink.bindToCustomAiById` command accepting `{ extensionId: string }` to programmatically bind custom AI assistants. Eliminates the picker step, enabling fully automated integration tests for all custom AI paste flows. Previously 6 custom AI tests required human-in-the-loop for binding and manual paste verification.
## Changes
- New `createBindToCustomAiByIdCommand.ts` — resolves extensionId to destination kind via `resolveKindByExtensionId()` and delegates to `PasteDestinationManager.bind()`
- `resolveKindByExtensionId()` in `destinationBuilders.ts` maps extension IDs to built-in or custom AI kinds
- Wiring through `wireSubscriptions.ts`, `createWiringServices.ts`, `commandIds.ts`, `package.json`
- 11 unit tests covering undefined/null/string/array args, missing/empty/number extensionId, unknown extensionId, built-in bind, custom bind, bind error passthrough
- 6 custom AI integration tests converted from `[assisted]` to fully automated using the new command
- 2 manual-paste tests (Tier 3, fallback) now verify toast, clipboard state, and logs without requiring human Cmd+V
- QA YAML: 6 entries updated from `automated: assisted` to `automated: true`
- Contract tests updated for new command and command-palette entry counts
- Documentation: CHANGELOG not needed (dev-facing infrastructure); README not needed (no user-facing changes)
## Test Plan
- [x] All 1947 unit tests pass
- [x] 154/156 automated integration tests pass (2 pre-existing Cursor-specific clipboard-preservation timeouts)
- [x] 11 new unit tests for createBindToCustomAiByIdCommand
- [x] 6 integration tests reworked from human-in-the-loop to fully automated
## Related
- Closes #569
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds rangelink.bindToCustomAiById: a hidden VS Code command that resolves extension IDs to built-in or custom assistant destination kinds, validates inputs, and delegates to the existing bind flow. Wiring, constants, and tests added; integration tests and QA YAML updated to replace picker-driven assisted flows with programmatic command invocation and log-based clipboard assertions. ChangesProgrammatic Custom AI Binding Command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
A new command 'Bind to Custom AI by ID' was added, which requires new test cases to verify its functionality. Suggested test cases:
Generated by QA Gap Check (GPT-4o-mini via GitHub Models) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts (1)
356-377:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd at least one log assertion in this behavior test.
This test verifies behavior (clipboard preservation) but currently has no logger assertion, which weakens diagnosability when it fails.
As per coding guidelines, "Include logger assertions in tests that verify method behavior - logging provides critical visibility for debugging. Consolidate with behavior tests, do not create separate logging tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts` around lines 356 - 377, Add a logger assertion to this test: after calling rangelink.copyLinkWithRelativePath and ss.settle(), use the existing getLogCapture() marker "before-tier1-clip" to fetch logs produced since that mark and assert that the expected diagnostic log entry appears (for example the clipboard-preservation or copy action message emitted by the extension). Locate getLogCapture() / logCapture.mark('before-tier1-clip') and add a single assertion on logCapture.getEntriesSince(...) (or equivalent helper used elsewhere in tests) to verify the expected log message is present before calling assertClipboardRestored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`:
- Around line 268-270: The call to
vscode.commands.executeCommand(CMD_BIND_TO_CUSTOM_AI_BY_ID, {...}) ignores the
command's return value and can hide non-throwing error Results; update each
invocation (including the instances around lines 320-322, 360-362, 383-385,
438-440, 502-504) to capture the returned Result, assert that it indicates
success (or throw/assert with the contained error message when it indicates
failure) so the test fails on bind errors instead of silently succeeding due to
prior state.
In
`@packages/rangelink-vscode-extension/src/__tests__/commands/createBindToCustomAiByIdCommand.test.ts`:
- Around line 167-194: The test is missing assertions that the logger was called
for this new behavior path; update the test that uses
createBindToCustomAiByIdCommand (and the related handler/result) to assert
mockLogger was invoked with the appropriate messages when
destinationBuilders.resolveKindByExtensionId is called and when mockManager.bind
succeeds: add expectations against mockLogger (e.g., mockLogger.info/warn/error
or whichever methods your mock exposes) after calling handler({ extensionId })
to verify a resolve/log entry and a bind/success log entry, referencing
resolveKindByExtensionId and mockManager.bind in your assertions so the test
verifies both behavior and corresponding logs.
- Line 212: The test currently uses a generic expect(result).toBeErr(); —
replace this with the project's Result matcher to assert the actual error using
expect(result).toBeErrWith(...) so the test verifies the error payload; locate
the failing assertion on the "result" variable in
createBindToCustomAiByIdCommand.test.ts and assert the specific error value (or
construct/constant used elsewhere in the test) with toBeErrWith to conform to
the project's Result matcher contract.
In `@packages/rangelink-vscode-extension/src/__tests__/wireSubscriptions.test.ts`:
- Around line 360-371: Add an assertion that the logger was called for this
flow: spy on services.logger (e.g., jest.spyOn(services.logger, 'info') or the
appropriate log level used in the implementation) before invoking
registrar.getHandler(CMD_BIND_TO_CUSTOM_AI_BY_ID), then after the handler runs
assert that the logger was called with a message indicating the extension
resolution and/or bind action (referencing resolveKindByExtensionId,
CMD_BIND_TO_CUSTOM_AI_BY_ID, and destinationManager.bind) and that the
resolveSpy was called as already asserted; keep the logger assertion combined in
this same test rather than in a separate logging test.
In
`@packages/rangelink-vscode-extension/src/errors/RangeLinkExtensionErrorCodes.ts`:
- Line 22: The exported enum value CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID does not
match the required contract (CUSTOM_AI_NOT_FOUND); update the
RangeLinkExtensionErrorCodes export to use CUSTOM_AI_NOT_FOUND instead, or add
CUSTOM_AI_NOT_FOUND as a stable alias that maps to the same string while keeping
CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID for backward compatibility; modify the enum
entry in RangeLinkExtensionErrorCodes (and any related references) so consumers
see the canonical CUSTOM_AI_NOT_FOUND identifier.
---
Outside diff comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`:
- Around line 356-377: Add a logger assertion to this test: after calling
rangelink.copyLinkWithRelativePath and ss.settle(), use the existing
getLogCapture() marker "before-tier1-clip" to fetch logs produced since that
mark and assert that the expected diagnostic log entry appears (for example the
clipboard-preservation or copy action message emitted by the extension). Locate
getLogCapture() / logCapture.mark('before-tier1-clip') and add a single
assertion on logCapture.getEntriesSince(...) (or equivalent helper used
elsewhere in tests) to verify the expected log message is present before calling
assertClipboardRestored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6e664d8-eeca-4678-a8df-03b340511b96
📒 Files selected for processing (15)
packages/rangelink-vscode-extension/package.jsonpackages/rangelink-vscode-extension/qa/qa-test-cases-v1.1.0.yamlpackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__tests__/commands/createBindToCustomAiByIdCommand.test.tspackages/rangelink-vscode-extension/src/__tests__/constants/packageJsonContracts.test.tspackages/rangelink-vscode-extension/src/__tests__/destinations/destinationBuilders.test.tspackages/rangelink-vscode-extension/src/__tests__/extension.test.tspackages/rangelink-vscode-extension/src/__tests__/helpers/createMockWiringServices.tspackages/rangelink-vscode-extension/src/__tests__/wireSubscriptions.test.tspackages/rangelink-vscode-extension/src/commands/createBindToCustomAiByIdCommand.tspackages/rangelink-vscode-extension/src/constants/commandIds.tspackages/rangelink-vscode-extension/src/createWiringServices.tspackages/rangelink-vscode-extension/src/destinations/destinationBuilders.tspackages/rangelink-vscode-extension/src/errors/RangeLinkExtensionErrorCodes.tspackages/rangelink-vscode-extension/src/wireSubscriptions.ts
Comment on lines
+268
to
+270
| await vscode.commands.executeCommand(CMD_BIND_TO_CUSTOM_AI_BY_ID, { | ||
| extensionId: 'rangelink.dummy-ai-extension', | ||
| }); |
Contributor
There was a problem hiding this comment.
Assert bind command success explicitly in each test.
These executeCommand(CMD_BIND_TO_CUSTOM_AI_BY_ID, ...) calls ignore the command result. If binding returns an error Result (without throwing), a previously bound destination can mask failures and create false positives.
Also applies to: 320-322, 360-362, 383-385, 438-440, 502-504
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.ts`
around lines 268 - 270, The call to
vscode.commands.executeCommand(CMD_BIND_TO_CUSTOM_AI_BY_ID, {...}) ignores the
command's return value and can hide non-throwing error Results; update each
invocation (including the instances around lines 320-322, 360-362, 383-385,
438-440, 502-504) to capture the returned Result, assert that it indicates
success (or throw/assert with the contained error message when it indicates
failure) so the test fails on bind errors instead of silently succeeding due to
prior state.
| BOOKMARK_NOT_FOUND = 'BOOKMARK_NOT_FOUND', | ||
| BOOKMARK_SAVE_FAILED = 'BOOKMARK_SAVE_FAILED', | ||
| BOOKMARK_STORE_NOT_AVAILABLE = 'BOOKMARK_STORE_NOT_AVAILABLE', | ||
| CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID = 'CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID', |
Contributor
There was a problem hiding this comment.
Use the required error-code contract name.
At Line 22, the new code is CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID, but the stated contract for this command is CUSTOM_AI_NOT_FOUND. This mismatch can break consumers that branch on the documented error code. Please align the enum/exported code to the required contract (or add a stable alias if backward compatibility is needed).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/rangelink-vscode-extension/src/errors/RangeLinkExtensionErrorCodes.ts`
at line 22, The exported enum value CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID does not
match the required contract (CUSTOM_AI_NOT_FOUND); update the
RangeLinkExtensionErrorCodes export to use CUSTOM_AI_NOT_FOUND instead, or add
CUSTOM_AI_NOT_FOUND as a stable alias that maps to the same string while keeping
CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID for backward compatibility; modify the enum
entry in RangeLinkExtensionErrorCodes (and any related references) so consumers
see the canonical CUSTOM_AI_NOT_FOUND identifier.
✅ CI / Integration Tests (with extensions) — run summary
|
This comment has been minimized.
This comment has been minimized.
Adds missing logger assertions to unit tests and clipboard preservation log assertions to 22 integration tests across four files. Replaces a bare toBeErr() with toBeRangeLinkExtensionErrorErr. Benefits: - Custom-kind bind test now verifies debug logging like its built-in counterpart - Error passthrough test now asserts the specific error code, message, and functionName - Wire delegation test now verifies the debug log emitted by createBindToCustomAiByIdCommand - All 22 integration tests that call assertClipboardRestored now verify both "Clipboard saved" and "Clipboard restored" log entries, proving the full preservation pipeline ran - clipboard-preservation-009 correctly asserts the NEGATIVE case (picker dismissed, no operation ran, so no preservation logs expected) Ignored Feedback: - Asserting executeCommand return values in integration tests: the suite verifies behaviour through log capture, not command return values. Adding return-value assertions would be inconsistent with every other executeCommand call in the test suite. - Renaming CUSTOM_AI_NOT_FOUND_BY_EXTENSION_ID back to CUSTOM_AI_NOT_FOUND: the rename was intentional per prior review — the original name was too vague. This is a new error code on this branch with no existing consumers. Ref: #597 (review)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/rangelink-vscode-extension/src/__integration-tests__/helpers/clipboardHelpers.ts`:
- Around line 37-44: The current assertClipboardPreservationRan checks only for
presence of "Clipboard saved" and "Clipboard restored" in lines from
logCapture.getLinesSince(markName); update it to also assert the restored entry
occurs after the saved entry by locating their indices in the lines array (use
lines.findIndex for the savedLine and restoredLine) and assert restoredIndex >
savedIndex, including operationLabel in the failure message; keep existing
presence checks (savedLine/restoredLine) but replace or augment them with the
index order check so assertClipboardPreservationRan verifies correct ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4939dc1-7a71-4a2f-98a1-4156b80fa233
📒 Files selected for processing (7)
packages/rangelink-vscode-extension/src/__integration-tests__/helpers/clipboardHelpers.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/builtInAiAssistants.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/clipboardPreservation.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/customAiAssistants.test.tspackages/rangelink-vscode-extension/src/__integration-tests__/suite/dirtyBufferWarning.test.tspackages/rangelink-vscode-extension/src/__tests__/commands/createBindToCustomAiByIdCommand.test.tspackages/rangelink-vscode-extension/src/__tests__/wireSubscriptions.test.ts
`assertClipboardPreservationRan` now verifies "Clipboard restored" appears after "Clipboard saved" in the log, not just that both exist. Presence-only assertions could pass if a restored line from a prior operation lingered before the saved line from the current operation. The ordering check proves the save→restore sequence ran within the same operation window. Ref: #597 (review)
❌ CI / Integration Tests (with extensions) — run summary
To re-run failed tests: |
This comment has been minimized.
This comment has been minimized.
❌ CI / Integration Tests (with extensions) — run summary
To re-run failed tests: |
✅ CI / Test & Validate — run summary
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds
rangelink.bindToCustomAiByIdcommand accepting{ extensionId: string }to programmatically bind custom AI assistants. Eliminates the picker step, enabling fully automated integration tests for all custom AI paste flows. Previously 6 custom AI tests required human-in-the-loop for binding and manual paste verification.Changes
createBindToCustomAiByIdCommand.ts— resolves extensionId to destination kind viaresolveKindByExtensionId()and delegates toPasteDestinationManager.bind()resolveKindByExtensionId()indestinationBuilders.tsmaps extension IDs to built-in or custom AI kindswireSubscriptions.ts,createWiringServices.ts,commandIds.ts,package.json[assisted]to fully automated using the new commandautomated: assistedtoautomated: trueTest Plan
Related
Summary by CodeRabbit
New Features
Tests