-
Notifications
You must be signed in to change notification settings - Fork 12
[CI-4659] Rename Assistant Module to Agent #386
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
Conversation
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.
Pull Request Overview
This PR refactors the Assistant module to Agent, providing a modern interface while maintaining backwards compatibility. The primary purpose is to rename the "Assistant" functionality to "Agent" while ensuring existing integrations continue to work seamlessly.
Key Changes
- Introduction of a new
Agentmodule with updated function names (e.g.,trackAgentSubmit) - Assistant module converted to a wrapper that extends Agent and provides deprecated aliases
- All tracker methods renamed from
trackAssistant*totrackAgent*with backwards-compatible wrappers - Updated service URLs to use
agentServiceUrlwithassistantServiceUrlfallback
Reviewed Changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/modules/tracker.js | Renamed tracking methods from assistant to agent and added deprecated backward-compatible wrappers |
| src/modules/assistant.js | Refactored to extend Agent class while maintaining deprecated interface |
| src/modules/agent.js | New module containing core agent functionality previously in assistant.js |
| src/constructorio.js | Added agent service URL configuration and instantiated new Agent module |
| spec/src/modules/tracker.js | Added comprehensive test coverage for new agent tracking methods |
| spec/src/modules/agent.js | Added test suite for the new Agent module functionality |
esezen
left a comment
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.
Great job! Just left 2 very simple comments. Let's also get another review from someone from the team
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.
Left some totally optional comments. Thank you for working on this. It is looking great!
esezen
left a comment
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.
@evanyan13 the changes look good but some tests are failing both in CI and locally. Do you mind taking a look why?
Hi @esezen Thank you for raising the issues!
Also, I'm facing a problem on the test buildsI. Here are my observations:
|
Can you please check with ASA team? It's not a blocker for this PR and I will merge this PR today but let's figure out the next steps there.
We have some flaky tests we need to address but not related to your PR, all good! |
esezen
left a comment
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.
LGTM!
PR Type
What kind of change does this PR introduce?
✅ Definition of done:
agentimport works identically to currentassistantassistantremains as a deprecated alias.Tasks
Move existing functions to
agentmoduleassistantfunctions toagentImport agent functions from
assistantmoduleRename all
assistantfunctions toagentintrackermoduleAdd tests for
agentmodule in/specassistantmodule to support backwards compatibilityUpdate README, docs, and TypeScript defs to reference
agentNote:
domain=assistantas a query param to the endpoint: https://agent.cnstrc.com/v1/intent/ now.domain=assistant– this is customer/integration-specific and may be any arbitrary value to distinguish multiple setups for the same ac_key.